[RFC] [DNM] dts: bindings: remove nordic,nrf-uarte from dts/dtsi/overlays/bindings

[barsok]

I have a suggestion to remove string: "nordic,nrf-uarte" from compatible and related .yaml file and would like to ask for comments.

The reasons are:

• "nordic,nrf-uarte" is actually never used, it is always overwritten with "nordic,nrf-uart"
• failing to overwrite leads to compilation errors as no code expects NORDIC_NRF_UARTE_40002000_BASE_ADDRESS
• uarte driver proposed in PR drivers: serial: nrf: Adding UARTE driver for the nRFx family #8640 is created with "compatible = nordic,nrf-uart" defines in mind

Signed-off-by: Bartosz Sokolski [email protected]

[carlescufi]

@barsok I have no objections to this as long as the selection of UART vs UARTE in SoCs where both are available can be done easily through some means. What would that be, Kconfig?

[barsok]

@carlescufi PR #8640 introduces new settings in Kconfig which let the user select whether they want to use UART or UARTE, depending on availibility of both

[codecov-io]

Codecov Report

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

@@           Coverage Diff           @@
##           master    #8735   +/-   ##
=======================================
  Coverage   52.36%   52.36%           
=======================================
  Files         195      195           
  Lines       24696    24696           
  Branches     5128     5128           
=======================================
  Hits        12931    12931           
  Misses       9695     9695           
  Partials     2070     2070

---

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 b4dae00...605eccd. Read the comment docs.

[galak]

Nack. This shouldn't be handled via Kconfig, it should be handled via DT like it is.

I don't understand what issue you are running into when the compatible is set to "nordic,nrf-uarte"

[carlescufi]

@galak I think the issue is that the same hardware can be accessed by 2 different drivers, same register address space. In the UARTE PR (#8640) the selection of the driver is done in Kconfig, not in DT because this is in the gray area of software/hardware.

[galak]

@galak I think the issue is that the same hardware can be accessed by 2 different drivers, same register address space. In the UARTE PR (#8640) the selection of the driver is done in Kconfig, not in DT because this is in the gray area of software/hardware.

At some point we will be able to remove the port selection from Kconfig, and it will just be the driver. So doing this with DT to start with is the way to go. Its pretty straight forward to get the UARTE driver to use DT.

[galak]

Closing this as the proper way is to fixup #8640 to handle DTS generation for UARTE.

[barsok]

@galak (@jarz-nordic, @carlescufi ) Hi Kumar. I appreciate your strong opinion about the DT and drivers. I also think that the device tree is the way to go and I believe that my PR did not counter this approach. Therefore I am surprised you have closed the PR without even asking "why" I had opened it. I am going to provide the "why" anyways and I will appreciate your comments and further discussion.

The facts for now:

1. At present UART-E is absolutely not handled via device tree. If you fail to switch compatible back to "nordic,nrf-uart", your code will not compile. The "generated_dts...h" will contain labels with "UARTE" and not "UART". Of course one could provide the fixup files, but these fixup files would "merge" defines to single "UART_0_BAUDRATE" etc, and then split again to different drivers. It's completely uncomprehensible.
2. UART and UART-E are not compatible and expression in dtsi "compatible = nordic,nrf-uart, nordic,nrf-uarte" is false. For example, they cannot share the driver. They share the same address space, but in silicon they are different devices. For now, they do share the purpose so some settings are the same, like rx-pin, tx-pin, baud rate etc so they can both be handled with one entry.
3. The introduced UART-E driver will take Kconfig approach (please remember: for now)

Discussion about the future:
How to prepare for the future? There could be a number of ways:

1. To me the easiest is to separate entities in the dtsi and have two of them: uart0:uart@40002000, compatible=nordic,nrf-uart and uarte0:uarte@4000200, compatible=nordic,nrf-uarte. Then the user would enable only one of the two. Feature set could be completely different, different yaml files inherently as well.
2. list all possible features in the dtsi for both uart and uarte and keep one entity. For example RAM buffer size which applies only to uart-e. Then different options would compile based on compatible string. In my opinion this is again misleading because how would the user know if the RAM buffer size applies or not? What if we enable other features which apply only to uart or uarte?

My conclusion is to "scrap" what we have now to prepare for the future with great and easy device tree. and hence this Pull Request. What is your opinon, again?

[galak]

At present UART-E is absolutely not handled via device tree. If you fail to switch compatible back to "nordic,nrf-uart", your code will not compile. The "generated_dts...h" will contain labels with "UARTE" and not "UART". Of course one could provide the fixup files, but these fixup files would "merge" defines to single "UART_0_BAUDRATE" etc, and then split again to different drivers. It's completely uncomprehensible.

This is done once in the SOC dts fixup so the user isn't going to

UART and UART-E are not compatible and expression in dtsi "compatible = nordic,nrf-uart, nordic,nrf-uarte" is false. For example, they cannot share the driver. They share the same address space, but in silicon they are different devices. For now, they do share the purpose so some settings are the same, like rx-pin, tx-pin, baud rate etc so they can both be handled with one entry.

I agree they aren't the same device and there for not the same compatible. We can remove the compatible property from the SoC dtsi and force the user / board dtsi to explicitly set it, the idea of "compatible = nordic,nrf-uart, nordic,nrf-uarte" was to convey that it should be one or the other.

The introduced UART-E driver will take Kconfig approach (please remember: for now)

I don't see why we need to take the Kconfig option.

[galak]

Take a look at #8761

[carlescufi]

I don't see why we need to take the Kconfig option.

This is what we cover in #8758

[galak]

This is what we cover in #8758

Fair enough, the Kconfig bit doesn't concern me right now, however we should be using the proper compatible in dt for the device that is actually used, and thus #8761 should hopefully clean all this up.