8000 Lower power by using the Wait for Interrupt (WFI) instruction by tannewt · Pull Request #2685 · adafruit/circuitpython · GitHub
[go: up one dir, main page]

Skip to content

Lower power by using the Wait for Interrupt (WFI) instruction #2685

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 62 commits into from
Apr 30, 2020

Conversation

tannewt
Copy link
Member
@tannewt tannewt commented Mar 7, 2020

No description provided.

@tannewt
Copy link
Member Author
tannewt commented Mar 7, 2020

@jepler Please take a look at this. I've only redone the nRF port so far and would like high level feedback before I do the others. The only remaining work for nRF is to get PulseIn working again which uses the monotonic clock for pulse timing.

@tannewt tannewt requested a review from jepler March 7, 2020 02:50
jepler
jepler previously approved these changes Mar 9, 2020
Copy link
@jepler jepler left a comment

Choose a reason for hiding this comment

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

I think this is going in a good direction. I didn't do a detailed review, but @tannewt let me know in the chat that he's just looking for high level review at this point.

@hierophect
Copy link
Collaborator
hierophect commented Mar 9, 2020

I can take a closer look at this tomorrow morning, but it seems like good progress so far.

@tannewt
Copy link
Member Author
tannewt commented Mar 9, 2020

Thanks @jepler and @hierophect for the high level review. I've gotta propagate the changes to other ports and just wanted to make sure I was heading the right direction.

hierophect
hierophect previously approved these changes Mar 10, 2020
@tannewt tannewt dismissed stale reviews from hierophect and jepler via 15e9e7d March 10, 2020 22:05
@tannewt
Copy link
Member Author
tannewt commented Apr 23, 2020

Thanks @dhalbert. sigh I may wait until next week to tackle M0 flakiness. I'm itching to work on ESP32-S2. Will see how I feel tomorrow.

@dhalbert
Copy link
Collaborator

@tannewt Yes, this can wait :)

@tannewt
Copy link
Member Author
tannewt commented Apr 28, 2020

@dhalbert I can't reproduce this flakiness with a Metro M0. Most of the time it works. Occasionally, I do see the boot hang very early but I think I can get the same behavior on master. I was testing it with an Arch Linux system and a Beagle sniffing the USB traffic.

I've merged in latest master which is what I tested with. Please try again and let me know if it's consistently bad for you still. I may need your help debugging it.

@dhalbert
Copy link
Collaborator
dhalbert commented Apr 28, 2020

@tannewt It's still failing on Metro M0, somewhat more than half the time. I just press reset and see what happens. Attached is a Beagle trace from a failure.

metro-m0-fail.tdc.zip

I will not work on it more this evening.

@xobs
Copy link
xobs commented Apr 28, 2020

I'm seeing a lot of assert errors in tinyusb with this patch when running on an nRF52833. For example:

(gdb) c
Continuing.

Program received signal SIGTRAP, Trace/breakpoint trap.
process_set_config (cfg_num=1 '\001', rhport=0 '\000') at ../../lib/tinyusb/src/device/usbd.c:731
731         TU_ASSERT( drv_id < USBD_CLASS_DRIVER_COUNT && drv_len >= sizeof(tusb_desc_interface_t) );
(gdb) c
Continuing.

Program received signal SIGTRAP, Trace/breakpoint trap.
process_control_request (p_request=0x2001ff20, rhport=0 '\000') at ../../lib/tinyusb/src/device/usbd.c:660
660         default: TU_BREAKPOINT(); return false;
(gdb)

tannewt added 2 commits April 27, 2020 20:28
Also, rename it so that matches the other board names.
@xobs
Copy link
xobs commented Apr 28, 2020
(gdb) c
Continuing.

Program received signal SIGTRAP, Trace/breakpoint trap.
process_set_config (cfg_num=1 '\001', rhport=0 '\000') at ../../lib/tinyusb/src/device/usbd.c:731
731         TU_ASSERT( drv_id < USBD_CLASS_DRIVER_COUNT && drv_len >= sizeof(tusb_desc_interface_t) );
(gdb)

Also, the ASSERT that gets hit is probably due to the tinyusb process not being called often enough, causing the host to send a USB RESET, which hits this bug: hathach/tinyusb#179

@tannewt
Copy link
Member Author
tannewt commented Apr 28, 2020

@xobs Would you mind PRing the '833 board defs? I have a dev kit I can try it on here.

@xobs
Copy link
xobs commented Apr 29, 2020

PR generated at #2828

Note that it requires a bootloader that's smaller (such as https://github.com/simmel-project/bootloader) in order to give more storage on the small internal flash.

@tannewt
Copy link
Member Author
tannewt commented Apr 30, 2020

@dhalbert This is ready for another look and test. Thanks!

@tannewt
Copy link
Member Author
tannewt commented Apr 30, 2020

Ok, @dhalbert. This commit should be a winner.

Copy link
Collaborator
@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

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

USB fix works great for me!

@tannewt
Copy link
Member Author
tannewt commented Apr 30, 2020

@jepler Ok for me to merge?

Copy link
@jepler jepler left a comment

Choose a reason for hiding this comment

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

Looks like my earlier requested changes were resolved. No new testing or review performed.

@tannewt tannewt merged commit b3b6a64 into adafruit:master Apr 30, 2020
@ladyada
Copy link
Member
ladyada commented May 1, 2020

congrats on the big lift!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants
0