Hi Geert,
> -----Original Message-----
> From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> Sent: 28 March 2025 17:11
> Subject: Re: [PATCH v7 07/18] can: rcar_canfd: Add rnc_stride variable to struct rcar_canfd_hw_info
>
> Hi Biju,
>
> On Fri, 28 Mar 2025 at 17:46, Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > > From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> On Fri, 28 Mar 2025
> > > at 16:39, Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > > > By introducing setrnc(), this patch is no more needed as
> > > > rnc_stride is local variable inside strnc(). So I would like to
> > > > drop this patch in next version.
> > > >
> > > > [1]
> > > > static void rcar_canfd_setrnc(struct rcar_canfd_global *gpriv, u32 ch,
> > > > int num_rules) {
> > > > u32 shift, rnc, offset, w, rnc_stride;
> > > >
> > > > rnc_stride = 32 / gpriv->info->rnc_field_width;
> > > > shift = 32 - ((ch % rnc_stride + 1) * gpriv->info->rnc_field_width);
> > > > rnc = (num_rules & (gpriv->info->num_supported_rules - 1)) << shift;
> > > > w = ch / rnc_stride;
> > > > offset = RCANFD_GAFLCFG(w);
> > > > rcar_canfd_set_bit(gpriv->base, offset, rnc); }
> > >
> > > Nice!
> > >
> > > Please combine variable declaration and assignment.
> > > While at it, perhaps "unsigned int ch" and "unsigned int num_rules"?
> > > Actually about everything above should be unsigned int, except rnc.
> > >
> > > I know this existed before, but do we need
> > >
> > > num_rules & (gpriv->info->num_supported_rules - 1)
> > >
> > > ? That "&" only works as long as num_supported_rules is a power of
> > > two, and I think num_rules can never be larger than num_supported_rules anyway.
> >
> > But this will make sure it fits into field_width for num_rules.
>
> Can it ever not fit?
No. ATM, we set num_rules = 1. So, it is safe to drop "num_supported_rules".
Cheers,
Biju
