[PATCH] 8xx_io/uart.c

[PATCH] 8xx_io/uart.c

[Joakim Tjernlund]

Hi

Found a bug in the serial driver. my_console_write() uses the wrong address in early console writes.
Removed some warnings as well.

Please can you change TX_NUM_FIFO to 8 and TX_BUF_SIZE to 96,
since chars are lost when pasting text into the console otherwise.

   Jocke

Index: arch/ppc/8xx_io/uart.c
===================================================================
RCS file: /home/cvsadmin/cvsroot/kernel/linuxppc/arch/ppc/8xx_io/uart.c,v
retrieving revision 1.3
diff -u -r1.3 uart.c
--- arch/ppc/8xx_io/uart.c	21 Nov 2002 15:16:27 -0000	1.3
+++ arch/ppc/8xx_io/uart.c	7 Feb 2003 13:23:23 -0000
@@ -2309,7 +2309,10 @@
 		/* if a LF, also do CR... */
 		if (*s == 10) {
 			while (bdp->cbd_sc & BD_SC_READY);
-			cp = info->tx_va_base + ((bdp - info->tx_bd_base) * TX_BUF_SIZE);
+			if ((uint)(bdp->cbd_bufaddr) > (uint)IMAP_ADDR)
+				cp = (u_char *)(bdp->cbd_bufaddr);
+			else
+				cp = info->tx_va_base + ((bdp - info->tx_bd_base) * TX_BUF_SIZE);
 			*cp = 13;
 			bdp->cbd_datlen = 1;
 			bdp->cbd_sc |= BD_SC_READY;
@@ -3000,10 +3003,10 @@
 	bdp->cbd_bufaddr = iopa(mem_addr);
 	(bdp+1)->cbd_bufaddr = iopa(mem_addr+4);

-	consinfo.rx_va_base = mem_addr;
-	consinfo.rx_bd_base = bdp;
-	consinfo.tx_va_base = mem_addr + 4;
-	consinfo.tx_bd_base = bdp+1;
+	consinfo.rx_va_base = (unsigned char *)mem_addr;
+	consinfo.rx_bd_base = (cbd_t *)bdp;
+	consinfo.tx_va_base = (unsigned char *)(mem_addr + 4);
+	consinfo.tx_bd_base = (cbd_t *)(bdp+1);

 	/* For the receive, set empty and wrap.
 	 * For transmit, set wrap.


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: [PATCH] 8xx_io/uart.c

[Dan Malek]

Joakim Tjernlund wrote:

> Found a bug in the serial driver. my_console_write() uses the wrong address in early console writes.
> Removed some warnings as well.

Well......console write doesn't get called this early.  The purpose
of these address modifications in other parts of the driver are for
kgdb using the serial port for early debugging.

I'd really like some supporting documentation (like a kernel panic
or other reproducable error) before you declare something a "bug",
along with showing the same test fixed the problem.

> Please can you change TX_NUM_FIFO to 8 and TX_BUF_SIZE to 96,
> since chars are lost when pasting text into the console otherwise.

What makes 96 a big enough number?  If you want to do this locally,
that's fine, but only flow control can guarantee you won't overflow
a serial port of any fifo depth.  I don't want to arbitrarily make
this fifo so large as there are other processing/latency/memory
tradeoffs.  The SMC is not a high performance interface and it
consumes lots of CPM cycles.

Thanks.


	-- Dan


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

RE: [PATCH] 8xx_io/uart.c

[Joakim Tjernlund]

> Joakim Tjernlund wrote:
>
> > Found a bug in the serial driver. my_console_write() uses the wrong address in early console writes.
> > Removed some warnings as well.
>
> Well......console write doesn't get called this early.  The purpose
> of these address modifications in other parts of the driver are for
> kgdb using the serial port for early debugging.

Early printk's will call it too.

>
> I'd really like some supporting documentation (like a kernel panic
> or other reproducable error) before you declare something a "bug",
> along with showing the same test fixed the problem.

I can not reproduce an error. 3-4 times I experienced a hang after "calibrating delay loop ..."
which I can't explain.

Now, why do you do the same thing just a few lines up if it wasn't needed?

How about checking your own facts and provide an accurate analysis before
bitching at me for trying to help out?

>
> > Please can you change TX_NUM_FIFO to 8 and TX_BUF_SIZE to 96,
> > since chars are lost when pasting text into the console otherwise.
>
> What makes 96 a big enough number?  If you want to do this locally,
> that's fine, but only flow control can guarantee you won't overflow
> a serial port of any fifo depth.  I don't want to arbitrarily make
> this fifo so large as there are other processing/latency/memory
> tradeoffs.  The SMC is not a high performance interface and it
> consumes lots of CPM cycles.

I tested my how much I had increase them until I didn't lose any chars.
I don't think this has anything to do with HW flow control. The
smc runs out of BD's when trying to echo back the chars.

I don't see how this would consume more CPM cycles. Yes, it will use
a little more dpram but there is room for it.

 Jocke


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: [PATCH] 8xx_io/uart.c

[Dan Malek]

Joakim Tjernlund wrote:

> Early printk's will call it too.

They shouldn't.  All printk's prior to serial device initialization
are queued in the log buffer.  After the serial device is initialized,
the debug console is configured and only at that time will the printk
information flow through the driver.  Anything queued up to that point
is blasted out the driver.  There are lots of console drivers that
will fail if Linux chooses to change this behavior.

> Now, why do you do the same thing just a few lines up if it wasn't needed?

It is needed there.  These same functions support xmon and kgdb.  In order
to do that the serial port is used in three different phases.  One, as it
was configured by the boot rom/loader, two, by an early serial initialization
before general memory management and buffering is available, and finally
after the driver has been completely initialized.  None of this is needed
to simply support the Linux console.  This has all been discussed too many
times before....

> How about checking your own facts and provide an accurate analysis before
> bitching at me for trying to help out?

All I asked is you somehow show it is broken, and that something has
been fixed before we start patching things.  If there is really something
wrong with it, I would like to know what that is and how it is triggered.

> I tested my how much I had increase them until I didn't lose any chars.
> I don't think this has anything to do with HW flow control. The
> smc runs out of BD's when trying to echo back the chars.

Then, someone upstream isn't watching return values from these functions
when the driver indicates there isn't space to output the characters.
It isn't the responsibility of the serial driver to provide sufficient
queueing for an arbitrary output request.  The depth of the fifo on the 8xx
is much deeper than most silicon uarts, and if that isn't enough I suspect
something outside of this driver is amiss (or interfaces have changed and
this driver wasn't updated).

Thanks.


	-- Dan


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: [PATCH] 8xx_io/uart.c

[Joakim Tjernlund]

> Joakim Tjernlund wrote:
>
> > Early printk's will call it too.
>
> They shouldn't.  All printk's prior to serial device initialization
> are queued in the log buffer.  After the serial device is initialized,
> the debug console is configured and only at that time will the printk
> information flow through the driver.  Anything queued up to that point
> is blasted out the driver.  There are lots of console drivers that
> will fail if Linux chooses to change this behavior.

Still the first half of the printk's has the if expression true, then it switch
to false for the remaing one's and stayed there during boot up.
This is how I tested it:
remove the *cp = 13 line and add:
if ((uint)(bdp->cbd_bufaddr) > (uint)IMAP_ADDR){
   cp = (u_char *)(bdp->cbd_bufaddr);
   *cp = 64;
} else {
   cp = info->tx_va_base + ((bdp - info->tx_bd_base) * TX_BUF_SIZE);
  *cp = 65;
}
Then just watch the a normal boot.
>
> > Now, why do you do the same thing just a few lines up if it wasn't needed?
>
> It is needed there.  These same functions support xmon and kgdb.  In order
> to do that the serial port is used in three different phases.  One, as it
> was configured by the boot rom/loader, two, by an early serial initialization
> before general memory management and buffering is available, and finally
> after the driver has been completely initialized.  None of this is needed
> to simply support the Linux console.  This has all been discussed too many
> times before....

I don't buy that it is needed only in one place but not the other.
Either none of them is needed or both. After a little investigation it looks like
none of them is needed, but I need to test that first to be sure.

These comments are also in there:
/*
 * We need to gracefully shut down the transmitter, disable
 * interrupts, then send our bytes out.
 */
and
/*
 * Finally, Wait for transmitter & holding register to empty
 *  and restore the IER
 */
and yet there is no sign of trying to do any of that.
Once user space gets going won't printk's from the kernel risk confusing
the real serial driver since interrupts are active?
This driver is not very easy to understand.

Also, the last 'if(info)' is rendundant. 'info' will never be NULL.
>
> > How about checking your own facts and provide an accurate analysis before
> > bitching at me for trying to help out?
>
> All I asked is you somehow show it is broken, and that something has
> been fixed before we start patching things.  If there is really something
> wrong with it, I would like to know what that is and how it is triggered.

OK, I was a little harsh, but your answer wasn't helping me much

>
> > I tested my how much I had increase them until I didn't lose any chars.
> > I don't think this has anything to do with HW flow control. The
> > smc runs out of BD's when trying to echo back the chars.
>
> Then, someone upstream isn't watching return values from these functions
> when the driver indicates there isn't space to output the characters.
> It isn't the responsibility of the serial driver to provide sufficient
> queueing for an arbitrary output request.  The depth of the fifo on the 8xx
> is much deeper than most silicon uarts, and if that isn't enough I suspect
> something outside of this driver is amiss (or interfaces have changed and
> this driver wasn't updated).

Yes, Paul Ruhland has found a bug in drivers/char/n_tty.c that explains
some of the problems I have seen.

   Jocke
PS.
    Could you please also comment on the enet.c driver patch I sent.
>
> Thanks.
>
> -- Dan


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: [PATCH] 8xx_io/uart.c

[Murray Jensen]

On Sun, 09 Feb 2003 15:52:14 -0500, Dan Malek <dan@embeddededge.com> writes:
>> Now, why do you do the same thing just a few lines up if it wasn't needed?
>
>It is needed there.  These same functions support xmon and kgdb.  In order
>to do that the serial port is used in three different phases.  One, as it
>was configured by the boot rom/loader, two, by an early serial initialization
>before general memory management and buffering is available, and finally
>after the driver has been completely initialized.  None of this is needed
>to simply support the Linux console.  This has all been discussed too many
>times before....
>
>> How about checking your own facts and provide an accurate analysis before
>> bitching at me for trying to help out?
>
>All I asked is you somehow show it is broken, and that something has
>been fixed before we start patching things.  If there is really something
>wrong with it, I would like to know what that is and how it is triggered.

It is quite clearly broken - a quick perusal of the code will convince you
of this. The problem will be triggered whenever a character with decimal
value 10 is transmitted via my_console_write(), and the buffer address resides
in DPRAM. Either this is a bug, or the DPRAM test is redundant.

It appears that the code originally didn't take into account buffer addresses
that may be in DPRAM, and when it was updated to do so, whoever did it, updated
the first instance of the code, but not the second (which is intended to
convert <linefeed> into <linefeed><carriage-return> - which I reckon is back
to front anyway :-).

It may be that no consequences arise from possibly writing the number 13 into
a random memory location, but I for one would like to see this bug fixed. In
fact, I have a vague recollection that the same bug also existed in the 8260
version of this code, and that I submitted a fix for it among the 8260 uart
patches I posted some time ago.

Having just checked my version of both linuxppc_2_4_devel and linuxppc-2.5
I see this bug still exists everywhere (both 2.4-devel and 2.5, and both
8xx and 8260) - and it is indeed fixed in my local version of 2.4_devel
(which means I did post this fix).

This serves to highlight the difficulty of getting stuff into the "official"
linuxppc kernel sources, although it is only a minor case. Old-timers such
as myself tend to post our patches so they exist on the public record, and
then basically go back to our work when the patches are ignored, usually for
the most lame reasons, simply because we don't have the time to debate the
issue.

However, I believe linuxppc suffers because of this - especially since we are
usually talking about the "development" version of the kernel which people
should expect to be broken occasionally. Another thing to note here, is that
certain people and/or companies have no problem getting their stuff in.

I don't know what to do about this, which is probably why I haven't bothered
to say anything up until now, I just thought this comment followed on logically
from this case. I guess it simply means that if you want the best linuxppc
kernel, you can't simply clone the linuxppc trees - you have to then hunt
around the mailing lists and other places for fixes for various things. Some
people have been so frustrated in the past, that they have set up their own
source trees. I don't like this sort of thing, but then again maybe my apathy
is worse. Food for thought. Cheers!
								Murray...
--
Murray Jensen, CSIRO Manufacturing & Infra. Tech.      Phone: +61 3 9662 7763
Locked Bag No. 9, Preston, Vic, 3072, Australia.         Fax: +61 3 9662 7853
Internet: Murray.Jensen@csiro.au

Hymod project: http://www.msa.cmst.csiro.au/projects/Hymod/


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: [PATCH] 8xx_io/uart.c

[Dan Malek]

Murray Jensen wrote:

> It is quite clearly broken - ....


Pay attention....:-)

You can also read this over and over again in the archives........

First, these functions are used by more than kernel printk.  They
are shared with xmon and kgdb debugger functions.

Second, the kernel printk doesn't call this driver until the driver
has been initialized, which means it has allocated all of the "normal"
memory buffers for fifo and the CPM BD's.

The ONLY time these functions will find a buffer in DPRAM is when xmon
or kgdb is used.  If you are using kgdb or xmon, and find something wrong
with the buffer management, then there is a bug.......xmon and kgdb don't
generate buffer contents that necessitates this test.  If you add this
code it will never get executed.

 > ...a quick perusal of the code will convince you
> of this.

No it won't.  Find the case where this happens and then I'll be convinced :-)


> It appears that the code originally didn't take into account buffer addresses
> that may be in DPRAM, and when it was updated to do so, whoever did it, updated
> the first instance of the code,

I did it.  It was updated to support xmon and kgdb.  Only the modifications
necessary to support these functions were added. In addition to these driver
changes, there are assumptions and interactions with boot roms/loaders so
we could start kgdb very early in the debugging phase, long before the driver
was formally initialized.

> ... I for one would like to see this bug fixed.

Prove it's a bug, it will get fixed.

I will mildly apoligize, but you have to understand I get lots of patches
from people claiming "bugs", when it isn't one and they don't take the time
to understand why something was done or even how it works.  I'll add the
code, it won't have any effect, it that makes you happy.

> ..... In
> fact, I have a vague recollection that the same bug also existed in the 8260
> version of this code, and that I submitted a fix for it among the 8260 uart
> patches I posted some time ago.

I took the time to look for something from you in the past and didn't find
anything.  If it showed up, you would have received the same discussion.
The 8260 driver was just a copy of this one with only the necessary changes
to make it work on that processor.

> This serves to highlight the difficulty of getting stuff into the "official"
> linuxppc kernel sources,

The real difficulty arises from the time many of us have to spend proving
patches work correctly or that a real bug exists.  In this case, no one took
the time to prove it was a bug, which it isn't, or that it was even fixed.
The reason we have only a few people with the ability to actually update
the trees is we take the time to understand how all of this stuff works
and do our best to ensure we aren't creating more problems than we are solving.


For example, was someone supposed to blindly accept Jocke's patch to make
the serial port fifos so much larger?  I had to take the time (over and over) :-)
to explain why this wasn't a bug.  It would be nice if some of you went through
the learning curve to understand how things are supposed to work before you
just declare bugs and submit patches.

 > ...I don't know what to do about this, ...

The thing to do about it is for _you_ to invest the time in some engineering
practices and show that you really found a bug, you can reproduce the bug,
you implemented some feature enhancement, your modifications didn't introduce
other problems, the code considers effects on other platforms, and that
your update does what it claims.  When I have to do this, the priority of
these updates falls well below my own work.

As a final example, Jocke keeps bugging me about his ethernet updates.  I
explained (yet again, it was in the archives) that this had been attempted
in the past, and it caused problems with packet routing/forwarding.  Why
do I have to take the time to set up the testing environment and once again
prove this is still a problem?  I hoped to find someone with a test
environment to prove this (as they mentioned the bug in the first place),
but they are also busy with their own projects.  Someday I may have time
to set this up, make some changes to the patch so it works better, and
do some testing to ensure the upstream alignment changes are done properly.
If someone else can take the time to ensure it works (or make it work)
in this condition where it was known to fail in the past, I would be more
likely to check it in without taking the time to do the testing myself.

It's easy to hack up code, throw it over the fence, and complain when
someone else won't take care of it.  Make that job easier for those of
us trying to maintain the source trees, it would be appreciated.

Thanks.


	-- Dan


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: [PATCH] 8xx_io/uart.c

[Murray Jensen]

On Mon, 10 Feb 2003 13:52:04 -0500, Dan Malek <dan@embeddededge.com> writes:
>> ... I for one would like to see this bug fixed.
>
>Prove it's a bug, it will get fixed.

If you simply looked at the code you would see the problem we are talking
about. What are you suggesting? That kgdb and xmon will _NEVER_ send a
character with decimal value 10 via my_console_write()?

>I will mildly apoligize, but you have to understand I get lots of patches
>from people claiming "bugs", when it isn't one and they don't take the time
>to understand why something was done or even how it works.  I'll add the
>code, it won't have any effect, it that makes you happy.

I really don't care either way - I gave up worrying about this sort of thing
a long time ago. I do think you have the wrong attitude for the position you
have assumed for yourself in the linuxppc community - you are unnecessarily
aggressive, assuming that everyone is a moron until proven otherwise.

However, having said this, I very much appreciate the effort you (and others)
put into it - the positives outweigh the negatives. This is why I keep my mouth
shut (usually), and will keep my mouth shut in the future (after this message).
If I don't like it, I will put my effort where my mouth is - or go use a
kernel tree set up by someone else.

>I took the time to look for something from you in the past and didn't find
>anything.  If it showed up, you would have received the same discussion.

http://lists.linuxppc.org/linuxppc-embedded/200205/msg00268.html

You probably didn't find it because I compressed the patch. I guess this is
a good example of why one shouldn't compress patches sent to the list.

>The 8260 driver was just a copy of this one with only the necessary changes
>to make it work on that processor.

That much is clear to all who are working on linuxppc on the 8260, I'm sure.

>> This serves to highlight the difficulty of getting stuff into the "official"
>> linuxppc kernel sources,
>
>The real difficulty arises from the time many of us have to spend proving
>patches work correctly or that a real bug exists.  In this case, no one took
>the time to prove it was a bug, which it isn't, or that it was even fixed.

It didn't need proving by testing - it was a simple logic exercise. Either
the first test against IMAP_ADDR is redundant, or the same test is needed
in the second instance of the code in the case where the character being
transmitted is equal to 10 (decimal). Please can't you just view the piece
of code under discussion and see what we are talking about?

>The reason we have only a few people with the ability to actually update
>the trees is we take the time to understand how all of this stuff works
>and do our best to ensure we aren't creating more problems than we are solving.

Like I said, I think you have the wrong attitude - are you suggesting that
I am incapable of understanding this stuff? The question under discussion is
quite trivial and if you'd actually look at it instead of assuming I was a
moron, you'd see.

>For example, was someone supposed to blindly accept Jocke's patch to make
>the serial port fifos so much larger?  I had to take the time (over and over)
>:-)
>to explain why this wasn't a bug.

You are mis-representing what I said. I wasn't suggesting blind acceptance.
What I am suggesting is that the list be used to discuss patches, and if after
a certain amount of time and discussion there are not any *objections* to a
patch, it should make its way into the *development* tree. If it then breaks
something, it can be backed out (easy with bk), and the person who sent it
can resubmit it via the list at a later stage when they have fixed the
reason it was breaking things. The development tree is supposed to be used
this way. When after time there is no obvious problem with a patch, it can
make its way into the stable tree - this process must be handled more
carefully, of course - the stable tree *must* be stable.

And in fact, the system worked well in the case you cited - the discussion in
the list led Jocke to look elsewhere for the cause of his problem and the result
was a patch to drivers/char/n_tty.c (which must be submitted via the linux
kernel mailing list, since it is a patch to a non-ppc file). I am sure Jocke
appreciated your input, and the issue was resolved satisfactorily.

>It would be nice if some of you went through
>the learning curve to understand how things are supposed to work before you
>just declare bugs and submit patches.

I actually have done so. Dan Malek is not the only person in the world who
understands this stuff. Like I say, you have a bad attitude - but again, I am
not in a position to put in the effort required to displace you, so I will just
put up with it.

> > ...I don't know what to do about this, ...
>
>The thing to do about it is for _you_ to invest the time in some engineering
>practices and show that you really found a bug, you can reproduce the bug,
>you implemented some feature enhancement, your modifications didn't introduce
>other problems, the code considers effects on other platforms, and that
>your update does what it claims.

This was a logic (i.e. thinking) exercise - not an engineering problem. For
less trivial examples, I am very careful about any code I submit - but of
course, no-one is perfect - including yourself.

>When I have to do this, the priority of
>these updates falls well below my own work.

You are not god's gift to linuxppc - other people have worthy contributions
to make and understand the way it all works just as well as (if not better
than) you. Most of your message was informing us how much effort you put in
and for us to put up or shut up, then you come out with this - which basically
says that you don't have the time to put in all this effort.

I admit I don't have the time. I will contribute as much as I can, when I can,
but if my patches are not accepted, I am not going to lose any sleep over that.
The patches are out there on the public record for anyone to use if they so
wish - they just need to put in a bit of effort to find them.

Please don't let my (hopefully constructive) criticism affect the amount of
effort you put in. I know you contribute a lot and you shouldn't say "well
bugger them, I'm not doing any more for the ungrateful bastards". I am grateful.
To you, and all the others who participate in this list. I have fixed quite a
few problems on our board from patches in the list that have not made it into
the "official" linuxppc tree.

A good example of this is the 8260 FCC ethernet driver - in fcc_restart(). If
you want to set Full Duplex, then both the Full Duplex Enable (FDE) bit *and*
the Local Protect (LPB) bit need to be set - if the LPB bit is not set, the
receiver is blocked while transmitting, thus negating true full duplex
operation. The symptom is poor throughput on fcc ethernet reception. A number
of people, including myself (although I didn't "discover" it, I just use it in
my local tree), have posted this fix, and it still has not made it into the
"official" linuxppc source. There is a lot of other good stuff out there - the
Denx site has heaps of good code that hasn't been accepted.

I will restate my position, then shut up. It needs to be made easier to get
code into the *development* kernel tree. The level of testing required is
by definition less than required for the "stable" tree. The current system is
not optimum. If there is a lack of time on the part of the current people
with write permission to the linuxppc *development* tree, then there needs to
be more "writers". Doing this will inevitably lead to problems, but the gains
will outweigh them, I'm sure. BitKeeper allows easy management of the problems.
Someone needs to be appointed the judge, or there needs to be a panel of
"judges", so that writers will not have patch backout wars. All this has been
done quite successfully in other projects. And finally, at the very least, it
needs to be seen to be fair, so that certain people and/or companies must not
appear to receive favourable treatment.

This discussion is wasting everyone's time (well, mine at least). Lets just
agree to disagree and get on with our work. While the current situation is
not optimal, I am getting by with it as it is. So unless you are going to
say "ahh, I see the problem, I will try to change the way I do things", there
really isn't any point in responding. Cheers!
								Murray...
--
Murray Jensen, CSIRO Manufacturing & Infra. Tech.      Phone: +61 3 9662 7763
Locked Bag No. 9, Preston, Vic, 3072, Australia.         Fax: +61 3 9662 7853
Internet: Murray.Jensen@csiro.au

Hymod project: http://www.msa.cmst.csiro.au/projects/Hymod/

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: [PATCH] 8xx_io/uart.c

[Dan Malek]

Murray Jensen wrote:

> If you simply looked at the code you would see the problem we are talking
> about. What are you suggesting? That kgdb and xmon will _NEVER_ send a
> character with decimal value 10 via my_console_write()?

I've looked at it.  I know it well.  I wrote it.  I know how its used.
xmon and kgdb format their own strings/packets and expect read/write of
a simple uart fifo.

> http://lists.linuxppc.org/linuxppc-embedded/200205/msg00268.html

Well, ahhh, there isn't anything mentioned there about fixing bugs.  From
your description it's all about yet another scc uart configuration method.
I have lots of those on the shelf, all conflicting, that always seem to
be useful to the person/platform/application.  The easiest way to keep
everyone (un)happy is to leave things alone.  I used to apply those,
a new one every other week, and the only person happy about it was the
last one that had the patch applied.  :-)

You can pull the uart "bugfix" from the 2_4_devel tree now, if you wish.


	-- Dan


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: [PATCH] 8xx_io/uart.c

[Murray Jensen]

On Tue, 11 Feb 2003 11:40:45 -0500, Dan Malek <dan@embeddededge.com> writes:
>I've looked at it.  I know it well.  I wrote it.  I know how its used.
>xmon and kgdb format their own strings/packets and expect read/write of
>a simple uart fifo.

The point was, and always has been, can it be guaranteed that kgdb/xmon will
never output a character with decimal value 10? As a simple logic exercise,
it cannot be guaranteed - of course, in practice it is unlikely to happen,
because they encode everything as two hex digits.

But someone in the future may decide to make everything scroll nicely on
output by adding a (redundant) linefeed to the end of every packet - then
the kernel starts crashing in early boot when KGDB or XMON is enabled and
they will have no idea why.

>> http://lists.linuxppc.org/linuxppc-embedded/200205/msg00268.html
>
>Well, ahhh, there isn't anything mentioned there about fixing bugs.

It wasn't mentioned because it is trivial - the code as it stood was
logically inconsistent - I fixed it, as have (many?) others.

>From
>your description it's all about yet another scc uart configuration method.
>I have lots of those on the shelf, all conflicting, that always seem to
>be useful to the person/platform/application.  The easiest way to keep
>everyone (un)happy is to leave things alone.  I used to apply those,
>a new one every other week, and the only person happy about it was the
>last one that had the patch applied.  :-)

This illustrates my point(s) - linuxppc on the 8260 is stuck with the amazingly
bad uart configuration/management arrangement it has had for years (and which
was spawned back in the 8xx days) because I don't work for the right company.

Like I said - I will shut up now - it isn't that important to me. I made my
points in the previous message, and it is on the public record (such as it
is), but now I don't want to distract either of us from our work. Cheers!
								Murray...
--
Murray Jensen, CSIRO Manufacturing & Infra. Tech.      Phone: +61 3 9662 7763
Locked Bag No. 9, Preston, Vic, 3072, Australia.         Fax: +61 3 9662 7853
Internet: Murray.Jensen@csiro.au

Hymod project: http://www.msa.cmst.csiro.au/projects/Hymod/


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: [PATCH] 8xx_io/uart.c

[Murray Jensen]

On Tue, 11 Feb 2003 11:40:45 -0500, Dan Malek <dan@embeddededge.com> writes:
>You can pull the uart "bugfix" from the 2_4_devel tree now, if you wish.

Whoops, forgot to ask whether you had also examined the need to set the
Local Protect Bit for full duplex mode in fcc_restart()? Cheers!
								Murray...
--
Murray Jensen, CSIRO Manufacturing & Infra. Tech.      Phone: +61 3 9662 7763
Locked Bag No. 9, Preston, Vic, 3072, Australia.         Fax: +61 3 9662 7853
Internet: Murray.Jensen@csiro.au

Hymod project: http://www.msa.cmst.csiro.au/projects/Hymod/


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: [PATCH] 8xx_io/uart.c

[Tom Rini]

On Tue, Feb 11, 2003 at 12:13:49PM +1100, Murray Jensen wrote:

[snip]
> >I took the time to look for something from you in the past and didn't find
> >anything.  If it showed up, you would have received the same discussion.
>
> http://lists.linuxppc.org/linuxppc-embedded/200205/msg00268.html
>
> You probably didn't find it because I compressed the patch. I guess this is
> a good example of why one shouldn't compress patches sent to the list.

I'm not going to say too much on this branch of the thread, but this is
also a _very_ good example of why I'm always asking for 1 logical change
per patch.

--
Tom Rini
http://gate.crashing.org/~trini/

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

RE: [PATCH] 8xx_io/uart.c

[Joakim Tjernlund]

> [mailto:owner-linuxppc-embedded@lists.linuxppc.org]On Behalf Of Dan Malek
> Murray Jensen wrote:
>
>  > ...a quick perusal of the code will convince you
> > of this.
>
> No it won't.  Find the case where this happens and then I'll be convinced :-)

Correct, it won't fail since the first instance is redundant. The info ptr is
set to &consinfo and consinfo has "good" values for tx_va_base and tx_bd_base.

However if my_console_write is called BEFORE consinfo has been setup(in
serial_console_setup) then both is needed. Will KGDB/XMON write this early and/or
is KGDB/XMON guaranteed not to write a LF? It sure seems fragile.

> For example, was someone supposed to blindly accept Jocke's patch to make
> the serial port fifos so much larger?  I had to take the time (over and over) :-)
> to explain why this wasn't a bug.  It would be nice if some of you went through
> the learning curve to understand how things are supposed to work before you
> just declare bugs and submit patches.

Well, the patch itself did not contain the FIFO part, but the mail contained
a request to make them bigger. I would had included the FIFO part in the patch
if I had been sure it was the right thing to do. Still think it's a good
idea to expand the # of TX BD and the FIFO length, but possibly one should
keep the lower length during early access to keep the usage of DPRAM low.

> As a final example, Jocke keeps bugging me about his ethernet updates.  I
> explained (yet again, it was in the archives) that this had been attempted
> in the past, and it caused problems with packet routing/forwarding.  Why
> do I have to take the time to set up the testing environment and once again
> prove this is still a problem?  I hoped to find someone with a test
> environment to prove this (as they mentioned the bug in the first place),
> but they are also busy with their own projects.  Someday I may have time
> to set this up, make some changes to the patch so it works better, and
> do some testing to ensure the upstream alignment changes are done properly.
> If someone else can take the time to ensure it works (or make it work)
> in this condition where it was known to fail in the past, I would be more
> likely to check it in without taking the time to do the testing myself.

Come on, the last constructive mail I got from you regarding my Ethernet patch
was http://lists.linuxppc.org/linuxppc-embedded/200211/msg00123.html
where you told me you were looking at the patch and would integrate
"something that hopefully works". You had some concerns about L1 cache alignment
and I explained that I carefully checked that the buffer were L1 cache aligned.

The buffer is kmalloc'ed and kmalloc always returns L1 cache aligned buffers.
Then  __dev_alloc_skb()/dev_alloc_skb() reserves 16 bytes in the beginning of the
buffer and since the L1 cache line is 16 bytes it is still L1 cache aligned.

If this is your only concern, I can make a new patch that allocates 2*L1_CACHE_BYTES
extra space and place the data buffer in between.

Since then you told me that you had sent the patch to some guys that would
be able to trigger the problem you are worried about and I have waited several
months to get the results(and sent reminder to you) and this the first I
hear from you about the outcome.

You reefer to the archives and I have tried to find it, but I didn't find anything.
Please supply search keys or a link when you reefer to the archives since
the search function does not work very well(you must known this already since you could
not locate the mail from Murray Jensen).

>
> It's easy to hack up code, throw it over the fence, and complain when
> someone else won't take care of it.  Make that job easier for those of
> us trying to maintain the source trees, it would be appreciated.

You think I just hacked this up and throwed it over the fence? I beg to differ, I have
been monitoring the success/failure of this patch all along, posted updates and answered
questions. The patch so far has been a success and I think it's more than ready to go in.

 Jocke


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: [PATCH] 8xx_io/uart.c

[Dan Malek]

Joakim Tjernlund wrote:

> ..... Will KGDB/XMON write this early and/or
> is KGDB/XMON guaranteed not to write a LF?

Yes, yes.  Making these work, especially kgdb, was a real PITA.
Both format their own packets/strings and expect behavior of
reading/writing directly to a uart FIFO.  Like I said, if you really
understand how xmon/kgdb work, you would understand why these functions
are written exactly as they are.

The printk never calls a serial driver before it is fully initialized.
Debugger serial interfaces, especially kgdb, expect a simple uart fifo
immediately available within a few hundred lines of kernel code execution.


> The buffer is kmalloc'ed and kmalloc always returns L1 cache aligned buffers.
> Then  __dev_alloc_skb()/dev_alloc_skb() reserves 16 bytes in the beginning of the
> buffer and since the L1 cache line is 16 bytes it is still L1 cache aligned.

It's the data at the end of the buffer that's the problem.  The IP stack (used
to) stores information in dynamically allocated memory that would get corrupted
when buffers were invalidated.  There was lots of discussion about this on kernel
and other mailing lists (like mips and arm).  After we found it, there was some
discussion about how to fix it, but I didn't follow up on all of the details.
To ensure this works, you have to configure a forwarding/routing multiple
ethernet environment and get the 8xx to forward packets across the links.
That was the failure condition, there are people using 8xx in these configurations,
and we can't introduce a "solution" that has been known to be a problem in
the past.

> You think I just hacked this up and throwed it over the fence?

No, but you could have saved yourself some time by reading the archives.
You could have started with:
	http://lists.linuxppc.org/linuxppc-embedded/200008/msg00105.html
and then:
	http://lists.linuxppc.org/linuxppc-embedded/200106/msg00078.html

After the problems with the 405gp and tulip driver I started looking around
at the other lists and asking network folks for solutions. They didn't have
any and suggested non coherent processors not dma into skbuffers.  We are
fortunate on 8xx that the driver is only used on this processor, so if we
can find some solution it doesn't have to work generically for others.  The
networking folks suggested the only solution was to give skbuf allocators
knowledge of cache line sizes, but that was never pursued as far as I know.


	-- Dan


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

RE: [PATCH] 8xx_io/uart.c

[Joakim Tjernlund]

> Joakim Tjernlund wrote:
>
> > ..... Will KGDB/XMON write this early and/or
> > is KGDB/XMON guaranteed not to write a LF?
>
> Yes, yes.  Making these work, especially kgdb, was a real PITA.
> Both format their own packets/strings and expect behavior of
> reading/writing directly to a uart FIFO.  Like I said, if you really
> understand how xmon/kgdb work, you would understand why these functions
> are written exactly as they are.

I think I know why the are as they are now. KGDB/XMON stuff is messy and if you
are sure that XMON/KGDB never outputs a LF all is fine.

>
> The printk never calls a serial driver before it is fully initialized.
> Debugger serial interfaces, especially kgdb, expect a simple uart fifo
> immediately available within a few hundred lines of kernel code execution.

printk's will go out after serial_console_setup() has executed which I don't
consider "fully initialized" but it's initialized enough for printk's to get
out safely.

>
>
> > The buffer is kmalloc'ed and kmalloc always returns L1 cache aligned buffers.
> > Then  __dev_alloc_skb()/dev_alloc_skb() reserves 16 bytes in the beginning of the
> > buffer and since the L1 cache line is 16 bytes it is still L1 cache aligned.
>
> It's the data at the end of the buffer that's the problem.  The IP stack (used
> to) stores information in dynamically allocated memory that would get corrupted
> when buffers were invalidated.  There was lots of discussion about this on kernel
> and other mailing lists (like mips and arm).  After we found it, there was some
> discussion about how to fix it, but I didn't follow up on all of the details.
> To ensure this works, you have to configure a forwarding/routing multiple
> ethernet environment and get the 8xx to forward packets across the links.
> That was the failure condition, there are people using 8xx in these configurations,
> and we can't introduce a "solution" that has been known to be a problem in
> the past.
>
> > You think I just hacked this up and throwed it over the fence?
>
> No, but you could have saved yourself some time by reading the archives.
> You could have started with:
> 	http://lists.linuxppc.org/linuxppc-embedded/200008/msg00105.html
> and then:
> 	http://lists.linuxppc.org/linuxppc-embedded/200106/msg00078.html
>

Like I said, if you had given me a better hint then "the archives" we
could had resolved this long ago.

I have checked the links above it looks like it's solved. As far as I can
tell alternative:
  0. change alloc_skb()/skb_add_mtu() to force the allocated size to be a
     cache line multiple.
has been impl. in the kernel:
 - skb_add_mtu() does not exist anymore.
 - alloc_skb() does L1 cache align the size:
       size = requested_size + 16;
       size = SKB_DATA_ALIGN(size); /* this does L1 cache alignment */
       data = kmalloc(size + sizeof(struct skb_shared_info), gfp_mask);
   This makes the skb_shared_info to always start on a new cache line.
   There is no risk that it is invalidated by dma_cache_inv() call.

 Jocke


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

RE: [PATCH] 8xx_io/uart.c

[Paul Mackerras]

Joakim Tjernlund writes:

> I think I know why the are as they are now. KGDB/XMON stuff is messy and if you
> are sure that XMON/KGDB never outputs a LF all is fine.

Well I can tell you for a fact that xmon will output \n.

Dan, the code looks fragile as it is - why shouldn't the second part
(inside the if (*s == 10) body) do the same test as the first part?
Relying on xmon/kgdb never outputting \n sounds like a bad idea to me.

Paul.

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

RE: [PATCH] 8xx_io/uart.c

[Joakim Tjernlund]

> I have checked the links above it looks like it's solved. As far as I can
> tell alternative:
>   0. change alloc_skb()/skb_add_mtu() to force the allocated size to be a
>      cache line multiple.
> has been impl. in the kernel:
>  - skb_add_mtu() does not exist anymore.
>  - alloc_skb() does L1 cache align the size:
>        size = requested_size + 16;
>        size = SKB_DATA_ALIGN(size); /* this does L1 cache alignment */
>        data = kmalloc(size + sizeof(struct skb_shared_info), gfp_mask);
>    This makes the skb_shared_info to always start on a new cache line.
>    There is no risk that it is invalidated by dma_cache_inv() call.
>
>  Jocke

Dan, any doubts still?

 Jocke


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: [PATCH] 8xx_io/uart.c

[Dan Malek]

Joakim Tjernlund wrote:

> Dan, any doubts still?

I'm in the midst of getting 2.5 working for 8xx.  I'll check in
all of these patches when I can get them into both trees.  It's
going well, should have it done shortly.

Once 8xx is running in 2.5, I'm going to ask for all 8xx/82xx patches
to be tested and will be applied to both 2.4 and 2.5 trees.

Thanks.


	-- Dan


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

RE: [PATCH] 8xx_io/uart.c

[Joakim Tjernlund]

> [mailto:owner-linuxppc-embedded@lists.linuxppc.org]On Behalf Of Dan Malek
> Joakim Tjernlund wrote:
> > Please can you change TX_NUM_FIFO to 8 and TX_BUF_SIZE to 96,
> > since chars are lost when pasting text into the console otherwise.
>
> What makes 96 a big enough number?  If you want to do this locally,
> that's fine, but only flow control can guarantee you won't overflow
> a serial port of any fifo depth.

The fix I posted for drivers/char/n_tty.c is buggy w.r.t looking and
noone wants to play with the tty layer since it is a mess.

So I have been looking into what can be done in the uart.c.
opost_block(in n_tty.c) will never write more that 80 chars in
one go so it makes sense if we can match that size(TX_BUF_SIZE >= 80).
That should not be a problem since this memory is allocated with
m8xx_cpm_hostalloc()and the only user that function is uart.c

Increasing TX_NUM_FIFO to 8 is not big deal either IMHO.
Possibly one could let serial_console_setup allocate all the
BDs that is needed and make rs_8xx_init() reuse them. That
will save 24 bytes(3 BDs) from dp memory. This may be tricky though.

Another thing that will help is to make rs_8xx_put_char() build
a transmit queue and impl. rs_8xx_flush_chars() to actually write
them. I have tested this and it helps but it won't solve all cases.

What do you think?

> I don't want to arbitrarily make
> this fifo so large as there are other processing/latency/memory
> tradeoffs.  The SMC is not a high performance interface and it
> consumes lots of CPM cycles.

  I can't see the processing and latency implications.

   Jocke


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

RE: [PATCH] 8xx_io/uart.c

[Ruhland, Paul]

The pasting into the console bug is actually caused in
'drivers/char/n_tty.c' (standard tty line discipline) by not handling a
failed call to 'opost(...)' (if returns -1 if tx buffer is full and char
must be retried) in 'echo_char()'.  The pasted data is received fine....just
the echo fails.

>From 'drivers/char/n_tty.c':
================================================
/* Must be called only when L_ECHO(tty) is true. */

static void echo_char(unsigned char c, struct tty_struct *tty)
{
	if (L_ECHOCTL(tty) && iscntrl(c) && c != '\t') {
		put_char('^', tty);
		put_char(c ^ 0100, tty);
		tty->column += 2;
	} else
		opost(c, tty);
}
===============================================



-----Original Message-----
From: Dan Malek [mailto:dan@embeddededge.com]
Sent: Friday, February 07, 2003 11:09 AM
To: joakim.tjernlund@lumentis.se
Cc: Linuxppc-Embedded@Lists. Linuxppc. Org
Subject: Re: [PATCH] 8xx_io/uart.c



Joakim Tjernlund wrote:

> Found a bug in the serial driver. my_console_write() uses the wrong
address in early console writes.
> Removed some warnings as well.

Well......console write doesn't get called this early.  The purpose
of these address modifications in other parts of the driver are for
kgdb using the serial port for early debugging.

I'd really like some supporting documentation (like a kernel panic
or other reproducable error) before you declare something a "bug",
along with showing the same test fixed the problem.

> Please can you change TX_NUM_FIFO to 8 and TX_BUF_SIZE to 96,
> since chars are lost when pasting text into the console otherwise.

What makes 96 a big enough number?  If you want to do this locally,
that's fine, but only flow control can guarantee you won't overflow
a serial port of any fifo depth.  I don't want to arbitrarily make
this fifo so large as there are other processing/latency/memory
tradeoffs.  The SMC is not a high performance interface and it
consumes lots of CPM cycles.

Thanks.


	-- Dan


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

RE: [PATCH] 8xx_io/uart.c

[Joakim Tjernlund]

> The pasting into the console bug is actually caused in
> 'drivers/char/n_tty.c' (standard tty line discipline) by not handling a
> failed call to 'opost(...)' (if returns -1 if tx buffer is full and char
> must be retried) in 'echo_char()'.  The pasted data is received fine....just
> the echo fails.

got the same here.

>
> >From 'drivers/char/n_tty.c':
> ================================================
> /* Must be called only when L_ECHO(tty) is true. */
>
> static void echo_char(unsigned char c, struct tty_struct *tty)
> {
> 	if (L_ECHOCTL(tty) && iscntrl(c) && c != '\t') {
> 		put_char('^', tty);
> 		put_char(c ^ 0100, tty);
> 		tty->column += 2;
> 	} else
> 		opost(c, tty);
> }
> ===============================================
>

Great, do you have a fix?

    Jocke


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

RE: [PATCH] 8xx_io/uart.c

[Joakim Tjernlund]

> The pasting into the console bug is actually caused in
> 'drivers/char/n_tty.c' (standard tty line discipline) by not handling a
> failed call to 'opost(...)' (if returns -1 if tx buffer is full and char
> must be retried) in 'echo_char()'.  The pasted data is received fine....just
> the echo fails.
>

Hi again

Thanks to Paul Ruhland I was able to fix my problem with chars getting
lost.

This little patch of mine works really fine for me, I just want to known
if it's a good solution or if something else is to prefer.

 Jocke

Index: drivers/char/n_tty.c
===================================================================
RCS file: /home/cvsadmin/cvsroot/kernel/linuxppc/drivers/char/n_tty.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 n_tty.c
--- drivers/char/n_tty.c	1 Nov 2002 13:43:27 -0000	1.1.1.1
+++ drivers/char/n_tty.c	10 Feb 2003 14:38:56 -0000
@@ -186,8 +186,10 @@
 static int opost(unsigned char c, struct tty_struct *tty)
 {
 	int	space, spaces;
+	unsigned long tmo = jiffies + HZ/10;

-	space = tty->driver.write_room(tty);
+	while(!(space = tty->driver.write_room(tty)) && !time_after(jiffies, tmo))
+		cond_resched();
 	if (!space)
 		return -1;


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: [PATCH] 8xx_io/uart.c

[Tom Rini]

On Mon, Feb 10, 2003 at 03:45:41PM +0100, Joakim Tjernlund wrote:
>
> > The pasting into the console bug is actually caused in
> > 'drivers/char/n_tty.c' (standard tty line discipline) by not handling a
> > failed call to 'opost(...)' (if returns -1 if tx buffer is full and char
> > must be retried) in 'echo_char()'.  The pasted data is received fine....just
> > the echo fails.
> >
>
> Hi again
>
> Thanks to Paul Ruhland I was able to fix my problem with chars getting
> lost.
>
> This little patch of mine works really fine for me, I just want to known
> if it's a good solution or if something else is to prefer.

Bring it up on linux-kernel@vger.kernel.org.  A brief description and
this patch should be enough to get people to listen.  Maybe send it to
Alan Cox too..

--
Tom Rini
http://gate.crashing.org/~trini/

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

RE: [PATCH] 8xx_io/uart.c

[Joakim Tjernlund]

>
> Bring it up on linux-kernel@vger.kernel.org.  A brief description and
> this patch should be enough to get people to listen.  Maybe send it to
> Alan Cox too..

Yep, this needs to be discussed elsewhere. I have received some comments
and these suggests that opost() can be executed in interrupt context so
something else (non trivial) needs to be done.

 Jocke


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: EV-64260-BP & GT64260 bi_recs

[Murray Jensen]

On Wed, 20 Mar 2002 13:51:01 -0500, Dan Malek <dan@embeddededge.com> writes:
>If we want to be passing arbitrary information into the kernel
>for anyone to use, perhaps we should consider using the standard
>argc, argv, envp, like other architectures.  Just pass ASCII
>strings into the kernel as you would any other program, and have
>a function that can search for and parse 'param=value' strings.

I like this idea (a lot), but I'm not sure you were serious. The problem with
this method starts when you have a lot of information to pass - it gets unwieldy
and becomes almost as unreadable as a hex dump (anyone ever had to read the
output from a make of the gcc compiler to decipher what went wrong? with all
those command line options passed to each invocation of the compiler, the real
information gets lost).

I don't think the method matters a lot, as long as there is only one way to do
it (that is unlimited, flexible, extensible), not the three different methods
we have today (command line, bd_t struct and bi_recs).

Oh, and for the record - I vote for b). The stuff posted by benh is a very
good start for this - I really like the idea of bi_recs within bi_recs - this
gives you effectively unlimited flexibility, but handlers at the top level can
just skip/pass the entire record without worrying about its content. Cheers!
								Murray...
--
Murray Jensen, CSIRO Manufacturing Sci & Tech,         Phone: +61 3 9662 7763
Locked Bag No. 9, Preston, Vic, 3072, Australia.         Fax: +61 3 9662 7853
Internet: Murray.Jensen@csiro.au

Hymod project: http://www.msa.cmst.csiro.au/projects/Hymod/


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: EV-64260-BP & GT64260 bi_recs

[Dan Malek]

Murray Jensen wrote:

> I like this idea (a lot), but I'm not sure you were serious.

I'm always serious :-).

> .... The problem with
> this method starts when you have a lot of information to pass

I'm going to (constantly :-) argue that if you are passing lots of information
then something isn't designed correctly.  It's one thing to be passing a few
hardware hints or small configuration values, but I think it's quite wrong
to design a bunch of dynamic software that requires a small data base to be
passed into to the kernel.  Please don't be confused by this comment and start
aruging OF device trees on Macs.  The OF tree is really just a bunch of small
configuration items that allows lots of generic software to run on a variety of
Mac platforms.


	-- Dan


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: EV-64260-BP & GT64260 bi_recs

[Murray Jensen]

On Thu, 21 Mar 2002 01:50:21 -0500, Dan Malek <dan@embeddededge.com> writes:
>> .... The problem with
>> this method starts when you have a lot of information to pass
>
>I'm going to (constantly :-) argue that if you are passing lots of information
>then something isn't designed correctly.

Then I guess that I am going to (constantly) disagree with you :-)

The more Linux can learn from the boot loader, the more generic the kernel
and the wider the set of hardware a particular kernel will run on without
recompilation.

I disagree with your contention (in another recent message) that this is bloat
and is bad for embedded systems. It actually reduces bloat because code in the
kernel that duplicates things that are done by the boot loader goes away - the
kernel is smaller and more generic, and the code is cleaner (fewer #ifdefs etc)
- and a more generic kernel is more user friendly (i.e. easier for someone
without 20 years Comp. Sci. experience to port).

>It's one thing to be passing a few
>hardware hints or small configuration values, but I think it's quite wrong
>to design a bunch of dynamic software that requires a small data base to be
>passed into to the kernel.

And I think this is what is needed. Just because we have always done it one
way, does not mean it is the right way, or that we have to continue to do it
that way.

>Please don't be confused by this comment and start
>aruging OF device trees on Macs.  The OF tree is really just a bunch of small
>configuration items that allows lots of generic software to run on a variety o
>f
>Mac platforms.

It is clear that (many?) others think along the same lines as I do. Evidence
is the many boot loaders that implement some sort of device tree - the
OpenBoot on my Ultra-60 is a good example. They are all just bunches of
small configuration items, but there might be a lot of them, and/or they might
have complex structure.

However, your position is the default position and requires little work. My
position requires a lot of work and coordination - I don't think it is going
to happen soon, if ever (although benh's posted code was the closest I've seen
yet and looks very promising). Cheers!
								Murray...
--
Murray Jensen, CSIRO Manufacturing Sci & Tech,         Phone: +61 3 9662 7763
Locked Bag No. 9, Preston, Vic, 3072, Australia.         Fax: +61 3 9662 7853
Internet: Murray.Jensen@csiro.au

Hymod project: http://www.msa.cmst.csiro.au/projects/Hymod/


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/