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

[Pengyu Luo]

On Thu, Apr 30, 2026 at 7:00 PM Vladimir Zapolskiy
<vladimir.zapolskiy@xxxxxxxxxx> wrote:
>
> 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@xxxxxxxxx>
> > ---
> > 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.
>

Should we register an unused node?

> > +
> >   / {
> >       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.
>

Ack

> >
> >               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.
>

Ack

Best wishes,
Pengyu

> > +
> >   &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