Re: [PATCH v3 7/7] ARM: dts: aspeed: g6: add aspeed,vuart-over-pci prop to vuart3 and 4

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Low] Adding `aspeed,vuart-over-pci` to nodes with multiple compatible strings violates the `const` constraint in the YAML binding, causing a schema validation failure.

Pre-existing issues:
- [High] Probe error paths fail to unregister the 8250 port and remove the sysfs group, resulting in a use-after-free of the devm-allocated driver state and resource leaks.
--

commit 3ce60a66a65e6d307209da21c88e6e5c758ddf74
Author: Grégoire Layet <gregoire.layet@9elements.com>
Date:   Tue Jun 23 14:25:45 2026 +0000

    ARM: dts: aspeed: g6: add aspeed,vuart-over-pci prop to vuart3 and 4
    
    The VUART 3 and 4 are VUART over PCI.
    This flag indicates this information.

> diff --git a/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
> index 5ff52697e8bb4..b33468288d4a5 100644
> --- a/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
> +++ b/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
> @@ -714,6 +714,7 @@ vuart3: serial@1e787800 {
>  				interrupts = <GIC_SPI 180 IRQ_TYPE_LEVEL_HIGH>;
>  				clocks = <&syscon ASPEED_CLK_APB2>;
>  				no-loopback-test;
> +				aspeed,vuart-over-pci;

[Severity: Low]
Does adding this property cause a device tree schema validation failure?

Looking at Documentation/devicetree/bindings/serial/8250.yaml, the schema
restricts the compatible property to a single string when
aspeed,vuart-over-pci is present:

        compatible:
          const: aspeed,ast2600-vuart

Since the vuart3 node uses multiple compatible strings
("aspeed,ast2600-vuart", "aspeed,ast2500-vuart"), does this violate the
constraint and cause dtbs_check to fail?

[Severity: High]
This isn't a bug introduced by this patch, but adding this property appears
to trigger a pre-existing resource leak and use-after-free in the driver's
error path.

In drivers/tty/serial/8250/8250_aspeed_vuart.c, if
aspeed_ast2600_vuart_over_pci_set_enabled() fails during
aspeed_vuart_probe(), the function returns directly:

        rc = aspeed_ast2600_vuart_over_pci_set_enabled(pdev);
        if (rc) {
                dev_err(dev, "could not enable VUART over PCI\n");
                return rc;
        }

Does this bypass calling serial8250_unregister_port() and
sysfs_remove_group() after a successful serial8250_register_8250_port()?
Since the devm-allocated driver state is freed on probe failure, does the
UART core retain a registered port pointing to freed memory?

>  				status = "disabled";
>  			};

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1782224059.git.gregoire.layet@9elements.com?part=7