Re: [PATCH RESEND] drm/rockchip: Unregister platform drivers in reverse order

Am Donnerstag, 15. August 2024, 19:26:54 CEST schrieb Cristian Ciocaltea:

On 8/15/24 5:21 PM, Heiko Stübner wrote:

quoted

Am Donnerstag, 8. August 2024, 13:58:02 CEST schrieb Cristian Ciocaltea:

quoted

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.