| Message ID | 20240329-module-owner-sdio-v1-1-e4010b11ccaa@linaro.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mmc/wifi/bluetooth: store owner from modules with sdio_register_driver() | expand |
Hello Krzysztof, On Fri, Mar 29, 2024 at 06:24:31PM +0100, Krzysztof Kozlowski wrote: > Modules registering driver with sdio_register_driver() might > forget to set .owner field. The field is used by some of other kernel ^^ double space here > parts for reference counting (try_module_get()), so it is expected that > drivers will set it. > > Solve the problem by moving this task away from the drivers to the core > code, just like we did for platform_driver in > commit 9447057eaff8 ("platform_device: use a macro instead of > platform_driver_register"). > > Since many drivers forget to set the .owner, this effectively will fix > them. Examples of fixed drivers are: ath6kl, b43, btsdio.c, ks7010, ^^ and here > libertas, MediaTek WiFi drivers, Realtek WiFi drivers, rsi, siano, > wilc1000, wl1251 and more. > > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> My nit comments are irrelevant, but given you did the same twice I felt like letting you know. Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
On 31/03/2024 22:37, Francesco Dolcini wrote: > Hello Krzysztof, > > On Fri, Mar 29, 2024 at 06:24:31PM +0100, Krzysztof Kozlowski wrote: >> Modules registering driver with sdio_register_driver() might >> forget to set .owner field. The field is used by some of other kernel > ^^ double space here > >> parts for reference counting (try_module_get()), so it is expected that >> drivers will set it. >> >> Solve the problem by moving this task away from the drivers to the core >> code, just like we did for platform_driver in >> commit 9447057eaff8 ("platform_device: use a macro instead of >> platform_driver_register"). >> >> Since many drivers forget to set the .owner, this effectively will fix >> them. Examples of fixed drivers are: ath6kl, b43, btsdio.c, ks7010, > ^^ and here Which is correct :). Double space is on purpose. > >> libertas, MediaTek WiFi drivers, Realtek WiFi drivers, rsi, siano, >> wilc1000, wl1251 and more. >> >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > > My nit comments are irrelevant, but given you did the same twice I felt > like letting you know. > > Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com> > Best regards, Krzysztof
diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c index 71d885fbc228..c5fdfe2325f8 100644 --- a/drivers/mmc/core/sdio_bus.c +++ b/drivers/mmc/core/sdio_bus.c @@ -265,16 +265,19 @@ void sdio_unregister_bus(void) } /** - * sdio_register_driver - register a function driver + * __sdio_register_driver - register a function driver * @drv: SDIO function driver + * @owner: owning module/driver */ -int sdio_register_driver(struct sdio_driver *drv) +int __sdio_register_driver(struct sdio_driver *drv, struct module *owner) { drv->drv.name = drv->name; drv->drv.bus = &sdio_bus_type; + drv->drv.owner = owner; + return driver_register(&drv->drv); } -EXPORT_SYMBOL_GPL(sdio_register_driver); +EXPORT_SYMBOL_GPL(__sdio_register_driver); /** * sdio_unregister_driver - unregister a function driver diff --git a/include/linux/mmc/sdio_func.h b/include/linux/mmc/sdio_func.h index 478855b8e406..fed1f5f4a8d3 100644 --- a/include/linux/mmc/sdio_func.h +++ b/include/linux/mmc/sdio_func.h @@ -106,7 +106,10 @@ struct sdio_driver { .class = (dev_class), \ .vendor = SDIO_ANY_ID, .device = SDIO_ANY_ID -extern int sdio_register_driver(struct sdio_driver *); +/* use a macro to avoid include chaining to get THIS_MODULE */ +#define sdio_register_driver(drv) \ + __sdio_register_driver(drv, THIS_MODULE) +extern int __sdio_register_driver(struct sdio_driver *, struct module *); extern void sdio_unregister_driver(struct sdio_driver *); /**
Modules registering driver with sdio_register_driver() might forget to set .owner field. The field is used by some of other kernel parts for reference counting (try_module_get()), so it is expected that drivers will set it. Solve the problem by moving this task away from the drivers to the core code, just like we did for platform_driver in commit 9447057eaff8 ("platform_device: use a macro instead of platform_driver_register"). Since many drivers forget to set the .owner, this effectively will fix them. Examples of fixed drivers are: ath6kl, b43, btsdio.c, ks7010, libertas, MediaTek WiFi drivers, Realtek WiFi drivers, rsi, siano, wilc1000, wl1251 and more. Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> --- drivers/mmc/core/sdio_bus.c | 9 ++++++--- include/linux/mmc/sdio_func.h | 5 ++++- 2 files changed, 10 insertions(+), 4 deletions(-)