This patch series applies to Cadence Ethernet MAC Controller.
The first patch in this series is related to the patch that converts the
driver to phylink interface in net-next "net: macb: convert to phylink".
Next patch is for adding support for C45 interface and the final patch
adds 10G MAC support.
The patch sequences are detailed as below
1. 0001-net-macb-fix-for-fixed-link-mode
This patch fix the issue with fixed-link mode in macb phylink interface.
In fixed-link we don't need to parse phandle because it's better handled
by phylink_of_phy_connect()
2. 0002-net-macb-add-support-for-C45-MDIO-read-write
This patch is to support C45 interface to PHY. This upgrade is downward compatible.
All versions of the MAC (old and new) using the GPL driver support both Clause 22 and
Clause 45 operation. Whether the access is in Clause 22 or Clause 45 format depends
on the data pattern written to the PHY management register.
3. 0003-net-macb-add-support-for-high-speed-interface
This patch add support for 10G fixed mode.
Changes since v1:
1. phy_node reference count handling in patch 0001-net-macb-fix-for-fixed-link-mode
2. Using fixed value for HS_MAC_SPEED_x
Thanks,
Milind Parab
Milind Parab (3):
net: macb: fix for fixed-link mode
net: macb: add support for C45 MDIO read/write
net: macb: add support for high speed interface
drivers/net/ethernet/cadence/macb.h | 65 ++++++-
drivers/net/ethernet/cadence/macb_main.c | 224 +++++++++++++++++++----
2 files changed, 247 insertions(+), 42 deletions(-)
--
2.17.1
This patch fix the issue with fixed link. With fixed-link
device opening fails due to macb_phylink_connect not
handling fixed-link mode, in which case no MAC-PHY connection
is needed and phylink_connect return success (0), however
in current driver attempt is made to search and connect to
PHY even for fixed-link.
Signed-off-by: Milind Parab <[email protected]>
---
drivers/net/ethernet/cadence/macb_main.c | 25 +++++++++++++++---------
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 9c767ee252ac..8c1812f39927 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -611,21 +611,25 @@ static const struct phylink_mac_ops macb_phylink_ops = {
.mac_link_up = macb_mac_link_up,
};
+static bool macb_phy_handle_exists(struct device_node *dn)
+{
+ dn = of_parse_phandle(dn, "phy-handle", 0);
+ of_node_put(dn);
+ return dn != NULL;
+}
+
+
static int macb_phylink_connect(struct macb *bp)
{
struct net_device *dev = bp->dev;
struct phy_device *phydev;
+ struct device_node *dn = bp->pdev->dev.of_node;
int ret;
- if (bp->pdev->dev.of_node &&
- of_parse_phandle(bp->pdev->dev.of_node, "phy-handle", 0)) {
- ret = phylink_of_phy_connect(bp->phylink, bp->pdev->dev.of_node,
- 0);
- if (ret) {
- netdev_err(dev, "Could not attach PHY (%d)\n", ret);
- return ret;
- }
- } else {
+ if (dn)
+ ret = phylink_of_phy_connect(bp->phylink, dn, 0);
+
+ if (!dn || (ret && !macb_phy_handle_exists(dn))) {
phydev = phy_find_first(bp->mii_bus);
if (!phydev) {
netdev_err(dev, "no PHY found\n");
@@ -638,6 +642,9 @@ static int macb_phylink_connect(struct macb *bp)
netdev_err(dev, "Could not attach to PHY (%d)\n", ret);
return ret;
}
+ } else if (ret) {
+ netdev_err(dev, "Could not attach PHY (%d)\n", ret);
+ return ret;
}
phylink_start(bp->phylink);
--
2.17.1
This patch modify MDIO read/write functions to support
communication with C45 PHY.
Signed-off-by: Milind Parab <[email protected]>
---
drivers/net/ethernet/cadence/macb.h | 15 ++++--
drivers/net/ethernet/cadence/macb_main.c | 61 +++++++++++++++++++-----
2 files changed, 61 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 19fe4f4867c7..dbf7070fcdba 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -630,10 +630,17 @@
#define GEM_CLK_DIV96 5
/* Constants for MAN register */
-#define MACB_MAN_SOF 1
-#define MACB_MAN_WRITE 1
-#define MACB_MAN_READ 2
-#define MACB_MAN_CODE 2
+#define MACB_MAN_C22_SOF 1
+#define MACB_MAN_C22_WRITE 1
+#define MACB_MAN_C22_READ 2
+#define MACB_MAN_C22_CODE 2
+
+#define MACB_MAN_C45_SOF 0
+#define MACB_MAN_C45_ADDR 0
+#define MACB_MAN_C45_WRITE 1
+#define MACB_MAN_C45_POST_READ_INCR 2
+#define MACB_MAN_C45_READ 3
+#define MACB_MAN_C45_CODE 2
/* Capability mask bits */
#define MACB_CAPS_ISR_CLEAR_ON_WRITE 0x00000001
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 8c1812f39927..ced32d2a85e1 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -337,11 +337,30 @@ static int macb_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
if (status < 0)
goto mdio_read_exit;
- macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
- | MACB_BF(RW, MACB_MAN_READ)
- | MACB_BF(PHYA, mii_id)
- | MACB_BF(REGA, regnum)
- | MACB_BF(CODE, MACB_MAN_CODE)));
+ if (regnum & MII_ADDR_C45) {
+ macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C45_SOF)
+ | MACB_BF(RW, MACB_MAN_C45_ADDR)
+ | MACB_BF(PHYA, mii_id)
+ | MACB_BF(REGA, (regnum >> 16) & 0x1F)
+ | MACB_BF(DATA, regnum & 0xFFFF)
+ | MACB_BF(CODE, MACB_MAN_C45_CODE)));
+
+ status = macb_mdio_wait_for_idle(bp);
+ if (status < 0)
+ goto mdio_read_exit;
+
+ macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C45_SOF)
+ | MACB_BF(RW, MACB_MAN_C45_READ)
+ | MACB_BF(PHYA, mii_id)
+ | MACB_BF(REGA, (regnum >> 16) & 0x1F)
+ | MACB_BF(CODE, MACB_MAN_C45_CODE)));
+ } else {
+ macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C22_SOF)
+ | MACB_BF(RW, MACB_MAN_C22_READ)
+ | MACB_BF(PHYA, mii_id)
+ | MACB_BF(REGA, regnum)
+ | MACB_BF(CODE, MACB_MAN_C22_CODE)));
+ }
status = macb_mdio_wait_for_idle(bp);
if (status < 0)
@@ -370,12 +389,32 @@ static int macb_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
if (status < 0)
goto mdio_write_exit;
- macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
- | MACB_BF(RW, MACB_MAN_WRITE)
- | MACB_BF(PHYA, mii_id)
- | MACB_BF(REGA, regnum)
- | MACB_BF(CODE, MACB_MAN_CODE)
- | MACB_BF(DATA, value)));
+ if (regnum & MII_ADDR_C45) {
+ macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C45_SOF)
+ | MACB_BF(RW, MACB_MAN_C45_ADDR)
+ | MACB_BF(PHYA, mii_id)
+ | MACB_BF(REGA, (regnum >> 16) & 0x1F)
+ | MACB_BF(DATA, regnum & 0xFFFF)
+ | MACB_BF(CODE, MACB_MAN_C45_CODE)));
+
+ status = macb_mdio_wait_for_idle(bp);
+ if (status < 0)
+ goto mdio_write_exit;
+
+ macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C45_SOF)
+ | MACB_BF(RW, MACB_MAN_C45_WRITE)
+ | MACB_BF(PHYA, mii_id)
+ | MACB_BF(REGA, (regnum >> 16) & 0x1F)
+ | MACB_BF(CODE, MACB_MAN_C45_CODE)
+ | MACB_BF(DATA, value)));
+ } else {
+ macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C22_SOF)
+ | MACB_BF(RW, MACB_MAN_C22_WRITE)
+ | MACB_BF(PHYA, mii_id)
+ | MACB_BF(REGA, regnum)
+ | MACB_BF(CODE, MACB_MAN_C22_CODE)
+ | MACB_BF(DATA, value)));
+ }
status = macb_mdio_wait_for_idle(bp);
if (status < 0)
--
2.17.1
This patch add support for high speed USXGMII PCS and 10G
speed in Cadence ethernet controller driver.
Signed-off-by: Milind Parab <[email protected]>
---
drivers/net/ethernet/cadence/macb.h | 50 ++++++++
drivers/net/ethernet/cadence/macb_main.c | 138 ++++++++++++++++++++---
2 files changed, 170 insertions(+), 18 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index dbf7070fcdba..b731807d1c49 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -76,10 +76,12 @@
#define MACB_RBQPH 0x04D4
/* GEM register offsets. */
+#define GEM_NCR 0x0000 /* Network Control */
#define GEM_NCFGR 0x0004 /* Network Config */
#define GEM_USRIO 0x000c /* User IO */
#define GEM_DMACFG 0x0010 /* DMA Configuration */
#define GEM_JML 0x0048 /* Jumbo Max Length */
+#define GEM_HS_MAC_CONFIG 0x0050 /* GEM high speed config */
#define GEM_HRB 0x0080 /* Hash Bottom */
#define GEM_HRT 0x0084 /* Hash Top */
#define GEM_SA1B 0x0088 /* Specific1 Bottom */
@@ -164,6 +166,9 @@
#define GEM_DCFG7 0x0298 /* Design Config 7 */
#define GEM_DCFG8 0x029C /* Design Config 8 */
#define GEM_DCFG10 0x02A4 /* Design Config 10 */
+#define GEM_DCFG12 0x02AC /* Design Config 12 */
+#define GEM_USX_CONTROL 0x0A80 /* USXGMII control register */
+#define GEM_USX_STATUS 0x0A88 /* USXGMII status register */
#define GEM_TXBDCTRL 0x04cc /* TX Buffer Descriptor control register */
#define GEM_RXBDCTRL 0x04d0 /* RX Buffer Descriptor control register */
@@ -270,11 +275,19 @@
#define MACB_IRXFCS_OFFSET 19
#define MACB_IRXFCS_SIZE 1
+/* GEM specific NCR bitfields. */
+#define GEM_ENABLE_HS_MAC_OFFSET 31
+#define GEM_ENABLE_HS_MAC_SIZE 1
+
/* GEM specific NCFGR bitfields. */
+#define GEM_FD_OFFSET 1 /* Full duplex */
+#define GEM_FD_SIZE 1
#define GEM_GBE_OFFSET 10 /* Gigabit mode enable */
#define GEM_GBE_SIZE 1
#define GEM_PCSSEL_OFFSET 11
#define GEM_PCSSEL_SIZE 1
+#define GEM_PAE_OFFSET 13 /* Pause enable */
+#define GEM_PAE_SIZE 1
#define GEM_CLK_OFFSET 18 /* MDC clock division */
#define GEM_CLK_SIZE 3
#define GEM_DBW_OFFSET 21 /* Data bus width */
@@ -455,11 +468,17 @@
#define MACB_REV_OFFSET 0
#define MACB_REV_SIZE 16
+/* Bitfield in HS_MAC_CONFIG */
+#define GEM_HS_MAC_SPEED_OFFSET 0
+#define GEM_HS_MAC_SPEED_SIZE 3
+
/* Bitfields in DCFG1. */
#define GEM_IRQCOR_OFFSET 23
#define GEM_IRQCOR_SIZE 1
#define GEM_DBWDEF_OFFSET 25
#define GEM_DBWDEF_SIZE 3
+#define GEM_NO_PCS_OFFSET 0
+#define GEM_NO_PCS_SIZE 1
/* Bitfields in DCFG2. */
#define GEM_RX_PKT_BUFF_OFFSET 20
@@ -494,6 +513,34 @@
#define GEM_RXBD_RDBUFF_OFFSET 8
#define GEM_RXBD_RDBUFF_SIZE 4
+/* Bitfields in DCFG12. */
+#define GEM_HIGH_SPEED_OFFSET 26
+#define GEM_HIGH_SPEED_SIZE 1
+
+/* Bitfields in USX_CONTROL. */
+#define GEM_USX_CTRL_SPEED_OFFSET 14
+#define GEM_USX_CTRL_SPEED_SIZE 3
+#define GEM_SERDES_RATE_OFFSET 12
+#define GEM_SERDES_RATE_SIZE 2
+#define GEM_RX_SCR_BYPASS_OFFSET 9
+#define GEM_RX_SCR_BYPASS_SIZE 1
+#define GEM_TX_SCR_BYPASS_OFFSET 8
+#define GEM_TX_SCR_BYPASS_SIZE 1
+#define GEM_RX_SYNC_RESET_OFFSET 2
+#define GEM_RX_SYNC_RESET_SIZE 1
+#define GEM_TX_EN_OFFSET 1
+#define GEM_TX_EN_SIZE 1
+#define GEM_SIGNAL_OK_OFFSET 0
+#define GEM_SIGNAL_OK_SIZE 1
+
+/* Bitfields in USX_STATUS. */
+#define GEM_USX_TX_FAULT_OFFSET 28
+#define GEM_USX_TX_FAULT_SIZE 1
+#define GEM_USX_RX_FAULT_OFFSET 27
+#define GEM_USX_RX_FAULT_SIZE 1
+#define GEM_USX_BLOCK_LOCK_OFFSET 0
+#define GEM_USX_BLOCK_LOCK_SIZE 1
+
/* Bitfields in TISUBN */
#define GEM_SUBNSINCR_OFFSET 0
#define GEM_SUBNSINCRL_OFFSET 24
@@ -656,6 +703,8 @@
#define MACB_CAPS_GIGABIT_MODE_AVAILABLE 0x20000000
#define MACB_CAPS_SG_DISABLED 0x40000000
#define MACB_CAPS_MACB_IS_GEM 0x80000000
+#define MACB_CAPS_PCS 0x01000000
+#define MACB_CAPS_HIGH_SPEED 0x02000000
/* LSO settings */
#define MACB_LSO_UFO_ENABLE 0x01
@@ -724,6 +773,7 @@
})
#define MACB_READ_NSR(bp) macb_readl(bp, NSR)
+#define GEM_READ_USX_STATUS(bp) gem_readl(bp, USX_STATUS)
/* struct macb_dma_desc - Hardware DMA descriptor
* @addr: DMA address of data buffer
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index ced32d2a85e1..5963a60d54b9 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -81,6 +81,14 @@ struct sifive_fu540_macb_mgmt {
#define MACB_WOL_HAS_MAGIC_PACKET (0x1 << 0)
#define MACB_WOL_ENABLED (0x1 << 1)
+#define HS_MAC_SPEED_100M 0
+#define HS_MAC_SPEED_1000M 1
+#define HS_MAC_SPEED_2500M 2
+#define HS_MAC_SPEED_5000M 3
+#define HS_MAC_SPEED_10000M 4
+
+#define MACB_SERDES_RATE_10G 1
+
/* Graceful stop timeouts in us. We should allow up to
* 1 frame time (10 Mbits/s, full-duplex, ignoring collisions)
*/
@@ -90,6 +98,8 @@ struct sifive_fu540_macb_mgmt {
#define MACB_MDIO_TIMEOUT 1000000 /* in usecs */
+#define MACB_USX_BLOCK_LOCK_TIMEOUT 1000000 /* in usecs */
+
/* DMA buffer descriptor might be different size
* depends on hardware configuration:
*
@@ -506,6 +516,7 @@ static void macb_validate(struct phylink_config *config,
state->interface != PHY_INTERFACE_MODE_RMII &&
state->interface != PHY_INTERFACE_MODE_GMII &&
state->interface != PHY_INTERFACE_MODE_SGMII &&
+ state->interface != PHY_INTERFACE_MODE_USXGMII &&
!phy_interface_mode_is_rgmii(state->interface)) {
bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
return;
@@ -518,6 +529,13 @@ static void macb_validate(struct phylink_config *config,
return;
}
+ if (state->interface == PHY_INTERFACE_MODE_USXGMII &&
+ !(bp->caps & MACB_CAPS_HIGH_SPEED &&
+ bp->caps & MACB_CAPS_PCS)) {
+ bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
+ return;
+ }
+
phylink_set_port_modes(mask);
phylink_set(mask, Autoneg);
phylink_set(mask, Asym_Pause);
@@ -527,6 +545,22 @@ static void macb_validate(struct phylink_config *config,
phylink_set(mask, 100baseT_Half);
phylink_set(mask, 100baseT_Full);
+ if (bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE &&
+ (state->interface == PHY_INTERFACE_MODE_NA ||
+ state->interface == PHY_INTERFACE_MODE_USXGMII)) {
+ phylink_set(mask, 10000baseCR_Full);
+ phylink_set(mask, 10000baseER_Full);
+ phylink_set(mask, 10000baseKR_Full);
+ phylink_set(mask, 10000baseLR_Full);
+ phylink_set(mask, 10000baseLRM_Full);
+ phylink_set(mask, 10000baseSR_Full);
+ phylink_set(mask, 10000baseT_Full);
+ phylink_set(mask, 5000baseT_Full);
+ phylink_set(mask, 2500baseX_Full);
+ phylink_set(mask, 1000baseX_Full);
+ phylink_set(mask, 1000baseT_Full);
+ }
+
if (bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE &&
(state->interface == PHY_INTERFACE_MODE_NA ||
state->interface == PHY_INTERFACE_MODE_GMII ||
@@ -544,6 +578,60 @@ static void macb_validate(struct phylink_config *config,
__ETHTOOL_LINK_MODE_MASK_NBITS);
}
+static int gem_mac_usx_configure(struct macb *bp,
+ const struct phylink_link_state *state)
+{
+ u32 speed, config, val;
+ int ret;
+
+ val = gem_readl(bp, NCFGR);
+ val = GEM_BIT(PCSSEL) | (~GEM_BIT(SGMIIEN) & val);
+ if (state->pause & MLO_PAUSE_TX)
+ val |= GEM_BIT(PAE);
+ gem_writel(bp, NCFGR, val);
+ gem_writel(bp, NCR, gem_readl(bp, NCR) | GEM_BIT(ENABLE_HS_MAC));
+ gem_writel(bp, NCFGR, gem_readl(bp, NCFGR) | GEM_BIT(FD));
+ config = gem_readl(bp, USX_CONTROL);
+ config = GEM_BFINS(SERDES_RATE, MACB_SERDES_RATE_10G, config);
+ config &= ~GEM_BIT(TX_SCR_BYPASS);
+ config &= ~GEM_BIT(RX_SCR_BYPASS);
+ gem_writel(bp, USX_CONTROL, config | GEM_BIT(TX_EN));
+ config = gem_readl(bp, USX_CONTROL);
+ gem_writel(bp, USX_CONTROL, config | GEM_BIT(SIGNAL_OK));
+ ret = readx_poll_timeout(GEM_READ_USX_STATUS, bp, val,
+ val & GEM_BIT(USX_BLOCK_LOCK),
+ 1, MACB_USX_BLOCK_LOCK_TIMEOUT);
+ if (ret < 0) {
+ netdev_warn(bp->dev, "USXGMII block lock failed");
+ return -ETIMEDOUT;
+ }
+
+ switch (state->speed) {
+ case SPEED_10000:
+ speed = HS_MAC_SPEED_10000M;
+ break;
+ case SPEED_5000:
+ speed = HS_MAC_SPEED_5000M;
+ break;
+ case SPEED_2500:
+ speed = HS_MAC_SPEED_2500M;
+ break;
+ case SPEED_1000:
+ speed = HS_MAC_SPEED_1000M;
+ break;
+ default:
+ case SPEED_100:
+ speed = HS_MAC_SPEED_100M;
+ break;
+ }
+
+ gem_writel(bp, HS_MAC_CONFIG, GEM_BFINS(HS_MAC_SPEED, speed,
+ gem_readl(bp, HS_MAC_CONFIG)));
+ gem_writel(bp, USX_CONTROL, GEM_BFINS(USX_CTRL_SPEED, speed,
+ gem_readl(bp, USX_CONTROL)));
+ return 0;
+}
+
static void macb_mac_pcs_get_state(struct phylink_config *config,
struct phylink_link_state *state)
{
@@ -565,30 +653,39 @@ static void macb_mac_config(struct phylink_config *config, unsigned int mode,
spin_lock_irqsave(&bp->lock, flags);
- old_ctrl = ctrl = macb_or_gem_readl(bp, NCFGR);
+ if (bp->phy_interface == PHY_INTERFACE_MODE_USXGMII) {
+ if (gem_mac_usx_configure(bp, state) < 0) {
+ spin_unlock_irqrestore(&bp->lock, flags);
+ phylink_mac_change(bp->phylink, false);
+ return;
+ }
+ } else {
+ old_ctrl = ctrl = macb_or_gem_readl(bp, NCFGR);
- /* Clear all the bits we might set later */
- ctrl &= ~(GEM_BIT(GBE) | MACB_BIT(SPD) | MACB_BIT(FD) | MACB_BIT(PAE) |
- GEM_BIT(SGMIIEN) | GEM_BIT(PCSSEL));
+ /* Clear all the bits we might set later */
+ ctrl &= ~(GEM_BIT(GBE) | MACB_BIT(SPD) |
+ MACB_BIT(FD) | MACB_BIT(PAE) |
+ GEM_BIT(SGMIIEN) | GEM_BIT(PCSSEL));
- if (state->speed == SPEED_1000)
- ctrl |= GEM_BIT(GBE);
- else if (state->speed == SPEED_100)
- ctrl |= MACB_BIT(SPD);
+ if (state->speed == SPEED_1000)
+ ctrl |= GEM_BIT(GBE);
+ else if (state->speed == SPEED_100)
+ ctrl |= MACB_BIT(SPD);
- if (state->duplex)
- ctrl |= MACB_BIT(FD);
+ if (state->duplex)
+ ctrl |= MACB_BIT(FD);
- /* We do not support MLO_PAUSE_RX yet */
- if (state->pause & MLO_PAUSE_TX)
- ctrl |= MACB_BIT(PAE);
+ /* We do not support MLO_PAUSE_RX yet */
+ if (state->pause & MLO_PAUSE_TX)
+ ctrl |= MACB_BIT(PAE);
- if (state->interface == PHY_INTERFACE_MODE_SGMII)
- ctrl |= GEM_BIT(SGMIIEN) | GEM_BIT(PCSSEL);
+ if (state->interface == PHY_INTERFACE_MODE_SGMII)
+ ctrl |= GEM_BIT(SGMIIEN) | GEM_BIT(PCSSEL);
- /* Apply the new configuration, if any */
- if (old_ctrl ^ ctrl)
- macb_or_gem_writel(bp, NCFGR, ctrl);
+ /* Apply the new configuration, if any */
+ if (old_ctrl ^ ctrl)
+ macb_or_gem_writel(bp, NCFGR, ctrl);
+ }
bp->speed = state->speed;
@@ -3407,6 +3504,11 @@ static void macb_configure_caps(struct macb *bp,
dcfg = gem_readl(bp, DCFG1);
if (GEM_BFEXT(IRQCOR, dcfg) == 0)
bp->caps |= MACB_CAPS_ISR_CLEAR_ON_WRITE;
+ if (GEM_BFEXT(NO_PCS, dcfg) == 0)
+ bp->caps |= MACB_CAPS_PCS;
+ dcfg = gem_readl(bp, DCFG12);
+ if (GEM_BFEXT(HIGH_SPEED, dcfg) == 1)
+ bp->caps |= MACB_CAPS_HIGH_SPEED;
dcfg = gem_readl(bp, DCFG2);
if ((dcfg & (GEM_BIT(RX_PKT_BUFF) | GEM_BIT(TX_PKT_BUFF))) == 0)
bp->caps |= MACB_CAPS_FIFO_MODE;
--
2.17.1
On Fri, 13 Dec 2019 09:41:01 +0000, Milind Parab wrote:
> This patch fix the issue with fixed link. With fixed-link
> device opening fails due to macb_phylink_connect not
> handling fixed-link mode, in which case no MAC-PHY connection
> is needed and phylink_connect return success (0), however
> in current driver attempt is made to search and connect to
> PHY even for fixed-link.
>
> Signed-off-by: Milind Parab <[email protected]>
We'll wait to give a chance for Russell, Andrew and others to review,
but this patch looks like a fix and the other ones look like features.
You should post the fix separately so it's included in Linus'es tree
ASAP (mark the patch with [PATCH net]), and the rest of the patches can
wait for the next merge window (mark [PATCH net-next]). Fixes should
also have an appropriate Fixes tag pointing at the first commit where
the bug was present.
Corrected typo in email id of Nicolas Ferre.
>-----Original Message-----
>From: Milind Parab <[email protected]>
>Sent: Friday, December 13, 2019 3:10 PM
>To: [email protected]; [email protected];
>[email protected]; [email protected]; [email protected]
>Cc: [email protected]; [email protected]; [email protected];
>[email protected]; Dhananjay Vilasrao Kangude
><[email protected]>; [email protected]; [email protected];
>Parshuram Raju Thombare <[email protected]>; Milind Parab
><[email protected]>
>Subject: [PATCH v2 0/3] net: macb: fix for fixed link, support for c45 mdio and
>10G
>
>This patch series applies to Cadence Ethernet MAC Controller.
>The first patch in this series is related to the patch that converts the
>driver to phylink interface in net-next "net: macb: convert to phylink".
>Next patch is for adding support for C45 interface and the final patch
>adds 10G MAC support.
>
>The patch sequences are detailed as below
>
>1. 0001-net-macb-fix-for-fixed-link-mode
>This patch fix the issue with fixed-link mode in macb phylink interface.
>In fixed-link we don't need to parse phandle because it's better handled
>by phylink_of_phy_connect()
>
>2. 0002-net-macb-add-support-for-C45-MDIO-read-write
>This patch is to support C45 interface to PHY. This upgrade is downward
>compatible.
>All versions of the MAC (old and new) using the GPL driver support both Clause 22
>and
>Clause 45 operation. Whether the access is in Clause 22 or Clause 45 format
>depends
>on the data pattern written to the PHY management register.
>
>3. 0003-net-macb-add-support-for-high-speed-interface
>This patch add support for 10G fixed mode.
>
>Changes since v1:
>1. phy_node reference count handling in patch 0001-net-macb-fix-for-fixed-link-
>mode
>2. Using fixed value for HS_MAC_SPEED_x
>
>Thanks,
>Milind Parab
>
>
>Milind Parab (3):
> net: macb: fix for fixed-link mode
> net: macb: add support for C45 MDIO read/write
> net: macb: add support for high speed interface
>
> drivers/net/ethernet/cadence/macb.h | 65 ++++++-
> drivers/net/ethernet/cadence/macb_main.c | 224 +++++++++++++++++++----
> 2 files changed, 247 insertions(+), 42 deletions(-)
>
>--
>2.17.1
On Fri, Dec 13, 2019 at 09:41:51AM +0000, Milind Parab wrote:
> This patch modify MDIO read/write functions to support
> communication with C45 PHY.
>
> Signed-off-by: Milind Parab <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
Andrew
On Fri, Dec 13, 2019 at 09:42:57AM +0000, Milind Parab wrote:
> This patch add support for high speed USXGMII PCS and 10G
> speed in Cadence ethernet controller driver.
>
> Signed-off-by: Milind Parab <[email protected]>
> ---
> drivers/net/ethernet/cadence/macb.h | 50 ++++++++
> drivers/net/ethernet/cadence/macb_main.c | 138 ++++++++++++++++++++---
> 2 files changed, 170 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> index dbf7070fcdba..b731807d1c49 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -76,10 +76,12 @@
> #define MACB_RBQPH 0x04D4
>
> /* GEM register offsets. */
> +#define GEM_NCR 0x0000 /* Network Control */
> #define GEM_NCFGR 0x0004 /* Network Config */
> #define GEM_USRIO 0x000c /* User IO */
> #define GEM_DMACFG 0x0010 /* DMA Configuration */
> #define GEM_JML 0x0048 /* Jumbo Max Length */
> +#define GEM_HS_MAC_CONFIG 0x0050 /* GEM high speed config */
> #define GEM_HRB 0x0080 /* Hash Bottom */
> #define GEM_HRT 0x0084 /* Hash Top */
> #define GEM_SA1B 0x0088 /* Specific1 Bottom */
> @@ -164,6 +166,9 @@
> #define GEM_DCFG7 0x0298 /* Design Config 7 */
> #define GEM_DCFG8 0x029C /* Design Config 8 */
> #define GEM_DCFG10 0x02A4 /* Design Config 10 */
> +#define GEM_DCFG12 0x02AC /* Design Config 12 */
> +#define GEM_USX_CONTROL 0x0A80 /* USXGMII control register */
> +#define GEM_USX_STATUS 0x0A88 /* USXGMII status register */
>
> #define GEM_TXBDCTRL 0x04cc /* TX Buffer Descriptor control register */
> #define GEM_RXBDCTRL 0x04d0 /* RX Buffer Descriptor control register */
> @@ -270,11 +275,19 @@
> #define MACB_IRXFCS_OFFSET 19
> #define MACB_IRXFCS_SIZE 1
>
> +/* GEM specific NCR bitfields. */
> +#define GEM_ENABLE_HS_MAC_OFFSET 31
> +#define GEM_ENABLE_HS_MAC_SIZE 1
> +
> /* GEM specific NCFGR bitfields. */
> +#define GEM_FD_OFFSET 1 /* Full duplex */
> +#define GEM_FD_SIZE 1
> #define GEM_GBE_OFFSET 10 /* Gigabit mode enable */
> #define GEM_GBE_SIZE 1
> #define GEM_PCSSEL_OFFSET 11
> #define GEM_PCSSEL_SIZE 1
> +#define GEM_PAE_OFFSET 13 /* Pause enable */
> +#define GEM_PAE_SIZE 1
> #define GEM_CLK_OFFSET 18 /* MDC clock division */
> #define GEM_CLK_SIZE 3
> #define GEM_DBW_OFFSET 21 /* Data bus width */
> @@ -455,11 +468,17 @@
> #define MACB_REV_OFFSET 0
> #define MACB_REV_SIZE 16
>
> +/* Bitfield in HS_MAC_CONFIG */
> +#define GEM_HS_MAC_SPEED_OFFSET 0
> +#define GEM_HS_MAC_SPEED_SIZE 3
> +
> /* Bitfields in DCFG1. */
> #define GEM_IRQCOR_OFFSET 23
> #define GEM_IRQCOR_SIZE 1
> #define GEM_DBWDEF_OFFSET 25
> #define GEM_DBWDEF_SIZE 3
> +#define GEM_NO_PCS_OFFSET 0
> +#define GEM_NO_PCS_SIZE 1
>
> /* Bitfields in DCFG2. */
> #define GEM_RX_PKT_BUFF_OFFSET 20
> @@ -494,6 +513,34 @@
> #define GEM_RXBD_RDBUFF_OFFSET 8
> #define GEM_RXBD_RDBUFF_SIZE 4
>
> +/* Bitfields in DCFG12. */
> +#define GEM_HIGH_SPEED_OFFSET 26
> +#define GEM_HIGH_SPEED_SIZE 1
> +
> +/* Bitfields in USX_CONTROL. */
> +#define GEM_USX_CTRL_SPEED_OFFSET 14
> +#define GEM_USX_CTRL_SPEED_SIZE 3
> +#define GEM_SERDES_RATE_OFFSET 12
> +#define GEM_SERDES_RATE_SIZE 2
> +#define GEM_RX_SCR_BYPASS_OFFSET 9
> +#define GEM_RX_SCR_BYPASS_SIZE 1
> +#define GEM_TX_SCR_BYPASS_OFFSET 8
> +#define GEM_TX_SCR_BYPASS_SIZE 1
> +#define GEM_RX_SYNC_RESET_OFFSET 2
> +#define GEM_RX_SYNC_RESET_SIZE 1
> +#define GEM_TX_EN_OFFSET 1
> +#define GEM_TX_EN_SIZE 1
> +#define GEM_SIGNAL_OK_OFFSET 0
> +#define GEM_SIGNAL_OK_SIZE 1
> +
> +/* Bitfields in USX_STATUS. */
> +#define GEM_USX_TX_FAULT_OFFSET 28
> +#define GEM_USX_TX_FAULT_SIZE 1
> +#define GEM_USX_RX_FAULT_OFFSET 27
> +#define GEM_USX_RX_FAULT_SIZE 1
> +#define GEM_USX_BLOCK_LOCK_OFFSET 0
> +#define GEM_USX_BLOCK_LOCK_SIZE 1
> +
> /* Bitfields in TISUBN */
> #define GEM_SUBNSINCR_OFFSET 0
> #define GEM_SUBNSINCRL_OFFSET 24
> @@ -656,6 +703,8 @@
> #define MACB_CAPS_GIGABIT_MODE_AVAILABLE 0x20000000
> #define MACB_CAPS_SG_DISABLED 0x40000000
> #define MACB_CAPS_MACB_IS_GEM 0x80000000
> +#define MACB_CAPS_PCS 0x01000000
> +#define MACB_CAPS_HIGH_SPEED 0x02000000
>
> /* LSO settings */
> #define MACB_LSO_UFO_ENABLE 0x01
> @@ -724,6 +773,7 @@
> })
>
> #define MACB_READ_NSR(bp) macb_readl(bp, NSR)
> +#define GEM_READ_USX_STATUS(bp) gem_readl(bp, USX_STATUS)
>
> /* struct macb_dma_desc - Hardware DMA descriptor
> * @addr: DMA address of data buffer
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index ced32d2a85e1..5963a60d54b9 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -81,6 +81,14 @@ struct sifive_fu540_macb_mgmt {
> #define MACB_WOL_HAS_MAGIC_PACKET (0x1 << 0)
> #define MACB_WOL_ENABLED (0x1 << 1)
>
> +#define HS_MAC_SPEED_100M 0
> +#define HS_MAC_SPEED_1000M 1
> +#define HS_MAC_SPEED_2500M 2
> +#define HS_MAC_SPEED_5000M 3
> +#define HS_MAC_SPEED_10000M 4
> +
> +#define MACB_SERDES_RATE_10G 1
> +
> /* Graceful stop timeouts in us. We should allow up to
> * 1 frame time (10 Mbits/s, full-duplex, ignoring collisions)
> */
> @@ -90,6 +98,8 @@ struct sifive_fu540_macb_mgmt {
>
> #define MACB_MDIO_TIMEOUT 1000000 /* in usecs */
>
> +#define MACB_USX_BLOCK_LOCK_TIMEOUT 1000000 /* in usecs */
> +
> /* DMA buffer descriptor might be different size
> * depends on hardware configuration:
> *
> @@ -506,6 +516,7 @@ static void macb_validate(struct phylink_config *config,
> state->interface != PHY_INTERFACE_MODE_RMII &&
> state->interface != PHY_INTERFACE_MODE_GMII &&
> state->interface != PHY_INTERFACE_MODE_SGMII &&
> + state->interface != PHY_INTERFACE_MODE_USXGMII &&
> !phy_interface_mode_is_rgmii(state->interface)) {
> bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
> return;
> @@ -518,6 +529,13 @@ static void macb_validate(struct phylink_config *config,
> return;
> }
>
> + if (state->interface == PHY_INTERFACE_MODE_USXGMII &&
> + !(bp->caps & MACB_CAPS_HIGH_SPEED &&
> + bp->caps & MACB_CAPS_PCS)) {
> + bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
> + return;
> + }
> +
> phylink_set_port_modes(mask);
> phylink_set(mask, Autoneg);
> phylink_set(mask, Asym_Pause);
> @@ -527,6 +545,22 @@ static void macb_validate(struct phylink_config *config,
> phylink_set(mask, 100baseT_Half);
> phylink_set(mask, 100baseT_Full);
>
> + if (bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE &&
> + (state->interface == PHY_INTERFACE_MODE_NA ||
> + state->interface == PHY_INTERFACE_MODE_USXGMII)) {
> + phylink_set(mask, 10000baseCR_Full);
> + phylink_set(mask, 10000baseER_Full);
> + phylink_set(mask, 10000baseKR_Full);
> + phylink_set(mask, 10000baseLR_Full);
> + phylink_set(mask, 10000baseLRM_Full);
> + phylink_set(mask, 10000baseSR_Full);
> + phylink_set(mask, 10000baseT_Full);
> + phylink_set(mask, 5000baseT_Full);
> + phylink_set(mask, 2500baseX_Full);
> + phylink_set(mask, 1000baseX_Full);
> + phylink_set(mask, 1000baseT_Full);
> + }
> +
> if (bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE &&
> (state->interface == PHY_INTERFACE_MODE_NA ||
> state->interface == PHY_INTERFACE_MODE_GMII ||
> @@ -544,6 +578,60 @@ static void macb_validate(struct phylink_config *config,
> __ETHTOOL_LINK_MODE_MASK_NBITS);
> }
>
> +static int gem_mac_usx_configure(struct macb *bp,
> + const struct phylink_link_state *state)
> +{
> + u32 speed, config, val;
> + int ret;
> +
> + val = gem_readl(bp, NCFGR);
> + val = GEM_BIT(PCSSEL) | (~GEM_BIT(SGMIIEN) & val);
> + if (state->pause & MLO_PAUSE_TX)
> + val |= GEM_BIT(PAE);
> + gem_writel(bp, NCFGR, val);
> + gem_writel(bp, NCR, gem_readl(bp, NCR) | GEM_BIT(ENABLE_HS_MAC));
> + gem_writel(bp, NCFGR, gem_readl(bp, NCFGR) | GEM_BIT(FD));
> + config = gem_readl(bp, USX_CONTROL);
> + config = GEM_BFINS(SERDES_RATE, MACB_SERDES_RATE_10G, config);
> + config &= ~GEM_BIT(TX_SCR_BYPASS);
> + config &= ~GEM_BIT(RX_SCR_BYPASS);
> + gem_writel(bp, USX_CONTROL, config | GEM_BIT(TX_EN));
> + config = gem_readl(bp, USX_CONTROL);
> + gem_writel(bp, USX_CONTROL, config | GEM_BIT(SIGNAL_OK));
> + ret = readx_poll_timeout(GEM_READ_USX_STATUS, bp, val,
> + val & GEM_BIT(USX_BLOCK_LOCK),
> + 1, MACB_USX_BLOCK_LOCK_TIMEOUT);
> + if (ret < 0) {
> + netdev_warn(bp->dev, "USXGMII block lock failed");
> + return -ETIMEDOUT;
> + }
I mentioned this last time around, so I'm a little surprised it's
still here. As I already stated, there is no requirement that the
USXGMII locks before any of these functions return.
You're also waiting for up to a second in a work queue, which is not
nice behaviour.
> +
> + switch (state->speed) {
> + case SPEED_10000:
> + speed = HS_MAC_SPEED_10000M;
> + break;
> + case SPEED_5000:
> + speed = HS_MAC_SPEED_5000M;
> + break;
> + case SPEED_2500:
> + speed = HS_MAC_SPEED_2500M;
> + break;
> + case SPEED_1000:
> + speed = HS_MAC_SPEED_1000M;
> + break;
> + default:
> + case SPEED_100:
> + speed = HS_MAC_SPEED_100M;
> + break;
macb_validate() goes down to 10Mbps, but you don't handle that here.
If it isn't supported, then macb_validate() shouldn't allow it for this
mode.
> + }
> +
> + gem_writel(bp, HS_MAC_CONFIG, GEM_BFINS(HS_MAC_SPEED, speed,
> + gem_readl(bp, HS_MAC_CONFIG)));
> + gem_writel(bp, USX_CONTROL, GEM_BFINS(USX_CTRL_SPEED, speed,
> + gem_readl(bp, USX_CONTROL)));
> + return 0;
> +}
> +
> static void macb_mac_pcs_get_state(struct phylink_config *config,
> struct phylink_link_state *state)
> {
> @@ -565,30 +653,39 @@ static void macb_mac_config(struct phylink_config *config, unsigned int mode,
>
> spin_lock_irqsave(&bp->lock, flags);
>
> - old_ctrl = ctrl = macb_or_gem_readl(bp, NCFGR);
> + if (bp->phy_interface == PHY_INTERFACE_MODE_USXGMII) {
Why bp->phy_interface and not state->interface?
If you don't support selecting between USXGMII and other modes at
runtime, should macb_validate() be allowing ethtool link modes for
it when it's different from the configured setting?
> + if (gem_mac_usx_configure(bp, state) < 0) {
> + spin_unlock_irqrestore(&bp->lock, flags);
> + phylink_mac_change(bp->phylink, false);
I guess this is the reason you're waiting for the USXGMII block
to lock - do you not have any way to raise an interrupt when
something changes with the USXGMII (or for that matter SGMII)
blocks? Without that, you're fixed to a single speed.
> + return;
> + }
> + } else {
> + old_ctrl = ctrl = macb_or_gem_readl(bp, NCFGR);
>
> - /* Clear all the bits we might set later */
> - ctrl &= ~(GEM_BIT(GBE) | MACB_BIT(SPD) | MACB_BIT(FD) | MACB_BIT(PAE) |
> - GEM_BIT(SGMIIEN) | GEM_BIT(PCSSEL));
> + /* Clear all the bits we might set later */
> + ctrl &= ~(GEM_BIT(GBE) | MACB_BIT(SPD) |
> + MACB_BIT(FD) | MACB_BIT(PAE) |
> + GEM_BIT(SGMIIEN) | GEM_BIT(PCSSEL));
>
> - if (state->speed == SPEED_1000)
> - ctrl |= GEM_BIT(GBE);
> - else if (state->speed == SPEED_100)
> - ctrl |= MACB_BIT(SPD);
> + if (state->speed == SPEED_1000)
> + ctrl |= GEM_BIT(GBE);
> + else if (state->speed == SPEED_100)
> + ctrl |= MACB_BIT(SPD);
>
> - if (state->duplex)
> - ctrl |= MACB_BIT(FD);
> + if (state->duplex)
> + ctrl |= MACB_BIT(FD);
>
> - /* We do not support MLO_PAUSE_RX yet */
> - if (state->pause & MLO_PAUSE_TX)
> - ctrl |= MACB_BIT(PAE);
> + /* We do not support MLO_PAUSE_RX yet */
> + if (state->pause & MLO_PAUSE_TX)
> + ctrl |= MACB_BIT(PAE);
>
> - if (state->interface == PHY_INTERFACE_MODE_SGMII)
> - ctrl |= GEM_BIT(SGMIIEN) | GEM_BIT(PCSSEL);
> + if (state->interface == PHY_INTERFACE_MODE_SGMII)
> + ctrl |= GEM_BIT(SGMIIEN) | GEM_BIT(PCSSEL);
>
> - /* Apply the new configuration, if any */
> - if (old_ctrl ^ ctrl)
> - macb_or_gem_writel(bp, NCFGR, ctrl);
> + /* Apply the new configuration, if any */
> + if (old_ctrl ^ ctrl)
> + macb_or_gem_writel(bp, NCFGR, ctrl);
> + }
>
> bp->speed = state->speed;
>
> @@ -3407,6 +3504,11 @@ static void macb_configure_caps(struct macb *bp,
> dcfg = gem_readl(bp, DCFG1);
> if (GEM_BFEXT(IRQCOR, dcfg) == 0)
> bp->caps |= MACB_CAPS_ISR_CLEAR_ON_WRITE;
> + if (GEM_BFEXT(NO_PCS, dcfg) == 0)
> + bp->caps |= MACB_CAPS_PCS;
> + dcfg = gem_readl(bp, DCFG12);
> + if (GEM_BFEXT(HIGH_SPEED, dcfg) == 1)
> + bp->caps |= MACB_CAPS_HIGH_SPEED;
> dcfg = gem_readl(bp, DCFG2);
> if ((dcfg & (GEM_BIT(RX_PKT_BUFF) | GEM_BIT(TX_PKT_BUFF))) == 0)
> bp->caps |= MACB_CAPS_FIFO_MODE;
> --
> 2.17.1
>
>
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
On Sun, Dec 15, 2019 at 03:12:49PM +0000, Russell King - ARM Linux admin wrote:
> On Fri, Dec 13, 2019 at 09:42:57AM +0000, Milind Parab wrote:
> > This patch add support for high speed USXGMII PCS and 10G
> > speed in Cadence ethernet controller driver.
> >
> > Signed-off-by: Milind Parab <[email protected]>
> > ---
> > drivers/net/ethernet/cadence/macb.h | 50 ++++++++
> > drivers/net/ethernet/cadence/macb_main.c | 138 ++++++++++++++++++++---
> > 2 files changed, 170 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> > index dbf7070fcdba..b731807d1c49 100644
> > --- a/drivers/net/ethernet/cadence/macb.h
> > +++ b/drivers/net/ethernet/cadence/macb.h
> > @@ -76,10 +76,12 @@
> > #define MACB_RBQPH 0x04D4
> >
> > /* GEM register offsets. */
> > +#define GEM_NCR 0x0000 /* Network Control */
> > #define GEM_NCFGR 0x0004 /* Network Config */
> > #define GEM_USRIO 0x000c /* User IO */
> > #define GEM_DMACFG 0x0010 /* DMA Configuration */
> > #define GEM_JML 0x0048 /* Jumbo Max Length */
> > +#define GEM_HS_MAC_CONFIG 0x0050 /* GEM high speed config */
> > #define GEM_HRB 0x0080 /* Hash Bottom */
> > #define GEM_HRT 0x0084 /* Hash Top */
> > #define GEM_SA1B 0x0088 /* Specific1 Bottom */
> > @@ -164,6 +166,9 @@
> > #define GEM_DCFG7 0x0298 /* Design Config 7 */
> > #define GEM_DCFG8 0x029C /* Design Config 8 */
> > #define GEM_DCFG10 0x02A4 /* Design Config 10 */
> > +#define GEM_DCFG12 0x02AC /* Design Config 12 */
> > +#define GEM_USX_CONTROL 0x0A80 /* USXGMII control register */
> > +#define GEM_USX_STATUS 0x0A88 /* USXGMII status register */
> >
> > #define GEM_TXBDCTRL 0x04cc /* TX Buffer Descriptor control register */
> > #define GEM_RXBDCTRL 0x04d0 /* RX Buffer Descriptor control register */
> > @@ -270,11 +275,19 @@
> > #define MACB_IRXFCS_OFFSET 19
> > #define MACB_IRXFCS_SIZE 1
> >
> > +/* GEM specific NCR bitfields. */
> > +#define GEM_ENABLE_HS_MAC_OFFSET 31
> > +#define GEM_ENABLE_HS_MAC_SIZE 1
> > +
> > /* GEM specific NCFGR bitfields. */
> > +#define GEM_FD_OFFSET 1 /* Full duplex */
> > +#define GEM_FD_SIZE 1
> > #define GEM_GBE_OFFSET 10 /* Gigabit mode enable */
> > #define GEM_GBE_SIZE 1
> > #define GEM_PCSSEL_OFFSET 11
> > #define GEM_PCSSEL_SIZE 1
> > +#define GEM_PAE_OFFSET 13 /* Pause enable */
> > +#define GEM_PAE_SIZE 1
> > #define GEM_CLK_OFFSET 18 /* MDC clock division */
> > #define GEM_CLK_SIZE 3
> > #define GEM_DBW_OFFSET 21 /* Data bus width */
> > @@ -455,11 +468,17 @@
> > #define MACB_REV_OFFSET 0
> > #define MACB_REV_SIZE 16
> >
> > +/* Bitfield in HS_MAC_CONFIG */
> > +#define GEM_HS_MAC_SPEED_OFFSET 0
> > +#define GEM_HS_MAC_SPEED_SIZE 3
> > +
> > /* Bitfields in DCFG1. */
> > #define GEM_IRQCOR_OFFSET 23
> > #define GEM_IRQCOR_SIZE 1
> > #define GEM_DBWDEF_OFFSET 25
> > #define GEM_DBWDEF_SIZE 3
> > +#define GEM_NO_PCS_OFFSET 0
> > +#define GEM_NO_PCS_SIZE 1
> >
> > /* Bitfields in DCFG2. */
> > #define GEM_RX_PKT_BUFF_OFFSET 20
> > @@ -494,6 +513,34 @@
> > #define GEM_RXBD_RDBUFF_OFFSET 8
> > #define GEM_RXBD_RDBUFF_SIZE 4
> >
> > +/* Bitfields in DCFG12. */
> > +#define GEM_HIGH_SPEED_OFFSET 26
> > +#define GEM_HIGH_SPEED_SIZE 1
> > +
> > +/* Bitfields in USX_CONTROL. */
> > +#define GEM_USX_CTRL_SPEED_OFFSET 14
> > +#define GEM_USX_CTRL_SPEED_SIZE 3
> > +#define GEM_SERDES_RATE_OFFSET 12
> > +#define GEM_SERDES_RATE_SIZE 2
> > +#define GEM_RX_SCR_BYPASS_OFFSET 9
> > +#define GEM_RX_SCR_BYPASS_SIZE 1
> > +#define GEM_TX_SCR_BYPASS_OFFSET 8
> > +#define GEM_TX_SCR_BYPASS_SIZE 1
> > +#define GEM_RX_SYNC_RESET_OFFSET 2
> > +#define GEM_RX_SYNC_RESET_SIZE 1
> > +#define GEM_TX_EN_OFFSET 1
> > +#define GEM_TX_EN_SIZE 1
> > +#define GEM_SIGNAL_OK_OFFSET 0
> > +#define GEM_SIGNAL_OK_SIZE 1
> > +
> > +/* Bitfields in USX_STATUS. */
> > +#define GEM_USX_TX_FAULT_OFFSET 28
> > +#define GEM_USX_TX_FAULT_SIZE 1
> > +#define GEM_USX_RX_FAULT_OFFSET 27
> > +#define GEM_USX_RX_FAULT_SIZE 1
> > +#define GEM_USX_BLOCK_LOCK_OFFSET 0
> > +#define GEM_USX_BLOCK_LOCK_SIZE 1
> > +
> > /* Bitfields in TISUBN */
> > #define GEM_SUBNSINCR_OFFSET 0
> > #define GEM_SUBNSINCRL_OFFSET 24
> > @@ -656,6 +703,8 @@
> > #define MACB_CAPS_GIGABIT_MODE_AVAILABLE 0x20000000
> > #define MACB_CAPS_SG_DISABLED 0x40000000
> > #define MACB_CAPS_MACB_IS_GEM 0x80000000
> > +#define MACB_CAPS_PCS 0x01000000
> > +#define MACB_CAPS_HIGH_SPEED 0x02000000
> >
> > /* LSO settings */
> > #define MACB_LSO_UFO_ENABLE 0x01
> > @@ -724,6 +773,7 @@
> > })
> >
> > #define MACB_READ_NSR(bp) macb_readl(bp, NSR)
> > +#define GEM_READ_USX_STATUS(bp) gem_readl(bp, USX_STATUS)
> >
> > /* struct macb_dma_desc - Hardware DMA descriptor
> > * @addr: DMA address of data buffer
> > diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> > index ced32d2a85e1..5963a60d54b9 100644
> > --- a/drivers/net/ethernet/cadence/macb_main.c
> > +++ b/drivers/net/ethernet/cadence/macb_main.c
> > @@ -81,6 +81,14 @@ struct sifive_fu540_macb_mgmt {
> > #define MACB_WOL_HAS_MAGIC_PACKET (0x1 << 0)
> > #define MACB_WOL_ENABLED (0x1 << 1)
> >
> > +#define HS_MAC_SPEED_100M 0
> > +#define HS_MAC_SPEED_1000M 1
> > +#define HS_MAC_SPEED_2500M 2
> > +#define HS_MAC_SPEED_5000M 3
> > +#define HS_MAC_SPEED_10000M 4
> > +
> > +#define MACB_SERDES_RATE_10G 1
> > +
> > /* Graceful stop timeouts in us. We should allow up to
> > * 1 frame time (10 Mbits/s, full-duplex, ignoring collisions)
> > */
> > @@ -90,6 +98,8 @@ struct sifive_fu540_macb_mgmt {
> >
> > #define MACB_MDIO_TIMEOUT 1000000 /* in usecs */
> >
> > +#define MACB_USX_BLOCK_LOCK_TIMEOUT 1000000 /* in usecs */
> > +
> > /* DMA buffer descriptor might be different size
> > * depends on hardware configuration:
> > *
> > @@ -506,6 +516,7 @@ static void macb_validate(struct phylink_config *config,
> > state->interface != PHY_INTERFACE_MODE_RMII &&
> > state->interface != PHY_INTERFACE_MODE_GMII &&
> > state->interface != PHY_INTERFACE_MODE_SGMII &&
> > + state->interface != PHY_INTERFACE_MODE_USXGMII &&
> > !phy_interface_mode_is_rgmii(state->interface)) {
> > bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
> > return;
> > @@ -518,6 +529,13 @@ static void macb_validate(struct phylink_config *config,
> > return;
> > }
> >
> > + if (state->interface == PHY_INTERFACE_MODE_USXGMII &&
> > + !(bp->caps & MACB_CAPS_HIGH_SPEED &&
> > + bp->caps & MACB_CAPS_PCS)) {
> > + bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
> > + return;
> > + }
> > +
> > phylink_set_port_modes(mask);
> > phylink_set(mask, Autoneg);
> > phylink_set(mask, Asym_Pause);
> > @@ -527,6 +545,22 @@ static void macb_validate(struct phylink_config *config,
> > phylink_set(mask, 100baseT_Half);
> > phylink_set(mask, 100baseT_Full);
> >
> > + if (bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE &&
> > + (state->interface == PHY_INTERFACE_MODE_NA ||
> > + state->interface == PHY_INTERFACE_MODE_USXGMII)) {
> > + phylink_set(mask, 10000baseCR_Full);
> > + phylink_set(mask, 10000baseER_Full);
> > + phylink_set(mask, 10000baseKR_Full);
> > + phylink_set(mask, 10000baseLR_Full);
> > + phylink_set(mask, 10000baseLRM_Full);
> > + phylink_set(mask, 10000baseSR_Full);
> > + phylink_set(mask, 10000baseT_Full);
> > + phylink_set(mask, 5000baseT_Full);
> > + phylink_set(mask, 2500baseX_Full);
> > + phylink_set(mask, 1000baseX_Full);
> > + phylink_set(mask, 1000baseT_Full);
> > + }
> > +
> > if (bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE &&
> > (state->interface == PHY_INTERFACE_MODE_NA ||
> > state->interface == PHY_INTERFACE_MODE_GMII ||
> > @@ -544,6 +578,60 @@ static void macb_validate(struct phylink_config *config,
> > __ETHTOOL_LINK_MODE_MASK_NBITS);
> > }
> >
> > +static int gem_mac_usx_configure(struct macb *bp,
> > + const struct phylink_link_state *state)
> > +{
> > + u32 speed, config, val;
> > + int ret;
> > +
> > + val = gem_readl(bp, NCFGR);
> > + val = GEM_BIT(PCSSEL) | (~GEM_BIT(SGMIIEN) & val);
> > + if (state->pause & MLO_PAUSE_TX)
> > + val |= GEM_BIT(PAE);
> > + gem_writel(bp, NCFGR, val);
> > + gem_writel(bp, NCR, gem_readl(bp, NCR) | GEM_BIT(ENABLE_HS_MAC));
> > + gem_writel(bp, NCFGR, gem_readl(bp, NCFGR) | GEM_BIT(FD));
> > + config = gem_readl(bp, USX_CONTROL);
> > + config = GEM_BFINS(SERDES_RATE, MACB_SERDES_RATE_10G, config);
> > + config &= ~GEM_BIT(TX_SCR_BYPASS);
> > + config &= ~GEM_BIT(RX_SCR_BYPASS);
> > + gem_writel(bp, USX_CONTROL, config | GEM_BIT(TX_EN));
> > + config = gem_readl(bp, USX_CONTROL);
> > + gem_writel(bp, USX_CONTROL, config | GEM_BIT(SIGNAL_OK));
> > + ret = readx_poll_timeout(GEM_READ_USX_STATUS, bp, val,
> > + val & GEM_BIT(USX_BLOCK_LOCK),
> > + 1, MACB_USX_BLOCK_LOCK_TIMEOUT);
> > + if (ret < 0) {
> > + netdev_warn(bp->dev, "USXGMII block lock failed");
> > + return -ETIMEDOUT;
> > + }
>
> I mentioned this last time around, so I'm a little surprised it's
> still here. As I already stated, there is no requirement that the
> USXGMII locks before any of these functions return.
>
> You're also waiting for up to a second in a work queue, which is not
> nice behaviour.
>
> > +
> > + switch (state->speed) {
> > + case SPEED_10000:
> > + speed = HS_MAC_SPEED_10000M;
> > + break;
> > + case SPEED_5000:
> > + speed = HS_MAC_SPEED_5000M;
> > + break;
> > + case SPEED_2500:
> > + speed = HS_MAC_SPEED_2500M;
> > + break;
> > + case SPEED_1000:
> > + speed = HS_MAC_SPEED_1000M;
> > + break;
> > + default:
> > + case SPEED_100:
> > + speed = HS_MAC_SPEED_100M;
> > + break;
>
> macb_validate() goes down to 10Mbps, but you don't handle that here.
> If it isn't supported, then macb_validate() shouldn't allow it for this
> mode.
>
> > + }
> > +
> > + gem_writel(bp, HS_MAC_CONFIG, GEM_BFINS(HS_MAC_SPEED, speed,
> > + gem_readl(bp, HS_MAC_CONFIG)));
> > + gem_writel(bp, USX_CONTROL, GEM_BFINS(USX_CTRL_SPEED, speed,
> > + gem_readl(bp, USX_CONTROL)));
> > + return 0;
> > +}
> > +
> > static void macb_mac_pcs_get_state(struct phylink_config *config,
> > struct phylink_link_state *state)
> > {
> > @@ -565,30 +653,39 @@ static void macb_mac_config(struct phylink_config *config, unsigned int mode,
> >
> > spin_lock_irqsave(&bp->lock, flags);
> >
> > - old_ctrl = ctrl = macb_or_gem_readl(bp, NCFGR);
> > + if (bp->phy_interface == PHY_INTERFACE_MODE_USXGMII) {
>
> Why bp->phy_interface and not state->interface?
>
> If you don't support selecting between USXGMII and other modes at
> runtime, should macb_validate() be allowing ethtool link modes for
> it when it's different from the configured setting?
>
> > + if (gem_mac_usx_configure(bp, state) < 0) {
> > + spin_unlock_irqrestore(&bp->lock, flags);
> > + phylink_mac_change(bp->phylink, false);
>
> I guess this is the reason you're waiting for the USXGMII block
> to lock - do you not have any way to raise an interrupt when
> something changes with the USXGMII (or for that matter SGMII)
> blocks? Without that, you're fixed to a single speed.
BTW, if you don't have an macb_mac_pcs_get_state() implementation,
and from what you described last time around, I don't see how SGMII
nor this new addition of USXGMII can work for you. Both these
protocols use in-band control words, which should be read and
interpreted in macb_mac_pcs_get_state().
What I think you're trying to do is to use your PCS PHY as a normal
PHY, or maybe you're ignoring the PCS PHY completely and relying on
an external PHY (and hence always using MLO_AN_PHY or MLO_AN_FIXED
mode.)
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
>>
>> Additional 3rd party I2C IP required (not part of GEM) for module
>> interrogation (MDIO to I2C handled by SW
>> +--------------+ +-----------+
>> | | | | | SFP+ |
>> | GEM MAC/DMA | <---> | SerDes | <---- SFI-----> | Optical |
>> | USX PCS| | | (PMA) | | Module |
>> +--------------+ +-----------+
>> ^
>> +--------+ |
>> | I2C | |
>> | Master | <-------------------------------------|
>> +--------+
>The kernel supports this through the sfp and phylink support. SFI is
>more commonly known as 10GBASE-R. Note that this is *not* USXGMII.
>Link status needs to come from the MAC side, so macb_mac_pcs_get_state()
>is required.
>
>> Rate determined by 10GBASE-T PHY capability through auto-negotiation.
>> I2C IP required
>> +--------------+ +-----------+
>> | | | | | SFP+ to |
>> | GEM MAC/DMA | <---> | SerDes | <---- SFI-----> | 10GBASE-T |
>> | USX PCS| | | (PMA) | | |
>> +--------------+ +-----------+
>> ^
>> +--------+ |
>> | I2C | |
>> | Master | <-------------------------------------|
>> +--------+
>
>The 10G copper module I have uses 10GBASE-R, 5000BASE-X, 2500BASE-X,
>and SGMII (without in-band status), dynamically switching between
>these depending on the results of the copper side negotiation.
>
>> USXGMII PHY. Uses MDIO or equivalent for status xfer
>> +-------------+ +--------+
>> | | | | | |
>> | GEM MAC/DMA | <---> | SerDes | <--- USXGMII ---> | PHY |
>> | USX PCS | | (PMA) | | |
>> +-------------+ +--------+
>> ^ ^
>> |_____________________ MDIO ______________________|
>
>Overall, please implement phylink properly for your MAC, rather than
>the current half-hearted approach that *will* break in various
>circumstances.
>
We would need more time to get back on the restructured implementation. While we
work on that, is it okay to accept patch 1/3 and patch 2/3?
>>
>> Additional 3rd party I2C IP required (not part of GEM) for module
>> interrogation (MDIO to I2C handled by SW
>> +--------------+ +-----------+
>> | | | | | SFP+ |
>> | GEM MAC/DMA | <---> | SerDes | <---- SFI-----> | Optical |
>> | USX PCS| | | (PMA) | | Module |
>> +--------------+ +-----------+
>> ^
>> +--------+ |
>> | I2C | |
>> | Master | <-------------------------------------|
>> +--------+
>The kernel supports this through the sfp and phylink support. SFI is
>more commonly known as 10GBASE-R. Note that this is *not* USXGMII.
>Link status needs to come from the MAC side, so macb_mac_pcs_get_state()
>is required.
>
>> Rate determined by 10GBASE-T PHY capability through auto-negotiation.
>> I2C IP required
>> +--------------+ +-----------+
>> | | | | | SFP+ to |
>> | GEM MAC/DMA | <---> | SerDes | <---- SFI-----> | 10GBASE-T |
>> | USX PCS| | | (PMA) | | |
>> +--------------+ +-----------+
>> ^
>> +--------+ |
>> | I2C | |
>> | Master | <-------------------------------------|
>> +--------+
>
>The 10G copper module I have uses 10GBASE-R, 5000BASE-X, 2500BASE-X,
>and SGMII (without in-band status), dynamically switching between
>these depending on the results of the copper side negotiation.
>
>> USXGMII PHY. Uses MDIO or equivalent for status xfer
>> +-------------+ +--------+
>> | | | | | |
>> | GEM MAC/DMA | <---> | SerDes | <--- USXGMII ---> | PHY |
>> | USX PCS | | (PMA) | | |
>> +-------------+ +--------+
>> ^ ^
>> |_____________________ MDIO ______________________|
>
>Overall, please implement phylink properly for your MAC, rather than
>the current half-hearted approach that *will* break in various
>circumstances.
>
We would need more time to get back on the restructured implementation.
While we work on that, is it okay to accept patch 1/3 and patch 2/3?
Le samedi 21 décembre 2019 à 11:08 +0000, Milind Parab a écrit :
> > > Additional 3rd party I2C IP required (not part of GEM) for module
> > > interrogation (MDIO to I2C handled by SW
> > > +--------------+ +-----------+
> > > | | | | | SFP+ |
> > > | GEM MAC/DMA | <---> | SerDes | <---- SFI-----> | Optical |
> > > | USX PCS| | | (PMA) | | Module |
> > > +--------------+ +-----------+
> > > ^
> > > +--------+ |
> > > | I2C | |
> > > | Master | <-------------------------------------|
> > > +--------+
> > The kernel supports this through the sfp and phylink support. SFI is
> > more commonly known as 10GBASE-R. Note that this is *not* USXGMII.
> > Link status needs to come from the MAC side, so macb_mac_pcs_get_state()
> > is required.
> >
> > > Rate determined by 10GBASE-T PHY capability through auto-negotiation.
> > > I2C IP required
> > > +--------------+ +-----------+
> > > | | | | | SFP+ to |
> > > | GEM MAC/DMA | <---> | SerDes | <---- SFI-----> | 10GBASE-T |
> > > | USX PCS| | | (PMA) | | |
> > > +--------------+ +-----------+
> > > ^
> > > +--------+ |
> > > | I2C | |
> > > | Master | <-------------------------------------|
> > > +--------+
> >
> > The 10G copper module I have uses 10GBASE-R, 5000BASE-X, 2500BASE-X,
> > and SGMII (without in-band status), dynamically switching between
> > these depending on the results of the copper side negotiation.
> >
> > > USXGMII PHY. Uses MDIO or equivalent for status xfer
> > > +-------------+ +--------+
> > > | | | | | |
> > > | GEM MAC/DMA | <---> | SerDes | <--- USXGMII ---> | PHY |
> > > | USX PCS | | (PMA) | | |
> > > +-------------+ +--------+
> > > ^ ^
> > > |_____________________ MDIO ______________________|
> >
> > Overall, please implement phylink properly for your MAC, rather than
> > the current half-hearted approach that *will* break in various
> > circumstances.
> >
>
> We would need more time to get back on the restructured implementation.
> While we work on that, is it okay to accept patch 1/3 and patch 2/3?
Milind,
I'm not against queuing patches 1 & 2 of this series while the 3rd one is
still in review.
However, I would prefer that you follow-up Jakub Kicinski's advice when he
answered to your v2 serries.
So please, re-send the patches 1 as a "fix" type of patch, making sure that it
still applies after latest Antoine's changes. Then, re-send the 2/3 patch of
the series as a v3 collecting reviews (as I didn't received v2).
When the first 2 are queued by David, we can resume the discussion about your
3rd patch and what I can tell you about this topic is that it's really by
following Russell comments and advice that we will make progress: I won't
accept anything without his acknowledgment, of course.
Best regards,
Nicolas