Subject: Re: [PATCH net-next v2 6/7] tsnep: convert to ndo_hwtstatmp API

From: Simon Horman <[email-masked]>
Date: Wed, 15 Oct 2025 11:03:43 +0100

On Tue, Oct 14, 2025 at 10:42:15PM +0000, Vadim Fedorenko wrote:
> Convert to .ndo_hwtstamp_get()/.ndo_hwtstamp_set() callbacks.
> After conversions the rest of tsnep_netdev_ioctl() becomes pure
> phy_do_ioctl_running(), so remove tsnep_netdev_ioctl() and replace
> it with phy_do_ioctl_running() in .ndo_eth_ioctl.
> 
> Signed-off-by: Vadim Fedorenko <[email-masked]>

...

> diff --git a/drivers/net/ethernet/engleder/tsnep_ptp.c b/drivers/net/ethernet/engleder/tsnep_ptp.c
> index 54fbf0126815..ae1308eb813d 100644
> --- a/drivers/net/ethernet/engleder/tsnep_ptp.c
> +++ b/drivers/net/ethernet/engleder/tsnep_ptp.c
> @@ -19,57 +19,53 @@ void tsnep_get_system_time(struct tsnep_adapter *adapter, u64 *time)
>  	*time = (((u64)high) << 32) | ((u64)low);
>  }
>  
> -int tsnep_ptp_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)
> +int tsnep_ptp_hwtstamp_get(struct net_device *netdev,
> +			   struct kernel_hwtstamp_config *config)
>  {
>  	struct tsnep_adapter *adapter = netdev_priv(netdev);
> -	struct hwtstamp_config config;
> -
> -	if (!ifr)
> -		return -EINVAL;
> -
> -	if (cmd == SIOCSHWTSTAMP) {
> -		if (copy_from_user(&config, ifr->ifr_data, sizeof(config)))
> -			return -EFAULT;
> -
> -		switch (config.tx_type) {
> -		case HWTSTAMP_TX_OFF:
> -		case HWTSTAMP_TX_ON:
> -			break;
> -		default:
> -			return -ERANGE;
> -		}
> -
> -		switch (config.rx_filter) {
> -		case HWTSTAMP_FILTER_NONE:
> -			break;
> -		case HWTSTAMP_FILTER_ALL:
> -		case HWTSTAMP_FILTER_PTP_V1_L4_EVENT:
> -		case HWTSTAMP_FILTER_PTP_V1_L4_SYNC:
> -		case HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ:
> -		case HWTSTAMP_FILTER_PTP_V2_L4_EVENT:
> -		case HWTSTAMP_FILTER_PTP_V2_L4_SYNC:
> -		case HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ:
> -		case HWTSTAMP_FILTER_PTP_V2_L2_EVENT:
> -		case HWTSTAMP_FILTER_PTP_V2_L2_SYNC:
> -		case HWTSTAMP_FILTER_PTP_V2_L2_DELAY_REQ:
> -		case HWTSTAMP_FILTER_PTP_V2_EVENT:
> -		case HWTSTAMP_FILTER_PTP_V2_SYNC:
> -		case HWTSTAMP_FILTER_PTP_V2_DELAY_REQ:
> -		case HWTSTAMP_FILTER_NTP_ALL:
> -			config.rx_filter = HWTSTAMP_FILTER_ALL;
> -			break;
> -		default:
> -			return -ERANGE;
> -		}

Hi Vadim,

I'm probably missing something obvious, but it's not clear to me why
removing the inner switch statements above is ok. Or, perhaps more to the
point, it seems inconsistent with other patches in this series.

OTOH, I do see why dropping the outer if conditions makes sense.

> -
> -		memcpy(&adapter->hwtstamp_config, &config,
> -		       sizeof(adapter->hwtstamp_config));
> +
> +	*config = adapter->hwtstamp_config;
> +	return 0;
> +}

...