Re: [PATCH v3 tty-next 1/2] dt-bindings: serial: ni,ni16650: add bindings

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 19/04/2023 00:37, Brenda Streiff wrote:
> Add bindings for the NI 16550 UART.

Do not attach (thread) your patchsets to some other threads (unrelated
or older versions). This buries them deep in the mailbox and might
interfere with applying entire sets.

> 
> Signed-off-by: Brenda Streiff <brenda.streiff@ni.com>
> Cc: Gratian Crisan <gratian.crisan@ni.com>
> Cc: Jason Smith <jason.smith@ni.com>
> ---
>  .../bindings/serial/ni,ni16550.yaml           | 64 +++++++++++++++++++
>  1 file changed, 64 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/serial/ni,ni16550.yaml
> 
> diff --git a/Documentation/devicetree/bindings/serial/ni,ni16550.yaml b/Documentation/devicetree/bindings/serial/ni,ni16550.yaml
> new file mode 100644
> index 000000000000..93b88c8206b9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serial/ni,ni16550.yaml
> @@ -0,0 +1,64 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/serial/ni,ni16550.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NI 16550 asynchronous serial interface (UART)
> +
> +maintainers:
> +  - Brenda Streiff <brenda.streiff@ni.com>
> +
> +allOf:
> +  - $ref: serial.yaml#
> +
> +properties:
> +  compatible:
> +    const: ni,ni16550
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    items:
> +      - const: baudclk

No need for names for one clock entry.

> +
> +  # legacy clock property; prefer 'clocks' instead
> +  clock-frequency: true

Drop it, we do not want legacy or deprecated properties on new bindings.
It's not a legacy binding...

> +
> +  ni,serial-port-mode:
> +    description: Indicates whether this is an RS-232 or RS-485 serial port.
> +    $ref: /schemas/types.yaml#/definitions/string
> +    enum: [ RS-232, RS-485 ]
> +    default: RS-485
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    serial@80000000 {
> +      compatible = "ni,ni16550";
> +      reg = <0x80000000 0x8>;
> +      interrupts = <0 30 4>;

I still wonder what are these. Flags? Then use common defines.

> +      clock-names = "baudclk";
> +      clocks = <&clk_uart>;
> +      ni,serial-port-mode = "RS-232";
> +    };
> +
> +    clk_uart: clock {

Drop node, not related to your code. You are not documenting usage of
fixed-clock as it is obvious and already documented in other place.


Best regards,
Krzysztof