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
