[OpenWrt-Devel] question about ath9k endianness check in eeprom_9287.c

Hello Felix,

I am working on the BT Home Hub 3A, which has ath9k wireless.
More or less everything is working now, apart from
reading the calibration data. To get that working too, 
endianness checking must be enabled. That requires a one line
fix to eeprom_9257.c . 


A similar fix was in fact present until quite recently
(till r42950 to be precise) in
package/kernel/mac80211/patches/501-ath9k-eeprom_endianess.patch
but in the wrong place (there are two calls to ath9k_hw_use_flash()
in eeprom_9287.c, and the patch changed the wrong one). With
changeset 42951 that fix was then dropped (together with the
equivalent fix to eeprom_4k.c).

A much much earlier version of 501-ath9k-eeprom_endianess.patch
(r31084) contains the fix in the correct place. I guess 
that somewhere in between r31084 and r42950 (I haven't tried to
work out exactly where) someone slipped up when updating this 
patch. And presumably there can't be many boards making use of
this code otherwise someone else would have noticed already.

If I had I been writing this email four months ago I would
simply  have submitted a patch correcting
501-ath9k-eeprom_endianess.patch.
However that patch and its successor have now disappeared. Am I
right in thinking that their contents have gone upstream? If 
so I should be grateful if you would let me know how to proceed.
Should I submit a new patch to be inserted into
package/kernel/mac80211/patches ? Or should I try to get the fix
into compat-wireless directly?

Many thanks, Ben Mulvihill

On Sun, 2014-12-28 at 01:02 +0100, Ben Mulvihill wrote:
> Hello Felix,
> 
> I am working on the BT Home Hub 3A, which has ath9k wireless.
> More or less everything is working now, apart from
> reading the calibration data. To get that working too, 
> endianness checking must be enabled. That requires a one line
> fix to eeprom_9257.c . 
> 
> --- a/drivers/net/wireless/ath/ath9k/eeprom_9287.c	2014-09-27 15:48:08.000000000 +0200
> +++ b/drivers/net/wireless/ath/ath9k/eeprom_9287.c	2014-12-28 00:55:52.000000000 +0100
> @@ -184,7 +184,7 @@ static int ath9k_hw_ar9287_check_eeprom(
>  	struct ar9287_eeprom *eep = &ah->eeprom.map9287;
>  	struct ath_common *common = ath9k_hw_common(ah);
>  
> -	if (!ath9k_hw_use_flash(ah)) {
> +	if (!(ah->ah_flags & AH_NO_EEP_SWAP)) {
>  		if (!ath9k_hw_nvram_read(ah, AR5416_EEPROM_MAGIC_OFFSET,
>  					 &magic)) {
>  			ath_err(common, "Reading Magic # failed\n");
> 
> A similar fix was in fact present until quite recently
> (till r42950 to be precise) in
> package/kernel/mac80211/patches/501-ath9k-eeprom_endianess.patch
> but in the wrong place (there are two calls to ath9k_hw_use_flash()
> in eeprom_9287.c, and the patch changed the wrong one). With
> changeset 42951 that fix was then dropped (together with the
> equivalent fix to eeprom_4k.c).
> 
> A much much earlier version of 501-ath9k-eeprom_endianess.patch
> (r31084) contains the fix in the correct place. I guess 
> that somewhere in between r31084 and r42950 (I haven't tried to
> work out exactly where) someone slipped up when updating this 
> patch. And presumably there can't be many boards making use of
> this code otherwise someone else would have noticed already.
> 
> If I had I been writing this email four months ago I would
> simply  have submitted a patch correcting
> 501-ath9k-eeprom_endianess.patch.
> However that patch and its successor have now disappeared. Am I
> right in thinking that their contents have gone upstream? If 
> so I should be grateful if you would let me know how to proceed.
> Should I submit a new patch to be inserted into
> package/kernel/mac80211/patches ? Or should I try to get the fix
> into compat-wireless directly?
> 
> Many thanks, Ben Mulvihill
> 

Any thoughts about this? Actually I think there are three options:

 - a new patch in package/kernel/mac80211/patches reinstating 
   a couple of lines from the old 501-ath9k-eeprom_endianess.patch

 - the same fix straight into the upstream code

 - no fix at all, and instead work round the bug during installation
   by reading the ath9k calibration partition, byte-swapping certain
   fields offline and re-flashing it.
   
Thanks in advance.

On 2015-01-16 10:12, Ben Mulvihill wrote:
> Any thoughts about this? Actually I think there are three options:
Hey Ben,

sorry for not getting back to you earlier, I was busy with a lot of stuff.

>  - a new patch in package/kernel/mac80211/patches reinstating 
>    a couple of lines from the old 501-ath9k-eeprom_endianess.patch
> 
>  - the same fix straight into the upstream code
> 
>  - no fix at all, and instead work round the bug during installation
>    by reading the ath9k calibration partition, byte-swapping certain
>    fields offline and re-flashing it.
I intend to make a patch to unify those checks across
eeprom_{def,4k,9287}.c to get rid of duplication. I will submit that
upstream and commit it to OpenWrt

- Felix