Hi Nishanth,
On 1/10/2026 12:57 AM, Nishanth Menon wrote:
On 16:17-20260106, Beleswar Padhi wrote:
The TI K3 J721S2, J784S4 and J742S2 SoCs have a HSM (High Security
Module) M4F core in the Wakeup Voltage Domain which could be used to run
secure services like Authentication. Add Device Tree Node definitions
for the HSM core in the respective SoC wakeup dtsi files.
[...]
diff --git a/arch/arm64/boot/dts/ti/k3-j721s2-mcu-wakeup.dtsi
b/arch/arm64/boot/dts/ti/k3-j721s2-mcu-wakeup.dtsi
index fd01437726ab4..c3d78d4a838a1 100644
--- a/arch/arm64/boot/dts/ti/k3-j721s2-mcu-wakeup.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j721s2-mcu-wakeup.dtsi
@@ -766,4 +766,19 @@ mcu_watchdog1: watchdog@40610000 {
/* reserved for MCU_R5F0_1 */
status = "reserved";
};
+
+ hsm_m4fss: m4fss@43c00000 {
You did fix this in the binding example.. but missed in dts.
The node name should use the generic type, not the instance name. It should
be "remoteproc@43c00000", not "m4fss@43c00000".
Additionally for the label, why not just use hsm: like we have for sms?
Thanks for catching this... Will fix in v2...
+ compatible = "ti,hsm-m4fss";
+ reg = <0x00 0x43c00000 0x00 0x20000>,
+ <0x00 0x43c20000 0x00 0x10000>,
+ <0x00 0x43c30000 0x00 0x10000>;
The total address range covered here is 0x43c00000-0x43c40000, which is
0x40000 bytes, matching the ranges entry. However, you're defining three
separate regions: 0x43c00000-0x43c20000 (0x20000), 0x43c20000-0x43c30000
(0x10000), and 0x43c30000-0x43c40000 (0x10000).
I assume you are doing this since the h/w integration could be
instantiated differently?
Yes... Will add a comment in v2...
+ reg-names = "sram0_0", "sram0_1", "sram1";
+ resets = <&k3_reset 304 1>;
+ firmware-name = "hsm.bin";
I am not a fan of putting firmware-name in SoC.dtsi - esp when it is
reserved,
I thought the opposite way. Since it is reserved (and not a general purpose
remote core), it is unlikely boards out there are going to use a separate
firmware. Does it make sense to override this 'required' property with
the same value in every other board level file?... Here is a diff for
just 2 of
the SoCs (J722S, AM62P). Let me know if you prefer this way and I will
fix in v2.
diff --git a/arch/arm64/boot/dts/ti/k3-am62p-verdin.dtsi
b/arch/arm64/boot/dts/ti/k3-am62p-verdin.dtsi
index 34954df692a39..a4026424b64dd 100644
--- a/arch/arm64/boot/dts/ti/k3-am62p-verdin.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62p-verdin.dtsi
@@ -1415,4 +1415,8 @@ &wkup_uart0 {
status = "disabled";
};
+&hsm {
+ firmware-name = "am62p-main-m4f-fw";
+};
+
#include "k3-am62p-ti-ipc-firmware.dtsi"
diff --git a/arch/arm64/boot/dts/ti/k3-am62p5-sk.dts
b/arch/arm64/boot/dts/ti/k3-am62p5-sk.dts
index 4f7f6f95b02ef..7b370a65238db 100644
--- a/arch/arm64/boot/dts/ti/k3-am62p5-sk.dts
+++ b/arch/arm64/boot/dts/ti/k3-am62p5-sk.dts
@@ -832,4 +832,8 @@ &mcu_uart0 {
<&system_standby>;
};
+&hsm {
+ firmware-name = "am62p-main-m4f-fw";
+};
+
#include "k3-am62p-ti-ipc-firmware.dtsi"
diff --git a/arch/arm64/boot/dts/ti/k3-am62p5-var-som.dtsi
b/arch/arm64/boot/dts/ti/k3-am62p5-var-som.dtsi
index fc5a3942cde00..79d371c54c52b 100644
--- a/arch/arm64/boot/dts/ti/k3-am62p5-var-som.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62p5-var-som.dtsi
@@ -432,4 +432,8 @@ &main_uart1 {
status = "reserved";
};
+&hsm {
+ firmware-name = "am62p-main-m4f-fw";
+};
+
#include "k3-am62p-ti-ipc-firmware.dtsi"
diff --git a/arch/arm64/boot/dts/ti/k3-am67a-beagley-ai.dts
b/arch/arm64/boot/dts/ti/k3-am67a-beagley-ai.dts
index 5255e04b9ac76..bb0c9857f907c 100644
--- a/arch/arm64/boot/dts/ti/k3-am67a-beagley-ai.dts
+++ b/arch/arm64/boot/dts/ti/k3-am67a-beagley-ai.dts
@@ -399,4 +399,8 @@ &sdhci1 {
status = "okay";
};
+&hsm {
+ firmware-name = "j722s-main-m4f-fw";
+};
+
#include "k3-j722s-ti-ipc-firmware.dtsi"
diff --git a/arch/arm64/boot/dts/ti/k3-am67a-kontron-sa67-base.dts
b/arch/arm64/boot/dts/ti/k3-am67a-kontron-sa67-base.dts
index 7169d934adac5..37f31c206f0b7 100644
--- a/arch/arm64/boot/dts/ti/k3-am67a-kontron-sa67-base.dts
+++ b/arch/arm64/boot/dts/ti/k3-am67a-kontron-sa67-base.dts
@@ -1089,3 +1089,7 @@ &wkup_uart0 {
bootph-all;
status = "reserved";
};
+
+&hsm {
+ firmware-name = "j722s-main-m4f-fw";
+};
diff --git a/arch/arm64/boot/dts/ti/k3-j722s-evm.dts
b/arch/arm64/boot/dts/ti/k3-j722s-evm.dts
index e66330c71593a..6b38488815c34 100644
--- a/arch/arm64/boot/dts/ti/k3-j722s-evm.dts
+++ b/arch/arm64/boot/dts/ti/k3-j722s-evm.dts
@@ -854,4 +854,8 @@ &mcu_i2c0 {
status = "okay";
};
+&hsm {
+ firmware-name = "j722s-main-m4f-fw";
+};
+
#include "k3-j722s-ti-ipc-firmware.dtsi"
further, so far we have been using j722s-wkup-r5f0_0-fw and
so on.. which allows for firmware specific to SoC.. which kind of makes
sense here as well.
Right, will fix this in v2...
+ ti,sci = <&sms>;
+ ti,sci-dev-id = <304>;
+ ti,sci-proc-ids = <0x80 0xff>;
+ status = "disabled";
As usual, document why? Additionally, should this be reserved?
Will fix in v2...
+ bootph-pre-ram;
"standard property"
Sorry my bad.. Will fix in v2...
Thanks,
Beleswar
[...]
