[U-Boot] [PATCH v3] am3517_evm: activate Ethernet PHY

From: Yegor Yefremov yegorslists@googlemail.com
Pin 30 is connected to PHY's RESET# signal, so it must be put to high. Otherwise PHY won't be found via MDIO interface.
Signed-off-by: Yegor Yefremov yegorslists@googlemail.com --- Changes: v3: use "__maybe_unused", instead of #if defined statement (Stefan Roese) v2: put ctr and reset under #if defined statement, to avoid compiler warnings, when EMAC is not selected
board/logicpd/am3517evm/am3517evm.c | 34 ++++++++++++++++++++++++++++++++++ board/logicpd/am3517evm/am3517evm.h | 2 +- 2 files changed, 35 insertions(+), 1 deletions(-)
diff --git a/board/logicpd/am3517evm/am3517evm.c b/board/logicpd/am3517evm/am3517evm.c index 1569905..3b1dfd1 100644 --- a/board/logicpd/am3517evm/am3517evm.c +++ b/board/logicpd/am3517evm/am3517evm.c @@ -22,6 +22,7 @@ #include <asm/arch/musb.h> #include <asm/mach-types.h> #include <asm/errno.h> +#include <asm/gpio.h> #include <linux/usb/ch9.h> #include <linux/usb/gadget.h> #include <linux/usb/musb.h> @@ -31,6 +32,9 @@
DECLARE_GLOBAL_DATA_PTR;
+#define AM3517_IP_SW_RESET 0x48002598 +#define CPGMACSS_SW_RST (1 << 1) + /* * Routine: board_init * Description: Early hardware init. @@ -98,6 +102,9 @@ static void am3517_evm_musb_init(void) */ int misc_init_r(void) { + __maybe_unused volatile unsigned int ctr; + __maybe_unused u32 reset; + #ifdef CONFIG_SYS_I2C_OMAP34XX i2c_init(CONFIG_SYS_OMAP24_I2C_SPEED, CONFIG_SYS_OMAP24_I2C_SLAVE); #endif @@ -106,6 +113,33 @@ int misc_init_r(void)
am3517_evm_musb_init();
+#if defined(CONFIG_DRIVER_TI_EMAC) + /* activate PHY reset */ + gpio_direction_output(30, 0); + gpio_set_value(30, 0); + + ctr = 0; + do { + udelay(1000); + ctr++; + } while (ctr < 300); + + /* deactivate PHY reset */ + gpio_set_value(30, 1); + + /* allow the PHY to stabilize and settle down */ + ctr = 0; + do { + udelay(1000); + ctr++; + } while (ctr < 300); + + /* ensure that the module is out of reset */ + reset = readl(AM3517_IP_SW_RESET); + reset &= (~CPGMACSS_SW_RST); + writel(reset,AM3517_IP_SW_RESET); +#endif + return 0; }
diff --git a/board/logicpd/am3517evm/am3517evm.h b/board/logicpd/am3517evm/am3517evm.h index 704af84..d407d66 100644 --- a/board/logicpd/am3517evm/am3517evm.h +++ b/board/logicpd/am3517evm/am3517evm.h @@ -315,7 +315,7 @@ const omap3_sysinfo sysinfo = { MUX_VAL(CP(SYS_CLKREQ), (IEN | PTD | DIS | M0)) \ MUX_VAL(CP(SYS_NIRQ), (IEN | PTU | EN | M0)) \ /*SYS_nRESWARM */\ - MUX_VAL(CP(SYS_NRESWARM), (IDIS | PTU | DIS | M4)) \ + MUX_VAL(CP(SYS_NRESWARM), (IDIS | PTU | EN | M4)) \ /* - GPIO30 */\ MUX_VAL(CP(SYS_BOOT0), (IEN | PTD | DIS | M4)) /*GPIO_2*/\ /* - PEN_IRQ */\

On 06.12.2013 11:17, yegorslists@googlemail.com wrote:
From: Yegor Yefremov yegorslists@googlemail.com
Pin 30 is connected to PHY's RESET# signal, so it must be put to high. Otherwise PHY won't be found via MDIO interface.
Signed-off-by: Yegor Yefremov yegorslists@googlemail.com
Looks good. One questions below though:
Changes: v3: use "__maybe_unused", instead of #if defined statement (Stefan Roese) v2: put ctr and reset under #if defined statement, to avoid compiler warnings, when EMAC is not selected
board/logicpd/am3517evm/am3517evm.c | 34 ++++++++++++++++++++++++++++++++++ board/logicpd/am3517evm/am3517evm.h | 2 +- 2 files changed, 35 insertions(+), 1 deletions(-)
diff --git a/board/logicpd/am3517evm/am3517evm.c b/board/logicpd/am3517evm/am3517evm.c index 1569905..3b1dfd1 100644 --- a/board/logicpd/am3517evm/am3517evm.c +++ b/board/logicpd/am3517evm/am3517evm.c @@ -22,6 +22,7 @@ #include <asm/arch/musb.h> #include <asm/mach-types.h> #include <asm/errno.h> +#include <asm/gpio.h> #include <linux/usb/ch9.h> #include <linux/usb/gadget.h> #include <linux/usb/musb.h> @@ -31,6 +32,9 @@
DECLARE_GLOBAL_DATA_PTR;
+#define AM3517_IP_SW_RESET 0x48002598 +#define CPGMACSS_SW_RST (1 << 1)
/*
- Routine: board_init
- Description: Early hardware init.
@@ -98,6 +102,9 @@ static void am3517_evm_musb_init(void) */ int misc_init_r(void) {
- __maybe_unused volatile unsigned int ctr;
- __maybe_unused u32 reset;
#ifdef CONFIG_SYS_I2C_OMAP34XX i2c_init(CONFIG_SYS_OMAP24_I2C_SPEED, CONFIG_SYS_OMAP24_I2C_SLAVE); #endif @@ -106,6 +113,33 @@ int misc_init_r(void)
am3517_evm_musb_init();
+#if defined(CONFIG_DRIVER_TI_EMAC)
- /* activate PHY reset */
- gpio_direction_output(30, 0);
- gpio_set_value(30, 0);
- ctr = 0;
- do {
udelay(1000);
ctr++;
- } while (ctr < 300);
- /* deactivate PHY reset */
- gpio_set_value(30, 1);
- /* allow the PHY to stabilize and settle down */
- ctr = 0;
- do {
udelay(1000);
ctr++;
- } while (ctr < 300);
- /* ensure that the module is out of reset */
- reset = readl(AM3517_IP_SW_RESET);
- reset &= (~CPGMACSS_SW_RST);
- writel(reset,AM3517_IP_SW_RESET);
+#endif
Why do you need to put this "#if defined(CONFIG_DRIVER_TI_EMAC)" here at all? Isn't CONFIG_DRIVER_TI_EMAC enabled for this board always?
Thanks, Stefan

On Fri, Dec 6, 2013 at 11:37 AM, Stefan Roese stefan.roese@gmail.com wrote:
On 06.12.2013 11:17, yegorslists@googlemail.com wrote:
From: Yegor Yefremov yegorslists@googlemail.com
Pin 30 is connected to PHY's RESET# signal, so it must be put to high. Otherwise PHY won't be found via MDIO interface.
Signed-off-by: Yegor Yefremov yegorslists@googlemail.com
Looks good. One questions below though:
Changes: v3: use "__maybe_unused", instead of #if defined statement (Stefan Roese) v2: put ctr and reset under #if defined statement, to avoid compiler warnings, when EMAC is not selected
board/logicpd/am3517evm/am3517evm.c | 34 ++++++++++++++++++++++++++++++++++ board/logicpd/am3517evm/am3517evm.h | 2 +- 2 files changed, 35 insertions(+), 1 deletions(-)
diff --git a/board/logicpd/am3517evm/am3517evm.c b/board/logicpd/am3517evm/am3517evm.c index 1569905..3b1dfd1 100644 --- a/board/logicpd/am3517evm/am3517evm.c +++ b/board/logicpd/am3517evm/am3517evm.c @@ -22,6 +22,7 @@ #include <asm/arch/musb.h> #include <asm/mach-types.h> #include <asm/errno.h> +#include <asm/gpio.h> #include <linux/usb/ch9.h> #include <linux/usb/gadget.h> #include <linux/usb/musb.h> @@ -31,6 +32,9 @@
DECLARE_GLOBAL_DATA_PTR;
+#define AM3517_IP_SW_RESET 0x48002598 +#define CPGMACSS_SW_RST (1 << 1)
/*
- Routine: board_init
- Description: Early hardware init.
@@ -98,6 +102,9 @@ static void am3517_evm_musb_init(void) */ int misc_init_r(void) {
__maybe_unused volatile unsigned int ctr;
__maybe_unused u32 reset;
#ifdef CONFIG_SYS_I2C_OMAP34XX i2c_init(CONFIG_SYS_OMAP24_I2C_SPEED, CONFIG_SYS_OMAP24_I2C_SLAVE); #endif @@ -106,6 +113,33 @@ int misc_init_r(void)
am3517_evm_musb_init();
+#if defined(CONFIG_DRIVER_TI_EMAC)
/* activate PHY reset */
gpio_direction_output(30, 0);
gpio_set_value(30, 0);
ctr = 0;
do {
udelay(1000);
ctr++;
} while (ctr < 300);
/* deactivate PHY reset */
gpio_set_value(30, 1);
/* allow the PHY to stabilize and settle down */
ctr = 0;
do {
udelay(1000);
ctr++;
} while (ctr < 300);
/* ensure that the module is out of reset */
reset = readl(AM3517_IP_SW_RESET);
reset &= (~CPGMACSS_SW_RST);
writel(reset,AM3517_IP_SW_RESET);
+#endif
Why do you need to put this "#if defined(CONFIG_DRIVER_TI_EMAC)" here at all? Isn't CONFIG_DRIVER_TI_EMAC enabled for this board always?
It is enabled in arago repository: http://arago-project.org/git/projects/?p=u-boot-omap3.git;a=blob;f=include/c..., but not in upstream u-boot. I was testing the stuff with my own board, so my am3517_evm.h is a little bit messy for now. That's why I wanted to have this change only in board/logicpd/am3517evm/am3517evm.c. It does no harm so far. I could patch am3517_evm.h later.
Yegor

On Fri, Dec 06, 2013 at 11:47:38AM +0100, Yegor Yefremov wrote:
On Fri, Dec 6, 2013 at 11:37 AM, Stefan Roese stefan.roese@gmail.com wrote:
On 06.12.2013 11:17, yegorslists@googlemail.com wrote:
From: Yegor Yefremov yegorslists@googlemail.com
Pin 30 is connected to PHY's RESET# signal, so it must be put to high. Otherwise PHY won't be found via MDIO interface.
Signed-off-by: Yegor Yefremov yegorslists@googlemail.com
Looks good. One questions below though:
Changes: v3: use "__maybe_unused", instead of #if defined statement (Stefan Roese) v2: put ctr and reset under #if defined statement, to avoid compiler warnings, when EMAC is not selected
board/logicpd/am3517evm/am3517evm.c | 34 ++++++++++++++++++++++++++++++++++ board/logicpd/am3517evm/am3517evm.h | 2 +- 2 files changed, 35 insertions(+), 1 deletions(-)
diff --git a/board/logicpd/am3517evm/am3517evm.c b/board/logicpd/am3517evm/am3517evm.c index 1569905..3b1dfd1 100644 --- a/board/logicpd/am3517evm/am3517evm.c +++ b/board/logicpd/am3517evm/am3517evm.c @@ -22,6 +22,7 @@ #include <asm/arch/musb.h> #include <asm/mach-types.h> #include <asm/errno.h> +#include <asm/gpio.h> #include <linux/usb/ch9.h> #include <linux/usb/gadget.h> #include <linux/usb/musb.h> @@ -31,6 +32,9 @@
DECLARE_GLOBAL_DATA_PTR;
+#define AM3517_IP_SW_RESET 0x48002598 +#define CPGMACSS_SW_RST (1 << 1)
/*
- Routine: board_init
- Description: Early hardware init.
@@ -98,6 +102,9 @@ static void am3517_evm_musb_init(void) */ int misc_init_r(void) {
__maybe_unused volatile unsigned int ctr;
__maybe_unused u32 reset;
#ifdef CONFIG_SYS_I2C_OMAP34XX i2c_init(CONFIG_SYS_OMAP24_I2C_SPEED, CONFIG_SYS_OMAP24_I2C_SLAVE); #endif @@ -106,6 +113,33 @@ int misc_init_r(void)
am3517_evm_musb_init();
+#if defined(CONFIG_DRIVER_TI_EMAC)
/* activate PHY reset */
gpio_direction_output(30, 0);
gpio_set_value(30, 0);
ctr = 0;
do {
udelay(1000);
ctr++;
} while (ctr < 300);
/* deactivate PHY reset */
gpio_set_value(30, 1);
/* allow the PHY to stabilize and settle down */
ctr = 0;
do {
udelay(1000);
ctr++;
} while (ctr < 300);
/* ensure that the module is out of reset */
reset = readl(AM3517_IP_SW_RESET);
reset &= (~CPGMACSS_SW_RST);
writel(reset,AM3517_IP_SW_RESET);
+#endif
Why do you need to put this "#if defined(CONFIG_DRIVER_TI_EMAC)" here at all? Isn't CONFIG_DRIVER_TI_EMAC enabled for this board always?
It is enabled in arago repository: http://arago-project.org/git/projects/?p=u-boot-omap3.git;a=blob;f=include/c..., but not in upstream u-boot. I was testing the stuff with my own board, so my am3517_evm.h is a little bit messy for now. That's why I wanted to have this change only in board/logicpd/am3517evm/am3517evm.c. It does no harm so far. I could patch am3517_evm.h later.
Ah, I forgot that I marked http://patchwork.ozlabs.org/patch/129720/ as Deferred way back when rather than merge it. I don't know why I didn't need the bit you pulled in when I was testing eth.
The right answer here is to do both of these so that we are using and testing the feature we add. I'll see if I can dig up and boot up my am3517_evm.

On Fri, Dec 06, 2013 at 06:59:52AM -0500, Tom Rini wrote:
On Fri, Dec 06, 2013 at 11:47:38AM +0100, Yegor Yefremov wrote:
On Fri, Dec 6, 2013 at 11:37 AM, Stefan Roese stefan.roese@gmail.com wrote:
On 06.12.2013 11:17, yegorslists@googlemail.com wrote:
From: Yegor Yefremov yegorslists@googlemail.com
Pin 30 is connected to PHY's RESET# signal, so it must be put to high. Otherwise PHY won't be found via MDIO interface.
Signed-off-by: Yegor Yefremov yegorslists@googlemail.com
Looks good. One questions below though:
Changes: v3: use "__maybe_unused", instead of #if defined statement (Stefan Roese) v2: put ctr and reset under #if defined statement, to avoid compiler warnings, when EMAC is not selected
board/logicpd/am3517evm/am3517evm.c | 34 ++++++++++++++++++++++++++++++++++ board/logicpd/am3517evm/am3517evm.h | 2 +- 2 files changed, 35 insertions(+), 1 deletions(-)
diff --git a/board/logicpd/am3517evm/am3517evm.c b/board/logicpd/am3517evm/am3517evm.c index 1569905..3b1dfd1 100644 --- a/board/logicpd/am3517evm/am3517evm.c +++ b/board/logicpd/am3517evm/am3517evm.c @@ -22,6 +22,7 @@ #include <asm/arch/musb.h> #include <asm/mach-types.h> #include <asm/errno.h> +#include <asm/gpio.h> #include <linux/usb/ch9.h> #include <linux/usb/gadget.h> #include <linux/usb/musb.h> @@ -31,6 +32,9 @@
DECLARE_GLOBAL_DATA_PTR;
+#define AM3517_IP_SW_RESET 0x48002598 +#define CPGMACSS_SW_RST (1 << 1)
/*
- Routine: board_init
- Description: Early hardware init.
@@ -98,6 +102,9 @@ static void am3517_evm_musb_init(void) */ int misc_init_r(void) {
__maybe_unused volatile unsigned int ctr;
__maybe_unused u32 reset;
#ifdef CONFIG_SYS_I2C_OMAP34XX i2c_init(CONFIG_SYS_OMAP24_I2C_SPEED, CONFIG_SYS_OMAP24_I2C_SLAVE); #endif @@ -106,6 +113,33 @@ int misc_init_r(void)
am3517_evm_musb_init();
+#if defined(CONFIG_DRIVER_TI_EMAC)
/* activate PHY reset */
gpio_direction_output(30, 0);
gpio_set_value(30, 0);
ctr = 0;
do {
udelay(1000);
ctr++;
} while (ctr < 300);
/* deactivate PHY reset */
gpio_set_value(30, 1);
/* allow the PHY to stabilize and settle down */
ctr = 0;
do {
udelay(1000);
ctr++;
} while (ctr < 300);
/* ensure that the module is out of reset */
reset = readl(AM3517_IP_SW_RESET);
reset &= (~CPGMACSS_SW_RST);
writel(reset,AM3517_IP_SW_RESET);
+#endif
Why do you need to put this "#if defined(CONFIG_DRIVER_TI_EMAC)" here at all? Isn't CONFIG_DRIVER_TI_EMAC enabled for this board always?
It is enabled in arago repository: http://arago-project.org/git/projects/?p=u-boot-omap3.git;a=blob;f=include/c..., but not in upstream u-boot. I was testing the stuff with my own board, so my am3517_evm.h is a little bit messy for now. That's why I wanted to have this change only in board/logicpd/am3517evm/am3517evm.c. It does no harm so far. I could patch am3517_evm.h later.
Ah, I forgot that I marked http://patchwork.ozlabs.org/patch/129720/ as Deferred way back when rather than merge it. I don't know why I didn't need the bit you pulled in when I was testing eth.
The right answer here is to do both of these so that we are using and testing the feature we add. I'll see if I can dig up and boot up my am3517_evm.
Bah, it's not quite trivial to apply the old patch, some other stuff needs to be enabled too, so I'll set this aside for now.

On Fri, Dec 6, 2013 at 1:14 PM, Tom Rini trini@ti.com wrote:
On Fri, Dec 06, 2013 at 06:59:52AM -0500, Tom Rini wrote:
On Fri, Dec 06, 2013 at 11:47:38AM +0100, Yegor Yefremov wrote:
On Fri, Dec 6, 2013 at 11:37 AM, Stefan Roese stefan.roese@gmail.com wrote:
On 06.12.2013 11:17, yegorslists@googlemail.com wrote:
From: Yegor Yefremov yegorslists@googlemail.com
Pin 30 is connected to PHY's RESET# signal, so it must be put to high. Otherwise PHY won't be found via MDIO interface.
Signed-off-by: Yegor Yefremov yegorslists@googlemail.com
Looks good. One questions below though:
Changes: v3: use "__maybe_unused", instead of #if defined statement (Stefan Roese) v2: put ctr and reset under #if defined statement, to avoid compiler warnings, when EMAC is not selected
board/logicpd/am3517evm/am3517evm.c | 34 ++++++++++++++++++++++++++++++++++ board/logicpd/am3517evm/am3517evm.h | 2 +- 2 files changed, 35 insertions(+), 1 deletions(-)
diff --git a/board/logicpd/am3517evm/am3517evm.c b/board/logicpd/am3517evm/am3517evm.c index 1569905..3b1dfd1 100644 --- a/board/logicpd/am3517evm/am3517evm.c +++ b/board/logicpd/am3517evm/am3517evm.c @@ -22,6 +22,7 @@ #include <asm/arch/musb.h> #include <asm/mach-types.h> #include <asm/errno.h> +#include <asm/gpio.h> #include <linux/usb/ch9.h> #include <linux/usb/gadget.h> #include <linux/usb/musb.h> @@ -31,6 +32,9 @@
DECLARE_GLOBAL_DATA_PTR;
+#define AM3517_IP_SW_RESET 0x48002598 +#define CPGMACSS_SW_RST (1 << 1)
/*
- Routine: board_init
- Description: Early hardware init.
@@ -98,6 +102,9 @@ static void am3517_evm_musb_init(void) */ int misc_init_r(void) {
__maybe_unused volatile unsigned int ctr;
__maybe_unused u32 reset;
#ifdef CONFIG_SYS_I2C_OMAP34XX i2c_init(CONFIG_SYS_OMAP24_I2C_SPEED, CONFIG_SYS_OMAP24_I2C_SLAVE); #endif @@ -106,6 +113,33 @@ int misc_init_r(void)
am3517_evm_musb_init();
+#if defined(CONFIG_DRIVER_TI_EMAC)
/* activate PHY reset */
gpio_direction_output(30, 0);
gpio_set_value(30, 0);
ctr = 0;
do {
udelay(1000);
ctr++;
} while (ctr < 300);
/* deactivate PHY reset */
gpio_set_value(30, 1);
/* allow the PHY to stabilize and settle down */
ctr = 0;
do {
udelay(1000);
ctr++;
} while (ctr < 300);
/* ensure that the module is out of reset */
reset = readl(AM3517_IP_SW_RESET);
reset &= (~CPGMACSS_SW_RST);
writel(reset,AM3517_IP_SW_RESET);
+#endif
Why do you need to put this "#if defined(CONFIG_DRIVER_TI_EMAC)" here at all? Isn't CONFIG_DRIVER_TI_EMAC enabled for this board always?
It is enabled in arago repository: http://arago-project.org/git/projects/?p=u-boot-omap3.git;a=blob;f=include/c..., but not in upstream u-boot. I was testing the stuff with my own board, so my am3517_evm.h is a little bit messy for now. That's why I wanted to have this change only in board/logicpd/am3517evm/am3517evm.c. It does no harm so far. I could patch am3517_evm.h later.
Ah, I forgot that I marked http://patchwork.ozlabs.org/patch/129720/ as Deferred way back when rather than merge it. I don't know why I didn't need the bit you pulled in when I was testing eth.
The right answer here is to do both of these so that we are using and testing the feature we add. I'll see if I can dig up and boot up my am3517_evm.
Bah, it's not quite trivial to apply the old patch, some other stuff needs to be enabled too, so I'll set this aside for now.
I've added #define CONFIG_PHYLIB and #define CONFIG_OMAP_GPIO
Yegor
participants (4)
-
Stefan Roese
-
Tom Rini
-
Yegor Yefremov
-
yegorslists@googlemail.com