[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aMw1eACI1VhV+CzP@lizhi-Precision-Tower-5810>
Date: Thu, 18 Sep 2025 12:38:16 -0400
From: Frank Li <Frank.li@....com>
To: Peng Fan <peng.fan@....com>
Cc: Bjorn Andersson <andersson@...nel.org>,
Mathieu Poirier <mathieu.poirier@...aro.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>,
linux-remoteproc@...r.kernel.org, imx@...ts.linux.dev,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/5] remoteproc: imx_rproc: Simplify clock enable logic
using dcfg flags
On Thu, Sep 18, 2025 at 07:51:16PM +0800, Peng Fan wrote:
> Simplify the clock enable logic by removing the dedicated
> imx_rproc_clk_enable() function and integrate the clock handling directly
> into the probe function to simplify the code.
>
> Add a new IMX_RPROC_NEED_CLKS flag in dcfg to indicate whether clock
> management is required for a given SoC. Update probe logic to conditionally
> enable clocks based on the new flag.
>
> Set the flag for applicable SoCs (e.g., i.MX7D, i.MX8MQ, i.MX93, etc.).
>
> No functional changes.
>
> Signed-off-by: Peng Fan <peng.fan@....com>
Reviewed-by: Frank Li <Frank.Li@....com>
> ---
> drivers/remoteproc/imx_rproc.c | 37 ++++++++++++-------------------------
> drivers/remoteproc/imx_rproc.h | 1 +
> 2 files changed, 13 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index 694fbbb2f34061de22a3a815f8a6114159585f9e..71617b20f9d0ae698e7f655aae22e8895434f32c 100644
> --- a/drivers/remoteproc/imx_rproc.c
> +++ b/drivers/remoteproc/imx_rproc.c
> @@ -1002,28 +1002,6 @@ static int imx_rproc_detect_mode(struct imx_rproc *priv)
> return dcfg->ops->detect_mode(priv->rproc);
> }
>
> -static int imx_rproc_clk_enable(struct imx_rproc *priv)
> -{
> - const struct imx_rproc_dcfg *dcfg = priv->dcfg;
> - struct device *dev = priv->dev;
> -
> - /* Remote core is not under control of Linux or it is managed by SCU API */
> - if (dcfg->method == IMX_RPROC_NONE || dcfg->method == IMX_RPROC_SCU_API)
> - return 0;
> -
> - /*
> - * clk for M4 block including memory. Should be
> - * enabled before .start for FW transfer.
> - */
> - priv->clk = devm_clk_get_enabled(dev, NULL);
> - if (IS_ERR(priv->clk)) {
> - dev_err(dev, "Failed to enable clock\n");
> - return PTR_ERR(priv->clk);
> - }
> -
> - return 0;
> -}
> -
> static int imx_rproc_sys_off_handler(struct sys_off_data *data)
> {
> struct rproc *rproc = data->cb_data;
> @@ -1108,9 +1086,12 @@ static int imx_rproc_probe(struct platform_device *pdev)
> if (ret)
> return dev_err_probe(dev, ret, "failed on detect mode\n");
>
> - ret = imx_rproc_clk_enable(priv);
> - if (ret)
> - return dev_err_probe(dev, ret, "failed to enable clks\n");
> + /* Remote core is under control of Linux or clock is not managed by firmware */
> + if (dcfg->flags & IMX_RPROC_NEED_CLKS) {
> + priv->clk = devm_clk_get_enabled(dev, NULL);
> + if (IS_ERR(priv->clk))
> + return dev_err_probe(dev, PTR_ERR(priv->clk), "Failed to enable clock\n");
> + }
>
> if (rproc->state != RPROC_DETACHED)
> rproc->auto_boot = of_property_read_bool(np, "fsl,auto-boot");
> @@ -1185,6 +1166,7 @@ static const struct imx_rproc_dcfg imx_rproc_cfg_imx8mn_mmio = {
> .att_size = ARRAY_SIZE(imx_rproc_att_imx8mn),
> .method = IMX_RPROC_MMIO,
> .ops = &imx_rproc_ops_mmio,
> + .flags = IMX_RPROC_NEED_CLKS,
> };
>
> static const struct imx_rproc_dcfg imx_rproc_cfg_imx8mn = {
> @@ -1192,6 +1174,7 @@ static const struct imx_rproc_dcfg imx_rproc_cfg_imx8mn = {
> .att_size = ARRAY_SIZE(imx_rproc_att_imx8mn),
> .method = IMX_RPROC_SMC,
> .ops = &imx_rproc_ops_arm_smc,
> + .flags = IMX_RPROC_NEED_CLKS,
> };
>
> static const struct imx_rproc_dcfg imx_rproc_cfg_imx8mq = {
> @@ -1203,6 +1186,7 @@ static const struct imx_rproc_dcfg imx_rproc_cfg_imx8mq = {
> .att_size = ARRAY_SIZE(imx_rproc_att_imx8mq),
> .method = IMX_RPROC_MMIO,
> .ops = &imx_rproc_ops_mmio,
> + .flags = IMX_RPROC_NEED_CLKS,
> };
>
> static const struct imx_rproc_dcfg imx_rproc_cfg_imx8qm = {
> @@ -1241,6 +1225,7 @@ static const struct imx_rproc_dcfg imx_rproc_cfg_imx7d = {
> .att_size = ARRAY_SIZE(imx_rproc_att_imx7d),
> .method = IMX_RPROC_MMIO,
> .ops = &imx_rproc_ops_mmio,
> + .flags = IMX_RPROC_NEED_CLKS,
> };
>
> static const struct imx_rproc_dcfg imx_rproc_cfg_imx6sx = {
> @@ -1252,6 +1237,7 @@ static const struct imx_rproc_dcfg imx_rproc_cfg_imx6sx = {
> .att_size = ARRAY_SIZE(imx_rproc_att_imx6sx),
> .method = IMX_RPROC_MMIO,
> .ops = &imx_rproc_ops_mmio,
> + .flags = IMX_RPROC_NEED_CLKS,
> };
>
> static const struct imx_rproc_dcfg imx_rproc_cfg_imx93 = {
> @@ -1259,6 +1245,7 @@ static const struct imx_rproc_dcfg imx_rproc_cfg_imx93 = {
> .att_size = ARRAY_SIZE(imx_rproc_att_imx93),
> .method = IMX_RPROC_SMC,
> .ops = &imx_rproc_ops_arm_smc,
> + .flags = IMX_RPROC_NEED_CLKS,
> };
>
> static const struct of_device_id imx_rproc_of_match[] = {
> diff --git a/drivers/remoteproc/imx_rproc.h b/drivers/remoteproc/imx_rproc.h
> index 3a9adaaf048b396102feeb45488cd2ff125a807a..a9cba623560c85ea37e47401c392c06dada500aa 100644
> --- a/drivers/remoteproc/imx_rproc.h
> +++ b/drivers/remoteproc/imx_rproc.h
> @@ -30,6 +30,7 @@ enum imx_rproc_method {
>
> /* dcfg flags */
> #define IMX_RPROC_NEED_SYSTEM_OFF BIT(0)
> +#define IMX_RPROC_NEED_CLKS BIT(1)
>
> struct imx_rproc_plat_ops {
> int (*start)(struct rproc *rproc);
>
> --
> 2.37.1
>
Powered by blists - more mailing lists