[PATCH 0/3] clk: renesas: rzv2h: Fixes and improvements

[PATCH 0/3] clk: renesas: rzv2h: Fixes and improvements

[Tommaso Merciai]

Dear All,

This patchset contains small fixes and improvements related to the rzv2h cpg
clock driver.

Thanks & Regards,
Tommaso

Tommaso Merciai (3):
  dt-bindings: clock: renesas: Fix description section
  clk: renesas: rzv2h: Improve rzv2h_ddiv_set_rate()
  clk: renesas: rzv2h: Simplify rzv2h_cpg_assert()/rzv2h_cpg_deassert()

 .../bindings/clock/renesas,rzv2h-cpg.yaml     |  2 +-
 drivers/clk/renesas/rzv2h-cpg.c               | 40 +++++++------------
 2 files changed, 15 insertions(+), 27 deletions(-)

-- 
2.43.0

[PATCH 1/3] dt-bindings: clock: renesas: Fix description section

[Tommaso Merciai]

Remove not needed "and" into description section.

Reported-by: Pavel Machek <pavel@denx.de>
Closes: https://lore.kernel.org/cip-dev/Z9P%2F51qOlq2B46FK@duo.ucw.cz/
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
 Documentation/devicetree/bindings/clock/renesas,rzv2h-cpg.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/clock/renesas,rzv2h-cpg.yaml b/Documentation/devicetree/bindings/clock/renesas,rzv2h-cpg.yaml
index c3fe76abd549..511692b7f84d 100644
--- a/Documentation/devicetree/bindings/clock/renesas,rzv2h-cpg.yaml
+++ b/Documentation/devicetree/bindings/clock/renesas,rzv2h-cpg.yaml
@@ -12,7 +12,7 @@ maintainers:
 description:
   On Renesas RZ/{G3E,V2H(P)} SoCs, the CPG (Clock Pulse Generator) handles
   generation and control of clock signals for the IP modules, generation and
-  control of resets, and control over booting, low power consumption and power
+  control of resets, control over booting, low power consumption and power
   supply domains.
 
 properties:
-- 
2.43.0

Re: [PATCH 1/3] dt-bindings: clock: renesas: Fix description section

[Geert Uytterhoeven]

Hi Tommaso,

On Mon, 17 Mar 2025 at 09:32, Tommaso Merciai
<tommaso.merciai.xr@bp.renesas.com> wrote:
> Remove not needed "and" into description section.
>
> Reported-by: Pavel Machek <pavel@denx.de>
> Closes: https://lore.kernel.org/cip-dev/Z9P%2F51qOlq2B46FK@duo.ucw.cz/
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>

Thanks for your patch!

> --- a/Documentation/devicetree/bindings/clock/renesas,rzv2h-cpg.yaml
> +++ b/Documentation/devicetree/bindings/clock/renesas,rzv2h-cpg.yaml
> @@ -12,7 +12,7 @@ maintainers:
>  description:
>    On Renesas RZ/{G3E,V2H(P)} SoCs, the CPG (Clock Pulse Generator) handles
>    generation and control of clock signals for the IP modules, generation and
> -  control of resets, and control over booting, low power consumption and power
> +  control of resets, control over booting, low power consumption and power
>    supply domains.
>
>  properties:

I think the original is fine.  When emphasizing the structure:

    The CPG handles:
      A. generation and control of clock signals for the IP modules,
      B. generation and control of resets, and
      C. control over booting, low power consumption and power supply domains.

i.e. the "and" is part of the typical "A, B, and C" construct?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

Re: [PATCH 1/3] dt-bindings: clock: renesas: Fix description section

[Tommaso Merciai]

Hi Geert,
Thanks for your review.

On Mon, Mar 17, 2025 at 11:08:19AM +0100, Geert Uytterhoeven wrote:
> Hi Tommaso,
> 
> On Mon, 17 Mar 2025 at 09:32, Tommaso Merciai
> <tommaso.merciai.xr@bp.renesas.com> wrote:
> > Remove not needed "and" into description section.
> >
> > Reported-by: Pavel Machek <pavel@denx.de>
> > Closes: https://lore.kernel.org/cip-dev/Z9P%2F51qOlq2B46FK@duo.ucw.cz/
> > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> 
> Thanks for your patch!
> 
> > --- a/Documentation/devicetree/bindings/clock/renesas,rzv2h-cpg.yaml
> > +++ b/Documentation/devicetree/bindings/clock/renesas,rzv2h-cpg.yaml
> > @@ -12,7 +12,7 @@ maintainers:
> >  description:
> >    On Renesas RZ/{G3E,V2H(P)} SoCs, the CPG (Clock Pulse Generator) handles
> >    generation and control of clock signals for the IP modules, generation and
> > -  control of resets, and control over booting, low power consumption and power
> > +  control of resets, control over booting, low power consumption and power
> >    supply domains.
> >
> >  properties:
> 
> I think the original is fine.  When emphasizing the structure:
> 
>     The CPG handles:
>       A. generation and control of clock signals for the IP modules,
>       B. generation and control of resets, and
>       C. control over booting, low power consumption and power supply domains.
> 
> i.e. the "and" is part of the typical "A, B, and C" construct?

Fine to me and thanks for the explanation.
If you agree I will drop this in v2.

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> -- 
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> 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

Thanks & Regards,
Tommaso

Re: [PATCH 1/3] dt-bindings: clock: renesas: Fix description section

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1604 bytes --]

Hi!

> On Mon, 17 Mar 2025 at 09:32, Tommaso Merciai
> <tommaso.merciai.xr@bp.renesas.com> wrote:
> > Remove not needed "and" into description section.
> >
> > Reported-by: Pavel Machek <pavel@denx.de>
> > Closes: https://lore.kernel.org/cip-dev/Z9P%2F51qOlq2B46FK@duo.ucw.cz/
> > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> 
> Thanks for your patch!
> 
> > --- a/Documentation/devicetree/bindings/clock/renesas,rzv2h-cpg.yaml
> > +++ b/Documentation/devicetree/bindings/clock/renesas,rzv2h-cpg.yaml
> > @@ -12,7 +12,7 @@ maintainers:
> >  description:
> >    On Renesas RZ/{G3E,V2H(P)} SoCs, the CPG (Clock Pulse Generator) handles
> >    generation and control of clock signals for the IP modules, generation and
> > -  control of resets, and control over booting, low power consumption and power
> > +  control of resets, control over booting, low power consumption and power
> >    supply domains.
> >
> >  properties:
> 
> I think the original is fine.  When emphasizing the structure:
> 
>     The CPG handles:
>       A. generation and control of clock signals for the IP modules,
>       B. generation and control of resets, and
>       C. control over booting, low power consumption and power supply domains.
> 
> i.e. the "and" is part of the typical "A, B, and C" construct?

Well, it is still horid sentence. What about making it into list, as
you did?

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[PATCH 2/3] clk: renesas: rzv2h: Improve rzv2h_ddiv_set_rate()

[Tommaso Merciai]

Remove duplicate code into rzv2h_ddiv_set_rate().

Reported-by: Pavel Machek <pavel@denx.de>
Closes: https://lore.kernel.org/cip-dev/Z9QBZo4GgtMjid0v@duo.ucw.cz/
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
 drivers/clk/renesas/rzv2h-cpg.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/clk/renesas/rzv2h-cpg.c b/drivers/clk/renesas/rzv2h-cpg.c
index 2b9771ab2b3f..bf3ccc0522d5 100644
--- a/drivers/clk/renesas/rzv2h-cpg.c
+++ b/drivers/clk/renesas/rzv2h-cpg.c
@@ -272,12 +272,6 @@ static int rzv2h_ddiv_set_rate(struct clk_hw *hw, unsigned long rate,
 	writel(val, divider->reg);
 
 	ret = rzv2h_cpg_wait_ddiv_clk_update_done(priv->base, ddiv->mon);
-	if (ret)
-		goto ddiv_timeout;
-
-	spin_unlock_irqrestore(divider->lock, flags);
-
-	return 0;
 
 ddiv_timeout:
 	spin_unlock_irqrestore(divider->lock, flags);
-- 
2.43.0

Re: [PATCH 2/3] clk: renesas: rzv2h: Improve rzv2h_ddiv_set_rate()

[Geert Uytterhoeven]

On Mon, 17 Mar 2025 at 09:33, Tommaso Merciai
<tommaso.merciai.xr@bp.renesas.com> wrote:
> Remove duplicate code into rzv2h_ddiv_set_rate().
>
> Reported-by: Pavel Machek <pavel@denx.de>
> Closes: https://lore.kernel.org/cip-dev/Z9QBZo4GgtMjid0v@duo.ucw.cz/
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-clk for v6.16.

Gr{oetje,eeting}s,

                        Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

[PATCH 3/3] clk: renesas: rzv2h: Simplify rzv2h_cpg_assert()/rzv2h_cpg_deassert()

[Tommaso Merciai]

rzv2h_cpg_assert() and rzv2h_cpg_deassert() functions are similar. Share
this code via __rzv2h_cpg_assert(). This avoid code duplication.

Reported-by: Pavel Machek <pavel@denx.de>
Closes: https://lore.kernel.org/cip-dev/Z9QA9rwuXCuVbOXp@duo.ucw.cz/
Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
 drivers/clk/renesas/rzv2h-cpg.c | 34 ++++++++++++++-------------------
 1 file changed, 14 insertions(+), 20 deletions(-)

diff --git a/drivers/clk/renesas/rzv2h-cpg.c b/drivers/clk/renesas/rzv2h-cpg.c
index bf3ccc0522d5..691bfe37ec1b 100644
--- a/drivers/clk/renesas/rzv2h-cpg.c
+++ b/drivers/clk/renesas/rzv2h-cpg.c
@@ -652,16 +652,17 @@ rzv2h_cpg_register_mod_clk(const struct rzv2h_mod_clk *mod,
 		mod->name, PTR_ERR(clk));
 }
 
-static int rzv2h_cpg_assert(struct reset_controller_dev *rcdev,
-			    unsigned long id)
+static int __rzv2h_cpg_assert(struct reset_controller_dev *rcdev,
+			      unsigned long id, bool assert)
 {
 	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;
+	u32 value = assert ? (mask << 16) : ((mask << 16) | mask);
 
-	dev_dbg(rcdev->dev, "assert id:%ld offset:0x%x\n", id, reg);
+	dev_dbg(rcdev->dev, "%s id:%ld offset:0x%x\n",
+		assert ? "assert" : "deassert", id, reg);
 
 	writel(value, priv->base + reg);
 
@@ -669,27 +670,20 @@ static int rzv2h_cpg_assert(struct reset_controller_dev *rcdev,
 	mask = BIT(monbit);
 
 	return readl_poll_timeout_atomic(priv->base + reg, value,
-					 value & mask, 10, 200);
+					 assert ? (value & mask) : !(value & mask),
+					 10, 200);
+}
+
+static int rzv2h_cpg_assert(struct reset_controller_dev *rcdev,
+			    unsigned long id)
+{
+	return __rzv2h_cpg_assert(rcdev, id, true);
 }
 
 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);
+	return __rzv2h_cpg_assert(rcdev, id, false);
 }
 
 static int rzv2h_cpg_reset(struct reset_controller_dev *rcdev,
-- 
2.43.0

Re: [PATCH 3/3] clk: renesas: rzv2h: Simplify rzv2h_cpg_assert()/rzv2h_cpg_deassert()

[Geert Uytterhoeven]

Hi Tommaso,

On Mon, 17 Mar 2025 at 09:33, Tommaso Merciai
<tommaso.merciai.xr@bp.renesas.com> wrote:
> rzv2h_cpg_assert() and rzv2h_cpg_deassert() functions are similar. Share
> this code via __rzv2h_cpg_assert(). This avoid code duplication.
>
> Reported-by: Pavel Machek <pavel@denx.de>
> Closes: https://lore.kernel.org/cip-dev/Z9QA9rwuXCuVbOXp@duo.ucw.cz/
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>

Thanks for your patch!

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-clk for v6.16.

> --- a/drivers/clk/renesas/rzv2h-cpg.c
> +++ b/drivers/clk/renesas/rzv2h-cpg.c
> @@ -652,16 +652,17 @@ rzv2h_cpg_register_mod_clk(const struct rzv2h_mod_clk *mod,
>                 mod->name, PTR_ERR(clk));
>  }
>
> -static int rzv2h_cpg_assert(struct reset_controller_dev *rcdev,
> -                           unsigned long id)
> +static int __rzv2h_cpg_assert(struct reset_controller_dev *rcdev,
> +                             unsigned long id, bool assert)
>  {
>         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;
> +       u32 value = assert ? (mask << 16) : ((mask << 16) | mask);

Do you mind if I change this to

    u32 value = mask << 16;

    if (!assert)
            value |= mask;

while applying?

>
> -       dev_dbg(rcdev->dev, "assert id:%ld offset:0x%x\n", id, reg);
> +       dev_dbg(rcdev->dev, "%s id:%ld offset:0x%x\n",
> +               assert ? "assert" : "deassert", id, reg);
>
>         writel(value, priv->base + reg);
>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

Re: [PATCH 3/3] clk: renesas: rzv2h: Simplify rzv2h_cpg_assert()/rzv2h_cpg_deassert()

[Tommaso Merciai]

Hi Geert,
Thanks for your review.

On Thu, Apr 10, 2025 at 06:25:13PM +0200, Geert Uytterhoeven wrote:
> Hi Tommaso,
> 
> On Mon, 17 Mar 2025 at 09:33, Tommaso Merciai
> <tommaso.merciai.xr@bp.renesas.com> wrote:
> > rzv2h_cpg_assert() and rzv2h_cpg_deassert() functions are similar. Share
> > this code via __rzv2h_cpg_assert(). This avoid code duplication.
> >
> > Reported-by: Pavel Machek <pavel@denx.de>
> > Closes: https://lore.kernel.org/cip-dev/Z9QA9rwuXCuVbOXp@duo.ucw.cz/
> > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> 
> Thanks for your patch!
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> i.e. will queue in renesas-clk for v6.16.
> 
> > --- a/drivers/clk/renesas/rzv2h-cpg.c
> > +++ b/drivers/clk/renesas/rzv2h-cpg.c
> > @@ -652,16 +652,17 @@ rzv2h_cpg_register_mod_clk(const struct rzv2h_mod_clk *mod,
> >                 mod->name, PTR_ERR(clk));
> >  }
> >
> > -static int rzv2h_cpg_assert(struct reset_controller_dev *rcdev,
> > -                           unsigned long id)
> > +static int __rzv2h_cpg_assert(struct reset_controller_dev *rcdev,
> > +                             unsigned long id, bool assert)
> >  {
> >         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;
> > +       u32 value = assert ? (mask << 16) : ((mask << 16) | mask);
> 
> Do you mind if I change this to
> 
>     u32 value = mask << 16;
> 
>     if (!assert)
>             value |= mask;
> 
> while applying?

Fine to me, thanks.

Regards,
Tommaso

> 
> >
> > -       dev_dbg(rcdev->dev, "assert id:%ld offset:0x%x\n", id, reg);
> > +       dev_dbg(rcdev->dev, "%s id:%ld offset:0x%x\n",
> > +               assert ? "assert" : "deassert", id, reg);
> >
> >         writel(value, priv->base + reg);
> >
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> -- 
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> 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