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
