netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next] tsnep: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
@ 2025-10-17 20:34 Gerhard Engleder
  2025-10-20 10:08 ` Simon Horman
  0 siblings, 1 reply; 4+ messages in thread
From: Gerhard Engleder @ 2025-10-17 20:34 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, edumazet, pabeni, andrew+netdev, Vladimir Oltean,
	Gerhard Engleder

From: Vladimir Oltean <vladimir.oltean@nxp.com>

I took over this patch from Vladimir Oltean. The only change from my
side is the adaption of the commit message. I hope I mentioned his work
correctly in the tags.

New timestamping API was introduced in commit 66f7223039c0 ("net: add
NDOs for configuring hardware timestamping") from kernel v6.6.

It is time to convert the tsnep driver to the new API, so that
timestamping configuration can be removed from the ndo_eth_ioctl()
path completely.

The driver does not need the interface to be down in order for
timestamping to be changed. Thus, the netif_running() restriction in
tsnep_netdev_ioctl() is not migrated to the new API. There is no
interaction with hardware registers for either operation, just a
concurrency with the data path which is fine.

After removing the PHY timestamping logic from tsnep_netdev_ioctl(),
the rest is almost equivalent to phy_do_ioctl_running(), except for the
return code on the !netif_running() condition: -EINVAL vs -ENODEV.
Let's make the conversion to phy_do_ioctl_running() anyway, on the
premise that a return code standardized tree-wide is less complex.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Signed-off-by: Gerhard Engleder <gerhard@engleder-embedded.com>
Tested-by: Gerhard Engleder <gerhard@engleder-embedded.com>
---
 drivers/net/ethernet/engleder/tsnep.h      |  8 +-
 drivers/net/ethernet/engleder/tsnep_main.c | 14 +---
 drivers/net/ethernet/engleder/tsnep_ptp.c  | 90 +++++++++++-----------
 3 files changed, 53 insertions(+), 59 deletions(-)

diff --git a/drivers/net/ethernet/engleder/tsnep.h b/drivers/net/ethernet/engleder/tsnep.h
index f188fba021a6..452ab982afbe 100644
--- a/drivers/net/ethernet/engleder/tsnep.h
+++ b/drivers/net/ethernet/engleder/tsnep.h
@@ -176,7 +176,7 @@ struct tsnep_adapter {
 	struct tsnep_gcl gcl[2];
 	int next_gcl;
 
-	struct hwtstamp_config hwtstamp_config;
+	struct kernel_hwtstamp_config hwtstamp_config;
 	struct ptp_clock *ptp_clock;
 	struct ptp_clock_info ptp_clock_info;
 	/* ptp clock lock */
@@ -203,7 +203,11 @@ extern const struct ethtool_ops tsnep_ethtool_ops;
 
 int tsnep_ptp_init(struct tsnep_adapter *adapter);
 void tsnep_ptp_cleanup(struct tsnep_adapter *adapter);
-int tsnep_ptp_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd);
+int tsnep_hwtstamp_get(struct net_device *netdev,
+		       struct kernel_hwtstamp_config *config);
+int tsnep_hwtstamp_set(struct net_device *netdev,
+		       struct kernel_hwtstamp_config *config,
+		       struct netlink_ext_ack *extack);
 
 int tsnep_tc_init(struct tsnep_adapter *adapter);
 void tsnep_tc_cleanup(struct tsnep_adapter *adapter);
diff --git a/drivers/net/ethernet/engleder/tsnep_main.c b/drivers/net/ethernet/engleder/tsnep_main.c
index eba73246f986..c12ad990a2b6 100644
--- a/drivers/net/ethernet/engleder/tsnep_main.c
+++ b/drivers/net/ethernet/engleder/tsnep_main.c
@@ -2168,16 +2168,6 @@ static netdev_tx_t tsnep_netdev_xmit_frame(struct sk_buff *skb,
 	return tsnep_xmit_frame_ring(skb, &adapter->tx[queue_mapping]);
 }
 
-static int tsnep_netdev_ioctl(struct net_device *netdev, struct ifreq *ifr,
-			      int cmd)
-{
-	if (!netif_running(netdev))
-		return -EINVAL;
-	if (cmd == SIOCSHWTSTAMP || cmd == SIOCGHWTSTAMP)
-		return tsnep_ptp_ioctl(netdev, ifr, cmd);
-	return phy_mii_ioctl(netdev->phydev, ifr, cmd);
-}
-
 static void tsnep_netdev_set_multicast(struct net_device *netdev)
 {
 	struct tsnep_adapter *adapter = netdev_priv(netdev);
@@ -2384,7 +2374,7 @@ static const struct net_device_ops tsnep_netdev_ops = {
 	.ndo_open = tsnep_netdev_open,
 	.ndo_stop = tsnep_netdev_close,
 	.ndo_start_xmit = tsnep_netdev_xmit_frame,
-	.ndo_eth_ioctl = tsnep_netdev_ioctl,
+	.ndo_eth_ioctl = phy_do_ioctl_running,
 	.ndo_set_rx_mode = tsnep_netdev_set_multicast,
 	.ndo_get_stats64 = tsnep_netdev_get_stats64,
 	.ndo_set_mac_address = tsnep_netdev_set_mac_address,
@@ -2394,6 +2384,8 @@ static const struct net_device_ops tsnep_netdev_ops = {
 	.ndo_bpf = tsnep_netdev_bpf,
 	.ndo_xdp_xmit = tsnep_netdev_xdp_xmit,
 	.ndo_xsk_wakeup = tsnep_netdev_xsk_wakeup,
+	.ndo_hwtstamp_get = tsnep_hwtstamp_get,
+	.ndo_hwtstamp_set = tsnep_hwtstamp_set,
 };
 
 static int tsnep_mac_init(struct tsnep_adapter *adapter)
diff --git a/drivers/net/ethernet/engleder/tsnep_ptp.c b/drivers/net/ethernet/engleder/tsnep_ptp.c
index 54fbf0126815..f2d001f58017 100644
--- a/drivers/net/ethernet/engleder/tsnep_ptp.c
+++ b/drivers/net/ethernet/engleder/tsnep_ptp.c
@@ -19,56 +19,54 @@ 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_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;
-		}
-
-		memcpy(&adapter->hwtstamp_config, &config,
-		       sizeof(adapter->hwtstamp_config));
+
+	*config = adapter->hwtstamp_config;
+
+	return 0;
+}
+
+int tsnep_hwtstamp_set(struct net_device *netdev,
+		       struct kernel_hwtstamp_config *config,
+		       struct netlink_ext_ack *extack)
+{
+	struct tsnep_adapter *adapter = netdev_priv(netdev);
+
+	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;
 	}
 
-	if (copy_to_user(ifr->ifr_data, &adapter->hwtstamp_config,
-			 sizeof(adapter->hwtstamp_config)))
-		return -EFAULT;
+	adapter->hwtstamp_config = *config;
 
 	return 0;
 }
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH net-next] tsnep: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
  2025-10-17 20:34 [PATCH net-next] tsnep: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set() Gerhard Engleder
@ 2025-10-20 10:08 ` Simon Horman
  2025-10-20 17:59   ` Gerhard Engleder
  0 siblings, 1 reply; 4+ messages in thread
From: Simon Horman @ 2025-10-20 10:08 UTC (permalink / raw)
  To: Gerhard Engleder
  Cc: netdev, davem, kuba, edumazet, pabeni, andrew+netdev,
	Vladimir Oltean, Vadim Fedorenko

+ Vadim

On Fri, Oct 17, 2025 at 10:34:30PM +0200, Gerhard Engleder wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> I took over this patch from Vladimir Oltean. The only change from my
> side is the adaption of the commit message. I hope I mentioned his work
> correctly in the tags.
> 
> New timestamping API was introduced in commit 66f7223039c0 ("net: add
> NDOs for configuring hardware timestamping") from kernel v6.6.
> 
> It is time to convert the tsnep driver to the new API, so that
> timestamping configuration can be removed from the ndo_eth_ioctl()
> path completely.
> 
> The driver does not need the interface to be down in order for
> timestamping to be changed. Thus, the netif_running() restriction in
> tsnep_netdev_ioctl() is not migrated to the new API. There is no
> interaction with hardware registers for either operation, just a
> concurrency with the data path which is fine.
> 
> After removing the PHY timestamping logic from tsnep_netdev_ioctl(),
> the rest is almost equivalent to phy_do_ioctl_running(), except for the
> return code on the !netif_running() condition: -EINVAL vs -ENODEV.
> Let's make the conversion to phy_do_ioctl_running() anyway, on the
> premise that a return code standardized tree-wide is less complex.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> Signed-off-by: Gerhard Engleder <gerhard@engleder-embedded.com>
> Tested-by: Gerhard Engleder <gerhard@engleder-embedded.com>

Hi Gerhard, Vladimir, Vadim, all,

Recently Vadim has been working on converting a number of drivers to
use ndo_hwtstamp_get() and ndo_hwtstamp_set(). And this includes a
patch, rather similar to this one, for the tsnep [1].

I think it would be good to agree on the way forward here.

[1] https://lore.kernel.org/all/20251016152515.3510991-7-vadim.fedorenko@linux.dev/

...

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net-next] tsnep: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
  2025-10-20 10:08 ` Simon Horman
@ 2025-10-20 17:59   ` Gerhard Engleder
  2025-10-21  9:45     ` Simon Horman
  0 siblings, 1 reply; 4+ messages in thread
From: Gerhard Engleder @ 2025-10-20 17:59 UTC (permalink / raw)
  To: Simon Horman, Vadim Fedorenko
  Cc: netdev, davem, kuba, edumazet, pabeni, andrew+netdev,
	Vladimir Oltean

On 20.10.25 12:08, Simon Horman wrote:
> + Vadim
> 
> On Fri, Oct 17, 2025 at 10:34:30PM +0200, Gerhard Engleder wrote:
>> From: Vladimir Oltean <vladimir.oltean@nxp.com>
>>
>> I took over this patch from Vladimir Oltean. The only change from my
>> side is the adaption of the commit message. I hope I mentioned his work
>> correctly in the tags.
>>
>> New timestamping API was introduced in commit 66f7223039c0 ("net: add
>> NDOs for configuring hardware timestamping") from kernel v6.6.
>>
>> It is time to convert the tsnep driver to the new API, so that
>> timestamping configuration can be removed from the ndo_eth_ioctl()
>> path completely.
>>
>> The driver does not need the interface to be down in order for
>> timestamping to be changed. Thus, the netif_running() restriction in
>> tsnep_netdev_ioctl() is not migrated to the new API. There is no
>> interaction with hardware registers for either operation, just a
>> concurrency with the data path which is fine.
>>
>> After removing the PHY timestamping logic from tsnep_netdev_ioctl(),
>> the rest is almost equivalent to phy_do_ioctl_running(), except for the
>> return code on the !netif_running() condition: -EINVAL vs -ENODEV.
>> Let's make the conversion to phy_do_ioctl_running() anyway, on the
>> premise that a return code standardized tree-wide is less complex.
>>
>> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
>> Signed-off-by: Gerhard Engleder <gerhard@engleder-embedded.com>
>> Tested-by: Gerhard Engleder <gerhard@engleder-embedded.com>
> 
> Hi Gerhard, Vladimir, Vadim, all,
> 
> Recently Vadim has been working on converting a number of drivers to
> use ndo_hwtstamp_get() and ndo_hwtstamp_set(). And this includes a
> patch, rather similar to this one, for the tsnep [1].
> 
> I think it would be good to agree on the way forward here.
> 
> [1] https://lore.kernel.org/all/20251016152515.3510991-7-vadim.fedorenko@linux.dev/

I already replied to Vadim, but on the first patch version, not on V3.

@Vadim: I reviewed your V3. Thanks for your work!

So this patch can be stopped.

Gerhard

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net-next] tsnep: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
  2025-10-20 17:59   ` Gerhard Engleder
@ 2025-10-21  9:45     ` Simon Horman
  0 siblings, 0 replies; 4+ messages in thread
From: Simon Horman @ 2025-10-21  9:45 UTC (permalink / raw)
  To: Gerhard Engleder
  Cc: Vadim Fedorenko, netdev, davem, kuba, edumazet, pabeni,
	andrew+netdev, Vladimir Oltean

On Mon, Oct 20, 2025 at 07:59:22PM +0200, Gerhard Engleder wrote:
> On 20.10.25 12:08, Simon Horman wrote:
> > + Vadim
> > 
> > On Fri, Oct 17, 2025 at 10:34:30PM +0200, Gerhard Engleder wrote:
> > > From: Vladimir Oltean <vladimir.oltean@nxp.com>
> > > 
> > > I took over this patch from Vladimir Oltean. The only change from my
> > > side is the adaption of the commit message. I hope I mentioned his work
> > > correctly in the tags.
> > > 
> > > New timestamping API was introduced in commit 66f7223039c0 ("net: add
> > > NDOs for configuring hardware timestamping") from kernel v6.6.
> > > 
> > > It is time to convert the tsnep driver to the new API, so that
> > > timestamping configuration can be removed from the ndo_eth_ioctl()
> > > path completely.
> > > 
> > > The driver does not need the interface to be down in order for
> > > timestamping to be changed. Thus, the netif_running() restriction in
> > > tsnep_netdev_ioctl() is not migrated to the new API. There is no
> > > interaction with hardware registers for either operation, just a
> > > concurrency with the data path which is fine.
> > > 
> > > After removing the PHY timestamping logic from tsnep_netdev_ioctl(),
> > > the rest is almost equivalent to phy_do_ioctl_running(), except for the
> > > return code on the !netif_running() condition: -EINVAL vs -ENODEV.
> > > Let's make the conversion to phy_do_ioctl_running() anyway, on the
> > > premise that a return code standardized tree-wide is less complex.
> > > 
> > > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> > > Signed-off-by: Gerhard Engleder <gerhard@engleder-embedded.com>
> > > Tested-by: Gerhard Engleder <gerhard@engleder-embedded.com>
> > 
> > Hi Gerhard, Vladimir, Vadim, all,
> > 
> > Recently Vadim has been working on converting a number of drivers to
> > use ndo_hwtstamp_get() and ndo_hwtstamp_set(). And this includes a
> > patch, rather similar to this one, for the tsnep [1].
> > 
> > I think it would be good to agree on the way forward here.
> > 
> > [1] https://lore.kernel.org/all/20251016152515.3510991-7-vadim.fedorenko@linux.dev/
> 
> I already replied to Vadim, but on the first patch version, not on V3.
> 
> @Vadim: I reviewed your V3. Thanks for your work!
> 
> So this patch can be stopped.

Thanks for the clarification, much appreciated.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-10-21  9:46 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-17 20:34 [PATCH net-next] tsnep: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set() Gerhard Engleder
2025-10-20 10:08 ` Simon Horman
2025-10-20 17:59   ` Gerhard Engleder
2025-10-21  9:45     ` Simon Horman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).