Re: [PATCHv3 (net.git) 2/2] stmmac: fix MDIO settings

["Frank Schäfer" <fschaefer.oss@googlemail.com>]

Hi Giuseppe,

Am 11.03.2016 um 14:33 schrieb Giuseppe Cavallaro:
> Initially the phy_bus_name was added to manipulate the
> driver name but It was recently just used to manage the
> fixed-link and then to take some decision at run-time
> inside the main (for example to skip EEE).
> So the patch uses the is_pseudo_fixed_link and removes
> removes the phy_bus_name variable not necessary anymore.
>
> The driver can manage the mdio registration by using phy-handle,
> dwmac-mdio and own parameter e.g. snps,phy-addr.
> This patch takes care about all these possible configurations
> and fixes the mdio registration in case of there is a real
> transceiver or a switch (that needs to be managed by using
> fixed-link).
>
> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> Reviewed-by: Andreas Färber <afaerber@suse.de>
> Tested-by: Frank Schäfer <fschaefer.oss@googlemail.com>
> Cc: Gabriel Fernandez <gabriel.fernandez@linaro.org>
> Cc: Dinh Nguyen <dinh.linux@gmail.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Phil Reid <preid@electromag.com.au>
> ---
>
> V2: use is_pseudo_fixed_link
> V3: parse device-tree driver parameters to allocate PHY resources considering
>     DSA case (+ fixed-link).
>
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  |   11 +--
>  drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c  |   19 +-----
>  .../net/ethernet/stmicro/stmmac/stmmac_platform.c  |   77 ++++++++++++++++----
>  include/linux/stmmac.h                             |    2 +-
>  4 files changed, 68 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index c21015b..389d7d0 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -271,7 +271,6 @@ static void stmmac_eee_ctrl_timer(unsigned long arg)
>   */
>  bool stmmac_eee_init(struct stmmac_priv *priv)
>  {
> -	char *phy_bus_name = priv->plat->phy_bus_name;
>  	unsigned long flags;
>  	bool ret = false;
>  
> @@ -283,7 +282,7 @@ bool stmmac_eee_init(struct stmmac_priv *priv)
>  		goto out;
>  
>  	/* Never init EEE in case of a switch is attached */
> -	if (phy_bus_name && (!strcmp(phy_bus_name, "fixed")))
> +	if (priv->phydev->is_pseudo_fixed_link)
>  		goto out;
>  
>  	/* MAC core supports the EEE feature. */
> @@ -820,12 +819,8 @@ static int stmmac_init_phy(struct net_device *dev)
>  		phydev = of_phy_connect(dev, priv->plat->phy_node,
>  					&stmmac_adjust_link, 0, interface);
>  	} else {
> -		if (priv->plat->phy_bus_name)
> -			snprintf(bus_id, MII_BUS_ID_SIZE, "%s-%x",
> -				 priv->plat->phy_bus_name, priv->plat->bus_id);
> -		else
> -			snprintf(bus_id, MII_BUS_ID_SIZE, "stmmac-%x",
> -				 priv->plat->bus_id);
> +		snprintf(bus_id, MII_BUS_ID_SIZE, "stmmac-%x",
> +			 priv->plat->bus_id);
>  
>  		snprintf(phy_id_fmt, MII_BUS_ID_SIZE + 3, PHY_ID_FMT, bus_id,
>  			 priv->plat->phy_addr);
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
> index 0faf163..3f5512f 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
> @@ -198,29 +198,12 @@ int stmmac_mdio_register(struct net_device *ndev)
>  	struct mii_bus *new_bus;
>  	struct stmmac_priv *priv = netdev_priv(ndev);
>  	struct stmmac_mdio_bus_data *mdio_bus_data = priv->plat->mdio_bus_data;
> +	struct device_node *mdio_node = priv->plat->mdio_node;
>  	int addr, found;
> -	struct device_node *mdio_node = NULL;
> -	struct device_node *child_node = NULL;
>  
>  	if (!mdio_bus_data)
>  		return 0;
>  
> -	if (IS_ENABLED(CONFIG_OF)) {
> -		for_each_child_of_node(priv->device->of_node, child_node) {
> -			if (of_device_is_compatible(child_node,
> -						    "snps,dwmac-mdio")) {
> -				mdio_node = child_node;
> -				break;
> -			}
> -		}
> -
> -		if (mdio_node) {
> -			netdev_dbg(ndev, "FOUND MDIO subnode\n");
> -		} else {
> -			netdev_warn(ndev, "No MDIO subnode found\n");
> -		}
> -	}
> -
>  	new_bus = mdiobus_alloc();
>  	if (new_bus == NULL)
>  		return -ENOMEM;
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> index 6a52fa1..d2322e9 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> @@ -96,6 +96,66 @@ static int dwmac1000_validate_ucast_entries(int ucast_entries)
>  }
>  
>  /**
> + * stmmac_dt_phy - parse device-tree driver parameters to allocate PHY resources
> + * @plat: driver data platform structure
> + * @np: device tree node
> + * @dev: device pointer
> + * Description:
> + * The mdio bus will be allocated in case of a phy transceiver is on board;
> + * it will be NULL if the fixed-link is configured.
> + * If there is the "snps,dwmac-mdio" sub-node the mdio will be allocated
> + * in any case (for DSA, mdio must be registered even if fixed-link).
> + * The table below sums the supported configurations:
> + *	-------------------------------
> + *	snps,phy-addr	|     Y
> + *	-------------------------------
> + *	phy-handle	|     Y
> + *	-------------------------------
> + *	fixed-link	|     N
> + *	-------------------------------
> + *	snps,dwmac-mdio	|
> + *	  even if	|     Y
> + *	fixed-link	|
> + *	-------------------------------
> + *
> + * It returns true in case of the mdio needs to be registered in the main.
> + */
> +static bool stmmac_dt_phy(struct plat_stmmacenet_data *plat,
> +			  struct device_node *np, struct device *dev)
> +{
> +	bool ret = true;
> +
> +	/* If phy-handle property is passed from DT, use it as the PHY */
> +	plat->phy_node = of_parse_phandle(np, "phy-handle", 0);
> +	if (plat->phy_node)
> +		dev_dbg(dev, "Found phy-handle subnode\n");
> +
> +	/* If phy-handle is not specified, check if we have a fixed-phy */
> +	if (!plat->phy_node && of_phy_is_fixed_link(np)) {
> +		if ((of_phy_register_fixed_link(np) < 0))
> +			return -ENODEV;
> +
> +		dev_dbg(dev, "Found fixed-link subnode\n");
> +		plat->phy_node = of_node_get(np);
> +
> +		ret = false;
> +	}
> +
> +	/* If snps,dwmac-mdio is passed from DT, always register the MDIO */
> +	for_each_child_of_node(np, plat->mdio_node) {
> +		if (of_device_is_compatible(plat->mdio_node, "snps,dwmac-mdio"))
> +			break;
> +	}
> +
> +	if (plat->mdio_node) {
> +		dev_dbg(dev, "Found MDIO subnode\n");
> +		ret = true;
> +	}
> +
> +	return ret;
> +}
> +
> +/**
>   * stmmac_probe_config_dt - parse device-tree driver parameters
>   * @pdev: platform_device structure
>   * @plat: driver data platform structure
> @@ -129,30 +189,19 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
>  	/* Default to phy auto-detection */
>  	plat->phy_addr = -1;
>  
> -	/* If we find a phy-handle property, use it as the PHY */
> -	plat->phy_node = of_parse_phandle(np, "phy-handle", 0);
> -
> -	/* If phy-handle is not specified, check if we have a fixed-phy */
> -	if (!plat->phy_node && of_phy_is_fixed_link(np)) {
> -		if ((of_phy_register_fixed_link(np) < 0))
> -			return ERR_PTR(-ENODEV);
> -
> -		plat->phy_node = of_node_get(np);
> -	}
> -
>  	/* "snps,phy-addr" is not a standard property. Mark it as deprecated
>  	 * and warn of its use. Remove this when phy node support is added.
>  	 */
>  	if (of_property_read_u32(np, "snps,phy-addr", &plat->phy_addr) == 0)
>  		dev_warn(&pdev->dev, "snps,phy-addr property is deprecated\n");
>  
> -	if ((plat->phy_node && !of_phy_is_fixed_link(np)) || plat->phy_bus_name)
> -		plat->mdio_bus_data = NULL;
> -	else
> +	/* To Configure PHY by using all device-tree supported properties */
> +	if (stmmac_dt_phy(plat, np, &pdev->dev)) {
>  		plat->mdio_bus_data =
>  			devm_kzalloc(&pdev->dev,
>  				     sizeof(struct stmmac_mdio_bus_data),
>  				     GFP_KERNEL);
> +	}
>  
>  	of_property_read_u32(np, "tx-fifo-depth", &plat->tx_fifo_size);
>  
> diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
> index eead8ab..8b1ff2b 100644
> --- a/include/linux/stmmac.h
> +++ b/include/linux/stmmac.h
> @@ -94,12 +94,12 @@ struct stmmac_dma_cfg {
>  };
>  
>  struct plat_stmmacenet_data {
> -	char *phy_bus_name;
>  	int bus_id;
>  	int phy_addr;
>  	int interface;
>  	struct stmmac_mdio_bus_data *mdio_bus_data;
>  	struct device_node *phy_node;
> +	struct device_node *mdio_node;
>  	struct stmmac_dma_cfg *dma_cfg;
>  	int clk_csr;
>  	int has_gmac;

v3 fixes the "libphy: PHY stmmac-0:ffffffff not found" issue on the
Banana Pi board, too.

Regards,
Frank