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 C0B0E371887 for ; Tue, 1 Sep 2026 10:02:23 +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=1788256945; cv=none; b=RiI2x4AMeE3/SwuxArvTC/wyBZ0GlIT7ykDzGl5MtNesXguv3717zCG5WxbgIZo3VTt6LLiov8fcUcaUHxLi5Zi98pePYHbLe/SlfKP4D1Bx4vaC0hQ5DVHf0yPHrBprXPCmU0xoWftAo57gR852+0a0l9c/M0LxyZR1dgwLcuU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1788256945; c=relaxed/simple; bh=twDk0Rj/dV1QlMAMTiAwYRISmwHf8IOb+JTwWiqezeU=; h=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Date: Message-Id; b=ue4p6o7GbIhLOKHcx5IyJknyROTr0CzqcVdbPUa9Sj3Xy+z1OqwI7UrnZwGDNwfyw//9R1BtyBCSz4hNILIeA7At7AeW2qTRFY09OGEW/0Rquo0KvXnGc7e9676FJKwy6ym6jRBUdf8pC41la2HV8oDzrpMOja2GLOm2K9F+HW8= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=AfGBBE+n; 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="AfGBBE+n" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 6981C1F000E9; Tue, 1 Sep 2026 10:02:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1788256943; bh=0C9eZJaIM1t9rtI3a0nwPUQy7FLvpmoKtmgclmsKJvE=; h=From:Subject:Reply-To:To:Cc:In-Reply-To:References:Date; b=AfGBBE+nYqYoXjyUjzvyBBCdPuwsrYlV3LHS0lzOPzidEAGLEtffCAx8OLuQ0QP6R vh8dVqkkD9wB375QxsCKbwEWHLAx9AF3fyGdOCXYY8amtAkrPhO41ZI4g26ndPWIh/ 8vFQbCkB66ZIMH2LNvvF6VNPRadv2iUbsK/wNcUIzW7uREVptIavY20RlolgvHJp9m Ujdu4fX2NaWJio3nwS83KOcHaxKXBg/PjyyLImiM9X9wZ6ZCHbEXicNy3h6z4zPYC/ RdhJrNLGKsNirX+QPF5L2hKrl47gXNdjem7e08INMgHYpLuv/KHqWfNvQrA9mfW1aZ /BDlmSaZUnGkw== From: sashiko-bot@kernel.org Subject: Re: [PATCH v8 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: <20260901-pixel3-camera-v8-1-99f159883348@ixit.cz> References: <20260901-pixel3-camera-v8-0-99f159883348@ixit.cz> <20260901-pixel3-camera-v8-1-99f159883348@ixit.cz> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Date: Tue, 01 Sep 2026 10:02:22 +0000 Message-Id: <20260901100223.6981C1F000E9@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 3 potential issue(= s) to consider: - [High] Moving the shared MCLK pinctrl to the CAMSS bridge node breaks sen= sor probing due to a PM lifecycle mismatch. - [Medium] Modifying cci0_sleep instead of cci1_sleep leaves the active bus= with a conflicting pull-down during suspend. - [Low] The patch includes a non-existent header file and uses an undefined= macro, causing a fatal build error. -- commit 92554691cd8ea8c05d2770873409ffec3a11dddf Author: David Heidelberg arm64: dts: qcom: sdm845-google: Add dual front IMX355 cameras This commit adds support for the dual front camera configuration on the Pixel 3. It connects two Sony IMX355 sensors via CSIPHY ports 1 and 2, controlling them over CCI I2C1 and using MCLK2 as the clock source. > --- a/arch/arm64/boot/dts/qcom/sdm845-google-common.dtsi > +++ b/arch/arm64/boot/dts/qcom/sdm845-google-common.dtsi > @@ -6,6 +6,8 @@ > #include > #include > #include > +#include [Severity: Low] Does this header file exist in the source tree? It looks like it might be missing, which would cause a device tree compilation error. > +#include > #include > =20 > #include "sdm845.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] Will assigning the shared MCLK pinctrl to the camss bridge node break the sensor probing due to a PM lifecycle mismatch? When the imx355 sensor asynchronously probes, it enables the CCF clock and attempts to read its I2C registers. Because camss is an independent device, it remains runtime-suspended during the sensor's probe. Since the pinctrl subsystem keeps suspended devices in their sleep state, the MCLK pin would remain in sleep mode, blocking the clock signal and causing an I2C timeout when the sensor reads its chip ID. While fw_devlink typically creates a DL_FLAG_PM_RUNTIME device link, the cross-referencing remote-endpoint properties create a dependency cycle that the driver core relaxes by dropping the PM runtime flag. Can the MCLK pad state be coupled to the sensor's PM lifecycle instead to ensure the clock is active during probe? > + > + status =3D "okay"; [ ... ] > +&cci0_sleep { > + /* bus has external pull-up */ > + /delete-property/ bias-pull-down; > + bias-disable; > +}; [Severity: Medium] Since the new sensors are added to the &cci_i2c1 bus, should this modification apply to cci1_sleep instead of cci0_sleep? Modifying cci0_sleep disables the internal pull-down for the unused I2C0 bus, while leaving the active I2C1 bus with its default pull-down behavior. Since I2C1 has an external pull-up, leaving the internal pull-down active could cause an unnecessary power leak during device sleep. > + > &cci { > status =3D "okay"; > }; > @@ -358,7 +444,63 @@ &cci { > &cci_i2c1 { > /* actuator @0c */ > =20 > - /* front camera, imx355 @1a */ > + front_cam: camera@10 { > + compatible =3D "sony,imx355"; > + reg =3D <0x10>; > + > + clocks =3D <&clock_camcc CAM_CC_MCLK2_CLK>; > + assigned-clocks =3D <&clock_camcc CAM_CC_MCLK2_CLK>; > + assigned-clock-rates =3D <24000000>; > + > + reset-gpios =3D <&tlmm 21 GPIO_ACTIVE_LOW>; > + > + avdd-supply =3D <&camera_front_avdd>; > + dvdd-supply =3D <&vreg_s3a_1p35>; > + dovdd-supply =3D <&vreg_lvs1_1p8>; > + > + pinctrl-0 =3D <&cam_front_reset_default>; > + pinctrl-names =3D "default"; > + > + rotation =3D <270>; > + orientation =3D ; [Severity: Low] Is MEDIA_ORIENTATION_FRONT defined? Since the corresponding header is missing, this looks like it will result in a DTC compilation error. --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260901-pixel3-cam= era-v8-0-99f159883348@ixit.cz?part=3D1