Subject: Re: [PATCH 00/10] Updated ML300 & ML403 patches

From: Grant Likely <[email-masked]>
Date: Tue, 17 Jan 2006 00:39:33 -0700

Let's keep this conversation on the mailing list.

Peter Ryser wrote:
> Hi Grant and Andrei,
> 
> I'm glad to see some activity on the linuxppc email alias for the MLxxx 
> boards and appreciate the work put into moving the 2.4 support to 2.6.
> 
> I just tried to boot the 2.6 kernel with Grant's patches applied to 
> Linus' latest tree on both the ML300 and the ML403 and in both cases end 
> up with the PLB Error LED lit up. Both boards print the messages from 
> the bootloader, print "Now booting the kernel" and then nothing (but the 
> error LED). Anything you can think of that is going wrong?

Hmm, did you use the ml403 and ml300 def configs?  What date did you 
pull Linus' tree?  Kumar and Paul were talking today about some serial 
subsystem breakage on the linux-2.6 tree this weekend... I'll fast 
forward tonight and try it on my board.

Try seeking to commit: 67daf5f11f06b9b15f8320de1d237ccc2e74fe43
That's what I generated the latest patches against.

> 
> Anyway, there is another issue that I would like to bring up and it has 
> to do with xparameters.h. The xparameters.h file, or more exactly, the 
> xparameters_* file, is automatically generated by EDK and is then used 
> to configure the devices in the Linux kernel at compile time. While I 
> understand the desire to get away from a static device definition to 
> device enumeration at run-time, the current set of patches is a step 
> backwards for users from a useability point of view. Users will now have 
> to modify xparameters*.h by hand which is an error-prone process. 

Actually, users should *never* modifiy generated files.  The intent is 
that board specific fixups go directly into the top level xparameters.h 
so that newly generated files don't have to be touched.  But yes, I 
understand what you mean.

> Additionally, the original 'redefines' are now replaced with redefines 
> in xparameters.h but differently for every board. I suggest we keep the 
> 2.4 methodology until we can come up with a better approach to enumerate 
> devices at run-time.

Andrei & I are already discussing this.  I'm going to change the 
xparameters redefines to provide a default set of mappings that can be 
used if xparameters_*.h has the linux specific mappings.

However, due to the fact that generated xparam files don't have the 
Linux redefines if the FPGA engineer doesn't select a linux bsp.  I 
think it's important to allow user defined 'fixups' for their board. 
(I've personally worked on a couple of projects where the FPGA engineer 
would not generate the Linux BSP).  Design specific fixups can go into 
the top level xparameters.h without touching the generated file

<rant> BTW; it really bugs me that edk will generate different xparam 
files depending on the bsp; why isn't there a single standard set of 
data that is loaded into all xparam files; regardless of software 
target?  Some no-OS targets need the same information that a Linux port 
needs. </rant>

I've avoided using the same names as used by the Linux redefines because 
I don't know how stable the linux bsp naming convention is, and I want 
to avoid a naming conflict.  If you can *guarantee* me that those linux 
redefines are stable, then I have no problem using them instead of the 
new defines that are currently in the patch.  If they are not; then I'll 
just do a one-to-one mapping into a non-conflicting namespace, and users 
can provide custom definitions as needed.

This really isn't a big deal anyway; most of this discussion will become 
moot in short order.  Sometime in the next few releases, linuxppc will 
flip over to using a flattened device tree to pass device information 
from the boot loader to the kernel.  xparameters will drop out of the 
kernel proper entirely except for the edk-generated device drivers 
(which is another issue entirely).  All the xparam stuff will be 
extracted into a device tree by u-boot or the zImage wrapper.  The 
kernel just won't care.  :)

>
> Specific to the patch: XPAR_DDR_SIZE is not the same as XPAR_MEM_*. 
> XPAR_DDR_SIZE is specifically defined by the user as part of the BSP 
> generation and indicates how much memory is available for Linux. This 
> can be (and typically is) the same as the physically available memory 
> but can be less than that. On the other hand XPAR_MEM_* can be the same 
> or a multiple of the physically available memory (aliasing for cached 
> and non-cached accesses). Statically defining the memory size in 
> xparameters_ml403.h is not desirable. This is especially true for the 
> multi-processor FPGA devices that might want to share the physically 
> available memory between themselves.

As you can see in embed_config.c; I already discovered this the hard way 
   :(

Hmmm, I don't see any XPAR mem defines in xparameters_ml300.h.  (I don't 
have a copy of the linux xparams for ml403 in front of me at the moment) 
  Is this something new?

Really, this isn't statically defined anyway.  The bootloader (u-boot or 
zImage) passes the memory size into the kernel; and in fact the kernel 
command line; or the board setup code can restrict the amount of mem 
used by the kernel.  XPAR_MEM_* isn't used by the kernel proper at all.

> - Peter

Thanks for the comments.

Another issue we need to discuss is if/how to support the xilinx 
generated BSP in the kernel proper; but I'll leave that for a different 
email.

If there's enough interest; I'll setup another git tree for the virtex 
specific patches.

g.

-- 
Grant Likely, B.Sc. P.Eng.
Secret Lab Technologies Ltd.
(403) 663-0761