[PATCH] sunxi: a10-lime: add regulator nodes

[PATCH] sunxi: a10-lime: add regulator nodes

[Iain Paton]

add pmic regulator definitions matching the manufacturers 3.4.x fex
file.

Signed-off-by: Iain Paton <ipaton0@gmail.com>
---

As this file belongs to Hans and he decided not to use axp209.dtsi in
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/315612.html
then I won't add it here.

 arch/arm/boot/dts/sun4i-a10-olinuxino-lime.dts | 68 ++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/arch/arm/boot/dts/sun4i-a10-olinuxino-lime.dts b/arch/arm/boot/dts/sun4i-a10-olinuxino-lime.dts
index 31dc2f1..f7a8fcc 100644
--- a/arch/arm/boot/dts/sun4i-a10-olinuxino-lime.dts
+++ b/arch/arm/boot/dts/sun4i-a10-olinuxino-lime.dts
@@ -67,6 +67,15 @@
 			default-state = "on";
 		};
 	};
+
+	reg_axp_ipsout: axp_ipsout {
+		compatible = "regulator-fixed";
+		regulator-name = "axp-ipsout";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		regulator-always-on;
+	};
+
 };
 
 &ahci {
@@ -97,10 +106,69 @@
 	axp209: pmic at 34 {
 		compatible = "x-powers,axp209";
 		reg = <0x34>;
+		interrupt-parent = <&intc>;
 		interrupts = <0>;
 
 		interrupt-controller;
 		#interrupt-cells = <1>;
+
+		acin-supply = <&reg_axp_ipsout>;
+		vin2-supply = <&reg_axp_ipsout>;
+		vin3-supply = <&reg_axp_ipsout>;
+		ldo24in-supply = <&reg_axp_ipsout>;
+		ldo3in-supply = <&reg_axp_ipsout>;
+
+		x-powers,dcdc-freq = <1500>;
+
+		regulators {
+			vdd_rtc: ldo1 {
+				regulator-min-microvolt = <1300000>;
+				regulator-max-microvolt = <1300000>;
+				regulator-always-on;
+				regulator-name="vdd_rtc";
+			};
+
+			avcc: ldo2 {
+				regulator-min-microvolt = <3000000>;
+				regulator-max-microvolt = <3000000>;
+				regulator-always-on;
+				regulator-name="avcc";
+			};
+
+			vcc_csi0: ldo3 {
+				regulator-min-microvolt = <2800000>;
+				regulator-max-microvolt = <2800000>;
+				regulator-always-on;
+				regulator-name="vcc_csi0";
+			};
+
+			vcc_csi1: ldo4 {
+				regulator-min-microvolt = <2800000>;
+				regulator-max-microvolt = <2800000>;
+				regulator-always-on;
+				regulator-name="vcc_csi1";
+			};
+
+			ldo5 {
+				regulator-min-microvolt = <2800000>;
+				regulator-max-microvolt = <2800000>;
+				regulator-name="reg_unused";
+			};
+
+			vdd_cpu: dcdc2 {
+				regulator-min-microvolt = <1250000>;
+				regulator-max-microvolt = <1550000>;
+				regulator-always-on;
+				regulator-name="vdd_cpu";
+			};
+
+			vdd_int: dcdc3 {
+				regulator-min-microvolt = <1250000>;
+				regulator-max-microvolt = <1250000>;
+				regulator-always-on;
+				regulator-name="vdd_int";
+			};
+		};
 	};
 };
 
-- 
2.1.3

[PATCH] sunxi: a10-lime: add regulator nodes

[Hans de Goede]

Hi Iain,

On 27-03-15 11:58, Iain Paton wrote:
> add pmic regulator definitions matching the manufacturers 3.4.x fex
> file.
>
> Signed-off-by: Iain Paton <ipaton0@gmail.com>
> ---
>
> As this file belongs to Hans and he decided not to use axp209.dtsi in
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/315612.html
> then I won't add it here.

That was because using it was breaking stuff, but now that we know better
which regulators are used for what, and which one we most add an always-on;
property too, I would greatly prefer you to acually use axp209.dtsi, see
e.g. the sun5i-a13-utoo-p66.dts file where I'm using it.

Can you please respin this patch using axp209.dtsi?

Also please do not add nodes for unused regulators, like the ldo-s for the csi-s.

Thanks & Regards,

Hans


>
>   arch/arm/boot/dts/sun4i-a10-olinuxino-lime.dts | 68 ++++++++++++++++++++++++++
>   1 file changed, 68 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun4i-a10-olinuxino-lime.dts b/arch/arm/boot/dts/sun4i-a10-olinuxino-lime.dts
> index 31dc2f1..f7a8fcc 100644
> --- a/arch/arm/boot/dts/sun4i-a10-olinuxino-lime.dts
> +++ b/arch/arm/boot/dts/sun4i-a10-olinuxino-lime.dts
> @@ -67,6 +67,15 @@
>   			default-state = "on";
>   		};
>   	};
> +
> +	reg_axp_ipsout: axp_ipsout {
> +		compatible = "regulator-fixed";
> +		regulator-name = "axp-ipsout";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		regulator-always-on;
> +	};
> +
>   };
>
>   &ahci {
> @@ -97,10 +106,69 @@
>   	axp209: pmic at 34 {
>   		compatible = "x-powers,axp209";
>   		reg = <0x34>;
> +		interrupt-parent = <&intc>;
>   		interrupts = <0>;
>
>   		interrupt-controller;
>   		#interrupt-cells = <1>;
> +
> +		acin-supply = <&reg_axp_ipsout>;
> +		vin2-supply = <&reg_axp_ipsout>;
> +		vin3-supply = <&reg_axp_ipsout>;
> +		ldo24in-supply = <&reg_axp_ipsout>;
> +		ldo3in-supply = <&reg_axp_ipsout>;
> +
> +		x-powers,dcdc-freq = <1500>;
> +
> +		regulators {
> +			vdd_rtc: ldo1 {
> +				regulator-min-microvolt = <1300000>;
> +				regulator-max-microvolt = <1300000>;
> +				regulator-always-on;
> +				regulator-name="vdd_rtc";
> +			};
> +
> +			avcc: ldo2 {
> +				regulator-min-microvolt = <3000000>;
> +				regulator-max-microvolt = <3000000>;
> +				regulator-always-on;
> +				regulator-name="avcc";
> +			};
> +
> +			vcc_csi0: ldo3 {
> +				regulator-min-microvolt = <2800000>;
> +				regulator-max-microvolt = <2800000>;
> +				regulator-always-on;
> +				regulator-name="vcc_csi0";
> +			};
> +
> +			vcc_csi1: ldo4 {
> +				regulator-min-microvolt = <2800000>;
> +				regulator-max-microvolt = <2800000>;
> +				regulator-always-on;
> +				regulator-name="vcc_csi1";
> +			};
> +
> +			ldo5 {
> +				regulator-min-microvolt = <2800000>;
> +				regulator-max-microvolt = <2800000>;
> +				regulator-name="reg_unused";
> +			};
> +
> +			vdd_cpu: dcdc2 {
> +				regulator-min-microvolt = <1250000>;
> +				regulator-max-microvolt = <1550000>;
> +				regulator-always-on;
> +				regulator-name="vdd_cpu";
> +			};
> +
> +			vdd_int: dcdc3 {
> +				regulator-min-microvolt = <1250000>;
> +				regulator-max-microvolt = <1250000>;
> +				regulator-always-on;
> +				regulator-name="vdd_int";
> +			};
> +		};
>   	};
>   };
>
>

[PATCH] sunxi: a10-lime: add regulator nodes

[Iain Paton]

On 27/03/15 11:02, Hans de Goede wrote:
> Hi Iain,
> 
> On 27-03-15 11:58, Iain Paton wrote:
>> add pmic regulator definitions matching the manufacturers 3.4.x fex
>> file.
>>
>> Signed-off-by: Iain Paton <ipaton0@gmail.com>
>> ---
>>
>> As this file belongs to Hans and he decided not to use axp209.dtsi in
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/315612.html
>> then I won't add it here.
> 
> That was because using it was breaking stuff, but now that we know better
> which regulators are used for what, and which one we most add an always-on;
> property too, I would greatly prefer you to acually use axp209.dtsi, see
> e.g. the sun5i-a13-utoo-p66.dts file where I'm using it.
> 
> Can you please respin this patch using axp209.dtsi?
> 
> Also please do not add nodes for unused regulators, like the ldo-s for the csi-s.

You would prefer that the csi is broken on the lime then?  Unlike the cubieboards, 
ldo3 & ldo4 really are used on the olimex boards. So it seems that as yet you still 
don't know enough about which regulators are used, or you wouldn't be asking for 
that.

That's the major disadvantage of axp209.dtsi, the regulator node isn't describing 
the pmic at all. Instead it's describing the stuff connected to the pmic outputs, 
which is undeniably board specific and therefore totally unsuitable for a generic
file. Leading to every board needing to override everything regardless.

So no, I'm not going to respin the patch. I strongly believe using axp209.dtsi is 
the wrong thing to do here.
If that means the patch doesn't make it in, then so be it.

Rgds,
Iain

[PATCH] sunxi: a10-lime: add regulator nodes

[Hans de Goede]

Hi,

On 03/28/2015 12:53 PM, Iain Paton wrote:
> On 27/03/15 11:02, Hans de Goede wrote:
>> Hi Iain,
>>
>> On 27-03-15 11:58, Iain Paton wrote:
>>> add pmic regulator definitions matching the manufacturers 3.4.x fex
>>> file.
>>>
>>> Signed-off-by: Iain Paton <ipaton0@gmail.com>
>>> ---
>>>
>>> As this file belongs to Hans and he decided not to use axp209.dtsi in
>>> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/315612.html
>>> then I won't add it here.
>>
>> That was because using it was breaking stuff, but now that we know better
>> which regulators are used for what, and which one we most add an always-on;
>> property too, I would greatly prefer you to acually use axp209.dtsi, see
>> e.g. the sun5i-a13-utoo-p66.dts file where I'm using it.
>>
>> Can you please respin this patch using axp209.dtsi?
>>
>> Also please do not add nodes for unused regulators, like the ldo-s for the csi-s.
>
> You would prefer that the csi is broken on the lime then?  Unlike the cubieboards,
> ldo3 & ldo4 really are used on the olimex boards.

So your olimex boards have camera sensors attached? Because if not then csi is not
used at all.

> So it seems that as yet you still
> don't know enough about which regulators are used, or you wouldn't be asking for
> that.

It would seem that you do not know what csi is for ...

> That's the major disadvantage of axp209.dtsi, the regulator node isn't describing
> the pmic at all. Instead it's describing the stuff connected to the pmic outputs,

Not it is not, not anymore it is just a skeleton creating the regulator nodes
so that you can address them by reference as is now the norm for configuring
nodes in a dts. Have you actually looked at the current axp209.dtsi it does
not contain any assumptions about how the regulators are used.

> which is undeniably board specific and therefore totally unsuitable for a generic
> file. Leading to every board needing to override everything regardless.
>
> So no, I'm not going to respin the patch. I strongly believe using axp209.dtsi is
> the wrong thing to do here.
> If that means the patch doesn't make it in, then so be it.

Please take a look at sun5i-a13-utoo-p66.dts, if you do something
similar like that, then ldo 3 and 4 will get disabled, please run
some tests this way, then I expect that you will see that everything still
works fine without them enabled, and if not you can easily enable them the
same way the other regulators are enabled in sun5i-a13-utoo-p66.dts

Regards,

Hans

[PATCH] sunxi: a10-lime: add regulator nodes

[Iain Paton]

On 28/03/15 23:50, Hans de Goede wrote:

> So your olimex boards have camera sensors attached? Because if not then csi is not
> used at all.

Yes.

>> So it seems that as yet you still
>> don't know enough about which regulators are used, or you wouldn't be asking for
>> that.
> 
> It would seem that you do not know what csi is for ...

I know quite well what it's for. My HW stops working when I do what you're asking.

Rgds,
Iain

[PATCH] sunxi: a10-lime: add regulator nodes

[Maxime Ripard]

On Sat, Mar 28, 2015 at 11:53:06AM +0000, Iain Paton wrote:
> On 27/03/15 11:02, Hans de Goede wrote:
> > Hi Iain,
> > 
> > On 27-03-15 11:58, Iain Paton wrote:
> >> add pmic regulator definitions matching the manufacturers 3.4.x fex
> >> file.
> >>
> >> Signed-off-by: Iain Paton <ipaton0@gmail.com>
> >> ---
> >>
> >> As this file belongs to Hans and he decided not to use axp209.dtsi in
> >> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/315612.html
> >> then I won't add it here.
> > 
> > That was because using it was breaking stuff, but now that we know better
> > which regulators are used for what, and which one we most add an always-on;
> > property too, I would greatly prefer you to acually use axp209.dtsi, see
> > e.g. the sun5i-a13-utoo-p66.dts file where I'm using it.
> > 
> > Can you please respin this patch using axp209.dtsi?
> > 
> > Also please do not add nodes for unused regulators, like the ldo-s for the csi-s.
> 
> You would prefer that the csi is broken on the lime then? 

CSI is not usable (no driver for it), neither it is actually used by
default on the lime, so disabling the regulator is the right thing to
do.

> Unlike the cubieboards, ldo3 & ldo4 really are used on the olimex
> boards. So it seems that as yet you still don't know enough about
> which regulators are used, or you wouldn't be asking for that.

They are not used. They are connected to something that is not usable
in itself, so leaving them enabled is just useless.

> That's the major disadvantage of axp209.dtsi, the regulator node
> isn't describing the pmic at all. Instead it's describing the stuff
> connected to the pmic outputs, which is undeniably board specific
> and therefore totally unsuitable for a generic file. Leading to
> every board needing to override everything regardless.

No, it's defining which regulators are provided by the regulator, and
the voltage boundaries they have. It doesn't make any assumption with
regards to what is connected to what, and if a particular regulator is
connected to something. That's something that the board DTS should
describe as accurately as possible.

> So no, I'm not going to respin the patch. I strongly believe using
> axp209.dtsi is the wrong thing to do here.  If that means the patch
> doesn't make it in, then so be it.

Too bad.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150330/7474f066/attachment.sig>

[PATCH] sunxi: a10-lime: add regulator nodes

[Iain Paton]

On 30/03/15 23:23, Maxime Ripard wrote:

> CSI is not usable (no driver for it), neither it is actually used by
> default on the lime, so disabling the regulator is the right thing to
> do.

My userspace code works just fine, until the regulator gets turned off.

Anyway, I don't have the patience Hans showed with the simplefb stuff.
So consider yourselves successful in wearing me down. I'm done debating 
any of this. One less contributor to linux-sunxi for you to worry about, 
I won't bother you anymore.

> Too bad.

For who?  

Rgds,
Iain

[PATCH] sunxi: a10-lime: add regulator nodes

[Hans de Goede]

Hi,

On 04-04-15 12:30, Iain Paton wrote:
> On 30/03/15 23:23, Maxime Ripard wrote:
>
>> CSI is not usable (no driver for it), neither it is actually used by
>> default on the lime, so disabling the regulator is the right thing to
>> do.
>
> My userspace code works just fine, until the regulator gets turned off.

So you've userspace code to use the csi interface? Cool! Any chance you
can publish this somewhere. I've writing v4l2 drivers for the csi
on the sunxi SoCs somewhere (low) on my todo list.

> Anyway, I don't have the patience Hans showed with the simplefb stuff.
> So consider yourselves successful in wearing me down. I'm done debating
> any of this. One less contributor to linux-sunxi for you to worry about,
> I won't bother you anymore.

I'm sorry to hear that, it would have helped tremendously here if
you had explained that you actually have a camera sensor hooked up
and are using that with an upstream kernel right from the start.

As I've hopefully shown when merging your u-boot patches we certainly
welcome you as a contributor, and I do hope you will keep contributing
in the future.

As for this specific patch I'm still of the opinion that your original
patch is not the right way forward, we are using axp209.dtsi with all
new boards and we are slowly moving existing boards over too.

Note that it is only a matter of time until someone does submit
a patch which follows the standard way of doing things for sunxi
boards and then your camera sensors will stop working.

Recent kernel support something known as devicetree overlays which
I believe is the way forward with this, what we would like to see
for cases like this one is to have a patch in the main dts which
works as Maxime and I requested, and then use a dts overlay to
enable ldo3 and ldo4 for your specific situation. This solution
should also be future proof, since as said someone will make the
requested changes eventually as we plan to move all boards to
using axp209.dtsi and not enabling unnecessary regulators.

You will likely object to me labeling the ldo3 and ldo4 regulators
as unnecessary but think this through for a moment Olimex has
sold thousands of lime boards, how much of those do you think
will have camera sensors attached? For the default upstream
dts file we need to do what is right for the normal / default
use case here. And for people who have special hardware /
daughtboards attached that is exactly what devicetree overlays
are designed to handle.

Regards,

Hans




>
>> Too bad.
>
> For who?
>
> Rgds,
> Iain
>

[PATCH] sunxi: a10-lime: add regulator nodes

[Javier Martinez Canillas]

[adding Mark Brown and Stephen Warren to cc list]

Hello,

On Tue, Mar 31, 2015 at 12:23 AM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Sat, Mar 28, 2015 at 11:53:06AM +0000, Iain Paton wrote:
>> On 27/03/15 11:02, Hans de Goede wrote:
>> > Hi Iain,
>> >
>> > On 27-03-15 11:58, Iain Paton wrote:
>> >> add pmic regulator definitions matching the manufacturers 3.4.x fex
>> >> file.
>> >>
>> >> Signed-off-by: Iain Paton <ipaton0@gmail.com>
>> >> ---
>> >>
>> >> As this file belongs to Hans and he decided not to use axp209.dtsi in
>> >> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/315612.html
>> >> then I won't add it here.
>> >
>> > That was because using it was breaking stuff, but now that we know better
>> > which regulators are used for what, and which one we most add an always-on;
>> > property too, I would greatly prefer you to acually use axp209.dtsi, see
>> > e.g. the sun5i-a13-utoo-p66.dts file where I'm using it.
>> >
>> > Can you please respin this patch using axp209.dtsi?
>> >
>> > Also please do not add nodes for unused regulators, like the ldo-s for the csi-s.
>>
>> You would prefer that the csi is broken on the lime then?
>
> CSI is not usable (no driver for it), neither it is actually used by
> default on the lime, so disabling the regulator is the right thing to
> do.
>
>> Unlike the cubieboards, ldo3 & ldo4 really are used on the olimex
>> boards. So it seems that as yet you still don't know enough about
>> which regulators are used, or you wouldn't be asking for that.
>
> They are not used. They are connected to something that is not usable
> in itself, so leaving them enabled is just useless.
>
>> That's the major disadvantage of axp209.dtsi, the regulator node
>> isn't describing the pmic at all. Instead it's describing the stuff
>> connected to the pmic outputs, which is undeniably board specific
>> and therefore totally unsuitable for a generic file. Leading to
>> every board needing to override everything regardless.
>
> No, it's defining which regulators are provided by the regulator, and
> the voltage boundaries they have. It doesn't make any assumption with
> regards to what is connected to what, and if a particular regulator is
> connected to something. That's something that the board DTS should
> describe as accurately as possible.
>
>> So no, I'm not going to respin the patch. I strongly believe using
>> axp209.dtsi is the wrong thing to do here.  If that means the patch
>> doesn't make it in, then so be it.
>
> Too bad.
>

I'm not familiar with the sunxi platform but just wanted to mention
that AFAIK Mark Brown thinks that generic PMIC .dtsi files describing
regulators is not a good idea. At least he nacked [0] when I tried to
do something similar to avoid having duplicated DTS fragments for the
tps65090 PMU used in the Exynos Chromebooks. But I've added him as cc
in case I misunderstood something.

Stephen Warren (cc'ed too) also said that those files are not very
useful [1] since are largely content free and the regulators
description would have to be duplicated in the DTS anyways since that
information is very board specific. Which is the point that Ian makes
for not using the axp209.dtsi.

The only two platforms that have included .dtsi for PMICs are sunxi
and omap (i.e: twl4030.dtsi) but Mark said that at least in the case
of the twl4030 pmic, the .dtsi is broken [0].

As I said, I'm not particularly interested in sunxi but I think it is
good to have consistency on how things are done across the different
platforms.

> Maxime

Best regards,
Javier

[0]: http://lists.infradead.org/pipermail/linux-arm-kernel/2014-August/278983.html
[1]: http://lists.infradead.org/pipermail/linux-arm-kernel/2014-August/279265.html

[PATCH] sunxi: a10-lime: add regulator nodes

[Mark Brown]

On Sat, Apr 04, 2015 at 02:18:12PM +0200, Javier Martinez Canillas wrote:
> On Tue, Mar 31, 2015 at 12:23 AM, Maxime Ripard

> > No, it's defining which regulators are provided by the regulator, and
> > the voltage boundaries they have. It doesn't make any assumption with
> > regards to what is connected to what, and if a particular regulator is
> > connected to something. That's something that the board DTS should
> > describe as accurately as possible.

This is broken - think about what this means.  If you are defining a
voltage (or any other constraint) you're saying that it's safe to use on
a given board.  If you provide a voltage constraint saying that the
maximum allowable range for a voltage regulator is safe.  That's
unlikely to be true on any given board, usually only a limited set of
regulators can vary voltages at runtime safely at all and then rarely
over their full supported range.  Similarly for other constraints, for
example allowing a regulator to be disabled when there are driverless
things (or drivers without regulator support or mappings for that board)
relying on it is going to break.

Providing a list of the regulators is safe but not really doing a huge
amount.

Note that I've not seen the original patch so it's possible it's doing
something different.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150404/a64ae418/attachment-0001.sig>

[PATCH] sunxi: a10-lime: add regulator nodes

[Maxime Ripard]

Hi Mark,

On Sat, Apr 04, 2015 at 01:33:02PM +0100, Mark Brown wrote:
> On Sat, Apr 04, 2015 at 02:18:12PM +0200, Javier Martinez Canillas wrote:
> > On Tue, Mar 31, 2015 at 12:23 AM, Maxime Ripard
> 
> > > No, it's defining which regulators are provided by the regulator, and
> > > the voltage boundaries they have. It doesn't make any assumption with
> > > regards to what is connected to what, and if a particular regulator is
> > > connected to something. That's something that the board DTS should
> > > describe as accurately as possible.
> 
> This is broken - think about what this means.  If you are defining a
> voltage (or any other constraint) you're saying that it's safe to use on
> a given board. If you provide a voltage constraint saying that the
> maximum allowable range for a voltage regulator is safe.  That's
> unlikely to be true on any given board, usually only a limited set of
> regulators can vary voltages at runtime safely at all and then rarely
> over their full supported range.  Similarly for other constraints, for
> example allowing a regulator to be disabled when there are driverless
> things (or drivers without regulator support or mappings for that board)
> relying on it is going to break.
> 
> Providing a list of the regulators is safe but not really doing a huge
> amount.
> 
> Note that I've not seen the original patch so it's possible it's doing
> something different.

If this model is broken, then I don't see how the full regulator
support is not broken.

Our PMIC DTSI sets up the regulators with the ranges supported by the
PMIC. We have two cases then:
  - Either the regulator is not used on the board, and it will be
    disabled by the framework.
  - Or the regulator is actually used on the board, it will be defined
    in the DTS, possibly with additional constraints.

I really don't see how your above comments apply to that
situation. What I do see though, is that if we drop the DTSI, all our
boards will have to define all the PMIC regulators, even if they're
not used at all on the board, just to have the right behaviour. And
that's clearly a no-go for me.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150404/7c372f6e/attachment.sig>

[PATCH] sunxi: a10-lime: add regulator nodes

[Hans de Goede]

Hi,

On 04-04-15 14:56, Maxime Ripard wrote:
> Hi Mark,
>
> On Sat, Apr 04, 2015 at 01:33:02PM +0100, Mark Brown wrote:
>> On Sat, Apr 04, 2015 at 02:18:12PM +0200, Javier Martinez Canillas wrote:
>>> On Tue, Mar 31, 2015 at 12:23 AM, Maxime Ripard
>>
>>>> No, it's defining which regulators are provided by the regulator, and
>>>> the voltage boundaries they have. It doesn't make any assumption with
>>>> regards to what is connected to what, and if a particular regulator is
>>>> connected to something. That's something that the board DTS should
>>>> describe as accurately as possible.
>>
>> This is broken - think about what this means.  If you are defining a
>> voltage (or any other constraint) you're saying that it's safe to use on
>> a given board. If you provide a voltage constraint saying that the
>> maximum allowable range for a voltage regulator is safe.  That's
>> unlikely to be true on any given board, usually only a limited set of
>> regulators can vary voltages at runtime safely at all and then rarely
>> over their full supported range.  Similarly for other constraints, for
>> example allowing a regulator to be disabled when there are driverless
>> things (or drivers without regulator support or mappings for that board)
>> relying on it is going to break.
>>
>> Providing a list of the regulators is safe but not really doing a huge
>> amount.
>>
>> Note that I've not seen the original patch so it's possible it's doing
>> something different.
>
> If this model is broken, then I don't see how the full regulator
> support is not broken.
>
> Our PMIC DTSI sets up the regulators with the ranges supported by the
> PMIC. We have two cases then:
>    - Either the regulator is not used on the board, and it will be
>      disabled by the framework.
>    - Or the regulator is actually used on the board, it will be defined
>      in the DTS, possibly with additional constraints.


Note that we've already stopped defining any constraints in the dtsi as
that indeed does not make much sense, if the dts does not provide
any constraints but does use / enable the regulator then:
1) That is a bug in the dts really
2) It will end up with the actual hardware constraints anyways as those
are coded into the driver.

>What I do see though, is that if we drop the DTSI, all our
> boards will have to define all the PMIC regulators, even if they're
> not used at all on the board, just to have the right behaviour. And
> that's clearly a no-go for me.

Right the advantage of the dtsi is that it lasts ALL regulators,
so that unused ones actually get turned off, where as a dts not using
the dtsi and only containing those it actually uses will result in
the unused regulators being untouched (since they are not listed)
rather then turned off.

Regards,

Hans

[PATCH] sunxi: a10-lime: add regulator nodes

[Mark Brown]

On Sat, Apr 04, 2015 at 03:05:44PM +0200, Hans de Goede wrote:
> On 04-04-15 14:56, Maxime Ripard wrote:

> >What I do see though, is that if we drop the DTSI, all our
> >boards will have to define all the PMIC regulators, even if they're
> >not used at all on the board, just to have the right behaviour. And
> >that's clearly a no-go for me.

> Right the advantage of the dtsi is that it lasts ALL regulators,
> so that unused ones actually get turned off, where as a dts not using
> the dtsi and only containing those it actually uses will result in
> the unused regulators being untouched (since they are not listed)
> rather then turned off.

Like I said in reply to Maxim a .dtsi does not seem the place to do this
- if it's safe to do it we should be doing it in the core code.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150404/0006e92a/attachment.sig>

[PATCH] sunxi: a10-lime: add regulator nodes

[Mark Brown]

On Sat, Apr 04, 2015 at 02:56:45PM +0200, Maxime Ripard wrote:
> On Sat, Apr 04, 2015 at 01:33:02PM +0100, Mark Brown wrote:

> > This is broken - think about what this means.  If you are defining a
> > voltage (or any other constraint) you're saying that it's safe to use on
> > a given board. If you provide a voltage constraint saying that the
> > maximum allowable range for a voltage regulator is safe.  That's
> > unlikely to be true on any given board, usually only a limited set of
> > regulators can vary voltages at runtime safely at all and then rarely
> > over their full supported range.  Similarly for other constraints, for
> > example allowing a regulator to be disabled when there are driverless
> > things (or drivers without regulator support or mappings for that board)
> > relying on it is going to break.

> If this model is broken, then I don't see how the full regulator
> support is not broken.

> Our PMIC DTSI sets up the regulators with the ranges supported by the
> PMIC. We have two cases then:
>   - Either the regulator is not used on the board, and it will be
>     disabled by the framework.

Or the regulator is used by some passive component on the board that
doesn't appear in DT (or some driver that doesn't yet have DT support
doesn't claim the regulator) and it gets switched off.  Or we power it
off and then later discover that we've been placing physical stress on
the system.

>   - Or the regulator is actually used on the board, it will be defined
>     in the DTS, possibly with additional constraints.

Especially for voltage range constraints you're assuming that every
system integrator will get everything right from their initial attempt
to power up Linux onwards and will anticipate future enhancements to the
drivers which try to vary voltages.  For any given design you can pretty
much guarantee that the electrical engineers would not approve of
software making use of the maximum allowable voltage variation but that
is what you're proposing to enable.  That is clearly not a sensible or
reasonable default, the best we can do is just not vary the voltage and
let the system integrator say if it's safe to do so.

Just listing the regulators is less likely to be an issue, it's more
just not particularly useful than anything else, though it can
potentially cause long term stresses too.

Providing .dtsis for reference designs can make sense, if there's a
widely cloned board with lots of common design elements then reusing
that .dtsi is fine but clearly that .dtsi isn't going to enable the full
flexibility of the regulators since there will be constraints from that
reference design.

> I really don't see how your above comments apply to that
> situation. What I do see though, is that if we drop the DTSI, all our
> boards will have to define all the PMIC regulators, even if they're
> not used at all on the board, just to have the right behaviour. And
> that's clearly a no-go for me.

Nothing else is safe.  Remember, if we get this wrong we risk system
instability and at worst physical damage to the system either in rare
cases immediately due to getting something badly wrong or more likely
from long term electrical stresses.  If what you were proposing were
safe then we should still not be enumerating everything in device trees,
we should just be enabling all operations for every regulator in the
system by default.  The fact that you have to override this should be a
warning sign that you shouldn't be doing it.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150404/a5fdb964/attachment.sig>

[PATCH] sunxi: a10-lime: add regulator nodes

[Maxime Ripard]

On Sat, Apr 04, 2015 at 07:47:16PM +0100, Mark Brown wrote:
> On Sat, Apr 04, 2015 at 02:56:45PM +0200, Maxime Ripard wrote:
> > On Sat, Apr 04, 2015 at 01:33:02PM +0100, Mark Brown wrote:
> 
> > > This is broken - think about what this means.  If you are defining a
> > > voltage (or any other constraint) you're saying that it's safe to use on
> > > a given board. If you provide a voltage constraint saying that the
> > > maximum allowable range for a voltage regulator is safe.  That's
> > > unlikely to be true on any given board, usually only a limited set of
> > > regulators can vary voltages at runtime safely at all and then rarely
> > > over their full supported range.  Similarly for other constraints, for
> > > example allowing a regulator to be disabled when there are driverless
> > > things (or drivers without regulator support or mappings for that board)
> > > relying on it is going to break.
> 
> > If this model is broken, then I don't see how the full regulator
> > support is not broken.
> 
> > Our PMIC DTSI sets up the regulators with the ranges supported by the
> > PMIC. We have two cases then:
> >   - Either the regulator is not used on the board, and it will be
> >     disabled by the framework.
> 
> Or the regulator is used by some passive component on the board that
> doesn't appear in DT (or some driver that doesn't yet have DT support
> doesn't claim the regulator) and it gets switched off.  Or we power it
> off and then later discover that we've been placing physical stress on
> the system.

Isn't that what the regulator-always-on property is here for?

> >   - Or the regulator is actually used on the board, it will be defined
> >     in the DTS, possibly with additional constraints.
> 
> Especially for voltage range constraints you're assuming that every
> system integrator will get everything right from their initial attempt
> to power up Linux onwards and will anticipate future enhancements to the
> drivers which try to vary voltages.  For any given design you can pretty
> much guarantee that the electrical engineers would not approve of
> software making use of the maximum allowable voltage variation but that
> is what you're proposing to enable.  That is clearly not a sensible or
> reasonable default, the best we can do is just not vary the voltage and
> let the system integrator say if it's safe to do so.
> 
> Just listing the regulators is less likely to be an issue, it's more
> just not particularly useful than anything else, though it can
> potentially cause long term stresses too.

My bad, that's actually just what we are doing, plus defining the
regulators that are at a fixed voltage and can't be disabled.

> Providing .dtsis for reference designs can make sense, if there's a
> widely cloned board with lots of common design elements then reusing
> that .dtsi is fine but clearly that .dtsi isn't going to enable the full
> flexibility of the regulators since there will be constraints from that
> reference design.

That PMIC is used with various SoCs, so that's not really an
option. And especially when it comes to regulators, I don't think
there's really a pattern that emerged yet...

> > I really don't see how your above comments apply to that
> > situation. What I do see though, is that if we drop the DTSI, all our
> > boards will have to define all the PMIC regulators, even if they're
> > not used at all on the board, just to have the right behaviour. And
> > that's clearly a no-go for me.
> 
> Nothing else is safe.  Remember, if we get this wrong we risk system
> instability and at worst physical damage to the system either in rare
> cases immediately due to getting something badly wrong or more likely
> from long term electrical stresses.  If what you were proposing were
> safe then we should still not be enumerating everything in device trees,
> we should just be enabling all operations for every regulator in the
> system by default.  The fact that you have to override this should be a
> warning sign that you shouldn't be doing it.

Well, we're doing low level stuff and (from experience) things like a
poor muxing option can cause physical damage too, and we don't have
any kind of protection or policy against that.

So, what do you suggest for the kernel to have a behaviour independant
of what the bootloader state was, without adding a lot of churn in
each and every DTS?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150407/f828df80/attachment.sig>

[PATCH] sunxi: a10-lime: add regulator nodes

[Mark Brown]

On Tue, Apr 07, 2015 at 12:17:57PM +0200, Maxime Ripard wrote:
> On Sat, Apr 04, 2015 at 07:47:16PM +0100, Mark Brown wrote:

> > Or the regulator is used by some passive component on the board that
> > doesn't appear in DT (or some driver that doesn't yet have DT support
> > doesn't claim the regulator) and it gets switched off.  Or we power it
> > off and then later discover that we've been placing physical stress on
> > the system.

> Isn't that what the regulator-always-on property is here for?

Only if it's actually added by someone after having looked at the
system...

> > Providing .dtsis for reference designs can make sense, if there's a
> > widely cloned board with lots of common design elements then reusing
> > that .dtsi is fine but clearly that .dtsi isn't going to enable the full
> > flexibility of the regulators since there will be constraints from that
> > reference design.

> That PMIC is used with various SoCs, so that's not really an
> option. And especially when it comes to regulators, I don't think
> there's really a pattern that emerged yet...

I'm not sure you saw the "reference design" part of the above?

> > Nothing else is safe.  Remember, if we get this wrong we risk system
> > instability and at worst physical damage to the system either in rare
> > cases immediately due to getting something badly wrong or more likely
> > from long term electrical stresses.  If what you were proposing were
> > safe then we should still not be enumerating everything in device trees,
> > we should just be enabling all operations for every regulator in the
> > system by default.  The fact that you have to override this should be a
> > warning sign that you shouldn't be doing it.

> Well, we're doing low level stuff and (from experience) things like a
> poor muxing option can cause physical damage too, and we don't have
> any kind of protection or policy against that.

Does the pinmux code actively go in and do things without being
explicitly configured?

> So, what do you suggest for the kernel to have a behaviour independant
> of what the bootloader state was, without adding a lot of churn in
> each and every DTS?

Well, I'm fairly happy with the current state.  I think having some sort
of board level property saying that undescribed regulators can be
enabled and disabled (which I think is all that this is really doing)
might be a viable solution?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150408/37e07606/attachment.sig>

[PATCH] sunxi: a10-lime: add regulator nodes

[Maxime Ripard]

On Wed, Apr 08, 2015 at 01:35:06PM +0100, Mark Brown wrote:
> On Tue, Apr 07, 2015 at 12:17:57PM +0200, Maxime Ripard wrote:
> > On Sat, Apr 04, 2015 at 07:47:16PM +0100, Mark Brown wrote:
> 
> > > Or the regulator is used by some passive component on the board that
> > > doesn't appear in DT (or some driver that doesn't yet have DT support
> > > doesn't claim the regulator) and it gets switched off.  Or we power it
> > > off and then later discover that we've been placing physical stress on
> > > the system.
> 
> > Isn't that what the regulator-always-on property is here for?
> 
> Only if it's actually added by someone after having looked at the
> system...

True, but that can also be said for pretty much any DT patch. If you
do something wrong in your DT and haven't looked carefuly at your
datasheet/schematics, here be dragons.

> > > Providing .dtsis for reference designs can make sense, if there's a
> > > widely cloned board with lots of common design elements then reusing
> > > that .dtsi is fine but clearly that .dtsi isn't going to enable the full
> > > flexibility of the regulators since there will be constraints from that
> > > reference design.
> 
> > That PMIC is used with various SoCs, so that's not really an
> > option. And especially when it comes to regulators, I don't think
> > there's really a pattern that emerged yet...
> 
> I'm not sure you saw the "reference design" part of the above?

I saw that, I was just saying that this doesn't really work for us
unfortunately.

> > > Nothing else is safe.  Remember, if we get this wrong we risk system
> > > instability and at worst physical damage to the system either in rare
> > > cases immediately due to getting something badly wrong or more likely
> > > from long term electrical stresses.  If what you were proposing were
> > > safe then we should still not be enumerating everything in device trees,
> > > we should just be enabling all operations for every regulator in the
> > > system by default.  The fact that you have to override this should be a
> > > warning sign that you shouldn't be doing it.
> 
> > Well, we're doing low level stuff and (from experience) things like a
> > poor muxing option can cause physical damage too, and we don't have
> > any kind of protection or policy against that.
> 
> Does the pinmux code actively go in and do things without being
> explicitly configured?

You have a point :)

> > So, what do you suggest for the kernel to have a behaviour
> > independant of what the bootloader state was, without adding a lot
> > of churn in each and every DTS?
> 
> Well, I'm fairly happy with the current state.  I think having some
> sort of board level property saying that undescribed regulators can
> be enabled and disabled (which I think is all that this is really
> doing) might be a viable solution?

It looks like configuration to me ;)

Especially since the semantic of it would be to disable something that
is not even declared in the DT in the first place.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150413/1390e7d1/attachment.sig>

[PATCH] sunxi: a10-lime: add regulator nodes

[Mark Brown]

On Mon, Apr 13, 2015 at 10:15:18AM +0200, Maxime Ripard wrote:
> On Wed, Apr 08, 2015 at 01:35:06PM +0100, Mark Brown wrote:
> > On Tue, Apr 07, 2015 at 12:17:57PM +0200, Maxime Ripard wrote:
> > > On Sat, Apr 04, 2015 at 07:47:16PM +0100, Mark Brown wrote:

> > > > Or the regulator is used by some passive component on the board that
> > > > doesn't appear in DT (or some driver that doesn't yet have DT support
> > > > doesn't claim the regulator) and it gets switched off.  Or we power it
> > > > off and then later discover that we've been placing physical stress on
> > > > the system.

> > > Isn't that what the regulator-always-on property is here for?

> > Only if it's actually added by someone after having looked at the
> > system...

> True, but that can also be said for pretty much any DT patch. If you
> do something wrong in your DT and haven't looked carefuly at your
> datasheet/schematics, here be dragons.

There's a difference between actively choosing to do something and
having it done by default - the goal here is to make sure that if
someone didn't even look at something we're as safe as possible.

> > > > Providing .dtsis for reference designs can make sense, if there's a
> > > > widely cloned board with lots of common design elements then reusing
> > > > that .dtsi is fine but clearly that .dtsi isn't going to enable the full
> > > > flexibility of the regulators since there will be constraints from that
> > > > reference design.

> > > That PMIC is used with various SoCs, so that's not really an
> > > option. And especially when it comes to regulators, I don't think
> > > there's really a pattern that emerged yet...

> > I'm not sure you saw the "reference design" part of the above?

> I saw that, I was just saying that this doesn't really work for us
> unfortunately.

I really don't understand how your explanation follows at all, sorry.
How does the ability to use the same PMIC with multiple SoCs influence
reference designs?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150413/03c6949b/attachment.sig>

[PATCH] sunxi: a10-lime: add regulator nodes

[Maxime Ripard]

On Mon, Apr 13, 2015 at 12:44:28PM +0100, Mark Brown wrote:
> On Mon, Apr 13, 2015 at 10:15:18AM +0200, Maxime Ripard wrote:
> > On Wed, Apr 08, 2015 at 01:35:06PM +0100, Mark Brown wrote:
> > > On Tue, Apr 07, 2015 at 12:17:57PM +0200, Maxime Ripard wrote:
> > > > On Sat, Apr 04, 2015 at 07:47:16PM +0100, Mark Brown wrote:
> 
> > > > > Or the regulator is used by some passive component on the board that
> > > > > doesn't appear in DT (or some driver that doesn't yet have DT support
> > > > > doesn't claim the regulator) and it gets switched off.  Or we power it
> > > > > off and then later discover that we've been placing physical stress on
> > > > > the system.
> 
> > > > Isn't that what the regulator-always-on property is here for?
> 
> > > Only if it's actually added by someone after having looked at the
> > > system...
> 
> > True, but that can also be said for pretty much any DT patch. If you
> > do something wrong in your DT and haven't looked carefuly at your
> > datasheet/schematics, here be dragons.
> 
> There's a difference between actively choosing to do something and
> having it done by default - the goal here is to make sure that if
> someone didn't even look at something we're as safe as possible.

I don't think that discussion is getting anywhere.

I do understand your point, but I also am under the impression that in
order to allow people to get the regulators wrong, we place the
maintainance burden on those who get it right...

> > > > > Providing .dtsis for reference designs can make sense, if there's a
> > > > > widely cloned board with lots of common design elements then reusing
> > > > > that .dtsi is fine but clearly that .dtsi isn't going to enable the full
> > > > > flexibility of the regulators since there will be constraints from that
> > > > > reference design.
> 
> > > > That PMIC is used with various SoCs, so that's not really an
> > > > option. And especially when it comes to regulators, I don't think
> > > > there's really a pattern that emerged yet...
> 
> > > I'm not sure you saw the "reference design" part of the above?
> 
> > I saw that, I was just saying that this doesn't really work for us
> > unfortunately.
> 
> I really don't understand how your explanation follows at all, sorry.
> How does the ability to use the same PMIC with multiple SoCs influence
> reference designs?

Maybe we don't have the same definition for a reference design then,
but to me, it's a board that is used as a starting point for other
vendors to create their boards.

If so, the central piece of this board is actually the SoC. Having
multiple SoCs will prevent any sharing of the DTSI, won't it?

And like I already said, we're not in such a case anyway. Pretty much
all boards we've seen so far wire the regulators differently.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150414/5e6c2651/attachment-0001.sig>

[PATCH] sunxi: a10-lime: add regulator nodes

[Mark Brown]

On Tue, Apr 14, 2015 at 06:27:38PM +0200, Maxime Ripard wrote:
> On Mon, Apr 13, 2015 at 12:44:28PM +0100, Mark Brown wrote:

> > There's a difference between actively choosing to do something and
> > having it done by default - the goal here is to make sure that if
> > someone didn't even look at something we're as safe as possible.

> I don't think that discussion is getting anywhere.

> I do understand your point, but I also am under the impression that in
> order to allow people to get the regulators wrong, we place the
> maintainance burden on those who get it right...

Someone is going to be unhappy no matter which default we pick and it
seems better that this is the person who had to type a bit more rather
than the person with hardware damage (or the person who finds their
board suddenly doesn't work because they changed a config option for
that matter).

> > I really don't understand how your explanation follows at all, sorry.
> > How does the ability to use the same PMIC with multiple SoCs influence
> > reference designs?

> Maybe we don't have the same definition for a reference design then,
> but to me, it's a board that is used as a starting point for other
> vendors to create their boards.

Right.

> If so, the central piece of this board is actually the SoC. Having
> multiple SoCs will prevent any sharing of the DTSI, won't it?

If the boards have different SoCs they're probably not the same
reference design, unless they're one of the fully pluggable reference
designs which really don't work well with DT at the best of times.  The
point is that the DTSI for a reference design is for that design not for
the SoC, so for example a DTSI for BeagleBone Black derivatives rather
than a DTSI for the SoC or PMIC on it.

> And like I already said, we're not in such a case anyway. Pretty much
> all boards we've seen so far wire the regulators differently.

OK, so that's the sort of case where this is going to be useful.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150414/0292f828/attachment.sig>

[PATCH] sunxi: a10-lime: add regulator nodes

[Hans de Goede]

Hi,

On 04-04-15 14:33, Mark Brown wrote:
> On Sat, Apr 04, 2015 at 02:18:12PM +0200, Javier Martinez Canillas wrote:
>> On Tue, Mar 31, 2015 at 12:23 AM, Maxime Ripard
>
>>> No, it's defining which regulators are provided by the regulator, and
>>> the voltage boundaries they have. It doesn't make any assumption with
>>> regards to what is connected to what, and if a particular regulator is
>>> connected to something. That's something that the board DTS should
>>> describe as accurately as possible.
>
> This is broken - think about what this means.  If you are defining a
> voltage (or any other constraint) you're saying that it's safe to use on
> a given board.  If you provide a voltage constraint saying that the
> maximum allowable range for a voltage regulator is safe.  That's
> unlikely to be true on any given board, usually only a limited set of
> regulators can vary voltages at runtime safely at all and then rarely
> over their full supported range.  Similarly for other constraints, for
> example allowing a regulator to be disabled when there are driverless
> things (or drivers without regulator support or mappings for that board)
> relying on it is going to break.

Right, this is why in later revisions the pmic dtsi file does not
contain any ranges, these must now all be set by the dts file.

> Providing a list of the regulators is safe but not really doing a huge
> amount.

This (providing a list) is pretty much all the dtsi does, this is actually
somewhat useful, because a regulator must be listed for it to get automatically
turned off when not used. So by having the dtsi + dts files referencing only
the regulators which they use we end up automatically turning off all the
other regulators.

FYI here is the full contents of the dtsi:

/*
  * Copyright 2015 Chen-Yu Tsai
  *
  * Chen-Yu Tsai <wens@csie.org>
  *
  * This file is dual-licensed: you can use it either under the terms
  * of the GPL or the X11 license, at your option. Note that this dual
  * licensing only applies to this file, and not this project as a
  * whole.
  *
  *  a) This file is free software; you can redistribute it and/or
  *     modify it under the terms of the GNU General Public License as
  *     published by the Free Software Foundation; either version 2 of the
  *     License, or (at your option) any later version.
  *
  *     This file is distributed in the hope that it will be useful,
  *     but WITHOUT ANY WARRANTY; without even the implied warranty of
  *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  *     GNU General Public License for more details.
  *
  *     You should have received a copy of the GNU General Public
  *     License along with this file; if not, write to the Free
  *     Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
  *     MA 02110-1301 USA
  *
  * Or, alternatively,
  *
  *  b) Permission is hereby granted, free of charge, to any person
  *     obtaining a copy of this software and associated documentation
  *     files (the "Software"), to deal in the Software without
  *     restriction, including without limitation the rights to use,
  *     copy, modify, merge, publish, distribute, sublicense, and/or
  *     sell copies of the Software, and to permit persons to whom the
  *     Software is furnished to do so, subject to the following
  *     conditions:
  *
  *     The above copyright notice and this permission notice shall be
  *     included in all copies or substantial portions of the Software.
  *
  *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
  *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
  *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
  *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
  *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
  *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
  *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
  *     OTHER DEALINGS IN THE SOFTWARE.
  */

/*
  * AXP202/209 Integrated Power Management Chip
  * http://www.x-powers.com/product/AXP20X.php
  * http://dl.linux-sunxi.org/AXP/AXP209%20Datasheet%20v1.0_cn.pdf
  */

&axp209 {
	compatible = "x-powers,axp209";
	interrupt-controller;
	#interrupt-cells = <1>;

	regulators {
		/* Default work frequency for buck regulators */
		x-powers,dcdc-freq = <1500>;

		reg_dcdc2: dcdc2 {
			regulator-name = "dcdc2";
		};

		reg_dcdc3: dcdc3 {
			regulator-name = "dcdc3";
		};

		reg_ldo1: ldo1 {
			/* LDO1 is a fixed output regulator */
			regulator-always-on;
			regulator-min-microvolt = <1300000>;
			regulator-max-microvolt = <1300000>;
			regulator-name = "ldo1";
		};

		reg_ldo2: ldo2 {
			regulator-name = "ldo2";
		};

		reg_ldo3: ldo3 {
			regulator-name = "ldo3";
		};

		reg_ldo4: ldo4 {
			regulator-name = "ldo4";
		};

		reg_ldo5: ldo5 {
			regulator-name = "ldo5";
		};
	};
};

Regards,

Hans

[PATCH] sunxi: a10-lime: add regulator nodes

[Mark Brown]

On Sat, Apr 04, 2015 at 02:58:23PM +0200, Hans de Goede wrote:

> 	regulators {
> 		/* Default work frequency for buck regulators */
> 		x-powers,dcdc-freq = <1500>;

Are you sure this is safe and correct for all boards (with most
regulator designs I'd expect it to be dictated by passive choice)?  

> 
> 		reg_dcdc2: dcdc2 {
> 			regulator-name = "dcdc2";
> 		};

These names should all be removed, they're vanishingly unlikely to
reflect the schematic supply name (which is what the property is there
for) and this should either be the same as the regulator API comes up
with by default or override any better naming it manages to provide.

> 		reg_ldo1: ldo1 {
> 			/* LDO1 is a fixed output regulator */
> 			regulator-always-on;

If there's no control this should be redundant.

> 			regulator-min-microvolt = <1300000>;
> 			regulator-max-microvolt = <1300000>;

Are you *sure* this voltage is the one used on all boards?  If it's the
physically fixed output voltage then it shouldn't be in the constraints
at all and the driver should just know.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150404/8012dd0b/attachment.sig>