From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jean Delvare Subject: Re: [PATCH 2/2] i2c-i801: Add PCI idents for Sandy Bridge SMBus controllers Date: Sun, 31 Oct 2010 11:20:18 +0100 Message-ID: <20101031112018.4a3e7b3e@endymion.delvare> References: <20101030183635.4899246f@endymion.delvare> <1288480550.4570.5.camel@macbook.infradead.org> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1288480550.4570.5.camel-uXGAPMMVk8bAQYKIod7YupZV94DADvEd@public.gmane.org> Sender: linux-i2c-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: David Woodhouse Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: linux-i2c@vger.kernel.org On Sat, 30 Oct 2010 19:15:50 -0400, David Woodhouse wrote: > On Sat, 2010-10-30 at 18:36 +0200, Jean Delvare wrote: > > As a side note, I really don't get the point of using 4 different PCI > > device IDs for exactly the same device. Intel should really start to > > worry about their numbering space if they keep attributing IDs when > > they don't need to. 16 bit is fast to exhaust... > > Don't we keep buying companies that have their own PCI vendor ID? :) > > I certainly agree about the 0x1d7[012] IDs -- those seem fairly > gratuitous. The main one (0x1d22) lacks slave mode though, so it is > different from the others. Not that we care right now. > > > > > > > Features supported by this driver: > > > Software PEC no > > > @@ -127,6 +131,11 @@ > > > SMBHSTSTS_BUS_ERR | SMBHSTSTS_DEV_ERR | \ > > > SMBHSTSTS_INTR) > > > > > > +#define PCI_DEVICE_ID_INTEL_SANDYBRIDGE_SMBUS_PCH 0x1d22 > > > +#define PCI_DEVICE_ID_INTEL_SANDYBRIDGE_SMBUS_EVA1 0x1d70 > > > +#define PCI_DEVICE_ID_INTEL_SANDYBRIDGE_SMBUS_EVA2 0x1d71 > > > +#define PCI_DEVICE_ID_INTEL_SANDYBRIDGE_SMBUS_EVA3 0x1d72 > > > + > > > > These should go to pci_ids.h together with all other similar defines. > > * Do not add new entries to this file unless the definitions > * are shared between multiple drivers. I get the idea, but then why don't we move all other IDs back to i2c-i801.c as well? It seems odd to have some of them in pci_ids.h and the rest in i2c-i801.c. Not something for this patch though, I agree. > > > struct i801_priv { > > > struct i2c_adapter adapter; > > > unsigned long smba; > > > @@ -602,6 +611,10 @@ static const struct pci_device_id i801_ids[] = { > > > { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH10_5) }, > > > { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_5_3400_SERIES_SMBUS) }, > > > { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_COUGARPOINT_SMBUS) }, > > > + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_SANDYBRIDGE_SMBUS_PCH) }, > > > + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_SANDYBRIDGE_SMBUS_EVA1) }, > > > + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_SANDYBRIDGE_SMBUS_EVA2) }, > > > + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_SANDYBRIDGE_SMBUS_EVA3) }, > > > { 0, } > > > }; > > > > > > > You also have to list the new device in drivers/i2c/busses/Kconfig and > > Documentation/i2c/busses/i2c-i801. > > Will update. Thanks, -- Jean Delvare