[PATCH 00/10] Updated ML300 & ML403 patches

[Grant Likely]

Peter Ryser wrote:
> 
>> 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. 
> 
> 
> Okay, please let me know how this works for you.
> 
>> Try seeking to commit: 67daf5f11f06b9b15f8320de1d237ccc2e74fe43
>> That's what I generated the latest patches against. 
> 
> 
> Hmm, I only recently switched to using git. Is this number string some 
> kind of a tag that I can synchronize my local git tree to? If so, how?
> 

Yea, the number is kind of like a raw tag without a name associated with 
it.  The cg-seek command can be used to get you there.  (But you also 
need to have cogito installed)

>>> 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. 
> 
> 
> An EDK user is free to choose arbitrary names for his peripherals. 
> Additionally, Base System Builder uses different names for various 
> boards (historically). With that it is impossible to make static 
> assignments in xparameters.h. If you go back to the 2.4 kernel and have 
> a look at xparameters_ml300.h you can see that the assignment of boards 
> specific parameters to Linux specific parameters is done in there and 
> that xparameters.h is basically used to chose the proper xparameters_* 
> file for a given board.

okay

> 
>>> 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. 
> 
> 
> Thanks. Why not just use the xparameters_ml300.h file created by the 
> system_linux.xmp in the EDK reference design for the ML403 and rename it 
> to xparameters_ml403.h for inclusion into the kernel tree? We could then 
> make a change in EDK, add a parameter that lets the user specify the 
> board he uses, and with that automatically create an xparameters_ml403.h 
> (or any other board for that matter).

I don't understand what you mean.  It sounds like your suggesting I do 
exactly opposite what you're arguing; hand modify one of the 
xparameters_*.h files.  Are you saying that edk can't generate Linux 
redefines for the ml403 at the moment?

I do *not* think I should replace the edk-generated xparameters_ml403.h 
with a hacked xparameters_ml300.h file.  I'd rather use the generated 
_ml403 file and change the infrastructure when the Linux redefines are 
ready.

> 
>> 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.
>
> 
> That's not a recommended flow. It's very easy to create an EDK design 
> with the proper settings and since it is very likely that things change 
> during the design process of the FPGA the small investment into making 
> the proper settings in the tool will save a lot of time in the end.

I understand that it's not *recommended*; I'm just saying it's not 
always *reality*  :p

> 
>>   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 
> 
> 
> I strongly believe that this approach fixes things in the wrong place. 
> The correct thing to do is to use EDK to create a proper xparameters_*.h 
> that matches the FPGA design. In your methodology, if the user decides 
> to change the peripheral names in EDK he will have to go back and change 
> the defines in xparameters.h. With the 2.4 kernel methodology that is 
> not necessary as such changes will be represented in a regenerated 
> board-specific xparameters_*.h

???

Yes; but I already said that I'll change the patch to use the Xilinx 
redefines.  My argument is simply that *if* changes are required, there 
is a way for the user to do it.  In the normal (recommended) case; 
nothing will need to be done.  (think Larry Wall's quote: "easy things 
easy; hard things possible)

When it is needed; the fixups will be in xparameters.h; not 
xparameters_*.h; and they'll be for a specific port.  The fixups will 
only need to be done once per project (most likely).

> 
>> <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> 
> 
> 
> EDK creates an xparameters.h that matches the names of the parameters in 
> the hardware design. However, EDK is capable of assuming other 
> personalities than 'standalone', for example Linux.

My point is that the Linux redefines are useful to more than just Linux 
ports.  Don't you think standalone apps could also benefit from a 
sane-set of defines for peripherals?  In other words; shouldn't the 
Linux redefines be always available (and called something more generic)?

> With the Linux 
> personality it creates the proper files AND directory structure for 
> inclusion into the Linux kernel. Ideally, the source files that are used 
> to create the Linux bsp for a given FPGA design should be included in 
> the kernel tree and be maintained in there (maybe, in the xparameters 
> directory). I'm not so sure though how well this would be accepted in 
> the community. Opinions?

I'll get back to you on this; I've got some thoughts; but they'll take a 
while to coallate.

> 
>> 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. 
> 
> 
> The names are stable. They have not changed since xparameters_ml300.h 
> has been initially published to the 2.4 repository and there are no 
> intentions on changing them. And again, we really want to move towards a 
> structure that allows for detecting peripherals at run-time. That will 
> improve useability by a magnitude as no recompilation of the Linux 
> kernel will be needed when the FPGA design changes.

okay, I'll change the patch to use those names.

> 
>> 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.  :) 
> 
> 
> I agree. That's the way to go. Let's work towards that goal and keep 
> xparameters_* as they have been in 2.4 for the moment.
> 
>>> 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   :( 
> 
> 
> Right. Sorry, I was quoting the wrong file. The value should not be 
> hard-coded in embed_config.c but instead XPAR_DDR_SIZE should be used 
> which is defined in xparameters_ml403.h.

ok

> 
>> 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? 
> 
> 
> I was referring to XPAR*MEM*, i.e. the base address and high address 
> definition for the memory in EDK.
> 
>> 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. 
> 
> 
> Agreed.
> 
>> Thanks for the comments. 
> 
> 
> Thanks for making this patch available. I know how much hard work it is 
> to get this done.
> 
>>
>>
>> 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. 
> 
> 
> Okay.
> 
>> If there's enough interest; I'll setup another git tree for the virtex 
>> specific patches. 
> 
> 
> Hmm, interesting idea. Let's see what others think.
> 
> - Peter

cool, thanks.

g.


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