Hi Vincent,
> -----Original Message-----
> From: Vincent Mailhol <mailhol.vincent@xxxxxxxxxx>
> Sent: 28 March 2025 11:06
> To: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>; Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx>
> Cc: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>; Geert Uytterhoeven <geert+renesas@xxxxxxxxx>; Uwe
> Kleine-König <u.kleine-koenig@xxxxxxxxxxxx>; linux-can@xxxxxxxxxxxxxxx; Prabhakar Mahadev Lad
> <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>; biju.das.au <biju.das.au@xxxxxxxxx>; linux-renesas-
> soc@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v7 09/18] can: rcar_canfd: Add max_aflpn variable to struct rcar_canfd_hw_info
>
> On 26/03/2025 at 21:19, Biju Das wrote:
> > R-Car Gen3 has maximum acceptance filter list page number of 31
> > whereas on R-Car Gen4 it is 127. Add max_aflpn variable to struct
> > rcar_canfd_hw_info in order to support RZ/G3E that has max AFLPN of 63.
> >
> > Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
>
> With below nitpick addressed, you can add my:
>
> Reviewed-by: Vincent Mailhol <mailhol.vincent@xxxxxxxxxx>
>
> > ---
> > v6->v7:
> > * Collected tag.
> > v6:
> > * New patch.
> > ---
> > drivers/net/can/rcar/rcar_canfd.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/can/rcar/rcar_canfd.c
> > b/drivers/net/can/rcar/rcar_canfd.c
> > index 62cde1efa0c0..7cef0673fbc8 100644
> > --- a/drivers/net/can/rcar/rcar_canfd.c
> > +++ b/drivers/net/can/rcar/rcar_canfd.c
> > @@ -93,7 +93,7 @@
> >
> > /* RSCFDnCFDGAFLECTR / RSCFDnGAFLECTR */
> > #define RCANFD_GAFLECTR_AFLDAE BIT(8)
> > -#define RCANFD_GAFLECTR_AFLPN(gpriv, x) ((x) & reg_gen4(gpriv, 0x7f, 0x1f))
> > +#define RCANFD_GAFLECTR_AFLPN(gpriv, x) ((x) & (gpriv)->info->max_aflpn)
> ^ While at it, can you rename that x to page_num in the next
> version? I don't mind if you do it in the same patch.
>
> I understand it was here before, but that's will be a nice quality of life improvement.
>
> Please do the same for the other macro which you are modifying in this series (I am not asking you to
> rewrite the full driver, so only do local improvement on the parts you are touching).
Is it ok, if I send a separate patch for that conversion for all macros
which will avoid inconsistency in the driver as some macros will have x and
some macros with meaning full name?
Cheers,
Biju
>
> > /* RSCFDnCFDGAFLIDj / RSCFDnGAFLIDj */
> > #define RCANFD_GAFLID_GAFLLB BIT(29)
> > @@ -507,6 +507,7 @@ struct rcar_canfd_hw_info {
> > u16 num_supported_rules;
> > u8 rnc_stride;
> > u8 rnc_field_width;
> > + u8 max_aflpn;
> > u8 max_channels;
> > u8 postdiv;
> > /* hardware features */
> > @@ -586,6 +587,7 @@ static const struct rcar_canfd_hw_info rcar_gen3_hw_info = {
> > .num_supported_rules = 256,
> > .rnc_stride = 4,
> > .rnc_field_width = 8,
> > + .max_aflpn = 31,
> > .max_channels = 2,
> > .postdiv = 2,
> > .shared_global_irqs = 1,
> > @@ -595,6 +597,7 @@ static const struct rcar_canfd_hw_info rcar_gen4_hw_info = {
> > .num_supported_rules = 512,
> > .rnc_stride = 2,
> > .rnc_field_width = 16,
> > + .max_aflpn = 127,
> > .max_channels = 8,
> > .postdiv = 2,
> > .shared_global_irqs = 1,
> > @@ -604,6 +607,7 @@ static const struct rcar_canfd_hw_info rzg2l_hw_info = {
> > .num_supported_rules = 256,
> > .rnc_stride = 4,
> > .rnc_field_width = 8,
> > + .max_aflpn = 31,
> > .max_channels = 2,
> > .postdiv = 1,
> > .multi_channel_irqs = 1,
>
>
> Yours sincerely,
> Vincent Mailhol
