sunxi: a10-lime: add regulator nodes

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

Signed-off-by Iain Paton <[email protected]>

---

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(+)

arch/arm/boot/dts/sun4i-a10-olinuxino-lime.dts +68 -0

index 31dc2f1..f7a8fcc 100644
@@ -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";
+			};
+		};
  	};
  };
 

Hi Iain,

Quoted 9 lines · Iain Paton

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

Signed-off-by Iain Paton <[email protected]>

---

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.

Quoted 18 lines · Hans de Goede

Hi Iain,

Quoted 9 lines · Iain Paton

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

Signed-off-by Iain Paton <[email protected]>

---

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.

Quoted 3 lines

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.

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

Quoted 3 lines

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 ...

Quoted 18 lines · Hans de Goede

Hi Iain,

Quoted 9 lines · Iain Paton

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

Signed-off-by Iain Paton <[email protected]>

---

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.

Quoted 3 lines

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.

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.

Too bad.

Quoted 3 lines · Maxime Ripard

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.

Quoted 2 lines

Too bad.

For who?

Rgds,
Iain

Quoted 19 lines · Iain Paton

Quoted 18 lines · Hans de Goede

Hi Iain,

Quoted 9 lines · Iain Paton

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

Signed-off-by Iain Paton <[email protected]>

---

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.

Quoted 5 lines

Quoted 3 lines

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.

Quoted 3 lines

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.

Quoted 5 lines

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.

Quoted 3 lines

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

On Tue, Mar 31, 2015 at 12:23 AM, Maxime Ripard

Quoted 5 lines

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.

Quoted 7 lines · Javier Martinez Canillas

On Tue, Mar 31, 2015 at 12:23 AM, Maxime Ripard

Quoted 5 lines

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.

Hi Mark,

Quoted 20 lines · Mark Brown

Quoted 7 lines · Javier Martinez Canillas

On Tue, Mar 31, 2015 at 12:23 AM, Maxime Ripard

Quoted 5 lines

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.

Quoted 5 lines

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.

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.

Quoted 4 lines · Maxime Ripard

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.

Quoted 10 lines · Mark Brown

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 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.

Quoted 13 lines · Maxime Ripard

Quoted 10 lines · Mark Brown

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.

Quoted 7 lines

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.

Quoted 2 lines

- 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.

Quoted 5 lines

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.

Quoted 5 lines · Mark Brown

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?

Quoted 5 lines

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...

Quoted 8 lines

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?

Quoted 7 lines · Maxime Ripard

Quoted 5 lines · Mark Brown

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.

Quoted 1 line

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...

Quoted 9 lines

Quoted 5 lines

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?

Quoted 8 lines

Quoted 8 lines

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.

Quoted 3 lines

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?

Quoted 3 lines

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?

Quoted 8 lines · Mark Brown

Quoted 7 lines · Maxime Ripard

Quoted 5 lines · Mark Brown

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.

Quoted 4 lines

Quoted 1 line

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.

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

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.

Quoted 5 lines

Quoted 3 lines

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.

Quoted 14 lines · Maxime Ripard

Quoted 13 lines · Mark Brown

Quoted 9 lines · Maxime Ripard

Quoted 5 lines · Mark Brown

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...

Quoted 3 lines

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.

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

Quoted 5 lines

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...

Quoted 4 lines

Quoted 1 line

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?

Quoted 3 lines · Mark Brown

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...

Quoted 3 lines

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.

Quoted 7 lines · Javier Martinez Canillas

On Tue, Mar 31, 2015 at 12:23 AM, Maxime Ripard

Quoted 5 lines

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.

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

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

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

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