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

[Tero Kristo]

On 12/01/2019 00:49, Stephen Boyd wrote:
> Quoting Tero Kristo (2019-01-03 23:28:58)
> > On 04/01/2019 01:39, Stephen Boyd wrote:
> > > Quoting Andreas Kemnade (2018-12-31 00:30:21)
> > > > On Mon, 31 Dec 2018 09:23:01 +0200
> > > > Tero Kristo <t-kristo@xxxxxx> wrote:
> > > >
> > > > > On 28/12/2018 22:02, Tony Lindgren wrote:
> > > > > > * Andreas Kemnade <andreas@xxxxxxxxxxxx> [181227 20:13]:
> > > > > > > Hi,
> > > > > > >
> > > > > > > On Tue, 4 Dec 2018 08:45:57 -0800
> > > > > > > Tony Lindgren <tony@xxxxxxxxxxx> wrote:
> > > > > > >    
> > > > > > > > * Andreas Kemnade <andreas@xxxxxxxxxxxx> [181204 06:17]:
> > > > > > > > > On Mon, 3 Dec 2018 07:39:10 -0800
> > > > > > > > > Tony Lindgren <tony@xxxxxxxxxxx> wrote:
> > > > > > > > > > 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?
> > > > > > > >
> > > > > > > > Yeah there are two issues: The fix needed for the issue you brought up,
> > > > > > > > and also how to let a reset driver to block autoidle for reset.
> > > > > > > >    
> > > > > > > Hmm, is this set now waiting for the famous "somebody" fixing all
> > > > > > > the stuff?
> > > > > >
> > > > > > Well I think we're still waiting on Tero to comment on this.
> > > > >
> > > > > The only item requiring immediate fixing is the point Stephen made out,
> > > > > removing the usage of CLK_IS_BASIC from this patch.
> > > > >
> > > > > Afaics, the reset related concerns Tony has can be handled later.
> > > > hmm, and there we need Stephen's opinion about having the allow/deny
> > > > autoidle functions in the main clk_ops struct.
> > >
> > > I have unanswered questions on the list for this thread[1].
> >
> > The reset portion we can't answer with the current knowledge I fear, we
> > need to prototype this a bit first and see which way to go.
> >
> > > I'm not sure
> > > what allow/deny autoidle functions mean to clk drivers. It looks like an
> > > OMAP specific addition to the clk_ops struct, which sounds wrong to put
> > > it plainly.
> >
> > Yeah, I don't think other SoCs implement the same functionality, at
> > least not in the same way. The autoidle bits are available in
> > omap2/omap3 only, where they control the HW autoidle functionality of
> > these clocks. If the bit is enabled, the HW can autonomously disable the
> > clock once it is not needed anymore by HW.
>
> Some qcom chips have automatic clock gating (basically hw clk gating)
> but they don't really need to involve that with the reset asserting or
> deasserting anymore. It used to be that they had to turn off the
> automatic mode, assert the reset, deassert the reset, and then reenable
> the automatic mode. So there is some precedence for this. But again, I
> think that the reset controller and the clk controller are the same
> device in both vendor instances so in theory the driver can be one
> driver for both clk and reset and do the proper things on the backend.
> So just use reset controller framework and register that from the clk
> controller driver?
>
> > > Hopefully it can be done outside of the clk framework by
> > > having the provider driver know more things about all the frameworks
> > > it's hooking into.
> >
> > This is how it has been done so far, however Andreas wants to expand the
> > functionality a bit where it breaks... unless we can use the
> > CLK_IS_BASIC flag to detect if we accessing an OMAP specific clock or
> > not. If we are passing in a clk pointer from a consumer level API, I
> > don't know if there is any other way to go with this if we can't modify
> > the generic clk_ops struct.
> >
> > The same flag check is used across TI clock driver already btw.
>
> Sure, it's not like this is a new problem. I'd just like to see if we
> can solve it now and get rid of the CLK_IS_BASIC flag now. It would be
> great if some extra effort could be put into it vs. punting the problem
> until 2020 or something.

Ok, let me see if I can figure out something for this...

-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki