Re: [PATCH v2] bcm63xx_enet: fix internal phy IRQ assignment

On 2/25/2021 2:56 PM, Heiner Kallweit wrote:

quoted

quoted

quoted

quoted

It's still an ugly workaround and a proper root cause analysis should be done
first. I can only imagine that phydev->irq is overwritten in phy_probe()
because phy_drv_supports_irq() is false. Can you please check whether
phydev->irq is properly set in phy_device_create(), and if yes, whether
it's reset to PHY_POLL in phy_probe()?.

Hi Heiner, I added some kernel prints:

[    2.712519] libphy: Fixed MDIO Bus: probed
[    2.721969] =======phy_device_create===========
[    2.726841] phy_device_create: dev->irq = 17
[    2.726841]
[    2.832620] =======phy_probe===========
[    2.836846] phy_probe: phydev->irq = 17
[    2.840950] phy_probe: phy_drv_supports_irq = 0, phy_interrupt_is_valid = 1
[    2.848267] phy_probe: phydev->irq = -1
[    2.848267]
[    2.854059] =======phy_probe===========
[    2.858174] phy_probe: phydev->irq = -1
[    2.862253] phy_probe: phydev->irq = -1
[    2.862253]
[    2.868121] libphy: bcm63xx_enet MII bus: probed
[    2.873320] Broadcom BCM63XX (1) bcm63xx_enet-0:01: attached PHY
driver [Broadcom BCM63XX (1)] (mii_bus:phy_addr=bcm63xx_enet-0:01,
irq=POLL)

Currently using kernel 5.4.99. I still have no idea what's going on.

Thanks for debugging. This confirms my assumption that the interrupt
is overwritten in phy_probe(). I'm just scratching my head how
phy_drv_supports_irq() can return 0. In 5.4.99 it's defined as:

static bool phy_drv_supports_irq(struct phy_driver *phydrv)
{
        return phydrv->config_intr && phydrv->ack_interrupt;
}

And that's the PHY driver:

static struct phy_driver bcm63xx_driver[] = {
{
        .phy_id         = 0x00406000,
        .phy_id_mask    = 0xfffffc00,
        .name           = "Broadcom BCM63XX (1)",
        /* PHY_BASIC_FEATURES */
        .flags          = PHY_IS_INTERNAL,
        .config_init    = bcm63xx_config_init,
        .ack_interrupt  = bcm_phy_ack_intr,
        .config_intr    = bcm63xx_config_intr,
}

So both callbacks are set. Can you extend your debugging and check
in phy_drv_supports_irq() which of the callbacks is missing?

Hi, both callbacks are missing on the first check. However on the next
calls they're there.

[    2.263909] libphy: Fixed MDIO Bus: probed

This is weird. The phy_device seems to show up on both MDIO buses,
the fixed one *and* the bcm63xx_enet bus.

Yes that does not make sense to me at all, but maybe something broke at
some point for non-Device Tree systems and we are just catching it now.
The largest rework that occurred during v4.4 and v4.9 was the
introduction of mdio_device.
-- 
Florian