On Thursday, March 21, 2019 7:26:08 PM CET Florian Fainelli wrote:
> +Christian,
>
> On 3/21/19 11:23 AM, Marek Behún wrote:
> > The MDIO addresses of the internal PHYs on this switch for ports 1-5
> > have addresses 0-4, not 1-5.
> >
>
> Can you provide a Fixes: tag for this? Your change will conflicts with
> Christian's patch series here:
>
> http://patchwork.ozlabs.org/project/netdev/list/?series=98063
http://patchwork.ozlabs.org/patch/1058655/ (link to the patch)
So, Yes and no ;)
We both have the same idea:
+static int
+qca8k_port_to_phy(int port)
+{
+ if (port < 1 || port > QCA8K_MDIO_MASTER_MAX_PORTS)
+ return -EINVAL;
+
+ return port - 1;
+}
I plan to sent v4 tomorrow, since I need to test it on the device first.
> > Signed-off-by: Marek Behún <marek.be...@nic.cz>
> > Cc: Andrew Lunn <and...@lunn.ch>
> > Cc: Florian Fainelli <f.faine...@gmail.com>
> > Cc: Michal Vokáč <voka...@gmail.com>
> > Cc: John Crispin <j...@phrozen.org>
> > Cc: Wei Yongjun <weiyongj...@huawei.com>
> > ---
> > drivers/net/dsa/qca8k.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
> > index cdcde7f8e0b2..eb199193cc3b 100644
> > --- a/drivers/net/dsa/qca8k.c
> > +++ b/drivers/net/dsa/qca8k.c
> > @@ -625,7 +625,7 @@ qca8k_phy_read(struct dsa_switch *ds, int phy, int
> > regnum)
> > {
> > struct qca8k_priv *priv = (struct qca8k_priv *)ds->priv;
> >
> > - return mdiobus_read(priv->bus, phy, regnum);
> > + return mdiobus_read(priv->bus, phy - 1, regnum);
> > }
> >
> > static int
> > @@ -633,7 +633,7 @@ qca8k_phy_write(struct dsa_switch *ds, int phy, int
> > regnum, u16 val)
> > {
> > struct qca8k_priv *priv = (struct qca8k_priv *)ds->priv;
> >
> > - return mdiobus_write(priv->bus, phy, regnum, val);
> > + return mdiobus_write(priv->bus, phy - 1, regnum, val);
> > }
Word of Warning: The priv->bus is pointing to the external mdio-bus that
the QCA8337 is connected to, so in my case I noticed that the *same* PHYs
are registered twice. The first time on the external mdio, which is fine
since we need that for the phy-handle of the "ports" node and a s second
time by the slave-mdio that net/dsa/slave.c provides. In my case I found
Florian's explanation in
https://patchwork.ozlabs.org/patch/1036309/#2084184
very useful:
|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 configuration.
