[PATCH 0/4] Enable DSP device for i.MX8ULP

[PATCH 0/4] Enable DSP device for i.MX8ULP

[shengjiu.wang]

From: Shengjiu Wang <shengjiu.wang@nxp.com>

The debug reset control is one of the HIFI4 DSP’s reset mechanisms used
by the debug interface; add documentation for the optional debug reset
controls and update the driver to use both the soft reset and debug reset
controller APIs (the soft reset is already documented).

A HiFi4 DSP node is added and the DSP device is enabled in the i.MX8ULP

Shengjiu Wang (4):
  dt-bindings: dsp: fsl,dsp: document debug reset control
  remoteproc: imx_dsp_rproc: Add softreset and dbgreset handling
  arm64: dts: imx8ulp: add HiFi4 DSP node
  arm64: dts: imx8ulp-evk: enable HiFi4 DSP device

 .../devicetree/bindings/dsp/fsl,dsp.yaml      |  2 +
 arch/arm64/boot/dts/freescale/imx8ulp-evk.dts | 34 +++++++++
 arch/arm64/boot/dts/freescale/imx8ulp.dtsi    | 23 ++++++
 drivers/remoteproc/imx_dsp_rproc.c            | 76 +++++--------------
 4 files changed, 80 insertions(+), 55 deletions(-)

-- 
2.34.1

[PATCH 1/4] dt-bindings: dsp: fsl,dsp: document debug reset control

[shengjiu.wang]

From: Shengjiu Wang <shengjiu.wang@nxp.com>

The HIFI4 DSP provides an additional debug reset control that can be
used by the debug interface.  This patch adds the `dbgreset` entry to
the `reset-names` property and expands the `resets` array to allow up to
three entries (runstall, softreset, dbgreset) by setting `maxItems: 3`.

Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
 Documentation/devicetree/bindings/dsp/fsl,dsp.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml b/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml
index e610b7636a08..4a75a271b7fa 100644
--- a/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml
+++ b/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml
@@ -84,12 +84,14 @@ properties:
 
   resets:
     minItems: 1
+    maxItems: 3
 
   reset-names:
     minItems: 1
     items:
       - const: runstall
       - const: softreset
+      - const: dbgreset
 
   access-controllers:
     maxItems: 1
-- 
2.34.1

[PATCH 2/4] remoteproc: imx_dsp_rproc: Add softreset and dbgreset handling

[shengjiu.wang]

From: Shengjiu Wang <shengjiu.wang@nxp.com>

Add `soft_rst` and `dbg_rst` members to `struct imx_dsp_rproc` as defined
in the DSP binding (they correspond to the softreset and dbgreset
controls).

In `imx_dsp_rproc_reset_ctrl_detect_mode` acquire the three reset controls
— runstall, softreset, and dbgreset, the latter two are optional.

Rewrite `imx8ulp_dsp_reset` to use the generic reset controller API
(`reset_control_assert` / `reset_control_deassert`) instead of direct
register writes. This change relies on the reset controller implementation
added in commit 5aac7afc752b ("reset: imx8mp-audiomix: Support i.MX8ULP SIM
LPAV").

With this new implementation the legacy `imx_dsp_rproc_mmio_*` functions
are no longer needed and can be removed.

Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
 drivers/remoteproc/imx_dsp_rproc.c | 76 +++++++++---------------------
 1 file changed, 21 insertions(+), 55 deletions(-)

diff --git a/drivers/remoteproc/imx_dsp_rproc.c b/drivers/remoteproc/imx_dsp_rproc.c
index 96d619daf3f5..0b0042525ea2 100644
--- a/drivers/remoteproc/imx_dsp_rproc.c
+++ b/drivers/remoteproc/imx_dsp_rproc.c
@@ -108,8 +108,9 @@ enum imx_dsp_rp_mbox_messages {
 
 /**
  * struct imx_dsp_rproc - DSP remote processor state
- * @regmap: regmap handler
  * @run_stall: reset control handle used for Run/Stall operation
+ * @soft_rst: reset control handle used for DSP core reset
+ * @dbg_rst: reset control handle used for DSP debug logic
  * @rproc: rproc handler
  * @dsp_dcfg: device configuration pointer
  * @clks: clocks needed by this device
@@ -125,8 +126,9 @@ enum imx_dsp_rp_mbox_messages {
  * @flags: control flags
  */
 struct imx_dsp_rproc {
-	struct regmap				*regmap;
 	struct reset_control			*run_stall;
+	struct reset_control			*soft_rst;
+	struct reset_control			*dbg_rst;
 	struct rproc				*rproc;
 	const struct imx_dsp_rproc_dcfg		*dsp_dcfg;
 	struct clk_bulk_data			clks[DSP_RPROC_CLK_MAX];
@@ -243,20 +245,15 @@ static int imx8ulp_dsp_reset(struct imx_dsp_rproc *priv)
 	struct arm_smccc_res res;
 
 	/* Put DSP into reset and stall */
-	regmap_update_bits(priv->regmap, IMX8ULP_SIM_LPAV_REG_SYSCTRL0,
-			   IMX8ULP_SYSCTRL0_DSP_RST, IMX8ULP_SYSCTRL0_DSP_RST);
-	regmap_update_bits(priv->regmap, IMX8ULP_SIM_LPAV_REG_SYSCTRL0,
-			   IMX8ULP_SYSCTRL0_DSP_STALL,
-			   IMX8ULP_SYSCTRL0_DSP_STALL);
+	reset_control_assert(priv->soft_rst);
+	reset_control_assert(priv->run_stall);
 
 	/* Configure resources of DSP through TFA */
 	arm_smccc_smc(IMX8ULP_SIP_HIFI_XRDC, 0, 0, 0, 0, 0, 0, 0, &res);
 
 	/* Take the DSP out of reset and keep stalled for FW loading */
-	regmap_update_bits(priv->regmap, IMX8ULP_SIM_LPAV_REG_SYSCTRL0,
-			   IMX8ULP_SYSCTRL0_DSP_RST, 0);
-	regmap_update_bits(priv->regmap, IMX8ULP_SIM_LPAV_REG_SYSCTRL0,
-			   IMX8ULP_SYSCTRL0_DSP_DBG_RST, 0);
+	reset_control_deassert(priv->soft_rst);
+	reset_control_deassert(priv->dbg_rst);
 
 	return 0;
 }
@@ -338,14 +335,6 @@ static int imx_dsp_rproc_handle_rsc(struct rproc *rproc, u32 rsc_type,
 	return RSC_HANDLED;
 }
 
-static int imx_dsp_rproc_mmio_start(struct rproc *rproc)
-{
-	struct imx_dsp_rproc *priv = rproc->priv;
-	const struct imx_rproc_dcfg *dcfg = priv->dsp_dcfg->dcfg;
-
-	return regmap_update_bits(priv->regmap, dcfg->src_reg, dcfg->src_mask, dcfg->src_start);
-}
-
 static int imx_dsp_rproc_reset_ctrl_start(struct rproc *rproc)
 {
 	struct imx_dsp_rproc *priv = rproc->priv;
@@ -391,14 +380,6 @@ static int imx_dsp_rproc_start(struct rproc *rproc)
 	return 0;
 }
 
-static int imx_dsp_rproc_mmio_stop(struct rproc *rproc)
-{
-	struct imx_dsp_rproc *priv = rproc->priv;
-	const struct imx_rproc_dcfg *dcfg = priv->dsp_dcfg->dcfg;
-
-	return regmap_update_bits(priv->regmap, dcfg->src_reg, dcfg->src_mask, dcfg->src_stop);
-}
-
 static int imx_dsp_rproc_reset_ctrl_stop(struct rproc *rproc)
 {
 	struct imx_dsp_rproc *priv = rproc->priv;
@@ -1024,23 +1005,6 @@ static int imx_dsp_attach_pm_domains(struct imx_dsp_rproc *priv)
 	return devm_pm_domain_attach_list(dev, NULL, &priv->pd_list);
 }
 
-static int imx_dsp_rproc_mmio_detect_mode(struct rproc *rproc)
-{
-	struct imx_dsp_rproc *priv = rproc->priv;
-	struct device *dev = rproc->dev.parent;
-	struct regmap *regmap;
-
-	regmap = syscon_regmap_lookup_by_phandle(dev->of_node, "fsl,dsp-ctrl");
-	if (IS_ERR(regmap)) {
-		dev_err(dev, "failed to find syscon\n");
-		return PTR_ERR(regmap);
-	}
-
-	priv->regmap = regmap;
-
-	return 0;
-}
-
 static int imx_dsp_rproc_reset_ctrl_detect_mode(struct rproc *rproc)
 {
 	struct imx_dsp_rproc *priv = rproc->priv;
@@ -1052,6 +1016,18 @@ static int imx_dsp_rproc_reset_ctrl_detect_mode(struct rproc *rproc)
 		return PTR_ERR(priv->run_stall);
 	}
 
+	priv->soft_rst = devm_reset_control_get_optional_exclusive(dev, "softreset");
+	if (IS_ERR(priv->soft_rst)) {
+		dev_err(dev, "Failed to get DSP soft reset control\n");
+		return PTR_ERR(priv->soft_rst);
+	}
+
+	priv->dbg_rst = devm_reset_control_get_optional_exclusive(dev, "dbgreset");
+	if (IS_ERR(priv->dbg_rst)) {
+		dev_err(dev, "Failed to get DSP debug reset control\n");
+		return PTR_ERR(priv->dbg_rst);
+	}
+
 	return 0;
 }
 
@@ -1316,12 +1292,6 @@ static const struct dev_pm_ops imx_dsp_rproc_pm_ops = {
 	RUNTIME_PM_OPS(imx_dsp_runtime_suspend, imx_dsp_runtime_resume, NULL)
 };
 
-static const struct imx_rproc_plat_ops imx_dsp_rproc_ops_mmio = {
-	.start		= imx_dsp_rproc_mmio_start,
-	.stop		= imx_dsp_rproc_mmio_stop,
-	.detect_mode	= imx_dsp_rproc_mmio_detect_mode,
-};
-
 static const struct imx_rproc_plat_ops imx_dsp_rproc_ops_reset_ctrl = {
 	.start		= imx_dsp_rproc_reset_ctrl_start,
 	.stop		= imx_dsp_rproc_reset_ctrl_stop,
@@ -1348,13 +1318,9 @@ static const struct imx_dsp_rproc_dcfg imx_dsp_rproc_cfg_imx8mp = {
 
 /* Specific configuration for i.MX8ULP */
 static const struct imx_rproc_dcfg dsp_rproc_cfg_imx8ulp = {
-	.src_reg	= IMX8ULP_SIM_LPAV_REG_SYSCTRL0,
-	.src_mask	= IMX8ULP_SYSCTRL0_DSP_STALL,
-	.src_start	= 0,
-	.src_stop	= IMX8ULP_SYSCTRL0_DSP_STALL,
 	.att		= imx_dsp_rproc_att_imx8ulp,
 	.att_size	= ARRAY_SIZE(imx_dsp_rproc_att_imx8ulp),
-	.ops		= &imx_dsp_rproc_ops_mmio,
+	.ops		= &imx_dsp_rproc_ops_reset_ctrl,
 };
 
 static const struct imx_dsp_rproc_dcfg imx_dsp_rproc_cfg_imx8ulp = {
-- 
2.34.1

[PATCH 3/4] arm64: dts: imx8ulp: add HiFi4 DSP node

[shengjiu.wang]

From: Shengjiu Wang <shengjiu.wang@nxp.com>

Add a HiFi4 DSP node to the i.MX8ULP device tree. The node is
introduced with all required registers, clocks, clock-names,
firmware reference and mailbox connections, but its status is set
to “disabled”. This prepares the SoC for HiFi4 DSP support while
leaving the node inactive until a platform explicitly enables it.

Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8ulp.dtsi | 23 ++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
index ac8293f81c6b..fd2c6b9d3c87 100644
--- a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
@@ -8,6 +8,7 @@
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/power/imx8ulp-power.h>
+#include <dt-bindings/reset/fsl,imx8ulp-sim-lpav.h>
 #include <dt-bindings/thermal/thermal.h>
 
 #include "imx8ulp-pinfunc.h"
@@ -208,6 +209,28 @@ soc: soc@0 {
 		ranges = <0x0 0x0 0x0 0x40000000>,
 			 <0x60000000 0x0 0x60000000 0x1000000>;
 
+		dsp: dsp@21170000 {
+			compatible = "fsl,imx8ulp-hifi4";
+			reg = <0x21170000 0x20000>;
+			clocks = <&cgc2 IMX8ULP_CLK_LPAV_BUS_DIV>,
+				 <&cgc2 IMX8ULP_CLK_HIFI_DIVPLAT>,
+				 <&cgc2 IMX8ULP_CLK_HIFI_DIVCORE>,
+				 <&cgc1 IMX8ULP_CLK_DUMMY>,
+				 <&pcc5 IMX8ULP_CLK_MU3_B>;
+			clock-names = "ipg", "ocram", "core", "debug", "mu";
+			firmware-name = "imx/dsp/hifi4.bin";
+			mboxes = <&mu3 0 0>,
+				 <&mu3 1 0>,
+				 <&mu3 3 0>;
+			mbox-names = "tx", "rx", "rxdb";
+			power-domains = <&scmi_devpd IMX8ULP_PD_HIFI4>;
+			resets = <&sim_lpav IMX8ULP_SIM_LPAV_HIFI4_DSP_STALL>,
+				 <&sim_lpav IMX8ULP_SIM_LPAV_HIFI4_DSP_RST>,
+				 <&sim_lpav IMX8ULP_SIM_LPAV_HIFI4_DSP_DBG_RST>;
+			reset-names = "runstall", "softreset", "dbgreset";
+			status = "disabled";
+		};
+
 		s4muap: mailbox@27020000 {
 			compatible = "fsl,imx8ulp-mu-s4";
 			reg = <0x27020000 0x10000>;
-- 
2.34.1

Re: [PATCH 3/4] arm64: dts: imx8ulp: add HiFi4 DSP node

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] arm64: dts: imx8ulp: dsp node bypasses sim_lpav clock gates
--

commit ab34f7c1f3585869d7aa05e36a532cefcf3ccd37
Author: Shengjiu Wang <shengjiu.wang@nxp.com>

arm64: dts: imx8ulp: add HiFi4 DSP node

This patch adds a disabled HiFi4 DSP node to arch/arm64/boot/dts/freescale/imx8ulp.dtsi with its register region, clocks, mailbox channels, power domain, and resets.

> diff --git a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
> index 777b89ed68490..4041f8ff5654d 100644
> --- a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
[ ... ]
> @@ -200,6 +201,28 @@ soc: soc@0 {
>  		ranges = <0x0 0x0 0x0 0x40000000>,
>  			 <0x60000000 0x0 0x60000000 0x1000000>;
>  
> +		dsp: dsp@21170000 {
> +			compatible = "fsl,imx8ulp-hifi4";
> +			reg = <0x21170000 0x20000>;
> +			clocks = <&cgc2 IMX8ULP_CLK_LPAV_BUS_DIV>,
> +				 <&cgc2 IMX8ULP_CLK_HIFI_DIVPLAT>,
> +				 <&cgc2 IMX8ULP_CLK_HIFI_DIVCORE>,
> +				 <&cgc1 IMX8ULP_CLK_DUMMY>,
> +				 <&pcc5 IMX8ULP_CLK_MU3_B>;
> +			clock-names = "ipg", "ocram", "core", "debug", "mu";

[Severity: High]
Should the ipg, ocram, and core clocks in dsp@21170000 reference the leaf
clock gates in sim_lpav rather than the parent dividers in cgc2 directly?

In arch/arm64/boot/dts/freescale/imx8ulp.dtsi, sim_lpav consumes these three
cgc2 dividers as its input clocks:

	sim_lpav: clock-controller@2da50000 {
		compatible = "fsl,imx8ulp-sim-lpav";
		reg = <0x2da50000 0x10000>;
		clocks = <&cgc2 IMX8ULP_CLK_LPAV_BUS_DIV>,
			 <&cgc2 IMX8ULP_CLK_HIFI_DIVCORE>,
			 <&cgc2 IMX8ULP_CLK_HIFI_DIVPLAT>;
		clock-names = "bus", "core", "plat";

In drivers/clk/imx/clk-imx8ulp-sim-lpav.c, clk_imx8ulp_sim_lpav_probe()
registers the downstream HiFi4 clock gates in SIM_LPAV SYSCTRL0 (bits 17, 18,
and 19) without CLK_IGNORE_UNUSED or CLK_IS_CRITICAL:

	static struct clk_imx8ulp_sim_lpav_gate gates[] = {
		IMX8ULP_HIFI_CLK_GATE("hifi_core", CORE, "core", 17),
		IMX8ULP_HIFI_CLK_GATE("hifi_pbclk", PBCLK, "bus", 18),
		IMX8ULP_HIFI_CLK_GATE("hifi_plat", PLAT, "plat", 19)
	};

Because dsp@21170000 points directly to &cgc2 instead of
<&sim_lpav IMX8ULP_CLK_SIM_LPAV_HIFI_PBCLK>,
<&sim_lpav IMX8ULP_CLK_SIM_LPAV_HIFI_PLAT>, and
<&sim_lpav IMX8ULP_CLK_SIM_LPAV_HIFI_CORE>, the sim_lpav gates have no
consumers and will be gated off by clk_disable_unused().

When the DSP is later enabled and booted via remoteproc,
imx_dsp_runtime_resume() -> clk_bulk_prepare_enable() will only enable the
upstream cgc2 dividers while leaving the SIM_LPAV SYSCTRL0 clock gates
disabled (and imx8ulp_dsp_reset() in drivers/remoteproc/imx_dsp_rproc.c only
manipulates the reset lines). Won't this leave the HiFi4 core, PBCLK, and
platform clocks gated off when imx_dsp_rproc_load() accesses DSP TCM/OCRAM at
0x21170000?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261029073257.84594-1-shengjiu.wang@oss.nxp.com?part=3

[PATCH 4/4] arm64: dts: imx8ulp-evk: enable HiFi4 DSP device

[shengjiu.wang]

From: Shengjiu Wang <shengjiu.wang@nxp.com>

Add a new `&dsp` node that references the required clocks,
clock-parents, clock-rates and the `memory-region` property pointing to
the three shared buffers (vdev0vring0, vdev0vring1, vdev0buffer) and
the reserved DSP region (`dsp_reserved`).  Enable the third MU
(`&mu3`) which provides the mailbox used by the DSP.

Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8ulp-evk.dts | 34 +++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8ulp-evk.dts b/arch/arm64/boot/dts/freescale/imx8ulp-evk.dts
index fdf3aab2224b..e253b99c2097 100644
--- a/arch/arm64/boot/dts/freescale/imx8ulp-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx8ulp-evk.dts
@@ -61,6 +61,27 @@ rsc_table: rsc-table@20000000 {
 			no-map;
 		};
 
+		dsp_reserved: dsp_reserved@8e000000 {
+			reg = <0 0x8e000000 0 0x1000000>;
+			no-map;
+		};
+
+		dsp_vdev0vring0: vdev0vring0@8fef0000 {
+			reg = <0 0x8fef0000 0 0x8000>;
+			no-map;
+		};
+
+		dsp_vdev0vring1: vdev0vring1@8fef8000 {
+			reg = <0 0x8fef8000 0 0x8000>;
+			no-map;
+		};
+
+		dsp_vdev0buffer: vdev0buffer@8ff00000 {
+			compatible = "shared-dma-pool";
+			reg = <0 0x8ff00000 0 0x100000>;
+			no-map;
+		};
+
 		vdev0vring0: vdev0vring0@aff00000 {
 			reg = <0 0xaff00000 0 0x8000>;
 			no-map;
@@ -145,6 +166,15 @@ &cm33 {
 	status = "okay";
 };
 
+&dsp {
+	assigned-clocks = <&cgc2 IMX8ULP_CLK_HIFI_SEL>, <&cgc2 IMX8ULP_CLK_HIFI_DIVCORE>;
+	assigned-clock-parents = <&cgc2 IMX8ULP_CLK_PLL4_PFD0>;
+	assigned-clock-rates = <0>, <475200000>;
+	memory-region = <&dsp_vdev0buffer>, <&dsp_vdev0vring0>,
+			<&dsp_vdev0vring1>, <&dsp_reserved>;
+	status = "okay";
+};
+
 &flexspi2 {
 	pinctrl-names = "default", "sleep";
 	pinctrl-0 = <&pinctrl_flexspi2_ptd>;
@@ -321,6 +351,10 @@ &mu {
 	status = "okay";
 };
 
+&mu3 {
+	status = "okay";
+};
+
 &iomuxc1 {
 	pinctrl_enet: enetgrp {
 		fsl,pins = <
-- 
2.34.1

Re: [PATCH 4/4] arm64: dts: imx8ulp-evk: enable HiFi4 DSP device

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] arm64: dts: imx8ulp: dsp node bypasses sim_lpav clock gates
- [Low] dt-bindings: dsp: fsl,dsp: missing required resets for imx8ulp-hifi4
- [Low] arm64: dts: imx8ulp-evk: invalid underscore in dsp_reserved node name
--

commit 66712dd8dcb982bd4722a81d3e966ce66f340e56
Author: Shengjiu Wang <shengjiu.wang@nxp.com>

arm64: dts: imx8ulp-evk: enable HiFi4 DSP device

This commit enables the HiFi4 DSP remoteproc node and its associated MU3
mailbox on imx8ulp-evk, configuring its assigned clocks and reserved memory
regions.

> diff --git a/arch/arm64/boot/dts/freescale/imx8ulp-evk.dts b/arch/arm64/boot/dts/freescale/imx8ulp-evk.dts
> index 4b925832bb662..358554a5e727e 100644
> --- a/arch/arm64/boot/dts/freescale/imx8ulp-evk.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8ulp-evk.dts
> @@ -60,6 +60,27 @@ rsc_table: rsc-table@20000000 {
>  			no-map;
>  		};
>  
> +		dsp_reserved: dsp_reserved@8e000000 {
> +			reg = <0 0x8e000000 0 0x1000000>;
> +			no-map;
> +		};

[Severity: Low]
Should the node name dsp_reserved@8e000000 avoid using an underscore?

Per Documentation/devicetree/bindings/dts-coding-style.rst and the
Devicetree specification, underscores are valid in labels like dsp_reserved,
but node names should use hyphens or generic names (such as
dsp-reserved@8e000000 or memory@8e000000) to avoid dtc W=2
node_name_chars_strict and dtbs_check warnings.

[ ... ]

> @@ -144,6 +165,15 @@ &cm33 {
>  	status = "okay";
>  };
>  
> +&dsp {
> +	assigned-clocks = <&cgc2 IMX8ULP_CLK_HIFI_SEL>, <&cgc2 IMX8ULP_CLK_HIFI_DIVCORE>;
> +	assigned-clock-parents = <&cgc2 IMX8ULP_CLK_PLL4_PFD0>;
> +	assigned-clock-rates = <0>, <475200000>;
> +	memory-region = <&dsp_vdev0buffer>, <&dsp_vdev0vring0>,
> +			<&dsp_vdev0vring1>, <&dsp_reserved>;
> +	status = "okay";
> +};

[Severity: High]
When enabling &dsp here, will the HiFi4 clock gates in sim_lpav be disabled
by clk_disable_unused() and left gated when the DSP boots?

In arch/arm64/boot/dts/freescale/imx8ulp.dtsi, the dsp node takes its ipg,
ocram, and core clocks directly from &cgc2 instead of &sim_lpav:

	dsp: dsp@21170000 {
		compatible = "fsl,imx8ulp-hifi4";
		reg = <0x21170000 0x20000>;
		clocks = <&cgc2 IMX8ULP_CLK_LPAV_BUS_DIV>,
			 <&cgc2 IMX8ULP_CLK_HIFI_DIVPLAT>,
			 <&cgc2 IMX8ULP_CLK_HIFI_DIVCORE>,
			 <&cgc1 IMX8ULP_CLK_DUMMY>,
			 <&pcc5 IMX8ULP_CLK_MU3_B>;
		clock-names = "ipg", "ocram", "core", "debug", "mu";

Meanwhile, sim_lpav in arch/arm64/boot/dts/freescale/imx8ulp.dtsi consumes
those three cgc2 clocks, and clk_imx8ulp_sim_lpav_probe() in
drivers/clk/imx/clk-imx8ulp-sim-lpav.c registers the three HiFi4 clock gates
in SYSCTRL0 without CLK_IGNORE_UNUSED or CLK_IS_CRITICAL:

	static struct clk_imx8ulp_sim_lpav_gate gates[] = {
		IMX8ULP_HIFI_CLK_GATE("hifi_core", CORE, "core", 17),
		IMX8ULP_HIFI_CLK_GATE("hifi_pbclk", PBCLK, "bus", 18),
		IMX8ULP_HIFI_CLK_GATE("hifi_plat", PLAT, "plat", 19)
	};

Because nothing references IMX8ULP_CLK_SIM_LPAV_HIFI_PBCLK,
IMX8ULP_CLK_SIM_LPAV_HIFI_PLAT, or IMX8ULP_CLK_SIM_LPAV_HIFI_CORE,
clk_disable_unused() clears bits 17, 18, and 19 in SYSCTRL0 at late_initcall.
When rproc_boot() -> imx_dsp_rproc_prepare() -> imx_dsp_runtime_resume() ->
clk_bulk_prepare_enable() later runs, only the upstream cgc2 dividers are
enabled while the sim_lpav gates remain closed during imx_dsp_rproc_load()
and imx_dsp_rproc_start().

Should the dsp node reference the three &sim_lpav clock gates for its ipg,
ocram, and core clocks instead of &cgc2 directly?

[Severity: Low]
Also related to enabling fsl,imx8ulp-hifi4, should
Documentation/devicetree/bindings/dsp/fsl,dsp.yaml be updated to require
resets and reset-names for fsl,imx8ulp-hifi4?

In drivers/remoteproc/imx_dsp_rproc.c, dsp_rproc_cfg_imx8ulp uses
imx_dsp_rproc_ops_reset_ctrl:

	static const struct imx_rproc_dcfg dsp_rproc_cfg_imx8ulp = {
		.att		= imx_dsp_rproc_att_imx8ulp,
		.att_size	= ARRAY_SIZE(imx_dsp_rproc_att_imx8ulp),
		.ops		= &imx_dsp_rproc_ops_reset_ctrl,
	};

During imx_dsp_rproc_probe(), imx_dsp_rproc_reset_ctrl_detect_mode() requires
the runstall reset control (and imx8ulp_dsp_reset() uses runstall, softreset,
and dbgreset), but the allOf conditional block in
Documentation/devicetree/bindings/dsp/fsl,dsp.yaml requiring resets and
reset-names currently only lists fsl,imx8mp-dsp and fsl,imx8mp-hifi4:

	  - if:
	      properties:
	        compatible:
	          contains:
	            enum:
	              - fsl,imx8mp-dsp
	              - fsl,imx8mp-hifi4
	    then:
	      required:
	        - resets
	        - reset-names

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261029073257.84594-1-shengjiu.wang@oss.nxp.com?part=4