Skip to content

Enable wakeup with uart shell from PM2 on RW612, fix uart issues #89760

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 6 commits into from
May 30, 2025
Merged

Enable wakeup with uart shell from PM2 on RW612, fix uart issues #89760

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
182866a
uart_mcux_flexcomm: Fix TX garbage char bug in PM
decsny May 10, 2025
8e6af6f
pm: refactor policy_device_ get/put
decsny May 9, 2025
fe1d1f9
pm: Add API to check if a state disables a device
decsny May 9, 2025
f2c48b6
soc: rw: Switch main clock on PM2 to LPOSC
decsny May 10, 2025
901e279
drivers: uart_mcux_flexcomm: Enable as wakeup src
decsny May 1, 2025
f1884c6
soc: rw: Enable flexcomm wakeup/sleep clocking
decsny May 10, 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
1 change: 1 addition & 0 deletions boards/nxp/frdm_rw612/frdm_rw612_common.dtsi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
current-speed = <115200>;
pinctrl-0 = <&pinmux_flexcomm3_usart>;
pinctrl-names = "default";
wakeup-source;
};

&flexcomm0 {
Expand Down
50 changes: 49 additions & 1 deletion drivers/clock_control/clock_control_mcux_syscon.c
View file
Open in desktop
Copy link
Collaborator

Choose a reason for hiding this comment

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

update copyright year

Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2020-2024 NXP
* Copyright 2020-2025 NXP
*
* SPDX-License-Identifier: Apache-2.0
*/
Expand Down Expand Up @@ -623,11 +623,59 @@ static int SYSCON_SET_FUNC_ATTR mcux_lpc_syscon_clock_control_set_subsys_rate(
}
}

static int mcux_lpc_syscon_clock_control_configure(const struct device *dev,
clock_control_subsys_t sub_system, void *data)
{
#ifdef CONFIG_SOC_SERIES_RW6XX
#define FLEXCOMM_LP_CLK_DECODE(n) (n & 0x80)
uint32_t clock_name = (uint32_t)sub_system;
int flexcomm_num = -1;

switch (clock_name) {
case MCUX_FLEXCOMM0_CLK:
case MCUX_FLEXCOMM0_LP_CLK:
flexcomm_num = 0;
break;
case MCUX_FLEXCOMM1_CLK:
case MCUX_FLEXCOMM1_LP_CLK:
flexcomm_num = 1;
break;
case MCUX_FLEXCOMM2_CLK:
case MCUX_FLEXCOMM2_LP_CLK:
flexcomm_num = 2;
break;
case MCUX_FLEXCOMM3_CLK:
case MCUX_FLEXCOMM3_LP_CLK:
flexcomm_num = 3;
break;
default:
return -ENOTSUP;
}

if (flexcomm_num >= 0) {
static uint32_t frgclksels[4];
static uint32_t frgctls[4];

if (FLEXCOMM_LP_CLK_DECODE(clock_name)) {
frgclksels[flexcomm_num] = CLKCTL1->FLEXCOMM[flexcomm_num].FRGCLKSEL;
frgctls[flexcomm_num] = CLKCTL1->FLEXCOMM[flexcomm_num].FRGCTL;
CLKCTL1->FLEXCOMM[flexcomm_num].FRGCLKSEL = 0;
CLKCTL1->FLEXCOMM[flexcomm_num].FRGCTL = 0;
} else {
CLKCTL1->FLEXCOMM[flexcomm_num].FRGCLKSEL = frgclksels[flexcomm_num];
CLKCTL1->FLEXCOMM[flexcomm_num].FRGCTL = frgctls[flexcomm_num];
}
}
#endif
return 0;
}

static DEVICE_API(clock_control, mcux_lpc_syscon_api) = {
.on = mcux_lpc_syscon_clock_control_on,
.off = mcux_lpc_syscon_clock_control_off,
.get_rate = mcux_lpc_syscon_clock_control_get_subsys_rate,
.set_rate = mcux_lpc_syscon_clock_control_set_subsys_rate,
.configure = mcux_lpc_syscon_clock_control_configure,
};

#define LPC_CLOCK_INIT(n) \
Expand Down
1 change: 1 addition & 0 deletions drivers/serial/Kconfig.mcux_flexcomm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ config UART_MCUX_FLEXCOMM
select SERIAL_SUPPORT_INTERRUPT
select SERIAL_SUPPORT_ASYNC if \
DT_HAS_NXP_LPC_DMA_ENABLED
select PM_POLICY_DEVICE_CONSTRAINTS if PM
Copy link
Member

Choose a reason for hiding this comment

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

The title references RW612 where this gets set already if PM is enabled (

zephyr/soc/nxp/rw/Kconfig.defconfig

Line 74 in fbdf4b9

config PM_POLICY_DEVICE_CONSTRAINTS
)

But I'm guessing that since this is shared IP we need this here. We might need to think through a bit this config and why we set it in the RW defconfig... yet still need to set it in the drivers Kconfig

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I put this here because it is not specific to RW, even though RW is the only platform right now in tree that is enabling PM deep enough to disable the UART. But any platform that disables the UART on low power mode will need this dependency to avoid the garbage character.

select DMA if UART_ASYNC_API
select PINCTRL
help
Expand Down
167 changes: 155 additions & 12 deletions drivers/serial/uart_mcux_flexcomm.c
View file
Open in desktop
Copy link
Collaborator

Choose a reason for hiding this comment

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

update copyright year

Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2017, 2022-2023 NXP
* Copyright 2017, 2022-2025 NXP
*
* SPDX-License-Identifier: Apache-2.0
*/
Expand All @@ -15,6 +15,7 @@
#include <zephyr/drivers/uart.h>
#include <zephyr/drivers/clock_control.h>
#include <zephyr/pm/policy.h>
#include <zephyr/pm/pm.h>
#include <zephyr/irq.h>
#include <zephyr/pm/device.h>
#include <fsl_usart.h>
Expand All @@ -26,6 +27,8 @@
#include <fsl_inputmux.h>
#endif

#define FC_UART_IS_WAKEUP (IS_ENABLED(CONFIG_PM) && DT_ANY_INST_HAS_BOOL_STATUS_OKAY(wakeup_source))

#ifdef CONFIG_UART_ASYNC_API
struct mcux_flexcomm_uart_dma_config {
const struct device *dev;
Expand All @@ -51,6 +54,13 @@ struct mcux_flexcomm_config {
void (*rx_timeout_func)(struct k_work *work);
void (*tx_timeout_func)(struct k_work *work);
#endif
#ifdef CONFIG_PM_POLICY_DEVICE_CONSTRAINTS
void (*pm_unlock_work_fn)(struct k_work *);
#endif
#if FC_UART_IS_WAKEUP
void (*wakeup_cfg)(void);
clock_control_subsys_t lp_clock_subsys;
#endif
};

#if CONFIG_UART_ASYNC_API
Expand Down Expand Up @@ -88,8 +98,14 @@ struct mcux_flexcomm_data {
#ifdef CONFIG_UART_USE_RUNTIME_CONFIGURE
struct uart_config uart_config;
#endif
#if FC_UART_IS_WAKEUP
struct pm_notifier pm_handles;
uint16_t old_brg;
uint8_t old_osr;
#endif
#ifdef CONFIG_PM_POLICY_DEVICE_CONSTRAINTS
bool pm_policy_state_lock;
struct k_work pm_lock_work;
#endif
};

Expand All @@ -104,16 +120,30 @@ static void mcux_flexcomm_pm_policy_state_lock_get(const struct device *dev)
}
}

static void mcux_flexcomm_pm_policy_state_lock_put(const struct device *dev)
static void mcux_flexcomm_pm_unlock_if_idle(const struct device *dev)
{
const struct mcux_flexcomm_config *config = dev->config;
struct mcux_flexcomm_data *data = dev->data;

if (data->pm_policy_state_lock) {
if (config->base->STAT & USART_STAT_TXIDLE_MASK) {
data->pm_policy_state_lock = false;
pm_policy_device_power_lock_put(dev);
} else {
/* can't block systemn workqueue so keep re-submitting until it's done */
Copy link
Collaborator

Choose a reason for hiding this comment

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

NIT, typo extra 'n'

k_work_submit(&data->pm_lock_work);
}
}
#endif

static void mcux_flexcomm_pm_policy_state_lock_put(const struct device *dev)
{
struct mcux_flexcomm_data *data = dev->data;

if (data->pm_policy_state_lock) {
/* we can't block on TXidle mask in IRQ context so offload */
k_work_submit(&data->pm_lock_work);
}
}
#endif /* CONFIG_PM_POLICY_DEVICE_CONSTRAINTS */

static int mcux_flexcomm_poll_in(const struct device *dev, unsigned char *c)
{
Expand Down Expand Up @@ -1128,6 +1158,40 @@ static int mcux_flexcomm_init_common(const struct device *dev)
return 0;
}

#if FC_UART_IS_WAKEUP
static void mcux_flexcomm_pm_prepare_wake(const struct device *dev, enum pm_state state)
{
const struct mcux_flexcomm_config *config = dev->config;
struct mcux_flexcomm_data *data = dev->data;
USART_Type *base = config->base;

/* Switch to the lowest possible baud rate, in order to
* both minimize power consumption and also be able to
* potentially wake up the chip from this mode.
*/
if (pm_policy_device_is_disabling_state(dev, state, 0)) {
clock_control_configure(config->clock_dev, config->lp_clock_subsys, NULL);
data->old_brg = base->BRG;
data->old_osr = base->OSR;
base->OSR = 8;
base->BRG = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

How did you decide on these values. A comment would be helpful.

Copy link
Member Author

@decsny decsny May 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

There already is a comment on line 1168-1171 explaining that we are switching to the lowest baud rate.

Copy link
Collaborator

@mmahadevan108 mmahadevan108 May 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

True, but how did you calculate 8 and 0?

Copy link
Member Author

Choose a reason for hiding this comment

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

There is no calculation involved, BRG is the baud rate generator register and I put 0 which is the min, and OSR is the clocks per bit register which I put 8 which is the max

Copy link
Collaborator

Choose a reason for hiding this comment

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

It would help if some of this is added to the comment. Not a blocker.

}
}

static void mcux_flexcomm_pm_restore_wake(const struct device *dev, enum pm_state state)
{
const struct mcux_flexcomm_config *config = dev->config;
struct mcux_flexcomm_data *data = dev->data;
USART_Type *base = config->base;

if (pm_policy_device_is_disabling_state(dev, state, 0)) {
clock_control_configure(config->clock_dev, config->clock_subsys, NULL);
base->OSR = data->old_osr;
base->BRG = data->old_brg;
}
}
#endif /* FC_UART_IS_WAKEUP */

static uint32_t usart_intenset;
static int mcux_flexcomm_pm_action(const struct device *dev, enum pm_device_action action)
{
Expand Down Expand Up @@ -1157,6 +1221,20 @@ static int mcux_flexcomm_pm_action(const struct device *dev, enum pm_device_acti

static int mcux_flexcomm_init(const struct device *dev)
{
#if FC_UART_IS_WAKEUP || defined(CONFIG_PM_POLICY_DEVICE_CONSTRAINTS)
const struct mcux_flexcomm_config *config = dev->config;
struct mcux_flexcomm_data *data = dev->data;
#endif

#if defined(CONFIG_PM_POLICY_DEVICE_CONSTRAINTS)
k_work_init(&data->pm_lock_work, config->pm_unlock_work_fn);
#endif

#if FC_UART_IS_WAKEUP
config->wakeup_cfg();
pm_notifier_register(&data->pm_handles);
#endif

/* Rest of the init is done from the PM_DEVICE_TURN_ON action
* which is invoked by pm_device_driver_init().
*/
Expand Down Expand Up @@ -1215,6 +1293,21 @@ static DEVICE_API(uart, mcux_flexcomm_driver_api) = {
#define UART_MCUX_FLEXCOMM_IRQ_CFG_FUNC_INIT(n)
#endif /* CONFIG_UART_MCUX_FLEXCOMM_ISR_SUPPORT */

#ifdef CONFIG_PM_POLICY_DEVICE_CONSTRAINTS
#define UART_MCUX_FLEXCOMM_PM_UNLOCK_FUNC_DEFINE(n) \
static void mcux_flexcomm_##n##_pm_unlock(struct k_work *work) \
{ \
const struct device *dev = DEVICE_DT_INST_GET(n); \
\
mcux_flexcomm_pm_unlock_if_idle(dev); \
}
#define UART_MCUX_FLEXCOMM_PM_UNLOCK_FUNC_BIND(n) \
.pm_unlock_work_fn = mcux_flexcomm_##n##_pm_unlock,
#else
#define UART_MCUX_FLEXCOMM_PM_UNLOCK_FUNC_DEFINE(n)
#define UART_MCUX_FLEXCOMM_PM_UNLOCK_FUNC_BIND(n)
#endif /* CONFIG_PM_POLICY_DEVICE_CONSTRAINTS */

#ifdef CONFIG_UART_ASYNC_API
#define UART_MCUX_FLEXCOMM_TX_TIMEOUT_FUNC(n) \
static void mcux_flexcomm_uart_##n##_tx_timeout(struct k_work *work) \
Expand Down Expand Up @@ -1277,6 +1370,47 @@ DT_INST_FOREACH_STATUS_OKAY(UART_MCUX_FLEXCOMM_RX_TIMEOUT_FUNC);
#define UART_MCUX_FLEXCOMM_ASYNC_CFG(n)
#endif /* CONFIG_UART_ASYNC_API */

#if FC_UART_IS_WAKEUP
#define UART_MCUX_FLEXCOMM_WAKEUP_CFG_DEFINE(n) \
static void serial_mcux_flexcomm_##n##_wakeup_cfg(void) \
{ \
IF_ENABLED(DT_INST_PROP(n, wakeup_source), ( \
POWER_EnableWakeup(DT_INST_IRQN(n)); \
)) \
}
#define UART_MCUX_FLEXCOMM_WAKEUP_CFG_BIND(n) \
.wakeup_cfg = serial_mcux_flexcomm_##n##_wakeup_cfg,

#define UART_MCUX_FLEXCOMM_PM_HANDLES_DEFINE(n) \
static void serial_mcux_flexcomm_##n##_pm_entry(enum pm_state state) \
{ \
IF_ENABLED(DT_INST_PROP(n, wakeup_source), ( \
mcux_flexcomm_pm_prepare_wake(DEVICE_DT_INST_GET(n), state); \
)) \
} \
\
static void serial_mcux_flexcomm_##n##_pm_exit(enum pm_state state) \
{ \
IF_ENABLED(DT_INST_PROP(n, wakeup_source), ( \
mcux_flexcomm_pm_restore_wake(DEVICE_DT_INST_GET(n), state); \
)) \
}
#define UART_MCUX_FLEXCOMM_PM_HANDLES_BIND(n) \
.pm_handles = { \
.state_entry = serial_mcux_flexcomm_##n##_pm_entry, \
.state_exit = serial_mcux_flexcomm_##n##_pm_exit, \
},
#define UART_MCUX_FLEXCOMM_LP_CLK_SUBSYS(n) \
.lp_clock_subsys = (clock_control_subsys_t) \
DT_INST_CLOCKS_CELL_BY_NAME(n, sleep, name),
#else
#define UART_MCUX_FLEXCOMM_WAKEUP_CFG_DEFINE(n)
#define UART_MCUX_FLEXCOMM_WAKEUP_CFG_BIND(n)
#define UART_MCUX_FLEXCOMM_PM_HANDLES_DEFINE(n)
#define UART_MCUX_FLEXCOMM_PM_HANDLES_BIND(n)
#define UART_MCUX_FLEXCOMM_LP_CLK_SUBSYS(n)
#endif /* FC_UART_IS_WAKEUP */

#define UART_MCUX_FLEXCOMM_INIT_CFG(n) \
static const struct mcux_flexcomm_config mcux_flexcomm_##n##_config = { \
.base = (USART_Type *)DT_INST_REG_ADDR(n), \
Expand All @@ -1288,17 +1422,30 @@ static const struct mcux_flexcomm_config mcux_flexcomm_##n##_config = { \
.pincfg = PINCTRL_DT_INST_DEV_CONFIG_GET(n), \
UART_MCUX_FLEXCOMM_IRQ_CFG_FUNC_INIT(n) \
UART_MCUX_FLEXCOMM_ASYNC_CFG(n) \
UART_MCUX_FLEXCOMM_PM_UNLOCK_FUNC_BIND(n) \
UART_MCUX_FLEXCOMM_WAKEUP_CFG_BIND(n) \
UART_MCUX_FLEXCOMM_LP_CLK_SUBSYS(n) \
};

#define UART_MCUX_FLEXCOMM_INIT_DATA(n) \
static struct mcux_flexcomm_data mcux_flexcomm_##n##_data = { \
UART_MCUX_FLEXCOMM_PM_HANDLES_BIND(n) \
};

#define UART_MCUX_FLEXCOMM_INIT(n) \
\
PINCTRL_DT_INST_DEFINE(n); \
\
static struct mcux_flexcomm_data mcux_flexcomm_##n##_data; \
UART_MCUX_FLEXCOMM_PM_UNLOCK_FUNC_DEFINE(n) \
UART_MCUX_FLEXCOMM_WAKEUP_CFG_DEFINE(n) \
UART_MCUX_FLEXCOMM_PM_HANDLES_DEFINE(n) \
PM_DEVICE_DT_INST_DEFINE(n, mcux_flexcomm_pm_action); \
\
static const struct mcux_flexcomm_config mcux_flexcomm_##n##_config; \
UART_MCUX_FLEXCOMM_INIT_DATA(n) \
\
PM_DEVICE_DT_INST_DEFINE(n, mcux_flexcomm_pm_action); \
UART_MCUX_FLEXCOMM_IRQ_CFG_FUNC(n) \
\
UART_MCUX_FLEXCOMM_INIT_CFG(n) \
\
DEVICE_DT_INST_DEFINE(n, \
&mcux_flexcomm_init, \
Expand All @@ -1307,10 +1454,6 @@ static const struct mcux_flexcomm_config mcux_flexcomm_##n##_config = { \
&mcux_flexcomm_##n##_config, \
PRE_KERNEL_1, \
CONFIG_SERIAL_INIT_PRIORITY, \
&mcux_flexcomm_driver_api); \
\
UART_MCUX_FLEXCOMM_IRQ_CFG_FUNC(n) \
\
UART_MCUX_FLEXCOMM_INIT_CFG(n)
&mcux_flexcomm_driver_api);

DT_INST_FOREACH_STATUS_OKAY(UART_MCUX_FLEXCOMM_INIT)
12 changes: 8 additions & 4 deletions dts/arm/nxp/nxp_rw6xx_common.dtsi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,8 @@
compatible = "nxp,lpc-flexcomm";
reg = <0x106000 0x1000>;
interrupts = <14 0>;
clocks = <&clkctl1 MCUX_FLEXCOMM0_CLK>;
clocks = <&clkctl1 MCUX_FLEXCOMM0_CLK &clkctl1 MCUX_FLEXCOMM0_LP_CLK>;
clock-names = "default", "sleep";
resets = <&rstctl1 NXP_SYSCON_RESET(0, 8)>;
dmas = <&dma0 0>, <&dma0 1>;
dma-names = "rx", "tx";
Expand All @@ -258,7 +259,8 @@
compatible = "nxp,lpc-flexcomm";
reg = <0x107000 0x1000>;
interrupts = <15 0>;
clocks = <&clkctl1 MCUX_FLEXCOMM1_CLK>;
clocks = <&clkctl1 MCUX_FLEXCOMM1_CLK &clkctl1 MCUX_FLEXCOMM1_LP_CLK>;
clock-names = "default", "sleep";
resets = <&rstctl1 NXP_SYSCON_RESET(0, 9)>;
dmas = <&dma0 2>, <&dma0 3>;
dma-names = "rx", "tx";
Expand All @@ -271,7 +273,8 @@
compatible = "nxp,lpc-flexcomm";
reg = <0x108000 0x1000>;
interrupts = <16 0>;
clocks = <&clkctl1 MCUX_FLEXCOMM2_CLK>;
clocks = <&clkctl1 MCUX_FLEXCOMM2_CLK &clkctl1 MCUX_FLEXCOMM2_LP_CLK>;
clock-names = "default", "sleep";
resets = <&rstctl1 NXP_SYSCON_RESET(0, 10)>;
dmas = <&dma0 4>, <&dma0 5>;
dma-names = "rx", "tx";
Expand All @@ -284,7 +287,8 @@
compatible = "nxp,lpc-flexcomm";
reg = <0x109000 0x1000>;
interrupts = <17 0>;
clocks = <&clkctl1 MCUX_FLEXCOMM3_CLK>;
clocks = <&clkctl1 MCUX_FLEXCOMM3_CLK &clkctl1 MCUX_FLEXCOMM3_LP_CLK>;
clock-names = "default", "sleep";
resets = <&rstctl1 NXP_SYSCON_RESET(0, 11)>;
dmas = <&dma0 6>, <&dma0 7>;
dma-names = "rx", "tx";
Expand Down
4 changes: 4 additions & 0 deletions include/zephyr/dt-bindings/clock/mcux_lpc_syscon_clock.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@
#define MCUX_FLEXCOMM2_CLK MCUX_LPC_CLK_ID(0x01, 0x02)
#define MCUX_FLEXCOMM3_CLK MCUX_LPC_CLK_ID(0x01, 0x03)
#define MCUX_FLEXCOMM4_CLK MCUX_LPC_CLK_ID(0x01, 0x04)
#define MCUX_FLEXCOMM0_LP_CLK MCUX_LPC_CLK_ID(0x01, 0x80)
#define MCUX_FLEXCOMM1_LP_CLK MCUX_LPC_CLK_ID(0x01, 0x81)
#define MCUX_FLEXCOMM2_LP_CLK MCUX_LPC_CLK_ID(0x01, 0x82)
#define MCUX_FLEXCOMM3_LP_CLK MCUX_LPC_CLK_ID(0x01, 0x83)
#define MCUX_FLEXCOMM5_CLK MCUX_LPC_CLK_ID(0x01, 0x05)
#define MCUX_FLEXCOMM6_CLK MCUX_LPC_CLK_ID(0x01, 0x06)
#define MCUX_FLEXCOMM7_CLK MCUX_LPC_CLK_ID(0x01, 0x07)
Expand Down
Loading