diff mbox series

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

Message ID a889b10386bebfbfd6cdb5491367235290d53247.1567587220.git.hns@goldelico.com (mailing list archive)
State New, archived
Headers show
Series OMAP3: convert opp-v1 to opp-v2 and read speed binned / 720MHz grade bits | expand

Commit Message

H. Nikolaus Schaller Sept. 4, 2019, 8:53 a.m. UTC
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(-)

Comments

Tony Lindgren Sept. 5, 2019, 2:32 p.m. UTC | #1
* 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>
Viresh Kumar Sept. 6, 2019, 3:01 a.m. UTC | #2
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.
Viresh Kumar Sept. 6, 2019, 3:04 a.m. UTC | #3
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
?
Tony Lindgren Sept. 6, 2019, 3:49 p.m. UTC | #4
* 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
H. Nikolaus Schaller Sept. 6, 2019, 8:46 p.m. UTC | #5
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
H. Nikolaus Schaller Sept. 7, 2019, 6:34 a.m. UTC | #6
> 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
Andreas Kemnade Sept. 7, 2019, 7:19 a.m. UTC | #7
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
diff mbox series

Patch

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, },
 	{},
 };