obtaining a regulator from a phandle

obtaining a regulator from a phandle

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1489 bytes --]

Hi,

I'm working on getting DT support for PCIe on Tegra working. The Tegra PCIe
controller needs a specific voltage supplied by a regulator on the board that
I use (Harmony-compatible), so what I did was add DT support for the PMU
(tps6586x) to provide the corresponding regulator:

	pmu: tps6586x@34 {
		[...]
		regulators {
			...
			ldo0_reg: ldo0 {
				regulator-min-microvolt = <3300000>;
				regulator-max-microvolt = <3300000>;
			};
		};
		[...]
	};

Within the device node of the PCIe controller, I reference the ldo0 regulator
like so:

	pcie@80000000 {
		pex-clk-supply = <&ldo0_reg>;
	};

This all works well, except there seems to be no API to obtain a regulator
from the DT phandle. I'm wondering whether this is because nobody has had any
use for it or whether I'm using it wrongly.

I could possibly work around this by giving the ldo0 an explicit name via the
regulator-name property and then look up that property from the phandle and
then obtain a reference to the regulator by calling the normal regulator_get()
and passing the name. That seems wasteful, though, because I already have a
direct link to the regulator (via its struct device_node) and all I really
need is to look up the corresponding struct regulator.

So my question really is if it would be acceptable to add such a helper, or
whether I have completely misunderstood how this works. In the latter case,
could anyone point me in the right direction?

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

RE: obtaining a regulator from a phandle

[Stephen Warren]

Thierry Reding wrote at Monday, March 05, 2012 8:22 AM:
> I'm working on getting DT support for PCIe on Tegra working. The Tegra PCIe
> controller needs a specific voltage supplied by a regulator on the board that
> I use (Harmony-compatible), so what I did was add DT support for the PMU
> (tps6586x) to provide the corresponding regulator:
...
> Within the device node of the PCIe controller, I reference the ldo0 regulator
> like so:
> 
> 	pcie@80000000 {
> 		pex-clk-supply = <&ldo0_reg>;
> 	};
> 
> This all works well, except there seems to be no API to obtain a regulator
> from the DT phandle.

regulator_get(pcie_dev, "pex-clk") should work.

Of course, the PCIe host on Tegra isn't yet a platform device, so would
have to be converted.

-- 
nvpublic

Re: obtaining a regulator from a phandle

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1173 bytes --]

* Stephen Warren wrote:
> Thierry Reding wrote at Monday, March 05, 2012 8:22 AM:
> > I'm working on getting DT support for PCIe on Tegra working. The Tegra PCIe
> > controller needs a specific voltage supplied by a regulator on the board that
> > I use (Harmony-compatible), so what I did was add DT support for the PMU
> > (tps6586x) to provide the corresponding regulator:
> ...
> > Within the device node of the PCIe controller, I reference the ldo0 regulator
> > like so:
> > 
> > 	pcie@80000000 {
> > 		pex-clk-supply = <&ldo0_reg>;
> > 	};
> > 
> > This all works well, except there seems to be no API to obtain a regulator
> > from the DT phandle.
> 
> regulator_get(pcie_dev, "pex-clk") should work.

Ah, I see. That would call regulator_dev_lookup(), which would call
of_get_regulator() which will lookup the pex-clk-supply property. I
will give that a shot.

> Of course, the PCIe host on Tegra isn't yet a platform device, so would
> have to be converted.

I've already converted that. I was going to send the patch when I've tested
that this works in a DT setup as well. So far I have only tested it on non-
DT Harmony.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: obtaining a regulator from a phandle

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2522 bytes --]

* Thierry Reding wrote:
> * Stephen Warren wrote:
> > Thierry Reding wrote at Monday, March 05, 2012 8:22 AM:
> > > This all works well, except there seems to be no API to obtain a regulator
> > > from the DT phandle.
> > 
> > regulator_get(pcie_dev, "pex-clk") should work.
> 
> Ah, I see. That would call regulator_dev_lookup(), which would call
> of_get_regulator() which will lookup the pex-clk-supply property. I
> will give that a shot.

regulator_get(&pdev->dev, "pex-clk") worked as expected.

> > Of course, the PCIe host on Tegra isn't yet a platform device, so would
> > have to be converted.
> 
> I've already converted that. I was going to send the patch when I've tested
> that this works in a DT setup as well. So far I have only tested it on non-
> DT Harmony.

Adding DT support seems to be more complicated than I anticipated. Starting
from the platform driver that works on non-DT Harmony I added DT support to
the tps6586x driver in order to be able to use its regulators from the PCIe
driver (also extended with DT support). That worked great.

However, the PCIe controller needs another (fixed) regulator as VDD supply.
On Harmony (and the hardware that I use) this is GPIO#2 of the PMU (tps6586x)
so I added another regulator to the device tree which uses that GPIO and
passed it to the PCIe device node (vdd-supply property).

And this is precisely where things get ugly. Now there is a dependency
problem between the fixed regulator and the PMU which provides the GPIO. Both
the PMU and the fixed regulator are setup via subsys_initcall(), and the
regulator beats the PMU, resulting in the fixed regulator being unable to get
the GPIO because it hasn't been setup yet. So I remembered Grant's deferred
probing patch from yesterday and thought I'd try it out. This worked great,
and the fixed regulator was properly set up after all other devices had been
probed.

The PCIe driver now also needs deferred probing until the fixed regulator is
present, which also works as expected. But then the problems start. Since now
we're so far into the boot process that all __init{,data} is gone, therefore
pci_common_init() is no longer present, resulting in an ugly crash.

So the only way I see out of this is patch up everything so that PCI
initialization can actually be done after init memory is freed. This would
have the added benefit that the driver could actually be built as a module.

Does anyone see another (easier) way out of this?

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: obtaining a regulator from a phandle

[Stephen Warren]

On 03/06/2012 04:39 AM, Thierry Reding wrote:
...
> And this is precisely where things get ugly. Now there is a dependency
> problem between the fixed regulator and the PMU which provides the GPIO. Both
> the PMU and the fixed regulator are setup via subsys_initcall(), and the
> regulator beats the PMU, resulting in the fixed regulator being unable to get
> the GPIO because it hasn't been setup yet. So I remembered Grant's deferred
> probing patch from yesterday and thought I'd try it out. This worked great,
> and the fixed regulator was properly set up after all other devices had been
> probed.

It's great that worked out.

> The PCIe driver now also needs deferred probing until the fixed regulator is
> present, which also works as expected. But then the problems start. Since now
> we're so far into the boot process that all __init{,data} is gone, therefore
> pci_common_init() is no longer present, resulting in an ugly crash.
> 
> So the only way I see out of this is patch up everything so that PCI
> initialization can actually be done after init memory is freed. This would
> have the added benefit that the driver could actually be built as a module.
> 
> Does anyone see another (easier) way out of this?

To me, that sounds like exactly how to solve it. You can probably mark
the code __devinit rather than just removing all the decorations
completely. You probably want to run this by the core ARM maintainers
though; I CC'd Russell and the ARM mailing list for comment.

obtaining a regulator from a phandle

[Stephen Warren]

On 03/06/2012 04:39 AM, Thierry Reding wrote:
...
> And this is precisely where things get ugly. Now there is a dependency
> problem between the fixed regulator and the PMU which provides the GPIO. Both
> the PMU and the fixed regulator are setup via subsys_initcall(), and the
> regulator beats the PMU, resulting in the fixed regulator being unable to get
> the GPIO because it hasn't been setup yet. So I remembered Grant's deferred
> probing patch from yesterday and thought I'd try it out. This worked great,
> and the fixed regulator was properly set up after all other devices had been
> probed.

It's great that worked out.

> The PCIe driver now also needs deferred probing until the fixed regulator is
> present, which also works as expected. But then the problems start. Since now
> we're so far into the boot process that all __init{,data} is gone, therefore
> pci_common_init() is no longer present, resulting in an ugly crash.
> 
> So the only way I see out of this is patch up everything so that PCI
> initialization can actually be done after init memory is freed. This would
> have the added benefit that the driver could actually be built as a module.
> 
> Does anyone see another (easier) way out of this?

To me, that sounds like exactly how to solve it. You can probably mark
the code __devinit rather than just removing all the decorations
completely. You probably want to run this by the core ARM maintainers
though; I CC'd Russell and the ARM mailing list for comment.