* [U-Boot] [RFC PATCH] net: macb: Fix build error for CONFIG_DM_ETH enabled
@ 2016-05-17 5:11 Wenyou Yang
2016-05-17 21:58 ` Simon Glass
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Wenyou Yang @ 2016-05-17 5:11 UTC (permalink / raw)
To: u-boot
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);
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [RFC PATCH] net: macb: Fix build error for CONFIG_DM_ETH enabled
2016-05-17 5:11 [U-Boot] [RFC PATCH] net: macb: Fix build error for CONFIG_DM_ETH enabled Wenyou Yang
@ 2016-05-17 21:58 ` Simon Glass
2016-05-24 15:22 ` Joe Hershberger
2016-08-04 17:06 ` Joe Hershberger
2016-08-15 20:33 ` [U-Boot] " Joe Hershberger
2 siblings, 1 reply; 6+ messages in thread
From: Simon Glass @ 2016-05-17 21:58 UTC (permalink / raw)
To: u-boot
+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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [RFC PATCH] net: macb: Fix build error for CONFIG_DM_ETH enabled
2016-05-17 21:58 ` Simon Glass
@ 2016-05-24 15:22 ` Joe Hershberger
0 siblings, 0 replies; 6+ messages in thread
From: Joe Hershberger @ 2016-05-24 15:22 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [RFC PATCH] net: macb: Fix build error for CONFIG_DM_ETH enabled
2016-05-17 5:11 [U-Boot] [RFC PATCH] net: macb: Fix build error for CONFIG_DM_ETH enabled Wenyou Yang
2016-05-17 21:58 ` Simon Glass
@ 2016-08-04 17:06 ` Joe Hershberger
2016-08-08 13:44 ` Wenyou.Yang at microchip.com
2016-08-15 20:33 ` [U-Boot] " Joe Hershberger
2 siblings, 1 reply; 6+ messages in thread
From: Joe Hershberger @ 2016-08-04 17:06 UTC (permalink / raw)
To: u-boot
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>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [RFC PATCH] net: macb: Fix build error for CONFIG_DM_ETH enabled
2016-08-04 17:06 ` Joe Hershberger
@ 2016-08-08 13:44 ` Wenyou.Yang at microchip.com
0 siblings, 0 replies; 6+ messages in thread
From: Wenyou.Yang at microchip.com @ 2016-08-08 13:44 UTC (permalink / raw)
To: u-boot
> -----Original Message-----
> From: Joe Hershberger [mailto:joe.hershberger at 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] net: macb: Fix build error for CONFIG_DM_ETH enabled
2016-05-17 5:11 [U-Boot] [RFC PATCH] net: macb: Fix build error for CONFIG_DM_ETH enabled Wenyou Yang
2016-05-17 21:58 ` Simon Glass
2016-08-04 17:06 ` Joe Hershberger
@ 2016-08-15 20:33 ` Joe Hershberger
2 siblings, 0 replies; 6+ messages in thread
From: Joe Hershberger @ 2016-08-15 20:33 UTC (permalink / raw)
To: u-boot
Hi Wenyou,
https://patchwork.ozlabs.org/patch/622869/ was applied to u-boot-net.git.
Thanks!
-Joe
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-08-15 20:33 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-17 5:11 [U-Boot] [RFC PATCH] net: macb: Fix build error for CONFIG_DM_ETH enabled Wenyou Yang
2016-05-17 21:58 ` Simon Glass
2016-05-24 15:22 ` Joe Hershberger
2016-08-04 17:06 ` Joe Hershberger
2016-08-08 13:44 ` Wenyou.Yang at microchip.com
2016-08-15 20:33 ` [U-Boot] " Joe Hershberger
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.