Skip to content

Work around various silicon errata #340

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

Merged
merged 10 commits into from
Mar 17, 2024

Conversation

theAlexes
Copy link
Collaborator

@theAlexes theAlexes commented Dec 17, 2023

Per the Microchip docs, one erratum may freeze the microcontroller (the SUPC/VREG operating in standby), another may cause increased power consumption from the TRNG not being properly disabled, and a third can hard-fault the chip if a standby occurs at the same time as a SysTick. This patch applies the recommended workarounds.

Also, we noticed that the sleep controller documentation specifies that you have to wait for the value to settle in the sleep mode register before actually WFI-ing, so we've modified the HAL to do that.

@theAlexes theAlexes force-pushed the theAlexes/silicon-erratum branch from 78cd763 to d96d6f9 Compare December 18, 2023 03:09
Copy link
Collaborator

@WesleyAC WesleyAC left a comment

Choose a reason for hiding this comment

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

Thanks for catching these!

Could you add references to specific section/page/etc numbers in the manual? That would be useful just to double check that this is all good.

I'm also curious if you have an idea of whether the RUNSTDBY / STDBYPL0 will affect power usage, although if the docs suggest it we should presumably do it either way, especially given recent reports of freezing issues.

@theAlexes
Copy link
Collaborator Author

Thanks for catching these!

Could you add references to specific section/page/etc numbers in the manual? That would be useful just to double check that this is all good.

updated accordingly, and included a link to the errata in the GH description.

I'm also curious if you have an idea of whether the RUNSTDBY / STDBYPL0 will affect power usage, although if the docs suggest it we should presumably do it either way, especially given recent reports of freezing issues.

power consumption is something we're pretty fuzzy on, unfortunately.

@theAlexes theAlexes requested a review from WesleyAC January 22, 2024 00:46
Copy link
Collaborator

@matheusmoreira matheusmoreira left a comment

Choose a reason for hiding this comment

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

Looks great to me. Well referenced and explained by the comments, ensuring any future readers know why things work this way.

@matheusmoreira
Copy link
Collaborator

I'm also curious if you have an idea of whether the RUNSTDBY / STDBYPL0 will affect power usage, although if the docs suggest it we should presumably do it either way, especially given recent reports of freezing issues.

power consumption is something we're pretty fuzzy on, unfortunately.

That seems to be case for us all. There are some power consumption figures in the sensor watch documentation but they were measured with a Power Profiler Kit II. Absent access to such a profiler, I don't believe any objective statements regarding the power cost of any proposed change can be made.

In any case, if the silicon errata tells us to do something, we should probably do it regardless of any impact on power consumption, right?

matheusmoreira added a commit to matheusmoreira/sensor-watch that referenced this pull request Mar 5, 2024
Implements the recommended workarounds for numerous silicon errata,
reducing power consumption and preventing freezes and hard faults.

Tested-by: Alex Maestas <git@se30.xyz>
Reviewed-by: Wesley Aptekar-Cassels <me@wesleyac.com>
Reviewed-by: Matheus Afonso Martins Moreira <matheus.a.m.moreira@gmail.com>
Signed-off-by: Matheus Afonso Martins Moreira <matheus.a.m.moreira@gmail.com>
GitHub-Pull-Request: joeycastillo#340
GitHub-Related-Issue: joeycastillo#361
GitHub-Related-Issue: joeycastillo#359
Reference: https://ww1.microchip.com/downloads/aemDocuments/documents/MCU32/ProductDocuments/Errata/SAM-L22-Family-Silicon-Errata-and-Data-Sheet-Clarification-DS80000782.pdf
matheusmoreira added a commit to matheusmoreira/sensor-watch that referenced this pull request Mar 5, 2024
Implements the recommended workarounds for numerous silicon errata,
reducing power consumption and preventing freezes and hard faults.

Tested-by: Alex Maestas <git@se30.xyz>
Reviewed-by: Wesley Aptekar-Cassels <me@wesleyac.com>
Reviewed-by: Matheus Afonso Martins Moreira <matheus.a.m.moreira@gmail.com>
Signed-off-by: Matheus Afonso Martins Moreira <matheus.a.m.moreira@gmail.com>
GitHub-Pull-Request: joeycastillo#340
GitHub-Related-Issue: joeycastillo#361
GitHub-Related-Issue: joeycastillo#359
Reference: https://ww1.microchip.com/downloads/aemDocuments/documents/MCU32/ProductDocuments/Errata/SAM-L22-Family-Silicon-Errata-and-Data-Sheet-Clarification-DS80000782.pdf
matheusmoreira added a commit to matheusmoreira/sensor-watch that referenced this pull request Mar 5, 2024
Implements the recommended workarounds for numerous silicon errata,
reducing power consumption and preventing freezes and hard faults.

Tested-by: Alex Maestas <git@se30.xyz>
Reviewed-by: Wesley Aptekar-Cassels <me@wesleyac.com>
Reviewed-by: Matheus Afonso Martins Moreira <matheus.a.m.moreira@gmail.com>
Signed-off-by: Matheus Afonso Martins Moreira <matheus.a.m.moreira@gmail.com>
GitHub-Pull-Request: joeycastillo#340
GitHub-Related-Issue: joeycastillo#361
GitHub-Related-Issue: joeycastillo#359
Reference: https://ww1.microchip.com/downloads/aemDocuments/documents/MCU32/ProductDocuments/Errata/SAM-L22-Family-Silicon-Errata-and-Data-Sheet-Clarification-DS80000782.pdf
matheusmoreira added a commit to matheusmoreira/sensor-watch that referenced this pull request Mar 5, 2024
Implements the recommended workarounds for numerous silicon errata,
reducing power consumption and preventing freezes and hard faults.

Tested-by: Alex Maestas <git@se30.xyz>
Reviewed-by: Wesley Aptekar-Cassels <me@wesleyac.com>
Reviewed-by: Matheus Afonso Martins Moreira <matheus.a.m.moreira@gmail.com>
Signed-off-by: Matheus Afonso Martins Moreira <matheus.a.m.moreira@gmail.com>
GitHub-Pull-Request: joeycastillo#340
GitHub-Related-Issue: joeycastillo#361
GitHub-Related-Issue: joeycastillo#359
Reference: https://ww1.microchip.com/downloads/aemDocuments/documents/MCU32/ProductDocuments/Errata/SAM-L22-Family-Silicon-Errata-and-Data-Sheet-Clarification-DS80000782.pdf
@matheusmoreira
Copy link
Collaborator

Am now running this code on the real hardware. No problems so far.

@matheusmoreira
Copy link
Collaborator

My Sensor Watch Lite board is apparently unable to enter sleep mode when running this code. Have you or anyone else experienced this sleep mode issue?

  • Reproduced the issue with multiple low energy timer preferences set
    • 10 minutes
    • 1 hour
  • Confirmed that the clock face is returning true from its loop
    • Via simulator debugging
    • Indicates that it's not blocking the sleep mode
  • This is the only patch set in my branch which touches sleep mode

The most recent change in main to sleep mode functionality:

// if we run through the loop again to time out, we need to reconsider whether or not we can sleep.
// if the first trip said true, but this trip said false, we need the false to override, thus
// we will be using boolean AND:
//
// first trip | can sleep | cannot sleep | can sleep | cannot sleep
// second trip | can sleep | cannot sleep | cannot sleep | can sleep
// && | can sleep | cannot sleep | cannot sleep | cannot sleep
can_sleep = can_sleep && wf->loop(event, &movement_state.settings, watch_face_contexts[movement_state.current_face_idx]);

I suspect it could be somewhat related due to short circuiting behavior which is potentially unintended and might be a bug. This is what actually happens:

// first trip  | can sleep | cannot sleep | can sleep    | cannot sleep
// second trip | can sleep | never runs   | cannot sleep | never runs
//          && | can sleep | cannot sleep | cannot sleep | cannot sleep

I need further testing to determine what impact this has, if any. However it appears you authored this change so I was hoping you'd clarify.

@matheusmoreira
Copy link
Collaborator

matheusmoreira commented Mar 6, 2024

This is the only patch set in my branch which touches sleep mode

I stand corrected. It seems to be working perfectly. I eventually isolated the issue to the deadline watch face, PR #266.

So far this has been tested on real hardware by both @theAlexes and myself. Seems stable.

@theAlexes theAlexes merged commit 592e18b into joeycastillo:main Mar 17, 2024
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

Successfully merging this pull request may close these issues.

3 participants