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

Rajendra Nayak
2011-09-16T07:21:14Z

On Thursday 15 September 2011 07:29 PM, Mark Brown wrote:
> 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.

agree, will change this to fall back if the dt 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?

I did this only so the mappings show up in sysfs.

>
>> @@ -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.

ok