Re: [PATCH v5 1/2] wireless: Driver for 60GHz card wil6210

[Vladimir Kondratiev <qca_vkondrat@qca.qualcomm.com>]

On Monday, November 12, 2012 07:22:19 PM Kalle Valo wrote:
> Vladimir Kondratiev <qca_vkondrat@qca.qualcomm.com> writes:
> >> Please see
> >> ath6kl's struct wmi_begin_scan_cmd on wmi.h for an example
> >> of how to specify endianess to ensure that you can use this
> >> driver on big or little endian. So although its great you
> >> managed to successfully use sparse with -D__CHECK_ENDIAN__
> >> endianess may be broken on this command and all over your
> >> wmi.h. I won't even review that file then.
> > 
> > I know about __le16 etc., but I have no big endian host to check it with.
> > Thus, I'd prefer to mention that code is not ready for big endian platform
> > rather then submit untested code. If there is a way to check endiannes
> > without having big endian host, I'd be happy to know.
> > Also, is there a good way to check in Kconfig we are on Little endian?
> 
> I think this is wrong aproach. The endian support doesn't need to be
> perfect, especially if you don't have any hardware to test with. Just
> add endian annotations to all the fields you know of, run sparse and fix
> the warnings. This should only take few hours and is much better than
> refusing to compile on big endian machines.
> 
> You will get very close to a working code with that method, and it means
> that all future code will have endian support "automatically" (as the
> code will follow the style already used in the driver). And whenever
> someone will try the driver in a big endian machine there's less to
> debug and fix, with very good luck (and careful review!) it might even
> work out of box :)

Yes, it make sense. I noticed also I have to use __raw_{read|write}l functions 
to copy buffers to/from PCI iomem, as otherwise io{read|write}32 would swap 
bytes for big endian hosts, since PCI is little endian.

I will go for it.

Thanks, Vladimir