Re: [PATCH net-next 4/5] net: ethernet: adi: adin1110: Add OA TC6 protocol support for ADIN1110

[Stolniceanu, Iustin]

On Wed, Nov 04, 2026 at 03:26:07PM +0100, Alvin Šipraga wrote:
> > The ADIN2111 is not supported in OPEN Alliance TC6 mode yet, and probe
> > fails with -EOPNOTSUPP.
>
> Not sure -EOPNOTSUPP is correct, maybe -EIO or something?

I went with -ENODEV in v2. The I/O itself works; the driver just can't
handle this device in this configuration. -ENODEV also matches what the
strap/DT mismatch check returns.

> guard(mutex)(&priv->lock)?
>
> OK, but the rest of the driver doesn't use that, so I can see why you
> refrained from going that way.

Andrew pointed out that netdev weakly prefers plain
lock/unlock, so I kept it.

> For the above, it wasn't quite clear to me why you don't call
> adin1110_port_set_forwarding_state()?

It does the same as the generic adin1110_net_open(), which also only 
sets the port state and the MAC address. The extra step in 
adin1110_port_set_forwarding_state() is adin1110_setup_rx_mode(), but 
the core calls ndo_set_rx_mode_async right after ndo_open, which already 
does that with the right flags. On the first open port_priv->flags is still 0, 
so calling it from open would briefly turn off broadcast reception. 
The hardware forwarding branch only applies to the ADIN2111. 


> Would it make sense to use the async mode for the non-tc6 case as well?
> Then you can drop the work there and have the same ndo callback for both.

It would, but it changes the generic SPI path, which is out of scope 
for this series, and that path needs its own regression testing 
(including ADIN2111). I'd prefer to do it as a separate follow-up after this 
series, if that's OK.

> Spurious newline above.
> Spurious newline.

Both fixed in v2. 

> > +struct mii_bus;
>
> A mistake here? Doesn't seem related.

It is needed. adin1110.c includes oa_tc6.h before linux/phy.h, and the
Clause 45 MDIO prototypes in oa_tc6.h use struct mii_bus. Without the
forward declaration a W=1 build fails with "'struct mii_bus' declared
inside parameter list". I've explained this in the v2 commit message.

Thanks,
Iustin