Re: [PATCH 07/10] ARM: dts: tango4: Make /serial compatible with ns16550a

Re: [PATCH 07/10] ARM: dts: tango4: Make /serial compatible with ns16550a

[Måns Rullgård]

Lubomir Rintel <lkundrak@v3.sk> writes:

> ralink,rt2880-uart is compatible with ns16550a and all other
> instances of RT2880 UART nodes include it in the compatible property.
> Add it also here, to make the binding schema simpler.
>
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>

Acked-by: Mans Rullgard <mans@mansr.com>

> ---
>  arch/arm/boot/dts/tango4-common.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/tango4-common.dtsi b/arch/arm/boot/dts/tango4-common.dtsi
> index ff72a8efb73d0..54fd522badfc9 100644
> --- a/arch/arm/boot/dts/tango4-common.dtsi
> +++ b/arch/arm/boot/dts/tango4-common.dtsi
> @@ -85,7 +85,7 @@ tick-counter@10048 {
>  		};
>
>  		uart: serial@10700 {
> -			compatible = "ralink,rt2880-uart";
> +			compatible = "ralink,rt2880-uart", "ns16550a";
>  			reg = <0x10700 0x30>;
>  			interrupts = <1 IRQ_TYPE_LEVEL_HIGH>;
>  			clock-frequency = <7372800>;
> -- 
> 2.25.1
>

-- 
Måns Rullgård

Re: [PATCH 08/10] powerpc/fsl: Make serial ports compatible with ns16550a

[Rob Herring]

On Fri, Mar 20, 2020 at 11:41 AM Lubomir Rintel <lkundrak@v3.sk> wrote:
>
> There are separate compatible strings for ns16550 and ns16550a and the
> Freescale serial port is compatible with the latter one, with working
> FIFO.

I don't think changing this is right. First, 'ns16550' is what's
documented in the DT spec (and ePAPR which it's based on).

Second, these dts files are pretty mature/stable, so I'm less inclined
to change them. AFAICT, this change either enables the fifo (which
would be an untested change in behavior) or has no effect because the
fifo is auto-detected.

> Use the appropriate compatible string that is also consistent with ARM64
> Freescale boards and thus avoiding one special case in the binding schema.

Well, that's annoying...

Rob

Re: [PATCH 10/10] dt-bindings: serial: Convert 8250 to json-schema

[Rob Herring]

On Fri, Mar 20, 2020 at 11:41 AM Lubomir Rintel <lkundrak@v3.sk> wrote:
>
> Some fixes were done during the conversion:

Thanks for doing this!

>
> Slightly better examples. The original example was for an OMAP serial
> port, which is not even described by this binding, but by
> omap_serial.txt instead.
>
> Added compatible strings, that were used, byt not documented:
> andestech,uart16550, cavium,octeon-3860-uart, fsl,16550-FIFO64,
> nvidia,tegra186-uart, nvidia,tegra194-uart, nxp,lpc1850-uart,
> opencores,uart16550-rtlsvn105, ralink,mt7620a-uart, ralink,rt3052-uart,
> ralink,rt3883-uart and xlnx,xps-uart16550-2.00.b.
>
> Removed "serial" compatible string. It's redundant with the node name
> (which, in OFW, serves the same purpose as the compatible string).

Yeah, that seems fairly useless to me.

> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> ---
>  .../devicetree/bindings/serial/8250.txt       | 102 --------
>  .../devicetree/bindings/serial/8250.yaml      | 246 ++++++++++++++++++
>  2 files changed, 246 insertions(+), 102 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/serial/8250.txt
>  create mode 100644 Documentation/devicetree/bindings/serial/8250.yaml

[...]

> diff --git a/Documentation/devicetree/bindings/serial/8250.yaml b/Documentation/devicetree/bindings/serial/8250.yaml
> new file mode 100644
> index 0000000000000..88192f94385b0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serial/8250.yaml
> @@ -0,0 +1,246 @@
> +# Copyright 2020 Lubomir Rintel <lkundrak@v3.sk>
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/serial/8250.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: UART (Universal Asynchronous Receiver/Transmitter) bindings

Perhaps: "8250 series UART..."

> +
> +maintainers:
> +  - devicetree@vger.kernel.org
> +
> +allOf:
> +  - $ref: /schemas/serial.yaml#

> +  - if:
> +      not:
> +        required:
> +          - clock-frequency
> +    then:
> +      required:
> +        - clocks
> +  - if:
> +      not:
> +        required:
> +          - clocks
> +    then:
> +      required:
> +        - clock-frequency

anyOf:
  - required: [ clock-frequency ]
  - required: [ clocks ]

> +  - if:
> +      required:
> +        - aspeed,sirq-polarity-sense
> +    then:
> +      properties:
> +        compatible:
> +          const: aspeed,ast2500-vuart
> +  - if:
> +      properties:
> +        compatible:
> +          const: mrvl,mmp-uart
> +    then:
> +      properties:
> +        reg-shift:
> +          const: 2
> +      required:
> +        - reg-shift
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - const: ns8250
> +      - const: ns16450
> +      - const: ns16550
> +      - const: ns16550a
> +      - const: ns16850
> +      - const: aspeed,ast2400-vuart
> +      - const: aspeed,ast2500-vuart
> +      - const: intel,xscale-uart
> +      - const: mrvl,pxa-uart
> +      - const: nuvoton,npcm750-uart
> +      - const: nvidia,tegra20-uart
> +      - const: nxp,lpc3220-uart
> +      - items:
> +          - enum:
> +              - altr,16550-FIFO32
> +              - altr,16550-FIFO64
> +              - altr,16550-FIFO128
> +              - fsl,16550-FIFO64
> +              - fsl,ns16550
> +              - andestech,uart16550
> +              - nxp,lpc1850-uart
> +              - opencores,uart16550-rtlsvn105
> +              - ti,da830-uart
> +          - const: ns16550a
> +      - items:
> +          - enum:
> +              - ns16750
> +              - cavium,octeon-3860-uart
> +              - xlnx,xps-uart16550-2.00.b
> +              - ralink,rt2880-uart
> +          - const: ns16550

Seems to me these are just as likely to be wrong as the fsl,ns16550 as
it's unlikely the FIFO is broken. Perhaps a note here to use
'ns16550a' instead unless you have a broken FIFO.

> +      - items:
> +          - enum:
> +            - ralink,mt7620a-uart
> +            - ralink,rt3052-uart
> +            - ralink,rt3883-uart
> +          - const: ralink,rt2880-uart
> +          - const: ns16550
> +      - items:
> +          - enum:
> +              - mediatek,mt7622-btif
> +              - mediatek,mt7623-btif
> +          - const: mediatek,mtk-btif
> +      - items:
> +          - enum:
> +              - mediatek,mt7622-btif
> +              - mediatek,mt7623-btif
> +          - const: mediatek,mtk-btif
> +      - items:
> +          - const: mrvl,mmp-uart
> +          - const: intel,xscale-uart
> +      - items:
> +          - enum:
> +              - nvidia,tegra30-uart
> +              - nvidia,tegra114-uart
> +              - nvidia,tegra124-uart
> +              - nvidia,tegra186-uart
> +              - nvidia,tegra194-uart
> +              - nvidia,tegra210-uart
> +          - const: nvidia,tegra20-uart
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clock-frequency: true
> +
> +  clocks:
> +    maxItems: 1
> +
> +  current-speed:
> +    description: The current active speed of the UART.

Needs a type reference.

> +  reg-offset:
> +    description: |
> +      Offset to apply to the mapbase from the start of the registers.
> +
> +  reg-shift:
> +    description: Quantity to shift the register offsets by.
> +
> +  reg-io-width:
> +    description: |
> +      The size (in bytes) of the IO accesses that should be performed on the
> +      device. There are some systems that require 32-bit accesses to the
> +      UART (e.g. TI davinci).

We'll assume these have a type definition elsewhere.

> +
> +  used-by-rtas:
> +    description: |
> +      Set to indicate that the port is in use by the OpenFirmware RTAS and
> +      should not be registered.

Needs a type reference.

> +
> +  no-loopback-test:
> +    description: |
> +      Set to indicate that the port does not implements loopback test mode.

s/implements/implement/

Needs a type reference (or 'type: boolean').

> +
> +  fifo-size:
> +    description: The fifo size of the UART.
> +
> +  auto-flow-control:
> +    description: |
> +      One way to enable automatic flow control support. The driver is
> +      allowed to detect support for the capability even without this
> +      property.
> +
> +  tx-threshold:
> +    description: |
> +      Specify the TX FIFO low water indication for parts with programmable
> +      TX FIFO thresholds.

Need a type reference for all these.

> +
> +  resets:
> +    description: Phandle + reset specifier pairs

Is there a range of number of resets? Use minItems/maxItems.

> +
> +  overrun-throttle-ms:
> +    description: |
> +      How long to pause uart rx when input overrun is encountered.
> +
> +  rts-gpios:
> +    description: |
> +      Specify a GPIO for RTS line to be used instead of the peripheral
> +      function pin for the UART feature.

No need to define properties from serial.yaml again. Just 'rts-gpios: true'.

> +
> +  cts-gpios:
> +    description: |
> +      Specify a GPIO for CTS line to be used instead of the peripheral
> +      function pin for the UART feature.
> +
> +  dtr-gpios:
> +    description: |
> +      Specify a GPIO for DTR line to be used instead of the peripheral
> +      function pin for the UART feature.
> +
> +  dsr-gpios:
> +    description: |
> +      Specify a GPIO for DSR line to be used instead of the peripheral
> +      function pin for the UART feature.
> +
> +  rng-gpios:
> +    description: |
> +      Specify a GPIO for RNG line to be used instead of the peripheral
> +      function pin for the UART feature.
> +
> +  dcd-gpios:
> +    description: |
> +      Specify a GPIO for DCD line to be used instead of the peripheral
> +      function pin for the UART feature.
> +
> +  aspeed,sirq-polarity-sense:
> +    $ref: '/schemas/types.yaml#/definitions/phandle-array'
> +    description: |
> +      Phandle to aspeed,ast2500-scu compatible syscon alongside register
> +      offset and bit number to identify how the SIRQ polarity should be
> +      configured. One possible data source is the LPC/eSPI mode bit. Only
> +      applicable to aspeed,ast2500-vuart.
> +
> +required:
> +  - reg
> +  - interrupts
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    serial@80230000 {
> +        compatible = "ns8250";
> +        reg = <0x80230000 0x100>;
> +        interrupts = <10>;
> +        reg-shift = <2>;
> +        clock-frequency = <48000000>;
> +    };
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    serial@49042000 {
> +        compatible = "andestech,uart16550", "ns16550a";
> +        reg = <0x49042000 0x400>;
> +        interrupts = <80>;
> +        clock-frequency = <48000000>;
> +        cts-gpios = <&gpio3 5 GPIO_ACTIVE_LOW>;
> +        rts-gpios = <&gpio3 6 GPIO_ACTIVE_LOW>;
> +        dtr-gpios = <&gpio1 12 GPIO_ACTIVE_LOW>;
> +        dsr-gpios = <&gpio1 13 GPIO_ACTIVE_LOW>;
> +        dcd-gpios = <&gpio1 14 GPIO_ACTIVE_LOW>;
> +        rng-gpios = <&gpio1 15 GPIO_ACTIVE_LOW>;
> +    };
> +  - |
> +    #include <dt-bindings/clock/aspeed-clock.h>
> +    serial@1e787000 {
> +        compatible = "aspeed,ast2500-vuart";
> +        reg = <0x1e787000 0x40>;
> +        reg-shift = <2>;
> +        interrupts = <8>;
> +        clocks = <&syscon ASPEED_CLK_APB>;
> +        no-loopback-test;
> +        aspeed,sirq-polarity-sense = <&syscon 0x70 25>;
> +    };
> +
> +...
> --
> 2.25.1
>

Re: [PATCH 0/10] NS 8250 UART Device Tree improvements

[Rob Herring]

On Fri, Mar 20, 2020 at 11:41 AM Lubomir Rintel <lkundrak@v3.sk> wrote:
>
> Hi,
>
> this series aims to make it possible to validate NS 8250 compatible serial port
> nodes in Device Tree. It ultimately ends up converting the 8250.txt binding
> specification to YAML for json-schema.
>
> It starts by fixing up a couple of issues that would fail validation of
> device trees for various boards. Note there might be validation issues in other
> boards -- I don't have computing power to run "make dtbs_check" with
> CONFIG_OF_ALL_DTBS=y at the moment. I'm happy to fix up issues if somebody
> runs the test output to me.
>
> Unless someone has a different idea, I'd like to submit this to arm-soc once
> I get the Acks from PXA and Tango maintainers as well as DT reviewers:
>
>   [PATCH 01/10] ARM: dts: pxa*: Don't redeclare phandle references
>   [PATCH 02/10] ARM: dts: pxa*: Fix serial port names
>   [PATCH 03/10] ARM: dts: pxa*: Make the serial ports compatible with
>   [PATCH 04/10] ARM: dts: mmp2-brownstone: Don't redeclare phandle
>   [PATCH 05/10] ARM: dts: mmp*: Fix serial port names
>   [PATCH 06/10] ARM: dts: mmp*: Make the serial ports compatible with
>   [PATCH 07/10] ARM: dts: tango4: Make /serial compatible with ns16550a

For 1-7:

Reviewed-by: Rob Herring <robh@kernel.org>