| Message ID | 1372210688-12214-4-git-send-email-shangw@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
> We have 2 fields in "struct pnv_phb" to trace the states. The patch > replace the fields with one and introduces flags for that. The patch > doesn't impact the logic. What is the benefit of this change? ... > + > +#define PNV_EEH_STATE_ENABLED (1 << 0) /* EEH enabled */ > +#define PNV_EEH_STATE_REMOVED (1 << 1) /* PHB removed */ > + > #endif /* CONFIG_EEH */ > > struct pnv_phb { > @@ -92,8 +96,7 @@ struct pnv_phb { > > #ifdef CONFIG_EEH > struct pnv_eeh_ops *eeh_ops; > - int eeh_enabled; > - int removed; > + int eeh_state; > #endif > > #ifdef CONFIG_DEBUG_FS All I can see is that it possibly reduces a structure by 4 bytes while adding extra code. (On 64 bit systems there might be a 4 byte pad added.) David
On Wed, Jun 26, 2013 at 10:12:16AM +0100, David Laight wrote: >> We have 2 fields in "struct pnv_phb" to trace the states. The patch >> replace the fields with one and introduces flags for that. The patch >> doesn't impact the logic. > >What is the benefit of this change? > There might have more flags coming in. Putting all flags together could be maintained more easily. It doesn't save much memory as you pointed. >> + >> +#define PNV_EEH_STATE_ENABLED (1 << 0) /* EEH enabled */ >> +#define PNV_EEH_STATE_REMOVED (1 << 1) /* PHB removed */ >> + >> #endif /* CONFIG_EEH */ >> >> struct pnv_phb { >> @@ -92,8 +96,7 @@ struct pnv_phb { >> >> #ifdef CONFIG_EEH >> struct pnv_eeh_ops *eeh_ops; >> - int eeh_enabled; >> - int removed; >> + int eeh_state; >> #endif >> >> #ifdef CONFIG_DEBUG_FS > >All I can see is that it possibly reduces a structure by 4 bytes >while adding extra code. >(On 64 bit systems there might be a 4 byte pad added.) > Thanks, Gavin
diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c index 84f3036..85025d7 100644 --- a/arch/powerpc/platforms/powernv/eeh-ioda.c +++ b/arch/powerpc/platforms/powernv/eeh-ioda.c @@ -132,7 +132,7 @@ static int ioda_eeh_post_init(struct pci_controller *hose) &ioda_eeh_dbgfs_ops); #endif - phb->eeh_enabled = 1; + phb->eeh_state |= PNV_EEH_STATE_ENABLED; } return 0; @@ -815,7 +815,7 @@ static int ioda_eeh_next_error(struct eeh_pe **pe) * removed, we needn't take care of it any more. */ phb = hose->private_data; - if (phb->removed) + if (phb->eeh_state & PNV_EEH_STATE_REMOVED) continue; rc = opal_pci_next_error(phb->opal_id, @@ -850,7 +850,7 @@ static int ioda_eeh_next_error(struct eeh_pe **pe) list_for_each_entry_safe(hose, tmp, &hose_list, list_node) { phb = hose->private_data; - phb->removed = 1; + phb->eeh_state |= PNV_EEH_STATE_REMOVED; } WARN(1, "EEH: dead IOC detected\n"); @@ -867,7 +867,7 @@ static int ioda_eeh_next_error(struct eeh_pe **pe) WARN(1, "EEH: dead PHB#%x detected\n", hose->global_number); - phb->removed = 1; + phb->eeh_state |= PNV_EEH_STATE_REMOVED; ret = 3; goto out; } else if (severity == OPAL_EEH_SEV_PHB_FENCED) { diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c index 6d9a506..1f31826 100644 --- a/arch/powerpc/platforms/powernv/pci.c +++ b/arch/powerpc/platforms/powernv/pci.c @@ -308,7 +308,7 @@ static int pnv_pci_read_config(struct pci_bus *bus, if (phb_pe && (phb_pe->state & EEH_PE_ISOLATED)) return PCIBIOS_SUCCESSFUL; - if (phb->eeh_enabled) { + if (phb->eeh_state & PNV_EEH_STATE_ENABLED) { if (*val == EEH_IO_ERROR_VALUE(size)) { busdn = pci_bus_to_OF_node(bus); for (dn = busdn->child; dn; dn = dn->sibling) { @@ -358,7 +358,7 @@ static int pnv_pci_write_config(struct pci_bus *bus, /* Check if the PHB got frozen due to an error (no response) */ #ifdef CONFIG_EEH - if (!phb->eeh_enabled) + if (!(phb->eeh_state & PNV_EEH_STATE_ENABLED)) pnv_pci_config_check_eeh(phb, bus, bdfn); #else pnv_pci_config_check_eeh(phb, bus, bdfn); diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h index 43906e3..40bdf02 100644 --- a/arch/powerpc/platforms/powernv/pci.h +++ b/arch/powerpc/platforms/powernv/pci.h @@ -78,6 +78,10 @@ struct pnv_eeh_ops { int (*configure_bridge)(struct eeh_pe *pe); int (*next_error)(struct eeh_pe **pe); }; + +#define PNV_EEH_STATE_ENABLED (1 << 0) /* EEH enabled */ +#define PNV_EEH_STATE_REMOVED (1 << 1) /* PHB removed */ + #endif /* CONFIG_EEH */ struct pnv_phb { @@ -92,8 +96,7 @@ struct pnv_phb { #ifdef CONFIG_EEH struct pnv_eeh_ops *eeh_ops; - int eeh_enabled; - int removed; + int eeh_state; #endif #ifdef CONFIG_DEBUG_FS
We have 2 fields in "struct pnv_phb" to trace the states. The patch replace the fields with one and introduces flags for that. The patch doesn't impact the logic. Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com> --- arch/powerpc/platforms/powernv/eeh-ioda.c | 8 ++++---- arch/powerpc/platforms/powernv/pci.c | 4 ++-- arch/powerpc/platforms/powernv/pci.h | 7 +++++-- 3 files changed, 11 insertions(+), 8 deletions(-)