diff mbox

[v8,3/3] aerdrv: Cleanup log output for AER

Message ID 20130102232743.5706.63153.stgit@grignak.americas.hpqcorp.net (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Lance Ortiz Jan. 2, 2013, 11:27 p.m. UTC 
These changes make cper_print_aer more consistent with aer_print_error
and clean things up by elimiating the use of the prefix variable and
replacing it with dev_printk.

v1-v2 fix some compile errors withinn the #ifdef
v3-v4 remove agent id stuff and kept print the same to avoid
compatibility issues
v7-v8 Updated to use dev_printk instated of prefix. Changed
log levels to KERN_ERR as per Mauro's suggestion.

Signed-off-by: Lance Ortiz <lance.ortiz@hp.com>
---

 drivers/pci/pcie/aer/aerdrv_errprint.c |   56 ++++++++++++++++----------------
 1 files changed, 28 insertions(+), 28 deletions(-)


--
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

Comments

Joe Perches Jan. 2, 2013, 11:27 p.m. UTC | #1 
On Wed, 2013-01-02 at 16:27 -0700, Lance Ortiz wrote:
> These changes make cper_print_aer more consistent with aer_print_error
> and clean things up by elimiating the use of the prefix variable and
> replacing it with dev_printk.
[]
> v7-v8 Updated to use dev_printk instated of prefix. Changed
> log levels to KERN_ERR as per Mauro's suggestion

Just use dev_err( instead of dev_printk(KERN_ERR,
It's a function and it makes the object code smaller.

> diff --git a/drivers/pci/pcie/aer/aerdrv_errprint.c b/drivers/pci/pcie/aer/aerdrv_errprint.c
[]
> @@ -144,26 +143,24 @@ static void __aer_print_error(const char *prefix,
>  				aer_uncorrectable_error_string[i] : NULL;
>  
>  		if (errmsg)
> -			printk("%s""   [%2d] %-22s%s\n", prefix, i, errmsg,
> +			dev_printk(KERN_ERR, &dev->dev,
> +				"   [%2d] %-22s%s\n", i, errmsg,
>  				info->first_error == i ? " (First)" : "");
>  		else
> -			printk("%s""   [%2d] Unknown Error Bit%s\n", prefix, i,
> -				info->first_error == i ? " (First)" : "");
> +			dev_printk(KERN_ERR, &dev->dev,
> +				"   [%2d] Unknown Error Bit%s\n",
> +				i, info->first_error == i ? " (First)" : "");
>  	}
[]
>  	if (info->status == 0) {
> -		printk("%s""PCIe Bus Error: severity=%s, type=Unaccessible, "
> -			"id=%04x(Unregistered Agent ID)\n", prefix,
> +		dev_printk(KERN_ERR, &dev->dev,
> +			"PCIe Bus Error: severity=%s, type=Unaccessible, "
> +			"id=%04x(Unregistered Agent ID)\n",
[]
> @@ -171,22 +168,24 @@ void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
[]
> +		dev_printk(KERN_ERR, &dev->dev,
> +			"PCIe Bus Error: severity=%s, type=%s, id=%04x(%s)\n",
> +			aer_error_severity_string[info->severity],
[]
> +		dev_printk(KERN_ERR, &dev->dev,
> +			"  device [%04x:%04x] error status/mask=%08x/%08x\n",
> +			dev->vendor, dev->device,
>  			info->status, info->mask);
[]
> +			dev_printk(KERN_ERR, &dev->dev, "  TLP Header:"
>  				" %02x%02x%02x%02x %02x%02x%02x%02x"
>  				" %02x%02x%02x%02x %02x%02x%02x%02x\n",
[]
> @@ -195,8 +194,9 @@ void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
[]
> +		dev_printk(KERN_ERR, &dev->dev,
> +			   "  Error of this Agent(%04x) is reported first\n",
> +			id);
[]
> @@ -244,21 +244,21 @@ void cper_print_aer(const char *prefix, struct pci_dev *dev, int cper_severity,
[]
> +	dev_printk(KERN_ERR, &dev->dev, "aer_status: 0x%08x, aer_mask: 0x%08x\n",
> +	       status, mask);
[]
> +	dev_printk(KERN_ERR, &dev->dev, "aer_layer=%s, aer_agent=%s\n",
>  	       aer_error_layer[layer], aer_agent_string[agent]);
[]
> +		dev_printk(KERN_ERR, &dev->dev, "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
Tony Luck Jan. 2, 2013, 11:41 p.m. UTC | #2 
On Wed, Jan 2, 2013 at 3:27 PM, Joe Perches <joe@perches.com> wrote:
> Just use dev_err( instead of dev_printk(KERN_ERR,
> It's a function and it makes the object code smaller.

Looks like we are almost converged on a solution (Lance: thanks for
your patience and diligence in making changes).

Anyone on the "To:" list want to claim this for their tree to commit?
The series touches pci, acpi, RAS, and tracing ... so there are
several possible owners.

If someone else wants it, then add an:
Acked-by: Tony Luck <tony.luck@intel.com>
to all three parts.

If there isn't a strong claim, I'll add v9[*] to the RAS tree
and see if the TIP tree folks will pull it from me.

-Tony

[*] When Lance makes the change suggested by Joe.
--
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
Borislav Petkov Jan. 3, 2013, 3:51 p.m. UTC | #3 
On Wed, Jan 02, 2013 at 03:41:19PM -0800, Tony Luck wrote:
> On Wed, Jan 2, 2013 at 3:27 PM, Joe Perches <joe@perches.com> wrote:
> > Just use dev_err( instead of dev_printk(KERN_ERR,
> > It's a function and it makes the object code smaller.
> 
> Looks like we are almost converged on a solution (Lance: thanks for
> your patience and diligence in making changes).
> 
> Anyone on the "To:" list want to claim this for their tree to commit?
> The series touches pci, acpi, RAS, and tracing ... so there are
> several possible owners.
> 
> If someone else wants it, then add an:
> Acked-by: Tony Luck <tony.luck@intel.com>
> to all three parts.

Ditto.

Acked-by: Borislav Petkov <bp@alien8.de>

Thanks.
diff mbox

Patch

diff --git a/drivers/pci/pcie/aer/aerdrv_errprint.c b/drivers/pci/pcie/aer/aerdrv_errprint.c
index d3e5fc5..dfd9641 100644
--- a/drivers/pci/pcie/aer/aerdrv_errprint.c
+++ b/drivers/pci/pcie/aer/aerdrv_errprint.c
@@ -124,12 +124,11 @@  static const char *aer_agent_string[] = {
 	"Transmitter ID"
 };
 
-static void __aer_print_error(const char *prefix,
+static void __aer_print_error(struct pci_dev *dev,
 			      struct aer_err_info *info)
 {
 	int i, status;
 	const char *errmsg = NULL;
-
 	status = (info->status & ~info->mask);
 
 	for (i = 0; i < 32; i++) {
@@ -144,26 +143,24 @@  static void __aer_print_error(const char *prefix,
 				aer_uncorrectable_error_string[i] : NULL;
 
 		if (errmsg)
-			printk("%s""   [%2d] %-22s%s\n", prefix, i, errmsg,
+			dev_printk(KERN_ERR, &dev->dev,
+				"   [%2d] %-22s%s\n", i, errmsg,
 				info->first_error == i ? " (First)" : "");
 		else
-			printk("%s""   [%2d] Unknown Error Bit%s\n", prefix, i,
-				info->first_error == i ? " (First)" : "");
+			dev_printk(KERN_ERR, &dev->dev,
+				"   [%2d] Unknown Error Bit%s\n",
+				i, info->first_error == i ? " (First)" : "");
 	}
 }
 
 void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
 {
 	int id = ((dev->bus->number << 8) | dev->devfn);
-	char prefix[44];
-
-	snprintf(prefix, sizeof(prefix), "%s%s %s: ",
-		 (info->severity == AER_CORRECTABLE) ? KERN_WARNING : KERN_ERR,
-		 dev_driver_string(&dev->dev), dev_name(&dev->dev));
 
 	if (info->status == 0) {
-		printk("%s""PCIe Bus Error: severity=%s, type=Unaccessible, "
-			"id=%04x(Unregistered Agent ID)\n", prefix,
+		dev_printk(KERN_ERR, &dev->dev,
+			"PCIe Bus Error: severity=%s, type=Unaccessible, "
+			"id=%04x(Unregistered Agent ID)\n",
 			aer_error_severity_string[info->severity], id);
 	} else {
 		int layer, agent;
@@ -171,22 +168,24 @@  void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
 		layer = AER_GET_LAYER_ERROR(info->severity, info->status);
 		agent = AER_GET_AGENT(info->severity, info->status);
 
-		printk("%s""PCIe Bus Error: severity=%s, type=%s, id=%04x(%s)\n",
-			prefix, aer_error_severity_string[info->severity],
+		dev_printk(KERN_ERR, &dev->dev,
+			"PCIe Bus Error: severity=%s, type=%s, id=%04x(%s)\n",
+			aer_error_severity_string[info->severity],
 			aer_error_layer[layer], id, aer_agent_string[agent]);
 
-		printk("%s""  device [%04x:%04x] error status/mask=%08x/%08x\n",
-			prefix, dev->vendor, dev->device,
+		dev_printk(KERN_ERR, &dev->dev,
+			"  device [%04x:%04x] error status/mask=%08x/%08x\n",
+			dev->vendor, dev->device,
 			info->status, info->mask);
 
-		__aer_print_error(prefix, info);
+		__aer_print_error(dev, info);
 
 		if (info->tlp_header_valid) {
 			unsigned char *tlp = (unsigned char *) &info->tlp;
-			printk("%s""  TLP Header:"
+			dev_printk(KERN_ERR, &dev->dev, "  TLP Header:"
 				" %02x%02x%02x%02x %02x%02x%02x%02x"
 				" %02x%02x%02x%02x %02x%02x%02x%02x\n",
-				prefix, *(tlp + 3), *(tlp + 2), *(tlp + 1), *tlp,
+				*(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),
@@ -195,8 +194,9 @@  void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
 	}
 
 	if (info->id && info->error_dev_num > 1 && info->id == id)
-		printk("%s""  Error of this Agent(%04x) is reported first\n",
-			prefix, id);
+		dev_printk(KERN_ERR, &dev->dev,
+			   "  Error of this Agent(%04x) is reported first\n",
+			id);
 	trace_aer_event(dev_name(&dev->dev), (info->status & ~info->mask),
 			info->severity);
 }
@@ -244,21 +244,21 @@  void cper_print_aer(const char *prefix, struct pci_dev *dev, int cper_severity,
 	}
 	layer = AER_GET_LAYER_ERROR(aer_severity, status);
 	agent = AER_GET_AGENT(aer_severity, status);
-	printk("%s""aer_status: 0x%08x, aer_mask: 0x%08x\n",
-	       prefix, status, mask);
+	dev_printk(KERN_ERR, &dev->dev, "aer_status: 0x%08x, aer_mask: 0x%08x\n",
+	       status, mask);
 	cper_print_bits(prefix, status, status_strs, status_strs_size);
-	printk("%s""aer_layer=%s, aer_agent=%s\n", prefix,
+	dev_printk(KERN_ERR, &dev->dev, "aer_layer=%s, aer_agent=%s\n",
 	       aer_error_layer[layer], aer_agent_string[agent]);
 	if (aer_severity != AER_CORRECTABLE)
-		printk("%s""aer_uncor_severity: 0x%08x\n",
-		       prefix, aer->uncor_severity);
+		dev_printk(KERN_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;
-		printk("%s""aer_tlp_header:"
+		dev_printk(KERN_ERR, &dev->dev, "aer_tlp_header:"
 			" %02x%02x%02x%02x %02x%02x%02x%02x"
 			" %02x%02x%02x%02x %02x%02x%02x%02x\n",
-			prefix, *(tlp + 3), *(tlp + 2), *(tlp + 1), *tlp,
+			*(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),