RE: [PATCH v5 1/4] clk: zynqmp: Use firmware specific common clock flags

[Rajan Vaja]

Hi Stephen,

> -----Original Message-----
> From: Stephen Boyd <sboyd@xxxxxxxxxx>
> Sent: Friday, June 25, 2021 4:22 PM
> To: Rajan Vaja <RAJANV@xxxxxxxxxx>; kristo@xxxxxxxxxx; lee.jones@xxxxxxxxxx;
> Michal Simek <michals@xxxxxxxxxx>; mturquette@xxxxxxxxxxxx;
> quanyang.wang@xxxxxxxxxxxxx
> Cc: linux-clk@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; Rajan Vaja <RAJANV@xxxxxxxxxx>
> Subject: Re: [PATCH v5 1/4] clk: zynqmp: Use firmware specific common clock
> flags
> 
> Quoting Rajan Vaja (2021-06-24 05:16:30)
> > diff --git a/drivers/clk/zynqmp/clkc.c b/drivers/clk/zynqmp/clkc.c
> > index db8d0d7161ce..af06a195ec46 100644
> > --- a/drivers/clk/zynqmp/clkc.c
> > +++ b/drivers/clk/zynqmp/clkc.c
> > @@ -271,6 +271,34 @@ static int zynqmp_pm_clock_get_topology(u32
> clock_id, u32 index,
> >         return ret;
> >  }
> >
> > +unsigned long zynqmp_clk_map_common_ccf_flags(const u32 zynqmp_flag)
> > +{
> > +       unsigned long ccf_flag = 0;
> > +
> > +       if (zynqmp_flag & ZYNQMP_CLK_SET_RATE_GATE)
> > +               ccf_flag |= CLK_SET_RATE_GATE;
> > +       if (zynqmp_flag & ZYNQMP_CLK_SET_PARENT_GATE)
> > +               ccf_flag |= CLK_SET_PARENT_GATE;
> > +       if (zynqmp_flag & ZYNQMP_CLK_SET_RATE_PARENT)
> > +               ccf_flag |= CLK_SET_RATE_PARENT;
> > +       if (zynqmp_flag & ZYNQMP_CLK_IGNORE_UNUSED)
> > +               ccf_flag |= CLK_IGNORE_UNUSED;
> > +       if (zynqmp_flag & ZYNQMP_CLK_GET_RATE_NOCACHE)
> > +               ccf_flag |= CLK_GET_RATE_NOCACHE;
> 
> Does the firmware really use all these flags? Ideally we get rid of the
> above two.
> 
> > +       if (zynqmp_flag & ZYNQMP_CLK_SET_RATE_NO_REPARENT)
> > +               ccf_flag |= CLK_SET_RATE_NO_REPARENT;
> > +       if (zynqmp_flag & ZYNQMP_CLK_GET_ACCURACY_NOCACHE)
> > +               ccf_flag |= CLK_GET_ACCURACY_NOCACHE;
> > +       if (zynqmp_flag & ZYNQMP_CLK_RECALC_NEW_RATES)
> > +               ccf_flag |= CLK_RECALC_NEW_RATES;
> 
> And this one.
> 
> > +       if (zynqmp_flag & ZYNQMP_CLK_SET_RATE_UNGATE)
> > +               ccf_flag |= CLK_SET_RATE_UNGATE;
> > +       if (zynqmp_flag & ZYNQMP_CLK_IS_CRITICAL)
> > +               ccf_flag |= CLK_IS_CRITICAL;
> 
> And this one.
> 
> I worry that supporting all these flags will mean we can never get rid
> of them. And we currently don't support setting critical via DT, which
> is essentially another firmware interface like this one.
[Rajan] firmware is using below flags:
ZYNQMP_CLK_SET_RATE_GATE
ZYNQMP_CLK_SET_PARENT_GATE
ZYNQMP_CLK_SET_RATE_PARENT
ZYNQMP_CLK_IGNORE_UNUSED
ZYNQMP_CLK_SET_RATE_NO_REPARENT
ZYNQMP_CLK_IS_CRITICAL

Other flags are unused. I will remove unused flags in next version.

Thanks,
Rajan
> 
> > +
> > +       return ccf_flag;
> > +}
> > +
> >  /**
> >   * zynqmp_clk_register_fixed_factor() - Register fixed factor with the
> >   *                                     clock framework