| Message ID | 20240223115214.682fb94159fa.I576bbf9fe7ca2948dbe3e00c0fa0f37594e85046@changeid (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | [v2] wifi: rsi: fix endian conversions | expand |
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.
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 --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);