Skip to content

Fixup NRF device tree support for UART & UARTE #8761

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

Merged
merged 2 commits into from
Jul 10, 2018

Conversation

galak
Copy link
Collaborator

@galak galak commented Jul 5, 2018

NRF related UART device tree fixes.

galak added 2 commits July 5, 2018 10:28
If we select UART0 as UARTE we will get different defines from the
generation script.  Support both UART and UARTE for UART0.  Also fixup
UART1 defines since this will always be UARTE.

Signed-off-by: Kumar Gala <[email protected]>
Since the UART0 @ 0x40002000 can either be UART or UARTE the user of the
soc.dtsi needs to select either compatible = "nordic,nrf-uarte" or
"nordic,nrf-uart"

Signed-off-by: Kumar Gala <[email protected]>
@carlescufi
Copy link
Member

cc @barsok

@@ -39,7 +39,8 @@

soc {
uart0: uart@40002000 {
compatible = "nordic,nrf-uarte", "nordic,nrf-uart";
/* uart can be either UART or UARTE, for the user to pick */
/* compatible = "nordic,nrf-uarte" or "nordic,nrf-uart"; */
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 it be defined a default compatible?
Does this compile?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@erwango it forces the .dts file that includes this .dtsi to set it explicitly. I'm guessing that it will compile, but might get a warning.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@erwango Checked. If the user forgets to set compatible string, everything compiles without warning, then no labels in generated...h are produced for the UART. Shound't this change involve modifying dts to h generator?

Copy link
Member

Choose a reason for hiding this comment

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

@galak ^^

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

hmm, let me look, wonder how that even works w/o a compatible.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@barsok what do you mean forget to set the compatible string. For example if I do the following for 96b_nitrogen:

diff --git a/boards/arm/96b_nitrogen/96b_nitrogen.dts b/boards/arm/96b_nitrogen/96b_nitrogen.dts
index 752b40a41f..2c06f489ea 100644
--- a/boards/arm/96b_nitrogen/96b_nitrogen.dts
+++ b/boards/arm/96b_nitrogen/96b_nitrogen.dts
@@ -28,7 +28,6 @@
 };
 
 &uart0 {
-       compatible = "nordic,nrf-uart";
        current-speed = <115200>;
        status = "ok";
 };

I get the following when trying to build hello_world:

/home/galak/git/zephyr/drivers/serial/uart_nrfx_uart.c: In function ‘uart_nrfx_init’:
/home/galak/git/zephyr/samples/hello_world/build/zephyr/include/generated/generated_dts_board.h:101:34: error: ‘NORDIC_NRF_UART_40002000_CURRENT_SPEED’ undeclared (first use in this function)
 #define CONFIG_UART_0_BAUD_RATE  NORDIC_NRF_UART_40002000_CURRENT_SPEED
                                  ^
/home/galak/git/zephyr/samples/hello_world/build/zephyr/include/generated/generated_dts_board.h:101:34: note: in definition of macro ‘CONFIG_UART_0_BAUD_RATE’
 #define CONFIG_UART_0_BAUD_RATE  NORDIC_NRF_UART_40002000_CURRENT_SPEED
                                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/galak/git/zephyr/samples/hello_world/build/zephyr/include/generated/generated_dts_board.h:101:34: note: each undeclared identifier is reported only once for each function it appears in
 #define CONFIG_UART_0_BAUD_RATE  NORDIC_NRF_UART_40002000_CURRENT_SPEED
                                  ^
/home/galak/git/zephyr/samples/hello_world/build/zephyr/include/generated/generated_dts_board.h:101:34: note: in definition of macro ‘CONFIG_UART_0_BAUD_RATE’
 #define CONFIG_UART_0_BAUD_RATE  NORDIC_NRF_UART_40002000_CURRENT_SPEED
                                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /home/galak/git/zephyr/include/uart.h:29:0,
                 from /home/galak/git/zephyr/drivers/serial/uart_nrfx_uart.c:11:
/home/galak/git/zephyr/drivers/serial/uart_nrfx_uart.c: At top level:
/home/galak/git/zephyr/samples/hello_world/build/zephyr/include/generated/generated_dts_board.h:102:29: error: ‘NORDIC_NRF_UART_40002000_LABEL’ undeclared here (not in a function)
 #define CONFIG_UART_0_NAME  NORDIC_NRF_UART_40002000_LABEL
                             ^
/home/galak/git/zephyr/include/device.h:106:11: note: in definition of macro ‘DEVICE_AND_API_INIT’
   .name = drv_name, .init = (init_fn),     \
           ^~~~~~~~
/home/galak/git/zephyr/drivers/serial/uart_nrfx_uart.c:475:7: note: in expansion of macro ‘CONFIG_UART_0_NAME’
       CONFIG_UART_0_NAME,
       ^~~~~~~~~~~~~~~~~~
make[2]: *** [zephyr/drivers/serial/CMakeFiles/drivers__serial.dir/build.make:63: zephyr/drivers/serial/CMakeFiles/drivers__serial.dir/uart_nrfx_uart.c.obj] Error 1
make[1]: *** [CMakeFiles/Makefile2:2168: zephyr/drivers/serial/CMakeFiles/drivers__serial.dir/all] Error 2
make: *** [Makefile:84: all] Error 2

@codecov-io
Copy link

Codecov Report

Merging #8761 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #8761   +/-   ##
=======================================
  Coverage   52.36%   52.36%           
=======================================
  Files         195      195           
  Lines       24713    24713           
  Branches     5136     5136           
=======================================
  Hits        12940    12940           
  Misses       9699     9699           
  Partials     2074     2074

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6c0d089...bf5b7bd. Read the comment docs.

@@ -40,7 +40,8 @@

soc {
uart0: uart@40002000 {
compatible = "nordic,nrf-uarte", "nordic,nrf-uart";
/* uart can be either UART or UARTE, for the user to pick */
/* compatible = "nordic,nrf-uarte" or "nordic,nrf-uart"; */
reg = <0x40002000 0x1000>;
interrupts = <2 1>;
status = "disabled";
Copy link
Collaborator

Choose a reason for hiding this comment

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

@galak where would you put default RAM size for uarte buffer? In this common data or anywhere else?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it would be a property in uarte node. So we'd add it to the yaml & .dtsi. However I do see how that would create a difference between uart and uarte.

@carlescufi
Copy link
Member

@jarz-nordic @barsok can you please re-review and approve or request changes?

Copy link
Member

@carlescufi carlescufi left a comment

Choose a reason for hiding this comment

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

Approving on behalf of @barsok, since he is on holiday

@galak galak merged commit f0450fc into zephyrproject-rtos:master Jul 10, 2018
@galak galak deleted the nrf-uart-dts branch July 10, 2018 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Devicetree area: UART Universal Asynchronous Receiver-Transmitter platform: nRF Nordic nRFx
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants