[U-Boot] [PATCH v1 0/3] net: macb: Add option and clock support

The CONFIG_MACB option is added in KConfig to be used to select the Cadence MACB Ethernet driver, and the clock is supported.
Wenyou Yang (3): net: Kconfig: Add CONFIG_MACB option net: macb: Add the clock support net: macb: Remove redundant #ifdef CONFIG_DM_ETH
drivers/net/Kconfig | 10 ++++++++ drivers/net/macb.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 77 insertions(+), 4 deletions(-)

Add CONFIG_MACB option in KConfig to be used to select the Cadence MACB Ethernet driver.
Signed-off-by: Wenyou Yang wenyou.yang@atmel.com ---
drivers/net/Kconfig | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig index 302c005..fc0a10f 100644 --- a/drivers/net/Kconfig +++ b/drivers/net/Kconfig @@ -137,6 +137,16 @@ config MVPP2 This driver supports the network interface units in the Marvell ARMADA 375 SoC.
+config MACB + bool "Cadence MACB/GEM Ethernet Interface" + depends on DM_ETH + select PHYLIB + help + The Cadence MACB ethernet interface is found on many Atmel + AT91 and SAMA5 parts. This driver also supports the Cadence + GEM (Gigabit Ethernet MAC) found in some ARM SoC devices. + Say Y to include support for the MACB/GEM chip. + config PCH_GBE bool "Intel Platform Controller Hub EG20T GMAC driver" depends on DM_ETH && DM_PCI

On Wed, Oct 19, 2016 at 9:55 PM, Wenyou Yang wenyou.yang@atmel.com wrote:
Add CONFIG_MACB option in KConfig to be used to select the Cadence MACB Ethernet driver.
Signed-off-by: Wenyou Yang wenyou.yang@atmel.com
It seem you need to run tools/moveconfig.py on this thing, yes?
Thanks, -Joe

On Tue, Nov 1, 2016 at 3:41 PM, Joe Hershberger joe.hershberger@gmail.com wrote:
On Wed, Oct 19, 2016 at 9:55 PM, Wenyou Yang wenyou.yang@atmel.com wrote:
Add CONFIG_MACB option in KConfig to be used to select the Cadence MACB Ethernet driver.
Signed-off-by: Wenyou Yang wenyou.yang@atmel.com
It seem you need to run tools/moveconfig.py on this thing, yes?
It seems you posted a v2 but never addressed this comment. I think you need to use moveconfig for this patch.
Thanks, -Joe

Due to introducing the at91 clock driver, add the clock support.
Signed-off-by: Wenyou Yang wenyou.yang@atmel.com ---
drivers/net/macb.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 67 insertions(+), 1 deletion(-)
diff --git a/drivers/net/macb.c b/drivers/net/macb.c index 01527f7..0628ff5 100644 --- a/drivers/net/macb.c +++ b/drivers/net/macb.c @@ -4,6 +4,7 @@ * SPDX-License-Identifier: GPL-2.0+ */ #include <common.h> +#include <clk.h> #include <dm.h>
/* @@ -112,6 +113,7 @@ struct macb_device { struct mii_dev *bus;
#ifdef CONFIG_DM_ETH + unsigned long pclk_rate; phy_interface_t phy_interface; #endif }; @@ -751,10 +753,18 @@ static int _macb_write_hwaddr(struct macb_device *macb, unsigned char *enetaddr) return 0; }
+#ifndef CONFIG_DM_ETH static u32 macb_mdc_clk_div(int id, struct macb_device *macb) +#else +static u32 macb_mdc_clk_div(unsigned long pck_rate) +#endif { u32 config; +#ifndef CONFIG_DM_ETH unsigned long macb_hz = get_macb_pclk_rate(id); +#else + unsigned long macb_hz = pck_rate; +#endif
if (macb_hz < 20000000) config = MACB_BF(CLK, MACB_CLK_DIV8); @@ -768,10 +778,18 @@ static u32 macb_mdc_clk_div(int id, struct macb_device *macb) return config; }
+#ifndef CONFIG_DM_ETH static u32 gem_mdc_clk_div(int id, struct macb_device *macb) +#else +static u32 gem_mdc_clk_div(unsigned long pck_rate) +#endif { u32 config; +#ifndef CONFIG_DM_ETH unsigned long macb_hz = get_macb_pclk_rate(id); +#else + unsigned long macb_hz = pck_rate; +#endif
if (macb_hz < 20000000) config = GEM_BF(CLK, GEM_CLK_DIV8); @@ -807,9 +825,19 @@ static u32 macb_dbw(struct macb_device *macb) } }
+#ifndef CONFIG_DM_ETH static void _macb_eth_initialize(struct macb_device *macb) +#else +static void _macb_eth_initialize(struct udevice *dev) +#endif { +#ifdef CONFIG_DM_ETH + struct macb_device *macb = dev_get_priv(dev); +#endif + +#ifndef CONFIG_DM_ETH int id = 0; /* This is not used by functions we call */ +#endif u32 ncfgr;
/* TODO: we need check the rx/tx_ring_dma is dcache line aligned */ @@ -827,10 +855,18 @@ static void _macb_eth_initialize(struct macb_device *macb) * to the PHY */ if (macb_is_gem(macb)) { +#ifndef CONFIG_DM_ETH ncfgr = gem_mdc_clk_div(id, macb); +#else + ncfgr = gem_mdc_clk_div(macb->pclk_rate); +#endif ncfgr |= macb_dbw(macb); } else { +#ifndef CONFIG_DM_ETH ncfgr = macb_mdc_clk_div(id, macb); +#else + ncfgr = macb_mdc_clk_div(macb->pclk_rate); +#endif }
macb_writel(macb, NCFGR, ncfgr); @@ -991,6 +1027,30 @@ static const struct eth_ops macb_eth_ops = { .write_hwaddr = macb_write_hwaddr, };
+static int macb_enable_clk(struct udevice *dev) +{ + struct macb_device *macb = dev_get_priv(dev); + struct clk clk; + ulong clk_rate; + int ret; + + ret = clk_get_by_index(dev, 0, &clk); + if (ret) + return -EINVAL; + + ret = clk_enable(&clk); + if (ret) + return ret; + + clk_rate = clk_get_rate(&clk); + if (!clk_rate) + return -EINVAL; + + macb->pclk_rate = clk_rate; + + return 0; +} + static int macb_eth_probe(struct udevice *dev) { struct eth_pdata *pdata = dev_get_platdata(dev); @@ -998,6 +1058,7 @@ static int macb_eth_probe(struct udevice *dev)
#ifdef CONFIG_DM_ETH const char *phy_mode; + int ret;
phy_mode = fdt_getprop(gd->fdt_blob, dev->of_offset, "phy-mode", NULL); if (phy_mode) @@ -1010,7 +1071,12 @@ static int macb_eth_probe(struct udevice *dev)
macb->regs = (void *)pdata->iobase;
- _macb_eth_initialize(macb); + ret = macb_enable_clk(dev); + if (ret) + return ret; + + _macb_eth_initialize(dev); + #if defined(CONFIG_CMD_MII) || defined(CONFIG_PHYLIB) int retval; struct mii_dev *mdiodev = mdio_alloc();

On Wed, Oct 19, 2016 at 9:55 PM, Wenyou Yang wenyou.yang@atmel.com wrote:
Due to introducing the at91 clock driver, add the clock support.
Signed-off-by: Wenyou Yang wenyou.yang@atmel.com
drivers/net/macb.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 67 insertions(+), 1 deletion(-)
diff --git a/drivers/net/macb.c b/drivers/net/macb.c index 01527f7..0628ff5 100644 --- a/drivers/net/macb.c +++ b/drivers/net/macb.c @@ -4,6 +4,7 @@
- SPDX-License-Identifier: GPL-2.0+
*/ #include <common.h> +#include <clk.h> #include <dm.h>
/* @@ -112,6 +113,7 @@ struct macb_device { struct mii_dev *bus;
#ifdef CONFIG_DM_ETH
unsigned long pclk_rate; phy_interface_t phy_interface;
#endif }; @@ -751,10 +753,18 @@ static int _macb_write_hwaddr(struct macb_device *macb, unsigned char *enetaddr) return 0; }
+#ifndef CONFIG_DM_ETH
Please use positive logic throughout. If you have an #else, always use #ifdef.
Please fix throughout.
static u32 macb_mdc_clk_div(int id, struct macb_device *macb)
What's the benefit of splitting the signature of this function instead of just passing in the result of get_macb_pclk_rate() in the !DM case?
+#else +static u32 macb_mdc_clk_div(unsigned long pck_rate) +#endif { u32 config; +#ifndef CONFIG_DM_ETH unsigned long macb_hz = get_macb_pclk_rate(id); +#else
unsigned long macb_hz = pck_rate;
+#endif
if (macb_hz < 20000000) config = MACB_BF(CLK, MACB_CLK_DIV8);
@@ -768,10 +778,18 @@ static u32 macb_mdc_clk_div(int id, struct macb_device *macb) return config; }
+#ifndef CONFIG_DM_ETH static u32 gem_mdc_clk_div(int id, struct macb_device *macb) +#else +static u32 gem_mdc_clk_div(unsigned long pck_rate) +#endif { u32 config; +#ifndef CONFIG_DM_ETH unsigned long macb_hz = get_macb_pclk_rate(id); +#else
unsigned long macb_hz = pck_rate;
+#endif
if (macb_hz < 20000000) config = GEM_BF(CLK, GEM_CLK_DIV8);
@@ -807,9 +825,19 @@ static u32 macb_dbw(struct macb_device *macb) } }
+#ifndef CONFIG_DM_ETH static void _macb_eth_initialize(struct macb_device *macb) +#else +static void _macb_eth_initialize(struct udevice *dev)
It appears you only use the macb pointer in this function, so why pass in the dev only to have to look up the macb priv ptr? Just always pass the macb ptr.
+#endif { +#ifdef CONFIG_DM_ETH
struct macb_device *macb = dev_get_priv(dev);
+#endif
+#ifndef CONFIG_DM_ETH int id = 0; /* This is not used by functions we call */
You could just hard-code the 0 (with comment) once in the call to get_macb_pclk_rate() and avoid this ifdef.
+#endif u32 ncfgr;
/* TODO: we need check the rx/tx_ring_dma is dcache line aligned */
@@ -827,10 +855,18 @@ static void _macb_eth_initialize(struct macb_device *macb) * to the PHY */ if (macb_is_gem(macb)) { +#ifndef CONFIG_DM_ETH ncfgr = gem_mdc_clk_div(id, macb); +#else
ncfgr = gem_mdc_clk_div(macb->pclk_rate);
+#endif ncfgr |= macb_dbw(macb); } else { +#ifndef CONFIG_DM_ETH ncfgr = macb_mdc_clk_div(id, macb); +#else
ncfgr = macb_mdc_clk_div(macb->pclk_rate);
+#endif }
macb_writel(macb, NCFGR, ncfgr);
@@ -991,6 +1027,30 @@ static const struct eth_ops macb_eth_ops = { .write_hwaddr = macb_write_hwaddr, };
+static int macb_enable_clk(struct udevice *dev) +{
struct macb_device *macb = dev_get_priv(dev);
struct clk clk;
ulong clk_rate;
int ret;
ret = clk_get_by_index(dev, 0, &clk);
if (ret)
return -EINVAL;
ret = clk_enable(&clk);
if (ret)
return ret;
clk_rate = clk_get_rate(&clk);
if (!clk_rate)
return -EINVAL;
macb->pclk_rate = clk_rate;
return 0;
+}
static int macb_eth_probe(struct udevice *dev) { struct eth_pdata *pdata = dev_get_platdata(dev); @@ -998,6 +1058,7 @@ static int macb_eth_probe(struct udevice *dev)
#ifdef CONFIG_DM_ETH const char *phy_mode;
int ret; phy_mode = fdt_getprop(gd->fdt_blob, dev->of_offset, "phy-mode", NULL); if (phy_mode)
@@ -1010,7 +1071,12 @@ static int macb_eth_probe(struct udevice *dev)
macb->regs = (void *)pdata->iobase;
_macb_eth_initialize(macb);
ret = macb_enable_clk(dev);
if (ret)
return ret;
_macb_eth_initialize(dev);
#if defined(CONFIG_CMD_MII) || defined(CONFIG_PHYLIB) int retval; struct mii_dev *mdiodev = mdio_alloc(); -- 2.7.4
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot

Remove the redundant #ifdef CONFIG_DM_ETH/#endif.
Signed-off-by: Wenyou Yang wenyou.yang@atmel.com ---
drivers/net/macb.c | 3 --- 1 file changed, 3 deletions(-)
diff --git a/drivers/net/macb.c b/drivers/net/macb.c index 0628ff5..72cf9c3 100644 --- a/drivers/net/macb.c +++ b/drivers/net/macb.c @@ -1055,8 +1055,6 @@ 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; int ret;
@@ -1067,7 +1065,6 @@ static int macb_eth_probe(struct udevice *dev) debug("%s: Invalid PHY interface '%s'\n", __func__, phy_mode); return -EINVAL; } -#endif
macb->regs = (void *)pdata->iobase;

On Wed, Oct 19, 2016 at 9:55 PM, Wenyou Yang wenyou.yang@atmel.com wrote:
Remove the redundant #ifdef CONFIG_DM_ETH/#endif.
Signed-off-by: Wenyou Yang wenyou.yang@atmel.com
Acked-by: Joe Hershberger joe.hershberger@ni.com
participants (2)
-
Joe Hershberger
-
Wenyou Yang