From: "Heiko Stübner" <heiko@sntech.de>
To: Sandy Huang <hjc@rock-chips.com>,
Andy Yan <andy.yan@rock-chips.com>,
Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
Maxime Ripard <mripard@kernel.org>,
Thomas Zimmermann <tzimmermann@suse.de>,
David Airlie <airlied@gmail.com>, Daniel Vetter <daniel@ffwll.ch>,
Sean Paul <seanpaul@chromium.org>,
Jeffy Chen <jeffy.chen@rock-chips.com>,
Andrzej Hajda <andrzej.hajda@intel.com>,
Mark Yao <markyao0591@gmail.com>,
Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
Cc: kernel@collabora.com, dri-devel@lists.freedesktop.org,
linux-arm-kernel@lists.infradead.org,
linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH RESEND] drm/rockchip: Unregister platform drivers in reverse order
Date: Thu, 15 Aug 2024 19:52:29 +0200 [thread overview]
Message-ID: <12056454.6ASCnIXroE@diego> (raw)
In-Reply-To: <80ffed99-7368-4f84-8dc9-4c980055e48d@collabora.com>
Am Donnerstag, 15. August 2024, 19:26:54 CEST schrieb Cristian Ciocaltea:
> On 8/15/24 5:21 PM, Heiko Stübner wrote:
> > Am Donnerstag, 8. August 2024, 13:58:02 CEST schrieb Cristian Ciocaltea:
> >> Move rockchip_drm_platform_driver unregistration after its sub-drivers,
> >> which ensures all drivers are unregistered in the reverse order used
> >> when they were registered.
> >
> > are you sure about that?
> >
> > I.e. currently rockchip_drm_init() does
> > platform_register_drivers(rockchip_sub_drivers, ...)
> > to register the sub-drivers and only after that registers the main
> > drm-platform-driver
> >
> > rockchip_drm_fini currently does the reverse of first unregistering the
> > main drm-platform-driver and after that unregistering the array of sub-
> > drivers.
> >
> >
> > So as it stands right now, rockchip_drm_fini does already do exactly the
> > reverse when de-registering.
>
> Indeed, somehow I overlooked this while debugging some module unloading
> issues. I guess it just felt more naturally to have the subdrivers
> unregistered first.
>
> Out of curiosity to see if there's a common pattern for handling this, I
> found that most drivers do indeed unregister the subdrivers before the main
> platform one, but weirdly enough, some of them do also keep the same order
> on registration, similarily to what this patch unintentionally does:
>
> drivers/power/supply/ab8500_charger.c
> drivers/gpu/drm/vc4/vc4_drv.c
> drivers/gpu/drm/mcde/mcde_drv.c
>
> Not sure if those are potential mistakes, or maybe it doesn't really matter?!
>
> Please let me know if you have a preference for it, and I'll update the
> patch accordingly, otherwise let's just ignore it altogether.
in theory it shouldn't matter, simply because the component framework
will only bind when all driver are present and unbind when the first driver
vanishes.
But I really like doing the reverse order more - so as it is now.
You also wouldn't disable clocks, before trying to deactivate some device-
function.
So deactivating stuff in the reverse order of them getting activated is most
likely less error prone.
prev parent reply other threads:[~2024-08-15 19:01 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20240808-rk-drm-fix-unreg-v1-1-c30d9a754722@collabora.com>
2024-08-12 2:14 ` Re:[PATCH RESEND] drm/rockchip: Unregister platform drivers in reverse order Andy Yan
2024-08-15 14:21 ` [PATCH " Heiko Stübner
[not found] ` <80ffed99-7368-4f84-8dc9-4c980055e48d@collabora.com>
2024-08-15 17:52 ` Heiko Stübner [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=12056454.6ASCnIXroE@diego \
--to=heiko@sntech.de \
--cc=airlied@gmail.com \
--cc=andrzej.hajda@intel.com \
--cc=andy.yan@rock-chips.com \
--cc=cristian.ciocaltea@collabora.com \
--cc=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=hjc@rock-chips.com \
--cc=jeffy.chen@rock-chips.com \
--cc=kernel@collabora.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=maarten.lankhorst@linux.intel.com \
--cc=markyao0591@gmail.com \
--cc=mripard@kernel.org \
--cc=seanpaul@chromium.org \
--cc=tzimmermann@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).