On Tue, Apr 20, 2021 at 3:47 AM Andrew Lunn <and...@lunn.ch> wrote: > > @@ -1381,25 +1382,12 @@ static struct eth_plat_info > > *ixp4xx_of_get_platdata(struct device *dev) > > /* NPE ID 0x00, 0x10, 0x20... */ > > plat->npe = (val << 4); > > > > - phy_np = of_parse_phandle(np, "phy-handle", 0); > > - if (phy_np) { > > - ret = of_property_read_u32(phy_np, "reg", &val); > > - if (ret) { > > - dev_err(dev, "cannot find phy reg\n"); > > - return NULL; > > - } > > - of_node_put(phy_np); > > - } else { > > - dev_err(dev, "cannot find phy instance\n"); > > - val = 0; > > - } > > - plat->phy = val; > > - > > Isn't this code you just added in the previous patch?
Yep. It's by the token of "one technical step per patch" but I suggested that maybe you prefer to take two technical steps in one big patch, then we can just squash patches 2 & 3. I'll fix it as you want it just tell me how :) > > - snprintf(phy_id, MII_BUS_ID_SIZE + 3, PHY_ID_FMT, > > - mdio_bus->id, plat->phy); > > - phydev = phy_connect(ndev, phy_id, &ixp4xx_adjust_link, > > - PHY_INTERFACE_MODE_MII); > > + if (np) { > > + phydev = of_phy_get_and_connect(ndev, np, ixp4xx_adjust_link); > > + } else { > > + snprintf(phy_id, MII_BUS_ID_SIZE + 3, PHY_ID_FMT, > > + mdio_bus->id, plat->phy); > > mdiobus_get_phy() and phy_connect_direct() might be better. Do you mean for the legacy code path (else clause), or the new code path with of_phy_get_and_connect() or both? I tried not to change the legacy code in order to not introduce regressions, so if I change that I suppose it should be a separate patch. On the other hand this driver has not been much maintained the recent years so we might need to be a bit rough when bringing it into shape. (After migrating all of IXP4xx to device tree a lot of the legacy code will eventually be deleted.) Yours, Linus Walleij