[PATCH net-next] net: dsa: mv88e6xxx: Work around mv886e6161 SERDES missing MII_PHYSID2

[PATCH net-next] net: dsa: mv88e6xxx: Work around mv886e6161 SERDES missing MII_PHYSID2

[Andrew Lunn]

We already have a workaround for a couple of switches whose internal
PHYs only have the Marvel OUI, but no model number. We detect such
PHYs and give them the 6390 ID as the model number. However the
mv88e6161 has two SERDES interfaces in the same address range as its
internal PHYs. These suffer from the same problem, the Marvell OUI,
but no model number. As a result, these SERDES interfaces were getting
the same PHY ID as the mv88e6390, even though they are not PHYs, and
the Marvell PHY driver was trying to drive them.

Add a special case to stop this happen.

Reported-by: Chris Healy <Chris.Healy@zii.aero>
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index fc0f508879d4..4906a4f39d62 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2524,11 +2524,22 @@ static int mv88e6xxx_mdio_read(struct mii_bus *bus, int phy, int reg)
 	mutex_unlock(&chip->reg_lock);
 
 	if (reg == MII_PHYSID2) {
-		/* Some internal PHYS don't have a model number.  Use
-		 * the mv88e6390 family model number instead.
-		 */
-		if (!(val & 0x3f0))
-			val |= MV88E6XXX_PORT_SWITCH_ID_PROD_6390 >> 4;
+		/* Some internal PHYS don't have a model number. */
+		if (chip->info->family != MV88E6XXX_FAMILY_6165)
+			/* Then there is the 6165 family. It gets is
+			 * PHYs correct. But it can also have two
+			 * SERDES interfaces in the PHY address
+			 * space. And these don't have a model
+			 * number. But they are not PHYs, so we don't
+			 * want to give them something a PHY driver
+			 * will recognise.
+			 *
+			 * Use the mv88e6390 family model number
+			 * instead, for anything which really could be
+			 * a PHY,
+			 */
+			if (!(val & 0x3f0))
+				val |= MV88E6XXX_PORT_SWITCH_ID_PROD_6390 >> 4;
 	}
 
 	return err ? err : val;
-- 
2.19.1

Re: [PATCH net-next] net: dsa: mv88e6xxx: Work around mv886e6161 SERDES missing MII_PHYSID2

[Sergei Shtylyov]

Hello!

On 11.11.2018 2:50, Andrew Lunn wrote:

> We already have a workaround for a couple of switches whose internal
> PHYs only have the Marvel OUI, but no model number. We detect such
> PHYs and give them the 6390 ID as the model number. However the
> mv88e6161 has two SERDES interfaces in the same address range as its
> internal PHYs. These suffer from the same problem, the Marvell OUI,
> but no model number. As a result, these SERDES interfaces were getting
> the same PHY ID as the mv88e6390, even though they are not PHYs, and
> the Marvell PHY driver was trying to drive them.
> 
> Add a special case to stop this happen.

    Dtop this from happening, maybe?

> 
> Reported-by: Chris Healy <Chris.Healy@zii.aero>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>   drivers/net/dsa/mv88e6xxx/chip.c | 21 ++++++++++++++++-----
>   1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index fc0f508879d4..4906a4f39d62 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -2524,11 +2524,22 @@ static int mv88e6xxx_mdio_read(struct mii_bus *bus, int phy, int reg)
>   	mutex_unlock(&chip->reg_lock);
>   
>   	if (reg == MII_PHYSID2) {
> -		/* Some internal PHYS don't have a model number.  Use
> -		 * the mv88e6390 family model number instead.
> -		 */
> -		if (!(val & 0x3f0))
> -			val |= MV88E6XXX_PORT_SWITCH_ID_PROD_6390 >> 4;
> +		/* Some internal PHYS don't have a model number. */

    PHYs (else you're inconsistent with the next comment).

> +		if (chip->info->family != MV88E6XXX_FAMILY_6165)
> +			/* Then there is the 6165 family. It gets is
> +			 * PHYs correct. But it can also have two
> +			 * SERDES interfaces in the PHY address
> +			 * space. And these don't have a model
> +			 * number. But they are not PHYs, so we don't
> +			 * want to give them something a PHY driver
> +			 * will recognise.
> +			 *
> +			 * Use the mv88e6390 family model number
> +			 * instead, for anything which really could be
> +			 * a PHY,
> +			 */
> +			if (!(val & 0x3f0))
> +				val |= MV88E6XXX_PORT_SWITCH_ID_PROD_6390 >> 4;
>   	}
>   
>   	return err ? err : val;

MBR, Sergei

Re: [PATCH net-next] net: dsa: mv88e6xxx: Work around mv886e6161 SERDES missing MII_PHYSID2

[David Miller]

From: Andrew Lunn <andrew@lunn.ch>
Date: Sun, 11 Nov 2018 00:50:39 +0100

> We already have a workaround for a couple of switches whose internal
> PHYs only have the Marvel OUI, but no model number. We detect such
> PHYs and give them the 6390 ID as the model number. However the
> mv88e6161 has two SERDES interfaces in the same address range as its
> internal PHYs. These suffer from the same problem, the Marvell OUI,
> but no model number. As a result, these SERDES interfaces were getting
> the same PHY ID as the mv88e6390, even though they are not PHYs, and
> the Marvell PHY driver was trying to drive them.
> 
> Add a special case to stop this happen.
> 
> Reported-by: Chris Healy <Chris.Healy@zii.aero>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>

Please address Sergei's feedback, thank you.