> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Thursday, July 26, 2012 1:26 AM
> To: Jia Hongtao-B38951
> Cc: Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org;
> ga...@kernel.crashing.org; Li Yang-R58472
> Subject: Re: [PATCH 3/6] powerpc/fsl-pci: Determine primary bus by looking
> for ISA node
>
> >>> +/*
> >>> + * Recursively scan all the children nodes of parent and find out if
> >> there
> >>> + * is "isa" node. Return 1 if parent has isa node otherwise return 0.
> >>> + */
> >>> +int has_isa_node(struct device_node *parent)
> >>> +{
> >>> + static int result;
> >>> + struct device_node *cur_child;
> >>> +
> >>> + cur_child = NULL;
> >>> + result = 0;
> >>> + while (!result && (cur_child = of_get_next_child(parent,
> >> cur_child))) {
> >>> + /* Get "isa" node and return 1 */
> >>> + if (of_node_cmp(cur_child->type, "isa") == 0)
> >>> + return result = 1;
> >>> + has_isa_node(cur_child);
> >>> + }
> >>> +
> >>> + return result;
> >>> +}
> >>
> >> Why are you reimplementing this? It's already in Linus's tree. See
> >> fsl_pci_init().
> >>
> >> Plus, your version is recursive which is unacceptable in kernel code
> >> with a small stack (outside of a few rare examples where the depth has
> a
> >> small fixed upper bound), and once it finds an ISA node, it returns 1
> >> forever, regardless of what node you pass in in the future.
> >>
> >> -Scott
> >
> > About recursion I will do some more investigation.
> > If it finds ISA it returns 1. But for next PCI node it will return 0 if
> > no ISA is found (note that I set result to 0 at the beginning).
>
> Sorry, I misread that as an initializer. Still, it's awkward (why not
> just use a return value?) and non-thread-safe (may not matter here, but
> it's a bad habit), and it's still an unacceptable use of recursion, and
> still a reimplementation of functionality that already exists. :-)
>
> -Scott
All this recursion thing I will try another way.
But this is not the same as you did. If we use platform driver probe function
will be called more than once. Your function is to find ISA node and check if
the parent equal to this PCI controller. My function is to search ISA under
each PCI controller. If the platform driver mechanism is used I think this
function is necessary and reasonable.
-Hongtao.
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
