Thread (14 messages) flat view 14 messages, 2 authors, 2d ago

Re: [PATCH net-next 2/3] net: ethernet: oa_tc6: deliver the PHY interrupt to phylib

From: Parthiban Veerasooran <parthiban.veerasooran@microchip.com>
Date: 2026-09-04 06:05:47
Also in: lkml

On 03/09/26 7:44 pm, Andrew Lunn wrote:
EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

On Thu, Sep 03, 2026 at 06:53:40PM +0530, Parthiban Veerasooran wrote:
quoted
Hi Andrew,

On 02/09/26 6:00 am, Andrew Lunn wrote:
quoted
EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
quoted
@@ -600,6 +642,16 @@ static int oa_tc6_phy_init(struct oa_tc6 *tc6)
                return -ENODEV;
        }

+     ret = oa_tc6_phy_irq_setup(tc6);
+     if (ret) {
+             oa_tc6_mdiobus_unregister(tc6);
+             return ret;
+     }
+
+     /* Deliver the PHY interrupt through the nested virtual IRQ. Set before
+      * phy_connect_direct() so phylib enters interrupt mode.
+      */
+     tc6->phydev->irq = tc6->phy_virq;
I don't know how messy it will be, but it is better to set
mii_bus->irq[] to the interrupt number. phy_device_create() will then
copy it into phydev->irq.
quoted
Thanks for the suggestion. To use mii_bus->irq[] so that phy_device_create()
picks it up, we would need to set mii_bus->irq[addr] with created virtual
irq number before mdiobus_register(). However, the PHY MDIO address is not
known until phy_find_first() returns, so we cannot pre-populate
mii_bus->irq[addr] before the bus scan runs.
This is why i made the comment, i did not know how messy it would be.

Where it becomes interesting is the recent patch:

https://patchwork.kernel.org/project/netdevbpf/patch/20260902080511.2211261-3-f@lex.la/

It just seems a bit brittle, phydev->irq says one thing, mii_bus->irq[]
says something else.

Maybe set all member of mii_bus->irq[]?
Thanks for pointing to that patch. Now the concern is clear: 
mdiobus_alloc() initialises all bus->irq[] entries to PHY_POLL, so if 
phydev->irq is set directly without updating mii_bus->irq[], 
phy_restore_genphy_irq() would restore phydev->irq back to PHY_POLL if 
the generic driver ever binds.

So the fix would be to move oa_tc6_phy_irq_setup() before 
mdiobus_register(), populate all mii_bus->irq[] entries with the virtual 
IRQ, and drop the direct phydev->irq assignment. phy_device_create() 
then copies the correct value from bus->irq[addr] during the bus scan, 
keeping both tables consistent.

+     /* Populate all irq[] entries before registration so
+      * phy_device_create() picks up the virtual IRQ regardless of
+      * the PHY's MDIO address.
+      */
+     for (i = 0; i < PHY_MAX_ADDR; i++)
+             tc6->mdiobus->irq[i] = tc6->phy_virq;

       ret = mdiobus_register(tc6->mdiobus);
       if (ret) {
               netdev_err(tc6->netdev, "Could not register MDIO bus\n");
               mdiobus_free(tc6->mdiobus);
               return ret;
       }

Hope this is what expected?

Best regards,
Parthiban V
       Andrew
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help