* Re: [PATCH 2/2] rt2x00: Implement support for rt2800pci
2009-10-17 14:54 ` Bartlomiej Zolnierkiewicz
@ 2009-10-17 15:08 ` Johannes Berg
2009-10-17 15:19 ` Bartlomiej Zolnierkiewicz
2009-10-17 21:18 ` Bartlomiej Zolnierkiewicz
` (2 subsequent siblings)
3 siblings, 1 reply; 16+ messages in thread
From: Johannes Berg @ 2009-10-17 15:08 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz
Cc: Ivo van Doorn, John Linville, linux-wireless, users,
Alban Browaeys, Benoit PAPILLAULT, Felix Fietkau, Luis Correia,
Mattias Nissler, Mark Asselstine, Xose Vazquez Perez,
linux-kernel
[-- 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 --]
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 2/2] rt2x00: Implement support for rt2800pci
2009-10-17 15:08 ` Johannes Berg
@ 2009-10-17 15:19 ` Bartlomiej Zolnierkiewicz
0 siblings, 0 replies; 16+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2009-10-17 15:19 UTC (permalink / raw)
To: Johannes Berg
Cc: Ivo van Doorn, John Linville, linux-wireless, users,
Alban Browaeys, Benoit PAPILLAULT, Felix Fietkau, Luis Correia,
Mattias Nissler, Mark Asselstine, Xose Vazquez Perez,
linux-kernel
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)
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] rt2x00: Implement support for rt2800pci
2009-10-17 14:54 ` Bartlomiej Zolnierkiewicz
2009-10-17 15:08 ` Johannes Berg
@ 2009-10-17 21:18 ` Bartlomiej Zolnierkiewicz
2009-10-18 9:40 ` Luis Correia
2009-10-18 3:08 ` Julian Calaby
2009-10-18 16:59 ` Ivo van Doorn
3 siblings, 1 reply; 16+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2009-10-17 21:18 UTC (permalink / raw)
To: Ivo van Doorn
Cc: John Linville, linux-wireless, users, Alban Browaeys,
Benoit PAPILLAULT, Felix Fietkau, Luis Correia, Mattias Nissler,
Mark Asselstine, Xose Vazquez Perez, linux-kernel
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.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] rt2x00: Implement support for rt2800pci
2009-10-17 21:18 ` Bartlomiej Zolnierkiewicz
@ 2009-10-18 9:40 ` Luis Correia
0 siblings, 0 replies; 16+ messages in thread
From: Luis Correia @ 2009-10-18 9:40 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz
Cc: Ivo van Doorn, John Linville, linux-wireless, users,
Alban Browaeys, Benoit PAPILLAULT, Felix Fietkau, Mattias Nissler,
Mark Asselstine, Xose Vazquez Perez, linux-kernel
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] rt2x00: Implement support for rt2800pci
2009-10-17 14:54 ` Bartlomiej Zolnierkiewicz
2009-10-17 15:08 ` Johannes Berg
2009-10-17 21:18 ` Bartlomiej Zolnierkiewicz
@ 2009-10-18 3:08 ` Julian Calaby
2009-10-18 16:59 ` Ivo van Doorn
3 siblings, 0 replies; 16+ messages in thread
From: Julian Calaby @ 2009-10-18 3:08 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz
Cc: Ivo van Doorn, John Linville, linux-wireless, users,
Alban Browaeys, Benoit PAPILLAULT, Felix Fietkau, Luis Correia,
Mattias Nissler, Mark Asselstine, Xose Vazquez Perez,
linux-kernel
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/
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] rt2x00: Implement support for rt2800pci
2009-10-17 14:54 ` Bartlomiej Zolnierkiewicz
` (2 preceding siblings ...)
2009-10-18 3:08 ` Julian Calaby
@ 2009-10-18 16:59 ` Ivo van Doorn
2009-10-19 15:56 ` Bartlomiej Zolnierkiewicz
2009-10-20 6:58 ` Holger Schurig
3 siblings, 2 replies; 16+ messages in thread
From: Ivo van Doorn @ 2009-10-18 16:59 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz
Cc: John Linville, linux-wireless, users, Alban Browaeys,
Benoit PAPILLAULT, Felix Fietkau, Luis Correia, Mattias Nissler,
Mark Asselstine, Xose Vazquez Perez, linux-kernel
> 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
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 2/2] rt2x00: Implement support for rt2800pci
2009-10-18 16:59 ` Ivo van Doorn
@ 2009-10-19 15:56 ` Bartlomiej Zolnierkiewicz
2009-10-19 17:42 ` Ivo van Doorn
2009-10-20 6:58 ` Holger Schurig
1 sibling, 1 reply; 16+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2009-10-19 15:56 UTC (permalink / raw)
To: Ivo van Doorn
Cc: John Linville, linux-wireless, users, Alban Browaeys,
Benoit PAPILLAULT, Felix Fietkau, Luis Correia, Mattias Nissler,
Mark Asselstine, Xose Vazquez Perez, linux-kernel
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.. ]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] rt2x00: Implement support for rt2800pci
2009-10-19 15:56 ` Bartlomiej Zolnierkiewicz
@ 2009-10-19 17:42 ` Ivo van Doorn
0 siblings, 0 replies; 16+ messages in thread
From: Ivo van Doorn @ 2009-10-19 17:42 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz
Cc: John Linville, linux-wireless, users, Alban Browaeys,
Benoit PAPILLAULT, Felix Fietkau, Luis Correia, Mattias Nissler,
Mark Asselstine, Xose Vazquez Perez, linux-kernel
> > > 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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] rt2x00: Implement support for rt2800pci
2009-10-18 16:59 ` Ivo van Doorn
2009-10-19 15:56 ` Bartlomiej Zolnierkiewicz
@ 2009-10-20 6:58 ` Holger Schurig
2009-10-20 16:31 ` Ivo van Doorn
1 sibling, 1 reply; 16+ messages in thread
From: Holger Schurig @ 2009-10-20 6:58 UTC (permalink / raw)
To: Ivo van Doorn
Cc: Bartlomiej Zolnierkiewicz, John Linville, linux-wireless, users,
Alban Browaeys, Benoit PAPILLAULT, Felix Fietkau, Luis Correia,
Mattias Nissler, Mark Asselstine, Xose Vazquez Perez,
linux-kernel
> 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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] rt2x00: Implement support for rt2800pci
2009-10-20 6:58 ` Holger Schurig
@ 2009-10-20 16:31 ` Ivo van Doorn
0 siblings, 0 replies; 16+ messages in thread
From: Ivo van Doorn @ 2009-10-20 16:31 UTC (permalink / raw)
To: Holger Schurig
Cc: Bartlomiej Zolnierkiewicz, John Linville, linux-wireless, users,
Alban Browaeys, Benoit PAPILLAULT, Felix Fietkau, Luis Correia,
Mattias Nissler, Mark Asselstine, Xose Vazquez Perez,
linux-kernel
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
^ permalink raw reply [flat|nested] 16+ messages in thread