Re: [PATCH] arm64: dts: qcom: qcs6490-rubikpi3: Enable cameras

[Hongyang Zhao <hongyang.zhao@thundersoft.com>]

Hi Konrad,

Thanks for the review.

On 6/16/26 9:29 AM, Hongyang Zhao wrote:
> > Enable the two Raspberry Pi camera connectors on RubikPi 3 with
> > IMX219 sensors. CAM1 is connected to CCI1 and CSIPHY1, while CAM2
> > is connected to CCI0 and CSIPHY4.
> >
> > Add the shared 24 MHz camera oscillator, camera power enable
> > regulators, CAMSS endpoints, CCI sensor nodes and privacy LED GPIOs.
> >
> > Signed-off-by: Hongyang Zhao <hongyang.zhao@thundersoft.com>
> > ---
>
> [...]
>
> > +	vreg_cam1_pwr: regulator-camera1-pwr {
> > +		compatible = "regulator-fixed";
> > +
> > +		regulator-name = "vreg_camera1_pwr";
> > +		regulator-min-microvolt = <3300000>;
> > +		regulator-max-microvolt = <3300000>;
> > +
> > +		gpio = <&tlmm 57 GPIO_ACTIVE_HIGH>;
> > +		enable-active-high;
> > +
> > +		pinctrl-0 = <&cam1_pwr_en>;
> > +		pinctrl-names = "default";
> > +
> > +		vin-supply = <&vreg_vcc3v3_output>;
> > +	};
> > +
> > +	vreg_cam2_pwr: regulator-camera2-pwr {
> > +		compatible = "regulator-fixed";
> > +
> > +		regulator-name = "vreg_camera2_pwr";
> > +		regulator-min-microvolt = <3300000>;
> > +		regulator-max-microvolt = <3300000>;
> > +
> > +		gpio = <&tlmm 58 GPIO_ACTIVE_HIGH>;
> > +		enable-active-high;
> > +
> > +		pinctrl-0 = <&cam2_pwr_en>;
> > +		pinctrl-names = "default";
> > +
> > +		vin-supply = <&vreg_vcc3v3_output>;
> > +	};

> Are these supplies part of the baseboard?

Yes. These are board-level camera power switches on RUBIK Pi 3.

The public schematic shows this on page 13, in the Camera1&2 sheet:
https://thundercomm.s3-accelerate.amazonaws.com/uploads/web/rubik-pi-3/RUBIKPI3-IOB-V02-RELEASE.pdf

CAMERA1_PWR_EN and CAMERA2_PWR_EN are driven from the SOM side and gate
the camera connector power rails derived from VCC3V3_OUTPUT.

> [...]
>
> > +	ports {
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		/* CAM1 is routed to CSI1 / CSIPHY1. */

> I believe these comments are self-explanatory given the names of the labels

Ack, I will drop these comments in v2.

> [...]
>
> > +	cam1_privacy_led_state: cam1-privacy-led-state {
> > +		pins = "gpio18";
> > +		function = "gpio";
> > +		drive-strength = <8>;
> > +		output-low;

> Please drop the output-foo properties from TLMM pins, the GPIO
> state will be controlled by the driver

Ack, I will drop the output-low/output-high properties from the TLMM
pinctrl states and let the GPIO consumers drive the state.

Thanks,
Hongyang