> -----Original Message-----
> From: Intel-wired-lan <[email protected]> On Behalf
> Of Shivani Gupta
> Sent: Saturday, August 15, 2026 3:08 AM
> To: [email protected]; Nguyen, Anthony L
> <[email protected]>; Kitszel, Przemyslaw
> <[email protected]>
> Cc: [email protected]; [email protected]; Andrew Lunn
> <[email protected]>; David S. Miller <[email protected]>; Eric
> Dumazet <[email protected]>; Jakub Kicinski <[email protected]>; Paolo
> Abeni <[email protected]>; Richard Cochran <[email protected]>;
> Simon Horman <[email protected]>; Alessio Igor Bogani
> <[email protected]>
> 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 <[email protected]>
> ---
>  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 <[email protected]>

Reply via email to