Thread (1 message) 1 message, 1 author, 2016-01-04

Re: [RFC PATCH] staging: rtl8723au: fix byte order problems

From: Jes Sorensen <hidden>
Date: 2016-01-04 15:47:15
Also in: lkml

Sven Dziadek [off-list ref] writes:
Remove byte order conversions.
Conversion is already done in usb_ops_linux.c when accessing usb port.
The deleted lines convert to little-endian and then call FillH2CCmd to
convert back. Additionally, they are applied to wrong types and
process wrong parts of variables later on.

Signed-off-by: Sven Dziadek <redacted>
---

I was looking for some Sparse warnings that I could fix and found this:

drivers/staging/rtl8723au//hal/rtl8723a_cmd.c:96:38: warning: cast to
restricted __le16
drivers/staging/rtl8723au//hal/rtl8723a_cmd.c:100:27: warning: cast to
restricted __le32
drivers/staging/rtl8723au//hal/rtl8723a_cmd.c:118:25: warning: incorrect
type in assignment (different base types)
drivers/staging/rtl8723au//hal/rtl8723a_cmd.c:118:25:    expected
unsigned int [unsigned] [usertype] <noident>
drivers/staging/rtl8723au//hal/rtl8723a_cmd.c:118:25:    got restricted
__le32 [usertype] <noident>

The rtl8723au drivers seems to work on many machines already, but
probably mostly on little-endian machines.
The file staging/rtl8723au/hal/rtl8723a_cmd.c contains four conversions
from or to little-endian that look suspicious.
The best example is:

int rtl8723a_set_rssi_cmd(struct rtw_adapter *padapter, u8 *param)
{
	*((u32 *)param) = cpu_to_le32(*((u32 *)param));

	FillH2CCmd(padapter, RSSI_SETTING_EID, 3, param);

	return _SUCCESS;
}

On little-endian, the conversion does nothing, but on big-endian, it
swaps the order and then only 3 bytes are used in FillH2CCmd (3rd
argument). So it actually ignores the original argument param.

I think it is best to delete these conversions because the actual
conversions are done when transferring data to or from the usb port already.

Unfortunately, I do not have the hardware to test it.
What do you think about the patch?
Be *very* careful with this - there is a lot of dodgy passing back and
forth to the hardware and the format needs to match what the firmware
expects.

Unless you are going to test this and confirm that it actually works,
then please stay away from this.

NAK

Thanks,
Jes
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help