Hi!
Patch against 2.4.0-test11-pre1. It replaces check_region() by
request_region(), fixes some small bugs and tries to cleanup a bit
radio drivers... I have just seen 240t11p1ac1 and I think this
patch is superior then ac's radio part ;)
Please apply...
--
Bartlomiej Zolnierkiewicz
<[email protected]>
Patch looks generally ok. Some of the whitespace/formatting changes are
questionable, I usually leave that up to the maintainer unless it is
very gratuitously opposite to CodingStyle.
Some of the driver messages ("foo version 1.0") are purposefully printed
-after-, not before, the device is probed and registered. Your patch
gets this wrong in at least one place.
Finally, a word to you, Alan, and others doing request_region work: it
is more informative to pass the device name (minor, etc.) into
request_region. Ditto for request_irq. Many (most, except net?)
drivers use board/chip name instead of registered interface name. If
you can use the interface name for request_region or request_irq, use
it... it allows differentiation between multiple boards of the same
type. That's especially when looking at ISA regions in /proc/ioports,
or interrupt counts in /proc/interrupts.
Jeff
--
Jeff Garzik |
Building 1024 | Would you like a Twinkie?
MandrakeSoft |
On Wed, 8 Nov 2000, Jeff Garzik wrote:
> Patch looks generally ok. Some of the whitespace/formatting changes are
> questionable, I usually leave that up to the maintainer unless it is
> very gratuitously opposite to CodingStyle.
>
These drivers seem to be unmantained :)
Anyway if this is a problem I can undo these changes ...
> Some of the driver messages ("foo version 1.0") are purposefully printed
> -after-, not before, the device is probed and registered. Your patch
> gets this wrong in at least one place.
>
Yes... I wasn't sure about this... can undo...
> Finally, a word to you, Alan, and others doing request_region work: it
> is more informative to pass the device name (minor, etc.) into
> request_region. Ditto for request_irq. Many (most, except net?)
> drivers use board/chip name instead of registered interface name. If
> you can use the interface name for request_region or request_irq, use
> it... it allows differentiation between multiple boards of the same
> type. That's especially when looking at ISA regions in /proc/ioports,
> or interrupt counts in /proc/interrupts.
>
> Jeff
Agree... but in this case it's less important until radio drivers
supports multiple boards...
thanks
--
Bartlomiej Zolnierkiewicz
<[email protected]>
Bartlomiej Zolnierkiewicz wrote:
>
> On Wed, 8 Nov 2000, Jeff Garzik wrote:
>
> > Patch looks generally ok. Some of the whitespace/formatting changes are
> > questionable, I usually leave that up to the maintainer unless it is
> > very gratuitously opposite to CodingStyle.
> >
>
> These drivers seem to be unmantained :)
> Anyway if this is a problem I can undo these changes ...
I don't have any problem with them. Make sure you CC the individual
maintainers of the drivers... some of the pop up every now and then :)
> > Some of the driver messages ("foo version 1.0") are purposefully printed
> > -after-, not before, the device is probed and registered. Your patch
> > gets this wrong in at least one place.
> >
>
> Yes... I wasn't sure about this... can undo...
When in doubt, follow the behavior of the existing driver. :)
Especially since we're in a freeze, and stuff...
--
Jeff Garzik |
Building 1024 | Would you like a Twinkie?
MandrakeSoft |
On Wed, Nov 08, 2000 at 07:13:46PM -0500, Jeff Garzik wrote:
Hi all,
>
> Finally, a word to you, Alan, and others doing request_region work: it
> is more informative to pass the device name (minor, etc.) into
> request_region. Ditto for request_irq. Many (most, except net?)
> drivers use board/chip name instead of registered interface name. If
> you can use the interface name for request_region or request_irq, use
> it... it allows differentiation between multiple boards of the same
> type. That's especially when looking at ISA regions in /proc/ioports,
> or interrupt counts in /proc/interrupts.
>
two question about this:
1) how about drivers requesting 2 (or more) irq for one device ?
AFAIK some PowerMac net drivers do it (bmac.c for example).
2) i found that some net drivers (3c527.c, sk_mca.c) use io region and
don't call request_region() at all. Should they be fixed ?
Best regards,
Andrey
Andrey Panin wrote:
> 1) how about drivers requesting 2 (or more) irq for one device ?
> AFAIK some PowerMac net drivers do it (bmac.c for example).
Should be fine.. If the driver distinguishes between the irqs, maybe
you should do "eth0-rx dma", "eth0-tx dma", etc.
> 2) i found that some net drivers (3c527.c, sk_mca.c) use io region and
> don't call request_region() at all. Should they be fixed ?
Probably... but there may be a reason for that, too.
drivers/net/atp.c, for example, does not use request_region because it
uses the standard parallel ports. (ideally, of course, it should use
the parport API...)
Jeff
--
Jeff Garzik |
Building 1024 | Would you like a Twinkie?
MandrakeSoft |
> 2) i found that some net drivers (3c527.c, sk_mca.c) use io region and
> don't call request_region() at all. Should they be fixed ?
Probably.
MCA bus ensures there can be no collisions of I/O space but it does mean the
user cannot see what is where as is
Alan Cox wrote:
>
> > 2) i found that some net drivers (3c527.c, sk_mca.c) use io region and
> > don't call request_region() at all. Should they be fixed ?
>
> Probably.
>
> MCA bus ensures there can be no collisions of I/O space but it does mean the
> user cannot see what is where as is
Ditto for PCI... it's also a good idea to do it so that another driver
doesn't trample on your I/O space. I don't think there are any
de4x5/tulip type situations for MCA, but ya never know...
--
Jeff Garzik |
Building 1024 | Would you like a Twinkie?
MandrakeSoft |
[ radio cards ]
> These drivers seem to be unmantained :)
Erm, no. I'm still behind the radio-aztech driver plus my mods on
radio-aimslab and radio-cadet. Calling them unmaintained is going too
far. As for the others, that's up to their respective authors.
I use the cadet every couple of days on my 2.4 box so any brokenness will
stand out right away. If the aztech and aimslab drivers have failed
recently, then it's possible that it has escaped detection. Otherwise I
generally leave the code alone since it's been pretty stable.
Please keep me in the loop on these things.