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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] The device tree heavily relies on undocumented downstream compatibles that lack mainline drivers or bindings.
- [Medium] Missing Developer Certificate of Origin (DCO) Sign-off promised in the changelog.
- [Low] Unidiomatic deletion and redefinition of a pinctrl node.
--

--- Patch [1]: [PATCH v6] arm64: dts: qcom: sm8550: add Xiaomi Pad 6S Pro (sheng) ---
commit 0ac17b1bf2e0cc5382a800a4519a79a941821b31
Author: Tianyuan Jiang <hiez832670@gmail.com>

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

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

>     Signed-off-by: Tianyuan Jiang <code_0002@163.com>
> 
> ---
> v6: address Sashiko AI review on v4
> - Fix cs35l43 reset-gpios polarity: use GPIO_ACTIVE_LOW
> - Add "kinetic,ktz8866" fallback compatible for backlight
> - Add map220v Signed-off-by (DCO chain fix)

[Severity: Medium]
The changelog mentions adding a Signed-off-by tag for map220v to address a
DCO chain issue, but it appears to be missing from the commit trailers.
Should the original author's sign-off be included in the commit message?

> 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
> index 0000000000000..fa24b1c1b4e65
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sm8550-xiaomi-sheng.dts
> @@ -0,0 +1,1761 @@

[ ... ]

> +/delete-node/ &hyp_ext_tags_mem;
> +/delete-node/ &hyp_ext_reserved_mem;
> +
> +/delete-node/ &qup_i2c8_data_clk;
> +
> +/delete-node/ &qup_i2c9_data_clk;

[Severity: Low]
Is there a specific reason to delete these pinctrl nodes at the root level
and manually redefine them later in the file? It is usually more idiomatic
in device trees to override the properties directly using a reference
(for example, &qup_i2c8_data_clk { ... }) instead of fully deleting and
redefining them.

[ ... ]

> +&i2c5 {
> +	clock-frequency = <1000000>;
> +	status = "okay";
> +
> +	cs35l43_trl: cs35l43@40 {
> +		compatible = "cirrus,cs35l43";

[Severity: Medium]
Does this compatible string have a corresponding upstream binding or driver?
Using undocumented downstream compatibles, along with several others in this
patch (such as nanosic,wn8030-sheng, novatek,nt36532e, and fpc,fpc1020),
causes validation failures with dtbs_check since their custom properties
are not defined in any schema.

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