2008-10-29 13:25:53

by Elias Oltmanns

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

"Nick Kossifidis" <[email protected]> wrote:
> 2008/10/27 Elias Oltmanns <[email protected]>:
>> "Nick Kossifidis" <[email protected]> wrote:
>
>>> 2008/10/26 Elias Oltmanns <[email protected]>:
>>>>
>>>
>>>> /*
>>>> * 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 <[email protected]>
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 <[email protected]>
---

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 } },
};


2008-10-29 23:28:58

by Elias Oltmanns

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

"Nick Kossifidis" <[email protected]> wrote:
> 2008/10/29 Bob Copeland <[email protected]>:
>> On Wed, Oct 29, 2008 at 11:07 AM, Elias Oltmanns <[email protected]> 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 <[email protected]>
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 <[email protected]>
---

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); */

2008-10-29 23:35:46

by John W. Linville

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

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

On Thu, Oct 30, 2008 at 12:25:08AM +0100, Elias Oltmanns wrote:
> "Nick Kossifidis" <[email protected]> wrote:
> > 2008/10/29 Bob Copeland <[email protected]>:
> >> On Wed, Oct 29, 2008 at 11:07 AM, Elias Oltmanns <[email protected]> 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 <[email protected]>
> 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 <[email protected]>
> ---
>
> 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
[email protected] of your literate lifestyle.

2008-10-30 00:06:54

by Elias Oltmanns

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

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

"John W. Linville" <[email protected]> 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

2008-10-29 23:56:54

by Elias Oltmanns

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

"John W. Linville" <[email protected]> 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

2008-10-29 17:51:39

by Nick Kossifidis

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

2008/10/29 Bob Copeland <[email protected]>:
> On Wed, Oct 29, 2008 at 11:07 AM, Elias Oltmanns <[email protected]> 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

2008-10-29 15:28:45

by Bob Copeland

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

On Wed, Oct 29, 2008 at 11:07 AM, Elias Oltmanns <[email protected]> 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 %% http://www.bobcopeland.com

2008-10-29 14:49:31

by Nick Kossifidis

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

2008/10/29 Elias Oltmanns <[email protected]>:
>
> 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

2008-10-29 15:07:18

by Elias Oltmanns

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

"Nick Kossifidis" <[email protected]> wrote:
> 2008/10/29 Elias Oltmanns <[email protected]>:
>>
>
>> 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