OMAP3: convert opp-v1 to opp-v2 and read speed binned / 720MHz grade bits

RFC Superseded H. Nikolaus Schaller 4 participants 21 messages Sep 4, 2019 – Sep 7, 2019

Subsystems (2)

Device tree bindings CPU frequency scaling framework

lore

Share & tools

lore

H. Nikolaus Schaller

author

2019-09-04 08:53 UTC #0 lore raw

v2 cover OMAP3: convert opp-v1 to opp-v2 and read speed binned / 720MHz grade bits

Changes V2:
* merge separate patch to remove opp-v1 table from n950-n9 into the general omap3xxx.dtsi patch
* add legacy compatibility to ti,omap3430 and ti,omap3630 for the ti-cpufreq driver
* make driver and omap3xxx.dtsi patches pass checkpatch
* add bulk patch to explicitly define compatibility to ti,omap3430 and ti,omap36xx in addition to ti,omap3 of all in-tree boards where it was missing

RFC V1 2019-09-02 12:55:55:

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 the high speed OPPs) on:

* BeagleBoard C2 (omap3430 600MHz)
* BeagleBoard XM B (dm3730 800MHz)
* GTA04A4 (dm3730 800MHz)
* GTA04A5 (dm3730 1GHz)

H. Nikolaus Schaller (3):
cpufreq: ti-cpufreq: add support for omap34xx and omap36xx
ARM: dts: replace opp-v1 tables by opp-v2 for omap34xx and omap36xx
ARM: dts: omap3: bulk convert compatible to be explicitly ti,omap3430
or ti,omap36xx

 arch/arm/boot/dts/am3517_mt_ventoux.dts       |  2 +-
 .../boot/dts/logicpd-som-lv-35xx-devkit.dts   |  2 +-
 .../boot/dts/logicpd-som-lv-37xx-devkit.dts   |  2 +-
 .../boot/dts/logicpd-torpedo-35xx-devkit.dts  |  2 +-
 .../boot/dts/logicpd-torpedo-37xx-devkit.dts  |  2 +-
 arch/arm/boot/dts/omap3-beagle.dts            |  2 +-
 arch/arm/boot/dts/omap3-cm-t3530.dts          |  2 +-
 arch/arm/boot/dts/omap3-devkit8000-lcd43.dts  |  2 +-
 arch/arm/boot/dts/omap3-devkit8000-lcd70.dts  |  2 +-
 arch/arm/boot/dts/omap3-devkit8000.dts        |  2 +-
 arch/arm/boot/dts/omap3-evm-37xx.dts          |  2 +-
 arch/arm/boot/dts/omap3-ha-lcd.dts            |  2 +-
 arch/arm/boot/dts/omap3-ha.dts                |  2 +-
 arch/arm/boot/dts/omap3-ldp.dts               |  2 +-
 arch/arm/boot/dts/omap3-n950-n9.dtsi          |  7 --
 arch/arm/boot/dts/omap3-sbc-t3530.dts         |  2 +-
 arch/arm/boot/dts/omap3-thunder.dts           |  2 +-
 arch/arm/boot/dts/omap3430-sdp.dts            |  2 +-
 arch/arm/boot/dts/omap34xx.dtsi               | 65 ++++++++++++--
 arch/arm/boot/dts/omap36xx.dtsi               | 53 +++++++++--
 drivers/cpufreq/cpufreq-dt-platdev.c          |  2 +-
 drivers/cpufreq/ti-cpufreq.c                  | 87 ++++++++++++++++++-
 22 files changed, 204 insertions(+), 44 deletions(-)

Signature · 1 line

2.19.1

H. Nikolaus Schaller

author

2019-09-04 08:53 UTC #1 lore raw

RFC v2 1/3 cpufreq: ti-cpufreq: add support for omap34xx and omap36xx

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 am33xx, am43, dra7), we add code to directly read the register values using ioremap() if syscon access fails.

Signed-off-by H. Nikolaus Schaller <[email protected]>

---
 drivers/cpufreq/ti-cpufreq.c | 87 +++++++++++++++++++++++++++++++++++-
 1 file changed, 85 insertions(+), 2 deletions(-)

drivers/cpufreq/ti-cpufreq.c +85 -2

index 2ad1ae17932d..b3de3162ea73 100644
@@ -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,11 @@ 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,omap34xx", .data = &omap34xx_soc_data, },
+	{ .compatible = "ti,omap36xx", .data = &omap36xx_soc_data, },
+	/* legacy */
+	{ .compatible = "ti,omap3430", .data = &omap34xx_soc_data, },
+	{ .compatible = "ti,omap3630", .data = &omap36xx_soc_data, },
 	{},
 };

Signature · 1 line

2.19.1

Tony Lindgren

Acked-by on v2 1/3

2019-09-05 14:32 UTC #2 lore raw

Re: [RFC v2 1/3] cpufreq: ti-cpufreq: add support for omap34xx and omap36xx

Quoted 7 lines · 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 am33xx, am43, dra7), we add code to directly read the register values using ioremap() if syscon access fails.

This is nice :) Seems to work for me based on a quick test on at least omap36xx.

Looks like n900 produces the following though:

core: _opp_supported_by_regulators: OPP minuV: 1270000 maxuV: 1270000, not supported by regulator cpu cpu0: _opp_add: OPP not supported by regulators (550000000)

But presumably that can be further patched. So for this patch:

Acked-by Tony Lindgren <[email protected]>

Viresh Kumar

maintainer

2019-09-06 03:01 UTC #3 lore raw

Re: [RFC v2 1/3] cpufreq: ti-cpufreq: add support for omap34xx and omap36xx

Quoted 9 lines · Tony Lindgren

Quoted 7 lines · 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 am33xx, am43, dra7), we add code to directly read the register values using ioremap() if syscon access fails.

Quoted 7 lines

This is nice :) Seems to work for me based on a quick test on at least omap36xx.
Looks like n900 produces the following though:
core: _opp_supported_by_regulators: OPP minuV: 1270000 maxuV: 1270000, not supported by regulator cpu cpu0: _opp_add: OPP not supported by regulators (550000000)

That's a DT thing I believe where the voltage doesn't fit what the regulator can support.

Quoted 4 lines

But presumably that can be further patched. So for this patch:
Acked-by Tony Lindgren <[email protected]>

Thanks.

Signature · 1 line

viresh

H. Nikolaus Schaller

author

2019-09-06 20:46 UTC #4 lore raw

Re: [RFC v2 1/3] cpufreq: ti-cpufreq: add support for omap34xx and omap36xx

Hi,

Quoted 15 lines

Am 06.09.2019 um 05:01 schrieb Viresh Kumar <[email protected]>:
Quoted 12 lines · Tony Lindgren
Quoted 7 lines · 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 am33xx, am43, dra7), we add code to directly read the register values using ioremap() if syscon access fails.
This is nice :) Seems to work for me based on a quick test on at least omap36xx.

Quoted 7 lines

Quoted 4 lines
Looks like n900 produces the following though:
core: _opp_supported_by_regulators: OPP minuV: 1270000 maxuV: 1270000, not supported by regulator cpu cpu0: _opp_add: OPP not supported by regulators (550000000)
That's a DT thing I believe where the voltage doesn't fit what the regulator can support.

I can confirm this on BeagleBoard C2:

root@gta04:~# dmesg|fgrep -i opp
[    2.347442] core: _opp_supported_by_regulators: OPP minuV: 1270000 maxuV: 1270000, not supported by regulator
[    2.359222] cpu cpu0: _opp_add: OPP not supported by regulators (550000000)
[    2.580993] omap2_set_init_voltage: unable to find boot up OPP for vdd_core
root@gta04:~#

Quoted 2 lines

Quoted 2 lines
But presumably that can be further patched.

Well, the opp-v1 table also has this voltage point:

			/* OMAP343x/OMAP35xx variants OPP1-5 */
			operating-points = <
				/* kHz    uV */
				125000   975000
				250000  1075000
				500000  1200000
				550000  1270000
				600000  1350000
			>;

This is OPP4 which is recommended by OMAP3530 data sheet to be 1.27V +/- 5%

Data sheet of tps65950 says

• VDD1: 1.2-A, buck DC/DC converter (VOUT = 0.6 V to 1.45 V, in steps of 12.5 mV)

This means 1270 mV is not a "step" and rejected by the twl4030 driver. Maybe nobody did notice yet because the opp-v1 drivers did not warn...

The closest value to 1.27V is 0.6V + 54 * 12.5mV is 1.275V

So let's also change the OPP4 to 1275000 uV in the opp-v2 table.

BR and thanks,
Nikolaus

H. Nikolaus Schaller

author

2019-09-07 06:34 UTC #5 lore raw

Re: [RFC v2 1/3] cpufreq: ti-cpufreq: add support for omap34xx and omap36xx

Quoted 56 lines

Am 06.09.2019 um 22:46 schrieb H. Nikolaus Schaller <[email protected]>:
Hi,
Quoted 15 lines
Am 06.09.2019 um 05:01 schrieb Viresh Kumar <[email protected]>:
Quoted 12 lines · Tony Lindgren
Quoted 7 lines · 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 am33xx, am43, dra7), we add code to directly read the register values using ioremap() if syscon access fails.
This is nice :) Seems to work for me based on a quick test on at least omap36xx.
Quoted 7 lines
Quoted 4 lines
Looks like n900 produces the following though:
core: _opp_supported_by_regulators: OPP minuV: 1270000 maxuV: 1270000, not supported by regulator cpu cpu0: _opp_add: OPP not supported by regulators (550000000)
That's a DT thing I believe where the voltage doesn't fit what the regulator can support.
I can confirm this on BeagleBoard C2:
root@gta04:~# dmesg|fgrep -i opp
[    2.347442] core: _opp_supported_by_regulators: OPP minuV: 1270000 maxuV: 1270000, not supported by regulator
[    2.359222] cpu cpu0: _opp_add: OPP not supported by regulators (550000000)
[    2.580993] omap2_set_init_voltage: unable to find boot up OPP for vdd_core
root@gta04:~# 
Quoted 2 lines
Quoted 2 lines
But presumably that can be further patched.
Well, the opp-v1 table also has this voltage point:
			/* OMAP343x/OMAP35xx variants OPP1-5 */
			operating-points = <
				/* kHz    uV */
				125000   975000
				250000  1075000
				500000  1200000
				550000  1270000
				600000  1350000
			>;
This is OPP4 which is recommended by OMAP3530 data sheet to be 1.27V +/- 5%
Data sheet of tps65950 says

Quoted 8 lines

• VDD1: 1.2-A, buck DC/DC converter (VOUT = 0.6 V to 1.45 V, in steps of 12.5 mV)
This means 1270 mV is not a "step" and rejected by the twl4030 driver. Maybe nobody did notice yet because the opp-v1 drivers did not warn...
The closest value to 1.27V is 0.6V + 54 * 12.5mV is 1.275V
So let's also change the OPP4 to 1275000 uV in the opp-v2 table.

The OPP is now available. Only

[    2.569519] omap2_set_init_voltage: unable to find boot up OPP for vdd_core

remains but this is a different issue (mismatch between U-Boot clock/vdd_core setup and kernel table). Most likely U-Boot runs with an 300MHz OPP which is not defined by data sheet or kernel opp tables.

BR,
Nikolaus

Andreas Kemnade

2019-09-07 07:19 UTC #6 lore raw

Re: [Letux-kernel] [RFC v2 1/3] cpufreq: ti-cpufreq: add support for omap34xx and omap36xx

In reply to H. Nikolaus Schaller

Quoted 49 lines · On Fri, 6 Sep 2019 22:46:49 +0200 "H. Nikolaus Schaller" <[email protected]>…

Hi,
Quoted 15 lines
Am 06.09.2019 um 05:01 schrieb Viresh Kumar <[email protected]>:
Quoted 12 lines · Tony Lindgren
Quoted 7 lines · 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 am33xx, am43, dra7), we add code to directly read the register values using ioremap() if syscon access fails.
This is nice :) Seems to work for me based on a quick test on at least omap36xx.
Quoted 7 lines
Quoted 4 lines
Looks like n900 produces the following though:
core: _opp_supported_by_regulators: OPP minuV: 1270000 maxuV: 1270000, not supported by regulator cpu cpu0: _opp_add: OPP not supported by regulators (550000000)
That's a DT thing I believe where the voltage doesn't fit what the regulator can support.
I can confirm this on BeagleBoard C2:
root@gta04:~# dmesg|fgrep -i opp
[    2.347442] core: _opp_supported_by_regulators: OPP minuV: 1270000 maxuV: 1270000, not supported by regulator
[    2.359222] cpu cpu0: _opp_add: OPP not supported by regulators (550000000)
[    2.580993] omap2_set_init_voltage: unable to find boot up OPP for vdd_core
root@gta04:~# 
Quoted 2 lines
Quoted 2 lines
But presumably that can be further patched.
Well, the opp-v1 table also has this voltage point:
			/* OMAP343x/OMAP35xx variants OPP1-5 */
			operating-points = <
				/* kHz    uV */
				125000   975000
				250000  1075000
				500000  1200000
				550000  1270000
				600000  1350000
			>;  

Quoted 9 lines

This is OPP4 which is recommended by OMAP3530 data sheet to be 1.27V +/- 5%
Data sheet of tps65950 says
• VDD1: 1.2-A, buck DC/DC converter (VOUT = 0.6 V to 1.45 V, in steps of 12.5 mV)
This means 1270 mV is not a "step" and rejected by the twl4030 driver. Maybe nobody did notice yet because the opp-v1 drivers did not warn...

The reason probably is that errors about supported voltages were handled incorrecly in opp code in former times. Then someone fixed and cpufreq did not work on omap3 at all due to twl-regulator not specifying voltages for VDD1.
Then I did a fix "regulator: twl: voltage lists for vdd1/2 on twl4030" which is still living in linux-next/pending-fixes (and probably also in Nikolaus's trees). Mark Brown
did apparently not send his pull request.

As a side effect of all that voltage checking corrections these errors are unveiled.

Regards,
Andreas

Viresh Kumar

maintainer

2019-09-06 03:04 UTC #7 lore raw

Re: [RFC v2 1/3] cpufreq: ti-cpufreq: add support for omap34xx and omap36xx

In reply to Tony Lindgren

Quoted 1 line · Tony Lindgren

Acked-by Tony Lindgren <[email protected]>

Do you want to pick the series instead as this has lots of DT changes ?

Signature · 1 line

viresh

Tony Lindgren

2019-09-06 15:49 UTC #8 lore raw

Re: [RFC v2 1/3] cpufreq: ti-cpufreq: add support for omap34xx and omap36xx

Quoted 5 lines · Viresh Kumar

Quoted 1 line · Tony Lindgren
Acked-by Tony Lindgren <[email protected]>
Do you want to pick the series instead as this has lots of DT changes ?

It unlikely these dts changes will conflict with anything so I have no problem acking them for you for the next set of patches.

Regards,

Tony

H. Nikolaus Schaller

author

2019-09-04 08:53 UTC #9 lore raw

RFC v2 2/3 ARM: dts: replace opp-v1 tables by opp-v2 for omap34xx and omap36xx

In reply to 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.

We also can remove opp-v1 table for omap3-n950-n9 since it is now automatically detected.

Signed-off-by H. Nikolaus Schaller <[email protected]>

---
 arch/arm/boot/dts/omap3-n950-n9.dtsi |  7 ---
 arch/arm/boot/dts/omap34xx.dtsi      | 65 ++++++++++++++++++++++++----
 arch/arm/boot/dts/omap36xx.dtsi      | 53 +++++++++++++++++++----
 drivers/cpufreq/cpufreq-dt-platdev.c |  2 +-
 4 files changed, 102 insertions(+), 25 deletions(-)

arch/arm/boot/dts/omap3-n950-n9.dtsi +0 -7

index 5441e9ffdbb4..e98b0c615f19 100644
@@ -11,13 +11,6 @@
 	cpus {
 		cpu@0 {
 			cpu0-supply = <&vcc>;
-			operating-points = <
-				/* kHz    uV */
-				300000  1012500
-				600000  1200000
-				800000  1325000
-				1000000	1375000
-			>;
 		};
 	};

arch/arm/boot/dts/omap34xx.dtsi +56 -9

75 lines not on this page · raw message

arch/arm/boot/dts/omap36xx.dtsi +45 -8

60 lines not on this page · raw message

drivers/cpufreq/cpufreq-dt-platdev.c +1 -1

index 03dc4244ab00..68b7fc4225f8 100644
@@ -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", },
 
 	{ }
 };

Signature · 1 line

2.19.1

H. Nikolaus Schaller

author

2019-09-04 08:53 UTC #10 lore raw

RFC v2 3/3 ARM: dts: omap3: bulk convert compatible to be explicitly ti,omap3430 or ti,omap36xx

In reply to H. Nikolaus Schaller

According to omap.txt bindings documentation, matching the ti-cpufreq driver needs to specify explicitly if a board uses an omap3430 or omap36xx chip.

This needs to add ti,omap3430 to most omap34xx boards and replace ti,omap3630 by ti,omap36xx for some omap36xx boards (most others already have done it right).

We also clean up some instances of missing ti,am3517 so that we can rely on seeing either one of:

ti,am3517
ti,omap34xx
ti,omap36xx

in addition to ti,omap3.

Signed-off-by H. Nikolaus Schaller <[email protected]>

---
 arch/arm/boot/dts/am3517_mt_ventoux.dts           | 2 +-
 arch/arm/boot/dts/logicpd-som-lv-35xx-devkit.dts  | 2 +-
 arch/arm/boot/dts/logicpd-som-lv-37xx-devkit.dts  | 2 +-
 arch/arm/boot/dts/logicpd-torpedo-35xx-devkit.dts | 2 +-
 arch/arm/boot/dts/logicpd-torpedo-37xx-devkit.dts | 2 +-
 arch/arm/boot/dts/omap3-beagle.dts                | 2 +-
 arch/arm/boot/dts/omap3-cm-t3530.dts              | 2 +-
 arch/arm/boot/dts/omap3-devkit8000-lcd43.dts      | 2 +-
 arch/arm/boot/dts/omap3-devkit8000-lcd70.dts      | 2 +-
 arch/arm/boot/dts/omap3-devkit8000.dts            | 2 +-
 arch/arm/boot/dts/omap3-evm-37xx.dts              | 2 +-
 arch/arm/boot/dts/omap3-ha-lcd.dts                | 2 +-
 arch/arm/boot/dts/omap3-ha.dts                    | 2 +-
 arch/arm/boot/dts/omap3-ldp.dts                   | 2 +-
 arch/arm/boot/dts/omap3-sbc-t3530.dts             | 2 +-
 arch/arm/boot/dts/omap3-thunder.dts               | 2 +-
 arch/arm/boot/dts/omap3430-sdp.dts                | 2 +-
 17 files changed, 17 insertions(+), 17 deletions(-)

arch/arm/boot/dts/am3517_mt_ventoux.dts +1 -1

index e507e4ae0d88..e7d7124a34ba 100644
@@ -8,7 +8,7 @@
 
 / {
 	model = "TeeJet Mt.Ventoux";
-	compatible = "teejet,mt_ventoux", "ti,omap3";
+	compatible = "teejet,mt_ventoux", "ti,am3517", "ti,omap3";
 
 	memory@80000000 {
 		device_type = "memory";

arch/arm/boot/dts/logicpd-som-lv-35xx-devkit.dts +1 -1

index f7a841a28865..2a0a98fe67f0 100644
@@ -9,5 +9,5 @@
 
 / {
 	model = "LogicPD Zoom OMAP35xx SOM-LV Development Kit";
-	compatible = "logicpd,dm3730-som-lv-devkit", "ti,omap3";
+	compatible = "logicpd,dm3730-som-lv-devkit", "ti,omap3430", "ti,omap3";
 };

arch/arm/boot/dts/logicpd-som-lv-37xx-devkit.dts +1 -1

index a604d92221a4..ea441acf000d 100644
@@ -9,5 +9,5 @@
 
 / {
 	model = "LogicPD Zoom DM3730 SOM-LV Development Kit";
-	compatible = "logicpd,dm3730-som-lv-devkit", "ti,omap3630", "ti,omap3";
+	compatible = "logicpd,dm3730-som-lv-devkit", "ti,omap36xx", "ti,omap3";
 };

arch/arm/boot/dts/logicpd-torpedo-35xx-devkit.dts +1 -1

index 7675bc3fa868..57bae2aa910e 100644
@@ -9,5 +9,5 @@
 
 / {
 	model = "LogicPD Zoom OMAP35xx Torpedo Development Kit";
-	compatible = "logicpd,dm3730-torpedo-devkit", "ti,omap3";
+	compatible = "logicpd,dm3730-torpedo-devkit", "ti,omap3430", "ti,omap3";
 };

arch/arm/boot/dts/logicpd-torpedo-37xx-devkit.dts +1 -1

index 18c27e85051f..1937a1c36ad3 100644
@@ -9,7 +9,7 @@
 
 / {
 	model = "LogicPD Zoom DM3730 Torpedo + Wireless Development Kit";
-	compatible = "logicpd,dm3730-torpedo-devkit", "ti,omap3630", "ti,omap3";
+	compatible = "logicpd,dm3730-torpedo-devkit", "ti,omap36xx", "ti,omap3";
 
 	wl12xx_vmmc: wl12xx_vmmc {
 		compatible = "regulator-fixed";

arch/arm/boot/dts/omap3-beagle.dts +1 -1

index e3df3c166902..4ed3f93f5841 100644
@@ -8,7 +8,7 @@
 
 / {
 	model = "TI OMAP3 BeagleBoard";
-	compatible = "ti,omap3-beagle", "ti,omap3";
+	compatible = "ti,omap3-beagle", "ti,omap3430", "ti,omap3";
 
 	cpus {
 		cpu@0 {

arch/arm/boot/dts/omap3-cm-t3530.dts +1 -1

index 76e52c78cbb4..f13644a776aa 100644
@@ -9,7 +9,7 @@
 
 / {
 	model = "CompuLab CM-T3530";
-	compatible = "compulab,omap3-cm-t3530", "ti,omap34xx", "ti,omap3";
+	compatible = "compulab,omap3-cm-t3530", "ti,omap3430", "ti,omap3";
 
 	/* Regulator to trigger the reset signal of the Wifi module */
 	mmc2_sdio_reset: regulator-mmc2-sdio-reset {

arch/arm/boot/dts/omap3-devkit8000-lcd43.dts +1 -1

index a80fc60bc773..afed85078ad8 100644
@@ -11,7 +11,7 @@
 #include "omap3-devkit8000-lcd-common.dtsi"
 / {
 	model = "TimLL OMAP3 Devkit8000 with 4.3'' LCD panel";
-	compatible = "timll,omap3-devkit8000", "ti,omap3";
+	compatible = "timll,omap3-devkit8000", "ti,omap3430", "ti,omap3";
 
 	lcd0: display {
 		panel-timing {

arch/arm/boot/dts/omap3-devkit8000-lcd70.dts +1 -1

index 0753776071f8..07c51a105c0d 100644
@@ -11,7 +11,7 @@
 #include "omap3-devkit8000-lcd-common.dtsi"
 / {
 	model = "TimLL OMAP3 Devkit8000 with 7.0'' LCD panel";
-	compatible = "timll,omap3-devkit8000", "ti,omap3";
+	compatible = "timll,omap3-devkit8000", "ti,omap3430", "ti,omap3";
 
 	lcd0: display {
 		panel-timing {

arch/arm/boot/dts/omap3-devkit8000.dts +1 -1

index faafc48d8f61..162d0726b008 100644
@@ -7,7 +7,7 @@
 #include "omap3-devkit8000-common.dtsi"
 / {
 	model = "TimLL OMAP3 Devkit8000";
-	compatible = "timll,omap3-devkit8000", "ti,omap3";
+	compatible = "timll,omap3-devkit8000", "ti,omap3430", "ti,omap3";
 
 	aliases {
 		display1 = &dvi0;

arch/arm/boot/dts/omap3-evm-37xx.dts +1 -1

index e0c0382388f0..311550ee4eae 100644
@@ -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 {

arch/arm/boot/dts/omap3-ha-lcd.dts +1 -1

index badb9b3c8897..74751798c00e 100644
@@ -8,7 +8,7 @@
 
 / {
 	model = "TI OMAP3 HEAD acoustics LCD-baseboard with TAO3530 SOM";
-	compatible = "headacoustics,omap3-ha-lcd", "technexion,omap3-tao3530", "ti,omap34xx", "ti,omap3";
+	compatible = "headacoustics,omap3-ha-lcd", "technexion,omap3-tao3530", "ti,omap3430", "ti,omap3";
 };
 
 &omap3_pmx_core {

arch/arm/boot/dts/omap3-ha.dts +1 -1

index a5365252bfbe..f36cbc0a3586 100644
@@ -8,7 +8,7 @@
 
 / {
 	model = "TI OMAP3 HEAD acoustics baseboard with TAO3530 SOM";
-	compatible = "headacoustics,omap3-ha", "technexion,omap3-tao3530", "ti,omap34xx", "ti,omap3";
+	compatible = "headacoustics,omap3-ha", "technexion,omap3-tao3530", "ti,omap3430", "ti,omap3";
 };
 
 &omap3_pmx_core {

arch/arm/boot/dts/omap3-ldp.dts +1 -1

index 9a5fde2d9bce..9947574bd0f8 100644
@@ -10,7 +10,7 @@
 
 / {
 	model = "TI OMAP3430 LDP (Zoom1 Labrador)";
-	compatible = "ti,omap3-ldp", "ti,omap3";
+	compatible = "ti,omap3-ldp", "ti,omap34xx, "ti,omap3";
 
 	memory@80000000 {
 		device_type = "memory";

arch/arm/boot/dts/omap3-sbc-t3530.dts +1 -1

index ae96002abb3b..19582ef68e60 100644
@@ -8,7 +8,7 @@
 
 / {
 	model = "CompuLab SBC-T3530 with CM-T3530";
-	compatible = "compulab,omap3-sbc-t3530", "compulab,omap3-cm-t3530", "ti,omap34xx", "ti,omap3";
+	compatible = "compulab,omap3-sbc-t3530", "compulab,omap3-cm-t3530", "ti,omap3430", "ti,omap3";
 
 	aliases {
 		display0 = &dvi0;

arch/arm/boot/dts/omap3-thunder.dts +1 -1

index 6276e7079b36..b12ff8ecae50 100644
@@ -8,7 +8,7 @@
 
 / {
 	model = "TI OMAP3 Thunder baseboard with TAO3530 SOM";
-	compatible = "technexion,omap3-thunder", "technexion,omap3-tao3530", "ti,omap34xx", "ti,omap3";
+	compatible = "technexion,omap3-thunder", "technexion,omap3-tao3530", "ti,omap3430", "ti,omap3";
 };
 
 &omap3_pmx_core {

arch/arm/boot/dts/omap3430-sdp.dts +1 -1

index 0abd61108a53..7bfde8aac7ae 100644
@@ -8,7 +8,7 @@
 
 / {
 	model = "TI OMAP3430 SDP";
-	compatible = "ti,omap3430-sdp", "ti,omap3";
+	compatible = "ti,omap3430-sdp", "ti,omap3430", "ti,omap3";
 
 	memory@80000000 {
 		device_type = "memory";

Signature · 1 line

2.19.1

Tony Lindgren

2019-09-05 14:27 UTC #11 lore raw

Re: [RFC v2 3/3] ARM: dts: omap3: bulk convert compatible to be explicitly ti,omap3430 or ti,omap36xx

Hi,

Quoted 6 lines · H. Nikolaus Schaller

According to omap.txt bindings documentation, matching the ti-cpufreq driver needs to specify explicitly if a board uses an omap3430 or omap36xx chip.
This needs to add ti,omap3430 to most omap34xx boards and replace ti,omap3630 by ti,omap36xx for some omap36xx boards (most others already have done it right).

Quoted 8 lines

We also clean up some instances of missing ti,am3517 so that we can rely on seeing either one of:
ti,am3517
ti,omap34xx
ti,omap36xx
in addition to ti,omap3.

Please set up things to use ti,omap3630 in addition to ti,omap36xx for compatible to avoid churning the same files later.

arch/arm/boot/dts/logicpd-som-lv-37xx-devkit.dts line 13

Quoted 10 lines

arch/arm/boot/dts/logicpd-som-lv-37xx-devkit.dts +1 -1

@@ -9,5 +9,5 @@
 
 / {
 	model = "LogicPD Zoom DM3730 SOM-LV Development Kit";
-	compatible = "logicpd,dm3730-som-lv-devkit", "ti,omap3630", "ti,omap3";
+	compatible = "logicpd,dm3730-som-lv-devkit", "ti,omap36xx", "ti,omap3";
 };

So just make this:

compatible = "logicpd,dm3730-som-lv-devkit", "ti,omap3630", "ti,omap36xx", "ti,omap3";

And so on. It's fine to use ti,omap3630 for 37xx too as they're the same.

arch/arm/boot/dts/omap3-ldp.dts line 13

Quoted 10 lines

arch/arm/boot/dts/omap3-ldp.dts +1 -1

index 9a5fde2d9bce..9947574bd0f8 100644
@@ -10,7 +10,7 @@
 
 / {
 	model = "TI OMAP3430 LDP (Zoom1 Labrador)";
-	compatible = "ti,omap3-ldp", "ti,omap3";
+	compatible = "ti,omap3-ldp", "ti,omap34xx, "ti,omap3";

This fails to compile, it's missing a '"' for ti,omap34xx. And here too, please update to use:

compatible = "ti,omap3-ldp", "ti,omap3430", "ti,omap34xx", "ti,omap3";

And again it's fine to add "ti,omap3430" for 3530 variants.

Regards,

Tony

H. Nikolaus Schaller

author

2019-09-06 07:53 UTC #12 lore raw

Re: [RFC v2 3/3] ARM: dts: omap3: bulk convert compatible to be explicitly ti,omap3430 or ti,omap36xx

Hi,
I am preparing the next PATCH version now. Incl. updating the commit messages to carry more documentation about the opp-supported-hw bit definitions.

So please do not merge yet.

Quoted 36 lines

Am 05.09.2019 um 16:27 schrieb Tony Lindgren <[email protected]>:
Hi,
Quoted 6 lines · H. Nikolaus Schaller
According to omap.txt bindings documentation, matching the ti-cpufreq driver needs to specify explicitly if a board uses an omap3430 or omap36xx chip.
This needs to add ti,omap3430 to most omap34xx boards and replace ti,omap3630 by ti,omap36xx for some omap36xx boards (most others already have done it right).
Quoted 8 lines
We also clean up some instances of missing ti,am3517 so that we can rely on seeing either one of:
ti,am3517
ti,omap34xx
ti,omap36xx
in addition to ti,omap3.
Please set up things to use ti,omap3630 in addition to ti,omap36xx for compatible to avoid churning the same files later.
arch/arm/boot/dts/logicpd-som-lv-37xx-devkit.dts line 13
Quoted 10 lines
arch/arm/boot/dts/logicpd-som-lv-37xx-devkit.dts +1 -1
@@ -9,5 +9,5 @@
 
 / {
 	model = "LogicPD Zoom DM3730 SOM-LV Development Kit";
-	compatible = "logicpd,dm3730-som-lv-devkit", "ti,omap3630", "ti,omap3";
+	compatible = "logicpd,dm3730-som-lv-devkit", "ti,omap36xx", "ti,omap3";
 };
So just make this:

Quoted 1 line

compatible = "logicpd,dm3730-som-lv-devkit", "ti,omap3630", "ti,omap36xx", "ti,omap3";

Do we really need both? The clock driver already checks for both variants and the ti-cpufreq will as well. So it suffices to have either "ti,omap3630" or "ti,omap36xx".

But yes, there may be user-space code that reads sysfs. So we should keep the "ti,omap3630" and add "ti,omap36xx".

Quoted 9 lines

And so on. It's fine to use ti,omap3630 for 37xx too as they're the same.
Quoted 6 lines
arch/arm/boot/dts/omap3-ldp.dts +0 -0
index 9a5fde2d9bce..9947574bd0f8 100644
@@ -10,7 +10,7 @@
 

Quoted 7 lines

arch/arm/boot/dts/omap3-ldp.dts line 13
Quoted 4 lines
arch/arm/boot/dts/omap3-ldp.dts +1 -1
 / {
 	model = "TI OMAP3430 LDP (Zoom1 Labrador)";
-	compatible = "ti,omap3-ldp", "ti,omap3";
+	compatible = "ti,omap3-ldp", "ti,omap34xx, "ti,omap3";
This fails to compile, it's missing a '"' for ti,omap34xx. And here too, please update to use:

Ah yes. I missed that (and my build script did not build all DTB).

Quoted 2 lines

compatible = "ti,omap3-ldp", "ti,omap3430", "ti,omap34xx", "ti,omap3";

After thinking a little about the whole topic the main rule of this change must be:

* do not break any existing in-tree DTS
=> only *add* to compatible what we need to distinguish between omap34 and omap36

* additions shall only follow new scheme

	=> we only add "ti,omap34xx" or "ti,omap36xx"
           but neither "ti,omap3630" nor "ti,omap3430"

* cover some out-of-tree DTS
=> make the ti-cpufreq driver still match "ti,omap3430" or "ti,omap3630"
even if this duplicates compatibility

This would mean that the logicpd-som-lv-37xx-devkit.dts gets the additional "ti,omap36xx" while the omap3-ldp.dts would only get an "ti,omap34xx" but no "ti,omap3430" (since we do not use it anywhere).

Could you agree on this approach?

Quoted 2 lines

And again it's fine to add "ti,omap3430" for 3530 variants.

Yes, sure.

BR,
Nikolaus

Tony Lindgren

2019-09-06 15:47 UTC #13 lore raw

Re: [RFC v2 3/3] ARM: dts: omap3: bulk convert compatible to be explicitly ti,omap3430 or ti,omap36xx

Quoted 11 lines · H. Nikolaus Schaller

Quoted 2 lines
Am 05.09.2019 um 16:27 schrieb Tony Lindgren <[email protected]>:
compatible = "ti,omap3-ldp", "ti,omap3430", "ti,omap34xx", "ti,omap3";
After thinking a little about the whole topic the main rule of this change must be:
* do not break any existing in-tree DTS
=> only *add* to compatible what we need to distinguish between omap34 and omap36
* additions shall only follow new scheme
	=> we only add "ti,omap34xx" or "ti,omap36xx"
           but neither "ti,omap3630" nor "ti,omap3430"

Sorry I don't follow you on this one.. We should always add "ti,omap3630" where "ti,omap36xx" is currently used so we can eventually get rid of "ti,omap36xx". And the same for 34xx.

Quoted 9 lines

* cover some out-of-tree DTS
=> make the ti-cpufreq driver still match "ti,omap3430" or "ti,omap3630"
even if this duplicates compatibility
This would mean that the logicpd-som-lv-37xx-devkit.dts gets the additional "ti,omap36xx" while the omap3-ldp.dts would only get an "ti,omap34xx" but no "ti,omap3430" (since we do not use it anywhere).
Could you agree on this approach?

Yeah sounds like logicpd-som-lv-37xx-devkit.dts currently still needs "ti,omap36xx" for now.

If modifying omap3-ldp.dts, also add "ti,omap3430" in additon to "ti,omap34xx" that it already has.

So basically let's assume the following:

"ti,omap3430" == "ti,omap34xx"
"ti,omap3630" == "ti,omap36xx"

This means code needs to parse both.

And eventually we just drop the "xx" variants.

So while patching compatibles, let's also update for this to avoid multiple patches churning the same compatibles over and over.

Regards,

Tony

H. Nikolaus Schaller

author

2019-09-06 17:08 UTC #14 lore raw

Re: [RFC v2 3/3] ARM: dts: omap3: bulk convert compatible to be explicitly ti,omap3430 or ti,omap36xx

Hi Tony,

Quoted 11 lines

Am 06.09.2019 um 17:47 schrieb Tony Lindgren <[email protected]>:
Quoted 8 lines · H. Nikolaus Schaller
Quoted 2 lines
Am 05.09.2019 um 16:27 schrieb Tony Lindgren <[email protected]>:
compatible = "ti,omap3-ldp", "ti,omap3430", "ti,omap34xx", "ti,omap3";
After thinking a little about the whole topic the main rule of this change must be:
* do not break any existing in-tree DTS
=> only *add* to compatible what we need to distinguish between omap34 and omap36

Quoted 7 lines

Quoted 3 lines
* additions shall only follow new scheme
	=> we only add "ti,omap34xx" or "ti,omap36xx"
          but neither "ti,omap3630" nor "ti,omap3430"
Sorry I don't follow you on this one.. We should always add "ti,omap3630" where "ti,omap36xx" is currently used so we can eventually get rid of "ti,omap36xx". And the same for 34xx.

Ah, ok now I see.

You want to make the "ti,omap3630" the official one and "ti,omap36xx" legacy. It is probably an arbitrary choice if we want to get rid of the xx or the 30.

I had thought to do it the other way round because I had done this statistics:

for i in 3430 34xx 3630 36xx; do echo $i $(fgrep '"'ti,omap$i'"' arch/arm/boot/dts/*.dts* | wc -l); done

3430 12
34xx 28
3630 3
36xx 23

which would indicate that 34xx and 36xx are more common.

Quoted 16 lines

Quoted 9 lines
* cover some out-of-tree DTS
=> make the ti-cpufreq driver still match "ti,omap3430" or "ti,omap3630"
even if this duplicates compatibility
This would mean that the logicpd-som-lv-37xx-devkit.dts gets the additional "ti,omap36xx" while the omap3-ldp.dts would only get an "ti,omap34xx" but no "ti,omap3430" (since we do not use it anywhere).
Could you agree on this approach?
Yeah sounds like logicpd-som-lv-37xx-devkit.dts currently still needs "ti,omap36xx" for now.
If modifying omap3-ldp.dts, also add "ti,omap3430" in additon to "ti,omap34xx" that it already has.

Quoted 6 lines

So basically let's assume the following:
"ti,omap3430" == "ti,omap34xx"
"ti,omap3630" == "ti,omap36xx"
This means code needs to parse both.

Yes, it already does everywhere.

BTW there is also some code that does special SoC detection based on soc_device_match(), mainly in omapdrm/dss.

If we were to use this mechanism in the ti-cpufreq driver we could match it to ti,omap3 and could avoid all these changes.

But make it less maintainable and code more complex.

Quoted 7 lines

And eventually we just drop the "xx" variants.
So while patching compatibles, let's also update for this to avoid multiple patches churning the same compatibles over and over.

Ok. I'll rework the patch so that we never add "ti,34xx" or "ti,36xx" but add "ti,3430" or "ti,3630" if missing.

I'll also take a look at omap.txt bindings since that likely needs an update as well to better describe what the official ones are and which are legacy.

BR and thanks,
Nikolaus

H. Nikolaus Schaller

author

2019-09-06 17:15 UTC #15 lore raw

Re: [RFC v2 3/3] ARM: dts: omap3: bulk convert compatible to be explicitly ti,omap3430 or ti,omap36xx

Quoted 29 lines

Am 06.09.2019 um 19:08 schrieb H. Nikolaus Schaller <[email protected]>:
Hi Tony,
Quoted 11 lines
Am 06.09.2019 um 17:47 schrieb Tony Lindgren <[email protected]>:
Quoted 8 lines · H. Nikolaus Schaller
Quoted 2 lines
Am 05.09.2019 um 16:27 schrieb Tony Lindgren <[email protected]>:
compatible = "ti,omap3-ldp", "ti,omap3430", "ti,omap34xx", "ti,omap3";
After thinking a little about the whole topic the main rule of this change must be:
* do not break any existing in-tree DTS
=> only *add* to compatible what we need to distinguish between omap34 and omap36
Quoted 7 lines
Quoted 3 lines
* additions shall only follow new scheme
	=> we only add "ti,omap34xx" or "ti,omap36xx"
         but neither "ti,omap3630" nor "ti,omap3430"
Sorry I don't follow you on this one.. We should always add "ti,omap3630" where "ti,omap36xx" is currently used so we can eventually get rid of "ti,omap36xx". And the same for 34xx.
Ah, ok now I see.
You want to make the "ti,omap3630" the official one and "ti,omap36xx" legacy. It is probably an arbitrary choice if we want to get rid of the xx or the 30.

Quoted 8 lines

I had thought to do it the other way round because I had done this statistics:
for i in 3430 34xx 3630 36xx; do echo $i $(fgrep '"'ti,omap$i'"' arch/arm/boot/dts/*.dts* | wc -l); done
3430 12
34xx 28
3630 3
36xx 23

sorry, here is the correct result. I had some .bak files sitting around:

3430 12
34xx 5
3630 3
36xx 23

Quoted 53 lines

which would indicate that 34xx and 36xx are more common.
Quoted 16 lines
Quoted 9 lines
* cover some out-of-tree DTS
=> make the ti-cpufreq driver still match "ti,omap3430" or "ti,omap3630"
even if this duplicates compatibility
This would mean that the logicpd-som-lv-37xx-devkit.dts gets the additional "ti,omap36xx" while the omap3-ldp.dts would only get an "ti,omap34xx" but no "ti,omap3430" (since we do not use it anywhere).
Could you agree on this approach?
Yeah sounds like logicpd-som-lv-37xx-devkit.dts currently still needs "ti,omap36xx" for now.
If modifying omap3-ldp.dts, also add "ti,omap3430" in additon to "ti,omap34xx" that it already has.
Quoted 6 lines
So basically let's assume the following:
"ti,omap3430" == "ti,omap34xx"
"ti,omap3630" == "ti,omap36xx"
This means code needs to parse both.
Yes, it already does everywhere.
BTW there is also some code that does special SoC detection based on soc_device_match(), mainly in omapdrm/dss.
If we were to use this mechanism in the ti-cpufreq driver we could match it to ti,omap3 and could avoid all these changes.
But make it less maintainable and code more complex.
Quoted 7 lines
And eventually we just drop the "xx" variants.
So while patching compatibles, let's also update for this to avoid multiple patches churning the same compatibles over and over.
Ok. I'll rework the patch so that we never add "ti,34xx" or "ti,36xx" but add "ti,3430" or "ti,3630" if missing.
I'll also take a look at omap.txt bindings since that likely needs an update as well to better describe what the official ones are and which are legacy.
BR and thanks,
Nikolaus

Tony Lindgren

2019-09-06 17:24 UTC #16 lore raw

Re: [RFC v2 3/3] ARM: dts: omap3: bulk convert compatible to be explicitly ti,omap3430 or ti,omap36xx

In reply to H. Nikolaus Schaller

Quoted 8 lines · H. Nikolaus Schaller

for i in 3430 34xx 3630 36xx; do echo $i $(fgrep '"'ti,omap$i'"' arch/arm/boot/dts/*.dts* | wc -l); done
3430 12
34xx 28
3630 3
36xx 23
which would indicate that 34xx and 36xx are more common.

Right, but the xx variants are against the device tree naming and that's why we should get rid of them in the dts. The compatible should be named after the first instance like "ti,omap3430" and similar devices can just use that.

Quoted 7 lines

BTW there is also some code that does special SoC detection based on soc_device_match(), mainly in omapdrm/dss.
If we were to use this mechanism in the ti-cpufreq driver we could match it to ti,omap3 and could avoid all these changes.
But make it less maintainable and code more complex.

Hmm right, yeah using soc_device_match() would remove this issue. It might be worth doing as these SoC variants do not change much and the code should not need updating. Up to you to decide.

Quoted 3 lines

I'll also take a look at omap.txt bindings since that likely needs an update as well to better describe what the official ones are and which are legacy.

OK. Just limit the compatible changes to the ones that need to be modified for this series, the rest can be done with a separate patches.

Regards,

Tony

H. Nikolaus Schaller

author

2019-09-06 17:50 UTC #17 lore raw

Re: [RFC v2 3/3] ARM: dts: omap3: bulk convert compatible to be explicitly ti,omap3430 or ti,omap36xx

Quoted 31 lines

Am 06.09.2019 um 19:24 schrieb Tony Lindgren <[email protected]>:
Quoted 8 lines · H. Nikolaus Schaller
for i in 3430 34xx 3630 36xx; do echo $i $(fgrep '"'ti,omap$i'"' arch/arm/boot/dts/*.dts* | wc -l); done
3430 12
34xx 28
3630 3
36xx 23
which would indicate that 34xx and 36xx are more common.
Right, but the xx variants are against the device tree naming and that's why we should get rid of them in the dts. The compatible should be named after the first instance like "ti,omap3430" and similar devices can just use that.
Quoted 7 lines
BTW there is also some code that does special SoC detection based on soc_device_match(), mainly in omapdrm/dss.
If we were to use this mechanism in the ti-cpufreq driver we could match it to ti,omap3 and could avoid all these changes.
But make it less maintainable and code more complex.
Hmm right, yeah using soc_device_match() would remove this issue. It might be worth doing as these SoC variants do not change much and the code should not need updating. Up to you to decide.

Quoted 7 lines

Quoted 3 lines
I'll also take a look at omap.txt bindings since that likely needs an update as well to better describe what the official ones are and which are legacy.
OK. Just limit the compatible changes to the ones that need to be modified for this series, the rest can be done with a separate patches.

Well, with this rule we modify almost all of them. In total ca. 35 files. But it looks right.

The rest is just one missing ti,am3517.

Here is the script I want to use for bulk conversion:

# add ti,omap3430 if missing
find arch/arm/boot/dts -name '*.dts*' -exec fgrep -q '"ti,omap34xx"' {} \; ! -exec fgrep -q '"ti,omap3430"' {} \; -exec sed -i '' 's/"ti,omap34xx"/"ti,omap3430", "ti,omap34xx"/' {} \;
# add ti,omap3630 if missing
find arch/arm/boot/dts -name '*.dts*' -exec fgrep -q '"ti,omap36xx"' {} \; ! -exec fgrep -q '"ti,omap3630"' {} \; -exec sed -i '' 's/"ti,omap36xx"/"ti,omap3630", "ti,omap36xx"/' {} \;
# add ti,omap3430 default if missing
find arch/arm/boot/dts -name 'omap*.dts*' -exec fgrep -q '"ti,omap3"' {} \; ! -exec fgrep -q '"ti,omap3630"' {} \; ! -exec fgrep -q '"ti,omap36xx"' {} \; ! -exec fgrep -q '"ti,am3517"' {} \; ! -exec fgrep -q '"ti,omap34xx"' {} \; ! -exec fgrep -q '"ti,omap3430"' {} \; -exec sed -i '' 's/"ti,omap3"/"ti,omap3430", "ti,omap3"/' {} \;

+ manually add ti,am3517 to arch/arm/boot/dts/am3517_mt_ventoux.dts

BR,
Nikolaus

H. Nikolaus Schaller

author

2019-09-06 17:55 UTC #18 lore raw

Re: [RFC v2 3/3] ARM: dts: omap3: bulk convert compatible to be explicitly ti,omap3430 or ti,omap36xx

In reply to Tony Lindgren

Quoted 11 lines

Am 06.09.2019 um 19:24 schrieb Tony Lindgren <[email protected]>:
Quoted 6 lines · H. Nikolaus Schaller
BTW there is also some code that does special SoC detection based on soc_device_match(), mainly in omapdrm/dss.
If we were to use this mechanism in the ti-cpufreq driver we could match it to ti,omap3 and could avoid all these changes.

Quoted 6 lines

Quoted 1 line
But make it less maintainable and code more complex.
Hmm right, yeah using soc_device_match() would remove this issue. It might be worth doing as these SoC variants do not change much and the code should not need updating. Up to you to decide.

I have looked through the structure of the ti-cpufreq driver but it assumes that each set of register offsets and bit masks has its own compatible so that it can just switch descriptor tables.

There is no provision to run soc_device_match() instead.

So let's forget this idea...

BR,
Nikolaus

Viresh Kumar

maintainer

2019-09-05 05:03 UTC #19 lore raw

In reply to H. Nikolaus Schaller

Quoted 50 lines · H. Nikolaus Schaller

Changes V2:
* merge separate patch to remove opp-v1 table from n950-n9 into the general omap3xxx.dtsi patch
* add legacy compatibility to ti,omap3430 and ti,omap3630 for the ti-cpufreq driver
* make driver and omap3xxx.dtsi patches pass checkpatch
* add bulk patch to explicitly define compatibility to ti,omap3430 and ti,omap36xx in addition to ti,omap3 of all in-tree boards where it was missing
RFC V1 2019-09-02 12:55:55:
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 the high speed OPPs) on:
* BeagleBoard C2 (omap3430 600MHz)
* BeagleBoard XM B (dm3730 800MHz)
* GTA04A4 (dm3730 800MHz)
* GTA04A5 (dm3730 1GHz)
H. Nikolaus Schaller (3):
cpufreq: ti-cpufreq: add support for omap34xx and omap36xx
ARM: dts: replace opp-v1 tables by opp-v2 for omap34xx and omap36xx
ARM: dts: omap3: bulk convert compatible to be explicitly ti,omap3430
or ti,omap36xx
 arch/arm/boot/dts/am3517_mt_ventoux.dts       |  2 +-
 .../boot/dts/logicpd-som-lv-35xx-devkit.dts   |  2 +-
 .../boot/dts/logicpd-som-lv-37xx-devkit.dts   |  2 +-
 .../boot/dts/logicpd-torpedo-35xx-devkit.dts  |  2 +-
 .../boot/dts/logicpd-torpedo-37xx-devkit.dts  |  2 +-
 arch/arm/boot/dts/omap3-beagle.dts            |  2 +-
 arch/arm/boot/dts/omap3-cm-t3530.dts          |  2 +-
 arch/arm/boot/dts/omap3-devkit8000-lcd43.dts  |  2 +-
 arch/arm/boot/dts/omap3-devkit8000-lcd70.dts  |  2 +-
 arch/arm/boot/dts/omap3-devkit8000.dts        |  2 +-
 arch/arm/boot/dts/omap3-evm-37xx.dts          |  2 +-
 arch/arm/boot/dts/omap3-ha-lcd.dts            |  2 +-
 arch/arm/boot/dts/omap3-ha.dts                |  2 +-
 arch/arm/boot/dts/omap3-ldp.dts               |  2 +-
 arch/arm/boot/dts/omap3-n950-n9.dtsi          |  7 --

Quoted 8 lines

 arch/arm/boot/dts/omap3-sbc-t3530.dts         |  2 +-
 arch/arm/boot/dts/omap3-thunder.dts           |  2 +-
 arch/arm/boot/dts/omap3430-sdp.dts            |  2 +-
 arch/arm/boot/dts/omap34xx.dtsi               | 65 ++++++++++++--
 arch/arm/boot/dts/omap36xx.dtsi               | 53 +++++++++--
 drivers/cpufreq/cpufreq-dt-platdev.c          |  2 +-
 drivers/cpufreq/ti-cpufreq.c                  | 87 ++++++++++++++++++-
 22 files changed, 204 insertions(+), 44 deletions(-)

Most of the stuff looks fine to me here. I will pick the patches when the SoC maintainers provide an Ack.

Signature · 1 line

viresh

Tony Lindgren

2019-09-05 14:33 UTC #20 lore raw

Quoted 2 lines · Viresh Kumar

Most of the stuff looks fine to me here. I will pick the patches when the SoC maintainers provide an Ack.

I noticed few issues with the dts changes but other than that looks good to me.

Regards,

Tony