| Message ID | 1407313964-20794-1-git-send-email-gong.chen@linux.intel.com |
|---|---|
| State | Superseded |
| Headers | show |
On Wed, Aug 06, 2014 at 11:32:23AM +0200, Borislav Petkov wrote: > Date: Wed, 6 Aug 2014 11:32:23 +0200 > From: Borislav Petkov <bp@alien8.de> > To: "Chen, Gong" <gong.chen@linux.intel.com> > Cc: bhelgaas@google.com, rdunlap@infradead.org, tony.luck@intel.com, > linux-pci@vger.kernel.org > Subject: Re: [PATCH 1/5] RAS, trace: Update error definition format > User-Agent: Mutt/1.5.23 (2014-03-12) > > On Wed, Aug 06, 2014 at 04:32:40AM -0400, Chen, Gong wrote: > > Previous format definition uses MACRO BIT(...), which is not very > > maintainable. > > What does that "not very maintainable" mean? > Bjorn ever mentioned for this: "I'd like to see all those "BIT(...)" things changed to use the #defines that already exist in include/uapi/linux/pci_regs.h, e.g., PCI_ERR_COR_RCVR. That way grep will find these uses, which will make maintenance easier."
On Wed, Aug 06, 2014 at 04:32:40AM -0400, Chen, Gong wrote: > Previous format definition uses MACRO BIT(...), which is not very > maintainable. What does that "not very maintainable" mean?
On Wed, Aug 06, 2014 at 05:08:54AM -0400, Chen, Gong wrote: > Bjorn ever mentioned for this: > "I'd like to see all those "BIT(...)" things changed to use the #defines > that already exist in include/uapi/linux/pci_regs.h, e.g., > PCI_ERR_COR_RCVR. That way grep will find these uses, which will make > maintenance easier." So explain that in the commit message but don't use some bits out of context. In general, when you read your own commit message, always ask yourself whether other people will be able to understand it, long time from now and out of context. If yes, only then send out the patch.
On Wed, Aug 06, 2014 at 11:59:39AM +0200, Borislav Petkov wrote: > > Bjorn ever mentioned for this: > > "I'd like to see all those "BIT(...)" things changed to use the #defines > > that already exist in include/uapi/linux/pci_regs.h, e.g., > > PCI_ERR_COR_RCVR. That way grep will find these uses, which will make > > maintenance easier." > > So explain that in the commit message but don't use some bits out of > context. > > In general, when you read your own commit message, always ask yourself > whether other people will be able to understand it, long time from now > and out of context. > > If yes, only then send out the patch. > Copy that. Thx a lot!
On Wed, Aug 06, 2014 at 04:32:40AM -0400, Chen, Gong wrote: > Previous format definition uses MACRO BIT(...), which is not very > maintainable. Use unified MACRO as substitution. > Hi, Bjorn Any comments for this series?
diff --git a/include/ras/ras_event.h b/include/ras/ras_event.h index 47da53c..0f2cca4 100644 --- a/include/ras/ras_event.h +++ b/include/ras/ras_event.h @@ -8,6 +8,7 @@ #include <linux/tracepoint.h> #include <linux/edac.h> #include <linux/ktime.h> +#include <linux/pci.h> #include <linux/aer.h> #include <linux/cper.h> @@ -174,24 +175,24 @@ TRACE_EVENT(mc_event, */ #define aer_correctable_errors \ - {BIT(0), "Receiver Error"}, \ - {BIT(6), "Bad TLP"}, \ - {BIT(7), "Bad DLLP"}, \ - {BIT(8), "RELAY_NUM Rollover"}, \ - {BIT(12), "Replay Timer Timeout"}, \ - {BIT(13), "Advisory Non-Fatal"} + {PCI_ERR_COR_RCVR, "Receiver Error"}, \ + {PCI_ERR_COR_BAD_TLP, "Bad TLP"}, \ + {PCI_ERR_COR_BAD_DLLP, "Bad DLLP"}, \ + {PCI_ERR_COR_REP_ROLL, "RELAY_NUM Rollover"}, \ + {PCI_ERR_COR_REP_TIMER, "Replay Timer Timeout"},\ + {PCI_ERR_COR_ADV_NFAT, "Advisory Non-Fatal"} #define aer_uncorrectable_errors \ - {BIT(4), "Data Link Protocol"}, \ - {BIT(12), "Poisoned TLP"}, \ - {BIT(13), "Flow Control Protocol"}, \ - {BIT(14), "Completion Timeout"}, \ - {BIT(15), "Completer Abort"}, \ - {BIT(16), "Unexpected Completion"}, \ - {BIT(17), "Receiver Overflow"}, \ - {BIT(18), "Malformed TLP"}, \ - {BIT(19), "ECRC"}, \ - {BIT(20), "Unsupported Request"} + {PCI_ERR_UNC_DLP, "Data Link Protocol"}, \ + {PCI_ERR_UNC_POISON_TLP,"Poisoned TLP"}, \ + {PCI_ERR_UNC_FCP, "Flow Control Protocol"}, \ + {PCI_ERR_UNC_COMP_TIME, "Completion Timeout"}, \ + {PCI_ERR_UNC_COMP_ABORT,"Completer Abort"}, \ + {PCI_ERR_UNC_UNX_COMP, "Unexpected Completion"}, \ + {PCI_ERR_UNC_RX_OVER, "Receiver Overflow"}, \ + {PCI_ERR_UNC_MALF_TLP, "Malformed TLP"}, \ + {PCI_ERR_UNC_ECRC, "ECRC"}, \ + {PCI_ERR_UNC_UNSUP, "Unsupported Request"} TRACE_EVENT(aer_event, TP_PROTO(const char *dev_name,
Previous format definition uses MACRO BIT(...), which is not very maintainable. Use unified MACRO as substitution. Signed-off-by: Chen, Gong <gong.chen@linux.intel.com> --- include/ras/ras_event.h | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-)