• Johannes Berg's avatar
    iwlwifi: fix EEPROM/OTP reading endian annotations and a bug · af6b8ee3
    Johannes Berg authored
    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: default avatarJohannes Berg <johannes@sipsolutions.net>
    Cc: stable@kernel.org
    Signed-off-by: default avatarReinette Chatre <reinette.chatre@intel.com>
    Signed-off-by: default avatarJohn W. Linville <linville@tuxdriver.com>
    af6b8ee3
iwl-dev.h 36.5 KB