Re: [PATCH v6 1/6] dt-bindings: serial: 8250: aspeed: add ast2600 and aspeed,vuart-over-pci

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The schema incorrectly enforces `aspeed,ast2500-vuart` as a mandatory fallback for PCI-routed `aspeed,ast2600-vuart` devices, breaking backward compatibility.
- [Low] The schema lacks a mutual exclusivity constraint to forbid LPC-specific properties when the VUART is explicitly configured for PCI.
--

--- Patch [1]: [PATCH v6 1/6] dt-bindings: serial: 8250: aspeed: add ast2600 and aspeed,vuart-over-pci ---
commit f87e924205ddfd55af4d5f7799223f4c9c2ffb59
Author: Grégoire Layet <gregoire.layet@9elements.com>

dt-bindings: serial: 8250: aspeed: add ast2600 and aspeed,vuart-over-pci

The ASPEED AST2600 can be used as a PCI device with 2 Virtual UARTs
accessible over PCI. This commit adds bindings for ast2600-vuart and
a boolean property for using VUART over PCI.

> diff --git a/Documentation/devicetree/bindings/serial/8250.yaml b/Documentation/devicetree/bindings/serial/8250.yaml
> index bb7b9c87a8072..8dd4b7af0c0d2 100644
> --- a/Documentation/devicetree/bindings/serial/8250.yaml
> +++ b/Documentation/devicetree/bindings/serial/8250.yaml

[ ... ]

>    - if:
>        anyOf:
>          - required:
>              - aspeed,sirq-polarity-sense
>      then:
>        properties:
>          compatible:
>            const: aspeed,ast2500-vuart
> +  - if:
> +      required:
> +        - aspeed,vuart-over-pci
> +    then:
> +      required:
> +        - aspeed,scu-syscon

[Severity: Low]
Does this allow logically contradictory device trees to pass schema
validation?

The VUART must be routed either over PCI or LPC, as UART FIFOs cannot
be safely shared between two concurrent host interfaces.

Should mutual exclusivity constraints be added here to reject
aspeed,lpc-io-reg and aspeed,lpc-interrupts when PCI mode is active?

> +      properties:
> +        compatible:
> +          contains:
> +            const: aspeed,ast2600-vuart
> +    else:
> +      properties:
> +        aspeed,scu-syscon: false

[ ... ]

> @@ -106,6 +128,9 @@ properties:
>        - const: ns16850
>        - const: aspeed,ast2400-vuart
>        - const: aspeed,ast2500-vuart
> +      - items:
> +          - const: aspeed,ast2600-vuart
> +          - const: aspeed,ast2500-vuart

[Severity: High]
Could this result in a regression if an older kernel boots and matches
the fallback?

An older kernel's aspeed_vuart_probe() driver does not recognize
aspeed,vuart-over-pci. It will fall back to its default LPC settings,
programming the VUART to route to the LPC bus at I/O port 0x3f8.

This fails to enable the intended PCI communication and actively drives
the LPC bus, potentially causing hardware conflicts or host crashes if
another device already occupies 0x3f8 on the LPC bus.

Should the fallback be omitted when aspeed,vuart-over-pci is set?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260818-aspeed-driver-v6-v6-0-fd7cc0b338ea@9elements.com?part=1