Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

applet.interface.uart: convert to nmigen #228

Merged
merged 3 commits into from Dec 29, 2020
Merged

applet.interface.uart: convert to nmigen #228

merged 3 commits into from Dec 29, 2020

Conversation

attie
Copy link
Member

@attie attie commented Nov 8, 2020

Addresses interface.uart item from #150.

@attie attie force-pushed the uart-nmigen branch 4 times, most recently from a741360 to 5b50d7e Compare November 9, 2020 01:03
@attie
Copy link
Member Author

attie commented Nov 9, 2020 •
edited

I'm aware of the failed test, but can't replicate it here... When running all tests, I see the following ("same" error, different test).

venv:$ python -W ignore::DeprecationWarning test.py
[...]
======================================================================
ERROR: test_build (glasgow.applet.memory.prom.MemoryPROMAppletTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/attie/proj_local/glasgow/glasgow/software/glasgow/applet/memory/prom/__init__.py", line 872, in test_build
    self.assertBuilds()
  File "/home/attie/proj_local/glasgow/glasgow/software/glasgow/applet/__init__.py", line 232, in assertBuilds
    target.build_plan().execute()
  File "/home/attie/proj_local/glasgow/glasgow/software/glasgow/target/hardware.py", line 118, in execute
    products  = self.lower.execute_local(build_dir)
  File "/home/attie/proj_local/glasgow/nmigen/nmigen/build/run.py", line 98, in execute_local
    subprocess.check_call(["sh", "{}.sh".format(self.script)])
  File "/home/attie/.bin/python3.8.2/lib/python3.8/subprocess.py", line 364, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sh', 'build_top.sh']' returned non-zero exit status 1.

----------------------------------------------------------------------
Ran 238 tests in 168.288s

FAILED (errors=1, skipped=7)

When running the test specifically, I see:

venv:$ python -W ignore::DeprecationWarning test.py -vk glasgow.applet.interface.uart.UARTAppletTestCase
test_build (glasgow.applet.interface.uart.UARTAppletTestCase) ... Warning: Wire top.$flatten\i2c_target.\bus.$verilog_initial_trigger has an unprocessed 'init' attribute.
Warning: Wire top.$verilog_initial_trigger has an unprocessed 'init' attribute.
ok
test_loopback (glasgow.applet.interface.uart.UARTAppletTestCase) ... ok

----------------------------------------------------------------------
Ran 2 tests in 5.379s

OK

I have also seen errors such as below, when my code is bad. I was considering raising an issue, though not sure if it's Yosys, nMigen or Glasgow.

venv:$ glasgow test uart
I: g.cli: testing applet 'uart'
test_build (glasgow.applet.interface.uart.UARTAppletTestCase) ...
ERROR: Can't open input file `/tmp/yosys-abc-3pHqzP/output.aig' for reading: No such file or directory
ERROR
Traceback (most recent call last):
  File "/home/attie/proj_local/glasgow/glasgow/software/glasgow/applet/interface/uart/__init__.py", line 381, in test_build
    self.assertBuilds()
  File "/home/attie/proj_local/glasgow/glasgow/software/glasgow/applet/__init__.py", line 232, in assertBuilds
    target.build_plan().execute()
  File "/home/attie/proj_local/glasgow/glasgow/software/glasgow/target/hardware.py", line 118, in execute
    products  = self.lower.execute_local(build_dir)
  File "/home/attie/proj_local/glasgow/nmigen/nmigen/build/run.py", line 98, in execute_local
    subprocess.check_call(["sh", "{}.sh".format(self.script)])
  File "/home/attie/.bin/python3.8.2/lib/python3.8/subprocess.py", line 364, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sh', 'build_top.sh']' returned non-zero exit status 1.

@whitequark
Copy link
Member

It is not your responsibility to fix the build error. (As in, I'll look into it, but not necessarily soon.)

@attie
Copy link
Member Author

attie commented Nov 9, 2020 •
edited

Ok, thanks.

The gateware and applet work, I've had to tweak things in the tests for the nmigen.compat -> nmigen, but I think it should be fine as it is now.

@whitequark
Copy link
Member

whitequark commented Nov 10, 2020 •
edited

Okay, so the error is:

The command has to terminate. Boxes are not in a topological order.
The following information may help debugging (numbers are 0-based):
Input 0 of BoxA 1 (1stCI = 474; 1stCO = 1) has TFI with CI 509,
which corresponds to output 1 of BoxB 34 (1stCI = 508; 1stCO = 38).
In a correct topological order, BoxB should precede BoxA.
Error: Abc_CommandAbc9If(): Mapping of GIA has failed.
** cmd error: aborting 'source /tmp/yosys-abc-000000/abc.script'
ERROR: Can't open input file `/tmp/yosys-abc-000000/output.aig' for reading: No such file or directory
E.ABC command line: "source /tmp/yosys-abc-000000/abc.script".

This comes from ABC, so ultimately this is a bug to be filed against Yosys.

Is your Yosys a very recent build from git? If not, that's probably why you don't see it. Try upgrading it, or use the CI configuration file to use YoWASP on your machine.

@whitequark
Copy link
Member

@attie Can you rebase? I think post #251 the failure might be fixed.

@attie attie mentioned this pull request Dec 29, 2020
Closed
30 tasks
@attie attie changed the title UART: convert to nmigen applet.interface.uart: convert to nmigen Dec 29, 2020
whitequark
whitequark requested changes Dec 29, 2020
View reviewed changes
Copy link
Member

@whitequark whitequark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See inline comments.

whitequark
whitequark previously approved these changes Dec 29, 2020
View reviewed changes
Copy link
Member

@whitequark whitequark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LGTM, not sure why tests are failing...

@attie-argentum
gateware.uart: convert to nmigen
728c857
@attie-argentum
gateware.uart: support words longer than 8-bits
0f9d3d4 
It seems that the shift register was being truncated to 8-bits
regardless of the requested word size, since the `data_bits`
attribute was introduced in 0f497e4.
@attie
Copy link
Member Author

attie commented Dec 29, 2020 •
edited

My fault, I added a missing .oe.eq(1) (the UART Tx was non-functional), and didn't realise that I needed to update Pin(width=1, dir='oe') too (from dir='o').

@whitequark whitequark merged commit 42d7cf7 into GlasgowEmbedded:master Dec 29, 2020
@whitequark
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@whitequark whitequark whitequark approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@attie @whitequark @attie-argentum