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(-)
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 hns@goldelico.com --- drivers/cpufreq/ti-cpufreq.c | 87 +++++++++++++++++++++++++++++++++++- 1 file changed, 85 insertions(+), 2 deletions(-)
diff --git a/drivers/cpufreq/ti-cpufreq.c b/drivers/cpufreq/ti-cpufreq.c index 2ad1ae17932d..b3de3162ea73 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,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, }, {}, };
* H. Nikolaus Schaller hns@goldelico.com [190904 08:54]:
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 tony@atomide.com
On 05-09-19, 07:32, Tony Lindgren wrote:
- H. Nikolaus Schaller hns@goldelico.com [190904 08:54]:
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)
That's a DT thing I believe where the voltage doesn't fit what the regulator can support.
But presumably that can be further patched. So for this patch:
Acked-by: Tony Lindgren tony@atomide.com
Thanks.
Hi,
Am 06.09.2019 um 05:01 schrieb Viresh Kumar viresh.kumar@linaro.org:
On 05-09-19, 07:32, Tony Lindgren wrote:
- H. Nikolaus Schaller hns@goldelico.com [190904 08:54]:
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)
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:~#
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
Am 06.09.2019 um 22:46 schrieb H. Nikolaus Schaller hns@goldelico.com:
Hi,
Am 06.09.2019 um 05:01 schrieb Viresh Kumar viresh.kumar@linaro.org:
On 05-09-19, 07:32, Tony Lindgren wrote:
- H. Nikolaus Schaller hns@goldelico.com [190904 08:54]:
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)
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:~#
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.
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
On Fri, 6 Sep 2019 22:46:49 +0200 "H. Nikolaus Schaller" hns@goldelico.com wrote:
Hi,
Am 06.09.2019 um 05:01 schrieb Viresh Kumar viresh.kumar@linaro.org:
On 05-09-19, 07:32, Tony Lindgren wrote:
- H. Nikolaus Schaller hns@goldelico.com [190904 08:54]:
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)
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:~#
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 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
On 05-09-19, 07:32, Tony Lindgren wrote:
Acked-by: Tony Lindgren tony@atomide.com
Do you want to pick the series instead as this has lots of DT changes ?
* Viresh Kumar viresh.kumar@linaro.org [190906 03:05]:
On 05-09-19, 07:32, Tony Lindgren wrote:
Acked-by: Tony Lindgren tony@atomide.com
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
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 hns@goldelico.com --- 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(-)
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 - >; }; };
diff --git a/arch/arm/boot/dts/omap34xx.dtsi b/arch/arm/boot/dts/omap34xx.dtsi index f572a477f74c..6408a0448834 100644 --- a/arch/arm/boot/dts/omap34xx.dtsi +++ b/arch/arm/boot/dts/omap34xx.dtsi @@ -16,19 +16,66 @@ / { 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). + * Format is: <target min max> + */ + opp-microvolt = <975000 975000 975000>; + /* + * first value is silicon revision bit mask + * second one 720MHz Device Identification bit mask + */ + 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..44f25b0eb45b 100644 --- a/arch/arm/boot/dts/omap36xx.dtsi +++ b/arch/arm/boot/dts/omap36xx.dtsi @@ -19,15 +19,52 @@ };
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) + * Format is: <target min max> + */ + opp-microvolt = <1012500 1012500 1012500>; + /* + * first value is silicon revision bit mask + * second one is "speed binned" bit mask + */ + 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", },
{ } };
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 hns@goldelico.com --- 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(-)
diff --git a/arch/arm/boot/dts/am3517_mt_ventoux.dts b/arch/arm/boot/dts/am3517_mt_ventoux.dts index e507e4ae0d88..e7d7124a34ba 100644 --- a/arch/arm/boot/dts/am3517_mt_ventoux.dts +++ b/arch/arm/boot/dts/am3517_mt_ventoux.dts @@ -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"; diff --git a/arch/arm/boot/dts/logicpd-som-lv-35xx-devkit.dts b/arch/arm/boot/dts/logicpd-som-lv-35xx-devkit.dts index f7a841a28865..2a0a98fe67f0 100644 --- a/arch/arm/boot/dts/logicpd-som-lv-35xx-devkit.dts +++ b/arch/arm/boot/dts/logicpd-som-lv-35xx-devkit.dts @@ -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"; }; diff --git a/arch/arm/boot/dts/logicpd-som-lv-37xx-devkit.dts b/arch/arm/boot/dts/logicpd-som-lv-37xx-devkit.dts index a604d92221a4..ea441acf000d 100644 --- a/arch/arm/boot/dts/logicpd-som-lv-37xx-devkit.dts +++ b/arch/arm/boot/dts/logicpd-som-lv-37xx-devkit.dts @@ -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"; }; diff --git a/arch/arm/boot/dts/logicpd-torpedo-35xx-devkit.dts b/arch/arm/boot/dts/logicpd-torpedo-35xx-devkit.dts index 7675bc3fa868..57bae2aa910e 100644 --- a/arch/arm/boot/dts/logicpd-torpedo-35xx-devkit.dts +++ b/arch/arm/boot/dts/logicpd-torpedo-35xx-devkit.dts @@ -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"; }; diff --git a/arch/arm/boot/dts/logicpd-torpedo-37xx-devkit.dts b/arch/arm/boot/dts/logicpd-torpedo-37xx-devkit.dts index 18c27e85051f..1937a1c36ad3 100644 --- a/arch/arm/boot/dts/logicpd-torpedo-37xx-devkit.dts +++ b/arch/arm/boot/dts/logicpd-torpedo-37xx-devkit.dts @@ -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"; diff --git a/arch/arm/boot/dts/omap3-beagle.dts b/arch/arm/boot/dts/omap3-beagle.dts index e3df3c166902..4ed3f93f5841 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,omap3430", "ti,omap3";
cpus { cpu@0 { diff --git a/arch/arm/boot/dts/omap3-cm-t3530.dts b/arch/arm/boot/dts/omap3-cm-t3530.dts index 76e52c78cbb4..f13644a776aa 100644 --- a/arch/arm/boot/dts/omap3-cm-t3530.dts +++ b/arch/arm/boot/dts/omap3-cm-t3530.dts @@ -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 { diff --git a/arch/arm/boot/dts/omap3-devkit8000-lcd43.dts b/arch/arm/boot/dts/omap3-devkit8000-lcd43.dts index a80fc60bc773..afed85078ad8 100644 --- a/arch/arm/boot/dts/omap3-devkit8000-lcd43.dts +++ b/arch/arm/boot/dts/omap3-devkit8000-lcd43.dts @@ -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 { diff --git a/arch/arm/boot/dts/omap3-devkit8000-lcd70.dts b/arch/arm/boot/dts/omap3-devkit8000-lcd70.dts index 0753776071f8..07c51a105c0d 100644 --- a/arch/arm/boot/dts/omap3-devkit8000-lcd70.dts +++ b/arch/arm/boot/dts/omap3-devkit8000-lcd70.dts @@ -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 { diff --git a/arch/arm/boot/dts/omap3-devkit8000.dts b/arch/arm/boot/dts/omap3-devkit8000.dts index faafc48d8f61..162d0726b008 100644 --- a/arch/arm/boot/dts/omap3-devkit8000.dts +++ b/arch/arm/boot/dts/omap3-devkit8000.dts @@ -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; 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 { diff --git a/arch/arm/boot/dts/omap3-ha-lcd.dts b/arch/arm/boot/dts/omap3-ha-lcd.dts index badb9b3c8897..74751798c00e 100644 --- a/arch/arm/boot/dts/omap3-ha-lcd.dts +++ b/arch/arm/boot/dts/omap3-ha-lcd.dts @@ -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 { diff --git a/arch/arm/boot/dts/omap3-ha.dts b/arch/arm/boot/dts/omap3-ha.dts index a5365252bfbe..f36cbc0a3586 100644 --- a/arch/arm/boot/dts/omap3-ha.dts +++ b/arch/arm/boot/dts/omap3-ha.dts @@ -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 { diff --git a/arch/arm/boot/dts/omap3-ldp.dts b/arch/arm/boot/dts/omap3-ldp.dts index 9a5fde2d9bce..9947574bd0f8 100644 --- a/arch/arm/boot/dts/omap3-ldp.dts +++ b/arch/arm/boot/dts/omap3-ldp.dts @@ -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"; diff --git a/arch/arm/boot/dts/omap3-sbc-t3530.dts b/arch/arm/boot/dts/omap3-sbc-t3530.dts index ae96002abb3b..19582ef68e60 100644 --- a/arch/arm/boot/dts/omap3-sbc-t3530.dts +++ b/arch/arm/boot/dts/omap3-sbc-t3530.dts @@ -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; diff --git a/arch/arm/boot/dts/omap3-thunder.dts b/arch/arm/boot/dts/omap3-thunder.dts index 6276e7079b36..b12ff8ecae50 100644 --- a/arch/arm/boot/dts/omap3-thunder.dts +++ b/arch/arm/boot/dts/omap3-thunder.dts @@ -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 { diff --git a/arch/arm/boot/dts/omap3430-sdp.dts b/arch/arm/boot/dts/omap3430-sdp.dts index 0abd61108a53..7bfde8aac7ae 100644 --- a/arch/arm/boot/dts/omap3430-sdp.dts +++ b/arch/arm/boot/dts/omap3430-sdp.dts @@ -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";
Hi,
* H. Nikolaus Schaller hns@goldelico.com [190904 08:54]:
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.
Please set up things to use ti,omap3630 in addition to ti,omap36xx for compatible to avoid churning the same files later.
diff --git a/arch/arm/boot/dts/logicpd-som-lv-37xx-devkit.dts b/arch/arm/boot/dts/logicpd-som-lv-37xx-devkit.dts --- a/arch/arm/boot/dts/logicpd-som-lv-37xx-devkit.dts +++ b/arch/arm/boot/dts/logicpd-som-lv-37xx-devkit.dts @@ -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.
diff --git a/arch/arm/boot/dts/omap3-ldp.dts b/arch/arm/boot/dts/omap3-ldp.dts index 9a5fde2d9bce..9947574bd0f8 100644 --- a/arch/arm/boot/dts/omap3-ldp.dts +++ b/arch/arm/boot/dts/omap3-ldp.dts @@ -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
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.
Am 05.09.2019 um 16:27 schrieb Tony Lindgren tony@atomide.com:
Hi,
- H. Nikolaus Schaller hns@goldelico.com [190904 08:54]:
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.
Please set up things to use ti,omap3630 in addition to ti,omap36xx for compatible to avoid churning the same files later.
diff --git a/arch/arm/boot/dts/logicpd-som-lv-37xx-devkit.dts b/arch/arm/boot/dts/logicpd-som-lv-37xx-devkit.dts --- a/arch/arm/boot/dts/logicpd-som-lv-37xx-devkit.dts +++ b/arch/arm/boot/dts/logicpd-som-lv-37xx-devkit.dts @@ -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";
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".
And so on. It's fine to use ti,omap3630 for 37xx too as they're the same.
diff --git a/arch/arm/boot/dts/omap3-ldp.dts b/arch/arm/boot/dts/omap3-ldp.dts index 9a5fde2d9bce..9947574bd0f8 100644 --- a/arch/arm/boot/dts/omap3-ldp.dts +++ b/arch/arm/boot/dts/omap3-ldp.dts @@ -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:
Ah yes. I missed that (and my build script did not build all DTB).
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?
And again it's fine to add "ti,omap3430" for 3530 variants.
Yes, sure.
BR, Nikolaus
* H. Nikolaus Schaller hns@goldelico.com [190906 07:53]:
Am 05.09.2019 um 16:27 schrieb Tony Lindgren tony@atomide.com: 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.
- 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
Hi Tony,
Am 06.09.2019 um 17:47 schrieb Tony Lindgren tony@atomide.com:
- H. Nikolaus Schaller hns@goldelico.com [190906 07:53]:
Am 05.09.2019 um 16:27 schrieb Tony Lindgren tony@atomide.com: 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.
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.
- 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.
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.
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
Am 06.09.2019 um 19:08 schrieb H. Nikolaus Schaller hns@goldelico.com:
Hi Tony,
Am 06.09.2019 um 17:47 schrieb Tony Lindgren tony@atomide.com:
- H. Nikolaus Schaller hns@goldelico.com [190906 07:53]:
Am 05.09.2019 um 16:27 schrieb Tony Lindgren tony@atomide.com: 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.
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
sorry, here is the correct result. I had some .bak files sitting around:
3430 12 34xx 5 3630 3 36xx 23
which would indicate that 34xx and 36xx are more common.
- 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.
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.
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 hns@goldelico.com [190906 17:09]:
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.
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.
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
Am 06.09.2019 um 19:24 schrieb Tony Lindgren tony@atomide.com:
- H. Nikolaus Schaller hns@goldelico.com [190906 17:09]:
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.
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.
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
Am 06.09.2019 um 19:24 schrieb Tony Lindgren tony@atomide.com:
- H. Nikolaus Schaller hns@goldelico.com [190906 17:09]:
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.
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
On 04-09-19, 10:53, H. Nikolaus Schaller wrote:
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(-)
Most of the stuff looks fine to me here. I will pick the patches when the SoC maintainers provide an Ack.
* Viresh Kumar viresh.kumar@linaro.org [190905 05:03]:
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