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.
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
