Re: [PATCH v12 2/2] arm64: dts: rockchip: Add ALIENTEK QuarkPi-CA2

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] arm64: dts: rockchip: misplaced hp_con pinctrl and duplicate DAPM routes
--

commit 07118101f721e520605d0c72d4bd5b261e3be1ae
Author: Bob Hui <bg9oxa@163.com>

arm64: dts: rockchip: Add ALIENTEK QuarkPi-CA2

This commit adds the device tree for the ALIENTEK QuarkPi-CA2, an RK3588S
based card computer.

> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-quarkpi-ca2.dts b/arch/arm64/boot/dts/rockchip/rk3588s-quarkpi-ca2.dts
> new file mode 100644
> index 0000000000000..c9cd01ba3869c
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-quarkpi-ca2.dts
> @@ -0,0 +1,1209 @@

[ ... ]

> +	analog_sound: analog-sound {
> +		compatible = "simple-audio-card";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&hp_det &hp_con>;
> +		simple-audio-card,name = "QuarkPi-CA2 ES8388";
> +		simple-audio-card,format = "i2s";
> +		simple-audio-card,bitclock-master = <&sound_cpu>;
> +		simple-audio-card,frame-master = <&sound_cpu>;
> +		simple-audio-card,mclk-fs = <256>;
> +		simple-audio-card,aux-devs = <&hp_amp>;
> +		simple-audio-card,hp-det-gpios = <&gpio1 RK_PC4 GPIO_ACTIVE_HIGH>;
> +		simple-audio-card,pin-switches = "Headphones", "Headset Mic";
> +		simple-audio-card,widgets =
> +			"Headphone", "Headphones",
> +			"Microphone", "Headset Mic";
> +		simple-audio-card,routing =
> +			"Headphones", "LOUT1",
> +			"Headphones", "ROUT1",
> +			"Headphones", "Headphone Power OUTL",
> +			"Headphones", "Headphone Power OUTR",
> +			"Headphone Power INL", "LOUT1",
> +			"Headphone Power INR", "ROUT1",
> +			"LINPUT1", "Headset Mic",
> +			"RINPUT1", "Headset Mic";
> +
> +		sound_codec: simple-audio-card,codec {
> +			sound-dai = <&es8388>;
> +			system-clock-frequency = <12288000>;
> +		};
> +
> +		sound_cpu: simple-audio-card,cpu {
> +			sound-dai = <&i2s0_8ch>;
> +		};
> +	};
> +
> +	hp_amp: audio-amplifier {
> +		compatible = "simple-audio-amplifier";
> +		sound-name-prefix = "Headphone Power";
> +		enable-gpios = <&gpio1 RK_PC6 GPIO_ACTIVE_HIGH>;
> +	};

[Severity: Low]
In analog_sound and hp_amp, hp_con configures gpio1 RK_PC6 with
pcfg_pull_down, which is the enable-gpios pin requested by hp_amp in
simple_amp_probe().

Because pinctrl_bind_pins() applies pinctrl-0 per platform device before
probe(), if hp_amp probes before analog_sound (or while analog_sound defers
waiting for es8388 or i2s0_8ch), hp_con is not bound to hp_amp when
devm_gpiod_get_optional() requests gpio1 RK_PC6. Should hp_con be moved from
analog_sound's pinctrl-0 to hp_amp instead?

Also, in analog_sound's simple-audio-card,routing property, LOUT1 and ROUT1
are routed both directly to Headphones and through the Headphone Power
amplifier widgets (Headphone Power INL/INR -> Headphone Power OUTL/OUTR ->
Headphones), creating parallel direct and amplified paths in the DAPM graph.
Should the direct "Headphones", "LOUT1" and "Headphones", "ROUT1" routes be
dropped so the signal only goes through the headphone amplifier?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102-quarkpi-ca2-v12-0-d749d9bb2bf9@163.com?part=2