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

[cw] Update CW Python dependency #173

Merged
merged 1 commit into from
Oct 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions cw/util/device.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,10 @@ def initialize_scope(self, scope_gain, num_samples, offset, pll_frequency):
scope.io.tio2 = "serial_rx"
scope.io.hs2 = "disabled"

# Make sure that clkgen_locked is true.
if husky:
scope.clock.clkgen_src = 'extclk'

Comment on lines +155 to +158
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this right? The comment seems not to be in line with the code. Also, we already set scope.clock.clkgen_src = 'extclk' above on Line 127.

Copy link
Member Author

Choose a reason for hiding this comment

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

Without this code line, the PLL was not locked making the ADC unable to lock itself. According to this, again setting clkgen_src fixes the issue, which also worked on my site.

Copy link
Collaborator

Choose a reason for hiding this comment

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

agree, odd, seems to be done twice now. does the if in L125 work properly?
I'd guess it evaluates false there even on husky?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is probably what prevented me from separating CW310 and Husky into two separate classes ... so I'd be interested :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Without Line 156 & 157, running the capture script for CW310 fails:

Traceback (most recent call last):

  File "/repo/cw/./capture.py", line 2267, in <module>
    app()

  File "/repo/cw/./capture.py", line 571, in aes_fvsr_key_batch
    capture_init(ctx, force_program_bitstream, num_traces, plot_traces)

  File "/repo/cw/./capture.py", line 183, in capture_init
    ctx.obj.ot = initialize_capture(cfg["device"], cfg["capture"])

  File "/repo/cw/./capture.py", line 100, in initialize_capture
    ot = device.OpenTitan(device_cfg["fpga_bitstream"],

  File "/repo/cw/util/device.py", line 68, in __init__
    self.scope = self.initialize_scope(scope_gain, num_samples, offset,

  File "/repo/cw/util/device.py", line 168, in initialize_scope
    raise RuntimeError(

RuntimeError: ADC failed to lock (attempts: 3).

I had a closer look into this issue and discovered that, after
scope.clock.clkgen_freq = pll_frequency
(Line 130) the PLL is not locked anymore and, subsequently, the ADC then also is not able to lock.
I don't know why again setting the clkgen_src fixes the issue but I assume that this is a bug in the CW framework. Alex from NewAE mentioned in the CW forum a similar issue.

Copy link
Collaborator

Choose a reason for hiding this comment

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

thx, so L130 is in fact executed I understand.
tanks again - maybe will try split the two CW310 and Husky into two separate classes instead of OpenTitan in device.py later with this fix

# TODO: Need to update error handling.
if not husky:
scope.clock.reset_adc()
Expand Down
2 changes: 1 addition & 1 deletion python-requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ joblib
# Development version of ChipWhisperer toolchain with latest features and
# bug fixes - Needs to be installed in editable mode. We fix the version
# for improved stability and manually update if necessary.
-e git+https://github.com/newaetech/chipwhisperer.git@c5181a87111d3aabab42d83b10dbc368140b43ef#egg=chipwhisperer
-e git+https://github.com/newaetech/chipwhisperer.git@3eace1719daf43d4f0965c1790c2c8a9e8b2f690#egg=chipwhisperer

# Linters
-r python-requirements-lint.txt
Expand Down