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