mbox series

[PATCHv1,0/3] Odroid c2 missing regulator linking

Message ID 20190828202723.1145-1-linux.amoon@gmail.com (mailing list archive)
Headers show
Series Odroid c2 missing regulator linking | expand

Message

Anand Moon Aug. 28, 2019, 8:27 p.m. UTC
Below small changes help re-configure or fix missing inter linking
of regulator node.

Changes based top on my prevoius series.

[0] https://patchwork.kernel.org/cover/11113091/

TOOD: Add support for DVFS GXBB odroid board in next series.

Best Regards
-Anand

Anand Moon (3):
  arm64: dts: meson: odroid-c2: Add missing regulator linked to P5V0
    regulator
  arm64: dts: meson: odroid-c2: Add missing regulator linked to
    VDDIO_AO3V3 regulator
  arm64: dts: meson: odroid-c2: Add missing regulator linked to HDMI
    supply

 .../boot/dts/amlogic/meson-gxbb-odroidc2.dts  | 44 ++++++++++++++++---
 1 file changed, 38 insertions(+), 6 deletions(-)

Comments

Neil Armstrong Aug. 29, 2019, 8:28 a.m. UTC | #1
On 28/08/2019 22:27, Anand Moon wrote:
> Below small changes help re-configure or fix missing inter linking
> of regulator node.
> 
> Changes based top on my prevoius series.

For the serie:
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>

> 
> [0] https://patchwork.kernel.org/cover/11113091/
> 
> TOOD: Add support for DVFS GXBB odroid board in next series.

I'm curious how you will do this !

> 
> Best Regards
> -Anand
> 
> Anand Moon (3):
>   arm64: dts: meson: odroid-c2: Add missing regulator linked to P5V0
>     regulator
>   arm64: dts: meson: odroid-c2: Add missing regulator linked to
>     VDDIO_AO3V3 regulator
>   arm64: dts: meson: odroid-c2: Add missing regulator linked to HDMI
>     supply
> 
>  .../boot/dts/amlogic/meson-gxbb-odroidc2.dts  | 44 ++++++++++++++++---
>  1 file changed, 38 insertions(+), 6 deletions(-)
>
Anand Moon Aug. 29, 2019, 6:35 p.m. UTC | #2
Hi Neil,

On Thu, 29 Aug 2019 at 13:58, Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> On 28/08/2019 22:27, Anand Moon wrote:
> > Below small changes help re-configure or fix missing inter linking
> > of regulator node.
> >
> > Changes based top on my prevoius series.
>
> For the serie:
> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
>

Thanks for your review.

> >
> > [0] https://patchwork.kernel.org/cover/11113091/
> >
> > TOOD: Add support for DVFS GXBB odroid board in next series.
>
> I'm curious how you will do this !

I was just studying you previous series on how you have implemented
this feature for C1, N2 and VIM3 boards.

[0] https://patchwork.kernel.org/cover/11114125/

I started gathering key inputs needed for this ie *clk / pwm*
like VDDCPU and VDDE clk changes.

But it looks like of the complex clk framework needed, so I leave this to the
expert like your team of developers to do this much quick and efficiently.

Best Regards,
-Anand
Neil Armstrong Aug. 30, 2019, 7:31 a.m. UTC | #3
On 29/08/2019 20:35, Anand Moon wrote:
> Hi Neil,
> 
> On Thu, 29 Aug 2019 at 13:58, Neil Armstrong <narmstrong@baylibre.com> wrote:
>>
>> On 28/08/2019 22:27, Anand Moon wrote:
>>> Below small changes help re-configure or fix missing inter linking
>>> of regulator node.
>>>
>>> Changes based top on my prevoius series.
>>
>> For the serie:
>> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
>>
> 
> Thanks for your review.
> 
>>>
>>> [0] https://patchwork.kernel.org/cover/11113091/
>>>
>>> TOOD: Add support for DVFS GXBB odroid board in next series.
>>
>> I'm curious how you will do this !
> 
> I was just studying you previous series on how you have implemented
> this feature for C1, N2 and VIM3 boards.
> 
> [0] https://patchwork.kernel.org/cover/11114125/
> 
> I started gathering key inputs needed for this ie *clk / pwm*
> like VDDCPU and VDDE clk changes.
> 
> But it looks like of the complex clk framework needed, so I leave this to the
> expert like your team of developers to do this much quick and efficiently.

On GXBB, GXL, GXM and AXG SoCs, CPU Frequency setting and PWM Regulator setup is
done by the SCPI Co-processor via the SCPI protocol.

Thus, we should not handle it from Linux, and even if we could, we don't have the
registers documentation of the CPU clusters clock tree.

SCPI works fine on all tested devices, except Odroid-C2, because Hardkernel left
the > 1.5GHz freq in the initial SCPI tables loaded by the BL2, i.e. packed with U-Boot.
Nowadays they have removed the bad frequencies, but still some devices uses the old
bootloader.

But in the SCPI case we trust the table returned by the firmware and use it as-in,
and there is no (simple ?) way to override the table and set a max frequency.

This is why we disabled SCPI.

See https://patchwork.kernel.org/patch/9500175/

Neil

> 
> Best Regards,
> -Anand
>
Anand Moon Aug. 30, 2019, 9:34 a.m. UTC | #4
Hi Neil,

On Fri, 30 Aug 2019 at 13:01, Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> On 29/08/2019 20:35, Anand Moon wrote:
> > Hi Neil,
> >
> > On Thu, 29 Aug 2019 at 13:58, Neil Armstrong <narmstrong@baylibre.com> wrote:
> >>
> >> On 28/08/2019 22:27, Anand Moon wrote:
> >>> Below small changes help re-configure or fix missing inter linking
> >>> of regulator node.
> >>>
> >>> Changes based top on my prevoius series.
> >>
> >> For the serie:
> >> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
> >>
> >
> > Thanks for your review.
> >
> >>>
> >>> [0] https://patchwork.kernel.org/cover/11113091/
> >>>
> >>> TOOD: Add support for DVFS GXBB odroid board in next series.
> >>
> >> I'm curious how you will do this !
> >
> > I was just studying you previous series on how you have implemented
> > this feature for C1, N2 and VIM3 boards.
> >
> > [0] https://patchwork.kernel.org/cover/11114125/
> >
> > I started gathering key inputs needed for this ie *clk / pwm*
> > like VDDCPU and VDDE clk changes.
> >
> > But it looks like of the complex clk framework needed, so I leave this to the
> > expert like your team of developers to do this much quick and efficiently.
>
> On GXBB, GXL, GXM and AXG SoCs, CPU Frequency setting and PWM Regulator setup is
> done by the SCPI Co-processor via the SCPI protocol.
>
> Thus, we should not handle it from Linux, and even if we could, we don't have the
> registers documentation of the CPU clusters clock tree.
>

Ok thanks.

> SCPI works fine on all tested devices, except Odroid-C2, because Hardkernel left
> the > 1.5GHz freq in the initial SCPI tables loaded by the BL2, i.e. packed with U-Boot.
> Nowadays they have removed the bad frequencies, but still some devices uses the old
> bootloader.
>
> But in the SCPI case we trust the table returned by the firmware and use it as-in,
> and there is no (simple ?) way to override the table and set a max frequency.
>
> This is why we disabled SCPI.
>
> See https://patchwork.kernel.org/patch/9500175/

I have quickly enable this on my board and here the cpufreq info

[alarm@alarm ~]$  cpupower frequency-info
analyzing CPU 0:
  driver: scpi-cpufreq
  CPUs which run at the same hardware frequency: 0 1 2 3
  CPUs which need to have their frequency coordinated by software: 0 1 2 3
  maximum transition latency: 200 us
  hardware limits: 100.0 MHz - 1.54 GHz
  available frequency steps:  100.0 MHz, 250 MHz, 500 MHz, 1000 MHz,
1.30 GHz, 1.54 GHz
  available cpufreq governors: conservative ondemand userspace
powersave performance schedutil
  current policy: frequency should be within 100.0 MHz and 1.54 GHz.
                  The governor "ondemand" may decide which speed to use
                  within this range.
  current CPU frequency: Unable to call hardware
  current CPU frequency: 250 MHz (asserted by call to kernel)

I did some simple stress testing and observed the freq scaling is
working fine when cpufreq governor is set to ondemand.

Powertop output.
            Package |            CPU 0
 100 MHz     5.2%   |  100 MHz     1.6%
 250 MHz     4.4%   |  250 MHz     4.3%
 500 MHz     2.6%   |  500 MHz     2.4%
1000 MHz     0.5%   | 1000 MHz     0.3%
1296 MHz     0.2%   | 1296 MHz     0.1%
1.54 GHz     0.2%   | 1.54 GHz     0.1%
Idle        86.9%   | Idle        91.2%

Here the output on the linaro's pm-qa testing for cpufreq.

[1] https://pastebin.com/h880WATn
Almost all the test case pass with this one as off now.

Best Regards
-Anand
Neil Armstrong Aug. 30, 2019, 9:50 a.m. UTC | #5
On 30/08/2019 11:34, Anand Moon wrote:
> Hi Neil,
> 
> On Fri, 30 Aug 2019 at 13:01, Neil Armstrong <narmstrong@baylibre.com> wrote:
>>
>> On 29/08/2019 20:35, Anand Moon wrote:
>>> Hi Neil,
>>>
>>> On Thu, 29 Aug 2019 at 13:58, Neil Armstrong <narmstrong@baylibre.com> wrote:
>>>>
>>>> On 28/08/2019 22:27, Anand Moon wrote:
>>>>> Below small changes help re-configure or fix missing inter linking
>>>>> of regulator node.
>>>>>
>>>>> Changes based top on my prevoius series.
>>>>
>>>> For the serie:
>>>> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
>>>>
>>>
>>> Thanks for your review.
>>>
>>>>>
>>>>> [0] https://patchwork.kernel.org/cover/11113091/
>>>>>
>>>>> TOOD: Add support for DVFS GXBB odroid board in next series.
>>>>
>>>> I'm curious how you will do this !
>>>
>>> I was just studying you previous series on how you have implemented
>>> this feature for C1, N2 and VIM3 boards.
>>>
>>> [0] https://patchwork.kernel.org/cover/11114125/
>>>
>>> I started gathering key inputs needed for this ie *clk / pwm*
>>> like VDDCPU and VDDE clk changes.
>>>
>>> But it looks like of the complex clk framework needed, so I leave this to the
>>> expert like your team of developers to do this much quick and efficiently.
>>
>> On GXBB, GXL, GXM and AXG SoCs, CPU Frequency setting and PWM Regulator setup is
>> done by the SCPI Co-processor via the SCPI protocol.
>>
>> Thus, we should not handle it from Linux, and even if we could, we don't have the
>> registers documentation of the CPU clusters clock tree.
>>
> 
> Ok thanks.
> 
>> SCPI works fine on all tested devices, except Odroid-C2, because Hardkernel left
>> the > 1.5GHz freq in the initial SCPI tables loaded by the BL2, i.e. packed with U-Boot.
>> Nowadays they have removed the bad frequencies, but still some devices uses the old
>> bootloader.
>>
>> But in the SCPI case we trust the table returned by the firmware and use it as-in,
>> and there is no (simple ?) way to override the table and set a max frequency.
>>
>> This is why we disabled SCPI.
>>
>> See https://patchwork.kernel.org/patch/9500175/
> 
> I have quickly enable this on my board and here the cpufreq info
> 
> [alarm@alarm ~]$  cpupower frequency-info
> analyzing CPU 0:
>   driver: scpi-cpufreq
>   CPUs which run at the same hardware frequency: 0 1 2 3
>   CPUs which need to have their frequency coordinated by software: 0 1 2 3
>   maximum transition latency: 200 us
>   hardware limits: 100.0 MHz - 1.54 GHz
>   available frequency steps:  100.0 MHz, 250 MHz, 500 MHz, 1000 MHz,
> 1.30 GHz, 1.54 GHz
>   available cpufreq governors: conservative ondemand userspace
> powersave performance schedutil
>   current policy: frequency should be within 100.0 MHz and 1.54 GHz.
>                   The governor "ondemand" may decide which speed to use
>                   within this range.
>   current CPU frequency: Unable to call hardware
>   current CPU frequency: 250 MHz (asserted by call to kernel)
> 
> I did some simple stress testing and observed the freq scaling is
> working fine when cpufreq governor is set to ondemand.
> 
> Powertop output.
>             Package |            CPU 0
>  100 MHz     5.2%   |  100 MHz     1.6%
>  250 MHz     4.4%   |  250 MHz     4.3%
>  500 MHz     2.6%   |  500 MHz     2.4%
> 1000 MHz     0.5%   | 1000 MHz     0.3%
> 1296 MHz     0.2%   | 1296 MHz     0.1%
> 1.54 GHz     0.2%   | 1.54 GHz     0.1%
> Idle        86.9%   | Idle        91.2%
> 
> Here the output on the linaro's pm-qa testing for cpufreq.
> 
> [1] https://pastebin.com/h880WATn
> Almost all the test case pass with this one as off now.

Thanks for passing the tests, no doubt it works with a recent
bootloader binary, but we can't leave alone the first Odroid-C2
devices loaded with an incorrect SCPI table.

I'll let Kevin decide for the following.

Neil

> 
> Best Regards
> -Anand
>
Martin Blumenstingl Sept. 3, 2019, 7:07 p.m. UTC | #6
Hi Anand,

On Fri, Aug 30, 2019 at 11:34 AM Anand Moon <linux.amoon@gmail.com> wrote:
[...]
> > SCPI works fine on all tested devices, except Odroid-C2, because Hardkernel left
> > the > 1.5GHz freq in the initial SCPI tables loaded by the BL2, i.e. packed with U-Boot.
> > Nowadays they have removed the bad frequencies, but still some devices uses the old
> > bootloader.
> >
> > But in the SCPI case we trust the table returned by the firmware and use it as-in,
> > and there is no (simple ?) way to override the table and set a max frequency.
> >
> > This is why we disabled SCPI.
> >
> > See https://patchwork.kernel.org/patch/9500175/
>
> I have quickly enable this on my board and here the cpufreq info
[...]
> Almost all the test case pass with this one as off now.
I suggest to send an RFC patch to (re-)enable DVFS on Odroid-C2
I find it easy to miss a DVFS discussion inside a "missing regulator" series

with a separate patch you can also get feedback from other Odroid-C2
owners who can help testing
coincidence or not: on Friday someone asked in the #linux-amlogic IRC
channel why Odroid-C2 didn't have DVFS enabled and what to do about it


Martin