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

Fix joystick functionality for ChibiOS and OTG (Blackpill) #18631

Merged
merged 5 commits into from
Oct 12, 2022

Conversation

fauxpark
Copy link
Member

@fauxpark fauxpark commented Oct 7, 2022

Description

This fixes an issue I was seeing where no joystick reports were being sent on my Blackpill testbed, but worked fine on other ARM boards (eg. Proton-C and Bluepill). Turns out the joystick endpoint setup (and likely digitizer too, but I cannot test it) was incorrect.

A bunch of empty IN notification callbacks were also removed.

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Issues Fixed or Closed by This PR

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).

Copy link
Contributor

@sigprof sigprof 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 this needs some fixes to support DIGITIZER_SHARED_EP properly (not actually tested).

tmk_core/protocol/chibios/usb_main.c Outdated Show resolved Hide resolved
tmk_core/protocol/chibios/usb_main.c Outdated Show resolved Hide resolved
@sigprof
Copy link
Contributor

sigprof commented Oct 7, 2022

There is a digitizer keymap for onekey that should send something, but I never actually tried it.

Co-authored-by: Sergey Vlasov <sigprof@gmail.com>
Copy link
Member

@drashna drashna left a comment

Choose a reason for hiding this comment

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

Doesn't seem to break anything, at least.

@fauxpark
Copy link
Member Author

fauxpark commented Oct 7, 2022

Looks like Blackpill onekey digitizer keymap already works on develop (somehow), but this PR doesn't break it 👍

@fauxpark fauxpark merged commit 2078a56 into qmk:develop Oct 12, 2022
@fauxpark fauxpark deleted the chibios-joystick-and-digitizer branch October 12, 2022 23:28
sigprof added a commit to sigprof/qmk_firmware that referenced this pull request Oct 22, 2022
In qmk#18631 some IN notification callbacks that were doing nothing were
removed, which should be a valid thing to do (ChibiOS HAL checks the
`in_cb` and `out_cb` pointers for being non-NULL before invoking those
optional callbacks).  However, it turned out that some less popular USB
LLDs (KINETIS and MIMXRT1062) have their own checks for those pointers,
and (incorrectly) skip the ChibiOS callback handling when those pointers
are NULL, which breaks the code for the `USB_USE_WAIT` configuration
option (the waiting thread never gets resumed if the corresponding
callback pointer is NULL).

Add those dummy callbacks again (but use a single function for all of
them instead of individual ones for each endpoint); this restores the
KINETIS and MIMXRT1062 boards to the working state while the LLDs are
getting fixed.
zvecr pushed a commit that referenced this pull request Oct 22, 2022
In #18631 some IN notification callbacks that were doing nothing were
removed, which should be a valid thing to do (ChibiOS HAL checks the
`in_cb` and `out_cb` pointers for being non-NULL before invoking those
optional callbacks).  However, it turned out that some less popular USB
LLDs (KINETIS and MIMXRT1062) have their own checks for those pointers,
and (incorrectly) skip the ChibiOS callback handling when those pointers
are NULL, which breaks the code for the `USB_USE_WAIT` configuration
option (the waiting thread never gets resumed if the corresponding
callback pointer is NULL).

Add those dummy callbacks again (but use a single function for all of
them instead of individual ones for each endpoint); this restores the
KINETIS and MIMXRT1062 boards to the working state while the LLDs are
getting fixed.
ramonimbao pushed a commit to ramonimbao/qmk_firmware that referenced this pull request Nov 28, 2022
Co-authored-by: Sergey Vlasov <sigprof@gmail.com>
ramonimbao pushed a commit to ramonimbao/qmk_firmware that referenced this pull request Nov 28, 2022
In qmk#18631 some IN notification callbacks that were doing nothing were
removed, which should be a valid thing to do (ChibiOS HAL checks the
`in_cb` and `out_cb` pointers for being non-NULL before invoking those
optional callbacks).  However, it turned out that some less popular USB
LLDs (KINETIS and MIMXRT1062) have their own checks for those pointers,
and (incorrectly) skip the ChibiOS callback handling when those pointers
are NULL, which breaks the code for the `USB_USE_WAIT` configuration
option (the waiting thread never gets resumed if the corresponding
callback pointer is NULL).

Add those dummy callbacks again (but use a single function for all of
them instead of individual ones for each endpoint); this restores the
KINETIS and MIMXRT1062 boards to the working state while the LLDs are
getting fixed.
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.

None yet

3 participants