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/[email protected]/
Suggested-by: Greg Kroah-Hartman <[email protected]>
Signed-off-by: Grzegorz Szymaszek <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]> # Edimax N150
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!
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
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 <[email protected]>
> ---
> 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