[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8a4692f8-4366-49f2-bcef-b087a7bcc24c@ti.com>
Date: Tue, 13 Jan 2026 21:34:34 +0530
From: "Padhi, Beleswar" <b-padhi@...com>
To: Nishanth Menon <nm@...com>
CC: <andersson@...nel.org>, <mathieu.poirier@...aro.org>, <robh@...nel.org>,
<krzk+dt@...nel.org>, <conor+dt@...nel.org>, <vigneshr@...com>,
<kristo@...nel.org>, <afd@...com>, <u-kumar1@...com>, <hnagalla@...com>,
<linux-remoteproc@...r.kernel.org>, <devicetree@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v2 2/3] arm64: dts: ti:
k3-{j784s4-j742s2/j721s2}-mcu-wakeup: Add HSM M4F node
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@...10000 {
>> /* reserved for MCU_R5F0_1 */
>> status = "reserved";
>> };
>> +
>> + hsm_m4fss: m4fss@...00000 {
> 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@...00000", not "m4fss@...00000".
>
> 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
> [...]
Powered by blists - more mailing lists