staging: r8188eu: add firmware dependency

[PATCH] staging: r8188eu: add firmware dependency

Posted by Grzegorz Szymaszek 3 years, 8 months ago

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 <gszymaszek@short.pl>
---
 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

Re: [PATCH] staging: r8188eu: add firmware dependency

Posted by Greg KH 3 years, 8 months ago

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 <gszymaszek@short.pl>
> ---
>  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

Re: [PATCH] staging: r8188eu: add firmware dependency

Posted by Grzegorz Szymaszek 3 years, 8 months ago

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

Re: [PATCH] staging: r8188eu: add firmware dependency

Posted by Greg KH 3 years, 8 months ago

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

[PATCH 1/3] staging: r8188eu: set firmware path in a macro

Posted by Grzegorz Szymaszek 3 years, 8 months ago

The r8188eu driver requires a firmware file, the path of which was
hardcoded as constant strings in two places:
(1) in core/rtw_fw.c, in function load_firmware(),
(2) in os_dep/os_intfs.c, in the MODULE_FIRMWARE() call.

Declare the path using a macro, FW_RTL8188EU, and replace the above
constant strings with the macro. That's the way it is done in many other
drivers. The new macro is defined in include/drv_types.h, because that
file is already included by both of the above files (or at least their
headers) and because it already contains other driver constants, like
its name and version.

Link: https://lore.kernel.org/lkml/YuoQ37PIKzWO1zIY@kroah.com/
Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Grzegorz Szymaszek <gszymaszek@short.pl>
---
 drivers/staging/r8188eu/core/rtw_fw.c       | 2 +-
 drivers/staging/r8188eu/include/drv_types.h | 1 +
 drivers/staging/r8188eu/os_dep/os_intfs.c   | 2 +-
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_fw.c b/drivers/staging/r8188eu/core/rtw_fw.c
index 0451e5177644..0fe6d4944694 100644
--- a/drivers/staging/r8188eu/core/rtw_fw.c
+++ b/drivers/staging/r8188eu/core/rtw_fw.c
@@ -209,7 +209,7 @@ static int load_firmware(struct rt_firmware *rtfw, struct device *device)
 {
 	int ret = _SUCCESS;
 	const struct firmware *fw;
-	const char *fw_name = "rtlwifi/rtl8188eufw.bin";
+	const char *fw_name = FW_RTL8188EU;
 	int err = request_firmware(&fw, fw_name, device);
 
 	if (err) {
diff --git a/drivers/staging/r8188eu/include/drv_types.h b/drivers/staging/r8188eu/include/drv_types.h
index bba88a0ede61..f51b83515953 100644
--- a/drivers/staging/r8188eu/include/drv_types.h
+++ b/drivers/staging/r8188eu/include/drv_types.h
@@ -37,6 +37,7 @@
 #include "rtw_fw.h"
 
 #define DRIVERVERSION	"v4.1.4_6773.20130222"
+#define FW_RTL8188EU	"rtlwifi/rtl8188eufw.bin"
 
 struct registry_priv {
 	u8	chip_version;
diff --git a/drivers/staging/r8188eu/os_dep/os_intfs.c b/drivers/staging/r8188eu/os_dep/os_intfs.c
index 5bd3022e4b40..5985054da935 100644
--- a/drivers/staging/r8188eu/os_dep/os_intfs.c
+++ b/drivers/staging/r8188eu/os_dep/os_intfs.c
@@ -18,7 +18,7 @@ MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("Realtek Wireless Lan Driver");
 MODULE_AUTHOR("Realtek Semiconductor Corp.");
 MODULE_VERSION(DRIVERVERSION);
-MODULE_FIRMWARE("rtlwifi/rtl8188eufw.bin");
+MODULE_FIRMWARE(FW_RTL8188EU);
 
 #define CONFIG_BR_EXT_BRNAME "br0"
 #define RTW_NOTCH_FILTER 0 /* 0:Disable, 1:Enable, */
-- 
2.35.1

Re: [PATCH 1/3] staging: r8188eu: set firmware path in a macro

Posted by Philipp Hortmann 3 years, 8 months ago

On 8/4/22 00:28, Grzegorz Szymaszek wrote:
> diff --git a/drivers/staging/r8188eu/os_dep/os_intfs.c b/drivers/staging/r8188eu/os_dep/os_intfs.c
> index 5bd3022e4b40..5985054da935 100644
> --- a/drivers/staging/r8188eu/os_dep/os_intfs.c
> +++ b/drivers/staging/r8188eu/os_dep/os_intfs.c
> @@ -18,7 +18,7 @@ MODULE_LICENSE("GPL");
>   MODULE_DESCRIPTION("Realtek Wireless Lan Driver");
>   MODULE_AUTHOR("Realtek Semiconductor Corp.");
>   MODULE_VERSION(DRIVERVERSION);
> -MODULE_FIRMWARE("rtlwifi/rtl8188eufw.bin");
> +MODULE_FIRMWARE(FW_RTL8188EU);
>   
>   #define CONFIG_BR_EXT_BRNAME "br0"
>   #define RTW_NOTCH_FILTER 0 /* 0:Disable, 1:Enable, */


It failed to apply your patch as the following line:
MODULE_FIRMWARE("rtlwifi/rtl8188eufw.bin");
is not in the repo. Inserted line in my repo to apply patch.

Why is the coverletter missing?

Tested all three patches.

Tested-by: Philipp Hortmann <philipp.g.hortmann@gmail.com> # Edimax N150

Re: [PATCH 1/3] staging: r8188eu: set firmware path in a macro

Posted by Grzegorz Szymaszek 3 years, 8 months ago

On Thu, Aug 04, 2022 at 10:11:58PM +0200, Philipp Hortmann wrote:
> On 8/4/22 00:28, Grzegorz Szymaszek wrote:
> > diff --git a/drivers/staging/r8188eu/os_dep/os_intfs.c b/drivers/staging/r8188eu/os_dep/os_intfs.c
> > -%<-
> > -MODULE_FIRMWARE("rtlwifi/rtl8188eufw.bin");
> > +MODULE_FIRMWARE(FW_RTL8188EU);
> 
> 
> It failed to apply your patch as the following line:
> MODULE_FIRMWARE("rtlwifi/rtl8188eufw.bin");
> is not in the repo. Inserted line in my repo to apply patch.

I’m sorry, I didn’t add the base tree reference. Right now, git
format-patch generates the following:

    base-commit: 9de1f9c8ca5100a02a2e271bdbde36202e251b4b
    prerequisite-patch-id: 79964bd0bcd260f1df53830a81e009c34993ee6f

The prerequisite patch is available at
<https://lore.kernel.org/lkml/YulcdKfhA8dPQ78s@nx64de-df6d00/>.

> Why is the coverletter missing?

I didn’t think it would be necessary, since the patches are quite
simple and there are just three of them. Again, I’m sorry, I don’t want
to make it harder for anyone to review my patches. Hopefully I will
learn more of the kernel development practises without wasting too much
time of patch reviewers.

Should I send an improved (with the base tree reference and with a short
cover letter) patch series?

> Tested all three patches.

Thanks!

Re: [PATCH 1/3] staging: r8188eu: set firmware path in a macro

Posted by Greg KH 3 years, 8 months ago

On Fri, Aug 05, 2022 at 12:23:12AM +0200, Grzegorz Szymaszek wrote:
> On Thu, Aug 04, 2022 at 10:11:58PM +0200, Philipp Hortmann wrote:
> > On 8/4/22 00:28, Grzegorz Szymaszek wrote:
> > > diff --git a/drivers/staging/r8188eu/os_dep/os_intfs.c b/drivers/staging/r8188eu/os_dep/os_intfs.c
> > > -%<-
> > > -MODULE_FIRMWARE("rtlwifi/rtl8188eufw.bin");
> > > +MODULE_FIRMWARE(FW_RTL8188EU);
> > 
> > 
> > It failed to apply your patch as the following line:
> > MODULE_FIRMWARE("rtlwifi/rtl8188eufw.bin");
> > is not in the repo. Inserted line in my repo to apply patch.
> 
> I’m sorry, I didn’t add the base tree reference. Right now, git
> format-patch generates the following:
> 
>     base-commit: 9de1f9c8ca5100a02a2e271bdbde36202e251b4b
>     prerequisite-patch-id: 79964bd0bcd260f1df53830a81e009c34993ee6f
> 
> The prerequisite patch is available at
> <https://lore.kernel.org/lkml/YulcdKfhA8dPQ78s@nx64de-df6d00/>.
> 
> > Why is the coverletter missing?
> 
> I didn’t think it would be necessary, since the patches are quite
> simple and there are just three of them. Again, I’m sorry, I don’t want
> to make it harder for anyone to review my patches. Hopefully I will
> learn more of the kernel development practises without wasting too much
> time of patch reviewers.
> 
> Should I send an improved (with the base tree reference and with a short
> cover letter) patch series?

Nah, it's fine as-is, I will take it when my tree opens up again after
-rc1 is out in a week or so.

thanks,

greg k-h

[PATCH 2/3] staging: r8188eu: make use of the DRV_NAME macro

Posted by Grzegorz Szymaszek 3 years, 8 months ago

The DRV_NAME macro is defined with the name of the r8188eu driver, but
it seems it wasn't actually used anywhere. Replace a hardcoded constant
string of the same value in the driver's struct rtw_usb_drv, field
.usbdrv.name. The affected file already includes include/drv_types.h,
where the macro is declared.

Signed-off-by: Grzegorz Szymaszek <gszymaszek@short.pl>
---
 drivers/staging/r8188eu/os_dep/usb_intf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/r8188eu/os_dep/usb_intf.c b/drivers/staging/r8188eu/os_dep/usb_intf.c
index 68869c5daeff..256b9045488e 100644
--- a/drivers/staging/r8188eu/os_dep/usb_intf.c
+++ b/drivers/staging/r8188eu/os_dep/usb_intf.c
@@ -54,7 +54,7 @@ struct rtw_usb_drv {
 };
 
 static struct rtw_usb_drv rtl8188e_usb_drv = {
-	.usbdrv.name = "r8188eu",
+	.usbdrv.name = DRV_NAME,
 	.usbdrv.probe = rtw_drv_init,
 	.usbdrv.disconnect = rtw_dev_remove,
 	.usbdrv.id_table = rtw_usb_id_tbl,
-- 
2.35.1

Re: [PATCH 2/3] staging: r8188eu: make use of the DRV_NAME macro

Posted by Greg KH 3 years, 8 months ago

On Thu, Aug 04, 2022 at 12:29:01AM +0200, Grzegorz Szymaszek wrote:
> The DRV_NAME macro is defined with the name of the r8188eu driver, but
> it seems it wasn't actually used anywhere. Replace a hardcoded constant
> string of the same value in the driver's struct rtw_usb_drv, field
> .usbdrv.name. The affected file already includes include/drv_types.h,
> where the macro is declared.
> 
> Signed-off-by: Grzegorz Szymaszek <gszymaszek@short.pl>
> ---
>  drivers/staging/r8188eu/os_dep/usb_intf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/r8188eu/os_dep/usb_intf.c b/drivers/staging/r8188eu/os_dep/usb_intf.c
> index 68869c5daeff..256b9045488e 100644
> --- a/drivers/staging/r8188eu/os_dep/usb_intf.c
> +++ b/drivers/staging/r8188eu/os_dep/usb_intf.c
> @@ -54,7 +54,7 @@ struct rtw_usb_drv {
>  };
>  
>  static struct rtw_usb_drv rtl8188e_usb_drv = {
> -	.usbdrv.name = "r8188eu",
> +	.usbdrv.name = DRV_NAME,

Should just be KBUILD_MODNAME.

thanks,

greg k-h

[PATCH 3/3] staging: r8188eu: make driver metadata macro names more consistent

Posted by Grzegorz Szymaszek 3 years, 8 months ago

Rename DRIVERVERSION to DRV_VERSION so that it looks more alike the
other macros, DRV_NAME and FW_*, and matches the most popular (as it
seems from a quick review) conventions in other drivers.

Signed-off-by: Grzegorz Szymaszek <gszymaszek@short.pl>
---
 drivers/staging/r8188eu/include/drv_types.h | 5 ++---
 drivers/staging/r8188eu/os_dep/os_intfs.c   | 2 +-
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/r8188eu/include/drv_types.h b/drivers/staging/r8188eu/include/drv_types.h
index f51b83515953..3328c66d1ef1 100644
--- a/drivers/staging/r8188eu/include/drv_types.h
+++ b/drivers/staging/r8188eu/include/drv_types.h
@@ -10,8 +10,6 @@
 #ifndef __DRV_TYPES_H__
 #define __DRV_TYPES_H__
 
-#define DRV_NAME "r8188eu"
-
 #include "osdep_service.h"
 #include "wlan_bssdef.h"
 #include "rtw_ht.h"
@@ -36,7 +34,8 @@
 #include "rtl8188e_hal.h"
 #include "rtw_fw.h"
 
-#define DRIVERVERSION	"v4.1.4_6773.20130222"
+#define DRV_NAME	"r8188eu"
+#define DRV_VERSION	"v4.1.4_6773.20130222"
 #define FW_RTL8188EU	"rtlwifi/rtl8188eufw.bin"
 
 struct registry_priv {
diff --git a/drivers/staging/r8188eu/os_dep/os_intfs.c b/drivers/staging/r8188eu/os_dep/os_intfs.c
index 5985054da935..d9abd2a98e1b 100644
--- a/drivers/staging/r8188eu/os_dep/os_intfs.c
+++ b/drivers/staging/r8188eu/os_dep/os_intfs.c
@@ -17,7 +17,7 @@
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("Realtek Wireless Lan Driver");
 MODULE_AUTHOR("Realtek Semiconductor Corp.");
-MODULE_VERSION(DRIVERVERSION);
+MODULE_VERSION(DRV_VERSION);
 MODULE_FIRMWARE(FW_RTL8188EU);
 
 #define CONFIG_BR_EXT_BRNAME "br0"
-- 
2.35.1

Re: [PATCH 3/3] staging: r8188eu: make driver metadata macro names more consistent

Posted by Greg KH 3 years, 8 months ago

On Thu, Aug 04, 2022 at 12:29:10AM +0200, Grzegorz Szymaszek wrote:
> Rename DRIVERVERSION to DRV_VERSION so that it looks more alike the
> other macros, DRV_NAME and FW_*, and matches the most popular (as it
> seems from a quick review) conventions in other drivers.
> 
> Signed-off-by: Grzegorz Szymaszek <gszymaszek@short.pl>
> ---
>  drivers/staging/r8188eu/include/drv_types.h | 5 ++---
>  drivers/staging/r8188eu/os_dep/os_intfs.c   | 2 +-
>  2 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/r8188eu/include/drv_types.h b/drivers/staging/r8188eu/include/drv_types.h
> index f51b83515953..3328c66d1ef1 100644
> --- a/drivers/staging/r8188eu/include/drv_types.h
> +++ b/drivers/staging/r8188eu/include/drv_types.h
> @@ -10,8 +10,6 @@
>  #ifndef __DRV_TYPES_H__
>  #define __DRV_TYPES_H__
>  
> -#define DRV_NAME "r8188eu"

This should just be KBUILD_MODNAME, no need to create yet-another-macro
for this one.

> -
>  #include "osdep_service.h"
>  #include "wlan_bssdef.h"
>  #include "rtw_ht.h"
> @@ -36,7 +34,8 @@
>  #include "rtl8188e_hal.h"
>  #include "rtw_fw.h"
>  
> -#define DRIVERVERSION	"v4.1.4_6773.20130222"
> +#define DRV_NAME	"r8188eu"

Again, KBUILD_MODNAME

> +#define DRV_VERSION	"v4.1.4_6773.20130222"

As the driver is now in the kernel, this "version" string can just go
away.  Can you redo this patch to do the DRV_NAME thing first, and then
drop the DRV_VERSION field after that?

thanks,

greg k-h

Re: [PATCH] staging: r8188eu: add firmware dependency

Posted by Phillip Potter 3 years, 8 months ago

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 <gszymaszek@short.pl>
> ---
>  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 <phil@philpotter.co.uk>

Thanks for the patch :-)

Regards,
Phil

Re: [PATCH] staging: r8188eu: add firmware dependency

Posted by Larry Finger 3 years, 8 months ago

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 <gszymaszek@short.pl>
> ---
>   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 <Larry.Finger@lwfinger.net>

Thanks,

Larry