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

Simon Horman <[email protected]>
Newsgroups org.osuosl.intel-wired-lan,org.kernel.vger.linux-kernel,org.kernel.vger.netdev
Message-ID <[email protected]>
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()?

...
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.