From mboxrd@z Thu Jan 1 00:00:00 1970 From: Philipp Zabel Subject: Re: [PATCH v3 2/2] drm/etnaviv: Fix driver unregistering Date: Wed, 27 Jun 2018 14:33:08 +0200 Message-ID: <1530102788.3419.12.camel@pengutronix.de> References: <1529954173-7728-1-git-send-email-festevam@gmail.com> <1529954173-7728-2-git-send-email-festevam@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1529954173-7728-2-git-send-email-festevam@gmail.com> Sender: stable-owner@vger.kernel.org To: Fabio Estevam , airlied@linux.ie Cc: etnaviv@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux@armlinux.org.uk, christian.gmeiner@gmail.com, stable@vger.kernel.org, kernel@pengutronix.de, linux+etnaviv@armlinux.org.uk, Fabio Estevam , l.stach@pengutronix.de List-Id: dri-devel@lists.freedesktop.org On Mon, 2018-06-25 at 16:16 -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:00000000 > [] (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 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 regards Philipp