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

[fpga] Increase CW310 base clock frequency to 24 MHz #19479

Merged
merged 3 commits into from
Sep 1, 2023

Conversation

a-will
Copy link
Contributor

@a-will a-will commented Aug 18, 2023

Cherry-pick of #19368 and #19397

This should bring back binary compatibility between the earlgrey_es and master branches for cached FPGA images.

@a-will a-will requested a review from a team as a code owner August 18, 2023 15:52
@a-will a-will requested review from moidx, cfrantz, msfschaffner and a team and removed request for a team August 18, 2023 15:52
@a-will
Copy link
Contributor Author

a-will commented Aug 18, 2023

We probably should consider a separate bucket for cached images for earlgrey_es, though (with a different expiration policy). master should be allowed to continue development, and that means there will be breaking changes at some point.

@moidx moidx removed the request for review from a team August 18, 2023 16:04
Copy link
Contributor

@moidx moidx left a comment

Choose a reason for hiding this comment

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

Thanks @a-will!

@a-will
Copy link
Contributor Author

a-will commented Aug 18, 2023

Or... if not a different expiration policy for the bucket, maybe a release tag with artifacts uploaded to GitHub.

@moidx
Copy link
Contributor

moidx commented Aug 18, 2023

We are planning to use gs://hotel-california for design archival process. We are still pending uploading the earlgrey_es release collateral. I am not sure if this is also a good place to place bitstream updates that require no TTL restrictions.

@moidx
Copy link
Contributor

moidx commented Aug 18, 2023

CC @timothytrippel

@a-will
Copy link
Contributor Author

a-will commented Aug 18, 2023

So this isn't going to work with the presigned ROM images from before the frequency change. What shall we do? Do those tests still provide value on the FPGA?

@timothytrippel
Copy link
Contributor

I can pull down these changes, regen the images and resign them, and re-push them if you want?

@a-will
Copy link
Contributor Author

a-will commented Aug 18, 2023

I can pull down these changes, regen the images and resign them, and re-push them if you want?

That also works. I'll leave it up to you whether you would prefer that over removing the test for CW310 (for example). :)

lschuermann pushed a commit to lschuermann/tock that referenced this pull request Aug 22, 2023
This is in accordance with an upstream change [1]. While the register
set and other chip attributes are frozen for EarlGrey, the clock
frequencies are not. This change will also be backported to the
lowRISC/opentitan `earlgrey_es` branch [2].

[1]: lowRISC/opentitan#19368
[2]: lowRISC/opentitan#19479
@timothytrippel
Copy link
Contributor

I pulled this down and signed the images (I added a single commit with the signed images to make sure they pass CI before squashing), and tried to force push update this branch with git push a-will earlgrey_es --force and I am getting an error as if the remote branch does not exist:

Screenshot 2023-08-26 at 10 47 39 AM

Which is odd because it does seem to exist, perhaps I don't have permissions to update it?

Put entropy_src into auto mode so the test doesn't run out.

Signed-off-by: Alexander Williams <awill@opentitan.org>
@timothytrippel
Copy link
Contributor

timothytrippel commented Aug 31, 2023

I was able to update the branch now, it was a configuration error on my side, my apologies. Let's see if this passes CI, then I can squash the last commit.

Set clock frequencies for CW310 to the following

clk_main:    24 MHz
clk_io_div4: 6 MHz

Also improve the SPI_HOST timing constraints to have the correct max
frequency, in addition to multicycle paths reflecting the correct
capture edges.

Lastly, re-sign pre-signed ROM e2e tests as these were updated.

Co-authored-by: Tim Trippel <ttrippel@google.com>
Signed-off-by: Alexander Williams <awill@opentitan.org>
The test did not initialize the entropy complex to auto mode, and it
would exhaust the available, finite entropy generated during boot,
leading to freezing and timeouts.

Add this test back to the FPGA suite.

Signed-off-by: Alexander Williams <awill@opentitan.org>
@timothytrippel
Copy link
Contributor

Since CI passed, I squashed my signing commit to the one that updates the binaries, to maintain the invariant that each commit builds cleanly. This should be read to merge once CI passes again.

@timothytrippel timothytrippel added the Status:Ready to merge PR is ready to be merged by a committer. label Aug 31, 2023
@timothytrippel timothytrippel merged commit 13c735c into lowRISC:earlgrey_es Sep 1, 2023
23 checks passed
@a-will a-will deleted the earlgrey_es branch September 11, 2023 09:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status:Ready to merge PR is ready to be merged by a committer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants