Re: [PATCH net-next v4 3/4] microchip: lan865x: enable PHY interrupt via virtual IRQ

[netdev-bot+sashiko@kernel.org]

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

Critical: 0 · High: 0 · Medium: 1 · 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] This patch sets OA_TC6_PHY_INT for LAN865X, which moves the
  internal PHY from phylib polling to interrupt mode.

--

microchip: lan865x: enable PHY interrupt via virtual IRQ

This patch passes the OA_TC6_PHY_INT quirk flag to oa_tc6_init() from
lan865x_probe(). The internal LAN865X PHY then runs phylib in interrupt
mode through the oa_tc6 virtual IRQ, so CDEN follows PLCA transitions via
lan86xx_handle_interrupt() and no longer depends on the polling fallback.

> diff --git a/drivers/net/ethernet/microchip/lan865x/lan865x.c b/drivers/net/ethernet/microchip/lan865x/lan865x.c
> index 127afb9e9f141..d47560f9847d5 100644
> --- a/drivers/net/ethernet/microchip/lan865x/lan865x.c
> +++ b/drivers/net/ethernet/microchip/lan865x/lan865x.c

[ ... ]

> @@ -346,7 +347,8 @@ static int lan865x_probe(struct spi_device *spi)
>  	spi_set_drvdata(spi, priv);
>  	INIT_WORK(&priv->multicast_work, lan865x_multicast_work_handler);
>  
> -	priv->tc6 = oa_tc6_init(spi, netdev, NULL);
> +	tc6_quirks.quirk_flags = OA_TC6_PHY_INT;
> +	priv->tc6 = oa_tc6_init(spi, netdev, &tc6_quirks);

[Severity: Medium]
Can one failed SPI transfer permanently stop CDEN tracking for LAN865X
once this flag is set?

With OA_TC6_PHY_INT, oa_tc6_mdiobus_register() puts tc6->phy_virq into
mii_bus->irq[]. phy_polling_mode() then returns false, and
_phy_state_machine() stops requeueing every PHY_STATE_TIME.

That means lan86xx_read_status()->lan86xx_update_cden() no longer runs
periodically. That periodic resync is the self-healing fallback added by
"net: phy: microchip_t1s: fix collision detection on PLCA status change".

In interrupt mode, oa_tc6_process_extended_status() masks PHYINT in
hardware before it schedules the worker:

drivers/net/ethernet/oa_tc6.c:oa_tc6_process_extended_status() {
    ...
	if (tc6->phy_virq && FIELD_GET(OA_TC6_STATUS0_PHY_INT, value)) {
		ret = oa_tc6_phy_irq_mask_hw(tc6);
		...
		else
			schedule_work(&tc6->phy_irq_work);
	}
    ...
}

The worker then tries to unmask it, but it only logs a failure:

drivers/net/ethernet/oa_tc6.c:oa_tc6_phy_irq_work() {
    ...
	handle_nested_irq(tc6->phy_virq);

	ret = oa_tc6_phy_irq_unmask_hw(tc6);
	if (ret)
		dev_err(&tc6->spi->dev, "Failed to unmask PHY interrupt: %d\n",
			ret);
}

Suppose the INT_MASK0 read or write in oa_tc6_phy_irq_unmask_hw() returns
an error. INT_MASK0.PHYINT then stays set in hardware, while
tc6->phy_irq_masked is still false.

After that, later PLCA status changes never raise EXST, so
lan86xx_handle_interrupt() stops running. Nothing retries or reschedules
the work, and polling is off.

The normal ifdown/ifup path (phy_stop()/phy_start()) does not go through
disable_irq()/enable_irq(), so oa_tc6_phy_irq_bus_sync_unlock() is never
called to restore the mask. Only suspend/resume or unbinding and
re-probing the driver seems to recover it.

Before this patch, the 1 second poll would have fixed CDEN shortly after
such an error. Would it make sense to retry or reschedule the unmask on
failure, or to keep some periodic CDEN resync for this case?

>  	if (IS_ERR(priv->tc6)) {
>  		ret = PTR_ERR(priv->tc6);
>  		goto free_netdev;

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260929125928.611784-1-parthiban.veerasooran%40microchip.com