LinuxLists.cc - [PATCH net-next v4 0/9] provide cable test support for the ksz886x switch

[permalink] [raw]

Subject: [PATCH net-next v4 4/9] net: phy: micrel: apply resume errata workaround for ksz8873 and ksz8863

The ksz8873 and ksz8863 switches are affected by following errata:

| "Receiver error in 100BASE-TX mode following Soft Power Down"
|
| Some KSZ8873 devices may exhibit receiver errors after transitioning
| from Soft Power Down mode to Normal mode, as controlled by register 195
| (0xC3) bits [1:0]. When exiting Soft Power Down mode, the receiver
| blocks may not start up properly, causing the PHY to miss data and
| exhibit erratic behavior. The problem may appear on either port 1 or
| port 2, or both ports. The problem occurs only for 100BASE-TX, not
| 10BASE-T.
|
| END USER IMPLICATIONS
| When the failure occurs, the following symptoms are seen on the affected
| port(s):
| - The port is able to link
| - LED0 blinks, even when there is no traffic
| - The MIB counters indicate receive errors (Rx Fragments, Rx Symbol
| Errors, Rx CRC Errors, Rx Alignment Errors)
| - Only a small fraction of packets is correctly received and forwarded
| through the switch. Most packets are dropped due to receive errors.
|
| The failing condition cannot be corrected by the following:
| - Removing and reconnecting the cable
| - Hardware reset
| - Software Reset and PCS Reset bits in register 67 (0x43)
|
| Work around:
| The problem can be corrected by setting and then clearing the Port Power
| Down bits (registers 29 (0x1D) and 45 (0x2D), bit 3). This must be done
| separately for each affected port after returning from Soft Power Down
| Mode to Normal Mode. The following procedure will ensure no further
| issues due to this erratum. To enter Soft Power Down Mode, set register
| 195 (0xC3), bits [1:0] = 10.
|
| To exit Soft Power Down Mode, follow these steps:
| 1. Set register 195 (0xC3), bits [1:0] = 00 // Exit soft power down mode
| 2. Wait 1ms minimum
| 3. Set register 29 (0x1D), bit [3] = 1 // Enter PHY port 1 power down mode
| 4. Set register 29 (0x1D), bit [3] = 0 // Exit PHY port 1 power down mode
| 5. Set register 45 (0x2D), bit [3] = 1 // Enter PHY port 2 power down mode
| 6. Set register 45 (0x2D), bit [3] = 0 // Exit PHY port 2 power down mode

This patch implements steps 2...6 of the suggested workaround. During
(initial) switch power up, step 1 is executed by the dsa/ksz8795
driver's probe function.

Note: In this workaround we toggle the MII_BMCR register's BMCR_PDOWN
bit, this is translated to the actual register and bit (as mentioned in
the arratum) by the ksz8_r_phy()/ksz8_w_phy() functions.

Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/phy/micrel.c | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 77640b990977..e462e718d68e 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -1048,6 +1048,26 @@ static int ksz8873mll_config_aneg(struct phy_device *phydev)
return 0;
}

+static int ksz886x_resume(struct phy_device *phydev)
+{
+ int ret;
+
+ /* Apply errata workaround for KSZ8863 and KSZ8873:
+ * Receiver error in 100BASE-TX mode following Soft Power Down
+ *
+ * When exiting Soft Power Down mode, the receiver blocks may not start
+ * up properly, causing the PHY to miss data and exhibit erratic
+ * behavior.
+ */
+ usleep_range(1000, 2000);
+
+ ret = phy_set_bits(phydev, MII_BMCR, BMCR_PDOWN);
+ if (ret)
+ return ret;
+
+ return phy_clear_bits(phydev, MII_BMCR, BMCR_PDOWN);
+}
+
static int kszphy_get_sset_count(struct phy_device *phydev)
{
return ARRAY_SIZE(kszphy_hw_stats);
@@ -1401,7 +1421,7 @@ static struct phy_driver ksphy_driver[] = {
/* PHY_BASIC_FEATURES */
.config_init = kszphy_config_init,
.suspend = genphy_suspend,
- .resume = genphy_resume,
+ .resume = ksz886x_resume,
}, {
.name = "Micrel KSZ87XX Switch",
/* PHY_BASIC_FEATURES */
--
2.29.2

2021-06-11 07:18:30

[permalink] [raw]

Subject: [PATCH net-next v4 8/9] net: dsa: dsa_slave_phy_connect(): extend phy's flags with port specific phy flags

This patch extends the flags of the phy that's being connected with the
port specific flags of the switch port.

This is needed to handle a port specific erratum of the KSZ8873 switch,
which is added in a later patch.

Signed-off-by: Oleksij Rempel <[email protected]>
---
net/dsa/slave.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index d4756b920108..ca344b37d02d 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1760,6 +1760,10 @@ static int dsa_slave_phy_connect(struct net_device *slave_dev, int addr)
return -ENODEV;
}

+ if (ds->ops->get_phy_flags)
+ slave_dev->phydev->dev_flags |=
+ ds->ops->get_phy_flags(ds, dp->index);
+
return phylink_connect_phy(dp->pl, slave_dev->phydev);
}

--
2.29.2

2021-06-11 07:19:06

[permalink] [raw]

Subject: [PATCH net-next v4 7/9] net: dsa: microchip: ksz8795: add LINK_MD register support

From: Oleksij Rempel <[email protected]>

Add mapping for LINK_MD register to enable cable testing functionality.

Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/dsa/microchip/ksz8795.c | 22 ++++++++++++++++++++++
drivers/net/dsa/microchip/ksz8795_reg.h | 5 +++--
2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index 690304c87b02..6e1d238da600 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -6,6 +6,7 @@
* Tristram Ha <[email protected]>
*/

+#include <linux/bitfield.h>
#include <linux/delay.h>
#include <linux/export.h>
#include <linux/gpio.h>
@@ -728,6 +729,7 @@ static void ksz8_r_phy(struct ksz_device *dev, u16 phy, u16 reg, u16 *val)
struct ksz8 *ksz8 = dev->priv;
u8 restart, speed, ctrl, link;
const u8 *regs = ksz8->regs;
+ u8 val1, val2;
int processed = true;
u16 data = 0;
u8 p = phy;
@@ -816,6 +818,22 @@ static void ksz8_r_phy(struct ksz_device *dev, u16 phy, u16 reg, u16 *val)
if (data & ~LPA_SLCT)
data |= LPA_LPACK;
break;
+ case PHY_REG_LINK_MD:
+ ksz_pread8(dev, p, REG_PORT_LINK_MD_CTRL, &val1);
+ ksz_pread8(dev, p, REG_PORT_LINK_MD_RESULT, &val2);
+ if (val1 & PORT_START_CABLE_DIAG)
+ data |= PHY_START_CABLE_DIAG;
+
+ if (val1 & PORT_CABLE_10M_SHORT)
+ data |= PHY_CABLE_10M_SHORT;
+
+ data |= FIELD_PREP(PHY_CABLE_DIAG_RESULT_M,
+ FIELD_GET(PORT_CABLE_DIAG_RESULT_M, val1));
+
+ data |= FIELD_PREP(PHY_CABLE_FAULT_COUNTER_M,
+ (FIELD_GET(PORT_CABLE_FAULT_COUNTER_H, val1) << 8) |
+ FIELD_GET(PORT_CABLE_FAULT_COUNTER_L, val2));
+ break;
case PHY_REG_PHY_CTRL:
ksz_pread8(dev, p, regs[P_LINK_STATUS], &link);
if (link & PORT_MDIX_STATUS)
@@ -932,6 +950,10 @@ static void ksz8_w_phy(struct ksz_device *dev, u16 phy, u16 reg, u16 val)
if (data != ctrl)
ksz_pwrite8(dev, p, regs[P_LOCAL_CTRL], data);
break;
+ case PHY_REG_LINK_MD:
+ if (val & PHY_START_CABLE_DIAG)
+ ksz_port_cfg(dev, p, REG_PORT_LINK_MD_CTRL, PORT_START_CABLE_DIAG, true);
+ break;
default:
break;
}
diff --git a/drivers/net/dsa/microchip/ksz8795_reg.h b/drivers/net/dsa/microchip/ksz8795_reg.h
index f925ddee5238..a32355624f31 100644
--- a/drivers/net/dsa/microchip/ksz8795_reg.h
+++ b/drivers/net/dsa/microchip/ksz8795_reg.h
@@ -249,7 +249,7 @@
#define REG_PORT_4_LINK_MD_CTRL 0x4A

#define PORT_CABLE_10M_SHORT BIT(7)
-#define PORT_CABLE_DIAG_RESULT_M 0x3
+#define PORT_CABLE_DIAG_RESULT_M GENMASK(6, 5)
#define PORT_CABLE_DIAG_RESULT_S 5
#define PORT_CABLE_STAT_NORMAL 0
#define PORT_CABLE_STAT_OPEN 1
@@ -753,13 +753,14 @@
#define PHY_REG_LINK_MD 0x1D

#define PHY_START_CABLE_DIAG BIT(15)
+#define PHY_CABLE_DIAG_RESULT_M GENMASK(14, 13)
#define PHY_CABLE_DIAG_RESULT 0x6000
#define PHY_CABLE_STAT_NORMAL 0x0000
#define PHY_CABLE_STAT_OPEN 0x2000
#define PHY_CABLE_STAT_SHORT 0x4000
#define PHY_CABLE_STAT_FAILED 0x6000
#define PHY_CABLE_10M_SHORT BIT(12)
-#define PHY_CABLE_FAULT_COUNTER 0x01FF
+#define PHY_CABLE_FAULT_COUNTER_M GENMASK(8, 0)

#define PHY_REG_PHY_CTRL 0x1F

--
2.29.2

2021-06-11 07:19:24

[permalink] [raw]

Subject: [PATCH net-next v4 5/9] net: phy/dsa micrel/ksz886x add MDI-X support

Add support for MDI-X status and configuration

Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/dsa/microchip/ksz8795.c | 5 ++
drivers/net/phy/micrel.c | 88 +++++++++++++++++++++++++++++
include/linux/micrel_phy.h | 2 +
3 files changed, 95 insertions(+)

diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index cfa2a5000cd3..690304c87b02 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -816,6 +816,11 @@ static void ksz8_r_phy(struct ksz_device *dev, u16 phy, u16 reg, u16 *val)
if (data & ~LPA_SLCT)
data |= LPA_LPACK;
break;
+ case PHY_REG_PHY_CTRL:
+ ksz_pread8(dev, p, regs[P_LINK_STATUS], &link);
+ if (link & PORT_MDIX_STATUS)
+ data |= KSZ886X_CTRL_MDIX_STAT;
+ break;
default:
processed = false;
break;
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index e462e718d68e..c421c1e7dd71 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -1048,6 +1048,92 @@ static int ksz8873mll_config_aneg(struct phy_device *phydev)
return 0;
}

+static int ksz886x_config_mdix(struct phy_device *phydev, u8 ctrl)
+{
+ u16 val;
+
+ switch (ctrl) {
+ case ETH_TP_MDI:
+ val = KSZ886X_BMCR_DISABLE_AUTO_MDIX;
+ break;
+ case ETH_TP_MDI_X:
+ /* Note: The naming of the bit KSZ886X_BMCR_FORCE_MDI is bit
+ * counter intuitive, the "-X" in "1 = Force MDI" in the data
+ * sheet seems to be missing:
+ * 1 = Force MDI (sic!) (transmit on RX+/RX- pins)
+ * 0 = Normal operation (transmit on TX+/TX- pins)
+ */
+ val = KSZ886X_BMCR_DISABLE_AUTO_MDIX | KSZ886X_BMCR_FORCE_MDI;
+ break;
+ case ETH_TP_MDI_AUTO:
+ val = 0;
+ break;
+ default:
+ return 0;
+ }
+
+ return phy_modify(phydev, MII_BMCR,
+ KSZ886X_BMCR_HP_MDIX | KSZ886X_BMCR_FORCE_MDI |
+ KSZ886X_BMCR_DISABLE_AUTO_MDIX,
+ KSZ886X_BMCR_HP_MDIX | val);
+}
+
+static int ksz886x_config_aneg(struct phy_device *phydev)
+{
+ int ret;
+
+ ret = genphy_config_aneg(phydev);
+ if (ret)
+ return ret;
+
+ /* The MDI-X configuration is automatically changed by the PHY after
+ * switching from autoneg off to on. So, take MDI-X configuration under
+ * own control and set it after autoneg configuration was done.
+ */
+ return ksz886x_config_mdix(phydev, phydev->mdix_ctrl);
+}
+
+static int ksz886x_mdix_update(struct phy_device *phydev)
+{
+ int ret;
+
+ ret = phy_read(phydev, MII_BMCR);
+ if (ret < 0)
+ return ret;
+
+ if (ret & KSZ886X_BMCR_DISABLE_AUTO_MDIX) {
+ if (ret & KSZ886X_BMCR_FORCE_MDI)
+ phydev->mdix_ctrl = ETH_TP_MDI_X;
+ else
+ phydev->mdix_ctrl = ETH_TP_MDI;
+ } else {
+ phydev->mdix_ctrl = ETH_TP_MDI_AUTO;
+ }
+
+ ret = phy_read(phydev, MII_KSZPHY_CTRL);
+ if (ret < 0)
+ return ret;
+
+ /* Same reverse logic as KSZ886X_BMCR_FORCE_MDI */
+ if (ret & KSZ886X_CTRL_MDIX_STAT)
+ phydev->mdix = ETH_TP_MDI_X;
+ else
+ phydev->mdix = ETH_TP_MDI;
+
+ return 0;
+}
+
+static int ksz886x_read_status(struct phy_device *phydev)
+{
+ int ret;
+
+ ret = ksz886x_mdix_update(phydev);
+ if (ret < 0)
+ return ret;
+
+ return genphy_read_status(phydev);
+}
+
static int ksz886x_resume(struct phy_device *phydev)
{
int ret;
@@ -1420,6 +1506,8 @@ static struct phy_driver ksphy_driver[] = {
.name = "Micrel KSZ8851 Ethernet MAC or KSZ886X Switch",
/* PHY_BASIC_FEATURES */
.config_init = kszphy_config_init,
+ .config_aneg = ksz886x_config_aneg,
+ .read_status = ksz886x_read_status,
.suspend = genphy_suspend,
.resume = ksz886x_resume,
}, {
diff --git a/include/linux/micrel_phy.h b/include/linux/micrel_phy.h
index b03e2afcb53f..58370abd9f4f 100644
--- a/include/linux/micrel_phy.h
+++ b/include/linux/micrel_phy.h
@@ -58,4 +58,6 @@
#define KSZ886X_BMCR_DISABLE_TRANSMIT BIT(1)
#define KSZ886X_BMCR_DISABLE_LED BIT(0)

+#define KSZ886X_CTRL_MDIX_STAT BIT(4)
+
#endif /* _MICREL_PHY_H */
--
2.29.2

2021-06-11 07:19:27

[permalink] [raw]

Subject: [PATCH net-next v4 2/9] net: dsa: microchip: ksz8795: add phylink support

From: Michael Grzeschik <[email protected]>

This patch adds the phylink support to the ksz8795 driver to provide
configuration exceptions on quirky KSZ8863 and KSZ8873 ports.

Signed-off-by: Michael Grzeschik <[email protected]>
Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/dsa/microchip/ksz8795.c | 55 +++++++++++++++++++++++++++++
1 file changed, 55 insertions(+)

diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index ba065003623f..cfa2a5000cd3 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -18,6 +18,7 @@
#include <linux/micrel_phy.h>
#include <net/dsa.h>
#include <net/switchdev.h>
+#include <linux/phylink.h>

#include "ksz_common.h"
#include "ksz8795_reg.h"
@@ -1420,11 +1421,65 @@ static int ksz8_setup(struct dsa_switch *ds)
return 0;
}

+static void ksz8_validate(struct dsa_switch *ds, int port,
+ unsigned long *supported,
+ struct phylink_link_state *state)
+{
+ __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
+ struct ksz_device *dev = ds->priv;
+
+ if (port == dev->cpu_port) {
+ if (state->interface != PHY_INTERFACE_MODE_RMII &&
+ state->interface != PHY_INTERFACE_MODE_MII &&
+ state->interface != PHY_INTERFACE_MODE_NA)
+ goto unsupported;
+ } else {
+ if (state->interface != PHY_INTERFACE_MODE_INTERNAL &&
+ state->interface != PHY_INTERFACE_MODE_NA)
+ goto unsupported;
+ }
+
+ /* Allow all the expected bits */
+ phylink_set_port_modes(mask);
+ phylink_set(mask, Autoneg);
+
+ /* Silicon Errata Sheet (DS80000830A):
+ * "Port 1 does not respond to received flow control PAUSE frames"
+ * So, disable Pause support on "Port 1" (port == 0) for all ksz88x3
+ * switches.
+ */
+ if (!ksz_is_ksz88x3(dev) || port)
+ phylink_set(mask, Pause);
+
+ /* Asym pause is not supported on KSZ8863 and KSZ8873 */
+ if (!ksz_is_ksz88x3(dev))
+ phylink_set(mask, Asym_Pause);
+
+ /* 10M and 100M are only supported */
+ phylink_set(mask, 10baseT_Half);
+ phylink_set(mask, 10baseT_Full);
+ phylink_set(mask, 100baseT_Half);
+ phylink_set(mask, 100baseT_Full);
+
+ bitmap_and(supported, supported, mask,
+ __ETHTOOL_LINK_MODE_MASK_NBITS);
+ bitmap_and(state->advertising, state->advertising, mask,
+ __ETHTOOL_LINK_MODE_MASK_NBITS);
+
+ return;
+
+unsupported:
+ bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
+ dev_err(ds->dev, "Unsupported interface: %s, port: %d\n",
+ phy_modes(state->interface), port);
+}
+
static const struct dsa_switch_ops ksz8_switch_ops = {
.get_tag_protocol = ksz8_get_tag_protocol,
.setup = ksz8_setup,
.phy_read = ksz_phy_read16,
.phy_write = ksz_phy_write16,
+ .phylink_validate = ksz8_validate,
.phylink_mac_link_down = ksz_mac_link_down,
.port_enable = ksz_enable_port,
.get_strings = ksz8_get_strings,
--
2.29.2

2021-06-11 07:20:07

[permalink] [raw]

Subject: [PATCH net-next v4 1/9] net: phy: micrel: move phy reg offsets to common header

From: Michael Grzeschik <[email protected]>

Some micrel devices share the same PHY register defines. This patch
moves them to one common header so other drivers can reuse them.
And reuse generic MII_* defines where possible.

Signed-off-by: Michael Grzeschik <[email protected]>
Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/dsa/microchip/ksz8795.c | 119 ++++++++++++------------
drivers/net/dsa/microchip/ksz8795_reg.h | 62 ------------
drivers/net/ethernet/micrel/ksz884x.c | 105 +++------------------
include/linux/micrel_phy.h | 13 +++
4 files changed, 88 insertions(+), 211 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index ad509a57a945..ba065003623f 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -15,6 +15,7 @@
#include <linux/phy.h>
#include <linux/etherdevice.h>
#include <linux/if_bridge.h>
+#include <linux/micrel_phy.h>
#include <net/dsa.h>
#include <net/switchdev.h>

@@ -731,88 +732,88 @@ static void ksz8_r_phy(struct ksz_device *dev, u16 phy, u16 reg, u16 *val)
u8 p = phy;

switch (reg) {
- case PHY_REG_CTRL:
+ case MII_BMCR:
ksz_pread8(dev, p, regs[P_NEG_RESTART_CTRL], &restart);
ksz_pread8(dev, p, regs[P_SPEED_STATUS], &speed);
ksz_pread8(dev, p, regs[P_FORCE_CTRL], &ctrl);
if (restart & PORT_PHY_LOOPBACK)
- data |= PHY_LOOPBACK;
+ data |= BMCR_LOOPBACK;
if (ctrl & PORT_FORCE_100_MBIT)
- data |= PHY_SPEED_100MBIT;
+ data |= BMCR_SPEED100;
if (ksz_is_ksz88x3(dev)) {
if ((ctrl & PORT_AUTO_NEG_ENABLE))
- data |= PHY_AUTO_NEG_ENABLE;
+ data |= BMCR_ANENABLE;
} else {
if (!(ctrl & PORT_AUTO_NEG_DISABLE))
- data |= PHY_AUTO_NEG_ENABLE;
+ data |= BMCR_ANENABLE;
}
if (restart & PORT_POWER_DOWN)
- data |= PHY_POWER_DOWN;
+ data |= BMCR_PDOWN;
if (restart & PORT_AUTO_NEG_RESTART)
- data |= PHY_AUTO_NEG_RESTART;
+ data |= BMCR_ANRESTART;
if (ctrl & PORT_FORCE_FULL_DUPLEX)
- data |= PHY_FULL_DUPLEX;
+ data |= BMCR_FULLDPLX;
if (speed & PORT_HP_MDIX)
- data |= PHY_HP_MDIX;
+ data |= KSZ886X_BMCR_HP_MDIX;
if (restart & PORT_FORCE_MDIX)
- data |= PHY_FORCE_MDIX;
+ data |= KSZ886X_BMCR_FORCE_MDI;
if (restart & PORT_AUTO_MDIX_DISABLE)
- data |= PHY_AUTO_MDIX_DISABLE;
+ data |= KSZ886X_BMCR_DISABLE_AUTO_MDIX;
if (restart & PORT_TX_DISABLE)
- data |= PHY_TRANSMIT_DISABLE;
+ data |= KSZ886X_BMCR_DISABLE_TRANSMIT;
if (restart & PORT_LED_OFF)
- data |= PHY_LED_DISABLE;
+ data |= KSZ886X_BMCR_DISABLE_LED;
break;
- case PHY_REG_STATUS:
+ case MII_BMSR:
ksz_pread8(dev, p, regs[P_LINK_STATUS], &link);
- data = PHY_100BTX_FD_CAPABLE |
- PHY_100BTX_CAPABLE |
- PHY_10BT_FD_CAPABLE |
- PHY_10BT_CAPABLE |
- PHY_AUTO_NEG_CAPABLE;
+ data = BMSR_100FULL |
+ BMSR_100HALF |
+ BMSR_10FULL |
+ BMSR_10HALF |
+ BMSR_ANEGCAPABLE;
if (link & PORT_AUTO_NEG_COMPLETE)
- data |= PHY_AUTO_NEG_ACKNOWLEDGE;
+ data |= BMSR_ANEGCOMPLETE;
if (link & PORT_STAT_LINK_GOOD)
- data |= PHY_LINK_STATUS;
+ data |= BMSR_LSTATUS;
break;
- case PHY_REG_ID_1:
+ case MII_PHYSID1:
data = KSZ8795_ID_HI;
break;
- case PHY_REG_ID_2:
+ case MII_PHYSID2:
if (ksz_is_ksz88x3(dev))
data = KSZ8863_ID_LO;
else
data = KSZ8795_ID_LO;
break;
- case PHY_REG_AUTO_NEGOTIATION:
+ case MII_ADVERTISE:
ksz_pread8(dev, p, regs[P_LOCAL_CTRL], &ctrl);
- data = PHY_AUTO_NEG_802_3;
+ data = ADVERTISE_CSMA;
if (ctrl & PORT_AUTO_NEG_SYM_PAUSE)
- data |= PHY_AUTO_NEG_SYM_PAUSE;
+ data |= ADVERTISE_PAUSE_CAP;
if (ctrl & PORT_AUTO_NEG_100BTX_FD)
- data |= PHY_AUTO_NEG_100BTX_FD;
+ data |= ADVERTISE_100FULL;
if (ctrl & PORT_AUTO_NEG_100BTX)
- data |= PHY_AUTO_NEG_100BTX;
+ data |= ADVERTISE_100HALF;
if (ctrl & PORT_AUTO_NEG_10BT_FD)
- data |= PHY_AUTO_NEG_10BT_FD;
+ data |= ADVERTISE_10FULL;
if (ctrl & PORT_AUTO_NEG_10BT)
- data |= PHY_AUTO_NEG_10BT;
+ data |= ADVERTISE_10HALF;
break;
- case PHY_REG_REMOTE_CAPABILITY:
+ case MII_LPA:
ksz_pread8(dev, p, regs[P_REMOTE_STATUS], &link);
- data = PHY_AUTO_NEG_802_3;
+ data = LPA_SLCT;
if (link & PORT_REMOTE_SYM_PAUSE)
- data |= PHY_AUTO_NEG_SYM_PAUSE;
+ data |= LPA_PAUSE_CAP;
if (link & PORT_REMOTE_100BTX_FD)
- data |= PHY_AUTO_NEG_100BTX_FD;
+ data |= LPA_100FULL;
if (link & PORT_REMOTE_100BTX)
- data |= PHY_AUTO_NEG_100BTX;
+ data |= LPA_100HALF;
if (link & PORT_REMOTE_10BT_FD)
- data |= PHY_AUTO_NEG_10BT_FD;
+ data |= LPA_10FULL;
if (link & PORT_REMOTE_10BT)
- data |= PHY_AUTO_NEG_10BT;
- if (data & ~PHY_AUTO_NEG_802_3)
- data |= PHY_REMOTE_ACKNOWLEDGE_NOT;
+ data |= LPA_10HALF;
+ if (data & ~LPA_SLCT)
+ data |= LPA_LPACK;
break;
default:
processed = false;
@@ -830,14 +831,14 @@ static void ksz8_w_phy(struct ksz_device *dev, u16 phy, u16 reg, u16 val)
u8 p = phy;

switch (reg) {
- case PHY_REG_CTRL:
+ case MII_BMCR:

/* Do not support PHY reset function. */
- if (val & PHY_RESET)
+ if (val & BMCR_RESET)
break;
ksz_pread8(dev, p, regs[P_SPEED_STATUS], &speed);
data = speed;
- if (val & PHY_HP_MDIX)
+ if (val & KSZ886X_BMCR_HP_MDIX)
data |= PORT_HP_MDIX;
else
data &= ~PORT_HP_MDIX;
@@ -846,12 +847,12 @@ static void ksz8_w_phy(struct ksz_device *dev, u16 phy, u16 reg, u16 val)
ksz_pread8(dev, p, regs[P_FORCE_CTRL], &ctrl);
data = ctrl;
if (ksz_is_ksz88x3(dev)) {
- if ((val & PHY_AUTO_NEG_ENABLE))
+ if ((val & BMCR_ANENABLE))
data |= PORT_AUTO_NEG_ENABLE;
else
data &= ~PORT_AUTO_NEG_ENABLE;
} else {
- if (!(val & PHY_AUTO_NEG_ENABLE))
+ if (!(val & BMCR_ANENABLE))
data |= PORT_AUTO_NEG_DISABLE;
else
data &= ~PORT_AUTO_NEG_DISABLE;
@@ -861,11 +862,11 @@ static void ksz8_w_phy(struct ksz_device *dev, u16 phy, u16 reg, u16 val)
data |= PORT_AUTO_NEG_DISABLE;
}

- if (val & PHY_SPEED_100MBIT)
+ if (val & BMCR_SPEED100)
data |= PORT_FORCE_100_MBIT;
else
data &= ~PORT_FORCE_100_MBIT;
- if (val & PHY_FULL_DUPLEX)
+ if (val & BMCR_FULLDPLX)
data |= PORT_FORCE_FULL_DUPLEX;
else
data &= ~PORT_FORCE_FULL_DUPLEX;
@@ -873,38 +874,38 @@ static void ksz8_w_phy(struct ksz_device *dev, u16 phy, u16 reg, u16 val)
ksz_pwrite8(dev, p, regs[P_FORCE_CTRL], data);
ksz_pread8(dev, p, regs[P_NEG_RESTART_CTRL], &restart);
data = restart;
- if (val & PHY_LED_DISABLE)
+ if (val & KSZ886X_BMCR_DISABLE_LED)
data |= PORT_LED_OFF;
else
data &= ~PORT_LED_OFF;
- if (val & PHY_TRANSMIT_DISABLE)
+ if (val & KSZ886X_BMCR_DISABLE_TRANSMIT)
data |= PORT_TX_DISABLE;
else
data &= ~PORT_TX_DISABLE;
- if (val & PHY_AUTO_NEG_RESTART)
+ if (val & BMCR_ANRESTART)
data |= PORT_AUTO_NEG_RESTART;
else
data &= ~(PORT_AUTO_NEG_RESTART);
- if (val & PHY_POWER_DOWN)
+ if (val & BMCR_PDOWN)
data |= PORT_POWER_DOWN;
else
data &= ~PORT_POWER_DOWN;
- if (val & PHY_AUTO_MDIX_DISABLE)
+ if (val & KSZ886X_BMCR_DISABLE_AUTO_MDIX)
data |= PORT_AUTO_MDIX_DISABLE;
else
data &= ~PORT_AUTO_MDIX_DISABLE;
- if (val & PHY_FORCE_MDIX)
+ if (val & KSZ886X_BMCR_FORCE_MDI)
data |= PORT_FORCE_MDIX;
else
data &= ~PORT_FORCE_MDIX;
- if (val & PHY_LOOPBACK)
+ if (val & BMCR_LOOPBACK)
data |= PORT_PHY_LOOPBACK;
else
data &= ~PORT_PHY_LOOPBACK;
if (data != restart)
ksz_pwrite8(dev, p, regs[P_NEG_RESTART_CTRL], data);
break;
- case PHY_REG_AUTO_NEGOTIATION:
+ case MII_ADVERTISE:
ksz_pread8(dev, p, regs[P_LOCAL_CTRL], &ctrl);
data = ctrl;
data &= ~(PORT_AUTO_NEG_SYM_PAUSE |
@@ -912,15 +913,15 @@ static void ksz8_w_phy(struct ksz_device *dev, u16 phy, u16 reg, u16 val)
PORT_AUTO_NEG_100BTX |
PORT_AUTO_NEG_10BT_FD |
PORT_AUTO_NEG_10BT);
- if (val & PHY_AUTO_NEG_SYM_PAUSE)
+ if (val & ADVERTISE_PAUSE_CAP)
data |= PORT_AUTO_NEG_SYM_PAUSE;
- if (val & PHY_AUTO_NEG_100BTX_FD)
+ if (val & ADVERTISE_100FULL)
data |= PORT_AUTO_NEG_100BTX_FD;
- if (val & PHY_AUTO_NEG_100BTX)
+ if (val & ADVERTISE_100HALF)
data |= PORT_AUTO_NEG_100BTX;
- if (val & PHY_AUTO_NEG_10BT_FD)
+ if (val & ADVERTISE_10FULL)
data |= PORT_AUTO_NEG_10BT_FD;
- if (val & PHY_AUTO_NEG_10BT)
+ if (val & ADVERTISE_10HALF)
data |= PORT_AUTO_NEG_10BT;
if (data != ctrl)
ksz_pwrite8(dev, p, regs[P_LOCAL_CTRL], data);
diff --git a/drivers/net/dsa/microchip/ksz8795_reg.h b/drivers/net/dsa/microchip/ksz8795_reg.h
index c2e52c40a54c..f925ddee5238 100644
--- a/drivers/net/dsa/microchip/ksz8795_reg.h
+++ b/drivers/net/dsa/microchip/ksz8795_reg.h
@@ -744,68 +744,6 @@

#define PORT_ACL_FORCE_DLR_MISS BIT(0)

-#ifndef PHY_REG_CTRL
-#define PHY_REG_CTRL 0
-
-#define PHY_RESET BIT(15)
-#define PHY_LOOPBACK BIT(14)
-#define PHY_SPEED_100MBIT BIT(13)
-#define PHY_AUTO_NEG_ENABLE BIT(12)
-#define PHY_POWER_DOWN BIT(11)
-#define PHY_MII_DISABLE BIT(10)
-#define PHY_AUTO_NEG_RESTART BIT(9)
-#define PHY_FULL_DUPLEX BIT(8)
-#define PHY_COLLISION_TEST_NOT BIT(7)
-#define PHY_HP_MDIX BIT(5)
-#define PHY_FORCE_MDIX BIT(4)
-#define PHY_AUTO_MDIX_DISABLE BIT(3)
-#define PHY_REMOTE_FAULT_DISABLE BIT(2)
-#define PHY_TRANSMIT_DISABLE BIT(1)
-#define PHY_LED_DISABLE BIT(0)
-
-#define PHY_REG_STATUS 1
-
-#define PHY_100BT4_CAPABLE BIT(15)
-#define PHY_100BTX_FD_CAPABLE BIT(14)
-#define PHY_100BTX_CAPABLE BIT(13)
-#define PHY_10BT_FD_CAPABLE BIT(12)
-#define PHY_10BT_CAPABLE BIT(11)
-#define PHY_MII_SUPPRESS_CAPABLE_NOT BIT(6)
-#define PHY_AUTO_NEG_ACKNOWLEDGE BIT(5)
-#define PHY_REMOTE_FAULT BIT(4)
-#define PHY_AUTO_NEG_CAPABLE BIT(3)
-#define PHY_LINK_STATUS BIT(2)
-#define PHY_JABBER_DETECT_NOT BIT(1)
-#define PHY_EXTENDED_CAPABILITY BIT(0)
-
-#define PHY_REG_ID_1 2
-#define PHY_REG_ID_2 3
-
-#define PHY_REG_AUTO_NEGOTIATION 4
-
-#define PHY_AUTO_NEG_NEXT_PAGE_NOT BIT(15)
-#define PHY_AUTO_NEG_REMOTE_FAULT_NOT BIT(13)
-#define PHY_AUTO_NEG_SYM_PAUSE BIT(10)
-#define PHY_AUTO_NEG_100BT4 BIT(9)
-#define PHY_AUTO_NEG_100BTX_FD BIT(8)
-#define PHY_AUTO_NEG_100BTX BIT(7)
-#define PHY_AUTO_NEG_10BT_FD BIT(6)
-#define PHY_AUTO_NEG_10BT BIT(5)
-#define PHY_AUTO_NEG_SELECTOR 0x001F
-#define PHY_AUTO_NEG_802_3 0x0001
-
-#define PHY_REG_REMOTE_CAPABILITY 5
-
-#define PHY_REMOTE_NEXT_PAGE_NOT BIT(15)
-#define PHY_REMOTE_ACKNOWLEDGE_NOT BIT(14)
-#define PHY_REMOTE_REMOTE_FAULT_NOT BIT(13)
-#define PHY_REMOTE_SYM_PAUSE BIT(10)
-#define PHY_REMOTE_100BTX_FD BIT(8)
-#define PHY_REMOTE_100BTX BIT(7)
-#define PHY_REMOTE_10BT_FD BIT(6)
-#define PHY_REMOTE_10BT BIT(5)
-#endif
-
#define KSZ8795_ID_HI 0x0022
#define KSZ8795_ID_LO 0x1550
#define KSZ8863_ID_LO 0x1430
diff --git a/drivers/net/ethernet/micrel/ksz884x.c b/drivers/net/ethernet/micrel/ksz884x.c
index 3532bfe936f6..7945eb5e2fe8 100644
--- a/drivers/net/ethernet/micrel/ksz884x.c
+++ b/drivers/net/ethernet/micrel/ksz884x.c
@@ -25,6 +25,7 @@
#include <linux/crc32.h>
#include <linux/sched.h>
#include <linux/slab.h>
+#include <linux/micrel_phy.h>

/* DMA Registers */
@@ -271,84 +272,15 @@

#define KS884X_PHY_CTRL_OFFSET 0x00

-/* Mode Control Register */
-#define PHY_REG_CTRL 0
-
-#define PHY_RESET 0x8000
-#define PHY_LOOPBACK 0x4000
-#define PHY_SPEED_100MBIT 0x2000
-#define PHY_AUTO_NEG_ENABLE 0x1000
-#define PHY_POWER_DOWN 0x0800
-#define PHY_MII_DISABLE 0x0400
-#define PHY_AUTO_NEG_RESTART 0x0200
-#define PHY_FULL_DUPLEX 0x0100
-#define PHY_COLLISION_TEST 0x0080
-#define PHY_HP_MDIX 0x0020
-#define PHY_FORCE_MDIX 0x0010
-#define PHY_AUTO_MDIX_DISABLE 0x0008
-#define PHY_REMOTE_FAULT_DISABLE 0x0004
-#define PHY_TRANSMIT_DISABLE 0x0002
-#define PHY_LED_DISABLE 0x0001
-
#define KS884X_PHY_STATUS_OFFSET 0x02

-/* Mode Status Register */
-#define PHY_REG_STATUS 1
-
-#define PHY_100BT4_CAPABLE 0x8000
-#define PHY_100BTX_FD_CAPABLE 0x4000
-#define PHY_100BTX_CAPABLE 0x2000
-#define PHY_10BT_FD_CAPABLE 0x1000
-#define PHY_10BT_CAPABLE 0x0800
-#define PHY_MII_SUPPRESS_CAPABLE 0x0040
-#define PHY_AUTO_NEG_ACKNOWLEDGE 0x0020
-#define PHY_REMOTE_FAULT 0x0010
-#define PHY_AUTO_NEG_CAPABLE 0x0008
-#define PHY_LINK_STATUS 0x0004
-#define PHY_JABBER_DETECT 0x0002
-#define PHY_EXTENDED_CAPABILITY 0x0001
-
#define KS884X_PHY_ID_1_OFFSET 0x04
#define KS884X_PHY_ID_2_OFFSET 0x06

-/* PHY Identifier Registers */
-#define PHY_REG_ID_1 2
-#define PHY_REG_ID_2 3
-
#define KS884X_PHY_AUTO_NEG_OFFSET 0x08

-/* Auto-Negotiation Advertisement Register */
-#define PHY_REG_AUTO_NEGOTIATION 4
-
-#define PHY_AUTO_NEG_NEXT_PAGE 0x8000
-#define PHY_AUTO_NEG_REMOTE_FAULT 0x2000
-/* Not supported. */
-#define PHY_AUTO_NEG_ASYM_PAUSE 0x0800
-#define PHY_AUTO_NEG_SYM_PAUSE 0x0400
-#define PHY_AUTO_NEG_100BT4 0x0200
-#define PHY_AUTO_NEG_100BTX_FD 0x0100
-#define PHY_AUTO_NEG_100BTX 0x0080
-#define PHY_AUTO_NEG_10BT_FD 0x0040
-#define PHY_AUTO_NEG_10BT 0x0020
-#define PHY_AUTO_NEG_SELECTOR 0x001F
-#define PHY_AUTO_NEG_802_3 0x0001
-
-#define PHY_AUTO_NEG_PAUSE (PHY_AUTO_NEG_SYM_PAUSE | PHY_AUTO_NEG_ASYM_PAUSE)
-
#define KS884X_PHY_REMOTE_CAP_OFFSET 0x0A

-/* Auto-Negotiation Link Partner Ability Register */
-#define PHY_REG_REMOTE_CAPABILITY 5
-
-#define PHY_REMOTE_NEXT_PAGE 0x8000
-#define PHY_REMOTE_ACKNOWLEDGE 0x4000
-#define PHY_REMOTE_REMOTE_FAULT 0x2000
-#define PHY_REMOTE_SYM_PAUSE 0x0400
-#define PHY_REMOTE_100BTX_FD 0x0100
-#define PHY_REMOTE_100BTX 0x0080
-#define PHY_REMOTE_10BT_FD 0x0040
-#define PHY_REMOTE_10BT 0x0020
-
/* P1VCT */
#define KS884X_P1VCT_P 0x04F0
#define KS884X_P1PHYCTRL_P 0x04F2
@@ -2886,15 +2818,6 @@ static void sw_block_addr(struct ksz_hw *hw)
}
}

-#define PHY_LINK_SUPPORT \
- (PHY_AUTO_NEG_ASYM_PAUSE | \
- PHY_AUTO_NEG_SYM_PAUSE | \
- PHY_AUTO_NEG_100BT4 | \
- PHY_AUTO_NEG_100BTX_FD | \
- PHY_AUTO_NEG_100BTX | \
- PHY_AUTO_NEG_10BT_FD | \
- PHY_AUTO_NEG_10BT)
-
static inline void hw_r_phy_ctrl(struct ksz_hw *hw, int phy, u16 *data)
{
*data = readw(hw->io + phy + KS884X_PHY_CTRL_OFFSET);
@@ -3238,16 +3161,18 @@ static void determine_flow_ctrl(struct ksz_hw *hw, struct ksz_port *port,
rx = tx = 0;
if (port->force_link)
rx = tx = 1;
- if (remote & PHY_AUTO_NEG_SYM_PAUSE) {
- if (local & PHY_AUTO_NEG_SYM_PAUSE) {
+ if (remote & LPA_PAUSE_CAP) {
+ if (local & ADVERTISE_PAUSE_CAP) {
rx = tx = 1;
- } else if ((remote & PHY_AUTO_NEG_ASYM_PAUSE) &&
- (local & PHY_AUTO_NEG_PAUSE) ==
- PHY_AUTO_NEG_ASYM_PAUSE) {
+ } else if ((remote & LPA_PAUSE_ASYM) &&
+ (local &
+ (ADVERTISE_PAUSE_CAP | ADVERTISE_PAUSE_ASYM)) ==
+ ADVERTISE_PAUSE_ASYM) {
tx = 1;
}
- } else if (remote & PHY_AUTO_NEG_ASYM_PAUSE) {
- if ((local & PHY_AUTO_NEG_PAUSE) == PHY_AUTO_NEG_PAUSE)
+ } else if (remote & LPA_PAUSE_ASYM) {
+ if ((local & (ADVERTISE_PAUSE_CAP | ADVERTISE_PAUSE_ASYM))
+ == (ADVERTISE_PAUSE_CAP | ADVERTISE_PAUSE_ASYM))
rx = 1;
}
if (!hw->ksz_switch)
@@ -3428,16 +3353,16 @@ static void port_force_link_speed(struct ksz_port *port)
phy = KS884X_PHY_1_CTRL_OFFSET + p * PHY_CTRL_INTERVAL;
hw_r_phy_ctrl(hw, phy, &data);

- data &= ~PHY_AUTO_NEG_ENABLE;
+ data &= ~BMCR_ANENABLE;

if (10 == port->speed)
- data &= ~PHY_SPEED_100MBIT;
+ data &= ~BMCR_SPEED100;
else if (100 == port->speed)
- data |= PHY_SPEED_100MBIT;
+ data |= BMCR_SPEED100;
if (1 == port->duplex)
- data &= ~PHY_FULL_DUPLEX;
+ data &= ~BMCR_FULLDPLX;
else if (2 == port->duplex)
- data |= PHY_FULL_DUPLEX;
+ data |= BMCR_FULLDPLX;
hw_w_phy_ctrl(hw, phy, data);
}
}
diff --git a/include/linux/micrel_phy.h b/include/linux/micrel_phy.h
index 416ee6dd2574..b03e2afcb53f 100644
--- a/include/linux/micrel_phy.h
+++ b/include/linux/micrel_phy.h
@@ -45,4 +45,17 @@
#define MICREL_KSZ9021_RGMII_CLK_CTRL_PAD_SCEW 0x104
#define MICREL_KSZ9021_RGMII_RX_DATA_PAD_SCEW 0x105

+/* Device specific MII_BMCR (Reg 0) bits */
+/* 1 = HP Auto MDI/MDI-X mode, 0 = Microchip Auto MDI/MDI-X mode */
+#define KSZ886X_BMCR_HP_MDIX BIT(5)
+/* 1 = Force MDI (transmit on RXP/RXM pins), 0 = Normal operation
+ * (transmit on TXP/TXM pins)
+ */
+#define KSZ886X_BMCR_FORCE_MDI BIT(4)
+/* 1 = Disable auto MDI-X */
+#define KSZ886X_BMCR_DISABLE_AUTO_MDIX BIT(3)
+#define KSZ886X_BMCR_DISABLE_FAR_END_FAULT BIT(2)
+#define KSZ886X_BMCR_DISABLE_TRANSMIT BIT(1)
+#define KSZ886X_BMCR_DISABLE_LED BIT(0)
+
#endif /* _MICREL_PHY_H */
--
2.29.2

2021-06-11 07:20:53

[permalink] [raw]

Subject: [PATCH net-next v4 9/9] net: phy: micrel: ksz886x/ksz8081: add cabletest support

This patch support for cable test for the ksz886x switches and the
ksz8081 PHY.

The patch was tested on a KSZ8873RLL switch with following results:

- port 1:
- provides invalid values, thus return -ENOTSUPP
(Errata: DS80000830A: "LinkMD does not work on Port 1",
http://ww1.microchip.com/downloads/en/DeviceDoc/KSZ8873-Errata-DS80000830A.pdf)

- port 2:
- can detect distance
- can detect open on each wire of pair A (wire 1 and 2)
- can detect open only on one wire of pair B (only wire 3)
- can detect short between wires of a pair (wires 1 + 2 or 3 + 6)
- short between pairs is detected as open.
For example short between wires 2 + 3 is detected as open.

Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/dsa/microchip/ksz8795.c | 13 ++
drivers/net/phy/micrel.c | 180 ++++++++++++++++++++++++++++
include/linux/micrel_phy.h | 1 +
3 files changed, 194 insertions(+)

diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index 6e1d238da600..cbb770b5ea37 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -970,6 +970,18 @@ static enum dsa_tag_protocol ksz8_get_tag_protocol(struct dsa_switch *ds,
DSA_TAG_PROTO_KSZ9893 : DSA_TAG_PROTO_KSZ8795;
}

+static u32 ksz8_sw_get_phy_flags(struct dsa_switch *ds, int port)
+{
+ /* Silicon Errata Sheet (DS80000830A):
+ * Port 1 does not work with LinkMD Cable-Testing.
+ * Port 1 does not respond to received PAUSE control frames.
+ */
+ if (!port)
+ return MICREL_KSZ8_P1_ERRATA;
+
+ return 0;
+}
+
static void ksz8_get_strings(struct dsa_switch *ds, int port,
u32 stringset, uint8_t *buf)
{
@@ -1503,6 +1515,7 @@ static void ksz8_validate(struct dsa_switch *ds, int port,

static const struct dsa_switch_ops ksz8_switch_ops = {
.get_tag_protocol = ksz8_get_tag_protocol,
+ .get_phy_flags = ksz8_sw_get_phy_flags,
.setup = ksz8_setup,
.phy_read = ksz_phy_read16,
.phy_write = ksz_phy_write16,
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index d091d3c3fb3b..2aa9f2aea05b 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -20,6 +20,7 @@
*/

#include <linux/bitfield.h>
+#include <linux/ethtool_netlink.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/phy.h>
@@ -53,6 +54,18 @@
#define KSZPHY_INTCS_STATUS (KSZPHY_INTCS_LINK_DOWN_STATUS |\
KSZPHY_INTCS_LINK_UP_STATUS)

+/* LinkMD Control/Status */
+#define KSZ8081_LMD 0x1d
+#define KSZ8081_LMD_ENABLE_TEST BIT(15)
+#define KSZ8081_LMD_STAT_NORMAL 0
+#define KSZ8081_LMD_STAT_OPEN 1
+#define KSZ8081_LMD_STAT_SHORT 2
+#define KSZ8081_LMD_STAT_FAIL 3
+#define KSZ8081_LMD_STAT_MASK GENMASK(14, 13)
+/* Short cable (<10 meter) has been detected by LinkMD */
+#define KSZ8081_LMD_SHORT_INDICATOR BIT(12)
+#define KSZ8081_LMD_DELTA_TIME_MASK GENMASK(8, 0)
+
/* PHY Control 1 */
#define MII_KSZPHY_CTRL_1 0x1e
#define KSZ8081_CTRL1_MDIX_STAT BIT(4)
@@ -1386,6 +1399,167 @@ static int kszphy_probe(struct phy_device *phydev)
return 0;
}

+static int ksz886x_cable_test_start(struct phy_device *phydev)
+{
+ if (phydev->dev_flags & MICREL_KSZ8_P1_ERRATA)
+ return -EOPNOTSUPP;
+
+ /* If autoneg is enabled, we won't be able to test cross pair
+ * short. In this case, the PHY will "detect" a link and
+ * confuse the internal state machine - disable auto neg here.
+ * If autoneg is disabled, we should set the speed to 10mbit.
+ */
+ return phy_clear_bits(phydev, MII_BMCR, BMCR_ANENABLE | BMCR_SPEED100);
+}
+
+static int ksz886x_cable_test_result_trans(u16 status)
+{
+ switch (FIELD_GET(KSZ8081_LMD_STAT_MASK, status)) {
+ case KSZ8081_LMD_STAT_NORMAL:
+ return ETHTOOL_A_CABLE_RESULT_CODE_OK;
+ case KSZ8081_LMD_STAT_SHORT:
+ return ETHTOOL_A_CABLE_RESULT_CODE_SAME_SHORT;
+ case KSZ8081_LMD_STAT_OPEN:
+ return ETHTOOL_A_CABLE_RESULT_CODE_OPEN;
+ case KSZ8081_LMD_STAT_FAIL:
+ fallthrough;
+ default:
+ return ETHTOOL_A_CABLE_RESULT_CODE_UNSPEC;
+ }
+}
+
+static bool ksz886x_cable_test_failed(u16 status)
+{
+ return FIELD_GET(KSZ8081_LMD_STAT_MASK, status) ==
+ KSZ8081_LMD_STAT_FAIL;
+}
+
+static bool ksz886x_cable_test_fault_length_valid(u16 status)
+{
+ switch (FIELD_GET(KSZ8081_LMD_STAT_MASK, status)) {
+ case KSZ8081_LMD_STAT_OPEN:
+ fallthrough;
+ case KSZ8081_LMD_STAT_SHORT:
+ return true;
+ }
+ return false;
+}
+
+static int ksz886x_cable_test_fault_length(u16 status)
+{
+ int dt;
+
+ /* According to the data sheet the distance to the fault is
+ * DELTA_TIME * 0.4 meters.
+ */
+ dt = FIELD_GET(KSZ8081_LMD_DELTA_TIME_MASK, status);
+
+ return (dt * 400) / 10;
+}
+
+static int ksz886x_cable_test_wait_for_completion(struct phy_device *phydev)
+{
+ int val, ret;
+
+ ret = phy_read_poll_timeout(phydev, KSZ8081_LMD, val,
+ !(val & KSZ8081_LMD_ENABLE_TEST),
+ 30000, 100000, true);
+
+ return ret < 0 ? ret : 0;
+}
+
+static int ksz886x_cable_test_one_pair(struct phy_device *phydev, int pair)
+{
+ static const int ethtool_pair[] = {
+ ETHTOOL_A_CABLE_PAIR_A,
+ ETHTOOL_A_CABLE_PAIR_B,
+ };
+ int ret, val, mdix;
+
+ /* There is no way to choice the pair, like we do one ksz9031.
+ * We can workaround this limitation by using the MDI-X functionality.
+ */
+ if (pair == 0)
+ mdix = ETH_TP_MDI;
+ else
+ mdix = ETH_TP_MDI_X;
+
+ switch (phydev->phy_id & MICREL_PHY_ID_MASK) {
+ case PHY_ID_KSZ8081:
+ ret = ksz8081_config_mdix(phydev, mdix);
+ break;
+ case PHY_ID_KSZ886X:
+ ret = ksz886x_config_mdix(phydev, mdix);
+ break;
+ default:
+ ret = -ENODEV;
+ }
+
+ if (ret)
+ return ret;
+
+ /* Now we are ready to fire. This command will send a 100ns pulse
+ * to the pair.
+ */
+ ret = phy_write(phydev, KSZ8081_LMD, KSZ8081_LMD_ENABLE_TEST);
+ if (ret)
+ return ret;
+
+ ret = ksz886x_cable_test_wait_for_completion(phydev);
+ if (ret)
+ return ret;
+
+ val = phy_read(phydev, KSZ8081_LMD);
+ if (val < 0)
+ return val;
+
+ if (ksz886x_cable_test_failed(val))
+ return -EAGAIN;
+
+ ret = ethnl_cable_test_result(phydev, ethtool_pair[pair],
+ ksz886x_cable_test_result_trans(val));
+ if (ret)
+ return ret;
+
+ if (!ksz886x_cable_test_fault_length_valid(val))
+ return 0;
+
+ return ethnl_cable_test_fault_length(phydev, ethtool_pair[pair],
+ ksz886x_cable_test_fault_length(val));
+}
+
+static int ksz886x_cable_test_get_status(struct phy_device *phydev,
+ bool *finished)
+{
+ unsigned long pair_mask = 0x3;
+ int retries = 20;
+ int pair, ret;
+
+ *finished = false;
+
+ /* Try harder if link partner is active */
+ while (pair_mask && retries--) {
+ for_each_set_bit(pair, &pair_mask, 4) {
+ ret = ksz886x_cable_test_one_pair(phydev, pair);
+ if (ret == -EAGAIN)
+ continue;
+ if (ret < 0)
+ return ret;
+ clear_bit(pair, &pair_mask);
+ }
+ /* If link partner is in autonegotiation mode it will send 2ms
+ * of FLPs with at least 6ms of silence.
+ * Add 2ms sleep to have better chances to hit this silence.
+ */
+ if (pair_mask)
+ msleep(2);
+ }
+
+ *finished = true;
+
+ return 0;
+}
+
static struct phy_driver ksphy_driver[] = {
{
.phy_id = PHY_ID_KS8737,
@@ -1492,6 +1666,7 @@ static struct phy_driver ksphy_driver[] = {
.phy_id = PHY_ID_KSZ8081,
.name = "Micrel KSZ8081 or KSZ8091",
.phy_id_mask = MICREL_PHY_ID_MASK,
+ .flags = PHY_POLL_CABLE_TEST,
/* PHY_BASIC_FEATURES */
.driver_data = &ksz8081_type,
.probe = kszphy_probe,
@@ -1506,6 +1681,8 @@ static struct phy_driver ksphy_driver[] = {
.get_stats = kszphy_get_stats,
.suspend = kszphy_suspend,
.resume = kszphy_resume,
+ .cable_test_start = ksz886x_cable_test_start,
+ .cable_test_get_status = ksz886x_cable_test_get_status,
}, {
.phy_id = PHY_ID_KSZ8061,
.name = "Micrel KSZ8061",
@@ -1594,11 +1771,14 @@ static struct phy_driver ksphy_driver[] = {
.phy_id_mask = MICREL_PHY_ID_MASK,
.name = "Micrel KSZ8851 Ethernet MAC or KSZ886X Switch",
/* PHY_BASIC_FEATURES */
+ .flags = PHY_POLL_CABLE_TEST,
.config_init = kszphy_config_init,
.config_aneg = ksz886x_config_aneg,
.read_status = ksz886x_read_status,
.suspend = genphy_suspend,
.resume = ksz886x_resume,
+ .cable_test_start = ksz886x_cable_test_start,
+ .cable_test_get_status = ksz886x_cable_test_get_status,
}, {
.name = "Micrel KSZ87XX Switch",
/* PHY_BASIC_FEATURES */
diff --git a/include/linux/micrel_phy.h b/include/linux/micrel_phy.h
index 58370abd9f4f..3d43c60b49fa 100644
--- a/include/linux/micrel_phy.h
+++ b/include/linux/micrel_phy.h
@@ -39,6 +39,7 @@
/* struct phy_device dev_flags definitions */
#define MICREL_PHY_50MHZ_CLK 0x00000001
#define MICREL_PHY_FXEN 0x00000002
+#define MICREL_KSZ8_P1_ERRATA 0x00000003

#define MICREL_KSZ9021_EXTREG_CTRL 0xB
#define MICREL_KSZ9021_EXTREG_DATA_WRITE 0xC
--
2.29.2

2021-06-11 07:21:03

[permalink] [raw]

Subject: [PATCH net-next v4 3/9] net: phy: micrel: use consistent alignments

This patch changes the alignments to one space between "#define" and the
macro.

Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/phy/micrel.c | 44 ++++++++++++++++++++--------------------
1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index a14a00328fa3..77640b990977 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -38,42 +38,42 @@

/* general Interrupt control/status reg in vendor specific block. */
#define MII_KSZPHY_INTCS 0x1B
-#define KSZPHY_INTCS_JABBER BIT(15)
-#define KSZPHY_INTCS_RECEIVE_ERR BIT(14)
-#define KSZPHY_INTCS_PAGE_RECEIVE BIT(13)
-#define KSZPHY_INTCS_PARELLEL BIT(12)
-#define KSZPHY_INTCS_LINK_PARTNER_ACK BIT(11)
-#define KSZPHY_INTCS_LINK_DOWN BIT(10)
-#define KSZPHY_INTCS_REMOTE_FAULT BIT(9)
-#define KSZPHY_INTCS_LINK_UP BIT(8)
-#define KSZPHY_INTCS_ALL (KSZPHY_INTCS_LINK_UP |\
+#define KSZPHY_INTCS_JABBER BIT(15)
+#define KSZPHY_INTCS_RECEIVE_ERR BIT(14)
+#define KSZPHY_INTCS_PAGE_RECEIVE BIT(13)
+#define KSZPHY_INTCS_PARELLEL BIT(12)
+#define KSZPHY_INTCS_LINK_PARTNER_ACK BIT(11)
+#define KSZPHY_INTCS_LINK_DOWN BIT(10)
+#define KSZPHY_INTCS_REMOTE_FAULT BIT(9)
+#define KSZPHY_INTCS_LINK_UP BIT(8)
+#define KSZPHY_INTCS_ALL (KSZPHY_INTCS_LINK_UP |\
KSZPHY_INTCS_LINK_DOWN)
-#define KSZPHY_INTCS_LINK_DOWN_STATUS BIT(2)
-#define KSZPHY_INTCS_LINK_UP_STATUS BIT(0)
-#define KSZPHY_INTCS_STATUS (KSZPHY_INTCS_LINK_DOWN_STATUS |\
+#define KSZPHY_INTCS_LINK_DOWN_STATUS BIT(2)
+#define KSZPHY_INTCS_LINK_UP_STATUS BIT(0)
+#define KSZPHY_INTCS_STATUS (KSZPHY_INTCS_LINK_DOWN_STATUS |\
KSZPHY_INTCS_LINK_UP_STATUS)

/* PHY Control 1 */
-#define MII_KSZPHY_CTRL_1 0x1e
+#define MII_KSZPHY_CTRL_1 0x1e

/* PHY Control 2 / PHY Control (if no PHY Control 1) */
-#define MII_KSZPHY_CTRL_2 0x1f
-#define MII_KSZPHY_CTRL MII_KSZPHY_CTRL_2
+#define MII_KSZPHY_CTRL_2 0x1f
+#define MII_KSZPHY_CTRL MII_KSZPHY_CTRL_2
/* bitmap of PHY register to set interrupt mode */
#define KSZPHY_CTRL_INT_ACTIVE_HIGH BIT(9)
#define KSZPHY_RMII_REF_CLK_SEL BIT(7)

/* Write/read to/from extended registers */
-#define MII_KSZPHY_EXTREG 0x0b
-#define KSZPHY_EXTREG_WRITE 0x8000
+#define MII_KSZPHY_EXTREG 0x0b
+#define KSZPHY_EXTREG_WRITE 0x8000

-#define MII_KSZPHY_EXTREG_WRITE 0x0c
-#define MII_KSZPHY_EXTREG_READ 0x0d
+#define MII_KSZPHY_EXTREG_WRITE 0x0c
+#define MII_KSZPHY_EXTREG_READ 0x0d

/* Extended registers */
-#define MII_KSZPHY_CLK_CONTROL_PAD_SKEW 0x104
-#define MII_KSZPHY_RX_DATA_PAD_SKEW 0x105
-#define MII_KSZPHY_TX_DATA_PAD_SKEW 0x106
+#define MII_KSZPHY_CLK_CONTROL_PAD_SKEW 0x104
+#define MII_KSZPHY_RX_DATA_PAD_SKEW 0x105
+#define MII_KSZPHY_TX_DATA_PAD_SKEW 0x106

#define PS_TO_REG 200

--
2.29.2

2021-06-11 19:14:39

[permalink] [raw]

Subject: Re: [PATCH net-next v4 1/9] net: phy: micrel: move phy reg offsets to common header

On Fri, Jun 11, 2021 at 09:15:19AM +0200, Oleksij Rempel wrote:
> From: Michael Grzeschik <[email protected]>
>
> Some micrel devices share the same PHY register defines. This patch
> moves them to one common header so other drivers can reuse them.
> And reuse generic MII_* defines where possible.
>
> Signed-off-by: Michael Grzeschik <[email protected]>
> Signed-off-by: Oleksij Rempel <[email protected]>
> ---

Reviewed-by: Vladimir Oltean <[email protected]>

2021-06-11 19:16:04

[permalink] [raw]

Subject: Re: [PATCH net-next v4 3/9] net: phy: micrel: use consistent alignments

On Fri, Jun 11, 2021 at 09:15:21AM +0200, Oleksij Rempel wrote:
> This patch changes the alignments to one space between "#define" and the
> macro.
>
> Signed-off-by: Oleksij Rempel <[email protected]>
> ---

Reviewed-by: Vladimir Oltean <[email protected]>

2021-06-11 19:16:41

[permalink] [raw]

Subject: Re: [PATCH net-next v4 2/9] net: dsa: microchip: ksz8795: add phylink support

On Fri, Jun 11, 2021 at 09:15:20AM +0200, Oleksij Rempel wrote:
> From: Michael Grzeschik <[email protected]>
>
> This patch adds the phylink support to the ksz8795 driver to provide
> configuration exceptions on quirky KSZ8863 and KSZ8873 ports.
>
> Signed-off-by: Michael Grzeschik <[email protected]>
> Signed-off-by: Oleksij Rempel <[email protected]>
> ---

Reviewed-by: Vladimir Oltean <[email protected]>

but it would be a good idea for Russell to take a look too.

2021-06-11 19:25:02

[permalink] [raw]

Subject: Re: [PATCH net-next v4 4/9] net: phy: micrel: apply resume errata workaround for ksz8873 and ksz8863

On Fri, Jun 11, 2021 at 09:15:22AM +0200, Oleksij Rempel wrote:
> The ksz8873 and ksz8863 switches are affected by following errata:
>
> | "Receiver error in 100BASE-TX mode following Soft Power Down"
> |
> | Some KSZ8873 devices may exhibit receiver errors after transitioning
> | from Soft Power Down mode to Normal mode, as controlled by register 195
> | (0xC3) bits [1:0]. When exiting Soft Power Down mode, the receiver
> | blocks may not start up properly, causing the PHY to miss data and
> | exhibit erratic behavior. The problem may appear on either port 1 or
> | port 2, or both ports. The problem occurs only for 100BASE-TX, not
> | 10BASE-T.
> |
> | END USER IMPLICATIONS
> | When the failure occurs, the following symptoms are seen on the affected
> | port(s):
> | - The port is able to link
> | - LED0 blinks, even when there is no traffic
> | - The MIB counters indicate receive errors (Rx Fragments, Rx Symbol
> | Errors, Rx CRC Errors, Rx Alignment Errors)
> | - Only a small fraction of packets is correctly received and forwarded
> | through the switch. Most packets are dropped due to receive errors.
> |
> | The failing condition cannot be corrected by the following:
> | - Removing and reconnecting the cable
> | - Hardware reset
> | - Software Reset and PCS Reset bits in register 67 (0x43)
> |
> | Work around:
> | The problem can be corrected by setting and then clearing the Port Power
> | Down bits (registers 29 (0x1D) and 45 (0x2D), bit 3). This must be done
> | separately for each affected port after returning from Soft Power Down
> | Mode to Normal Mode. The following procedure will ensure no further
> | issues due to this erratum. To enter Soft Power Down Mode, set register
> | 195 (0xC3), bits [1:0] = 10.
> |
> | To exit Soft Power Down Mode, follow these steps:
> | 1. Set register 195 (0xC3), bits [1:0] = 00 // Exit soft power down mode
> | 2. Wait 1ms minimum
> | 3. Set register 29 (0x1D), bit [3] = 1 // Enter PHY port 1 power down mode
> | 4. Set register 29 (0x1D), bit [3] = 0 // Exit PHY port 1 power down mode
> | 5. Set register 45 (0x2D), bit [3] = 1 // Enter PHY port 2 power down mode
> | 6. Set register 45 (0x2D), bit [3] = 0 // Exit PHY port 2 power down mode
>
> This patch implements steps 2...6 of the suggested workaround. During
> (initial) switch power up, step 1 is executed by the dsa/ksz8795
> driver's probe function.
>
> Note: In this workaround we toggle the MII_BMCR register's BMCR_PDOWN
> bit, this is translated to the actual register and bit (as mentioned in
> the arratum) by the ksz8_r_phy()/ksz8_w_phy() functions.

s/arratum/erratum/

Also, the commit message is still missing this piece of information you
gave in the previous thread:

| this issue was seen at some early point of development (back in 2019)
| reproducible on system start. Where switch was in some default state or
| on a state configured by the bootloader. I didn't tried to reproduce it
| now.

Years from now, some poor souls might struggle to understand why this
patch was done this way. If it is indeed the case that the issue is only
seen during the handover between bootloader and kernel, there is really
no reason to implement the ERR workaround in phy_resume instead of doing
it once at probe time.

>
> Signed-off-by: Oleksij Rempel <[email protected]>
> ---

2021-06-11 19:26:30

[permalink] [raw]

Subject: Re: [PATCH net-next v4 8/9] net: dsa: dsa_slave_phy_connect(): extend phy's flags with port specific phy flags

On Fri, Jun 11, 2021 at 09:15:26AM +0200, Oleksij Rempel wrote:
> This patch extends the flags of the phy that's being connected with the
> port specific flags of the switch port.
>
> This is needed to handle a port specific erratum of the KSZ8873 switch,
> which is added in a later patch.
>
> Signed-off-by: Oleksij Rempel <[email protected]>
> ---

What happens differently between having this patch and not having it?

2021-06-11 23:30:44

[permalink] [raw]

Subject: Re: [PATCH net-next v4 8/9] net: dsa: dsa_slave_phy_connect(): extend phy's flags with port specific phy flags

On 6/11/2021 12:24 PM, Vladimir Oltean wrote:
> On Fri, Jun 11, 2021 at 09:15:26AM +0200, Oleksij Rempel wrote:
>> This patch extends the flags of the phy that's being connected with the
>> port specific flags of the switch port.
>>
>> This is needed to handle a port specific erratum of the KSZ8873 switch,
>> which is added in a later patch.
>>
>> Signed-off-by: Oleksij Rempel <[email protected]>
>> ---
>
> What happens differently between having this patch and not having it?

The current get_phy_flags() is only processed when we connect to a PHY
via a designed phy-handle property via phylink_of_phy_connect((, but if
we fallback on the internal MDIO bus created by a switch and take the
dsa_slave_phy_connect() path then we would not be processing that flag
and using it at PHY connection time. Oleksij, your proposed patch fails
to check that dsa_switch_ops::get_phy_flags is actually non-NULL, how
about this approach instead where we only fetch the flags once, and we
deal with an option get_phy_flags callback too:

diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index d4756b920108..ba7866ec946f 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1749,7 +1749,7 @@ static void dsa_slave_phylink_fixed_state(struct
phylink_config *config,
}

/* slave device setup
*******************************************************/
-static int dsa_slave_phy_connect(struct net_device *slave_dev, int addr)
+static int dsa_slave_phy_connect(struct net_device *slave_dev, int
addr, u32 flags)
{
struct dsa_port *dp = dsa_slave_to_port(slave_dev);
struct dsa_switch *ds = dp->ds;
@@ -1760,6 +1760,8 @@ static int dsa_slave_phy_connect(struct net_device
*slave_dev, int addr)
return -ENODEV;
}

+ slave_dev->phydev->dev_flags |= flags;
+
return phylink_connect_phy(dp->pl, slave_dev->phydev);
}

@@ -1804,7 +1806,7 @@ static int dsa_slave_phy_setup(struct net_device
*slave_dev)
/* We could not connect to a designated PHY or SFP, so
try to
* use the switch internal MDIO bus instead
*/
- ret = dsa_slave_phy_connect(slave_dev, dp->index);
+ ret = dsa_slave_phy_connect(slave_dev, dp->index,
phy_flags);
if (ret) {
netdev_err(slave_dev,
"failed to connect to port %d: %d\n",
--
Florian

2021-06-11 23:31:04

[permalink] [raw]

Subject: Re: [PATCH net-next v4 2/9] net: dsa: microchip: ksz8795: add phylink support

On 6/11/2021 12:15 AM, Oleksij Rempel wrote:
> From: Michael Grzeschik <[email protected]>
>
> This patch adds the phylink support to the ksz8795 driver to provide
> configuration exceptions on quirky KSZ8863 and KSZ8873 ports.
>
> Signed-off-by: Michael Grzeschik <[email protected]>
> Signed-off-by: Oleksij Rempel <[email protected]>

Reviewed-by: Florian Fainelli <[email protected]>
--
Florian

2021-06-11 23:31:10

[permalink] [raw]

Subject: Re: [PATCH net-next v4 1/9] net: phy: micrel: move phy reg offsets to common header

On 6/11/2021 12:15 AM, Oleksij Rempel wrote:
> From: Michael Grzeschik <[email protected]>
>
> Some micrel devices share the same PHY register defines. This patch
> moves them to one common header so other drivers can reuse them.
> And reuse generic MII_* defines where possible.
>
> Signed-off-by: Michael Grzeschik <[email protected]>
> Signed-off-by: Oleksij Rempel <[email protected]>

Reviewed-by: Florian Fainelli <[email protected]>
--
Florian

2021-06-11 23:31:56

[permalink] [raw]

Subject: Re: [PATCH net-next v4 3/9] net: phy: micrel: use consistent alignments

On 6/11/2021 12:15 AM, Oleksij Rempel wrote:
> This patch changes the alignments to one space between "#define" and the
> macro.
>
> Signed-off-by: Oleksij Rempel <[email protected]>

Reviewed-by: Florian Fainelli <[email protected]>
--
Florian

2021-06-12 04:30:13

[permalink] [raw]

Subject: Re: [PATCH net-next v4 4/9] net: phy: micrel: apply resume errata workaround for ksz8873 and ksz8863

On Fri, Jun 11, 2021 at 10:20:10PM +0300, Vladimir Oltean wrote:
> On Fri, Jun 11, 2021 at 09:15:22AM +0200, Oleksij Rempel wrote:
> > The ksz8873 and ksz8863 switches are affected by following errata:
> >
> > | "Receiver error in 100BASE-TX mode following Soft Power Down"
> > |
> > | Some KSZ8873 devices may exhibit receiver errors after transitioning
> > | from Soft Power Down mode to Normal mode, as controlled by register 195
> > | (0xC3) bits [1:0]. When exiting Soft Power Down mode, the receiver
> > | blocks may not start up properly, causing the PHY to miss data and
> > | exhibit erratic behavior. The problem may appear on either port 1 or
> > | port 2, or both ports. The problem occurs only for 100BASE-TX, not
> > | 10BASE-T.
> > |
> > | END USER IMPLICATIONS
> > | When the failure occurs, the following symptoms are seen on the affected
> > | port(s):
> > | - The port is able to link
> > | - LED0 blinks, even when there is no traffic
> > | - The MIB counters indicate receive errors (Rx Fragments, Rx Symbol
> > | Errors, Rx CRC Errors, Rx Alignment Errors)
> > | - Only a small fraction of packets is correctly received and forwarded
> > | through the switch. Most packets are dropped due to receive errors.
> > |
> > | The failing condition cannot be corrected by the following:
> > | - Removing and reconnecting the cable
> > | - Hardware reset
> > | - Software Reset and PCS Reset bits in register 67 (0x43)
> > |
> > | Work around:
> > | The problem can be corrected by setting and then clearing the Port Power
> > | Down bits (registers 29 (0x1D) and 45 (0x2D), bit 3). This must be done
> > | separately for each affected port after returning from Soft Power Down
> > | Mode to Normal Mode. The following procedure will ensure no further
> > | issues due to this erratum. To enter Soft Power Down Mode, set register
> > | 195 (0xC3), bits [1:0] = 10.
> > |
> > | To exit Soft Power Down Mode, follow these steps:
> > | 1. Set register 195 (0xC3), bits [1:0] = 00 // Exit soft power down mode
> > | 2. Wait 1ms minimum
> > | 3. Set register 29 (0x1D), bit [3] = 1 // Enter PHY port 1 power down mode
> > | 4. Set register 29 (0x1D), bit [3] = 0 // Exit PHY port 1 power down mode
> > | 5. Set register 45 (0x2D), bit [3] = 1 // Enter PHY port 2 power down mode
> > | 6. Set register 45 (0x2D), bit [3] = 0 // Exit PHY port 2 power down mode
> >
> > This patch implements steps 2...6 of the suggested workaround. During
> > (initial) switch power up, step 1 is executed by the dsa/ksz8795
> > driver's probe function.
> >
> > Note: In this workaround we toggle the MII_BMCR register's BMCR_PDOWN
> > bit, this is translated to the actual register and bit (as mentioned in
> > the arratum) by the ksz8_r_phy()/ksz8_w_phy() functions.
>
> s/arratum/erratum/
>
> Also, the commit message is still missing this piece of information you
> gave in the previous thread:
>
> | this issue was seen at some early point of development (back in 2019)
> | reproducible on system start. Where switch was in some default state or
> | on a state configured by the bootloader. I didn't tried to reproduce it
> | now.
>
> Years from now, some poor souls might struggle to understand why this
> patch was done this way. If it is indeed the case that the issue is only
> seen during the handover between bootloader and kernel, there is really
> no reason to implement the ERR workaround in phy_resume instead of doing
> it once at probe time.

Ok, i'll drop this patch for now.

Regards,
Oleksij
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |

2021-06-12 15:17:20

[permalink] [raw]

Subject: Re: [PATCH net-next v4 4/9] net: phy: micrel: apply resume errata workaround for ksz8873 and ksz8863

On Sat, Jun 12, 2021 at 06:26:39AM +0200, Oleksij Rempel wrote:
> On Fri, Jun 11, 2021 at 10:20:10PM +0300, Vladimir Oltean wrote:
> > On Fri, Jun 11, 2021 at 09:15:22AM +0200, Oleksij Rempel wrote:
> > > The ksz8873 and ksz8863 switches are affected by following errata:
> > >
> > > | "Receiver error in 100BASE-TX mode following Soft Power Down"
> > > |
> > > | Some KSZ8873 devices may exhibit receiver errors after transitioning
> > > | from Soft Power Down mode to Normal mode, as controlled by register 195
> > > | (0xC3) bits [1:0]. When exiting Soft Power Down mode, the receiver
> > > | blocks may not start up properly, causing the PHY to miss data and
> > > | exhibit erratic behavior. The problem may appear on either port 1 or
> > > | port 2, or both ports. The problem occurs only for 100BASE-TX, not
> > > | 10BASE-T.
> > > |
> > > | END USER IMPLICATIONS
> > > | When the failure occurs, the following symptoms are seen on the affected
> > > | port(s):
> > > | - The port is able to link
> > > | - LED0 blinks, even when there is no traffic
> > > | - The MIB counters indicate receive errors (Rx Fragments, Rx Symbol
> > > | Errors, Rx CRC Errors, Rx Alignment Errors)
> > > | - Only a small fraction of packets is correctly received and forwarded
> > > | through the switch. Most packets are dropped due to receive errors.
> > > |
> > > | The failing condition cannot be corrected by the following:
> > > | - Removing and reconnecting the cable
> > > | - Hardware reset
> > > | - Software Reset and PCS Reset bits in register 67 (0x43)
> > > |
> > > | Work around:
> > > | The problem can be corrected by setting and then clearing the Port Power
> > > | Down bits (registers 29 (0x1D) and 45 (0x2D), bit 3). This must be done
> > > | separately for each affected port after returning from Soft Power Down
> > > | Mode to Normal Mode. The following procedure will ensure no further
> > > | issues due to this erratum. To enter Soft Power Down Mode, set register
> > > | 195 (0xC3), bits [1:0] = 10.
> > > |
> > > | To exit Soft Power Down Mode, follow these steps:
> > > | 1. Set register 195 (0xC3), bits [1:0] = 00 // Exit soft power down mode
> > > | 2. Wait 1ms minimum
> > > | 3. Set register 29 (0x1D), bit [3] = 1 // Enter PHY port 1 power down mode
> > > | 4. Set register 29 (0x1D), bit [3] = 0 // Exit PHY port 1 power down mode
> > > | 5. Set register 45 (0x2D), bit [3] = 1 // Enter PHY port 2 power down mode
> > > | 6. Set register 45 (0x2D), bit [3] = 0 // Exit PHY port 2 power down mode
> > >
> > > This patch implements steps 2...6 of the suggested workaround. During
> > > (initial) switch power up, step 1 is executed by the dsa/ksz8795
> > > driver's probe function.
> > >
> > > Note: In this workaround we toggle the MII_BMCR register's BMCR_PDOWN
> > > bit, this is translated to the actual register and bit (as mentioned in
> > > the arratum) by the ksz8_r_phy()/ksz8_w_phy() functions.
> >
> > s/arratum/erratum/
> >
> > Also, the commit message is still missing this piece of information you
> > gave in the previous thread:
> >
> > | this issue was seen at some early point of development (back in 2019)
> > | reproducible on system start. Where switch was in some default state or
> > | on a state configured by the bootloader. I didn't tried to reproduce it
> > | now.
> >
> > Years from now, some poor souls might struggle to understand why this
> > patch was done this way. If it is indeed the case that the issue is only
> > seen during the handover between bootloader and kernel, there is really
> > no reason to implement the ERR workaround in phy_resume instead of doing
> > it once at probe time.
>
> Ok, i'll drop this patch for now.

I mean, you don't have to drop it, you just have to provide a competent
explanation for how the patch addresses the ERR as described by Microchip.
Do you still have a board with this switch?

2021-06-12 18:15:27

by Andrew Lunn

[permalink] [raw]

Subject: Re: [PATCH net-next v4 5/9] net: phy/dsa micrel/ksz886x add MDI-X support

On Fri, Jun 11, 2021 at 09:15:23AM +0200, Oleksij Rempel wrote:
> Add support for MDI-X status and configuration
>
> Signed-off-by: Oleksij Rempel <[email protected]>

Reviewed-by: Andrew Lunn <[email protected]>

Andrew

2021-06-12 18:20:24

by Andrew Lunn

[permalink] [raw]

Subject: Re: [PATCH net-next v4 7/9] net: dsa: microchip: ksz8795: add LINK_MD register support

On Fri, Jun 11, 2021 at 09:15:25AM +0200, Oleksij Rempel wrote:
> From: Oleksij Rempel <[email protected]>
>
> Add mapping for LINK_MD register to enable cable testing functionality.
>
> Signed-off-by: Oleksij Rempel <[email protected]>

Reviewed-by: Andrew Lunn <[email protected]>

Andrew

2021-06-12 18:25:12

by Andrew Lunn

[permalink] [raw]

Subject: Re: [PATCH net-next v4 9/9] net: phy: micrel: ksz886x/ksz8081: add cabletest support

> +static int ksz886x_cable_test_get_status(struct phy_device *phydev,
> + bool *finished)
> +{
> + unsigned long pair_mask = 0x3;
> + int retries = 20;
> + int pair, ret;
> +
> + *finished = false;
> +
> + /* Try harder if link partner is active */
> + while (pair_mask && retries--) {
> + for_each_set_bit(pair, &pair_mask, 4) {
> + ret = ksz886x_cable_test_one_pair(phydev, pair);
> + if (ret == -EAGAIN)
> + continue;
> + if (ret < 0)
> + return ret;
> + clear_bit(pair, &pair_mask);
> + }
> + /* If link partner is in autonegotiation mode it will send 2ms
> + * of FLPs with at least 6ms of silence.
> + * Add 2ms sleep to have better chances to hit this silence.
> + */
> + if (pair_mask)
> + msleep(2);
> + }
> +
> + *finished = true;
> +
> + return 0;

If ksz886x_cable_test_one_pair() returns -EAGAIN 20x and it gives up,
you end up returning 0. Maybe it would be better to return ret?

Andrew

2021-06-13 02:16:49

[permalink] [raw]

Subject: Re: [PATCH net-next v4 7/9] net: dsa: microchip: ksz8795: add LINK_MD register support

On 6/11/2021 12:15 AM, Oleksij Rempel wrote:
> From: Oleksij Rempel <[email protected]>
>
> Add mapping for LINK_MD register to enable cable testing functionality.
>
> Signed-off-by: Oleksij Rempel <[email protected]>

Reviewed-by: Florian Fainelli <[email protected]>
--
Florian

2021-06-13 02:17:27

[permalink] [raw]

Subject: Re: [PATCH net-next v4 5/9] net: phy/dsa micrel/ksz886x add MDI-X support

On 6/11/2021 12:15 AM, Oleksij Rempel wrote:
> Add support for MDI-X status and configuration
>
> Signed-off-by: Oleksij Rempel <[email protected]>

Reviewed-by: Florian Fainelli <[email protected]>
--
Florian

2021-06-14 03:39:40

[permalink] [raw]

Subject: Re: [PATCH net-next v4 8/9] net: dsa: dsa_slave_phy_connect(): extend phy's flags with port specific phy flags

On Fri, Jun 11, 2021 at 04:26:33PM -0700, Florian Fainelli wrote:
>
>
> On 6/11/2021 12:24 PM, Vladimir Oltean wrote:
> > On Fri, Jun 11, 2021 at 09:15:26AM +0200, Oleksij Rempel wrote:
> >> This patch extends the flags of the phy that's being connected with the
> >> port specific flags of the switch port.
> >>
> >> This is needed to handle a port specific erratum of the KSZ8873 switch,
> >> which is added in a later patch.
> >>
> >> Signed-off-by: Oleksij Rempel <[email protected]>
> >> ---
> >
> > What happens differently between having this patch and not having it?
>
> The current get_phy_flags() is only processed when we connect to a PHY
> via a designed phy-handle property via phylink_of_phy_connect((, but if
> we fallback on the internal MDIO bus created by a switch and take the
> dsa_slave_phy_connect() path then we would not be processing that flag
> and using it at PHY connection time. Oleksij, your proposed patch fails
> to check that dsa_switch_ops::get_phy_flags is actually non-NULL, how
> about this approach instead where we only fetch the flags once, and we
> deal with an option get_phy_flags callback too:

ok, sounds good.
Thank you.

> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index d4756b920108..ba7866ec946f 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -1749,7 +1749,7 @@ static void dsa_slave_phylink_fixed_state(struct
> phylink_config *config,
> }
>
> /* slave device setup
> *******************************************************/
> -static int dsa_slave_phy_connect(struct net_device *slave_dev, int addr)
> +static int dsa_slave_phy_connect(struct net_device *slave_dev, int
> addr, u32 flags)
> {
> struct dsa_port *dp = dsa_slave_to_port(slave_dev);
> struct dsa_switch *ds = dp->ds;
> @@ -1760,6 +1760,8 @@ static int dsa_slave_phy_connect(struct net_device
> *slave_dev, int addr)
> return -ENODEV;
> }
>
> + slave_dev->phydev->dev_flags |= flags;
> +
> return phylink_connect_phy(dp->pl, slave_dev->phydev);
> }
>
> @@ -1804,7 +1806,7 @@ static int dsa_slave_phy_setup(struct net_device
> *slave_dev)
> /* We could not connect to a designated PHY or SFP, so
> try to
> * use the switch internal MDIO bus instead
> */
> - ret = dsa_slave_phy_connect(slave_dev, dp->index);
> + ret = dsa_slave_phy_connect(slave_dev, dp->index,
> phy_flags);
> if (ret) {
> netdev_err(slave_dev,
> "failed to connect to port %d: %d\n",
> --
> Florian
>
>

--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |

2021-06-14 03:40:05

[permalink] [raw]

Subject: Re: [PATCH net-next v4 8/9] net: dsa: dsa_slave_phy_connect(): extend phy's flags with port specific phy flags

On Fri, Jun 11, 2021 at 10:24:17PM +0300, Vladimir Oltean wrote:
> On Fri, Jun 11, 2021 at 09:15:26AM +0200, Oleksij Rempel wrote:
> > This patch extends the flags of the phy that's being connected with the
> > port specific flags of the switch port.
> >
> > This is needed to handle a port specific erratum of the KSZ8873 switch,
> > which is added in a later patch.
> >
> > Signed-off-by: Oleksij Rempel <[email protected]>
> > ---
>
> What happens differently between having this patch and not having it?

Without this patch, the PHY driver will not get the phyflag provided by the
DSA driver.

Regards,
Oleksij
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |

2021-06-14 03:58:12

[permalink] [raw]

Subject: Re: [PATCH net-next v4 9/9] net: phy: micrel: ksz886x/ksz8081: add cabletest support

On Sat, Jun 12, 2021 at 08:23:53PM +0200, Andrew Lunn wrote:
> > +static int ksz886x_cable_test_get_status(struct phy_device *phydev,
> > + bool *finished)
> > +{
> > + unsigned long pair_mask = 0x3;
> > + int retries = 20;
> > + int pair, ret;
> > +
> > + *finished = false;
> > +
> > + /* Try harder if link partner is active */
> > + while (pair_mask && retries--) {
> > + for_each_set_bit(pair, &pair_mask, 4) {
> > + ret = ksz886x_cable_test_one_pair(phydev, pair);
> > + if (ret == -EAGAIN)
> > + continue;
> > + if (ret < 0)
> > + return ret;
> > + clear_bit(pair, &pair_mask);
> > + }
> > + /* If link partner is in autonegotiation mode it will send 2ms
> > + * of FLPs with at least 6ms of silence.
> > + * Add 2ms sleep to have better chances to hit this silence.
> > + */
> > + if (pair_mask)
> > + msleep(2);
> > + }
> > +
> > + *finished = true;
> > +
> > + return 0;
>
> If ksz886x_cable_test_one_pair() returns -EAGAIN 20x and it gives up,
> you end up returning 0. Maybe it would be better to return ret?

Good point. Fixed.

Regards,
Oleksij
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |

2021-06-14 06:57:21