Skip to contentSkip to conversation

net: fec_mxc: Convert to DM

PATCH Superseded Jagan Teki 2 participants 5 messages Oct 2, 2016 – Oct 5, 2016

Subsystems (3)

v3 of 7 Superseded Latest is v7 →

lore
Share & tools
lore

Jagan Teki

author
#0 lore raw

v3 cover net: fec_mxc: Convert to DM

From: Jagan Teki <[email protected]>

This series convert fec_mxc to DM and tested both dm and non-dm code. This is on top of engicam qdl[1]

Changes for v3:
- Add ARM: dts: imx6qdl-icore: Add FEC support
- icorem6: Use CONFIG_DM_ETH support
Changes for v2:
- Add TODO for implementing the enet reset code

[1] https://www.mail-archive.com/u-boot at lists.denx.de/msg226671.html

Jagan Teki (3):
net: fec_mxc: Convert into driver model
ARM: dts: imx6qdl-icore: Add FEC support
icorem6: Use CONFIG_DM_ETH support

 arch/arm/cpu/armv7/mx6/Kconfig       |   1 +
 arch/arm/dts/imx6qdl-icore.dtsi      |  24 +++
 board/engicam/icorem6/icorem6.c      |  71 ---------
 configs/imx6qdl_icore_mmc_defconfig  |   1 -
 configs/imx6qdl_icore_nand_defconfig |   1 -
 drivers/net/fec_mxc.c                | 273 +++++++++++++++++++++++++++++++----
 drivers/net/fec_mxc.h                |  11 ++
 7 files changed, 283 insertions(+), 99 deletions(-)
Signature · 1 line
2.7.4
author
#1 lore raw

PATCH v3 1/3 net: fec_mxc: Convert into driver model

From: Jagan Teki <[email protected]>

This patch add driver model support for fec_mxc driver.

Cc Simon Glass <[email protected]>
Cc Joe Hershberger <[email protected]>
Cc Peng Fan <[email protected]>
Cc Stefano Babic <[email protected]>
Cc Michael Trimarchi <[email protected]>
---
 drivers/net/fec_mxc.c | 273 +++++++++++++++++++++++++++++++++++++++++++++-----
 drivers/net/fec_mxc.h |  11 ++
 2 files changed, 258 insertions(+), 26 deletions(-)
drivers/net/fec_mxc.c +247 -26
index e871b3e..d4f6a08 100644
@@ -9,6 +9,7 @@
  */
 
 #include <common.h>
+#include <dm.h>
 #include <malloc.h>
 #include <memalign.h>
 #include <net.h>
@@ -362,17 +363,32 @@ static void fec_rbd_clean(int last, struct fec_bd *pRbd)
 	writew(0, &pRbd->data_length);
 }
 
+#ifdef CONFIG_DM_ETH
+static int fec_get_hwaddr(struct udevice *dev, int dev_id,
+						unsigned char *mac)
+#else
 static int fec_get_hwaddr(struct eth_device *dev, int dev_id,
 						unsigned char *mac)
+#endif
 {
 	imx_get_mac_from_fuse(dev_id, mac);
 	return !is_valid_ethaddr(mac);
 }
 
+#ifdef CONFIG_DM_ETH
+static int fecmxc_set_hwaddr(struct udevice *dev)
+#else
 static int fec_set_hwaddr(struct eth_device *dev)
+#endif
 {
+#ifdef CONFIG_DM_ETH
+	struct fec_priv *fec = dev_get_priv(dev);
+	struct eth_pdata *pdata = dev_get_platdata(dev);
+	uchar *mac = pdata->enetaddr;
+#else
 	uchar *mac = dev->enetaddr;
 	struct fec_priv *fec = (struct fec_priv *)dev->priv;
+#endif
 
 	writel(0, &fec->eth->iaddr1);
 	writel(0, &fec->eth->iaddr2);
@@ -427,9 +443,17 @@ static void fec_reg_setup(struct fec_priv *fec)
  * Start the FEC engine
  * @param[in] dev Our device to handle
  */
+#ifdef CONFIG_DM_ETH
+static int fec_open(struct udevice *dev)
+#else
 static int fec_open(struct eth_device *edev)
+#endif
 {
+#ifdef CONFIG_DM_ETH
+	struct fec_priv *fec = dev_get_priv(dev);
+#else
 	struct fec_priv *fec = (struct fec_priv *)edev->priv;
+#endif
 	int speed;
 	uint32_t addr, size;
 	int i;
@@ -535,14 +559,26 @@ static int fec_open(struct eth_device *edev)
 	return 0;
 }
 
+#ifdef CONFIG_DM_ETH
+static int fecmxc_init(struct udevice *dev)
+#else
 static int fec_init(struct eth_device *dev, bd_t* bd)
+#endif
 {
+#ifdef CONFIG_DM_ETH
+	struct fec_priv *fec = dev_get_priv(dev);
+#else
 	struct fec_priv *fec = (struct fec_priv *)dev->priv;
+#endif
 	uint32_t mib_ptr = (uint32_t)&fec->eth->rmon_t_drop;
 	int i;
 
 	/* Initialize MAC address */
+#ifdef CONFIG_DM_ETH
+	fecmxc_set_hwaddr(dev);
+#else
 	fec_set_hwaddr(dev);
+#endif
 
 	/*
 	 * Setup transmit descriptors, there are two in total.
@@ -596,9 +632,17 @@ static int fec_init(struct eth_device *dev, bd_t* bd)
  * Halt the FEC engine
  * @param[in] dev Our device to handle
  */
+#ifdef CONFIG_DM_ETH
+static void fecmxc_halt(struct udevice *dev)
+#else
 static void fec_halt(struct eth_device *dev)
+#endif
 {
+#ifdef CONFIG_DM_ETH
+	struct fec_priv *fec = dev_get_priv(dev);
+#else
 	struct fec_priv *fec = (struct fec_priv *)dev->priv;
+#endif
 	int counter = 0xffff;
 
 	/*
@@ -638,7 +682,11 @@ static void fec_halt(struct eth_device *dev)
  * @param[in] length Data count in bytes
  * @return 0 on success
  */
+#ifdef CONFIG_DM_ETH
+static int fecmxc_send(struct udevice *dev, void *packet, int length)
+#else
 static int fec_send(struct eth_device *dev, void *packet, int length)
+#endif
 {
 	unsigned int status;
 	uint32_t size, end;
@@ -650,7 +698,11 @@ static int fec_send(struct eth_device *dev, void *packet, int length)
 	 * This routine transmits one frame.  This routine only accepts
 	 * 6-byte Ethernet addresses.
 	 */
+#ifdef CONFIG_DM_ETH
+	struct fec_priv *fec = dev_get_priv(dev);
+#else
 	struct fec_priv *fec = (struct fec_priv *)dev->priv;
+#endif
 
 	/*
 	 * Check for valid length of data.
@@ -783,9 +835,17 @@ out:
  * @param[in] dev Our ethernet device to handle
  * @return Length of packet read
  */
+#ifdef CONFIG_DM_ETH
+static int fecmxc_recv(struct udevice *dev, int flags, uchar **packetp)
+#else
 static int fec_recv(struct eth_device *dev)
+#endif
 {
+#ifdef CONFIG_DM_ETH
+	struct fec_priv *fec = dev_get_priv(dev);
+#else
 	struct fec_priv *fec = (struct fec_priv *)dev->priv;
+#endif
 	struct fec_bd *rbd = &fec->rbd_base[fec->rbd_index];
 	unsigned long ievent;
 	int frame_length, len = 0;
@@ -801,8 +861,13 @@ static int fec_recv(struct eth_device *dev)
 	writel(ievent, &fec->eth->ievent);
 	debug("fec_recv: ievent 0x%lx\n", ievent);
 	if (ievent & FEC_IEVENT_BABR) {
+#ifdef CONFIG_DM_ETH
+		fecmxc_halt(dev);
+		fecmxc_init(dev);
+#else
 		fec_halt(dev);
 		fec_init(dev, fec->bd);
+#endif
 		printf("some error: 0x%08lx\n", ievent);
 		return 0;
 	}
@@ -814,10 +879,18 @@ static int fec_recv(struct eth_device *dev)
 	if (ievent & FEC_IEVENT_GRA) {
 		/* Graceful stop complete */
 		if (readl(&fec->eth->x_cntrl) & 0x00000001) {
+#ifdef CONFIG_DM_ETH
+			fecmxc_halt(dev);
+#else
 			fec_halt(dev);
+#endif
 			writel(~0x00000001 & readl(&fec->eth->x_cntrl),
 					&fec->eth->x_cntrl);
+#ifdef CONFIG_DM_ETH
+			fecmxc_init(dev);
+#else
 			fec_init(dev, fec->bd);
+#endif
 		}
 	}
 
@@ -971,6 +1044,33 @@ static void fec_free_descs(struct fec_priv *fec)
 	free(fec->tbd_base);
 }
 
+struct mii_dev *fec_get_miibus(uint32_t base_addr, int dev_id)
+{
+	struct ethernet_regs *eth = (struct ethernet_regs *)base_addr;
+	struct mii_dev *bus;
+	int ret;
+
+	bus = mdio_alloc();
+	if (!bus) {
+		printf("mdio_alloc failed\n");
+		return NULL;
+	}
+	bus->read = fec_phy_read;
+	bus->write = fec_phy_write;
+	bus->priv = eth;
+	fec_set_dev_name(bus->name, dev_id);
+
+	ret = mdio_register(bus);
+	if (ret) {
+		printf("mdio_register failed\n");
+		free(bus);
+		return NULL;
+	}
+	fec_mii_setspeed(eth);
+	return bus;
+}
+
+#ifndef CONFIG_DM_ETH
 #ifdef CONFIG_PHYLIB
 int fec_probe(bd_t *bd, int dev_id, uint32_t base_addr,
 		struct mii_dev *bus, struct phy_device *phydev)
@@ -1062,32 +1162,6 @@ err1:
 	return ret;
 }
 
-struct mii_dev *fec_get_miibus(uint32_t base_addr, int dev_id)
-{
-	struct ethernet_regs *eth = (struct ethernet_regs *)base_addr;
-	struct mii_dev *bus;
-	int ret;
-
-	bus = mdio_alloc();
-	if (!bus) {
-		printf("mdio_alloc failed\n");
-		return NULL;
-	}
-	bus->read = fec_phy_read;
-	bus->write = fec_phy_write;
-	bus->priv = eth;
-	fec_set_dev_name(bus->name, dev_id);
-
-	ret = mdio_register(bus);
-	if (ret) {
-		printf("mdio_register failed\n");
-		free(bus);
-		return NULL;
-	}
-	fec_mii_setspeed(eth);
-	return bus;
-}
-
 int fecmxc_initialize_multi(bd_t *bd, int dev_id, int phy_id, uint32_t addr)
 {
 	uint32_t base_mii;
@@ -1147,3 +1221,150 @@ int fecmxc_register_mii_postcall(struct eth_device *dev, int (*cb)(int))
 	return 0;
 }
 #endif
+
+#else
+
+static const struct eth_ops fecmxc_ops = {
+	.start			= fecmxc_init,
+	.send			= fecmxc_send,
+	.recv			= fecmxc_recv,
+	.stop			= fecmxc_halt,
+	.write_hwaddr		= fecmxc_set_hwaddr,
+};
+
+static int fec_phy_init(struct fec_priv *priv, struct udevice *dev)
+{
+	struct phy_device *phydev;
+	int mask = 0xffffffff;
+
+#ifdef CONFIG_PHYLIB
+	mask = 1 << CONFIG_FEC_MXC_PHYADDR;
+#endif
+
+	phydev = phy_find_by_mask(priv->bus, mask, priv->interface);
+	if (!phydev)
+		return -ENODEV;
+
+	phy_connect_dev(phydev, dev);
+
+	priv->phydev = phydev;
+	phy_config(phydev);
+
+	return 0;
+}
+
+static int fecmxc_probe(struct udevice *dev)
+{
+	struct eth_pdata *pdata = dev_get_platdata(dev);
+	struct fec_priv *priv = dev_get_priv(dev);
+	struct mii_dev *bus = NULL;
+	int dev_id = -1;
+	unsigned char ethaddr[6];
+	uint32_t start;
+	int ret;
+
+	ret = fec_alloc_descs(priv);
+	if (ret)
+		return ret;
+
+	bus = fec_get_miibus((uint32_t)priv->eth, dev_id);
+	if (!bus)
+		goto err_mii;
+
+	priv->bus = bus;
+	priv->xcv_type = CONFIG_FEC_XCV_TYPE;
+	priv->interface = pdata->phy_interface;
+	ret = fec_phy_init(priv, dev);
+	if (ret)
+		goto err_phy;
+
+	/* Reset chip. */
+	writel(readl(&priv->eth->ecntrl) | FEC_ECNTRL_RESET, &priv->eth->ecntrl);
+	start = get_timer(0);
+	while (readl(&priv->eth->ecntrl) & FEC_ECNTRL_RESET) {
+		if (get_timer(start) > (CONFIG_SYS_HZ * 5)) {
+			printf("FEC MXC: Timeout reseting chip\n");
+			goto err_timeout;
+		}
+		udelay(10);
+	}
+
+	fec_reg_setup(priv);
+	fec_set_dev_name((char *)dev->name, dev_id);
+	priv->dev_id = (dev_id == -1) ? 0 : dev_id;
+
+	ret = fec_get_hwaddr(dev, dev_id, ethaddr);
+	if (!ret) {
+		debug("got MAC%d address from fuse: %pM\n", dev_id, ethaddr);
+		memcpy(pdata->enetaddr, ethaddr, 6);
+		if (!getenv("ethaddr"))
+			eth_setenv_enetaddr("ethaddr", ethaddr);
+	}
+
+	return 0;
+
+err_timeout:
+	free(priv->phydev);
+err_phy:
+	mdio_unregister(bus);
+	free(bus);
+err_mii:
+	fec_free_descs(priv);
+	return ret;
+}
+
+static int fecmxc_remove(struct udevice *dev)
+{
+	struct fec_priv *priv = dev_get_priv(dev);
+
+	free(priv->phydev);
+	fec_free_descs(priv);
+	mdio_unregister(priv->bus);
+	mdio_free(priv->bus);
+
+	return 0;
+}
+
+static int fecmxc_ofdata_to_platdata(struct udevice *dev)
+{
+	struct eth_pdata *pdata = dev_get_platdata(dev);
+	struct fec_priv *priv = dev_get_priv(dev);
+	const char *phy_mode;
+
+	pdata->iobase = (phys_addr_t)dev_get_addr(dev);
+	priv->eth = (struct ethernet_regs *)pdata->iobase;
+
+	pdata->phy_interface = -1;
+	phy_mode = fdt_getprop(gd->fdt_blob, dev->of_offset, "phy-mode", NULL);
+	if (phy_mode)
+		pdata->phy_interface = phy_get_interface_by_name(phy_mode);
+	if (pdata->phy_interface == -1) {
+		debug("%s: Invalid PHY interface '%s'\n", __func__, phy_mode);
+		return -EINVAL;
+	}
+
+	/* TODO
+	 * Need to get the reset-gpio and related properties from DT
+	 * and implemet the enet reset code on .probe call
+	 */
+
+	return 0;
+}
+
+static const struct udevice_id fecmxc_ids[] = {
+	{ .compatible = "fsl,imx6q-fec" },
+	{ }
+};
+
+U_BOOT_DRIVER(fecmxc_gem) = {
+	.name	= "fecmxc",
+	.id	= UCLASS_ETH,
+	.of_match = fecmxc_ids,
+	.ofdata_to_platdata = fecmxc_ofdata_to_platdata,
+	.probe	= fecmxc_probe,
+	.remove	= fecmxc_remove,
+	.ops	= &fecmxc_ops,
+	.priv_auto_alloc_size = sizeof(struct fec_priv),
+	.platdata_auto_alloc_size = sizeof(struct eth_pdata),
+};
+#endif
drivers/net/fec_mxc.h +11 -0
index 0717cc6..84f4149 100644
@@ -18,6 +18,10 @@
 #ifndef __FEC_MXC_H
 #define __FEC_MXC_H
 
+#ifdef CONFIG_DM_GPIO
+#include <asm-generic/gpio.h>
+#endif
+
 void imx_get_mac_from_fuse(int dev_id, unsigned char *mac);
 
 /**
@@ -264,6 +268,13 @@ struct fec_priv {
 	int phy_id;
 	int (*mii_postcall)(int);
 #endif
+
+#ifdef CONFIG_DM_GPIO
+	struct gpio_desc reset_gpio;
+#endif
+#ifdef CONFIG_DM_ETH
+	u32 interface;
+#endif
 };
 
 /**
Signature · 1 line
2.7.4
#2 lore raw

PATCH v3 1/3 net: fec_mxc: Convert into driver model

Hi,

Quoted 10 lines · Jagan Teki

From: Jagan Teki <[email protected]>

This patch add driver model support for fec_mxc driver.

Cc Simon Glass <[email protected]>
Cc Joe Hershberger <[email protected]>
Cc Peng Fan <[email protected]>
Cc Stefano Babic <[email protected]>
Cc Michael Trimarchi <[email protected]>
Quoted 4 lines
---
 drivers/net/fec_mxc.c | 273 +++++++++++++++++++++++++++++++++++++++++++++-----
 drivers/net/fec_mxc.h |  11 ++
 2 files changed, 258 insertions(+), 26 deletions(-)

I think you would have an easier time if you move all the common code into common functions, and just have stubs for the legacy and new DM path. For example fecmxc_probe() is repeating code. There really should be almost no duplicated code in the driver. It just makes it hard to maintain, and understand what is happening.

I think it is best to avoid renaming the functions. So for example:

#ifdef CONFIG_DM_ETH
static int fecmxc_recv(struct udevice *dev, int flags, uchar **packetp) #else
static int fec_recv(struct eth_device *dev)
#endif
{

You may as well keep the name the same - fec_recv().

In one case you have not put the DM_ETH case first. It should be:

#ifdef CONFIG_DM_ETH
...
#else
#endif

rather than:

#ifndef CONFIG_DM_ETH
...
#else
#endif

I'm not sure you are dealing with all the cases. Unfortunately the driver already has #idefs. For example if CONFIG_PHYLIB is not defined. With CONFIG_DM_ETH, struct eth_device will not be available, so you need to make sure no code builds with that.

Also fecmxc_recv() does not appear to work. It needs to set packetp and return a packet length. Also, do you need a free_pkt()?

Regards,
Simon

author
#3 lore raw

PATCH v3 2/3 ARM: dts: imx6qdl-icore: Add FEC support

In reply to Jagan Teki

From: Jagan Teki <[email protected]>

Add FEC dts support for Engicam i.CoreM6 dql modules.

Cc Joe Hershberger <[email protected]>
Cc Stefano Babic <[email protected]>
Cc Matteo Lisi <[email protected]>
Cc Michael Trimarchi <[email protected]>
---
 arch/arm/dts/imx6qdl-icore.dtsi | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)
arch/arm/dts/imx6qdl-icore.dtsi +24 -0
index f424cd5..a485c3e 100644
@@ -75,6 +75,14 @@
 	assigned-clock-parents = <&clks IMX6QDL_CLK_OSC>;
 };
 
+&fec {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_enet>;
+	phy-reset-gpios = <&gpio7 12 GPIO_ACTIVE_LOW>;
+	phy-mode = "rmii";
+	status = "okay";
+};
+
 &gpmi {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_gpmi_nand>;
@@ -118,6 +126,22 @@
 };
 
 &iomuxc {
+	pinctrl_enet: enetgrp {
+		fsl,pins = <
+			MX6QDL_PAD_ENET_CRS_DV__ENET_RX_EN	0x1b0b0
+			MX6QDL_PAD_GPIO_16__ENET_REF_CLK	0x1b0b1
+			MX6QDL_PAD_ENET_TX_EN__ENET_TX_EN	0x1b0b0
+			MX6QDL_PAD_ENET_RXD1__ENET_RX_DATA1	0x1b0b0
+			MX6QDL_PAD_ENET_RXD0__ENET_RX_DATA0	0x1b0b0
+			MX6QDL_PAD_ENET_TXD1__ENET_TX_DATA1	0x1b0b0
+			MX6QDL_PAD_ENET_TXD0__ENET_TX_DATA0	0x1b0b0
+			MX6QDL_PAD_ENET_MDC__ENET_MDC		0x1b0b0
+			MX6QDL_PAD_ENET_MDIO__ENET_MDIO		0x1b0b0
+			MX6QDL_PAD_ENET_REF_CLK__GPIO1_IO23	0x1b0b0
+			MX6QDL_PAD_GPIO_17__GPIO7_IO12		0x1b0b0
+		>;
+	};
+
 	pinctrl_flexcan1: flexcan1grp {
 		fsl,pins = <
 			MX6QDL_PAD_KEY_ROW2__FLEXCAN1_RX 0x1b020
Signature · 1 line
2.7.4
author
#4 lore raw

PATCH v3 3/3 icorem6: Use CONFIG_DM_ETH support

In reply to Jagan Teki

From: Jagan Teki <[email protected]>

Use CONFIG_DM_ETH and remove board_eth_init code
from board files.

Cc Joe Hershberger <[email protected]>
Cc Peng Fan <[email protected]>
Cc Stefano Babic <[email protected]>
Cc Michael Trimarchi <[email protected]>
---
 arch/arm/cpu/armv7/mx6/Kconfig       |  1 +
 board/engicam/icorem6/icorem6.c      | 71 ------------------------------------
 configs/imx6qdl_icore_mmc_defconfig  |  1 -
 configs/imx6qdl_icore_nand_defconfig |  1 -
 4 files changed, 1 insertion(+), 73 deletions(-)
arch/arm/cpu/armv7/mx6/Kconfig +1 -0
index 762a581..8456b0e 100644
@@ -100,6 +100,7 @@ config TARGET_MX6Q_ICORE
 	select MX6QDL
 	select OF_CONTROL
 	select DM
+	select DM_ETH
 	select DM_GPIO
 	select DM_MMC
 	select DM_THERMAL
board/engicam/icorem6/icorem6.c +0 -71

88 lines not on this page · raw message

configs/imx6qdl_icore_mmc_defconfig +0 -1
index b2d8cdf..a8199d5 100644
@@ -28,7 +28,6 @@ CONFIG_CMD_FS_GENERIC=y
 CONFIG_OF_LIBFDT=y
 CONFIG_FEC_MXC=y
 CONFIG_MXC_UART=y
-CONFIG_NETDEVICES=y
 CONFIG_IMX_THERMAL=y
 CONFIG_PINCTRL=y
 CONFIG_PINCTRL_IMX6=y
configs/imx6qdl_icore_nand_defconfig +0 -1
index ca4ff0f..45f4808 100644
@@ -24,7 +24,6 @@ CONFIG_OF_LIBFDT=y
 CONFIG_FEC_MXC=y
 CONFIG_MXC_UART=y
 CONFIG_NAND_MXS=y
-CONFIG_NETDEVICES=y
 CONFIG_IMX_THERMAL=y
 CONFIG_PINCTRL=y
 CONFIG_PINCTRL_IMX6=y
Signature · 1 line
2.7.4