Re: [PATCH] regulator: core: use correct device for device supplylookup

[Laxman Dewangan]

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.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/