linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH kernel] powerpc/powernv/ioda: Reduce a number of hooks in pnv_phb
@ 2018-10-16  2:34 Alexey Kardashevskiy
  2018-11-15  3:24 ` Sam Bobroff
  2018-12-23 13:28 ` [kernel] " Michael Ellerman
  0 siblings, 2 replies; 4+ messages in thread
From: Alexey Kardashevskiy @ 2018-10-16  2:34 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Alexey Kardashevskiy

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);
 
 	/* 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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH kernel] powerpc/powernv/ioda: Reduce a number of hooks in pnv_phb
  2018-10-16  2:34 [PATCH kernel] powerpc/powernv/ioda: Reduce a number of hooks in pnv_phb Alexey Kardashevskiy
@ 2018-11-15  3:24 ` Sam Bobroff
  2018-11-15 12:13   ` Michael Ellerman
  2018-12-23 13:28 ` [kernel] " Michael Ellerman
  1 sibling, 1 reply; 4+ messages in thread
From: Sam Bobroff @ 2018-11-15  3:24 UTC (permalink / raw)
  To: Alexey Kardashevskiy; +Cc: linuxppc-dev

[-- 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 --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH kernel] powerpc/powernv/ioda: Reduce a number of hooks in pnv_phb
  2018-11-15  3:24 ` Sam Bobroff
@ 2018-11-15 12:13   ` Michael Ellerman
  0 siblings, 0 replies; 4+ messages in thread
From: Michael Ellerman @ 2018-11-15 12:13 UTC (permalink / raw)
  To: Sam Bobroff, Alexey Kardashevskiy
  Cc: Michael Neuling, linuxppc-dev, Stewart Smith

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [kernel] powerpc/powernv/ioda: Reduce a number of hooks in pnv_phb
  2018-10-16  2:34 [PATCH kernel] powerpc/powernv/ioda: Reduce a number of hooks in pnv_phb Alexey Kardashevskiy
  2018-11-15  3:24 ` Sam Bobroff
@ 2018-12-23 13:28 ` Michael Ellerman
  1 sibling, 0 replies; 4+ messages in thread
From: Michael Ellerman @ 2018-12-23 13:28 UTC (permalink / raw)
  To: Alexey Kardashevskiy, linuxppc-dev; +Cc: Alexey Kardashevskiy

On Tue, 2018-10-16 at 02:34:09 UTC, 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>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/a25de7af340fcd19a59978ded2ff04

cheers

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-12-23 14:25 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-10-16  2:34 [PATCH kernel] powerpc/powernv/ioda: Reduce a number of hooks in pnv_phb Alexey Kardashevskiy
2018-11-15  3:24 ` Sam Bobroff
2018-11-15 12:13   ` Michael Ellerman
2018-12-23 13:28 ` [kernel] " Michael Ellerman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).