Re: [PATCH 08/22] aerdrv: rework do_recovery

[Kenji Kaneshige <kaneshige.kenji@xxxxxxxxxxxxxx>]

Reviewed-by: Kenji Kaneshige <kaneshige.kenji@xxxxxxxxxxxxxx>

Hidetoshi Seto wrote:
> Move dev_printks for debug into do_recovery().
> This allows do_recovery() to return void.
>
> Signed-off-by: Hidetoshi Seto <seto.hidetoshi@xxxxxxxxxxxxxx>
> ---
>  drivers/pci/pcie/aer/aerdrv_core.c |   38 ++++++++++++++++-------------------
>  1 files changed, 17 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/pci/pcie/aer/aerdrv_core.c b/drivers/pci/pcie/aer/aerdrv_core.c
> index 1d6a9ec..da62951 100644
> --- a/drivers/pci/pcie/aer/aerdrv_core.c
> +++ b/drivers/pci/pcie/aer/aerdrv_core.c
> @@ -476,8 +476,7 @@ static pci_ers_result_t reset_link(struct pcie_device *aerdev,
>   * error detected message to all downstream drivers within a hierarchy in
>   * question and return the returned code.
>   */
> -static pci_ers_result_t do_recovery(struct pcie_device *aerdev,
> -		struct pci_dev *dev,
> +static void do_recovery(struct pcie_device *aerdev, struct pci_dev *dev,
>  		int severity)
>  {
>  	pci_ers_result_t status, result = PCI_ERS_RESULT_RECOVERED;
> @@ -495,10 +494,8 @@ static pci_ers_result_t do_recovery(struct pcie_device *aerdev,
>  
>  	if (severity == AER_FATAL) {
>  		result = reset_link(aerdev, dev);
> -		if (result != PCI_ERS_RESULT_RECOVERED) {
> -			/* TODO: Should panic here? */
> -			return result;
> -		}
> +		if (result != PCI_ERS_RESULT_RECOVERED)
> +			goto failed;
>  	}
>  
>  	if (status == PCI_ERS_RESULT_CAN_RECOVER)
> @@ -519,13 +516,22 @@ static pci_ers_result_t do_recovery(struct pcie_device *aerdev,
>  				report_slot_reset);
>  	}
>  
> -	if (status == PCI_ERS_RESULT_RECOVERED)
> -		broadcast_error_message(dev,
> +	if (status != PCI_ERS_RESULT_RECOVERED)
> +		goto failed;
> +
> +	broadcast_error_message(dev,
>  				state,
>  				"resume",
>  				report_resume);
>  
> -	return status;
> +	dev_printk(KERN_DEBUG, &dev->dev,
> +		"AER driver successfully recovered\n");
> +	return;
> +
> +failed:
> +	/* TODO: Should kernel panic here? */
> +	dev_printk(KERN_DEBUG, &dev->dev,
> +		"AER driver didn't recover\n");
>  }
>  
>  /**
> @@ -540,7 +546,6 @@ static void handle_error_source(struct pcie_device *aerdev,
>  	struct pci_dev *dev,
>  	struct aer_err_info *info)
>  {
> -	pci_ers_result_t status = 0;
>  	int pos;
>  
>  	if (info->severity == AER_CORRECTABLE) {
> @@ -552,17 +557,8 @@ static void handle_error_source(struct pcie_device *aerdev,
>  		if (pos)
>  			pci_write_config_dword(dev, pos + PCI_ERR_COR_STATUS,
>  					info->status);
> -	} else {
> -		status = do_recovery(aerdev, dev, info->severity);
> -		if (status == PCI_ERS_RESULT_RECOVERED) {
> -			dev_printk(KERN_DEBUG, &dev->dev, "AER driver "
> -				   "successfully recovered\n");
> -		} else {
> -			/* TODO: Should kernel panic here? */
> -			dev_printk(KERN_DEBUG, &dev->dev, "AER driver didn't "
> -				   "recover\n");
> -		}
> -	}
> +	} else
> +		do_recovery(aerdev, dev, info->severity);
>  }
>  
>  /**


--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html