Re: [PATCH v5 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
- [Low] arm64: dts: qcom: mix of DT binding schema change and board DTS in one commit
--

commit 247a3e361ec98ef920ebf8c9ec415c99b8590dce
Author: Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>

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

This commit adds the lenovo,elden compatible to the Qualcomm ARM board
bindings and introduces a device tree for the Lenovo Legion Y700 Gen4
(SM8750) describing its board clocks, WL2868C camera PMIC regulators, and
rear camera sensor and lens.

> diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
> index ff8a35df7862d..41d5e65c4eea5 100644
> --- a/Documentation/devicetree/bindings/arm/qcom.yaml
> +++ b/Documentation/devicetree/bindings/arm/qcom.yaml
> @@ -1168,6 +1168,7 @@ properties:
>  
>        - items:
>            - enum:
> +              - lenovo,elden
>                - qcom,sm8750-mtp
>                - qcom,sm8750-qrd
>            - const: qcom,sm8750
> 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

[Severity: Low]
Should the Documentation/devicetree/bindings/arm/qcom.yaml schema change be
split into a separate prerequisite patch with a dt-bindings: subject prefix
rather than combined in the same commit as the arch/arm64/boot/dts/qcom/
changes, per Documentation/devicetree/bindings/submitting-patches.rst?

[ ... ]

> 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";

[ ... ]

> +&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]
As reported by Sashiko on v4 [1], does adding rear_lens (lens@c) and
rear_camera (camera@56) under &cci0_i2c0 in
arch/arm64/boot/dts/qcom/sm8750-lenovo-elden.dts cause make dtbs_check and
scripts/checkpatch.pl regressions because their bindings are not in the tree?

Specifically, rear_lens uses compatible = "giantec,gt9764" with properties
vddio-supply, giantec,aac-mode, giantec,aac-timing, and settling-time-us,
while 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.

Similarly, rear_camera uses compatible = "samsung,s5kjns" with dovdd-supply,
dvdd-supply, and avdd-supply, which are not documented in any in-tree schema
(Documentation/devicetree/bindings/media/i2c/samsung,s5kjn1.yaml only
documents samsung,s5kjn1 with vdda-supply, vddd-supply, vddio-supply, and
afvdd-supply).

In addition, rear_camera_ep defines a port endpoint without a remote-endpoint
property because sm8750.dtsi does not yet have a CAMSS or CSIPHY receiver
node. Should rear_lens and rear_camera be deferred until their DT bindings
and the SM8750 CAMSS receiver nodes are present in the tree, just as was done
for the front sensor?

> +
> +&gpi_dma2 {
> +	status = "okay";
> +};
> +
> +&qupv3_2 {
> +	status = "okay";
> +};
> +
> +&i2c10 {
> +	status = "okay";
> +
> +	pmic@2f {
> +		compatible = "willsemi,wl2868c";
> +		reg = <0x2f>;
> +		reset-gpios = <&tlmm 126 GPIO_ACTIVE_LOW>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&camera_pmic_default>;

[Severity: Medium]
As reported by Sashiko on v4 [2], is it expected to introduce
arch/arm64/boot/dts/qcom/sm8750-lenovo-elden.dts as a new top-level board DTB
containing only clocks and camera I2C/PMIC nodes without the foundational
board infrastructure?

Unlike sm8750-mtp.dts and sm8750-qrd.dts, this file omits the SPMI PMICs,
RPMh regulators (&apps_rsc, including the vin12-supply, vin34-supply,
vin5-supply, and vin7-supply parent supplies for pmic@2f itself), chosen
stdout-path console, and boot storage peripherals, leaving a non-bootable
board DTB where the WL2868C regulator driver binds against dummy parent
supplies.

Also, in sm8750-lenovo-elden.dts, &qupv3_2 is placed before &i2c10 rather
than in alphabetical label order, and rear_lens places the standard property
settling-time-us after the vendor-specific giantec,aac-mode and
giantec,aac-timing properties. Should this patch wait for the base board
enablement series and follow the ordering rules in
Documentation/devicetree/bindings/dts-coding-style.rst?

[1] https://lore.kernel.org/r/9ef48d26aa4480b2dc1e20ad7af55f51aa22fec7.1793427041.git.xiaqinghuan@mail.ustc.edu.cn
[2] https://lore.kernel.org/r/9ef48d26aa4480b2dc1e20ad7af55f51aa22fec7.1793427041.git.xiaqinghuan@mail.ustc.edu.cn

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