* Re: [PATCH] staging: r8188eu: add firmware dependency
2022-08-02 17:18 ` [PATCH] staging: r8188eu: add firmware dependency Grzegorz Szymaszek
@ 2022-08-02 18:03 ` Larry Finger
2022-08-02 19:02 ` Phillip Potter
2022-08-03 6:08 ` Greg KH
2 siblings, 0 replies; 18+ messages in thread
From: Larry Finger @ 2022-08-02 18:03 UTC (permalink / raw)
To: Grzegorz Szymaszek, Greg KH, Phillip Potter, linux-kernel, linux-staging
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
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] staging: r8188eu: add firmware dependency
2022-08-02 17:18 ` [PATCH] staging: r8188eu: add firmware dependency Grzegorz Szymaszek
2022-08-02 18:03 ` Larry Finger
@ 2022-08-02 19:02 ` Phillip Potter
2022-08-03 6:08 ` Greg KH
2 siblings, 0 replies; 18+ messages in thread
From: Phillip Potter @ 2022-08-02 19:02 UTC (permalink / raw)
To: Grzegorz Szymaszek
Cc: Greg KH, Grzegorz Szymaszek, linux-kernel, linux-staging, Larry.Finger
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
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] staging: r8188eu: add firmware dependency
2022-08-02 17:18 ` [PATCH] staging: r8188eu: add firmware dependency Grzegorz Szymaszek
2022-08-02 18:03 ` Larry Finger
2022-08-02 19:02 ` Phillip Potter
@ 2022-08-03 6:08 ` Greg KH
2022-08-03 6:33 ` Grzegorz Szymaszek
2022-08-03 22:28 ` [PATCH 1/3] staging: r8188eu: set firmware path in a macro Grzegorz Szymaszek
2 siblings, 2 replies; 18+ messages in thread
From: Greg KH @ 2022-08-03 6:08 UTC (permalink / raw)
To: Grzegorz Szymaszek, Larry Finger, Phillip Potter, linux-kernel,
linux-staging
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
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] staging: r8188eu: add firmware dependency
2022-08-03 6:08 ` Greg KH
@ 2022-08-03 6:33 ` Grzegorz Szymaszek
2022-08-03 6:37 ` Greg KH
2022-08-03 22:28 ` [PATCH 1/3] staging: r8188eu: set firmware path in a macro Grzegorz Szymaszek
1 sibling, 1 reply; 18+ messages in thread
From: Grzegorz Szymaszek @ 2022-08-03 6:33 UTC (permalink / raw)
To: Greg KH
Cc: Larry Finger, Phillip Potter, Grzegorz Szymaszek, linux-kernel,
linux-staging
[-- Attachment #1: Type: text/plain, Size: 754 bytes --]
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] staging: r8188eu: add firmware dependency
2022-08-03 6:33 ` Grzegorz Szymaszek
@ 2022-08-03 6:37 ` Greg KH
0 siblings, 0 replies; 18+ messages in thread
From: Greg KH @ 2022-08-03 6:37 UTC (permalink / raw)
To: Grzegorz Szymaszek, Larry Finger, Phillip Potter, linux-kernel,
linux-staging
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
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/3] staging: r8188eu: set firmware path in a macro
2022-08-03 6:08 ` Greg KH
2022-08-03 6:33 ` Grzegorz Szymaszek
@ 2022-08-03 22:28 ` Grzegorz Szymaszek
2022-08-03 22:29 ` [PATCH 2/3] staging: r8188eu: make use of the DRV_NAME macro Grzegorz Szymaszek
` (2 more replies)
1 sibling, 3 replies; 18+ messages in thread
From: Grzegorz Szymaszek @ 2022-08-03 22:28 UTC (permalink / raw)
To: Larry Finger, Greg KH, Phillip Potter
Cc: Grzegorz Szymaszek, linux-kernel, linux-staging
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
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/3] staging: r8188eu: make use of the DRV_NAME macro
2022-08-03 22:28 ` [PATCH 1/3] staging: r8188eu: set firmware path in a macro Grzegorz Szymaszek
@ 2022-08-03 22:29 ` Grzegorz Szymaszek
2022-08-05 6:40 ` Greg KH
2022-08-03 22:29 ` [PATCH 3/3] staging: r8188eu: make driver metadata macro names more consistent Grzegorz Szymaszek
2022-08-04 20:11 ` [PATCH 1/3] staging: r8188eu: set firmware path in a macro Philipp Hortmann
2 siblings, 1 reply; 18+ messages in thread
From: Grzegorz Szymaszek @ 2022-08-03 22:29 UTC (permalink / raw)
To: Larry Finger, Phillip Potter
Cc: Greg KH, Grzegorz Szymaszek, linux-kernel, linux-staging
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
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] staging: r8188eu: make use of the DRV_NAME macro
2022-08-03 22:29 ` [PATCH 2/3] staging: r8188eu: make use of the DRV_NAME macro Grzegorz Szymaszek
@ 2022-08-05 6:40 ` Greg KH
0 siblings, 0 replies; 18+ messages in thread
From: Greg KH @ 2022-08-05 6:40 UTC (permalink / raw)
To: Grzegorz Szymaszek, Larry Finger, Phillip Potter, linux-kernel,
linux-staging
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
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 3/3] staging: r8188eu: make driver metadata macro names more consistent
2022-08-03 22:28 ` [PATCH 1/3] staging: r8188eu: set firmware path in a macro Grzegorz Szymaszek
2022-08-03 22:29 ` [PATCH 2/3] staging: r8188eu: make use of the DRV_NAME macro Grzegorz Szymaszek
@ 2022-08-03 22:29 ` Grzegorz Szymaszek
2022-08-05 6:39 ` Greg KH
2022-08-04 20:11 ` [PATCH 1/3] staging: r8188eu: set firmware path in a macro Philipp Hortmann
2 siblings, 1 reply; 18+ messages in thread
From: Grzegorz Szymaszek @ 2022-08-03 22:29 UTC (permalink / raw)
To: Larry Finger, Phillip Potter
Cc: Greg KH, Grzegorz Szymaszek, linux-kernel, linux-staging
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
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] staging: r8188eu: make driver metadata macro names more consistent
2022-08-03 22:29 ` [PATCH 3/3] staging: r8188eu: make driver metadata macro names more consistent Grzegorz Szymaszek
@ 2022-08-05 6:39 ` Greg KH
0 siblings, 0 replies; 18+ messages in thread
From: Greg KH @ 2022-08-05 6:39 UTC (permalink / raw)
To: Grzegorz Szymaszek, Larry Finger, Phillip Potter, linux-kernel,
linux-staging
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
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] staging: r8188eu: set firmware path in a macro
2022-08-03 22:28 ` [PATCH 1/3] staging: r8188eu: set firmware path in a macro Grzegorz Szymaszek
2022-08-03 22:29 ` [PATCH 2/3] staging: r8188eu: make use of the DRV_NAME macro Grzegorz Szymaszek
2022-08-03 22:29 ` [PATCH 3/3] staging: r8188eu: make driver metadata macro names more consistent Grzegorz Szymaszek
@ 2022-08-04 20:11 ` Philipp Hortmann
2022-08-04 22:23 ` Grzegorz Szymaszek
2 siblings, 1 reply; 18+ messages in thread
From: Philipp Hortmann @ 2022-08-04 20:11 UTC (permalink / raw)
To: Grzegorz Szymaszek, Larry Finger, Greg KH, Phillip Potter,
linux-kernel, linux-staging
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
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] staging: r8188eu: set firmware path in a macro
2022-08-04 20:11 ` [PATCH 1/3] staging: r8188eu: set firmware path in a macro Philipp Hortmann
@ 2022-08-04 22:23 ` Grzegorz Szymaszek
2022-08-05 4:13 ` Greg KH
0 siblings, 1 reply; 18+ messages in thread
From: Grzegorz Szymaszek @ 2022-08-04 22:23 UTC (permalink / raw)
To: Philipp Hortmann
Cc: Larry Finger, Greg KH, Phillip Potter, Grzegorz Szymaszek,
linux-kernel, linux-staging
[-- Attachment #1: Type: text/plain, Size: 1336 bytes --]
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!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] staging: r8188eu: set firmware path in a macro
2022-08-04 22:23 ` Grzegorz Szymaszek
@ 2022-08-05 4:13 ` Greg KH
0 siblings, 0 replies; 18+ messages in thread
From: Greg KH @ 2022-08-05 4:13 UTC (permalink / raw)
To: Grzegorz Szymaszek, Philipp Hortmann, Larry Finger,
Phillip Potter, linux-kernel, linux-staging
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
^ permalink raw reply [flat|nested] 18+ messages in thread