| Message ID | 20220111033333.403448-8-peng.fan@oss.nxp.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [V2] remoteproc: imx_rproc: use imx specific hook for find_loaded_rsc_table | expand |
On Tue, Jan 11, 2022 at 11:33:29AM +0800, Peng Fan (OSS) wrote: > From: Peng Fan <peng.fan@nxp.com> > > To i.MX8QXP/QM/ULP and i.MX7ULP, Mcore maybe out of control of Linux. > And in such case, no need clk, so make clk optional with has_clk. > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > --- > drivers/remoteproc/imx_rproc.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c > index 7b2578177ea8..0e99a3ca6fbc 100644 > --- a/drivers/remoteproc/imx_rproc.c > +++ b/drivers/remoteproc/imx_rproc.c > @@ -89,6 +89,7 @@ struct imx_rproc { > struct work_struct rproc_work; > struct workqueue_struct *workqueue; > void __iomem *rsc_table; > + bool has_clk; I am usually weary of bloating structures with flags. I suggest achieving the same functionality with a macro that compares priv->dcfg with the right imx_rproc_dcfg structure. > }; > > static const struct imx_rproc_att imx_rproc_att_imx8mn[] = { > @@ -724,6 +725,9 @@ static int imx_rproc_clk_enable(struct imx_rproc *priv) > if (dcfg->method == IMX_RPROC_NONE) > return 0; > > + if (!priv->has_clk) > + return 0; > + > priv->clk = devm_clk_get(dev, NULL); > if (IS_ERR(priv->clk)) { > dev_err(dev, "Failed to get clock\n"); > @@ -768,6 +772,7 @@ static int imx_rproc_probe(struct platform_device *pdev) > priv->rproc = rproc; > priv->dcfg = dcfg; > priv->dev = dev; > + priv->has_clk = true; > > dev_set_drvdata(dev, rproc); > priv->workqueue = create_workqueue(dev_name(dev)); > -- > 2.25.1 >
> Subject: Re: [PATCH 5/9] remoteproc: imx_rproc: make clk optional > > On Tue, Jan 11, 2022 at 11:33:29AM +0800, Peng Fan (OSS) wrote: > > From: Peng Fan <peng.fan@nxp.com> > > > > To i.MX8QXP/QM/ULP and i.MX7ULP, Mcore maybe out of control of Linux. > > And in such case, no need clk, so make clk optional with has_clk. > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > --- > > drivers/remoteproc/imx_rproc.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/drivers/remoteproc/imx_rproc.c > > b/drivers/remoteproc/imx_rproc.c index 7b2578177ea8..0e99a3ca6fbc > > 100644 > > --- a/drivers/remoteproc/imx_rproc.c > > +++ b/drivers/remoteproc/imx_rproc.c > > @@ -89,6 +89,7 @@ struct imx_rproc { > > struct work_struct rproc_work; > > struct workqueue_struct *workqueue; > > void __iomem *rsc_table; > > + bool has_clk; > > I am usually weary of bloating structures with flags. I suggest achieving the > same functionality with a macro that compares priv->dcfg with the right > imx_rproc_dcfg structure. priv->dcfg is some kind fixed settings, however has_clk could be runtime changed, because i.MX platform M-core support multiple booting method and it could work w/o clk handled by Linux depending on some pre-configuration such as moving M-core in an separate hardware partition. Thanks, Peng. > > > }; > > > > static const struct imx_rproc_att imx_rproc_att_imx8mn[] = { @@ > > -724,6 +725,9 @@ static int imx_rproc_clk_enable(struct imx_rproc *priv) > > if (dcfg->method == IMX_RPROC_NONE) > > return 0; > > > > + if (!priv->has_clk) > > + return 0; > > + > > priv->clk = devm_clk_get(dev, NULL); > > if (IS_ERR(priv->clk)) { > > dev_err(dev, "Failed to get clock\n"); @@ -768,6 +772,7 @@ static > > int imx_rproc_probe(struct platform_device *pdev) > > priv->rproc = rproc; > > priv->dcfg = dcfg; > > priv->dev = dev; > > + priv->has_clk = true; > > > > dev_set_drvdata(dev, rproc); > > priv->workqueue = create_workqueue(dev_name(dev)); > > -- > > 2.25.1 > >
On Wed, Jan 19, 2022 at 02:25:48AM +0000, Peng Fan wrote: > > Subject: Re: [PATCH 5/9] remoteproc: imx_rproc: make clk optional > > > > On Tue, Jan 11, 2022 at 11:33:29AM +0800, Peng Fan (OSS) wrote: > > > From: Peng Fan <peng.fan@nxp.com> > > > > > > To i.MX8QXP/QM/ULP and i.MX7ULP, Mcore maybe out of control of Linux. > > > And in such case, no need clk, so make clk optional with has_clk. > > > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > > --- > > > drivers/remoteproc/imx_rproc.c | 5 +++++ > > > 1 file changed, 5 insertions(+) > > > > > > diff --git a/drivers/remoteproc/imx_rproc.c > > > b/drivers/remoteproc/imx_rproc.c index 7b2578177ea8..0e99a3ca6fbc > > > 100644 > > > --- a/drivers/remoteproc/imx_rproc.c > > > +++ b/drivers/remoteproc/imx_rproc.c > > > @@ -89,6 +89,7 @@ struct imx_rproc { > > > struct work_struct rproc_work; > > > struct workqueue_struct *workqueue; > > > void __iomem *rsc_table; > > > + bool has_clk; > > > > I am usually weary of bloating structures with flags. I suggest achieving the > > same functionality with a macro that compares priv->dcfg with the right > > imx_rproc_dcfg structure. > > priv->dcfg is some kind fixed settings, however has_clk could be runtime changed, > because i.MX platform M-core support multiple booting method and it > could work w/o clk handled by Linux depending on some pre-configuration > such as moving M-core in an separate hardware partition. Unless there is an FPGA in the mix, clocks and power domains should not change. Either clocks are handled by the remote processor or the application processor, regardless of the mode (attached or detached) the platform is booting into. > > Thanks, > Peng. > > > > > > }; > > > > > > static const struct imx_rproc_att imx_rproc_att_imx8mn[] = { @@ > > > -724,6 +725,9 @@ static int imx_rproc_clk_enable(struct imx_rproc *priv) > > > if (dcfg->method == IMX_RPROC_NONE) > > > return 0; > > > > > > + if (!priv->has_clk) > > > + return 0; > > > + > > > priv->clk = devm_clk_get(dev, NULL); > > > if (IS_ERR(priv->clk)) { > > > dev_err(dev, "Failed to get clock\n"); @@ -768,6 +772,7 @@ static > > > int imx_rproc_probe(struct platform_device *pdev) > > > priv->rproc = rproc; > > > priv->dcfg = dcfg; > > > priv->dev = dev; > > > + priv->has_clk = true; > > > > > > dev_set_drvdata(dev, rproc); > > > priv->workqueue = create_workqueue(dev_name(dev)); > > > -- > > > 2.25.1 > > >
diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c index 7b2578177ea8..0e99a3ca6fbc 100644 --- a/drivers/remoteproc/imx_rproc.c +++ b/drivers/remoteproc/imx_rproc.c @@ -89,6 +89,7 @@ struct imx_rproc { struct work_struct rproc_work; struct workqueue_struct *workqueue; void __iomem *rsc_table; + bool has_clk; }; static const struct imx_rproc_att imx_rproc_att_imx8mn[] = { @@ -724,6 +725,9 @@ static int imx_rproc_clk_enable(struct imx_rproc *priv) if (dcfg->method == IMX_RPROC_NONE) return 0; + if (!priv->has_clk) + return 0; + priv->clk = devm_clk_get(dev, NULL); if (IS_ERR(priv->clk)) { dev_err(dev, "Failed to get clock\n"); @@ -768,6 +772,7 @@ static int imx_rproc_probe(struct platform_device *pdev) priv->rproc = rproc; priv->dcfg = dcfg; priv->dev = dev; + priv->has_clk = true; dev_set_drvdata(dev, rproc); priv->workqueue = create_workqueue(dev_name(dev));