[U-Boot] [PATCH] net: Add \n before warning message so it prints on a new line.

Signed-off-by: Philip Balister philip@opensdr.com --- net/eth.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/eth.c b/net/eth.c index dbd1e2d..67a8039 100644 --- a/net/eth.c +++ b/net/eth.c @@ -305,7 +305,7 @@ int eth_initialize(bd_t *bis) puts("\nWarning: eth device name has a space!\n");
if (eth_write_hwaddr(dev, "eth", eth_number)) - puts("Warning: failed to set MAC address\n"); + puts("\nWarning: failed to set MAC address\n");
eth_number++; dev = dev->next;

The existing timing does not quite meet the minimum requirements in the LAN9221 datasheet. The timing in this patch solves problems noticed on some parts.
Signed-off-by: Philip Balister philip@opensdr.com --- board/overo/overo.h | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/board/overo/overo.h b/board/overo/overo.h index 68e1243..617c0c3 100644 --- a/board/overo/overo.h +++ b/board/overo/overo.h @@ -35,10 +35,10 @@ const omap3_sysinfo sysinfo = {
/* GPMC CS 5 connected to an SMSC LAN9221 ethernet controller */ #define NET_LAN9221_GPMC_CONFIG1 0x00001000 -#define NET_LAN9221_GPMC_CONFIG2 0x00080701 +#define NET_LAN9221_GPMC_CONFIG2 0x00060700 #define NET_LAN9221_GPMC_CONFIG3 0x00020201 -#define NET_LAN9221_GPMC_CONFIG4 0x08030703 -#define NET_LAN9221_GPMC_CONFIG5 0x00060908 +#define NET_LAN9221_GPMC_CONFIG4 0x06000700 +#define NET_LAN9221_GPMC_CONFIG5 0x0006090A #define NET_LAN9221_GPMC_CONFIG6 0x87030000 #define NET_LAN9221_GPMC_CONFIG7 0x00000f6c

On Wed, Sep 7, 2011 at 4:57 AM, Philip Balister philip@balister.org wrote:
The existing timing does not quite meet the minimum requirements in the LAN9221 datasheet. The timing in this patch solves problems noticed on some parts.
Signed-off-by: Philip Balister philip@opensdr.com
Acked-by: Steve Sakoman steve@sakoman.com Tested-by: Steve Sakoman steve@sakoman.com
board/overo/overo.h | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/board/overo/overo.h b/board/overo/overo.h index 68e1243..617c0c3 100644 --- a/board/overo/overo.h +++ b/board/overo/overo.h @@ -35,10 +35,10 @@ const omap3_sysinfo sysinfo = {
/* GPMC CS 5 connected to an SMSC LAN9221 ethernet controller */ #define NET_LAN9221_GPMC_CONFIG1 0x00001000 -#define NET_LAN9221_GPMC_CONFIG2 0x00080701 +#define NET_LAN9221_GPMC_CONFIG2 0x00060700 #define NET_LAN9221_GPMC_CONFIG3 0x00020201 -#define NET_LAN9221_GPMC_CONFIG4 0x08030703 -#define NET_LAN9221_GPMC_CONFIG5 0x00060908 +#define NET_LAN9221_GPMC_CONFIG4 0x06000700 +#define NET_LAN9221_GPMC_CONFIG5 0x0006090A #define NET_LAN9221_GPMC_CONFIG6 0x87030000 #define NET_LAN9221_GPMC_CONFIG7 0x00000f6c
-- 1.7.4.4
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot

2011/9/8 Steve Sakoman sakoman@gmail.com:
On Wed, Sep 7, 2011 at 4:57 AM, Philip Balister philip@balister.org wrote:
The existing timing does not quite meet the minimum requirements in the LAN9221 datasheet. The timing in this patch solves problems noticed on some parts.
Signed-off-by: Philip Balister philip@opensdr.com
Acked-by: Steve Sakoman steve@sakoman.com Tested-by: Steve Sakoman steve@sakoman.com
board/overo/overo.h | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/board/overo/overo.h b/board/overo/overo.h index 68e1243..617c0c3 100644 --- a/board/overo/overo.h +++ b/board/overo/overo.h @@ -35,10 +35,10 @@ const omap3_sysinfo sysinfo = {
/* GPMC CS 5 connected to an SMSC LAN9221 ethernet controller */ #define NET_LAN9221_GPMC_CONFIG1 0x00001000 -#define NET_LAN9221_GPMC_CONFIG2 0x00080701 +#define NET_LAN9221_GPMC_CONFIG2 0x00060700 #define NET_LAN9221_GPMC_CONFIG3 0x00020201 -#define NET_LAN9221_GPMC_CONFIG4 0x08030703 -#define NET_LAN9221_GPMC_CONFIG5 0x00060908 +#define NET_LAN9221_GPMC_CONFIG4 0x06000700 +#define NET_LAN9221_GPMC_CONFIG5 0x0006090A #define NET_LAN9221_GPMC_CONFIG6 0x87030000 #define NET_LAN9221_GPMC_CONFIG7 0x00000f6c
-- 1.7.4.4
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Other boards like IGEP v2 board uses the same LAN9221 ethernet controller, so this code is duplicated on overo.h and igep0020.h files. Maybe could be a good idea move this part to arch/arm/include/asm/arch-omap3/omap_gpmc.h or another file to not duplicate the code.

On 09/08/2011 10:45 AM, Enric Balletbò i Serra wrote:
2011/9/8 Steve Sakomansakoman@gmail.com:
On Wed, Sep 7, 2011 at 4:57 AM, Philip Balisterphilip@balister.org wrote:
The existing timing does not quite meet the minimum requirements in the LAN9221 datasheet. The timing in this patch solves problems noticed on some parts.
Signed-off-by: Philip Balisterphilip@opensdr.com
Acked-by: Steve Sakomansteve@sakoman.com Tested-by: Steve Sakomansteve@sakoman.com
board/overo/overo.h | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/board/overo/overo.h b/board/overo/overo.h index 68e1243..617c0c3 100644 --- a/board/overo/overo.h +++ b/board/overo/overo.h @@ -35,10 +35,10 @@ const omap3_sysinfo sysinfo = {
/* GPMC CS 5 connected to an SMSC LAN9221 ethernet controller */ #define NET_LAN9221_GPMC_CONFIG1 0x00001000 -#define NET_LAN9221_GPMC_CONFIG2 0x00080701 +#define NET_LAN9221_GPMC_CONFIG2 0x00060700 #define NET_LAN9221_GPMC_CONFIG3 0x00020201 -#define NET_LAN9221_GPMC_CONFIG4 0x08030703 -#define NET_LAN9221_GPMC_CONFIG5 0x00060908 +#define NET_LAN9221_GPMC_CONFIG4 0x06000700 +#define NET_LAN9221_GPMC_CONFIG5 0x0006090A #define NET_LAN9221_GPMC_CONFIG6 0x87030000 #define NET_LAN9221_GPMC_CONFIG7 0x00000f6c
-- 1.7.4.4
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Other boards like IGEP v2 board uses the same LAN9221 ethernet controller, so this code is duplicated on overo.h and igep0020.h files. Maybe could be a good idea move this part to arch/arm/include/asm/arch-omap3/omap_gpmc.h or another file to not duplicate the code.
Is there anyone around with an igep v2 board who could test/ack such a patch?
Philip

2011/9/8 Philip Balister philip@opensdr.com:
On 09/08/2011 10:45 AM, Enric Balletbò i Serra wrote:
2011/9/8 Steve Sakomansakoman@gmail.com:
On Wed, Sep 7, 2011 at 4:57 AM, Philip Balisterphilip@balister.org wrote:
The existing timing does not quite meet the minimum requirements in the LAN9221 datasheet. The timing in this patch solves problems noticed on some parts.
Signed-off-by: Philip Balisterphilip@opensdr.com
Acked-by: Steve Sakomansteve@sakoman.com Tested-by: Steve Sakomansteve@sakoman.com
board/overo/overo.h | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/board/overo/overo.h b/board/overo/overo.h index 68e1243..617c0c3 100644 --- a/board/overo/overo.h +++ b/board/overo/overo.h @@ -35,10 +35,10 @@ const omap3_sysinfo sysinfo = {
/* GPMC CS 5 connected to an SMSC LAN9221 ethernet controller */ #define NET_LAN9221_GPMC_CONFIG1 0x00001000 -#define NET_LAN9221_GPMC_CONFIG2 0x00080701 +#define NET_LAN9221_GPMC_CONFIG2 0x00060700 #define NET_LAN9221_GPMC_CONFIG3 0x00020201 -#define NET_LAN9221_GPMC_CONFIG4 0x08030703 -#define NET_LAN9221_GPMC_CONFIG5 0x00060908 +#define NET_LAN9221_GPMC_CONFIG4 0x06000700 +#define NET_LAN9221_GPMC_CONFIG5 0x0006090A #define NET_LAN9221_GPMC_CONFIG6 0x87030000 #define NET_LAN9221_GPMC_CONFIG7 0x00000f6c
-- 1.7.4.4
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Other boards like IGEP v2 board uses the same LAN9221 ethernet controller, so this code is duplicated on overo.h and igep0020.h files. Maybe could be a good idea move this part to arch/arm/include/asm/arch-omap3/omap_gpmc.h or another file to not duplicate the code.
Is there anyone around with an igep v2 board who could test/ack such a patch?
Philip
Yes, I can, I expect test the patch tomorrow ...
Enric

Without this change CS's configured for synchronous clocking cannot read data.
Signed-off-by: Philip Balister philip@opensdr.com --- board/overo/overo.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/board/overo/overo.h b/board/overo/overo.h index 68e1243..1f94669 100644 --- a/board/overo/overo.h +++ b/board/overo/overo.h @@ -128,7 +128,7 @@ const omap3_sysinfo sysinfo = { MUX_VAL(CP(GPMC_NCS6), (IEN | PTD | DIS | M0)) /*GPMC_nCS6*/\ MUX_VAL(CP(GPMC_NCS7), (IEN | PTU | EN | M0)) /*GPMC_nCS7*/\ MUX_VAL(CP(GPMC_NBE1), (IEN | PTD | DIS | M0)) /*GPMC_nCS3*/\ - MUX_VAL(CP(GPMC_CLK), (IDIS | PTU | EN | M0)) /*GPMC_CLK*/\ + MUX_VAL(CP(GPMC_CLK), (IEN | PTU | EN | M0)) /*GPMC_CLK*/\ MUX_VAL(CP(GPMC_NADV_ALE), (IDIS | PTD | DIS | M0)) /*GPMC_nADV_ALE*/\ MUX_VAL(CP(GPMC_NOE), (IDIS | PTD | DIS | M0)) /*GPMC_nOE*/\ MUX_VAL(CP(GPMC_NWE), (IDIS | PTD | DIS | M0)) /*GPMC_nWE*/\

Hi Philip,
Le 07/09/2011 13:57, Philip Balister a écrit :
Signed-off-by: Philip Balisterphilip@opensdr.com
net/eth.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/eth.c b/net/eth.c index dbd1e2d..67a8039 100644 --- a/net/eth.c +++ b/net/eth.c @@ -305,7 +305,7 @@ int eth_initialize(bd_t *bis) puts("\nWarning: eth device name has a space!\n");
if (eth_write_hwaddr(dev, "eth", eth_number))
puts("Warning: failed to set MAC address\n");
puts("\nWarning: failed to set MAC address\n");
I believe warning messages with \n on more than one end are frowned upon.
eth_number++; dev = dev->next;
Amicalement,

On 09/08/2011 11:01 AM, Albert ARIBAUD wrote:
Hi Philip,
Le 07/09/2011 13:57, Philip Balister a écrit :
Signed-off-by: Philip Balisterphilip@opensdr.com
net/eth.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/eth.c b/net/eth.c index dbd1e2d..67a8039 100644 --- a/net/eth.c +++ b/net/eth.c @@ -305,7 +305,7 @@ int eth_initialize(bd_t *bis) puts("\nWarning: eth device name has a space!\n");
if (eth_write_hwaddr(dev, "eth", eth_number))
- puts("Warning: failed to set MAC address\n");
- puts("\nWarning: failed to set MAC address\n");
I believe warning messages with \n on more than one end are frowned upon.
Look closely at the patch, the warning message above has to do the same thing. Without the leading \n on the message, it prints directly after the ethernet chip name, with no space. This is not right. I chose to copy the existing code, rather than add a leading space.
Philip
eth_number++; dev = dev->next;
Amicalement,

Le 08/09/2011 17:04, Philip Balister a écrit :
On 09/08/2011 11:01 AM, Albert ARIBAUD wrote:
Hi Philip,
Le 07/09/2011 13:57, Philip Balister a écrit :
Signed-off-by: Philip Balisterphilip@opensdr.com
net/eth.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/eth.c b/net/eth.c index dbd1e2d..67a8039 100644 --- a/net/eth.c +++ b/net/eth.c @@ -305,7 +305,7 @@ int eth_initialize(bd_t *bis) puts("\nWarning: eth device name has a space!\n");
if (eth_write_hwaddr(dev, "eth", eth_number))
- puts("Warning: failed to set MAC address\n");
- puts("\nWarning: failed to set MAC address\n");
I believe warning messages with \n on more than one end are frowned upon.
Look closely at the patch, the warning message above has to do the same thing. Without the leading \n on the message, it prints directly after the ethernet chip name, with no space. This is not right. I chose to copy the existing code, rather than add a leading space.
Just because original code has an error does not make it right to reproduce it. :)
More seriously, heterogeneous \n placement makes it complicated to get and keep printing right -- for instance here the warning messages have a trailing \n but the code loop adds one at the end.
I would prefer that the code do a first loop (while dev!= eth_devices) to try and set up ethernet devices, emitting simple "warning\n" messages as needed, then a second loop (for dev=0 to eth_number-1) to print a summary of the final list of eth devices found and initialized.
Amicalement,
participants (5)
-
Albert ARIBAUD
-
Enric Balletbò i Serra
-
Philip Balister
-
Philip Balister
-
Steve Sakoman