Hi Fabio,
On Sun, 2018-06-24 at 19:40 -0300, Fabio Estevam wrote:
From: Fabio Estevam fabio.estevam@nxp.com
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: [<c00157d4>] (dump_backtrace) from [<c0015b8c>] (show_stack+0x18/0x1c) r6:ef033e38 r5:ee07b340 r4:edb9d000 r3:00000000 [<c0015b74>] (show_stack) from [<c0620784>] (dump_stack+0x20/0x28) [<c0620764>] (dump_stack) from [<c01bcd24>] (sysfs_warn_dup+0x5c/0x70) [<c01bccc8>] (sysfs_warn_dup) from [<c01bce14>] (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: stable@vger.kernel.org Fixes: 246774d17fc0 ("drm/etnaviv: remove the need for a gpu-subsystem DT node") Reported-by: Russell King linux@armlinux.org.uk Signed-off-by: Fabio Estevam fabio.estevam@nxp.com
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