Re: [PATCH 2/2] dt-bindings: serial: Document WK2xxx SPI UART bindings

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Multi-port UART device incorrectly modeled as a single flat node without subnodes, preventing per-port independent configuration.
- [Low] Missing trailing newline at the end of the YAML file.
- [Low] Missing `default` property for `clock-frequency` despite the description claiming a default exists.
--

commit beaa6c014e093e4f6483f239f3024922536014b5
Author: zjzhao <zjzhao@edatec.cn>

dt-bindings: serial: Document WK2xxx SPI UART bindings

Add a DT binding for the WK2xxx SPI to UART bridge ICs (WK2124, WK2132,
WK2168, WK2202 and WK2204). It covers the compatible strings, SPI properties,
interrupt line, and the crystal clock frequency. It also registers the wkmic
vendor prefix.

> diff --git a/Documentation/devicetree/bindings/serial/wkmic,wk2xxx.yaml b/Documentation/devicetree/bindings/serial/wkmic,wk2xxx.yaml
> new file mode 100644
> index 0000000000000..0ef76957d73f1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serial/wkmic,wk2xxx.yaml
> @@ -0,0 +1,69 @@
[ ... ]
> +  clock-frequency:
> +    description:
> +      Crystal or external clock frequency for the IC in Hz. The WK2xxx has
> +      no internal clock, so this must match the connected crystal. Defaults
> +      to 11059200 Hz when the property is absent.
> +    minimum: 1
> +    maximum: 30000000

[Severity: Low]
Is it possible to include the default property value of 11059200 alongside
the minimum and maximum constraints?

Schema validation tools and external parsers cannot automatically infer the
default fallback value because it is documented in text but not defined as a
JSON schema constraint.

> +
> +required:
> +  - compatible
> +  - reg
> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +  - $ref: /schemas/serial/serial.yaml#
> +  - $ref: /schemas/serial/rs485.yaml#

[Severity: High]
Does this single-node model prevent independent configuration of the
individual UART ports?

Because this binding applies the serial.yaml and rs485.yaml definitions
directly to the parent SPI device node, rather than defining child nodes like
serial@0 and serial@1, all configuration properties apply globally to every
channel.

If a user needs to configure RS-485 on only one of the 4 UART channels using
device tree properties, the single flat node approach would force the driver
to apply the configuration identically to every port. Should this binding be
structured to support child nodes for individual port configuration to prevent
permanently restricting these hardware capabilities in the ABI?

> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        serial@0 {
> +            compatible = "wkmic,wk2132";
> +            reg = <0>;
> +            spi-max-frequency = <10000000>;
> +            clock-frequency = <11059200>;
> +            interrupt-parent = <&gpio>;
> +            interrupts = <24 IRQ_TYPE_LEVEL_LOW>;
> +        };
> +    };
> \ No newline at end of file

[Severity: Low]
Could a trailing newline be added to the end of this file?

Text files without a trailing newline violate standard POSIX conventions and
kernel coding styles, which can cause issues with git diffs, patching, and
text processing tools.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904072027.67473-1-zjzhao@edatec.cn?part=2