Hello, [Changed email address for David Woodhouse from intel to infradead] On Mon, Oct 09, 2023 at 10:43:46AM +0200, Arnd Bergmann wrote: > On Mon, Oct 9, 2023, at 09:22, Masahiro Yamada wrote: > > On Mon, Oct 9, 2023 at 5:02 AM Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> wrote: > >> > >> As described in the added code comment, a reference to .exit.text is ok > >> for drivers registered via module_platform_driver_probe(). Make this > >> explicit to prevent a section mismatch warning with > > > > > We have thousands of module_platform_drivers. > > I would be scared if they started to add __refdata. > > > > I am not sure if this is the right direction. > > For a normal module_platform_driver(), this would indeed be > wrong, but as Uwe said above there is a special case for > module_platform_driver_probe(), which implicitly sets the > drv->driver.suppress_bind_attrs=true flag. > > > In my understanding of the current DT overlay, > > there is no way to create/remove a platform device dynamically. > > I do not know if that will happen in the future. > > For drivers without suppress_bind_attrs, you can manually > unbind the device from a driver, which in case of a loadable > module ends up calling the .remove callback (this is fine), > but in a built-in driver this would use a NULL pointer for > .remove and cause unexpected behavior. only a slight correction: As not having a remove callback can be fine and platform_remove() only calls .remove (or .remove_new) when non-NULL we're not hitting a NULL pointer dereference in the presence of .remove = __exit_p(somefunc), But a problem can arise later if some resource isn't properly freed and so it might be used at a later point in time which then most likely oopses. I didn't double check Arnd's list, but otherwise I agree to his analysis. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ |
Attachment:
signature.asc
Description: PGP signature
