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

[Parthiban.Veerasooran]

On 03/10/26 7:06 pm, netdev-bot+sashiko@xxxxxxxxxx wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> 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?

OA_TC6_PHY_INT intentionally switches LAN865X from phylib polling to the 
PHY interrupt path, since LAN865X reliably provides the in-band PHYINT 
indication. A failed oa_tc6_phy_irq_unmask_hw() is a MAC-PHY control 
transaction failure, so the resulting hardware mask state cannot be 
reliably determined and is reported as an error. Retrying asynchronously 
or retaining a periodic CDEN resync would introduce a second recovery 
path and additional IRQ/lifecycle synchronization. The normal interrupt 
path has been tested with LAN865X on the final series.

Best regards,
Parthiban V

>>        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