Re: [PATCH] ar9170: added phy register initialisation from eeprom values

Re: [PATCH] ar9170: added phy register initialisation from eeprom values

[Chunkeey]

> "Joerg Albert" <jal2@gmx.de> wrote:
>
> This patch adds the initialisation of some PHY registers
> from the modal_header[] values in the EEPROM (see otus/hal/hpmain.c, line 333 ff.)
> 
> Signed-off-by: Joerg Albert <jal2@gmx.de>

meh, no hardware... and only a shitty kiosk with web-interface here.
testing has to wait until the weekend.
 
> diff --git a/drivers/net/wireless/ath/ar9170/phy.c b/drivers/net/wireless/ath/ar9170/phy.c
> index cb8b5cd..47a5e5c 100644
> --- a/drivers/net/wireless/ath/ar9170/phy.c
> +++ b/drivers/net/wireless/ath/ar9170/phy.c
> @@ -396,6 +396,131 @@ static struct ar9170_phy_init ar5416_phy_init[] = {
>  	{ 0x1c9384, 0xf3307ff0, 0xf3307ff0, 0xf3307ff0, 0xf3307ff0, }
>  };
>  
> +/* look up a certain register in ar5416_phy_init[] and return the init. value
> +   for the band and bandwidth given. Return 0 if register address not found. */
See Documentation/CodingStyle - Chapter 8

The preferred style for long (multi-line) comments is:
/*
 * look up a certain register in ar5416_phy_init[] and return the init. value
 * for the band and bandwidth given. Return 0 if register address not found.
 */

> +u32 ar9170_get_default_phy_reg_val(int reg, bool is_2ghz, bool is_40mhz)
please consider this instead: 
static u32 ar9170_get_default_phy_reg_val(u32 reg, [...])
(static function, and reg is a u32)

> +{
> +	struct ar9170_phy_init *p;
> +	struct ar9170_phy_init *endp =
> +		ar5416_phy_init+ARRAY_SIZE(ar5416_phy_init);
(see Codingstyle: 3.1:  Spaces
  => Use one space around (on each side of) most binary [...] operators)

> +
> +	for (p = ar5416_phy_init; p < endp; p++)
a extra { } wouldn't hurt. The following statements also has multiple lines.

> +		if (p->reg == reg) {
> +			if (is_2ghz)
> +				return is_40mhz ? p->_2ghz_40 : p->_2ghz_20;
> +			else
> +				return is_40mhz ? p->_5ghz_40 : p->_5ghz_20;
> +		}
> +	return 0;
> +}
hmm, it's a bit odd to use pointers over a fixed array,
what about (unfortunately space and line damaged... and untested):

{
        unsigned int i;
        for (i = 0; i < ARRAY_SIZE(ar5416_phy_init); i++) {
                if (ar5416_phy_init[i].reg != reg)
                        continue;

                if (is_2ghz) {
                        if (is_40mhz)
                                return ar5416_phy_init[i]._2ghz_40;
                        else
                                return ar5416_phy_init[i]._2ghz_20;
                } else {
                        if (is_40mhz)
                                return ar5416_phy_init[i]._5ghz_40;
                        else
                                return ar5416_phy_init[i]._5ghz_20;
                }
        }
        return 0;
}
this follows the looks of the rest of the code. (e.g ar9170_init_phy)
(of course, either version should be fine.
 so stay with yours if you have doubts.)

> +/* initialize some phy regs from eeprom values in modal_header[]
> +   acc. to band and bandwith */
(multi-line comment, but I guess you know what do here now...)

> +static int ar9170_init_phy_from_eeprom(struct ar9170 *ar,
> +				bool is_2ghz, bool is_40mhz)
> +{
> +	const u8 xpd2pd[16] = {
> +		0x2, 0x2, 0x2, 0x1, 0x2, 0x2, 0x6, 0x2,
> +		0x2, 0x3, 0x7, 0x2, 0xB, 0x2, 0x2, 0x2
> +	};
static const u8 xpd2pd ?

> +	u32 defval, newval; /* two separate for debugging the changes */
> +	/* pointer to the modal_header acc. to band */
> +	struct ar9170_eeprom_modal *m = ar->eeprom.modal_header +
> +		(is_2ghz ? 1 : 0);
what about:
struct ar9170_eeprom_modal *m = &ar->eeprom.modal_header[is_2ghz]; ?

> +	ar9170_regwrite_begin(ar);
> +
> +	/* ant common control (index 0) */
> +	defval = ar9170_get_default_phy_reg_val(0x1c5964, is_2ghz, is_40mhz);
> +	newval = le32_to_cpu(m->antCtrlCommon);
> +	ar9170_regwrite(0x1c5964, newval);
> +
> +	/* ant control chain 0 (index 1) */
> +	defval = ar9170_get_default_phy_reg_val(0x1c5960, is_2ghz, is_40mhz);
> +	newval = le32_to_cpu(m->antCtrlChain[0]);
> +	ar9170_regwrite(0x1c5960, newval);
> +
> +	/* ant control chain 2 (index 2) */
> +	defval = ar9170_get_default_phy_reg_val(0x1c7960, is_2ghz, is_40mhz);
> +	newval = le32_to_cpu(m->antCtrlChain[1]);
> +	ar9170_regwrite(0x1c7960, newval);
> +
> +	/* SwSettle (index 3) */
> +	if (!is_40mhz) {
> +		defval = ar9170_get_default_phy_reg_val(0x1c5844,
> +							is_2ghz, is_40mhz);
> +		newval = (defval & ~0x3f80) | ((m->switchSettling & 0x7f)<<7);
(well )<<7 is a bit tight, but it looks like you ran out of space in this line?)
  
> +		ar9170_regwrite(0x1c5844, newval);
> +	}
> +
> +	/* adcDesired, pdaDesired (index 4) */
> +	defval = ar9170_get_default_phy_reg_val(0x1c5850, is_2ghz, is_40mhz);
> +	newval = (defval & ~0xffff) | ((u8)m->pgaDesiredSize << 8) |
> +		((u8)m->adcDesiredSize);
> +	ar9170_regwrite(0x1c5850, newval);
> +
> +	/* TxEndToXpaOff, TxFrameToXpaOn (index 5) */
> +	defval = ar9170_get_default_phy_reg_val(0x1c5834, is_2ghz, is_40mhz);
> +	newval = (m->txEndToXpaOff << 24) | (m->txEndToXpaOff << 16) |
> +		(m->txFrameToXpaOn << 8) | m->txFrameToXpaOn;
> +	ar9170_regwrite(0x1c5834, newval);
> +
> +	/* TxEndToRxOn (index 6) */
> +	defval = ar9170_get_default_phy_reg_val(0x1c5828, is_2ghz, is_40mhz);
> +	newval = (defval & ~0xff0000) | (m->txEndToRxOn << 16);
> +	ar9170_regwrite(0x1c5828, newval);
> +
> +	/* thresh62 (index 7) */
> +	defval = ar9170_get_default_phy_reg_val(0x1c8864, is_2ghz, is_40mhz);
> +	newval = (defval & ~0x7f000) | (m->thresh62 << 12);
> +	ar9170_regwrite(0x1c8864, newval);
> +
> +	/* tx/rx attenuation chain 0 (index 8) */
> +	defval = ar9170_get_default_phy_reg_val(0x1c5848, is_2ghz, is_40mhz);
> +	newval = (defval & ~0x3f000) | ((m->txRxAttenCh[0] & 0x3f) << 12);
> +	ar9170_regwrite(0x1c5848, newval);
> +
> +	/* tx/rx attenuation chain 2 (index 9) */
> +	defval = ar9170_get_default_phy_reg_val(0x1c7848, is_2ghz, is_40mhz);
> +	newval = (defval & ~0x3f000) | ((m->txRxAttenCh[1] & 0x3f) << 12);
> +	ar9170_regwrite(0x1c7848, newval);
> +
> +	/* tx/rx margin chain 0 (index 10) */
> +	defval = ar9170_get_default_phy_reg_val(0x1c620c, is_2ghz, is_40mhz);
> +	newval = (defval & ~0xfc0000) | ((m->rxTxMarginCh[0] & 0x3f) << 18);
> +	/* bsw margin chain 0 for 5GHz only */
> +	if (!is_2ghz)
> +		newval = (newval & ~0x3c00) | ((m->bswMargin[0] & 0xf) << 10);
> +	ar9170_regwrite(0x1c620c, newval);
> +
> +	/* tx/rx margin chain 2 (index 11) */
> +	defval = ar9170_get_default_phy_reg_val(0x1c820c, is_2ghz, is_40mhz);
> +	newval = (defval & ~0xfc0000) | ((m->rxTxMarginCh[1] & 0x3f) << 18);
> +	ar9170_regwrite(0x1c820c, newval);
> +
> +	/* iqCall, iqCallq chain 0 (index 12) */
> +	defval = ar9170_get_default_phy_reg_val(0x1c5920, is_2ghz, is_40mhz);
> +	newval = (defval & ~0x7ff) | (((u8)m->iqCalICh[0] & 0x3f) << 5) |
> +		((u8)m->iqCalQCh[0] & 0x1f);
> +	ar9170_regwrite(0x1c5920, newval);
> +
> +	/* iqCall, iqCallq chain 2 (index 13) */
> +	defval = ar9170_get_default_phy_reg_val(0x1c7920, is_2ghz, is_40mhz);
> +	newval = (defval & ~0x7ff) | (((u8)m->iqCalICh[1] & 0x3f) << 5) |
> +		((u8)m->iqCalQCh[1] & 0x1f);
> +	ar9170_regwrite(0x1c7920, newval);
> +
> +	/* xpd gain mask (index 14) */
> +	defval = ar9170_get_default_phy_reg_val(0x1c6258, is_2ghz, is_40mhz);
> +	newval = (defval & ~0xf0000) | (xpd2pd[m->xpdGain & 0xf] << 16);
> +	ar9170_regwrite(0x1c6258, newval);
(cannot test this now. it looks good, though.)
It's amazing how much **** you can _cut_ from the vendor driver.

BTW: does this patch help the 1-stage fw stability, or is it still broken?

Regards,
   Chr

________________________________________________________________
Neu: WEB.DE Doppel-FLAT mit Internet-Flatrate + Telefon-Flatrate
für nur 19,99 Euro/mtl.!* http://produkte.web.de/go/02/

Re: [PATCH] ar9170: added phy register initialisation from eeprom values

[Joerg Albert]

On 08/31/2009 11:53 AM, Chunkeey@web.de wrote:
> ...
> See Documentation/CodingStyle - Chapter 8
> 
> The preferred style for long (multi-line) comments is:
> /*
>  * look up a certain register in ar5416_phy_init[] and return the init. value
>  * for the band and bandwidth given. Return 0 if register address not found.
>  */
> ...

Thanks for the comments. I agree with all of them and will re-spin a patch.

> It's amazing how much **** you can _cut_ from the vendor driver.

Yes, at first sight it looks really complex but it isn't.

> BTW: does this patch help the 1-stage fw stability, or is it still broken?

Turned out I had a corrupt firmware file. After downloading from Luis' URL it works fine
with my WNDA3100. Guess I had an earlier version from before 2009/05/28 or firefox corrupted it during download.

What's your setup to get 80+ MBit/s throughput with the two-stage firmware?
I've used an 802.11g AP so far, but have access to an 802.11n dual-band AP now.
Guess I should use 5GHz as 2.4 is rather crowded here.
How can I put ar9170 into 802.11n/40MHz mode?

Regards,
Jörg.

Re: [PATCH] ar9170: added phy register initialisation from eeprom values

[Christian Lamparter]

(changed cc', web.de finally pissed me off with a 500 mail quota)

2009/8/31 Joerg Albert <jal2@gmx.de>:
> On 08/31/2009 11:53 AM, Chunkeey@web.de wrote:
>> ...
>> See Documentation/CodingStyle - Chapter 8
>>
>> The preferred style for long (multi-line) comments is:
>> /*
>>  * look up a certain register in ar5416_phy_init[] and return the init. value
>>  * for the band and bandwidth given. Return 0 if register address not found.
>>  */
>> ...
>
> Thanks for the comments. I agree with all of them and will re-spin a patch.
Great!

>> It's amazing how much **** you can _cut_ from the vendor driver.
>
> Yes, at first sight it looks really complex but it isn't.
well, there's still a thing... we can only copy what's there in driver,
But since the two-stage fw is capable of reinitializing
the rf/phy we could do all sorts of stuff without breaking it.


>> BTW: does this patch help the 1-stage fw stability, or is it still broken?
>
> Turned out I had a corrupt firmware file. After downloading from Luis' URL it works fine
> with my WNDA3100. Guess I had an earlier version from before 2009/05/28 or firefox
> corrupted it during download.
>
> What's your setup to get 80+ MBit/s throughput with the two-stage firmware?
simply another 11n card a few meters away (but in the same room of course,
since the 5GHz band doesn't like to go through concrete)

I tested a few configurations and only with a rt2860pci was able to get a
satisfying result over a small distance (7 m, but still in the same room ;) )
Other cards: like the ar5416 also worked, but not that well (tx rates halved)
and iwl4965, iwl5300 didn't work at all in n. (but they're flying with 11a)

> I've used an 802.11g AP so far, but have access to an 802.11n dual-band AP now.
> Guess I should use 5GHz as 2.4 is rather crowded here.
> How can I put ar9170 into 802.11n/40MHz mode?

well you need a patch which lets you use the MCS rates and
kicks off BlockAck sessions. I posted a version some time
ago, (AFAIK initial RFC?). However due to fact that the code belongs
into the rc-algorithm and the lack of "out-house testing" feedback,
I had to drop it. I can send you an _updated_ (well, it should apply without
fuzz... but you still have to select the MCS by hand) version if you want,
however not until Friday. Of course, If you have free time on your hand,
you could do the fix-ups by yourself and start the madNess right on! ;-)

(Note: Felix is looking into minstrel/HT. I expect once the code is
available, that we only have to patch ar9170 just a _little_ bit to get
it _sort of_ working.
(it might never probably work with the current available firmwares,
since the BA tx_status is totally bogus)

Regards,
    Chr

[PATCH v2] ar9170: added phy register initialisation from eeprom values

[Joerg Albert]

This patch adds the initialisation of some PHY registers
from the modal_header[] values in the EEPROM
(see otus/hal/hpmain.c, line 333 ff.)

Signed-off-by: Joerg Albert <jal2@gmx.de>
---

 Compared to v1 I've included Christian's suggestions and removed
 three unnecessary defval assignments for "ant * control".

 This patch seems to depend on Christian's
 "[RFT] ar9170: use eeprom's frequency calibration values"
 (2009-08-21), I get a poor throughput here without it (1-stage fw,
 802.11g AP).

 drivers/net/wireless/ath/ar9170/phy.c |  136 ++++++++++++++++++++++++++++++++-
 1 files changed, 135 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/ath/ar9170/phy.c b/drivers/net/wireless/ath/ar9170/phy.c
index df86f70..8fb1fa8 100644
--- a/drivers/net/wireless/ath/ar9170/phy.c
+++ b/drivers/net/wireless/ath/ar9170/phy.c
@@ -396,6 +396,138 @@ static struct ar9170_phy_init ar5416_phy_init[] = {
 	{ 0x1c9384, 0xf3307ff0, 0xf3307ff0, 0xf3307ff0, 0xf3307ff0, }
 };
 
+/*
+ * look up a certain register in ar5416_phy_init[] and return the init. value
+ * for the band and bandwidth given. Return 0 if register address not found.
+ */
+static u32 ar9170_get_default_phy_reg_val(u32 reg, bool is_2ghz, bool is_40mhz)
+{
+	unsigned int i;
+	for (i = 0; i < ARRAY_SIZE(ar5416_phy_init); i++) {
+		if (ar5416_phy_init[i].reg != reg)
+			continue;
+
+		if (is_2ghz) {
+			if (is_40mhz)
+				return ar5416_phy_init[i]._2ghz_40;
+			else
+				return ar5416_phy_init[i]._2ghz_20;
+		} else {
+			if (is_40mhz)
+				return ar5416_phy_init[i]._5ghz_40;
+			else
+				return ar5416_phy_init[i]._5ghz_20;
+		}
+	}
+	return 0;
+}
+
+/*
+ * initialize some phy regs from eeprom values in modal_header[]
+ * acc. to band and bandwith
+ */
+static int ar9170_init_phy_from_eeprom(struct ar9170 *ar,
+				bool is_2ghz, bool is_40mhz)
+{
+	static const u8 xpd2pd[16] = {
+		0x2, 0x2, 0x2, 0x1, 0x2, 0x2, 0x6, 0x2,
+		0x2, 0x3, 0x7, 0x2, 0xB, 0x2, 0x2, 0x2
+	};
+	u32 defval, newval;
+	/* pointer to the modal_header acc. to band */
+	struct ar9170_eeprom_modal *m = &ar->eeprom.modal_header[is_2ghz];
+
+	ar9170_regwrite_begin(ar);
+
+	/* ant common control (index 0) */
+	newval = le32_to_cpu(m->antCtrlCommon);
+	ar9170_regwrite(0x1c5964, newval);
+
+	/* ant control chain 0 (index 1) */
+	newval = le32_to_cpu(m->antCtrlChain[0]);
+	ar9170_regwrite(0x1c5960, newval);
+
+	/* ant control chain 2 (index 2) */
+	newval = le32_to_cpu(m->antCtrlChain[1]);
+	ar9170_regwrite(0x1c7960, newval);
+
+	/* SwSettle (index 3) */
+	if (!is_40mhz) {
+		defval = ar9170_get_default_phy_reg_val(0x1c5844,
+							is_2ghz, is_40mhz);
+		newval = (defval & ~0x3f80) | 
+			((m->switchSettling & 0x7f) << 7);
+		ar9170_regwrite(0x1c5844, newval);
+	}
+
+	/* adcDesired, pdaDesired (index 4) */
+	defval = ar9170_get_default_phy_reg_val(0x1c5850, is_2ghz, is_40mhz);
+	newval = (defval & ~0xffff) | ((u8)m->pgaDesiredSize << 8) |
+		((u8)m->adcDesiredSize);
+	ar9170_regwrite(0x1c5850, newval);
+
+	/* TxEndToXpaOff, TxFrameToXpaOn (index 5) */
+	defval = ar9170_get_default_phy_reg_val(0x1c5834, is_2ghz, is_40mhz);
+	newval = (m->txEndToXpaOff << 24) | (m->txEndToXpaOff << 16) |
+		(m->txFrameToXpaOn << 8) | m->txFrameToXpaOn;
+	ar9170_regwrite(0x1c5834, newval);
+
+	/* TxEndToRxOn (index 6) */
+	defval = ar9170_get_default_phy_reg_val(0x1c5828, is_2ghz, is_40mhz);
+	newval = (defval & ~0xff0000) | (m->txEndToRxOn << 16);
+	ar9170_regwrite(0x1c5828, newval);
+
+	/* thresh62 (index 7) */
+	defval = ar9170_get_default_phy_reg_val(0x1c8864, is_2ghz, is_40mhz);
+	newval = (defval & ~0x7f000) | (m->thresh62 << 12);
+	ar9170_regwrite(0x1c8864, newval);
+
+	/* tx/rx attenuation chain 0 (index 8) */
+	defval = ar9170_get_default_phy_reg_val(0x1c5848, is_2ghz, is_40mhz);
+	newval = (defval & ~0x3f000) | ((m->txRxAttenCh[0] & 0x3f) << 12);
+	ar9170_regwrite(0x1c5848, newval);
+
+	/* tx/rx attenuation chain 2 (index 9) */
+	defval = ar9170_get_default_phy_reg_val(0x1c7848, is_2ghz, is_40mhz);
+	newval = (defval & ~0x3f000) | ((m->txRxAttenCh[1] & 0x3f) << 12);
+	ar9170_regwrite(0x1c7848, newval);
+
+	/* tx/rx margin chain 0 (index 10) */
+	defval = ar9170_get_default_phy_reg_val(0x1c620c, is_2ghz, is_40mhz);
+	newval = (defval & ~0xfc0000) | ((m->rxTxMarginCh[0] & 0x3f) << 18);
+	/* bsw margin chain 0 for 5GHz only */
+	if (!is_2ghz)
+		newval = (newval & ~0x3c00) | ((m->bswMargin[0] & 0xf) << 10);
+	ar9170_regwrite(0x1c620c, newval);
+
+	/* tx/rx margin chain 2 (index 11) */
+	defval = ar9170_get_default_phy_reg_val(0x1c820c, is_2ghz, is_40mhz);
+	newval = (defval & ~0xfc0000) | ((m->rxTxMarginCh[1] & 0x3f) << 18);
+	ar9170_regwrite(0x1c820c, newval);
+
+	/* iqCall, iqCallq chain 0 (index 12) */
+	defval = ar9170_get_default_phy_reg_val(0x1c5920, is_2ghz, is_40mhz);
+	newval = (defval & ~0x7ff) | (((u8)m->iqCalICh[0] & 0x3f) << 5) |
+		((u8)m->iqCalQCh[0] & 0x1f);
+	ar9170_regwrite(0x1c5920, newval);
+
+	/* iqCall, iqCallq chain 2 (index 13) */
+	defval = ar9170_get_default_phy_reg_val(0x1c7920, is_2ghz, is_40mhz);
+	newval = (defval & ~0x7ff) | (((u8)m->iqCalICh[1] & 0x3f) << 5) |
+		((u8)m->iqCalQCh[1] & 0x1f);
+	ar9170_regwrite(0x1c7920, newval);
+
+	/* xpd gain mask (index 14) */
+	defval = ar9170_get_default_phy_reg_val(0x1c6258, is_2ghz, is_40mhz);
+	newval = (defval & ~0xf0000) | (xpd2pd[m->xpdGain & 0xf] << 16);
+	ar9170_regwrite(0x1c6258, newval);
+
+
+	ar9170_regwrite_finish();
+
+	return ar9170_regwrite_result();
+}
+
 int ar9170_init_phy(struct ar9170 *ar, enum ieee80211_band band)
 {
 	int i, err;
@@ -426,7 +558,9 @@ int ar9170_init_phy(struct ar9170 *ar, enum ieee80211_band band)
 	if (err)
 		return err;
 
-	/* XXX: use EEPROM data here! */
+	err = ar9170_init_phy_from_eeprom(ar, is_2ghz, is_40mhz);
+	if (err)
+		return err;
 
 	err = ar9170_init_power_cal(ar);
 	if (err)
-- 
1.6.0.4

Re: [PATCH v2] ar9170: added phy register initialisation from eeprom values

[Christian Lamparter]

On Thursday 03 September 2009 01:02:59 Joerg Albert wrote:
> 
> This patch adds the initialisation of some PHY registers
> from the modal_header[] values in the EEPROM
> (see otus/hal/hpmain.c, line 333 ff.)
> 
> Signed-off-by: Joerg Albert <jal2@gmx.de>
Acked-by: Christian Lamparter <chunkeey@googlemail.com>

> ---
> 
>  Compared to v1 I've included Christian's suggestions and removed
>  three unnecessary defval assignments for "ant * control".
> 
>  This patch seems to depend on Christian's
>  "[RFT] ar9170: use eeprom's frequency calibration values"
>  (2009-08-21), I get a poor throughput here without it (1-stage fw,
>  802.11g AP).
Good work!

with both patches applied, my WNDA is finally able to pick up some
speed. But, it's still a mbit slower than the two-stage fw.
  
>  drivers/net/wireless/ath/ar9170/phy.c |  136 ++++++++++++++++++++++++++++++++-
>  1 files changed, 135 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ar9170/phy.c b/drivers/net/wireless/ath/ar9170/phy.c
> index df86f70..8fb1fa8 100644
> --- a/drivers/net/wireless/ath/ar9170/phy.c
> +++ b/drivers/net/wireless/ath/ar9170/phy.c
> @@ -396,6 +396,138 @@ static struct ar9170_phy_init ar5416_phy_init[] = {
>  	{ 0x1c9384, 0xf3307ff0, 0xf3307ff0, 0xf3307ff0, 0xf3307ff0, }
>  };
>  
> +/*
> + * look up a certain register in ar5416_phy_init[] and return the init. value
> + * for the band and bandwidth given. Return 0 if register address not found.
> + */
> +static u32 ar9170_get_default_phy_reg_val(u32 reg, bool is_2ghz, bool is_40mhz)
[...]
> +	/* xpd gain mask (index 14) */
> +	defval = ar9170_get_default_phy_reg_val(0x1c6258, is_2ghz, is_40mhz);
> +	newval = (defval & ~0xf0000) | (xpd2pd[m->xpdGain & 0xf] << 16);
> +	ar9170_regwrite(0x1c6258, newval);
> +
> +
John, can you please nuke those two empty lines?

> +	ar9170_regwrite_finish();
> +
> +	return ar9170_regwrite_result();
> +}
> +
>  int ar9170_init_phy(struct ar9170 *ar, enum ieee80211_band band)
>  {
>  	int i, err;
> @@ -426,7 +558,9 @@ int ar9170_init_phy(struct ar9170 *ar, enum ieee80211_band band)
>  	if (err)
>  		return err;
>  
> -	/* XXX: use EEPROM data here! */
uh, I think we're still missing the heavy clip/conformance?
(otus/hal/hpmain.c, line 3723 ff.).

It should be modified to say something like:
	/* TODO: (heavy clip) regulatory domain power level fine-tuning. */

> +	err = ar9170_init_phy_from_eeprom(ar, is_2ghz, is_40mhz);
> +	if (err)
> +		return err;
>  
>  	err = ar9170_init_power_cal(ar);
>  	if (err)

Regards,
	Chr

Re: [PATCH] ar9170: added phy register initialisation from eeprom values

[Christian Lamparter]

[-- Attachment #1: Type: Text/Plain, Size: 1188 bytes --]

(CC linux-wireless)
On Thursday 03 September 2009 00:44:02 Joerg Albert wrote:
> On 08/31/2009 11:37 PM, Christian Lamparter wrote:
> 
> > well you need a patch which lets you use the MCS rates and
> > kicks off BlockAck sessions. I posted a version some time
> > ago, (AFAIK initial RFC?). However due to fact that the code belongs
> > into the rc-algorithm and the lack of "out-house testing" feedback,
> > I had to drop it. I can send you an _updated_ (well, it should apply without
> > fuzz... but you still have to select the MCS by hand) version if you want,
> > however not until Friday. Of course, If you have free time on your hand,
> > you could do the fix-ups by yourself and start the madNess right on! ;-)
> 
> As I'm not that familiar with 802.11n (and always short of free time)
> I'd like to use your patch.
> Guess the old version is
> "[WIP][RFT][RFC] ar9170: aggregation xmit (aka the _other_ part)" from 2009/06/06?
exactly... it's old & buggy

this version even has a bogus rc.
(Of course, you can select your own MCS rate by changing)

+	rate->idx = sta_info->current_rate;
to a static value between 0 and 15.
(which is _translated_ into: MCS 0 - 15)

Regards,
	Chr

[-- Attachment #2: ar9170-rate-v4.2.diff --]
[-- Type: text/x-patch, Size: 10373 bytes --]

diff --git a/drivers/net/wireless/ath/ar9170/ar9170.h b/drivers/net/wireless/ath/ar9170/ar9170.h
index 914e471..d3ba91c 100644
--- a/drivers/net/wireless/ath/ar9170/ar9170.h
+++ b/drivers/net/wireless/ath/ar9170/ar9170.h
@@ -131,6 +131,7 @@ struct ar9170_rxstream_mpdu_merge {
 enum ar9170_tid_state {
 	AR9170_TID_STATE_INVALID,
 	AR9170_TID_STATE_SHUTDOWN,
+	AR9170_TID_STATE_STARTING,
 	AR9170_TID_STATE_PROGRESS,
 	AR9170_TID_STATE_COMPLETE,
 };
@@ -233,6 +234,10 @@ struct ar9170 {
 	struct list_head tx_ampdu_list;
 	unsigned int tx_ampdu_pending;
 
+	spinlock_t addba_list_lock;
+	struct list_head addba_list;
+	struct work_struct start_tx_ba_work;
+
 	/* rxstream mpdu merge */
 	struct ar9170_rxstream_mpdu_merge rx_mpdu;
 	struct sk_buff *rx_failover;
@@ -246,6 +251,12 @@ struct ar9170 {
 struct ar9170_sta_info {
 	struct ar9170_sta_tid agg[AR9170_NUM_TID];
 	unsigned int ampdu_max_len;
+
+#ifndef AR9170_MAC80211_RC_MCS
+	unsigned int max_mcs;
+	int score;
+	unsigned int current_rate;
+#endif /* AR9170_MAC80211_RC_MCS */
 };
 
 #define AR9170_TX_FLAG_WAIT_FOR_ACK	BIT(0)
diff --git a/drivers/net/wireless/ath/ar9170/main.c b/drivers/net/wireless/ath/ar9170/main.c
index b007357..89aa831 100644
--- a/drivers/net/wireless/ath/ar9170/main.c
+++ b/drivers/net/wireless/ath/ar9170/main.c
@@ -49,7 +49,7 @@ static int modparam_nohwcrypt;
 module_param_named(nohwcrypt, modparam_nohwcrypt, bool, S_IRUGO);
 MODULE_PARM_DESC(nohwcrypt, "Disable hardware encryption.");
 
-static int modparam_ht;
+static int modparam_ht = 1;
 module_param_named(ht, modparam_ht, bool, S_IRUGO);
 MODULE_PARM_DESC(ht, "enable MPDU aggregation.");
 
@@ -285,6 +285,124 @@ static void ar9170_dump_tx_status_ampdu(struct ar9170 *ar)
 
 #endif /* AR9170_TXAGG_DEBUG */
 
+static void ar9170_setup_aggregation(struct ar9170 *ar, struct sk_buff *skb)
+{
+	unsigned long flags;
+	struct ieee80211_sta *sta;
+	struct ieee80211_hdr *hdr = (void *) skb->data;
+	struct ar9170_sta_tid *tid_info;
+	u16 tid = ar9170_get_tid(skb);
+
+	if (!conf_is_ht(&ar->hw->conf))
+		return ;
+
+	/* don't start aggregation for non-qos / WPA handshake */
+	if ((skb->protocol == cpu_to_le16(ETH_P_PAE)) ||
+	    !ieee80211_is_data_qos(hdr->frame_control))
+		return ;
+
+	rcu_read_lock();
+	sta = ieee80211_find_sta(ar->hw, ieee80211_get_DA(hdr));
+	if (!sta)
+		goto out_unlock;
+
+	if (!sta->ht_cap.ht_supported)
+		goto out_unlock;
+
+	tid_info = &((struct ar9170_sta_info *) sta->drv_priv)->agg[tid];
+
+	if (tid_info->retry++ > AR9170_NUM_MAX_BA_RETRY) {
+#ifdef AR9170_TXAGG_DEBUG
+		printk(KERN_DEBUG "%s: too many addba retries for "
+		       "ESS:[%pM], tid:%d.\n", wiphy_name(ar->hw->wiphy),
+		       sta->addr, tid);
+#endif /* AR9170_TXAGG_DEBUG */
+		goto out_unlock;
+	}
+
+	if (tid_info->state == AR9170_TID_STATE_SHUTDOWN) {
+		if (!list_empty(&tid_info->list)) {
+#ifdef AR9170_TXAGG_DEBUG
+			printk(KERN_DEBUG "%s: addba already queued.\n",
+			       wiphy_name(ar->hw->wiphy));
+#endif /* AR9170_TXAGG_DEBUG */
+			goto out_unlock;
+		}
+
+		/* FIXME: no idea if this is right... */
+		tid_info->ssn = GET_NEXT_SEQ_FROM_SKB(skb);
+
+		spin_lock_irqsave(&ar->addba_list_lock, flags);
+		memcpy(tid_info->addr, sta->addr, ETH_ALEN);
+		tid_info->state = AR9170_TID_STATE_STARTING;
+		list_add_tail(&tid_info->list, &ar->addba_list);
+		spin_unlock_irqrestore(&ar->addba_list_lock, flags);
+		ieee80211_queue_work(ar->hw, &ar->start_tx_ba_work);
+	}
+
+out_unlock:
+	rcu_read_unlock();
+}
+
+#ifndef AR9170_MAC80211_RC_MCS
+static void ar9170_feedback_11nrate(struct ar9170 *ar, struct sk_buff *skb)
+{
+	struct ieee80211_tx_info *txinfo = IEEE80211_SKB_CB(skb);
+	struct ieee80211_tx_rate *txrate = txinfo->status.rates;
+	struct ieee80211_sta *sta;
+	struct ar9170_sta_info *sta_info;
+
+	rcu_read_lock();
+	sta = ieee80211_find_sta(ar->hw, ieee80211_get_DA((void *)skb->data));
+	if (sta == NULL)
+		goto out_unlock;
+
+	sta_info = (void *) sta->drv_priv;
+	sta_info->score = (sta_info->score * 99998) / 99999;
+
+	if (txinfo->flags & IEEE80211_TX_STAT_ACK)
+		sta_info->score += 16 - txrate->idx;
+	else
+		sta_info->score -= 16 - txrate->idx;
+
+	if (sta_info->score < 0) {
+		if (sta_info->current_rate)
+			sta_info->current_rate--;
+	} else if (sta_info->score > (1 << txrate->idx) &&
+		   sta_info->current_rate < sta_info->max_mcs)
+			sta_info->current_rate++;
+
+out_unlock:
+	rcu_read_unlock();
+}
+
+static void ar9170_select_11nrate(struct ar9170 *ar,
+				  struct ieee80211_tx_rate *rate,
+				  struct ieee80211_tx_info *info)
+{
+	struct ieee80211_sta *sta = info->control.sta;
+	struct ar9170_sta_info *sta_info = (void *) sta->drv_priv;
+
+	info->flags &= ~IEEE80211_TX_INTFL_RCALGO;
+	rate->flags = IEEE80211_TX_RC_MCS;
+	rate->idx = sta_info->current_rate;
+
+	if ((rate->idx > 4) &&
+	    conf_is_ht40(&ar->hw->conf) &&
+	    (sta->ht_cap.cap & IEEE80211_HT_CAP_SUP_WIDTH_20_40))
+		rate->flags |= IEEE80211_TX_RC_40_MHZ_WIDTH;
+
+	if ((rate->idx == (sta_info->max_mcs + 1)) &&
+	    (rate->flags & IEEE80211_TX_RC_40_MHZ_WIDTH) &&
+	    (sta->ht_cap.cap & IEEE80211_HT_CAP_SGI_40))
+		rate->flags |= IEEE80211_TX_RC_SHORT_GI;
+#ifdef AR9170_RC_MCS_DEBUG
+	if (net_ratelimit())
+		printk(KERN_DEBUG "rate:%x flags:%x\n", rate->idx, rate->flags);
+#endif
+}
+#endif /* AR9170_MAC80211_RC_MCS */
+
 /* caller must guarantee exclusive access for _bin_ queue. */
 static void ar9170_recycle_expired(struct ar9170 *ar,
 				   struct sk_buff_head *queue,
@@ -353,6 +471,9 @@ static void ar9170_tx_status(struct ar9170 *ar, struct sk_buff *skb,
 
 	txinfo->status.rates[0].count = retries + 1;
 	skb_pull(skb, sizeof(struct ar9170_tx_control));
+	if (tx_status != AR9170_TX_STATUS_FAILED)
+		ar9170_setup_aggregation(ar, skb);
+
 	ieee80211_tx_status_irqsafe(ar->hw, skb);
 }
 
@@ -388,6 +509,9 @@ static void ar9170_tx_fake_ampdu_status(struct ar9170 *ar)
 		txinfo->status.rates[0].count = 1;
 
 		skb_pull(skb, sizeof(struct ar9170_tx_control));
+#ifndef AR9170_MAC80211_RC_MCS
+		ar9170_feedback_11nrate(ar, skb);
+#endif /* AR9170_MAC80211_RC_MCS */
 		ieee80211_tx_status_irqsafe(ar->hw, skb);
 	}
 
@@ -542,6 +666,9 @@ static void ar9170_handle_block_ack(struct ar9170 *ar, u16 count, u16 r)
 		txinfo->status.rates[0].count = 1;
 
 		skb_pull(skb, sizeof(struct ar9170_tx_control));
+#ifndef AR9170_MAC80211_RC_MCS
+		ar9170_feedback_11nrate(ar, skb);
+#endif /* AR9170_MAC80211_RC_MCS */
 		ieee80211_tx_status_irqsafe(ar->hw, skb);
 		count--;
 	}
@@ -1295,6 +1422,7 @@ static void ar9170_op_stop(struct ieee80211_hw *hw)
 	cancel_delayed_work_sync(&ar->led_work);
 #endif
 	cancel_work_sync(&ar->beacon_work);
+	cancel_work_sync(&ar->start_tx_ba_work);
 
 	mutex_lock(&ar->mutex);
 
@@ -1414,6 +1542,10 @@ static int ar9170_tx_prepare(struct ar9170 *ar, struct sk_buff *skb)
 			txc->mac_control |= cpu_to_le16(AR9170_TX_MAC_AGGR);
 			arinfo->flags = AR9170_TX_FLAG_BLOCK_ACK;
 
+#ifndef AR9170_MAC80211_RC_MCS
+			ar9170_select_11nrate(ar, txrate, info);
+#endif /* AR9170_MAC80211_RC_MCS */
+
 			goto out;
 		}
 
@@ -1475,6 +1607,13 @@ static void ar9170_tx_prepare_phy(struct ar9170 *ar, struct sk_buff *skb)
 		u32 r = txrate->idx;
 		u8 *txpower;
 
+#ifndef AR9170_MAC80211_RC_MCS
+		if (ar->eeprom.tx_mask == 1)
+			txrate->idx = r = min_t(s8, txrate->idx, 7);
+		else
+			txrate->idx = r = min_t(s8, txrate->idx, 15);
+#endif /* AR9170_MAC80211_RC_MCS */
+
 		/* heavy clip control */
 		txc->phy_control |= cpu_to_le32((r & 0x7) << 7);
 
@@ -1540,6 +1679,7 @@ static void ar9170_tx_prepare_phy(struct ar9170 *ar, struct sk_buff *skb)
 		/* >= 36M legacy OFDM - use only one chain */
 		if (rate && rate->bitrate >= 360)
 			chains = AR9170_TX_PHY_TXCHAIN_1;
+
 	}
 	txc->phy_control |= cpu_to_le32(chains << AR9170_TX_PHY_TXCHAIN_SHIFT);
 }
@@ -2363,6 +2503,18 @@ static void ar9170_sta_notify(struct ieee80211_hw *hw,
 		}
 
 		sta_info->ampdu_max_len = 1 << (3 + sta->ht_cap.ampdu_factor);
+
+#ifndef AR9170_MAC80211_RC_MCS
+		{
+			unsigned int tx_mcs;
+			unsigned int rx_mcs;
+
+			tx_mcs = ar->eeprom.tx_mask == 1 ? 7 : 15;
+			rx_mcs = find_last_bit((unsigned long *)sta->ht_cap.mcs.rx_mask, 16);
+
+			sta_info->max_mcs = min(tx_mcs, rx_mcs);
+		}
+#endif /* AR9170_MAC80211_RC_MCS */
 		break;
 
 	case STA_NOTIFY_REMOVE:
@@ -2445,7 +2597,7 @@ static int ar9170_ampdu_action(struct ieee80211_hw *hw,
 	switch (action) {
 	case IEEE80211_AMPDU_TX_START:
 		spin_lock_irqsave(&ar->tx_ampdu_list_lock, flags);
-		if (tid_info->state != AR9170_TID_STATE_SHUTDOWN ||
+		if (tid_info->state != AR9170_TID_STATE_STARTING ||
 		    !list_empty(&tid_info->list)) {
 			spin_unlock_irqrestore(&ar->tx_ampdu_list_lock, flags);
 #ifdef AR9170_TXAGG_DEBUG
@@ -2514,6 +2666,35 @@ static const struct ieee80211_ops ar9170_ops = {
 	.ampdu_action		= ar9170_ampdu_action,
 };
 
+static void ar9170_start_tx_ba(struct work_struct *work)
+{
+	struct ar9170 *ar = container_of(work, struct ar9170,
+					 start_tx_ba_work);
+	unsigned long flags;
+
+	while (!list_empty(&ar->addba_list)) {
+		struct ar9170_sta_tid *tid_info;
+
+		spin_lock_irqsave(&ar->addba_list_lock, flags);
+		tid_info = list_first_entry(&ar->addba_list,
+					    struct ar9170_sta_tid, list);
+		if (tid_info) {
+			tid_info->retry = 0;
+			list_del_init(&tid_info->list);
+
+			if (tid_info->state != AR9170_TID_STATE_STARTING) {
+				WARN_ON(1);
+				tid_info = NULL;
+			}
+		}
+		spin_unlock_irqrestore(&ar->addba_list_lock, flags);
+
+		if (tid_info)
+			ieee80211_start_tx_ba_session(ar->hw, tid_info->addr,
+						      tid_info->tid);
+	}
+}
+
 void *ar9170_alloc(size_t priv_size)
 {
 	struct ieee80211_hw *hw;
@@ -2542,6 +2723,7 @@ void *ar9170_alloc(size_t priv_size)
 	mutex_init(&ar->mutex);
 	spin_lock_init(&ar->cmdlock);
 	spin_lock_init(&ar->tx_stats_lock);
+	spin_lock_init(&ar->addba_list_lock);
 	spin_lock_init(&ar->tx_ampdu_list_lock);
 	skb_queue_head_init(&ar->tx_status_ampdu);
 	for (i = 0; i < __AR9170_NUM_TXQ; i++) {
@@ -2550,8 +2732,10 @@ void *ar9170_alloc(size_t priv_size)
 	}
 	ar9170_rx_reset_rx_mpdu(ar);
 	INIT_WORK(&ar->beacon_work, ar9170_new_beacon);
+	INIT_WORK(&ar->start_tx_ba_work, ar9170_start_tx_ba);
 	INIT_DELAYED_WORK(&ar->tx_janitor, ar9170_tx_janitor);
 	INIT_LIST_HEAD(&ar->tx_ampdu_list);
+	INIT_LIST_HEAD(&ar->addba_list);
 
 	/* all hw supports 2.4 GHz, so set channel to 1 by default */
 	ar->channel = &ar9170_2ghz_chantable[0];