Re: [PATCH v5 01/20] clk: renesas: rzv2h: Add PLLDSI clk mux support

[Geert Uytterhoeven]

Hi Tommaso,

On Fri, 27 Feb 2026 at 18:24, Tommaso Merciai
<tommaso.merciai.xr@xxxxxxxxxxxxxx> wrote:
> On Fri, Feb 27, 2026 at 11:47:58AM +0100, Geert Uytterhoeven wrote:
> > On Fri, 13 Feb 2026 at 17:28, Tommaso Merciai
> > <tommaso.merciai.xr@xxxxxxxxxxxxxx> wrote:
> > > Add PLLDSI clk mux support to select PLLDSI clock from different clock
> > > sources.
> > >
> > > Introduce the DEF_PLLDSI_SMUX() macro to define these muxes and register
> > > them in the clock driver.
> > >
> > > Extend the determine_rate callback to calculate and propagate PLL
> > > parameters via rzv2h_get_pll_dtable_pars() when LVDS output is selected,
> > > using a new helper function rzv2h_cpg_plldsi_smux_lvds_determine_rate().
> > >
> > > The CLK_SMUX2_DSI{0,1}_CLK clock multiplexers select between two paths
> > > with different duty cycles:
> > >
> > > - CDIV7_DSIx_CLK (LVDS path, parent index 0): asymmetric H/L=4/3 duty (4/7)
> > > - CSDIV_DSIx (DSI/RGB path, parent index 1): symmetric 50% duty (1/2)
> > >
> > > Implement rzv2h_cpg_plldsi_smux_{get,set}_duty_cycle clock operations to
> > > allow the DRM driver to query and configure the appropriate clock path
> > > based on the required output duty cycle.
> > >
> > > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@xxxxxxxxxxxxxx>
> >
> > Thanks for your patch!
> >
> > > --- a/drivers/clk/renesas/rzv2h-cpg.c
> > > +++ b/drivers/clk/renesas/rzv2h-cpg.c

> > > +       clk_hw_data = devm_kzalloc(priv->dev, sizeof(*clk_hw_data), GFP_KERNEL);
> > > +       if (!clk_hw_data)
> > > +               return ERR_PTR(-ENOMEM);
> > > +
> > > +       clk_hw_data->priv = priv;
> > > +
> > > +       init.name = core->name;
> > > +       init.ops = &rzv2h_cpg_plldsi_smux_ops;
> > > +       init.flags = core->flag;
> > > +       init.parent_names = core->parent_names;
> > > +       init.num_parents = core->num_parents;
> > > +
> > > +       clk_hw_data->mux.reg = priv->base + smux.offset;
> > > +
> > > +       clk_hw_data->mux.shift = smux.shift;
> > > +       clk_hw_data->mux.mask = smux.width;
> >
> > Again, smux.width is not a mask.
> > Perhaps GENMASK_U16(smux.shift - 1, 0)?
>
> Or maybe we can use:
>
>         clk_hw_data->mux.mask = clk_div_mask(smux.width);
> ?

Thanks, I forgot about clk_div_mask(), which is definitely a better
choice here.

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