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(ext-power): Cut power when PM is sleeping #417

Merged
merged 1 commit into from
Dec 29, 2020

Conversation

Nicell
Copy link
Member

@Nicell Nicell commented Nov 23, 2020

This adds support for cutting ext-power when the power management policy is set to low power using the central Zephyr device power management system.

@Nicell Nicell added core Core functionality/behavior of ZMK enhancement New feature or request labels Nov 23, 2020
@Nicell Nicell force-pushed the ext-power/device-sleep branch 2 times, most recently from ea24617 to d9cda27 Compare December 15, 2020 19:19
Copy link
Contributor

@innovaker innovaker left a comment

Choose a reason for hiding this comment

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

This looks OK on the face of it, but I don't know this part of the system yet (and I can't test it), so I'm going to assign it to @petejohanson.

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.

Just one comment. Probably not needed yet, but when we get to turning devices to low power state due to user idle, we'll want the ability to switch power back on again.

switch (ctrl_command) {
case DEVICE_PM_SET_POWER_STATE:
if (*((uint32_t *)context) == DEVICE_PM_ACTIVE_STATE) {
data->pm_state = DEVICE_PM_ACTIVE_STATE;
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this also be calling ext_power_generic_enable?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not in this case. When the device turns back on from sleep, the ext_power device is restarted and will automatically turn ext power back to its last saved state from settings. When we disable before going to sleep, that off state isn't saved in settings, so we'll properly retrieve the last state.

If we explicitly enabled here, people's power cut offs would turn back on whenever they wake up and get saved that way.

Copy link
Contributor

Choose a reason for hiding this comment

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

My point is, when we add per-device power state management, we will want to cut/restore power state based on idle/active.

So we'll need an API for "on/off" that isn't state persisting. So we can call that from here.

Doesn't need to be this PR, so I'm going to approve for now.

We also need to consider that other devices may say "no, don't sleep now!" and abort the sleep state change, I believe.

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.

Approved, with the knowledge that this is a first step.

Thanks @Nicell!

@petejohanson petejohanson merged commit 43f6d79 into zmkfirmware:main Dec 29, 2020
static int ext_power_generic_pm_control(const struct device *dev, uint32_t ctrl_command,
void *context, device_pm_cb cb, void *arg) {
int rc;
struct ext_power_generic_data *data = dev->driver_data;
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like the Zephyr 2.3.0 API. Was this tested with 2.4.0?

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.

3 participants