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

[Michael Walle]

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?

> +	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.

> +		addr = fdt_node_offset_by_phandle(blob, addr);
> +		dmvgbe->phyaddr = fdtdec_get_int(blob, addr, "reg", 0);
> +	}
> +
> +	return 0;
> +}


-michael