Enhance hard reset signal implementation

[AleksandrDanilovIntel]

No description provided.

[mandolaerik]

This is redundant in a sense; you can override signal_lower and call default instead.

(One could argue that it would be better with a simulator-agnostic method to override, but that sort of functionality would rather belong to the signal_port template.)

[AleksandrDanilovIntel]

Would be better to hide the implementation and expose only the high level methods as hard_reset()
is hreset;
port HRESET {
saved bool reset_asserted;
implement signal {
method signal_lower() {
hard_reset_deassert();
default();
}

[svenwest]

I'm with Erik on this. What's the added value of overriding hard_reset_deassert instead of overriding signal_lower?

If we add a hard_reset_deassert method like this we'd need to consider some more questions:
For example

• Should hard_reset_deassert be called after a dev.hard_reset()?
• Should reset_asserted be true during a call to dev.hard_reset()?
• Why is there no hard_reset_assert() method?

If we instead let the user override signal_lower in the HRESET port it's very clear what behavior is being overridden, and what can be expected.

[AleksandrDanilovIntel]

agreed

[mandolaerik]

It would make more sense to start with template hreset is (hard_reset, signal_port) and continue from there. IIRC, @svenwest tried this when adding signal_port, but it was dropped since it caused too much short-term SSM breakage. I think it can be done, but requires a careful migration plan.

[AleksandrDanilovIntel]

Both templates are define default methods. Using spec violation is also doubtful.

[svenwest]

I think what Erik means is this:

// Instantiate this on top level to support hard reset.
template hreset is hard_reset {
    port HRESET is signal_port {
        implement signal {
            method signal_raise() default {
                default();
                dev.hard_reset();
            }
            method signal_lower() default {
                default();
            }
        }
    }

    in each init_val {
        is _init_val_hard_reset;
    }
}

Using spec_viol is not doubtful in my opinion. The API docs for signal_interface_t specifically states:

Once raised, the same initiator may not call signal_raise() again without an intervening call to signal_lower(). Similarly, an already low signal may not be lowered again by a signal_lower() call from the same initiator.

I tried adding this template in a PR some time ago. Perhaps we could try again with careful steps not to break SSM builds.

[AleksandrDanilovIntel]

Is it possible to call default() from the default method?
method signal_lower() default {
default();
}

[AleksandrDanilovIntel]

I would prefer to copy a logic from signal port template

template hreset is hard_reset {
    port HRESET {
        saved bool high;
        implement signal {
            method signal_raise() default {
                high = true;
                dev.hard_reset();
            }
            method signal_lower() default {
                high = false;
            }