Re: [PATCH v5 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] Pinctrl for MCLK2 is attached to the camss consumer, causing a probe deadlock that breaks camera initialization.
- [Medium] Incorrect pinctrl sleep state override for I2C bus causes power leakage and configuration conflict.
--

--- Patch [1]: [PATCH v5 1/2] arm64: dts: qcom: sdm845-google: Add dual front IMX355 cameras ---
commit 650af867be78a11fabbd3346603a6fc8f7cb758f
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 (standard and wide-angle). Sensors are connected via CSIPHY
port 1 and 2 and controlled over CCI I2C1, using MCLK2 as the clock source.
This enables support for the dual front camera configuration.

> diff --git a/arch/arm64/boot/dts/qcom/sdm845-google-common.dtsi b/arch/arm64/boot/dts/qcom/sdm845-google-common.dtsi
> index 9368fd6b87515..cb5c261802bc9 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845-google-common.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845-google-common.dtsi

[ ... ]

> @@ -351,6 +389,52 @@ 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]
Will this cause a probe deadlock that breaks camera initialization?

Since fw_devlink parses the remote-endpoint properties, it establishes that
camss depends on the imx355 sensors, meaning imx355 will probe first.
During the imx355 probe, it requests and enables MCLK2.

However, because the pinctrl for MCLK2 is attached to the camss consumer
rather than the provider (camcc) or a dedicated gpio-gate-clock node, the
physical pin (GPIO15) won't be configured to output the clock until camss
probes.

Without the clock active, will the imx355 sensor fail to respond to the I2C
read for its CHIP_ID and fail to probe entirely?

[ ... ]

> +};
> +
> +&cci0_sleep {
> +	/* bus has external pull-up, don't pull down */
> +	bias-disable;
> +};

[Severity: Medium]
Is this targeting the correct CCI bus?

The commit message states the cameras are controlled over CCI I2C1, which
uses cci1_sleep. By overriding cci0_sleep instead, does this leave the
I2C lines for the cameras pulled down against their external pull-ups
during sleep, causing a continuous power leak?

Additionally, without using /delete-property/ bias-pull-down;, won't
cci0_sleep incorrectly contain both bias-disable and bias-pull-down
conflicting settings?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260719-pixel3-camera-v5-0-99593f1bd8ec@ixit.cz?part=1