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?
> You mean drop "num_supported_rules" and use mum_rules directly ??
>
> rnc = num_rules << shift;
Exactly.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
