[U-Boot] [RFC PATCH] net: macb: Fix build error for CONFIG_DM_ETH enabled

Use the right phy_connect() prototype for CONFIGF_DM_ETH. Support to get the phy interface from dt and set GMAC_UR.
Signed-off-by: Wenyou Yang wenyou.yang@atmel.com --- This patch is based on the patch set, [PATCH 00/18] at91: Convert Ethernet and LCD to driver model http://lists.denx.de/pipermail/u-boot/2016-May/253559.html
Hi Simon,
With this patch, the ethernet works on SAMA5D2 Xplained board. But it includes a lot of #ifdef, I think it is not pretty.
What is your opinions?
drivers/net/macb.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 69 insertions(+), 8 deletions(-)
diff --git a/drivers/net/macb.c b/drivers/net/macb.c index 63fb466..ddb9e23 100644 --- a/drivers/net/macb.c +++ b/drivers/net/macb.c @@ -43,6 +43,8 @@
#include "macb.h"
+DECLARE_GLOBAL_DATA_PTR; + #define MACB_RX_BUFFER_SIZE 4096 #define MACB_RX_RING_SIZE (MACB_RX_BUFFER_SIZE / 128) #define MACB_TX_RING_SIZE 16 @@ -108,6 +110,10 @@ struct macb_device { #endif unsigned short phy_addr; struct mii_dev *bus; + +#ifdef CONFIG_DM_ETH + phy_interface_t phy_interface; +#endif }; #ifndef CONFIG_DM_ETH #define to_macb(_nd) container_of(_nd, struct macb_device, netdev) @@ -434,7 +440,7 @@ static void macb_phy_reset(struct macb_device *macb, const char *name) }
#ifdef CONFIG_MACB_SEARCH_PHY -static int macb_phy_find(struct macb_device *macb) +static int macb_phy_find(struct macb_device *macb, const char *name) { int i; u16 phy_id; @@ -444,21 +450,27 @@ static int macb_phy_find(struct macb_device *macb) macb->phy_addr = i; phy_id = macb_mdio_read(macb, MII_PHYSID1); if (phy_id != 0xffff) { - printf("%s: PHY present at %d\n", macb->netdev.name, i); + printf("%s: PHY present at %d\n", name, i); return 1; } }
/* PHY isn't up to snuff */ - printf("%s: PHY not found\n", macb->netdev.name); + printf("%s: PHY not found\n", name);
return 0; } #endif /* CONFIG_MACB_SEARCH_PHY */
- +#ifdef CONFIG_DM_ETH +static int macb_phy_init(struct udevice *dev, const char *name) +#else static int macb_phy_init(struct macb_device *macb, const char *name) +#endif { +#ifdef CONFIG_DM_ETH + struct macb_device *macb = dev_get_priv(dev); +#endif #ifdef CONFIG_PHYLIB struct phy_device *phydev; #endif @@ -470,7 +482,7 @@ static int macb_phy_init(struct macb_device *macb, const char *name) arch_get_mdio_control(name); #ifdef CONFIG_MACB_SEARCH_PHY /* Auto-detect phy_addr */ - if (!macb_phy_find(macb)) + if (!macb_phy_find(macb, name)) return 0; #endif /* CONFIG_MACB_SEARCH_PHY */
@@ -482,9 +494,14 @@ static int macb_phy_init(struct macb_device *macb, const char *name) }
#ifdef CONFIG_PHYLIB +#ifdef CONFIG_DM_ETH + phydev = phy_connect(macb->bus, macb->phy_addr, dev, + macb->phy_interface); +#else /* need to consider other phy interface mode */ phydev = phy_connect(macb->bus, macb->phy_addr, &macb->netdev, PHY_INTERFACE_MODE_RGMII); +#endif if (!phydev) { printf("phy_connect failed\n"); return -ENODEV; @@ -585,8 +602,15 @@ static int gmac_init_multi_queues(struct macb_device *macb) return 0; }
+#ifdef CONFIG_DM_ETH +static int _macb_init(struct udevice *dev, const char *name) +#else static int _macb_init(struct macb_device *macb, const char *name) +#endif { +#ifdef CONFIG_DM_ETH + struct macb_device *macb = dev_get_priv(dev); +#endif unsigned long paddr; int i;
@@ -634,13 +658,35 @@ static int _macb_init(struct macb_device *macb, const char *name) * When the GMAC IP without GE feature, this bit is used * to select interface between RMII and MII. */ +#ifdef CONFIG_DM_ETH + if (macb->phy_interface == PHY_INTERFACE_MODE_RMII) + gem_writel(macb, UR, GEM_BIT(RGMII)); + else + gem_writel(macb, UR, 0); +#else #if defined(CONFIG_RGMII) || defined(CONFIG_RMII) gem_writel(macb, UR, GEM_BIT(RGMII)); #else gem_writel(macb, UR, 0); #endif +#endif } else { /* choose RMII or MII mode. This depends on the board */ +#ifdef CONFIG_DM_ETH +#ifdef CONFIG_AT91FAMILY + if (macb->phy_interface == PHY_INTERFACE_MODE_RMII) { + macb_writel(macb, USRIO, + MACB_BIT(RMII) | MACB_BIT(CLKEN)); + } else { + macb_writel(macb, USRIO, MACB_BIT(CLKEN)); + } +#else + if (macb->phy_interface == PHY_INTERFACE_MODE_RMII) + macb_writel(macb, USRIO, 0); + else + macb_writel(macb, USRIO, MACB_BIT(MII)); +#endif +#else #ifdef CONFIG_RMII #ifdef CONFIG_AT91FAMILY macb_writel(macb, USRIO, MACB_BIT(RMII) | MACB_BIT(CLKEN)); @@ -654,9 +700,14 @@ static int _macb_init(struct macb_device *macb, const char *name) macb_writel(macb, USRIO, MACB_BIT(MII)); #endif #endif /* CONFIG_RMII */ +#endif }
+#ifdef CONFIG_DM_ETH + if (!macb_phy_init(dev, name)) +#else if (!macb_phy_init(macb, name)) +#endif return -1;
/* Enable TX and RX */ @@ -873,9 +924,7 @@ int macb_eth_initialize(int id, void *regs, unsigned int phy_addr)
static int macb_start(struct udevice *dev) { - struct macb_device *macb = dev_get_priv(dev); - - return _macb_init(macb, dev->name); + return _macb_init(dev, dev->name); }
static int macb_send(struct udevice *dev, void *packet, int length) @@ -933,6 +982,18 @@ static int macb_eth_probe(struct udevice *dev) struct eth_pdata *pdata = dev_get_platdata(dev); struct macb_device *macb = dev_get_priv(dev);
+#ifdef CONFIG_DM_ETH + const char *phy_mode; + + phy_mode = fdt_getprop(gd->fdt_blob, dev->of_offset, "phy-mode", NULL); + if (phy_mode) + macb->phy_interface = phy_get_interface_by_name(phy_mode); + if (macb->phy_interface == -1) { + debug("%s: Invalid PHY interface '%s'\n", __func__, phy_mode); + return -EINVAL; + } +#endif + macb->regs = (void *)pdata->iobase;
_macb_eth_initialize(macb);

+Joe
On 16 May 2016 at 23:11, Wenyou Yang wenyou.yang@atmel.com wrote:
Use the right phy_connect() prototype for CONFIGF_DM_ETH. Support to get the phy interface from dt and set GMAC_UR.
Signed-off-by: Wenyou Yang wenyou.yang@atmel.com
This patch is based on the patch set, [PATCH 00/18] at91: Convert Ethernet and LCD to driver model http://lists.denx.de/pipermail/u-boot/2016-May/253559.html
Hi Simon,
With this patch, the ethernet works on SAMA5D2 Xplained board. But it includes a lot of #ifdef, I think it is not pretty.
What is your opinions?
I believe there is a new PHY interface that might help. I added Joe in case he can help.
We should move PHYs to driver model but apparently that is tricky to do at present.
drivers/net/macb.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 69 insertions(+), 8 deletions(-)
diff --git a/drivers/net/macb.c b/drivers/net/macb.c index 63fb466..ddb9e23 100644 --- a/drivers/net/macb.c +++ b/drivers/net/macb.c @@ -43,6 +43,8 @@
#include "macb.h"
+DECLARE_GLOBAL_DATA_PTR;
#define MACB_RX_BUFFER_SIZE 4096 #define MACB_RX_RING_SIZE (MACB_RX_BUFFER_SIZE / 128) #define MACB_TX_RING_SIZE 16 @@ -108,6 +110,10 @@ struct macb_device { #endif unsigned short phy_addr; struct mii_dev *bus;
+#ifdef CONFIG_DM_ETH
phy_interface_t phy_interface;
+#endif }; #ifndef CONFIG_DM_ETH #define to_macb(_nd) container_of(_nd, struct macb_device, netdev) @@ -434,7 +440,7 @@ static void macb_phy_reset(struct macb_device *macb, const char *name) }
#ifdef CONFIG_MACB_SEARCH_PHY -static int macb_phy_find(struct macb_device *macb) +static int macb_phy_find(struct macb_device *macb, const char *name) { int i; u16 phy_id; @@ -444,21 +450,27 @@ static int macb_phy_find(struct macb_device *macb) macb->phy_addr = i; phy_id = macb_mdio_read(macb, MII_PHYSID1); if (phy_id != 0xffff) {
printf("%s: PHY present at %d\n", macb->netdev.name, i);
printf("%s: PHY present at %d\n", name, i); return 1; } } /* PHY isn't up to snuff */
printf("%s: PHY not found\n", macb->netdev.name);
printf("%s: PHY not found\n", name); return 0;
} #endif /* CONFIG_MACB_SEARCH_PHY */
+#ifdef CONFIG_DM_ETH +static int macb_phy_init(struct udevice *dev, const char *name) +#else static int macb_phy_init(struct macb_device *macb, const char *name) +#endif { +#ifdef CONFIG_DM_ETH
struct macb_device *macb = dev_get_priv(dev);
+#endif #ifdef CONFIG_PHYLIB struct phy_device *phydev; #endif @@ -470,7 +482,7 @@ static int macb_phy_init(struct macb_device *macb, const char *name) arch_get_mdio_control(name); #ifdef CONFIG_MACB_SEARCH_PHY /* Auto-detect phy_addr */
if (!macb_phy_find(macb))
if (!macb_phy_find(macb, name)) return 0;
#endif /* CONFIG_MACB_SEARCH_PHY */
@@ -482,9 +494,14 @@ static int macb_phy_init(struct macb_device *macb, const char *name) }
#ifdef CONFIG_PHYLIB +#ifdef CONFIG_DM_ETH
phydev = phy_connect(macb->bus, macb->phy_addr, dev,
macb->phy_interface);
+#else /* need to consider other phy interface mode */ phydev = phy_connect(macb->bus, macb->phy_addr, &macb->netdev, PHY_INTERFACE_MODE_RGMII); +#endif if (!phydev) { printf("phy_connect failed\n"); return -ENODEV; @@ -585,8 +602,15 @@ static int gmac_init_multi_queues(struct macb_device *macb) return 0; }
+#ifdef CONFIG_DM_ETH +static int _macb_init(struct udevice *dev, const char *name) +#else static int _macb_init(struct macb_device *macb, const char *name) +#endif { +#ifdef CONFIG_DM_ETH
struct macb_device *macb = dev_get_priv(dev);
+#endif unsigned long paddr; int i;
@@ -634,13 +658,35 @@ static int _macb_init(struct macb_device *macb, const char *name) * When the GMAC IP without GE feature, this bit is used * to select interface between RMII and MII. */ +#ifdef CONFIG_DM_ETH
if (macb->phy_interface == PHY_INTERFACE_MODE_RMII)
gem_writel(macb, UR, GEM_BIT(RGMII));
else
gem_writel(macb, UR, 0);
+#else #if defined(CONFIG_RGMII) || defined(CONFIG_RMII) gem_writel(macb, UR, GEM_BIT(RGMII)); #else gem_writel(macb, UR, 0); #endif +#endif } else { /* choose RMII or MII mode. This depends on the board */ +#ifdef CONFIG_DM_ETH +#ifdef CONFIG_AT91FAMILY
if (macb->phy_interface == PHY_INTERFACE_MODE_RMII) {
macb_writel(macb, USRIO,
MACB_BIT(RMII) | MACB_BIT(CLKEN));
} else {
macb_writel(macb, USRIO, MACB_BIT(CLKEN));
}
+#else
if (macb->phy_interface == PHY_INTERFACE_MODE_RMII)
macb_writel(macb, USRIO, 0);
else
macb_writel(macb, USRIO, MACB_BIT(MII));
+#endif +#else #ifdef CONFIG_RMII #ifdef CONFIG_AT91FAMILY macb_writel(macb, USRIO, MACB_BIT(RMII) | MACB_BIT(CLKEN)); @@ -654,9 +700,14 @@ static int _macb_init(struct macb_device *macb, const char *name) macb_writel(macb, USRIO, MACB_BIT(MII)); #endif #endif /* CONFIG_RMII */ +#endif }
+#ifdef CONFIG_DM_ETH
if (!macb_phy_init(dev, name))
+#else if (!macb_phy_init(macb, name)) +#endif return -1;
/* Enable TX and RX */
@@ -873,9 +924,7 @@ int macb_eth_initialize(int id, void *regs, unsigned int phy_addr)
static int macb_start(struct udevice *dev) {
struct macb_device *macb = dev_get_priv(dev);
return _macb_init(macb, dev->name);
return _macb_init(dev, dev->name);
}
static int macb_send(struct udevice *dev, void *packet, int length) @@ -933,6 +982,18 @@ static int macb_eth_probe(struct udevice *dev) struct eth_pdata *pdata = dev_get_platdata(dev); struct macb_device *macb = dev_get_priv(dev);
+#ifdef CONFIG_DM_ETH
const char *phy_mode;
phy_mode = fdt_getprop(gd->fdt_blob, dev->of_offset, "phy-mode", NULL);
if (phy_mode)
macb->phy_interface = phy_get_interface_by_name(phy_mode);
if (macb->phy_interface == -1) {
debug("%s: Invalid PHY interface '%s'\n", __func__, phy_mode);
return -EINVAL;
}
+#endif
macb->regs = (void *)pdata->iobase; _macb_eth_initialize(macb);
-- 2.7.4
Regards, Simon

On Tue, May 17, 2016 at 4:58 PM, Simon Glass sjg@chromium.org wrote:
+Joe
On 16 May 2016 at 23:11, Wenyou Yang wenyou.yang@atmel.com wrote:
Use the right phy_connect() prototype for CONFIGF_DM_ETH. Support to get the phy interface from dt and set GMAC_UR.
Signed-off-by: Wenyou Yang wenyou.yang@atmel.com
This patch is based on the patch set, [PATCH 00/18] at91: Convert Ethernet and LCD to driver model http://lists.denx.de/pipermail/u-boot/2016-May/253559.html
Hi Simon,
With this patch, the ethernet works on SAMA5D2 Xplained board. But it includes a lot of #ifdef, I think it is not pretty.
What is your opinions?
I believe there is a new PHY interface that might help. I added Joe in case he can help.
We should move PHYs to driver model but apparently that is tricky to do at present.
This is pretty messy, indeed. The issue is that there is no common way to define phy connections. Each MAC driver has its own way. I think if the implementation is this messy, maybe we are better off waiting.
-Joe

On Tue, May 17, 2016 at 12:11 AM, Wenyou Yang wenyou.yang@atmel.com wrote:
Use the right phy_connect() prototype for CONFIGF_DM_ETH. Support to get the phy interface from dt and set GMAC_UR.
Signed-off-by: Wenyou Yang wenyou.yang@atmel.com
This patch is based on the patch set, [PATCH 00/18] at91: Convert Ethernet and LCD to driver model http://lists.denx.de/pipermail/u-boot/2016-May/253559.html
Hi Simon,
With this patch, the ethernet works on SAMA5D2 Xplained board. But it includes a lot of #ifdef, I think it is not pretty.
What is your opinions?
I think this is OK to get that board working for now. We will address all such mess in the future.
Acked-by: Joe Hershberger joe.hershberger@ni.com

-----Original Message----- From: Joe Hershberger [mailto:joe.hershberger@gmail.com] Sent: 2016年8月5日 1:06 To: Wenyou Yang wenyou.yang@atmel.com Cc: U-Boot Mailing List u-boot@lists.denx.de Subject: Re: [U-Boot] [RFC PATCH] net: macb: Fix build error for CONFIG_DM_ETH enabled
On Tue, May 17, 2016 at 12:11 AM, Wenyou Yang wenyou.yang@atmel.com wrote:
Use the right phy_connect() prototype for CONFIGF_DM_ETH. Support to get the phy interface from dt and set GMAC_UR.
Signed-off-by: Wenyou Yang wenyou.yang@atmel.com
This patch is based on the patch set, [PATCH 00/18] at91: Convert Ethernet and LCD to driver model http://lists.denx.de/pipermail/u-boot/2016-May/253559.html
Hi Simon,
With this patch, the ethernet works on SAMA5D2 Xplained board. But it includes a lot of #ifdef, I think it is not pretty.
What is your opinions?
I think this is OK to get that board working for now. We will address all such mess in the future
I agree with you.
Acked-by: Joe Hershberger joe.hershberger@ni.com
Thank for your Acked-by
Best Regards, Wenyou Yang

participants (5)
-
Joe Hershberger
-
Joe Hershberger
-
Simon Glass
-
Wenyou Yang
-
Wenyou.Yang@microchip.com