Re: [PATCH kernel] powerpc/powernv/ioda: Reduce a number of hooks in pnv_phb

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

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

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
> 

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