[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