Messages in this thread | | | Date | Sun, 20 May 2012 00:33:32 +0530 | From | Laxman Dewangan <> | Subject | Re: [PATCH] regulator: core: use correct device for device supply lookup |
| |
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.
| |