[PATCH net-next 00/13] net: mvpp2: comphy configuration

[PATCH net-next 00/13] net: mvpp2: comphy configuration

[Antoine Tenart]

Hi all,

This series, following up the one one the GoP/MAC configuration, aims at
stopping to depend on the firmware/bootloader configuration when using
the PPv2 engine. With this series the PPv2 driver does not need to rely
on a previous configuration, and dynamic reconfiguration while the
kernel is running can be done (i.e. switch one port from SGMII to 10GKR,
or the opposite). A port can now be configured in a different mode than
what's done in the firmware/bootloader as well.

The series first contain patches in the generic PHY framework to support
what is called the comphy (common PHYs), which is an h/w block providing
PHYs that can be configured in various modes ranging from SGMII, 10GKR
to SATA and others. As of now only the SGMII and 10GKR modes are
supported by the comphy driver.

Then patches are modifying the PPv2 driver to first add the comphy
initialization sequence (i.e. calls to the generic PHY framework) and to
then take advantage of this to allow dynamic reconfiguration (i.e.
configuring the mode of a port given what's connected, between sgmii and
10G). Note the use of the comphy in the PPv2 driver is kept optional
(i.e. if not described in dt the driver still as before an relies on the
firmware/bootloader configuration).

Finally there are dt/defconfig patches to describe and take advantage of
this.

This was tested on a range of devices: 8040-db, 8040-mcbin and 7040-db.

Thanks!
Antoine

Antoine Tenart (12):
  phy: add sgmii and 10gkr modes to the phy_mode enum
  phy: add the mvebu cp110 comphy driver
  Documentation/bindings: phy: document the Marvell comphy driver
  net: mvpp2: initialize the comphy
  net: mvpp2: do not force the link mode
  net: mvpp2: simplify the link_event function
  net: mvpp2: improve the link management function
  net: mvpp2: check the netif is running in the link_event function
  net: mvpp2: dynamic reconfiguration of the PHY mode
  arm64: dts: marvell: extend the cp110 syscon register area length
  arm64: dts: marvell: add comphy nodes on cp110 master and slave
  arm64: dts: marvell: mcbin: add comphy references to Ethernet ports

Miquel Raynal (1):
  arm64: defconfig: enable Marvell CP110 comphy

 .../devicetree/bindings/phy/phy-mvebu-comphy.txt   |  42 ++
 arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts  |   3 +
 .../boot/dts/marvell/armada-cp110-master.dtsi      |  40 +-
 .../arm64/boot/dts/marvell/armada-cp110-slave.dtsi |  40 +-
 arch/arm64/configs/defconfig                       |   1 +
 drivers/net/ethernet/marvell/mvpp2.c               | 111 ++--
 drivers/phy/marvell/Kconfig                        |  10 +
 drivers/phy/marvell/Makefile                       |   1 +
 drivers/phy/marvell/phy-mvebu-cp110-comphy.c       | 656 +++++++++++++++++++++
 include/linux/phy/phy.h                            |   2 +
 10 files changed, 871 insertions(+), 35 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/phy/phy-mvebu-comphy.txt
 create mode 100644 drivers/phy/marvell/phy-mvebu-cp110-comphy.c

-- 
2.13.5

[PATCH net-next 01/13] phy: add sgmii and 10gkr modes to the phy_mode enum

[Antoine Tenart]

This patch adds more PHY modes to the phy_mode enum, to allow
configuring PHYs to the SGMII and/or the 10GKR mode by using the
set_mode callback.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 include/linux/phy/phy.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 78bb0d7f6b11..e694d4008c4a 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -27,6 +27,8 @@ enum phy_mode {
 	PHY_MODE_USB_HOST,
 	PHY_MODE_USB_DEVICE,
 	PHY_MODE_USB_OTG,
+	PHY_MODE_SGMII,
+	PHY_MODE_10GKR,
 };
 
 /**
-- 
2.13.5

Re: [PATCH net-next 01/13] phy: add sgmii and 10gkr modes to the phy_mode enum

[Andrew Lunn]

On Thu, Aug 24, 2017 at 10:38:11AM +0200, Antoine Tenart wrote:
> This patch adds more PHY modes to the phy_mode enum, to allow
> configuring PHYs to the SGMII and/or the 10GKR mode by using the
> set_mode callback.

Hi Antoine

You had me confused for a while here. If there is need for a respin,
could you change the commit log and prefix PHY with either generic or
Ethernet, just to make it clear which PHY subsystem we are talking
about.

	Andrew

Re: [PATCH net-next 01/13] phy: add sgmii and 10gkr modes to the phy_mode enum

[Antoine Tenart]

[-- Attachment #1: Type: text/plain, Size: 741 bytes --]

Hi Andrew,

On Thu, Aug 24, 2017 at 03:19:38PM +0200, Andrew Lunn wrote:
> On Thu, Aug 24, 2017 at 10:38:11AM +0200, Antoine Tenart wrote:
> > This patch adds more PHY modes to the phy_mode enum, to allow
> > configuring PHYs to the SGMII and/or the 10GKR mode by using the
> > set_mode callback.
> 
> You had me confused for a while here. If there is need for a respin,
> could you change the commit log and prefix PHY with either generic or
> Ethernet, just to make it clear which PHY subsystem we are talking
> about.

I understand the confusion :) I'll update the commit log if there is a
respin.

Thanks!
Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH net-next 02/13] phy: add the mvebu cp110 comphy driver

[Antoine Tenart]

On the CP110 unit, which can be found on various Marvell platforms such
as the 7k and 8k (currently), a comphy (common PHYs) hardware block can
be found. This block provides a number of PHYs which can be used in
various modes by other controllers (network, SATA ...). These common
PHYs must be configured for the controllers using them to work correctly
either at boot time, or when the system runs to switch the mode used.
This patch adds a driver for this comphy hardware block, providing
callbacks for the its PHYs so that consumers can configure the modes
used.

As of this commit, two modes are supported by the comphy driver: sgmii
and 10gkr.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 drivers/phy/marvell/Kconfig                  |  10 +
 drivers/phy/marvell/Makefile                 |   1 +
 drivers/phy/marvell/phy-mvebu-cp110-comphy.c | 656 +++++++++++++++++++++++++++
 3 files changed, 667 insertions(+)
 create mode 100644 drivers/phy/marvell/phy-mvebu-cp110-comphy.c

diff --git a/drivers/phy/marvell/Kconfig b/drivers/phy/marvell/Kconfig
index 048d8893bc2e..26755f3d1a9a 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_CP110_COMPHY
+	tristate "Marvell CP110 comphy driver"
+	depends on ARCH_MVEBU && OF
+	select GENERIC_PHY
+	help
+	  This driver allows to control the comphy, an hardware block providing
+	  shared serdes PHYs on Marvell Armada 7k/8k (in the CP110). Its serdes
+	  lanes can be used by various controllers (Ethernet, sata, usb,
+	  PCIe...).
+
 config PHY_MVEBU_SATA
 	def_bool y
 	depends on ARCH_DOVE || MACH_DOVE || MACH_KIRKWOOD
diff --git a/drivers/phy/marvell/Makefile b/drivers/phy/marvell/Makefile
index 3fc188f59118..0cf6a7cbaf9f 100644
--- a/drivers/phy/marvell/Makefile
+++ b/drivers/phy/marvell/Makefile
@@ -1,6 +1,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_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
 obj-$(CONFIG_PHY_PXA_28NM_USB2)		+= phy-pxa-28nm-usb2.o
diff --git a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
new file mode 100644
index 000000000000..4e8dfa82a479
--- /dev/null
+++ b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
@@ -0,0 +1,656 @@
+/*
+ * Copyright (C) 2017 Marvell
+ *
+ * Antoine Tenart <antoine.tenart@free-electrons.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+/* Relative to priv->base */
+#define MVEBU_COMPHY_SERDES_CFG0(n)		(0x0 + (n) * 0x1000)
+#define     MVEBU_COMPHY_SERDES_CFG0_PU_PLL	BIT(1)
+#define     MVEBU_COMPHY_SERDES_CFG0_GEN_RX(n)	((n) << 3)
+#define     MVEBU_COMPHY_SERDES_CFG0_GEN_TX(n)	((n) << 7)
+#define     MVEBU_COMPHY_SERDES_CFG0_PU_RX	BIT(11)
+#define     MVEBU_COMPHY_SERDES_CFG0_PU_TX	BIT(12)
+#define     MVEBU_COMPHY_SERDES_CFG0_HALF_BUS	BIT(14)
+#define MVEBU_COMPHY_SERDES_CFG1(n)		(0x4 + (n) * 0x1000)
+#define     MVEBU_COMPHY_SERDES_CFG1_RESET	BIT(3)
+#define     MVEBU_COMPHY_SERDES_CFG1_RX_INIT	BIT(4)
+#define     MVEBU_COMPHY_SERDES_CFG1_CORE_RESET	BIT(5)
+#define     MVEBU_COMPHY_SERDES_CFG1_RF_RESET	BIT(6)
+#define MVEBU_COMPHY_SERDES_CFG2(n)		(0x8 + (n) * 0x1000)
+#define     MVEBU_COMPHY_SERDES_CFG2_DFE_EN	BIT(4)
+#define MVEBU_COMPHY_SERDES_STATUS0(n)		(0x18 + (n) * 0x1000)
+#define     MVEBU_COMPHY_SERDES_STATUS0_TX_PLL_RDY	BIT(2)
+#define     MVEBU_COMPHY_SERDES_STATUS0_RX_PLL_RDY	BIT(3)
+#define     MVEBU_COMPHY_SERDES_STATUS0_RX_INIT		BIT(4)
+#define MVEBU_COMPHY_PWRPLL_CTRL(n)		(0x804 + (n) * 0x1000)
+#define     MVEBU_COMPHY_PWRPLL_CTRL_RFREQ(n)	((n) << 0)
+#define     MVEBU_COMPHY_PWRPLL_PHY_MODE(n)	((n) << 5)
+#define MVEBU_COMPHY_IMP_CAL(n)			(0x80c + (n) * 0x1000)
+#define     MVEBU_COMPHY_IMP_CAL_TX_EXT(n)	((n) << 10)
+#define     MVEBU_COMPHY_IMP_CAL_TX_EXT_EN	BIT(15)
+#define MVEBU_COMPHY_DFE_RES(n)			(0x81c + (n) * 0x1000)
+#define     MVEBU_COMPHY_DFE_RES_FORCE_GEN_TBL	BIT(15)
+#define MVEBU_COMPHY_COEF(n)			(0x828 + (n) * 0x1000)
+#define     MVEBU_COMPHY_COEF_DFE_EN		BIT(14)
+#define     MVEBU_COMPHY_COEF_DFE_CTRL		BIT(15)
+#define MVEBU_COMPHY_GEN1_S0(n)			(0x834 + (n) * 0x1000)
+#define     MVEBU_COMPHY_GEN1_S0_TX_AMP(n)	((n) << 1)
+#define     MVEBU_COMPHY_GEN1_S0_TX_EMPH(n)	((n) << 7)
+#define MVEBU_COMPHY_GEN1_S1(n)			(0x838 + (n) * 0x1000)
+#define     MVEBU_COMPHY_GEN1_S1_RX_MUL_PI(n)	((n) << 0)
+#define     MVEBU_COMPHY_GEN1_S1_RX_MUL_PF(n)	((n) << 3)
+#define     MVEBU_COMPHY_GEN1_S1_RX_MUL_FI(n)	((n) << 6)
+#define     MVEBU_COMPHY_GEN1_S1_RX_MUL_FF(n)	((n) << 8)
+#define     MVEBU_COMPHY_GEN1_S1_RX_DFE_EN	BIT(10)
+#define     MVEBU_COMPHY_GEN1_S1_RX_DIV(n)	((n) << 11)
+#define MVEBU_COMPHY_GEN1_S2(n)			(0x8f4 + (n) * 0x1000)
+#define     MVEBU_COMPHY_GEN1_S2_TX_EMPH(n)	((n) << 0)
+#define     MVEBU_COMPHY_GEN1_S2_TX_EMPH_EN	BIT(4)
+#define MVEBU_COMPHY_LOOPBACK(n)		(0x88c + (n) * 0x1000)
+#define     MVEBU_COMPHY_LOOPBACK_DBUS_WIDTH(n)	((n) << 1)
+#define MVEBU_COMPHY_VDD_CAL0(n)		(0x908 + (n) * 0x1000)
+#define     MVEBU_COMPHY_VDD_CAL0_CONT_MODE	BIT(15)
+#define MVEBU_COMPHY_EXT_SELV(n)		(0x914 + (n) * 0x1000)
+#define     MVEBU_COMPHY_EXT_SELV_RX_SAMPL(n)	((n) << 5)
+#define MVEBU_COMPHY_MISC_CTRL0(n)		(0x93c + (n) * 0x1000)
+#define     MVEBU_COMPHY_MISC_CTRL0_ICP_FORCE	BIT(5)
+#define     MVEBU_COMPHY_MISC_CTRL0_REFCLK_SEL	BIT(10)
+#define MVEBU_COMPHY_RX_CTRL1(n)		(0x940 + (n) * 0x1000)
+#define     MVEBU_COMPHY_RX_CTRL1_RXCLK2X_SEL	BIT(11)
+#define     MVEBU_COMPHY_RX_CTRL1_CLK8T_EN	BIT(12)
+#define MVEBU_COMPHY_SPEED_DIV(n)		(0x954 + (n) * 0x1000)
+#define     MVEBU_COMPHY_SPEED_DIV_TX_FORCE	BIT(7)
+#define MVEBU_SP_CALIB(n)			(0x96c + (n) * 0x1000)
+#define     MVEBU_SP_CALIB_SAMPLER(n)		((n) << 8)
+#define     MVEBU_SP_CALIB_SAMPLER_EN		BIT(12)
+#define MVEBU_COMPHY_TX_SLEW_RATE(n)		(0x974 + (n) * 0x1000)
+#define     MVEBU_COMPHY_TX_SLEW_RATE_EMPH(n)	((n) << 5)
+#define     MVEBU_COMPHY_TX_SLEW_RATE_SLC(n)	((n) << 10)
+#define MVEBU_COMPHY_DLT_CTRL(n)		(0x984 + (n) * 0x1000)
+#define     MVEBU_COMPHY_DLT_CTRL_DTL_FLOOP_EN	BIT(2)
+#define MVEBU_COMPHY_FRAME_DETECT0(n)		(0xa14 + (n) * 0x1000)
+#define     MVEBU_COMPHY_FRAME_DETECT0_PATN(n)	((n) << 7)
+#define MVEBU_COMPHY_FRAME_DETECT3(n)		(0xa20 + (n) * 0x1000)
+#define     MVEBU_COMPHY_FRAME_DETECT3_LOST_TIMEOUT_EN	BIT(12)
+#define MVEBU_COMPHY_DME(n)			(0xa28 + (n) * 0x1000)
+#define     MVEBU_COMPHY_DME_ETH_MODE		BIT(7)
+#define MVEBU_COMPHY_TRAINING0(n)		(0xa68 + (n) * 0x1000)
+#define     MVEBU_COMPHY_TRAINING0_P2P_HOLD	BIT(15)
+#define MVEBU_COMPHY_TRAINING5(n)		(0xaa4 + (n) * 0x1000)
+#define	    MVEBU_COMPHY_TRAINING5_RX_TIMER(n)	((n) << 0)
+#define MVEBU_COMPHY_TX_TRAIN_PRESET(n)		(0xb1c + (n) * 0x1000)
+#define     MVEBU_COMPHY_TX_TRAIN_PRESET_16B_AUTO_EN	BIT(8)
+#define     MVEBU_COMPHY_TX_TRAIN_PRESET_PRBS11		BIT(9)
+#define MVEBU_COMPHY_GEN1_S3(n)			(0xc40 + (n) * 0x1000)
+#define     MVEBU_COMPHY_GEN1_S3_FBCK_SEL	BIT(9)
+#define MVEBU_COMPHY_GEN1_S4(n)			(0xc44 + (n) * 0x1000)
+#define	    MVEBU_COMPHY_GEN1_S4_DFE_RES(n)	((n) << 8)
+#define MVEBU_COMPHY_TX_PRESET(n)		(0xc68 + (n) * 0x1000)
+#define     MVEBU_COMPHY_TX_PRESET_INDEX(n)	((n) << 0)
+#define MVEBU_COMPHY_GEN1_S5(n)			(0xd38 + (n) * 0x1000)
+#define     MVEBU_COMPHY_GEN1_S5_ICP(n)		((n) << 0)
+
+/* Relative to priv->regmap */
+#define MVEBU_COMPHY_CONF1(n)			(0x1000 + (n) * 0x28)
+#define     MVEBU_COMPHY_CONF1_PWRUP		BIT(1)
+#define     MVEBU_COMPHY_CONF1_USB_PCIE		BIT(2)	/* 0: Ethernet/SATA */
+#define MVEBU_COMPHY_CONF6(n)			(0x1014 + (n) * 0x28)
+#define     MVEBU_COMPHY_CONF6_40B		BIT(18)
+#define MVEBU_COMPHY_SELECTOR			0x1140
+#define     MVEBU_COMPHY_SELECTOR_PHY(n)	((n) * 0x4)
+
+#define MVEBU_COMPHY_LANES	6
+#define MVEBU_COMPHY_PORTS	3
+
+struct mvebu_comhy_conf {
+	enum phy_mode mode;
+	unsigned lane;
+	unsigned port;
+	u32 mux;
+};
+
+#define MVEBU_COMPHY_CONF(_lane, _port, _mode, _mux)	\
+	{						\
+		.lane = _lane,				\
+		.port = _port,				\
+		.mode = _mode,				\
+		.mux = _mux,				\
+	}
+
+static const struct mvebu_comhy_conf mvebu_comphy_modes[] = {
+	/* lane 0 */
+	MVEBU_COMPHY_CONF(0, 1, PHY_MODE_SGMII, 0x1),
+	/* lane 1 */
+	MVEBU_COMPHY_CONF(1, 2, PHY_MODE_SGMII, 0x1),
+	/* lane 2 */
+	MVEBU_COMPHY_CONF(2, 0, PHY_MODE_SGMII, 0x1),
+	MVEBU_COMPHY_CONF(2, 0, PHY_MODE_10GKR, 0x1),
+	/* lane 3 */
+	MVEBU_COMPHY_CONF(3, 1, PHY_MODE_SGMII, 0x2),
+	/* lane 4 */
+	MVEBU_COMPHY_CONF(4, 0, PHY_MODE_SGMII, 0x2),
+	MVEBU_COMPHY_CONF(4, 0, PHY_MODE_10GKR, 0x2),
+	MVEBU_COMPHY_CONF(4, 1, PHY_MODE_SGMII, 0x1),
+	/* lane 5 */
+	MVEBU_COMPHY_CONF(5, 2, PHY_MODE_SGMII, 0x1),
+};
+
+struct mvebu_comphy_priv {
+	void __iomem *base;
+	struct regmap *regmap;
+	struct device *dev;
+	struct phy *phys[MVEBU_COMPHY_LANES];
+	int modes[MVEBU_COMPHY_LANES];
+};
+
+struct mvebu_comphy_lane {
+	struct mvebu_comphy_priv *priv;
+	struct device_node *of_node;
+	unsigned id;
+	enum phy_mode mode;
+	int port;
+};
+
+static int mvebu_comphy_get_mux(int lane, int port, enum phy_mode mode)
+{
+	int i, n = ARRAY_SIZE(mvebu_comphy_modes);
+
+	/* Unused PHY mux value is 0x0 */
+	if (mode == PHY_MODE_INVALID)
+		return 0;
+
+	for (i = 0; i < n; i++) {
+		if (mvebu_comphy_modes[i].lane == lane &&
+		    mvebu_comphy_modes[i].port == port &&
+		    mvebu_comphy_modes[i].mode == mode)
+			break;
+	}
+
+	if (i == n)
+		return -EINVAL;
+
+	return mvebu_comphy_modes[i].mux;
+}
+
+static void mvebu_comphy_ethernet_init_reset(struct mvebu_comphy_lane *lane,
+					     enum phy_mode mode)
+{
+	struct mvebu_comphy_priv *priv = lane->priv;
+	u32 val;
+
+	regmap_read(priv->regmap, MVEBU_COMPHY_CONF1(lane->id), &val);
+	val &= ~MVEBU_COMPHY_CONF1_USB_PCIE;
+	val |= MVEBU_COMPHY_CONF1_PWRUP;
+	regmap_write(priv->regmap, MVEBU_COMPHY_CONF1(lane->id), val);
+
+	/* Select baud rates and PLLs */
+	val = readl(priv->base + MVEBU_COMPHY_SERDES_CFG0(lane->id));
+	val &= ~(MVEBU_COMPHY_SERDES_CFG0_PU_PLL |
+		 MVEBU_COMPHY_SERDES_CFG0_PU_RX |
+		 MVEBU_COMPHY_SERDES_CFG0_PU_TX |
+		 MVEBU_COMPHY_SERDES_CFG0_HALF_BUS |
+		 MVEBU_COMPHY_SERDES_CFG0_GEN_RX(0xf) |
+		 MVEBU_COMPHY_SERDES_CFG0_GEN_TX(0xf));
+	if (mode == PHY_MODE_10GKR)
+		val |= MVEBU_COMPHY_SERDES_CFG0_GEN_RX(0xe) |
+		       MVEBU_COMPHY_SERDES_CFG0_GEN_TX(0xe);
+	else if (mode == PHY_MODE_SGMII)
+		val |= MVEBU_COMPHY_SERDES_CFG0_GEN_RX(0x6) |
+		       MVEBU_COMPHY_SERDES_CFG0_GEN_TX(0x6) |
+		       MVEBU_COMPHY_SERDES_CFG0_HALF_BUS;
+	writel(val, priv->base + MVEBU_COMPHY_SERDES_CFG0(lane->id));
+
+	/* reset */
+	val = readl(priv->base + MVEBU_COMPHY_SERDES_CFG1(lane->id));
+	val &= ~(MVEBU_COMPHY_SERDES_CFG1_RESET |
+		 MVEBU_COMPHY_SERDES_CFG1_CORE_RESET |
+		 MVEBU_COMPHY_SERDES_CFG1_RF_RESET);
+	writel(val, priv->base + MVEBU_COMPHY_SERDES_CFG1(lane->id));
+
+	/* de-assert reset */
+	val = readl(priv->base + MVEBU_COMPHY_SERDES_CFG1(lane->id));
+	val |= MVEBU_COMPHY_SERDES_CFG1_RESET |
+	       MVEBU_COMPHY_SERDES_CFG1_CORE_RESET;
+	writel(val, priv->base + MVEBU_COMPHY_SERDES_CFG1(lane->id));
+
+	/* wait until clocks are ready */
+	mdelay(1);
+
+	/* exlicitly disable 40B, the bits isn't clear on reset */
+	regmap_read(priv->regmap, MVEBU_COMPHY_CONF6(lane->id), &val);
+	val &= ~MVEBU_COMPHY_CONF6_40B;
+	regmap_write(priv->regmap, MVEBU_COMPHY_CONF6(lane->id), val);
+
+	/* refclk selection */
+	val = readl(priv->base + MVEBU_COMPHY_MISC_CTRL0(lane->id));
+	val &= ~MVEBU_COMPHY_MISC_CTRL0_REFCLK_SEL;
+	if (mode == PHY_MODE_10GKR)
+		val |= MVEBU_COMPHY_MISC_CTRL0_ICP_FORCE;
+	writel(val, priv->base + MVEBU_COMPHY_MISC_CTRL0(lane->id));
+
+	/* power and pll selection */
+	val = readl(priv->base + MVEBU_COMPHY_PWRPLL_CTRL(lane->id));
+	val &= ~(MVEBU_COMPHY_PWRPLL_CTRL_RFREQ(0x1f) |
+		 MVEBU_COMPHY_PWRPLL_PHY_MODE(0x7));
+	val |= MVEBU_COMPHY_PWRPLL_CTRL_RFREQ(0x1) |
+	       MVEBU_COMPHY_PWRPLL_PHY_MODE(0x4);
+	writel(val, priv->base + MVEBU_COMPHY_PWRPLL_CTRL(lane->id));
+
+	val = readl(priv->base + MVEBU_COMPHY_LOOPBACK(lane->id));
+	val &= ~MVEBU_COMPHY_LOOPBACK_DBUS_WIDTH(0x7);
+	val |= MVEBU_COMPHY_LOOPBACK_DBUS_WIDTH(0x1);
+	writel(val, priv->base + MVEBU_COMPHY_LOOPBACK(lane->id));
+}
+
+static int mvebu_comphy_init_plls(struct mvebu_comphy_lane *lane,
+				  enum phy_mode mode)
+{
+	struct mvebu_comphy_priv *priv = lane->priv;
+	u32 val;
+
+	/* SERDES external config */
+	val = readl(priv->base + MVEBU_COMPHY_SERDES_CFG0(lane->id));
+	val |= MVEBU_COMPHY_SERDES_CFG0_PU_PLL |
+	       MVEBU_COMPHY_SERDES_CFG0_PU_RX |
+	       MVEBU_COMPHY_SERDES_CFG0_PU_TX;
+	writel(val, priv->base + MVEBU_COMPHY_SERDES_CFG0(lane->id));
+
+	/* check rx/tx pll */
+	readl_poll_timeout(priv->base + MVEBU_COMPHY_SERDES_STATUS0(lane->id),
+			   val,
+			   val & (MVEBU_COMPHY_SERDES_STATUS0_RX_PLL_RDY |
+				  MVEBU_COMPHY_SERDES_STATUS0_TX_PLL_RDY),
+			   1000, 150000);
+	if (!(val & (MVEBU_COMPHY_SERDES_STATUS0_RX_PLL_RDY |
+		     MVEBU_COMPHY_SERDES_STATUS0_TX_PLL_RDY)))
+		return -ETIMEDOUT;
+
+	/* rx init */
+	val = readl(priv->base + MVEBU_COMPHY_SERDES_CFG1(lane->id));
+	val |= MVEBU_COMPHY_SERDES_CFG1_RX_INIT;
+	writel(val, priv->base + MVEBU_COMPHY_SERDES_CFG1(lane->id));
+
+	/* check rx */
+	readl_poll_timeout(priv->base + MVEBU_COMPHY_SERDES_STATUS0(lane->id),
+			   val, val & MVEBU_COMPHY_SERDES_STATUS0_RX_INIT,
+			   1000, 10000);
+	if (!(val & MVEBU_COMPHY_SERDES_STATUS0_RX_INIT))
+		return -ETIMEDOUT;
+
+	val = readl(priv->base + MVEBU_COMPHY_SERDES_CFG1(lane->id));
+	val &= ~MVEBU_COMPHY_SERDES_CFG1_RX_INIT;
+	writel(val, priv->base + MVEBU_COMPHY_SERDES_CFG1(lane->id));
+
+	return 0;
+}
+
+static int mvebu_comphy_set_mode_sgmii(struct phy *phy, enum phy_mode mode)
+{
+	struct mvebu_comphy_lane *lane = phy_get_drvdata(phy);
+	struct mvebu_comphy_priv *priv = lane->priv;
+	u32 val;
+
+	mvebu_comphy_ethernet_init_reset(lane, mode);
+
+	val = readl(priv->base + MVEBU_COMPHY_RX_CTRL1(lane->id));
+	val &= ~MVEBU_COMPHY_RX_CTRL1_CLK8T_EN;
+	val |= MVEBU_COMPHY_RX_CTRL1_RXCLK2X_SEL;
+	writel(val, priv->base + MVEBU_COMPHY_RX_CTRL1(lane->id));
+
+	val = readl(priv->base + MVEBU_COMPHY_DLT_CTRL(lane->id));
+	val &= ~MVEBU_COMPHY_DLT_CTRL_DTL_FLOOP_EN;
+	writel(val, priv->base + MVEBU_COMPHY_DLT_CTRL(lane->id));
+
+	regmap_read(priv->regmap, MVEBU_COMPHY_CONF1(lane->id), &val);
+	val &= ~MVEBU_COMPHY_CONF1_USB_PCIE;
+	val |= MVEBU_COMPHY_CONF1_PWRUP;
+	regmap_write(priv->regmap, MVEBU_COMPHY_CONF1(lane->id), val);
+
+	val = readl(priv->base + MVEBU_COMPHY_GEN1_S0(lane->id));
+	val &= ~MVEBU_COMPHY_GEN1_S0_TX_EMPH(0xf);
+	val |= MVEBU_COMPHY_GEN1_S0_TX_EMPH(0x1);
+	writel(val, priv->base + MVEBU_COMPHY_GEN1_S0(lane->id));
+
+	return mvebu_comphy_init_plls(lane, mode);
+}
+
+static int mvebu_comphy_set_mode_10gkr(struct phy *phy, enum phy_mode mode)
+{
+	struct mvebu_comphy_lane *lane = phy_get_drvdata(phy);
+	struct mvebu_comphy_priv *priv = lane->priv;
+	u32 val;
+
+	mvebu_comphy_ethernet_init_reset(lane, mode);
+
+	val = readl(priv->base + MVEBU_COMPHY_RX_CTRL1(lane->id));
+	val |= MVEBU_COMPHY_RX_CTRL1_RXCLK2X_SEL |
+	       MVEBU_COMPHY_RX_CTRL1_CLK8T_EN;
+	writel(val, priv->base + MVEBU_COMPHY_RX_CTRL1(lane->id));
+
+	val = readl(priv->base + MVEBU_COMPHY_DLT_CTRL(lane->id));
+	val |= MVEBU_COMPHY_DLT_CTRL_DTL_FLOOP_EN;
+	writel(val, priv->base + MVEBU_COMPHY_DLT_CTRL(lane->id));
+
+	/* Speed divider */
+	val = readl(priv->base + MVEBU_COMPHY_SPEED_DIV(lane->id));
+	val |= MVEBU_COMPHY_SPEED_DIV_TX_FORCE;
+	writel(val, priv->base + MVEBU_COMPHY_SPEED_DIV(lane->id));
+
+	val = readl(priv->base + MVEBU_COMPHY_SERDES_CFG2(lane->id));
+	val |= MVEBU_COMPHY_SERDES_CFG2_DFE_EN;
+	writel(val, priv->base + MVEBU_COMPHY_SERDES_CFG2(lane->id));
+
+	/* DFE resolution */
+	val = readl(priv->base + MVEBU_COMPHY_DFE_RES(lane->id));
+	val |= MVEBU_COMPHY_DFE_RES_FORCE_GEN_TBL;
+	writel(val, priv->base + MVEBU_COMPHY_DFE_RES(lane->id));
+
+	val = readl(priv->base + MVEBU_COMPHY_GEN1_S0(lane->id));
+	val &= ~(MVEBU_COMPHY_GEN1_S0_TX_AMP(0x1f) |
+		 MVEBU_COMPHY_GEN1_S0_TX_EMPH(0xf));
+	val |= MVEBU_COMPHY_GEN1_S0_TX_AMP(0x1c) |
+	       MVEBU_COMPHY_GEN1_S0_TX_EMPH(0xe);
+	writel(val, priv->base + MVEBU_COMPHY_GEN1_S0(lane->id));
+
+	val = readl(priv->base + MVEBU_COMPHY_GEN1_S2(lane->id));
+	val &= ~MVEBU_COMPHY_GEN1_S2_TX_EMPH(0xf);
+	val |= MVEBU_COMPHY_GEN1_S2_TX_EMPH_EN;
+	writel(val, priv->base + MVEBU_COMPHY_GEN1_S2(lane->id));
+
+	val = readl(priv->base + MVEBU_COMPHY_TX_SLEW_RATE(lane->id));
+	val |= MVEBU_COMPHY_TX_SLEW_RATE_EMPH(0x3) |
+	       MVEBU_COMPHY_TX_SLEW_RATE_SLC(0x3f);
+	writel(val, priv->base + MVEBU_COMPHY_TX_SLEW_RATE(lane->id));
+
+	/* Impedance calibration */
+	val = readl(priv->base + MVEBU_COMPHY_IMP_CAL(lane->id));
+	val &= ~MVEBU_COMPHY_IMP_CAL_TX_EXT(0x1f);
+	val |= MVEBU_COMPHY_IMP_CAL_TX_EXT(0xe) |
+	       MVEBU_COMPHY_IMP_CAL_TX_EXT_EN;
+	writel(val, priv->base + MVEBU_COMPHY_IMP_CAL(lane->id));
+
+	val = readl(priv->base + MVEBU_COMPHY_GEN1_S5(lane->id));
+	val &= ~MVEBU_COMPHY_GEN1_S5_ICP(0xf);
+	writel(val, priv->base + MVEBU_COMPHY_GEN1_S5(lane->id));
+
+	val = readl(priv->base + MVEBU_COMPHY_GEN1_S1(lane->id));
+	val &= ~(MVEBU_COMPHY_GEN1_S1_RX_MUL_PI(0x7) |
+		 MVEBU_COMPHY_GEN1_S1_RX_MUL_PF(0x7) |
+		 MVEBU_COMPHY_GEN1_S1_RX_MUL_FI(0x3) |
+		 MVEBU_COMPHY_GEN1_S1_RX_MUL_FF(0x3));
+	val |= MVEBU_COMPHY_GEN1_S1_RX_DFE_EN |
+	       MVEBU_COMPHY_GEN1_S1_RX_MUL_PI(0x2) |
+	       MVEBU_COMPHY_GEN1_S1_RX_MUL_PF(0x2) |
+	       MVEBU_COMPHY_GEN1_S1_RX_MUL_FF(0x1) |
+	       MVEBU_COMPHY_GEN1_S1_RX_DIV(0x3);
+	writel(val, priv->base + MVEBU_COMPHY_GEN1_S1(lane->id));
+
+	val = readl(priv->base + MVEBU_COMPHY_COEF(lane->id));
+	val &= ~(MVEBU_COMPHY_COEF_DFE_EN | MVEBU_COMPHY_COEF_DFE_CTRL);
+	writel(val, priv->base + MVEBU_COMPHY_COEF(lane->id));
+
+	val = readl(priv->base + MVEBU_COMPHY_GEN1_S4(lane->id));
+	val &= ~MVEBU_COMPHY_GEN1_S4_DFE_RES(0x3);
+	val |= MVEBU_COMPHY_GEN1_S4_DFE_RES(0x1);
+	writel(val, priv->base + MVEBU_COMPHY_GEN1_S4(lane->id));
+
+	val = readl(priv->base + MVEBU_COMPHY_GEN1_S3(lane->id));
+	val |= MVEBU_COMPHY_GEN1_S3_FBCK_SEL;
+	writel(val, priv->base + MVEBU_COMPHY_GEN1_S3(lane->id));
+
+	/* rx training timer */
+	val = readl(priv->base + MVEBU_COMPHY_TRAINING5(lane->id));
+	val &= ~MVEBU_COMPHY_TRAINING5_RX_TIMER(0x3ff);
+	val |= MVEBU_COMPHY_TRAINING5_RX_TIMER(0x13);
+	writel(val, priv->base + MVEBU_COMPHY_TRAINING5(lane->id));
+
+	/* tx train peak to peak hold */
+	val = readl(priv->base + MVEBU_COMPHY_TRAINING0(lane->id));
+	val |= MVEBU_COMPHY_TRAINING0_P2P_HOLD;
+	writel(val, priv->base + MVEBU_COMPHY_TRAINING0(lane->id));
+
+	val = readl(priv->base + MVEBU_COMPHY_TX_PRESET(lane->id));
+	val &= ~MVEBU_COMPHY_TX_PRESET_INDEX(0xf);
+	val |= MVEBU_COMPHY_TX_PRESET_INDEX(0x2);	/* preset coeff */
+	writel(val, priv->base + MVEBU_COMPHY_TX_PRESET(lane->id));
+
+	val = readl(priv->base + MVEBU_COMPHY_FRAME_DETECT3(lane->id));
+	val &= ~MVEBU_COMPHY_FRAME_DETECT3_LOST_TIMEOUT_EN;
+	writel(val, priv->base + MVEBU_COMPHY_FRAME_DETECT3(lane->id));
+
+	val = readl(priv->base + MVEBU_COMPHY_TX_TRAIN_PRESET(lane->id));
+	val |= MVEBU_COMPHY_TX_TRAIN_PRESET_16B_AUTO_EN |
+	       MVEBU_COMPHY_TX_TRAIN_PRESET_PRBS11;
+	writel(val, priv->base + MVEBU_COMPHY_TX_TRAIN_PRESET(lane->id));
+
+	val = readl(priv->base + MVEBU_COMPHY_FRAME_DETECT0(lane->id));
+	val &= ~MVEBU_COMPHY_FRAME_DETECT0_PATN(0x1ff);
+	val |= MVEBU_COMPHY_FRAME_DETECT0_PATN(0x88);
+	writel(val, priv->base + MVEBU_COMPHY_FRAME_DETECT0(lane->id));
+
+	val = readl(priv->base + MVEBU_COMPHY_DME(lane->id));
+	val |= MVEBU_COMPHY_DME_ETH_MODE;
+	writel(val, priv->base + MVEBU_COMPHY_DME(lane->id));
+
+	val = readl(priv->base + MVEBU_COMPHY_VDD_CAL0(lane->id));
+	val |= MVEBU_COMPHY_VDD_CAL0_CONT_MODE;
+	writel(val, priv->base + MVEBU_COMPHY_VDD_CAL0(lane->id));
+
+	val = readl(priv->base + MVEBU_SP_CALIB(lane->id));
+	val &= ~MVEBU_SP_CALIB_SAMPLER(0x3);
+	val |= MVEBU_SP_CALIB_SAMPLER(0x3) |
+	       MVEBU_SP_CALIB_SAMPLER_EN;
+	writel(val, priv->base + MVEBU_SP_CALIB(lane->id));
+	val &= ~MVEBU_SP_CALIB_SAMPLER_EN;
+	writel(val, priv->base + MVEBU_SP_CALIB(lane->id));
+
+	/* External rx regulator */
+	val = readl(priv->base + MVEBU_COMPHY_EXT_SELV(lane->id));
+	val &= ~MVEBU_COMPHY_EXT_SELV_RX_SAMPL(0x1f);
+	val |= MVEBU_COMPHY_EXT_SELV_RX_SAMPL(0x1a);
+	writel(val, priv->base + MVEBU_COMPHY_EXT_SELV(lane->id));
+
+	return mvebu_comphy_init_plls(lane, mode);
+}
+
+static int mvebu_comphy_power_on(struct phy *phy)
+{
+	struct mvebu_comphy_lane *lane = phy_get_drvdata(phy);
+	struct mvebu_comphy_priv *priv = lane->priv;
+	int ret;
+	u32 mux, val;
+
+	mux = mvebu_comphy_get_mux(lane->id, lane->port, lane->mode);
+	if (mux < 0)
+		return -ENOTSUPP;
+
+	regmap_read(priv->regmap, MVEBU_COMPHY_SELECTOR, &val);
+	val &= ~(0xf << MVEBU_COMPHY_SELECTOR_PHY(lane->id));
+	val |= mux << MVEBU_COMPHY_SELECTOR_PHY(lane->id);
+	regmap_write(priv->regmap, MVEBU_COMPHY_SELECTOR, val);
+
+	switch (lane->mode) {
+	case PHY_MODE_SGMII:
+		ret = mvebu_comphy_set_mode_sgmii(phy, lane->mode);
+		break;
+	case PHY_MODE_10GKR:
+		ret = mvebu_comphy_set_mode_10gkr(phy, lane->mode);
+		break;
+	default:
+		return -ENOTSUPP;
+	}
+
+	/* digital reset */
+	val = readl(priv->base + MVEBU_COMPHY_SERDES_CFG1(lane->id));
+	val |= MVEBU_COMPHY_SERDES_CFG1_RF_RESET;
+	writel(val, priv->base + MVEBU_COMPHY_SERDES_CFG1(lane->id));
+
+	return ret;
+}
+
+static int mvebu_comphy_set_mode(struct phy *phy, enum phy_mode mode)
+{
+	struct mvebu_comphy_lane *lane = phy_get_drvdata(phy);
+
+	if (mvebu_comphy_get_mux(lane->id, lane->port, mode) < 0)
+		return -EINVAL;
+
+	lane->mode = mode;
+	return 0;
+}
+
+static int mvebu_comphy_power_off(struct phy *phy)
+{
+	struct mvebu_comphy_lane *lane = phy_get_drvdata(phy);
+	struct mvebu_comphy_priv *priv = lane->priv;
+	u32 val;
+
+	val = readl(priv->base + MVEBU_COMPHY_SERDES_CFG1(lane->id));
+	val &= ~(MVEBU_COMPHY_SERDES_CFG1_RESET |
+		 MVEBU_COMPHY_SERDES_CFG1_CORE_RESET |
+		 MVEBU_COMPHY_SERDES_CFG1_RF_RESET);
+	writel(val, priv->base + MVEBU_COMPHY_SERDES_CFG1(lane->id));
+
+	regmap_read(priv->regmap, MVEBU_COMPHY_SELECTOR, &val);
+	val &= ~(0xf << MVEBU_COMPHY_SELECTOR_PHY(lane->id));
+	regmap_write(priv->regmap, MVEBU_COMPHY_SELECTOR, val);
+
+	return 0;
+}
+
+static const struct phy_ops mvebu_comphy_ops = {
+	.power_on	= mvebu_comphy_power_on,
+	.power_off	= mvebu_comphy_power_off,
+	.set_mode	= mvebu_comphy_set_mode,
+};
+
+static struct phy *mvebu_comphy_xlate(struct device *dev,
+				      struct of_phandle_args *args)
+{
+	struct mvebu_comphy_priv *priv = dev_get_drvdata(dev);
+	struct mvebu_comphy_lane *lane;
+	int i;
+
+	if (WARN_ON(args->args[0] >= MVEBU_COMPHY_PORTS))
+		return ERR_PTR(-EINVAL);
+
+	for (i = 0; i < MVEBU_COMPHY_LANES; i++) {
+		if (!priv->phys[i])
+			continue;
+
+		lane = phy_get_drvdata(priv->phys[i]);
+		if (priv->phys[i] && args->np == lane->of_node)
+			break;
+	}
+
+	if (i == MVEBU_COMPHY_LANES)
+		return ERR_PTR(-ENODEV);
+
+	if (lane->port >= 0)
+		return ERR_PTR(-EBUSY);
+
+	lane->port = args->args[0];
+	return priv->phys[i];
+}
+
+static int mvebu_comphy_probe(struct platform_device *pdev)
+{
+	struct mvebu_comphy_priv *priv;
+	struct phy_provider *provider;
+	struct device_node *child;
+	struct resource *res;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->dev = &pdev->dev;
+	priv->regmap =
+		syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
+						"marvell,system-controller");
+	if (IS_ERR(priv->regmap))
+		return PTR_ERR(priv->regmap);
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	priv->base = devm_ioremap_resource(&pdev->dev, res);
+	if (!priv->base)
+		return -ENOMEM;
+
+	for_each_available_child_of_node(pdev->dev.of_node, child) {
+		struct mvebu_comphy_lane *lane;
+		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 >= MVEBU_COMPHY_LANES) {
+			dev_err(&pdev->dev, "invalid 'reg' property\n");
+			continue;
+		}
+
+		lane = devm_kzalloc(&pdev->dev, sizeof(*lane), GFP_KERNEL);
+		if (!lane)
+			return -ENOMEM;
+
+		phy = devm_phy_create(&pdev->dev, NULL, &mvebu_comphy_ops);
+		if (IS_ERR(phy))
+			return PTR_ERR(phy);
+
+		lane->priv = priv;
+		lane->of_node = child;
+		lane->mode = PHY_MODE_INVALID;
+		lane->id = val;
+		lane->port = -1;
+		phy_set_drvdata(phy, lane);
+
+		priv->phys[val] = phy;
+
+		/*
+		 * Once all modes are supported in this driver we should call
+		 * mvebu_comphy_power_off(phy) here to avoid relying on the
+		 * bootloader/firmware configuration.
+		 */
+	}
+
+	dev_set_drvdata(&pdev->dev, priv);
+	provider = devm_of_phy_provider_register(&pdev->dev,
+						 mvebu_comphy_xlate);
+	return PTR_ERR_OR_ZERO(provider);
+}
+
+static const struct of_device_id mvebu_comphy_of_match_table[] = {
+	{ .compatible = "marvell,comphy-cp110" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, mvebu_comphy_of_match_table);
+
+static struct platform_driver mvebu_comphy_driver = {
+	.probe	= mvebu_comphy_probe,
+	.driver	= {
+		.name = "mvebu-comphy",
+		.of_match_table = mvebu_comphy_of_match_table,
+	},
+};
+module_platform_driver(mvebu_comphy_driver);
+
+MODULE_AUTHOR("Antoine Tenart <antoine.tenart@free-electrons.com>");
+MODULE_DESCRIPTION("Common PHY driver for mvebu SoCs");
+MODULE_LICENSE("GPL v2");
-- 
2.13.5

Re: [PATCH net-next 02/13] phy: add the mvebu cp110 comphy driver

[Andrew Lunn]

> +static const struct mvebu_comhy_conf mvebu_comphy_modes[] = {
> +	/* lane 0 */
> +	MVEBU_COMPHY_CONF(0, 1, PHY_MODE_SGMII, 0x1),
> +	/* lane 1 */
> +	MVEBU_COMPHY_CONF(1, 2, PHY_MODE_SGMII, 0x1),
> +	/* lane 2 */
> +	MVEBU_COMPHY_CONF(2, 0, PHY_MODE_SGMII, 0x1),
> +	MVEBU_COMPHY_CONF(2, 0, PHY_MODE_10GKR, 0x1),
> +	/* lane 3 */
> +	MVEBU_COMPHY_CONF(3, 1, PHY_MODE_SGMII, 0x2),
> +	/* lane 4 */
> +	MVEBU_COMPHY_CONF(4, 0, PHY_MODE_SGMII, 0x2),
> +	MVEBU_COMPHY_CONF(4, 0, PHY_MODE_10GKR, 0x2),
> +	MVEBU_COMPHY_CONF(4, 1, PHY_MODE_SGMII, 0x1),
> +	/* lane 5 */
> +	MVEBU_COMPHY_CONF(5, 2, PHY_MODE_SGMII, 0x1),
> +};

Do other Marvell SoCs re-use this IP? Maybe add cp110 to the name here
to indicate what SoC this configuration belongs to? I guess at some
point, the compatible string will be used to select the correct table
for the hardware variant.

> +static const struct of_device_id mvebu_comphy_of_match_table[] = {
> +	{ .compatible = "marvell,comphy-cp110" },

Is that specific enough? It seems like this table is easy to change in
the VHDL. Could there be another cp110 with a different configuration?

    Andrew

Re: [PATCH net-next 02/13] phy: add the mvebu cp110 comphy driver

[Antoine Tenart]

[-- Attachment #1: Type: text/plain, Size: 1685 bytes --]

Hi Andrew,

On Thu, Aug 24, 2017 at 03:39:22PM +0200, Andrew Lunn wrote:
> > +static const struct mvebu_comhy_conf mvebu_comphy_modes[] = {
> > +	/* lane 0 */
> > +	MVEBU_COMPHY_CONF(0, 1, PHY_MODE_SGMII, 0x1),
> > +	/* lane 1 */
> > +	MVEBU_COMPHY_CONF(1, 2, PHY_MODE_SGMII, 0x1),
> > +	/* lane 2 */
> > +	MVEBU_COMPHY_CONF(2, 0, PHY_MODE_SGMII, 0x1),
> > +	MVEBU_COMPHY_CONF(2, 0, PHY_MODE_10GKR, 0x1),
> > +	/* lane 3 */
> > +	MVEBU_COMPHY_CONF(3, 1, PHY_MODE_SGMII, 0x2),
> > +	/* lane 4 */
> > +	MVEBU_COMPHY_CONF(4, 0, PHY_MODE_SGMII, 0x2),
> > +	MVEBU_COMPHY_CONF(4, 0, PHY_MODE_10GKR, 0x2),
> > +	MVEBU_COMPHY_CONF(4, 1, PHY_MODE_SGMII, 0x1),
> > +	/* lane 5 */
> > +	MVEBU_COMPHY_CONF(5, 2, PHY_MODE_SGMII, 0x1),
> > +};
> 
> Do other Marvell SoCs re-use this IP? Maybe add cp110 to the name here
> to indicate what SoC this configuration belongs to? I guess at some
> point, the compatible string will be used to select the correct table
> for the hardware variant.

OK, I'll rename the variable mvebu_comphy_cp110_modes.

> > +static const struct of_device_id mvebu_comphy_of_match_table[] = {
> > +	{ .compatible = "marvell,comphy-cp110" },
> 
> Is that specific enough? It seems like this table is easy to change in
> the VHDL. Could there be another cp110 with a different configuration?

As far as I understand CP110 is the name of the CP, should there be
another one it should be named differently. But I can't be 100% sure,
you never know what comes next :)

How would you name it if not "comphy-cp110"?

Thanks!
Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH net-next 02/13] phy: add the mvebu cp110 comphy driver

[Andrew Lunn]

Hi Antione.

> How would you name it if not "comphy-cp110"?

Good question...

'7000-cpmphy-cp110'
'8000-cpmphy-cp110'

??

	Andrew

RE: [PATCH net-next 02/13] phy: add the mvebu cp110 comphy driver

[Stefan Chulski]

> -----Original Message-----
> From: Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: Thursday, August 24, 2017 4:51 PM
> To: Antoine Tenart <antoine.tenart@free-electrons.com>
> Cc: davem@davemloft.net; kishon@ti.com; jason@lakedaemon.net;
> sebastian.hesselbarth@gmail.com; gregory.clement@free-electrons.com;
> thomas.petazzoni@free-electrons.com; Nadav Haklai <nadavh@marvell.com>;
> linux@armlinux.org.uk; linux-kernel@vger.kernel.org; mw@semihalf.com;
> Stefan Chulski <stefanc@marvell.com>; miquel.raynal@free-electrons.com;
> netdev@vger.kernel.org
> Subject: Re: [PATCH net-next 02/13] phy: add the mvebu cp110 comphy driver
> 
> Hi Antione.
> 
> > How would you name it if not "comphy-cp110"?
> 
> Good question...
> 
> '7000-cpmphy-cp110'
> '8000-cpmphy-cp110'
> 
> ??
> 
> 	Andrew

A8K Marvell SoC has two South Bridge communication controllers
and A7K only one communication controllers, but its identical
communication controllers 110. Next generation will has another number 1xx.

Stefan,
Regards.

RE: [PATCH net-next 02/13] phy: add the mvebu cp110 comphy driver

[Stefan Chulski]

> -----Original Message-----
> From: Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: Thursday, August 24, 2017 4:51 PM
> To: Antoine Tenart <antoine.tenart@free-electrons.com>
> Cc: davem@davemloft.net; kishon@ti.com; jason@lakedaemon.net;
> sebastian.hesselbarth@gmail.com; gregory.clement@free-electrons.com;
> thomas.petazzoni@free-electrons.com; Nadav Haklai <nadavh@marvell.com>;
> linux@armlinux.org.uk; linux-kernel@vger.kernel.org; mw@semihalf.com;
> Stefan Chulski <stefanc@marvell.com>; miquel.raynal@free-electrons.com;
> netdev@vger.kernel.org
> Subject: Re: [PATCH net-next 02/13] phy: add the mvebu cp110 comphy driver
> 
> Hi Antione.
> 
> > How would you name it if not "comphy-cp110"?
> 
> Good question...
> 
> '7000-cpmphy-cp110'
> '8000-cpmphy-cp110'
> 
> ??
> 
> 	Andrew

A8K Marvell SoC has two South Bridge communication controllers
and A7K only one communication controllers, but its identical
communication controllers 110. Next generation will has another number 1xx.

Stefan,
Regards.

Re: [PATCH net-next 02/13] phy: add the mvebu cp110 comphy driver

[Antoine Tenart]

[-- Attachment #1: Type: text/plain, Size: 642 bytes --]

Hi Stefan,

On Thu, Aug 24, 2017 at 01:57:04PM +0000, Stefan Chulski wrote:
> 
> > > How would you name it if not "comphy-cp110"?
> > 
> > Good question...
> > 
> > '7000-cpmphy-cp110'
> > '8000-cpmphy-cp110'
> > 
> > ??
> > 
> > 	Andrew
> 
> A8K Marvell SoC has two South Bridge communication controllers
> and A7K only one communication controllers, but its identical
> communication controllers 110. Next generation will has another number 1xx.

OK, so I guess we can keep 'comphy-cp110' then.

Thanks!
Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH net-next 02/13] phy: add the mvebu cp110 comphy driver

[Antoine Tenart]

[-- Attachment #1: Type: text/plain, Size: 642 bytes --]

Hi Stefan,

On Thu, Aug 24, 2017 at 01:57:04PM +0000, Stefan Chulski wrote:
> 
> > > How would you name it if not "comphy-cp110"?
> > 
> > Good question...
> > 
> > '7000-cpmphy-cp110'
> > '8000-cpmphy-cp110'
> > 
> > ??
> > 
> > 	Andrew
> 
> A8K Marvell SoC has two South Bridge communication controllers
> and A7K only one communication controllers, but its identical
> communication controllers 110. Next generation will has another number 1xx.

OK, so I guess we can keep 'comphy-cp110' then.

Thanks!
Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH net-next 02/13] phy: add the mvebu cp110 comphy driver

[Andrew Lunn]

> > -----Original Message-----
> > From: Andrew Lunn [mailto:andrew@lunn.ch]
> > Sent: Thursday, August 24, 2017 4:51 PM
> > To: Antoine Tenart <antoine.tenart@free-electrons.com>
> > Cc: davem@davemloft.net; kishon@ti.com; jason@lakedaemon.net;
> > sebastian.hesselbarth@gmail.com; gregory.clement@free-electrons.com;
> > thomas.petazzoni@free-electrons.com; Nadav Haklai <nadavh@marvell.com>;
> > linux@armlinux.org.uk; linux-kernel@vger.kernel.org; mw@semihalf.com;
> > Stefan Chulski <stefanc@marvell.com>; miquel.raynal@free-electrons.com;
> > netdev@vger.kernel.org
> > Subject: Re: [PATCH net-next 02/13] phy: add the mvebu cp110 comphy driver
> > 
> > Hi Antione.
> > 
> > > How would you name it if not "comphy-cp110"?
> > 
> > Good question...
> > 
> > '7000-cpmphy-cp110'
> > '8000-cpmphy-cp110'
> > 
> > ??
> > 
> > 	Andrew
> 
> A8K Marvell SoC has two South Bridge communication controllers
> and A7K only one communication controllers, but its identical
> communication controllers 110. Next generation will has another number 1xx.

Save this email, so we know who to blame when Marvell does something
different :-)

	  Andrew

Re: [PATCH net-next 02/13] phy: add the mvebu cp110 comphy driver

[Andrew Lunn]

> > -----Original Message-----
> > From: Andrew Lunn [mailto:andrew@lunn.ch]
> > Sent: Thursday, August 24, 2017 4:51 PM
> > To: Antoine Tenart <antoine.tenart@free-electrons.com>
> > Cc: davem@davemloft.net; kishon@ti.com; jason@lakedaemon.net;
> > sebastian.hesselbarth@gmail.com; gregory.clement@free-electrons.com;
> > thomas.petazzoni@free-electrons.com; Nadav Haklai <nadavh@marvell.com>;
> > linux@armlinux.org.uk; linux-kernel@vger.kernel.org; mw@semihalf.com;
> > Stefan Chulski <stefanc@marvell.com>; miquel.raynal@free-electrons.com;
> > netdev@vger.kernel.org
> > Subject: Re: [PATCH net-next 02/13] phy: add the mvebu cp110 comphy driver
> > 
> > Hi Antione.
> > 
> > > How would you name it if not "comphy-cp110"?
> > 
> > Good question...
> > 
> > '7000-cpmphy-cp110'
> > '8000-cpmphy-cp110'
> > 
> > ??
> > 
> > 	Andrew
> 
> A8K Marvell SoC has two South Bridge communication controllers
> and A7K only one communication controllers, but its identical
> communication controllers 110. Next generation will has another number 1xx.

Save this email, so we know who to blame when Marvell does something
different :-)

	  Andrew

Re: [PATCH net-next 02/13] phy: add the mvebu cp110 comphy driver

[Andrew Lunn]

> +	for_each_available_child_of_node(pdev->dev.of_node, child) {
> +		struct mvebu_comphy_lane *lane;
> +		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;
> +		}

I'm just wondering why we need this. We know how many lanes there are
from the table. So just create a generic PHY for each lane?

     Andrew

Re: [PATCH net-next 02/13] phy: add the mvebu cp110 comphy driver

[Antoine Tenart]

[-- Attachment #1: Type: text/plain, Size: 1033 bytes --]

Hi Andrew,

On Thu, Aug 24, 2017 at 03:45:04PM +0200, Andrew Lunn wrote:
> > +	for_each_available_child_of_node(pdev->dev.of_node, child) {
> > +		struct mvebu_comphy_lane *lane;
> > +		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;
> > +		}
> 
> I'm just wondering why we need this. We know how many lanes there are
> from the table. So just create a generic PHY for each lane?

At first I did this statically. I moved to this solution because:
1. It represents the h/w correctly (there are 6 lanes duplicated here).
2. It eases the dt representation, we would have something like:
   <&cpm_comphy 0 1>; otherwise.
3. If the number of lanes changes in future revisions it'll be quite
   easy to handle.

Thanks!
Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH net-next 03/13] Documentation/bindings: phy: document the Marvell comphy driver

[Antoine Tenart]

The Marvell Armada 7K/8K SoCs contains an hardware block called COMPHY
that provides a number of shared PHYs used by various interfaces in the
SoC: network, SATA, PCIe, etc. This Device Tree binding allows to
describe this COMPHY hardware block.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 .../devicetree/bindings/phy/phy-mvebu-comphy.txt   | 42 ++++++++++++++++++++++
 1 file changed, 42 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/phy-mvebu-comphy.txt

diff --git a/Documentation/devicetree/bindings/phy/phy-mvebu-comphy.txt b/Documentation/devicetree/bindings/phy/phy-mvebu-comphy.txt
new file mode 100644
index 000000000000..dc3726269e1d
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/phy-mvebu-comphy.txt
@@ -0,0 +1,42 @@
+mvebu comphy driver
+-------------------
+
+A comphy controller can be found on Marvell Armada 7k/8k on the CP110. It
+provides a number of shared PHYs used by various interfaces (network, sata,
+usb, PCIe...).
+
+Required properties:
+
+- compatible: should be "marvell,comphy-cp110"
+- reg: should contain the comphy register location and length.
+- marvell,system-controller: should contain a phandle to the
+                             system controller node.
+- #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.
+
+Example:
+
+	cpm_comphy: phy@120000 {
+		compatible = "marvell,comphy-cp110";
+		reg = <0x120000 0x6000>;
+		marvell,system-controller = <&cpm_syscon0>;
+		#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.13.5

[PATCH net-next 04/13] net: mvpp2: initialize the comphy

[Antoine Tenart]

On some platforms, the comphy is between the MAC GoP and the PHYs. The
mvpp2 driver currently relies on the firmware/bootloader to configure
the comphy. As a comphy driver was added to the generic PHY framework,
this patch uses it in the mvpp2 driver to configure the comphy at boot
time to avoid relying on the bootloader.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 44 +++++++++++++++++++++++++++++++++++-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index 7fa251bf91ae..5f188de63caf 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -28,6 +28,7 @@
 #include <linux/of_address.h>
 #include <linux/of_device.h>
 #include <linux/phy.h>
+#include <linux/phy/phy.h>
 #include <linux/clk.h>
 #include <linux/hrtimer.h>
 #include <linux/ktime.h>
@@ -861,6 +862,7 @@ struct mvpp2_port {
 
 	phy_interface_t phy_interface;
 	struct device_node *phy_node;
+	struct phy *comphy;
 	unsigned int link;
 	unsigned int duplex;
 	unsigned int speed;
@@ -4420,6 +4422,32 @@ static int mvpp22_gop_init(struct mvpp2_port *port)
 	return -EINVAL;
 }
 
+static int mvpp22_comphy_init(struct mvpp2_port *port)
+{
+	enum phy_mode mode;
+	int ret;
+
+	if (!port->comphy)
+		return 0;
+
+	switch (port->phy_interface) {
+	case PHY_INTERFACE_MODE_SGMII:
+		mode = PHY_MODE_SGMII;
+		break;
+	case PHY_INTERFACE_MODE_10GKR:
+		mode = PHY_MODE_10GKR;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	ret = phy_set_mode(port->comphy, mode);
+	if (ret)
+		return ret;
+
+	return phy_power_on(port->comphy);
+}
+
 static void mvpp2_port_mii_gmac_configure_mode(struct mvpp2_port *port)
 {
 	u32 val;
@@ -6404,8 +6432,10 @@ static void mvpp2_start_dev(struct mvpp2_port *port)
 	/* Enable interrupts on all CPUs */
 	mvpp2_interrupts_enable(port);
 
-	if (port->priv->hw_version == MVPP22)
+	if (port->priv->hw_version == MVPP22) {
+		mvpp22_comphy_init(port);
 		mvpp22_gop_init(port);
+	}
 
 	mvpp2_port_mii_set(port);
 	mvpp2_port_enable(port);
@@ -6436,6 +6466,7 @@ static void mvpp2_stop_dev(struct mvpp2_port *port)
 	mvpp2_egress_disable(port);
 	mvpp2_port_disable(port);
 	phy_stop(ndev->phydev);
+	phy_power_off(port->comphy);
 }
 
 static int mvpp2_check_ringparam_valid(struct net_device *dev,
@@ -7242,6 +7273,7 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 			    struct mvpp2 *priv)
 {
 	struct device_node *phy_node;
+	struct phy *comphy;
 	struct mvpp2_port *port;
 	struct mvpp2_port_pcpu *port_pcpu;
 	struct net_device *dev;
@@ -7285,6 +7317,15 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 		goto err_free_netdev;
 	}
 
+	comphy = devm_of_phy_get(&pdev->dev, port_node, NULL);
+	if (IS_ERR(comphy)) {
+		if (PTR_ERR(comphy) == -EPROBE_DEFER) {
+			err = -EPROBE_DEFER;
+			goto err_free_netdev;
+		}
+		comphy = NULL;
+	}
+
 	if (of_property_read_u32(port_node, "port-id", &id)) {
 		err = -EINVAL;
 		dev_err(&pdev->dev, "missing port-id value\n");
@@ -7318,6 +7359,7 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 
 	port->phy_node = phy_node;
 	port->phy_interface = phy_mode;
+	port->comphy = comphy;
 
 	if (priv->hw_version == MVPP21) {
 		res = platform_get_resource(pdev, IORESOURCE_MEM, 2 + id);
-- 
2.13.5

[PATCH net-next 05/13] net: mvpp2: do not force the link mode

[Antoine Tenart]

The link mode (speed, duplex) was forced based on what the phylib
returns. This should not be the case, and only forced by ethtool
functions manually. This patch removes the link mode enforcement from
the phylib link_event callback.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 24 ------------------------
 1 file changed, 24 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index 5f188de63caf..49506bdde19a 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -5741,30 +5741,10 @@ static void mvpp2_link_event(struct net_device *dev)
 	struct mvpp2_port *port = netdev_priv(dev);
 	struct phy_device *phydev = dev->phydev;
 	int status_change = 0;
-	u32 val;
 
 	if (phydev->link) {
 		if ((port->speed != phydev->speed) ||
 		    (port->duplex != phydev->duplex)) {
-			u32 val;
-
-			val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
-			val &= ~(MVPP2_GMAC_CONFIG_MII_SPEED |
-				 MVPP2_GMAC_CONFIG_GMII_SPEED |
-				 MVPP2_GMAC_CONFIG_FULL_DUPLEX |
-				 MVPP2_GMAC_AN_SPEED_EN |
-				 MVPP2_GMAC_AN_DUPLEX_EN);
-
-			if (phydev->duplex)
-				val |= MVPP2_GMAC_CONFIG_FULL_DUPLEX;
-
-			if (phydev->speed == SPEED_1000)
-				val |= MVPP2_GMAC_CONFIG_GMII_SPEED;
-			else if (phydev->speed == SPEED_100)
-				val |= MVPP2_GMAC_CONFIG_MII_SPEED;
-
-			writel(val, port->base + MVPP2_GMAC_AUTONEG_CONFIG);
-
 			port->duplex = phydev->duplex;
 			port->speed  = phydev->speed;
 		}
@@ -5782,10 +5762,6 @@ static void mvpp2_link_event(struct net_device *dev)
 
 	if (status_change) {
 		if (phydev->link) {
-			val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
-			val |= (MVPP2_GMAC_FORCE_LINK_PASS |
-				MVPP2_GMAC_FORCE_LINK_DOWN);
-			writel(val, port->base + MVPP2_GMAC_AUTONEG_CONFIG);
 			mvpp2_egress_enable(port);
 			mvpp2_ingress_enable(port);
 		} else {
-- 
2.13.5

[PATCH net-next 06/13] net: mvpp2: simplify the link_event function

[Antoine Tenart]

The link_event function is somewhat complicated. This cosmetic patch
simplifies it.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index 49506bdde19a..ebcc89b8f792 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -5740,7 +5740,6 @@ static void mvpp2_link_event(struct net_device *dev)
 {
 	struct mvpp2_port *port = netdev_priv(dev);
 	struct phy_device *phydev = dev->phydev;
-	int status_change = 0;
 
 	if (phydev->link) {
 		if ((port->speed != phydev->speed) ||
@@ -5751,23 +5750,19 @@ static void mvpp2_link_event(struct net_device *dev)
 	}
 
 	if (phydev->link != port->link) {
-		if (!phydev->link) {
-			port->duplex = -1;
-			port->speed = 0;
-		}
-
 		port->link = phydev->link;
-		status_change = 1;
-	}
 
-	if (status_change) {
 		if (phydev->link) {
 			mvpp2_egress_enable(port);
 			mvpp2_ingress_enable(port);
 		} else {
+			port->duplex = -1;
+			port->speed = 0;
+
 			mvpp2_ingress_disable(port);
 			mvpp2_egress_disable(port);
 		}
+
 		phy_print_status(phydev);
 	}
 }
-- 
2.13.5

[PATCH net-next 07/13] net: mvpp2: improve the link management function

[Antoine Tenart]

When the link status changes, the phylib calls the link_event function
in the mvpp2 driver. Before this patch only the egress/ingress transmit
was enabled/disabled. This patch adds more functionality to the link
status management code by enabling/disabling the port per-cpu
interrupts, and the port itself. The queues are now stopped as well, and
the netif carrier helpers are called.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index ebcc89b8f792..99847fec1c5a 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -5753,14 +5753,24 @@ static void mvpp2_link_event(struct net_device *dev)
 		port->link = phydev->link;
 
 		if (phydev->link) {
+			mvpp2_interrupts_enable(port);
+			mvpp2_port_enable(port);
+
 			mvpp2_egress_enable(port);
 			mvpp2_ingress_enable(port);
+			netif_carrier_on(dev);
+			netif_tx_wake_all_queues(dev);
 		} else {
 			port->duplex = -1;
 			port->speed = 0;
 
+			netif_tx_stop_all_queues(dev);
+			netif_carrier_off(dev);
 			mvpp2_ingress_disable(port);
 			mvpp2_egress_disable(port);
+
+			mvpp2_port_disable(port);
+			mvpp2_interrupts_disable(port);
 		}
 
 		phy_print_status(phydev);
-- 
2.13.5

Re: [PATCH net-next 07/13] net: mvpp2: improve the link management function

[Andrew Lunn]

On Thu, Aug 24, 2017 at 10:38:17AM +0200, Antoine Tenart wrote:
> When the link status changes, the phylib calls the link_event function
> in the mvpp2 driver. Before this patch only the egress/ingress transmit
> was enabled/disabled. This patch adds more functionality to the link
> status management code by enabling/disabling the port per-cpu
> interrupts, and the port itself. The queues are now stopped as well, and
> the netif carrier helpers are called.
> 
> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> ---
>  drivers/net/ethernet/marvell/mvpp2.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
> index ebcc89b8f792..99847fec1c5a 100644
> --- a/drivers/net/ethernet/marvell/mvpp2.c
> +++ b/drivers/net/ethernet/marvell/mvpp2.c
> @@ -5753,14 +5753,24 @@ static void mvpp2_link_event(struct net_device *dev)
>  		port->link = phydev->link;
>  
>  		if (phydev->link) {
> +			mvpp2_interrupts_enable(port);
> +			mvpp2_port_enable(port);
> +
>  			mvpp2_egress_enable(port);
>  			mvpp2_ingress_enable(port);
> +			netif_carrier_on(dev);

Hi Antoine

Have you seen cases where it is required to change the carrier state?
The phy state machine should be doing this. e.g. when autoneg has
completed, force link configuration, the link goes down etc.

	   Andrew

Re: [PATCH net-next 07/13] net: mvpp2: improve the link management function

[Antoine Tenart]

[-- Attachment #1: Type: text/plain, Size: 994 bytes --]

Hi Andrew,

On Thu, Aug 24, 2017 at 04:06:25PM +0200, Andrew Lunn wrote:
> On Thu, Aug 24, 2017 at 10:38:17AM +0200, Antoine Tenart wrote:
> > @@ -5753,14 +5753,24 @@ static void mvpp2_link_event(struct net_device *dev)
> >  		port->link = phydev->link;
> >  
> >  		if (phydev->link) {
> > +			mvpp2_interrupts_enable(port);
> > +			mvpp2_port_enable(port);
> > +
> >  			mvpp2_egress_enable(port);
> >  			mvpp2_ingress_enable(port);
> > +			netif_carrier_on(dev);
> 
> Have you seen cases where it is required to change the carrier state?
> The phy state machine should be doing this. e.g. when autoneg has
> completed, force link configuration, the link goes down etc.

I don't think I saw a case where this was needed. And if phylib already
handle this I think it should be removed from here as the
mvpp2_link_event only is called by phylib.

Thanks!
Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH net-next 08/13] net: mvpp2: check the netif is running in the link_event function

[Antoine Tenart]

This patch adds an extra check when the link_event function is called,
so that it won't do anything when the netif isn't running.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index 99847fec1c5a..e9fd4fa81a2e 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -5741,6 +5741,9 @@ static void mvpp2_link_event(struct net_device *dev)
 	struct mvpp2_port *port = netdev_priv(dev);
 	struct phy_device *phydev = dev->phydev;
 
+	if (!netif_running(dev))
+		return;
+
 	if (phydev->link) {
 		if ((port->speed != phydev->speed) ||
 		    (port->duplex != phydev->duplex)) {
-- 
2.13.5

[PATCH net-next 09/13] net: mvpp2: dynamic reconfiguration of the PHY mode

[Antoine Tenart]

This patch adds logic to reconfigure the comphy/gop when the link status
change at runtime. This is very useful on boards such as the mcbin which
have SFP and Ethernet ports connected to the same MAC port: depending on
what the user connects the driver will automatically reconfigure the
link mode.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index e9fd4fa81a2e..31903c2a41c5 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -5740,6 +5740,7 @@ static void mvpp2_link_event(struct net_device *dev)
 {
 	struct mvpp2_port *port = netdev_priv(dev);
 	struct phy_device *phydev = dev->phydev;
+	bool link_reconfigured = false;
 
 	if (!netif_running(dev))
 		return;
@@ -5750,9 +5751,27 @@ static void mvpp2_link_event(struct net_device *dev)
 			port->duplex = phydev->duplex;
 			port->speed  = phydev->speed;
 		}
+
+		if (port->phy_interface != phydev->interface && port->comphy) {
+	                /* disable current port for reconfiguration */
+	                mvpp2_interrupts_disable(port);
+	                netif_carrier_off(port->dev);
+	                mvpp2_port_disable(port);
+			phy_power_off(port->comphy);
+
+	                /* comphy reconfiguration */
+	                port->phy_interface = phydev->interface;
+	                mvpp22_comphy_init(port);
+
+	                /* gop/mac reconfiguration */
+	                mvpp22_gop_init(port);
+	                mvpp2_port_mii_set(port);
+
+	                link_reconfigured = true;
+		}
 	}
 
-	if (phydev->link != port->link) {
+	if (phydev->link != port->link || link_reconfigured) {
 		port->link = phydev->link;
 
 		if (phydev->link) {
-- 
2.13.5

Re: [PATCH net-next 09/13] net: mvpp2: dynamic reconfiguration of the PHY mode

[Andrew Lunn]

On Thu, Aug 24, 2017 at 10:38:19AM +0200, Antoine Tenart wrote:
> This patch adds logic to reconfigure the comphy/gop when the link status
> change at runtime. This is very useful on boards such as the mcbin which
> have SFP and Ethernet ports connected to the same MAC port: depending on
> what the user connects the driver will automatically reconfigure the
> link mode.

Hi Antoine

I would expect each of these external Ethernet ports to have its own
Ethernet PHY. Don't you need to disconnect from one Ethernet phy and
connect to the other Ethernet PHY when you change external Ethernet
port?

	Andrew

Re: [PATCH net-next 09/13] net: mvpp2: dynamic reconfiguration of the PHY mode

[Antoine Tenart]

[-- Attachment #1: Type: text/plain, Size: 1084 bytes --]

Hi Andrew,

On Thu, Aug 24, 2017 at 04:56:09PM +0200, Andrew Lunn wrote:
> On Thu, Aug 24, 2017 at 10:38:19AM +0200, Antoine Tenart wrote:
> > This patch adds logic to reconfigure the comphy/gop when the link status
> > change at runtime. This is very useful on boards such as the mcbin which
> > have SFP and Ethernet ports connected to the same MAC port: depending on
> > what the user connects the driver will automatically reconfigure the
> > link mode.
> 
> I would expect each of these external Ethernet ports to have its own
> Ethernet PHY. Don't you need to disconnect from one Ethernet phy and
> connect to the other Ethernet PHY when you change external Ethernet
> port?

That's the other way around. The engines outputs (say GoP#) are
connected to the comphy inputs. In the SoC. Then there's a single output
of this comphy lane to the board. So when switching modes, you do not
have to connect to a different Ethernet PHY, it's the same.

Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH net-next 09/13] net: mvpp2: dynamic reconfiguration of the PHY mode

[Andrew Lunn]

On Thu, Aug 24, 2017 at 05:52:41PM +0200, Antoine Tenart wrote:
> Hi Andrew,
> 
> On Thu, Aug 24, 2017 at 04:56:09PM +0200, Andrew Lunn wrote:
> > On Thu, Aug 24, 2017 at 10:38:19AM +0200, Antoine Tenart wrote:
> > > This patch adds logic to reconfigure the comphy/gop when the link status
> > > change at runtime. This is very useful on boards such as the mcbin which
> > > have SFP and Ethernet ports connected to the same MAC port: depending on
> > > what the user connects the driver will automatically reconfigure the
> > > link mode.
> > 
> > I would expect each of these external Ethernet ports to have its own
> > Ethernet PHY. Don't you need to disconnect from one Ethernet phy and
> > connect to the other Ethernet PHY when you change external Ethernet
> > port?
> 
> That's the other way around. The engines outputs (say GoP#) are
> connected to the comphy inputs. In the SoC. Then there's a single output
> of this comphy lane to the board. So when switching modes, you do not
> have to connect to a different Ethernet PHY, it's the same.

Hi Antoine

I think there is a mixup here between generic PHY and Ethernet PHY.

When you swap from the copper RJ45 to the fibre SFP, the phylib needs
to swap from the Copper Ethernet PHY driving the RJ45, to the PHY
driving the SFP module, which is probably a fixed-phy.

I actually think this is why you have the carrier_on/off calls in the
link modify callback.

Imagine phylib is using the copper Ethernet PHY, but the MAC is using
the SFP port. Somebody pulls out the copper cable, phylib says the
link is down, turns the carrier off and calls the callback. Not good,
since your SFP cable is still plugged in...  Ethtool is
returning/setting stuff in the Copper Ethernet PHY, when in fact you
intend to be setting SFP settings.

       Andrew

Re: [PATCH net-next 09/13] net: mvpp2: dynamic reconfiguration of the PHY mode

[Antoine Tenart]

[-- Attachment #1: Type: text/plain, Size: 2621 bytes --]

On Thu, Aug 24, 2017 at 06:01:24PM +0200, Andrew Lunn wrote:
> On Thu, Aug 24, 2017 at 05:52:41PM +0200, Antoine Tenart wrote:
> > On Thu, Aug 24, 2017 at 04:56:09PM +0200, Andrew Lunn wrote:
> > > On Thu, Aug 24, 2017 at 10:38:19AM +0200, Antoine Tenart wrote:
> > > > This patch adds logic to reconfigure the comphy/gop when the link status
> > > > change at runtime. This is very useful on boards such as the mcbin which
> > > > have SFP and Ethernet ports connected to the same MAC port: depending on
> > > > what the user connects the driver will automatically reconfigure the
> > > > link mode.
> > > 
> > > I would expect each of these external Ethernet ports to have its own
> > > Ethernet PHY. Don't you need to disconnect from one Ethernet phy and
> > > connect to the other Ethernet PHY when you change external Ethernet
> > > port?
> > 
> > That's the other way around. The engines outputs (say GoP#) are
> > connected to the comphy inputs. In the SoC. Then there's a single output
> > of this comphy lane to the board. So when switching modes, you do not
> > have to connect to a different Ethernet PHY, it's the same.
> 
> I think there is a mixup here between generic PHY and Ethernet PHY.
> 
> When you swap from the copper RJ45 to the fibre SFP, the phylib needs
> to swap from the Copper Ethernet PHY driving the RJ45, to the PHY
> driving the SFP module, which is probably a fixed-phy.

Or on the mcbin the Alaska X 88X3310 which can operate from 10G to 10M.
This PHY changes its interface mode depending on what speed is
negotiated.

> I actually think this is why you have the carrier_on/off calls in the
> link modify callback.

Well, I still do not know if these calls are *really* needed. At least
in our case.

> Imagine phylib is using the copper Ethernet PHY, but the MAC is using
> the SFP port. Somebody pulls out the copper cable, phylib says the
> link is down, turns the carrier off and calls the callback. Not good,
> since your SFP cable is still plugged in...  Ethtool is
> returning/setting stuff in the Copper Ethernet PHY, when in fact you
> intend to be setting SFP settings.

I see what could be the issue but I do not understand one aspect though:
how could we switch from one PHY to another, as there's only one output
between the SoC (and so a given GoP#) and the board. So if a given PHY
can handle multiple modes I see, but in the other case a muxing
somewhere would be needed? Or did I miss something?

Thanks!
Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH net-next 09/13] net: mvpp2: dynamic reconfiguration of the PHY mode

[Andrew Lunn]

> I see what could be the issue but I do not understand one aspect though:
> how could we switch from one PHY to another, as there's only one output
> between the SoC (and so a given GoP#) and the board. So if a given PHY
> can handle multiple modes I see, but in the other case a muxing
> somewhere would be needed? Or did I miss something?

I think we need a hardware diagram...

How are the RJ45, copper PHY, SFP module connected to the SoC?

Somewhere there must be a mux, to select between copper and
fibre. Where is that mux?

    Andrew

Re: [PATCH net-next 09/13] net: mvpp2: dynamic reconfiguration of the PHY mode

[Russell King - ARM Linux]

On Thu, Aug 24, 2017 at 06:57:43PM +0200, Andrew Lunn wrote:
> > I see what could be the issue but I do not understand one aspect though:
> > how could we switch from one PHY to another, as there's only one output
> > between the SoC (and so a given GoP#) and the board. So if a given PHY
> > can handle multiple modes I see, but in the other case a muxing
> > somewhere would be needed? Or did I miss something?
> 
> I think we need a hardware diagram...
> 
> How are the RJ45, copper PHY, SFP module connected to the SoC?
> 
> Somewhere there must be a mux, to select between copper and
> fibre. Where is that mux?

In the 88x3310 PHY:

                     .------- RJ45
MVPP2 ----- 88x3310 PHY
                     `------- SFP+

Here's the commentry I've provided at the very top of the 88x3310 driver
which describes all these modes:

 * There appears to be several different data paths through the PHY which
 * are automatically managed by the PHY.  The following has been determined
 * via observation and experimentation:
 *
 *       SGMII PHYXS -- BASE-T PCS -- 10G PMA -- AN -- Copper (for <= 1G)
 *  10GBASE-KR PHYXS -- BASE-T PCS -- 10G PMA -- AN -- Copper (for 10G)
 *  10GBASE-KR PHYXS -- BASE-R PCS -- Fiber
 *
 * If both the fiber and copper ports are connected, the first to gain
 * link takes priority and the other port is completely locked out.

It's not a copper-only PHY, it's just like most other PHYs out there
that support multiple connections, like the 88e151x series that support
both RJ45 and fibre and can auto-switch between them.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

Re: [PATCH net-next 09/13] net: mvpp2: dynamic reconfiguration of the PHY mode

[Antoine Tenart]

[-- Attachment #1: Type: text/plain, Size: 1135 bytes --]

On Thu, Aug 24, 2017 at 06:04:01PM +0100, Russell King - ARM Linux wrote:
> On Thu, Aug 24, 2017 at 06:57:43PM +0200, Andrew Lunn wrote:
> > > I see what could be the issue but I do not understand one aspect though:
> > > how could we switch from one PHY to another, as there's only one output
> > > between the SoC (and so a given GoP#) and the board. So if a given PHY
> > > can handle multiple modes I see, but in the other case a muxing
> > > somewhere would be needed? Or did I miss something?
> > 
> > I think we need a hardware diagram...
> > 
> > How are the RJ45, copper PHY, SFP module connected to the SoC?
> > 
> > Somewhere there must be a mux, to select between copper and
> > fibre. Where is that mux?
> 
> In the 88x3310 PHY:
> 
>                      .------- RJ45
> MVPP2 ----- 88x3310 PHY
>                      `------- SFP+

And the "MVPP2" part can be expended to:

        .-- GoP #0 --.
MVPP2 ----- GoP #1 ---- Comphy lane #X -- 88x3310
        `-- GoP #2 --'

Thanks!
Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

RE: [EXT] Re: [PATCH net-next 09/13] net: mvpp2: dynamic reconfiguration of the PHY mode

[Stefan Chulski]

> >                      .------- RJ45
> > MVPP2 ----- 88x3310 PHY
> >                      `------- SFP+
> 
> And the "MVPP2" part can be expended to:
> 
>         .-- GoP #0 --.
> MVPP2 ----- GoP #1 ---- Comphy lane #X -- 88x3310
>         `-- GoP #2 --'
> 
> Thanks!
> Antoine

One more point, Alaska 3310 PHY SoC and Marvell Embedded Processor SoC(A8K) are different SoC's.
Comphy driver configures Serdes IP in Marvell Embedded Processor SoC and media(SFP or RJ45) autodetect is done in Alaska 3310 PHY SoC(mostly by firmware).

Regards,
Stefan

RE: [EXT] Re: [PATCH net-next 09/13] net: mvpp2: dynamic reconfiguration of the PHY mode

[Stefan Chulski]

> >                      .------- RJ45
> > MVPP2 ----- 88x3310 PHY
> >                      `------- SFP+
> 
> And the "MVPP2" part can be expended to:
> 
>         .-- GoP #0 --.
> MVPP2 ----- GoP #1 ---- Comphy lane #X -- 88x3310
>         `-- GoP #2 --'
> 
> Thanks!
> Antoine

One more point, Alaska 3310 PHY SoC and Marvell Embedded Processor SoC(A8K) are different SoC's.
Comphy driver configures Serdes IP in Marvell Embedded Processor SoC and media(SFP or RJ45) autodetect is done in Alaska 3310 PHY SoC(mostly by firmware).

Regards,
Stefan

RE: [EXT] Re: [PATCH net-next 09/13] net: mvpp2: dynamic reconfiguration of the PHY mode

[Stefan Chulski]

> > Imagine phylib is using the copper Ethernet PHY, but the MAC is using
> > the SFP port. Somebody pulls out the copper cable, phylib says the
> > link is down, turns the carrier off and calls the callback. Not good,
> > since your SFP cable is still plugged in...  Ethtool is
> > returning/setting stuff in the Copper Ethernet PHY, when in fact you
> > intend to be setting SFP settings.
> 
> I see what could be the issue but I do not understand one aspect though:
> how could we switch from one PHY to another, as there's only one output
> between the SoC (and so a given GoP#) and the board. So if a given PHY can
> handle multiple modes I see, but in the other case a muxing somewhere would
> be needed? Or did I miss something?

I think PHY name and PHY mode struct that describe here both MAC to PHY and PHY to PHY connection create confusion...
Serdes IP lane doesn't care if connector is SFP, RJ45 or direct attached cable. mvpp22_comphy_init only configures MAC to PHY
connection. SFI for 10G(KR in mainline), SGMII for 1G and HS_SGMII for 2.5G.

Regards,
Stefan.

RE: [EXT] Re: [PATCH net-next 09/13] net: mvpp2: dynamic reconfiguration of the PHY mode

[Stefan Chulski]

> > Imagine phylib is using the copper Ethernet PHY, but the MAC is using
> > the SFP port. Somebody pulls out the copper cable, phylib says the
> > link is down, turns the carrier off and calls the callback. Not good,
> > since your SFP cable is still plugged in...  Ethtool is
> > returning/setting stuff in the Copper Ethernet PHY, when in fact you
> > intend to be setting SFP settings.
> 
> I see what could be the issue but I do not understand one aspect though:
> how could we switch from one PHY to another, as there's only one output
> between the SoC (and so a given GoP#) and the board. So if a given PHY can
> handle multiple modes I see, but in the other case a muxing somewhere would
> be needed? Or did I miss something?

I think PHY name and PHY mode struct that describe here both MAC to PHY and PHY to PHY connection create confusion...
Serdes IP lane doesn't care if connector is SFP, RJ45 or direct attached cable. mvpp22_comphy_init only configures MAC to PHY
connection. SFI for 10G(KR in mainline), SGMII for 1G and HS_SGMII for 2.5G.

Regards,
Stefan.

Re: [EXT] Re: [PATCH net-next 09/13] net: mvpp2: dynamic reconfiguration of the PHY mode

[Antoine Tenart]

[-- Attachment #1: Type: text/plain, Size: 1452 bytes --]

On Thu, Aug 24, 2017 at 05:08:29PM +0000, Stefan Chulski wrote:
> > > Imagine phylib is using the copper Ethernet PHY, but the MAC is using
> > > the SFP port. Somebody pulls out the copper cable, phylib says the
> > > link is down, turns the carrier off and calls the callback. Not good,
> > > since your SFP cable is still plugged in...  Ethtool is
> > > returning/setting stuff in the Copper Ethernet PHY, when in fact you
> > > intend to be setting SFP settings.
> > 
> > I see what could be the issue but I do not understand one aspect though:
> > how could we switch from one PHY to another, as there's only one output
> > between the SoC (and so a given GoP#) and the board. So if a given PHY can
> > handle multiple modes I see, but in the other case a muxing somewhere would
> > be needed? Or did I miss something?
> 
> I think PHY name and PHY mode struct that describe here both MAC to
> PHY and PHY to PHY connection create confusion...  Serdes IP lane
> doesn't care if connector is SFP, RJ45 or direct attached cable.
> mvpp22_comphy_init only configures MAC to PHY
> connection. SFI for 10G(KR in mainline), SGMII for 1G and HS_SGMII for
> 2.5G.

So maybe one confusion was to name them PHY_MODE_10GKR and
PHY_MODE_SGMII. It could be PHY_MODE_10G and PHY_MODE_1G instead.

Does that sound right?

Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [EXT] Re: [PATCH net-next 09/13] net: mvpp2: dynamic reconfiguration of the PHY mode

[Antoine Tenart]

[-- Attachment #1: Type: text/plain, Size: 1452 bytes --]

On Thu, Aug 24, 2017 at 05:08:29PM +0000, Stefan Chulski wrote:
> > > Imagine phylib is using the copper Ethernet PHY, but the MAC is using
> > > the SFP port. Somebody pulls out the copper cable, phylib says the
> > > link is down, turns the carrier off and calls the callback. Not good,
> > > since your SFP cable is still plugged in...  Ethtool is
> > > returning/setting stuff in the Copper Ethernet PHY, when in fact you
> > > intend to be setting SFP settings.
> > 
> > I see what could be the issue but I do not understand one aspect though:
> > how could we switch from one PHY to another, as there's only one output
> > between the SoC (and so a given GoP#) and the board. So if a given PHY can
> > handle multiple modes I see, but in the other case a muxing somewhere would
> > be needed? Or did I miss something?
> 
> I think PHY name and PHY mode struct that describe here both MAC to
> PHY and PHY to PHY connection create confusion...  Serdes IP lane
> doesn't care if connector is SFP, RJ45 or direct attached cable.
> mvpp22_comphy_init only configures MAC to PHY
> connection. SFI for 10G(KR in mainline), SGMII for 1G and HS_SGMII for
> 2.5G.

So maybe one confusion was to name them PHY_MODE_10GKR and
PHY_MODE_SGMII. It could be PHY_MODE_10G and PHY_MODE_1G instead.

Does that sound right?

Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

RE: [EXT] Re: [PATCH net-next 09/13] net: mvpp2: dynamic reconfiguration of the PHY mode

[Stefan Chulski]

> So maybe one confusion was to name them PHY_MODE_10GKR and
> PHY_MODE_SGMII. It could be PHY_MODE_10G and PHY_MODE_1G instead.

1G can be RGMII...

Regards,
Stefan.

RE: [EXT] Re: [PATCH net-next 09/13] net: mvpp2: dynamic reconfiguration of the PHY mode

[Stefan Chulski]

> So maybe one confusion was to name them PHY_MODE_10GKR and
> PHY_MODE_SGMII. It could be PHY_MODE_10G and PHY_MODE_1G instead.

1G can be RGMII...

Regards,
Stefan.

Re: [EXT] Re: [PATCH net-next 09/13] net: mvpp2: dynamic reconfiguration of the PHY mode

[Russell King - ARM Linux]

On Thu, Aug 24, 2017 at 07:14:18PM +0200, Antoine Tenart wrote:
> On Thu, Aug 24, 2017 at 05:08:29PM +0000, Stefan Chulski wrote:
> > > > Imagine phylib is using the copper Ethernet PHY, but the MAC is using
> > > > the SFP port. Somebody pulls out the copper cable, phylib says the
> > > > link is down, turns the carrier off and calls the callback. Not good,
> > > > since your SFP cable is still plugged in...  Ethtool is
> > > > returning/setting stuff in the Copper Ethernet PHY, when in fact you
> > > > intend to be setting SFP settings.
> > > 
> > > I see what could be the issue but I do not understand one aspect though:
> > > how could we switch from one PHY to another, as there's only one output
> > > between the SoC (and so a given GoP#) and the board. So if a given PHY can
> > > handle multiple modes I see, but in the other case a muxing somewhere would
> > > be needed? Or did I miss something?
> > 
> > I think PHY name and PHY mode struct that describe here both MAC to
> > PHY and PHY to PHY connection create confusion...  Serdes IP lane
> > doesn't care if connector is SFP, RJ45 or direct attached cable.
> > mvpp22_comphy_init only configures MAC to PHY
> > connection. SFI for 10G(KR in mainline), SGMII for 1G and HS_SGMII for
> > 2.5G.
> 
> So maybe one confusion was to name them PHY_MODE_10GKR and
> PHY_MODE_SGMII. It could be PHY_MODE_10G and PHY_MODE_1G instead.

SGMII mode supports 100M and 10M as well using data repetition, so 1G
makes it look like those speeds are not supported.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

Re: [EXT] Re: [PATCH net-next 09/13] net: mvpp2: dynamic reconfiguration of the PHY mode

[Russell King - ARM Linux]

On Thu, Aug 24, 2017 at 07:14:18PM +0200, Antoine Tenart wrote:
> On Thu, Aug 24, 2017 at 05:08:29PM +0000, Stefan Chulski wrote:
> > > > Imagine phylib is using the copper Ethernet PHY, but the MAC is using
> > > > the SFP port. Somebody pulls out the copper cable, phylib says the
> > > > link is down, turns the carrier off and calls the callback. Not good,
> > > > since your SFP cable is still plugged in...  Ethtool is
> > > > returning/setting stuff in the Copper Ethernet PHY, when in fact you
> > > > intend to be setting SFP settings.
> > > 
> > > I see what could be the issue but I do not understand one aspect though:
> > > how could we switch from one PHY to another, as there's only one output
> > > between the SoC (and so a given GoP#) and the board. So if a given PHY can
> > > handle multiple modes I see, but in the other case a muxing somewhere would
> > > be needed? Or did I miss something?
> > 
> > I think PHY name and PHY mode struct that describe here both MAC to
> > PHY and PHY to PHY connection create confusion...  Serdes IP lane
> > doesn't care if connector is SFP, RJ45 or direct attached cable.
> > mvpp22_comphy_init only configures MAC to PHY
> > connection. SFI for 10G(KR in mainline), SGMII for 1G and HS_SGMII for
> > 2.5G.
> 
> So maybe one confusion was to name them PHY_MODE_10GKR and
> PHY_MODE_SGMII. It could be PHY_MODE_10G and PHY_MODE_1G instead.

SGMII mode supports 100M and 10M as well using data repetition, so 1G
makes it look like those speeds are not supported.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

Re: [PATCH net-next 09/13] net: mvpp2: dynamic reconfiguration of the PHY mode

[Russell King - ARM Linux]

On Thu, Aug 24, 2017 at 04:56:09PM +0200, Andrew Lunn wrote:
> On Thu, Aug 24, 2017 at 10:38:19AM +0200, Antoine Tenart wrote:
> > This patch adds logic to reconfigure the comphy/gop when the link status
> > change at runtime. This is very useful on boards such as the mcbin which
> > have SFP and Ethernet ports connected to the same MAC port: depending on
> > what the user connects the driver will automatically reconfigure the
> > link mode.
> 
> Hi Antoine
> 
> I would expect each of these external Ethernet ports to have its own
> Ethernet PHY. Don't you need to disconnect from one Ethernet phy and
> connect to the other Ethernet PHY when you change external Ethernet
> port?

I think you're all getting confused.  The link mode has very little to
do with whether you're using SFP+ or whether you're using the RJ45 at
10G speeds.  The link mode has everything to do with the speed at which
the link is negotiated at.

So please, put SFP+ out of your minds for this - SFP+ isn't the reason
why you need to switch the MAC link mode.

In all cases, the mvpp2 to 88x3310 link ends up in one of two modes:
1. SGMII for RJ45 speeds less than 10G.  Autonegotiation on SGMII
    at the mvpp2 end *must* be enabled for the PHY to work.

2. 10Gbase-R for 10G speeds, whether that be for SFP+ or RJ45 at 10G.
   Note: mcbin does not support SFP (1G) modules on the SFP+ ports.

The 88x3310 driver in the kernel knows about these combinations and
sets the phy interface parameter correctly depending on whether the
PHY has configured itself for copper at whatever speed or SFP+.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

Re: [PATCH net-next 09/13] net: mvpp2: dynamic reconfiguration of the PHY mode

[Andrew Lunn]

Hi Russell

> I think you're all getting confused.

Yes, i was at least.

> The 88x3310 driver in the kernel knows about these combinations and
> sets the phy interface parameter correctly depending on whether the
> PHY has configured itself for copper at whatever speed or SFP+.

So when the PHY decides to swap from copper to fibre etc, is the
phylib state machine kept up to date. Does it see a down, followed by
an up?

   Andrew

Re: [PATCH net-next 09/13] net: mvpp2: dynamic reconfiguration of the PHY mode

[Russell King - ARM Linux]

On Thu, Aug 24, 2017 at 07:45:19PM +0200, Andrew Lunn wrote:
> > The 88x3310 driver in the kernel knows about these combinations and
> > sets the phy interface parameter correctly depending on whether the
> > PHY has configured itself for copper at whatever speed or SFP+.
> 
> So when the PHY decides to swap from copper to fibre etc, is the
> phylib state machine kept up to date. Does it see a down, followed by
> an up?

I'd have to re-check to make sure, but I believe it does, because the
negotiation is held off on the "other" media until the currently active
link has gone down.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

[PATCH net-next 10/13] arm64: dts: marvell: extend the cp110 syscon register area length

[Antoine Tenart]

This patch extends on both cp110 the system register area length to
include some of the comphy registers as well.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi | 2 +-
 arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi b/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi
index 18299e164cb7..9b2581473183 100644
--- a/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi
@@ -118,7 +118,7 @@
 
 			cpm_syscon0: system-controller@440000 {
 				compatible = "syscon", "simple-mfd";
-				reg = <0x440000 0x1000>;
+				reg = <0x440000 0x2000>;
 
 				cpm_clk: clock {
 					compatible = "marvell,cp110-clock";
diff --git a/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi b/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
index 5ae8fa575859..d3902f218c46 100644
--- a/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
@@ -125,7 +125,7 @@
 
 			cps_syscon0: system-controller@440000 {
 				compatible = "syscon", "simple-mfd";
-				reg = <0x440000 0x1000>;
+				reg = <0x440000 0x2000>;
 
 				cps_clk: clock {
 					compatible = "marvell,cp110-clock";
-- 
2.13.5

[PATCH net-next 11/13] arm64: dts: marvell: add comphy nodes on cp110 master and slave

[Antoine Tenart]

Now that the comphy driver is available, this patch adds the
corresponding nodes in the cp110 master and slave device trees.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 .../boot/dts/marvell/armada-cp110-master.dtsi      | 38 ++++++++++++++++++++++
 .../arm64/boot/dts/marvell/armada-cp110-slave.dtsi | 38 ++++++++++++++++++++++
 2 files changed, 76 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi b/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi
index 9b2581473183..f2a50552bad4 100644
--- a/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi
@@ -91,6 +91,44 @@
 				};
 			};
 
+			cpm_comphy: phy@120000 {
+				compatible = "marvell,comphy-cp110";
+				reg = <0x120000 0x6000>;
+				marvell,system-controller = <&cpm_syscon0>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				cpm_comphy0: phy@0 {
+					reg = <0>;
+					#phy-cells = <1>;
+				};
+
+				cpm_comphy1: phy@1 {
+					reg = <1>;
+					#phy-cells = <1>;
+				};
+
+				cpm_comphy2: phy@2 {
+					reg = <2>;
+					#phy-cells = <1>;
+				};
+
+				cpm_comphy3: phy@3 {
+					reg = <3>;
+					#phy-cells = <1>;
+				};
+
+				cpm_comphy4: phy@4 {
+					reg = <4>;
+					#phy-cells = <1>;
+				};
+
+				cpm_comphy5: phy@5 {
+					reg = <5>;
+					#phy-cells = <1>;
+				};
+			};
+
 			cpm_mdio: mdio@12a200 {
 				#address-cells = <1>;
 				#size-cells = <0>;
diff --git a/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi b/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
index d3902f218c46..bd7f7d0e6de9 100644
--- a/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
@@ -98,6 +98,44 @@
 				};
 			};
 
+			cps_comphy: phy@120000 {
+				compatible = "marvell,comphy-cp110";
+				reg = <0x120000 0x6000>;
+				marvell,system-controller = <&cps_syscon0>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				cps_comphy0: phy@0 {
+					reg = <0>;
+					#phy-cells = <1>;
+				};
+
+				cps_comphy1: phy@1 {
+					reg = <1>;
+					#phy-cells = <1>;
+				};
+
+				cps_comphy2: phy@2 {
+					reg = <2>;
+					#phy-cells = <1>;
+				};
+
+				cps_comphy3: phy@3 {
+					reg = <3>;
+					#phy-cells = <1>;
+				};
+
+				cps_comphy4: phy@4 {
+					reg = <4>;
+					#phy-cells = <1>;
+				};
+
+				cps_comphy5: phy@5 {
+					reg = <5>;
+					#phy-cells = <1>;
+				};
+			};
+
 			cps_mdio: mdio@12a200 {
 				#address-cells = <1>;
 				#size-cells = <0>;
-- 
2.13.5

[PATCH net-next 12/13] arm64: dts: marvell: mcbin: add comphy references to Ethernet ports

[Antoine Tenart]

This patch adds comphy phandles to the Ethernet ports in the mcbin
device tree. The comphy is used to configure the serdes PHYs used by
these ports.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts b/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts
index 6cb4b000e1ac..dc4b6f3f25f6 100644
--- a/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts
+++ b/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts
@@ -148,6 +148,7 @@
 &cpm_eth0 {
 	status = "okay";
 	phy = <&phy0>;
+	phys = <&cpm_comphy4 0>;
 	phy-mode = "10gbase-kr";
 };
 
@@ -181,6 +182,7 @@
 &cps_eth0 {
 	status = "okay";
 	phy = <&phy1>;
+	phys = <&cps_comphy4 0>;
 	phy-mode = "10gbase-kr";
 };
 
@@ -189,6 +191,7 @@
 	status = "okay";
 	phy = <&ge_phy>;
 	phy-mode = "sgmii";
+	phys = <&cps_comphy0 1>;
 };
 
 &cps_sata0 {
-- 
2.13.5

Re: [PATCH net-next 12/13] arm64: dts: marvell: mcbin: add comphy references to Ethernet ports

[Andrew Lunn]

> @@ -189,6 +191,7 @@
>  	status = "okay";
>  	phy = <&ge_phy>;
>  	phy-mode = "sgmii";
> +	phys = <&cps_comphy0 1>;

Does the binding document describe the meaning of the specifier?

     Andrew

Re: [PATCH net-next 12/13] arm64: dts: marvell: mcbin: add comphy references to Ethernet ports

[Antoine Tenart]

[-- Attachment #1: Type: text/plain, Size: 576 bytes --]

Hi Andrew,

On Thu, Aug 24, 2017 at 03:58:13PM +0200, Andrew Lunn wrote:
> > @@ -189,6 +191,7 @@
> >  	status = "okay";
> >  	phy = <&ge_phy>;
> >  	phy-mode = "sgmii";
> > +	phys = <&cps_comphy0 1>;
> 
> Does the binding document describe the meaning of the specifier?

Ahhh no you're right! It's the port number i.e. there are multiple
inputs, each of which can support different modes. So say the input is
GoP#0, it can support 10G and SGMII.

Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH net-next 13/13] arm64: defconfig: enable Marvell CP110 comphy

[Antoine Tenart]

From: Miquel Raynal <miquel.raynal@free-electrons.com>

The comphy is an hardware block giving access to common PHYs that can be
used by various other engines (Network, SATA, ...). This is used on
Marvell 7k/8k platforms for now. Enable the corresponding driver.

Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com>
Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index cdde4f56a281..e671e37b30af 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -509,6 +509,7 @@ CONFIG_PWM_TEGRA=m
 CONFIG_PHY_RCAR_GEN3_USB2=y
 CONFIG_PHY_HI6220_USB=y
 CONFIG_PHY_SUN4I_USB=y
+CONFIG_PHY_MVEBU_CP110_COMPHY=y
 CONFIG_PHY_ROCKCHIP_INNO_USB2=y
 CONFIG_PHY_ROCKCHIP_EMMC=y
 CONFIG_PHY_ROCKCHIP_PCIE=m
-- 
2.13.5