Hi Geert,
> -----Original Message-----
> From: Biju Das
> Sent: 17 February 2025 11:08
> Subject: RE: [PATCH v5 10/12] irqchip/renesas-rzv2h: Drop TSSR_TIEN macro
>
>
>
> > -----Original Message-----
> > From: Biju Das
> > Sent: 17 February 2025 11:03
> > Subject: RE: [PATCH v5 10/12] irqchip/renesas-rzv2h: Drop TSSR_TIEN
> > macro
> >
> > Hi Geert,
> >
> > Thanks for the feedback.
> >
> > > -----Original Message-----
> > > From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> > > Sent: 17 February 2025 10:47
> > > Subject: Re: [PATCH v5 10/12] irqchip/renesas-rzv2h: Drop TSSR_TIEN
> > > macro
> > >
> > > Hi Biju,
> > >
> > > On Mon, 17 Feb 2025 at 10:36, Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > >
> > > > > -----Original Message-----
> > > > > From: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > > > > Sent: 12 February 2025 11:12
> > > > > Subject: [PATCH v5 10/12] irqchip/renesas-rzv2h: Drop TSSR_TIEN
> > > > > macro
> > > > >
> > > > > On RZ/G3E, TIEN bit position is at 15 compared to 7 on RZ/V2H.
> > > > > The macro
> > > > > ICU_TSSR_TIEN(n) can be replaced with the inline logic
> > > > > BIT(field_width - 1) << (n * fieldwidth) for supporting both SoCs.
> > > > >
> > > > > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > > > > ---
> > > > > v4->v5:
> > > > > * Shortened tssr calculation in rzv2h_tint_irq_endisable().
> > > > > * Added tssr_shift_factor variable for optimizing the calculation
> > > > > in rzv2h_tint_set_type() as the next patch uses the same factor.
> > > > > v4:
> > > > > * New patch
> > > > > ---
> > > > > drivers/irqchip/irq-renesas-rzv2h.c | 9 +++++----
> > > > > 1 file changed, 5 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/drivers/irqchip/irq-renesas-rzv2h.c
> > > > > b/drivers/irqchip/irq-renesas-rzv2h.c
> > > > > index 98a6a7cd3611..3635597ae4c1 100644
> > > > > --- a/drivers/irqchip/irq-renesas-rzv2h.c
> > > > > +++ b/drivers/irqchip/irq-renesas-rzv2h.c
> > > > > @@ -66,7 +66,6 @@
> > > > >
> > > > > #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_TIEN(n) (BIT(7) << ((n) * 8))
> > > >
> > > >
> > > > Please let me know, instead I should retain this macro with[1] and see changes below inlined??
> > >
> > > What does [1] refer to?
> >
> > Oops [1] refers to
> >
> > [1]
> >
> > #define ICU_TSSR_TIEN(mask, n, shift) ((mask) << ((n) * (shift)))
> >
> >
> > >
> > > > #define ICU_TSSR_TIEN(mask, n, shift) ((mask) << ((n) * (shift)))
> > >
> > > Isn't "mask" always "BIT(shift -1)"?
> >
> > That is correct.
> >
> > >
> > > "shift" is not the shift value (that is "n * shift"), but the field width.
> >
> > Ok, Good point, now it can be shortened as
> >
> > #define ICU_TSSR_TIEN(field_width, n) (BIT((field_width) - 1) << ((n)
> > * (field_width) - 1))
>
> Typo. Correct one is
>
> #define ICU_TSSR_TIEN(field_width, n) (BIT((field_width) - 1) << ((n) * (field_width))
Finally corrected this macro as below to fix the Warnings/error [2]
#define ICU_TSSR_TIEN(_field_width, n) \
({\
typeof(_field_width) (field_width) = (_field_width); \
BIT((field_width) - 1) << ((n) * (field_width)); \
})
[2]
ERROR: Macros with complex values should be enclosed in parentheses
#24: FILE: drivers/irqchip/irq-renesas-rzv2h.c:69:
+#define ICU_TSSR_TIEN(f_width, n) BIT((f_width) - 1) << ((n) * (f_width))
CHECK: Macro argument reuse 'f_width' - possible side-effects?
#24: FILE: drivers/irqchip/irq-renesas-rzv2h.c:69:
+#define ICU_TSSR_TIEN(f_width, n) BIT((f_width) - 1) << ((n) * (f_width))
total: 1 errors, 0 warnings, 1 checks, 27 lines checked
Cheers,
Biju
