Re: [PATCH net v3 2/2] net: ethtool: let tsconfig reach a PHY-only timestamp provider

[Nicolai Buchwitz <nb@tipi-net.de>]

Hi (Cl|S)ashiko

On 26.9.2026 15:54, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 0 · Medium: 1 · Low: 0
> 
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let 
> patchwork
> know by including "pw-bot: cr" as a separate line at the end of your 
> reply
> (one such reply per series is enough).
> 
> - [Medium] The removed ethnl_set_tsconfig_validate() required both
>   ndo_hwtstamp_set and ndo_hwtstamp_get.
> 
> --
> 
> net: ethtool: let tsconfig reach a PHY-only timestamp provider
> 
> This patch lets TSCONFIG_GET and TSCONFIG_SET accept a device whose
> timestamping is served by a default timestamping PHY or an already
> installed hwprov, even when the netdev has no hwtstamp NDOs. It removes
> the set_validate callback and moves the check into 
> ethnl_set_tsconfig().
> It also stops selecting a netdev source without ndo_hwtstamp_set, and
> adds a NULL check for ndo_hwtstamp_get in dev_get_hwtstamp_phylib().
> 
>> diff --git a/net/ethtool/tsconfig.c b/net/ethtool/tsconfig.c
>> index 6be3aa5d4bc1e..2db0e7ba8b9f5 100644
>> --- a/net/ethtool/tsconfig.c
>> +++ b/net/ethtool/tsconfig.c

> [...]

>> @@ -313,6 +305,11 @@ static int ethnl_set_tsconfig(struct 
>> ethnl_req_info *req_base,
>>  	if (!netif_device_present(dev))
>>  		return -ENODEV;
>> 
>> +	if (!dev->netdev_ops->ndo_hwtstamp_set &&
>> +	    !phy_is_default_hwtstamp(dev->phydev) &&
>> +	    !netdev_ops_lock_dereference(dev->hwprov, dev))
>> +		return -EOPNOTSUPP;
>> +
> 
> [Severity: Medium]
> 
> The removed ethnl_set_tsconfig_validate() required both 
> ndo_hwtstamp_set
> and ndo_hwtstamp_get. This new check in ethnl_set_tsconfig() only tests
> ndo_hwtstamp_set. The matching check in tsconfig_prepare_data() only
> tests ndo_hwtstamp_get.
> 
> What happens on a netdev that has ndo_hwtstamp_set but no
> ndo_hwtstamp_get, and has neither a default timestamping PHY nor a
> hwprov?

There is no in-tree driver with ndo_hwtstamp_set but not _get. All 73 
files
have them in pairs...

Even further, the asymmetry is on purpose. The get side tests 
ndo_hwtstamp_get
and the set side tests ndo_hwtstamp_set, each gate testing what its own 
operation
needs.

If anyone insists on the symmetry, a v4 could add || !ndo_hwtstamp_get. 
But before
I respin again, let's hear if this is really needed.

> [...]

Thanks,
Nicolai