Re: [PATCH] ath5k: Fix reset sequence for AR5212 in general and RF5111 in particular

Re: [PATCH] ath5k: Fix reset sequence for AR5212 in general and RF5111 in particular

[Nick Kossifidis]

2008/10/23 Elias Oltmanns <eo@nebensachen.de>:
> Take care to handle register 0xa228 exactly as in the HAL released by
> Atheros. This change is required to make ath5k work again on my system
> since commit 2203d6be (ath5k: Misc hw_reset updates), thus fixing a
> regression in 2.6.27 and therefore hopefully eligible for inclusion into
> a stable release.
>
> Cc: stable <stable@kernel.org>
> Signed-off-by: Elias Oltmanns <eo@nebensachen.de>
> ---
> Applies to 2.6.27. Please let me know if I should do something
> differently and resend.
>
>  drivers/net/wireless/ath5k/hw.c       |    1 +
>  drivers/net/wireless/ath5k/initvals.c |    2 ++
>  2 files changed, 3 insertions(+), 0 deletions(-)
>

Current code works fine on my CM6 (AR5212 + RF5111) and i see nothing like

data |= 0x00000800

on the HAL Atheros released

Problem is that we are updating 0xa228 (RSSI threshold in this case)
for all AR5212 based cards and we are not checking right now for BB
revision. These settings according to Atheros HAL are only for BB
revisions > 2. I'm going to update reset.c this week hopefully.

BTW can you please run latest ath_info on your card ? Let's see what's
the rssi threshold stored on EEPROM and how much different is from my
card.

-- 
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick

Re: [PATCH] ath5k: Fix reset sequence for AR5212 in general and RF5111 in particular

[Elias Oltmanns]

[Bcc'ing Nils who has reported similar issues with ath5k on 2.6.27.
Nils, can you perhaps test my patch below and report back? ]

"Nick Kossifidis" <mickflemm@gmail.com> wrote:
> 2008/10/23 Elias Oltmanns <eo@nebensachen.de>:
>> Take care to handle register 0xa228 exactly as in the HAL released by
>
>> Atheros. This change is required to make ath5k work again on my system
>> since commit 2203d6be (ath5k: Misc hw_reset updates), thus fixing a
>> regression in 2.6.27 and therefore hopefully eligible for inclusion into
>> a stable release.
>>
>> Cc: stable <stable@kernel.org>
>> Signed-off-by: Elias Oltmanns <eo@nebensachen.de>
>> ---
>> Applies to 2.6.27. Please let me know if I should do something
>> differently and resend.
>>
>>  drivers/net/wireless/ath5k/hw.c       |    1 +
>>  drivers/net/wireless/ath5k/initvals.c |    2 ++
>>  2 files changed, 3 insertions(+), 0 deletions(-)
>>
>
> Current code works fine on my CM6 (AR5212 + RF5111) and i see nothing like
>
> data |= 0x00000800
>
> on the HAL Atheros released

Let me quote from legacy-hal:

ar5212/ar5212_reset.c:
373:		OS_REG_RMW_FIELD(ah, AR_PHY_DAG_CTRLCCK,
374:			AR_PHY_DAG_CTRLCCK_RSSI_THR, 2);

ah_internal.h:
598:	#define OS_REG_RMW_FIELD(_a, _r, _f, _v) \
599:	    OS_REG_WRITE(_a, _r, \
600:	        (OS_REG_READ(_a, _r) &~ _f) | (((_v) << _f##_S) & _f))

Please note the last argument to OS_REG_RMW_FIELD() which is 2 in this
case. Unless I've made a mistake, this translates into the

data |= 0x00000800;

in my patch.

There definitely is no way for me to connect to an ap (or even get a
list of aps) with current 2.6.27.4. Perhaps we'll hear something from
Nils on that matter too.

>
> Problem is that we are updating 0xa228 (RSSI threshold in this case)
> for all AR5212 based cards and we are not checking right now for BB
> revision. These settings according to Atheros HAL are only for BB
> revisions > 2. I'm going to update reset.c this week hopefully.
>
> BTW can you please run latest ath_info on your card ? Let's see what's
> the rssi threshold stored on EEPROM and how much different is from my
> card.

Well, I don't see how to get that information from the dump. Please have
a look for yourself, I'm going to send you my dump by pm.

Since my original email has not appeared on linux-wireless (due to a
mistake of mine), I'm attaching the patch again, so others can have a
look at it and, more importantly, Nils can test it.

Regards,

Elias

--------
From: Elias Oltmanns <eo@nebensachen.de>
Subject: [PATCH] ath5k: Fix reset sequence for AR5212 in general and RF5111 in particular

Take care to handle register 0xa228 exactly as in the HAL released by
Atheros. This change is required to make ath5k work again on my system
since commit 2203d6be (ath5k: Misc hw_reset updates), thus fixing a
regression in 2.6.27 and therefore hopefully eligible for inclusion into
a stable release.

Cc: stable <stable@kernel.org>
Signed-off-by: Elias Oltmanns <eo@nebensachen.de>
---

 drivers/net/wireless/ath5k/hw.c       |    1 +
 drivers/net/wireless/ath5k/initvals.c |    2 ++
 2 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ath5k/hw.c b/drivers/net/wireless/ath5k/hw.c
index ad1a5b4..f1a7f71 100644
--- a/drivers/net/wireless/ath5k/hw.c
+++ b/drivers/net/wireless/ath5k/hw.c
@@ -858,6 +858,7 @@ int ath5k_hw_reset(struct ath5k_hw *ah, enum ieee80211_if_types op_mode,
 
 			data = ath5k_hw_reg_read(ah, 0xa228);
 			data &= 0xfffe03ff;
+			data |= 0x00000800;
 			ath5k_hw_reg_write(ah, data, 0xa228);
 			data = 0;
 
diff --git a/drivers/net/wireless/ath5k/initvals.c b/drivers/net/wireless/ath5k/initvals.c
index 2806b21..cf7ebd1 100644
--- a/drivers/net/wireless/ath5k/initvals.c
+++ b/drivers/net/wireless/ath5k/initvals.c
@@ -810,6 +810,8 @@ static const struct ath5k_ini_mode ar5212_rf5111_ini_mode_end[] = {
 		{ 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000 } },
 	{ AR5K_PHY(642),
 		{ 0xd03e6788, 0xd03e6788, 0xd03e6788, 0xd03e6788, 0xd03e6788 } },
+	{ 0xa228,
+		{ 0x000001b5, 0x000001b5, 0x000001b5, 0x000001b5, 0x000001b5 } },
 	{ 0xa23c,
 		{ 0x13c889af, 0x13c889af, 0x13c889af, 0x13c889af, 0x13c889af } },
 };

Re: [PATCH] ath5k: Fix reset sequence for AR5212 in general and RF5111 in particular

[Nick Kossifidis]

2008/10/26 Elias Oltmanns <eo@nebensachen.de>:
>
> Let me quote from legacy-hal:
>
> ar5212/ar5212_reset.c:
> 373:            OS_REG_RMW_FIELD(ah, AR_PHY_DAG_CTRLCCK,
> 374:                    AR_PHY_DAG_CTRLCCK_RSSI_THR, 2);
>
> ah_internal.h:
> 598:    #define OS_REG_RMW_FIELD(_a, _r, _f, _v) \
> 599:        OS_REG_WRITE(_a, _r, \
> 600:            (OS_REG_READ(_a, _r) &~ _f) | (((_v) << _f##_S) & _f))
>
> Please note the last argument to OS_REG_RMW_FIELD() which is 2 in this
> case. Unless I've made a mistake, this translates into the
>
> data |= 0x00000800;
>
> in my patch.
>
> There definitely is no way for me to connect to an ap (or even get a
> list of aps) with current 2.6.27.4. Perhaps we'll hear something from
> Nils on that matter too.
>

Some lines above...

	/* Overwrite INI values for revised chipsets */
	if (AH_PRIVATE(ah)->ah_phyRev >= AR_PHY_CHIP_ID_REV_2) {

so

		/* Add barker RSSI thresh enable as disabled */
		OS_REG_CLR_BIT(ah, AR_PHY_DAG_CTRLCCK,
			AR_PHY_DAG_CTRLCCK_EN_RSSI_THR);
		OS_REG_RMW_FIELD(ah, AR_PHY_DAG_CTRLCCK,
			AR_PHY_DAG_CTRLCCK_RSSI_THR, 2);

is for phy (BB) revisions > 2 since you have RF5111 this code should
not run but (and here is our bug) instead of checking bb revision we
run this code for all AR5212 chips

		/*
		 * Write some more initial register settings
		 */
		if (ah->ah_version == AR5K_AR5212) {

> --- a/drivers/net/wireless/ath5k/initvals.c
> +++ b/drivers/net/wireless/ath5k/initvals.c
> @@ -810,6 +810,8 @@ static const struct ath5k_ini_mode ar5212_rf5111_ini_mode_end[] = {
>                { 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000 } },
>        { AR5K_PHY(642),
>                { 0xd03e6788, 0xd03e6788, 0xd03e6788, 0xd03e6788, 0xd03e6788 } },
> +       { 0xa228,
> +               { 0x000001b5, 0x000001b5, 0x000001b5, 0x000001b5, 0x000001b5 } },
>        { 0xa23c,
>                { 0x13c889af, 0x13c889af, 0x13c889af, 0x13c889af, 0x13c889af } },
>  };
>

This chunk is correct

So can you please change the check on ath5k_hw_reset + add initial
rssi threshold setting and retry ?


-- 
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick

Re: [PATCH] ath5k: Fix reset sequence for AR5212 in general and RF5111 in particular

[Elias Oltmanns]

"Nick Kossifidis" <mickflemm@gmail.com> wrote:
> 2008/10/26 Elias Oltmanns <eo@nebensachen.de>:
>>
>
>> Let me quote from legacy-hal:
>>
>> ar5212/ar5212_reset.c:
>> 373:            OS_REG_RMW_FIELD(ah, AR_PHY_DAG_CTRLCCK,
>> 374:                    AR_PHY_DAG_CTRLCCK_RSSI_THR, 2);
>>
>> ah_internal.h:
>> 598:    #define OS_REG_RMW_FIELD(_a, _r, _f, _v) \
>> 599:        OS_REG_WRITE(_a, _r, \
>> 600:            (OS_REG_READ(_a, _r) &~ _f) | (((_v) << _f##_S) & _f))
>>
>> Please note the last argument to OS_REG_RMW_FIELD() which is 2 in this
>> case. Unless I've made a mistake, this translates into the
>>
>> data |= 0x00000800;
>>
>> in my patch.
>>
>> There definitely is no way for me to connect to an ap (or even get a
>> list of aps) with current 2.6.27.4. Perhaps we'll hear something from
>> Nils on that matter too.
>>
>
> Some lines above...
>
> 	/* Overwrite INI values for revised chipsets */
> 	if (AH_PRIVATE(ah)->ah_phyRev >= AR_PHY_CHIP_ID_REV_2) {
>
> so
>
> 		/* Add barker RSSI thresh enable as disabled */
> 		OS_REG_CLR_BIT(ah, AR_PHY_DAG_CTRLCCK,
> 			AR_PHY_DAG_CTRLCCK_EN_RSSI_THR);
> 		OS_REG_RMW_FIELD(ah, AR_PHY_DAG_CTRLCCK,
> 			AR_PHY_DAG_CTRLCCK_RSSI_THR, 2);
>
> is for phy (BB) revisions > 2 since you have RF5111 this code should
> not run but (and here is our bug) instead of checking bb revision we
> run this code for all AR5212 chips
>
> 		/*
> 		 * Write some more initial register settings
> 		 */
> 		if (ah->ah_version == AR5K_AR5212) {
>
[...]
> So can you please change the check on ath5k_hw_reset + add initial
> rssi threshold setting and retry ?

Is this what you had in mind? It works for me.

Regards,

Elias

--------
From: Elias Oltmanns <eo@nebensachen.de>
Subject: ath5k: Fix reset sequence for AR5212 in general and RF5111 in particular

Take care to handle register 0xa228 exactly as in the HAL released by
Atheros. This change is required to make ath5k work again on my system
since commit 2203d6be (ath5k: Misc hw_reset updates), thus fixing a
regression in 2.6.27 and therefore hopefully eligible for inclusion into
a stable release.

v2: Only overwrite initial register values on later revisions of AR5212
    chips.

Cc: stable <stable@kernel.org>
Signed-off-by: Elias Oltmanns <eo@nebensachen.de>
---

 drivers/net/wireless/ath5k/hw.c       |    4 +++-
 drivers/net/wireless/ath5k/initvals.c |    2 ++
 2 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/ath5k/hw.c b/drivers/net/wireless/ath5k/hw.c
index ad1a5b4..68cf2ae 100644
--- a/drivers/net/wireless/ath5k/hw.c
+++ b/drivers/net/wireless/ath5k/hw.c
@@ -828,7 +828,8 @@ int ath5k_hw_reset(struct ath5k_hw *ah, enum ieee80211_if_types op_mode,
 		/*
 		 * Write some more initial register settings
 		 */
-		if (ah->ah_version == AR5K_AR5212) {
+		if (ah->ah_version == AR5K_AR5212 &&
+		    ah->ah_radio > AR5K_RF5111) {
 			ath5k_hw_reg_write(ah, 0x0002a002, 0x982c);
 
 			if (channel->hw_value == CHANNEL_G)
@@ -858,6 +859,7 @@ int ath5k_hw_reset(struct ath5k_hw *ah, enum ieee80211_if_types op_mode,
 
 			data = ath5k_hw_reg_read(ah, 0xa228);
 			data &= 0xfffe03ff;
+			data |= 0x00000800;
 			ath5k_hw_reg_write(ah, data, 0xa228);
 			data = 0;
 
diff --git a/drivers/net/wireless/ath5k/initvals.c b/drivers/net/wireless/ath5k/initvals.c
index 2806b21..cf7ebd1 100644
--- a/drivers/net/wireless/ath5k/initvals.c
+++ b/drivers/net/wireless/ath5k/initvals.c
@@ -810,6 +810,8 @@ static const struct ath5k_ini_mode ar5212_rf5111_ini_mode_end[] = {
 		{ 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000 } },
 	{ AR5K_PHY(642),
 		{ 0xd03e6788, 0xd03e6788, 0xd03e6788, 0xd03e6788, 0xd03e6788 } },
+	{ 0xa228,
+		{ 0x000001b5, 0x000001b5, 0x000001b5, 0x000001b5, 0x000001b5 } },
 	{ 0xa23c,
 		{ 0x13c889af, 0x13c889af, 0x13c889af, 0x13c889af, 0x13c889af } },
 };

Re: [PATCH] ath5k: Fix reset sequence for AR5212 in general and RF5111 in particular

[Nick Kossifidis]

2008/10/26 Elias Oltmanns <eo@nebensachen.de>:
>
>                /*
>                 * Write some more initial register settings
>                 */
> -               if (ah->ah_version == AR5K_AR5212) {
> +               if (ah->ah_version == AR5K_AR5212 &&
> +                   ah->ah_radio > AR5K_RF5111) {

even better: ah->ah_phy_revision > AR5K_SREV_PHY_5212

>                        ath5k_hw_reg_write(ah, 0x0002a002, 0x982c);
>
>                        if (channel->hw_value == CHANNEL_G)
> @@ -858,6 +859,7 @@ int ath5k_hw_reset(struct ath5k_hw *ah, enum ieee80211_if_types op_mode,
>
>                        data = ath5k_hw_reg_read(ah, 0xa228);
>                        data &= 0xfffe03ff;
> +                       data |= 0x00000800;
>                        ath5k_hw_reg_write(ah, data, 0xa228);
>                        data = 0;
>

Can you please use the macros from ath5k.h to set these parameters ?
(i got the above from decompiling binary HAL that's why i have fixed
masks -no magic values- but now that we know what's going on we must
make the code human-readable)
And also please change 0xa228 with it's name from reg.h :

2557 #define AR5K_PHY_DAG_CCK_CTL                    0xa228
2558 #define AR5K_PHY_DAG_CCK_CTL_EN_RSSI_THR        0x00000200
2559 #define AR5K_PHY_DAG_CCK_CTL_RSSI_THR           0x0001fc00
2560 #define AR5K_PHY_DAG_CCK_CTL_RSSI_THR_S         10

Normaly i'll update reset.c this week so you can always wait for a
more generic fix ;-)

-- 
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick

Re: [PATCH] ath5k: Fix reset sequence for AR5212 in general and RF5111 in particular

[Elias Oltmanns]

"Nick Kossifidis" <mickflemm@gmail.com> wrote:
> 2008/10/26 Elias Oltmanns <eo@nebensachen.de>:
>>
>
>>                /*
>>                 * Write some more initial register settings
>>                 */
>> -               if (ah->ah_version == AR5K_AR5212) {
>> +               if (ah->ah_version == AR5K_AR5212 &&
>> +                   ah->ah_radio > AR5K_RF5111) {
>
> even better: ah->ah_phy_revision > AR5K_SREV_PHY_5212

Just to be absolutely sure: ah->ah_phy_revision >= AR5K_SREV_PHY_5212
right?

>
>>                        ath5k_hw_reg_write(ah, 0x0002a002, 0x982c);
>>
>>                        if (channel->hw_value == CHANNEL_G)
>> @@ -858,6 +859,7 @@ int ath5k_hw_reset(struct ath5k_hw *ah, enum ieee80211_if_types op_mode,
>>
>>                        data = ath5k_hw_reg_read(ah, 0xa228);
>>                        data &= 0xfffe03ff;
>> +                       data |= 0x00000800;
>>                        ath5k_hw_reg_write(ah, data, 0xa228);
>>                        data = 0;
>>
>
> Can you please use the macros from ath5k.h to set these parameters ?
> (i got the above from decompiling binary HAL that's why i have fixed
> masks -no magic values- but now that we know what's going on we must
> make the code human-readable)
> And also please change 0xa228 with it's name from reg.h :
>
> 2557 #define AR5K_PHY_DAG_CCK_CTL                    0xa228
> 2558 #define AR5K_PHY_DAG_CCK_CTL_EN_RSSI_THR        0x00000200
> 2559 #define AR5K_PHY_DAG_CCK_CTL_RSSI_THR           0x0001fc00
> 2560 #define AR5K_PHY_DAG_CCK_CTL_RSSI_THR_S         10
>
> Normaly i'll update reset.c this week so you can always wait for a
> more generic fix ;-)

Well, I'm always happy to leave the work to others ;-). The only reason
why I kept resending patches was to make sure that a minimal fix would
get merged into a stable release eventually. Presumably, a rework of the
reset sequence in itself would not be appropriate for stable. Still, if
you'll take care get something into stable, that's fine with me.
Otherwise, just let me know and I'll keep those patches coming and you
occupied reviewing them ;-).

Regards,

Elias

Re: [PATCH] ath5k: Fix reset sequence for AR5212 in general and RF5111 in particular

[Nick Kossifidis]

2008/10/27 Elias Oltmanns <eo@nebensachen.de>:
> "Nick Kossifidis" <mickflemm@gmail.com> wrote:
>> 2008/10/26 Elias Oltmanns <eo@nebensachen.de>:
>>>
>>
>>>                /*
>>>                 * Write some more initial register settings
>>>                 */
>>> -               if (ah->ah_version == AR5K_AR5212) {
>>> +               if (ah->ah_version == AR5K_AR5212 &&
>>> +                   ah->ah_radio > AR5K_RF5111) {
>>
>> even better: ah->ah_phy_revision > AR5K_SREV_PHY_5212
>
> Just to be absolutely sure: ah->ah_phy_revision >= AR5K_SREV_PHY_5212
> right?
>

AR5K_SREV_PHY_5212 is bb rev 1 so its

ah->ah_phy > AR5K_SREV_PHY_5212

>
> Well, I'm always happy to leave the work to others ;-). The only reason
> why I kept resending patches was to make sure that a minimal fix would
> get merged into a stable release eventually. Presumably, a rework of the
> reset sequence in itself would not be appropriate for stable. Still, if
> you'll take care get something into stable, that's fine with me.
> Otherwise, just let me know and I'll keep those patches coming and you
> occupied reviewing them ;-).
>
> Regards,
>
> Elias
>

O.K. then we 'll include your patch on stable and my work will go on
wireless-dev for further testing ;-)

-- 
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick

Re: [PATCH] ath5k: Fix reset sequence for AR5212 in general and RF5111 in particular

[Elias Oltmanns]

"Nick Kossifidis" <mickflemm@gmail.com> wrote:
> 2008/10/27 Elias Oltmanns <eo@nebensachen.de>:
>> "Nick Kossifidis" <mickflemm@gmail.com> wrote:
>
>>> 2008/10/26 Elias Oltmanns <eo@nebensachen.de>:
>>>>
>>>
>>>>                /*
>>>>                 * Write some more initial register settings
>>>>                 */
>>>> -               if (ah->ah_version == AR5K_AR5212) {
>>>> +               if (ah->ah_version == AR5K_AR5212 &&
>>>> +                   ah->ah_radio > AR5K_RF5111) {
>>>
>>> even better: ah->ah_phy_revision > AR5K_SREV_PHY_5212
>>
>> Just to be absolutely sure: ah->ah_phy_revision >= AR5K_SREV_PHY_5212
>> right?
>>
>
> AR5K_SREV_PHY_5212 is bb rev 1 so its
>
> ah->ah_phy > AR5K_SREV_PHY_5212

I see, thanks for clarifying.

>
>>
>> Well, I'm always happy to leave the work to others ;-). The only reason
>> why I kept resending patches was to make sure that a minimal fix would
>> get merged into a stable release eventually. Presumably, a rework of the
>> reset sequence in itself would not be appropriate for stable. Still, if
>> you'll take care get something into stable, that's fine with me.
>> Otherwise, just let me know and I'll keep those patches coming and you
>> occupied reviewing them ;-).
>>
>> Regards,
>>
>> Elias
>>
>
> O.K. then we 'll include your patch on stable and my work will go on
> wireless-dev for further testing ;-)

Right, so here is another attempt. Unfortunately, in 2.6.27 no constants
have been defined for the phy revisions, nor the 0xa228 register and the
bit masks involved in resetting this register. Perhaps we should consult
the stable team on this matter because the minimal approach, as
presented below, certainly lacks even the smallest degree of readability
even though we actually know reasonably well what we are doing here. The
changelog, by the way, will be adapted to quote your changeset fixing
this in mainline once it has been committed. What do you think?

Regards,

Elias
--------
From: Elias Oltmanns <eo@nebensachen.de>
Subject: ath5k: Fix reset sequence for AR5212 in general and RF5111 in particular

Take care to handle register 0xa228 exactly as in the HAL released by
Atheros. This change is required to make ath5k work again on my system
since commit 2203d6be (ath5k: Misc hw_reset updates), thus fixing a
regression in 2.6.27 and therefore hopefully eligible for inclusion into
a stable release.

v2: Only overwrite initial register values on later revisions of AR5212
    chips.
v3: Use standard macros to manipulate the register.

Signed-off-by: Elias Oltmanns <eo@nebensachen.de>
---

 drivers/net/wireless/ath5k/hw.c       |   22 +++++++---------------
 drivers/net/wireless/ath5k/initvals.c |    2 ++
 2 files changed, 9 insertions(+), 15 deletions(-)

diff --git a/drivers/net/wireless/ath5k/hw.c b/drivers/net/wireless/ath5k/hw.c
index ad1a5b4..67b71e7 100644
--- a/drivers/net/wireless/ath5k/hw.c
+++ b/drivers/net/wireless/ath5k/hw.c
@@ -826,9 +826,10 @@ int ath5k_hw_reset(struct ath5k_hw *ah, enum ieee80211_if_types op_mode,
 		mdelay(1);
 
 		/*
-		 * Write some more initial register settings
+		 * Write some more initial register settings for revised chips
 		 */
-		if (ah->ah_version == AR5K_AR5212) {
+		if (ah->ah_version == AR5K_AR5212 &&
+		    ah->ah_phy_revision > 0x41) {
 			ath5k_hw_reg_write(ah, 0x0002a002, 0x982c);
 
 			if (channel->hw_value == CHANNEL_G)
@@ -847,19 +848,10 @@ int ath5k_hw_reset(struct ath5k_hw *ah, enum ieee80211_if_types op_mode,
 			else
 				ath5k_hw_reg_write(ah, 0x00000000, 0x994c);
 
-			/* Some bits are disabled here, we know nothing about
-			 * register 0xa228 yet, most of the times this ends up
-			 * with a value 0x9b5 -haven't seen any dump with
-			 * a different value- */
-			/* Got this from decompiling binary HAL */
-			data = ath5k_hw_reg_read(ah, 0xa228);
-			data &= 0xfffffdff;
-			ath5k_hw_reg_write(ah, data, 0xa228);
-
-			data = ath5k_hw_reg_read(ah, 0xa228);
-			data &= 0xfffe03ff;
-			ath5k_hw_reg_write(ah, data, 0xa228);
-			data = 0;
+			/* Got this from legacy-hal */
+			AR5K_REG_DISABLE_BITS(ah, 0xa228, 0x200);
+
+			AR5K_REG_MASKED_BITS(ah, 0xa228, 0x800, 0xfffe03ff);
 
 			/* Just write 0x9b5 ? */
 			/* ath5k_hw_reg_write(ah, 0x000009b5, 0xa228); */
diff --git a/drivers/net/wireless/ath5k/initvals.c b/drivers/net/wireless/ath5k/initvals.c
index 2806b21..cf7ebd1 100644
--- a/drivers/net/wireless/ath5k/initvals.c
+++ b/drivers/net/wireless/ath5k/initvals.c
@@ -810,6 +810,8 @@ static const struct ath5k_ini_mode ar5212_rf5111_ini_mode_end[] = {
 		{ 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000 } },
 	{ AR5K_PHY(642),
 		{ 0xd03e6788, 0xd03e6788, 0xd03e6788, 0xd03e6788, 0xd03e6788 } },
+	{ 0xa228,
+		{ 0x000001b5, 0x000001b5, 0x000001b5, 0x000001b5, 0x000001b5 } },
 	{ 0xa23c,
 		{ 0x13c889af, 0x13c889af, 0x13c889af, 0x13c889af, 0x13c889af } },
 };

Re: [PATCH] ath5k: Fix reset sequence for AR5212 in general and RF5111 in particular

[Nick Kossifidis]

2008/10/29 Elias Oltmanns <eo@nebensachen.de>:
>
> Right, so here is another attempt. Unfortunately, in 2.6.27 no constants
> have been defined for the phy revisions, nor the 0xa228 register and the
> bit masks involved in resetting this register. Perhaps we should consult
> the stable team on this matter because the minimal approach, as
> presented below, certainly lacks even the smallest degree of readability
> even though we actually know reasonably well what we are doing here. The
> changelog, by the way, will be adapted to quote your changeset fixing
> this in mainline once it has been committed. What do you think?
>

Patch looks OK, thanks a lot ;-)

John can we get this get in stable until i update reset.c on wireless-testing ?
Would it be possible later to update mainline from wireless-testing ?


-- 
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick

Re: [PATCH] ath5k: Fix reset sequence for AR5212 in general and RF5111 in particular

[Elias Oltmanns]

"Nick Kossifidis" <mickflemm@gmail.com> wrote:
> 2008/10/29 Elias Oltmanns <eo@nebensachen.de>:
>>
>
>> Right, so here is another attempt. Unfortunately, in 2.6.27 no constants
>> have been defined for the phy revisions, nor the 0xa228 register and the
>> bit masks involved in resetting this register. Perhaps we should consult
>> the stable team on this matter because the minimal approach, as
>> presented below, certainly lacks even the smallest degree of readability
>> even though we actually know reasonably well what we are doing here. The
>> changelog, by the way, will be adapted to quote your changeset fixing
>> this in mainline once it has been committed. What do you think?
>>
>
> Patch looks OK, thanks a lot ;-)
>
> John can we get this get in stable until i update reset.c on wireless-testing ?
> Would it be possible later to update mainline from wireless-testing ?

Reading Documentation/stable_kernel_rules.txt, I very much doubt that
this patch will go in unless we can at least point out something
equivalent in mainline. So, we'll have to wait until your changes to
reset.c have reached mainline, I'm afraid.

Regards,

Elias

Re: [PATCH] ath5k: Fix reset sequence for AR5212 in general and RF5111 in particular

[Bob Copeland]

On Wed, Oct 29, 2008 at 11:07 AM, Elias Oltmanns <eo@nebensachen.de> wrote:
>>
>> Patch looks OK, thanks a lot ;-)
>>
>> John can we get this get in stable until i update reset.c on wireless-testing ?
>> Would it be possible later to update mainline from wireless-testing ?
>
> Reading Documentation/stable_kernel_rules.txt, I very much doubt that
> this patch will go in unless we can at least point out something
> equivalent in mainline. So, we'll have to wait until your changes to
> reset.c have reached mainline, I'm afraid.

Or, we could just send your patch to both stable and for 2.6.28.  Then Nick
can fix up the difference or revert it in his rework.  The rework is 2.6.29
material, right?

-- 
Bob Copeland %% www.bobcopeland.com

Re: [PATCH] ath5k: Fix reset sequence for AR5212 in general and RF5111 in particular

[Nick Kossifidis]

2008/10/29 Bob Copeland <me@bobcopeland.com>:
> On Wed, Oct 29, 2008 at 11:07 AM, Elias Oltmanns <eo@nebensachen.de> wrote:
>>>
>>> Patch looks OK, thanks a lot ;-)
>>>
>>> John can we get this get in stable until i update reset.c on wireless-testing ?
>>> Would it be possible later to update mainline from wireless-testing ?
>>
>> Reading Documentation/stable_kernel_rules.txt, I very much doubt that
>> this patch will go in unless we can at least point out something
>> equivalent in mainline. So, we'll have to wait until your changes to
>> reset.c have reached mainline, I'm afraid.
>
> Or, we could just send your patch to both stable and for 2.6.28.  Then Nick
> can fix up the difference or revert it in his rework.  The rework is 2.6.29
> material, right?
>

ACK



-- 
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick

Re: [PATCH] ath5k: Fix reset sequence for AR5212 in general and RF5111 in particular

[Elias Oltmanns]

"Nick Kossifidis" <mickflemm@gmail.com> wrote:
> 2008/10/29 Bob Copeland <me@bobcopeland.com>:
>> On Wed, Oct 29, 2008 at 11:07 AM, Elias Oltmanns <eo@nebensachen.de> wrote:
>
>>>>
>>>> Patch looks OK, thanks a lot ;-)
>>>>
>>>> John can we get this get in stable until i update reset.c on wireless-testing ?
>>>> Would it be possible later to update mainline from wireless-testing ?
>>>
>>> Reading Documentation/stable_kernel_rules.txt, I very much doubt that
>>> this patch will go in unless we can at least point out something
>>> equivalent in mainline. So, we'll have to wait until your changes to
>>> reset.c have reached mainline, I'm afraid.
>>
>> Or, we could just send your patch to both stable and for 2.6.28.  Then Nick
>> can fix up the difference or revert it in his rework.  The rework is 2.6.29
>> material, right?
>>
>
> ACK

In that case, I'd suggest the following patch for 2.6.28-rc2. Once it
has been merged, I'll send the previous one off to the stable team.

Regards,

Elias
--------
From: Elias Oltmanns <eo@nebensachen.de>
Subject: ath5k: Fix reset sequence for AR5212 in general and RF5111 in particular

Take care to handle register 0xa228 exactly as in the HAL released by
Atheros. This change is required to make ath5k work again on my system
since commit 2203d6be (ath5k: Misc hw_reset updates), thus fixing a
regression in 2.6.27 and therefore hopefully eligible for inclusion into
a stable release.

v2: Only overwrite initial register values on later revisions of AR5212
    chips.
v3: Use standard macros to manipulate the register.
v4: Use appropriate constants (new to 2.6.28) for register access.

Signed-off-by: Elias Oltmanns <eo@nebensachen.de>
---

 drivers/net/wireless/ath5k/initvals.c |    2 ++
 drivers/net/wireless/ath5k/reset.c    |   25 ++++++++++---------------
 2 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/net/wireless/ath5k/initvals.c b/drivers/net/wireless/ath5k/initvals.c
index ea2e1a2..ceaa6c4 100644
--- a/drivers/net/wireless/ath5k/initvals.c
+++ b/drivers/net/wireless/ath5k/initvals.c
@@ -806,6 +806,8 @@ static const struct ath5k_ini_mode ar5212_rf5111_ini_mode_end[] = {
 		{ 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000 } },
 	{ AR5K_PHY(642),
 		{ 0xd03e6788, 0xd03e6788, 0xd03e6788, 0xd03e6788, 0xd03e6788 } },
+	{ 0xa228,
+		{ 0x000001b5, 0x000001b5, 0x000001b5, 0x000001b5, 0x000001b5 } },
 	{ 0xa23c,
 		{ 0x13c889af, 0x13c889af, 0x13c889af, 0x13c889af, 0x13c889af } },
 };
diff --git a/drivers/net/wireless/ath5k/reset.c b/drivers/net/wireless/ath5k/reset.c
index 8f18868..8fdb092 100644
--- a/drivers/net/wireless/ath5k/reset.c
+++ b/drivers/net/wireless/ath5k/reset.c
@@ -537,9 +537,10 @@ int ath5k_hw_reset(struct ath5k_hw *ah, enum nl80211_iftype op_mode,
 		mdelay(1);
 
 		/*
-		 * Write some more initial register settings
+		 * Write some more initial register settings for revised chips
 		 */
-		if (ah->ah_version == AR5K_AR5212) {
+		if (ah->ah_version == AR5K_AR5212 &&
+		    ah->ah_phy_revision > AR5K_SREV_PHY_5212) {
 			ath5k_hw_reg_write(ah, 0x0002a002, 0x982c);
 
 			if (channel->hw_value == CHANNEL_G)
@@ -558,19 +559,13 @@ int ath5k_hw_reset(struct ath5k_hw *ah, enum nl80211_iftype op_mode,
 			else
 				ath5k_hw_reg_write(ah, 0x00000000, 0x994c);
 
-			/* Some bits are disabled here, we know nothing about
-			 * register 0xa228 yet, most of the times this ends up
-			 * with a value 0x9b5 -haven't seen any dump with
-			 * a different value- */
-			/* Got this from decompiling binary HAL */
-			data = ath5k_hw_reg_read(ah, 0xa228);
-			data &= 0xfffffdff;
-			ath5k_hw_reg_write(ah, data, 0xa228);
-
-			data = ath5k_hw_reg_read(ah, 0xa228);
-			data &= 0xfffe03ff;
-			ath5k_hw_reg_write(ah, data, 0xa228);
-			data = 0;
+			/* Got this from legacy-hal */
+			AR5K_REG_DISABLE_BITS(ah, AR5K_PHY_DAG_CCK_CTL,
+					      AR5K_PHY_DAG_CCK_CTL_EN_RSSI_THR);
+
+			AR5K_REG_MASKED_BITS(ah, AR5K_PHY_DAG_CCK_CTL,
+					     2 << AR5K_PHY_DAG_CCK_CTL_RSSI_THR_S,
+					     ~AR5K_PHY_DAG_CCK_CTL_RSSI_THR);
 
 			/* Just write 0x9b5 ? */
 			/* ath5k_hw_reg_write(ah, 0x000009b5, 0xa228); */

Re: [PATCH] ath5k: Fix reset sequence for AR5212 in general and RF5111 in particular

[John W. Linville]

Well, I already merged v3...what is new here?

On Thu, Oct 30, 2008 at 12:25:08AM +0100, Elias Oltmanns wrote:
> "Nick Kossifidis" <mickflemm@gmail.com> wrote:
> > 2008/10/29 Bob Copeland <me@bobcopeland.com>:
> >> On Wed, Oct 29, 2008 at 11:07 AM, Elias Oltmanns <eo@nebensachen.de> wrote:
> >
> >>>>
> >>>> Patch looks OK, thanks a lot ;-)
> >>>>
> >>>> John can we get this get in stable until i update reset.c on wireless-testing ?
> >>>> Would it be possible later to update mainline from wireless-testing ?
> >>>
> >>> Reading Documentation/stable_kernel_rules.txt, I very much doubt that
> >>> this patch will go in unless we can at least point out something
> >>> equivalent in mainline. So, we'll have to wait until your changes to
> >>> reset.c have reached mainline, I'm afraid.
> >>
> >> Or, we could just send your patch to both stable and for 2.6.28.  Then Nick
> >> can fix up the difference or revert it in his rework.  The rework is 2.6.29
> >> material, right?
> >>
> >
> > ACK
> 
> In that case, I'd suggest the following patch for 2.6.28-rc2. Once it
> has been merged, I'll send the previous one off to the stable team.
> 
> Regards,
> 
> Elias
> --------
> From: Elias Oltmanns <eo@nebensachen.de>
> Subject: ath5k: Fix reset sequence for AR5212 in general and RF5111 in particular
> 
> Take care to handle register 0xa228 exactly as in the HAL released by
> Atheros. This change is required to make ath5k work again on my system
> since commit 2203d6be (ath5k: Misc hw_reset updates), thus fixing a
> regression in 2.6.27 and therefore hopefully eligible for inclusion into
> a stable release.
> 
> v2: Only overwrite initial register values on later revisions of AR5212
>     chips.
> v3: Use standard macros to manipulate the register.
> v4: Use appropriate constants (new to 2.6.28) for register access.
> 
> Signed-off-by: Elias Oltmanns <eo@nebensachen.de>
> ---
> 
>  drivers/net/wireless/ath5k/initvals.c |    2 ++
>  drivers/net/wireless/ath5k/reset.c    |   25 ++++++++++---------------
>  2 files changed, 12 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath5k/initvals.c b/drivers/net/wireless/ath5k/initvals.c
> index ea2e1a2..ceaa6c4 100644
> --- a/drivers/net/wireless/ath5k/initvals.c
> +++ b/drivers/net/wireless/ath5k/initvals.c
> @@ -806,6 +806,8 @@ static const struct ath5k_ini_mode ar5212_rf5111_ini_mode_end[] = {
>  		{ 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000 } },
>  	{ AR5K_PHY(642),
>  		{ 0xd03e6788, 0xd03e6788, 0xd03e6788, 0xd03e6788, 0xd03e6788 } },
> +	{ 0xa228,
> +		{ 0x000001b5, 0x000001b5, 0x000001b5, 0x000001b5, 0x000001b5 } },
>  	{ 0xa23c,
>  		{ 0x13c889af, 0x13c889af, 0x13c889af, 0x13c889af, 0x13c889af } },
>  };
> diff --git a/drivers/net/wireless/ath5k/reset.c b/drivers/net/wireless/ath5k/reset.c
> index 8f18868..8fdb092 100644
> --- a/drivers/net/wireless/ath5k/reset.c
> +++ b/drivers/net/wireless/ath5k/reset.c
> @@ -537,9 +537,10 @@ int ath5k_hw_reset(struct ath5k_hw *ah, enum nl80211_iftype op_mode,
>  		mdelay(1);
>  
>  		/*
> -		 * Write some more initial register settings
> +		 * Write some more initial register settings for revised chips
>  		 */
> -		if (ah->ah_version == AR5K_AR5212) {
> +		if (ah->ah_version == AR5K_AR5212 &&
> +		    ah->ah_phy_revision > AR5K_SREV_PHY_5212) {
>  			ath5k_hw_reg_write(ah, 0x0002a002, 0x982c);
>  
>  			if (channel->hw_value == CHANNEL_G)
> @@ -558,19 +559,13 @@ int ath5k_hw_reset(struct ath5k_hw *ah, enum nl80211_iftype op_mode,
>  			else
>  				ath5k_hw_reg_write(ah, 0x00000000, 0x994c);
>  
> -			/* Some bits are disabled here, we know nothing about
> -			 * register 0xa228 yet, most of the times this ends up
> -			 * with a value 0x9b5 -haven't seen any dump with
> -			 * a different value- */
> -			/* Got this from decompiling binary HAL */
> -			data = ath5k_hw_reg_read(ah, 0xa228);
> -			data &= 0xfffffdff;
> -			ath5k_hw_reg_write(ah, data, 0xa228);
> -
> -			data = ath5k_hw_reg_read(ah, 0xa228);
> -			data &= 0xfffe03ff;
> -			ath5k_hw_reg_write(ah, data, 0xa228);
> -			data = 0;
> +			/* Got this from legacy-hal */
> +			AR5K_REG_DISABLE_BITS(ah, AR5K_PHY_DAG_CCK_CTL,
> +					      AR5K_PHY_DAG_CCK_CTL_EN_RSSI_THR);
> +
> +			AR5K_REG_MASKED_BITS(ah, AR5K_PHY_DAG_CCK_CTL,
> +					     2 << AR5K_PHY_DAG_CCK_CTL_RSSI_THR_S,
> +					     ~AR5K_PHY_DAG_CCK_CTL_RSSI_THR);
>  
>  			/* Just write 0x9b5 ? */
>  			/* ath5k_hw_reg_write(ah, 0x000009b5, 0xa228); */
> 

-- 
John W. Linville		Linux should be at the core
linville@tuxdriver.com			of your literate lifestyle.

Re: [PATCH] ath5k: Fix reset sequence for AR5212 in general and RF5111 in particular

[Elias Oltmanns]

"John W. Linville" <linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org> wrote:
> Well, I already merged v3...what is new here?

Yes, that's alright. The only difference is that I've replaced all the
ugly hex-numbers by the appropriate constants in ath5k/reg.h that have
been introduced in 2.6.28-rc1. So, it's really just a matter of style.

Regards,

Elias

Re: [PATCH] ath5k: Fix reset sequence for AR5212 in general and RF5111 in particular

[Elias Oltmanns]

[ Resending due to a totally screwed header. Time to go to bed now. ]

"John W. Linville" <linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org> wrote:
> Well, I already merged v3...what is new here?

Yes, that's alright. The only difference is that I've replaced all the
ugly hex-numbers by the appropriate constants in ath5k/reg.h that have
been introduced in 2.6.28-rc1. So, it's really just a matter of style.

Regards,

Elias

Re: [PATCH] ath5k: Fix reset sequence for AR5212 in general and RF5111 in particular

[Nils]

> [Bcc'ing Nils who has reported similar issues with ath5k on 2.6.27.
> Nils, can you perhaps test my patch below and report back? ]

Yes! I test the patch with 2.6.27.4 and everything works fine.

"iwlist ath0 scan" shows all wireless LANs at my location and I can connect to 
my accesspoint.

Thanks,
Nils


> "Nick Kossifidis" <mickflemm@gmail.com> wrote:
> > 2008/10/23 Elias Oltmanns <eo@nebensachen.de>:
> >> Take care to handle register 0xa228 exactly as in the HAL released by
> >>
> >> Atheros. This change is required to make ath5k work again on my system
> >> since commit 2203d6be (ath5k: Misc hw_reset updates), thus fixing a
> >> regression in 2.6.27 and therefore hopefully eligible for inclusion into
> >> a stable release.
> >>
> >> Cc: stable <stable@kernel.org>
> >> Signed-off-by: Elias Oltmanns <eo@nebensachen.de>
> >> ---
> >> Applies to 2.6.27. Please let me know if I should do something
> >> differently and resend.
> >>
> >>  drivers/net/wireless/ath5k/hw.c       |    1 +
> >>  drivers/net/wireless/ath5k/initvals.c |    2 ++
> >>  2 files changed, 3 insertions(+), 0 deletions(-)
> >
> > Current code works fine on my CM6 (AR5212 + RF5111) and i see nothing
> > like
> >
> > data |= 0x00000800
> >
> > on the HAL Atheros released
>
> Let me quote from legacy-hal:
>
> ar5212/ar5212_reset.c:
> 373:		OS_REG_RMW_FIELD(ah, AR_PHY_DAG_CTRLCCK,
> 374:			AR_PHY_DAG_CTRLCCK_RSSI_THR, 2);
>
> ah_internal.h:
> 598:	#define OS_REG_RMW_FIELD(_a, _r, _f, _v) \
> 599:	    OS_REG_WRITE(_a, _r, \
> 600:	        (OS_REG_READ(_a, _r) &~ _f) | (((_v) << _f##_S) & _f))
>
> Please note the last argument to OS_REG_RMW_FIELD() which is 2 in this
> case. Unless I've made a mistake, this translates into the
>
> data |= 0x00000800;
>
> in my patch.
>
> There definitely is no way for me to connect to an ap (or even get a
> list of aps) with current 2.6.27.4. Perhaps we'll hear something from
> Nils on that matter too.
>
> > Problem is that we are updating 0xa228 (RSSI threshold in this case)
> > for all AR5212 based cards and we are not checking right now for BB
> > revision. These settings according to Atheros HAL are only for BB
> > revisions > 2. I'm going to update reset.c this week hopefully.
> >
> > BTW can you please run latest ath_info on your card ? Let's see what's
> > the rssi threshold stored on EEPROM and how much different is from my
> > card.
>
> Well, I don't see how to get that information from the dump. Please have
> a look for yourself, I'm going to send you my dump by pm.
>
> Since my original email has not appeared on linux-wireless (due to a
> mistake of mine), I'm attaching the patch again, so others can have a
> look at it and, more importantly, Nils can test it.
>
> Regards,
>
> Elias
>
> --------
> From: Elias Oltmanns <eo@nebensachen.de>
> Subject: [PATCH] ath5k: Fix reset sequence for AR5212 in general and RF5111
> in particular
>
> Take care to handle register 0xa228 exactly as in the HAL released by
> Atheros. This change is required to make ath5k work again on my system
> since commit 2203d6be (ath5k: Misc hw_reset updates), thus fixing a
> regression in 2.6.27 and therefore hopefully eligible for inclusion into
> a stable release.
>
> Cc: stable <stable@kernel.org>
> Signed-off-by: Elias Oltmanns <eo@nebensachen.de>
> ---
>
>  drivers/net/wireless/ath5k/hw.c       |    1 +
>  drivers/net/wireless/ath5k/initvals.c |    2 ++
>  2 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/wireless/ath5k/hw.c
> b/drivers/net/wireless/ath5k/hw.c index ad1a5b4..f1a7f71 100644
> --- a/drivers/net/wireless/ath5k/hw.c
> +++ b/drivers/net/wireless/ath5k/hw.c
> @@ -858,6 +858,7 @@ int ath5k_hw_reset(struct ath5k_hw *ah, enum
> ieee80211_if_types op_mode,
>
>  			data = ath5k_hw_reg_read(ah, 0xa228);
>  			data &= 0xfffe03ff;
> +			data |= 0x00000800;
>  			ath5k_hw_reg_write(ah, data, 0xa228);
>  			data = 0;
>
> diff --git a/drivers/net/wireless/ath5k/initvals.c
> b/drivers/net/wireless/ath5k/initvals.c index 2806b21..cf7ebd1 100644
> --- a/drivers/net/wireless/ath5k/initvals.c
> +++ b/drivers/net/wireless/ath5k/initvals.c
> @@ -810,6 +810,8 @@ static const struct ath5k_ini_mode
> ar5212_rf5111_ini_mode_end[] = { { 0x00000000, 0x00000000, 0x00000000,
> 0x00000000, 0x00000000 } }, { AR5K_PHY(642),
>  		{ 0xd03e6788, 0xd03e6788, 0xd03e6788, 0xd03e6788, 0xd03e6788 } },
> +	{ 0xa228,
> +		{ 0x000001b5, 0x000001b5, 0x000001b5, 0x000001b5, 0x000001b5 } },
>  	{ 0xa23c,
>  		{ 0x13c889af, 0x13c889af, 0x13c889af, 0x13c889af, 0x13c889af } },
>  };