[PATCH net-next 0/2] net: sgi: ioc3-eth: two changes for the IOC3 on SGI IP35 machines

[PATCH net-next 0/2] net: sgi: ioc3-eth: two changes for the IOC3 on SGI IP35 machines

[Imre Kaloz]

With the IP35 support series [1] posted, these two changes let ioc3-eth
drive the IOC3 of the SGI Fuel, which sits behind an XBridge and has no
MAC PROM.

Patch 1 applies the DMA attributes and the RX barrier's upper address
bits only to 64-bit direct-mapped addresses, so the IOC3 can use a
bridge's 32-bit direct window. Patch 2 asks
eth_platform_get_mac_address() when the 1-wire MAC PROM read fails,
and uses a random address once deferred probing gives up.

Both were posted as 25/53 and 38/53 of the IP35 series v2 [1] and are
reposted on their own for net-next, as Jakub asked [2]. The rest of that
series puts an IOC3 behind an XBridge or PIC on the 32-bit window and
supplies the MAC address from the L1 system controller. There is no
build dependency in either direction, and IOC3 Ethernet on IP35 machines
needs both sides.

Tested on an SGI Fuel.

[1] https://lore.kernel.org/all/cover.1791890466.git.kaloz@kernel.org/
[2] https://lore.kernel.org/all/20261021073921.07632a37@kernel.org/

Imre Kaloz (2):
  net: sgi: ioc3-eth: apply the DMA attributes only to 64-bit addresses
  net: sgi: ioc3-eth: fall back to the platform MAC address

 drivers/net/ethernet/sgi/ioc3-eth.c | 43 ++++++++++++++++++++++++-----
 1 file changed, 36 insertions(+), 7 deletions(-)


base-commit: 97ced2c090bfc2156db31fca9f52cf253e4bc186
-- 
2.47.3

[PATCH net-next 1/2] net: sgi: ioc3-eth: apply the DMA attributes only to 64-bit addresses

[Imre Kaloz]

A Bridge 64-bit direct-mapped DMA address carries the target widget and
the DMA attributes. An address in the 2GB 32-bit direct window carries
neither, so attributes OR-ed onto one aim it at widget 0, and every
receive raises a PCI error.

Apply them, and the RX barrier's upper address bits, only to 64-bit
addresses. IP27 and IP30 give the IOC3 64-bit addresses; behind an
XBridge or PIC it can use the window.

Signed-off-by: Imre Kaloz <kaloz@kernel.org>
---
Previously posted as 25/53 of the IP35 series v2: https://lore.kernel.org/all/cover.1791890466.git.kaloz@kernel.org/

 drivers/net/ethernet/sgi/ioc3-eth.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/sgi/ioc3-eth.c b/drivers/net/ethernet/sgi/ioc3-eth.c
index 009f37105eaf..25139cf5ff62 100644
--- a/drivers/net/ethernet/sgi/ioc3-eth.c
+++ b/drivers/net/ethernet/sgi/ioc3-eth.c
@@ -152,19 +152,37 @@ static inline int ioc3_alloc_skb(struct ioc3_private *ip, struct sk_buff **skb,
 }
 
 #ifdef CONFIG_PCI_XTALK_BRIDGE
+/* A Bridge 64-bit direct-mapped DMA address carries the target widget in
+ * bits 63:60 and the DMA attributes below it.  An address in the 32-bit
+ * direct window carries neither, so attributes OR-ed onto one form a
+ * 64-bit address aimed at widget 0.
+ */
 static inline unsigned long ioc3_map(dma_addr_t addr, unsigned long attr)
 {
+	if (!(addr & PCI64_ATTR_TARG_MASK))
+		return addr;
+
 	return (addr & ~PCI64_ATTR_BAR) | attr;
 }
 
-#define ERBAR_VAL	(ERBAR_BARRIER_BIT << ERBAR_RXBARR_SHIFT)
+/* The RX barrier's upper address bits follow the same form. */
+static inline u32 ioc3_erbar(dma_addr_t addr)
+{
+	if (!(addr & PCI64_ATTR_TARG_MASK))
+		return 0;
+
+	return ERBAR_BARRIER_BIT << ERBAR_RXBARR_SHIFT;
+}
 #else
 static inline unsigned long ioc3_map(dma_addr_t addr, unsigned long attr)
 {
 	return addr;
 }
 
-#define ERBAR_VAL	0
+static inline u32 ioc3_erbar(dma_addr_t addr)
+{
+	return 0;
+}
 #endif
 
 static int ioc3eth_nvmem_match(struct device *dev, const void *data)
@@ -727,7 +745,7 @@ static void ioc3_init(struct net_device *dev)
 	readl(&regs->emcr);
 
 	/* Misc registers  */
-	writel(ERBAR_VAL, &regs->erbar);
+	writel(ioc3_erbar(ip->rxr_dma), &regs->erbar);
 	readl(&regs->etcdc);			/* Clear on read */
 	writel(15, &regs->ercsr);		/* RX low watermark  */
 	writel(0, &regs->ertr);			/* Interrupt immediately */
-- 
2.47.3

[PATCH net-next 2/2] net: sgi: ioc3-eth: fall back to the platform MAC address

[Imre Kaloz]

The IOC3 on Fuel's system board has no MAC PROM on its 1-wire bus, so
ioc3eth_get_mac_addr() cannot succeed there and the probe defers for
ever. Ask eth_platform_get_mac_address() before deferring, so a platform
that knows the address can supply it. Once deferred probing gives up
with neither source answering, use a random address.

Signed-off-by: Imre Kaloz <kaloz@kernel.org>
---
Previously posted as 38/53 of the IP35 series v2: https://lore.kernel.org/all/cover.1791890466.git.kaloz@kernel.org/

 drivers/net/ethernet/sgi/ioc3-eth.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/sgi/ioc3-eth.c b/drivers/net/ethernet/sgi/ioc3-eth.c
index 25139cf5ff62..5296614bf3fb 100644
--- a/drivers/net/ethernet/sgi/ioc3-eth.c
+++ b/drivers/net/ethernet/sgi/ioc3-eth.c
@@ -847,6 +847,7 @@ static int ioc3eth_probe(struct platform_device *pdev)
 {
 	u32 sw_physid1, sw_physid2, vendor, model, rev;
 	struct ioc3_private *ip;
+	bool random_mac = false;
 	struct net_device *dev;
 	struct resource *regs;
 	u8 mac_addr[6];
@@ -857,9 +858,16 @@ static int ioc3eth_probe(struct platform_device *pdev)
 		dev_err(&pdev->dev, "Invalid resource\n");
 		return -EINVAL;
 	}
-	/* get mac addr from one wire prom */
-	if (ioc3eth_get_mac_addr(regs, mac_addr))
-		return -EPROBE_DEFER; /* not available yet */
+	/* get mac addr from one wire prom, or from the platform */
+	if (ioc3eth_get_mac_addr(regs, mac_addr) &&
+	    eth_platform_get_mac_address(&pdev->dev, mac_addr)) {
+		err = driver_deferred_probe_check_state(&pdev->dev);
+		if (err == -EPROBE_DEFER)
+			return err; /* not available yet */
+
+		dev_warn(&pdev->dev, "no MAC address, using a random one\n");
+		random_mac = true;
+	}
 
 	dev = alloc_etherdev(sizeof(struct ioc3_private));
 	if (!dev)
@@ -938,7 +946,10 @@ static int ioc3eth_probe(struct platform_device *pdev)
 
 	ioc3_mii_start(ip);
 	ioc3_ssram_disc(ip);
-	eth_hw_addr_set(dev, mac_addr);
+	if (random_mac)
+		eth_hw_addr_random(dev);
+	else
+		eth_hw_addr_set(dev, mac_addr);
 
 	/* The IOC3-specific entries in the device structure. */
 	dev->watchdog_timeo	= 5 * HZ;
-- 
2.47.3

Re: [PATCH net-next 1/2] net: sgi: ioc3-eth: apply the DMA attributes only to 64-bit addresses

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 2 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] ioc3_init() picks the device-wide RX barrier upper-address
  bits from the RX descriptor ring's coherent address:…
- [Medium] For 32-bit direct-window addresses the patch drops every per-
  transaction Bridge attribute the driver used to request: - RX buffers…
- [Low] In this tree, the new 32-bit-window branches in ioc3_map() and
  ioc3_erbar() (drivers/net/ethernet/sgi/ioc3-eth.c:162-163 and 171-172,…

--

net: sgi: ioc3-eth: apply the DMA attributes only to 64-bit addresses

This patch changes ioc3_map() so that addresses with bits 63:60 clear
(the Bridge 32-bit direct window) are returned unchanged, without the
DMA attributes OR-ed in. It also replaces the ERBAR_VAL constant with
ioc3_erbar(), which sets the RX barrier upper address bits only when the
RX ring address is a 64-bit direct-mapped address.

> diff --git a/drivers/net/ethernet/sgi/ioc3-eth.c b/drivers/net/ethernet/sgi/ioc3-eth.c
> index 009f37105eafd..25139cf5ff62c 100644
> --- a/drivers/net/ethernet/sgi/ioc3-eth.c
> +++ b/drivers/net/ethernet/sgi/ioc3-eth.c
> @@ -152,19 +152,37 @@ static inline int ioc3_alloc_skb(struct ioc3_private *ip, struct sk_buff **skb,
>  }
>  
>  #ifdef CONFIG_PCI_XTALK_BRIDGE
> +/* A Bridge 64-bit direct-mapped DMA address carries the target widget in
> + * bits 63:60 and the DMA attributes below it.  An address in the 32-bit
> + * direct window carries neither, so attributes OR-ed onto one form a
> + * 64-bit address aimed at widget 0.
> + */
>  static inline unsigned long ioc3_map(dma_addr_t addr, unsigned long attr)
>  {
> +	if (!(addr & PCI64_ATTR_TARG_MASK))
> +		return addr;

[Severity: Low]
Can the code currently in the tree ever reach this branch? With
CONFIG_PCI_XTALK_BRIDGE, phys_to_dma() is the only source of IOC3 DMA
addresses:

arch/mips/pci/pci-xtalk-bridge.c:phys_to_dma() {
	...
	return bc->baddr + paddr;
}

and bridge_probe() always sets:

	bc->baddr = (u64)bd->masterwid << 60 | PCI64_ATTR_BAR;

masterwid is never zero. IP27 uses 0xa for a directly attached Bridge,
or the hub's xbow port from xbow_probe(). IP30 uses IP30_WIDGET_HEART
(8). That means every address the IOC3 gets from dma_map_single() or
dma_alloc_coherent() has bits 63:60 set. The same applies to the new
check in ioc3_erbar().

dma_to_phys() only masks bits 63:56, so it also could not round-trip a
window address.

The commit message says:

  "An address in the 2GB 32-bit direct window carries neither, so
  attributes OR-ed onto one aim it at widget 0, and every receive
  raises a PCI error."

and:

  "IP27 and IP30 give the IOC3 64-bit addresses; behind an XBridge or
  PIC it can use the window."

In this tree, XBridges are handled by the IP27 and IP30 code
(XBRIDGE_WIDGET_PART_NUM in probe_one_port() and xtalk_init_widget()),
and they still get 64-bit addresses. There is no PIC or IP35 support
yet.

Could the commit message say that this prepares for platform code that
hands out window addresses? As written, it reads like a fix for an RX
failure on existing systems.

> +
>  	return (addr & ~PCI64_ATTR_BAR) | attr;
>  }
>  
> -#define ERBAR_VAL	(ERBAR_BARRIER_BIT << ERBAR_RXBARR_SHIFT)
> +/* The RX barrier's upper address bits follow the same form. */
> +static inline u32 ioc3_erbar(dma_addr_t addr)
> +{
> +	if (!(addr & PCI64_ATTR_TARG_MASK))
> +		return 0;
> +
> +	return ERBAR_BARRIER_BIT << ERBAR_RXBARR_SHIFT;
> +}

[Severity: Medium]
For a window address, ioc3_map() and ioc3_erbar() now drop every Bridge
attribute the driver used to request:

  ioc3_alloc_rx_bufs() and ioc3_rx(): RX buffers lose PCI64_ATTR_BAR
  ioc3_start(): RX and TX ring bases lose PCI64_ATTR_PREC
  ioc3_start_xmit(): TX buffers lose PCI64_ATTR_PREF
  ioc3_init(): ERBAR is written as 0

Window addresses cannot carry these attributes. For 32-bit direct DMA
the Bridge takes them from the slot's b_device register instead.
BRIDGE_DEV_D32_BITS in arch/mips/include/asm/pci/bridge.h lists
BRIDGE_DEV_PREF, BRIDGE_DEV_PRECISE, BRIDGE_DEV_COH and
BRIDGE_DEV_BARRIER for that.

In this tree, the only b_device setup that affects the IOC3 slot is:

arch/mips/pci/pci-xtalk-bridge.c:bridge_probe() {
	...
	for (slot = 0; slot < 8; slot++) {
		bridge_set(bc, b_device[slot].reg, BRIDGE_DEV_SWAP_DIR);
	...
}

followed by the IOC3 fixup:

arch/mips/pci/pci-xtalk-bridge.c:bridge_disable_swapping() {
	...
	bridge_clr(bc, b_device[slot].reg, BRIDGE_DEV_SWAP_DIR);
	...
}

Nothing sets BRIDGE_DEV_BARRIER or the other D32 attribute bits.

Without the barrier, could the RX status write to rxb->w0 become visible
before the packet data that ioc3_rx() reads once it sees ERXBUF_V?

Does the window path rely on platform code setting the D32 bits in
b_device for the IOC3 slot? If so, could the commit message mention
that, or could the setup be done together with this change?

>  #else
>  static inline unsigned long ioc3_map(dma_addr_t addr, unsigned long attr)
>  {

[ ... ]

> @@ -727,7 +745,7 @@ static void ioc3_init(struct net_device *dev)
>  	readl(&regs->emcr);
>  
>  	/* Misc registers  */
> -	writel(ERBAR_VAL, &regs->erbar);
> +	writel(ioc3_erbar(ip->rxr_dma), &regs->erbar);

[Severity: Medium]
Should the ERBAR value be decided from the RX descriptor ring address?
ERBAR is set once for the whole device from the coherent ring address,
but the RX barrier applies to writes into the RX data buffers.

Each buffer is mapped on its own by dma_map_single() in
ioc3_alloc_skb(), and its form is decided per buffer:

ioc3_alloc_rx_bufs():
	ip->rxr[i] = cpu_to_be64(ioc3_map(d, PCI64_ATTR_BAR));

ioc3_rx():
	rxr[n_entry] = cpu_to_be64(ioc3_map(d, PCI64_ATTR_BAR));

drivers/mfd/ioc3.c sets a 64-bit DMA mask, and the 2GB window covers
only part of memory. A platform that picks the window or the 64-bit
form by physical address could therefore put the ring in one form and
the buffers in the other.

If the ring is 64-bit and the buffers use the window, ERBAR still adds
the BAR attribute to barrier writes into window-addressed buffers. That
gives the widget 0 PCI error the commit message describes.

If the ring uses the window and the buffers are 64-bit, the barrier is
cleared for buffers that need it.

Could ioc3_erbar() use something that also holds for the RX buffers?
Alternatively, could the buffer mappings be kept in the same form as the
ring?

>  	readl(&regs->etcdc);			/* Clear on read */
>  	writel(15, &regs->ercsr);		/* RX low watermark  */
>  	writel(0, &regs->ertr);			/* Interrupt immediately */

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261111095138.1757-1-kaloz%40kernel.org

Re: [PATCH net-next 2/2] net: sgi: ioc3-eth: fall back to the platform MAC address

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 0 (2) · Medium: 3 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The new 'or from the platform' fallback in ioc3eth_probe() can
  never succeed for any ioc3-eth device in the tree.
- [Medium] The patch puts a permanent random-MAC fallback on every IOC3
  platform, not just Fuel.
- [Medium] net: sgi: ioc3-eth: premature platform MAC fallback in
  ioc3eth_probe()

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but
  ioc3eth_probe() never calls platform_set_drvdata(pdev, dev).
- [High] This problem wasn't introduced by this patch, but
  ioc3eth_probe() registers its handler with…

--

net: sgi: ioc3-eth: fall back to the platform MAC address

With this patch, ioc3eth_probe() calls eth_platform_get_mac_address()
when the 1-wire MAC PROM cannot be read. The probe now defers only while
driver_deferred_probe_check_state() returns -EPROBE_DEFER. After deferred
probing gives up, the netdev is registered with a random MAC address.

> diff --git a/drivers/net/ethernet/sgi/ioc3-eth.c b/drivers/net/ethernet/sgi/ioc3-eth.c
> index 25139cf5ff62c..5296614bf3fb8 100644
> --- a/drivers/net/ethernet/sgi/ioc3-eth.c
> +++ b/drivers/net/ethernet/sgi/ioc3-eth.c

[ ... ]

> @@ -857,9 +858,16 @@ static int ioc3eth_probe(struct platform_device *pdev)
>  		dev_err(&pdev->dev, "Invalid resource\n");
>  		return -EINVAL;
>  	}
> -	/* get mac addr from one wire prom */
> -	if (ioc3eth_get_mac_addr(regs, mac_addr))
> -		return -EPROBE_DEFER; /* not available yet */
> +	/* get mac addr from one wire prom, or from the platform */
> +	if (ioc3eth_get_mac_addr(regs, mac_addr) &&
> +	    eth_platform_get_mac_address(&pdev->dev, mac_addr)) {

[Severity: Medium]
Does this ordering let the platform address win before the 1-wire PROM
has had a chance to appear?

eth_platform_get_mac_address() runs as soon as ioc3eth_get_mac_addr()
fails. That happens before driver_deferred_probe_check_state() is
consulted at all. On the first probe attempt the PROM nvmem device
essentially never exists yet:

- mfd_add_devices() registers the ioc3-eth cell before the sgi_w1 cell
  in ioc3_eth_cells[].
- sgi_w1_probe() only registers the master with w1_add_master_device().
  The bus search runs later in the asynchronous w1_process() kthread.
- The nvmem device only appears once w1_ds250x binds the slave in
  w1_eprom_add_slave().

The IOC3 MFD cells have no of_node, so eth_platform_get_mac_address()
falls through to arch_get_platform_mac_address(). That hook takes no
device argument and returns the same system-wide address to every
caller.

Suppose a platform implements that hook, as I assume the rest of the
IP35 series does for Fuel. Then every IOC3 in the machine takes the
system-board address on its first probe and never defers. That covers:

- PCI add-on IOC3 cards such as CAD DUO, MENET or MENET4
- IOC3s in secondary IO bricks

All of these carry their own factory PROM. Their real addresses are
ignored, and several interfaces end up sharing one MAC. That breaks as
soon as two of them sit on the same segment. Nothing re-reads the PROM
later, so the duplicate address stays for as long as the device is
bound.

Should the platform address only be consulted once
driver_deferred_probe_check_state() stops returning -EPROBE_DEFER? If
that brings back the long boot delay on Fuel, could the platform
fallback instead be limited to the system-board IOC3 that is known to
have no PROM?

[Severity: Medium]
Can eth_platform_get_mac_address() ever succeed for this device?

The ioc3-eth platform device is only ever created by this cell in
drivers/mfd/ioc3.c:

static struct mfd_cell ioc3_eth_cells[] = {
	{
		.name = "ioc3-eth",
		.resources = ioc3_eth_resources,
		.num_resources = ARRAY_SIZE(ioc3_eth_resources),
	},

The cell has no of_compatible, swnode or platform_data. So
mfd_add_device() leaves pdev->dev.of_node NULL and attaches no software
node.

eth_platform_get_mac_address() checks only two sources:

net/ethernet/eth.c:eth_platform_get_mac_address() {
	ret = of_get_mac_address(dev->of_node, mac_addr);
	if (!ret)
		return 0;

	addr = arch_get_platform_mac_address();
	if (!addr)
		return -ENODEV;
	...
}

Only sparc overrides arch_get_platform_mac_address(). On MIPS
(IP27/IP30/IP35) the __weak default returns NULL, so this call always
returns -ENODEV.

The commit message says "so a platform that knows the address can supply
it". As far as I can tell, no platform can do that today.

Adding a software node or device property later would not help either,
because eth_platform_get_mac_address() never looks at fwnode properties.
That would need something like device_get_ethdev_address() or
fwnode_get_mac_address().

On Fuel, every probe therefore goes straight to the deferral logic. The
device binds only after the deferred probe timeout expires (up to 10
seconds with CONFIG_MODULES). It then always gets a random MAC that
changes on every boot.

> +		err = driver_deferred_probe_check_state(&pdev->dev);
> +		if (err == -EPROBE_DEFER)
> +			return err; /* not available yet */
> +
> +		dev_warn(&pdev->dev, "no MAC address, using a random one\n");
> +		random_mac = true;
> +	}

[Severity: Medium]
Could IP27/IP30 boards that do have a DS250x MAC PROM end up with a
random MAC here?

driver_deferred_probe_check_state() decides using only global state
(initcalls_done and driver_deferred_probe_timeout). It has no way to know
whether this device's 1-wire PROM can still appear.

ioc3-eth and its PROM provider sgi_w1 are sibling cells in
ioc3_eth_cells[]. The PROM nvmem only appears after these steps:

- the asynchronous w1_process() kthread runs w1_search_process()
- the slave gets attached
- w1_ds250x registers the nvmem device in w1_eprom_add_slave()

SGI_IOC3_ETH and SGI_MFD_IOC3 are bool. W1, W1_MASTER_SGI and
W1_SLAVE_DS250X are tristate and not selected, so they are often modules
that udev loads later.

Before this patch, ioc3-eth returned -EPROBE_DEFER until the slave bound,
so it always picked up the factory MAC. With this patch, the following
sequence is possible:

deferred_probe_timeout_work_func()
  driver_deferred_probe_timeout = 0, retrigger deferred probe
ioc3eth_probe()
  ioc3eth_get_mac_addr()
    nvmem_device_find() fails, PROM not enumerated yet
  driver_deferred_probe_check_state() returns -ETIMEDOUT
  random_mac = true
  eth_hw_addr_random(dev)

On CONFIG_MODULES=n kernels, -ENODEV is returned as soon as
initcalls_done is set. In that case the outcome depends on a race with
the w1 kthread search.

After this, nothing reads the PROM again, so the random address stays
for as long as the device is bound. A transient nvmem or 1-wire read
error in ioc3eth_get_mac_addr() after the deferral window closes has the
same permanent effect.

Should the random fallback be limited to boards that are known to have
no PROM?

> @@ -938,7 +946,10 @@ static int ioc3eth_probe(struct platform_device *pdev)
>  
>  	ioc3_mii_start(ip);
>  	ioc3_ssram_disc(ip);
> -	eth_hw_addr_set(dev, mac_addr);
> +	if (random_mac)
> +		eth_hw_addr_random(dev);
> +	else
> +		eth_hw_addr_set(dev, mac_addr);
>  
>  	/* The IOC3-specific entries in the device structure. */
>  	dev->watchdog_timeo	= 5 * HZ;

[Severity: High]
This is a pre-existing issue, not one introduced by this patch. Now that
ioc3eth_probe() can bind on boards like Fuel, which used to defer
forever, will unbinding the device oops in ioc3eth_remove()?

ioc3eth_probe() never calls platform_set_drvdata(pdev, dev), but
ioc3eth_remove() depends on it:

drivers/net/ethernet/sgi/ioc3-eth.c:ioc3eth_remove() {
	struct net_device *dev = platform_get_drvdata(pdev);
	struct ioc3_private *ip = netdev_priv(dev);

	unregister_netdev(dev);
	timer_delete_sync(&ip->ioc3_timer);
	...
	free_netdev(dev);
}

Here dev is NULL. unregister_netdev(NULL) and
timer_delete_sync(&ip->ioc3_timer) therefore dereference a NULL-derived
pointer. The registered netdev, the timer and the DMA rings are never
torn down.

Two ways to reach this:

- writing to /sys/bus/platform/drivers/ioc3-eth/unbind (the driver does
  not set suppress_bind_attrs)
- unbinding the parent IOC3 PCI device, which goes through
  mfd_remove_devices()

[Severity: High]
This is also a pre-existing issue, not one introduced by this patch. The
patch does make the later probe error paths reachable on Fuel. Is the
shared IRQ handler safe with the current ordering in ioc3eth_probe()?

The IRQ is requested before the lock and timer it uses are initialized:

	if (devm_request_irq(&pdev->dev, dev->irq, ioc3_interrupt,
			     IRQF_SHARED, "ioc3-eth", dev)) {
	...
	spin_lock_init(&ip->ioc3_lock);
	timer_setup(&ip->ioc3_timer, ioc3_timer, 0);

	ioc3_stop(ip);

Suppose a shared-line interrupt arrives in that window:

- ioc3_interrupt()->ioc3_tx() or ioc3_error() would take an
  uninitialized ip->ioc3_lock.
- ioc3_error() would do ring maintenance while ip->rxr is still NULL.

The IRQ is devm-managed, so it is only released after probe returns.
Every error path after the request (ring allocation failure, "Didn't
find a PHY", register_netdev() failure) ends with:

out_free:
	free_netdev(dev);
	return err;

At that point the handler is still registered, and its dev_id points at
the freed net_device. ioc3_interrupt() then reads ip->regs from freed
memory:

	struct ioc3_private *ip = netdev_priv(dev_id);
	struct ioc3_ethregs *regs = ip->regs;
	u32 eisr;

	eisr = readl(&regs->eisr);

With CONFIG_DEBUG_SHIRQ, __free_irq() always calls the handler for
IRQF_SHARED actions when the devres IRQ is released. So on these error
paths this use-after-free happens every time.

ioc3eth_remove() has the same ordering: it calls free_netdev() before
the devres IRQ is released. Today remove oopses before reaching that
point, because drvdata is never set.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261111095138.1757-1-kaloz%40kernel.org