Re: [PATCH 2/2] regulator: da9052: add device tree support

[Mark Brown <broonie@opensource.wolfsonmicro.com>]

[-- Attachment #1: Type: text/plain, Size: 961 bytes --]

On Thu, Apr 12, 2012 at 11:39:42PM +0800, Ying-Chun Liu (PaulLiu) wrote:

> +#ifdef CONFIG_OF
> +		struct device_node *nproot = da9052->dev->of_node;
> +		struct device_node *np;
> +		int c;
> +
> +		if (!nproot) {
> +			ret = -ENODEV;
> +			goto err;
> +		}
> +
> +		nproot = of_find_node_by_name(nproot, "regulators");
> +		if (!nproot) {
> +			ret = -ENODEV;
> +			goto err;
> +		}
> +
> +		c = 0;
> +		for (np = of_get_next_child(nproot, NULL);
> +		     np != NULL;
> +		     np = of_get_next_child(nproot, np)) {
> +			if (c == pdev->id) {
> +				initdata = of_get_regulator_init_data(
> +					&pdev->dev, np);
> +				break;
> +			}
> +			c++;
> +		}

This is really quite unclear but it looks like this is relying on the
order of regulators in the OF table to match things.  As I said in my
reply to the first patch this is really poor for usability and it's
also making the code here more obscure - we should be looking for the
regulator nodes by name.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]