Re: [PATCH 1/1] ethtool : added get_phy_stats,get_strings,get_sset_count

[Andrew Lunn <andrew@lunn.ch>]

> >  static int cpsw_get_sset_count(struct net_device *ndev, int sset)
> >  {
> > +	struct cpsw_priv *priv = netdev_priv(ndev);
> > +	int slave_no = cpsw_slave_index(priv);
> > +	int count;
> > +
> >  	switch (sset) {
> >  	case ETH_SS_STATS:
> > -		return CPSW_STATS_LEN;
> > +		count = CPSW_STATS_LEN;
> > +		count += phy_ethtool_get_sset_count(priv->slaves[slave_no].phy,
> > +						    sset);
> > +		return count;
> >  	default:
> >  		return -EOPNOTSUPP;
> >  	}
> 
> We already have a way to obtain PHY specific statistics through the
> ETH_SS_PHY_STATS string set, cannot we use that here too? It certainly
> makes it easier to overlay cpsw statistics with PHY device statistics,
> but when Andrew added support for that, AFAIR this was made
> intentionally separate.

I don't particularly like this. It makes the cpsw driver different to
all other drivers, in that it returns both MAC and PHY statistics.

If you are going to do this, please do it a higher level so that it
applies to all Ethernet drivers, not just cpsw.

But personally, i don't see the point of this change. What is wrong
with ethtool --phy-statistics?

     Andrew