2007-06-11 10:39:17

by Andy Green

[permalink] [raw]
Subject: [PATCH 2/2] bcm43xx-mac80211: Fix reported rx frequency and channel

bcm43xx-mac80211 is reporting bogus frequencies and channels back to
mac80211 at the moment (eg, actual ch1 (2412MHz) reported as 2424MHz).

Prior to this patch, the hardware rx channel value is reported as
starting at 0x18 and rising by 0x0a per channel. Code in bcm43xx_xmit.c
tries to take this value and add 2400 to it to get the rx frequency.
It seems the intention is that the hardware reports the (rx freq - 2400),
so we want the value starting at 0x0c and rising by 0x05 per channel.

If the value read is shifted one more bit to the right, it will
succeed in doing this. Therefore this patch increases the shifting constant
by one and reduces the mask by one lsb.

The rx frequency reported in the radiotap rx and then, eg, tcpdump,
is then correct. I didn't test ch 14 but I guess the hardware is
consistent about it.

CC: Larry Finger <[email protected]>
CC: Michael Buesch <[email protected]>
Signed-off-by: Andy Green <[email protected]>

diff --git a/drivers/net/wireless/mac80211/bcm43xx/bcm43xx_xmit.h b/drivers/net/wireless/mac80211/bcm43xx/bcm43xx_xmit.h
index 44fa515..0372064 100644
--- a/drivers/net/wireless/mac80211/bcm43xx/bcm43xx_xmit.h
+++ b/drivers/net/wireless/mac80211/bcm43xx/bcm43xx_xmit.h
@@ -190,8 +190,8 @@ struct bcm43xx_rxhdr_fw4 {
/* RX channel */
#define BCM43xx_RX_CHAN_GAIN 0xFC00 /* Gain */
#define BCM43xx_RX_CHAN_GAIN_SHIFT 10
-#define BCM43xx_RX_CHAN_ID 0x03FC /* Channel ID */
-#define BCM43xx_RX_CHAN_ID_SHIFT 2
+#define BCM43xx_RX_CHAN_ID 0x03F8 /* Channel ID */
+#define BCM43xx_RX_CHAN_ID_SHIFT 3
#define BCM43xx_RX_CHAN_PHYTYPE 0x0003 /* PHY type */



--


2007-06-11 11:45:24

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 2/2] bcm43xx-mac80211: Fix reported rx frequency and channel


> Damn. This is a firmware thing, the firmware later changed because they
> needed more than 2 bits for the PHY type so things were shifted up by
> one. That means the actual channel ID mask will be 0x7f8 now and this is
> in the current specs. Can you please post which firmware you're using?
> We'll want to force people to use newer firmware with this behaviour
> -or- figure out at what point they made the transition...

See bottom of http://bcm-v4.sipsolutions.net/802.11/RX

Actually, I guess we can test this. Beacons contain the channel, we can
determine the frequency from that and see what's going on. I'll leave it
to Michael to decide what to do.

johannes


Attachments:
signature.asc (190.00 B)
This is a digitally signed message part

2007-06-11 11:26:03

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 2/2] bcm43xx-mac80211: Fix reported rx frequency and channel


> --- a/drivers/net/wireless/mac80211/bcm43xx/bcm43xx_xmit.h
> +++ b/drivers/net/wireless/mac80211/bcm43xx/bcm43xx_xmit.h
> @@ -190,8 +190,8 @@ struct bcm43xx_rxhdr_fw4 {
> /* RX channel */
> #define BCM43xx_RX_CHAN_GAIN 0xFC00 /* Gain */
> #define BCM43xx_RX_CHAN_GAIN_SHIFT 10
> -#define BCM43xx_RX_CHAN_ID 0x03FC /* Channel ID */
> -#define BCM43xx_RX_CHAN_ID_SHIFT 2
> +#define BCM43xx_RX_CHAN_ID 0x03F8 /* Channel ID */
> +#define BCM43xx_RX_CHAN_ID_SHIFT 3
> #define BCM43xx_RX_CHAN_PHYTYPE 0x0003 /* PHY type */

Damn. This is a firmware thing, the firmware later changed because they
needed more than 2 bits for the PHY type so things were shifted up by
one. That means the actual channel ID mask will be 0x7f8 now and this is
in the current specs. Can you please post which firmware you're using?
We'll want to force people to use newer firmware with this behaviour
-or- figure out at what point they made the transition...

johannes


Attachments:
signature.asc (190.00 B)
This is a digitally signed message part

2007-07-19 19:40:50

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH 2/2] bcm43xx-mac80211: Fix reported rx frequency and channel

On Thursday 19 July 2007 21:10:52 John W. Linville wrote:
> On Mon, Jun 11, 2007 at 11:38:30AM +0100, [email protected] wrote:
> > bcm43xx-mac80211 is reporting bogus frequencies and channels back to
> > mac80211 at the moment (eg, actual ch1 (2412MHz) reported as 2424MHz).
> >
> > Prior to this patch, the hardware rx channel value is reported as
> > starting at 0x18 and rising by 0x0a per channel. Code in bcm43xx_xmit.c
> > tries to take this value and add 2400 to it to get the rx frequency.
> > It seems the intention is that the hardware reports the (rx freq - 2400),
> > so we want the value starting at 0x0c and rising by 0x05 per channel.
> >
> > If the value read is shifted one more bit to the right, it will
> > succeed in doing this. Therefore this patch increases the shifting constant
> > by one and reduces the mask by one lsb.
> >
> > The rx frequency reported in the radiotap rx and then, eg, tcpdump,
> > is then correct. I didn't test ch 14 but I guess the hardware is
> > consistent about it.
> >
> > CC: Larry Finger <[email protected]>
> > CC: Michael Buesch <[email protected]>
> > Signed-off-by: Andy Green <[email protected]>
>
> Larry, Michael, Johannes -- ack/nak?

nak, the issue is more difficult than this.
The point is that firmware changed and we don't know the exact revision, yet.
But it is actually no problem in reality, as the use-it-or-die
firmware doesn't have this problem. So if someone uses another
firmware than the one we suggest, he will probably run into more
problems, as well.
The fix is called: Use the correct firmware.
For now, at least.

Of course, the author of this patch should help us to find
out the exact firmware rev where this change happened. So we
can come up with a real fix.

--
Greetings Michael.

2007-07-20 20:41:12

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 2/2] bcm43xx-mac80211: Fix reported rx frequency and channel

On Fri, 2007-07-20 at 11:58 -0500, Larry Finger wrote:

> Unfortunately, that is a necessary result of this type of reverse-engineering. If Broadcom put some
> function in the firmware, we have to leave it there as we have no idea what would break.

:)

All we're talking here in the thread is this: go to
http://bcm-v4.sipsolutions.net/802.11/RX and you see I said "(newer
ucode)" there. We need to find out what revision that is. If people send
me a whole bunch of firmware files I can disassemble them to find out.
Or, which is faster, they can just test if they see the right info.

johannes


Attachments:
signature.asc (190.00 B)
This is a digitally signed message part

2007-07-20 16:58:40

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH 2/2] bcm43xx-mac80211: Fix reported rx frequency and channel

Andy Green wrote:
>
> Understood, that is why I consider it a bad thing that functionality
> that can be done in the mac80211 driver is pushed into the binary-only
> firmware when there is a choice (otherwise known as "paranoia", apparently).

Unfortunately, that is a necessary result of this type of reverse-engineering. If Broadcom put some
function in the firmware, we have to leave it there as we have no idea what would break.

> However you stripped some quoting from Michael:
>
> ''But it is actually no problem in reality, as the use-it-or-die
> firmware doesn't have this problem. So if someone uses another
> firmware than the one we suggest, he will probably run into more
> problems, as well.
> The fix is called: Use the correct firmware.
> For now, at least.''
>
> I would summarize this that Michael is telling me one pariticular
> version of firmware - "use it or die firmware" - is especially
> blessed/correct. It might be an idea to let people know they have
> strayed from the dependency of the required firmware version in dmesg if
> indeed there is an effective dependency of the driver on it.

It isn't that it is blessed or correct, but that it has been tested. Your version has not. Who knows
what else might have changed? Once we know the version with the different behavior, a warning
message can be prepared. I don't think anyone knew about this problem until you submitted your patch.

> Can I still get the firmware version from fwcutter if I don't have the
> original Windows binary the firmware came from?

AFAIK, fwcutter can only get the version from the foreign driver. It can be gotten from the dmesg
output of your inlaws computer, or if you have the extracted firmware files here, you can bundle
them up and email them to me privately.


Larry

2007-07-20 14:57:11

by Andy Green

[permalink] [raw]
Subject: Re: [PATCH 2/2] bcm43xx-mac80211: Fix reported rx frequency and channel

Somebody in the thread at some point said:
> Andy Green wrote:
>>
>> I never heard of a special blessed firmware before, I don't think that
>> the driver made any dmesg when given the non-blessed one, if that is
>> what it is? I was given this firmware by the traditional "person on
>> IRC" and don't know where it came from, but I still have it here if any
>> versioning can be extracted from it.
>
> Just as API's and ABI's change, the writers of firmware can change their
> layout, which is what happened here. Nothing "blessed", just one
> behavior before, and a second after, the change. The version info can be
> gotten from fwcutter.

Understood, that is why I consider it a bad thing that functionality
that can be done in the mac80211 driver is pushed into the binary-only
firmware when there is a choice (otherwise known as "paranoia", apparently).

However you stripped some quoting from Michael:

''But it is actually no problem in reality, as the use-it-or-die
firmware doesn't have this problem. So if someone uses another
firmware than the one we suggest, he will probably run into more
problems, as well.
The fix is called: Use the correct firmware.
For now, at least.''

I would summarize this that Michael is telling me one pariticular
version of firmware - "use it or die firmware" - is especially
blessed/correct. It might be an idea to let people know they have
strayed from the dependency of the required firmware version in dmesg if
indeed there is an effective dependency of the driver on it.

Can I still get the firmware version from fwcutter if I don't have the
original Windows binary the firmware came from?

-Andy

2007-07-20 17:53:09

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH 2/2] bcm43xx-mac80211: Fix reported rx frequency and channel

On Friday 20 July 2007 19:48:20 Andy Green wrote:
> Actually, I can't read dmesg on my In-laws' box that has the card the
> last few weeks: I read dmesg all the time if I suspect kernel-side
> trouble. It's pretty simple and cheap to do a printk and give someone
> with problems the ability to dig themselves out of the trouble with a
> clue and even a URL in this case.
>
> If the driver is dependent on a particular version of an external file,
> and you can read the version of that file at runtime, you really ought
> to be issuing a diagnostic if the dependency is violated.

Well, alternatively you can help to debug this bug, as this is the only
known bug regarding fw version.
You could get different versions and test them. So kind of bisecting
the actual change version.

--
Greetings Michael.

2007-07-20 14:36:14

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH 2/2] bcm43xx-mac80211: Fix reported rx frequency and channel

Andy Green wrote:
>
> I never heard of a special blessed firmware before, I don't think that
> the driver made any dmesg when given the non-blessed one, if that is
> what it is? I was given this firmware by the traditional "person on
> IRC" and don't know where it came from, but I still have it here if any
> versioning can be extracted from it.

Just as API's and ABI's change, the writers of firmware can change their layout, which is what
happened here. Nothing "blessed", just one behavior before, and a second after, the change. The
version info can be gotten from fwcutter.

Larry


2007-07-20 07:37:43

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 2/2] bcm43xx-mac80211: Fix reported rx frequency and channel

On Fri, 2007-07-20 at 07:05 +0100, Andy Green wrote:

> After seeing patch tracking issues on arm-linux mailing list as well,
> those not involving me, I started making a PHP project called
> "patchfillet" for the purpose of extracting patches from a mailing list
> and trying to track the patch lifecycle in an automated way driven by
> the contents of the threads (eg, keywords like adding Acked-by: <blah>
> in a reply) and maybe one day by looking at an upstream git repo commits
> (I found that the Index: line in git-committed patches can change
> destroying what would otherwise be a hash match).

Sounds like `patchwork'.

> I must say an explicit ACK or NAK for patches is a great idea.

I'd also appreciate explicit davem-style "thanks I merged this" but hey,
I get by checking the various git trees all the time :)

johannes


Attachments:
signature.asc (190.00 B)
This is a digitally signed message part

2007-07-20 14:03:08

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH 2/2] bcm43xx-mac80211: Fix reported rx frequency and channel

On Fri, Jul 20, 2007 at 09:37:24AM +0200, Johannes Berg wrote:
> On Fri, 2007-07-20 at 07:05 +0100, Andy Green wrote:

> > I must say an explicit ACK or NAK for patches is a great idea.
>
> I'd also appreciate explicit davem-style "thanks I merged this" but hey,
> I get by checking the various git trees all the time :)

Yes, I could do better. Still, checking the "Please pull..." message
short logs might be more time efficient.

John
--
John W. Linville
[email protected]

2007-07-20 17:48:29

by Andy Green

[permalink] [raw]
Subject: Re: [PATCH 2/2] bcm43xx-mac80211: Fix reported rx frequency and channel

Somebody in the thread at some point said:
> On Friday 20 July 2007 18:58:24 Larry Finger wrote:
>> Andy Green wrote:
>>> Understood, that is why I consider it a bad thing that functionality
>>> that can be done in the mac80211 driver is pushed into the binary-only
>>> firmware when there is a choice (otherwise known as "paranoia", apparently).
>> Unfortunately, that is a necessary result of this type of reverse-engineering. If Broadcom put some
>> function in the firmware, we have to leave it there as we have no idea what would break.
>>
>>> However you stripped some quoting from Michael:
>>>
>>> ''But it is actually no problem in reality, as the use-it-or-die
>>> firmware doesn't have this problem. So if someone uses another
>>> firmware than the one we suggest, he will probably run into more
>>> problems, as well.
>>> The fix is called: Use the correct firmware.
>>> For now, at least.''
>>>
>>> I would summarize this that Michael is telling me one pariticular
>>> version of firmware - "use it or die firmware" - is especially
>>> blessed/correct. It might be an idea to let people know they have
>>> strayed from the dependency of the required firmware version in dmesg if
>>> indeed there is an effective dependency of the driver on it.
>> It isn't that it is blessed or correct, but that it has been tested. Your version has not. Who knows
>> what else might have changed? Once we know the version with the different behavior, a warning
>> message can be prepared. I don't think anyone knew about this problem until you submitted your patch.
>
> People don't read dmesg. Adding a "This firmware is unsupported"
> would have no effect.
> I experience same thing for the "bcm43xx-does-not-support-v3-issue".
> There is a clear error message saying what to do exactly. And _still_
> people mail me asking what this message means.

Sure, but naturally you don't hear from the presumably nonzero amount of
people who saw and understood the message. Whereas if there was no
message at all, only disgusted and baffled people are possible.

> The use-or-die firmware is here:
> http://linuxwireless.org/en/users/Drivers/bcm43xx
>
>>> Can I still get the firmware version from fwcutter if I don't have the
>>> original Windows binary the firmware came from?
>> AFAIK, fwcutter can only get the version from the foreign driver. It can be gotten from the dmesg
>> output of your inlaws computer, or if you have the extracted firmware files here, you can bundle
>> them up and email them to me privately.
>
> People don't read dmesg.
> q.e.d. ;)

Actually, I can't read dmesg on my In-laws' box that has the card the
last few weeks: I read dmesg all the time if I suspect kernel-side
trouble. It's pretty simple and cheap to do a printk and give someone
with problems the ability to dig themselves out of the trouble with a
clue and even a URL in this case.

If the driver is dependent on a particular version of an external file,
and you can read the version of that file at runtime, you really ought
to be issuing a diagnostic if the dependency is violated.

-Andy

2007-07-20 17:38:13

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH 2/2] bcm43xx-mac80211: Fix reported rx frequency and channel

On Friday 20 July 2007 18:58:24 Larry Finger wrote:
> Andy Green wrote:
> >
> > Understood, that is why I consider it a bad thing that functionality
> > that can be done in the mac80211 driver is pushed into the binary-only
> > firmware when there is a choice (otherwise known as "paranoia", apparently).
>
> Unfortunately, that is a necessary result of this type of reverse-engineering. If Broadcom put some
> function in the firmware, we have to leave it there as we have no idea what would break.
>
> > However you stripped some quoting from Michael:
> >
> > ''But it is actually no problem in reality, as the use-it-or-die
> > firmware doesn't have this problem. So if someone uses another
> > firmware than the one we suggest, he will probably run into more
> > problems, as well.
> > The fix is called: Use the correct firmware.
> > For now, at least.''
> >
> > I would summarize this that Michael is telling me one pariticular
> > version of firmware - "use it or die firmware" - is especially
> > blessed/correct. It might be an idea to let people know they have
> > strayed from the dependency of the required firmware version in dmesg if
> > indeed there is an effective dependency of the driver on it.
>
> It isn't that it is blessed or correct, but that it has been tested. Your version has not. Who knows
> what else might have changed? Once we know the version with the different behavior, a warning
> message can be prepared. I don't think anyone knew about this problem until you submitted your patch.

People don't read dmesg. Adding a "This firmware is unsupported"
would have no effect.
I experience same thing for the "bcm43xx-does-not-support-v3-issue".
There is a clear error message saying what to do exactly. And _still_
people mail me asking what this message means.

The use-or-die firmware is here:
http://linuxwireless.org/en/users/Drivers/bcm43xx

> > Can I still get the firmware version from fwcutter if I don't have the
> > original Windows binary the firmware came from?
>
> AFAIK, fwcutter can only get the version from the foreign driver. It can be gotten from the dmesg
> output of your inlaws computer, or if you have the extracted firmware files here, you can bundle
> them up and email them to me privately.

People don't read dmesg.
q.e.d. ;)

--
Greetings Michael.

2007-07-20 06:05:46

by Andy Green

[permalink] [raw]
Subject: Re: [PATCH 2/2] bcm43xx-mac80211: Fix reported rx frequency and channel

:
> On Thursday 19 July 2007 21:10:52 John W. Linville wrote:
>> On Mon, Jun 11, 2007 at 11:38:30AM +0100, [email protected] wrote:
>>> bcm43xx-mac80211 is reporting bogus frequencies and channels back to
>>> mac80211 at the moment (eg, actual ch1 (2412MHz) reported as 2424MHz).
>>>
>>> Prior to this patch, the hardware rx channel value is reported as
>>> starting at 0x18 and rising by 0x0a per channel. Code in bcm43xx_xmit.c
>>> tries to take this value and add 2400 to it to get the rx frequency.
>>> It seems the intention is that the hardware reports the (rx freq - 2400),
>>> so we want the value starting at 0x0c and rising by 0x05 per channel.
>>>
>>> If the value read is shifted one more bit to the right, it will
>>> succeed in doing this. Therefore this patch increases the shifting constant
>>> by one and reduces the mask by one lsb.
>>>
>>> The rx frequency reported in the radiotap rx and then, eg, tcpdump,
>>> is then correct. I didn't test ch 14 but I guess the hardware is
>>> consistent about it.
>>>
>>> CC: Larry Finger <[email protected]>
>>> CC: Michael Buesch <[email protected]>
>>> Signed-off-by: Andy Green <[email protected]>
>> Larry, Michael, Johannes -- ack/nak?
>
> nak, the issue is more difficult than this.

Although for this and the zd1211rw patch I found out they were
deprecated/more complex already using a "polling method",
I must say an explicit ACK or NAK for patches is a great idea.

After seeing patch tracking issues on arm-linux mailing list as well,
those not involving me, I started making a PHP project called
"patchfillet" for the purpose of extracting patches from a mailing list
and trying to track the patch lifecycle in an automated way driven by
the contents of the threads (eg, keywords like adding Acked-by: <blah>
in a reply) and maybe one day by looking at an upstream git repo commits
(I found that the Index: line in git-committed patches can change
destroying what would otherwise be a hash match).

The idea is it generates dynamic HTML on a website, and once it can be
trusted enough mails reports to the same mailing list about patches
without a resolution, patches accepted and rejected for the week and so
on. Right now it is able to extract all the patches, inline or
attached, from an IMAP server folder into a MySQL database along with
message ID and thread info, and to run patches against an external git
tree and checkpatch.pl. If this is interesting to anyone by all means
send me ideas for what is useful on or off list.

> The point is that firmware changed and we don't know the exact revision, yet.
> But it is actually no problem in reality, as the use-it-or-die
> firmware doesn't have this problem. So if someone uses another
> firmware than the one we suggest, he will probably run into more
> problems, as well.
> The fix is called: Use the correct firmware.
> For now, at least.
>
> Of course, the author of this patch should help us to find
> out the exact firmware rev where this change happened. So we
> can come up with a real fix.

I never heard of a special blessed firmware before, I don't think that
the driver made any dmesg when given the non-blessed one, if that is
what it is? I was given this firmware by the traditional "person on
IRC" and don't know where it came from, but I still have it here if any
versioning can be extracted from it.

Unfortunately the only sport I practice is one I named "extreme admin".
This involves maintaining Fedora installs on the machines of remote
friends and relatives during infrequent visits. My bcm43xx card is
currently living with my In-laws in Spain (and doing fine using
bcm43xx-mac80211 ;-) ).

-Andy


2007-07-19 19:33:59

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH 2/2] bcm43xx-mac80211: Fix reported rx frequency and channel

On Mon, Jun 11, 2007 at 11:38:30AM +0100, [email protected] wrote:
> bcm43xx-mac80211 is reporting bogus frequencies and channels back to
> mac80211 at the moment (eg, actual ch1 (2412MHz) reported as 2424MHz).
>
> Prior to this patch, the hardware rx channel value is reported as
> starting at 0x18 and rising by 0x0a per channel. Code in bcm43xx_xmit.c
> tries to take this value and add 2400 to it to get the rx frequency.
> It seems the intention is that the hardware reports the (rx freq - 2400),
> so we want the value starting at 0x0c and rising by 0x05 per channel.
>
> If the value read is shifted one more bit to the right, it will
> succeed in doing this. Therefore this patch increases the shifting constant
> by one and reduces the mask by one lsb.
>
> The rx frequency reported in the radiotap rx and then, eg, tcpdump,
> is then correct. I didn't test ch 14 but I guess the hardware is
> consistent about it.
>
> CC: Larry Finger <[email protected]>
> CC: Michael Buesch <[email protected]>
> Signed-off-by: Andy Green <[email protected]>

Larry, Michael, Johannes -- ack/nak?

John
--
John W. Linville
[email protected]