+ Martin

On 3/14/2018 3:34 PM, Kalle Valo wrote:

Bas Vermeulen [off-list ref] writes:

quoted

quoted

quoted

quoted

quoted

--- 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.

quoted

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: Are there any changes you want me to make in order to get this
accepted? I didn't see anything for me to resolve, but I may have
missed something.

I can submit a patch to not set the AH_NO_EEP_SWAP flag by default if
you prefer, as that would fix my problem as well. I am just not sure
that doesn't break things for some platform/device I don't have.

I'm not really sure what to do. Basically this is a choise between bad
for user experience (the module parameter) or risk of regressions
(disable AH_NO_EEP_SWAP by default). As ath9k is used in very exotic
hardware I'm starting to lean towards the module parameter approach
(your patch) but would like to know what others think, especially Felix
(CCed).

Hi Kalle,

Sorry for barging in, but I figured git history might tell us something. 
The flag was originally added by Felix (commit a59dadbeeaf7 ("ath9k: add 
support for endian swap of eeprom from platform data")) and the function 
ath9k_of_init() was added by Martin (CCed):

commit 138b41253d9c9f9a06c8b086880cd3e839a23d69
Author: Martin Blumenstingl [off-list ref]
Date:   Sun Oct 16 22:59:07 2016 +0200

     ath9k: parse the device configuration from an OF node

Maybe he recalls what device(s) needed it.

Regards,
Arend