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

feat(kscan): Add toggle-mode to updated direct-wire kscan #1305

Merged
merged 6 commits into from May 19, 2022

Conversation

kurtis-lew
Copy link
Contributor

An updated version of #1173. Currently being tested by @ebastler without any issues thus far.

Implements a toggle-mode boolean to direct-wire kscans to disable pullup/pulldown resistors on active pins, removing the 244.4uA quiescent current that would otherwise exist without this feature.

Copy link
Contributor

@petejohanson petejohanson left a comment

Choose a reason for hiding this comment

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

A couple questions/thoughts.

app/drivers/kscan/kscan_gpio_direct.c Outdated Show resolved Hide resolved
app/drivers/kscan/kscan_gpio_direct.c Outdated Show resolved Hide resolved
kurtis-lew and others added 2 commits May 17, 2022 15:41
Co-authored-by: Pete Johanson <peter@peterjohanson.com>
- Fix logic in getting pulls from ACTIVE_LOW vs. ACTIVE_HIGH DT flags
- Add pulls on init
Copy link
Contributor

@petejohanson petejohanson left a comment

Choose a reason for hiding this comment

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

Seems closer! Tested just fine on my BDN9, but a few tweaks left for the code, IMHO.

app/drivers/kscan/kscan_gpio_direct.c Outdated Show resolved Hide resolved
app/drivers/kscan/kscan_gpio_direct.c Outdated Show resolved Hide resolved
@kurtis-lew kurtis-lew force-pushed the toggleswitch_2.0_PR branch 2 times, most recently from dcab9a3 to 24affa0 Compare May 18, 2022 05:47
Copy link
Contributor

@petejohanson petejohanson left a comment

Choose a reason for hiding this comment

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

One lingering final nitpick. Thanks for the quick turnaround.

@@ -124,6 +125,32 @@ static void kscan_direct_irq_callback_handler(const struct device *port, struct
}
#endif

static gpio_flags_t kscan_gpio_get_extra_flags(const struct gpio_dt_spec *gpio, bool active) {
gpio_flags_t flags = BIT(0) & gpio->dt_flags;
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we need to return the first bit here at all, since that will be set in the DT still. We need to check it, but not include in the "extra" flags we'll use.

Minor thing, but in case we end up using this some other way later, good to really only just return the extra flags we need, not also include the active bit.

@netlify
Copy link

netlify bot commented May 19, 2022

👷 Deploy request for zmk accepted.

Name Link
🔨 Latest commit 6697abb
🔍 Latest deploy log https://app.netlify.com/sites/zmk/deploys/62865b2c5fab860008d7fabc

Copy link
Contributor

@petejohanson petejohanson left a comment

Choose a reason for hiding this comment

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

Thanks!

@petejohanson petejohanson merged commit 0bde987 into zmkfirmware:main May 19, 2022
@Mithrandir2k18
Copy link

The docs on this aren't quite clear to me. If I have a single-row 18 key half, where each switch connects different GPIO pin to ground, is toggle-mode a straight up upgrade to input-gpios?

@ebastler
Copy link
Contributor

Toggle mode would not work with single switches. It is intended for 2, 3, or 4 way slider switches. kurtis originally developed it for the selector switch BT1/BT2/USB on the electronic materials office Altar-I, and I have not seen many other boards that use similar switches, so it's pretty niche.

Those switches always have at least one GPIO shorted to GND which leads to very high battery draw even in idle. Toggle mode circumvents this, by disabling keyscan on a pin as soon as it is switched to ON, then reenabling it the moment another pin goes to ON, and disabling that pin.

On single keyboard pushbuttons wired from GPIO to GND that issue does not exist, as they are only drawing power when actively pressed down by the user, and once disabled they would never be reenabled as the MCU can not know when it is depressed.

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.

None yet

4 participants