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

Jia Hongtao-B38951 B38951 at freescale.com
Mon Aug 6 13:07:34 EST 2012



> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Saturday, August 04, 2012 12:28 AM
> To: Jia Hongtao-B38951
> Cc: linuxppc-dev at lists.ozlabs.org; galak at kernel.crashing.org; Li Yang-
> R58472; Wood Scott-B07421
> Subject: Re: [PATCH V5 3/3] powerpc/fsl-pci: Unify pci/pcie
> initialization code
> 
> On 08/03/2012 05:14 AM, Jia Hongtao wrote:
> > -void __devinit fsl_pci_init(void)
> > +/* Checkout if PCI contains ISA node */
> > +static int of_pci_has_isa(struct device_node *pci_node)
> > +{
> > +	struct device_node *np;
> > +	int ret = 0;
> > +
> > +	if (!pci_node)
> > +		return 0;
> > +
> > +	read_lock(&devtree_lock);
> > +	np = pci_node->allnext;
> > +
> > +	/* Only scan the children of PCI node */
> > +	for (; np != pci_node->sibling; np = np->allnext) {
> > +		if (np->type && (of_node_cmp(np->type, "isa") == 0)
> > +		    && of_node_get(np)) {
> > +			ret = 1;
> > +			break;
> > +		}
> > +	}
> > +
> > +	of_node_put(pci_node);
> > +	read_unlock(&devtree_lock);
> > +
> > +	return ret;
> > +}
> 
> Why do you keep insisting on substituting your ISA search code here?
> What advantages does it have over the code that is already there?  It
> unnecessarily digs into the internals of the tree representation.
> 

I want ISA search is done from probe. Also this way is more efficient due
to we just search the children of PCI.

> > +
> > +static int __devinit fsl_pci_probe(struct platform_device *pdev)
> >  {
> >  	int ret;
> > -	struct device_node *node;
> >  	struct pci_controller *hose;
> > -	dma_addr_t max = 0xffffffff;
> > +	int is_primary = 0;
> >
> > -	/* Callers can specify the primary bus using other means. */
> >  	if (!fsl_pci_primary) {
> > -		/* If a PCI host bridge contains an ISA node, it's primary.
> */
> > -		node = of_find_node_by_type(NULL, "isa");
> > -		while ((fsl_pci_primary = of_get_parent(node))) {
> > -			of_node_put(node);
> > -			node = fsl_pci_primary;
> > -
> > -			if (of_match_node(pci_ids, node))
> > -				break;
> > -		}
> > +		is_primary = of_pci_has_isa(pdev->dev.of_node);
> > +		if (is_primary)
> > +			fsl_pci_primary = pdev->dev.of_node;
> >  	}
> 
> As I explained before, this has to be done globally, not from the probe
> function, so we can assign a default primary bus if there isn't any ISA.
>  There are bugs in the Linux PPC PCI code relating to not having any
> primary bus.
> 
> -Scott

In my way of searching ISA you can also assign a default primary bus in board
specific files. 

I read your code and found that if there is no ISA node you will assign the
first PCI bus scanned as primary. It's not all right. Take ge_imp3a as an
example: The second PCI bus (9000) is primary not the first one.

I doubt that there are bugs if no primary assigned. Like mpc85xx_rdb assigned
no primary at all. Some other boards has no primary ether like p1022ds, p1021mds,
p1010rdb, p1023rds, all corenet boards (p2041_rdb, p3041_ds, p4080_ds, p5020_ds,
p5040_ds). If no primary is a bug then all these boards above are not correctly
setting up.

-Hongtao. 
 



More information about the Linuxppc-dev mailing list