Skip to content

Conversation

@JordanYates
Copy link
Contributor

@JordanYates JordanYates commented Jul 16, 2023

Adds a helper function for initializing devices into the expected power
state, through the devices pm_device_action_cb_t. This eliminates code
duplication between the init functions and the PM callback.

The expected device states in order of priority are:

  • No power applied to device, OFF
  • zephyr,pm-device-runtime-auto enabled, SUSPEND
  • Otherwise, ACTIVE

Depends on #60410 (first 3 commits) to pass CI

Jordan Yates added 4 commits July 16, 2023 10:47
Generate a zero padded variant of `_ORD` that is suitable for use in
linker scripts with the `SORT` property, so that `6` is correctly placed
before `24`, and so on.

Signed-off-by: Jordan Yates <[email protected]>
Add a sub-priority field to `Z_INIT_ENTRY_SECTION`, which is used to
ensure that multiple drivers running at the same init priority still
run in an order that respects their devicetree dependencies.

This is primarily useful when multiple instances of the same device are
instantiated that can depend on each other, for example power domains
and i2c muxes.

Signed-off-by: Jordan Yates <[email protected]>
Add tests for sorting devices at the same init priority based on their
devicetree ordinals.

Signed-off-by: Jordan Yates <[email protected]>
Implement `gpio_pin_get_config` for the stellaris platform, and by
extension `qemu_cortex_m3`.

Signed-off-by: Jordan Yates <[email protected]>
Jordan Yates added 4 commits July 16, 2023 21:32
Adds a helper function for initializing devices into the expected power
state, through the devices `pm_device_action_cb_t`. This eliminates code
duplication between the init functions and the PM callback.

The expected device states in order of priority are:
 * No power applied to device, `OFF`
 * `zephyr,pm-device-runtime-auto` enabled, `SUSPEND`
 * Otherwise, `ACTIVE`

Signed-off-by: Jordan Yates <[email protected]>
Let the driver compile without `PM_DEVICE_POWER_DOMAIN`, in which case
the driver only controls the GPIO, without notifying dependant devices.

Signed-off-by: Jordan Yates <[email protected]>
Startup power domains according to the expected final state given the
power supply and PM device runtime support.

Signed-off-by: Jordan Yates <[email protected]>
Update the expected power domain behaviour in the tests in line with
the driver implementation changes.

Signed-off-by: Jordan Yates <[email protected]>
Test that `pm_device_driver_init` puts devices into the appropriate
state depending on the devicetree configuration.

Signed-off-by: Jordan Yates <[email protected]>
if (rc == 0) {
rc = action_cb(dev, PM_DEVICE_ACTION_RESUME);
}
return rc;
Copy link
Member

Choose a reason for hiding this comment

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

This may be confusing, if the target is built without PM_DEVICE enabled, we don't have a struct pm_device associated with it and consequently no tracking of device power states. My take is that this exists to re-use the action callback and avoid code duplication. Which is a valid reason imho.

Copy link
Member

Choose a reason for hiding this comment

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

blahh ... I've jumped into the changes before looking the pr message :)

Copy link
Member

@erwango erwango left a comment

Choose a reason for hiding this comment

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

Some comments of someone that didn't followed power domains introduction and is trying to catch up.

Copy link
Member

Choose a reason for hiding this comment

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

Not blocking but I feel not mentioning Power Domain in the test name makes it confusing.

bool pm_device_is_powered(const struct device *dev);

/**
* @brief Setup a device driver into the lowest valid power mode
Copy link
Member

Choose a reason for hiding this comment

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

I think we can remove "valid" here (unless there are invalid power modes, but this should be defined).
This being said it is unclear to me what would be the lowest power mode for a device. If s2ram is supported, could it be device in ram retention ?

@carlescufi carlescufi merged commit a4cfc1e into zephyrproject-rtos:main Jul 25, 2023
@JordanYates
Copy link
Contributor Author

This should not have been merged due to the dependence on #60410

@JordanYates
Copy link
Contributor Author

Sorry, I should have added a DNM label

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.

6 participants