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(ext-power): Initialize as soon as settings are available #675

Merged
merged 2 commits into from
Feb 16, 2021

Conversation

Nicell
Copy link
Member

@Nicell Nicell commented Feb 9, 2021

This adjusts the initialization of the ext_power driver to be right after the flash systems have been initialized (All flash systems initialize by POST_KERNEL priority 80). This way we know ext_power is properly initialized for the rest of the devices that need external power on start.

Currently, this fixes displays not turning on after deep sleep, but this should prevent similar issues in the future.

This is a step towards recovering devices when ext_power is in use. The next step is tying more devices to the PM policy so they can re-initialize without the entire system needing to be reset when ext_power is used while we keep context (idle, behaviors, etc).

I'm wondering if this init priority should be a Kconfig value.

Some boards' power switch takes some substantial time to actually switch to powered on, so the init-delay-ms property has been added, which holds the system for n milliseconds after enabling (or disabling) the power switch on initialization. This makes sure that when devices like the OLED are initialized, the power is ready to be used.

This is also not fully tested but works in theory. We need to reconfirm this change works and doesn't break the settings integration with ext_power Re-init tested by @mcrosson, @tominabox1, and @karnadii. Settings confirmed to hold on start by @mcrosson.

@Nicell Nicell added enhancement New feature or request core Core functionality/behavior of ZMK labels Feb 9, 2021
@mcrosson
Copy link
Contributor

mcrosson commented Feb 9, 2021

Initial testing looks good. Settings for "off" on ext_pwr also seem to work properly.

@tominabox1
Copy link
Contributor

The additional delay fixed the issue I was having with my BLe Chiffre 👍🏼

@mcrosson
Copy link
Contributor

bump? sounds like this is working all around for folks

@karnadii
Copy link

The additional delay fixed the issue I was having with my BLe Chiffre 👍🏼

does this fix the wake up after deep sleep issue?

@tominabox1
Copy link
Contributor

tominabox1 commented Feb 14, 2021 via email

@karnadii
Copy link

karnadii commented Feb 15, 2021

it is definitely fixes problem wake up after sleep (the oled reinit and no freeze after wake up). but when ext vcc is cut off the oled still not reinit. ext power toggle and rgb underglow toggle still won't turn oled back on.

@Nicell
Copy link
Member Author

Nicell commented Feb 15, 2021

it is definitely fixes problem wake up after sleep (the oled reinit and no freeze after wake up). but when ext vcc is cut off the oled still not reinit. ext power toggle and rgb underglow toggle still won't turn oled back on.

This is known and is a separate issue than the one this fixes. Power management policy related items need to be added to drivers that make use of ext power, but for now this fixes ext power not being on early enough during full initialization.

@karnadii
Copy link

it is definitely fixes problem wake up after sleep (the oled reinit and no freeze after wake up). but when ext vcc is cut off the oled still not reinit. ext power toggle and rgb underglow toggle still won't turn oled back on.

This is known and is a separate issue than the one this fixes. Power management policy related items need to be added to drivers that make use of ext power, but for now this fixes ext power not being on early enough during full initialization.

ok, seems I misunderstand the PR. at least we have deep sleep now. I won't worry about my battery now. thanks for the PR.

@karnadii
Copy link

I have tried on my marvelous65-split using nrfmicro 1.3. but it did not work, the oled did not re-init and after a while it freeze. while my marvelous65 (nrfmicro 1.4, non-split) worked. Please someone with split keyboard confirm this.

@tominabox1
Copy link
Contributor

in your .dts file you have to add the appropriate init delay

ext-power { compatible = "zmk,ext-power-generic"; label = "EXT_POWER"; control-gpios = <&gpio0 7 GPIO_ACTIVE_LOW>; init-delay-ms = <250>; };

@karnadii
Copy link

karnadii commented Feb 15, 2021

in your .dts file you have to add the appropriate init delay

ext-power { compatible = "zmk,ext-power-generic"; label = "EXT_POWER"; control-gpios = <&gpio0 7 GPIO_ACTIVE_LOW>; init-delay-ms = <250>; };

what is this control-gpios = <&gpio0 7 GPIO_ACTIVE_LOW>? is it like the underglow will be ignored? unused pin?

@tominabox1
Copy link
Contributor

in your .dts file you have to add the appropriate init delay
ext-power { compatible = "zmk,ext-power-generic"; label = "EXT_POWER"; control-gpios = <&gpio0 7 GPIO_ACTIVE_LOW>; init-delay-ms = <250>; };

what is this control-gpios = <&gpio0 7 GPIO_ACTIVE_LOW>? is it like the underglow will be ignored? unused pin?

in the .dts file for your keyboard, if your board supports ext power toggle, there will be this section. With this PR, you have to add the init-delay-ms line to that block of code.

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 for digging into this!

@@ -223,13 +229,15 @@ static const struct ext_power_api api = {.enable = ext_power_generic_enable,
.disable = ext_power_generic_disable,
.get = ext_power_generic_get};

#define ZMK_EXT_POWER_INIT_PRIORITY 81
Copy link
Contributor

Choose a reason for hiding this comment

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

This is fine for now, but something to consider moving to Kconfig as needed.

@petejohanson petejohanson merged commit 5766136 into zmkfirmware:main Feb 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Core functionality/behavior of ZMK enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants