[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20251112183508.3c20e21d@kernel.org>
Date: Wed, 12 Nov 2025 18:35:08 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
Cc: Manish Chopra <manishc@...vell.com>, Andrew Lunn
<andrew+netdev@...n.ch>, "David S. Miller" <davem@...emloft.net>, Eric
Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>, Richard
Cochran <richardcochran@...il.com>, Simon Horman <horms@...nel.org>, Jacob
Keller <jacob.e.keller@...el.com>, Kory Maincent
<kory.maincent@...tlin.com>, netdev@...r.kernel.org
Subject: Re: [PATCH net-next v3 2/2] qede: convert to use ndo_hwtstamp
callbacks
On Tue, 11 Nov 2025 15:19:00 +0000 Vadim Fedorenko wrote:
> The driver implemented SIOCSHWTSTAMP ioctl cmd only, but it stores
> configuration in private structure, so it can be reported back to users.
> Implement both ndo_hwtstamp_set and ndo_hwtstamp_set callbacks.
> ndo_hwtstamp_set implements a check of unsupported 1-step timestamping
> and qede_ptp_cfg_filters() becomes void as it cannot fail anymore.
>
> Reviewed-by: Kory Maincent <kory.maincent@...tlin.com>
> Signed-off-by: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
> -static int qede_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
> -{
> - struct qede_dev *edev = netdev_priv(dev);
> -
> - if (!netif_running(dev))
> - return -EAGAIN;
Isn't this running check gone after conversion?
> - switch (cmd) {
> - case SIOCSHWTSTAMP:
> - return qede_ptp_hw_ts(edev, ifr);
> - default:
> - DP_VERBOSE(edev, QED_MSG_DEBUG,
> - "default IOCTL cmd 0x%x\n", cmd);
> - return -EOPNOTSUPP;
> - }
> -
> - return 0;
> -}
> -
> + switch (config->tx_type) {
> + case HWTSTAMP_TX_ONESTEP_SYNC:
> + case HWTSTAMP_TX_ONESTEP_P2P:
> + NL_SET_ERR_MSG_MOD(extack,
> + "One-step timestamping is not supported");
> + return -ERANGE;
> + }
Eh, I guess the warning I was imagining isn't actually enabled at W=1 :(
And config->.x_type does not use enums..
Could you switch this to the slightly more resilient:
switch (config->tx_type) {
case HWTSTAMP_TX_ON:
case HWTSTAMP_TX_OFF:
break;
default:
NL_SET_ERR_MSG_MOD(extack,
"One-step timestamping is not supported");
return -ERANGE;
}
? Guess similar adjustment would also work for patch 1.
--
pw-bot: cr
Powered by blists - more mailing lists