Re: [PATCH v2 2/3] clk: ti: check clock type before doing autoidle ops

From: Andreas Kemnade
Date: Tue Dec 04 2018 - 01:17:40 EST


On Mon, 3 Dec 2018 07:39:10 -0800
Tony Lindgren <tony@xxxxxxxxxxx> wrote:

> * Stephen Boyd <sboyd@xxxxxxxxxx> [181130 23:52]:
> > Quoting Tony Lindgren (2018-11-30 07:37:29)
> > > Hi,
> > >
> > > * Tero Kristo <t-kristo@xxxxxx> [181130 09:21]:
> > > > On 30/11/2018 09:57, Stephen Boyd wrote:
> > > > > No that is not preferred. Can the omap2_clk_deny_idle() function be
> > > > > integrated closer into the clk framework in some way that allows it to
> > > > > be part of the clk_ops structure? And then have that take a clk_hw
> > > > > structure instead of a struct clk? I haven't looked at this in any
> > > > > detail whatsoever so I may be way off right now.
> > > >
> > > > It could be added under the main clk_ops struct, however this would
> > > > introduce two new func pointers to it which are not used by anything else
> > > > but OMAP. Are you aware of any other platforms requiring similar feature?
> > >
> > > From consumer usage point of view, I'm still wondering about
> > > the relationship of clk_deny_idle() and clkdm_deny_idle().
> > >
> > > It seems that we need to allow reset control drivers call
> > > clk_deny_idle() for the duration of reset. And it seems the
> > > clk_deny_idle() should propagate to also up to the related
> > > clock domain driver to do clkdm_deny_idle().
> > >
> > > So maybe clk_deny_idle() is could just be something like:
> > >
> > > dev = clk_get_device(clk);
> > > ...
> > > error = pm_runtime_get(dev);
> > > ...
> > > pm_runtime_put(dev);
> > > ...
> > >
> > > And that way it would just propagate to the parent clock
> > > domain driver and the clock framework does not need to know
> > > about clockdomains. A clockdomain could be just a genpd
> > > domain.
> > >
> > > Or do you guys have better ideas?
> > >
> >
> > Wouldn't the device link in clk framework patches do this for you if we
> > had the RUNTIME_PM flag passed in. If this is about keeping the clock
> > controller active when a consumer device is using it then I think it may
> > work.
>
> The consumer device stays active just fine with PM runtime
> calls. So yes, the problem is keeping a clock controller forced
> active for the period of consumer device reset. Other than
> that typically autoidle can be just kept enabled.
>
Are we still talking about the same problem? Maybe I am losing track
here. Just to make sure.
The patch series was about disabling autoidle for devices which cannot
work with it during normal operation. Not during reset or something
like that.
Or is the keep-clock-active-during-reset just a requirement for bigger
restructuring ideas?

Regards,
Andreas

Attachment: pgpkF6negcQk8.pgp
Description: OpenPGP digital signature