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. thanks, greg k-h _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
