| Message ID | 20250208154350.75316-1-wejmanpm@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v2] net: e1000e: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set() | expand |
On 08/02/2025 15:43, Piotr Wejman wrote: > Update the driver to use the new hardware timestamping API added in commit > 66f7223039c0 ("net: add NDOs for configuring hardware timestamping"). > Use Netlink extack for error reporting in e1000e_hwtstamp_set. > Align the indentation of net_device_ops. > > Signed-off-by: Piotr Wejman <wejmanpm@gmail.com> > --- > Changes in v2: > - amend commit message > - use extack for error reporting > - rename e1000_mii_ioctl to e1000_ioctl > - Link to v1: https://lore.kernel.org/netdev/20250202170839.47375-1-piotrwejman90@gmail.com/ > Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
On Sat, 8 Feb 2025 16:43:49 +0100 Piotr Wejman wrote: > - if (!(adapter->flags & FLAG_HAS_HW_TIMESTAMP)) > + if (!(adapter->flags & FLAG_HAS_HW_TIMESTAMP)) { > + NL_SET_ERR_MSG(extack, "No HW timestamp support\n"); No new lines at the end of extack messages, please. User space adds its own line breaks.
On 2/8/2025 5:43 PM, Piotr Wejman wrote: > Update the driver to use the new hardware timestamping API added in commit > 66f7223039c0 ("net: add NDOs for configuring hardware timestamping"). > Use Netlink extack for error reporting in e1000e_hwtstamp_set. > Align the indentation of net_device_ops. > > Signed-off-by: Piotr Wejman <wejmanpm@gmail.com> > --- > Changes in v2: > - amend commit message > - use extack for error reporting > - rename e1000_mii_ioctl to e1000_ioctl > - Link to v1: https://lore.kernel.org/netdev/20250202170839.47375-1-piotrwejman90@gmail.com/ > > drivers/net/ethernet/intel/e1000e/e1000.h | 2 +- > drivers/net/ethernet/intel/e1000e/netdev.c | 68 ++++++++++------------ > 2 files changed, 31 insertions(+), 39 deletions(-) > > diff --git a/drivers/net/ethernet/intel/e1000e/e1000.h b/drivers/net/ethernet/intel/e1000e/e1000.h > index ba9c19e6994c..952898151565 100644 > --- a/drivers/net/ethernet/intel/e1000e/e1000.h > +++ b/drivers/net/ethernet/intel/e1000e/e1000.h > @@ -319,7 +319,7 @@ struct e1000_adapter { > u16 tx_ring_count; > u16 rx_ring_count; > > - struct hwtstamp_config hwtstamp_config; > + struct kernel_hwtstamp_config hwtstamp_config; > struct delayed_work systim_overflow_work; > struct sk_buff *tx_hwtstamp_skb; > unsigned long tx_hwtstamp_start; > diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c > index 286155efcedf..43933e64819b 100644 > --- a/drivers/net/ethernet/intel/e1000e/netdev.c > +++ b/drivers/net/ethernet/intel/e1000e/netdev.c > @@ -3574,6 +3574,7 @@ s32 e1000e_get_base_timinca(struct e1000_adapter *adapter, u32 *timinca) > * e1000e_config_hwtstamp - configure the hwtstamp registers and enable/disable > * @adapter: board private structure > * @config: timestamp configuration > + * @extack: netlink extended ACK for error report > * > * Outgoing time stamping can be enabled and disabled. Play nice and > * disable it when requested, although it shouldn't cause any overhead > @@ -3587,7 +3588,8 @@ s32 e1000e_get_base_timinca(struct e1000_adapter *adapter, u32 *timinca) > * exception of "all V2 events regardless of level 2 or 4". > **/ > static int e1000e_config_hwtstamp(struct e1000_adapter *adapter, > - struct hwtstamp_config *config) > + struct kernel_hwtstamp_config *config, > + struct netlink_ext_ack *extack) > { > struct e1000_hw *hw = &adapter->hw; > u32 tsync_tx_ctl = E1000_TSYNCTXCTL_ENABLED; > @@ -3598,8 +3600,10 @@ static int e1000e_config_hwtstamp(struct e1000_adapter *adapter, > bool is_l2 = false; > u32 regval; > > - if (!(adapter->flags & FLAG_HAS_HW_TIMESTAMP)) > + if (!(adapter->flags & FLAG_HAS_HW_TIMESTAMP)) { > + NL_SET_ERR_MSG(extack, "No HW timestamp support\n"); > return -EINVAL; > + } > > switch (config->tx_type) { > case HWTSTAMP_TX_OFF: > @@ -3608,6 +3612,7 @@ static int e1000e_config_hwtstamp(struct e1000_adapter *adapter, > case HWTSTAMP_TX_ON: > break; > default: > + NL_SET_ERR_MSG(extack, "Unsupported TX HW timestamp type\n"); > return -ERANGE; > } > > @@ -3681,6 +3686,7 @@ static int e1000e_config_hwtstamp(struct e1000_adapter *adapter, > config->rx_filter = HWTSTAMP_FILTER_ALL; > break; > default: > + NL_SET_ERR_MSG(extack, "Unsupported RX HW timestamp filter\n"); > return -ERANGE; > } > > @@ -3693,7 +3699,7 @@ static int e1000e_config_hwtstamp(struct e1000_adapter *adapter, > ew32(TSYNCTXCTL, regval); > if ((er32(TSYNCTXCTL) & E1000_TSYNCTXCTL_ENABLED) != > (regval & E1000_TSYNCTXCTL_ENABLED)) { > - e_err("Timesync Tx Control register not set as expected\n"); > + NL_SET_ERR_MSG(extack, "Timesync Tx Control register not set as expected\n"); In the case where this function is being called from e1000e_systim_reset function, won't it cause this debug print to do nothing? > return -EAGAIN; > } > > @@ -3706,7 +3712,7 @@ static int e1000e_config_hwtstamp(struct e1000_adapter *adapter, > E1000_TSYNCRXCTL_TYPE_MASK)) != > (regval & (E1000_TSYNCRXCTL_ENABLED | > E1000_TSYNCRXCTL_TYPE_MASK))) { > - e_err("Timesync Rx Control register not set as expected\n"); Same question here. > + NL_SET_ERR_MSG(extack, "Timesync Rx Control register not set as expected\n"); > return -EAGAIN; > } > > @@ -3932,7 +3938,7 @@ static void e1000e_systim_reset(struct e1000_adapter *adapter) > spin_unlock_irqrestore(&adapter->systim_lock, flags); > > /* restore the previous hwtstamp configuration settings */ > - e1000e_config_hwtstamp(adapter, &adapter->hwtstamp_config); > + e1000e_config_hwtstamp(adapter, &adapter->hwtstamp_config, NULL); > } > > /** > @@ -6079,8 +6085,8 @@ static int e1000_change_mtu(struct net_device *netdev, int new_mtu) > return 0; > } > > -static int e1000_mii_ioctl(struct net_device *netdev, struct ifreq *ifr, > - int cmd) > +static int e1000_ioctl(struct net_device *netdev, struct ifreq *ifr, > + int cmd) > { > struct e1000_adapter *adapter = netdev_priv(netdev); > struct mii_ioctl_data *data = if_mii(ifr); > @@ -6140,7 +6146,8 @@ static int e1000_mii_ioctl(struct net_device *netdev, struct ifreq *ifr, > /** > * e1000e_hwtstamp_set - control hardware time stamping > * @netdev: network interface device structure > - * @ifr: interface request > + * @config: timestamp configuration > + * @extack: netlink extended ACK report > * > * Outgoing time stamping can be enabled and disabled. Play nice and > * disable it when requested, although it shouldn't cause any overhead > @@ -6153,20 +6160,18 @@ static int e1000_mii_ioctl(struct net_device *netdev, struct ifreq *ifr, > * specified. Matching the kind of event packet is not supported, with the > * exception of "all V2 events regardless of level 2 or 4". > **/ > -static int e1000e_hwtstamp_set(struct net_device *netdev, struct ifreq *ifr) > +static int e1000e_hwtstamp_set(struct net_device *netdev, > + struct kernel_hwtstamp_config *config, > + struct netlink_ext_ack *extack) > { > struct e1000_adapter *adapter = netdev_priv(netdev); > - struct hwtstamp_config config; > int ret_val; > > - if (copy_from_user(&config, ifr->ifr_data, sizeof(config))) > - return -EFAULT; > - > - ret_val = e1000e_config_hwtstamp(adapter, &config); > + ret_val = e1000e_config_hwtstamp(adapter, config, extack); > if (ret_val) > return ret_val; > > - switch (config.rx_filter) { > + switch (config->rx_filter) { > case HWTSTAMP_FILTER_PTP_V2_L4_SYNC: > case HWTSTAMP_FILTER_PTP_V2_L2_SYNC: > case HWTSTAMP_FILTER_PTP_V2_SYNC: > @@ -6178,38 +6183,23 @@ static int e1000e_hwtstamp_set(struct net_device *netdev, struct ifreq *ifr) > * by hardware so notify the caller the requested packets plus > * some others are time stamped. > */ > - config.rx_filter = HWTSTAMP_FILTER_SOME; > + config->rx_filter = HWTSTAMP_FILTER_SOME; > break; > default: > break; > } > > - return copy_to_user(ifr->ifr_data, &config, > - sizeof(config)) ? -EFAULT : 0; > + return 0; > } > > -static int e1000e_hwtstamp_get(struct net_device *netdev, struct ifreq *ifr) > +static int e1000e_hwtstamp_get(struct net_device *netdev, > + struct kernel_hwtstamp_config *kernel_config) > { > struct e1000_adapter *adapter = netdev_priv(netdev); > > - return copy_to_user(ifr->ifr_data, &adapter->hwtstamp_config, > - sizeof(adapter->hwtstamp_config)) ? -EFAULT : 0; > -} > + *kernel_config = adapter->hwtstamp_config; > > -static int e1000_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd) > -{ > - switch (cmd) { > - case SIOCGMIIPHY: > - case SIOCGMIIREG: > - case SIOCSMIIREG: > - return e1000_mii_ioctl(netdev, ifr, cmd); > - case SIOCSHWTSTAMP: > - return e1000e_hwtstamp_set(netdev, ifr); > - case SIOCGHWTSTAMP: > - return e1000e_hwtstamp_get(netdev, ifr); > - default: > - return -EOPNOTSUPP; > - } > + return 0; > } > > static int e1000_init_phy_wakeup(struct e1000_adapter *adapter, u32 wufc) > @@ -7346,9 +7336,11 @@ static const struct net_device_ops e1000e_netdev_ops = { > #ifdef CONFIG_NET_POLL_CONTROLLER > .ndo_poll_controller = e1000_netpoll, > #endif > - .ndo_set_features = e1000_set_features, > - .ndo_fix_features = e1000_fix_features, > + .ndo_set_features = e1000_set_features, > + .ndo_fix_features = e1000_fix_features, > .ndo_features_check = passthru_features_check, > + .ndo_hwtstamp_get = e1000e_hwtstamp_get, > + .ndo_hwtstamp_set = e1000e_hwtstamp_set, > }; > > /** > Also you are missing a subject prefix, I assume that you mean to send it to iwl-next since it is not a bug fix. Please add it to your patch.
On Wed, Feb 12, 2025 at 3:10 PM Lifshits, Vitaly <vitaly.lifshits@intel.com> wrote: > > > > On 2/8/2025 5:43 PM, Piotr Wejman wrote: > > Update the driver to use the new hardware timestamping API added in commit > > 66f7223039c0 ("net: add NDOs for configuring hardware timestamping"). > > Use Netlink extack for error reporting in e1000e_hwtstamp_set. > > Align the indentation of net_device_ops. > > > > Signed-off-by: Piotr Wejman <wejmanpm@gmail.com> > > --- > > Changes in v2: > > - amend commit message > > - use extack for error reporting > > - rename e1000_mii_ioctl to e1000_ioctl > > - Link to v1: https://lore.kernel.org/netdev/20250202170839.47375-1-piotrwejman90@gmail.com/ > > > > drivers/net/ethernet/intel/e1000e/e1000.h | 2 +- > > drivers/net/ethernet/intel/e1000e/netdev.c | 68 ++++++++++------------ > > 2 files changed, 31 insertions(+), 39 deletions(-) > > > > diff --git a/drivers/net/ethernet/intel/e1000e/e1000.h b/drivers/net/ethernet/intel/e1000e/e1000.h > > index ba9c19e6994c..952898151565 100644 > > --- a/drivers/net/ethernet/intel/e1000e/e1000.h > > +++ b/drivers/net/ethernet/intel/e1000e/e1000.h > > @@ -319,7 +319,7 @@ struct e1000_adapter { > > u16 tx_ring_count; > > u16 rx_ring_count; > > > > - struct hwtstamp_config hwtstamp_config; > > + struct kernel_hwtstamp_config hwtstamp_config; > > struct delayed_work systim_overflow_work; > > struct sk_buff *tx_hwtstamp_skb; > > unsigned long tx_hwtstamp_start; > > diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c > > index 286155efcedf..43933e64819b 100644 > > --- a/drivers/net/ethernet/intel/e1000e/netdev.c > > +++ b/drivers/net/ethernet/intel/e1000e/netdev.c > > @@ -3574,6 +3574,7 @@ s32 e1000e_get_base_timinca(struct e1000_adapter *adapter, u32 *timinca) > > * e1000e_config_hwtstamp - configure the hwtstamp registers and enable/disable > > * @adapter: board private structure > > * @config: timestamp configuration > > + * @extack: netlink extended ACK for error report > > * > > * Outgoing time stamping can be enabled and disabled. Play nice and > > * disable it when requested, although it shouldn't cause any overhead > > @@ -3587,7 +3588,8 @@ s32 e1000e_get_base_timinca(struct e1000_adapter *adapter, u32 *timinca) > > * exception of "all V2 events regardless of level 2 or 4". > > **/ > > static int e1000e_config_hwtstamp(struct e1000_adapter *adapter, > > - struct hwtstamp_config *config) > > + struct kernel_hwtstamp_config *config, > > + struct netlink_ext_ack *extack) > > { > > struct e1000_hw *hw = &adapter->hw; > > u32 tsync_tx_ctl = E1000_TSYNCTXCTL_ENABLED; > > @@ -3598,8 +3600,10 @@ static int e1000e_config_hwtstamp(struct e1000_adapter *adapter, > > bool is_l2 = false; > > u32 regval; > > > > - if (!(adapter->flags & FLAG_HAS_HW_TIMESTAMP)) > > + if (!(adapter->flags & FLAG_HAS_HW_TIMESTAMP)) { > > + NL_SET_ERR_MSG(extack, "No HW timestamp support\n"); > > return -EINVAL; > > + } > > > > switch (config->tx_type) { > > case HWTSTAMP_TX_OFF: > > @@ -3608,6 +3612,7 @@ static int e1000e_config_hwtstamp(struct e1000_adapter *adapter, > > case HWTSTAMP_TX_ON: > > break; > > default: > > + NL_SET_ERR_MSG(extack, "Unsupported TX HW timestamp type\n"); > > return -ERANGE; > > } > > > > @@ -3681,6 +3686,7 @@ static int e1000e_config_hwtstamp(struct e1000_adapter *adapter, > > config->rx_filter = HWTSTAMP_FILTER_ALL; > > break; > > default: > > + NL_SET_ERR_MSG(extack, "Unsupported RX HW timestamp filter\n"); > > return -ERANGE; > > } > > > > @@ -3693,7 +3699,7 @@ static int e1000e_config_hwtstamp(struct e1000_adapter *adapter, > > ew32(TSYNCTXCTL, regval); > > if ((er32(TSYNCTXCTL) & E1000_TSYNCTXCTL_ENABLED) != > > (regval & E1000_TSYNCTXCTL_ENABLED)) { > > - e_err("Timesync Tx Control register not set as expected\n"); > > + NL_SET_ERR_MSG(extack, "Timesync Tx Control register not set as expected\n"); > > In the case where this function is being called from e1000e_systim_reset > function, won't it cause this debug print to do nothing? Yes, you're right. > > > return -EAGAIN; > > } > > > > @@ -3706,7 +3712,7 @@ static int e1000e_config_hwtstamp(struct e1000_adapter *adapter, > > E1000_TSYNCRXCTL_TYPE_MASK)) != > > (regval & (E1000_TSYNCRXCTL_ENABLED | > > E1000_TSYNCRXCTL_TYPE_MASK))) { > > - e_err("Timesync Rx Control register not set as expected\n"); > > Same question here. > > > + NL_SET_ERR_MSG(extack, "Timesync Rx Control register not set as expected\n"); > > return -EAGAIN; > > } > > > > @@ -3932,7 +3938,7 @@ static void e1000e_systim_reset(struct e1000_adapter *adapter) > > spin_unlock_irqrestore(&adapter->systim_lock, flags); > > > > /* restore the previous hwtstamp configuration settings */ > > - e1000e_config_hwtstamp(adapter, &adapter->hwtstamp_config); > > + e1000e_config_hwtstamp(adapter, &adapter->hwtstamp_config, NULL); I'll pass an extack instead of NULL and add a print here. > > } > > > > /** > > @@ -6079,8 +6085,8 @@ static int e1000_change_mtu(struct net_device *netdev, int new_mtu) > > return 0; > > } > > > > -static int e1000_mii_ioctl(struct net_device *netdev, struct ifreq *ifr, > > - int cmd) > > +static int e1000_ioctl(struct net_device *netdev, struct ifreq *ifr, > > + int cmd) > > { > > struct e1000_adapter *adapter = netdev_priv(netdev); > > struct mii_ioctl_data *data = if_mii(ifr); > > @@ -6140,7 +6146,8 @@ static int e1000_mii_ioctl(struct net_device *netdev, struct ifreq *ifr, > > /** > > * e1000e_hwtstamp_set - control hardware time stamping > > * @netdev: network interface device structure > > - * @ifr: interface request > > + * @config: timestamp configuration > > + * @extack: netlink extended ACK report > > * > > * Outgoing time stamping can be enabled and disabled. Play nice and > > * disable it when requested, although it shouldn't cause any overhead > > @@ -6153,20 +6160,18 @@ static int e1000_mii_ioctl(struct net_device *netdev, struct ifreq *ifr, > > * specified. Matching the kind of event packet is not supported, with the > > * exception of "all V2 events regardless of level 2 or 4". > > **/ > > -static int e1000e_hwtstamp_set(struct net_device *netdev, struct ifreq *ifr) > > +static int e1000e_hwtstamp_set(struct net_device *netdev, > > + struct kernel_hwtstamp_config *config, > > + struct netlink_ext_ack *extack) > > { > > struct e1000_adapter *adapter = netdev_priv(netdev); > > - struct hwtstamp_config config; > > int ret_val; > > > > - if (copy_from_user(&config, ifr->ifr_data, sizeof(config))) > > - return -EFAULT; > > - > > - ret_val = e1000e_config_hwtstamp(adapter, &config); > > + ret_val = e1000e_config_hwtstamp(adapter, config, extack); > > if (ret_val) > > return ret_val; > > > > - switch (config.rx_filter) { > > + switch (config->rx_filter) { > > case HWTSTAMP_FILTER_PTP_V2_L4_SYNC: > > case HWTSTAMP_FILTER_PTP_V2_L2_SYNC: > > case HWTSTAMP_FILTER_PTP_V2_SYNC: > > @@ -6178,38 +6183,23 @@ static int e1000e_hwtstamp_set(struct net_device *netdev, struct ifreq *ifr) > > * by hardware so notify the caller the requested packets plus > > * some others are time stamped. > > */ > > - config.rx_filter = HWTSTAMP_FILTER_SOME; > > + config->rx_filter = HWTSTAMP_FILTER_SOME; > > break; > > default: > > break; > > } > > > > - return copy_to_user(ifr->ifr_data, &config, > > - sizeof(config)) ? -EFAULT : 0; > > + return 0; > > } > > > > -static int e1000e_hwtstamp_get(struct net_device *netdev, struct ifreq *ifr) > > +static int e1000e_hwtstamp_get(struct net_device *netdev, > > + struct kernel_hwtstamp_config *kernel_config) > > { > > struct e1000_adapter *adapter = netdev_priv(netdev); > > > > - return copy_to_user(ifr->ifr_data, &adapter->hwtstamp_config, > > - sizeof(adapter->hwtstamp_config)) ? -EFAULT : 0; > > -} > > + *kernel_config = adapter->hwtstamp_config; > > > > -static int e1000_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd) > > -{ > > - switch (cmd) { > > - case SIOCGMIIPHY: > > - case SIOCGMIIREG: > > - case SIOCSMIIREG: > > - return e1000_mii_ioctl(netdev, ifr, cmd); > > - case SIOCSHWTSTAMP: > > - return e1000e_hwtstamp_set(netdev, ifr); > > - case SIOCGHWTSTAMP: > > - return e1000e_hwtstamp_get(netdev, ifr); > > - default: > > - return -EOPNOTSUPP; > > - } > > + return 0; > > } > > > > static int e1000_init_phy_wakeup(struct e1000_adapter *adapter, u32 wufc) > > @@ -7346,9 +7336,11 @@ static const struct net_device_ops e1000e_netdev_ops = { > > #ifdef CONFIG_NET_POLL_CONTROLLER > > .ndo_poll_controller = e1000_netpoll, > > #endif > > - .ndo_set_features = e1000_set_features, > > - .ndo_fix_features = e1000_fix_features, > > + .ndo_set_features = e1000_set_features, > > + .ndo_fix_features = e1000_fix_features, > > .ndo_features_check = passthru_features_check, > > + .ndo_hwtstamp_get = e1000e_hwtstamp_get, > > + .ndo_hwtstamp_set = e1000e_hwtstamp_set, > > }; > > > > /** > > > > > Also you are missing a subject prefix, I assume that you mean to send it > to iwl-next since it is not a bug fix. Please add it to your patch.
diff --git a/drivers/net/ethernet/intel/e1000e/e1000.h b/drivers/net/ethernet/intel/e1000e/e1000.h index ba9c19e6994c..952898151565 100644 --- a/drivers/net/ethernet/intel/e1000e/e1000.h +++ b/drivers/net/ethernet/intel/e1000e/e1000.h @@ -319,7 +319,7 @@ struct e1000_adapter { u16 tx_ring_count; u16 rx_ring_count; - struct hwtstamp_config hwtstamp_config; + struct kernel_hwtstamp_config hwtstamp_config; struct delayed_work systim_overflow_work; struct sk_buff *tx_hwtstamp_skb; unsigned long tx_hwtstamp_start; diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c index 286155efcedf..43933e64819b 100644 --- a/drivers/net/ethernet/intel/e1000e/netdev.c +++ b/drivers/net/ethernet/intel/e1000e/netdev.c @@ -3574,6 +3574,7 @@ s32 e1000e_get_base_timinca(struct e1000_adapter *adapter, u32 *timinca) * e1000e_config_hwtstamp - configure the hwtstamp registers and enable/disable * @adapter: board private structure * @config: timestamp configuration + * @extack: netlink extended ACK for error report * * Outgoing time stamping can be enabled and disabled. Play nice and * disable it when requested, although it shouldn't cause any overhead @@ -3587,7 +3588,8 @@ s32 e1000e_get_base_timinca(struct e1000_adapter *adapter, u32 *timinca) * exception of "all V2 events regardless of level 2 or 4". **/ static int e1000e_config_hwtstamp(struct e1000_adapter *adapter, - struct hwtstamp_config *config) + struct kernel_hwtstamp_config *config, + struct netlink_ext_ack *extack) { struct e1000_hw *hw = &adapter->hw; u32 tsync_tx_ctl = E1000_TSYNCTXCTL_ENABLED; @@ -3598,8 +3600,10 @@ static int e1000e_config_hwtstamp(struct e1000_adapter *adapter, bool is_l2 = false; u32 regval; - if (!(adapter->flags & FLAG_HAS_HW_TIMESTAMP)) + if (!(adapter->flags & FLAG_HAS_HW_TIMESTAMP)) { + NL_SET_ERR_MSG(extack, "No HW timestamp support\n"); return -EINVAL; + } switch (config->tx_type) { case HWTSTAMP_TX_OFF: @@ -3608,6 +3612,7 @@ static int e1000e_config_hwtstamp(struct e1000_adapter *adapter, case HWTSTAMP_TX_ON: break; default: + NL_SET_ERR_MSG(extack, "Unsupported TX HW timestamp type\n"); return -ERANGE; } @@ -3681,6 +3686,7 @@ static int e1000e_config_hwtstamp(struct e1000_adapter *adapter, config->rx_filter = HWTSTAMP_FILTER_ALL; break; default: + NL_SET_ERR_MSG(extack, "Unsupported RX HW timestamp filter\n"); return -ERANGE; } @@ -3693,7 +3699,7 @@ static int e1000e_config_hwtstamp(struct e1000_adapter *adapter, ew32(TSYNCTXCTL, regval); if ((er32(TSYNCTXCTL) & E1000_TSYNCTXCTL_ENABLED) != (regval & E1000_TSYNCTXCTL_ENABLED)) { - e_err("Timesync Tx Control register not set as expected\n"); + NL_SET_ERR_MSG(extack, "Timesync Tx Control register not set as expected\n"); return -EAGAIN; } @@ -3706,7 +3712,7 @@ static int e1000e_config_hwtstamp(struct e1000_adapter *adapter, E1000_TSYNCRXCTL_TYPE_MASK)) != (regval & (E1000_TSYNCRXCTL_ENABLED | E1000_TSYNCRXCTL_TYPE_MASK))) { - e_err("Timesync Rx Control register not set as expected\n"); + NL_SET_ERR_MSG(extack, "Timesync Rx Control register not set as expected\n"); return -EAGAIN; } @@ -3932,7 +3938,7 @@ static void e1000e_systim_reset(struct e1000_adapter *adapter) spin_unlock_irqrestore(&adapter->systim_lock, flags); /* restore the previous hwtstamp configuration settings */ - e1000e_config_hwtstamp(adapter, &adapter->hwtstamp_config); + e1000e_config_hwtstamp(adapter, &adapter->hwtstamp_config, NULL); } /** @@ -6079,8 +6085,8 @@ static int e1000_change_mtu(struct net_device *netdev, int new_mtu) return 0; } -static int e1000_mii_ioctl(struct net_device *netdev, struct ifreq *ifr, - int cmd) +static int e1000_ioctl(struct net_device *netdev, struct ifreq *ifr, + int cmd) { struct e1000_adapter *adapter = netdev_priv(netdev); struct mii_ioctl_data *data = if_mii(ifr); @@ -6140,7 +6146,8 @@ static int e1000_mii_ioctl(struct net_device *netdev, struct ifreq *ifr, /** * e1000e_hwtstamp_set - control hardware time stamping * @netdev: network interface device structure - * @ifr: interface request + * @config: timestamp configuration + * @extack: netlink extended ACK report * * Outgoing time stamping can be enabled and disabled. Play nice and * disable it when requested, although it shouldn't cause any overhead @@ -6153,20 +6160,18 @@ static int e1000_mii_ioctl(struct net_device *netdev, struct ifreq *ifr, * specified. Matching the kind of event packet is not supported, with the * exception of "all V2 events regardless of level 2 or 4". **/ -static int e1000e_hwtstamp_set(struct net_device *netdev, struct ifreq *ifr) +static int e1000e_hwtstamp_set(struct net_device *netdev, + struct kernel_hwtstamp_config *config, + struct netlink_ext_ack *extack) { struct e1000_adapter *adapter = netdev_priv(netdev); - struct hwtstamp_config config; int ret_val; - if (copy_from_user(&config, ifr->ifr_data, sizeof(config))) - return -EFAULT; - - ret_val = e1000e_config_hwtstamp(adapter, &config); + ret_val = e1000e_config_hwtstamp(adapter, config, extack); if (ret_val) return ret_val; - switch (config.rx_filter) { + switch (config->rx_filter) { case HWTSTAMP_FILTER_PTP_V2_L4_SYNC: case HWTSTAMP_FILTER_PTP_V2_L2_SYNC: case HWTSTAMP_FILTER_PTP_V2_SYNC: @@ -6178,38 +6183,23 @@ static int e1000e_hwtstamp_set(struct net_device *netdev, struct ifreq *ifr) * by hardware so notify the caller the requested packets plus * some others are time stamped. */ - config.rx_filter = HWTSTAMP_FILTER_SOME; + config->rx_filter = HWTSTAMP_FILTER_SOME; break; default: break; } - return copy_to_user(ifr->ifr_data, &config, - sizeof(config)) ? -EFAULT : 0; + return 0; } -static int e1000e_hwtstamp_get(struct net_device *netdev, struct ifreq *ifr) +static int e1000e_hwtstamp_get(struct net_device *netdev, + struct kernel_hwtstamp_config *kernel_config) { struct e1000_adapter *adapter = netdev_priv(netdev); - return copy_to_user(ifr->ifr_data, &adapter->hwtstamp_config, - sizeof(adapter->hwtstamp_config)) ? -EFAULT : 0; -} + *kernel_config = adapter->hwtstamp_config; -static int e1000_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd) -{ - switch (cmd) { - case SIOCGMIIPHY: - case SIOCGMIIREG: - case SIOCSMIIREG: - return e1000_mii_ioctl(netdev, ifr, cmd); - case SIOCSHWTSTAMP: - return e1000e_hwtstamp_set(netdev, ifr); - case SIOCGHWTSTAMP: - return e1000e_hwtstamp_get(netdev, ifr); - default: - return -EOPNOTSUPP; - } + return 0; } static int e1000_init_phy_wakeup(struct e1000_adapter *adapter, u32 wufc) @@ -7346,9 +7336,11 @@ static const struct net_device_ops e1000e_netdev_ops = { #ifdef CONFIG_NET_POLL_CONTROLLER .ndo_poll_controller = e1000_netpoll, #endif - .ndo_set_features = e1000_set_features, - .ndo_fix_features = e1000_fix_features, + .ndo_set_features = e1000_set_features, + .ndo_fix_features = e1000_fix_features, .ndo_features_check = passthru_features_check, + .ndo_hwtstamp_get = e1000e_hwtstamp_get, + .ndo_hwtstamp_set = e1000e_hwtstamp_set, }; /**
Update the driver to use the new hardware timestamping API added in commit 66f7223039c0 ("net: add NDOs for configuring hardware timestamping"). Use Netlink extack for error reporting in e1000e_hwtstamp_set. Align the indentation of net_device_ops. Signed-off-by: Piotr Wejman <wejmanpm@gmail.com> --- Changes in v2: - amend commit message - use extack for error reporting - rename e1000_mii_ioctl to e1000_ioctl - Link to v1: https://lore.kernel.org/netdev/20250202170839.47375-1-piotrwejman90@gmail.com/ drivers/net/ethernet/intel/e1000e/e1000.h | 2 +- drivers/net/ethernet/intel/e1000e/netdev.c | 68 ++++++++++------------ 2 files changed, 31 insertions(+), 39 deletions(-)