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(_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(_platform_driver);
> + platform_driver_unregister(_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(_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(_platform_driver);
+ platform_driver_unregister(_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
