2022-08-02 17:34:32

by Grzegorz Szymaszek

[permalink] [raw]
Subject: [PATCH] staging: r8188eu: add firmware dependency

The old rtl8188eu module, removed in commit 55dfa29b43d2 ("staging:
rtl8188eu: remove rtl8188eu driver from staging dir") (Linux kernel
v5.15-rc1), required (through a MODULE_FIRMWARE call()) the
rtlwifi/rtl8188eufw.bin firmware file, which the new r8188eu driver no
longer requires.

I have tested a few RTL8188EUS-based Wi-Fi cards and, while supported by
both drivers, they do not work when using the new one and the firmware
wasn't manually loaded. According to Larry Finger, the module
maintainer, all such cards need the firmware and the driver should
depend on it (see the linked mails).

Add a proper MODULE_FIRMWARE() call, like it was done in the old driver.

Thanks to Greg Kroah-Hartman and Larry Finger for quick responses to my
questions.

Link: https://answers.launchpad.net/ubuntu/+source/linux-meta-hwe-5.15/+question/702611
Link: https://lore.kernel.org/lkml/YukkBu3TNODO3or9@nx64de-df6d00/
Signed-off-by: Grzegorz Szymaszek <[email protected]>
---
drivers/staging/r8188eu/os_dep/os_intfs.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/staging/r8188eu/os_dep/os_intfs.c b/drivers/staging/r8188eu/os_dep/os_intfs.c
index 891c85b088ca..5bd3022e4b40 100644
--- a/drivers/staging/r8188eu/os_dep/os_intfs.c
+++ b/drivers/staging/r8188eu/os_dep/os_intfs.c
@@ -18,6 +18,7 @@ MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("Realtek Wireless Lan Driver");
MODULE_AUTHOR("Realtek Semiconductor Corp.");
MODULE_VERSION(DRIVERVERSION);
+MODULE_FIRMWARE("rtlwifi/rtl8188eufw.bin");

#define CONFIG_BR_EXT_BRNAME "br0"
#define RTW_NOTCH_FILTER 0 /* 0:Disable, 1:Enable, */
--
2.35.1


2022-08-02 18:15:49

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] staging: r8188eu: add firmware dependency

On 8/2/22 12:18, Grzegorz Szymaszek wrote:
> The old rtl8188eu module, removed in commit 55dfa29b43d2 ("staging:
> rtl8188eu: remove rtl8188eu driver from staging dir") (Linux kernel
> v5.15-rc1), required (through a MODULE_FIRMWARE call()) the
> rtlwifi/rtl8188eufw.bin firmware file, which the new r8188eu driver no
> longer requires.
>
> I have tested a few RTL8188EUS-based Wi-Fi cards and, while supported by
> both drivers, they do not work when using the new one and the firmware
> wasn't manually loaded. According to Larry Finger, the module
> maintainer, all such cards need the firmware and the driver should
> depend on it (see the linked mails).
>
> Add a proper MODULE_FIRMWARE() call, like it was done in the old driver.
>
> Thanks to Greg Kroah-Hartman and Larry Finger for quick responses to my
> questions.
>
> Link: https://answers.launchpad.net/ubuntu/+source/linux-meta-hwe-5.15/+question/702611
> Link: https://lore.kernel.org/lkml/YukkBu3TNODO3or9@nx64de-df6d00/
> Signed-off-by: Grzegorz Szymaszek <[email protected]>
> ---
> drivers/staging/r8188eu/os_dep/os_intfs.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/staging/r8188eu/os_dep/os_intfs.c b/drivers/staging/r8188eu/os_dep/os_intfs.c
> index 891c85b088ca..5bd3022e4b40 100644
> --- a/drivers/staging/r8188eu/os_dep/os_intfs.c
> +++ b/drivers/staging/r8188eu/os_dep/os_intfs.c
> @@ -18,6 +18,7 @@ MODULE_LICENSE("GPL");
> MODULE_DESCRIPTION("Realtek Wireless Lan Driver");
> MODULE_AUTHOR("Realtek Semiconductor Corp.");
> MODULE_VERSION(DRIVERVERSION);
> +MODULE_FIRMWARE("rtlwifi/rtl8188eufw.bin");
>
> #define CONFIG_BR_EXT_BRNAME "br0"
> #define RTW_NOTCH_FILTER 0 /* 0:Disable, 1:Enable, */

Acked-by: Larry Finger <[email protected]>

Thanks,

Larry


2022-08-02 19:05:16

by Phillip Potter

[permalink] [raw]
Subject: Re: [PATCH] staging: r8188eu: add firmware dependency

On Tue, Aug 02, 2022 at 07:18:44PM +0200, Grzegorz Szymaszek wrote:
> The old rtl8188eu module, removed in commit 55dfa29b43d2 ("staging:
> rtl8188eu: remove rtl8188eu driver from staging dir") (Linux kernel
> v5.15-rc1), required (through a MODULE_FIRMWARE call()) the
> rtlwifi/rtl8188eufw.bin firmware file, which the new r8188eu driver no
> longer requires.
>
> I have tested a few RTL8188EUS-based Wi-Fi cards and, while supported by
> both drivers, they do not work when using the new one and the firmware
> wasn't manually loaded. According to Larry Finger, the module
> maintainer, all such cards need the firmware and the driver should
> depend on it (see the linked mails).
>
> Add a proper MODULE_FIRMWARE() call, like it was done in the old driver.
>
> Thanks to Greg Kroah-Hartman and Larry Finger for quick responses to my
> questions.
>
> Link: https://answers.launchpad.net/ubuntu/+source/linux-meta-hwe-5.15/+question/702611
> Link: https://lore.kernel.org/lkml/YukkBu3TNODO3or9@nx64de-df6d00/
> Signed-off-by: Grzegorz Szymaszek <[email protected]>
> ---
> drivers/staging/r8188eu/os_dep/os_intfs.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/staging/r8188eu/os_dep/os_intfs.c b/drivers/staging/r8188eu/os_dep/os_intfs.c
> index 891c85b088ca..5bd3022e4b40 100644
> --- a/drivers/staging/r8188eu/os_dep/os_intfs.c
> +++ b/drivers/staging/r8188eu/os_dep/os_intfs.c
> @@ -18,6 +18,7 @@ MODULE_LICENSE("GPL");
> MODULE_DESCRIPTION("Realtek Wireless Lan Driver");
> MODULE_AUTHOR("Realtek Semiconductor Corp.");
> MODULE_VERSION(DRIVERVERSION);
> +MODULE_FIRMWARE("rtlwifi/rtl8188eufw.bin");
>
> #define CONFIG_BR_EXT_BRNAME "br0"
> #define RTW_NOTCH_FILTER 0 /* 0:Disable, 1:Enable, */
> --
> 2.35.1

Acked-by: Phillip Potter <[email protected]>

Thanks for the patch :-)

Regards,
Phil

2022-08-03 06:19:21

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] staging: r8188eu: add firmware dependency

On Tue, Aug 02, 2022 at 07:18:44PM +0200, Grzegorz Szymaszek wrote:
> The old rtl8188eu module, removed in commit 55dfa29b43d2 ("staging:
> rtl8188eu: remove rtl8188eu driver from staging dir") (Linux kernel
> v5.15-rc1), required (through a MODULE_FIRMWARE call()) the
> rtlwifi/rtl8188eufw.bin firmware file, which the new r8188eu driver no
> longer requires.
>
> I have tested a few RTL8188EUS-based Wi-Fi cards and, while supported by
> both drivers, they do not work when using the new one and the firmware
> wasn't manually loaded. According to Larry Finger, the module
> maintainer, all such cards need the firmware and the driver should
> depend on it (see the linked mails).
>
> Add a proper MODULE_FIRMWARE() call, like it was done in the old driver.
>
> Thanks to Greg Kroah-Hartman and Larry Finger for quick responses to my
> questions.
>
> Link: https://answers.launchpad.net/ubuntu/+source/linux-meta-hwe-5.15/+question/702611
> Link: https://lore.kernel.org/lkml/YukkBu3TNODO3or9@nx64de-df6d00/
> Signed-off-by: Grzegorz Szymaszek <[email protected]>
> ---
> drivers/staging/r8188eu/os_dep/os_intfs.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/staging/r8188eu/os_dep/os_intfs.c b/drivers/staging/r8188eu/os_dep/os_intfs.c
> index 891c85b088ca..5bd3022e4b40 100644
> --- a/drivers/staging/r8188eu/os_dep/os_intfs.c
> +++ b/drivers/staging/r8188eu/os_dep/os_intfs.c
> @@ -18,6 +18,7 @@ MODULE_LICENSE("GPL");
> MODULE_DESCRIPTION("Realtek Wireless Lan Driver");
> MODULE_AUTHOR("Realtek Semiconductor Corp.");
> MODULE_VERSION(DRIVERVERSION);
> +MODULE_FIRMWARE("rtlwifi/rtl8188eufw.bin");

This looks good, and I'll apply it after 5.20-rc1 is out, but you might
want to send a follow-on patch that removes the hard-coded string in 2
places in the driver, and just puts it into a single define somewhere,
to make it a bit easier over time. Most other drivers do this as well,
so there are examples to look at.

thanks,

greg k-h

2022-08-03 06:41:43

by Grzegorz Szymaszek

[permalink] [raw]
Subject: Re: [PATCH] staging: r8188eu: add firmware dependency

On Wed, Aug 03, 2022 at 08:08:31AM +0200, Greg KH wrote:
> This looks good, and I'll apply it after 5.20-rc1 is out,

I didn’t Cc stable since the “Submitting patches” guide says it’s for
“severe bugs”, but would it be possible to backport the patch to the
older kernels?

> but you might want to send a follow-on patch that removes the hard-coded
> string in 2 places in the driver, and just puts it into a single define
> somewhere, to make it a bit easier over time.

Good idea, will do so. I didn’t check if the filename is already used.
Would something like “this patch depends on patch…” (again from the
guide) be enough (assuming I will send the new patch before this one
is applied)?

--
Grzegorz Szymaszek


Attachments:
(No filename) (754.00 B)
signature.asc (849.00 B)
Download all attachments

2022-08-03 06:45:22

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] staging: r8188eu: add firmware dependency

On Wed, Aug 03, 2022 at 08:33:35AM +0200, Grzegorz Szymaszek wrote:
> On Wed, Aug 03, 2022 at 08:08:31AM +0200, Greg KH wrote:
> > This looks good, and I'll apply it after 5.20-rc1 is out,
>
> I didn’t Cc stable since the “Submitting patches” guide says it’s for
> “severe bugs”, but would it be possible to backport the patch to the
> older kernels?

Yes, I will tag it for backporting.

> > but you might want to send a follow-on patch that removes the hard-coded
> > string in 2 places in the driver, and just puts it into a single define
> > somewhere, to make it a bit easier over time.
>
> Good idea, will do so. I didn’t check if the filename is already used.
> Would something like “this patch depends on patch…” (again from the
> guide) be enough (assuming I will send the new patch before this one
> is applied)?

No need to add the "this patch depends on", I'll know that implicitly as
it was sent after this one :)

thanks,

greg k-h