[U-Boot] [PATCH 2/2] IOMUX: Add console multiplexing support.

[U-Boot] [PATCH 2/2] IOMUX: Add console multiplexing support.

[Gary Jennejohn]

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
-- 
1.5.4.3


---
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 at denx.de
*********************************************************************

[U-Boot] [PATCH 2/2] IOMUX: Add console multiplexing support.

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
A Puritan is someone who is deathly afraid that  someone,  somewhere,
is having fun.

[U-Boot] [PATCH 2/2] IOMUX: Add console multiplexing support.

[Gary Jennejohn]

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:
> 
> 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?
> 

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
> > +
> 
> 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?
> 

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 at denx.de
*********************************************************************

[U-Boot] [PATCH 2/2] IOMUX: Add console multiplexing support.

[Wolfgang Denk]

Dear Gary Jennejohn,

In message <20080914191918.7de10c5c@peedub.jennejohn.org> you wrote:
> 
> > 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?
> 
> 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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Oblivion together does not frighten me, beloved.
	-- Thalassa (in Anne Mulhall's body), "Return to Tomorrow",
	   stardate 4770.3.

[U-Boot] [PATCH 2/2] IOMUX: Add console multiplexing support.

[Gary Jennejohn]

On Sun, 14 Sep 2008 20:34:27 +0200
Wolfgang Denk <wd@denx.de> wrote:

> In message <20080914191918.7de10c5c@peedub.jennejohn.org> you wrote:
> > 
> > > 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?
> > 
> > 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 at denx.de
*********************************************************************

[U-Boot] [PATCH 2/2] IOMUX: Add console multiplexing support.

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Gravitation cannot be held responsible for people falling in  love."
- Albert Einstein

[U-Boot] [PATCH 2/2 V2] IOMUX: Add console multiplexing support.

[Gary Jennejohn]

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
-- 
1.5.4.3

---
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 at denx.de
*********************************************************************

[U-Boot] [PATCH 2/2 V2] IOMUX: Add console multiplexing support.

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Work 8 hours, sleep 8 hours; but not the same 8 hours.

[U-Boot] [PATCH 2/2 V2] IOMUX: Add console multiplexing support.

[Gary Jennejohn]

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 at denx.de
*********************************************************************

[U-Boot] [PATCH 2/2 V2] IOMUX: Add console multiplexing support.

[Gary Jennejohn]

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 at denx.de
*********************************************************************

[U-Boot] [PATCH 2/2 V2] IOMUX: Add console multiplexing support.

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
No problem is insoluble.
	-- Dr. Janet Wallace, "The Deadly Years", stardate 3479.4

[U-Boot] [PATCH 2/2 V2] IOMUX: Add console multiplexing support.

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
When all is said and done, more is said than done.

[U-Boot] [PATCH 2/2 V2] IOMUX: Add console multiplexing support.

[Ben Warren]

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

[U-Boot] [PATCH 2/2 V2] IOMUX: Add console multiplexing support.

[Gary Jennejohn]

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 at denx.de
*********************************************************************

[U-Boot] [PATCH 2/2 V2] IOMUX: Add console multiplexing support.

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Consistency requires you to be as ignorant today as you were a  year
ago."                                              - Bernard Berenson

[U-Boot] [PATCH 2/2 V2] IOMUX: Add console multiplexing support.

[Gary Jennejohn]

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 at denx.de
*********************************************************************