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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, May 20, 2012 at 01:04:05PM +0530, Laxman Dewangan wrote:

> By executing the following code in tps65910-regulator.c, ptobe(),
>                config.of_node =
> of_find_node_by_name(tps65910->dev->of_node,
>                                                         info->name);
> is always returning NULL.

> This is because the info->name which are "ldo1" or "ldo2" are looked
> on the parent node i.e. pdev->dev.parent->of_node, not inside child
> node "regulator" of pdev->dev.parent->of_node.  The function

No.  This is happening because the device tree doesn't have any supplies
mapped for the regulators.  This is nothing at all to do with where the
code looks for the supplies, no matter where it looks there's nothing to
find.

> of_find_node_by_name() only looked for props on that node
> ("tps65911"), does not search from child node "regulator".

This is exactly what I'd expect it to do.  Why would it do anything
different?  Why does it make sense to change the code rather than map
the supplies where the code is currently looking for them?

> So for fixing this,
> The search for info->name should start from the child node
> "regulator" of the "tps65911" to get proper regulator of_node for
> for regulator being register.

You keep saying this but you're still not giving any motivation at all
for making this change.  It's *vital* that you explain why you want to
make this change.  Simply saying that this is the "proper node" over and
over again doesn't do that.

>         ldo2_reg: ldo2 {
>             ::::::::
>             /** regulatr entry */
>             ::::::::::::
>             ldo2-supply = <&ldo1_reg>; /* So ldo1 supply the ldo2. */

This mapping should be moved up to the chip top level; this is just like
any other supply for the chip.

> ldo1 registration went fine.
> During ldo2 registration, I passed the regulator_desc->supply_name as ldo2.

I'd be somewhat surprised if this is what the pin is actually called,
idiomatically the supply name should be whatever the pin is named on the
chip.

> Here we are passing the config.dev = pdev->dev.parent.
> And hence the config.dev.of_node is containing the node of "tps6511".

Of course, what's the problem here?

> Does following change in core.c make sense to handle the case where
> config->of_node and dev->of_node is not same? Here we still use the
> dev which is passed by config->dev and make use of config->of_node.

But *why* do we want to use config->of_node?  This is the bit that's
missing, you keep on repeating that this is the "proper node" over and
over again but you've not been explaining the reasoning behind this.

Supplies for regulators should be no different to supplies for anything
else in the system but you're trying to add a special case for them.
This isn't obviously sensible.  What makes them special?

Attachment: signature.asc
Description: Digital signature


[Index of Archives]

  Powered by Linux

[Older Kernel Discussion]     [Yosemite National Park Forum]     [Large Format Photos]     [Gimp]     [Yosemite Photos]     [Stuff]     [Index of Other Archives]