-
Notifications
You must be signed in to change notification settings - Fork 49
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
Fields not having a reset mask are still parsed with a mask of all zeros #41
Comments
I see the problem, and we should fix it somehow, but I would like to make sure that the solution works with the new API that was introduced in ipyxact 0.3.0, which doesn't depend on the awkward inline yaml solution that I came up with first. I tried porting your example to the new API, but I'm not sure I understand what the result is supposed to be and I also don't remember how this parse_uint thing is supposed to work so I just copied the function here. Not sure if it's of any help, but here it is
|
Sorry for the delay in response and Happy New Year. I wasn't aware of the API change but I see this now. For what is worth I didn't personally think the original YAML solution was awkward (actually I found it rather elegant), but I understand the benefit of generating the API directly from the XML schema. Tried it again using the new API as suggested above: import ipyxact.ipxact2014 as ipxact
component = ipxact.parse("ipxact.xml", silence=True)
register = component.memoryMaps.memoryMap[0].addressBlock[0].register[0]
fields = register.field
print(f'Printing fields from register {register.name}:')
for f in fields:
_reset = ipxact.unsignedPositiveIntExpression.parse_uint(f.resets.reset[0].value)
_mask = None
_mask_string = str(_mask)
_overall_reset = _reset
if f.resets.reset[0].mask is not None:
_mask = ipxact.unsignedPositiveIntExpression.parse_uint(f.resets.reset[0].mask)
_mask_string = f"{_mask:X}"
_overall_reset &= _mask
print(f" Field {f.name} has reset value of x{_reset:X} and mask of x{_mask_string} --> overall reset value: x{_overall_reset:X}") This yields:
Which looks correct. The user can check the mask against The problem still remains using the old API. It's up to you whether that is worth fixing (as I was saying above a workaround might involve few changes to |
Good to hear it's working with the new API! And for context, I thought my original idea was pretty neat too, but in addition to only covering a small subset of the standard, it also gave rise to some subtle bugs that turned out to be hard to fix. I would happily apply a contributed fix for the old API, as you're probably right that it's still widely used. I don't have the time to work on it myself though. |
The IP-XACT standard allows for an optional mask element inside a field reset element. This is used to define which bits of the field have a known reset value. The absence of the mask element makes this N/A, i.e. it is equivalent to a mask of the same size as the register field and consisting of all 1's.
When the latter situation occurs, the
ipyxact
parser (v0.3.2
) still evaluates the mask as all 0's. This violates the standard and makes impossible to discriminate between a mask of all 0's and an absent mask (i.e. N/A or all 1's).For instance, consider the
ipxact.xml
sample below:When parsed:
It yields:
As you can see, fields
without_mask
andwith_zero_mask
are both evaluated with a mask of all 0's and there's no way to tell the former doesn't have a mask defined in the first place. I believe the above considerations also applies to register resets as per IP-XACT 2009, even though I haven't tested it.I suspect this behavior is due to:
IpxactInt
type being used for mask insideipyxact_yaml.py
.ipyxact.py
setting an attribute value ofeval(_type)()
if the element is not found when parsing the IP-XACT.The preferred behavior for me would be not to create a mask attribute inside the reset class if there's no mask in the IP-XACT, but it may involve a re-structuring on how the parser works.
A sub-optimal solution could be creating another integer type that defaults to all 1's and use it for the mask. However, note that in this case there's no easy way to dynamically adjust the number of bits that should be 1 w.r.t. the field/register size, so a worst-case guess is needed (a bit of an hack).
The text was updated successfully, but these errors were encountered: