Re: [PATCH 1/3] dt-bindings: serial: Add binding for Cortina-Access UART

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] The board compatible strings restrict the array to a single string, forbidding the required generic SoC fallback.
- [Low] The UART binding example defines a 4-cell `reg` property without specifying the parent bus `#address-cells` and `#size-cells`, which causes `dt_binding_check` to fail.
- [Low] Alphabetical sorting violation in `vendor-prefixes.yaml` and `MAINTAINERS`, breaking devicetree tooling and sorting guidelines.
- [Low] The UART binding uses a generic device class compatible string instead of a specific implementation string.
--

commit 32a23126a643cf7b114be8e3c3826bc3ea97f447
Author: Jason Li <jason.lee651024@gmail.com>

dt-bindings: serial: Add binding for Cortina-Access UART
    
This commit adds a new device tree binding schema for the Cortina-Access
UART controller, targeting the CAXXXX SoC family. It also introduces the
vendor prefix for Cortina Access and a top-level ARM board binding for the
CA8289 (Venus) SoC.

> diff --git a/Documentation/devicetree/bindings/arm/cortina-access.yaml b/Documentation/devicetree/bindings/arm/cortina-access.yaml
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/cortina-access.yaml
[ ... ]
> +properties:
> +  $nodename:
> +    const: /
> +  compatible:
> +    oneOf:
> +      - description: Cortina-Access CA8289 (Venus) engineering board
> +        const: cortina-access,ca8289-engboard

[Severity: Medium]
Will rigidly enforcing a single compatible string cause dtbs_check to fail
on standard board device trees?

Standard ARM platform device trees typically define the board compatible as
an array containing the specific board followed by the generic SoC string.
Should this use an items array to explicitly define and allow both the
board string and the required SoC fallback string instead of const?

> +
> +      - description: Cortina-Access CA8289 (Venus) reference board
> +        const: cortina-access,ca8289-refboard
[ ... ]
> diff --git a/Documentation/devicetree/bindings/serial/cortina-access,serial.yaml b/Documentation/devicetree/bindings/serial/cortina-access,serial.yaml
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serial/cortina-access,serial.yaml
[ ... ]
> +properties:
> +  compatible:
> +    const: cortina-access,serial

[Severity: Low]
Is this compatible string too generic?

The Devicetree specification generally advises against using generic device
class names in compatible strings. Would it be better to name the
compatible string after the first specific SoC implementation, such as
cortina-access,ca8289-uart, and use that as the generic fallback?

> +
> +  reg:
> +    maxItems: 1
[ ... ]
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    serial@f4329188 {
> +        compatible = "cortina-access,serial";
> +        reg = <0 0xf4329188 0 0x30>;

[Severity: Low]
Does this example fail dt_binding_check?

The schema restricts the reg property to maxItems: 1. Because no parent bus
node is defined, the dt-extract-example tool wraps this in a default root
node with 1-cell address and size. This causes the 4-cell reg array here to
be parsed as two distinct register regions, violating the maxItems constraint.

> +        interrupts = <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>;
> +        clocks = <&apb_pclk>;
> +    };
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> @@ -367,6 +367,8 @@ patternProperties:
>      description: Chengdu Corpro Technology Co., Ltd.
>    "^corechips,.*":
>      description: Shenzhen Corechips Microelectronics Co., Ltd.
> +  "^cortina-access,.*":
> +    description: Cortina Access, Inc.

[Severity: Low]
Could this insertion break schema validation due to alphabetical sorting?

The vendor-prefixes.yaml file enforces strict ASCII alphabetical sorting.
A comma (ASCII 44) is less than a hyphen (ASCII 45), so ^cortina,.* should
precede ^cortina-access,.*.

>    "^cortina,.*":
>      description: Cortina Systems, Inc.
>    "^cosmic,.*":
> diff --git a/MAINTAINERS b/MAINTAINERS
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2749,6 +2749,13 @@ F:	tools/perf/tests/shell/lib/*coresight*
>  F:	tools/perf/util/cs-etm-decoder/*
>  F:	tools/perf/util/cs-etm.*
>  
> +ARM/CORTINA-ACCESS VENUS ARM ARCHITECTURE
> +M:	Jason Li <jason.li@cortina-access.com>
> +L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/arm/cortina-access.yaml
> +F:	Documentation/devicetree/bindings/serial/cortina-access,serial.yaml
> +

[Severity: Low]
Does this entry violate the alphabetical sorting guidelines for the
MAINTAINERS file?

A space (ASCII 32) is less than a hyphen (ASCII 45), so ARM/CORTINA SYSTEMS
should appear before ARM/CORTINA-ACCESS.

>  ARM/CORTINA SYSTEMS GEMINI ARM ARCHITECTURE
>  M:	Hans Ulli Kroll <ulli.kroll@googlemail.com>
>  M:	Linus Walleij <linusw@kernel.org>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260610112821.3030099-1-jason.li@cortina-access.com?part=1