diff mbox

[5/8] pcie, aer: fix report of multiple errors

Message ID 4A8E18D0.6050701@jp.fujitsu.com (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Hidetoshi Seto Aug. 21, 2009, 3:47 a.m. UTC
The flag AER_MULTI_ERROR_VALID_FLAG in info->flag does mean that the
root port received multiple error messages.  Error messages can be
posted from different devices, so it does not mean that each reported
device has multiple errors.

If there are multiple error devices and the root port has valid error
source ID, it would be nice to report which device is the error source
reported first.

Signed-off-by: Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com>
---
 drivers/pci/pcie/aer/aerdrv_errprint.c |   19 ++++++++-----------
 1 files changed, 8 insertions(+), 11 deletions(-)

Comments

Andrew Patterson Aug. 27, 2009, 4:51 a.m. UTC | #1
On Fri, 2009-08-21 at 12:47 +0900, Hidetoshi Seto wrote:
> The flag AER_MULTI_ERROR_VALID_FLAG in info->flag does mean that the
> root port received multiple error messages.  Error messages can be
> posted from different devices, so it does not mean that each reported
> device has multiple errors.
> 
> If there are multiple error devices and the root port has valid error
> source ID, it would be nice to report which device is the error source
> reported first.
> 
> Signed-off-by: Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com>
> ---
>  drivers/pci/pcie/aer/aerdrv_errprint.c |   19 ++++++++-----------
>  1 files changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/pci/pcie/aer/aerdrv_errprint.c b/drivers/pci/pcie/aer/aerdrv_errprint.c
> index 48f70fa..d584ffd 100644
> --- a/drivers/pci/pcie/aer/aerdrv_errprint.c
> +++ b/drivers/pci/pcie/aer/aerdrv_errprint.c
> @@ -185,6 +185,7 @@ void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
>  {
>  	char *errmsg;
>  	int err_layer, agent;
> +	int id = ((dev->bus->number << 8) | dev->devfn);
>  
>  	AER_PR(info, "+------ PCI-Express Device Error ------+\n");
>  	AER_PR(info, "Error Severity\t\t: %s\n",
> @@ -192,11 +193,7 @@ void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
>  
>  	if (info->status == 0) {
>  		AER_PR(info, "PCIE Bus Error type\t: (Unaccessible)\n");
> -		AER_PR(info, "Unaccessible Received\t: %s\n",
> -			info->flags & AER_MULTI_ERROR_VALID_FLAG ?
> -				"Multiple" : "First");
> -		AER_PR(info, "Unregistered Agent ID\t: %04x\n",
> -			(dev->bus->number << 8) | dev->devfn);
> +		AER_PR(info, "Unregistered Agent ID\t: %04x\n", id);
>  	} else {
>  		err_layer = AER_GET_LAYER_ERROR(info->severity, info->status);
>  		AER_PR(info, "PCIE Bus Error type\t: %s\n",
> @@ -206,15 +203,11 @@ void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
>  		errmsg = aer_get_error_source_name(info->severity,
>  				info->status,
>  				errmsg_buff);
> -		AER_PR(info, "%s\t: %s\n", errmsg,
> -			info->flags & AER_MULTI_ERROR_VALID_FLAG ?
> -				"Multiple" : "First");
> +		AER_PR(info, "%s\t:\n", errmsg);
>  		spin_unlock(&logbuf_lock);
>  
>  		agent = AER_GET_AGENT(info->severity, info->status);
> -		AER_PR(info, "%s\t\t: %04x\n",
> -			aer_agent_string[agent],
> -			(dev->bus->number << 8) | dev->devfn);
> +		AER_PR(info, "%s\t\t: %04x\n", aer_agent_string[agent], id);
>  
>  		AER_PR(info, "VendorID=%04xh, DeviceID=%04xh,"
>  			" Bus=%02xh, Device=%02xh, Function=%02xh\n",
> @@ -236,4 +229,8 @@ void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
>  				*(tlp + 13), *(tlp + 12));
>  		}
>  	}
> +
> +	if (info->id && info->error_dev_num > 1)
> +		AER_PR(info, "Source Reported First\t: %s\n",

Do we need every word capitalized?  I realize that the capitalization is
sort of consistently followed in the rest of the function, but perhaps
we can stop extending it. How about "Source reported first\t:"? 

I don't think this message will make much sense to the user. How about:

"Device that first reported an error\t:"

Or something similar?

> +			info->id == id ? "yes" : "no");
>  }
diff mbox

Patch

diff --git a/drivers/pci/pcie/aer/aerdrv_errprint.c b/drivers/pci/pcie/aer/aerdrv_errprint.c
index 48f70fa..d584ffd 100644
--- a/drivers/pci/pcie/aer/aerdrv_errprint.c
+++ b/drivers/pci/pcie/aer/aerdrv_errprint.c
@@ -185,6 +185,7 @@  void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
 {
 	char *errmsg;
 	int err_layer, agent;
+	int id = ((dev->bus->number << 8) | dev->devfn);
 
 	AER_PR(info, "+------ PCI-Express Device Error ------+\n");
 	AER_PR(info, "Error Severity\t\t: %s\n",
@@ -192,11 +193,7 @@  void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
 
 	if (info->status == 0) {
 		AER_PR(info, "PCIE Bus Error type\t: (Unaccessible)\n");
-		AER_PR(info, "Unaccessible Received\t: %s\n",
-			info->flags & AER_MULTI_ERROR_VALID_FLAG ?
-				"Multiple" : "First");
-		AER_PR(info, "Unregistered Agent ID\t: %04x\n",
-			(dev->bus->number << 8) | dev->devfn);
+		AER_PR(info, "Unregistered Agent ID\t: %04x\n", id);
 	} else {
 		err_layer = AER_GET_LAYER_ERROR(info->severity, info->status);
 		AER_PR(info, "PCIE Bus Error type\t: %s\n",
@@ -206,15 +203,11 @@  void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
 		errmsg = aer_get_error_source_name(info->severity,
 				info->status,
 				errmsg_buff);
-		AER_PR(info, "%s\t: %s\n", errmsg,
-			info->flags & AER_MULTI_ERROR_VALID_FLAG ?
-				"Multiple" : "First");
+		AER_PR(info, "%s\t:\n", errmsg);
 		spin_unlock(&logbuf_lock);
 
 		agent = AER_GET_AGENT(info->severity, info->status);
-		AER_PR(info, "%s\t\t: %04x\n",
-			aer_agent_string[agent],
-			(dev->bus->number << 8) | dev->devfn);
+		AER_PR(info, "%s\t\t: %04x\n", aer_agent_string[agent], id);
 
 		AER_PR(info, "VendorID=%04xh, DeviceID=%04xh,"
 			" Bus=%02xh, Device=%02xh, Function=%02xh\n",
@@ -236,4 +229,8 @@  void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
 				*(tlp + 13), *(tlp + 12));
 		}
 	}
+
+	if (info->id && info->error_dev_num > 1)
+		AER_PR(info, "Source Reported First\t: %s\n",
+			info->id == id ? "yes" : "no");
 }