Re: [PATCH v4 3/3] arm64: dts: amlogic: add support for Orbbec Zora P1

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Hardware pin conflict on GPIOX_16, which is simultaneously assigned as an SDIO reset GPIO and a PWM_E clock output.
- [Low] The overridden 'led-blue' node fails to delete the inherited 'label' property, causing the LED sysfs device name to incorrectly remain 'n2:blue' and ignoring the new 'color' and 'function' properties.
--

--- Patch [3]: [PATCH v4 3/3] arm64: dts: amlogic: add support for Orbbec Zora P1 ---
commit 42dd3ecbc89a1e42aabbfd5aa86b80cf4d5cb181
Author: Alberto Robazza <robazzalberto@gmail.com>

arm64: dts: amlogic: add support for Orbbec Zora P1

The Orbbec Zora P1 is an A311D board modded from the ODROID-N2. This commit
adds a new device tree that builds on meson-g12b-a311d.dtsi and
meson-g12b-odroid-n2.dtsi, adding board-specific GPIO wiring for Wi-Fi/BT
and modifying the LED configuration.

> diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-zora-p1.dts b/arch/arm64/boot/dts/amlogic/meson-g12b-zora-p1.dts
> new file mode 100644
> index 0000000000000..6e696ddbcafea
> --- /dev/null
> +++ b/arch/arm64/boot/dts/amlogic/meson-g12b-zora-p1.dts

[ ... ]

> +	leds {
> +		/* Status led on Zora is actually green, same pin as N2, but reversed polarity */
> +		led-blue {
> +			function = LED_FUNCTION_STATUS;
> +			color = <LED_COLOR_ID_GREEN>;
> +			gpios = <&gpio_ao GPIOAO_11 GPIO_ACTIVE_LOW>;

[Severity: Low]
Will the LED subsystem ignore the new color and function properties here?

Since this node overrides led-blue from the base meson-g12b-odroid-n2.dtsi
(which sets label = "n2:blue"), not explicitly removing the label with
/delete-property/ label; means the original label is inherited.

In led_parse_fwnode_props(), if a label is present, it uses that exclusively
for the sysfs name and ignores the color and function properties.

> +		};
> +	};
> +
> +	sdio_pwrseq: sdio-pwrseq {
> +		compatible = "mmc-pwrseq-simple";
> +		reset-gpios = <&gpio GPIOX_16 GPIO_ACTIVE_LOW>;
> +		clocks = <&clock_32768>;
> +		clock-names = "ext_clock";
> +	};

[ ... ]

> +&pwm_ef {
> +	status = "okay";
> +	pinctrl-0 = <&pwm_e_pins>;

[Severity: High]
Does this create a hardware pin conflict on GPIOX_16?

The sdio_pwrseq node explicitly requests GPIOX_16 as a reset GPIO. At the
same time, pwm_ef claims pwm_e_pins. On Amlogic G12A/G12B platforms, the
pwm_e_pins group maps directly to GPIOX_16.

This looks like the exact same pin is requested for two incompatible roles,
which might conflict between the GPIO and pinctrl subsystems and prevent the
Wi-Fi module from working.

> +	pinctrl-names = "default";
> +};
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729141234.65037-1-robazzalberto@gmail.com?part=3

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic