[PATCH net-next] tg3: normalize inherited M3000 register byte order

[PATCH net-next] tg3: normalize inherited M3000 register byte order

[Magnus Lindholm]

M3000 firmware can leave BCM5718 vendor registers byte-swapped while
standard PCI fields retain normal byte order. Match the Fujitsu 10cf:165a
subsystem, IKKAKU model and swapped revision/product signature before
restoring host control; reject failed PCI accesses or register readbacks.

Keep this in probe so failures can abort initialization; SPARC firmware
enumeration skips PCI_FIXUP_EARLY. Normal rebinds and other platforms
retain their existing path.

Use tg3.h's MISC_HOST_CTRL_BYTE_SWAP and TG3PCI_GEN2_PRODID_ASICREV;
the inherited state was observed on M3000 hardware.

Signed-off-by: Magnus Lindholm <linmag7@gmail.com>
---
Previously posted as patch 6/7 of the M3000 series; resending separately
as requested by Jakub Kicinski. The diff is unchanged from that posting.
This patch has no code dependency on the M3000 platform patches.

Link: https://lore.kernel.org/sparclinux/20261002161515.932316-1-linmag7@gmail.com/

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

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 73a4b569b03e..9335bcc0d8d3 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -56,6 +56,7 @@
 #include <linux/hwmon-sysfs.h>
 #include <linux/crc32.h>
 #include <linux/dmi.h>
+#include <linux/of.h>
 
 #include <net/checksum.h>
 #include <net/gso.h>
@@ -16248,6 +16249,83 @@ static bool tg3_10_100_only_device(struct tg3 *tp,
 	return false;
 }
 
+/* M3000 firmware can leave the on-board BCM5718 registers byte-swapped. */
+static bool tg3_is_m3000(struct pci_dev *pdev)
+{
+	struct device_node *root;
+	const char *model;
+	bool match;
+
+	if (pdev->vendor != PCI_VENDOR_ID_BROADCOM ||
+	    pdev->device != TG3PCI_DEVICE_TIGON3_5718 ||
+	    pdev->subsystem_vendor != 0x10cf ||
+	    pdev->subsystem_device != 0x165a)
+		return false;
+
+	root = of_find_node_by_path("/");
+	match = !of_property_read_string(root, "model", &model) &&
+		!strcmp(model, "IKKAKU");
+	of_node_put(root);
+	return match;
+}
+
+static int tg3_m3000_fw_byteorder(struct tg3 *tp, u32 *misc_ctrl_reg)
+{
+	struct pci_dev *pdev = tp->pdev;
+	u32 prodid, expected_prodid, normalized, readback;
+	int reg, err;
+
+	if (!tg3_is_m3000(pdev) ||
+	    (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, "M3000: unexpected swapped 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, "M3000: 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, "M3000: product-ID readback mismatch %08x\n",
+			prodid);
+		return -EIO;
+	}
+	*misc_ctrl_reg = readback;
+	dev_info(&pdev->dev, "M3000: normalized firmware register byte order\n");
+	return 0;
+
+config_error:
+	dev_err(&pdev->dev, "M3000: 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 +16350,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 && tg3_is_m3000(tp->pdev)) {
+		dev_err(&tp->pdev->dev, "PCI host-control read failed (%d)\n",
+			err);
+		return pcibios_err_to_errno(err);
+	}
+	err = tg3_m3000_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

Re: [PATCH net-next] tg3: normalize inherited M3000 register byte order

[Andrew Lunn]

On Fri, Oct 09, 2026 at 12:04:03AM +0200, Magnus Lindholm wrote:
> M3000 firmware can leave BCM5718 vendor registers byte-swapped while
> standard PCI fields retain normal byte order. Match the Fujitsu 10cf:165a
> subsystem, IKKAKU model and swapped revision/product signature before
> restoring host control; reject failed PCI accesses or register readbacks.
> 
> Keep this in probe so failures can abort initialization; SPARC firmware
> enumeration skips PCI_FIXUP_EARLY. Normal rebinds and other platforms
> retain their existing path.
> 
> Use tg3.h's MISC_HOST_CTRL_BYTE_SWAP and TG3PCI_GEN2_PRODID_ASICREV;
> the inherited state was observed on M3000 hardware.
> 
> Signed-off-by: Magnus Lindholm <linmag7@gmail.com>

> +/* M3000 firmware can leave the on-board BCM5718 registers byte-swapped. */
> +static bool tg3_is_m3000(struct pci_dev *pdev)
> +{
> +	struct device_node *root;
> +	const char *model;
> +	bool match;
> +
> +	if (pdev->vendor != PCI_VENDOR_ID_BROADCOM ||
> +	    pdev->device != TG3PCI_DEVICE_TIGON3_5718 ||
> +	    pdev->subsystem_vendor != 0x10cf ||
> +	    pdev->subsystem_device != 0x165a)
> +		return false;
> +
> +	root = of_find_node_by_path("/");
> +	match = !of_property_read_string(root, "model", &model) &&
> +		!strcmp(model, "IKKAKU");

The DT Maintainers generally don't like this.

Is there a legitimate reason you would want the bytes are swapped?

Can you not just probe the registers, and if they are swapped undo it?
Does it really need to be conditional on the machine?

	Andrew

Re: [PATCH net-next] tg3: normalize inherited M3000 register byte order

[Magnus Lindholm]

Hi Andrew,

On Fri, Oct 9, 2026 at 3:13 AM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Fri, Oct 09, 2026 at 12:04:03AM +0200, Magnus Lindholm wrote:
> > M3000 firmware can leave BCM5718 vendor registers byte-swapped while
> > standard PCI fields retain normal byte order. Match the Fujitsu 10cf:165a
> > subsystem, IKKAKU model and swapped revision/product signature before
> > restoring host control; reject failed PCI accesses or register readbacks.
> >
> > Keep this in probe so failures can abort initialization; SPARC firmware
> > enumeration skips PCI_FIXUP_EARLY. Normal rebinds and other platforms
> > retain their existing path.
> >
> > Use tg3.h's MISC_HOST_CTRL_BYTE_SWAP and TG3PCI_GEN2_PRODID_ASICREV;
> > the inherited state was observed on M3000 hardware.
> >
> > Signed-off-by: Magnus Lindholm <linmag7@gmail.com>
>
> > +/* M3000 firmware can leave the on-board BCM5718 registers byte-swapped. */
> > +static bool tg3_is_m3000(struct pci_dev *pdev)
> > +{
> > +     struct device_node *root;
> > +     const char *model;
> > +     bool match;
> > +
> > +     if (pdev->vendor != PCI_VENDOR_ID_BROADCOM ||
> > +         pdev->device != TG3PCI_DEVICE_TIGON3_5718 ||
> > +         pdev->subsystem_vendor != 0x10cf ||
> > +         pdev->subsystem_device != 0x165a)
> > +             return false;
> > +
> > +     root = of_find_node_by_path("/");
> > +     match = !of_property_read_string(root, "model", &model) &&
> > +             !strcmp(model, "IKKAKU");
>
> The DT Maintainers generally don't like this.
>
> Is there a legitimate reason you would want the bytes are swapped?
>
> Can you not just probe the registers, and if they are swapped undo it?
> Does it really need to be conditional on the machine?
>

Thanks for the suggestion. I've tested a revised approach without
the machine-model or subsystem checks. It checks the BCM5718 register
state, leaving a normal revision signature untouched and requiring
the swapped revision, byte-swap bit and product ID before correcting
the byte order. Both register readbacks are checked afterward.

On the M3000, all four ports normalized at boot, with no additional
normalization on driver rebind. Link cycling and bidirectional
file-transfer checks also passed.

I'll send v2 shortly.

Thanks,
Magnus

Re: [PATCH net-next] tg3: normalize inherited M3000 register byte order

[Andrew Lunn]

> > Is there a legitimate reason you would want the bytes are swapped?

You missed answering this. I'm guessing, but does the hardware offer
this to make big endian machines more efficient?

Please could you look at the driver and think about what would happen
on a big endian machine. Is the change still correct?

	Andrew

Re: [PATCH net-next] tg3: normalize inherited M3000 register byte order

[Magnus Lindholm]

Hi Andrew,

On Fri, Oct 9, 2026 at 2:09 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > > Is there a legitimate reason you would want the bytes are swapped?
>
> You missed answering this. I'm guessing, but does the hardware offer
> this to make big endian machines more efficient?
>
> Please could you look at the driver and think about what would happen
> on a big endian machine. Is the change still correct?
>
>         Andrew

You're right, my earlier reply missed the underlying question.

That is a plausible reason for providing the hardware swap mode:
it can accommodate big-endian accesses without software byte
swapping. I haven't verified Broadcom's design rationale.

However, Linux tg3 already initializes misc_host_ctrl with WORD_SWAP set
and BYTE_SWAP clear on both little- and big-endian builds, before
this helper runs.

The helper restores that existing setting. Its swab32() compensates
for the device's inherited register state, not CPU endianness.
Subsequent accesses use the normal driver path, and the separate
__BIG_ENDIAN handling for DMA remains unchanged.

That is why I believe the correction is appropriate on either
endianness. The runtime tests were on the M3000 running big-endian
SPARC64.

Regards,
Magnus