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

[Ian Abbott]

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 at mev.co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-