Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next v3] tg3: normalize inherited BCM5718 register byte order
@ 2026-10-11 19:41 Magnus Lindholm
  2026-10-16 21:38 ` Sebastian Andrzej Siewior
  2026-10-17  0:50 ` patchwork-bot+netdevbpf
  0 siblings, 2 replies; 3+ messages in thread
From: Magnus Lindholm @ 2026-10-11 19:41 UTC (permalink / raw)
  To: pavan.chebbi
  Cc: andrew+netdev, edumazet, kuba, pabeni, netdev, sparclinux,
	linux-kernel, linmag7

Firmware can leave BCM5718 vendor registers byte-swapped while standard
PCI fields retain normal byte order. Detect this from the host-control
revision marker, byte-swap bit and swapped product ID, without matching
a platform model or PCI subsystem. Prefer the normal revision marker
so already-normal controllers do not enter the normalization path.

Hardware swapping can accommodate big-endian register accesses, but tg3
already initializes misc_host_ctrl with WORD_SWAP set and BYTE_SWAP clear
on both little- and big-endian builds. Its normal MMIO path uses
readl()/writel(); on SPARC64 these use little-endian accesses. Restore
that existing register mode rather than choosing a mode based on host
endianness. The swab32() on the initial write compensates for the device's
inherited state; subsequent accesses use the normal driver path.

DMA byte ordering is controlled separately through GRC_MODE. Leave those
settings, including the __BIG_ENDIAN handling for non-frame data,
unchanged. Check both register readbacks and keep this in probe so PCI
access or validation failures can abort initialization.

Limit detection to BCM5718, whose register layout and inherited state
were checked on M3000 hardware. Other controller families retain their
existing initialization path.

Signed-off-by: Magnus Lindholm <linmag7@gmail.com>
---
Changes since v2:
- Expand the commit message in response to Andrew Lunn's review: explain
  the existing register mode on both host endiannesses, the purpose of
  the initial swab32(), and the separate big-endian DMA configuration.
- No code changes; the diff is identical to the tested v2.

Changes since v1:
- Detect BCM5718 register state without OF model or subsystem matching.
- Prefer the normal revision marker and retain checked PCI accesses and
  product-ID/readback validation. Use dev_dbg() for successful correction.

The unchanged v2 code was tested on big-endian SPARC64 with M3000 v2:
normalization at boot on all four ports, rebind without further
normalization, three active-link cycles and a 16 MiB bidirectional
transfer with matching SHA-256. Kernel taint remained zero. Standalone
SPARC and x86-64 CONFIG_OF=n driver builds passed; no little-endian
hardware runtime test was performed.

 drivers/net/ethernet/broadcom/tg3.c | 72 ++++++++++++++++++++++++++++-
 1 file changed, 70 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 73a4b569b03e..ef0f6e5f6440 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -16248,6 +16248,65 @@ static bool tg3_10_100_only_device(struct tg3 *tp,
 	return false;
 }
 
+static int tg3_fw_byteorder(struct tg3 *tp, u32 *misc_ctrl_reg)
+{
+	struct pci_dev *pdev = tp->pdev;
+	u32 prodid, expected_prodid, normalized, readback;
+	int reg, err;
+
+	/* Prefer the normal revision marker before testing the swapped state. */
+	if (pdev->device != TG3PCI_DEVICE_TIGON3_5718 ||
+	    (*misc_ctrl_reg >> 28) == ASIC_REV_USE_PROD_ID_REG ||
+	    (swab32(*misc_ctrl_reg) >> 28) != ASIC_REV_USE_PROD_ID_REG ||
+	    !(swab32(*misc_ctrl_reg) & MISC_HOST_CTRL_BYTE_SWAP))
+		return 0;
+
+	/* Standard PCI fields are normal; verify the swapped vendor registers. */
+	reg = TG3PCI_GEN2_PRODID_ASICREV;
+	err = pci_read_config_dword(pdev, reg, &prodid);
+	if (err)
+		goto config_error;
+	expected_prodid = swab32(prodid);
+	if ((expected_prodid >> 12) != ASIC_REV_5717) {
+		dev_err(&pdev->dev, "unexpected swapped BCM5718 product ID %08x\n",
+			prodid);
+		return -ENODEV;
+	}
+
+	normalized = (swab32(*misc_ctrl_reg) & MISC_HOST_CTRL_CHIPREV) |
+		     tp->misc_host_ctrl;
+	reg = TG3PCI_MISC_HOST_CTRL;
+	err = pci_write_config_dword(pdev, reg, swab32(normalized));
+	if (err)
+		goto config_error;
+	err = pci_read_config_dword(pdev, reg, &readback);
+	if (err)
+		goto config_error;
+	if (readback != normalized) {
+		dev_err(&pdev->dev, "BCM5718 host-control readback mismatch %08x\n",
+			readback);
+		return -EIO;
+	}
+
+	reg = TG3PCI_GEN2_PRODID_ASICREV;
+	err = pci_read_config_dword(pdev, reg, &prodid);
+	if (err)
+		goto config_error;
+	if (prodid != expected_prodid) {
+		dev_err(&pdev->dev, "BCM5718 product-ID readback mismatch %08x\n",
+			prodid);
+		return -EIO;
+	}
+	*misc_ctrl_reg = readback;
+	dev_dbg(&pdev->dev, "normalized firmware register byte order\n");
+	return 0;
+
+config_error:
+	dev_err(&pdev->dev, "BCM5718 PCI config access at %#x failed (%d)\n",
+		reg, err);
+	return pcibios_err_to_errno(err);
+}
+
 static int tg3_get_invariants(struct tg3 *tp, const struct pci_device_id *ent)
 {
 	u32 misc_ctrl_reg;
@@ -16272,8 +16331,17 @@ static int tg3_get_invariants(struct tg3 *tp, const struct pci_device_id *ent)
 	 * sure that indirect register accesses are enabled before
 	 * the first operation.
 	 */
-	pci_read_config_dword(tp->pdev, TG3PCI_MISC_HOST_CTRL,
-			      &misc_ctrl_reg);
+	err = pci_read_config_dword(tp->pdev, TG3PCI_MISC_HOST_CTRL,
+				    &misc_ctrl_reg);
+	if (err && tp->pdev->device == TG3PCI_DEVICE_TIGON3_5718) {
+		dev_err(&tp->pdev->dev, "PCI host-control read failed (%d)\n",
+			err);
+		return pcibios_err_to_errno(err);
+	}
+	err = tg3_fw_byteorder(tp, &misc_ctrl_reg);
+	if (err)
+		return err;
+
 	tp->misc_host_ctrl |= (misc_ctrl_reg &
 			       MISC_HOST_CTRL_CHIPREV);
 	pci_write_config_dword(tp->pdev, TG3PCI_MISC_HOST_CTRL,

base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH net-next v3] tg3: normalize inherited BCM5718 register byte order
  2026-10-11 19:41 [PATCH net-next v3] tg3: normalize inherited BCM5718 register byte order Magnus Lindholm
@ 2026-10-16 21:38 ` Sebastian Andrzej Siewior
  2026-10-17  0:50 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 3+ messages in thread
From: Sebastian Andrzej Siewior @ 2026-10-16 21:38 UTC (permalink / raw)
  To: Magnus Lindholm
  Cc: pavan.chebbi, andrew+netdev, edumazet, kuba, pabeni, netdev,
	sparclinux, linux-kernel

On 2026-10-11 21:41:58 [+0200], Magnus Lindholm wrote:
> Firmware can leave BCM5718 vendor registers byte-swapped while standard
> PCI fields retain normal byte order. Detect this from the host-control
> revision marker, byte-swap bit and swapped product ID, without matching
> a platform model or PCI subsystem. Prefer the normal revision marker
> so already-normal controllers do not enter the normalization path.
…
> 
> Limit detection to BCM5718, whose register layout and inherited state
> were checked on M3000 hardware. Other controller families retain their
> existing initialization path.

This boils down to firmware sets BYTE_SWAP and we need get it out in
order to have the device working and break anything while at it.
I have here DEVICE_TIGON3_5720 on x86. I forced that byte in, had the
register swapped and then it fixed it back.

> Signed-off-by: Magnus Lindholm <linmag7@gmail.com>
Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

Sebastian

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net-next v3] tg3: normalize inherited BCM5718 register byte order
  2026-10-11 19:41 [PATCH net-next v3] tg3: normalize inherited BCM5718 register byte order Magnus Lindholm
  2026-10-16 21:38 ` Sebastian Andrzej Siewior
@ 2026-10-17  0:50 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 3+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-10-17  0:50 UTC (permalink / raw)
  To: Magnus Lindholm
  Cc: pavan.chebbi, andrew+netdev, edumazet, kuba, pabeni, netdev,
	sparclinux, linux-kernel

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Sun, 11 Oct 2026 21:41:58 +0200 you wrote:
> Firmware can leave BCM5718 vendor registers byte-swapped while standard
> PCI fields retain normal byte order. Detect this from the host-control
> revision marker, byte-swap bit and swapped product ID, without matching
> a platform model or PCI subsystem. Prefer the normal revision marker
> so already-normal controllers do not enter the normalization path.
> 
> Hardware swapping can accommodate big-endian register accesses, but tg3
> already initializes misc_host_ctrl with WORD_SWAP set and BYTE_SWAP clear
> on both little- and big-endian builds. Its normal MMIO path uses
> readl()/writel(); on SPARC64 these use little-endian accesses. Restore
> that existing register mode rather than choosing a mode based on host
> endianness. The swab32() on the initial write compensates for the device's
> inherited state; subsequent accesses use the normal driver path.
> 
> [...]

Here is the summary with links:
  - [net-next,v3] tg3: normalize inherited BCM5718 register byte order
    https://git.kernel.org/netdev/net-next/c/4f9fbb30e341

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-10-17  0:50 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-11 19:41 [PATCH net-next v3] tg3: normalize inherited BCM5718 register byte order Magnus Lindholm
2026-10-16 21:38 ` Sebastian Andrzej Siewior
2026-10-17  0:50 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox