[RFC 0/5] OMAP3: convert opp-v1 to opp-v2 and read speed binned / 720MHz grade bits

[RFC 0/5] OMAP3: convert opp-v1 to opp-v2 and read speed binned / 720MHz grade bits

[H. Nikolaus Schaller]

This patch set converts the opp tables to opp-v2 format
and extends the ti-cpufreq to support omap3.

It adds 720 MHz (omap34xx) and 1 GHz (omap36xx) OPPs but
tells the ti-cpufreq driver to disable them if the speed
binned / 720MHz grade eFuse bits indicate that the chip
is not rated for that speed. 

It has been tested (for chip variant detection, not reliability
of high speed OPPs) on:
* BeagleBoard C2 (omap3430 600MHz)
* BeagleBoard XM B (dm3730 800MHz)
* GTA04A4 (dm3730 800MHz)
* GTA04A5 (dm3730 1GHz)


H. Nikolaus Schaller (5):
  cpufreq: ti-cpufreq: add support for omap34xx and omap36xx
  ARM: dts: add support for opp-v2 for omap34xx and omap36xx
  ARM: dts: omap3-evm-37xx: fix compatible from omap3630 to omap36xx
  ARM: dts: omap3-n950-n9: remove opp-v1 table
  ARM: dts: omap3-beagle: make explicitly compatible to ti,omap34xx

 arch/arm/boot/dts/omap3-beagle.dts   |  2 +-
 arch/arm/boot/dts/omap3-evm-37xx.dts |  2 +-
 arch/arm/boot/dts/omap3-n950-n9.dtsi |  7 ---
 arch/arm/boot/dts/omap34xx.dtsi      | 59 ++++++++++++++++---
 arch/arm/boot/dts/omap36xx.dtsi      | 47 ++++++++++++---
 drivers/cpufreq/cpufreq-dt-platdev.c |  2 +-
 drivers/cpufreq/ti-cpufreq.c         | 86 +++++++++++++++++++++++++++-
 7 files changed, 176 insertions(+), 29 deletions(-)

-- 
2.19.1

[RFC 1/5] cpufreq: ti-cpufreq: add support for omap34xx and omap36xx

[H. Nikolaus Schaller]

This adds code and tables to read the silicon revision and
eFuse (speed binned / 720 MHz grade) bits for selecting
opp-v2 table entries.

Since these bits are not always part of the syscon register
range (like for am3, am4, dra7), we add code to directly read
the register values using ioremap() if syscon access fails.

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 drivers/cpufreq/ti-cpufreq.c | 86 +++++++++++++++++++++++++++++++++++-
 1 file changed, 84 insertions(+), 2 deletions(-)

diff --git a/drivers/cpufreq/ti-cpufreq.c b/drivers/cpufreq/ti-cpufreq.c
index 2ad1ae17932d..5c5ca4577093 100644
--- a/drivers/cpufreq/ti-cpufreq.c
+++ b/drivers/cpufreq/ti-cpufreq.c
@@ -31,6 +31,11 @@
 #define DRA7_EFUSE_OD_MPU_OPP			BIT(1)
 #define DRA7_EFUSE_HIGH_MPU_OPP			BIT(2)
 
+#define OMAP3_CONTROL_DEVICE_STATUS		0x4800244C
+#define OMAP3_CONTROL_IDCODE			0x4830A204
+#define OMAP34xx_ProdID_SKUID			0x4830A20C
+#define OMAP3_SYSCON_BASE	(0x48000000 + 0x2000 + 0x270)
+
 #define VERSION_COUNT				2
 
 struct ti_cpufreq_data;
@@ -84,6 +89,13 @@ static unsigned long dra7_efuse_xlate(struct ti_cpufreq_data *opp_data,
 	return calculated_efuse;
 }
 
+static unsigned long omap3_efuse_xlate(struct ti_cpufreq_data *opp_data,
+				      unsigned long efuse)
+{
+	/* OPP enable bit ("Speed Binned") */
+	return BIT(efuse);
+}
+
 static struct ti_cpufreq_soc_data am3x_soc_data = {
 	.efuse_xlate = amx3_efuse_xlate,
 	.efuse_fallback = AM33XX_800M_ARM_MPU_MAX_FREQ,
@@ -111,6 +123,56 @@ static struct ti_cpufreq_soc_data dra7_soc_data = {
 	.multi_regulator = true,
 };
 
+/*
+ * OMAP35x TRM (SPRUF98K):
+ *  CONTROL_IDCODE (0x4830 A204) describes Silicon revisions.
+ *  Control OMAP Status Register 15:0 (Address 0x4800 244C)
+ *    to separate between omap3503, omap3515, omap3525, omap3530
+ *    and feature presence.
+ *    There are encodings for versions limited to 400/266MHz
+ *    but we ignore.
+ *    Not clear if this also holds for omap34xx.
+ *  some eFuse values e.g. CONTROL_FUSE_OPP1_VDD1
+ *    are stored in the SYSCON register range
+ *  Register 0x4830A20C [ProdID.SKUID] [0:3]
+ *    0x0 for normal 600/430MHz device.
+ *    0x8 for 720/520MHz device.
+ *    Not clear what omap34xx value is.
+ */
+
+static struct ti_cpufreq_soc_data omap34xx_soc_data = {
+	.efuse_xlate = omap3_efuse_xlate,
+	.efuse_offset = OMAP34xx_ProdID_SKUID - OMAP3_SYSCON_BASE,
+	.efuse_shift = 3,
+	.efuse_mask = BIT(3),
+	.rev_offset = OMAP3_CONTROL_IDCODE - OMAP3_SYSCON_BASE,
+	.multi_regulator = false,
+};
+
+/*
+ * AM/DM37x TRM (SPRUGN4M)
+ *  CONTROL_IDCODE (0x4830 A204) describes Silicon revisions.
+ *  Control Device Status Register 15:0 (Address 0x4800 244C)
+ *    to separate between am3703, am3715, dm3725, dm3730
+ *    and feature presence.
+ *   Speed Binned = Bit 9
+ *     0 800/600 MHz
+ *     1 1000/800 MHz
+ *  some eFuse values e.g. CONTROL_FUSE_OPP 1G_VDD1
+ *    are stored in the SYSCON register range.
+ *  There is no 0x4830A20C [ProdID.SKUID] register (exists but
+ *    seems to always read as 0).
+ */
+
+static struct ti_cpufreq_soc_data omap36xx_soc_data = {
+	.efuse_xlate = omap3_efuse_xlate,
+	.efuse_offset = OMAP3_CONTROL_DEVICE_STATUS - OMAP3_SYSCON_BASE,
+	.efuse_shift = 9,
+	.efuse_mask = BIT(9),
+	.rev_offset = OMAP3_CONTROL_IDCODE - OMAP3_SYSCON_BASE,
+	.multi_regulator = false,
+};
+
 /**
  * ti_cpufreq_get_efuse() - Parse and return efuse value present on SoC
  * @opp_data: pointer to ti_cpufreq_data context
@@ -127,7 +189,15 @@ static int ti_cpufreq_get_efuse(struct ti_cpufreq_data *opp_data,
 
 	ret = regmap_read(opp_data->syscon, opp_data->soc_data->efuse_offset,
 			  &efuse);
-	if (ret) {
+	if (ret == -EIO) {
+		/* not a syscon register! */
+		void __iomem *regs = ioremap(OMAP3_SYSCON_BASE +
+				opp_data->soc_data->efuse_offset, 4);
+
+		efuse = readl(regs);
+		iounmap(regs);
+		}
+	else if (ret) {
 		dev_err(dev,
 			"Failed to read the efuse value from syscon: %d\n",
 			ret);
@@ -158,7 +228,15 @@ static int ti_cpufreq_get_rev(struct ti_cpufreq_data *opp_data,
 
 	ret = regmap_read(opp_data->syscon, opp_data->soc_data->rev_offset,
 			  &revision);
-	if (ret) {
+	if (ret == -EIO) {
+		/* not a syscon register! */
+		void __iomem *regs = ioremap(OMAP3_SYSCON_BASE +
+				opp_data->soc_data->rev_offset, 4);
+
+		revision = readl(regs);
+		iounmap(regs);
+		}
+	else if (ret) {
 		dev_err(dev,
 			"Failed to read the revision number from syscon: %d\n",
 			ret);
@@ -190,6 +268,10 @@ static const struct of_device_id ti_cpufreq_of_match[] = {
 	{ .compatible = "ti,am33xx", .data = &am3x_soc_data, },
 	{ .compatible = "ti,am43", .data = &am4x_soc_data, },
 	{ .compatible = "ti,dra7", .data = &dra7_soc_data },
+	{ .compatible = "ti,omap3430", .data = &omap34xx_soc_data, },
+	{ .compatible = "ti,omap34xx", .data = &omap34xx_soc_data, },
+	{ .compatible = "ti,omap3630", .data = &omap36xx_soc_data, },
+	{ .compatible = "ti,omap36xx", .data = &omap36xx_soc_data, },
 	{},
 };
 
-- 
2.19.1

[RFC 2/5] ARM: dts: add support for opp-v2 for omap34xx and omap36xx

[H. Nikolaus Schaller]

In addition, move omap3 from whitelist to blacklist in cpufreq-dt-platdev
in the same patch, because doing either first breaks operation and
may make trouble in bisect.

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 arch/arm/boot/dts/omap34xx.dtsi      | 59 +++++++++++++++++++++++-----
 arch/arm/boot/dts/omap36xx.dtsi      | 47 ++++++++++++++++++----
 drivers/cpufreq/cpufreq-dt-platdev.c |  2 +-
 3 files changed, 90 insertions(+), 18 deletions(-)

diff --git a/arch/arm/boot/dts/omap34xx.dtsi b/arch/arm/boot/dts/omap34xx.dtsi
index f572a477f74c..b92645d6d981 100644
--- a/arch/arm/boot/dts/omap34xx.dtsi
+++ b/arch/arm/boot/dts/omap34xx.dtsi
@@ -16,19 +16,60 @@
 / {
 	cpus {
 		cpu: cpu@0 {
-			/* OMAP343x/OMAP35xx variants OPP1-5 */
-			operating-points = <
-				/* kHz    uV */
-				125000   975000
-				250000  1075000
-				500000  1200000
-				550000  1270000
-				600000  1350000
-			>;
+			/* OMAP343x/OMAP35xx variants OPP1-6 */
+			operating-points-v2 = <&cpu0_opp_table>;
+
 			clock-latency = <300000>; /* From legacy driver */
 		};
 	};
 
+	/* see Documentation/devicetree/bindings/opp/opp.txt */
+	cpu0_opp_table: opp-table {
+		compatible = "operating-points-v2-ti-cpu";
+		syscon = <&scm_conf>;
+
+		opp1-125000000 {
+			opp-hz = /bits/ 64 <125000000>;
+			// we currently only select the max voltage from table Table 3-3 of the omap3530 Data sheet (SPRS507F)
+			// <target min max> could also be single <target>
+			opp-microvolt = <975000 975000 975000>;
+			// first value is silicon revision, second one 720MHz Device Identification
+			opp-supported-hw = <0xffffffff 3>;
+		};
+
+		opp2-250000000 {
+			opp-hz = /bits/ 64 <250000000>;
+			opp-microvolt = <1075000 1075000 1075000>;
+			opp-supported-hw = <0xffffffff 3>;
+			opp-suspend;
+		};
+
+		opp3-500000000 {
+			opp-hz = /bits/ 64 <500000000>;
+			opp-microvolt = <1200000 1200000 1200000>;
+			opp-supported-hw = <0xffffffff 3>;
+		};
+
+		opp4-550000000 {
+			opp-hz = /bits/ 64 <550000000>;
+			opp-microvolt = <1270000 1270000 1270000>;
+			opp-supported-hw = <0xffffffff 3>;
+		};
+
+		opp5-600000000 {
+			opp-hz = /bits/ 64 <600000000>;
+			opp-microvolt = <1350000 1350000 1350000>;
+			opp-supported-hw = <0xffffffff 3>;
+		};
+
+		opp6-720000000 {
+			opp-hz = /bits/ 64 <720000000>;
+			opp-microvolt = <1350000 1350000 1350000>;
+			/* only high-speed grade omap3530 devices */
+			opp-supported-hw = <0xffffffff 2>;
+		};
+	};
+
 	ocp@68000000 {
 		omap3_pmx_core2: pinmux@480025d8 {
 			compatible = "ti,omap3-padconf", "pinctrl-single";
diff --git a/arch/arm/boot/dts/omap36xx.dtsi b/arch/arm/boot/dts/omap36xx.dtsi
index 6fb23ada1f64..1595814585df 100644
--- a/arch/arm/boot/dts/omap36xx.dtsi
+++ b/arch/arm/boot/dts/omap36xx.dtsi
@@ -19,15 +19,46 @@
 	};
 
 	cpus {
-		/* OMAP3630/OMAP37xx 'standard device' variants OPP50 to OPP130 */
+		/* OMAP3630/OMAP37xx variants OPP50 to OPP130 and OPP1G */
 		cpu: cpu@0 {
-			operating-points = <
-				/* kHz    uV */
-				300000  1012500
-				600000  1200000
-				800000  1325000
-			>;
-			clock-latency = <300000>; /* From legacy driver */
+			operating-points-v2 = <&cpu0_opp_table>;
+
+			clock-latency = <300000>; /* From omap-cpufreq driver */
+		};
+	};
+
+	/* see Documentation/devicetree/bindings/opp/opp.txt */
+	cpu0_opp_table: opp-table {
+		compatible = "operating-points-v2-ti-cpu";
+		syscon = <&scm_conf>;
+
+		opp50-300000000 {
+			opp-hz = /bits/ 64 <300000000>;
+			// we currently only select the max voltage from table Table 4-19 of the DM3730 Data sheet (SPRS685B)
+			// <target min max> could also be single <target>
+			opp-microvolt = <1012500 1012500 1012500>;
+			// first value is silicon revision, second one is "speed binned
+			opp-supported-hw = <0xffffffff 3>;
+			opp-suspend;
+		};
+
+		opp100-600000000 {
+			opp-hz = /bits/ 64 <600000000>;
+			opp-microvolt = <1200000 1200000 1200000>;
+			opp-supported-hw = <0xffffffff 3>;
+		};
+
+		opp130-800000000 {
+			opp-hz = /bits/ 64 <800000000>;
+			opp-microvolt = <1325000 1325000 1325000>;
+			opp-supported-hw = <0xffffffff 3>;
+		};
+
+		opp1g-1000000000 {
+			opp-hz = /bits/ 64 <1000000000>;
+			opp-microvolt = <1375000 1375000 1375000>;
+			/* only on am/dm37x with speed-binned bit set */
+			opp-supported-hw = <0xffffffff 2>;
 		};
 	};
 
diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c
index 03dc4244ab00..68b7fc4225f8 100644
--- a/drivers/cpufreq/cpufreq-dt-platdev.c
+++ b/drivers/cpufreq/cpufreq-dt-platdev.c
@@ -86,7 +86,6 @@ static const struct of_device_id whitelist[] __initconst = {
 	{ .compatible = "st-ericsson,u9540", },
 
 	{ .compatible = "ti,omap2", },
-	{ .compatible = "ti,omap3", },
 	{ .compatible = "ti,omap4", },
 	{ .compatible = "ti,omap5", },
 
@@ -132,6 +131,7 @@ static const struct of_device_id blacklist[] __initconst = {
 	{ .compatible = "ti,am33xx", },
 	{ .compatible = "ti,am43", },
 	{ .compatible = "ti,dra7", },
+	{ .compatible = "ti,omap3", },
 
 	{ }
 };
-- 
2.19.1

Re: [RFC 2/5] ARM: dts: add support for opp-v2 for omap34xx and omap36xx

[Viresh Kumar]

On 02-09-19, 12:55, H. Nikolaus Schaller wrote:
> +		opp1-125000000 {
> +			opp-hz = /bits/ 64 <125000000>;
> +			// we currently only select the max voltage from table Table 3-3 of the omap3530 Data sheet (SPRS507F)
> +			// <target min max> could also be single <target>
> +			opp-microvolt = <975000 975000 975000>;
> +			// first value is silicon revision, second one 720MHz Device Identification

> +			opp-supported-hw = <0xffffffff 3>;

I don't see the driver changes using this field, am I missing
something ?

-- 
viresh

Re: [RFC 2/5] ARM: dts: add support for opp-v2 for omap34xx and omap36xx

[Viresh Kumar]

On 03-09-19, 08:08, Viresh Kumar wrote:
> On 02-09-19, 12:55, H. Nikolaus Schaller wrote:
> > +		opp1-125000000 {
> > +			opp-hz = /bits/ 64 <125000000>;
> > +			// we currently only select the max voltage from table Table 3-3 of the omap3530 Data sheet (SPRS507F)
> > +			// <target min max> could also be single <target>
> > +			opp-microvolt = <975000 975000 975000>;
> > +			// first value is silicon revision, second one 720MHz Device Identification
> 
> > +			opp-supported-hw = <0xffffffff 3>;
> 
> I don't see the driver changes using this field, am I missing
> something ?

The driver already had them it seems. Hmm..

-- 
viresh

Re: [RFC 2/5] ARM: dts: add support for opp-v2 for omap34xx and omap36xx

[H. Nikolaus Schaller]

> Am 03.09.2019 um 04:38 schrieb Viresh Kumar <viresh.kumar@linaro.org>:
> 
> On 02-09-19, 12:55, H. Nikolaus Schaller wrote:
>> +		opp1-125000000 {
>> +			opp-hz = /bits/ 64 <125000000>;
>> +			// we currently only select the max voltage from table Table 3-3 of the omap3530 Data sheet (SPRS507F)
>> +			// <target min max> could also be single <target>
>> +			opp-microvolt = <975000 975000 975000>;
>> +			// first value is silicon revision, second one 720MHz Device Identification
> 
>> +			opp-supported-hw = <0xffffffff 3>;
> 
> I don't see the driver changes using this field, am I missing
> something ?

This is parsed by _opp_is_supported() which is called
indirectly by ti_cpufreq_probe().

BR,
Nikolaus

[RFC 3/5] ARM: dts: omap3-evm-37xx: fix compatible from omap3630 to omap36xx

[H. Nikolaus Schaller]

All other boards use compatible = "omap36xx".

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 arch/arm/boot/dts/omap3-evm-37xx.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/omap3-evm-37xx.dts b/arch/arm/boot/dts/omap3-evm-37xx.dts
index e0c0382388f0..311550ee4eae 100644
--- a/arch/arm/boot/dts/omap3-evm-37xx.dts
+++ b/arch/arm/boot/dts/omap3-evm-37xx.dts
@@ -10,7 +10,7 @@
 
 / {
 	model = "TI OMAP37XX EVM (TMDSEVM3730)";
-	compatible = "ti,omap3-evm-37xx", "ti,omap3630", "ti,omap3";
+	compatible = "ti,omap3-evm-37xx", "ti,omap36xx", "ti,omap3";
 };
 
 &omap3_pmx_core2 {
-- 
2.19.1

[RFC 4/5] ARM: dts: omap3-n950-n9: remove opp-v1 table

[H. Nikolaus Schaller]

With opp-v2 in omap36xx.dtsi and ti-cpufreq driver the
1GHz capability is automatically detected.

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 arch/arm/boot/dts/omap3-n950-n9.dtsi | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/arch/arm/boot/dts/omap3-n950-n9.dtsi b/arch/arm/boot/dts/omap3-n950-n9.dtsi
index 5441e9ffdbb4..e98b0c615f19 100644
--- a/arch/arm/boot/dts/omap3-n950-n9.dtsi
+++ b/arch/arm/boot/dts/omap3-n950-n9.dtsi
@@ -11,13 +11,6 @@
 	cpus {
 		cpu@0 {
 			cpu0-supply = <&vcc>;
-			operating-points = <
-				/* kHz    uV */
-				300000  1012500
-				600000  1200000
-				800000  1325000
-				1000000	1375000
-			>;
 		};
 	};
 
-- 
2.19.1

Re: [RFC 4/5] ARM: dts: omap3-n950-n9: remove opp-v1 table

[Viresh Kumar]

On 02-09-19, 12:55, H. Nikolaus Schaller wrote:
> With opp-v2 in omap36xx.dtsi and ti-cpufreq driver the
> 1GHz capability is automatically detected.
> 
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> ---
>  arch/arm/boot/dts/omap3-n950-n9.dtsi | 7 -------
>  1 file changed, 7 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/omap3-n950-n9.dtsi b/arch/arm/boot/dts/omap3-n950-n9.dtsi
> index 5441e9ffdbb4..e98b0c615f19 100644
> --- a/arch/arm/boot/dts/omap3-n950-n9.dtsi
> +++ b/arch/arm/boot/dts/omap3-n950-n9.dtsi
> @@ -11,13 +11,6 @@
>  	cpus {
>  		cpu@0 {
>  			cpu0-supply = <&vcc>;
> -			operating-points = <
> -				/* kHz    uV */
> -				300000  1012500
> -				600000  1200000
> -				800000  1325000
> -				1000000	1375000
> -			>;
>  		};
>  	};

This should be merged with 2/5 ?

-- 
viresh

Re: [RFC 4/5] ARM: dts: omap3-n950-n9: remove opp-v1 table

[H. Nikolaus Schaller]

> Am 03.09.2019 um 04:36 schrieb Viresh Kumar <viresh.kumar@linaro.org>:
> 
> On 02-09-19, 12:55, H. Nikolaus Schaller wrote:
>> With opp-v2 in omap36xx.dtsi and ti-cpufreq driver the
>> 1GHz capability is automatically detected.
>> 
>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>> ---
>> arch/arm/boot/dts/omap3-n950-n9.dtsi | 7 -------
>> 1 file changed, 7 deletions(-)
>> 
>> diff --git a/arch/arm/boot/dts/omap3-n950-n9.dtsi b/arch/arm/boot/dts/omap3-n950-n9.dtsi
>> index 5441e9ffdbb4..e98b0c615f19 100644
>> --- a/arch/arm/boot/dts/omap3-n950-n9.dtsi
>> +++ b/arch/arm/boot/dts/omap3-n950-n9.dtsi
>> @@ -11,13 +11,6 @@
>> 	cpus {
>> 		cpu@0 {
>> 			cpu0-supply = <&vcc>;
>> -			operating-points = <
>> -				/* kHz    uV */
>> -				300000  1012500
>> -				600000  1200000
>> -				800000  1325000
>> -				1000000	1375000
>> -			>;
>> 		};
>> 	};
> 
> This should be merged with 2/5 ?

Well, it bloats 2/5.

What I hope (I can't test) is that this opp-v1 table
is ignored if an opp-v2 table exists. So that it can be
removed by a separate follow-up patch.

BR,
Nikolaus

Re: [RFC 4/5] ARM: dts: omap3-n950-n9: remove opp-v1 table

[Viresh Kumar]

On 03-09-19, 08:01, H. Nikolaus Schaller wrote:
> 
> > Am 03.09.2019 um 04:36 schrieb Viresh Kumar <viresh.kumar@linaro.org>:
> > 
> > On 02-09-19, 12:55, H. Nikolaus Schaller wrote:
> >> With opp-v2 in omap36xx.dtsi and ti-cpufreq driver the
> >> 1GHz capability is automatically detected.
> >> 
> >> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> >> ---
> >> arch/arm/boot/dts/omap3-n950-n9.dtsi | 7 -------
> >> 1 file changed, 7 deletions(-)
> >> 
> >> diff --git a/arch/arm/boot/dts/omap3-n950-n9.dtsi b/arch/arm/boot/dts/omap3-n950-n9.dtsi
> >> index 5441e9ffdbb4..e98b0c615f19 100644
> >> --- a/arch/arm/boot/dts/omap3-n950-n9.dtsi
> >> +++ b/arch/arm/boot/dts/omap3-n950-n9.dtsi
> >> @@ -11,13 +11,6 @@
> >> 	cpus {
> >> 		cpu@0 {
> >> 			cpu0-supply = <&vcc>;
> >> -			operating-points = <
> >> -				/* kHz    uV */
> >> -				300000  1012500
> >> -				600000  1200000
> >> -				800000  1325000
> >> -				1000000	1375000
> >> -			>;
> >> 		};
> >> 	};
> > 
> > This should be merged with 2/5 ?
> 
> Well, it bloats 2/5.

It is logically the right place to do this as that's where we are
adding opp-v2.

> What I hope (I can't test) is that this opp-v1 table
> is ignored if an opp-v2 table exists. So that it can be
> removed by a separate follow-up patch.

It should work as that's what we are doing in OPP core, but I still
feel this better get merged with 2/5.

-- 
viresh

Re: [RFC 4/5] ARM: dts: omap3-n950-n9: remove opp-v1 table

[H. Nikolaus Schaller]

> Am 03.09.2019 um 08:14 schrieb Viresh Kumar <viresh.kumar@linaro.org>:
> 
> On 03-09-19, 08:01, H. Nikolaus Schaller wrote:
>> 
>>> Am 03.09.2019 um 04:36 schrieb Viresh Kumar <viresh.kumar@linaro.org>:
>>> 
>>> On 02-09-19, 12:55, H. Nikolaus Schaller wrote:
>>>> With opp-v2 in omap36xx.dtsi and ti-cpufreq driver the
>>>> 1GHz capability is automatically detected.
>>>> 
>>>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>>>> ---
>>>> arch/arm/boot/dts/omap3-n950-n9.dtsi | 7 -------
>>>> 1 file changed, 7 deletions(-)
>>>> 
>>>> diff --git a/arch/arm/boot/dts/omap3-n950-n9.dtsi b/arch/arm/boot/dts/omap3-n950-n9.dtsi
>>>> index 5441e9ffdbb4..e98b0c615f19 100644
>>>> --- a/arch/arm/boot/dts/omap3-n950-n9.dtsi
>>>> +++ b/arch/arm/boot/dts/omap3-n950-n9.dtsi
>>>> @@ -11,13 +11,6 @@
>>>> 	cpus {
>>>> 		cpu@0 {
>>>> 			cpu0-supply = <&vcc>;
>>>> -			operating-points = <
>>>> -				/* kHz    uV */
>>>> -				300000  1012500
>>>> -				600000  1200000
>>>> -				800000  1325000
>>>> -				1000000	1375000
>>>> -			>;
>>>> 		};
>>>> 	};
>>> 
>>> This should be merged with 2/5 ?
>> 
>> Well, it bloats 2/5.
> 
> It is logically the right place to do this as that's where we are
> adding opp-v2.

Well, sometimes the philosophy of patches is to add something new
first and remove the old in a second separate patch if the system
can live with both. This makes it easier to digest single patches
(because they are smaller) and might also better pinpoint an issue
by bisect.

> 
>> What I hope (I can't test) is that this opp-v1 table
>> is ignored if an opp-v2 table exists. So that it can be
>> removed by a separate follow-up patch.
> 
> It should work as that's what we are doing in OPP core, but I still
> feel this better get merged with 2/5.

Ok, I see. Noted for RFCv2.

There will also be a big batch of changes for the compatible record
(omap3530->omap35xx, add omap34xx where needed) of ca. 10 board definition
DTS files. Should this then also become part of the new 2/5?

BR and thanks,
Nikolaus

Re: [RFC 4/5] ARM: dts: omap3-n950-n9: remove opp-v1 table

[Viresh Kumar]

On 03-09-19, 08:23, H. Nikolaus Schaller wrote:
> 
> > Am 03.09.2019 um 08:14 schrieb Viresh Kumar <viresh.kumar@linaro.org>:
> > 
> > On 03-09-19, 08:01, H. Nikolaus Schaller wrote:
> >> 
> >>> Am 03.09.2019 um 04:36 schrieb Viresh Kumar <viresh.kumar@linaro.org>:
> >>> 
> >>> On 02-09-19, 12:55, H. Nikolaus Schaller wrote:
> >>>> With opp-v2 in omap36xx.dtsi and ti-cpufreq driver the
> >>>> 1GHz capability is automatically detected.
> >>>> 
> >>>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> >>>> ---
> >>>> arch/arm/boot/dts/omap3-n950-n9.dtsi | 7 -------
> >>>> 1 file changed, 7 deletions(-)
> >>>> 
> >>>> diff --git a/arch/arm/boot/dts/omap3-n950-n9.dtsi b/arch/arm/boot/dts/omap3-n950-n9.dtsi
> >>>> index 5441e9ffdbb4..e98b0c615f19 100644
> >>>> --- a/arch/arm/boot/dts/omap3-n950-n9.dtsi
> >>>> +++ b/arch/arm/boot/dts/omap3-n950-n9.dtsi
> >>>> @@ -11,13 +11,6 @@
> >>>> 	cpus {
> >>>> 		cpu@0 {
> >>>> 			cpu0-supply = <&vcc>;
> >>>> -			operating-points = <
> >>>> -				/* kHz    uV */
> >>>> -				300000  1012500
> >>>> -				600000  1200000
> >>>> -				800000  1325000
> >>>> -				1000000	1375000
> >>>> -			>;
> >>>> 		};
> >>>> 	};
> >>> 
> >>> This should be merged with 2/5 ?
> >> 
> >> Well, it bloats 2/5.
> > 
> > It is logically the right place to do this as that's where we are
> > adding opp-v2.
> 
> Well, sometimes the philosophy of patches is to add something new
> first and remove the old in a second separate patch if the system
> can live with both. This makes it easier to digest single patches
> (because they are smaller) and might also better pinpoint an issue
> by bisect.

Right, but you already removed some of the opp-v1 stuff in patch 2/5.
Why leave this one out ?

> > 
> >> What I hope (I can't test) is that this opp-v1 table
> >> is ignored if an opp-v2 table exists. So that it can be
> >> removed by a separate follow-up patch.
> > 
> > It should work as that's what we are doing in OPP core, but I still
> > feel this better get merged with 2/5.
> 
> Ok, I see. Noted for RFCv2.
> 
> There will also be a big batch of changes for the compatible record
> (omap3530->omap35xx, add omap34xx where needed) of ca. 10 board definition
> DTS files. Should this then also become part of the new 2/5?

Compatible thing should be separate patch anyway, I was just talking
about replacing opp-v1 with v2.

-- 
viresh

Re: [RFC 4/5] ARM: dts: omap3-n950-n9: remove opp-v1 table

[H. Nikolaus Schaller]

> Am 03.09.2019 um 08:28 schrieb Viresh Kumar <viresh.kumar@linaro.org>:
> 
> On 03-09-19, 08:23, H. Nikolaus Schaller wrote:
>> 
>>> Am 03.09.2019 um 08:14 schrieb Viresh Kumar <viresh.kumar@linaro.org>:
>>> 
>>> On 03-09-19, 08:01, H. Nikolaus Schaller wrote:
>>>> 
>>>>> Am 03.09.2019 um 04:36 schrieb Viresh Kumar <viresh.kumar@linaro.org>:
>>>>> 
>>>>> On 02-09-19, 12:55, H. Nikolaus Schaller wrote:
>>>>>> With opp-v2 in omap36xx.dtsi and ti-cpufreq driver the
>>>>>> 1GHz capability is automatically detected.
>>>>>> 
>>>>>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>>>>>> ---
>>>>>> arch/arm/boot/dts/omap3-n950-n9.dtsi | 7 -------
>>>>>> 1 file changed, 7 deletions(-)
>>>>>> 
>>>>>> diff --git a/arch/arm/boot/dts/omap3-n950-n9.dtsi b/arch/arm/boot/dts/omap3-n950-n9.dtsi
>>>>>> index 5441e9ffdbb4..e98b0c615f19 100644
>>>>>> --- a/arch/arm/boot/dts/omap3-n950-n9.dtsi
>>>>>> +++ b/arch/arm/boot/dts/omap3-n950-n9.dtsi
>>>>>> @@ -11,13 +11,6 @@
>>>>>> 	cpus {
>>>>>> 		cpu@0 {
>>>>>> 			cpu0-supply = <&vcc>;
>>>>>> -			operating-points = <
>>>>>> -				/* kHz    uV */
>>>>>> -				300000  1012500
>>>>>> -				600000  1200000
>>>>>> -				800000  1325000
>>>>>> -				1000000	1375000
>>>>>> -			>;
>>>>>> 		};
>>>>>> 	};
>>>>> 
>>>>> This should be merged with 2/5 ?
>>>> 
>>>> Well, it bloats 2/5.
>>> 
>>> It is logically the right place to do this as that's where we are
>>> adding opp-v2.
>> 
>> Well, sometimes the philosophy of patches is to add something new
>> first and remove the old in a second separate patch if the system
>> can live with both. This makes it easier to digest single patches
>> (because they are smaller) and might also better pinpoint an issue
>> by bisect.
> 
> Right, but you already removed some of the opp-v1 stuff in patch 2/5.
> Why leave this one out ?
> 
>>> 
>>>> What I hope (I can't test) is that this opp-v1 table
>>>> is ignored if an opp-v2 table exists. So that it can be
>>>> removed by a separate follow-up patch.
>>> 
>>> It should work as that's what we are doing in OPP core, but I still
>>> feel this better get merged with 2/5.
>> 
>> Ok, I see. Noted for RFCv2.
>> 
>> There will also be a big batch of changes for the compatible record
>> (omap3530->omap35xx, add omap34xx where needed) of ca. 10 board definition
>> DTS files. Should this then also become part of the new 2/5?
> 
> Compatible thing should be separate patch anyway, I was just talking
> about replacing opp-v1 with v2.

Ok, understood.

BR and thanks,
Nikolaus

[RFC 5/5] ARM: dts: omap3-beagle: make explicitly compatible to ti,omap34xx

[H. Nikolaus Schaller]

Matching the ti-cpufreq driver needs to specify explicitly if
a board uses an omap34xx or omap36xx chip.

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 arch/arm/boot/dts/omap3-beagle.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/omap3-beagle.dts b/arch/arm/boot/dts/omap3-beagle.dts
index e3df3c166902..d47213c7a4d0 100644
--- a/arch/arm/boot/dts/omap3-beagle.dts
+++ b/arch/arm/boot/dts/omap3-beagle.dts
@@ -8,7 +8,7 @@
 
 / {
 	model = "TI OMAP3 BeagleBoard";
-	compatible = "ti,omap3-beagle", "ti,omap3";
+	compatible = "ti,omap3-beagle", "ti,omap34xx", "ti,omap3";
 
 	cpus {
 		cpu@0 {
-- 
2.19.1

Re: [RFC 5/5] ARM: dts: omap3-beagle: make explicitly compatible to ti,omap34xx

[Tony Lindgren]

* H. Nikolaus Schaller <hns@goldelico.com> [190902 10:56]:
> Matching the ti-cpufreq driver needs to specify explicitly if
> a board uses an omap34xx or omap36xx chip.
> 
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> ---
>  arch/arm/boot/dts/omap3-beagle.dts | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/omap3-beagle.dts b/arch/arm/boot/dts/omap3-beagle.dts
> index e3df3c166902..d47213c7a4d0 100644
> --- a/arch/arm/boot/dts/omap3-beagle.dts
> +++ b/arch/arm/boot/dts/omap3-beagle.dts
> @@ -8,7 +8,7 @@
>  
>  / {
>  	model = "TI OMAP3 BeagleBoard";
> -	compatible = "ti,omap3-beagle", "ti,omap3";
> +	compatible = "ti,omap3-beagle", "ti,omap34xx", "ti,omap3";
>  
>  	cpus {
>  		cpu@0 {

For a clean-up patch, we should just use the following compatibles
in general for omap3:

ti,omap3	omap3
ti,omap34	omap34xx and omap35xx
ti,omap36	omap36xx and dm37xx
ti,am35		am35xx

So we should just leave out the "xx" part. But we still need parse
also the legacy binding with "xx" in drivers.

Regards,

Tony

Re: [RFC 5/5] ARM: dts: omap3-beagle: make explicitly compatible to ti,omap34xx

[H. Nikolaus Schaller]

Hi Tony,

> Am 03.09.2019 um 15:40 schrieb Tony Lindgren <tony@atomide.com>:
> 
> * H. Nikolaus Schaller <hns@goldelico.com> [190902 10:56]:
>> Matching the ti-cpufreq driver needs to specify explicitly if
>> a board uses an omap34xx or omap36xx chip.
>> 
>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>> ---
>> arch/arm/boot/dts/omap3-beagle.dts | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/arch/arm/boot/dts/omap3-beagle.dts b/arch/arm/boot/dts/omap3-beagle.dts
>> index e3df3c166902..d47213c7a4d0 100644
>> --- a/arch/arm/boot/dts/omap3-beagle.dts
>> +++ b/arch/arm/boot/dts/omap3-beagle.dts
>> @@ -8,7 +8,7 @@
>> 
>> / {
>> 	model = "TI OMAP3 BeagleBoard";
>> -	compatible = "ti,omap3-beagle", "ti,omap3";
>> +	compatible = "ti,omap3-beagle", "ti,omap34xx", "ti,omap3";
>> 
>> 	cpus {
>> 		cpu@0 {
> 
> For a clean-up patch, we should just use the following compatibles
> in general for omap3:
> 
> ti,omap3	omap3
> ti,omap34	omap34xx and omap35xx
> ti,omap36	omap36xx and dm37xx
> ti,am35		am35xx
> 
> So we should just leave out the "xx" part. But we still need parse
> also the legacy binding with "xx" in drivers.

Yes, getting rid of the xx in the device trees would be nice.

But I have looked through the kernel tree and that makes more problems
than it solves...

There is code that explicitly checks for "ti,omap36xx" in drivers/clk/ti/dpll.c

Maybe that should be replaced by a check for a "ti,dpll5" property
which is only defined in omap36xx.dtsi to make the code depend
on a feature rather than SoC family string. Although such a change
may break other things or we have to keep even more legacy code around.

And there is a binding document omap.txt that does not describe
it the way you propose. IMHO that should be changed first.

Next is that omap.txt explicitly says that ti,omap3 defauls to OMAP3430.
And OMAP3430 should be specified as compatible = "ti,omap3430", "ti,omap3"
while OMAP3630 should have compatible = "ti,omap36xx", "ti,omap3".
AM3517 is compatible = "ti,am3517", "ti,omap3" which seems to ignore
the AM3505 (maybe there was never a board using it).

So this clean up is much more than what we need for moving from
opp-v1 to opp-v2 and it should therefore be addressed separately.

Therefore I'd prefer if we can keep all these problems isolated
into a separate set of patches. And because of this I have prepared
an RFC-V2 which only adds the "ti,omap3430" to those boards which
do not yet have it (in accordance to existing omap.txt).

AM3517 does not seem to have any opp table and therefore I also
exclude it at the moment.

BR and thanks,
Nikolaus