[PATCH v3 0/3] ARM: da850: enable cpufreq in DT mode

[PATCH v3 0/3] ARM: da850: enable cpufreq in DT mode

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

This series adds cpufreq-dt operating points for da850 boards supported
with device tree (da850-lcdk, da850-lego-ev3, da850-evm).

Last patch enables CPUFREQ_DT in davinci_all_defconfig.

v1 -> v2:
- use the VDCDC3_1.2V regulator as cpu-supply on da850-evm

v2 -> v3:
- drop patch 1, as the revision tag is in fact correctly passed to the kernel
  by u-boot
- only enable the 375 operating point for da850-evm as this is the standard
  frequency for this board

Bartosz Golaszewski (1):
  ARM: dts: da850-evm: enable cpufreq

David Lechner (2):
  ARM: dts: da850: add cpu node and operating points to DT
  ARM: davinci_all_defconfig: Enable CPUFREQ_DT

 arch/arm/boot/dts/da850-evm.dts        | 13 ++++++
 arch/arm/boot/dts/da850-lcdk.dts       | 31 ++++++++++++++
 arch/arm/boot/dts/da850-lego-ev3.dts   | 30 ++++++++++++++
 arch/arm/boot/dts/da850.dtsi           | 56 ++++++++++++++++++++++++++
 arch/arm/configs/davinci_all_defconfig |  1 +
 5 files changed, 131 insertions(+)

-- 
2.21.0

[PATCH v3 1/3] ARM: dts: da850: add cpu node and operating points to DT

[Bartosz Golaszewski]

From: David Lechner <david@lechnology.com>

This adds a cpu node and operating points to the common da850.dtsi file.

Additionally, a regulator is added to the LEGO EV3 board along with
some board-specific CPU configuration.

Regulators need to be hooked up on other boards to get them working.

Signed-off-by: David Lechner <david@lechnology.com>
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 arch/arm/boot/dts/da850-lcdk.dts     | 31 +++++++++++++++
 arch/arm/boot/dts/da850-lego-ev3.dts | 30 +++++++++++++++
 arch/arm/boot/dts/da850.dtsi         | 56 ++++++++++++++++++++++++++++
 3 files changed, 117 insertions(+)

diff --git a/arch/arm/boot/dts/da850-lcdk.dts b/arch/arm/boot/dts/da850-lcdk.dts
index 26f453dc8370..f29ed9010812 100644
--- a/arch/arm/boot/dts/da850-lcdk.dts
+++ b/arch/arm/boot/dts/da850-lcdk.dts
@@ -155,12 +155,43 @@
 			};
 		};
 	};
+
+	cvdd: regulator0 {
+		compatible = "regulator-fixed";
+		regulator-name = "cvdd";
+		regulator-min-microvolt = <1300000>;
+		regulator-max-microvolt = <1300000>;
+		regulator-always-on;
+		regulator-boot-on;
+	};
 };
 
 &ref_clk {
 	clock-frequency = <24000000>;
 };
 
+&cpu {
+	cpu-supply = <&cvdd>;
+};
+
+/* LCDK has a fixed CVDD of 1.3V, so only op points >= 300MHz are valid */
+
+&opp_100 {
+	status = "disabled";
+};
+
+&opp_200 {
+	status = "disabled";
+};
+
+&opp_375 {
+	status = "okay";
+};
+
+&opp_456 {
+	status = "okay";
+};
+
 &pmx_core {
 	status = "okay";
 
diff --git a/arch/arm/boot/dts/da850-lego-ev3.dts b/arch/arm/boot/dts/da850-lego-ev3.dts
index 66fcadf0ba91..553717f84483 100644
--- a/arch/arm/boot/dts/da850-lego-ev3.dts
+++ b/arch/arm/boot/dts/da850-lego-ev3.dts
@@ -125,6 +125,15 @@
 		amp-supply = <&amp>;
 	};
 
+	cvdd: regulator0 {
+		compatible = "regulator-fixed";
+		regulator-name = "cvdd";
+		regulator-min-microvolt = <1200000>;
+		regulator-max-microvolt = <1200000>;
+		regulator-always-on;
+		regulator-boot-on;
+	};
+
 	/*
 	 * This is a 5V current limiting regulator that is shared by USB,
 	 * the sensor (input) ports, the motor (output) ports and the A/DC.
@@ -204,6 +213,27 @@
 	clock-frequency = <24000000>;
 };
 
+&cpu {
+	cpu-supply = <&cvdd>;
+};
+
+/* since we have a fixed regulator, we can't run at these points */
+&opp_100 {
+	status = "disabled";
+};
+
+&opp_200 {
+	status = "disabled";
+};
+
+/*
+ * The SoC is actually the 456MHz version, but because of the fixed regulator
+ * This is the fastest we can go.
+ */
+&opp_375 {
+	status = "okay";
+};
+
 &pmx_core {
 	status = "okay";
 
diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index 559659b399d0..ee61d1253b58 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -20,6 +20,62 @@
 		reg = <0xc0000000 0x0>;
 	};
 
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		cpu: cpu@0 {
+			compatible = "arm,arm926ej-s";
+			device_type = "cpu";
+			reg = <0>;
+			clocks = <&psc0 14>;
+			operating-points-v2 = <&opp_table>;
+		};
+	};
+
+	opp_table: opp-table {
+		compatible = "operating-points-v2";
+
+		opp_100: opp100-100000000 {
+			opp-hz = /bits/ 64 <100000000>;
+			opp-microvolt = <1000000 950000 1050000>;
+		};
+
+		opp_200: opp110-200000000 {
+			opp-hz = /bits/ 64 <200000000>;
+			opp-microvolt = <1100000 1050000 1160000>;
+		};
+
+		opp_300: opp120-300000000 {
+			opp-hz = /bits/ 64 <300000000>;
+			opp-microvolt = <1200000 1140000 1320000>;
+		};
+
+		/*
+		 * Original silicon was 300MHz max, so higher frequencies
+		 * need to be enabled on a per-board basis if the chip is
+		 * capable.
+		 */
+
+		opp_375: opp120-375000000 {
+			status = "disabled";
+			opp-hz = /bits/ 64 <375000000>;
+			opp-microvolt = <1200000 1140000 1320000>;
+		};
+
+		opp_415: opp130-415000000 {
+			status = "disabled";
+			opp-hz = /bits/ 64 <415000000>;
+			opp-microvolt = <1300000 1250000 1350000>;
+		};
+
+		opp_456: opp130-456000000 {
+			status = "disabled";
+			opp-hz = /bits/ 64 <456000000>;
+			opp-microvolt = <1300000 1250000 1350000>;
+		};
+	};
+
 	arm {
 		#address-cells = <1>;
 		#size-cells = <1>;
-- 
2.21.0

[PATCH v3 2/3] ARM: dts: da850-evm: enable cpufreq

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Enable cpufreq-dt support for da850-evm. The cvdd is supplied by the
tps6507 pmic with configurable output voltage, so all operating points
can be enabled.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 arch/arm/boot/dts/da850-evm.dts | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts
index f04bc3e15332..f94bb38fdad9 100644
--- a/arch/arm/boot/dts/da850-evm.dts
+++ b/arch/arm/boot/dts/da850-evm.dts
@@ -191,6 +191,19 @@
 	};
 };
 
+&cpu {
+	cpu-supply = <&vdcdc3_reg>;
+};
+
+/*
+ * The standard da850-evm kits and SOM's are 375MHz so enable this operating
+ * point by default. Higher frequencies must be enabled for custom boards with
+ * other variants of the SoC.
+ */
+&opp_375 {
+	status = "okay";
+};
+
 &sata {
 	status = "okay";
 };
-- 
2.21.0

[PATCH v3 3/3] ARM: davinci_all_defconfig: Enable CPUFREQ_DT

[Bartosz Golaszewski]

From: David Lechner <david@lechnology.com>

This sets CONFIG_CPUFREQ_DT=m in davinci_all_defconfig. This is used for
frequency scaling on device tree boards.

Signed-off-by: David Lechner <david@lechnology.com>
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 arch/arm/configs/davinci_all_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/davinci_all_defconfig b/arch/arm/configs/davinci_all_defconfig
index 207962a656a2..c3502236132e 100644
--- a/arch/arm/configs/davinci_all_defconfig
+++ b/arch/arm/configs/davinci_all_defconfig
@@ -45,6 +45,7 @@ CONFIG_CPU_FREQ_DEFAULT_GOV_USERSPACE=y
 CONFIG_CPU_FREQ_GOV_PERFORMANCE=m
 CONFIG_CPU_FREQ_GOV_POWERSAVE=m
 CONFIG_CPU_FREQ_GOV_ONDEMAND=m
+CONFIG_CPUFREQ_DT=m
 CONFIG_CPU_IDLE=y
 CONFIG_NET=y
 CONFIG_PACKET=y
-- 
2.21.0

Re: [PATCH v3 1/3] ARM: dts: da850: add cpu node and operating points to DT

[David Lechner]

On 4/8/19 2:59 AM, Bartosz Golaszewski wrote:
> From: David Lechner <david@lechnology.com>
> 
> This adds a cpu node and operating points to the common da850.dtsi file.
> 
> Additionally, a regulator is added to the LEGO EV3 board along with
> some board-specific CPU configuration.
> 
> Regulators need to be hooked up on other boards to get them working.
> 

I still have the same comments that I made on v1. It would be nice to
also mention the LCDK board in the commit message since it is included
in this patch.

Also, the 415MHz operating point is something I just made up for testing
and I'm not sure that it is actually useful in general. It could be
omitted from this patch.

Re: [PATCH v3 1/3] ARM: dts: da850: add cpu node and operating points to DT

[Bartosz Golaszewski]

pon., 8 kwi 2019 o 15:47 David Lechner <david@lechnology.com> napisał(a):
>
> On 4/8/19 2:59 AM, Bartosz Golaszewski wrote:
> > From: David Lechner <david@lechnology.com>
> >
> > This adds a cpu node and operating points to the common da850.dtsi file.
> >
> > Additionally, a regulator is added to the LEGO EV3 board along with
> > some board-specific CPU configuration.
> >
> > Regulators need to be hooked up on other boards to get them working.
> >
>
> I still have the same comments that I made on v1. It would be nice to
> also mention the LCDK board in the commit message since it is included
> in this patch.
>

My bad, I forgot it.

> Also, the 415MHz operating point is something I just made up for testing
> and I'm not sure that it is actually useful in general. It could be
> omitted from this patch.
>

The board file has the 408 opp, I guess this is in line with other
operating points defined by you which have slightly higher frequencies
than those in the da850.c. Let's see what Sekhar thinks.

Bart

Re: [PATCH v3 1/3] ARM: dts: da850: add cpu node and operating points to DT

[David Lechner]

On 4/8/19 8:51 AM, Bartosz Golaszewski wrote:
> pon., 8 kwi 2019 o 15:47 David Lechner <david@lechnology.com> napisał(a):
>>
>> On 4/8/19 2:59 AM, Bartosz Golaszewski wrote:
>>> From: David Lechner <david@lechnology.com>
>>>
>>> This adds a cpu node and operating points to the common da850.dtsi file.
>>>
>>> Additionally, a regulator is added to the LEGO EV3 board along with
>>> some board-specific CPU configuration.
>>>
>>> Regulators need to be hooked up on other boards to get them working.
>>>
>>
>> I still have the same comments that I made on v1. It would be nice to
>> also mention the LCDK board in the commit message since it is included
>> in this patch.
>>
> 
> My bad, I forgot it.
> 
>> Also, the 415MHz operating point is something I just made up for testing
>> and I'm not sure that it is actually useful in general. It could be
>> omitted from this patch.
>>
> 
> The board file has the 408 opp, I guess this is in line with other
> operating points defined by you which have slightly higher frequencies
> than those in the da850.c. Let's see what Sekhar thinks.
> 
> Bart
> 

My bad this time. I guess I put more thought into it that I remembered.
:-)

Re: [PATCH v3 1/3] ARM: dts: da850: add cpu node and operating points to DT

[Sekhar Nori]

Hi Bartosz,

On 08/04/19 1:29 PM, Bartosz Golaszewski wrote:
> From: David Lechner <david@lechnology.com>
> 
> This adds a cpu node and operating points to the common da850.dtsi file.
> 
> Additionally, a regulator is added to the LEGO EV3 board along with
> some board-specific CPU configuration.
> 
> Regulators need to be hooked up on other boards to get them working.
> 
> Signed-off-by: David Lechner <david@lechnology.com>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>

I remember you mentioning about some problems using OCHI and cpufreq
together. Are those resolved now? CPU PLL on DA850 can affect other
peripheral clock frequencies too. So enabling it should really be a
per-board decision.

No problems with adding OPPs to da850.dtsi, but which of those are
enabled on any board should be after some thorough testing and analysis.

Because of that, I think its also better to split da850.dtsi from board
specific changes in this patch.

> +	opp_table: opp-table {
> +		compatible = "operating-points-v2";
> +
> +		opp_100: opp100-100000000 {
> +			opp-hz = /bits/ 64 <100000000>;
> +			opp-microvolt = <1000000 950000 1050000>;
> +		};
> +
> +		opp_200: opp110-200000000 {
> +			opp-hz = /bits/ 64 <200000000>;
> +			opp-microvolt = <1100000 1050000 1160000>;
> +		};
> +
> +		opp_300: opp120-300000000 {
> +			opp-hz = /bits/ 64 <300000000>;
> +			opp-microvolt = <1200000 1140000 1320000>;
> +		};
> +
> +		/*
> +		 * Original silicon was 300MHz max, so higher frequencies
> +		 * need to be enabled on a per-board basis if the chip is
> +		 * capable.
> +		 */
> +
> +		opp_375: opp120-375000000 {
> +			status = "disabled";
> +			opp-hz = /bits/ 64 <375000000>;
> +			opp-microvolt = <1200000 1140000 1320000>;
> +		};
> +
> +		opp_415: opp130-415000000 {
> +			status = "disabled";
> +			opp-hz = /bits/ 64 <415000000>;
> +			opp-microvolt = <1300000 1250000 1350000>;
> +		};
> +
> +		opp_456: opp130-456000000 {
> +			status = "disabled";
> +			opp-hz = /bits/ 64 <456000000>;
> +			opp-microvolt = <1300000 1250000 1350000>;
> +		};

Here, lets stick to OPPs defined in OMAP-L138 data manual (irrespective
of what existing board code has). Page 93 of
http://www.ti.com/lit/ds/symlink/omap-l138.pdf

Thanks,
Sekhar

Re: [PATCH v3 1/3] ARM: dts: da850: add cpu node and operating points to DT

[Bartosz Golaszewski]

pt., 12 kwi 2019 o 13:26 Sekhar Nori <nsekhar@ti.com> napisał(a):
>
> Hi Bartosz,
>
> On 08/04/19 1:29 PM, Bartosz Golaszewski wrote:
> > From: David Lechner <david@lechnology.com>
> >
> > This adds a cpu node and operating points to the common da850.dtsi file.
> >
> > Additionally, a regulator is added to the LEGO EV3 board along with
> > some board-specific CPU configuration.
> >
> > Regulators need to be hooked up on other boards to get them working.
> >
> > Signed-off-by: David Lechner <david@lechnology.com>
> > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> I remember you mentioning about some problems using OCHI and cpufreq
> together. Are those resolved now? CPU PLL on DA850 can affect other
> peripheral clock frequencies too. So enabling it should really be a
> per-board decision.
>

The problems are still there. I've never been able to find the
culprit, but it also occurs on TI BSP in the same way (a couple
cpufreq transitions will make the controller unresponsive).

> No problems with adding OPPs to da850.dtsi, but which of those are
> enabled on any board should be after some thorough testing and analysis.
>
> Because of that, I think its also better to split da850.dtsi from board
> specific changes in this patch.
>

Sure, I'll split it.

> > +     opp_table: opp-table {
> > +             compatible = "operating-points-v2";
> > +
> > +             opp_100: opp100-100000000 {
> > +                     opp-hz = /bits/ 64 <100000000>;
> > +                     opp-microvolt = <1000000 950000 1050000>;
> > +             };
> > +
> > +             opp_200: opp110-200000000 {
> > +                     opp-hz = /bits/ 64 <200000000>;
> > +                     opp-microvolt = <1100000 1050000 1160000>;
> > +             };
> > +
> > +             opp_300: opp120-300000000 {
> > +                     opp-hz = /bits/ 64 <300000000>;
> > +                     opp-microvolt = <1200000 1140000 1320000>;
> > +             };
> > +
> > +             /*
> > +              * Original silicon was 300MHz max, so higher frequencies
> > +              * need to be enabled on a per-board basis if the chip is
> > +              * capable.
> > +              */
> > +
> > +             opp_375: opp120-375000000 {
> > +                     status = "disabled";
> > +                     opp-hz = /bits/ 64 <375000000>;
> > +                     opp-microvolt = <1200000 1140000 1320000>;
> > +             };
> > +
> > +             opp_415: opp130-415000000 {
> > +                     status = "disabled";
> > +                     opp-hz = /bits/ 64 <415000000>;
> > +                     opp-microvolt = <1300000 1250000 1350000>;
> > +             };
> > +
> > +             opp_456: opp130-456000000 {
> > +                     status = "disabled";
> > +                     opp-hz = /bits/ 64 <456000000>;
> > +                     opp-microvolt = <1300000 1250000 1350000>;
> > +             };
>
> Here, lets stick to OPPs defined in OMAP-L138 data manual (irrespective
> of what existing board code has). Page 93 of
> http://www.ti.com/lit/ds/symlink/omap-l138.pdf
>

Will do in the next version.

Bart

> Thanks,
> Sekhar

Re: [PATCH v3 2/3] ARM: dts: da850-evm: enable cpufreq

[Sekhar Nori]

On 08/04/19 1:29 PM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> Enable cpufreq-dt support for da850-evm. The cvdd is supplied by the
> tps6507 pmic with configurable output voltage, so all operating points
> can be enabled.
> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Adam, can you please ack? Looks good to me.

Thanks,
Sekhar

> ---
>  arch/arm/boot/dts/da850-evm.dts | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts
> index f04bc3e15332..f94bb38fdad9 100644
> --- a/arch/arm/boot/dts/da850-evm.dts
> +++ b/arch/arm/boot/dts/da850-evm.dts
> @@ -191,6 +191,19 @@
>  	};
>  };
>  
> +&cpu {
> +	cpu-supply = <&vdcdc3_reg>;
> +};
> +
> +/*
> + * The standard da850-evm kits and SOM's are 375MHz so enable this operating
> + * point by default. Higher frequencies must be enabled for custom boards with
> + * other variants of the SoC.
> + */
> +&opp_375 {
> +	status = "okay";
> +};
> +
>  &sata {
>  	status = "okay";
>  };
> -- 
> 2.21.0
> 

Re: [PATCH v3 2/3] ARM: dts: da850-evm: enable cpufreq

[Bartosz Golaszewski]

pt., 12 kwi 2019 o 15:06 Sekhar Nori <nsekhar@ti.com> napisał(a):
>
> On 08/04/19 1:29 PM, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >
> > Enable cpufreq-dt support for da850-evm. The cvdd is supplied by the
> > tps6507 pmic with configurable output voltage, so all operating points
> > can be enabled.
> >
> > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> Adam, can you please ack? Looks good to me.
>

Actually I just noticed the commit message is no longer in line with
the changeset. I'll fix it in v4 shortly.

Bart

> Thanks,
> Sekhar
>
> > ---
> >  arch/arm/boot/dts/da850-evm.dts | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts
> > index f04bc3e15332..f94bb38fdad9 100644
> > --- a/arch/arm/boot/dts/da850-evm.dts
> > +++ b/arch/arm/boot/dts/da850-evm.dts
> > @@ -191,6 +191,19 @@
> >       };
> >  };
> >
> > +&cpu {
> > +     cpu-supply = <&vdcdc3_reg>;
> > +};
> > +
> > +/*
> > + * The standard da850-evm kits and SOM's are 375MHz so enable this operating
> > + * point by default. Higher frequencies must be enabled for custom boards with
> > + * other variants of the SoC.
> > + */
> > +&opp_375 {
> > +     status = "okay";
> > +};
> > +
> >  &sata {
> >       status = "okay";
> >  };
> > --
> > 2.21.0
> >
>

Re: [PATCH v3 1/3] ARM: dts: da850: add cpu node and operating points to DT

[Sekhar Nori]

On 12/04/19 5:41 PM, Bartosz Golaszewski wrote:
> pt., 12 kwi 2019 o 13:26 Sekhar Nori <nsekhar@ti.com> napisał(a):
>>
>> Hi Bartosz,
>>
>> On 08/04/19 1:29 PM, Bartosz Golaszewski wrote:
>>> From: David Lechner <david@lechnology.com>
>>>
>>> This adds a cpu node and operating points to the common da850.dtsi file.
>>>
>>> Additionally, a regulator is added to the LEGO EV3 board along with
>>> some board-specific CPU configuration.
>>>
>>> Regulators need to be hooked up on other boards to get them working.
>>>
>>> Signed-off-by: David Lechner <david@lechnology.com>
>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>
>> I remember you mentioning about some problems using OCHI and cpufreq
>> together. Are those resolved now? CPU PLL on DA850 can affect other
>> peripheral clock frequencies too. So enabling it should really be a
>> per-board decision.
>>
> 
> The problems are still there. I've never been able to find the
> culprit, but it also occurs on TI BSP in the same way (a couple
> cpufreq transitions will make the controller unresponsive).

Is that on LCDK as well? As I recall cpufreq was never enabled on LCDK
in TI BSP.

If the OHCI problem is present on LCDK, then there is a user visible
regression on mainline after this patch. Lets enable cpufreq in LCDK
only if all working peripherals keep working afterwards.

Thanks,
Sekhar

Re: [PATCH v3 2/3] ARM: dts: da850-evm: enable cpufreq

[Adam Ford]

On Fri, Apr 12, 2019 at 8:06 AM Sekhar Nori <nsekhar@ti.com> wrote:
>
> On 08/04/19 1:29 PM, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >
> > Enable cpufreq-dt support for da850-evm. The cvdd is supplied by the
> > tps6507 pmic with configurable output voltage, so all operating points
> > can be enabled.
> >
> > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> Adam, can you please ack? Looks good to me.

Sekhar,
Based on what I read in the thread about the LCDK, I want to do some
tests on the da850-evm before put my name on it.  I like what I see in
general, so I am excited to this this here.

Bartosz - which kernel branch should I use as a starting point to apply these?

adam
>
> Thanks,
> Sekhar
>
> > ---
> >  arch/arm/boot/dts/da850-evm.dts | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts
> > index f04bc3e15332..f94bb38fdad9 100644
> > --- a/arch/arm/boot/dts/da850-evm.dts
> > +++ b/arch/arm/boot/dts/da850-evm.dts
> > @@ -191,6 +191,19 @@
> >       };
> >  };
> >
> > +&cpu {
> > +     cpu-supply = <&vdcdc3_reg>;
> > +};
> > +
> > +/*
> > + * The standard da850-evm kits and SOM's are 375MHz so enable this operating
> > + * point by default. Higher frequencies must be enabled for custom boards with
> > + * other variants of the SoC.
> > + */
> > +&opp_375 {
> > +     status = "okay";
> > +};
> > +
> >  &sata {
> >       status = "okay";
> >  };
> > --
> > 2.21.0
> >
>

Re: [PATCH v3 2/3] ARM: dts: da850-evm: enable cpufreq

[Bartosz Golaszewski]

pt., 12 kwi 2019 o 15:59 Adam Ford <aford173@gmail.com> napisał(a):
>
> On Fri, Apr 12, 2019 at 8:06 AM Sekhar Nori <nsekhar@ti.com> wrote:
> >
> > On 08/04/19 1:29 PM, Bartosz Golaszewski wrote:
> > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > >
> > > Enable cpufreq-dt support for da850-evm. The cvdd is supplied by the
> > > tps6507 pmic with configurable output voltage, so all operating points
> > > can be enabled.
> > >
> > > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >
> > Adam, can you please ack? Looks good to me.
>
> Sekhar,
> Based on what I read in the thread about the LCDK, I want to do some
> tests on the da850-evm before put my name on it.  I like what I see in
> general, so I am excited to this this here.
>
> Bartosz - which kernel branch should I use as a starting point to apply these?
>

Upstream v5.1-rc4.

Bartosz

> adam
> >
> > Thanks,
> > Sekhar
> >
> > > ---
> > >  arch/arm/boot/dts/da850-evm.dts | 13 +++++++++++++
> > >  1 file changed, 13 insertions(+)
> > >
> > > diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts
> > > index f04bc3e15332..f94bb38fdad9 100644
> > > --- a/arch/arm/boot/dts/da850-evm.dts
> > > +++ b/arch/arm/boot/dts/da850-evm.dts
> > > @@ -191,6 +191,19 @@
> > >       };
> > >  };
> > >
> > > +&cpu {
> > > +     cpu-supply = <&vdcdc3_reg>;
> > > +};
> > > +
> > > +/*
> > > + * The standard da850-evm kits and SOM's are 375MHz so enable this operating
> > > + * point by default. Higher frequencies must be enabled for custom boards with
> > > + * other variants of the SoC.
> > > + */
> > > +&opp_375 {
> > > +     status = "okay";
> > > +};
> > > +
> > >  &sata {
> > >       status = "okay";
> > >  };
> > > --
> > > 2.21.0
> > >
> >

Re: [PATCH v3 1/3] ARM: dts: da850: add cpu node and operating points to DT

[Bartosz Golaszewski]

pt., 12 kwi 2019 o 15:53 Sekhar Nori <nsekhar@ti.com> napisał(a):
>
> On 12/04/19 5:41 PM, Bartosz Golaszewski wrote:
> > pt., 12 kwi 2019 o 13:26 Sekhar Nori <nsekhar@ti.com> napisał(a):
> >>
> >> Hi Bartosz,
> >>
> >> On 08/04/19 1:29 PM, Bartosz Golaszewski wrote:
> >>> From: David Lechner <david@lechnology.com>
> >>>
> >>> This adds a cpu node and operating points to the common da850.dtsi file.
> >>>
> >>> Additionally, a regulator is added to the LEGO EV3 board along with
> >>> some board-specific CPU configuration.
> >>>
> >>> Regulators need to be hooked up on other boards to get them working.
> >>>
> >>> Signed-off-by: David Lechner <david@lechnology.com>
> >>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >>
> >> I remember you mentioning about some problems using OCHI and cpufreq
> >> together. Are those resolved now? CPU PLL on DA850 can affect other
> >> peripheral clock frequencies too. So enabling it should really be a
> >> per-board decision.
> >>
> >
> > The problems are still there. I've never been able to find the
> > culprit, but it also occurs on TI BSP in the same way (a couple
> > cpufreq transitions will make the controller unresponsive).
>
> Is that on LCDK as well? As I recall cpufreq was never enabled on LCDK
> in TI BSP.
>

Yes, I just verified that the bug occurs on LCDK with patches from this series.

> If the OHCI problem is present on LCDK, then there is a user visible
> regression on mainline after this patch. Lets enable cpufreq in LCDK
> only if all working peripherals keep working afterwards.
>

The OHCI driver doesn't register any cpufreq transition notifier
callbacks. I can't really find anything in the datasheet, but I'm
wondering if we shouldn't do something similar to what the driver for
davinci i2c controller does. I'll try a couple things tomorrow.

Bart

Re: [PATCH v3 1/3] ARM: dts: da850: add cpu node and operating points to DT

[Sekhar Nori]

On 12/04/19 9:01 PM, Bartosz Golaszewski wrote:
> pt., 12 kwi 2019 o 15:53 Sekhar Nori <nsekhar@ti.com> napisał(a):
>>
>> On 12/04/19 5:41 PM, Bartosz Golaszewski wrote:
>>> pt., 12 kwi 2019 o 13:26 Sekhar Nori <nsekhar@ti.com> napisał(a):
>>>>
>>>> Hi Bartosz,
>>>>
>>>> On 08/04/19 1:29 PM, Bartosz Golaszewski wrote:
>>>>> From: David Lechner <david@lechnology.com>
>>>>>
>>>>> This adds a cpu node and operating points to the common da850.dtsi file.
>>>>>
>>>>> Additionally, a regulator is added to the LEGO EV3 board along with
>>>>> some board-specific CPU configuration.
>>>>>
>>>>> Regulators need to be hooked up on other boards to get them working.
>>>>>
>>>>> Signed-off-by: David Lechner <david@lechnology.com>
>>>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>>>
>>>> I remember you mentioning about some problems using OCHI and cpufreq
>>>> together. Are those resolved now? CPU PLL on DA850 can affect other
>>>> peripheral clock frequencies too. So enabling it should really be a
>>>> per-board decision.
>>>>
>>>
>>> The problems are still there. I've never been able to find the
>>> culprit, but it also occurs on TI BSP in the same way (a couple
>>> cpufreq transitions will make the controller unresponsive).
>>
>> Is that on LCDK as well? As I recall cpufreq was never enabled on LCDK
>> in TI BSP.
>>
> 
> Yes, I just verified that the bug occurs on LCDK with patches from this series.
> 
>> If the OHCI problem is present on LCDK, then there is a user visible
>> regression on mainline after this patch. Lets enable cpufreq in LCDK
>> only if all working peripherals keep working afterwards.
>>
> 
> The OHCI driver doesn't register any cpufreq transition notifier
> callbacks. I can't really find anything in the datasheet, but I'm
> wondering if we shouldn't do something similar to what the driver for
> davinci i2c controller does. I'll try a couple things tomorrow.

Even if OHCI issue is fixed, with a fixed regulator like on LCDK, I am
not sure the benefits of just frequency scaling will be justifiable enough.

Fixing the OHCI issue may help in other boards like da850-evm use it
though. So that will be a good thing.

How do you feel about keeping all OPPs disabled by default in da850.dtsi
and enabling only the ones that make sense for a board in <board>.dts?

Empty OPP table is illegal, so this does mean that every board must
enable at least one OPP.

Thanks,
Sekhar

Re: [PATCH v3 1/3] ARM: dts: da850: add cpu node and operating points to DT

[Bartosz Golaszewski]

pon., 15 kwi 2019 o 12:21 Sekhar Nori <nsekhar@ti.com> napisał(a):
>
> On 12/04/19 9:01 PM, Bartosz Golaszewski wrote:
> > pt., 12 kwi 2019 o 15:53 Sekhar Nori <nsekhar@ti.com> napisał(a):
> >>
> >> On 12/04/19 5:41 PM, Bartosz Golaszewski wrote:
> >>> pt., 12 kwi 2019 o 13:26 Sekhar Nori <nsekhar@ti.com> napisał(a):
> >>>>
> >>>> Hi Bartosz,
> >>>>
> >>>> On 08/04/19 1:29 PM, Bartosz Golaszewski wrote:
> >>>>> From: David Lechner <david@lechnology.com>
> >>>>>
> >>>>> This adds a cpu node and operating points to the common da850.dtsi file.
> >>>>>
> >>>>> Additionally, a regulator is added to the LEGO EV3 board along with
> >>>>> some board-specific CPU configuration.
> >>>>>
> >>>>> Regulators need to be hooked up on other boards to get them working.
> >>>>>
> >>>>> Signed-off-by: David Lechner <david@lechnology.com>
> >>>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >>>>
> >>>> I remember you mentioning about some problems using OCHI and cpufreq
> >>>> together. Are those resolved now? CPU PLL on DA850 can affect other
> >>>> peripheral clock frequencies too. So enabling it should really be a
> >>>> per-board decision.
> >>>>
> >>>
> >>> The problems are still there. I've never been able to find the
> >>> culprit, but it also occurs on TI BSP in the same way (a couple
> >>> cpufreq transitions will make the controller unresponsive).
> >>
> >> Is that on LCDK as well? As I recall cpufreq was never enabled on LCDK
> >> in TI BSP.
> >>
> >
> > Yes, I just verified that the bug occurs on LCDK with patches from this series.
> >
> >> If the OHCI problem is present on LCDK, then there is a user visible
> >> regression on mainline after this patch. Lets enable cpufreq in LCDK
> >> only if all working peripherals keep working afterwards.
> >>
> >
> > The OHCI driver doesn't register any cpufreq transition notifier
> > callbacks. I can't really find anything in the datasheet, but I'm
> > wondering if we shouldn't do something similar to what the driver for
> > davinci i2c controller does. I'll try a couple things tomorrow.
>
> Even if OHCI issue is fixed, with a fixed regulator like on LCDK, I am
> not sure the benefits of just frequency scaling will be justifiable enough.
>
> Fixing the OHCI issue may help in other boards like da850-evm use it
> though. So that will be a good thing.
>
> How do you feel about keeping all OPPs disabled by default in da850.dtsi
> and enabling only the ones that make sense for a board in <board>.dts?
>
> Empty OPP table is illegal, so this does mean that every board must
> enable at least one OPP.
>

I guess in that case we'd need to make 300 MHz the default for
da850-evm, but on da850-lcdk I'd go for 456 MHz (due to the regulator
being fixed anyway).

Bart

Re: [PATCH v3 1/3] ARM: dts: da850: add cpu node and operating points to DT

[Adam Ford]

On Mon, Apr 15, 2019 at 6:55 AM Bartosz Golaszewski
<bgolaszewski@baylibre.com> wrote:
>
> pon., 15 kwi 2019 o 12:21 Sekhar Nori <nsekhar@ti.com> napisał(a):
> >
> > On 12/04/19 9:01 PM, Bartosz Golaszewski wrote:
> > > pt., 12 kwi 2019 o 15:53 Sekhar Nori <nsekhar@ti.com> napisał(a):
> > >>
> > >> On 12/04/19 5:41 PM, Bartosz Golaszewski wrote:
> > >>> pt., 12 kwi 2019 o 13:26 Sekhar Nori <nsekhar@ti.com> napisał(a):
> > >>>>
> > >>>> Hi Bartosz,
> > >>>>
> > >>>> On 08/04/19 1:29 PM, Bartosz Golaszewski wrote:
> > >>>>> From: David Lechner <david@lechnology.com>
> > >>>>>
> > >>>>> This adds a cpu node and operating points to the common da850.dtsi file.
> > >>>>>
> > >>>>> Additionally, a regulator is added to the LEGO EV3 board along with
> > >>>>> some board-specific CPU configuration.
> > >>>>>
> > >>>>> Regulators need to be hooked up on other boards to get them working.
> > >>>>>
> > >>>>> Signed-off-by: David Lechner <david@lechnology.com>
> > >>>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > >>>>
> > >>>> I remember you mentioning about some problems using OCHI and cpufreq
> > >>>> together. Are those resolved now? CPU PLL on DA850 can affect other
> > >>>> peripheral clock frequencies too. So enabling it should really be a
> > >>>> per-board decision.
> > >>>>
> > >>>
> > >>> The problems are still there. I've never been able to find the
> > >>> culprit, but it also occurs on TI BSP in the same way (a couple
> > >>> cpufreq transitions will make the controller unresponsive).
> > >>
> > >> Is that on LCDK as well? As I recall cpufreq was never enabled on LCDK
> > >> in TI BSP.
> > >>
> > >
> > > Yes, I just verified that the bug occurs on LCDK with patches from this series.
> > >
> > >> If the OHCI problem is present on LCDK, then there is a user visible
> > >> regression on mainline after this patch. Lets enable cpufreq in LCDK
> > >> only if all working peripherals keep working afterwards.
> > >>
> > >
> > > The OHCI driver doesn't register any cpufreq transition notifier
> > > callbacks. I can't really find anything in the datasheet, but I'm
> > > wondering if we shouldn't do something similar to what the driver for
> > > davinci i2c controller does. I'll try a couple things tomorrow.
> >
> > Even if OHCI issue is fixed, with a fixed regulator like on LCDK, I am
> > not sure the benefits of just frequency scaling will be justifiable enough.
> >
> > Fixing the OHCI issue may help in other boards like da850-evm use it
> > though. So that will be a good thing.
> >
> > How do you feel about keeping all OPPs disabled by default in da850.dtsi
> > and enabling only the ones that make sense for a board in <board>.dts?
> >
> > Empty OPP table is illegal, so this does mean that every board must
> > enable at least one OPP.
> >
>
> I guess in that case we'd need to make 300 MHz the default for
> da850-evm, but on da850-lcdk I'd go for 456 MHz (due to the regulator
> being fixed anyway).

Based on what I've seen of your V4 patch, I think it's fine to let the
da850-evm run at 375.  I am not sure why it would need to be default
to 300.
I agree that the cpu-supply is correctly referencing vdcdc3_reg, and
it's currently setup to handle .95 to 1.35V, which I think is OK based
on what I've seen and the OPP's look right from what I can tell.

adam
>
> Bart

Re: [PATCH v3 1/3] ARM: dts: da850: add cpu node and operating points to DT

[Bartosz Golaszewski]

pon., 15 kwi 2019 o 12:21 Sekhar Nori <nsekhar@ti.com> napisał(a):
>
> On 12/04/19 9:01 PM, Bartosz Golaszewski wrote:
> > pt., 12 kwi 2019 o 15:53 Sekhar Nori <nsekhar@ti.com> napisał(a):
> >>
> >> On 12/04/19 5:41 PM, Bartosz Golaszewski wrote:
> >>> pt., 12 kwi 2019 o 13:26 Sekhar Nori <nsekhar@ti.com> napisał(a):
> >>>>
> >>>> Hi Bartosz,
> >>>>
> >>>> On 08/04/19 1:29 PM, Bartosz Golaszewski wrote:
> >>>>> From: David Lechner <david@lechnology.com>
> >>>>>
> >>>>> This adds a cpu node and operating points to the common da850.dtsi file.
> >>>>>
> >>>>> Additionally, a regulator is added to the LEGO EV3 board along with
> >>>>> some board-specific CPU configuration.
> >>>>>
> >>>>> Regulators need to be hooked up on other boards to get them working.
> >>>>>
> >>>>> Signed-off-by: David Lechner <david@lechnology.com>
> >>>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >>>>
> >>>> I remember you mentioning about some problems using OCHI and cpufreq
> >>>> together. Are those resolved now? CPU PLL on DA850 can affect other
> >>>> peripheral clock frequencies too. So enabling it should really be a
> >>>> per-board decision.
> >>>>
> >>>
> >>> The problems are still there. I've never been able to find the
> >>> culprit, but it also occurs on TI BSP in the same way (a couple
> >>> cpufreq transitions will make the controller unresponsive).
> >>
> >> Is that on LCDK as well? As I recall cpufreq was never enabled on LCDK
> >> in TI BSP.
> >>
> >
> > Yes, I just verified that the bug occurs on LCDK with patches from this series.
> >
> >> If the OHCI problem is present on LCDK, then there is a user visible
> >> regression on mainline after this patch. Lets enable cpufreq in LCDK
> >> only if all working peripherals keep working afterwards.
> >>
> >
> > The OHCI driver doesn't register any cpufreq transition notifier
> > callbacks. I can't really find anything in the datasheet, but I'm
> > wondering if we shouldn't do something similar to what the driver for
> > davinci i2c controller does. I'll try a couple things tomorrow.
>
> Even if OHCI issue is fixed, with a fixed regulator like on LCDK, I am
> not sure the benefits of just frequency scaling will be justifiable enough.
>
> Fixing the OHCI issue may help in other boards like da850-evm use it
> though. So that will be a good thing.
>

I've been trying different things, like suspending the device before
the transition, resetting the controller or playing with the clock
during transitions but it always results in the same kind of error:

ohci-da8xx 1e25000.usb: frame counter not updating; disabled
ohci-da8xx 1e25000.usb: HC died; cleaning up
usb 1-1: USB disconnect, device number 2

If you have any idea - let me know, otherwise I'll give up.

If we agree on the direction of these patches, then I can go with a
single enabled OPP for lcdk (456 MHz) and all OPPs up to 375 MHz
enabled for da850-evm.

David - do you want to keep the lego board as is?

Bart

> How do you feel about keeping all OPPs disabled by default in da850.dtsi
> and enabling only the ones that make sense for a board in <board>.dts?
>
> Empty OPP table is illegal, so this does mean that every board must
> enable at least one OPP.
>
> Thanks,
> Sekhar

Re: [PATCH v3 1/3] ARM: dts: da850: add cpu node and operating points to DT

[Adam Ford]

On Tue, Apr 16, 2019 at 3:38 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> pon., 15 kwi 2019 o 12:21 Sekhar Nori <nsekhar@ti.com> napisał(a):
> >
> > On 12/04/19 9:01 PM, Bartosz Golaszewski wrote:
> > > pt., 12 kwi 2019 o 15:53 Sekhar Nori <nsekhar@ti.com> napisał(a):
> > >>
> > >> On 12/04/19 5:41 PM, Bartosz Golaszewski wrote:
> > >>> pt., 12 kwi 2019 o 13:26 Sekhar Nori <nsekhar@ti.com> napisał(a):
> > >>>>
> > >>>> Hi Bartosz,
> > >>>>
> > >>>> On 08/04/19 1:29 PM, Bartosz Golaszewski wrote:
> > >>>>> From: David Lechner <david@lechnology.com>
> > >>>>>
> > >>>>> This adds a cpu node and operating points to the common da850.dtsi file.
> > >>>>>
> > >>>>> Additionally, a regulator is added to the LEGO EV3 board along with
> > >>>>> some board-specific CPU configuration.
> > >>>>>
> > >>>>> Regulators need to be hooked up on other boards to get them working.
> > >>>>>
> > >>>>> Signed-off-by: David Lechner <david@lechnology.com>
> > >>>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > >>>>
> > >>>> I remember you mentioning about some problems using OCHI and cpufreq
> > >>>> together. Are those resolved now? CPU PLL on DA850 can affect other
> > >>>> peripheral clock frequencies too. So enabling it should really be a
> > >>>> per-board decision.
> > >>>>
> > >>>
> > >>> The problems are still there. I've never been able to find the
> > >>> culprit, but it also occurs on TI BSP in the same way (a couple
> > >>> cpufreq transitions will make the controller unresponsive).
> > >>
> > >> Is that on LCDK as well? As I recall cpufreq was never enabled on LCDK
> > >> in TI BSP.
> > >>
> > >
> > > Yes, I just verified that the bug occurs on LCDK with patches from this series.
> > >
> > >> If the OHCI problem is present on LCDK, then there is a user visible
> > >> regression on mainline after this patch. Lets enable cpufreq in LCDK
> > >> only if all working peripherals keep working afterwards.
> > >>
> > >
> > > The OHCI driver doesn't register any cpufreq transition notifier
> > > callbacks. I can't really find anything in the datasheet, but I'm
> > > wondering if we shouldn't do something similar to what the driver for
> > > davinci i2c controller does. I'll try a couple things tomorrow.
> >
> > Even if OHCI issue is fixed, with a fixed regulator like on LCDK, I am
> > not sure the benefits of just frequency scaling will be justifiable enough.
> >
> > Fixing the OHCI issue may help in other boards like da850-evm use it
> > though. So that will be a good thing.
> >
>
> I've been trying different things, like suspending the device before
> the transition, resetting the controller or playing with the clock
> during transitions but it always results in the same kind of error:
>
> ohci-da8xx 1e25000.usb: frame counter not updating; disabled
> ohci-da8xx 1e25000.usb: HC died; cleaning up
> usb 1-1: USB disconnect, device number 2
>
> If you have any idea - let me know, otherwise I'll give up.
>
> If we agree on the direction of these patches, then I can go with a
> single enabled OPP for lcdk (456 MHz) and all OPPs up to 375 MHz
> enabled for da850-evm.

One last questions, and this probably directed at Sekhar, but what
happens if you modify the OPP for the boards with fixed regulators to
enable all the frequencies but with the only available voltage.  Is
there harm is running the processor at a higher voltage than
necessary?  I did some quick experiments on a different ARM board and
I saw some changes in power consumption.  I would think some power
savings might be better than none, but I don't know if it causes
damage.

adam
>
> David - do you want to keep the lego board as is?
>
> Bart
>
> > How do you feel about keeping all OPPs disabled by default in da850.dtsi
> > and enabling only the ones that make sense for a board in <board>.dts?
> >
> > Empty OPP table is illegal, so this does mean that every board must
> > enable at least one OPP.
> >
> > Thanks,
> > Sekhar

Re: [PATCH v3 1/3] ARM: dts: da850: add cpu node and operating points to DT

[David Lechner]

On 4/16/19 3:37 AM, Bartosz Golaszewski wrote:
> pon., 15 kwi 2019 o 12:21 Sekhar Nori <nsekhar@ti.com> napisał(a):
>>
>> On 12/04/19 9:01 PM, Bartosz Golaszewski wrote:
>>> pt., 12 kwi 2019 o 15:53 Sekhar Nori <nsekhar@ti.com> napisał(a):
>>>>
>>>> On 12/04/19 5:41 PM, Bartosz Golaszewski wrote:
>>>>> pt., 12 kwi 2019 o 13:26 Sekhar Nori <nsekhar@ti.com> napisał(a):
>>>>>>
>>>>>> Hi Bartosz,
>>>>>>
>>>>>> On 08/04/19 1:29 PM, Bartosz Golaszewski wrote:
>>>>>>> From: David Lechner <david@lechnology.com>
>>>>>>>
>>>>>>> This adds a cpu node and operating points to the common da850.dtsi file.
>>>>>>>
>>>>>>> Additionally, a regulator is added to the LEGO EV3 board along with
>>>>>>> some board-specific CPU configuration.
>>>>>>>
>>>>>>> Regulators need to be hooked up on other boards to get them working.
>>>>>>>
>>>>>>> Signed-off-by: David Lechner <david@lechnology.com>
>>>>>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>>>>>
>>>>>> I remember you mentioning about some problems using OCHI and cpufreq
>>>>>> together. Are those resolved now? CPU PLL on DA850 can affect other
>>>>>> peripheral clock frequencies too. So enabling it should really be a
>>>>>> per-board decision.
>>>>>>
>>>>>
>>>>> The problems are still there. I've never been able to find the
>>>>> culprit, but it also occurs on TI BSP in the same way (a couple
>>>>> cpufreq transitions will make the controller unresponsive).
>>>>
>>>> Is that on LCDK as well? As I recall cpufreq was never enabled on LCDK
>>>> in TI BSP.
>>>>
>>>
>>> Yes, I just verified that the bug occurs on LCDK with patches from this series.
>>>
>>>> If the OHCI problem is present on LCDK, then there is a user visible
>>>> regression on mainline after this patch. Lets enable cpufreq in LCDK
>>>> only if all working peripherals keep working afterwards.
>>>>
>>>
>>> The OHCI driver doesn't register any cpufreq transition notifier
>>> callbacks. I can't really find anything in the datasheet, but I'm
>>> wondering if we shouldn't do something similar to what the driver for
>>> davinci i2c controller does. I'll try a couple things tomorrow.
>>
>> Even if OHCI issue is fixed, with a fixed regulator like on LCDK, I am
>> not sure the benefits of just frequency scaling will be justifiable enough.
>>
>> Fixing the OHCI issue may help in other boards like da850-evm use it
>> though. So that will be a good thing.
>>
> 
> I've been trying different things, like suspending the device before
> the transition, resetting the controller or playing with the clock
> during transitions but it always results in the same kind of error:
> 
> ohci-da8xx 1e25000.usb: frame counter not updating; disabled
> ohci-da8xx 1e25000.usb: HC died; cleaning up
> usb 1-1: USB disconnect, device number 2
> 
> If you have any idea - let me know, otherwise I'll give up.
> 
> If we agree on the direction of these patches, then I can go with a
> single enabled OPP for lcdk (456 MHz) and all OPPs up to 375 MHz
> enabled for da850-evm.
> 
> David - do you want to keep the lego board as is?

Yes, I think so. Even if we can't use CPU frequency throttling because
of the USB issue, I think it would still be nice if we could use this
to increase the frequency once during early boot so that we can run
faster than what the bootloader set.

I've actually been running one of my EV3 bricks at 456MHz for almost
a year now without noticing any problems. (This is of course out of
spec since we only have 1.2V so I'm not saying we should add that
operating point to the mainline kernel.)

> 
> Bart
> 
>> How do you feel about keeping all OPPs disabled by default in da850.dtsi
>> and enabling only the ones that make sense for a board in <board>.dts?
>>
>> Empty OPP table is illegal, so this does mean that every board must
>> enable at least one OPP.
>>
>> Thanks,
>> Sekhar

Re: [PATCH v3 1/3] ARM: dts: da850: add cpu node and operating points to DT

[Sekhar Nori]

On 16/04/19 5:18 PM, Adam Ford wrote:
> On Tue, Apr 16, 2019 at 3:38 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>>
>> pon., 15 kwi 2019 o 12:21 Sekhar Nori <nsekhar@ti.com> napisał(a):
>>>
>>> On 12/04/19 9:01 PM, Bartosz Golaszewski wrote:
>>>> pt., 12 kwi 2019 o 15:53 Sekhar Nori <nsekhar@ti.com> napisał(a):
>>>>>
>>>>> On 12/04/19 5:41 PM, Bartosz Golaszewski wrote:
>>>>>> pt., 12 kwi 2019 o 13:26 Sekhar Nori <nsekhar@ti.com> napisał(a):
>>>>>>>
>>>>>>> Hi Bartosz,
>>>>>>>
>>>>>>> On 08/04/19 1:29 PM, Bartosz Golaszewski wrote:
>>>>>>>> From: David Lechner <david@lechnology.com>
>>>>>>>>
>>>>>>>> This adds a cpu node and operating points to the common da850.dtsi file.
>>>>>>>>
>>>>>>>> Additionally, a regulator is added to the LEGO EV3 board along with
>>>>>>>> some board-specific CPU configuration.
>>>>>>>>
>>>>>>>> Regulators need to be hooked up on other boards to get them working.
>>>>>>>>
>>>>>>>> Signed-off-by: David Lechner <david@lechnology.com>
>>>>>>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>>>>>>
>>>>>>> I remember you mentioning about some problems using OCHI and cpufreq
>>>>>>> together. Are those resolved now? CPU PLL on DA850 can affect other
>>>>>>> peripheral clock frequencies too. So enabling it should really be a
>>>>>>> per-board decision.
>>>>>>>
>>>>>>
>>>>>> The problems are still there. I've never been able to find the
>>>>>> culprit, but it also occurs on TI BSP in the same way (a couple
>>>>>> cpufreq transitions will make the controller unresponsive).
>>>>>
>>>>> Is that on LCDK as well? As I recall cpufreq was never enabled on LCDK
>>>>> in TI BSP.
>>>>>
>>>>
>>>> Yes, I just verified that the bug occurs on LCDK with patches from this series.
>>>>
>>>>> If the OHCI problem is present on LCDK, then there is a user visible
>>>>> regression on mainline after this patch. Lets enable cpufreq in LCDK
>>>>> only if all working peripherals keep working afterwards.
>>>>>
>>>>
>>>> The OHCI driver doesn't register any cpufreq transition notifier
>>>> callbacks. I can't really find anything in the datasheet, but I'm
>>>> wondering if we shouldn't do something similar to what the driver for
>>>> davinci i2c controller does. I'll try a couple things tomorrow.
>>>
>>> Even if OHCI issue is fixed, with a fixed regulator like on LCDK, I am
>>> not sure the benefits of just frequency scaling will be justifiable enough.
>>>
>>> Fixing the OHCI issue may help in other boards like da850-evm use it
>>> though. So that will be a good thing.
>>>
>>
>> I've been trying different things, like suspending the device before
>> the transition, resetting the controller or playing with the clock
>> during transitions but it always results in the same kind of error:
>>
>> ohci-da8xx 1e25000.usb: frame counter not updating; disabled
>> ohci-da8xx 1e25000.usb: HC died; cleaning up
>> usb 1-1: USB disconnect, device number 2
>>
>> If you have any idea - let me know, otherwise I'll give up.
>>
>> If we agree on the direction of these patches, then I can go with a
>> single enabled OPP for lcdk (456 MHz) and all OPPs up to 375 MHz
>> enabled for da850-evm.
> 
> One last questions, and this probably directed at Sekhar, but what
> happens if you modify the OPP for the boards with fixed regulators to
> enable all the frequencies but with the only available voltage.  Is
> there harm is running the processor at a higher voltage than
> necessary?  I did some quick experiments on a different ARM board and
> I saw some changes in power consumption.  I would think some power
> savings might be better than none, but I don't know if it causes> damage.

The OMAP-L138 datasheet mentions two versions of devices in core voltage
specification:

Variable (1.2V-1.0V) for 375 MHz version
Variable (1.3V-1.0V) for 456 MHz version

If you have 375 MHz version of device, I do not think you should run at
1.3V. I don't know what "damage" it will cause or how long it takes for
any of it to be visible.

Keeping that aside, I doubt there will be a lot of power-saving benefit
without voltage scaling. Even if you see a slightly lower power number
when you reduce frequency, there is also work to be done for the scaling
operation itself.

Thanks,
Sekhar

Re: [PATCH v3 1/3] ARM: dts: da850: add cpu node and operating points to DT

[Sekhar Nori]

On 16/04/19 2:07 PM, Bartosz Golaszewski wrote:

> If we agree on the direction of these patches, then I can go with a
> single enabled OPP for lcdk (456 MHz) and all OPPs up to 375 MHz
> enabled for da850-evm.

Sounds good to me.

Thanks,
Sekhar