| Message ID | 20240130151556.2315951-5-kvalo@kernel.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | wifi: drivers: fix some sparse warnings | expand |
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/
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
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/
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
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); }
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(-)