[PATCH 2/2] rt2x00: Implement support for rt2800pci

[PATCH 2/2] rt2x00: Implement support for rt2800pci

[Ivo van Doorn]

Add support for the rt2860/rt3090 chipsets from Ralink.

Includes various patches from a lot of people who helped
getting this driver into the current shape.

Signed-off-by: Alban Browaeys <prahal@yahoo.com>
Signed-off-by: Benoit PAPILLAULT <benoit.papillault@free.fr>
Signed-off-by: Felix Fietkau <nbd@openwrt.org>
Signed-off-by: Luis Correia <luis.f.correia@gmail.com>
Signed-off-by: Mattias Nissler <mattias.nissler@gmx.de>
Signed-off-by: Mark Asselstine <asselsm@gmail.com>
Signed-off-by: Xose Vazquez Perez <xose.vazquez@gmail.com>
Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>
---
http://kernel.org/pub/linux/kernel/people/ivd/patches/0003-rt2x00-Implement-support-for-rt2800pci.patch

Re: [PATCH 2/2] rt2x00: Implement support for rt2800pci

[Simon Raffeiner]

Hello List,

I was comparing rt2800pci and the INF file of the original RT2860 windows 
driver out curiosity. The original INF file contains 108 PCI id entries, 
rt2800pci.c only 20. At first I thought that an entry like 

PCI\VEN_1814&DEV_0601&SUBSYS_77281432

(Edimax card) in the INF file will be automatically matched by rt2800pci for 
the "1814:0601" vendor/device id, but then I found an additional entry for 
"1432:7728" (the susbsystem id) in the rt2800pci pci_device_id table. 

I am a bit confused: Should every entry from the INF file have a PCI_DEVICE() 
counterpart in our driver (resulting in 108 entries), or is this a special 
case where the Manufacturer (Edimax) produces cards that actually have 
"1432:7728" as vendor/device id.

You can probably tell that I don't have much experience developing kernel 
drivers, but I am willing to get it right.

regards,
Simon Raffeiner




Am Donnerstag, 15. Oktober 2009 22:04:14 schrieb Ivo van Doorn:
> Add support for the rt2860/rt3090 chipsets from Ralink.
> 
> Includes various patches from a lot of people who helped
> getting this driver into the current shape.
> 
> Signed-off-by: Alban Browaeys <prahal@yahoo.com>
> Signed-off-by: Benoit PAPILLAULT <benoit.papillault@free.fr>
> Signed-off-by: Felix Fietkau <nbd@openwrt.org>
> Signed-off-by: Luis Correia <luis.f.correia@gmail.com>
> Signed-off-by: Mattias Nissler <mattias.nissler@gmx.de>
> Signed-off-by: Mark Asselstine <asselsm@gmail.com>
> Signed-off-by: Xose Vazquez Perez <xose.vazquez@gmail.com>
> Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>
> ---
> http://kernel.org/pub/linux/kernel/people/ivd/patches/0003-rt2x00-Implement
> -support-for-rt2800pci.patch --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless"
>  in the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [rt2x00-users] [PATCH 2/2] rt2x00: Implement support for rt2800pci

[Ivo van Doorn]

Hi,

> I was comparing rt2800pci and the INF file of the original RT2860 windows 
> driver out curiosity. The original INF file contains 108 PCI id entries, 
> rt2800pci.c only 20. At first I thought that an entry like 
> 
> PCI\VEN_1814&DEV_0601&SUBSYS_77281432
> 
> (Edimax card) in the INF file will be automatically matched by rt2800pci for 
> the "1814:0601" vendor/device id, but then I found an additional entry for 
> "1432:7728" (the susbsystem id) in the rt2800pci pci_device_id table. 
> 
> I am a bit confused: Should every entry from the INF file have a PCI_DEVICE() 
> counterpart in our driver (resulting in 108 entries), or is this a special 
> case where the Manufacturer (Edimax) produces cards that actually have 
> "1432:7728" as vendor/device id.
> 
> You can probably tell that I don't have much experience developing kernel 
> drivers, but I am willing to get it right.

Well the main PCI and DEVICE id's should be listed in the PCI_DEVICE table,
the SUBSYS ID should only be added in case there can be different drivers
based on that ID.

As far as the ID 1432:7728, that one was send by Xose who got the ID from
the Windows driver, I don't know if he has mistakenly grabbed the subsystem ID or not...
Xose, could you give an update about this?

Ivo

Re: [rt2x00-users] [PATCH 2/2] rt2x00: Implement support for rt2800pci

[Xose Vazquez Perez]

On Fri, Oct 16, 2009 at 12:55, Ivo van Doorn <ivdoorn@gmail.com> wrote:

> Well the main PCI and DEVICE id's should be listed in the PCI_DEVICE table,
> the SUBSYS ID should only be added in case there can be different drivers
> based on that ID.
>
> As far as the ID 1432:7728, that one was send by Xose who got the ID from
> the Windows driver, I don't know if he has mistakenly grabbed the subsystem ID or not...
> Xose, could you give an update about this?

It came from ralink linux driver.
See 2009_0918_RT2860_Linux_STA_v2.2.0.0/os/linux/pci_main_dev.c:

        {PCI_DEVICE(NIC_PCI_VENDOR_ID, NIC2860_PCI_DEVICE_ID)},
 //RT28602.4G
        {PCI_DEVICE(NIC_PCI_VENDOR_ID, NIC2860_PCIe_DEVICE_ID)},
        {PCI_DEVICE(NIC_PCI_VENDOR_ID, NIC2760_PCI_DEVICE_ID)},
        {PCI_DEVICE(NIC_PCI_VENDOR_ID, NIC2790_PCIe_DEVICE_ID)},
        {PCI_DEVICE(VEN_AWT_PCI_VENDOR_ID, VEN_AWT_PCIe_DEVICE_ID)},
        {PCI_DEVICE(EDIMAX_PCI_VENDOR_ID, 0x7708)},
->>>>>    {PCI_DEVICE(EDIMAX_PCI_VENDOR_ID, 0x7728)},
        {PCI_DEVICE(EDIMAX_PCI_VENDOR_ID, 0x7758)},
        {PCI_DEVICE(EDIMAX_PCI_VENDOR_ID, 0x7727)},
        {PCI_DEVICE(EDIMAX_PCI_VENDOR_ID, 0x7738)},
        {PCI_DEVICE(EDIMAX_PCI_VENDOR_ID, 0x7748)},
        {PCI_DEVICE(EDIMAX_PCI_VENDOR_ID, 0x7768)},

regards,
-- 
«Allá muevan feroz guerra, ciegos reyes por un palmo más de tierra;
que yo aquí tengo por mío cuanto abarca el mar bravío, a quien nadie
impuso leyes. Y no hay playa, sea cualquiera, ni bandera de esplendor,
que no sienta mi derecho y dé pecho a mi valor.»

Re: [rt2x00-users] [PATCH 2/2] rt2x00: Implement support for rt2800pci

[Simon Raffeiner]

Am Freitag, 16. Oktober 2009 13:12:32 schrieb Xose Vazquez Perez:
> On Fri, Oct 16, 2009 at 12:55, Ivo van Doorn <ivdoorn@gmail.com> wrote:
> > Well the main PCI and DEVICE id's should be listed in the PCI_DEVICE
> > table, the SUBSYS ID should only be added in case there can be different
> > drivers based on that ID.
> >
> > As far as the ID 1432:7728, that one was sent by Xose who got the ID from
> > the Windows driver, I don't know if he has mistakenly grabbed the
> > subsystem ID or not... Xose, could you give an update about this?
> 
> It came from ralink linux driver.

Thanks, that explains it.

regards,
Simon Raffeiner

Re: [PATCH 2/2] rt2x00: Implement support for rt2800pci

[Bartlomiej Zolnierkiewicz]

[ I somehow missed this one by not being on cc: ]

On Thursday 15 October 2009 22:04:14 Ivo van Doorn wrote:
> Add support for the rt2860/rt3090 chipsets from Ralink.
> 
> Includes various patches from a lot of people who helped
> getting this driver into the current shape.
> 
> Signed-off-by: Alban Browaeys <prahal@yahoo.com>
> Signed-off-by: Benoit PAPILLAULT <benoit.papillault@free.fr>
> Signed-off-by: Felix Fietkau <nbd@openwrt.org>
> Signed-off-by: Luis Correia <luis.f.correia@gmail.com>
> Signed-off-by: Mattias Nissler <mattias.nissler@gmx.de>
> Signed-off-by: Mark Asselstine <asselsm@gmail.com>
> Signed-off-by: Xose Vazquez Perez <xose.vazquez@gmail.com>
> Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>
> ---
> http://kernel.org/pub/linux/kernel/people/ivd/patches/0003-rt2x00-Implement-support-for-rt2800pci.patch

First let me say that I'm very happy to see this patch finally being
submitted and I appreciate the effort..

(I'll give it a spin on Eee 901 w/ 2.6.32-rc5 sometime later..)


Now to the less happy part..

I also used the opportunity to take a closer look at this driver and
it seems that it needlessly adds around 2 KLOC to kernel by duplicating
the common content of rt2800usb.h to rt2800pci.h instead of moving it
to the shared header (like it is done in the staging crap drivers):

$ wc -l drivers/net/wireless/rt2x00/rt2800usb.h drivers/net/wireless/rt2x00/rt2800pci.h
  1951 drivers/net/wireless/rt2x00/rt2800usb.h
  1960 drivers/net/wireless/rt2x00/rt2800pci.h
  3911 total

$ diff -u drivers/net/wireless/rt2x00/rt2800usb.h drivers/net/wireless/rt2x00/rt2800pci.h|diffstat
 rt2800pci.h |  213 +++++++++++++++++++++++++++++++-----------------------------
  1 file changed, 111 insertions(+), 102 deletions(-)

Similarly it looks like most of the code between rt2800usb.c and rt2800pci.c
could also be shared (up to another 2 KLOC saved) by adding abstraction layer
for accessing chipset registers over different buses (again like it is done
in staging crap drivers):

$ wc -l drivers/net/wireless/rt2x00/rt2800usb.c drivers/net/wireless/rt2x00/rt2800pci.c
  3077 drivers/net/wireless/rt2x00/rt2800usb.c
  3323 drivers/net/wireless/rt2x00/rt2800pci.c
  6400 total

$ diff -u drivers/net/wireless/rt2x00/rt2800usb.c drivers/net/wireless/rt2x00/rt2800pci.c|diffstat
 rt2800pci.c | 2190 +++++++++++++++++++++++++++++++++---------------------------
 1 file changed, 1218 insertions(+), 972 deletions(-)

[ for better visualization of issues raised diffs themselves are available at:
  http://kernel.org/pub/linux/kernel/people/bart/rt2800/ ]

All in all, the total amount of the kernel code needed for implementing
rt2800pci functionality should 1-2 KLOC instead of the current 5 KLOC.

Re: [PATCH 2/2] rt2x00: Implement support for rt2800pci

[Bartlomiej Zolnierkiewicz]

[ I somehow missed this one by not being on cc: ]

On Thursday 15 October 2009 22:04:14 Ivo van Doorn wrote:
> Add support for the rt2860/rt3090 chipsets from Ralink.
> 
> Includes various patches from a lot of people who helped
> getting this driver into the current shape.
> 
> Signed-off-by: Alban Browaeys <prahal@yahoo.com>
> Signed-off-by: Benoit PAPILLAULT <benoit.papillault@free.fr>
> Signed-off-by: Felix Fietkau <nbd@openwrt.org>
> Signed-off-by: Luis Correia <luis.f.correia@gmail.com>
> Signed-off-by: Mattias Nissler <mattias.nissler@gmx.de>
> Signed-off-by: Mark Asselstine <asselsm@gmail.com>
> Signed-off-by: Xose Vazquez Perez <xose.vazquez@gmail.com>
> Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>
> ---
> http://kernel.org/pub/linux/kernel/people/ivd/patches/0003-rt2x00-Implement-support-for-rt2800pci.patch

First let me say that I'm very happy to see this patch finally being
submitted and I appreciate the effort..

(I'll give it a spin on Eee 901 w/ 2.6.32-rc5 sometime later..)


Now to the less happy part..

I also used the opportunity to take a closer look at this driver and
it seems that it needlessly adds around 2 KLOC to kernel by duplicating
the common content of rt2800usb.h to rt2800pci.h instead of moving it
to the shared header (like it is done in the staging crap drivers):

$ wc -l drivers/net/wireless/rt2x00/rt2800usb.h drivers/net/wireless/rt2x00/rt2800pci.h
  1951 drivers/net/wireless/rt2x00/rt2800usb.h
  1960 drivers/net/wireless/rt2x00/rt2800pci.h
  3911 total

$ diff -u drivers/net/wireless/rt2x00/rt2800usb.h drivers/net/wireless/rt2x00/rt2800pci.h|diffstat
 rt2800pci.h |  213 +++++++++++++++++++++++++++++++-----------------------------
  1 file changed, 111 insertions(+), 102 deletions(-)

Similarly it looks like most of the code between rt2800usb.c and rt2800pci.c
could also be shared (up to another 2 KLOC saved) by adding abstraction layer
for accessing chipset registers over different buses (again like it is done
in staging crap drivers):

$ wc -l drivers/net/wireless/rt2x00/rt2800usb.c drivers/net/wireless/rt2x00/rt2800pci.c
  3077 drivers/net/wireless/rt2x00/rt2800usb.c
  3323 drivers/net/wireless/rt2x00/rt2800pci.c
  6400 total

$ diff -u drivers/net/wireless/rt2x00/rt2800usb.c drivers/net/wireless/rt2x00/rt2800pci.c|diffstat
 rt2800pci.c | 2190 +++++++++++++++++++++++++++++++++---------------------------
 1 file changed, 1218 insertions(+), 972 deletions(-)

[ for better visualization of issues raised diffs themselves are available at:
  http://kernel.org/pub/linux/kernel/people/bart/rt2800/ ]

All in all, the total amount of the kernel code needed for implementing
rt2800pci functionality should 1-2 KLOC instead of the current 5 KLOC.

Re: [PATCH 2/2] rt2x00: Implement support for rt2800pci

[Johannes Berg]

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

On Sat, 2009-10-17 at 16:54 +0200, Bartlomiej Zolnierkiewicz wrote:

> I also used the opportunity to take a closer look at this driver and
> it seems that it needlessly adds around 2 KLOC to kernel by duplicating
> the common content of rt2800usb.h to rt2800pci.h instead of moving it
> to the shared header (like it is done in the staging crap drivers):

Tell me you're kidding -- comparing 2k duplicated LOC with a driver that
ships its own wifi stack?

johannes

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

Re: [PATCH 2/2] rt2x00: Implement support for rt2800pci

[Johannes Berg]

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

On Sat, 2009-10-17 at 16:54 +0200, Bartlomiej Zolnierkiewicz wrote:

> I also used the opportunity to take a closer look at this driver and
> it seems that it needlessly adds around 2 KLOC to kernel by duplicating
> the common content of rt2800usb.h to rt2800pci.h instead of moving it
> to the shared header (like it is done in the staging crap drivers):

Tell me you're kidding -- comparing 2k duplicated LOC with a driver that
ships its own wifi stack?

johannes

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

Re: [PATCH 2/2] rt2x00: Implement support for rt2800pci

[Bartlomiej Zolnierkiewicz]

On Saturday 17 October 2009 17:08:24 Johannes Berg wrote:
> On Sat, 2009-10-17 at 16:54 +0200, Bartlomiej Zolnierkiewicz wrote:
> 
> > I also used the opportunity to take a closer look at this driver and
> > it seems that it needlessly adds around 2 KLOC to kernel by duplicating
> > the common content of rt2800usb.h to rt2800pci.h instead of moving it
> > to the shared header (like it is done in the staging crap drivers):
> 
> Tell me you're kidding -- comparing 2k duplicated LOC with a driver that
> ships its own wifi stack?

Why would I be?

1) The patch is submitted to kernel _proper_ not kernel staging so I see
no excuse for duplicating 2-4 KLOC and it should be fixed.

2) The fact that the some staging driver consists in 90% of crap doesn't
mean that it doesn't have some good design ideas..  (i.e. abstracting chipset
registers access in a discussed case)

Re: [PATCH 2/2] rt2x00: Implement support for rt2800pci

[Bartlomiej Zolnierkiewicz]

On Saturday 17 October 2009 17:08:24 Johannes Berg wrote:
> On Sat, 2009-10-17 at 16:54 +0200, Bartlomiej Zolnierkiewicz wrote:
> 
> > I also used the opportunity to take a closer look at this driver and
> > it seems that it needlessly adds around 2 KLOC to kernel by duplicating
> > the common content of rt2800usb.h to rt2800pci.h instead of moving it
> > to the shared header (like it is done in the staging crap drivers):
> 
> Tell me you're kidding -- comparing 2k duplicated LOC with a driver that
> ships its own wifi stack?

Why would I be?

1) The patch is submitted to kernel _proper_ not kernel staging so I see
no excuse for duplicating 2-4 KLOC and it should be fixed.

2) The fact that the some staging driver consists in 90% of crap doesn't
mean that it doesn't have some good design ideas..  (i.e. abstracting chipset
registers access in a discussed case)

Re: [PATCH 2/2] rt2x00: Implement support for rt2800pci

[Bartlomiej Zolnierkiewicz]

On Saturday 17 October 2009 16:54:03 Bartlomiej Zolnierkiewicz wrote:
> 
> [ I somehow missed this one by not being on cc: ]
> 
> On Thursday 15 October 2009 22:04:14 Ivo van Doorn wrote:
> > Add support for the rt2860/rt3090 chipsets from Ralink.
> > 
> > Includes various patches from a lot of people who helped
> > getting this driver into the current shape.
> > 
> > Signed-off-by: Alban Browaeys <prahal@yahoo.com>
> > Signed-off-by: Benoit PAPILLAULT <benoit.papillault@free.fr>
> > Signed-off-by: Felix Fietkau <nbd@openwrt.org>
> > Signed-off-by: Luis Correia <luis.f.correia@gmail.com>
> > Signed-off-by: Mattias Nissler <mattias.nissler@gmx.de>
> > Signed-off-by: Mark Asselstine <asselsm@gmail.com>
> > Signed-off-by: Xose Vazquez Perez <xose.vazquez@gmail.com>
> > Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>
> > ---
> > http://kernel.org/pub/linux/kernel/people/ivd/patches/0003-rt2x00-Implement-support-for-rt2800pci.patch
> 
> First let me say that I'm very happy to see this patch finally being
> submitted and I appreciate the effort..
> 
> (I'll give it a spin on Eee 901 w/ 2.6.32-rc5 sometime later..)

Unfortunately still no luck with it, works just like it worked when I first
tried it half a year ago (loads OK but "iwlist wlan0 scan" doesn't bring any
results)..

Could somebody with the genuine RT2860 revision of the chip try the driver
and confirm that it works w/ 2.6.32-rc5 so I will know whether I should be
looking for some generic problem or RT2880 revision specific one?

Thanks.

Re: [PATCH 2/2] rt2x00: Implement support for rt2800pci

[Bartlomiej Zolnierkiewicz]

On Saturday 17 October 2009 16:54:03 Bartlomiej Zolnierkiewicz wrote:
> 
> [ I somehow missed this one by not being on cc: ]
> 
> On Thursday 15 October 2009 22:04:14 Ivo van Doorn wrote:
> > Add support for the rt2860/rt3090 chipsets from Ralink.
> > 
> > Includes various patches from a lot of people who helped
> > getting this driver into the current shape.
> > 
> > Signed-off-by: Alban Browaeys <prahal@yahoo.com>
> > Signed-off-by: Benoit PAPILLAULT <benoit.papillault@free.fr>
> > Signed-off-by: Felix Fietkau <nbd@openwrt.org>
> > Signed-off-by: Luis Correia <luis.f.correia@gmail.com>
> > Signed-off-by: Mattias Nissler <mattias.nissler@gmx.de>
> > Signed-off-by: Mark Asselstine <asselsm@gmail.com>
> > Signed-off-by: Xose Vazquez Perez <xose.vazquez@gmail.com>
> > Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>
> > ---
> > http://kernel.org/pub/linux/kernel/people/ivd/patches/0003-rt2x00-Implement-support-for-rt2800pci.patch
> 
> First let me say that I'm very happy to see this patch finally being
> submitted and I appreciate the effort..
> 
> (I'll give it a spin on Eee 901 w/ 2.6.32-rc5 sometime later..)

Unfortunately still no luck with it, works just like it worked when I first
tried it half a year ago (loads OK but "iwlist wlan0 scan" doesn't bring any
results)..

Could somebody with the genuine RT2860 revision of the chip try the driver
and confirm that it works w/ 2.6.32-rc5 so I will know whether I should be
looking for some generic problem or RT2880 revision specific one?

Thanks.

Re: [PATCH 2/2] rt2x00: Implement support for rt2800pci

[Luis Correia]

On Sat, Oct 17, 2009 at 22:18, Bartlomiej Zolnierkiewicz
<bzolnier@gmail.com> wrote:
> On Saturday 17 October 2009 16:54:03 Bartlomiej Zolnierkiewicz wrote:
>>
>> [ I somehow missed this one by not being on cc: ]
>>
>> On Thursday 15 October 2009 22:04:14 Ivo van Doorn wrote:
>> > Add support for the rt2860/rt3090 chipsets from Ralink.
>> >
>> > Includes various patches from a lot of people who helped
>> > getting this driver into the current shape.
>> >
>> > Signed-off-by: Alban Browaeys <prahal@yahoo.com>
>> > Signed-off-by: Benoit PAPILLAULT <benoit.papillault@free.fr>
>> > Signed-off-by: Felix Fietkau <nbd@openwrt.org>
>> > Signed-off-by: Luis Correia <luis.f.correia@gmail.com>
>> > Signed-off-by: Mattias Nissler <mattias.nissler@gmx.de>
>> > Signed-off-by: Mark Asselstine <asselsm@gmail.com>
>> > Signed-off-by: Xose Vazquez Perez <xose.vazquez@gmail.com>
>> > Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>
>> > ---
>> > http://kernel.org/pub/linux/kernel/people/ivd/patches/0003-rt2x00-Implement-support-for-rt2800pci.patch
>>
>> First let me say that I'm very happy to see this patch finally being
>> submitted and I appreciate the effort..
>>
>> (I'll give it a spin on Eee 901 w/ 2.6.32-rc5 sometime later..)
>
> Unfortunately still no luck with it, works just like it worked when I first
> tried it half a year ago (loads OK but "iwlist wlan0 scan" doesn't bring any
> results)..
>
> Could somebody with the genuine RT2860 revision of the chip try the driver
> and confirm that it works w/ 2.6.32-rc5 so I will know whether I should be
> looking for some generic problem or RT2880 revision specific one?

With my RaLink RT2760 Wireless 802.11n 1T/2R Cardbus [1814:0701], it
fails right after loading the firmware to the card.
(this is an internal PCI card, not cardbus)

Oct 16 23:08:15 lc kernel: phy0 -> rt2x00_set_chip: Info - Chipset
detected - rt: 0701, rf: 0003, rev: 28600102.
Oct 16 23:08:23 lc kernel: phy0 -> rt2x00lib_request_firmware: Info -
Loading firmware file 'rt2860.bin'.
Oct 16 23:08:23 lc kernel: phy0 -> rt2x00lib_request_firmware: Info -
Firmware detected - version: 0.11.
Oct 16 23:08:23 lc kernel: phy0 -> rt2x00pci_regbusy_read: Error -
Indirect register access failed: offset=0x00007010, value=0xffffffff
Oct 16 23:08:23 lc kernel: phy0 -> rt2x00pci_regbusy_read: Error -
Indirect register access failed: offset=0x00007010, value=0xffffffff
Oct 16 23:08:23 lc kernel: phy0 -> rt2x00pci_regbusy_read: Error -
Indirect register access failed: offset=0x00007010, value=0xffffffff
Oct 16 23:08:23 lc kernel: phy0 -> rt2x00mac_conf_tx: Info -
Configured TX queue 0 - CWmin: 3, CWmax: 4, Aifs: 2, TXop: 102.
Oct 16 23:08:23 lc kernel: phy0 -> rt2x00mac_conf_tx: Info -
Configured TX queue 1 - CWmin: 4, CWmax: 5, Aifs: 2, TXop: 188.
Oct 16 23:08:23 lc kernel: phy0 -> rt2x00mac_conf_tx: Info -
Configured TX queue 2 - CWmin: 5, CWmax: 10, Aifs: 3, TXop: 0.
Oct 16 23:08:23 lc kernel: phy0 -> rt2x00mac_conf_tx: Info -
Configured TX queue 3 - CWmin: 5, CWmax: 10, Aifs: 7, TXop: 0.

We get those "regbusy errors" and then, the MCU no longer accepts any command.

Oddly though, it can scan and detect the surrounding AP's.


Luis Correia,
rt2x0 project admin

Re: [PATCH 2/2] rt2x00: Implement support for rt2800pci

[Luis Correia]

On Sat, Oct 17, 2009 at 22:18, Bartlomiej Zolnierkiewicz
<bzolnier@gmail.com> wrote:
> On Saturday 17 October 2009 16:54:03 Bartlomiej Zolnierkiewicz wrote:
>>
>> [ I somehow missed this one by not being on cc: ]
>>
>> On Thursday 15 October 2009 22:04:14 Ivo van Doorn wrote:
>> > Add support for the rt2860/rt3090 chipsets from Ralink.
>> >
>> > Includes various patches from a lot of people who helped
>> > getting this driver into the current shape.
>> >
>> > Signed-off-by: Alban Browaeys <prahal@yahoo.com>
>> > Signed-off-by: Benoit PAPILLAULT <benoit.papillault@free.fr>
>> > Signed-off-by: Felix Fietkau <nbd@openwrt.org>
>> > Signed-off-by: Luis Correia <luis.f.correia@gmail.com>
>> > Signed-off-by: Mattias Nissler <mattias.nissler@gmx.de>
>> > Signed-off-by: Mark Asselstine <asselsm@gmail.com>
>> > Signed-off-by: Xose Vazquez Perez <xose.vazquez@gmail.com>
>> > Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>
>> > ---
>> > http://kernel.org/pub/linux/kernel/people/ivd/patches/0003-rt2x00-Implement-support-for-rt2800pci.patch
>>
>> First let me say that I'm very happy to see this patch finally being
>> submitted and I appreciate the effort..
>>
>> (I'll give it a spin on Eee 901 w/ 2.6.32-rc5 sometime later..)
>
> Unfortunately still no luck with it, works just like it worked when I first
> tried it half a year ago (loads OK but "iwlist wlan0 scan" doesn't bring any
> results)..
>
> Could somebody with the genuine RT2860 revision of the chip try the driver
> and confirm that it works w/ 2.6.32-rc5 so I will know whether I should be
> looking for some generic problem or RT2880 revision specific one?

With my RaLink RT2760 Wireless 802.11n 1T/2R Cardbus [1814:0701], it
fails right after loading the firmware to the card.
(this is an internal PCI card, not cardbus)

Oct 16 23:08:15 lc kernel: phy0 -> rt2x00_set_chip: Info - Chipset
detected - rt: 0701, rf: 0003, rev: 28600102.
Oct 16 23:08:23 lc kernel: phy0 -> rt2x00lib_request_firmware: Info -
Loading firmware file 'rt2860.bin'.
Oct 16 23:08:23 lc kernel: phy0 -> rt2x00lib_request_firmware: Info -
Firmware detected - version: 0.11.
Oct 16 23:08:23 lc kernel: phy0 -> rt2x00pci_regbusy_read: Error -
Indirect register access failed: offset=0x00007010, value=0xffffffff
Oct 16 23:08:23 lc kernel: phy0 -> rt2x00pci_regbusy_read: Error -
Indirect register access failed: offset=0x00007010, value=0xffffffff
Oct 16 23:08:23 lc kernel: phy0 -> rt2x00pci_regbusy_read: Error -
Indirect register access failed: offset=0x00007010, value=0xffffffff
Oct 16 23:08:23 lc kernel: phy0 -> rt2x00mac_conf_tx: Info -
Configured TX queue 0 - CWmin: 3, CWmax: 4, Aifs: 2, TXop: 102.
Oct 16 23:08:23 lc kernel: phy0 -> rt2x00mac_conf_tx: Info -
Configured TX queue 1 - CWmin: 4, CWmax: 5, Aifs: 2, TXop: 188.
Oct 16 23:08:23 lc kernel: phy0 -> rt2x00mac_conf_tx: Info -
Configured TX queue 2 - CWmin: 5, CWmax: 10, Aifs: 3, TXop: 0.
Oct 16 23:08:23 lc kernel: phy0 -> rt2x00mac_conf_tx: Info -
Configured TX queue 3 - CWmin: 5, CWmax: 10, Aifs: 7, TXop: 0.

We get those "regbusy errors" and then, the MCU no longer accepts any command.

Oddly though, it can scan and detect the surrounding AP's.


Luis Correia,
rt2x0 project admin

Re: [PATCH 2/2] rt2x00: Implement support for rt2800pci

[Julian Calaby]

On Sun, Oct 18, 2009 at 01:54, Bartlomiej Zolnierkiewicz
<bzolnier@gmail.com> wrote:
> Now to the less happy part..
>
> I also used the opportunity to take a closer look at this driver and
> it seems that it needlessly adds around 2 KLOC to kernel by duplicating
> the common content of rt2800usb.h to rt2800pci.h instead of moving it
> to the shared header (like it is done in the staging crap drivers):
>
> $ wc -l drivers/net/wireless/rt2x00/rt2800usb.h drivers/net/wireless/rt2x00/rt2800pci.h
>  1951 drivers/net/wireless/rt2x00/rt2800usb.h
>  1960 drivers/net/wireless/rt2x00/rt2800pci.h
>  3911 total
>
> $ diff -u drivers/net/wireless/rt2x00/rt2800usb.h drivers/net/wireless/rt2x00/rt2800pci.h|diffstat
>  rt2800pci.h |  213 +++++++++++++++++++++++++++++++-----------------------------
>  1 file changed, 111 insertions(+), 102 deletions(-)
>
> Similarly it looks like most of the code between rt2800usb.c and rt2800pci.c
> could also be shared (up to another 2 KLOC saved) by adding abstraction layer
> for accessing chipset registers over different buses (again like it is done
> in staging crap drivers):
>
> $ wc -l drivers/net/wireless/rt2x00/rt2800usb.c drivers/net/wireless/rt2x00/rt2800pci.c
>  3077 drivers/net/wireless/rt2x00/rt2800usb.c
>  3323 drivers/net/wireless/rt2x00/rt2800pci.c
>  6400 total
>
> $ diff -u drivers/net/wireless/rt2x00/rt2800usb.c drivers/net/wireless/rt2x00/rt2800pci.c|diffstat
>  rt2800pci.c | 2190 +++++++++++++++++++++++++++++++++---------------------------
>  1 file changed, 1218 insertions(+), 972 deletions(-)

Nobody else has said it, so:

Patches welcome!

Your points with the vendor drivers apply here too - consolidating the
common parts of the drivers will enhance readability, and make
development easier, and mean that, with your help, these might get to
a everyday usable state quicker.

And remember that this is a preliminary release of the code. There's
still a long way to go.

Thanks,

-- 

Julian Calaby

Email: julian.calaby@gmail.com
.Plan: http://sites.google.com/site/juliancalaby/

Re: [PATCH 2/2] rt2x00: Implement support for rt2800pci

[Julian Calaby]

On Sun, Oct 18, 2009 at 01:54, Bartlomiej Zolnierkiewicz
<bzolnier@gmail.com> wrote:
> Now to the less happy part..
>
> I also used the opportunity to take a closer look at this driver and
> it seems that it needlessly adds around 2 KLOC to kernel by duplicating
> the common content of rt2800usb.h to rt2800pci.h instead of moving it
> to the shared header (like it is done in the staging crap drivers):
>
> $ wc -l drivers/net/wireless/rt2x00/rt2800usb.h drivers/net/wireless/rt2x00/rt2800pci.h
>  1951 drivers/net/wireless/rt2x00/rt2800usb.h
>  1960 drivers/net/wireless/rt2x00/rt2800pci.h
>  3911 total
>
> $ diff -u drivers/net/wireless/rt2x00/rt2800usb.h drivers/net/wireless/rt2x00/rt2800pci.h|diffstat
>  rt2800pci.h |  213 +++++++++++++++++++++++++++++++-----------------------------
>  1 file changed, 111 insertions(+), 102 deletions(-)
>
> Similarly it looks like most of the code between rt2800usb.c and rt2800pci.c
> could also be shared (up to another 2 KLOC saved) by adding abstraction layer
> for accessing chipset registers over different buses (again like it is done
> in staging crap drivers):
>
> $ wc -l drivers/net/wireless/rt2x00/rt2800usb.c drivers/net/wireless/rt2x00/rt2800pci.c
>  3077 drivers/net/wireless/rt2x00/rt2800usb.c
>  3323 drivers/net/wireless/rt2x00/rt2800pci.c
>  6400 total
>
> $ diff -u drivers/net/wireless/rt2x00/rt2800usb.c drivers/net/wireless/rt2x00/rt2800pci.c|diffstat
>  rt2800pci.c | 2190 +++++++++++++++++++++++++++++++++---------------------------
>  1 file changed, 1218 insertions(+), 972 deletions(-)

Nobody else has said it, so:

Patches welcome!

Your points with the vendor drivers apply here too - consolidating the
common parts of the drivers will enhance readability, and make
development easier, and mean that, with your help, these might get to
a everyday usable state quicker.

And remember that this is a preliminary release of the code. There's
still a long way to go.

Thanks,

-- 

Julian Calaby

Email: julian.calaby@gmail.com
.Plan: http://sites.google.com/site/juliancalaby/

Re: [PATCH 2/2] rt2x00: Implement support for rt2800pci

[Ivo van Doorn]

> I also used the opportunity to take a closer look at this driver and
> it seems that it needlessly adds around 2 KLOC to kernel by duplicating
> the common content of rt2800usb.h to rt2800pci.h instead of moving it
> to the shared header (like it is done in the staging crap drivers):
> 
> $ wc -l drivers/net/wireless/rt2x00/rt2800usb.h drivers/net/wireless/rt2x00/rt2800pci.h
>   1951 drivers/net/wireless/rt2x00/rt2800usb.h
>   1960 drivers/net/wireless/rt2x00/rt2800pci.h
>   3911 total
> 
> $ diff -u drivers/net/wireless/rt2x00/rt2800usb.h drivers/net/wireless/rt2x00/rt2800pci.h|diffstat
>  rt2800pci.h |  213 +++++++++++++++++++++++++++++++-----------------------------
>   1 file changed, 111 insertions(+), 102 deletions(-)

Creating rt2800.h with common register defines has been on the todo list for some time already,
it will likely happen in the future, but until I get around to update my debugfs scripts the seperate
rt2800pci.h and rt2800usb.h files make debugging and register analysis a bit easier.

> Similarly it looks like most of the code between rt2800usb.c and rt2800pci.c
> could also be shared (up to another 2 KLOC saved) by adding abstraction layer
> for accessing chipset registers over different buses (again like it is done
> in staging crap drivers):
> 
> $ wc -l drivers/net/wireless/rt2x00/rt2800usb.c drivers/net/wireless/rt2x00/rt2800pci.c
>   3077 drivers/net/wireless/rt2x00/rt2800usb.c
>   3323 drivers/net/wireless/rt2x00/rt2800pci.c
>   6400 total
> 
> $ diff -u drivers/net/wireless/rt2x00/rt2800usb.c drivers/net/wireless/rt2x00/rt2800pci.c|diffstat
>  rt2800pci.c | 2190 +++++++++++++++++++++++++++++++++---------------------------
>  1 file changed, 1218 insertions(+), 972 deletions(-)

I don't agree on this, for starters the whole "abstraction layer as done in the staging driver, really
obfuscated the code in multiple areas, so whatever abstraction layer will be needed for rt2x00 it
should be done much cleaner without the ugly defines and crap like that. So unless there is a _clean_
and very _readable_ solution for such abstraction layer I might consider it.

Secondly, you can't merge them until both drivers would correctly for all users/chipsets, because only
then you can see what registers are initialized equaly, and where potential pitfalls are between the
2 busses.

> All in all, the total amount of the kernel code needed for implementing
> rt2800pci functionality should 1-2 KLOC instead of the current 5 KLOC.

Ivo

Re: [PATCH 2/2] rt2x00: Implement support for rt2800pci

[Ivo van Doorn]

> I also used the opportunity to take a closer look at this driver and
> it seems that it needlessly adds around 2 KLOC to kernel by duplicating
> the common content of rt2800usb.h to rt2800pci.h instead of moving it
> to the shared header (like it is done in the staging crap drivers):
> 
> $ wc -l drivers/net/wireless/rt2x00/rt2800usb.h drivers/net/wireless/rt2x00/rt2800pci.h
>   1951 drivers/net/wireless/rt2x00/rt2800usb.h
>   1960 drivers/net/wireless/rt2x00/rt2800pci.h
>   3911 total
> 
> $ diff -u drivers/net/wireless/rt2x00/rt2800usb.h drivers/net/wireless/rt2x00/rt2800pci.h|diffstat
>  rt2800pci.h |  213 +++++++++++++++++++++++++++++++-----------------------------
>   1 file changed, 111 insertions(+), 102 deletions(-)

Creating rt2800.h with common register defines has been on the todo list for some time already,
it will likely happen in the future, but until I get around to update my debugfs scripts the seperate
rt2800pci.h and rt2800usb.h files make debugging and register analysis a bit easier.

> Similarly it looks like most of the code between rt2800usb.c and rt2800pci.c
> could also be shared (up to another 2 KLOC saved) by adding abstraction layer
> for accessing chipset registers over different buses (again like it is done
> in staging crap drivers):
> 
> $ wc -l drivers/net/wireless/rt2x00/rt2800usb.c drivers/net/wireless/rt2x00/rt2800pci.c
>   3077 drivers/net/wireless/rt2x00/rt2800usb.c
>   3323 drivers/net/wireless/rt2x00/rt2800pci.c
>   6400 total
> 
> $ diff -u drivers/net/wireless/rt2x00/rt2800usb.c drivers/net/wireless/rt2x00/rt2800pci.c|diffstat
>  rt2800pci.c | 2190 +++++++++++++++++++++++++++++++++---------------------------
>  1 file changed, 1218 insertions(+), 972 deletions(-)

I don't agree on this, for starters the whole "abstraction layer as done in the staging driver, really
obfuscated the code in multiple areas, so whatever abstraction layer will be needed for rt2x00 it
should be done much cleaner without the ugly defines and crap like that. So unless there is a _clean_
and very _readable_ solution for such abstraction layer I might consider it.

Secondly, you can't merge them until both drivers would correctly for all users/chipsets, because only
then you can see what registers are initialized equaly, and where potential pitfalls are between the
2 busses.

> All in all, the total amount of the kernel code needed for implementing
> rt2800pci functionality should 1-2 KLOC instead of the current 5 KLOC.

Ivo

Re: [PATCH 2/2] rt2x00: Implement support for rt2800pci

[Bartlomiej Zolnierkiewicz]

On Sunday 18 October 2009 18:59:21 Ivo van Doorn wrote:
> > I also used the opportunity to take a closer look at this driver and
> > it seems that it needlessly adds around 2 KLOC to kernel by duplicating
> > the common content of rt2800usb.h to rt2800pci.h instead of moving it
> > to the shared header (like it is done in the staging crap drivers):
> > 
> > $ wc -l drivers/net/wireless/rt2x00/rt2800usb.h drivers/net/wireless/rt2x00/rt2800pci.h
> >   1951 drivers/net/wireless/rt2x00/rt2800usb.h
> >   1960 drivers/net/wireless/rt2x00/rt2800pci.h
> >   3911 total
> > 
> > $ diff -u drivers/net/wireless/rt2x00/rt2800usb.h drivers/net/wireless/rt2x00/rt2800pci.h|diffstat
> >  rt2800pci.h |  213 +++++++++++++++++++++++++++++++-----------------------------
> >   1 file changed, 111 insertions(+), 102 deletions(-)
> 
> Creating rt2800.h with common register defines has been on the todo list for some time already,
> it will likely happen in the future, but until I get around to update my debugfs scripts the seperate
> rt2800pci.h and rt2800usb.h files make debugging and register analysis a bit easier.

Knowing that this driver has been in more-or-less unchanged state in your
tree for at least past half year I would say that there was a plenty of time
to fix this trivial issue..

Also are the said scripts available anywhere?

> > Similarly it looks like most of the code between rt2800usb.c and rt2800pci.c
> > could also be shared (up to another 2 KLOC saved) by adding abstraction layer
> > for accessing chipset registers over different buses (again like it is done
> > in staging crap drivers):
> > 
> > $ wc -l drivers/net/wireless/rt2x00/rt2800usb.c drivers/net/wireless/rt2x00/rt2800pci.c
> >   3077 drivers/net/wireless/rt2x00/rt2800usb.c
> >   3323 drivers/net/wireless/rt2x00/rt2800pci.c
> >   6400 total
> > 
> > $ diff -u drivers/net/wireless/rt2x00/rt2800usb.c drivers/net/wireless/rt2x00/rt2800pci.c|diffstat
> >  rt2800pci.c | 2190 +++++++++++++++++++++++++++++++++---------------------------
> >  1 file changed, 1218 insertions(+), 972 deletions(-)
> 
> I don't agree on this, for starters the whole "abstraction layer as done in the staging driver, really
> obfuscated the code in multiple areas, so whatever abstraction layer will be needed for rt2x00 it
> should be done much cleaner without the ugly defines and crap like that. So unless there is a _clean_
> and very _readable_ solution for such abstraction layer I might consider it.

Nobody was talking about duplicating any obfuscations present in the staging
driver -- adding struct rt2800_ops would be the preferred way to go.

> Secondly, you can't merge them until both drivers would correctly for all users/chipsets, because only
> then you can see what registers are initialized equaly, and where potential pitfalls are between the
> 2 busses.

Merge should be done sooner than later, or the drivers will diverge even
more, i.e. eFUSE support is needed for rt2800usb and is currently present
only in rt2800pci driver.

You can always split them later, OTOH joining diverged code bases is much
more difficult task (i.e. unifying rt2860, rt2870, rt3070 and rt3090 was)

Moreover rt2800pci doesn't work at all currently (why it is not labeled as
EXPERIMENTAL BTW, ditto for rt2800usb?) so there is no rush in merging it
and you have a plenty of time to improve your code for the usual kernel
standards (alternatively you may want consider submitting it for staging).

Could you please start looking into addressing valid review comments?

[ If you do not have a time for it, my offer to take over maintenance of
  rt2800 drivers is still actual.. ]

Re: [PATCH 2/2] rt2x00: Implement support for rt2800pci

[Bartlomiej Zolnierkiewicz]

On Sunday 18 October 2009 18:59:21 Ivo van Doorn wrote:
> > I also used the opportunity to take a closer look at this driver and
> > it seems that it needlessly adds around 2 KLOC to kernel by duplicating
> > the common content of rt2800usb.h to rt2800pci.h instead of moving it
> > to the shared header (like it is done in the staging crap drivers):
> > 
> > $ wc -l drivers/net/wireless/rt2x00/rt2800usb.h drivers/net/wireless/rt2x00/rt2800pci.h
> >   1951 drivers/net/wireless/rt2x00/rt2800usb.h
> >   1960 drivers/net/wireless/rt2x00/rt2800pci.h
> >   3911 total
> > 
> > $ diff -u drivers/net/wireless/rt2x00/rt2800usb.h drivers/net/wireless/rt2x00/rt2800pci.h|diffstat
> >  rt2800pci.h |  213 +++++++++++++++++++++++++++++++-----------------------------
> >   1 file changed, 111 insertions(+), 102 deletions(-)
> 
> Creating rt2800.h with common register defines has been on the todo list for some time already,
> it will likely happen in the future, but until I get around to update my debugfs scripts the seperate
> rt2800pci.h and rt2800usb.h files make debugging and register analysis a bit easier.

Knowing that this driver has been in more-or-less unchanged state in your
tree for at least past half year I would say that there was a plenty of time
to fix this trivial issue..

Also are the said scripts available anywhere?

> > Similarly it looks like most of the code between rt2800usb.c and rt2800pci.c
> > could also be shared (up to another 2 KLOC saved) by adding abstraction layer
> > for accessing chipset registers over different buses (again like it is done
> > in staging crap drivers):
> > 
> > $ wc -l drivers/net/wireless/rt2x00/rt2800usb.c drivers/net/wireless/rt2x00/rt2800pci.c
> >   3077 drivers/net/wireless/rt2x00/rt2800usb.c
> >   3323 drivers/net/wireless/rt2x00/rt2800pci.c
> >   6400 total
> > 
> > $ diff -u drivers/net/wireless/rt2x00/rt2800usb.c drivers/net/wireless/rt2x00/rt2800pci.c|diffstat
> >  rt2800pci.c | 2190 +++++++++++++++++++++++++++++++++---------------------------
> >  1 file changed, 1218 insertions(+), 972 deletions(-)
> 
> I don't agree on this, for starters the whole "abstraction layer as done in the staging driver, really
> obfuscated the code in multiple areas, so whatever abstraction layer will be needed for rt2x00 it
> should be done much cleaner without the ugly defines and crap like that. So unless there is a _clean_
> and very _readable_ solution for such abstraction layer I might consider it.

Nobody was talking about duplicating any obfuscations present in the staging
driver -- adding struct rt2800_ops would be the preferred way to go.

> Secondly, you can't merge them until both drivers would correctly for all users/chipsets, because only
> then you can see what registers are initialized equaly, and where potential pitfalls are between the
> 2 busses.

Merge should be done sooner than later, or the drivers will diverge even
more, i.e. eFUSE support is needed for rt2800usb and is currently present
only in rt2800pci driver.

You can always split them later, OTOH joining diverged code bases is much
more difficult task (i.e. unifying rt2860, rt2870, rt3070 and rt3090 was)

Moreover rt2800pci doesn't work at all currently (why it is not labeled as
EXPERIMENTAL BTW, ditto for rt2800usb?) so there is no rush in merging it
and you have a plenty of time to improve your code for the usual kernel
standards (alternatively you may want consider submitting it for staging).

Could you please start looking into addressing valid review comments?

[ If you do not have a time for it, my offer to take over maintenance of
  rt2800 drivers is still actual.. ]

Re: [PATCH 2/2] rt2x00: Implement support for rt2800pci

[Ivo van Doorn]

> > > I also used the opportunity to take a closer look at this driver and
> > > it seems that it needlessly adds around 2 KLOC to kernel by duplicating
> > > the common content of rt2800usb.h to rt2800pci.h instead of moving it
> > > to the shared header (like it is done in the staging crap drivers):
> > > 
> > > $ wc -l drivers/net/wireless/rt2x00/rt2800usb.h drivers/net/wireless/rt2x00/rt2800pci.h
> > >   1951 drivers/net/wireless/rt2x00/rt2800usb.h
> > >   1960 drivers/net/wireless/rt2x00/rt2800pci.h
> > >   3911 total
> > > 
> > > $ diff -u drivers/net/wireless/rt2x00/rt2800usb.h drivers/net/wireless/rt2x00/rt2800pci.h|diffstat
> > >  rt2800pci.h |  213 +++++++++++++++++++++++++++++++-----------------------------
> > >   1 file changed, 111 insertions(+), 102 deletions(-)
> > 
> > Creating rt2800.h with common register defines has been on the todo list for some time already,
> > it will likely happen in the future, but until I get around to update my debugfs scripts the seperate
> > rt2800pci.h and rt2800usb.h files make debugging and register analysis a bit easier.
> 
> Knowing that this driver has been in more-or-less unchanged state in your
> tree for at least past half year I would say that there was a plenty of time
> to fix this trivial issue..

Well good to see you actually read the mails I send in this, and the previous, discussion regarding
the rt2800 development. Because that would have answered your question regarding the "plenty of time" part.

> Also are the said scripts available anywhere?

http://kernel.org/pub/linux/kernel/people/ivd/tools/

There is no howto for the scripts...
I have an updated script somewhere, but that is mostly performance fixes.

> > Secondly, you can't merge them until both drivers would correctly for all users/chipsets, because only
> > then you can see what registers are initialized equaly, and where potential pitfalls are between the
> > 2 busses.
> 
> Merge should be done sooner than later, or the drivers will diverge even
> more, i.e. eFUSE support is needed for rt2800usb and is currently present
> only in rt2800pci driver.

Well nobody mentioned which USB chipsets require the eFuse EEPROM,
and I am not adding it until that is figured out.

> You can always split them later, OTOH joining diverged code bases is much
> more difficult task (i.e. unifying rt2860, rt2870, rt3070 and rt3090 was)

There have been way to many problems with the rt2800pci/usb drivers where
the ordering of the register initialization really matters. And those parts seem
to be different between rt2800pci/usb or at least they depend on different registers
to make it work.
So until that has been figured out we can try to unify the drivers then,

> Moreover rt2800pci doesn't work at all currently (why it is not labeled as
> EXPERIMENTAL BTW, ditto for rt2800usb?)

+config RT2800PCI
+	tristate "Ralink rt2800 (PCI/PCMCIA) support"
+	depends on (RT2800PCI_PCI || RT2800PCI_SOC) && EXPERIMENTAL
                                                                                            ^^^^^^^^^

The same exists for RT2800USB....

> so there is no rush in merging it 
> and you have a plenty of time to improve your code for the usual kernel
> standards (alternatively you may want consider submitting it for staging).
>
> Could you please start looking into addressing valid review comments?

Haven't I been answering all your points already?

> [ If you do not have a time for it, my offer to take over maintenance of
>   rt2800 drivers is still actual.. ]

If I am going to hand over the maintainership to somebody else
(and don't worry I will in the near future), I will hand it over to somebody
who has experience with the rt2x00 driver (i.e. actually wrote code for
the drivers).
The second requirement is that the new maintainer needs to be interested
in _all_ rt2x00 drivers, and is willing to give the priority of the development
to those drivers rather then then staging drivers.

Ivo

Re: [PATCH 2/2] rt2x00: Implement support for rt2800pci

[Ivo van Doorn]

> > > I also used the opportunity to take a closer look at this driver and
> > > it seems that it needlessly adds around 2 KLOC to kernel by duplicating
> > > the common content of rt2800usb.h to rt2800pci.h instead of moving it
> > > to the shared header (like it is done in the staging crap drivers):
> > > 
> > > $ wc -l drivers/net/wireless/rt2x00/rt2800usb.h drivers/net/wireless/rt2x00/rt2800pci.h
> > >   1951 drivers/net/wireless/rt2x00/rt2800usb.h
> > >   1960 drivers/net/wireless/rt2x00/rt2800pci.h
> > >   3911 total
> > > 
> > > $ diff -u drivers/net/wireless/rt2x00/rt2800usb.h drivers/net/wireless/rt2x00/rt2800pci.h|diffstat
> > >  rt2800pci.h |  213 +++++++++++++++++++++++++++++++-----------------------------
> > >   1 file changed, 111 insertions(+), 102 deletions(-)
> > 
> > Creating rt2800.h with common register defines has been on the todo list for some time already,
> > it will likely happen in the future, but until I get around to update my debugfs scripts the seperate
> > rt2800pci.h and rt2800usb.h files make debugging and register analysis a bit easier.
> 
> Knowing that this driver has been in more-or-less unchanged state in your
> tree for at least past half year I would say that there was a plenty of time
> to fix this trivial issue..

Well good to see you actually read the mails I send in this, and the previous, discussion regarding
the rt2800 development. Because that would have answered your question regarding the "plenty of time" part.

> Also are the said scripts available anywhere?

http://kernel.org/pub/linux/kernel/people/ivd/tools/

There is no howto for the scripts...
I have an updated script somewhere, but that is mostly performance fixes.

> > Secondly, you can't merge them until both drivers would correctly for all users/chipsets, because only
> > then you can see what registers are initialized equaly, and where potential pitfalls are between the
> > 2 busses.
> 
> Merge should be done sooner than later, or the drivers will diverge even
> more, i.e. eFUSE support is needed for rt2800usb and is currently present
> only in rt2800pci driver.

Well nobody mentioned which USB chipsets require the eFuse EEPROM,
and I am not adding it until that is figured out.

> You can always split them later, OTOH joining diverged code bases is much
> more difficult task (i.e. unifying rt2860, rt2870, rt3070 and rt3090 was)

There have been way to many problems with the rt2800pci/usb drivers where
the ordering of the register initialization really matters. And those parts seem
to be different between rt2800pci/usb or at least they depend on different registers
to make it work.
So until that has been figured out we can try to unify the drivers then,

> Moreover rt2800pci doesn't work at all currently (why it is not labeled as
> EXPERIMENTAL BTW, ditto for rt2800usb?)

+config RT2800PCI
+	tristate "Ralink rt2800 (PCI/PCMCIA) support"
+	depends on (RT2800PCI_PCI || RT2800PCI_SOC) && EXPERIMENTAL
                                                                                            ^^^^^^^^^

The same exists for RT2800USB....

> so there is no rush in merging it 
> and you have a plenty of time to improve your code for the usual kernel
> standards (alternatively you may want consider submitting it for staging).
>
> Could you please start looking into addressing valid review comments?

Haven't I been answering all your points already?

> [ If you do not have a time for it, my offer to take over maintenance of
>   rt2800 drivers is still actual.. ]

If I am going to hand over the maintainership to somebody else
(and don't worry I will in the near future), I will hand it over to somebody
who has experience with the rt2x00 driver (i.e. actually wrote code for
the drivers).
The second requirement is that the new maintainer needs to be interested
in _all_ rt2x00 drivers, and is willing to give the priority of the development
to those drivers rather then then staging drivers.

Ivo

Re: [PATCH 2/2] rt2x00: Implement support for rt2800pci

[Holger Schurig]

> I don't agree on this, for starters the whole "abstraction
> layer as done in the staging driver, really obfuscated the code
> in multiple areas 

Ivo, you could look at Orinoco or Libertas. Both WLAN drivers 
support a multitude of different hardware (Libertas: CF/PCMCIA, 
SDIO, SD, USB and Orinoco: CF/PCMCIA, PCI, PPC_PMAC). And both 
have hardware abstraction layers that don't suck, obfuscate or 
create lots of duplicate code.

So AFAIK it's not the question *IF* to do hardware abstraction 
but only a question *HOW* to do it in an intelligent way. Don't 
luck at one bad implementation and disregard the whole 
concept :-)

-- 
http://www.holgerschurig.de

Re: [PATCH 2/2] rt2x00: Implement support for rt2800pci

[Holger Schurig]

> I don't agree on this, for starters the whole "abstraction
> layer as done in the staging driver, really obfuscated the code
> in multiple areas 

Ivo, you could look at Orinoco or Libertas. Both WLAN drivers 
support a multitude of different hardware (Libertas: CF/PCMCIA, 
SDIO, SD, USB and Orinoco: CF/PCMCIA, PCI, PPC_PMAC). And both 
have hardware abstraction layers that don't suck, obfuscate or 
create lots of duplicate code.

So AFAIK it's not the question *IF* to do hardware abstraction 
but only a question *HOW* to do it in an intelligent way. Don't 
luck at one bad implementation and disregard the whole 
concept :-)

-- 
http://www.holgerschurig.de

Re: [PATCH 2/2] rt2x00: Implement support for rt2800pci

[Ivo van Doorn]

On Tuesday 20 October 2009, Holger Schurig wrote:
> > I don't agree on this, for starters the whole "abstraction
> > layer as done in the staging driver, really obfuscated the code
> > in multiple areas 
> 
> Ivo, you could look at Orinoco or Libertas. Both WLAN drivers 
> support a multitude of different hardware (Libertas: CF/PCMCIA, 
> SDIO, SD, USB and Orinoco: CF/PCMCIA, PCI, PPC_PMAC). And both 
> have hardware abstraction layers that don't suck, obfuscate or 
> create lots of duplicate code.

Thanks,

> So AFAIK it's not the question *IF* to do hardware abstraction 
> but only a question *HOW* to do it in an intelligent way. Don't 
> luck at one bad implementation and disregard the whole 
> concept :-)

Oh I completely agree, I am not against the extra abstraction layer,
but I do want a nice looking solution. :) I'll take a look at the Orinoco approach.

Thanks,

Ivo

Re: [PATCH 2/2] rt2x00: Implement support for rt2800pci

[Ivo van Doorn]

On Tuesday 20 October 2009, Holger Schurig wrote:
> > I don't agree on this, for starters the whole "abstraction
> > layer as done in the staging driver, really obfuscated the code
> > in multiple areas 
> 
> Ivo, you could look at Orinoco or Libertas. Both WLAN drivers 
> support a multitude of different hardware (Libertas: CF/PCMCIA, 
> SDIO, SD, USB and Orinoco: CF/PCMCIA, PCI, PPC_PMAC). And both 
> have hardware abstraction layers that don't suck, obfuscate or 
> create lots of duplicate code.

Thanks,

> So AFAIK it's not the question *IF* to do hardware abstraction 
> but only a question *HOW* to do it in an intelligent way. Don't 
> luck at one bad implementation and disregard the whole 
> concept :-)

Oh I completely agree, I am not against the extra abstraction layer,
but I do want a nice looking solution. :) I'll take a look at the Orinoco approach.

Thanks,

Ivo