Messages in this thread | | | Date | Wed, 10 Sep 2014 10:42:04 -0700 | From | Nicolin Chen <> | Subject | Re: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module |
| |
On Wed, Sep 10, 2014 at 04:12:53PM +0800, Shengjiu Wang wrote: > > Then we can get a patch like: > > open() { > > + clk_prepare_enable(); > > .... > > } > > > > close() { > > .... > > + clk_disable_unprepare() > > }
> what is the open() and close()? do you mean the fsl_ssi_startup() > and fsl_ssi_shutdown()?
Yea.
> > probe() { > > clk_get(); > > clk_prepare_enable(); > > .... > > if (xxx) > > - goto err_xx; > > + return ret; > > .... > > + clk_disable_unprepare(); > > return 0; > > -err_xx: > > - clk_disable_unprepare() > > }
> If this probe() is fsl_ssi_imx_probe(), I think no need to add > clk_prepare_enable() or clk_disable_unprepare(), seems there is no > registers accessing in this probe.
This is trying to be safe, especially for such a driver being used by multiple platforms. You can omit this as long as the patch can pass the test on old imx, PowerPC, and AC97 platforms.
>
And another risk just came to my mind is that there would be a possibility that a machine driver would call set_dai_fmt() early, after SSI's probe() and before SSI's startup(), if the machine driver contains dai_fmt assignment in its probe(). Then, without regmap_mmio_clk(), it'll be tough for us over here because we may also need to add clock enable/disable for set_dai_fmt/set_sysclk(), even if there might be still tiny risk that we missed something.
Then there could be a selfish approach to circumvent it is to use regmap_mmio_clk() with "ipg" at the beginning and call regmap_mmio() without "ipg" if getting a failed return value from regmap_mmio_clk, and meanwhile to keep the clock always enabled for the regmap_mmio() case just like what the current driver is doing. This may result those non-ipg-clk platforms can't benefit from this refinement unless they update their DT bindings -- use "ipg" for core clock This might be the safest and simplest way for us, I'm not sure everyone would be comfortable with this idea though.
Best regards, Nicolin
| |