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

Hello,

I work in the Xilinx software group.

I'm replying to this email thread because Grant suggested there be a GIT
tree for Virtex specific changes.

I am wondering if the open source community would prefer or see a
benefit to Xilinx owning/hosting the source trees (CVS or GIT or
whatever) for our drivers, and in particular the Linux adapter drivers.
If we did this we would provide a web site with the information along
with instructions on how to submit changes.

We are exploring this idea and wanted to know what others thought of
this.

- John

-----Original Message-----
From: linuxppc-embedded-bounces@ozlabs.org
[mailto:linuxppc-embedded-bounces@ozlabs.org] On Behalf Of Grant Likely
Sent: Tuesday, January 17, 2006 8:41 AM
To: peter.ryser
Cc: Grant Likely; Andrei Konovalov; rick.moleres; linuxppc-embedded
Subject: Re: [PATCH 00/10] Updated ML300 & ML403 patches

Peter Ryser wrote:

=20

quoted

Hmm, did you use the ml403 and ml300 def configs?  What date did you=20
pull Linus' tree?  Kumar and Paul were talking today about some

serial=20

quoted

subsystem breakage on the linux-2.6 tree this weekend... I'll fast=20
forward tonight and try it on my board.=20

=20
=20
Okay, please let me know how this works for you.
=20

quoted

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

=20
=20
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?
=20

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=20
need to have cogito installed)

quoted

quoted

Anyway, there is another issue that I would like to bring up and it=20
has to do with xparameters.h. The xparameters.h file, or more=20
exactly, the xparameters_* file, is automatically generated by EDK=20
and is then used to configure the devices in the Linux kernel at=20
compile time. While I understand the desire to get away from a

static=20

quoted

quoted

device definition to device enumeration at run-time, the current set

quoted

quoted

of patches is a step backwards for users from a useability point of=20
view. Users will now have to modify xparameters*.h by hand which is=20
an error-prone process.=20

Actually, users should *never* modifiy generated files.  The intent

is=20

quoted

that board specific fixups go directly into the top level=20
xparameters.h so that newly generated files don't have to be touched.

quoted

But yes, I understand what you mean.=20

=20
=20
An EDK user is free to choose arbitrary names for his peripherals.=20
Additionally, Base System Builder uses different names for various=20
boards (historically). With that it is impossible to make static=20
assignments in xparameters.h. If you go back to the 2.4 kernel and

have=20

a look at xparameters_ml300.h you can see that the assignment of

boards=20

specific parameters to Linux specific parameters is done in there and=20
that xparameters.h is basically used to chose the proper xparameters_*

file for a given board.

okay

=20

quoted

quoted

Additionally, the original 'redefines' are now replaced with=20
redefines in xparameters.h but differently for every board. I

suggest=20

quoted

quoted

we keep the 2.4 methodology until we can come up with a better=20
approach to enumerate devices at run-time.

Andrei & I are already discussing this.  I'm going to change the=20
xparameters redefines to provide a default set of mappings that can

be=20

quoted

used if xparameters_*.h has the linux specific mappings.=20

=20
=20
Thanks. Why not just use the xparameters_ml300.h file created by the=20
system_linux.xmp in the EDK reference design for the ML403 and rename

it=20

to xparameters_ml403.h for inclusion into the kernel tree? We could

then=20

make a change in EDK, add a parameter that lets the user specify the=20
board he uses, and with that automatically create an

xparameters_ml403.h=20

(or any other board for that matter).

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

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

=20

quoted

However, due to the fact that generated xparam files don't have the=20
Linux redefines if the FPGA engineer doesn't select a linux bsp.

=20
That's not a recommended flow. It's very easy to create an EDK design=20
with the proper settings and since it is very likely that things

change=20

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=20
always *reality*  :p

=20

quoted

  I think it's important to allow user defined 'fixups' for their=20
board. (I've personally worked on a couple of projects where the FPGA

quoted

engineer would not generate the Linux BSP).  Design specific fixups=20
can go into the top level xparameters.h without touching the

generated=20

quoted

file=20

=20
=20
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=20

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=20

the defines in xparameters.h. With the 2.4 kernel methodology that is=20
not necessary as such changes will be represented in a regenerated=20
board-specific xparameters_*.h

???

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

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

=20

quoted

<rant> BTW; it really bugs me that edk will generate different xparam

quoted

files depending on the bsp; why isn't there a single standard set of=20
data that is loaded into all xparam files; regardless of software=20
target?  Some no-OS targets need the same information that a Linux=20
port needs. </rant>=20

=20
=20
EDK creates an xparameters.h that matches the names of the parameters

in=20

the hardware design. However, EDK is capable of assuming other=20
personalities than 'standalone', for example Linux.

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

With the Linux=20
personality it creates the proper files AND directory structure for=20
inclusion into the Linux kernel. Ideally, the source files that are

used=20

to create the Linux bsp for a given FPGA design should be included in=20
the kernel tree and be maintained in there (maybe, in the xparameters=20
directory). I'm not so sure though how well this would be accepted in=20
the community. Opinions?

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

while to coallate.

=20

quoted

I've avoided using the same names as used by the Linux redefines=20
because I don't know how stable the linux bsp naming convention is,=20
and I want to avoid a naming conflict.  If you can *guarantee* me

that=20

quoted

those linux redefines are stable, then I have no problem using them=20
instead of the new defines that are currently in the patch.  If they=20
are not; then I'll just do a one-to-one mapping into a

non-conflicting=20

quoted

namespace, and users can provide custom definitions as needed.=20

=20
=20
The names are stable. They have not changed since xparameters_ml300.h=20
has been initially published to the 2.4 repository and there are no=20
intentions on changing them. And again, we really want to move towards

a=20

structure that allows for detecting peripherals at run-time. That will

improve useability by a magnitude as no recompilation of the Linux=20
kernel will be needed when the FPGA design changes.

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

=20

quoted

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

the=20

quoted

zImage wrapper.  The kernel just won't care.  :)=20

=20
=20
I agree. That's the way to go. Let's work towards that goal and keep=20
xparameters_* as they have been in 2.4 for the moment.
=20

quoted

quoted

Specific to the patch: XPAR_DDR_SIZE is not the same as XPAR_MEM_*.=20
XPAR_DDR_SIZE is specifically defined by the user as part of the BSP

quoted

quoted

generation and indicates how much memory is available for Linux.

This=20

quoted

quoted

can be (and typically is) the same as the physically available

memory=20

quoted

quoted

but can be less than that. On the other hand XPAR_MEM_* can be the=20
same or a multiple of the physically available memory (aliasing for=20
cached and non-cached accesses). Statically defining the memory size

quoted

quoted

in xparameters_ml403.h is not desirable. This is especially true for

quoted

quoted

the multi-processor FPGA devices that might want to share the=20
physically available memory between themselves.

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

=20
=20
Right. Sorry, I was quoting the wrong file. The value should not be=20
hard-coded in embed_config.c but instead XPAR_DDR_SIZE should be used=20
which is defined in xparameters_ml403.h.

ok

=20

quoted

Hmmm, I don't see any XPAR mem defines in xparameters_ml300.h.  (I=20
don't have a copy of the linux xparams for ml403 in front of me at

the=20

quoted

moment)  Is this something new?=20

=20
=20
I was referring to XPAR*MEM*, i.e. the base address and high address=20
definition for the memory in EDK.
=20

quoted

Really, this isn't statically defined anyway.  The bootloader (u-boot

quoted

or zImage) passes the memory size into the kernel; and in fact the=20
kernel command line; or the board setup code can restrict the amount=20
of mem used by the kernel.  XPAR_MEM_* isn't used by the kernel

proper=20

quoted

at all.=20

=20
=20
Agreed.
=20

quoted

Thanks for the comments.=20

=20
=20
Thanks for making this patch available. I know how much hard work it

is=20

to get this done.
=20

quoted

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

=20
=20
Okay.
=20

quoted

If there's enough interest; I'll setup another git tree for the

virtex=20

quoted

specific patches.=20

=20
=20
Hmm, interesting idea. Let's see what others think.
=20
- Peter

cool, thanks.

g.


--=20
Grant Likely, B.Sc. P.Eng.
Secret Lab Technologies Ltd.
(403) 663-0761
_______________________________________________
Linuxppc-embedded mailing list
Linuxppc-embedded@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-embedded