Re: [PATCH 3/4] arm64: dts: qcom: sc8280xp: gaokun3: add front camera sensor node

[Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>]

On 4/25/26 13:52, Pengyu Luo wrote:
> Hi846 is found on my Gaokun3, descripting it.

To the best of my knowledege there is no 'descripting' word.

Say it simpler like

	Describe Hi846 camera sensor found on Gaokun3.

> 
> Note that it seems that only Goakun3(3.0GHz) version is equipped with
> Hi846. Goakun3(2.69GHz) version is equipped with S5K4H7.
> 
> Signed-off-by: Pengyu Luo <mitltlatltl@gmail.com>
> ---
>   .../boot/dts/qcom/sc8280xp-huawei-gaokun3.dts | 128 ++++++++++++++++++
>   1 file changed, 128 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-huawei-gaokun3.dts b/arch/arm64/boot/dts/qcom/sc8280xp-huawei-gaokun3.dts
> index 9819454abe13..39e559e91289 100644
> --- a/arch/arm64/boot/dts/qcom/sc8280xp-huawei-gaokun3.dts
> +++ b/arch/arm64/boot/dts/qcom/sc8280xp-huawei-gaokun3.dts
> @@ -15,6 +15,7 @@
>   #include <dt-bindings/gpio/gpio.h>
>   #include <dt-bindings/input/gpio-keys.h>
>   #include <dt-bindings/input/input.h>
> +#include <dt-bindings/leds/common.h>
>   #include <dt-bindings/regulator/qcom,rpmh-regulator.h>
>   #include <dt-bindings/phy/phy.h>
>   
> @@ -27,6 +28,7 @@ / {
>   	compatible = "huawei,gaokun3", "qcom,sc8280xp";
>   
>   	aliases {
> +		i2c2 = &cci2_i2c1;

I don't think that it makes big sense to add this I2C alias, please
consider to remove it.

>   		i2c4 = &i2c4;
>   		i2c15 = &i2c15;
>   		serial1 = &uart2;
> @@ -47,6 +49,21 @@ framebuffer0: framebuffer@c6200000 {
>   		};
>   	};
>   
> +	leds {
> +		compatible = "gpio-leds";
> +
> +		pinctrl-0 = <&cam_indicator_en>;
> +		pinctrl-names = "default";
> +
> +		privacy_led: privacy-led {
> +			function = LED_FUNCTION_INDICATOR;
> +			color = <LED_COLOR_ID_WHITE>;
> +			gpios = <&tlmm 34 GPIO_ACTIVE_HIGH>;
> +			default-state = "off";
> +			panic-indicator;
> +		};
> +	};
> +
>   	wcd938x: audio-codec {
>   		compatible = "qcom,wcd9380-codec";
>   
> @@ -98,6 +115,20 @@ switch-mode {
>   		};
>   	};
>   
> +	vreg_camf_1p2: regulator-camf-1p2 {
> +		compatible = "regulator-fixed";
> +
> +		regulator-name = "vreg_camf_1p2";
> +		regulator-min-microvolt = <1200000>;
> +		regulator-max-microvolt = <1200000>;
> +
> +		gpio = <&tlmm 44 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +
> +		pinctrl-0 = <&camf_1p2_reg_en>;
> +		pinctrl-names = "default";
> +	};
> +
>   	vreg_misc_3p3: regulator-misc-3p3 {
>   		compatible = "regulator-fixed";
>   
> @@ -462,6 +493,13 @@ vreg_l8c: ldo8 {
>   						   RPMH_REGULATOR_MODE_HPM>;
>   		};
>   
> +		vreg_l11c: ldo11 {
> +			regulator-name = "vreg_l11c";
> +			regulator-min-microvolt = <2800000>;
> +			regulator-max-microvolt = <2800000>;
> +			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> +		};
> +
>   		vreg_l12c: ldo12 {
>   			regulator-name = "vreg_l12c";
>   			regulator-min-microvolt = <1800000>;
> @@ -577,6 +615,66 @@ vreg_l10d: ldo10 {
>   	};
>   };
>   
> +&camss {
> +	vdda-phy-supply = <&vreg_l6b>;
> +	vdda-pll-supply = <&vreg_l3b>;
> +
> +	status = "okay";
> +
> +	ports {
> +		port@3 {
> +			csiphy3_ep: endpoint@0 {
> +				reg = <0>;
> +
> +				clock-lanes = <7>;

Please remove 'clock-lanes' property from above.

> +				data-lanes = <0 1 2 3>;
> +				remote-endpoint = <&hi846_ep>;
> +			};
> +		};
> +	};
> +};
> +
> +&cci2 {
> +	status = "okay";
> +};
> +
> +&cci2_i2c1 {
> +	/* On Goakun3(2.69GHz), S5K4H7@2d, image quality is better than hi846 */
> +	camera_front: camera@20 {

'camera_front' label is unused and should be removed.

> +		compatible = "hynix,hi846";
> +		reg = <0x20>;
> +
> +		pinctrl-0 = <&camf_rgb_default>;
> +		pinctrl-names = "default";
> +
> +		clocks = <&camcc CAMCC_MCLK3_CLK>;
> +
> +		assigned-clocks = <&camcc CAMCC_MCLK3_CLK>;
> +		assigned-clock-rates = <24000000>;
> +
> +		reset-gpios = <&tlmm 15 GPIO_ACTIVE_LOW>;
> +		vddio-supply = <&vreg_l2c>;
> +		vdda-supply = <&vreg_l11c>;
> +		vddd-supply = <&vreg_camf_1p2>;
> +
> +		leds = <&privacy_led>;
> +		led-names = "privacy";
> +
> +		orientation = <0>;	/* Front facing */
> +		rotation = <0>;

Both properties above can be removed as the default ones.

> +
> +		port {
> +			hi846_ep: endpoint {
> +				data-lanes = <1 2 3 4>;
> +				link-frequencies = /bits/ 64 <80000000 144000000 200000000 288000000>;
> +				remote-endpoint = <&csiphy3_ep>;
> +			};
> +		};
> +	};
> +
> +	/* actuator???@58, Goakun3(2.69GHz) only, eeprom@50 */
> +};
> +
>   &dispcc0 {
>   	status = "okay";
>   };
> @@ -1318,6 +1416,36 @@ hstp-sw-ctrl-pins {
>   		};
>   	};
>   
> +	cam_indicator_en: cam-indicator-en-state {
> +		pins = "gpio34";
> +		function = "gpio";
> +		drive-strength = <2>;
> +		bias-disable;
> +	};
> +
> +	camf_1p2_reg_en: camf-1p2-reg-en-state {
> +		pins = "gpio44";
> +		function = "gpio";
> +		drive-strength = <2>;
> +		bias-disable;
> +	};
> +
> +	camf_rgb_default: camf-rgb-default-state {
> +		mclk-pins {
> +			pins = "gpio17";
> +			function = "cam_mclk";
> +			drive-strength = <6>;
> +			bias-disable;
> +		};

cam_mclk pad function descriptions on all 8 pads supporting this function
shall be added to the platform in a separate commit to sc8280xp.dtsi

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

-- 
Best wishes,
Vladimir