From: leoliu-oc <[email protected]>
Define secondary uncorrectable error mask register, secondary
uncorrectable error severity register and secondary error capabilities and
control register bits in AER capability for PCIe to PCI/PCI-X Bridge.
Please refer to PCIe to PCI/PCI-X Bridge Specification, sec 5.2.3.2,
5.2.3.3 and 5.2.3.4.
Signed-off-by: leoliu-oc <[email protected]>
---
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 57b8e2ffb1dd..37f3baa336d7 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -799,6 +799,11 @@
#define PCI_ERR_ROOT_AER_IRQ 0xf8000000 /* Advanced Error Interrupt Message Number */
#define PCI_ERR_ROOT_ERR_SRC 0x34 /* Error Source Identification */
+/* PCIe advanced error reporting extended capabilities for PCIe to PCI/PCI-X Bridge */
+#define PCI_ERR_UNCOR_MASK2 0x30 /* Secondary Uncorrectable Error Mask */
+#define PCI_ERR_UNCOR_SEVER2 0x34 /* Secondary Uncorrectable Error Severit */
+#define PCI_ERR_CAP2 0x38 /* Secondary Advanced Error Capabilities */
+
/* Virtual Channel */
#define PCI_VC_PORT_CAP1 0x04
#define PCI_VC_CAP1_EVCC 0x00000007 /* extended VC count */
--
2.20.1
Since this patch has nothing to do with ACPI, update subject line to:
PCI: Add PCIe to PCI/PCI-X Bridge AER fields
On Tue, Nov 15, 2022 at 11:12:44AM +0800, LeoLiu-oc wrote:
> From: leoliu-oc <[email protected]>
>
> Define secondary uncorrectable error mask register, secondary
> uncorrectable error severity register and secondary error capabilities and
> control register bits in AER capability for PCIe to PCI/PCI-X Bridge.
> Please refer to PCIe to PCI/PCI-X Bridge Specification, sec 5.2.3.2,
> 5.2.3.3 and 5.2.3.4.
Capitalize register names to match the spec usage.
> Signed-off-by: leoliu-oc <[email protected]>
Assuming this goes along with a patch series that adds uses of these
definitions:
Acked-by: Bjorn Helgaas <[email protected]>
> ---
> 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 57b8e2ffb1dd..37f3baa336d7 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -799,6 +799,11 @@
> #define PCI_ERR_ROOT_AER_IRQ 0xf8000000 /* Advanced Error Interrupt Message Number */
> #define PCI_ERR_ROOT_ERR_SRC 0x34 /* Error Source Identification */
>
> +/* PCIe advanced error reporting extended capabilities for PCIe to PCI/PCI-X Bridge */
> +#define PCI_ERR_UNCOR_MASK2 0x30 /* Secondary Uncorrectable Error Mask */
> +#define PCI_ERR_UNCOR_SEVER2 0x34 /* Secondary Uncorrectable Error Severit */
> +#define PCI_ERR_CAP2 0x38 /* Secondary Advanced Error Capabilities */
Please squash these right up next to the other PCI_ERR_* definitions
so it's obvious that they overlap PCI_ERR_ROOT_STATUS and
PCI_ERR_ROOT_ERR_SRC (which is fine since one device can't have both),
e.g.,
#define PCI_ERR_ROOT_STATUS 0x30
#define PCI_ERR_ROOT_COR_RCV 0x00000001 /* ERR_COR Received */
...
#define PCI_ERR_ROOT_ERR_SRC 0x34 /* Error Source Identification */
#define PCI_ERR_UNCOR_MASK2 0x30 /* PCIe to PCI/PCI-X bridge */
#define PCI_ERR_UNCOR_SEVER2 0x34 /* PCIe to PCI/PCI-X bridge */
#define PCI_ERR_CAP2 0x38 /* PCIe to PCI/PCI-X bridge */
> /* Virtual Channel */
> #define PCI_VC_PORT_CAP1 0x04
> #define PCI_VC_CAP1_EVCC 0x00000007 /* extended VC count */
> --
> 2.20.1
>
在 2023/4/8 7:22, Bjorn Helgaas 写道:
> Since this patch has nothing to do with ACPI, update subject line to:
>
> PCI: Add PCIe to PCI/PCI-X Bridge AER fields
>
Your description is more reasonable and I will update the header of this
patch later.
Yours sincerely,
Leoliu-oc
> On Tue, Nov 15, 2022 at 11:12:44AM +0800, LeoLiu-oc wrote:
>> From: leoliu-oc <[email protected]>
>>
>> Define secondary uncorrectable error mask register, secondary
>> uncorrectable error severity register and secondary error capabilities and
>> control register bits in AER capability for PCIe to PCI/PCI-X Bridge.
>> Please refer to PCIe to PCI/PCI-X Bridge Specification, sec 5.2.3.2,
>> 5.2.3.3 and 5.2.3.4.
>
> Capitalize register names to match the spec usage.
>
Your suggestion is right, I'll update this in the next release.
Yours sincerely,
Leoliu-oc
>> Signed-off-by: leoliu-oc <[email protected]>
>
> Assuming this goes along with a patch series that adds uses of these
> definitions:
>
> Acked-by: Bjorn Helgaas <[email protected]>
>
>> ---
>> 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 57b8e2ffb1dd..37f3baa336d7 100644
>> --- a/include/uapi/linux/pci_regs.h
>> +++ b/include/uapi/linux/pci_regs.h
>> @@ -799,6 +799,11 @@
>> #define PCI_ERR_ROOT_AER_IRQ 0xf8000000 /* Advanced Error Interrupt Message Number */
>> #define PCI_ERR_ROOT_ERR_SRC 0x34 /* Error Source Identification */
>>
>> +/* PCIe advanced error reporting extended capabilities for PCIe to PCI/PCI-X Bridge */
>> +#define PCI_ERR_UNCOR_MASK2 0x30 /* Secondary Uncorrectable Error Mask */
>> +#define PCI_ERR_UNCOR_SEVER2 0x34 /* Secondary Uncorrectable Error Severit */
>> +#define PCI_ERR_CAP2 0x38 /* Secondary Advanced Error Capabilities */
>
> Please squash these right up next to the other PCI_ERR_* definitions
> so it's obvious that they overlap PCI_ERR_ROOT_STATUS and
> PCI_ERR_ROOT_ERR_SRC (which is fine since one device can't have both),
> e.g.,
>
> #define PCI_ERR_ROOT_STATUS 0x30
> #define PCI_ERR_ROOT_COR_RCV 0x00000001 /* ERR_COR Received */
> ...
> #define PCI_ERR_ROOT_ERR_SRC 0x34 /* Error Source Identification */
> #define PCI_ERR_UNCOR_MASK2 0x30 /* PCIe to PCI/PCI-X bridge */
> #define PCI_ERR_UNCOR_SEVER2 0x34 /* PCIe to PCI/PCI-X bridge */
> #define PCI_ERR_CAP2 0x38 /* PCIe to PCI/PCI-X bridge */
>
I don't seem to understand what you mean. PCI_ERR_UNCOR_MASK2,
PCI_ERR_UNCOR_SEVER2, and PCI_ERR_CAP2 represent the control and
handling of individual errors that occur on traditional PCI or PCI-x
secondary bus interfaces, these registers are valid only for Bridge.
Although PCI_ERR_ROOT_ERR_SRC and PCI_ERR_UNCOR_SEVER2 have the same
value, they represent register definitions for different device types.
Yours sincerely,
Leoliu-oc
>> /* Virtual Channel */
>> #define PCI_VC_PORT_CAP1 0x04
>> #define PCI_VC_CAP1_EVCC 0x00000007 /* extended VC count */
>> --
>> 2.20.1
>>
On Wed, Apr 12, 2023 at 05:49:55PM +0800, LeoLiuoc wrote:
> > > diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> > > index 57b8e2ffb1dd..37f3baa336d7 100644
> > > --- a/include/uapi/linux/pci_regs.h
> > > +++ b/include/uapi/linux/pci_regs.h
> > > @@ -799,6 +799,11 @@
> > > #define PCI_ERR_ROOT_AER_IRQ 0xf8000000 /* Advanced Error Interrupt Message Number */
> > > #define PCI_ERR_ROOT_ERR_SRC 0x34 /* Error Source Identification */
> > > +/* PCIe advanced error reporting extended capabilities for PCIe to PCI/PCI-X Bridge */
> > > +#define PCI_ERR_UNCOR_MASK2 0x30 /* Secondary Uncorrectable Error Mask */
> > > +#define PCI_ERR_UNCOR_SEVER2 0x34 /* Secondary Uncorrectable Error Severit */
> > > +#define PCI_ERR_CAP2 0x38 /* Secondary Advanced Error Capabilities */
> >
> > Please squash these right up next to the other PCI_ERR_* definitions
> > so it's obvious that they overlap PCI_ERR_ROOT_STATUS and
> > PCI_ERR_ROOT_ERR_SRC (which is fine since one device can't have both),
> > e.g.,
> >
> > #define PCI_ERR_ROOT_STATUS 0x30
> > #define PCI_ERR_ROOT_COR_RCV 0x00000001 /* ERR_COR Received */
> > ...
> > #define PCI_ERR_ROOT_ERR_SRC 0x34 /* Error Source Identification */
> > #define PCI_ERR_UNCOR_MASK2 0x30 /* PCIe to PCI/PCI-X bridge */
> > #define PCI_ERR_UNCOR_SEVER2 0x34 /* PCIe to PCI/PCI-X bridge */
> > #define PCI_ERR_CAP2 0x38 /* PCIe to PCI/PCI-X bridge */
>
> I don't seem to understand what you mean. PCI_ERR_UNCOR_MASK2,
> PCI_ERR_UNCOR_SEVER2, and PCI_ERR_CAP2 represent the control and handling of
> individual errors that occur on traditional PCI or PCI-x secondary bus
> interfaces, these registers are valid only for Bridge. Although
> PCI_ERR_ROOT_ERR_SRC and PCI_ERR_UNCOR_SEVER2 have the same value, they
> represent register definitions for different device types.
Right. I just don't want the blank line in the middle because it
might be mistaken for items in a different capability. All the other
AER capability registers are defined together in a block, with no
blank lines in the middle, so I think these new ones should be part of
that block.
Bjorn
在 2023/4/13 0:10, Bjorn Helgaas 写道:
> On Wed, Apr 12, 2023 at 05:49:55PM +0800, LeoLiuoc wrote:
>
>>>> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
>>>> index 57b8e2ffb1dd..37f3baa336d7 100644
>>>> --- a/include/uapi/linux/pci_regs.h
>>>> +++ b/include/uapi/linux/pci_regs.h
>>>> @@ -799,6 +799,11 @@
>>>> #define PCI_ERR_ROOT_AER_IRQ 0xf8000000 /* Advanced Error Interrupt Message Number */
>>>> #define PCI_ERR_ROOT_ERR_SRC 0x34 /* Error Source Identification */
>>>> +/* PCIe advanced error reporting extended capabilities for PCIe to PCI/PCI-X Bridge */
>>>> +#define PCI_ERR_UNCOR_MASK2 0x30 /* Secondary Uncorrectable Error Mask */
>>>> +#define PCI_ERR_UNCOR_SEVER2 0x34 /* Secondary Uncorrectable Error Severit */
>>>> +#define PCI_ERR_CAP2 0x38 /* Secondary Advanced Error Capabilities */
>>>
>>> Please squash these right up next to the other PCI_ERR_* definitions
>>> so it's obvious that they overlap PCI_ERR_ROOT_STATUS and
>>> PCI_ERR_ROOT_ERR_SRC (which is fine since one device can't have both),
>>> e.g.,
>>>
>>> #define PCI_ERR_ROOT_STATUS 0x30
>>> #define PCI_ERR_ROOT_COR_RCV 0x00000001 /* ERR_COR Received */
>>> ...
>>> #define PCI_ERR_ROOT_ERR_SRC 0x34 /* Error Source Identification */
>>> #define PCI_ERR_UNCOR_MASK2 0x30 /* PCIe to PCI/PCI-X bridge */
>>> #define PCI_ERR_UNCOR_SEVER2 0x34 /* PCIe to PCI/PCI-X bridge */
>>> #define PCI_ERR_CAP2 0x38 /* PCIe to PCI/PCI-X bridge */
>>
>> I don't seem to understand what you mean. PCI_ERR_UNCOR_MASK2,
>> PCI_ERR_UNCOR_SEVER2, and PCI_ERR_CAP2 represent the control and handling of
>> individual errors that occur on traditional PCI or PCI-x secondary bus
>> interfaces, these registers are valid only for Bridge. Although
>> PCI_ERR_ROOT_ERR_SRC and PCI_ERR_UNCOR_SEVER2 have the same value, they
>> represent register definitions for different device types.
>
> Right. I just don't want the blank line in the middle because it
> might be mistaken for items in a different capability. All the other
> AER capability registers are defined together in a block, with no
> blank lines in the middle, so I think these new ones should be part of
> that block.
>
> Bjorn
Ok，I see your point. Do you think this line of comment is still necessary?
/* PCIe advanced error reporting extended capabilities for PCIe to
PCI/PCI-X Bridge */
Yours sincerely,
Leoliu-oc
On Tue, Apr 18, 2023 at 10:38:58AM +0800, LeoLiuoc wrote:
> 在 2023/4/13 0:10, Bjorn Helgaas 写道:
> > On Wed, Apr 12, 2023 at 05:49:55PM +0800, LeoLiuoc wrote:
> > > > ...
> > > > #define PCI_ERR_ROOT_ERR_SRC 0x34 /* Error Source Identification */
> > > > #define PCI_ERR_UNCOR_MASK2 0x30 /* PCIe to PCI/PCI-X bridge */
> > > > #define PCI_ERR_UNCOR_SEVER2 0x34 /* PCIe to PCI/PCI-X bridge */
> > > > #define PCI_ERR_CAP2 0x38 /* PCIe to PCI/PCI-X bridge */
> > >
> > > I don't seem to understand what you mean. PCI_ERR_UNCOR_MASK2,
> > > PCI_ERR_UNCOR_SEVER2, and PCI_ERR_CAP2 represent the control and handling of
> > > individual errors that occur on traditional PCI or PCI-x secondary bus
> > > interfaces, these registers are valid only for Bridge. Although
> > > PCI_ERR_ROOT_ERR_SRC and PCI_ERR_UNCOR_SEVER2 have the same value, they
> > > represent register definitions for different device types.
> >
> > Right. I just don't want the blank line in the middle because it
> > might be mistaken for items in a different capability. All the other
> > AER capability registers are defined together in a block, with no
> > blank lines in the middle, so I think these new ones should be part of
> > that block.
>
> Ok，I see your point. Do you think this line of comment is still necessary?
> /* PCIe advanced error reporting extended capabilities for PCIe to PCI/PCI-X
> Bridge */
I suggested a trailing comment ("PCIe to PCI/PCI-X bridge"). If we
use that, I don't think the other is necessary.
Bjorn