[U-Boot] [PATCH v2 00/12] Add support for Rockchip RK3288 Ethernet

[U-Boot] [PATCH v2 00/12] Add support for Rockchip RK3288 Ethernet

[Sjoerd Simons]

Second round to add support for the GMAC Ethernet interface on RK3288
SoCs. I had hope to follow up a lot earlier, but things were bussier
then expected :(

To add support I've taken a slightly different approach then some of the
other boards with a designware IP block, by creating a new driver to
take care of the platfrom glue which subclasses the main designware driver
instead of adding the compatibility string the designware driver
directly and doing the SoC specific setup in the board files. This seems
quite a bit more elegant in a device model based world.

I've only tested this series on a Radxa Rock 2 board, it would be great
if someone could test this on other boards with the designware IP
especially for those with the reset GPIO in devicetree (e.g. some of the
Allwinner boards).

Compared to the first one round the pinctrl related bits were dropped as
RK3288 now has a full pinctrl driver. Furthermore the started hook in the
designware driver was renamed to fix_mac_speed in line with what linux
uses and moved to the dw_link_adjust function.


Changes in v2:
- Store probe gpio in private data not platdata
- Drop unneeded header file
- Move the hook into the dw_adjust_link function
- Rename the hook to fix_mac_speed, similar to Linux
- Fix various coding style nits
- Adjust to new hook name

Sjoerd Simons (12):
  net: designware: support phy reset device-tree bindings
  net: designware: Export various functions/struct to allow subclassing
  net: designware: Add a fix_mac_speed hook
  rockchip: rk3288: pinctrl: support more pins
  rockchip: rk3288: Add clock support for the gmac ethernet interface
  rockchip: rk3288: grf: Define GRF_SOC_CON1 and GRF_SOC_CON3
  net: gmac_rk3288: Add RK3288 GMAC driver
  rockchip: rk3288-firefly: Add gmac definition
  rockchip: rock2: dts: use status = "okay" not ok
  rockchip: Enable networking support on rock2 and firefly
  rockchip: Add PXE and DHCP to the default boot targets
  rockchip: Drop Ethernet from the TODO

 arch/arm/dts/rk3288-firefly.dtsi                |  16 +++
 arch/arm/dts/rk3288-rock2-square.dts            |   2 +-
 arch/arm/include/asm/arch-rockchip/cru_rk3288.h |  17 ++++
 arch/arm/include/asm/arch-rockchip/grf_rk3288.h |  53 ++++++++++
 configs/firefly-rk3288_defconfig                |   5 +
 configs/rock2_defconfig                         |   5 +
 doc/README.rockchip                             |   1 -
 drivers/clk/clk_rk3288.c                        |  14 +++
 drivers/net/Kconfig                             |   7 ++
 drivers/net/Makefile                            |   1 +
 drivers/net/designware.c                        |  90 ++++++++++++++---
 drivers/net/designware.h                        |  18 ++++
 drivers/net/gmac_rk3288.c                       | 125 ++++++++++++++++++++++++
 drivers/pinctrl/rockchip/pinctrl_rk3288.c       |   2 +-
 include/configs/rk3288_common.h                 |   4 +-
 15 files changed, 345 insertions(+), 15 deletions(-)
 create mode 100644 drivers/net/gmac_rk3288.c

-- 
2.7.0

[U-Boot] [PATCH v2 01/12] net: designware: support phy reset device-tree bindings

[Sjoerd Simons]

Add support for the snps,reset-gpio, snps,reset-active-low (optional) and
snps,reset-delays-us device-tree bindings. The combination of these
three define how the PHY should be reset to ensure it's in a sane state.

Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>

---

Changes in v2:
- Store probe gpio in private data not platdata
- Drop unneeded header file

 drivers/net/designware.c | 76 ++++++++++++++++++++++++++++++++++++++++++++----
 drivers/net/designware.h | 10 +++++++
 2 files changed, 80 insertions(+), 6 deletions(-)

diff --git a/drivers/net/designware.c b/drivers/net/designware.c
index ca58f34..14dd7b8 100644
--- a/drivers/net/designware.c
+++ b/drivers/net/designware.c
@@ -24,7 +24,12 @@ DECLARE_GLOBAL_DATA_PTR;
 
 static int dw_mdio_read(struct mii_dev *bus, int addr, int devad, int reg)
 {
+#ifdef CONFIG_DM_ETH
+	struct dw_eth_dev *priv = dev_get_priv((struct udevice *)bus->priv);
+	struct eth_mac_regs *mac_p = priv->mac_regs_p;
+#else
 	struct eth_mac_regs *mac_p = bus->priv;
+#endif
 	ulong start;
 	u16 miiaddr;
 	int timeout = CONFIG_MDIO_TIMEOUT;
@@ -47,7 +52,12 @@ static int dw_mdio_read(struct mii_dev *bus, int addr, int devad, int reg)
 static int dw_mdio_write(struct mii_dev *bus, int addr, int devad, int reg,
 			u16 val)
 {
+#ifdef CONFIG_DM_ETH
+	struct dw_eth_dev *priv = dev_get_priv((struct udevice *)bus->priv);
+	struct eth_mac_regs *mac_p = priv->mac_regs_p;
+#else
 	struct eth_mac_regs *mac_p = bus->priv;
+#endif
 	ulong start;
 	u16 miiaddr;
 	int ret = -ETIMEDOUT, timeout = CONFIG_MDIO_TIMEOUT;
@@ -70,7 +80,41 @@ static int dw_mdio_write(struct mii_dev *bus, int addr, int devad, int reg,
 	return ret;
 }
 
-static int dw_mdio_init(const char *name, struct eth_mac_regs *mac_regs_p)
+#if CONFIG_DM_ETH
+static int dw_mdio_reset(struct mii_dev *bus)
+{
+	struct udevice *dev = bus->priv;
+	struct dw_eth_dev *priv = dev_get_priv(dev);
+	struct dw_eth_pdata *pdata = dev_get_platdata(dev);
+	int ret;
+
+	if (!dm_gpio_is_valid(&priv->reset_gpio))
+		return 0;
+
+	/* reset the phy */
+	ret = dm_gpio_set_value(&priv->reset_gpio, 0);
+	if (ret)
+		return ret;
+
+	udelay(pdata->reset_delays[0]);
+
+	ret = dm_gpio_set_value(&priv->reset_gpio, 1);
+	if (ret)
+		return ret;
+
+	udelay(pdata->reset_delays[1]);
+
+	ret = dm_gpio_set_value(&priv->reset_gpio, 0);
+	if (ret)
+		return ret;
+
+	udelay(pdata->reset_delays[2]);
+
+	return 0;
+}
+#endif
+
+static int dw_mdio_init(const char *name, void *priv)
 {
 	struct mii_dev *bus = mdio_alloc();
 
@@ -82,8 +126,11 @@ static int dw_mdio_init(const char *name, struct eth_mac_regs *mac_regs_p)
 	bus->read = dw_mdio_read;
 	bus->write = dw_mdio_write;
 	snprintf(bus->name, sizeof(bus->name), "%s", name);
+#ifdef CONFIG_DM_ETH
+	bus->reset = dw_mdio_reset;
+#endif
 
-	bus->priv = (void *)mac_regs_p;
+	bus->priv = priv;
 
 	return mdio_register(bus);
 }
@@ -606,7 +653,7 @@ static int designware_eth_probe(struct udevice *dev)
 	priv->interface = pdata->phy_interface;
 	priv->max_speed = pdata->max_speed;
 
-	dw_mdio_init(dev->name, priv->mac_regs_p);
+	dw_mdio_init(dev->name, dev);
 	priv->bus = miiphy_get_dev_by_name(dev->name);
 
 	ret = dw_phy_init(priv, dev);
@@ -637,9 +684,13 @@ static const struct eth_ops designware_eth_ops = {
 
 static int designware_eth_ofdata_to_platdata(struct udevice *dev)
 {
-	struct eth_pdata *pdata = dev_get_platdata(dev);
+	struct dw_eth_pdata *dw_pdata = dev_get_platdata(dev);
+	struct dw_eth_dev *priv = dev_get_priv(dev);
+	struct eth_pdata *pdata = &dw_pdata->eth_pdata;
 	const char *phy_mode;
 	const fdt32_t *cell;
+	int reset_flags = GPIOD_IS_OUT;
+	int ret = 0;
 
 	pdata->iobase = dev_get_addr(dev);
 	pdata->phy_interface = -1;
@@ -656,7 +707,20 @@ static int designware_eth_ofdata_to_platdata(struct udevice *dev)
 	if (cell)
 		pdata->max_speed = fdt32_to_cpu(*cell);
 
-	return 0;
+	if (fdtdec_get_bool(gd->fdt_blob, dev->of_offset,
+			    "snps,reset-active-low"))
+		reset_flags |= GPIOD_ACTIVE_LOW;
+
+	ret = gpio_request_by_name(dev, "snps,reset-gpio", 0,
+		&priv->reset_gpio, reset_flags);
+	if (ret == 0) {
+		ret = fdtdec_get_int_array(gd->fdt_blob, dev->of_offset,
+			"snps,reset-delays-us", dw_pdata->reset_delays, 3);
+	} else if (ret == -ENOENT) {
+		ret = 0;
+	}
+
+	return ret;
 }
 
 static const struct udevice_id designware_eth_ids[] = {
@@ -675,7 +739,7 @@ U_BOOT_DRIVER(eth_designware) = {
 	.remove	= designware_eth_remove,
 	.ops	= &designware_eth_ops,
 	.priv_auto_alloc_size = sizeof(struct dw_eth_dev),
-	.platdata_auto_alloc_size = sizeof(struct eth_pdata),
+	.platdata_auto_alloc_size = sizeof(struct dw_eth_pdata),
 	.flags = DM_FLAG_ALLOC_PRIV_DMA,
 };
 
diff --git a/drivers/net/designware.h b/drivers/net/designware.h
index ed6344c..04a45e0 100644
--- a/drivers/net/designware.h
+++ b/drivers/net/designware.h
@@ -8,6 +8,8 @@
 #ifndef _DW_ETH_H
 #define _DW_ETH_H
 
+#include <asm/gpio.h>
+
 #define CONFIG_TX_DESCR_NUM	16
 #define CONFIG_RX_DESCR_NUM	16
 #define CONFIG_ETH_BUFSIZE	2048
@@ -232,8 +234,16 @@ struct dw_eth_dev {
 #ifndef CONFIG_DM_ETH
 	struct eth_device *dev;
 #endif
+	struct gpio_desc reset_gpio;
 	struct phy_device *phydev;
 	struct mii_dev *bus;
 };
 
+#ifdef CONFIG_DM_ETH
+struct dw_eth_pdata {
+	struct eth_pdata eth_pdata;
+	u32 reset_delays[3];
+};
+#endif
+
 #endif
-- 
2.7.0

[U-Boot] [PATCH v2 02/12] net: designware: Export various functions/struct to allow subclassing

[Sjoerd Simons]

To allow other DM drivers to subclass the designware driver various
functions and structures need to be exported. Export these.

Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Acked-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 drivers/net/designware.c | 6 +++---
 drivers/net/designware.h | 4 ++++
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/net/designware.c b/drivers/net/designware.c
index 14dd7b8..8834506 100644
--- a/drivers/net/designware.c
+++ b/drivers/net/designware.c
@@ -625,7 +625,7 @@ static int designware_eth_bind(struct udevice *dev)
 	return 0;
 }
 
-static int designware_eth_probe(struct udevice *dev)
+int designware_eth_probe(struct udevice *dev)
 {
 	struct eth_pdata *pdata = dev_get_platdata(dev);
 	struct dw_eth_dev *priv = dev_get_priv(dev);
@@ -673,7 +673,7 @@ static int designware_eth_remove(struct udevice *dev)
 	return 0;
 }
 
-static const struct eth_ops designware_eth_ops = {
+const struct eth_ops designware_eth_ops = {
 	.start			= designware_eth_start,
 	.send			= designware_eth_send,
 	.recv			= designware_eth_recv,
@@ -682,7 +682,7 @@ static const struct eth_ops designware_eth_ops = {
 	.write_hwaddr		= designware_eth_write_hwaddr,
 };
 
-static int designware_eth_ofdata_to_platdata(struct udevice *dev)
+int designware_eth_ofdata_to_platdata(struct udevice *dev)
 {
 	struct dw_eth_pdata *dw_pdata = dev_get_platdata(dev);
 	struct dw_eth_dev *priv = dev_get_priv(dev);
diff --git a/drivers/net/designware.h b/drivers/net/designware.h
index 04a45e0..6b4bfd7 100644
--- a/drivers/net/designware.h
+++ b/drivers/net/designware.h
@@ -240,6 +240,10 @@ struct dw_eth_dev {
 };
 
 #ifdef CONFIG_DM_ETH
+int designware_eth_ofdata_to_platdata(struct udevice *dev);
+int designware_eth_probe(struct udevice *dev);
+extern const struct eth_ops designware_eth_ops;
+
 struct dw_eth_pdata {
 	struct eth_pdata eth_pdata;
 	u32 reset_delays[3];
-- 
2.7.0

[U-Boot] [PATCH v2 03/12] net: designware: Add a fix_mac_speed hook

[Sjoerd Simons]

Add the ability for e.g. drivers subclassing to register a function to
be called after phy link negotiation. This is useful if e.g. the driver
needs to change the mac configuration based on the negotiated speed.

Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>

---

Changes in v2:
- Move the hook into the dw_adjust_link function
- Rename the hook to fix_mac_speed, similar to Linux

 drivers/net/designware.c | 8 ++++++--
 drivers/net/designware.h | 4 ++++
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/net/designware.c b/drivers/net/designware.c
index 8834506..5eaa1de 100644
--- a/drivers/net/designware.c
+++ b/drivers/net/designware.c
@@ -231,7 +231,8 @@ static int _dw_write_hwaddr(struct dw_eth_dev *priv, u8 *mac_id)
 	return 0;
 }
 
-static void dw_adjust_link(struct eth_mac_regs *mac_p,
+static void dw_adjust_link(struct dw_eth_dev *priv,
+			   struct eth_mac_regs *mac_p,
 			   struct phy_device *phydev)
 {
 	u32 conf = readl(&mac_p->conf) | FRAMEBURSTENABLE | DISABLERXOWN;
@@ -257,6 +258,9 @@ static void dw_adjust_link(struct eth_mac_regs *mac_p,
 	printf("Speed: %d, %s duplex%s\n", phydev->speed,
 	       (phydev->duplex) ? "full" : "half",
 	       (phydev->port == PORT_FIBRE) ? ", fiber mode" : "");
+
+	if (priv->fix_mac_speed)
+		priv->fix_mac_speed(priv);
 }
 
 static void _dw_eth_halt(struct dw_eth_dev *priv)
@@ -322,7 +326,7 @@ static int _dw_eth_init(struct dw_eth_dev *priv, u8 *enetaddr)
 		return ret;
 	}
 
-	dw_adjust_link(mac_p, priv->phydev);
+	dw_adjust_link(priv, mac_p, priv->phydev);
 
 	if (!priv->phydev->link)
 		return -EIO;
diff --git a/drivers/net/designware.h b/drivers/net/designware.h
index 6b4bfd7..792af7c 100644
--- a/drivers/net/designware.h
+++ b/drivers/net/designware.h
@@ -237,6 +237,10 @@ struct dw_eth_dev {
 	struct gpio_desc reset_gpio;
 	struct phy_device *phydev;
 	struct mii_dev *bus;
+
+#ifdef CONFIG_DM_ETH
+	int (*fix_mac_speed)(struct dw_eth_dev *priv);
+#endif
 };
 
 #ifdef CONFIG_DM_ETH
-- 
2.7.0

[U-Boot] [PATCH v2 04/12] rockchip: rk3288: pinctrl: support more pins

[Sjoerd Simons]

The rgmii_pins node in rk3288.dtsi configures 15 pins. Increase the size
of the cell array to accomedate that, otherwise only the first 10 get
configured.

Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
---

Changes in v2: None

 drivers/pinctrl/rockchip/pinctrl_rk3288.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/rockchip/pinctrl_rk3288.c b/drivers/pinctrl/rockchip/pinctrl_rk3288.c
index 7c769bd..1fa1daa 100644
--- a/drivers/pinctrl/rockchip/pinctrl_rk3288.c
+++ b/drivers/pinctrl/rockchip/pinctrl_rk3288.c
@@ -623,7 +623,7 @@ static int rk3288_pinctrl_set_state(struct udevice *dev, struct udevice *config)
 {
 	const void *blob = gd->fdt_blob;
 	int pcfg_node, ret, flags, count, i;
-	u32 cell[40], *ptr;
+	u32 cell[60], *ptr;
 
 	debug("%s: %s %s\n", __func__, dev->name, config->name);
 	ret = fdtdec_get_int_array_count(blob, config->of_offset,
-- 
2.7.0

[U-Boot] [PATCH v2 05/12] rockchip: rk3288: Add clock support for the gmac ethernet interface

[Sjoerd Simons]

Setup the clocks for the gmac ethernet interface. This assumes the mac
clock is fed by an external clock which is common on RK3288 based
devices.

Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
---

Changes in v2: None

 arch/arm/include/asm/arch-rockchip/cru_rk3288.h | 17 +++++++++++++++++
 drivers/clk/clk_rk3288.c                        | 14 ++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/arch/arm/include/asm/arch-rockchip/cru_rk3288.h b/arch/arm/include/asm/arch-rockchip/cru_rk3288.h
index d2690c7..8a8ca9c 100644
--- a/arch/arm/include/asm/arch-rockchip/cru_rk3288.h
+++ b/arch/arm/include/asm/arch-rockchip/cru_rk3288.h
@@ -90,6 +90,23 @@ enum {
 	SDIO0_DIV_MASK		= 0x3f,
 };
 
+/* CRU_CLKSEL21_CON */
+enum {
+	MAC_DIV_CON_SHIFT = 0xf,
+	MAC_DIV_CON_MASK = 0x1f,
+
+	RMII_EXTCLK_SHIFT = 4,
+	RMII_EXTCLK_MASK = 1,
+	RMII_EXTCLK_SELECT_INT_DIV_CLK = 0,
+	RMII_EXTCLK_SELECT_EXT_CLK = 1,
+
+	EMAC_PLL_SHIFT = 0,
+	EMAC_PLL_MASK = 0x3,
+	EMAC_PLL_SELECT_NEW = 0x0,
+	EMAC_PLL_SELECT_CODEC = 0x1,
+	EMAC_PLL_SELECT_GENERAL = 0x2,
+};
+
 /* CRU_CLKSEL25_CON */
 enum {
 	SPI1_PLL_SHIFT		= 0xf,
diff --git a/drivers/clk/clk_rk3288.c b/drivers/clk/clk_rk3288.c
index 2a85e93..a110a1c 100644
--- a/drivers/clk/clk_rk3288.c
+++ b/drivers/clk/clk_rk3288.c
@@ -326,6 +326,17 @@ static int pll_para_config(ulong freq_hz, struct pll_div *div, uint *ext_div)
 	return 0;
 }
 
+static int rockchip_mac_set_clk(struct rk3288_cru *cru,
+				  int periph, uint freq)
+{
+	/* Assuming mac_clk is fed by an external clock */
+	rk_clrsetreg(&cru->cru_clksel_con[21],
+		     RMII_EXTCLK_MASK << RMII_EXTCLK_SHIFT,
+		     RMII_EXTCLK_SELECT_EXT_CLK << RMII_EXTCLK_SHIFT);
+
+	 return 0;
+}
+
 static int rockchip_vop_set_clk(struct rk3288_cru *cru, struct rk3288_grf *grf,
 				int periph, unsigned int rate_hz)
 {
@@ -759,6 +770,9 @@ static ulong rk3288_set_periph_rate(struct udevice *dev, int periph, ulong rate)
 		new_rate = rockchip_spi_set_clk(cru, gclk_rate, periph, rate);
 		break;
 #ifndef CONFIG_SPL_BUILD
+	case SCLK_MAC:
+		new_rate = rockchip_mac_set_clk(priv->cru, periph, rate);
+		break;
 	case DCLK_VOP0:
 	case DCLK_VOP1:
 		new_rate = rockchip_vop_set_clk(cru, priv->grf, periph, rate);
-- 
2.7.0

[U-Boot] [PATCH v2 06/12] rockchip: rk3288: grf: Define GRF_SOC_CON1 and GRF_SOC_CON3

[Sjoerd Simons]

Add definitions for GRF_SOC_CON1 and GRF_SOC_CON3 which contain various
GMAC related fields.

Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
---

Changes in v2: None

 arch/arm/include/asm/arch-rockchip/grf_rk3288.h | 53 +++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/arch/arm/include/asm/arch-rockchip/grf_rk3288.h b/arch/arm/include/asm/arch-rockchip/grf_rk3288.h
index 0117a17..aaffd19 100644
--- a/arch/arm/include/asm/arch-rockchip/grf_rk3288.h
+++ b/arch/arm/include/asm/arch-rockchip/grf_rk3288.h
@@ -718,6 +718,40 @@ enum {
 	MSCH0_MAINPARTIALPOP_MASK = 1,
 };
 
+/* GRF_SOC_CON1 */
+enum {
+	RMII_MODE_SHIFT = 0xe,
+	RMII_MODE_MASK = 1,
+	RMII_MODE = 1,
+
+	GMAC_CLK_SEL_SHIFT	= 0xc,
+	GMAC_CLK_SEL_MASK	= 3,
+	GMAC_CLK_SEL_125M	= 0,
+	GMAC_CLK_SEL_25M	= 0x3,
+	GMAC_CLK_SEL_2_5M	= 0x2,
+
+	RMII_CLK_SEL_SHIFT	= 0xb,
+	RMII_CLK_SEL_MASK	= 1,
+	RMII_CLK_SEL_2_5M	= 0,
+	RMII_CLK_SEL_25M,
+
+	GMAC_SPEED_SHIFT	= 0xa,
+	GMAC_SPEED_MASK		= 1,
+	GMAC_SPEED_10M		= 0,
+	GMAC_SPEED_100M,
+
+	GMAC_FLOWCTRL_SHIFT	= 0x9,
+	GMAC_FLOWCTRL_MASK	= 1,
+
+	GMAC_PHY_INTF_SEL_SHIFT	= 0x6,
+	GMAC_PHY_INTF_SEL_MASK	= 0x7,
+	GMAC_PHY_INTF_SEL_RGMII	= 0x1,
+	GMAC_PHY_INTF_SEL_RMII	= 0x4,
+
+	HOST_REMAP_SHIFT	= 0x5,
+	HOST_REMAP_MASK		= 1
+};
+
 /* GRF_SOC_CON2 */
 enum {
 	UPCTL1_LPDDR3_ODT_EN_SHIFT = 0xd,
@@ -765,4 +799,23 @@ enum {
 	PWM_PWM			= 0,
 };
 
+/* GRF_SOC_CON3 */
+enum {
+	RXCLK_DLY_ENA_GMAC_SHIFT	= 0xf,
+	RXCLK_DLY_ENA_GMAC_MASK		= 1,
+	RXCLK_DLY_ENA_GMAC_DISABLE	= 0,
+	RXCLK_DLY_ENA_GMAC_ENABLE,
+
+	TXCLK_DLY_ENA_GMAC_SHIFT	= 0xe,
+	TXCLK_DLY_ENA_GMAC_MASK		= 1,
+	TXCLK_DLY_ENA_GMAC_DISABLE	= 0,
+	TXCLK_DLY_ENA_GMAC_ENABLE,
+
+	CLK_RX_DL_CFG_GMAC_SHIFT	= 0x7,
+	CLK_RX_DL_CFG_GMAC_MASK		= 0x7f,
+
+	CLK_TX_DL_CFG_GMAC_SHIFT	= 0x0,
+	CLK_TX_DL_CFG_GMAC_MASK		= 0x7f,
+};
+
 #endif
-- 
2.7.0

[U-Boot] [PATCH v2 07/12] net: gmac_rk3288: Add RK3288 GMAC driver

[Sjoerd Simons]

Add a new driver for the GMAC ethernet interface present in Rockchip
RK3288 SOCs. This driver subclasses the generic design-ware driver to
add the glue needed specifically for Rockchip.

Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>

---

Changes in v2:
- Fix various coding style nits
- Adjust to new hook name

 drivers/net/Kconfig       |   7 +++
 drivers/net/Makefile      |   1 +
 drivers/net/gmac_rk3288.c | 125 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 133 insertions(+)
 create mode 100644 drivers/net/gmac_rk3288.c

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index bc2f51d..fa49856 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -133,4 +133,11 @@ config PIC32_ETH
 	  This driver implements 10/100 Mbps Ethernet and MAC layer for
 	  Microchip PIC32 microcontrollers.
 
+config GMAC_RK3288
+	bool "Rockchip RK3288 Synopsys Designware Ethernet MAC"
+	depends on DM_ETH && ETH_DESIGNWARE
+	help
+	  This driver provides Rockchip RK3288 network support based on the
+	  Synopsys Designware driver.
+
 endif # NETDEVICES
diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index 33a81ee..d0a8009 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -32,6 +32,7 @@ obj-$(CONFIG_FTGMAC100) += ftgmac100.o
 obj-$(CONFIG_FTMAC110) += ftmac110.o
 obj-$(CONFIG_FTMAC100) += ftmac100.o
 obj-$(CONFIG_GRETH) += greth.o
+obj-$(CONFIG_GMAC_RK3288) += gmac_rk3288.o
 obj-$(CONFIG_DRIVER_TI_KEYSTONE_NET) += keystone_net.o
 obj-$(CONFIG_KS8851_MLL) += ks8851_mll.o
 obj-$(CONFIG_LAN91C96) += lan91c96.o
diff --git a/drivers/net/gmac_rk3288.c b/drivers/net/gmac_rk3288.c
new file mode 100644
index 0000000..5400b2c
--- /dev/null
+++ b/drivers/net/gmac_rk3288.c
@@ -0,0 +1,125 @@
+/*
+ * (C) Copyright 2015 Sjoerd Simons <sjoerd.simons@collabora.co.uk>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+/* * Rockchip GMAC ethernet IP driver for U-Boot */
+#include <common.h>
+#include <dm.h>
+#include <dm/pinctrl.h>
+#include <asm/gpio.h>
+#include <clk.h>
+#include <phy.h>
+#include <syscon.h>
+#include <asm/io.h>
+#include <asm/arch/periph.h>
+#include <asm/arch/clock.h>
+#include <asm/arch/grf_rk3288.h>
+#include "designware.h"
+#include <dt-bindings/clock/rk3288-cru.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+struct gmac_rk3288_platdata {
+	struct dw_eth_pdata dw_eth_pdata;
+	int tx_delay;
+	int rx_delay;
+};
+
+static int gmac_rk3288_ofdata_to_platdata(struct udevice *dev)
+{
+	struct gmac_rk3288_platdata *pdata = dev_get_platdata(dev);
+
+	pdata->tx_delay = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
+					 "tx_delay", 0x30);
+	pdata->rx_delay = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
+					 "rx_delay", 0x10);
+
+	return designware_eth_ofdata_to_platdata(dev);
+}
+
+static int gmac_rk3288_fix_mac_speed(struct dw_eth_dev *priv)
+{
+	struct rk3288_grf *grf;
+	int clk;
+
+	switch (priv->phydev->speed) {
+	case 10:
+		clk = GMAC_CLK_SEL_2_5M;
+		break;
+	case 100:
+		clk = GMAC_CLK_SEL_25M;
+		break;
+	case 1000:
+		clk = GMAC_CLK_SEL_125M;
+		break;
+	default:
+		printf("Unknown phy speed: %d\n", priv->phydev->speed);
+		return -EINVAL;
+	}
+
+	grf = syscon_get_first_range(ROCKCHIP_SYSCON_GRF);
+
+	rk_clrsetreg(&grf->soc_con1,
+		     GMAC_CLK_SEL_MASK << GMAC_CLK_SEL_SHIFT,
+		     clk << GMAC_CLK_SEL_SHIFT);
+
+	return 0;
+}
+
+static int gmac_rk3288_probe(struct udevice *dev)
+{
+	int ret;
+	struct gmac_rk3288_platdata *pdata = dev_get_platdata(dev);
+	struct dw_eth_dev *priv = dev_get_priv(dev);
+	struct rk3288_grf *grf;
+	struct udevice *clk;
+
+
+	ret = uclass_get_device(UCLASS_CLK, CLK_GENERAL, &clk);
+	if (ret)
+		return ret;
+
+	ret = clk_set_periph_rate(clk, SCLK_MAC, 0);
+	if (ret)
+		return ret;
+
+	/* Set to RGMII mode */
+	grf = syscon_get_first_range(ROCKCHIP_SYSCON_GRF);
+	rk_clrsetreg(&grf->soc_con1,
+		     RMII_MODE_MASK << RMII_MODE_SHIFT |
+		     GMAC_PHY_INTF_SEL_MASK << GMAC_PHY_INTF_SEL_SHIFT,
+		     GMAC_PHY_INTF_SEL_RGMII << GMAC_PHY_INTF_SEL_SHIFT);
+
+	rk_clrsetreg(&grf->soc_con3,
+		     RXCLK_DLY_ENA_GMAC_MASK <<  RXCLK_DLY_ENA_GMAC_SHIFT |
+		     TXCLK_DLY_ENA_GMAC_MASK <<  TXCLK_DLY_ENA_GMAC_SHIFT |
+		     CLK_RX_DL_CFG_GMAC_MASK <<  CLK_RX_DL_CFG_GMAC_SHIFT |
+		     CLK_TX_DL_CFG_GMAC_MASK <<  CLK_TX_DL_CFG_GMAC_SHIFT,
+		     RXCLK_DLY_ENA_GMAC_ENABLE << RXCLK_DLY_ENA_GMAC_SHIFT |
+		     TXCLK_DLY_ENA_GMAC_ENABLE << TXCLK_DLY_ENA_GMAC_SHIFT |
+		     pdata->rx_delay << CLK_RX_DL_CFG_GMAC_SHIFT |
+		     pdata->tx_delay << CLK_TX_DL_CFG_GMAC_SHIFT);
+
+	priv->fix_mac_speed = gmac_rk3288_fix_mac_speed;
+
+	return designware_eth_probe(dev);
+}
+
+static const struct udevice_id rk3288_gmac_ids[] = {
+	{ .compatible = "rockchip,rk3288-gmac" },
+	{ }
+};
+
+U_BOOT_DRIVER(eth_gmac_rk3288) = {
+	.name	= "gmac_rk3288",
+	.id	= UCLASS_ETH,
+	.of_match = rk3288_gmac_ids,
+	.ofdata_to_platdata = gmac_rk3288_ofdata_to_platdata,
+	.probe	= gmac_rk3288_probe,
+	.ops	= &designware_eth_ops,
+	.priv_auto_alloc_size = sizeof(struct dw_eth_dev),
+	.platdata_auto_alloc_size = sizeof(struct gmac_rk3288_platdata),
+	.flags = DM_FLAG_ALLOC_PRIV_DMA,
+};
-- 
2.7.0

[U-Boot] [PATCH v2 08/12] rockchip: rk3288-firefly: Add gmac definition

[Sjoerd Simons]

Add a definition for the gmac interface to the firefly device-tree.
Copied verbatim from the linux kernel.

Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
Acked-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 arch/arm/dts/rk3288-firefly.dtsi | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/arm/dts/rk3288-firefly.dtsi b/arch/arm/dts/rk3288-firefly.dtsi
index 5aec1b8..072eaa6 100644
--- a/arch/arm/dts/rk3288-firefly.dtsi
+++ b/arch/arm/dts/rk3288-firefly.dtsi
@@ -146,6 +146,22 @@
 	status = "okay";
 };
 
+&gmac {
+	assigned-clocks = <&cru SCLK_MAC>;
+	assigned-clock-parents = <&ext_gmac>;
+	clock_in_out = "input";
+	pinctrl-names = "default";
+	pinctrl-0 = <&rgmii_pins>, <&phy_rst>, <&phy_pmeb>, <&phy_int>;
+	phy-supply = <&vcc_lan>;
+	phy-mode = "rgmii";
+	snps,reset-active-low;
+	snps,reset-delays-us = <0 10000 1000000>;
+	snps,reset-gpio = <&gpio4 8 GPIO_ACTIVE_LOW>;
+	tx_delay = <0x30>;
+	rx_delay = <0x10>;
+	status = "okay";
+};
+
 &hdmi {
 	ddc-i2c-bus = <&i2c5>;
 	status = "okay";
-- 
2.7.0

[U-Boot] [PATCH v2 09/12] rockchip: rock2: dts: use status = "okay" not ok

[Sjoerd Simons]

u-boot only recognize okay to enable a node (Linux seems to be more
lenient here). So use okay instead.

Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
---

Changes in v2: None

 arch/arm/dts/rk3288-rock2-square.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/dts/rk3288-rock2-square.dts b/arch/arm/dts/rk3288-rock2-square.dts
index 8d7446f..34073c9 100644
--- a/arch/arm/dts/rk3288-rock2-square.dts
+++ b/arch/arm/dts/rk3288-rock2-square.dts
@@ -111,7 +111,7 @@
 };
 
 &gmac {
-	status = "ok";
+	status = "okay";
 };
 
 &hdmi {
-- 
2.7.0

[U-Boot] [PATCH v2 10/12] rockchip: Enable networking support on rock2 and firefly

[Sjoerd Simons]

Enable the various configuration option required to get the ethernet
interface up and running on Radxa Rock2 and Firefly.

Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
---

Changes in v2: None

 configs/firefly-rk3288_defconfig | 5 +++++
 configs/rock2_defconfig          | 5 +++++
 2 files changed, 10 insertions(+)

diff --git a/configs/firefly-rk3288_defconfig b/configs/firefly-rk3288_defconfig
index 5aa4166..2ae9ca1 100644
--- a/configs/firefly-rk3288_defconfig
+++ b/configs/firefly-rk3288_defconfig
@@ -27,6 +27,11 @@ CONFIG_LED_GPIO=y
 CONFIG_RESET=y
 CONFIG_DM_MMC=y
 CONFIG_ROCKCHIP_DWMMC=y
+CONFIG_DM_ETH=y
+CONFIG_NETDEVICES=y
+CONFIG_ETH_DESIGNWARE=y
+CONFIG_GMAC_RK3288=y
+CONFIG_SYS_I2C_ROCKCHIP=y
 CONFIG_PINCTRL=y
 CONFIG_SPL_PINCTRL=y
 # CONFIG_SPL_PINCTRL_FULL is not set
diff --git a/configs/rock2_defconfig b/configs/rock2_defconfig
index f33daf1..563305b 100644
--- a/configs/rock2_defconfig
+++ b/configs/rock2_defconfig
@@ -25,6 +25,11 @@ CONFIG_SYS_I2C_ROCKCHIP=y
 CONFIG_RESET=y
 CONFIG_DM_MMC=y
 CONFIG_ROCKCHIP_DWMMC=y
+CONFIG_DM_ETH=y
+CONFIG_NETDEVICES=y
+CONFIG_ETH_DESIGNWARE=y
+CONFIG_GMAC_RK3288=y
+CONFIG_SYS_I2C_ROCKCHIP=y
 CONFIG_PINCTRL=y
 CONFIG_SPL_PINCTRL=y
 # CONFIG_SPL_PINCTRL_FULL is not set
-- 
2.7.0

[U-Boot] [PATCH v2 11/12] rockchip: Add PXE and DHCP to the default boot targets

[Sjoerd Simons]

Now that at least on the firefly board we have network support, enable
PXE and DHCP boot targets by default.

Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
Acked-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 include/configs/rk3288_common.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/configs/rk3288_common.h b/include/configs/rk3288_common.h
index 427ac4b..dba1a22 100644
--- a/include/configs/rk3288_common.h
+++ b/include/configs/rk3288_common.h
@@ -108,7 +108,9 @@
 /* First try to boot from SD (index 0), then eMMC (index 1 */
 #define BOOT_TARGET_DEVICES(func) \
 	func(MMC, mmc, 0) \
-	func(MMC, mmc, 1)
+	func(MMC, mmc, 1) \
+	func(PXE, pxe, na) \
+	func(DHCP, dchp, na)
 
 #include <config_distro_bootcmd.h>
 
-- 
2.7.0

[U-Boot] [PATCH v2 12/12] rockchip: Drop Ethernet from the TODO

[Sjoerd Simons]

Now that ethernet support works, it can be dropped from the rockchip
TODO

Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
Acked-by: Simon Glass <sjg@chromium.org>

---

Changes in v2: None

 doc/README.rockchip | 1 -
 1 file changed, 1 deletion(-)

diff --git a/doc/README.rockchip b/doc/README.rockchip
index e0572c8..3ce12d3 100644
--- a/doc/README.rockchip
+++ b/doc/README.rockchip
@@ -177,7 +177,6 @@ Immediate priorities are:
 - USB host
 - USB device
 - Run CPU at full speed (code exists but we only see ~60 DMIPS maximum)
-- Ethernet
 - NAND flash
 - Support for other Rockchip parts
 - Boot U-Boot proper over USB OTG (at present only SPL works)
-- 
2.7.0

[U-Boot] [PATCH v2 01/12] net: designware: support phy reset device-tree bindings

[Simon Glass]

On 28 February 2016 at 14:24, Sjoerd Simons
<sjoerd.simons@collabora.co.uk> wrote:
> Add support for the snps,reset-gpio, snps,reset-active-low (optional) and
> snps,reset-delays-us device-tree bindings. The combination of these
> three define how the PHY should be reset to ensure it's in a sane state.
>
> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
>
> ---
>
> Changes in v2:
> - Store probe gpio in private data not platdata
> - Drop unneeded header file
>
>  drivers/net/designware.c | 76 ++++++++++++++++++++++++++++++++++++++++++++----
>  drivers/net/designware.h | 10 +++++++
>  2 files changed, 80 insertions(+), 6 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH v2 03/12] net: designware: Add a fix_mac_speed hook

[Simon Glass]

Hi Sjoerd,

On 28 February 2016 at 14:24, Sjoerd Simons
<sjoerd.simons@collabora.co.uk> wrote:
> Add the ability for e.g. drivers subclassing to register a function to
> be called after phy link negotiation. This is useful if e.g. the driver
> needs to change the mac configuration based on the negotiated speed.
>
> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
>
> ---
>
> Changes in v2:
> - Move the hook into the dw_adjust_link function
> - Rename the hook to fix_mac_speed, similar to Linux
>
>  drivers/net/designware.c | 8 ++++++--
>  drivers/net/designware.h | 4 ++++
>  2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/designware.c b/drivers/net/designware.c
> index 8834506..5eaa1de 100644
> --- a/drivers/net/designware.c
> +++ b/drivers/net/designware.c
> @@ -231,7 +231,8 @@ static int _dw_write_hwaddr(struct dw_eth_dev *priv, u8 *mac_id)
>         return 0;
>  }
>
> -static void dw_adjust_link(struct eth_mac_regs *mac_p,
> +static void dw_adjust_link(struct dw_eth_dev *priv,
> +                          struct eth_mac_regs *mac_p,
>                            struct phy_device *phydev)
>  {
>         u32 conf = readl(&mac_p->conf) | FRAMEBURSTENABLE | DISABLERXOWN;
> @@ -257,6 +258,9 @@ static void dw_adjust_link(struct eth_mac_regs *mac_p,
>         printf("Speed: %d, %s duplex%s\n", phydev->speed,
>                (phydev->duplex) ? "full" : "half",
>                (phydev->port == PORT_FIBRE) ? ", fiber mode" : "");
> +
> +       if (priv->fix_mac_speed)
> +               priv->fix_mac_speed(priv);
>  }
>
>  static void _dw_eth_halt(struct dw_eth_dev *priv)
> @@ -322,7 +326,7 @@ static int _dw_eth_init(struct dw_eth_dev *priv, u8 *enetaddr)
>                 return ret;
>         }
>
> -       dw_adjust_link(mac_p, priv->phydev);
> +       dw_adjust_link(priv, mac_p, priv->phydev);
>
>         if (!priv->phydev->link)
>                 return -EIO;
> diff --git a/drivers/net/designware.h b/drivers/net/designware.h
> index 6b4bfd7..792af7c 100644
> --- a/drivers/net/designware.h
> +++ b/drivers/net/designware.h
> @@ -237,6 +237,10 @@ struct dw_eth_dev {
>         struct gpio_desc reset_gpio;
>         struct phy_device *phydev;
>         struct mii_dev *bus;
> +
> +#ifdef CONFIG_DM_ETH
> +       int (*fix_mac_speed)(struct dw_eth_dev *priv);
> +#endif

I'd really like to avoid function pointers with driver model. This
seems to be called at the end of init. Can it be handled in your
driver instead?

>  };
>
>  #ifdef CONFIG_DM_ETH
> --
> 2.7.0
>

Regards,
Simon

[U-Boot] [PATCH v2 04/12] rockchip: rk3288: pinctrl: support more pins

[Simon Glass]

On 28 February 2016 at 14:24, Sjoerd Simons
<sjoerd.simons@collabora.co.uk> wrote:
> The rgmii_pins node in rk3288.dtsi configures 15 pins. Increase the size
> of the cell array to accomedate that, otherwise only the first 10 get
> configured.
>
> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
> ---
>
> Changes in v2: None
>
>  drivers/pinctrl/rockchip/pinctrl_rk3288.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

>
> diff --git a/drivers/pinctrl/rockchip/pinctrl_rk3288.c b/drivers/pinctrl/rockchip/pinctrl_rk3288.c
> index 7c769bd..1fa1daa 100644
> --- a/drivers/pinctrl/rockchip/pinctrl_rk3288.c
> +++ b/drivers/pinctrl/rockchip/pinctrl_rk3288.c
> @@ -623,7 +623,7 @@ static int rk3288_pinctrl_set_state(struct udevice *dev, struct udevice *config)
>  {
>         const void *blob = gd->fdt_blob;
>         int pcfg_node, ret, flags, count, i;
> -       u32 cell[40], *ptr;
> +       u32 cell[60], *ptr;
>
>         debug("%s: %s %s\n", __func__, dev->name, config->name);
>         ret = fdtdec_get_int_array_count(blob, config->of_offset,

It's tempting to add

 if (ret == ARRAY_SIZE(cell))
   return -ENOSPC;

> --
> 2.7.0
>

[U-Boot] [PATCH v2 05/12] rockchip: rk3288: Add clock support for the gmac ethernet interface

[Simon Glass]

On 28 February 2016 at 14:24, Sjoerd Simons
<sjoerd.simons@collabora.co.uk> wrote:
> Setup the clocks for the gmac ethernet interface. This assumes the mac
> clock is fed by an external clock which is common on RK3288 based
> devices.
>
> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
> ---
>
> Changes in v2: None
>
>  arch/arm/include/asm/arch-rockchip/cru_rk3288.h | 17 +++++++++++++++++
>  drivers/clk/clk_rk3288.c                        | 14 ++++++++++++++
>  2 files changed, 31 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH v2 06/12] rockchip: rk3288: grf: Define GRF_SOC_CON1 and GRF_SOC_CON3

[Simon Glass]

On 28 February 2016 at 14:25, Sjoerd Simons
<sjoerd.simons@collabora.co.uk> wrote:
> Add definitions for GRF_SOC_CON1 and GRF_SOC_CON3 which contain various
> GMAC related fields.
>
> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
> ---
>
> Changes in v2: None
>
>  arch/arm/include/asm/arch-rockchip/grf_rk3288.h | 53 +++++++++++++++++++++++++
>  1 file changed, 53 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH v2 07/12] net: gmac_rk3288: Add RK3288 GMAC driver

[Simon Glass]

Hi Sjoerd,

On 28 February 2016 at 14:25, Sjoerd Simons
<sjoerd.simons@collabora.co.uk> wrote:
> Add a new driver for the GMAC ethernet interface present in Rockchip
> RK3288 SOCs. This driver subclasses the generic design-ware driver to
> add the glue needed specifically for Rockchip.
>
> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
>
> ---
>
> Changes in v2:
> - Fix various coding style nits
> - Adjust to new hook name
>
>  drivers/net/Kconfig       |   7 +++
>  drivers/net/Makefile      |   1 +
>  drivers/net/gmac_rk3288.c | 125 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 133 insertions(+)
>  create mode 100644 drivers/net/gmac_rk3288.c
>
> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> index bc2f51d..fa49856 100644
> --- a/drivers/net/Kconfig
> +++ b/drivers/net/Kconfig
> @@ -133,4 +133,11 @@ config PIC32_ETH
>           This driver implements 10/100 Mbps Ethernet and MAC layer for
>           Microchip PIC32 microcontrollers.
>
> +config GMAC_RK3288
> +       bool "Rockchip RK3288 Synopsys Designware Ethernet MAC"
> +       depends on DM_ETH && ETH_DESIGNWARE
> +       help
> +         This driver provides Rockchip RK3288 network support based on the
> +         Synopsys Designware driver.
> +
>  endif # NETDEVICES
> diff --git a/drivers/net/Makefile b/drivers/net/Makefile
> index 33a81ee..d0a8009 100644
> --- a/drivers/net/Makefile
> +++ b/drivers/net/Makefile
> @@ -32,6 +32,7 @@ obj-$(CONFIG_FTGMAC100) += ftgmac100.o
>  obj-$(CONFIG_FTMAC110) += ftmac110.o
>  obj-$(CONFIG_FTMAC100) += ftmac100.o
>  obj-$(CONFIG_GRETH) += greth.o
> +obj-$(CONFIG_GMAC_RK3288) += gmac_rk3288.o
>  obj-$(CONFIG_DRIVER_TI_KEYSTONE_NET) += keystone_net.o
>  obj-$(CONFIG_KS8851_MLL) += ks8851_mll.o
>  obj-$(CONFIG_LAN91C96) += lan91c96.o
> diff --git a/drivers/net/gmac_rk3288.c b/drivers/net/gmac_rk3288.c
> new file mode 100644
> index 0000000..5400b2c
> --- /dev/null
> +++ b/drivers/net/gmac_rk3288.c
> @@ -0,0 +1,125 @@
> +/*
> + * (C) Copyright 2015 Sjoerd Simons <sjoerd.simons@collabora.co.uk>
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + */
> +
> +/* * Rockchip GMAC ethernet IP driver for U-Boot */

/* Rockchip...

> +#include <common.h>
> +#include <dm.h>
> +#include <dm/pinctrl.h>
> +#include <asm/gpio.h>
> +#include <clk.h>
> +#include <phy.h>
> +#include <syscon.h>
> +#include <asm/io.h>
> +#include <asm/arch/periph.h>
> +#include <asm/arch/clock.h>
> +#include <asm/arch/grf_rk3288.h>
> +#include "designware.h"
> +#include <dt-bindings/clock/rk3288-cru.h>

Can you sort these?

http://www.denx.de/wiki/U-Boot/CodingStyle

> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +struct gmac_rk3288_platdata {
> +       struct dw_eth_pdata dw_eth_pdata;
> +       int tx_delay;
> +       int rx_delay;
> +};
> +
> +static int gmac_rk3288_ofdata_to_platdata(struct udevice *dev)
> +{
> +       struct gmac_rk3288_platdata *pdata = dev_get_platdata(dev);
> +
> +       pdata->tx_delay = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
> +                                        "tx_delay", 0x30);
> +       pdata->rx_delay = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
> +                                        "rx_delay", 0x10);
> +
> +       return designware_eth_ofdata_to_platdata(dev);
> +}
> +
> +static int gmac_rk3288_fix_mac_speed(struct dw_eth_dev *priv)
> +{
> +       struct rk3288_grf *grf;
> +       int clk;
> +
> +       switch (priv->phydev->speed) {
> +       case 10:
> +               clk = GMAC_CLK_SEL_2_5M;
> +               break;
> +       case 100:
> +               clk = GMAC_CLK_SEL_25M;
> +               break;
> +       case 1000:
> +               clk = GMAC_CLK_SEL_125M;
> +               break;
> +       default:
> +               printf("Unknown phy speed: %d\n", priv->phydev->speed);
> +               return -EINVAL;
> +       }
> +
> +       grf = syscon_get_first_range(ROCKCHIP_SYSCON_GRF);
> +
> +       rk_clrsetreg(&grf->soc_con1,
> +                    GMAC_CLK_SEL_MASK << GMAC_CLK_SEL_SHIFT,
> +                    clk << GMAC_CLK_SEL_SHIFT);
> +
> +       return 0;
> +}
> +
> +static int gmac_rk3288_probe(struct udevice *dev)
> +{
> +       int ret;
> +       struct gmac_rk3288_platdata *pdata = dev_get_platdata(dev);
> +       struct dw_eth_dev *priv = dev_get_priv(dev);
> +       struct rk3288_grf *grf;
> +       struct udevice *clk;
> +
> +

Remove extra blank line

> +       ret = uclass_get_device(UCLASS_CLK, CLK_GENERAL, &clk);
> +       if (ret)
> +               return ret;

Can you use clk_get_by_index() ?

> +
> +       ret = clk_set_periph_rate(clk, SCLK_MAC, 0);
> +       if (ret)
> +               return ret;
> +
> +       /* Set to RGMII mode */
> +       grf = syscon_get_first_range(ROCKCHIP_SYSCON_GRF);
> +       rk_clrsetreg(&grf->soc_con1,
> +                    RMII_MODE_MASK << RMII_MODE_SHIFT |
> +                    GMAC_PHY_INTF_SEL_MASK << GMAC_PHY_INTF_SEL_SHIFT,
> +                    GMAC_PHY_INTF_SEL_RGMII << GMAC_PHY_INTF_SEL_SHIFT);
> +
> +       rk_clrsetreg(&grf->soc_con3,
> +                    RXCLK_DLY_ENA_GMAC_MASK <<  RXCLK_DLY_ENA_GMAC_SHIFT |
> +                    TXCLK_DLY_ENA_GMAC_MASK <<  TXCLK_DLY_ENA_GMAC_SHIFT |
> +                    CLK_RX_DL_CFG_GMAC_MASK <<  CLK_RX_DL_CFG_GMAC_SHIFT |
> +                    CLK_TX_DL_CFG_GMAC_MASK <<  CLK_TX_DL_CFG_GMAC_SHIFT,
> +                    RXCLK_DLY_ENA_GMAC_ENABLE << RXCLK_DLY_ENA_GMAC_SHIFT |
> +                    TXCLK_DLY_ENA_GMAC_ENABLE << TXCLK_DLY_ENA_GMAC_SHIFT |
> +                    pdata->rx_delay << CLK_RX_DL_CFG_GMAC_SHIFT |
> +                    pdata->tx_delay << CLK_TX_DL_CFG_GMAC_SHIFT);
> +
> +       priv->fix_mac_speed = gmac_rk3288_fix_mac_speed;
> +
> +       return designware_eth_probe(dev);

This presumably called gmac_rk3288_fix_mac_speed(). Is it possible to
split the init so that you can call gmac_rk3288_fix_mac_speed()
directly here?

> +}
> +
> +static const struct udevice_id rk3288_gmac_ids[] = {
> +       { .compatible = "rockchip,rk3288-gmac" },
> +       { }
> +};
> +
> +U_BOOT_DRIVER(eth_gmac_rk3288) = {
> +       .name   = "gmac_rk3288",
> +       .id     = UCLASS_ETH,
> +       .of_match = rk3288_gmac_ids,
> +       .ofdata_to_platdata = gmac_rk3288_ofdata_to_platdata,
> +       .probe  = gmac_rk3288_probe,
> +       .ops    = &designware_eth_ops,
> +       .priv_auto_alloc_size = sizeof(struct dw_eth_dev),
> +       .platdata_auto_alloc_size = sizeof(struct gmac_rk3288_platdata),
> +       .flags = DM_FLAG_ALLOC_PRIV_DMA,
> +};
> --
> 2.7.0
>

Regards,
Simon

[U-Boot] [PATCH v2 09/12] rockchip: rock2: dts: use status = "okay" not ok

[Simon Glass]

On 28 February 2016 at 14:25, Sjoerd Simons
<sjoerd.simons@collabora.co.uk> wrote:
> u-boot only recognize okay to enable a node (Linux seems to be more
> lenient here). So use okay instead.
>
> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
> ---
>
> Changes in v2: None
>
>  arch/arm/dts/rk3288-rock2-square.dts | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH v2 10/12] rockchip: Enable networking support on rock2 and firefly

[Simon Glass]

On 28 February 2016 at 14:25, Sjoerd Simons
<sjoerd.simons@collabora.co.uk> wrote:
> Enable the various configuration option required to get the ethernet
> interface up and running on Radxa Rock2 and Firefly.
>
> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
> ---
>
> Changes in v2: None
>
>  configs/firefly-rk3288_defconfig | 5 +++++
>  configs/rock2_defconfig          | 5 +++++
>  2 files changed, 10 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH v2 07/12] net: gmac_rk3288: Add RK3288 GMAC driver

[Sjoerd Simons]

On Mon, 2016-02-29 at 19:03 -0700, Simon Glass wrote:
> Hi Sjoerd,
> 
> On 28 February 2016 at 14:25, Sjoerd Simons
> <sjoerd.simons@collabora.co.uk> wrote:
> > 
> >?
> > diff --git a/drivers/net/gmac_rk3288.c b/drivers/net/gmac_rk3288.c
> > new file mode 100644
> > index 0000000..5400b2c
> > --- /dev/null
> > +++ b/drivers/net/gmac_rk3288.c
> > @@ -0,0 +1,125 @@
> >?
> > +???????priv->fix_mac_speed = gmac_rk3288_fix_mac_speed;
> > +
> > +???????return designware_eth_probe(dev);
> This presumably called gmac_rk3288_fix_mac_speed(). Is it possible to
> split the init so that you can call gmac_rk3288_fix_mac_speed()
> directly here?

No it gets called down the line from dw eth_ops start function once the
ethernet driver is started and the phy speed is known so the mac can be
adjust to match.

So to do it in the driver, it would need to override the eth start op
from its parent. Possible by exporting more of the designwares
implementation so specific ops can be overridden.?

Seems less elegant then having a hook in the semantically right spot
(e.g. after the link speed is determined)?

Ooi what's your worry with function pointers in DM ?



-- 
Sjoerd Simons
Collabora Ltd.

[U-Boot] [PATCH v2 07/12] net: gmac_rk3288: Add RK3288 GMAC driver

[Simon Glass]

Hi Sjeord,

On 11 March 2016 at 14:56, Sjoerd Simons <sjoerd.simons@collabora.co.uk> wrote:
> On Mon, 2016-02-29 at 19:03 -0700, Simon Glass wrote:
>> Hi Sjoerd,
>>
>> On 28 February 2016 at 14:25, Sjoerd Simons
>> <sjoerd.simons@collabora.co.uk> wrote:
>> >
>> >
>> > diff --git a/drivers/net/gmac_rk3288.c b/drivers/net/gmac_rk3288.c
>> > new file mode 100644
>> > index 0000000..5400b2c
>> > --- /dev/null
>> > +++ b/drivers/net/gmac_rk3288.c
>> > @@ -0,0 +1,125 @@
>> >
>> > +       priv->fix_mac_speed = gmac_rk3288_fix_mac_speed;
>> > +
>> > +       return designware_eth_probe(dev);
>> This presumably called gmac_rk3288_fix_mac_speed(). Is it possible to
>> split the init so that you can call gmac_rk3288_fix_mac_speed()
>> directly here?
>
> No it gets called down the line from dw eth_ops start function once the
> ethernet driver is started and the phy speed is known so the mac can be
> adjust to match.
>
> So to do it in the driver, it would need to override the eth start op
> from its parent. Possible by exporting more of the designwares
> implementation so specific ops can be overridden.
>
> Seems less elegant then having a hook in the semantically right spot
> (e.g. after the link speed is determined)?
>
> Ooi what's your worry with function pointers in DM ?

I'm trying to avoid them. It is effectively an unofficial API that
doesn't go through DM. It suggests there is an extra layer of
software, but not quite...it is messy.

In the same vein I'm not keen on weak functions.

It looks to me as if the designware driver, which is currently
stand-alone, is being extended into a slightly different driver by
your patch. We really avoid that because we'll end up with more and
more fiddling for each SoC to make it work the way we want. This is
the kind of thing that driver model aims to avoid.

In that case here's my suggestion:
- Turn designware.c into a code library instead of a driver, and
export various functions
- Add a new dw_mac.c or something, which has the compatible strings
and the U_BOOT_DRIVER() declaration, and uses the
designware_eth_start(), etc. calls from designware.c
- Make your new driver do the same, except use your own start function

I see at the end of _dw_eth_init() there is a writel() that might need
to happen after your link adjustment. In that case this could be moved
into a separate function perhaps.

Regards,
Simon

[U-Boot] [PATCH v2 01/12] net: designware: support phy reset device-tree bindings

[Simon Glass]

On 29 February 2016 at 19:03, Simon Glass <sjg@chromium.org> wrote:
> On 28 February 2016 at 14:24, Sjoerd Simons
> <sjoerd.simons@collabora.co.uk> wrote:
>> Add support for the snps,reset-gpio, snps,reset-active-low (optional) and
>> snps,reset-delays-us device-tree bindings. The combination of these
>> three define how the PHY should be reset to ensure it's in a sane state.
>>
>> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
>>
>> ---
>>
>> Changes in v2:
>> - Store probe gpio in private data not platdata
>> - Drop unneeded header file
>>
>>  drivers/net/designware.c | 76 ++++++++++++++++++++++++++++++++++++++++++++----
>>  drivers/net/designware.h | 10 +++++++
>>  2 files changed, 80 insertions(+), 6 deletions(-)
>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot-rockchip/next, thanks!

[U-Boot] [PATCH v2 04/12] rockchip: rk3288: pinctrl: support more pins

[Simon Glass]

On 29 February 2016 at 19:03, Simon Glass <sjg@chromium.org> wrote:
> On 28 February 2016 at 14:24, Sjoerd Simons
> <sjoerd.simons@collabora.co.uk> wrote:
>> The rgmii_pins node in rk3288.dtsi configures 15 pins. Increase the size
>> of the cell array to accomedate that, otherwise only the first 10 get
>> configured.
>>
>> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
>> ---
>>
>> Changes in v2: None
>>
>>  drivers/pinctrl/rockchip/pinctrl_rk3288.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
>
>>
>> diff --git a/drivers/pinctrl/rockchip/pinctrl_rk3288.c b/drivers/pinctrl/rockchip/pinctrl_rk3288.c
>> index 7c769bd..1fa1daa 100644
>> --- a/drivers/pinctrl/rockchip/pinctrl_rk3288.c
>> +++ b/drivers/pinctrl/rockchip/pinctrl_rk3288.c
>> @@ -623,7 +623,7 @@ static int rk3288_pinctrl_set_state(struct udevice *dev, struct udevice *config)
>>  {
>>         const void *blob = gd->fdt_blob;
>>         int pcfg_node, ret, flags, count, i;
>> -       u32 cell[40], *ptr;
>> +       u32 cell[60], *ptr;
>>
>>         debug("%s: %s %s\n", __func__, dev->name, config->name);
>>         ret = fdtdec_get_int_array_count(blob, config->of_offset,
>
> It's tempting to add
>
>  if (ret == ARRAY_SIZE(cell))
>    return -ENOSPC;
>
>> --
>> 2.7.0
>>

Applied to u-boot-rockchip/next, thanks!

[U-Boot] [PATCH v2 05/12] rockchip: rk3288: Add clock support for the gmac ethernet interface

[Simon Glass]

On 29 February 2016 at 19:03, Simon Glass <sjg@chromium.org> wrote:
> On 28 February 2016 at 14:24, Sjoerd Simons
> <sjoerd.simons@collabora.co.uk> wrote:
>> Setup the clocks for the gmac ethernet interface. This assumes the mac
>> clock is fed by an external clock which is common on RK3288 based
>> devices.
>>
>> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
>> ---
>>
>> Changes in v2: None
>>
>>  arch/arm/include/asm/arch-rockchip/cru_rk3288.h | 17 +++++++++++++++++
>>  drivers/clk/clk_rk3288.c                        | 14 ++++++++++++++
>>  2 files changed, 31 insertions(+)
>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot-rockchip/next, thanks!

[U-Boot] [PATCH v2 06/12] rockchip: rk3288: grf: Define GRF_SOC_CON1 and GRF_SOC_CON3

[Simon Glass]

On 29 February 2016 at 19:03, Simon Glass <sjg@chromium.org> wrote:
> On 28 February 2016 at 14:25, Sjoerd Simons
> <sjoerd.simons@collabora.co.uk> wrote:
>> Add definitions for GRF_SOC_CON1 and GRF_SOC_CON3 which contain various
>> GMAC related fields.
>>
>> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
>> ---
>>
>> Changes in v2: None
>>
>>  arch/arm/include/asm/arch-rockchip/grf_rk3288.h | 53 +++++++++++++++++++++++++
>>  1 file changed, 53 insertions(+)
>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot-rockchip/next, thanks!

[U-Boot] [PATCH v2 08/12] rockchip: rk3288-firefly: Add gmac definition

[Simon Glass]

On 28 February 2016 at 14:25, Sjoerd Simons
<sjoerd.simons@collabora.co.uk> wrote:
> Add a definition for the gmac interface to the firefly device-tree.
> Copied verbatim from the linux kernel.
>
> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
> Acked-by: Simon Glass <sjg@chromium.org>
> ---
>
> Changes in v2: None
>
>  arch/arm/dts/rk3288-firefly.dtsi | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)

Applied to u-boot-rockchip/next, thanks!

[U-Boot] [PATCH v2 09/12] rockchip: rock2: dts: use status = "okay" not ok

[Simon Glass]

On 29 February 2016 at 19:03, Simon Glass <sjg@chromium.org> wrote:
> On 28 February 2016 at 14:25, Sjoerd Simons
> <sjoerd.simons@collabora.co.uk> wrote:
>> u-boot only recognize okay to enable a node (Linux seems to be more
>> lenient here). So use okay instead.
>>
>> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
>> ---
>>
>> Changes in v2: None
>>
>>  arch/arm/dts/rk3288-rock2-square.dts | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot-rockchip/next, thanks!