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

[Stephen Boyd]

Quoting Tero Kristo (2018-11-29 23:35:35)
> On 30/11/2018 09:20, Stephen Boyd wrote:
> > Quoting Andreas Kemnade (2018-11-29 22:15:34)
> >> Hi Stephen,
> >>
> >> On Thu, 29 Nov 2018 16:25:05 -0800
> >> Stephen Boyd <sboyd@xxxxxxxxxx> wrote:
> >>
> >>> Quoting Andreas Kemnade (2018-11-10 12:31:14)
> >>>> Code might use autoidle api with clocks not being omap2 clocks,
> >>>> so check if clock type is not basic
> >>>>
> >>>> Signed-off-by: Andreas Kemnade <andreas@xxxxxxxxxxxx>
> >>>> ---
> >>>> New in v2
> >>>> ---
> >>>>   drivers/clk/ti/autoidle.c | 12 ++++++++++--
> >>>>   1 file changed, 10 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/drivers/clk/ti/autoidle.c b/drivers/clk/ti/autoidle.c
> >>>> index 161f67850393..5bdae5552d38 100644
> >>>> --- a/drivers/clk/ti/autoidle.c
> >>>> +++ b/drivers/clk/ti/autoidle.c
> >>>> @@ -54,8 +54,12 @@ static DEFINE_SPINLOCK(autoidle_spinlock);
> >>>>   int omap2_clk_deny_idle(struct clk *clk)
> >>>>   {
> >>>>          struct clk_hw_omap *c;
> >>>> +       struct clk_hw *hw = __clk_get_hw(clk);
> >>>>   
> >>>> -       c = to_clk_hw_omap(__clk_get_hw(clk));
> >>>> +       if (clk_hw_get_flags(hw) & CLK_IS_BASIC)
> >>>
> >>> Please try to avoid using CLK_IS_BASIC if at all possible. Can you?
> >>> Maybe add some flag in clk_hw_omap() instead?
> >>>
> >> hmm, Tero suggested that.
> >> But to check flags in clk_hw_omap I first need to know that there is a
> >> clk_hw_omap behind clk_hw. And for that I either need to check flags in
> >> clk_hw or do more changes in the omap_hwmod code.
> > 
> > Can you do it? The omap code is the only user of CLK_IS_BASIC. All the
> > other users are marking clks with this but there is no reason to do so.
> > I'll go make another pass over the tree and nuke those ones from orbit.
> 
> The reason for using this flag is because OMAP uses two clock types 
> around, the basic clocks like fixed-factor-clock/fixed-clock, and then 
> all the omap derivatives, which can be cast to clk_hw_omap. If we want 
> to avoid usage of CLK_IS_BASIC, we need to copy paste the remaining 
> basic code under drivers/clk/ti/ and convert them to use clk_hw_omap as 
> internal datatype. Is this preferred?
> 

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.