[rtl, aon_timer] Reset core.prescale_count on wkup_ctrl write

[antmarzam]

The AON timer core has an internal counter prescale_count_q which is not reset upon each write to wkup_ctrl.
This causes issues if the WKUP timer is re-configured to a lower prescaler value than it was before, and the prescale_count_q variable is greater than the new prescaler value, forcing the prescale_count_q counter to overflow itself causing a non-spec compliant behavior.

In order to mitigate the above, the internal prescale_count_q is reset after wkup_ctrl write. Thus, the aon timer is forced to count from scratch the new intended prescaler value.

Docs PR is #25904

[marnovandermaas]

I think the RTL now looks right to me. Would you mind adding some documentation on the new behavior. Specifically that any write to the wkup_ctrl register now resets the prescalar including the enable bit.

[marnovandermaas]

Can you try this instead to fix the lint errors:

    { name: "WKUP_CTRL",
      desc: "Wakeup Timer Control register",
      swaccess: "rw",
      hwaccess: "hro",
      async: "clk_aon_i",
      fields: [
        { bits: "0",
          name: "enable",
          desc: "When set to 1, the wakeup timer will count",
        }
        { bits: "12:1",
          name: "prescaler",
          desc: "Pre-scaler value for wakeup timer count",
          hwqe: "true",
        }
      ],
    },

[antmarzam]

I think the RTL now looks right to me. Would you mind adding some documentation on the new behavior. Specifically that any write to the wkup_ctrl register now resets the prescalar including the enable bit.

Yep - I'll create a PR adding this new behavior to the doc. Will add you as a reviewer and link here.

Also, just pushed the changes adding hwqe = true to the field only. The behaviour is the same just that hw2reg.wkup_ctrl.enable.qe is now not connected.

[Razer6]

HW looks good to me.

[antmarzam]

@marnovandermaas I've moved the doc commit here as you suggested

[marnovandermaas]

This looks correct to me, thanks for pursuing this fix.

[marnovandermaas]

CHANGE AUTHORIZED: hw/ip/aon_timer/data/aon_timer.hjson
CHANGE AUTHORIZED: hw/ip/aon_timer/rtl/aon_timer_core.sv
CHANGE AUTHORIZED: hw/ip/aon_timer/rtl/aon_timer_reg_pkg.sv
CHANGE AUTHORIZED: hw/ip/aon_timer/rtl/aon_timer_reg_top.sv

[rswarbrick]

This looks sensible to me. Thanks for sorting it out. (I wrote two versions of "isn't this wrong because...", and then realised that the change is correct!)

[rswarbrick]

CHANGE AUTHORIZED: hw/ip/aon_timer/data/aon_timer.hjson
CHANGE AUTHORIZED: hw/ip/aon_timer/rtl/aon_timer_core.sv
CHANGE AUTHORIZED: hw/ip/aon_timer/rtl/aon_timer_reg_pkg.sv
CHANGE AUTHORIZED: hw/ip/aon_timer/rtl/aon_timer_reg_top.sv

The only change to behaviour comes from the change to aon_timer_core.sv in the first commit. This looks sensible and is tested (indeed, that's why the PR was needed!), so the risk to the taped out design is very low.