[PATCH 0/2] igc: fix BUG() during the driver probe

[PATCH 0/2] igc: fix BUG() during the driver probe

[Mika Westerberg]

Hi,

I noticed that with the v6.10-rc1 kernel I get BUG() because the
spinlock used by the PTP code is not properly initialized. This is due
the ordering change done recently and I suspect there might be other
issues the change causes. These two patches try to address this by
reverting the commit in question and then fixing the original issue by
using PCI device pointer instead for logging.

Mika Westerberg (2):
  Revert "igc: fix a log entry using uninitialized netdev"
  igc: Use PCI device pointer in logging in igc_ptp_init()

 drivers/net/ethernet/intel/igc/igc_main.c | 5 ++---
 drivers/net/ethernet/intel/igc/igc_ptp.c  | 4 ++--
 2 files changed, 4 insertions(+), 5 deletions(-)

-- 
2.43.0

[PATCH 1/2] Revert "igc: fix a log entry using uninitialized netdev"

[Mika Westerberg]

This reverts commit 86167183a17e03ec77198897975e9fdfbd53cb0b.

The commit in question moved call to igc_ptp_init() to happen after
igc_reset() which calls igc_ptp_reset() with the spinlock not yet
initialized so we get following splat:

 BUG: spinlock bad magic on CPU#4, udevd/249
  lock: 0xffff98f317e6d4b0, .magic: 00000000, .owner: <none>/-1, .owner_cpu: 0
 CPU: 4 PID: 249 Comm: udevd Not tainted 6.10.0-rc1+ #1312
 Call Trace:
  <TASK>
  dump_stack_lvl+0x6c/0x90
  dump_stack+0x10/0x20
  spin_bug+0x8c/0xc0
  do_raw_spin_lock+0x6c/0xc0
  _raw_spin_lock_irqsave+0x30/0x40
  igc_ptp_clear_tx_tstamp+0x2e/0xc0 [igc]
  igc_ptp_set_timestamp_mode+0x191/0x280 [igc]
  igc_ptp_reset+0x33/0x230 [igc]
  igc_reset+0xba/0x100 [igc]
  igc_probe+0x7d1/0xa10 [igc]

It is likely that there are other things igc_ptp_init() does that are
required by igc_ptp_reset(), so for this reason revert the commit in
question.

Fixes: 86167183a17e ("igc: fix a log entry using uninitialized netdev")
Cc: Corinna Vinschen <vinschen@redhat.com>
Cc: Hariprasad Kelam <hkelam@marvell.com>
Cc: Vinicius Costa Gomes <vinicius.gomes@intel.com>
Cc: Naama Meir <naamax.meir@linux.intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/net/ethernet/intel/igc/igc_main.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 12f004f46082..ace2fbfd87d6 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -7028,6 +7028,8 @@ static int igc_probe(struct pci_dev *pdev,
 	device_set_wakeup_enable(&adapter->pdev->dev,
 				 adapter->flags & IGC_FLAG_WOL_SUPPORTED);
 
+	igc_ptp_init(adapter);
+
 	igc_tsn_clear_schedule(adapter);
 
 	/* reset the hardware with the new settings */
@@ -7049,9 +7051,6 @@ static int igc_probe(struct pci_dev *pdev,
 	/* Check if Media Autosense is enabled */
 	adapter->ei = *ei;
 
-	/* do hw tstamp init after resetting */
-	igc_ptp_init(adapter);
-
 	/* print pcie link status and MAC address */
 	pcie_print_link_status(pdev);
 	netdev_info(netdev, "MAC: %pM\n", netdev->dev_addr);
-- 
2.43.0

[PATCH 2/2] igc: Use PCI device pointer in logging in igc_ptp_init()

[Mika Westerberg]

As described in the commit 86167183a17e ("igc: fix a log entry using
uninitialized netdev"), the netdev has not yet been registered so we get
weird log entry like:

[    5.133667] igc 0000:01:00.0 (unnamed net_device) (uninitialized): PHC added

Fix this by using the PCI device pointer instead, that's valid and
available at this point.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/net/ethernet/intel/igc/igc_ptp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc_ptp.c b/drivers/net/ethernet/intel/igc/igc_ptp.c
index 1bb026232efc..c4a5ddbe6f34 100644
--- a/drivers/net/ethernet/intel/igc/igc_ptp.c
+++ b/drivers/net/ethernet/intel/igc/igc_ptp.c
@@ -1169,9 +1169,9 @@ void igc_ptp_init(struct igc_adapter *adapter)
 						&adapter->pdev->dev);
 	if (IS_ERR(adapter->ptp_clock)) {
 		adapter->ptp_clock = NULL;
-		netdev_err(netdev, "ptp_clock_register failed\n");
+		dev_err(&adapter->pdev->dev, "ptp_clock_register failed\n");
 	} else if (adapter->ptp_clock) {
-		netdev_info(netdev, "PHC added\n");
+		dev_info(&adapter->pdev->dev, "PHC added\n");
 		adapter->ptp_flags |= IGC_PTP_ENABLED;
 	}
 }
-- 
2.43.0

Re: [PATCH 1/2] Revert "igc: fix a log entry using uninitialized netdev"

[Linux regression tracking (Thorsten Leemhuis)]

On 01.06.24 07:06, Mika Westerberg wrote:
> This reverts commit 86167183a17e03ec77198897975e9fdfbd53cb0b.

TWIMC, Sasha Neftin already submitted a revert for that commit on Wednesday:
https://lore.kernel.org/all/20240529051307.3094901-1-sasha.neftin@intel.com/

Regression reports for this problem I'm tracking:
https://lore.kernel.org/lkml/CABXGCsOkiGxAfA9tPKjYX7wqjBZQxqK2PzTcW-RgLfgo8G74EQ@mail.gmail.com/
https://lore.kernel.org/intel-gfx/87o78rmkhu.fsf@intel.com/

Ciao, Thorsten