2024-03-29 17:24:56

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH 0/7] mmc/wifi/bluetooth: store owner from modules with sdio_register_driver()

Merging
=======
All further patches depend on the first patch. Everything could go via
one tree, e.g. MMC, or the cleanup patches removing owner would wait a
cycle.

Description
===========
Modules registering driver with sdio_register_driver() might
forget to set .owner field.

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").

Best regards,
Krzysztof

---
Krzysztof Kozlowski (7):
mmc: sdio: store owner from modules with sdio_register_driver()
bluetooth: btmrvl_sdio: drop driver owner initialization
bluetooth: btmtksdio: drop driver owner initialization
wifi: ath10k: sdio: drop driver owner initialization
wifi: brcm80211: drop driver owner initialization
wifi: marvell: mwifiex: drop driver owner initialization
wifi: silabs: wfx: drop driver owner initialization

drivers/bluetooth/btmrvl_sdio.c | 1 -
drivers/bluetooth/btmtksdio.c | 1 -
drivers/mmc/core/sdio_bus.c | 9 ++++++---
drivers/net/wireless/ath/ath10k/sdio.c | 1 -
drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 1 -
drivers/net/wireless/marvell/mwifiex/sdio.c | 1 -
drivers/net/wireless/silabs/wfx/bus_sdio.c | 1 -
include/linux/mmc/sdio_func.h | 5 ++++-
8 files changed, 10 insertions(+), 10 deletions(-)
---
base-commit: 087c142b2b04898c897aa77938d05a93907150e5
change-id: 20240329-module-owner-sdio-abd5de3f1d74

Best regards,
--
Krzysztof Kozlowski <[email protected]>



2024-03-29 17:25:12

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH 1/7] mmc: sdio: store owner from modules with sdio_register_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 <[email protected]>
---
drivers/mmc/core/sdio_bus.c | 9 ++++++---
include/linux/mmc/sdio_func.h | 5 ++++-
2 files changed, 10 insertions(+), 4 deletions(-)

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 *);

/**

--
2.34.1


2024-03-29 17:25:58

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH 3/7] bluetooth: btmtksdio: drop driver owner initialization

Core in sdio_register_driver() already sets the .owner, so driver does
not need to.

Signed-off-by: Krzysztof Kozlowski <[email protected]>

---

Depends on the first patch.
---
drivers/bluetooth/btmtksdio.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/btmtksdio.c
index ff4868c83cd8..8ded9ef8089a 100644
--- a/drivers/bluetooth/btmtksdio.c
+++ b/drivers/bluetooth/btmtksdio.c
@@ -1519,7 +1519,6 @@ static struct sdio_driver btmtksdio_driver = {
.remove = btmtksdio_remove,
.id_table = btmtksdio_table,
.drv = {
- .owner = THIS_MODULE,
.pm = BTMTKSDIO_PM_OPS,
}
};

--
2.34.1


2024-03-29 17:26:19

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH 4/7] wifi: ath10k: sdio: drop driver owner initialization

Core in sdio_register_driver() already sets the .owner, so driver does
not need to.

Signed-off-by: Krzysztof Kozlowski <[email protected]>

---

Depends on the first patch.
---
drivers/net/wireless/ath/ath10k/sdio.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/sdio.c b/drivers/net/wireless/ath/ath10k/sdio.c
index 1acb9fba9a8e..cddd9e3010ee 100644
--- a/drivers/net/wireless/ath/ath10k/sdio.c
+++ b/drivers/net/wireless/ath/ath10k/sdio.c
@@ -2667,7 +2667,6 @@ static struct sdio_driver ath10k_sdio_driver = {
.probe = ath10k_sdio_probe,
.remove = ath10k_sdio_remove,
.drv = {
- .owner = THIS_MODULE,
.pm = ATH10K_SDIO_PM_OPS,
},
};

--
2.34.1


2024-03-29 17:27:16

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH 6/7] wifi: marvell: mwifiex: drop driver owner initialization

Core in sdio_register_driver() already sets the .owner, so driver does
not need to.

Signed-off-by: Krzysztof Kozlowski <[email protected]>

---

Depends on the first patch.
---
drivers/net/wireless/marvell/mwifiex/sdio.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c b/drivers/net/wireless/marvell/mwifiex/sdio.c
index e1b58ca1b3ba..588140453821 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -979,7 +979,6 @@ static struct sdio_driver mwifiex_sdio = {
.probe = mwifiex_sdio_probe,
.remove = mwifiex_sdio_remove,
.drv = {
- .owner = THIS_MODULE,
.coredump = mwifiex_sdio_coredump,
.pm = &mwifiex_sdio_pm_ops,
}

--
2.34.1


2024-03-29 17:30:12

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH 7/7] wifi: silabs: wfx: drop driver owner initialization

Core in sdio_register_driver() already sets the .owner, so driver does
not need to.

Signed-off-by: Krzysztof Kozlowski <[email protected]>

---

Depends on the first patch.
---
drivers/net/wireless/silabs/wfx/bus_sdio.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/net/wireless/silabs/wfx/bus_sdio.c b/drivers/net/wireless/silabs/wfx/bus_sdio.c
index 909d5f346a01..f290eecde773 100644
--- a/drivers/net/wireless/silabs/wfx/bus_sdio.c
+++ b/drivers/net/wireless/silabs/wfx/bus_sdio.c
@@ -267,7 +267,6 @@ struct sdio_driver wfx_sdio_driver = {
.probe = wfx_sdio_probe,
.remove = wfx_sdio_remove,
.drv = {
- .owner = THIS_MODULE,
.of_match_table = wfx_sdio_of_match,
}
};

--
2.34.1


2024-03-29 23:23:00

by Jeff Johnson

[permalink] [raw]
Subject: Re: [PATCH 4/7] wifi: ath10k: sdio: drop driver owner initialization

On 3/29/2024 10:24 AM, Krzysztof Kozlowski wrote:
> Core in sdio_register_driver() already sets the .owner, so driver does
> not need to.
>
> Signed-off-by: Krzysztof Kozlowski <[email protected]>

Acked-by: Jeff Johnson <[email protected]>

>
> ---
>
> Depends on the first patch.
> ---
> drivers/net/wireless/ath/ath10k/sdio.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/sdio.c b/drivers/net/wireless/ath/ath10k/sdio.c
> index 1acb9fba9a8e..cddd9e3010ee 100644
> --- a/drivers/net/wireless/ath/ath10k/sdio.c
> +++ b/drivers/net/wireless/ath/ath10k/sdio.c
> @@ -2667,7 +2667,6 @@ static struct sdio_driver ath10k_sdio_driver = {
> .probe = ath10k_sdio_probe,
> .remove = ath10k_sdio_remove,
> .drv = {
> - .owner = THIS_MODULE,
> .pm = ATH10K_SDIO_PM_OPS,
> },
> };
>


2024-03-31 20:29:09

by Francesco Dolcini

[permalink] [raw]
Subject: Re: [PATCH 6/7] wifi: marvell: mwifiex: drop driver owner initialization

On Fri, Mar 29, 2024 at 06:24:36PM +0100, Krzysztof Kozlowski wrote:
> Core in sdio_register_driver() already sets the .owner, so driver does
> not need to.
>
> Signed-off-by: Krzysztof Kozlowski <[email protected]>

Reviewed-by: Francesco Dolcini <[email protected]>


2024-03-31 20:37:39

by Francesco Dolcini

[permalink] [raw]
Subject: Re: [PATCH 1/7] mmc: sdio: store owner from modules with sdio_register_driver()

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 <[email protected]>

My nit comments are irrelevant, but given you did the same twice I felt
like letting you know.

Reviewed-by: Francesco Dolcini <[email protected]>


2024-04-02 10:59:04

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH 0/7] mmc/wifi/bluetooth: store owner from modules with sdio_register_driver()

On Fri, 29 Mar 2024 at 18:24, Krzysztof Kozlowski
<[email protected]> wrote:
>
> Merging
> =======
> All further patches depend on the first patch. Everything could go via
> one tree, e.g. MMC, or the cleanup patches removing owner would wait a
> cycle.

Patch 1 applied for next, thanks!

I can certainly pick the remaining in the series through my mmc tree,
but waiting a bit to queue them up to allow some more acks to be
received.

Kind regards
Uffe


>
> Description
> ===========
> Modules registering driver with sdio_register_driver() might
> forget to set .owner field.
>
> 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").
>
> Best regards,
> Krzysztof
>
> ---
> Krzysztof Kozlowski (7):
> mmc: sdio: store owner from modules with sdio_register_driver()
> bluetooth: btmrvl_sdio: drop driver owner initialization
> bluetooth: btmtksdio: drop driver owner initialization
> wifi: ath10k: sdio: drop driver owner initialization
> wifi: brcm80211: drop driver owner initialization
> wifi: marvell: mwifiex: drop driver owner initialization
> wifi: silabs: wfx: drop driver owner initialization
>
> drivers/bluetooth/btmrvl_sdio.c | 1 -
> drivers/bluetooth/btmtksdio.c | 1 -
> drivers/mmc/core/sdio_bus.c | 9 ++++++---
> drivers/net/wireless/ath/ath10k/sdio.c | 1 -
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 1 -
> drivers/net/wireless/marvell/mwifiex/sdio.c | 1 -
> drivers/net/wireless/silabs/wfx/bus_sdio.c | 1 -
> include/linux/mmc/sdio_func.h | 5 ++++-
> 8 files changed, 10 insertions(+), 10 deletions(-)
> ---
> base-commit: 087c142b2b04898c897aa77938d05a93907150e5
> change-id: 20240329-module-owner-sdio-abd5de3f1d74
>
> Best regards,
> --
> Krzysztof Kozlowski <[email protected]>
>

2024-04-03 13:55:02

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 7/7] wifi: silabs: wfx: drop driver owner initialization

Krzysztof Kozlowski <[email protected]> writes:

> Core in sdio_register_driver() already sets the .owner, so driver does
> not need to.
>
> Signed-off-by: Krzysztof Kozlowski <[email protected]>

Also here the preferred title format is:

wifi: wfx: drop driver owner initialization

Feel free to take this via sdio tree:

Acked-by: Kalle Valo <[email protected]>

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches