2007-03-20 20:50:52

by Larry Finger

Subject: [PATCH] bcm43xx-mac80211: Change radio_init2050 to match specs

The code in the mac80211 version of radio_init2050 differs from the specs
in two places.

Signed-off-by: Larry Finger <[email protected]>
---

Index: wireless-mb/drivers/net/wireless/mac80211/bcm43xx/bcm43xx_phy.c
===================================================================
--- wireless-mb.orig/drivers/net/wireless/mac80211/bcm43xx/bcm43xx_phy.c
+++ wireless-mb/drivers/net/wireless/mac80211/bcm43xx/bcm43xx_phy.c
@@ -3444,9 +3444,9 @@ static u16 radio2050_rfover_val(struct b
if (phy_register == BCM43xx_PHY_RFOVER) {
return 0x9B3;
} else if (phy_register == BCM43xx_PHY_RFOVERVAL) {
- extlna |= (i << 8);
if (extlna)
extlna |= 0x8000;
+ extlna |= (i << 8);
switch (lpd) {
case LPD(0, 1, 1):
return 0x8F92;
@@ -3724,9 +3724,10 @@ u16 bcm43xx_radio_init2050(struct bcm43x
bcm43xx_phy_stackrestore(stack, BCM43xx_PHY_ANALOGOVERVAL);
bcm43xx_phy_stackrestore(stack, BCM43xx_PHY_CRS0);
bcm43xx_phy_stackrestore(stack, BCM43xx_PHY_CLASSCTL);
- bcm43xx_write16(dev, BCM43xx_MMIO_PHY_RADIO,
- bcm43xx_read16(dev, BCM43xx_MMIO_PHY_RADIO)
- & 0x7FFF);
+ if (has_loopback_gain(phy)) {
+ bcm43xx_phy_stackrestore(stack, BCM43xx_PHY_LO_MASK);
+ bcm43xx_phy_stackrestore(stack, BCM43xx_PHY_LO_CTL);
+ }
}
if (i > 15)
ret = radio78;

2007-03-21 16:18:04

by Michael Büsch

Subject: Re: [PATCH] bcm43xx-mac80211: Change radio_init2050 to match specs

On Wednesday 21 March 2007 16:29, Larry Finger wrote:
> Michael Buesch wrote:
> > On Tuesday 20 March 2007 21:49, Larry Finger wrote:
> >> The code in the mac80211 version of radio_init2050 differs from the specs
> >> in two places.
> >>
> >> Signed-off-by: Larry Finger <[email protected]>
> >> ---
> >>
> >> Index: wireless-mb/drivers/net/wireless/mac80211/bcm43xx/bcm43xx_phy.c
> >> ===================================================================
> >> --- wireless-mb.orig/drivers/net/wireless/mac80211/bcm43xx/bcm43xx_phy.c
> >> +++ wireless-mb/drivers/net/wireless/mac80211/bcm43xx/bcm43xx_phy.c
> >> @@ -3444,9 +3444,9 @@ static u16 radio2050_rfover_val(struct b
> >> if (phy_register == BCM43xx_PHY_RFOVER) {
> >> return 0x9B3;
> >> } else if (phy_register == BCM43xx_PHY_RFOVERVAL) {
> >> - extlna |= (i << 8);
> >> if (extlna)
> >> extlna |= 0x8000;
> >> + extlna |= (i << 8);
> >
> > What's the difference?
>
> The nonzero test for extlna should be made on the table value, not the table value or'd with i << 8.

Eh, wait.
The spec is not clear (at least to me) on this point:

# OR the loop position value left shifted by 8 with the External LNA Control Value
# If the External LNA Value isn't 0
1. OR the value with 0x8000
# OR this value with the table values below which are marked with Yes

>From my understanding current code is correct.
But I see that one could interpret the spec in your way as well.
Joseph?

--
Greetings Michael.

2007-03-21 14:02:09

by Michael Büsch

Subject: Re: [PATCH] bcm43xx-mac80211: Change radio_init2050 to match specs

On Tuesday 20 March 2007 21:49, Larry Finger wrote:
> The code in the mac80211 version of radio_init2050 differs from the specs
> in two places.
>
> Signed-off-by: Larry Finger <[email protected]>
> ---
>
> Index: wireless-mb/drivers/net/wireless/mac80211/bcm43xx/bcm43xx_phy.c
> ===================================================================
> --- wireless-mb.orig/drivers/net/wireless/mac80211/bcm43xx/bcm43xx_phy.c
> +++ wireless-mb/drivers/net/wireless/mac80211/bcm43xx/bcm43xx_phy.c
> @@ -3444,9 +3444,9 @@ static u16 radio2050_rfover_val(struct b
> if (phy_register == BCM43xx_PHY_RFOVER) {
> return 0x9B3;
> } else if (phy_register == BCM43xx_PHY_RFOVERVAL) {
> - extlna |= (i << 8);
> if (extlna)
> extlna |= 0x8000;
> + extlna |= (i << 8);

What's the difference?

> switch (lpd) {
> case LPD(0, 1, 1):
> return 0x8F92;
> @@ -3724,9 +3724,10 @@ u16 bcm43xx_radio_init2050(struct bcm43x
> bcm43xx_phy_stackrestore(stack, BCM43xx_PHY_ANALOGOVERVAL);
> bcm43xx_phy_stackrestore(stack, BCM43xx_PHY_CRS0);
> bcm43xx_phy_stackrestore(stack, BCM43xx_PHY_CLASSCTL);
> - bcm43xx_write16(dev, BCM43xx_MMIO_PHY_RADIO,
> - bcm43xx_read16(dev, BCM43xx_MMIO_PHY_RADIO)
> - & 0x7FFF);
> + if (has_loopback_gain(phy)) {
> + bcm43xx_phy_stackrestore(stack, BCM43xx_PHY_LO_MASK);
> + bcm43xx_phy_stackrestore(stack, BCM43xx_PHY_LO_CTL);
> + }

Makes sense. Thanks.

> }
> if (i > 15)
> ret = radio78;
>
>

--
Greetings Michael.

2007-03-21 15:33:44

by Michael Büsch

Subject: Re: [PATCH] bcm43xx-mac80211: Change radio_init2050 to match specs

On Wednesday 21 March 2007 16:29, Larry Finger wrote:
> Michael Buesch wrote:
> > On Tuesday 20 March 2007 21:49, Larry Finger wrote:
> >> The code in the mac80211 version of radio_init2050 differs from the specs
> >> in two places.
> >>
> >> Signed-off-by: Larry Finger <[email protected]>
> >> ---
> >>
> >> Index: wireless-mb/drivers/net/wireless/mac80211/bcm43xx/bcm43xx_phy.c
> >> ===================================================================
> >> --- wireless-mb.orig/drivers/net/wireless/mac80211/bcm43xx/bcm43xx_phy.c
> >> +++ wireless-mb/drivers/net/wireless/mac80211/bcm43xx/bcm43xx_phy.c
> >> @@ -3444,9 +3444,9 @@ static u16 radio2050_rfover_val(struct b
> >> if (phy_register == BCM43xx_PHY_RFOVER) {
> >> return 0x9B3;
> >> } else if (phy_register == BCM43xx_PHY_RFOVERVAL) {
> >> - extlna |= (i << 8);
> >> if (extlna)
> >> extlna |= 0x8000;
> >> + extlna |= (i << 8);
> >
> > What's the difference?
>
> The nonzero test for extlna should be made on the table value, not the table value or'd with i << 8.

Oh, I see. Some brain screwage on my side :)

--
Greetings Michael.

2007-03-21 15:30:09

by Larry Finger

Subject: Re: [PATCH] bcm43xx-mac80211: Change radio_init2050 to match specs

Michael Buesch wrote:
> On Tuesday 20 March 2007 21:49, Larry Finger wrote:
>> The code in the mac80211 version of radio_init2050 differs from the specs
>> in two places.
>>
>> Signed-off-by: Larry Finger <[email protected]>
>> ---
>>
>> Index: wireless-mb/drivers/net/wireless/mac80211/bcm43xx/bcm43xx_phy.c
>> ===================================================================
>> --- wireless-mb.orig/drivers/net/wireless/mac80211/bcm43xx/bcm43xx_phy.c
>> +++ wireless-mb/drivers/net/wireless/mac80211/bcm43xx/bcm43xx_phy.c
>> @@ -3444,9 +3444,9 @@ static u16 radio2050_rfover_val(struct b
>> if (phy_register == BCM43xx_PHY_RFOVER) {
>> return 0x9B3;
>> } else if (phy_register == BCM43xx_PHY_RFOVERVAL) {
>> - extlna |= (i << 8);
>> if (extlna)
>> extlna |= 0x8000;
>> + extlna |= (i << 8);
>
> What's the difference?

The nonzero test for extlna should be made on the table value, not the table value or'd with i << 8.

Larry

2007-03-22 00:44:19

by Joseph Jezak

Subject: Re: [PATCH] bcm43xx-mac80211: Change radio_init2050 to match specs

>>From my understanding current code is correct.
> But I see that one could interpret the spec in your way as well.
> Joseph?

If the External LNA value, as read from the table, is non-zero, OR
that value with 0x8000 (which I think is the "enable the External
LNA flag).

I've updated the specs to reflect this.

-Joe