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

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

 



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@xxxxxxxxx>
---
  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@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