Re: [PATCH net-next v3 04/13] net: ethernet: oa_tc6: Export the C45 access functions

[sashiko-bot@kernel.org]

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

New issues:
- [Low] Missing forward declaration of `struct mii_bus` in `include/linux/oa_tc6.h` makes the header not self-contained.

Pre-existing issues:
- [High] Lockless check in `oa_tc6_start_xmit` races with the SPI thread, leaving the network transmit queue permanently stopped (TX deadlock).
--

commit 03dcdcc6f016c85b284a9ab34918acf9f00e484a
Author: Ciprian Regus <ciprian.regus@analog.com>

net: ethernet: oa_tc6: Export the C45 access functions

This commit exports the C45 MDIO bus access functions from the oa_tc6
driver. It modifies the function signatures to be non-static and adds
their declarations to the driver header file.

> diff --git a/include/linux/oa_tc6.h b/include/linux/oa_tc6.h
> index 62e3d89f80ed0..2660eefa35043 100644
> --- a/include/linux/oa_tc6.h
> +++ b/include/linux/oa_tc6.h
> @@ -31,3 +31,7 @@ int oa_tc6_read_registers(struct oa_tc6 *tc6, u32 address, u32 value[],
>  			  u8 length);
>  netdev_tx_t oa_tc6_start_xmit(struct oa_tc6 *tc6, struct sk_buff *skb);
>  int oa_tc6_zero_align_receive_frame_enable(struct oa_tc6 *tc6);
> +int oa_tc6_mdiobus_read_c45(struct mii_bus *bus, int addr, int devnum,
> +			    int regnum);
> +int oa_tc6_mdiobus_write_c45(struct mii_bus *bus, int addr, int devnum,
> +			     int regnum, u16 val);

[Severity: Low]
Is a forward declaration for struct mii_bus missing in this header?

Since struct mii_bus is used in the parameter list but not declared, it is
scoped to the function prototype. Any file including this header before
declaring struct mii_bus will produce a warning about it not being visible
outside of this declaration.

[Severity: High]
This is a pre-existing issue, but could the lockless check in
oa_tc6_start_xmit() race with the SPI thread and cause a TX deadlock?

Looking at oa_tc6_start_xmit() in drivers/net/ethernet/oa_tc6.c:

	if (tc6->waiting_tx_skb) {
		netif_stop_queue(tc6->netdev);
		return NETDEV_TX_BUSY;
	}

If this evaluates to true, but concurrently the SPI kthread running
oa_tc6_try_spi_transfer() takes tx_skb_lock, moves waiting_tx_skb to
ongoing_tx_skb, and sets waiting_tx_skb to NULL, could the queue wake up be
missed?

oa_tc6_try_spi_transfer() evaluates the queue status:

		if (!tc6->waiting_tx_skb && netif_queue_stopped(tc6->netdev))
			netif_wake_queue(tc6->netdev);

If the kthread evaluates netif_queue_stopped() as false before
oa_tc6_start_xmit() calls netif_stop_queue(), start_xmit will then
unconditionally stop the queue. Since waiting_tx_skb is now NULL, the SPI
thread will not process TX packets and wake the queue again, potentially
deadlocking the TX path permanently unless RX traffic generates an interrupt.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260604-adin1140-driver-v3-0-5debdb3173c4@analog.com?part=4