Re: [PATCH 11/14] powerpc/eeh: Set attention indicator while recovering

[Sam Bobroff <sbobroff@linux.ibm.com>]

[-- Attachment #1: Type: text/plain, Size: 2535 bytes --]

On Tue, Sep 03, 2019 at 08:16:02PM +1000, Oliver O'Halloran wrote:
> I am the RAS team. Hear me roar.
> 
> Roar.
> 
> On a more serious note, being able to locate failed devices can be helpful.
> Set the attention indicator if the slot supports it once we've determined
> the device is present and only clear it if the device is fully recovered.
> 
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>

Looks good, although I think it would be clearer if you could separate
checking the slot from raising the alert.

Reviewed-by: Sam Bobroff <sbobroff@linux.ibm.com>

> ---
>  arch/powerpc/kernel/eeh_driver.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
> index 0d34cc12c529..80bd157fcb45 100644
> --- a/arch/powerpc/kernel/eeh_driver.c
> +++ b/arch/powerpc/kernel/eeh_driver.c
> @@ -803,6 +803,10 @@ static bool eeh_slot_presence_check(struct pci_dev *pdev)
>  	if (!ops || !ops->get_adapter_status)
>  		return true;
>  
> +	/* set the attention indicator while we've got the slot ops */
> +	if (ops->set_attention_status)
> +		ops->set_attention_status(slot->hotplug, 1);
> +
>  	rc = ops->get_adapter_status(slot->hotplug, &state);
>  	if (rc)
>  		return true;
> @@ -810,6 +814,28 @@ static bool eeh_slot_presence_check(struct pci_dev *pdev)
>  	return !!state;
>  }
>  
> +static void eeh_clear_slot_attention(struct pci_dev *pdev)
> +{
> +	const struct hotplug_slot_ops *ops;
> +	struct pci_slot *slot;
> +
> +	if (!pdev)
> +		return;
> +
> +	if (pdev->error_state == pci_channel_io_perm_failure)
> +		return;
> +
> +	slot = pdev->slot;
> +	if (!slot || !slot->hotplug)
> +		return;
> +
> +	ops = slot->hotplug->ops;
> +	if (!ops || !ops->set_attention_status)
> +		return;
> +
> +	ops->set_attention_status(slot->hotplug, 0);
> +}
> +
>  /**
>   * eeh_handle_normal_event - Handle EEH events on a specific PE
>   * @pe: EEH PE - which should not be used after we return, as it may
> @@ -1098,6 +1124,12 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
>  	 * we don't want to modify the PE tree structure so we do it here.
>  	 */
>  	eeh_pe_cleanup(pe);
> +
> +	/* clear the slot attention LED for all recovered devices */
> +	eeh_for_each_pe(pe, tmp_pe)
> +		eeh_pe_for_each_dev(tmp_pe, edev, tmp)
> +			eeh_clear_slot_attention(edev->pdev);
> +
>  	eeh_pe_state_clear(pe, EEH_PE_RECOVERING, true);
>  }
>  
> -- 
> 2.21.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]