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

[Michael Ellerman]

Sam Bobroff <sbobroff at 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 at 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