[U-Boot-Users] FPGA mess

[Stefan Roese]

Andre,

On Thursday 06 March 2008, Andre Schwarz wrote:
> sorry for being impatient ... I'm just trying to get clean and universal
> code with no need for recoding twice a year.

No problem.

> Stefan Roese schrieb:
> > Andre,
> >
> > On Wednesday 05 March 2008, Andre Schwarz wrote:
> >> you're listed as maintainer for the alpr board.
> >
> > Correct. But I didn't use this board for quite some time
>
> ok - no problem. I just need to start somewhere.

Good.

> >> It's the only board that uses the ACEX1K.c file for FPGA loading...
> >
> > Does it? To use this file you have to define CONFIG_FPGA_ACEX1K. I don't
> > see it defined anywhere in the whole U-Boot tree. So perhaps this code is
> > not used at all.
>
> correct - so you can kick it away.

If no official board uses it and it looks broken to you, then please send a 
patch to remove it.

> There's a include/ACEX1K.h that introduces two interfaces for obviously
> the same funtionality.
> The first one is specific to ACEX1K and the other one is a general
> cyclone implementation as it should be.

Please clean it up too.

> >> I'm quite sure there are many boards with Altera FPGAs outside and can't
> >> believe they have all mounted
> >> platform flashes and therefore don't use u-boot for loading the FPGA.
> >> Nevertheless those board don't show up in the tree. Maybe they all keep
> >> their own patches like me ...
> >
> > Maybe. I suggest you start pushing your code to change this situation. :)
>
> I've been trying to submit patches for years now - there's always a
> point when you give up.
> If there's a will to change this and to discard unused/scrambled code
> I'll provide a patch as soon as my board is up and running.

As you may know, we changed our development infrastructure by adding 
submaintainers (custodians) for subsystems. Currently this is working not 
perfect, but seems to get better over the time. So we are improving here.

Unfortunately we don't have a custodian for the FPGA stuff. Since it's getting 
more and more complex, it would be great if somebody would step up here. Any 
volunteers? :)

> >> There are some points that doesn't seem to work out very well in
> >> general. This is where some questions come up (top->down) :
> >>
> >> common/cmd_fpga:
> >> In function do_fpga there's a used environment variable "fpgadata" that
> >> obviously stores the location of the bitstream.
> >> What sense does this make if the "data_size" parameter (=arg4) is _not_
> >> optional ?
> >> Instead the command "fpga load 0 0x..." leads to a load function with
> >> zero length.
> >> The user always has to supply all 4 args (load, nr, data, size).
> >> Of course the loading function could use the pre-defined bitstream size
> >> from the header or the device struct...
> >
> > I have to admit that I don't understand what you are really asking here.
> > Please ask more specific questions. Or send a patch to fix something that
> > is "broken".
>
> ok - you'll get a patch.
> The point is that it makes no sense to provide a function "fpga"
> (declared in common/cmd_fpga) that accepts up to 4 args with
> arg2 and arg3 being optional and arg4 not ...

Right. With this description it makes sense to me. But we have to be careful 
here, not to change the command syntax in a non backward compatible way.
 
> >> common/altera.c
> >> What's that ACEX1K ? Isn't it a Cyclone chip and should use that
> >> interface ? Why does this need special treatment throughout the
> >> interface ?
> >
> > I didn't introduce this file. Perhaps we should ask the author who
> > committed this code.
>
> ok - can you do this ?

Yes. Steven Scholz as original author is on CC.

> >> include/ACEX1K.h
> >> Obviously there are some confusions about the various file formats and
> >> sizes that can be output
> >> from Altera's SoPC Builder. Compression is also possible with
> >> de-compression on the fly during load ...
> >> Of course the defined file sizes should match a raw bit file that
> >> represents the true size of the device.
> >>
> >> Why is ACEX1K and Cyclone not merged ?
> >
> > No idea. If you have some fixes, please send patches.
> >
> >> Does _any_ real board use the Altera path ? scanning the config files
> >> ... no.
> >
> > alpr seems to use the cyclone2.c code.
>
> no - it uses common/ACEX1K.c .... and common/cyclon2.c is derived from it.

No, it doesn't use common/ACEX1K.c.

> >> CYC2_ps_load in common/cyclon2.c the nCONFIG pin is never de-asserted
> >> during preparation. This code can't work.
> >
> > No idea. I have to admit, that I didn't implement the FPGA booting on
> > this board. But it seems to work fine.
>
> yes - because it's a board specific implementation with an "general"
> interface.

???

> >> Is there any interest in getting this fixed ?
> >
> > Sure.
>
> But implementing the Altera path in a clean way means discarding the
> ACEX1K and breaking the alpr borad.
> I'm quite sure that Wolfgang will reject those changes.

Yes, I will reject this too. :)

> How can we solve this ?

By trying to solve it in a compatible way. I added Heiko Schocher to CC too, 
since he was responsible for the FPGA booting implementation of the alpr 
board.

> >> What about Liberty's Stratix code ? It's living and working !
> >
> > Sorry, but I have no clue what "Liberty's Stratix code" is.
>
> Look out for mails from "eran.liberty". Stratix and Arria are high-end
> FPGA families from Altera.
> There's some work left to get those chips implemented well.

Sorry, I currently don't have the time to read through this whole email 
thread. Please write an answer to this thread if you have some comments 
there.

Thanks.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================