Skip to content

drivers: dma: cc23x0: Add power management #92219

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
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
58 changes: 46 additions & 12 deletions drivers/dma/dma_ti_cc23x0.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ LOG_MODULE_REGISTER(dma_cc23x0, CONFIG_DMA_LOG_LEVEL);
#include <zephyr/device.h>
#include <zephyr/drivers/dma.h>
#include <zephyr/irq.h>
#include <zephyr/pm/device.h>
#include <zephyr/pm/policy.h>
#include <zephyr/sys/util.h>

#include <driverlib/clkctl.h>
Expand Down Expand Up @@ -56,6 +58,18 @@ struct dma_cc23x0_data {
struct dma_cc23x0_channel channels[UDMA_NUM_CHANNELS];
};

static inline void dma_cc23x0_pm_policy_state_lock_get(void)
{
pm_policy_state_lock_get(PM_STATE_RUNTIME_IDLE, PM_ALL_SUBSTATES);
pm_policy_state_lock_get(PM_STATE_STANDBY, PM_ALL_SUBSTATES);
}

static inline void dma_cc23x0_pm_policy_state_lock_put(void)
{
pm_policy_state_lock_put(PM_STATE_STANDBY, PM_ALL_SUBSTATES);
pm_policy_state_lock_put(PM_STATE_RUNTIME_IDLE, PM_ALL_SUBSTATES);
}

/*
* If the channel is a software channel, then the completion will be signaled
* on this DMA dedicated interrupt.
Expand Down Expand Up @@ -343,27 +357,44 @@ static int dma_cc23x0_get_status(const struct device *dev, uint32_t channel,
return 0;
}

static int dma_cc23x0_init(const struct device *dev)
static int dma_cc23x0_pm_action(const struct device *dev, enum pm_device_action action)
{
struct dma_cc23x0_data *data = dev->data;
int ret = 0;

switch (action) {
case PM_DEVICE_ACTION_SUSPEND:
uDMADisable();
CLKCTLDisable(CLKCTL_BASE, CLKCTL_DMA);
dma_cc23x0_pm_policy_state_lock_put();
break;
case PM_DEVICE_ACTION_RESUME:
dma_cc23x0_pm_policy_state_lock_get();
CLKCTLEnable(CLKCTL_BASE, CLKCTL_DMA);
uDMAEnable();
/* Set base address for channel control table (descriptors) */
uDMASetControlBase(data->desc);
break;
case PM_DEVICE_ACTION_TURN_ON:
case PM_DEVICE_ACTION_TURN_OFF:
break;
default:
ret = -ENOTSUP;
}

return ret;
}
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't you be handling TURN_ON and OFF in case this device is part of a power domain ?

Also, I am not sure if those early return in the switch statement aren't violating our code guidelines. I rather have one variable holding the return value and one return statement in the end of this function :p

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the review @ceolin. Your feedbacks above are now implemented.


static int dma_cc23x0_init(const struct device *dev)
{
IRQ_CONNECT(DT_INST_IRQN(0),
DT_INST_IRQ(0, priority),
dma_cc23x0_isr,
DEVICE_DT_INST_GET(0),
0);
irq_enable(DT_INST_IRQN(0));

/* Enable clock */
CLKCTLEnable(CLKCTL_BASE, CLKCTL_DMA);

/* Enable DMA */
uDMAEnable();

/* Set base address for channel control table (descriptors) */
uDMASetControlBase(data->desc);

return 0;
return pm_device_driver_init(dev, dma_cc23x0_pm_action);
}

static struct dma_cc23x0_data cc23x0_data;
Expand All @@ -376,7 +407,10 @@ static DEVICE_API(dma, dma_cc23x0_api) = {
.get_status = dma_cc23x0_get_status,
};

DEVICE_DT_INST_DEFINE(0, dma_cc23x0_init, NULL,
PM_DEVICE_DT_INST_DEFINE(0, dma_cc23x0_pm_action);

DEVICE_DT_INST_DEFINE(0, dma_cc23x0_init,
PM_DEVICE_DT_INST_GET(0),
&cc23x0_data, NULL,
PRE_KERNEL_1, CONFIG_DMA_INIT_PRIORITY,
&dma_cc23x0_api);
Loading