Skip to contentSkip to conversation

Armada 38x comphy driver to support 2.5Gbps networking

RFC PATCH Applied Russell King - ARM Linux 6 participants 30 messages Nov 12, 2018 – Dec 7, 2018

Subsystems (5)
lore
Share & tools
lore
#0 lore raw

cover Armada 38x comphy driver to support 2.5Gbps networking

Hi,

This series adds support for dynamically switching between 1Gbps and 2.5Gbps networking for the Marvell Armada 38x SoCs, tested on Armada 388 on the Clearfog platform.

This is necessary to be able to connect (eg) a Clearfog platform with a Macchiatobin platform via the SFP sockets, as Clearfog currently only supports 1Gbps networking via the SFP socket and Macchiatobin defaults to 2.5Gbps when using Fiberchannel SFPs.

In order to allow dynamic switching, we need to implement a common phy driver to switch the ethernet serdes lane speed - 2.5Gbps is just 1Gbps up-clocked by 2.5x. We implement a simple comphy driver to achieve this, which only supports networking.

With this, we are able to support both Fiberchannel SFPs operating at 2.5Gbps or 1Gbps, and 1G ethernet SFPs plugged into the Clearfog platform, dynamically selecting according to the SFPs abilities.

I'm aware of the proposed changes to the PHY layer, changing phy_set_mode() to take the ethernet phy interface type, hence why this is RFC - there's also the question about how this will be merged. This series is currently based on 4.20-rc1, but will likely need to be rebased when the PHY layer changes hit.

 .../bindings/net/marvell-armada-370-neta.txt       |   2 +-
 .../bindings/phy/phy-armada38x-comphy.txt          |  40 ++++
 arch/arm/boot/dts/armada-388-clearfog.dtsi         |   2 +
 arch/arm/boot/dts/armada-38x.dtsi                  |  37 ++++
 drivers/net/ethernet/marvell/mvneta.c              |  58 ++++-
 drivers/phy/marvell/Kconfig                        |  10 +
 drivers/phy/marvell/Makefile                       |   1 +
 drivers/phy/marvell/phy-armada38x-comphy.c         | 236 +++++++++++++++++++++
 8 files changed, 378 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/phy/phy-armada38x-comphy.txt
 create mode 100644 drivers/phy/marvell/phy-armada38x-comphy.c
Signature · 3 lines
RMK's Patch system: http://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
#1 lore raw

RFC PATCH 1/6 dt-bindings: phy: Armada 38x common phy bindings

Add the Marvell Armada 38x common phy bindings.

---
 .../bindings/phy/phy-armada38x-comphy.txt          | 40 ++++++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/phy-armada38x-comphy.txt
Documentation/devicetree/bindings/phy/phy-armada38x-comphy.txt new file +40 -0
new file mode 100644
index 000000000000..ad49e5c01334
@@ -0,0 +1,40 @@
+mvebu armada 38x comphy driver
+------------------------------
+
+This comphy controller can be found on Marvell Armada 38x. It provides a
+number of shared PHYs used by various interfaces (network, sata, usb,
+PCIe...).
+
+Required properties:
+
+- compatible: should be "marvell,armada-380-comphy"
+- reg: should contain the comphy register location and length.
+- #address-cells: should be 1.
+- #size-cells: should be 0.
+
+A sub-node is required for each comphy lane provided by the comphy.
+
+Required properties (child nodes):
+
+- reg: comphy lane number.
+- #phy-cells : from the generic phy bindings, must be 1. Defines the
+               input port to use for a given comphy lane.
+
+Example:
+
+	comphy: phy at 18300 {
+		compatible = "marvell,armada-380-comphy";
+		reg = <0x18300 0x100>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		cpm_comphy0: phy at 0 {
+			reg = <0>;
+			#phy-cells = <1>;
+		};
+
+		cpm_comphy1: phy at 1 {
+			reg = <1>;
+			#phy-cells = <1>;
+		};
+	};
Signature · 1 line
2.7.4
maintainer Reviewed-by on v1 1/6
#2 lore raw

Re: [RFC PATCH 1/6] dt-bindings: phy: Armada 38x common phy bindings

Quoted 8 lines · Russell King

Add the Marvell Armada 38x common phy bindings.

---
 .../bindings/phy/phy-armada38x-comphy.txt          | 40 ++++++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/phy-armada38x-comphy.txt
#3 lore raw

RFC PATCH 2/6 phy: armada38x: add common phy support

In reply to Russell King - ARM Linux

Add support for the Armada 38x common phy to allow us to change the speed of the Ethernet serdes lane. This driver only supports manipulation of the speed, it does not support configuration of the common phy.

---
 drivers/phy/marvell/Kconfig                |  10 ++
 drivers/phy/marvell/Makefile               |   1 +
 drivers/phy/marvell/phy-armada38x-comphy.c | 236 +++++++++++++++++++++++++++++
 3 files changed, 247 insertions(+)
 create mode 100644 drivers/phy/marvell/phy-armada38x-comphy.c
drivers/phy/marvell/Kconfig +10 -0
index 6fb4b56e4c14..224ea4e6a46d 100644
@@ -21,6 +21,16 @@ config PHY_BERLIN_USB
 	help
 	  Enable this to support the USB PHY on Marvell Berlin SoCs.
 
+config PHY_MVEBU_A38X_COMPHY
+	tristate "Marvell Armada 38x comphy driver"
+	depends on ARCH_MVEBU || COMPILE_TEST
+	depends on OF
+	select GENERIC_PHY
+	help
+	  This driver allows to control the comphy, an hardware block providing
+	  shared serdes PHYs on Marvell Armada 38x. Its serdes lanes can be
+	  used by various controllers (Ethernet, sata, usb, PCIe...).
+
 config PHY_MVEBU_CP110_COMPHY
 	tristate "Marvell CP110 comphy driver"
 	depends on ARCH_MVEBU || COMPILE_TEST
drivers/phy/marvell/Makefile +1 -0
index 3975b144f8ec..59b6c03ef756 100644
@@ -2,6 +2,7 @@
 obj-$(CONFIG_ARMADA375_USBCLUSTER_PHY)	+= phy-armada375-usb2.o
 obj-$(CONFIG_PHY_BERLIN_SATA)		+= phy-berlin-sata.o
 obj-$(CONFIG_PHY_BERLIN_USB)		+= phy-berlin-usb.o
+obj-$(CONFIG_PHY_MVEBU_A38X_COMPHY)	+= phy-armada38x-comphy.o
 obj-$(CONFIG_PHY_MVEBU_CP110_COMPHY)	+= phy-mvebu-cp110-comphy.o
 obj-$(CONFIG_PHY_MVEBU_SATA)		+= phy-mvebu-sata.o
 obj-$(CONFIG_PHY_PXA_28NM_HSIC)		+= phy-pxa-28nm-hsic.o
drivers/phy/marvell/phy-armada38x-comphy.c new file +236 -0

236 lines not on this page · raw message

Signature · 1 line
2.7.4
#4 lore raw

Re: [RFC PATCH 2/6] phy: armada38x: add common phy support

Quoted 4 lines
drivers/phy/marvell/phy-armada38x-comphy.c new file +4 -0
+static int a38x_comphy_poll(struct a38x_comphy_lane *lane,
+			    unsigned int offset, u32 mask, u32 value)
+{
+	unsigned int timeout = 10;

drivers/phy/marvell/phy-armada38x-comphy.c line 87

Quoted 10 lines
drivers/phy/marvell/phy-armada38x-comphy.c new file +10 -0
+	u32 val;
+
+	while (1) {
+		val = readl_relaxed(lane->base + offset);
+		if ((val & mask) == value)
+			return 0;
+		if (!timeout--)
+			break;
+		udelay(10);
+	}

Hi Russell

Maybe use one of the readx_poll_timeout() variants?

Andrew

#5 lore raw

Re: [RFC PATCH 2/6] phy: armada38x: add common phy support

In reply to Russell King

Hi,

Quoted 155 lines · Russell King

Add support for the Armada 38x common phy to allow us to change the speed of the Ethernet serdes lane. This driver only supports manipulation of the speed, it does not support configuration of the common phy.

---
 drivers/phy/marvell/Kconfig                |  10 ++
 drivers/phy/marvell/Makefile               |   1 +
 drivers/phy/marvell/phy-armada38x-comphy.c | 236 +++++++++++++++++++++++++++++
 3 files changed, 247 insertions(+)
 create mode 100644 drivers/phy/marvell/phy-armada38x-comphy.c
drivers/phy/marvell/Kconfig +10 -0
index 6fb4b56e4c14..224ea4e6a46d 100644
@@ -21,6 +21,16 @@ config PHY_BERLIN_USB
 	help
 	  Enable this to support the USB PHY on Marvell Berlin SoCs.
 
+config PHY_MVEBU_A38X_COMPHY
+	tristate "Marvell Armada 38x comphy driver"
+	depends on ARCH_MVEBU || COMPILE_TEST
+	depends on OF
+	select GENERIC_PHY
+	help
+	  This driver allows to control the comphy, an hardware block providing
+	  shared serdes PHYs on Marvell Armada 38x. Its serdes lanes can be
+	  used by various controllers (Ethernet, sata, usb, PCIe...).
+
 config PHY_MVEBU_CP110_COMPHY
 	tristate "Marvell CP110 comphy driver"
 	depends on ARCH_MVEBU || COMPILE_TEST
drivers/phy/marvell/Makefile +1 -0
index 3975b144f8ec..59b6c03ef756 100644
@@ -2,6 +2,7 @@
 obj-$(CONFIG_ARMADA375_USBCLUSTER_PHY)	+= phy-armada375-usb2.o
 obj-$(CONFIG_PHY_BERLIN_SATA)		+= phy-berlin-sata.o
 obj-$(CONFIG_PHY_BERLIN_USB)		+= phy-berlin-usb.o
+obj-$(CONFIG_PHY_MVEBU_A38X_COMPHY)	+= phy-armada38x-comphy.o
 obj-$(CONFIG_PHY_MVEBU_CP110_COMPHY)	+= phy-mvebu-cp110-comphy.o
 obj-$(CONFIG_PHY_MVEBU_SATA)		+= phy-mvebu-sata.o
 obj-$(CONFIG_PHY_PXA_28NM_HSIC)		+= phy-pxa-28nm-hsic.o
drivers/phy/marvell/phy-armada38x-comphy.c new file +103 -0
new file mode 100644
index 000000000000..61d1965e1cf6
@@ -0,0 +1,236 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Russell King, Deep Blue Solutions Ltd.
+ *
+ * Partly derived from CP110 comphy driver by Antoine Tenart
+ * <[email protected]>
+ */
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+
+#define MAX_A38X_COMPHY	6
+#define MAX_A38X_PORTS	3
+
+#define COMPHY_CFG1		0x00
+#define  COMPHY_CFG1_GEN_TX(x)		((x) << 26)
+#define  COMPHY_CFG1_GEN_TX_MSK		COMPHY_CFG1_GEN_TX(15)
+#define  COMPHY_CFG1_GEN_RX(x)		((x) << 22)
+#define  COMPHY_CFG1_GEN_RX_MSK		COMPHY_CFG1_GEN_RX(15)
+#define  GEN_SGMII_1_25GBPS		6
+#define  GEN_SGMII_3_125GBPS		8
+
+#define COMPHY_STAT1		0x18
+#define  COMPHY_STAT1_PLL_RDY_TX	BIT(3)
+#define  COMPHY_STAT1_PLL_RDY_RX	BIT(2)
+
+#define COMPHY_SELECTOR		0xfc
+
+struct a38x_comphy;
+
+struct a38x_comphy_lane {
+	void __iomem *base;
+	struct a38x_comphy *priv;
+	unsigned int n;
+
+	int port;
+};
+
+struct a38x_comphy {
+	void __iomem *base;
+	struct device *dev;
+	struct a38x_comphy_lane lane[MAX_A38X_COMPHY];
+};
+
+static const u8 gbe_mux[MAX_A38X_COMPHY][MAX_A38X_PORTS] = {
+	{ 0, 0, 0 },
+	{ 4, 5, 0 },
+	{ 0, 4, 0 },
+	{ 0, 0, 4 },
+	{ 0, 3, 0 },
+	{ 0, 0, 3 },
+};
+
+static void a38x_comphy_set_reg(struct a38x_comphy_lane *lane,
+				unsigned int offset, u32 mask, u32 value)
+{
+	u32 val;
+
+	val = readl_relaxed(lane->base + offset) & ~mask;
+	writel(val | value, lane->base + offset);
+}
+
+static void a38x_comphy_set_speed(struct a38x_comphy_lane *lane,
+				  unsigned int gen_tx, unsigned int gen_rx)
+{
+	a38x_comphy_set_reg(lane, COMPHY_CFG1,
+			    COMPHY_CFG1_GEN_TX_MSK | COMPHY_CFG1_GEN_RX_MSK,
+			    COMPHY_CFG1_GEN_TX(gen_tx) |
+		            COMPHY_CFG1_GEN_RX(gen_rx));
+}
+
+static int a38x_comphy_poll(struct a38x_comphy_lane *lane,
+			    unsigned int offset, u32 mask, u32 value)
+{
+	unsigned int timeout = 10;
+	u32 val;
+
+	while (1) {
+		val = readl_relaxed(lane->base + offset);
+		if ((val & mask) == value)
+			return 0;
+		if (!timeout--)
+			break;
+		udelay(10);
+	}
+
+	dev_err(lane->priv->dev, "comphy%u: timed out waiting for status\n",
+		lane->n);
+
+	return -ETIMEDOUT;
+}
+
+/*
+ * We only support changing the speed for comphys configured for GBE.
+ * Since that is all we do, we only poll for PLL ready status.
+ */
+static int a38x_comphy_set_mode(struct phy *phy, enum phy_mode mode)
+{
+	struct a38x_comphy_lane *lane = phy_get_drvdata(phy);
+	unsigned int gen;
+	u32 val;

drivers/phy/marvell/phy-armada38x-comphy.c line 113

Quoted 10 lines
drivers/phy/marvell/phy-armada38x-comphy.c new file +10 -0
+
+	val = readl_relaxed(lane->priv->base + COMPHY_SELECTOR);
+	val = (val >> (4 * lane->n)) & 0xf;
+
+	if (!gbe_mux[lane->n][lane->port] ||
+	    val != gbe_mux[lane->n][lane->port]) {
+		dev_warn(lane->priv->dev,
+			 "comphy%u: not configured for GBE\n", lane->n);
+		return -EINVAL;
+	}

Any reason for not doing this check in a38x_comphy_xlate? We could fail during phy_get if comphy is not configured for GBE then.

Thanks
Kishon

Tested-by on v1 2/6
#6 lore raw

Re: [RFC PATCH 2/6] phy: armada38x: add common phy support

In reply to Russell King

Hi Russell,

Quoted 4 lines · On Mon, 12 Nov 2018 12:30:52 +0000 Russell King <[email protected]>…

Add support for the Armada 38x common phy to allow us to change the speed of the Ethernet serdes lane. This driver only supports manipulation of the speed, it does not support configuration of the common phy.

Quoted 1 line
Signature · 3 lines
Maxime Chevallier, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
#7 lore raw

RFC PATCH 3/6 ARM: dts: add description for Armada 38x common phy

In reply to Russell King - ARM Linux

Add the DT description for the Armada 38x common phy.

---
 arch/arm/boot/dts/armada-38x.dtsi | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)
arch/arm/boot/dts/armada-38x.dtsi +37 -0
index 929459c42760..7b2e2bd6479b 100644
@@ -335,6 +335,43 @@
 				#clock-cells = <1>;
 			};
 
+			comphy: phy at 18300 {
+				compatible = "marvell,armada-380-comphy";
+				reg = <0x18300 0x100>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				comphy0: phy at 0 {
+					reg = <0>;
+					#phy-cells = <1>;
+				};
+
+				comphy1: phy at 1 {
+					reg = <1>;
+					#phy-cells = <1>;
+				};
+
+				comphy2: phy at 2 {
+					reg = <2>;
+					#phy-cells = <1>;
+				};
+
+				comphy3: phy at 3 {
+					reg = <3>;
+					#phy-cells = <1>;
+				};
+
+				comphy4: phy at 4 {
+					reg = <4>;
+					#phy-cells = <1>;
+				};
+
+				comphy5: phy at 5 {
+					reg = <5>;
+					#phy-cells = <1>;
+				};
+			};
+
 			coreclk: mvebu-sar at 18600 {
 				compatible = "marvell,armada-380-core-clock";
 				reg = <0x18600 0x04>;
Signature · 1 line
2.7.4
Tested-by on v1 3/6
#8 lore raw

Re: [RFC PATCH 3/6] ARM: dts: add description for Armada 38x common phy

Hi Russell,

Quoted 1 line · On Mon, 12 Nov 2018 12:30:57 +0000 Russell King <[email protected]>…

Add the DT description for the Armada 38x common phy.

Quoted 1 line
Signature · 3 lines
Maxime Chevallier, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
#9 lore raw

RFC PATCH 4/6 dt-bindings: update mvneta binding document

In reply to Russell King - ARM Linux
---
 Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt +1 -1
index bedcfd5a52cd..691f886cfc4a 100644
@@ -19,7 +19,7 @@
   "marvell,armada-370-neta" and 9800B for others.
 - clock-names: List of names corresponding to clocks property; shall be
   "core" for core clock and "bus" for the optional bus clock.
-
+- phys: comphy for the ethernet port, see ../phy/phy-bindings.txt
 
 Optional properties (valid only for Armada XP/38x):
 
Signature · 1 line
2.7.4
maintainer
#10 lore raw

Re: [RFC PATCH 4/6] dt-bindings: update mvneta binding document

Quoted 1 line · Russell King

Needs a better subject and a commit msg.

Quoted 20 lines
---
 Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt +1 -1
index bedcfd5a52cd..691f886cfc4a 100644
@@ -19,7 +19,7 @@
   "marvell,armada-370-neta" and 9800B for others.
 - clock-names: List of names corresponding to clocks property; shall be
   "core" for core clock and "bus" for the optional bus clock.
-
+- phys: comphy for the ethernet port, see ../phy/phy-bindings.txt
 
 Optional properties (valid only for Armada XP/38x):
 
Signature · 1 line
2.7.4
#11 lore raw

Re: [RFC PATCH 4/6] dt-bindings: update mvneta binding document

Quoted 4 lines · Rob Herring
Quoted 1 line · Russell King

Needs a better subject and a commit msg.

Hmm, not sure why it didn't contain:

"dt-bindings: net: mvneta: add phys property

Add an optional phys property to the mvneta binding documentation for the common phy.

as the commit message. With the correct commit message, are you happy with it?

Quoted 21 lines
Quoted 21 lines
---
 Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt +1 -1
index bedcfd5a52cd..691f886cfc4a 100644
@@ -19,7 +19,7 @@
   "marvell,armada-370-neta" and 9800B for others.
 - clock-names: List of names corresponding to clocks property; shall be
   "core" for core clock and "bus" for the optional bus clock.
-
+- phys: comphy for the ethernet port, see ../phy/phy-bindings.txt
 
 Optional properties (valid only for Armada XP/38x):
 
Signature · 1 line
2.7.4
Signature · 3 lines
RMK's Patch system: http://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
maintainer Reviewed-by
#12 lore raw

Re: [RFC PATCH 4/6] dt-bindings: update mvneta binding document

Quoted 11 lines · Russell King - ARM Linux
Quoted 4 lines · Rob Herring
Quoted 1 line · Russell King

Needs a better subject and a commit msg.

Hmm, not sure why it didn't contain:

"dt-bindings: net: mvneta: add phys property

Quoted 7 lines

Add an optional phys property to the mvneta binding documentation for the common phy.

as the commit message. With the correct commit message, are you happy with it?

Yes.

#13 lore raw

Re: [RFC PATCH 4/6] dt-bindings: update mvneta binding document

Hi Russell,

Quoted 16 lines · Rob Herring
Quoted 14 lines · Russell King - ARM Linux
Quoted 4 lines · Rob Herring
Quoted 1 line · Russell King

Needs a better subject and a commit msg.

Hmm, not sure why it didn't contain:

"dt-bindings: net: mvneta: add phys property

Add an optional phys property to the mvneta binding documentation for the common phy.

Quoted 8 lines
Quoted 4 lines

as the commit message. With the correct commit message, are you happy with it?

Yes.

Are you planning to resend this series?

Thanks
Kishon

#14 lore raw

Re: [RFC PATCH 4/6] dt-bindings: update mvneta binding document

Quoted 21 lines · Kishon Vijay Abraham I

Hi Russell,

Quoted 18 lines · Rob Herring
Quoted 16 lines · Russell King - ARM Linux
Quoted 4 lines · Rob Herring
Quoted 1 line · Russell King

Needs a better subject and a commit msg.

Hmm, not sure why it didn't contain:

"dt-bindings: net: mvneta: add phys property

Add an optional phys property to the mvneta binding documentation for the common phy.

Quoted 8 lines
Quoted 6 lines
Quoted 2 lines

as the commit message. With the correct commit message, are you happy with it?

Yes.

Are you planning to resend this series?

I'm not - you said previously that you had merged the first three patches into the phy tree, which are fine. The next two could be merged via netdev. However, we must avoid merging the last patch with patch 5 if the patches are going via different trees or mvneta will break.

Signature · 3 lines
RMK's Patch system: http://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
#15 lore raw

Re: [RFC PATCH 4/6] dt-bindings: update mvneta binding document

Russell,

Quoted 25 lines · Russell King - ARM Linux
Quoted 24 lines · Kishon Vijay Abraham I

Hi Russell,

Quoted 21 lines · Rob Herring
Quoted 18 lines · Russell King - ARM Linux
Quoted 4 lines · Rob Herring
Quoted 1 line · Russell King

Needs a better subject and a commit msg.

Hmm, not sure why it didn't contain:

"dt-bindings: net: mvneta: add phys property

Add an optional phys property to the mvneta binding documentation for the common phy.

as the commit message. With the correct commit message, are you happy with it?

Quoted 8 lines
Quoted 5 lines
Quoted 3 lines

Yes.

Are you planning to resend this series?

I'm not - you said previously that you had merged the first three patches into the phy tree, which are fine. The next two could be

No, I haven't merged patches from this series. That would have failed compilation since Grygorii modified enum phy_mode which is used in this series. You have also noted this in your cover letter.

Thanks
Kishon

#16 lore raw

Re: [RFC PATCH 4/6] dt-bindings: update mvneta binding document

Quoted 5 lines · Kishon Vijay Abraham I

Russell,

No, I haven't merged patches from this series. That would have failed compilation since Grygorii modified enum phy_mode which is used in this series. You have also noted this in your cover letter.

Ok, but in any case, given the complexities of modifying the patch and properly testing it, I think I'll wait until those changes have hit mainline before re-spinning this series. Alternatively, if you're happy to take just build-tested version, I could re-spin with that so at least we can get the phy bits queued for the merge window.

In any case, I'm busy trying to get to the bottom of several OMAP4 bugs while ill, so this isn't something I want to do at the moment.

Signature · 3 lines
RMK's Patch system: http://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
#17 lore raw

Re: [RFC PATCH 4/6] dt-bindings: update mvneta binding document

Hi,

Quoted 7 lines · Russell King - ARM Linux
Quoted 5 lines · Kishon Vijay Abraham I

Russell,

No, I haven't merged patches from this series. That would have failed compilation since Grygorii modified enum phy_mode which is used in this series. You have also noted this in your cover letter.

Quoted 6 lines

Ok, but in any case, given the complexities of modifying the patch and properly testing it, I think I'll wait until those changes have hit mainline before re-spinning this series. Alternatively, if you're happy to take just build-tested version, I could re-spin with that so at least we can get the phy bits queued for the merge window.

I'd prefer we test it before merging.

Quoted 3 lines

In any case, I'm busy trying to get to the bottom of several OMAP4 bugs while ill, so this isn't something I want to do at the moment.

Take care!

Thanks
Kishon

Quoted 1 line
#18 lore raw

Re: [RFC PATCH 4/6] dt-bindings: update mvneta binding document

Quoted 10 lines · Kishon Vijay Abraham I

Hi,

Quoted 7 lines · Russell King - ARM Linux
Quoted 5 lines · Kishon Vijay Abraham I

Russell,

No, I haven't merged patches from this series. That would have failed compilation since Grygorii modified enum phy_mode which is used in this series. You have also noted this in your cover letter.

Quoted 8 lines
Quoted 6 lines

Ok, but in any case, given the complexities of modifying the patch and properly testing it, I think I'll wait until those changes have hit mainline before re-spinning this series. Alternatively, if you're happy to take just build-tested version, I could re-spin with that so at least we can get the phy bits queued for the merge window.

I'd prefer we test it before merging.

Okay, expect it sometime after Christmas. In any case, waiting for the upheaval in the phy API to hit mainline will need to happen to that netdev is in sync with the revised phy API.

Signature · 3 lines
RMK's Patch system: http://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
#19 lore raw

RFC PATCH 5/6 net: marvell: neta: add support for 2500base-X

In reply to Russell King - ARM Linux
---
 drivers/net/ethernet/marvell/mvneta.c | 58 ++++++++++++++++++++++++++++++-----
 1 file changed, 51 insertions(+), 7 deletions(-)
drivers/net/ethernet/marvell/mvneta.c +51 -7
index 5bfd349bf41a..7305d4cc0630 100644
@@ -27,6 +27,7 @@
 #include <linux/of_irq.h>
 #include <linux/of_mdio.h>
 #include <linux/of_net.h>
+#include <linux/phy/phy.h>
 #include <linux/phy.h>
 #include <linux/phylink.h>
 #include <linux/platform_device.h>
@@ -437,6 +438,7 @@ struct mvneta_port {
 	struct device_node *dn;
 	unsigned int tx_csum_limit;
 	struct phylink *phylink;
+	struct phy *comphy;
 
 	struct mvneta_bm *bm_priv;
 	struct mvneta_bm_pool *pool_long;
@@ -3150,6 +3152,8 @@ static void mvneta_start_dev(struct mvneta_port *pp)
 {
 	int cpu;
 
+	WARN_ON(phy_power_on(pp->comphy));
+
 	mvneta_max_rx_size_set(pp, pp->pkt_size);
 	mvneta_txq_max_tx_size_set(pp, pp->pkt_size);
 
@@ -3212,6 +3216,8 @@ static void mvneta_stop_dev(struct mvneta_port *pp)
 
 	mvneta_tx_reset(pp);
 	mvneta_rx_reset(pp);
+
+	WARN_ON(phy_power_off(pp->comphy));
 }
 
 static void mvneta_percpu_enable(void *arg)
@@ -3337,6 +3343,7 @@ static int mvneta_set_mac_addr(struct net_device *dev, void *addr)
 static void mvneta_validate(struct net_device *ndev, unsigned long *supported,
 			    struct phylink_link_state *state)
 {
+	struct mvneta_port *pp = netdev_priv(ndev);
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
 
 	/* We only support QSGMII, SGMII, 802.3z and RGMII modes */
@@ -3357,14 +3364,14 @@ static void mvneta_validate(struct net_device *ndev, unsigned long *supported,
 	/* Asymmetric pause is unsupported */
 	phylink_set(mask, Pause);
 
-	/* We cannot use 1Gbps when using the 2.5G interface. */
-	if (state->interface == PHY_INTERFACE_MODE_2500BASEX) {
-		phylink_set(mask, 2500baseT_Full);
-		phylink_set(mask, 2500baseX_Full);
-	} else {
+	/* Half-duplex at speeds higher than 100Mbit is unsupported */
+	if (pp->comphy || state->interface != PHY_INTERFACE_MODE_2500BASEX) {
 		phylink_set(mask, 1000baseT_Full);
 		phylink_set(mask, 1000baseX_Full);
 	}
+	if (pp->comphy || state->interface == PHY_INTERFACE_MODE_2500BASEX) {
+		phylink_set(mask, 2500baseX_Full);
+	}
 
 	if (!phy_interface_mode_is_8023z(state->interface)) {
 		/* 10M and 100M are only supported in non-802.3z mode */
@@ -3378,6 +3385,11 @@ static void mvneta_validate(struct net_device *ndev, unsigned long *supported,
 		   __ETHTOOL_LINK_MODE_MASK_NBITS);
 	bitmap_and(state->advertising, state->advertising, mask,
 		   __ETHTOOL_LINK_MODE_MASK_NBITS);
+
+	/* We can only operate at 2500BaseX or 1000BaseX.  If requested
+	 * to advertise both, only report advertising at 2500BaseX.
+	 */
+	phylink_helper_basex_speed(state);
 }
 
 static int mvneta_mac_link_state(struct net_device *ndev,
@@ -3389,7 +3401,9 @@ static int mvneta_mac_link_state(struct net_device *ndev,
 	gmac_stat = mvreg_read(pp, MVNETA_GMAC_STATUS);
 
 	if (gmac_stat & MVNETA_GMAC_SPEED_1000)
-		state->speed = SPEED_1000;
+		state->speed =
+			state->interface == PHY_INTERFACE_MODE_2500BASEX ?
+			SPEED_2500 : SPEED_1000;
 	else if (gmac_stat & MVNETA_GMAC_SPEED_100)
 		state->speed = SPEED_100;
 	else
@@ -3504,12 +3518,32 @@ static void mvneta_mac_config(struct net_device *ndev, unsigned int mode,
 			    MVNETA_GMAC_FORCE_LINK_DOWN);
 	}
 
+
 	/* When at 2.5G, the link partner can send frames with shortened
 	 * preambles.
 	 */
 	if (state->speed == SPEED_2500)
 		new_ctrl4 |= MVNETA_GMAC4_SHORT_PREAMBLE_ENABLE;
 
+	if (pp->comphy) {
+		enum phy_mode mode = PHY_MODE_INVALID;
+
+		switch (state->interface) {
+		case PHY_INTERFACE_MODE_SGMII:
+		case PHY_INTERFACE_MODE_1000BASEX:
+			mode = PHY_MODE_SGMII;
+			break;
+		case PHY_INTERFACE_MODE_2500BASEX:
+			mode = PHY_MODE_2500SGMII;
+			break;
+		default:
+			break;
+		}
+
+		if (mode != PHY_MODE_INVALID)
+			WARN_ON(phy_set_mode(pp->comphy, mode));
+	}
+
 	if (new_ctrl0 != gmac_ctrl0)
 		mvreg_write(pp, MVNETA_GMAC_CTRL_0, new_ctrl0);
 	if (new_ctrl2 != gmac_ctrl2)
@@ -4411,7 +4445,7 @@ static int mvneta_port_power_up(struct mvneta_port *pp, int phy_mode)
 	if (phy_mode == PHY_INTERFACE_MODE_QSGMII)
 		mvreg_write(pp, MVNETA_SERDES_CFG, MVNETA_QSGMII_SERDES_PROTO);
 	else if (phy_mode == PHY_INTERFACE_MODE_SGMII ||
-		 phy_mode == PHY_INTERFACE_MODE_1000BASEX)
+		 phy_interface_mode_is_8023z(phy_mode))
 		mvreg_write(pp, MVNETA_SERDES_CFG, MVNETA_SGMII_SERDES_PROTO);
 	else if (!phy_interface_mode_is_rgmii(phy_mode))
 		return -EINVAL;
@@ -4428,6 +4462,7 @@ static int mvneta_probe(struct platform_device *pdev)
 	struct mvneta_port *pp;
 	struct net_device *dev;
 	struct phylink *phylink;
+	struct phy *comphy;
 	const char *dt_mac_addr;
 	char hw_mac_addr[ETH_ALEN];
 	const char *mac_from;
@@ -4453,6 +4488,14 @@ static int mvneta_probe(struct platform_device *pdev)
 		goto err_free_irq;
 	}
 
+	comphy = devm_of_phy_get(&pdev->dev, dn, NULL);
+	if (comphy == ERR_PTR(-EPROBE_DEFER)) {
+		err = -EPROBE_DEFER;
+		goto err_free_irq;
+	} else if (IS_ERR(comphy)) {
+		comphy = NULL;
+	}
+
 	phylink = phylink_create(dev, pdev->dev.fwnode, phy_mode,
 				 &mvneta_phylink_ops);
 	if (IS_ERR(phylink)) {
@@ -4469,6 +4512,7 @@ static int mvneta_probe(struct platform_device *pdev)
 	pp = netdev_priv(dev);
 	spin_lock_init(&pp->lock);
 	pp->phylink = phylink;
+	pp->comphy = comphy;
 	pp->phy_interface = phy_mode;
 	pp->dn = dn;
 
Signature · 1 line
2.7.4
#20 lore raw

Re: [RFC PATCH 5/6] net: marvell: neta: add support for 2500base-X

Hi,

Quoted 145 lines · Russell King
---
 drivers/net/ethernet/marvell/mvneta.c | 58 ++++++++++++++++++++++++++++++-----
 1 file changed, 51 insertions(+), 7 deletions(-)
drivers/net/ethernet/marvell/mvneta.c +42 -7
index 5bfd349bf41a..7305d4cc0630 100644
@@ -27,6 +27,7 @@
 #include <linux/of_irq.h>
 #include <linux/of_mdio.h>
 #include <linux/of_net.h>
+#include <linux/phy/phy.h>
 #include <linux/phy.h>
 #include <linux/phylink.h>
 #include <linux/platform_device.h>
@@ -437,6 +438,7 @@ struct mvneta_port {
 	struct device_node *dn;
 	unsigned int tx_csum_limit;
 	struct phylink *phylink;
+	struct phy *comphy;
 
 	struct mvneta_bm *bm_priv;
 	struct mvneta_bm_pool *pool_long;
@@ -3150,6 +3152,8 @@ static void mvneta_start_dev(struct mvneta_port *pp)
 {
 	int cpu;
 
+	WARN_ON(phy_power_on(pp->comphy));
+
 	mvneta_max_rx_size_set(pp, pp->pkt_size);
 	mvneta_txq_max_tx_size_set(pp, pp->pkt_size);
 
@@ -3212,6 +3216,8 @@ static void mvneta_stop_dev(struct mvneta_port *pp)
 
 	mvneta_tx_reset(pp);
 	mvneta_rx_reset(pp);
+
+	WARN_ON(phy_power_off(pp->comphy));
 }
 
 static void mvneta_percpu_enable(void *arg)
@@ -3337,6 +3343,7 @@ static int mvneta_set_mac_addr(struct net_device *dev, void *addr)
 static void mvneta_validate(struct net_device *ndev, unsigned long *supported,
 			    struct phylink_link_state *state)
 {
+	struct mvneta_port *pp = netdev_priv(ndev);
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
 
 	/* We only support QSGMII, SGMII, 802.3z and RGMII modes */
@@ -3357,14 +3364,14 @@ static void mvneta_validate(struct net_device *ndev, unsigned long *supported,
 	/* Asymmetric pause is unsupported */
 	phylink_set(mask, Pause);
 
-	/* We cannot use 1Gbps when using the 2.5G interface. */
-	if (state->interface == PHY_INTERFACE_MODE_2500BASEX) {
-		phylink_set(mask, 2500baseT_Full);
-		phylink_set(mask, 2500baseX_Full);
-	} else {
+	/* Half-duplex at speeds higher than 100Mbit is unsupported */
+	if (pp->comphy || state->interface != PHY_INTERFACE_MODE_2500BASEX) {
 		phylink_set(mask, 1000baseT_Full);
 		phylink_set(mask, 1000baseX_Full);
 	}
+	if (pp->comphy || state->interface == PHY_INTERFACE_MODE_2500BASEX) {
+		phylink_set(mask, 2500baseX_Full);
+	}
 
 	if (!phy_interface_mode_is_8023z(state->interface)) {
 		/* 10M and 100M are only supported in non-802.3z mode */
@@ -3378,6 +3385,11 @@ static void mvneta_validate(struct net_device *ndev, unsigned long *supported,
 		   __ETHTOOL_LINK_MODE_MASK_NBITS);
 	bitmap_and(state->advertising, state->advertising, mask,
 		   __ETHTOOL_LINK_MODE_MASK_NBITS);
+
+	/* We can only operate at 2500BaseX or 1000BaseX.  If requested
+	 * to advertise both, only report advertising at 2500BaseX.
+	 */
+	phylink_helper_basex_speed(state);
 }
 
 static int mvneta_mac_link_state(struct net_device *ndev,
@@ -3389,7 +3401,9 @@ static int mvneta_mac_link_state(struct net_device *ndev,
 	gmac_stat = mvreg_read(pp, MVNETA_GMAC_STATUS);
 
 	if (gmac_stat & MVNETA_GMAC_SPEED_1000)
-		state->speed = SPEED_1000;
+		state->speed =
+			state->interface == PHY_INTERFACE_MODE_2500BASEX ?
+			SPEED_2500 : SPEED_1000;
 	else if (gmac_stat & MVNETA_GMAC_SPEED_100)
 		state->speed = SPEED_100;
 	else
@@ -3504,12 +3518,32 @@ static void mvneta_mac_config(struct net_device *ndev, unsigned int mode,
 			    MVNETA_GMAC_FORCE_LINK_DOWN);
 	}
 
+
 	/* When at 2.5G, the link partner can send frames with shortened
 	 * preambles.
 	 */
 	if (state->speed == SPEED_2500)
 		new_ctrl4 |= MVNETA_GMAC4_SHORT_PREAMBLE_ENABLE;
 
+	if (pp->comphy) {
+		enum phy_mode mode = PHY_MODE_INVALID;
+
+		switch (state->interface) {
+		case PHY_INTERFACE_MODE_SGMII:
+		case PHY_INTERFACE_MODE_1000BASEX:
+			mode = PHY_MODE_SGMII;
+			break;
+		case PHY_INTERFACE_MODE_2500BASEX:
+			mode = PHY_MODE_2500SGMII;
+			break;
+		default:
+			break;
+		}
+
+		if (mode != PHY_MODE_INVALID)
+			WARN_ON(phy_set_mode(pp->comphy, mode));
+	}
+
 	if (new_ctrl0 != gmac_ctrl0)
 		mvreg_write(pp, MVNETA_GMAC_CTRL_0, new_ctrl0);
 	if (new_ctrl2 != gmac_ctrl2)
@@ -4411,7 +4445,7 @@ static int mvneta_port_power_up(struct mvneta_port *pp, int phy_mode)
 	if (phy_mode == PHY_INTERFACE_MODE_QSGMII)
 		mvreg_write(pp, MVNETA_SERDES_CFG, MVNETA_QSGMII_SERDES_PROTO);
 	else if (phy_mode == PHY_INTERFACE_MODE_SGMII ||
-		 phy_mode == PHY_INTERFACE_MODE_1000BASEX)
+		 phy_interface_mode_is_8023z(phy_mode))
 		mvreg_write(pp, MVNETA_SERDES_CFG, MVNETA_SGMII_SERDES_PROTO);
 	else if (!phy_interface_mode_is_rgmii(phy_mode))
 		return -EINVAL;
@@ -4428,6 +4462,7 @@ static int mvneta_probe(struct platform_device *pdev)
 	struct mvneta_port *pp;
 	struct net_device *dev;
 	struct phylink *phylink;
+	struct phy *comphy;
 	const char *dt_mac_addr;
 	char hw_mac_addr[ETH_ALEN];
 	const char *mac_from;

drivers/net/ethernet/marvell/mvneta.c static int mvneta_probe(struct platform_device *pdev) · line 4497

Quoted 10 lines
drivers/net/ethernet/marvell/mvneta.c +7 -0
@@ -4453,6 +4488,14 @@ static int mvneta_probe(struct platform_device *pdev)
 		goto err_free_irq;
 	}
 
+	comphy = devm_of_phy_get(&pdev->dev, dn, NULL);
+	if (comphy == ERR_PTR(-EPROBE_DEFER)) {
+		err = -EPROBE_DEFER;
+		goto err_free_irq;
+	} else if (IS_ERR(comphy)) {
+		comphy = NULL;
+	}

devm_phy_optional_get can be used here instead.

Thanks
Kishon

#21 lore raw

Re: [RFC PATCH 5/6] net: marvell: neta: add support for 2500base-X

Quoted 151 lines · Kishon Vijay Abraham I

Hi,

Quoted 148 lines · Russell King
---
 drivers/net/ethernet/marvell/mvneta.c | 58 ++++++++++++++++++++++++++++++-----
 1 file changed, 51 insertions(+), 7 deletions(-)
drivers/net/ethernet/marvell/mvneta.c +42 -7
index 5bfd349bf41a..7305d4cc0630 100644
@@ -27,6 +27,7 @@
 #include <linux/of_irq.h>
 #include <linux/of_mdio.h>
 #include <linux/of_net.h>
+#include <linux/phy/phy.h>
 #include <linux/phy.h>
 #include <linux/phylink.h>
 #include <linux/platform_device.h>
@@ -437,6 +438,7 @@ struct mvneta_port {
 	struct device_node *dn;
 	unsigned int tx_csum_limit;
 	struct phylink *phylink;
+	struct phy *comphy;
 
 	struct mvneta_bm *bm_priv;
 	struct mvneta_bm_pool *pool_long;
@@ -3150,6 +3152,8 @@ static void mvneta_start_dev(struct mvneta_port *pp)
 {
 	int cpu;
 
+	WARN_ON(phy_power_on(pp->comphy));
+
 	mvneta_max_rx_size_set(pp, pp->pkt_size);
 	mvneta_txq_max_tx_size_set(pp, pp->pkt_size);
 
@@ -3212,6 +3216,8 @@ static void mvneta_stop_dev(struct mvneta_port *pp)
 
 	mvneta_tx_reset(pp);
 	mvneta_rx_reset(pp);
+
+	WARN_ON(phy_power_off(pp->comphy));
 }
 
 static void mvneta_percpu_enable(void *arg)
@@ -3337,6 +3343,7 @@ static int mvneta_set_mac_addr(struct net_device *dev, void *addr)
 static void mvneta_validate(struct net_device *ndev, unsigned long *supported,
 			    struct phylink_link_state *state)
 {
+	struct mvneta_port *pp = netdev_priv(ndev);
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
 
 	/* We only support QSGMII, SGMII, 802.3z and RGMII modes */
@@ -3357,14 +3364,14 @@ static void mvneta_validate(struct net_device *ndev, unsigned long *supported,
 	/* Asymmetric pause is unsupported */
 	phylink_set(mask, Pause);
 
-	/* We cannot use 1Gbps when using the 2.5G interface. */
-	if (state->interface == PHY_INTERFACE_MODE_2500BASEX) {
-		phylink_set(mask, 2500baseT_Full);
-		phylink_set(mask, 2500baseX_Full);
-	} else {
+	/* Half-duplex at speeds higher than 100Mbit is unsupported */
+	if (pp->comphy || state->interface != PHY_INTERFACE_MODE_2500BASEX) {
 		phylink_set(mask, 1000baseT_Full);
 		phylink_set(mask, 1000baseX_Full);
 	}
+	if (pp->comphy || state->interface == PHY_INTERFACE_MODE_2500BASEX) {
+		phylink_set(mask, 2500baseX_Full);
+	}
 
 	if (!phy_interface_mode_is_8023z(state->interface)) {
 		/* 10M and 100M are only supported in non-802.3z mode */
@@ -3378,6 +3385,11 @@ static void mvneta_validate(struct net_device *ndev, unsigned long *supported,
 		   __ETHTOOL_LINK_MODE_MASK_NBITS);
 	bitmap_and(state->advertising, state->advertising, mask,
 		   __ETHTOOL_LINK_MODE_MASK_NBITS);
+
+	/* We can only operate at 2500BaseX or 1000BaseX.  If requested
+	 * to advertise both, only report advertising at 2500BaseX.
+	 */
+	phylink_helper_basex_speed(state);
 }
 
 static int mvneta_mac_link_state(struct net_device *ndev,
@@ -3389,7 +3401,9 @@ static int mvneta_mac_link_state(struct net_device *ndev,
 	gmac_stat = mvreg_read(pp, MVNETA_GMAC_STATUS);
 
 	if (gmac_stat & MVNETA_GMAC_SPEED_1000)
-		state->speed = SPEED_1000;
+		state->speed =
+			state->interface == PHY_INTERFACE_MODE_2500BASEX ?
+			SPEED_2500 : SPEED_1000;
 	else if (gmac_stat & MVNETA_GMAC_SPEED_100)
 		state->speed = SPEED_100;
 	else
@@ -3504,12 +3518,32 @@ static void mvneta_mac_config(struct net_device *ndev, unsigned int mode,
 			    MVNETA_GMAC_FORCE_LINK_DOWN);
 	}
 
+
 	/* When at 2.5G, the link partner can send frames with shortened
 	 * preambles.
 	 */
 	if (state->speed == SPEED_2500)
 		new_ctrl4 |= MVNETA_GMAC4_SHORT_PREAMBLE_ENABLE;
 
+	if (pp->comphy) {
+		enum phy_mode mode = PHY_MODE_INVALID;
+
+		switch (state->interface) {
+		case PHY_INTERFACE_MODE_SGMII:
+		case PHY_INTERFACE_MODE_1000BASEX:
+			mode = PHY_MODE_SGMII;
+			break;
+		case PHY_INTERFACE_MODE_2500BASEX:
+			mode = PHY_MODE_2500SGMII;
+			break;
+		default:
+			break;
+		}
+
+		if (mode != PHY_MODE_INVALID)
+			WARN_ON(phy_set_mode(pp->comphy, mode));
+	}
+
 	if (new_ctrl0 != gmac_ctrl0)
 		mvreg_write(pp, MVNETA_GMAC_CTRL_0, new_ctrl0);
 	if (new_ctrl2 != gmac_ctrl2)
@@ -4411,7 +4445,7 @@ static int mvneta_port_power_up(struct mvneta_port *pp, int phy_mode)
 	if (phy_mode == PHY_INTERFACE_MODE_QSGMII)
 		mvreg_write(pp, MVNETA_SERDES_CFG, MVNETA_QSGMII_SERDES_PROTO);
 	else if (phy_mode == PHY_INTERFACE_MODE_SGMII ||
-		 phy_mode == PHY_INTERFACE_MODE_1000BASEX)
+		 phy_interface_mode_is_8023z(phy_mode))
 		mvreg_write(pp, MVNETA_SERDES_CFG, MVNETA_SGMII_SERDES_PROTO);
 	else if (!phy_interface_mode_is_rgmii(phy_mode))
 		return -EINVAL;
@@ -4428,6 +4462,7 @@ static int mvneta_probe(struct platform_device *pdev)
 	struct mvneta_port *pp;
 	struct net_device *dev;
 	struct phylink *phylink;
+	struct phy *comphy;
 	const char *dt_mac_addr;
 	char hw_mac_addr[ETH_ALEN];
 	const char *mac_from;
@@ -4453,6 +4488,14 @@ static int mvneta_probe(struct platform_device *pdev)
 		goto err_free_irq;
 	}
 
Quoted 9 lines

drivers/net/ethernet/marvell/mvneta.c static int mvneta_probe(struct platform_device *pdev) · line 4497

Quoted 7 lines
drivers/net/ethernet/marvell/mvneta.c +7 -0
+	comphy = devm_of_phy_get(&pdev->dev, dn, NULL);
+	if (comphy == ERR_PTR(-EPROBE_DEFER)) {
+		err = -EPROBE_DEFER;
+		goto err_free_irq;
+	} else if (IS_ERR(comphy)) {
+		comphy = NULL;
+	}

devm_phy_optional_get can be used here instead.

I don't think that will work with a NULL string.

devm_phy_optional_get() ultimately ends up calling phy_get(), which in this case would receive a NULL string. It will pass that NULL string to of_property_match_string().

of_property_match_string() will try to find the "phy-names" property, which will not exist, and hence will return -EINVAL.

phy_get() doesn't check for error conditions, but passes this directly to _of_phy_get() as the index. _of_phy_get() passes that on to of_parse_phandle_with_args(), which will fail to find an entry with cur_index == -EINVAL (since it counts up from zero.) Hence, _of_phy_get() will return -ENODEV, thereby causing devm_phy_optional_get() to return NULL, even if there's a phys= property present.

of_phy_get() and phy_get() have different behaviours when a NULL string is passed in - the of_phy_get() family will get the first PHY specified in the DT phys= property, whereas the phy_get() family of functions will fail.

Since there is no devm_of_phy_optional_get(), that leads people down the path of coding that functionality at the callsites, such as in drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c.

Signature · 3 lines
RMK's Patch system: http://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
#22 lore raw

Re: [RFC PATCH 5/6] net: marvell: neta: add support for 2500base-X

Hi,

Quoted 162 lines · Russell King - ARM Linux
Quoted 151 lines · Kishon Vijay Abraham I

Hi,

Quoted 148 lines · Russell King
---
 drivers/net/ethernet/marvell/mvneta.c | 58 ++++++++++++++++++++++++++++++-----
 1 file changed, 51 insertions(+), 7 deletions(-)
drivers/net/ethernet/marvell/mvneta.c +42 -7
index 5bfd349bf41a..7305d4cc0630 100644
@@ -27,6 +27,7 @@
 #include <linux/of_irq.h>
 #include <linux/of_mdio.h>
 #include <linux/of_net.h>
+#include <linux/phy/phy.h>
 #include <linux/phy.h>
 #include <linux/phylink.h>
 #include <linux/platform_device.h>
@@ -437,6 +438,7 @@ struct mvneta_port {
 	struct device_node *dn;
 	unsigned int tx_csum_limit;
 	struct phylink *phylink;
+	struct phy *comphy;
 
 	struct mvneta_bm *bm_priv;
 	struct mvneta_bm_pool *pool_long;
@@ -3150,6 +3152,8 @@ static void mvneta_start_dev(struct mvneta_port *pp)
 {
 	int cpu;
 
+	WARN_ON(phy_power_on(pp->comphy));
+
 	mvneta_max_rx_size_set(pp, pp->pkt_size);
 	mvneta_txq_max_tx_size_set(pp, pp->pkt_size);
 
@@ -3212,6 +3216,8 @@ static void mvneta_stop_dev(struct mvneta_port *pp)
 
 	mvneta_tx_reset(pp);
 	mvneta_rx_reset(pp);
+
+	WARN_ON(phy_power_off(pp->comphy));
 }
 
 static void mvneta_percpu_enable(void *arg)
@@ -3337,6 +3343,7 @@ static int mvneta_set_mac_addr(struct net_device *dev, void *addr)
 static void mvneta_validate(struct net_device *ndev, unsigned long *supported,
 			    struct phylink_link_state *state)
 {
+	struct mvneta_port *pp = netdev_priv(ndev);
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
 
 	/* We only support QSGMII, SGMII, 802.3z and RGMII modes */
@@ -3357,14 +3364,14 @@ static void mvneta_validate(struct net_device *ndev, unsigned long *supported,
 	/* Asymmetric pause is unsupported */
 	phylink_set(mask, Pause);
 
-	/* We cannot use 1Gbps when using the 2.5G interface. */
-	if (state->interface == PHY_INTERFACE_MODE_2500BASEX) {
-		phylink_set(mask, 2500baseT_Full);
-		phylink_set(mask, 2500baseX_Full);
-	} else {
+	/* Half-duplex at speeds higher than 100Mbit is unsupported */
+	if (pp->comphy || state->interface != PHY_INTERFACE_MODE_2500BASEX) {
 		phylink_set(mask, 1000baseT_Full);
 		phylink_set(mask, 1000baseX_Full);
 	}
+	if (pp->comphy || state->interface == PHY_INTERFACE_MODE_2500BASEX) {
+		phylink_set(mask, 2500baseX_Full);
+	}
 
 	if (!phy_interface_mode_is_8023z(state->interface)) {
 		/* 10M and 100M are only supported in non-802.3z mode */
@@ -3378,6 +3385,11 @@ static void mvneta_validate(struct net_device *ndev, unsigned long *supported,
 		   __ETHTOOL_LINK_MODE_MASK_NBITS);
 	bitmap_and(state->advertising, state->advertising, mask,
 		   __ETHTOOL_LINK_MODE_MASK_NBITS);
+
+	/* We can only operate at 2500BaseX or 1000BaseX.  If requested
+	 * to advertise both, only report advertising at 2500BaseX.
+	 */
+	phylink_helper_basex_speed(state);
 }
 
 static int mvneta_mac_link_state(struct net_device *ndev,
@@ -3389,7 +3401,9 @@ static int mvneta_mac_link_state(struct net_device *ndev,
 	gmac_stat = mvreg_read(pp, MVNETA_GMAC_STATUS);
 
 	if (gmac_stat & MVNETA_GMAC_SPEED_1000)
-		state->speed = SPEED_1000;
+		state->speed =
+			state->interface == PHY_INTERFACE_MODE_2500BASEX ?
+			SPEED_2500 : SPEED_1000;
 	else if (gmac_stat & MVNETA_GMAC_SPEED_100)
 		state->speed = SPEED_100;
 	else
@@ -3504,12 +3518,32 @@ static void mvneta_mac_config(struct net_device *ndev, unsigned int mode,
 			    MVNETA_GMAC_FORCE_LINK_DOWN);
 	}
 
+
 	/* When at 2.5G, the link partner can send frames with shortened
 	 * preambles.
 	 */
 	if (state->speed == SPEED_2500)
 		new_ctrl4 |= MVNETA_GMAC4_SHORT_PREAMBLE_ENABLE;
 
+	if (pp->comphy) {
+		enum phy_mode mode = PHY_MODE_INVALID;
+
+		switch (state->interface) {
+		case PHY_INTERFACE_MODE_SGMII:
+		case PHY_INTERFACE_MODE_1000BASEX:
+			mode = PHY_MODE_SGMII;
+			break;
+		case PHY_INTERFACE_MODE_2500BASEX:
+			mode = PHY_MODE_2500SGMII;
+			break;
+		default:
+			break;
+		}
+
+		if (mode != PHY_MODE_INVALID)
+			WARN_ON(phy_set_mode(pp->comphy, mode));
+	}
+
 	if (new_ctrl0 != gmac_ctrl0)
 		mvreg_write(pp, MVNETA_GMAC_CTRL_0, new_ctrl0);
 	if (new_ctrl2 != gmac_ctrl2)
@@ -4411,7 +4445,7 @@ static int mvneta_port_power_up(struct mvneta_port *pp, int phy_mode)
 	if (phy_mode == PHY_INTERFACE_MODE_QSGMII)
 		mvreg_write(pp, MVNETA_SERDES_CFG, MVNETA_QSGMII_SERDES_PROTO);
 	else if (phy_mode == PHY_INTERFACE_MODE_SGMII ||
-		 phy_mode == PHY_INTERFACE_MODE_1000BASEX)
+		 phy_interface_mode_is_8023z(phy_mode))
 		mvreg_write(pp, MVNETA_SERDES_CFG, MVNETA_SGMII_SERDES_PROTO);
 	else if (!phy_interface_mode_is_rgmii(phy_mode))
 		return -EINVAL;
@@ -4428,6 +4462,7 @@ static int mvneta_probe(struct platform_device *pdev)
 	struct mvneta_port *pp;
 	struct net_device *dev;
 	struct phylink *phylink;
+	struct phy *comphy;
 	const char *dt_mac_addr;
 	char hw_mac_addr[ETH_ALEN];
 	const char *mac_from;
@@ -4453,6 +4488,14 @@ static int mvneta_probe(struct platform_device *pdev)
 		goto err_free_irq;
 	}
 
Quoted 9 lines

drivers/net/ethernet/marvell/mvneta.c static int mvneta_probe(struct platform_device *pdev) · line 4497

Quoted 7 lines
drivers/net/ethernet/marvell/mvneta.c +7 -0
+	comphy = devm_of_phy_get(&pdev->dev, dn, NULL);
+	if (comphy == ERR_PTR(-EPROBE_DEFER)) {
+		err = -EPROBE_DEFER;
+		goto err_free_irq;
+	} else if (IS_ERR(comphy)) {
+		comphy = NULL;
+	}

devm_phy_optional_get can be used here instead.

Quoted 1 line

I don't think that will work with a NULL string.

That's correct.

Quoted 4 lines

devm_phy_optional_get() ultimately ends up calling phy_get(), which in this case would receive a NULL string. It will pass that NULL string to of_property_match_string().

There is a check for (string == NULL), even before of_property_match_string is invoked in phy_get()

Quoted 12 lines

of_property_match_string() will try to find the "phy-names" property, which will not exist, and hence will return -EINVAL.

phy_get() doesn't check for error conditions, but passes this directly to _of_phy_get() as the index. _of_phy_get() passes that on to of_parse_phandle_with_args(), which will fail to find an entry with cur_index == -EINVAL (since it counts up from zero.) Hence, _of_phy_get() will return -ENODEV, thereby causing devm_phy_optional_get() to return NULL, even if there's a phys= property present.

Quoted 4 lines

of_phy_get() and phy_get() have different behaviours when a NULL string is passed in - the of_phy_get() family will get the first PHY specified in the DT phys= property, whereas the phy_get() family of functions will fail.

Agreed. I'll fix this so that we have similar behavior with of_phy_get() and phy_get().

Quoted 4 lines

Since there is no devm_of_phy_optional_get(), that leads people down the path of coding that functionality at the callsites, such as in drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c.

Okay.

Thanks
Kishon

Quoted 1 line
Tested-by on v1 5/6
#23 lore raw

Re: [RFC PATCH 5/6] net: marvell: neta: add support for 2500base-X

In reply to Russell King

Hi Russell,

On Mon, 12 Nov 2018 12:31:07 +0000
Russell King <[email protected]> wrote:

Maybe missing a commit log ?

Otherwise,

Quoted 1 line
Signature · 3 lines
Maxime Chevallier, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
#24 lore raw

RFC PATCH 6/6 ARM: dts: clearfog: add comphy settings for Ethernet interfaces

In reply to Russell King - ARM Linux

Add the comphy settings for the Ethernet interfaces.

---
 arch/arm/boot/dts/armada-388-clearfog.dtsi | 2 ++
 1 file changed, 2 insertions(+)
arch/arm/boot/dts/armada-388-clearfog.dtsi +2 -0
index 1b0d0680c8b6..0d81600ca247 100644
@@ -93,6 +93,7 @@
 	bm,pool-long = <2>;
 	bm,pool-short = <1>;
 	buffer-manager = <&bm>;
+	phys = <&comphy1 1>;
 	phy-mode = "sgmii";
 	status = "okay";
 };
@@ -103,6 +104,7 @@
 	bm,pool-short = <1>;
 	buffer-manager = <&bm>;
 	managed = "in-band-status";
+	phys = <&comphy5 2>;
 	phy-mode = "sgmii";
 	sfp = <&sfp>;
 	status = "okay";
Signature · 1 line
2.7.4
#26 lore raw
In reply to Russell King - ARM Linux

Hi,

Quoted 20 lines · Russell King - ARM Linux

Hi,

This series adds support for dynamically switching between 1Gbps and 2.5Gbps networking for the Marvell Armada 38x SoCs, tested on Armada 388 on the Clearfog platform.

This is necessary to be able to connect (eg) a Clearfog platform with a Macchiatobin platform via the SFP sockets, as Clearfog currently only supports 1Gbps networking via the SFP socket and Macchiatobin defaults to 2.5Gbps when using Fiberchannel SFPs.

In order to allow dynamic switching, we need to implement a common phy driver to switch the ethernet serdes lane speed - 2.5Gbps is just 1Gbps up-clocked by 2.5x. We implement a simple comphy driver to achieve this, which only supports networking.

With this, we are able to support both Fiberchannel SFPs operating at 2.5Gbps or 1Gbps, and 1G ethernet SFPs plugged into the Clearfog platform, dynamically selecting according to the SFPs abilities.

Quoted 5 lines

I'm aware of the proposed changes to the PHY layer, changing phy_set_mode() to take the ethernet phy interface type, hence why this is RFC - there's also the question about how this will be merged. This series is currently based on 4.20-rc1, but will likely need to be rebased when the PHY layer changes hit.

For this case, I'd prefer the phy_set_mode series and the phy and net changes here (after rebasing) go via linux-phy tree.

Thanks
Kishon

#27 lore raw
Quoted 23 lines · Kishon Vijay Abraham I

Hi,

Quoted 20 lines · Russell King - ARM Linux

Hi,

This series adds support for dynamically switching between 1Gbps and 2.5Gbps networking for the Marvell Armada 38x SoCs, tested on Armada 388 on the Clearfog platform.

This is necessary to be able to connect (eg) a Clearfog platform with a Macchiatobin platform via the SFP sockets, as Clearfog currently only supports 1Gbps networking via the SFP socket and Macchiatobin defaults to 2.5Gbps when using Fiberchannel SFPs.

In order to allow dynamic switching, we need to implement a common phy driver to switch the ethernet serdes lane speed - 2.5Gbps is just 1Gbps up-clocked by 2.5x. We implement a simple comphy driver to achieve this, which only supports networking.

With this, we are able to support both Fiberchannel SFPs operating at 2.5Gbps or 1Gbps, and 1G ethernet SFPs plugged into the Clearfog platform, dynamically selecting according to the SFPs abilities.

Quoted 8 lines
Quoted 5 lines

I'm aware of the proposed changes to the PHY layer, changing phy_set_mode() to take the ethernet phy interface type, hence why this is RFC - there's also the question about how this will be merged. This series is currently based on 4.20-rc1, but will likely need to be rebased when the PHY layer changes hit.

For this case, I'd prefer the phy_set_mode series and the phy and net changes here (after rebasing) go via linux-phy tree.

Please let me know when they've hit, thanks.

Signature · 3 lines
RMK's Patch system: http://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
#28 lore raw

Hi Russell,

Quoted 30 lines · Russell King - ARM Linux
Quoted 29 lines · Kishon Vijay Abraham I

Hi,

Quoted 25 lines · Russell King - ARM Linux

Hi,

This series adds support for dynamically switching between 1Gbps and 2.5Gbps networking for the Marvell Armada 38x SoCs, tested on Armada 388 on the Clearfog platform.

This is necessary to be able to connect (eg) a Clearfog platform with a Macchiatobin platform via the SFP sockets, as Clearfog currently only supports 1Gbps networking via the SFP socket and Macchiatobin defaults to 2.5Gbps when using Fiberchannel SFPs.

In order to allow dynamic switching, we need to implement a common phy driver to switch the ethernet serdes lane speed - 2.5Gbps is just 1Gbps up-clocked by 2.5x. We implement a simple comphy driver to achieve this, which only supports networking.

With this, we are able to support both Fiberchannel SFPs operating at 2.5Gbps or 1Gbps, and 1G ethernet SFPs plugged into the Clearfog platform, dynamically selecting according to the SFPs abilities.

I'm aware of the proposed changes to the PHY layer, changing phy_set_mode() to take the ethernet phy interface type, hence why this is RFC - there's also the question about how this will be merged. This series is currently based on 4.20-rc1, but will likely need to be rebased when the PHY layer changes hit.

Quoted 4 lines
Quoted 2 lines

For this case, I'd prefer the phy_set_mode series and the phy and net changes here (after rebasing) go via linux-phy tree.

Please let me know when they've hit, thanks.

I've merged the phy_set_mode series to linux-phy -next.

Thanks
Kishon

Quoted 1 line
#29 lore raw
In reply to Russell King - ARM Linux

Hello Russell,

Quoted 5 lines · On Mon, 12 Nov 2018 12:29:33 +0000 Russell King - ARM Linux…

Hi,

This series adds support for dynamically switching between 1Gbps and 2.5Gbps networking for the Marvell Armada 38x SoCs, tested on Armada 388 on the Clearfog platform.

I've performed some tests on my side on a Clearfog too, and also on an Armada 385 board. It works fine on both setups.

Thanks for implementing this,

Maxime