Hi Daniel,
> Subject: Re: [PATCH 1/5] remoteproc: imx_rproc: Simplify clock enable
> logic using dcfg flags
> > + /* Remote core is under control of Linux or clock is not
> > + managed by firmware */
>
> I see that you negate the comment from imx_rproc_clk_enable but
> with the negation OR becomes AND.
>
> So, the comment should be:
>
> /* Handle clocks when remote core is under control of Linux AND the
> clocks are not managed by remote side FW */
I thought this flag is clear that clk should be managed by driver.
I will update the comment.
>
> Also, do we really need this flag?
> Shouldn't we just make a decision based on the fact that clk is in the
> device tree or not?
From hardware perspective, there is always clk for the remote cores.
So DT describe hardware, a clk property should be there.
But NXP system firmware manages the CLK automatically, no need
driver to do that. So this flag is required here.
Thanks,
Peng.
>
> > + 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");
