| Message ID | e290dd28ecb68b4e164172a905da18a5a2d438a1.1739525488.git.geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | Under Review |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | ARM: dts: renesas: r9a06g032: UART dtbs_check fixes | expand |
Hello Geert, On 14/02/2025 at 10:42:04 +01, Geert Uytterhoeven <geert+renesas@glider.be> wrote: > make dtbs_check: > > arch/arm/boot/dts/renesas/r9a06g032-rzn1d400-db.dtb: serial@40060000: compatible: 'oneOf' conditional failed, one must be fixed: > ['renesas,r9a06g032-uart', 'renesas,rzn1-uart', 'snps,dw-apb-uart'] is too long > ... > > As per commit 72b0505f0830df95 ("dt: serial: Add Renesas RZ/N1 binding > documentation"), the RZ/N1 UART is a modified Synopsys DesignWare UART. > The modifications only relate to DMA, so you could actually use the > controller with the Synopsys compatible string if you are not using DMA, > but you should not do so. Hence the first three UARTs (which don't > support DMA) were added with a "snps,dw-apb-uart" fallback, to use the > existing Synopsys DesignWare UART support. > > Since support for the RZ/N1-specific compatible value was added to the > driver a long time ago (commit 2ff5fa7f742ab0c6 ("serial: 8250_dw: Add > compatible string for Renesas RZ/N1 UART") in v4.19), the extra > compatible value can be dropped safely. > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- > Alternatively, the bindings[1] could be augmented with an extra section: > > - items: > - enum: > - renesas,r9a06g032-uart > - renesas,r9a06g033-uart > - const: renesas,rzn1-uart > - const: snps,dw-apb-uart # RZ/N1 without DMA > > and perhaps extra logic to prohibit the dmas property when both > renesas,rzn1-uart and snps,dw-apb-uart are present. I must say that I prefer this secondary approach, which feels more accurate. I won't block the one that your proposed below for sure, but I feel like it is more relevant to add this third entry in the bindings rather than removing it from the DT. Either way, fine by me. > > Given the complexity of the latter, I went for the simple solution. > > [1] Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml Thanks, Miquèl
diff --git a/arch/arm/boot/dts/renesas/r9a06g032.dtsi b/arch/arm/boot/dts/renesas/r9a06g032.dtsi index 87e03446fb4de705..fc523106c2a4c6a0 100644 --- a/arch/arm/boot/dts/renesas/r9a06g032.dtsi +++ b/arch/arm/boot/dts/renesas/r9a06g032.dtsi @@ -171,7 +171,7 @@ usb@2,0 { }; uart0: serial@40060000 { - compatible = "renesas,r9a06g032-uart", "renesas,rzn1-uart", "snps,dw-apb-uart"; + compatible = "renesas,r9a06g032-uart", "renesas,rzn1-uart"; reg = <0x40060000 0x400>; interrupts = <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>; reg-shift = <2>; @@ -182,7 +182,7 @@ uart0: serial@40060000 { }; uart1: serial@40061000 { - compatible = "renesas,r9a06g032-uart", "renesas,rzn1-uart", "snps,dw-apb-uart"; + compatible = "renesas,r9a06g032-uart", "renesas,rzn1-uart"; reg = <0x40061000 0x400>; interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>; reg-shift = <2>; @@ -193,7 +193,7 @@ uart1: serial@40061000 { }; uart2: serial@40062000 { - compatible = "renesas,r9a06g032-uart", "renesas,rzn1-uart", "snps,dw-apb-uart"; + compatible = "renesas,r9a06g032-uart", "renesas,rzn1-uart"; reg = <0x40062000 0x400>; interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>; reg-shift = <2>;
make dtbs_check: arch/arm/boot/dts/renesas/r9a06g032-rzn1d400-db.dtb: serial@40060000: compatible: 'oneOf' conditional failed, one must be fixed: ['renesas,r9a06g032-uart', 'renesas,rzn1-uart', 'snps,dw-apb-uart'] is too long ... As per commit 72b0505f0830df95 ("dt: serial: Add Renesas RZ/N1 binding documentation"), the RZ/N1 UART is a modified Synopsys DesignWare UART. The modifications only relate to DMA, so you could actually use the controller with the Synopsys compatible string if you are not using DMA, but you should not do so. Hence the first three UARTs (which don't support DMA) were added with a "snps,dw-apb-uart" fallback, to use the existing Synopsys DesignWare UART support. Since support for the RZ/N1-specific compatible value was added to the driver a long time ago (commit 2ff5fa7f742ab0c6 ("serial: 8250_dw: Add compatible string for Renesas RZ/N1 UART") in v4.19), the extra compatible value can be dropped safely. Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> --- Alternatively, the bindings[1] could be augmented with an extra section: - items: - enum: - renesas,r9a06g032-uart - renesas,r9a06g033-uart - const: renesas,rzn1-uart - const: snps,dw-apb-uart # RZ/N1 without DMA and perhaps extra logic to prohibit the dmas property when both renesas,rzn1-uart and snps,dw-apb-uart are present. Given the complexity of the latter, I went for the simple solution. [1] Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml --- arch/arm/boot/dts/renesas/r9a06g032.dtsi | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)