[PATCH V8] powerpc/fsl-pci: Unify pci/pcie initialization code

[Jia Hongtao-B38951]

> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Tuesday, August 21, 2012 6:04 AM
> To: Jia Hongtao-B38951
> Cc: linuxppc-dev at lists.ozlabs.org; galak at kernel.crashing.org; Li Yang-
> R58472; Bradley Hughes
> Subject: Re: [PATCH V8] powerpc/fsl-pci: Unify pci/pcie initialization
> code
> 
> On 08/20/2012 05:06 AM, Jia Hongtao wrote:
> > We unified the Freescale pci/pcie initialization by changing the
> > fsl_pci to a platform driver. In previous PCI code architecture the
> > initialization routine is called at board_setup_arch stage. Now the
> > initialization is done in probe function which is architectural
> > better. Also It's convenient for adding PM support for PCI controller
> in later patch.
> >
> > Now we registered pci controllers as platform devices. So we combine
> > two initialization code as one platform driver.
> >
> > Signed-off-by: Jia Hongtao <B38951 at freescale.com>
> > Signed-off-by: Li Yang <leoli at freescale.com>
> > Signed-off-by: Chunhe Lan <Chunhe.Lan at freescale.com>
> > ---
> > Changes for V8:
> > * Use previous primary determination. Based on the point that there are
> >   bugs on primary-less system.
> > * Add exceptional support on ge_imp3a in which the primary bus is not
> the
> >   first pci bus detected.
> 
> The exceptional thing about ge_imp3a is that it has no isa node, but
> we're not sure if it actually has isa or not.  We should not be relying
> on probe order in any case.  Device tree nodes are not ordered.
> 
> Another interesting case is stxssa8555.dts, which has an i8259 node but
> no ISA node (are there any other instances of this?).  However, I can't
> tell if stx_gp3.c is the platform file that goes with this device tree,
> or if the platform code for stxssa8555 is out-of-tree (or some other file
> that I'm not seeing).
> 

Hi Scott and Kumar:

Obviously stx_gp3_8560.dts without isa or i8259 node goes with the file
stx_gp3.c. But I'm not sure the board has ISA bridge or not.


> > -void __devinit fsl_pci_init(void)
> > +void fsl_pci_assign_primary(void)
> >  {
> > -	int ret;
> >  	struct device_node *node;
> > -	struct pci_controller *hose;
> > -	dma_addr_t max = 0xffffffff;
> >
> >  	/* Callers can specify the primary bus using other means. */
> >  	if (!fsl_pci_primary) {
> 
> Since the whole point of this function is now to find the primary, just
> return if it's already set, instead of indenting the rest of the function.
> 
> > @@ -842,38 +839,60 @@ void __devinit fsl_pci_init(void)
> >  			node = fsl_pci_primary;
> >
> >  			if (of_match_node(pci_ids, node))
> > -				break;
> > +				return;
> >  		}
> > -	}
> >
> > -	node = NULL;
> > -	for_each_node_by_type(node, "pci") {
> > -		if (of_match_node(pci_ids, node)) {
> > +		node = of_find_node_by_type(NULL, "pci");
> > +		if (of_match_node(pci_ids, node))
> >
> 
> What if the node returned doesn't match?  If you're checking for this,
> handle the else-case (even if just with an error message).
> 
> -Scott