linux-kernel - RE: [PATCH v2 4/5] remoteproc: imx_rproc: Add support for System Manager API

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID:
 <PAXPR04MB84595E1EC0AD10E892DE2D1C88C5A@PAXPR04MB8459.eurprd04.prod.outlook.com>
Date: Wed, 5 Nov 2025 08:51:27 +0000
From: Peng Fan <peng.fan@....com>
To: Iuliana Prodan <iuliana.prodan@....com>, Bjorn Andersson
	<andersson@...nel.org>, Mathieu Poirier <mathieu.poirier@...aro.org>, Rob
 Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor
 Dooley <conor+dt@...nel.org>, Shawn Guo <shawnguo@...nel.org>, Sascha Hauer
	<s.hauer@...gutronix.de>, Pengutronix Kernel Team <kernel@...gutronix.de>,
	Fabio Estevam <festevam@...il.com>, Daniel Baluta <daniel.baluta@....com>,
	Frank Li <frank.li@....com>
CC: "linux-remoteproc@...r.kernel.org" <linux-remoteproc@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"imx@...ts.linux.dev" <imx@...ts.linux.dev>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v2 4/5] remoteproc: imx_rproc: Add support for System
 Manager API

Hi Iuliana,

> Subject: Re: [PATCH v2 4/5] remoteproc: imx_rproc: Add support for
> System Manager API
> 
> >   	depends on HAVE_ARM_SMCCC
> > +	depends on IMX_SCMI_CPU_DRV || !IMX_SCMI_CPU_DRV
> > +	depends on IMX_SCMI_LMM_DRV || !IMX_SCMI_LMM_DRV
> 
> This always evaluates to true.
> If you want to ensure that when a dependency is m, imx_rproc must
> also be a m, you use:
> depends on IMX_SCMI_CPU_DRV
> depends on IMX_SCMI_LMM_DRV

No, this does not work. We need support non-SCMI platform.

I followed what Arnd did, see
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index ad20dc8506f9..6b497dd87562 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -2297,6 +2297,7 @@ config SENSORS_TMP103
config SENSORS_TMP108
        tristate "Texas Instruments TMP108"
        depends on I2C
+       depends on I3C || !I3C


>
...
 
> > +	/* For i.MX System Manager based systems */
> > +	u32				flags;
> 
> Add some info about how and why this new field is used/needed.

ok. Will add that in above comment area.

> 
> >   };
> >
> >   static const struct imx_rproc_att imx_rproc_att_imx93[] = { @@
> > -313,6 +321,44 @@ static int imx_rproc_scu_api_start(struct rproc
> *rproc)
> >   	return imx_sc_pm_cpu_start(priv->ipc_handle, priv->rsrc_id,
> true, priv->entry);
> >   }
> >
> > +static int imx_rproc_sm_cpu_start(struct rproc *rproc) {
> > +	struct imx_rproc *priv = rproc->priv;
> > +	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
> > +	int ret;
> > +
> > +	ret = scmi_imx_cpu_reset_vector_set(dcfg->cpuid, 0, true, false,
> false);
> > +	if (ret) {
> > +		dev_err(priv->dev, "Failed to set reset vector
> cpuid(%u): %d\n", dcfg->cpuid, ret);
> > +		return ret;
> > +	}
> > +
> > +	return scmi_imx_cpu_start(dcfg->cpuid, true); }
> > +
> > +static int imx_rproc_sm_lmm_start(struct rproc *rproc) {
> > +	struct imx_rproc *priv = rproc->priv;
> > +	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
> > +	struct device *dev = priv->dev;
> > +	int ret;
> > +
> > +	ret = scmi_imx_lmm_reset_vector_set(dcfg->lmid, dcfg->cpuid,
> 0, 0);
> > +	if (ret) {
> > +		dev_err(dev, "Failed to set reset vector lmid(%u),
> cpuid(%u): %d\n",
> > +			dcfg->lmid, dcfg->cpuid, ret);
> > +		return ret;
> > +	}
> > +
> > +	ret = scmi_imx_lmm_operation(dcfg->lmid,
> SCMI_IMX_LMM_BOOT, 0);
> > +	if (ret) {
> > +		dev_err(dev, "Failed to boot lmm(%d): %d\n", dcfg-
> >lmid, ret);
> > +		return ret;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >   static int imx_rproc_start(struct rproc *rproc)
> >   {
> >   	struct imx_rproc *priv = rproc->priv; @@ -369,6 +415,25 @@
> static
> > int imx_rproc_scu_api_stop(struct rproc *rproc)
> >   	return imx_sc_pm_cpu_start(priv->ipc_handle, priv->rsrc_id,
> false, priv->entry);
> >   }
> >
> > +static int imx_rproc_sm_cpu_stop(struct rproc *rproc) {
> > +	struct imx_rproc *priv = rproc->priv;
> > +	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
> > +
> > +	return scmi_imx_cpu_start(dcfg->cpuid, false); }
> > +
> > +static int imx_rproc_sm_lmm_stop(struct rproc *rproc) {
> > +	struct imx_rproc *priv = rproc->priv;
> > +	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
> > +
> > +	if (!(priv->flags & IMX_RPROC_FLAGS_SM_LMM_AVAIL))
> > +		return -EACCES;
> > +
> > +	return scmi_imx_lmm_operation(dcfg->lmid,
> SCMI_IMX_LMM_SHUTDOWN, 0);
> > +}
> > +
> >   static int imx_rproc_stop(struct rproc *rproc)
> >   {
> >   	struct imx_rproc *priv = rproc->priv; @@ -485,6 +550,37 @@
> static
> > int imx_rproc_mem_release(struct rproc *rproc,
> >   	return 0;
> >   }
> >
> > +static int imx_rproc_sm_lmm_prepare(struct rproc *rproc) {
> > +	struct imx_rproc *priv = rproc->priv;
> > +	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
> > +	int ret;
> > +
> > +	/*
> > +	 * IMX_RPROC_FLAGS_SM_LMM_AVAIL not set indicates Linux
> is not able
> > +	 * to start/stop rproc LM, then if rproc is not in detached state,
> > +	 * prepare should fail. If in detached state, this is in
> rproc_attach()
> > +	 * path.
> > +	 */
> > +	if (!(priv->flags & IMX_RPROC_FLAGS_SM_LMM_AVAIL)) {
> > +		if (rproc->state != RPROC_DETACHED)
> > +			return -EACCES;
> > +
> > +		return 0;
> > +	}
> > +
> 
> IMO is simpler this way:
> if (!(priv->flags & IMX_RPROC_FLAGS_SM_LMM_AVAIL))
> 	return (rproc->state == RPROC_DETACHED) ? 0 : -EACCES;

Yeah. Thanks.

Thanks,
Peng