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 NumRegField byte(s) read #392

Merged
merged 2 commits into from
Aug 14, 2023
Merged

Fix NumRegField byte(s) read #392

merged 2 commits into from
Aug 14, 2023

Conversation

prgeor
Copy link
Collaborator

@prgeor prgeor commented Aug 9, 2023

Description

NumRegField read worked fine when the fields were single bit. Now that we have RegBitsFields with multiple bits we need to adjust the mask accordingly.

Motivation and Context

NumRegFields having RegBitsFields did not read the byte correctly. This is now fixed with this change.

How Has This Been Tested?

Unit test passed

Additional Information (Optional)

@prgeor prgeor requested a review from mihirpat1 August 9, 2023 05:43
@prgeor
Copy link
Collaborator Author

prgeor commented Aug 9, 2023

@andywongarista can you review this fix

@@ -86,6 +87,7 @@ def __init__(self, name, bitpos, offset=None, **kwargs):
super(RegBitField, self).__init__(name, offset, **kwargs)
assert bitpos < 64
self.bitpos = bitpos
self.bitmask = 1 << self.bitpos
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we using bitmask in RegBitField ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@andywongarista not directly, but when RegBitField is grouped inside a NumberRegField or CodeRegField

Copy link
Contributor

Choose a reason for hiding this comment

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

@prgeor ok. Thanks

@yxieca yxieca merged commit 636a3a9 into sonic-net:master Aug 14, 2023
4 checks passed
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