[PATCH 0/4] wifi: drivers: fix some sparse warnings

[PATCH 0/4] wifi: drivers: fix some sparse warnings

[Kalle Valo]

Trying to reduce our sparse warnings, end goal is that wireless drivers would
be sparse warning free.

Kalle Valo (4):
  wifi: zd1211rw: remove __nocast from zd_addr_t
  wifi: rsi: fix restricted __le32 degrades to integer sparse warnings
  wifi: cw1200: fix __le16 sparse warnings
  wifi: rt2x00: fix __le32 sparse warnings

 drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 15 +++++++++++----
 drivers/net/wireless/rsi/rsi_91x_usb.c         | 12 ++++++------
 drivers/net/wireless/st/cw1200/cw1200_spi.c    |  6 ------
 drivers/net/wireless/zydas/zd1211rw/zd_def.h   |  2 +-
 4 files changed, 18 insertions(+), 17 deletions(-)


base-commit: 28b3df1fe6ba2cb439ba109f095aa841fef3a54f
-- 
2.39.2

[PATCH 1/4] wifi: zd1211rw: remove __nocast from zd_addr_t

[Kalle Valo]

Sparse warns:

drivers/net/wireless/zydas/zd1211rw/zd_usb.c:383:24: warning: implicit cast from nocast type
drivers/net/wireless/zydas/zd1211rw/zd_usb.c:419:24: warning: implicit cast from nocast type

This is an ancient driver which has not have any meaningfuli changes for a long
time and hopefully removed soon. So just remove the __nocast to get rid of the
sparse warnings.

Compile tested only.

Signed-off-by: Kalle Valo <kvalo@kernel.org>
---
 drivers/net/wireless/zydas/zd1211rw/zd_def.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/zydas/zd1211rw/zd_def.h b/drivers/net/wireless/zydas/zd1211rw/zd_def.h
index 8ca2d0aab170..2f55e8deee82 100644
--- a/drivers/net/wireless/zydas/zd1211rw/zd_def.h
+++ b/drivers/net/wireless/zydas/zd1211rw/zd_def.h
@@ -12,7 +12,7 @@
 #include <linux/stringify.h>
 #include <linux/device.h>
 
-typedef u16 __nocast zd_addr_t;
+typedef u16 zd_addr_t;
 
 #define dev_printk_f(level, dev, fmt, args...) \
 	dev_printk(level, dev, "%s() " fmt, __func__, ##args)
-- 
2.39.2

Re: [PATCH 1/4] wifi: zd1211rw: remove __nocast from zd_addr_t

[Kalle Valo]

Kalle Valo <kvalo@kernel.org> wrote:

> Sparse warns:
> 
> drivers/net/wireless/zydas/zd1211rw/zd_usb.c:383:24: warning: implicit cast from nocast type
> drivers/net/wireless/zydas/zd1211rw/zd_usb.c:419:24: warning: implicit cast from nocast type
> 
> This is an ancient driver which has not have any meaningfuli changes for a long
> time and hopefully removed soon. So just remove the __nocast to get rid of the
> sparse warnings.
> 
> Compile tested only.
> 
> Signed-off-by: Kalle Valo <kvalo@kernel.org>

3 patches applied to wireless-next.git, thanks.

2b59c9c30b9c wifi: zd1211rw: remove __nocast from zd_addr_t
0583e5acaf43 wifi: rsi: fix restricted __le32 degrades to integer sparse warnings
7ceade653429 wifi: cw1200: fix __le16 sparse warnings

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20240130151556.2315951-2-kvalo@kernel.org/

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

[PATCH 2/4] wifi: rsi: fix restricted __le32 degrades to integer sparse warnings

[Kalle Valo]

drivers/net/wireless/rsi/rsi_91x_usb.c:235:27: warning: restricted __le32 degrades to integer
drivers/net/wireless/rsi/rsi_91x_usb.c:236:27: warning: restricted __le32 degrades to integer
drivers/net/wireless/rsi/rsi_91x_usb.c:237:27: warning: restricted __le32 degrades to integer
drivers/net/wireless/rsi/rsi_91x_usb.c:238:27: warning: restricted __le32 degrades to integer
drivers/net/wireless/rsi/rsi_91x_usb.c:244:36: warning: restricted __le32 degrades to integer
drivers/net/wireless/rsi/rsi_91x_usb.c:245:35: warning: restricted __le32 degrades to integer

These cpu_to_le32() are not making sense. With usb_reg_buf we handle the values
byte at a time to make sure usb_reg_buf is in little endian so no need to
convert anything. And usb_control_msg() expects to have the values in native
endian anyway. So just remove these so they are not spamming our logs.

Compile tested only.

Signed-off-by: Kalle Valo <kvalo@kernel.org>
---
 drivers/net/wireless/rsi/rsi_91x_usb.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/rsi/rsi_91x_usb.c b/drivers/net/wireless/rsi/rsi_91x_usb.c
index 10a465686439..dccc139cabb2 100644
--- a/drivers/net/wireless/rsi/rsi_91x_usb.c
+++ b/drivers/net/wireless/rsi/rsi_91x_usb.c
@@ -232,17 +232,17 @@ static int rsi_usb_reg_write(struct usb_device *usbdev,
 	if (!usb_reg_buf)
 		return status;
 
-	usb_reg_buf[0] = (cpu_to_le32(value) & 0x00ff);
-	usb_reg_buf[1] = (cpu_to_le32(value) & 0xff00) >> 8;
-	usb_reg_buf[2] = (cpu_to_le32(value) & 0x00ff0000) >> 16;
-	usb_reg_buf[3] = (cpu_to_le32(value) & 0xff000000) >> 24;
+	usb_reg_buf[0] = value & 0x00ff;
+	usb_reg_buf[1] = (value & 0xff00) >> 8;
+	usb_reg_buf[2] = (value & 0x00ff0000) >> 16;
+	usb_reg_buf[3] = (value & 0xff000000) >> 24;
 
 	status = usb_control_msg(usbdev,
 				 usb_sndctrlpipe(usbdev, 0),
 				 USB_VENDOR_REGISTER_WRITE,
 				 RSI_USB_REQ_OUT,
-				 ((cpu_to_le32(reg) & 0xffff0000) >> 16),
-				 (cpu_to_le32(reg) & 0xffff),
+				 (reg & 0xffff0000) >> 16,
+				 reg & 0xffff,
 				 (void *)usb_reg_buf,
 				 len,
 				 USB_CTRL_SET_TIMEOUT);
-- 
2.39.2

[PATCH 3/4] wifi: cw1200: fix __le16 sparse warnings

[Kalle Valo]

Sparse warns:

drivers/net/wireless/st/cw1200/cw1200_spi.c:83:17:    got restricted __le16 [usertype]
drivers/net/wireless/st/cw1200/cw1200_spi.c:148:17: warning: incorrect type in assignment (different base types)
drivers/net/wireless/st/cw1200/cw1200_spi.c:148:17:    expected unsigned short [addressable] [assigned] [usertype] regaddr
drivers/net/wireless/st/cw1200/cw1200_spi.c:148:17:    got restricted __le16 [usertype]

These cpu_to_le16() calls are not really making any sense to me. On a big
endian system we first convert regaddr from big to little using cpu_to_le16()
but immediately after we convert them back to big endian? So just remove them.

Compile tested only.

Signed-off-by: Kalle Valo <kvalo@kernel.org>
---
 drivers/net/wireless/st/cw1200/cw1200_spi.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/net/wireless/st/cw1200/cw1200_spi.c b/drivers/net/wireless/st/cw1200/cw1200_spi.c
index c82c0688b549..b27b57fc25bc 100644
--- a/drivers/net/wireless/st/cw1200/cw1200_spi.c
+++ b/drivers/net/wireless/st/cw1200/cw1200_spi.c
@@ -79,9 +79,6 @@ static int cw1200_spi_memcpy_fromio(struct hwbus_priv *self,
 	pr_info("READ : %04d from 0x%02x (%04x)\n", count, addr, regaddr);
 #endif
 
-	/* Header is LE16 */
-	regaddr = cpu_to_le16(regaddr);
-
 	/* We have to byteswap if the SPI bus is limited to 8b operation
 	   or we are running on a Big Endian system
 	*/
@@ -144,9 +141,6 @@ static int cw1200_spi_memcpy_toio(struct hwbus_priv *self,
 	pr_info("WRITE: %04d  to  0x%02x (%04x)\n", count, addr, regaddr);
 #endif
 
-	/* Header is LE16 */
-	regaddr = cpu_to_le16(regaddr);
-
 	/* We have to byteswap if the SPI bus is limited to 8b operation
 	   or we are running on a Big Endian system
 	*/
-- 
2.39.2

[PATCH 4/4] wifi: rt2x00: fix __le32 sparse warnings

[Kalle Valo]

Sparse warns:

drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10949:39: warning: incorrect type in assignment (different base types)
drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10949:39:    expected unsigned int [usertype]
drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10949:39:    got restricted __le32 [usertype]
drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10951:43: warning: incorrect type in assignment (different base types)
drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10951:43:    expected unsigned int [usertype]
drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10951:43:    got restricted __le32 [usertype]
drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10953:43: warning: incorrect type in assignment (different base types)
drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10953:43:    expected unsigned int [usertype]
drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10953:43:    got restricted __le32 [usertype]
drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10955:43: warning: incorrect type in assignment (different base types)
drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10955:43:    expected unsigned int [usertype]
drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10955:43:    got restricted __le32 [usertype]

rt2x00 does some wicked casting here so no wonder sparse warns. Clean that up
and use cpu_to_le16() to avoid any warnings.

Compile tested only.

Signed-off-by: Kalle Valo <kvalo@kernel.org>
---
 drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
index aaf31857ae1e..8e8fab88fb2b 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
@@ -10946,13 +10946,20 @@ static void rt2800_efuse_read(struct rt2x00_dev *rt2x00dev, unsigned int i)
 	/* Apparently the data is read from end to start */
 	reg = rt2800_register_read_lock(rt2x00dev, efuse_data3_reg);
 	/* The returned value is in CPU order, but eeprom is le */
-	*(u32 *)&rt2x00dev->eeprom[i] = cpu_to_le32(reg);
+	rt2x00dev->eeprom[i] = cpu_to_le16(reg);
+	rt2x00dev->eeprom[i + 1] = cpu_to_le16(upper_16_bits(reg));
+
 	reg = rt2800_register_read_lock(rt2x00dev, efuse_data2_reg);
-	*(u32 *)&rt2x00dev->eeprom[i + 2] = cpu_to_le32(reg);
+	rt2x00dev->eeprom[i + 2] = cpu_to_le16(reg);
+	rt2x00dev->eeprom[i + 3] = cpu_to_le16(upper_16_bits(reg));
+
 	reg = rt2800_register_read_lock(rt2x00dev, efuse_data1_reg);
-	*(u32 *)&rt2x00dev->eeprom[i + 4] = cpu_to_le32(reg);
+	rt2x00dev->eeprom[i + 4] = cpu_to_le16(reg);
+	rt2x00dev->eeprom[i + 5] = cpu_to_le16(upper_16_bits(reg));
+
 	reg = rt2800_register_read_lock(rt2x00dev, efuse_data0_reg);
-	*(u32 *)&rt2x00dev->eeprom[i + 6] = cpu_to_le32(reg);
+	rt2x00dev->eeprom[i + 6] = cpu_to_le16(reg);
+	rt2x00dev->eeprom[i + 7] = cpu_to_le16(upper_16_bits(reg));
 
 	mutex_unlock(&rt2x00dev->csr_mutex);
 }
-- 
2.39.2

Re: [PATCH 4/4] wifi: rt2x00: fix __le32 sparse warnings

[Kalle Valo]

Kalle Valo <kvalo@kernel.org> writes:

> Sparse warns:
>
> drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10949:39: warning: incorrect type in assignment (different base types)
> drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10949:39:    expected unsigned int [usertype]
> drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10949:39:    got restricted __le32 [usertype]
> drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10951:43: warning: incorrect type in assignment (different base types)
> drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10951:43:    expected unsigned int [usertype]
> drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10951:43:    got restricted __le32 [usertype]
> drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10953:43: warning: incorrect type in assignment (different base types)
> drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10953:43:    expected unsigned int [usertype]
> drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10953:43:    got restricted __le32 [usertype]
> drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10955:43: warning: incorrect type in assignment (different base types)
> drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10955:43:    expected unsigned int [usertype]
> drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10955:43:    got restricted __le32 [usertype]
>
> rt2x00 does some wicked casting here so no wonder sparse warns. Clean that up
> and use cpu_to_le16() to avoid any warnings.
>
> Compile tested only.
>
> Signed-off-by: Kalle Valo <kvalo@kernel.org>

Stanislaw, sorry somehow I corrupted your address but fixed it now. Let
me know what you think, patch here:

https://patchwork.kernel.org/project/linux-wireless/patch/20240130151556.2315951-5-kvalo@kernel.org/

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

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

Re: [PATCH 4/4] wifi: rt2x00: fix __le32 sparse warnings

[Stanislaw Gruszka]

Hi Kalle

On Wed, Jan 31, 2024 at 09:48:44AM +0200, Kalle Valo wrote:
> Kalle Valo <kvalo@kernel.org> writes:
> 
> > Sparse warns:
> >
> > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10949:39: warning: incorrect type in assignment (different base types)
> > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10949:39:    expected unsigned int [usertype]
> > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10949:39:    got restricted __le32 [usertype]
> > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10951:43: warning: incorrect type in assignment (different base types)
> > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10951:43:    expected unsigned int [usertype]
> > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10951:43:    got restricted __le32 [usertype]
> > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10953:43: warning: incorrect type in assignment (different base types)
> > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10953:43:    expected unsigned int [usertype]
> > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10953:43:    got restricted __le32 [usertype]
> > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10955:43: warning: incorrect type in assignment (different base types)
> > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10955:43:    expected unsigned int [usertype]
> > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10955:43:    got restricted __le32 [usertype]
> >
> > rt2x00 does some wicked casting here so no wonder sparse warns. Clean that up
> > and use cpu_to_le16() to avoid any warnings.
> >
> > Compile tested only.
> >
> > Signed-off-by: Kalle Valo <kvalo@kernel.org>
> 
> Stanislaw, sorry somehow I corrupted your address but fixed it now. Let
> me know what you think, patch here:
> 
> https://patchwork.kernel.org/project/linux-wireless/patch/20240130151556.2315951-5-kvalo@kernel.org/

If I analyse it correctly, patch is not ok on Big Endian machines
where we do bytes swapping.

Let asume reg value is 0x0D0C0B0A

On previous code:

-	*(u32 *)&rt2x00dev->eeprom[i] = cpu_to_le32(reg);

result is

eeprom[0] = 0x0D
eeprom[1] = 0x0C
eeprom[2] = 0x0B
eeprom[3] = 0x0A

After modification:

+	rt2x00dev->eeprom[i] = cpu_to_le16(reg);
+	rt2x00dev->eeprom[i + 1] = cpu_to_le16(upper_16_bits(reg));

result will be

eeprom[0] = 0x0B
eeprom[1] = 0x0A
eeprom[2] = 0x0D
eeprom[2] = 0x0C

Maybe just casting to __le32, i.e:
	
	*(__le32 *)&rt2x00dev->eeprom[i] = cpu_to_le32(reg);

would solve the problem ?

Regards
Stanislaw

Re: [PATCH 4/4] wifi: rt2x00: fix __le32 sparse warnings

[Kalle Valo]

Stanislaw Gruszka <stf_xl@wp.pl> writes:

> On Wed, Jan 31, 2024 at 09:48:44AM +0200, Kalle Valo wrote:
>> Kalle Valo <kvalo@kernel.org> writes:
>> 
>> > Sparse warns:
>> >
>> > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10949:39: warning: incorrect type in assignment (different base types)
>> > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10949:39:    expected unsigned int [usertype]
>> > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10949:39:    got restricted __le32 [usertype]
>> > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10951:43: warning: incorrect type in assignment (different base types)
>> > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10951:43:    expected unsigned int [usertype]
>> > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10951:43:    got restricted __le32 [usertype]
>> > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10953:43: warning: incorrect type in assignment (different base types)
>> > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10953:43:    expected unsigned int [usertype]
>> > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10953:43:    got restricted __le32 [usertype]
>> > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10955:43: warning: incorrect type in assignment (different base types)
>> > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10955:43:    expected unsigned int [usertype]
>> > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10955:43:    got restricted __le32 [usertype]
>> >
>> > rt2x00 does some wicked casting here so no wonder sparse warns. Clean that up
>> > and use cpu_to_le16() to avoid any warnings.
>> >
>> > Compile tested only.
>> >
>> > Signed-off-by: Kalle Valo <kvalo@kernel.org>
>> 
>> Stanislaw, sorry somehow I corrupted your address but fixed it now. Let
>> me know what you think, patch here:
>> 
>> https://patchwork.kernel.org/project/linux-wireless/patch/20240130151556.2315951-5-kvalo@kernel.org/
>
> If I analyse it correctly, patch is not ok on Big Endian machines
> where we do bytes swapping.

Ouch, thanks for catching this. Johannes sent a new version:

https://patchwork.kernel.org/project/linux-wireless/patch/20240223114023.ce0c714124e9.I2b5710b761f63522574fbe7654d37151c31e0b77@changeid/

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

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

Re: [PATCH 4/4] wifi: rt2x00: fix __le32 sparse warnings

[Stanislaw Gruszka]

On Wed, Feb 28, 2024 at 11:44:25AM +0200, Kalle Valo wrote:
> Stanislaw Gruszka <stf_xl@wp.pl> writes:
> 
> > On Wed, Jan 31, 2024 at 09:48:44AM +0200, Kalle Valo wrote:
> >> Kalle Valo <kvalo@kernel.org> writes:
> >> 
> >> > Sparse warns:
> >> >
> >> > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10949:39: warning: incorrect type in assignment (different base types)
> >> > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10949:39:    expected unsigned int [usertype]
> >> > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10949:39:    got restricted __le32 [usertype]
> >> > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10951:43: warning: incorrect type in assignment (different base types)
> >> > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10951:43:    expected unsigned int [usertype]
> >> > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10951:43:    got restricted __le32 [usertype]
> >> > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10953:43: warning: incorrect type in assignment (different base types)
> >> > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10953:43:    expected unsigned int [usertype]
> >> > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10953:43:    got restricted __le32 [usertype]
> >> > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10955:43: warning: incorrect type in assignment (different base types)
> >> > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10955:43:    expected unsigned int [usertype]
> >> > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10955:43:    got restricted __le32 [usertype]
> >> >
> >> > rt2x00 does some wicked casting here so no wonder sparse warns. Clean that up
> >> > and use cpu_to_le16() to avoid any warnings.
> >> >
> >> > Compile tested only.
> >> >
> >> > Signed-off-by: Kalle Valo <kvalo@kernel.org>
> >> 
> >> Stanislaw, sorry somehow I corrupted your address but fixed it now. Let
> >> me know what you think, patch here:
> >> 
> >> https://patchwork.kernel.org/project/linux-wireless/patch/20240130151556.2315951-5-kvalo@kernel.org/
> >
> > If I analyse it correctly, patch is not ok on Big Endian machines
> > where we do bytes swapping.
> 
> Ouch, thanks for catching this. Johannes sent a new version:
> 
> https://patchwork.kernel.org/project/linux-wireless/patch/20240223114023.ce0c714124e9.I2b5710b761f63522574fbe7654d37151c31e0b77@changeid/
That one looks good.

Thanks
Stanislaw