Hi Geert,
> -----Original Message-----
> From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> Sent: 28 March 2025 16:03
> 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 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.
You mean drop "num_supported_rules" and use mum_rules directly ??
rnc = num_rules << shift;
Cheers,
Biju
