2024-02-23 11:36:37

by Johannes Berg

[permalink] [raw]
Subject: [PATCH v2] wifi: rsi: fix endian conversions

From: Johannes Berg <[email protected]>

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 <[email protected]>
---
drivers/net/wireless/rsi/rsi_91x_usb.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)

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);
--
2.43.2



2024-02-28 11:44:45

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v2] wifi: rsi: fix endian conversions

On Wed, 2024-02-28 at 11:36 +0000, Kalle Valo wrote:
> Johannes Berg <[email protected]> wrote:
>
> > From: Johannes Berg <[email protected]>
> >
> > 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 <[email protected]>
>
> 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

2024-02-28 12:20:31

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH v2] wifi: rsi: fix endian conversions

Johannes Berg <[email protected]> wrote:

> From: Johannes Berg <[email protected]>
>
> 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 <[email protected]>

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.

--
https://patchwork.kernel.org/project/linux-wireless/patch/20240223115214.682fb94159fa.I576bbf9fe7ca2948dbe3e00c0fa0f37594e85046@changeid/

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