Re: [PATCH 4/4] arm64: dts: qcom: sc8280xp: gaokun3: describe rear camera module information as musch as possible

Vladimir Zapolskiy
2026-04-30T11:00:44Z

On 4/25/26 13:53, Pengyu Luo wrote:
> The rear sensor is S5K3L6, describing it but dropping compatible
> string, since there is no upstream driver. A funcitonal downstream
> driver is in comment.
>
> The VCM is dw9714, describe it.
>
> Signed-off-by: Pengyu Luo <mitltlatltl@gmail.com>
> ---
> Please take this patch as a RFC, I am not sure, how much I am allowed
> to add without a sensor driver.
> ---
> .../boot/dts/qcom/sc8280xp-huawei-gaokun3.dts | 129 +++++++++++++++++-
> 1 file changed, 123 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-huawei-gaokun3.dts b/arch/arm64/boot/dts/qcom/sc8280xp-huawei-gaokun3.dts
> index 39e559e91289..76b1ecb3819d 100644
> --- a/arch/arm64/boot/dts/qcom/sc8280xp-huawei-gaokun3.dts
> +++ b/arch/arm64/boot/dts/qcom/sc8280xp-huawei-gaokun3.dts
> @@ -22,12 +22,18 @@
> #include "sc8280xp.dtsi"
> #include "sc8280xp-pmics.dtsi"
>
> +/* remove due to gpio pins collision, skip 2nd instance won't break things */
> +/delete-node/ &cci1_i2c1;
> +/delete-node/ &cci1_i2c1_default;
> +/delete-node/ &cci1_i2c1_sleep;

Instead of removal 'cci1_default' and 'cci1_sleep' nodes shall be rewritten
by excluding 'cci1_i2c1_default' and 'cci1_i2c1_sleep' from them.

> +
> / {
> chassis-type = "tablet";
> model = "Matebook E Go";
> compatible = "huawei,gaokun3", "qcom,sc8280xp";
>
> aliases {
> + i2c1 = &cci1_i2c0;

Likely this I2C alias can be removed.

> i2c2 = &cci2_i2c1;
> i2c4 = &i2c4;
> i2c15 = &i2c15;
> @@ -52,9 +58,17 @@ framebuffer0: framebuffer@c6200000 {
> leds {
> compatible = "gpio-leds";
>
> - pinctrl-0 = <&cam_indicator_en>;
> + pinctrl-0 = <&cam_indicator_en>, <&camera_flash_en>;
> pinctrl-names = "default";
>
> + camera_flash: led {
> + function = LED_FUNCTION_FLASH;
> + color = <LED_COLOR_ID_WHITE>;
> + gpios = <&tlmm 93 GPIO_ACTIVE_HIGH>;
> + linux,default-trigger = "none";
> + default-state = "off";
> + };
> +
> privacy_led: privacy-led {
> function = LED_FUNCTION_INDICATOR;
> color = <LED_COLOR_ID_WHITE>;
> @@ -129,6 +143,18 @@ vreg_camf_1p2: regulator-camf-1p2 {
> pinctrl-names = "default";
> };
>
> + vreg_camr: regulator-camr {
> + compatible = "regulator-fixed";
> +
> + regulator-name = "vreg_camr";
> +
> + gpio = <&tlmm 92 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> +
> + pinctrl-0 = <&camr_reg_en>;
> + pinctrl-names = "default";
> + };
> +
> vreg_misc_3p3: regulator-misc-3p3 {
> compatible = "regulator-fixed";
>
> @@ -387,8 +413,8 @@ vreg_l1b: ldo1 {
>
> vreg_l2b: ldo2 {
> regulator-name = "vreg_l2b";
> - regulator-min-microvolt = <1904000>;
> - regulator-max-microvolt = <1904000>;
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <2800000>;
> regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> };
>
> @@ -433,10 +459,9 @@ vreg_l6b: ldo6 {
>
> vreg_l7b: ldo7 {
> regulator-name = "vreg_l7b";
> - regulator-min-microvolt = <1800000>;
> - regulator-max-microvolt = <1800000>;
> + regulator-min-microvolt = <2800000>;
> + regulator-max-microvolt = <2800000>;
> regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> - regulator-boot-on;
> };

ldo2 and ldo7 changes shall be done in separate commits.

>
> vreg_l9b: ldo9 {
> @@ -622,6 +647,16 @@ &camss {
> status = "okay";
>
> ports {
> + port@0 {
> + csiphy0_ep: endpoint@0 {
> + reg = <0>;
> +
> + clock-lanes = <7>;

Please remove 'clock-lanes' property here.

> + data-lanes = <0 1 2 3>;
> + remote-endpoint = <&s5k3l6_ep>;
> + };
> + };
> +
> port@3 {
> csiphy3_ep: endpoint@0 {
> reg = <0>;
> @@ -634,6 +669,58 @@ csiphy3_ep: endpoint@0 {
> };
> };
>
> +&cci1 {
> + status = "okay";
> +};
> +
> +&cci1_i2c0 {
> + voice_coil_motor: vcm@c {
> + compatible = "dongwoon,dw9714";
> + reg = <0xc>;
> + vcc-supply = <&vreg_l7b>; /* FIXME: require l2c on first */
> + };
> +
> + /*
> + * https://source.puri.sm/Librem5/linux/-/blob/pureos/latest/drivers/media/i2c/s5k3l6xx.c
> + *
> + * This sensor has never been detected on Goakun3(2.69GHz)
> + */
> + camera_rear: camera@10 {
> + reg = <0x10>;
> +
> + pinctrl-0 = <&camr_rgb_default>;
> + pinctrl-names = "default";
> +
> + clocks = <&camcc CAMCC_MCLK4_CLK>;
> + clock-names = "mclk";
> + clock-frequency = <24000000>;
> +
> + rstn-gpios = <&tlmm 7 GPIO_ACTIVE_LOW>;
> +
> + vddio-supply = <&vreg_camr>;
> + vdda-supply = <&vreg_l2b>;
> + vddd-supply = <&vreg_l2c>;
> +
> + /* &camera_flash can't be enabled directly for now */
> + leds = <&privacy_led>;
> + led-names = "privacy";
> +
> + lens-focus = <&voice_coil_motor>;
> +
> + orientation = <1>;
> + rotation = <180>;
> +
> + port {
> + s5k3l6_ep: endpoint {
> + data-lanes = <1 2 3 4>;
> + remote-endpoint = <&csiphy0_ep>;
> + };
> + };
> + };
> +
> + /* eeprom@50/51 */
> +};

I believe it is unacceptable to add device tree nodes like this one
without a compatible property. While the motivation behind it is clear,
unfortunately it has to be removed.

> +
> &cci2 {
> status = "okay";
> };
> @@ -1423,6 +1510,13 @@ cam_indicator_en: cam-indicator-en-state {
> bias-disable;
> };
>
> + camera_flash_en: camera-flash-en-state {
> + pins = "gpio93";
> + function = "gpio";
> + drive-strength = <2>;
> + bias-disable;
> + };
> +
> camf_1p2_reg_en: camf-1p2-reg-en-state {
> pins = "gpio44";
> function = "gpio";
> @@ -1446,6 +1540,29 @@ sc-rgb-xshut-n-pins {
> };
> };
>
> + camr_reg_en: camr-reg-en-state {
> + pins = "gpio92";
> + function = "gpio";
> + drive-strength = <2>;
> + bias-disable;
> + };
> +
> + camr_rgb_default: camr-rgb-default-state {
> + mclk-pins {
> + pins = "gpio6";
> + function = "cam_mclk";
> + drive-strength = <6>;
> + bias-disable;
> + };

MCLK pad function shoul be a part of the change to sc8280xp.dtsi

> +
> + sc-rgb-xshut-n-pins {
> + pins = "gpio7";
> + function = "gpio";
> + drive-strength = <2>;
> + bias-disable;
> + };
> + };
> +
> i2c4_default: i2c4-default-state {
> pins = "gpio171", "gpio172";
> function = "qup4";

--
Best wishes,
Vladimir