[PATCH v3 0/5] Add OPP_LOW support for J7200
This series adds OPP_LOW spec data in k3_avs driver and enables a config option to select the OPP_LOW performance point.
J7200 SOC supports OPP_LOW and OPP_NOM as two Operating Performance Points as per (7.5 Operating Performance Points) section in the Datasheet [0]. - A72SS/MSMC at 2 GHz/1GHz operation must use OPP_NOM. - A72SS/MSMC at 1 GHz/500 MHz operation can use OPP_NOM or OPP_LOW voltage (though OPP_LOW voltage is recommended to reduce power consumption).
The actual OPP voltage for the device is read from the efuse and updated in k3_avs_probe().
The default j7200 devicetree and k3_avs driver set OPP_NOM spec frequency and voltage.
In the board init file, if K3_OPP_LOW config is enabled, Check if OPP_LOW AVS voltage read from efuse is valid and update frequency (A72 and MSMC) and voltage (VDD_CPU) as per the OPP_LOW spec.
[0]: https://www.ti.com/lit/gpn/dra821u (J7200 Datasheet)
Test logs: https://gist.github.com/aniket-l/328ad93ed60c2419ed7be9f85e6b6075 - With series applied on master and CONFIG_K3_OPP_LOW enabled in j7200_evm_r5_defconfig - Logs shown with and without efuse register programmed for OPP_0 (Errors out if OPP_0 not found, programs OPP_LOW spec if found) - Voltage update verified using 'i2c md 0x4c 0xe' in u-boot - Frequency update verified using 'k3conf clock dump' in linux
--- v4: * Manorit - Update function name to k3_avs_check_opp and update description - Move Kconfig definition to arch/arm/mach-k3/r5/Kconfig
- Update commit messages - Fixup patch styling problems - In previous versions, fdt_fixup_a72ss_clock_frequency() assumed a fixed order of assigned-clock-rates in the DT to update clk freqs. This may not always be a valid assumption. So, add a new function get_clock_index_by_dev_id() to search the indices for A72 CPU and MSMC clk IDs and then use that index to update the clock rates in place. - Link to v3: https://lore.kernel.org/u-boot/20241116071615.839623-1-a-limaye@ti.com/
v3: * Manorit - Use more descriptive name for fdt_fixup_a72ss_clock_frequency() and make function static. - Move error prints (with error codes) before else conditions. Helps with code readability to map error print with the function. - Remove k3_avs_set_opp() from board_init_f altogether. Reasoning being that the value being set through k3_avs_set_opp() will anyway be (correctly) overridden by the k3_avs_notify_freq() call later in the boot process, when a72 freq is actually set from clk_k3.
- Add msmc clock at the end to preserve current ordering of core and gtc clocks - Add Kconfig dependency on K3_AVS0 and Update commit msg to make it more clear
- Link to v2: https://lore.kernel.org/u-boot/20241023130033.1826413-1-a-limaye@ti.com/
v2: * Neha - Fix indentation - Updates to commit msgs
- Re-format patches 3/5 and 4/5 with logical changes in each patch
- Link to v1: https://lore.kernel.org/u-boot/20241017062911.2241167-1-a-limaye@ti.com/
Reid Tonking (5): arm: dts: k3-j7200-r5-common: Add msmc clk to a72 node misc: k3_avs: Add OPP_LOW voltage and frequency to vd_data misc: k3_avs: Check validity of efuse voltage data arm: mach-k3: j721e-init.c: Add support for CONFIG_K3_OPP_LOW configs: j7200_evm_r5_defconfig: Define K3_OPP_LOW
.../arm/dts/k3-j7200-r5-common-proc-board.dts | 10 ++--- arch/arm/mach-k3/Kconfig | 7 ++++ arch/arm/mach-k3/j721e/j721e_init.c | 41 ++++++++++++++++++- configs/j7200_evm_r5_defconfig | 1 + drivers/misc/k3_avs.c | 34 +++++++++++++++ include/k3-avs.h | 2 + 6 files changed, 89 insertions(+), 6 deletions(-)
From: Reid Tonking reidt@ti.com
The j7200 SOC has a single DDR controller and hence no need for configuring the MSMC interleaver. Hence we do not have an explicit node for MSMC in j7200 DT, unlike j721s2/j784s4.
Also, MSMC clk id is described under A72SS0_CORE0 Device in TISCI documentation [0].
Considering the above, define the MSMC clk in the a72 node.
[0]: https://software-dl.ti.com/tisci/esd/latest/5_soc_doc/j7200/clocks.html#cloc...
Signed-off-by: Reid Tonking reidt@ti.com Signed-off-by: Aniket Limaye a-limaye@ti.com ---
v4: - Fixup patch styling problems - Link to v3: https://lore.kernel.org/u-boot/20241116071615.839623-2-a-limaye@ti.com/
v3: - Add msmc clock at the end to preserve current ordering of core and gtc clocks - Link to v2: https://lore.kernel.org/u-boot/20241023130033.1826413-2-a-limaye@ti.com/
v2: - Update commit msg to justify location of msmc clk in DT - Link to v1: https://lore.kernel.org/u-boot/20241017062911.2241167-2-a-limaye@ti.com/ --- arch/arm/dts/k3-j7200-r5-common-proc-board.dts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/arch/arm/dts/k3-j7200-r5-common-proc-board.dts b/arch/arm/dts/k3-j7200-r5-common-proc-board.dts index f096b102793..06fffe2a11b 100644 --- a/arch/arm/dts/k3-j7200-r5-common-proc-board.dts +++ b/arch/arm/dts/k3-j7200-r5-common-proc-board.dts @@ -23,11 +23,12 @@ <&k3_pds 202 TI_SCI_PD_EXCLUSIVE>, <&k3_pds 4 TI_SCI_PD_EXCLUSIVE>; resets = <&k3_reset 202 0>; - clocks = <&k3_clks 61 1>, <&k3_clks 202 2>; - clock-names = "gtc", "core"; - assigned-clocks = <&k3_clks 202 2>, <&k3_clks 61 1>, <&k3_clks 323 0>; - assigned-clock-parents= <0>, <0>, <&k3_clks 323 2>; - assigned-clock-rates = <2000000000>, <200000000>; + clocks = <&k3_clks 61 1>, <&k3_clks 202 2>, <&k3_clks 4 1> ; + clock-names = "gtc", "core", "msmc"; + assigned-clocks = <&k3_clks 202 2>, <&k3_clks 61 1>, <&k3_clks 4 1>, + <&k3_clks 323 0>; + assigned-clock-parents= <0>, <0>, <0>, <&k3_clks 323 2>; + assigned-clock-rates = <2000000000>, <200000000>, <1000000000>; ti,sci = <&dmsc>; ti,sci-proc-id = <32>; ti,sci-host-id = <10>;
Hi Aniket,
On 06:02-20241119, Aniket Limaye wrote:
From: Reid Tonking reidt@ti.com
The j7200 SOC has a single DDR controller and hence no need for configuring the MSMC interleaver. Hence we do not have an explicit node for MSMC in j7200 DT, unlike j721s2/j784s4.
Also, MSMC clk id is described under A72SS0_CORE0 Device in TISCI documentation [0].
Considering the above, define the MSMC clk in the a72 node.
Signed-off-by: Reid Tonking reidt@ti.com
[..]
arch/arm/dts/k3-j7200-r5-common-proc-board.dts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/arch/arm/dts/k3-j7200-r5-common-proc-board.dts b/arch/arm/dts/k3-j7200-r5-common-proc-board.dts index f096b102793..06fffe2a11b 100644 --- a/arch/arm/dts/k3-j7200-r5-common-proc-board.dts +++ b/arch/arm/dts/k3-j7200-r5-common-proc-board.dts @@ -23,11 +23,12 @@ <&k3_pds 202 TI_SCI_PD_EXCLUSIVE>, <&k3_pds 4 TI_SCI_PD_EXCLUSIVE>; resets = <&k3_reset 202 0>;
clocks = <&k3_clks 61 1>, <&k3_clks 202 2>;clock-names = "gtc", "core";assigned-clocks = <&k3_clks 202 2>, <&k3_clks 61 1>, <&k3_clks 323 0>;assigned-clock-parents= <0>, <0>, <&k3_clks 323 2>;assigned-clock-rates = <2000000000>, <200000000>;
clocks = <&k3_clks 61 1>, <&k3_clks 202 2>, <&k3_clks 4 1> ;clock-names = "gtc", "core", "msmc";assigned-clocks = <&k3_clks 202 2>, <&k3_clks 61 1>, <&k3_clks 4 1>,<&k3_clks 323 0>;assigned-clock-parents= <0>, <0>, <0>, <&k3_clks 323 2>;assigned-clock-rates = <2000000000>, <200000000>, <1000000000>;
Reviewed-by: Manorit Chawdhry m-chawdhry@ti.com
Regards, Manorit
ti,sci = <&dmsc>; ti,sci-proc-id = <32>; ti,sci-host-id = <10>;-- 2.47.0
From: Reid Tonking reidt@ti.com
J7200 SOC supports OPP_LOW and OPP_NOM as two Operating Performance Points as per (7.5 Operating Performance Points) section in the Datasheet [0]. - A72SS/MSMC at 2 GHz/1GHz operation must use OPP_NOM. - A72SS/MSMC at 1 GHz/500 MHz operation can use OPP_NOM or OPP_LOW voltage (though OPP_LOW voltage is recommended to reduce power consumption).
Add OPP_LOW frequency->voltage entry to vd_data.
The actual OPP voltage for the device is read from the efuse and updated in k3_avs_probe(). OPP_NOM corresponds to OPP_1 and OPP_LOW to OPP_0 efuse register fields, as described in the Datasheet [0] The register offsets and fields are described in the TRM (5.2.6.1.5 WKUP_VTM_VD_OPPVID_j Register) [1].
[0]: https://www.ti.com/lit/gpn/dra821u (J7200 Datasheet) [1]: https://www.ti.com/lit/pdf/spruiu1 (J7200 TRM)
Signed-off-by: Reid Tonking reidt@ti.com Signed-off-by: Aniket Limaye a-limaye@ti.com --- drivers/misc/k3_avs.c | 4 ++++ include/k3-avs.h | 1 + 2 files changed, 5 insertions(+)
diff --git a/drivers/misc/k3_avs.c b/drivers/misc/k3_avs.c index 99a18a109b7..9d950d034a5 100644 --- a/drivers/misc/k3_avs.c +++ b/drivers/misc/k3_avs.c @@ -501,6 +501,10 @@ static struct vd_data j721e_vd_data[] = { .dev_id = 202, /* J721E_DEV_A72SS0_CORE0 */ .clk_id = 2, /* ARM clock */ .opps = { + [AM6_OPP_LOW] = { + .volt = 0, /* voltage TBD after OPP fuse reading */ + .freq = 1000000000, + }, [AM6_OPP_NOM] = { .volt = 880000, /* TBD in DM */ .freq = 2000000000, diff --git a/include/k3-avs.h b/include/k3-avs.h index 1014d5d114d..f6f1031c9cc 100644 --- a/include/k3-avs.h +++ b/include/k3-avs.h @@ -20,6 +20,7 @@
#define NUM_OPPS 4
+#define AM6_OPP_LOW 0 #define AM6_OPP_NOM 1 #define AM6_OPP_OD 2 #define AM6_OPP_TURBO 3
From: Reid Tonking reidt@ti.com
k3_avs driver checks opp_ids when probing and overwrites the voltage values in vd_data for the respective board. The new k3_avs_check_opp() can be called from board files to check the efuse data and returns 0 if valid.
Also add the same check in k3_avs_program_voltage() to error out if the efuse data was not valid.
Signed-off-by: Reid Tonking reidt@ti.com Signed-off-by: Aniket Limaye a-limaye@ti.com ---
v4: - Update function name to k3_avs_check_opp and update description - Link to v3: https://lore.kernel.org/u-boot/20241116071615.839623-4-a-limaye@ti.com/
v3: - Fix voltage spelling in k3_check_opp() function description. No functional change - Link to v2: https://lore.kernel.org/u-boot/20241023130033.1826413-4-a-limaye@ti.com/
v2: - Update to also handle invalid efuse voltage reading in k3_avs_program_voltage() and reflect the same change in commit msg. - Link to v1: https://lore.kernel.org/u-boot/20241017062911.2241167-4-a-limaye@ti.com/ --- drivers/misc/k3_avs.c | 32 ++++++++++++++++++++++++++++++++ include/k3-avs.h | 1 + 2 files changed, 33 insertions(+)
diff --git a/drivers/misc/k3_avs.c b/drivers/misc/k3_avs.c index 9d950d034a5..0774e0a4c9e 100644 --- a/drivers/misc/k3_avs.c +++ b/drivers/misc/k3_avs.c @@ -121,6 +121,11 @@ static int k3_avs_program_voltage(struct k3_avs_privdata *priv, if (!vd->supply) return -ENODEV;
+ if (!volt) { + dev_err(priv->dev, "No efuse found for opp_%d\n", opp_id); + return -EINVAL; + } + vd->opp = opp_id; vd->flags |= VD_FLAG_INIT_DONE;
@@ -192,6 +197,33 @@ static int match_opp(struct vd_data *vd, u32 freq) return -EINVAL; }
+/** + * k3_check_opp: Check for presence of opp efuse + * @dev: AVS device + * @vdd_id: voltage domain ID + * @opp_id: opp id to check if voltage is present + * + * Checks to see if an opp has voltage. k3_avs probe will populate + * voltage data if efuse is present. Returns 0 if data is valid. + */ +int k3_avs_check_opp(struct udevice *dev, int vdd_id, int opp_id) +{ + struct k3_avs_privdata *priv = dev_get_priv(dev); + struct vd_data *vd; + int volt; + + vd = get_vd(priv, vdd_id); + if (!vd) + return -EINVAL; + + volt = vd->opps[opp_id].volt; + if (volt) + return 0; + + printf("No efuse found for opp_%d\n", opp_id); + return -EINVAL; +} + /** * k3_avs_notify_freq: Notify clock rate change towards AVS subsystem * @dev_id: Device ID for the clock to be changed diff --git a/include/k3-avs.h b/include/k3-avs.h index f6f1031c9cc..5a973e4ed45 100644 --- a/include/k3-avs.h +++ b/include/k3-avs.h @@ -27,5 +27,6 @@
int k3_avs_set_opp(struct udevice *dev, int vdd_id, int opp_id); int k3_avs_notify_freq(int dev_id, int clk_id, u32 freq); +int k3_avs_check_opp(struct udevice *dev, int vdd_id, int opp_id);
#endif
Hi Aniket,
On 06:02-20241119, Aniket Limaye wrote:
From: Reid Tonking reidt@ti.com
k3_avs driver checks opp_ids when probing and overwrites the voltage values in vd_data for the respective board. The new k3_avs_check_opp() can be called from board files to check the efuse data and returns 0 if valid.
Also add the same check in k3_avs_program_voltage() to error out if the efuse data was not valid.
Signed-off-by: Reid Tonking reidt@ti.com Signed-off-by: Aniket Limaye a-limaye@ti.com
[..]
drivers/misc/k3_avs.c | 32 ++++++++++++++++++++++++++++++++ include/k3-avs.h | 1 + 2 files changed, 33 insertions(+)
diff --git a/drivers/misc/k3_avs.c b/drivers/misc/k3_avs.c index 9d950d034a5..0774e0a4c9e 100644 --- a/drivers/misc/k3_avs.c +++ b/drivers/misc/k3_avs.c @@ -121,6 +121,11 @@ static int k3_avs_program_voltage(struct k3_avs_privdata *priv, if (!vd->supply) return -ENODEV;
- if (!volt) {
dev_err(priv->dev, "No efuse found for opp_%d\n", opp_id);return -EINVAL;- }
- vd->opp = opp_id; vd->flags |= VD_FLAG_INIT_DONE;
@@ -192,6 +197,33 @@ static int match_opp(struct vd_data *vd, u32 freq) return -EINVAL; }
+/**
- k3_check_opp: Check for presence of opp efuse
- @dev: AVS device
- @vdd_id: voltage domain ID
- @opp_id: opp id to check if voltage is present
- Checks to see if an opp has voltage. k3_avs probe will populate
- voltage data if efuse is present. Returns 0 if data is valid.
- */
+int k3_avs_check_opp(struct udevice *dev, int vdd_id, int opp_id) +{
- struct k3_avs_privdata *priv = dev_get_priv(dev);
- struct vd_data *vd;
- int volt;
- vd = get_vd(priv, vdd_id);
- if (!vd)
return -EINVAL;- volt = vd->opps[opp_id].volt;
- if (volt)
return 0;- printf("No efuse found for opp_%d\n", opp_id);
- return -EINVAL;
+}
/**
- k3_avs_notify_freq: Notify clock rate change towards AVS subsystem
- @dev_id: Device ID for the clock to be changed
diff --git a/include/k3-avs.h b/include/k3-avs.h index f6f1031c9cc..5a973e4ed45 100644 --- a/include/k3-avs.h +++ b/include/k3-avs.h @@ -27,5 +27,6 @@
int k3_avs_set_opp(struct udevice *dev, int vdd_id, int opp_id); int k3_avs_notify_freq(int dev_id, int clk_id, u32 freq); +int k3_avs_check_opp(struct udevice *dev, int vdd_id, int opp_id);
Reviewed-by: Manorit Chawdhry m-chawdhry@ti.com
Regards, Manorit
#endif
2.47.0
The default j7200 devicetree and k3_avs driver set 2GHz/1GHz frequency for A72/MSMC clks and the OPP_NOM voltage.
J7200 SOCs may support OPP_LOW Operating Performance Point: 1GHz/500MHz clks for A72/MSMC and OPP_LOW AVS voltage read from efuse.
Hence, add a config check in board_init_f() to select OPP_LOW specs: - Check if OPP_LOW AVS voltage read from efuse is valid. - Use the device IDs and clock IDs (TISCI docs [0]) to find the A72 and MSMC clock frequencies in the devicetree. - Fixup the clock frequencies in devicetree as per OPP_LOW spec.
k3_avs driver programs the OPP_LOW AVS voltage for VDD_CPU through k3_avs_notify_freq() callback from clk_k3.
[0]: https://software-dl.ti.com/tisci/esd/latest/5_soc_doc/j7200/clocks.html
Signed-off-by: Aniket Limaye a-limaye@ti.com ---
v4: - Fixup patch styling problems - In previous versions, fdt_fixup_a72ss_clock_frequency() assumed a fixed order of assigned-clock-rates in the DT to update clk freqs. This may not always be a valid assumption. So, add a new function get_clock_index_by_dev_id() to search the indices for A72 CPU and MSMC clk IDs and then use that index to update the clock rates in place. - Link to v3: https://lore.kernel.org/u-boot/20241116071615.839623-5-a-limaye@ti.com/
v3: - Use more descriptive name for fdt_fixup_a72ss_clock_frequency() and make function static. - Add error codes to prints. - Move error prints (with error codes) before else conditions. Helps with code readability to map error print with the function. - Remove k3_avs_set_opp() from here altogether. Reasoning being that the value being set through k3_avs_set_opp() will anyway be (correctly) overridden by the k3_avs_notify_freq() call later in the boot process, when a72 freq is actually set from clk_k3. - Link to v2: https://lore.kernel.org/u-boot/20241023130033.1826413-5-a-limaye@ti.com/
v2: - Fix indentation in fix_freq() - Remove the efuse data check addition from this commit, as it's not related to adding support for CONFIG_K3_OPP_LOW. The same addition was moved to the previous patch in this series. - Link to v1: https://lore.kernel.org/u-boot/20241017062911.2241167-5-a-limaye@ti.com/ --- arch/arm/mach-k3/j721e/j721e_init.c | 92 ++++++++++++++++++++++++++++- 1 file changed, 91 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-k3/j721e/j721e_init.c b/arch/arm/mach-k3/j721e/j721e_init.c index e9ed8cb267c..805b28af8e4 100644 --- a/arch/arm/mach-k3/j721e/j721e_init.c +++ b/arch/arm/mach-k3/j721e/j721e_init.c @@ -19,6 +19,7 @@ #include <fdtdec.h> #include <mmc.h> #include <remoteproc.h> +#include <k3-avs.h>
#include "../sysfw-loader.h" #include "../common.h" @@ -39,6 +40,12 @@ #define NB_THREADMAP_BIT0 BIT(0) #define NB_THREADMAP_BIT1 BIT(1)
+/* TISCI DEV ID for A72, MSMC Clock */ +#define DEV_A72SS0_CORE0_0_ID 202 +#define DEV_A72SS0_CORE0_0_ARM_CLK_CLK_ID 2 +#define DEV_A72SS0_CORE0_ID 4 +#define DEV_A72SS0_CORE0_MSMC_CLK_ID 1 + #ifdef CONFIG_K3_LOAD_SYSFW struct fwl_data cbass_hc_cfg0_fwls[] = { #if defined(CONFIG_TARGET_J721E_R5_EVM) @@ -147,6 +154,78 @@ static void setup_navss_nb(void) writel(NB_THREADMAP_BIT1, (uintptr_t)NAVSS0_NBSS_NB1_CFG_NB_THREADMAP); }
+#if defined(CONFIG_CPU_V7R) && defined(CONFIG_K3_AVS0) +static int get_clock_index_by_dev_id(ofnode node, u32 dev_id, u32 clk_id) +{ + ofnode clknode; + int count, i; + struct ofnode_phandle_args phandle_args; + + clknode = ofnode_by_compatible(ofnode_null(), "ti,k2g-sci-clk"); + if (!ofnode_valid(clknode)) { + printf("%s: clock-controller not found\n", __func__); + return -ENODEV; + } + + count = ofnode_count_phandle_with_args(node, "assigned-clocks", "#clock-cells", 0); + for (i = 0; i < count; i++) { + if (ofnode_parse_phandle_with_args(node, "assigned-clocks", + "#clock-cells", 0, i, &phandle_args)) { + printf("%s: Could not parse assigned-clocks at index %d\n", __func__, i); + continue; + } + if (ofnode_equal(clknode, phandle_args.node) && + phandle_args.args[0] == dev_id && phandle_args.args[1] == clk_id) + return i; + } + return -1; +} + +static int fdt_fixup_a72ss_clock_frequency(void) +{ + int index, size; + u32 *rates; + ofnode node; + + node = ofnode_by_compatible(ofnode_null(), "ti,am654-rproc"); + if (!ofnode_valid(node)) { + printf("%s: A72 not found\n", __func__); + return -ENODEV; + } + + rates = fdt_getprop_w(ofnode_to_fdt(node), ofnode_to_offset(node), + "assigned-clock-rates", &size); + if (!rates) { + printf("%s: Wrong A72 assigned-clocks-rates configuration\n", __func__); + return -1; + } + + /* Update A72 Clock Frequency to OPP_LOW spec */ + index = get_clock_index_by_dev_id(node, + DEV_A72SS0_CORE0_0_ID, + DEV_A72SS0_CORE0_0_ARM_CLK_CLK_ID); + if (index < 0 || index >= (size / sizeof(u32))) { + printf("%s: Wrong A72 assigned-clocks configuration\n", __func__); + return -1; + } + rates[index] = cpu_to_fdt32(1000000000); + printf("Changed A72 CPU frequency to %dHz in DT\n", 1000000000); + + /* Update MSMC Clock Frequency to OPP_LOW spec */ + index = get_clock_index_by_dev_id(node, + DEV_A72SS0_CORE0_ID, + DEV_A72SS0_CORE0_MSMC_CLK_ID); + if (index < 0 || index >= (size / sizeof(u32))) { + printf("%s: Wrong A72 assigned-clocks configuration\n", __func__); + return -1; + } + rates[index] = cpu_to_fdt32(500000000); + printf("Changed MSMC frequency to %dHz in DT\n", 500000000); + + return 0; +} +#endif + /* * This uninitialized global variable would normal end up in the .bss section, * but the .bss is cleared between writing and reading this variable, so move @@ -301,8 +380,19 @@ void board_init_f(ulong dummy) #if defined(CONFIG_CPU_V7R) && defined(CONFIG_K3_AVS0) ret = uclass_get_device_by_driver(UCLASS_MISC, DM_DRIVER_GET(k3_avs), &dev); - if (ret) + if (ret) { printf("AVS init failed: %d\n", ret); + } else if (IS_ENABLED(CONFIG_K3_OPP_LOW)) { + ret = k3_avs_check_opp(dev, J721E_VDD_MPU, AM6_OPP_LOW); + if (ret) { + printf("OPP_LOW: k3_avs_check_opp failed: %d\n", ret); + } else { + ret = fdt_fixup_a72ss_clock_frequency(); + if (ret) + printf("OPP_LOW: fdt_fixup_a72ss_clock_frequency failed: %d\n", + ret); + } + } #endif
#if defined(CONFIG_K3_J721E_DDRSS)
Hi Aniket,
On 06:02-20241119, Aniket Limaye wrote:
The default j7200 devicetree and k3_avs driver set 2GHz/1GHz frequency for A72/MSMC clks and the OPP_NOM voltage.
J7200 SOCs may support OPP_LOW Operating Performance Point: 1GHz/500MHz clks for A72/MSMC and OPP_LOW AVS voltage read from efuse.
Hence, add a config check in board_init_f() to select OPP_LOW specs:
- Check if OPP_LOW AVS voltage read from efuse is valid.
- Use the device IDs and clock IDs (TISCI docs [0]) to find the A72 and MSMC clock frequencies in the devicetree.
- Fixup the clock frequencies in devicetree as per OPP_LOW spec.
k3_avs driver programs the OPP_LOW AVS voltage for VDD_CPU through k3_avs_notify_freq() callback from clk_k3.
Signed-off-by: Aniket Limaye a-limaye@ti.com
[..]
arch/arm/mach-k3/j721e/j721e_init.c | 92 ++++++++++++++++++++++++++++- 1 file changed, 91 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-k3/j721e/j721e_init.c b/arch/arm/mach-k3/j721e/j721e_init.c index e9ed8cb267c..805b28af8e4 100644 --- a/arch/arm/mach-k3/j721e/j721e_init.c +++ b/arch/arm/mach-k3/j721e/j721e_init.c @@ -19,6 +19,7 @@
[..]
+#if defined(CONFIG_CPU_V7R) && defined(CONFIG_K3_AVS0) +static int get_clock_index_by_dev_id(ofnode node, u32 dev_id, u32 clk_id)
I think the following function can be re-usable by other K3 socs too, can we put it in some common place instead of j721e_init? Maybe clk-k3.c and exported from there?
+static int fdt_fixup_a72ss_clock_frequency(void) +{
- int index, size;
- u32 *rates;
- ofnode node;
- node = ofnode_by_compatible(ofnode_null(), "ti,am654-rproc");
- if (!ofnode_valid(node)) {
printf("%s: A72 not found\n", __func__);return -ENODEV;- }
- rates = fdt_getprop_w(ofnode_to_fdt(node), ofnode_to_offset(node),
"assigned-clock-rates", &size);- if (!rates) {
printf("%s: Wrong A72 assigned-clocks-rates configuration\n", __func__);return -1;- }
- /* Update A72 Clock Frequency to OPP_LOW spec */
- index = get_clock_index_by_dev_id(node,
DEV_A72SS0_CORE0_0_ID,DEV_A72SS0_CORE0_0_ARM_CLK_CLK_ID);- if (index < 0 || index >= (size / sizeof(u32))) {
When would we encounter the second if..?
printf("%s: Wrong A72 assigned-clocks configuration\n", __func__);return -1;- }
- rates[index] = cpu_to_fdt32(1000000000);
- printf("Changed A72 CPU frequency to %dHz in DT\n", 1000000000);
- /* Update MSMC Clock Frequency to OPP_LOW spec */
- index = get_clock_index_by_dev_id(node,
DEV_A72SS0_CORE0_ID,DEV_A72SS0_CORE0_MSMC_CLK_ID);- if (index < 0 || index >= (size / sizeof(u32))) {
printf("%s: Wrong A72 assigned-clocks configuration\n", __func__);return -1;- }
- rates[index] = cpu_to_fdt32(500000000);
- printf("Changed MSMC frequency to %dHz in DT\n", 500000000);
- return 0;
+} +#endif
[..]
Regards, Manorit
Hi Manorit,
On 19/11/24 11:42, Manorit Chawdhry wrote:
Hi Aniket,
On 06:02-20241119, Aniket Limaye wrote:
The default j7200 devicetree and k3_avs driver set 2GHz/1GHz frequency for A72/MSMC clks and the OPP_NOM voltage.
J7200 SOCs may support OPP_LOW Operating Performance Point: 1GHz/500MHz clks for A72/MSMC and OPP_LOW AVS voltage read from efuse.
Hence, add a config check in board_init_f() to select OPP_LOW specs:
- Check if OPP_LOW AVS voltage read from efuse is valid.
- Use the device IDs and clock IDs (TISCI docs [0]) to find the A72 and MSMC clock frequencies in the devicetree.
- Fixup the clock frequencies in devicetree as per OPP_LOW spec.
k3_avs driver programs the OPP_LOW AVS voltage for VDD_CPU through k3_avs_notify_freq() callback from clk_k3.
Signed-off-by: Aniket Limaye a-limaye@ti.com
[..]
arch/arm/mach-k3/j721e/j721e_init.c | 92 ++++++++++++++++++++++++++++- 1 file changed, 91 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-k3/j721e/j721e_init.c b/arch/arm/mach-k3/j721e/j721e_init.c index e9ed8cb267c..805b28af8e4 100644 --- a/arch/arm/mach-k3/j721e/j721e_init.c +++ b/arch/arm/mach-k3/j721e/j721e_init.c @@ -19,6 +19,7 @@
[..]
+#if defined(CONFIG_CPU_V7R) && defined(CONFIG_K3_AVS0) +static int get_clock_index_by_dev_id(ofnode node, u32 dev_id, u32 clk_id)
I think the following function can be re-usable by other K3 socs too, can we put it in some common place instead of j721e_init? Maybe clk-k3.c and exported from there?
Yeah that does make sense to do... There are already similar usages, I think, in other k3 platforms. Having said that I would like to take that up as a separate series since the DT index search is not directly related to OPP_LOW Perf Point setting for j7200.
Let me know if you think that the k3 DT clock index search belongs in the same series itself...
+static int fdt_fixup_a72ss_clock_frequency(void) +{
- int index, size;
- u32 *rates;
- ofnode node;
- node = ofnode_by_compatible(ofnode_null(), "ti,am654-rproc");
- if (!ofnode_valid(node)) {
printf("%s: A72 not found\n", __func__);return -ENODEV;- }
- rates = fdt_getprop_w(ofnode_to_fdt(node), ofnode_to_offset(node),
"assigned-clock-rates", &size);- if (!rates) {
printf("%s: Wrong A72 assigned-clocks-rates configuration\n", __func__);return -1;- }
- /* Update A72 Clock Frequency to OPP_LOW spec */
- index = get_clock_index_by_dev_id(node,
DEV_A72SS0_CORE0_0_ID,DEV_A72SS0_CORE0_0_ARM_CLK_CLK_ID);- if (index < 0 || index >= (size / sizeof(u32))) {
When would we encounter the second if..?
Assuming you mean: index >= (size / sizeof(u32))
The RHS is the count of "assigned-clock-rates" in the node and the LHS (index) is the id of the "assigned-clocks". It's possible (and I see it in a few cases too) that count of "assigned-clocks" > count of "assigned-clock-rates". In that case the "index" here, even if parsed correctly, could turn out to be outside of the bounds of the array "rates". Which should be check against.
Regards, Aniket
printf("%s: Wrong A72 assigned-clocks configuration\n", __func__);return -1;- }
- rates[index] = cpu_to_fdt32(1000000000);
- printf("Changed A72 CPU frequency to %dHz in DT\n", 1000000000);
- /* Update MSMC Clock Frequency to OPP_LOW spec */
- index = get_clock_index_by_dev_id(node,
DEV_A72SS0_CORE0_ID,DEV_A72SS0_CORE0_MSMC_CLK_ID);- if (index < 0 || index >= (size / sizeof(u32))) {
printf("%s: Wrong A72 assigned-clocks configuration\n", __func__);return -1;- }
- rates[index] = cpu_to_fdt32(500000000);
- printf("Changed MSMC frequency to %dHz in DT\n", 500000000);
- return 0;
+} +#endif
[..]
Regards, Manorit
Hi Aniket,
On 13:02-20241119, Aniket Limaye wrote:
Hi Manorit,
On 19/11/24 11:42, Manorit Chawdhry wrote:
Hi Aniket,
On 06:02-20241119, Aniket Limaye wrote:
The default j7200 devicetree and k3_avs driver set 2GHz/1GHz frequency for A72/MSMC clks and the OPP_NOM voltage.
J7200 SOCs may support OPP_LOW Operating Performance Point: 1GHz/500MHz clks for A72/MSMC and OPP_LOW AVS voltage read from efuse.
Hence, add a config check in board_init_f() to select OPP_LOW specs:
- Check if OPP_LOW AVS voltage read from efuse is valid.
- Use the device IDs and clock IDs (TISCI docs [0]) to find the A72 and MSMC clock frequencies in the devicetree.
- Fixup the clock frequencies in devicetree as per OPP_LOW spec.
k3_avs driver programs the OPP_LOW AVS voltage for VDD_CPU through k3_avs_notify_freq() callback from clk_k3.
Signed-off-by: Aniket Limaye a-limaye@ti.com
[..]
arch/arm/mach-k3/j721e/j721e_init.c | 92 ++++++++++++++++++++++++++++- 1 file changed, 91 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-k3/j721e/j721e_init.c b/arch/arm/mach-k3/j721e/j721e_init.c index e9ed8cb267c..805b28af8e4 100644 --- a/arch/arm/mach-k3/j721e/j721e_init.c +++ b/arch/arm/mach-k3/j721e/j721e_init.c @@ -19,6 +19,7 @@
[..]
+#if defined(CONFIG_CPU_V7R) && defined(CONFIG_K3_AVS0) +static int get_clock_index_by_dev_id(ofnode node, u32 dev_id, u32 clk_id)
I think the following function can be re-usable by other K3 socs too, can we put it in some common place instead of j721e_init? Maybe clk-k3.c and exported from there?
Yeah that does make sense to do... There are already similar usages, I think, in other k3 platforms. Having said that I would like to take that up as a separate series since the DT index search is not directly related to OPP_LOW Perf Point setting for j7200.
Let me know if you think that the k3 DT clock index search belongs in the same series itself...
I mean you did make the function so I think might as well put it in the correct location but it's a non blocker for me, was just a suggestion. Would let you see how you want to do it.
+static int fdt_fixup_a72ss_clock_frequency(void) +{
- int index, size;
- u32 *rates;
- ofnode node;
- node = ofnode_by_compatible(ofnode_null(), "ti,am654-rproc");
- if (!ofnode_valid(node)) {
printf("%s: A72 not found\n", __func__);return -ENODEV;- }
- rates = fdt_getprop_w(ofnode_to_fdt(node), ofnode_to_offset(node),
"assigned-clock-rates", &size);- if (!rates) {
printf("%s: Wrong A72 assigned-clocks-rates configuration\n", __func__);return -1;- }
- /* Update A72 Clock Frequency to OPP_LOW spec */
- index = get_clock_index_by_dev_id(node,
DEV_A72SS0_CORE0_0_ID,DEV_A72SS0_CORE0_0_ARM_CLK_CLK_ID);- if (index < 0 || index >= (size / sizeof(u32))) {
When would we encounter the second if..?
Assuming you mean: index >= (size / sizeof(u32))
The RHS is the count of "assigned-clock-rates" in the node and the LHS (index) is the id of the "assigned-clocks". It's possible (and I see it in a few cases too) that count of "assigned-clocks" > count of "assigned-clock-rates". In that case the "index" here, even if parsed correctly, could turn out to be outside of the bounds of the array "rates". Which should be check against.
Ah okay, thanks for explaining!
Reviewed-by: Manorit Chawdhry m-chawdhry@ti.com
Regards, Manorit
Regards, Aniket
printf("%s: Wrong A72 assigned-clocks configuration\n", __func__);return -1;- }
- rates[index] = cpu_to_fdt32(1000000000);
- printf("Changed A72 CPU frequency to %dHz in DT\n", 1000000000);
- /* Update MSMC Clock Frequency to OPP_LOW spec */
- index = get_clock_index_by_dev_id(node,
DEV_A72SS0_CORE0_ID,DEV_A72SS0_CORE0_MSMC_CLK_ID);- if (index < 0 || index >= (size / sizeof(u32))) {
printf("%s: Wrong A72 assigned-clocks configuration\n", __func__);return -1;- }
- rates[index] = cpu_to_fdt32(500000000);
- printf("Changed MSMC frequency to %dHz in DT\n", 500000000);
- return 0;
+} +#endif
[..]
Regards, Manorit
From: Reid Tonking reidt@ti.com
Define new CONFIG_K3_OPP_LOW under arm/mach-k3/r5/Kconfig and add default value to j7200_evm_r5_defconfig
Signed-off-by: Reid Tonking reidt@ti.com Signed-off-by: Aniket Limaye a-limaye@ti.com ---
v4: - Move Config definition to arch/arm/mach-k3/r5/Kconfig - Link to v3: https://lore.kernel.org/u-boot/20241116071615.839623-6-a-limaye@ti.com/
v3: - Add Kconfig dependency on K3_AVS0 - Update commit msg to make it more clear - Link to v2: https://lore.kernel.org/u-boot/20241023130033.1826413-6-a-limaye@ti.com/ --- arch/arm/mach-k3/r5/Kconfig | 7 +++++++ configs/j7200_evm_r5_defconfig | 1 + 2 files changed, 8 insertions(+)
diff --git a/arch/arm/mach-k3/r5/Kconfig b/arch/arm/mach-k3/r5/Kconfig index 878087fbf56..12335880e10 100644 --- a/arch/arm/mach-k3/r5/Kconfig +++ b/arch/arm/mach-k3/r5/Kconfig @@ -1,6 +1,13 @@ config K3_LOAD_SYSFW bool
+config K3_OPP_LOW + depends on ARCH_K3 && K3_AVS0 + bool "Enable OPP_LOW on supported TI K3 SoCs" + help + Enabling this will allow Socs with the proper efuse to run at a lower + MPU core voltage and adjust frequency according to SoC TRM + config K3_QOS bool "Enable Quality of Service (QoS) Settings for TI K3 SoCs" default y if SOC_K3_AM62A7 diff --git a/configs/j7200_evm_r5_defconfig b/configs/j7200_evm_r5_defconfig index f036a6fd46b..217759e5d1b 100644 --- a/configs/j7200_evm_r5_defconfig +++ b/configs/j7200_evm_r5_defconfig @@ -102,6 +102,7 @@ CONFIG_K3_SEC_PROXY=y CONFIG_FS_LOADER=y CONFIG_SPL_FS_LOADER=y CONFIG_K3_AVS0=y +# CONFIG_K3_OPP_LOW is not set CONFIG_SUPPORT_EMMC_BOOT=y CONFIG_SPL_MMC_HS400_SUPPORT=y CONFIG_MMC_SDHCI=y
On 11/19/2024 6:02 AM, Aniket Limaye wrote:
This series adds OPP_LOW spec data in k3_avs driver and enables a config option to select the OPP_LOW performance point.
Whoops, this series is supposed to be v4, missed to update the patch version only in the cover letter.
Will wait for other review comments before resending, if that's fine.
Regards, Aniket
J7200 SOC supports OPP_LOW and OPP_NOM as two Operating Performance Points as per (7.5 Operating Performance Points) section in the Datasheet [0].
- A72SS/MSMC at 2 GHz/1GHz operation must use OPP_NOM.
- A72SS/MSMC at 1 GHz/500 MHz operation can use OPP_NOM or OPP_LOW voltage (though OPP_LOW voltage is recommended to reduce power consumption).
The actual OPP voltage for the device is read from the efuse and updated in k3_avs_probe().
The default j7200 devicetree and k3_avs driver set OPP_NOM spec frequency and voltage.
In the board init file, if K3_OPP_LOW config is enabled, Check if OPP_LOW AVS voltage read from efuse is valid and update frequency (A72 and MSMC) and voltage (VDD_CPU) as per the OPP_LOW spec.
Test logs: https://gist.github.com/aniket-l/328ad93ed60c2419ed7be9f85e6b6075
- With series applied on master and CONFIG_K3_OPP_LOW enabled in j7200_evm_r5_defconfig
- Logs shown with and without efuse register programmed for OPP_0 (Errors out if OPP_0 not found, programs OPP_LOW spec if found)
- Voltage update verified using 'i2c md 0x4c 0xe' in u-boot
- Frequency update verified using 'k3conf clock dump' in linux
v4:
- Manorit
Update function name to k3_avs_check_opp and update description
Move Kconfig definition to arch/arm/mach-k3/r5/Kconfig
Update commit messages
Fixup patch styling problems
In previous versions, fdt_fixup_a72ss_clock_frequency() assumed a fixed order of assigned-clock-rates in the DT to update clk freqs. This may not always be a valid assumption. So, add a new function get_clock_index_by_dev_id() to search the indices for A72 CPU and MSMC clk IDs and then use that index to update the clock rates in place.
Link to v3: https://lore.kernel.org/u-boot/20241116071615.839623-1-a-limaye@ti.com/
v3:
- Manorit
Use more descriptive name for fdt_fixup_a72ss_clock_frequency() and make function static.
Move error prints (with error codes) before else conditions. Helps with code readability to map error print with the function.
Remove k3_avs_set_opp() from board_init_f altogether. Reasoning being that the value being set through k3_avs_set_opp() will anyway be (correctly) overridden by the k3_avs_notify_freq() call later in the boot process, when a72 freq is actually set from clk_k3.
Add msmc clock at the end to preserve current ordering of core and gtc clocks
Add Kconfig dependency on K3_AVS0 and Update commit msg to make it more clear
Link to v2: https://lore.kernel.org/u-boot/20241023130033.1826413-1-a-limaye@ti.com/
v2:
- Neha
Fix indentation
Updates to commit msgs
Re-format patches 3/5 and 4/5 with logical changes in each patch
Link to v1: https://lore.kernel.org/u-boot/20241017062911.2241167-1-a-limaye@ti.com/
Reid Tonking (5): arm: dts: k3-j7200-r5-common: Add msmc clk to a72 node misc: k3_avs: Add OPP_LOW voltage and frequency to vd_data misc: k3_avs: Check validity of efuse voltage data arm: mach-k3: j721e-init.c: Add support for CONFIG_K3_OPP_LOW configs: j7200_evm_r5_defconfig: Define K3_OPP_LOW
.../arm/dts/k3-j7200-r5-common-proc-board.dts | 10 ++--- arch/arm/mach-k3/Kconfig | 7 ++++ arch/arm/mach-k3/j721e/j721e_init.c | 41 ++++++++++++++++++- configs/j7200_evm_r5_defconfig | 1 + drivers/misc/k3_avs.c | 34 +++++++++++++++ include/k3-avs.h | 2 + 6 files changed, 89 insertions(+), 6 deletions(-)
On Tue, 19 Nov 2024 06:02:54 +0530, Aniket Limaye wrote:
This series adds OPP_LOW spec data in k3_avs driver and enables a config option to select the OPP_LOW performance point.
J7200 SOC supports OPP_LOW and OPP_NOM as two Operating Performance Points as per (7.5 Operating Performance Points) section in the Datasheet [0].
- A72SS/MSMC at 2 GHz/1GHz operation must use OPP_NOM.
- A72SS/MSMC at 1 GHz/500 MHz operation can use OPP_NOM or OPP_LOW voltage (though OPP_LOW voltage is recommended to reduce power consumption).
[...]
Applied to u-boot/next, thanks!
participants (4)
-
Aniket Limaye -
Limaye, Aniket
-
Manorit Chawdhry -
Tom Rini