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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux