diff mbox series

[v6,11/12] irqchip/renesas-rzv2h: Update macros ICU_TSSR_TSSEL_{MASK,PREP}

Message ID 20250224131253.134199-12-biju.das.jz@bp.renesas.com (mailing list archive)
State New
Delegated to: Geert Uytterhoeven
Headers show
Series Add Support for RZ/G3E ICU | expand

Commit Message

Biju Das Feb. 24, 2025, 1:11 p.m. UTC 
On RZ/G3E, TSSEL register field is 8 bits wide compared to 7 on RZ/V2H.
Also bits 8..14 is reserved on RZ/G3E and any writes on these reserved
bits is ignored. Use the bitmask GENMASK(field_width - 2, 0) on both SoCs
for extracting TSSEL and then update the macros ICU_TSSR_TSSEL_PREP and
ICU_TSSR_TSSEL_MASK for supporting both SoCs.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v5->v6:
 * Dropped Rb tag from Geert as it retain macros instead of dropping it.
 * Retained the macros  ICU_TSSR_TSSEL_PREP and ICU_TSSR_TSSEL_MASK by 
   adding field_width parameter.
v4->v5:
 * Used tssr_shift_factor in rzv2h_tint_set_type to optimize the
   calculation.
 * Dropped unnecessary parenthesis for calculating tssr.
 * Added Rb tag from Geert.
v4:
 * New patch
---
 drivers/irqchip/irq-renesas-rzv2h.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

Comments

Geert Uytterhoeven Feb. 24, 2025, 1:51 p.m. UTC | #1 
Hi Biju,

On Mon, 24 Feb 2025 at 14:13, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> On RZ/G3E, TSSEL register field is 8 bits wide compared to 7 on RZ/V2H.
> Also bits 8..14 is reserved on RZ/G3E and any writes on these reserved
> bits is ignored. Use the bitmask GENMASK(field_width - 2, 0) on both SoCs
> for extracting TSSEL and then update the macros ICU_TSSR_TSSEL_PREP and
> ICU_TSSR_TSSEL_MASK for supporting both SoCs.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
> v5->v6:
>  * Dropped Rb tag from Geert as it retain macros instead of dropping it.
>  * Retained the macros  ICU_TSSR_TSSEL_PREP and ICU_TSSR_TSSEL_MASK by
>    adding field_width parameter.
> v4->v5:
>  * Used tssr_shift_factor in rzv2h_tint_set_type to optimize the
>    calculation.
>  * Dropped unnecessary parenthesis for calculating tssr.
>  * Added Rb tag from Geert.

Thanks for the updates!

> --- a/drivers/irqchip/irq-renesas-rzv2h.c
> +++ b/drivers/irqchip/irq-renesas-rzv2h.c
> @@ -64,8 +64,13 @@
>  #define ICU_TINT_LEVEL_HIGH                    2
>  #define ICU_TINT_LEVEL_LOW                     3
>
> -#define ICU_TSSR_TSSEL_PREP(tssel, n)          ((tssel) << ((n) * 8))
> -#define ICU_TSSR_TSSEL_MASK(n)                 ICU_TSSR_TSSEL_PREP(0x7F, n)
> +#define ICU_TSSR_TSSEL_PREP(tssel, n, f_width) ((tssel) << ((n) * (f_width)))
> +#define ICU_TSSR_TSSEL_MASK(n, _field_width)   \

Nit: please use f_width or field_width consistently.

> +({\
> +               typeof(_field_width) (field_width) = (_field_width); \
> +               ICU_TSSR_TSSEL_PREP((GENMASK(((field_width) - 2), 0)), (n), field_width); \
> +})
> +
>  #define ICU_TSSR_TIEN(n, _field_width) \
>  ({\
>                 typeof(_field_width) (field_width) = (_field_width); \

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert
Biju Das Feb. 24, 2025, 1:59 p.m. UTC | #2 
Hi Geert,

Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: 24 February 2025 13:52
> Subject: Re: [PATCH v6 11/12] irqchip/renesas-rzv2h: Update macros ICU_TSSR_TSSEL_{MASK,PREP}
> 
> Hi Biju,
> 
> On Mon, 24 Feb 2025 at 14:13, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > On RZ/G3E, TSSEL register field is 8 bits wide compared to 7 on RZ/V2H.
> > Also bits 8..14 is reserved on RZ/G3E and any writes on these reserved
> > bits is ignored. Use the bitmask GENMASK(field_width - 2, 0) on both
> > SoCs for extracting TSSEL and then update the macros
> > ICU_TSSR_TSSEL_PREP and ICU_TSSR_TSSEL_MASK for supporting both SoCs.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> > v5->v6:
> >  * Dropped Rb tag from Geert as it retain macros instead of dropping it.
> >  * Retained the macros  ICU_TSSR_TSSEL_PREP and ICU_TSSR_TSSEL_MASK by
> >    adding field_width parameter.
> > v4->v5:
> >  * Used tssr_shift_factor in rzv2h_tint_set_type to optimize the
> >    calculation.
> >  * Dropped unnecessary parenthesis for calculating tssr.
> >  * Added Rb tag from Geert.
> 
> Thanks for the updates!
> 
> > --- a/drivers/irqchip/irq-renesas-rzv2h.c
> > +++ b/drivers/irqchip/irq-renesas-rzv2h.c
> > @@ -64,8 +64,13 @@
> >  #define ICU_TINT_LEVEL_HIGH                    2
> >  #define ICU_TINT_LEVEL_LOW                     3
> >
> > -#define ICU_TSSR_TSSEL_PREP(tssel, n)          ((tssel) << ((n) * 8))
> > -#define ICU_TSSR_TSSEL_MASK(n)                 ICU_TSSR_TSSEL_PREP(0x7F, n)
> > +#define ICU_TSSR_TSSEL_PREP(tssel, n, f_width) ((tssel) << ((n) * (f_width)))
> > +#define ICU_TSSR_TSSEL_MASK(n, _field_width)   \
> 
> Nit: please use f_width or field_width consistently.

With field_width there was some alignment issue, that is
the reason it is shortened to f_width.

If there are no other comments for this patch, I will switch using
field_width in the next version.


Cheers,
Biju
Thomas Gleixner Feb. 24, 2025, 3:53 p.m. UTC | #3 
On Mon, Feb 24 2025 at 13:59, Biju Das wrote:
>> From: Geert Uytterhoeven <geert@linux-m68k.org>
>> > -#define ICU_TSSR_TSSEL_PREP(tssel, n)          ((tssel) << ((n) * 8))
>> > -#define ICU_TSSR_TSSEL_MASK(n)                 ICU_TSSR_TSSEL_PREP(0x7F, n)
>> > +#define ICU_TSSR_TSSEL_PREP(tssel, n, f_width) ((tssel) << ((n) * (f_width)))
>> > +#define ICU_TSSR_TSSEL_MASK(n, _field_width)   \
>> 
>> Nit: please use f_width or field_width consistently.
>
> With field_width there was some alignment issue, that is
> the reason it is shortened to f_width.
>
> If there are no other comments for this patch, I will switch using
> field_width in the next version.

If this is the only change, then please just post an updated version of
_this_ patch (11/12) as a reply to the patch itself. No need to resend
the whole series for that cosmetic change,

Thanks,

        tglx
Biju Das Feb. 24, 2025, 4:40 p.m. UTC | #4 
Hi Thomas,

> -----Original Message-----
> From: Thomas Gleixner <tglx@linutronix.de>
> Sent: 24 February 2025 15:53
> Subject: RE: [PATCH v6 11/12] irqchip/renesas-rzv2h: Update macros ICU_TSSR_TSSEL_{MASK,PREP}
> 
> On Mon, Feb 24 2025 at 13:59, Biju Das wrote:
> >> From: Geert Uytterhoeven <geert@linux-m68k.org>
> >> > -#define ICU_TSSR_TSSEL_PREP(tssel, n)          ((tssel) << ((n) * 8))
> >> > -#define ICU_TSSR_TSSEL_MASK(n)                 ICU_TSSR_TSSEL_PREP(0x7F, n)
> >> > +#define ICU_TSSR_TSSEL_PREP(tssel, n, f_width) ((tssel) << ((n) * (f_width)))
> >> > +#define ICU_TSSR_TSSEL_MASK(n, _field_width)   \
> >>
> >> Nit: please use f_width or field_width consistently.
> >
> > With field_width there was some alignment issue, that is the reason it
> > is shortened to f_width.
> >
> > If there are no other comments for this patch, I will switch using
> > field_width in the next version.
> 
> If this is the only change, then please just post an updated version of _this_ patch (11/12) as a
> reply to the patch itself. No need to resend the whole series for that cosmetic change,

I will send both 10/12 and 11/12 with replacing _field_width->field_width and
f_width-> field_width as macro parameters as reply to respective patches

But patch#10 needs updating commit description _field_width->field_width.

Cheers,
Biju
Biju Das Feb. 24, 2025, 4:51 p.m. UTC | #5 
Hi Thomas,

Updated patch#11 by using field_width as macro parameter. 

Cheers,
Biju

> -----Original Message-----
> From: Biju Das
> Sent: 24 February 2025 16:41
> To: Thomas Gleixner <tglx@linutronix.de>; Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>; biju.das.au
> <biju.das.au@gmail.com>; linux-renesas-soc@vger.kernel.org
> Subject: RE: [PATCH v6 11/12] irqchip/renesas-rzv2h: Update macros ICU_TSSR_TSSEL_{MASK,PREP}
> 
> Hi Thomas,
> 
> > -----Original Message-----
> > From: Thomas Gleixner <tglx@linutronix.de>
> > Sent: 24 February 2025 15:53
> > Subject: RE: [PATCH v6 11/12] irqchip/renesas-rzv2h: Update macros
> > ICU_TSSR_TSSEL_{MASK,PREP}
> >
> > On Mon, Feb 24 2025 at 13:59, Biju Das wrote:
> > >> From: Geert Uytterhoeven <geert@linux-m68k.org>
> > >> > -#define ICU_TSSR_TSSEL_PREP(tssel, n)          ((tssel) << ((n) * 8))
> > >> > -#define ICU_TSSR_TSSEL_MASK(n)                 ICU_TSSR_TSSEL_PREP(0x7F, n)
> > >> > +#define ICU_TSSR_TSSEL_PREP(tssel, n, f_width) ((tssel) << ((n) * (f_width)))
> > >> > +#define ICU_TSSR_TSSEL_MASK(n, _field_width)   \
> > >>
> > >> Nit: please use f_width or field_width consistently.
> > >
> > > With field_width there was some alignment issue, that is the reason
> > > it is shortened to f_width.
> > >
> > > If there are no other comments for this patch, I will switch using
> > > field_width in the next version.
> >
> > If this is the only change, then please just post an updated version
> > of _this_ patch (11/12) as a reply to the patch itself. No need to
> > resend the whole series for that cosmetic change,

diff --git a/drivers/irqchip/irq-renesas-rzv2h.c b/drivers/irqchip/irq-renesas-rzv2h.c
index 8d0bd4d69de2..7bc4397ec149 100644
--- a/drivers/irqchip/irq-renesas-rzv2h.c
+++ b/drivers/irqchip/irq-renesas-rzv2h.c
@@ -64,8 +64,13 @@
 #define ICU_TINT_LEVEL_HIGH			2
 #define ICU_TINT_LEVEL_LOW			3
 
-#define ICU_TSSR_TSSEL_PREP(tssel, n)		((tssel) << ((n) * 8))
-#define ICU_TSSR_TSSEL_MASK(n)			ICU_TSSR_TSSEL_PREP(0x7F, n)
+#define ICU_TSSR_TSSEL_PREP(tssel, n, field_width)	((tssel) << ((n) * (field_width)))
+#define ICU_TSSR_TSSEL_MASK(n, field_width)	\
+({\
+		typeof(field_width) (_field_width) = (field_width); \
+		ICU_TSSR_TSSEL_PREP((GENMASK(((_field_width) - 2), 0)), (n), _field_width); \
+})
+
 #define ICU_TSSR_TIEN(n, field_width)	\
 ({\
 		typeof(field_width) (_field_width) = (field_width); \
@@ -326,8 +331,8 @@ static int rzv2h_tint_set_type(struct irq_data *d, unsigned int type)
 	guard(raw_spinlock)(&priv->lock);
 
 	tssr = readl_relaxed(priv->base + priv->info->t_offs + ICU_TSSR(tssr_k));
-	tssr &= ~(ICU_TSSR_TSSEL_MASK(tssel_n) | tien);
-	tssr |= ICU_TSSR_TSSEL_PREP(tint, tssel_n);
+	tssr &= ~(ICU_TSSR_TSSEL_MASK(tssel_n, priv->info->field_width) | tien);
+	tssr |= ICU_TSSR_TSSEL_PREP(tint, tssel_n, priv->info->field_width);
 
 	writel_relaxed(tssr, priv->base + priv->info->t_offs + ICU_TSSR(tssr_k));
diff mbox series

Patch

diff --git a/drivers/irqchip/irq-renesas-rzv2h.c b/drivers/irqchip/irq-renesas-rzv2h.c
index ac71ce9810f8..351303839636 100644
--- a/drivers/irqchip/irq-renesas-rzv2h.c
+++ b/drivers/irqchip/irq-renesas-rzv2h.c
@@ -64,8 +64,13 @@ 
 #define ICU_TINT_LEVEL_HIGH			2
 #define ICU_TINT_LEVEL_LOW			3
 
-#define ICU_TSSR_TSSEL_PREP(tssel, n)		((tssel) << ((n) * 8))
-#define ICU_TSSR_TSSEL_MASK(n)			ICU_TSSR_TSSEL_PREP(0x7F, n)
+#define ICU_TSSR_TSSEL_PREP(tssel, n, f_width)	((tssel) << ((n) * (f_width)))
+#define ICU_TSSR_TSSEL_MASK(n, _field_width)	\
+({\
+		typeof(_field_width) (field_width) = (_field_width); \
+		ICU_TSSR_TSSEL_PREP((GENMASK(((field_width) - 2), 0)), (n), field_width); \
+})
+
 #define ICU_TSSR_TIEN(n, _field_width)	\
 ({\
 		typeof(_field_width) (field_width) = (_field_width); \
@@ -326,8 +331,8 @@  static int rzv2h_tint_set_type(struct irq_data *d, unsigned int type)
 	guard(raw_spinlock)(&priv->lock);
 
 	tssr = readl_relaxed(priv->base + priv->info->t_offs + ICU_TSSR(tssr_k));
-	tssr &= ~(ICU_TSSR_TSSEL_MASK(tssel_n) | tien);
-	tssr |= ICU_TSSR_TSSEL_PREP(tint, tssel_n);
+	tssr &= ~(ICU_TSSR_TSSEL_MASK(tssel_n, priv->info->field_width) | tien);
+	tssr |= ICU_TSSR_TSSEL_PREP(tint, tssel_n, priv->info->field_width);
 
 	writel_relaxed(tssr, priv->base + priv->info->t_offs + ICU_TSSR(tssr_k));