This patch set converts the opp tables to opp-v2 format and extends the ti-cpufreq to support omap3.
It adds 720 MHz (omap34xx) and 1 GHz (omap36xx) OPPs but tells the ti-cpufreq driver to disable them if the speed binned / 720MHz grade eFuse bits indicate that the chip is not rated for that speed.
It has been tested (for chip variant detection, not reliability of high speed OPPs) on: * BeagleBoard C2 (omap3430 600MHz) * BeagleBoard XM B (dm3730 800MHz) * GTA04A4 (dm3730 800MHz) * GTA04A5 (dm3730 1GHz)
H. Nikolaus Schaller (5): cpufreq: ti-cpufreq: add support for omap34xx and omap36xx ARM: dts: add support for opp-v2 for omap34xx and omap36xx ARM: dts: omap3-evm-37xx: fix compatible from omap3630 to omap36xx ARM: dts: omap3-n950-n9: remove opp-v1 table ARM: dts: omap3-beagle: make explicitly compatible to ti,omap34xx
arch/arm/boot/dts/omap3-beagle.dts | 2 +- arch/arm/boot/dts/omap3-evm-37xx.dts | 2 +- arch/arm/boot/dts/omap3-n950-n9.dtsi | 7 --- arch/arm/boot/dts/omap34xx.dtsi | 59 ++++++++++++++++--- arch/arm/boot/dts/omap36xx.dtsi | 47 ++++++++++++--- drivers/cpufreq/cpufreq-dt-platdev.c | 2 +- drivers/cpufreq/ti-cpufreq.c | 86 +++++++++++++++++++++++++++- 7 files changed, 176 insertions(+), 29 deletions(-)
This adds code and tables to read the silicon revision and eFuse (speed binned / 720 MHz grade) bits for selecting opp-v2 table entries.
Since these bits are not always part of the syscon register range (like for am3, am4, dra7), we add code to directly read the register values using ioremap() if syscon access fails.
Signed-off-by: H. Nikolaus Schaller hns@goldelico.com --- drivers/cpufreq/ti-cpufreq.c | 86 +++++++++++++++++++++++++++++++++++- 1 file changed, 84 insertions(+), 2 deletions(-)
diff --git a/drivers/cpufreq/ti-cpufreq.c b/drivers/cpufreq/ti-cpufreq.c index 2ad1ae17932d..5c5ca4577093 100644 --- a/drivers/cpufreq/ti-cpufreq.c +++ b/drivers/cpufreq/ti-cpufreq.c @@ -31,6 +31,11 @@ #define DRA7_EFUSE_OD_MPU_OPP BIT(1) #define DRA7_EFUSE_HIGH_MPU_OPP BIT(2)
+#define OMAP3_CONTROL_DEVICE_STATUS 0x4800244C +#define OMAP3_CONTROL_IDCODE 0x4830A204 +#define OMAP34xx_ProdID_SKUID 0x4830A20C +#define OMAP3_SYSCON_BASE (0x48000000 + 0x2000 + 0x270) + #define VERSION_COUNT 2
struct ti_cpufreq_data; @@ -84,6 +89,13 @@ static unsigned long dra7_efuse_xlate(struct ti_cpufreq_data *opp_data, return calculated_efuse; }
+static unsigned long omap3_efuse_xlate(struct ti_cpufreq_data *opp_data, + unsigned long efuse) +{ + /* OPP enable bit ("Speed Binned") */ + return BIT(efuse); +} + static struct ti_cpufreq_soc_data am3x_soc_data = { .efuse_xlate = amx3_efuse_xlate, .efuse_fallback = AM33XX_800M_ARM_MPU_MAX_FREQ, @@ -111,6 +123,56 @@ static struct ti_cpufreq_soc_data dra7_soc_data = { .multi_regulator = true, };
+/* + * OMAP35x TRM (SPRUF98K): + * CONTROL_IDCODE (0x4830 A204) describes Silicon revisions. + * Control OMAP Status Register 15:0 (Address 0x4800 244C) + * to separate between omap3503, omap3515, omap3525, omap3530 + * and feature presence. + * There are encodings for versions limited to 400/266MHz + * but we ignore. + * Not clear if this also holds for omap34xx. + * some eFuse values e.g. CONTROL_FUSE_OPP1_VDD1 + * are stored in the SYSCON register range + * Register 0x4830A20C [ProdID.SKUID] [0:3] + * 0x0 for normal 600/430MHz device. + * 0x8 for 720/520MHz device. + * Not clear what omap34xx value is. + */ + +static struct ti_cpufreq_soc_data omap34xx_soc_data = { + .efuse_xlate = omap3_efuse_xlate, + .efuse_offset = OMAP34xx_ProdID_SKUID - OMAP3_SYSCON_BASE, + .efuse_shift = 3, + .efuse_mask = BIT(3), + .rev_offset = OMAP3_CONTROL_IDCODE - OMAP3_SYSCON_BASE, + .multi_regulator = false, +}; + +/* + * AM/DM37x TRM (SPRUGN4M) + * CONTROL_IDCODE (0x4830 A204) describes Silicon revisions. + * Control Device Status Register 15:0 (Address 0x4800 244C) + * to separate between am3703, am3715, dm3725, dm3730 + * and feature presence. + * Speed Binned = Bit 9 + * 0 800/600 MHz + * 1 1000/800 MHz + * some eFuse values e.g. CONTROL_FUSE_OPP 1G_VDD1 + * are stored in the SYSCON register range. + * There is no 0x4830A20C [ProdID.SKUID] register (exists but + * seems to always read as 0). + */ + +static struct ti_cpufreq_soc_data omap36xx_soc_data = { + .efuse_xlate = omap3_efuse_xlate, + .efuse_offset = OMAP3_CONTROL_DEVICE_STATUS - OMAP3_SYSCON_BASE, + .efuse_shift = 9, + .efuse_mask = BIT(9), + .rev_offset = OMAP3_CONTROL_IDCODE - OMAP3_SYSCON_BASE, + .multi_regulator = false, +}; + /** * ti_cpufreq_get_efuse() - Parse and return efuse value present on SoC * @opp_data: pointer to ti_cpufreq_data context @@ -127,7 +189,15 @@ static int ti_cpufreq_get_efuse(struct ti_cpufreq_data *opp_data,
ret = regmap_read(opp_data->syscon, opp_data->soc_data->efuse_offset, &efuse); - if (ret) { + if (ret == -EIO) { + /* not a syscon register! */ + void __iomem *regs = ioremap(OMAP3_SYSCON_BASE + + opp_data->soc_data->efuse_offset, 4); + + efuse = readl(regs); + iounmap(regs); + } + else if (ret) { dev_err(dev, "Failed to read the efuse value from syscon: %d\n", ret); @@ -158,7 +228,15 @@ static int ti_cpufreq_get_rev(struct ti_cpufreq_data *opp_data,
ret = regmap_read(opp_data->syscon, opp_data->soc_data->rev_offset, &revision); - if (ret) { + if (ret == -EIO) { + /* not a syscon register! */ + void __iomem *regs = ioremap(OMAP3_SYSCON_BASE + + opp_data->soc_data->rev_offset, 4); + + revision = readl(regs); + iounmap(regs); + } + else if (ret) { dev_err(dev, "Failed to read the revision number from syscon: %d\n", ret); @@ -190,6 +268,10 @@ static const struct of_device_id ti_cpufreq_of_match[] = { { .compatible = "ti,am33xx", .data = &am3x_soc_data, }, { .compatible = "ti,am43", .data = &am4x_soc_data, }, { .compatible = "ti,dra7", .data = &dra7_soc_data }, + { .compatible = "ti,omap3430", .data = &omap34xx_soc_data, }, + { .compatible = "ti,omap34xx", .data = &omap34xx_soc_data, }, + { .compatible = "ti,omap3630", .data = &omap36xx_soc_data, }, + { .compatible = "ti,omap36xx", .data = &omap36xx_soc_data, }, {}, };
In addition, move omap3 from whitelist to blacklist in cpufreq-dt-platdev in the same patch, because doing either first breaks operation and may make trouble in bisect.
Signed-off-by: H. Nikolaus Schaller hns@goldelico.com --- arch/arm/boot/dts/omap34xx.dtsi | 59 +++++++++++++++++++++++----- arch/arm/boot/dts/omap36xx.dtsi | 47 ++++++++++++++++++---- drivers/cpufreq/cpufreq-dt-platdev.c | 2 +- 3 files changed, 90 insertions(+), 18 deletions(-)
diff --git a/arch/arm/boot/dts/omap34xx.dtsi b/arch/arm/boot/dts/omap34xx.dtsi index f572a477f74c..b92645d6d981 100644 --- a/arch/arm/boot/dts/omap34xx.dtsi +++ b/arch/arm/boot/dts/omap34xx.dtsi @@ -16,19 +16,60 @@ / { cpus { cpu: cpu@0 { - /* OMAP343x/OMAP35xx variants OPP1-5 */ - operating-points = < - /* kHz uV */ - 125000 975000 - 250000 1075000 - 500000 1200000 - 550000 1270000 - 600000 1350000 - >; + /* OMAP343x/OMAP35xx variants OPP1-6 */ + operating-points-v2 = <&cpu0_opp_table>; + clock-latency = <300000>; /* From legacy driver */ }; };
+ /* see Documentation/devicetree/bindings/opp/opp.txt */ + cpu0_opp_table: opp-table { + compatible = "operating-points-v2-ti-cpu"; + syscon = <&scm_conf>; + + opp1-125000000 { + opp-hz = /bits/ 64 <125000000>; + // we currently only select the max voltage from table Table 3-3 of the omap3530 Data sheet (SPRS507F) + // <target min max> could also be single <target> + opp-microvolt = <975000 975000 975000>; + // first value is silicon revision, second one 720MHz Device Identification + opp-supported-hw = <0xffffffff 3>; + }; + + opp2-250000000 { + opp-hz = /bits/ 64 <250000000>; + opp-microvolt = <1075000 1075000 1075000>; + opp-supported-hw = <0xffffffff 3>; + opp-suspend; + }; + + opp3-500000000 { + opp-hz = /bits/ 64 <500000000>; + opp-microvolt = <1200000 1200000 1200000>; + opp-supported-hw = <0xffffffff 3>; + }; + + opp4-550000000 { + opp-hz = /bits/ 64 <550000000>; + opp-microvolt = <1270000 1270000 1270000>; + opp-supported-hw = <0xffffffff 3>; + }; + + opp5-600000000 { + opp-hz = /bits/ 64 <600000000>; + opp-microvolt = <1350000 1350000 1350000>; + opp-supported-hw = <0xffffffff 3>; + }; + + opp6-720000000 { + opp-hz = /bits/ 64 <720000000>; + opp-microvolt = <1350000 1350000 1350000>; + /* only high-speed grade omap3530 devices */ + opp-supported-hw = <0xffffffff 2>; + }; + }; + ocp@68000000 { omap3_pmx_core2: pinmux@480025d8 { compatible = "ti,omap3-padconf", "pinctrl-single"; diff --git a/arch/arm/boot/dts/omap36xx.dtsi b/arch/arm/boot/dts/omap36xx.dtsi index 6fb23ada1f64..1595814585df 100644 --- a/arch/arm/boot/dts/omap36xx.dtsi +++ b/arch/arm/boot/dts/omap36xx.dtsi @@ -19,15 +19,46 @@ };
cpus { - /* OMAP3630/OMAP37xx 'standard device' variants OPP50 to OPP130 */ + /* OMAP3630/OMAP37xx variants OPP50 to OPP130 and OPP1G */ cpu: cpu@0 { - operating-points = < - /* kHz uV */ - 300000 1012500 - 600000 1200000 - 800000 1325000 - >; - clock-latency = <300000>; /* From legacy driver */ + operating-points-v2 = <&cpu0_opp_table>; + + clock-latency = <300000>; /* From omap-cpufreq driver */ + }; + }; + + /* see Documentation/devicetree/bindings/opp/opp.txt */ + cpu0_opp_table: opp-table { + compatible = "operating-points-v2-ti-cpu"; + syscon = <&scm_conf>; + + opp50-300000000 { + opp-hz = /bits/ 64 <300000000>; + // we currently only select the max voltage from table Table 4-19 of the DM3730 Data sheet (SPRS685B) + // <target min max> could also be single <target> + opp-microvolt = <1012500 1012500 1012500>; + // first value is silicon revision, second one is "speed binned + opp-supported-hw = <0xffffffff 3>; + opp-suspend; + }; + + opp100-600000000 { + opp-hz = /bits/ 64 <600000000>; + opp-microvolt = <1200000 1200000 1200000>; + opp-supported-hw = <0xffffffff 3>; + }; + + opp130-800000000 { + opp-hz = /bits/ 64 <800000000>; + opp-microvolt = <1325000 1325000 1325000>; + opp-supported-hw = <0xffffffff 3>; + }; + + opp1g-1000000000 { + opp-hz = /bits/ 64 <1000000000>; + opp-microvolt = <1375000 1375000 1375000>; + /* only on am/dm37x with speed-binned bit set */ + opp-supported-hw = <0xffffffff 2>; }; };
diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c index 03dc4244ab00..68b7fc4225f8 100644 --- a/drivers/cpufreq/cpufreq-dt-platdev.c +++ b/drivers/cpufreq/cpufreq-dt-platdev.c @@ -86,7 +86,6 @@ static const struct of_device_id whitelist[] __initconst = { { .compatible = "st-ericsson,u9540", },
{ .compatible = "ti,omap2", }, - { .compatible = "ti,omap3", }, { .compatible = "ti,omap4", }, { .compatible = "ti,omap5", },
@@ -132,6 +131,7 @@ static const struct of_device_id blacklist[] __initconst = { { .compatible = "ti,am33xx", }, { .compatible = "ti,am43", }, { .compatible = "ti,dra7", }, + { .compatible = "ti,omap3", },
{ } };
On 02-09-19, 12:55, H. Nikolaus Schaller wrote:
opp1-125000000 {opp-hz = /bits/ 64 <125000000>;// we currently only select the max voltage from table Table 3-3 of the omap3530 Data sheet (SPRS507F)// <target min max> could also be single <target>opp-microvolt = <975000 975000 975000>;// first value is silicon revision, second one 720MHz Device Identification
opp-supported-hw = <0xffffffff 3>;
I don't see the driver changes using this field, am I missing something ?
On 03-09-19, 08:08, Viresh Kumar wrote:
On 02-09-19, 12:55, H. Nikolaus Schaller wrote:
opp1-125000000 {opp-hz = /bits/ 64 <125000000>;// we currently only select the max voltage from table Table 3-3 of the omap3530 Data sheet (SPRS507F)// <target min max> could also be single <target>opp-microvolt = <975000 975000 975000>;// first value is silicon revision, second one 720MHz Device Identification
opp-supported-hw = <0xffffffff 3>;I don't see the driver changes using this field, am I missing something ?
The driver already had them it seems. Hmm..
Am 03.09.2019 um 04:38 schrieb Viresh Kumar viresh.kumar@linaro.org:
On 02-09-19, 12:55, H. Nikolaus Schaller wrote:
opp1-125000000 {opp-hz = /bits/ 64 <125000000>;// we currently only select the max voltage from table Table 3-3 of the omap3530 Data sheet (SPRS507F)// <target min max> could also be single <target>opp-microvolt = <975000 975000 975000>;// first value is silicon revision, second one 720MHz Device Identification
opp-supported-hw = <0xffffffff 3>;I don't see the driver changes using this field, am I missing something ?
This is parsed by _opp_is_supported() which is called indirectly by ti_cpufreq_probe().
BR, Nikolaus
All other boards use compatible = "omap36xx".
Signed-off-by: H. Nikolaus Schaller hns@goldelico.com --- arch/arm/boot/dts/omap3-evm-37xx.dts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/omap3-evm-37xx.dts b/arch/arm/boot/dts/omap3-evm-37xx.dts index e0c0382388f0..311550ee4eae 100644 --- a/arch/arm/boot/dts/omap3-evm-37xx.dts +++ b/arch/arm/boot/dts/omap3-evm-37xx.dts @@ -10,7 +10,7 @@
/ { model = "TI OMAP37XX EVM (TMDSEVM3730)"; - compatible = "ti,omap3-evm-37xx", "ti,omap3630", "ti,omap3"; + compatible = "ti,omap3-evm-37xx", "ti,omap36xx", "ti,omap3"; };
&omap3_pmx_core2 {
With opp-v2 in omap36xx.dtsi and ti-cpufreq driver the 1GHz capability is automatically detected.
Signed-off-by: H. Nikolaus Schaller hns@goldelico.com --- arch/arm/boot/dts/omap3-n950-n9.dtsi | 7 ------- 1 file changed, 7 deletions(-)
diff --git a/arch/arm/boot/dts/omap3-n950-n9.dtsi b/arch/arm/boot/dts/omap3-n950-n9.dtsi index 5441e9ffdbb4..e98b0c615f19 100644 --- a/arch/arm/boot/dts/omap3-n950-n9.dtsi +++ b/arch/arm/boot/dts/omap3-n950-n9.dtsi @@ -11,13 +11,6 @@ cpus { cpu@0 { cpu0-supply = <&vcc>; - operating-points = < - /* kHz uV */ - 300000 1012500 - 600000 1200000 - 800000 1325000 - 1000000 1375000 - >; }; };
On 02-09-19, 12:55, H. Nikolaus Schaller wrote:
With opp-v2 in omap36xx.dtsi and ti-cpufreq driver the 1GHz capability is automatically detected.
Signed-off-by: H. Nikolaus Schaller hns@goldelico.com
arch/arm/boot/dts/omap3-n950-n9.dtsi | 7 ------- 1 file changed, 7 deletions(-)
diff --git a/arch/arm/boot/dts/omap3-n950-n9.dtsi b/arch/arm/boot/dts/omap3-n950-n9.dtsi index 5441e9ffdbb4..e98b0c615f19 100644 --- a/arch/arm/boot/dts/omap3-n950-n9.dtsi +++ b/arch/arm/boot/dts/omap3-n950-n9.dtsi @@ -11,13 +11,6 @@ cpus { cpu@0 { cpu0-supply = <&vcc>;
operating-points = </* kHz uV */300000 1012500600000 1200000800000 13250001000000 1375000>;
This should be merged with 2/5 ?
Am 03.09.2019 um 04:36 schrieb Viresh Kumar viresh.kumar@linaro.org:
On 02-09-19, 12:55, H. Nikolaus Schaller wrote:
With opp-v2 in omap36xx.dtsi and ti-cpufreq driver the 1GHz capability is automatically detected.
Signed-off-by: H. Nikolaus Schaller hns@goldelico.com
arch/arm/boot/dts/omap3-n950-n9.dtsi | 7 ------- 1 file changed, 7 deletions(-)
diff --git a/arch/arm/boot/dts/omap3-n950-n9.dtsi b/arch/arm/boot/dts/omap3-n950-n9.dtsi index 5441e9ffdbb4..e98b0c615f19 100644 --- a/arch/arm/boot/dts/omap3-n950-n9.dtsi +++ b/arch/arm/boot/dts/omap3-n950-n9.dtsi @@ -11,13 +11,6 @@ cpus { cpu@0 { cpu0-supply = <&vcc>;
operating-points = </* kHz uV */300000 1012500600000 1200000800000 13250001000000 1375000>;This should be merged with 2/5 ?
Well, it bloats 2/5.
What I hope (I can't test) is that this opp-v1 table is ignored if an opp-v2 table exists. So that it can be removed by a separate follow-up patch.
BR, Nikolaus
On 03-09-19, 08:01, H. Nikolaus Schaller wrote:
Am 03.09.2019 um 04:36 schrieb Viresh Kumar viresh.kumar@linaro.org:
On 02-09-19, 12:55, H. Nikolaus Schaller wrote:
With opp-v2 in omap36xx.dtsi and ti-cpufreq driver the 1GHz capability is automatically detected.
Signed-off-by: H. Nikolaus Schaller hns@goldelico.com
arch/arm/boot/dts/omap3-n950-n9.dtsi | 7 ------- 1 file changed, 7 deletions(-)
diff --git a/arch/arm/boot/dts/omap3-n950-n9.dtsi b/arch/arm/boot/dts/omap3-n950-n9.dtsi index 5441e9ffdbb4..e98b0c615f19 100644 --- a/arch/arm/boot/dts/omap3-n950-n9.dtsi +++ b/arch/arm/boot/dts/omap3-n950-n9.dtsi @@ -11,13 +11,6 @@ cpus { cpu@0 { cpu0-supply = <&vcc>;
operating-points = </* kHz uV */300000 1012500600000 1200000800000 13250001000000 1375000>;This should be merged with 2/5 ?
Well, it bloats 2/5.
It is logically the right place to do this as that's where we are adding opp-v2.
What I hope (I can't test) is that this opp-v1 table is ignored if an opp-v2 table exists. So that it can be removed by a separate follow-up patch.
It should work as that's what we are doing in OPP core, but I still feel this better get merged with 2/5.
Am 03.09.2019 um 08:14 schrieb Viresh Kumar viresh.kumar@linaro.org:
On 03-09-19, 08:01, H. Nikolaus Schaller wrote:
Am 03.09.2019 um 04:36 schrieb Viresh Kumar viresh.kumar@linaro.org:
On 02-09-19, 12:55, H. Nikolaus Schaller wrote:
With opp-v2 in omap36xx.dtsi and ti-cpufreq driver the 1GHz capability is automatically detected.
Signed-off-by: H. Nikolaus Schaller hns@goldelico.com
arch/arm/boot/dts/omap3-n950-n9.dtsi | 7 ------- 1 file changed, 7 deletions(-)
diff --git a/arch/arm/boot/dts/omap3-n950-n9.dtsi b/arch/arm/boot/dts/omap3-n950-n9.dtsi index 5441e9ffdbb4..e98b0c615f19 100644 --- a/arch/arm/boot/dts/omap3-n950-n9.dtsi +++ b/arch/arm/boot/dts/omap3-n950-n9.dtsi @@ -11,13 +11,6 @@ cpus { cpu@0 { cpu0-supply = <&vcc>;
operating-points = </* kHz uV */300000 1012500600000 1200000800000 13250001000000 1375000>;This should be merged with 2/5 ?
Well, it bloats 2/5.
It is logically the right place to do this as that's where we are adding opp-v2.
Well, sometimes the philosophy of patches is to add something new first and remove the old in a second separate patch if the system can live with both. This makes it easier to digest single patches (because they are smaller) and might also better pinpoint an issue by bisect.
What I hope (I can't test) is that this opp-v1 table is ignored if an opp-v2 table exists. So that it can be removed by a separate follow-up patch.
It should work as that's what we are doing in OPP core, but I still feel this better get merged with 2/5.
Ok, I see. Noted for RFCv2.
There will also be a big batch of changes for the compatible record (omap3530->omap35xx, add omap34xx where needed) of ca. 10 board definition DTS files. Should this then also become part of the new 2/5?
BR and thanks, Nikolaus
On 03-09-19, 08:23, H. Nikolaus Schaller wrote:
Am 03.09.2019 um 08:14 schrieb Viresh Kumar viresh.kumar@linaro.org:
On 03-09-19, 08:01, H. Nikolaus Schaller wrote:
Am 03.09.2019 um 04:36 schrieb Viresh Kumar viresh.kumar@linaro.org:
On 02-09-19, 12:55, H. Nikolaus Schaller wrote:
With opp-v2 in omap36xx.dtsi and ti-cpufreq driver the 1GHz capability is automatically detected.
Signed-off-by: H. Nikolaus Schaller hns@goldelico.com
arch/arm/boot/dts/omap3-n950-n9.dtsi | 7 ------- 1 file changed, 7 deletions(-)
diff --git a/arch/arm/boot/dts/omap3-n950-n9.dtsi b/arch/arm/boot/dts/omap3-n950-n9.dtsi index 5441e9ffdbb4..e98b0c615f19 100644 --- a/arch/arm/boot/dts/omap3-n950-n9.dtsi +++ b/arch/arm/boot/dts/omap3-n950-n9.dtsi @@ -11,13 +11,6 @@ cpus { cpu@0 { cpu0-supply = <&vcc>;
operating-points = </* kHz uV */300000 1012500600000 1200000800000 13250001000000 1375000>;This should be merged with 2/5 ?
Well, it bloats 2/5.
It is logically the right place to do this as that's where we are adding opp-v2.
Well, sometimes the philosophy of patches is to add something new first and remove the old in a second separate patch if the system can live with both. This makes it easier to digest single patches (because they are smaller) and might also better pinpoint an issue by bisect.
Right, but you already removed some of the opp-v1 stuff in patch 2/5. Why leave this one out ?
What I hope (I can't test) is that this opp-v1 table is ignored if an opp-v2 table exists. So that it can be removed by a separate follow-up patch.
It should work as that's what we are doing in OPP core, but I still feel this better get merged with 2/5.
Ok, I see. Noted for RFCv2.
There will also be a big batch of changes for the compatible record (omap3530->omap35xx, add omap34xx where needed) of ca. 10 board definition DTS files. Should this then also become part of the new 2/5?
Compatible thing should be separate patch anyway, I was just talking about replacing opp-v1 with v2.
Am 03.09.2019 um 08:28 schrieb Viresh Kumar viresh.kumar@linaro.org:
On 03-09-19, 08:23, H. Nikolaus Schaller wrote:
Am 03.09.2019 um 08:14 schrieb Viresh Kumar viresh.kumar@linaro.org:
On 03-09-19, 08:01, H. Nikolaus Schaller wrote:
Am 03.09.2019 um 04:36 schrieb Viresh Kumar viresh.kumar@linaro.org:
On 02-09-19, 12:55, H. Nikolaus Schaller wrote:
With opp-v2 in omap36xx.dtsi and ti-cpufreq driver the 1GHz capability is automatically detected.
Signed-off-by: H. Nikolaus Schaller hns@goldelico.com
arch/arm/boot/dts/omap3-n950-n9.dtsi | 7 ------- 1 file changed, 7 deletions(-)
diff --git a/arch/arm/boot/dts/omap3-n950-n9.dtsi b/arch/arm/boot/dts/omap3-n950-n9.dtsi index 5441e9ffdbb4..e98b0c615f19 100644 --- a/arch/arm/boot/dts/omap3-n950-n9.dtsi +++ b/arch/arm/boot/dts/omap3-n950-n9.dtsi @@ -11,13 +11,6 @@ cpus { cpu@0 { cpu0-supply = <&vcc>;
operating-points = </* kHz uV */300000 1012500600000 1200000800000 13250001000000 1375000>;This should be merged with 2/5 ?
Well, it bloats 2/5.
It is logically the right place to do this as that's where we are adding opp-v2.
Well, sometimes the philosophy of patches is to add something new first and remove the old in a second separate patch if the system can live with both. This makes it easier to digest single patches (because they are smaller) and might also better pinpoint an issue by bisect.
Right, but you already removed some of the opp-v1 stuff in patch 2/5. Why leave this one out ?
What I hope (I can't test) is that this opp-v1 table is ignored if an opp-v2 table exists. So that it can be removed by a separate follow-up patch.
It should work as that's what we are doing in OPP core, but I still feel this better get merged with 2/5.
Ok, I see. Noted for RFCv2.
There will also be a big batch of changes for the compatible record (omap3530->omap35xx, add omap34xx where needed) of ca. 10 board definition DTS files. Should this then also become part of the new 2/5?
Compatible thing should be separate patch anyway, I was just talking about replacing opp-v1 with v2.
Ok, understood.
BR and thanks, Nikolaus
Matching the ti-cpufreq driver needs to specify explicitly if a board uses an omap34xx or omap36xx chip.
Signed-off-by: H. Nikolaus Schaller hns@goldelico.com --- arch/arm/boot/dts/omap3-beagle.dts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/omap3-beagle.dts b/arch/arm/boot/dts/omap3-beagle.dts index e3df3c166902..d47213c7a4d0 100644 --- a/arch/arm/boot/dts/omap3-beagle.dts +++ b/arch/arm/boot/dts/omap3-beagle.dts @@ -8,7 +8,7 @@
/ { model = "TI OMAP3 BeagleBoard"; - compatible = "ti,omap3-beagle", "ti,omap3"; + compatible = "ti,omap3-beagle", "ti,omap34xx", "ti,omap3";
cpus { cpu@0 {
* H. Nikolaus Schaller hns@goldelico.com [190902 10:56]:
Matching the ti-cpufreq driver needs to specify explicitly if a board uses an omap34xx or omap36xx chip.
Signed-off-by: H. Nikolaus Schaller hns@goldelico.com
arch/arm/boot/dts/omap3-beagle.dts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/omap3-beagle.dts b/arch/arm/boot/dts/omap3-beagle.dts index e3df3c166902..d47213c7a4d0 100644 --- a/arch/arm/boot/dts/omap3-beagle.dts +++ b/arch/arm/boot/dts/omap3-beagle.dts @@ -8,7 +8,7 @@
/ { model = "TI OMAP3 BeagleBoard";
- compatible = "ti,omap3-beagle", "ti,omap3";
compatible = "ti,omap3-beagle", "ti,omap34xx", "ti,omap3";
cpus { cpu@0 {
For a clean-up patch, we should just use the following compatibles in general for omap3:
ti,omap3 omap3 ti,omap34 omap34xx and omap35xx ti,omap36 omap36xx and dm37xx ti,am35 am35xx
So we should just leave out the "xx" part. But we still need parse also the legacy binding with "xx" in drivers.
Regards,
Tony
Hi Tony,
Am 03.09.2019 um 15:40 schrieb Tony Lindgren tony@atomide.com:
- H. Nikolaus Schaller hns@goldelico.com [190902 10:56]:
Matching the ti-cpufreq driver needs to specify explicitly if a board uses an omap34xx or omap36xx chip.
Signed-off-by: H. Nikolaus Schaller hns@goldelico.com
arch/arm/boot/dts/omap3-beagle.dts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/omap3-beagle.dts b/arch/arm/boot/dts/omap3-beagle.dts index e3df3c166902..d47213c7a4d0 100644 --- a/arch/arm/boot/dts/omap3-beagle.dts +++ b/arch/arm/boot/dts/omap3-beagle.dts @@ -8,7 +8,7 @@
/ { model = "TI OMAP3 BeagleBoard";
- compatible = "ti,omap3-beagle", "ti,omap3";
compatible = "ti,omap3-beagle", "ti,omap34xx", "ti,omap3";
cpus { cpu@0 {
For a clean-up patch, we should just use the following compatibles in general for omap3:
ti,omap3 omap3 ti,omap34 omap34xx and omap35xx ti,omap36 omap36xx and dm37xx ti,am35 am35xx
So we should just leave out the "xx" part. But we still need parse also the legacy binding with "xx" in drivers.
Yes, getting rid of the xx in the device trees would be nice.
But I have looked through the kernel tree and that makes more problems than it solves...
There is code that explicitly checks for "ti,omap36xx" in drivers/clk/ti/dpll.c
Maybe that should be replaced by a check for a "ti,dpll5" property which is only defined in omap36xx.dtsi to make the code depend on a feature rather than SoC family string. Although such a change may break other things or we have to keep even more legacy code around.
And there is a binding document omap.txt that does not describe it the way you propose. IMHO that should be changed first.
Next is that omap.txt explicitly says that ti,omap3 defauls to OMAP3430. And OMAP3430 should be specified as compatible = "ti,omap3430", "ti,omap3" while OMAP3630 should have compatible = "ti,omap36xx", "ti,omap3". AM3517 is compatible = "ti,am3517", "ti,omap3" which seems to ignore the AM3505 (maybe there was never a board using it).
So this clean up is much more than what we need for moving from opp-v1 to opp-v2 and it should therefore be addressed separately.
Therefore I'd prefer if we can keep all these problems isolated into a separate set of patches. And because of this I have prepared an RFC-V2 which only adds the "ti,omap3430" to those boards which do not yet have it (in accordance to existing omap.txt).
AM3517 does not seem to have any opp table and therefore I also exclude it at the moment.
BR and thanks, Nikolaus
-
H. Nikolaus Schaller -
Tony Lindgren -
Viresh Kumar