On Fri, Sep 05, 2014 at 05:15:43PM -0600, Bjorn Helgaas wrote:
> Date: Fri, 5 Sep 2014 17:15:43 -0600
> From: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> To: "Chen, Gong" <gong.chen@xxxxxxxxxxxxxxx>
> Cc: rdunlap@xxxxxxxxxxxxx, bp@xxxxxxxxx, tony.luck@xxxxxxxxx,
> linux-pci@xxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [RESEND 2/5] PCIe, AER: Replenish missed AER status bits for
> AER driver
> User-Agent: Mutt/1.5.21 (2010-09-15)
>
> On Wed, Aug 13, 2014 at 02:22:38AM -0400, Chen, Gong wrote:
> > Since commit 6c2b374d is commited, the capability of PCI-e AER
> > has changed a lot. This patch adds all missed CE/UC error bits
> > existed in PCI-e SPEC r3.0. Meanwhile, adjust the code format
> > to make it simpler to read/maintain.
> >
> > Signed-off-by: Chen, Gong <gong.chen@xxxxxxxxxxxxxxx>
> > ---
> > drivers/pci/pcie/aer/aerdrv_errprint.c | 60 ++++++++++++++--------------------
> > 1 file changed, 25 insertions(+), 35 deletions(-)
> >
> > diff --git a/drivers/pci/pcie/aer/aerdrv_errprint.c b/drivers/pci/pcie/aer/aerdrv_errprint.c
> > index 35d06e177917..5c4f7e252e5e 100644
> > --- a/drivers/pci/pcie/aer/aerdrv_errprint.c
> > +++ b/drivers/pci/pcie/aer/aerdrv_errprint.c
> > @@ -75,44 +75,34 @@ static const char *aer_error_layer[] = {
> > };
> >
> > static const char *aer_correctable_error_string[] = {
> > - "Receiver Error", /* Bit Position 0 */
> > - NULL,
> > - NULL,
> > - NULL,
> > - NULL,
> > - NULL,
> > - "Bad TLP", /* Bit Position 6 */
> > - "Bad DLLP", /* Bit Position 7 */
> > - "RELAY_NUM Rollover", /* Bit Position 8 */
> > - NULL,
> > - NULL,
> > - NULL,
> > - "Replay Timer Timeout", /* Bit Position 12 */
> > - "Advisory Non-Fatal", /* Bit Position 13 */
> > + [0] = "Receiver Error",
> > + [6] = "Bad TLP",
> > + [7] = "Bad DLLP",
> > + [8] = "RELAY_NUM Rollover",
> > + [12] = "Replay Timer Timeout",
> > + [13] = "Advisory Non-Fatal Error",
> > + [14] = "Corrected Internal Error",
> > + [15] = "Header Log Overflow",
>
> You replaced bare numbers with the existing #defines in the previous patch
> (thank you), but now we're adding them here. I'm pretty sure you can use
> the #defines here, e.g.,
>
> [PCI_ERR_COR_RCVR] = "Receiver Error",
Considering PCI_ERR_COR_* stuff are not BIT offset, I need a conversion like
[ilog2(PCI_ERR_COR_RCVR)] = "xxx". But in ras_event.h I need the same
conversion like aer_correctable_error_string[ilog2(PCI_ERR_COR_RCVR)]. It looks
like a little bit clumsy and suboptimal. I can add extra BIT definition in
include/uapi/linux/pci_regs.h like below:
#define PCI_ERR_COR_RCVR 0x00000001 /* Receiver Error Status */
+#define PCI_ERR_COR_RCVR_BIT ilog2(PCI_ERR_COR_RCVR)
or more direct way:
#define PCI_ERR_COR_RCVR 0x00000001 /* Receiver Error Status */
+#define PCI_ERR_COR_RCVR_BIT 0
I can't find better method by now.
>
> In fact, it would be really nice if you could figure out a way to have only
> one set of these strings. Right now, we have the set in
As above implied, I can export aer_correctable_error_string etc.
Attachment:
signature.asc
Description: Digital signature
