Thread (21 messages) 21 messages, 9 authors, 2018-04-12

Re: [PATCH] ath9k: introduce endian_check module parameter

From: Kalle Valo <hidden>
Date: 2018-03-14 14:34:55

Bas Vermeulen [off-list ref] writes:
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).

Full patch here:

https://patchwork.kernel.org/patch/10241731/

-- 
Kalle Valo
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help