[U-Boot] [PATCH 2/2] IOMUX: Add console multiplexing support.
Since this patch touches net/eth.c it is being sent separately.
When CONFIG_IO_MUX, CONFIG_NETCONSOLE and CFG_CONSOLE_IS_IN_ENV are all defined together it is possible that nc (netconsole) is defined as an output device. In this case it is necessary to set GD_FLG_DEVINIT after the network devices have been initialized, otherwise u-boot might try to send output to a device before it is ready, which leads to various errors.
Signed-off-by: Gary Jennejohn garyj@denx.de --- net/eth.c | 24 ++++++++++++++++++++++++ 1 files changed, 24 insertions(+), 0 deletions(-)
diff --git a/net/eth.c b/net/eth.c index 432dd60..94b6e3a 100644 --- a/net/eth.c +++ b/net/eth.c @@ -26,6 +26,11 @@ #include <net.h> #include <miiphy.h>
+#if defined(CONFIG_IO_MUX) && defined(CONFIG_NETCONSOLE) && \ + defined(CFG_CONSOLE_IS_IN_ENV) +DECLARE_GLOBAL_DATA_PTR; +#endif + #if defined(CONFIG_CMD_NET) && defined(CONFIG_NET_MULTI)
/* @@ -256,6 +261,15 @@ int eth_initialize(bd_t *bis) putc ('\n'); }
+#if defined(CONFIG_IO_MUX) && defined(CONFIG_NETCONSOLE) && \ + defined(CFG_CONSOLE_IS_IN_ENV) + /* + * Must do this very late because a network device may be set as a + * console at boot time. + */ + gd->flags |= GD_FLG_DEVINIT; /* device initialization completed */ +#endif + return eth_number; }
@@ -532,6 +546,16 @@ int eth_initialize(bd_t *bis) #if defined(CONFIG_DRIVER_TI_EMAC) davinci_eth_miiphy_initialize(bis); #endif + +#if defined(CONFIG_IO_MUX) && defined(CONFIG_NETCONSOLE) && \ + defined(CFG_CONSOLE_IS_IN_ENV) + /* + * Must do this very late because a network device may be set as a + * console at boot time. + */ + gd->flags |= GD_FLG_DEVINIT; /* device initialization completed */ +#endif + return 0; } #endif
Dear Gary Jennejohn,
In message 20080914164530.0a59c6ca@peedub.jennejohn.org you wrote:
Since this patch touches net/eth.c it is being sent separately.
When CONFIG_IO_MUX, CONFIG_NETCONSOLE and CFG_CONSOLE_IS_IN_ENV are all defined together it is possible that nc (netconsole) is defined as an output device. In this case it is necessary to set GD_FLG_DEVINIT after the network devices have been initialized, otherwise u-boot might try to send output to a device before it is ready, which leads to various errors.
I don't understand this patch for two reasons:
1) What has CONFIG_IO_MUX to do with that? We can have a netconsole defined with or without that new feature, so why would it make any difference?
Signed-off-by: Gary Jennejohn garyj@denx.de
net/eth.c | 24 ++++++++++++++++++++++++ 1 files changed, 24 insertions(+), 0 deletions(-)
diff --git a/net/eth.c b/net/eth.c index 432dd60..94b6e3a 100644 --- a/net/eth.c +++ b/net/eth.c @@ -26,6 +26,11 @@ #include <net.h> #include <miiphy.h>
+#if defined(CONFIG_IO_MUX) && defined(CONFIG_NETCONSOLE) && \
- defined(CFG_CONSOLE_IS_IN_ENV)
+DECLARE_GLOBAL_DATA_PTR; +#endif
#if defined(CONFIG_CMD_NET) && defined(CONFIG_NET_MULTI)
/* @@ -256,6 +261,15 @@ int eth_initialize(bd_t *bis) putc ('\n'); }
+#if defined(CONFIG_IO_MUX) && defined(CONFIG_NETCONSOLE) && \
- defined(CFG_CONSOLE_IS_IN_ENV)
- /*
* Must do this very late because a network device may be set as a* console at boot time.*/- gd->flags |= GD_FLG_DEVINIT; /* device initialization completed */
+#endif
- return eth_number;
}
@@ -532,6 +546,16 @@ int eth_initialize(bd_t *bis) #if defined(CONFIG_DRIVER_TI_EMAC) davinci_eth_miiphy_initialize(bis); #endif
+#if defined(CONFIG_IO_MUX) && defined(CONFIG_NETCONSOLE) && \
- defined(CFG_CONSOLE_IS_IN_ENV)
- /*
* Must do this very late because a network device may be set as a* console at boot time.*/- gd->flags |= GD_FLG_DEVINIT; /* device initialization completed */
+#endif
2) You only add new points where the GD_FLG_DEVINIT bit gets set in gd->flags. That means there are two possibilities when your newly added code is run: either, this bit is already set by other parts of the codem than the operation was redundant and couldbe omitted; or, the flasg was not set yet, then you set it now, which means you set it EARLIE than it would have been set before.
But your comment suggests that that would be done LATER now.
So what exactly is the purpose of this patch?
Best regards,
Wolfgang Denk
On Sun, 14 Sep 2008 18:07:42 +0200 Wolfgang Denk wd@denx.de wrote:
In message 20080914164530.0a59c6ca@peedub.jennejohn.org you wrote:
Since this patch touches net/eth.c it is being sent separately.
When CONFIG_IO_MUX, CONFIG_NETCONSOLE and CFG_CONSOLE_IS_IN_ENV are all defined together it is possible that nc (netconsole) is defined as an output device. In this case it is necessary to set GD_FLG_DEVINIT after the network devices have been initialized, otherwise u-boot might try to send output to a device before it is ready, which leads to various errors.
I don't understand this patch for two reasons:
- What has CONFIG_IO_MUX to do with that? We can have a netconsole
defined with or without that new feature, so why would it make any difference?
This is a valid comment. I had IOMUX on the brain when I wrote this code since I had to make this change in the course of developing it. This patch would still be valid without CONFIG_IO_MUX.
Signed-off-by: Gary Jennejohn garyj@denx.de
net/eth.c | 24 ++++++++++++++++++++++++ 1 files changed, 24 insertions(+), 0 deletions(-)
diff --git a/net/eth.c b/net/eth.c index 432dd60..94b6e3a 100644 --- a/net/eth.c +++ b/net/eth.c @@ -26,6 +26,11 @@ #include <net.h> #include <miiphy.h>
+#if defined(CONFIG_IO_MUX) && defined(CONFIG_NETCONSOLE) && \
- defined(CFG_CONSOLE_IS_IN_ENV)
+DECLARE_GLOBAL_DATA_PTR; +#endif
#if defined(CONFIG_CMD_NET) && defined(CONFIG_NET_MULTI)
/* @@ -256,6 +261,15 @@ int eth_initialize(bd_t *bis) putc ('\n'); }
+#if defined(CONFIG_IO_MUX) && defined(CONFIG_NETCONSOLE) && \
- defined(CFG_CONSOLE_IS_IN_ENV)
- /*
* Must do this very late because a network device may be set as a* console at boot time.*/- gd->flags |= GD_FLG_DEVINIT; /* device initialization completed */
+#endif
- return eth_number;
}
@@ -532,6 +546,16 @@ int eth_initialize(bd_t *bis) #if defined(CONFIG_DRIVER_TI_EMAC) davinci_eth_miiphy_initialize(bis); #endif
+#if defined(CONFIG_IO_MUX) && defined(CONFIG_NETCONSOLE) && \
- defined(CFG_CONSOLE_IS_IN_ENV)
- /*
* Must do this very late because a network device may be set as a* console at boot time.*/- gd->flags |= GD_FLG_DEVINIT; /* device initialization completed */
+#endif
- You only add new points where the GD_FLG_DEVINIT bit gets set in
gd->flags. That means there are two possibilities when your newly added code is run: either, this bit is already set by other parts of the codem than the operation was redundant and couldbe omitted; or, the flasg was not set yet, then you set it now, which means you set it EARLIE than it would have been set before.
But your comment suggests that that would be done LATER now.
So what exactly is the purpose of this patch?
This bit is normally ser in console_init_r, which is called very early.
You have to look at this patch in conjunction with the first patch and not as a separate entity, which it most definitely is not.
--- 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 *********************************************************************
Dear Gary Jennejohn,
In message 20080914191918.7de10c5c@peedub.jennejohn.org you wrote:
- You only add new points where the GD_FLG_DEVINIT bit gets set in
gd->flags. That means there are two possibilities when your newly added code is run: either, this bit is already set by other parts of the codem than the operation was redundant and couldbe omitted; or, the flasg was not set yet, then you set it now, which means you set it EARLIE than it would have been set before.
But your comment suggests that that would be done LATER now.
So what exactly is the purpose of this patch?
This bit is normally ser in console_init_r, which is called very early.
You have to look at this patch in conjunction with the first patch and not as a separate entity, which it most definitely is not.
Sorry, but this doesn't work. If you split patches, you have to do it in an orthogoanl way, such that each patch on it's own makes sense. This patch doesn't make any sense as is. Maybe theree are parts missing that may be buried somewhere in some other patch, but please do not expect that we will try to find them.
Please re-split patches such that they are independent of each other (except maybe that one has to be applied first), and make sure that each patch is complete in itself.
Thanks.
Best regards,
Wolfgang Denk
On Sun, 14 Sep 2008 20:34:27 +0200 Wolfgang Denk wd@denx.de wrote:
In message 20080914191918.7de10c5c@peedub.jennejohn.org you wrote:
- You only add new points where the GD_FLG_DEVINIT bit gets set in
gd->flags. That means there are two possibilities when your newly added code is run: either, this bit is already set by other parts of the codem than the operation was redundant and couldbe omitted; or, the flasg was not set yet, then you set it now, which means you set it EARLIE than it would have been set before.
But your comment suggests that that would be done LATER now.
So what exactly is the purpose of this patch?
This bit is normally ser in console_init_r, which is called very early.
You have to look at this patch in conjunction with the first patch and not as a separate entity, which it most definitely is not.
Sorry, but this doesn't work. If you split patches, you have to do it in an orthogoanl way, such that each patch on it's own makes sense. This patch doesn't make any sense as is. Maybe theree are parts missing that may be buried somewhere in some other patch, but please do not expect that we will try to find them.
Please re-split patches such that they are independent of each other (except maybe that one has to be applied first), and make sure that each patch is complete in itself.
I did it this way because I didn't want to send the net custodian an unnecessary patch. I though that was the way patches were supposed to be handled. I know I've had complaints from custodians in the past about this.
A consistent policy certainly would be nice.
--- 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 *********************************************************************
Dear Gary Jennejohn,
In message 20080915104647.79c73006@peedub.jennejohn.org you wrote:
Sorry, but this doesn't work. If you split patches, you have to do it in an orthogoanl way, such that each patch on it's own makes sense.
That should read "orthogonal", of course.
This patch doesn't make any sense as is. Maybe theree are parts missing that may be buried somewhere in some other patch, but please do not expect that we will try to find them.
Please read this again. Patches must be self-contained. You cannot submit a patch which contains only one half of the modification, while some other important parts are in some other, unrelated patch.
Please re-split patches such that they are independent of each other (except maybe that one has to be applied first), and make sure that each patch is complete in itself.
I did it this way because I didn't want to send the net custodian an unnecessary patch. I though that was the way patches were supposed to be handled. I know I've had complaints from custodians in the past about this.
A consistent policy certainly would be nice.
The policiy is clear and consistent:
Patches should always contain exactly one complete logical change, i. e. * Changes that contain different, unrelated modifications shall be submitted as separate patches, one patch per changeset. * If one logical set of modifications affects or creates several files, all these changes shall be submitted in a single patch.
See http://www.denx.de/wiki/U-Boot/Patches
It's bullet 2 that applies here.
Best regards,
Wolfgang Denk
When both CONFIG_SYS_CONSOLE_IS_IN_ENV and CONFIG_NETCONSOLE are defined the user can have stdout set to nc (netconsole).
This causes problems because u-boot will try to write to nc as soon as GD_FLG_DEVINIT is set in gd->flags, which happens before the network devices are initialized in net/eth.c. This results in error messages being spewed out.
To prevent this problem set GD_FLG_DEVINIT in net/eth.c:eth_initialize(), after the network devices have been initialized, instead of in common/console.c:console_init_r().
Signed-off-by: Gary Jennejohn garyj@denx.de --- common/console.c | 11 +++++++++++ net/eth.c | 22 ++++++++++++++++++++++ 2 files changed, 33 insertions(+), 0 deletions(-)
diff --git a/common/console.c b/common/console.c index 6f0846f..e0d7541 100644 --- a/common/console.c +++ b/common/console.c @@ -447,7 +447,18 @@ int console_init_r (void) console_setfile (stdin, inputdev); }
+#ifdef CONFIG_NETCONSOLE + /* + * Must do this later in net/eth.c because a network device may + * be set as a console at boot. + * + * Note that the code is left here to make it clear that gd->flags + * would normally have been set at this point. + */ + gd->flags |= 0; +#else gd->flags |= GD_FLG_DEVINIT; /* device initialization completed */ +#endif /* CONFIG_NETCONSOLE */
#ifndef CONFIG_SYS_CONSOLE_INFO_QUIET /* Print information */ diff --git a/net/eth.c b/net/eth.c index ccd871a..6c00ff7 100644 --- a/net/eth.c +++ b/net/eth.c @@ -26,6 +26,10 @@ #include <net.h> #include <miiphy.h>
+#if defined(CONFIG_NETCONSOLE) && defined(CONFIG_SYS_CONSOLE_IS_IN_ENV) +DECLARE_GLOBAL_DATA_PTR; +#endif + #if defined(CONFIG_CMD_NET) && defined(CONFIG_NET_MULTI)
/* @@ -262,6 +266,15 @@ int eth_initialize(bd_t *bis) putc ('\n'); }
+#if defined(CONFIG_NETCONSOLE) && defined(CONFIG_SYS_CONSOLE_IS_IN_ENV) + /* + * Must do this late because a network device may be set as a + * console at boot. gd->flags is normally set quite early in + * console_init_r. + */ + gd->flags |= GD_FLG_DEVINIT; /* device initialization completed */ +#endif + return eth_number; }
@@ -538,6 +551,15 @@ int eth_initialize(bd_t *bis) #if defined(CONFIG_DRIVER_TI_EMAC) davinci_eth_miiphy_initialize(bis); #endif +#if defined(CONFIG_NETCONSOLE) && defined(CONFIG_SYS_CONSOLE_IS_IN_ENV) + /* + * Must do this late because a network device may be set as a + * console at boot. gd->flags is normally set quite early in + * console_init_r. + */ + gd->flags |= GD_FLG_DEVINIT; /* device initialization completed */ +#endif + return 0; } #endif
Dear Gary Jennejohn,
Is there another part of the patch, part 1/2, too?
In message 20081020135849.371fe4d1@ernst.jennejohn.org you wrote:
When both CONFIG_SYS_CONSOLE_IS_IN_ENV and CONFIG_NETCONSOLE are defined the user can have stdout set to nc (netconsole).
This causes problems because u-boot will try to write to nc as soon as GD_FLG_DEVINIT is set in gd->flags, which happens before the network devices are initialized in net/eth.c. This results in error messages being spewed out.
It seems this can happen only if CONFIG_SYS_CONSOLE_IS_IN_ENV is defined, right?
To prevent this problem set GD_FLG_DEVINIT in net/eth.c:eth_initialize(), after the network devices have been initialized, instead of in common/console.c:console_init_r().
I have to admit that I don't like the idea of splitting the GD_FLG_DEVINIT into several, unrelated parts of the code.
Would it not make more sense to have the netconsole part wait with output until it's been initialized? And/or move the netweork init to an earlier point, when netconsole is enabled?
Best regards,
Wolfgang Denk
On Mon, 20 Oct 2008 15:24:53 +0200 Wolfgang Denk wd@denx.de wrote:
Dear Wolfgang Denk,
Is there another part of the patch, part 1/2, too?
Yes, but I did this part first because it's small and easily generated. Since it also affects net I wanted to get it to the custodian. The other part adds the console multiplexing and isn't directly related to this. Note this is V2 of the patch and the original version also depended on CONFIG_IO_MUX, which we decided in this ML wasn't really relevant.
In message 20081020135849.371fe4d1@ernst.jennejohn.org you wrote:
When both CONFIG_SYS_CONSOLE_IS_IN_ENV and CONFIG_NETCONSOLE are defined the user can have stdout set to nc (netconsole).
This causes problems because u-boot will try to write to nc as soon as GD_FLG_DEVINIT is set in gd->flags, which happens before the network devices are initialized in net/eth.c. This results in error messages being spewed out.
It seems this can happen only if CONFIG_SYS_CONSOLE_IS_IN_ENV is defined, right?
Correct, as I stated in the comment. Note that it isn't evident in the patch, but the console_init_r() which I changed is #ifdef CONFIG_SYS_CONSOLE_IS_IN_ENV (there's another version in the #else-branch which isn't).
To prevent this problem set GD_FLG_DEVINIT in net/eth.c:eth_initialize(), after the network devices have been initialized, instead of in common/console.c:console_init_r().
I have to admit that I don't like the idea of splitting the GD_FLG_DEVINIT into several, unrelated parts of the code.
I don't like it too much myself, but it seemed like the logical approach to me at the time I made this modification.
Would it not make more sense to have the netconsole part wait with output until it's been initialized? And/or move the netweork init to an earlier point, when netconsole is enabled?
Not a bad idea. I think it would be most logical to do it in the netconsole code, rather than moving up the network initialization.
I'll take a look at that.
--- 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 *********************************************************************
On Mon, 20 Oct 2008 15:57:32 +0200 Gary Jennejohn garyj@denx.de wrote:
On Mon, 20 Oct 2008 15:24:53 +0200 Wolfgang Denk wd@denx.de wrote:
I have to admit that I don't like the idea of splitting the GD_FLG_DEVINIT into several, unrelated parts of the code.
I don't like it too much myself, but it seemed like the logical approach to me at the time I made this modification.
Would it not make more sense to have the netconsole part wait with output until it's been initialized? And/or move the netweork init to an earlier point, when netconsole is enabled?
Not a bad idea. I think it would be most logical to do it in the netconsole code, rather than moving up the network initialization.
I'll take a look at that.
I looked at this some more. eth_initialize() is called in every architecture-specific library which means changing 8 files to move it up to before the initialization of netconsole.
netconsole is intialized in devices_init() which is called even before console_init_r() and long before eth_initialize().
What's the opinion of the ML? Is it worth all the repository churn to move eth_initialize() up rather than changing just two files?
One alrernative which occurs to me would be to introduce a new flag GD_FLG_ETHINIT, but this is even worse because it requires modifying 12 header files and 3 or more C files.
--- 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 *********************************************************************
Dear Gary Jennejohn,
In message 20081020182635.4ed9ca48@ernst.jennejohn.org you wrote:
I looked at this some more. eth_initialize() is called in every architecture-specific library which means changing 8 files to move it up to before the initialization of netconsole.
netconsole is intialized in devices_init() which is called even before console_init_r() and long before eth_initialize().
Well, maybe there is a less intrusive approach to solve the problem.
One alrernative which occurs to me would be to introduce a new flag GD_FLG_ETHINIT, but this is even worse because it requires modifying 12 header files and 3 or more C files.
Hm... let's sum up what exactly the problem is; you wrote:
| This causes problems because u-boot will try to write to nc as soon | as GD_FLG_DEVINIT is set in gd->flags, which happens before the | network devices are initialized in net/eth.c. This results in error | messages being spewed out.
I read this that what we actually want to do is stopping NC to transmit too early. Correct?
Well, nc_send_packet() (see "drivers/net/netconsole.c") can be easily shortcut if we find a way to make eth_get_dev() return NULL.
And eth_get_dev() (see "net/eth.c") just returns "eth_current".
So maybe there is a way to make sure "eth_current" is set to NULL until it's OK for netconsole to transmit?
Maybe, maybe eventually the real cause of your problems is that "eth_current" is not read as NULL while running from flash? Maybe changing the declaration in "net/eth.c" into something like this would help?
static struct eth_device *eth_current __attribute__ ((section(".data"))) = NULL;
What do you think?
Best regards,
Wolfgang Denk
Wolfgang Denk wrote:
Dear Gary Jennejohn,
In message 20081020182635.4ed9ca48@ernst.jennejohn.org you wrote:
I looked at this some more. eth_initialize() is called in every architecture-specific library which means changing 8 files to move it up to before the initialization of netconsole.
netconsole is intialized in devices_init() which is called even before console_init_r() and long before eth_initialize().
Well, maybe there is a less intrusive approach to solve the problem.
One alrernative which occurs to me would be to introduce a new flag GD_FLG_ETHINIT, but this is even worse because it requires modifying 12 header files and 3 or more C files.
Hm... let's sum up what exactly the problem is; you wrote:
| This causes problems because u-boot will try to write to nc as soon | as GD_FLG_DEVINIT is set in gd->flags, which happens before the | network devices are initialized in net/eth.c. This results in error | messages being spewed out.
I read this that what we actually want to do is stopping NC to transmit too early. Correct?
Well, nc_send_packet() (see "drivers/net/netconsole.c") can be easily shortcut if we find a way to make eth_get_dev() return NULL.
And eth_get_dev() (see "net/eth.c") just returns "eth_current".
So maybe there is a way to make sure "eth_current" is set to NULL until it's OK for netconsole to transmit?
Maybe, maybe eventually the real cause of your problems is that "eth_current" is not read as NULL while running from flash? Maybe changing the declaration in "net/eth.c" into something like this would help?
static struct eth_device *eth_current __attribute__ ((section(".data"))) = NULL;
What do you think?
This looks like a good idea. eth_current is a variable that should definitely be in a known state.
Best regards,
Wolfgang Denk
regards, Ben
On Mon, 20 Oct 2008 13:13:32 -0700 Ben Warren biggerbadderben@gmail.com wrote:
Wolfgang Denk wrote:
Dear Gary Jennejohn,
In message 20081020182635.4ed9ca48@ernst.jennejohn.org you wrote:
I looked at this some more. eth_initialize() is called in every architecture-specific library which means changing 8 files to move it up to before the initialization of netconsole.
netconsole is intialized in devices_init() which is called even before console_init_r() and long before eth_initialize().
Well, maybe there is a less intrusive approach to solve the problem.
One alrernative which occurs to me would be to introduce a new flag GD_FLG_ETHINIT, but this is even worse because it requires modifying 12 header files and 3 or more C files.
Hm... let's sum up what exactly the problem is; you wrote:
| This causes problems because u-boot will try to write to nc as soon | as GD_FLG_DEVINIT is set in gd->flags, which happens before the | network devices are initialized in net/eth.c. This results in error | messages being spewed out.
I read this that what we actually want to do is stopping NC to transmit too early. Correct?
Well, nc_send_packet() (see "drivers/net/netconsole.c") can be easily shortcut if we find a way to make eth_get_dev() return NULL.
And eth_get_dev() (see "net/eth.c") just returns "eth_current".
So maybe there is a way to make sure "eth_current" is set to NULL until it's OK for netconsole to transmit?
Maybe, maybe eventually the real cause of your problems is that "eth_current" is not read as NULL while running from flash? Maybe changing the declaration in "net/eth.c" into something like this would help?
static struct eth_device *eth_current __attribute__ ((section(".data"))) = NULL;
What do you think?
This looks like a good idea. eth_current is a variable that should definitely be in a known state.
By the time the problem arises we're already running out of RAM.
I think the real problem is that console_init_r() is called too early. If it were called *after* eth_initialize() then there would be no difficulties.
AFAICT moving console_init_r() shouldn't present any problems, since until its invocation U-Boot will simply use the serial interface. I can't imagine that waiting a few milliseconds longer to switch to whatever console the user has set in the environment would present a problem.
This has the following advantages a) nc has been initialized and can be used as a console, if so desired b) any hardware initializations, such as SPI or I2C, which may be required to initialize an ethernet device have already happened c) the ethernet device is already initialized and ready to go d) no need to split up setting gd->flags
What do you think about this idea? It requires changing a lot of files but seems like the cleanest approach to me.
It isn't clear to me whether the architecture-specific libs have custodians. Does anyone know?
--- 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 *********************************************************************
Dear Gary Jennejohn,
In message 20081021114543.1c44ff03@ernst.jennejohn.org you wrote:
By the time the problem arises we're already running out of RAM.
So maybe you can describe exactly what happens, and when?
I think the real problem is that console_init_r() is called too early. If it were called *after* eth_initialize() then there would be no difficulties.
AFAICT moving console_init_r() shouldn't present any problems, since until its invocation U-Boot will simply use the serial interface.
I am not so sure. Moving things around not as trivial as it may seem, as we have all kinds of configurations with LCD display, modem support etc.
I can't imagine that waiting a few milliseconds longer to switch to whatever console the user has set in the environment would present a problem.
It's not the millisecondsa, but the sequence that may matter.
This has the following advantages a) nc has been initialized and can be used as a console, if so desired b) any hardware initializations, such as SPI or I2C, which may be required to initialize an ethernet device have already happened c) the ethernet device is already initialized and ready to go d) no need to split up setting gd->flags
And it has the disadvantage that you have to re-test on some 500+ boards if anything breaks.
It isn't clear to me whether the architecture-specific libs have custodians. Does anyone know?
What is not clear about that? Do we have custodians for architecture specific files? Yes, we do. So whay do you doubt that the libs fall into their responsibility, too?
Best regards,
Wolfgang Denk
On Tue, 21 Oct 2008 12:34:30 +0200 Wolfgang Denk wd@denx.de wrote:
In message 20081021114543.1c44ff03@ernst.jennejohn.org you wrote:
By the time the problem arises we're already running out of RAM.
So maybe you can describe exactly what happens, and when?
I think the real problem is that console_init_r() is called too early. If it were called *after* eth_initialize() then there would be no difficulties.
AFAICT moving console_init_r() shouldn't present any problems, since until its invocation U-Boot will simply use the serial interface.
I am not so sure. Moving things around not as trivial as it may seem, as we have all kinds of configurations with LCD display, modem support etc.
All console_init_r() does is to search for devices which can be used for STDIN/STDOUT/STDERR (whether in the environment or the device list), call console_setfile() and then print out the list of devices found. console_setfile() actually calls the device start routines. The actual registration of the devices happens in device_init() which is called before console_init_r().
So I see no reason why moving console_init_r() would have any effect.
It isn't clear to me whether the architecture-specific libs have custodians. Does anyone know?
What is not clear about that? Do we have custodians for architecture specific files? Yes, we do. So whay do you doubt that the libs fall into their responsibility, too?
Did I write that I doubt anything? No. I just asked for some clarification because it wasn't clear to me from the very abbreviated descriptions on the Custodian page just exactly what is within the purview of a custodian.
--- 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 *********************************************************************
Dear Gary Jennejohn,
In message 20081020155732.2653f2d7@ernst.jennejohn.org you wrote:
Yes, but I did this part first because it's small and easily generated. Since it also affects net I wanted to get it to the custodian. The other part adds the console multiplexing and isn't directly related to this.
If the other patch is unrelated, you should not number thse patches so they look as if they ware two parts that belong together, and in a certain sequence.
Normally I would not even read the 2/2 part before 1/2 was posted.
Not a bad idea. I think it would be most logical to do it in the netconsole code, rather than moving up the network initialization.
I'll take a look at that.
Mee too.
Best regards,
Wolfgang Denk
participants (3)
-
Ben Warren -
Gary Jennejohn -
Wolfgang Denk