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

[Vadim Fedorenko <vadim.fedorenko@linux.dev>]

On 15/10/2025 11:03, Simon Horman wrote:
> 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 <vadim.fedorenko@linux.dev>
> 
> ...
> 
>> 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.

I believe it's just a question for git diff. It replaces original
tsnep_ptp_ioctl() function with get() callback. The only thing that new 
function does is copying actual config into reply.

The switch statement goes to set() callback where the logic is kept as
is. Original tsnep_ptp_ioctl() was serving both get and set operations,
but the logic was applied to set operation only.

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