[PATCH v5 0/6] ti: k3-am62{a,p}x-sk: add opp frequencies

Dhruva Gole posted 6 patches 2 months ago
There is a newer version of this series
arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi         | 10 +++--
arch/arm64/boot/dts/ti/k3-am625.dtsi               |  2 +-
arch/arm64/boot/dts/ti/k3-am62a-wakeup.dtsi        |  5 +++
arch/arm64/boot/dts/ti/k3-am62a7-sk.dts            |  9 ++++
arch/arm64/boot/dts/ti/k3-am62a7.dtsi              | 51 ++++++++++++++++++++++
.../boot/dts/ti/k3-am62p-j722s-common-wakeup.dtsi  |  5 +++
arch/arm64/boot/dts/ti/k3-am62p5-sk.dts            |  9 ++++
arch/arm64/boot/dts/ti/k3-am62p5.dtsi              | 47 ++++++++++++++++++++
drivers/cpufreq/ti-cpufreq.c                       |  5 +--
9 files changed, 135 insertions(+), 8 deletions(-)
[PATCH v5 0/6] ti: k3-am62{a,p}x-sk: add opp frequencies
Posted by Dhruva Gole 2 months ago
Hello everyone

This series adds in the OPPs for the Cortex-A53s on the AM62Ax and
AM62Px SoC families along with the defining the
WKUP_MMR0_WKUP0_CTRL_MMR0_JTAG_USER_ID which we can use to properly
limit the OPPs available for that variant.

Moreover, it also adds a driver cleanup where we can finally get rid of
the rev_offsets and zero the efuse_offsets as they are now taken from socinfo and
DT efuse node register respectively. This series has been tested on all
devices that it affects and has been confirmed to cause no known regressions.

Test logs:
https://gist.github.com/DhruvaG2000/63f5e28636d52787488f776e5bf39498

**NOTE:** Special attention needed on the driver patch 5/6 & 6/6:

Viresh and Vignesh,

The driver fix needs to go with PATCH 5/6.

Subject: [PATCH V4 5/6] arm64: dts: ti: k3-am62: use opp_efuse_table for
 opp-table syscon

That patch fixes the efuse offset in the AM625 DT.
Without it, the driver will read from an incorrect efuse offset, and end
up breaking things in -next till all the DT changes make it in.
Hence, it would be preferrable if this entire series goes via a single
maintainer's tree.
Viresh, perhaps if you can ack this single patch, then Vignesh could take it up
if there are no objections?

I am sorry that this breaks compatibility with older AM625 devicetree.
However, the old devicetree was marking the entire wkup_conf as "syscon",
"simple-mfd" which was wrong and needed to be fixed.

This series finally tries to bring order to DT and the driver.

However, if there is still any way to maintain the backward
compatibility, then I am open to suggestions. Please try
and understand here that the ask for backward compatibility here
is to ask the driver to support a case where the register offset itself
was to be picked from a different node. I am not sure if there's any
cleaner way to do this.

Changelog:

---
Changes in v5:
- Based on Andrew's review on v4 of "arm64: dts: ti: k3-am62: use opp_efuse_table for opp-table syscon",
	- s/syscon@43000000/bus@43000000/
	- Drop the "reg = <>;" line
- Link to v4: https://lore.kernel.org/all/20240919082809.174589-1-d-gole@ti.com/

Changes in v4:
- Add 2 more patches to this series:
	- Driver cleanup as described above.
	- AM625 DT fixups (Link to v1 of that patch: https://lore.kernel.org/all/20240902093222.2828345-2-d-gole@ti.com/)
- Link to v3: https://lore.kernel.org/all/20240826-opp-v3-0-0934f8309e13@ti.com/

Changes in v3:
- Miscellaneous spelling fixes in commit body
- Link to v2: https://lore.kernel.org/r/20240823-opp-v2-0-e2f67b37c299@ti.com

Changes in v2:
- Expanded on commit descriptions
- Split board file and SoC fdt changes into different patches
- Link to v1: https://lore.kernel.org/r/20240809-opp-v1-0-fea8efeaf963@ti.com

---

Bryan Brattlof (4):
  arm64: dts: ti: k3-am62a: add opp frequencies
  arm64: dts: ti: k3-am62a7-sk: add 1.4ghz opp entry
  arm64: dts: ti: k3-am62p: add opp frequencies
  arm64: dts: ti: k3-am62p5-sk: add 1.4ghz opp entry

Dhruva Gole (2):
  arm64: dts: ti: k3-am62: use opp_efuse_table for opp-table syscon
  cpufreq: ti-cpufreq: Update efuse/rev offsets in AM62 family

 arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi    |  7 ++-
 arch/arm64/boot/dts/ti/k3-am625.dtsi          |  2 +-
 arch/arm64/boot/dts/ti/k3-am62a-wakeup.dtsi   |  5 ++
 arch/arm64/boot/dts/ti/k3-am62a7-sk.dts       |  9 ++++
 arch/arm64/boot/dts/ti/k3-am62a7.dtsi         | 51 +++++++++++++++++++
 .../dts/ti/k3-am62p-j722s-common-wakeup.dtsi  |  5 ++
 arch/arm64/boot/dts/ti/k3-am62p5-sk.dts       |  9 ++++
 arch/arm64/boot/dts/ti/k3-am62p5.dtsi         | 47 +++++++++++++++++
 drivers/cpufreq/ti-cpufreq.c                  |  5 +-
 9 files changed, 134 insertions(+), 6 deletions(-)

base-commit: 55bcd2e0d04c1171d382badef1def1fd04ef66c5
--
2.34.1

---
Bryan Brattlof (4):
      arm64: dts: ti: k3-am62a: add opp frequencies
      arm64: dts: ti: k3-am62a7-sk: add 1.4ghz opp entry
      arm64: dts: ti: k3-am62p: add opp frequencies
      arm64: dts: ti: k3-am62p5-sk: add 1.4ghz opp entry

Dhruva Gole (2):
      arm64: dts: ti: k3-am62: use opp_efuse_table for opp-table syscon
      cpufreq: ti-cpufreq: Update efuse/rev offsets in AM62 family

 arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi         | 10 +++--
 arch/arm64/boot/dts/ti/k3-am625.dtsi               |  2 +-
 arch/arm64/boot/dts/ti/k3-am62a-wakeup.dtsi        |  5 +++
 arch/arm64/boot/dts/ti/k3-am62a7-sk.dts            |  9 ++++
 arch/arm64/boot/dts/ti/k3-am62a7.dtsi              | 51 ++++++++++++++++++++++
 .../boot/dts/ti/k3-am62p-j722s-common-wakeup.dtsi  |  5 +++
 arch/arm64/boot/dts/ti/k3-am62p5-sk.dts            |  9 ++++
 arch/arm64/boot/dts/ti/k3-am62p5.dtsi              | 47 ++++++++++++++++++++
 drivers/cpufreq/ti-cpufreq.c                       |  5 +--
 9 files changed, 135 insertions(+), 8 deletions(-)
---
base-commit: 55bcd2e0d04c1171d382badef1def1fd04ef66c5
change-id: 20240924-ti-cpufreq-fixes-v5-dbb3e5056b6c

Best regards,
-- 
Dhruva Gole <d-gole@ti.com>
Re: [PATCH v5 0/6] ti: k3-am62{a,p}x-sk: add opp frequencies
Posted by Nishanth Menon 2 months ago
On 15:20-20240924, Dhruva Gole wrote:
[...]
> 
> I am sorry that this breaks compatibility with older AM625 devicetree.
> However, the old devicetree was marking the entire wkup_conf as "syscon",
> "simple-mfd" which was wrong and needed to be fixed.
> 
> This series finally tries to bring order to DT and the driver.
> 
> However, if there is still any way to maintain the backward
> compatibility, then I am open to suggestions. Please try
> and understand here that the ask for backward compatibility here
> is to ask the driver to support a case where the register offset itself
> was to be picked from a different node. I am not sure if there's any
> cleaner way to do this.


Have you tried to handle this with quirks? I am not in favor of breaking
backward compatibility.

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D
Re: [PATCH v5 0/6] ti: k3-am62{a,p}x-sk: add opp frequencies
Posted by Dhruva Gole 2 months ago
On Sep 24, 2024 at 07:15:44 -0500, Nishanth Menon wrote:
> On 15:20-20240924, Dhruva Gole wrote:
> [...]
> > 
> > I am sorry that this breaks compatibility with older AM625 devicetree.
> > However, the old devicetree was marking the entire wkup_conf as "syscon",
> > "simple-mfd" which was wrong and needed to be fixed.
> > 
> > This series finally tries to bring order to DT and the driver.
> > 
> > However, if there is still any way to maintain the backward
> > compatibility, then I am open to suggestions. Please try
> > and understand here that the ask for backward compatibility here
> > is to ask the driver to support a case where the register offset itself
> > was to be picked from a different node. I am not sure if there's any
> > cleaner way to do this.
> 
> 
> Have you tried to handle this with quirks? I am not in favor of breaking
> backward compatibility.

I was thinking of something on those lines, but quirks makes sense for
the case that there's a quirky behaviour in the SoC itself. Here it
seems to me that we are adding a quirk to handle quirk in some old devicetree.

There's no way to detect the devicetree version or somehow distinguish
within the driver if it's an old or a new DT. One way I could think of
is on these lines:

8<---------------------------------------------------------------------------
diff --git a/drivers/cpufreq/ti-cpufreq.c b/drivers/cpufreq/ti-cpufreq.c
index 870ab0b376c1..e1b22c5d4ab8 100644
--- a/drivers/cpufreq/ti-cpufreq.c
+++ b/drivers/cpufreq/ti-cpufreq.c
@@ -93,6 +93,7 @@ struct ti_cpufreq_soc_data {
        bool multi_regulator;
 /* Backward compatibility hack: Might have missing syscon */
 #define TI_QUIRK_SYSCON_MAY_BE_MISSING 0x1
+#define TI_QUIRK_SYSCON_MAY_BE_INCORRECT       0x2
        u8 quirks;
 };

@@ -317,6 +318,7 @@ static struct ti_cpufreq_soc_data am625_soc_data = {
        .efuse_mask = 0x07c0,
        .efuse_shift = 0x6,
        .multi_regulator = false,
+       .quirks = TI_QUIRK_SYSCON_MAY_BE_INCORRECT,
 };

 static struct ti_cpufreq_soc_data am62a7_soc_data = {
@@ -349,6 +351,9 @@ static int ti_cpufreq_get_efuse(struct
ti_cpufreq_data *opp_data,
        u32 efuse;
        int ret;

+       if (opp_data->soc_data->quirks & TI_QUIRK_SYSCON_MAY_BE_INCORRECT )
+               opp_data->soc_data->efuse_offset = 0x0018;
+
        ret = regmap_read(opp_data->syscon,
opp_data->soc_data->efuse_offset,
                          &efuse);
        if (opp_data->soc_data->quirks & TI_QUIRK_SYSCON_MAY_BE_MISSING
&& ret == -EIO) {

---------------------------------------------------------------------------->8

Then, additionally read the soc_data->syscon value, compare it against
some hard coded value to check if the address needs the 0x0018 offset or
not... All this feels extremely hackish and hence I was against doing
this.

Am I missing some other obvious way to distinguish between old/new DT? I
don't suppose we can just go ahead and create a new binding just for
this.

-- 
Best regards,
Dhruva Gole
Texas Instruments Incorporated
Re: [PATCH v5 0/6] ti: k3-am62{a,p}x-sk: add opp frequencies
Posted by Nishanth Menon 2 months ago
On 13:17-20240925, Dhruva Gole wrote:
> On Sep 24, 2024 at 07:15:44 -0500, Nishanth Menon wrote:
> > On 15:20-20240924, Dhruva Gole wrote:
> > [...]
> > > 
> > > I am sorry that this breaks compatibility with older AM625 devicetree.
> > > However, the old devicetree was marking the entire wkup_conf as "syscon",
> > > "simple-mfd" which was wrong and needed to be fixed.
> > > 
> > > This series finally tries to bring order to DT and the driver.
> > > 
> > > However, if there is still any way to maintain the backward
> > > compatibility, then I am open to suggestions. Please try
> > > and understand here that the ask for backward compatibility here
> > > is to ask the driver to support a case where the register offset itself
> > > was to be picked from a different node. I am not sure if there's any
> > > cleaner way to do this.
> > 
> > 
> > Have you tried to handle this with quirks? I am not in favor of breaking
> > backward compatibility.
> 
> I was thinking of something on those lines, but quirks makes sense for
> the case that there's a quirky behaviour in the SoC itself. Here it
> seems to me that we are adding a quirk to handle quirk in some old devicetree.
> 
> There's no way to detect the devicetree version or somehow distinguish
> within the driver if it's an old or a new DT. One way I could think of
> is on these lines:

I suggest going and experimenting a bit. Sorry, changes that break
backward compatibility: NAK!

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D
Re: [PATCH v5 0/6] ti: k3-am62{a,p}x-sk: add opp frequencies
Posted by Dhruva Gole 2 months ago
On Sep 25, 2024 at 07:51:15 -0500, Nishanth Menon wrote:
> On 13:17-20240925, Dhruva Gole wrote:
> > On Sep 24, 2024 at 07:15:44 -0500, Nishanth Menon wrote:
> > > On 15:20-20240924, Dhruva Gole wrote:
> > > [...]
> > > > 
> > > > I am sorry that this breaks compatibility with older AM625 devicetree.
> > > > However, the old devicetree was marking the entire wkup_conf as "syscon",
> > > > "simple-mfd" which was wrong and needed to be fixed.
> > > > 
> > > > This series finally tries to bring order to DT and the driver.
> > > > 
> > > > However, if there is still any way to maintain the backward
> > > > compatibility, then I am open to suggestions. Please try
> > > > and understand here that the ask for backward compatibility here
> > > > is to ask the driver to support a case where the register offset itself
> > > > was to be picked from a different node. I am not sure if there's any
> > > > cleaner way to do this.
> > > 
> > > 
> > > Have you tried to handle this with quirks? I am not in favor of breaking
> > > backward compatibility.
> > 
> > I was thinking of something on those lines, but quirks makes sense for
> > the case that there's a quirky behaviour in the SoC itself. Here it
> > seems to me that we are adding a quirk to handle quirk in some old devicetree.
> > 
> > There's no way to detect the devicetree version or somehow distinguish
> > within the driver if it's an old or a new DT. One way I could think of
> > is on these lines:
> 
> I suggest going and experimenting a bit. Sorry, changes that break
> backward compatibility: NAK!

OK, let me try using some information from old DT to distinguish and add
the offset based on that. Sending those patches soon.

-- 
Best regards,
Dhruva Gole
Texas Instruments Incorporated