| Message ID | 17f0dc9c30a139f19dceefd09689d34c3ad01a17.1553050609.git.sbobroff@linux.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch next (de3c83c2fd2b87cf68214eda76dfa66989d78cb6) |
| snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 99 lines checked |
On 20/03/2019 13:58, Sam Bobroff wrote: > The PHB flag, PNV_PHB_FLAG_EEH, is set (on PowerNV) individually on > each PHB once the EEH subsystem is ready. It is the only use of the > flags member of the phb struct. Then why to keep pnv_phb::flags? > However there is no need to store this separately on each PHB, so > convert it to a global flag. For symmetry, the flag is now also set > for pSeries; although it is currently unused it may be useful in the > future. Just using eeh_enabled() instead of (phb->flags & PNV_PHB_FLAG_EEH) seems easier and cleaner; also pseries does not use it so there is no point defining it there either. > > Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com> > --- > arch/powerpc/include/asm/eeh.h | 11 +++++++++++ > arch/powerpc/platforms/powernv/eeh-powernv.c | 14 +++----------- > arch/powerpc/platforms/powernv/pci.c | 7 +++---- > arch/powerpc/platforms/powernv/pci.h | 2 -- > arch/powerpc/platforms/pseries/pci.c | 4 ++++ > 5 files changed, 21 insertions(+), 17 deletions(-) > > diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h > index 3613a56281f2..fe4cf7208890 100644 > --- a/arch/powerpc/include/asm/eeh.h > +++ b/arch/powerpc/include/asm/eeh.h > @@ -43,6 +43,7 @@ struct pci_dn; > #define EEH_VALID_PE_ZERO 0x10 /* PE#0 is valid */ > #define EEH_ENABLE_IO_FOR_LOG 0x20 /* Enable IO for log */ > #define EEH_EARLY_DUMP_LOG 0x40 /* Dump log immediately */ > +#define EEH_PHB_ENABLED 0x80 /* PHB recovery uses EEH */ > > /* > * Delay for PE reset, all in ms > @@ -245,6 +246,11 @@ static inline bool eeh_enabled(void) > return eeh_has_flag(EEH_ENABLED) && !eeh_has_flag(EEH_FORCE_DISABLED); > } > > +static inline bool eeh_phb_enabled(void) > +{ > + return eeh_has_flag(EEH_PHB_ENABLED); > +} > + > static inline void eeh_serialize_lock(unsigned long *flags) > { > raw_spin_lock_irqsave(&confirm_error_lock, *flags); > @@ -332,6 +338,11 @@ static inline bool eeh_enabled(void) > return false; > } > > +static inline bool eeh_phb_enabled(void) > +{ > + return false; > +} > + > static inline void eeh_probe_devices(void) { } > > static inline void *eeh_dev_init(struct pci_dn *pdn, void *data) > diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c > index 6fc1a463b796..f0a95f663810 100644 > --- a/arch/powerpc/platforms/powernv/eeh-powernv.c > +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c > @@ -264,22 +264,14 @@ int pnv_eeh_post_init(void) > return ret; > } > > - if (!eeh_enabled()) > + if (eeh_enabled()) > + eeh_add_flag(EEH_PHB_ENABLED); > + else > disable_irq(eeh_event_irq); > > list_for_each_entry(hose, &hose_list, list_node) { > phb = hose->private_data; > > - /* > - * If EEH is enabled, we're going to rely on that. > - * Otherwise, we restore to conventional mechanism > - * to clear frozen PE during PCI config access. > - */ > - if (eeh_enabled()) > - phb->flags |= PNV_PHB_FLAG_EEH; > - else > - phb->flags &= ~PNV_PHB_FLAG_EEH; > - > /* Create debugfs entries */ > #ifdef CONFIG_DEBUG_FS > if (phb->has_dbgfs || !phb->dbgfs) > diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c > index 307181fd8a17..d2b50f3bf6b1 100644 > --- a/arch/powerpc/platforms/powernv/pci.c > +++ b/arch/powerpc/platforms/powernv/pci.c > @@ -717,10 +717,9 @@ int pnv_pci_cfg_write(struct pci_dn *pdn, > static bool pnv_pci_cfg_check(struct pci_dn *pdn) > { > struct eeh_dev *edev = NULL; > - struct pnv_phb *phb = pdn->phb->private_data; > > /* EEH not enabled ? */ > - if (!(phb->flags & PNV_PHB_FLAG_EEH)) > + if (!eeh_phb_enabled()) > return true; > > /* PE reset or device removed ? */ > @@ -761,7 +760,7 @@ static int pnv_pci_read_config(struct pci_bus *bus, > > ret = pnv_pci_cfg_read(pdn, where, size, val); > phb = pdn->phb->private_data; > - if (phb->flags & PNV_PHB_FLAG_EEH && pdn->edev) { > + if (eeh_phb_enabled() && pdn->edev) { > if (*val == EEH_IO_ERROR_VALUE(size) && > eeh_dev_check_failure(pdn->edev)) > return PCIBIOS_DEVICE_NOT_FOUND; > @@ -789,7 +788,7 @@ static int pnv_pci_write_config(struct pci_bus *bus, > > ret = pnv_pci_cfg_write(pdn, where, size, val); > phb = pdn->phb->private_data; > - if (!(phb->flags & PNV_PHB_FLAG_EEH)) > + if (!eeh_phb_enabled()) > pnv_pci_config_check_eeh(pdn); > > return ret; > diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h > index 8e36da379252..eb0add61397b 100644 > --- a/arch/powerpc/platforms/powernv/pci.h > +++ b/arch/powerpc/platforms/powernv/pci.h > @@ -85,8 +85,6 @@ struct pnv_ioda_pe { > struct list_head list; > }; > > -#define PNV_PHB_FLAG_EEH (1 << 0) > - > struct pnv_phb { > struct pci_controller *hose; > enum pnv_phb_type type; > diff --git a/arch/powerpc/platforms/pseries/pci.c b/arch/powerpc/platforms/pseries/pci.c > index 37a77e57893e..7be80882c08d 100644 > --- a/arch/powerpc/platforms/pseries/pci.c > +++ b/arch/powerpc/platforms/pseries/pci.c > @@ -244,6 +244,10 @@ void __init pSeries_final_fixup(void) > > eeh_probe_devices(); > eeh_addr_cache_build(); > +#ifdef CONFIG_EEH > + if (eeh_enabled()) > + eeh_add_flag(EEH_PHB_ENABLED); > +#endif > > #ifdef CONFIG_PCI_IOV > ppc_md.pcibios_sriov_enable = pseries_pcibios_sriov_enable; >
On Wed, Mar 20, 2019 at 05:02:44PM +1100, Alexey Kardashevskiy wrote: > > > On 20/03/2019 13:58, Sam Bobroff wrote: > > The PHB flag, PNV_PHB_FLAG_EEH, is set (on PowerNV) individually on > > each PHB once the EEH subsystem is ready. It is the only use of the > > flags member of the phb struct. > > > Then why to keep pnv_phb::flags? No reason. I'll remove it in the next version. > > However there is no need to store this separately on each PHB, so > > convert it to a global flag. For symmetry, the flag is now also set > > for pSeries; although it is currently unused it may be useful in the > > future. > > Just using eeh_enabled() instead of (phb->flags & PNV_PHB_FLAG_EEH) > seems easier and cleaner; also pseries does not use it so there is no > point defining it there either. I do want to do that. However, eeh_enabled() seems to be slightly different from PNV_PHB_FLAG_EEH: - eeh_enabled() is true as soon as the first device with EEH support is detected. - eeh_phb_enabled() is true after EEH support has been enabled on every device that supports it. So I was concerned that using eeh_enabled() would cause problems in pnv_pci_config_check_eeh() if EEH was detected *during* the initial PCI scanning phase when eeh_enabled() was true but EEH had not yet been set up on the device or PHB where it was detected. Does that make sense? Would it be reasonable to keep this patch as it is for now and investigate cleaning it up in a future patch? > > > > Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com> > > --- > > arch/powerpc/include/asm/eeh.h | 11 +++++++++++ > > arch/powerpc/platforms/powernv/eeh-powernv.c | 14 +++----------- > > arch/powerpc/platforms/powernv/pci.c | 7 +++---- > > arch/powerpc/platforms/powernv/pci.h | 2 -- > > arch/powerpc/platforms/pseries/pci.c | 4 ++++ > > 5 files changed, 21 insertions(+), 17 deletions(-) > > > > diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h > > index 3613a56281f2..fe4cf7208890 100644 > > --- a/arch/powerpc/include/asm/eeh.h > > +++ b/arch/powerpc/include/asm/eeh.h > > @@ -43,6 +43,7 @@ struct pci_dn; > > #define EEH_VALID_PE_ZERO 0x10 /* PE#0 is valid */ > > #define EEH_ENABLE_IO_FOR_LOG 0x20 /* Enable IO for log */ > > #define EEH_EARLY_DUMP_LOG 0x40 /* Dump log immediately */ > > +#define EEH_PHB_ENABLED 0x80 /* PHB recovery uses EEH */ > > > > /* > > * Delay for PE reset, all in ms > > @@ -245,6 +246,11 @@ static inline bool eeh_enabled(void) > > return eeh_has_flag(EEH_ENABLED) && !eeh_has_flag(EEH_FORCE_DISABLED); > > } > > > > +static inline bool eeh_phb_enabled(void) > > +{ > > + return eeh_has_flag(EEH_PHB_ENABLED); > > +} > > + > > static inline void eeh_serialize_lock(unsigned long *flags) > > { > > raw_spin_lock_irqsave(&confirm_error_lock, *flags); > > @@ -332,6 +338,11 @@ static inline bool eeh_enabled(void) > > return false; > > } > > > > +static inline bool eeh_phb_enabled(void) > > +{ > > + return false; > > +} > > + > > static inline void eeh_probe_devices(void) { } > > > > static inline void *eeh_dev_init(struct pci_dn *pdn, void *data) > > diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c > > index 6fc1a463b796..f0a95f663810 100644 > > --- a/arch/powerpc/platforms/powernv/eeh-powernv.c > > +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c > > @@ -264,22 +264,14 @@ int pnv_eeh_post_init(void) > > return ret; > > } > > > > - if (!eeh_enabled()) > > + if (eeh_enabled()) > > + eeh_add_flag(EEH_PHB_ENABLED); > > + else > > disable_irq(eeh_event_irq); > > > > list_for_each_entry(hose, &hose_list, list_node) { > > phb = hose->private_data; > > > > - /* > > - * If EEH is enabled, we're going to rely on that. > > - * Otherwise, we restore to conventional mechanism > > - * to clear frozen PE during PCI config access. > > - */ > > - if (eeh_enabled()) > > - phb->flags |= PNV_PHB_FLAG_EEH; > > - else > > - phb->flags &= ~PNV_PHB_FLAG_EEH; > > - > > /* Create debugfs entries */ > > #ifdef CONFIG_DEBUG_FS > > if (phb->has_dbgfs || !phb->dbgfs) > > diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c > > index 307181fd8a17..d2b50f3bf6b1 100644 > > --- a/arch/powerpc/platforms/powernv/pci.c > > +++ b/arch/powerpc/platforms/powernv/pci.c > > @@ -717,10 +717,9 @@ int pnv_pci_cfg_write(struct pci_dn *pdn, > > static bool pnv_pci_cfg_check(struct pci_dn *pdn) > > { > > struct eeh_dev *edev = NULL; > > - struct pnv_phb *phb = pdn->phb->private_data; > > > > /* EEH not enabled ? */ > > - if (!(phb->flags & PNV_PHB_FLAG_EEH)) > > + if (!eeh_phb_enabled()) > > return true; > > > > /* PE reset or device removed ? */ > > @@ -761,7 +760,7 @@ static int pnv_pci_read_config(struct pci_bus *bus, > > > > ret = pnv_pci_cfg_read(pdn, where, size, val); > > phb = pdn->phb->private_data; > > - if (phb->flags & PNV_PHB_FLAG_EEH && pdn->edev) { > > + if (eeh_phb_enabled() && pdn->edev) { > > if (*val == EEH_IO_ERROR_VALUE(size) && > > eeh_dev_check_failure(pdn->edev)) > > return PCIBIOS_DEVICE_NOT_FOUND; > > @@ -789,7 +788,7 @@ static int pnv_pci_write_config(struct pci_bus *bus, > > > > ret = pnv_pci_cfg_write(pdn, where, size, val); > > phb = pdn->phb->private_data; > > - if (!(phb->flags & PNV_PHB_FLAG_EEH)) > > + if (!eeh_phb_enabled()) > > pnv_pci_config_check_eeh(pdn); > > > > return ret; > > diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h > > index 8e36da379252..eb0add61397b 100644 > > --- a/arch/powerpc/platforms/powernv/pci.h > > +++ b/arch/powerpc/platforms/powernv/pci.h > > @@ -85,8 +85,6 @@ struct pnv_ioda_pe { > > struct list_head list; > > }; > > > > -#define PNV_PHB_FLAG_EEH (1 << 0) > > - > > struct pnv_phb { > > struct pci_controller *hose; > > enum pnv_phb_type type; > > diff --git a/arch/powerpc/platforms/pseries/pci.c b/arch/powerpc/platforms/pseries/pci.c > > index 37a77e57893e..7be80882c08d 100644 > > --- a/arch/powerpc/platforms/pseries/pci.c > > +++ b/arch/powerpc/platforms/pseries/pci.c > > @@ -244,6 +244,10 @@ void __init pSeries_final_fixup(void) > > > > eeh_probe_devices(); > > eeh_addr_cache_build(); > > +#ifdef CONFIG_EEH > > + if (eeh_enabled()) > > + eeh_add_flag(EEH_PHB_ENABLED); > > +#endif > > > > #ifdef CONFIG_PCI_IOV > > ppc_md.pcibios_sriov_enable = pseries_pcibios_sriov_enable; > > > > -- > Alexey >
On Wed, 2019-03-20 at 13:58 +1100, Sam Bobroff wrote: > The PHB flag, PNV_PHB_FLAG_EEH, is set (on PowerNV) individually on > each PHB once the EEH subsystem is ready. It is the only use of the > flags member of the phb struct. > > However there is no need to store this separately on each PHB, so > convert it to a global flag. For symmetry, the flag is now also set > for pSeries; although it is currently unused it may be useful in the > future. I'd drop this patch and keep it as a seperate flag. Making this a global flag doesn't really buy us anything either. It's also worth remembering that we do have virtual PHBs, like the NVLink ones, that don't have real EEH support and we should be doing something more intelligent about that. If you are going to remove the PNV_PHB flag then i would look at moving that flag into the generic pci_controller structure that we use for tracking PHBs since that would give us a way to toggle EEH support on a per PHB basis on pseries and powernv. > Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com> > --- > arch/powerpc/include/asm/eeh.h | 11 +++++++++++ > arch/powerpc/platforms/powernv/eeh-powernv.c | 14 +++----------- > arch/powerpc/platforms/powernv/pci.c | 7 +++---- > arch/powerpc/platforms/powernv/pci.h | 2 -- > arch/powerpc/platforms/pseries/pci.c | 4 ++++ > 5 files changed, 21 insertions(+), 17 deletions(-) > > diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h > index 3613a56281f2..fe4cf7208890 100644 > --- a/arch/powerpc/include/asm/eeh.h > +++ b/arch/powerpc/include/asm/eeh.h > @@ -43,6 +43,7 @@ struct pci_dn; > #define EEH_VALID_PE_ZERO 0x10 /* PE#0 is valid */ > #define EEH_ENABLE_IO_FOR_LOG 0x20 /* Enable IO for log */ > #define EEH_EARLY_DUMP_LOG 0x40 /* Dump log immediately */ > +#define EEH_PHB_ENABLED 0x80 /* PHB recovery uses EEH */ > > /* > * Delay for PE reset, all in ms > @@ -245,6 +246,11 @@ static inline bool eeh_enabled(void) > return eeh_has_flag(EEH_ENABLED) && !eeh_has_flag(EEH_FORCE_DISABLED); > } > > +static inline bool eeh_phb_enabled(void) > +{ > + return eeh_has_flag(EEH_PHB_ENABLED); > +} > + > static inline void eeh_serialize_lock(unsigned long *flags) > { > raw_spin_lock_irqsave(&confirm_error_lock, *flags); > @@ -332,6 +338,11 @@ static inline bool eeh_enabled(void) > return false; > } > > +static inline bool eeh_phb_enabled(void) > +{ > + return false; > +} > + > static inline void eeh_probe_devices(void) { } > > static inline void *eeh_dev_init(struct pci_dn *pdn, void *data) > diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c > index 6fc1a463b796..f0a95f663810 100644 > --- a/arch/powerpc/platforms/powernv/eeh-powernv.c > +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c > @@ -264,22 +264,14 @@ int pnv_eeh_post_init(void) > return ret; > } > > - if (!eeh_enabled()) > + if (eeh_enabled()) > + eeh_add_flag(EEH_PHB_ENABLED); > + else > disable_irq(eeh_event_irq); > > list_for_each_entry(hose, &hose_list, list_node) { > phb = hose->private_data; > > - /* > - * If EEH is enabled, we're going to rely on that. > - * Otherwise, we restore to conventional mechanism > - * to clear frozen PE during PCI config access. > - */ > - if (eeh_enabled()) > - phb->flags |= PNV_PHB_FLAG_EEH; > - else > - phb->flags &= ~PNV_PHB_FLAG_EEH; > - > /* Create debugfs entries */ > #ifdef CONFIG_DEBUG_FS > if (phb->has_dbgfs || !phb->dbgfs) > diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c > index 307181fd8a17..d2b50f3bf6b1 100644 > --- a/arch/powerpc/platforms/powernv/pci.c > +++ b/arch/powerpc/platforms/powernv/pci.c > @@ -717,10 +717,9 @@ int pnv_pci_cfg_write(struct pci_dn *pdn, > static bool pnv_pci_cfg_check(struct pci_dn *pdn) > { > struct eeh_dev *edev = NULL; > - struct pnv_phb *phb = pdn->phb->private_data; > > /* EEH not enabled ? */ > - if (!(phb->flags & PNV_PHB_FLAG_EEH)) > + if (!eeh_phb_enabled()) > return true; > > /* PE reset or device removed ? */ > @@ -761,7 +760,7 @@ static int pnv_pci_read_config(struct pci_bus *bus, > > ret = pnv_pci_cfg_read(pdn, where, size, val); > phb = pdn->phb->private_data; > - if (phb->flags & PNV_PHB_FLAG_EEH && pdn->edev) { > + if (eeh_phb_enabled() && pdn->edev) { > if (*val == EEH_IO_ERROR_VALUE(size) && > eeh_dev_check_failure(pdn->edev)) > return PCIBIOS_DEVICE_NOT_FOUND; > @@ -789,7 +788,7 @@ static int pnv_pci_write_config(struct pci_bus *bus, > > ret = pnv_pci_cfg_write(pdn, where, size, val); > phb = pdn->phb->private_data; > - if (!(phb->flags & PNV_PHB_FLAG_EEH)) > + if (!eeh_phb_enabled()) > pnv_pci_config_check_eeh(pdn); > > return ret; > diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h > index 8e36da379252..eb0add61397b 100644 > --- a/arch/powerpc/platforms/powernv/pci.h > +++ b/arch/powerpc/platforms/powernv/pci.h > @@ -85,8 +85,6 @@ struct pnv_ioda_pe { > struct list_head list; > }; > > -#define PNV_PHB_FLAG_EEH (1 << 0) > - > struct pnv_phb { > struct pci_controller *hose; > enum pnv_phb_type type; > diff --git a/arch/powerpc/platforms/pseries/pci.c b/arch/powerpc/platforms/pseries/pci.c > index 37a77e57893e..7be80882c08d 100644 > --- a/arch/powerpc/platforms/pseries/pci.c > +++ b/arch/powerpc/platforms/pseries/pci.c > @@ -244,6 +244,10 @@ void __init pSeries_final_fixup(void) > > eeh_probe_devices(); > eeh_addr_cache_build(); > +#ifdef CONFIG_EEH > + if (eeh_enabled()) > + eeh_add_flag(EEH_PHB_ENABLED); > +#endif > > #ifdef CONFIG_PCI_IOV > ppc_md.pcibios_sriov_enable = pseries_pcibios_sriov_enable;
On Thu, Apr 18, 2019 at 07:51:40PM +1000, Oliver O'Halloran wrote: > On Wed, 2019-03-20 at 13:58 +1100, Sam Bobroff wrote: > > The PHB flag, PNV_PHB_FLAG_EEH, is set (on PowerNV) individually on > > each PHB once the EEH subsystem is ready. It is the only use of the > > flags member of the phb struct. > > > > However there is no need to store this separately on each PHB, so > > convert it to a global flag. For symmetry, the flag is now also set > > for pSeries; although it is currently unused it may be useful in the > > future. > > I'd drop this patch and keep it as a seperate flag. Making this a > global flag doesn't really buy us anything either. It's also worth > remembering that we do have virtual PHBs, like the NVLink ones, that > don't have real EEH support and we should be doing something more > intelligent about that. > > If you are going to remove the PNV_PHB flag then i would look at moving > that flag into the generic pci_controller structure that we use for > tracking PHBs since that would give us a way to toggle EEH support on a > per PHB basis on pseries and powernv. OK. I want to keep these changes as small as possible, so I'll try a simpler approach and use the exsiting flag. (I only touched this area because it's now necessary to set the EEH_ENABLED flag both at boot time and after it.) > > Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com> > > --- > > arch/powerpc/include/asm/eeh.h | 11 +++++++++++ > > arch/powerpc/platforms/powernv/eeh-powernv.c | 14 +++----------- > > arch/powerpc/platforms/powernv/pci.c | 7 +++---- > > arch/powerpc/platforms/powernv/pci.h | 2 -- > > arch/powerpc/platforms/pseries/pci.c | 4 ++++ > > 5 files changed, 21 insertions(+), 17 deletions(-) > > > > diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h > > index 3613a56281f2..fe4cf7208890 100644 > > --- a/arch/powerpc/include/asm/eeh.h > > +++ b/arch/powerpc/include/asm/eeh.h > > @@ -43,6 +43,7 @@ struct pci_dn; > > #define EEH_VALID_PE_ZERO 0x10 /* PE#0 is valid */ > > #define EEH_ENABLE_IO_FOR_LOG 0x20 /* Enable IO for log */ > > #define EEH_EARLY_DUMP_LOG 0x40 /* Dump log immediately */ > > +#define EEH_PHB_ENABLED 0x80 /* PHB recovery uses EEH */ > > > > /* > > * Delay for PE reset, all in ms > > @@ -245,6 +246,11 @@ static inline bool eeh_enabled(void) > > return eeh_has_flag(EEH_ENABLED) && !eeh_has_flag(EEH_FORCE_DISABLED); > > } > > > > +static inline bool eeh_phb_enabled(void) > > +{ > > + return eeh_has_flag(EEH_PHB_ENABLED); > > +} > > + > > static inline void eeh_serialize_lock(unsigned long *flags) > > { > > raw_spin_lock_irqsave(&confirm_error_lock, *flags); > > @@ -332,6 +338,11 @@ static inline bool eeh_enabled(void) > > return false; > > } > > > > +static inline bool eeh_phb_enabled(void) > > +{ > > + return false; > > +} > > + > > static inline void eeh_probe_devices(void) { } > > > > static inline void *eeh_dev_init(struct pci_dn *pdn, void *data) > > diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c > > index 6fc1a463b796..f0a95f663810 100644 > > --- a/arch/powerpc/platforms/powernv/eeh-powernv.c > > +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c > > @@ -264,22 +264,14 @@ int pnv_eeh_post_init(void) > > return ret; > > } > > > > - if (!eeh_enabled()) > > + if (eeh_enabled()) > > + eeh_add_flag(EEH_PHB_ENABLED); > > + else > > disable_irq(eeh_event_irq); > > > > list_for_each_entry(hose, &hose_list, list_node) { > > phb = hose->private_data; > > > > - /* > > - * If EEH is enabled, we're going to rely on that. > > - * Otherwise, we restore to conventional mechanism > > - * to clear frozen PE during PCI config access. > > - */ > > - if (eeh_enabled()) > > - phb->flags |= PNV_PHB_FLAG_EEH; > > - else > > - phb->flags &= ~PNV_PHB_FLAG_EEH; > > - > > /* Create debugfs entries */ > > #ifdef CONFIG_DEBUG_FS > > if (phb->has_dbgfs || !phb->dbgfs) > > diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c > > index 307181fd8a17..d2b50f3bf6b1 100644 > > --- a/arch/powerpc/platforms/powernv/pci.c > > +++ b/arch/powerpc/platforms/powernv/pci.c > > @@ -717,10 +717,9 @@ int pnv_pci_cfg_write(struct pci_dn *pdn, > > static bool pnv_pci_cfg_check(struct pci_dn *pdn) > > { > > struct eeh_dev *edev = NULL; > > - struct pnv_phb *phb = pdn->phb->private_data; > > > > /* EEH not enabled ? */ > > - if (!(phb->flags & PNV_PHB_FLAG_EEH)) > > + if (!eeh_phb_enabled()) > > return true; > > > > /* PE reset or device removed ? */ > > @@ -761,7 +760,7 @@ static int pnv_pci_read_config(struct pci_bus *bus, > > > > ret = pnv_pci_cfg_read(pdn, where, size, val); > > phb = pdn->phb->private_data; > > - if (phb->flags & PNV_PHB_FLAG_EEH && pdn->edev) { > > + if (eeh_phb_enabled() && pdn->edev) { > > if (*val == EEH_IO_ERROR_VALUE(size) && > > eeh_dev_check_failure(pdn->edev)) > > return PCIBIOS_DEVICE_NOT_FOUND; > > @@ -789,7 +788,7 @@ static int pnv_pci_write_config(struct pci_bus *bus, > > > > ret = pnv_pci_cfg_write(pdn, where, size, val); > > phb = pdn->phb->private_data; > > - if (!(phb->flags & PNV_PHB_FLAG_EEH)) > > + if (!eeh_phb_enabled()) > > pnv_pci_config_check_eeh(pdn); > > > > return ret; > > diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h > > index 8e36da379252..eb0add61397b 100644 > > --- a/arch/powerpc/platforms/powernv/pci.h > > +++ b/arch/powerpc/platforms/powernv/pci.h > > @@ -85,8 +85,6 @@ struct pnv_ioda_pe { > > struct list_head list; > > }; > > > > -#define PNV_PHB_FLAG_EEH (1 << 0) > > - > > struct pnv_phb { > > struct pci_controller *hose; > > enum pnv_phb_type type; > > diff --git a/arch/powerpc/platforms/pseries/pci.c b/arch/powerpc/platforms/pseries/pci.c > > index 37a77e57893e..7be80882c08d 100644 > > --- a/arch/powerpc/platforms/pseries/pci.c > > +++ b/arch/powerpc/platforms/pseries/pci.c > > @@ -244,6 +244,10 @@ void __init pSeries_final_fixup(void) > > > > eeh_probe_devices(); > > eeh_addr_cache_build(); > > +#ifdef CONFIG_EEH > > + if (eeh_enabled()) > > + eeh_add_flag(EEH_PHB_ENABLED); > > +#endif > > > > #ifdef CONFIG_PCI_IOV > > ppc_md.pcibios_sriov_enable = pseries_pcibios_sriov_enable; >
diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h index 3613a56281f2..fe4cf7208890 100644 --- a/arch/powerpc/include/asm/eeh.h +++ b/arch/powerpc/include/asm/eeh.h @@ -43,6 +43,7 @@ struct pci_dn; #define EEH_VALID_PE_ZERO 0x10 /* PE#0 is valid */ #define EEH_ENABLE_IO_FOR_LOG 0x20 /* Enable IO for log */ #define EEH_EARLY_DUMP_LOG 0x40 /* Dump log immediately */ +#define EEH_PHB_ENABLED 0x80 /* PHB recovery uses EEH */ /* * Delay for PE reset, all in ms @@ -245,6 +246,11 @@ static inline bool eeh_enabled(void) return eeh_has_flag(EEH_ENABLED) && !eeh_has_flag(EEH_FORCE_DISABLED); } +static inline bool eeh_phb_enabled(void) +{ + return eeh_has_flag(EEH_PHB_ENABLED); +} + static inline void eeh_serialize_lock(unsigned long *flags) { raw_spin_lock_irqsave(&confirm_error_lock, *flags); @@ -332,6 +338,11 @@ static inline bool eeh_enabled(void) return false; } +static inline bool eeh_phb_enabled(void) +{ + return false; +} + static inline void eeh_probe_devices(void) { } static inline void *eeh_dev_init(struct pci_dn *pdn, void *data) diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c index 6fc1a463b796..f0a95f663810 100644 --- a/arch/powerpc/platforms/powernv/eeh-powernv.c +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c @@ -264,22 +264,14 @@ int pnv_eeh_post_init(void) return ret; } - if (!eeh_enabled()) + if (eeh_enabled()) + eeh_add_flag(EEH_PHB_ENABLED); + else disable_irq(eeh_event_irq); list_for_each_entry(hose, &hose_list, list_node) { phb = hose->private_data; - /* - * If EEH is enabled, we're going to rely on that. - * Otherwise, we restore to conventional mechanism - * to clear frozen PE during PCI config access. - */ - if (eeh_enabled()) - phb->flags |= PNV_PHB_FLAG_EEH; - else - phb->flags &= ~PNV_PHB_FLAG_EEH; - /* Create debugfs entries */ #ifdef CONFIG_DEBUG_FS if (phb->has_dbgfs || !phb->dbgfs) diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c index 307181fd8a17..d2b50f3bf6b1 100644 --- a/arch/powerpc/platforms/powernv/pci.c +++ b/arch/powerpc/platforms/powernv/pci.c @@ -717,10 +717,9 @@ int pnv_pci_cfg_write(struct pci_dn *pdn, static bool pnv_pci_cfg_check(struct pci_dn *pdn) { struct eeh_dev *edev = NULL; - struct pnv_phb *phb = pdn->phb->private_data; /* EEH not enabled ? */ - if (!(phb->flags & PNV_PHB_FLAG_EEH)) + if (!eeh_phb_enabled()) return true; /* PE reset or device removed ? */ @@ -761,7 +760,7 @@ static int pnv_pci_read_config(struct pci_bus *bus, ret = pnv_pci_cfg_read(pdn, where, size, val); phb = pdn->phb->private_data; - if (phb->flags & PNV_PHB_FLAG_EEH && pdn->edev) { + if (eeh_phb_enabled() && pdn->edev) { if (*val == EEH_IO_ERROR_VALUE(size) && eeh_dev_check_failure(pdn->edev)) return PCIBIOS_DEVICE_NOT_FOUND; @@ -789,7 +788,7 @@ static int pnv_pci_write_config(struct pci_bus *bus, ret = pnv_pci_cfg_write(pdn, where, size, val); phb = pdn->phb->private_data; - if (!(phb->flags & PNV_PHB_FLAG_EEH)) + if (!eeh_phb_enabled()) pnv_pci_config_check_eeh(pdn); return ret; diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h index 8e36da379252..eb0add61397b 100644 --- a/arch/powerpc/platforms/powernv/pci.h +++ b/arch/powerpc/platforms/powernv/pci.h @@ -85,8 +85,6 @@ struct pnv_ioda_pe { struct list_head list; }; -#define PNV_PHB_FLAG_EEH (1 << 0) - struct pnv_phb { struct pci_controller *hose; enum pnv_phb_type type; diff --git a/arch/powerpc/platforms/pseries/pci.c b/arch/powerpc/platforms/pseries/pci.c index 37a77e57893e..7be80882c08d 100644 --- a/arch/powerpc/platforms/pseries/pci.c +++ b/arch/powerpc/platforms/pseries/pci.c @@ -244,6 +244,10 @@ void __init pSeries_final_fixup(void) eeh_probe_devices(); eeh_addr_cache_build(); +#ifdef CONFIG_EEH + if (eeh_enabled()) + eeh_add_flag(EEH_PHB_ENABLED); +#endif #ifdef CONFIG_PCI_IOV ppc_md.pcibios_sriov_enable = pseries_pcibios_sriov_enable;
The PHB flag, PNV_PHB_FLAG_EEH, is set (on PowerNV) individually on each PHB once the EEH subsystem is ready. It is the only use of the flags member of the phb struct. However there is no need to store this separately on each PHB, so convert it to a global flag. For symmetry, the flag is now also set for pSeries; although it is currently unused it may be useful in the future. Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com> --- arch/powerpc/include/asm/eeh.h | 11 +++++++++++ arch/powerpc/platforms/powernv/eeh-powernv.c | 14 +++----------- arch/powerpc/platforms/powernv/pci.c | 7 +++---- arch/powerpc/platforms/powernv/pci.h | 2 -- arch/powerpc/platforms/pseries/pci.c | 4 ++++ 5 files changed, 21 insertions(+), 17 deletions(-)