[PATCH v3 07/11] drm: omapdrm: Register omapdrm platform device in omapdss driver

[Tomi Valkeinen]

Hi Laurent,


Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

On 11/08/17 16:49, Laurent Pinchart wrote:
> The omapdrm platform device is a virtual device created for the sole
> purpose of handling the omapdss/omapdrm driver split. It should
> eventually be removed. As a first step to ease refactoring move its
> registration from platform code to driver code.
> 
> The omapdrm driver name must be changed internally to avoid probing both
> the device registered in platform code and the device registered in the
> omapdss driver, as that would otherwise break bisection.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Reviewed-by: Tomi Valkeinen <tomi.valkeinen at ti.com>
> ---
> Changes since v1:
> 
> - Drop the CONFIG_DRM_OMAP conditional compilation
> - Unregister the platform device at exit time
> ---
>  drivers/gpu/drm/omapdrm/dss/core.c | 15 +++++++++++++++
>  drivers/gpu/drm/omapdrm/omap_drv.c |  2 +-
>  2 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/core.c b/drivers/gpu/drm/omapdrm/dss/core.c
> index 4dabe32c7098..8678d8b4efce 100644
> --- a/drivers/gpu/drm/omapdrm/dss/core.c
> +++ b/drivers/gpu/drm/omapdrm/dss/core.c
> @@ -22,6 +22,7 @@
>  
>  #define DSS_SUBSYS_NAME "CORE"
>  
> +#include <linux/dma-mapping.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/clk.h>
> @@ -103,6 +104,14 @@ static void (*dss_output_drv_unreg_funcs[])(void) = {
>  	dss_uninit_platform_driver,
>  };
>  
> +static struct platform_device omap_drm_device = {
> +	.dev = {
> +		.coherent_dma_mask = DMA_BIT_MASK(32),
> +	},
> +	.name = "omapdrm_",
> +	.id = 0,
> +};
> +
>  static int __init omap_dss_init(void)
>  {
>  	int r;
> @@ -118,6 +127,10 @@ static int __init omap_dss_init(void)
>  			goto err_reg;
>  	}
>  
> +	r = platform_device_register(&omap_drm_device);
> +	if (r)
> +		goto err_reg;
> +
>  	return 0;
>  
>  err_reg:
> @@ -135,6 +148,8 @@ static void __exit omap_dss_exit(void)
>  {
>  	int i;
>  
> +	platform_device_unregister(&omap_drm_device);
> +
>  	for (i = 0; i < ARRAY_SIZE(dss_output_drv_unreg_funcs); ++i)
>  		dss_output_drv_unreg_funcs[i]();
>  
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
> index 022029ea6972..9ab22e0c0b84 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> @@ -734,7 +734,7 @@ static SIMPLE_DEV_PM_OPS(omapdrm_pm_ops, omap_drm_suspend, omap_drm_resume);
>  
>  static struct platform_driver pdev = {
>  	.driver = {
> -		.name = DRIVER_NAME,
> +		.name = "omapdrm_",
>  		.pm = &omapdrm_pm_ops,
>  	},
>  	.probe = pdev_probe,
 
When unloading:

[   40.099847] WARNING: CPU: 1 PID: 247 at drivers/base/core.c:818 device_release+0x8c/0x9c
[   40.107992] Device 'omapdrm_.0' does not have a release() function, it is broken and must be fixed.
[   40.119120] Modules linked in: cfbfillrect cfbimgblt cfbcopyarea omapdss(-) omapdss_base [last unloaded: snd_soc_omap_hdmi_audio]
[   40.131311] CPU: 1 PID: 247 Comm: rmmod Not tainted 4.13.0-rc3-00056-g2e3e69f7fd0d #26
[   40.139279] Hardware name: Generic DRA74X (Flattened Device Tree)
[   40.145412] Backtrace: 
[   40.147903] [<c010dc10>] (dump_backtrace) from [<c010debc>] (show_stack+0x18/0x1c)
[   40.155521]  r7:c0ed4910 r6:60080013 r5:00000000 r4:c0ed4910
[   40.161241] [<c010dea4>] (show_stack) from [<c09288c4>] (dump_stack+0xa8/0xdc)
r[   40.168517] [<c092881c>] (dump_stack) from [<c013bd80>] (__warn+0xdc/0x108)
[   40.175599]  r9:c05caef0 r8:00000332 r7:00000009 r6:c0c2eb78 r5:00000000 r4:ed0ebe98
[   40.183396] [<c013bca4>] (__warn) from [<c013bde8>] (warn_slowpath_fmt+0x3c/0x44)
[   40.190926]  r9:ed0ea000 r8:c0108c04 r7:ee2415c0 r6:ed10da80 r5:bf041010 r4:c0c2ebbc
[   40.198722] [<c013bdb0>] (warn_slowpath_fmt) from [<c05caef0>] (device_release+0x8c/0x9c)
[   40.206947]  r3:ee2415c0 r2:c0c2ebbc
[   40.210550]  r4:bf041018
[   40.213115] [<c05cae64>] (device_release) from [<c092ce94>] (kobject_put+0xf8/0x228)
[   40.220905]  r7:ee2415c0 r6:c0eec2b0 r5:c0ea1870 r4:bf041018
        [   40.226616] [<c092cd9c>] (kobject_put) from [<c05cb3d8>] (put_device+0x1c/0x20)
[   40.234050]  r7:00000081 r6:0003cd4c r5:00000800 r4:bf041000
[   40.239758] [<c05cb3bc>] (put_device) from [<c05d2dd8>] (platform_device_unregister+0x24/0x28)
[   40.248493] [<c05d2db4>] (platform_device_unregister) from [<bf032f1c>] (omap_dss_exit+0x14/0x3c [omapdss])
[   40.258291]  r5:00000800 r4:bf042880
[   40.261961] [<bf032f08>] (omap_dss_exit [omapdss]) from [<c01ec8e4>] (SyS_delete_module+0x178/0x250)
[   40.271147]  r5:00000800 r4:bf042880
[   40.274759] [<c01ec76c>] (SyS_delete_module) from [<c0108a60>] (ret_fast_syscall+0x0/0x1c)
[   40.283074]  r6:beac9c10 r5:0003cd10 r4:beac9dfc
[   40.289781] ---[ end trace 197bbc25cd50eb0f ]---

I think we didn't encounter this earlier, as the device was never freed.

 Tomi