Re: [PATCH 4/6] net: fec: add support for DM_MDIO

[Vladimir Oltean <vladimir.oltean@nxp.com>]

On Tue, Mar 29, 2022 at 03:52:38PM -0700, Tim Harvey wrote:
> Add support for DM_MDIO by registering a UCLASS_MDIO driver and
> attempting to use it. This is necessary if wanting to use a DSA
> driver for example hanging off of the FEC MAC.
> 
> Care is taken to fallback to non DM_MDIO as several boards define
> DM_MDIO without having the proper device-tree configuration necessary
> such as an mdio subnode, a phy-mode prop, and either a valid phy-handle
> prop or fixed-phy subnode.
> 
> Signed-off-by: Tim Harvey <tharvey@gateworks.com>
> ---
>  drivers/net/fec_mxc.c | 113 ++++++++++++++++++++++++++++++++++++++----
>  drivers/net/fec_mxc.h |   1 +
>  2 files changed, 104 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
> index e8ebef09032a..374ef9d67d5e 100644
> --- a/drivers/net/fec_mxc.c
> +++ b/drivers/net/fec_mxc.c
> @@ -30,6 +30,8 @@
>  #include <asm/arch/imx-regs.h>
>  #include <asm/mach-imx/sys_proto.h>
>  #include <asm-generic/gpio.h>
> +#include <dm/device_compat.h>
> +#include <dm/lists.h>
>  
>  #include "fec_mxc.h"
>  #include <eth_phy.h>
> @@ -1025,6 +1027,80 @@ struct mii_dev *fec_get_miibus(ulong base_addr, int dev_id)
>  	return bus;
>  }
>  
> +#ifdef CONFIG_DM_MDIO
> +struct dm_fec_mdio_priv {
> +	struct ethernet_regs *regs;
> +};
> +
> +static int dm_fec_mdio_read(struct udevice *dev, int addr, int devad, int reg)
> +{
> +	struct dm_fec_mdio_priv *priv = dev_get_priv(dev);
> +
> +	return fec_mdio_read(priv->regs, addr, reg);
> +}
> +
> +static int dm_fec_mdio_write(struct udevice *dev, int addr, int devad, int reg, u16 data)
> +{
> +	struct dm_fec_mdio_priv *priv = dev_get_priv(dev);
> +
> +	return fec_mdio_write(priv->regs, addr, reg, data);
> +}
> +
> +static const struct mdio_ops dm_fec_mdio_ops = {
> +	.read = dm_fec_mdio_read,
> +	.write = dm_fec_mdio_write,
> +};
> +
> +static int dm_fec_mdio_probe(struct udevice *dev)
> +{
> +	struct dm_fec_mdio_priv *priv = dev_get_priv(dev);
> +
> +	priv->regs = (struct ethernet_regs *)ofnode_get_addr(dev_ofnode(dev->parent));
> +
> +	return 0;
> +}
> +
> +U_BOOT_DRIVER(fec_mdio) = {
> +	.name		= "fec_mdio",
> +	.id		= UCLASS_MDIO,
> +	.probe		= dm_fec_mdio_probe,
> +	.ops		= &dm_fec_mdio_ops,
> +	.priv_auto	= sizeof(struct dm_fec_mdio_priv),
> +};
> +
> +static int dm_fec_bind_mdio(struct udevice *dev)
> +{
> +	struct udevice *mdiodev;
> +	const char *name;
> +	ofnode mdio;
> +	int ret;
> +
> +	/* for a UCLASS_MDIO driver we need to bind and probe manually
> +	 * for an internal MDIO bus that has no dt compatible of its own
> +	 */
> +	ofnode_for_each_subnode(mdio, dev_ofnode(dev)) {
> +		name = ofnode_get_name(mdio);
> +
> +		if (strcmp(name, "mdio"))
> +			continue;
> +
> +		ret = device_bind_driver_to_node(dev, "fec_mdio",
> +						 name, mdio, &mdiodev);
> +		if (ret)
> +			return ret;
> +
> +		/* need to probe it as there is no compatible to do so */
> +		ret = uclass_get_device_by_ofnode(UCLASS_MDIO, mdio, &mdiodev);
> +		if (!ret)
> +			return 0;
> +
> +		break;
> +	}
> +
> +	return ret;
> +}
> +#endif
> +

On which branch does this apply? The context above fecmxc_read_rom_hwaddr()
is different in the branches I've checked:
https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/net/fec_mxc.c#L1276
https://source.denx.de/u-boot/custodians/u-boot-net/-/blob/next/drivers/net/fec_mxc.c#L1276

>  static int fecmxc_read_rom_hwaddr(struct udevice *dev)
>  {
>  	struct fec_priv *priv = dev_get_priv(dev);
> @@ -1088,7 +1164,7 @@ static int device_get_phy_addr(struct fec_priv *priv, struct udevice *dev)
>  
>  static int fec_phy_init(struct fec_priv *priv, struct udevice *dev)
>  {
> -	struct phy_device *phydev;
> +	struct phy_device *phydev = NULL;
>  	int addr;
>  
>  	addr = device_get_phy_addr(priv, dev);
> @@ -1096,7 +1172,12 @@ static int fec_phy_init(struct fec_priv *priv, struct udevice *dev)
>  	addr = CONFIG_FEC_MXC_PHYADDR;
>  #endif
>  
> -	phydev = phy_connect(priv->bus, addr, dev, priv->interface);
> +#ifdef CONFIG_DM_MDIO
> +	if (priv->dm_mdio)
> +		phydev = dm_eth_phy_connect(dev);
> +#endif
> +	if (!phydev)
> +		phydev = phy_connect(priv->bus, addr, dev, priv->interface);
>  	if (!phydev)
>  		return -ENODEV;
>  
> @@ -1227,11 +1308,19 @@ static int fecmxc_probe(struct udevice *dev)
>  
>  	priv->dev_id = dev_seq(dev);
>  
> +#ifdef CONFIG_DM_MDIO
> +	ret = dm_fec_bind_mdio(dev);
> +	if (!ret) {
> +		ret = fec_phy_init(priv, dev);
> +		if (!ret)
> +			priv->dm_mdio = true;
> +	}
> +#endif
>  #ifdef CONFIG_DM_ETH_PHY
>  	bus = eth_phy_get_mdio_bus(dev);
>  #endif
>  
> -	if (!bus) {
> +	if (!bus && !priv->dm_mdio) {
>  		dm_mii_bus = false;
>  #ifdef CONFIG_FEC_MXC_MDIO_BASE
>  		bus = fec_get_miibus((ulong)CONFIG_FEC_MXC_MDIO_BASE,
> @@ -1240,7 +1329,7 @@ static int fecmxc_probe(struct udevice *dev)
>  		bus = fec_get_miibus((ulong)priv->eth, dev_seq(dev));
>  #endif
>  	}
> -	if (!bus) {
> +	if (!bus && !priv->dm_mdio) {
>  		ret = -ENOMEM;
>  		goto err_mii;
>  	}
> @@ -1271,14 +1360,16 @@ static int fecmxc_probe(struct udevice *dev)
>  		break;
>  	}
>  
> -	ret = fec_phy_init(priv, dev);
> -	if (ret)
> -		goto err_phy;
> +	if (!priv->dm_mdio) {
> +		ret = fec_phy_init(priv, dev);
> +		if (ret)
> +			goto err_phy;
> +	}
>  
>  	return 0;
>  
>  err_phy:
> -	if (!dm_mii_bus) {
> +	if (!dm_mii_bus && !priv->dm_mdio) {
>  		mdio_unregister(bus);
>  		free(bus);
>  	}
> @@ -1294,8 +1385,10 @@ static int fecmxc_remove(struct udevice *dev)
>  
>  	free(priv->phydev);
>  	fec_free_descs(priv);
> -	mdio_unregister(priv->bus);
> -	mdio_free(priv->bus);
> +	if (priv->bus) {
> +		mdio_unregister(priv->bus);
> +		mdio_free(priv->bus);
> +	}
>  
>  #ifdef CONFIG_DM_REGULATOR
>  	if (priv->phy_supply)
> diff --git a/drivers/net/fec_mxc.h b/drivers/net/fec_mxc.h
> index 48faa33d66ec..c297880ccc54 100644
> --- a/drivers/net/fec_mxc.h
> +++ b/drivers/net/fec_mxc.h
> @@ -248,6 +248,7 @@ struct fec_priv {
>  	uint8_t *tdb_ptr;
>  	int dev_id;
>  	struct mii_dev *bus;
> +	bool dm_mdio;
>  #ifdef CONFIG_PHYLIB
>  	struct phy_device *phydev;
>  	ofnode phy_of_node;
> -- 
> 2.17.1
>