[RFC] Inconsistent sifive,fu540-c000-uart binding.

[RFC] Inconsistent sifive,fu540-c000-uart binding.

[Sebastian Andrzej Siewior]

| $ git grep fu540-c000-uart
| Documentation/devicetree/bindings/serial/sifive-serial.yaml:          - sifive,fu540-c000-uart
| Documentation/devicetree/bindings/serial/sifive-serial.yaml:        compatible = "sifive,fu540-c000-uart", "sifive,uart0";
| Documentation/devicetree/bindings/sifive/sifive-blocks-ip-versioning.txt:"sifive,fu540-c000-uart".  This way, if SoC-specific
| Documentation/devicetree/bindings/sifive/sifive-blocks-ip-versioning.txt:    compatible = "sifive,fu540-c000-uart", "sifive,uart0";
| arch/riscv/boot/dts/sifive/fu540-c000.dtsi:                     compatible = "sifive,fu540-c000-uart", "sifive,uart0";
| arch/riscv/boot/dts/sifive/fu540-c000.dtsi:                     compatible = "sifive,fu540-c000-uart", "sifive,uart0";
| drivers/tty/serial/sifive.c:OF_EARLYCON_DECLARE(sifive, "sifive,fu540-c000-uart0",
| drivers/tty/serial/sifive.c:    { .compatible = "sifive,fu540-c000-uart0" },

note that the driver has a trailing 0 in the binding while the yaml
description and the DT part does not.
The 'sifive,uart' has a trailing 0 where the 0 denotes the version UART
IP.

Was this also intended for the fu540-c000-uart binding? Should the 0 be
added everywhere or removed from the driver?

Sebastian

Re: [RFC] Inconsistent sifive,fu540-c000-uart binding.

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1579 bytes --]

On Mon, Mar 04, 2024 at 11:59:47AM +0100, Sebastian Andrzej Siewior wrote:
> | $ git grep fu540-c000-uart
> | Documentation/devicetree/bindings/serial/sifive-serial.yaml:          - sifive,fu540-c000-uart
> | Documentation/devicetree/bindings/serial/sifive-serial.yaml:        compatible = "sifive,fu540-c000-uart", "sifive,uart0";
> | Documentation/devicetree/bindings/sifive/sifive-blocks-ip-versioning.txt:"sifive,fu540-c000-uart".  This way, if SoC-specific
> | Documentation/devicetree/bindings/sifive/sifive-blocks-ip-versioning.txt:    compatible = "sifive,fu540-c000-uart", "sifive,uart0";
> | arch/riscv/boot/dts/sifive/fu540-c000.dtsi:                     compatible = "sifive,fu540-c000-uart", "sifive,uart0";
> | arch/riscv/boot/dts/sifive/fu540-c000.dtsi:                     compatible = "sifive,fu540-c000-uart", "sifive,uart0";
> | drivers/tty/serial/sifive.c:OF_EARLYCON_DECLARE(sifive, "sifive,fu540-c000-uart0",
> | drivers/tty/serial/sifive.c:    { .compatible = "sifive,fu540-c000-uart0" },
> 
> note that the driver has a trailing 0 in the binding while the yaml
> description and the DT part does not.
> The 'sifive,uart' has a trailing 0 where the 0 denotes the version UART
> IP.
> 
> Was this also intended for the fu540-c000-uart binding? Should the 0 be
> added everywhere or removed from the driver?

I suspect that the driver is what's incorrect, given there's little
value in putting the IP version in the SoC-specific compatible as it's
a fixed implementation. I'd change the driver to match the bindings.

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [RFC] Inconsistent sifive,fu540-c000-uart binding.

[Paul Walmsley]

On Mon, 4 Mar 2024, Conor Dooley wrote:

> On Mon, Mar 04, 2024 at 11:59:47AM +0100, Sebastian Andrzej Siewior wrote:
> > | $ git grep fu540-c000-uart
> > | Documentation/devicetree/bindings/serial/sifive-serial.yaml:          - sifive,fu540-c000-uart
> > | Documentation/devicetree/bindings/serial/sifive-serial.yaml:        compatible = "sifive,fu540-c000-uart", "sifive,uart0";
> > | Documentation/devicetree/bindings/sifive/sifive-blocks-ip-versioning.txt:"sifive,fu540-c000-uart".  This way, if SoC-specific
> > | Documentation/devicetree/bindings/sifive/sifive-blocks-ip-versioning.txt:    compatible = "sifive,fu540-c000-uart", "sifive,uart0";
> > | arch/riscv/boot/dts/sifive/fu540-c000.dtsi:                     compatible = "sifive,fu540-c000-uart", "sifive,uart0";
> > | arch/riscv/boot/dts/sifive/fu540-c000.dtsi:                     compatible = "sifive,fu540-c000-uart", "sifive,uart0";
> > | drivers/tty/serial/sifive.c:OF_EARLYCON_DECLARE(sifive, "sifive,fu540-c000-uart0",
> > | drivers/tty/serial/sifive.c:    { .compatible = "sifive,fu540-c000-uart0" },
> > 
> > note that the driver has a trailing 0 in the binding while the yaml
> > description and the DT part does not.
> > The 'sifive,uart' has a trailing 0 where the 0 denotes the version UART
> > IP.
> > 
> > Was this also intended for the fu540-c000-uart binding? Should the 0 be
> > added everywhere or removed from the driver?
> 
> I suspect that the driver is what's incorrect, given there's little
> value in putting the IP version in the SoC-specific compatible as it's
> a fixed implementation. I'd change the driver to match the bindings.

Agreed


- Paul