Hi Geert,
Thank you for the review.
On Tue, Apr 15, 2025 at 4:51 PM Geert Uytterhoeven <ge...@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Tue, 8 Apr 2025 at 22:09, Prabhakar <prabhakar.cse...@gmail.com> wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad...@bp.renesas.com>
> >
> > Add clock and reset entries for the DSI and LCDC peripherals.
> >
> > Co-developed-by: Fabrizio Castro <fabrizio.castro...@renesas.com>
> > Signed-off-by: Fabrizio Castro <fabrizio.castro...@renesas.com>
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad...@bp.renesas.com>
> > ---
> > v1->v2:
> > - No changes
>
> You did change CSDIV0_DIVCTL2 to the NO_RMW-variant...
>
Ouch missed that.
> > --- a/drivers/clk/renesas/r9a09g057-cpg.c
> > +++ b/drivers/clk/renesas/r9a09g057-cpg.c
>
> > @@ -148,6 +182,12 @@ static const struct cpg_core_clk r9a09g057_core_clks[]
> > __initconst = {
> > DEF_SMUX(".smux2_gbe0_rxclk", CLK_SMUX2_GBE0_RXCLK, SSEL0_SELCTL3,
> > smux2_gbe0_rxclk),
> > DEF_SMUX(".smux2_gbe1_txclk", CLK_SMUX2_GBE1_TXCLK, SSEL1_SELCTL0,
> > smux2_gbe1_txclk),
> > DEF_SMUX(".smux2_gbe1_rxclk", CLK_SMUX2_GBE1_RXCLK, SSEL1_SELCTL1,
> > smux2_gbe1_rxclk),
> > + DEF_FIXED(".plleth_lpclk_div4", CLK_DIV_PLLETH_LPCLK, CLK_PLLETH,
> > 1, 4),
> > + DEF_CSDIV(".plleth_lpclk", CLK_CSDIV_PLLETH_LPCLK,
> > CLK_DIV_PLLETH_LPCLK,
> > + CSDIV0_DIVCTL2, dtable_16_128),
>
> The clock names look strange to me: "plleth_lpclk_div4" sounds like it is
> "plleth_lpclk" divided by four, but that is not the case here.
>
Maybe ".cdiv4_plleth_lpclk" and ".plleth_lpclk_gear"?
> > +
> > + DEF_PLLDSI_DIV(".plldsi_sdiv2", CLK_PLLDSI_SDIV2, CLK_PLLDSI,
> > + CSDIV1_DIVCTL2, dtable_2_32),
> >
> > DEF_DDIV(".pllgpu_gear", CLK_PLLGPU_GEAR, CLK_PLLGPU,
> > CDDIV3_DIVCTL1, dtable_2_64),
> >
>
> > --- a/drivers/clk/renesas/rzv2h-cpg.h
> > +++ b/drivers/clk/renesas/rzv2h-cpg.h
>
> > @@ -117,6 +118,8 @@ struct smuxed {
> >
> > #define CSDIV0_DIVCTL0 DDIV_PACK(CPG_CSDIV0, 0, 2, CSDIV_NO_MON)
> > #define CSDIV0_DIVCTL1 DDIV_PACK(CPG_CSDIV0, 4, 2, CSDIV_NO_MON)
> > +#define CSDIV0_DIVCTL2 DDIV_PACK_NO_RMW(CPG_CSDIV0, 8, 2, CSDIV_NO_MON)
>
> The documentation says:
>
> DIVCTL2[3:2] is reserved.
> When writing, read modify write is needed.
>
> So the NO_RMW-logic seems to be inverted?
>
Yes.
Cheers,
Prabhakar
