From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D15ED2222D9 for ; Mon, 7 Sep 2026 16:12:00 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1788797522; cv=none; b=BPn2ZG50m56YWu4nPhkwV5srUT1WMjr3/d9GZc9W2b7oD9myby4gz7gjXI//k1NABVJYHfM7Lj1ZLNu9gxnwyL9FflaKoVAwcrr+0TF7tjwIwCJuTY5R7x9yglOLjLf14VEDeLqqmmCbZhlV9G8PRPybgy72MBh/Ht/0HUXq7ys= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1788797522; c=relaxed/simple; bh=pzQscGUAbRqiasCZM/EfR7Q9AOGP3r5Avec8CMCgGgY=; h=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Date: Message-Id; b=GaoXGN2so60Ztyd4lU3bYAqsIpyLykIKBw1FUuNnloX1nvGZgbSh3QfH5zs4F3FDTL+qH0P/zyAbrZpLelCWjryP2Q4+W7shAN54HWC10HCH+I+ZilaTsJVYYXfWY11iwanZjt9D84qQXYwZguHV99DhIg/JvfiT53C/UN4/6bY= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=L+FAA74I; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="L+FAA74I" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 0F1861F00A3A; Mon, 7 Sep 2026 16:12:00 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1788797520; bh=T8Ap462CsEqbkZmrB0AhwgIlwRGKsyyLJ4QMWOStsaw=; h=From:Subject:Reply-To:To:Cc:In-Reply-To:References:Date; b=L+FAA74IJudeyQejZ2STtvrFOqvMhs1rfvV1tqnEDOKjpAs5WUb3nHfZ7xEPJkMcI zzy/cQeXTXoU2Yq7VCmpFeDkx1M3/4HfMnBJaStda4esLuU3Uoxccqq4LMAbjrdxfg FYHX3UeWReUdldST5+tepru590hwnqFfIHsm057vqjQdNgiRtYZvP+1gQyUnXqBhku UBjkgW8G5Ws3NfvOThcSvtaPeD7JMLxq7XKCJG8e7ItvKlHNrf/EdjpbB1rxue92mv QS2aXh9iCy17NzkCZVXfblg/1q1hNuz88Lb3oYxHHw5AuPwIChy9Tp2W6MIZWR5Wdm 7PuOYC9PN2FTQ== From: sashiko-bot@kernel.org Subject: Re: [PATCH v9 1/2] arm64: dts: qcom: sdm845-google: Add dual front IMX355 cameras Reply-To: sashiko-reviews@lists.linux.dev To: "David Heidelberg" Cc: devicetree@vger.kernel.org, robh@kernel.org, conor+dt@kernel.org In-Reply-To: <20260907-pixel3-camera-v9-1-b1981f2d3291@ixit.cz> References: <20260907-pixel3-camera-v9-0-b1981f2d3291@ixit.cz> <20260907-pixel3-camera-v9-1-b1981f2d3291@ixit.cz> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Date: Mon, 07 Sep 2026 16:11:59 +0000 Message-Id: <20260907161200.0F1861F00A3A@smtp.kernel.org> Precedence: bulk X-Mailing-List: devicetree@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: 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, c= reating a probe deadlock that prevents the IMX355 sensors from initializing. - [Medium] The patch incorrectly modifies the sleep state of `cci0_sleep` i= nstead of `cci1_sleep` to disable internal pull-downs, leaving the newly ad= ded `cci_i2c1` bus with a pull-down conflict. -- commit 5866dcb6e26829a5869d717eae07ad902b36446e Author: David Heidelberg 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/ar= m64/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 { > }; > }; > =20 > +&camss { > + vdda-phy-supply =3D <&vreg_l1a_0p875>; > + vdda-pll-supply =3D <&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 =3D <&cam_mclk2_default>; > + pinctrl-1 =3D <&cam_mclk2_sleep>; > + pinctrl-names =3D "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 =3D "okay"; [ ... ] > + port@2 { > + reg =3D <2>; > + > + camss_endpoint2: endpoint { > + bus-type =3D ; > + data-lanes =3D <0 1 2 3>; > + remote-endpoint =3D <&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 =3D "okay"; > +}; [ ... ] --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260907-pixel3-cam= era-v9-0-b1981f2d3291@ixit.cz?part=3D1