ath9k: Simplify and fix eeprom endianness swapping

RFC Superseded Martin Blumenstingl 2 participants 5 messages Oct 14, 2015 – Dec 8, 2015 Qualcomm Atheros ATH9K wireless driver

lore

Share & tools

lore

Martin Blumenstingl

author

2015-10-14 22:33 UTC #0 lore raw

ath9k has some code which is supposed to adjust the endianness of the eeprom's contents in case it does not match the CPU endianness. However, it seems that this code was only working properly for chips that used eeprom_def.

I personally have a device with an AR9227 chipset which uses an eeprom with "incorrect endianness".
Feeding ath9k with this eeprom results in "no band has been marked as supported in EEPROM". An OpenWrt user has tried to work around this and documented the required steps in a ticket: [0].

The original goal of my patch was to fix the eeprom endianness handling. However, it seemed easier to take the existing (working) code from eeprom_def and make it re-usable by the other eeprom implementations (namely _9287 and _4k).

I have tested my patch on a single AR9227 device and there it's working fine. It would be great if more people would give this a go before merging this patch.

Regards,
Martin

[0] https://dev.openwrt.org/ticket/13924

Martin Blumenstingl

author

2015-10-14 22:33 UTC #1 lore raw

PATCH 1/1 ath9k: Simplify and fix eeprom endianness swapping

The three eeprom implementations had quite some duplicate code when it came to endianness swapping.
Additionally there was a bug in eeprom_4k and eeprom_9287 which prevented the endianness swapping from working correctly, because the swapping code was guarded within an "if (!ath9k_hw_use_flash(ah))". In eeprom_def this check did not exist, so it seems that eeprom_def was the only implementation where endianness swapping worked.

This patch takes the duplicate code and moves it from eeprom_* to eeprom.c. The new code is derived from eeprom_def, while taking into account the specifics from the other implementations.

Signed-off-by Martin Blumenstingl <[email protected]>

---
 drivers/net/wireless/ath/ath9k/eeprom.c      | 75 +++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath9k/eeprom.h      |  3 ++
 drivers/net/wireless/ath/ath9k/eeprom_4k.c   | 76 ++++++----------------------
 drivers/net/wireless/ath/ath9k/eeprom_9287.c | 68 ++++++-------------------
 drivers/net/wireless/ath/ath9k/eeprom_def.c  | 61 ++++++----------------
 5 files changed, 124 insertions(+), 159 deletions(-)

drivers/net/wireless/ath/ath9k/eeprom.c +75 -0

index cc81482..9257d77 100644
@@ -138,6 +138,81 @@ bool ath9k_hw_nvram_read(struct ath_hw *ah, u32 off, u16 *data)
 	return ret;
 }
 
+int ath9k_hw_nvram_swap_data(struct ath_hw *ah, bool *swap_needed, int size)
+{
+	u16 magic;
+	u16 *eepdata;
+	int i;
+	struct ath_common *common = ath9k_hw_common(ah);
+
+	if (!ath9k_hw_nvram_read(ah, AR5416_EEPROM_MAGIC_OFFSET, &magic)) {
+		ath_err(common, "Reading Magic # failed\n");
+		return -EIO;
+	}
+
+	if (magic == AR5416_EEPROM_MAGIC) {
+		*swap_needed = false;
+	} else if (swab16(magic) == AR5416_EEPROM_MAGIC) {
+		if (ah->ah_flags & AH_NO_EEP_SWAP) {
+			ath_err(common, "EEPROM Endianness mismatch.\n");
+			return -EINVAL;
+		}
+
+		*swap_needed = true;
+	} else {
+		ath_err(common,
+			"Invalid EEPROM Magic (0x%04x).\n", magic);
+		return -EINVAL;
+	}
+
+	eepdata = (u16 *)(&ah->eeprom);
+
+	if (*swap_needed) {
+		ath_dbg(common, EEPROM,
+			"EEPROM Endianness is not native.. Changing.\n");
+
+		for (i = 0; i < size; i++)
+			eepdata[i] = swab16(eepdata[i]);
+	} else {
+		ath_dbg(common, EEPROM,
+			"EEPROM Endianness is native.. no swap required.\n");
+	}
+
+	return 0;
+}
+
+bool ath9k_hw_nvram_validate_checksum(struct ath_hw *ah, int size)
+{
+	u32 i, sum = 0;
+	u16 *eepdata = (u16 *)(&ah->eeprom);
+	struct ath_common *common = ath9k_hw_common(ah);
+
+	for (i = 0; i < size; i++)
+		sum ^= eepdata[i];
+
+	if (sum != 0xffff) {
+		ath_err(common, "Bad EEPROM checksum 0x%x\n", sum);
+		return false;
+	}
+
+	return true;
+}
+
+bool ath9k_hw_nvram_check_version(struct ath_hw *ah, int version, int minrev)
+{
+	struct ath_common *common = ath9k_hw_common(ah);
+
+	if (ah->eep_ops->get_eeprom_ver(ah) != version ||
+	    ah->eep_ops->get_eeprom_rev(ah) < minrev) {
+		ath_err(common, "Bad EEPROM VER 0x%04x or REV 0x%04x\n",
+			ah->eep_ops->get_eeprom_ver(ah),
+			ah->eep_ops->get_eeprom_rev(ah));
+		return -EINVAL;
+	}
+
+	return true;
+}
+
 void ath9k_hw_fill_vpd_table(u8 pwrMin, u8 pwrMax, u8 *pPwrList,
 			     u8 *pVpdList, u16 numIntercepts,
 			     u8 *pRetVpdList)

drivers/net/wireless/ath/ath9k/eeprom.h +3 -0

index 40d4f62..4465c65 100644
@@ -664,6 +664,9 @@ int16_t ath9k_hw_interpolate(u16 target, u16 srcLeft, u16 srcRight,
 bool ath9k_hw_get_lower_upper_index(u8 target, u8 *pList, u16 listSize,
 				    u16 *indexL, u16 *indexR);
 bool ath9k_hw_nvram_read(struct ath_hw *ah, u32 off, u16 *data);
+int ath9k_hw_nvram_swap_data(struct ath_hw *ah, bool *swap_needed, int size);
+bool ath9k_hw_nvram_validate_checksum(struct ath_hw *ah, int size);
+bool ath9k_hw_nvram_check_version(struct ath_hw *ah, int version, int minrev);
 void ath9k_hw_usb_gen_fill_eeprom(struct ath_hw *ah, u16 *eep_data,
 				  int eep_start_loc, int size);
 void ath9k_hw_fill_vpd_table(u8 pwrMin, u8 pwrMax, u8 *pPwrList,

drivers/net/wireless/ath/ath9k/eeprom_4k.c +15 ~~-61~~

106 lines not on this page · raw message

drivers/net/wireless/ath/ath9k/eeprom_9287.c +16 ~~-52~~

91 lines not on this page · raw message

drivers/net/wireless/ath/ath9k/eeprom_def.c +15 ~~-46~~

100 lines not on this page · raw message

Signature · 1 line

2.6.1

Martin Blumenstingl

author

2015-10-31 12:57 UTC #2 lore raw

PATCH v2 1/1 ath9k: Simplify and fix eeprom endianness swapping

This patch takes the duplicate code and moves it from eeprom_* to eeprom.c. The new code is derived from eeprom_def, while taking into account the specifics from the other implementations.

Signed-off-by Martin Blumenstingl <[email protected]>

---
V2: Use the AH_NO_EEP_SWAP flag to decide if swapping is required.
If this flag is set then we assume that the contents of the EEPROM are using the correct endianness even though the magic bytes are not.
This restores the old behavior from eeprom_def.c.

 drivers/net/wireless/ath/ath9k/eeprom.c      | 74 +++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath9k/eeprom.h      |  3 ++
 drivers/net/wireless/ath/ath9k/eeprom_4k.c   | 76 ++++++----------------------
 drivers/net/wireless/ath/ath9k/eeprom_9287.c | 68 ++++++-------------------
 drivers/net/wireless/ath/ath9k/eeprom_def.c  | 61 ++++++----------------
 5 files changed, 123 insertions(+), 159 deletions(-)

drivers/net/wireless/ath/ath9k/eeprom.c +74 -0

index cc81482..f8c5065 100644
@@ -138,6 +138,80 @@ bool ath9k_hw_nvram_read(struct ath_hw *ah, u32 off, u16 *data)
 	return ret;
 }
 
+int ath9k_hw_nvram_swap_data(struct ath_hw *ah, bool *swap_needed, int size)
+{
+	u16 magic;
+	u16 *eepdata;
+	int i;
+	struct ath_common *common = ath9k_hw_common(ah);
+
+	if (!ath9k_hw_nvram_read(ah, AR5416_EEPROM_MAGIC_OFFSET, &magic)) {
+		ath_err(common, "Reading Magic # failed\n");
+		return -EIO;
+	}
+
+	if (magic == AR5416_EEPROM_MAGIC) {
+		*swap_needed = false;
+	} else if (swab16(magic) == AR5416_EEPROM_MAGIC) {
+		if (ah->ah_flags & AH_NO_EEP_SWAP) {
+			ath_info(common,
+				 "Ignoring endianness difference in EEPROM magic bytes.\n");
+
+			*swap_needed = false;
+		} else {
+			*swap_needed = true;
+		}
+	} else {
+		ath_err(common,
+			"Invalid EEPROM Magic (0x%04x).\n", magic);
+		return -EINVAL;
+	}
+
+	eepdata = (u16 *)(&ah->eeprom);
+
+	if (*swap_needed) {
+		ath_dbg(common, EEPROM,
+			"EEPROM Endianness is not native.. Changing.\n");
+
+		for (i = 0; i < size; i++)
+			eepdata[i] = swab16(eepdata[i]);
+	}
+
+	return 0;
+}
+
+bool ath9k_hw_nvram_validate_checksum(struct ath_hw *ah, int size)
+{
+	u32 i, sum = 0;
+	u16 *eepdata = (u16 *)(&ah->eeprom);
+	struct ath_common *common = ath9k_hw_common(ah);
+
+	for (i = 0; i < size; i++)
+		sum ^= eepdata[i];
+
+	if (sum != 0xffff) {
+		ath_err(common, "Bad EEPROM checksum 0x%x\n", sum);
+		return false;
+	}
+
+	return true;
+}
+
+bool ath9k_hw_nvram_check_version(struct ath_hw *ah, int version, int minrev)
+{
+	struct ath_common *common = ath9k_hw_common(ah);
+
+	if (ah->eep_ops->get_eeprom_ver(ah) != version ||
+	    ah->eep_ops->get_eeprom_rev(ah) < minrev) {
+		ath_err(common, "Bad EEPROM VER 0x%04x or REV 0x%04x\n",
+			ah->eep_ops->get_eeprom_ver(ah),
+			ah->eep_ops->get_eeprom_rev(ah));
+		return -EINVAL;
+	}
+
+	return true;
+}
+
 void ath9k_hw_fill_vpd_table(u8 pwrMin, u8 pwrMax, u8 *pPwrList,
 			     u8 *pVpdList, u16 numIntercepts,
 			     u8 *pRetVpdList)

drivers/net/wireless/ath/ath9k/eeprom.h +3 -0

index 40d4f62..4465c65 100644
@@ -664,6 +664,9 @@ int16_t ath9k_hw_interpolate(u16 target, u16 srcLeft, u16 srcRight,
 bool ath9k_hw_get_lower_upper_index(u8 target, u8 *pList, u16 listSize,
 				    u16 *indexL, u16 *indexR);
 bool ath9k_hw_nvram_read(struct ath_hw *ah, u32 off, u16 *data);
+int ath9k_hw_nvram_swap_data(struct ath_hw *ah, bool *swap_needed, int size);
+bool ath9k_hw_nvram_validate_checksum(struct ath_hw *ah, int size);
+bool ath9k_hw_nvram_check_version(struct ath_hw *ah, int version, int minrev);
 void ath9k_hw_usb_gen_fill_eeprom(struct ath_hw *ah, u16 *eep_data,
 				  int eep_start_loc, int size);
 void ath9k_hw_fill_vpd_table(u8 pwrMin, u8 pwrMax, u8 *pPwrList,

drivers/net/wireless/ath/ath9k/eeprom_4k.c +15 ~~-61~~

106 lines not on this page · raw message

drivers/net/wireless/ath/ath9k/eeprom_9287.c +16 ~~-52~~

91 lines not on this page · raw message

drivers/net/wireless/ath/ath9k/eeprom_def.c +15 ~~-46~~

100 lines not on this page · raw message

Signature · 1 line

2.6.2

Kalle Valo

Applied

2015-12-08 14:56 UTC #3 lore raw

PATCH v2 1/1 ath9k: Simplify and fix eeprom endianness swapping

Quoted 8 lines · Martin Blumenstingl

The three eeprom implementations had quite some duplicate code when it came to endianness swapping.
Additionally there was a bug in eeprom_4k and eeprom_9287 which prevented the endianness swapping from working correctly, because the swapping code was guarded within an "if (!ath9k_hw_use_flash(ah))". In eeprom_def this check did not exist, so it seems that eeprom_def was the only implementation where endianness swapping worked.

Quoted 5 lines

This patch takes the duplicate code and moves it from eeprom_* to eeprom.c. The new code is derived from eeprom_def, while taking into account the specifics from the other implementations.
Signed-off-by Martin Blumenstingl <[email protected]>

Applied to ath.git, thanks.

Signature · 1 line

Kalle Valo

Martin Blumenstingl

author

2015-10-25 22:51 UTC #4 lore raw

In reply to Martin Blumenstingl

Quoted 3 lines · Martin Blumenstingl

I have tested my patch on a single AR9227 device and there it's working fine. It would be great if more people would give this a go before merging this patch.

I have just tested it on a second AR9227 device which uses the "other swapped" EEPROM format.
Unfortunately my patch seems to break that, so please do not merge it for now.

I will send an updated patch once I have sorted this out.