Re: [PATCH] ath9k: introduce endian_check module parameter

[Kalle Valo <kvalo@codeaurora.org>]

Bas Vermeulen <bvermeul@blackstar.nl> writes:

> On 26-02-18 10:54, Kalle Valo wrote:
>> Bas Vermeulen <bvermeul@blackstar.nl> writes:
>>
>>> A random (little endian eeprom'd) ar9278 card didn't work on my
>>> PowerMac G5 without allowing the driver to byte-swap the eeprom.
>>>
>>> Introduce a module parameter endian_check to allow this to happen,
>>> and the PCIe card to function correctly on BE powerpc.
>>>
>>> Signed-off-by: Bas Vermeulen <bvermeul@blackstar.nl>
>>> ---
>>>   drivers/net/wireless/ath/ath9k/init.c | 6 +++++-
>>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
>>> index fa58a32227f5..421039dc060a 100644
>>> --- a/drivers/net/wireless/ath/ath9k/init.c
>>> +++ b/drivers/net/wireless/ath/ath9k/init.c
>>> @@ -67,6 +67,9 @@ static int ath9k_ps_enable;
>>>   module_param_named(ps_enable, ath9k_ps_enable, int, 0444);
>>>   MODULE_PARM_DESC(ps_enable, "Enable WLAN PowerSave");
>>>   +static int ath9k_endian_check;
>>> +module_param_named(endian_check, ath9k_endian_check, int, 0444);
>>> +MODULE_PARM_DESC(endian_check, "Check EEPROM for endianness compatibility");
>>>   #ifdef CONFIG_ATH9K_CHANNEL_CONTEXT
>>>     int ath9k_use_chanctx;
>>> @@ -587,7 +590,8 @@ static int ath9k_of_init(struct ath_softc *sc)
>>>   		ether_addr_copy(common->macaddr, mac);
>>>     	ah->ah_flags &= ~AH_USE_EEPROM;
>>> -	ah->ah_flags |= AH_NO_EEP_SWAP;
>>> +	if (!ath9k_endian_check)
>>> +		ah->ah_flags |= AH_NO_EEP_SWAP;
>> A bit annoying to have a module parameter, isn't there any automatic way
>> to detect/try this? But on the other hand I guess this isn't a common
>> problem as nobody has reported this before?
>
> There is an automatic way to detect this, but that is disabled by the
> AH_NO_EEP_SWAP flag.

Ah, I didn't check the code at all.

> The platform initialisation does not set this flag if the endian_check
> member of pdata is set to true, but there is no way to not set this
> when using a device tree. I used a module parameter instead of a
> device tree variable because I don't know of a way to modify the
> device tree my PowerMac boots with.

Ok, makes sense. A module parameter is not an ideal solution I guess
it's ok in this case.

-- 
Kalle Valo