Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx> writes:
> On Sat, Dec 22, 2018 at 7:09 AM Daniel F. Dickinson
> <cshored@xxxxxxxxxxxxx> wrote:
>>
>> ath9k_of_init() function[0] was initially written on the assumption that
>> if someone had an explicit ath9k OF node that "there must be something
>> wrong, why would someone add an OF node if everything is fine"[1]
>> (Quoting Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx>)
>>
>> "it turns out it's not that simple. with your requirements I'm now aware
>> of two use-cases where the current code in ath9k_of_init() doesn't work
>> without modifications"[1]
>>
>> The "your requirements" Martin speaks of is the result of the fact that I
>> have a device (PowerCloud Systems CR5000) has some kind of default - not
>> unique mac address - set and requires to set the correct MAC address via
>> mac-address devicetree property, however:
>>
>> "some cards come with a physical EEPROM chip [or OTP] so "qca,no-eeprom"
>> should not be set (your use-case). in this case AH_USE_EEPROM should be
>> set (which is the default when there is no OF node)"[1]
>>
>> The other use case is:
>>
>> the firmware on some PowerMac G5 seems to add a OF node for the ath9k
>> card automatically. depending on the EEPROM on the card AH_NO_EEP_SWAP
>> should be unset (which is the default when there is no OF node). see [3]
>>
>> After this patch to ath9k_of_init() the new behavior will be:
>>
>> if there's no OF node then everything is the same as before
>> if there's an empty OF node then ath9k will use the hardware EEPROM
>> (before ath9k would fail to initialize because no EEPROM data was
>> provided by userspace)
>> if there's an OF node with only a MAC address then ath9k will use
>> the MAC address and the hardware EEPROM (see the case above)
>> with "qca,no-eeprom" EEPROM data from userspace will be requested.
>> the behavior here will not change
>> [1]
>>
>> Martin provides additional background on EEPROM swapping[1].
>>
>> Thanks to Christian Lamparter <chunkeey@xxxxxxxxx> for all his help on
>> troubleshooting this issue and the basis for this patch.
>>
>> Fixes: 138b41253d9c ("ath9k: parse the device configuration from an OF node")
>>
>> [0]https://elixir.bootlin.com/linux/v4.20-rc7/source/drivers/net/wireless/ath/ath9k/init.c#L615
>> [1]https://github.com/openwrt/openwrt/pull/1645#issuecomment-448027058
>> [2]https://github.com/openwrt/openwrt/pull/1613
>> [3]https://patchwork.kernel.org/patch/10241731/
>
> +Cc Bas Vermeulen: can you please try Daniel's patch on your PowerMac
> G5? I hope that this replaces your "endian_check" module parameter!
>
>> Signed-off-by: Daniel F. Dickinson <cshored@xxxxxxxxxxxxx>
>
> Reviewed-by: Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx>
> (to me the description makes sense, but that said: I contributed some
> of the description)
>
> as well as:
> Tested-by: Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx>
> (on my BT HomeHub 5A - which sets qca,no-eeprom - using OpenWrt, which
> is currently running backports v4.19.7)
Daniel noticed that patchwork dropped Martin's reply, adding it to
patchwork now.
--
Kalle Valo
