Re: [PATCH V3 2/5] clk: qcom: Remove support to set CAL_L field in lucid evo pll configure

[Jagadeesh Kona]

Hi Dmitry,

Thanks for your review!

On 6/1/2023 8:16 PM, Dmitry Baryshkov wrote:
> On 01/06/2023 17:34, Jagadeesh Kona wrote:
> > For lucid evo and ole pll's the CAL_L, RINGOSC_CAL_L and L_VAL are
> > part of the same register, hence update the l configuration value
> > to include these fields across all the chipsets.
> >
> > Since the l configuration value now includes both L and CAL_L fields,
> > there is no need to explicitly set CAL_L field again in lucid evo pll
> > configure, Hence remove support to explicity set CAL_L field for evo pll.
> >
> > Fixes: 260e36606a03 ("clk: qcom: clk-alpha-pll: add Lucid EVO PLL 
> > configuration interfaces")
> > Signed-off-by: Taniya Das <quic_tdas@xxxxxxxxxxx>
> > Signed-off-by: Jagadeesh Kona <quic_jkona@xxxxxxxxxxx>
> > ---
> > Changes since V2:
> >   - Squashed update L val and remove explicit cal_l configuration to 
> > single patch
> >   - Updated L configuration for gpucc-sm8450 as well which was merged 
> > recently
> > Changes since V1:
> >   - Newly added.
> >
> >   drivers/clk/qcom/camcc-sm8450.c  | 24 ++++++++++++++++--------
> >   drivers/clk/qcom/clk-alpha-pll.c |  6 +-----
> >   drivers/clk/qcom/dispcc-sm8450.c |  6 ++++--
> >   drivers/clk/qcom/dispcc-sm8550.c |  6 ++++--
> >   drivers/clk/qcom/gpucc-sa8775p.c |  6 ++++--
> >   drivers/clk/qcom/gpucc-sm8450.c  |  6 ++++--
> >   6 files changed, 33 insertions(+), 21 deletions(-)
>
> I'd say, this is still not a correct solution from my point of view. A 
> correct solution would be to follow the existing code and use constants 
> for the constant values (of CAL_L, and RINGOSC_CAL_L).

Sure, will keep the existing code as is and will remove this patch in 
the next series.

> > diff --git a/drivers/clk/qcom/camcc-sm8450.c 
> > b/drivers/clk/qcom/camcc-sm8450.c
> > index 51338a2884d2..6a5a08f88598 100644
> > --- a/drivers/clk/qcom/camcc-sm8450.c
> > +++ b/drivers/clk/qcom/camcc-sm8450.c
> > @@ -57,7 +57,8 @@ static const struct pll_vco rivian_evo_vco[] = {
> >   static const struct clk_parent_data pll_parent_data_tcxo = { .index 
> > = DT_BI_TCXO };
> >   static const struct alpha_pll_config cam_cc_pll0_config = {
> > -    .l = 0x3e,
> > +    /* .l includes CAL_L_VAL, L_VAL fields */
> > +    .l = 0x0044003e,
> >       .alpha = 0x8000,
> >       .config_ctl_val = 0x20485699,
> >       .config_ctl_hi_val = 0x00182261,
> > @@ -128,7 +129,8 @@ static struct clk_alpha_pll_postdiv 
> > cam_cc_pll0_out_odd = {
> >   };

[skipped]

Thanks & Regards,
Jagadeesh