lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 17 Jun 2009 21:24:53 -0300 From: Herton Ronaldo Krzesinski <herton@...driva.com.br> To: fmhess@...rs.sourceforge.net Cc: Ian Abbott <abbotti@....co.uk>, Ian Abbott <ian.abbott@....co.uk>, Greg KH <gregkh@...e.de>, LKML <linux-kernel@...r.kernel.org>, Gianluca Palli <gpalli@...s.unibo.it>, David Schleef <ds@...leef.org> Subject: Re: staging driver s626 clashes with philips SAA7146 media/dvb based cards Em Qua 17 Jun 2009, às 21:05:56, Herton Ronaldo Krzesinski escreveu: > Em Qua 17 Jun 2009, às 20:35:13, Frank Mori Hess escreveu: > > On Wednesday 17 June 2009, Herton Ronaldo Krzesinski wrote: > > > Also s626_attach is modified to use now pci_get_subsys instead of > > > pci_get_device as reported by Ian Abbott, additionaly ensuring that > > > subvendor or subdevice id is set in pci id table entries. > > > > > > Reference: http://lkml.org/lkml/2009/6/16/552 > > > --- > > > drivers/staging/comedi/drivers/s626.c | 35 > > > ++++++++++++++++++++------------ 1 files changed, 22 insertions(+), 13 > > > deletions(-) > > > > > > diff --git a/drivers/staging/comedi/drivers/s626.c > > > b/drivers/staging/comedi/drivers/s626.c index 30dec9d..3ec4407 100644 > > > --- a/drivers/staging/comedi/drivers/s626.c > > > +++ b/drivers/staging/comedi/drivers/s626.c > > > @@ -110,9 +110,9 @@ static const struct s626_board s626_boards[] = { > > > #define PCI_VENDOR_ID_S626 0x1131 > > > #define PCI_DEVICE_ID_S626 0x7146 > > > > > > +/* sub pci id must be specified, see s626_attach for more details */ > > > static DEFINE_PCI_DEVICE_TABLE(s626_pci_table) = { > > > - {PCI_VENDOR_ID_S626, PCI_DEVICE_ID_S626, PCI_ANY_ID, PCI_ANY_ID, 0, 0, > > > - 0}, > > > + {PCI_VENDOR_ID_S626, PCI_DEVICE_ID_S626, 0x6000, 0x0272, 0, 0, 0}, > > > {0} > > > }; > > > > > > @@ -498,24 +498,33 @@ static int s626_attach(struct comedi_device *dev, > > > struct comedi_devconfig *it) resource_size_t resourceStart; > > > dma_addr_t appdma; > > > struct comedi_subdevice *s; > > > - struct pci_dev *pdev; > > > + const struct pci_device_id *ids; > > > + struct pci_dev *pdev = NULL; > > > > > > if (alloc_private(dev, sizeof(struct s626_private)) < 0) > > > return -ENOMEM; > > > > > > - for (pdev = pci_get_device(PCI_VENDOR_ID_S626, PCI_DEVICE_ID_S626, > > > - NULL); pdev != NULL; > > > - pdev = pci_get_device(PCI_VENDOR_ID_S626, > > > - PCI_DEVICE_ID_S626, pdev)) { > > > + /* > > > + * Require also one of sub pci ids to be defined (see check below), > > > + * otherwise there will be a clash with Philips SAA7146 media/dvb > > > + * based cards (they have same vendor:device == 0x1131:0x7146 pair > > > + * as main S626 cards) > > > + */ > > > + for (ids = s626_pci_table; > > > + (ids->vendor && (ids->subvendor || ids->subdevice)) && !pdev; > > > + ids++) { > > > + pdev = pci_get_subsys(ids->vendor, ids->device, ids->subvendor, > > > + ids->subdevice, NULL); > > > + if (!pdev) > > > + continue; > > > + > > > if (it->options[0] || it->options[1]) { > > > + /* matches requested bus/slot */ > > > if (pdev->bus->number == it->options[0] && > > > - PCI_SLOT(pdev->devfn) == it->options[1]) { > > > - /* matches requested bus/slot */ > > > + PCI_SLOT(pdev->devfn) == it->options[1]) > > > break; > > > - } > > > - } else { > > > - /* no bus/slot specified */ > > > - break; > > > + pci_dev_put(pdev); > > > + pdev = NULL; > > > } > > > } > > > devpriv->pdev = pdev; > > > > > > This patch looks buggy. It's changing the logic beyond just checking for > > subvendor/subdevice ids. > > > > That's the intention here, so that it avoids someone adding a new pci id > without specifying either subvendor or subdevice id for 0x1131:0x7146 boards, > but yes there will be a problem if boards with vendor:id not equal to > 0x1131:0x7146 appear in future, as you will be obliged to add > subvendor:subdevice id even if not needed. > > If not wanted and it gone too far, I can revert to use the same logic as > pci_match_id, or just simplify this in case it's unlikely more s626 boards > appear. > > The current situation is ugly, comedi subsystem could have a better way to deal > with hotplug and probe of devices, without you having to reimplement what pci > subsystem functions already does. Erm... forget that, you mean the "/* no bus/slot specified */" I removed. That previous logic to me didn't made much sense, why you must always request a particular bus/slot? -- []'s Herton -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists