On Mon, Aug 29, 2016 at 2:10 PM, Arnd Bergmann <arnd@xxxxxxxx> wrote:
>> Without patch 4 from this series the EEPROM data is loaded like this:
>> 1. out of tree code extracts the EEPROM data from NOR/SPI/NAND flash
>> 2. board specific entry (usually device-tree) tells the code to apply
>> swab16 to the data
>> 3. board specific entry (usually device-tree again) sets the
>> "endian_check" property in the ath9k_platform_data to true
>> 4.a ath9k sees that the magic bytes are not matching and applies swab16
>> 4.b while doing that it notifies the EEPROM format specific swapping
>>
>> However, with patch 4 from this series steps 4.a and 4.b are replaced with:
>> 4. ath9k checks the eepMisc field of the EEPROM and applies the EEPROM
>> format specific swapping
>
> I think the intention of the patch is right, but it needs to go further:
> It seems wrong to have 'u32' members e.g. in
>
> struct modal_eep_ar9287_header {
> u32 antCtrlChain[AR9287_MAX_CHAINS];
> u32 antCtrlCommon;
>
> and then do a swab32() on them. I suspect these should just be __le32
> (and __le16, respectively where needed), using appropriate accessors
> everywhere that those fields are being read instead of having one function
> that tries to turn them into cpu-native ordering.
I'm not sure if we want those fields to be __le32:
BIT(0) in the eepMisc field indicates whether to interpret the data as
big or little endian.
When this bit is set we would want these fields to be __be32 instead -
so I guess the current implementation is "OK" for this specific
use-case.
> If we can manage to convert the code into doing this consistently,
> maybe only the 16-bit swaps of the data stream are left over.
we don't need the 16bit swap anymore - at least on the lantiq devices
that I know (not sure about other platforms / devices though).
Martin
