GNU bug report logs -
#78233
[PATCH 0/2 electronics-team] Upgrade nextpnr.
Previous Next
Reported by: Cayetano Santos <csantosb <at> inventati.org>
Date: Sat, 3 May 2025 17:52:02 UTC
Severity: normal
Tags: patch
Done: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Bug is archived. No further changes may be made.
Full log
View this message in rfc822 format
[Message part 1 (text/plain, inline)]
Your bug report
#78233: [PATCH 0/2 electronics-team] Upgrade nextpnr.
which was filed against the guix-patches package, has been closed.
The explanation is attached below, along with your original report.
If you require more details, please reply to 78233 <at> debbugs.gnu.org.
--
78233: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=78233
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems
[Message part 2 (message/rfc822, inline)]
Hi,
Cayetano Santos <csantosb <at> inventati.org> writes:
> n<#secure method=pgpmime mode=sign>
>
> Hi Maxim,
>
> Thanks for all your comments ! They’re (almost) included in v2 series.
>
>>jeu. 08 mai 2025 at 22:07, Maxim Cournoyer <maxim.cournoyer <at> gmail.com> wrote:
>
>> Interesting, I had never seen the opendir/closedir approach, perhaps
>> because the code base in Guix embraces functional rather than imperative
>> style. It'd be more conventional (in Guix at least) to use (ice-9
>> ftw)'s scandir to list the files and apply for-each to its value, and I
>> think it'd be more succinct too.
>>
>> Something like:
>>
>> (with-directory-excursion "icestorm/examples"
>> (for-each (cut invoke "make" "-C" <>)
>> (scandir "."
>> (negate (cut member <> '("." ".."))))))
>
> I just learned about cut existence, thanks again; I understand its utility
> by looking at examples all around the code. No doubt, this is the correct
> alternative.
>
> Unfortunately, I’m unable at this point to use it, I miss the necessary
> skills: I fall in frequent "unbound variable <>" errors that I don’t
> known yet how to handle, I suspect gexps add a extra layer of complexity
> which becomes a barrier to me, sorry for that.
>
>> Hm, this relocation in the same diff as the changes muddled the scope.
>> I thought it was a new entry above, so reviewed it as such. Could you
>> please split the relocation as a prior commit with your actual changes
>> to review in a 2nd commit on top? This should give a nicer view, as
>> Gabriel also noted.
>
> Let’s forget the relocation by now, as it complicates things with no
> real utility by itself. We will sort things out later on.
Change clarity/transparency and easing the review is worth the extra
effort, in my opinion.
> Just consider that, in reality, nextpnr replaces former nextpnr-ice40
> (it is strongly based on it); new nextpnr-ice40 is a new package (even
> if it carries the name of previous one). This is not evident with two commits.
>
> Two commits, without breaking things up, are necessarily somehow
> confusing (and kind of useless, to that matter). To me, the only means
> of achieving a nicer diff view is using a single commit, where one may
> compare new nextpnr with old nextpnr-ice40, at the same time as it becomes
> evident that the new nextpnr-ice40 is an addition.
>
> V2 is in its way.
Thanks for V2, I've used it as the base of nextpnr-ice40 package, for
which I've also added patches, as mentioned previously. By first
updating nextpnr-ice40 to v0.8 and related changes, and later renaming
it to nextpnr with few adjustments needed, the commit history is as
clear as it can be. Please have a look/test to make sure everything
still works as intended.
I didn't keep the phase building the examples as the package already has
a test suite, and I didn't see the gain/effort ratio of doing so as
worth it.
--
Thanks,
Maxim
[Message part 3 (message/rfc822, inline)]
Hi,
This patch series adds a new nextpnr package (strongly based on former nextpnt-ice40), common to all derived nextpnr-xxx, which will inherit from it.
Other device packages will follow (nextpnr-ecp5, which uses prjtrellis as backend, for example). See [0]
First commit
- updates to 0.8
- updates substitutions in Makefiles
Second commit modifies the device specific nextpnr-ice40, which now inherits from nextpnr, using icestorm as a backend. With respect to nextpnr:
- adds icestorm as propagated input
- adds yosys as native input, for tests
- addapts config flags to ice40 architecture
- uses tests from icestorm/examples
[0] https://github.com/YosysHQ/nextpnr/blob/master/README.md
Cayetano Santos (2):
gnu: Add nextpnr.
gnu: nextpnr-ice40: Update to 0.8.
gnu/packages/fpga.scm | 216 +++++++++++++++++++++++-------------------
1 file changed, 119 insertions(+), 97 deletions(-)
base-commit: fa1149d3fd8d2ce94968dd05d5dc08561cb283ed
--
2.49.0
This bug report was last modified 95 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.