* [PATCH v5 0/2] Add initial dual front camera and rear flash support for Pixel 3 / 3 XL
@ 2026-07-19 18:03 David Heidelberg via B4 Relay
2026-07-19 18:03 ` [PATCH v5 1/2] arm64: dts: qcom: sdm845-google: Add dual front IMX355 cameras David Heidelberg via B4 Relay
2026-07-19 18:03 ` [PATCH v5 2/2] arm64: dts: qcom: sdm845-google: Enable PMI8998 camera flash LEDs David Heidelberg via B4 Relay
0 siblings, 2 replies; 9+ messages in thread
From: David Heidelberg via B4 Relay @ 2026-07-19 18:03 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: Petr Hodina, Richard Acayan, linux-arm-msm, devicetree,
linux-kernel, phone-devel, David Heidelberg
Describe the dual front-facing IMX355 sensors (standard and wide)
and enable the PMI8998 flash LED with hardware-accurate limits.
This brings up the basic camera topology and flash support in DT.
Signed-off-by: David Heidelberg <david@ixit.cz>
---
Changes in v5:
- Added function-enumarator to flash LEDs. (Sashiko)
- Dropped l22a regulator needed only for NFOV actuator. (Sashiko)
- Dropped vdda-csi[0-2]-supply, as it was unused. (Sashiko).
- Added previously omited cam_mclk2_sleep sleep state.
- Link to v4: https://lore.kernel.org/r/20260719-pixel3-camera-v4-0-48cab1236852@ixit.cz
Changes in v4:
- Use 24 MHz ext clock for the cameras, as the downstream does, since
the upstream driver can now support it.
- Added additional LED, as the phone contains two separately controlled
white LEDs flash (upper and bottom, in one module).
- Increase flash-max-microamp, as I took wrong "flash" from the
downstream sources and dropped Dmitry R-b.
- Address formatting issues. (Konrad)
- Link to v3: https://lore.kernel.org/r/20260412-pixel3-camera-v3-0-e26b090a6110@ixit.cz
Changes in v3:
- Dropped cam_vio label. (Dmitry)
- Move the MCLK2 pinctrl from cameras to common camss. (Dmitry)
- Link to v2: https://lore.kernel.org/r/20260411-pixel3-camera-v2-0-41b889abb14c@ixit.cz
Changes in v2:
- leds.h include escaped the initial submission. Fixed.
- Link to v1: https://lore.kernel.org/r/20260411-pixel3-camera-v1-0-2757606515b6@ixit.cz
---
David Heidelberg (2):
arm64: dts: qcom: sdm845-google: Add dual front IMX355 cameras
arm64: dts: qcom: sdm845-google: Enable PMI8998 camera flash LEDs
arch/arm64/boot/dts/qcom/sdm845-google-common.dtsi | 197 ++++++++++++++++++++-
1 file changed, 196 insertions(+), 1 deletion(-)
---
base-commit: 0718283ab28bc3907e10b61a6b4be6fefa1cbb2f
change-id: 20260315-pixel3-camera-a9989bf589ee
Best regards,
--
David Heidelberg <david@ixit.cz>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v5 1/2] arm64: dts: qcom: sdm845-google: Add dual front IMX355 cameras
2026-07-19 18:03 [PATCH v5 0/2] Add initial dual front camera and rear flash support for Pixel 3 / 3 XL David Heidelberg via B4 Relay
@ 2026-07-19 18:03 ` David Heidelberg via B4 Relay
2026-07-19 18:24 ` sashiko-bot
2026-07-20 8:24 ` Konrad Dybcio
2026-07-19 18:03 ` [PATCH v5 2/2] arm64: dts: qcom: sdm845-google: Enable PMI8998 camera flash LEDs David Heidelberg via B4 Relay
1 sibling, 2 replies; 9+ messages in thread
From: David Heidelberg via B4 Relay @ 2026-07-19 18:03 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: Petr Hodina, Richard Acayan, linux-arm-msm, devicetree,
linux-kernel, phone-devel, David Heidelberg
From: David Heidelberg <david@ixit.cz>
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.
Signed-off-by: David Heidelberg <david@ixit.cz>
---
arch/arm64/boot/dts/qcom/sdm845-google-common.dtsi | 170 ++++++++++++++++++++-
1 file changed, 169 insertions(+), 1 deletion(-)
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
@@ -1,16 +1,17 @@
// SPDX-License-Identifier: GPL-2.0-only
/dts-v1/;
#include <dt-bindings/arm/qcom,ids.h>
#include <dt-bindings/dma/qcom-gpi.h>
#include <dt-bindings/input/linux-event-codes.h>
#include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/media/video-interfaces.h>
#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
#include "sdm845.dtsi"
#include "pm8998.dtsi"
#include "pmi8998.dtsi"
/delete-node/ &mpss_region;
/delete-node/ &venus_mem;
@@ -127,16 +128,48 @@ vreg_s4a_1p8: regulator-vreg-s4a-1p8 {
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <1800000>;
regulator-always-on;
regulator-boot-on;
vin-supply = <&vph_pwr>;
};
+ camera_front_avdd: front-cam-avdd-regulator {
+ compatible = "regulator-fixed";
+ regulator-name = "front_cam_avdd";
+
+ regulator-min-microvolt = <2800000>;
+ regulator-max-microvolt = <2800000>;
+
+ gpios = <&tlmm 8 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+
+ pinctrl-0 = <&cam_front_avdd_default_pin>;
+ pinctrl-names = "default";
+
+ vin-supply = <&vreg_bob>;
+ };
+
+ camera_front_aux_avdd: front-cam-aux-avdd-regulator {
+ compatible = "regulator-fixed";
+ regulator-name = "front_cam_aux_avdd";
+
+ regulator-min-microvolt = <2800000>;
+ regulator-max-microvolt = <2800000>;
+
+ gpios = <&tlmm 14 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+
+ pinctrl-0 = <&cam_front_aux_avdd_default_pin>;
+ pinctrl-names = "default";
+
+ vin-supply = <&vreg_bob>;
+ };
+
wcn3990-pmu {
compatible = "qcom,wcn3990-pmu";
vddio-supply = <&vreg_s4a_1p8>;
vddxo-supply = <&vreg_l7a_1p8>;
vddrf-supply = <&vreg_l17a_1p3>;
vddch0-supply = <&vreg_l25a_3p3>;
@@ -314,16 +347,21 @@ vreg_l28a_3p0: ldo28 {
regulator-initial-mode = <RPMH_REGULATOR_MODE_LPM>;
regulator-boot-on;
/*
* We can't properly bring the panel back if it gets turned off
* so keep it's regulators always on for now.
*/
regulator-always-on;
};
+
+ vreg_lvs1_1p8: lvs1 {
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ };
};
regulators-1 {
compatible = "qcom,pmi8998-rpmh-regulators";
qcom,pmic-id = "b";
vdd-bob-supply = <&vph_pwr>;
@@ -346,24 +384,126 @@ regulators-2 {
vreg_s3c_0p6: smps3 {
regulator-min-microvolt = <600000>;
regulator-max-microvolt = <600000>;
};
};
};
+&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";
+
+ status = "okay";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@1 {
+ reg = <1>;
+
+ camss_endpoint1: endpoint {
+ bus-type = <MEDIA_BUS_TYPE_CSI2_DPHY>;
+ data-lanes = <0 1 2 3>;
+ remote-endpoint = <&cam_aux_front_endpoint>;
+ };
+ };
+
+ 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, don't pull down */
+ bias-disable;
+};
+
&cci {
status = "okay";
};
&cci_i2c1 {
/* actuator @0c */
- /* front camera, imx355 @1a */
+ front_cam: camera@10 {
+ compatible = "sony,imx355";
+ reg = <0x10>;
+
+ clocks = <&clock_camcc CAM_CC_MCLK2_CLK>;
+ assigned-clocks = <&clock_camcc CAM_CC_MCLK2_CLK>;
+ assigned-clock-rates = <24000000>;
+
+ reset-gpios = <&tlmm 21 GPIO_ACTIVE_LOW>;
+
+ avdd-supply = <&camera_front_avdd>;
+ dvdd-supply = <&vreg_s3a_1p35>;
+ dovdd-supply = <&vreg_lvs1_1p8>;
+
+ pinctrl-0 = <&cam_front_reset_default_pin>;
+ pinctrl-names = "default";
+
+ rotation = <270>;
+ orientation = <0>;
+
+ port {
+ cam_front_endpoint: endpoint {
+ data-lanes = <1 2 3 4>;
+ link-frequencies = /bits/ 64 <360000000>;
+ remote-endpoint = <&camss_endpoint2>;
+ };
+ };
+ };
+
+ front_aux_cam: camera@1a {
+ compatible = "sony,imx355";
+ reg = <0x1a>;
+
+ clocks = <&clock_camcc CAM_CC_MCLK2_CLK>;
+ assigned-clocks = <&clock_camcc CAM_CC_MCLK2_CLK>;
+ assigned-clock-rates = <24000000>;
+
+ reset-gpios = <&tlmm 9 GPIO_ACTIVE_LOW>;
+
+ avdd-supply = <&camera_front_aux_avdd>;
+ dvdd-supply = <&vreg_s3a_1p35>;
+ dovdd-supply = <&vreg_lvs1_1p8>;
+
+ pinctrl-0 = <&cam_front_aux_reset_default_pin>;
+ pinctrl-names = "default";
+
+ rotation = <270>;
+ orientation = <0>;
+
+ port {
+ cam_aux_front_endpoint: endpoint {
+ data-lanes = <1 2 3 4>;
+ link-frequencies = /bits/ 64 <360000000>;
+ remote-endpoint = <&camss_endpoint1>;
+ };
+ };
+ };
/* eeprom @50, at24 driver says 8K */
};
&cdsp_pas {
firmware-name = "qcom/sdm845/Google/blueline/cdsp.mbn";
status = "okay";
@@ -485,16 +625,44 @@ nfc_int_default: nfc-int-default-state {
nfc_enable_default: nfc-enable-default-state {
pins = "gpio25", "gpio79";
function = "gpio";
drive-strength = <2>;
bias-pull-down;
};
+ cam_front_avdd_default_pin: cam-avdd-default-pins {
+ pins = "gpio8";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
+ };
+
+ cam_front_aux_reset_default_pin: cam-front-aux-reset-default-pins {
+ pins = "gpio9";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
+ };
+
+ cam_front_aux_avdd_default_pin: cam-avdd-aux-default-pins {
+ pins = "gpio14";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
+ };
+
+ cam_front_reset_default_pin: cam-front-reset-default-pins {
+ pins = "gpio21";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
+ };
+
touchscreen_reset: ts-reset-state {
pins = "gpio99";
function = "gpio";
drive-strength = <8>;
bias-pull-up;
};
touchscreen_pins: ts-pins-gpio-state {
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v5 2/2] arm64: dts: qcom: sdm845-google: Enable PMI8998 camera flash LEDs
2026-07-19 18:03 [PATCH v5 0/2] Add initial dual front camera and rear flash support for Pixel 3 / 3 XL David Heidelberg via B4 Relay
2026-07-19 18:03 ` [PATCH v5 1/2] arm64: dts: qcom: sdm845-google: Add dual front IMX355 cameras David Heidelberg via B4 Relay
@ 2026-07-19 18:03 ` David Heidelberg via B4 Relay
2026-07-19 18:12 ` sashiko-bot
2026-07-20 8:26 ` Konrad Dybcio
1 sibling, 2 replies; 9+ messages in thread
From: David Heidelberg via B4 Relay @ 2026-07-19 18:03 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: Petr Hodina, Richard Acayan, linux-arm-msm, devicetree,
linux-kernel, phone-devel, David Heidelberg
From: David Heidelberg <david@ixit.cz>
Enable the PMI8998 flash LED block and describe two white flash LEDs
used for the rear camera.
Configure the LED in flash mode with hardware limits matching the
original device configuration, including maximum current and timeout.
In contrary to downstream, we can control both LEDs separately.
Signed-off-by: David Heidelberg <david@ixit.cz>
---
arch/arm64/boot/dts/qcom/sdm845-google-common.dtsi | 27 ++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sdm845-google-common.dtsi b/arch/arm64/boot/dts/qcom/sdm845-google-common.dtsi
index cb5c261802bc9..f3c7333a4076e 100644
--- a/arch/arm64/boot/dts/qcom/sdm845-google-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845-google-common.dtsi
@@ -1,16 +1,17 @@
// SPDX-License-Identifier: GPL-2.0-only
/dts-v1/;
#include <dt-bindings/arm/qcom,ids.h>
#include <dt-bindings/dma/qcom-gpi.h>
#include <dt-bindings/input/linux-event-codes.h>
#include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/leds/common.h>
#include <dt-bindings/media/video-interfaces.h>
#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
#include "sdm845.dtsi"
#include "pm8998.dtsi"
#include "pmi8998.dtsi"
/delete-node/ &mpss_region;
@@ -589,16 +590,42 @@ &pm8998_resin {
};
&pmi8998_charger {
monitored-battery = <&battery>;
status = "okay";
};
+&pmi8998_flash {
+ status = "okay";
+
+ /* upper part */
+ led-0 {
+ function = LED_FUNCTION_FLASH;
+ color = <LED_COLOR_ID_WHITE>;
+ led-sources = <1>;
+ led-max-microamp = <500000>;
+ flash-max-microamp = <1500000>;
+ flash-max-timeout-us = <1280000>;
+ function-enumerator = <0>;
+ };
+
+ /* bottom part */
+ led-1 {
+ function = LED_FUNCTION_FLASH;
+ color = <LED_COLOR_ID_WHITE>;
+ led-sources = <2>;
+ led-max-microamp = <500000>;
+ flash-max-microamp = <1500000>;
+ flash-max-timeout-us = <1280000>;
+ function-enumerator = <1>;
+ };
+};
+
&qupv3_id_0 {
status = "okay";
};
&qupv3_id_1 {
status = "okay";
};
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v5 2/2] arm64: dts: qcom: sdm845-google: Enable PMI8998 camera flash LEDs
2026-07-19 18:03 ` [PATCH v5 2/2] arm64: dts: qcom: sdm845-google: Enable PMI8998 camera flash LEDs David Heidelberg via B4 Relay
@ 2026-07-19 18:12 ` sashiko-bot
2026-07-20 8:26 ` Konrad Dybcio
1 sibling, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-07-19 18:12 UTC (permalink / raw)
To: David Heidelberg; +Cc: devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] A preexisting logical error in the teardown and error paths of `leds-qcom-flash.c` reads out-of-bounds of the `v4l2_flash` array, causing it to either leak successfully initialized V4L2 flash devices or trigger a kernel panic.
--
--- Patch [2]: [PATCH v5 2/2] arm64: dts: qcom: sdm845-google: Enable PMI8998 camera flash LEDs ---
commit 93ed1a73fa53c221bf41460abbfe7fe7c99737a0
Author: David Heidelberg <david@ixit.cz>
arm64: dts: qcom: sdm845-google: Enable PMI8998 camera flash LEDs
Enable the PMI8998 flash LED block and describe two white flash LEDs
used for the rear camera.
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-google-common.dtsi b/arch/arm64/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
[ ... ]
> +&pmi8998_flash {
> + status = "okay";
> +
> + /* upper part */
> + led-0 {
> + function = LED_FUNCTION_FLASH;
> + color = <LED_COLOR_ID_WHITE>;
> + led-sources = <1>;
> + led-max-microamp = <500000>;
> + flash-max-microamp = <1500000>;
> + flash-max-timeout-us = <1280000>;
> + function-enumerator = <0>;
> + };
> +
> + /* bottom part */
> + led-1 {
> + function = LED_FUNCTION_FLASH;
> + color = <LED_COLOR_ID_WHITE>;
> + led-sources = <2>;
> + led-max-microamp = <500000>;
> + flash-max-microamp = <1500000>;
> + flash-max-timeout-us = <1280000>;
> + function-enumerator = <1>;
> + };
> +};
[Severity: High]
This is a pre-existing issue, but configuring multiple flash LEDs exposes an
out-of-bounds array access in the leds-qcom-flash driver.
During driver teardown in qcom_flash_led_remove(), the code attempts to
access the v4l2_flash array at index leds_count:
drivers/leds/flash/leds-qcom-flash.c:qcom_flash_led_remove() {
...
while (flash_data->v4l2_flash[flash_data->leds_count] &&
flash_data->leds_count)
v4l2_flash_release(flash_data->v4l2_flash[flash_data->leds_count--]);
...
}
If the probe is successful, leds_count equals the number of LEDs. Accessing
the array at that index reads past the end of the dynamically allocated
structure, potentially causing a panic or leaking resources if the
out-of-bounds memory happens to be NULL.
The probe error path in qcom_flash_led_probe() shares this logic. If probing
fails on the second LED, v4l2_flash at that index is NULL. The loop
evaluates to false immediately and terminates without freeing the
successfully initialized first LED.
Could we fix the array bounds check and release logic in the driver before
exposing it via this multi-LED device tree configuration?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260719-pixel3-camera-v5-0-99593f1bd8ec@ixit.cz?part=2
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5 1/2] arm64: dts: qcom: sdm845-google: Add dual front IMX355 cameras
2026-07-19 18:03 ` [PATCH v5 1/2] arm64: dts: qcom: sdm845-google: Add dual front IMX355 cameras David Heidelberg via B4 Relay
@ 2026-07-19 18:24 ` sashiko-bot
2026-07-19 18:57 ` David Heidelberg
2026-07-20 8:24 ` Konrad Dybcio
1 sibling, 1 reply; 9+ messages in thread
From: sashiko-bot @ 2026-07-19 18:24 UTC (permalink / raw)
To: David Heidelberg; +Cc: devicetree, robh, conor+dt
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5 1/2] arm64: dts: qcom: sdm845-google: Add dual front IMX355 cameras
2026-07-19 18:24 ` sashiko-bot
@ 2026-07-19 18:57 ` David Heidelberg
0 siblings, 0 replies; 9+ messages in thread
From: David Heidelberg @ 2026-07-19 18:57 UTC (permalink / raw)
To: sashiko-reviews; +Cc: devicetree, robh, conor+dt
On 19/07/2026 20:24, sashiko-bot@kernel.org wrote:
> 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?
No
>
> 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?
Yes
>
> [ ... ]
>
>> +};
>> +
>> +&cci0_sleep {
>> + /* bus has external pull-up, don't pull down */
>> + bias-disable;
>> +};
>
> [Severity: Medium]
> Is this targeting the correct CCI bus?
Yes.
>
> 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?
>
--
David Heidelberg
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5 1/2] arm64: dts: qcom: sdm845-google: Add dual front IMX355 cameras
2026-07-19 18:03 ` [PATCH v5 1/2] arm64: dts: qcom: sdm845-google: Add dual front IMX355 cameras David Heidelberg via B4 Relay
2026-07-19 18:24 ` sashiko-bot
@ 2026-07-20 8:24 ` Konrad Dybcio
2026-07-20 10:49 ` David Heidelberg
1 sibling, 1 reply; 9+ messages in thread
From: Konrad Dybcio @ 2026-07-20 8:24 UTC (permalink / raw)
To: david, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: Petr Hodina, Richard Acayan, linux-arm-msm, devicetree,
linux-kernel, phone-devel
On 7/19/26 8:03 PM, David Heidelberg via B4 Relay wrote:
> From: David Heidelberg <david@ixit.cz>
>
> 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.
>
> Signed-off-by: David Heidelberg <david@ixit.cz>
> ---
[...]
> @@ -127,16 +128,48 @@ vreg_s4a_1p8: regulator-vreg-s4a-1p8 {
/\ is regulator-foo
> regulator-min-microvolt = <1800000>;
> regulator-max-microvolt = <1800000>;
> regulator-always-on;
> regulator-boot-on;
>
> vin-supply = <&vph_pwr>;
> };
>
\/ this is foo-regulator, please stick to the former
> + camera_front_avdd: front-cam-avdd-regulator {
> + compatible = "regulator-fixed";
> + regulator-name = "front_cam_avdd";
[...]
> + cam_front_avdd_default_pin: cam-avdd-default-pins {
> + pins = "gpio8";
> + function = "gpio";
> + drive-strength = <2>;
> + bias-disable;
> + };
> +
> + cam_front_aux_reset_default_pin: cam-front-aux-reset-default-pins {
> + pins = "gpio9";
> + function = "gpio";
> + drive-strength = <2>;
> + bias-disable;
> + };
> +
> + cam_front_aux_avdd_default_pin: cam-avdd-aux-default-pins {
> + pins = "gpio14";
> + function = "gpio";
> + drive-strength = <2>;
> + bias-disable;
> + };
> +
> + cam_front_reset_default_pin: cam-front-reset-default-pins {
> + pins = "gpio21";
> + function = "gpio";
> + drive-strength = <2>;
> + bias-disable;
> + };
This addition fails dtbs_check
Konrad
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5 2/2] arm64: dts: qcom: sdm845-google: Enable PMI8998 camera flash LEDs
2026-07-19 18:03 ` [PATCH v5 2/2] arm64: dts: qcom: sdm845-google: Enable PMI8998 camera flash LEDs David Heidelberg via B4 Relay
2026-07-19 18:12 ` sashiko-bot
@ 2026-07-20 8:26 ` Konrad Dybcio
1 sibling, 0 replies; 9+ messages in thread
From: Konrad Dybcio @ 2026-07-20 8:26 UTC (permalink / raw)
To: david, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: Petr Hodina, Richard Acayan, linux-arm-msm, devicetree,
linux-kernel, phone-devel
On 7/19/26 8:03 PM, David Heidelberg via B4 Relay wrote:
> From: David Heidelberg <david@ixit.cz>
>
> Enable the PMI8998 flash LED block and describe two white flash LEDs
> used for the rear camera.
>
> Configure the LED in flash mode with hardware limits matching the
> original device configuration, including maximum current and timeout.
>
> In contrary to downstream, we can control both LEDs separately.
Progress!
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5 1/2] arm64: dts: qcom: sdm845-google: Add dual front IMX355 cameras
2026-07-20 8:24 ` Konrad Dybcio
@ 2026-07-20 10:49 ` David Heidelberg
0 siblings, 0 replies; 9+ messages in thread
From: David Heidelberg @ 2026-07-20 10:49 UTC (permalink / raw)
To: Konrad Dybcio, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: Petr Hodina, Richard Acayan, linux-arm-msm, devicetree,
linux-kernel, phone-devel
On 20/07/2026 10:24, Konrad Dybcio wrote:
> On 7/19/26 8:03 PM, David Heidelberg via B4 Relay wrote:
>> From: David Heidelberg <david@ixit.cz>
>>
>> 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.
>>
>> Signed-off-by: David Heidelberg <david@ixit.cz>
>> ---
[...]
> This addition fails dtbs_check
Oh... at least point I didn't realized I left there something which fails
dtbs_check. Fixed :)
Thanks
David
>
> Konrad
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-07-20 10:49 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-19 18:03 [PATCH v5 0/2] Add initial dual front camera and rear flash support for Pixel 3 / 3 XL David Heidelberg via B4 Relay
2026-07-19 18:03 ` [PATCH v5 1/2] arm64: dts: qcom: sdm845-google: Add dual front IMX355 cameras David Heidelberg via B4 Relay
2026-07-19 18:24 ` sashiko-bot
2026-07-19 18:57 ` David Heidelberg
2026-07-20 8:24 ` Konrad Dybcio
2026-07-20 10:49 ` David Heidelberg
2026-07-19 18:03 ` [PATCH v5 2/2] arm64: dts: qcom: sdm845-google: Enable PMI8998 camera flash LEDs David Heidelberg via B4 Relay
2026-07-19 18:12 ` sashiko-bot
2026-07-20 8:26 ` Konrad Dybcio
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox