Search Linux Wireless

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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Sven Dziadek <sven.dziadek@xxxxxx> 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 <sven.dziadek@xxxxxx>
> ---
>
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux