Re: [PATCH net-next v2 10/10] net: ethernet: adi: Add a driver for the ADIN1140 MACPHY

[Andrew Lunn]

Hi Nuno

> > +static void adin1140_stats_work(struct work_struct *work)
> > +{
> > +	struct delayed_work *dwork = to_delayed_work(work);
> > +	u64 stat_buff[ADIN1140_STATS_CNT] = {};
> > +	struct adin1140_priv *priv;
> > +	u32 reg_val;
> > +	int ret;
> > +	u32 i;
> > +
> > +	priv = container_of(dwork, struct adin1140_priv, stats_work);
> > +
> > +	for (i = 0; i < ARRAY_SIZE(adin1140_stat_regs); i++) {
> > +		ret = oa_tc6_read_register(priv->tc6, adin1140_stat_regs[i],
> > +					   &reg_val);
> > +		if (ret)
> > +			break;
> > +
> > +		stat_buff[i] = reg_val;
> > +	}
> > +
> > +	spin_lock(&priv->stat_lock);
> 
> Maybe consider using scoped_guard() and similar for other places?
> Marginal win though so up to you.

Please trim the text when replying so just the needed context is
provided. It is easy to miss comments when you need to repeatedly page
down, page down, page down to find something.

> > +	ret = register_netdev(netdev);
> > +	if (ret) {
> > +		dev_err(&spi->dev, "Failed to register netdev (%d)", ret);
> 
> If we go to devm, this could be return dev_err_probe().

dev_err_probe() is not really about devm, but handling EPROBE_DEFFER,
and not issues an error message when it is not wanted. I don't think
register_netdev() can return EPROBE_DEFFER, so it probably does not
apply here.

    Andrew