This patches provide support for cable testing on the ksz886x switches.
Since it has one special port, we needed to add phylink with validation
and extra quirk for the PHY to signal, that one port will not provide
valid cable testing reports.
Michael Grzeschik (3):
net: phy: micrel: move phy reg offsets to common header
net: dsa: microchip: ksz8795: add phylink support
net: phy: micrel: add patch for erratas on port1
Oleksij Rempel (6):
net: phy: micrel: use consistent indention after define
net: phy: micrel: apply resume errata workaround for ksz8873 and
ksz8863
net: phy: micrel: ksz886x add MDI-X support
net: phy: micrel: ksz8081 add MDI-X support
net: dsa: microchip: ksz8795: add LINK_MD register support
net: phy: micrel: ksz886x/ksz8081: add cabletest support
drivers/net/dsa/microchip/ksz8795.c | 110 +++++++
drivers/net/dsa/microchip/ksz8795_reg.h | 67 +---
drivers/net/ethernet/micrel/ksz884x.c | 70 +---
drivers/net/phy/micrel.c | 416 +++++++++++++++++++++++-
drivers/net/phy/phylink.c | 2 +-
include/linux/micrel_phy.h | 64 ++++
net/dsa/slave.c | 4 +
7 files changed, 586 insertions(+), 147 deletions(-)
--
2.29.2
Add support for MDI-X status and configuration
Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/phy/micrel.c | 89 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 89 insertions(+)
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index ea30cd6bd7bc..47fa8b02630a 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -68,11 +68,17 @@
/* PHY Control 1 */
#define MII_KSZPHY_CTRL_1 0x1e
+#define KSZ8081_CTRL1_MDIX_STAT BIT(4)
/* PHY Control 2 / PHY Control (if no PHY Control 1) */
#define MII_KSZPHY_CTRL_2 0x1f
#define MII_KSZPHY_CTRL MII_KSZPHY_CTRL_2
/* bitmap of PHY register to set interrupt mode */
+#define KSZ8081_CTRL2_HP_MDIX BIT(15)
+#define KSZ8081_CTRL2_MDI_MDI_X_SELECT BIT(14)
+#define KSZ8081_CTRL2_DISABLE_AUTO_MDIX BIT(13)
+#define KSZ8081_CTRL2_FORCE_LINK BIT(11)
+#define KSZ8081_CTRL2_POWER_SAVING BIT(10)
#define KSZPHY_CTRL_INT_ACTIVE_HIGH BIT(9)
#define KSZPHY_RMII_REF_CLK_SEL BIT(7)
#define KSZ886X_CTRL_MDIX_STAT BIT(4)
@@ -436,6 +442,87 @@ static int ksz8081_config_init(struct phy_device *phydev)
return kszphy_config_init(phydev);
}
+static int ksz8081_config_mdix(struct phy_device *phydev, u8 ctrl)
+{
+ u16 val;
+
+ switch (ctrl) {
+ case ETH_TP_MDI:
+ val = KSZ8081_CTRL2_DISABLE_AUTO_MDIX;
+ break;
+ case ETH_TP_MDI_X:
+ val = KSZ8081_CTRL2_DISABLE_AUTO_MDIX |
+ KSZ8081_CTRL2_MDI_MDI_X_SELECT;
+ break;
+ case ETH_TP_MDI_AUTO:
+ val = 0;
+ break;
+ default:
+ return 0;
+ }
+
+ return phy_modify(phydev, MII_KSZPHY_CTRL_2,
+ KSZ8081_CTRL2_HP_MDIX |
+ KSZ8081_CTRL2_MDI_MDI_X_SELECT |
+ KSZ8081_CTRL2_DISABLE_AUTO_MDIX,
+ KSZ8081_CTRL2_HP_MDIX | val);
+}
+
+static int ksz8081_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 ksz8081_config_mdix(phydev, phydev->mdix_ctrl);
+}
+
+static int ksz8081_mdix_update(struct phy_device *phydev)
+{
+ int ret;
+
+ ret = phy_read(phydev, MII_KSZPHY_CTRL_2);
+ if (ret < 0)
+ return ret;
+
+ if (ret & KSZ8081_CTRL2_DISABLE_AUTO_MDIX) {
+ if (ret & KSZ8081_CTRL2_MDI_MDI_X_SELECT)
+ 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_1);
+ if (ret < 0)
+ return ret;
+
+ if (ret & KSZ8081_CTRL1_MDIX_STAT)
+ phydev->mdix = ETH_TP_MDI;
+ else
+ phydev->mdix = ETH_TP_MDI_X;
+
+ return 0;
+}
+
+static int ksz8081_read_status(struct phy_device *phydev)
+{
+ int ret;
+
+ ret = ksz8081_mdix_update(phydev);
+ if (ret < 0)
+ return ret;
+
+ return genphy_read_status(phydev);
+}
+
static int ksz8061_config_init(struct phy_device *phydev)
{
int ret;
@@ -1423,6 +1510,8 @@ static struct phy_driver ksphy_driver[] = {
.probe = kszphy_probe,
.config_init = ksz8081_config_init,
.soft_reset = genphy_soft_reset,
+ .config_aneg = ksz8081_config_aneg,
+ .read_status = ksz8081_read_status,
.config_intr = kszphy_config_intr,
.handle_interrupt = kszphy_handle_interrupt,
.get_sset_count = kszphy_get_sset_count,
--
2.29.2
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:
- cannot detect any distance
- provides inverted values
(Errata: DS80000830A: "LinkMD does not work on Port 1",
http://ww1.microchip.com/downloads/en/DeviceDoc/KSZ8873-Errata-DS80000830A.pdf)
- Reports "short" on open or ok.
- Reports "ok" on short.
- 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.
In order to work around the errata for port 1, the ksz8795 switch driver
should be extended to provide proper device tree support for the related
PHY nodes. So we can set a DT property to mark the port 1 as affected by
the errata.
Signed-off-by: Oleksij Rempel <[email protected]>
---
- added PHY_POLL_CABLE_TEST to make it work in interrupt mode
---
drivers/net/phy/micrel.c | 177 +++++++++++++++++++++++++++++++++++++++
1 file changed, 177 insertions(+)
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 47fa8b02630a..f0ca7b53bcf9 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>
@@ -66,6 +67,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)
@@ -1399,6 +1412,164 @@ static int kszphy_probe(struct phy_device *phydev)
return 0;
}
+static int ksz886x_cable_test_start(struct phy_device *phydev)
+{
+ /* 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:
+ /* fall through */
+ 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:
+ /* fall through */
+ 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,
@@ -1505,6 +1676,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,
@@ -1519,6 +1691,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",
@@ -1607,11 +1781,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 */
--
2.29.2
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. The first
step needs to be implemented in the switch driver.
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 227d88db7d27..f03188ed953a 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
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.
Signed-off-by: Michael Grzeschik <[email protected]>
---
drivers/net/dsa/microchip/ksz8795.c | 1 +
drivers/net/dsa/microchip/ksz8795_reg.h | 62 ----------------------
drivers/net/ethernet/micrel/ksz884x.c | 70 +------------------------
include/linux/micrel_phy.h | 63 ++++++++++++++++++++++
4 files changed, 65 insertions(+), 131 deletions(-)
diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index ad509a57a945..4ca352fbe81c 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>
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 9ed264ed7070..481426d0bda7 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
diff --git a/include/linux/micrel_phy.h b/include/linux/micrel_phy.h
index 416ee6dd2574..ee23acc4d551 100644
--- a/include/linux/micrel_phy.h
+++ b/include/linux/micrel_phy.h
@@ -45,4 +45,67 @@
#define MICREL_KSZ9021_RGMII_CLK_CTRL_PAD_SCEW 0x104
#define MICREL_KSZ9021_RGMII_RX_DATA_PAD_SCEW 0x105
+#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_ASYM_PAUSE BIT(11)
+#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_AUTO_NEG_PAUSE (PHY_AUTO_NEG_SYM_PAUSE | PHY_AUTO_NEG_ASYM_PAUSE)
+
+#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 /* _MICREL_PHY_H */
--
2.29.2
On Wed, May 05, 2021 at 11:20:24AM +0200, Oleksij Rempel wrote:
> 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:
> - cannot detect any distance
> - provides inverted values
> (Errata: DS80000830A: "LinkMD does not work on Port 1",
> http://ww1.microchip.com/downloads/en/DeviceDoc/KSZ8873-Errata-DS80000830A.pdf)
> - Reports "short" on open or ok.
> - Reports "ok" on short.
Quite broken. Distance is optional, simply don't report it. Status is
harder. Reporting ETHTOOL_A_CABLE_RESULT_CODE_OK should really mean
the cable is O.K. If you cannot tell open from O.K, i would return
ETHTOOL_A_CABLE_RESULT_CODE_UNSPEC.
More later.
Andrew
> +#define PHY_REG_CTRL 0
MII_BMCR
> +
> +#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)
All the above appear to be standard BMCR bits. Please use the existing
#defines in include/uapi/linux/mii.h
> +#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)
Since you are moving into a global scope header, please add a device
prefix.
> +
> +#define PHY_REG_STATUS 1
MII_BMSR
> +
> +#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)
These also look to be pretty standard BMSR defines.
> +
> +#define PHY_REG_ID_1 2
> +#define PHY_REG_ID_2 3
MII_PHYSID1 & MII_PHYSID2
Please remove everything which directly matches the existing defines.
Just add defines for bits which don't follow 802.3 c22.
Andrew
On Wed, May 05, 2021 at 02:47:19PM +0200, Andrew Lunn wrote:
> On Wed, May 05, 2021 at 11:20:24AM +0200, Oleksij Rempel wrote:
> > 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:
> > - cannot detect any distance
> > - provides inverted values
> > (Errata: DS80000830A: "LinkMD does not work on Port 1",
> > http://ww1.microchip.com/downloads/en/DeviceDoc/KSZ8873-Errata-DS80000830A.pdf)
> > - Reports "short" on open or ok.
> > - Reports "ok" on short.
>
> Quite broken. Distance is optional, simply don't report it. Status is
> harder. Reporting ETHTOOL_A_CABLE_RESULT_CODE_OK should really mean
> the cable is O.K. If you cannot tell open from O.K, i would return
> ETHTOOL_A_CABLE_RESULT_CODE_UNSPEC.
>
Yes, patch "net: phy: micrel: add patch for erratas on port1" provides
a flag to return -ENOTSUPP on this port.
Is it acceptable way? Should I squash this patches?
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 |
On Mon, May 10, 2021 at 11:06:56AM +0200, Oleksij Rempel wrote:
> On Wed, May 05, 2021 at 02:47:19PM +0200, Andrew Lunn wrote:
> > On Wed, May 05, 2021 at 11:20:24AM +0200, Oleksij Rempel wrote:
> > > 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:
> > > - cannot detect any distance
> > > - provides inverted values
> > > (Errata: DS80000830A: "LinkMD does not work on Port 1",
> > > http://ww1.microchip.com/downloads/en/DeviceDoc/KSZ8873-Errata-DS80000830A.pdf)
> > > - Reports "short" on open or ok.
> > > - Reports "ok" on short.
> >
> > Quite broken. Distance is optional, simply don't report it. Status is
> > harder. Reporting ETHTOOL_A_CABLE_RESULT_CODE_OK should really mean
> > the cable is O.K. If you cannot tell open from O.K, i would return
> > ETHTOOL_A_CABLE_RESULT_CODE_UNSPEC.
> >
>
> Yes, patch "net: phy: micrel: add patch for erratas on port1" provides
> a flag to return -ENOTSUPP on this port.
>
> Is it acceptable way? Should I squash this patches?
This is O.K. Maybe add a comment that later patches in the series with
handle the errata?
Andrew