| Message ID | 1463461895-15149-1-git-send-email-wenyou.yang@atmel.com |
|---|---|
| State | Accepted |
| Commit | a212b66 |
| Delegated to: | Joe Hershberger |
| Headers | show |
+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
Hi Wenyou, https://patchwork.ozlabs.org/patch/622869/ was applied to u-boot-net.git. Thanks! -Joe
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);
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(-)