Skip to content
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

uart/ns16550: Use DT_ prefix for remaining device configs #13760

Merged
merged 1 commit into from
Feb 27, 2019

Conversation

dcpleung
Copy link
Member

Previous rename from CONFIG_* to DT_* left a few remaining
CONFIG_*. So rename them manually now.

Fix #13753

Signed-off-by: Daniel Leung daniel.leung@intel.com

Previous rename from CONFIG_* to DT_* left a few remaining
CONFIG_*. So rename them manually now.

Fix zephyrproject-rtos#13753

Signed-off-by: Daniel Leung <daniel.leung@intel.com>
@ulfalizer
Copy link
Collaborator

ulfalizer commented Feb 26, 2019

Will e.g. {CONFIG/DT}_UART_NS16550_PORT_0_BAUD_RATE always come from DTS? There's still a Kconfig symbol definition for it in drivers/serial/Kconfig.ns16550, within an if !HAS_DTS block.

I think the thinking behind it is that dts_fixup.h will have a #define CONFIG_* for cases where the value could come from either DTS or Kconfig (so that code that uses the setting can work for both cases). That's not the case for e.g. *_UART_NS16550_PORT_2_BAUD_RATE (note, 2 instead of 0) though, because there is no Kconfig symbol called UART_NS16550_PORT_2_BAUD_RATE (unlike for index 0).

@dcpleung
Copy link
Member Author

I think we should unify how all NS16550 ports are being configured. So if there is a baud rate kconfig for one, all four ports should have that. However, I think we should all move to DTS configuration. Though, if we go that route, can we remove those options now, after feature freeze? If not, I can always change those DT_* back to CONFIG_*.

@ulfalizer
Copy link
Collaborator

@dcpleung
Don't know what our policy is there. @nashif or @galak might.

I'm happy with getting rid of Kconfig symbols at least. We have too many.

@codecov-io
Copy link

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #13760   +/-   ##
=======================================
  Coverage   52.24%   52.24%           
=======================================
  Files         307      307           
  Lines       45413    45413           
  Branches    10508    10508           
=======================================
  Hits        23726    23726           
  Misses      16898    16898           
  Partials     4789     4789

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 f5951cd...c1e7b4e. Read the comment docs.

@galak galak merged commit 603f068 into zephyrproject-rtos:master Feb 27, 2019
ulfalizer added a commit to ulfalizer/zephyr that referenced this pull request Feb 27, 2019
These symbols are not referenced anywhere. The values always come from
DTS, at least if zephyrproject-rtos#13760
was correct.

Signed-off-by: Ulf Magnusson <Ulf.Magnusson@nordicsemi.no>
@dcpleung dcpleung deleted the fix_uart_ns16550_13753 branch February 27, 2019 18:12
nashif pushed a commit that referenced this pull request Mar 1, 2019
These symbols are not referenced anywhere. The values always come from
DTS, at least if #13760
was correct.

Signed-off-by: Ulf Magnusson <Ulf.Magnusson@nordicsemi.no>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants