diff mbox series

[v2] wifi: rsi: fix endian conversions

Message ID 20240223115214.682fb94159fa.I576bbf9fe7ca2948dbe3e00c0fa0f37594e85046@changeid
State New
Headers show
Series [v2] wifi: rsi: fix endian conversions | expand

Commit Message

Johannes Berg Feb. 23, 2024, 10:52 a.m. UTC
From: Johannes Berg <johannes.berg@intel.com>

This really seems like a bug, endian conversions now happen
twice in this code.

Note also that prior to the commit mentioned below, the code
was putting 16-bit values 0xBBAA as bytes "AA BB 00 00", and
the commit mentions making it work for 32-bit values and
makes no mention of fixing endian conversion; however, after
it, the bytes for 0xBBAA would now be "00 00 BB AA" on big
endian platforms.

Remove one conversion to make sparse no longer warn.

Not sure anyone can, has, or ever will use this on big endian
platforms though.

Fixes: 0a60014b76f5 ("rsi: miscallaneous changes for 9116 and common")
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 drivers/net/wireless/rsi/rsi_91x_usb.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

Comments

Kalle Valo Feb. 28, 2024, 11:36 a.m. UTC | #1
Johannes Berg <johannes@sipsolutions.net> wrote:

> From: Johannes Berg <johannes.berg@intel.com>
> 
> This really seems like a bug, endian conversions now happen
> twice in this code.
> 
> Note also that prior to the commit mentioned below, the code
> was putting 16-bit values 0xBBAA as bytes "AA BB 00 00", and
> the commit mentions making it work for 32-bit values and
> makes no mention of fixing endian conversion; however, after
> it, the bytes for 0xBBAA would now be "00 00 BB AA" on big
> endian platforms.
> 
> Remove one conversion to make sparse no longer warn.
> 
> Not sure anyone can, has, or ever will use this on big endian
> platforms though.
> 
> Fixes: 0a60014b76f5 ("rsi: miscallaneous changes for 9116 and common")
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>

Fails to apply, I guess because I had a patch for this warning earlier.

Recorded preimage for 'drivers/net/wireless/rsi/rsi_91x_usb.c'
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Applying: wifi: rsi: fix endian conversions
Using index info to reconstruct a base tree...
M	drivers/net/wireless/rsi/rsi_91x_usb.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/net/wireless/rsi/rsi_91x_usb.c
CONFLICT (content): Merge conflict in drivers/net/wireless/rsi/rsi_91x_usb.c
Patch failed at 0001 wifi: rsi: fix endian conversions

Patch set to Changes Requested.
Johannes Berg Feb. 28, 2024, 11:36 a.m. UTC | #2
On Wed, 2024-02-28 at 11:36 +0000, Kalle Valo wrote:
> Johannes Berg <johannes@sipsolutions.net> wrote:
> 
> > From: Johannes Berg <johannes.berg@intel.com>
> > 
> > This really seems like a bug, endian conversions now happen
> > twice in this code.
> > 
> > Note also that prior to the commit mentioned below, the code
> > was putting 16-bit values 0xBBAA as bytes "AA BB 00 00", and
> > the commit mentions making it work for 32-bit values and
> > makes no mention of fixing endian conversion; however, after
> > it, the bytes for 0xBBAA would now be "00 00 BB AA" on big
> > endian platforms.
> > 
> > Remove one conversion to make sparse no longer warn.
> > 
> > Not sure anyone can, has, or ever will use this on big endian
> > platforms though.
> > 
> > Fixes: 0a60014b76f5 ("rsi: miscallaneous changes for 9116 and common")
> > Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> 
> Fails to apply, I guess because I had a patch for this warning earlier.
> 

You did pretty much the same thing anyway, no need to do anything else.

johannes
diff mbox series

Patch

diff --git a/drivers/net/wireless/rsi/rsi_91x_usb.c b/drivers/net/wireless/rsi/rsi_91x_usb.c
index 10a465686439..0ce8c9aad1f1 100644
--- a/drivers/net/wireless/rsi/rsi_91x_usb.c
+++ b/drivers/net/wireless/rsi/rsi_91x_usb.c
@@ -222,7 +222,7 @@  static int rsi_usb_reg_write(struct usb_device *usbdev,
 			     u32 value,
 			     u16 len)
 {
-	u8 *usb_reg_buf;
+	__le32 *usb_reg_buf;
 	int status = -ENOMEM;
 
 	if (len > RSI_USB_CTRL_BUF_SIZE)
@@ -232,17 +232,14 @@  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] = cpu_to_le32(value);
 
 	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),
+				 upper_16_bits(reg),
+				 lower_16_bits(reg),
 				 (void *)usb_reg_buf,
 				 len,
 				 USB_CTRL_SET_TIMEOUT);