Re: [PATCH 6.1.y-cip 05/85] clk: renesas: Add family-specific clock driver for RZ/V2H(P)

Tommaso Merciai
2025-03-14T11:09:44Z

Hi Pavel,

Thanks for your review.

On Fri, Mar 14, 2025 at 11:12:06AM +0100, Pavel Machek wrote:
> Hi!
>
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > commit dd22e56217495e2d392ded86a1e11a908e424e64 upstream.
> >
> > Add family-specific clock driver for RZ/V2H(P) SoCs.
>
> So RZ/V2H... seems to be something different than "RZ/G3E" SoC. Good
> place to split the series?
>
> > --- /dev/null
> > +++ b/drivers/clk/renesas/rzv2h-cpg.c
> > +static int rzv2h_cpg_assert(struct reset_controller_dev *rcdev,
> > + unsigned long id)
> > +{
> > + struct rzv2h_cpg_priv *priv = rcdev_to_priv(rcdev);
> > + unsigned int reg = GET_RST_OFFSET(priv->resets[id].reset_index);
> > + u32 mask = BIT(priv->resets[id].reset_bit);
> > + u8 monbit = priv->resets[id].mon_bit;
> > + u32 value = mask << 16;
> > +
> > + dev_dbg(rcdev->dev, "assert id:%ld offset:0x%x\n", id, reg);
> > +
> > + writel(value, priv->base + reg);
> > +
> > + reg = GET_RST_MON_OFFSET(priv->resets[id].mon_index);
> > + mask = BIT(monbit);
> > +
> > + return readl_poll_timeout_atomic(priv->base + reg, value,
> > + value & mask, 10, 200);
> > +}
> > +
> > +static int rzv2h_cpg_deassert(struct reset_controller_dev *rcdev,
> > + unsigned long id)
> > +{
> > + struct rzv2h_cpg_priv *priv = rcdev_to_priv(rcdev);
> > + unsigned int reg = GET_RST_OFFSET(priv->resets[id].reset_index);
> > + u32 mask = BIT(priv->resets[id].reset_bit);
> > + u8 monbit = priv->resets[id].mon_bit;
> > + u32 value = (mask << 16) | mask;
> > +
> > + dev_dbg(rcdev->dev, "deassert id:%ld offset:0x%x\n", id, reg);
> > +
> > + writel(value, priv->base + reg);
> > +
> > + reg = GET_RST_MON_OFFSET(priv->resets[id].mon_index);
> > + mask = BIT(monbit);
> > +
> > + return readl_poll_timeout_atomic(priv->base + reg, value,
> > + !(value & mask), 10, 200);
> > +}
>
> Assert&deassert are fairly similar, can you share the code.
>
> (This and the other cleanups are not critical, will not block merge,
> but may be worth fixing in mainline).

Fully agree I will fix this into mainline first, thanks.

>
> Best regards,
> Pavel
> --
> DENX Software Engineering GmbH, Managing Director: Erika Unter
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

Regards,
Tommaso