On Mon, 2018-06-25 at 16:16 -0300, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@xxxxxxx>
>
> 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@xxxxxxxxxxxxxxx>
> Fixes: 246774d17fc0 ("drm/etnaviv: remove the need for a gpu-subsystem DT node")
> Reported-by: Russell King <linux@xxxxxxxxxxxxxxx>
> Signed-off-by: Fabio Estevam <fabio.estevam@xxxxxxx>
> ---
> Changes since v2:
> - Add a 'available_gpu' bool variable to indicate that an available
> "vivante,gc" has been found
>
> drivers/gpu/drm/etnaviv/etnaviv_drv.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> index ac23c0f..9044bf9 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> @@ -632,6 +632,7 @@ static struct platform_driver etnaviv_platform_driver = {
> };
>
> static struct platform_device *etnaviv_drm;
> +static bool available_gpu;
This would not be necessary if etnaviv_drm weren't ever set to an error
value.
> static int __init etnaviv_init(void)
> {
> @@ -655,6 +656,7 @@ static int __init etnaviv_init(void)
> for_each_compatible_node(np, NULL, "vivante,gc") {
> if (!of_device_is_available(np))
> continue;
> + available_gpu = true;
> etnaviv_drm = platform_device_register_simple("etnaviv", -1,
> NULL, 0);
> if (IS_ERR(etnaviv_drm)) {
> @@ -677,8 +679,10 @@ module_init(etnaviv_init);
>
> static void __exit etnaviv_exit(void)
> {
> - platform_driver_unregister(&etnaviv_gpu_driver);
> + if (available_gpu)
> + platform_device_unregister(etnaviv_drm);
Both platform_device_del and platform_device_put (called by
platform_device_unregister) will gracefully degrade to a no-op if
etnaviv_drm == NULL, so this could just be:
platform_device_unregister(etnaviv_drm);
> platform_driver_unregister(&etnaviv_platform_driver);
> + platform_driver_unregister(&etnaviv_gpu_driver);
> }
> module_exit(etnaviv_exit);
I'd change the first patch and drop the available_gpu static variable,
but either way:
Reviewed-by: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>
regards
Philipp
