Skip to content

Conversation

@Yusmed
Copy link
Contributor

@Yusmed Yusmed commented Sep 27, 2022

adding pm support

Copy link
Member

Choose a reason for hiding this comment

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

you are defining it but not binding to the driver instance. Define it inside the UART_MCUX_INIT loop and bind it to the instance. Basically, on DEVICE_DT_INST_DEFINE instead of NULL you pass PM_DEVICE_DT_INST_GET(n)

@ceolin ceolin self-requested a review September 29, 2022 01:01
ceolin
ceolin previously requested changes Sep 29, 2022
Copy link
Member

@ceolin ceolin left a comment

Choose a reason for hiding this comment

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

Thanks a lot to send it ! Please address the comments and remove the merge commit. You just need the commit with your change.

Copy link
Member

Choose a reason for hiding this comment

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

@mmahadevan108 can you check it please ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Minor issue, please fix the indentation of the break commands

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this bit keeps the UART clock running during low power modes. Should this be something the user gets to decide through a device tree property?

Copy link
Contributor

Choose a reason for hiding this comment

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

Another point is that we don't need to clear and enable this bit in suspend and resume. This is a one time operation where the user decides if the UART clock should be active in low power modes.

@Yusmed Yusmed force-pushed the Yusuf-Uart-Branch branch 2 times, most recently from b3fc1c9 to 526aa1a Compare September 30, 2022 01:04
@Yusmed Yusmed force-pushed the Yusuf-Uart-Branch branch from 526aa1a to b3b6240 Compare October 17, 2022 17:39
Copy link
Member

Choose a reason for hiding this comment

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

that is not correct, what you have to do is;

#define UART_MCUX_INIT(n)						\
	PINCTRL_DEFINE(n)						\
									\
	static struct uart_mcux_data uart_mcux_##n##_data = {		\
		.uart_cfg = {						\
			.stop_bits = UART_CFG_STOP_BITS_1,		\
			.data_bits = UART_CFG_DATA_BITS_8,		\
			.baudrate  = DT_INST_PROP(n, current_speed),	\
			.parity    = UART_CFG_PARITY_NONE,		\
			.flow_ctrl = DT_INST_PROP(n, hw_flow_control) ?	\
				UART_CFG_FLOW_CTRL_RTS_CTS : UART_CFG_FLOW_CTRL_NONE,\
		},							\
	};								\
									\
	static const struct uart_mcux_config uart_mcux_##n##_config;	\
									\
        PM_DEVICE_DT_INST_DEFINE(n, uart_mcux_pm_action);\
	DEVICE_DT_INST_DEFINE(n,					\
			    &uart_mcux_init,				\
			    PM_DEVICE_DT_INST_GET(n),					\
			    &uart_mcux_##n##_data,			\
			    &uart_mcux_##n##_config,			\
			    PRE_KERNEL_1,				\
			    CONFIG_SERIAL_INIT_PRIORITY,		\
			    &uart_mcux_driver_api);			\
									\
	UART_MCUX_CONFIG_FUNC(n)					\
									\
	UART_MCUX_INIT_CFG(n);

DT_INST_FOREACH_STATUS_OKAY(UART_MCUX_INIT)

@Yusmed Yusmed force-pushed the Yusuf-Uart-Branch branch 4 times, most recently from 737cb97 to c5e1506 Compare October 19, 2022 20:29
@Yusmed Yusmed force-pushed the Yusuf-Uart-Branch branch from c5e1506 to 62ff477 Compare October 24, 2022 19:47
@Yusmed Yusmed marked this pull request as ready for review October 26, 2022 20:08
@zephyrbot zephyrbot added area: UART Universal Asynchronous Receiver-Transmitter platform: NXP NXP labels Oct 26, 2022
@Yusmed Yusmed force-pushed the Yusuf-Uart-Branch branch from 62ff477 to 1f7a893 Compare November 2, 2022 19:57
@Yusmed Yusmed force-pushed the Yusuf-Uart-Branch branch 2 times, most recently from 118a0cd to fcad698 Compare December 7, 2022 01:42
@Yusmed Yusmed force-pushed the Yusuf-Uart-Branch branch from 293d50b to 2149dc9 Compare January 23, 2023 21:34
dcpleung
dcpleung previously approved these changes Jan 23, 2023
Copy link
Contributor

@edersondisouza edersondisouza left a comment

Choose a reason for hiding this comment

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

The commit message doesn't make much sense. The bit " Using bit mask instead of individual bit
manipulation" appears to belong to an update of the PR. Just state what the patch does and any noteworthy stuff, if needed (like, stating that PM for this device means gating the clock for it).

edersondisouza
edersondisouza previously approved these changes Jan 24, 2023
dcpleung
dcpleung previously approved these changes Jan 24, 2023
Adding PM support to uart_mcux by gating clock and disabling transmitter

Signed-off-by: Muhammed Ahmed <[email protected]>
@stephanosio stephanosio added this to the v3.4.0 milestone Feb 1, 2023
@nashif nashif merged commit 3f0fc7f into zephyrproject-rtos:main Feb 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: UART Universal Asynchronous Receiver-Transmitter platform: NXP NXP

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants