[PATCH 0/6] remoteproc: imx_proc: Simplify driver by removing the switch-case

[PATCH 0/6] remoteproc: imx_proc: Simplify driver by removing the switch-case

[Peng Fan]

This patchset serves as a preparing patchset for i.MX95 support.

The current code logic is complicated, with mix the usage of switch-case
and if-else.

To simplify the code logic:
 Introduce struct imx_rproc_plat_ops to wrap platform start,stop,detect_mode.
 Each imx_rproc_dcfg data structure is assigned a ops pointer.
 The common imx_rproc_{start,stop}() directly invokes the plat ops, no
 need the switch-case.
 mmio/smc/scu_api ops are included.
 No functional changes.

Thanks to Daniel and Frank for the help.

Test on i.MX8MM for MMIO ops, i.MX8MP for SMC ops. For SCU-API ops,
I currently not have a setup at hand, I will give a setup and test
later, but everything should be fine, no functional changes.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
Peng Fan (6):
      remoteproc: imx_rproc: Introduce start/stop/detect_mode ops for imx_rproc_dcfg
      remoteproc: imx_rproc: Move imx_rproc_dcfg closer to imx_rproc_of_match
      remoteproc: imx_rproc: Simplify IMX_RPROC_MMIO switch case
      remoteproc: imx_rproc: Simplify IMX_RPROC_SCU_API switch case
      remoteproc: imx_rproc: Simplify IMX_RPROC_SMC switch case
      remoteproc: imx_rproc: Clean up after ops introduction

 drivers/remoteproc/imx_rproc.c | 448 +++++++++++++++++++++++------------------
 drivers/remoteproc/imx_rproc.h |   7 +
 2 files changed, 264 insertions(+), 191 deletions(-)
---
base-commit: 3e8e5822146bc396d2a7e5fbb7be13271665522a
change-id: 20250908-imx-rproc-cleanup-6f3b546b9fdf

Best regards,
-- 
Peng Fan <peng.fan@nxp.com>

[PATCH 1/6] remoteproc: imx_rproc: Introduce start/stop/detect_mode ops for imx_rproc_dcfg

[Peng Fan]

Simplify the logic in imx_rproc_start(), imx_rproc_stop() and
imx_rproc_detect_mode(), introduce start, stop and detect_mode ops for the
imx_rproc_dcfg structure. Allows each platform to provide its own
implementation of start/stop/detect_mode operations, and prepares to
eliminate the need for multiple switch-case statements.

Improves code readability and maintainability by encapsulating
platform-specific behavior.

No functional changes.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/remoteproc/imx_rproc.c | 15 +++++++++++++++
 drivers/remoteproc/imx_rproc.h |  7 +++++++
 2 files changed, 22 insertions(+)

diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
index a6eef0080ca9e46efe60dcb3878b9efdbdc0f08e..5cdc5045e57566e817170ed3c708dad6108d2e46 100644
--- a/drivers/remoteproc/imx_rproc.c
+++ b/drivers/remoteproc/imx_rproc.c
@@ -376,6 +376,11 @@ static int imx_rproc_start(struct rproc *rproc)
 	if (ret)
 		return ret;
 
+	if (dcfg->ops && dcfg->ops->start) {
+		ret = dcfg->ops->start(rproc);
+		goto start_ret;
+	}
+
 	switch (dcfg->method) {
 	case IMX_RPROC_MMIO:
 		if (priv->gpr) {
@@ -398,6 +403,7 @@ static int imx_rproc_start(struct rproc *rproc)
 		return -EOPNOTSUPP;
 	}
 
+start_ret:
 	if (ret)
 		dev_err(dev, "Failed to enable remote core!\n");
 
@@ -412,6 +418,11 @@ static int imx_rproc_stop(struct rproc *rproc)
 	struct arm_smccc_res res;
 	int ret;
 
+	if (dcfg->ops && dcfg->ops->stop) {
+		ret = dcfg->ops->stop(rproc);
+		goto stop_ret;
+	}
+
 	switch (dcfg->method) {
 	case IMX_RPROC_MMIO:
 		if (priv->gpr) {
@@ -440,6 +451,7 @@ static int imx_rproc_stop(struct rproc *rproc)
 		return -EOPNOTSUPP;
 	}
 
+stop_ret:
 	if (ret)
 		dev_err(dev, "Failed to stop remote core\n");
 	else
@@ -933,6 +945,9 @@ static int imx_rproc_detect_mode(struct imx_rproc *priv)
 	u32 val;
 	u8 pt;
 
+	if (dcfg->ops && dcfg->ops->detect_mode)
+		return dcfg->ops->detect_mode(priv->rproc);
+
 	switch (dcfg->method) {
 	case IMX_RPROC_NONE:
 		priv->rproc->state = RPROC_DETACHED;
diff --git a/drivers/remoteproc/imx_rproc.h b/drivers/remoteproc/imx_rproc.h
index cfd38d37e1467d1d9e6f89be146c0b53262b92a0..3a9adaaf048b396102feeb45488cd2ff125a807a 100644
--- a/drivers/remoteproc/imx_rproc.h
+++ b/drivers/remoteproc/imx_rproc.h
@@ -31,6 +31,12 @@ enum imx_rproc_method {
 /* dcfg flags */
 #define IMX_RPROC_NEED_SYSTEM_OFF	BIT(0)
 
+struct imx_rproc_plat_ops {
+	int (*start)(struct rproc *rproc);
+	int (*stop)(struct rproc *rproc);
+	int (*detect_mode)(struct rproc *rproc);
+};
+
 struct imx_rproc_dcfg {
 	u32				src_reg;
 	u32				src_mask;
@@ -42,6 +48,7 @@ struct imx_rproc_dcfg {
 	size_t				att_size;
 	enum imx_rproc_method		method;
 	u32				flags;
+	const struct imx_rproc_plat_ops	*ops;
 };
 
 #endif /* _IMX_RPROC_H */

-- 
2.37.1

[PATCH 2/6] remoteproc: imx_rproc: Move imx_rproc_dcfg closer to imx_rproc_of_match

[Peng Fan]

Move the imx_rproc_dcfg structure definitions closer to imx_rproc_of_match
to prepare for adding start/stop/detect_mode ops for each i.MX variant.

This relocation avoids the need to declare function prototypes such as
'static int imx_rproc_xtr_mbox_init(struct rproc *rproc, bool tx_block)'
at the beginning of the file, improving code organization and readability.

No functional changes.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/remoteproc/imx_rproc.c | 158 ++++++++++++++++++++---------------------
 1 file changed, 79 insertions(+), 79 deletions(-)

diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
index 5cdc5045e57566e817170ed3c708dad6108d2e46..af7b69d750deed734668cb413b29378e5ca7c64e 100644
--- a/drivers/remoteproc/imx_rproc.c
+++ b/drivers/remoteproc/imx_rproc.c
@@ -285,85 +285,6 @@ static const struct imx_rproc_att imx_rproc_att_imx6sx[] = {
 	{ 0x80000000, 0x80000000, 0x60000000, 0 },
 };
 
-static const struct imx_rproc_dcfg imx_rproc_cfg_imx8mn_mmio = {
-	.src_reg	= IMX7D_SRC_SCR,
-	.src_mask	= IMX7D_M4_RST_MASK,
-	.src_start	= IMX7D_M4_START,
-	.src_stop	= IMX8M_M7_STOP,
-	.gpr_reg	= IMX8M_GPR22,
-	.gpr_wait	= IMX8M_GPR22_CM7_CPUWAIT,
-	.att		= imx_rproc_att_imx8mn,
-	.att_size	= ARRAY_SIZE(imx_rproc_att_imx8mn),
-	.method		= IMX_RPROC_MMIO,
-};
-
-static const struct imx_rproc_dcfg imx_rproc_cfg_imx8mn = {
-	.att		= imx_rproc_att_imx8mn,
-	.att_size	= ARRAY_SIZE(imx_rproc_att_imx8mn),
-	.method		= IMX_RPROC_SMC,
-};
-
-static const struct imx_rproc_dcfg imx_rproc_cfg_imx8mq = {
-	.src_reg	= IMX7D_SRC_SCR,
-	.src_mask	= IMX7D_M4_RST_MASK,
-	.src_start	= IMX7D_M4_START,
-	.src_stop	= IMX7D_M4_STOP,
-	.att		= imx_rproc_att_imx8mq,
-	.att_size	= ARRAY_SIZE(imx_rproc_att_imx8mq),
-	.method		= IMX_RPROC_MMIO,
-};
-
-static const struct imx_rproc_dcfg imx_rproc_cfg_imx8qm = {
-	.att            = imx_rproc_att_imx8qm,
-	.att_size       = ARRAY_SIZE(imx_rproc_att_imx8qm),
-	.method         = IMX_RPROC_SCU_API,
-};
-
-static const struct imx_rproc_dcfg imx_rproc_cfg_imx8qxp = {
-	.att		= imx_rproc_att_imx8qxp,
-	.att_size	= ARRAY_SIZE(imx_rproc_att_imx8qxp),
-	.method		= IMX_RPROC_SCU_API,
-};
-
-static const struct imx_rproc_dcfg imx_rproc_cfg_imx8ulp = {
-	.att		= imx_rproc_att_imx8ulp,
-	.att_size	= ARRAY_SIZE(imx_rproc_att_imx8ulp),
-	.method		= IMX_RPROC_NONE,
-};
-
-static const struct imx_rproc_dcfg imx_rproc_cfg_imx7ulp = {
-	.att		= imx_rproc_att_imx7ulp,
-	.att_size	= ARRAY_SIZE(imx_rproc_att_imx7ulp),
-	.method		= IMX_RPROC_NONE,
-	.flags		= IMX_RPROC_NEED_SYSTEM_OFF,
-};
-
-static const struct imx_rproc_dcfg imx_rproc_cfg_imx7d = {
-	.src_reg	= IMX7D_SRC_SCR,
-	.src_mask	= IMX7D_M4_RST_MASK,
-	.src_start	= IMX7D_M4_START,
-	.src_stop	= IMX7D_M4_STOP,
-	.att		= imx_rproc_att_imx7d,
-	.att_size	= ARRAY_SIZE(imx_rproc_att_imx7d),
-	.method		= IMX_RPROC_MMIO,
-};
-
-static const struct imx_rproc_dcfg imx_rproc_cfg_imx6sx = {
-	.src_reg	= IMX6SX_SRC_SCR,
-	.src_mask	= IMX6SX_M4_RST_MASK,
-	.src_start	= IMX6SX_M4_START,
-	.src_stop	= IMX6SX_M4_STOP,
-	.att		= imx_rproc_att_imx6sx,
-	.att_size	= ARRAY_SIZE(imx_rproc_att_imx6sx),
-	.method		= IMX_RPROC_MMIO,
-};
-
-static const struct imx_rproc_dcfg imx_rproc_cfg_imx93 = {
-	.att		= imx_rproc_att_imx93,
-	.att_size	= ARRAY_SIZE(imx_rproc_att_imx93),
-	.method		= IMX_RPROC_SMC,
-};
-
 static int imx_rproc_start(struct rproc *rproc)
 {
 	struct imx_rproc *priv = rproc->priv;
@@ -1222,6 +1143,85 @@ static void imx_rproc_remove(struct platform_device *pdev)
 	destroy_workqueue(priv->workqueue);
 }
 
+static const struct imx_rproc_dcfg imx_rproc_cfg_imx8mn_mmio = {
+	.src_reg	= IMX7D_SRC_SCR,
+	.src_mask	= IMX7D_M4_RST_MASK,
+	.src_start	= IMX7D_M4_START,
+	.src_stop	= IMX8M_M7_STOP,
+	.gpr_reg	= IMX8M_GPR22,
+	.gpr_wait	= IMX8M_GPR22_CM7_CPUWAIT,
+	.att		= imx_rproc_att_imx8mn,
+	.att_size	= ARRAY_SIZE(imx_rproc_att_imx8mn),
+	.method		= IMX_RPROC_MMIO,
+};
+
+static const struct imx_rproc_dcfg imx_rproc_cfg_imx8mn = {
+	.att		= imx_rproc_att_imx8mn,
+	.att_size	= ARRAY_SIZE(imx_rproc_att_imx8mn),
+	.method		= IMX_RPROC_SMC,
+};
+
+static const struct imx_rproc_dcfg imx_rproc_cfg_imx8mq = {
+	.src_reg	= IMX7D_SRC_SCR,
+	.src_mask	= IMX7D_M4_RST_MASK,
+	.src_start	= IMX7D_M4_START,
+	.src_stop	= IMX7D_M4_STOP,
+	.att		= imx_rproc_att_imx8mq,
+	.att_size	= ARRAY_SIZE(imx_rproc_att_imx8mq),
+	.method		= IMX_RPROC_MMIO,
+};
+
+static const struct imx_rproc_dcfg imx_rproc_cfg_imx8qm = {
+	.att            = imx_rproc_att_imx8qm,
+	.att_size       = ARRAY_SIZE(imx_rproc_att_imx8qm),
+	.method         = IMX_RPROC_SCU_API,
+};
+
+static const struct imx_rproc_dcfg imx_rproc_cfg_imx8qxp = {
+	.att		= imx_rproc_att_imx8qxp,
+	.att_size	= ARRAY_SIZE(imx_rproc_att_imx8qxp),
+	.method		= IMX_RPROC_SCU_API,
+};
+
+static const struct imx_rproc_dcfg imx_rproc_cfg_imx8ulp = {
+	.att		= imx_rproc_att_imx8ulp,
+	.att_size	= ARRAY_SIZE(imx_rproc_att_imx8ulp),
+	.method		= IMX_RPROC_NONE,
+};
+
+static const struct imx_rproc_dcfg imx_rproc_cfg_imx7ulp = {
+	.att		= imx_rproc_att_imx7ulp,
+	.att_size	= ARRAY_SIZE(imx_rproc_att_imx7ulp),
+	.method		= IMX_RPROC_NONE,
+	.flags		= IMX_RPROC_NEED_SYSTEM_OFF,
+};
+
+static const struct imx_rproc_dcfg imx_rproc_cfg_imx7d = {
+	.src_reg	= IMX7D_SRC_SCR,
+	.src_mask	= IMX7D_M4_RST_MASK,
+	.src_start	= IMX7D_M4_START,
+	.src_stop	= IMX7D_M4_STOP,
+	.att		= imx_rproc_att_imx7d,
+	.att_size	= ARRAY_SIZE(imx_rproc_att_imx7d),
+	.method		= IMX_RPROC_MMIO,
+};
+
+static const struct imx_rproc_dcfg imx_rproc_cfg_imx6sx = {
+	.src_reg	= IMX6SX_SRC_SCR,
+	.src_mask	= IMX6SX_M4_RST_MASK,
+	.src_start	= IMX6SX_M4_START,
+	.src_stop	= IMX6SX_M4_STOP,
+	.att		= imx_rproc_att_imx6sx,
+	.att_size	= ARRAY_SIZE(imx_rproc_att_imx6sx),
+	.method		= IMX_RPROC_MMIO,
+};
+
+static const struct imx_rproc_dcfg imx_rproc_cfg_imx93 = {
+	.att		= imx_rproc_att_imx93,
+	.att_size	= ARRAY_SIZE(imx_rproc_att_imx93),
+	.method		= IMX_RPROC_SMC,
+};
+
 static const struct of_device_id imx_rproc_of_match[] = {
 	{ .compatible = "fsl,imx7ulp-cm4", .data = &imx_rproc_cfg_imx7ulp },
 	{ .compatible = "fsl,imx7d-cm4", .data = &imx_rproc_cfg_imx7d },

-- 
2.37.1

[PATCH 3/6] remoteproc: imx_rproc: Simplify IMX_RPROC_MMIO switch case

[Peng Fan]

Introduce imx_rproc_mmio_{start, stop, detect_mode}() helper functions for
all i.MX variants using IMX_RPROC_MMIO to manage remote processors.

This allows the removal of the IMX_RPROC_MMIO switch-case blocks from
imx_rproc_start(), imx_rproc_stop(), and imx_rproc_detect_mode(), resulting
in cleaner and more maintainable code.

No functional changes.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/remoteproc/imx_rproc.c | 148 ++++++++++++++++++++++++-----------------
 1 file changed, 86 insertions(+), 62 deletions(-)

diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
index af7b69d750deed734668cb413b29378e5ca7c64e..c37dd67595960f08fd85c0b516d0d03855cec9fc 100644
--- a/drivers/remoteproc/imx_rproc.c
+++ b/drivers/remoteproc/imx_rproc.c
@@ -285,6 +285,17 @@ static const struct imx_rproc_att imx_rproc_att_imx6sx[] = {
 	{ 0x80000000, 0x80000000, 0x60000000, 0 },
 };
 
+static int imx_rproc_mmio_start(struct rproc *rproc)
+{
+	struct imx_rproc *priv = rproc->priv;
+	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
+
+	if (priv->gpr)
+		return regmap_clear_bits(priv->gpr, dcfg->gpr_reg, dcfg->gpr_wait);
+
+	return regmap_update_bits(priv->regmap, dcfg->src_reg, dcfg->src_mask, dcfg->src_start);
+}
+
 static int imx_rproc_start(struct rproc *rproc)
 {
 	struct imx_rproc *priv = rproc->priv;
@@ -303,16 +314,6 @@ static int imx_rproc_start(struct rproc *rproc)
 	}
 
 	switch (dcfg->method) {
-	case IMX_RPROC_MMIO:
-		if (priv->gpr) {
-			ret = regmap_clear_bits(priv->gpr, dcfg->gpr_reg,
-						dcfg->gpr_wait);
-		} else {
-			ret = regmap_update_bits(priv->regmap, dcfg->src_reg,
-						 dcfg->src_mask,
-						 dcfg->src_start);
-		}
-		break;
 	case IMX_RPROC_SMC:
 		arm_smccc_smc(IMX_SIP_RPROC, IMX_SIP_RPROC_START, 0, 0, 0, 0, 0, 0, &res);
 		ret = res.a0;
@@ -331,6 +332,23 @@ static int imx_rproc_start(struct rproc *rproc)
 	return ret;
 }
 
+static int imx_rproc_mmio_stop(struct rproc *rproc)
+{
+	struct imx_rproc *priv = rproc->priv;
+	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
+	int ret;
+
+	if (priv->gpr) {
+		ret = regmap_set_bits(priv->gpr, dcfg->gpr_reg, dcfg->gpr_wait);
+		if (ret) {
+			dev_err(priv->dev, "Failed to quiescence M4 platform!\n");
+			return ret;
+		}
+	}
+
+	return regmap_update_bits(priv->regmap, dcfg->src_reg, dcfg->src_mask, dcfg->src_stop);
+}
+
 static int imx_rproc_stop(struct rproc *rproc)
 {
 	struct imx_rproc *priv = rproc->priv;
@@ -345,20 +363,6 @@ static int imx_rproc_stop(struct rproc *rproc)
 	}
 
 	switch (dcfg->method) {
-	case IMX_RPROC_MMIO:
-		if (priv->gpr) {
-			ret = regmap_set_bits(priv->gpr, dcfg->gpr_reg,
-					      dcfg->gpr_wait);
-			if (ret) {
-				dev_err(priv->dev,
-					"Failed to quiescence M4 platform!\n");
-				return ret;
-			}
-		}
-
-		ret = regmap_update_bits(priv->regmap, dcfg->src_reg, dcfg->src_mask,
-					 dcfg->src_stop);
-		break;
 	case IMX_RPROC_SMC:
 		arm_smccc_smc(IMX_SIP_RPROC, IMX_SIP_RPROC_STOP, 0, 0, 0, 0, 0, 0, &res);
 		ret = res.a0;
@@ -855,15 +859,60 @@ static int imx_rproc_attach_pd(struct imx_rproc *priv)
 	return 0;
 }
 
-static int imx_rproc_detect_mode(struct imx_rproc *priv)
+static int imx_rproc_mmio_detect_mode(struct rproc *rproc)
 {
-	struct regmap_config config = { .name = "imx-rproc" };
+	const struct regmap_config config = { .name = "imx-rproc" };
+	struct imx_rproc *priv = rproc->priv;
 	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
 	struct device *dev = priv->dev;
 	struct regmap *regmap;
+	u32 val;
+	int ret;
+
+	priv->gpr = syscon_regmap_lookup_by_phandle(dev->of_node, "fsl,iomuxc-gpr");
+	if (IS_ERR(priv->gpr))
+		priv->gpr = NULL;
+
+	regmap = syscon_regmap_lookup_by_phandle(dev->of_node, "syscon");
+	if (IS_ERR(regmap)) {
+		dev_err(dev, "failed to find syscon\n");
+		return PTR_ERR(regmap);
+	}
+
+	priv->regmap = regmap;
+	regmap_attach_dev(dev, regmap, &config);
+
+	if (priv->gpr) {
+		ret = regmap_read(priv->gpr, dcfg->gpr_reg, &val);
+		if (val & dcfg->gpr_wait) {
+			/*
+			 * After cold boot, the CM indicates its in wait
+			 * state, but not fully powered off. Power it off
+			 * fully so firmware can be loaded into it.
+			 */
+			imx_rproc_stop(priv->rproc);
+			return 0;
+		}
+	}
+
+	ret = regmap_read(regmap, dcfg->src_reg, &val);
+	if (ret) {
+		dev_err(dev, "Failed to read src\n");
+		return ret;
+	}
+
+	if ((val & dcfg->src_mask) != dcfg->src_stop)
+		priv->rproc->state = RPROC_DETACHED;
+
+	return 0;
+}
+
+static int imx_rproc_detect_mode(struct imx_rproc *priv)
+{
+	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
+	struct device *dev = priv->dev;
 	struct arm_smccc_res res;
 	int ret;
-	u32 val;
 	u8 pt;
 
 	if (dcfg->ops && dcfg->ops->detect_mode)
@@ -937,41 +986,6 @@ static int imx_rproc_detect_mode(struct imx_rproc *priv)
 		break;
 	}
 
-	priv->gpr = syscon_regmap_lookup_by_phandle(dev->of_node, "fsl,iomuxc-gpr");
-	if (IS_ERR(priv->gpr))
-		priv->gpr = NULL;
-
-	regmap = syscon_regmap_lookup_by_phandle(dev->of_node, "syscon");
-	if (IS_ERR(regmap)) {
-		dev_err(dev, "failed to find syscon\n");
-		return PTR_ERR(regmap);
-	}
-
-	priv->regmap = regmap;
-	regmap_attach_dev(dev, regmap, &config);
-
-	if (priv->gpr) {
-		ret = regmap_read(priv->gpr, dcfg->gpr_reg, &val);
-		if (val & dcfg->gpr_wait) {
-			/*
-			 * After cold boot, the CM indicates its in wait
-			 * state, but not fully powered off. Power it off
-			 * fully so firmware can be loaded into it.
-			 */
-			imx_rproc_stop(priv->rproc);
-			return 0;
-		}
-	}
-
-	ret = regmap_read(regmap, dcfg->src_reg, &val);
-	if (ret) {
-		dev_err(dev, "Failed to read src\n");
-		return ret;
-	}
-
-	if ((val & dcfg->src_mask) != dcfg->src_stop)
-		priv->rproc->state = RPROC_DETACHED;
-
 	return 0;
 }
 
@@ -1143,6 +1157,12 @@ static void imx_rproc_remove(struct platform_device *pdev)
 	destroy_workqueue(priv->workqueue);
 }
 
+static const struct imx_rproc_plat_ops imx_rproc_ops_mmio = {
+	.start		= imx_rproc_mmio_start,
+	.stop		= imx_rproc_mmio_stop,
+	.detect_mode	= imx_rproc_mmio_detect_mode,
+};
+
 static const struct imx_rproc_dcfg imx_rproc_cfg_imx8mn_mmio = {
 	.src_reg	= IMX7D_SRC_SCR,
 	.src_mask	= IMX7D_M4_RST_MASK,
@@ -1153,6 +1173,7 @@ static const struct imx_rproc_dcfg imx_rproc_cfg_imx8mn_mmio = {
 	.att		= imx_rproc_att_imx8mn,
 	.att_size	= ARRAY_SIZE(imx_rproc_att_imx8mn),
 	.method		= IMX_RPROC_MMIO,
+	.ops		= &imx_rproc_ops_mmio,
 };
 
 static const struct imx_rproc_dcfg imx_rproc_cfg_imx8mn = {
@@ -1169,6 +1190,7 @@ static const struct imx_rproc_dcfg imx_rproc_cfg_imx8mq = {
 	.att		= imx_rproc_att_imx8mq,
 	.att_size	= ARRAY_SIZE(imx_rproc_att_imx8mq),
 	.method		= IMX_RPROC_MMIO,
+	.ops		= &imx_rproc_ops_mmio,
 };
 
 static const struct imx_rproc_dcfg imx_rproc_cfg_imx8qm = {
@@ -1204,6 +1226,7 @@ static const struct imx_rproc_dcfg imx_rproc_cfg_imx7d = {
 	.att		= imx_rproc_att_imx7d,
 	.att_size	= ARRAY_SIZE(imx_rproc_att_imx7d),
 	.method		= IMX_RPROC_MMIO,
+	.ops		= &imx_rproc_ops_mmio,
 };
 
 static const struct imx_rproc_dcfg imx_rproc_cfg_imx6sx = {
@@ -1214,6 +1237,7 @@ static const struct imx_rproc_dcfg imx_rproc_cfg_imx6sx = {
 	.att		= imx_rproc_att_imx6sx,
 	.att_size	= ARRAY_SIZE(imx_rproc_att_imx6sx),
 	.method		= IMX_RPROC_MMIO,
+	.ops		= &imx_rproc_ops_mmio,
 };
 
 static const struct imx_rproc_dcfg imx_rproc_cfg_imx93 = {

-- 
2.37.1

[PATCH 4/6] remoteproc: imx_rproc: Simplify IMX_RPROC_SCU_API switch case

[Peng Fan]

Introduce imx_rproc_scu_api_{start, stop, detect_mode}() helper functions
for all i.MX variants using IMX_RPROC_SCU_API to manage remote processors.

This allows the removal of the IMX_RPROC_SCU_API switch-case blocks from
imx_rproc_start(), imx_rproc_stop(), and imx_rproc_detect_mode(), resulting
in cleaner and more maintainable code.

No functional changes.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/remoteproc/imx_rproc.c | 149 +++++++++++++++++++++++------------------
 1 file changed, 85 insertions(+), 64 deletions(-)

diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
index c37dd67595960f08fd85c0b516d0d03855cec9fc..ea34080970c6a5a9b035ef0d389014b8268660a9 100644
--- a/drivers/remoteproc/imx_rproc.c
+++ b/drivers/remoteproc/imx_rproc.c
@@ -296,6 +296,13 @@ static int imx_rproc_mmio_start(struct rproc *rproc)
 	return regmap_update_bits(priv->regmap, dcfg->src_reg, dcfg->src_mask, dcfg->src_start);
 }
 
+static int imx_rproc_scu_api_start(struct rproc *rproc)
+{
+	struct imx_rproc *priv = rproc->priv;
+
+	return imx_sc_pm_cpu_start(priv->ipc_handle, priv->rsrc_id, true, priv->entry);
+}
+
 static int imx_rproc_start(struct rproc *rproc)
 {
 	struct imx_rproc *priv = rproc->priv;
@@ -318,9 +325,6 @@ static int imx_rproc_start(struct rproc *rproc)
 		arm_smccc_smc(IMX_SIP_RPROC, IMX_SIP_RPROC_START, 0, 0, 0, 0, 0, 0, &res);
 		ret = res.a0;
 		break;
-	case IMX_RPROC_SCU_API:
-		ret = imx_sc_pm_cpu_start(priv->ipc_handle, priv->rsrc_id, true, priv->entry);
-		break;
 	default:
 		return -EOPNOTSUPP;
 	}
@@ -349,6 +353,13 @@ static int imx_rproc_mmio_stop(struct rproc *rproc)
 	return regmap_update_bits(priv->regmap, dcfg->src_reg, dcfg->src_mask, dcfg->src_stop);
 }
 
+static int imx_rproc_scu_api_stop(struct rproc *rproc)
+{
+	struct imx_rproc *priv = rproc->priv;
+
+	return imx_sc_pm_cpu_start(priv->ipc_handle, priv->rsrc_id, false, priv->entry);
+}
+
 static int imx_rproc_stop(struct rproc *rproc)
 {
 	struct imx_rproc *priv = rproc->priv;
@@ -369,9 +380,6 @@ static int imx_rproc_stop(struct rproc *rproc)
 		if (res.a1)
 			dev_info(dev, "Not in wfi, force stopped\n");
 		break;
-	case IMX_RPROC_SCU_API:
-		ret = imx_sc_pm_cpu_start(priv->ipc_handle, priv->rsrc_id, false, priv->entry);
-		break;
 	default:
 		return -EOPNOTSUPP;
 	}
@@ -907,14 +915,74 @@ static int imx_rproc_mmio_detect_mode(struct rproc *rproc)
 	return 0;
 }
 
-static int imx_rproc_detect_mode(struct imx_rproc *priv)
+static int imx_rproc_scu_api_detect_mode(struct rproc *rproc)
 {
-	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
+	struct imx_rproc *priv = rproc->priv;
 	struct device *dev = priv->dev;
-	struct arm_smccc_res res;
 	int ret;
 	u8 pt;
 
+	ret = imx_scu_get_handle(&priv->ipc_handle);
+	if (ret)
+		return ret;
+	ret = of_property_read_u32(dev->of_node, "fsl,resource-id", &priv->rsrc_id);
+	if (ret) {
+		dev_err(dev, "No fsl,resource-id property\n");
+		return ret;
+	}
+
+	if (priv->rsrc_id == IMX_SC_R_M4_1_PID0)
+		priv->core_index = 1;
+	else
+		priv->core_index = 0;
+
+	/*
+	 * If Mcore resource is not owned by Acore partition, It is kicked by ROM,
+	 * and Linux could only do IPC with Mcore and nothing else.
+	 */
+	if (imx_sc_rm_is_resource_owned(priv->ipc_handle, priv->rsrc_id)) {
+		if (of_property_read_u32(dev->of_node, "fsl,entry-address", &priv->entry))
+			return -EINVAL;
+
+		return imx_rproc_attach_pd(priv);
+	}
+
+	priv->rproc->state = RPROC_DETACHED;
+	priv->rproc->recovery_disabled = false;
+	rproc_set_feature(priv->rproc, RPROC_FEAT_ATTACH_ON_RECOVERY);
+
+	/* Get partition id and enable irq in SCFW */
+	ret = imx_sc_rm_get_resource_owner(priv->ipc_handle, priv->rsrc_id, &pt);
+	if (ret) {
+		dev_err(dev, "not able to get resource owner\n");
+		return ret;
+	}
+
+	priv->rproc_pt = pt;
+	priv->rproc_nb.notifier_call = imx_rproc_partition_notify;
+
+	ret = imx_scu_irq_register_notifier(&priv->rproc_nb);
+	if (ret) {
+		dev_err(dev, "register scu notifier failed, %d\n", ret);
+		return ret;
+	}
+
+	ret = imx_scu_irq_group_enable(IMX_SC_IRQ_GROUP_REBOOTED, BIT(priv->rproc_pt),
+				       true);
+	if (ret) {
+		imx_scu_irq_unregister_notifier(&priv->rproc_nb);
+		dev_err(dev, "Enable irq failed, %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int imx_rproc_detect_mode(struct imx_rproc *priv)
+{
+	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
+	struct arm_smccc_res res;
+
 	if (dcfg->ops && dcfg->ops->detect_mode)
 		return dcfg->ops->detect_mode(priv->rproc);
 
@@ -927,61 +995,6 @@ static int imx_rproc_detect_mode(struct imx_rproc *priv)
 		if (res.a0)
 			priv->rproc->state = RPROC_DETACHED;
 		return 0;
-	case IMX_RPROC_SCU_API:
-		ret = imx_scu_get_handle(&priv->ipc_handle);
-		if (ret)
-			return ret;
-		ret = of_property_read_u32(dev->of_node, "fsl,resource-id", &priv->rsrc_id);
-		if (ret) {
-			dev_err(dev, "No fsl,resource-id property\n");
-			return ret;
-		}
-
-		if (priv->rsrc_id == IMX_SC_R_M4_1_PID0)
-			priv->core_index = 1;
-		else
-			priv->core_index = 0;
-
-		/*
-		 * If Mcore resource is not owned by Acore partition, It is kicked by ROM,
-		 * and Linux could only do IPC with Mcore and nothing else.
-		 */
-		if (imx_sc_rm_is_resource_owned(priv->ipc_handle, priv->rsrc_id)) {
-			if (of_property_read_u32(dev->of_node, "fsl,entry-address", &priv->entry))
-				return -EINVAL;
-
-			return imx_rproc_attach_pd(priv);
-		}
-
-		priv->rproc->state = RPROC_DETACHED;
-		priv->rproc->recovery_disabled = false;
-		rproc_set_feature(priv->rproc, RPROC_FEAT_ATTACH_ON_RECOVERY);
-
-		/* Get partition id and enable irq in SCFW */
-		ret = imx_sc_rm_get_resource_owner(priv->ipc_handle, priv->rsrc_id, &pt);
-		if (ret) {
-			dev_err(dev, "not able to get resource owner\n");
-			return ret;
-		}
-
-		priv->rproc_pt = pt;
-		priv->rproc_nb.notifier_call = imx_rproc_partition_notify;
-
-		ret = imx_scu_irq_register_notifier(&priv->rproc_nb);
-		if (ret) {
-			dev_err(dev, "register scu notifier failed, %d\n", ret);
-			return ret;
-		}
-
-		ret = imx_scu_irq_group_enable(IMX_SC_IRQ_GROUP_REBOOTED, BIT(priv->rproc_pt),
-					       true);
-		if (ret) {
-			imx_scu_irq_unregister_notifier(&priv->rproc_nb);
-			dev_err(dev, "Enable irq failed, %d\n", ret);
-			return ret;
-		}
-
-		return 0;
 	default:
 		break;
 	}
@@ -1163,6 +1176,12 @@ static const struct imx_rproc_plat_ops imx_rproc_ops_mmio = {
 	.detect_mode	= imx_rproc_mmio_detect_mode,
 };
 
+static const struct imx_rproc_plat_ops imx_rproc_ops_scu_api = {
+	.start		= imx_rproc_scu_api_start,
+	.stop		= imx_rproc_scu_api_stop,
+	.detect_mode	= imx_rproc_scu_api_detect_mode,
+};
+
 static const struct imx_rproc_dcfg imx_rproc_cfg_imx8mn_mmio = {
 	.src_reg	= IMX7D_SRC_SCR,
 	.src_mask	= IMX7D_M4_RST_MASK,
@@ -1197,12 +1216,14 @@ static const struct imx_rproc_dcfg imx_rproc_cfg_imx8qm = {
 	.att            = imx_rproc_att_imx8qm,
 	.att_size       = ARRAY_SIZE(imx_rproc_att_imx8qm),
 	.method         = IMX_RPROC_SCU_API,
+	.ops		= &imx_rproc_ops_scu_api,
 };
 
 static const struct imx_rproc_dcfg imx_rproc_cfg_imx8qxp = {
 	.att		= imx_rproc_att_imx8qxp,
 	.att_size	= ARRAY_SIZE(imx_rproc_att_imx8qxp),
 	.method		= IMX_RPROC_SCU_API,
+	.ops		= &imx_rproc_ops_scu_api,
 };
 
 static const struct imx_rproc_dcfg imx_rproc_cfg_imx8ulp = {

-- 
2.37.1

[PATCH 5/6] remoteproc: imx_rproc: Simplify IMX_RPROC_SMC switch case

[Peng Fan]

Introduce imx_rproc_arm_smc_{start, stop, detect_mode}() helper functions
for all i.MX variants using IMX_RPROC_SMC to manage remote processors.

This allows the removal of the IMX_RPROC_SMC switch-case blocks from
imx_rproc_start(), imx_rproc_stop(), and imx_rproc_detect_mode(), resulting
in cleaner and more maintainable code.

Since this is the last switch in imx_rproc_{start,stop}{}, remove
the switch-case.

No functional changes.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/remoteproc/imx_rproc.c | 69 ++++++++++++++++++++++++++----------------
 1 file changed, 43 insertions(+), 26 deletions(-)

diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
index ea34080970c6a5a9b035ef0d389014b8268660a9..5fa729f4286f6ac939357c32fef41d7d97e5f860 100644
--- a/drivers/remoteproc/imx_rproc.c
+++ b/drivers/remoteproc/imx_rproc.c
@@ -285,6 +285,15 @@ static const struct imx_rproc_att imx_rproc_att_imx6sx[] = {
 	{ 0x80000000, 0x80000000, 0x60000000, 0 },
 };
 
+static int imx_rproc_arm_smc_start(struct rproc *rproc)
+{
+	struct arm_smccc_res res;
+
+	arm_smccc_smc(IMX_SIP_RPROC, IMX_SIP_RPROC_START, 0, 0, 0, 0, 0, 0, &res);
+
+	return res.a0;
+}
+
 static int imx_rproc_mmio_start(struct rproc *rproc)
 {
 	struct imx_rproc *priv = rproc->priv;
@@ -308,7 +317,6 @@ static int imx_rproc_start(struct rproc *rproc)
 	struct imx_rproc *priv = rproc->priv;
 	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
 	struct device *dev = priv->dev;
-	struct arm_smccc_res res;
 	int ret;
 
 	ret = imx_rproc_xtr_mbox_init(rproc, true);
@@ -320,14 +328,7 @@ static int imx_rproc_start(struct rproc *rproc)
 		goto start_ret;
 	}
 
-	switch (dcfg->method) {
-	case IMX_RPROC_SMC:
-		arm_smccc_smc(IMX_SIP_RPROC, IMX_SIP_RPROC_START, 0, 0, 0, 0, 0, 0, &res);
-		ret = res.a0;
-		break;
-	default:
-		return -EOPNOTSUPP;
-	}
+	return -EOPNOTSUPP;
 
 start_ret:
 	if (ret)
@@ -336,6 +337,18 @@ static int imx_rproc_start(struct rproc *rproc)
 	return ret;
 }
 
+static int imx_rproc_arm_smc_stop(struct rproc *rproc)
+{
+	struct imx_rproc *priv = rproc->priv;
+	struct arm_smccc_res res;
+
+	arm_smccc_smc(IMX_SIP_RPROC, IMX_SIP_RPROC_STOP, 0, 0, 0, 0, 0, 0, &res);
+	if (res.a1)
+		dev_info(priv->dev, "Not in wfi, force stopped\n");
+
+	return res.a0;
+}
+
 static int imx_rproc_mmio_stop(struct rproc *rproc)
 {
 	struct imx_rproc *priv = rproc->priv;
@@ -365,7 +378,6 @@ static int imx_rproc_stop(struct rproc *rproc)
 	struct imx_rproc *priv = rproc->priv;
 	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
 	struct device *dev = priv->dev;
-	struct arm_smccc_res res;
 	int ret;
 
 	if (dcfg->ops && dcfg->ops->stop) {
@@ -373,16 +385,7 @@ static int imx_rproc_stop(struct rproc *rproc)
 		goto stop_ret;
 	}
 
-	switch (dcfg->method) {
-	case IMX_RPROC_SMC:
-		arm_smccc_smc(IMX_SIP_RPROC, IMX_SIP_RPROC_STOP, 0, 0, 0, 0, 0, 0, &res);
-		ret = res.a0;
-		if (res.a1)
-			dev_info(dev, "Not in wfi, force stopped\n");
-		break;
-	default:
-		return -EOPNOTSUPP;
-	}
+	return -EOPNOTSUPP;
 
 stop_ret:
 	if (ret)
@@ -867,6 +870,18 @@ static int imx_rproc_attach_pd(struct imx_rproc *priv)
 	return 0;
 }
 
+static int imx_rproc_arm_smc_detect_mode(struct rproc *rproc)
+{
+	struct imx_rproc *priv = rproc->priv;
+	struct arm_smccc_res res;
+
+	arm_smccc_smc(IMX_SIP_RPROC, IMX_SIP_RPROC_STARTED, 0, 0, 0, 0, 0, 0, &res);
+	if (res.a0)
+		priv->rproc->state = RPROC_DETACHED;
+
+	return 0;
+}
+
 static int imx_rproc_mmio_detect_mode(struct rproc *rproc)
 {
 	const struct regmap_config config = { .name = "imx-rproc" };
@@ -981,7 +996,6 @@ static int imx_rproc_scu_api_detect_mode(struct rproc *rproc)
 static int imx_rproc_detect_mode(struct imx_rproc *priv)
 {
 	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
-	struct arm_smccc_res res;
 
 	if (dcfg->ops && dcfg->ops->detect_mode)
 		return dcfg->ops->detect_mode(priv->rproc);
@@ -990,11 +1004,6 @@ static int imx_rproc_detect_mode(struct imx_rproc *priv)
 	case IMX_RPROC_NONE:
 		priv->rproc->state = RPROC_DETACHED;
 		return 0;
-	case IMX_RPROC_SMC:
-		arm_smccc_smc(IMX_SIP_RPROC, IMX_SIP_RPROC_STARTED, 0, 0, 0, 0, 0, 0, &res);
-		if (res.a0)
-			priv->rproc->state = RPROC_DETACHED;
-		return 0;
 	default:
 		break;
 	}
@@ -1170,6 +1179,12 @@ static void imx_rproc_remove(struct platform_device *pdev)
 	destroy_workqueue(priv->workqueue);
 }
 
+static const struct imx_rproc_plat_ops imx_rproc_ops_arm_smc = {
+	.start		= imx_rproc_arm_smc_start,
+	.stop		= imx_rproc_arm_smc_stop,
+	.detect_mode	= imx_rproc_arm_smc_detect_mode,
+};
+
 static const struct imx_rproc_plat_ops imx_rproc_ops_mmio = {
 	.start		= imx_rproc_mmio_start,
 	.stop		= imx_rproc_mmio_stop,
@@ -1199,6 +1214,7 @@ static const struct imx_rproc_dcfg imx_rproc_cfg_imx8mn = {
 	.att		= imx_rproc_att_imx8mn,
 	.att_size	= ARRAY_SIZE(imx_rproc_att_imx8mn),
 	.method		= IMX_RPROC_SMC,
+	.ops		= &imx_rproc_ops_arm_smc,
 };
 
 static const struct imx_rproc_dcfg imx_rproc_cfg_imx8mq = {
@@ -1265,6 +1281,7 @@ static const struct imx_rproc_dcfg imx_rproc_cfg_imx93 = {
 	.att		= imx_rproc_att_imx93,
 	.att_size	= ARRAY_SIZE(imx_rproc_att_imx93),
 	.method		= IMX_RPROC_SMC,
+	.ops		= &imx_rproc_ops_arm_smc,
 };
 
 static const struct of_device_id imx_rproc_of_match[] = {

-- 
2.37.1

[PATCH 6/6] remoteproc: imx_rproc: Clean up after ops introduction

[Peng Fan]

With the switch-case in imx_rproc_{start,stop}{} removed, simplify
the code logic by removing 'goto'. The last switch-case in
imx_rproc_detect_mode() are no longer needed and can be removed.

This cleanup improves code readability and aligns with the new ops-based
design.

No functional changes.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/remoteproc/imx_rproc.c | 25 +++++++------------------
 1 file changed, 7 insertions(+), 18 deletions(-)

diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
index 5fa729f4286f6ac939357c32fef41d7d97e5f860..ed8395b097d07d02e5323550ff497819f9b95ad6 100644
--- a/drivers/remoteproc/imx_rproc.c
+++ b/drivers/remoteproc/imx_rproc.c
@@ -323,14 +323,11 @@ static int imx_rproc_start(struct rproc *rproc)
 	if (ret)
 		return ret;
 
-	if (dcfg->ops && dcfg->ops->start) {
+	if (dcfg->ops && dcfg->ops->start)
 		ret = dcfg->ops->start(rproc);
-		goto start_ret;
-	}
-
-	return -EOPNOTSUPP;
+	else
+		return -EOPNOTSUPP;
 
-start_ret:
 	if (ret)
 		dev_err(dev, "Failed to enable remote core!\n");
 
@@ -380,14 +377,11 @@ static int imx_rproc_stop(struct rproc *rproc)
 	struct device *dev = priv->dev;
 	int ret;
 
-	if (dcfg->ops && dcfg->ops->stop) {
+	if (dcfg->ops && dcfg->ops->stop)
 		ret = dcfg->ops->stop(rproc);
-		goto stop_ret;
-	}
-
-	return -EOPNOTSUPP;
+	else
+		return -EOPNOTSUPP;
 
-stop_ret:
 	if (ret)
 		dev_err(dev, "Failed to stop remote core\n");
 	else
@@ -1000,13 +994,8 @@ static int imx_rproc_detect_mode(struct imx_rproc *priv)
 	if (dcfg->ops && dcfg->ops->detect_mode)
 		return dcfg->ops->detect_mode(priv->rproc);
 
-	switch (dcfg->method) {
-	case IMX_RPROC_NONE:
+	if (dcfg->method == IMX_RPROC_NONE)
 		priv->rproc->state = RPROC_DETACHED;
-		return 0;
-	default:
-		break;
-	}
 
 	return 0;
 }

-- 
2.37.1

Re: [PATCH 1/6] remoteproc: imx_rproc: Introduce start/stop/detect_mode ops for imx_rproc_dcfg

[Frank Li]

On Mon, Sep 08, 2025 at 09:07:34PM +0800, Peng Fan wrote:
> Simplify the logic in imx_rproc_start(), imx_rproc_stop() and
> imx_rproc_detect_mode(), introduce start, stop and detect_mode ops for the
> imx_rproc_dcfg structure. Allows each platform to provide its own
> implementation of start/stop/detect_mode operations, and prepares to
> eliminate the need for multiple switch-case statements.
>

Nit:
s/Allows/Allow
s/prepares/prepare

> Improves code readability and maintainability by encapsulating

s/Improves/Improve

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> platform-specific behavior.
>
> No functional changes.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/remoteproc/imx_rproc.c | 15 +++++++++++++++
>  drivers/remoteproc/imx_rproc.h |  7 +++++++
>  2 files changed, 22 insertions(+)
>
> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index a6eef0080ca9e46efe60dcb3878b9efdbdc0f08e..5cdc5045e57566e817170ed3c708dad6108d2e46 100644
> --- a/drivers/remoteproc/imx_rproc.c
> +++ b/drivers/remoteproc/imx_rproc.c
> @@ -376,6 +376,11 @@ static int imx_rproc_start(struct rproc *rproc)
>  	if (ret)
>  		return ret;
>
> +	if (dcfg->ops && dcfg->ops->start) {
> +		ret = dcfg->ops->start(rproc);
> +		goto start_ret;
> +	}
> +
>  	switch (dcfg->method) {
>  	case IMX_RPROC_MMIO:
>  		if (priv->gpr) {
> @@ -398,6 +403,7 @@ static int imx_rproc_start(struct rproc *rproc)
>  		return -EOPNOTSUPP;
>  	}
>
> +start_ret:
>  	if (ret)
>  		dev_err(dev, "Failed to enable remote core!\n");
>
> @@ -412,6 +418,11 @@ static int imx_rproc_stop(struct rproc *rproc)
>  	struct arm_smccc_res res;
>  	int ret;
>
> +	if (dcfg->ops && dcfg->ops->stop) {
> +		ret = dcfg->ops->stop(rproc);
> +		goto stop_ret;
> +	}
> +
>  	switch (dcfg->method) {
>  	case IMX_RPROC_MMIO:
>  		if (priv->gpr) {
> @@ -440,6 +451,7 @@ static int imx_rproc_stop(struct rproc *rproc)
>  		return -EOPNOTSUPP;
>  	}
>
> +stop_ret:
>  	if (ret)
>  		dev_err(dev, "Failed to stop remote core\n");
>  	else
> @@ -933,6 +945,9 @@ static int imx_rproc_detect_mode(struct imx_rproc *priv)
>  	u32 val;
>  	u8 pt;
>
> +	if (dcfg->ops && dcfg->ops->detect_mode)
> +		return dcfg->ops->detect_mode(priv->rproc);
> +
>  	switch (dcfg->method) {
>  	case IMX_RPROC_NONE:
>  		priv->rproc->state = RPROC_DETACHED;
> diff --git a/drivers/remoteproc/imx_rproc.h b/drivers/remoteproc/imx_rproc.h
> index cfd38d37e1467d1d9e6f89be146c0b53262b92a0..3a9adaaf048b396102feeb45488cd2ff125a807a 100644
> --- a/drivers/remoteproc/imx_rproc.h
> +++ b/drivers/remoteproc/imx_rproc.h
> @@ -31,6 +31,12 @@ enum imx_rproc_method {
>  /* dcfg flags */
>  #define IMX_RPROC_NEED_SYSTEM_OFF	BIT(0)
>
> +struct imx_rproc_plat_ops {
> +	int (*start)(struct rproc *rproc);
> +	int (*stop)(struct rproc *rproc);
> +	int (*detect_mode)(struct rproc *rproc);
> +};
> +
>  struct imx_rproc_dcfg {
>  	u32				src_reg;
>  	u32				src_mask;
> @@ -42,6 +48,7 @@ struct imx_rproc_dcfg {
>  	size_t				att_size;
>  	enum imx_rproc_method		method;
>  	u32				flags;
> +	const struct imx_rproc_plat_ops	*ops;
>  };
>
>  #endif /* _IMX_RPROC_H */
>
> --
> 2.37.1
>

Re: [PATCH 2/6] remoteproc: imx_rproc: Move imx_rproc_dcfg closer to imx_rproc_of_match

[Frank Li]

On Mon, Sep 08, 2025 at 09:07:35PM +0800, Peng Fan wrote:
> Move the imx_rproc_dcfg structure definitions closer to imx_rproc_of_match
> to prepare for adding start/stop/detect_mode ops for each i.MX variant.
>
> This relocation avoids the need to declare function prototypes such as
> 'static int imx_rproc_xtr_mbox_init(struct rproc *rproc, bool tx_block)'
> at the beginning of the file, improving code organization and readability.
>
> No functional changes.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> ---
>  drivers/remoteproc/imx_rproc.c | 158 ++++++++++++++++++++---------------------
>  1 file changed, 79 insertions(+), 79 deletions(-)
>
> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index 5cdc5045e57566e817170ed3c708dad6108d2e46..af7b69d750deed734668cb413b29378e5ca7c64e 100644
> --- a/drivers/remoteproc/imx_rproc.c
> +++ b/drivers/remoteproc/imx_rproc.c
> @@ -285,85 +285,6 @@ static const struct imx_rproc_att imx_rproc_att_imx6sx[] = {
>  	{ 0x80000000, 0x80000000, 0x60000000, 0 },
>  };
>
> -static const struct imx_rproc_dcfg imx_rproc_cfg_imx8mn_mmio = {
> -	.src_reg	= IMX7D_SRC_SCR,
> -	.src_mask	= IMX7D_M4_RST_MASK,
> -	.src_start	= IMX7D_M4_START,
> -	.src_stop	= IMX8M_M7_STOP,
> -	.gpr_reg	= IMX8M_GPR22,
> -	.gpr_wait	= IMX8M_GPR22_CM7_CPUWAIT,
> -	.att		= imx_rproc_att_imx8mn,
> -	.att_size	= ARRAY_SIZE(imx_rproc_att_imx8mn),
> -	.method		= IMX_RPROC_MMIO,
> -};
> -
> -static const struct imx_rproc_dcfg imx_rproc_cfg_imx8mn = {
> -	.att		= imx_rproc_att_imx8mn,
> -	.att_size	= ARRAY_SIZE(imx_rproc_att_imx8mn),
> -	.method		= IMX_RPROC_SMC,
> -};
> -
> -static const struct imx_rproc_dcfg imx_rproc_cfg_imx8mq = {
> -	.src_reg	= IMX7D_SRC_SCR,
> -	.src_mask	= IMX7D_M4_RST_MASK,
> -	.src_start	= IMX7D_M4_START,
> -	.src_stop	= IMX7D_M4_STOP,
> -	.att		= imx_rproc_att_imx8mq,
> -	.att_size	= ARRAY_SIZE(imx_rproc_att_imx8mq),
> -	.method		= IMX_RPROC_MMIO,
> -};
> -
> -static const struct imx_rproc_dcfg imx_rproc_cfg_imx8qm = {
> -	.att            = imx_rproc_att_imx8qm,
> -	.att_size       = ARRAY_SIZE(imx_rproc_att_imx8qm),
> -	.method         = IMX_RPROC_SCU_API,
> -};
> -
> -static const struct imx_rproc_dcfg imx_rproc_cfg_imx8qxp = {
> -	.att		= imx_rproc_att_imx8qxp,
> -	.att_size	= ARRAY_SIZE(imx_rproc_att_imx8qxp),
> -	.method		= IMX_RPROC_SCU_API,
> -};
> -
> -static const struct imx_rproc_dcfg imx_rproc_cfg_imx8ulp = {
> -	.att		= imx_rproc_att_imx8ulp,
> -	.att_size	= ARRAY_SIZE(imx_rproc_att_imx8ulp),
> -	.method		= IMX_RPROC_NONE,
> -};
> -
> -static const struct imx_rproc_dcfg imx_rproc_cfg_imx7ulp = {
> -	.att		= imx_rproc_att_imx7ulp,
> -	.att_size	= ARRAY_SIZE(imx_rproc_att_imx7ulp),
> -	.method		= IMX_RPROC_NONE,
> -	.flags		= IMX_RPROC_NEED_SYSTEM_OFF,
> -};
> -
> -static const struct imx_rproc_dcfg imx_rproc_cfg_imx7d = {
> -	.src_reg	= IMX7D_SRC_SCR,
> -	.src_mask	= IMX7D_M4_RST_MASK,
> -	.src_start	= IMX7D_M4_START,
> -	.src_stop	= IMX7D_M4_STOP,
> -	.att		= imx_rproc_att_imx7d,
> -	.att_size	= ARRAY_SIZE(imx_rproc_att_imx7d),
> -	.method		= IMX_RPROC_MMIO,
> -};
> -
> -static const struct imx_rproc_dcfg imx_rproc_cfg_imx6sx = {
> -	.src_reg	= IMX6SX_SRC_SCR,
> -	.src_mask	= IMX6SX_M4_RST_MASK,
> -	.src_start	= IMX6SX_M4_START,
> -	.src_stop	= IMX6SX_M4_STOP,
> -	.att		= imx_rproc_att_imx6sx,
> -	.att_size	= ARRAY_SIZE(imx_rproc_att_imx6sx),
> -	.method		= IMX_RPROC_MMIO,
> -};
> -
> -static const struct imx_rproc_dcfg imx_rproc_cfg_imx93 = {
> -	.att		= imx_rproc_att_imx93,
> -	.att_size	= ARRAY_SIZE(imx_rproc_att_imx93),
> -	.method		= IMX_RPROC_SMC,
> -};
> -
>  static int imx_rproc_start(struct rproc *rproc)
>  {
>  	struct imx_rproc *priv = rproc->priv;
> @@ -1222,6 +1143,85 @@ static void imx_rproc_remove(struct platform_device *pdev)
>  	destroy_workqueue(priv->workqueue);
>  }
>
> +static const struct imx_rproc_dcfg imx_rproc_cfg_imx8mn_mmio = {
> +	.src_reg	= IMX7D_SRC_SCR,
> +	.src_mask	= IMX7D_M4_RST_MASK,
> +	.src_start	= IMX7D_M4_START,
> +	.src_stop	= IMX8M_M7_STOP,
> +	.gpr_reg	= IMX8M_GPR22,
> +	.gpr_wait	= IMX8M_GPR22_CM7_CPUWAIT,
> +	.att		= imx_rproc_att_imx8mn,
> +	.att_size	= ARRAY_SIZE(imx_rproc_att_imx8mn),
> +	.method		= IMX_RPROC_MMIO,
> +};
> +
> +static const struct imx_rproc_dcfg imx_rproc_cfg_imx8mn = {
> +	.att		= imx_rproc_att_imx8mn,
> +	.att_size	= ARRAY_SIZE(imx_rproc_att_imx8mn),
> +	.method		= IMX_RPROC_SMC,
> +};
> +
> +static const struct imx_rproc_dcfg imx_rproc_cfg_imx8mq = {
> +	.src_reg	= IMX7D_SRC_SCR,
> +	.src_mask	= IMX7D_M4_RST_MASK,
> +	.src_start	= IMX7D_M4_START,
> +	.src_stop	= IMX7D_M4_STOP,
> +	.att		= imx_rproc_att_imx8mq,
> +	.att_size	= ARRAY_SIZE(imx_rproc_att_imx8mq),
> +	.method		= IMX_RPROC_MMIO,
> +};
> +
> +static const struct imx_rproc_dcfg imx_rproc_cfg_imx8qm = {
> +	.att            = imx_rproc_att_imx8qm,
> +	.att_size       = ARRAY_SIZE(imx_rproc_att_imx8qm),
> +	.method         = IMX_RPROC_SCU_API,
> +};
> +
> +static const struct imx_rproc_dcfg imx_rproc_cfg_imx8qxp = {
> +	.att		= imx_rproc_att_imx8qxp,
> +	.att_size	= ARRAY_SIZE(imx_rproc_att_imx8qxp),
> +	.method		= IMX_RPROC_SCU_API,
> +};
> +
> +static const struct imx_rproc_dcfg imx_rproc_cfg_imx8ulp = {
> +	.att		= imx_rproc_att_imx8ulp,
> +	.att_size	= ARRAY_SIZE(imx_rproc_att_imx8ulp),
> +	.method		= IMX_RPROC_NONE,
> +};
> +
> +static const struct imx_rproc_dcfg imx_rproc_cfg_imx7ulp = {
> +	.att		= imx_rproc_att_imx7ulp,
> +	.att_size	= ARRAY_SIZE(imx_rproc_att_imx7ulp),
> +	.method		= IMX_RPROC_NONE,
> +	.flags		= IMX_RPROC_NEED_SYSTEM_OFF,
> +};
> +
> +static const struct imx_rproc_dcfg imx_rproc_cfg_imx7d = {
> +	.src_reg	= IMX7D_SRC_SCR,
> +	.src_mask	= IMX7D_M4_RST_MASK,
> +	.src_start	= IMX7D_M4_START,
> +	.src_stop	= IMX7D_M4_STOP,
> +	.att		= imx_rproc_att_imx7d,
> +	.att_size	= ARRAY_SIZE(imx_rproc_att_imx7d),
> +	.method		= IMX_RPROC_MMIO,
> +};
> +
> +static const struct imx_rproc_dcfg imx_rproc_cfg_imx6sx = {
> +	.src_reg	= IMX6SX_SRC_SCR,
> +	.src_mask	= IMX6SX_M4_RST_MASK,
> +	.src_start	= IMX6SX_M4_START,
> +	.src_stop	= IMX6SX_M4_STOP,
> +	.att		= imx_rproc_att_imx6sx,
> +	.att_size	= ARRAY_SIZE(imx_rproc_att_imx6sx),
> +	.method		= IMX_RPROC_MMIO,
> +};
> +
> +static const struct imx_rproc_dcfg imx_rproc_cfg_imx93 = {
> +	.att		= imx_rproc_att_imx93,
> +	.att_size	= ARRAY_SIZE(imx_rproc_att_imx93),
> +	.method		= IMX_RPROC_SMC,
> +};
> +
>  static const struct of_device_id imx_rproc_of_match[] = {
>  	{ .compatible = "fsl,imx7ulp-cm4", .data = &imx_rproc_cfg_imx7ulp },
>  	{ .compatible = "fsl,imx7d-cm4", .data = &imx_rproc_cfg_imx7d },
>
> --
> 2.37.1
>

Re: [PATCH 3/6] remoteproc: imx_rproc: Simplify IMX_RPROC_MMIO switch case

[Frank Li]

On Mon, Sep 08, 2025 at 09:07:36PM +0800, Peng Fan wrote:
> Introduce imx_rproc_mmio_{start, stop, detect_mode}() helper functions for
> all i.MX variants using IMX_RPROC_MMIO to manage remote processors.
>
> This allows the removal of the IMX_RPROC_MMIO switch-case blocks from
> imx_rproc_start(), imx_rproc_stop(), and imx_rproc_detect_mode(), resulting
> in cleaner and more maintainable code.
>
> No functional changes.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
> ---
>  drivers/remoteproc/imx_rproc.c | 148 ++++++++++++++++++++++++-----------------
>  1 file changed, 86 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index af7b69d750deed734668cb413b29378e5ca7c64e..c37dd67595960f08fd85c0b516d0d03855cec9fc 100644
> --- a/drivers/remoteproc/imx_rproc.c
> +++ b/drivers/remoteproc/imx_rproc.c
> @@ -285,6 +285,17 @@ static const struct imx_rproc_att imx_rproc_att_imx6sx[] = {
>  	{ 0x80000000, 0x80000000, 0x60000000, 0 },
>  };
>
> +static int imx_rproc_mmio_start(struct rproc *rproc)
> +{
> +	struct imx_rproc *priv = rproc->priv;
> +	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
> +
> +	if (priv->gpr)
> +		return regmap_clear_bits(priv->gpr, dcfg->gpr_reg, dcfg->gpr_wait);
> +
> +	return regmap_update_bits(priv->regmap, dcfg->src_reg, dcfg->src_mask, dcfg->src_start);
> +}
> +
>  static int imx_rproc_start(struct rproc *rproc)
>  {
>  	struct imx_rproc *priv = rproc->priv;
> @@ -303,16 +314,6 @@ static int imx_rproc_start(struct rproc *rproc)
>  	}
>
>  	switch (dcfg->method) {
> -	case IMX_RPROC_MMIO:
> -		if (priv->gpr) {
> -			ret = regmap_clear_bits(priv->gpr, dcfg->gpr_reg,
> -						dcfg->gpr_wait);
> -		} else {
> -			ret = regmap_update_bits(priv->regmap, dcfg->src_reg,
> -						 dcfg->src_mask,
> -						 dcfg->src_start);
> -		}
> -		break;
>  	case IMX_RPROC_SMC:
>  		arm_smccc_smc(IMX_SIP_RPROC, IMX_SIP_RPROC_START, 0, 0, 0, 0, 0, 0, &res);
>  		ret = res.a0;
> @@ -331,6 +332,23 @@ static int imx_rproc_start(struct rproc *rproc)
>  	return ret;
>  }
>
> +static int imx_rproc_mmio_stop(struct rproc *rproc)
> +{
> +	struct imx_rproc *priv = rproc->priv;
> +	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
> +	int ret;
> +
> +	if (priv->gpr) {
> +		ret = regmap_set_bits(priv->gpr, dcfg->gpr_reg, dcfg->gpr_wait);
> +		if (ret) {
> +			dev_err(priv->dev, "Failed to quiescence M4 platform!\n");
> +			return ret;
> +		}
> +	}
> +
> +	return regmap_update_bits(priv->regmap, dcfg->src_reg, dcfg->src_mask, dcfg->src_stop);
> +}
> +
>  static int imx_rproc_stop(struct rproc *rproc)
>  {
>  	struct imx_rproc *priv = rproc->priv;
> @@ -345,20 +363,6 @@ static int imx_rproc_stop(struct rproc *rproc)
>  	}
>
>  	switch (dcfg->method) {
> -	case IMX_RPROC_MMIO:
> -		if (priv->gpr) {
> -			ret = regmap_set_bits(priv->gpr, dcfg->gpr_reg,
> -					      dcfg->gpr_wait);
> -			if (ret) {
> -				dev_err(priv->dev,
> -					"Failed to quiescence M4 platform!\n");
> -				return ret;
> -			}
> -		}
> -
> -		ret = regmap_update_bits(priv->regmap, dcfg->src_reg, dcfg->src_mask,
> -					 dcfg->src_stop);
> -		break;
>  	case IMX_RPROC_SMC:
>  		arm_smccc_smc(IMX_SIP_RPROC, IMX_SIP_RPROC_STOP, 0, 0, 0, 0, 0, 0, &res);
>  		ret = res.a0;
> @@ -855,15 +859,60 @@ static int imx_rproc_attach_pd(struct imx_rproc *priv)
>  	return 0;
>  }
>
> -static int imx_rproc_detect_mode(struct imx_rproc *priv)
> +static int imx_rproc_mmio_detect_mode(struct rproc *rproc)
>  {
> -	struct regmap_config config = { .name = "imx-rproc" };
> +	const struct regmap_config config = { .name = "imx-rproc" };
> +	struct imx_rproc *priv = rproc->priv;
>  	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
>  	struct device *dev = priv->dev;
>  	struct regmap *regmap;
> +	u32 val;
> +	int ret;
> +
> +	priv->gpr = syscon_regmap_lookup_by_phandle(dev->of_node, "fsl,iomuxc-gpr");
> +	if (IS_ERR(priv->gpr))
> +		priv->gpr = NULL;
> +
> +	regmap = syscon_regmap_lookup_by_phandle(dev->of_node, "syscon");
> +	if (IS_ERR(regmap)) {
> +		dev_err(dev, "failed to find syscon\n");
> +		return PTR_ERR(regmap);
> +	}
> +
> +	priv->regmap = regmap;
> +	regmap_attach_dev(dev, regmap, &config);
> +
> +	if (priv->gpr) {
> +		ret = regmap_read(priv->gpr, dcfg->gpr_reg, &val);
> +		if (val & dcfg->gpr_wait) {
> +			/*
> +			 * After cold boot, the CM indicates its in wait
> +			 * state, but not fully powered off. Power it off
> +			 * fully so firmware can be loaded into it.
> +			 */
> +			imx_rproc_stop(priv->rproc);
> +			return 0;
> +		}
> +	}
> +
> +	ret = regmap_read(regmap, dcfg->src_reg, &val);
> +	if (ret) {
> +		dev_err(dev, "Failed to read src\n");
> +		return ret;
> +	}
> +
> +	if ((val & dcfg->src_mask) != dcfg->src_stop)
> +		priv->rproc->state = RPROC_DETACHED;
> +
> +	return 0;
> +}
> +
> +static int imx_rproc_detect_mode(struct imx_rproc *priv)
> +{
> +	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
> +	struct device *dev = priv->dev;
>  	struct arm_smccc_res res;
>  	int ret;
> -	u32 val;
>  	u8 pt;
>
>  	if (dcfg->ops && dcfg->ops->detect_mode)
> @@ -937,41 +986,6 @@ static int imx_rproc_detect_mode(struct imx_rproc *priv)
>  		break;
>  	}
>
> -	priv->gpr = syscon_regmap_lookup_by_phandle(dev->of_node, "fsl,iomuxc-gpr");
> -	if (IS_ERR(priv->gpr))
> -		priv->gpr = NULL;
> -
> -	regmap = syscon_regmap_lookup_by_phandle(dev->of_node, "syscon");
> -	if (IS_ERR(regmap)) {
> -		dev_err(dev, "failed to find syscon\n");
> -		return PTR_ERR(regmap);
> -	}
> -
> -	priv->regmap = regmap;
> -	regmap_attach_dev(dev, regmap, &config);
> -
> -	if (priv->gpr) {
> -		ret = regmap_read(priv->gpr, dcfg->gpr_reg, &val);
> -		if (val & dcfg->gpr_wait) {
> -			/*
> -			 * After cold boot, the CM indicates its in wait
> -			 * state, but not fully powered off. Power it off
> -			 * fully so firmware can be loaded into it.
> -			 */
> -			imx_rproc_stop(priv->rproc);
> -			return 0;
> -		}
> -	}
> -
> -	ret = regmap_read(regmap, dcfg->src_reg, &val);
> -	if (ret) {
> -		dev_err(dev, "Failed to read src\n");
> -		return ret;
> -	}
> -
> -	if ((val & dcfg->src_mask) != dcfg->src_stop)
> -		priv->rproc->state = RPROC_DETACHED;
> -
>  	return 0;
>  }
>
> @@ -1143,6 +1157,12 @@ static void imx_rproc_remove(struct platform_device *pdev)
>  	destroy_workqueue(priv->workqueue);
>  }
>
> +static const struct imx_rproc_plat_ops imx_rproc_ops_mmio = {
> +	.start		= imx_rproc_mmio_start,
> +	.stop		= imx_rproc_mmio_stop,
> +	.detect_mode	= imx_rproc_mmio_detect_mode,
> +};
> +
>  static const struct imx_rproc_dcfg imx_rproc_cfg_imx8mn_mmio = {
>  	.src_reg	= IMX7D_SRC_SCR,
>  	.src_mask	= IMX7D_M4_RST_MASK,
> @@ -1153,6 +1173,7 @@ static const struct imx_rproc_dcfg imx_rproc_cfg_imx8mn_mmio = {
>  	.att		= imx_rproc_att_imx8mn,
>  	.att_size	= ARRAY_SIZE(imx_rproc_att_imx8mn),
>  	.method		= IMX_RPROC_MMIO,
> +	.ops		= &imx_rproc_ops_mmio,
>  };
>
>  static const struct imx_rproc_dcfg imx_rproc_cfg_imx8mn = {
> @@ -1169,6 +1190,7 @@ static const struct imx_rproc_dcfg imx_rproc_cfg_imx8mq = {
>  	.att		= imx_rproc_att_imx8mq,
>  	.att_size	= ARRAY_SIZE(imx_rproc_att_imx8mq),
>  	.method		= IMX_RPROC_MMIO,
> +	.ops		= &imx_rproc_ops_mmio,
>  };
>
>  static const struct imx_rproc_dcfg imx_rproc_cfg_imx8qm = {
> @@ -1204,6 +1226,7 @@ static const struct imx_rproc_dcfg imx_rproc_cfg_imx7d = {
>  	.att		= imx_rproc_att_imx7d,
>  	.att_size	= ARRAY_SIZE(imx_rproc_att_imx7d),
>  	.method		= IMX_RPROC_MMIO,
> +	.ops		= &imx_rproc_ops_mmio,
>  };
>
>  static const struct imx_rproc_dcfg imx_rproc_cfg_imx6sx = {
> @@ -1214,6 +1237,7 @@ static const struct imx_rproc_dcfg imx_rproc_cfg_imx6sx = {
>  	.att		= imx_rproc_att_imx6sx,
>  	.att_size	= ARRAY_SIZE(imx_rproc_att_imx6sx),
>  	.method		= IMX_RPROC_MMIO,
> +	.ops		= &imx_rproc_ops_mmio,
>  };
>
>  static const struct imx_rproc_dcfg imx_rproc_cfg_imx93 = {
>
> --
> 2.37.1
>

Re: [PATCH 4/6] remoteproc: imx_rproc: Simplify IMX_RPROC_SCU_API switch case

[Frank Li]

On Mon, Sep 08, 2025 at 09:07:37PM +0800, Peng Fan wrote:
> Introduce imx_rproc_scu_api_{start, stop, detect_mode}() helper functions
> for all i.MX variants using IMX_RPROC_SCU_API to manage remote processors.
>
> This allows the removal of the IMX_RPROC_SCU_API switch-case blocks from
> imx_rproc_start(), imx_rproc_stop(), and imx_rproc_detect_mode(), resulting
> in cleaner and more maintainable code.
>
> No functional changes.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/remoteproc/imx_rproc.c | 149 +++++++++++++++++++++++------------------
>  1 file changed, 85 insertions(+), 64 deletions(-)
>
> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index c37dd67595960f08fd85c0b516d0d03855cec9fc..ea34080970c6a5a9b035ef0d389014b8268660a9 100644
> --- a/drivers/remoteproc/imx_rproc.c
> +++ b/drivers/remoteproc/imx_rproc.c
> @@ -296,6 +296,13 @@ static int imx_rproc_mmio_start(struct rproc *rproc)
>  	return regmap_update_bits(priv->regmap, dcfg->src_reg, dcfg->src_mask, dcfg->src_start);
>  }
>
> +static int imx_rproc_scu_api_start(struct rproc *rproc)
> +{
> +	struct imx_rproc *priv = rproc->priv;
> +
> +	return imx_sc_pm_cpu_start(priv->ipc_handle, priv->rsrc_id, true, priv->entry);
> +}
> +
>  static int imx_rproc_start(struct rproc *rproc)
>  {
>  	struct imx_rproc *priv = rproc->priv;
> @@ -318,9 +325,6 @@ static int imx_rproc_start(struct rproc *rproc)
>  		arm_smccc_smc(IMX_SIP_RPROC, IMX_SIP_RPROC_START, 0, 0, 0, 0, 0, 0, &res);
>  		ret = res.a0;
>  		break;
> -	case IMX_RPROC_SCU_API:
> -		ret = imx_sc_pm_cpu_start(priv->ipc_handle, priv->rsrc_id, true, priv->entry);
> -		break;
>  	default:
>  		return -EOPNOTSUPP;
>  	}
> @@ -349,6 +353,13 @@ static int imx_rproc_mmio_stop(struct rproc *rproc)
>  	return regmap_update_bits(priv->regmap, dcfg->src_reg, dcfg->src_mask, dcfg->src_stop);
>  }
>
> +static int imx_rproc_scu_api_stop(struct rproc *rproc)
> +{
> +	struct imx_rproc *priv = rproc->priv;
> +
> +	return imx_sc_pm_cpu_start(priv->ipc_handle, priv->rsrc_id, false, priv->entry);
> +}
> +
>  static int imx_rproc_stop(struct rproc *rproc)
>  {
>  	struct imx_rproc *priv = rproc->priv;
> @@ -369,9 +380,6 @@ static int imx_rproc_stop(struct rproc *rproc)
>  		if (res.a1)
>  			dev_info(dev, "Not in wfi, force stopped\n");
>  		break;
> -	case IMX_RPROC_SCU_API:
> -		ret = imx_sc_pm_cpu_start(priv->ipc_handle, priv->rsrc_id, false, priv->entry);
> -		break;
>  	default:
>  		return -EOPNOTSUPP;
>  	}
> @@ -907,14 +915,74 @@ static int imx_rproc_mmio_detect_mode(struct rproc *rproc)
>  	return 0;
>  }
>
> -static int imx_rproc_detect_mode(struct imx_rproc *priv)
> +static int imx_rproc_scu_api_detect_mode(struct rproc *rproc)
>  {
> -	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
> +	struct imx_rproc *priv = rproc->priv;
>  	struct device *dev = priv->dev;
> -	struct arm_smccc_res res;
>  	int ret;
>  	u8 pt;
>
> +	ret = imx_scu_get_handle(&priv->ipc_handle);
> +	if (ret)
> +		return ret;
> +	ret = of_property_read_u32(dev->of_node, "fsl,resource-id", &priv->rsrc_id);
> +	if (ret) {
> +		dev_err(dev, "No fsl,resource-id property\n");
> +		return ret;
> +	}
> +
> +	if (priv->rsrc_id == IMX_SC_R_M4_1_PID0)
> +		priv->core_index = 1;
> +	else
> +		priv->core_index = 0;
> +
> +	/*
> +	 * If Mcore resource is not owned by Acore partition, It is kicked by ROM,
> +	 * and Linux could only do IPC with Mcore and nothing else.
> +	 */
> +	if (imx_sc_rm_is_resource_owned(priv->ipc_handle, priv->rsrc_id)) {
> +		if (of_property_read_u32(dev->of_node, "fsl,entry-address", &priv->entry))
> +			return -EINVAL;
> +
> +		return imx_rproc_attach_pd(priv);
> +	}
> +
> +	priv->rproc->state = RPROC_DETACHED;
> +	priv->rproc->recovery_disabled = false;
> +	rproc_set_feature(priv->rproc, RPROC_FEAT_ATTACH_ON_RECOVERY);
> +
> +	/* Get partition id and enable irq in SCFW */
> +	ret = imx_sc_rm_get_resource_owner(priv->ipc_handle, priv->rsrc_id, &pt);
> +	if (ret) {
> +		dev_err(dev, "not able to get resource owner\n");
> +		return ret;
> +	}
> +
> +	priv->rproc_pt = pt;
> +	priv->rproc_nb.notifier_call = imx_rproc_partition_notify;
> +
> +	ret = imx_scu_irq_register_notifier(&priv->rproc_nb);
> +	if (ret) {
> +		dev_err(dev, "register scu notifier failed, %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = imx_scu_irq_group_enable(IMX_SC_IRQ_GROUP_REBOOTED, BIT(priv->rproc_pt),
> +				       true);
> +	if (ret) {
> +		imx_scu_irq_unregister_notifier(&priv->rproc_nb);
> +		dev_err(dev, "Enable irq failed, %d\n", ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int imx_rproc_detect_mode(struct imx_rproc *priv)
> +{
> +	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
> +	struct arm_smccc_res res;
> +
>  	if (dcfg->ops && dcfg->ops->detect_mode)
>  		return dcfg->ops->detect_mode(priv->rproc);
>
> @@ -927,61 +995,6 @@ static int imx_rproc_detect_mode(struct imx_rproc *priv)
>  		if (res.a0)
>  			priv->rproc->state = RPROC_DETACHED;
>  		return 0;
> -	case IMX_RPROC_SCU_API:
> -		ret = imx_scu_get_handle(&priv->ipc_handle);
> -		if (ret)
> -			return ret;
> -		ret = of_property_read_u32(dev->of_node, "fsl,resource-id", &priv->rsrc_id);
> -		if (ret) {
> -			dev_err(dev, "No fsl,resource-id property\n");
> -			return ret;
> -		}
> -
> -		if (priv->rsrc_id == IMX_SC_R_M4_1_PID0)
> -			priv->core_index = 1;
> -		else
> -			priv->core_index = 0;
> -
> -		/*
> -		 * If Mcore resource is not owned by Acore partition, It is kicked by ROM,
> -		 * and Linux could only do IPC with Mcore and nothing else.
> -		 */
> -		if (imx_sc_rm_is_resource_owned(priv->ipc_handle, priv->rsrc_id)) {
> -			if (of_property_read_u32(dev->of_node, "fsl,entry-address", &priv->entry))
> -				return -EINVAL;
> -
> -			return imx_rproc_attach_pd(priv);
> -		}
> -
> -		priv->rproc->state = RPROC_DETACHED;
> -		priv->rproc->recovery_disabled = false;
> -		rproc_set_feature(priv->rproc, RPROC_FEAT_ATTACH_ON_RECOVERY);
> -
> -		/* Get partition id and enable irq in SCFW */
> -		ret = imx_sc_rm_get_resource_owner(priv->ipc_handle, priv->rsrc_id, &pt);
> -		if (ret) {
> -			dev_err(dev, "not able to get resource owner\n");
> -			return ret;
> -		}
> -
> -		priv->rproc_pt = pt;
> -		priv->rproc_nb.notifier_call = imx_rproc_partition_notify;
> -
> -		ret = imx_scu_irq_register_notifier(&priv->rproc_nb);
> -		if (ret) {
> -			dev_err(dev, "register scu notifier failed, %d\n", ret);
> -			return ret;
> -		}
> -
> -		ret = imx_scu_irq_group_enable(IMX_SC_IRQ_GROUP_REBOOTED, BIT(priv->rproc_pt),
> -					       true);
> -		if (ret) {
> -			imx_scu_irq_unregister_notifier(&priv->rproc_nb);
> -			dev_err(dev, "Enable irq failed, %d\n", ret);
> -			return ret;
> -		}
> -
> -		return 0;
>  	default:
>  		break;
>  	}
> @@ -1163,6 +1176,12 @@ static const struct imx_rproc_plat_ops imx_rproc_ops_mmio = {
>  	.detect_mode	= imx_rproc_mmio_detect_mode,
>  };
>
> +static const struct imx_rproc_plat_ops imx_rproc_ops_scu_api = {
> +	.start		= imx_rproc_scu_api_start,
> +	.stop		= imx_rproc_scu_api_stop,
> +	.detect_mode	= imx_rproc_scu_api_detect_mode,
> +};
> +
>  static const struct imx_rproc_dcfg imx_rproc_cfg_imx8mn_mmio = {
>  	.src_reg	= IMX7D_SRC_SCR,
>  	.src_mask	= IMX7D_M4_RST_MASK,
> @@ -1197,12 +1216,14 @@ static const struct imx_rproc_dcfg imx_rproc_cfg_imx8qm = {
>  	.att            = imx_rproc_att_imx8qm,
>  	.att_size       = ARRAY_SIZE(imx_rproc_att_imx8qm),
>  	.method         = IMX_RPROC_SCU_API,
> +	.ops		= &imx_rproc_ops_scu_api,
>  };
>
>  static const struct imx_rproc_dcfg imx_rproc_cfg_imx8qxp = {
>  	.att		= imx_rproc_att_imx8qxp,
>  	.att_size	= ARRAY_SIZE(imx_rproc_att_imx8qxp),
>  	.method		= IMX_RPROC_SCU_API,
> +	.ops		= &imx_rproc_ops_scu_api,
>  };
>
>  static const struct imx_rproc_dcfg imx_rproc_cfg_imx8ulp = {
>
> --
> 2.37.1
>

Re: [PATCH 5/6] remoteproc: imx_rproc: Simplify IMX_RPROC_SMC switch case

[Frank Li]

On Mon, Sep 08, 2025 at 09:07:38PM +0800, Peng Fan wrote:
> Introduce imx_rproc_arm_smc_{start, stop, detect_mode}() helper functions
> for all i.MX variants using IMX_RPROC_SMC to manage remote processors.
>
> This allows the removal of the IMX_RPROC_SMC switch-case blocks from
> imx_rproc_start(), imx_rproc_stop(), and imx_rproc_detect_mode(), resulting
> in cleaner and more maintainable code.
>
> Since this is the last switch in imx_rproc_{start,stop}{}, remove
> the switch-case.
>
> No functional changes.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> ---
>  drivers/remoteproc/imx_rproc.c | 69 ++++++++++++++++++++++++++----------------
>  1 file changed, 43 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index ea34080970c6a5a9b035ef0d389014b8268660a9..5fa729f4286f6ac939357c32fef41d7d97e5f860 100644
> --- a/drivers/remoteproc/imx_rproc.c
> +++ b/drivers/remoteproc/imx_rproc.c
> @@ -285,6 +285,15 @@ static const struct imx_rproc_att imx_rproc_att_imx6sx[] = {
>  	{ 0x80000000, 0x80000000, 0x60000000, 0 },
>  };
>
> +static int imx_rproc_arm_smc_start(struct rproc *rproc)
> +{
> +	struct arm_smccc_res res;
> +
> +	arm_smccc_smc(IMX_SIP_RPROC, IMX_SIP_RPROC_START, 0, 0, 0, 0, 0, 0, &res);
> +
> +	return res.a0;
> +}
> +
>  static int imx_rproc_mmio_start(struct rproc *rproc)
>  {
>  	struct imx_rproc *priv = rproc->priv;
> @@ -308,7 +317,6 @@ static int imx_rproc_start(struct rproc *rproc)
>  	struct imx_rproc *priv = rproc->priv;
>  	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
>  	struct device *dev = priv->dev;
> -	struct arm_smccc_res res;
>  	int ret;
>
>  	ret = imx_rproc_xtr_mbox_init(rproc, true);
> @@ -320,14 +328,7 @@ static int imx_rproc_start(struct rproc *rproc)
>  		goto start_ret;
>  	}
>
> -	switch (dcfg->method) {
> -	case IMX_RPROC_SMC:
> -		arm_smccc_smc(IMX_SIP_RPROC, IMX_SIP_RPROC_START, 0, 0, 0, 0, 0, 0, &res);
> -		ret = res.a0;
> -		break;
> -	default:
> -		return -EOPNOTSUPP;
> -	}
> +	return -EOPNOTSUPP;
>
>  start_ret:
>  	if (ret)
> @@ -336,6 +337,18 @@ static int imx_rproc_start(struct rproc *rproc)
>  	return ret;
>  }
>
> +static int imx_rproc_arm_smc_stop(struct rproc *rproc)
> +{
> +	struct imx_rproc *priv = rproc->priv;
> +	struct arm_smccc_res res;
> +
> +	arm_smccc_smc(IMX_SIP_RPROC, IMX_SIP_RPROC_STOP, 0, 0, 0, 0, 0, 0, &res);
> +	if (res.a1)
> +		dev_info(priv->dev, "Not in wfi, force stopped\n");
> +
> +	return res.a0;
> +}
> +
>  static int imx_rproc_mmio_stop(struct rproc *rproc)
>  {
>  	struct imx_rproc *priv = rproc->priv;
> @@ -365,7 +378,6 @@ static int imx_rproc_stop(struct rproc *rproc)
>  	struct imx_rproc *priv = rproc->priv;
>  	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
>  	struct device *dev = priv->dev;
> -	struct arm_smccc_res res;
>  	int ret;
>
>  	if (dcfg->ops && dcfg->ops->stop) {
> @@ -373,16 +385,7 @@ static int imx_rproc_stop(struct rproc *rproc)
>  		goto stop_ret;
>  	}
>
> -	switch (dcfg->method) {
> -	case IMX_RPROC_SMC:
> -		arm_smccc_smc(IMX_SIP_RPROC, IMX_SIP_RPROC_STOP, 0, 0, 0, 0, 0, 0, &res);
> -		ret = res.a0;
> -		if (res.a1)
> -			dev_info(dev, "Not in wfi, force stopped\n");
> -		break;
> -	default:
> -		return -EOPNOTSUPP;
> -	}
> +	return -EOPNOTSUPP;
>
>  stop_ret:
>  	if (ret)
> @@ -867,6 +870,18 @@ static int imx_rproc_attach_pd(struct imx_rproc *priv)
>  	return 0;
>  }
>
> +static int imx_rproc_arm_smc_detect_mode(struct rproc *rproc)
> +{
> +	struct imx_rproc *priv = rproc->priv;
> +	struct arm_smccc_res res;
> +
> +	arm_smccc_smc(IMX_SIP_RPROC, IMX_SIP_RPROC_STARTED, 0, 0, 0, 0, 0, 0, &res);
> +	if (res.a0)
> +		priv->rproc->state = RPROC_DETACHED;
> +
> +	return 0;
> +}
> +
>  static int imx_rproc_mmio_detect_mode(struct rproc *rproc)
>  {
>  	const struct regmap_config config = { .name = "imx-rproc" };
> @@ -981,7 +996,6 @@ static int imx_rproc_scu_api_detect_mode(struct rproc *rproc)
>  static int imx_rproc_detect_mode(struct imx_rproc *priv)
>  {
>  	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
> -	struct arm_smccc_res res;
>
>  	if (dcfg->ops && dcfg->ops->detect_mode)
>  		return dcfg->ops->detect_mode(priv->rproc);
> @@ -990,11 +1004,6 @@ static int imx_rproc_detect_mode(struct imx_rproc *priv)
>  	case IMX_RPROC_NONE:
>  		priv->rproc->state = RPROC_DETACHED;
>  		return 0;
> -	case IMX_RPROC_SMC:
> -		arm_smccc_smc(IMX_SIP_RPROC, IMX_SIP_RPROC_STARTED, 0, 0, 0, 0, 0, 0, &res);
> -		if (res.a0)
> -			priv->rproc->state = RPROC_DETACHED;
> -		return 0;
>  	default:
>  		break;
>  	}
> @@ -1170,6 +1179,12 @@ static void imx_rproc_remove(struct platform_device *pdev)
>  	destroy_workqueue(priv->workqueue);
>  }
>
> +static const struct imx_rproc_plat_ops imx_rproc_ops_arm_smc = {
> +	.start		= imx_rproc_arm_smc_start,
> +	.stop		= imx_rproc_arm_smc_stop,
> +	.detect_mode	= imx_rproc_arm_smc_detect_mode,
> +};
> +
>  static const struct imx_rproc_plat_ops imx_rproc_ops_mmio = {
>  	.start		= imx_rproc_mmio_start,
>  	.stop		= imx_rproc_mmio_stop,
> @@ -1199,6 +1214,7 @@ static const struct imx_rproc_dcfg imx_rproc_cfg_imx8mn = {
>  	.att		= imx_rproc_att_imx8mn,
>  	.att_size	= ARRAY_SIZE(imx_rproc_att_imx8mn),
>  	.method		= IMX_RPROC_SMC,
> +	.ops		= &imx_rproc_ops_arm_smc,
>  };
>
>  static const struct imx_rproc_dcfg imx_rproc_cfg_imx8mq = {
> @@ -1265,6 +1281,7 @@ static const struct imx_rproc_dcfg imx_rproc_cfg_imx93 = {
>  	.att		= imx_rproc_att_imx93,
>  	.att_size	= ARRAY_SIZE(imx_rproc_att_imx93),
>  	.method		= IMX_RPROC_SMC,
> +	.ops		= &imx_rproc_ops_arm_smc,
>  };
>
>  static const struct of_device_id imx_rproc_of_match[] = {
>
> --
> 2.37.1
>

Re: [PATCH 6/6] remoteproc: imx_rproc: Clean up after ops introduction

[Frank Li]

On Mon, Sep 08, 2025 at 09:07:39PM +0800, Peng Fan wrote:
> With the switch-case in imx_rproc_{start,stop}{} removed, simplify
> the code logic by removing 'goto'. The last switch-case in
> imx_rproc_detect_mode() are no longer needed and can be removed.
>
> This cleanup improves code readability and aligns with the new ops-based
> design.
>
> No functional changes.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/remoteproc/imx_rproc.c | 25 +++++++------------------
>  1 file changed, 7 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index 5fa729f4286f6ac939357c32fef41d7d97e5f860..ed8395b097d07d02e5323550ff497819f9b95ad6 100644
> --- a/drivers/remoteproc/imx_rproc.c
> +++ b/drivers/remoteproc/imx_rproc.c
> @@ -323,14 +323,11 @@ static int imx_rproc_start(struct rproc *rproc)
>  	if (ret)
>  		return ret;
>
> -	if (dcfg->ops && dcfg->ops->start) {
> +	if (dcfg->ops && dcfg->ops->start)
>  		ret = dcfg->ops->start(rproc);
> -		goto start_ret;
> -	}
> -
> -	return -EOPNOTSUPP;
> +	else
> +		return -EOPNOTSUPP;
>
> -start_ret:
>  	if (ret)
>  		dev_err(dev, "Failed to enable remote core!\n");
>
> @@ -380,14 +377,11 @@ static int imx_rproc_stop(struct rproc *rproc)
>  	struct device *dev = priv->dev;
>  	int ret;
>
> -	if (dcfg->ops && dcfg->ops->stop) {
> +	if (dcfg->ops && dcfg->ops->stop)
>  		ret = dcfg->ops->stop(rproc);
> -		goto stop_ret;
> -	}
> -
> -	return -EOPNOTSUPP;
> +	else
> +		return -EOPNOTSUPP;
>
> -stop_ret:
>  	if (ret)
>  		dev_err(dev, "Failed to stop remote core\n");
>  	else
> @@ -1000,13 +994,8 @@ static int imx_rproc_detect_mode(struct imx_rproc *priv)
>  	if (dcfg->ops && dcfg->ops->detect_mode)
>  		return dcfg->ops->detect_mode(priv->rproc);
>
> -	switch (dcfg->method) {
> -	case IMX_RPROC_NONE:
> +	if (dcfg->method == IMX_RPROC_NONE)

Is it possible remove "method"? Can you check dcfg->ops->dectect_mode NULL
here?

Frank Li
>  		priv->rproc->state = RPROC_DETACHED;
> -		return 0;
> -	default:
> -		break;
> -	}
>
>  	return 0;
>  }
>
> --
> 2.37.1
>

Re: [PATCH 6/6] remoteproc: imx_rproc: Clean up after ops introduction

[Peng Fan]

On Mon, Sep 08, 2025 at 11:03:46AM -0400, Frank Li wrote:
>On Mon, Sep 08, 2025 at 09:07:39PM +0800, Peng Fan wrote:
[...]
>>  	if (dcfg->ops && dcfg->ops->detect_mode)
>>  		return dcfg->ops->detect_mode(priv->rproc);
>>
>> -	switch (dcfg->method) {
>> -	case IMX_RPROC_NONE:
>> +	if (dcfg->method == IMX_RPROC_NONE)
>
>Is it possible remove "method"? Can you check dcfg->ops->dectect_mode NULL
>here?

Yeah. we could do as below.
	if (dcfg->ops && dcfg->ops->detect_mode)
		return dcfg->ops->detect_mode(priv->rproc);
	else
		priv->rproc->state = RPROC_DETACHED;

Thanks,
Peng

>
>Frank Li
>>  		priv->rproc->state = RPROC_DETACHED;
>> -		return 0;
>> -	default:
>> -		break;
>> -	}
>>
>>  	return 0;
>>  }
>>
>> --
>> 2.37.1
>>

Re: [PATCH 0/6] remoteproc: imx_proc: Simplify driver by removing the switch-case

[Peng Fan]

On Mon, Sep 08, 2025 at 09:07:33PM +0800, Peng Fan wrote:
>This patchset serves as a preparing patchset for i.MX95 support.
>
>The current code logic is complicated, with mix the usage of switch-case
>and if-else.
>
>To simplify the code logic:
> Introduce struct imx_rproc_plat_ops to wrap platform start,stop,detect_mode.
> Each imx_rproc_dcfg data structure is assigned a ops pointer.
> The common imx_rproc_{start,stop}() directly invokes the plat ops, no
> need the switch-case.
> mmio/smc/scu_api ops are included.
> No functional changes.
>
>Thanks to Daniel and Frank for the help.
>
>Test on i.MX8MM for MMIO ops, i.MX8MP for SMC ops. For SCU-API ops,
>I currently not have a setup at hand, I will give a setup and test
>later, but everything should be fine, no functional changes.

Update: tested on i.MX8QM-MEK with SCU-API ops.

Thanks,
Peng

Re: [PATCH 0/6] remoteproc: imx_proc: Simplify driver by removing the switch-case

[Daniel Baluta]

On Mon, Sep 8, 2025 at 4:08 PM Peng Fan <peng.fan@nxp.com> wrote:
>
> This patchset serves as a preparing patchset for i.MX95 support.
>
> The current code logic is complicated, with mix the usage of switch-case
> and if-else.
>
> To simplify the code logic:
>  Introduce struct imx_rproc_plat_ops to wrap platform start,stop,detect_mode.
>  Each imx_rproc_dcfg data structure is assigned a ops pointer.
>  The common imx_rproc_{start,stop}() directly invokes the plat ops, no
>  need the switch-case.
>  mmio/smc/scu_api ops are included.
>  No functional changes.
>
> Thanks to Daniel and Frank for the help.
>
> Test on i.MX8MM for MMIO ops, i.MX8MP for SMC ops. For SCU-API ops,
> I currently not have a setup at hand, I will give a setup and test
> later, but everything should be fine, no functional changes.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>


Thanks a lot Peng. Entire series looks good to me.

Reviewed-by: Daniel Baluta <daniel.baluta@nxp.com>

Re: [PATCH 6/6] remoteproc: imx_rproc: Clean up after ops introduction

[Mathieu Poirier]

On Mon, Sep 08, 2025 at 09:07:39PM +0800, Peng Fan wrote:
> With the switch-case in imx_rproc_{start,stop}{} removed, simplify
> the code logic by removing 'goto'. The last switch-case in
> imx_rproc_detect_mode() are no longer needed and can be removed.
> 
> This cleanup improves code readability and aligns with the new ops-based
> design.
> 
> No functional changes.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/remoteproc/imx_rproc.c | 25 +++++++------------------
>  1 file changed, 7 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index 5fa729f4286f6ac939357c32fef41d7d97e5f860..ed8395b097d07d02e5323550ff497819f9b95ad6 100644
> --- a/drivers/remoteproc/imx_rproc.c
> +++ b/drivers/remoteproc/imx_rproc.c
> @@ -323,14 +323,11 @@ static int imx_rproc_start(struct rproc *rproc)
>  	if (ret)
>  		return ret;
>  
> -	if (dcfg->ops && dcfg->ops->start) {
> +	if (dcfg->ops && dcfg->ops->start)
>  		ret = dcfg->ops->start(rproc);
> -		goto start_ret;
> -	}
> -
> -	return -EOPNOTSUPP;
> +	else
> +		return -EOPNOTSUPP;


Here I would simply refactor this to:

        if (!dcfg->ops || !dcfg->ops_start)
                return -EONOTSUPP;

Otherwise this patchset is going in the right direction.

Thanks,
Mathieu

>  
> -start_ret:
>  	if (ret)
>  		dev_err(dev, "Failed to enable remote core!\n");
>  
> @@ -380,14 +377,11 @@ static int imx_rproc_stop(struct rproc *rproc)
>  	struct device *dev = priv->dev;
>  	int ret;
>  
> -	if (dcfg->ops && dcfg->ops->stop) {
> +	if (dcfg->ops && dcfg->ops->stop)
>  		ret = dcfg->ops->stop(rproc);
> -		goto stop_ret;
> -	}
> -
> -	return -EOPNOTSUPP;
> +	else
> +		return -EOPNOTSUPP;
>  
> -stop_ret:
>  	if (ret)
>  		dev_err(dev, "Failed to stop remote core\n");
>  	else
> @@ -1000,13 +994,8 @@ static int imx_rproc_detect_mode(struct imx_rproc *priv)
>  	if (dcfg->ops && dcfg->ops->detect_mode)
>  		return dcfg->ops->detect_mode(priv->rproc);
>  
> -	switch (dcfg->method) {
> -	case IMX_RPROC_NONE:
> +	if (dcfg->method == IMX_RPROC_NONE)
>  		priv->rproc->state = RPROC_DETACHED;
> -		return 0;
> -	default:
> -		break;
> -	}
>  
>  	return 0;
>  }
> 
> -- 
> 2.37.1
>