netdev - Re: [PATCH v1] net: dsa: qca8k: implement DT-based ports <-> phy translation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 4 Feb 2019 14:11:41 -0800
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Christian Lamparter <chunkeey@...il.com>, netdev@...r.kernel.org
Cc:     Vivien Didelot <vivien.didelot@...il.com>,
        Andrew Lunn <andrew@...n.ch>
Subject: Re: [PATCH v1] net: dsa: qca8k: implement DT-based ports <-> phy
 translation

On 2/4/19 1:35 PM, Christian Lamparter wrote:
> The QCA8337 enumerates 5 PHYs on the MDC/MDIO access: PHY0-PHY4.
> Based on the System Block Diagram in Section 1.2 of the
> QCA8337's datasheet. These PHYs are internally connected
> to MACs of PORT 1 - PORT 5. However, neither qca8k's slave
> mdio access functions qca8k_phy_read()/qca8k_phy_write()
> nor the dsa framework is set up for that.
> 
> This version of the patch uses the existing phy-handle
> properties of each specified DSA Port in the DT to map
> each PORT/MAC to its exposed PHY on the MDIO bus. This
> is supported by the current binding document qca8k.txt
> as well.

I don't think you should have to do any of this translation, because you
can do a couple of things with DSA/Device Tree:

- you can not provide a phy-handle property at all, in which case, the
core DSA layer assumes that the PHY is part of the switch's internal
MDIO bus which is implictly created by dsa_slave_mii_bus_create()

- you can specify a phy-handle property and then the PHY device tree
node can be placed pretty much anywhere in Device Tree, including on a
separate MDIO bus Device Tre node which is "external" to the switch

In either case, the PHY device's MDIO bus parent and its address are
taken care of by drivers/of/of_mdio.c. You can look at mx88e6xxx for how
it deals with its internal vs. external MDIO bus controller and that
driver is used on a wide variety of cconfiguration.

> 
> Signed-off-by: Christian Lamparter <chunkeey@...il.com>
> ---
>  drivers/net/dsa/qca8k.c | 35 +++++++++++++++++++++++++++++++++--
>  1 file changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
> index a4b6cda38016..6558b7ed855d 100644
> --- a/drivers/net/dsa/qca8k.c
> +++ b/drivers/net/dsa/qca8k.c
> @@ -11,6 +11,7 @@
>  #include <linux/netdevice.h>
>  #include <net/dsa.h>
>  #include <linux/of_net.h>
> +#include <linux/of_mdio.h>
>  #include <linux/of_platform.h>
>  #include <linux/if_bridge.h>
>  #include <linux/mdio.h>
> @@ -612,20 +613,50 @@ qca8k_adjust_link(struct dsa_switch *ds, int port, struct phy_device *phy)
>  	qca8k_port_set_status(priv, port, 1);
>  }
>  
> +static int
> +qca8k_to_real_phy(struct dsa_switch *ds, int phy)
> +{
> +	struct device_node *phy_dn, *port_dn;
> +	int id;
> +
> +	if (phy >= ds->num_ports)
> +		return -EINVAL;
> +
> +	port_dn = ds->ports[phy].dn;
> +	if (!port_dn)
> +		return -EINVAL;
> +
> +	phy_dn = of_parse_phandle(port_dn, "phy-handle", 0);
> +	if (!phy_dn)
> +		return phy;
> +
> +	id = of_mdio_parse_addr(ds->dev, phy_dn);
> +	of_node_put(phy_dn);
> +	return id;
> +}
> +
>  static int
>  qca8k_phy_read(struct dsa_switch *ds, int phy, int regnum)
>  {
>  	struct qca8k_priv *priv = (struct qca8k_priv *)ds->priv;
> +	int realphy = qca8k_to_real_phy(ds, phy);
> +
> +	if (realphy < 0)
> +		return realphy;
>  
> -	return mdiobus_read(priv->bus, phy, regnum);
> +	return mdiobus_read(priv->bus, realphy, regnum);
>  }
>  
>  static int
>  qca8k_phy_write(struct dsa_switch *ds, int phy, int regnum, u16 val)
>  {
>  	struct qca8k_priv *priv = (struct qca8k_priv *)ds->priv;
> +	int realphy = qca8k_to_real_phy(ds, phy);
> +
> +	if (realphy < 0)
> +		return realphy;
>  
> -	return mdiobus_write(priv->bus, phy, regnum, val);
> +	return mdiobus_write(priv->bus, realphy, regnum, val);
>  }
>  
>  static void
> 


-- 
Florian