Re: [PATCH v1 2/3] clk: qcom: rcg2: Add support for hardware control mode

[Stephen Boyd <sboyd@kernel.org>]

Quoting Taniya Das (2019-07-30 03:50:08)
> Hello Stephen,
> 
> On 7/17/2019 4:48 AM, Stephen Boyd wrote:
> > Quoting Taniya Das (2019-07-15 21:19:02)
> >> Hello Stephen,
> >>
> >> Thanks for your review.
> >>
> >> On 7/16/2019 4:22 AM, Stephen Boyd wrote:
> >>> Quoting Taniya Das (2019-05-08 11:24:54)
> >>>> diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
> >>>> index 57dbac9..5bb6d45 100644
> >>>> --- a/drivers/clk/qcom/clk-rcg2.c
> >>>> +++ b/drivers/clk/qcom/clk-rcg2.c
> >>>> @@ -289,6 +289,9 @@ static int __clk_rcg2_configure(struct clk_rcg2 *rcg, const struct freq_tbl *f)
> >>>>           cfg |= rcg->parent_map[index].cfg << CFG_SRC_SEL_SHIFT;
> >>>>           if (rcg->mnd_width && f->n && (f->m != f->n))
> >>>>                   cfg |= CFG_MODE_DUAL_EDGE;
> >>>> +       if (rcg->flags & HW_CLK_CTRL_MODE)
> >>>> +               cfg |= CFG_HW_CLK_CTRL_MASK;
> >>>> +
> >>>
> >>> Above this we have commit bdc3bbdd40ba ("clk: qcom: Clear hardware clock
> >>> control bit of RCG") that clears this bit. Is it possible to always set
> >>> this bit and then have an override flag used in sdm845 that says to
> >>> _not_ set this bit? Presumably on earlier platforms writing the bit is a
> >>> no-op so it's safe to write the bit on those platforms.
> >>>
> >>> This way, if it's going to be the default we can avoid setting the flag
> >>> and only set the flag on older platforms where it shouldn't be done for
> >>> some reason.
> >>>
> >>
> >> Not all the subsystem clock controllers might have this hardware control
> >> bit set from design. Thus we want to set them based on the flag.
> > 
> > Yes but what's the percentage of clks that are going to set this flag
> > vs. not set this flag? If that is low right now then it's fine but if it
> > eventually becomes the standard mechanism it will be easier to opt-out
> > of the feature if necessary instead of opt-in.
> > 
> 
> Currently all the RCGs in GCC need to clear the bit and few RCGs from
> the other CCs(DISPCC/VIDEOCC) where the bit is not set from HW requires 
> this bit to be set. Thus we want to use this flag mechanism to have the 
> flexibility to set.
> 
> Once it is a standard we could cleanup to remove this.
> 

OK. Please send this patch along with whatever code/driver needs the new
flag.