| Message ID | 1386515571-23252-1-git-send-email-bp@alien8.de (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On Sun, 2013-12-08 at 16:12 +0100, Borislav Petkov wrote: > From: Borislav Petkov <bp@suse.de> > > ... and call it instead of duplicating the large printk format > statement. > > No functionality change. You do change the output. Perhaps it'd be better to pass the header prefix too. > diff --git a/drivers/pci/pcie/aer/aerdrv_errprint.c b/drivers/pci/pcie/aer/aerdrv_errprint.c [] > @@ -124,6 +124,21 @@ static const char *aer_agent_string[] = { > +static void __print_tlp_header(struct pci_dev *dev, > + struct aer_header_log_regs *t) > +{ > + unsigned char *tlp = (unsigned char *)&t; > + > + dev_err(&dev->dev, " TLP Header:" Always indents and emits " TLP Header:" > @@ -250,18 +256,10 @@ void cper_print_aer(struct pci_dev *dev, int cper_severity, > if (aer_severity != AER_CORRECTABLE) > dev_err(&dev->dev, "aer_uncor_severity: 0x%08x\n", > aer->uncor_severity); > - if (tlp_header_valid) { > - const unsigned char *tlp; > - tlp = (const unsigned char *)&aer->header_log; > - dev_err(&dev->dev, "aer_tlp_header:" vs removing this "aer_tlp_header:" -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Dec 08, 2013 at 08:05:24AM -0800, Joe Perches wrote: > On Sun, 2013-12-08 at 16:12 +0100, Borislav Petkov wrote: > > From: Borislav Petkov <bp@suse.de> > > > > ... and call it instead of duplicating the large printk format > > statement. > > > > No functionality change. > > You do change the output. That's ok. "TLP Header:" is the proper way to print it anyway.
On Sun, 2013-12-08 at 16:12 +0100, Borislav Petkov wrote: > From: Borislav Petkov <bp@suse.de> > > ... and call it instead of duplicating the large printk format > statement. [] > diff --git a/drivers/pci/pcie/aer/aerdrv_errprint.c b/drivers/pci/pcie/aer/aerdrv_errprint.c [] > @@ -124,6 +124,21 @@ static const char *aer_agent_string[] = { > +static void __print_tlp_header(struct pci_dev *dev, > + struct aer_header_log_regs *t) > +{ > + unsigned char *tlp = (unsigned char *)&t; > + > + dev_err(&dev->dev, " TLP Header:" > + " %02x%02x%02x%02x %02x%02x%02x%02x" > + " %02x%02x%02x%02x %02x%02x%02x%02x\n", > + *(tlp + 3), *(tlp + 2), *(tlp + 1), *tlp, > + *(tlp + 7), *(tlp + 6), *(tlp + 5), *(tlp + 4), > + *(tlp + 11), *(tlp + 10), *(tlp + 9), > + *(tlp + 8), *(tlp + 15), *(tlp + 14), > + *(tlp + 13), *(tlp + 12)); > +} Hey again Borislav: Come to think of it, given this struct: include/linux/aer.h:struct aer_header_log_regs { include/linux/aer.h- unsigned int dw0; include/linux/aer.h- unsigned int dw1; include/linux/aer.h- unsigned int dw2; include/linux/aer.h- unsigned int dw3; include/linux/aer.h-}; this would be a lot more readable as: static void __print_tlp_header(struct pci_dev *dev, struct aer_header_log_regs *t) { dev_err(&dev->dev, " TLP Header: %08x %08x %08x %08x\n", tlp->dw0, tlp->dw1, tlp->dw2, tlp->dw3); } though if the desire really is to show le output, maybe these should use: dev_err(&dev->dev, " TLP Header: %08x %08x %08x %08x\n", cpu_to_le32(tlp->dw0), cpu_to_le32(tlp->dw1), cpu_to_le32(tlp->dw2), cpu_to_le32(tlp->dw3)); -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Dec 08, 2013 at 04:12:50PM +0100, Borislav Petkov wrote: > From: Borislav Petkov <bp@suse.de> > > ... and call it instead of duplicating the large printk format > statement. > > No functionality change. > > Signed-off-by: Borislav Petkov <bp@suse.de> I applied both of these to my pci/aer branch for v3.14, thanks! Bjorn > --- > drivers/pci/pcie/aer/aerdrv_errprint.c | 44 ++++++++++++++++------------------ > 1 file changed, 21 insertions(+), 23 deletions(-) > > diff --git a/drivers/pci/pcie/aer/aerdrv_errprint.c b/drivers/pci/pcie/aer/aerdrv_errprint.c > index 2c7c9f5f592c..32efc5e0d2eb 100644 > --- a/drivers/pci/pcie/aer/aerdrv_errprint.c > +++ b/drivers/pci/pcie/aer/aerdrv_errprint.c > @@ -124,6 +124,21 @@ static const char *aer_agent_string[] = { > "Transmitter ID" > }; > > +static void __print_tlp_header(struct pci_dev *dev, > + struct aer_header_log_regs *t) > +{ > + unsigned char *tlp = (unsigned char *)&t; > + > + dev_err(&dev->dev, " TLP Header:" > + " %02x%02x%02x%02x %02x%02x%02x%02x" > + " %02x%02x%02x%02x %02x%02x%02x%02x\n", > + *(tlp + 3), *(tlp + 2), *(tlp + 1), *tlp, > + *(tlp + 7), *(tlp + 6), *(tlp + 5), *(tlp + 4), > + *(tlp + 11), *(tlp + 10), *(tlp + 9), > + *(tlp + 8), *(tlp + 15), *(tlp + 14), > + *(tlp + 13), *(tlp + 12)); > +} > + > static void __aer_print_error(struct pci_dev *dev, > struct aer_err_info *info) > { > @@ -178,17 +193,8 @@ void aer_print_error(struct pci_dev *dev, struct aer_err_info *info) > > __aer_print_error(dev, info); > > - if (info->tlp_header_valid) { > - unsigned char *tlp = (unsigned char *) &info->tlp; > - dev_err(&dev->dev, " TLP Header:" > - " %02x%02x%02x%02x %02x%02x%02x%02x" > - " %02x%02x%02x%02x %02x%02x%02x%02x\n", > - *(tlp + 3), *(tlp + 2), *(tlp + 1), *tlp, > - *(tlp + 7), *(tlp + 6), *(tlp + 5), *(tlp + 4), > - *(tlp + 11), *(tlp + 10), *(tlp + 9), > - *(tlp + 8), *(tlp + 15), *(tlp + 14), > - *(tlp + 13), *(tlp + 12)); > - } > + if (info->tlp_header_valid) > + __print_tlp_header(dev, &info->tlp); > } > > if (info->id && info->error_dev_num > 1 && info->id == id) > @@ -250,18 +256,10 @@ void cper_print_aer(struct pci_dev *dev, int cper_severity, > if (aer_severity != AER_CORRECTABLE) > dev_err(&dev->dev, "aer_uncor_severity: 0x%08x\n", > aer->uncor_severity); > - if (tlp_header_valid) { > - const unsigned char *tlp; > - tlp = (const unsigned char *)&aer->header_log; > - dev_err(&dev->dev, "aer_tlp_header:" > - " %02x%02x%02x%02x %02x%02x%02x%02x" > - " %02x%02x%02x%02x %02x%02x%02x%02x\n", > - *(tlp + 3), *(tlp + 2), *(tlp + 1), *tlp, > - *(tlp + 7), *(tlp + 6), *(tlp + 5), *(tlp + 4), > - *(tlp + 11), *(tlp + 10), *(tlp + 9), > - *(tlp + 8), *(tlp + 15), *(tlp + 14), > - *(tlp + 13), *(tlp + 12)); > - } > + > + if (tlp_header_valid) > + __print_tlp_header(dev, &aer->header_log); > + > trace_aer_event(dev_name(&dev->dev), (status & ~mask), > aer_severity); > } > -- > 1.8.4 > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/pci/pcie/aer/aerdrv_errprint.c b/drivers/pci/pcie/aer/aerdrv_errprint.c index 2c7c9f5f592c..32efc5e0d2eb 100644 --- a/drivers/pci/pcie/aer/aerdrv_errprint.c +++ b/drivers/pci/pcie/aer/aerdrv_errprint.c @@ -124,6 +124,21 @@ static const char *aer_agent_string[] = { "Transmitter ID" }; +static void __print_tlp_header(struct pci_dev *dev, + struct aer_header_log_regs *t) +{ + unsigned char *tlp = (unsigned char *)&t; + + dev_err(&dev->dev, " TLP Header:" + " %02x%02x%02x%02x %02x%02x%02x%02x" + " %02x%02x%02x%02x %02x%02x%02x%02x\n", + *(tlp + 3), *(tlp + 2), *(tlp + 1), *tlp, + *(tlp + 7), *(tlp + 6), *(tlp + 5), *(tlp + 4), + *(tlp + 11), *(tlp + 10), *(tlp + 9), + *(tlp + 8), *(tlp + 15), *(tlp + 14), + *(tlp + 13), *(tlp + 12)); +} + static void __aer_print_error(struct pci_dev *dev, struct aer_err_info *info) { @@ -178,17 +193,8 @@ void aer_print_error(struct pci_dev *dev, struct aer_err_info *info) __aer_print_error(dev, info); - if (info->tlp_header_valid) { - unsigned char *tlp = (unsigned char *) &info->tlp; - dev_err(&dev->dev, " TLP Header:" - " %02x%02x%02x%02x %02x%02x%02x%02x" - " %02x%02x%02x%02x %02x%02x%02x%02x\n", - *(tlp + 3), *(tlp + 2), *(tlp + 1), *tlp, - *(tlp + 7), *(tlp + 6), *(tlp + 5), *(tlp + 4), - *(tlp + 11), *(tlp + 10), *(tlp + 9), - *(tlp + 8), *(tlp + 15), *(tlp + 14), - *(tlp + 13), *(tlp + 12)); - } + if (info->tlp_header_valid) + __print_tlp_header(dev, &info->tlp); } if (info->id && info->error_dev_num > 1 && info->id == id) @@ -250,18 +256,10 @@ void cper_print_aer(struct pci_dev *dev, int cper_severity, if (aer_severity != AER_CORRECTABLE) dev_err(&dev->dev, "aer_uncor_severity: 0x%08x\n", aer->uncor_severity); - if (tlp_header_valid) { - const unsigned char *tlp; - tlp = (const unsigned char *)&aer->header_log; - dev_err(&dev->dev, "aer_tlp_header:" - " %02x%02x%02x%02x %02x%02x%02x%02x" - " %02x%02x%02x%02x %02x%02x%02x%02x\n", - *(tlp + 3), *(tlp + 2), *(tlp + 1), *tlp, - *(tlp + 7), *(tlp + 6), *(tlp + 5), *(tlp + 4), - *(tlp + 11), *(tlp + 10), *(tlp + 9), - *(tlp + 8), *(tlp + 15), *(tlp + 14), - *(tlp + 13), *(tlp + 12)); - } + + if (tlp_header_valid) + __print_tlp_header(dev, &aer->header_log); + trace_aer_event(dev_name(&dev->dev), (status & ~mask), aer_severity); }