2020-03-09 08:37:36

by Jose Abreu

[permalink] [raw]
Subject: [PATCH net-next 7/8] net: phy: Add Synopsys DesignWare XPCS MDIO module

Synopsys DesignWare XPCS is an MMD that can manage link status,
auto-negotiation, link training, ...

In this commit we add basic support for XPCS using USXGMII interface and
Clause 73 Auto-negotiation.

This is highly tied with PHYLINK and can't be used without it. A given
ethernet driver can use the provided callbacks to add the support for
XPCS.

Signed-off-by: Jose Abreu <[email protected]>

---
Cc: Giuseppe Cavallaro <[email protected]>
Cc: Alexandre Torgue <[email protected]>
Cc: Jose Abreu <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Jakub Kicinski <[email protected]>
Cc: Maxime Coquelin <[email protected]>
Cc: Russell King <[email protected]>
Cc: Andrew Lunn <[email protected]>
Cc: Florian Fainelli <[email protected]>
Cc: Heiner Kallweit <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
MAINTAINERS | 7 +
drivers/net/phy/Kconfig | 6 +
drivers/net/phy/Makefile | 1 +
drivers/net/phy/mdio-xpcs.c | 612 ++++++++++++++++++++++++++++++++++++++++++++
include/linux/mdio-xpcs.h | 41 +++
5 files changed, 667 insertions(+)
create mode 100644 drivers/net/phy/mdio-xpcs.c
create mode 100644 include/linux/mdio-xpcs.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 2ec6a539fa42..47f594df18cf 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16111,6 +16111,13 @@ L: [email protected]
S: Supported
F: drivers/net/ethernet/synopsys/

+SYNOPSYS DESIGNWARE ETHERNET XPCS DRIVER
+M: Jose Abreu <[email protected]>
+L: [email protected]
+S: Supported
+F: drivers/net/phy/mdio-xpcs.c
+F: include/linux/mdio-xpcs.h
+
SYNOPSYS DESIGNWARE I2C DRIVER
M: Jarkko Nikula <[email protected]>
R: Andy Shevchenko <[email protected]>
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index d6f197e06134..cc7f1df855da 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -214,6 +214,12 @@ config MDIO_XGENE
This module provides a driver for the MDIO busses found in the
APM X-Gene SoC's.

+config MDIO_XPCS
+ tristate "Synopsys DesignWare XPCS controller"
+ help
+ This module provides helper functions for Synopsys DesignWare XPCS
+ controllers.
+
endif
endif

diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index d9b3c0fec8e3..26f8039f300f 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -44,6 +44,7 @@ obj-$(CONFIG_MDIO_OCTEON) += mdio-octeon.o
obj-$(CONFIG_MDIO_SUN4I) += mdio-sun4i.o
obj-$(CONFIG_MDIO_THUNDER) += mdio-thunder.o
obj-$(CONFIG_MDIO_XGENE) += mdio-xgene.o
+obj-$(CONFIG_MDIO_XPCS) += mdio-xpcs.o

obj-$(CONFIG_NETWORK_PHY_TIMESTAMPING) += mii_timestamper.o

diff --git a/drivers/net/phy/mdio-xpcs.c b/drivers/net/phy/mdio-xpcs.c
new file mode 100644
index 000000000000..973f588146f7
--- /dev/null
+++ b/drivers/net/phy/mdio-xpcs.c
@@ -0,0 +1,612 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2020 Synopsys, Inc. and/or its affiliates.
+ * Synopsys DesignWare XPCS helpers
+ *
+ * Author: Jose Abreu <[email protected]>
+ */
+
+#include <linux/delay.h>
+#include <linux/mdio.h>
+#include <linux/mdio-xpcs.h>
+#include <linux/phylink.h>
+#include <linux/workqueue.h>
+
+#define SYNOPSYS_XPCS_USXGMII_ID 0x7996ced0
+#define SYNOPSYS_XPCS_10GKR_ID 0x7996ced0
+#define SYNOPSYS_XPCS_MASK 0xffffffff
+
+/* Vendor regs access */
+#define DW_VENDOR BIT(15)
+
+/* VR_XS_PCS */
+#define DW_USXGMII_RST BIT(10)
+#define DW_USXGMII_EN BIT(9)
+#define DW_VR_XS_PCS_DIG_STS 0x0010
+#define DW_RXFIFO_ERR GENMASK(6, 5)
+
+/* SR_MII */
+#define DW_USXGMII_FULL BIT(8)
+#define DW_USXGMII_SS_MASK (BIT(13) | BIT(6) | BIT(5))
+#define DW_USXGMII_10000 (BIT(13) | BIT(6))
+#define DW_USXGMII_5000 (BIT(13) | BIT(5))
+#define DW_USXGMII_2500 (BIT(5))
+#define DW_USXGMII_1000 (BIT(6))
+#define DW_USXGMII_100 (BIT(13))
+#define DW_USXGMII_10 (0)
+
+/* SR_AN */
+#define DW_SR_AN_ADV1 0x10
+#define DW_SR_AN_ADV2 0x11
+#define DW_SR_AN_ADV3 0x12
+#define DW_SR_AN_LP_ABL1 0x13
+#define DW_SR_AN_LP_ABL2 0x14
+#define DW_SR_AN_LP_ABL3 0x15
+
+/* Clause 73 Defines */
+/* AN_LP_ABL1 */
+#define DW_C73_PAUSE BIT(10)
+#define DW_C73_ASYM_PAUSE BIT(11)
+#define DW_C73_AN_ADV_SF 0x1
+/* AN_LP_ABL2 */
+#define DW_C73_1000KX BIT(5)
+#define DW_C73_10000KX4 BIT(6)
+#define DW_C73_10000KR BIT(7)
+/* AN_LP_ABL3 */
+#define DW_C73_2500KX BIT(0)
+#define DW_C73_5000KR BIT(1)
+
+static const int xpcs_usxgmii_features[] = {
+ ETHTOOL_LINK_MODE_Pause_BIT,
+ ETHTOOL_LINK_MODE_Asym_Pause_BIT,
+ ETHTOOL_LINK_MODE_Autoneg_BIT,
+ ETHTOOL_LINK_MODE_1000baseKX_Full_BIT,
+ ETHTOOL_LINK_MODE_10000baseKX4_Full_BIT,
+ ETHTOOL_LINK_MODE_10000baseKR_Full_BIT,
+ ETHTOOL_LINK_MODE_2500baseX_Full_BIT,
+ __ETHTOOL_LINK_MODE_MASK_NBITS,
+};
+
+static const int xpcs_10gkr_features[] = {
+ ETHTOOL_LINK_MODE_Pause_BIT,
+ ETHTOOL_LINK_MODE_Asym_Pause_BIT,
+ ETHTOOL_LINK_MODE_10000baseKR_Full_BIT,
+ __ETHTOOL_LINK_MODE_MASK_NBITS,
+};
+
+static const phy_interface_t xpcs_usxgmii_interfaces[] = {
+ PHY_INTERFACE_MODE_USXGMII,
+ PHY_INTERFACE_MODE_MAX,
+};
+
+static const phy_interface_t xpcs_10gkr_interfaces[] = {
+ PHY_INTERFACE_MODE_10GKR,
+ PHY_INTERFACE_MODE_MAX,
+};
+
+static struct xpcs_id {
+ u32 id;
+ u32 mask;
+ const int *supported;
+ const phy_interface_t *interface;
+} xpcs_id_list[] = {
+ {
+ .id = SYNOPSYS_XPCS_USXGMII_ID,
+ .mask = SYNOPSYS_XPCS_MASK,
+ .supported = xpcs_usxgmii_features,
+ .interface = xpcs_usxgmii_interfaces,
+ }, {
+ .id = SYNOPSYS_XPCS_10GKR_ID,
+ .mask = SYNOPSYS_XPCS_MASK,
+ .supported = xpcs_10gkr_features,
+ .interface = xpcs_10gkr_interfaces,
+ },
+};
+
+static int xpcs_read(struct mdio_xpcs_args *xpcs, int dev, u32 reg)
+{
+ u32 reg_addr = MII_ADDR_C45 | dev << 16 | reg;
+
+ return mdiobus_read(xpcs->bus, xpcs->addr, reg_addr);
+}
+
+static int xpcs_write(struct mdio_xpcs_args *xpcs, int dev, u32 reg, u16 val)
+{
+ u32 reg_addr = MII_ADDR_C45 | dev << 16 | reg;
+
+ return mdiobus_write(xpcs->bus, xpcs->addr, reg_addr, val);
+}
+
+static int xpcs_read_vendor(struct mdio_xpcs_args *xpcs, int dev, u32 reg)
+{
+ return xpcs_read(xpcs, dev, DW_VENDOR | reg);
+}
+
+static int xpcs_write_vendor(struct mdio_xpcs_args *xpcs, int dev, int reg,
+ u16 val)
+{
+ return xpcs_write(xpcs, dev, DW_VENDOR | reg, val);
+}
+
+static int xpcs_read_vpcs(struct mdio_xpcs_args *xpcs, int reg)
+{
+ return xpcs_read_vendor(xpcs, MDIO_MMD_PCS, reg);
+}
+
+static int xpcs_write_vpcs(struct mdio_xpcs_args *xpcs, int reg, u16 val)
+{
+ return xpcs_write_vendor(xpcs, MDIO_MMD_PCS, reg, val);
+}
+
+static int xpcs_poll_reset(struct mdio_xpcs_args *xpcs, int dev)
+{
+ /* Poll until the reset bit clears (50ms per retry == 0.6 sec) */
+ unsigned int retries = 12;
+ int ret;
+
+ do {
+ msleep(50);
+ ret = xpcs_read(xpcs, dev, MDIO_CTRL1);
+ if (ret < 0)
+ return ret;
+ } while (ret & MDIO_CTRL1_RESET && --retries);
+
+ return (ret & MDIO_CTRL1_RESET) ? -ETIMEDOUT : 0;
+}
+
+static int xpcs_soft_reset(struct mdio_xpcs_args *xpcs, int dev)
+{
+ int ret;
+
+ ret = xpcs_write(xpcs, dev, MDIO_CTRL1, MDIO_CTRL1_RESET);
+ if (ret < 0)
+ return ret;
+
+ return xpcs_poll_reset(xpcs, dev);
+}
+
+#define xpcs_warn(__xpcs, __state, __args...) \
+({ \
+ if ((__state)->link) \
+ dev_warn(&(__xpcs)->bus->dev, ##__args); \
+})
+
+static int xpcs_read_fault(struct mdio_xpcs_args *xpcs,
+ struct phylink_link_state *state)
+{
+ int ret;
+
+ ret = xpcs_read(xpcs, MDIO_MMD_PCS, MDIO_STAT1);
+ if (ret < 0)
+ return ret;
+
+ if (ret & MDIO_STAT1_FAULT) {
+ xpcs_warn(xpcs, state, "Link fault condition detected!\n");
+ return -EFAULT;
+ }
+
+ ret = xpcs_read(xpcs, MDIO_MMD_PCS, MDIO_STAT2);
+ if (ret < 0)
+ return ret;
+
+ if (ret & MDIO_STAT2_RXFAULT)
+ xpcs_warn(xpcs, state, "Receiver fault detected!\n");
+ if (ret & MDIO_STAT2_TXFAULT)
+ xpcs_warn(xpcs, state, "Transmitter fault detected!\n");
+
+ ret = xpcs_read_vendor(xpcs, MDIO_MMD_PCS, DW_VR_XS_PCS_DIG_STS);
+ if (ret < 0)
+ return ret;
+
+ if (ret & DW_RXFIFO_ERR) {
+ xpcs_warn(xpcs, state, "FIFO fault condition detected!\n");
+ return -EFAULT;
+ }
+
+ ret = xpcs_read(xpcs, MDIO_MMD_PCS, MDIO_PCS_10GBRT_STAT1);
+ if (ret < 0)
+ return ret;
+
+ if (!(ret & MDIO_PCS_10GBRT_STAT1_BLKLK))
+ xpcs_warn(xpcs, state, "Link is not locked!\n");
+
+ ret = xpcs_read(xpcs, MDIO_MMD_PCS, MDIO_PCS_10GBRT_STAT2);
+ if (ret < 0)
+ return ret;
+
+ if (ret & MDIO_PCS_10GBRT_STAT2_ERR)
+ xpcs_warn(xpcs, state, "Link has errors!\n");
+
+ return 0;
+}
+
+static int xpcs_read_link(struct mdio_xpcs_args *xpcs, bool an)
+{
+ bool link = true;
+ int ret;
+
+ ret = xpcs_read(xpcs, MDIO_MMD_PCS, MDIO_STAT1);
+ if (ret < 0)
+ return ret;
+
+ if (!(ret & MDIO_STAT1_LSTATUS))
+ link = false;
+
+ if (an) {
+ ret = xpcs_read(xpcs, MDIO_MMD_AN, MDIO_STAT1);
+ if (ret < 0)
+ return ret;
+
+ if (!(ret & MDIO_STAT1_LSTATUS))
+ link = false;
+ }
+
+ return link;
+}
+
+static int xpcs_get_max_usxgmii_speed(const unsigned long *supported)
+{
+ int max = SPEED_UNKNOWN;
+
+ if (phylink_test(supported, 1000baseKX_Full))
+ max = SPEED_1000;
+ if (phylink_test(supported, 2500baseX_Full))
+ max = SPEED_2500;
+ if (phylink_test(supported, 10000baseKX4_Full))
+ max = SPEED_10000;
+ if (phylink_test(supported, 10000baseKR_Full))
+ max = SPEED_10000;
+
+ return max;
+}
+
+static int xpcs_config_usxgmii(struct mdio_xpcs_args *xpcs, int speed)
+{
+ int ret, speed_sel;
+
+ switch (speed) {
+ case SPEED_10:
+ speed_sel = DW_USXGMII_10;
+ break;
+ case SPEED_100:
+ speed_sel = DW_USXGMII_100;
+ break;
+ case SPEED_1000:
+ speed_sel = DW_USXGMII_1000;
+ break;
+ case SPEED_2500:
+ speed_sel = DW_USXGMII_2500;
+ break;
+ case SPEED_5000:
+ speed_sel = DW_USXGMII_5000;
+ break;
+ case SPEED_10000:
+ speed_sel = DW_USXGMII_10000;
+ break;
+ default:
+ /* Nothing to do here */
+ return -EINVAL;
+ }
+
+ ret = xpcs_read_vpcs(xpcs, MDIO_CTRL1);
+ if (ret < 0)
+ return ret;
+
+ ret = xpcs_write_vpcs(xpcs, MDIO_CTRL1, ret | DW_USXGMII_EN);
+ if (ret < 0)
+ return ret;
+
+ ret = xpcs_read(xpcs, MDIO_MMD_VEND2, MDIO_CTRL1);
+ if (ret < 0)
+ return ret;
+
+ ret &= ~DW_USXGMII_SS_MASK;
+ ret |= speed_sel | DW_USXGMII_FULL;
+
+ ret = xpcs_write(xpcs, MDIO_MMD_VEND2, MDIO_CTRL1, ret);
+ if (ret < 0)
+ return ret;
+
+ ret = xpcs_read_vpcs(xpcs, MDIO_CTRL1);
+ if (ret < 0)
+ return ret;
+
+ return xpcs_write_vpcs(xpcs, MDIO_CTRL1, ret | DW_USXGMII_RST);
+}
+
+static int xpcs_config_aneg_c73(struct mdio_xpcs_args *xpcs)
+{
+ int ret, adv;
+
+ /* By default, in USXGMII mode XPCS operates at 10G baud and
+ * replicates data to achieve lower speeds. Hereby, in this
+ * default configuration we need to advertise all supported
+ * modes and not only the ones we want to use.
+ */
+
+ /* SR_AN_ADV3 */
+ adv = 0;
+ if (phylink_test(xpcs->supported, 2500baseX_Full))
+ adv |= DW_C73_2500KX;
+
+ /* TODO: 5000baseKR */
+
+ ret = xpcs_write(xpcs, MDIO_MMD_AN, DW_SR_AN_ADV3, adv);
+ if (ret < 0)
+ return ret;
+
+ /* SR_AN_ADV2 */
+ adv = 0;
+ if (phylink_test(xpcs->supported, 1000baseKX_Full))
+ adv |= DW_C73_1000KX;
+ if (phylink_test(xpcs->supported, 10000baseKX4_Full))
+ adv |= DW_C73_10000KX4;
+ if (phylink_test(xpcs->supported, 10000baseKR_Full))
+ adv |= DW_C73_10000KR;
+
+ ret = xpcs_write(xpcs, MDIO_MMD_AN, DW_SR_AN_ADV2, adv);
+ if (ret < 0)
+ return ret;
+
+ /* SR_AN_ADV1 */
+ adv = DW_C73_AN_ADV_SF;
+ if (phylink_test(xpcs->supported, Pause))
+ adv |= DW_C73_PAUSE;
+ if (phylink_test(xpcs->supported, Asym_Pause))
+ adv |= DW_C73_ASYM_PAUSE;
+
+ return xpcs_write(xpcs, MDIO_MMD_AN, DW_SR_AN_ADV1, adv);
+}
+
+static int xpcs_config_aneg(struct mdio_xpcs_args *xpcs)
+{
+ int ret;
+
+ ret = xpcs_config_aneg_c73(xpcs);
+ if (ret < 0)
+ return ret;
+
+ ret = xpcs_read(xpcs, MDIO_MMD_AN, MDIO_CTRL1);
+ if (ret < 0)
+ return ret;
+
+ ret |= MDIO_AN_CTRL1_ENABLE | MDIO_AN_CTRL1_RESTART;
+
+ return xpcs_write(xpcs, MDIO_MMD_AN, MDIO_CTRL1, ret);
+}
+
+static int xpcs_aneg_done(struct mdio_xpcs_args *xpcs,
+ struct phylink_link_state *state)
+{
+ int ret;
+
+ ret = xpcs_read(xpcs, MDIO_MMD_AN, MDIO_STAT1);
+ if (ret < 0)
+ return ret;
+
+ if (ret & MDIO_AN_STAT1_COMPLETE) {
+ ret = xpcs_read(xpcs, MDIO_MMD_AN, DW_SR_AN_LP_ABL1);
+ if (ret < 0)
+ return ret;
+
+ /* Check if Aneg outcome is valid */
+ if (!(ret & DW_C73_AN_ADV_SF))
+ return 0;
+
+ return 1;
+ }
+
+ return 0;
+}
+
+static int xpcs_read_lpa(struct mdio_xpcs_args *xpcs,
+ struct phylink_link_state *state)
+{
+ int ret;
+
+ ret = xpcs_read(xpcs, MDIO_MMD_AN, MDIO_STAT1);
+ if (ret < 0)
+ return ret;
+
+ if (!(ret & MDIO_AN_STAT1_LPABLE)) {
+ phylink_clear(state->lp_advertising, Autoneg);
+ return 0;
+ }
+
+ phylink_set(state->lp_advertising, Autoneg);
+
+ /* Clause 73 outcome */
+ ret = xpcs_read(xpcs, MDIO_MMD_AN, DW_SR_AN_LP_ABL3);
+ if (ret < 0)
+ return ret;
+
+ if (ret & DW_C73_2500KX)
+ phylink_set(state->lp_advertising, 2500baseX_Full);
+
+ ret = xpcs_read(xpcs, MDIO_MMD_AN, DW_SR_AN_LP_ABL2);
+ if (ret < 0)
+ return ret;
+
+ if (ret & DW_C73_1000KX)
+ phylink_set(state->lp_advertising, 1000baseKX_Full);
+ if (ret & DW_C73_10000KX4)
+ phylink_set(state->lp_advertising, 10000baseKX4_Full);
+ if (ret & DW_C73_10000KR)
+ phylink_set(state->lp_advertising, 10000baseKR_Full);
+
+ ret = xpcs_read(xpcs, MDIO_MMD_AN, DW_SR_AN_LP_ABL1);
+ if (ret < 0)
+ return ret;
+
+ if (ret & DW_C73_PAUSE)
+ phylink_set(state->lp_advertising, Pause);
+ if (ret & DW_C73_ASYM_PAUSE)
+ phylink_set(state->lp_advertising, Asym_Pause);
+
+ linkmode_and(state->lp_advertising, state->lp_advertising,
+ state->advertising);
+ return 0;
+}
+
+static void xpcs_resolve_lpa(struct mdio_xpcs_args *xpcs,
+ struct phylink_link_state *state)
+{
+ int max_speed = xpcs_get_max_usxgmii_speed(state->lp_advertising);
+
+ state->pause = MLO_PAUSE_TX | MLO_PAUSE_RX;
+ state->speed = max_speed;
+ state->duplex = DUPLEX_FULL;
+}
+
+static void xpcs_resolve_pma(struct mdio_xpcs_args *xpcs,
+ struct phylink_link_state *state)
+{
+ state->pause = MLO_PAUSE_TX | MLO_PAUSE_RX;
+ state->duplex = DUPLEX_FULL;
+
+ switch (state->interface) {
+ case PHY_INTERFACE_MODE_10GKR:
+ state->speed = SPEED_10000;
+ break;
+ default:
+ state->speed = SPEED_UNKNOWN;
+ break;
+ }
+}
+
+static int xpcs_validate(struct mdio_xpcs_args *xpcs,
+ unsigned long *supported,
+ struct phylink_link_state *state)
+{
+ linkmode_and(supported, supported, xpcs->supported);
+ linkmode_and(state->advertising, state->advertising, xpcs->supported);
+ return 0;
+}
+
+static int xpcs_config(struct mdio_xpcs_args *xpcs,
+ const struct phylink_link_state *state)
+{
+ int ret;
+
+ if (state->an_enabled) {
+ ret = xpcs_config_aneg(xpcs);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static int xpcs_get_state(struct mdio_xpcs_args *xpcs,
+ struct phylink_link_state *state)
+{
+ int ret;
+
+ /* Link needs to be read first ... */
+ state->link = xpcs_read_link(xpcs, state->an_enabled) > 0 ? 1 : 0;
+
+ /* ... and then we check the faults. */
+ ret = xpcs_read_fault(xpcs, state);
+ if (ret) {
+ ret = xpcs_soft_reset(xpcs, MDIO_MMD_PCS);
+ if (ret)
+ return ret;
+
+ state->link = 0;
+
+ return xpcs_config(xpcs, state);
+ }
+
+ if (state->link && state->an_enabled && xpcs_aneg_done(xpcs, state)) {
+ state->an_complete = true;
+ xpcs_read_lpa(xpcs, state);
+ xpcs_resolve_lpa(xpcs, state);
+ } else if (state->link) {
+ xpcs_resolve_pma(xpcs, state);
+ }
+
+ return 0;
+}
+
+static int xpcs_link_up(struct mdio_xpcs_args *xpcs, int speed,
+ phy_interface_t interface)
+{
+ if (interface == PHY_INTERFACE_MODE_USXGMII)
+ return xpcs_config_usxgmii(xpcs, speed);
+
+ return 0;
+}
+
+static u32 xpcs_get_id(struct mdio_xpcs_args *xpcs)
+{
+ int ret;
+ u32 id;
+
+ ret = xpcs_read(xpcs, MDIO_MMD_PCS, MII_PHYSID1);
+ if (ret < 0)
+ return 0xffffffff;
+
+ id = ret << 16;
+
+ ret = xpcs_read(xpcs, MDIO_MMD_PCS, MII_PHYSID2);
+ if (ret < 0)
+ return 0xffffffff;
+
+ return id | ret;
+}
+
+static bool xpcs_check_features(struct mdio_xpcs_args *xpcs,
+ struct xpcs_id *match,
+ phy_interface_t interface)
+{
+ int i;
+
+ for (i = 0; match->interface[i] != PHY_INTERFACE_MODE_MAX; i++) {
+ if (match->interface[i] == interface)
+ break;
+ }
+
+ if (match->interface[i] == PHY_INTERFACE_MODE_MAX)
+ return false;
+
+ for (i = 0; match->supported[i] != __ETHTOOL_LINK_MODE_MASK_NBITS; i++)
+ set_bit(match->supported[i], xpcs->supported);
+
+ return true;
+}
+
+static int xpcs_probe(struct mdio_xpcs_args *xpcs, phy_interface_t interface)
+{
+ u32 xpcs_id = xpcs_get_id(xpcs);
+ struct xpcs_id *match = NULL;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(xpcs_id_list); i++) {
+ struct xpcs_id *entry = &xpcs_id_list[i];
+
+ if ((xpcs_id & entry->mask) == entry->id) {
+ match = entry;
+
+ if (xpcs_check_features(xpcs, match, interface))
+ return 0;
+ }
+ }
+
+ return -ENODEV;
+}
+
+static struct mdio_xpcs_ops xpcs_ops = {
+ .validate = xpcs_validate,
+ .config = xpcs_config,
+ .get_state = xpcs_get_state,
+ .link_up = xpcs_link_up,
+ .probe = xpcs_probe,
+};
+
+struct mdio_xpcs_ops *mdio_xpcs_get_ops(void)
+{
+ return &xpcs_ops;
+}
+EXPORT_SYMBOL_GPL(mdio_xpcs_get_ops);
+
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mdio-xpcs.h b/include/linux/mdio-xpcs.h
new file mode 100644
index 000000000000..9a841aa5982d
--- /dev/null
+++ b/include/linux/mdio-xpcs.h
@@ -0,0 +1,41 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2020 Synopsys, Inc. and/or its affiliates.
+ * Synopsys DesignWare XPCS helpers
+ */
+
+#ifndef __LINUX_MDIO_XPCS_H
+#define __LINUX_MDIO_XPCS_H
+
+#include <linux/phy.h>
+#include <linux/phylink.h>
+
+struct mdio_xpcs_args {
+ __ETHTOOL_DECLARE_LINK_MODE_MASK(supported);
+ struct mii_bus *bus;
+ int addr;
+};
+
+struct mdio_xpcs_ops {
+ int (*validate)(struct mdio_xpcs_args *xpcs,
+ unsigned long *supported,
+ struct phylink_link_state *state);
+ int (*config)(struct mdio_xpcs_args *xpcs,
+ const struct phylink_link_state *state);
+ int (*get_state)(struct mdio_xpcs_args *xpcs,
+ struct phylink_link_state *state);
+ int (*link_up)(struct mdio_xpcs_args *xpcs, int speed,
+ phy_interface_t interface);
+ int (*probe)(struct mdio_xpcs_args *xpcs, phy_interface_t interface);
+};
+
+#if IS_ENABLED(CONFIG_MDIO_XPCS)
+struct mdio_xpcs_ops *mdio_xpcs_get_ops(void);
+#else
+static inline struct mdio_xpcs_ops *mdio_xpcs_get_ops(void)
+{
+ return NULL;
+}
+#endif
+
+#endif /* __LINUX_MDIO_XPCS_H */
--
2.7.4


2020-06-05 17:15:04

by Russell King (Oracle)

[permalink] [raw]
Subject: Re: [PATCH net-next 7/8] net: phy: Add Synopsys DesignWare XPCS MDIO module

Hi Jose,

I just tripped over a bug while grepping for something else and
reading a bit of this driver:

On Mon, Mar 09, 2020 at 09:36:26AM +0100, Jose Abreu wrote:
> +static int xpcs_read_lpa(struct mdio_xpcs_args *xpcs,
> + struct phylink_link_state *state)
> +{
> + int ret;
> +
> + ret = xpcs_read(xpcs, MDIO_MMD_AN, MDIO_STAT1);
> + if (ret < 0)
> + return ret;
> +
> + if (!(ret & MDIO_AN_STAT1_LPABLE)) {
> + phylink_clear(state->lp_advertising, Autoneg);
> + return 0;
> + }
> +
> + phylink_set(state->lp_advertising, Autoneg);
> +
> + /* Clause 73 outcome */
> + ret = xpcs_read(xpcs, MDIO_MMD_AN, DW_SR_AN_LP_ABL3);
> + if (ret < 0)
> + return ret;
> +
> + if (ret & DW_C73_2500KX)
> + phylink_set(state->lp_advertising, 2500baseX_Full);
> +
> + ret = xpcs_read(xpcs, MDIO_MMD_AN, DW_SR_AN_LP_ABL2);
> + if (ret < 0)
> + return ret;
> +
> + if (ret & DW_C73_1000KX)
> + phylink_set(state->lp_advertising, 1000baseKX_Full);
> + if (ret & DW_C73_10000KX4)
> + phylink_set(state->lp_advertising, 10000baseKX4_Full);
> + if (ret & DW_C73_10000KR)
> + phylink_set(state->lp_advertising, 10000baseKR_Full);
> +
> + ret = xpcs_read(xpcs, MDIO_MMD_AN, DW_SR_AN_LP_ABL1);
> + if (ret < 0)
> + return ret;
> +
> + if (ret & DW_C73_PAUSE)
> + phylink_set(state->lp_advertising, Pause);
> + if (ret & DW_C73_ASYM_PAUSE)
> + phylink_set(state->lp_advertising, Asym_Pause);
> +
> + linkmode_and(state->lp_advertising, state->lp_advertising,
> + state->advertising);

This is incorrect - you should not mask the link partner's advertisement
with our advertisement like this; consider the table in 802.3 for
resolving the pause modes, where simply doing a bitwise-and operation
between the two advertisements would severely restrict the resulting
resolution to either symmetric pause or nothing at all.

You want to do this when you resolve the speed, but only _temporarily_
in order to resolve the speed - you do not want to write back the
result to state->lp_advertising.

You may wish to fix that.

Thanks.

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC for 0.8m (est. 1762m) line in suburbia: sync at 13.1Mbps down 424kbps up

2020-06-09 14:29:30

by Jose Abreu

[permalink] [raw]
Subject: RE: [PATCH net-next 7/8] net: phy: Add Synopsys DesignWare XPCS MDIO module

From: Russell King - ARM Linux admin <[email protected]>
Date: Jun/05/2020, 18:10:34 (UTC+00:00)

> This is incorrect - you should not mask the link partner's advertisement
> with our advertisement like this; consider the table in 802.3 for
> resolving the pause modes, where simply doing a bitwise-and operation
> between the two advertisements would severely restrict the resulting
> resolution to either symmetric pause or nothing at all.
>
> You want to do this when you resolve the speed, but only _temporarily_
> in order to resolve the speed - you do not want to write back the
> result to state->lp_advertising.

Thanks for bringing this up. Indeed I believe I did this in order to
resolve the speed, as you said.

> You may wish to fix that.

Added to the list of my pending fixes. Thanks!

---
Thanks,
Jose Miguel Abreu