[RFC PATCH 0/6] Armada 38x comphy driver to support 2.5Gbps networking

[RFC PATCH 0/6] Armada 38x comphy driver to support 2.5Gbps networking

[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.

 .../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


-- 
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

[RFC PATCH 0/6] Armada 38x comphy driver to support 2.5Gbps networking

[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.

 .../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


-- 
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

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

[Russell King]

Add the Marvell Armada 38x common phy bindings.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 .../bindings/phy/phy-armada38x-comphy.txt          | 40 ++++++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/phy-armada38x-comphy.txt

diff --git a/Documentation/devicetree/bindings/phy/phy-armada38x-comphy.txt b/Documentation/devicetree/bindings/phy/phy-armada38x-comphy.txt
new file mode 100644
index 000000000000..ad49e5c01334
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/phy-armada38x-comphy.txt
@@ -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@18300 {
+		compatible = "marvell,armada-380-comphy";
+		reg = <0x18300 0x100>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		cpm_comphy0: phy@0 {
+			reg = <0>;
+			#phy-cells = <1>;
+		};
+
+		cpm_comphy1: phy@1 {
+			reg = <1>;
+			#phy-cells = <1>;
+		};
+	};
-- 
2.7.4

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

[Russell King]

Add the Marvell Armada 38x common phy bindings.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 .../bindings/phy/phy-armada38x-comphy.txt          | 40 ++++++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/phy-armada38x-comphy.txt

diff --git a/Documentation/devicetree/bindings/phy/phy-armada38x-comphy.txt b/Documentation/devicetree/bindings/phy/phy-armada38x-comphy.txt
new file mode 100644
index 000000000000..ad49e5c01334
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/phy-armada38x-comphy.txt
@@ -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>;
+		};
+	};
-- 
2.7.4

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

[Rob Herring]

On Mon, 12 Nov 2018 12:30:47 +0000, Russell King wrote:
> Add the Marvell Armada 38x common phy bindings.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  .../bindings/phy/phy-armada38x-comphy.txt          | 40 ++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/phy-armada38x-comphy.txt
> 

Reviewed-by: Rob Herring <robh@kernel.org>

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

[Rob Herring]

On Mon, 12 Nov 2018 12:30:47 +0000, Russell King wrote:
> Add the Marvell Armada 38x common phy bindings.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  .../bindings/phy/phy-armada38x-comphy.txt          | 40 ++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/phy-armada38x-comphy.txt
> 

Reviewed-by: Rob Herring <robh@kernel.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[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.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 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

diff --git a/drivers/phy/marvell/Kconfig b/drivers/phy/marvell/Kconfig
index 6fb4b56e4c14..224ea4e6a46d 100644
--- a/drivers/phy/marvell/Kconfig
+++ b/drivers/phy/marvell/Kconfig
@@ -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
diff --git a/drivers/phy/marvell/Makefile b/drivers/phy/marvell/Makefile
index 3975b144f8ec..59b6c03ef756 100644
--- a/drivers/phy/marvell/Makefile
+++ b/drivers/phy/marvell/Makefile
@@ -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
diff --git a/drivers/phy/marvell/phy-armada38x-comphy.c b/drivers/phy/marvell/phy-armada38x-comphy.c
new file mode 100644
index 000000000000..61d1965e1cf6
--- /dev/null
+++ b/drivers/phy/marvell/phy-armada38x-comphy.c
@@ -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
+ * <antoine.tenart@bootlin.com>
+ */
+#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;
+
+	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;
+	}
+
+	switch (mode) {
+	case PHY_MODE_SGMII:
+		gen = GEN_SGMII_1_25GBPS;
+		break;
+
+	case PHY_MODE_2500SGMII:
+		gen = GEN_SGMII_3_125GBPS;
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	a38x_comphy_set_speed(lane, gen, gen);
+
+	return a38x_comphy_poll(lane, COMPHY_STAT1,
+				COMPHY_STAT1_PLL_RDY_TX |
+				COMPHY_STAT1_PLL_RDY_RX,
+				COMPHY_STAT1_PLL_RDY_TX |
+				COMPHY_STAT1_PLL_RDY_RX);
+}
+
+static const struct phy_ops a38x_comphy_ops = {
+	.set_mode	= a38x_comphy_set_mode,
+	.owner		= THIS_MODULE,
+};
+
+static struct phy *a38x_comphy_xlate(struct device *dev,
+				     struct of_phandle_args *args)
+{
+	struct a38x_comphy_lane *lane;
+	struct phy *phy;
+
+	if (WARN_ON(args->args[0] >= MAX_A38X_PORTS))
+		return ERR_PTR(-EINVAL);
+
+	phy = of_phy_simple_xlate(dev, args);
+	if (IS_ERR(phy))
+		return phy;
+
+	lane = phy_get_drvdata(phy);
+	if (lane->port >= 0)
+		return ERR_PTR(-EBUSY);
+
+	lane->port = args->args[0];
+
+	return phy;
+}
+
+static int a38x_comphy_probe(struct platform_device *pdev)
+{
+	struct a38x_comphy *priv;
+	struct phy_provider *provider;
+	struct device_node *child;
+	struct resource *res;
+	void __iomem *base;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	priv->dev = &pdev->dev;
+	priv->base = base;
+
+	for_each_available_child_of_node(pdev->dev.of_node, child) {
+		struct phy *phy;
+		int ret;
+		u32 val;
+
+		ret = of_property_read_u32(child, "reg", &val);
+		if (ret < 0) {
+			dev_err(&pdev->dev, "missing 'reg' property (%d)\n",
+				ret);
+			continue;
+		}
+
+		if (val >= MAX_A38X_COMPHY || priv->lane[val].base) {
+			dev_err(&pdev->dev, "invalid 'reg' property\n");
+			continue;
+		}
+
+		phy = devm_phy_create(&pdev->dev, child, &a38x_comphy_ops);
+		if (IS_ERR(phy))
+			return PTR_ERR(phy);
+
+		priv->lane[val].base = base + 0x28 * val;
+		priv->lane[val].priv = priv;
+		priv->lane[val].n = val;
+		priv->lane[val].port = -1;
+		phy_set_drvdata(phy, &priv->lane[val]);
+	}
+
+	dev_set_drvdata(&pdev->dev, priv);
+
+	provider = devm_of_phy_provider_register(&pdev->dev, a38x_comphy_xlate);
+
+	return PTR_ERR_OR_ZERO(provider);
+}
+
+static const struct of_device_id a38x_comphy_of_match_table[] = {
+	{ .compatible = "marvell,armada-380-comphy" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, a38x_comphy_of_match_table);
+
+static struct platform_driver a38x_comphy_driver = {
+	.probe	= a38x_comphy_probe,
+	.driver	= {
+		.name = "armada-38x-comphy",
+		.of_match_table = a38x_comphy_of_match_table,
+	},
+};
+module_platform_driver(a38x_comphy_driver);
+
+MODULE_AUTHOR("Russell King <rmk+kernel@armlinux.org.uk>");
+MODULE_DESCRIPTION("Common PHY driver for Armada 38x SoCs");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

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

[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.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 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

diff --git a/drivers/phy/marvell/Kconfig b/drivers/phy/marvell/Kconfig
index 6fb4b56e4c14..224ea4e6a46d 100644
--- a/drivers/phy/marvell/Kconfig
+++ b/drivers/phy/marvell/Kconfig
@@ -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
diff --git a/drivers/phy/marvell/Makefile b/drivers/phy/marvell/Makefile
index 3975b144f8ec..59b6c03ef756 100644
--- a/drivers/phy/marvell/Makefile
+++ b/drivers/phy/marvell/Makefile
@@ -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
diff --git a/drivers/phy/marvell/phy-armada38x-comphy.c b/drivers/phy/marvell/phy-armada38x-comphy.c
new file mode 100644
index 000000000000..61d1965e1cf6
--- /dev/null
+++ b/drivers/phy/marvell/phy-armada38x-comphy.c
@@ -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
+ * <antoine.tenart@bootlin.com>
+ */
+#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;
+
+	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;
+	}
+
+	switch (mode) {
+	case PHY_MODE_SGMII:
+		gen = GEN_SGMII_1_25GBPS;
+		break;
+
+	case PHY_MODE_2500SGMII:
+		gen = GEN_SGMII_3_125GBPS;
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	a38x_comphy_set_speed(lane, gen, gen);
+
+	return a38x_comphy_poll(lane, COMPHY_STAT1,
+				COMPHY_STAT1_PLL_RDY_TX |
+				COMPHY_STAT1_PLL_RDY_RX,
+				COMPHY_STAT1_PLL_RDY_TX |
+				COMPHY_STAT1_PLL_RDY_RX);
+}
+
+static const struct phy_ops a38x_comphy_ops = {
+	.set_mode	= a38x_comphy_set_mode,
+	.owner		= THIS_MODULE,
+};
+
+static struct phy *a38x_comphy_xlate(struct device *dev,
+				     struct of_phandle_args *args)
+{
+	struct a38x_comphy_lane *lane;
+	struct phy *phy;
+
+	if (WARN_ON(args->args[0] >= MAX_A38X_PORTS))
+		return ERR_PTR(-EINVAL);
+
+	phy = of_phy_simple_xlate(dev, args);
+	if (IS_ERR(phy))
+		return phy;
+
+	lane = phy_get_drvdata(phy);
+	if (lane->port >= 0)
+		return ERR_PTR(-EBUSY);
+
+	lane->port = args->args[0];
+
+	return phy;
+}
+
+static int a38x_comphy_probe(struct platform_device *pdev)
+{
+	struct a38x_comphy *priv;
+	struct phy_provider *provider;
+	struct device_node *child;
+	struct resource *res;
+	void __iomem *base;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	priv->dev = &pdev->dev;
+	priv->base = base;
+
+	for_each_available_child_of_node(pdev->dev.of_node, child) {
+		struct phy *phy;
+		int ret;
+		u32 val;
+
+		ret = of_property_read_u32(child, "reg", &val);
+		if (ret < 0) {
+			dev_err(&pdev->dev, "missing 'reg' property (%d)\n",
+				ret);
+			continue;
+		}
+
+		if (val >= MAX_A38X_COMPHY || priv->lane[val].base) {
+			dev_err(&pdev->dev, "invalid 'reg' property\n");
+			continue;
+		}
+
+		phy = devm_phy_create(&pdev->dev, child, &a38x_comphy_ops);
+		if (IS_ERR(phy))
+			return PTR_ERR(phy);
+
+		priv->lane[val].base = base + 0x28 * val;
+		priv->lane[val].priv = priv;
+		priv->lane[val].n = val;
+		priv->lane[val].port = -1;
+		phy_set_drvdata(phy, &priv->lane[val]);
+	}
+
+	dev_set_drvdata(&pdev->dev, priv);
+
+	provider = devm_of_phy_provider_register(&pdev->dev, a38x_comphy_xlate);
+
+	return PTR_ERR_OR_ZERO(provider);
+}
+
+static const struct of_device_id a38x_comphy_of_match_table[] = {
+	{ .compatible = "marvell,armada-380-comphy" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, a38x_comphy_of_match_table);
+
+static struct platform_driver a38x_comphy_driver = {
+	.probe	= a38x_comphy_probe,
+	.driver	= {
+		.name = "armada-38x-comphy",
+		.of_match_table = a38x_comphy_of_match_table,
+	},
+};
+module_platform_driver(a38x_comphy_driver);
+
+MODULE_AUTHOR("Russell King <rmk+kernel@armlinux.org.uk>");
+MODULE_DESCRIPTION("Common PHY driver for Armada 38x SoCs");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

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

[Andrew Lunn]

> +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);
> +	}

Hi Russell

Maybe use one of the readx_poll_timeout() variants?

      Andrew

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

[Andrew Lunn]

> +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);
> +	}

Hi Russell

Maybe use one of the readx_poll_timeout() variants?

      Andrew

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

[Kishon Vijay Abraham I]

Hi,

On 12/11/18 6:00 PM, Russell King wrote:
> 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.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  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
> 
> diff --git a/drivers/phy/marvell/Kconfig b/drivers/phy/marvell/Kconfig
> index 6fb4b56e4c14..224ea4e6a46d 100644
> --- a/drivers/phy/marvell/Kconfig
> +++ b/drivers/phy/marvell/Kconfig
> @@ -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
> diff --git a/drivers/phy/marvell/Makefile b/drivers/phy/marvell/Makefile
> index 3975b144f8ec..59b6c03ef756 100644
> --- a/drivers/phy/marvell/Makefile
> +++ b/drivers/phy/marvell/Makefile
> @@ -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
> diff --git a/drivers/phy/marvell/phy-armada38x-comphy.c b/drivers/phy/marvell/phy-armada38x-comphy.c
> new file mode 100644
> index 000000000000..61d1965e1cf6
> --- /dev/null
> +++ b/drivers/phy/marvell/phy-armada38x-comphy.c
> @@ -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
> + * <antoine.tenart@bootlin.com>
> + */
> +#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;
> +
> +	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

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

[Kishon Vijay Abraham I]

Hi,

On 12/11/18 6:00 PM, Russell King wrote:
> 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.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  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
> 
> diff --git a/drivers/phy/marvell/Kconfig b/drivers/phy/marvell/Kconfig
> index 6fb4b56e4c14..224ea4e6a46d 100644
> --- a/drivers/phy/marvell/Kconfig
> +++ b/drivers/phy/marvell/Kconfig
> @@ -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
> diff --git a/drivers/phy/marvell/Makefile b/drivers/phy/marvell/Makefile
> index 3975b144f8ec..59b6c03ef756 100644
> --- a/drivers/phy/marvell/Makefile
> +++ b/drivers/phy/marvell/Makefile
> @@ -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
> diff --git a/drivers/phy/marvell/phy-armada38x-comphy.c b/drivers/phy/marvell/phy-armada38x-comphy.c
> new file mode 100644
> index 000000000000..61d1965e1cf6
> --- /dev/null
> +++ b/drivers/phy/marvell/phy-armada38x-comphy.c
> @@ -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
> + * <antoine.tenart@bootlin.com>
> + */
> +#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;
> +
> +	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

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

[Kishon Vijay Abraham I]

Hi,

On 12/11/18 6:00 PM, Russell King wrote:
> 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.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  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
> 
> diff --git a/drivers/phy/marvell/Kconfig b/drivers/phy/marvell/Kconfig
> index 6fb4b56e4c14..224ea4e6a46d 100644
> --- a/drivers/phy/marvell/Kconfig
> +++ b/drivers/phy/marvell/Kconfig
> @@ -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
> diff --git a/drivers/phy/marvell/Makefile b/drivers/phy/marvell/Makefile
> index 3975b144f8ec..59b6c03ef756 100644
> --- a/drivers/phy/marvell/Makefile
> +++ b/drivers/phy/marvell/Makefile
> @@ -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
> diff --git a/drivers/phy/marvell/phy-armada38x-comphy.c b/drivers/phy/marvell/phy-armada38x-comphy.c
> new file mode 100644
> index 000000000000..61d1965e1cf6
> --- /dev/null
> +++ b/drivers/phy/marvell/phy-armada38x-comphy.c
> @@ -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
> + * <antoine.tenart@bootlin.com>
> + */
> +#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;
> +
> +	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

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

[Maxime Chevallier]

Hi Russell,

On Mon, 12 Nov 2018 12:30:52 +0000
Russell King <rmk+kernel@armlinux.org.uk> wrote:

>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.

Tested-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

>Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

-- 
Maxime Chevallier, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

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

[Maxime Chevallier]

Hi Russell,

On Mon, 12 Nov 2018 12:30:52 +0000
Russell King <rmk+kernel@armlinux.org.uk> wrote:

>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.

Tested-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

>Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

-- 
Maxime Chevallier, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

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

[Russell King]

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

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 arch/arm/boot/dts/armada-38x.dtsi | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/arch/arm/boot/dts/armada-38x.dtsi b/arch/arm/boot/dts/armada-38x.dtsi
index 929459c42760..7b2e2bd6479b 100644
--- a/arch/arm/boot/dts/armada-38x.dtsi
+++ b/arch/arm/boot/dts/armada-38x.dtsi
@@ -335,6 +335,43 @@
 				#clock-cells = <1>;
 			};
 
+			comphy: phy@18300 {
+				compatible = "marvell,armada-380-comphy";
+				reg = <0x18300 0x100>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				comphy0: phy@0 {
+					reg = <0>;
+					#phy-cells = <1>;
+				};
+
+				comphy1: phy@1 {
+					reg = <1>;
+					#phy-cells = <1>;
+				};
+
+				comphy2: phy@2 {
+					reg = <2>;
+					#phy-cells = <1>;
+				};
+
+				comphy3: phy@3 {
+					reg = <3>;
+					#phy-cells = <1>;
+				};
+
+				comphy4: phy@4 {
+					reg = <4>;
+					#phy-cells = <1>;
+				};
+
+				comphy5: phy@5 {
+					reg = <5>;
+					#phy-cells = <1>;
+				};
+			};
+
 			coreclk: mvebu-sar@18600 {
 				compatible = "marvell,armada-380-core-clock";
 				reg = <0x18600 0x04>;
-- 
2.7.4

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

[Russell King]

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

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 arch/arm/boot/dts/armada-38x.dtsi | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/arch/arm/boot/dts/armada-38x.dtsi b/arch/arm/boot/dts/armada-38x.dtsi
index 929459c42760..7b2e2bd6479b 100644
--- a/arch/arm/boot/dts/armada-38x.dtsi
+++ b/arch/arm/boot/dts/armada-38x.dtsi
@@ -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>;
-- 
2.7.4

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

[Maxime Chevallier]

Hi Russell,

On Mon, 12 Nov 2018 12:30:57 +0000
Russell King <rmk+kernel@armlinux.org.uk> wrote:

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

Tested-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

>Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

-- 
Maxime Chevallier, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

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

[Maxime Chevallier]

Hi Russell,

On Mon, 12 Nov 2018 12:30:57 +0000
Russell King <rmk+kernel@armlinux.org.uk> wrote:

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

Tested-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

>Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

-- 
Maxime Chevallier, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

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

[Russell King]

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt b/Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt
index bedcfd5a52cd..691f886cfc4a 100644
--- a/Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt
+++ b/Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt
@@ -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):
 
-- 
2.7.4

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

[Russell King]

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt b/Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt
index bedcfd5a52cd..691f886cfc4a 100644
--- a/Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt
+++ b/Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt
@@ -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):
 
-- 
2.7.4

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

[Rob Herring]

On Mon, Nov 12, 2018 at 12:31:02PM +0000, Russell King wrote:
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

Needs a better subject and a commit msg.

> ---
>  Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt b/Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt
> index bedcfd5a52cd..691f886cfc4a 100644
> --- a/Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt
> +++ b/Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt
> @@ -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):
>  
> -- 
> 2.7.4
> 

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

[Rob Herring]

On Mon, Nov 12, 2018 at 12:31:02PM +0000, Russell King wrote:
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

Needs a better subject and a commit msg.

> ---
>  Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt b/Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt
> index bedcfd5a52cd..691f886cfc4a 100644
> --- a/Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt
> +++ b/Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt
> @@ -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):
>  
> -- 
> 2.7.4
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Russell King - ARM Linux]

On Mon, Dec 03, 2018 at 05:54:55PM -0600, Rob Herring wrote:
> On Mon, Nov 12, 2018 at 12:31:02PM +0000, Russell King wrote:
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> 
> 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.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>"

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

> 
> > ---
> >  Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt b/Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt
> > index bedcfd5a52cd..691f886cfc4a 100644
> > --- a/Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt
> > +++ b/Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt
> > @@ -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):
> >  
> > -- 
> > 2.7.4
> > 

-- 
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

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

[Russell King - ARM Linux]

On Mon, Dec 03, 2018 at 05:54:55PM -0600, Rob Herring wrote:
> On Mon, Nov 12, 2018 at 12:31:02PM +0000, Russell King wrote:
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> 
> 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.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>"

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

> 
> > ---
> >  Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt b/Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt
> > index bedcfd5a52cd..691f886cfc4a 100644
> > --- a/Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt
> > +++ b/Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt
> > @@ -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):
> >  
> > -- 
> > 2.7.4
> > 

-- 
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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Rob Herring]

On Wed, Dec 5, 2018 at 5:00 AM Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
>
> On Mon, Dec 03, 2018 at 05:54:55PM -0600, Rob Herring wrote:
> > On Mon, Nov 12, 2018 at 12:31:02PM +0000, Russell King wrote:
> > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> >
> > 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.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>"
>
> as the commit message.  With the correct commit message, are you happy
> with it?

Yes.

Reviewed-by: Rob Herring <robh@kernel.org>

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

[Rob Herring]

On Wed, Dec 5, 2018 at 5:00 AM Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
>
> On Mon, Dec 03, 2018 at 05:54:55PM -0600, Rob Herring wrote:
> > On Mon, Nov 12, 2018 at 12:31:02PM +0000, Russell King wrote:
> > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> >
> > 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.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>"
>
> as the commit message.  With the correct commit message, are you happy
> with it?

Yes.

Reviewed-by: Rob Herring <robh@kernel.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Kishon Vijay Abraham I]

Hi Russell,

On 05/12/18 9:00 PM, Rob Herring wrote:
> On Wed, Dec 5, 2018 at 5:00 AM Russell King - ARM Linux
> <linux@armlinux.org.uk> wrote:
>>
>> On Mon, Dec 03, 2018 at 05:54:55PM -0600, Rob Herring wrote:
>>> On Mon, Nov 12, 2018 at 12:31:02PM +0000, Russell King wrote:
>>>> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
>>>
>>> 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.
>>
>> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>"
>>
>> as the commit message.  With the correct commit message, are you happy
>> with it?
> 
> Yes.
> 
> Reviewed-by: Rob Herring <robh@kernel.org>

Are you planning to resend this series?

Thanks
Kishon

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

[Kishon Vijay Abraham I]

Hi Russell,

On 05/12/18 9:00 PM, Rob Herring wrote:
> On Wed, Dec 5, 2018 at 5:00 AM Russell King - ARM Linux
> <linux@armlinux.org.uk> wrote:
>>
>> On Mon, Dec 03, 2018 at 05:54:55PM -0600, Rob Herring wrote:
>>> On Mon, Nov 12, 2018 at 12:31:02PM +0000, Russell King wrote:
>>>> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
>>>
>>> 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.
>>
>> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>"
>>
>> as the commit message.  With the correct commit message, are you happy
>> with it?
> 
> Yes.
> 
> Reviewed-by: Rob Herring <robh@kernel.org>

Are you planning to resend this series?

Thanks
Kishon

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Russell King - ARM Linux]

On Fri, Dec 07, 2018 at 09:37:54AM +0530, Kishon Vijay Abraham I wrote:
> Hi Russell,
> 
> On 05/12/18 9:00 PM, Rob Herring wrote:
> > On Wed, Dec 5, 2018 at 5:00 AM Russell King - ARM Linux
> > <linux@armlinux.org.uk> wrote:
> >>
> >> On Mon, Dec 03, 2018 at 05:54:55PM -0600, Rob Herring wrote:
> >>> On Mon, Nov 12, 2018 at 12:31:02PM +0000, Russell King wrote:
> >>>> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> >>>
> >>> 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.
> >>
> >> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>"
> >>
> >> as the commit message.  With the correct commit message, are you happy
> >> with it?
> > 
> > Yes.
> > 
> > Reviewed-by: Rob Herring <robh@kernel.org>
> 
> 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.

-- 
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

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

[Russell King - ARM Linux]

On Fri, Dec 07, 2018 at 09:37:54AM +0530, Kishon Vijay Abraham I wrote:
> Hi Russell,
> 
> On 05/12/18 9:00 PM, Rob Herring wrote:
> > On Wed, Dec 5, 2018 at 5:00 AM Russell King - ARM Linux
> > <linux@armlinux.org.uk> wrote:
> >>
> >> On Mon, Dec 03, 2018 at 05:54:55PM -0600, Rob Herring wrote:
> >>> On Mon, Nov 12, 2018 at 12:31:02PM +0000, Russell King wrote:
> >>>> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> >>>
> >>> 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.
> >>
> >> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>"
> >>
> >> as the commit message.  With the correct commit message, are you happy
> >> with it?
> > 
> > Yes.
> > 
> > Reviewed-by: Rob Herring <robh@kernel.org>
> 
> 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.

-- 
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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Kishon Vijay Abraham I]

Russell,

On 07/12/18 4:11 PM, Russell King - ARM Linux wrote:
> On Fri, Dec 07, 2018 at 09:37:54AM +0530, Kishon Vijay Abraham I wrote:
>> Hi Russell,
>>
>> On 05/12/18 9:00 PM, Rob Herring wrote:
>>> On Wed, Dec 5, 2018 at 5:00 AM Russell King - ARM Linux
>>> <linux@armlinux.org.uk> wrote:
>>>>
>>>> On Mon, Dec 03, 2018 at 05:54:55PM -0600, Rob Herring wrote:
>>>>> On Mon, Nov 12, 2018 at 12:31:02PM +0000, Russell King wrote:
>>>>>> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
>>>>>
>>>>> 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.
>>>>
>>>> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>"
>>>>
>>>> as the commit message.  With the correct commit message, are you happy
>>>> with it?
>>>
>>> Yes.
>>>
>>> Reviewed-by: Rob Herring <robh@kernel.org>
>>
>> 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

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

[Kishon Vijay Abraham I]

Russell,

On 07/12/18 4:11 PM, Russell King - ARM Linux wrote:
> On Fri, Dec 07, 2018 at 09:37:54AM +0530, Kishon Vijay Abraham I wrote:
>> Hi Russell,
>>
>> On 05/12/18 9:00 PM, Rob Herring wrote:
>>> On Wed, Dec 5, 2018 at 5:00 AM Russell King - ARM Linux
>>> <linux@armlinux.org.uk> wrote:
>>>>
>>>> On Mon, Dec 03, 2018 at 05:54:55PM -0600, Rob Herring wrote:
>>>>> On Mon, Nov 12, 2018 at 12:31:02PM +0000, Russell King wrote:
>>>>>> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
>>>>>
>>>>> 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.
>>>>
>>>> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>"
>>>>
>>>> as the commit message.  With the correct commit message, are you happy
>>>> with it?
>>>
>>> Yes.
>>>
>>> Reviewed-by: Rob Herring <robh@kernel.org>
>>
>> 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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Russell King - ARM Linux]

On Fri, Dec 07, 2018 at 04:43:27PM +0530, Kishon Vijay Abraham I wrote:
> 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.

-- 
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

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

[Russell King - ARM Linux]

On Fri, Dec 07, 2018 at 04:43:27PM +0530, Kishon Vijay Abraham I wrote:
> 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.

-- 
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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Kishon Vijay Abraham I]

Hi,

On 07/12/18 5:03 PM, Russell King - ARM Linux wrote:
> On Fri, Dec 07, 2018 at 04:43:27PM +0530, Kishon Vijay Abraham I wrote:
>> 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.

I'd prefer we test it before merging.
> 
> 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
> 

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

[Kishon Vijay Abraham I]

Hi,

On 07/12/18 5:03 PM, Russell King - ARM Linux wrote:
> On Fri, Dec 07, 2018 at 04:43:27PM +0530, Kishon Vijay Abraham I wrote:
>> 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.

I'd prefer we test it before merging.
> 
> 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
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Russell King - ARM Linux]

On Fri, Dec 07, 2018 at 05:30:52PM +0530, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On 07/12/18 5:03 PM, Russell King - ARM Linux wrote:
> > On Fri, Dec 07, 2018 at 04:43:27PM +0530, Kishon Vijay Abraham I wrote:
> >> 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.
> 
> 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.

-- 
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

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

[Russell King - ARM Linux]

On Fri, Dec 07, 2018 at 05:30:52PM +0530, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On 07/12/18 5:03 PM, Russell King - ARM Linux wrote:
> > On Fri, Dec 07, 2018 at 04:43:27PM +0530, Kishon Vijay Abraham I wrote:
> >> 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.
> 
> 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.

-- 
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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Russell King]

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/marvell/mvneta.c | 58 ++++++++++++++++++++++++++++++-----
 1 file changed, 51 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index 5bfd349bf41a..7305d4cc0630 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -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;
 
-- 
2.7.4

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

[Russell King]

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/marvell/mvneta.c | 58 ++++++++++++++++++++++++++++++-----
 1 file changed, 51 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index 5bfd349bf41a..7305d4cc0630 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -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;
 
-- 
2.7.4

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

[Kishon Vijay Abraham I]

Hi,

On 12/11/18 6:01 PM, Russell King wrote:
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  drivers/net/ethernet/marvell/mvneta.c | 58 ++++++++++++++++++++++++++++++-----
>  1 file changed, 51 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> index 5bfd349bf41a..7305d4cc0630 100644
> --- a/drivers/net/ethernet/marvell/mvneta.c
> +++ b/drivers/net/ethernet/marvell/mvneta.c
> @@ -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;
> +	}

devm_phy_optional_get can be used here instead.

Thanks
Kishon

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

[Kishon Vijay Abraham I]

Hi,

On 12/11/18 6:01 PM, Russell King wrote:
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  drivers/net/ethernet/marvell/mvneta.c | 58 ++++++++++++++++++++++++++++++-----
>  1 file changed, 51 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> index 5bfd349bf41a..7305d4cc0630 100644
> --- a/drivers/net/ethernet/marvell/mvneta.c
> +++ b/drivers/net/ethernet/marvell/mvneta.c
> @@ -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;
> +	}

devm_phy_optional_get can be used here instead.

Thanks
Kishon

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

[Kishon Vijay Abraham I]

Hi,

On 12/11/18 6:01 PM, Russell King wrote:
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  drivers/net/ethernet/marvell/mvneta.c | 58 ++++++++++++++++++++++++++++++-----
>  1 file changed, 51 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> index 5bfd349bf41a..7305d4cc0630 100644
> --- a/drivers/net/ethernet/marvell/mvneta.c
> +++ b/drivers/net/ethernet/marvell/mvneta.c
> @@ -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;
> +	}

devm_phy_optional_get can be used here instead.

Thanks
Kishon

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

[Russell King - ARM Linux]

On Wed, Nov 14, 2018 at 02:18:14PM +0530, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On 12/11/18 6:01 PM, Russell King wrote:
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > ---
> >  drivers/net/ethernet/marvell/mvneta.c | 58 ++++++++++++++++++++++++++++++-----
> >  1 file changed, 51 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> > index 5bfd349bf41a..7305d4cc0630 100644
> > --- a/drivers/net/ethernet/marvell/mvneta.c
> > +++ b/drivers/net/ethernet/marvell/mvneta.c
> > @@ -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;
> > +	}
> 
> 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.

-- 
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

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

[Russell King - ARM Linux]

On Wed, Nov 14, 2018 at 02:18:14PM +0530, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On 12/11/18 6:01 PM, Russell King wrote:
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > ---
> >  drivers/net/ethernet/marvell/mvneta.c | 58 ++++++++++++++++++++++++++++++-----
> >  1 file changed, 51 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> > index 5bfd349bf41a..7305d4cc0630 100644
> > --- a/drivers/net/ethernet/marvell/mvneta.c
> > +++ b/drivers/net/ethernet/marvell/mvneta.c
> > @@ -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;
> > +	}
> 
> 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.

-- 
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

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

[Kishon Vijay Abraham I]

Hi,

On 14/11/18 4:41 PM, Russell King - ARM Linux wrote:
> On Wed, Nov 14, 2018 at 02:18:14PM +0530, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On 12/11/18 6:01 PM, Russell King wrote:
>>> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
>>> ---
>>>  drivers/net/ethernet/marvell/mvneta.c | 58 ++++++++++++++++++++++++++++++-----
>>>  1 file changed, 51 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
>>> index 5bfd349bf41a..7305d4cc0630 100644
>>> --- a/drivers/net/ethernet/marvell/mvneta.c
>>> +++ b/drivers/net/ethernet/marvell/mvneta.c
>>> @@ -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;
>>> +	}
>>
>> devm_phy_optional_get can be used here instead.
> 
> I don't think that will work with a NULL string.

That's correct.
> 
> 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()
> 
> 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.

Agreed. I'll fix this so that we have similar behavior with of_phy_get() and
phy_get().
> 
> 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


> 

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

[Kishon Vijay Abraham I]

Hi,

On 14/11/18 4:41 PM, Russell King - ARM Linux wrote:
> On Wed, Nov 14, 2018 at 02:18:14PM +0530, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On 12/11/18 6:01 PM, Russell King wrote:
>>> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
>>> ---
>>>  drivers/net/ethernet/marvell/mvneta.c | 58 ++++++++++++++++++++++++++++++-----
>>>  1 file changed, 51 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
>>> index 5bfd349bf41a..7305d4cc0630 100644
>>> --- a/drivers/net/ethernet/marvell/mvneta.c
>>> +++ b/drivers/net/ethernet/marvell/mvneta.c
>>> @@ -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;
>>> +	}
>>
>> devm_phy_optional_get can be used here instead.
> 
> I don't think that will work with a NULL string.

That's correct.
> 
> 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()
> 
> 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.

Agreed. I'll fix this so that we have similar behavior with of_phy_get() and
phy_get().
> 
> 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


> 

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

[Maxime Chevallier]

Hi Russell,

On Mon, 12 Nov 2018 12:31:07 +0000
Russell King <rmk+kernel@armlinux.org.uk> wrote:

Maybe missing a commit log ?

Otherwise,

Tested-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

>Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>


-- 
Maxime Chevallier, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

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

[Maxime Chevallier]

Hi Russell,

On Mon, 12 Nov 2018 12:31:07 +0000
Russell King <rmk+kernel@armlinux.org.uk> wrote:

Maybe missing a commit log ?

Otherwise,

Tested-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

>Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>


-- 
Maxime Chevallier, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

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

[Russell King]

Add the comphy settings for the Ethernet interfaces.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 arch/arm/boot/dts/armada-388-clearfog.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/armada-388-clearfog.dtsi b/arch/arm/boot/dts/armada-388-clearfog.dtsi
index 1b0d0680c8b6..0d81600ca247 100644
--- a/arch/arm/boot/dts/armada-388-clearfog.dtsi
+++ b/arch/arm/boot/dts/armada-388-clearfog.dtsi
@@ -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";
-- 
2.7.4

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

[Russell King]

Add the comphy settings for the Ethernet interfaces.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 arch/arm/boot/dts/armada-388-clearfog.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/armada-388-clearfog.dtsi b/arch/arm/boot/dts/armada-388-clearfog.dtsi
index 1b0d0680c8b6..0d81600ca247 100644
--- a/arch/arm/boot/dts/armada-388-clearfog.dtsi
+++ b/arch/arm/boot/dts/armada-388-clearfog.dtsi
@@ -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";
-- 
2.7.4

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

[Maxime Chevallier]

Hi Russell,

On Mon, 12 Nov 2018 12:31:12 +0000
Russell King <rmk+kernel@armlinux.org.uk> wrote:

>Add the comphy settings for the Ethernet interfaces.

Tested-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

>Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

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

[Maxime Chevallier]

Hi Russell,

On Mon, 12 Nov 2018 12:31:12 +0000
Russell King <rmk+kernel@armlinux.org.uk> wrote:

>Add the comphy settings for the Ethernet interfaces.

Tested-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

>Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

Re: [RFC PATCH 0/6] Armada 38x comphy driver to support 2.5Gbps networking

[Kishon Vijay Abraham I]

Hi,

On 12/11/18 5:59 PM, Russell King - ARM Linux wrote:
> 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.

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

[RFC PATCH 0/6] Armada 38x comphy driver to support 2.5Gbps networking

[Kishon Vijay Abraham I]

Hi,

On 12/11/18 5:59 PM, Russell King - ARM Linux wrote:
> 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.

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

Re: [RFC PATCH 0/6] Armada 38x comphy driver to support 2.5Gbps networking

[Kishon Vijay Abraham I]

Hi,

On 12/11/18 5:59 PM, Russell King - ARM Linux wrote:
> 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.

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

Re: [RFC PATCH 0/6] Armada 38x comphy driver to support 2.5Gbps networking

[Russell King - ARM Linux]

On Wed, Nov 14, 2018 at 01:39:29PM +0530, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On 12/11/18 5:59 PM, Russell King - ARM Linux wrote:
> > 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.
> 
> 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.

-- 
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

[RFC PATCH 0/6] Armada 38x comphy driver to support 2.5Gbps networking

[Russell King - ARM Linux]

On Wed, Nov 14, 2018 at 01:39:29PM +0530, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On 12/11/18 5:59 PM, Russell King - ARM Linux wrote:
> > 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.
> 
> 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.

-- 
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

Re: [RFC PATCH 0/6] Armada 38x comphy driver to support 2.5Gbps networking

[Kishon Vijay Abraham I]

Hi Russell,

On 14/11/18 4:26 PM, Russell King - ARM Linux wrote:
> On Wed, Nov 14, 2018 at 01:39:29PM +0530, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On 12/11/18 5:59 PM, Russell King - ARM Linux wrote:
>>> 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.
>>
>> 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
> 

[RFC PATCH 0/6] Armada 38x comphy driver to support 2.5Gbps networking

[Kishon Vijay Abraham I]

Hi Russell,

On 14/11/18 4:26 PM, Russell King - ARM Linux wrote:
> On Wed, Nov 14, 2018 at 01:39:29PM +0530, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On 12/11/18 5:59 PM, Russell King - ARM Linux wrote:
>>> 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.
>>
>> 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
> 

Re: [RFC PATCH 0/6] Armada 38x comphy driver to support 2.5Gbps networking

[Maxime Chevallier]

Hello Russell,

On Mon, 12 Nov 2018 12:29:33 +0000
Russell King - ARM Linux <linux@armlinux.org.uk> wrote:

>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

[RFC PATCH 0/6] Armada 38x comphy driver to support 2.5Gbps networking

[Maxime Chevallier]

Hello Russell,

On Mon, 12 Nov 2018 12:29:33 +0000
Russell King - ARM Linux <linux@armlinux.org.uk> wrote:

>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