Re: [PATCH v4] arm64: dts: qcom: sm8550: add Xiaomi Pad 6S Pro (sheng)

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [High] Adding GPIO_OPEN_DRAIN to shared reset-gpios does not bypass the gpiolib exclusive request check, causing subsequent codec probes to fail with -EBUSY.
- [High] The reset-gpios for cs35l43 codecs use GPIO_ACTIVE_HIGH, which inverts the required active-low reset polarity.
- [High] Invalid compatible string `kinetic,ktz8866a` prevents the backlight driver from probing.
- [High] Missing pinctrl configurations for critical interrupt and reset GPIOs, which may leave inputs floating.
- [Medium] The commit violates the kernel's Developer Certificate of Origin (DCO) and Submitting Patches guidelines regarding identity and chain of custody.
- [Medium] The `output-high` properties were incorrectly applied to hardware-muxed pins in `i2s1_default` and were not removed as claimed in the changelog.
--

--- Patch [1]: [PATCH v4] arm64: dts: qcom: sm8550: add Xiaomi Pad 6S Pro (sheng) ---
commit 67dc6a3d188573154b653dea21a66bad2811a5b2
Author: map220v <map220v300@gmail.com>

arm64: dts: qcom: sm8550: add Xiaomi Pad 6S Pro (sheng)

Add device tree for Xiaomi Pad 6S Pro (sheng) based on SM8550.

> Author: map220v <map220v300@gmail.com>
[ ... ]
> Signed-off-by: code002-2 <hiez832670@gmail.com>

[Severity: Medium]
Do these identities meet the kernel's Developer Certificate of Origin (DCO)
requirements? Both the Author and the Signed-off-by tags appear to use
pseudonyms.

Furthermore, the Author's identity is missing from the Signed-off-by chain,
which breaks the custody tracking.

> diff --git a/arch/arm64/boot/dts/qcom/sm8550-xiaomi-sheng.dts b/arch/arm64/boot/dts/qcom/sm8550-xiaomi-sheng.dts
> new file mode 100644
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sm8550-xiaomi-sheng.dts
[ ... ]
> +&i2c1 {
> +	status = "okay";
> +
> +	backlight: backlight@11 {
> +		compatible = "kinetic,ktz8866a";

[Severity: High]
Does the upstream Kinetic KTZ8866 backlight driver recognize the 'a' suffix?

The driver in drivers/video/backlight/ktz8866.c and its bindings seem to only
support "kinetic,ktz8866". Without a fallback, could this silently fail to
probe and leave the display unlit?

[ ... ]
> +&i2c5 {
> +	clock-frequency = <1000000>;
> +	status = "okay";
> +
> +	cs35l43_trl: cs35l43@40 {
> +		compatible = "cirrus,cs35l43";
> +		reg = <0x40>;
> +		interrupt-parent = <&tlmm>;
> +		interrupts = <98 IRQ_TYPE_LEVEL_LOW>;
> +		reset-gpios = <&tlmm 90 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;

[Severity: High]
Does adding GPIO_OPEN_DRAIN bypass the gpiolib exclusive request check?

Since all six cs35l43 codec nodes share this exact reset GPIO, gpiolib will
likely return -EBUSY for all subsequent codec probes after the first one
succeeds.

[Severity: High]
Also, does (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN) invert the intended active-low
reset polarity?

By specifying GPIO_ACTIVE_HIGH, gpiolib will translate a driver's logical 0
into a physical LOW, which might hold the codecs in reset permanently.

[ ... ]
> +&spi4 {
> +	status = "okay";
> +
> +	touchscreen@0 {
> +		compatible = "novatek,NVT-ts-spi";
> +		reg = <0>;
> +
> +		spi-max-frequency = <19200000>;
> +		novatek,irq-gpio = <&tlmm 25 GPIO_ACTIVE_LOW>;

[Severity: High]
Is a pinctrl configuration missing for this interrupt pin?

The device tree relies on interrupt pins like gpio25 for the touchscreen,
gpio62 for the MCU, codec interrupts, and the shared reset pin gpio90, but
fails to define and attach pinctrl states for them.

Leaving interrupt pins floating without explicit biases risks spurious
interrupt storms, and unconfigured outputs risk failing to reset hardware
correctly.

[ ... ]
> +	i2s1_default: i2s1-default-state {
> +		sck-pins {
> +			pins = "gpio121";
> +			function = "i2s1_sck";
> +			drive-strength = <8>;
> +			bias-disable;
> +			output-high;
> +		};
> +
> +		data0-pins {
> +			pins = "gpio122";
> +			function = "i2s1_data0";
> +			drive-strength = <8>;
> +			bias-disable;
> +		};
> +
> +		ws-pins {
> +			pins = "gpio123";
> +			function = "i2s1_ws";
> +			drive-strength = <8>;
> +			bias-disable;
> +			output-high;
> +		};
> +
> +		data1-pins {
> +			pins = "gpio124";
> +			function = "i2s1_data1";
> +			drive-strength = <8>;
> +			bias-disable;
> +			output-high;
> +		};
> +	};

[Severity: Medium]
Are these output-high properties correctly applied to hardware-muxed pins?

In Qualcomm pinctrl, static output states should typically only be configured
when function = "gpio". Applying output-high to hardware-driven pins can
conflict with the hardware controller block.

The commit message also mentions removing output-low/output-high from TLMM
pinctrls, but they appear to still be present here.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721145822.1278-1-code_0002@163.com?part=1