Re: [PATCH v4 4/5] remoteproc: imx_rproc: Add support for System Manager API

[Peng Fan <peng.fan@oss.nxp.com>]

Hi Mathieu,
On Wed, Dec 17, 2025 at 04:19:04PM -0700, Mathieu Poirier wrote:
>On Tue, Dec 16, 2025 at 09:51:02AM +0800, Peng Fan (OSS) wrote:
>
>Why are these not at the bottom of the file with the rest of the
>imx_rproc_plat_ops?  That way you wouldn't have to declare
>imx_rproc_sm_detect_mode() at the top of the file...
>
>> +
>> +static int imx_rproc_sm_lmm_check(struct rproc *rproc, bool started)
>> +	if (ret) {
>> +		priv->flags &= ~IMX_RPROC_FLAGS_SM_LMM_AVAIL;
>> +		dev_err(dev, "shutdown lmm(%d) failed: %d\n", dcfg->lmid, ret);
>> +		return ret;
>> +	}
>
>If you move this call to scmi_imx_lmm_operation() (and the error check) before
>setting priv->flags, you won't have to reset it in the error path.
>
>Let's see how the next revision goes...

Thanks for the review.

V5 has been sent out just now with both comments addressed:
- Moved the SM LMM functions to the bottom of the file with the
  rest of imx_rproc_plat_ops
- Moved the scmi_imx_lmm_operation() call before setting priv->flags
  to simplify error handling

Thanks,
Peng

>
>Thanks,
>Mathieu
>
>> +
>> +	return 0;
>> +}
>> +
>> +static int imx_rproc_sm_detect_mode(struct rproc *rproc)
>> +{
>> +	struct imx_rproc *priv = rproc->priv;
>> +	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
>> +	struct device *dev = priv->dev;
>> +	struct scmi_imx_lmm_info info;
>> +	bool started = false;
>> +	int ret;
>> +
>> +	ret = scmi_imx_cpu_started(dcfg->cpuid, &started);
>> +	if (ret) {
>> +		dev_err(dev, "Failed to detect cpu(%d) status: %d\n", dcfg->cpuid, ret);
>> +		return ret;
>> +	}
>> +
>> +	if (started)
>> +		priv->rproc->state = RPROC_DETACHED;
>> +
>> +	/* Get current Linux Logical Machine ID */
>> +	ret = scmi_imx_lmm_info(LMM_ID_DISCOVER, &info);
>> +	if (ret) {
>> +		dev_err(dev, "Failed to get current LMM ID err: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	/*
>> +	 * Check whether rproc is in the same LM as host core(running Linux)
>> +	 * If yes, use CPU protocol API to manage rproc.
>> +	 * If no, use Logical Machine API to manage rproc.
>> +	 */
>> +	if (dcfg->lmid == info.lmid) {
>> +		priv->ops = &imx_rproc_ops_sm_cpu;
>> +		dev_info(dev, "Using CPU Protocol OPS\n");
>> +		return 0;
>> +	}
>> +
>> +	priv->ops = &imx_rproc_ops_sm_lmm;
>> +	dev_info(dev, "Using LMM Protocol OPS\n");
>> +
>> +	return imx_rproc_sm_lmm_check(rproc, started);
>> +}
>> +
>>  static int imx_rproc_detect_mode(struct imx_rproc *priv)
>>  {
>>  	/*
>> diff --git a/drivers/remoteproc/imx_rproc.h b/drivers/remoteproc/imx_rproc.h
>> index 37417568a0ade2ae4d6a4e3d0f139ea52b185254..d37e6f90548cec727b4aeb874680b42af85bdbb4 100644
>> --- a/drivers/remoteproc/imx_rproc.h
>> +++ b/drivers/remoteproc/imx_rproc.h
>> @@ -38,6 +38,9 @@ struct imx_rproc_dcfg {
>>  	size_t				att_size;
>>  	u32				flags;
>>  	const struct imx_rproc_plat_ops	*ops;
>> +	/* For System Manager(SM) based SoCs */
>> +	u32				cpuid; /* ID of the remote core */
>> +	u32				lmid;  /* ID of the Logcial Machine */
>>  };
>>  
>>  #endif /* _IMX_RPROC_H */
>> 
>> -- 
>> 2.37.1
>>