Message ID | 1485266031-4980-6-git-send-email-andrew@lunn.ch |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
Hi Andrew, Sorry but the previous patches with a list of MDIO busses and refactored PHY ops still seems too much complex to just toggle a bit. We know which switch port has an external PHY attached to it, right? So why not just something like this: struct mv88e6xxx_chip { ... u16 external_phys; ... } int mv88e6xxx_g2_smi_phy_read(struct mv88e6xxx_chip *chip, int phy, int reg, u16 *val) { u16 cmd = GLOBAL2_SMI_PHY_CMD_OP_22_READ_DATA | (addr << 5) | reg; int err; if (chip->external_phys & BIT(phy)) cmd |= GLOBAL2_SMI_PHY_CMD_EXTERNAL; ... } Thanks! Vivien
On Tue, Jan 24, 2017 at 12:48:02PM -0500, Vivien Didelot wrote: > Hi Andrew, > > Sorry but the previous patches with a list of MDIO busses and refactored > PHY ops still seems too much complex to just toggle a bit. > > We know which switch port has an external PHY attached to it, right? No, not really. We have the full flexibility of phandles in the device tree. The PHYs on the external bus might not even be connected to the switch, they could be for the host interfaces, for example. And you don't even need to use the internal PHYs. If you are using the SERDES interfaces, you could have an external SGMII PHY connect to a port. This would make sense with SFP cages, with a copper PHY inserted into the cage. All ports of the 6390 allow this. The hardware has two really separate MDIO busses. Both busses could have a PHY on the same address. We should really model this as two Linux MDIO devices. It is actually a shame older chips don't have this. Early versions of the ClearFog have a hardware design error. The external PHY on port6 is using address 0. So it clashes with the internal PHY on port0. Two MDIO busses would solve this. And i actually expect quite a few designs using the 6390 will put an external phy at address 0 and 1, not 9 and 10. Few PHYs have that many strapping pins for address selection. Andrew
> diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h > index 7d24add45e74..572d585dc1e2 100644 > --- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h > +++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h > @@ -387,6 +387,7 @@ > #define GLOBAL2_PTP_AVB_DATA 0x17 > #define GLOBAL2_SMI_PHY_CMD 0x18 > #define GLOBAL2_SMI_PHY_CMD_BUSY BIT(15) > +#define GLOBAL2_SMI_PHY_CMD_EXTERNAL BIT(13) > #define GLOBAL2_SMI_PHY_CMD_MODE_22 BIT(12) > #define GLOBAL2_SMI_PHY_CMD_OP_22_WRITE_DATA ((0x1 << 10) | \ Hi Gregory Please could you check if the 88E6341 has an external MDIO. Global 2, register 0x18, bit 13. Thanks Andrew
Hi Andrew, On mer., janv. 25 2017, Andrew Lunn <andrew@lunn.ch> wrote: >> diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h >> index 7d24add45e74..572d585dc1e2 100644 >> --- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h >> +++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h >> @@ -387,6 +387,7 @@ >> #define GLOBAL2_PTP_AVB_DATA 0x17 >> #define GLOBAL2_SMI_PHY_CMD 0x18 >> #define GLOBAL2_SMI_PHY_CMD_BUSY BIT(15) >> +#define GLOBAL2_SMI_PHY_CMD_EXTERNAL BIT(13) >> #define GLOBAL2_SMI_PHY_CMD_MODE_22 BIT(12) >> #define GLOBAL2_SMI_PHY_CMD_OP_22_WRITE_DATA ((0x1 << 10) | \ > > Hi Gregory > > Please could you check if the 88E6341 has an external MDIO. Global 2, > register 0x18, bit 13. I confirm that 88E6341 has Global 2, register 0x18, bit 13 referred as "External access" Gregory > > Thanks > Andrew
diff --git a/drivers/net/dsa/mv88e6xxx/global2.c b/drivers/net/dsa/mv88e6xxx/global2.c index 1f9a12a1fad9..353e26bea3c3 100644 --- a/drivers/net/dsa/mv88e6xxx/global2.c +++ b/drivers/net/dsa/mv88e6xxx/global2.c @@ -506,8 +506,12 @@ int mv88e6xxx_g2_smi_phy_read(struct mv88e6xxx_chip *chip, int addr, int reg, u16 *val) { u16 cmd = GLOBAL2_SMI_PHY_CMD_OP_22_READ_DATA | (addr << 5) | reg; + struct mv88e6xxx_mdio_bus *mdio_bus = bus->priv; int err; + if (mdio_bus->external) + cmd |= GLOBAL2_SMI_PHY_CMD_EXTERNAL; + err = mv88e6xxx_g2_smi_phy_wait(chip); if (err) return err; @@ -524,8 +528,12 @@ int mv88e6xxx_g2_smi_phy_write(struct mv88e6xxx_chip *chip, int addr, int reg, u16 val) { u16 cmd = GLOBAL2_SMI_PHY_CMD_OP_22_WRITE_DATA | (addr << 5) | reg; + struct mv88e6xxx_mdio_bus *mdio_bus = bus->priv; int err; + if (mdio_bus->external) + cmd |= GLOBAL2_SMI_PHY_CMD_EXTERNAL; + err = mv88e6xxx_g2_smi_phy_wait(chip); if (err) return err; diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h index 7d24add45e74..572d585dc1e2 100644 --- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h +++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h @@ -387,6 +387,7 @@ #define GLOBAL2_PTP_AVB_DATA 0x17 #define GLOBAL2_SMI_PHY_CMD 0x18 #define GLOBAL2_SMI_PHY_CMD_BUSY BIT(15) +#define GLOBAL2_SMI_PHY_CMD_EXTERNAL BIT(13) #define GLOBAL2_SMI_PHY_CMD_MODE_22 BIT(12) #define GLOBAL2_SMI_PHY_CMD_OP_22_WRITE_DATA ((0x1 << 10) | \ GLOBAL2_SMI_PHY_CMD_MODE_22 | \
With all the infrastructure in place, implement access to the external MDIO bus on the 6390 family. Signed-off-by: Andrew Lunn <andrew@lunn.ch> --- drivers/net/dsa/mv88e6xxx/global2.c | 8 ++++++++ drivers/net/dsa/mv88e6xxx/mv88e6xxx.h | 1 + 2 files changed, 9 insertions(+)