From: Vadim Fedorenko <vadim.fedorenko@linux.dev>
To: Vladimir Oltean <vladimir.oltean@nxp.com>, netdev@vger.kernel.org
Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>,
Alexandre Torgue <alexandre.torgue@foss.st.com>,
linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org,
Russell King <linux@armlinux.org.uk>,
Andrew Lunn <andrew+netdev@lunn.ch>,
"David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
Simon Horman <horms@kernel.org>,
Richard Cochran <richardcochran@gmail.com>,
Furong Xu <0x1207@gmail.com>
Subject: Re: [PATCH net-next] net: stmmac: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
Date: Mon, 12 May 2025 16:50:35 +0100 [thread overview]
Message-ID: <2ca5f592-74d3-41ff-8282-4359cb5ec171@linux.dev> (raw)
In-Reply-To: <20250512143607.595490-1-vladimir.oltean@nxp.com>
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)
next prev parent reply other threads:[~2025-05-12 16:14 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2025-05-12 15:58 ` Vladimir Oltean
2025-05-12 16:31 ` Vadim Fedorenko
2025-05-12 17:22 ` Jakub Kicinski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2ca5f592-74d3-41ff-8282-4359cb5ec171@linux.dev \
--to=vadim.fedorenko@linux.dev \
--cc=0x1207@gmail.com \
--cc=alexandre.torgue@foss.st.com \
--cc=andrew+netdev@lunn.ch \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=horms@kernel.org \
--cc=kuba@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-stm32@st-md-mailman.stormreply.com \
--cc=linux@armlinux.org.uk \
--cc=mcoquelin.stm32@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=richardcochran@gmail.com \
--cc=vladimir.oltean@nxp.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.