[U-Boot] [PATCH 4/5] powerpc: keymile: Add a check for the PIGGY debug board

Check the presence of the PIGGY on the keymile boards mgcoge, mgsuvd and kmeter1. If the PIGGY is not present, dont register this Ethernet device.
Signed-off-by: Heiko Schocher hs@denx.de --- board/keymile/common/common.c | 16 ++++++++++++++-- board/keymile/kmeter1/Makefile | 6 ++++-- board/keymile/kmeter1/kmeter1.c | 7 ++++++- board/keymile/mgcoge/mgcoge.c | 9 +++++++-- board/keymile/mgsuvd/mgsuvd.c | 8 +++++++- cpu/mpc8260/ether_scc.c | 8 ++++++++ cpu/mpc8xx/scc.c | 6 ++++++ drivers/qe/uec.c | 7 +++++++ include/configs/kmeter1.h | 10 ++++++++++ include/configs/mgcoge.h | 11 +++++++++++ include/configs/mgsuvd.h | 10 ++++++++++ include/net.h | 4 +++- 12 files changed, 93 insertions(+), 9 deletions(-)
diff --git a/board/keymile/common/common.c b/board/keymile/common/common.c index a4cf24c..54c8730 100644 --- a/board/keymile/common/common.c +++ b/board/keymile/common/common.c @@ -22,10 +22,14 @@ */
#include <common.h> +#if defined(CONFIG_MGCOGE) #include <mpc8260.h> +#endif #include <ioports.h> #include <malloc.h> #include <hush.h> +#include <net.h> +#include <asm/io.h>
#if defined(CONFIG_OF_BOARD_SETUP) && defined(CONFIG_OF_LIBFDT) #include <libfdt.h> @@ -33,8 +37,6 @@
#if defined(CONFIG_HARD_I2C) || defined(CONFIG_SOFT_I2C) #include <i2c.h> -#endif -#include <asm/io.h>
extern int i2c_soft_read_pin (void);
@@ -495,6 +497,7 @@ void i2c_init_board(void) #endif } #endif +#endif
#if defined(CONFIG_OF_BOARD_SETUP) && defined(CONFIG_OF_LIBFDT) int fdt_set_node_and_value (void *blob, @@ -521,3 +524,12 @@ int fdt_set_node_and_value (void *blob, return ret; } #endif + +#if defined(CONFIG_CHECK_ETHERNET_PRESENT) +int ethernet_present (int index) +{ + int ret; + ret = in_8((u8 *)CONFIG_SYS_PIGGY_BASE + CONFIG_SYS_SLOT_ID_OFF) & 0x80; + return ret; +} +#endif diff --git a/board/keymile/kmeter1/Makefile b/board/keymile/kmeter1/Makefile index 88b79f3..12a1518 100644 --- a/board/keymile/kmeter1/Makefile +++ b/board/keymile/kmeter1/Makefile @@ -22,12 +22,14 @@ #
include $(TOPDIR)/config.mk +ifneq ($(OBJTREE),$(SRCTREE)) +$(shell mkdir -p $(obj)../common) +endif
LIB = $(obj)lib$(BOARD).a
-COBJS-y += $(BOARD).o +COBJS += $(BOARD).o ../common/common.o
-COBJS := $(COBJS-y) SRCS := $(SOBJS:.o=.S) $(COBJS:.o=.c) OBJS := $(addprefix $(obj),$(COBJS)) SOBJS := $(addprefix $(obj),$(SOBJS)) diff --git a/board/keymile/kmeter1/kmeter1.c b/board/keymile/kmeter1/kmeter1.c index f9a59a6..03f5dda 100644 --- a/board/keymile/kmeter1/kmeter1.c +++ b/board/keymile/kmeter1/kmeter1.c @@ -141,7 +141,12 @@ phys_size_t initdram (int board_type)
int checkboard (void) { - puts ("Board: Keymile kmeter1\n"); + puts ("Board: Keymile kmeter1"); +#if defined(CONFIG_CHECK_ETHERNET_PRESENT) + if (ethernet_present (0)) + puts (" with PIGGY."); +#endif + puts ("\n"); return 0; }
diff --git a/board/keymile/mgcoge/mgcoge.c b/board/keymile/mgcoge/mgcoge.c index 3683417..98a4d43 100644 --- a/board/keymile/mgcoge/mgcoge.c +++ b/board/keymile/mgcoge/mgcoge.c @@ -25,6 +25,7 @@ #include <mpc8260.h> #include <ioports.h> #include <malloc.h> +#include <net.h> #include <asm/io.h>
#if defined(CONFIG_OF_BOARD_SETUP) && defined(CONFIG_OF_LIBFDT) @@ -285,8 +286,12 @@ phys_size_t initdram (int board_type)
int checkboard(void) { - puts ("Board: mgcoge\n"); - + puts ("Board: Keymile mgcoge"); +#if defined(CONFIG_CHECK_ETHERNET_PRESENT) + if (ethernet_present (0)) + puts (" with PIGGY."); +#endif + puts ("\n"); return 0; }
diff --git a/board/keymile/mgsuvd/mgsuvd.c b/board/keymile/mgsuvd/mgsuvd.c index 3726acf..eb4c285 100644 --- a/board/keymile/mgsuvd/mgsuvd.c +++ b/board/keymile/mgsuvd/mgsuvd.c @@ -22,6 +22,7 @@ */ #include <common.h> #include <mpc8xx.h> +#include <net.h> #include <asm/io.h>
#if defined(CONFIG_OF_BOARD_SETUP) && defined(CONFIG_OF_LIBFDT) @@ -60,7 +61,12 @@ const uint sdram_table[] =
int checkboard (void) { - puts ("Board: Keymile mgsuvd\n"); + puts ("Board: Keymile mgsuvd"); +#if defined(CONFIG_CHECK_ETHERNET_PRESENT) + if (ethernet_present (0)) + puts (" with PIGGY."); +#endif + puts ("\n"); return (0); }
diff --git a/cpu/mpc8260/ether_scc.c b/cpu/mpc8260/ether_scc.c index c65f0e0..a79d56c 100644 --- a/cpu/mpc8260/ether_scc.c +++ b/cpu/mpc8260/ether_scc.c @@ -191,6 +191,14 @@ int eth_init(bd_t *bis) scc_enet_t *pram_ptr; uint dpaddr;
+#if defined(CONFIG_CHECK_ETHERNET_PRESENT) + if (ethernet_present (CONFIG_ETHER_INDEX) == 0) { + printf("Ethernet index: %d not present.\n", + CONFIG_ETHER_INDEX); + return -1; + } +#endif + rxIdx = 0; txIdx = 0;
diff --git a/cpu/mpc8xx/scc.c b/cpu/mpc8xx/scc.c index effb967..4be4d89 100644 --- a/cpu/mpc8xx/scc.c +++ b/cpu/mpc8xx/scc.c @@ -74,6 +74,12 @@ int scc_initialize(bd_t *bis) { struct eth_device* dev;
+#if defined(CONFIG_CHECK_ETHERNET_PRESENT) + if (ethernet_present (0) == 0) { + printf("Ethernet not present.\n"); + return -1; + } +#endif dev = (struct eth_device*) malloc(sizeof *dev); memset(dev, 0, sizeof *dev);
diff --git a/drivers/qe/uec.c b/drivers/qe/uec.c index ed7ed65..8ae6d0f 100644 --- a/drivers/qe/uec.c +++ b/drivers/qe/uec.c @@ -1385,6 +1385,13 @@ int uec_initialize(int index) uec_info_t *uec_info; int err;
+#if defined(CONFIG_CHECK_ETHERNET_PRESENT) + if (ethernet_present (index) == 0) { + printf("Ethernet index: %d not present.\n", + index); + return -1; + } +#endif dev = (struct eth_device *)malloc(sizeof(struct eth_device)); if (!dev) return 0; diff --git a/include/configs/kmeter1.h b/include/configs/kmeter1.h index d0fe6a3..e443086 100644 --- a/include/configs/kmeter1.h +++ b/include/configs/kmeter1.h @@ -314,6 +314,16 @@ #define CONFIG_SYS_LOADS_BAUD_CHANGE 1 /* allow baudrate change */
/* + * How to get access to the slot ID. Put this here to make it easy + * to modify in a centralized location. This is used in the HDLC + * driver to set the MAC. +*/ +#define CONFIG_CHECK_ETHERNET_PRESENT 1 +#define CONFIG_SYS_SLOT_ID_BASE CONFIG_SYS_PIGGY_BASE +#define CONFIG_SYS_SLOT_ID_OFF (0x07) /* register offset */ +#define CONFIG_SYS_SLOT_ID_MASK (0x3f) /* mask for slot ID bits */ + +/* * BOOTP options */ #define CONFIG_BOOTP_BOOTFILESIZE diff --git a/include/configs/mgcoge.h b/include/configs/mgcoge.h index dc9b311..bfac3b0 100644 --- a/include/configs/mgcoge.h +++ b/include/configs/mgcoge.h @@ -400,4 +400,15 @@ #define OF_TBCLK (bd->bi_busfreq / 4) #define OF_STDOUT_PATH "/soc/cpm/serial@11a90"
+/* + * How to get access to the slot ID. Put this here to make it easy + * to modify in a centralized location. This is used in the HDLC + * driver to set the MAC. +*/ +#define CONFIG_CHECK_ETHERNET_PRESENT 1 +#define CONFIG_SYS_SLOT_ID_BASE CONFIG_SYS_PIGGY_BASE +#define CONFIG_SYS_SLOT_ID_OFF (0x07) /* register offset */ +#define CONFIG_SYS_SLOT_ID_MASK (0x3f) /* mask for slot ID bits */ + + #endif /* __CONFIG_H */ diff --git a/include/configs/mgsuvd.h b/include/configs/mgsuvd.h index fca2e55..0bd2f40 100644 --- a/include/configs/mgsuvd.h +++ b/include/configs/mgsuvd.h @@ -211,6 +211,16 @@ #define CONFIG_ENV_OFFSET_REDUND (CONFIG_ENV_OFFSET+CONFIG_ENV_SECT_SIZE) #define CONFIG_ENV_SIZE_REDUND (CONFIG_ENV_SIZE)
+/* + * How to get access to the slot ID. Put this here to make it easy + * to modify in a centralized location. This is used in the HDLC + * driver to set the MAC. +*/ +#define CONFIG_CHECK_ETHERNET_PRESENT 1 +#define CONFIG_SYS_SLOT_ID_BASE CONFIG_SYS_PIGGY_BASE +#define CONFIG_SYS_SLOT_ID_OFF (0x07) /* register offset */ +#define CONFIG_SYS_SLOT_ID_MASK (0x3f) /* mask for slot ID bits */ + /*----------------------------------------------------------------------- * Cache Configuration */ diff --git a/include/net.h b/include/net.h index a5a256b..112dd7b 100644 --- a/include/net.h +++ b/include/net.h @@ -489,6 +489,8 @@ extern ushort getenv_VLAN(char *); /* copy a filename (allow for "..." notation, limit length) */ extern void copy_filename (char *dst, char *src, int size);
-/**********************************************************************/ +#if defined(CONFIG_CHECK_ETHERNET_PRESENT) +extern int ethernet_present (int index); +#endif
#endif /* __NET_H__ */

Hi Heiko,
Heiko Schocher wrote:
Check the presence of the PIGGY on the keymile boards mgcoge, mgsuvd and kmeter1. If the PIGGY is not present, dont register this Ethernet device.
Signed-off-by: Heiko Schocher hs@denx.de
This looks like useful stuff to have, but I'd prefer that you put the check logic in board_eth_init() rather than adding to the individual device drivers. I know the 8260 SCC driver is the older style, which precludes the use of board_eth_init, but I'll convert it if you're able to test.
regards, Ben

Hello Ben
Ben Warren wrote:
Heiko Schocher wrote:
Check the presence of the PIGGY on the keymile boards mgcoge, mgsuvd and kmeter1. If the PIGGY is not present, dont register this Ethernet device.
Signed-off-by: Heiko Schocher hs@denx.de
This looks like useful stuff to have, but I'd prefer that you put the check logic in board_eth_init() rather than adding to the individual device drivers. I know the 8260 SCC driver is the older style, which precludes the use of board_eth_init, but I'll convert it if you're able to test.
Yes, I could test such a change for you, but hmm... I am not sure, if board_eth_init () is the right place for my purpose. I need for every Ethernet device a selection, if this device is present or not. Correct me if I am wrong, but it looks like board_eth_init () is not made for this purpose. (Ok, I can do a specific device init in board_eth_init (), but then we must do something, that prevents that the device is again initialized in eth_initialize () ...
Hmm... while writing this it comes a idea in my mind: we could move all the *_initialize functions in eth_initialize () in a seperate function, say eth_hardware_init() and maybe making this function "weak", so a board writer can write his own eth_hardware_init() ... in such a function, I could check which device is present, and only initialize the present devices ... what do you think?
bye Heiko

Heiko Schocher wrote:
Hello Ben
Ben Warren wrote:
Heiko Schocher wrote:
Check the presence of the PIGGY on the keymile boards mgcoge, mgsuvd and kmeter1. If the PIGGY is not present, dont register this Ethernet device.
Signed-off-by: Heiko Schocher hs@denx.de
This looks like useful stuff to have, but I'd prefer that you put the check logic in board_eth_init() rather than adding to the individual device drivers. I know the 8260 SCC driver is the older style, which precludes the use of board_eth_init, but I'll convert it if you're able to test.
Yes, I could test such a change for you, but hmm... I am not sure, if board_eth_init () is the right place for my purpose. I need for every Ethernet device a selection, if this device is present or not. Correct me if I am wrong, but it looks like board_eth_init () is not made for this purpose. (Ok, I can do a specific device init in board_eth_init (), but then we must do something, that prevents that the device is again initialized in eth_initialize () ...
board_eth_init() was introduced for exactly this sort of thing. Have a look at the net repo (I've sent a pull request to Wolfgang so the current changes will make it into the 12.2008 release). There aren't any device initializations left in eth_initialize(), so there's no issue of a device being initialized twice. The goal is for all devices to be started by cpu_eth_int() or board_eth_init().
Hmm... while writing this it comes a idea in my mind: we could move all the *_initialize functions in eth_initialize () in a seperate function, say eth_hardware_init() and maybe making this function "weak", so a board writer can write his own eth_hardware_init() ... in such a function, I could check which device is present, and only initialize the present devices ... what do you think?
That's what board_eth_init() and cpu_eth_init() are for. In addition, I forgot to mention that a couple of days ago Gary Jennejohn submitted a patch for changing the 82xx SCC driver over to CONFIG_NET_MULTI style.
regards, Ben

Hello Ben,
Ben Warren wrote:
Heiko Schocher wrote:
Hello Ben
Ben Warren wrote:
Heiko Schocher wrote:
Check the presence of the PIGGY on the keymile boards mgcoge, mgsuvd and kmeter1. If the PIGGY is not present, dont register this Ethernet device.
Signed-off-by: Heiko Schocher hs@denx.de
This looks like useful stuff to have, but I'd prefer that you put the check logic in board_eth_init() rather than adding to the individual device drivers. I know the 8260 SCC driver is the older style, which precludes the use of board_eth_init, but I'll convert it if you're able to test.
Yes, I could test such a change for you, but hmm... I am not sure, if board_eth_init () is the right place for my purpose. I need for every Ethernet device a selection, if this device is present or not. Correct me if I am wrong, but it looks like board_eth_init () is not made for this purpose. (Ok, I can do a specific device init in board_eth_init (), but then we must do something, that prevents that the device is again initialized in eth_initialize () ...
board_eth_init() was introduced for exactly this sort of thing. Have a look at the net repo (I've sent a pull request to Wolfgang so the current changes will make it into the 12.2008 release). There aren't any device initializations left in eth_initialize(), so there's no issue of a device being initialized twice. The goal is for all devices to be started by cpu_eth_int() or board_eth_init().
Ahh... now I see it. I adjust my patch, thanks.
bye Heiko

On Thu, 13 Nov 2008 09:30:51 -0800 Ben Warren biggerbadderben@gmail.com wrote:
Heiko Schocher wrote:
Hello Ben
Ben Warren wrote:
Heiko Schocher wrote:
Check the presence of the PIGGY on the keymile boards mgcoge, mgsuvd and kmeter1. If the PIGGY is not present, dont register this Ethernet device.
Signed-off-by: Heiko Schocher hs@denx.de
This looks like useful stuff to have, but I'd prefer that you put the check logic in board_eth_init() rather than adding to the individual device drivers. I know the 8260 SCC driver is the older style, which precludes the use of board_eth_init, but I'll convert it if you're able to test.
Unfortunately, this approach won't work. First of all, the 82xx SCC driver is now initialized in cpu_eth_init(), which knows nothing about board-specific peculiarities like the PIGGY. Secondly, the HDLC driver for Keymile has to be initialized in board_eth_init(), and it has nothing to do with the PIGGY. Putting the check in board_eth_init() would break it completely. I looked at Heiko's latest patch and couldn't figure out a way to cleanly differentiate between initializing the HDLC driver and checking whether the PIGGY was present fo the other ENET drivers.
Yes, I could test such a change for you, but hmm... I am not sure, if board_eth_init () is the right place for my purpose. I need for every Ethernet device a selection, if this device is present or not. Correct me if I am wrong, but it looks like board_eth_init () is not made for this purpose. (Ok, I can do a specific device init in board_eth_init (), but then we must do something, that prevents that the device is again initialized in eth_initialize () ...
board_eth_init() was introduced for exactly this sort of thing. Have a look at the net repo (I've sent a pull request to Wolfgang so the current changes will make it into the 12.2008 release). There aren't any device initializations left in eth_initialize(), so there's no issue of a device being initialized twice. The goal is for all devices to be started by cpu_eth_int() or board_eth_init().
Hmm... while writing this it comes a idea in my mind: we could move all the *_initialize functions in eth_initialize () in a seperate function, say eth_hardware_init() and maybe making this function "weak", so a board writer can write his own eth_hardware_init() ... in such a function, I could check which device is present, and only initialize the present devices ... what do you think?
That's what board_eth_init() and cpu_eth_init() are for. In addition, I forgot to mention that a couple of days ago Gary Jennejohn submitted a patch for changing the 82xx SCC driver over to CONFIG_NET_MULTI style.
Correct, and it must explicitly call ethernet_present() - see above.
--- Gary Jennejohn ********************************************************************* DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de *********************************************************************

Hello Gray,
Gary Jennejohn schrieb:
On Thu, 13 Nov 2008 09:30:51 -0800 Ben Warren biggerbadderben@gmail.com wrote:
Heiko Schocher wrote:
Hello Ben
Ben Warren wrote:
Heiko Schocher wrote:
Check the presence of the PIGGY on the keymile boards mgcoge, mgsuvd and kmeter1. If the PIGGY is not present, dont register this Ethernet device.
Signed-off-by: Heiko Schocher hs@denx.de
This looks like useful stuff to have, but I'd prefer that you put the check logic in board_eth_init() rather than adding to the individual device drivers. I know the 8260 SCC driver is the older style, which precludes the use of board_eth_init, but I'll convert it if you're able to test.
Unfortunately, this approach won't work. First of all, the 82xx SCC driver is now initialized in cpu_eth_init(), which knows nothing about board-specific peculiarities like the PIGGY. Secondly, the HDLC driver for Keymile has to be initialized in board_eth_init(), and it has nothing to do with the PIGGY. Putting the check in board_eth_init() would break it completely. I looked at Heiko's latest patch and couldn't figure out a way to cleanly differentiate between initializing the HDLC driver and checking whether the PIGGY was present fo the other ENET drivers.
Hmm.. you can now initialize in board_eth_init () in board/keymile/common/common.c your HDLC specific things, or?
[...]
That's what board_eth_init() and cpu_eth_init() are for. In addition, I forgot to mention that a couple of days ago Gary Jennejohn submitted a patch for changing the 82xx SCC driver over to CONFIG_NET_MULTI style.
Correct, and it must explicitly call ethernet_present() - see above.
If I understand the actual code right, then _now_ (which means actual top of tree+ patch 4/5) it gets first called board_eth_init (). There, I check if ethernet_present (), and if so, I return -1, which will result in calling cpu_eth_init(). If the ethernet is not present, it returns 0, which will result in not calling cpu_eth_init() ... And your HDLC initialization can be done in board_eth_init() ... thats what we need, or?
bye, Heiko

On Wed, 19 Nov 2008 14:03:44 +0100 Heiko Schocher hs@denx.de wrote:
Gary Jennejohn schrieb:
On Thu, 13 Nov 2008 09:30:51 -0800 Ben Warren biggerbadderben@gmail.com wrote:
[snip]
That's what board_eth_init() and cpu_eth_init() are for. In addition, I forgot to mention that a couple of days ago Gary Jennejohn submitted a patch for changing the 82xx SCC driver over to CONFIG_NET_MULTI style.
Correct, and it must explicitly call ethernet_present() - see above.
If I understand the actual code right, then _now_ (which means actual top of tree+ patch 4/5) it gets first called board_eth_init (). There, I check if ethernet_present (), and if so, I return -1, which will result in calling cpu_eth_init(). If the ethernet is not present, it returns 0, which will result in not calling cpu_eth_init() ... And your HDLC initialization can be done in board_eth_init() ... thats what we need, or?
Ah yes. My face is red. I read the logic backwards. If the PIGGY is present it returns -1 ==> call cpu_eth_init(), otherwise 0.
And keymile_hdlc_enet_initialize() always succeeds so it can always be called.
OK. Then all we need is to add a call to keymile_hdlc_enet_initialize() to eth_board_init() before the PIGGY check. Case closed.
And I have to remove the call to ethernet_present() from the 82xx SCC driver.
Thanks Heiko.
--- Gary Jennejohn ********************************************************************* DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de *********************************************************************

Hi Gary,
Gary Jennejohn wrote:
On Thu, 13 Nov 2008 09:30:51 -0800 Ben Warren biggerbadderben@gmail.com wrote:
Heiko Schocher wrote:
Hello Ben
Ben Warren wrote:
Heiko Schocher wrote:
Check the presence of the PIGGY on the keymile boards mgcoge, mgsuvd and kmeter1. If the PIGGY is not present, dont register this Ethernet device.
Signed-off-by: Heiko Schocher hs@denx.de
This looks like useful stuff to have, but I'd prefer that you put the check logic in board_eth_init() rather than adding to the individual device drivers. I know the 8260 SCC driver is the older style, which precludes the use of board_eth_init, but I'll convert it if you're able to test.
Unfortunately, this approach won't work. First of all, the 82xx SCC driver is now initialized in cpu_eth_init(), which knows nothing about board-specific peculiarities like the PIGGY. Secondly, the HDLC driver for Keymile has to be initialized in board_eth_init(), and it has nothing to do with the PIGGY. Putting the check in board_eth_init() would break it completely. I looked at Heiko's latest patch and couldn't figure out a way to cleanly differentiate between initializing the HDLC driver and checking whether the PIGGY was present fo the other ENET drivers.
I think you're missing something. You can put whatever logic you want in board_eth_init(), *including* calling cpu_eth_init(). As long as board_eth_init() returns >= 0, cpu_eth_init() will never get called by eth_initialize(). When I designed this, my intention (although not well communicated) was that board_eth_init() would always return >= 0, and that the -1 return code would only be used by the weak function in net/eth.c There's pretty serious flexibility here and I urge you to look again.
regards, Ben
participants (4)
-
Ben Warren
-
Gary Jennejohn
-
Heiko Schocher
-
Heiko Schocher