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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
Kalle Valo <[email protected]> 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 <[email protected]>
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/[email protected]/
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Hi Kalle
On Wed, Jan 31, 2024 at 09:48:44AM +0200, Kalle Valo wrote:
> Kalle Valo <[email protected]> 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 <[email protected]>
>
> 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/[email protected]/
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
Kalle Valo <[email protected]> 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 <[email protected]>
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/[email protected]/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Stanislaw Gruszka <[email protected]> writes:
> On Wed, Jan 31, 2024 at 09:48:44AM +0200, Kalle Valo wrote:
>> Kalle Valo <[email protected]> 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 <[email protected]>
>>
>> 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/[email protected]/
>
> 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
On Wed, Feb 28, 2024 at 11:44:25AM +0200, Kalle Valo wrote:
> Stanislaw Gruszka <[email protected]> writes:
>
> > On Wed, Jan 31, 2024 at 09:48:44AM +0200, Kalle Valo wrote:
> >> Kalle Valo <[email protected]> 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 <[email protected]>
> >>
> >> 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/[email protected]/
> >
> > 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