diff mbox series

[v2,09/13] clk: renesas: rzg2l: Add support for RZ/V2M reset monitor reg

Message ID 20220330154024.112270-10-phil.edworthy@renesas.com (mailing list archive)
State Superseded, archived
Headers show
Series Add new Renesas RZ/V2M SoC and Renesas RZ/V2M EVK support | expand

Commit Message

Phil Edworthy March 30, 2022, 3:40 p.m. UTC 
The RZ/V2M doesn't have a matching set of reset monitor regs for each reset
reg like the RZ/G2L. Instead, it has a single CPG_RST_MON reg which has a
single bit per module.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/clk/renesas/rzg2l-cpg.c |  6 +++++-
 drivers/clk/renesas/rzg2l-cpg.h | 10 ++++++++--
 2 files changed, 13 insertions(+), 3 deletions(-)

Comments

Geert Uytterhoeven April 26, 2022, 3:37 p.m. UTC | #1 
Hi Phil,

On Wed, Mar 30, 2022 at 5:42 PM Phil Edworthy <phil.edworthy@renesas.com> wrote:
> The RZ/V2M doesn't have a matching set of reset monitor regs for each reset
> reg like the RZ/G2L. Instead, it has a single CPG_RST_MON reg which has a
> single bit per module.
>
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>

Thanks for your patch!

> --- a/drivers/clk/renesas/rzg2l-cpg.c
> +++ b/drivers/clk/renesas/rzg2l-cpg.c
> @@ -748,8 +748,12 @@ static int rzg2l_cpg_status(struct reset_controller_dev *rcdev,
>         const struct rzg2l_cpg_info *info = priv->info;
>         unsigned int reg = info->resets[id].off;
>         u32 bitmask = BIT(info->resets[id].bit);
> +       u32 monbitmask = BIT(info->resets[id].monbit);

BIT(-1) is not defined...

>
> -       return !(readl(priv->base + CLK_MRST_R(reg)) & bitmask);
> +       if (info->has_clk_mon_regs)
> +               return !(readl(priv->base + CLK_MRST_R(reg)) & bitmask);
> +       else
> +               return !!(readl(priv->base + CPG_RST_MON) & monbitmask);

... hence the above may behave badly when the reset has no bit in
CPG_RST_MON (69 resets do not have a bit in CPG_RST_MON).

> --- a/drivers/clk/renesas/rzg2l-cpg.h
> +++ b/drivers/clk/renesas/rzg2l-cpg.h
> @@ -18,6 +18,7 @@
>  #define CPG_PL3_SSEL           (0x408)
>  #define CPG_PL6_SSEL           (0x414)
>  #define CPG_PL6_ETH_SSEL       (0x418)
> +#define CPG_RST_MON            (0x680)
>
>  #define CPG_CLKSTATUS_SELSDHI0_STS     BIT(28)
>  #define CPG_CLKSTATUS_SELSDHI1_STS     BIT(29)
> @@ -151,17 +152,22 @@ struct rzg2l_mod_clk {
>   *
>   * @off: register offset
>   * @bit: reset bit
> + * @monbit: monitor bit in CPG_RST_MON register, -1 if none
>   */
>  struct rzg2l_reset {
>         u16 off;
>         u8 bit;
> +       s8 monbit;
>  };
>
> -#define DEF_RST(_id, _off, _bit)       \
> +#define DEF_RST_MON(_id, _off, _bit, _monbit)  \
>         [_id] = { \
>                 .off = (_off), \
> -               .bit = (_bit) \
> +               .bit = (_bit), \
> +               .monbit = (_monbit) \
>         }
> +#define DEF_RST(_id, _off, _bit)       \
> +       DEF_RST_MON(_id, _off, _bit, -1)
>
>  /**
>   * struct rzg2l_cpg_info - SoC-specific CPG Description

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
Phil Edworthy April 27, 2022, 6 p.m. UTC | #2 
Hi Geert,

On  26 April 2022 16:37 Geert Uytterhoeven wrote:
> On Wed, Mar 30, 2022 at 5:42 PM Phil Edworthy wrote:
> > The RZ/V2M doesn't have a matching set of reset monitor regs for each
> reset
> > reg like the RZ/G2L. Instead, it has a single CPG_RST_MON reg which has
> a
> > single bit per module.
> >
> > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Thanks for your patch!
> 
> > --- a/drivers/clk/renesas/rzg2l-cpg.c
> > +++ b/drivers/clk/renesas/rzg2l-cpg.c
> > @@ -748,8 +748,12 @@ static int rzg2l_cpg_status(struct
> reset_controller_dev *rcdev,
> >         const struct rzg2l_cpg_info *info = priv->info;
> >         unsigned int reg = info->resets[id].off;
> >         u32 bitmask = BIT(info->resets[id].bit);
> > +       u32 monbitmask = BIT(info->resets[id].monbit);
> 
> BIT(-1) is not defined...
> 
> >
> > -       return !(readl(priv->base + CLK_MRST_R(reg)) & bitmask);
> > +       if (info->has_clk_mon_regs)
> > +               return !(readl(priv->base + CLK_MRST_R(reg)) & bitmask);
> > +       else
> > +               return !!(readl(priv->base + CPG_RST_MON) & monbitmask);
> 
> ... hence the above may behave badly when the reset has no bit in
> CPG_RST_MON (69 resets do not have a bit in CPG_RST_MON).

Ah, right. The SoCs other than RZ/V2M have monbit = -1, but they all
have info->has_clk_mon_regs = 1.
Still, I take you point that it's not very good code.


> > --- a/drivers/clk/renesas/rzg2l-cpg.h
> > +++ b/drivers/clk/renesas/rzg2l-cpg.h
> > @@ -18,6 +18,7 @@
> >  #define CPG_PL3_SSEL           (0x408)
> >  #define CPG_PL6_SSEL           (0x414)
> >  #define CPG_PL6_ETH_SSEL       (0x418)
> > +#define CPG_RST_MON            (0x680)
> >
> >  #define CPG_CLKSTATUS_SELSDHI0_STS     BIT(28)
> >  #define CPG_CLKSTATUS_SELSDHI1_STS     BIT(29)
> > @@ -151,17 +152,22 @@ struct rzg2l_mod_clk {
> >   *
> >   * @off: register offset
> >   * @bit: reset bit
> > + * @monbit: monitor bit in CPG_RST_MON register, -1 if none
> >   */
> >  struct rzg2l_reset {
> >         u16 off;
> >         u8 bit;
> > +       s8 monbit;
> >  };
> >
> > -#define DEF_RST(_id, _off, _bit)       \
> > +#define DEF_RST_MON(_id, _off, _bit, _monbit)  \
> >         [_id] = { \
> >                 .off = (_off), \
> > -               .bit = (_bit) \
> > +               .bit = (_bit), \
> > +               .monbit = (_monbit) \
> >         }
> > +#define DEF_RST(_id, _off, _bit)       \
> > +       DEF_RST_MON(_id, _off, _bit, -1)
> >
> >  /**
> >   * struct rzg2l_cpg_info - SoC-specific CPG Description

Thanks
Phil
diff mbox series

Patch

diff --git a/drivers/clk/renesas/rzg2l-cpg.c b/drivers/clk/renesas/rzg2l-cpg.c
index c357b0bfa119..220955366538 100644
--- a/drivers/clk/renesas/rzg2l-cpg.c
+++ b/drivers/clk/renesas/rzg2l-cpg.c
@@ -748,8 +748,12 @@  static int rzg2l_cpg_status(struct reset_controller_dev *rcdev,
 	const struct rzg2l_cpg_info *info = priv->info;
 	unsigned int reg = info->resets[id].off;
 	u32 bitmask = BIT(info->resets[id].bit);
+	u32 monbitmask = BIT(info->resets[id].monbit);
 
-	return !(readl(priv->base + CLK_MRST_R(reg)) & bitmask);
+	if (info->has_clk_mon_regs)
+		return !(readl(priv->base + CLK_MRST_R(reg)) & bitmask);
+	else
+		return !!(readl(priv->base + CPG_RST_MON) & monbitmask);
 }
 
 static const struct reset_control_ops rzg2l_cpg_reset_ops = {
diff --git a/drivers/clk/renesas/rzg2l-cpg.h b/drivers/clk/renesas/rzg2l-cpg.h
index f04671376af5..d1d08669066b 100644
--- a/drivers/clk/renesas/rzg2l-cpg.h
+++ b/drivers/clk/renesas/rzg2l-cpg.h
@@ -18,6 +18,7 @@ 
 #define CPG_PL3_SSEL		(0x408)
 #define CPG_PL6_SSEL		(0x414)
 #define CPG_PL6_ETH_SSEL	(0x418)
+#define CPG_RST_MON		(0x680)
 
 #define CPG_CLKSTATUS_SELSDHI0_STS	BIT(28)
 #define CPG_CLKSTATUS_SELSDHI1_STS	BIT(29)
@@ -151,17 +152,22 @@  struct rzg2l_mod_clk {
  *
  * @off: register offset
  * @bit: reset bit
+ * @monbit: monitor bit in CPG_RST_MON register, -1 if none
  */
 struct rzg2l_reset {
 	u16 off;
 	u8 bit;
+	s8 monbit;
 };
 
-#define DEF_RST(_id, _off, _bit)	\
+#define DEF_RST_MON(_id, _off, _bit, _monbit)	\
 	[_id] = { \
 		.off = (_off), \
-		.bit = (_bit) \
+		.bit = (_bit), \
+		.monbit = (_monbit) \
 	}
+#define DEF_RST(_id, _off, _bit)	\
+	DEF_RST_MON(_id, _off, _bit, -1)
 
 /**
  * struct rzg2l_cpg_info - SoC-specific CPG Description