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

fix implementation of NOR polling APIs #77

Merged
merged 3 commits into from
Jan 29, 2019
Merged

fix implementation of NOR polling APIs #77

merged 3 commits into from
Jan 29, 2019

Conversation

drossetti
Copy link
Contributor

This was observed during a code review and needs to be thoroughly tested.
Note that NOR support is currently disabled by default and is only supported on Volta+.

the loop exit condition was not correctly waiting for all the bits in the word to be unset.
fixes #76
@drossetti drossetti requested a review from e-ago January 14, 2019 18:33
@drossetti
Copy link
Contributor Author

@e-ago ping for this PR

@e-ago
Copy link
Collaborator

e-ago commented Jan 28, 2019

Considering:

With this PR the test hangs and returns several errors:

...
ERR:  [main] [63274] unexpected tx ev 0, batch len 20
ERR:  [main] [63274] unexpected rx ev 1, batch len 20
ERR:  [main] [63375] unexpected tx ev 0, batch len 20
ERR:  [main] [63375] unexpected rx ev 1, batch len 20
ERR:  [main] [63453] unexpected tx ev 0, batch len 20
ERR:  [main] [63453] unexpected rx ev 1, batch len 20
ERR:  [main] [63543] unexpected tx ev 0, batch len 20
ERR:  [main] [63543] unexpected rx ev 1, batch len 20
ERR:  [main] [48] unexpected rx ev 0, batch len 20
ERR:  [main] [49] unexpected rx ev 0, batch len 20
ERR:  [main] [50] unexpected rx ev 0, batch len 20
...

Without this PR, everything works correctly.
You should change done = (0 == ~(data | value)); into done = (0 != ~(data | value));

… change but make the original bitwise condition a bit more explicit.
@drossetti
Copy link
Contributor Author

@e-ago you are right, I misinterpreted the HW spec once again. So here the patch simply makes the condition more explicit. Sorry for the noise.

@e-ago e-ago merged commit 3d0333d into master Jan 29, 2019
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.

2 participants