RE: [PATCH 11/13] clk: renesas: rzg2l-cpg: drop determine_rate op and use CLK_ROUNDING_FW_MANAGED flag

From: Biju Das

Date: Fri Feb 27 2026 - 11:12:44 EST


Hi Geert,

> -----Original Message-----
> From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> Sent: 27 February 2026 16:01
>
> Hi Biju,
>
> On Fri, 27 Feb 2026 at 16:57, Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > > From: Brian Masney <bmasney@xxxxxxxxxx> On Fri, Feb 27, 2026 at
> > > 03:23:19PM +0000, Biju Das wrote:
> > > > > From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> On Fri, 27 Feb
> > > > > 2026 at 16:01, Brian Masney <bmasney@xxxxxxxxxx> wrote:
> > > > > > On Fri, Feb 27, 2026 at 09:20:09AM +0100, Geert Uytterhoeven wrote:
> > > > > > > On Thu, 26 Feb 2026 at 19:18, Brian Masney <bmasney@xxxxxxxxxx> wrote:
> > > > > > > > This clk driver has a noop determine_rate clk op. Drop
> > > > > > > > this empty function, and enable the CLK_ROUNDING_FW_MANAGED flag.
> > > > > > > >
> > > > > > > > Signed-off-by: Brian Masney <bmasney@xxxxxxxxxx>
> > > > > > >
> > > > > > > Thanks for your patch!
> > > > > > >
> > > > > > > > --- a/drivers/clk/renesas/rzg2l-cpg.c
> > > > > > > > +++ b/drivers/clk/renesas/rzg2l-cpg.c
> > > > >
> > > > > > > > @@ -1041,7 +1034,7 @@ rzg2l_cpg_sipll5_register(const struct cpg_core_clk *core,
> > > > > > > > init.name = core->name;
> > > > > > > > parent_name = __clk_get_name(parent);
> > > > > > > > init.ops = &rzg2l_cpg_sipll5_ops;
> > > > > > > > - init.flags = 0;
> > > > > > > > + init.flags = CLK_ROUNDING_FW_MANAGED;
> > > > > > >
> > > > > > > Iff this is the Right Thing To Do (TM), it needs a comment,
> > > > > > > as this clock is not managed by firmware.
> > > > > >
> > > > > > Before I start a larger discussion on patch 1 with more people
> > > > > > about a name for this flag, help me understand why this
> > > > > > provider has a noop determine rate. Is the hardware eventually
> > > > > > programmed with a rate that's close enough to what was passed
> > > > > > in? Or it doesn't really matter what the clock rate is, just
> > > > > > as long as it is running? Or should the determine_rate function be filled out in this
> particular case?
> > > > >
> > > > > I'd like to defer to Biju, who added the empty round^Wdetermine rate function.
> > > >
> > > > PLL is capable of generating any frequency. that is the reason.
> > > >
> > > > But we could, call rzg2l_cpg_get_foutpostdiv_rate() from
> > > > determine_rate() as modified rzg2l_cpg_get_foutpostdiv_rate()[1]
> > > > can return errors
> > > >
> > > > [1]
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.gi
> > > > t/tr
> > > > ee/drivers/clk/renesas/rzg2l-cpg.c?h=next-20260227#n590
> > >
> > > OK. So how do you all want to proceed here?
> > >
> > > Do you want to fill out the determine rate in this driver? If so, we
> > > can just drop this particular patch from this series.
> >
> > We plan to fill out the determine rate later, as it can return error.
> >
> > >
> > > Since the PLL is capable of any frequency, do you want to keep the empty determine rate
> functionality?
> > > If so, we could use a different name for this flag. Maybe CLK_ROUNDING_NOOP?
> >
> > I guess, maybe we could use CLK_ROUNDING_NOOP, till we have proper .determine_rate() for this
> driver???
>
> Just keeping the empty function should work, too, right?

Yes, it will work.

Cheers,
Biju