> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Monday, August 06, 2012 11:10 PM
> To: Jia Hongtao-B38951
> Cc: Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org;
> ga...@kernel.crashing.org; Li Yang-R58472
> Subject: Re: [PATCH V5 3/3] powerpc/fsl-pci: Unify pci/pcie
> initialization code
>
> On 08/05/2012 10:07 PM, Jia Hongtao-B38951 wrote:
> >
> >
> >> -----Original Message-----
> >> From: Wood Scott-B07421
> >> Sent: Saturday, August 04, 2012 12:28 AM
> >> To: Jia Hongtao-B38951
> >> Cc: linuxppc-dev@lists.ozlabs.org; ga...@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.
>
> Too bad. You're breaking the case where there's no ISA node.
>
> > Also this way is more efficient due
> > to we just search the children of PCI.
>
> It is not more efficient, because you're doing the search for every PCIe
> bus rather than once. Not that it matters in this context.
>
> >>> +
> >>> +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.
>
> That was meant for when the board file had an alternate way of searching
> for the primary bus (e.g. look for i8259), not as a replacement for the
> mechanism that guarantees there's a primary bus.
>
> You are causing a regression in the qemu_e500.c platform.
>
> > 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.
>
> Does that board have ISA on it, that isn't described by the device tree?
> If so, before converting to the new init mechanism, the board code will
> need to set fsl_pci_primary based on its own knowledge of where that ISA
> is. If it doesn't have ISA, it doesn't matter which one we designate as
> primary.
>
> > I doubt that there are bugs if no primary assigned.
>
> Yeah, I just implemented the fallback for fun. Come on.
>
> It was recently discussed on this list. PCI under QEMU did not work
> without it.
>
> > 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.
>
> Those boards are not being correctly set up. On real hardware things
> work by chance, but not under QEMU.
>
> -Scott
I am really not sure that all boards need primary bus. Could you give me
the link of discussion about primary that you mentioned?
-Hongtao.
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
