Re: [PATCH v2 2/3] arm64: dts: ti: k3-{j784s4-j742s2/j721s2}-mcu-wakeup: Add HSM M4F node

["Padhi, Beleswar" <b-padhi@ti.com>]

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

> [...]