| Message ID | 20170723102749.17323-11-icenowy@aosc.io (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Sun, Jul 23, 2017 at 6:27 PM, Icenowy Zheng <icenowy@aosc.io> wrote: > From: Ondrej Jirman <megous@megous.com> > > Add SY8106A regulator to r_i2c bus and enable the r_i2c bus on > Orange Pi PC, then set the power supply of the ARM cores to this > regulator, in order to enable DVFS. > > Signed-off-by: Ondrej Jirman <megous@megous.com> > [Icenowy: Enable DVFS in this patch, slight changes and change commit > message] > Signed-off-by: Icenowy Zheng <icenowy@aosc.io> > --- > arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts > index 998b60f8d295..d855f8b6254e 100644 > --- a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts > +++ b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts > @@ -98,6 +98,10 @@ > status = "okay"; > }; > > +&cpu0 { > + cpu-supply = <®_sy8106a>; > +}; > + > &ehci0 { > status = "okay"; > }; > @@ -160,6 +164,21 @@ > }; > }; > > +&r_i2c { > + status = "okay"; > + > + reg_sy8106a: regulator@65 { > + compatible = "silergy,sy8106a"; > + reg = <0x65>; > + regulator-name = "vdd-cpux"; > + regulator-min-microvolt = <1000000>; According to the H3 datasheet, the minimum voltage is 1.1V, not 1V. Otherwse > + regulator-max-microvolt = <1400000>; > + regulator-ramp-delay = <200>; Is this an actual constraint of the SoC? Or is it a characteristic of the regulator? If it is the latter, it belongs in the driver. AFAIK the regulator supports varying the ramp delay (slew rate). ChenYu > + regulator-boot-on; > + regulator-always-on; > + }; > +}; > + > &r_pio { > leds_r_opc: led_pins@0 { > pins = "PL10"; > -- > 2.13.0 > > -- > You received this message because you are subscribed to the Google Groups "linux-sunxi" group. > To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com. > For more options, visit https://groups.google.com/d/optout.
于 2017年7月26日 GMT+08:00 下午3:08:06, Chen-Yu Tsai <wens@csie.org> 写到: >On Sun, Jul 23, 2017 at 6:27 PM, Icenowy Zheng <icenowy@aosc.io> wrote: >> From: Ondrej Jirman <megous@megous.com> >> >> Add SY8106A regulator to r_i2c bus and enable the r_i2c bus on >> Orange Pi PC, then set the power supply of the ARM cores to this >> regulator, in order to enable DVFS. >> >> Signed-off-by: Ondrej Jirman <megous@megous.com> >> [Icenowy: Enable DVFS in this patch, slight changes and change commit >> message] >> Signed-off-by: Icenowy Zheng <icenowy@aosc.io> >> --- >> arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts | 19 +++++++++++++++++++ >> 1 file changed, 19 insertions(+) >> >> diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts >b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts >> index 998b60f8d295..d855f8b6254e 100644 >> --- a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts >> +++ b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts >> @@ -98,6 +98,10 @@ >> status = "okay"; >> }; >> >> +&cpu0 { >> + cpu-supply = <®_sy8106a>; >> +}; >> + >> &ehci0 { >> status = "okay"; >> }; >> @@ -160,6 +164,21 @@ >> }; >> }; >> >> +&r_i2c { >> + status = "okay"; >> + >> + reg_sy8106a: regulator@65 { >> + compatible = "silergy,sy8106a"; >> + reg = <0x65>; >> + regulator-name = "vdd-cpux"; >> + regulator-min-microvolt = <1000000>; > >According to the H3 datasheet, the minimum voltage is 1.1V, not 1V. But the Armbian OPP table for H3 contains several OPP under 1.1V... > >Otherwse > >> + regulator-max-microvolt = <1400000>; >> + regulator-ramp-delay = <200>; > >Is this an actual constraint of the SoC? Or is it a characteristic >of the regulator? If it is the latter, it belongs in the driver. >AFAIK the regulator supports varying the ramp delay (slew rate). > >ChenYu > >> + regulator-boot-on; >> + regulator-always-on; >> + }; >> +}; >> + >> &r_pio { >> leds_r_opc: led_pins@0 { >> pins = "PL10"; >> -- >> 2.13.0 >> >> -- >> You received this message because you are subscribed to the Google >Groups "linux-sunxi" group. >> To unsubscribe from this group and stop receiving emails from it, >send an email to linux-sunxi+unsubscribe@googlegroups.com. >> For more options, visit https://groups.google.com/d/optout.
On Wed, Jul 26, 2017 at 3:16 PM, Icenowy Zheng <icenowy@aosc.io> wrote: > > > 于 2017年7月26日 GMT+08:00 下午3:08:06, Chen-Yu Tsai <wens@csie.org> 写到: >>On Sun, Jul 23, 2017 at 6:27 PM, Icenowy Zheng <icenowy@aosc.io> wrote: >>> From: Ondrej Jirman <megous@megous.com> >>> >>> Add SY8106A regulator to r_i2c bus and enable the r_i2c bus on >>> Orange Pi PC, then set the power supply of the ARM cores to this >>> regulator, in order to enable DVFS. >>> >>> Signed-off-by: Ondrej Jirman <megous@megous.com> >>> [Icenowy: Enable DVFS in this patch, slight changes and change commit >>> message] >>> Signed-off-by: Icenowy Zheng <icenowy@aosc.io> >>> --- >>> arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts | 19 +++++++++++++++++++ >>> 1 file changed, 19 insertions(+) >>> >>> diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts >>b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts >>> index 998b60f8d295..d855f8b6254e 100644 >>> --- a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts >>> +++ b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts >>> @@ -98,6 +98,10 @@ >>> status = "okay"; >>> }; >>> >>> +&cpu0 { >>> + cpu-supply = <®_sy8106a>; >>> +}; >>> + >>> &ehci0 { >>> status = "okay"; >>> }; >>> @@ -160,6 +164,21 @@ >>> }; >>> }; >>> >>> +&r_i2c { >>> + status = "okay"; >>> + >>> + reg_sy8106a: regulator@65 { >>> + compatible = "silergy,sy8106a"; >>> + reg = <0x65>; >>> + regulator-name = "vdd-cpux"; >>> + regulator-min-microvolt = <1000000>; >> >>According to the H3 datasheet, the minimum voltage is 1.1V, not 1V. > > But the Armbian OPP table for H3 contains several > OPP under 1.1V... Can you provide a link? If Armbian users have actually field tested this (a big if), then I would like to see some evidence of the SoC running stably at those OPPs with those lower voltages under full load. Even then you should still leave a note describing why we allow voltages below the recommended range. ChenYu > >> >>Otherwse >> >>> + regulator-max-microvolt = <1400000>; >>> + regulator-ramp-delay = <200>; >> >>Is this an actual constraint of the SoC? Or is it a characteristic >>of the regulator? If it is the latter, it belongs in the driver. >>AFAIK the regulator supports varying the ramp delay (slew rate). >> >>ChenYu >> >>> + regulator-boot-on; >>> + regulator-always-on; >>> + }; >>> +}; >>> + >>> &r_pio { >>> leds_r_opc: led_pins@0 { >>> pins = "PL10"; >>> -- >>> 2.13.0 >>>
在 2017-07-26 15:30，Chen-Yu Tsai 写道： > On Wed, Jul 26, 2017 at 3:16 PM, Icenowy Zheng <icenowy@aosc.io> wrote: >> >> >> 于 2017年7月26日 GMT+08:00 下午3:08:06, Chen-Yu Tsai <wens@csie.org> 写到: >>> On Sun, Jul 23, 2017 at 6:27 PM, Icenowy Zheng <icenowy@aosc.io> >>> wrote: >>>> From: Ondrej Jirman <megous@megous.com> >>>> >>>> Add SY8106A regulator to r_i2c bus and enable the r_i2c bus on >>>> Orange Pi PC, then set the power supply of the ARM cores to this >>>> regulator, in order to enable DVFS. >>>> >>>> Signed-off-by: Ondrej Jirman <megous@megous.com> >>>> [Icenowy: Enable DVFS in this patch, slight changes and change >>>> commit >>>> message] >>>> Signed-off-by: Icenowy Zheng <icenowy@aosc.io> >>>> --- >>>> arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts | 19 +++++++++++++++++++ >>>> 1 file changed, 19 insertions(+) >>>> >>>> diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts >>> b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts >>>> index 998b60f8d295..d855f8b6254e 100644 >>>> --- a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts >>>> +++ b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts >>>> @@ -98,6 +98,10 @@ >>>> status = "okay"; >>>> }; >>>> >>>> +&cpu0 { >>>> + cpu-supply = <®_sy8106a>; >>>> +}; >>>> + >>>> &ehci0 { >>>> status = "okay"; >>>> }; >>>> @@ -160,6 +164,21 @@ >>>> }; >>>> }; >>>> >>>> +&r_i2c { >>>> + status = "okay"; >>>> + >>>> + reg_sy8106a: regulator@65 { >>>> + compatible = "silergy,sy8106a"; >>>> + reg = <0x65>; >>>> + regulator-name = "vdd-cpux"; >>>> + regulator-min-microvolt = <1000000>; >>> >>> According to the H3 datasheet, the minimum voltage is 1.1V, not 1V. >> >> But the Armbian OPP table for H3 contains several >> OPP under 1.1V... > > Can you provide a link? > > If Armbian users have actually field tested this (a big if), > then I would like to see some evidence of the SoC running > stably at those OPPs with those lower voltages under full load. See [1]. [1] https://github.com/armbian/build/blob/master/config/fex/orangepipc.fex#L736 > > Even then you should still leave a note describing why we allow > voltages below the recommended range. > > ChenYu > >> >>> >>> Otherwse >>> >>>> + regulator-max-microvolt = <1400000>; >>>> + regulator-ramp-delay = <200>; >>> >>> Is this an actual constraint of the SoC? Or is it a characteristic >>> of the regulator? If it is the latter, it belongs in the driver. >>> AFAIK the regulator supports varying the ramp delay (slew rate). I don't know... Maybe I should ask Ondrej? >>> >>> ChenYu >>> >>>> + regulator-boot-on; >>>> + regulator-always-on; >>>> + }; >>>> +}; >>>> + >>>> &r_pio { >>>> leds_r_opc: led_pins@0 { >>>> pins = "PL10"; >>>> -- >>>> 2.13.0 >>>>
Hi, icenowy@aosc.io píše v St 26. 07. 2017 v 15:36 +0800: > > > > > > > > > Otherwse > > > > > > > > > + regulator-max-microvolt = <1400000>; > > > > > + regulator-ramp-delay = <200>; > > > > > > > > Is this an actual constraint of the SoC? Or is it a characteristic > > > > of the regulator? If it is the latter, it belongs in the driver. > > > > AFAIK the regulator supports varying the ramp delay (slew rate). > > I don't know... > > Maybe I should ask Ondrej? It is probably neither. It is used to calculate a delay inserted by the kernel between setting a new target voltage over I2C and changing the frequency of the CPU. The actual delay is calculated by the difference between previous and the new voltage. I don't remember seeing anything in the datasheet of the regulator. This is just some low value that works. It would probably be dependent on the capacitance on the output of the regulator, actual load (which varies), etc. So it is a board specific value. One could measure it with an oscilloscope if there's a need to optimize this. regards, o. > > > > > > > > ChenYu > > > > > > > > > + regulator-boot-on; > > > > > + regulator-always-on; > > > > > + }; > > > > > +}; > > > > > + > > > > > &r_pio { > > > > > leds_r_opc: led_pins@0 { > > > > > pins = "PL10"; > > > > > -- > > > > > 2.13.0 > > > > >
Hi, On Wed, Jul 26, 2017 at 12:23:48PM +0200, Ondřej Jirman wrote: > Hi, > > icenowy@aosc.io píše v St 26. 07. 2017 v 15:36 +0800: > > > > > > > > > > > > Otherwse > > > > > > > > > > > + regulator-max-microvolt = <1400000>; > > > > > > + regulator-ramp-delay = <200>; > > > > > > > > > > Is this an actual constraint of the SoC? Or is it a characteristic > > > > > of the regulator? If it is the latter, it belongs in the driver. > > > > > AFAIK the regulator supports varying the ramp delay (slew rate). > > > > I don't know... > > > > Maybe I should ask Ondrej? > > It is probably neither. > > It is used to calculate a delay inserted by the kernel between setting > a new target voltage over I2C and changing the frequency of the CPU. > The actual delay is calculated by the difference between previous and > the new voltage. > > I don't remember seeing anything in the datasheet of the regulator. > This is just some low value that works. > > It would probably be dependent on the capacitance on the output of the > regulator, actual load (which varies), etc. So it is a board specific > value. One could measure it with an oscilloscope if there's a need to > optimize this. If this is a reasonable default, then this should be in the driver. You can't expect anyone to properly calculate a ramp delay and have access to both a scope and the CPU power lines. Maxime
在 2017-07-26 19:44，Maxime Ripard 写道： > Hi, > > On Wed, Jul 26, 2017 at 12:23:48PM +0200, Ondřej Jirman wrote: >> Hi, >> >> icenowy@aosc.io píše v St 26. 07. 2017 v 15:36 +0800: >> > >> > > > > >> > > > > Otherwse >> > > > > >> > > > > > + regulator-max-microvolt = <1400000>; >> > > > > > + regulator-ramp-delay = <200>; >> > > > > >> > > > > Is this an actual constraint of the SoC? Or is it a characteristic >> > > > > of the regulator? If it is the latter, it belongs in the driver. >> > > > > AFAIK the regulator supports varying the ramp delay (slew rate). >> > >> > I don't know... >> > >> > Maybe I should ask Ondrej? >> >> It is probably neither. >> >> It is used to calculate a delay inserted by the kernel between setting >> a new target voltage over I2C and changing the frequency of the CPU. >> The actual delay is calculated by the difference between previous and >> the new voltage. >> >> I don't remember seeing anything in the datasheet of the regulator. >> This is just some low value that works. >> >> It would probably be dependent on the capacitance on the output of the >> regulator, actual load (which varies), etc. So it is a board specific >> value. One could measure it with an oscilloscope if there's a need to >> optimize this. > > If this is a reasonable default, then this should be in the > driver. You can't expect anyone to properly calculate a ramp delay and > have access to both a scope and the CPU power lines. It seems that in regulator_desc structure a default value of ramp delay can be set, and the ones specified in dt can override it. So just add .ramp_delay = 200 in the driver's regulator_desc part? Should a comment be added that explains it's only an experienced value on Allwinner H3/H5 boards VDD-CPUX usage? > > Maxime > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Maxime Ripard píše v St 26. 07. 2017 v 13:44 +0200: > Hi, > > On Wed, Jul 26, 2017 at 12:23:48PM +0200, Ondřej Jirman wrote: > > Hi, > > > > icenowy@aosc.io píše v St 26. 07. 2017 v 15:36 +0800: > > > > > > > > > > > > > > > Otherwse > > > > > > > > > > > > > + regulator-max-microvolt = <1400000>; > > > > > > > + regulator-ramp-delay = <200>; > > > > > > > > > > > > Is this an actual constraint of the SoC? Or is it a characteristic > > > > > > of the regulator? If it is the latter, it belongs in the driver. > > > > > > AFAIK the regulator supports varying the ramp delay (slew rate). > > > > > > I don't know... > > > > > > Maybe I should ask Ondrej? > > > > It is probably neither. > > > > It is used to calculate a delay inserted by the kernel between setting > > a new target voltage over I2C and changing the frequency of the CPU. > > The actual delay is calculated by the difference between previous and > > the new voltage. > > > > I don't remember seeing anything in the datasheet of the regulator. > > This is just some low value that works. > > > > It would probably be dependent on the capacitance on the output of the > > regulator, actual load (which varies), etc. So it is a board specific > > value. One could measure it with an oscilloscope if there's a need to > > optimize this. > > If this is a reasonable default, then this should be in the > driver. You can't expect anyone to properly calculate a ramp delay and > have access to both a scope and the CPU power lines. It translates to 1ms per 0.2V which is highly conservative. The real times will be in 1-10us range. So I guess this could be a default in the driver. regards, o. > Maxime > > -- > Maxime Ripard, Free Electrons > Embedded Linux and Kernel engineering > http://free-electrons.com >
diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts index 998b60f8d295..d855f8b6254e 100644 --- a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts +++ b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts @@ -98,6 +98,10 @@ status = "okay"; }; +&cpu0 { + cpu-supply = <®_sy8106a>; +}; + &ehci0 { status = "okay"; }; @@ -160,6 +164,21 @@ }; }; +&r_i2c { + status = "okay"; + + reg_sy8106a: regulator@65 { + compatible = "silergy,sy8106a"; + reg = <0x65>; + regulator-name = "vdd-cpux"; + regulator-min-microvolt = <1000000>; + regulator-max-microvolt = <1400000>; + regulator-ramp-delay = <200>; + regulator-boot-on; + regulator-always-on; + }; +}; + &r_pio { leds_r_opc: led_pins@0 { pins = "PL10";