Skip to content

soc: silabs: siwx91x: link pm state to power domain and adjust driver #100116

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Dec 1, 2025
+46 −49
Merged

soc: silabs: siwx91x: link pm state to power domain and adjust driver #100116

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c1a6252
drivers: power_domain: siwx91x: fix the link between pd and cpu state
Martinhoff-maker Nov 26, 2025
241784b
drivers: spi: siwx91x: ensure device runtime is released on error
Martinhoff-maker Nov 26, 2025
997023f
drivers: dma: siwx91x: replace pm_state with pm_device (udma and gpdma)
Martinhoff-maker Nov 26, 2025
0848a82
drivers: dma: siwx91x: fix interrupt flag reset
Martinhoff-maker Nov 26, 2025
9f1da5c
drivers: i2s: siwx91x: ensure device runtime is released
Martinhoff-maker Nov 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 9 additions & 20 deletions drivers/dma/dma_silabs_siwx91x.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
#include <zephyr/drivers/clock_control.h>
#include <zephyr/logging/log.h>
#include <zephyr/pm/device.h>
#include <zephyr/pm/policy.h>
#include <zephyr/pm/device_runtime.h>
#include <zephyr/types.h>
#include "rsi_rom_udma.h"
#include "rsi_rom_udma_wrapper.h"
Expand Down Expand Up @@ -63,16 +63,6 @@ struct dma_siwx91x_data {
*/
};

static void siwx91x_dma_pm_policy_state_lock_get(void)
{
pm_policy_state_lock_get(PM_STATE_SUSPEND_TO_IDLE, PM_ALL_SUBSTATES);
}

static void siwx91x_dma_pm_policy_state_lock_put(void)
{
pm_policy_state_lock_put(PM_STATE_SUSPEND_TO_IDLE, PM_ALL_SUBSTATES);
}

static enum dma_xfer_dir siwx91x_transfer_direction(uint32_t dir)
{
if (dir == MEMORY_TO_MEMORY) {
Expand Down Expand Up @@ -495,15 +485,14 @@ static int siwx91x_dma_start(const struct device *dev, uint32_t channel)
return -EINVAL;
}

/* Get the power management policy state lock */
if (!data->zephyr_channel_info[channel].channel_active) {
Copy link
Contributor

@smalae smalae Nov 27, 2025

Choose a reason for hiding this comment

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

Should we also call pm_device_runtime_put() and pm_device_runtime_get() in DMA drivers to ensure that the user doesn’t have to explicitly manage this for memory-to-memory transfers?

Copy link
Member Author

@Martinhoff-maker Martinhoff-maker Nov 27, 2025

Choose a reason for hiding this comment

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

I wonder to add that like intel has done here: dma_intel_adsp_hda.c
I will try but I fear that we miss some cases that introduce some bugs.

Copy link
Member Author

@Martinhoff-maker Martinhoff-maker Nov 27, 2025

Choose a reason for hiding this comment

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

that's done, seems to works

siwx91x_dma_pm_policy_state_lock_get();
pm_device_runtime_get(dev);
data->zephyr_channel_info[channel].channel_active = true;
}

if (RSI_UDMA_ChannelEnable(udma_handle, channel) != 0) {
if (data->zephyr_channel_info[channel].channel_active) {
siwx91x_dma_pm_policy_state_lock_put();
pm_device_runtime_put(dev);
data->zephyr_channel_info[channel].channel_active = false;
}
return -EINVAL;
Expand Down Expand Up @@ -534,7 +523,7 @@ static int siwx91x_dma_stop(const struct device *dev, uint32_t channel)
}

if (data->zephyr_channel_info[channel].channel_active) {
siwx91x_dma_pm_policy_state_lock_put();
pm_device_runtime_put(dev);
data->zephyr_channel_info[channel].channel_active = false;
}

Expand Down Expand Up @@ -680,16 +669,16 @@ static void siwx91x_dma_isr(const struct device *dev)
}

if (data->chan_info[channel].Cnt == data->chan_info[channel].Size) {
sys_write32(BIT(channel), (mem_addr_t)&cfg->reg->UDMA_DONE_STATUS_REG);
if (data->zephyr_channel_info[channel].channel_active) {
pm_device_runtime_put_async(dev, K_NO_WAIT);
data->zephyr_channel_info[channel].channel_active = false;
}
if (data->zephyr_channel_info[channel].dma_callback) {
/* Transfer complete, call user callback */
data->zephyr_channel_info[channel].dma_callback(
dev, data->zephyr_channel_info[channel].cb_data, channel, 0);
}
sys_write32(BIT(channel), (mem_addr_t)&cfg->reg->UDMA_DONE_STATUS_REG);
if (data->zephyr_channel_info[channel].channel_active) {
siwx91x_dma_pm_policy_state_lock_put();
data->zephyr_channel_info[channel].channel_active = false;
}
} else {
/* Call UDMA ROM IRQ handler. */
ROMAPI_UDMA_WRAPPER_API->uDMAx_IRQHandler(&udma_resources, udma_resources.desc,
Expand Down
46 changes: 24 additions & 22 deletions drivers/dma/dma_silabs_siwx91x_gpdma.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
#include <zephyr/drivers/clock_control.h>
#include <zephyr/logging/log.h>
#include <zephyr/pm/device.h>
#include <zephyr/pm/policy.h>
#include <zephyr/pm/device_runtime.h>
#include "rsi_gpdma.h"
#include "rsi_rom_gpdma.h"

Expand Down Expand Up @@ -43,6 +43,7 @@ struct siwx91x_gpdma_channel_info {
void *cb_data;
RSI_GPDMA_DESC_T *desc;
enum gpdma_xfer_dir xfer_direction;
bool channel_active;
};

struct siwx91x_gpdma_config {
Expand All @@ -62,16 +63,6 @@ struct siwx19x_gpdma_data {
uint8_t reload_compatible;
};

static void siwx91x_gpdma_pm_policy_state_lock_get(void)
{
pm_policy_state_lock_get(PM_STATE_SUSPEND_TO_IDLE, PM_ALL_SUBSTATES);
}

static void siwx91x_gpdma_pm_policy_state_lock_put(void)
{
pm_policy_state_lock_put(PM_STATE_SUSPEND_TO_IDLE, PM_ALL_SUBSTATES);
}

static bool siwx91x_gpdma_is_priority_valid(uint32_t channel_priority)
{
return (channel_priority >= GPDMA_MIN_PRIORITY && channel_priority <= GPDMA_MAX_PRIORITY);
Expand Down Expand Up @@ -408,19 +399,22 @@ static int siwx91x_gpdma_reload(const struct device *dev, uint32_t channel, uint

static int siwx91x_gpdma_start(const struct device *dev, uint32_t channel)
{
const struct siwx91x_gpdma_config *cfg = dev->config;
struct siwx19x_gpdma_data *data = dev->data;

if (channel >= data->dma_ctx.dma_channels) {
return -EINVAL;
}

if (!sys_test_bit((mem_addr_t)&cfg->reg->GLOBAL.DMA_CHNL_ENABLE_REG, channel)) {
siwx91x_gpdma_pm_policy_state_lock_get();
if (data->chan_info[channel].channel_active) {
pm_device_runtime_get(dev);
data->chan_info[channel].channel_active = true;
}

if (RSI_GPDMA_DMAChannelTrigger(&data->hal_ctx, channel)) {
siwx91x_gpdma_pm_policy_state_lock_put();
if (data->chan_info[channel].channel_active) {
pm_device_runtime_put(dev);
data->chan_info[channel].channel_active = false;
}
return -EINVAL;
}

Expand All @@ -429,18 +423,13 @@ static int siwx91x_gpdma_start(const struct device *dev, uint32_t channel)

static int siwx91x_gpdma_stop(const struct device *dev, uint32_t channel)
{
const struct siwx91x_gpdma_config *cfg = dev->config;
struct siwx19x_gpdma_data *data = dev->data;
k_spinlock_key_t key;

if (channel >= data->dma_ctx.dma_channels) {
return -EINVAL;
}

if (sys_test_bit((mem_addr_t)&cfg->reg->GLOBAL.DMA_CHNL_ENABLE_REG, channel)) {
siwx91x_gpdma_pm_policy_state_lock_put();
}

if (RSI_GPDMA_AbortChannel(&data->hal_ctx, channel)) {
return -EINVAL;
}
Expand All @@ -449,6 +438,11 @@ static int siwx91x_gpdma_stop(const struct device *dev, uint32_t channel)
siwx91x_gpdma_free_desc(data->desc_pool, data->chan_info[channel].desc);
k_spin_unlock(&data->desc_pool_lock, key);

if (data->chan_info[channel].channel_active) {
pm_device_runtime_put(dev);
data->chan_info[channel].channel_active = false;
}

return 0;
}

Expand Down Expand Up @@ -552,7 +546,10 @@ static void siwx91x_gpdma_isr(const struct device *dev)
if (channel_int_status & abort_mask) {
RSI_GPDMA_AbortChannel(&data->hal_ctx, channel);
cfg->reg->GLOBAL.INTERRUPT_STAT_REG = abort_mask;
siwx91x_gpdma_pm_policy_state_lock_put();
if (data->chan_info[channel].channel_active) {
pm_device_runtime_put_async(dev, K_NO_WAIT);
data->chan_info[channel].channel_active = false;
}
}

if (channel_int_status & desc_fetch_mask) {
Expand All @@ -569,11 +566,16 @@ static void siwx91x_gpdma_isr(const struct device *dev)
k_spin_unlock(&data->desc_pool_lock, key);
data->chan_info[channel].desc = NULL;
cfg->reg->GLOBAL.INTERRUPT_STAT_REG = done_mask;

if (data->chan_info[channel].channel_active) {
pm_device_runtime_put_async(dev, K_NO_WAIT);
data->chan_info[channel].channel_active = false;
}

if (data->chan_info[channel].cb) {
data->chan_info[channel].cb(dev, data->chan_info[channel].cb_data, channel,
0);
}
siwx91x_gpdma_pm_policy_state_lock_put();
}
}

Expand Down
8 changes: 2 additions & 6 deletions drivers/i2s/i2s_silabs_siwx91x.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -442,9 +442,7 @@ static void i2s_siwx91x_dma_rx_callback(const struct device *dma_dev, void *user

rx_disable:
i2s_siwx91x_stream_disable(stream, dma_dev);
if (stream->state == I2S_STATE_READY && stream->last_block) {
pm_device_runtime_put_async(i2s_dev, K_NO_WAIT);
}
pm_device_runtime_put_async(i2s_dev, K_NO_WAIT);
}

static void i2s_siwx91x_dma_tx_callback(const struct device *dma_dev, void *user_data,
Expand Down Expand Up @@ -511,9 +509,7 @@ static void i2s_siwx91x_dma_tx_callback(const struct device *dma_dev, void *user

tx_disable:
i2s_siwx91x_stream_disable(stream, dma_dev);
if (stream->state == I2S_STATE_READY && stream->last_block) {
pm_device_runtime_put_async(i2s_dev, K_NO_WAIT);
}
pm_device_runtime_put_async(i2s_dev, K_NO_WAIT);
}

static int i2s_siwx91x_param_config(const struct device *dev, enum i2s_dir dir)
Expand Down
4 changes: 3 additions & 1 deletion drivers/power_domain/power_domain_silabs_siwx91x.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,20 @@
*/

#include <zephyr/pm/device.h>
#include <zephyr/pm/device_runtime.h>
#include <zephyr/pm/policy.h>

#define DT_DRV_COMPAT silabs_siwx91x_power_domain

static int siwx91x_pd_pm_action(const struct device *dev, enum pm_device_action action)
{
switch (action) {
case PM_DEVICE_ACTION_RESUME:
pm_policy_device_power_lock_get(dev);
pm_device_children_action_run(dev, PM_DEVICE_ACTION_TURN_ON, NULL);
break;
case PM_DEVICE_ACTION_SUSPEND:
pm_device_children_action_run(dev, PM_DEVICE_ACTION_TURN_OFF, NULL);
pm_policy_device_power_lock_put(dev);
break;
case PM_DEVICE_ACTION_TURN_ON:
break;
Expand Down
4 changes: 4 additions & 0 deletions drivers/spi/spi_silabs_siwx91x_gspi.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -555,6 +555,7 @@ static int gspi_siwx91x_transceive(const struct device *dev, const struct spi_co
ret = gspi_siwx91x_config(dev, config, cb, userdata);
if (ret) {
spi_context_release(&data->ctx, ret);
pm_device_runtime_put(dev);
return ret;
}
}
Expand All @@ -566,6 +567,9 @@ static int gspi_siwx91x_transceive(const struct device *dev, const struct spi_co
if (spi_siwx91x_is_dma_enabled_instance(dev)) {
/* Perform DMA transceive */
ret = gspi_siwx91x_transceive_dma(dev, config);
if (ret < 0) {
pm_device_runtime_put(dev);
Copy link
Contributor

@jerome-pouiller jerome-pouiller Nov 27, 2025

Choose a reason for hiding this comment

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

What happens if asynchronous==false && dma=true && ret==0 ?

Copy link
Member Author

@Martinhoff-maker Martinhoff-maker Nov 27, 2025

Choose a reason for hiding this comment

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

Since this PR is mainly about PM, would do it in another PR.

}
spi_context_release(&data->ctx, ret);
Copy link
Contributor

@jerome-pouiller jerome-pouiller Nov 27, 2025

Choose a reason for hiding this comment

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

BTW, is the context released when asynchronous==true && dma=true && ret<0 ?

Copy link
Member Author

@Martinhoff-maker Martinhoff-maker Nov 27, 2025

Choose a reason for hiding this comment

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

same as above comment.

} else {
/* Perform synchronous polling transceive */
Expand Down
1 change: 1 addition & 0 deletions dts/arm/silabs/siwg917.dtsi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@
siwx91x_soc_pd: siwx91x-soc-pd {
compatible = "silabs,siwx91x-power-domain";
#power-domain-cells = <0>;
zephyr,disabling-power-states = <&pstate_ps4_sleep>;
zephyr,pm-device-runtime-auto;
status = "okay";
};
Expand Down
3 changes: 3 additions & 0 deletions soc/silabs/silabs_siwx91x/Kconfig.defconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ configdefault PM_DEVICE_RUNTIME
configdefault POWER_DOMAIN
default y

configdefault PM_POLICY_DEVICE_CONSTRAINTS
default y

endif # PM_DEVICE

if SILABS_SIWX91X_NWP
Expand Down