Re: EV-64260-BP & GT64260 bi_recs

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/

[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: EV-64260-BP & GT64260 bi_recs

[Michael Sokolov]

Mark A. Greer <mgreer@mvista.com> wrote:

> Attempt #6:

Go for it!

MS

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

Re: EV-64260-BP & GT64260 bi_recs

[Michael Sokolov]

benh@kernel.crashing.org wrote:

> The main problem I have with putting a "structure" inside one bi_rec like
> BI_GT64260_ETH_CFG is that it won't evolve. I mean, let's say today, you
> define it contains an HW ethernet address. Then, you figure out you have
> wired your PHY directly, it's not on the MII and you need your driver to
> know about it, how do you add that info ? Or you figure out that your
> driver would need some tunning (different LED wiring on the PHY), you
> change the structure layout to pass new parameters ?
> You create unnecessary compatibility hell.

OK, I accept that you are right, your flexible structure is better. Now are you
guys going to actually implement it or just talk about it?

MS

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

Re: EV-64260-BP & GT64260 bi_recs

[benh]

>OK, I accept that you are right, your flexible structure is better. Now
>are you
>guys going to actually implement it or just talk about it?

It will be done soon, I hope this week-end, in bk 2.5, then
eventually backported to 2.4

Ben.


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

Re: EV-64260-BP & GT64260 bi_recs

[Michael Sokolov]

Mark A. Greer <mgreer@mvista.com> wrote:

> I messed up the size on the BI_LAST struct, should be 12.  Attempt #5:

BI_FIRST and BI_LAST are normally dataless (size 8), but the parser doesn't
care.

MS

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

Re: EV-64260-BP & GT64260 bi_recs

[Michael Sokolov]

Mark A. Greer <mgreer@mvista.com> wrote:

> Yeah, that's right, I messed up.  So what should be done for the nested ones?
> What is we use the BI_DEVICE as a quasi-BI_FIRST and then add a BI_LAST to each
> nested one?  This is what it would look like:

I think the whole idea of nested bi_recs is completely unnecessary complexity.
A simple BI_GT64260_ETH_CFG is better.

MS

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

Re: EV-64260-BP & GT64260 bi_recs

[benh]

>> Yeah, that's right, I messed up.  So what should be done for the nested
>ones?
>> What is we use the BI_DEVICE as a quasi-BI_FIRST and then add a BI_LAST
>to each
>> nested one?  This is what it would look like:
>
>I think the whole idea of nested bi_recs is completely unnecessary
complexity.
>A simple BI_GT64260_ETH_CFG is better.

Well, I find it pretty simple, and it helps in lots of situations, especially
when you deal with devices on the PCI bus.

The main problem I have with putting a "structure" inside one bi_rec like
BI_GT64260_ETH_CFG is that it won't evolve. I mean, let's say today, you
define it contains an HW ethernet address. Then, you figure out you have
wired your PHY directly, it's not on the MII and you need your driver to
know about it, how do you add that info ? Or you figure out that your
driver would need some tunning (different LED wiring on the PHY), you
change the structure layout to pass new parameters ?
You create unnecessary compatibility hell.

Ben.


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

Re: EV-64260-BP & GT64260 bi_recs

[Michael Sokolov]

Mark A. Greer <mgreer@mvista.com> wrote:

> Okay, this may be it, attempt #4:

Looks fine, except for:

> [6] 0x00000000                          (NULL Termination of whole list)

Currently the entire bi_recs list starts with BI_FIRST (that's how we detect a
bi_recs list) and ends with BI_LAST (that's the terminator). We must maintain
compatibility.

MS

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

Re: EV-64260-BP & GT64260 bi_recs

[Mark A. Greer]

Michael Sokolov wrote:

> Mark A. Greer <mgreer@mvista.com> wrote:
>
> > Okay, this may be it, attempt #4:
>
> Looks fine, except for:
>
> > [6] 0x00000000                          (NULL Termination of whole list)
>
> Currently the entire bi_recs list starts with BI_FIRST (that's how we detect a
> bi_recs list) and ends with BI_LAST (that's the terminator). We must maintain
> compatibility.

Yeah, that's right, I messed up.  So what should be done for the nested ones?
What is we use the BI_DEVICE as a quasi-BI_FIRST and then add a BI_LAST to each
nested one?  This is what it would look like:

Example for ev64260:
--------------------

[0] tag: BI_FIRST
    size: 4
    data: 0x00000000                    (don't care)

[1] tag: BI_CMD_LINE
    size: 36                            (4 + 4 + 26 chars + 2 pad)
    data: "console=ttyS0,115200 ip=on"
    pad: 2

[2] tag: BI_MEMSIZE
    size: 12
    data: 0x33554432                    (32 MB)

[3] tag: BI_GT64260_BASE
    size: 12
    data: 0xfc000000

[4] tag: BI_DEVICE                      (embedded enet cltr 0)
    size: 60                            (8 + 2*12 + 16 + 12 == 60)

    [4.0] tag: BI_DEV_TYPE
          size: 12
          data: BI_DEV_TYPE_GT_ETH

    [4.1] tag: BI_DEV_ID
          size: 12
          data: 0x00000000              (1st enet device)

    [4.2] tag: BI_MAC_ADDR
          size: 16
          data: 0xaabbccddeeff          (6 byte MAC addr)
          pad: 2

    [4.3] tag: BI_LAST
          size: 4
          data: 0x00000000              (don't care)

[5] tag: BI_DEVICE                      (embedded enet cltr 1)
    size: 60                            (8 + 2*12 + 16 + 12 == 60)

    [5.0] tag: BI_DEV_TYPE
          size: 12
          data: BI_DEV_TYPE_GT_ETH

    [5.1] tag: BI_DEV_ID
          size: 12
          data: 0x00000001              (2nd enet device)

    [5.2] tag: BI_MAC_ADDR
          size: 16
          data: 0xgghhiijjkkll          (6 byte MAC addr)
          pad: 2

    [5.3] tag: BI_LAST
          size: 4
          data: 0x00000000              (don't care)

[6] tag: BI_DEVICE                      (embedded enet cltr 2)
    size: 60                            (8 + 2*12 + 16 + 12 == 60)

    [6.0] tag: BI_DEV_TYPE
          size: 12
          data: BI_DEV_TYPE_GT_ETH

    [6.1] tag: BI_DEV_ID
          size: 12
          data: 0x00000002              (3rd enet device)

    [6.2] tag: BI_MAC_ADDR
          size: 16
          data: 0xmmnnooppqqrr          (6 byte MAC addr)
          pad: 2

    [6.3] tag: BI_LAST
          size: 4
          data: 0x00000000              (don't care)

[7] tag: BI_LAST
    size: 4
    data: 0x00000000                    (don't care)


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

Re: EV-64260-BP & GT64260 bi_recs

[Mark A. Greer]

"Mark A. Greer" wrote:

> Yeah, that's right, I messed up.  So what should be done for the nested ones?
> What is we use the BI_DEVICE as a quasi-BI_FIRST and then add a BI_LAST to each
> nested one?  This is what it would look like:

I messed up the size on the BI_LAST struct, should be 12.  Attempt #5:

Example for ev64260:
--------------------

[0] tag: BI_FIRST
    size: 12
    data: 0x00000000                    (don't care)

[1] tag: BI_CMD_LINE
    size: 36                            (4 + 4 + 26 chars + 2 pad)
    data: "console=ttyS0,115200 ip=on"
    pad: 2

[2] tag: BI_MEMSIZE
    size: 12
    data: 0x33554432                    (32 MB)

[3] tag: BI_GT64260_BASE
    size: 12
    data: 0xfc000000

[4] tag: BI_DEVICE                      (embedded enet cltr 0)
    size: 60                            (8 + 2*12 + 16 + 12 == 60)

    [4.0] tag: BI_DEV_TYPE
          size: 12
          data: BI_DEV_TYPE_GT_ETH

    [4.1] tag: BI_DEV_ID
          size: 12
          data: 0x00000000              (1st enet device)

    [4.2] tag: BI_MAC_ADDR
          size: 16
          data: 0xaabbccddeeff          (6 byte MAC addr)
          pad: 2

    [4.3] tag: BI_LAST
          size: 12
          data: 0x00000000              (don't care)

[5] tag: BI_DEVICE                      (embedded enet cltr 1)
    size: 60                            (8 + 2*12 + 16 + 12 == 60)

    [5.0] tag: BI_DEV_TYPE
          size: 12
          data: BI_DEV_TYPE_GT_ETH

    [5.1] tag: BI_DEV_ID
          size: 12
          data: 0x00000001              (2nd enet device)

    [5.2] tag: BI_MAC_ADDR
          size: 16
          data: 0xgghhiijjkkll          (6 byte MAC addr)
          pad: 2

    [5.3] tag: BI_LAST
          size: 12
          data: 0x00000000              (don't care)

[6] tag: BI_DEVICE                      (embedded enet cltr 2)
    size: 60                            (8 + 2*12 + 16 + 12 == 60)

    [6.0] tag: BI_DEV_TYPE
          size: 12
          data: BI_DEV_TYPE_GT_ETH

    [6.1] tag: BI_DEV_ID
          size: 12
          data: 0x00000002              (3rd enet device)

    [6.2] tag: BI_MAC_ADDR
          size: 16
          data: 0xmmnnooppqqrr          (6 byte MAC addr)
          pad: 2

    [6.3] tag: BI_LAST
          size: 12
          data: 0x00000000              (don't care)

[7] tag: BI_LAST
    size: 12
    data: 0x00000000                    (don't care)


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

Re: EV-64260-BP & GT64260 bi_recs

[benh]

>Yeah, that's right, I messed up.  So what should be done for the nested ones?
>What is we use the BI_DEVICE as a quasi-BI_FIRST and then add a BI_LAST
>to each
>nested one?  This is what it would look like:

Any reason why BI_LAST has those 4 bytes of data set to 0, why not
just BI_LAST size=0 and no data ?

Ben.


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

Re: EV-64260-BP & GT64260 bi_recs

[Mark A. Greer]

benh@kernel.crashing.org wrote:

> Any reason why BI_LAST has those 4 bytes of data set to 0, why not
> just BI_LAST size=0 and no data ?

Only reason is that it is how its implemented now so to keep backward
compatibility...

E.g., _2_4_devel, arch/ppc/boot/common/misc-simple.c:decompress_kernel()

        rec->tag = BI_LAST;
        rec->size = sizeof(struct bi_record);
        rec = (struct bi_record *)((unsigned long)rec + rec->size);

Mark


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

Re: EV-64260-BP & GT64260 bi_recs

[Mark A. Greer]

"Mark A. Greer" wrote:

> benh@kernel.crashing.org wrote:
>
> > Any reason why BI_LAST has those 4 bytes of data set to 0, why not
> > just BI_LAST size=0 and no data ?
>
> Only reason is that it is how its implemented now so to keep backward
> compatibility...
>
> E.g., _2_4_devel, arch/ppc/boot/common/misc-simple.c:decompress_kernel()
>
>         rec->tag = BI_LAST;
>         rec->size = sizeof(struct bi_record);
>         rec = (struct bi_record *)((unsigned long)rec + rec->size);

Ahh shit, I was thinking bi_rec had data[1], not data[0].

Attempt #6:


[0] tag: BI_FIRST
    size: 8

[1] tag: BI_CMD_LINE
    size: 36                            (4 + 4 + 26 chars + 2 pad)
    data: "console=ttyS0,115200 ip=on"
    pad: 2

[2] tag: BI_MEMSIZE
    size: 12
    data: 0x33554432                    (32 MB)

[3] tag: BI_GT64260_BASE
    size: 12
    data: 0xfc000000

[4] tag: BI_DEVICE                      (embedded enet cltr 0)
    size: 56                            (8 + 2*12 + 16 + 8 == 56)

    [4.0] tag: BI_DEV_TYPE
          size: 12
          data: BI_DEV_TYPE_GT_ETH

    [4.1] tag: BI_DEV_ID
          size: 12
          data: 0x00000000              (1st enet device)

    [4.2] tag: BI_MAC_ADDR
          size: 16
          data: 0xaabbccddeeff          (6 byte MAC addr)
          pad: 2

    [4.3] tag: BI_LAST
          size: 8

[5] tag: BI_DEVICE                      (embedded enet cltr 1)
    size: 56                            (8 + 2*12 + 16 + 8 == 56)

    [5.0] tag: BI_DEV_TYPE
          size: 12
          data: BI_DEV_TYPE_GT_ETH

    [5.1] tag: BI_DEV_ID
          size: 12
          data: 0x00000001              (2nd enet device)

    [5.2] tag: BI_MAC_ADDR
          size: 16
          data: 0xgghhiijjkkll          (6 byte MAC addr)
          pad: 2

    [5.3] tag: BI_LAST
          size: 8
[6] tag: BI_DEVICE                      (embedded enet cltr 2)
    size: 56                            (8 + 2*12 + 16 + 8 == 56)

    [6.0] tag: BI_DEV_TYPE
          size: 12
          data: BI_DEV_TYPE_GT_ETH

    [6.1] tag: BI_DEV_ID
          size: 12
          data: 0x00000002              (3rd enet device)

    [6.2] tag: BI_MAC_ADDR
          size: 16
          data: 0xmmnnooppqqrr          (6 byte MAC addr)
          pad: 2

    [6.3] tag: BI_LAST
          size: 8

[7] tag: BI_LAST
    size: 8


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

Re: EV-64260-BP & GT64260 bi_recs

[Michael Sokolov]

Mark A. Greer <mgreer@mvista.com> wrote:

> Forget the last one, here's attempt #3:

I can live with this.

But why is the MAC address in ASCII? Why not simply put the 6 bytes in there?
Why the extra burden of parsing the ASCII?

MS

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

Re: EV-64260-BP & GT64260 bi_recs

[Mark A. Greer]

Michael Sokolov wrote:

> Mark A. Greer <mgreer@mvista.com> wrote:
>
> > Forget the last one, here's attempt #3:
>
> I can live with this.

I like it too.

> But why is the MAC address in ASCII? Why not simply put the 6 bytes in there?
> Why the extra burden of parsing the ASCII?

Yes, it is ASCII.  I prefer the actual 6 bytes too but I know others have
expressed a strong preference for ASCII when dumping the bi_rec data (although
in this example the mac addr is just as easy to read in a hex dump).

Does anyone have a problem if the MAC address is changed to be binary?  Come to
think of it, since its of BI_DEV_TYPE_GT_ETH we can probably do what we want
since it only affects us.  Let'see if anyone comes up with a valid reason not to
change...

Mark


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

Re: EV-64260-BP & GT64260 bi_recs

[benh]

>But why is the MAC address in ASCII? Why not simply put the 6 bytes in there?
>Why the extra burden of parsing the ASCII?

Well, I would put the 6 bytes, I beleive he wrote it to make it
readable for us ;)

The point of bi_recs is indeed to avoid ascii (to avoid wasting too
much space). Or else we would just use a device_tree or an xml
tree :)

Ben.


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

Re: EV-64260-BP & GT64260 bi_recs

[Mark A. Greer]

benh@kernel.crashing.org wrote:

> >But why is the MAC address in ASCII? Why not simply put the 6 bytes in there?
> >Why the extra burden of parsing the ASCII?
>
> Well, I would put the 6 bytes, I beleive he wrote it to make it
> readable for us ;)
>
> The point of bi_recs is indeed to avoid ascii (to avoid wasting too
> much space). Or else we would just use a device_tree or an xml
> tree :

Okay, this may be it, attempt #4:

Example for ev64260:
--------------------

[0] tag: BI_CMD_LINE
    size: 36                            (4 + 4 + 26 chars + 2 pad)
    data: "console=ttyS0,115200 ip=on"
    pad: 2

[1] tag: BI_MEMSIZE
    size: 12
    data: 0x33554432                    (32 MB)

[2] tag: BI_GT64260_BASE
    size: 12
    data: 0xfc000000

[3] tag: BI_DEVICE                      (embedded enet cltr 0)
    size: 52                            (8 + 2*12 + 16 + 4 == 52)

    [3.0] tag: BI_DEV_TYPE
          size: 12
          data: BI_DEV_TYPE_GT_ETH

    [3.1] tag: BI_DEV_ID
          size: 12
          data: 0x00000000              (1st enet device)

    [3.2] tag: BI_MAC_ADDR
          size: 16
          data: 0xaabbccddeeff          (6 byte MAC addr)
          pad: 2

    [3.3] 0x00000000                    (NULL Termination of BI_STRUCT)

[4] tag: BI_DEVICE                      (embedded enet cltr 1)
    size: 52                            (8 + 2*12 + 16 + 4 == 52)

    [4.0] tag: BI_DEV_TYPE
          size: 12
          data: BI_DEV_TYPE_GT_ETH

    [4.1] tag: BI_DEV_ID
          size: 12
          data: 0x00000001              (2nd enet device)

    [4.2] tag: BI_MAC_ADDR
          size: 16
          data: 0xgghhiijjkkll          (6 byte MAC addr)
          pad: 2

    [4.3] 0x00000000                    (NULL Termination of BI_STRUCT)

[5] tag: BI_DEVICE                      (embedded enet cltr 2)
    size: 52                            (8 + 2*12 + 16 + 4 == 52)

    [5.0] tag: BI_DEV_TYPE
          size: 12
          data: BI_DEV_TYPE_GT_ETH

    [5.1] tag: BI_DEV_ID
          size: 12
          data: 0x00000002              (3rd enet device)

    [5.2] tag: BI_MAC_ADDR
          size: 16
          data: 0xmmnnooppqqrr          (6 byte MAC addr)
          pad: 2

    [5.3] 0x00000000                    (NULL Termination of BI_STRUCT)

[6] 0x00000000                          (NULL Termination of whole list)


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

Re: EV-64260-BP & GT64260 bi_recs

[Michael Sokolov]

Mark A. Greer <mgreer@mvista.com> wrote:

> First, we need to see if this is going to fit with everything else in
> 2_4/2_4_devel and then it will get implemented.

It would set a very bad example if it is implemented. In your proposal the
records containing the GT-64260 Ethernet information have no indication in them
whatsoever that they are for GT-64260 and not some other Ethernet. If you make
the gt64260_eth driver call a function to grab all bi_recs of this kind, the
next engineer who designs a board with other hard-wired Ethernet interfaces on
it besides the GT-64260 (like an MPC8260 + GT-64260 system) will be simply
stuck out of luck as the gt64260_eth driver will unceremoniously grab all
records for all Ethernets and there will be no way to tell which MAC address
belongs to each Ethernet.

Bad bad bad.

MS

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

Re: EV-64260-BP & GT64260 bi_recs

[benh]

>
>It would set a very bad example if it is implemented. In your proposal the
>records containing the GT-64260 Ethernet information have no indication
>in them
>whatsoever that they are for GT-64260 and not some other Ethernet. If
you make
>the gt64260_eth driver call a function to grab all bi_recs of this kind, the
>next engineer who designs a board with other hard-wired Ethernet
interfaces on
>it besides the GT-64260 (like an MPC8260 + GT-64260 system) will be simply
>stuck out of luck as the gt64260_eth driver will unceremoniously grab all
>records for all Ethernets and there will be no way to tell which MAC address
>belongs to each Ethernet.

Yah, this is why BI_DEV_TYPE should be GT64260_xxxx

We have several choices here for the design, I'm not sure which is best, so
please speak up:

When dealing with combo-chips like the GT or PPC 4xx/8xx etc..., we can
either:

 - define one BI_DEV_TYPE per chip (BI_DEV_TYPE_GT64260, ...). The actual
device within the GT64260 (ethernet in our case) is referenced via the
BI_DEV_CLASS (ethernet), BI_DEV_ID beeing optionally there in case a
given device in the chip exist in more than one instance.

 - define one BI_DEV_TYPE per chip (same as above), and define a BI_DEV_ID
containing both function and the index (for example 'ETH0') thus we don't
need BI_DEV_CLASS.

 - define a specific BI_DEV_TYPE for each function (that is
BI_DEV_TYPE_GT64260_ETH), BI_DEV_ID is only an index.

I tend to prefer solution 2)

Ben.


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

Re: EV-64260-BP & GT64260 bi_recs

[Matt Porter]

On Tue, Mar 26, 2002 at 10:52:35PM +0100, benh@kernel.crashing.org wrote:
>
> >
> >It would set a very bad example if it is implemented. In your proposal the
> >records containing the GT-64260 Ethernet information have no indication
> >in them
> >whatsoever that they are for GT-64260 and not some other Ethernet. If
> you make
> >the gt64260_eth driver call a function to grab all bi_recs of this kind, the
> >next engineer who designs a board with other hard-wired Ethernet
> interfaces on
> >it besides the GT-64260 (like an MPC8260 + GT-64260 system) will be simply
> >stuck out of luck as the gt64260_eth driver will unceremoniously grab all
> >records for all Ethernets and there will be no way to tell which MAC address
> >belongs to each Ethernet.
>
> Yah, this is why BI_DEV_TYPE should be GT64260_xxxx
>
> We have several choices here for the design, I'm not sure which is best, so
> please speak up:
>
> When dealing with combo-chips like the GT or PPC 4xx/8xx etc..., we can
> either:
>
>  - define one BI_DEV_TYPE per chip (BI_DEV_TYPE_GT64260, ...). The actual
> device within the GT64260 (ethernet in our case) is referenced via the
> BI_DEV_CLASS (ethernet), BI_DEV_ID beeing optionally there in case a
> given device in the chip exist in more than one instance.
>
>  - define one BI_DEV_TYPE per chip (same as above), and define a BI_DEV_ID
> containing both function and the index (for example 'ETH0') thus we don't
> need BI_DEV_CLASS.
>
>  - define a specific BI_DEV_TYPE for each function (that is
> BI_DEV_TYPE_GT64260_ETH), BI_DEV_ID is only an index.
>
> I tend to prefer solution 2)

Since device macro cells are commonly used across different chips,
it might be wise to orient the records around the device macro
rather than the SoC it is implemented in.  For example, there
are tens of 405 variants plus a similar future of 440 variants
that all share the EMAC macro device and corresponding driver.
A single BI_DEV_TYPE_IBM_EMAC would be most appropriate.  Mot
has a similar scheme with the many 825x/826x variants sharing
the same ethernet macro.  Marvell probably will use the same
eth macro in any forthcoming chips since they already share the
same eth macro between their MIPS and PPC offerings.

Regards,
--
Matt Porter
MontaVista Software, Inc.
mporter@mvista.com

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

Re: EV-64260-BP & GT64260 bi_recs

[Mark A. Greer]

Matt Porter wrote:

> On Tue, Mar 26, 2002 at 10:52:35PM +0100, benh@kernel.crashing.org wrote:
> >
> > >
> > >It would set a very bad example if it is implemented. In your proposal the
> > >records containing the GT-64260 Ethernet information have no indication
> > >in them
> > >whatsoever that they are for GT-64260 and not some other Ethernet. If
> > you make
> > >the gt64260_eth driver call a function to grab all bi_recs of this kind, the
> > >next engineer who designs a board with other hard-wired Ethernet
> > interfaces on
>

<snip>

> >
> > Yah, this is why BI_DEV_TYPE should be GT64260_xxxx
> >
> > We have several choices here for the design, I'm not sure which is best, so
> > please speak up:
> >
> > When dealing with combo-chips like the GT or PPC 4xx/8xx etc..., we can
> > either:
> >
> >  - define one BI_DEV_TYPE per chip (BI_DEV_TYPE_GT64260, ...). The actual
> > device within the GT64260 (ethernet in our case) is referenced via the
> > BI_DEV_CLASS (ethernet), BI_DEV_ID beeing optionally there in case a
> > given device in the chip exist in more than one instance.
> >
> >  - define one BI_DEV_TYPE per chip (same as above), and define a BI_DEV_ID
> > containing both function and the index (for example 'ETH0') thus we don't
> > need BI_DEV_CLASS.
> >
> >  - define a specific BI_DEV_TYPE for each function (that is
> > BI_DEV_TYPE_GT64260_ETH), BI_DEV_ID is only an index.
> >
> > I tend to prefer solution 2)
>
> Since device macro cells are commonly used across different chips,
> it might be wise to orient the records around the device macro
> rather than the SoC it is implemented in.  For example, there
> are tens of 405 variants plus a similar future of 440 variants
> that all share the EMAC macro device and corresponding driver.
> A single BI_DEV_TYPE_IBM_EMAC would be most appropriate.  Mot

<snip>

All good points.  Given Matt's comments, I think sol'n 3) may be the best.
Comments?

Here's the next iteration using Ben's sol'n 3)

I also adde NULL termination to the overall bi_rec and to each nested one so the
parsing code can stop when a NULL is hit whether its a nested one or not
(therefore we can never have a valid tag with value 0).  I also adjusted the size
of the BI_STRUCTS to include the 4 byte NULL terminator.  I also fixed an error on
the length of the BI_MAC_ADDR (was 25, should be 28 to count padding).

Mark
--

Example for ev64260:
--------------------

[0] tag: BI_CMD_LINE
    size: 36                            (4 + 4 + 26 chars + 2 pad)
    data: "console=ttyS0,115200 ip=on"
    pad: 2

[1] tag: BI_MEMSIZE
    size: 12
    data: 33554432                      (32 MB)

[2] tag: BI_GT64260_BASE
    size: 12
    data: fc000000

[3] tag: BI_STRUCT                      (embedded enet cltr 0)
    size: 68                            (12 + 2*12 + 28 + 4 == 68)
    data: BI_DEVICE

    [3.0] tag: BI_DEV_TYPE
          size: 12
          data: BI_DEV_TYPE_GT_ETH

    [3.2] tag: BI_DEV_ID
          size: 12
          data: 0                       (1st enet device)

    [3.3] tag: BI_MAC_ADDR
          size: 28
          data: aa:bb:cc:dd:ee:ff       (ascii)
          pad: 3

    [3.4] 0x00000000                    (NULL Termination of BI_STRUCT)

[4] tag: BI_STRUCT                      (embedded enet cltr 1)
    size: 68                            (12 + 2*12 + 28 + 4 == 68)
    data: BI_DEVICE

    [4.0] tag: BI_DEV_TYPE
          size: 12
          data: BI_DEV_TYPE_GT_ETH

    [4.2] tag: BI_DEV_ID
          size: 12
          data: 1                       (2nd enet device)

    [4.3] tag: BI_MAC_ADDR
          size: 28
          data: gg:hh:ii:jj:kk:ll       (ascii)
          pad: 3

    [4.4] 0x00000000                    (NULL Termination of BI_STRUCT)

[5] tag: BI_STRUCT                      (embedded enet cltr 2)
    size: 68                            (12 + 2*12 + 28 + 4 == 68)
    data: BI_DEVICE

    [5.0] tag: BI_DEV_TYPE
          size: 12
          data: BI_DEV_TYPE_GT_ETH

    [5.2] tag: BI_DEV_ID
          size: 12
          data: 2                       (3rd enet device)

    [5.3] tag: BI_MAC_ADDR
          size: 28
          data: mm:nn:oo:pp:qq:rr       (ascii)
          pad: 3

    [5.4] 0x00000000                    (NULL Termination of BI_STRUCT)

[6] 0x00000000                          (NULL Termination of whole list)


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

Re: EV-64260-BP & GT64260 bi_recs

[Mark A. Greer]

Forget the last one, here's attempt #3:

Example for ev64260:
--------------------

[0] tag: BI_CMD_LINE
    size: 36                            (4 + 4 + 26 chars + 2 pad)
    data: "console=ttyS0,115200 ip=on"
    pad: 2

[1] tag: BI_MEMSIZE
    size: 12
    data: 33554432                      (32 MB)

[2] tag: BI_GT64260_BASE
    size: 12
    data: fc000000

[3] tag: BI_DEVICE                      (embedded enet cltr 0)
    size: 64                            (8 + 2*12 + 28 + 4 == 64)

    [3.0] tag: BI_DEV_TYPE
          size: 12
          data: BI_DEV_TYPE_GT_ETH

    [3.2] tag: BI_DEV_ID
          size: 12
          data: 0                       (1st enet device)

    [3.3] tag: BI_MAC_ADDR
          size: 28
          data: aa:bb:cc:dd:ee:ff       (ascii)
          pad: 3

    [3.4] 0x00000000                    (NULL Termination of BI_STRUCT)

[4] tag: BI_DEVICE                      (embedded enet cltr 1)
    size: 64                            (8 + 2*12 + 28 + 4 == 64)

    [4.0] tag: BI_DEV_TYPE
          size: 12
          data: BI_DEV_TYPE_GT_ETH

    [4.2] tag: BI_DEV_ID
          size: 12
          data: 1                       (2nd enet device)

    [4.3] tag: BI_MAC_ADDR
          size: 28
          data: gg:hh:ii:jj:kk:ll       (ascii)
          pad: 3

    [4.4] 0x00000000                    (NULL Termination of BI_STRUCT)

[5] tag: BI_DEVICE                      (embedded enet cltr 2)
    size: 64                            (8 + 2*12 + 28 + 4 == 64)

    [5.0] tag: BI_DEV_TYPE
          size: 12
          data: BI_DEV_TYPE_GT_ETH

    [5.2] tag: BI_DEV_ID
          size: 12
          data: 2                       (3rd enet device)

    [5.3] tag: BI_MAC_ADDR
          size: 28
          data: mm:nn:oo:pp:qq:rr       (ascii)
          pad: 3

    [5.4] 0x00000000                    (NULL Termination of BI_STRUCT)

[6] 0x00000000                          (NULL Termination of whole list)


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

Re: EV-64260-BP & GT64260 bi_recs

[benh]

>Forget the last one, here's attempt #3:

Looks fine for me.

So everybody agree around this mecanism ?

Ben.


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

Re: EV-64260-BP & GT64260 bi_recs

[Geert Uytterhoeven]

On Wed, 27 Mar 2002, Mark A. Greer wrote:
> I also adde NULL termination to the overall bi_rec and to each nested one so the
> parsing code can stop when a NULL is hit whether its a nested one or not
> (therefore we can never have a valid tag with value 0).  I also adjusted the size

Don't we have BI_LAST for that?

Note that on m68k and on PPC/APUS, BI_LAST == 0, but on other PPC platforms
BI_LAST = 0x1011.

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds


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

Re: EV-64260-BP & GT64260 bi_recs

[Michael Sokolov]

Mark A. Greer <mgreer@mvista.com> wrote:

> [3] tag: BI_STRUCT                      (embedded enet cltr 0)
>     size: 76                            (12 + 3*12 + 25 + 3 == 76)
>     data: BI_DEVICE
>
>     [3.0] tag: BI_DEV_TYPE
>           size: 12
>           data: BI_DEV_EMBEDDED
>
>     [3.1] tag: BI_DEV_CLASS
>           size: 12
>           data: BI_CLASS_ENET
>
>     [3.2] tag: BI_DEV_ID
>           size: 12
>           data: 0                       (1st enet device)
>
>     [3.3] tag: BI_MAC_ADDR
>           size: 25
>           data: aa:bb:cc:dd:ee:ff       (ascii)
>     pad: 3

[Skipped identical structs for the other two ports]

I disagree. I think BI_GT64260_ETH_CFG as I implemented it is better.

Now if you actually implement your way in code and make it work, I'll accept it
even though I dislike it, but if your above proposal remains verbiage, my patch
must be pushed instead. Unimplemented verbiage must not take precedence over
working code.

MS

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

Re: EV-64260-BP & GT64260 bi_recs

[Mark A. Greer]

Michael Sokolov wrote:

> Now if you actually implement your way in code and make it work, I'll accept it
> even though I dislike it, but if your above proposal remains verbiage, my patch
> must be pushed instead. Unimplemented verbiage must not take precedence over
> working code.

First, we need to see if this is going to fit with everything else in
2_4/2_4_devel and then it will get implemented.

Its not that much work:

1) Make a function (or two) like what's already been talked about to search for
bi_recs of certain types (starting from a specified location so you don't keep
finding the same one).

2) Change parse_bootinfo to use that function but only look up truly generic ones
like BI_CMD_LINE

3) Change the gt64260_enet driver to search for the ones specific to it (and
remove the code for it that is in parse_bootinfo in your patch)

Mark


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

Re: EV-64260-BP & GT64260 bi_recs

[Michael Sokolov]

Troy Benjegerdes <hozer@drgw.net> wrote:

> when I start merging the _galileo tree code to _devel, I'm likely to blow
> your changes away because I can't test them.

If you do I'll scream very loud and make you fix your breakage.

> And PLEASE give me a different eth driver ;)
>
> The one in the tree right now works for single cpus, but
> a) is ugly
> b) blows up very nicely with SMP.

I'll discuss this in a bit.

> The _galileo tree was created to keep us from thrashing in _devel with a
> bunch of crappy (but working) code.
           ^^^^^^^^^^^^^^^^^^^^^^^^^

If you have crappy code, you have to be prepared for someone else having
sparkling beautiful code ready to go to Marcelo, and you have to accept it
being used over your crappy one. (Just check out my patch from my CVS to see
what I mean by sparkling beautiful code. Just compare the EV64260 and EV64260MS
ports.)

> FYI, I am about ready to merge Zuma's gt64260_mpsc driver to 2_4_devel
> since it seems to work reasonably well, and the code is mostly sane. If
> you have another mpsc driver, speak now or forever hold your peace.

I don't have another MPSC driver nor do I have plans to do any MPSC work right
now. At SBS I was planning on making a done-right MPSC driver for 2_4_devel
because their boards all used MPSC, but then I got fired from SBS and my
current paying client doesn't care about MPSC (he's got a UART on his board
just like on the EVB).

MS

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

Re: EV-64260-BP & GT64260 bi_recs

[Michael Sokolov]

Dan Malek <dan@embeddededge.com> wrote:

> The only reason _any_ Ethernet
> information had been passed between the bootloader and the kernel
> was for the processors with internal peripherals and a variety of
> methods that were used for retrieving the MAC address.

Yes, and this is how it should be.

Now not just for Ethernet, but for any peripherals built into the CPU, the
system controller, or the board custom logic we need bi_recs to tell the kernel
how it's wired. Like if some random peripheral chip (not PCI or somesuch, but
"system" type that we have to have hard knowledge of) has a pin that can be
tied high or low that affects its operation and the Linux driver needs to know
how it's tied, we need to have the board firmware pass this magic info via
bi_recs (now it's done either with a board #ifdef maze or .config options). I
don't think there should be .config options for things that are fixed hard in
PCB traces or in silicon, and board #ifdefs are ugly. bi_recs are the answer.
That's why I included is_rmii and phy_addr in BI_GT64260_ETH_CFG.

> There are
> already Linux methods for configuring network interfaces, we don't
> need to add another one.

Exactly. Things like 10 vs. 100 Mbps shouldn't be in bi_recs because they are
up to the user to decide and Linux already provides standard mechanisms for
them. bi_recs should be used only for stuff that's completely involuntary,
i.e., to describe how the board is wired. This way they can be completely
hidden from the user who can't override a PCB trace in software anyway.

MS

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

Re: EV-64260-BP & GT64260 bi_recs

[Dan Malek]

Michael Sokolov wrote:


> ......pass this magic info via
> bi_recs (now it's done either with a board #ifdef maze or .config options). I
> don't think there should be .config options for things that are fixed hard in
> PCB traces or in silicon, and board #ifdefs are ugly. bi_recs are the answer.

I don't agree.  I believe if you can't configure the software in the usual
way with configuration options, you can't do it cleanly with bi_recs, either.
There are pin multiplex configurations on 8xx that you just can't (or shouldn't
due to lots of complexity) define in a dynamic manner.  The purpose of the
current Linux configuration is to create a binary that fits a particular board
and architecture.  Not so long ago, we had great discussions over creating kernels
with lots of dynamic configuration that would boot on multiple boards and how
this wasn't suitable for embedded processors due to limited resources.  We
painstakingly made changes to #defines and rewrote software so the compiler
would apply optimizations to create binaries as small as possible.  I guess no
one remembers that anymore :-).  The config options are exactly for things that
are fixed by board and processor design.  The bi_recs are for only those things
that aren't fixed by board or processor design, like memory size, cpu speed,
MAC address.  I almost have to give into Wolfgang's request for flash details,
but I would still rather have mtd configurations for that :-).

Just take a look at and understand how some of this software has evolved, and
the 8xx is a great example.  Lots of boards use the same processor (and built in
peripherals) and are wired slightly different, but there is still a separate
configuration for all of them and it would be overly complex to not do this.
Yes, you _could_, but I don't see any value in doing so other than proving you
can do it.  It creates code that just consumes more resources.  Once you have
a configuration for the board, the variations are the things I keep mentioning..
...memory size, cpu speed, MAC address, console port baud rate.  It's been working
nicely for years, and it is conceptually consistent with other types of Linux
configuration on other platforms.


	-- Dan


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

Re: EV-64260-BP & GT64260 bi_recs

[Paul Mackerras]

Dan Malek writes:

> I don't agree.  I believe if you can't configure the software in the usual
> way with configuration options, you can't do it cleanly with bi_recs, either.
> There are pin multiplex configurations on 8xx that you just can't (or shouldn't
> due to lots of complexity) define in a dynamic manner.  The purpose of the
> current Linux configuration is to create a binary that fits a particular board
> and architecture.

I don't particularly like the way we define what is on a given
embedded board with what boils down to a shell script.  It seems ugly
and clumsy to me, as does the pile of #defines and #ifdefs that we end
up with as the way for drivers to know where their devices are.

What I would like to see is a readable, compact, clear description of
the hardware (for a given embedded board) in one place, i.e. one file.
Probably not in C code, rather some sort of syntax that can be parsed
with lex/yacc and describe a tree as the main structure but with other
cross-connections as well.

What do we do with such a machine description?  I see three things we
could use it for:

1. Derive a set of config option values (or recommendations for those
   values, at least) for including the necessary kernel drivers to
   support the devices on the board.

2. Make a list of devices and the information that their drivers will
   need about them, such as register base addresses, interrupt
   assignments, etc., for inclusion in the kernel.

3. Make such a list for inclusion in a bootloader so that it can
   supply it to the kernel, for people who aren't so worried about the
   size of their kernels and who want to be able to use the same
   kernel binary on several different boards.

The idea of 2 (and 3) is to make it easy for drivers to find out how
many instances of their device exist, and for each instance, the
critical information that the driver needs about it.  This could be
implemented in various ways.  The simplest would be to have a list in
one place with query functions defined which the drivers would use.
That would be relatively easy to implement but would maybe take up
more bytes than absolutely necessary.

For the situations where every byte is precious, we could get a little
cleverer and have preprocessing code which would scan the driver code
and convert the query function calls into references to an initialized
array, or even into just a constant if there is only one instance of
the device.

This would be needed primarily for on-chip or on-board peripherals
that are not self-describing, i.e. not for PCI devices.  We already
have enumeration and query functions for PCI devices.

Paul.

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

Re: EV-64260-BP & GT64260 bi_recs

[Dan Malek]

Paul Mackerras wrote:

> I don't particularly like the way we define what is on a given
> embedded board with what boils down to a shell script.

Why single out embedded boards?

> What I would like to see is a readable, compact, clear description of
> the hardware (for a given embedded board) in one place,

One of my peeves is all I have ever done with Linux is embedded boards,
for many years, no one seemed to care what I thought would make it
easier, and recently people that have little experience with them
now seem to want to solve all of the problems in one weekend :-).

I'm all for making it better, but I have yet to see anything that
really does.  Everyone has an opinion, few people will ever agree
on the single solution, and all we seem to be doing is swapping
one equally successful (or poor) solution for another.  It's different,
but not better.

> 1. Derive a set of config option values (or recommendations for those
>    values, at least) for including the necessary kernel drivers to
>    support the devices on the board.

The interdependencies among the resources on an intergrated processor
often require cooperation among drivers.  Oh you could probably remove
a few #ifdefs and replace it with addtional overhead of some generic
function calls.  Now, you have moved a few ifdefs into a "common" place,
but along with that you also moved the mental context of the code.
Rather than describing and implementing in the place where I can
understand it (the driver itself), I have to look someplace else and
remember why I am looking there.

> 2. Make a list of devices and the information that their drivers will
>    need about them, such as register base addresses, interrupt
>    assignments, etc., for inclusion in the kernel.

For PowerPC (well, 8xx anyway, I don't know what 4xx looks like these days)
all of this stuff is already defined in a board description configuration
file or in the generic processor files.  IMHO, it isn't going to get
any better than this (and it seems sufficient for other processors).

> 3. Make such a list for inclusion in a bootloader so that it can
>    supply it to the kernel, for people who aren't so worried about the
>    size of their kernels and who want to be able to use the same
>    kernel binary on several different boards.

Bleh.....You have to compile the kernel anyway, why bother complicating
everything by passing information through the bootloader?  We have gone
around and around with this "common binary" before.  How come once we
decide to go one direction (no common binaries), we now have to expend
energy going the other direction again?  Not long ago even you argued
for separate binaries, what happened? (change of business pressure? :-)


> The idea of 2 (and 3) is to make it easy for drivers to find out how
> many instances of their device exist,

For embedded boards you know this at compile/config time, especially
in a production environment.  It only takes a few seconds to rebuild a
kernel these days, so just do it.  Lots of this flexibility was nicer
a half dozen years ago when it took an hour to build a kernel.

> For the situations where every byte is precious,....

Surprisingly, there are still many systems that care about this, and
I'm still trying to understand how a dynamic and complex boot/driver
interface is going to be better than something well defined at compile
time with a trivial boot interface.

> For the situations where every byte is precious, we could get a little
> cleverer and have preprocessing code....

Why have all of this complexity?  For people that understand these
processors and are actively using them, a few #ifdefs and configuration
scripts work just fine.  If you don't understand how the peripherals
interact, share processor resources, and the driver implementations operate,
I don't believe you can guide any configuration tool to the proper
results.

Rather rewrite drivers for the sake of removing some #ifdefs, let's write
a smart configuration tool that understands how to select the proper #ifdefs
and build the kernel.  I thought we were headed this way with CML-2?

I would like to make it better, but making sweeping statements about
a "hardware description" that is going to make my life better doesn't cut it.
The devil is in the details, and unless you live those everyday to see
how people are developing software for real products I don't understand
how you can appreciate these suggestions.

I work on lots of processor and projects other than PowerPC, so I get
to see alternatives to what is done here.  It's interesting how some of
those better ideas are met with such resistance here, in particular if
I have to modify something related to a workstation platform :-).  IMHO,
there are many more important technical challenges to solve than worrying
about replacing some #ifdefs with a complex configuration process.

Thanks.


	-- Dan


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

Re: EV-64260-BP & GT64260 bi_recs

[Paul Mackerras]

Dan Malek writes:

> Paul Mackerras wrote:
>
> > I don't particularly like the way we define what is on a given
> > embedded board with what boils down to a shell script.
>
> Why single out embedded boards?

Two reasons: (a) it seems to be that (for example) most of the if
statements in arch/ppc/config.in are concerned with embedded boards,
and (b) embedded boards seem to have high proportion of devices that
aren't self-describing (by self-describing I mean devices such as PCI
devices where there is a straightforward way of enumerating the
devices and finding out what each device is).

> > What I would like to see is a readable, compact, clear description of
> > the hardware (for a given embedded board) in one place,
>
> One of my peeves is all I have ever done with Linux is embedded boards,
> for many years, no one seemed to care what I thought would make it
> easier, and recently people that have little experience with them
> now seem to want to solve all of the problems in one weekend :-).

So it's no use anyone discussing how to solve the problems?

> > 1. Derive a set of config option values (or recommendations for those
> >    values, at least) for including the necessary kernel drivers to
> >    support the devices on the board.
>
> The interdependencies among the resources on an intergrated processor
> often require cooperation among drivers.  Oh you could probably remove
> a few #ifdefs and replace it with addtional overhead of some generic
> function calls.  Now, you have moved a few ifdefs into a "common" place,

No, I don't want to move ifdefs around, I want to get rid of them
altogether. :)

> but along with that you also moved the mental context of the code.
> Rather than describing and implementing in the place where I can
> understand it (the driver itself), I have to look someplace else and
> remember why I am looking there.

The information about the register addresses and interrupt assignments
isn't (or shouldn't need to be) part of the mental context of driver
code.  Nor should the number of instances of the device.

> > 2. Make a list of devices and the information that their drivers will
> >    need about them, such as register base addresses, interrupt
> >    assignments, etc., for inclusion in the kernel.
>
> For PowerPC (well, 8xx anyway, I don't know what 4xx looks like these days)
> all of this stuff is already defined in a board description configuration
> file or in the generic processor files.  IMHO, it isn't going to get
> any better than this (and it seems sufficient for other processors).

My concern is that the current way of doing things is going to become
increasingly unmanageable as we get more and more chips with different
combinations and permutations of on-board peripherals.

I have this dream that one day we will get to the point where adding
support for a new board only involves adding files to the kernel
tree, with no changes to existing files needed.

> > 3. Make such a list for inclusion in a bootloader so that it can
> >    supply it to the kernel, for people who aren't so worried about the
> >    size of their kernels and who want to be able to use the same
> >    kernel binary on several different boards.
>
> Bleh.....You have to compile the kernel anyway, why bother complicating
> everything by passing information through the bootloader?  We have gone
> around and around with this "common binary" before.  How come once we
> decide to go one direction (no common binaries), we now have to expend
> energy going the other direction again?  Not long ago even you argued
> for separate binaries, what happened? (change of business pressure? :-)

I'll assume that was purely a joke.

> > For the situations where every byte is precious,....
>
> Surprisingly, there are still many systems that care about this, and

Well one would then wonder why they are using PowerPC, not known for
its code density...
(Just kidding :-)

Paul.

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

Re: EV-64260-BP & GT64260 bi_recs

[Tom Rini]

On Tue, Apr 02, 2002 at 03:32:55PM +1000, Paul Mackerras wrote:
>
> Dan Malek writes:
>
> > Paul Mackerras wrote:
> >
> > > I don't particularly like the way we define what is on a given
> > > embedded board with what boils down to a shell script.
> >
> > Why single out embedded boards?
>
> Two reasons: (a) it seems to be that (for example) most of the if
> statements in arch/ppc/config.in are concerned with embedded boards,

That's just because pmac hw is so boring and limited.  That and pmacs
are designed around an OS which depends on the hardware to describe
itself.  That's the main reason it's so easy to do lots of things at
runtime on chrp/pmac, OF even describes the non self-describing devices
for you usually.

> > > 1. Derive a set of config option values (or recommendations for those
> > >    values, at least) for including the necessary kernel drivers to
> > >    support the devices on the board.
> >
> > The interdependencies among the resources on an intergrated processor
> > often require cooperation among drivers.  Oh you could probably remove
> > a few #ifdefs and replace it with addtional overhead of some generic
> > function calls.  Now, you have moved a few ifdefs into a "common" place,
>
> No, I don't want to move ifdefs around, I want to get rid of them
> altogether. :)

Which is either generated code at runtime (which we're doing a bit of
when possible) or runtime, which isn't as desired.

Unless we get the proper symlink magic to do
#include <asm/platform.h>
which will get the right platform from CONFIG_foo.  Which would get a
lot more of your 'just adding files' idea.

> Nor should the number of instances of the device.

That's a different problem, mainly drivers which weren't initially
designed around multiple instances.

> > > 2. Make a list of devices and the information that their drivers will
> > >    need about them, such as register base addresses, interrupt
> > >    assignments, etc., for inclusion in the kernel.
> >
> > For PowerPC (well, 8xx anyway, I don't know what 4xx looks like these days)
> > all of this stuff is already defined in a board description configuration
> > file or in the generic processor files.  IMHO, it isn't going to get
> > any better than this (and it seems sufficient for other processors).
>
> My concern is that the current way of doing things is going to become
> increasingly unmanageable as we get more and more chips with different
> combinations and permutations of on-board peripherals.

There's some cleanup which needs to be done (asm/commproc.h needs some
cleanups, which Wolfgang did at one point and I will push to 2.5 soon).

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

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

Re: EV-64260-BP & GT64260 bi_recs

[Dan Malek]

I'm glad you found _some_ of the humor in it :-)

Paul Mackerras wrote:


> Two reasons: (a) it seems to be that (for example) most of the if
> statements in arch/ppc/config.in are concerned with embedded boards,

Well, yes, and at one point I tried to create config.in files that were
unique to the boards/peripherals that were just callouts from config.in.
That didn't last long because all of these were incorporated (not by me :-)
into config.in and other places.  I thought having these board specific
configuration files was kind of nice, kept arch/ppc/config.in cleaner,
kept the interdependencies localized to a sensible place, but I guess
someone thought different when it got merged from the development tree.

We are always discussing this separation of information and software
among the different platforms.  I truly want it separate....everything.
The drivers, the configuration files, anything unique to a board that
isn't "common" across a wide range of platforms and architectures.

> So it's no use anyone discussing how to solve the problems?

I guess the "problem" is in the eye of the beholder :-).  I don't
see lots of defconfig files or some complexity to config.in, or
a few #ifdefs as a "problem" :-).  These discussions become a
problem for me because I spend all of my time discussing why something
is the way it is, and why that isn't necessarily bad, considering
all of the details involved in development of the software.  Then,
after I lose the battle, a bunch of code is changed, someone discovers
we don't have anything better than we started, and the "geeze I never
thought of that detail" discussions start so we can crawl out of a
hole.  We then start all over again making changes that in the
end just traded one "problem" for another, making little progress
on the quality or features of the software.......In a year or so
I'll bring up this bi_recs discussion again as an example :-)

> No, I don't want to move ifdefs around, I want to get rid of them
> altogether. :)

I don't understand why that in itself has to be a goal.  As we
have more experience with platforms and the use of the software,
these will be change and perhaps go away.  I don't know how you
just declare this goal without having a better idea of how the
software can be placed into reusable modules.

> The information about the register addresses and interrupt assignments
> isn't (or shouldn't need to be) part of the mental context of driver
> code.  Nor should the number of instances of the device.

As far as I know, the interrupts and registers are fixed at compile
time.  The more challenging resources are I/O pin multiplexing and
bus control.  Again, taking the 8xx and 8260 as an example (of a well
thought out integrated processor design :-) you have flexibility that
can be wired at compile time, making the drivers much more simplified
(removing #ifdefs).  In some cases, there is no way for a bootloader
to know this information (unless it's compiled in there, so why bother)
and there are performance trade offs you can make with these configuration
options (so you are building a kernel anyway).

> My concern is that the current way of doing things is going to become
> increasingly unmanageable as we get more and more chips with different
> combinations and permutations of on-board peripherals.

If you are using the 4xx stuff as an example, I don't really like the
way that is being done, but I'm severely outnumbered in that battle so
I had to retreat and work somewhere else (like MIPS :-).  I still
prefer to wait and see how unmanagable it gets before we start reworking
everything.  Again, I don't see how using a bootloader to provide
information and adding more software complexity to drivers solves this
problem.  If you can't make it work with #define constants and #ifdef
code sections, how will changing a #define with a variable name and
an #ifdef with an if () {} make it eaiser?

> I have this dream that one day we will get to the point where adding
> support for a new board only involves adding files to the kernel
> tree, with no changes to existing files needed.

I pretty much live that dream every day with the 8xx stuff.  Give me
a working piece of hardware and with a few file changes I can have
it booting in no time :-)

> I'll assume that was purely a joke.

Not all of it :-)  I thought we blew off trying to do common binaries
long ago.  We try to do common source code as much as possible,
but we had the common binary discussion long ago.  I guess I should
read some of the old archives to remember correctly.

> Well one would then wonder why they are using PowerPC, not known for
> its code density...
> (Just kidding :-)

The code density is similar to any other risc processor, and it has
some pretty nice instructions.  Quite honestly, Motorola knows how
to get embedded processors done right :-)


	-- Dan


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

Re: EV-64260-BP & GT64260 bi_recs

[Armin]

Dan Malek wrote:
>
> If you are using the 4xx stuff as an example, I don't really like the
> way that is being done, but I'm severely outnumbered in that battle so
> I had to retreat and work somewhere else (like MIPS :-).  I still
> prefer to wait and see how unmanagable it gets before we start reworking
> everything.  Again, I don't see how using a bootloader to provide
> information and adding more software complexity to drivers solves this
> problem.  If you can't make it work with #define constants and #ifdef
> code sections, how will changing a #define with a variable name and
> an #ifdef with an if () {} make it eaiser?

What battle would that be?? you're copied on all my changes to 4xx and
if you have a problem with it , you should say so:)

>
>> I have this dream that one day we will get to the point where adding
>> support for a new board only involves adding files to the kernel
>> tree, with no changes to existing files needed.
>

4xx you need 1 config , board.c, board.h and add board.h to ibm4xx.h at
a minimum.  The 4xx drivers when conversion to ocp is completed will
make it possible:) Think of ocp as a static or manual pci driver

>
>     -- Dan
>
>
>
>

armin


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

Re: EV-64260-BP & GT64260 bi_recs

[Tom Rini]

On Tue, Apr 02, 2002 at 12:29:55PM -0500, Dan Malek wrote:
>
> I'm glad you found _some_ of the humor in it :-)
>
> Paul Mackerras wrote:
>
>
> >Two reasons: (a) it seems to be that (for example) most of the if
> >statements in arch/ppc/config.in are concerned with embedded boards,
>
> Well, yes, and at one point I tried to create config.in files that were
> unique to the boards/peripherals that were just callouts from config.in.
> That didn't last long because all of these were incorporated (not by me :-)
> into config.in and other places.  I thought having these board specific
> configuration files was kind of nice, kept arch/ppc/config.in cleaner,
> kept the interdependencies localized to a sensible place, but I guess
> someone thought different when it got merged from the development tree.

It was probably me.  One thing I want to do in the 2.5 tree is make
arch/ppc/platforms/{8xx,4xx}.in and source those from
arch/ppc/config.in, as well as move network things to network Config
files, and so on.

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

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

Re: EV-64260-BP & GT64260 bi_recs

[Dan Malek]

Tom Rini wrote:

> It was probably me.

Heh.....I'm just having some fun :-).  I also know the reason
you probably changed it was to either make munuconfig or
something work, or had plans for CML-2.  I think some of the
confusing stuff we have for configuration is due to the tools
themselves, and that sometimes is reflected in the way the
code (#define and #ifdef) is written.


> ...  One thing I want to do in the 2.5 tree is make
> arch/ppc/platforms/{8xx,4xx}.in and source those from
> arch/ppc/config.in,

I know that would help.  What happened to CML-2?

Thanks.


	-- Dan

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

Re: EV-64260-BP & GT64260 bi_recs

[Tom Rini]

On Tue, Apr 02, 2002 at 04:02:56PM -0500, Dan Malek wrote:
> Tom Rini wrote:
>
> >It was probably me.
>
> Heh.....I'm just having some fun :-).

Ya, I figured :)

> I also know the reason
> you probably changed it was to either make munuconfig or
> something work, or had plans for CML-2.  I think some of the
> confusing stuff we have for configuration is due to the tools
> themselves, and that sometimes is reflected in the way the
> code (#define and #ifdef) is written.

Yes, there are some neat things which might be less feasible, or at
least less of a good idea due to the current tools.

> >...  One thing I want to do in the 2.5 tree is make
> >arch/ppc/platforms/{8xx,4xx}.in and source those from
> >arch/ppc/config.in,
>
> I know that would help.  What happened to CML-2?

I have a feeling that ESR stopped announcing releases of it on l-k since
it wasn't adopted yet, and Linus has decided to fix more important
things than the config system (and ESR refuses to try and transition
from CML1 to CML2 in a manner that can be easily verified to be correct
or stop trying to tackle the Aunt Tillie problem at the same time).

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

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

Re: EV-64260-BP & GT64260 bi_recs

[Michael Sokolov]

Wolfgang Denk <wd@denx.de> wrote:

> I don't  know  the  GT64260  that  well;  what  makes  it's  ethernet
> interface  so  much  different  from all other devices that it cannot
> share a common BI_ETH_CFG?

I tell you what Wolfgang, if you can make a patch counter to mine that does it
your way and that works on GT-64260 and does what's needed there, I'll gladly
accept your solution. But until you do that please let me push my patch through
the maintainers.

I only have bandwidth for GT-64260 right now, not for 8xx or 8260 or anything
else.

MS

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

Re: EV-64260-BP & GT64260 bi_recs

[Michael Sokolov]

Mark A. Greer <mgreer@mvista.com> wrote:

> I see three options:
>
> a) Do it the same was as we're going to do it in 2.5
>
> [...]
>
> b) Do a more advanced bi_rec scheme (like benh's proposal)
>
> c) Do mininimum necessary in 2.4; do it "right" in 2.5

I vote for doing 'c' now and keeping it unchanged for 2.5, 2.6, 2.10, 3.0, and
the eternity.

> Can we focus on this question so we can stop going in circles?  Besides if we
> reach a concensus in a "few days" we can take MS' $50.  :)

Then we have to agree on how many days is a few. And remember, the count
started yesterday at 16:53 PST. Also you have to have not only a consensus, but
a working patch counter to mine, otherwise you have to push mine and I keep my
$50.

MS

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

Re: EV-64260-BP & GT64260 bi_recs

[Dan Malek]

Michael Sokolov wrote:


> I vote for doing 'c' now and keeping it unchanged for 2.5, 2.6, 2.10, 3.0, and
> the eternity.

You should know better than to ask for that :-).  As I have said in the past,
the current interfaces have changed several times and no one but Wolfgang
even noticed.  The changes this time should have been limited in exactly
the same way, so I'm constantly amazed by the amount of discussion and
disagreement.  It used to be no one wanted to work on this stuff (and few
understood how it worked), and now it has taken precidence over real useful
development that needs to be done :-).

We will never reach consensus, and it will always be changing.........


	-- Dan


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

Re: EV-64260-BP & GT64260 bi_recs

[Michael Sokolov]

Tom Rini <trini@kernel.crashing.org> wrote:

> Because you enjoy our conversations? :)

Normally I do, but then we got on the subject of which patches should or
shouldn't be accepted, and I'm not going to discuss that with anyone except the
actual responsible maintainers.

> And then you'll have to go and improve the patch anyhow, if you're going
> to stick to your word. :)

Oh sure, I have a ton of improvements in mind, I just want to do them one step
at a time, i.e., not work on patch N+1 until N is in the tree.

> How is that a step backward?

The steps forward are linuxppc_2_4_devel -> linuxppc_2_4 -> Marcelo -> 2.4
tarballs on kernel.org -> Debian, Yellow Dog, etc. Sidetrack trees like
2_4_galileo are a step backward.

> All of the current galileo work is in that
> tree.

No, a better competing version of the GT-64260 work is in the 2_4_msokolov tree
on my machine.

> And a quick diff of the enet drivers in the two trees shows a
> good deal of differences, many of which aren't just cosmetic.  Isn't it
> more 'natural' to update the most up to date file?

The gt64260_eth driver in both trees is so busted that I plan to rewrite it
from scratch anyway like I already did with the rest of the GT-64260 code. (And
when I do I'll make it vastly superior to yours to convince Paulus/Marcelo to
use it over yours.)

MS

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

Re: EV-64260-BP & GT64260 bi_recs

[Wolfgang Denk]

In message <3C98B189.78A92DFE@mvista.com> you wrote:
>
> I think this is a result of a fundamental difference in views of how much
> work should be put into 2.4 bi_rec's.

Probably.

> a) Do it the same was as we're going to do it in 2.5
> pros:
> -----
> - 2.4 & 2.5 are consistent
> cons:
> -----
> - we don't know what we're doing in 2.5 yet and probably won't for some time
> - its more kernel/driver work in the short term for 2.4
>
> b) Do a more advanced bi_rec scheme (like benh's proposal)
> pros:
> -----
> - more general/flexible than c) below
> - may be less work than a) above (depends on what 2.5 does)
> cons:
> -----
> - a 2.4 solution only (unless its ends up being what 2.5 uses too)
> - more kernel work in short term than c)
>
> c) Do mininimum necessary in 2.4; do it "right" in 2.5
> pros:
> -----
> - minimal up front kernel work in 2.4
> - can get it done "now"
> cons:
> -----
> - a 2.4 solution only
> - less general solution

I've deleted the ppcboot entries from that list; PPCBoot is just  one
of  many boot loaders, and doesn;t need any special handling. We will
adapt with what we have to - one way or another.

> Although the list I provided assumed that c) was the path, I personally have
> no opinion on this.  I just want us to all agree on which path then bang out
> the details.

So how do we come to the agreement? By voting? My vote is b),  hoping
that the result will be good enough to be accepted for 2.5, too.

> Can we focus on this question so we can stop going in circles?  Besides if we

I stop going in cricles here (for this round).

Wolfgang Denk

--
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-4596-87  Fax: (+49)-8142-4596-88  Email: wd@denx.de
"How is this place run - is it an anarchy?"
"No, I wouldn't say so; it is not that well organised..."

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

Re: EV-64260-BP & GT64260 bi_recs

[Val Henson]

On Wed, Mar 20, 2002 at 01:56:09PM -0500, Mark A. Greer wrote:
>
> Wolfgang Denk wrote:
>
> > So how do we come to the agreement? By voting? My vote is b),  hoping
> > that the result will be good enough to be accepted for 2.5, too.
>
> Okay, 1 for b).

Somebody notify Eric Raymond!  New method of Linux development:
maintenance by committee!  I bet Paul is happy to have the
responsibility lifted from him.

I think that this is only slightly crazier than giving more than one
person read/write access to the linuxppc trees.

-VAL

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

Re: EV-64260-BP & GT64260 bi_recs

[Michael Sokolov]

Wolfgang Denk <wd@denx.de> wrote:

> I'm getting a bit tired about this discussion.

I got tired a long time ago.

> We  start  with  siome
> nice  ideas,  then  some open issues pop up, and instead of trying to
> find a solution we drop everything again.
                  ^^^^^^^^^^^^^^^^^^^^^^^^^

No, we don't drop everything again, there is my patch, and Mark said he'll push
it if there is no consensus in the next few days, which I've already bet $50
on.

> For instance. Hey, but we are talking about a very, very special case
> here - when you have set of non-standard controllers which don't know
> their own addresses _and_ need to get a specific assignment _and_ you
> don't know the initialization sequence _and_ ...
>
> For 98% of all practical purposes it will be sufficient to  pass  the
> list of MAC addresses, and for 1.95% it will be sufficient to combine
> the  MAC  address  with  information about the driver type (GT, 82xx,
> ...).

But this is lousy logic, it's this kind of logic (it's OK for 98% of cases)
that has given us the crap we have right now.

Just what is wrong with my solution of BI_GT64260_ETH_CFG, BI_8260_ETH_CFG,
BI_8XX_ETH_CFG, etc?

MS

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

Re: EV-64260-BP & GT64260 bi_recs

[Wolfgang Denk]

In message <0203201711.AA10183@ivan.Harhan.ORG> you wrote:
>
> Just what is wrong with my solution of BI_GT64260_ETH_CFG, BI_8260_ETH_CFG,
> BI_8XX_ETH_CFG, etc?

I don't think you need to pass different information around; in fact,
all these probably need exactly the same information... There  is  no
inherent  difference between the ethernet interfaces on a 8xx or on a
82xx. But both the 8xx and 82xx come with two pretty  much  different
onchip ethernet controllers (SCC/FEC on 8xx, SCC/FCC on 82xx).

So   should   we   have    BI_8XX_SCC_ETH_CFG,    BI_8XX_FCC_ETH_CFG,
BI_8260_SCC_ETH_CFG, BI_8260_FCC_ETH_CFG, ...? Certainly not.

I don't  know  the  GT64260  that  well;  what  makes  it's  ethernet
interface  so  much  different  from all other devices that it cannot
share a common BI_ETH_CFG?

Wolfgang Denk

--
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-4596-87  Fax: (+49)-8142-4596-88  Email: wd@denx.de
"The question of whether a computer can think is no more  interesting
than the question of whether a submarine can swim"
- Edsgar W.  Dijkstra

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

Re: EV-64260-BP & GT64260 bi_recs

[Michael Sokolov]

Tom Rini <trini@kernel.crashing.org> wrote:

> Sure there is, for it to be accepted.

Wait a minute, why am I even talking to you? Mark said he is the responsible
maintainer and promised to push my patch in a few days if there is no consensus
by then, and I think I'll just wait a few days and then ask him to stick to his
word. (I'm willing to bet $50 that in the next few days there will be no
consensus nor will anyone else offer a counterpatch.)

> And would you please make it
> versus the _galileo tree

Sorry, no, that would be a step backward and we need to move forward.

MS

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

Re: EV-64260-BP & GT64260 bi_recs

[Tom Rini]

On Wed, Mar 20, 2002 at 09:04:35AM -0800, Michael Sokolov wrote:
> Tom Rini <trini@kernel.crashing.org> wrote:
>
> > Sure there is, for it to be accepted.
>
> Wait a minute, why am I even talking to you?

Because you enjoy our conversations? :)

> Mark said he is the responsible
> maintainer and promised to push my patch in a few days if there is no
> consensus by then, and I think I'll just wait a few days and then ask
> him to stick to his word.

And then you'll have to go and improve the patch anyhow, if you're going
to stick to your word. :)

> (I'm willing to bet $50 that in the next few days there will be no
> consensus nor will anyone else offer a counterpatch.)
>
> > And would you please make it
> > versus the _galileo tree
>
> Sorry, no, that would be a step backward and we need to move forward.

How is that a step backward?  All of the current galileo work is in that
tree.  And a quick diff of the enet drivers in the two trees shows a
good deal of differences, many of which aren't just cosmetic.  Isn't it
more 'natural' to update the most up to date file?

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

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

Re: EV-64260-BP & GT64260 bi_recs

[Troy Benjegerdes]

On Wed, Mar 20, 2002 at 09:04:35AM -0800, Michael Sokolov wrote:
>
> Tom Rini <trini@kernel.crashing.org> wrote:
>
> > Sure there is, for it to be accepted.
>
> Wait a minute, why am I even talking to you? Mark said he is the responsible
> maintainer and promised to push my patch in a few days if there is no consensus
> by then, and I think I'll just wait a few days and then ask him to stick to his
> word. (I'm willing to bet $50 that in the next few days there will be no
> consensus nor will anyone else offer a counterpatch.)

Well, If I remember right, Mark doesn't even have a galileo board anymore
(someone else got it). So I could probably make an argument I'm the
maintainer now.

>
> > And would you please make it
> > versus the _galileo tree
>
> Sorry, no, that would be a step backward and we need to move forward.

I strongly suggest you submit me (or the -galileo) list patches, otherwise
when I start merging the _galileo tree code to _devel, I'm likely to blow
your changes away because I can't test them.

And PLEASE give me a different eth driver ;)

The one in the tree right now works for single cpus, but
a) is ugly
b) blows up very nicely with SMP.


The _galileo tree was created to keep us from thrashing in _devel with a
bunch of crappy (but working) code.

FYI, I am about ready to merge Zuma's gt64260_mpsc driver to 2_4_devel
since it seems to work reasonably well, and the code is mostly sane. If
you have another mpsc driver, speak now or forever hold your peace.

--
Troy Benjegerdes | master of mispeeling | 'da hozer' |  hozer@drgw.net
-----"If this message isn't misspelled, I didn't write it" -- Me -----
"Why do musicians compose symphonies and poets write poems? They do it
because life wouldn't have any meaning for them if they didn't. That's
why I draw cartoons. It's my life." -- Charles Schulz

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

Re: EV-64260-BP & GT64260 bi_recs

[Mark A. Greer]

Troy Benjegerdes wrote:

> Well, If I remember right, Mark doesn't even have a galileo board anymore
> (someone else got it). So I could probably make an argument I'm the
> maintainer now.

Mine broke.  I'm supposed to get one yesterday/today/tomorrow.  No FedEx guy so far
today...


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

Re: EV-64260-BP & GT64260 bi_recs

[Jim Potter]

Now that things are settling down a bit -- who's got which tree running on which
platform and in what condition?  Basically, what's the conditon of the 2_4_galileo and
2_4_devel tree's on EVB, MVP, (etc) -- SMP y/n, PCI cards (scsi, ide, etc) y/n,
running solid y/n, and under which boot environment (dink, ppcboot, starmon, etc)?


> Troy Benjegerdes wrote:
>
> > Well, If I remember right, Mark doesn't even have a galileo board anymore
> > (someone else got it). So I could probably make an argument I'm the
> > maintainer now.
>
> Mine broke.  I'm supposed to get one yesterday/today/tomorrow.  No FedEx guy so far
> today...

--
Sincerely,

Jim Potter
45th Parallel Processing


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

Re: EV-64260-BP & GT64260 bi_recs

[Wolfgang Denk]

In message <20020320164025.31626@mailhost.mipsys.com> you wrote:
...
> The basic idea was to have bi_recs containing bi_recs, with some
> type/value pairs. something like:
...
> In the above example, the entire BI_STRUCT is a single record
> containing other records. A BI_STRUCT contains a first u32 indicating
> the structure type and then it's sub-records.

Thanks, Ben! This really helps this discussion.

> I also want to have the bi_rec types & records be defined as 4
> ASCII chars in an u32, making things readable when you debug,
> and defining, a bit like Apple does in MacOS, that full lowercase
> letters are reserved types, while others are free for use by vendors
> to pass whatever additional infos, thus a guarantee of avoiding
> clashes between vendor added bi_rec's and future kernels.

Sounds good to me :)

Wolfgang Denk

--
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-4596-87  Fax: (+49)-8142-4596-88  Email: wd@denx.de
Quantum particles: The dreams that stuff is made of.

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

Re: EV-64260-BP & GT64260 bi_recs

[Wolfgang Denk]

In message <20020320155551.GD3762@opus.bloom.county> you wrote:
>
> Well, I do think the idea of a generic BI_ETH_DEVICE was killed.  Unless
> you can work out how to deal with the multiple driver issue above.

I'm getting a bit tired about this discussion. We  start  with  siome
nice  ideas,  then  some open issues pop up, and instead of trying to
find a solution we drop everything again. I think I rather lean  back
and  wait for the next round of this discussin in about 2 or 3 months
from now.

We are just wasting time. it seems.

> Well if this works now, why wouldn't it work later?

It works now because we usually have only a single network interface,
and there is a single MAC address entry in bd_info. There are  a  few
systems which have more interfaces (with a strong tendency that their
number  is  growing) and they use a private (non-standard) version of
bd_info which is incompatible with anything else. Or  they  use  some
hard-wired mechanism to compute the additional MAC addresses from the
first one which is usually not acceptable.

> > I am not aware of something that looks like an acceptable solution to
> > me.
>
> So you don't like the patch you made or you don't recall the patch I'm
> talking about?

I'm not really sure which patch you are referring to; I think I  know
which one you mean, and that one was ugly ;-)

> I don't see how an index helps (enet drivers are in a 'random' order,
> normally) and some bytes for specific info just means there's some bytes
> there.  Are you saying the first set of bytes we toss in say this is
> driver X ?

For instance. Hey, but we are talking about a very, very special case
here - when you have set of non-standard controllers which don't know
their own addresses _and_ need to get a specific assignment _and_ you
don't know the initialization sequence _and_ ...

For 98% of all practical purposes it will be sufficient to  pass  the
list of MAC addresses, and for 1.95% it will be sufficient to combine
the  MAC  address  with  information about the driver type (GT, 82xx,
...).

> > So far all 824x systems we (DENX) have seen  looked  like  "standard"
> > PCI devices, using standard device drivers under Linux.
>
> Er, you lost me there.  Either you're saying it's using "standard" chips
> (tulip, eepro, et al), or (and I don't recall if 824x has it's own
> special enet like 8260) the enet device shows up normally on the PCI bus
> and you've got drivers for it..

They are using hardware that looks like standard chips to the system.

> But either way it's sounding like it's not a problem now..

Right.

The biggest problem to me is a couple of 8260 systems with  dedicated
assigment of MAC addresses to several ethernet interfaces.

Wolfgang Denk

--
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-4596-87  Fax: (+49)-8142-4596-88  Email: wd@denx.de
Quote from the Boss after overriding the decision of a task force  he
created  to  find  a  solution:  "I'm  sorry  if  I ever gave you the
impression your input would have any effect on my  decision  for  the
outcome of this project!"

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

Re: EV-64260-BP & GT64260 bi_recs

[Wolfgang Denk]

In message <20020320150119.GB3762@opus.bloom.county> you wrote:
>
> > Has been shot down? When? Where? Why? Am I missing something?
>
> You were CC'ed.   One problem with a generic record is that what happens
> when you have multiple drivers (8260 + gt-64260, for example).  Also Dan
> Malek pointed out that most of the information (not all) should be up to
> the driver (or userland) to handle.

We talked about problems, yes. But  that  doesn't  usually  mean  the
whole idea gets dropped, or does it?

I really like to be able to configure the network interface(s)  of  a
memory-restricted  embedded  system  using  the IP-autoconfig feature
just by passing it a "ip=..." command line agument.  This  saves  (1)
the  memory  for tools like ifconfig etc in the application image and
(2) the need to  modify  the  application  when  the  network  config
changes.

> > We need to deal with boards with more than  one  ethernet  interface,
> > which  are already active in the firmware (net-booting from redundand
> > interfaces, using separate MAC addresses).
>
> Well, you had a patch awhile ago which allowed for this to be
> configured, yes?

I am not aware of something that looks like an acceptable solution to
me.

> > We added an index field to BI_ETH_CFG, didn't we?  The  driver  would
> > then "know" how to map this...
>
> How would it "know" which ones it can grab?  Ben said the driver should
> do something like find_bi_rec(BI_XXX_ETH_CFG), which would presumbly
> return NULL or >= 1 set of bi_recs of BI_XXX_ETH_CFG.  Then you get the
> 1st and 2nd GT64260 enet devices, and say the 1st and 2nd 8260 enet
> devices, and whichever drivers registers first gets eth0+eth1 and the
> other eth2+eth3.

...which may be perfectly fine in many situations. If you need a more
selective assignment, there was the proposal to add some  index  (me)
or  "some bytes for driver specific info (e.g., on gt64260, is_rmii)"
(Mark A. Greer) which could be easily used to mark a bi_rec as  [not]
acceptable by a certain ethernet driver.


> > We _do_ care, a little for 8xx, very much for 8260, also for 824x and
>
> We also need to fix the drivers for these at the same time.  Do we
> actually make use of special 824x enet right now?  I think one of the

So far all 824x systems we (DENX) have seen  looked  like  "standard"
PCI devices, using standard device drivers under Linux.

> other problems we'll run into is some of the rough edges of 8xx and 8260
> support...

Right. Some work will be necessary.

Wolfgang Denk

--
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-4596-87  Fax: (+49)-8142-4596-88  Email: wd@denx.de
In an infinite universe all things are possible, including the possi-
bility that the universe does not exist.
                        - Terry Pratchett, _The Dark Side of the Sun_

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

Re: EV-64260-BP & GT64260 bi_recs

[Michael Sokolov]

Mark A. Greer <mgreer@mvista.com> wrote:

> Man, can I ever turn a quiet email day into a hurricane!!  :)

Ever heard the term filibuster? That's what I think happening here: lots of
talk, no work. Also note that so far I'm the only one who has posted a patch,
the rest is verbiage.

> Point well taken and you're right, perhaps the cmd_line is a better way to
> pass in hdx/fdx & speed, phy addr & is_rmii.

The PHY address and MII vs. RMII are things fixed by board circuitry and would
normally be hard-coded. The only reason I added them to BI_GT64260_ETH_CFG was
because I imagined the #ifdef maze that would otherwise eventually grow in the
gt64260_eth driver as people add ports to different boards with these things
wired differently.

As for speed, etc. no other driver I know of requires the bootloader to pass
this info. It's normally set by ethconfig, miitool, etc. Please don't add that
in there, I have no slightest idea what to put in those fields if they were in
the bi_rec.

MS

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

Re: EV-64260-BP & GT64260 bi_recs

[Michael Sokolov]

Mark A. Greer <mgreer@mvista.com> wrote:

> I like what Michael has done but want to add more and change the
> BI_GT64260_ETH_CFG to a more generic BI_ETH_CFG.

I disagree. I don't see how a generic BI_ETH_CFG is possible. See how I've
implemented BI_GT64260_ETH_CFG in arch/ppc/kernel/setup.c:parse_bootinfo: it
injects the information from this record directly into the gt64260_eth driver,
which is where this information is needed.

Next when someone wants to do the same for, say, 8260 Ethernet, they'll have to
add BI_8260_ETH_CFG or whatever. I don't see any other way: what if I built a
board with an 8260 CPU and a GT-64260 attached to the 60x bus coming out of the
8260 (such a board was in the works at SBS before I left) and both 8260 and GT
Ethernets are used? What will parse_bootinfo() do with a BI_ETH_CFG then? Is it
going to inject the MAC address into the 8260 Ethernet driver, into the GT
Ethernet driver, or where? How do you make sure that each Ethernet interface
gets the MAC address that belongs to it?

MS

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

Re: EV-64260-BP & GT64260 bi_recs

[Mark A. Greer]

Michael Sokolov wrote:

> Mark A. Greer <mgreer@mvista.com> wrote:
>
> > I like what Michael has done but want to add more and change the
> > BI_GT64260_ETH_CFG to a more generic BI_ETH_CFG.
>
> I disagree. I don't see how a generic BI_ETH_CFG is possible. See how I've
> implemented BI_GT64260_ETH_CFG in arch/ppc/kernel/setup.c:parse_bootinfo: it
> injects the information from this record directly into the gt64260_eth driver,
> which is where this information is needed.

My point is that "you way" would change to a more generic way for all enet driver.

The BI_ETH_CFG has already been shot down so we don't need to worry about it
anyway.

> Next when someone wants to do the same for, say, 8260 Ethernet, they'll have to
> add BI_8260_ETH_CFG or whatever. I don't see any other way: what if I built a
> board with an 8260 CPU and a GT-64260 attached to the 60x bus coming out of the
> 8260 (such a board was in the works at SBS before I left) and both 8260 and GT
> Ethernets are used? What will parse_bootinfo() do with a BI_ETH_CFG then? Is it
> going to inject the MAC address into the 8260 Ethernet driver, into the GT
> Ethernet driver, or where? How do you make sure that each Ethernet interface
> gets the MAC address that belongs to it?

This is a good point and this is an issue with any generic ethernet bi_rec
scheme.  At this point, I'm back to preferring what you've done since...well...its
done and I don't see anyone else caring much about this for other drivers.

Mark


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

Re: EV-64260-BP & GT64260 bi_recs

[Wolfgang Denk]

In message <3C97C301.B5A95C8B@mvista.com> you wrote:
>
>
> The BI_ETH_CFG has already been shot down so we don't need to worry about it
> anyway.

Has been shot down? When? Where? Why? Am I missing something?

So how are we going to solve this issue - in 2.4 and 2.5?

We need to deal with boards with more than  one  ethernet  interface,
which  are already active in the firmware (net-booting from redundand
interfaces, using separate MAC addresses).

> > Ethernet driver, or where? How do you make sure that each Ethernet interface
> > gets the MAC address that belongs to it?

We added an index field to BI_ETH_CFG, didn't we?  The  driver  would
then "know" how to map this...

> This is a good point and this is an issue with any generic ethernet bi_rec
> scheme.  At this point, I'm back to preferring what you've done since...well...its
> done and I don't see anyone else caring much about this for other drivers.

We _do_ care, a little for 8xx, very much for 8260, also for 824x and
4xx.

Wolfgang Denk

--
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-4596-87  Fax: (+49)-8142-4596-88  Email: wd@denx.de
Fools ignore complexity. Pragmatists suffer it. Some  can  avoid  it.
Geniuses remove it.
- Perlis's Programming Proverb #58, SIGPLAN Notices, Sept.  1982

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

Re: EV-64260-BP & GT64260 bi_recs

[benh]

>
>I disagree. I don't see how a generic BI_ETH_CFG is possible. See how I've
>implemented BI_GT64260_ETH_CFG in arch/ppc/kernel/setup.c:parse_bootinfo: it
>injects the information from this record directly into the gt64260_eth
driver,
>which is where this information is needed.

This is the wrong approach. What we should do instead is have the gt eth
driver
do some kind of find_bi_rec(BI_GT64260_ETH_CFG). setup.c doesn't have to be
changed each time a new birec is added, and your approach seem wrong if
that driver ever becomes a module.

What I have not yet decided regarding what to do of birec's in 2.5.
That is either providing some structure to them so they represent devices
with all the mapping & routing information needed (flexible enough to
accomodate
most embedded needs), that is some kind of lightweight device-tree, or if
we just
stay to what we have now (minus bd_t), that is everybody defines it's own
set of
bi_recs.

I've started writing a draft of what the first option could be last year, I'll
see if I can still find it and post it for you guys to fight over some
material ;)

The _important_ point however is that the common arch code and the common
CPU code mustn't have to care (or be modified) when a bi_rec is added and
then later used by whatever driver you want to pass it to.


Ben.


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

Re: EV-64260-BP & GT64260 bi_recs

[Michael Sokolov]

benh@kernel.crashing.org wrote:

> This is the wrong approach. What we should do instead is have the gt eth
> driver
> do some kind of find_bi_rec(BI_GT64260_ETH_CFG). setup.c doesn't have to be
> changed each time a new birec is added, and your approach seem wrong if
> that driver ever becomes a module.

Then push my patch defining the new bi_recs, and once it's in I'll make another
one to implement what you just suggested. But if patches aren't being accepted
there is no incentive for me to make or improve them.

MS

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

Re: EV-64260-BP & GT64260 bi_recs

[Tom Rini]

On Wed, Mar 20, 2002 at 07:30:38AM -0800, Michael Sokolov wrote:
>
> benh@kernel.crashing.org wrote:
>
> > This is the wrong approach. What we should do instead is have the gt eth
> > driver
> > do some kind of find_bi_rec(BI_GT64260_ETH_CFG). setup.c doesn't have to be
> > changed each time a new birec is added, and your approach seem wrong if
> > that driver ever becomes a module.
>
> Then push my patch defining the new bi_recs, and once it's in I'll make another
> one to implement what you just suggested. But if patches aren't being accepted
> there is no incentive for me to make or improve them.

Sure there is, for it to be accepted.  And would you please make it
versus the _galileo tree so that other users of GT-64260 can easily test
your changes?

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

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

Re: EV-64260-BP & GT64260 bi_recs

[benh]

Ok, here is my older document, though now that I re-read it, I find
it rather too bloated. Food for though...

Ben.

/* Here's a try at defining new birecs a bit more
 *
 * Of course, most of this is _optional_, OF based machines
 * won't use but a few of these, embedded developers may use
 * and customize these, we will try to make embedded specific
 * driver rely solely on defined bi_recs....
 *
 * Note. The high 8 bits of the size field contains a type code
 * for the bi_rec. The types are defined below. The composite
 * type allow embedding of "properties", in that case the birec
 * tag is considered as a "name". For records of type meminfo,
 * bootinfo, and cpuinfo, the tag identify the information passed.
 * All lowercase tags are reserved for future use by the kernel,
 * All uppercase or mixed case tags can be freely used by board
 * implementors for custom applications.
 */

struct bi_record {
    unsigned long tag;		/* tag ID */
    unsigned long size;		/* size of record (in bytes) */
    unsigned long data[0];	/* data */
};

#define BI_SIZEMASK		0x00ffffff
#define BI_TYPEMASK		0xff000000

#define BI_TYPE_FIRST		0x01000000
#define BI_TYPE_CPUINFO		0x02000000	/* single values, fixed type */
#define BI_TYPE_BOARDINFO	0x03000000	/* single values, fixed type */
#define BI_TYPE_MEMINFO		0x04000000	/* physical memory layout */
#define BI_TYPE_BOOTINFO	0x05000000	/* boot infos (cmdline, ...) */
#define BI_TYPE_COMPOSITE	0x42000000	/* more birecs after a bi_composite */
#define BI_TYPE_SINGLE_MASK	0x80000000	/* single value (no size, value in
size field */
#define BI_TYPE_END_COMPOSITE	0xfe000000
#define BI_TYPE_LAST		0xff000000

/*
 * CPU Info
 */
#define BI_CPU_4xx_CORE_CLOCK	'cclk'		/* Core clock of a 4xx (ulong, Hz) */
#define BI_CPU_4xx_PLB_CLOCK	'pclk'		/* PLB clock of a 4xx (ulong, Hz) */
#define BI_CPU_4xx_OPB_CLOCK	'oclk'		/* OPB clock of a 4xx (ulong, Hz) */
#define BI_CPU_4xx_PCI_CLOCK	'iclk'		/* PCI clock of a 4xx (ulong, Hz) */

/*
 * Board info
 */
#define BI_BOARD_NAME		'name'		/* 0 term string */
#define BI_BOARD_MACHINE_TYPE	'mach'		/* Machine type (common config) */
#define BI_BOARD_MACHINE_MODEL	'modl'		/* Model within machine type */
#define BI_BOARD_MACHINE_FAMILY	'fami'		/* Familly within machine type */

#define BI_BOARD_SERIALNUMBER	'sern'		/* Why not ? 0 term string */

	/* To be defined, or specific to a given board */

/*
 * Mem info
 */
#define BI_MEM_TOTALMEM		'totm'		/* Total memory */
#define BI_MEM_PHYSMAP		'phym'		/* Physmap, contiguous @ 0 if absent */
#define BI_MEM_DIMMMAP		'dimm'		/* Why not ? ... */

/*
 * Boot info
 */
#define BI_BOOT_CMDLINE		'cmdl'		/* Command line (0 term string) */
#define BI_BOOT_INITRD		'rimg'		/* Initial ramdisk */
	struct bi_boot_initrd {
		unsigned long	addr;		/* Physical address */
		unsigned long	csize;		/* Compressed size */
		unsigned long	msize;		/* Memory size (max rd driver size) */
	};
	/* Note: we may want to define a format for scattered ramdisks */
#define BI_BOOT_ROOT_DEVICE	'root'		/* Major & minor of root device in
2x32 bits ! */

/*
 * Composite record format is a suite of bi_recs finishing
 * with a BI_TYPE_END_COMPOSITE.
 * They have a "name" which is the tag of the composite
 * record, and they begin with a "type" long. Then, is
 * a suite of bi_recs.
 */
struct bi_composite {
    unsigned long type_tag;		/* type tag */
    unsigned long data[0];		/* more birecs */
};

/* Here are some defined type tags */

#define BI_COMP_DEVGROUP	'dgrp'	/* A group of devices (more composite) */
#define BI_COMP_DEVICE		'devi'	/* Generic device */
#define BI_COMP_PCI_HOST	'phst'	/* PCI host (contains it's devices !) */
#define BI_COMP_PCI_DEVICE	'pdev'	/* PCI device */
#define BI_COMP_CPU_DEVICE	'pdev'	/* CPU embedded device */
#define BI_COMP_IRQ_NODE	'irqn'	/* Interrupt node (controller or bridge) */

/*
 * An interrupt node defines interrupt routing.
 */

/* To be written ;) */


/*
 * A device has those generic properties defined.
 */

/* An IO resource is an abstract entity to designate a memory
 * or IO region used by a device. The exact meaning of those
 * fields depends on the atualy type of device (the bus where
 * resides). A generic device is meant to be used for your own
 * motherboard devices, you can use the values below the way
 * you want, each driver defines it's own meaning there.
 * It wouldn't be efficient to provide more abstraction imho.
 */
#define BI_PROP_IO_RESOURCE		'iors'
struct bi_prop_io_resource {
	unsigned long	base_addr;
	unsigned long	size;
	unsigned long	flags;
};

/* An interrupt is designated by a parent node ID and a
 * number. The parent node structure will be then define
 * routing informations. If the interrupt is wired to the
 * CPU's internal interrupt controller directly, it's parent
 * node ID is 0, and the number represents an interrupt number
 * in the native CPU numbering.
 * Additionally, flags are added to the interrupt definition.
 * Only 2 bits are currently defined for those, the high 16
 * bits are free to be used by the implementor of a given board.
 */
#define BI_PROP_INTERRUPT		'irq '
struct bi_prop_interrupt {
	unsigned long	parent;		/* tagID of the parent node */
	unsigned long	number;		/* Interrupt number */
	unsigned long	flags;		/* see below */
};
#define BI_IRQ_LEVEL	0x00000000	/* bit 0 = 0 */
#define BI_IRQ_EDGE	0x00000001	/* bit 0 = 1 */
#define BI_IRQ_NEG	0x00000000	/* bit 1 = 0 */
#define BI_IRQ_POS	0x00000002	/* bit 1 = 1 */

/* The device type (or class), mostly for information purpose for now */
#define BI_PROP_DEVICE_CLASS		'clas'	/* ulong */
#define  BI_DEV_CLASS_ETHERNET		'eth '
#define  BI_DEV_CLASS_UART		'uart'
#define  BI_DEV_CLASS_SCC		'scc '
	/* etc... */

/* The driver is used to define "standard" type of devices, like
 * PC-like uart. Note that devices defined below rely on a specific
 * format of the io resources and eventual additional properties
 */
#define BI_PROP_DEVICE_DRIVER		'drvr' /* ulong */

 /* define required properties of legacy serial here */

/*
 * Properties relative to PCI hosts
 */

  /* define bus numbers, and bases here along with a type
   * property indicating the host type, a generic type can
   * be used for indirect
   */

/*
 * Properties relative to PCI devices. This includes the
 * above properties (all of them eventually). The IO resource
 * property additionally defines some flags. The actual mapping
 * of the host bridge (ISA, etc...) isn't defined here but is
 * defined
 */
#define BI_PROP_PCI_DEVFN		'dvfn' /* ulong */
#define BI_PROP_PCI_VENDORID		'vid ' /* Do we need that here ? */
#define BI_PROP_PCI_DEVICEID		'did ' /* Do we need that here ? */

/* Note: I appologize for bit numbering ;) */


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

Re: EV-64260-BP & GT64260 bi_recs

[Val Henson]

Thanks for writing this up, Ben!  My main objection is that I really
don't see any reason to make bi_recs more complicated than:

record type
size
data

What added functionality does the whole "structure" concept give us?
Here's the current bi_rec parsing code:

while (rec->tag != BI_LAST) {
  /* fiddle with each individual record */
  rec = (struct bi_record *)((ulong)rec + rec->size);
}

The simplicity of this system is valuable - I don't want to give it up
unless we get lots and lots of added functionality in return.  Dan
Malek's comments suggest that bi_recs should only be used for only a
small, simple class of information.  In this context, I don't see what
structures buy us.

I have to admit, bi_rec structures are a really cool idea, but I
prefer simplicity over coolness. :)

-VAL

On Wed, Mar 20, 2002 at 05:58:25PM +0100, benh@kernel.crashing.org wrote:
>
> Ok, here is my older document, though now that I re-read it, I find
> it rather too bloated. Food for though...
>
> Ben.
>
> /* Here's a try at defining new birecs a bit more
>  *
>  * Of course, most of this is _optional_, OF based machines
>  * won't use but a few of these, embedded developers may use
>  * and customize these, we will try to make embedded specific
>  * driver rely solely on defined bi_recs....
>  *
>  * Note. The high 8 bits of the size field contains a type code
>  * for the bi_rec. The types are defined below. The composite
>  * type allow embedding of "properties", in that case the birec
>  * tag is considered as a "name". For records of type meminfo,
>  * bootinfo, and cpuinfo, the tag identify the information passed.
>  * All lowercase tags are reserved for future use by the kernel,
>  * All uppercase or mixed case tags can be freely used by board
>  * implementors for custom applications.
>  */
>
> struct bi_record {
>     unsigned long tag;		/* tag ID */
>     unsigned long size;		/* size of record (in bytes) */
>     unsigned long data[0];	/* data */
> };
>
> #define BI_SIZEMASK		0x00ffffff
> #define BI_TYPEMASK		0xff000000
>
> #define BI_TYPE_FIRST		0x01000000
> #define BI_TYPE_CPUINFO		0x02000000	/* single values, fixed type */
> #define BI_TYPE_BOARDINFO	0x03000000	/* single values, fixed type */
> #define BI_TYPE_MEMINFO		0x04000000	/* physical memory layout */
> #define BI_TYPE_BOOTINFO	0x05000000	/* boot infos (cmdline, ...) */
> #define BI_TYPE_COMPOSITE	0x42000000	/* more birecs after a bi_composite */
> #define BI_TYPE_SINGLE_MASK	0x80000000	/* single value (no size, value in
> size field */
> #define BI_TYPE_END_COMPOSITE	0xfe000000
> #define BI_TYPE_LAST		0xff000000
>
> /*
>  * CPU Info
>  */
> #define BI_CPU_4xx_CORE_CLOCK	'cclk'		/* Core clock of a 4xx (ulong, Hz) */
> #define BI_CPU_4xx_PLB_CLOCK	'pclk'		/* PLB clock of a 4xx (ulong, Hz) */
> #define BI_CPU_4xx_OPB_CLOCK	'oclk'		/* OPB clock of a 4xx (ulong, Hz) */
> #define BI_CPU_4xx_PCI_CLOCK	'iclk'		/* PCI clock of a 4xx (ulong, Hz) */
>
> /*
>  * Board info
>  */
> #define BI_BOARD_NAME		'name'		/* 0 term string */
> #define BI_BOARD_MACHINE_TYPE	'mach'		/* Machine type (common config) */
> #define BI_BOARD_MACHINE_MODEL	'modl'		/* Model within machine type */
> #define BI_BOARD_MACHINE_FAMILY	'fami'		/* Familly within machine type */
>
> #define BI_BOARD_SERIALNUMBER	'sern'		/* Why not ? 0 term string */
>
> 	/* To be defined, or specific to a given board */
>
> /*
>  * Mem info
>  */
> #define BI_MEM_TOTALMEM		'totm'		/* Total memory */
> #define BI_MEM_PHYSMAP		'phym'		/* Physmap, contiguous @ 0 if absent */
> #define BI_MEM_DIMMMAP		'dimm'		/* Why not ? ... */
>
> /*
>  * Boot info
>  */
> #define BI_BOOT_CMDLINE		'cmdl'		/* Command line (0 term string) */
> #define BI_BOOT_INITRD		'rimg'		/* Initial ramdisk */
> 	struct bi_boot_initrd {
> 		unsigned long	addr;		/* Physical address */
> 		unsigned long	csize;		/* Compressed size */
> 		unsigned long	msize;		/* Memory size (max rd driver size) */
> 	};
> 	/* Note: we may want to define a format for scattered ramdisks */
> #define BI_BOOT_ROOT_DEVICE	'root'		/* Major & minor of root device in
> 2x32 bits ! */
>
> /*
>  * Composite record format is a suite of bi_recs finishing
>  * with a BI_TYPE_END_COMPOSITE.
>  * They have a "name" which is the tag of the composite
>  * record, and they begin with a "type" long. Then, is
>  * a suite of bi_recs.
>  */
> struct bi_composite {
>     unsigned long type_tag;		/* type tag */
>     unsigned long data[0];		/* more birecs */
> };
>
> /* Here are some defined type tags */
>
> #define BI_COMP_DEVGROUP	'dgrp'	/* A group of devices (more composite) */
> #define BI_COMP_DEVICE		'devi'	/* Generic device */
> #define BI_COMP_PCI_HOST	'phst'	/* PCI host (contains it's devices !) */
> #define BI_COMP_PCI_DEVICE	'pdev'	/* PCI device */
> #define BI_COMP_CPU_DEVICE	'pdev'	/* CPU embedded device */
> #define BI_COMP_IRQ_NODE	'irqn'	/* Interrupt node (controller or bridge) */
>
> /*
>  * An interrupt node defines interrupt routing.
>  */
>
> /* To be written ;) */
>
>
> /*
>  * A device has those generic properties defined.
>  */
>
> /* An IO resource is an abstract entity to designate a memory
>  * or IO region used by a device. The exact meaning of those
>  * fields depends on the atualy type of device (the bus where
>  * resides). A generic device is meant to be used for your own
>  * motherboard devices, you can use the values below the way
>  * you want, each driver defines it's own meaning there.
>  * It wouldn't be efficient to provide more abstraction imho.
>  */
> #define BI_PROP_IO_RESOURCE		'iors'
> struct bi_prop_io_resource {
> 	unsigned long	base_addr;
> 	unsigned long	size;
> 	unsigned long	flags;
> };
>
> /* An interrupt is designated by a parent node ID and a
>  * number. The parent node structure will be then define
>  * routing informations. If the interrupt is wired to the
>  * CPU's internal interrupt controller directly, it's parent
>  * node ID is 0, and the number represents an interrupt number
>  * in the native CPU numbering.
>  * Additionally, flags are added to the interrupt definition.
>  * Only 2 bits are currently defined for those, the high 16
>  * bits are free to be used by the implementor of a given board.
>  */
> #define BI_PROP_INTERRUPT		'irq '
> struct bi_prop_interrupt {
> 	unsigned long	parent;		/* tagID of the parent node */
> 	unsigned long	number;		/* Interrupt number */
> 	unsigned long	flags;		/* see below */
> };
> #define BI_IRQ_LEVEL	0x00000000	/* bit 0 = 0 */
> #define BI_IRQ_EDGE	0x00000001	/* bit 0 = 1 */
> #define BI_IRQ_NEG	0x00000000	/* bit 1 = 0 */
> #define BI_IRQ_POS	0x00000002	/* bit 1 = 1 */
>
> /* The device type (or class), mostly for information purpose for now */
> #define BI_PROP_DEVICE_CLASS		'clas'	/* ulong */
> #define  BI_DEV_CLASS_ETHERNET		'eth '
> #define  BI_DEV_CLASS_UART		'uart'
> #define  BI_DEV_CLASS_SCC		'scc '
> 	/* etc... */
>
> /* The driver is used to define "standard" type of devices, like
>  * PC-like uart. Note that devices defined below rely on a specific
>  * format of the io resources and eventual additional properties
>  */
> #define BI_PROP_DEVICE_DRIVER		'drvr' /* ulong */
>
>  /* define required properties of legacy serial here */
>
> /*
>  * Properties relative to PCI hosts
>  */
>
>   /* define bus numbers, and bases here along with a type
>    * property indicating the host type, a generic type can
>    * be used for indirect
>    */
>
> /*
>  * Properties relative to PCI devices. This includes the
>  * above properties (all of them eventually). The IO resource
>  * property additionally defines some flags. The actual mapping
>  * of the host bridge (ISA, etc...) isn't defined here but is
>  * defined
>  */
> #define BI_PROP_PCI_DEVFN		'dvfn' /* ulong */
> #define BI_PROP_PCI_VENDORID		'vid ' /* Do we need that here ? */
> #define BI_PROP_PCI_DEVICEID		'did ' /* Do we need that here ? */
>
> /* Note: I appologize for bit numbering ;) */
>
>

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

Re: EV-64260-BP & GT64260 bi_recs

[Murray Jensen]

On Fri, 22 Mar 2002 21:01:03 -0700, Val Henson <val@nmt.edu> writes:
>while (rec->tag != BI_LAST) {
>  /* fiddle with each individual record */
>  rec = (struct bi_record *)((ulong)rec + rec->size);
>}

Won't this code still work with benh's proposal? The "fiddle" part above
may delve into the structure, but at the top level the record is still
treated exactly like above.

>The simplicity of this system is valuable - I don't want to give it up
>unless we get lots and lots of added functionality in return.

I agree with this. In fact, we shouldn't give up simplicity for any reason.

>Dan
>Malek's comments suggest that bi_recs should only be used for only a
>small, simple class of information.  In this context, I don't see what
>structures buy us.

That's because he sees no value in having a lot of data passed in from the
boot loader. On the other hand, I believe that if the kernel can learn
something from the boot loader, it should - rather than duplicate the code
that is already in the boot loader, or hard-wire configuration information -
and if this ends up being a lot of data and/or with a complicated structure
(this isn't automatically the case btw), then so be it. This can be achieved
with "simple" bi_recs, if they are defined in the right way.

>I have to admit, bi_rec structures are a really cool idea, but I
>prefer simplicity over coolness. :)

The beauty of the proposal is that it *is* simple. bi_recs within bi_recs.
The top level treats the record as one (typeless) chunk of data. I have
actually requested *more* types (e.g. arrays), and also the saving of
certain (most?) bi_recs so that loadable modules can consult them later
(rather than requiring code in the kernel startup to parse the records).
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

[Benjamin Herrenschmidt]

>The beauty of the proposal is that it *is* simple. bi_recs within bi_recs.
>The top level treats the record as one (typeless) chunk of data. I have
>actually requested *more* types (e.g. arrays), and also the saving of
>certain (most?) bi_recs so that loadable modules can consult them later
>(rather than requiring code in the kernel startup to parse the records).
>Cheers!

The kernel startup should stop parsing them. The interface between kernel
& drivers to bi_recs should instead be of the type "find_birec(type)" or
"find_dev_birec(class, id)" for BI_DEVICE ones.

There should be no reason to have to touch the kernel support files when
adding new types of bi_rec nor to have them all parsed in a single
location.

Ben.


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

Re: EV-64260-BP & GT64260 bi_recs

[Benjamin Herrenschmidt]

>What added functionality does the whole "structure" concept give us?
>Here's the current bi_rec parsing code:
>
>while (rec->tag != BI_LAST) {
>  /* fiddle with each individual record */
>  rec = (struct bi_record *)((ulong)rec + rec->size);
>}
>
>The simplicity of this system is valuable - I don't want to give it up
>unless we get lots and lots of added functionality in return.  Dan
>Malek's comments suggest that bi_recs should only be used for only a
>small, simple class of information.  In this context, I don't see what
>structures buy us.
>
>I have to admit, bi_rec structures are a really cool idea, but I
>prefer simplicity over coolness. :)

Well, my "draft" proposal is indeed to complicated, I agree. Though
I want to stay in the idea that a given bi_rec can contain itself
some bi_recs.

What I now want to do is to keep the exising scheme for most
informations (that is a flat list of bi_recs), with only one
exception that doesn't affect the current parsing code, only
a couple of helpers need to be added for drivers to use the
additional information:

We define a special BI_DEVICE type of bi_rec that represents
a HW device for which the firmware provides some informations. Since
the firmware is free to provide whatever informations it want (that
set is not fully defined), the content of the BI_DEVICE record is itself
a list of bi_recs.

In there, only a couple are generically defined, BI_DEV_TYPE indicates
the "type" of device (not the class or function, but rather the type of
wiring), and an optional BI_DEV_ID, whose content depends only on BI_DEV_TYPE.

For example, a PCI device for which we want to provide some additional
informations (like an HW ethernet address) would have
BI_DEV_TYPE = BI_DEV_TYPE_PCI, and BI_DEV_ID containing a PCI bus/devfn.

On chip devices just need a specific BI_DEV_TYPE (for example
BI_DEV_TYPE_4xx_OCP), in which case BI_DEV_ID indicates what on chip
periph is designated (ex: BI_DEV_ID_4xx_ETH).
Eventually, we can also define a BI_DEV_CLASS indicating the calss
(function) of the device (ex: BI_DEV_CLASS_ETH). If we go this way, then
for things like on-chip devices, we identify them with BI_DEV_TYPE and
BI_DEV_CLASS, keeping BI_DEV_ID solely as an index number for chips that
support more than 1 instance of a given cell.

The "standard" kernel only define a few BI_DEV_TYPE's (like PCI,
4xx OCP, 8xx OCP). Drivers define attributes they can read from the
BI_DEVICE (4xx ethernet can read a HW eth adress for example).
Board vendors are free to provide additional information in the
BI_DEVICE, and add the ability to the driver (patches welcome) to
make good use of that information ;) (Could be, for example, wiring
of the PHY since it may not use MII, etc...).

What do you think ?
Ben.


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

Re: EV-64260-BP & GT64260 bi_recs

[Val Henson]

On Sun, Mar 24, 2002 at 01:20:48PM +0100, Benjamin Herrenschmidt wrote:
>
> We define a special BI_DEVICE type of bi_rec that represents
> a HW device for which the firmware provides some informations. Since
> the firmware is free to provide whatever informations it want (that
> set is not fully defined), the content of the BI_DEVICE record is itself
> a list of bi_recs.
<snip>
> The "standard" kernel only define a few BI_DEV_TYPE's (like PCI,
> 4xx OCP, 8xx OCP). Drivers define attributes they can read from the
> BI_DEVICE (4xx ethernet can read a HW eth adress for example).
> Board vendors are free to provide additional information in the
> BI_DEVICE, and add the ability to the driver (patches welcome) to
> make good use of that information ;) (Could be, for example, wiring
> of the PHY since it may not use MII, etc...).
>
> What do you think ?

I had an amazing and brilliant insight (which I'm sure everyone else
has already had). :) The kernel just ignores bi_recs it doesn't
understand.  Really, you don't need any BI_DEV_TYPE's for non-core
kernel code - just a type that the kernel is guaranteed never to use
for any other bi_rec type.

How about one BI_IGNORE type, and driver writers and firmware authors
can put whatever they feel like inside that bi_rec?  The BI_IGNORE
bi_rec can contain whatever you want - more bi_recs, object code,
random data - and it would be the driver and firmware writers'
responsibility to make them match up.  I personally think this is an
awful idea, but it would give everyone the freedom they want while
staying within the very nice bi_rec interface.  The rest of the
bi_recs, the ones that the core kernel code will interpret, can be
simple, one-dimensional bi_recs.  What do you think, Ben?

I really agree with Dan Malek on this - we shouldn't use bi_recs as a
way to reimplement methods of passing information that already exist,
for example, the __setup() functions.  It should be a way of passing
information that only a bootloader can know, such as the location of a
ramdisk, or the command line that the user typed into the bootloader.
People who don't agree with this philosophy can shove whatever they
like into the BI_IGNORE record type, and suffer the consequences. :)

-VAL

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

Re: EV-64260-BP & GT64260 bi_recs

[Benjamin Herrenschmidt]

>
>I had an amazing and brilliant insight (which I'm sure everyone else
>has already had). :) The kernel just ignores bi_recs it doesn't
>understand.  Really, you don't need any BI_DEV_TYPE's for non-core
>kernel code - just a type that the kernel is guaranteed never to use
>for any other bi_rec type.

Of course you want to add whatever additional bi_recs, and among the things
I propose is the definition that the kernel will only use all-lowercase
bi_rec types for it's own use leaving any other combo for other uses.

The point is to pass informations to drivers in a bit cleaner
way than inventing a bi_rec type for each combination of driver
and attribute (expecially if a given board can decline in several
models with, for example, a different number of on-chip eth controllers,
or things like that).

>How about one BI_IGNORE type, and driver writers and firmware authors
>can put whatever they feel like inside that bi_rec?  The BI_IGNORE
>bi_rec can contain whatever you want - more bi_recs, object code,
>random data - and it would be the driver and firmware writers'
>responsibility to make them match up.  I personally think this is an
>awful idea, but it would give everyone the freedom they want while
>staying within the very nice bi_rec interface.  The rest of the
>bi_recs, the ones that the core kernel code will interpret, can be
>simple, one-dimensional bi_recs.  What do you think, Ben?

Which means that as soon as you want to add more infos, you will have
to deal with all the pre-bi_rec problems when you own stuffs have to
evolve. (versionning etc...).

Again, for anything not realted to device drivers infos (things like
HW ethernet addresses, PHY IDs, eventually interrupt routing), a whole
bunch of bi_rec types will be left free by the kernel for use by your
proprietary stuff the way you want, so you can pretty much define
whatever you want (or not, it's up to you).

But, I feel it's more convenient for drivers to use this signle-level
BI_DEVICE bi_rec that contains itself bi_recs.

>I really agree with Dan Malek on this - we shouldn't use bi_recs as a
>way to reimplement methods of passing information that already exist,
>for example, the __setup() functions.  It should be a way of passing
>information that only a bootloader can know, such as the location of a
>ramdisk, or the command line that the user typed into the bootloader.

Which is exactly what I'm proposing. Passing generic informations like
CPU core clocks, command line, ramdisk, etc... is done via bi_recs at
the toplevel.

The BI_DEVICE bi_rec's allow to provide other informations that are
also only known by the firmware (most of the time), like eth MAC
address, PHY wiring, or other kind of wiring informations related
to a given rev. of a board, etc... provided that those infos
concern a given driver for a specific device. It's also a convenient
way to provide interrupt routing informations.

>People who don't agree with this philosophy can shove whatever they
>like into the BI_IGNORE record type, and suffer the consequences. :)

No. A BI_IGNORE makes no sense. A whole class of bi_rec types guaranteed
not to be used by the kernel makes sense. I don't want to fix the kernel
side of the problem just to put back the same problem on my vendor specific
infos (I have various ones; depending on the product, they can have to evolve
especially as I have to maintain different revisions of the produce, and if
possible with the same kernel / firmware). It's always a win when you don't
have to change a line of the kernel code because your HW engineer wired an
interrupt differently. That means I only have to update the tables in
the firmware and not touch the kernel version.

Ben.


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

Re: EV-64260-BP & GT64260 bi_recs

[Geert Uytterhoeven]

On Sun, 24 Mar 2002, Benjamin Herrenschmidt wrote:
> Which means that as soon as you want to add more infos, you will have
> to deal with all the pre-bi_rec problems when you own stuffs have to
> evolve. (versionning etc...).
>
> Again, for anything not realted to device drivers infos (things like
> HW ethernet addresses, PHY IDs, eventually interrupt routing), a whole
> bunch of bi_rec types will be left free by the kernel for use by your
> proprietary stuff the way you want, so you can pretty much define
> whatever you want (or not, it's up to you).
>
> But, I feel it's more convenient for drivers to use this signle-level
> BI_DEVICE bi_rec that contains itself bi_recs.
>
> >I really agree with Dan Malek on this - we shouldn't use bi_recs as a
> >way to reimplement methods of passing information that already exist,
> >for example, the __setup() functions.  It should be a way of passing
> >information that only a bootloader can know, such as the location of a
> >ramdisk, or the command line that the user typed into the bootloader.
>
> Which is exactly what I'm proposing. Passing generic informations like
> CPU core clocks, command line, ramdisk, etc... is done via bi_recs at
> the toplevel.
>
> The BI_DEVICE bi_rec's allow to provide other informations that are
> also only known by the firmware (most of the time), like eth MAC
> address, PHY wiring, or other kind of wiring informations related
> to a given rev. of a board, etc... provided that those infos
> concern a given driver for a specific device. It's also a convenient
> way to provide interrupt routing informations.

Alternatively, why not create a whole device tree for this? There already
exists a standard for device trees: IEEE 1275 Open Firmware. Then you need only
one extra bi_rec tag: BI_OF_TREE.

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds


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

Re: EV-64260-BP & GT64260 bi_recs

[Benjamin Herrenschmidt]

>Alternatively, why not create a whole device tree for this? There already
>exists a standard for device trees: IEEE 1275 Open Firmware. Then you
>need only
>one extra bi_rec tag: BI_OF_TREE.

Well, BI_OF_TREE may exist if we ever decide to move the OF interface
out of the kernel and to the wrapper. Though it's not well suited for
embedded platforms where we really want something as small as possible.

The device-tree has the inconvenient of duplicating a lot of informations
that we don't necessarily need. In most embedded cases, I will only
need about 2 or 3 BI_DEVICE records, those containing only a couple
of informations.

I'm still not completely sure about the BI_DEV_TYPE/BI_DEV_ID mecanism
of retreiving a given BI_DEVICE yet. I have a case here where I want to
use BI_DEVICE to carry informations about the chip select & IRQ on which
a chip is wired on the processor external bus, along with a couple of
flags related to some broken address select on some board revs, I want my
driver to be able to lookup the chip itself, not a "location", so I'm still
thinking about the best way to provide that info. Maybe an additional (and
optional as always BI_DEV_NAME).

That would give us 2 utility routines to be used by drivers to locate
BI_DEVICE bi_recs:

  bi_find_device_type(ulong dev_type, void* dev_id, size_t dev_id_size)
  bi_find_device_name(const char* name)

The first one compares BI_DEV_TYPE with dev_type, and if dev_id is non
NULL, matches it with the variable sized BI_DEV_ID. The second one
compares BI_DEV_NAME as a C string.

The basic function for finding a bi_rec will be

  bi_find(struct bi_rec* where, ulong tag);

"where" will be NULL for a toplevel search, and will point to a bi_rec
returned by one of the 2 bi_find_device routines for looking up an
attribute within a BI_DEVICE.

Any comments ? Should I start putting this together in _2_4_devel or
only 2_5 ?

Ben.


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

Re: EV-64260-BP & GT64260 bi_recs

[Val Henson]

On Sun, Mar 24, 2002 at 05:46:49PM +0100, Benjamin Herrenschmidt wrote:
>
> Of course you want to add whatever additional bi_recs, and among the things
> I propose is the definition that the kernel will only use all-lowercase
> bi_rec types for it's own use leaving any other combo for other uses.

Did you mean, the kernel will use all-uppercase bi_rec types?  That's
what it's currently using.  If you meant to say that the kernel will
use all-lowercase bi_rec types, I'm interested in knowing why it's
worth breaking backwards compatibility.

> No. A BI_IGNORE makes no sense. A whole class of bi_rec types guaranteed
> not to be used by the kernel makes sense.

Sure, that makes sense.  Like the vendor specific major/minor numbers.

-VAL

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

Re: EV-64260-BP & GT64260 bi_recs

[Benjamin Herrenschmidt]

>On Sun, Mar 24, 2002 at 05:46:49PM +0100, Benjamin Herrenschmidt wrote:
>>
>> Of course you want to add whatever additional bi_recs, and among the things
>> I propose is the definition that the kernel will only use all-lowercase
>> bi_rec types for it's own use leaving any other combo for other uses.
>
>Did you mean, the kernel will use all-uppercase bi_rec types?  That's
>what it's currently using.  If you meant to say that the kernel will
>use all-lowercase bi_rec types, I'm interested in knowing why it's
>worth breaking backwards compatibility.

Yes, sorry for the confusion.

>> No. A BI_IGNORE makes no sense. A whole class of bi_rec types guaranteed
>> not to be used by the kernel makes sense.
>
>Sure, that makes sense.  Like the vendor specific major/minor numbers.


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

Re: EV-64260-BP & GT64260 bi_recs

[Geert Uytterhoeven]

On Sun, 24 Mar 2002, Val Henson wrote:
> On Sun, Mar 24, 2002 at 01:20:48PM +0100, Benjamin Herrenschmidt wrote:
> > We define a special BI_DEVICE type of bi_rec that represents
> > a HW device for which the firmware provides some informations. Since
> > the firmware is free to provide whatever informations it want (that
> > set is not fully defined), the content of the BI_DEVICE record is itself
> > a list of bi_recs.
> <snip>
> > The "standard" kernel only define a few BI_DEV_TYPE's (like PCI,
> > 4xx OCP, 8xx OCP). Drivers define attributes they can read from the
> > BI_DEVICE (4xx ethernet can read a HW eth adress for example).
> > Board vendors are free to provide additional information in the
> > BI_DEVICE, and add the ability to the driver (patches welcome) to
> > make good use of that information ;) (Could be, for example, wiring
> > of the PHY since it may not use MII, etc...).
> >
> > What do you think ?
>
> I had an amazing and brilliant insight (which I'm sure everyone else
> has already had). :) The kernel just ignores bi_recs it doesn't
> understand.  Really, you don't need any BI_DEV_TYPE's for non-core
> kernel code - just a type that the kernel is guaranteed never to use
> for any other bi_rec type.

Yes indeed.

BTW, I just thought of something different: some people want to keep data
stored in bi_recs for later use, after the initialization of the kernel. What
about a new tag to mark data that is to be copied and stored for later use?

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds


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

Re: EV-64260-BP & GT64260 bi_recs

[Benjamin Herrenschmidt]

>Yes indeed.
>
>BTW, I just thought of something different: some people want to keep data
>stored in bi_recs for later use, after the initialization of the kernel. What
>about a new tag to mark data that is to be copied and stored for later use?

Currently, I plan to keep everything. Though if we decide to put some
flag bits in the high part of the size field, we could specify what
to keep and what to strip.

Ben.


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

Re: EV-64260-BP & GT64260 bi_recs

[Benjamin Herrenschmidt]

>>BTW, I just thought of something different: some people want to keep data
>>stored in bi_recs for later use, after the initialization of the
kernel. What
>>about a new tag to mark data that is to be copied and stored for later use?
>
>Currently, I plan to keep everything. Though if we decide to put some
>flag bits in the high part of the size field, we could specify what
>to keep and what to strip.

Ok, rather that hacking flag bits, which I want to avoid, what about
that: we define an optional BI_PERSISTENT tag. Any bi_rec _before_
that tag is lost after __init (typically cmdline, initrd, informations
used in *_setup). Any bi_rec after this tag is kept around. Tyîcally,
this means those bi_recs are copied to separate pages (this will
typically be BI_DEVICE recs). If you want, you can put them before
BI_PERSISTENT and build your drivers in. If you prefer using modules,
you can put your BI_DEVICE entries after it.

An important point about bi_recs is that they shouldn't contain
pointers to other things within bi_recs. They can (and will) be
moved around, so unless explicitely defined (like initrd ptrs),
no pointers in bi_recs.

Again, all of this is just a small superset of the existing mecanism
with a few utiliy functions for kernel code and drivers to locate
bi_rec's by tag, either globally or within another bi_rec (that
is typically BI_DEVICE). The point is that once this is coded, I
expect bd_t & other various facilities used by bootloaders to go
away, or at worst be moved to the wrapper.

Ben.


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

Re: EV-64260-BP & GT64260 bi_recs

[Murray Jensen]

On Sun, 24 Mar 2002 12:09:30 -0700, Val Henson <val@nmt.edu> writes:
>How about one BI_IGNORE type, and driver writers and firmware authors
>can put whatever they feel like inside that bi_rec?

The kernel should simply ignore any bi_rec it doesn't know about. You don't
need a special record tag for this. Any bi_recs within bi_recs are all ignored
in one fell swoop, since the outer record tag is unknown.

>It should be a way of passing
>information that only a bootloader can know, such as the location of a
>ramdisk, or the command line that the user typed into the bootloader.

Why only that? Why not use it to pass anything that the bootloader knows
so that you don't have to duplicate effort in the Linux kernel?

And anticipating the obvious answer, if a boot loader does not support bi_recs
and/or cannot provide some required information, then you need a piggy back
loader in arch/ppc/boot. 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

[Val Henson]

On Mon, Mar 25, 2002 at 11:44:10AM +1100, Murray Jensen wrote:
>
> On Sun, 24 Mar 2002 12:09:30 -0700, Val Henson <val@nmt.edu> writes:
> >How about one BI_IGNORE type, and driver writers and firmware authors
> >can put whatever they feel like inside that bi_rec?
>
> The kernel should simply ignore any bi_rec it doesn't know about. You don't
> need a special record tag for this. Any bi_recs within bi_recs are all ignored
> in one fell swoop, since the outer record tag is unknown.

I know most of you understand this, but I just wanted to make it
clear:

The kernel should simply ignore any bi_rec it doesn't know about - and
does.  The question is, how can we guarantee that the kernel doesn't
know about that bi_rec?  The answer is that we need to reserve at
least one (or several, under Ben's plan) bi_rec types that are
guaranteed not to be used by the kernel.  That's all I was trying to
say.

-VAL

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

Re: EV-64260-BP & GT64260 bi_recs

[Val Henson]

On Sun, Mar 24, 2002 at 01:20:48PM +0100, Benjamin Herrenschmidt wrote:
>
> What I now want to do is to keep the exising scheme for most
> informations (that is a flat list of bi_recs), with only one
> exception that doesn't affect the current parsing code, only
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
I like this part!

<A bunch of stuff about various ways to identify driver specific
bi_recs>

> What do you think ?

I like the general idea - have a bunch of bi_recs that the kernel
ignores, and then define a driver interface to easily get that
information.

I would like a simpler, more generic interface between drivers and
bi_recs.  The model is this: Firmware creates a top-level bi_rec and
puts something in it.  When the driver wants to get the information,
it calls:

  my_bi_rec = bi_find_id(NULL, BI_SOME_ID);

This gives it a pointer to the top-level bi_rec containing its info.
If the firmware put more bi_recs inside that bi_rec, the driver calls:

  my_other_bi_rec = bi_find_id(&(my_bi_rec->data), BI_SOME_OTHER_ID);

Basically, don't create special cases, just treat the generic case of
a driver and a bootloader and some information you want to pass
between them.  I don't like BI_DEVICE, I do like BI_NOT_FOR_THE_KERNEL
or something shorter but equivalent. :)

I'm happy to hear reasons for why a more complicated interface is a
good idea.

-VAL

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

Re: EV-64260-BP & GT64260 bi_recs

[Murray Jensen]

On Mon, 25 Mar 2002 20:21:32 -0700, Val Henson <val@nmt.edu> writes:
>I'm happy to hear reasons for why a more complicated interface is a
>good idea.

benh wanted to "find" a particular BI_DEVICE bi_rec within multiple instances.
I think what you are suggesting requires that there be only one instance of any
given (top-level) tag. yes? If not, how do you determine which bi_rec to return?
Always the first one you come across? or maintain a pointer and have subsequent
calls return the next one, until there are no more, at which time you return NULL?

I think benh was suggesting that you have many "top-level" BI_DEVICE bi_recs,
then have some interface to "find" one of them (either by "name", or by some
variable length "device id") - each driver (that wants to) then looks for it's
bi_rec when it is initialised. This would require that every BI_DEVICE bi_rec
"contains" either a "name" bi_rec or a "device id" bi_rec - otherwise it would
never be found (using the proposed interface functions). 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

[benh]

>Basically, don't create special cases, just treat the generic case of
>a driver and a bootloader and some information you want to pass
>between them.  I don't like BI_DEVICE, I do like BI_NOT_FOR_THE_KERNEL
>or something shorter but equivalent. :)
>
>I'm happy to hear reasons for why a more complicated interface is a
>good idea.

Ok, so first, the generic case is there as you can always stuff
things inside a toplevel bi_rec and use bi_find within that.

The point of BI_DEVICE is to standardize a bit the interface
between known drivers and the bootloader. Your scheme would
require each device on my board to have a different bi_rec,
which will lead into proprietary stuff & more driver patching.

For example, let's say we fix some "common" ethernet drivers
like pcnet32, tulip, etc... to be able to locate a bi_rec
for a BI_DEV_TYPE_PCI based on the bus/devfn and use the HW
address in there. Once that's done, no kernel/driver change
is needed, even if you happen to have 3 instances of the
ethernet chip on your PCI bus, just pass the appropriate infos
from the bootloader. Of course, this is mostly useful for
embedded cases where the PCI device is wired, not in a slot.

Also, a set BI_DEV_TYPE_PCI with just a BI_IRQ_NUMBER
(no indication of the device class/name) can also be used
to provide the PCI interrupt routing for PCI slots, though
in this specific case, I'd prefer seeing the firmware fill
the PCI_INTERRUPT_LINE register in the config space and the
kernel retreive that.

Ben.


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

Re: EV-64260-BP & GT64260 bi_recs

[Gabriel Paubert]

On Tue, 26 Mar 2002 benh@kernel.crashing.org wrote:

> Also, a set BI_DEV_TYPE_PCI with just a BI_IRQ_NUMBER
> (no indication of the device class/name) can also be used
> to provide the PCI interrupt routing for PCI slots, though
> in this specific case, I'd prefer seeing the firmware fill
> the PCI_INTERRUPT_LINE register in the config space and the
> kernel retreive that.

Won't fly, this is a byte wide register and there are machines with more
than 256 interrupt lines. And they are even more likely with PCI 2.2+ so
called message passing interrupts, which is a simple way to admit that the
way of doing interrupts was so dumb that even (one physical PCB trace per
vector) the Cro-Magnon man would have done better

Basically, PCI is going to implement interrupts in the way similar to waht
VME/m68k (and even Z80) hardware have done for over 25 years. The
electrical details are different, but it is the device that puts the
interrupt vector on the bus, not some PIC (where P stands for
paleolithic) through a maze of PCB traces.

BTW, I don't even allow sharing VME interrupt vectors in my PCI<->VME
bridge driver, because I have something like 1792 (7*256) separately
identifiable interrupt sources.

	Regards,
	Gabriel.


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

Re: EV-64260-BP & GT64260 bi_recs

[benh]

>
>Won't fly, this is a byte wide register and there are machines with more
>than 256 interrupt lines. And they are even more likely with PCI 2.2+ so
>called message passing interrupts, which is a simple way to admit that the
>way of doing interrupts was so dumb that even (one physical PCB trace per
>vector) the Cro-Magnon man would have done better

Well, I would expect such machines to have a device-tree :) Anyway, a
bi_rec would indeed help if really needed here but again, it's optional
and I would expect embedded not to require it for PCI routing.

>Basically, PCI is going to implement interrupts in the way similar to waht
>VME/m68k (and even Z80) hardware have done for over 25 years. The
>electrical details are different, but it is the device that puts the
>interrupt vector on the bus, not some PIC (where P stands for
>paleolithic) through a maze of PCB traces.
>
>BTW, I don't even allow sharing VME interrupt vectors in my PCI<->VME
>bridge driver, because I have something like 1792 (7*256) separately
>identifiable interrupt sources.

Ben.


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

Re: EV-64260-BP & GT64260 bi_recs

[Mark A. Greer]

Ben, et. al.,

Sorry for dropping out the last few days, I've been very busy with other things.

I've skimmed the email but I'll admit I didn't read them all in depth.  I've made
an example of actual bi_rec's using the ev64260 to see if my skimming worked :)
In the example, I used a BI_STRUCT to pass in  the mac addr for the 3 embedded
enet ctlrs on the gt64260.  I used the DEV_TYPE + DEV_CLASS concept but made an
"EMBEDDED" DEV_TYPE to go along with DEV_PCI so we don't have to add a new one
for each embedded ctlr.  The square brackets are only there to make it easier to
follow.  I'm assuming that every bi_rec is responsible for padding out to a
4-byte boundary.  Is that what I'm supposed to do?

Please let me know if I'm on the right track.  If so, we--the ev64260 people--can
flesh out some more bi_rec's and implement them.

Mark
--
Example for ev64260:
--------------------

[0] tag: BI_CMD_LINE
    size: 36                            (4 + 4 + 26 chars + 2 pad)
    data: "console=ttyS0,115200 ip=on"
    pad: 2

[1] tag: BI_MEMSIZE
    size: 12
    data: 33554432                      (32 MB)

[2] tag: BI_GT64260_BASE
    size: 12
    data: fc000000

[3] tag: BI_STRUCT                      (embedded enet cltr 0)
    size: 76                            (12 + 3*12 + 25 + 3 == 76)
    data: BI_DEVICE

    [3.0] tag: BI_DEV_TYPE
          size: 12
          data: BI_DEV_EMBEDDED

    [3.1] tag: BI_DEV_CLASS
          size: 12
          data: BI_CLASS_ENET

    [3.2] tag: BI_DEV_ID
          size: 12
          data: 0                       (1st enet device)

    [3.3] tag: BI_MAC_ADDR
          size: 25
          data: aa:bb:cc:dd:ee:ff       (ascii)
    pad: 3

[4] tag: BI_STRUCT
    size: 76
    data: BI_DEVICE

    [4.0] tag: BI_DEV_TYPE
          size: 12
          data: BI_DEV_EMBEDDED

    [4.1] tag: BI_DEV_CLASS
          size: 12
          data: BI_CLASS_ENET

    [4.2] tag: BI_DEV_ID
          size: 12
          data: 1                       (2nd enet device)

    [4.3] tag: BI_MAC_ADDR
          size: 25
          data: gg:hh:ii:jj:kk:ll       (ascii)
    pad: 3

[5] tag: BI_STRUCT
    size: 76
    data: BI_DEVICE

    [5.0] tag: BI_DEV_TYPE
          size: 12
          data: BI_DEV_EMBEDDED

    [5.1] tag: BI_DEV_CLASS
          size: 12
          data: BI_CLASS_ENET

    [5.2] tag: BI_DEV_ID
          size: 12
          data: 2                       (3rd enet device)

    [5.3] tag: BI_MAC_ADDR
          size: 25
          data: mm:nn:oo:pp:qq:rr       (ascii)
    pad: 3


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

Re: EV-64260-BP & GT64260 bi_recs

[benh]

>Ben, et. al.,
>
>Sorry for dropping out the last few days, I've been very busy with other
>things.
>
>I've skimmed the email but I'll admit I didn't read them all in depth.
>I've made
>an example of actual bi_rec's using the ev64260 to see if my skimming
>worked :)
>In the example, I used a BI_STRUCT to pass in  the mac addr for the 3
embedded
>enet ctlrs on the gt64260.  I used the DEV_TYPE + DEV_CLASS concept but
>made an
>"EMBEDDED" DEV_TYPE to go along with DEV_PCI so we don't have to add a
new one
>for each embedded ctlr.  The square brackets are only there to make it
>easier to
>follow.  I'm assuming that every bi_rec is responsible for padding out to a
>4-byte boundary.  Is that what I'm supposed to do?
>
>Please let me know if I'm on the right track.  If so, we--the ev64260
>people--can
>flesh out some more bi_rec's and implement them.

Looks fine except for a couple of points:

 - BI_STRUCT is gone, it's BI_DEVICE and the first "type" field is gone,
that is BI_DEVICE just contains tags without a header. There is no need
to "know" if a given BI_REC is actually a struct or not, though if we
still want to do that for debugging reasons, we can define this info is
stored in the the high bit of the size field (and make sure bi_rec parsing
routines ignore that bit when reading size)

 - I'm not sure about the "EMBEDDED" dev type. I'd rather have a specific
type for EV64260_EMBEDDED, and another one for 405_EMBEDDED for example,
though this is mostly a matter of taste, we have enough room for types
to define as many as we want ;)

Ben.


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

Re: EV-64260-BP & GT64260 bi_recs

[Mark A. Greer]

benh@kernel.crashing.org wrote:

> Looks fine except for a couple of points:
>
>  - BI_STRUCT is gone, it's BI_DEVICE and the first "type" field is gone,
> that is BI_DEVICE just contains tags without a header. There is no need
> to "know" if a given BI_REC is actually a struct or not, though if we
> still want to do that for debugging reasons, we can define this info is
> stored in the the high bit of the size field (and make sure bi_rec parsing
> routines ignore that bit when reading size)
>
>  - I'm not sure about the "EMBEDDED" dev type. I'd rather have a specific
> type for EV64260_EMBEDDED, and another one for 405_EMBEDDED for example,
> though this is mostly a matter of taste, we have enough room for types
> to define as many as we want ;)
>
> Ben.

Ahh, oops, didn't see this until after I send the second iteration the the
bi_rec's.
I'll send a third in a few minutes.

Mark


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

Re: EV-64260-BP & GT64260 bi_recs

[Wolfgang Denk]

In message <20020320000500.GV3762@opus.bloom.county> you wrote:
>
> > > need to change the driver to handle multiple enets, yes?
> >
> > Only some, like the 8xx. Other  configurations  run  just  fine  with
> > several ethernet interfaces.
>
> Well yes, but the ones which run fine with several ethernet interfaces
> are a) generic drivers (tulip, eepro, et al) and b) just work right now
> with multiple devices, and I'm not sure why they'd want to care about
> bi_recs.

Take the 8260 for an example - it is not a generic driver, and  there
is no existing solution to pass more than one MAC address.

> Me.  But it's gotta be made to work sometime.  And as a bonus, Linus is
> away for 2 weeks so things probably won't get worse than they are right
> now for about 2 weeks. :)

This is not the way I want to work.

Wolfgang Denk

--
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-4596-87  Fax: (+49)-8142-4596-88  Email: wd@denx.de
The human mind  ordinarily  operates  at  only  ten  percent  of  its
capacity. The rest is overhead for the operating system.

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

Re: EV-64260-BP & GT64260 bi_recs

[Wolfgang Denk]

In message <20020319235815.GU3762@opus.bloom.county> you wrote:
>
> > The systems I see shipping in the next 6 months or so will  be  based
> > on 2.4 kernel; I think we should define this feature rather sooner or
> > later to try it out and get it right.
>
> Yes, but the place to work on things isn't 2.4, it's 2.5.

OK. Then stop all efforts on 2.4 _now_ ???

> > The command line is pretty limited in leght, and there are many  more
> > "interesting"  things  you want to pass to proprietary drivers etc. I
> > rather had common stuff like this somewhere else...
>
> Yes, but the question isn't what we'd rather do, it's what we need to do
> for right now, and what we need to do for the future.  This does make
> sense for the future, but not for right now.

We are changing the interface from bd_info to bi_recs.

We had the baudrate in  bd_info,  and  nobody  complained.  I'm  just
asking  to preserve the current state with as minimal changes visible
to the user as possible.

It is _you_ who is trying to start some cleanup removing things  that
have "always" been there!


> Right NOW we have something which works for most of the problems.  What
> Mark is talking about would be adding in a few more defines to
> include/asm-ppc/bootinfo.h and using them.  Not sort-of implementing
> what's been talked about and then do something better later.

If we replace  bd_info  now,  we  should  keep  all  the  entries  it
provided, right?


Wolfgang Denk

--
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-4596-87  Fax: (+49)-8142-4596-88  Email: wd@denx.de
I used to be indecisive, now I'm not sure.

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

Re: EV-64260-BP & GT64260 bi_recs

[Wolfgang Denk]

In message <20020319231628.GQ3762@opus.bloom.county> you wrote:
>
> I sort-of remember this too.  For 2.4 I'm not sure if it's better to
> have a massive ammount of compile time options or not.  Either way we'd

What sort of compile time options? I don't want to have MAC addresses
built into the kernel images!

> need to change the driver to handle multiple enets, yes?

Only some, like the 8xx. Other  configurations  run  just  fine  with
several ethernet interfaces.

> > BTW: the structure should probably be extended by one entry  for  the
> > interface (eth0, eth1, etc...).
>
> Forcing ethX is a bad idea I think, but we do need a way of saying all
> of this applies to *this* enet device.

Right, probably an index is all we need.

> Yes, but we're speaking in terms of GT-64260, which has ns1655x and it's
> own, but I'm not as clear about having 'em both on.  But, does 8xx not
> allow for reading of what things are currently set at now or must it be
> specified?

But isn't this the whole idea behind the new  interface  -  to  allow
certain  things to be done just ONCE, in ONE place, and not again and
again and again in several instances of  the  same  code,  with  good
chances that at least one instance is buggy?

Of course I can retrieve this information in some CPU and  eventually
board  dependend  way  somehow - but why should I bother to implement
this code when the information is already available?

With the same argiment we could drop the MEMSIZE record - the  kernel
can  detect the memory size as well. Not that I think that would be a
good idea...

And BTW: I'm not talking about 8xx systems only.

> > You are wrong. We'll switch faster than you seem to think.
>
> I know you'll switch when it's all set and working.

Then, or before :-)

> There's a time and place for everything, and it's called the development
> line.  Big rewrites/changes are supposed to be done there and maybe
> backported.

Ummm... someone called 2.5 "fs-eating-non-compiling-crap,  so  ignore
that for a few months". Guess who?

Wolfgang Denk

--
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-4596-87  Fax: (+49)-8142-4596-88  Email: wd@denx.de
"I say we take off; nuke the site from orbit. It's the only way to be
sure."                                  - Corporal Hicks, in "Aliens"

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

Re: EV-64260-BP & GT64260 bi_recs

[Wolfgang Denk]

In message <3C97A9C1.EBA150B6@mvista.com> you wrote:
>
> > How about my proposal to replace MEMSIZE and  MEMSTART  with  a  more
> > general description which allows for _several_ memory areas, probably
> > of  different type (RAM, ROM, Flash, SRAM, ...), different bus witdth
> > etc. ?
>
> I think its a good idea but overkill for now isn't it ("now" being 2.4 since
> its going to change in 2.5)?
...
> Can we push this off into 2.5?  I was hoping to keep it fairly simple and
> just get it done in 2.4.

The systems I see shipping in the next 6 months or so will  be  based
on 2.4 kernel; I think we should define this feature rather sooner or
later to try it out and get it right.

Of course, we can always add this locally, but why not do it now when
we know it needs to get done?

> But every board/driver can still use the console=xxx on the cmd line, right?
> Can we get by with that?

The command line is pretty limited in leght, and there are many  more
"interesting"  things  you want to pass to proprietary drivers etc. I
rather had common stuff like this somewhere else...

> The only reason I'm pushing for simple bi_recs now is because I expect all
> this to change significantly in 2.5.  If you want the 2.5 way back-ported to
> 2.4 and everyone agrees that's what should be done, then I agree we should do
> it the "right way" now.  Otherwise, I say do the minimum now and do it right
> in 2.5.

If you ask for my opinion: let's do it right NOW, and stick with this
in 2.5, rather than wasting some work  now  for  a  preliminary  ugly
solution  which we think will get thrown avay (which might not happen
as expected), and then spending even more work to do it right.

If you know what  the  final  version  will  look  like,  then  let's
implement this now. And be done with it.

Wolfgang Denk

--
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-4596-87  Fax: (+49)-8142-4596-88  Email: wd@denx.de
Above all else -- sky.

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

Re: EV-64260-BP & GT64260 bi_recs

[Wolfgang Denk]

In message <20020319224420.GO3762@opus.bloom.county> you wrote:
>
> > BI_GT64260_ETH_CFG to a more generic BI_ETH_CFG.
>
> I don't know about this.  Why?  From what I can tell, this is something
> that will be useful for 8xx/8260, but would require changing the driver
> to know about this, and it's currently over bd_t, and I'd rather leave
> it for now.

I  have  seen  enough  requests  for   a   more   flexible   ethernet
configuration  (systems with 2 or 3 or more ethernet interfaces) that
I'm willing to adapt the 8xx and  82xx  drivers  rather  sooner  than
later.

BTW: the structure should probably be extended by one entry  for  the
interface (eth0, eth1, etc...).

> > New (and [pretty much] generic):
> > --------------------------------
> >   BI_MEMSTART -- mem start
>
> GT-64260 doesn't use this, does it?  We'll need this for Nubus and other
> discontig memory situations, but not right now (except for APUS).

How about my proposal to replace MEMSIZE and  MEMSTART  with  a  more
general description which allows for _several_ memory areas, probably
of  different type (RAM, ROM, Flash, SRAM, ...), different bus witdth
etc. ?

For instance, we have MPC8260 systems with both global and local  RAM
which  are  not  contiguous  (they  could  be,  but it makes no sense
anyway).

How to handle this?

We have boards with several flash banks - one 8 bit wide,  the  other
64 bit.

Etc. etc. IMHO we should do it right from the  beginning  and  use  a
more flexible way to describe memory.

You can probably #define a simple macro for backward compatibility.

> >   BI_INTFREQ -- internal CPU frequency??
> >   BI_BUSFREQ -- CPU bus freqency?
> >   BI_ETH_CFG -> struct with following:
> >    -- mac addr
> >    -- hdx/fdx; 10/100/1000/...; OR autonegotiate
> >    -- phy addr
> >    -- some bytes for driver specific info (e.g., on gt64260, is_rmii)

==> please add interface (index)

> >   BI_BAUDRATE -- Console baudrate
>
> Is this actually needed?  iirc, the ns1655x serial driver _should_ pick
> up what things are running at.  But either way I don't see how we'd use
> this, except in custom serial drivers.

Yes, This _is_ necessary. Not every board has a ns1655x UART.

> >   BI_IMMRBASE -- Base Address of CPU internal memory (82xx/8xx only)
> >   BI_SCCFREQ  -- SCC frequency in Hz (82xx, 8xx only)
>
> See my comment about about bd_t stuffs.  I don't think bi_recs will
> become useful to PPCBoot until 2.5, when we can expand/otherwise flesh

You are wrong. We'll switch faster than you seem to think.

> out things and be done with it, and use it in 2.6.0.

Why postpone a decision that will make life easier to us?

We have these requirements NOW, and no clean way to solve them. Let's
do it NOW.

Wolfgang Denk

--
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-4596-87  Fax: (+49)-8142-4596-88  Email: wd@denx.de
The software required `Windows 95 or better', so I installed Linux.

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

Re: EV-64260-BP & GT64260 bi_recs

[Gabriel Paubert]

On Tue, 19 Mar 2002, Tom Rini wrote:

>
> On Tue, Mar 19, 2002 at 02:56:14PM -0500, Mark A. Greer wrote:
>
> > I like what Michael has done but want to add more and change the
> > BI_GT64260_ETH_CFG to a more generic BI_ETH_CFG.
>
> I don't know about this.  Why?  From what I can tell, this is something
> that will be useful for 8xx/8260, but would require changing the driver
> to know about this, and it's currently over bd_t, and I'd rather leave
> it for now.
>
> > New (and [pretty much] generic):
> > --------------------------------
> >   BI_MEMSTART -- mem start
>
> GT-64260 doesn't use this, does it?  We'll need this for Nubus and other
> discontig memory situations, but not right now (except for APUS).
>
> >   BI_INTFREQ -- internal CPU frequency??
> >   BI_BUSFREQ -- CPU bus freqency?
> >   BI_ETH_CFG -> struct with following:
> >    -- mac addr
> >    -- hdx/fdx; 10/100/1000/...; OR autonegotiate
> >    -- phy addr
> >    -- some bytes for driver specific info (e.g., on gt64260, is_rmii)
>
> Looks fine, see above of course...

Don't forget timebase/decrementer frequency, it is not always easy to
measure and very important for timekeeping.

Still fighting lack of reproducible timebase frequency measurements at
boot on some boards. Not very actively since I've got more urgent
problems and NTP ends up hiding most of it.

	Gabriel.


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