[PATCH v2] net: e1000e: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()

[PATCH v2] net: e1000e: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()

[Piotr Wejman]

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");
 		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,
 };
 
 /**
-- 
2.25.1

Re: [PATCH v2] net: e1000e: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()

[Vadim Fedorenko]

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>

Re: [PATCH v2] net: e1000e: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()

[Jakub Kicinski]

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.
-- 
pw-bot: cr

Re: [Intel-wired-lan] [PATCH v2] net: e1000e: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()

[Lifshits, Vitaly]

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.

Re: [Intel-wired-lan] [PATCH v2] net: e1000e: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()

[Piotr Wejman]

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.