Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1300835

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

Show all headers | View raw

Sven Dziadek <sven.dziadek@gmx.de> 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@gmx.de>
> ---
>
> 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-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[RFC PATCH] staging: rtl8723au: fix byte order problems Sven Dziadek <sven.dziadek@gmx.de> - 2016-01-04 16:40 +0100
  Re: [RFC PATCH] staging: rtl8723au: fix byte order problems Jes Sorensen <Jes.Sorensen@redhat.com> - 2016-01-04 16:50 +0100
  Re: [RFC PATCH] staging: rtl8723au: fix byte order problems Julian Calaby <julian.calaby@gmail.com> - 2016-01-04 23:00 +0100
    Re: [RFC PATCH] staging: rtl8723au: fix byte order problems Jes Sorensen <Jes.Sorensen@redhat.com> - 2016-01-05 17:00 +0100
      Re: [RFC PATCH] staging: rtl8723au: fix byte order problems Larry Finger <Larry.Finger@lwfinger.net> - 2016-01-05 17:20 +0100

csiph-web