Re: [PATCH v2] drm/etnaviv: Fix driver unregistering
Hi Philipp, On Mon, Jun 25, 2018 at 5:02 AM, Philipp Zabel wrote: >> +static struct platform_device *etnaviv_drm; >> + >> static int __init etnaviv_init(void) >> { >> int ret; >> @@ -654,7 +656,8 @@ static int __init etnaviv_init(void) >> if (!of_device_is_available(np)) >> continue; >> >> - platform_device_register_simple("etnaviv", -1, NULL, 0); >> + etnaviv_drm = platform_device_register_simple("etnaviv", -1, >> + NULL, 0); > > If there are multiple vivante,gc device nodes, this overwrites > etnaviv_drm with each additional device after the first one. In fact there are multiple vivante,gc device nodes, but platform_device_register_simple() is only called once as expected. I will make this clear and will also check for the error code on platform_device_register_simple(). Thanks ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/etnaviv: Fix driver unregistering
Hi Fabio, On Sun, 2018-06-24 at 19:40 -0300, Fabio Estevam wrote: > From: Fabio Estevam > > Russell King reported: > > "When removing and reloading the etnaviv module, the following splat > occurs: > > sysfs: cannot create duplicate filename '/devices/platform/etnaviv' > CPU: 0 PID: 1471 Comm: modprobe Not tainted 4.17.0+ #1608 > Hardware name: Marvell Dove (Cubox) > Backtrace: > [] (dump_backtrace) from [] (show_stack+0x18/0x1c) > r6:ef033e38 r5:ee07b340 r4:edb9d000 r3: > [] (show_stack) from [] (dump_stack+0x20/0x28) > [] (dump_stack) from [] (sysfs_warn_dup+0x5c/0x70) > [] (sysfs_warn_dup) from [] > (sysfs_create_dir_ns+0x90/0x98) > ..." > > Commit 246774d17fc0 ("drm/etnaviv: remove the need for a gpu-subsystem > DT node") introduced DRM registration via > platform_device_register_simple(), but missed to call > platform_device_unregister() inside etnaviv_exit(). > > Fix the problem by calling platform_device_unregister() inside > etnaviv_exit(). While at it, also rearrange the function calls > in the exit path to make them happen in the opposite order of > registration. > > Tested on a imx6-sabresd board. > > Cc: > Fixes: 246774d17fc0 ("drm/etnaviv: remove the need for a gpu-subsystem DT > node") > Reported-by: Russell King > Signed-off-by: Fabio Estevam > --- > Changes since v1: > - Make the exit path symmetrical to the init path by calling > platform_device_unregister() inside for_each_compatible_node() > > drivers/gpu/drm/etnaviv/etnaviv_drv.c | 18 -- > 1 file changed, 16 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c > b/drivers/gpu/drm/etnaviv/etnaviv_drv.c > index e5013a9..626ad8b 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c > +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c > @@ -631,6 +631,8 @@ static struct platform_driver etnaviv_platform_driver = { > }, > }; > > +static struct platform_device *etnaviv_drm; > + > static int __init etnaviv_init(void) > { > int ret; > @@ -654,7 +656,8 @@ static int __init etnaviv_init(void) > if (!of_device_is_available(np)) > continue; > > - platform_device_register_simple("etnaviv", -1, NULL, 0); > + etnaviv_drm = platform_device_register_simple("etnaviv", -1, > + NULL, 0); If there are multiple vivante,gc device nodes, this overwrites etnaviv_drm with each additional device after the first one. Also, it would be better to store NULL instead of the error value. > of_node_put(np); > break; > } > @@ -665,8 +668,19 @@ module_init(etnaviv_init); > > static void __exit etnaviv_exit(void) > { > - platform_driver_unregister(&etnaviv_gpu_driver); > + struct device_node *np; > + > + for_each_compatible_node(np, NULL, "vivante,gc") { > + if (!of_device_is_available(np)) > + continue; > + > + platform_device_unregister(etnaviv_drm); If there are multiple vivante,gc nodes, this will try to unregister the last device multiple times. Also, this will break if the pointer contains an error value. > + of_node_put(np); > + break; > + } > + > platform_driver_unregister(&etnaviv_platform_driver); > + platform_driver_unregister(&etnaviv_gpu_driver); > } > module_exit(etnaviv_exit); > regards Philipp ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm/etnaviv: Fix driver unregistering
From: Fabio Estevam Russell King reported: "When removing and reloading the etnaviv module, the following splat occurs: sysfs: cannot create duplicate filename '/devices/platform/etnaviv' CPU: 0 PID: 1471 Comm: modprobe Not tainted 4.17.0+ #1608 Hardware name: Marvell Dove (Cubox) Backtrace: [] (dump_backtrace) from [] (show_stack+0x18/0x1c) r6:ef033e38 r5:ee07b340 r4:edb9d000 r3: [] (show_stack) from [] (dump_stack+0x20/0x28) [] (dump_stack) from [] (sysfs_warn_dup+0x5c/0x70) [] (sysfs_warn_dup) from [] (sysfs_create_dir_ns+0x90/0x98) ..." Commit 246774d17fc0 ("drm/etnaviv: remove the need for a gpu-subsystem DT node") introduced DRM registration via platform_device_register_simple(), but missed to call platform_device_unregister() inside etnaviv_exit(). Fix the problem by calling platform_device_unregister() inside etnaviv_exit(). While at it, also rearrange the function calls in the exit path to make them happen in the opposite order of registration. Tested on a imx6-sabresd board. Cc: Fixes: 246774d17fc0 ("drm/etnaviv: remove the need for a gpu-subsystem DT node") Reported-by: Russell King Signed-off-by: Fabio Estevam --- Changes since v1: - Make the exit path symmetrical to the init path by calling platform_device_unregister() inside for_each_compatible_node() drivers/gpu/drm/etnaviv/etnaviv_drv.c | 18 -- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c index e5013a9..626ad8b 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c @@ -631,6 +631,8 @@ static struct platform_driver etnaviv_platform_driver = { }, }; +static struct platform_device *etnaviv_drm; + static int __init etnaviv_init(void) { int ret; @@ -654,7 +656,8 @@ static int __init etnaviv_init(void) if (!of_device_is_available(np)) continue; - platform_device_register_simple("etnaviv", -1, NULL, 0); + etnaviv_drm = platform_device_register_simple("etnaviv", -1, + NULL, 0); of_node_put(np); break; } @@ -665,8 +668,19 @@ module_init(etnaviv_init); static void __exit etnaviv_exit(void) { - platform_driver_unregister(&etnaviv_gpu_driver); + struct device_node *np; + + for_each_compatible_node(np, NULL, "vivante,gc") { + if (!of_device_is_available(np)) + continue; + + platform_device_unregister(etnaviv_drm); + of_node_put(np); + break; + } + platform_driver_unregister(&etnaviv_platform_driver); + platform_driver_unregister(&etnaviv_gpu_driver); } module_exit(etnaviv_exit); -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel