I recently found some places where the G-PHY initialization
specifications were wrong. I updated the relevant parts of the V3
specifications at http://bcm-specs.sipsolutions.net - the pages for
GPHY, B6PHY and B5PHY were changed.
When I prepared a patch containing these changes, the speed of the
BCM4306/2 for OFDM rates more than doubled. As an example, when
iwconfig is used to set the rate at 36M, the TX rate increased from
6.8 to 15.8 Mb/s.
Unfortunately, direct submission of my patch would violate the
clean-room rules. Michael Buesch has other things that are more
important. As a result, we need someone to look at the changes on the
web page and prepare a patch for submission. I will be happy to test
your submission.
Larry
[email protected] wrote:
> I haven't tried a build yet, but please let me know if I'm on the right
> track.
>
> E
> # diff -uN /tmp/phy.c drivers/net/wireless/b43legacy/phy.c
> --- /tmp/phy.c 2008-09-06 15:13:33.000000000 -0700
> +++ drivers/net/wireless/b43legacy/phy.c 2008-09-06
> 15:54:03.000000000 -0700
> @@ -1010,7 +1010,7 @@
> b43legacy_phy_initb5(dev);
> else
> b43legacy_phy_initb6(dev);
> - if (phy->rev >= 2 || phy->gmode)
> + if (phy->rev >= 2 && phy->gmode)
> b43legacy_phy_inita(dev);
>
> if (phy->rev >= 2) {
> @@ -1021,21 +1021,26 @@
> b43legacy_phy_write(dev, 0x0811, 0x0000);
> b43legacy_phy_write(dev, 0x0015, 0x00C0);
> }
> - if (phy->rev > 5) {
> + if (phy->rev >= 3) {
AFAIK, this change is an error in the specs. I have since changed it.
Sorry I didn't catch it earlier.
Otherwise, this patch seems to be correct. All you need now are the
fixes for b43legacy_phy_initb5() and b43legacy_phy_initb6().
Larry
Larry Finger wrote:
> [email protected] wrote:
>> I haven't tried a build yet, but please let me know if I'm on the
>> right track.
>>
>> E
>> # diff -uN /tmp/phy.c drivers/net/wireless/b43legacy/phy.c
>> --- /tmp/phy.c 2008-09-06 15:13:33.000000000 -0700
>> +++ drivers/net/wireless/b43legacy/phy.c 2008-09-06
>> 15:54:03.000000000 -0700
>> @@ -1010,7 +1010,7 @@
>> b43legacy_phy_initb5(dev);
>> else
>> b43legacy_phy_initb6(dev);
>> - if (phy->rev >= 2 || phy->gmode)
>> + if (phy->rev >= 2 && phy->gmode)
>> b43legacy_phy_inita(dev);
>>
>> if (phy->rev >= 2) {
>> @@ -1021,21 +1021,26 @@
>> b43legacy_phy_write(dev, 0x0811, 0x0000);
>> b43legacy_phy_write(dev, 0x0015, 0x00C0);
>> }
>> - if (phy->rev > 5) {
>> + if (phy->rev >= 3) {
>
> AFAIK, this change is an error in the specs. I have since changed it.
> Sorry I didn't catch it earlier.
>
> Otherwise, this patch seems to be correct. All you need now are the
> fixes for b43legacy_phy_initb5() and b43legacy_phy_initb6().
>
> Larry
Ok, I've re-looked at the specs and made the appropriate corrections.
I've also gone through all of the PHY specs and found one other
correction. It's enclosed below for review.
Where do I go to find the stuff for ...initb5() and ...initb6()?
Thanks
E
# diff -uN /tmp/phy.c drivers/net/wireless/b43legacy/phy.c
--- /tmp/phy.c 2008-09-06 15:13:33.000000000 -0700
+++ drivers/net/wireless/b43legacy/phy.c 2008-09-06
16:42:40.000000000 -0700
@@ -1010,7 +1010,7 @@
b43legacy_phy_initb5(dev);
else
b43legacy_phy_initb6(dev);
- if (phy->rev >= 2 || phy->gmode)
+ if (phy->rev >= 2 && phy->gmode)
b43legacy_phy_inita(dev);
if (phy->rev >= 2) {
@@ -1025,17 +1025,22 @@
b43legacy_phy_write(dev, 0x0811, 0x0400);
b43legacy_phy_write(dev, 0x0015, 0x00C0);
}
- if (phy->rev >= 2 || phy->gmode) {
+ if (phy->gmode) {
tmp = b43legacy_phy_read(dev, 0x0400) & 0xFF;
- if (tmp == 3 || tmp == 5) {
+ if (tmp == 3) {
+ b43legacy_phy_write(dev, 0x04C2, 0x1816);
+ b43legacy_phy_write(dev, 0x04C3, 0x8606);
+ }
+ if (tmp == 4 || tmp == 5) {
b43legacy_phy_write(dev, 0x04C2, 0x1816);
b43legacy_phy_write(dev, 0x04C3, 0x8006);
- if (tmp == 5)
- b43legacy_phy_write(dev, 0x04CC,
- (b43legacy_phy_read(dev,
- 0x04CC) & 0x00FF) |
- 0x1F00);
+ b43legacy_phy_write(dev, 0x04CC,
+ (b43legacy_phy_read(dev,
+ 0x04CC) & 0x00FF) |
+ 0x1F00);
}
+ }
+ if (phy-->rev >= 2)
b43legacy_phy_write(dev, 0x047E, 0x0078);
}
if (phy->radio_rev == 8) {
@@ -1092,7 +1097,7 @@
*/
b43legacy_nrssi_hw_update(dev, 0xFFFF);
b43legacy_calc_nrssi_threshold(dev);
- } else if (phy->gmode || phy->rev >= 2) {
+ } else if (phy->gmode) {
if (phy->nrssi[0] == -1000) {
B43legacy_WARN_ON(phy->nrssi[1] != -1000);
b43legacy_calc_nrssi_slope(dev);
[email protected] wrote:
>
> Try #5:
> # diff -uN /tmp/phy.c drivers/net/wireless/b43legacy/phy.c
> --- /tmp/phy.c 2008-09-06 15:13:33.000000000 -0700
This one is pretty close, but I think you missed a change at line 16a
of B6PHY.
One other thing - use the -uNp switch for diff.
Larry
Larry Finger wrote:
> [email protected] wrote:
>> I haven't tried a build yet, but please let me know if I'm on the
>> right track.
>>
>> E
>> # diff -uN /tmp/phy.c drivers/net/wireless/b43legacy/phy.c
>> --- /tmp/phy.c 2008-09-06 15:13:33.000000000 -0700
>> +++ drivers/net/wireless/b43legacy/phy.c 2008-09-06
>> 15:54:03.000000000 -0700
>> @@ -1010,7 +1010,7 @@
>> b43legacy_phy_initb5(dev);
>> else
>> b43legacy_phy_initb6(dev);
>> - if (phy->rev >= 2 || phy->gmode)
>> + if (phy->rev >= 2 && phy->gmode)
>> b43legacy_phy_inita(dev);
>>
>> if (phy->rev >= 2) {
>> @@ -1021,21 +1021,26 @@
>> b43legacy_phy_write(dev, 0x0811, 0x0000);
>> b43legacy_phy_write(dev, 0x0015, 0x00C0);
>> }
>> - if (phy->rev > 5) {
>> + if (phy->rev >= 3) {
>
> AFAIK, this change is an error in the specs. I have since changed it.
> Sorry I didn't catch it earlier.
>
> Otherwise, this patch seems to be correct. All you need now are the
> fixes for b43legacy_phy_initb5() and b43legacy_phy_initb6().
>
> Larry
Ok, this one applies changes to the PHY, B5PHY, and B6PHY changes.
E
# diff -uN /tmp/phy.c drivers/net/wireless/b43legacy/phy.c
--- /tmp/phy.c 2008-09-06 15:13:33.000000000 -0700
+++ drivers/net/wireless/b43legacy/phy.c 2008-09-06
17:13:31.000000000 -0700
@@ -595,12 +595,14 @@
0x0035) & 0xFFC0) | 0x0064);
b43legacy_phy_write(dev, 0x005D, (b43legacy_phy_read(dev,
0x005D) & 0xFF80) | 0x000A);
+ b43legacy_phy_write(dev, 0x5B, 0x0000);
+ b43legacy_phy_write(dev, 0x5C, 0x0000);
}
if (dev->bad_frames_preempt)
b43legacy_phy_write(dev, B43legacy_PHY_RADIO_BITFIELD,
b43legacy_phy_read(dev,
- B43legacy_PHY_RADIO_BITFIELD) | (1 << 11));
+ B43legacy_PHY_RADIO_BITFIELD) | (1 << 12));
if (phy->analog == 1) {
b43legacy_phy_write(dev, 0x0026, 0xCE00);
@@ -771,7 +773,7 @@
b43legacy_phy_write(dev, 0x002A, 0x8AC0);
b43legacy_phy_write(dev, 0x0038, 0x0668);
b43legacy_radio_set_txpower_bg(dev, 0xFFFF, 0xFFFF, 0xFFFF);
- if (phy->radio_rev <= 5)
+ if (phy->radio_rev == 4 || phy->radio_rev == 5)
b43legacy_phy_write(dev, 0x005D, (b43legacy_phy_read(dev,
0x005D) & 0xFF80) | 0x0003);
if (phy->radio_rev <= 2)
@@ -1010,7 +1012,7 @@
b43legacy_phy_initb5(dev);
else
b43legacy_phy_initb6(dev);
- if (phy->rev >= 2 || phy->gmode)
+ if (phy->rev >= 2 && phy->gmode)
b43legacy_phy_inita(dev);
if (phy->rev >= 2) {
@@ -1025,17 +1027,22 @@
b43legacy_phy_write(dev, 0x0811, 0x0400);
b43legacy_phy_write(dev, 0x0015, 0x00C0);
}
- if (phy->rev >= 2 || phy->gmode) {
+ if (phy->gmode) {
tmp = b43legacy_phy_read(dev, 0x0400) & 0xFF;
- if (tmp == 3 || tmp == 5) {
+ if (tmp == 3) {
+ b43legacy_phy_write(dev, 0x04C2, 0x1816);
+ b43legacy_phy_write(dev, 0x04C3, 0x8606);
+ }
+ if (tmp == 4 || tmp == 5) {
b43legacy_phy_write(dev, 0x04C2, 0x1816);
b43legacy_phy_write(dev, 0x04C3, 0x8006);
- if (tmp == 5)
- b43legacy_phy_write(dev, 0x04CC,
- (b43legacy_phy_read(dev,
- 0x04CC) & 0x00FF) |
- 0x1F00);
+ b43legacy_phy_write(dev, 0x04CC,
+ (b43legacy_phy_read(dev,
+ 0x04CC) & 0x00FF) |
+ 0x1F00);
}
+ }
+ if (phy-->rev >= 2)
b43legacy_phy_write(dev, 0x047E, 0x0078);
}
if (phy->radio_rev == 8) {
@@ -1092,7 +1099,7 @@
*/
b43legacy_nrssi_hw_update(dev, 0xFFFF);
b43legacy_calc_nrssi_threshold(dev);
- } else if (phy->gmode || phy->rev >= 2) {
+ } else if (phy->gmode) {
if (phy->nrssi[0] == -1000) {
B43legacy_WARN_ON(phy->nrssi[1] != -1000);
b43legacy_calc_nrssi_slope(dev);
Larry Finger wrote:
> [email protected] wrote:
>>
>> Try #5:
>> # diff -uN /tmp/phy.c drivers/net/wireless/b43legacy/phy.c
>> --- /tmp/phy.c 2008-09-06 15:13:33.000000000 -0700
>
> This one is pretty close, but I think you missed a change at line 16a
> of B6PHY.
>
> One other thing - use the -uNp switch for diff.
>
> Larry
>
T6:
# diff -uNp /tmp/phy.c drivers/net/wireless/b43legacy/phy.c
--- /tmp/phy.c 2008-09-06 15:13:33.000000000 -0700
+++ drivers/net/wireless/b43legacy/phy.c 2008-09-06
17:40:20.000000000 -0700
@@ -595,12 +595,14 @@ static void b43legacy_phy_initb5(struct
0x0035) & 0xFFC0) | 0x0064);
b43legacy_phy_write(dev, 0x005D, (b43legacy_phy_read(dev,
0x005D) & 0xFF80) | 0x000A);
+ b43legacy_phy_write(dev, 0x5B, 0x0000);
+ b43legacy_phy_write(dev, 0x5C, 0x0000);
}
if (dev->bad_frames_preempt)
b43legacy_phy_write(dev, B43legacy_PHY_RADIO_BITFIELD,
b43legacy_phy_read(dev,
- B43legacy_PHY_RADIO_BITFIELD) | (1 << 11));
+ B43legacy_PHY_RADIO_BITFIELD) | (1 << 12));
if (phy->analog == 1) {
b43legacy_phy_write(dev, 0x0026, 0xCE00);
@@ -753,7 +755,7 @@ static void b43legacy_phy_initb6(struct
b43legacy_radio_write16(dev, 0x0050, 0x0020);
}
if (phy->radio_rev <= 2) {
- b43legacy_radio_write16(dev, 0x007C, 0x0020);
+ b43legacy_radio_write16(dev, 0x0050, 0x0020);
b43legacy_radio_write16(dev, 0x005A, 0x0070);
b43legacy_radio_write16(dev, 0x005B, 0x007B);
b43legacy_radio_write16(dev, 0x005C, 0x00B0);
@@ -771,7 +773,7 @@ static void b43legacy_phy_initb6(struct
b43legacy_phy_write(dev, 0x002A, 0x8AC0);
b43legacy_phy_write(dev, 0x0038, 0x0668);
b43legacy_radio_set_txpower_bg(dev, 0xFFFF, 0xFFFF, 0xFFFF);
- if (phy->radio_rev <= 5)
+ if (phy->radio_rev == 4 || phy->radio_rev == 5)
b43legacy_phy_write(dev, 0x005D, (b43legacy_phy_read(dev,
0x005D) & 0xFF80) | 0x0003);
if (phy->radio_rev <= 2)
@@ -1010,7 +1012,7 @@ static void b43legacy_phy_initg(struct b
b43legacy_phy_initb5(dev);
else
b43legacy_phy_initb6(dev);
- if (phy->rev >= 2 || phy->gmode)
+ if (phy->rev >= 2 && phy->gmode)
b43legacy_phy_inita(dev);
if (phy->rev >= 2) {
@@ -1025,17 +1027,22 @@ static void b43legacy_phy_initg(struct b
b43legacy_phy_write(dev, 0x0811, 0x0400);
b43legacy_phy_write(dev, 0x0015, 0x00C0);
}
- if (phy->rev >= 2 || phy->gmode) {
+ if (phy->gmode) {
tmp = b43legacy_phy_read(dev, 0x0400) & 0xFF;
- if (tmp == 3 || tmp == 5) {
+ if (tmp == 3) {
+ b43legacy_phy_write(dev, 0x04C2, 0x1816);
+ b43legacy_phy_write(dev, 0x04C3, 0x8606);
+ }
+ if (tmp == 4 || tmp == 5) {
b43legacy_phy_write(dev, 0x04C2, 0x1816);
b43legacy_phy_write(dev, 0x04C3, 0x8006);
- if (tmp == 5)
- b43legacy_phy_write(dev, 0x04CC,
- (b43legacy_phy_read(dev,
- 0x04CC) & 0x00FF) |
- 0x1F00);
+ b43legacy_phy_write(dev, 0x04CC,
+ (b43legacy_phy_read(dev,
+ 0x04CC) & 0x00FF) |
+ 0x1F00);
}
+ }
+ if (phy->rev >= 2)
b43legacy_phy_write(dev, 0x047E, 0x0078);
}
if (phy->radio_rev == 8) {
@@ -1078,7 +1085,7 @@ static void b43legacy_phy_initg(struct b
else
b43legacy_phy_write(dev, 0x002F, 0x0202);
}
- if (phy->gmode || phy->rev >= 2) {
+ if (phy->gmode) {
b43legacy_phy_lo_adjust(dev, 0);
b43legacy_phy_write(dev, 0x080F, 0x8078);
Larry Finger wrote:
> [email protected] wrote:
>> Is this close?
>> E
>>
>
> It is close, but I think you are working on b43. My changes are for
> b43legacy and all changes will be in drivers/net/wireless/b43legacy/phy.c
>
> Larry
Ok, here's try #2.
E
/home/2.6.27/rc4-wl/drivers/net/wireless/b43legacy# diff -uN /tmp/phy.c
phy.c
--- /tmp/phy.c 2008-09-05 21:56:20.000000000 -0700
+++ phy.c 2008-09-05 22:03:28.000000000 -0700
@@ -1010,7 +1010,7 @@
b43legacy_phy_initb5(dev);
else
b43legacy_phy_initb6(dev);
- if (phy->rev >= 2 || phy->gmode)
+ if (phy->rev >= 2 && phy->gmode)
b43legacy_phy_inita(dev);
if (phy->rev >= 2) {
@@ -1027,15 +1027,17 @@
}
if (phy->rev >= 2 || phy->gmode) {
tmp = b43legacy_phy_read(dev, 0x0400) & 0xFF;
- if (tmp == 3 || tmp == 5) {
+ if (tmp == 4 || tmp == 5) {
b43legacy_phy_write(dev, 0x04C2, 0x1816);
- b43legacy_phy_write(dev, 0x04C3, 0x8006);
+ b43legacy_phy_write(dev, 0x04C3, 0x8606);
if (tmp == 5)
b43legacy_phy_write(dev, 0x04CC,
(b43legacy_phy_read(dev,
0x04CC) & 0x00FF) |
0x1F00);
}
+ }
+ if (phy->rev >= 2)
b43legacy_phy_write(dev, 0x047E, 0x0078);
}
if (phy->radio_rev == 8) {
Larry Finger wrote:
> [email protected] wrote:
>>
>>
>>
>> Ok, here's try #2.
>> E
...
>
> This hunk does not match the specs. In addition, I think there are too
> many right-hand curly braces for it to compile.
>
> Larry
>
I haven't tried a build yet, but please let me know if I'm on the right
track.
E
# diff -uN /tmp/phy.c drivers/net/wireless/b43legacy/phy.c
--- /tmp/phy.c 2008-09-06 15:13:33.000000000 -0700
+++ drivers/net/wireless/b43legacy/phy.c 2008-09-06
15:54:03.000000000 -0700
@@ -1010,7 +1010,7 @@
b43legacy_phy_initb5(dev);
else
b43legacy_phy_initb6(dev);
- if (phy->rev >= 2 || phy->gmode)
+ if (phy->rev >= 2 && phy->gmode)
b43legacy_phy_inita(dev);
if (phy->rev >= 2) {
@@ -1021,21 +1021,26 @@
b43legacy_phy_write(dev, 0x0811, 0x0000);
b43legacy_phy_write(dev, 0x0015, 0x00C0);
}
- if (phy->rev > 5) {
+ if (phy->rev >= 3) {
b43legacy_phy_write(dev, 0x0811, 0x0400);
b43legacy_phy_write(dev, 0x0015, 0x00C0);
}
- if (phy->rev >= 2 || phy->gmode) {
+ if (phy->gmode) {
tmp = b43legacy_phy_read(dev, 0x0400) & 0xFF;
- if (tmp == 3 || tmp == 5) {
+ if (tmp == 3) {
+ b43legacy_phy_write(dev, 0x04C2, 0x1816);
+ b43legacy_phy_write(dev, 0x04C3, 0x8606);
+ }
+ if (tmp == 4 || tmp == 5) {
b43legacy_phy_write(dev, 0x04C2, 0x1816);
b43legacy_phy_write(dev, 0x04C3, 0x8006);
- if (tmp == 5)
- b43legacy_phy_write(dev, 0x04CC,
- (b43legacy_phy_read(dev,
- 0x04CC) & 0x00FF) |
- 0x1F00);
+ b43legacy_phy_write(dev, 0x04CC,
+ (b43legacy_phy_read(dev,
+ 0x04CC) & 0x00FF) |
+ 0x1F00);
}
+ }
+ if (phy-->rev >= 2)
b43legacy_phy_write(dev, 0x047E, 0x0078);
}
if (phy->radio_rev == 8) {
On Saturday 06 September 2008 07:04:36 [email protected] wrote:
> + }
> + if (phy->rev >= 2)
> b43legacy_phy_write(dev, 0x047E, 0x0078);
> }
Does this even compile?
--
Greetings Michael.
Larry Finger wrote:
> [email protected] wrote:
>>
>>
>> Larry Finger wrote:
>>> [email protected] wrote:
>>>> I haven't tried a build yet, but please let me know if I'm on the
>>>> right track.
>>>>
>>>> E
>>>> # diff -uN /tmp/phy.c drivers/net/wireless/b43legacy/phy.c
>>>> --- /tmp/phy.c 2008-09-06 15:13:33.000000000 -0700
>>>> +++ drivers/net/wireless/b43legacy/phy.c 2008-09-06
>>>> 15:54:03.000000000 -0700
>>>> @@ -1010,7 +1010,7 @@
>>>> b43legacy_phy_initb5(dev);
>>>> else
>>>> b43legacy_phy_initb6(dev);
>>>> - if (phy->rev >= 2 || phy->gmode)
>>>> + if (phy->rev >= 2 && phy->gmode)
>>>> b43legacy_phy_inita(dev);
>>>>
>>>> if (phy->rev >= 2) {
>>>> @@ -1021,21 +1021,26 @@
>>>> b43legacy_phy_write(dev, 0x0811, 0x0000);
>>>> b43legacy_phy_write(dev, 0x0015, 0x00C0);
>>>> }
>>>> - if (phy->rev > 5) {
>>>> + if (phy->rev >= 3) {
>>>
>>> AFAIK, this change is an error in the specs. I have since changed
>>> it. Sorry I didn't catch it earlier.
>>>
>>> Otherwise, this patch seems to be correct. All you need now are the
>>> fixes for b43legacy_phy_initb5() and b43legacy_phy_initb6().
>>>
>>> Larry
>>
>> Ok, I've re-looked at the specs and made the appropriate
>> corrections. I've also gone through all of the PHY specs and found
>> one other correction. It's enclosed below for review.
>>
>> Where do I go to find the stuff for ...initb5() and ...initb6()?
>
> That one was also an error in the specs - fixed now.
>
> On the V3 specifications site, click on the RecentChanges button and
> select B5PHY and B6PHY to get the specs for the other routines. I
> rechecked the specs, and all agree with my current (revised) routines.
>
>
> Larry
Try #5:
# diff -uN /tmp/phy.c drivers/net/wireless/b43legacy/phy.c
--- /tmp/phy.c 2008-09-06 15:13:33.000000000 -0700
+++ drivers/net/wireless/b43legacy/phy.c 2008-09-06
17:24:29.000000000 -0700
@@ -595,12 +595,14 @@
0x0035) & 0xFFC0) | 0x0064);
b43legacy_phy_write(dev, 0x005D, (b43legacy_phy_read(dev,
0x005D) & 0xFF80) | 0x000A);
+ b43legacy_phy_write(dev, 0x5B, 0x0000);
+ b43legacy_phy_write(dev, 0x5C, 0x0000);
}
if (dev->bad_frames_preempt)
b43legacy_phy_write(dev, B43legacy_PHY_RADIO_BITFIELD,
b43legacy_phy_read(dev,
- B43legacy_PHY_RADIO_BITFIELD) | (1 << 11));
+ B43legacy_PHY_RADIO_BITFIELD) | (1 << 12));
if (phy->analog == 1) {
b43legacy_phy_write(dev, 0x0026, 0xCE00);
@@ -771,7 +773,7 @@
b43legacy_phy_write(dev, 0x002A, 0x8AC0);
b43legacy_phy_write(dev, 0x0038, 0x0668);
b43legacy_radio_set_txpower_bg(dev, 0xFFFF, 0xFFFF, 0xFFFF);
- if (phy->radio_rev <= 5)
+ if (phy->radio_rev == 4 || phy->radio_rev == 5)
b43legacy_phy_write(dev, 0x005D, (b43legacy_phy_read(dev,
0x005D) & 0xFF80) | 0x0003);
if (phy->radio_rev <= 2)
@@ -1010,7 +1012,7 @@
b43legacy_phy_initb5(dev);
else
b43legacy_phy_initb6(dev);
- if (phy->rev >= 2 || phy->gmode)
+ if (phy->rev >= 2 && phy->gmode)
b43legacy_phy_inita(dev);
if (phy->rev >= 2) {
@@ -1025,17 +1027,22 @@
b43legacy_phy_write(dev, 0x0811, 0x0400);
b43legacy_phy_write(dev, 0x0015, 0x00C0);
}
- if (phy->rev >= 2 || phy->gmode) {
+ if (phy->gmode) {
tmp = b43legacy_phy_read(dev, 0x0400) & 0xFF;
- if (tmp == 3 || tmp == 5) {
+ if (tmp == 3) {
+ b43legacy_phy_write(dev, 0x04C2, 0x1816);
+ b43legacy_phy_write(dev, 0x04C3, 0x8606);
+ }
+ if (tmp == 4 || tmp == 5) {
b43legacy_phy_write(dev, 0x04C2, 0x1816);
b43legacy_phy_write(dev, 0x04C3, 0x8006);
- if (tmp == 5)
- b43legacy_phy_write(dev, 0x04CC,
- (b43legacy_phy_read(dev,
- 0x04CC) & 0x00FF) |
- 0x1F00);
+ b43legacy_phy_write(dev, 0x04CC,
+ (b43legacy_phy_read(dev,
+ 0x04CC) & 0x00FF) |
+ 0x1F00);
}
+ }
+ if (phy->rev >= 2)
b43legacy_phy_write(dev, 0x047E, 0x0078);
}
if (phy->radio_rev == 8) {
@@ -1078,7 +1085,7 @@
else
b43legacy_phy_write(dev, 0x002F, 0x0202);
}
- if (phy->gmode || phy->rev >= 2) {
+ if (phy->gmode) {
b43legacy_phy_lo_adjust(dev, 0);
b43legacy_phy_write(dev, 0x080F, 0x8078);
}
[email protected] wrote:
>
>
> Larry Finger wrote:
>> [email protected] wrote:
>>> I haven't tried a build yet, but please let me know if I'm on the
>>> right track.
>>>
>>> E
>>> # diff -uN /tmp/phy.c drivers/net/wireless/b43legacy/phy.c
>>> --- /tmp/phy.c 2008-09-06 15:13:33.000000000 -0700
>>> +++ drivers/net/wireless/b43legacy/phy.c 2008-09-06
>>> 15:54:03.000000000 -0700
>>> @@ -1010,7 +1010,7 @@
>>> b43legacy_phy_initb5(dev);
>>> else
>>> b43legacy_phy_initb6(dev);
>>> - if (phy->rev >= 2 || phy->gmode)
>>> + if (phy->rev >= 2 && phy->gmode)
>>> b43legacy_phy_inita(dev);
>>>
>>> if (phy->rev >= 2) {
>>> @@ -1021,21 +1021,26 @@
>>> b43legacy_phy_write(dev, 0x0811, 0x0000);
>>> b43legacy_phy_write(dev, 0x0015, 0x00C0);
>>> }
>>> - if (phy->rev > 5) {
>>> + if (phy->rev >= 3) {
>>
>> AFAIK, this change is an error in the specs. I have since changed it.
>> Sorry I didn't catch it earlier.
>>
>> Otherwise, this patch seems to be correct. All you need now are the
>> fixes for b43legacy_phy_initb5() and b43legacy_phy_initb6().
>>
>> Larry
>
> Ok, I've re-looked at the specs and made the appropriate corrections.
> I've also gone through all of the PHY specs and found one other
> correction. It's enclosed below for review.
>
> Where do I go to find the stuff for ...initb5() and ...initb6()?
That one was also an error in the specs - fixed now.
On the V3 specifications site, click on the RecentChanges button and
select B5PHY and B6PHY to get the specs for the other routines. I
rechecked the specs, and all agree with my current (revised) routines.
Larry
On Saturday 06 September 2008 20:07:15 [email protected] wrote:
> correct these issues. I did see several other ways in which the code
> does not match the specs. Should that be documented in the code or
> should the code be conformed to the specs even if that ends up breaking
> the driver?
You're really considering breaking the driver just to conform the specs?
> Without getting into specifics it's cases where the specs will say "When
> something=value" but the code says "when variable >=(value -1)".
Please show us the code, so we can give you advise.
--
Greetings Michael.
Is this close?
E
--- /tmp/phy_g.c 2008-09-05 21:06:57.000000000 -0700
+++ phy_g.c 2008-09-05 21:36:18.000000000 -0700
@@ -2198,7 +2198,7 @@
else
b43_phy_initb6(dev);
- if (phy->rev >= 2 || phy->gmode)
+ if (phy->rev >= 2 && phy->gmode)
b43_phy_inita(dev);
if (phy->rev >= 2) {
@@ -2216,9 +2216,9 @@
if (phy->gmode || phy->rev >= 2) {
tmp = b43_phy_read(dev, B43_PHY_VERSION_OFDM);
tmp &= B43_PHYVER_VERSION;
- if (tmp == 3 || tmp == 5) {
+ if (tmp == 4 || tmp == 5) {
b43_phy_write(dev, B43_PHY_OFDM(0xC2), 0x1816);
- b43_phy_write(dev, B43_PHY_OFDM(0xC3), 0x8006);
+ b43_phy_write(dev, B43_PHY_OFDM(0xC3), 0x8606);
}
if (tmp == 5) {
b43_phy_write(dev, B43_PHY_OFDM(0xCC),
@@ -2226,7 +2226,7 @@
& 0x00FF) | 0x1F00);
}
}
- if ((phy->rev <= 2 && phy->gmode) || phy->rev >= 2)
+ if ((phy->rev >=2)
b43_phy_write(dev, B43_PHY_OFDM(0x7E), 0x78);
if (phy->radio_rev == 8) {
b43_phy_write(dev, B43_PHY_EXTG(0x01),
Larry Finger wrote:
> I recently found some places where the G-PHY initialization
> specifications were wrong. I updated the relevant parts of the V3
> specifications at http://bcm-specs.sipsolutions.net - the pages for
> GPHY, B6PHY and B5PHY were changed.
>
> When I prepared a patch containing these changes, the speed of the
> BCM4306/2 for OFDM rates more than doubled. As an example, when
> iwconfig is used to set the rate at 36M, the TX rate increased from
> 6.8 to 15.8 Mb/s.
>
> Unfortunately, direct submission of my patch would violate the
> clean-room rules. Michael Buesch has other things that are more
> important. As a result, we need someone to look at the changes on the
> web page and prepare a patch for submission. I will be happy to test
> your submission.
>
> Larry
> _______________________________________________
> Bcm43xx-dev mailing list
> [email protected]
> https://lists.berlios.de/mailman/listinfo/bcm43xx-dev
>
Larry Finger wrote:
> [email protected] wrote:
>>
>>
>>
>> Ok, here's try #2.
>> E
>> /home/2.6.27/rc4-wl/drivers/net/wireless/b43legacy# diff -uN
>> /tmp/phy.c phy.c
>> --- /tmp/phy.c 2008-09-05 21:56:20.000000000 -0700
>> +++ phy.c 2008-09-05 22:03:28.000000000 -0700
>
> For kernel patches, you need to be working in the base directory of
> the kernel sources. For your tree, that would be in
> /home/2.6.27/rc4-wl. That way the patches will apply with the
> effective command of 'patch -p1 < patch_file'. For kernel patches, I
> use quilt so that patches are easy to apply and remove.
>
>> @@ -1010,7 +1010,7 @@
>> b43legacy_phy_initb5(dev);
>> else
>> b43legacy_phy_initb6(dev);
>> - if (phy->rev >= 2 || phy->gmode)
>> + if (phy->rev >= 2 && phy->gmode)
>> b43legacy_phy_inita(dev);
>>
>> if (phy->rev >= 2) {
>
> The above hunk is correct.
>
>> @@ -1027,15 +1027,17 @@
>> }
>> if (phy->rev >= 2 || phy->gmode) {
>
> This does not match step 7 of the specs. It was not changed recently,
> but the code did not match what was on the web site. No, I don't know
> why.
>
>> tmp = b43legacy_phy_read(dev, 0x0400) & 0xFF;
>> - if (tmp == 3 || tmp == 5) {
>> + if (tmp == 4 || tmp == 5) {
>> b43legacy_phy_write(dev, 0x04C2, 0x1816);
>> - b43legacy_phy_write(dev, 0x04C3, 0x8006);
>> + b43legacy_phy_write(dev, 0x04C3, 0x8606);
>> if (tmp == 5)
>> b43legacy_phy_write(dev, 0x04CC,
>> (b43legacy_phy_read(dev,
>> 0x04CC) & 0x00FF) |
>> 0x1F00);
>> }
>> + }
>> + if (phy->rev >= 2)
>> b43legacy_phy_write(dev, 0x047E, 0x0078);
>> }
>> if (phy->radio_rev == 8) {
>
> This hunk does not match the specs. In addition, I think there are too
> many right-hand curly braces for it to compile.
>
> Larry
>
I've been sitting on a git clone that refuses to proceed faster than 6
KiB/s (it's a problem here in Melbourne). Should it complete I will
correct these issues. I did see several other ways in which the code
does not match the specs. Should that be documented in the code or
should the code be conformed to the specs even if that ends up breaking
the driver?
Without getting into specifics it's cases where the specs will say "When
something=value" but the code says "when variable >=(value -1)".
Ehud
[email protected] wrote:
>
>
>
> Ok, here's try #2.
> E
> /home/2.6.27/rc4-wl/drivers/net/wireless/b43legacy# diff -uN /tmp/phy.c
> phy.c
> --- /tmp/phy.c 2008-09-05 21:56:20.000000000 -0700
> +++ phy.c 2008-09-05 22:03:28.000000000 -0700
For kernel patches, you need to be working in the base directory of
the kernel sources. For your tree, that would be in
/home/2.6.27/rc4-wl. That way the patches will apply with the
effective command of 'patch -p1 < patch_file'. For kernel patches, I
use quilt so that patches are easy to apply and remove.
> @@ -1010,7 +1010,7 @@
> b43legacy_phy_initb5(dev);
> else
> b43legacy_phy_initb6(dev);
> - if (phy->rev >= 2 || phy->gmode)
> + if (phy->rev >= 2 && phy->gmode)
> b43legacy_phy_inita(dev);
>
> if (phy->rev >= 2) {
The above hunk is correct.
> @@ -1027,15 +1027,17 @@
> }
> if (phy->rev >= 2 || phy->gmode) {
This does not match step 7 of the specs. It was not changed recently,
but the code did not match what was on the web site. No, I don't know why.
> tmp = b43legacy_phy_read(dev, 0x0400) & 0xFF;
> - if (tmp == 3 || tmp == 5) {
> + if (tmp == 4 || tmp == 5) {
> b43legacy_phy_write(dev, 0x04C2, 0x1816);
> - b43legacy_phy_write(dev, 0x04C3, 0x8006);
> + b43legacy_phy_write(dev, 0x04C3, 0x8606);
> if (tmp == 5)
> b43legacy_phy_write(dev, 0x04CC,
> (b43legacy_phy_read(dev,
> 0x04CC) & 0x00FF) |
> 0x1F00);
> }
> + }
> + if (phy->rev >= 2)
> b43legacy_phy_write(dev, 0x047E, 0x0078);
> }
> if (phy->radio_rev == 8) {
This hunk does not match the specs. In addition, I think there are too
many right-hand curly braces for it to compile.
Larry
[email protected] wrote:
> I've been sitting on a git clone that refuses to proceed faster than 6
> KiB/s (it's a problem here in Melbourne). Should it complete I will
> correct these issues. I did see several other ways in which the code
> does not match the specs. Should that be documented in the code or
> should the code be conformed to the specs even if that ends up breaking
> the driver?
>
> Without getting into specifics it's cases where the specs will say "When
> something=value" but the code says "when variable >=(value -1)".
As Michael said, we need to see the specifics. Those specs have not
been updated for a long time, and there are certainly errors in them.
The only thing I can say with certainty is that in step 7 of the GPHY
page, the specs are right and the code is wrong. That has been tested!
Larry
[email protected] wrote:
> Is this close?
> E
> --- /tmp/phy_g.c 2008-09-05 21:06:57.000000000 -0700
> +++ phy_g.c 2008-09-05 21:36:18.000000000 -0700
> @@ -2198,7 +2198,7 @@
> else
> b43_phy_initb6(dev);
>
> - if (phy->rev >= 2 || phy->gmode)
> + if (phy->rev >= 2 && phy->gmode)
> b43_phy_inita(dev);
>
> if (phy->rev >= 2) {
> @@ -2216,9 +2216,9 @@
> if (phy->gmode || phy->rev >= 2) {
> tmp = b43_phy_read(dev, B43_PHY_VERSION_OFDM);
> tmp &= B43_PHYVER_VERSION;
> - if (tmp == 3 || tmp == 5) {
> + if (tmp == 4 || tmp == 5) {
> b43_phy_write(dev, B43_PHY_OFDM(0xC2), 0x1816);
> - b43_phy_write(dev, B43_PHY_OFDM(0xC3), 0x8006);
> + b43_phy_write(dev, B43_PHY_OFDM(0xC3), 0x8606);
> }
> if (tmp == 5) {
> b43_phy_write(dev, B43_PHY_OFDM(0xCC),
> @@ -2226,7 +2226,7 @@
> & 0x00FF) | 0x1F00);
> }
> }
> - if ((phy->rev <= 2 && phy->gmode) || phy->rev >= 2)
> + if ((phy->rev >=2)
> b43_phy_write(dev, B43_PHY_OFDM(0x7E), 0x78);
> if (phy->radio_rev == 8) {
> b43_phy_write(dev, B43_PHY_EXTG(0x01),
It is close, but I think you are working on b43. My changes are for
b43legacy and all changes will be in drivers/net/wireless/b43legacy/phy.c
Larry