Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 12 Apr 2023 11:10:12 -0500
From:   Bjorn Helgaas <helgaas@kernel.org>
To:     LeoLiuoc <LeoLiu-oc@zhaoxin.com>
Cc:     rafael@kernel.org, lenb@kernel.org, james.morse@arm.com,
        tony.luck@intel.com, bp@alien8.de, robert.moore@intel.com,
        ying.huang@intel.com, rdunlap@infradead.org, bhelgaas@google.com,
        linux-acpi@vger.kernel.org, linux-pci@vger.kernel.org,
        linux-kernel@vger.kernel.org, devel@acpica.org,
        CobeChen@zhaoxin.com, TonyWWang@zhaoxin.com, ErosZhang@zhaoxin.com
Subject: Re: [PATCH v2 3/5] ACPI/PCI: Add AER bits #defines for PCIe to
 PCI/PCI-X Bridge
Message-ID: <20230412161012.GA48719@bhelgaas>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <f3c4f38c-d45b-cf68-33e5-2e73dd73213d@zhaoxin.com>
Precedence: bulk

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