Re: [PATCH v4 4/4] arm64: dts: qcom: sm8750-lenovo-elden: describe the WL2868C regulators

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] arm64: dts: qcom: undocumented camera/lens compatibles and unlinked endpoint
- [Medium] arm64: dts: qcom: new sm8750-lenovo-elden board DT lacks boot infrastructure
- [Medium] arm64: dts: qcom: sm8750 cci0_i2c1 and cci1_i2c1 disabled despite bus 1 pinctrl
--

commit 22e373234557a88250e1a13f2288188945ab03f9
Author: Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>

arm64: dts: qcom: sm8750-lenovo-elden: describe the WL2868C regulators

This commit introduces the sm8750-lenovo-elden devicetree for the Lenovo
Legion Tab (TB322FC), describing its board clocks, the WL2868C camera PMIC
regulators on i2c10, and the rear camera sensor and lens on cci0_i2c0.

> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> index 1c86e7e98f55d..a18fea7bebaf7 100644
> --- a/arch/arm64/boot/dts/qcom/Makefile
> +++ b/arch/arm64/boot/dts/qcom/Makefile
> @@ -406,6 +406,7 @@ dtb-$(CONFIG_ARCH_QCOM)	+= sm8650-hdk-rear-camera-card.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= sm8650-hdk.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= sm8650-mtp.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= sm8650-qrd.dtb
> +dtb-$(CONFIG_ARCH_QCOM)	+= sm8750-lenovo-elden.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= sm8750-mtp.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= sm8750-qrd.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= talos-evk.dtb
> diff --git a/arch/arm64/boot/dts/qcom/sm8750-lenovo-elden.dts b/arch/arm64/boot/dts/qcom/sm8750-lenovo-elden.dts
> new file mode 100644
> index 0000000000000..de186b447725d
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sm8750-lenovo-elden.dts
> @@ -0,0 +1,175 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Copyright (c) 2026, Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>
> + */
> +
> +/dts-v1/;
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include "sm8750.dtsi"
> +
> +/ {
> +	model = "Lenovo Legion Y700 Gen4";
> +	compatible = "lenovo,elden", "qcom,sm8750";
> +	chassis-type = "tablet";

[Severity: Medium]
The commit subject describes this patch as adding the WL2868C regulators to
sm8750-lenovo-elden, but the patch creates arch/arm64/boot/dts/qcom/sm8750-lenovo-elden.dts
from scratch and adds sm8750-lenovo-elden.dtb to arch/arm64/boot/dts/qcom/Makefile.

Unlike the existing sm8750-mtp.dts and sm8750-qrd.dts board files, this new
top-level board devicetree only defines the fixed board clocks, cci0,
gpi_dma2/qupv3_2/i2c10, and tlmm camera pins. It does not include any SPMI
PMICs (such as sm8750-pmics.dtsi), RPMh regulators under apps_rsc (including
the parent VIN supplies for the WL2868C PMIC), a chosen stdout-path console,
or any boot/storage peripherals.

Should this patch series include the base board infrastructure needed to boot
the Lenovo Legion Y700 Gen4, or is a base board patch missing from the series?

[ ... ]

> +&cci0 {
> +	status = "okay";
> +};

[Severity: Medium]
When cci0 is enabled here, cci_probe() in drivers/i2c/busses/i2c-qcom-cci.c
applies the default pinctrl state from arch/arm64/boot/dts/qcom/sm8750.dtsi:

	pinctrl-0 = <&cci0_0_default &cci0_1_default>;
	pinctrl-1 = <&cci0_0_sleep &cci0_1_sleep>;

which claims and muxes the pins for both bus 0 (gpio113/114) and bus 1
(gpio115/116).

However, commit b3f712e6e2f5 ("arm64: dts: qcom: sm8750: describe two of the
three CCI I2C controllers") in this series defines cci0_i2c1 and cci1_i2c1 in
arch/arm64/boot/dts/qcom/sm8750.dtsi with status = "disabled":

	cci0_i2c1: i2c-bus@1 {
		status = "disabled";
		reg = <1>;
		clock-frequency = <400000>;
		#address-cells = <1>;
		#size-cells = <0>;
	};

even though its commit message states that both buses are left available as on
sm8550 and sm8650. Because cci_probe() iterates over child nodes with
for_each_available_child_of_node_scoped(), any board populating cci0_i2c1 or
cci1_i2c1 without also overriding status = "okay" on the i2c-bus@1 node will
have bus 1 skipped while its pins are still muxed by the parent CCI node.

Should status = "disabled" be dropped from cci0_i2c1 and cci1_i2c1 in
sm8750.dtsi to match sm8550.dtsi, sm8650.dtsi, and the default pinctrl
configuration?

> +
> +&cci0_i2c0 {
> +	clock-frequency = <400000>;
> +
> +	rear_lens: lens@c {
> +		compatible = "giantec,gt9764";
> +		reg = <0x0c>;
> +		vddio-supply = <&camera_ldo7>;
> +		vdd-supply = <&camera_ldo5>;
> +		giantec,aac-mode = <0x40>;
> +		giantec,aac-timing = <0x7c>;
> +		settling-time-us = <7000>;
> +	};
> +
> +	rear_camera: camera@56 {
> +		compatible = "samsung,s5kjns";
> +		reg = <0x56>;
> +		clocks = <&cambistmclkcc CAM_BIST_MCLK_CC_MCLK3_CLK>;
> +		assigned-clocks = <&cambistmclkcc CAM_BIST_MCLK_CC_MCLK3_CLK>;
> +		assigned-clock-rates = <19200000>;
> +		reset-gpios = <&tlmm 11 GPIO_ACTIVE_LOW>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&rear_camera_default>;
> +		dovdd-supply = <&camera_ldo7>;
> +		dvdd-supply = <&camera_ldo1>;
> +		avdd-supply = <&camera_ldo4>;
> +		lens-focus = <&rear_lens>;
> +		orientation = <1>;
> +		rotation = <90>;
> +
> +		port {
> +			rear_camera_ep: endpoint {
> +				data-lanes = <1 2 3 4>;
> +				link-frequencies = /bits/ 64 <672000000>;
> +			};
> +		};
> +	};
> +};

[Severity: Medium]
The commit message notes that the front camera node was deferred because its
binding is still pending, but rear_lens (lens@c) and rear_camera (camera@56)
under cci0_i2c0 also use compatibles and properties that are not documented
in Documentation/devicetree/bindings/:

1. In rear_lens, compatible = "giantec,gt9764" along with vddio-supply,
   giantec,aac-mode, giantec,aac-timing, and settling-time-us are not
   documented in any binding schema. Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml
   only documents dongwoon,dw9768 and giantec,gt9769 with vin-supply,
   vdd-supply, dongwoon,aac-mode, and dongwoon,aac-timing.

2. In rear_camera, compatible = "samsung,s5kjns" along with dovdd-supply,
   dvdd-supply, and avdd-supply are not documented in any binding schema.
   Documentation/devicetree/bindings/media/i2c/samsung,s5kjn1.yaml only
   documents samsung,s5kjn1 with vdda-supply, vddd-supply, vddio-supply,
   and afvdd-supply.

3. In rear_camera_ep, the endpoint defines data-lanes and link-frequencies
   without a remote-endpoint link because sm8750.dtsi does not yet have the
   CAMSS / CSIPHY nodes.

Will these nodes cause make dtbs_check validation failures until the
corresponding bindings and CSI receiver endpoints are added?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1793427040.git.xiaqinghuan@mail.ustc.edu.cn?part=4