[U-Boot] [PATCH v2 2/2] net: mvgbe: convert to DM

[Chris Packham]

On Mon, Jul 9, 2018 at 9:05 AM Michael Walle <michael at 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 <michael at 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