diff mbox series

[V3,1/3] PCI: Add defines for Designated Vendor-Specific Capability

Message ID 20200714062323.19990-2-david.e.box@linux.intel.com
State New
Headers show
Series Intel Platform Monitoring Technology | expand

Commit Message

David E. Box July 14, 2020, 6:23 a.m. UTC 
Add PCIe DVSEC extended capability ID and defines for the header offsets.
Defined in PCIe r5.0, sec 7.9.6.

Signed-off-by: David E. Box <david.e.box@linux.intel.com>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
---
 include/uapi/linux/pci_regs.h | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Andy Shevchenko July 14, 2020, 8:40 a.m. UTC | #1 
On Tue, Jul 14, 2020 at 9:22 AM David E. Box
<david.e.box@linux.intel.com> wrote:
>
> Add PCIe DVSEC extended capability ID and defines for the header offsets.
> Defined in PCIe r5.0, sec 7.9.6.
>

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
>  include/uapi/linux/pci_regs.h | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> index f9701410d3b5..09daa9f07b6b 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -720,6 +720,7 @@
>  #define PCI_EXT_CAP_ID_DPC     0x1D    /* Downstream Port Containment */
>  #define PCI_EXT_CAP_ID_L1SS    0x1E    /* L1 PM Substates */
>  #define PCI_EXT_CAP_ID_PTM     0x1F    /* Precision Time Measurement */
> +#define PCI_EXT_CAP_ID_DVSEC   0x23    /* Designated Vendor-Specific */
>  #define PCI_EXT_CAP_ID_DLF     0x25    /* Data Link Feature */
>  #define PCI_EXT_CAP_ID_PL_16GT 0x26    /* Physical Layer 16.0 GT/s */
>  #define PCI_EXT_CAP_ID_MAX     PCI_EXT_CAP_ID_PL_16GT
> @@ -1062,6 +1063,10 @@
>  #define  PCI_L1SS_CTL1_LTR_L12_TH_SCALE        0xe0000000  /* LTR_L1.2_THRESHOLD_Scale */
>  #define PCI_L1SS_CTL2          0x0c    /* Control 2 Register */
>
> +/* Designated Vendor-Specific (DVSEC, PCI_EXT_CAP_ID_DVSEC) */
> +#define PCI_DVSEC_HEADER1              0x4 /* Vendor-Specific Header1 */
> +#define PCI_DVSEC_HEADER2              0x8 /* Vendor-Specific Header2 */
> +
>  /* Data Link Feature */
>  #define PCI_DLF_CAP            0x04    /* Capabilities Register */
>  #define  PCI_DLF_EXCHANGE_ENABLE       0x80000000  /* Data Link Feature Exchange Enable */
> --
> 2.20.1
>
Randy Dunlap July 16, 2020, 2:55 a.m. UTC | #2 
On 7/13/20 11:23 PM, David E. Box wrote:
> Add PCIe DVSEC extended capability ID and defines for the header offsets.
> Defined in PCIe r5.0, sec 7.9.6.
> 
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
>  include/uapi/linux/pci_regs.h | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> index f9701410d3b5..09daa9f07b6b 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -720,6 +720,7 @@
> +#define PCI_EXT_CAP_ID_DVSEC	0x23	/* Designated Vendor-Specific */
> @@ -1062,6 +1063,10 @@
> +/* Designated Vendor-Specific (DVSEC, PCI_EXT_CAP_ID_DVSEC) */
> +#define PCI_DVSEC_HEADER1		0x4 /* Vendor-Specific Header1 */
> +#define PCI_DVSEC_HEADER2		0x8 /* Vendor-Specific Header2 */

Just a little comment: It would make more sense to me to
s/DVSEC/DVSPEC/g.

But then I don't have the PCIe documentation.
Bjorn Helgaas July 16, 2020, 3:07 p.m. UTC | #3 
On Wed, Jul 15, 2020 at 07:55:11PM -0700, Randy Dunlap wrote:
> On 7/13/20 11:23 PM, David E. Box wrote:
> > Add PCIe DVSEC extended capability ID and defines for the header offsets.
> > Defined in PCIe r5.0, sec 7.9.6.
> > 
> > Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> > Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> > ---
> >  include/uapi/linux/pci_regs.h | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> > index f9701410d3b5..09daa9f07b6b 100644
> > --- a/include/uapi/linux/pci_regs.h
> > +++ b/include/uapi/linux/pci_regs.h
> > @@ -720,6 +720,7 @@
> > +#define PCI_EXT_CAP_ID_DVSEC	0x23	/* Designated Vendor-Specific */
> > @@ -1062,6 +1063,10 @@
> > +/* Designated Vendor-Specific (DVSEC, PCI_EXT_CAP_ID_DVSEC) */
> > +#define PCI_DVSEC_HEADER1		0x4 /* Vendor-Specific Header1 */
> > +#define PCI_DVSEC_HEADER2		0x8 /* Vendor-Specific Header2 */
> 
> Just a little comment: It would make more sense to me to
> s/DVSEC/DVSPEC/g.

Yeah, that is confusing, but "DVSEC" is the term used in the spec.  I
think it stands for "Designated Vendor-Specific Extended Capability".
Randy Dunlap July 16, 2020, 3:07 p.m. UTC | #4 
On 7/16/20 8:07 AM, Bjorn Helgaas wrote:
> On Wed, Jul 15, 2020 at 07:55:11PM -0700, Randy Dunlap wrote:
>> On 7/13/20 11:23 PM, David E. Box wrote:
>>> Add PCIe DVSEC extended capability ID and defines for the header offsets.
>>> Defined in PCIe r5.0, sec 7.9.6.
>>>
>>> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
>>> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
>>> ---
>>>  include/uapi/linux/pci_regs.h | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
>>> index f9701410d3b5..09daa9f07b6b 100644
>>> --- a/include/uapi/linux/pci_regs.h
>>> +++ b/include/uapi/linux/pci_regs.h
>>> @@ -720,6 +720,7 @@
>>> +#define PCI_EXT_CAP_ID_DVSEC	0x23	/* Designated Vendor-Specific */
>>> @@ -1062,6 +1063,10 @@
>>> +/* Designated Vendor-Specific (DVSEC, PCI_EXT_CAP_ID_DVSEC) */
>>> +#define PCI_DVSEC_HEADER1		0x4 /* Vendor-Specific Header1 */
>>> +#define PCI_DVSEC_HEADER2		0x8 /* Vendor-Specific Header2 */
>>
>> Just a little comment: It would make more sense to me to
>> s/DVSEC/DVSPEC/g.
> 
> Yeah, that is confusing, but "DVSEC" is the term used in the spec.  I
> think it stands for "Designated Vendor-Specific Extended Capability".

Right. I noticed that after I sent the email.

thanks.
Alexander Duyck July 16, 2020, 5:18 p.m. UTC | #5 
On 7/15/2020 7:55 PM, Randy Dunlap wrote:
> On 7/13/20 11:23 PM, David E. Box wrote:
>> Add PCIe DVSEC extended capability ID and defines for the header offsets.
>> Defined in PCIe r5.0, sec 7.9.6.
>>
>> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
>> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
>> ---
>>   include/uapi/linux/pci_regs.h | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
>> index f9701410d3b5..09daa9f07b6b 100644
>> --- a/include/uapi/linux/pci_regs.h
>> +++ b/include/uapi/linux/pci_regs.h
>> @@ -720,6 +720,7 @@
>> +#define PCI_EXT_CAP_ID_DVSEC	0x23	/* Designated Vendor-Specific */
>> @@ -1062,6 +1063,10 @@
>> +/* Designated Vendor-Specific (DVSEC, PCI_EXT_CAP_ID_DVSEC) */
>> +#define PCI_DVSEC_HEADER1		0x4 /* Vendor-Specific Header1 */
>> +#define PCI_DVSEC_HEADER2		0x8 /* Vendor-Specific Header2 */
> 
> Just a little comment: It would make more sense to me to
> s/DVSEC/DVSPEC/g.
> 
> But then I don't have the PCIe documentation.

Arguably some of the confusion might be from the patch title. DVSEC is 
acronym for Designated Vendor-Specific Extended Capability if I recall 
correctly. It would probably be best to call that out since the extended 
implies it lives in the config space accessible via the memory mapped 
config.
David E. Box July 16, 2020, 6:31 p.m. UTC | #6 
On Thu, 2020-07-16 at 10:18 -0700, Alexander Duyck wrote:
> 
> On 7/15/2020 7:55 PM, Randy Dunlap wrote:
> > On 7/13/20 11:23 PM, David E. Box wrote:
> > > Add PCIe DVSEC extended capability ID and defines for the header
> > > offsets.
> > > Defined in PCIe r5.0, sec 7.9.6.
> > > 
> > > Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> > > Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> > > ---
> > >   include/uapi/linux/pci_regs.h | 5 +++++
> > >   1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/include/uapi/linux/pci_regs.h
> > > b/include/uapi/linux/pci_regs.h
> > > index f9701410d3b5..09daa9f07b6b 100644
> > > --- a/include/uapi/linux/pci_regs.h
> > > +++ b/include/uapi/linux/pci_regs.h
> > > @@ -720,6 +720,7 @@
> > > +#define PCI_EXT_CAP_ID_DVSEC	0x23	/* Designated Vendor-
> > > Specific */
> > > @@ -1062,6 +1063,10 @@
> > > +/* Designated Vendor-Specific (DVSEC, PCI_EXT_CAP_ID_DVSEC) */
> > > +#define PCI_DVSEC_HEADER1		0x4 /* Vendor-Specific
> > > Header1 */
> > > +#define PCI_DVSEC_HEADER2		0x8 /* Vendor-Specific
> > > Header2 */

These comments I'll fix to say "Designated Vendor-Specific"

> > 
> > Just a little comment: It would make more sense to me to
> > s/DVSEC/DVSPEC/g.
> > 
> > But then I don't have the PCIe documentation.
> 
> Arguably some of the confusion might be from the patch title. DVSEC
> is 
> acronym for Designated Vendor-Specific Extended Capability if I
> recall 
> correctly. It would probably be best to call that out since the
> extended 
> implies it lives in the config space accessible via the memory
> mapped 
> config.

I'll change the patch title as well, but agree DVSEC is better as it's
consistent with the spec.

Thanks

David
diff mbox series

Patch

diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index f9701410d3b5..09daa9f07b6b 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -720,6 +720,7 @@ 
 #define PCI_EXT_CAP_ID_DPC	0x1D	/* Downstream Port Containment */
 #define PCI_EXT_CAP_ID_L1SS	0x1E	/* L1 PM Substates */
 #define PCI_EXT_CAP_ID_PTM	0x1F	/* Precision Time Measurement */
+#define PCI_EXT_CAP_ID_DVSEC	0x23	/* Designated Vendor-Specific */
 #define PCI_EXT_CAP_ID_DLF	0x25	/* Data Link Feature */
 #define PCI_EXT_CAP_ID_PL_16GT	0x26	/* Physical Layer 16.0 GT/s */
 #define PCI_EXT_CAP_ID_MAX	PCI_EXT_CAP_ID_PL_16GT
@@ -1062,6 +1063,10 @@ 
 #define  PCI_L1SS_CTL1_LTR_L12_TH_SCALE	0xe0000000  /* LTR_L1.2_THRESHOLD_Scale */
 #define PCI_L1SS_CTL2		0x0c	/* Control 2 Register */
 
+/* Designated Vendor-Specific (DVSEC, PCI_EXT_CAP_ID_DVSEC) */
+#define PCI_DVSEC_HEADER1		0x4 /* Vendor-Specific Header1 */
+#define PCI_DVSEC_HEADER2		0x8 /* Vendor-Specific Header2 */
+
 /* Data Link Feature */
 #define PCI_DLF_CAP		0x04	/* Capabilities Register */
 #define  PCI_DLF_EXCHANGE_ENABLE	0x80000000  /* Data Link Feature Exchange Enable */