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

Field defined as external #4

Open
benoitdenkinger opened this issue May 21, 2024 · 1 comment
Open

Field defined as external #4

benoitdenkinger opened this issue May 21, 2024 · 1 comment

Comments

@benoitdenkinger
Copy link

Hello,

Regarding the 'external' type, I don't find the specification 2.0 to be super clear for fields (and signals).

For example, it is stated in 9.6.1:

A field with an onread value of ruser shall be external

But in 9.3 the syntax does not shows the possibility for the 'external' type (while the memory and register sections clearly show it). The systemrdl compiler does not allows it either (core/ComponentVisitor.py)

internal/external instance type is not valid for signal or field components

But inside the class LateElabListener (core/elaborate.py), the fields inherit their parent external property:

    def enter_Field(self, node: FieldNode) -> None:
        # Inherits internal/external of parent reg
        assert node.parent is not None
        node.inst.external = node.parent.inst.external

On the plugin side, it uses the node external property which is true if the parent register is external but then generates an error from the compiler side as this is not accepted (above quote from core/ComponentVisitor.py).

def enter_Field(self, node: 'FieldNode') -> None:
        suffix = f"[{node.msb}:{node.lsb}]"
        reset = node.get_property('reset')
        if isinstance(reset, int):
            suffix += f" = 0x{reset:X}"

        self.push("field", node.inst_name, suffix=suffix, is_external=node.external)

So the specification clearly state that a field can be external, but it does not specify how this is translated in SystemRDL.

@benoitdenkinger
Copy link
Author

Is there any interest to fix this inconsistency between this plugin and the systemrdl-compiler? I did a quick fix on my fork and it is working for me, but I didn't do extensive testing (e.g., running the repo tests). I'll be happy to propose a PR if this is of interest.

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

No branches or pull requests

1 participant