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

From: Tero Kristo
Date: Fri Jan 04 2019 - 02:30:13 EST

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]:

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.

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.



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