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

H Hartley Sweeten hartleys at visionengravers.com
Tue May 29 17:13:17 UTC 2012


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.

Regards,
Hartley




More information about the devel mailing list