Re: [PATCH 2/2] clk: renesas: rzv2h: Report the DSI duty cycle from the LVDS divider

[Geert Uytterhoeven]

Hi Biju,

On Fri, 6 Nov 2026 at 08:41, Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> > On Thu, 29 Oct 2026 at 15:26, Tommaso Merciai <tommaso.merciai.xr@xxxxxxxxxxxxxx> wrote:
> > > The 4/7 duty cycle of the LVDS path comes from the division by 7, not
> > > from the PLLDSI mux, as an odd divider cannot produce a 50% duty cycle.
> >
> > Note that the latter is not true in general: if an odd divider looks at both edges of the parent clock
> > signal, the output will have a 50% duty cycle (assumed the parent has a 50% duty cycle).
> > If an odd divider looks only at rising or falling edges, the output will indeed have a non-50% duty
> > cycle.
> >
> > > Report it from a dedicated LVDS divider clock type and let the core
> > > walk up to the selected parent through CLK_DUTY_CYCLE_PARENT. The
> > > reported values do not change, the mux keeps selecting the path, and
> > > its duty cycle constants go away.
> > >
> > > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@xxxxxxxxxxxxxx>
> >
> > Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx> i.e. will queue in renesas-clk for v7.5.
> >
> > > --- a/drivers/clk/renesas/rzv2h-cpg.c
> > > +++ b/drivers/clk/renesas/rzv2h-cpg.c
> >
> > > @@ -890,6 +863,94 @@ rzv2h_cpg_fixed_mod_status_clk_register(const struct cpg_core_clk *core,
> > >         return &clk_hw_data->fix.hw;
> > >  }
> > >
> > > +static int rzv2h_cpg_plldsi_div_lvds_get_duty_cycle(struct clk_hw *hw,
> > > +                                                   struct clk_duty
> > > +*duty) {
> > > +       struct clk_fixed_factor *fix = to_clk_fixed_factor(hw);
> > > +
> > > +       /*
> > > +        * An odd divider cannot generate a 50% duty cycle: the output stays
> > > +        * high for (div + 1) / 2 input clock cycles out of div, e.g. a divider
> > > +        * of 7 gives a duty cycle of 4/7.
> > > +        */
> > > +       duty->num = DIV_ROUND_UP(fix->div, 2);
> > > +       duty->den = fix->div;
> > > +
> > > +       return 0;
> > > +}
> > > +
> > > +static int rzv2h_cpg_plldsi_div_lvds_determine_rate(struct clk_hw *hw,
> > > +                                                   struct
> > > +clk_rate_request *req) {
> > > +       return clk_fixed_factor_ops.determine_rate(hw, req); }
> > > +
> > > +static int rzv2h_cpg_plldsi_div_lvds_set_rate(struct clk_hw *hw,
> > > +                                             unsigned long rate,
> > > +                                             unsigned long
> > > +parent_rate) {
> > > +       return clk_fixed_factor_ops.set_rate(hw, rate, parent_rate); }
> > > +
> > > +static unsigned long rzv2h_cpg_plldsi_div_lvds_recalc_rate(struct clk_hw *hw,
> > > +                                                          unsigned
> > > +long parent_rate) {
> > > +       return clk_fixed_factor_ops.recalc_rate(hw, parent_rate); }
> > > +
> > > +static unsigned long
> > > +rzv2h_cpg_plldsi_div_lvds_recalc_accuracy(struct clk_hw *hw,
> > > +                                         unsigned long
> > > +parent_accuracy) {
> > > +       return clk_fixed_factor_ops.recalc_accuracy(hw,
> > > +parent_accuracy); }
> > > +
> > > +static const struct clk_ops rzv2h_cpg_plldsi_div_lvds_ops = {
> > > +       .determine_rate = rzv2h_cpg_plldsi_div_lvds_determine_rate,
> > > +       .set_rate = rzv2h_cpg_plldsi_div_lvds_set_rate,
> > > +       .recalc_rate = rzv2h_cpg_plldsi_div_lvds_recalc_rate,
> > > +       .recalc_accuracy = rzv2h_cpg_plldsi_div_lvds_recalc_accuracy,
> > > +       .get_duty_cycle = rzv2h_cpg_plldsi_div_lvds_get_duty_cycle,
> > > +};
> > > +
> > > +static struct clk_hw * __init
> > > +rzv2h_cpg_plldsi_div_lvds_clk_register(const struct cpg_core_clk *core,
> > > +                                      struct rzv2h_cpg_priv *priv) {
> > > +       struct clk_init_data init = { };
> > > +       struct clk_fixed_factor *fix;
> > > +       const struct clk_hw *parent;
> > > +       const char *parent_name;
> > > +       int ret;
> > > +
> > > +       WARN_DEBUG(core->parent >= priv->num_core_clks);
> > > +       parent = priv->clks[core->parent];
> > > +       if (IS_ERR(parent))
> > > +               return ERR_CAST(parent);
> > > +
> > > +       parent_name = clk_hw_get_name(parent);
> > > +
> > > +       fix = devm_kzalloc(priv->dev, sizeof(*fix), GFP_KERNEL);
> > > +       if (!fix)
> > > +               return ERR_PTR(-ENOMEM);
> > > +
> > > +       init.name = core->name;
> > > +       init.ops = &rzv2h_cpg_plldsi_div_lvds_ops;
> > > +       init.flags = CLK_SET_RATE_PARENT;
> > > +       init.parent_names = &parent_name;
> > > +       init.num_parents = 1;
> > > +
> > > +       fix->hw.init = &init;
> > > +       fix->mult = core->mult;
> > > +       fix->div = core->div;
> > > +
> > > +       ret = devm_clk_hw_register(priv->dev, &fix->hw);
> > > +       if (ret)
> > > +               return ERR_PTR(ret);
> > > +
> > > +       return &fix->hw;
> > > +}
> > > +
> > >  static struct clk_hw
> > >  *rzv2h_cpg_clk_src_twocell_get(struct of_phandle_args *clkspec,
> > >                                void *data)
> >
> > Since there are now two copies of this code (see rzg3l_cpg_lvds_div_*), perhaps the time is ripe to
> > add odd duty cycle support to clk_fixed_factor?  As duty->num can be rounded up or down, I am
> > envisioning two new mutually-exclusive fixflags parameters, e.g.:
> >
> >     CLK_FIXED_FACTOR_ODD_DUTY_DOWN
> >     CLK_FIXED_FACTOR_ODD_DUTY_UP
>
> For Renesas it is 4/7 odd divider
>
> For other vendor SoCs it can be 1/7 .. 6/7
> So not sure CLK_FIXED_FACTOR_ODD_DUTY_{UP,DOWN}
> applies? I believe if it is generic we need to pass
> both Nr/Dr for the duty cycle.
>
> I may be wrong. Please correct me if it is the case.

Right, some dividers may use other duty cycle values.
Looking at users of clk_duty, there don't seem to be many
that could make use of this...

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds