iwlwifi: fix EEPROM/OTP reading endian annotations and a bug
authorJohannes Berg <johannes@sipsolutions.net>
Mon, 14 Dec 2009 22:12:08 +0000 (14:12 -0800)
committerGreg Kroah-Hartman <gregkh@suse.de>
Wed, 6 Jan 2010 23:04:24 +0000 (15:04 -0800)
commitdf5d119f46660631b9d50b91b1b888cdb512bdb2
tree090b0f2f635c0a4f73c0045c7c5572f4f30ca3d1
parent0c0cdaff83704490511b462c7ba76d3ff2d0eedb
iwlwifi: fix EEPROM/OTP reading endian annotations and a bug

commit af6b8ee38833b39f70946f767740565ceb126961 upstream.

The construct "le16_to_cpu((__force __le16)(r >> 16))" has
always bothered me when looking through the iwlwifi code,
it shouldn't be necessary to __force anything, and before
this code, "r" was obtained with an ioread32, which swaps
each of the two u16 values in it properly when swapping the
entire u32 value. I've had arguments about this code with
people before, but always conceded they were right because
removing it only made things not work at all on big endian
platforms.

However, analysing a failure of the OTP reading code, I now
finally figured out what is going on, and why my intuition
about that code being wrong was right all along.

It turns out that the 'priv->eeprom' u8 array really wants
to have the data in it in little endian. So the force code
above and all really converts *to* little endian, not from
it. Cf., for instance, the function iwl_eeprom_query16() --
it reads two u8 values and combines them into a u16, in a
little-endian way. And considering it more, it makes sense
to have the eeprom array as on the device, after all not
all values really are 16-bit values, the MAC address for
instance is not.

Now, what this really means is that all the annotations are
completely wrong. The eeprom reading code should fill the
priv->eeprom array as a __le16 array, with __le16 values.

This also means that iwl_read_otp_word() should really have
a __le16 pointer as the data argument, since it should be
filling that in a format suitable for priv->eeprom.

Propagating these changes throughout, iwl_find_otp_image()
is found to be, now obviously visible, defective -- it uses
the data returned by iwl_read_otp_word() directly as if it
was CPU endianness. Fixing that, which is this hunk of the
patch:

-               next_link_addr = link_value * sizeof(u16);
+               next_link_addr = le16_to_cpu(link_value) * sizeof(u16);

is the only real change of this patch. Everything else is
just fixing the sparse annotations.

Also, the bug only shows up on big endian platforms with a
1000 series card. 5000 and previous series do not use OTP,
and 6000 series has shadow RAM support which means we don't
ever use the defective code on any cards but 1000.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
drivers/net/wireless/iwlwifi/iwl-dev.h
drivers/net/wireless/iwlwifi/iwl-eeprom.c