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.
>
> Signed-off-by: Beleswar Padhi <[email protected]>
> ---
> v2: Changelog:
> 1. None
>
> Link to v1:
> https://lore.kernel.org/all/[email protected]/
>
> arch/arm64/boot/dts/ti/k3-j721s2-mcu-wakeup.dtsi | 15 +++++++++++++++
> .../ti/k3-j784s4-j742s2-mcu-wakeup-common.dtsi | 15 +++++++++++++++
> 2 files changed, 30 insertions(+)
>
> 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?
> + 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?
> + 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, 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.
> + 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?
> + bootph-pre-ram;
"standard property"
Documentation/devicetree/bindings/dts-coding-style.rst - note the order:
1. "compatible"
2. "reg"
3. "ranges"
4. Standard/common properties (defined by common bindings, e.g. without
vendor-prefixes)
5. Vendor-specific properties
6. "status" (if applicable), preceded by a blank line if there is content
before the property
7. Child nodes, where each node is preceded with a blank line
> + };
> };
Same for the rest of the patches and nodes
> diff --git a/arch/arm64/boot/dts/ti/k3-j784s4-j742s2-mcu-wakeup-common.dtsi
> b/arch/arm64/boot/dts/ti/k3-j784s4-j742s2-mcu-wakeup-common.dtsi
> index cc22bfb5f5996..42565f41b7bac 100644
> --- a/arch/arm64/boot/dts/ti/k3-j784s4-j742s2-mcu-wakeup-common.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-j784s4-j742s2-mcu-wakeup-common.dtsi
> @@ -762,4 +762,19 @@ mcu_watchdog1: watchdog@40610000 {
> /* reserved for MCU_R5F0_1 */
> status = "reserved";
> };
> +
> + hsm_m4fss: m4fss@43c00000 {
> + compatible = "ti,hsm-m4fss";
> + reg = <0x00 0x43c00000 0x00 0x20000>,
> + <0x00 0x43c20000 0x00 0x10000>,
> + <0x00 0x43c30000 0x00 0x10000>;
> + reg-names = "sram0_0", "sram0_1", "sram1";
> + resets = <&k3_reset 371 1>;
> + firmware-name = "hsm.bin";
> + ti,sci = <&sms>;
> + ti,sci-dev-id = <371>;
> + ti,sci-proc-ids = <0x80 0xff>;
> + status = "disabled";
> + bootph-pre-ram;
> + };
> };
> --
> 2.34.1
>
--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5
849D 1736 249D
https://ti.com/opensource
