On Wed, 2014-01-29 at 10:00 +0400, Max Filippov wrote: > The following methods are implemented: > - get/set settings; > - get registers length/registers; > - get link state (standard implementation); > - get/set ring parameters; > - get timestamping info (standard implementation). [...] > --- a/drivers/net/ethernet/ethoc.c > +++ b/drivers/net/ethernet/ethoc.c > [...] > +static int ethoc_get_settings(struct net_device *dev, struct ethtool_cmd > *cmd) > +{ > + struct ethoc *priv = netdev_priv(dev); > + struct phy_device *phydev = priv->phy; > + > + if (!phydev) > + return -ENODEV; > + > + return phy_ethtool_gset(phydev, cmd); > +} > + > +static int ethoc_set_settings(struct net_device *dev, struct ethtool_cmd > *cmd) > +{ > + struct ethoc *priv = netdev_priv(dev); > + struct phy_device *phydev = priv->phy; > + > + if (!phydev) > + return -ENODEV; > + > + return phy_ethtool_sset(phydev, cmd); > +}
I think these should return -EOPNOTSUPP in the PHY-less case, just as if the set_settings pointer was not set. [...] > +static void ethoc_get_ringparam(struct net_device *dev, > + struct ethtool_ringparam *ring) > +{ > + struct ethoc *priv = netdev_priv(dev); > + > + ring->rx_max_pending = priv->num_bd; > + ring->rx_mini_max_pending = 0; > + ring->rx_jumbo_max_pending = 0; > + ring->tx_max_pending = priv->num_bd; > + > + ring->rx_pending = priv->num_rx; > + ring->rx_mini_pending = 0; > + ring->rx_jumbo_pending = 0; > + ring->tx_pending = priv->num_tx; > +} > + > +static int ethoc_set_ringparam(struct net_device *dev, > + struct ethtool_ringparam *ring) > +{ > + struct ethoc *priv = netdev_priv(dev); > + > + if (netif_running(dev)) > + return -EBUSY; This is unhelpful. Most implementations of this operation will restart the interface if it's running. > + priv->num_tx = rounddown_pow_of_two(ring->tx_pending); Range check? > + priv->num_rx = priv->num_bd - priv->num_tx; > + if (priv->num_rx > ring->rx_pending) > + priv->num_rx = ring->rx_pending; So the RX ring may only ever be shrunk?! Did you mean to compare with priv->num_bd instead? > + return 0; > +} > + > +const struct ethtool_ops ethoc_ethtool_ops = { static > + .get_settings = ethoc_get_settings, > + .set_settings = ethoc_set_settings, > + .get_regs_len = ethoc_get_regs_len, > + .get_regs = ethoc_get_regs, > + .get_link = ethtool_op_get_link, > + .get_ringparam = ethoc_get_ringparam, > + .set_ringparam = ethoc_set_ringparam, > + .get_ts_info = ethtool_op_get_ts_info, > +}; [...] -- Ben Hutchings It is a miracle that curiosity survives formal education. - Albert Einstein
signature.asc
Description: This is a digitally signed message part