[RFC PATCH 00/11] net: ethernet: ti: cpsw: replace cpsw-phy-sel with phy driver

[RFC PATCH 00/11] net: ethernet: ti: cpsw: replace cpsw-phy-sel with phy driver

[Grygorii Strashko]

TI am335x/am437x/dra7(am5)/dm814x CPSW3G Ethernet Subsystem supports 
two 10/100/1000 Ethernet ports with selectable G/MII, RMII, and RGMII interfaces.
The interface mode is selected by configuring the MII mode selection register(s)
(GMII_SEL) in the System Control Module chapter (SCM).
                                               +--------------+
        +-------------------------------+      |SCM           |
        |                     CPSW      |      |  +---------+ |
        |        +--------------------------------+gmii_sel | |
        |        |                      |      |  +---------+ |
        |   +----v---+     +--------+   |      +--------------+
        |   |Port 1..<--+-->GMII/MII<------->
        |   |        |  |  |        |   |
        |   +--------+  |  +--------+   |
        |               |               |
        |               |  +--------+   |
        |               |  | RMII   <------->
        |               +-->        |   |
        |               |  +--------+   |
        |               |               |
        |               |  +--------+   |
        |               |  | RGMII  <------->
        |               +-->        |   |
        |                  +--------+   |
        +-------------------------------+

GMII_SEL register(s) and bit fields placement in SCM are different between SoCs
while fields meaning is the same. GMII_SEL(s) allows to select -
Port GMII/MII/RMII/RGMII Mode; RGMII Internal Delay Mode (SoC dependant) and
RMII Reference Clock Output mode (SoC dependant).

Historically CPSW external Port's interface mode selection configuartion was
introduced using custom driver and API cpsw-phy-sel.c.
This leads to unnecessary driver, DT binding and custom API support effort.
Moreover, even definition of cpsw-phy-sel node in DTs is logically incorrect [1]

mac: ethernet@4a100000 {
	compatible = "ti,am4372-cpsw","ti,cpsw";
	...

	phy_sel: cpsw-phy-sel@44e10650 {
		compatible = "ti,am43xx-cpsw-phy-sel";
		reg= <0x44e10650 0x4>;
		reg-names = "gmii-sel";
	};
};

This series introduces attempt to drop custom CPSW Port interface selection
implementation (cpsw-phy-sel.c) and use well defined Linux PHY framework instead.
it introduces CPSW Port's PHY Interface Mode selection Driver (phy-gmii-sel)
which implements standard Linux PHY interface. The phy-gmii-sel PHY device
should defined as child device of SCM node (scm_conf) and can be attached to
each CPSW port node using standard PHY bindings (cell 1 - port number,
cell 2 - RMII refclk mode).

scm_conf: scm_conf@0 {
	compatible = "syscon", "simple-bus";

	gmii_sel_phy: cpsw-sel-netif {
		compatible = "ti,am43xx-gmii-sel-phy";
		syscon-scm = <&scm_conf>;
		#phy-cells = <2>;
	};
};

mac: ethernet@4a100000 {
	compatible = "ti,am4372-cpsw","ti,cpsw";

	cpsw_emac0: slave@4a100200 {
		phy-mode = "rgmii";
		phys = <&gmii_sel_phy 1 0>;
	};
};

The CPSW driver requests phy-gmii-sel PHY for each external port and uses newly
introduced API phy_set_netif_mode() (Patch 1) for port interface mode selection
when netdev is opened.

	slave->data->gmii_sel_phy = devm_of_phy_get(&pdev->dev, port_node, NULL);
	slave->data->phy_if = of_get_phy_mode(port_node);

cpsw_ndo_open()
	phy_set_netif_mode(slave->data->gmii_sel_phy, slave->data->phy_if);

Note. CPSW Port interface has to be reconfigured every time netdev is opened for
proper System Suspend support where CPSW can lose context.

I've considered two options while working on this series:
1) extend enum phy_mode {} and introduce more enum elements to cover missing,
required Network PHY's Interface Mode definitions, like MII/GMII/RGMII(-XID),
but it'd mean copy-past and data duplication from phy_interface_t -> phy_mode.
More over, phy_interface_t can still continue growing.

2) introduce new PHY API for network interface mode selection which will use
already defined set of modes from phy_interface_t.

Option 2 was selected for this series.

[1] https://www.mail-archive.com/netdev@vger.kernel.org/msg247135.html

Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Tony Lindgren <tony@atomide.com>

Grygorii Strashko (11):
  phy: core add phy_set_netif_mode() api
  dt-bindings: phy: add cpsw port interface mode selection phy bindings
  phy: ti: introduce phy-gmii-sel driver
  dt-bindings: net: ti: cpsw: switch to use phy-gmii-sel phy
  net: ethernet: ti: cpsw: add support for port interface mode selection
    phy
  ARM: dts: dra7: switch to use phy-gmii-sel
  ARM: dts: dm814x: switch to use phy-gmii-sel
  ARM: dts: am4372: switch to use phy-gmii-sel
  ARM: dts: am335x: switch to use phy-gmii-sel
  dt-bindings: net: ti: deprecate cpsw-phy-sel bindings
  net: ethernet: ti: cpsw: deprecate cpsw-phy-sel driver

 .../devicetree/bindings/net/cpsw-phy-sel.txt       |   2 +-
 Documentation/devicetree/bindings/net/cpsw.txt     |   8 +-
 .../devicetree/bindings/phy/ti-phy-gmii-sel.txt    |  68 ++++
 arch/arm/boot/dts/am335x-baltos-ir2110.dts         |   4 -
 arch/arm/boot/dts/am335x-baltos-ir3220.dts         |   3 -
 arch/arm/boot/dts/am335x-baltos-ir5221.dts         |   3 -
 arch/arm/boot/dts/am335x-chiliboard.dts            |   3 -
 arch/arm/boot/dts/am335x-icev2.dts                 |   4 -
 arch/arm/boot/dts/am335x-igep0033.dtsi             |   3 -
 arch/arm/boot/dts/am335x-lxm.dts                   |   3 -
 arch/arm/boot/dts/am335x-moxa-uc-8100-me-t.dts     |   5 -
 arch/arm/boot/dts/am335x-phycore-som.dtsi          |   3 -
 arch/arm/boot/dts/am33xx.dtsi                      |  14 +-
 arch/arm/boot/dts/am4372.dtsi                      |  16 +-
 arch/arm/boot/dts/am43x-epos-evm.dts               |   5 +-
 arch/arm/boot/dts/dm814x.dtsi                      |  15 +-
 arch/arm/boot/dts/dra7.dtsi                        |  14 +-
 drivers/net/ethernet/ti/Kconfig                    |   6 +-
 drivers/net/ethernet/ti/cpsw.c                     |  18 +-
 drivers/net/ethernet/ti/cpsw.h                     |   6 +
 drivers/phy/phy-core.c                             |  15 +
 drivers/phy/ti/Kconfig                             |  10 +
 drivers/phy/ti/Makefile                            |   1 +
 drivers/phy/ti/phy-gmii-sel.c                      | 345 +++++++++++++++++++++
 include/linux/phy/phy.h                            |  12 +
 25 files changed, 520 insertions(+), 66 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/phy/ti-phy-gmii-sel.txt
 create mode 100644 drivers/phy/ti/phy-gmii-sel.c

-- 
2.10.5

[RFC PATCH 01/11] phy: core add phy_set_netif_mode() api

[Grygorii Strashko]

Add new API phy_set_netif_mode(struct phy *phy, phy_interface_t mode) and
new PHY operation callback .set_netif_mode() which intended to be implemnte
by PHY drivers which supports Network interrfaces mode selection. Both
accepts phy_interface_t vlaue as input parameter.

Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/phy/phy-core.c  | 15 +++++++++++++++
 include/linux/phy/phy.h | 12 ++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 35fd38c..d9aba1a 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -377,6 +377,21 @@ int phy_set_mode(struct phy *phy, enum phy_mode mode)
 }
 EXPORT_SYMBOL_GPL(phy_set_mode);
 
+int phy_set_netif_mode(struct phy *phy, phy_interface_t mode)
+{
+	int ret;
+
+	if (!phy || !phy->ops->set_netif_mode)
+		return 0;
+
+	mutex_lock(&phy->mutex);
+	ret = phy->ops->set_netif_mode(phy, mode);
+	mutex_unlock(&phy->mutex);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(phy_set_netif_mode);
+
 int phy_reset(struct phy *phy)
 {
 	int ret;
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 9713aeb..bc73d2b 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -17,6 +17,7 @@
 #include <linux/err.h>
 #include <linux/of.h>
 #include <linux/device.h>
+#include <linux/phy.h>
 #include <linux/pm_runtime.h>
 #include <linux/regulator/consumer.h>
 
@@ -49,6 +50,7 @@ enum phy_mode {
  * @power_on: powering on the phy
  * @power_off: powering off the phy
  * @set_mode: set the mode of the phy
+ * @set_netif_mode: set the mode of the net interface phy
  * @reset: resetting the phy
  * @calibrate: calibrate the phy
  * @owner: the module owner containing the ops
@@ -59,6 +61,7 @@ struct phy_ops {
 	int	(*power_on)(struct phy *phy);
 	int	(*power_off)(struct phy *phy);
 	int	(*set_mode)(struct phy *phy, enum phy_mode mode);
+	int	(*set_netif_mode)(struct phy *phy, phy_interface_t mode);
 	int	(*reset)(struct phy *phy);
 	int	(*calibrate)(struct phy *phy);
 	struct module *owner;
@@ -163,6 +166,7 @@ int phy_exit(struct phy *phy);
 int phy_power_on(struct phy *phy);
 int phy_power_off(struct phy *phy);
 int phy_set_mode(struct phy *phy, enum phy_mode mode);
+int phy_set_netif_mode(struct phy *phy, phy_interface_t mode);
 static inline enum phy_mode phy_get_mode(struct phy *phy)
 {
 	return phy->attrs.mode;
@@ -283,6 +287,14 @@ static inline int phy_set_mode(struct phy *phy, enum phy_mode mode)
 	return -ENOSYS;
 }
 
+static inline int phy_set_netif_mode(struct phy *phy,
+				     phy_interface_t mode)
+{
+	if (!phy)
+		return 0;
+	return -ENOTSUPP;
+}
+
 static inline enum phy_mode phy_get_mode(struct phy *phy)
 {
 	return PHY_MODE_INVALID;
-- 
2.10.5

[RFC PATCH 02/11] dt-bindings: phy: add cpsw port interface mode selection phy bindings

[Grygorii Strashko]

Add CPSW Port's Interface Mode Selection PHY (phy-gmii-sel) DT Bindings

Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 .../devicetree/bindings/phy/ti-phy-gmii-sel.txt    | 68 ++++++++++++++++++++++
 1 file changed, 68 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/ti-phy-gmii-sel.txt

diff --git a/Documentation/devicetree/bindings/phy/ti-phy-gmii-sel.txt b/Documentation/devicetree/bindings/phy/ti-phy-gmii-sel.txt
new file mode 100644
index 0000000..fd81421
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/ti-phy-gmii-sel.txt
@@ -0,0 +1,68 @@
+CPSW Port's Interface Mode Selection PHY Tree Bindings
+-----------------------------------------------
+
+TI am335x/am437x/dra7(am5)/dm814x CPSW3G Ethernet Subsystem supports
+two 10/100/1000 Ethernet ports with selectable G/MII, RMII, and RGMII interfaces.
+The interface mode is selected by configuring the MII mode selection register(s)
+(GMII_SEL) in the System Control Module chapter (SCM). GMII_SEL register(s) and
+bit fields placement in SCM are different between SoCs while fields meaning
+is the same.
+                                               +--------------+
+        +-------------------------------+      |SCM           |
+        |                     CPSW      |      |  +---------+ |
+        |        +--------------------------------+gmii_sel | |
+        |        |                      |      |  +---------+ |
+        |   +----v---+     +--------+   |      +--------------+
+        |   |Port 1..<--+-->GMII/MII<------->
+        |   |        |  |  |        |   |
+        |   +--------+  |  +--------+   |
+        |               |               |
+        |               |  +--------+   |
+        |               |  | RMII   <------->
+        |               +-->        |   |
+        |               |  +--------+   |
+        |               |               |
+        |               |  +--------+   |
+        |               |  | RGMII  <------->
+        |               +-->        |   |
+        |                  +--------+   |
+        +-------------------------------+
+
+CPSW Port's Interface Mode Selection PHY describes MII interface mode between
+CPSW Port and Ethernet PHY which depends on Eth PHY and board configuration.
+
+CPSW Port's Interface Mode Selection PHY device should defined as child device
+of SCM node (scm_conf) and can be attached to each CPSW port node using standard
+PHY bindings (See phy/phy-bindings.txt).
+
+Required properties:
+- compatible		: Should be "ti,am3352-phy-gmii-sel" for am335x platform
+			  "ti,dra7xx-phy-gmii-sel" for dra7xx/am57xx platform
+			  "ti,am43xx-phy-gmii-sel" for am43xx platform
+			  "ti,dm814-phy-gmii-sel" for dm814x platform
+- #phy-cells		: must be 2.
+			  cell 1 - CPSW port number (starting from 1)
+			  cell 2 - RMII refclk mode
+- syscon-scm		: phandle on SCM node (mfd/syscon.txt)
+
+Examples:
+	phy_gmii_sel: phy-gmii-sel {
+		compatible = "ti,am3352-phy-gmii-sel";
+		syscon-scm = <&scm_conf>;
+		#phy-cells = <2>;
+	};
+
+	mac: ethernet@4a100000 {
+		compatible = "ti,am335x-cpsw","ti,cpsw";
+		...
+
+		cpsw_emac0: slave@4a100200 {
+			...
+			phys = <&phy_gmii_sel 1 1>;
+		};
+
+		cpsw_emac1: slave@4a100300 {
+			...
+			phys = <&phy_gmii_sel 2 1>;
+		};
+	};
-- 
2.10.5

[RFC PATCH 03/11] phy: ti: introduce phy-gmii-sel driver

[Grygorii Strashko]

TI am335x/am437x/dra7(am5)/dm814x CPSW3G Ethernet Subsystem supports two
10/100/1000 Ethernet ports with selectable G/MII, RMII, and RGMII
interfaces. The interface mode is selected by configuring the MII mode
selection register(s) (GMII_SEL) in the System Control Module chapter
(SCM). GMII_SEL register(s) and bit fields placement in SCM are different
between SoCs while fields meaning is the same.

Historically CPSW external Port's interface mode selection configuartion
was introduced using custom API and driver cpsw-phy-sel.c. This leads to
unnecessary driver, DT binding and custom API support effort.

This patch introduces CPSW Port's PHY Interface Mode selection Driver
(phy-gmii-sel) which implements standard Linux PHY interface and proposed
as a replacement for TI's specific driver cpsw-phy-sel.c and coresponding
custom API.

Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/phy/ti/Kconfig        |  10 ++
 drivers/phy/ti/Makefile       |   1 +
 drivers/phy/ti/phy-gmii-sel.c | 345 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 356 insertions(+)
 create mode 100644 drivers/phy/ti/phy-gmii-sel.c

diff --git a/drivers/phy/ti/Kconfig b/drivers/phy/ti/Kconfig
index 2050356..f137e01 100644
--- a/drivers/phy/ti/Kconfig
+++ b/drivers/phy/ti/Kconfig
@@ -76,3 +76,13 @@ config TWL4030_USB
 	  family chips (including the TWL5030 and TPS659x0 devices).
 	  This transceiver supports high and full speed devices plus,
 	  in host mode, low speed.
+
+config PHY_TI_GMII_SEL
+	tristate
+	default y if TI_CPSW=y
+	depends on TI_CPSW || COMPILE_TEST
+	select GENERIC_PHY
+	default m
+	help
+	  This driver supports configuring of the TI CPSW Port mode depending on
+	  the Ethernet PHY connected to the CPSW Port.
diff --git a/drivers/phy/ti/Makefile b/drivers/phy/ti/Makefile
index 9f36175..bea8f25 100644
--- a/drivers/phy/ti/Makefile
+++ b/drivers/phy/ti/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_OMAP_USB2)			+= phy-omap-usb2.o
 obj-$(CONFIG_TI_PIPE3)			+= phy-ti-pipe3.o
 obj-$(CONFIG_PHY_TUSB1210)		+= phy-tusb1210.o
 obj-$(CONFIG_TWL4030_USB)		+= phy-twl4030-usb.o
+obj-$(CONFIG_PHY_TI_GMII_SEL)		+= phy-gmii-sel.o
diff --git a/drivers/phy/ti/phy-gmii-sel.c b/drivers/phy/ti/phy-gmii-sel.c
new file mode 100644
index 0000000..61a3aa1
--- /dev/null
+++ b/drivers/phy/ti/phy-gmii-sel.c
@@ -0,0 +1,345 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Texas Instruments CPSW Port's PHY Interface Mode selection Driver
+ *
+ * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/
+ *
+ * Based on cpsw-phy-sel.c driver created by Mugunthan V N <mugunthanvnm@ti.com>
+ */
+
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of.h>
+#include <linux/of_net.h>
+#include <linux/phy.h>
+#include <linux/phy/phy.h>
+#include <linux/regmap.h>
+
+/* AM33xx SoC specific definitions for the CONTROL port */
+#define AM33XX_GMII_SEL_MODE_MII	0
+#define AM33XX_GMII_SEL_MODE_RMII	1
+#define AM33XX_GMII_SEL_MODE_RGMII	2
+
+enum {
+	PHY_GMII_SEL_PORT_MODE,
+	PHY_GMII_SEL_RGMII_ID_MODE,
+	PHY_GMII_SEL_RMII_IO_CLK_EN,
+	PHY_GMII_SEL_LAST,
+};
+
+struct phy_gmii_sel_phy_priv {
+	struct phy_gmii_sel_priv *priv;
+	u32		id;
+	struct phy	*if_phy;
+	int		rmii_clock_external;
+	int		phy_if_mode;
+	struct regmap_field *fields[PHY_GMII_SEL_LAST];
+};
+
+struct phy_gmii_sel_soc_data {
+	u32 num_ports;
+	u32 features;
+	const struct reg_field (*regfields)[PHY_GMII_SEL_LAST];
+};
+
+struct phy_gmii_sel_priv {
+	struct device *dev;
+	const struct phy_gmii_sel_soc_data *soc_data;
+	struct regmap *regmap;
+	struct phy_provider *phy_provider;
+
+	struct phy_gmii_sel_phy_priv *if_phys;
+};
+
+static int phy_gmii_sel_mode(struct phy *phy, phy_interface_t intf_mode)
+{
+	struct phy_gmii_sel_phy_priv *if_phy = phy_get_drvdata(phy);
+	const struct phy_gmii_sel_soc_data *soc_data = if_phy->priv->soc_data;
+	struct device *dev = if_phy->priv->dev;
+	struct regmap_field *regfield;
+	int ret, rgmii_id = 0;
+	u32 mode = 0;
+
+	if_phy->phy_if_mode = intf_mode;
+
+	switch (if_phy->phy_if_mode) {
+	case PHY_INTERFACE_MODE_RMII:
+		mode = AM33XX_GMII_SEL_MODE_RMII;
+		break;
+
+	case PHY_INTERFACE_MODE_RGMII:
+		mode = AM33XX_GMII_SEL_MODE_RGMII;
+		break;
+
+	case PHY_INTERFACE_MODE_RGMII_ID:
+	case PHY_INTERFACE_MODE_RGMII_RXID:
+	case PHY_INTERFACE_MODE_RGMII_TXID:
+		mode = AM33XX_GMII_SEL_MODE_RGMII;
+		rgmii_id = 1;
+		break;
+
+	default:
+		dev_warn(dev,
+			 "port%u: unsupported mode: \"%s\". Defaulting to MII.\n",
+			 if_phy->id, phy_modes(rgmii_id));
+		/* fall through */
+	case PHY_INTERFACE_MODE_MII:
+		mode = AM33XX_GMII_SEL_MODE_MII;
+		break;
+	};
+
+	dev_dbg(dev, "%s id:%u mode:%u rgmii_id:%d rmii_clk_ext:%d\n",
+		__func__, if_phy->id, mode, rgmii_id,
+		if_phy->rmii_clock_external);
+
+	regfield = if_phy->fields[PHY_GMII_SEL_PORT_MODE];
+	ret = regmap_field_write(regfield, mode);
+
+	if (soc_data->features & BIT(PHY_GMII_SEL_RGMII_ID_MODE) &&
+	    if_phy->fields[PHY_GMII_SEL_RGMII_ID_MODE]) {
+		regfield = if_phy->fields[PHY_GMII_SEL_RGMII_ID_MODE];
+		ret |= regmap_field_write(regfield, rgmii_id);
+	}
+
+	if (soc_data->features & BIT(PHY_GMII_SEL_RMII_IO_CLK_EN) &&
+	    if_phy->fields[PHY_GMII_SEL_RMII_IO_CLK_EN]) {
+		regfield = if_phy->fields[PHY_GMII_SEL_RMII_IO_CLK_EN];
+		ret |= regmap_field_write(regfield,
+					  if_phy->rmii_clock_external);
+	}
+
+	if (ret) {
+		dev_err(dev, "port%u: set mode fail %d", if_phy->id, ret);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static const
+struct reg_field phy_gmii_sel_fields_am33xx[][PHY_GMII_SEL_LAST] = {
+	{
+		[PHY_GMII_SEL_PORT_MODE] = REG_FIELD(0x650, 0, 1),
+		[PHY_GMII_SEL_RGMII_ID_MODE] = REG_FIELD(0x650, 4, 4),
+		[PHY_GMII_SEL_RMII_IO_CLK_EN] = REG_FIELD(0x650, 6, 6),
+	},
+	{
+		[PHY_GMII_SEL_PORT_MODE] = REG_FIELD(0x650, 2, 3),
+		[PHY_GMII_SEL_RGMII_ID_MODE] = REG_FIELD(0x650, 5, 5),
+		[PHY_GMII_SEL_RMII_IO_CLK_EN] = REG_FIELD(0x650, 7, 7),
+	},
+};
+
+static const
+struct phy_gmii_sel_soc_data phy_gmii_sel_soc_am33xx = {
+	.num_ports = 2,
+	.features = BIT(PHY_GMII_SEL_RGMII_ID_MODE) |
+		    BIT(PHY_GMII_SEL_RMII_IO_CLK_EN),
+	.regfields = phy_gmii_sel_fields_am33xx,
+};
+
+static const
+struct reg_field phy_gmii_sel_fields_dra7[][PHY_GMII_SEL_LAST] = {
+	{
+		[PHY_GMII_SEL_PORT_MODE] = REG_FIELD(0x554, 0, 1),
+		[PHY_GMII_SEL_RGMII_ID_MODE] = REG_FIELD((~0), 0, 0),
+		[PHY_GMII_SEL_RMII_IO_CLK_EN] = REG_FIELD((~0), 0, 0),
+	},
+	{
+		[PHY_GMII_SEL_PORT_MODE] = REG_FIELD(0x554, 4, 5),
+		[PHY_GMII_SEL_RGMII_ID_MODE] = REG_FIELD((~0), 0, 0),
+		[PHY_GMII_SEL_RMII_IO_CLK_EN] = REG_FIELD((~0), 0, 0),
+	},
+};
+
+static const
+struct phy_gmii_sel_soc_data phy_gmii_sel_soc_dra7 = {
+	.num_ports = 2,
+	.regfields = phy_gmii_sel_fields_dra7,
+};
+
+static const
+struct phy_gmii_sel_soc_data phy_gmii_sel_soc_dm814 = {
+	.num_ports = 2,
+	.features = BIT(PHY_GMII_SEL_RGMII_ID_MODE),
+	.regfields = phy_gmii_sel_fields_am33xx,
+};
+
+static const struct of_device_id phy_gmii_sel_id_table[] = {
+	{
+		.compatible	= "ti,am3352-phy-gmii-sel",
+		.data		= &phy_gmii_sel_soc_am33xx,
+	},
+	{
+		.compatible	= "ti,dra7xx-phy-gmii-sel",
+		.data		= &phy_gmii_sel_soc_dra7,
+	},
+	{
+		.compatible	= "ti,am43xx-phy-gmii-sel",
+		.data		= &phy_gmii_sel_soc_am33xx,
+	},
+	{
+		.compatible	= "ti,dm814-phy-gmii-sel",
+		.data		= &phy_gmii_sel_soc_dm814,
+	},
+	{}
+};
+MODULE_DEVICE_TABLE(of, phy_gmii_sel_id_table);
+
+static const struct phy_ops phy_gmii_sel_ops = {
+	.set_netif_mode	= phy_gmii_sel_mode,
+	.owner		= THIS_MODULE,
+};
+
+static struct phy *phy_gmii_sel_of_xlate(struct device *dev,
+						struct of_phandle_args *args)
+{
+	struct phy_gmii_sel_priv *priv = dev_get_drvdata(dev);
+	int phy_id = args->args[0];
+
+	if (args->args_count < 1)
+		return ERR_PTR(-EINVAL);
+	if (priv->soc_data->features & BIT(PHY_GMII_SEL_RMII_IO_CLK_EN) &&
+	    args->args_count < 2)
+		return ERR_PTR(-EINVAL);
+	if (!priv || !priv->if_phys)
+		return ERR_PTR(-ENODEV);
+	if (phy_id > priv->soc_data->num_ports)
+		return ERR_PTR(-EINVAL);
+	if (phy_id != priv->if_phys[phy_id - 1].id)
+		return ERR_PTR(-EINVAL);
+
+	phy_id--;
+	if (priv->soc_data->features & BIT(PHY_GMII_SEL_RMII_IO_CLK_EN))
+		priv->if_phys[phy_id].rmii_clock_external = args->args[1];
+	dev_dbg(dev, "%s id:%u ext:%d\n", __func__,
+		priv->if_phys[phy_id].id, args->args[1]);
+
+	return priv->if_phys[phy_id].if_phy;
+}
+
+static int phy_gmii_sel_init_ports(struct phy_gmii_sel_priv *priv)
+{
+	const struct phy_gmii_sel_soc_data *soc_data = priv->soc_data;
+	struct device *dev = priv->dev;
+	struct phy_gmii_sel_phy_priv *if_phys;
+	int i, num_ports, ret;
+
+	num_ports = priv->soc_data->num_ports;
+
+	if_phys = devm_kcalloc(priv->dev, num_ports,
+			       sizeof(*if_phys), GFP_KERNEL);
+	if (!if_phys)
+		return -ENOMEM;
+	dev_dbg(dev, "%s %d\n", __func__, num_ports);
+
+	for (i = 0; i < num_ports; i++) {
+		const struct reg_field *field;
+		struct regmap_field *regfield;
+
+		if_phys[i].id = i + 1;
+		if_phys[i].priv = priv;
+
+		field = &soc_data->regfields[i][PHY_GMII_SEL_PORT_MODE];
+		dev_dbg(dev, "%s field %x %d %d\n", __func__,
+			field->reg, field->msb, field->lsb);
+
+		regfield = devm_regmap_field_alloc(dev, priv->regmap, *field);
+		if (IS_ERR(regfield))
+			return PTR_ERR(regfield);
+		if_phys[i].fields[PHY_GMII_SEL_PORT_MODE] = regfield;
+
+		field = &soc_data->regfields[i][PHY_GMII_SEL_RGMII_ID_MODE];
+		if (field->reg != (~0)) {
+			regfield = devm_regmap_field_alloc(dev,
+							   priv->regmap,
+							   *field);
+			if (IS_ERR(regfield))
+				return PTR_ERR(regfield);
+			if_phys[i].fields[PHY_GMII_SEL_RGMII_ID_MODE] =
+				regfield;
+		}
+
+		field = &soc_data->regfields[i][PHY_GMII_SEL_RMII_IO_CLK_EN];
+		if (field->reg != (~0)) {
+			regfield = devm_regmap_field_alloc(dev,
+							   priv->regmap,
+							   *field);
+			if (IS_ERR(regfield))
+				return PTR_ERR(regfield);
+			if_phys[i].fields[PHY_GMII_SEL_RMII_IO_CLK_EN] =
+				regfield;
+		}
+
+		if_phys[i].if_phy = devm_phy_create(dev,
+						    priv->dev->of_node,
+						    &phy_gmii_sel_ops);
+		if (IS_ERR(if_phys[i].if_phy)) {
+			ret = PTR_ERR(if_phys[i].if_phy);
+			dev_err(dev, "Failed to create phy%d %d\n", i, ret);
+			return ret;
+		}
+		phy_set_drvdata(if_phys[i].if_phy, &if_phys[i]);
+	}
+
+	priv->if_phys = if_phys;
+	return 0;
+}
+
+static int phy_gmii_sel_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *node = dev->of_node;
+	const struct of_device_id *of_id;
+	struct phy_gmii_sel_priv *priv;
+	int ret;
+
+	of_id = of_match_node(phy_gmii_sel_id_table, pdev->dev.of_node);
+	if (!of_id)
+		return -EINVAL;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->dev = &pdev->dev;
+	priv->soc_data = of_id->data;
+
+	priv->regmap = syscon_regmap_lookup_by_phandle(node, "syscon-scm");
+	if (IS_ERR(priv->regmap)) {
+		ret = PTR_ERR(priv->regmap);
+		dev_err(dev, "Failed to get syscon %d\n", ret);
+		return ret;
+	}
+
+	ret = phy_gmii_sel_init_ports(priv);
+	if (ret)
+		return ret;
+
+	dev_set_drvdata(&pdev->dev, priv);
+
+	priv->phy_provider =
+		devm_of_phy_provider_register(dev,
+					      phy_gmii_sel_of_xlate);
+	if (IS_ERR(priv->phy_provider)) {
+		ret = PTR_ERR(priv->phy_provider);
+		dev_err(dev, "Failed to create phy provider %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static struct platform_driver phy_gmii_sel_driver = {
+	.probe		= phy_gmii_sel_probe,
+	.driver		= {
+		.name	= "phy-gmii-sel",
+		.of_match_table = phy_gmii_sel_id_table,
+	},
+};
+module_platform_driver(phy_gmii_sel_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Grygorii Strashko <grygorii.strashko@ti.com>");
+MODULE_DESCRIPTION("TI CPSW Port's PHY Interface Mode selection Driver");
-- 
2.10.5

[RFC PATCH 04/11] dt-bindings: net: ti: cpsw: switch to use phy-gmii-sel phy

[Grygorii Strashko]

The cpsw-phy-sel driver was replaced with new PHY driver phy-gmii-sel, so
deprecate cpsw-phy-sel bindings and update CPSW binding to use phy-gmii-sel
PHY bindings.

Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 Documentation/devicetree/bindings/net/cpsw.txt | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
index b3acebe..69d7ef8 100644
--- a/Documentation/devicetree/bindings/net/cpsw.txt
+++ b/Documentation/devicetree/bindings/net/cpsw.txt
@@ -22,7 +22,8 @@ Required properties:
 - cpsw-phy-sel		: Specifies the phandle to the CPSW phy mode selection
 			  device. See also cpsw-phy-sel.txt for it's binding.
 			  Note that in legacy cases cpsw-phy-sel may be
-			  a child device instead of a phandle.
+			  a child device instead of a phandle
+			  (DEPRECATED, use phy-gmii-sel PHY phandle).
 
 Optional properties:
 - ti,hwmods		: Must be "cpgmac0"
@@ -44,6 +45,7 @@ Optional properties:
 Slave Properties:
 Required properties:
 - phy-mode		: See ethernet.txt file in the same directory
+- phys			: phandle on phy-gmii-sel PHY (see phy/ti-phy-gmii-sel.txt)
 
 Optional properties:
 - dual_emac_res_vlan	: Specifies VID to be used to segregate the ports
@@ -85,12 +87,14 @@ Examples:
 			phy-mode = "rgmii-txid";
 			/* Filled in by U-Boot */
 			mac-address = [ 00 00 00 00 00 00 ];
+			phys = <&phy_gmii_sel 1 0>;
 		};
 		cpsw_emac1: slave@1 {
 			phy_id = <&davinci_mdio>, <1>;
 			phy-mode = "rgmii-txid";
 			/* Filled in by U-Boot */
 			mac-address = [ 00 00 00 00 00 00 ];
+			phys = <&phy_gmii_sel 2 0>;
 		};
 	};
 
@@ -114,11 +118,13 @@ Examples:
 			phy-mode = "rgmii-txid";
 			/* Filled in by U-Boot */
 			mac-address = [ 00 00 00 00 00 00 ];
+			phys = <&phy_gmii_sel 1 0>;
 		};
 		cpsw_emac1: slave@1 {
 			phy_id = <&davinci_mdio>, <1>;
 			phy-mode = "rgmii-txid";
 			/* Filled in by U-Boot */
 			mac-address = [ 00 00 00 00 00 00 ];
+			phys = <&phy_gmii_sel 2 0>;
 		};
 	};
-- 
2.10.5

[RFC PATCH 05/11] net: ethernet: ti: cpsw: add support for port interface mode selection phy

[Grygorii Strashko]

Add support for port interface mode selection phy (phy-gmii-sel):
- try to request interface mode selection phy from Port DT node and fail
silently if not defined and old CONFIG_TI_CPSW_PHY_SEL driver enabled.
- use new phy if requested successfully.

Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/net/ethernet/ti/cpsw.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 832bce0..4607de2 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -26,6 +26,7 @@
 #include <linux/netdevice.h>
 #include <linux/net_tstamp.h>
 #include <linux/phy.h>
+#include <linux/phy/phy.h>
 #include <linux/workqueue.h>
 #include <linux/delay.h>
 #include <linux/pm_runtime.h>
@@ -387,6 +388,7 @@ struct cpsw_slave_data {
 	int		phy_if;
 	u8		mac_addr[ETH_ALEN];
 	u16		dual_emac_res_vlan;	/* Reserved VLAN for DualEMAC */
+	struct phy	*ifphy;
 };
 
 struct cpsw_platform_data {
@@ -1502,7 +1504,11 @@ static void cpsw_slave_open(struct cpsw_slave *slave, struct cpsw_priv *priv)
 	phy_start(slave->phy);
 
 	/* Configure GMII_SEL register */
-	cpsw_phy_sel(cpsw->dev, slave->phy->interface, slave->slave_num);
+	if (!IS_ERR(slave->data->ifphy))
+		phy_set_netif_mode(slave->data->ifphy, slave->data->phy_if);
+	else
+		cpsw_phy_sel(cpsw->dev, slave->phy->interface,
+			     slave->slave_num);
 }
 
 static inline void cpsw_add_default_vlan(struct cpsw_priv *priv)
@@ -3135,6 +3141,16 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
 		if (strcmp(slave_node->name, "slave"))
 			continue;
 
+		slave_data->ifphy = devm_of_phy_get(&pdev->dev, slave_node,
+						    NULL);
+		if (!IS_ENABLED(CONFIG_TI_CPSW_PHY_SEL) &&
+		    IS_ERR(slave_data->ifphy)) {
+			ret = PTR_ERR(slave_data->ifphy);
+			dev_err(&pdev->dev,
+				"%d: Error retrieving port phy: %d\n", i, ret);
+			return ret;
+		}
+
 		slave_data->phy_node = of_parse_phandle(slave_node,
 							"phy-handle", 0);
 		parp = of_get_property(slave_node, "phy_id", &lenp);
-- 
2.10.5

[RFC PATCH 06/11] ARM: dts: dra7: switch to use phy-gmii-sel

[Grygorii Strashko]

Switch to use phy-gmii-sel PHY instead of cpsw-phy-sel.

Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 arch/arm/boot/dts/dra7.dtsi | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
index a0ddf49..07a0edf 100644
--- a/arch/arm/boot/dts/dra7.dtsi
+++ b/arch/arm/boot/dts/dra7.dtsi
@@ -187,6 +187,12 @@
 						};
 					};
 
+					phy_gmii_sel: phy-gmii-sel {
+						compatible = "ti,dra7xx-phy-gmii-sel";
+						syscon-scm = <&scm_conf>;
+						#phy-cells = <1>;
+					};
+
 					scm_conf_clocks: clocks {
 						#address-cells = <1>;
 						#size-cells = <0>;
@@ -1879,17 +1885,13 @@
 			cpsw_emac0: slave@48480200 {
 				/* Filled in by U-Boot */
 				mac-address = [ 00 00 00 00 00 00 ];
+				phys = <&phy_gmii_sel 1>;
 			};
 
 			cpsw_emac1: slave@48480300 {
 				/* Filled in by U-Boot */
 				mac-address = [ 00 00 00 00 00 00 ];
-			};
-
-			phy_sel: cpsw-phy-sel@4a002554 {
-				compatible = "ti,dra7xx-cpsw-phy-sel";
-				reg= <0x4a002554 0x4>;
-				reg-names = "gmii-sel";
+				phys = <&phy_gmii_sel 2>;
 			};
 		};
 
-- 
2.10.5

[RFC PATCH 07/11] ARM: dts: dm814x: switch to use phy-gmii-sel

[Grygorii Strashko]

Switch to use phy-gmii-sel PHY instead of cpsw-phy-sel.

Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 arch/arm/boot/dts/dm814x.dtsi | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/arch/arm/boot/dts/dm814x.dtsi b/arch/arm/boot/dts/dm814x.dtsi
index 601c57a..5728f96 100644
--- a/arch/arm/boot/dts/dm814x.dtsi
+++ b/arch/arm/boot/dts/dm814x.dtsi
@@ -343,6 +343,12 @@
 					#size-cells = <1>;
 					ranges = <0 0 0x800>;
 
+					phy_gmii_sel: phy-gmii-sel {
+						compatible = "ti,dm814-phy-gmii-sel";
+						syscon-scm = <&scm_conf>;
+						#phy-cells = <1>;
+					};
+
 					scm_clocks: clocks {
 						#address-cells = <1>;
 						#size-cells = <0>;
@@ -549,17 +555,14 @@
 			cpsw_emac0: slave@4a100200 {
 				/* Filled in by U-Boot */
 				mac-address = [ 00 00 00 00 00 00 ];
+				phys = <&phy_gmii_sel 1>;
+
 			};
 
 			cpsw_emac1: slave@4a100300 {
 				/* Filled in by U-Boot */
 				mac-address = [ 00 00 00 00 00 00 ];
-			};
-
-			phy_sel: cpsw-phy-sel@48140650 {
-				compatible = "ti,am3352-cpsw-phy-sel";
-				reg= <0x48140650 0x4>;
-				reg-names = "gmii-sel";
+				phys = <&phy_gmii_sel 2>;
 			};
 		};
 
-- 
2.10.5

[RFC PATCH 08/11] ARM: dts: am4372: switch to use phy-gmii-sel

[Grygorii Strashko]

Switch to use phy-gmii-sel PHY instead of cpsw-phy-sel.

Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 arch/arm/boot/dts/am4372.dtsi        | 16 +++++++++-------
 arch/arm/boot/dts/am43x-epos-evm.dts |  5 +----
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi
index d4b7c59..3dd6d2e 100644
--- a/arch/arm/boot/dts/am4372.dtsi
+++ b/arch/arm/boot/dts/am4372.dtsi
@@ -212,11 +212,17 @@
 				};
 
 				scm_conf: scm_conf@0 {
-					compatible = "syscon";
+					compatible = "syscon", "simple-bus";
 					reg = <0x0 0x800>;
 					#address-cells = <1>;
 					#size-cells = <1>;
 
+					phy_gmii_sel: phy-gmii-sel {
+						compatible = "ti,am43xx-phy-gmii-sel";
+						syscon-scm = <&scm_conf>;
+						#phy-cells = <2>;
+					};
+
 					scm_clocks: clocks {
 						#address-cells = <1>;
 						#size-cells = <0>;
@@ -714,17 +720,13 @@
 			cpsw_emac0: slave@4a100200 {
 				/* Filled in by U-Boot */
 				mac-address = [ 00 00 00 00 00 00 ];
+				phys = <&phy_gmii_sel 1 0>;
 			};
 
 			cpsw_emac1: slave@4a100300 {
 				/* Filled in by U-Boot */
 				mac-address = [ 00 00 00 00 00 00 ];
-			};
-
-			phy_sel: cpsw-phy-sel@44e10650 {
-				compatible = "ti,am43xx-cpsw-phy-sel";
-				reg= <0x44e10650 0x4>;
-				reg-names = "gmii-sel";
+				phys = <&phy_gmii_sel 2 0>;
 			};
 		};
 
diff --git a/arch/arm/boot/dts/am43x-epos-evm.dts b/arch/arm/boot/dts/am43x-epos-evm.dts
index 6502d33..8677f4a 100644
--- a/arch/arm/boot/dts/am43x-epos-evm.dts
+++ b/arch/arm/boot/dts/am43x-epos-evm.dts
@@ -580,10 +580,7 @@
 &cpsw_emac0 {
 	phy_id = <&davinci_mdio>, <16>;
 	phy-mode = "rmii";
-};
-
-&phy_sel {
-	rmii-clock-ext;
+	phys = <&phy_gmii_sel 1 1>;
 };
 
 &i2c0 {
-- 
2.10.5

[RFC PATCH 09/11] ARM: dts: am335x: switch to use phy-gmii-sel

[Grygorii Strashko]

Switch to use phy-gmii-sel PHY instead of cpsw-phy-sel.

Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 arch/arm/boot/dts/am335x-baltos-ir2110.dts     |  4 ----
 arch/arm/boot/dts/am335x-baltos-ir3220.dts     |  3 ---
 arch/arm/boot/dts/am335x-baltos-ir5221.dts     |  3 ---
 arch/arm/boot/dts/am335x-chiliboard.dts        |  3 ---
 arch/arm/boot/dts/am335x-icev2.dts             |  4 ----
 arch/arm/boot/dts/am335x-igep0033.dtsi         |  3 ---
 arch/arm/boot/dts/am335x-lxm.dts               |  3 ---
 arch/arm/boot/dts/am335x-moxa-uc-8100-me-t.dts |  5 -----
 arch/arm/boot/dts/am335x-phycore-som.dtsi      |  3 ---
 arch/arm/boot/dts/am33xx.dtsi                  | 14 ++++++++------
 10 files changed, 8 insertions(+), 37 deletions(-)

diff --git a/arch/arm/boot/dts/am335x-baltos-ir2110.dts b/arch/arm/boot/dts/am335x-baltos-ir2110.dts
index 75de1e7..50dcf12 100644
--- a/arch/arm/boot/dts/am335x-baltos-ir2110.dts
+++ b/arch/arm/boot/dts/am335x-baltos-ir2110.dts
@@ -72,7 +72,3 @@
 	dual_emac_res_vlan = <2>;
 	phy-handle = <&phy1>;
 };
-
-&phy_sel {
-	rmii-clock-ext = <1>;
-};
diff --git a/arch/arm/boot/dts/am335x-baltos-ir3220.dts b/arch/arm/boot/dts/am335x-baltos-ir3220.dts
index 1b215c4..44f7858 100644
--- a/arch/arm/boot/dts/am335x-baltos-ir3220.dts
+++ b/arch/arm/boot/dts/am335x-baltos-ir3220.dts
@@ -115,6 +115,3 @@
 	phy-handle = <&phy1>;
 };
 
-&phy_sel {
-	rmii-clock-ext = <1>;
-};
diff --git a/arch/arm/boot/dts/am335x-baltos-ir5221.dts b/arch/arm/boot/dts/am335x-baltos-ir5221.dts
index 832ead8..1e10813 100644
--- a/arch/arm/boot/dts/am335x-baltos-ir5221.dts
+++ b/arch/arm/boot/dts/am335x-baltos-ir5221.dts
@@ -133,9 +133,6 @@
 	phy-handle = <&phy1>;
 };
 
-&phy_sel {
-	rmii-clock-ext = <1>;
-};
 
 &dcan1 {
 	pinctrl-names = "default";
diff --git a/arch/arm/boot/dts/am335x-chiliboard.dts b/arch/arm/boot/dts/am335x-chiliboard.dts
index 59431b2..7b77315 100644
--- a/arch/arm/boot/dts/am335x-chiliboard.dts
+++ b/arch/arm/boot/dts/am335x-chiliboard.dts
@@ -147,9 +147,6 @@
 	phy-mode = "rmii";
 };
 
-&phy_sel {
-	rmii-clock-ext;
-};
 
 /* USB */
 &usb {
diff --git a/arch/arm/boot/dts/am335x-icev2.dts b/arch/arm/boot/dts/am335x-icev2.dts
index f2005ec..9ac775c 100644
--- a/arch/arm/boot/dts/am335x-icev2.dts
+++ b/arch/arm/boot/dts/am335x-icev2.dts
@@ -484,10 +484,6 @@
 	dual_emac;
 };
 
-&phy_sel {
-	rmii-clock-ext;
-};
-
 &davinci_mdio {
 	pinctrl-names = "default", "sleep";
 	pinctrl-0 = <&davinci_mdio_default>;
diff --git a/arch/arm/boot/dts/am335x-igep0033.dtsi b/arch/arm/boot/dts/am335x-igep0033.dtsi
index a5769a8..2bc8456 100644
--- a/arch/arm/boot/dts/am335x-igep0033.dtsi
+++ b/arch/arm/boot/dts/am335x-igep0033.dtsi
@@ -114,9 +114,6 @@
 	phy-mode = "rmii";
 };
 
-&phy_sel {
-	rmii-clock-ext;
-};
 
 &elm {
 	status = "okay";
diff --git a/arch/arm/boot/dts/am335x-lxm.dts b/arch/arm/boot/dts/am335x-lxm.dts
index 1d6c6fa..56b354b1 100644
--- a/arch/arm/boot/dts/am335x-lxm.dts
+++ b/arch/arm/boot/dts/am335x-lxm.dts
@@ -328,9 +328,6 @@
 	dual_emac_res_vlan = <3>;
 };
 
-&phy_sel {
-	rmii-clock-ext;
-};
 
 &mac {
 	pinctrl-names = "default", "sleep";
diff --git a/arch/arm/boot/dts/am335x-moxa-uc-8100-me-t.dts b/arch/arm/boot/dts/am335x-moxa-uc-8100-me-t.dts
index f82233c..5563e92 100644
--- a/arch/arm/boot/dts/am335x-moxa-uc-8100-me-t.dts
+++ b/arch/arm/boot/dts/am335x-moxa-uc-8100-me-t.dts
@@ -438,11 +438,6 @@
 	dual_emac_res_vlan = <2>;
 };
 
-&phy_sel {
-	reg= <0x44e10650 0xf5>;
-	rmii-clock-ext;
-};
-
 &sham {
 	status = "okay";
 };
diff --git a/arch/arm/boot/dts/am335x-phycore-som.dtsi b/arch/arm/boot/dts/am335x-phycore-som.dtsi
index 428a25e..cf40a2b 100644
--- a/arch/arm/boot/dts/am335x-phycore-som.dtsi
+++ b/arch/arm/boot/dts/am335x-phycore-som.dtsi
@@ -100,9 +100,6 @@
 	status = "okay";
 };
 
-&phy_sel {
-	rmii-clock-ext;
-};
 
 /* I2C Busses */
 &am33xx_pinmux {
diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index d3dd6a1..3179b24 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -222,6 +222,12 @@
 					#size-cells = <1>;
 					ranges = <0 0 0x800>;
 
+					phy_gmii_sel: phy-gmii-sel {
+						compatible = "ti,am3352-phy-gmii-sel";
+						syscon-scm = <&scm_conf>;
+						#phy-cells = <2>;
+					};
+
 					scm_clocks: clocks {
 						#address-cells = <1>;
 						#size-cells = <0>;
@@ -890,17 +896,13 @@
 			cpsw_emac0: slave@4a100200 {
 				/* Filled in by U-Boot */
 				mac-address = [ 00 00 00 00 00 00 ];
+				phys = <&phy_gmii_sel 1 1>;
 			};
 
 			cpsw_emac1: slave@4a100300 {
 				/* Filled in by U-Boot */
 				mac-address = [ 00 00 00 00 00 00 ];
-			};
-
-			phy_sel: cpsw-phy-sel@44e10650 {
-				compatible = "ti,am3352-cpsw-phy-sel";
-				reg= <0x44e10650 0x4>;
-				reg-names = "gmii-sel";
+				phys = <&phy_gmii_sel 2 1>;
 			};
 		};
 
-- 
2.10.5

[RFC PATCH 10/11] dt-bindings: net: ti: deprecate cpsw-phy-sel bindings

[Grygorii Strashko]

The cpsw-phy-sel driver was replaced with new PHY driver phy-gmii-sel, so
deprecate cpsw-phy-sel bindings.

Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 Documentation/devicetree/bindings/net/cpsw-phy-sel.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/cpsw-phy-sel.txt b/Documentation/devicetree/bindings/net/cpsw-phy-sel.txt
index 764c0c7..5d76f99 100644
--- a/Documentation/devicetree/bindings/net/cpsw-phy-sel.txt
+++ b/Documentation/devicetree/bindings/net/cpsw-phy-sel.txt
@@ -1,4 +1,4 @@
-TI CPSW Phy mode Selection Device Tree Bindings
+TI CPSW Phy mode Selection Device Tree Bindings (DEPRECATED)
 -----------------------------------------------
 
 Required properties:
-- 
2.10.5

[RFC PATCH 11/11] net: ethernet: ti: cpsw: deprecate cpsw-phy-sel driver

[Grygorii Strashko]

Deprecate cpsw-phy-sel driver as it's been replaced with new
TI phy-gmii-sel PHY driver.

Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/net/ethernet/ti/Kconfig | 6 +++---
 drivers/net/ethernet/ti/cpsw.h  | 6 ++++++
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig
index f932923..96415da 100644
--- a/drivers/net/ethernet/ti/Kconfig
+++ b/drivers/net/ethernet/ti/Kconfig
@@ -49,10 +49,11 @@ config TI_DAVINCI_CPDMA
 	  will be called davinci_cpdma.  This is recommended.
 
 config TI_CPSW_PHY_SEL
-	bool
+	bool "TI CPSW Phy mode Selection (DEPRECATED)"
+	default n
 	---help---
 	  This driver supports configuring of the phy mode connected to
-	  the CPSW.
+	  the CPSW. DEPRECATED: use PHY_TI_GMII_SEL.
 
 config TI_CPSW_ALE
 	tristate "TI CPSW ALE Support"
@@ -64,7 +65,6 @@ config TI_CPSW
 	depends on ARCH_DAVINCI || ARCH_OMAP2PLUS || COMPILE_TEST
 	select TI_DAVINCI_CPDMA
 	select TI_DAVINCI_MDIO
-	select TI_CPSW_PHY_SEL
 	select TI_CPSW_ALE
 	select MFD_SYSCON
 	select REGMAP
diff --git a/drivers/net/ethernet/ti/cpsw.h b/drivers/net/ethernet/ti/cpsw.h
index cf111db..907e05fc 100644
--- a/drivers/net/ethernet/ti/cpsw.h
+++ b/drivers/net/ethernet/ti/cpsw.h
@@ -21,7 +21,13 @@
 			 ((mac)[2] << 16) | ((mac)[3] << 24))
 #define mac_lo(mac)	(((mac)[4] << 0) | ((mac)[5] << 8))
 
+#if IS_ENABLED(CONFIG_TI_CPSW_PHY_SEL)
 void cpsw_phy_sel(struct device *dev, phy_interface_t phy_mode, int slave);
+#else
+static inline
+void cpsw_phy_sel(struct device *dev, phy_interface_t phy_mode, int slave)
+{}
+#endif
 int ti_cm_get_macid(struct device *dev, int slave, u8 *mac_addr);
 
 #endif /* __CPSW_H__ */
-- 
2.10.5

Re: [RFC PATCH 03/11] phy: ti: introduce phy-gmii-sel driver

[Andrew Lunn]

On Mon, Oct 08, 2018 at 06:49:41PM -0500, Grygorii Strashko wrote:
> +static int phy_gmii_sel_mode(struct phy *phy, phy_interface_t intf_mode)
> +{
> +	struct phy_gmii_sel_phy_priv *if_phy = phy_get_drvdata(phy);
> +	const struct phy_gmii_sel_soc_data *soc_data = if_phy->priv->soc_data;
> +	struct device *dev = if_phy->priv->dev;
> +	struct regmap_field *regfield;
> +	int ret, rgmii_id = 0;
> +	u32 mode = 0;
> +
> +	if_phy->phy_if_mode = intf_mode;
> +
> +	switch (if_phy->phy_if_mode) {
> +	case PHY_INTERFACE_MODE_RMII:
> +		mode = AM33XX_GMII_SEL_MODE_RMII;
> +		break;
> +
> +	case PHY_INTERFACE_MODE_RGMII:
> +		mode = AM33XX_GMII_SEL_MODE_RGMII;
> +		break;
> +
> +	case PHY_INTERFACE_MODE_RGMII_ID:
> +	case PHY_INTERFACE_MODE_RGMII_RXID:
> +	case PHY_INTERFACE_MODE_RGMII_TXID:
> +		mode = AM33XX_GMII_SEL_MODE_RGMII;
> +		rgmii_id = 1;
> +		break;

Hi Grygorii

It looks like the MAC can do AM33XX_GMII_SEL_MODE_RGMII and
AM33XX_GMII_SEL_MODE_RGMII_ID. I don't think it can do
AM33XX_GMII_SEL_MODE_RGMII_RXID or AM33XX_GMII_SEL_MODE_RGMII_TXID?  I
would prefer it return -EINVAL when asked to do something it cannot
do.

> +
> +	default:
> +		dev_warn(dev,
> +			 "port%u: unsupported mode: \"%s\". Defaulting to MII.\n",
> +			 if_phy->id, phy_modes(rgmii_id));
> +		/* fall through */

Returning -EINVAL would be better. Otherwise the DT might never get
fixed.

> +	case PHY_INTERFACE_MODE_MII:
> +		mode = AM33XX_GMII_SEL_MODE_MII;
> +		break;
> +	};
> +
> +	dev_dbg(dev, "%s id:%u mode:%u rgmii_id:%d rmii_clk_ext:%d\n",
> +		__func__, if_phy->id, mode, rgmii_id,
> +		if_phy->rmii_clock_external);
> +
> +	regfield = if_phy->fields[PHY_GMII_SEL_PORT_MODE];
> +	ret = regmap_field_write(regfield, mode);
> +
> +	if (soc_data->features & BIT(PHY_GMII_SEL_RGMII_ID_MODE) &&
> +	    if_phy->fields[PHY_GMII_SEL_RGMII_ID_MODE]) {
> +		regfield = if_phy->fields[PHY_GMII_SEL_RGMII_ID_MODE];
> +		ret |= regmap_field_write(regfield, rgmii_id);
> +	}
> +
> +	if (soc_data->features & BIT(PHY_GMII_SEL_RMII_IO_CLK_EN) &&
> +	    if_phy->fields[PHY_GMII_SEL_RMII_IO_CLK_EN]) {
> +		regfield = if_phy->fields[PHY_GMII_SEL_RMII_IO_CLK_EN];
> +		ret |= regmap_field_write(regfield,
> +					  if_phy->rmii_clock_external);
> +	}
> +
> +	if (ret) {
> +		dev_err(dev, "port%u: set mode fail %d", if_phy->id, ret);
> +		return -EIO;
> +	}

I would prefer each write had its own error check. The fact you don't
return ret means you know ret could be -EINVAL|-EOIO, making
-EMORECOFFEE.

	Andrew

Re: [RFC PATCH 05/11] net: ethernet: ti: cpsw: add support for port interface mode selection phy

[Andrew Lunn]

>  	/* Configure GMII_SEL register */
> -	cpsw_phy_sel(cpsw->dev, slave->phy->interface, slave->slave_num);
> +	if (!IS_ERR(slave->data->ifphy))
> +		phy_set_netif_mode(slave->data->ifphy, slave->data->phy_if);

Is slave->data->phy_if also passed to phy_connect()? So you are going
to end up with both the MAC and the PHY inserting RGMII delays, and it
not working.

You need to somehow decide if the MAC is going to do the delay, or the
PHY. But not both.

	Andrew

Re: [RFC PATCH 01/11] phy: core add phy_set_netif_mode() api

[Kishon Vijay Abraham I]

Hi Grygorii,

On Tuesday 09 October 2018 05:19 AM, Grygorii Strashko wrote:
> Add new API phy_set_netif_mode(struct phy *phy, phy_interface_t mode) and
> new PHY operation callback .set_netif_mode() which intended to be implemnte
> by PHY drivers which supports Network interrfaces mode selection. Both
> accepts phy_interface_t vlaue as input parameter.
> 
> Cc: Kishon Vijay Abraham I <kishon@ti.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
>  drivers/phy/phy-core.c  | 15 +++++++++++++++
>  include/linux/phy/phy.h | 12 ++++++++++++
>  2 files changed, 27 insertions(+)
> 
> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index 35fd38c..d9aba1a 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
> @@ -377,6 +377,21 @@ int phy_set_mode(struct phy *phy, enum phy_mode mode)
>  }
>  EXPORT_SYMBOL_GPL(phy_set_mode);
>  
> +int phy_set_netif_mode(struct phy *phy, phy_interface_t mode)
> +{
> +	int ret;
> +
> +	if (!phy || !phy->ops->set_netif_mode)
> +		return 0;
> +
> +	mutex_lock(&phy->mutex);
> +	ret = phy->ops->set_netif_mode(phy, mode);
> +	mutex_unlock(&phy->mutex);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(phy_set_netif_mode);

We should try to add only generic PHY APIs and not subsystem specific APIs. In
this case I think phy_set_mode should suffice.

Thanks
Kishon

> +
>  int phy_reset(struct phy *phy)
>  {
>  	int ret;
> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> index 9713aeb..bc73d2b 100644
> --- a/include/linux/phy/phy.h
> +++ b/include/linux/phy/phy.h
> @@ -17,6 +17,7 @@
>  #include <linux/err.h>
>  #include <linux/of.h>
>  #include <linux/device.h>
> +#include <linux/phy.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/regulator/consumer.h>
>  
> @@ -49,6 +50,7 @@ enum phy_mode {
>   * @power_on: powering on the phy
>   * @power_off: powering off the phy
>   * @set_mode: set the mode of the phy
> + * @set_netif_mode: set the mode of the net interface phy
>   * @reset: resetting the phy
>   * @calibrate: calibrate the phy
>   * @owner: the module owner containing the ops
> @@ -59,6 +61,7 @@ struct phy_ops {
>  	int	(*power_on)(struct phy *phy);
>  	int	(*power_off)(struct phy *phy);
>  	int	(*set_mode)(struct phy *phy, enum phy_mode mode);
> +	int	(*set_netif_mode)(struct phy *phy, phy_interface_t mode);
>  	int	(*reset)(struct phy *phy);
>  	int	(*calibrate)(struct phy *phy);
>  	struct module *owner;
> @@ -163,6 +166,7 @@ int phy_exit(struct phy *phy);
>  int phy_power_on(struct phy *phy);
>  int phy_power_off(struct phy *phy);
>  int phy_set_mode(struct phy *phy, enum phy_mode mode);
> +int phy_set_netif_mode(struct phy *phy, phy_interface_t mode);
>  static inline enum phy_mode phy_get_mode(struct phy *phy)
>  {
>  	return phy->attrs.mode;
> @@ -283,6 +287,14 @@ static inline int phy_set_mode(struct phy *phy, enum phy_mode mode)
>  	return -ENOSYS;
>  }
>  
> +static inline int phy_set_netif_mode(struct phy *phy,
> +				     phy_interface_t mode)
> +{
> +	if (!phy)
> +		return 0;
> +	return -ENOTSUPP;
> +}
> +
>  static inline enum phy_mode phy_get_mode(struct phy *phy)
>  {
>  	return PHY_MODE_INVALID;
> 

Re: [RFC PATCH 00/11] net: ethernet: ti: cpsw: replace cpsw-phy-sel with phy driver

[Tony Lindgren]

* Grygorii Strashko <grygorii.strashko@ti.com> [181008 23:54]:
> 2) introduce new PHY API for network interface mode selection which will use
> already defined set of modes from phy_interface_t.
> 
> Option 2 was selected for this series.

Looks good to me :) The dts files will cause merge conflicts with
what I have pending for the ti-sysc changes so please send the dts
changes in a separate series when posting without RFC.

Thanks,

Tony

Re: [RFC PATCH 02/11] dt-bindings: phy: add cpsw port interface mode selection phy bindings

[Tony Lindgren]

* Grygorii Strashko <grygorii.strashko@ti.com> [181008 23:54]:
> +Examples:
> +	phy_gmii_sel: phy-gmii-sel {
> +		compatible = "ti,am3352-phy-gmii-sel";
> +		syscon-scm = <&scm_conf>;
> +		#phy-cells = <2>;
> +	};

Now that this driver can live in it's proper place in the
dts, you may want to consider just using standard reg
property for it instead of the syscon-scm. And also get
rid of the syscon reads and writes.

Regards,

Tony

Re: [RFC PATCH 02/11] dt-bindings: phy: add cpsw port interface mode selection phy bindings

[Grygorii Strashko]

On 10/09/2018 09:40 AM, Tony Lindgren wrote:
> * Grygorii Strashko <grygorii.strashko@ti.com> [181008 23:54]:
>> +Examples:
>> +	phy_gmii_sel: phy-gmii-sel {
>> +		compatible = "ti,am3352-phy-gmii-sel";
>> +		syscon-scm = <&scm_conf>;
>> +		#phy-cells = <2>;
>> +	};
> 
> Now that this driver can live in it's proper place in the

right

> dts, you may want to consider just using standard reg
> property for it instead of the syscon-scm. And also get
> rid of the syscon reads and writes.

Could you help clarify how to get syscon in this case?
syscon_node_to_regmap(dev->parent->of_node)?

Also, there are could be more then one gmii_sel registers in SCM in the future,
so I hidden offsets in of_match data. 
As result, "reg" not needed at all now.

-- 
regards,
-grygorii

Re: [RFC PATCH 03/11] phy: ti: introduce phy-gmii-sel driver

[Grygorii Strashko]

Hi Andrew,

On 10/08/2018 07:39 PM, Andrew Lunn wrote:
> On Mon, Oct 08, 2018 at 06:49:41PM -0500, Grygorii Strashko wrote:
>> +static int phy_gmii_sel_mode(struct phy *phy, phy_interface_t intf_mode)
>> +{
>> +	struct phy_gmii_sel_phy_priv *if_phy = phy_get_drvdata(phy);
>> +	const struct phy_gmii_sel_soc_data *soc_data = if_phy->priv->soc_data;
>> +	struct device *dev = if_phy->priv->dev;
>> +	struct regmap_field *regfield;
>> +	int ret, rgmii_id = 0;
>> +	u32 mode = 0;
>> +
>> +	if_phy->phy_if_mode = intf_mode;
>> +
>> +	switch (if_phy->phy_if_mode) {
>> +	case PHY_INTERFACE_MODE_RMII:
>> +		mode = AM33XX_GMII_SEL_MODE_RMII;
>> +		break;
>> +
>> +	case PHY_INTERFACE_MODE_RGMII:
>> +		mode = AM33XX_GMII_SEL_MODE_RGMII;
>> +		break;
>> +
>> +	case PHY_INTERFACE_MODE_RGMII_ID:
>> +	case PHY_INTERFACE_MODE_RGMII_RXID:
>> +	case PHY_INTERFACE_MODE_RGMII_TXID:
>> +		mode = AM33XX_GMII_SEL_MODE_RGMII;
>> +		rgmii_id = 1;
>> +		break;
> 
> Hi Grygorii
> 
> It looks like the MAC can do AM33XX_GMII_SEL_MODE_RGMII and
> AM33XX_GMII_SEL_MODE_RGMII_ID. I don't think it can do
> AM33XX_GMII_SEL_MODE_RGMII_RXID or AM33XX_GMII_SEL_MODE_RGMII_TXID?

Sry, but would prefer not to thought this logic as part of this series as i moved it here
unchanged rom cpsw-phy-sel.c (except adding possibility to update only supported field)
and any changes here would require separate review (including all existing TI DT boards)
and testing.

  I
> would prefer it return -EINVAL when asked to do something it cannot
> do.

Just to clarify rgmii_id = 1 means *disable* CPSW Internal Delay Mode.
> 
>> +
>> +	default:
>> +		dev_warn(dev,
>> +			 "port%u: unsupported mode: \"%s\". Defaulting to MII.\n",
>> +			 if_phy->id, phy_modes(rgmii_id));
>> +		/* fall through */
> 
> Returning -EINVAL would be better. Otherwise the DT might never get
> fixed.

ok

> 
>> +	case PHY_INTERFACE_MODE_MII:
>> +		mode = AM33XX_GMII_SEL_MODE_MII;
>> +		break;
>> +	};
>> +
>> +	dev_dbg(dev, "%s id:%u mode:%u rgmii_id:%d rmii_clk_ext:%d\n",
>> +		__func__, if_phy->id, mode, rgmii_id,
>> +		if_phy->rmii_clock_external);
>> +
>> +	regfield = if_phy->fields[PHY_GMII_SEL_PORT_MODE];
>> +	ret = regmap_field_write(regfield, mode);
>> +
>> +	if (soc_data->features & BIT(PHY_GMII_SEL_RGMII_ID_MODE) &&
>> +	    if_phy->fields[PHY_GMII_SEL_RGMII_ID_MODE]) {
>> +		regfield = if_phy->fields[PHY_GMII_SEL_RGMII_ID_MODE];
>> +		ret |= regmap_field_write(regfield, rgmii_id);
>> +	}
>> +
>> +	if (soc_data->features & BIT(PHY_GMII_SEL_RMII_IO_CLK_EN) &&
>> +	    if_phy->fields[PHY_GMII_SEL_RMII_IO_CLK_EN]) {
>> +		regfield = if_phy->fields[PHY_GMII_SEL_RMII_IO_CLK_EN];
>> +		ret |= regmap_field_write(regfield,
>> +					  if_phy->rmii_clock_external);
>> +	}
>> +
>> +	if (ret) {
>> +		dev_err(dev, "port%u: set mode fail %d", if_phy->id, ret);
>> +		return -EIO;
>> +	}
> 
> I would prefer each write had its own error check. The fact you don't
> return ret means you know ret could be -EINVAL|-EOIO, making
> -EMORECOFFEE.

:) right, sry.

-- 
regards,
-grygorii

Re: [RFC PATCH 05/11] net: ethernet: ti: cpsw: add support for port interface mode selection phy

[Grygorii Strashko]

Hi Andrew,

On 10/08/2018 07:50 PM, Andrew Lunn wrote:
>>   	/* Configure GMII_SEL register */
>> -	cpsw_phy_sel(cpsw->dev, slave->phy->interface, slave->slave_num);
>> +	if (!IS_ERR(slave->data->ifphy))
>> +		phy_set_netif_mode(slave->data->ifphy, slave->data->phy_if);
> 
> Is slave->data->phy_if also passed to phy_connect()? So you are going
> to end up with both the MAC and the PHY inserting RGMII delays, and it
> not working.

No. This logic not changed comparing to how it was.

  * "rgmii" (RX and TX delays are added by the MAC when required)

rgmii_id = 0 --> CPSW: 0 : Internal Delay, PHY - no delay

  * "rgmii-id" (RGMII with internal RX and TX delays provided by the PHY, the
     MAC should not add the RX or TX delays in this case)
  * "rgmii-rxid" (RGMII with internal RX delay provided by the PHY, the MAC
     should not add an RX delay in this case)
  * "rgmii-txid" (RGMII with internal TX delay provided by the PHY, the MAC
     should not add an TX delay in this case)

rgmii_id = 1 --> CPSW: 1 : No Internal Delay, PHY/board -  delay

> 
> You need to somehow decide if the MAC is going to do the delay, or the
> PHY. But not both.

Again, this series does not change logic - only interfaces and DT.

Thank you for review.

-- 
regards,
-grygorii

Re: [RFC PATCH 02/11] dt-bindings: phy: add cpsw port interface mode selection phy bindings

[Tony Lindgren]

* Grygorii Strashko <grygorii.strashko@ti.com> [181009 20:10]:
> 
> 
> On 10/09/2018 09:40 AM, Tony Lindgren wrote:
> > * Grygorii Strashko <grygorii.strashko@ti.com> [181008 23:54]:
> >> +Examples:
> >> +	phy_gmii_sel: phy-gmii-sel {
> >> +		compatible = "ti,am3352-phy-gmii-sel";
> >> +		syscon-scm = <&scm_conf>;
> >> +		#phy-cells = <2>;
> >> +	};
> > 
> > Now that this driver can live in it's proper place in the
> 
> right
> 
> > dts, you may want to consider just using standard reg
> > property for it instead of the syscon-scm. And also get
> > rid of the syscon reads and writes.
> 
> Could you help clarify how to get syscon in this case?
> syscon_node_to_regmap(dev->parent->of_node)?

Hmm I don't think you need syscon at all now. You can just
ioremap the register(s) and use readl/writel and that's it.
Or use regmap without syscon if you prefer that.

The ioremap in this case should be hitting cached ranges
anyways, so no extra overhead there.

> Also, there are could be more then one gmii_sel registers in SCM in the future,
> so I hidden offsets in of_match data. 
> As result, "reg" not needed at all now.

But then you have to patch driver for various SoCs
instead of just configuring the standard reg property
in the dts file :)

Regards,

Tony

Re: [RFC PATCH 00/11] net: ethernet: ti: cpsw: replace cpsw-phy-sel with phy driver

[Grygorii Strashko]

On 10/09/2018 09:36 AM, Tony Lindgren wrote:
> * Grygorii Strashko <grygorii.strashko@ti.com> [181008 23:54]:
>> 2) introduce new PHY API for network interface mode selection which will use
>> already defined set of modes from phy_interface_t.
>>
>> Option 2 was selected for this series.
> 
> Looks good to me :) The dts files will cause merge conflicts with
> what I have pending for the ti-sysc changes so please send the dts
> changes in a separate series when posting without RFC.

expected. I did on top of mauster, but will rebase and resend if approved.

-- 
regards,
-grygorii

Re: [RFC PATCH 02/11] dt-bindings: phy: add cpsw port interface mode selection phy bindings

[Grygorii Strashko]

On 10/09/2018 03:30 PM, Tony Lindgren wrote:
> * Grygorii Strashko <grygorii.strashko@ti.com> [181009 20:10]:
>>
>>
>> On 10/09/2018 09:40 AM, Tony Lindgren wrote:
>>> * Grygorii Strashko <grygorii.strashko@ti.com> [181008 23:54]:
>>>> +Examples:
>>>> +	phy_gmii_sel: phy-gmii-sel {
>>>> +		compatible = "ti,am3352-phy-gmii-sel";
>>>> +		syscon-scm = <&scm_conf>;
>>>> +		#phy-cells = <2>;
>>>> +	};
>>>
>>> Now that this driver can live in it's proper place in the
>>
>> right
>>
>>> dts, you may want to consider just using standard reg
>>> property for it instead of the syscon-scm. And also get
>>> rid of the syscon reads and writes.
>>
>> Could you help clarify how to get syscon in this case?
>> syscon_node_to_regmap(dev->parent->of_node)?
> 
> Hmm I don't think you need syscon at all now. You can just
> ioremap the register(s) and use readl/writel and that's it.
> Or use regmap without syscon if you prefer that.

It will overlap with already remapped SCM syscon and i'd like to avoid this.
+ it seems common practice to use syscon for devices/drivers which are
child to SCM node - makes overall system more consistent.

> 
> The ioremap in this case should be hitting cached ranges
> anyways, so no extra overhead there.
> 
>> Also, there are could be more then one gmii_sel registers in SCM in the future,
>> so I hidden offsets in of_match data.
>> As result, "reg" not needed at all now.
> 
> But then you have to patch driver for various SoCs
> instead of just configuring the standard reg property
> in the dts file :)

Problem is that they are not guarantee to be standard between SoC's families 
(number of regs and fields placement), as result it might require to change
driver any way for various SoCs to handle properly new fields placement.

I prefer to fix driver then fight with DT ;) as it's static for SoC family
and need to be changed only once when new SoC family introduced.

-- 
regards,
-grygorii

Re: [RFC PATCH 02/11] dt-bindings: phy: add cpsw port interface mode selection phy bindings

[Tony Lindgren]

* Grygorii Strashko <grygorii.strashko@ti.com> [181009 22:04]:
> 
> 
> On 10/09/2018 03:30 PM, Tony Lindgren wrote:
> > * Grygorii Strashko <grygorii.strashko@ti.com> [181009 20:10]:
> >>
> >>
> >> On 10/09/2018 09:40 AM, Tony Lindgren wrote:
> >>> * Grygorii Strashko <grygorii.strashko@ti.com> [181008 23:54]:
> >>>> +Examples:
> >>>> +	phy_gmii_sel: phy-gmii-sel {
> >>>> +		compatible = "ti,am3352-phy-gmii-sel";
> >>>> +		syscon-scm = <&scm_conf>;
> >>>> +		#phy-cells = <2>;
> >>>> +	};
> >>>
> >>> Now that this driver can live in it's proper place in the
> >>
> >> right
> >>
> >>> dts, you may want to consider just using standard reg
> >>> property for it instead of the syscon-scm. And also get
> >>> rid of the syscon reads and writes.
> >>
> >> Could you help clarify how to get syscon in this case?
> >> syscon_node_to_regmap(dev->parent->of_node)?
> > 
> > Hmm I don't think you need syscon at all now. You can just
> > ioremap the register(s) and use readl/writel and that's it.
> > Or use regmap without syscon if you prefer that.
> 
> It will overlap with already remapped SCM syscon and i'd like to avoid this.
> + it seems common practice to use syscon for devices/drivers which are
> child to SCM node - makes overall system more consistent.

Well it was just set up with syscon in deperation earlier with
drivers just blindly mapping registers outside of their
range..

> > The ioremap in this case should be hitting cached ranges
> > anyways, so no extra overhead there.
> > 
> >> Also, there are could be more then one gmii_sel registers in SCM in the future,
> >> so I hidden offsets in of_match data.
> >> As result, "reg" not needed at all now.
> > 
> > But then you have to patch driver for various SoCs
> > instead of just configuring the standard reg property
> > in the dts file :)
> 
> Problem is that they are not guarantee to be standard between SoC's families 
> (number of regs and fields placement), as result it might require to change
> driver any way for various SoCs to handle properly new fields placement.
> 
> I prefer to fix driver then fight with DT ;) as it's static for SoC family
> and need to be changed only once when new SoC family introduced.

Fine with me, that can be changed later too no problem.

Regards,

Tony

Re: [RFC PATCH 01/11] phy: core add phy_set_netif_mode() api

[Grygorii Strashko]

On 10/09/2018 12:22 AM, Kishon Vijay Abraham I wrote:
> Hi Grygorii,
> 
> On Tuesday 09 October 2018 05:19 AM, Grygorii Strashko wrote:
>> Add new API phy_set_netif_mode(struct phy *phy, phy_interface_t mode) and
>> new PHY operation callback .set_netif_mode() which intended to be implemnte
>> by PHY drivers which supports Network interrfaces mode selection. Both
>> accepts phy_interface_t vlaue as input parameter.
>>
>> Cc: Kishon Vijay Abraham I <kishon@ti.com>
>> Cc: Tony Lindgren <tony@atomide.com>
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>> ---
>>   drivers/phy/phy-core.c  | 15 +++++++++++++++
>>   include/linux/phy/phy.h | 12 ++++++++++++
>>   2 files changed, 27 insertions(+)
>>
>> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
>> index 35fd38c..d9aba1a 100644
>> --- a/drivers/phy/phy-core.c
>> +++ b/drivers/phy/phy-core.c
>> @@ -377,6 +377,21 @@ int phy_set_mode(struct phy *phy, enum phy_mode mode)
>>   }
>>   EXPORT_SYMBOL_GPL(phy_set_mode);
>>   
>> +int phy_set_netif_mode(struct phy *phy, phy_interface_t mode)
>> +{
>> +	int ret;
>> +
>> +	if (!phy || !phy->ops->set_netif_mode)
>> +		return 0;
>> +
>> +	mutex_lock(&phy->mutex);
>> +	ret = phy->ops->set_netif_mode(phy, mode);
>> +	mutex_unlock(&phy->mutex);
>> +
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(phy_set_netif_mode);
> 
> We should try to add only generic PHY APIs and not subsystem specific APIs. In
> this case I think phy_set_mode should suffice.


This is what I've had in mind first, but all my guts argued against it after I've tried:

diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index bc73d2b..961b156 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -41,6 +41,14 @@ enum phy_mode {
        PHY_MODE_10GKR,
        PHY_MODE_UFS_HS_A,
        PHY_MODE_UFS_HS_B,
+       PHY_MODE_MODE_MII,
+       PHY_MODE_MODE_GMII,
+       PHY_MODE_MODE_SGMII,
+       PHY_MODE_MODE_RMII,
+       PHY_MODE_MODE_RGMII,
+       PHY_MODE_MODE_RGMII_ID,
+       PHY_MODE_MODE_RGMII_RXID,
+       PHY_MODE_MODE_RGMII_TXID,
 };

above introduces ugly constants duplication and required every network phy driver
to maintain conversation table  phy_interface_t -> enum phy_mode.
More over, if above change happens third time (first time PHY_MODE_SGMII/PHY_MODE_10GKR were added,
second - PHY_MODE_2500SGMII) it will never ends (there are ~15 more items only in phy_interface_t).
As result, enum phy_mode might became a un-maintainable monster.

So, as per above, and considering that Network subsystem is based on standards (phy_interface_t lists standard intf)
I've tried to add separate PHY API.

As an idea:
- seems it could be reasonable to introduce PHY_MODE_NETWORK (or PHY_MODE_ETHERNET) and
add generic phy_set_submode(struct phy *phy, long submode). 

So, single functional PHY device can just use phy_set_submode() and
multi-functional devices (like serdes which can be muxed between PCIe, USB, NET), can use:

phy_set_mode(PHY_MODE_ETHERNET)
phy_set_submode(X);

Any way, if you still agree to just add new above phy_modes - i'll redo patches.

-- 
regards,
-grygorii

Re: [RFC PATCH 02/11] dt-bindings: phy: add cpsw port interface mode selection phy bindings

[Rob Herring]

On Tue, Oct 09, 2018 at 03:10:34PM -0500, Grygorii Strashko wrote:
> 
> 
> On 10/09/2018 09:40 AM, Tony Lindgren wrote:
> > * Grygorii Strashko <grygorii.strashko@ti.com> [181008 23:54]:
> >> +Examples:
> >> +	phy_gmii_sel: phy-gmii-sel {
> >> +		compatible = "ti,am3352-phy-gmii-sel";
> >> +		syscon-scm = <&scm_conf>;
> >> +		#phy-cells = <2>;
> >> +	};
> > 
> > Now that this driver can live in it's proper place in the
> 
> right
> 
> > dts, you may want to consider just using standard reg
> > property for it instead of the syscon-scm. And also get
> > rid of the syscon reads and writes.
> 
> Could you help clarify how to get syscon in this case?
> syscon_node_to_regmap(dev->parent->of_node)?
> 
> Also, there are could be more then one gmii_sel registers in SCM in the future,
> so I hidden offsets in of_match data. 
> As result, "reg" not needed at all now.

If there's a defined register range which doesn't overlap with other 
things (other than the parent), then use reg whether you currently need 
it or not.

Rob

Re: [RFC PATCH 04/11] dt-bindings: net: ti: cpsw: switch to use phy-gmii-sel phy

[Rob Herring]

On Mon, Oct 08, 2018 at 06:49:42PM -0500, Grygorii Strashko wrote:
> The cpsw-phy-sel driver was replaced with new PHY driver phy-gmii-sel, so
> deprecate cpsw-phy-sel bindings and update CPSW binding to use phy-gmii-sel
> PHY bindings.
> 
> Cc: Kishon Vijay Abraham I <kishon@ti.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
>  Documentation/devicetree/bindings/net/cpsw.txt | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
> index b3acebe..69d7ef8 100644
> --- a/Documentation/devicetree/bindings/net/cpsw.txt
> +++ b/Documentation/devicetree/bindings/net/cpsw.txt
> @@ -22,7 +22,8 @@ Required properties:
>  - cpsw-phy-sel		: Specifies the phandle to the CPSW phy mode selection
>  			  device. See also cpsw-phy-sel.txt for it's binding.
>  			  Note that in legacy cases cpsw-phy-sel may be
> -			  a child device instead of a phandle.
> +			  a child device instead of a phandle
> +			  (DEPRECATED, use phy-gmii-sel PHY phandle).

phy-gmii-sel is outside the scope of this binding. Just say use 'phys' 
property instead.

>  
>  Optional properties:
>  - ti,hwmods		: Must be "cpgmac0"
> @@ -44,6 +45,7 @@ Optional properties:
>  Slave Properties:
>  Required properties:
>  - phy-mode		: See ethernet.txt file in the same directory
> +- phys			: phandle on phy-gmii-sel PHY (see phy/ti-phy-gmii-sel.txt)
>  
>  Optional properties:
>  - dual_emac_res_vlan	: Specifies VID to be used to segregate the ports
> @@ -85,12 +87,14 @@ Examples:
>  			phy-mode = "rgmii-txid";
>  			/* Filled in by U-Boot */
>  			mac-address = [ 00 00 00 00 00 00 ];
> +			phys = <&phy_gmii_sel 1 0>;
>  		};
>  		cpsw_emac1: slave@1 {
>  			phy_id = <&davinci_mdio>, <1>;
>  			phy-mode = "rgmii-txid";
>  			/* Filled in by U-Boot */
>  			mac-address = [ 00 00 00 00 00 00 ];
> +			phys = <&phy_gmii_sel 2 0>;
>  		};
>  	};
>  
> @@ -114,11 +118,13 @@ Examples:
>  			phy-mode = "rgmii-txid";
>  			/* Filled in by U-Boot */
>  			mac-address = [ 00 00 00 00 00 00 ];
> +			phys = <&phy_gmii_sel 1 0>;
>  		};
>  		cpsw_emac1: slave@1 {
>  			phy_id = <&davinci_mdio>, <1>;
>  			phy-mode = "rgmii-txid";
>  			/* Filled in by U-Boot */
>  			mac-address = [ 00 00 00 00 00 00 ];
> +			phys = <&phy_gmii_sel 2 0>;
>  		};
>  	};
> -- 
> 2.10.5
> 

Re: [RFC PATCH 10/11] dt-bindings: net: ti: deprecate cpsw-phy-sel bindings

[Rob Herring]

On Mon, 8 Oct 2018 18:49:48 -0500, Grygorii Strashko wrote:
> The cpsw-phy-sel driver was replaced with new PHY driver phy-gmii-sel, so
> deprecate cpsw-phy-sel bindings.
> 
> Cc: Kishon Vijay Abraham I <kishon@ti.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
>  Documentation/devicetree/bindings/net/cpsw-phy-sel.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

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

Re: [RFC PATCH 01/11] phy: core add phy_set_netif_mode() api

[Kishon Vijay Abraham I]

Hi,

On Wednesday 10 October 2018 04:13 AM, Grygorii Strashko wrote:
> 
> 
> On 10/09/2018 12:22 AM, Kishon Vijay Abraham I wrote:
>> Hi Grygorii,
>>
>> On Tuesday 09 October 2018 05:19 AM, Grygorii Strashko wrote:
>>> Add new API phy_set_netif_mode(struct phy *phy, phy_interface_t mode) and
>>> new PHY operation callback .set_netif_mode() which intended to be implemnte
>>> by PHY drivers which supports Network interrfaces mode selection. Both
>>> accepts phy_interface_t vlaue as input parameter.
>>>
>>> Cc: Kishon Vijay Abraham I <kishon@ti.com>
>>> Cc: Tony Lindgren <tony@atomide.com>
>>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>>> ---
>>>   drivers/phy/phy-core.c  | 15 +++++++++++++++
>>>   include/linux/phy/phy.h | 12 ++++++++++++
>>>   2 files changed, 27 insertions(+)
>>>
>>> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
>>> index 35fd38c..d9aba1a 100644
>>> --- a/drivers/phy/phy-core.c
>>> +++ b/drivers/phy/phy-core.c
>>> @@ -377,6 +377,21 @@ int phy_set_mode(struct phy *phy, enum phy_mode mode)
>>>   }
>>>   EXPORT_SYMBOL_GPL(phy_set_mode);
>>>   
>>> +int phy_set_netif_mode(struct phy *phy, phy_interface_t mode)
>>> +{
>>> +	int ret;
>>> +
>>> +	if (!phy || !phy->ops->set_netif_mode)
>>> +		return 0;
>>> +
>>> +	mutex_lock(&phy->mutex);
>>> +	ret = phy->ops->set_netif_mode(phy, mode);
>>> +	mutex_unlock(&phy->mutex);
>>> +
>>> +	return ret;
>>> +}
>>> +EXPORT_SYMBOL_GPL(phy_set_netif_mode);
>>
>> We should try to add only generic PHY APIs and not subsystem specific APIs. In
>> this case I think phy_set_mode should suffice.
> 
> 
> This is what I've had in mind first, but all my guts argued against it after I've tried:
> 
> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> index bc73d2b..961b156 100644
> --- a/include/linux/phy/phy.h
> +++ b/include/linux/phy/phy.h
> @@ -41,6 +41,14 @@ enum phy_mode {
>         PHY_MODE_10GKR,
>         PHY_MODE_UFS_HS_A,
>         PHY_MODE_UFS_HS_B,
> +       PHY_MODE_MODE_MII,
> +       PHY_MODE_MODE_GMII,
> +       PHY_MODE_MODE_SGMII,
> +       PHY_MODE_MODE_RMII,
> +       PHY_MODE_MODE_RGMII,
> +       PHY_MODE_MODE_RGMII_ID,
> +       PHY_MODE_MODE_RGMII_RXID,
> +       PHY_MODE_MODE_RGMII_TXID,
>  };
> 
> above introduces ugly constants duplication and required every network phy driver
> to maintain conversation table  phy_interface_t -> enum phy_mode.
> More over, if above change happens third time (first time PHY_MODE_SGMII/PHY_MODE_10GKR were added,
> second - PHY_MODE_2500SGMII) it will never ends (there are ~15 more items only in phy_interface_t).
> As result, enum phy_mode might became a un-maintainable monster.
> 
> So, as per above, and considering that Network subsystem is based on standards (phy_interface_t lists standard intf)
> I've tried to add separate PHY API.
> 
> As an idea:
> - seems it could be reasonable to introduce PHY_MODE_NETWORK (or PHY_MODE_ETHERNET) and
> add generic phy_set_submode(struct phy *phy, long submode). 
> 
> So, single functional PHY device can just use phy_set_submode() and
> multi-functional devices (like serdes which can be muxed between PCIe, USB, NET), can use:
> 
> phy_set_mode(PHY_MODE_ETHERNET)
> phy_set_submode(X);

Agreed on the constant duplication comment above. We can modify set_mode to
take submode as an additional parameter and fix all the users of phy_set_mode.
int phy_set_mode(struct phy *phy, enum phy_mode mode, int submode)

Thanks
Kishon