Remove the #10088 hotfix for Teensy 3.1-like Input:Club keyboards #12870

firetech · 2021-05-11T18:32:51Z

Description

On at least my Ergodox Infinity, the #10088 hotfix causes the issue it intended to solve. That is, waking the keyboard from suspend after the keyboard has caused the host to wake works fine without the #10088 hotfix, but with it, the keyboard gets stuck sleeping.

This PR removes the #10088 hotfix for Ergodox Infinity, K-Type and Whitefox (all using the same MK20DX256 MCU).

Types of Changes

Checklist

My code follows the code style of this project: C, Python
I have read the PR Checklist document and have made the appropriate changes.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
I have tested the changes and verified that they work and don't break anything (as well as I can manage).

It seems to _cause_ the issue it intended to solve there.

tzarc · 2021-05-11T20:58:22Z

I can't approve this PR in its current form. ChibiOS is supposed to abstract this stuff away, and only in cases where ChibiOS doesn't support pieces of functionality (such as bootloaders etc.) do we delve into processor-specific handling of issues.

I'd much rather see a proper fix.

firetech · 2021-05-11T21:10:01Z

I'd much rather see a proper fix.

Fair enough.

Since I don't own any keyboard that experiences the original issue, I can't really help with a proper fix. Should I change this PR to just revert the original "fix" altogether? Or add a specific define to skip the USB driver restart?

drashna · 2021-05-11T22:55:55Z

Just curious if you've tried #define USB_SUSPEND_WAKEUP_DELAY 200, as that would effect this.

firetech · 2021-05-11T22:57:30Z

Just curious if you've tried #define USB_SUSPEND_WAKEUP_DELAY 200, as that would effect this.

Yep. It had no effect.

stale · 2021-06-26T00:57:44Z

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

firetech · 2021-06-26T21:57:01Z

I think something should be done to fix this issue. Maybe not exactly this patch in its current state, but I'd be happy to revise it to something more acceptable.

drashna · 2021-07-03T07:28:38Z

I think something should be done to fix this issue. Maybe not exactly this patch in its current state, but I'd be happy to revise it to something more acceptable.

How about making the function weak?

That way, could throw a revised version in the boards.c in /platforms/chibios/ or wherever.

Now only affects Ergodox Infinity, Whitefox and K-type, though. Switches over Ergodox Infinity to the `IC_TEENSY_3_1` board, since that was a nice place to implement the `restart_usb_driver` override. However, I would guess this issue is present for other K20x/Teensy 3.1 boards as well...

firetech · 2021-07-03T10:42:00Z

I implemented @drashna's suggestion instead. As a side effect, the fix now only affects Input:Club boards (specifically K-Type, Whitefox and Ergodox Infinity).

Since I found there was an existing IC_TEENSY_3_1 (where IC is for Input:Club) board in platforms/chibios, only used by K-Type and Whitefox, I switched over Ergodox Infinity to that and implemented the empty restart_usb_driver override there. No idea why Ergodox Infinity wasn't using that board, but it seems to work fine after switching to it. A comment in K-Type's rules.mk suggests that the only difference between PJRC_TEENSY_3_1 and IC_TEENSY_3_1 is a hack related to the watchdog, and given these boards common pedigree, that might be of use on the Ergodox Infinity as well?

for history see: SonixQMK/qmk_firmware#28 qmk/qmk_firmware#12870

for history see: #28 qmk#12870 SonixQMK/ChibiOS-Contrib#24

for history see: #28 qmk#12870 SonixQMK/ChibiOS-Contrib#24 Co-authored-by: stdvar <[email protected]>

…K#123) for history see: SonixQMK#28 qmk#12870 SonixQMK/ChibiOS-Contrib#24 Co-authored-by: stdvar <[email protected]>

for history see: SonixQMK/qmk_firmware#28 qmk/qmk_firmware#12870

…mk#12870) * Remove the qmk#10088 hotfix for K20x MCU:s. It seems to _cause_ the issue it intended to solve there. * Cleaner way of removing qmk#10088 hotfix. Now only affects Ergodox Infinity, Whitefox and K-type, though. Switches over Ergodox Infinity to the `IC_TEENSY_3_1` board, since that was a nice place to implement the `restart_usb_driver` override. However, I would guess this issue is present for other K20x/Teensy 3.1 boards as well... * Fix comment regarding `IC_TEENSY_3_1` for all keyboards using it.

for history see: SonixQMK/qmk_firmware#28 qmk/qmk_firmware#12870

…mk#12870) * Remove the qmk#10088 hotfix for K20x MCU:s. It seems to _cause_ the issue it intended to solve there. * Cleaner way of removing qmk#10088 hotfix. Now only affects Ergodox Infinity, Whitefox and K-type, though. Switches over Ergodox Infinity to the `IC_TEENSY_3_1` board, since that was a nice place to implement the `restart_usb_driver` override. However, I would guess this issue is present for other K20x/Teensy 3.1 boards as well... * Fix comment regarding `IC_TEENSY_3_1` for all keyboards using it.

Remove the qmk#10088 hotfix for K20x MCU:s.

bc8a81b

It seems to _cause_ the issue it intended to solve there.

github-actions bot added the core label May 11, 2021

stale bot added the awaiting changes label Jun 26, 2021

tzarc added the in progress label Jun 27, 2021

drashna requested a review from a team July 3, 2021 07:29

stale bot removed the awaiting changes label Jul 3, 2021

github-actions bot added the keyboard label Jul 3, 2021

firetech changed the title ~~Remove the #10088 hotfix for K20x MCU:s~~ Remove the #10088 hotfix for Teensy 3.1-like Input:Club keyboards Jul 3, 2021

Fix comment regarding IC_TEENSY_3_1 for all keyboards using it.

6b56865

drashna approved these changes Jul 3, 2021

View reviewed changes

drashna requested a review from a team July 3, 2021 14:46

drashna removed the in progress label Jul 3, 2021

tzarc approved these changes Aug 3, 2021

View reviewed changes

tzarc merged commit 1409b36 into qmk:develop Aug 3, 2021

firetech deleted the fix_wakeup_k20x branch August 3, 2021 21:39

stdvar pushed a commit to SonixQMK/ChibiOS-Contrib that referenced this pull request Oct 11, 2021

Override restart_usb_driver for SN32 to avoid kb crash on remote wakeup

9d7ec1f

for history see: SonixQMK/qmk_firmware#28 qmk/qmk_firmware#12870

stdvar mentioned this pull request Oct 11, 2021

Override restart_usb_driver for SN32 to avoid kb crash on remote wakeup SonixQMK/ChibiOS-Contrib#24

Merged

stdvar pushed a commit to SonixQMK/qmk_firmware that referenced this pull request Oct 11, 2021

Use restart_usb_driver redefined in board chibios board file

9dfc2fa

for history see: #28 qmk#12870 SonixQMK/ChibiOS-Contrib#24

stdvar mentioned this pull request Oct 11, 2021

Use restart_usb_driver redefined in board chibios board file SonixQMK/qmk_firmware#123

Merged

14 tasks

dexter93 pushed a commit to SonixQMK/qmk_firmware that referenced this pull request Oct 12, 2021

Use restart_usb_driver redefined in board chibios board file (#123)

1d3688a

for history see: #28 qmk#12870 SonixQMK/ChibiOS-Contrib#24 Co-authored-by: stdvar <[email protected]>

sigprof mentioned this pull request Oct 30, 2021

Fix remote wakeup issue #14988

Closed

14 tasks

dexter93 pushed a commit to SonixQMK/ChibiOS-Contrib that referenced this pull request Nov 21, 2021

Override restart_usb_driver for SN32 to avoid kb crash on remote wakeup

a9fd30b

for history see: SonixQMK/qmk_firmware#28 qmk/qmk_firmware#12870

dexter93 pushed a commit to dexter93/ChibiOS-Contrib that referenced this pull request Jan 4, 2022

Override restart_usb_driver for SN32 to avoid kb crash on remote wakeup

240d946

for history see: SonixQMK/qmk_firmware#28 qmk/qmk_firmware#12870

dexter93 pushed a commit to SonixQMK/ChibiOS-Contrib that referenced this pull request Feb 12, 2022

Override restart_usb_driver for SN32 to avoid kb crash on remote wakeup

ac26bc1

for history see: SonixQMK/qmk_firmware#28 qmk/qmk_firmware#12870

sigprof mentioned this pull request Feb 13, 2022

IC_TEENSY3_1 board config wakes from sleep successfully #16326

Closed

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove the #10088 hotfix for Teensy 3.1-like Input:Club keyboards #12870

Remove the #10088 hotfix for Teensy 3.1-like Input:Club keyboards #12870

firetech commented May 11, 2021 •

edited

Loading

tzarc commented May 11, 2021

firetech commented May 11, 2021 •

edited

Loading

drashna commented May 11, 2021

firetech commented May 11, 2021

stale bot commented Jun 26, 2021

firetech commented Jun 26, 2021

drashna commented Jul 3, 2021

firetech commented Jul 3, 2021

Remove the #10088 hotfix for Teensy 3.1-like Input:Club keyboards #12870

Remove the #10088 hotfix for Teensy 3.1-like Input:Club keyboards #12870

Conversation

firetech commented May 11, 2021 • edited Loading

Description

Types of Changes

Checklist

tzarc commented May 11, 2021

firetech commented May 11, 2021 • edited Loading

drashna commented May 11, 2021

firetech commented May 11, 2021

stale bot commented Jun 26, 2021

firetech commented Jun 26, 2021

drashna commented Jul 3, 2021

firetech commented Jul 3, 2021

firetech commented May 11, 2021 •

edited

Loading

firetech commented May 11, 2021 •

edited

Loading