Skip to content

Serial driver migration to DT #8847

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

Conversation

jakub-uC
Copy link
Collaborator

No description provided.

@jakub-uC
Copy link
Collaborator Author

@anangl @ioannisg @carlescufi : please take a look

@ioannisg ioannisg added the platform: nRF Nordic nRFx label Jul 11, 2018
@jakub-uC jakub-uC force-pushed the serial_driver_migration_to_DT branch from e1b35df to 7d28a65 Compare July 11, 2018 12:11
@codecov-io
Copy link

codecov-io commented Jul 11, 2018

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #8847   +/-   ##
=======================================
  Coverage   52.31%   52.31%           
=======================================
  Files         195      195           
  Lines       24732    24732           
  Branches     5141     5141           
=======================================
  Hits        12939    12939           
  Misses       9719     9719           
  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 6b836d6...916ecab. Read the comment docs.

anangl
anangl previously requested changes Jul 11, 2018
@@ -563,6 +567,9 @@ static int uarte_instance_init(struct device *dev,
#define UARTE_CONFIG_INT(idx) \
.tx_buff_size = UARTE_TX_BUFFER_SIZE(idx)

#define UARTE_IRQ_NUMBER_SET(idx) \
.irq_number = CONFIG_UART_##idx##_IRQ_NUM,
Copy link
Member

Choose a reason for hiding this comment

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

Instead of adding a dedicated field for this, it would be better to move irq_enable to the UARTE_NRF_IRQ_ENABLED macro.

@@ -13,6 +13,8 @@
#include <hal/nrf_gpio.h>


static NRF_UART_Type *uart_addr = (NRF_UART_Type *)CONFIG_UART_0_BASE;
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: uart_addr -> uart0_addr

@jakub-uC jakub-uC force-pushed the serial_driver_migration_to_DT branch from 7d28a65 to f5644a0 Compare July 12, 2018 10:56
@jakub-uC
Copy link
Collaborator Author

Fixed

@@ -13,6 +13,8 @@
#include <hal/nrf_gpio.h>


static NRF_UART_Type *uart0_addr = (NRF_UART_Type *)CONFIG_UART_0_BASE;
Copy link
Member

Choose a reason for hiding this comment

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

I'd add const. After the *, of course.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

1. dts.fixup files updated with peripheral address and IRQ NUMBER.
2. Peripheral address is taken from DT.
3. IRQ number is taken from DT.

Signed-off-by: Jakub Rzeszutko <[email protected]>
@carlescufi carlescufi dismissed anangl’s stale review July 12, 2018 16:30

Issues fixed

@@ -483,9 +483,6 @@ static int uarte_instance_init(struct device *dev,

#if UARTE_INTERRUPT_DRIVEN
if (interrupts_active) {

irq_enable(NRFX_IRQ_NUMBER_GET(uarte));
Copy link
Member

Choose a reason for hiding this comment

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

Is this intentional?

Copy link
Member

Choose a reason for hiding this comment

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

OK, I see this is moved to the UARTE___IRQ_ENABLED() macro function

@@ -549,7 +546,8 @@ static int uarte_instance_init(struct device *dev,
CONFIG_UART_##idx##_IRQ_PRI, \
uarte_nrfx_isr, \
DEVICE_GET(uart_nrfx_uarte##idx), \
0)
0); \
irq_enable(CONFIG_UART_##idx##_IRQ_NUM)
Copy link
Member

Choose a reason for hiding this comment

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

I am wondering if this should be UARTE_, instead

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is convention from DT and driver names like:
uart_nrfx_uarte.c

Copy link
Member

Choose a reason for hiding this comment

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

OK, I see that .fixup defines same macros for UART and UARTE devices

@carlescufi carlescufi merged commit f0de6e0 into zephyrproject-rtos:master Jul 13, 2018
@jakub-uC jakub-uC deleted the serial_driver_migration_to_DT branch January 30, 2019 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants