Re: [PATCH 5/9] remoteproc: imx_rproc: make clk optional

From: Mathieu Poirier <mathieu.poirier@linaro.org>
To: Peng Fan <peng.fan@nxp.com>
Cc: "Peng Fan (OSS)" <peng.fan@oss.nxp.com>,
	"bjorn.andersson@linaro.org" <bjorn.andersson@linaro.org>,
	"shawnguo@kernel.org" <shawnguo@kernel.org>,
	"s.hauer@pengutronix.de" <s.hauer@pengutronix.de>,
	"kernel@pengutronix.de" <kernel@pengutronix.de>,
	"festevam@gmail.com" <festevam@gmail.com>,
	dl-linux-imx <linux-imx@nxp.com>,
	"linux-remoteproc@vger.kernel.org"
	<linux-remoteproc@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 5/9] remoteproc: imx_rproc: make clk optional
Date: Wed, 19 Jan 2022 10:49:47 -0700	[thread overview]
Message-ID: <20220119174947.GA1282028@p14s> (raw)
In-Reply-To: <DU0PR04MB9417B0AC80F41D425F43277C88599@DU0PR04MB9417.eurprd04.prod.outlook.com>

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
> > >