Re: [PATCH] powerpc: Add of_register_i2c_devices()

[Segher Boessenkool <segher@kernel.crashing.org>]

> +	while ((node = of_get_next_child(adap_node, node))) {

of_for_each_child_node() or whatever it is called?

> +		addr = of_get_property(node, "reg", &len);
> +		if (!addr || len < sizeof(int) || *addr > 0xffff)
> +			continue;

Addresses aren't 16 bit AFAIK?

> +		name = of_get_property(node, "compatible", NULL);
> +		if (!name)
> +			name = node->name;
> +		if (!name)
> +			continue;

Look at "name" first, then look at "compatible", not
the other way around.

> +		model = of_get_property(node, "model", NULL);
> +		if (!model)
> +			model = name;

No way.  "model" and "name" have nothing in common
semantically.

> +		/* FIXME: the i2c code should allow drivers to specify
> +		 * multiple match names; board code shouldn't need to
> +		 * know what driver will handle a given type.
> +		 */

This should be handled by the OF matching stuff, there
is no direct connection between device tree naming and
Linux driver naming.  I don't want to see this patch
going in without this problem being fixed first.


Segher