* [PATCH net-next] net: stmmac: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
@ 2025-05-12 14:36 Vladimir Oltean
2025-05-12 15:50 ` Vadim Fedorenko
2025-05-12 17:22 ` Jakub Kicinski
0 siblings, 2 replies; 5+ messages in thread
From: Vladimir Oltean @ 2025-05-12 14:36 UTC (permalink / raw)
To: netdev
Cc: Maxime Coquelin, Alexandre Torgue, linux-stm32, linux-arm-kernel,
Russell King, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Vadim Fedorenko,
Richard Cochran, Furong Xu
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 stmmac driver to the new API, so that the
ndo_eth_ioctl() path can be removed completely.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 2 +-
.../net/ethernet/stmicro/stmmac/stmmac_main.c | 80 +++++++++----------
2 files changed, 37 insertions(+), 45 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index 1686e559f66e..cda09cf5dcca 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -301,7 +301,7 @@ struct stmmac_priv {
unsigned int mode;
unsigned int chain_mode;
int extend_desc;
- struct hwtstamp_config tstamp_config;
+ struct kernel_hwtstamp_config tstamp_config;
struct ptp_clock *ptp_clock;
struct ptp_clock_info ptp_clock_ops;
unsigned int default_addend;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index a19b6f940bf3..c090247d2a29 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -568,18 +568,19 @@ static void stmmac_get_rx_hwtstamp(struct stmmac_priv *priv, struct dma_desc *p,
/**
* stmmac_hwtstamp_set - control hardware timestamping.
* @dev: device pointer.
- * @ifr: An IOCTL specific structure, that can contain a pointer to
- * a proprietary structure used to pass information to the driver.
+ * @config: the timestamping configuration.
+ * @extack: netlink extended ack structure for error reporting.
* Description:
* This function configures the MAC to enable/disable both outgoing(TX)
* and incoming(RX) packets time stamping based on user input.
* Return Value:
* 0 on success and an appropriate -ve integer on failure.
*/
-static int stmmac_hwtstamp_set(struct net_device *dev, struct ifreq *ifr)
+static int stmmac_hwtstamp_set(struct net_device *dev,
+ struct kernel_hwtstamp_config *config,
+ struct netlink_ext_ack *extack)
{
struct stmmac_priv *priv = netdev_priv(dev);
- struct hwtstamp_config config;
u32 ptp_v2 = 0;
u32 tstamp_all = 0;
u32 ptp_over_ipv4_udp = 0;
@@ -590,34 +591,30 @@ static int stmmac_hwtstamp_set(struct net_device *dev, struct ifreq *ifr)
u32 ts_event_en = 0;
if (!(priv->dma_cap.time_stamp || priv->adv_ts)) {
- netdev_alert(priv->dev, "No support for HW time stamping\n");
+ NL_SET_ERR_MSG_MOD(extack, "No support for HW time stamping\n");
priv->hwts_tx_en = 0;
priv->hwts_rx_en = 0;
return -EOPNOTSUPP;
}
- if (copy_from_user(&config, ifr->ifr_data,
- sizeof(config)))
- return -EFAULT;
-
netdev_dbg(priv->dev, "%s config flags:0x%x, tx_type:0x%x, rx_filter:0x%x\n",
- __func__, config.flags, config.tx_type, config.rx_filter);
+ __func__, config->flags, config->tx_type, config->rx_filter);
- if (config.tx_type != HWTSTAMP_TX_OFF &&
- config.tx_type != HWTSTAMP_TX_ON)
+ if (config->tx_type != HWTSTAMP_TX_OFF &&
+ config->tx_type != HWTSTAMP_TX_ON)
return -ERANGE;
if (priv->adv_ts) {
- switch (config.rx_filter) {
+ switch (config->rx_filter) {
case HWTSTAMP_FILTER_NONE:
/* time stamp no incoming packet at all */
- config.rx_filter = HWTSTAMP_FILTER_NONE;
+ config->rx_filter = HWTSTAMP_FILTER_NONE;
break;
case HWTSTAMP_FILTER_PTP_V1_L4_EVENT:
/* PTP v1, UDP, any kind of event packet */
- config.rx_filter = HWTSTAMP_FILTER_PTP_V1_L4_EVENT;
+ config->rx_filter = HWTSTAMP_FILTER_PTP_V1_L4_EVENT;
/* 'xmac' hardware can support Sync, Pdelay_Req and
* Pdelay_resp by setting bit14 and bits17/16 to 01
* This leaves Delay_Req timestamps out.
@@ -631,7 +628,7 @@ static int stmmac_hwtstamp_set(struct net_device *dev, struct ifreq *ifr)
case HWTSTAMP_FILTER_PTP_V1_L4_SYNC:
/* PTP v1, UDP, Sync packet */
- config.rx_filter = HWTSTAMP_FILTER_PTP_V1_L4_SYNC;
+ config->rx_filter = HWTSTAMP_FILTER_PTP_V1_L4_SYNC;
/* take time stamp for SYNC messages only */
ts_event_en = PTP_TCR_TSEVNTENA;
@@ -641,7 +638,7 @@ static int stmmac_hwtstamp_set(struct net_device *dev, struct ifreq *ifr)
case HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ:
/* PTP v1, UDP, Delay_req packet */
- config.rx_filter = HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ;
+ config->rx_filter = HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ;
/* take time stamp for Delay_Req messages only */
ts_master_en = PTP_TCR_TSMSTRENA;
ts_event_en = PTP_TCR_TSEVNTENA;
@@ -652,7 +649,7 @@ static int stmmac_hwtstamp_set(struct net_device *dev, struct ifreq *ifr)
case HWTSTAMP_FILTER_PTP_V2_L4_EVENT:
/* PTP v2, UDP, any kind of event packet */
- config.rx_filter = HWTSTAMP_FILTER_PTP_V2_L4_EVENT;
+ config->rx_filter = HWTSTAMP_FILTER_PTP_V2_L4_EVENT;
ptp_v2 = PTP_TCR_TSVER2ENA;
/* take time stamp for all event messages */
snap_type_sel = PTP_TCR_SNAPTYPSEL_1;
@@ -663,7 +660,7 @@ static int stmmac_hwtstamp_set(struct net_device *dev, struct ifreq *ifr)
case HWTSTAMP_FILTER_PTP_V2_L4_SYNC:
/* PTP v2, UDP, Sync packet */
- config.rx_filter = HWTSTAMP_FILTER_PTP_V2_L4_SYNC;
+ config->rx_filter = HWTSTAMP_FILTER_PTP_V2_L4_SYNC;
ptp_v2 = PTP_TCR_TSVER2ENA;
/* take time stamp for SYNC messages only */
ts_event_en = PTP_TCR_TSEVNTENA;
@@ -674,7 +671,7 @@ static int stmmac_hwtstamp_set(struct net_device *dev, struct ifreq *ifr)
case HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ:
/* PTP v2, UDP, Delay_req packet */
- config.rx_filter = HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ;
+ config->rx_filter = HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ;
ptp_v2 = PTP_TCR_TSVER2ENA;
/* take time stamp for Delay_Req messages only */
ts_master_en = PTP_TCR_TSMSTRENA;
@@ -686,7 +683,7 @@ static int stmmac_hwtstamp_set(struct net_device *dev, struct ifreq *ifr)
case HWTSTAMP_FILTER_PTP_V2_EVENT:
/* PTP v2/802.AS1 any layer, any kind of event packet */
- config.rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT;
+ config->rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT;
ptp_v2 = PTP_TCR_TSVER2ENA;
snap_type_sel = PTP_TCR_SNAPTYPSEL_1;
if (priv->synopsys_id < DWMAC_CORE_4_10)
@@ -698,7 +695,7 @@ static int stmmac_hwtstamp_set(struct net_device *dev, struct ifreq *ifr)
case HWTSTAMP_FILTER_PTP_V2_SYNC:
/* PTP v2/802.AS1, any layer, Sync packet */
- config.rx_filter = HWTSTAMP_FILTER_PTP_V2_SYNC;
+ config->rx_filter = HWTSTAMP_FILTER_PTP_V2_SYNC;
ptp_v2 = PTP_TCR_TSVER2ENA;
/* take time stamp for SYNC messages only */
ts_event_en = PTP_TCR_TSEVNTENA;
@@ -710,7 +707,7 @@ static int stmmac_hwtstamp_set(struct net_device *dev, struct ifreq *ifr)
case HWTSTAMP_FILTER_PTP_V2_DELAY_REQ:
/* PTP v2/802.AS1, any layer, Delay_req packet */
- config.rx_filter = HWTSTAMP_FILTER_PTP_V2_DELAY_REQ;
+ config->rx_filter = HWTSTAMP_FILTER_PTP_V2_DELAY_REQ;
ptp_v2 = PTP_TCR_TSVER2ENA;
/* take time stamp for Delay_Req messages only */
ts_master_en = PTP_TCR_TSMSTRENA;
@@ -724,7 +721,7 @@ static int stmmac_hwtstamp_set(struct net_device *dev, struct ifreq *ifr)
case HWTSTAMP_FILTER_NTP_ALL:
case HWTSTAMP_FILTER_ALL:
/* time stamp any incoming packet */
- config.rx_filter = HWTSTAMP_FILTER_ALL;
+ config->rx_filter = HWTSTAMP_FILTER_ALL;
tstamp_all = PTP_TCR_TSENALL;
break;
@@ -732,18 +729,18 @@ static int stmmac_hwtstamp_set(struct net_device *dev, struct ifreq *ifr)
return -ERANGE;
}
} else {
- switch (config.rx_filter) {
+ switch (config->rx_filter) {
case HWTSTAMP_FILTER_NONE:
- config.rx_filter = HWTSTAMP_FILTER_NONE;
+ config->rx_filter = HWTSTAMP_FILTER_NONE;
break;
default:
/* PTP v1, UDP, any kind of event packet */
- config.rx_filter = HWTSTAMP_FILTER_PTP_V1_L4_EVENT;
+ config->rx_filter = HWTSTAMP_FILTER_PTP_V1_L4_EVENT;
break;
}
}
- priv->hwts_rx_en = ((config.rx_filter == HWTSTAMP_FILTER_NONE) ? 0 : 1);
- priv->hwts_tx_en = config.tx_type == HWTSTAMP_TX_ON;
+ priv->hwts_rx_en = config->rx_filter != HWTSTAMP_FILTER_NONE;
+ priv->hwts_tx_en = config->tx_type == HWTSTAMP_TX_ON;
priv->systime_flags = STMMAC_HWTS_ACTIVE;
@@ -756,31 +753,30 @@ static int stmmac_hwtstamp_set(struct net_device *dev, struct ifreq *ifr)
stmmac_config_hw_tstamping(priv, priv->ptpaddr, priv->systime_flags);
- memcpy(&priv->tstamp_config, &config, sizeof(config));
+ priv->tstamp_config = *config;
- return copy_to_user(ifr->ifr_data, &config,
- sizeof(config)) ? -EFAULT : 0;
+ return 0;
}
/**
* stmmac_hwtstamp_get - read hardware timestamping.
* @dev: device pointer.
- * @ifr: An IOCTL specific structure, that can contain a pointer to
- * a proprietary structure used to pass information to the driver.
+ * @config: the timestamping configuration.
* Description:
* This function obtain the current hardware timestamping settings
* as requested.
*/
-static int stmmac_hwtstamp_get(struct net_device *dev, struct ifreq *ifr)
+static int stmmac_hwtstamp_get(struct net_device *dev,
+ struct kernel_hwtstamp_config *config)
{
struct stmmac_priv *priv = netdev_priv(dev);
- struct hwtstamp_config *config = &priv->tstamp_config;
if (!(priv->dma_cap.time_stamp || priv->dma_cap.atime_stamp))
return -EOPNOTSUPP;
- return copy_to_user(ifr->ifr_data, config,
- sizeof(*config)) ? -EFAULT : 0;
+ *config = priv->tstamp_config;
+
+ return 0;
}
/**
@@ -6228,12 +6224,6 @@ static int stmmac_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
case SIOCSMIIREG:
ret = phylink_mii_ioctl(priv->phylink, rq, cmd);
break;
- case SIOCSHWTSTAMP:
- ret = stmmac_hwtstamp_set(dev, rq);
- break;
- case SIOCGHWTSTAMP:
- ret = stmmac_hwtstamp_get(dev, rq);
- break;
default:
break;
}
@@ -7172,6 +7162,8 @@ static const struct net_device_ops stmmac_netdev_ops = {
.ndo_bpf = stmmac_bpf,
.ndo_xdp_xmit = stmmac_xdp_xmit,
.ndo_xsk_wakeup = stmmac_xsk_wakeup,
+ .ndo_hwtstamp_get = stmmac_hwtstamp_get,
+ .ndo_hwtstamp_set = stmmac_hwtstamp_set,
};
static void stmmac_reset_subtask(struct stmmac_priv *priv)
--
2.43.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] net: stmmac: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
2025-05-12 14:36 [PATCH net-next] net: stmmac: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set() Vladimir Oltean
@ 2025-05-12 15:50 ` Vadim Fedorenko
2025-05-12 15:58 ` Vladimir Oltean
2025-05-12 17:22 ` Jakub Kicinski
1 sibling, 1 reply; 5+ messages in thread
From: Vadim Fedorenko @ 2025-05-12 15:50 UTC (permalink / raw)
To: Vladimir Oltean, netdev
Cc: Maxime Coquelin, Alexandre Torgue, linux-stm32, linux-arm-kernel,
Russell King, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Richard Cochran,
Furong Xu
On 12/05/2025 15:36, Vladimir Oltean wrote:
> 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 stmmac driver to the new API, so that the
> ndo_eth_ioctl() path can be removed completely.
The conversion to the new API looks good, but stmmac_ioctl() isn't
removed keeping ndo_eth_ioctl() path in place. Did I miss something in
the patch?
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> drivers/net/ethernet/stmicro/stmmac/stmmac.h | 2 +-
> .../net/ethernet/stmicro/stmmac/stmmac_main.c | 80 +++++++++----------
> 2 files changed, 37 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> index 1686e559f66e..cda09cf5dcca 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> @@ -301,7 +301,7 @@ struct stmmac_priv {
> unsigned int mode;
> unsigned int chain_mode;
> int extend_desc;
> - struct hwtstamp_config tstamp_config;
> + struct kernel_hwtstamp_config tstamp_config;
> struct ptp_clock *ptp_clock;
> struct ptp_clock_info ptp_clock_ops;
> unsigned int default_addend;
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index a19b6f940bf3..c090247d2a29 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -568,18 +568,19 @@ static void stmmac_get_rx_hwtstamp(struct stmmac_priv *priv, struct dma_desc *p,
> /**
> * stmmac_hwtstamp_set - control hardware timestamping.
> * @dev: device pointer.
> - * @ifr: An IOCTL specific structure, that can contain a pointer to
> - * a proprietary structure used to pass information to the driver.
> + * @config: the timestamping configuration.
> + * @extack: netlink extended ack structure for error reporting.
> * Description:
> * This function configures the MAC to enable/disable both outgoing(TX)
> * and incoming(RX) packets time stamping based on user input.
> * Return Value:
> * 0 on success and an appropriate -ve integer on failure.
> */
> -static int stmmac_hwtstamp_set(struct net_device *dev, struct ifreq *ifr)
> +static int stmmac_hwtstamp_set(struct net_device *dev,
> + struct kernel_hwtstamp_config *config,
> + struct netlink_ext_ack *extack)
> {
> struct stmmac_priv *priv = netdev_priv(dev);
> - struct hwtstamp_config config;
> u32 ptp_v2 = 0;
> u32 tstamp_all = 0;
> u32 ptp_over_ipv4_udp = 0;
> @@ -590,34 +591,30 @@ static int stmmac_hwtstamp_set(struct net_device *dev, struct ifreq *ifr)
> u32 ts_event_en = 0;
>
> if (!(priv->dma_cap.time_stamp || priv->adv_ts)) {
> - netdev_alert(priv->dev, "No support for HW time stamping\n");
> + NL_SET_ERR_MSG_MOD(extack, "No support for HW time stamping\n");
> priv->hwts_tx_en = 0;
> priv->hwts_rx_en = 0;
>
> return -EOPNOTSUPP;
> }
>
> - if (copy_from_user(&config, ifr->ifr_data,
> - sizeof(config)))
> - return -EFAULT;
> -
> netdev_dbg(priv->dev, "%s config flags:0x%x, tx_type:0x%x, rx_filter:0x%x\n",
> - __func__, config.flags, config.tx_type, config.rx_filter);
> + __func__, config->flags, config->tx_type, config->rx_filter);
>
> - if (config.tx_type != HWTSTAMP_TX_OFF &&
> - config.tx_type != HWTSTAMP_TX_ON)
> + if (config->tx_type != HWTSTAMP_TX_OFF &&
> + config->tx_type != HWTSTAMP_TX_ON)
> return -ERANGE;
>
> if (priv->adv_ts) {
> - switch (config.rx_filter) {
> + switch (config->rx_filter) {
> case HWTSTAMP_FILTER_NONE:
> /* time stamp no incoming packet at all */
> - config.rx_filter = HWTSTAMP_FILTER_NONE;
> + config->rx_filter = HWTSTAMP_FILTER_NONE;
> break;
>
> case HWTSTAMP_FILTER_PTP_V1_L4_EVENT:
> /* PTP v1, UDP, any kind of event packet */
> - config.rx_filter = HWTSTAMP_FILTER_PTP_V1_L4_EVENT;
> + config->rx_filter = HWTSTAMP_FILTER_PTP_V1_L4_EVENT;
> /* 'xmac' hardware can support Sync, Pdelay_Req and
> * Pdelay_resp by setting bit14 and bits17/16 to 01
> * This leaves Delay_Req timestamps out.
> @@ -631,7 +628,7 @@ static int stmmac_hwtstamp_set(struct net_device *dev, struct ifreq *ifr)
>
> case HWTSTAMP_FILTER_PTP_V1_L4_SYNC:
> /* PTP v1, UDP, Sync packet */
> - config.rx_filter = HWTSTAMP_FILTER_PTP_V1_L4_SYNC;
> + config->rx_filter = HWTSTAMP_FILTER_PTP_V1_L4_SYNC;
> /* take time stamp for SYNC messages only */
> ts_event_en = PTP_TCR_TSEVNTENA;
>
> @@ -641,7 +638,7 @@ static int stmmac_hwtstamp_set(struct net_device *dev, struct ifreq *ifr)
>
> case HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ:
> /* PTP v1, UDP, Delay_req packet */
> - config.rx_filter = HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ;
> + config->rx_filter = HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ;
> /* take time stamp for Delay_Req messages only */
> ts_master_en = PTP_TCR_TSMSTRENA;
> ts_event_en = PTP_TCR_TSEVNTENA;
> @@ -652,7 +649,7 @@ static int stmmac_hwtstamp_set(struct net_device *dev, struct ifreq *ifr)
>
> case HWTSTAMP_FILTER_PTP_V2_L4_EVENT:
> /* PTP v2, UDP, any kind of event packet */
> - config.rx_filter = HWTSTAMP_FILTER_PTP_V2_L4_EVENT;
> + config->rx_filter = HWTSTAMP_FILTER_PTP_V2_L4_EVENT;
> ptp_v2 = PTP_TCR_TSVER2ENA;
> /* take time stamp for all event messages */
> snap_type_sel = PTP_TCR_SNAPTYPSEL_1;
> @@ -663,7 +660,7 @@ static int stmmac_hwtstamp_set(struct net_device *dev, struct ifreq *ifr)
>
> case HWTSTAMP_FILTER_PTP_V2_L4_SYNC:
> /* PTP v2, UDP, Sync packet */
> - config.rx_filter = HWTSTAMP_FILTER_PTP_V2_L4_SYNC;
> + config->rx_filter = HWTSTAMP_FILTER_PTP_V2_L4_SYNC;
> ptp_v2 = PTP_TCR_TSVER2ENA;
> /* take time stamp for SYNC messages only */
> ts_event_en = PTP_TCR_TSEVNTENA;
> @@ -674,7 +671,7 @@ static int stmmac_hwtstamp_set(struct net_device *dev, struct ifreq *ifr)
>
> case HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ:
> /* PTP v2, UDP, Delay_req packet */
> - config.rx_filter = HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ;
> + config->rx_filter = HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ;
> ptp_v2 = PTP_TCR_TSVER2ENA;
> /* take time stamp for Delay_Req messages only */
> ts_master_en = PTP_TCR_TSMSTRENA;
> @@ -686,7 +683,7 @@ static int stmmac_hwtstamp_set(struct net_device *dev, struct ifreq *ifr)
>
> case HWTSTAMP_FILTER_PTP_V2_EVENT:
> /* PTP v2/802.AS1 any layer, any kind of event packet */
> - config.rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT;
> + config->rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT;
> ptp_v2 = PTP_TCR_TSVER2ENA;
> snap_type_sel = PTP_TCR_SNAPTYPSEL_1;
> if (priv->synopsys_id < DWMAC_CORE_4_10)
> @@ -698,7 +695,7 @@ static int stmmac_hwtstamp_set(struct net_device *dev, struct ifreq *ifr)
>
> case HWTSTAMP_FILTER_PTP_V2_SYNC:
> /* PTP v2/802.AS1, any layer, Sync packet */
> - config.rx_filter = HWTSTAMP_FILTER_PTP_V2_SYNC;
> + config->rx_filter = HWTSTAMP_FILTER_PTP_V2_SYNC;
> ptp_v2 = PTP_TCR_TSVER2ENA;
> /* take time stamp for SYNC messages only */
> ts_event_en = PTP_TCR_TSEVNTENA;
> @@ -710,7 +707,7 @@ static int stmmac_hwtstamp_set(struct net_device *dev, struct ifreq *ifr)
>
> case HWTSTAMP_FILTER_PTP_V2_DELAY_REQ:
> /* PTP v2/802.AS1, any layer, Delay_req packet */
> - config.rx_filter = HWTSTAMP_FILTER_PTP_V2_DELAY_REQ;
> + config->rx_filter = HWTSTAMP_FILTER_PTP_V2_DELAY_REQ;
> ptp_v2 = PTP_TCR_TSVER2ENA;
> /* take time stamp for Delay_Req messages only */
> ts_master_en = PTP_TCR_TSMSTRENA;
> @@ -724,7 +721,7 @@ static int stmmac_hwtstamp_set(struct net_device *dev, struct ifreq *ifr)
> case HWTSTAMP_FILTER_NTP_ALL:
> case HWTSTAMP_FILTER_ALL:
> /* time stamp any incoming packet */
> - config.rx_filter = HWTSTAMP_FILTER_ALL;
> + config->rx_filter = HWTSTAMP_FILTER_ALL;
> tstamp_all = PTP_TCR_TSENALL;
> break;
>
> @@ -732,18 +729,18 @@ static int stmmac_hwtstamp_set(struct net_device *dev, struct ifreq *ifr)
> return -ERANGE;
> }
> } else {
> - switch (config.rx_filter) {
> + switch (config->rx_filter) {
> case HWTSTAMP_FILTER_NONE:
> - config.rx_filter = HWTSTAMP_FILTER_NONE;
> + config->rx_filter = HWTSTAMP_FILTER_NONE;
> break;
> default:
> /* PTP v1, UDP, any kind of event packet */
> - config.rx_filter = HWTSTAMP_FILTER_PTP_V1_L4_EVENT;
> + config->rx_filter = HWTSTAMP_FILTER_PTP_V1_L4_EVENT;
> break;
> }
> }
> - priv->hwts_rx_en = ((config.rx_filter == HWTSTAMP_FILTER_NONE) ? 0 : 1);
> - priv->hwts_tx_en = config.tx_type == HWTSTAMP_TX_ON;
> + priv->hwts_rx_en = config->rx_filter != HWTSTAMP_FILTER_NONE;
> + priv->hwts_tx_en = config->tx_type == HWTSTAMP_TX_ON;
>
> priv->systime_flags = STMMAC_HWTS_ACTIVE;
>
> @@ -756,31 +753,30 @@ static int stmmac_hwtstamp_set(struct net_device *dev, struct ifreq *ifr)
>
> stmmac_config_hw_tstamping(priv, priv->ptpaddr, priv->systime_flags);
>
> - memcpy(&priv->tstamp_config, &config, sizeof(config));
> + priv->tstamp_config = *config;
>
> - return copy_to_user(ifr->ifr_data, &config,
> - sizeof(config)) ? -EFAULT : 0;
> + return 0;
> }
>
> /**
> * stmmac_hwtstamp_get - read hardware timestamping.
> * @dev: device pointer.
> - * @ifr: An IOCTL specific structure, that can contain a pointer to
> - * a proprietary structure used to pass information to the driver.
> + * @config: the timestamping configuration.
> * Description:
> * This function obtain the current hardware timestamping settings
> * as requested.
> */
> -static int stmmac_hwtstamp_get(struct net_device *dev, struct ifreq *ifr)
> +static int stmmac_hwtstamp_get(struct net_device *dev,
> + struct kernel_hwtstamp_config *config)
> {
> struct stmmac_priv *priv = netdev_priv(dev);
> - struct hwtstamp_config *config = &priv->tstamp_config;
>
> if (!(priv->dma_cap.time_stamp || priv->dma_cap.atime_stamp))
> return -EOPNOTSUPP;
>
> - return copy_to_user(ifr->ifr_data, config,
> - sizeof(*config)) ? -EFAULT : 0;
> + *config = priv->tstamp_config;
> +
> + return 0;
> }
>
> /**
> @@ -6228,12 +6224,6 @@ static int stmmac_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
> case SIOCSMIIREG:
> ret = phylink_mii_ioctl(priv->phylink, rq, cmd);
> break;
> - case SIOCSHWTSTAMP:
> - ret = stmmac_hwtstamp_set(dev, rq);
> - break;
> - case SIOCGHWTSTAMP:
> - ret = stmmac_hwtstamp_get(dev, rq);
> - break;
> default:
> break;
> }
> @@ -7172,6 +7162,8 @@ static const struct net_device_ops stmmac_netdev_ops = {
> .ndo_bpf = stmmac_bpf,
> .ndo_xdp_xmit = stmmac_xdp_xmit,
> .ndo_xsk_wakeup = stmmac_xsk_wakeup,
> + .ndo_hwtstamp_get = stmmac_hwtstamp_get,
> + .ndo_hwtstamp_set = stmmac_hwtstamp_set,
> };
>
> static void stmmac_reset_subtask(struct stmmac_priv *priv)
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] net: stmmac: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
2025-05-12 15:50 ` Vadim Fedorenko
@ 2025-05-12 15:58 ` Vladimir Oltean
2025-05-12 16:31 ` Vadim Fedorenko
0 siblings, 1 reply; 5+ messages in thread
From: Vladimir Oltean @ 2025-05-12 15:58 UTC (permalink / raw)
To: Vadim Fedorenko
Cc: netdev, Maxime Coquelin, Alexandre Torgue, linux-stm32,
linux-arm-kernel, Russell King, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
Richard Cochran, Furong Xu
On Mon, May 12, 2025 at 04:50:35PM +0100, Vadim Fedorenko wrote:
> On 12/05/2025 15:36, Vladimir Oltean wrote:
> > 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 stmmac driver to the new API, so that the
> > ndo_eth_ioctl() path can be removed completely.
>
> The conversion to the new API looks good, but stmmac_ioctl() isn't
> removed keeping ndo_eth_ioctl() path in place. Did I miss something in
> the patch?
I was never intending with this work to remove ndo_eth_ioctl()
completely, but instead to completely remove the timestamping
configuration path that goes through ndo_eth_ioctl(). I apologize for
any false marketing and I will be more explicit about this in further
patches.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] net: stmmac: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
2025-05-12 15:58 ` Vladimir Oltean
@ 2025-05-12 16:31 ` Vadim Fedorenko
0 siblings, 0 replies; 5+ messages in thread
From: Vadim Fedorenko @ 2025-05-12 16:31 UTC (permalink / raw)
To: Vladimir Oltean
Cc: netdev, Maxime Coquelin, Alexandre Torgue, linux-stm32,
linux-arm-kernel, Russell King, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
Richard Cochran, Furong Xu
On 12/05/2025 16:58, Vladimir Oltean wrote:
> On Mon, May 12, 2025 at 04:50:35PM +0100, Vadim Fedorenko wrote:
>> On 12/05/2025 15:36, Vladimir Oltean wrote:
>>> 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 stmmac driver to the new API, so that the
>>> ndo_eth_ioctl() path can be removed completely.
>>
>> The conversion to the new API looks good, but stmmac_ioctl() isn't
>> removed keeping ndo_eth_ioctl() path in place. Did I miss something in
>> the patch?
>
> I was never intending with this work to remove ndo_eth_ioctl()
> completely, but instead to completely remove the timestamping
> configuration path that goes through ndo_eth_ioctl(). I apologize for
> any false marketing and I will be more explicit about this in further
> patches.
Got it. The patch itself looks good:
Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] net: stmmac: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
2025-05-12 14:36 [PATCH net-next] net: stmmac: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set() Vladimir Oltean
2025-05-12 15:50 ` Vadim Fedorenko
@ 2025-05-12 17:22 ` Jakub Kicinski
1 sibling, 0 replies; 5+ messages in thread
From: Jakub Kicinski @ 2025-05-12 17:22 UTC (permalink / raw)
To: Vladimir Oltean
Cc: netdev, Maxime Coquelin, Alexandre Torgue, linux-stm32,
linux-arm-kernel, Russell King, Andrew Lunn, David S. Miller,
Eric Dumazet, Paolo Abeni, Simon Horman, Vadim Fedorenko,
Richard Cochran, Furong Xu
On Mon, 12 May 2025 17:36:07 +0300 Vladimir Oltean wrote:
> - netdev_alert(priv->dev, "No support for HW time stamping\n");
> + NL_SET_ERR_MSG_MOD(extack, "No support for HW time stamping\n");
coccinelle says:
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:594:29-64: WARNING avoid newline at end of message in NL_SET_ERR_MSG_MOD
Feel free to repost without the 24h wait.
--
pw-bot: cr
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-05-12 17:37 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-12 14:36 [PATCH net-next] net: stmmac: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set() Vladimir Oltean
2025-05-12 15:50 ` Vadim Fedorenko
2025-05-12 15:58 ` Vladimir Oltean
2025-05-12 16:31 ` Vadim Fedorenko
2025-05-12 17:22 ` Jakub Kicinski
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).