Re: [PATCH v2 2/3] arm64: dts: freescale: add Aquila iMX95 support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] The pinctrl nodes for 8-bit and 4-bit FlexSPI configurations share the exact same node name, causing unintended merging and overwriting.
- [High] The DisplayPort bridge (`som_dsi2dp_bridge`) input endpoint is disconnected, rendering the DisplayPort output non-functional.
- [Low] Cadence-specific QSPI timing properties are defined within a flash node connected to an NXP FlexSPI controller.
- [Low] The commit message claims support for hardware interfaces (10GbE, LVDS, CSI) that are omitted from the Device Tree.
--

commit cac45cd52355d1edaa432d9d405d04c96f38d7b2
Author: João Paulo Gonçalves <joao.goncalves@toradex.com>

arm64: dts: freescale: add Aquila iMX95 support

This commit adds device tree support for the Toradex Aquila iMX95 computer
on module and its corresponding development carrier board.

Link: https://www.toradex.com/computer-on-modules/aquila-arm-family/nxp-imx95
Link: https://www.toradex.com/products/carrier-board/aquila-development-board-kit

>     The module consists of an NXP i.MX95 family SoC, up to 16GB LPDDR5 RAM,
>     up to 128GB of storage, a USB 3.2 OTG and USB 2.0 Host, a Gigabit
>     Ethernet PHY, a 10 Gigabit Ethernet interface, an I2C EEPROM and
>     Temperature Sensor, an RX8130 RTC, one Quad lane CSI interface, one Quad
>     lane DSI or CSI interface, one LVDS interface (one or two channels), and
>     some optional addons: DisplayPort (through a DSI-DP bridge), TPM 2.0,
>     and a WiFi/BT module.

[Severity: Low]
Does this commit message accurately reflect the contents of the patch?

The commit message claims support for a 10 Gigabit Ethernet interface, one
LVDS interface, and one Quad lane CSI interface, but there doesn't seem to be
any device tree node configurations, enablement, or pin multiplexing for these
interfaces.

> diff --git a/arch/arm64/boot/dts/freescale/imx95-aquila-dev.dts b/arch/arm64/boot/dts/freescale/imx95-aquila-dev.dts
> new file mode 100644
> index 0000000000000..b2a53847d5079
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx95-aquila-dev.dts
[ ... ]
> +	flash@0 {
> +		compatible = "jedec,spi-nor";
> +		reg = <0x0>;
> +		spi-max-frequency = <66000000>;
> +		spi-rx-bus-width = <4>;
> +		spi-tx-bus-width = <4>;
> +		cdns,read-delay = <0>;
> +		cdns,tchsh-ns = <3>;
> +		cdns,tsd2d-ns = <10>;
> +		cdns,tshsl-ns = <30>;
> +		cdns,tslch-ns = <8>;

[Severity: Low]
Is it necessary to include these Cadence-specific timing properties here?

The parent controller is the NXP FlexSPI controller, which doesn't appear to
use or parse the cdns,* properties.

> +	};
> +};

> diff --git a/arch/arm64/boot/dts/freescale/imx95-aquila.dtsi b/arch/arm64/boot/dts/freescale/imx95-aquila.dtsi
> new file mode 100644
> index 0000000000000..c6dc48852ac6c
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx95-aquila.dtsi
[ ... ]
> +			port@0 {
> +				reg = <0>;
> +
> +				dsi2dp_in: endpoint {
> +				};

[Severity: High]
Does this DisplayPort bridge configuration work without a remote-endpoint?

The input endpoint dsi2dp_in is declared without any remote-endpoint
linking it to the SoC's DSI controller, which breaks the video pipeline and
prevents the DisplayPort output from functioning.

> +			};
> +
> +			port@1 {
> +				reg = <1>;
[ ... ]
> +	/* Aquila QSPI_1 (8 bit) */
> +	pinctrl_flexspi1_8bit: flexspi18bitgrp {
> +		fsl,pins = <IMX95_PAD_XSPI1_SCLK__FLEXSPI1_A_SCLK	0x3fe>, /* Aquila B65 */
> +			   <IMX95_PAD_XSPI1_DATA0__FLEXSPI1_A_DATA_BIT0	0x3fe>, /* Aquila B68 */
> +			   <IMX95_PAD_XSPI1_DATA1__FLEXSPI1_A_DATA_BIT1	0x3fe>, /* Aquila B67 */
> +			   <IMX95_PAD_XSPI1_DATA2__FLEXSPI1_A_DATA_BIT2	0x3fe>, /* Aquila B61 */
> +			   <IMX95_PAD_XSPI1_DATA3__FLEXSPI1_A_DATA_BIT3	0x3fe>, /* Aquila B60 */
> +			   <IMX95_PAD_XSPI1_DATA4__FLEXSPI1_A_DATA_BIT4	0x3fe>, /* Aquila B70 */
> +			   <IMX95_PAD_XSPI1_DATA5__FLEXSPI1_A_DATA_BIT5	0x3fe>, /* Aquila B71 */
> +			   <IMX95_PAD_XSPI1_DATA6__FLEXSPI1_A_DATA_BIT6	0x3fe>, /* Aquila B72 */
> +			   <IMX95_PAD_XSPI1_DATA7__FLEXSPI1_A_DATA_BIT7	0x3fe>, /* Aquila B73 */
> +			   <IMX95_PAD_XSPI1_DQS__FLEXSPI1_A_DQS		0x3fe>; /* Aquila B63 */
> +	};
> +
> +	/* Aquila QSPI_1 (4 bit) */
> +	pinctrl_flexspi1_4bit: flexspi18bitgrp {

[Severity: High]
Is this node name meant to be different from the 8-bit configuration above?

Because this 4-bit configuration shares the exact same node name
(flexspi18bitgrp) as the 8-bit configuration under the same parent
(&scmi_iomuxc), the device tree compiler will merge them.

This silently overwrites the 8-bit configuration with the 4-bit properties,
causing 8-bit QSPI transfers to fail because data pins 4-7 are left
unconfigured.

> +		fsl,pins = <IMX95_PAD_XSPI1_SCLK__FLEXSPI1_A_SCLK	0x3fe>, /* Aquila B65 */
> +			   <IMX95_PAD_XSPI1_DATA0__FLEXSPI1_A_DATA_BIT0	0x3fe>, /* Aquila B68 */
> +			   <IMX95_PAD_XSPI1_DATA1__FLEXSPI1_A_DATA_BIT1	0x3fe>, /* Aquila B67 */
> +			   <IMX95_PAD_XSPI1_DATA2__FLEXSPI1_A_DATA_BIT2	0x3fe>, /* Aquila B61 */
> +			   <IMX95_PAD_XSPI1_DATA3__FLEXSPI1_A_DATA_BIT3	0x3fe>, /* Aquila B60 */
> +			   <IMX95_PAD_XSPI1_DQS__FLEXSPI1_A_DQS		0x3fe>; /* Aquila B63 */
> +	};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520-add-aquila-imx95-v2-0-06424a51e33a@toradex.com?part=2