[PATCH 8/8] staging: comedi: amplc_pc236: Add attach_pci() hook

Ian Abbott abbotti at mev.co.uk
Wed May 30 08:55:55 UTC 2012


On 2012-05-29 18:13, H Hartley Sweeten wrote:
> On Tuesday, May 29, 2012 6:50 AM, Ian Abbott wrote:
>> Implement the attach_pci() hook as function pc236_attach_pci().  This is
>> called bu comedi_pci_auto_config() in preference to the old attach()
>> hook (implemented by pc236_attach()) and avoids searching for the probed
>> PCI device.
>>
>> Factor out code common to pc236_find_pci() and pc236_attach_pci() into
>> new function pc236_find_pci_board().  Factor out most code common to
>> pc236_attach() and pc236_attach_pci() into new functions
>> pc236_pci_common_attach() and pc236_common_attach().
>>
>> Also #if out member 'devid' from struct pc236_board unless PCI boards
>> are supported as it is not used for ISA boards.
>>
>> Signed-off-by: Ian Abbott<abbotti at mev.co.uk>
>> ---
>>   drivers/staging/comedi/drivers/amplc_pc236.c |  220 +++++++++++++++-----------
>>   1 files changed, 129 insertions(+), 91 deletions(-)
>>
>> diff --git a/drivers/staging/comedi/drivers/amplc_pc236.c b/drivers/staging/comedi/drivers/amplc_pc236.c
>> index 118fced..e0960ce 100644
>> --- a/drivers/staging/comedi/drivers/amplc_pc236.c
>> +++ b/drivers/staging/comedi/drivers/amplc_pc236.c
>
> Ian,
>
> I haven't taken a good look at this series yet but I have a comment...
>
>> +#if IS_ENABLED(CONFIG_COMEDI_AMPLC_PC236_PCI)
>> +	case pci_bustype: {
>> +			int bus = it->options[0];
>> +			int slot = it->options[1];
>> +			struct pci_dev *pci_dev;
>> +
>> +			pci_dev = pc236_find_pci(dev, bus, slot);
>> +			if (pci_dev == NULL)
>> +				return -EIO;
>> +			return pc236_pci_common_attach(dev, pci_dev);
>> +		}
>> +		break;
>
> Why is this even here?
>
> If your using the attach_pci() hook, the pci_dev is already known and passed
> directly to the driver. The bus and slot information isn't even filled in. You
> should just need to find the matching boardinfo and then do the common
> "attach" stuff.
>
> I may be wrong, but I don't think this code path will ever get walked for PCI
> devices that use this driver.

Hi Hartley,

The code is reachable via the COMEDI_DEVCONFIG ioctl call for manual 
configuration of comedi devices.  It's possible to disable automatic 
configuration of devices via the 'comedi_autoconfig' module parameter in 
the core comedi module.  Sometimes that is useful, for example the 
driver module might support extra parameters in it->options[] where the 
comedi PCI autoconfig mechanism only sets the first two of those to bus 
and slot (e.g. the amplc_pci224 driver has a number of options for 
jumper settings but the auto-configuration assumes the device has its 
default jumper settings.)

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti at mev.co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-



More information about the devel mailing list