RE: [PATCH 1/5] remoteproc: imx_rproc: Simplify clock enable logic using dcfg flags

[Peng Fan]

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");