Re: Use of generic device ids in special purpose drivers

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

 



On Wed, Jun 25, 2008 at 01:50:03PM +0200, Bj?rn Mork wrote:
> I just stumbled across this in drivers/scsi/megaraid.c:
> 
> static struct pci_device_id megaraid_pci_tbl[] = {
> 	{PCI_VENDOR_ID_AMI, PCI_DEVICE_ID_AMI_MEGARAID,
> 		PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
> 	{PCI_VENDOR_ID_AMI, PCI_DEVICE_ID_AMI_MEGARAID2,
> 		PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
> 	{PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_AMI_MEGARAID3,
> 		PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
> 	{0,}
> };
> 
> 
> Doesn't look wrong at first, but do notice the PCI_VENDOR_ID_INTEL in
> the last entry.  This doesn't just match megaraid devices, but anything
> using an Intel i960 processor...
> 
> 
> $ grep 1960 include/linux/pci_ids.h
> #define PCI_DEVICE_ID_AMI_MEGARAID3     0x1960
> #define PCI_DEVICE_ID_INTEL_80960_RP    0x1960

So AMI/LSI already know about this problem.  You can see it in the
megaraid_probe_one function:

        /*
         * The megaraid3 stuff reports the ID of the Intel part which is not
         * remotely specific to the megaraid
         */
        if (pdev->vendor == PCI_VENDOR_ID_INTEL) {
                u16 magic;
                /*
                 * Don't fall over the Compaq management cards using the same
                 * PCI identifier
                 */
                if (pdev->subsystem_vendor == PCI_VENDOR_ID_COMPAQ &&
                    pdev->subsystem_device == 0xC000)
                        return -ENODEV;


> Wouldn't it be more appropriate to do additional matching on
> subvendor/subdevice here?  And preferably use
> PCI_DEVICE_ID_INTEL_80960_RP with the Intel vendor id.
> 
> 
> 
> The reason I noticed this is because I don't have any megaraid device,
> but I do have aanother device with an i960:
> 
> bjorn@canardo:~$ lspci -vvnns 04:00
> 04:00.0 PCI bridge [0604]: Intel Corporation 80960RP (i960RP) Microprocessor/Bridge [8086:0960] (rev 05)
>         Kernel modules: shpchp

How interesting.  shpchp shouldn't attach to this bridge; it's not a
hotplug bridge.

> 04:00.1 Memory controller [0580]: Intel Corporation 80960RP (i960RP) Microprocessor [8086:1960] (rev 05)
>         Subsystem: Compaq Computer Corporation Device [0e11:c000]
>         Kernel modules: megaraid
> 
> This is a Compaq "Remote Insight Lights Out Edition" card.

I don't understand how megaraid manages to attach to that when it should
be returning -ENODEV from the probe function as shown above.

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux