drivers: uart: Add low power uart module #2591

nordic-krch · 2020-07-15T07:15:37Z

Low Power UART is a driver which implements standard asynchronous UART API and uses two control lines (instead
of standard hardware flow control lines) protocol to allow disabling UART receiver during idle period. This allows achieving low power consumption since high frequency clock (required when UART receiver is enabled) can be shut down when UART is in idle.

PR provides driver and sample.

See documentation for operation details.

I'm planning to add test in additional PR.

Primary use case is to use it for HCI communication between nrf52 and nrf91.

NordicBuilder · 2020-07-15T07:19:17Z

All checks are passing now.

checkpatch (informational only, not a failure)

No additional types will be considered - file &#x27;scripts/checkpatch/typedefsfile&#x27;: No such file or directory
-:842: WARNING:LONG_LINE: line over 80 characters
#842: FILE: drivers/uart/uart_nrf_lpuart.c:633:
+			(DT_INST_PROP(0, req_pin) - 32) : DT_INST_PROP(0, req_pin),

-:844: WARNING:LONG_LINE: line over 80 characters
#844: FILE: drivers/uart/uart_nrf_lpuart.c:635:
+			(DT_INST_PROP(0, rdy_pin) - 32) : DT_INST_PROP(0, rdy_pin)

- total: 0 errors, 2 warnings, 1115 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

Your patch has style problems, please review.

NOTE: Ignored message types: AVOID_EXTERNS BRACES CONFIG_EXPERIMENTAL CONST_STRUCT DATE_TIME FILE_PATH_CHANGES MINMAX MULTISTATEMENT_MACRO_USE_DO_WHILE NETWORKING_BLOCK_COMMENT_STYLE PRINTK_WITHOUT_KERN_LEVEL SPDX_LICENSE_TAG SPLIT_STRING VOLATILE

NOTE: If any of the errors are false positives, please report
      them to the maintainers.

Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages.

Mierunski · 2020-07-17T07:06:11Z

drivers/uart/Kconfig

+config NRF_LPUART_REQ_PIN
+	int "Transfer request pin"
+	default 0
+
+config NRF_LPUART_RDY_PIN
+	int "Receiver ready pin"
+	default 1


Why not define those pins in dts?

that's where border between kconfig is unclear. I could create something build on top or uart as a bus. As for now, ncs has only dts/bindings/sensor and dts/bindings/net and lpuart does not fit there. I could add something like dts/bindings/serial/nordic,nrf-lpuart.yaml but then it suggests as if that is a hw peripheral. What if some next gen SoC gets low power uart?

@anangl @Mierunski what do you think?

software pwm is defined in nordic,nrf-sw-pwm.yaml, how about adding nordic,nrf-sw-lpuart.yaml which would extend standard uart?

moved to DT

Mierunski · 2020-07-17T07:12:00Z

samples/peripheral/lpuart/README.rst

+Overview
+********
+
+Sample implements simple loopback using single UART instance.Sample by default


Missing space

Mierunski · 2020-07-17T07:12:40Z

samples/peripheral/lpuart/README.rst

+  * |nRF52840DK|
+  * |nRF9160DK|
+
+* Pins shortend


Shorted or connected, same below in line 34

Mierunski · 2020-07-17T07:43:49Z

drivers/uart/uart_nrf_lpuart.c

+	data->rx_state = RX_ACTIVE;
+}
+
+/* Called when end of transfer is detected. It sets resposnse pin to idle and


carlescufi · 2020-07-17T11:25:29Z

@nordic-krch this seems to use the standard UART API, so is there a reason why this is downstream?

carlescufi · 2020-07-17T11:25:41Z

@anangl can you take a look please?

nordic-krch · 2020-07-20T09:00:49Z

@carlescufi It is not standard uart (custom pin protocol) and multiple customers is asking for this and zephyr means longer review and more time till it is supported by ncs (upmerge). Because of that I put it here but there is no long term obstacles. I can as well move downstram (now or later).

nordic-krch · 2020-07-20T14:03:31Z

@greg-fer can you take o look or add other doc reviewer?

samples/peripheral/lpuart/CMakeLists.txt

samples/peripheral/lpuart/boards/nrf9160dk_nrf9160.overlay

samples/peripheral/lpuart/sample.yaml

doc/nrf/drivers/uart_nrf_lpuart.rst

samples/peripheral/lpuart/README.rst

doc/nrf/drivers/uart_nrf_lpuart.rst

nordic-krch · 2020-09-04T05:38:34Z

@anangl thanks for comments, please recheck.

anangl · 2020-09-04T06:06:17Z

samples/peripheral/lpuart/README.rst

+   :start-after: set18_start
+   :end-before: set18_end


Shouldn't these two be set22_start and set22_end, respectively?

well spotted. fixed

anangl · 2020-09-04T06:19:30Z

One more detail: the commit title "drivers: uart: Add low power uart module" should actually start with "drivers: serial:".

anangl · 2020-09-04T08:42:51Z

drivers/serial/uart_nrf_sw_lpuart.c

+#define GPIO(id) DT_NODELABEL(gpio##id)
+
+#define GET_PORT(pin_prop) \
+	COND_CODE_1(DT_NODE_EXISTS(DT_INST(1, nordic_nrf_gpio)), \


Suggested change

COND_CODE_1(DT_NODE_EXISTS(DT_INST(1, nordic_nrf_gpio)), \

COND_CODE_1(DT_NODE_EXISTS(GPIO(1)), \

anangl · 2020-09-04T08:43:21Z

drivers/serial/uart_nrf_sw_lpuart.c

+		.pin = (DT_INST_PROP(0, pin_prop) >= 32) ? \
+		 (DT_INST_PROP(0, pin_prop) - 32) : DT_INST_PROP(0, pin_prop), \


Suggested change

.pin = (DT_INST_PROP(0, pin_prop) >= 32) ? \

(DT_INST_PROP(0, pin_prop) - 32) : DT_INST_PROP(0, pin_prop), \

.pin = (DT_INST_PROP(0, pin_prop) & 0x1F), \

nordic-krch · 2020-09-07T11:05:22Z

@utzig can you review changes in doc/nrf/conf.py?

utzig

LGTM

On vacation. Comments addressed

nordic-krch · 2020-09-09T05:10:23Z

@rlubos @tejlmand maybe you are more familiar with CI failure analysis but i can't find failures related to that PR. If there are no failures related could you merge as further work is pending on that for days already.

rlubos · 2020-09-09T15:16:37Z

Apart from all of the recent wackiness of the CI, this particular error in the previous doc build looks like a valid one:

AssertionError: There were warnings in: nrf/doc.log

https://jenkins-ncs.nordicsemi.no/job/latest/job/sub/job/test-fw-nrfconnect-nrf_doc/job/master/4664/artifact/_build/nrf/doc.log

But we need some doc expert to interpret this.

nordic-krch · 2020-09-09T15:47:27Z

it states:

../../test_doc/_build/nrf/rst/doc/nrf/includes/boardname_tables/sample_boardnames.txt:13: WARNING: Inline literal start-string without end-string.

But this PR does not modify line 13 of this file and line 13 looks perfectly normal
@greg-fer @FrancescoSer @ru-fu any ideas?

Now it is again:

process apparently never started in /jenkins_workspace/workspace/t-fw-nrfconnect-nrf_doc_master_2/test_doc/DOC@tmp/durable-909d16d7

b-gent · 2020-09-09T22:51:52Z

it states:
../../test_doc/_build/nrf/rst/doc/nrf/includes/boardname_tables/sample_boardnames.txt:13: WARNING: Inline literal start-string without end-string.
But this PR does not modify line 13 of this file and line 13 looks perfectly normal
@greg-fer @FrancescoSer @ru-fu any ideas?

Now it is again:
process apparently never started in /jenkins_workspace/workspace/t-fw-nrfconnect-nrf_doc_master_2/test_doc/DOC@tmp/durable-909d16d7

There is a backquote missing here in line 322 of this file:
``nrf5340pdk_nrf5340_cpuapp`

Low power UART implements UART asynchronous API and is using 2 additional pins to control the transfer. Based on pin status receiver can be disabled during idle phase. Signed-off-by: Krzysztof Chruscinski <krzysztof.chruscinski@nordicsemi.no>

Added documentation for low power UART module. Signed-off-by: Krzysztof Chruscinski <krzysztof.chruscinski@nordicsemi.no>

Sample application used to sanity check low power uart and measure power consumption. Signed-off-by: Krzysztof Chruscinski <krzysztof.chruscinski@nordicsemi.no>

Added nordic-krch as sample codeowner. Signed-off-by: Krzysztof Chruscinski <krzysztof.chruscinski@nordicsemi.no>

Adding DTS lexer. Signed-off-by: Krzysztof Chruscinski <krzysztof.chruscinski@nordicsemi.no>

nordic-krch · 2020-09-10T05:17:54Z

@b-gent thanks for pointing it out.

rlubos · 2020-09-10T07:19:55Z

The CI failures are unrelated to this PR, merging.

nordic-krch requested a review from anangl July 15, 2020 07:15

nordic-krch requested a review from carlescufi as a code owner July 15, 2020 07:15

nordic-krch requested a review from Mierunski July 15, 2020 07:15

nordic-krch mentioned this pull request Jul 15, 2020

From tree changes required by low power UART (#2591) nrfconnect/sdk-zephyr#332

Closed

nordic-krch requested a review from tejlmand as a code owner July 15, 2020 07:32

nordic-krch force-pushed the lpuart branch 2 times, most recently from 7b8545c to eeee391 Compare July 15, 2020 07:54

Mierunski suggested changes Jul 17, 2020

View reviewed changes

nordic-krch force-pushed the lpuart branch 2 times, most recently from f507070 to c5721ae Compare July 20, 2020 14:02

b-gent added the doc-required PR must not be merged without tech writer approval. label Aug 6, 2020

b-gent requested review from b-gent and FrancescoSer August 6, 2020 12:32

tejlmand requested changes Aug 7, 2020

View reviewed changes

samples/peripheral/lpuart/CMakeLists.txt Outdated Show resolved Hide resolved

samples/peripheral/lpuart/boards/nrf9160dk_nrf9160.overlay Show resolved Hide resolved

samples/peripheral/lpuart/sample.yaml Outdated Show resolved Hide resolved

FrancescoSer force-pushed the lpuart branch 2 times, most recently from 6f1b8bb to cabc866 Compare August 10, 2020 13:31

b-gent requested changes Aug 12, 2020

View reviewed changes

FrancescoSer force-pushed the lpuart branch from cabc866 to cdb36bb Compare August 13, 2020 15:39

FrancescoSer approved these changes Aug 18, 2020

View reviewed changes

Mierunski approved these changes Aug 19, 2020

View reviewed changes

b-gent previously requested changes Aug 19, 2020

View reviewed changes

doc/nrf/drivers/uart_nrf_lpuart.rst Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

FrancescoSer force-pushed the lpuart branch from cdb36bb to 2efe694 Compare August 21, 2020 13:15

nordic-krch force-pushed the lpuart branch 2 times, most recently from 1871457 to ec57439 Compare September 1, 2020 11:43

anangl reviewed Sep 4, 2020

View reviewed changes

nordic-krch force-pushed the lpuart branch from 09b90d1 to c26e55a Compare September 4, 2020 06:10

anangl approved these changes Sep 4, 2020

View reviewed changes

nordic-krch force-pushed the lpuart branch 3 times, most recently from c9b08a2 to 63e66fa Compare September 4, 2020 08:31

anangl reviewed Sep 4, 2020

View reviewed changes

nordic-krch force-pushed the lpuart branch 2 times, most recently from a09497e to 6d7cdbf Compare September 7, 2020 05:42

utzig approved these changes Sep 7, 2020

View reviewed changes

nordic-krch force-pushed the lpuart branch 2 times, most recently from c044bae to a2a5c69 Compare September 8, 2020 11:40

nordic-krch force-pushed the lpuart branch from a2a5c69 to f414ec5 Compare September 9, 2020 07:07

nordic-krch added 5 commits September 10, 2020 07:16

drivers: serial: Add low power uart module

0e94f9f

Low power UART implements UART asynchronous API and is using 2 additional pins to control the transfer. Based on pin status receiver can be disabled during idle phase. Signed-off-by: Krzysztof Chruscinski <krzysztof.chruscinski@nordicsemi.no>

doc: nrf: drivers: Add low power UART documentation

5c0c0ed

Added documentation for low power UART module. Signed-off-by: Krzysztof Chruscinski <krzysztof.chruscinski@nordicsemi.no>

samples: peripheral: Add low power uart sample

596eb65

Sample application used to sanity check low power uart and measure power consumption. Signed-off-by: Krzysztof Chruscinski <krzysztof.chruscinski@nordicsemi.no>

CODEOWNERS: Add samples/peripheral/lpuart codeowner

d89f255

Added nordic-krch as sample codeowner. Signed-off-by: Krzysztof Chruscinski <krzysztof.chruscinski@nordicsemi.no>

doc: nrf: conf.py: Add DTS lexer

e68ed60

Adding DTS lexer. Signed-off-by: Krzysztof Chruscinski <krzysztof.chruscinski@nordicsemi.no>

nordic-krch force-pushed the lpuart branch from f414ec5 to e68ed60 Compare September 10, 2020 05:17

rlubos merged commit b31ecab into nrfconnect:master Sep 10, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

drivers: uart: Add low power uart module #2591

drivers: uart: Add low power uart module #2591

nordic-krch commented Jul 15, 2020

NordicBuilder commented Jul 15, 2020 •

edited

Mierunski Jul 17, 2020

nordic-krch Jul 20, 2020

Mierunski Jul 20, 2020

nordic-krch Jul 20, 2020

Mierunski Jul 17, 2020

nordic-krch Jul 20, 2020

Mierunski Jul 17, 2020

nordic-krch Jul 20, 2020

Mierunski Jul 17, 2020

nordic-krch Jul 20, 2020

carlescufi commented Jul 17, 2020

carlescufi commented Jul 17, 2020

nordic-krch commented Jul 20, 2020

nordic-krch commented Jul 20, 2020

This comment has been minimized.

nordic-krch commented Sep 4, 2020

anangl Sep 4, 2020

nordic-krch Sep 4, 2020

anangl commented Sep 4, 2020

anangl Sep 4, 2020

anangl Sep 4, 2020

nordic-krch commented Sep 7, 2020

utzig left a comment

nordic-krch commented Sep 9, 2020

rlubos commented Sep 9, 2020

nordic-krch commented Sep 9, 2020

b-gent commented Sep 9, 2020

nordic-krch commented Sep 10, 2020

rlubos commented Sep 10, 2020

	COND_CODE_1(DT_NODE_EXISTS(DT_INST(1, nordic_nrf_gpio)), \
	COND_CODE_1(DT_NODE_EXISTS(GPIO(1)), \

		.pin = (DT_INST_PROP(0, pin_prop) >= 32) ? \
		(DT_INST_PROP(0, pin_prop) - 32) : DT_INST_PROP(0, pin_prop), \

	.pin = (DT_INST_PROP(0, pin_prop) >= 32) ? \
	(DT_INST_PROP(0, pin_prop) - 32) : DT_INST_PROP(0, pin_prop), \
	.pin = (DT_INST_PROP(0, pin_prop) & 0x1F), \

drivers: uart: Add low power uart module #2591

drivers: uart: Add low power uart module #2591

Conversation

nordic-krch commented Jul 15, 2020

NordicBuilder commented Jul 15, 2020 • edited

checkpatch (informational only, not a failure)

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carlescufi commented Jul 17, 2020

carlescufi commented Jul 17, 2020

nordic-krch commented Jul 20, 2020

nordic-krch commented Jul 20, 2020

This comment has been minimized.

nordic-krch commented Sep 4, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anangl commented Sep 4, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nordic-krch commented Sep 7, 2020

utzig left a comment

Choose a reason for hiding this comment

nordic-krch commented Sep 9, 2020

rlubos commented Sep 9, 2020

nordic-krch commented Sep 9, 2020

b-gent commented Sep 9, 2020

nordic-krch commented Sep 10, 2020

rlubos commented Sep 10, 2020

NordicBuilder commented Jul 15, 2020 •

edited