| Message ID | 20181016023409.42238-1-aik@ozlabs.ru (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | a25de7af340fcd19a59978ded2ff04ed329bca06 |
| Headers | show |
| Series | [kernel] powerpc/powernv/ioda: Reduce a number of hooks in pnv_phb | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | next/apply_patch Successfully applied |
| snowpatch_ozlabs/checkpatch | success | Test checkpatch on branch next |
| snowpatch_ozlabs/build-ppc64le | success | Test build-ppc64le on branch next |
| snowpatch_ozlabs/build-ppc64be | success | Test build-ppc64be on branch next |
| snowpatch_ozlabs/build-ppc64e | success | Test build-ppc64e on branch next |
| snowpatch_ozlabs/build-ppc32 | success | Test build-ppc32 on branch next |
On Tue, Oct 16, 2018 at 01:34:09PM +1100, Alexey Kardashevskiy wrote: > fixup_phb() is never used, this removes it. > > pick_m64_pe() and reserve_m64_pe() are always defined for all powernv > PHBs: they are initialized by pnv_ioda_parse_m64_window() which is > called unconditionally from pnv_pci_init_ioda_phb() which initializes > all known PHB types on powernv so we can open code them. > > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > --- > arch/powerpc/platforms/powernv/pci.h | 4 ---- > arch/powerpc/platforms/powernv/pci-ioda.c | 9 +++------ > 2 files changed, 3 insertions(+), 10 deletions(-) > > diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h > index 8b37b28..2131373 100644 > --- a/arch/powerpc/platforms/powernv/pci.h > +++ b/arch/powerpc/platforms/powernv/pci.h > @@ -115,11 +115,7 @@ struct pnv_phb { > unsigned int hwirq, unsigned int virq, > unsigned int is_64, struct msi_msg *msg); > void (*dma_dev_setup)(struct pnv_phb *phb, struct pci_dev *pdev); > - void (*fixup_phb)(struct pci_controller *hose); > int (*init_m64)(struct pnv_phb *phb); > - void (*reserve_m64_pe)(struct pci_bus *bus, > - unsigned long *pe_bitmap, bool all); > - struct pnv_ioda_pe *(*pick_m64_pe)(struct pci_bus *bus, bool all); > int (*get_pe_state)(struct pnv_phb *phb, int pe_no); > void (*freeze_pe)(struct pnv_phb *phb, int pe_no); > int (*unfreeze_pe)(struct pnv_phb *phb, int pe_no, int opt); > diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c > index 78b61f0..15a4556 100644 > --- a/arch/powerpc/platforms/powernv/pci-ioda.c > +++ b/arch/powerpc/platforms/powernv/pci-ioda.c > @@ -518,8 +518,6 @@ static void __init pnv_ioda_parse_m64_window(struct pnv_phb *phb) > phb->init_m64 = pnv_ioda1_init_m64; > else > phb->init_m64 = pnv_ioda2_init_m64; > - phb->reserve_m64_pe = pnv_ioda_reserve_m64_pe; > - phb->pick_m64_pe = pnv_ioda_pick_m64_pe; > } > > static void pnv_ioda_freeze_pe(struct pnv_phb *phb, int pe_no) > @@ -1161,8 +1159,8 @@ static struct pnv_ioda_pe *pnv_ioda_setup_bus_PE(struct pci_bus *bus, bool all) > pe = &phb->ioda.pe_array[phb->ioda.root_pe_idx]; > > /* Check if PE is determined by M64 */ > - if (!pe && phb->pick_m64_pe) > - pe = phb->pick_m64_pe(bus, all); > + if (!pe) > + pe = pnv_ioda_pick_m64_pe(bus, all); What about the cases where pnv_ioda_parse_m64_window() returns before setting pick_m64_pe or reserve_m64_pe? For example, if !firmware_has_feature(FW_FEATURE_OPAL). In that case, it looks like this change would cause pnv_ioda_pick_m64_pe() to be called, and it would try to perform an OPAL call without OPAL support. (Is it even possible to have powernv without OPAL?) > > /* The PE number isn't pinned by M64 */ > if (!pe) > @@ -3395,8 +3393,7 @@ static void pnv_pci_setup_bridge(struct pci_bus *bus, unsigned long type) > return; > > /* Reserve PEs according to used M64 resources */ > - if (phb->reserve_m64_pe) > - phb->reserve_m64_pe(bus, NULL, all); > + pnv_ioda_reserve_m64_pe(bus, NULL, all); > > /* > * Assign PE. We might run here because of partial hotplug. > -- > 2.11.0 >
Sam Bobroff <sbobroff@linux.ibm.com> writes: > On Tue, Oct 16, 2018 at 01:34:09PM +1100, Alexey Kardashevskiy wrote: >> fixup_phb() is never used, this removes it. >> >> pick_m64_pe() and reserve_m64_pe() are always defined for all powernv >> PHBs: they are initialized by pnv_ioda_parse_m64_window() which is >> called unconditionally from pnv_pci_init_ioda_phb() which initializes >> all known PHB types on powernv so we can open code them. >> >> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> >> --- >> arch/powerpc/platforms/powernv/pci.h | 4 ---- >> arch/powerpc/platforms/powernv/pci-ioda.c | 9 +++------ >> 2 files changed, 3 insertions(+), 10 deletions(-) >> >> diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h >> index 8b37b28..2131373 100644 >> --- a/arch/powerpc/platforms/powernv/pci.h >> +++ b/arch/powerpc/platforms/powernv/pci.h >> @@ -115,11 +115,7 @@ struct pnv_phb { >> unsigned int hwirq, unsigned int virq, >> unsigned int is_64, struct msi_msg *msg); >> void (*dma_dev_setup)(struct pnv_phb *phb, struct pci_dev *pdev); >> - void (*fixup_phb)(struct pci_controller *hose); >> int (*init_m64)(struct pnv_phb *phb); >> - void (*reserve_m64_pe)(struct pci_bus *bus, >> - unsigned long *pe_bitmap, bool all); >> - struct pnv_ioda_pe *(*pick_m64_pe)(struct pci_bus *bus, bool all); >> int (*get_pe_state)(struct pnv_phb *phb, int pe_no); >> void (*freeze_pe)(struct pnv_phb *phb, int pe_no); >> int (*unfreeze_pe)(struct pnv_phb *phb, int pe_no, int opt); >> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c >> index 78b61f0..15a4556 100644 >> --- a/arch/powerpc/platforms/powernv/pci-ioda.c >> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c >> @@ -518,8 +518,6 @@ static void __init pnv_ioda_parse_m64_window(struct pnv_phb *phb) >> phb->init_m64 = pnv_ioda1_init_m64; >> else >> phb->init_m64 = pnv_ioda2_init_m64; >> - phb->reserve_m64_pe = pnv_ioda_reserve_m64_pe; >> - phb->pick_m64_pe = pnv_ioda_pick_m64_pe; >> } >> >> static void pnv_ioda_freeze_pe(struct pnv_phb *phb, int pe_no) >> @@ -1161,8 +1159,8 @@ static struct pnv_ioda_pe *pnv_ioda_setup_bus_PE(struct pci_bus *bus, bool all) >> pe = &phb->ioda.pe_array[phb->ioda.root_pe_idx]; >> >> /* Check if PE is determined by M64 */ >> - if (!pe && phb->pick_m64_pe) >> - pe = phb->pick_m64_pe(bus, all); >> + if (!pe) >> + pe = pnv_ioda_pick_m64_pe(bus, all); > > What about the cases where pnv_ioda_parse_m64_window() returns before > setting pick_m64_pe or reserve_m64_pe? > > For example, if !firmware_has_feature(FW_FEATURE_OPAL). In that case, > it looks like this change would cause pnv_ioda_pick_m64_pe() to be > called, and it would try to perform an OPAL call without OPAL support. > > (Is it even possible to have powernv without OPAL?) It is, though I'm not sure how well tested it is these days. We should probably either start testing it regularly, or drop it entirely. But if we are running without OPAL then we don't probe PCI at all, see pnv_pci_init(). So the check in pnv_ioda_parse_m64_window() can be removed AFAICS. cheers
diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h index 8b37b28..2131373 100644 --- a/arch/powerpc/platforms/powernv/pci.h +++ b/arch/powerpc/platforms/powernv/pci.h @@ -115,11 +115,7 @@ struct pnv_phb { unsigned int hwirq, unsigned int virq, unsigned int is_64, struct msi_msg *msg); void (*dma_dev_setup)(struct pnv_phb *phb, struct pci_dev *pdev); - void (*fixup_phb)(struct pci_controller *hose); int (*init_m64)(struct pnv_phb *phb); - void (*reserve_m64_pe)(struct pci_bus *bus, - unsigned long *pe_bitmap, bool all); - struct pnv_ioda_pe *(*pick_m64_pe)(struct pci_bus *bus, bool all); int (*get_pe_state)(struct pnv_phb *phb, int pe_no); void (*freeze_pe)(struct pnv_phb *phb, int pe_no); int (*unfreeze_pe)(struct pnv_phb *phb, int pe_no, int opt); diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c index 78b61f0..15a4556 100644 --- a/arch/powerpc/platforms/powernv/pci-ioda.c +++ b/arch/powerpc/platforms/powernv/pci-ioda.c @@ -518,8 +518,6 @@ static void __init pnv_ioda_parse_m64_window(struct pnv_phb *phb) phb->init_m64 = pnv_ioda1_init_m64; else phb->init_m64 = pnv_ioda2_init_m64; - phb->reserve_m64_pe = pnv_ioda_reserve_m64_pe; - phb->pick_m64_pe = pnv_ioda_pick_m64_pe; } static void pnv_ioda_freeze_pe(struct pnv_phb *phb, int pe_no) @@ -1161,8 +1159,8 @@ static struct pnv_ioda_pe *pnv_ioda_setup_bus_PE(struct pci_bus *bus, bool all) pe = &phb->ioda.pe_array[phb->ioda.root_pe_idx]; /* Check if PE is determined by M64 */ - if (!pe && phb->pick_m64_pe) - pe = phb->pick_m64_pe(bus, all); + if (!pe) + pe = pnv_ioda_pick_m64_pe(bus, all); /* The PE number isn't pinned by M64 */ if (!pe) @@ -3395,8 +3393,7 @@ static void pnv_pci_setup_bridge(struct pci_bus *bus, unsigned long type) return; /* Reserve PEs according to used M64 resources */ - if (phb->reserve_m64_pe) - phb->reserve_m64_pe(bus, NULL, all); + pnv_ioda_reserve_m64_pe(bus, NULL, all); /* * Assign PE. We might run here because of partial hotplug.
fixup_phb() is never used, this removes it. pick_m64_pe() and reserve_m64_pe() are always defined for all powernv PHBs: they are initialized by pnv_ioda_parse_m64_window() which is called unconditionally from pnv_pci_init_ioda_phb() which initializes all known PHB types on powernv so we can open code them. Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> --- arch/powerpc/platforms/powernv/pci.h | 4 ---- arch/powerpc/platforms/powernv/pci-ioda.c | 9 +++------ 2 files changed, 3 insertions(+), 10 deletions(-)