Re: [PATCH v9 1/2] arm64: dts: qcom: sdm845-google: Add dual front IMX355 cameras

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The `cam_mclk2_default` pinctrl is assigned to the `camss` node, creating a probe deadlock that prevents the IMX355 sensors from initializing.
- [Medium] The patch incorrectly modifies the sleep state of `cci0_sleep` instead of `cci1_sleep` to disable internal pull-downs, leaving the newly added `cci_i2c1` bus with a pull-down conflict.
--

commit 5866dcb6e26829a5869d717eae07ad902b36446e
Author: David Heidelberg <david@ixit.cz>

arm64: dts: qcom: sdm845-google: Add dual front IMX355 cameras

The Pixel 3 features two front-facing Sony IMX355 sensors with different
focal lengths. This commit enables the dual front camera configuration
by adding them to CSIPHY ports 1 and 2, controlled over CCI I2C1, using
MCLK2 as the clock source.

> diff --git a/arch/arm64/boot/dts/qcom/sdm845-google-common.dtsi b/arch/arm64/boot/dts/qcom/sdm845-google-common.dtsi
> index f1379857e2742..02a392fea7212 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845-google-common.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845-google-common.dtsi

[ ... ]

> @@ -351,6 +390,53 @@ vreg_s3c_0p6: smps3 {
>  	};
>  };
>  
> +&camss {
> +	vdda-phy-supply = <&vreg_l1a_0p875>;
> +	vdda-pll-supply = <&vreg_l26a_1p2>;
> +
> +	/*
> +	 * MCLK2 (GPIO15) is shared between both front camera sensors.
> +	 * The clock is generated by CAMCC, therefore the pin is
> +	 * configured here rather than in individual sensor nodes.
> +	 */
> +	pinctrl-0 = <&cam_mclk2_default>;
> +	pinctrl-1 = <&cam_mclk2_sleep>;
> +	pinctrl-names = "default", "sleep";

[Severity: High]
Does assigning the MCLK pinctrl to the consumer node (&camss) break the
initialization sequence?

Because &camss has remote-endpoint links pointing to the front sensors,
fw_devlink treats the sensors as suppliers and &camss as the consumer,
forcing &camss to probe after the sensors. When the sensors probe, they
attempt to read their chip ID over I2C, which requires the MCLK clock.

Since &camss has not yet probed at that time, the GPIO15 pin remains
unconfigured, the MCLK signal never reaches the sensors, and their I2C reads
fail with a timeout. This creates a probe deadlock preventing the sensors
from initializing.

> +
> +	status = "okay";

[ ... ]

> +		port@2 {
> +			reg = <2>;
> +
> +			camss_endpoint2: endpoint {
> +				bus-type = <MEDIA_BUS_TYPE_CSI2_DPHY>;
> +				data-lanes = <0 1 2 3>;
> +				remote-endpoint = <&cam_front_endpoint>;
> +			};
> +		};
> +	};
> +};
> +
> +&cci0_sleep {
> +	/* bus has external pull-up */
> +	/delete-property/ bias-pull-down;
> +	bias-disable;
> +};

[Severity: Medium]
Is this modifying the sleep state of the wrong I2C bus?

The newly added front IMX355 cameras are connected to the cci_i2c1 bus,
whose sleep pin state is defined by &cci1_sleep. By applying this
configuration to &cci0_sleep instead, &cci1_sleep is left unmodified.

This leaves the cci_i2c1 pins with internal pull-downs enabled during system
sleep, which conflicts with the external pull-ups on the bus and causes a
continuous power leak.

> +
> +&cci {
> +	status = "okay";
> +};

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907-pixel3-camera-v9-0-b1981f2d3291@ixit.cz?part=1