All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next 0/3] net: stmmac: RK3566/RK3568
@ 2021-04-13 21:02 ` Ezequiel Garcia
  0 siblings, 0 replies; 25+ messages in thread
From: Ezequiel Garcia @ 2021-04-13 21:02 UTC (permalink / raw)
  To: netdev, linux-rockchip
  Cc: Jose Abreu, Heiko Stuebner, David S . Miller, Jakub Kicinski,
	Peter Geis, Ezequiel Garcia, kernel

Add support for Rockchip RK3566 and RK3568 SoCs.

This has been tested on a Pine64 RK3566 Quartz64 Model B board,
DHCP and iperf are looking good.

While here, I'm adding a small patch from David Wu, for some
sanity checks for dwmac-rockchip-specific non-NULL ops.

Thanks!

David Wu (2):
  net: stmmac: dwmac-rk: Check platform-specific ops
  net: stmmac: Add RK3566/RK3568 SoC support

Ezequiel Garcia (1):
  net: stmmac: Don't set has_gmac if has_gmac4 is set

 .../bindings/net/rockchip-dwmac.txt           |   2 +
 .../net/ethernet/stmicro/stmmac/dwmac-rk.c    | 179 +++++++++++++++++-
 2 files changed, 178 insertions(+), 3 deletions(-)

-- 
2.30.0


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH net-next 0/3] net: stmmac: RK3566/RK3568
@ 2021-04-13 21:02 ` Ezequiel Garcia
  0 siblings, 0 replies; 25+ messages in thread
From: Ezequiel Garcia @ 2021-04-13 21:02 UTC (permalink / raw)
  To: netdev, linux-rockchip
  Cc: Jose Abreu, Heiko Stuebner, David S . Miller, Jakub Kicinski,
	Peter Geis, Ezequiel Garcia, kernel

Add support for Rockchip RK3566 and RK3568 SoCs.

This has been tested on a Pine64 RK3566 Quartz64 Model B board,
DHCP and iperf are looking good.

While here, I'm adding a small patch from David Wu, for some
sanity checks for dwmac-rockchip-specific non-NULL ops.

Thanks!

David Wu (2):
  net: stmmac: dwmac-rk: Check platform-specific ops
  net: stmmac: Add RK3566/RK3568 SoC support

Ezequiel Garcia (1):
  net: stmmac: Don't set has_gmac if has_gmac4 is set

 .../bindings/net/rockchip-dwmac.txt           |   2 +
 .../net/ethernet/stmicro/stmmac/dwmac-rk.c    | 179 +++++++++++++++++-
 2 files changed, 178 insertions(+), 3 deletions(-)

-- 
2.30.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH net-next 1/3] net: stmmac: Don't set has_gmac if has_gmac4 is set
  2021-04-13 21:02 ` Ezequiel Garcia
@ 2021-04-13 21:02   ` Ezequiel Garcia
  -1 siblings, 0 replies; 25+ messages in thread
From: Ezequiel Garcia @ 2021-04-13 21:02 UTC (permalink / raw)
  To: netdev, linux-rockchip
  Cc: Jose Abreu, Heiko Stuebner, David S . Miller, Jakub Kicinski,
	Peter Geis, Ezequiel Garcia, kernel

Some Rockchip platforms have a GMAC4 core, and therefore
'plat_stmmacenet_data.has_gmac' shouldn't be set if
'plat_stmmacenet_data.has_gmac4' is set.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
index 6ef30252bfe0..c432a9592489 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
@@ -1400,7 +1400,11 @@ static int rk_gmac_probe(struct platform_device *pdev)
 	if (IS_ERR(plat_dat))
 		return PTR_ERR(plat_dat);
 
-	plat_dat->has_gmac = true;
+	/* If the stmmac is not already selected as gmac4,
+	 * then make sure we fallback to gmac.
+	 */
+	if (!plat_dat->has_gmac4)
+		plat_dat->has_gmac = true;
 	plat_dat->fix_mac_speed = rk_fix_speed;
 
 	plat_dat->bsp_priv = rk_gmac_setup(pdev, plat_dat, data);
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH net-next 1/3] net: stmmac: Don't set has_gmac if has_gmac4 is set
@ 2021-04-13 21:02   ` Ezequiel Garcia
  0 siblings, 0 replies; 25+ messages in thread
From: Ezequiel Garcia @ 2021-04-13 21:02 UTC (permalink / raw)
  To: netdev, linux-rockchip
  Cc: Jose Abreu, Heiko Stuebner, David S . Miller, Jakub Kicinski,
	Peter Geis, Ezequiel Garcia, kernel

Some Rockchip platforms have a GMAC4 core, and therefore
'plat_stmmacenet_data.has_gmac' shouldn't be set if
'plat_stmmacenet_data.has_gmac4' is set.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
index 6ef30252bfe0..c432a9592489 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
@@ -1400,7 +1400,11 @@ static int rk_gmac_probe(struct platform_device *pdev)
 	if (IS_ERR(plat_dat))
 		return PTR_ERR(plat_dat);
 
-	plat_dat->has_gmac = true;
+	/* If the stmmac is not already selected as gmac4,
+	 * then make sure we fallback to gmac.
+	 */
+	if (!plat_dat->has_gmac4)
+		plat_dat->has_gmac = true;
 	plat_dat->fix_mac_speed = rk_fix_speed;
 
 	plat_dat->bsp_priv = rk_gmac_setup(pdev, plat_dat, data);
-- 
2.30.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH net-next 2/3] net: stmmac: dwmac-rk: Check platform-specific ops
  2021-04-13 21:02 ` Ezequiel Garcia
@ 2021-04-13 21:02   ` Ezequiel Garcia
  -1 siblings, 0 replies; 25+ messages in thread
From: Ezequiel Garcia @ 2021-04-13 21:02 UTC (permalink / raw)
  To: netdev, linux-rockchip
  Cc: Jose Abreu, Heiko Stuebner, David S . Miller, Jakub Kicinski,
	Peter Geis, David Wu, kernel, Ezequiel Garcia

From: David Wu <david.wu@rock-chips.com>

Add a check for non-null struct rk_gmac_ops for the
configured PHY interface mode, failing if unsupported.

Signed-off-by: David Wu <david.wu@rock-chips.com>
[Ezequiel: Refactor so it fails if unsupported]
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 .../net/ethernet/stmicro/stmmac/dwmac-rk.c    | 31 +++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
index c432a9592489..d2637d83899e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
@@ -1294,11 +1294,36 @@ static struct rk_priv_data *rk_gmac_setup(struct platform_device *pdev,
 	return bsp_priv;
 }
 
+static int rk_gmac_check_ops(struct rk_priv_data *bsp_priv)
+{
+	switch (bsp_priv->phy_iface) {
+	case PHY_INTERFACE_MODE_RGMII:
+	case PHY_INTERFACE_MODE_RGMII_ID:
+	case PHY_INTERFACE_MODE_RGMII_RXID:
+	case PHY_INTERFACE_MODE_RGMII_TXID:
+		if (!bsp_priv->ops->set_to_rgmii)
+			return -EINVAL;
+		break;
+	case PHY_INTERFACE_MODE_RMII:
+		if (!bsp_priv->ops->set_to_rmii)
+			return -EINVAL;
+		break;
+	default:
+		dev_err(&bsp_priv->pdev->dev,
+			"unsupported interface %d", bsp_priv->phy_iface);
+	}
+	return 0;
+}
+
 static int rk_gmac_powerup(struct rk_priv_data *bsp_priv)
 {
 	int ret;
 	struct device *dev = &bsp_priv->pdev->dev;
 
+	ret = rk_gmac_check_ops(bsp_priv);
+	if (ret)
+		return ret;
+
 	ret = gmac_clk_enable(bsp_priv, true);
 	if (ret)
 		return ret;
@@ -1369,10 +1394,12 @@ static void rk_fix_speed(void *priv, unsigned int speed)
 	case PHY_INTERFACE_MODE_RGMII_ID:
 	case PHY_INTERFACE_MODE_RGMII_RXID:
 	case PHY_INTERFACE_MODE_RGMII_TXID:
-		bsp_priv->ops->set_rgmii_speed(bsp_priv, speed);
+		if (bsp_priv->ops->set_rgmii_speed)
+			bsp_priv->ops->set_rgmii_speed(bsp_priv, speed);
 		break;
 	case PHY_INTERFACE_MODE_RMII:
-		bsp_priv->ops->set_rmii_speed(bsp_priv, speed);
+		if (bsp_priv->ops->set_rmii_speed)
+			bsp_priv->ops->set_rmii_speed(bsp_priv, speed);
 		break;
 	default:
 		dev_err(dev, "unsupported interface %d", bsp_priv->phy_iface);
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH net-next 2/3] net: stmmac: dwmac-rk: Check platform-specific ops
@ 2021-04-13 21:02   ` Ezequiel Garcia
  0 siblings, 0 replies; 25+ messages in thread
From: Ezequiel Garcia @ 2021-04-13 21:02 UTC (permalink / raw)
  To: netdev, linux-rockchip
  Cc: Jose Abreu, Heiko Stuebner, David S . Miller, Jakub Kicinski,
	Peter Geis, David Wu, kernel, Ezequiel Garcia

From: David Wu <david.wu@rock-chips.com>

Add a check for non-null struct rk_gmac_ops for the
configured PHY interface mode, failing if unsupported.

Signed-off-by: David Wu <david.wu@rock-chips.com>
[Ezequiel: Refactor so it fails if unsupported]
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 .../net/ethernet/stmicro/stmmac/dwmac-rk.c    | 31 +++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
index c432a9592489..d2637d83899e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
@@ -1294,11 +1294,36 @@ static struct rk_priv_data *rk_gmac_setup(struct platform_device *pdev,
 	return bsp_priv;
 }
 
+static int rk_gmac_check_ops(struct rk_priv_data *bsp_priv)
+{
+	switch (bsp_priv->phy_iface) {
+	case PHY_INTERFACE_MODE_RGMII:
+	case PHY_INTERFACE_MODE_RGMII_ID:
+	case PHY_INTERFACE_MODE_RGMII_RXID:
+	case PHY_INTERFACE_MODE_RGMII_TXID:
+		if (!bsp_priv->ops->set_to_rgmii)
+			return -EINVAL;
+		break;
+	case PHY_INTERFACE_MODE_RMII:
+		if (!bsp_priv->ops->set_to_rmii)
+			return -EINVAL;
+		break;
+	default:
+		dev_err(&bsp_priv->pdev->dev,
+			"unsupported interface %d", bsp_priv->phy_iface);
+	}
+	return 0;
+}
+
 static int rk_gmac_powerup(struct rk_priv_data *bsp_priv)
 {
 	int ret;
 	struct device *dev = &bsp_priv->pdev->dev;
 
+	ret = rk_gmac_check_ops(bsp_priv);
+	if (ret)
+		return ret;
+
 	ret = gmac_clk_enable(bsp_priv, true);
 	if (ret)
 		return ret;
@@ -1369,10 +1394,12 @@ static void rk_fix_speed(void *priv, unsigned int speed)
 	case PHY_INTERFACE_MODE_RGMII_ID:
 	case PHY_INTERFACE_MODE_RGMII_RXID:
 	case PHY_INTERFACE_MODE_RGMII_TXID:
-		bsp_priv->ops->set_rgmii_speed(bsp_priv, speed);
+		if (bsp_priv->ops->set_rgmii_speed)
+			bsp_priv->ops->set_rgmii_speed(bsp_priv, speed);
 		break;
 	case PHY_INTERFACE_MODE_RMII:
-		bsp_priv->ops->set_rmii_speed(bsp_priv, speed);
+		if (bsp_priv->ops->set_rmii_speed)
+			bsp_priv->ops->set_rmii_speed(bsp_priv, speed);
 		break;
 	default:
 		dev_err(dev, "unsupported interface %d", bsp_priv->phy_iface);
-- 
2.30.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH net-next 3/3] net: stmmac: Add RK3566/RK3568 SoC support
  2021-04-13 21:02 ` Ezequiel Garcia
@ 2021-04-13 21:02   ` Ezequiel Garcia
  -1 siblings, 0 replies; 25+ messages in thread
From: Ezequiel Garcia @ 2021-04-13 21:02 UTC (permalink / raw)
  To: netdev, linux-rockchip
  Cc: Jose Abreu, Heiko Stuebner, David S . Miller, Jakub Kicinski,
	Peter Geis, David Wu, kernel, Ezequiel Garcia

From: David Wu <david.wu@rock-chips.com>

Add constants and callback functions for the dwmac present
on RK3566 and RK3568 SoCs. As can be seen, the base structure
is the same, only registers and the bits in them moved slightly.

RK3568 supports two MACs, and RK3566 support just one.

Signed-off-by: David Wu <david.wu@rock-chips.com>
[Ezequiel: Add rockchip,rk3566-gmac compatible string]
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 .../bindings/net/rockchip-dwmac.txt           |   2 +
 .../net/ethernet/stmicro/stmmac/dwmac-rk.c    | 142 ++++++++++++++++++
 2 files changed, 144 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/rockchip-dwmac.txt b/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
index 3b71da7e8742..80203b16b652 100644
--- a/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
+++ b/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
@@ -12,6 +12,8 @@ Required properties:
    "rockchip,rk3366-gmac": found on RK3366 SoCs
    "rockchip,rk3368-gmac": found on RK3368 SoCs
    "rockchip,rk3399-gmac": found on RK3399 SoCs
+   "rockchip,rk3566-gmac": found on RK3566 SoCs
+   "rockchip,rk3568-gmac": found on RK3568 SoCs
    "rockchip,rv1108-gmac": found on RV1108 SoCs
  - reg: addresses and length of the register sets for the device.
  - interrupts: Should contain the GMAC interrupts.
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
index d2637d83899e..096965b0fec9 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
@@ -948,6 +948,146 @@ static const struct rk_gmac_ops rk3399_ops = {
 	.set_rmii_speed = rk3399_set_rmii_speed,
 };
 
+#define RK3568_GRF_GMAC0_CON0		0x0380
+#define RK3568_GRF_GMAC0_CON1		0x0384
+#define RK3568_GRF_GMAC1_CON0		0x0388
+#define RK3568_GRF_GMAC1_CON1		0x038c
+
+/* RK3568_GRF_GMAC0_CON1 && RK3568_GRF_GMAC1_CON1 */
+#define RK3568_GMAC_PHY_INTF_SEL_RGMII	\
+		(GRF_BIT(4) | GRF_CLR_BIT(5) | GRF_CLR_BIT(6))
+#define RK3568_GMAC_PHY_INTF_SEL_RMII	\
+		(GRF_CLR_BIT(4) | GRF_CLR_BIT(5) | GRF_BIT(6))
+#define RK3568_GMAC_FLOW_CTRL			GRF_BIT(3)
+#define RK3568_GMAC_FLOW_CTRL_CLR		GRF_CLR_BIT(3)
+#define RK3568_GMAC_RXCLK_DLY_ENABLE		GRF_BIT(1)
+#define RK3568_GMAC_RXCLK_DLY_DISABLE		GRF_CLR_BIT(1)
+#define RK3568_GMAC_TXCLK_DLY_ENABLE		GRF_BIT(0)
+#define RK3568_GMAC_TXCLK_DLY_DISABLE		GRF_CLR_BIT(0)
+
+/* RK3568_GRF_GMAC0_CON0 && RK3568_GRF_GMAC1_CON0 */
+#define RK3568_GMAC_CLK_RX_DL_CFG(val)	HIWORD_UPDATE(val, 0x7F, 8)
+#define RK3568_GMAC_CLK_TX_DL_CFG(val)	HIWORD_UPDATE(val, 0x7F, 0)
+
+static void rk3566_set_to_rgmii(struct rk_priv_data *bsp_priv,
+				int tx_delay, int rx_delay)
+{
+	struct device *dev = &bsp_priv->pdev->dev;
+
+	if (IS_ERR(bsp_priv->grf)) {
+		dev_err(dev, "Missing rockchip,grf property\n");
+		return;
+	}
+
+	regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
+		     RK3568_GMAC_PHY_INTF_SEL_RGMII |
+		     RK3568_GMAC_RXCLK_DLY_ENABLE |
+		     RK3568_GMAC_TXCLK_DLY_ENABLE);
+
+	regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON0,
+		     RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
+		     RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
+}
+
+static void rk3566_set_to_rmii(struct rk_priv_data *bsp_priv)
+{
+	struct device *dev = &bsp_priv->pdev->dev;
+
+	if (IS_ERR(bsp_priv->grf)) {
+		dev_err(dev, "%s: Missing rockchip,grf property\n", __func__);
+		return;
+	}
+
+	regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
+		     RK3568_GMAC_PHY_INTF_SEL_RMII);
+}
+
+static void rk3568_set_to_rgmii(struct rk_priv_data *bsp_priv,
+				int tx_delay, int rx_delay)
+{
+	struct device *dev = &bsp_priv->pdev->dev;
+
+	if (IS_ERR(bsp_priv->grf)) {
+		dev_err(dev, "Missing rockchip,grf property\n");
+		return;
+	}
+
+	regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON1,
+		     RK3568_GMAC_PHY_INTF_SEL_RGMII |
+		     RK3568_GMAC_RXCLK_DLY_ENABLE |
+		     RK3568_GMAC_TXCLK_DLY_ENABLE);
+
+	regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON0,
+		     RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
+		     RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
+
+	regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
+		     RK3568_GMAC_PHY_INTF_SEL_RGMII |
+		     RK3568_GMAC_RXCLK_DLY_ENABLE |
+		     RK3568_GMAC_TXCLK_DLY_ENABLE);
+
+	regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON0,
+		     RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
+		     RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
+}
+
+static void rk3568_set_to_rmii(struct rk_priv_data *bsp_priv)
+{
+	struct device *dev = &bsp_priv->pdev->dev;
+
+	if (IS_ERR(bsp_priv->grf)) {
+		dev_err(dev, "%s: Missing rockchip,grf property\n", __func__);
+		return;
+	}
+
+	regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON1,
+		     RK3568_GMAC_PHY_INTF_SEL_RMII);
+
+	regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
+		     RK3568_GMAC_PHY_INTF_SEL_RMII);
+}
+
+static void rk3568_set_gmac_speed(struct rk_priv_data *bsp_priv, int speed)
+{
+	struct device *dev = &bsp_priv->pdev->dev;
+	unsigned long rate;
+	int ret;
+
+	switch (speed) {
+	case 10:
+		rate = 2500000;
+		break;
+	case 100:
+		rate = 25000000;
+		break;
+	case 1000:
+		rate = 125000000;
+		break;
+	default:
+		dev_err(dev, "unknown speed value for GMAC speed=%d", speed);
+		return;
+	}
+
+	ret = clk_set_rate(bsp_priv->clk_mac_speed, rate);
+	if (ret)
+		dev_err(dev, "%s: set clk_mac_speed rate %ld failed %d\n",
+			__func__, rate, ret);
+}
+
+static const struct rk_gmac_ops rk3568_ops = {
+	.set_to_rgmii = rk3568_set_to_rgmii,
+	.set_to_rmii = rk3568_set_to_rmii,
+	.set_rgmii_speed = rk3568_set_gmac_speed,
+	.set_rmii_speed = rk3568_set_gmac_speed,
+};
+
+static const struct rk_gmac_ops rk3566_ops = {
+	.set_to_rgmii = rk3566_set_to_rgmii,
+	.set_to_rmii = rk3566_set_to_rmii,
+	.set_rgmii_speed = rk3568_set_gmac_speed,
+	.set_rmii_speed = rk3568_set_gmac_speed,
+};
+
 #define RV1108_GRF_GMAC_CON0		0X0900
 
 /* RV1108_GRF_GMAC_CON0 */
@@ -1512,6 +1652,8 @@ static const struct of_device_id rk_gmac_dwmac_match[] = {
 	{ .compatible = "rockchip,rk3366-gmac", .data = &rk3366_ops },
 	{ .compatible = "rockchip,rk3368-gmac", .data = &rk3368_ops },
 	{ .compatible = "rockchip,rk3399-gmac", .data = &rk3399_ops },
+	{ .compatible = "rockchip,rk3568-gmac", .data = &rk3568_ops },
+	{ .compatible = "rockchip,rk3566-gmac", .data = &rk3566_ops },
 	{ .compatible = "rockchip,rv1108-gmac", .data = &rv1108_ops },
 	{ }
 };
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH net-next 3/3] net: stmmac: Add RK3566/RK3568 SoC support
@ 2021-04-13 21:02   ` Ezequiel Garcia
  0 siblings, 0 replies; 25+ messages in thread
From: Ezequiel Garcia @ 2021-04-13 21:02 UTC (permalink / raw)
  To: netdev, linux-rockchip
  Cc: Jose Abreu, Heiko Stuebner, David S . Miller, Jakub Kicinski,
	Peter Geis, David Wu, kernel, Ezequiel Garcia

From: David Wu <david.wu@rock-chips.com>

Add constants and callback functions for the dwmac present
on RK3566 and RK3568 SoCs. As can be seen, the base structure
is the same, only registers and the bits in them moved slightly.

RK3568 supports two MACs, and RK3566 support just one.

Signed-off-by: David Wu <david.wu@rock-chips.com>
[Ezequiel: Add rockchip,rk3566-gmac compatible string]
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 .../bindings/net/rockchip-dwmac.txt           |   2 +
 .../net/ethernet/stmicro/stmmac/dwmac-rk.c    | 142 ++++++++++++++++++
 2 files changed, 144 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/rockchip-dwmac.txt b/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
index 3b71da7e8742..80203b16b652 100644
--- a/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
+++ b/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
@@ -12,6 +12,8 @@ Required properties:
    "rockchip,rk3366-gmac": found on RK3366 SoCs
    "rockchip,rk3368-gmac": found on RK3368 SoCs
    "rockchip,rk3399-gmac": found on RK3399 SoCs
+   "rockchip,rk3566-gmac": found on RK3566 SoCs
+   "rockchip,rk3568-gmac": found on RK3568 SoCs
    "rockchip,rv1108-gmac": found on RV1108 SoCs
  - reg: addresses and length of the register sets for the device.
  - interrupts: Should contain the GMAC interrupts.
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
index d2637d83899e..096965b0fec9 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
@@ -948,6 +948,146 @@ static const struct rk_gmac_ops rk3399_ops = {
 	.set_rmii_speed = rk3399_set_rmii_speed,
 };
 
+#define RK3568_GRF_GMAC0_CON0		0x0380
+#define RK3568_GRF_GMAC0_CON1		0x0384
+#define RK3568_GRF_GMAC1_CON0		0x0388
+#define RK3568_GRF_GMAC1_CON1		0x038c
+
+/* RK3568_GRF_GMAC0_CON1 && RK3568_GRF_GMAC1_CON1 */
+#define RK3568_GMAC_PHY_INTF_SEL_RGMII	\
+		(GRF_BIT(4) | GRF_CLR_BIT(5) | GRF_CLR_BIT(6))
+#define RK3568_GMAC_PHY_INTF_SEL_RMII	\
+		(GRF_CLR_BIT(4) | GRF_CLR_BIT(5) | GRF_BIT(6))
+#define RK3568_GMAC_FLOW_CTRL			GRF_BIT(3)
+#define RK3568_GMAC_FLOW_CTRL_CLR		GRF_CLR_BIT(3)
+#define RK3568_GMAC_RXCLK_DLY_ENABLE		GRF_BIT(1)
+#define RK3568_GMAC_RXCLK_DLY_DISABLE		GRF_CLR_BIT(1)
+#define RK3568_GMAC_TXCLK_DLY_ENABLE		GRF_BIT(0)
+#define RK3568_GMAC_TXCLK_DLY_DISABLE		GRF_CLR_BIT(0)
+
+/* RK3568_GRF_GMAC0_CON0 && RK3568_GRF_GMAC1_CON0 */
+#define RK3568_GMAC_CLK_RX_DL_CFG(val)	HIWORD_UPDATE(val, 0x7F, 8)
+#define RK3568_GMAC_CLK_TX_DL_CFG(val)	HIWORD_UPDATE(val, 0x7F, 0)
+
+static void rk3566_set_to_rgmii(struct rk_priv_data *bsp_priv,
+				int tx_delay, int rx_delay)
+{
+	struct device *dev = &bsp_priv->pdev->dev;
+
+	if (IS_ERR(bsp_priv->grf)) {
+		dev_err(dev, "Missing rockchip,grf property\n");
+		return;
+	}
+
+	regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
+		     RK3568_GMAC_PHY_INTF_SEL_RGMII |
+		     RK3568_GMAC_RXCLK_DLY_ENABLE |
+		     RK3568_GMAC_TXCLK_DLY_ENABLE);
+
+	regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON0,
+		     RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
+		     RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
+}
+
+static void rk3566_set_to_rmii(struct rk_priv_data *bsp_priv)
+{
+	struct device *dev = &bsp_priv->pdev->dev;
+
+	if (IS_ERR(bsp_priv->grf)) {
+		dev_err(dev, "%s: Missing rockchip,grf property\n", __func__);
+		return;
+	}
+
+	regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
+		     RK3568_GMAC_PHY_INTF_SEL_RMII);
+}
+
+static void rk3568_set_to_rgmii(struct rk_priv_data *bsp_priv,
+				int tx_delay, int rx_delay)
+{
+	struct device *dev = &bsp_priv->pdev->dev;
+
+	if (IS_ERR(bsp_priv->grf)) {
+		dev_err(dev, "Missing rockchip,grf property\n");
+		return;
+	}
+
+	regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON1,
+		     RK3568_GMAC_PHY_INTF_SEL_RGMII |
+		     RK3568_GMAC_RXCLK_DLY_ENABLE |
+		     RK3568_GMAC_TXCLK_DLY_ENABLE);
+
+	regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON0,
+		     RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
+		     RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
+
+	regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
+		     RK3568_GMAC_PHY_INTF_SEL_RGMII |
+		     RK3568_GMAC_RXCLK_DLY_ENABLE |
+		     RK3568_GMAC_TXCLK_DLY_ENABLE);
+
+	regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON0,
+		     RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
+		     RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
+}
+
+static void rk3568_set_to_rmii(struct rk_priv_data *bsp_priv)
+{
+	struct device *dev = &bsp_priv->pdev->dev;
+
+	if (IS_ERR(bsp_priv->grf)) {
+		dev_err(dev, "%s: Missing rockchip,grf property\n", __func__);
+		return;
+	}
+
+	regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON1,
+		     RK3568_GMAC_PHY_INTF_SEL_RMII);
+
+	regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
+		     RK3568_GMAC_PHY_INTF_SEL_RMII);
+}
+
+static void rk3568_set_gmac_speed(struct rk_priv_data *bsp_priv, int speed)
+{
+	struct device *dev = &bsp_priv->pdev->dev;
+	unsigned long rate;
+	int ret;
+
+	switch (speed) {
+	case 10:
+		rate = 2500000;
+		break;
+	case 100:
+		rate = 25000000;
+		break;
+	case 1000:
+		rate = 125000000;
+		break;
+	default:
+		dev_err(dev, "unknown speed value for GMAC speed=%d", speed);
+		return;
+	}
+
+	ret = clk_set_rate(bsp_priv->clk_mac_speed, rate);
+	if (ret)
+		dev_err(dev, "%s: set clk_mac_speed rate %ld failed %d\n",
+			__func__, rate, ret);
+}
+
+static const struct rk_gmac_ops rk3568_ops = {
+	.set_to_rgmii = rk3568_set_to_rgmii,
+	.set_to_rmii = rk3568_set_to_rmii,
+	.set_rgmii_speed = rk3568_set_gmac_speed,
+	.set_rmii_speed = rk3568_set_gmac_speed,
+};
+
+static const struct rk_gmac_ops rk3566_ops = {
+	.set_to_rgmii = rk3566_set_to_rgmii,
+	.set_to_rmii = rk3566_set_to_rmii,
+	.set_rgmii_speed = rk3568_set_gmac_speed,
+	.set_rmii_speed = rk3568_set_gmac_speed,
+};
+
 #define RV1108_GRF_GMAC_CON0		0X0900
 
 /* RV1108_GRF_GMAC_CON0 */
@@ -1512,6 +1652,8 @@ static const struct of_device_id rk_gmac_dwmac_match[] = {
 	{ .compatible = "rockchip,rk3366-gmac", .data = &rk3366_ops },
 	{ .compatible = "rockchip,rk3368-gmac", .data = &rk3368_ops },
 	{ .compatible = "rockchip,rk3399-gmac", .data = &rk3399_ops },
+	{ .compatible = "rockchip,rk3568-gmac", .data = &rk3568_ops },
+	{ .compatible = "rockchip,rk3566-gmac", .data = &rk3566_ops },
 	{ .compatible = "rockchip,rv1108-gmac", .data = &rv1108_ops },
 	{ }
 };
-- 
2.30.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH net-next 3/3] net: stmmac: Add RK3566/RK3568 SoC support
  2021-04-13 21:02   ` Ezequiel Garcia
@ 2021-04-13 22:51     ` Peter Geis
  -1 siblings, 0 replies; 25+ messages in thread
From: Peter Geis @ 2021-04-13 22:51 UTC (permalink / raw)
  To: Ezequiel Garcia
  Cc: Linux Kernel Network Developers, open list:ARM/Rockchip SoC...,
	Jose Abreu, Heiko Stuebner, David S . Miller, Jakub Kicinski,
	David Wu, kernel

On Tue, Apr 13, 2021 at 5:03 PM Ezequiel Garcia <ezequiel@collabora.com> wrote:
>
> From: David Wu <david.wu@rock-chips.com>
>
> Add constants and callback functions for the dwmac present
> on RK3566 and RK3568 SoCs. As can be seen, the base structure
> is the same, only registers and the bits in them moved slightly.
>
> RK3568 supports two MACs, and RK3566 support just one.

Tested this driver on the rk3566-quartz64.
It fails to fully probe the gmac with the following error:
[    5.711127] rk_gmac-dwmac fe010000.ethernet: IRQ eth_lpi not found
[    5.714147] rk_gmac-dwmac fe010000.ethernet: no regulator found
[    5.714766] rk_gmac-dwmac fe010000.ethernet: clock input or output? (input).
[    5.715474] rk_gmac-dwmac fe010000.ethernet: TX delay(0x4f).
[    5.716058] rk_gmac-dwmac fe010000.ethernet: RX delay(0x25).
[    5.716694] rk_gmac-dwmac fe010000.ethernet: integrated PHY? (no).
[    5.718413] rk_gmac-dwmac fe010000.ethernet: clock input from PHY
[    5.724140] rk_gmac-dwmac fe010000.ethernet: init for RGMII
[    5.726802] rk_gmac-dwmac fe010000.ethernet: Version ID not available
[    5.727525] rk_gmac-dwmac fe010000.ethernet:         DWMAC1000
[    5.728064] rk_gmac-dwmac fe010000.ethernet: DMA HW capability
register supported
[    5.729026] rk_gmac-dwmac fe010000.ethernet: Normal descriptors
[    5.729624] rk_gmac-dwmac fe010000.ethernet: Ring mode enabled
[    5.731123] rk_gmac-dwmac fe010000.ethernet: Unbalanced pm_runtime_enable!
[    5.873329] libphy: stmmac: probed
[    5.905599] rk_gmac-dwmac fe010000.ethernet: Cannot register the MDIO bus
[    5.906335] rk_gmac-dwmac fe010000.ethernet: stmmac_dvr_probe: MDIO
bus (id: 1) registration failed
[    5.914338] rk_gmac-dwmac: probe of fe010000.ethernet failed with error -5

This is due to the lack of setting has_gmac4 = true.

>
> Signed-off-by: David Wu <david.wu@rock-chips.com>
> [Ezequiel: Add rockchip,rk3566-gmac compatible string]
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
>  .../bindings/net/rockchip-dwmac.txt           |   2 +
>  .../net/ethernet/stmicro/stmmac/dwmac-rk.c    | 142 ++++++++++++++++++
>  2 files changed, 144 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/rockchip-dwmac.txt b/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
> index 3b71da7e8742..80203b16b652 100644
> --- a/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
> +++ b/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
> @@ -12,6 +12,8 @@ Required properties:
>     "rockchip,rk3366-gmac": found on RK3366 SoCs
>     "rockchip,rk3368-gmac": found on RK3368 SoCs
>     "rockchip,rk3399-gmac": found on RK3399 SoCs
> +   "rockchip,rk3566-gmac": found on RK3566 SoCs
> +   "rockchip,rk3568-gmac": found on RK3568 SoCs
>     "rockchip,rv1108-gmac": found on RV1108 SoCs
>   - reg: addresses and length of the register sets for the device.
>   - interrupts: Should contain the GMAC interrupts.
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> index d2637d83899e..096965b0fec9 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> @@ -948,6 +948,146 @@ static const struct rk_gmac_ops rk3399_ops = {
>         .set_rmii_speed = rk3399_set_rmii_speed,
>  };
>
> +#define RK3568_GRF_GMAC0_CON0          0x0380
> +#define RK3568_GRF_GMAC0_CON1          0x0384
> +#define RK3568_GRF_GMAC1_CON0          0x0388
> +#define RK3568_GRF_GMAC1_CON1          0x038c
> +
> +/* RK3568_GRF_GMAC0_CON1 && RK3568_GRF_GMAC1_CON1 */
> +#define RK3568_GMAC_PHY_INTF_SEL_RGMII \
> +               (GRF_BIT(4) | GRF_CLR_BIT(5) | GRF_CLR_BIT(6))
> +#define RK3568_GMAC_PHY_INTF_SEL_RMII  \
> +               (GRF_CLR_BIT(4) | GRF_CLR_BIT(5) | GRF_BIT(6))
> +#define RK3568_GMAC_FLOW_CTRL                  GRF_BIT(3)
> +#define RK3568_GMAC_FLOW_CTRL_CLR              GRF_CLR_BIT(3)
> +#define RK3568_GMAC_RXCLK_DLY_ENABLE           GRF_BIT(1)
> +#define RK3568_GMAC_RXCLK_DLY_DISABLE          GRF_CLR_BIT(1)
> +#define RK3568_GMAC_TXCLK_DLY_ENABLE           GRF_BIT(0)
> +#define RK3568_GMAC_TXCLK_DLY_DISABLE          GRF_CLR_BIT(0)
> +
> +/* RK3568_GRF_GMAC0_CON0 && RK3568_GRF_GMAC1_CON0 */
> +#define RK3568_GMAC_CLK_RX_DL_CFG(val) HIWORD_UPDATE(val, 0x7F, 8)
> +#define RK3568_GMAC_CLK_TX_DL_CFG(val) HIWORD_UPDATE(val, 0x7F, 0)
> +
> +static void rk3566_set_to_rgmii(struct rk_priv_data *bsp_priv,
> +                               int tx_delay, int rx_delay)
> +{
> +       struct device *dev = &bsp_priv->pdev->dev;
> +
> +       if (IS_ERR(bsp_priv->grf)) {
> +               dev_err(dev, "Missing rockchip,grf property\n");
> +               return;
> +       }
> +
> +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
> +                    RK3568_GMAC_PHY_INTF_SEL_RGMII |
> +                    RK3568_GMAC_RXCLK_DLY_ENABLE |
> +                    RK3568_GMAC_TXCLK_DLY_ENABLE);
> +
> +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON0,
> +                    RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
> +                    RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
> +}
> +
> +static void rk3566_set_to_rmii(struct rk_priv_data *bsp_priv)
> +{
> +       struct device *dev = &bsp_priv->pdev->dev;
> +
> +       if (IS_ERR(bsp_priv->grf)) {
> +               dev_err(dev, "%s: Missing rockchip,grf property\n", __func__);
> +               return;
> +       }
> +
> +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
> +                    RK3568_GMAC_PHY_INTF_SEL_RMII);
> +}
> +
> +static void rk3568_set_to_rgmii(struct rk_priv_data *bsp_priv,
> +                               int tx_delay, int rx_delay)
> +{
> +       struct device *dev = &bsp_priv->pdev->dev;
> +
> +       if (IS_ERR(bsp_priv->grf)) {
> +               dev_err(dev, "Missing rockchip,grf property\n");
> +               return;
> +       }
> +
> +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON1,
> +                    RK3568_GMAC_PHY_INTF_SEL_RGMII |
> +                    RK3568_GMAC_RXCLK_DLY_ENABLE |
> +                    RK3568_GMAC_TXCLK_DLY_ENABLE);
> +
> +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON0,
> +                    RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
> +                    RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
> +
> +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
> +                    RK3568_GMAC_PHY_INTF_SEL_RGMII |
> +                    RK3568_GMAC_RXCLK_DLY_ENABLE |
> +                    RK3568_GMAC_TXCLK_DLY_ENABLE);
> +
> +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON0,
> +                    RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
> +                    RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));

Since there are two GMACs on the rk3568, and either, or, or both may
be enabled in various configurations, we should only configure the
controller we are currently operating.

> +}
> +
> +static void rk3568_set_to_rmii(struct rk_priv_data *bsp_priv)
> +{
> +       struct device *dev = &bsp_priv->pdev->dev;
> +
> +       if (IS_ERR(bsp_priv->grf)) {
> +               dev_err(dev, "%s: Missing rockchip,grf property\n", __func__);
> +               return;
> +       }
> +
> +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON1,
> +                    RK3568_GMAC_PHY_INTF_SEL_RMII);
> +
> +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
> +                    RK3568_GMAC_PHY_INTF_SEL_RMII);

Same as above.

> +}
> +
> +static void rk3568_set_gmac_speed(struct rk_priv_data *bsp_priv, int speed)
> +{
> +       struct device *dev = &bsp_priv->pdev->dev;
> +       unsigned long rate;
> +       int ret;
> +
> +       switch (speed) {
> +       case 10:
> +               rate = 2500000;
> +               break;
> +       case 100:
> +               rate = 25000000;
> +               break;
> +       case 1000:
> +               rate = 125000000;
> +               break;
> +       default:
> +               dev_err(dev, "unknown speed value for GMAC speed=%d", speed);
> +               return;
> +       }
> +
> +       ret = clk_set_rate(bsp_priv->clk_mac_speed, rate);
> +       if (ret)
> +               dev_err(dev, "%s: set clk_mac_speed rate %ld failed %d\n",
> +                       __func__, rate, ret);
> +}
> +
> +static const struct rk_gmac_ops rk3568_ops = {
> +       .set_to_rgmii = rk3568_set_to_rgmii,
> +       .set_to_rmii = rk3568_set_to_rmii,
> +       .set_rgmii_speed = rk3568_set_gmac_speed,
> +       .set_rmii_speed = rk3568_set_gmac_speed,
> +};
> +
> +static const struct rk_gmac_ops rk3566_ops = {
> +       .set_to_rgmii = rk3566_set_to_rgmii,
> +       .set_to_rmii = rk3566_set_to_rmii,
> +       .set_rgmii_speed = rk3568_set_gmac_speed,
> +       .set_rmii_speed = rk3568_set_gmac_speed,
> +};
> +
>  #define RV1108_GRF_GMAC_CON0           0X0900
>
>  /* RV1108_GRF_GMAC_CON0 */
> @@ -1512,6 +1652,8 @@ static const struct of_device_id rk_gmac_dwmac_match[] = {
>         { .compatible = "rockchip,rk3366-gmac", .data = &rk3366_ops },
>         { .compatible = "rockchip,rk3368-gmac", .data = &rk3368_ops },
>         { .compatible = "rockchip,rk3399-gmac", .data = &rk3399_ops },
> +       { .compatible = "rockchip,rk3568-gmac", .data = &rk3568_ops },
> +       { .compatible = "rockchip,rk3566-gmac", .data = &rk3566_ops },
>         { .compatible = "rockchip,rv1108-gmac", .data = &rv1108_ops },
>         { }
>  };
> --
> 2.30.0
>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH net-next 3/3] net: stmmac: Add RK3566/RK3568 SoC support
@ 2021-04-13 22:51     ` Peter Geis
  0 siblings, 0 replies; 25+ messages in thread
From: Peter Geis @ 2021-04-13 22:51 UTC (permalink / raw)
  To: Ezequiel Garcia
  Cc: Linux Kernel Network Developers, open list:ARM/Rockchip SoC...,
	Jose Abreu, Heiko Stuebner, David S . Miller, Jakub Kicinski,
	David Wu, kernel

On Tue, Apr 13, 2021 at 5:03 PM Ezequiel Garcia <ezequiel@collabora.com> wrote:
>
> From: David Wu <david.wu@rock-chips.com>
>
> Add constants and callback functions for the dwmac present
> on RK3566 and RK3568 SoCs. As can be seen, the base structure
> is the same, only registers and the bits in them moved slightly.
>
> RK3568 supports two MACs, and RK3566 support just one.

Tested this driver on the rk3566-quartz64.
It fails to fully probe the gmac with the following error:
[    5.711127] rk_gmac-dwmac fe010000.ethernet: IRQ eth_lpi not found
[    5.714147] rk_gmac-dwmac fe010000.ethernet: no regulator found
[    5.714766] rk_gmac-dwmac fe010000.ethernet: clock input or output? (input).
[    5.715474] rk_gmac-dwmac fe010000.ethernet: TX delay(0x4f).
[    5.716058] rk_gmac-dwmac fe010000.ethernet: RX delay(0x25).
[    5.716694] rk_gmac-dwmac fe010000.ethernet: integrated PHY? (no).
[    5.718413] rk_gmac-dwmac fe010000.ethernet: clock input from PHY
[    5.724140] rk_gmac-dwmac fe010000.ethernet: init for RGMII
[    5.726802] rk_gmac-dwmac fe010000.ethernet: Version ID not available
[    5.727525] rk_gmac-dwmac fe010000.ethernet:         DWMAC1000
[    5.728064] rk_gmac-dwmac fe010000.ethernet: DMA HW capability
register supported
[    5.729026] rk_gmac-dwmac fe010000.ethernet: Normal descriptors
[    5.729624] rk_gmac-dwmac fe010000.ethernet: Ring mode enabled
[    5.731123] rk_gmac-dwmac fe010000.ethernet: Unbalanced pm_runtime_enable!
[    5.873329] libphy: stmmac: probed
[    5.905599] rk_gmac-dwmac fe010000.ethernet: Cannot register the MDIO bus
[    5.906335] rk_gmac-dwmac fe010000.ethernet: stmmac_dvr_probe: MDIO
bus (id: 1) registration failed
[    5.914338] rk_gmac-dwmac: probe of fe010000.ethernet failed with error -5

This is due to the lack of setting has_gmac4 = true.

>
> Signed-off-by: David Wu <david.wu@rock-chips.com>
> [Ezequiel: Add rockchip,rk3566-gmac compatible string]
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
>  .../bindings/net/rockchip-dwmac.txt           |   2 +
>  .../net/ethernet/stmicro/stmmac/dwmac-rk.c    | 142 ++++++++++++++++++
>  2 files changed, 144 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/rockchip-dwmac.txt b/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
> index 3b71da7e8742..80203b16b652 100644
> --- a/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
> +++ b/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
> @@ -12,6 +12,8 @@ Required properties:
>     "rockchip,rk3366-gmac": found on RK3366 SoCs
>     "rockchip,rk3368-gmac": found on RK3368 SoCs
>     "rockchip,rk3399-gmac": found on RK3399 SoCs
> +   "rockchip,rk3566-gmac": found on RK3566 SoCs
> +   "rockchip,rk3568-gmac": found on RK3568 SoCs
>     "rockchip,rv1108-gmac": found on RV1108 SoCs
>   - reg: addresses and length of the register sets for the device.
>   - interrupts: Should contain the GMAC interrupts.
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> index d2637d83899e..096965b0fec9 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> @@ -948,6 +948,146 @@ static const struct rk_gmac_ops rk3399_ops = {
>         .set_rmii_speed = rk3399_set_rmii_speed,
>  };
>
> +#define RK3568_GRF_GMAC0_CON0          0x0380
> +#define RK3568_GRF_GMAC0_CON1          0x0384
> +#define RK3568_GRF_GMAC1_CON0          0x0388
> +#define RK3568_GRF_GMAC1_CON1          0x038c
> +
> +/* RK3568_GRF_GMAC0_CON1 && RK3568_GRF_GMAC1_CON1 */
> +#define RK3568_GMAC_PHY_INTF_SEL_RGMII \
> +               (GRF_BIT(4) | GRF_CLR_BIT(5) | GRF_CLR_BIT(6))
> +#define RK3568_GMAC_PHY_INTF_SEL_RMII  \
> +               (GRF_CLR_BIT(4) | GRF_CLR_BIT(5) | GRF_BIT(6))
> +#define RK3568_GMAC_FLOW_CTRL                  GRF_BIT(3)
> +#define RK3568_GMAC_FLOW_CTRL_CLR              GRF_CLR_BIT(3)
> +#define RK3568_GMAC_RXCLK_DLY_ENABLE           GRF_BIT(1)
> +#define RK3568_GMAC_RXCLK_DLY_DISABLE          GRF_CLR_BIT(1)
> +#define RK3568_GMAC_TXCLK_DLY_ENABLE           GRF_BIT(0)
> +#define RK3568_GMAC_TXCLK_DLY_DISABLE          GRF_CLR_BIT(0)
> +
> +/* RK3568_GRF_GMAC0_CON0 && RK3568_GRF_GMAC1_CON0 */
> +#define RK3568_GMAC_CLK_RX_DL_CFG(val) HIWORD_UPDATE(val, 0x7F, 8)
> +#define RK3568_GMAC_CLK_TX_DL_CFG(val) HIWORD_UPDATE(val, 0x7F, 0)
> +
> +static void rk3566_set_to_rgmii(struct rk_priv_data *bsp_priv,
> +                               int tx_delay, int rx_delay)
> +{
> +       struct device *dev = &bsp_priv->pdev->dev;
> +
> +       if (IS_ERR(bsp_priv->grf)) {
> +               dev_err(dev, "Missing rockchip,grf property\n");
> +               return;
> +       }
> +
> +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
> +                    RK3568_GMAC_PHY_INTF_SEL_RGMII |
> +                    RK3568_GMAC_RXCLK_DLY_ENABLE |
> +                    RK3568_GMAC_TXCLK_DLY_ENABLE);
> +
> +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON0,
> +                    RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
> +                    RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
> +}
> +
> +static void rk3566_set_to_rmii(struct rk_priv_data *bsp_priv)
> +{
> +       struct device *dev = &bsp_priv->pdev->dev;
> +
> +       if (IS_ERR(bsp_priv->grf)) {
> +               dev_err(dev, "%s: Missing rockchip,grf property\n", __func__);
> +               return;
> +       }
> +
> +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
> +                    RK3568_GMAC_PHY_INTF_SEL_RMII);
> +}
> +
> +static void rk3568_set_to_rgmii(struct rk_priv_data *bsp_priv,
> +                               int tx_delay, int rx_delay)
> +{
> +       struct device *dev = &bsp_priv->pdev->dev;
> +
> +       if (IS_ERR(bsp_priv->grf)) {
> +               dev_err(dev, "Missing rockchip,grf property\n");
> +               return;
> +       }
> +
> +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON1,
> +                    RK3568_GMAC_PHY_INTF_SEL_RGMII |
> +                    RK3568_GMAC_RXCLK_DLY_ENABLE |
> +                    RK3568_GMAC_TXCLK_DLY_ENABLE);
> +
> +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON0,
> +                    RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
> +                    RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
> +
> +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
> +                    RK3568_GMAC_PHY_INTF_SEL_RGMII |
> +                    RK3568_GMAC_RXCLK_DLY_ENABLE |
> +                    RK3568_GMAC_TXCLK_DLY_ENABLE);
> +
> +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON0,
> +                    RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
> +                    RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));

Since there are two GMACs on the rk3568, and either, or, or both may
be enabled in various configurations, we should only configure the
controller we are currently operating.

> +}
> +
> +static void rk3568_set_to_rmii(struct rk_priv_data *bsp_priv)
> +{
> +       struct device *dev = &bsp_priv->pdev->dev;
> +
> +       if (IS_ERR(bsp_priv->grf)) {
> +               dev_err(dev, "%s: Missing rockchip,grf property\n", __func__);
> +               return;
> +       }
> +
> +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON1,
> +                    RK3568_GMAC_PHY_INTF_SEL_RMII);
> +
> +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
> +                    RK3568_GMAC_PHY_INTF_SEL_RMII);

Same as above.

> +}
> +
> +static void rk3568_set_gmac_speed(struct rk_priv_data *bsp_priv, int speed)
> +{
> +       struct device *dev = &bsp_priv->pdev->dev;
> +       unsigned long rate;
> +       int ret;
> +
> +       switch (speed) {
> +       case 10:
> +               rate = 2500000;
> +               break;
> +       case 100:
> +               rate = 25000000;
> +               break;
> +       case 1000:
> +               rate = 125000000;
> +               break;
> +       default:
> +               dev_err(dev, "unknown speed value for GMAC speed=%d", speed);
> +               return;
> +       }
> +
> +       ret = clk_set_rate(bsp_priv->clk_mac_speed, rate);
> +       if (ret)
> +               dev_err(dev, "%s: set clk_mac_speed rate %ld failed %d\n",
> +                       __func__, rate, ret);
> +}
> +
> +static const struct rk_gmac_ops rk3568_ops = {
> +       .set_to_rgmii = rk3568_set_to_rgmii,
> +       .set_to_rmii = rk3568_set_to_rmii,
> +       .set_rgmii_speed = rk3568_set_gmac_speed,
> +       .set_rmii_speed = rk3568_set_gmac_speed,
> +};
> +
> +static const struct rk_gmac_ops rk3566_ops = {
> +       .set_to_rgmii = rk3566_set_to_rgmii,
> +       .set_to_rmii = rk3566_set_to_rmii,
> +       .set_rgmii_speed = rk3568_set_gmac_speed,
> +       .set_rmii_speed = rk3568_set_gmac_speed,
> +};
> +
>  #define RV1108_GRF_GMAC_CON0           0X0900
>
>  /* RV1108_GRF_GMAC_CON0 */
> @@ -1512,6 +1652,8 @@ static const struct of_device_id rk_gmac_dwmac_match[] = {
>         { .compatible = "rockchip,rk3366-gmac", .data = &rk3366_ops },
>         { .compatible = "rockchip,rk3368-gmac", .data = &rk3368_ops },
>         { .compatible = "rockchip,rk3399-gmac", .data = &rk3399_ops },
> +       { .compatible = "rockchip,rk3568-gmac", .data = &rk3568_ops },
> +       { .compatible = "rockchip,rk3566-gmac", .data = &rk3566_ops },
>         { .compatible = "rockchip,rv1108-gmac", .data = &rv1108_ops },
>         { }
>  };
> --
> 2.30.0
>

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH net-next 3/3] net: stmmac: Add RK3566/RK3568 SoC support
  2021-04-13 22:51     ` Peter Geis
  (?)
@ 2021-04-13 23:37     ` Ezequiel Garcia
  2021-04-14 11:03         ` Peter Geis
  -1 siblings, 1 reply; 25+ messages in thread
From: Ezequiel Garcia @ 2021-04-13 23:37 UTC (permalink / raw)
  To: Peter Geis
  Cc: Linux Kernel Network Developers, open list:ARM/Rockchip SoC...,
	Jose Abreu, Heiko Stuebner, David S . Miller, Jakub Kicinski,
	David Wu, kernel

On Tuesday, April 13, 2021 19:51 -03, Peter Geis <pgwipeout@gmail.com> wrote: 
 
> On Tue, Apr 13, 2021 at 5:03 PM Ezequiel Garcia <ezequiel@collabora.com> wrote:
> >
> > From: David Wu <david.wu@rock-chips.com>
> >
> > Add constants and callback functions for the dwmac present
> > on RK3566 and RK3568 SoCs. As can be seen, the base structure
> > is the same, only registers and the bits in them moved slightly.
> >
> > RK3568 supports two MACs, and RK3566 support just one.
> 
> Tested this driver on the rk3566-quartz64.
> It fails to fully probe the gmac with the following error:
> [    5.711127] rk_gmac-dwmac fe010000.ethernet: IRQ eth_lpi not found
> [    5.714147] rk_gmac-dwmac fe010000.ethernet: no regulator found
> [    5.714766] rk_gmac-dwmac fe010000.ethernet: clock input or output? (input).
> [    5.715474] rk_gmac-dwmac fe010000.ethernet: TX delay(0x4f).
> [    5.716058] rk_gmac-dwmac fe010000.ethernet: RX delay(0x25).
> [    5.716694] rk_gmac-dwmac fe010000.ethernet: integrated PHY? (no).
> [    5.718413] rk_gmac-dwmac fe010000.ethernet: clock input from PHY
> [    5.724140] rk_gmac-dwmac fe010000.ethernet: init for RGMII
> [    5.726802] rk_gmac-dwmac fe010000.ethernet: Version ID not available
> [    5.727525] rk_gmac-dwmac fe010000.ethernet:         DWMAC1000
> [    5.728064] rk_gmac-dwmac fe010000.ethernet: DMA HW capability
> register supported
> [    5.729026] rk_gmac-dwmac fe010000.ethernet: Normal descriptors
> [    5.729624] rk_gmac-dwmac fe010000.ethernet: Ring mode enabled
> [    5.731123] rk_gmac-dwmac fe010000.ethernet: Unbalanced pm_runtime_enable!
> [    5.873329] libphy: stmmac: probed
> [    5.905599] rk_gmac-dwmac fe010000.ethernet: Cannot register the MDIO bus
> [    5.906335] rk_gmac-dwmac fe010000.ethernet: stmmac_dvr_probe: MDIO
> bus (id: 1) registration failed
> [    5.914338] rk_gmac-dwmac: probe of fe010000.ethernet failed with error -5
> 
> This is due to the lack of setting has_gmac4 = true.
> 

You are probably missing a "snps,dwmac-4.20a" in your compatible string, i.e.:
  compatible = "rockchip,rk3566-gmac", "snps,dwmac-4.20a";

This seems to be common practice, judging from arch/arm/boot/dts/stm32***.

Let me know how that goes!
Ezequiel

> >
> > Signed-off-by: David Wu <david.wu@rock-chips.com>
> > [Ezequiel: Add rockchip,rk3566-gmac compatible string]
> > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> > ---
> >  .../bindings/net/rockchip-dwmac.txt           |   2 +
> >  .../net/ethernet/stmicro/stmmac/dwmac-rk.c    | 142 ++++++++++++++++++
> >  2 files changed, 144 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/net/rockchip-dwmac.txt b/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
> > index 3b71da7e8742..80203b16b652 100644
> > --- a/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
> > +++ b/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
> > @@ -12,6 +12,8 @@ Required properties:
> >     "rockchip,rk3366-gmac": found on RK3366 SoCs
> >     "rockchip,rk3368-gmac": found on RK3368 SoCs
> >     "rockchip,rk3399-gmac": found on RK3399 SoCs
> > +   "rockchip,rk3566-gmac": found on RK3566 SoCs
> > +   "rockchip,rk3568-gmac": found on RK3568 SoCs
> >     "rockchip,rv1108-gmac": found on RV1108 SoCs
> >   - reg: addresses and length of the register sets for the device.
> >   - interrupts: Should contain the GMAC interrupts.
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > index d2637d83899e..096965b0fec9 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > @@ -948,6 +948,146 @@ static const struct rk_gmac_ops rk3399_ops = {
> >         .set_rmii_speed = rk3399_set_rmii_speed,
> >  };
> >
> > +#define RK3568_GRF_GMAC0_CON0          0x0380
> > +#define RK3568_GRF_GMAC0_CON1          0x0384
> > +#define RK3568_GRF_GMAC1_CON0          0x0388
> > +#define RK3568_GRF_GMAC1_CON1          0x038c
> > +
> > +/* RK3568_GRF_GMAC0_CON1 && RK3568_GRF_GMAC1_CON1 */
> > +#define RK3568_GMAC_PHY_INTF_SEL_RGMII \
> > +               (GRF_BIT(4) | GRF_CLR_BIT(5) | GRF_CLR_BIT(6))
> > +#define RK3568_GMAC_PHY_INTF_SEL_RMII  \
> > +               (GRF_CLR_BIT(4) | GRF_CLR_BIT(5) | GRF_BIT(6))
> > +#define RK3568_GMAC_FLOW_CTRL                  GRF_BIT(3)
> > +#define RK3568_GMAC_FLOW_CTRL_CLR              GRF_CLR_BIT(3)
> > +#define RK3568_GMAC_RXCLK_DLY_ENABLE           GRF_BIT(1)
> > +#define RK3568_GMAC_RXCLK_DLY_DISABLE          GRF_CLR_BIT(1)
> > +#define RK3568_GMAC_TXCLK_DLY_ENABLE           GRF_BIT(0)
> > +#define RK3568_GMAC_TXCLK_DLY_DISABLE          GRF_CLR_BIT(0)
> > +
> > +/* RK3568_GRF_GMAC0_CON0 && RK3568_GRF_GMAC1_CON0 */
> > +#define RK3568_GMAC_CLK_RX_DL_CFG(val) HIWORD_UPDATE(val, 0x7F, 8)
> > +#define RK3568_GMAC_CLK_TX_DL_CFG(val) HIWORD_UPDATE(val, 0x7F, 0)
> > +
> > +static void rk3566_set_to_rgmii(struct rk_priv_data *bsp_priv,
> > +                               int tx_delay, int rx_delay)
> > +{
> > +       struct device *dev = &bsp_priv->pdev->dev;
> > +
> > +       if (IS_ERR(bsp_priv->grf)) {
> > +               dev_err(dev, "Missing rockchip,grf property\n");
> > +               return;
> > +       }
> > +
> > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
> > +                    RK3568_GMAC_PHY_INTF_SEL_RGMII |
> > +                    RK3568_GMAC_RXCLK_DLY_ENABLE |
> > +                    RK3568_GMAC_TXCLK_DLY_ENABLE);
> > +
> > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON0,
> > +                    RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
> > +                    RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
> > +}
> > +
> > +static void rk3566_set_to_rmii(struct rk_priv_data *bsp_priv)
> > +{
> > +       struct device *dev = &bsp_priv->pdev->dev;
> > +
> > +       if (IS_ERR(bsp_priv->grf)) {
> > +               dev_err(dev, "%s: Missing rockchip,grf property\n", __func__);
> > +               return;
> > +       }
> > +
> > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
> > +                    RK3568_GMAC_PHY_INTF_SEL_RMII);
> > +}
> > +
> > +static void rk3568_set_to_rgmii(struct rk_priv_data *bsp_priv,
> > +                               int tx_delay, int rx_delay)
> > +{
> > +       struct device *dev = &bsp_priv->pdev->dev;
> > +
> > +       if (IS_ERR(bsp_priv->grf)) {
> > +               dev_err(dev, "Missing rockchip,grf property\n");
> > +               return;
> > +       }
> > +
> > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON1,
> > +                    RK3568_GMAC_PHY_INTF_SEL_RGMII |
> > +                    RK3568_GMAC_RXCLK_DLY_ENABLE |
> > +                    RK3568_GMAC_TXCLK_DLY_ENABLE);
> > +
> > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON0,
> > +                    RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
> > +                    RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
> > +
> > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
> > +                    RK3568_GMAC_PHY_INTF_SEL_RGMII |
> > +                    RK3568_GMAC_RXCLK_DLY_ENABLE |
> > +                    RK3568_GMAC_TXCLK_DLY_ENABLE);
> > +
> > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON0,
> > +                    RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
> > +                    RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
> 
> Since there are two GMACs on the rk3568, and either, or, or both may
> be enabled in various configurations, we should only configure the
> controller we are currently operating.
> 
> > +}
> > +
> > +static void rk3568_set_to_rmii(struct rk_priv_data *bsp_priv)
> > +{
> > +       struct device *dev = &bsp_priv->pdev->dev;
> > +
> > +       if (IS_ERR(bsp_priv->grf)) {
> > +               dev_err(dev, "%s: Missing rockchip,grf property\n", __func__);
> > +               return;
> > +       }
> > +
> > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON1,
> > +                    RK3568_GMAC_PHY_INTF_SEL_RMII);
> > +
> > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
> > +                    RK3568_GMAC_PHY_INTF_SEL_RMII);
> 
> Same as above.
> 
> > +}
> > +
> > +static void rk3568_set_gmac_speed(struct rk_priv_data *bsp_priv, int speed)
> > +{
> > +       struct device *dev = &bsp_priv->pdev->dev;
> > +       unsigned long rate;
> > +       int ret;
> > +
> > +       switch (speed) {
> > +       case 10:
> > +               rate = 2500000;
> > +               break;
> > +       case 100:
> > +               rate = 25000000;
> > +               break;
> > +       case 1000:
> > +               rate = 125000000;
> > +               break;
> > +       default:
> > +               dev_err(dev, "unknown speed value for GMAC speed=%d", speed);
> > +               return;
> > +       }
> > +
> > +       ret = clk_set_rate(bsp_priv->clk_mac_speed, rate);
> > +       if (ret)
> > +               dev_err(dev, "%s: set clk_mac_speed rate %ld failed %d\n",
> > +                       __func__, rate, ret);
> > +}
> > +
> > +static const struct rk_gmac_ops rk3568_ops = {
> > +       .set_to_rgmii = rk3568_set_to_rgmii,
> > +       .set_to_rmii = rk3568_set_to_rmii,
> > +       .set_rgmii_speed = rk3568_set_gmac_speed,
> > +       .set_rmii_speed = rk3568_set_gmac_speed,
> > +};
> > +
> > +static const struct rk_gmac_ops rk3566_ops = {
> > +       .set_to_rgmii = rk3566_set_to_rgmii,
> > +       .set_to_rmii = rk3566_set_to_rmii,
> > +       .set_rgmii_speed = rk3568_set_gmac_speed,
> > +       .set_rmii_speed = rk3568_set_gmac_speed,
> > +};
> > +
> >  #define RV1108_GRF_GMAC_CON0           0X0900
> >
> >  /* RV1108_GRF_GMAC_CON0 */
> > @@ -1512,6 +1652,8 @@ static const struct of_device_id rk_gmac_dwmac_match[] = {
> >         { .compatible = "rockchip,rk3366-gmac", .data = &rk3366_ops },
> >         { .compatible = "rockchip,rk3368-gmac", .data = &rk3368_ops },
> >         { .compatible = "rockchip,rk3399-gmac", .data = &rk3399_ops },
> > +       { .compatible = "rockchip,rk3568-gmac", .data = &rk3568_ops },
> > +       { .compatible = "rockchip,rk3566-gmac", .data = &rk3566_ops },
> >         { .compatible = "rockchip,rv1108-gmac", .data = &rv1108_ops },
> >         { }
> >  };
> > --
> > 2.30.0
> >


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH net-next 3/3] net: stmmac: Add RK3566/RK3568 SoC support
  2021-04-13 23:37     ` Ezequiel Garcia
@ 2021-04-14 11:03         ` Peter Geis
  0 siblings, 0 replies; 25+ messages in thread
From: Peter Geis @ 2021-04-14 11:03 UTC (permalink / raw)
  To: Ezequiel Garcia
  Cc: Linux Kernel Network Developers, open list:ARM/Rockchip SoC...,
	Jose Abreu, Heiko Stuebner, David S . Miller, Jakub Kicinski,
	David Wu, kernel

On Tue, Apr 13, 2021 at 7:37 PM Ezequiel Garcia <ezequiel@collabora.com> wrote:
>
> On Tuesday, April 13, 2021 19:51 -03, Peter Geis <pgwipeout@gmail.com> wrote:
>
> > On Tue, Apr 13, 2021 at 5:03 PM Ezequiel Garcia <ezequiel@collabora.com> wrote:
> > >
> > > From: David Wu <david.wu@rock-chips.com>
> > >
> > > Add constants and callback functions for the dwmac present
> > > on RK3566 and RK3568 SoCs. As can be seen, the base structure
> > > is the same, only registers and the bits in them moved slightly.
> > >
> > > RK3568 supports two MACs, and RK3566 support just one.
> >
> > Tested this driver on the rk3566-quartz64.
> > It fails to fully probe the gmac with the following error:
> > [    5.711127] rk_gmac-dwmac fe010000.ethernet: IRQ eth_lpi not found
> > [    5.714147] rk_gmac-dwmac fe010000.ethernet: no regulator found
> > [    5.714766] rk_gmac-dwmac fe010000.ethernet: clock input or output? (input).
> > [    5.715474] rk_gmac-dwmac fe010000.ethernet: TX delay(0x4f).
> > [    5.716058] rk_gmac-dwmac fe010000.ethernet: RX delay(0x25).
> > [    5.716694] rk_gmac-dwmac fe010000.ethernet: integrated PHY? (no).
> > [    5.718413] rk_gmac-dwmac fe010000.ethernet: clock input from PHY
> > [    5.724140] rk_gmac-dwmac fe010000.ethernet: init for RGMII
> > [    5.726802] rk_gmac-dwmac fe010000.ethernet: Version ID not available
> > [    5.727525] rk_gmac-dwmac fe010000.ethernet:         DWMAC1000
> > [    5.728064] rk_gmac-dwmac fe010000.ethernet: DMA HW capability
> > register supported
> > [    5.729026] rk_gmac-dwmac fe010000.ethernet: Normal descriptors
> > [    5.729624] rk_gmac-dwmac fe010000.ethernet: Ring mode enabled
> > [    5.731123] rk_gmac-dwmac fe010000.ethernet: Unbalanced pm_runtime_enable!
> > [    5.873329] libphy: stmmac: probed
> > [    5.905599] rk_gmac-dwmac fe010000.ethernet: Cannot register the MDIO bus
> > [    5.906335] rk_gmac-dwmac fe010000.ethernet: stmmac_dvr_probe: MDIO
> > bus (id: 1) registration failed
> > [    5.914338] rk_gmac-dwmac: probe of fe010000.ethernet failed with error -5
> >
> > This is due to the lack of setting has_gmac4 = true.
> >
>
> You are probably missing a "snps,dwmac-4.20a" in your compatible string, i.e.:
>   compatible = "rockchip,rk3566-gmac", "snps,dwmac-4.20a";

Ah yes, I had disabled that because my variant took a different path.
Thanks!

>
> This seems to be common practice, judging from arch/arm/boot/dts/stm32***.
>
> Let me know how that goes!
> Ezequiel
>
> > >
> > > Signed-off-by: David Wu <david.wu@rock-chips.com>
> > > [Ezequiel: Add rockchip,rk3566-gmac compatible string]
> > > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> > > ---
> > >  .../bindings/net/rockchip-dwmac.txt           |   2 +
> > >  .../net/ethernet/stmicro/stmmac/dwmac-rk.c    | 142 ++++++++++++++++++
> > >  2 files changed, 144 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/net/rockchip-dwmac.txt b/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
> > > index 3b71da7e8742..80203b16b652 100644
> > > --- a/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
> > > +++ b/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
> > > @@ -12,6 +12,8 @@ Required properties:
> > >     "rockchip,rk3366-gmac": found on RK3366 SoCs
> > >     "rockchip,rk3368-gmac": found on RK3368 SoCs
> > >     "rockchip,rk3399-gmac": found on RK3399 SoCs
> > > +   "rockchip,rk3566-gmac": found on RK3566 SoCs
> > > +   "rockchip,rk3568-gmac": found on RK3568 SoCs
> > >     "rockchip,rv1108-gmac": found on RV1108 SoCs
> > >   - reg: addresses and length of the register sets for the device.
> > >   - interrupts: Should contain the GMAC interrupts.
> > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > > index d2637d83899e..096965b0fec9 100644
> > > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > > @@ -948,6 +948,146 @@ static const struct rk_gmac_ops rk3399_ops = {
> > >         .set_rmii_speed = rk3399_set_rmii_speed,
> > >  };
> > >
> > > +#define RK3568_GRF_GMAC0_CON0          0x0380
> > > +#define RK3568_GRF_GMAC0_CON1          0x0384
> > > +#define RK3568_GRF_GMAC1_CON0          0x0388
> > > +#define RK3568_GRF_GMAC1_CON1          0x038c
> > > +
> > > +/* RK3568_GRF_GMAC0_CON1 && RK3568_GRF_GMAC1_CON1 */
> > > +#define RK3568_GMAC_PHY_INTF_SEL_RGMII \
> > > +               (GRF_BIT(4) | GRF_CLR_BIT(5) | GRF_CLR_BIT(6))
> > > +#define RK3568_GMAC_PHY_INTF_SEL_RMII  \
> > > +               (GRF_CLR_BIT(4) | GRF_CLR_BIT(5) | GRF_BIT(6))
> > > +#define RK3568_GMAC_FLOW_CTRL                  GRF_BIT(3)
> > > +#define RK3568_GMAC_FLOW_CTRL_CLR              GRF_CLR_BIT(3)
> > > +#define RK3568_GMAC_RXCLK_DLY_ENABLE           GRF_BIT(1)
> > > +#define RK3568_GMAC_RXCLK_DLY_DISABLE          GRF_CLR_BIT(1)
> > > +#define RK3568_GMAC_TXCLK_DLY_ENABLE           GRF_BIT(0)
> > > +#define RK3568_GMAC_TXCLK_DLY_DISABLE          GRF_CLR_BIT(0)
> > > +
> > > +/* RK3568_GRF_GMAC0_CON0 && RK3568_GRF_GMAC1_CON0 */
> > > +#define RK3568_GMAC_CLK_RX_DL_CFG(val) HIWORD_UPDATE(val, 0x7F, 8)
> > > +#define RK3568_GMAC_CLK_TX_DL_CFG(val) HIWORD_UPDATE(val, 0x7F, 0)
> > > +
> > > +static void rk3566_set_to_rgmii(struct rk_priv_data *bsp_priv,
> > > +                               int tx_delay, int rx_delay)
> > > +{
> > > +       struct device *dev = &bsp_priv->pdev->dev;
> > > +
> > > +       if (IS_ERR(bsp_priv->grf)) {
> > > +               dev_err(dev, "Missing rockchip,grf property\n");
> > > +               return;
> > > +       }
> > > +
> > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
> > > +                    RK3568_GMAC_PHY_INTF_SEL_RGMII |
> > > +                    RK3568_GMAC_RXCLK_DLY_ENABLE |
> > > +                    RK3568_GMAC_TXCLK_DLY_ENABLE);
> > > +
> > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON0,
> > > +                    RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
> > > +                    RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
> > > +}
> > > +
> > > +static void rk3566_set_to_rmii(struct rk_priv_data *bsp_priv)
> > > +{
> > > +       struct device *dev = &bsp_priv->pdev->dev;
> > > +
> > > +       if (IS_ERR(bsp_priv->grf)) {
> > > +               dev_err(dev, "%s: Missing rockchip,grf property\n", __func__);
> > > +               return;
> > > +       }
> > > +
> > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
> > > +                    RK3568_GMAC_PHY_INTF_SEL_RMII);
> > > +}
> > > +
> > > +static void rk3568_set_to_rgmii(struct rk_priv_data *bsp_priv,
> > > +                               int tx_delay, int rx_delay)
> > > +{
> > > +       struct device *dev = &bsp_priv->pdev->dev;
> > > +
> > > +       if (IS_ERR(bsp_priv->grf)) {
> > > +               dev_err(dev, "Missing rockchip,grf property\n");
> > > +               return;
> > > +       }
> > > +
> > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON1,
> > > +                    RK3568_GMAC_PHY_INTF_SEL_RGMII |
> > > +                    RK3568_GMAC_RXCLK_DLY_ENABLE |
> > > +                    RK3568_GMAC_TXCLK_DLY_ENABLE);
> > > +
> > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON0,
> > > +                    RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
> > > +                    RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
> > > +
> > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
> > > +                    RK3568_GMAC_PHY_INTF_SEL_RGMII |
> > > +                    RK3568_GMAC_RXCLK_DLY_ENABLE |
> > > +                    RK3568_GMAC_TXCLK_DLY_ENABLE);
> > > +
> > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON0,
> > > +                    RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
> > > +                    RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
> >
> > Since there are two GMACs on the rk3568, and either, or, or both may
> > be enabled in various configurations, we should only configure the
> > controller we are currently operating.

Perhaps we should have match data (such as reg = <0>, or against the
address) to identify the individual controllers.

> >
> > > +}
> > > +
> > > +static void rk3568_set_to_rmii(struct rk_priv_data *bsp_priv)
> > > +{
> > > +       struct device *dev = &bsp_priv->pdev->dev;
> > > +
> > > +       if (IS_ERR(bsp_priv->grf)) {
> > > +               dev_err(dev, "%s: Missing rockchip,grf property\n", __func__);
> > > +               return;
> > > +       }
> > > +
> > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON1,
> > > +                    RK3568_GMAC_PHY_INTF_SEL_RMII);
> > > +
> > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
> > > +                    RK3568_GMAC_PHY_INTF_SEL_RMII);
> >
> > Same as above.
> >
> > > +}
> > > +
> > > +static void rk3568_set_gmac_speed(struct rk_priv_data *bsp_priv, int speed)
> > > +{
> > > +       struct device *dev = &bsp_priv->pdev->dev;
> > > +       unsigned long rate;
> > > +       int ret;
> > > +
> > > +       switch (speed) {
> > > +       case 10:
> > > +               rate = 2500000;
> > > +               break;
> > > +       case 100:
> > > +               rate = 25000000;
> > > +               break;
> > > +       case 1000:
> > > +               rate = 125000000;
> > > +               break;
> > > +       default:
> > > +               dev_err(dev, "unknown speed value for GMAC speed=%d", speed);
> > > +               return;
> > > +       }
> > > +
> > > +       ret = clk_set_rate(bsp_priv->clk_mac_speed, rate);
> > > +       if (ret)
> > > +               dev_err(dev, "%s: set clk_mac_speed rate %ld failed %d\n",
> > > +                       __func__, rate, ret);
> > > +}
> > > +
> > > +static const struct rk_gmac_ops rk3568_ops = {
> > > +       .set_to_rgmii = rk3568_set_to_rgmii,
> > > +       .set_to_rmii = rk3568_set_to_rmii,
> > > +       .set_rgmii_speed = rk3568_set_gmac_speed,
> > > +       .set_rmii_speed = rk3568_set_gmac_speed,
> > > +};
> > > +
> > > +static const struct rk_gmac_ops rk3566_ops = {
> > > +       .set_to_rgmii = rk3566_set_to_rgmii,
> > > +       .set_to_rmii = rk3566_set_to_rmii,
> > > +       .set_rgmii_speed = rk3568_set_gmac_speed,
> > > +       .set_rmii_speed = rk3568_set_gmac_speed,
> > > +};
> > > +
> > >  #define RV1108_GRF_GMAC_CON0           0X0900
> > >
> > >  /* RV1108_GRF_GMAC_CON0 */
> > > @@ -1512,6 +1652,8 @@ static const struct of_device_id rk_gmac_dwmac_match[] = {
> > >         { .compatible = "rockchip,rk3366-gmac", .data = &rk3366_ops },
> > >         { .compatible = "rockchip,rk3368-gmac", .data = &rk3368_ops },
> > >         { .compatible = "rockchip,rk3399-gmac", .data = &rk3399_ops },
> > > +       { .compatible = "rockchip,rk3568-gmac", .data = &rk3568_ops },
> > > +       { .compatible = "rockchip,rk3566-gmac", .data = &rk3566_ops },
> > >         { .compatible = "rockchip,rv1108-gmac", .data = &rv1108_ops },
> > >         { }
> > >  };
> > > --
> > > 2.30.0
> > >
>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH net-next 3/3] net: stmmac: Add RK3566/RK3568 SoC support
@ 2021-04-14 11:03         ` Peter Geis
  0 siblings, 0 replies; 25+ messages in thread
From: Peter Geis @ 2021-04-14 11:03 UTC (permalink / raw)
  To: Ezequiel Garcia
  Cc: Linux Kernel Network Developers, open list:ARM/Rockchip SoC...,
	Jose Abreu, Heiko Stuebner, David S . Miller, Jakub Kicinski,
	David Wu, kernel

On Tue, Apr 13, 2021 at 7:37 PM Ezequiel Garcia <ezequiel@collabora.com> wrote:
>
> On Tuesday, April 13, 2021 19:51 -03, Peter Geis <pgwipeout@gmail.com> wrote:
>
> > On Tue, Apr 13, 2021 at 5:03 PM Ezequiel Garcia <ezequiel@collabora.com> wrote:
> > >
> > > From: David Wu <david.wu@rock-chips.com>
> > >
> > > Add constants and callback functions for the dwmac present
> > > on RK3566 and RK3568 SoCs. As can be seen, the base structure
> > > is the same, only registers and the bits in them moved slightly.
> > >
> > > RK3568 supports two MACs, and RK3566 support just one.
> >
> > Tested this driver on the rk3566-quartz64.
> > It fails to fully probe the gmac with the following error:
> > [    5.711127] rk_gmac-dwmac fe010000.ethernet: IRQ eth_lpi not found
> > [    5.714147] rk_gmac-dwmac fe010000.ethernet: no regulator found
> > [    5.714766] rk_gmac-dwmac fe010000.ethernet: clock input or output? (input).
> > [    5.715474] rk_gmac-dwmac fe010000.ethernet: TX delay(0x4f).
> > [    5.716058] rk_gmac-dwmac fe010000.ethernet: RX delay(0x25).
> > [    5.716694] rk_gmac-dwmac fe010000.ethernet: integrated PHY? (no).
> > [    5.718413] rk_gmac-dwmac fe010000.ethernet: clock input from PHY
> > [    5.724140] rk_gmac-dwmac fe010000.ethernet: init for RGMII
> > [    5.726802] rk_gmac-dwmac fe010000.ethernet: Version ID not available
> > [    5.727525] rk_gmac-dwmac fe010000.ethernet:         DWMAC1000
> > [    5.728064] rk_gmac-dwmac fe010000.ethernet: DMA HW capability
> > register supported
> > [    5.729026] rk_gmac-dwmac fe010000.ethernet: Normal descriptors
> > [    5.729624] rk_gmac-dwmac fe010000.ethernet: Ring mode enabled
> > [    5.731123] rk_gmac-dwmac fe010000.ethernet: Unbalanced pm_runtime_enable!
> > [    5.873329] libphy: stmmac: probed
> > [    5.905599] rk_gmac-dwmac fe010000.ethernet: Cannot register the MDIO bus
> > [    5.906335] rk_gmac-dwmac fe010000.ethernet: stmmac_dvr_probe: MDIO
> > bus (id: 1) registration failed
> > [    5.914338] rk_gmac-dwmac: probe of fe010000.ethernet failed with error -5
> >
> > This is due to the lack of setting has_gmac4 = true.
> >
>
> You are probably missing a "snps,dwmac-4.20a" in your compatible string, i.e.:
>   compatible = "rockchip,rk3566-gmac", "snps,dwmac-4.20a";

Ah yes, I had disabled that because my variant took a different path.
Thanks!

>
> This seems to be common practice, judging from arch/arm/boot/dts/stm32***.
>
> Let me know how that goes!
> Ezequiel
>
> > >
> > > Signed-off-by: David Wu <david.wu@rock-chips.com>
> > > [Ezequiel: Add rockchip,rk3566-gmac compatible string]
> > > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> > > ---
> > >  .../bindings/net/rockchip-dwmac.txt           |   2 +
> > >  .../net/ethernet/stmicro/stmmac/dwmac-rk.c    | 142 ++++++++++++++++++
> > >  2 files changed, 144 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/net/rockchip-dwmac.txt b/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
> > > index 3b71da7e8742..80203b16b652 100644
> > > --- a/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
> > > +++ b/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
> > > @@ -12,6 +12,8 @@ Required properties:
> > >     "rockchip,rk3366-gmac": found on RK3366 SoCs
> > >     "rockchip,rk3368-gmac": found on RK3368 SoCs
> > >     "rockchip,rk3399-gmac": found on RK3399 SoCs
> > > +   "rockchip,rk3566-gmac": found on RK3566 SoCs
> > > +   "rockchip,rk3568-gmac": found on RK3568 SoCs
> > >     "rockchip,rv1108-gmac": found on RV1108 SoCs
> > >   - reg: addresses and length of the register sets for the device.
> > >   - interrupts: Should contain the GMAC interrupts.
> > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > > index d2637d83899e..096965b0fec9 100644
> > > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > > @@ -948,6 +948,146 @@ static const struct rk_gmac_ops rk3399_ops = {
> > >         .set_rmii_speed = rk3399_set_rmii_speed,
> > >  };
> > >
> > > +#define RK3568_GRF_GMAC0_CON0          0x0380
> > > +#define RK3568_GRF_GMAC0_CON1          0x0384
> > > +#define RK3568_GRF_GMAC1_CON0          0x0388
> > > +#define RK3568_GRF_GMAC1_CON1          0x038c
> > > +
> > > +/* RK3568_GRF_GMAC0_CON1 && RK3568_GRF_GMAC1_CON1 */
> > > +#define RK3568_GMAC_PHY_INTF_SEL_RGMII \
> > > +               (GRF_BIT(4) | GRF_CLR_BIT(5) | GRF_CLR_BIT(6))
> > > +#define RK3568_GMAC_PHY_INTF_SEL_RMII  \
> > > +               (GRF_CLR_BIT(4) | GRF_CLR_BIT(5) | GRF_BIT(6))
> > > +#define RK3568_GMAC_FLOW_CTRL                  GRF_BIT(3)
> > > +#define RK3568_GMAC_FLOW_CTRL_CLR              GRF_CLR_BIT(3)
> > > +#define RK3568_GMAC_RXCLK_DLY_ENABLE           GRF_BIT(1)
> > > +#define RK3568_GMAC_RXCLK_DLY_DISABLE          GRF_CLR_BIT(1)
> > > +#define RK3568_GMAC_TXCLK_DLY_ENABLE           GRF_BIT(0)
> > > +#define RK3568_GMAC_TXCLK_DLY_DISABLE          GRF_CLR_BIT(0)
> > > +
> > > +/* RK3568_GRF_GMAC0_CON0 && RK3568_GRF_GMAC1_CON0 */
> > > +#define RK3568_GMAC_CLK_RX_DL_CFG(val) HIWORD_UPDATE(val, 0x7F, 8)
> > > +#define RK3568_GMAC_CLK_TX_DL_CFG(val) HIWORD_UPDATE(val, 0x7F, 0)
> > > +
> > > +static void rk3566_set_to_rgmii(struct rk_priv_data *bsp_priv,
> > > +                               int tx_delay, int rx_delay)
> > > +{
> > > +       struct device *dev = &bsp_priv->pdev->dev;
> > > +
> > > +       if (IS_ERR(bsp_priv->grf)) {
> > > +               dev_err(dev, "Missing rockchip,grf property\n");
> > > +               return;
> > > +       }
> > > +
> > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
> > > +                    RK3568_GMAC_PHY_INTF_SEL_RGMII |
> > > +                    RK3568_GMAC_RXCLK_DLY_ENABLE |
> > > +                    RK3568_GMAC_TXCLK_DLY_ENABLE);
> > > +
> > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON0,
> > > +                    RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
> > > +                    RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
> > > +}
> > > +
> > > +static void rk3566_set_to_rmii(struct rk_priv_data *bsp_priv)
> > > +{
> > > +       struct device *dev = &bsp_priv->pdev->dev;
> > > +
> > > +       if (IS_ERR(bsp_priv->grf)) {
> > > +               dev_err(dev, "%s: Missing rockchip,grf property\n", __func__);
> > > +               return;
> > > +       }
> > > +
> > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
> > > +                    RK3568_GMAC_PHY_INTF_SEL_RMII);
> > > +}
> > > +
> > > +static void rk3568_set_to_rgmii(struct rk_priv_data *bsp_priv,
> > > +                               int tx_delay, int rx_delay)
> > > +{
> > > +       struct device *dev = &bsp_priv->pdev->dev;
> > > +
> > > +       if (IS_ERR(bsp_priv->grf)) {
> > > +               dev_err(dev, "Missing rockchip,grf property\n");
> > > +               return;
> > > +       }
> > > +
> > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON1,
> > > +                    RK3568_GMAC_PHY_INTF_SEL_RGMII |
> > > +                    RK3568_GMAC_RXCLK_DLY_ENABLE |
> > > +                    RK3568_GMAC_TXCLK_DLY_ENABLE);
> > > +
> > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON0,
> > > +                    RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
> > > +                    RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
> > > +
> > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
> > > +                    RK3568_GMAC_PHY_INTF_SEL_RGMII |
> > > +                    RK3568_GMAC_RXCLK_DLY_ENABLE |
> > > +                    RK3568_GMAC_TXCLK_DLY_ENABLE);
> > > +
> > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON0,
> > > +                    RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
> > > +                    RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
> >
> > Since there are two GMACs on the rk3568, and either, or, or both may
> > be enabled in various configurations, we should only configure the
> > controller we are currently operating.

Perhaps we should have match data (such as reg = <0>, or against the
address) to identify the individual controllers.

> >
> > > +}
> > > +
> > > +static void rk3568_set_to_rmii(struct rk_priv_data *bsp_priv)
> > > +{
> > > +       struct device *dev = &bsp_priv->pdev->dev;
> > > +
> > > +       if (IS_ERR(bsp_priv->grf)) {
> > > +               dev_err(dev, "%s: Missing rockchip,grf property\n", __func__);
> > > +               return;
> > > +       }
> > > +
> > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON1,
> > > +                    RK3568_GMAC_PHY_INTF_SEL_RMII);
> > > +
> > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
> > > +                    RK3568_GMAC_PHY_INTF_SEL_RMII);
> >
> > Same as above.
> >
> > > +}
> > > +
> > > +static void rk3568_set_gmac_speed(struct rk_priv_data *bsp_priv, int speed)
> > > +{
> > > +       struct device *dev = &bsp_priv->pdev->dev;
> > > +       unsigned long rate;
> > > +       int ret;
> > > +
> > > +       switch (speed) {
> > > +       case 10:
> > > +               rate = 2500000;
> > > +               break;
> > > +       case 100:
> > > +               rate = 25000000;
> > > +               break;
> > > +       case 1000:
> > > +               rate = 125000000;
> > > +               break;
> > > +       default:
> > > +               dev_err(dev, "unknown speed value for GMAC speed=%d", speed);
> > > +               return;
> > > +       }
> > > +
> > > +       ret = clk_set_rate(bsp_priv->clk_mac_speed, rate);
> > > +       if (ret)
> > > +               dev_err(dev, "%s: set clk_mac_speed rate %ld failed %d\n",
> > > +                       __func__, rate, ret);
> > > +}
> > > +
> > > +static const struct rk_gmac_ops rk3568_ops = {
> > > +       .set_to_rgmii = rk3568_set_to_rgmii,
> > > +       .set_to_rmii = rk3568_set_to_rmii,
> > > +       .set_rgmii_speed = rk3568_set_gmac_speed,
> > > +       .set_rmii_speed = rk3568_set_gmac_speed,
> > > +};
> > > +
> > > +static const struct rk_gmac_ops rk3566_ops = {
> > > +       .set_to_rgmii = rk3566_set_to_rgmii,
> > > +       .set_to_rmii = rk3566_set_to_rmii,
> > > +       .set_rgmii_speed = rk3568_set_gmac_speed,
> > > +       .set_rmii_speed = rk3568_set_gmac_speed,
> > > +};
> > > +
> > >  #define RV1108_GRF_GMAC_CON0           0X0900
> > >
> > >  /* RV1108_GRF_GMAC_CON0 */
> > > @@ -1512,6 +1652,8 @@ static const struct of_device_id rk_gmac_dwmac_match[] = {
> > >         { .compatible = "rockchip,rk3366-gmac", .data = &rk3366_ops },
> > >         { .compatible = "rockchip,rk3368-gmac", .data = &rk3368_ops },
> > >         { .compatible = "rockchip,rk3399-gmac", .data = &rk3399_ops },
> > > +       { .compatible = "rockchip,rk3568-gmac", .data = &rk3568_ops },
> > > +       { .compatible = "rockchip,rk3566-gmac", .data = &rk3566_ops },
> > >         { .compatible = "rockchip,rv1108-gmac", .data = &rv1108_ops },
> > >         { }
> > >  };
> > > --
> > > 2.30.0
> > >
>

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH net-next 3/3] net: stmmac: Add RK3566/RK3568 SoC support
  2021-04-14 11:03         ` Peter Geis
@ 2021-04-14 11:15           ` Heiko Stübner
  -1 siblings, 0 replies; 25+ messages in thread
From: Heiko Stübner @ 2021-04-14 11:15 UTC (permalink / raw)
  To: Ezequiel Garcia, Peter Geis
  Cc: Linux Kernel Network Developers, open list:ARM/Rockchip SoC...,
	Jose Abreu, David S . Miller, Jakub Kicinski, David Wu, kernel

Am Mittwoch, 14. April 2021, 13:03:25 CEST schrieb Peter Geis:
> On Tue, Apr 13, 2021 at 7:37 PM Ezequiel Garcia <ezequiel@collabora.com> wrote:
> > > > +static void rk3566_set_to_rmii(struct rk_priv_data *bsp_priv)
> > > > +{
> > > > +       struct device *dev = &bsp_priv->pdev->dev;
> > > > +
> > > > +       if (IS_ERR(bsp_priv->grf)) {
> > > > +               dev_err(dev, "%s: Missing rockchip,grf property\n", __func__);
> > > > +               return;
> > > > +       }
> > > > +
> > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
> > > > +                    RK3568_GMAC_PHY_INTF_SEL_RMII);
> > > > +}
> > > > +
> > > > +static void rk3568_set_to_rgmii(struct rk_priv_data *bsp_priv,
> > > > +                               int tx_delay, int rx_delay)
> > > > +{
> > > > +       struct device *dev = &bsp_priv->pdev->dev;
> > > > +
> > > > +       if (IS_ERR(bsp_priv->grf)) {
> > > > +               dev_err(dev, "Missing rockchip,grf property\n");
> > > > +               return;
> > > > +       }
> > > > +
> > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON1,
> > > > +                    RK3568_GMAC_PHY_INTF_SEL_RGMII |
> > > > +                    RK3568_GMAC_RXCLK_DLY_ENABLE |
> > > > +                    RK3568_GMAC_TXCLK_DLY_ENABLE);
> > > > +
> > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON0,
> > > > +                    RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
> > > > +                    RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
> > > > +
> > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
> > > > +                    RK3568_GMAC_PHY_INTF_SEL_RGMII |
> > > > +                    RK3568_GMAC_RXCLK_DLY_ENABLE |
> > > > +                    RK3568_GMAC_TXCLK_DLY_ENABLE);
> > > > +
> > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON0,
> > > > +                    RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
> > > > +                    RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
> > >
> > > Since there are two GMACs on the rk3568, and either, or, or both may
> > > be enabled in various configurations, we should only configure the
> > > controller we are currently operating.
> 
> Perhaps we should have match data (such as reg = <0>, or against the
> address) to identify the individual controllers.

Hmm, "reg" will be used by the actual mmio address of the controller,
so matching against that should be the way I guess.

We're already doing something similar for dsi:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c#n1170


Heiko






^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH net-next 3/3] net: stmmac: Add RK3566/RK3568 SoC support
@ 2021-04-14 11:15           ` Heiko Stübner
  0 siblings, 0 replies; 25+ messages in thread
From: Heiko Stübner @ 2021-04-14 11:15 UTC (permalink / raw)
  To: Ezequiel Garcia, Peter Geis
  Cc: Linux Kernel Network Developers, open list:ARM/Rockchip SoC...,
	Jose Abreu, David S . Miller, Jakub Kicinski, David Wu, kernel

Am Mittwoch, 14. April 2021, 13:03:25 CEST schrieb Peter Geis:
> On Tue, Apr 13, 2021 at 7:37 PM Ezequiel Garcia <ezequiel@collabora.com> wrote:
> > > > +static void rk3566_set_to_rmii(struct rk_priv_data *bsp_priv)
> > > > +{
> > > > +       struct device *dev = &bsp_priv->pdev->dev;
> > > > +
> > > > +       if (IS_ERR(bsp_priv->grf)) {
> > > > +               dev_err(dev, "%s: Missing rockchip,grf property\n", __func__);
> > > > +               return;
> > > > +       }
> > > > +
> > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
> > > > +                    RK3568_GMAC_PHY_INTF_SEL_RMII);
> > > > +}
> > > > +
> > > > +static void rk3568_set_to_rgmii(struct rk_priv_data *bsp_priv,
> > > > +                               int tx_delay, int rx_delay)
> > > > +{
> > > > +       struct device *dev = &bsp_priv->pdev->dev;
> > > > +
> > > > +       if (IS_ERR(bsp_priv->grf)) {
> > > > +               dev_err(dev, "Missing rockchip,grf property\n");
> > > > +               return;
> > > > +       }
> > > > +
> > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON1,
> > > > +                    RK3568_GMAC_PHY_INTF_SEL_RGMII |
> > > > +                    RK3568_GMAC_RXCLK_DLY_ENABLE |
> > > > +                    RK3568_GMAC_TXCLK_DLY_ENABLE);
> > > > +
> > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON0,
> > > > +                    RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
> > > > +                    RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
> > > > +
> > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
> > > > +                    RK3568_GMAC_PHY_INTF_SEL_RGMII |
> > > > +                    RK3568_GMAC_RXCLK_DLY_ENABLE |
> > > > +                    RK3568_GMAC_TXCLK_DLY_ENABLE);
> > > > +
> > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON0,
> > > > +                    RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
> > > > +                    RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
> > >
> > > Since there are two GMACs on the rk3568, and either, or, or both may
> > > be enabled in various configurations, we should only configure the
> > > controller we are currently operating.
> 
> Perhaps we should have match data (such as reg = <0>, or against the
> address) to identify the individual controllers.

Hmm, "reg" will be used by the actual mmio address of the controller,
so matching against that should be the way I guess.

We're already doing something similar for dsi:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c#n1170


Heiko






_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH net-next 3/3] net: stmmac: Add RK3566/RK3568 SoC support
  2021-04-14 11:15           ` Heiko Stübner
@ 2021-04-14 13:48             ` Ezequiel Garcia
  -1 siblings, 0 replies; 25+ messages in thread
From: Ezequiel Garcia @ 2021-04-14 13:48 UTC (permalink / raw)
  To: Heiko Stübner, Peter Geis
  Cc: Linux Kernel Network Developers, open list:ARM/Rockchip SoC...,
	Jose Abreu, David S . Miller, Jakub Kicinski, David Wu, kernel

Hi Peter, Heiko,

On Wed, 2021-04-14 at 13:15 +0200, Heiko Stübner wrote:
> Am Mittwoch, 14. April 2021, 13:03:25 CEST schrieb Peter Geis:
> > On Tue, Apr 13, 2021 at 7:37 PM Ezequiel Garcia <ezequiel@collabora.com> wrote:
> > > > > +static void rk3566_set_to_rmii(struct rk_priv_data *bsp_priv)
> > > > > +{
> > > > > +       struct device *dev = &bsp_priv->pdev->dev;
> > > > > +
> > > > > +       if (IS_ERR(bsp_priv->grf)) {
> > > > > +               dev_err(dev, "%s: Missing rockchip,grf property\n", __func__);
> > > > > +               return;
> > > > > +       }
> > > > > +
> > > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
> > > > > +                    RK3568_GMAC_PHY_INTF_SEL_RMII);
> > > > > +}
> > > > > +
> > > > > +static void rk3568_set_to_rgmii(struct rk_priv_data *bsp_priv,
> > > > > +                               int tx_delay, int rx_delay)
> > > > > +{
> > > > > +       struct device *dev = &bsp_priv->pdev->dev;
> > > > > +
> > > > > +       if (IS_ERR(bsp_priv->grf)) {
> > > > > +               dev_err(dev, "Missing rockchip,grf property\n");
> > > > > +               return;
> > > > > +       }
> > > > > +
> > > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON1,
> > > > > +                    RK3568_GMAC_PHY_INTF_SEL_RGMII |
> > > > > +                    RK3568_GMAC_RXCLK_DLY_ENABLE |
> > > > > +                    RK3568_GMAC_TXCLK_DLY_ENABLE);
> > > > > +
> > > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON0,
> > > > > +                    RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
> > > > > +                    RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
> > > > > +
> > > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
> > > > > +                    RK3568_GMAC_PHY_INTF_SEL_RGMII |
> > > > > +                    RK3568_GMAC_RXCLK_DLY_ENABLE |
> > > > > +                    RK3568_GMAC_TXCLK_DLY_ENABLE);
> > > > > +
> > > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON0,
> > > > > +                    RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
> > > > > +                    RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
> > > > 
> > > > Since there are two GMACs on the rk3568, and either, or, or both may
> > > > be enabled in various configurations, we should only configure the
> > > > controller we are currently operating.
> > 
> > Perhaps we should have match data (such as reg = <0>, or against the
> > address) to identify the individual controllers.
> 
> Hmm, "reg" will be used by the actual mmio address of the controller,
> so matching against that should be the way I guess.
> 
> We're already doing something similar for dsi:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c#n1170
> 

I have to admit, I'm not a fan of hardcoding the registers in the kernel.

David Wu solved this in the downstream kernel by using bus_id,
which parses the devicetree "ethernet@0" node, i.e.:

  plat->bus_id = of_alias_get_id(np, "ethernet");

I'm inclined for this solution. Maybe Jose can suggest how to approach it?

Thanks!
Ezequiel



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH net-next 3/3] net: stmmac: Add RK3566/RK3568 SoC support
@ 2021-04-14 13:48             ` Ezequiel Garcia
  0 siblings, 0 replies; 25+ messages in thread
From: Ezequiel Garcia @ 2021-04-14 13:48 UTC (permalink / raw)
  To: Heiko Stübner, Peter Geis
  Cc: Linux Kernel Network Developers, open list:ARM/Rockchip SoC...,
	Jose Abreu, David S . Miller, Jakub Kicinski, David Wu, kernel

Hi Peter, Heiko,

On Wed, 2021-04-14 at 13:15 +0200, Heiko Stübner wrote:
> Am Mittwoch, 14. April 2021, 13:03:25 CEST schrieb Peter Geis:
> > On Tue, Apr 13, 2021 at 7:37 PM Ezequiel Garcia <ezequiel@collabora.com> wrote:
> > > > > +static void rk3566_set_to_rmii(struct rk_priv_data *bsp_priv)
> > > > > +{
> > > > > +       struct device *dev = &bsp_priv->pdev->dev;
> > > > > +
> > > > > +       if (IS_ERR(bsp_priv->grf)) {
> > > > > +               dev_err(dev, "%s: Missing rockchip,grf property\n", __func__);
> > > > > +               return;
> > > > > +       }
> > > > > +
> > > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
> > > > > +                    RK3568_GMAC_PHY_INTF_SEL_RMII);
> > > > > +}
> > > > > +
> > > > > +static void rk3568_set_to_rgmii(struct rk_priv_data *bsp_priv,
> > > > > +                               int tx_delay, int rx_delay)
> > > > > +{
> > > > > +       struct device *dev = &bsp_priv->pdev->dev;
> > > > > +
> > > > > +       if (IS_ERR(bsp_priv->grf)) {
> > > > > +               dev_err(dev, "Missing rockchip,grf property\n");
> > > > > +               return;
> > > > > +       }
> > > > > +
> > > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON1,
> > > > > +                    RK3568_GMAC_PHY_INTF_SEL_RGMII |
> > > > > +                    RK3568_GMAC_RXCLK_DLY_ENABLE |
> > > > > +                    RK3568_GMAC_TXCLK_DLY_ENABLE);
> > > > > +
> > > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON0,
> > > > > +                    RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
> > > > > +                    RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
> > > > > +
> > > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
> > > > > +                    RK3568_GMAC_PHY_INTF_SEL_RGMII |
> > > > > +                    RK3568_GMAC_RXCLK_DLY_ENABLE |
> > > > > +                    RK3568_GMAC_TXCLK_DLY_ENABLE);
> > > > > +
> > > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON0,
> > > > > +                    RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
> > > > > +                    RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
> > > > 
> > > > Since there are two GMACs on the rk3568, and either, or, or both may
> > > > be enabled in various configurations, we should only configure the
> > > > controller we are currently operating.
> > 
> > Perhaps we should have match data (such as reg = <0>, or against the
> > address) to identify the individual controllers.
> 
> Hmm, "reg" will be used by the actual mmio address of the controller,
> so matching against that should be the way I guess.
> 
> We're already doing something similar for dsi:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c#n1170
> 

I have to admit, I'm not a fan of hardcoding the registers in the kernel.

David Wu solved this in the downstream kernel by using bus_id,
which parses the devicetree "ethernet@0" node, i.e.:

  plat->bus_id = of_alias_get_id(np, "ethernet");

I'm inclined for this solution. Maybe Jose can suggest how to approach it?

Thanks!
Ezequiel



_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH net-next 3/3] net: stmmac: Add RK3566/RK3568 SoC support
  2021-04-14 11:03         ` Peter Geis
@ 2021-04-14 13:48           ` Ezequiel Garcia
  -1 siblings, 0 replies; 25+ messages in thread
From: Ezequiel Garcia @ 2021-04-14 13:48 UTC (permalink / raw)
  To: Peter Geis
  Cc: Linux Kernel Network Developers, open list:ARM/Rockchip SoC...,
	Jose Abreu, Heiko Stuebner, David S . Miller, Jakub Kicinski,
	David Wu, kernel

On Wed, 2021-04-14 at 07:03 -0400, Peter Geis wrote:
> On Tue, Apr 13, 2021 at 7:37 PM Ezequiel Garcia <ezequiel@collabora.com> wrote:
> > 
> > On Tuesday, April 13, 2021 19:51 -03, Peter Geis <pgwipeout@gmail.com> wrote:
> > 
> > > On Tue, Apr 13, 2021 at 5:03 PM Ezequiel Garcia <ezequiel@collabora.com> wrote:
> > > > 
> > > > From: David Wu <david.wu@rock-chips.com>
> > > > 
> > > > Add constants and callback functions for the dwmac present
> > > > on RK3566 and RK3568 SoCs. As can be seen, the base structure
> > > > is the same, only registers and the bits in them moved slightly.
> > > > 
> > > > RK3568 supports two MACs, and RK3566 support just one.
> > > 
> > > Tested this driver on the rk3566-quartz64.
> > > It fails to fully probe the gmac with the following error:
> > > [    5.711127] rk_gmac-dwmac fe010000.ethernet: IRQ eth_lpi not found
> > > [    5.714147] rk_gmac-dwmac fe010000.ethernet: no regulator found
> > > [    5.714766] rk_gmac-dwmac fe010000.ethernet: clock input or output? (input).
> > > [    5.715474] rk_gmac-dwmac fe010000.ethernet: TX delay(0x4f).
> > > [    5.716058] rk_gmac-dwmac fe010000.ethernet: RX delay(0x25).
> > > [    5.716694] rk_gmac-dwmac fe010000.ethernet: integrated PHY? (no).
> > > [    5.718413] rk_gmac-dwmac fe010000.ethernet: clock input from PHY
> > > [    5.724140] rk_gmac-dwmac fe010000.ethernet: init for RGMII
> > > [    5.726802] rk_gmac-dwmac fe010000.ethernet: Version ID not available
> > > [    5.727525] rk_gmac-dwmac fe010000.ethernet:         DWMAC1000
> > > [    5.728064] rk_gmac-dwmac fe010000.ethernet: DMA HW capability
> > > register supported
> > > [    5.729026] rk_gmac-dwmac fe010000.ethernet: Normal descriptors
> > > [    5.729624] rk_gmac-dwmac fe010000.ethernet: Ring mode enabled
> > > [    5.731123] rk_gmac-dwmac fe010000.ethernet: Unbalanced pm_runtime_enable!
> > > [    5.873329] libphy: stmmac: probed
> > > [    5.905599] rk_gmac-dwmac fe010000.ethernet: Cannot register the MDIO bus
> > > [    5.906335] rk_gmac-dwmac fe010000.ethernet: stmmac_dvr_probe: MDIO
> > > bus (id: 1) registration failed
> > > [    5.914338] rk_gmac-dwmac: probe of fe010000.ethernet failed with error -5
> > > 
> > > This is due to the lack of setting has_gmac4 = true.
> > > 
> > 
> > You are probably missing a "snps,dwmac-4.20a" in your compatible string, i.e.:
> >   compatible = "rockchip,rk3566-gmac", "snps,dwmac-4.20a";
> 
> Ah yes, I had disabled that because my variant took a different path.
> Thanks!
> 

Is that a Tested-by :-) ?

Thanks,
Ezequiel


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH net-next 3/3] net: stmmac: Add RK3566/RK3568 SoC support
@ 2021-04-14 13:48           ` Ezequiel Garcia
  0 siblings, 0 replies; 25+ messages in thread
From: Ezequiel Garcia @ 2021-04-14 13:48 UTC (permalink / raw)
  To: Peter Geis
  Cc: Linux Kernel Network Developers, open list:ARM/Rockchip SoC...,
	Jose Abreu, Heiko Stuebner, David S . Miller, Jakub Kicinski,
	David Wu, kernel

On Wed, 2021-04-14 at 07:03 -0400, Peter Geis wrote:
> On Tue, Apr 13, 2021 at 7:37 PM Ezequiel Garcia <ezequiel@collabora.com> wrote:
> > 
> > On Tuesday, April 13, 2021 19:51 -03, Peter Geis <pgwipeout@gmail.com> wrote:
> > 
> > > On Tue, Apr 13, 2021 at 5:03 PM Ezequiel Garcia <ezequiel@collabora.com> wrote:
> > > > 
> > > > From: David Wu <david.wu@rock-chips.com>
> > > > 
> > > > Add constants and callback functions for the dwmac present
> > > > on RK3566 and RK3568 SoCs. As can be seen, the base structure
> > > > is the same, only registers and the bits in them moved slightly.
> > > > 
> > > > RK3568 supports two MACs, and RK3566 support just one.
> > > 
> > > Tested this driver on the rk3566-quartz64.
> > > It fails to fully probe the gmac with the following error:
> > > [    5.711127] rk_gmac-dwmac fe010000.ethernet: IRQ eth_lpi not found
> > > [    5.714147] rk_gmac-dwmac fe010000.ethernet: no regulator found
> > > [    5.714766] rk_gmac-dwmac fe010000.ethernet: clock input or output? (input).
> > > [    5.715474] rk_gmac-dwmac fe010000.ethernet: TX delay(0x4f).
> > > [    5.716058] rk_gmac-dwmac fe010000.ethernet: RX delay(0x25).
> > > [    5.716694] rk_gmac-dwmac fe010000.ethernet: integrated PHY? (no).
> > > [    5.718413] rk_gmac-dwmac fe010000.ethernet: clock input from PHY
> > > [    5.724140] rk_gmac-dwmac fe010000.ethernet: init for RGMII
> > > [    5.726802] rk_gmac-dwmac fe010000.ethernet: Version ID not available
> > > [    5.727525] rk_gmac-dwmac fe010000.ethernet:         DWMAC1000
> > > [    5.728064] rk_gmac-dwmac fe010000.ethernet: DMA HW capability
> > > register supported
> > > [    5.729026] rk_gmac-dwmac fe010000.ethernet: Normal descriptors
> > > [    5.729624] rk_gmac-dwmac fe010000.ethernet: Ring mode enabled
> > > [    5.731123] rk_gmac-dwmac fe010000.ethernet: Unbalanced pm_runtime_enable!
> > > [    5.873329] libphy: stmmac: probed
> > > [    5.905599] rk_gmac-dwmac fe010000.ethernet: Cannot register the MDIO bus
> > > [    5.906335] rk_gmac-dwmac fe010000.ethernet: stmmac_dvr_probe: MDIO
> > > bus (id: 1) registration failed
> > > [    5.914338] rk_gmac-dwmac: probe of fe010000.ethernet failed with error -5
> > > 
> > > This is due to the lack of setting has_gmac4 = true.
> > > 
> > 
> > You are probably missing a "snps,dwmac-4.20a" in your compatible string, i.e.:
> >   compatible = "rockchip,rk3566-gmac", "snps,dwmac-4.20a";
> 
> Ah yes, I had disabled that because my variant took a different path.
> Thanks!
> 

Is that a Tested-by :-) ?

Thanks,
Ezequiel


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH net-next 3/3] net: stmmac: Add RK3566/RK3568 SoC support
  2021-04-14 13:48             ` Ezequiel Garcia
@ 2021-04-14 16:33               ` Chen-Yu Tsai
  -1 siblings, 0 replies; 25+ messages in thread
From: Chen-Yu Tsai @ 2021-04-14 16:33 UTC (permalink / raw)
  To: Ezequiel Garcia
  Cc: Heiko Stübner, Peter Geis, Linux Kernel Network Developers,
	open list:ARM/Rockchip SoC...,
	Jose Abreu, David S . Miller, Jakub Kicinski, David Wu, kernel

On Thu, Apr 15, 2021 at 12:14 AM Ezequiel Garcia <ezequiel@collabora.com> wrote:
>
> Hi Peter, Heiko,
>
> On Wed, 2021-04-14 at 13:15 +0200, Heiko Stübner wrote:
> > Am Mittwoch, 14. April 2021, 13:03:25 CEST schrieb Peter Geis:
> > > On Tue, Apr 13, 2021 at 7:37 PM Ezequiel Garcia <ezequiel@collabora.com> wrote:
> > > > > > +static void rk3566_set_to_rmii(struct rk_priv_data *bsp_priv)
> > > > > > +{
> > > > > > +       struct device *dev = &bsp_priv->pdev->dev;
> > > > > > +
> > > > > > +       if (IS_ERR(bsp_priv->grf)) {
> > > > > > +               dev_err(dev, "%s: Missing rockchip,grf property\n", __func__);
> > > > > > +               return;
> > > > > > +       }
> > > > > > +
> > > > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
> > > > > > +                    RK3568_GMAC_PHY_INTF_SEL_RMII);
> > > > > > +}
> > > > > > +
> > > > > > +static void rk3568_set_to_rgmii(struct rk_priv_data *bsp_priv,
> > > > > > +                               int tx_delay, int rx_delay)
> > > > > > +{
> > > > > > +       struct device *dev = &bsp_priv->pdev->dev;
> > > > > > +
> > > > > > +       if (IS_ERR(bsp_priv->grf)) {
> > > > > > +               dev_err(dev, "Missing rockchip,grf property\n");
> > > > > > +               return;
> > > > > > +       }
> > > > > > +
> > > > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON1,
> > > > > > +                    RK3568_GMAC_PHY_INTF_SEL_RGMII |
> > > > > > +                    RK3568_GMAC_RXCLK_DLY_ENABLE |
> > > > > > +                    RK3568_GMAC_TXCLK_DLY_ENABLE);
> > > > > > +
> > > > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON0,
> > > > > > +                    RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
> > > > > > +                    RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
> > > > > > +
> > > > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
> > > > > > +                    RK3568_GMAC_PHY_INTF_SEL_RGMII |
> > > > > > +                    RK3568_GMAC_RXCLK_DLY_ENABLE |
> > > > > > +                    RK3568_GMAC_TXCLK_DLY_ENABLE);
> > > > > > +
> > > > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON0,
> > > > > > +                    RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
> > > > > > +                    RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
> > > > >
> > > > > Since there are two GMACs on the rk3568, and either, or, or both may
> > > > > be enabled in various configurations, we should only configure the
> > > > > controller we are currently operating.
> > >
> > > Perhaps we should have match data (such as reg = <0>, or against the
> > > address) to identify the individual controllers.
> >
> > Hmm, "reg" will be used by the actual mmio address of the controller,
> > so matching against that should be the way I guess.
> >
> > We're already doing something similar for dsi:
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c#n1170
> >
>
> I have to admit, I'm not a fan of hardcoding the registers in the kernel.
>
> David Wu solved this in the downstream kernel by using bus_id,
> which parses the devicetree "ethernet@0" node, i.e.:
>
>   plat->bus_id = of_alias_get_id(np, "ethernet");

What happens when one adds another ethernet controller (USB or PCIe) to
the board and wants to change the numbering order?

Or maybe only the second ethernet controller is routed on some board
and the submitter / vendor wants that one to be ethernet0, because
it's the only usable controller?

This seems even more fragile than hardcoding the registers.

Regards
ChenYu

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH net-next 3/3] net: stmmac: Add RK3566/RK3568 SoC support
@ 2021-04-14 16:33               ` Chen-Yu Tsai
  0 siblings, 0 replies; 25+ messages in thread
From: Chen-Yu Tsai @ 2021-04-14 16:33 UTC (permalink / raw)
  To: Ezequiel Garcia
  Cc: Heiko Stübner, Peter Geis, Linux Kernel Network Developers,
	open list:ARM/Rockchip SoC...,
	Jose Abreu, David S . Miller, Jakub Kicinski, David Wu, kernel

On Thu, Apr 15, 2021 at 12:14 AM Ezequiel Garcia <ezequiel@collabora.com> wrote:
>
> Hi Peter, Heiko,
>
> On Wed, 2021-04-14 at 13:15 +0200, Heiko Stübner wrote:
> > Am Mittwoch, 14. April 2021, 13:03:25 CEST schrieb Peter Geis:
> > > On Tue, Apr 13, 2021 at 7:37 PM Ezequiel Garcia <ezequiel@collabora.com> wrote:
> > > > > > +static void rk3566_set_to_rmii(struct rk_priv_data *bsp_priv)
> > > > > > +{
> > > > > > +       struct device *dev = &bsp_priv->pdev->dev;
> > > > > > +
> > > > > > +       if (IS_ERR(bsp_priv->grf)) {
> > > > > > +               dev_err(dev, "%s: Missing rockchip,grf property\n", __func__);
> > > > > > +               return;
> > > > > > +       }
> > > > > > +
> > > > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
> > > > > > +                    RK3568_GMAC_PHY_INTF_SEL_RMII);
> > > > > > +}
> > > > > > +
> > > > > > +static void rk3568_set_to_rgmii(struct rk_priv_data *bsp_priv,
> > > > > > +                               int tx_delay, int rx_delay)
> > > > > > +{
> > > > > > +       struct device *dev = &bsp_priv->pdev->dev;
> > > > > > +
> > > > > > +       if (IS_ERR(bsp_priv->grf)) {
> > > > > > +               dev_err(dev, "Missing rockchip,grf property\n");
> > > > > > +               return;
> > > > > > +       }
> > > > > > +
> > > > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON1,
> > > > > > +                    RK3568_GMAC_PHY_INTF_SEL_RGMII |
> > > > > > +                    RK3568_GMAC_RXCLK_DLY_ENABLE |
> > > > > > +                    RK3568_GMAC_TXCLK_DLY_ENABLE);
> > > > > > +
> > > > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON0,
> > > > > > +                    RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
> > > > > > +                    RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
> > > > > > +
> > > > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
> > > > > > +                    RK3568_GMAC_PHY_INTF_SEL_RGMII |
> > > > > > +                    RK3568_GMAC_RXCLK_DLY_ENABLE |
> > > > > > +                    RK3568_GMAC_TXCLK_DLY_ENABLE);
> > > > > > +
> > > > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON0,
> > > > > > +                    RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
> > > > > > +                    RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
> > > > >
> > > > > Since there are two GMACs on the rk3568, and either, or, or both may
> > > > > be enabled in various configurations, we should only configure the
> > > > > controller we are currently operating.
> > >
> > > Perhaps we should have match data (such as reg = <0>, or against the
> > > address) to identify the individual controllers.
> >
> > Hmm, "reg" will be used by the actual mmio address of the controller,
> > so matching against that should be the way I guess.
> >
> > We're already doing something similar for dsi:
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c#n1170
> >
>
> I have to admit, I'm not a fan of hardcoding the registers in the kernel.
>
> David Wu solved this in the downstream kernel by using bus_id,
> which parses the devicetree "ethernet@0" node, i.e.:
>
>   plat->bus_id = of_alias_get_id(np, "ethernet");

What happens when one adds another ethernet controller (USB or PCIe) to
the board and wants to change the numbering order?

Or maybe only the second ethernet controller is routed on some board
and the submitter / vendor wants that one to be ethernet0, because
it's the only usable controller?

This seems even more fragile than hardcoding the registers.

Regards
ChenYu

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH net-next 3/3] net: stmmac: Add RK3566/RK3568 SoC support
  2021-04-14 16:33               ` Chen-Yu Tsai
@ 2021-04-14 16:35                 ` Heiko Stübner
  -1 siblings, 0 replies; 25+ messages in thread
From: Heiko Stübner @ 2021-04-14 16:35 UTC (permalink / raw)
  To: Ezequiel Garcia, Chen-Yu Tsai
  Cc: Peter Geis, Linux Kernel Network Developers,
	open list:ARM/Rockchip SoC...,
	Jose Abreu, David S . Miller, Jakub Kicinski, David Wu, kernel

Am Mittwoch, 14. April 2021, 18:33:12 CEST schrieb Chen-Yu Tsai:
> On Thu, Apr 15, 2021 at 12:14 AM Ezequiel Garcia <ezequiel@collabora.com> wrote:
> >
> > Hi Peter, Heiko,
> >
> > On Wed, 2021-04-14 at 13:15 +0200, Heiko Stübner wrote:
> > > Am Mittwoch, 14. April 2021, 13:03:25 CEST schrieb Peter Geis:
> > > > On Tue, Apr 13, 2021 at 7:37 PM Ezequiel Garcia <ezequiel@collabora.com> wrote:
> > > > > > > +static void rk3566_set_to_rmii(struct rk_priv_data *bsp_priv)
> > > > > > > +{
> > > > > > > +       struct device *dev = &bsp_priv->pdev->dev;
> > > > > > > +
> > > > > > > +       if (IS_ERR(bsp_priv->grf)) {
> > > > > > > +               dev_err(dev, "%s: Missing rockchip,grf property\n", __func__);
> > > > > > > +               return;
> > > > > > > +       }
> > > > > > > +
> > > > > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
> > > > > > > +                    RK3568_GMAC_PHY_INTF_SEL_RMII);
> > > > > > > +}
> > > > > > > +
> > > > > > > +static void rk3568_set_to_rgmii(struct rk_priv_data *bsp_priv,
> > > > > > > +                               int tx_delay, int rx_delay)
> > > > > > > +{
> > > > > > > +       struct device *dev = &bsp_priv->pdev->dev;
> > > > > > > +
> > > > > > > +       if (IS_ERR(bsp_priv->grf)) {
> > > > > > > +               dev_err(dev, "Missing rockchip,grf property\n");
> > > > > > > +               return;
> > > > > > > +       }
> > > > > > > +
> > > > > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON1,
> > > > > > > +                    RK3568_GMAC_PHY_INTF_SEL_RGMII |
> > > > > > > +                    RK3568_GMAC_RXCLK_DLY_ENABLE |
> > > > > > > +                    RK3568_GMAC_TXCLK_DLY_ENABLE);
> > > > > > > +
> > > > > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON0,
> > > > > > > +                    RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
> > > > > > > +                    RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
> > > > > > > +
> > > > > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
> > > > > > > +                    RK3568_GMAC_PHY_INTF_SEL_RGMII |
> > > > > > > +                    RK3568_GMAC_RXCLK_DLY_ENABLE |
> > > > > > > +                    RK3568_GMAC_TXCLK_DLY_ENABLE);
> > > > > > > +
> > > > > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON0,
> > > > > > > +                    RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
> > > > > > > +                    RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
> > > > > >
> > > > > > Since there are two GMACs on the rk3568, and either, or, or both may
> > > > > > be enabled in various configurations, we should only configure the
> > > > > > controller we are currently operating.
> > > >
> > > > Perhaps we should have match data (such as reg = <0>, or against the
> > > > address) to identify the individual controllers.
> > >
> > > Hmm, "reg" will be used by the actual mmio address of the controller,
> > > so matching against that should be the way I guess.
> > >
> > > We're already doing something similar for dsi:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c#n1170
> > >
> >
> > I have to admit, I'm not a fan of hardcoding the registers in the kernel.
> >
> > David Wu solved this in the downstream kernel by using bus_id,
> > which parses the devicetree "ethernet@0" node, i.e.:
> >
> >   plat->bus_id = of_alias_get_id(np, "ethernet");
> 
> What happens when one adds another ethernet controller (USB or PCIe) to
> the board and wants to change the numbering order?
> 
> Or maybe only the second ethernet controller is routed on some board
> and the submitter / vendor wants that one to be ethernet0, because
> it's the only usable controller?

Which matches a discussion I had with Arnd about the mmc numbering.
I.e. there the first mmc device is supposed to be mmc0 and so on,
without gaps - for probably the same reasons.


> 
> This seems even more fragile than hardcoding the registers.
> 
> Regards
> ChenYu
> 





^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH net-next 3/3] net: stmmac: Add RK3566/RK3568 SoC support
@ 2021-04-14 16:35                 ` Heiko Stübner
  0 siblings, 0 replies; 25+ messages in thread
From: Heiko Stübner @ 2021-04-14 16:35 UTC (permalink / raw)
  To: Ezequiel Garcia, Chen-Yu Tsai
  Cc: Peter Geis, Linux Kernel Network Developers,
	open list:ARM/Rockchip SoC...,
	Jose Abreu, David S . Miller, Jakub Kicinski, David Wu, kernel

Am Mittwoch, 14. April 2021, 18:33:12 CEST schrieb Chen-Yu Tsai:
> On Thu, Apr 15, 2021 at 12:14 AM Ezequiel Garcia <ezequiel@collabora.com> wrote:
> >
> > Hi Peter, Heiko,
> >
> > On Wed, 2021-04-14 at 13:15 +0200, Heiko Stübner wrote:
> > > Am Mittwoch, 14. April 2021, 13:03:25 CEST schrieb Peter Geis:
> > > > On Tue, Apr 13, 2021 at 7:37 PM Ezequiel Garcia <ezequiel@collabora.com> wrote:
> > > > > > > +static void rk3566_set_to_rmii(struct rk_priv_data *bsp_priv)
> > > > > > > +{
> > > > > > > +       struct device *dev = &bsp_priv->pdev->dev;
> > > > > > > +
> > > > > > > +       if (IS_ERR(bsp_priv->grf)) {
> > > > > > > +               dev_err(dev, "%s: Missing rockchip,grf property\n", __func__);
> > > > > > > +               return;
> > > > > > > +       }
> > > > > > > +
> > > > > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
> > > > > > > +                    RK3568_GMAC_PHY_INTF_SEL_RMII);
> > > > > > > +}
> > > > > > > +
> > > > > > > +static void rk3568_set_to_rgmii(struct rk_priv_data *bsp_priv,
> > > > > > > +                               int tx_delay, int rx_delay)
> > > > > > > +{
> > > > > > > +       struct device *dev = &bsp_priv->pdev->dev;
> > > > > > > +
> > > > > > > +       if (IS_ERR(bsp_priv->grf)) {
> > > > > > > +               dev_err(dev, "Missing rockchip,grf property\n");
> > > > > > > +               return;
> > > > > > > +       }
> > > > > > > +
> > > > > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON1,
> > > > > > > +                    RK3568_GMAC_PHY_INTF_SEL_RGMII |
> > > > > > > +                    RK3568_GMAC_RXCLK_DLY_ENABLE |
> > > > > > > +                    RK3568_GMAC_TXCLK_DLY_ENABLE);
> > > > > > > +
> > > > > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON0,
> > > > > > > +                    RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
> > > > > > > +                    RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
> > > > > > > +
> > > > > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
> > > > > > > +                    RK3568_GMAC_PHY_INTF_SEL_RGMII |
> > > > > > > +                    RK3568_GMAC_RXCLK_DLY_ENABLE |
> > > > > > > +                    RK3568_GMAC_TXCLK_DLY_ENABLE);
> > > > > > > +
> > > > > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON0,
> > > > > > > +                    RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
> > > > > > > +                    RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
> > > > > >
> > > > > > Since there are two GMACs on the rk3568, and either, or, or both may
> > > > > > be enabled in various configurations, we should only configure the
> > > > > > controller we are currently operating.
> > > >
> > > > Perhaps we should have match data (such as reg = <0>, or against the
> > > > address) to identify the individual controllers.
> > >
> > > Hmm, "reg" will be used by the actual mmio address of the controller,
> > > so matching against that should be the way I guess.
> > >
> > > We're already doing something similar for dsi:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c#n1170
> > >
> >
> > I have to admit, I'm not a fan of hardcoding the registers in the kernel.
> >
> > David Wu solved this in the downstream kernel by using bus_id,
> > which parses the devicetree "ethernet@0" node, i.e.:
> >
> >   plat->bus_id = of_alias_get_id(np, "ethernet");
> 
> What happens when one adds another ethernet controller (USB or PCIe) to
> the board and wants to change the numbering order?
> 
> Or maybe only the second ethernet controller is routed on some board
> and the submitter / vendor wants that one to be ethernet0, because
> it's the only usable controller?

Which matches a discussion I had with Arnd about the mmc numbering.
I.e. there the first mmc device is supposed to be mmc0 and so on,
without gaps - for probably the same reasons.


> 
> This seems even more fragile than hardcoding the registers.
> 
> Regards
> ChenYu
> 





_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH net-next 3/3] net: stmmac: Add RK3566/RK3568 SoC support
  2021-04-14 16:35                 ` Heiko Stübner
@ 2021-04-14 16:44                   ` Ezequiel Garcia
  -1 siblings, 0 replies; 25+ messages in thread
From: Ezequiel Garcia @ 2021-04-14 16:44 UTC (permalink / raw)
  To: Heiko Stübner, Chen-Yu Tsai
  Cc: Peter Geis, Linux Kernel Network Developers,
	open list:ARM/Rockchip SoC...,
	Jose Abreu, David S . Miller, Jakub Kicinski, David Wu, kernel

On Wed, 2021-04-14 at 18:35 +0200, Heiko Stübner wrote:
> Am Mittwoch, 14. April 2021, 18:33:12 CEST schrieb Chen-Yu Tsai:
> > On Thu, Apr 15, 2021 at 12:14 AM Ezequiel Garcia <ezequiel@collabora.com> wrote:
> > > 
> > > Hi Peter, Heiko,
> > > 
> > > On Wed, 2021-04-14 at 13:15 +0200, Heiko Stübner wrote:
> > > > Am Mittwoch, 14. April 2021, 13:03:25 CEST schrieb Peter Geis:
> > > > > On Tue, Apr 13, 2021 at 7:37 PM Ezequiel Garcia <ezequiel@collabora.com> wrote:
> > > > > > > > +static void rk3566_set_to_rmii(struct rk_priv_data *bsp_priv)
> > > > > > > > +{
> > > > > > > > +       struct device *dev = &bsp_priv->pdev->dev;
> > > > > > > > +
> > > > > > > > +       if (IS_ERR(bsp_priv->grf)) {
> > > > > > > > +               dev_err(dev, "%s: Missing rockchip,grf property\n", __func__);
> > > > > > > > +               return;
> > > > > > > > +       }
> > > > > > > > +
> > > > > > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
> > > > > > > > +                    RK3568_GMAC_PHY_INTF_SEL_RMII);
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static void rk3568_set_to_rgmii(struct rk_priv_data *bsp_priv,
> > > > > > > > +                               int tx_delay, int rx_delay)
> > > > > > > > +{
> > > > > > > > +       struct device *dev = &bsp_priv->pdev->dev;
> > > > > > > > +
> > > > > > > > +       if (IS_ERR(bsp_priv->grf)) {
> > > > > > > > +               dev_err(dev, "Missing rockchip,grf property\n");
> > > > > > > > +               return;
> > > > > > > > +       }
> > > > > > > > +
> > > > > > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON1,
> > > > > > > > +                    RK3568_GMAC_PHY_INTF_SEL_RGMII |
> > > > > > > > +                    RK3568_GMAC_RXCLK_DLY_ENABLE |
> > > > > > > > +                    RK3568_GMAC_TXCLK_DLY_ENABLE);
> > > > > > > > +
> > > > > > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON0,
> > > > > > > > +                    RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
> > > > > > > > +                    RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
> > > > > > > > +
> > > > > > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
> > > > > > > > +                    RK3568_GMAC_PHY_INTF_SEL_RGMII |
> > > > > > > > +                    RK3568_GMAC_RXCLK_DLY_ENABLE |
> > > > > > > > +                    RK3568_GMAC_TXCLK_DLY_ENABLE);
> > > > > > > > +
> > > > > > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON0,
> > > > > > > > +                    RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
> > > > > > > > +                    RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
> > > > > > > 
> > > > > > > Since there are two GMACs on the rk3568, and either, or, or both may
> > > > > > > be enabled in various configurations, we should only configure the
> > > > > > > controller we are currently operating.
> > > > > 
> > > > > Perhaps we should have match data (such as reg = <0>, or against the
> > > > > address) to identify the individual controllers.
> > > > 
> > > > Hmm, "reg" will be used by the actual mmio address of the controller,
> > > > so matching against that should be the way I guess.
> > > > 
> > > > We're already doing something similar for dsi:
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c#n1170
> > > > 
> > > 
> > > I have to admit, I'm not a fan of hardcoding the registers in the kernel.
> > > 
> > > David Wu solved this in the downstream kernel by using bus_id,
> > > which parses the devicetree "ethernet@0" node, i.e.:
> > > 
> > >   plat->bus_id = of_alias_get_id(np, "ethernet");
> > 
> > What happens when one adds another ethernet controller (USB or PCIe) to
> > the board and wants to change the numbering order?
> > 
> > Or maybe only the second ethernet controller is routed on some board
> > and the submitter / vendor wants that one to be ethernet0, because
> > it's the only usable controller?
> 
> Which matches a discussion I had with Arnd about the mmc numbering.
> I.e. there the first mmc device is supposed to be mmc0 and so on,
> without gaps - for probably the same reasons.
> 
> 

Well, given each controller has its own register space, maybe
just model it with a new reg cell or a DT property that is able
to directly encode the base address?

Looking at the vendor kernel, XPCS will have another set of registers,
separate for each MAC.

Thanks,
Ezequiel

> > 
> > This seems even more fragile than hardcoding the registers.
> > 
> > Regards
> > ChenYu
> > 
> 
> 
> 
> 



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH net-next 3/3] net: stmmac: Add RK3566/RK3568 SoC support
@ 2021-04-14 16:44                   ` Ezequiel Garcia
  0 siblings, 0 replies; 25+ messages in thread
From: Ezequiel Garcia @ 2021-04-14 16:44 UTC (permalink / raw)
  To: Heiko Stübner, Chen-Yu Tsai
  Cc: Peter Geis, Linux Kernel Network Developers,
	open list:ARM/Rockchip SoC...,
	Jose Abreu, David S . Miller, Jakub Kicinski, David Wu, kernel

On Wed, 2021-04-14 at 18:35 +0200, Heiko Stübner wrote:
> Am Mittwoch, 14. April 2021, 18:33:12 CEST schrieb Chen-Yu Tsai:
> > On Thu, Apr 15, 2021 at 12:14 AM Ezequiel Garcia <ezequiel@collabora.com> wrote:
> > > 
> > > Hi Peter, Heiko,
> > > 
> > > On Wed, 2021-04-14 at 13:15 +0200, Heiko Stübner wrote:
> > > > Am Mittwoch, 14. April 2021, 13:03:25 CEST schrieb Peter Geis:
> > > > > On Tue, Apr 13, 2021 at 7:37 PM Ezequiel Garcia <ezequiel@collabora.com> wrote:
> > > > > > > > +static void rk3566_set_to_rmii(struct rk_priv_data *bsp_priv)
> > > > > > > > +{
> > > > > > > > +       struct device *dev = &bsp_priv->pdev->dev;
> > > > > > > > +
> > > > > > > > +       if (IS_ERR(bsp_priv->grf)) {
> > > > > > > > +               dev_err(dev, "%s: Missing rockchip,grf property\n", __func__);
> > > > > > > > +               return;
> > > > > > > > +       }
> > > > > > > > +
> > > > > > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
> > > > > > > > +                    RK3568_GMAC_PHY_INTF_SEL_RMII);
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static void rk3568_set_to_rgmii(struct rk_priv_data *bsp_priv,
> > > > > > > > +                               int tx_delay, int rx_delay)
> > > > > > > > +{
> > > > > > > > +       struct device *dev = &bsp_priv->pdev->dev;
> > > > > > > > +
> > > > > > > > +       if (IS_ERR(bsp_priv->grf)) {
> > > > > > > > +               dev_err(dev, "Missing rockchip,grf property\n");
> > > > > > > > +               return;
> > > > > > > > +       }
> > > > > > > > +
> > > > > > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON1,
> > > > > > > > +                    RK3568_GMAC_PHY_INTF_SEL_RGMII |
> > > > > > > > +                    RK3568_GMAC_RXCLK_DLY_ENABLE |
> > > > > > > > +                    RK3568_GMAC_TXCLK_DLY_ENABLE);
> > > > > > > > +
> > > > > > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON0,
> > > > > > > > +                    RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
> > > > > > > > +                    RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
> > > > > > > > +
> > > > > > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
> > > > > > > > +                    RK3568_GMAC_PHY_INTF_SEL_RGMII |
> > > > > > > > +                    RK3568_GMAC_RXCLK_DLY_ENABLE |
> > > > > > > > +                    RK3568_GMAC_TXCLK_DLY_ENABLE);
> > > > > > > > +
> > > > > > > > +       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON0,
> > > > > > > > +                    RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
> > > > > > > > +                    RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
> > > > > > > 
> > > > > > > Since there are two GMACs on the rk3568, and either, or, or both may
> > > > > > > be enabled in various configurations, we should only configure the
> > > > > > > controller we are currently operating.
> > > > > 
> > > > > Perhaps we should have match data (such as reg = <0>, or against the
> > > > > address) to identify the individual controllers.
> > > > 
> > > > Hmm, "reg" will be used by the actual mmio address of the controller,
> > > > so matching against that should be the way I guess.
> > > > 
> > > > We're already doing something similar for dsi:
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c#n1170
> > > > 
> > > 
> > > I have to admit, I'm not a fan of hardcoding the registers in the kernel.
> > > 
> > > David Wu solved this in the downstream kernel by using bus_id,
> > > which parses the devicetree "ethernet@0" node, i.e.:
> > > 
> > >   plat->bus_id = of_alias_get_id(np, "ethernet");
> > 
> > What happens when one adds another ethernet controller (USB or PCIe) to
> > the board and wants to change the numbering order?
> > 
> > Or maybe only the second ethernet controller is routed on some board
> > and the submitter / vendor wants that one to be ethernet0, because
> > it's the only usable controller?
> 
> Which matches a discussion I had with Arnd about the mmc numbering.
> I.e. there the first mmc device is supposed to be mmc0 and so on,
> without gaps - for probably the same reasons.
> 
> 

Well, given each controller has its own register space, maybe
just model it with a new reg cell or a DT property that is able
to directly encode the base address?

Looking at the vendor kernel, XPCS will have another set of registers,
separate for each MAC.

Thanks,
Ezequiel

> > 
> > This seems even more fragile than hardcoding the registers.
> > 
> > Regards
> > ChenYu
> > 
> 
> 
> 
> 



_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2021-04-14 16:45 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-13 21:02 [PATCH net-next 0/3] net: stmmac: RK3566/RK3568 Ezequiel Garcia
2021-04-13 21:02 ` Ezequiel Garcia
2021-04-13 21:02 ` [PATCH net-next 1/3] net: stmmac: Don't set has_gmac if has_gmac4 is set Ezequiel Garcia
2021-04-13 21:02   ` Ezequiel Garcia
2021-04-13 21:02 ` [PATCH net-next 2/3] net: stmmac: dwmac-rk: Check platform-specific ops Ezequiel Garcia
2021-04-13 21:02   ` Ezequiel Garcia
2021-04-13 21:02 ` [PATCH net-next 3/3] net: stmmac: Add RK3566/RK3568 SoC support Ezequiel Garcia
2021-04-13 21:02   ` Ezequiel Garcia
2021-04-13 22:51   ` Peter Geis
2021-04-13 22:51     ` Peter Geis
2021-04-13 23:37     ` Ezequiel Garcia
2021-04-14 11:03       ` Peter Geis
2021-04-14 11:03         ` Peter Geis
2021-04-14 11:15         ` Heiko Stübner
2021-04-14 11:15           ` Heiko Stübner
2021-04-14 13:48           ` Ezequiel Garcia
2021-04-14 13:48             ` Ezequiel Garcia
2021-04-14 16:33             ` Chen-Yu Tsai
2021-04-14 16:33               ` Chen-Yu Tsai
2021-04-14 16:35               ` Heiko Stübner
2021-04-14 16:35                 ` Heiko Stübner
2021-04-14 16:44                 ` Ezequiel Garcia
2021-04-14 16:44                   ` Ezequiel Garcia
2021-04-14 13:48         ` Ezequiel Garcia
2021-04-14 13:48           ` Ezequiel Garcia

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.