On Wed, 2013-12-25 at 16:58 +0800, Gavin Shan wrote:
> After reset on the specific PE or PHB, we never configure AER
> correctly on PowerNV platform. We needn't care it on pSeries
> platform. The patch introduces additional EEH operation eeh_ops::
> restore_bars() so that we have chance to configure AER correctly
> for PowerNV platform.

Why call it "restore_bars" if it restores something else (in this case
AER) ?

I would call it "restore_config" instead... Also rather than adding
the knowledge of what AER bit works or not for the device, would it
make sense to instead have a FW call to re-init the device ?

Otherwise, you introduce duplication between Linux and Firmware with
the risk of getting out of sync...

Cheers,
Ben.

> Signed-off-by: Gavin Shan <sha...@linux.vnet.ibm.com>
> ---
>  arch/powerpc/include/asm/eeh.h               |    1 +
>  arch/powerpc/kernel/eeh_pe.c                 |    3 +++
>  arch/powerpc/platforms/pseries/eeh_pseries.c |    4 +++-
>  3 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
> index d3e5e9b..4b709bf 100644
> --- a/arch/powerpc/include/asm/eeh.h
> +++ b/arch/powerpc/include/asm/eeh.h
> @@ -157,6 +157,7 @@ struct eeh_ops {
>       int (*read_config)(struct device_node *dn, int where, int size, u32 
> *val);
>       int (*write_config)(struct device_node *dn, int where, int size, u32 
> val);
>       int (*next_error)(struct eeh_pe **pe);
> +     void (*restore_bars)(struct device_node *dn);
>  };
>  
>  extern struct eeh_ops *eeh_ops;
> diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
> index f945053..19eb95a 100644
> --- a/arch/powerpc/kernel/eeh_pe.c
> +++ b/arch/powerpc/kernel/eeh_pe.c
> @@ -737,6 +737,9 @@ static void *eeh_restore_one_device_bars(void *data, void 
> *flag)
>       else
>               eeh_restore_device_bars(edev, dn);
>  
> +     if (eeh_ops->restore_bars)
> +             eeh_ops->restore_bars(dn);
> +
>       return NULL;
>  }
>  
> diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c 
> b/arch/powerpc/platforms/pseries/eeh_pseries.c
> index ccb633e..623adaf 100644
> --- a/arch/powerpc/platforms/pseries/eeh_pseries.c
> +++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
> @@ -689,7 +689,9 @@ static struct eeh_ops pseries_eeh_ops = {
>       .get_log                = pseries_eeh_get_log,
>       .configure_bridge       = pseries_eeh_configure_bridge,
>       .read_config            = pseries_eeh_read_config,
> -     .write_config           = pseries_eeh_write_config
> +     .write_config           = pseries_eeh_write_config,
> +     .next_error             = NULL,
> +     .restore_bars           = NULL
>  };
>  
>  /**


_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Reply via email to