Re: [PATCH 5/5] ath9k: Make EEPROM endianness swapping configurable via devicetree

On Tuesday 30 August 2016, Martin Blumenstingl wrote:

new logic (assuming that we went for __le16/__le32 fields):
- reading data: use le16_to_cpu and le32_to_cpu for all fields

LE system:
- LE EEPROM -> no swap will be applied
- BE EEPROM -> be16_to_cpu / be32_to_cpu (or swab16 / swab32 as before)
BE system:
- LE EEPROM -> le16_to_cpu / le32_to_cpu (or swab16 / swab32 as before)
- BE EEPROM -> no swap will be applied

I think this should be:

LE and BE systems:
 - LE EEPROM -> no swap will be applied
 - BE EEPROM -> or swab16 / swab32

The upside of this is that we no longer care about what the CPU is,
and in my opinion that makes the code easier to understand.

There is one downside of the "new approach" I can think of: you need
to swap the data twice in some cases (BE EEPROM on a BE machine).
- first swap while writing the data to __le16/__le32 fields
- second swap while reading the data from the __le16/__le32 fields
If you forget to swap a field in either place then things will be broken.

Correct. Fortunately, "make C=1" with sparse helps you find those bugs
at compile time.

Maybe someone else wants to state his/her opinion on this - I guess
some fresh thoughts could help us a lot!

Yes, that would be helpful. It's possible I'm missing something here,
or that changing this will just add confusion with other people.

	Arnd