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

Add required changes for zmk power domain support #8

Open
wants to merge 7 commits into
base: v3.0.0+zmk-fixes
Choose a base branch
from

Conversation

infused-kim
Copy link

@infused-kim infused-kim commented May 14, 2022

Hey guys, these are the necessary changes to add power domain support in ZMK.

Please review it in conjunction with the zmk PR: zmkfirmware/zmk#1300

Background Info

To be completely honest... I don't full understand many of the decisions behind zephyr's original code.

For example...

PM_DEVICE_ACTION_TURN_ON sets the device state to PM_DEVICE_STATE_SUSPENDED

I am not sure why, I would have expected it to set it to PM_DEVICE_STATE_ACTIVE.

Because of this, in the zmk code, I use PM_DEVICE_ACTION_RESUME in zmk's code to enable power domains.

You can find it here:
https://github.com/infused-kim/zmk-zephyr/blob/v3.0.0%2Bzmk-fixes%2Bpd-oled/subsys/pm/device.c#L140-L146

pd_gpio_pm_action forwards different actions to children

Another thing that seems off is that pd_gpio_pm_action forwards different actions to it's children.

For example:

  • PM_DEVICE_ACTION_RESUME runs PM_DEVICE_ACTION_TURN_ON on the children
  • PM_DEVICE_ACTION_SUSPEND runs PM_DEVICE_ACTION_TURN_OFF on the children
  • PM_DEVICE_ACTION_TURN_ON runs nothing on the children
  • PM_DEVICE_ACTION_TURN_OFF runs nothing on the children

I have modified it to always run the matching action to the children.

https://github.com/zmkfirmware/zephyr/blob/v3.0.0%2Bzmk-fixes/drivers/power_domain/power_domain_gpio.c#L48

pd_gpio_pm_action only toggled GPIO pin for RESUME and SUSPEND actions

For example:

  • PM_DEVICE_ACTION_RESUME set the pin to 1
  • PM_DEVICE_ACTION_SUSPEND set the pin 0
  • PM_DEVICE_ACTION_TURN_ON did NOT set the pin to 1, but instead configured it to GPIO_OUTPUT_INACTIVE
  • PM_DEVICE_ACTION_TURN_OFF did NOT set the pin to 0, but instead configured it to GPIO_DISCONNECTED

To be completely honest, I have no idea what it means for the pin to be configured to GPIO_OUTPUT_INACTIVE and GPIO_DISCONNECTED.

So I just changed the actions to turn and off the pins instead of configuring them.

https://github.com/zmkfirmware/zephyr/blob/v3.0.0%2Bzmk-fixes/drivers/power_domain/power_domain_gpio.c#L59

pd_gpio_init could fail if gpio_pin_configure_dt failed

This is another issue I ran into when adding a power domain as a child to another power domain.

For reference:
https://github.com/zmkfirmware/zephyr/blob/v3.0.0%2Bzmk-fixes/drivers/power_domain/power_domain_gpio.c#L84-L88

If a power domain is a child of another power domain, then the code attempts to configure the pin as GPIO_DISCONNECTED.

If that fails, the init function returns a non 0 return code.

That in turn causes the device to be marked as not initialized.

When attempting to get the device through device_get_binding(), it fails and prevents zmk code to not work properly.

So I changed this by ignoring the return code of gpio_pin_configure_dt and to always return 0.

In conclusion...

I have made it work, but I don't fully understand the original intentions of the zephyr developers.

Maybe these were bugs and I fixed them or maybe I just don't understand how power domains are supposed to work and made wrong assumptions.

I would really appreciate it if you could guide me to the right solution if what I did is not correct.

Thank you.

infused-kim and others added 7 commits May 3, 2022 23:26
Add API to add devices to a power domain in runtime. The number of
devices that can be added is defined in build time.

The script gen_handles.py will check the number defined in
`CONFIG_PM_DEVICE_POWER_DOMAIN_DYNAMIC` to resize the handles vector,
adding empty slots in the supported sector to be used later.

Signed-off-by: Flavio Ceolin <[email protected]>
Added log to pd_gpio_pm_action
When a power domain is connected to another power domain,`pd_gpio_init` attempts to configure the gpio pin to `GPIO_DISCONNECTED`.

But if that failed, the init function returned -EIO, which caused the device to be marked as not initialized / ready.

This meant that it couldn’t be retrieved through `device_get_binding()`.
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.

1 participant