[PATCH 7/8] pci: Add CircuitCo VENDOR ID and MinnowBoard DEVICE ID

[PATCH 7/8] pci: Add CircuitCo VENDOR ID and MinnowBoard DEVICE ID

[Darren Hart]

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

Re: [PATCH 7/8] pci: Add CircuitCo VENDOR ID and MinnowBoard DEVICE ID

[Bjorn Helgaas]

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
 

Re: [PATCH 7/8] pci: Add CircuitCo VENDOR ID and MinnowBoard DEVICE ID

[Darren Hart]

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?

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Technical Lead - Linux Kernel

Re: [PATCH 7/8] pci: Add CircuitCo VENDOR ID and MinnowBoard DEVICE ID

[Bjorn Helgaas]

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

Re: [PATCH 7/8] pci: Add CircuitCo VENDOR ID and MinnowBoard DEVICE ID

[Darren Hart]

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?

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Technical Lead - Linux Kernel

Re: [PATCH 7/8] pci: Add CircuitCo VENDOR ID and MinnowBoard DEVICE ID

[Bjorn Helgaas]

On Wed, Jun 26, 2013 at 02:16:56PM -0700, Darren Hart wrote:
> 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?

No need.  I'll just apply the following patch.  Obviously your follow-on
pch_gbe patch will need a corresponding change.

Let me know if it needs tweaking, or if you decide I was talking nonsense
above.


commit c2efb42ab6033388f3bbed2dbe0fb76f2402e04a
Author: Darren Hart <dvhart@linux.intel.com>
Date:   Tue Jun 25 20:08:46 2013 -0600

    PCI: Add CircuitCo vendor ID and subsystem ID
    
    Add CircuitCo's newly created VENDOR ID and their first board subsystem
    ID for the MinnowBoard.
    
    [bhelgaas: sort, change DEVICE_ID to SUBSYSTEM_ID]
    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..4c7b349 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -2476,6 +2476,9 @@
 
 #define PCI_VENDOR_ID_ASMEDIA		0x1b21
 
+#define PCI_VENDOR_ID_CIRCUITCO		0x1cc8
+#define PCI_SUBSYSTEM_ID_CIRCUITCO_MINNOWBOARD	0x0001
+
 #define PCI_VENDOR_ID_TEKRAM		0x1de1
 #define PCI_DEVICE_ID_TEKRAM_DC290	0xdc29
 

Re: [PATCH 7/8] pci: Add CircuitCo VENDOR ID and MinnowBoard DEVICE ID

[H. Peter Anvin]

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

Re: [PATCH 7/8] pci: Add CircuitCo VENDOR ID and MinnowBoard DEVICE ID

[Bjorn Helgaas]

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

Re: [PATCH 7/8] pci: Add CircuitCo VENDOR ID and MinnowBoard DEVICE ID

[H. Peter Anvin]

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