Skip to content

Conversation

@Mierunski
Copy link

Add UART configure function for UART and UARTE shims.

Signed-off-by: Mieszko Mierunski [email protected]

@codecov-io
Copy link

codecov-io commented Nov 21, 2018

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #11577   +/-   ##
=======================================
  Coverage   48.21%   48.21%           
=======================================
  Files         279      279           
  Lines       43303    43303           
  Branches    10370    10370           
=======================================
  Hits        20878    20878           
  Misses      18279    18279           
  Partials     4146     4146

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 bb68ac6...e5bf097. Read the comment docs.

@jakub-uC jakub-uC added the area: UART Universal Asynchronous Receiver-Transmitter label Nov 22, 2018
@Mierunski Mierunski force-pushed the nrf_uart_configure branch 4 times, most recently from 917e492 to 3e36b2f Compare November 23, 2018 14:06
@zephyrproject-rtos zephyrproject-rtos deleted a comment from zephyrbot Nov 23, 2018
@Mierunski Mierunski force-pushed the nrf_uart_configure branch 2 times, most recently from 20b7aea to 2a46ae6 Compare November 26, 2018 08:42
@Mierunski
Copy link
Author

@anangl All should be fixed now

@Mierunski Mierunski force-pushed the nrf_uart_configure branch 3 times, most recently from 901d7b8 to 1342b9c Compare November 28, 2018 08:34
@Mierunski Mierunski force-pushed the nrf_uart_configure branch 2 times, most recently from 367c564 to 198391e Compare November 28, 2018 13:20
@Mierunski
Copy link
Author

@anangl Fixed

Copy link
Member

@anangl anangl left a comment

Choose a reason for hiding this comment

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

Almost there. A few minor corrections needed.

Add UART configure function for UART and UARTE shims.

Signed-off-by: Mieszko Mierunski <[email protected]>
Copy link
Member

@anangl anangl left a comment

Choose a reason for hiding this comment

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

Beautiful. As much as it was possible. ;-)
Thanks for your tenacity.

Copy link
Contributor

@jakub-uC jakub-uC left a comment

Choose a reason for hiding this comment

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

Minor change proposals -not a merge blocker.

}

if (baudrate_set(dev, cfg->baudrate) != 0) {
return -ENOTSUP;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that value -EINVAL is better here.

Copy link
Author

Choose a reason for hiding this comment

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

API specifies that -ENOTSUP will be returned if configuration is not supported by driver. Same applies to other places

parity = NRF_UART_PARITY_INCLUDED;
break;
default:
return -ENOTSUP;
Copy link
Contributor

Choose a reason for hiding this comment

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

-EINVAL

#ifndef DT_NORDIC_NRF_UART_UART_0_CTS_PIN
#error Flow control for UART0 is enabled, but CTS pin is not defined.
#endif
#if defined(DT_NORDIC_NRF_UART_UART_0_RTS_PIN) && \
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use IS_ENABLED macro.

Copy link
Author

Choose a reason for hiding this comment

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

Functions called in lines below use value of this define, I can use #if with IS_ENABLED but I don't think it will improve anything. Using if() is not possible here

}
break;
default:
return -ENOTSUP;
Copy link
Contributor

Choose a reason for hiding this comment

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

-EINVAL

}

if (baudrate_set(dev, cfg->baudrate) != 0) {
return -ENOTSUP;
Copy link
Contributor

Choose a reason for hiding this comment

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

-EINVAL

@Mierunski
Copy link
Author

@carlescufi Please take a look

@carlescufi carlescufi merged commit 94b72e7 into zephyrproject-rtos:master Nov 29, 2018
@Mierunski Mierunski deleted the nrf_uart_configure branch November 29, 2018 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: Drivers area: UART Universal Asynchronous Receiver-Transmitter platform: nRF Nordic nRFx

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants