Re: [PATCH v5 10/10] arm64: dts: Add support for LX2160 Twins board in single configuration

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Undocumented and unrelated SoC revision change for the Clearfog CX board.
- [High] The PCI-E OCP card connector is not enabled in the device tree.
- [Low] Duplicate `function-enumerator` value in the SFP LED definitions.
- [Low] The `dpmac` nodes referenced as LED `trigger-sources` lack the mandatory `#trigger-source-cells` property.
- [Low] The `gpio-mux` configured for PWM fan monitoring has no label and no consumer node.
--

commit e131b341e534ec21c95289bd4bedcef8bb6fcf54
Author: Josua Mayer <josua@solid-run.com>

arm64: dts: Add support for LX2160 Twins board in single configuration
    
This commit adds support for the SolidRun LX2160A Twins board in its
single CPU configuration. The board features several connectors including
SFP, PCIe OCP, USB-3.0, and microSD.

> - PCI-E OCP card connector

The commit message explicitly lists the PCI-E OCP card connector as a
supported feature. However, looking at the device tree, are there any PCIe
host controller nodes enabled?

All PCIe host controller nodes default to disabled in the base SoC device
trees, and this device tree doesn't seem to override their status to "okay".

> diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a-clearfog-cx.dts b/arch/arm64/boot/dts/freescale/fsl-lx2160a-clearfog-cx.dts
> index 802d7611c6479..6078ce47fabf1 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-lx2160a-clearfog-cx.dts
> +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a-clearfog-cx.dts
> @@ -6,7 +6,7 @@
>  
>  /dts-v1/;
>  
> -#include "fsl-lx2160a.dtsi"
> +#include "fsl-lx2160a-rev2.dtsi"

Does this commit intend to change the SoC revision for the Clearfog CX board?

This hardware definition change is not documented in the commit message.

> diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a-half-twins.dts b/arch/arm64/boot/dts/freescale/fsl-lx2160a-half-twins.dts
> new file mode 100644
> index 0000000000000..434b3f4873008
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a-half-twins.dts

[ ... ]

> +	leds {
> +		compatible = "gpio-leds";

[ ... ]

> +		led_ht_c3_bb: led-sfp-2 {
> +			color = <LED_COLOR_ID_GREEN>;
> +			default-state = "off";
> +			function = LED_FUNCTION_LAN;
> +			function-enumerator = <2>;
> +			gpios = <&expander3 13 GPIO_ACTIVE_LOW>;
> +			trigger-sources = <&dpmac15>;
> +			linux,default-trigger = "netdev";
> +		};
> +
> +		led_ht_c3_at: led-sfp-3 {
> +			color = <LED_COLOR_ID_GREEN>;
> +			default-state = "off";
> +			function = LED_FUNCTION_LAN;
> +			function-enumerator = <2>;

Does this node have a duplicate function-enumerator?

It shares the same value as the preceding led_ht_c3_bb (led-sfp-2) node.

> +			gpios = <&expander3 11 GPIO_ACTIVE_LOW>;
> +			trigger-sources = <&dpmac6>;

Do the referenced dpmac nodes lack the #trigger-source-cells property?

According to the standard LED trigger-source binding, any provider node must
define this property. The base dpmac nodes in fsl-lx2160a.dtsi do not appear
to define it.

[ ... ]

> +	mux-controller {
> +		compatible = "gpio-mux";
> +		#mux-control-cells = <0>;
> +		/*
> +		 * This gpio controlled mux can route the tacho signals of 6 PWM FAN connectors
> +		 * to the tacho inputs of both CEX-7 modules (twins).
> +		 *
> +		 * The first twin controls this mux and monitors four fan connectors, two intended
> +		 * for itself, and two for the OCP card.
> +		 *
> +		 * The second twin monitors only two fan connectors intended for itself.
> +		 *
> +		 * The table below maps selector GPIO states to monitored fan connector per twin:
> +		 *
> +		 * | SEL1 | SEL0 | Twin 1 | Twin 2 |
> +		 * | ---: | ---: | :------| ------ |
> +		 * |    0 |    0 | J10    |  J5024 |
> +		 * |    0 |    1 | J5016  |  J5024 |
> +		 * |    1 |    0 | J5026  |  J5025 |
> +		 * |    1 |    1 | J5013  |  J5025 |
> +		 */
> +		mux-gpios = <&expander0 8 GPIO_ACTIVE_HIGH>, /* SEL0 */
> +			    <&expander0 15 GPIO_ACTIVE_HIGH>; /* SEL1 */
> +	};

Is this mux-controller missing a label and a consumer node?

There are no hwmon, fan, or other consumer nodes defined that reference it.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260510-lx2160-pci-v5-0-540b83852227@solid-run.com?part=10