[PATCH iwl-net] igb: Reject hwtstamp requests when PTP is unavailable

[PATCH iwl-net] igb: Reject hwtstamp requests when PTP is unavailable

[Shivani Gupta]

IGB depends on PTP_1588_CLOCK_OPTIONAL, so ptp_clock_register() can
return NULL when PTP is compiled out and can fail at runtime.

Commit b888c510f7b3 ("igb: Avoid starting unnecessary workqueues")
moved the PTP locks, work items, timestamp configuration, and clock
reset behind successful clock registration. The netdev is already
registered when igb_ptp_init() runs, however, and the hwtstamp entry
points were not gated on registration. On 82576 a TX timestamp request
can therefore schedule a never-initialized ptp_tx_work.

Initialize the passive PTP state and hardware clock before registering
the PHC. INIT_WORK() and INIT_DELAYED_WORK() do not queue any work; the
overflow work is started only after registration succeeds. This also
ensures that PHC callbacks and the netdev timestamping paths never
observe partially initialized state.

Reject hwtstamp get and set requests with -EOPNOTSUPP while no PTP clock
is registered, and advertise software timestamping only in that state.
Successful registration retains the existing behavior on 82576, 82580,
i350, i354, i210, and i211. On 82575, an OFF request that was previously
accepted as a no-op now reports that hardware timestamping is
unsupported.

Reproduced with CONFIG_IGB=y and CONFIG_PTP_1588_CLOCK=n on the QEMU
82576 model: SIOCSHWTSTAMP(HWTSTAMP_TX_ON) followed by a hardware
timestamp request warned in __queue_work() before this change and
returns EOPNOTSUPP afterwards.

Fixes: b888c510f7b3 ("igb: Avoid starting unnecessary workqueues")
Signed-off-by: Shivani Gupta <shivani07g@gmail.com>
---
 drivers/net/ethernet/intel/igb/igb_ethtool.c |  9 +++++
 drivers/net/ethernet/intel/igb/igb_ptp.c     | 40 ++++++++++++++------
 2 files changed, 38 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index f7938c1da835..4ebd447117cc 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -2396,6 +2396,15 @@ static int igb_get_ts_info(struct net_device *dev,
 	case e1000_i354:
 	case e1000_i210:
 	case e1000_i211:
+		/* No PTP clock, no hardware timestamping. Advertise what
+		 * igb_ptp_hwtstamp_set() will actually accept.
+		 */
+		if (!(adapter->ptp_flags & IGB_PTP_ENABLED)) {
+			info->so_timestamping =
+				SOF_TIMESTAMPING_TX_SOFTWARE;
+			return 0;
+		}
+
 		info->so_timestamping =
 			SOF_TIMESTAMPING_TX_SOFTWARE |
 			SOF_TIMESTAMPING_TX_HARDWARE |
diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
index 638d8242b66b..01992a73b37b 100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -1104,6 +1104,9 @@ int igb_ptp_hwtstamp_get(struct net_device *netdev,
 {
 	struct igb_adapter *adapter = netdev_priv(netdev);
 
+	if (!(adapter->ptp_flags & IGB_PTP_ENABLED))
+		return -EOPNOTSUPP;
+
 	*config = adapter->tstamp_config;
 
 	return 0;
@@ -1285,6 +1288,9 @@ int igb_ptp_hwtstamp_set(struct net_device *netdev,
 	struct igb_adapter *adapter = netdev_priv(netdev);
 	int err;
 
+	if (!(adapter->ptp_flags & IGB_PTP_ENABLED))
+		return -EOPNOTSUPP;
+
 	err = igb_ptp_set_timestamp_mode(adapter, config);
 	if (err)
 		return err;
@@ -1378,6 +1384,25 @@ void igb_ptp_init(struct igb_adapter *adapter)
 		return;
 	}
 
+	/* Initialize all state used by the PHC and timestamping paths before
+	 * registering either interface. INIT_WORK() only initializes the work
+	 * item; no work is queued until timestamping is enabled.
+	 */
+	spin_lock_init(&adapter->tmreg_lock);
+	INIT_WORK(&adapter->ptp_tx_work, igb_ptp_tx_work);
+
+	if (adapter->ptp_flags & IGB_PTP_OVERFLOW_CHECK)
+		INIT_DELAYED_WORK(&adapter->ptp_overflow_work,
+				  igb_ptp_overflow_check);
+
+	adapter->tstamp_config.rx_filter = HWTSTAMP_FILTER_NONE;
+	adapter->tstamp_config.tx_type = HWTSTAMP_TX_OFF;
+
+	/* Initialize the hardware clock before ptp_clock_register() makes its
+	 * callbacks visible. The overflow work is started after registration.
+	 */
+	igb_ptp_reset(adapter);
+
 	adapter->ptp_clock = ptp_clock_register(&adapter->ptp_caps,
 						&adapter->pdev->dev);
 	if (IS_ERR(adapter->ptp_clock)) {
@@ -1388,17 +1413,9 @@ void igb_ptp_init(struct igb_adapter *adapter)
 			 adapter->netdev->name);
 		adapter->ptp_flags |= IGB_PTP_ENABLED;
 
-		spin_lock_init(&adapter->tmreg_lock);
-		INIT_WORK(&adapter->ptp_tx_work, igb_ptp_tx_work);
-
 		if (adapter->ptp_flags & IGB_PTP_OVERFLOW_CHECK)
-			INIT_DELAYED_WORK(&adapter->ptp_overflow_work,
-					  igb_ptp_overflow_check);
-
-		adapter->tstamp_config.rx_filter = HWTSTAMP_FILTER_NONE;
-		adapter->tstamp_config.tx_type = HWTSTAMP_TX_OFF;
-
-		igb_ptp_reset(adapter);
+			schedule_delayed_work(&adapter->ptp_overflow_work,
+					      IGB_SYSTIM_OVERFLOW_PERIOD);
 	}
 }
 
@@ -1513,7 +1530,8 @@ void igb_ptp_reset(struct igb_adapter *adapter)
 
 	wrfl();
 
-	if (adapter->ptp_flags & IGB_PTP_OVERFLOW_CHECK)
+	if ((adapter->ptp_flags & IGB_PTP_ENABLED) &&
+	    (adapter->ptp_flags & IGB_PTP_OVERFLOW_CHECK))
 		schedule_delayed_work(&adapter->ptp_overflow_work,
 				      IGB_SYSTIM_OVERFLOW_PERIOD);
 }

RE: [Intel-wired-lan] [PATCH iwl-net] igb: Reject hwtstamp requests when PTP is unavailable

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Shivani Gupta
> Sent: Saturday, August 15, 2026 3:08 AM
> To: intel-wired-lan@lists.osuosl.org; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; 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>; Richard Cochran <richardcochran@gmail.com>;
> Simon Horman <horms@kernel.org>; Alessio Igor Bogani
> <alessio.bogani@elettra.eu>
> Subject: [Intel-wired-lan] [PATCH iwl-net] igb: Reject hwtstamp
> requests when PTP is unavailable
> 
> IGB depends on PTP_1588_CLOCK_OPTIONAL, so ptp_clock_register() can
> return NULL when PTP is compiled out and can fail at runtime.
> 
> Commit b888c510f7b3 ("igb: Avoid starting unnecessary workqueues")
> moved the PTP locks, work items, timestamp configuration, and clock
> reset behind successful clock registration. The netdev is already
> registered when igb_ptp_init() runs, however, and the hwtstamp entry
> points were not gated on registration. On 82576 a TX timestamp request
> can therefore schedule a never-initialized ptp_tx_work.
> 
> Initialize the passive PTP state and hardware clock before registering
> the PHC. INIT_WORK() and INIT_DELAYED_WORK() do not queue any work;
> the overflow work is started only after registration succeeds. This
> also ensures that PHC callbacks and the netdev timestamping paths
> never observe partially initialized state.
> 
> Reject hwtstamp get and set requests with -EOPNOTSUPP while no PTP
> clock is registered, and advertise software timestamping only in that
> state.
> Successful registration retains the existing behavior on 82576, 82580,
> i350, i354, i210, and i211. On 82575, an OFF request that was
> previously accepted as a no-op now reports that hardware timestamping
> is unsupported.
> 
> Reproduced with CONFIG_IGB=y and CONFIG_PTP_1588_CLOCK=n on the QEMU
> 82576 model: SIOCSHWTSTAMP(HWTSTAMP_TX_ON) followed by a hardware
> timestamp request warned in __queue_work() before this change and
> returns EOPNOTSUPP afterwards.
> 
> Fixes: b888c510f7b3 ("igb: Avoid starting unnecessary workqueues")
> Signed-off-by: Shivani Gupta <shivani07g@gmail.com>
> ---
>  drivers/net/ethernet/intel/igb/igb_ethtool.c |  9 +++++
>  drivers/net/ethernet/intel/igb/igb_ptp.c     | 40 ++++++++++++++-----
> -
>  2 files changed, 38 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c
> b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> index f7938c1da835..4ebd447117cc 100644
> --- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
> +++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> @@ -2396,6 +2396,15 @@ static int igb_get_ts_info(struct net_device
> *dev,
>  	case e1000_i354:
>  	case e1000_i210:
>  	case e1000_i211:
> +		/* No PTP clock, no hardware timestamping. Advertise
> what
> +		 * igb_ptp_hwtstamp_set() will actually accept.
> +		 */
> +		if (!(adapter->ptp_flags & IGB_PTP_ENABLED)) {
> +			info->so_timestamping =
> +				SOF_TIMESTAMPING_TX_SOFTWARE;
> +			return 0;
> +		}
> +
>  		info->so_timestamping =
>  			SOF_TIMESTAMPING_TX_SOFTWARE |
>  			SOF_TIMESTAMPING_TX_HARDWARE |
> diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c
> b/drivers/net/ethernet/intel/igb/igb_ptp.c
> index 638d8242b66b..01992a73b37b 100644
> --- a/drivers/net/ethernet/intel/igb/igb_ptp.c
> +++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
> @@ -1104,6 +1104,9 @@ int igb_ptp_hwtstamp_get(struct net_device
> *netdev,  {
>  	struct igb_adapter *adapter = netdev_priv(netdev);
> 
> +	if (!(adapter->ptp_flags & IGB_PTP_ENABLED))
> +		return -EOPNOTSUPP;
> +
>  	*config = adapter->tstamp_config;
> 
>  	return 0;
> @@ -1285,6 +1288,9 @@ int igb_ptp_hwtstamp_set(struct net_device
> *netdev,
>  	struct igb_adapter *adapter = netdev_priv(netdev);
>  	int err;
> 
> +	if (!(adapter->ptp_flags & IGB_PTP_ENABLED))
> +		return -EOPNOTSUPP;
> +
>  	err = igb_ptp_set_timestamp_mode(adapter, config);
>  	if (err)
>  		return err;
> @@ -1378,6 +1384,25 @@ void igb_ptp_init(struct igb_adapter *adapter)
>  		return;
>  	}
> 
> +	/* Initialize all state used by the PHC and timestamping paths
> before
> +	 * registering either interface. INIT_WORK() only initializes
> the work
> +	 * item; no work is queued until timestamping is enabled.
> +	 */
> +	spin_lock_init(&adapter->tmreg_lock);
> +	INIT_WORK(&adapter->ptp_tx_work, igb_ptp_tx_work);
> +
> +	if (adapter->ptp_flags & IGB_PTP_OVERFLOW_CHECK)
> +		INIT_DELAYED_WORK(&adapter->ptp_overflow_work,
> +				  igb_ptp_overflow_check);
> +
> +	adapter->tstamp_config.rx_filter = HWTSTAMP_FILTER_NONE;
> +	adapter->tstamp_config.tx_type = HWTSTAMP_TX_OFF;
> +
> +	/* Initialize the hardware clock before ptp_clock_register()
> makes its
> +	 * callbacks visible. The overflow work is started after
> registration.
> +	 */
> +	igb_ptp_reset(adapter);
> +
>  	adapter->ptp_clock = ptp_clock_register(&adapter->ptp_caps,
>  						&adapter->pdev->dev);
>  	if (IS_ERR(adapter->ptp_clock)) {
> @@ -1388,17 +1413,9 @@ void igb_ptp_init(struct igb_adapter *adapter)
>  			 adapter->netdev->name);
>  		adapter->ptp_flags |= IGB_PTP_ENABLED;
> 
> -		spin_lock_init(&adapter->tmreg_lock);
> -		INIT_WORK(&adapter->ptp_tx_work, igb_ptp_tx_work);
> -
>  		if (adapter->ptp_flags & IGB_PTP_OVERFLOW_CHECK)
> -			INIT_DELAYED_WORK(&adapter->ptp_overflow_work,
> -					  igb_ptp_overflow_check);
> -
> -		adapter->tstamp_config.rx_filter = HWTSTAMP_FILTER_NONE;
> -		adapter->tstamp_config.tx_type = HWTSTAMP_TX_OFF;
> -
> -		igb_ptp_reset(adapter);
> +			schedule_delayed_work(&adapter-
> >ptp_overflow_work,
> +					      IGB_SYSTIM_OVERFLOW_PERIOD);
>  	}
>  }
> 
> @@ -1513,7 +1530,8 @@ void igb_ptp_reset(struct igb_adapter *adapter)
> 
>  	wrfl();
> 
> -	if (adapter->ptp_flags & IGB_PTP_OVERFLOW_CHECK)
> +	if ((adapter->ptp_flags & IGB_PTP_ENABLED) &&
> +	    (adapter->ptp_flags & IGB_PTP_OVERFLOW_CHECK))
>  		schedule_delayed_work(&adapter->ptp_overflow_work,
>  				      IGB_SYSTIM_OVERFLOW_PERIOD);
>  }


Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

Re: [PATCH iwl-net] igb: Reject hwtstamp requests when PTP is unavailable

[Simon Horman]

On Sat, Aug 15, 2026 at 01:08:15AM +0000, Shivani Gupta wrote:

...

> diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c

...

> @@ -1378,6 +1384,25 @@ void igb_ptp_init(struct igb_adapter *adapter)
>  		return;
>  	}
>  
> +	/* Initialize all state used by the PHC and timestamping paths before
> +	 * registering either interface. INIT_WORK() only initializes the work
> +	 * item; no work is queued until timestamping is enabled.
> +	 */
> +	spin_lock_init(&adapter->tmreg_lock);
> +	INIT_WORK(&adapter->ptp_tx_work, igb_ptp_tx_work);
> +
> +	if (adapter->ptp_flags & IGB_PTP_OVERFLOW_CHECK)
> +		INIT_DELAYED_WORK(&adapter->ptp_overflow_work,
> +				  igb_ptp_overflow_check);
> +
> +	adapter->tstamp_config.rx_filter = HWTSTAMP_FILTER_NONE;
> +	adapter->tstamp_config.tx_type = HWTSTAMP_TX_OFF;
> +
> +	/* Initialize the hardware clock before ptp_clock_register() makes its
> +	 * callbacks visible. The overflow work is started after registration.
> +	 */
> +	igb_ptp_reset(adapter);
> +
>  	adapter->ptp_clock = ptp_clock_register(&adapter->ptp_caps,
>  						&adapter->pdev->dev);
>  	if (IS_ERR(adapter->ptp_clock)) {

Hi Shivani,

There is an AI-generated review of this patch available at
https://sashiko.dev/#/patchset/20260815010815.91898-1-shivani07g%40gmail.com

Of that review the following item stands out to me.
I would appreciate it if you could look over it in particular.

  Does this code leave the hardware interrupt unmasked if ptp_clock_register()
  fails?

  The call to igb_ptp_reset() unconditionally unmasks the Time Sync hardware
  interrupt. If ptp_clock_register() fails, adapter->ptp_clock is set to NULL,
  but the hardware state is not reverted.

  If a hardware interrupt occurs, igb_tsync_interrupt() is called:

  igb_tsync_interrupt()
      if (adapter->ptp_caps.pps)
          ptp_clock_event(adapter->ptp_clock, &event);

  Will this result in a NULL pointer dereference when adapter->ptp_clock is
  passed to ptp_clock_event()?

...