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

[adc_ctrl] Updates to LP -> NP transition #21829

Merged
merged 4 commits into from
Mar 6, 2024

Conversation

msfschaffner
Copy link
Contributor

@msfschaffner msfschaffner commented Mar 4, 2024

This contains the following patches:

  1. the adc_ctrl FSM has one more arc now that allows it to go back to LP mode iff LP mode was enabled in the first place, and a match could not be confirmed after transitioning from LP -> NP mode. this addresses [adc_ctrl] Enhance adc_ctrl low power scan to automatically return to low power #13725 and [adc_ctrl] Issues with the current design #18511.
  2. an FSM state observation register is added. his is intended for debug purposes only.

3) in preparation for 4), the redundant filter_status register is removed as described in #11354 -> this is reverted as it is not strictly required, and would change the current wakeup detection logic to get intertwined with the interrupt logic. this in turn would require a more significant DV update for which we currently do not have the resources for.

  1. another internal interrupt and wakeup source is added that allows to wake up or interrupt the system if an LP -> NP transition has occurred. this is intended for debug purposes only.

@msfschaffner msfschaffner self-assigned this Mar 4, 2024
@msfschaffner msfschaffner requested a review from a-will March 4, 2024 21:36
@msfschaffner msfschaffner force-pushed the adc-ctrl-fix branch 3 times, most recently from e15b213 to 85ba216 Compare March 4, 2024 21:45
@msfschaffner
Copy link
Contributor Author

This is not fully clean and needs DIF/DV alignments.

I just wanted to put this draft up for review to see whether these CSR changes are along the lines of what we think we need.

@jesultra
Copy link
Contributor

jesultra commented Mar 4, 2024

From the description, it sounds exactly as what we need.

@@ -312,24 +312,6 @@
]
}

{ name: "filter_status",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@msfschaffner msfschaffner force-pushed the adc-ctrl-fix branch 5 times, most recently from 9e8e652 to 1fb7fe8 Compare March 5, 2024 04:21
@msfschaffner
Copy link
Contributor Author

The DIF has been aligned, but the DV environment still needs patches and fails currently.

hw/ip/adc_ctrl/data/adc_ctrl.hjson Outdated Show resolved Hide resolved
hw/ip/adc_ctrl/rtl/adc_ctrl_intr.sv Outdated Show resolved Hide resolved
Signed-off-by: Michael Schaffner <[email protected]>
@msfschaffner msfschaffner force-pushed the adc-ctrl-fix branch 5 times, most recently from b9a1454 to 7ed6ad1 Compare March 5, 2024 21:17
Copy link
Contributor

@timothytrippel timothytrippel left a comment

Choose a reason for hiding this comment

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

DIF changes LGTM.

sw/device/lib/dif/dif_adc_ctrl.h Outdated Show resolved Hide resolved
@msfschaffner
Copy link
Contributor Author

msfschaffner commented Mar 6, 2024

Ok the latest force push updates the block-level DV and models the new FSM behavior and the new interrupt/wakeup source, so that it is implicitly tested by randomized sequences like adc_ctrl_filters_wakeup. The FSM state output register for debug is just tested with an SVA right now.

@msfschaffner msfschaffner marked this pull request as ready for review March 6, 2024 03:39
@msfschaffner msfschaffner requested review from a team as code owners March 6, 2024 03:39
@msfschaffner msfschaffner force-pushed the adc-ctrl-fix branch 2 times, most recently from 136c051 to 8ba5e97 Compare March 6, 2024 03:43
The ADC_CTRL FSM has a low power sensing mode where ADC samples are
taken at a low sampling rate. If a match is seen, it transitions to
normal sampling mode in order to confirm the match.

Unfortunately, if it does not confirm the match and the chip is in a
low-power (sleep) state, the ADC_CTRL FSM stays in normal sampling mode
and does not wake up the chip. This is not ideal from a power
perspective, since in normal sampling mode, the average power
consumption is much higher due to regular ADC sampling.

This patch fixes that and introduces a condition in the normal sampling
mode that automatically sends the FSM back to low power sampling if a
match cannot be confirmed in the normal sampling mode (in case the
ADC_CTRL is configured do use low power sampling).

Fixes lowRISC#13725

Signed-off-by: Michael Schaffner <[email protected]>
Note that this register is automatically synced from AON to bus clock
domain as part of the auto-generated reg node.

Signed-off-by: Michael Schaffner <[email protected]>
Copy link

@mvertescher mvertescher left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the ADC debug changes!

@msfschaffner msfschaffner merged commit 4c06f37 into lowRISC:master Mar 6, 2024
32 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
7 participants