Re: [PATCH] stmmac: selftest: avoid large stack usage

[Arnd Bergmann]

On Thu, Sep 19, 2019 at 9:58 AM Jose Abreu <Jose.Abreu@xxxxxxxxxxxx> wrote:
>
> From: Arnd Bergmann <arnd@xxxxxxxx>
> Date: Sep/18/2019, 20:54:34 (UTC+00:00)
>
> > +     if (!cfg || !cfg->enable) {
> >               value &= ~XGMAC_RSSE;
> >               writel(value, ioaddr + XGMAC_RSS_CTRL);
> >               return 0;
> >       }
> >
> >       for (i = 0; i < (sizeof(cfg->key) / sizeof(u32)); i++) {
> > -             ret = dwxgmac2_rss_write_reg(ioaddr, true, i, *key++);
> > +             if (cfg)
> > +                     ret = dwxgmac2_rss_write_reg(ioaddr, true, i, cfg->key[i]);
> > +             else
> > +                     ret = dwxgmac2_rss_write_reg(ioaddr, true, i, 0);
> > +
> >               if (ret)
> >                       return ret;
> >       }
> >
> >       for (i = 0; i < ARRAY_SIZE(cfg->table); i++) {
> > -             ret = dwxgmac2_rss_write_reg(ioaddr, false, i, cfg->table[i]);
> > +             if (cfg)
> > +                     ret = dwxgmac2_rss_write_reg(ioaddr, false, i, cfg->table[i]);
> > +             else
> > +                     ret = dwxgmac2_rss_write_reg(ioaddr, false, i, 0);
> > +
>
> I don't get these "if (cfg)" checks. You already check earlier if cfg is
> NULL and return if so. I don't think you need these extra checks.

Ah, you are right, I missed the 'return 0', that makes it much easier.

> Also, your subject line should be something like: "net: stmmac:
> selftests: ..."

I think both styles is common for network drivers, though I think most
just leave out the 'net:'. I changed it in v2 now.

      Arnd