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@xxxxxxxxxx> On Behalf
> Of Shivani Gupta
> Sent: Saturday, August 15, 2026 3:08 AM
> To: intel-wired-lan@xxxxxxxxxxxxxxxx; Nguyen, Anthony L
> <anthony.l.nguyen@xxxxxxxxx>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@xxxxxxxxx>
> Cc: netdev@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Andrew Lunn
> <andrew+netdev@xxxxxxx>; David S. Miller <davem@xxxxxxxxxxxxx>; Eric
> Dumazet <edumazet@xxxxxxxxxx>; Jakub Kicinski <kuba@xxxxxxxxxx>; Paolo
> Abeni <pabeni@xxxxxxxxxx>; Richard Cochran <richardcochran@xxxxxxxxx>;
> Simon Horman <horms@xxxxxxxxxx>; Alessio Igor Bogani
> <alessio.bogani@xxxxxxxxxx>
> 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@xxxxxxxxx>
> ---
>  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@xxxxxxxxx>