Re: [RFC PATCH 10/11] regulator: Implement consumer regulator mapping from device tree

Mark Brown
2011-09-15T13:59:41Z

On Thu, Sep 15, 2011 at 04:52:06PM +0530, Rajendra Nayak wrote:

> +#ifdef CONFIG_OF
> + struct device_node *node;
> + node = of_get_regulator(dev, id);
> + if (!node)
> + goto out;
> + list_for_each_entry(rdev, &regulator_list, list)
> + if (node == rdev->node)
> + goto found;
> +#else
> list_for_each_entry(map, &regulator_map_list, list) {
> /* If the mapping has a device set up it must match */
> if (map->dev_name &&
> @@ -1178,6 +1189,7 @@ static struct regulator *_regulator_get(struct device *dev, const char *id,
> goto found;
> }
> }
> +#endif

This forces all machines to use device tree when CONFIG_OF is enabled.
I'd expect to see both mechanisms supported simultaneously, for example
by falling back to the current code if the device tree lookup fails.

> @@ -1216,6 +1228,15 @@ found:
> if (!try_module_get(rdev->owner))
> goto out;
>
> +#ifdef CONFIG_OF
> + ret = set_consumer_device_supply(rdev, dev, devname, id);
> + if (ret < 0) {
> + dev_err(dev, "Failed to set supply %d\n", ret);
> + unset_regulator_supplies(rdev);
> + goto out;
> + }
> +#endif
> +

This seems wrong, why are we adding things to the regulator_map which is
really only there for lookups when we already did the lookup using the
device tree?

> @@ -2619,6 +2640,8 @@ struct regulator_dev *regulator_register(struct regulator_desc *regulator_desc,
> rdev->reg_data = driver_data;
> rdev->owner = regulator_desc->owner;
> rdev->desc = regulator_desc;
> + if (dev && dev->of_node)
> + rdev->node = dev->of_node;

dev is mandatory.