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

hwp-pmx: reset protection code in clear_alarm #641

Merged
merged 1 commit into from
Mar 13, 2024
Merged

hwp-pmx: reset protection code in clear_alarm #641

merged 1 commit into from
Mar 13, 2024

Conversation

ykyohei
Copy link
Contributor

@ykyohei ykyohei commented Mar 12, 2024

Description

Minor debug of hwp-pmx.
We need to reset the protection code (self.prot) in clear_alarm task.
This was implemented in the previous version of agent (following link), but I forgot to include this self.prot reset when we refactored the hwp-pmx agent to be TimoutLock less.

self.prot = 0

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

@ykyohei ykyohei requested review from jlashner and d1ssk March 12, 2024 23:45
Copy link
Contributor

@d1ssk d1ssk left a comment

Choose a reason for hiding this comment

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

Looks good.

@jlashner
Copy link
Collaborator

Seems like this could also be done by allowing prot to be set back to 0 in the acq function:

if prot_code != 0:
self.prot = prot_code

Is there a reason we don't do that? It's generally preferable to read from hardware when possible instead of setting value directly in the agent.

@ykyohei
Copy link
Contributor Author

ykyohei commented Mar 13, 2024

Agree that it's generally preferable to read from hardware. But this is intentional.

When the pmx protection is triggered, pmx outputs protection code only once, you will get 0 from the second queries. However, pmx remains in protected state. If you run clear_alarm, pmx will go back to the normal state. If there is still problem, pmx will output protection code again, but only once.

For this reason, we made the agent to have self.prot for monitoring purposes.

Copy link
Collaborator

@jlashner jlashner left a comment

Choose a reason for hiding this comment

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

Makes sense, thanks!

@jlashner jlashner merged commit e2aa4ee into main Mar 13, 2024
5 checks passed
@jlashner jlashner deleted the hwp-pmx branch March 13, 2024 14:21
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