On 2012-10-25 17:19, Greg Kroah-Hartman wrote:
On Thu, Oct 25, 2012 at 03:02:37PM +0100, Ian Abbott wrote:
On 2012-10-24 15:19, Ian Abbott wrote:
On 2012-10-24 05:16, Greg Kroah-Hartman wrote:
On Tue, Oct 23, 2012 at 10:53:22AM +0100, Ian Abbott wrote:
On 2012-10-22 20:44, Greg Kroah-Hartman wrote:
On Mon, Oct 15, 2012 at 01:07:30PM +0100, Ian Abbott wrote:
The auto-configuration/auto-attach mechanism in Comedi used by most of
the low-level PCI and USB Comedi hardware drivers currently uses
separate hooks in `struct comedi_driver` for each bus type -
`->attach_usb()` and `->attach_pci()`. I'd like to move away from that
and use a single auto-attach hook, `->auto_attach()` and make it easy
for an implementation of this hook in a low-level driver to get a
pointer to the `struct pci_dev`, `struct usb_interface` or whatever
device wrapper is relevant to the device being auto-attached to comedi.
Also, the Comedi core exports nice wrapper functions for auto-attaching
PCI or USB devices (`comedi_pci_auto_config()` and
`comedi_usb_auto_config()`) along with functions for auto-detaching
them, but there is currently no way to auto-attach devices on some
arbitrary type of bus. Export functions `comedi_auto_config()`
(different to the current static function of the same name) and
`comedi_auto_unconfig()`) to allow this.
Eventually, there will be nice wrapper functions for some more commonly
used device types such as PCMCIA, but it should be easy enough to
implement those as static inline wrappers around `comedi_auto_config()`
and `comedi_auto_unconfig()`.
This patch series only changes one PCI driver and one USB driver to use
the new `auto_attach()` hook, but it will be easy enough to change the
others. Once that is done, the `attach_pci()` and `attach_usb()` hooks
can be removed from `struct comedi_driver`.
I took the first two patches here only, care to rework the rest and
resend them?
I'm prepared to discuss the patches. One thing I want to allow is
for drivers to call comedi_auto_config() directly using whatever
context value they want (as long as it can be represented by an
unsigned long), and to have the same value passed back in their
auto_attach routine.
Ok, as long as nothing else outside that driver is trying to intrepret
that value as meaning anything.
Only the driver would interpret the context value, although most would
just ignore it. If the driver called comedi_auto_config() directly, it
would get the context back in its ->auto_config() hook. If the driver
used one of the wrappers around comedi_auto_config() such as
comedi_pci_auto_config(), it would get the predefined magic value as the
context "for free"; well, the context parameter would have to be set to
_something_ and some magic value that depends on which wrapper was
called is possibly more useful than a plain old 0 or NULL value.
Having comedi_pci_auto_config() pass a pointer to the `struct pci_dev`
in the context (and similar for other wrappers around
comedi_auto_config() handling different device types) wouldn't be
terribly useful to an ->auto_config() hook that could handle multiple
types of device wrappers as it wouldn't be able to tell what sort of
device type it should be dealing with as one random pointer value looks
pretty much like any other.
Okay, I'm just looking for a final decision before redoing the
patches (if necessary).
(1) Should drivers calling comedi_auto_config() directly be able to
pass a context value and have the same context value passed back via
their auto_attach() method?
Yes.
(2) If 1, what is the most useful type for that context value?
I think you got it right with an unsigned long.
(3) If 1, should pre-defined wrappers around comedi_auto_config()
such as comedi_pci_auto_config() and comedi_usb_auto_config() pass
pre-defined, "magic" values (which the driver will probably ignore)
as the context value in the driver's auto_attach() method or just
pass some "null" value?
That's trickier. I don't know enough about the comedi layer to answer
this, but generally, I would stay away from having anyone except the
driver that set the value, be the ones that can do anything with the
value. In other words, no magic flags here.
If you want a "magic" flag, use a new variable for it and make it an
enumerated type.
It's just that if comedi_pci_auto_config() ends up calling the driver's
auto_attach() method (and that will be the only option eventually), the
context parameter value of the driver's auto_attach() method either has
to be conjured out of thin air (i.e. it's something the driver never
really asked for in the first place and probably has no interest in
unless it handles multiple types of devices such as PCI and PCMCIA), or
we'd also have to add the context parameter to comedi_pci_auto_config()
and pass it through.
Perhaps comedi_pci_auto_config() &co. should just set the context to 0
for now (which is really just a _different_ context value the driver
never asked for!).
--
-=( Ian Abbott @ MEV Ltd. E-mail: <abbotti@xxxxxxxxx> )=-
-=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=-
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
