[Pkg-electronics-devel] Bug#1028137: nextpnr: Support for more FPGA architectures (esp. ECP5)

[Daniel Gröber]

Hi Jonathan,

Thanks for the detailed review and testing!

On Sat, Jun 24, 2023 at 12:35:12PM +0200, Jonathan Neuschäfer wrote:
> [ Dropping the Debian mailing lists / bugtracker for now ]

No need, I belive reviews should be public :) I've bounced your mail back
to BTS/pkg-electronics.

> Some notes:
> 
> prjtrellis:
> 
> - I tried to build it from source (using the tarballs, with dpkg-buildpackage),
>   but it failed because it can't find version.cpp. After a dirty hack to ensure
>   that version.cpp is present and usable, and unpacking the orig-database
>   tarball to the right location, the package builds for me.

Did you build with git/gbp or using the dsc? I see that the build system
tries to extract the version using git commands. However I always test with
sbuild so my build environment should also have been a plain unpacked dsc
and not a git repo. Strange.

AFAICT I can define CURRENT_GIT_VERSION from d/rules and that should take
care of it.

> apycula:
> 
> - I think the source package name should rather be apicula (with 'i',
>   not 'y'), which matches the upstream repo and readme.md. Apycula is
>   the python binding, AFAIUI.

NACK, what I'm packaging here is the a-py-cula pipy.org package which is
the main output of project a-pi-cula.

> - The debian orig tarball contains a few GW1N*.pickle files (in the
>   apycula subdirectory), that don't appear in the upstream git repo.
>   They seem to be generated, and are later used in the nextpnr build, but
>   it's unclear to me where they come from, where the source is.

Indeed I agree it's a bit dodgy and probably warrants an explaination in
d/README.source. So what project apicula did is write fuzzers to
map/document what bits in the FPGA bitstream perform what function.

They do this by running the proprietary FPGA-vendor toolchain using their
automated fuzzing tools. The pickle files encompass the output of these
tools and essentially provide a map of what logical function is where in
the hardware.

The a-pi-cula source repo contains these fuzzin tools and produces as
output the a-py-cula pipy package which is uploaded by the maintainer.

I haven't consulted the sacred scriptures on the finer points yet but IMO
the pickle files contain facts about the physical world, not computer
code. My understanding is facts are not copyrightable in the first place so
the usual derrivative work considerations don't apply.

You can imagine I'm looking forward to ftp-master review ;)

> nextpnr:
> 
> - debian/upstream/metadata: Some of the authors have updated names that
>   should be used instead of the old names. As far as I can gather:
>    - Myrtle Shah[1] (a.k.a. gatecat)
>    - Claire Xenia Wolf[2] (a.k.a. Claire Xen)

This one is tricky. The license asks that the notices in the source be
preserved, so it's not clear to me we can honor such name changes without
explicit permission from the respective copyright holder.

> - Because I removed the GWIN*.pickle files of unknown origin, I skipped the
>   gowin targets.

Do note prjtrellis has the same layout information in the -database
component just in a more human readable format (json). Have a look at that
and you'll get an idea of what sort of data we're talking about here.

> - Building multiple times in the same source directory fails, override_dh_auto_clean needs a
>   "rm -rf debian/nextpnr-*-qt-chipdb/usr/share/nextpnr/chipdb-*.bin" or similar.
>   Or maybe upstream "make clean" needs to be fixed.
> - Somehow the chipdb files are installed to /usr/share/nextpnr/chipdb-*.bin
>   directly, while nextpnr expects them in per-arch subdirectories. As a quick
>   work-around I'm symlinked the expected subdirectories to .
> 
> 
> After all this, I've built a simple test design for ECP5 (OrangeCrab),
> and it worked!

Excellent news!

> Thanks for your work! I hope my comments help to improve it further :)

Definitely :)

Thanks,
--Daniel