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

[Li Yang-R58472]

> -----Original Message-----
> From: Jia Hongtao-B38951
> Sent: Tuesday, August 21, 2012 11:26 AM
> To: Wood Scott-B07421
> 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
> 
> 
> 
> > -----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.
> 
> Yes... We don't know if ge_imp3a actually has isa and still no answer
> from
> board owner. I just set primary as the board used to. At least we don't
> do
> any harm.
> 
> >
> > 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).
> 
> MPC8541_CDS and MPC8555_CDS also has i8259 but no ISA node. stx_gp3 seems
> go
> with stxssa8555.dts but I'm not sure ether.
> 
> So you mean we have to look for i8259 too for determining primary.
> Take device tree as evidence we can tell that real primary ether has isa
> node
> or i8259 node. And if there is no isa we just arbitrarily designate one.
> 
> If this logic works well then ok.

If there is i8259 node in the device tree, it should be suggesting that there is a PCI to ISA bridge but not explicitly described in the device tree.  Then we need to fix the device tree to add the ISA nodes.

- Leo
> 
> 
> >
> > > -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