linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC/RFT] p54: Fix for big-endian architecture
@ 2008-10-09 15:19 Larry Finger
  2008-10-09 16:58 ` Christian Lamparter
  2008-10-13 21:44 ` Pavel Roskin
  0 siblings, 2 replies; 20+ messages in thread
From: Larry Finger @ 2008-10-09 15:19 UTC (permalink / raw)
  To: Pavel Roskin, Chr; +Cc: wireless

This patch is meant for testing on big-endian hardware. Every indication
is that the data in the bootrec structure is little endian. This patch fixes
a usage of the u32 data array as a string of u8's.

Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---

Index: wireless-testing/drivers/net/wireless/p54/p54common.h
===================================================================
--- wireless-testing.orig/drivers/net/wireless/p54/p54common.h
+++ wireless-testing/drivers/net/wireless/p54/p54common.h
@@ -18,7 +18,10 @@
 struct bootrec {
 	__le32 code;
 	__le32 len;
-	u32 data[10];
+	union {
+		u32 data[10];
+		u8 data_char[40];
+	} __attribute__((packed));
 	__le16 rx_mtu;
 } __attribute__((packed));

Index: wireless-testing/drivers/net/wireless/p54/p54common.c
===================================================================
--- wireless-testing.orig/drivers/net/wireless/p54/p54common.c
+++ wireless-testing/drivers/net/wireless/p54/p54common.c
@@ -171,8 +171,8 @@ int p54_parse_firmware(struct ieee80211_
 			break;
 		case BR_CODE_COMPONENT_VERSION:
 			/* 24 bytes should be enough for all firmwares */
-			if (strnlen((unsigned char*)bootrec->data, 24) < 24)
-				fw_version = (unsigned char*)bootrec->data;
+			if (strnlen(bootrec->data_char, 24) < 24)
+				fw_version = bootrec->data_char;
 			break;
 		case BR_CODE_DESCR: {
 			struct bootrec_desc *desc =


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC/RFT] p54: Fix for big-endian architecture
  2008-10-09 15:19 [RFC/RFT] p54: Fix for big-endian architecture Larry Finger
@ 2008-10-09 16:58 ` Christian Lamparter
  2008-10-10  0:39   ` Larry Finger
  2008-10-13 21:44 ` Pavel Roskin
  1 sibling, 1 reply; 20+ messages in thread
From: Christian Lamparter @ 2008-10-09 16:58 UTC (permalink / raw)
  To: Larry Finger; +Cc: Pavel Roskin, wireless

On Thursday 09 October 2008 17:19:23 Larry Finger wrote:
> This patch is meant for testing on big-endian hardware. Every indication
> is that the data in the bootrec structure is little endian. This patch fixes
> a usage of the u32 data array as a string of u8's.
> 
> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> ---
> 
> Index: wireless-testing/drivers/net/wireless/p54/p54common.h
> ===================================================================
> --- wireless-testing.orig/drivers/net/wireless/p54/p54common.h
> +++ wireless-testing/drivers/net/wireless/p54/p54common.h
> @@ -18,7 +18,10 @@
>  struct bootrec {
>  	__le32 code;
>  	__le32 len;
> -	u32 data[10];
> +	union {
> +		u32 data[10];
> +		u8 data_char[40];
> +	} __attribute__((packed));
>  	__le16 rx_mtu;
>  } __attribute__((packed));
> 
? how did rx_mtu end up there? it should be a part of bootrec_desc
(which is a few lines down in p54common.h)
struct bootrec_desc {
        __le16 modes;
        __le16 flags;
        __le32 rx_start;
        __le32 rx_end;
        u8 headroom;
        u8 tailroom;
        u8 unimportant[6];
        u8 rates[16];
+	__le16 rx_mtu;
} __attribute__((packed));

This structure is equivalent to struct s_lm_descr from "lmac_longbow.h", which
can be found on wireless.kernel.org's resource page.

Regards,
	Chr

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC/RFT] p54: Fix for big-endian architecture
  2008-10-09 16:58 ` Christian Lamparter
@ 2008-10-10  0:39   ` Larry Finger
  2008-10-10  1:29     ` Christian Lamparter
  0 siblings, 1 reply; 20+ messages in thread
From: Larry Finger @ 2008-10-10  0:39 UTC (permalink / raw)
  To: Christian Lamparter; +Cc: Pavel Roskin, wireless

Christian Lamparter wrote:
> On Thursday 09 October 2008 17:19:23 Larry Finger wrote:
>> Index: wireless-testing/drivers/net/wireless/p54/p54common.h
>> ===================================================================
>> --- wireless-testing.orig/drivers/net/wireless/p54/p54common.h
>> +++ wireless-testing/drivers/net/wireless/p54/p54common.h
>> @@ -18,7 +18,10 @@
>>  struct bootrec {
>>  	__le32 code;
>>  	__le32 len;
>> -	u32 data[10];
>> +	union {
>> +		u32 data[10];
>> +		u8 data_char[40];
>> +	} __attribute__((packed));
>>  	__le16 rx_mtu;
>>  } __attribute__((packed));
>>
> ? how did rx_mtu end up there? it should be a part of bootrec_desc
> (which is a few lines down in p54common.h)
> struct bootrec_desc {
>         __le16 modes;
>         __le16 flags;
>         __le32 rx_start;
>         __le32 rx_end;
>         u8 headroom;
>         u8 tailroom;
>         u8 unimportant[6];
>         u8 rates[16];
> +	__le16 rx_mtu;
> } __attribute__((packed));
> 
> This structure is equivalent to struct s_lm_descr from "lmac_longbow.h", which
> can be found on wireless.kernel.org's resource page.

As the original code got the data from bootrec, I kept it the same. On
reflection, the statement above could be

priv->rx_mtu = le16_to_cpu(desc->rx_mtu);

I think which structure it is in is purely a matter of taste, but I will make
that change, test it, and submit a new patch. The big-endian fix above will also
need to be changed. I considered converting to a single struct using unions, but
that would get pretty messy.

Larry




^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC/RFT] p54: Fix for big-endian architecture
  2008-10-10  0:39   ` Larry Finger
@ 2008-10-10  1:29     ` Christian Lamparter
  0 siblings, 0 replies; 20+ messages in thread
From: Christian Lamparter @ 2008-10-10  1:29 UTC (permalink / raw)
  To: Larry Finger; +Cc: Pavel Roskin, wireless

On Friday 10 October 2008 02:39:14 Larry Finger wrote:
> Christian Lamparter wrote:
> 
> As the original code got the data from bootrec, I kept it the same. On
> reflection, the statement above could be
> 
> priv->rx_mtu = le16_to_cpu(desc->rx_mtu);
> 
> I think which structure it is in is purely a matter of taste, but I will make
> that change, test it, and submit a new patch. The big-endian fix above will also
> need to be changed. I considered converting to a single struct using unions, but
> that would get pretty messy.
> 
> Larry

I finally found the definitions by Intersil (again):
> http://gxaafoot.homelinux.org/cgi-bin/archzoom.cgi/jean-baptiste.note@m4x.org--libre/prism54-usb--devo--0.0--patch-182/islsm_bra.h?diff=2
> tinyurl link: http://tinyurl.com/3wo45y
Unfortunately, back in 2002 noone though about big/little endian and 64bit :-D

/*
 * Each boot record is identified by a unique code, followed
 * by the length (in 32 bit words) of the data in the record. 
 */
struct s_bootrec_hdr {
	unsigned long code;
	unsigned long length;	
};

/* Generic bootrecord structure that can be mapped to each bootrecord */
struct s_bootrecord {
	struct s_bootrec_hdr hdr;
	char start_of_data;
};
[...]

so, what about changing bootrec to:
struct bootrec {
        __le32 code;
        __le32 len;
	char data[0];
} __attribute__((packed));
And put this endless discussion to rest

Regards,
	Chr

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC/RFT] p54: Fix for big-endian architecture
  2008-10-09 15:19 [RFC/RFT] p54: Fix for big-endian architecture Larry Finger
  2008-10-09 16:58 ` Christian Lamparter
@ 2008-10-13 21:44 ` Pavel Roskin
  2008-10-13 22:24   ` Johannes Berg
  1 sibling, 1 reply; 20+ messages in thread
From: Pavel Roskin @ 2008-10-13 21:44 UTC (permalink / raw)
  To: Larry Finger; +Cc: Chr, wireless

On Thu, 2008-10-09 at 08:19 -0700, Larry Finger wrote:
> This patch is meant for testing on big-endian hardware. Every indication
> is that the data in the bootrec structure is little endian. This patch fixes
> a usage of the u32 data array as a string of u8's.

I'm sorry, I didn't test your patch yet, but I could capture the output
on the serial console.  It doesn't appear to be related to p54 specific
data structures.  The driver is initialized correctly.  I can bring the
interface up.  But once I try scanning, I get a sequence of the
following messages in the kernel log:

Badness at /home/proski/src/linux-2.6/net/mac80211/rx.c:2200
NIP: c02bc850 LR: c02ab268 CTR: 00000000
REGS: ef01fcc0 TRAP: 0700   Tainted: G        W  (2.6.27-wl)
MSR: 00029032 <EE,ME,IR,DR>  CR: 22004084  XER: 20000000
TASK = c1a58800[1778] 'p54pci' THREAD: ef01e000
GPR00: 00000008 ef01fd70 c1a58800 c1a51180 ef082cc0 ef01fdb8 ef01fddc 0000000c 
GPR08: 00000080 c1a51044 f103a6d0 0000000c 42004082 00000000 01729138 000000db 
GPR16: 0172920c 41400000 0173dba4 00241678 c03b237c ef84dfb0 ef84dfac 00000000 
GPR24: c041dde0 c0420a8c c1a51254 00000000 c1a51180 ef01fdb8 c1a51180 ef082cc0 
NIP [c02bc850] __ieee80211_rx+0x17c/0x638
LR [c02ab268] ieee80211_tasklet_handler+0x104/0x120
Call Trace:
[ef01fd70] [c1a0c020] 0xc1a0c020 (unreliable)
[ef01fdb0] [c02ab268] ieee80211_tasklet_handler+0x104/0x120
[ef01fe00] [c002e318] tasklet_action+0x80/0xfc
[ef01fe20] [c002ebf8] __do_softirq+0x8c/0xfc
[ef01fe50] [c00062ac] do_softirq+0x58/0x5c
[ef01fe60] [c002eaf8] irq_exit+0x84/0x88
[ef01fe70] [c0006320] do_IRQ+0x70/0xc8
[ef01fe80] [c00106fc] ret_from_except+0x0/0x14
--- Exception: 501 at ieee80211_hw_config+0xac/0xbc
    LR = ieee80211_hw_config+0xa0/0xbc
[ef01ff50] [c02b04f0] ieee80211_scan_work+0x144/0x1b8
[ef01ff60] [c003b26c] run_workqueue+0xc4/0x160
[ef01ff90] [c003b850] worker_thread+0x54/0xb8
[ef01ffd0] [c003f788] kthread+0x50/0x88
[ef01fff0] [c000fe88] kernel_thread+0x44/0x60
Instruction dump:
6c09ffff 2f898800 419e0244 80010044 7f83e378 7f04c378 7fa5eb78 7ea6ab78 
baa10014 7c0803a6 38210040 4bfff4d4 <0fe00000> 80010044 baa10014 38210040 

That's the current wireless-testing.

If the patch makes any difference, I'll report it separately.

-- 
Regards,
Pavel Roskin

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC/RFT] p54: Fix for big-endian architecture
  2008-10-13 21:44 ` Pavel Roskin
@ 2008-10-13 22:24   ` Johannes Berg
  2008-10-13 22:36     ` Pavel Roskin
  0 siblings, 1 reply; 20+ messages in thread
From: Johannes Berg @ 2008-10-13 22:24 UTC (permalink / raw)
  To: Pavel Roskin; +Cc: Larry Finger, Chr, wireless

[-- Attachment #1: Type: text/plain, Size: 823 bytes --]

On Mon, 2008-10-13 at 17:44 -0400, Pavel Roskin wrote:
> On Thu, 2008-10-09 at 08:19 -0700, Larry Finger wrote:
> > This patch is meant for testing on big-endian hardware. Every indication
> > is that the data in the bootrec structure is little endian. This patch fixes
> > a usage of the u32 data array as a string of u8's.
> 
> I'm sorry, I didn't test your patch yet, but I could capture the output
> on the serial console.  It doesn't appear to be related to p54 specific
> data structures.  The driver is initialized correctly.  I can bring the
> interface up.  But once I try scanning, I get a sequence of the
> following messages in the kernel log:
> 
> Badness at /home/proski/src/linux-2.6/net/mac80211/rx.c:2200

That _is_ the driver's fault, it's not setting the rate index correctly.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC/RFT] p54: Fix for big-endian architecture
  2008-10-13 22:24   ` Johannes Berg
@ 2008-10-13 22:36     ` Pavel Roskin
  2008-10-13 22:55       ` Christian Lamparter
  0 siblings, 1 reply; 20+ messages in thread
From: Pavel Roskin @ 2008-10-13 22:36 UTC (permalink / raw)
  To: Johannes Berg; +Cc: Larry Finger, Chr, wireless

On Tue, 2008-10-14 at 00:24 +0200, Johannes Berg wrote:
> On Mon, 2008-10-13 at 17:44 -0400, Pavel Roskin wrote:
> > On Thu, 2008-10-09 at 08:19 -0700, Larry Finger wrote:
> > > This patch is meant for testing on big-endian hardware. Every indication
> > > is that the data in the bootrec structure is little endian. This patch fixes
> > > a usage of the u32 data array as a string of u8's.
> > 
> > I'm sorry, I didn't test your patch yet, but I could capture the output
> > on the serial console.  It doesn't appear to be related to p54 specific
> > data structures.  The driver is initialized correctly.  I can bring the
> > interface up.  But once I try scanning, I get a sequence of the
> > following messages in the kernel log:
> > 
> > Badness at /home/proski/src/linux-2.6/net/mac80211/rx.c:2200
> 
> That _is_ the driver's fault, it's not setting the rate index correctly.

I put a printk() there, and it shows:
status->rate_idx = 12, sband->n_bitrates = 8

status->rate_idx should be less or equal sband->n_bitrates.

I don't get that on i386.  But the values don't seem to be corrupted by
byte-swapping.

-- 
Regards,
Pavel Roskin

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC/RFT] p54: Fix for big-endian architecture
  2008-10-13 22:55       ` Christian Lamparter
@ 2008-10-13 22:55         ` Pavel Roskin
  2008-10-13 23:05           ` Pavel Roskin
  0 siblings, 1 reply; 20+ messages in thread
From: Pavel Roskin @ 2008-10-13 22:55 UTC (permalink / raw)
  To: Christian Lamparter; +Cc: Johannes Berg, Larry Finger, wireless

On Tue, 2008-10-14 at 00:55 +0200, Christian Lamparter wrote:
> > I don't get that on i386.  But the values don't seem to be corrupted
> by
> > byte-swapping.
> >
> Hmm, this should be impossible since:
> "p54: report appropriate rate and band values for 802.11a"
> 5f840304b5f7dff0028407fa9b284aecb85a94aa

I see.  It's recent code, so maybe it wasn't broken yet when I was
testing it on i386.

This code in drivers/net/wireless/p54/p54common.c sets rate_idx:

  rx_status.rate_idx = (dev->conf.channel->band == IEEE80211_BAND_2GHZ ?
  hdr->rate : (hdr->rate - 4)) & 0xf;

printk() shows:

dev->conf.channel->band = 1, hdr->rate = 0, rx_status.rate_idx = 12

-- 
Regards,
Pavel Roskin

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC/RFT] p54: Fix for big-endian architecture
  2008-10-13 22:36     ` Pavel Roskin
@ 2008-10-13 22:55       ` Christian Lamparter
  2008-10-13 22:55         ` Pavel Roskin
  0 siblings, 1 reply; 20+ messages in thread
From: Christian Lamparter @ 2008-10-13 22:55 UTC (permalink / raw)
  To: Pavel Roskin; +Cc: Johannes Berg, Larry Finger, wireless

On Tuesday 14 October 2008 00:36:32 Pavel Roskin wrote:
> On Tue, 2008-10-14 at 00:24 +0200, Johannes Berg wrote:
> > On Mon, 2008-10-13 at 17:44 -0400, Pavel Roskin wrote:
> > > On Thu, 2008-10-09 at 08:19 -0700, Larry Finger wrote:
> > > > This patch is meant for testing on big-endian hardware. Every indication
> > > > is that the data in the bootrec structure is little endian. This patch fixes
> > > > a usage of the u32 data array as a string of u8's.
> > > 
> > > I'm sorry, I didn't test your patch yet, but I could capture the output
> > > on the serial console.  It doesn't appear to be related to p54 specific
> > > data structures.  The driver is initialized correctly.  I can bring the
> > > interface up.  But once I try scanning, I get a sequence of the
> > > following messages in the kernel log:
> > > 
> > > Badness at /home/proski/src/linux-2.6/net/mac80211/rx.c:2200
> > 
> > That _is_ the driver's fault, it's not setting the rate index correctly.
> 
> I put a printk() there, and it shows:
> status->rate_idx = 12, sband->n_bitrates = 8
> 
> status->rate_idx should be less or equal sband->n_bitrates.
> 
> I don't get that on i386.  But the values don't seem to be corrupted by
> byte-swapping.
>
Hmm, this should be impossible since:
"p54: report appropriate rate and band values for 802.11a"
5f840304b5f7dff0028407fa9b284aecb85a94aa

regards,
	Chr

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC/RFT] p54: Fix for big-endian architecture
  2008-10-13 22:55         ` Pavel Roskin
@ 2008-10-13 23:05           ` Pavel Roskin
  2008-10-13 23:07             ` Johannes Berg
  2008-10-13 23:31             ` Christian Lamparter
  0 siblings, 2 replies; 20+ messages in thread
From: Pavel Roskin @ 2008-10-13 23:05 UTC (permalink / raw)
  To: Christian Lamparter; +Cc: Johannes Berg, Larry Finger, wireless

On Mon, 2008-10-13 at 18:55 -0400, Pavel Roskin wrote:
> On Tue, 2008-10-14 at 00:55 +0200, Christian Lamparter wrote:
> > > I don't get that on i386.  But the values don't seem to be corrupted
> > by
> > > byte-swapping.
> > >
> > Hmm, this should be impossible since:
> > "p54: report appropriate rate and band values for 802.11a"
> > 5f840304b5f7dff0028407fa9b284aecb85a94aa
> 
> I see.  It's recent code, so maybe it wasn't broken yet when I was
> testing it on i386.
> 
> This code in drivers/net/wireless/p54/p54common.c sets rate_idx:
> 
>   rx_status.rate_idx = (dev->conf.channel->band == IEEE80211_BAND_2GHZ ?
>   hdr->rate : (hdr->rate - 4)) & 0xf;
> 
> printk() shows:
> 
> dev->conf.channel->band = 1, hdr->rate = 0, rx_status.rate_idx = 12

And if I set rx_status.rate_idx to 0, I still get that badness for the
same reason (status->rate_idx = 12, sband->n_bitrates = 8).  Also, there
is another badness reported sometimes:

Badness at /home/proski/src/linux-2.6/net/mac80211/main.c:232
NIP: c02aa6a8 LR: c02aa6a0 CTR: c02cb578
REGS: eeacfe90 TRAP: 0700   Tainted: G        W  (2.6.27-wl)
MSR: 00029032 <EE,ME,IR,DR>  CR: 24004024  XER: 20000000
TASK = ef081800[6660] 'p54pci' THREAD: eeace000
GPR00: 00000001 eeacff40 ef081800 ffffffea c1bd1a20 00000056 00000031 ef10a000 
GPR08: 00000031 ffffffe9 00000056 c02cb578 00000011 00000000 01729138 000000db 
GPR16: 0172920c 41400000 0173dba4 00241678 c03b237c ef84dfb0 ef84dfac 00000001 
GPR24: c032cfa0 c0359d90 c032cfd8 00000001 00000000 eeace000 f103a6a8 c1a19180 
NIP [c02aa6a8] ieee80211_hw_config+0xa8/0xbc
LR [c02aa6a0] ieee80211_hw_config+0xa0/0xbc
Call Trace:
[eeacff40] [c02aa6a0] ieee80211_hw_config+0xa0/0xbc (unreliable)
[eeacff50] [c02b04f0] ieee80211_scan_work+0x144/0x1b8
[eeacff60] [c003b26c] run_workqueue+0xc4/0x160
[eeacff90] [c003b850] worker_thread+0x54/0xb8
[eeacffd0] [c003f788] kthread+0x50/0x88
[eeacfff0] [c000fe88] kernel_thread+0x44/0x60
Instruction dump:
80090014 7f805800 41bdffd0 4bffffc0 812306e8 4bffff90 81230060 81290014 
7d2903a6 4e800421 3123ffff 7c091910 <0f000000> 80010014 38210010 7c0803a6 

The code is:

        if (changed && local->open_count) {
                ret = local->ops->config(local_to_hw(local), changed);
                /*
                 * HW reconfiguration should never fail, the driver has told
                 * us what it can support so it should live up to that promise.
                 */
                WARN_ON(ret);
        }

-- 
Regards,
Pavel Roskin

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC/RFT] p54: Fix for big-endian architecture
  2008-10-13 23:05           ` Pavel Roskin
@ 2008-10-13 23:07             ` Johannes Berg
  2008-10-13 23:36               ` Pavel Roskin
  2008-10-13 23:31             ` Christian Lamparter
  1 sibling, 1 reply; 20+ messages in thread
From: Johannes Berg @ 2008-10-13 23:07 UTC (permalink / raw)
  To: Pavel Roskin; +Cc: Christian Lamparter, Larry Finger, wireless

[-- Attachment #1: Type: text/plain, Size: 517 bytes --]

On Mon, 2008-10-13 at 19:05 -0400, Pavel Roskin wrote:

> And if I set rx_status.rate_idx to 0, I still get that badness for the
> same reason (status->rate_idx = 12, sband->n_bitrates = 8).  

??
That's not right, if you set rx_status.rate_idx to 0 status->rate_idx
should be 0 :)

> Also, there
> is another badness reported sometimes:

That's new. On a whim I decided that I wanted to know why config() had a
return value ;) Will see why p54 makes it fail, maybe there's a mac80211
bug.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC/RFT] p54: Fix for big-endian architecture
  2008-10-13 23:05           ` Pavel Roskin
  2008-10-13 23:07             ` Johannes Berg
@ 2008-10-13 23:31             ` Christian Lamparter
  2008-10-13 23:36               ` Johannes Berg
  1 sibling, 1 reply; 20+ messages in thread
From: Christian Lamparter @ 2008-10-13 23:31 UTC (permalink / raw)
  To: Pavel Roskin; +Cc: Johannes Berg, Larry Finger, wireless

On Tuesday 14 October 2008 01:05:51 Pavel Roskin wrote:
> On Mon, 2008-10-13 at 18:55 -0400, Pavel Roskin wrote:
> > On Tue, 2008-10-14 at 00:55 +0200, Christian Lamparter wrote:
> > > > I don't get that on i386.  But the values don't seem to be corrupted
> > > by
> > > > byte-swapping.
> > > >
> > > Hmm, this should be impossible since:
> > > "p54: report appropriate rate and band values for 802.11a"
> > > 5f840304b5f7dff0028407fa9b284aecb85a94aa
> > 
> > I see.  It's recent code, so maybe it wasn't broken yet when I was
> > testing it on i386.
> > 
> > This code in drivers/net/wireless/p54/p54common.c sets rate_idx:
> > 
> >   rx_status.rate_idx = (dev->conf.channel->band == IEEE80211_BAND_2GHZ ?
> >   hdr->rate : (hdr->rate - 4)) & 0xf;
> > 
> > printk() shows:
> > 
> > dev->conf.channel->band = 1, hdr->rate = 0, rx_status.rate_idx = 12
> 
> And if I set rx_status.rate_idx to 0, I still get that badness for the
> same reason (status->rate_idx = 12, sband->n_bitrates = 8).  Also, there
> is another badness reported sometimes:
> 
hmm, strange...
so dev->conf.channel->band = 1 (=> IEEE80211_BAND_2GHZ)
Ok, but why are there only 8 rates? It should be twelve for 2.4GHz Band!
(What Card/MAC & RF-Chip have you got? Indigo/Duette/Firsbee or XBow?)

Regards,
	Chr

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC/RFT] p54: Fix for big-endian architecture
  2008-10-13 23:07             ` Johannes Berg
@ 2008-10-13 23:36               ` Pavel Roskin
  2008-10-14  0:00                 ` Pavel Roskin
  0 siblings, 1 reply; 20+ messages in thread
From: Pavel Roskin @ 2008-10-13 23:36 UTC (permalink / raw)
  To: Johannes Berg; +Cc: Christian Lamparter, Larry Finger, wireless

On Tue, 2008-10-14 at 01:07 +0200, Johannes Berg wrote:
> On Mon, 2008-10-13 at 19:05 -0400, Pavel Roskin wrote:
> 
> > And if I set rx_status.rate_idx to 0, I still get that badness for the
> > same reason (status->rate_idx = 12, sband->n_bitrates = 8).  
> 
> ??
> That's not right, if you set rx_status.rate_idx to 0 status->rate_idx
> should be 0 :)

Sorry, I didn't code it right.  You are correct.

> > Also, there
> > is another badness reported sometimes:
> 
> That's new. On a whim I decided that I wanted to know why config() had a
> return value ;) Will see why p54 makes it fail, maybe there's a mac80211
> bug.

p54_config() calls p54_set_freq(), which can fail with -EINVAL.  I'm
looking why it can happen.

-- 
Regards,
Pavel Roskin

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC/RFT] p54: Fix for big-endian architecture
  2008-10-13 23:31             ` Christian Lamparter
@ 2008-10-13 23:36               ` Johannes Berg
  0 siblings, 0 replies; 20+ messages in thread
From: Johannes Berg @ 2008-10-13 23:36 UTC (permalink / raw)
  To: Christian Lamparter; +Cc: Pavel Roskin, Larry Finger, wireless

[-- Attachment #1: Type: text/plain, Size: 192 bytes --]

On Tue, 2008-10-14 at 01:31 +0200, Christian Lamparter wrote:

> hmm, strange...
> so dev->conf.channel->band = 1 (=> IEEE80211_BAND_2GHZ)

Ummm, no, 0 is 2 GHz, 1 is 5 GHz.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC/RFT] p54: Fix for big-endian architecture
  2008-10-13 23:36               ` Pavel Roskin
@ 2008-10-14  0:00                 ` Pavel Roskin
  2008-10-14  0:15                   ` Christian Lamparter
  0 siblings, 1 reply; 20+ messages in thread
From: Pavel Roskin @ 2008-10-14  0:00 UTC (permalink / raw)
  To: Johannes Berg; +Cc: Christian Lamparter, Larry Finger, wireless

On Mon, 2008-10-13 at 19:36 -0400, Pavel Roskin wrote:

> p54_config() calls p54_set_freq(), which can fail with -EINVAL.  I'm
> looking why it can happen.

The calibration table only has frequencies in the 2.4GHz band, but the
scan goes through the 5GHz band as well.  That's how it fails:

freq = 5825
i = 0, priv->iq_autocal[i].freq = 2412
i = 1, priv->iq_autocal[i].freq = 2417
i = 2, priv->iq_autocal[i].freq = 2422
i = 3, priv->iq_autocal[i].freq = 2427
i = 4, priv->iq_autocal[i].freq = 2432
i = 5, priv->iq_autocal[i].freq = 2437
i = 6, priv->iq_autocal[i].freq = 2442
i = 7, priv->iq_autocal[i].freq = 2447
i = 8, priv->iq_autocal[i].freq = 2452
i = 9, priv->iq_autocal[i].freq = 2457
i = 10, priv->iq_autocal[i].freq = 2462
i = 11, priv->iq_autocal[i].freq = 2467
i = 12, priv->iq_autocal[i].freq = 2472
i = 13, priv->iq_autocal[i].freq = 2484
phy9: frequency change failed
ret = -22

Either we should be using a different table for the 5GHz band, or the
driver should not announce 5GHz channels if the calibration table lacks
them.

-- 
Regards,
Pavel Roskin

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC/RFT] p54: Fix for big-endian architecture
  2008-10-14  0:00                 ` Pavel Roskin
@ 2008-10-14  0:15                   ` Christian Lamparter
  2008-10-14  1:20                     ` Pavel Roskin
  0 siblings, 1 reply; 20+ messages in thread
From: Christian Lamparter @ 2008-10-14  0:15 UTC (permalink / raw)
  To: Pavel Roskin; +Cc: Johannes Berg, Larry Finger, wireless

On Tuesday 14 October 2008 02:00:39 Pavel Roskin wrote:
> On Mon, 2008-10-13 at 19:36 -0400, Pavel Roskin wrote:
> 
> > p54_config() calls p54_set_freq(), which can fail with -EINVAL.  I'm
> > looking why it can happen.
> 
> The calibration table only has frequencies in the 2.4GHz band, but the
> scan goes through the 5GHz band as well.  That's how it fails:
> 
> freq = 5825
> i = 0, priv->iq_autocal[i].freq = 2412
> i = 1, priv->iq_autocal[i].freq = 2417
> i = 2, priv->iq_autocal[i].freq = 2422
> i = 3, priv->iq_autocal[i].freq = 2427
> i = 4, priv->iq_autocal[i].freq = 2432
> i = 5, priv->iq_autocal[i].freq = 2437
> i = 6, priv->iq_autocal[i].freq = 2442
> i = 7, priv->iq_autocal[i].freq = 2447
> i = 8, priv->iq_autocal[i].freq = 2452
> i = 9, priv->iq_autocal[i].freq = 2457
> i = 10, priv->iq_autocal[i].freq = 2462
> i = 11, priv->iq_autocal[i].freq = 2467
> i = 12, priv->iq_autocal[i].freq = 2472
> i = 13, priv->iq_autocal[i].freq = 2484
> phy9: frequency change failed
> ret = -22
> 
> Either we should be using a different table for the 5GHz band, or the
> driver should not announce 5GHz channels if the calibration table lacks
> them.
I ask again:
What Card/MAC & RF-Chip do you have? Indigo/Duette/Firsbee or XBow?

see switch instruction in p54common.c line 430 in p54_parse_eeprom

Regards,
	Chr

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC/RFT] p54: Fix for big-endian architecture
  2008-10-14  0:15                   ` Christian Lamparter
@ 2008-10-14  1:20                     ` Pavel Roskin
  2008-10-14  2:10                       ` Christian Lamparter
  0 siblings, 1 reply; 20+ messages in thread
From: Pavel Roskin @ 2008-10-14  1:20 UTC (permalink / raw)
  To: Christian Lamparter; +Cc: Johannes Berg, Larry Finger, wireless

Quoting Christian Lamparter <chunkeey@web.de>:

> I ask again:
> What Card/MAC & RF-Chip do you have? Indigo/Duette/Firsbee or XBow?

Sorry if I missed your question.

This is a MiniPCI card that has a label with "XG-600" printed on it.   
The driver shows following in the kernel log:

firmware: requesting isl3886
p54: LM86 firmware
p54: FW rev 2.7.0.0 - Softmac protocol 4.1
p54: unknown eeprom code : 0x1
p54: unknown eeprom code : 0x3
p54: unknown eeprom code : 0x1007
p54: unknown eeprom code : 0x1008
p54: unknown eeprom code : 0x1100
p54: unknown eeprom code : 0x1905
phy10: hwaddr 00:60:b3:c9:04:f2, MAC:isl3890 RF:Indigo?
phy10: Selected rate control algorithm 'pid'
udev: renamed network interface wlan1 to wlan2
firmware: requesting isl3886

I understand the driver detects it as Indigo.

# lspci -v -s 01:04.0
01:04.0 Network controller: Intersil Corporation ISL3890 [Prism  
GT/Prism Duette]/ISL3886 [Prism Javelin/Prism Xbow] (rev 01)
         Subsystem: Z-Com, Inc. XG-600 and clones Wireless Adapter
         Flags: bus master, medium devsel, latency 16, IRQ 25
         Memory at 80882000 (32-bit, non-prefetchable) [size=8K]
         Capabilities: [dc] Power Management version 1
         Kernel driver in use: p54pci
         Kernel modules: prism54, p54pci

-- 
Regards,
Pavel Roskin

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC/RFT] p54: Fix for big-endian architecture
  2008-10-14  1:20                     ` Pavel Roskin
@ 2008-10-14  2:10                       ` Christian Lamparter
  2008-10-14  2:42                         ` [RFC/RFT][PATCH] p54: enable 2.4/5GHz spectrum by eeprom bits Christian Lamparter
  2008-10-14  8:15                         ` [RFC/RFT] p54: Fix for big-endian architecture Johannes Berg
  0 siblings, 2 replies; 20+ messages in thread
From: Christian Lamparter @ 2008-10-14  2:10 UTC (permalink / raw)
  To: Pavel Roskin; +Cc: Johannes Berg, Larry Finger, wireless

On Tuesday 14 October 2008 03:20:39 Pavel Roskin wrote:
> Quoting Christian Lamparter <chunkeey@web.de>:
> 
> > I ask again:
> > What Card/MAC & RF-Chip do you have? Indigo/Duette/Firsbee or XBow?
> 
> Sorry if I missed your question.
np
> 
> This is a MiniPCI card that has a label with "XG-600" printed on it.   
> The driver shows following in the kernel log:
> 
> firmware: requesting isl3886
> p54: LM86 firmware
> p54: FW rev 2.7.0.0 - Softmac protocol 4.1
> p54: unknown eeprom code : 0x1
> p54: unknown eeprom code : 0x3
> p54: unknown eeprom code : 0x1007
> p54: unknown eeprom code : 0x1008
> p54: unknown eeprom code : 0x1100
> p54: unknown eeprom code : 0x1905
> phy10: hwaddr 00:60:b3:c9:04:f2, MAC:isl3890 RF:Indigo?
> phy10: Selected rate control algorithm 'pid'
> udev: renamed network interface wlan1 to wlan2
> firmware: requesting isl3886
> 
> I understand the driver detects it as Indigo.
> 
> # lspci -v -s 01:04.0
> 01:04.0 Network controller: Intersil Corporation ISL3890 [Prism  
> GT/Prism Duette]/ISL3886 [Prism Javelin/Prism Xbow] (rev 01)
>          Subsystem: Z-Com, Inc. XG-600 and clones Wireless Adapter
>          Flags: bus master, medium devsel, latency 16, IRQ 25
>          Memory at 80882000 (32-bit, non-prefetchable) [size=8K]
>          Capabilities: [dc] Power Management version 1
>          Kernel driver in use: p54pci
>          Kernel modules: prism54, p54pci
> 
Alright, your eeprom says that your card is technically 802.11a capable.
It has the right MAC (ISL3890 Duette) and a 5GHz Phy/Synth/RF (whatever it's called).
 
but unfortunatly, it doesn't provide any calibration data...
And that's the problem here. Since without it, p54_set_freq can't put the card into 802.11a
and the card stays at the last selected channel (somewhere in the 2.4GHz spectrum!).
However after p54_config returns with -EINVAL, mac80211 doesn't revert dev->conf.channel
back to the original 2.4GHz setting...

with your card still listening on 802.11bg channels, it could capture a beacon, or something else
that is transmitted with 1MBit (hdr->rate = 0) and crash.
note: hdr->rate is a u8. so: 0 - 4 = 0xfc and (0xfc & 0xf) = 0xc => 12

so, I guess we have to do two things...
1. check if calibration data includes 5GHz channels AND if it has the Phy/Synth/RF.
2. cache dev->conf.channel locally in the driver.

Regards,
	Chr

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [RFC/RFT][PATCH] p54: enable 2.4/5GHz spectrum by eeprom bits.
  2008-10-14  2:10                       ` Christian Lamparter
@ 2008-10-14  2:42                         ` Christian Lamparter
  2008-10-14  8:15                         ` [RFC/RFT] p54: Fix for big-endian architecture Johannes Berg
  1 sibling, 0 replies; 20+ messages in thread
From: Christian Lamparter @ 2008-10-14  2:42 UTC (permalink / raw)
  To: Pavel Roskin; +Cc: Johannes Berg, Larry Finger, wireless

Rather than setting the available bands by looking only at the phy-chip,
we now use bits in the eeprom, which indicates the real capabilities.

Reported-by: Pavel Roskin <proski@gnu.org>
Signed-off-by: Christian Lamparter
---
diff --git a/drivers/net/wireless/p54/p54common.c b/drivers/net/wireless/p54/p54common.c
index 0180466..48f91f3 100644
--- a/drivers/net/wireless/p54/p54common.c
+++ b/drivers/net/wireless/p54/p54common.c
@@ -306,8 +306,8 @@ static int p54_convert_rev1(struct ieee80211_hw *dev,
 	return 0;
 }
 
-static const char *p54_rf_chips[] = { "NULL", "Indigo?", "Duette",
-                              "Frisbee", "Xbow", "Longbow" };
+static const char *p54_rf_chips[] = { "NULL", "Duette3", "Duette2",
+                              "Frisbee", "Xbow", "Longbow", "NULL", "NULL" };
 static int p54_init_xbow_synth(struct ieee80211_hw *dev);
 
 static int p54_parse_eeprom(struct ieee80211_hw *dev, void *eeprom, int len)
@@ -319,6 +319,7 @@ static int p54_parse_eeprom(struct ieee80211_hw *dev, void *eeprom, int len)
 	void *tmp;
 	int err;
 	u8 *end = (u8 *)eeprom + len;
+	u16 synth;
 	DECLARE_MAC_BUF(mac);
 
 	wrap = (struct eeprom_pda_wrap *) eeprom;
@@ -400,8 +401,8 @@ static int p54_parse_eeprom(struct ieee80211_hw *dev, void *eeprom, int len)
 			tmp = entry->data;
 			while ((u8 *)tmp < entry->data + data_len) {
 				struct bootrec_exp_if *exp_if = tmp;
-				if (le16_to_cpu(exp_if->if_id) == 0xF)
-					priv->rxhw = le16_to_cpu(exp_if->variant) & 0x07;
+				if (le16_to_cpu(exp_if->if_id) == 0xf)
+					synth = le16_to_cpu(exp_if->variant);
 				tmp += sizeof(struct bootrec_exp_if);
 			}
 			break;
@@ -427,22 +428,13 @@ static int p54_parse_eeprom(struct ieee80211_hw *dev, void *eeprom, int len)
 		goto err;
 	}
 
-	switch (priv->rxhw) {
-	case 4: /* XBow */
+	priv->rxhw = synth & 0x07;
+	if (priv->rxhw == 4)
 		p54_init_xbow_synth(dev);
-	case 1: /* Indigo? */
-	case 2: /* Duette */
-		dev->wiphy->bands[IEEE80211_BAND_5GHZ] = &band_5GHz;
-	case 3: /* Frisbee */
-	case 5: /* Longbow */
+	if (!(synth & 0x40))
 		dev->wiphy->bands[IEEE80211_BAND_2GHZ] = &band_2GHz;
-		break;
-	default:
-		printk(KERN_ERR "%s: unsupported RF-Chip\n",
-			wiphy_name(dev->wiphy));
-		err = -EINVAL;
-		goto err;
-	}
+	if (!(synth & 0x80))
+		dev->wiphy->bands[IEEE80211_BAND_5GHZ] = &band_5GHz;
 
 	if (!is_valid_ether_addr(dev->wiphy->perm_addr)) {
 		u8 perm_addr[ETH_ALEN];

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [RFC/RFT] p54: Fix for big-endian architecture
  2008-10-14  2:10                       ` Christian Lamparter
  2008-10-14  2:42                         ` [RFC/RFT][PATCH] p54: enable 2.4/5GHz spectrum by eeprom bits Christian Lamparter
@ 2008-10-14  8:15                         ` Johannes Berg
  1 sibling, 0 replies; 20+ messages in thread
From: Johannes Berg @ 2008-10-14  8:15 UTC (permalink / raw)
  To: Christian Lamparter; +Cc: Pavel Roskin, Larry Finger, wireless

[-- Attachment #1: Type: text/plain, Size: 1217 bytes --]

On Tue, 2008-10-14 at 04:10 +0200, Christian Lamparter wrote:

> Alright, your eeprom says that your card is technically 802.11a capable.
> It has the right MAC (ISL3890 Duette) and a 5GHz Phy/Synth/RF (whatever it's called).
>  
> but unfortunatly, it doesn't provide any calibration data...
> And that's the problem here. Since without it, p54_set_freq can't put the card into 802.11a
> and the card stays at the last selected channel (somewhere in the 2.4GHz spectrum!).
> However after p54_config returns with -EINVAL, mac80211 doesn't revert dev->conf.channel
> back to the original 2.4GHz setting...

Heh, so this actually helped catch a problem, nice.

> with your card still listening on 802.11bg channels, it could capture a beacon, or something else
> that is transmitted with 1MBit (hdr->rate = 0) and crash.
> note: hdr->rate is a u8. so: 0 - 4 = 0xfc and (0xfc & 0xf) = 0xc => 12
> 
> so, I guess we have to do two things...
> 1. check if calibration data includes 5GHz channels AND if it has the Phy/Synth/RF.
> 2. cache dev->conf.channel locally in the driver.

Just (1) as your patch did seems fine to me, then mac80211 will not
attempt to go to the other channels.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2008-10-14  8:16 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-10-09 15:19 [RFC/RFT] p54: Fix for big-endian architecture Larry Finger
2008-10-09 16:58 ` Christian Lamparter
2008-10-10  0:39   ` Larry Finger
2008-10-10  1:29     ` Christian Lamparter
2008-10-13 21:44 ` Pavel Roskin
2008-10-13 22:24   ` Johannes Berg
2008-10-13 22:36     ` Pavel Roskin
2008-10-13 22:55       ` Christian Lamparter
2008-10-13 22:55         ` Pavel Roskin
2008-10-13 23:05           ` Pavel Roskin
2008-10-13 23:07             ` Johannes Berg
2008-10-13 23:36               ` Pavel Roskin
2008-10-14  0:00                 ` Pavel Roskin
2008-10-14  0:15                   ` Christian Lamparter
2008-10-14  1:20                     ` Pavel Roskin
2008-10-14  2:10                       ` Christian Lamparter
2008-10-14  2:42                         ` [RFC/RFT][PATCH] p54: enable 2.4/5GHz spectrum by eeprom bits Christian Lamparter
2008-10-14  8:15                         ` [RFC/RFT] p54: Fix for big-endian architecture Johannes Berg
2008-10-13 23:31             ` Christian Lamparter
2008-10-13 23:36               ` Johannes Berg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).