On Mon, Jul 9, 2018 at 9:05 AM Michael Walle <mich...@walle.cc> wrote: > > > Hi Chris, > > thanks for your efforts. This basically works for me on my Kirkwood > LSCHLv2 board. So you may add > > Tested-by: Michael Walle <mich...@walle.cc> > > But there are some issues. See below. > > > [snip] > > > -#if defined(CONFIG_PHYLIB) > > +#if defined(CONFIG_PHYLIB) || defined(CONFIG_DM_ETH) > > +#if defined(CONFIG_DM_ETH) > > +static struct phy_device *__mvgbe_phy_init(struct udevice *dev, > > + struct mii_dev *bus, > > + phy_interface_t phy_interface, > > + int phyid) > > +#else > > +static struct phy_device *__mvgbe_phy_init(struct eth_device *dev, > > + struct mii_dev *bus, > > + phy_interface_t phy_interface, > > + int phyid) > > +#endif > > +{ > > + struct phy_device *phydev; > > + > > + /* Set phy address of the port */ > > + miiphy_write(dev->name, MV_PHY_ADR_REQUEST, MV_PHY_ADR_REQUEST, > > + phyid); > > + > > + phydev = phy_connect(bus, phyid, dev, phy_interface); > > So CONFIG_PHYLIB is required if CONFIG_DM_ETH is enabled? If so, then > can we add > > selects PHYLIB if DM_ETH > > to the CONFIG_MVGBE Kconfig part? >
Yes will do. I even erroneously added it at one point (without the if DM_ETH). > > + if (!phydev) { > > + printf("phy_connect failed\n"); > > + return NULL; > > + } > > + > > + phy_config(phydev); > > + phy_startup(phydev); > > + > > + return phydev; > > +} > > +#endif /* CONFIG_PHYLIB || CONFIG_DM_ETH */ > > + > > [snip] > > > +static int mvgbe_ofdata_to_platdata(struct udevice *dev) > > +{ > > + struct eth_pdata *pdata = dev_get_platdata(dev); > > + struct mvgbe_device *dmvgbe = dev_get_priv(dev); > > + void *blob = (void *)gd->fdt_blob; > > + int node = dev_of_offset(dev); > > + const char *phy_mode; > > + int fl_node; > > + unsigned long addr; > > + > > + pdata->iobase = devfdt_get_addr(dev); > > + > > + /* Get phy-mode / phy_interface from DT */ > > + pdata->phy_interface = -1; > > + phy_mode = fdt_getprop(gd->fdt_blob, dev_of_offset(dev), "phy-mode", > > + NULL); > > + if (phy_mode) > > + pdata->phy_interface = phy_get_interface_by_name(phy_mode); > > + if (pdata->phy_interface == -1) { > > + debug("%s: Invalid PHY interface '%s'\n", __func__, phy_mode); > > + return -EINVAL; > > + } > > + > > + dmvgbe->phy_interface = pdata->phy_interface; > > + > > + /* fetch 'fixed-link' property */ > > + fl_node = fdt_subnode_offset(blob, node, "fixed-link"); > > + if (fl_node != -FDT_ERR_NOTFOUND) { > > + /* set phy_addr to invalid value for fixed link */ > > + dmvgbe->phyaddr = PHY_MAX_ADDR + 1; > > + dmvgbe->duplex = fdtdec_get_bool(blob, fl_node, > > "full-duplex"); > > + dmvgbe->speed = fdtdec_get_int(blob, fl_node, "speed", 0); > > + } else { > > + /* Now read phyaddr from DT */ > > + addr = fdtdec_get_int(blob, node, "phy", 0); > > Should be "phy-handle", shouldn't it? And isn't there already function > for this which gets the phy address? Also, the phy-handle is on the port > subnode in the device tree. I guess, this applies to phy-mode, too. > Hmm my hacky dts for testing this called it "phy" and attached it to the eth node. I'll look at a couple of proper boards and code to them. > > + addr = fdt_node_offset_by_phandle(blob, addr); > > + dmvgbe->phyaddr = fdtdec_get_int(blob, addr, "reg", 0); > > + } > > + > > + return 0; > > +} > > > -michael _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot