Re: coding style lesson: iwlwifi vs. endianness

[Johannes Berg <johannes@sipsolutions.net>]

[-- Attachment #1: Type: text/plain, Size: 1600 bytes --]


> my_data = read_drom_device()
> swap_to_cpu(my_data)
> do_whatever(my_data)
> store the data somewhere else in the device structs for
> later use, maybe.
> do_something_else(my_data)
> 
> You see that now you have only _one_ place that you have to care about.
> And if you have to write your data back at some point, simply do it
> just before the write.

The thing is that Tomas is saying that because they don't have a
function to "read_from_device()" but that is rather only
"shared_structure->something" it is special and completely different
than regular drivers.

The absolute worst thing imo is in the current radiotap code (before my
patch) where it passes around a value as "u16" but then needs to convert
it to __le16 before accessing bits in it because things are defined as
LE.

Also, let me reiterate the argument that this is not using more
code/run-time-memory size:

Consider

u16 phyflags = le16_to_cpu(shared_struct->phy_flags);

[...]
if (phyflags & ...)
[...]

vs.

[...]
if (shared_struct->phy_flags & ...)
[...]

If you have a LE machine, then the compiler should in both cases emit
the same code. If you have BE machine, you have a *single* "superfluous"
byteswap, and if there are multi-bit fields you have far *fewer*
byteswaps. If you instead wrote

u16 phyflags = le16_to_cpup(&shared_struct->phy_flags);

then most likely the byteswap would be microcoded/done by hardware
during the load.

The actual runtime penalty of byteswaps is insignificant since fetching
the value from memory already takes forever.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]