On 3/30/2020 9:30 AM, Andrew Lunn wrote: > On Mon, Mar 30, 2020 at 07:01:36PM +0300, Codrin Ciubotariu wrote: >> Some ethernet controllers, such as cadence's macb, have an embedded MDIO. >> For this reason, the ethernet PHY nodes are not under an MDIO bus, but >> directly under the ethernet node. Since these drivers might use >> of_mdiobus_child_is_phy(), we should fix this function by returning false >> if a fixed-link is found. > > So i assume the problem occurs here: > > static int macb_mdiobus_register(struct macb *bp) > { > struct device_node *child, *np = bp->pdev->dev.of_node; > > /* Only create the PHY from the device tree if at least one PHY is > * described. Otherwise scan the entire MDIO bus. We do this to support > * old device tree that did not follow the best practices and did not > * describe their network PHYs. > */ > for_each_available_child_of_node(np, child) > if (of_mdiobus_child_is_phy(child)) { > /* The loop increments the child refcount, > * decrement it before returning. > */ > of_node_put(child); > > return of_mdiobus_register(bp->mii_bus, np); > } > > return mdiobus_register(bp->mii_bus); > } > > I think a better solution is > > for_each_available_child_of_node(np, child) > + if (of_phy_is_fixed_link(child) > + continue; > if (of_mdiobus_child_is_phy(child)) { > /* The loop increments the child refcount, > * decrement it before returning. > */ > of_node_put(child); > > return of_mdiobus_register(bp->mii_bus, np); > } > > return mdiobus_register(bp->mii_bus); > } > > This problem is only an issue for macb, so keep the fix local to macb. Agree, there is no reason for of_mdiobus_child_is_phy() to be checking for a fixed-link. If you submit this formally: Acked-by: Florian Fainelli <f.fainelli@xxxxxxxxx> -- Florian