Re: [PATCH 08/11] remoteproc: imx_dsp_rproc: Simplify IMX_RPROC_MMIO switch case

[Frank Li <Frank.li@nxp.com>]

On Fri, Oct 31, 2025 at 05:08:37PM +0800, Peng Fan wrote:
> Introduce imx_dsp_rproc_mmio_{start, stop, detect_mode}() helper functions
> for i.MX variants using IMX_RPROC_MMIO to manage remote processors.
>
> Allows the removal of the IMX_RPROC_MMIO switch-case blocks from
> imx_dsp_rproc_[start,stop,detect_mode](), resulting in cleaner and more
> maintainable code.
>
> No functional changes.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/remoteproc/imx_dsp_rproc.c | 63 +++++++++++++++++++++++++-------------
>  drivers/remoteproc/imx_rproc.h     |  2 --
>  2 files changed, 42 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/remoteproc/imx_dsp_rproc.c b/drivers/remoteproc/imx_dsp_rproc.c
> index f28d25cab3f1d89e5cde37a04b528870a59abeed..456a46f163d3d823a25d16d11fb79fa9fceb2ddb 100644
> --- a/drivers/remoteproc/imx_dsp_rproc.c
> +++ b/drivers/remoteproc/imx_dsp_rproc.c
> @@ -338,6 +338,15 @@ 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_dsp_rproc_dcfg *dsp_dcfg = priv->dsp_dcfg;
> +	const struct imx_rproc_dcfg *dcfg = dsp_dcfg->dcfg;
> +
> +	return regmap_update_bits(priv->regmap, dcfg->src_reg, dcfg->src_mask, dcfg->src_start);
> +}
> +
>  /*
>   * Start function for rproc_ops
>   *
> @@ -360,12 +369,6 @@ static int imx_dsp_rproc_start(struct rproc *rproc)
>  	}
>
>  	switch (dcfg->method) {
> -	case IMX_RPROC_MMIO:
> -		ret = regmap_update_bits(priv->regmap,
> -					 dcfg->src_reg,
> -					 dcfg->src_mask,
> -					 dcfg->src_start);
> -		break;
>  	case IMX_RPROC_SCU_API:
>  		ret = imx_sc_pm_cpu_start(priv->ipc_handle,
>  					  IMX_SC_R_DSP,
> @@ -388,6 +391,15 @@ static int imx_dsp_rproc_start(struct rproc *rproc)
>  	return ret;
>  }
>
> +static int imx_dsp_rproc_mmio_stop(struct rproc *rproc)
> +{
> +	struct imx_dsp_rproc *priv = rproc->priv;
> +	const struct imx_dsp_rproc_dcfg *dsp_dcfg = priv->dsp_dcfg;
> +	const struct imx_rproc_dcfg *dcfg = dsp_dcfg->dcfg;

can you add helper macro convert rproc to dcfg to avoid duplicate above
3 line codes.

Frank
> +
> +	return regmap_update_bits(priv->regmap, dcfg->src_reg, dcfg->src_mask, dcfg->src_stop);
> +}
> +
>  /*
>   * Stop function for rproc_ops
>   * It clears the REMOTE_IS_READY flags
> @@ -411,10 +423,6 @@ static int imx_dsp_rproc_stop(struct rproc *rproc)
>  	}
>
>  	switch (dcfg->method) {
> -	case IMX_RPROC_MMIO:
> -		ret = regmap_update_bits(priv->regmap, dcfg->src_reg, dcfg->src_mask,
> -					 dcfg->src_stop);
> -		break;
>  	case IMX_RPROC_SCU_API:
>  		ret = imx_sc_pm_cpu_start(priv->ipc_handle,
>  					  IMX_SC_R_DSP,
> @@ -1032,6 +1040,23 @@ 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;
> +}
> +
>  /**
>   * imx_dsp_rproc_detect_mode() - detect DSP control mode
>   * @priv: private data pointer
> @@ -1049,7 +1074,6 @@ static int imx_dsp_rproc_detect_mode(struct imx_dsp_rproc *priv)
>  	const struct imx_dsp_rproc_dcfg *dsp_dcfg = priv->dsp_dcfg;
>  	const struct imx_rproc_dcfg *dcfg = dsp_dcfg->dcfg;
>  	struct device *dev = priv->rproc->dev.parent;
> -	struct regmap *regmap;
>  	int ret = 0;
>
>  	if (dcfg->ops && dcfg->ops->detect_mode)
> @@ -1061,15 +1085,6 @@ static int imx_dsp_rproc_detect_mode(struct imx_dsp_rproc *priv)
>  		if (ret)
>  			return ret;
>  		break;
> -	case IMX_RPROC_MMIO:
> -		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;
> -		break;
>  	case IMX_RPROC_RESET_CONTROLLER:
>  		priv->run_stall = devm_reset_control_get_exclusive(dev, "runstall");
>  		if (IS_ERR(priv->run_stall)) {
> @@ -1301,6 +1316,12 @@ 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,
> +};
> +
>  /* Specific configuration for i.MX8MP */
>  static const struct imx_rproc_dcfg dsp_rproc_cfg_imx8mp = {
>  	.att		= imx_dsp_rproc_att_imx8mp,
> @@ -1321,7 +1342,7 @@ static const struct imx_rproc_dcfg dsp_rproc_cfg_imx8ulp = {
>  	.src_stop	= IMX8ULP_SYSCTRL0_DSP_STALL,
>  	.att		= imx_dsp_rproc_att_imx8ulp,
>  	.att_size	= ARRAY_SIZE(imx_dsp_rproc_att_imx8ulp),
> -	.method		= IMX_RPROC_MMIO,
> +	.ops		= &imx_dsp_rproc_ops_mmio,
>  };
>
>  static const struct imx_dsp_rproc_dcfg imx_dsp_rproc_cfg_imx8ulp = {
> diff --git a/drivers/remoteproc/imx_rproc.h b/drivers/remoteproc/imx_rproc.h
> index aeed08bdfb5619c7afd7201589f417cfd6745818..912827c39c0dedeed76c13740efd42a8e7cf9c45 100644
> --- a/drivers/remoteproc/imx_rproc.h
> +++ b/drivers/remoteproc/imx_rproc.h
> @@ -18,8 +18,6 @@ struct imx_rproc_att {
>  /* Remote core start/stop method */
>  enum imx_rproc_method {
>  	IMX_RPROC_NONE,
> -	/* Through syscon regmap */
> -	IMX_RPROC_MMIO,
>  	/* Through ARM SMCCC */
>  	IMX_RPROC_SMC,
>  	/* Through System Control Unit API */
>
> --
> 2.37.1
>