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 98BDD3F39F3 for ; Mon, 20 Jul 2026 11:11:15 +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=1784545876; cv=none; b=H8qG03foVzO1ssrKV0+mwyQTIacwvYxCgdDFILCRMM1waV5FxuKkq72S8LMn0x1y0R1DvRbwl1/3pEkWQTRXwCDmPcaKKf2TEWZPoJ7+Q5RQkIDCgcPI0/Cb+uLWMDshFMEP45TTcRFNFWF0NuDWPMHKJgZy4DPGR5S54VRwuoY= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784545876; c=relaxed/simple; bh=QiyVVbCylb8hAZjxIhgjieMzRnTXe5o+SDxLE17NUEM=; h=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Date: Message-Id; b=ij/DHOU6bu9Ou5/QwPzgeCAp9SPYuNZA81wBoyz/QPkYnLP/RsI6Bkb0vEs6x5HnJN1T31tLEp5DAfctZ1Bi40ffVm7gEjNPJ4qHP1/PLD5kiQnmNWySY+f+74nELWl7qmj9zbr0KR+4GRQ+GVmS7u8FR0DX6ow3ErGmdnm7JT4= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=Tyqn9bEM; 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="Tyqn9bEM" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 3E2681F00A3A; Mon, 20 Jul 2026 11:11:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1784545875; bh=UUH8X5BZgqpcJVb86Ch3E/oF03Ek4Tg6O1EQwPJkXI0=; h=From:Subject:Reply-To:To:Cc:In-Reply-To:References:Date; b=Tyqn9bEMZANEj7twNhzLCHcZEUAC+dSnKYLkgbUKwoHGUqav0uExjo5xZ98XV72QP J8l/Z+TJECES7Lgrox6x4qVQXHlXQmsI1IlUtzjzbRmOum7kfhtJPStXDie6ng/CS4 QOx1Y9d/jLcg3nuM8NFTx0lz7GlelG5oHP+lr6kYbgKcyzsqOUxe2HhTAid9/CUBXR 1rAkrtaVaXPjntc2VHHXCBE5tIYALDU4KcsuRAfhNpWO+WBXpQ3uGFWmnKMNHSJ3c9 hWFO/iDgogMxS7b3uEA9ymWxOl7OdK/SGbR6fbJjogaAeN/3AiIg0vQRfTY0gXymBe fYZZXPt0IQl1w== From: sashiko-bot@kernel.org Subject: Re: [PATCH v6 1/2] arm64: dts: qcom: sdm845-google: Add dual front IMX355 cameras Reply-To: sashiko-reviews@lists.linux.dev To: "David Heidelberg" Cc: robh@kernel.org, conor+dt@kernel.org, devicetree@vger.kernel.org In-Reply-To: <20260720-pixel3-camera-v6-1-afe419bef476@ixit.cz> References: <20260720-pixel3-camera-v6-0-afe419bef476@ixit.cz> <20260720-pixel3-camera-v6-1-afe419bef476@ixit.cz> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Date: Mon, 20 Jul 2026 11:11:14 +0000 Message-Id: <20260720111115.3E2681F00A3A@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] Assigning the MCLK2 `pinctrl` to the `camss` node breaks the camer= a sensor probe due to decoupled runtime PM lifecycles. - [Medium] Incorrect CCI I2C sleep state modification overrides `&cci0_slee= p` instead of `&cci1_sleep`. -- --- Patch [1]: [PATCH v6 1/2] arm64: dts: qcom: sdm845-google: Add dual fro= nt IMX355 cameras --- commit c205a00f81a6553c73e74c2e70cc2c7b6bcef19f Author: David Heidelberg arm64: dts: qcom: sdm845-google: Add dual front IMX355 cameras This commit adds device tree nodes for the two front-facing Sony IMX355 sensors on the Pixel 3. The sensors are connected via CSIPHY ports and controlled over CCI I2C1, sharing MCLK2. > diff --git a/arch/arm64/boot/dts/qcom/sdm845-google-common.dtsi b/arch/ar= m64/boot/dts/qcom/sdm845-google-common.dtsi > --- a/arch/arm64/boot/dts/qcom/sdm845-google-common.dtsi > +++ b/arch/arm64/boot/dts/qcom/sdm845-google-common.dtsi [ ... ] > +&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 putting the MCLK2 pinctrl states in the camss node break the sensor pr= obe? Because camss operates with decoupled asynchronous runtime PM, it will like= ly be suspended during the imx355 sensor probe. The PM core will put the pin i= nto the sleep state, which configures bias-pull-down.=20 When the imx355 driver calls clk_prepare_enable(mclk) during I2C probe to r= ead its chip ID, the clock controller generates the clock, but the sleep state = on camss will prevent the clock signal from reaching the sensors, causing the probe to abort. > + > + status =3D "okay"; [ ... ] > +}; > + > +&cci0_sleep { > + /* bus has external pull-up */ > + /delete-property/ bias-pull-down; > + bias-disable; > +}; [Severity: Medium] Since the front cameras are added under the cci_i2c1 bus, should this be modifying &cci1_sleep instead of &cci0_sleep? This leaves the cci_i2c1 sleep state with its default bias-pull-down settin= g, which will fight the external pull-up and could drain the battery during sleep. It also incorrectly modifies cci_i2c0 which might affect other devic= es on that bus. > + > +&cci { > + status =3D "okay"; > +}; > @@ -358,7 +443,63 @@ &cci { > &cci_i2c1 { > /* actuator @0c */ > =20 > - /* front camera, imx355 @1a */ > + front_cam: camera@10 { > + compatible =3D "sony,imx355"; [ ... ] --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260720-pixel3-cam= era-v6-0-afe419bef476@ixit.cz?part=3D1