On Thu, Sep 19, 2019 at 9:58 AM Jose Abreu <jose.ab...@synopsys.com> wrote:
>
> From: Arnd Bergmann <a...@arndb.de>
> 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
