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.

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

add user LED support in master, satellite variants for Kasli v2.0 and Kasli-SoC #2262

Merged
merged 3 commits into from
Nov 8, 2023

Conversation

SimonRenblad
Copy link
Contributor

@SimonRenblad SimonRenblad commented Oct 18, 2023

ARTIQ Pull Request

Description of Changes

From #2245:

artiq/frontend/artiq_ddb_template.py does not automatically add led0 and led1 for all kasli variants. Currently they are only added for standalone. This makes getting started harder for new users and debugging of existing systems harder.

Changes:

  • Gateware generates user LEDs for satellite and master variants in Kasli v2.0
  • Added missing user LEDs not located on front panel
  • artiq_ddb_template now generates a variable amount of board LEDs depending on board type, version and role

artiq_ddb_template testing

ddb_template_test
read 'M' = master, 'S' = satellite

Related Issue

#2245

Type of Changes

Type
✨ New feature

Steps

All Pull Requests

  • Use correct spelling and grammar.
  • Check the copyright situation of your changes and sign off your patches (git commit --signoff, see copyright).

Code Changes

  • Run flake8 to check code style (follow PEP-8 style). flake8 has issues with parsing Migen/gateware code, ignore as necessary.
  • Test your changes or have someone test them. Mention what was tested and how.
    • Test with experiment on Kasli v2.0 master-satellite config
    • Test with experiment on Kasli-SoC master-satellite config
    • Test Kasli v2.0 ddb template generates additional LEDs
    • Test Kasli-SoC ddb template generates additional LEDs
    • Test Kasli v1.1 idle kernel with additional user_leds

Git Logistics

  • Split your contribution into logically separate changes (git rebase --interactive). Merge/squash/fixup commits that just fix or amend previous commits. Remove unintended changes & cleanup. See tutorial.
  • Write short & meaningful commit messages. Review each commit for messages (git show). Format:
    topic: description. < 50 characters total.
    
    Longer description. < 70 characters per line
    

Licensing

See copyright & licensing for more info.
ARTIQ files that do not contain a license header are copyrighted by M-Labs Limited and are licensed under LGPLv3+.

@SimonRenblad SimonRenblad marked this pull request as ready for review October 24, 2023 03:12
@jbqubit
Copy link
Contributor

jbqubit commented Oct 24, 2023

Why do you skip support of L2 on Kasli-SOC?

@SimonRenblad
Copy link
Contributor Author

Why do you skip support of L2 on Kasli-SOC?

As I mentioned in #2245 L2 is currently missing from the migen repo, and as such cannot be added to the gateware until that is fixed. Support for L2 can be easily added to artiq_ddb_template once we have that fixed.

@sbourdeauducq
Copy link
Member

Just add it then. It's a 1-line change no?

@SimonRenblad
Copy link
Contributor Author

Just add it then. It's a 1-line change no?

Pushed up PR to migen: m-labs/migen#280

@SimonRenblad
Copy link
Contributor Author

Just add it then. It's a 1-line change no?

This was my initial thought as well. However as mentioned here m-labs/migen#280 user led 2 is controlled by the ps7 and would need edits in the migen-axi gateware and potentially the firmware to make available. It's being worked on at the moment and will update on any developments.

@sbourdeauducq
Copy link
Member

PS LED sounds complicated to connect to RTIO and doesn't seem worthwhile.

@SimonRenblad
Copy link
Contributor Author

PS LED sounds complicated to connect to RTIO and doesn't seem worthwhile.

Agreed. The current PR should be sufficient to ensure functionality of the other LEDs as described and will de-prioritize adding the Kasli-SoC L2 LED.

@sbourdeauducq
Copy link
Member

Needs update of RELEASE_NOTES

Add additional LED RTIO devices.
Add support for additional user LEDs.
RELEASE_NOTES.rst Outdated Show resolved Hide resolved
@jbqubit
Copy link
Contributor

jbqubit commented Nov 8, 2023

Thanks guys!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants