Re: [PATCH v2] arm64: dts: qcom: sc8280xp-x13s: Enable RGB sensor

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

Hi Bryan.

On 8/2/24 14:05, Bryan O'Donoghue wrote:
> Enable the main RGB sensor on the Lenovo x13s a five megapixel 2 lane DPHY
> MIPI sensor connected to cisphy0.
> 
> With the pm8008 patches recently applied to the x13s dtsi we can now also
> enable the RGB sensor. Once done we have all upstream support necessary for
> the RGB sensor on x13s.
> 
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
> The Lenovo x13s has a five megapixel ov5675 sensor as well as a one
> megpixel ov9734 B+W NOIR sensor for low-light face detection login.
> 
> This patch enables the RGB sensor.
> 
> A gpio exists in the upstream dts to indicate camera activity which
> currently we don't tie to CAMSS activity yet.
> 
> Running
> 
> - A Linux distro which ships libcamera > 0.3.0
> - Firefox nightly
> - Setting Firefox about:config:media.webrtc.capture.allow-pipewire = true
> 
> It should then be possible to use the on-board MIPI camera for Zoom,
> Hangouts etc.
> 
> Changes in v2:
> - Drops useless assigned-clocks entry
> - Link to v1: https://lore.kernel.org/r/20240801-b4-linux-next-24-07-31-camss-sc8280xp-lenovo-rgb-v2-v1-1-30622c6a0c48@linaro.org
> ---
>   .../dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts     | 72 ++++++++++++++++++++++
>   1 file changed, 72 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
> index a7c5a3f5926c7..2bdfff2a16cad 100644
> --- a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
> +++ b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
> @@ -592,6 +592,62 @@ vreg_l10d: ldo10 {
>   	};
>   };
>   
> +&camcc {
> +	status = "okay";
> +};

This is not needed, it's not disabled, please remove.

> +
> +&camss {
> +	vdda-phy-supply = <&vreg_l6d>;
> +	vdda-pll-supply = <&vreg_l4d>;
> +
> +	status = "okay";
> +
> +	ports {
> +		/* The port index denotes CSIPHY id i.e. csiphy0 */

I believe a better place for such a comment would be at sc8280xp.dtsi

> +		port@0 {
> +			csiphy0_lanes01_ep: endpoint@0 {
> +				reg = <0>;
> +				clock-lanes = <7>;
> +				data-lanes = <0 1>;
> +				remote-endpoint = <&ov5675_ep>;
> +			};
> +		};
> +	};
> +};
> +
> +&cci2 {
> +	status = "okay";
> +};
> +
> +&cci2_i2c1 {
> +	camera@10 {
> +		compatible = "ovti,ov5675";
> +		reg = <0x10>;
> +
> +		reset-gpios = <&tlmm 15 GPIO_ACTIVE_LOW>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&cam_rgb_default>;
> +
> +		clocks = <&camcc CAMCC_MCLK3_CLK>;
> +
> +		orientation = <0>;	/* Front facing */
> +
> +		avdd-supply = <&vreg_l6q>;
> +		dvdd-supply = <&vreg_l2q>;
> +		dovdd-supply = <&vreg_l7q>;
> +
> +		port {
> +			ov5675_ep: endpoint {
> +				clock-lanes = <0>;
> +				data-lanes = <1 2>;
> +				link-frequencies = /bits/ 64 <450000000>;
> +				remote-endpoint = <&csiphy0_lanes01_ep>;
> +			};
> +		};
> +
> +	};
> +};
> +
>   &dispcc0 {
>   	status = "okay";
>   };
> @@ -1436,6 +1492,22 @@ cam_indicator_en: cam-indicator-en-state {
>   		bias-disable;
>   	};
>   
> +	cam_rgb_default: cam-rgb-default-state {
> +		mclk-pins {
> +			pins = "gpio17";
> +			function = "cam_mclk";
> +			drive-strength = <16>;
> +			bias-disable;
> +		};
> +
> +		sc-rgb-xshut-n-pins {
> +			pins = "gpio15";
> +			function = "gpio";
> +			drive-strength = <2>;
> +			bias-disable;
> +		};
> +	};
> +
>   	edp_reg_en: edp-reg-en-state {
>   		pins = "gpio25";
>   		function = "gpio";
> 
> ---
> base-commit: cd19ac2f903276b820f5d0d89de0c896c27036ed
> change-id: 20240731-b4-linux-next-24-07-31-camss-sc8280xp-lenovo-rgb-v2-525b250dfb84

Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>

--
Best wishes,
Vladimir