Re: [PATCH 0/9] staging: comedi: update auto-attach mechanism

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 25/10/12 23:29, H Hartley Sweeten wrote:
On Thursday, October 25, 2012 10:03 AM, Ian Abbott wrote:
On 2012-10-25 17:19, Greg Kroah-Hartman wrote:
On Thu, Oct 25, 2012 at 03:02:37PM +0100, Ian Abbott wrote:
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,

At this point I think you should repost an updated patch.

It looks like an unsigned long type is correct for the 'context'. But, only
the driver should be using this 'context'. It's the only user that really
knows what it is.

If you still need to pass a "magic" value for drivers that could auto attach
different types of drivers, I think you need to add an additional parameter
to the auto_attach() and pass it as an enumerated type. Most drivers will
simply ignore both the context and the flag.

The drivers that need the flag can check it to see if they need to use one
of the comedi_to_* helpers to get the context from the dev->hw_dev
or if the 'context' passed is the drivers private context.

Hmm.. make about as much sense to me as a mud milkshake....

To do that, I'd have to have an internal version of comedi_auto_attach() (say __comedi_auto_attach()) that had one more parameter of an enumerated type that indicated what called it, either comedi_auto_attach(), comedi_pci_auto_attach(), comedi_usb_auto_attach(), etc., and this information would be passed to the driver's auto_attach() hook in an extra parameter.

It seems a bit messier than my original and a bit unnecessary as the driver in some sense already "knows" what the context parameters of it's auto_attach() call are going to be set to as it is in the same function call chain, lower down the stack than the function call from the driver that triggered it.

I eventually wanted to turn comedi_pci_auto_config() and comedi_usb_auto_attach() into a simple inline function that just called the exported function comedi_auto_attach(), but with the new scheme it would make more sense to export __comedi_auto_attach(), and turn comedi_auto_attach(), comedi_pci_auto_attach() and comedi_usb_auto_attach() into simple inline functions that called with different enumerated values.

A cheeky function could call __comedi_auto_attach() directly if it is exported, in which case we're back to the situation where one of the parameters of the auto_attach() method is either a "magic" value or a driver-specified value. This can be broken either by "forbidding" drivers to call __comedi_auto_attach() directly or by not exporting it, forcing us to export all the wrappers instead.

--
-=( 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


[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux