[PATCH 3/6] powerpc/fsl-pci: Determine primary bus by looking for ISA node

Jia Hongtao-B38951 B38951 at freescale.com
Wed Jul 25 19:01:22 EST 2012


> > +/*
> > + * 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

Yes, recursive function is not recommended for kernel but maybe it's not unacceptable.
This function is not so deep stacked and simple. In my opinion this is acceptable.

Thanks.
-Hongtao.



More information about the Linuxppc-dev mailing list