[PATCH] ARM: dts: rockchip: Add vdd_logic to rk3288-veyron

[PATCH] ARM: dts: rockchip: Add vdd_logic to rk3288-veyron

[Douglas Anderson]

The vdd_logic rail controls the voltage supplied to misc logic on
rk3288, including the voltage supplied to the memory controller.  The
vcc logic is implemented by a PWM regulator.

Right now there are no consumers of vdd_logic on veyron but if anyone
ever wants to try to add DDR Freq they'd need it.

Note that in the downstream Chrome OS kernel the PWM regulator has
a voltage table with these points:
  1350000 0%
  1300000 10%
  1250000 20%
  1200000 31%
  1150000 41%
  1125000 46%
  1100000 52%
  1050000 62%
  1000000 72%
   950000 83%

The DDR Freq driver in the downstream kernel only uses some of those
points, namely:
  DDR3:  1200000, 1150000, 1100000, 1050000
  LPDDR:          1150000, 1100000, 1050000

When adapting the downstream kernel to upstream I have opted to switch
to using the "continuous" mode of the PWM regulator driver.  This was
the only way I could get the upstream driver to achieve _exactly_ the
same voltages as the downstream driver could.  Specifically note that
the old driver in downstream Chrome OS 3.14 _didn't_ have the
DIV_ROUND_CLOSEST_ULL() in the Rockchip PLL driver.  That means if I
use the same (downstream) table I might end up with a duty cycle
that's 1 larger than was used downstream, leading to a slightly
different voltage.  Due to the way the rounding worked I couldn't even
just adjust the "percent" by 1 for a given voltage level--certain duty
cycles just aren't achievable with the upstream math for voltage
tables.

Using continuous mode you can achieve the exact same duty cycle by
simply adjusting the voltage you use by a tad bit.  The voltages that
are equivalent to the ones used in the downstream kernel's table are:
  1350000, 1304472, 1255691, 1200407, 1154878,
  1128862, 1099593, 1050813, 1005285, 950000

Note that the top/bottom voltage is exactly the same just due to the
way that continuous mode is calculated and the fact that I used those
as anchors.  I didn't make any attempt to do the resistor math (as was
done on rk3399-gru).

If anyone ever gets DDRFreq working on veyron upstream they should
thus adjust the voltage specified in the DDRFreq operating points
slightly (as per the above) to obtain the existing/tested values.  AKA
you'd use:
  DDR3:  1200407, 1154878, 1099593, 1050813
  LPDDR:          1154878, 1099593, 1050813

A few other notes:
- The "period" here (1994) is different than the "period" downstream
  (2000) for similar reasons: there's a DIV_ROUND_CLOSEST_ULL() that
  wasn't downstream.  With 1994 upstream comes up with the same value
  (0x94) to program into the hardware that downstream put there.  As
  far as I can tell 0x94 actually means 1993.27.
- The duty cycle unit of 0x94 was picked by just matching the period
  which nicely allows us to insert 0x7b as that value to program into
  the hardware for 950mV.  The 0x7b was found by observing what the
  downstream kernel calculated (not that the system can actually run
  with vdd_log at 950 mV).
- The downstream kernel can also be seen to program a different value
  into the CTRL field.  Upstream achieves 0x0b and downstream 0x1b.
  This is because the upstream commit bc834d7b07b4 ("pwm: rockchip:
  Move the configuration of polarity") fixed a bug by adding "ctrl &=
  ~PWM_POLARITY_MASK".  Downstream accidentally left bit 4 set.
  Luckily this bit doesn't matter--it's only used when the PWM goes
  inactive (AKA if it's in oneshot mode or is disabled) and we don't
  do that for the PWM regulator.

I measured the voltage of vdd_log while adjusting it and found that
with the upstream kernel voltage difference between requested and
actual was 9.2 mV at 950 mV and 13.4 mV at 1350 mV with in-between
voltages consistently showing ~1% error.  This error is likely
expected as voltage can be seen to sag a bit when more load is put on
the rail.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

 arch/arm/boot/dts/rk3288-veyron.dtsi | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/arm/boot/dts/rk3288-veyron.dtsi b/arch/arm/boot/dts/rk3288-veyron.dtsi
index 0bc2409f6903..5181d9435fda 100644
--- a/arch/arm/boot/dts/rk3288-veyron.dtsi
+++ b/arch/arm/boot/dts/rk3288-veyron.dtsi
@@ -95,6 +95,23 @@
 		regulator-boot-on;
 		vin-supply = <&vcc_5v>;
 	};
+
+	vdd_logic: vdd-logic {
+		compatible = "pwm-regulator";
+		regulator-name = "vdd_logic";
+
+		pwms = <&pwm1 0 1994 0>;
+		pwm-supply = <&vcc33_sys>;
+
+		pwm-dutycycle-range = <0x7b 0>;
+		pwm-dutycycle-unit = <0x94>;
+
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <950000>;
+		regulator-max-microvolt = <1350000>;
+		regulator-ramp-delay = <4000>;
+	};
 };
 
 &cpu0 {
-- 
2.21.0.225.g810b269d1ac-goog

Re: [PATCH] ARM: dts: rockchip: Add vdd_logic to rk3288-veyron

[Heiko Stuebner]

Am Donnerstag, 21. März 2019, 21:19:44 CET schrieb Douglas Anderson:
> The vdd_logic rail controls the voltage supplied to misc logic on
> rk3288, including the voltage supplied to the memory controller.  The
> vcc logic is implemented by a PWM regulator.
> 
> Right now there are no consumers of vdd_logic on veyron but if anyone
> ever wants to try to add DDR Freq they'd need it.
> 
> Note that in the downstream Chrome OS kernel the PWM regulator has
> a voltage table with these points:
>   1350000 0%
>   1300000 10%
>   1250000 20%
>   1200000 31%
>   1150000 41%
>   1125000 46%
>   1100000 52%
>   1050000 62%
>   1000000 72%
>    950000 83%
> 
> The DDR Freq driver in the downstream kernel only uses some of those
> points, namely:
>   DDR3:  1200000, 1150000, 1100000, 1050000
>   LPDDR:          1150000, 1100000, 1050000
> 
> When adapting the downstream kernel to upstream I have opted to switch
> to using the "continuous" mode of the PWM regulator driver.  This was
> the only way I could get the upstream driver to achieve _exactly_ the
> same voltages as the downstream driver could.  Specifically note that
> the old driver in downstream Chrome OS 3.14 _didn't_ have the
> DIV_ROUND_CLOSEST_ULL() in the Rockchip PLL driver.  That means if I
> use the same (downstream) table I might end up with a duty cycle
> that's 1 larger than was used downstream, leading to a slightly
> different voltage.  Due to the way the rounding worked I couldn't even
> just adjust the "percent" by 1 for a given voltage level--certain duty
> cycles just aren't achievable with the upstream math for voltage
> tables.
> 
> Using continuous mode you can achieve the exact same duty cycle by
> simply adjusting the voltage you use by a tad bit.  The voltages that
> are equivalent to the ones used in the downstream kernel's table are:
>   1350000, 1304472, 1255691, 1200407, 1154878,
>   1128862, 1099593, 1050813, 1005285, 950000
> 
> Note that the top/bottom voltage is exactly the same just due to the
> way that continuous mode is calculated and the fact that I used those
> as anchors.  I didn't make any attempt to do the resistor math (as was
> done on rk3399-gru).
> 
> If anyone ever gets DDRFreq working on veyron upstream they should
> thus adjust the voltage specified in the DDRFreq operating points
> slightly (as per the above) to obtain the existing/tested values.  AKA
> you'd use:
>   DDR3:  1200407, 1154878, 1099593, 1050813
>   LPDDR:          1154878, 1099593, 1050813
> 
> A few other notes:
> - The "period" here (1994) is different than the "period" downstream
>   (2000) for similar reasons: there's a DIV_ROUND_CLOSEST_ULL() that
>   wasn't downstream.  With 1994 upstream comes up with the same value
>   (0x94) to program into the hardware that downstream put there.  As
>   far as I can tell 0x94 actually means 1993.27.
> - The duty cycle unit of 0x94 was picked by just matching the period
>   which nicely allows us to insert 0x7b as that value to program into
>   the hardware for 950mV.  The 0x7b was found by observing what the
>   downstream kernel calculated (not that the system can actually run
>   with vdd_log at 950 mV).
> - The downstream kernel can also be seen to program a different value
>   into the CTRL field.  Upstream achieves 0x0b and downstream 0x1b.
>   This is because the upstream commit bc834d7b07b4 ("pwm: rockchip:
>   Move the configuration of polarity") fixed a bug by adding "ctrl &=
>   ~PWM_POLARITY_MASK".  Downstream accidentally left bit 4 set.
>   Luckily this bit doesn't matter--it's only used when the PWM goes
>   inactive (AKA if it's in oneshot mode or is disabled) and we don't
>   do that for the PWM regulator.
> 
> I measured the voltage of vdd_log while adjusting it and found that
> with the upstream kernel voltage difference between requested and
> actual was 9.2 mV at 950 mV and 13.4 mV at 1350 mV with in-between
> voltages consistently showing ~1% error.  This error is likely
> expected as voltage can be seen to sag a bit when more load is put on
> the rail.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>

applied for 5.2

Thanks
Heiko