| Message ID | 20130626163250.GA29299@google.com |
|---|---|
| State | Accepted |
| Headers | show |
On Wed, 2013-06-26 at 10:32 -0600, Bjorn Helgaas wrote: > On Tue, Jun 25, 2013 at 06:53:27PM -0700, Darren Hart wrote: > > Add CircuitCo's newly created VENDOR ID and their first board subsystem > > ID for the MinnowBoard. > > > > Signed-off-by: Darren Hart <dvhart@linux.intel.com> > > Cc: Bjorn Helgaas <bhelgaas@google.com> > > Cc: David Anders <danders@circuitco.com> > > Cc: "H. Peter Anvin" <hpa@zytor.com> > > Cc: Peter Waskiewicz <peter.p.waskiewicz.jr@intel.com> > > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Cc: linux-pci@vger.kernel.org > > --- > > include/linux/pci_ids.h | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h > > index c129162..b2879ce 100644 > > --- a/include/linux/pci_ids.h > > +++ b/include/linux/pci_ids.h > > @@ -149,6 +149,9 @@ > > #define PCI_DEVICE_ID_BERKOM_A4T 0xffa4 > > #define PCI_DEVICE_ID_BERKOM_SCITEL_QUADRO 0xffa8 > > > > +#define PCI_VENDOR_ID_CIRCUITCO 0x1cc8 > > +#define PCI_DEVICE_ID_CIRCUITCO_MINNOWBOARD 0x0001 > > + > > #define PCI_VENDOR_ID_COMPAQ 0x0e11 > > #define PCI_DEVICE_ID_COMPAQ_TOKENRING 0x0508 > > #define PCI_DEVICE_ID_COMPAQ_TACHYON 0xa0fc > > -- > > 1.8.1.2 > > > > Per conversation at [1], I merged the patch below to my pci/misc branch > for v3.11: > > [1] https://lkml.kernel.org/r/6c27e79870ec93f7a8c6692d4bcfebaee589fa6b.1372211451.git.dvhart@linux.intel.com > > commit 9f4de80a09667538e1162fdecff96ccb5bb354a8 > Author: Darren Hart <dvhart@linux.intel.com> > Date: Tue Jun 25 20:08:46 2013 -0600 > > PCI: Add CircuitCo vendor ID > > Add CircuitCo's newly created VENDOR ID > > [bhelgaas: sort, drop device ID (only used once)] > Signed-off-by: Darren Hart <dvhart@linux.intel.com> > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> > > diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h > index c129162..8b1aa7f 100644 > --- a/include/linux/pci_ids.h > +++ b/include/linux/pci_ids.h > @@ -2476,6 +2476,8 @@ > > #define PCI_VENDOR_ID_ASMEDIA 0x1b21 > > +#define PCI_VENDOR_ID_CIRCUITCO 0x1cc8 > + > #define PCI_VENDOR_ID_TEKRAM 0x1de1 > #define PCI_DEVICE_ID_TEKRAM_DC290 0xdc29 Thanks Bjorn. When I reuse this Subsystem ID and there is more than one usage, I should send a patch to pci_ids.h adding it and replace the hex value in all drivers with the new define. Is that right?
On Wed, Jun 26, 2013 at 11:15 AM, Darren Hart <dvhart@linux.intel.com> wrote: > On Wed, 2013-06-26 at 10:32 -0600, Bjorn Helgaas wrote: >> +#define PCI_VENDOR_ID_CIRCUITCO 0x1cc8 >> + >> #define PCI_VENDOR_ID_TEKRAM 0x1de1 >> #define PCI_DEVICE_ID_TEKRAM_DC290 0xdc29 > > > Thanks Bjorn. When I reuse this Subsystem ID and there is more than one > usage, I should send a patch to pci_ids.h adding it and replace the hex > value in all drivers with the new define. Is that right? Yeah, that's what I was thinking. But Peter's comment makes more sense to me now. The spec refers to that config register as "Subsystem ID," not "Subsystem Device ID," but I was confused because most existing usage treats it as a device ID. For example, the field in struct pci_device_id is named "subdevice," and all the existing #defines in pci_ids.h are of the form PCI_SUBDEVICE_ID_*. Device IDs are pretty specific identifiers, so I was thinking that a "sub-device ID" would be even more specific. Then it would make no sense to have a "sub-device ID" that was as generic as "MINNOWBOARD." But the register is actually *not* a "sub-device ID," and I can see that using the same Subsystem ID for all the devices on a board might make sense. So I think the name PCI_DEVICE_ID_CIRCUITCO_MINNOWBOARD is a bit of a misnomer, and something like PCI_SUBSYSTEM_CIRCUITCO_MINNOWBOARD would make it more clear that it really isn't sharing the device ID space assigned to CircuitCo. It would make perfect sense to have a Device ID, e.g., "PCI_DEVICE_ID_CIRCUIT_CO_xxx 0x0001," that has nothing to do with the Subsystem ID 0x0001. If you want to do something like that (or even keep your original patch), I can put that in my -next branch. Just let me know. Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2013-06-26 at 13:37 -0600, Bjorn Helgaas wrote: > On Wed, Jun 26, 2013 at 11:15 AM, Darren Hart <dvhart@linux.intel.com> wrote: > > On Wed, 2013-06-26 at 10:32 -0600, Bjorn Helgaas wrote: > > >> +#define PCI_VENDOR_ID_CIRCUITCO 0x1cc8 > >> + > >> #define PCI_VENDOR_ID_TEKRAM 0x1de1 > >> #define PCI_DEVICE_ID_TEKRAM_DC290 0xdc29 > > > > > > Thanks Bjorn. When I reuse this Subsystem ID and there is more than one > > usage, I should send a patch to pci_ids.h adding it and replace the hex > > value in all drivers with the new define. Is that right? > > Yeah, that's what I was thinking. > > But Peter's comment makes more sense to me now. The spec refers to > that config register as "Subsystem ID," not "Subsystem Device ID," but > I was confused because most existing usage treats it as a device ID. > For example, the field in struct pci_device_id is named "subdevice," > and all the existing #defines in pci_ids.h are of the form > PCI_SUBDEVICE_ID_*. > > Device IDs are pretty specific identifiers, so I was thinking that a > "sub-device ID" would be even more specific. Then it would make no > sense to have a "sub-device ID" that was as generic as "MINNOWBOARD." > But the register is actually *not* a "sub-device ID," and I can see > that using the same Subsystem ID for all the devices on a board might > make sense. > > So I think the name PCI_DEVICE_ID_CIRCUITCO_MINNOWBOARD is a bit of a > misnomer, and something like PCI_SUBSYSTEM_CIRCUITCO_MINNOWBOARD would > make it more clear that it really isn't sharing the device ID space > assigned to CircuitCo. It would make perfect sense to have a Device > ID, e.g., "PCI_DEVICE_ID_CIRCUIT_CO_xxx 0x0001," that has nothing to > do with the Subsystem ID 0x0001. > > If you want to do something like that (or even keep your original > patch), I can put that in my -next branch. Just let me know. > > Bjorn I would be happy to change DEVICE to SUBSYSTEM in the the pci_ids.h: +#define PCI_VENDOR_ID_CIRCUITCO 0x1cc8 +#define PCI_SUBSYSTEM_ID_CITCUITCO_MINNOWBOARD 0x0001 Would you like me to send another patch?
On 06/26/2013 12:37 PM, Bjorn Helgaas wrote: > > Yeah, that's what I was thinking. > > But Peter's comment makes more sense to me now. The spec refers to > that config register as "Subsystem ID," not "Subsystem Device ID," but > I was confused because most existing usage treats it as a device ID. > For example, the field in struct pci_device_id is named "subdevice," > and all the existing #defines in pci_ids.h are of the form > PCI_SUBDEVICE_ID_*. > > Device IDs are pretty specific identifiers, so I was thinking that a > "sub-device ID" would be even more specific. Then it would make no > sense to have a "sub-device ID" that was as generic as "MINNOWBOARD." > But the register is actually *not* a "sub-device ID," and I can see > that using the same Subsystem ID for all the devices on a board might > make sense. > Subsystem IDs is basically a board ID in the traditional PC view, but they didn't call it that because it would have been confusing in other, nontraditional configurations. Microsoft has a "best practices" document, which may end up becoming basis for a future PCI-SIG document clarifying the standard: http://msdn.microsoft.com/en-us/library/windows/hardware/gg463287.aspx -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jun 26, 2013 at 3:30 PM, H. Peter Anvin <hpa@zytor.com> wrote: > On 06/26/2013 12:37 PM, Bjorn Helgaas wrote: >> >> Yeah, that's what I was thinking. >> >> But Peter's comment makes more sense to me now. The spec refers to >> that config register as "Subsystem ID," not "Subsystem Device ID," but >> I was confused because most existing usage treats it as a device ID. >> For example, the field in struct pci_device_id is named "subdevice," >> and all the existing #defines in pci_ids.h are of the form >> PCI_SUBDEVICE_ID_*. >> >> Device IDs are pretty specific identifiers, so I was thinking that a >> "sub-device ID" would be even more specific. Then it would make no >> sense to have a "sub-device ID" that was as generic as "MINNOWBOARD." >> But the register is actually *not* a "sub-device ID," and I can see >> that using the same Subsystem ID for all the devices on a board might >> make sense. >> > > Subsystem IDs is basically a board ID in the traditional PC view, but > they didn't call it that because it would have been confusing in other, > nontraditional configurations. > > Microsoft has a "best practices" document, which may end up becoming > basis for a future PCI-SIG document clarifying the standard: > > http://msdn.microsoft.com/en-us/library/windows/hardware/gg463287.aspx Interesting, thanks for the link. If I read that correctly, the MinnowBoard is basically a motherboard, and any board layout change or component value change will require a new Subsystem ID, which will in turn require a pch_gbe update. That doesn't sound optimal, but maybe people don't actually interpret it that strictly. Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 06/26/2013 02:46 PM, Bjorn Helgaas wrote: >> >> Microsoft has a "best practices" document, which may end up becoming >> basis for a future PCI-SIG document clarifying the standard: >> >> http://msdn.microsoft.com/en-us/library/windows/hardware/gg463287.aspx > > Interesting, thanks for the link. If I read that correctly, the > MinnowBoard is basically a motherboard, and any board layout change or > component value change will require a new Subsystem ID, which will in > turn require a pch_gbe update. That doesn't sound optimal, but maybe > people don't actually interpret it that strictly. > It could be interpreted that literally, yes. This is part of why a "subsystem" isn't necessarily forced to be a "board". -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h index c129162..8b1aa7f 100644 --- a/include/linux/pci_ids.h +++ b/include/linux/pci_ids.h @@ -2476,6 +2476,8 @@ #define PCI_VENDOR_ID_ASMEDIA 0x1b21 +#define PCI_VENDOR_ID_CIRCUITCO 0x1cc8 + #define PCI_VENDOR_ID_TEKRAM 0x1de1 #define PCI_DEVICE_ID_TEKRAM_DC290 0xdc29