2017-07-27 12:51:44

by David Wu

[permalink] [raw]
Subject: [PATCH v2 00/11] Add the internal phy support

The rk3228 and rk3328 support internal phy inside, let's enable
it to work. And the internal phy need to do some special setting, so
register the rockchip internal phy driver.

David Wu (11):
net: phy: Add rockchip phy driver support
multi_v7_defconfig: Make rockchip phy built-in
arm64: defconfig: Enable CONFIG_ROCKCHIP_PHY
net: stmmac: dwmac-rk: Remove unwanted code for rk3328_set_to_rmii()
net: stmmac: dwmac-rk: Add internal phy support
net: stmmac: dwmac-rk: Add internal phy support for rk3228
net: stmmac: dwmac-rk: Add internal phy supprot for rk3328
ARM: dts: rk322x: Add support internal phy for gmac
ARM: dts: rk3228-evb: Enable the internal phy for gmac
ARM64: dts: rockchip: Add gmac2phy node support for rk3328
ARM64: dts: rockchip: Enable gmac2phy for rk3328-evb

.../devicetree/bindings/net/rockchip-dwmac.txt | 4 +-
arch/arm/boot/dts/rk3228-evb.dts | 19 +++
arch/arm/boot/dts/rk322x.dtsi | 8 +-
arch/arm/configs/multi_v7_defconfig | 1 +
arch/arm64/boot/dts/rockchip/rk3328-evb.dts | 17 +++
arch/arm64/boot/dts/rockchip/rk3328.dtsi | 24 ++++
arch/arm64/configs/defconfig | 1 +
drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 135 +++++++++++++++++++--
drivers/net/phy/Kconfig | 5 +
drivers/net/phy/Makefile | 1 +
drivers/net/phy/rockchip.c | 128 +++++++++++++++++++
11 files changed, 328 insertions(+), 15 deletions(-)
create mode 100644 drivers/net/phy/rockchip.c

--
1.9.1



2017-07-27 12:51:54

by David Wu

[permalink] [raw]
Subject: [PATCH v2 04/11] net: stmmac: dwmac-rk: Remove unwanted code for rk3328_set_to_rmii()

This is wrong setting for rk3328_set_to_rmii(), so remove it.

Signed-off-by: David Wu <[email protected]>
---
drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 3 ---
1 file changed, 3 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
index f0df519..a8e8fd5 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
@@ -365,9 +365,6 @@ static void rk3328_set_to_rmii(struct rk_priv_data *bsp_priv)
regmap_write(bsp_priv->grf, RK3328_GRF_MAC_CON1,
RK3328_GMAC_PHY_INTF_SEL_RMII |
RK3328_GMAC_RMII_MODE);
-
- /* set MAC to RMII mode */
- regmap_write(bsp_priv->grf, RK3328_GRF_MAC_CON1, GRF_BIT(11));
}

static void rk3328_set_rgmii_speed(struct rk_priv_data *bsp_priv, int speed)
--
1.9.1


2017-07-27 12:51:52

by David Wu

[permalink] [raw]
Subject: [PATCH v2 03/11] arm64: defconfig: Enable CONFIG_ROCKCHIP_PHY

Make the rockchip phy driver built into the kernel.

Signed-off-by: David Wu <[email protected]>
---
arch/arm64/configs/defconfig | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 6c7d147..925bd478 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -203,6 +203,7 @@ CONFIG_MARVELL_PHY=m
CONFIG_MESON_GXL_PHY=m
CONFIG_MICREL_PHY=y
CONFIG_REALTEK_PHY=m
+CONFIG_ROCKCHIP_PHY=y
CONFIG_USB_PEGASUS=m
CONFIG_USB_RTL8150=m
CONFIG_USB_RTL8152=m
--
1.9.1


2017-07-27 12:51:51

by David Wu

[permalink] [raw]
Subject: [PATCH v2 02/11] multi_v7_defconfig: Make rockchip phy built-in

Enable the rockchip phy for multi_v7_defconfig builds.

Signed-off-by: David Wu <[email protected]>
---
arch/arm/configs/multi_v7_defconfig | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index 4d19c1b..94d7e71 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -270,6 +270,7 @@ CONFIG_ICPLUS_PHY=y
CONFIG_REALTEK_PHY=y
CONFIG_MICREL_PHY=y
CONFIG_FIXED_PHY=y
+CONFIG_ROCKCHIP_PHY=y
CONFIG_USB_PEGASUS=y
CONFIG_USB_RTL8152=m
CONFIG_USB_USBNET=y
--
1.9.1


2017-07-27 12:53:15

by David Wu

[permalink] [raw]
Subject: [PATCH v2 01/11] net: phy: Add rockchip phy driver support

Support internal ephy currently.

Signed-off-by: David Wu <[email protected]>
---
changes in v2:
- Alphabetic order for Kconfig and Makefile.
- Add analog register init.
- Disable auto-mdix for workround.
- Rename config

drivers/net/phy/Kconfig | 5 ++
drivers/net/phy/Makefile | 1 +
drivers/net/phy/rockchip.c | 128 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 134 insertions(+)
create mode 100644 drivers/net/phy/rockchip.c

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 2dda720..8dc6cd7 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -334,6 +334,11 @@ config REALTEK_PHY
---help---
Supports the Realtek 821x PHY.

+config ROCKCHIP_PHY
+ tristate "Drivers for ROCKCHIP PHYs"
+ ---help---
+ Currently supports the internal ephy.
+
config SMSC_PHY
tristate "SMSC PHYs"
---help---
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 8e9b9f3..350520e 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -66,6 +66,7 @@ obj-$(CONFIG_MICROSEMI_PHY) += mscc.o
obj-$(CONFIG_NATIONAL_PHY) += national.o
obj-$(CONFIG_QSEMI_PHY) += qsemi.o
obj-$(CONFIG_REALTEK_PHY) += realtek.o
+obj-$(CONFIG_ROCKCHIP_PHY) += rockchip.o
obj-$(CONFIG_SMSC_PHY) += smsc.o
obj-$(CONFIG_STE10XP) += ste10Xp.o
obj-$(CONFIG_TERANETICS_PHY) += teranetics.o
diff --git a/drivers/net/phy/rockchip.c b/drivers/net/phy/rockchip.c
new file mode 100644
index 0000000..3f74658
--- /dev/null
+++ b/drivers/net/phy/rockchip.c
@@ -0,0 +1,128 @@
+/**
+ * drivers/net/phy/rockchip.c
+ *
+ * Driver for ROCKCHIP PHY
+ *
+ * Copyright (c) 2017, Fuzhou Rockchip Electronics Co., Ltd
+ *
+ * David Wu<[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mii.h>
+#include <linux/ethtool.h>
+#include <linux/phy.h>
+#include <linux/netdevice.h>
+
+#define MII_INTERNAL_CTRL_STATUS 17
+#define SMI_ADDR_TSTCNTL 20
+#define SMI_ADDR_TSTREAD1 21
+#define SMI_ADDR_TSTREAD2 22
+#define SMI_ADDR_TSTWRITE 23
+
+#define AUTOMDIX_EN BIT(7)
+#define TSTCNTL_RD (BIT(15) | BIT(10))
+#define TSTCNTL_WR (BIT(14) | BIT(10))
+
+#define WR_ADDR_A7CFG 0x18
+
+static void rockchip_init_tstmode(struct phy_device *phydev)
+{
+ /* Enable access to Analog and DSP register banks */
+ phy_write(phydev, SMI_ADDR_TSTCNTL, 0x0400);
+ phy_write(phydev, SMI_ADDR_TSTCNTL, 0x0000);
+ phy_write(phydev, SMI_ADDR_TSTCNTL, 0x0400);
+}
+
+static void rockchip_close_tstmode(struct phy_device *phydev)
+{
+ /* Back to basic register bank */
+ phy_write(phydev, SMI_ADDR_TSTCNTL, 0x0000);
+}
+
+static void rockchip_internal_phy_analog_init(struct phy_device *phydev)
+{
+ rockchip_init_tstmode(phydev);
+
+ /*
+ * Adjust tx amplitude to make sginal better,
+ * the default value is 0x8.
+ */
+ phy_write(phydev, SMI_ADDR_TSTWRITE, 0xB);
+ phy_write(phydev, SMI_ADDR_TSTCNTL, TSTCNTL_WR | WR_ADDR_A7CFG);
+
+ rockchip_close_tstmode(phydev);
+}
+
+static int rockchip_internal_phy_config_init(struct phy_device *phydev)
+{
+ int val;
+
+ /*
+ * The auto MIDX has linked problem on some board,
+ * workround to disable auto MDIX.
+ */
+ val = phy_read(phydev, MII_INTERNAL_CTRL_STATUS);
+ val &= ~AUTOMDIX_EN;
+ phy_write(phydev, MII_INTERNAL_CTRL_STATUS, val);
+
+ rockchip_internal_phy_analog_init(phydev);
+
+ return 0;
+}
+
+static int rockchip_internal_phy_read_status(struct phy_device *phydev)
+{
+ int ret, old_speed;
+
+ old_speed = phydev->speed;
+ ret = genphy_read_status(phydev);
+ if (ret)
+ return ret;
+
+ /*
+ * If mode switch happens from 10BT to 100BT, all DSP/AFE
+ * registers are set to default values. So any AFE/DSP
+ * registers have to be re-initialized in this case.
+ */
+ if ((old_speed == SPEED_10) && (phydev->speed == SPEED_100))
+ rockchip_internal_phy_analog_init(phydev);
+
+ return ret;
+}
+
+static struct phy_driver rockchip_phy_driver[] = {
+{
+ .phy_id = 0x1234d400,
+ .phy_id_mask = 0xffff0000,
+ .name = "rockchip internal ephy",
+ .features = (PHY_BASIC_FEATURES | SUPPORTED_Pause
+ | SUPPORTED_Asym_Pause),
+ .soft_reset = genphy_soft_reset,
+ .config_init = rockchip_internal_phy_config_init,
+ .config_aneg = genphy_config_aneg,
+ .read_status = rockchip_internal_phy_read_status,
+ .suspend = genphy_suspend,
+ .resume = genphy_resume,
+},
+};
+
+module_phy_driver(rockchip_phy_driver);
+
+static struct mdio_device_id __maybe_unused rockchip_phy_tbl[] = {
+ { 0x1234d400, 0xffff0000 },
+ { }
+};
+
+MODULE_DEVICE_TABLE(mdio, rockchip_phy_tbl);
+
+MODULE_AUTHOR("David Wu<[email protected]>");
+MODULE_DESCRIPTION("Rockchip phy driver");
+MODULE_LICENSE("GPL v2");
--
1.9.1


2017-07-27 12:58:02

by David Wu

[permalink] [raw]
Subject: [PATCH v2 06/11] net: stmmac: dwmac-rk: Add internal phy support for rk3228

There is only one mac controller in rk3228, which could connect to
external phy or internal phy, use the grf_com_mux bit15 to route
external/internal phy.

Signed-off-by: David Wu <[email protected]>
---
drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
index ec280d1..3ec9cd8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
@@ -86,6 +86,8 @@ struct rk_priv_data {
#define RK3228_GRF_MAC_CON0 0x0900
#define RK3228_GRF_MAC_CON1 0x0904

+#define RK3228_GRF_CON_MUX 0x50
+
/* RK3228_GRF_MAC_CON0 */
#define RK3228_GMAC_CLK_RX_DL_CFG(val) HIWORD_UPDATE(val, 0x7F, 7)
#define RK3228_GMAC_CLK_TX_DL_CFG(val) HIWORD_UPDATE(val, 0x7F, 0)
@@ -111,6 +113,9 @@ struct rk_priv_data {
#define RK3228_GMAC_RXCLK_DLY_ENABLE GRF_BIT(1)
#define RK3228_GMAC_RXCLK_DLY_DISABLE GRF_CLR_BIT(1)

+/* RK3228_GRF_COM_MUX */
+#define RK3228_GRF_CON_MUX_GMAC_INTERNAL_PHY GRF_BIT(15)
+
static void rk3228_set_to_rgmii(struct rk_priv_data *bsp_priv,
int tx_delay, int rx_delay)
{
@@ -191,11 +196,18 @@ static void rk3228_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
dev_err(dev, "unknown speed value for RMII! speed=%d", speed);
}

+static void rk3228_internal_phy_powerup(struct rk_priv_data *priv)
+{
+ regmap_write(priv->grf, RK3228_GRF_CON_MUX,
+ RK3228_GRF_CON_MUX_GMAC_INTERNAL_PHY);
+}
+
static const struct rk_gmac_ops rk3228_ops = {
.set_to_rgmii = rk3228_set_to_rgmii,
.set_to_rmii = rk3228_set_to_rmii,
.set_rgmii_speed = rk3228_set_rgmii_speed,
.set_rmii_speed = rk3228_set_rmii_speed,
+ .internal_phy_powerup = rk3228_internal_phy_powerup,
};

#define RK3288_GRF_SOC_CON1 0x0248
--
1.9.1


2017-07-27 12:58:10

by David Wu

[permalink] [raw]
Subject: [PATCH v2 07/11] net: stmmac: dwmac-rk: Add internal phy supprot for rk3328

There are two mac controllers in the rk3328, the one connects
to external phy, and the other one connects to internal phy.
Like the mac of external phy, the internal phy's mac also needs to
configure the related mac registers at GRF.

Signed-off-by: David Wu <[email protected]>
---
drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 26 +++++++++++++++++++++++---
1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
index 3ec9cd8..67066a3 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
@@ -323,6 +323,8 @@ static void rk3288_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)

#define RK3328_GRF_MAC_CON0 0x0900
#define RK3328_GRF_MAC_CON1 0x0904
+#define RK3328_GRF_MAC_CON2 0x0908
+#define RK3328_GRF_MACPHY_CON1 0xb04

/* RK3328_GRF_MAC_CON0 */
#define RK3328_GMAC_CLK_RX_DL_CFG(val) HIWORD_UPDATE(val, 0x7F, 7)
@@ -349,6 +351,9 @@ static void rk3288_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
#define RK3328_GMAC_RXCLK_DLY_ENABLE GRF_BIT(1)
#define RK3328_GMAC_RXCLK_DLY_DISABLE GRF_CLR_BIT(0)

+/* RK3328_GRF_MACPHY_CON1 */
+#define RK3328_MACPHY_RMII_MODE GRF_BIT(9)
+
static void rk3328_set_to_rgmii(struct rk_priv_data *bsp_priv,
int tx_delay, int rx_delay)
{
@@ -373,13 +378,17 @@ static void rk3328_set_to_rgmii(struct rk_priv_data *bsp_priv,
static void rk3328_set_to_rmii(struct rk_priv_data *bsp_priv)
{
struct device *dev = &bsp_priv->pdev->dev;
+ unsigned int reg;

if (IS_ERR(bsp_priv->grf)) {
dev_err(dev, "Missing rockchip,grf property\n");
return;
}

- regmap_write(bsp_priv->grf, RK3328_GRF_MAC_CON1,
+ reg = bsp_priv->internal_phy ? RK3328_GRF_MAC_CON2 :
+ RK3328_GRF_MAC_CON1;
+
+ regmap_write(bsp_priv->grf, reg,
RK3328_GMAC_PHY_INTF_SEL_RMII |
RK3328_GMAC_RMII_MODE);
}
@@ -409,29 +418,40 @@ static void rk3328_set_rgmii_speed(struct rk_priv_data *bsp_priv, int speed)
static void rk3328_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
{
struct device *dev = &bsp_priv->pdev->dev;
+ unsigned int reg;

if (IS_ERR(bsp_priv->grf)) {
dev_err(dev, "Missing rockchip,grf property\n");
return;
}

+ reg = bsp_priv->internal_phy ? RK3328_GRF_MAC_CON2 :
+ RK3328_GRF_MAC_CON1;
+
if (speed == 10)
- regmap_write(bsp_priv->grf, RK3328_GRF_MAC_CON1,
+ regmap_write(bsp_priv->grf, reg,
RK3328_GMAC_RMII_CLK_2_5M |
RK3328_GMAC_SPEED_10M);
else if (speed == 100)
- regmap_write(bsp_priv->grf, RK3328_GRF_MAC_CON1,
+ regmap_write(bsp_priv->grf, reg,
RK3328_GMAC_RMII_CLK_25M |
RK3328_GMAC_SPEED_100M);
else
dev_err(dev, "unknown speed value for RMII! speed=%d", speed);
}

+static void rk3328_internal_phy_powerup(struct rk_priv_data *priv)
+{
+ regmap_write(priv->grf, RK3328_GRF_MACPHY_CON1,
+ RK3328_MACPHY_RMII_MODE);
+}
+
static const struct rk_gmac_ops rk3328_ops = {
.set_to_rgmii = rk3328_set_to_rgmii,
.set_to_rmii = rk3328_set_to_rmii,
.set_rgmii_speed = rk3328_set_rgmii_speed,
.set_rmii_speed = rk3328_set_rmii_speed,
+ .internal_phy_powerup = rk3328_internal_phy_powerup,
};

#define RK3366_GRF_SOC_CON6 0x0418
--
1.9.1


2017-07-27 12:58:20

by David Wu

[permalink] [raw]
Subject: [PATCH v2 08/11] ARM: dts: rk322x: Add support internal phy for gmac

This patch adds internal mac phy clock and internal mac phy reset
for rk gmac using.

Signed-off-by: David Wu <[email protected]>
---
arch/arm/boot/dts/rk322x.dtsi | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/rk322x.dtsi b/arch/arm/boot/dts/rk322x.dtsi
index f3e4ffd..3778f7d 100644
--- a/arch/arm/boot/dts/rk322x.dtsi
+++ b/arch/arm/boot/dts/rk322x.dtsi
@@ -611,13 +611,13 @@
clocks = <&cru SCLK_MAC>, <&cru SCLK_MAC_RX>,
<&cru SCLK_MAC_TX>, <&cru SCLK_MAC_REF>,
<&cru SCLK_MAC_REFOUT>, <&cru ACLK_GMAC>,
- <&cru PCLK_GMAC>;
+ <&cru PCLK_GMAC>, <&cru SCLK_MAC_PHY>;
clock-names = "stmmaceth", "mac_clk_rx",
"mac_clk_tx", "clk_mac_ref",
"clk_mac_refout", "aclk_mac",
- "pclk_mac";
- resets = <&cru SRST_GMAC>;
- reset-names = "stmmaceth";
+ "pclk_mac", "clk_macphy";
+ resets = <&cru SRST_GMAC>, <&cru SRST_MACPHY>;
+ reset-names = "stmmaceth", "mac-phy";
rockchip,grf = <&grf>;
status = "disabled";
};
--
1.9.1


2017-07-27 12:59:18

by David Wu

[permalink] [raw]
Subject: [PATCH v2 09/11] ARM: dts: rk3228-evb: Enable the internal phy for gmac

This patch enables the internal phy for rk3228 evb board
by default.
To use the external 1000M phy on evb board, need to make
some switch of evb board to be on.

Signed-off-by: David Wu <[email protected]>
---
arch/arm/boot/dts/rk3228-evb.dts | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)

diff --git a/arch/arm/boot/dts/rk3228-evb.dts b/arch/arm/boot/dts/rk3228-evb.dts
index 5883433..73b88d3 100644
--- a/arch/arm/boot/dts/rk3228-evb.dts
+++ b/arch/arm/boot/dts/rk3228-evb.dts
@@ -50,6 +50,16 @@
device_type = "memory";
reg = <0x60000000 0x40000000>;
};
+
+ vcc_phy: vcc-phy-regulator {
+ compatible = "regulator-fixed";
+ enable-active-high;
+ regulator-name = "vcc_phy";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-always-on;
+ regulator-boot-on;
+ };
};

&emmc {
@@ -60,6 +70,15 @@
status = "okay";
};

+&gmac {
+ assigned-clocks = <&cru SCLK_MAC_SRC>;
+ assigned-clock-rates = <50000000>;
+ clock_in_out = "output";
+ phy-supply = <&vcc_phy>;
+ phy-mode = "internal";
+ status = "okay";
+};
+
&tsadc {
status = "okay";

--
1.9.1


2017-07-27 12:57:59

by David Wu

[permalink] [raw]
Subject: [PATCH v2 05/11] net: stmmac: dwmac-rk: Add internal phy support

To make internal phy work, need to configure the phy_clock,
phy cru_reset and related registers.

Signed-off-by: David Wu <[email protected]>
---
changes in v2:
- Use the standard "phy-mode" property for internal phy. (Florian)
- Move the internal macphy clock to the optional properties. (Heiko)

.../devicetree/bindings/net/rockchip-dwmac.txt | 4 +-
drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 94 +++++++++++++++++++++-
2 files changed, 93 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/rockchip-dwmac.txt b/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
index 8f42755..ecebab8 100644
--- a/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
+++ b/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
@@ -25,7 +25,8 @@ Required properties:
- clock-names: One name for each entry in the clocks property.
- phy-mode: See ethernet.txt file in the same directory.
- pinctrl-names: Names corresponding to the numbered pinctrl states.
- - pinctrl-0: pin-control mode. can be <&rgmii_pins> or <&rmii_pins>.
+ - pinctrl-0: pin-control mode. can be <&rgmii_pins>, <&rmii_pins> or led pins
+ for internal phy mode.
- clock_in_out: For RGMII, it must be "input", means main clock(125MHz)
is not sourced from SoC's PLL, but input from PHY; For RMII, "input" means
PHY provides the reference clock(50MHz), "output" means GMAC provides the
@@ -40,6 +41,7 @@ Optional properties:
- tx_delay: Delay value for TXD timing. Range value is 0~0x7F, 0x30 as default.
- rx_delay: Delay value for RXD timing. Range value is 0~0x7F, 0x10 as default.
- phy-supply: phandle to a regulator if the PHY needs one
+ - clocks: <&cru MAC_PHY>: clock for internal macphy

Example:

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
index a8e8fd5..ec280d1 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
@@ -41,6 +41,7 @@ struct rk_gmac_ops {
void (*set_to_rmii)(struct rk_priv_data *bsp_priv);
void (*set_rgmii_speed)(struct rk_priv_data *bsp_priv, int speed);
void (*set_rmii_speed)(struct rk_priv_data *bsp_priv, int speed);
+ void (*internal_phy_powerup)(struct rk_priv_data *bsp_priv);
};

struct rk_priv_data {
@@ -52,6 +53,7 @@ struct rk_priv_data {

bool clk_enabled;
bool clock_input;
+ bool internal_phy;

struct clk *clk_mac;
struct clk *gmac_clkin;
@@ -61,6 +63,9 @@ struct rk_priv_data {
struct clk *clk_mac_refout;
struct clk *aclk_mac;
struct clk *pclk_mac;
+ struct clk *clk_macphy;
+
+ struct reset_control *macphy_reset;

int tx_delay;
int rx_delay;
@@ -750,6 +755,50 @@ static void rk3399_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
.set_rmii_speed = rk3399_set_rmii_speed,
};

+#define RK_GRF_MACPHY_CON0 0xb00
+#define RK_GRF_MACPHY_CON1 0xb04
+#define RK_GRF_MACPHY_CON2 0xb08
+#define RK_GRF_MACPHY_CON3 0xb0c
+
+#define RK_MACPHY_ENABLE GRF_BIT(0)
+#define RK_MACPHY_DISABLE GRF_CLR_BIT(0)
+#define RK_MACPHY_CFG_CLK_50M GRF_BIT(14)
+#define RK_GMAC2PHY_RMII_MODE (GRF_BIT(6) | GRF_CLR_BIT(7))
+#define RK_GRF_CON2_MACPHY_ID HIWORD_UPDATE(0x1234, 0xffff, 0)
+#define RK_GRF_CON3_MACPHY_ID HIWORD_UPDATE(0x35, 0x3f, 0)
+
+static void rk_gmac_internal_phy_powerup(struct rk_priv_data *priv)
+{
+ if (priv->ops->internal_phy_powerup)
+ priv->ops->internal_phy_powerup(priv);
+
+ regmap_write(priv->grf, RK_GRF_MACPHY_CON0, RK_MACPHY_CFG_CLK_50M);
+ regmap_write(priv->grf, RK_GRF_MACPHY_CON0, RK_GMAC2PHY_RMII_MODE);
+
+ regmap_write(priv->grf, RK_GRF_MACPHY_CON2, RK_GRF_CON2_MACPHY_ID);
+ regmap_write(priv->grf, RK_GRF_MACPHY_CON3, RK_GRF_CON3_MACPHY_ID);
+
+ if (priv->macphy_reset) {
+ /* macphy needs to be disabled before trying to reset it */
+ regmap_write(priv->grf, RK_GRF_MACPHY_CON0, RK_MACPHY_DISABLE);
+ if (priv->macphy_reset)
+ reset_control_assert(priv->macphy_reset);
+ usleep_range(10, 20);
+ if (priv->macphy_reset)
+ reset_control_deassert(priv->macphy_reset);
+ usleep_range(10, 20);
+ regmap_write(priv->grf, RK_GRF_MACPHY_CON0, RK_MACPHY_ENABLE);
+ msleep(30);
+ }
+}
+
+static void rk_gmac_internal_phy_powerdown(struct rk_priv_data *priv)
+{
+ regmap_write(priv->grf, RK_GRF_MACPHY_CON0, RK_MACPHY_DISABLE);
+ if (priv->macphy_reset)
+ reset_control_assert(priv->macphy_reset);
+}
+
static int gmac_clk_init(struct rk_priv_data *bsp_priv)
{
struct device *dev = &bsp_priv->pdev->dev;
@@ -781,7 +830,8 @@ static int gmac_clk_init(struct rk_priv_data *bsp_priv)
dev_err(dev, "cannot get clock %s\n",
"stmmaceth");

- if (bsp_priv->phy_iface == PHY_INTERFACE_MODE_RMII) {
+ if (bsp_priv->phy_iface == PHY_INTERFACE_MODE_RMII ||
+ bsp_priv->phy_iface == PHY_INTERFACE_MODE_INTERNAL) {
bsp_priv->clk_mac_ref = devm_clk_get(dev, "clk_mac_ref");
if (IS_ERR(bsp_priv->clk_mac_ref))
dev_err(dev, "cannot get clock %s\n",
@@ -799,10 +849,19 @@ static int gmac_clk_init(struct rk_priv_data *bsp_priv)
if (bsp_priv->clock_input) {
dev_info(dev, "clock input from PHY\n");
} else {
- if (bsp_priv->phy_iface == PHY_INTERFACE_MODE_RMII)
+ if (bsp_priv->phy_iface == PHY_INTERFACE_MODE_RMII ||
+ bsp_priv->phy_iface == PHY_INTERFACE_MODE_INTERNAL)
clk_set_rate(bsp_priv->clk_mac, 50000000);
}

+ if (bsp_priv->internal_phy) {
+ bsp_priv->clk_macphy = devm_clk_get(dev, "clk_macphy");
+ if (IS_ERR(bsp_priv->clk_macphy))
+ dev_err(dev, "cannot get %s clock\n", "clk_macphy");
+ else
+ clk_set_rate(bsp_priv->clk_macphy, 50000000);
+ }
+
return 0;
}

@@ -812,7 +871,8 @@ static int gmac_clk_enable(struct rk_priv_data *bsp_priv, bool enable)

if (enable) {
if (!bsp_priv->clk_enabled) {
- if (phy_iface == PHY_INTERFACE_MODE_RMII) {
+ if (phy_iface == PHY_INTERFACE_MODE_RMII ||
+ phy_iface == PHY_INTERFACE_MODE_INTERNAL) {
if (!IS_ERR(bsp_priv->mac_clk_rx))
clk_prepare_enable(
bsp_priv->mac_clk_rx);
@@ -826,6 +886,9 @@ static int gmac_clk_enable(struct rk_priv_data *bsp_priv, bool enable)
bsp_priv->clk_mac_refout);
}

+ if (!IS_ERR(bsp_priv->clk_macphy))
+ clk_prepare_enable(bsp_priv->clk_macphy);
+
if (!IS_ERR(bsp_priv->aclk_mac))
clk_prepare_enable(bsp_priv->aclk_mac);

@@ -844,7 +907,8 @@ static int gmac_clk_enable(struct rk_priv_data *bsp_priv, bool enable)
}
} else {
if (bsp_priv->clk_enabled) {
- if (phy_iface == PHY_INTERFACE_MODE_RMII) {
+ if (phy_iface == PHY_INTERFACE_MODE_RMII ||
+ phy_iface == PHY_INTERFACE_MODE_INTERNAL) {
if (!IS_ERR(bsp_priv->mac_clk_rx))
clk_disable_unprepare(
bsp_priv->mac_clk_rx);
@@ -858,6 +922,9 @@ static int gmac_clk_enable(struct rk_priv_data *bsp_priv, bool enable)
bsp_priv->clk_mac_refout);
}

+ if (!IS_ERR(bsp_priv->clk_macphy))
+ clk_disable_unprepare(bsp_priv->clk_macphy);
+
if (!IS_ERR(bsp_priv->aclk_mac))
clk_disable_unprepare(bsp_priv->aclk_mac);

@@ -940,6 +1007,17 @@ static struct rk_priv_data *rk_gmac_setup(struct platform_device *pdev,
bsp_priv->clock_input = false;
}

+ if (bsp_priv->phy_iface == PHY_INTERFACE_MODE_INTERNAL) {
+ bsp_priv->internal_phy = true;
+ bsp_priv->macphy_reset = devm_reset_control_get(dev, "mac-phy");
+ if (IS_ERR(bsp_priv->macphy_reset)) {
+ dev_info(dev, "no macphy_reset control found\n");
+ bsp_priv->macphy_reset = NULL;
+ }
+ }
+ dev_info(dev, "internal PHY? (%s).\n",
+ bsp_priv->internal_phy ? "yes" : "no");
+
ret = of_property_read_u32(dev->of_node, "tx_delay", &value);
if (ret) {
bsp_priv->tx_delay = 0x30;
@@ -1000,6 +1078,7 @@ static int rk_gmac_powerup(struct rk_priv_data *bsp_priv)
bsp_priv->ops->set_to_rgmii(bsp_priv, 0, bsp_priv->rx_delay);
break;
case PHY_INTERFACE_MODE_RMII:
+ case PHY_INTERFACE_MODE_INTERNAL:
dev_info(dev, "init for RMII\n");
bsp_priv->ops->set_to_rmii(bsp_priv);
break;
@@ -1014,6 +1093,9 @@ static int rk_gmac_powerup(struct rk_priv_data *bsp_priv)
pm_runtime_enable(dev);
pm_runtime_get_sync(dev);

+ if (bsp_priv->internal_phy)
+ rk_gmac_internal_phy_powerup(bsp_priv);
+
return 0;
}

@@ -1021,6 +1103,9 @@ static void rk_gmac_powerdown(struct rk_priv_data *gmac)
{
struct device *dev = &gmac->pdev->dev;

+ if (gmac->internal_phy)
+ rk_gmac_internal_phy_powerdown(gmac);
+
pm_runtime_put_sync(dev);
pm_runtime_disable(dev);

@@ -1041,6 +1126,7 @@ static void rk_fix_speed(void *priv, unsigned int speed)
bsp_priv->ops->set_rgmii_speed(bsp_priv, speed);
break;
case PHY_INTERFACE_MODE_RMII:
+ case PHY_INTERFACE_MODE_INTERNAL:
bsp_priv->ops->set_rmii_speed(bsp_priv, speed);
break;
default:
--
1.9.1


2017-07-27 13:04:24

by David Wu

[permalink] [raw]
Subject: [PATCH v2 10/11] ARM64: dts: rockchip: Add gmac2phy node support for rk3328

The gmac2phy controller of rk3328 is connected to internal phy
directly inside, add the node for the internal phy support.

Signed-off-by: David Wu <[email protected]>
---
arch/arm64/boot/dts/rockchip/rk3328.dtsi | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3328.dtsi b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
index 0be96ce..4349492 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
@@ -63,6 +63,8 @@
i2c1 = &i2c1;
i2c2 = &i2c2;
i2c3 = &i2c3;
+ ethernet0 = &gmac2io;
+ ethernet1 = &gmac2phy;
};

cpus {
@@ -424,6 +426,28 @@
status = "disabled";
};

+ gmac2phy: eth@ff550000 {
+ compatible = "rockchip,rk3328-gmac";
+ reg = <0x0 0xff550000 0x0 0x10000>;
+ rockchip,grf = <&grf>;
+ interrupts = <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "macirq";
+ clocks = <&cru SCLK_MAC2PHY_SRC>, <&cru SCLK_MAC2PHY_RXTX>,
+ <&cru SCLK_MAC2PHY_RXTX>, <&cru SCLK_MAC2PHY_REF>,
+ <&cru ACLK_MAC2PHY>, <&cru PCLK_MAC2PHY>,
+ <&cru SCLK_MAC2PHY_OUT>;
+ clock-names = "stmmaceth", "mac_clk_rx",
+ "mac_clk_tx", "clk_mac_ref",
+ "aclk_mac", "pclk_mac",
+ "clk_macphy";
+ resets = <&cru SRST_GMAC2PHY_A>, <&cru SRST_MACPHY>;
+ reset-names = "stmmaceth", "mac-phy";
+ phy-mode = "internal";
+ pinctrl-names = "default";
+ pinctrl-0 = <&fephyled_rxm1 &fephyled_linkm1>;
+ status = "disabled";
+ };
+
gic: interrupt-controller@ff811000 {
compatible = "arm,gic-400";
#interrupt-cells = <3>;
--
1.9.1


2017-07-27 13:06:31

by David Wu

[permalink] [raw]
Subject: [PATCH v2 11/11] ARM64: dts: rockchip: Enable gmac2phy for rk3328-evb

Enable the gmac2phy, make the gmac2phy work on
the rk3328-evb board.

Signed-off-by: David Wu <[email protected]>
---
arch/arm64/boot/dts/rockchip/rk3328-evb.dts | 17 +++++++++++++++++
1 file changed, 17 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3328-evb.dts b/arch/arm64/boot/dts/rockchip/rk3328-evb.dts
index cf27239..b9f36da 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328-evb.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3328-evb.dts
@@ -50,6 +50,23 @@
chosen {
stdout-path = "serial2:1500000n8";
};
+
+ vcc_phy: vcc-phy-regulator {
+ compatible = "regulator-fixed";
+ regulator-name = "vcc_phy";
+ regulator-always-on;
+ regulator-boot-on;
+ };
+};
+
+&gmac2phy {
+ phy-supply = <&vcc_phy>;
+ clock_in_out = "output";
+ assigned-clocks = <&cru SCLK_MAC2PHY_SRC>;
+ assigned-clock-rate = <50000000>;
+ assigned-clocks = <&cru SCLK_MAC2PHY>;
+ assigned-clock-parents = <&cru SCLK_MAC2PHY_SRC>;
+ status = "okay";
};

&uart2 {
--
1.9.1


2017-07-27 13:38:22

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH v2 01/11] net: phy: Add rockchip phy driver support

> + /*
> + * If mode switch happens from 10BT to 100BT, all DSP/AFE
> + * registers are set to default values. So any AFE/DSP
> + * registers have to be re-initialized in this case.
> + */

Hi David

Are they also lost on suspend and resume?

Andrew

2017-07-27 13:48:54

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH v2 05/11] net: stmmac: dwmac-rk: Add internal phy support

On Thu, Jul 27, 2017 at 09:02:16PM +0800, David Wu wrote:
> To make internal phy work, need to configure the phy_clock,
> phy cru_reset and related registers.
>
> Signed-off-by: David Wu <[email protected]>
> ---
> changes in v2:
> - Use the standard "phy-mode" property for internal phy. (Florian)

I think we need to discuss this. This PHY appears to be on an MDIO
bus, it uses a standard PHY driver, and it appears to be using an RMII
interface. So it is just an ordinary PHY.

Internal is supposed to be something which is not ordinary, does not
use one of the standard phy modes, needs something special to make it
work.

Florain, it appears to be your suggestion to use internal. What do you
say?

Andrew

2017-07-27 16:51:39

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH v2 01/11] net: phy: Add rockchip phy driver support

On 07/27/2017 05:55 AM, David Wu wrote:
> Support internal ephy currently.
>
> Signed-off-by: David Wu <[email protected]>
> ---
> changes in v2:
> - Alphabetic order for Kconfig and Makefile.
> - Add analog register init.
> - Disable auto-mdix for workround.
> - Rename config
>
> drivers/net/phy/Kconfig | 5 ++
> drivers/net/phy/Makefile | 1 +
> drivers/net/phy/rockchip.c | 128 +++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 134 insertions(+)
> create mode 100644 drivers/net/phy/rockchip.c
>
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index 2dda720..8dc6cd7 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -334,6 +334,11 @@ config REALTEK_PHY
> ---help---
> Supports the Realtek 821x PHY.
>
> +config ROCKCHIP_PHY
> + tristate "Drivers for ROCKCHIP PHYs"

"Driver for Rockchip Ethernet PHYs" would seem more appropriate, this is
just one driver for now.

> + ---help---
> + Currently supports the internal ephy.

EPHY is usually how an Ethernet PHY is abbreviated.

> +
> config SMSC_PHY
> tristate "SMSC PHYs"
> ---help---
> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> index 8e9b9f3..350520e 100644
> --- a/drivers/net/phy/Makefile
> +++ b/drivers/net/phy/Makefile
> @@ -66,6 +66,7 @@ obj-$(CONFIG_MICROSEMI_PHY) += mscc.o
> obj-$(CONFIG_NATIONAL_PHY) += national.o
> obj-$(CONFIG_QSEMI_PHY) += qsemi.o
> obj-$(CONFIG_REALTEK_PHY) += realtek.o
> +obj-$(CONFIG_ROCKCHIP_PHY) += rockchip.o
> obj-$(CONFIG_SMSC_PHY) += smsc.o
> obj-$(CONFIG_STE10XP) += ste10Xp.o
> obj-$(CONFIG_TERANETICS_PHY) += teranetics.o
> diff --git a/drivers/net/phy/rockchip.c b/drivers/net/phy/rockchip.c
> new file mode 100644
> index 0000000..3f74658
> --- /dev/null
> +++ b/drivers/net/phy/rockchip.c
> @@ -0,0 +1,128 @@
> +/**
> + * drivers/net/phy/rockchip.c
> + *
> + * Driver for ROCKCHIP PHY
> + *
> + * Copyright (c) 2017, Fuzhou Rockchip Electronics Co., Ltd
> + *
> + * David Wu<[email protected]>

Missing space between your last name and your email address, there is
another typo like this in the MODULE_AUTHOR() macro.

> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mii.h>
> +#include <linux/ethtool.h>
> +#include <linux/phy.h>
> +#include <linux/netdevice.h>
> +
> +#define MII_INTERNAL_CTRL_STATUS 17
> +#define SMI_ADDR_TSTCNTL 20
> +#define SMI_ADDR_TSTREAD1 21
> +#define SMI_ADDR_TSTREAD2 22
> +#define SMI_ADDR_TSTWRITE 23
> +
> +#define AUTOMDIX_EN BIT(7)
> +#define TSTCNTL_RD (BIT(15) | BIT(10))
> +#define TSTCNTL_WR (BIT(14) | BIT(10))
> +
> +#define WR_ADDR_A7CFG 0x18
> +
> +static void rockchip_init_tstmode(struct phy_device *phydev)
> +{
> + /* Enable access to Analog and DSP register banks */
> + phy_write(phydev, SMI_ADDR_TSTCNTL, 0x0400);
> + phy_write(phydev, SMI_ADDR_TSTCNTL, 0x0000);
> + phy_write(phydev, SMI_ADDR_TSTCNTL, 0x0400);
> +}
> +
> +static void rockchip_close_tstmode(struct phy_device *phydev)
> +{
> + /* Back to basic register bank */
> + phy_write(phydev, SMI_ADDR_TSTCNTL, 0x0000);
> +}
> +
> +static void rockchip_internal_phy_analog_init(struct phy_device *phydev)
> +{
> + rockchip_init_tstmode(phydev);

Technically MDIO writes can fail, but you are not propagating the return
value, so you could be stuck on a bad page/bank.

> +
> + /*
> + * Adjust tx amplitude to make sginal better,
> + * the default value is 0x8.
> + */
> + phy_write(phydev, SMI_ADDR_TSTWRITE, 0xB);
> + phy_write(phydev, SMI_ADDR_TSTCNTL, TSTCNTL_WR | WR_ADDR_A7CFG);

Likewise.

> +
> + rockchip_close_tstmode(phydev);

Same here.

> +}
> +
> +static int rockchip_internal_phy_config_init(struct phy_device *phydev)
> +{
> + int val;
> +
> + /*
> + * The auto MIDX has linked problem on some board,
> + * workround to disable auto MDIX.
> + */

If this a board-specific problem you may consider register a PHY fixup
for the affected boards, or introduce a specific property to illustrate
that MDI-X is broken.

> + val = phy_read(phydev, MII_INTERNAL_CTRL_STATUS);
> + val &= ~AUTOMDIX_EN;
> + phy_write(phydev, MII_INTERNAL_CTRL_STATUS, val);

You also need to reject MDI configuration requests coming via
phy_ethtool_ksettings_set() which you should see in the
phyddrv::config_ane callback.

> +
> + rockchip_internal_phy_analog_init(phydev);
> +
> + return 0;
> +}
> +
> +static int rockchip_internal_phy_read_status(struct phy_device *phydev)
> +{
> + int ret, old_speed;
> +
> + old_speed = phydev->speed;
> + ret = genphy_read_status(phydev);
> + if (ret)
> + return ret;
> +
> + /*
> + * If mode switch happens from 10BT to 100BT, all DSP/AFE
> + * registers are set to default values. So any AFE/DSP
> + * registers have to be re-initialized in this case.
> + */
> + if ((old_speed == SPEED_10) && (phydev->speed == SPEED_100))
> + rockchip_internal_phy_analog_init(phydev);

link changes can be intercepted with a link_change_notify callback,
which would be more appropriate than doing this during read_status here.

> +
> + return ret;
> +}
> +
> +static struct phy_driver rockchip_phy_driver[] = {
> +{
> + .phy_id = 0x1234d400,
> + .phy_id_mask = 0xffff0000,

This mask is way too wide, the industry practice is to have the last 4
digits contain the PHY revision, so I would expect to see 0xffff_fff0
here as a mask.

> + .name = "rockchip internal ephy",

Rockchip internal EPHY?

> + .features = (PHY_BASIC_FEATURES | SUPPORTED_Pause
> + | SUPPORTED_Asym_Pause),
> + .soft_reset = genphy_soft_reset,

Since this is a driver for an internal PHY you also need:

.flags = PHY_IS_INTERNAL

> + .config_init = rockchip_internal_phy_config_init,
> + .config_aneg = genphy_config_aneg,
> + .read_status = rockchip_internal_phy_read_status,
> + .suspend = genphy_suspend,
> + .resume = genphy_resume,

You probably need your resume function to restore the AFE/DSP settings
that were done in rockchip_internal_phy_config_init()

> +},
> +};
> +
> +module_phy_driver(rockchip_phy_driver);
> +
> +static struct mdio_device_id __maybe_unused rockchip_phy_tbl[] = {
> + { 0x1234d400, 0xffff0000 },

Please also fix up the mask here.

> + { }
> +};
> +
> +MODULE_DEVICE_TABLE(mdio, rockchip_phy_tbl);
> +
> +MODULE_AUTHOR("David Wu<[email protected]>");

Typo here (same as in the heading).

> +MODULE_DESCRIPTION("Rockchip phy driver");

"Rockchip Ethernet PHY driver" so make it clear what type of PHY it manages?

> +MODULE_LICENSE("GPL v2");
>


--
Florian

2017-07-27 16:54:08

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH v2 05/11] net: stmmac: dwmac-rk: Add internal phy support

On 07/27/2017 06:48 AM, Andrew Lunn wrote:
> On Thu, Jul 27, 2017 at 09:02:16PM +0800, David Wu wrote:
>> To make internal phy work, need to configure the phy_clock,
>> phy cru_reset and related registers.
>>
>> Signed-off-by: David Wu <[email protected]>
>> ---
>> changes in v2:
>> - Use the standard "phy-mode" property for internal phy. (Florian)
>
> I think we need to discuss this. This PHY appears to be on an MDIO
> bus, it uses a standard PHY driver, and it appears to be using an RMII
> interface. So it is just an ordinary PHY.

First, the fact that the internal PHY also appears through MDIO is
orthogonal to the fact that it is internal or external. Plenty of
designs have internal PHYs exposed through MDIO because that is
convenient. What matters though is how the data/clock lines are wired
internally, which is what "phy-mode" describes.

>
> Internal is supposed to be something which is not ordinary, does not
> use one of the standard phy modes, needs something special to make it
> work.
>
> Florain, it appears to be your suggestion to use internal. What do you
> say?

phy-mode = "internal" really means that it is not a standard MII variant
to connect the data/clock lines between the Ethernet MAC and the PHY,
and this can happen in some designs (although quite unlikely). So from
there we could do several things depending on the requirements:

- if you can have your Ethernet MAC driver perform the necessary
configuration *after* you have been able to bind the PHY device with its
PHY driver, then the PHY driver should have PHY_IS_INTERNAL in its
flags, and you can use phy_is_internal() from PHYLIB to tell you that
and we could imagine using: phy-mode = "rmii" because that would not too
much of a stretch

- if you need knowledge about this PHY connection type prior to binding
the PHY device and its driver (that is, before of_phy_connect()) we
could add a boolean property e.g: "phy-is-internal" that allows us to
know that, or we can have a new phy-mode value, e.g: "internal-rmii"
which describes that, either way would probably be fine, but the former
scales better

Then again, using phy-mode = "internal" even though this is Reduced MII
is not big of a deal IMHO as long as there is no loss of information and
that internal de-facto means internal reduced MII for instance.
--
Florian

2017-07-27 17:32:05

by Corentin Labbe

[permalink] [raw]
Subject: Re: [PATCH v2 05/11] net: stmmac: dwmac-rk: Add internal phy support

On Thu, Jul 27, 2017 at 09:54:01AM -0700, Florian Fainelli wrote:
> On 07/27/2017 06:48 AM, Andrew Lunn wrote:
> > On Thu, Jul 27, 2017 at 09:02:16PM +0800, David Wu wrote:
> >> To make internal phy work, need to configure the phy_clock,
> >> phy cru_reset and related registers.
> >>
> >> Signed-off-by: David Wu <[email protected]>
> >> ---
> >> changes in v2:
> >> - Use the standard "phy-mode" property for internal phy. (Florian)
> >
> > I think we need to discuss this. This PHY appears to be on an MDIO
> > bus, it uses a standard PHY driver, and it appears to be using an RMII
> > interface. So it is just an ordinary PHY.
>
> First, the fact that the internal PHY also appears through MDIO is
> orthogonal to the fact that it is internal or external. Plenty of
> designs have internal PHYs exposed through MDIO because that is
> convenient. What matters though is how the data/clock lines are wired
> internally, which is what "phy-mode" describes.
>
> >
> > Internal is supposed to be something which is not ordinary, does not
> > use one of the standard phy modes, needs something special to make it
> > work.
> >
> > Florain, it appears to be your suggestion to use internal. What do you
> > say?
>
> phy-mode = "internal" really means that it is not a standard MII variant
> to connect the data/clock lines between the Ethernet MAC and the PHY,
> and this can happen in some designs (although quite unlikely). So from
> there we could do several things depending on the requirements:
>
> - if you can have your Ethernet MAC driver perform the necessary
> configuration *after* you have been able to bind the PHY device with its
> PHY driver, then the PHY driver should have PHY_IS_INTERNAL in its
> flags, and you can use phy_is_internal() from PHYLIB to tell you that
> and we could imagine using: phy-mode = "rmii" because that would not too
> much of a stretch
>
> - if you need knowledge about this PHY connection type prior to binding
> the PHY device and its driver (that is, before of_phy_connect()) we
> could add a boolean property e.g: "phy-is-internal" that allows us to
> know that, or we can have a new phy-mode value, e.g: "internal-rmii"
> which describes that, either way would probably be fine, but the former
> scales better
>

Hello

We have the same problem on Allwinner SoCs for dwmac-sun8i, we need to set a syscon for chossing between internal/external PHY.
Having this phy-is-internal would be very helpfull. (adding internal-xmii will add too many flags in our case)

Thanks
Regards
Corentin Labbe

2017-07-28 06:46:36

by David Wu

[permalink] [raw]
Subject: Re: [PATCH v2 05/11] net: stmmac: dwmac-rk: Add internal phy support

Hi Andrew,

?? 2017/7/27 21:48, Andrew Lunn д??:
> I think we need to discuss this. This PHY appears to be on an MDIO
> bus, it uses a standard PHY driver, and it appears to be using an RMII
> interface. So it is just an ordinary PHY.
>
> Internal is supposed to be something which is not ordinary, does not
> use one of the standard phy modes, needs something special to make it
> work.

Yes, it is a ordinary PHY in fact, using MDIO bus, but it is a internal
phy inside Soc, so the "internal" is not the internal as Florain said.

2017-07-28 06:56:13

by David Wu

[permalink] [raw]
Subject: Re: [PATCH v2 05/11] net: stmmac: dwmac-rk: Add internal phy support

Hi Florian,

在 2017/7/28 0:54, Florian Fainelli 写道:
> - if you need knowledge about this PHY connection type prior to binding
> the PHY device and its driver (that is, before of_phy_connect()) we
> could add a boolean property e.g: "phy-is-internal" that allows us to
> know that, or we can have a new phy-mode value, e.g: "internal-rmii"
> which describes that, either way would probably be fine, but the former
> scales better
>

Using "phy-is-internal" is very helpful, but it is easy to confuse with
the real internal PHY, may we use the other words like phy is inlined????.

> Then again, using phy-mode = "internal" even though this is Reduced MII
> is not big of a deal IMHO as long as there is no loss of information and
> that internal de-facto means internal reduced MII for instance.
> --

2017-07-28 07:39:39

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v2 05/11] net: stmmac: dwmac-rk: Add internal phy support

On Thu, Jul 27, 2017 at 07:31:52PM +0200, Corentin Labbe wrote:
> On Thu, Jul 27, 2017 at 09:54:01AM -0700, Florian Fainelli wrote:
> > On 07/27/2017 06:48 AM, Andrew Lunn wrote:
> > > On Thu, Jul 27, 2017 at 09:02:16PM +0800, David Wu wrote:
> > >> To make internal phy work, need to configure the phy_clock,
> > >> phy cru_reset and related registers.
> > >>
> > >> Signed-off-by: David Wu <[email protected]>
> > >> ---
> > >> changes in v2:
> > >> - Use the standard "phy-mode" property for internal phy. (Florian)
> > >
> > > I think we need to discuss this. This PHY appears to be on an MDIO
> > > bus, it uses a standard PHY driver, and it appears to be using an RMII
> > > interface. So it is just an ordinary PHY.
> >
> > First, the fact that the internal PHY also appears through MDIO is
> > orthogonal to the fact that it is internal or external. Plenty of
> > designs have internal PHYs exposed through MDIO because that is
> > convenient. What matters though is how the data/clock lines are wired
> > internally, which is what "phy-mode" describes.
> >
> > >
> > > Internal is supposed to be something which is not ordinary, does not
> > > use one of the standard phy modes, needs something special to make it
> > > work.
> > >
> > > Florain, it appears to be your suggestion to use internal. What do you
> > > say?
> >
> > phy-mode = "internal" really means that it is not a standard MII variant
> > to connect the data/clock lines between the Ethernet MAC and the PHY,
> > and this can happen in some designs (although quite unlikely). So from
> > there we could do several things depending on the requirements:
> >
> > - if you can have your Ethernet MAC driver perform the necessary
> > configuration *after* you have been able to bind the PHY device with its
> > PHY driver, then the PHY driver should have PHY_IS_INTERNAL in its
> > flags, and you can use phy_is_internal() from PHYLIB to tell you that
> > and we could imagine using: phy-mode = "rmii" because that would not too
> > much of a stretch
> >
> > - if you need knowledge about this PHY connection type prior to binding
> > the PHY device and its driver (that is, before of_phy_connect()) we
> > could add a boolean property e.g: "phy-is-internal" that allows us to
> > know that, or we can have a new phy-mode value, e.g: "internal-rmii"
> > which describes that, either way would probably be fine, but the former
> > scales better
>
> We have the same problem on Allwinner SoCs for dwmac-sun8i, we need
> to set a syscon for chossing between internal/external PHY.
>
> Having this phy-is-internal would be very helpfull. (adding
> internal-xmii will add too many flags in our case)

In our case, we'll always have a phy node, so we can have a compatible
that will give you the same information.

Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


Attachments:
(No filename) (2.83 kB)
signature.asc (801.00 B)
Download all attachments

2017-07-28 07:48:21

by David Wu

[permalink] [raw]
Subject: Re: [PATCH v2 01/11] net: phy: Add rockchip phy driver support

Hi Andrew,

?? 2017/7/27 21:38, Andrew Lunn д??:
>> + /*
>> + * If mode switch happens from 10BT to 100BT, all DSP/AFE
>> + * registers are set to default values. So any AFE/DSP
>> + * registers have to be re-initialized in this case.
>> + */
>
> Hi David
>
> Are they also lost on suspend and resume?

Yes, i will add the re-initialized on suspend and resume??and thank you??

>
> Andrew
>
>
>

2017-07-28 08:20:04

by David Wu

[permalink] [raw]
Subject: Re: [PATCH v2 01/11] net: phy: Add rockchip phy driver support

Hi Florian,

在 2017/7/28 0:51, Florian Fainelli 写道:
>> +}
>> +
>> +static int rockchip_internal_phy_config_init(struct phy_device *phydev)
>> +{
>> + int val;
>> +
>> + /*
>> + * The auto MIDX has linked problem on some board,
>> + * workround to disable auto MDIX.
>> + */
> If this a board-specific problem you may consider register a PHY fixup
> for the affected boards, or introduce a specific property to illustrate
> that MDI-X is broken.

You mean is that if I need disable Auto-mdix, then I'm going to add a
property to DTS, from here to decide whether or not to close Auto-mdix?

>

2017-08-02 03:47:23

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH v2 05/11] net: stmmac: dwmac-rk: Add internal phy support

Hi David, Florian, Andrew

(resent in plain text)

On Fri, Jul 28, 2017 at 2:56 PM, David.Wu <[email protected]> wrote:
>
> Hi Florian,
>
> 在 2017/7/28 0:54, Florian Fainelli 写道:
>>
>> - if you need knowledge about this PHY connection type prior to binding
>> the PHY device and its driver (that is, before of_phy_connect()) we
>> could add a boolean property e.g: "phy-is-internal" that allows us to
>> know that, or we can have a new phy-mode value, e.g: "internal-rmii"
>> which describes that, either way would probably be fine, but the former
>> scales better
>>
>
> Using "phy-is-internal" is very helpful, but it is easy to confuse with
> the real internal PHY, may we use the other words like phy is inlined.

If "internal" is confusing, would "phy-is-integrated" in the MAC node work?

Either way we would like to have a definitive solution to this. Our
dwmac-sun8i driver is already in v4.13-rc1, with a somewhat flaky
method of knowing whether the internal PHY is used (phy-mode = "mii").

We really want a fix for this release, otherwise we would be force
to revert either the internal PHY part or the whole driver.

ChenYu

>
>> Then again, using phy-mode = "internal" even though this is Reduced MII
>> is not big of a deal IMHO as long as there is no loss of information and
>> that internal de-facto means internal reduced MII for instance.
>> --
>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel