Re: [PATCH] regulator: core: use correct device for device supply lookup

[Laxman Dewangan <ldewangan@nvidia.com>]

On Saturday 19 May 2012 11:56 PM, Mark Brown wrote:
> * PGP Signed by an unknown key
>
> On Sat, May 19, 2012 at 11:26:00PM +0530, Laxman Dewangan wrote:
>
>> At the time of registration,  as becasue there is valid
>> reg_desc->supply_name and hence it tries to lookup the entry for
>> <name>-supply i.e. v2-supply in this case for getting regulator_dev.
>> regulator_register() {
>> static struct regulator_dev *regulator_dev_lookup(struct device *dev,
>>                                                    const char *supply,
>>                                                    int *ret)
>> {
>>          /* first do a dt based lookup */
>> ---->  Checked here, dev is not null but dev->of_node is null.
>>          if (dev&&  dev->of_node) {
>>
>> ------------>The issue is that I am not getting here as dev->node is
>> null here.
> But how is this related your patch?  What your patch does is change
> things so that instead of trying to look up the supply in the context of
> whatever device was passed in by the driver we try to look it up in the
> context of the class device we create.  I can't think of any situation
> where I'd expect that to make matters any better - the class device
> should certainly never appear in the device tree and isn't going to have
> a stable name for non-DT systems either.
>
> I'm just not seeing any problem in the core here.  It sounds to me like
> the problem might be either with the regulator driver doing something
> odd with the struct device it specifies when registering the regulator
> (though I'm guessing that it's the tps65910 which looks to be doing
> something sensible currently) or the device tree for the board being
> odd.  Looking at the changes you posted to tps65910 I suspect the issue
> is that you've changed the driver to pass in the platform device for the
> regulators as their device rather than the I2C device but it's the I2C
> device which appears in the device tree bindings.
>
My board dts file is
       pmu: tps65910@d2 {
                 compatible = "ti,tps65910";
                 reg = <0xd2>;
                 interrupt-parent = <&intc>;
                 interrupts = < 0 118 0x04 >;

                 #gpio-cells = <2>;
                 gpio-controller;

                 #interrupt-cells = <2>;
                 interrupt-controller;

                 regulators {
                         vdd1_reg: vdd1 {
                                 regulator-min-microvolt = < 600000>;
                                 regulator-max-microvolt = <1500000>;
                                 regulator-always-on;
                                 regulator-boot-on;
                                 ti,regulator-ext-sleep-control = <0>;
                         };
                         vdd2_reg: vdd2 {
                                 regulator-min-microvolt = < 600000>;
                                 regulator-max-microvolt = <1500000>;
                                 regulator-always-on;
                                 regulator-boot-on;
                                 ti,regulator-ext-sleep-control = <4>;
                         };
             };
     };

So currently, when regulator_register gets called in 
tps65910-regulator.c, it sets the
config.dev = tps65910->dev;
config.of_node as
                 config.of_node = 
of_find_node_by_name(tps65910->dev->of_node,
                                                         info->name);
So here config.of_node always shows NULL as the in 
tps65910->dev->of_node does not have name same as info->name ie. 
regulator name like vdd1, vdd2.

So I changed it to pass the node containing"regulators" for 
of_find_node_by_name() and then it got proper of_node like vdd1_reg or 
vdd2_reg as per info_name.

Now this node for vdd1_reg, vdd2_reg, is being passed to 
regulator_register. And for lookup of node, we should use the 
config.of_node which is set.

If we still want to use the parent device for lookup then other way to 
use the config.of_node is to take the parameter of device node in this api.

For this it is require to change the apis as

static struct regulator_dev *regulator_dev_lookup(struct device *dev,
                                                   const char *supply,
                                                   int *ret)

to
static struct regulator_dev *regulator_dev_lookup(struct device *dev, 
struct device_node *dev_node,
                                                   const char *supply,
                                                   int *ret)


and at the time of regulator registration, we should pass config->of_node.
In this way we can still pass parent device and of_node which is passed 
by  regulator driver.
         if (supply) {
                 struct regulator_dev *r;

                 r = regulator_dev_lookup(dev, config->of_node, supply, 
&ret);


> If there is a change needed in the core you need to explain what you
> believe that change will do.
>
I though this is straight but seems it is becoming more complex now.
I will describe all this details if we agree to change require.