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

Add support for cmod_a7 and cmod_s7 board. #185

Merged
merged 17 commits into from
Mar 7, 2023
Merged

Add support for cmod_a7 and cmod_s7 board. #185

merged 17 commits into from
Mar 7, 2023

Conversation

bl0x
Copy link
Contributor

@bl0x bl0x commented Oct 8, 2021

I just got a Cmod A7 35 today to play around with. So, here ya go!

rroohhh
rroohhh reviewed Oct 8, 2021
View reviewed changes
Copy link
Contributor

@rroohhh rroohhh left a comment

Choose a reason for hiding this comment

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

Is there a specific reason you are leaving out the (P)SRAM and the 1-wire crypto element?

nmigen_boards/cmod_a7.py Outdated Show resolved Hide resolved
nmigen_boards/cmod_a7.py Outdated Show resolved Hide resolved
rroohhh
rroohhh reviewed Oct 8, 2021
View reviewed changes
nmigen_boards/cmod_a7.py Outdated Show resolved Hide resolved
@bl0x
Copy link
Contributor Author

bl0x commented Oct 8, 2021

@rroohhh , thanks for the review and comments!

crypto: I have no idea what this pin is for, and what is connected to it. But no problem, I could add it as a Connector.

sram: I forgot about it, mainly because I don't need it (now). I'll put it in.

@bl0x
Copy link
Contributor Author

bl0x commented Oct 13, 2021

Dear @rroohhh , am I still missing something?

rroohhh
rroohhh reviewed Oct 13, 2021
View reviewed changes
nmigen_boards/cmod_a7.py Outdated Show resolved Hide resolved
rroohhh
rroohhh reviewed Oct 13, 2021
View reviewed changes
nmigen_boards/cmod_a7.py Outdated Show resolved Hide resolved
rroohhh
rroohhh reviewed Oct 13, 2021
View reviewed changes
nmigen_boards/cmod_a7.py Outdated Show resolved Hide resolved
rroohhh
rroohhh reviewed Oct 13, 2021
View reviewed changes
nmigen_boards/cmod_a7.py Outdated Show resolved Hide resolved
rroohhh
rroohhh reviewed Oct 14, 2021
View reviewed changes
nmigen_boards/cmod_a7.py Outdated Show resolved Hide resolved
@bl0x
Copy link
Contributor Author

bl0x commented Oct 21, 2021

I think that all things are fixed now.

@bl0x
Copy link
Contributor Author

bl0x commented Nov 24, 2021

I've rebased to current nmigen:master

@bl0x
Copy link
Contributor Author

bl0x commented Dec 10, 2021

@rroohhh Am I missing something to make this ready for merging?

@bl0x
Copy link
Contributor Author

bl0x commented Dec 19, 2021

Hi,

I've migrated from nmigen->amaranth.
The failing test is for genesys2, so unrelated to these commits.

@rroohhh
Copy link
Contributor

rroohhh commented Dec 19, 2021 •
edited
Loading

@rroohhh Am I missing something to make this ready for merging?

Looks fine from what I can see, but @whitequark is the maintainer, I was just interested in the board and took a look :)

@bl0x
Copy link
Contributor Author

bl0x commented Jan 23, 2022

I rebased onto main and updated. Also fixed two more things.

@bl0x
Copy link
Contributor Author

bl0x commented Jan 23, 2022

This PR now depends on PR 054b5c1 (or a similar solution) to be merged. Otherwise the test for genesys2 will fail again.

@bl0x
Copy link
Contributor Author

bl0x commented Oct 13, 2022

Hi all,

I've also added support for the CMOD S7 board (very similar) and switched from using openocd to openFPGALoader.
I'd be happy to get this merged.

Tests fail because of broken CI #200 .

@bl0x
Revert "Also allow strings as keys when using dicts to describe pins.…
a7334ed 
… Does not allow mixing of ints and strings. The string key is then used instead of default_name in _SplitResource."

This reverts commit 58710cc.

The reason is that this proposal was not accepted.
@bl0x
Remove nmigen compatibility.
24d0cce
@bl0x
Copy link
Contributor Author

bl0x commented Feb 24, 2023 •
edited
Loading

Updated this branch to remove nmigen compat layer and revert the change that introduced allowing strings as keys for describing pins. Now, the changes are just the two new board files.

@bl0x bl0x requested a review from whitequark March 6, 2023 21:11
@bl0x
Copy link
Contributor Author

bl0x commented Mar 6, 2023

Sorry for nagging, but it would be very nice to get this merged. Is anything still missing?

@bl0x bl0x changed the title Add support for cmod_a7 board. Add support for cmod_a7 and cmod_s7 board. Mar 6, 2023
whitequark
whitequark approved these changes Mar 7, 2023
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.

LGTM

@whitequark whitequark merged commit ce26e35 into amaranth-lang:main Mar 7, 2023
@whitequark
Copy link
Member

Thank you!

@bl0x
Copy link
Contributor Author

bl0x commented Mar 7, 2023

great, thanks!

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

@rroohhh rroohhh rroohhh left review comments

@whitequark whitequark whitequark approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@bl0x @rroohhh @whitequark