diff mbox

[net-next,5/5] net: dsa: mv88e6xxx: Implement the 6390 external MDIO bus

Message ID 1485266031-4980-6-git-send-email-andrew@lunn.ch
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Andrew Lunn Jan. 24, 2017, 1:53 p.m. UTC
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(+)

Comments

Vivien Didelot Jan. 24, 2017, 5:48 p.m. UTC | #1
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
Andrew Lunn Jan. 24, 2017, 7:54 p.m. UTC | #2
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
Andrew Lunn Jan. 25, 2017, 1:32 a.m. UTC | #3
> 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
Gregory CLEMENT Jan. 25, 2017, 8:13 a.m. UTC | #4
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 mbox

Patch

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 | \