Re: [PATCH 3/3] arm64: dts: qcom: sm7225-fairphone-fp4: Add Sony IMX576 front camera support

[Himanshu Bhavani]

Hi Luca,

>Hi Himanshu,
>
>On Wed May 20, 2026 at 1:56 PM CEST, Himanshu Bhavani wrote:
>> From: Hardevsinh Palaniya <hardevsinh.palaniya@xxxxxxxxxxxxxxxxx>
>>
>> Add device tree support for the Sony IMX576 front camera
>> sensor and connect it to CAMSS via CSIPHY3.
>>
>> Signed-off-by: Hardevsinh Palaniya <hardevsinh.palaniya@xxxxxxxxxxxxxxxxx>
>
>Your Signed-off-by is missing here as well

Noted, I will add it in the next revision.

>
>> ---
>>  .../boot/dts/qcom/sm7225-fairphone-fp4.dts    | 54 +++++++++++++++++++
>>  1 file changed, 54 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts b/arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts
>> index 3964aae47fd4..5fb0f3eddf66 100644
>> --- a/arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts
>> +++ b/arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts
>> @@ -17,6 +17,7 @@
>>  #include <dt-bindings/iio/qcom,spmi-adc7-pmk8350.h>
>>  #include <dt-bindings/input/input.h>
>>  #include <dt-bindings/leds/common.h>
>> +#include <dt-bindings/media/video-interfaces.h>
>>  #include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
>>  #include <dt-bindings/regulator/qcom,rpmh-regulator.h>
>>  #include <dt-bindings/sound/qcom,q6asm.h>
>> @@ -547,6 +548,24 @@ vreg_bob: bob {
>>  	};
>>  };
>>
>> +&camss {
>> +
>
>random extra newline
>
>> +	vdd-csiphy3-0p9-supply = <&vreg_l18a>;
>> +	vdd-csiphy3-1p25-supply = <&vreg_l22a>;
>
>If you're already enabling that, please also add the supplies for the
>other CSIPHYs (0-3).

I will add the remaining CSIPHYs supplies as well. I initially thought they were unnecessary since only IMX576 is added.

>
>But also, since camss for sm6350 hasn't been merged upstream yet, this
>patchset depends on
>https://lore.kernel.org/linux-arm-msm/20260216-sm6350-camss-v4-0-b9df35f87edb@xxxxxxxxxxxxx/
>You need to declare that in the cover letter!

Sure, I’ll mention it in the cover letter.

>
>> +
>> +	status = "okay";
>> +
>> +	ports {
>> +		port@3 {
>> +			csiphy3_ep: endpoint {
>> +				data-lanes = <0 1 2 3>;
>> +				bus-type = <MEDIA_BUS_TYPE_CSI2_DPHY>;
>> +				remote-endpoint = <&camera_imx576_ep>;
>> +			};
>> +		};
>> +	};
>> +};
>> +
>>  &cci0 {
>>  	status = "okay";
>>  };
>> @@ -582,6 +601,34 @@ &cci1 {
>>
>>  &cci1_i2c0 {
>>  	/* Front cam (Sony IMX576) @ 0x10 */
>
>Remove the comment now since you're adding the node.

Noted.

>
>> +	camera@10 {
>> +		compatible = "sony,imx576";
>> +		reg = <0x10>;
>> +
>> +		avdd-supply = <&vreg_l3p>;
>> +		dovdd-supply = <&vreg_32m_cam_dvdd_1p05>;
>> +		dvdd-supply = <&vreg_l6p>;
>> +
>> +		clocks = <&camcc CAMCC_MCLK1_CLK>;
>> +		assigned-clocks = <&camcc CAMCC_MCLK1_CLK>;
>> +		assigned-clock-rates = <24000000>;
>> +
>> +		reset-gpios = <&tlmm 35 GPIO_ACTIVE_LOW>;
>> +		pinctrl-0 = <&cam_mclk1_default>;
>> +		pinctrl-names = "default";
>> +
>> +		orientation = <0>;
>
>I personally like adding a comment to make clear what '0' is:
>
>		orientation = <0>; /* Front facing */

I will add the comment.

>
>> +		rotation = <90>;
>> +
>> +		port {
>> +			camera_imx576_ep: endpoint {
>> +				data-lanes = <1 2 3 4>;
>> +				bus-type = <MEDIA_BUS_TYPE_CSI2_DPHY>;
>> +				link-frequencies = /bits/ 64 <600000000>;
>> +				remote-endpoint = <&csiphy3_ep>;
>> +			};
>> +		};
>> +	};
>>
>>  	eeprom@50 {
>>  		compatible = "giantec,gt24p64a", "atmel,24c64";
>> @@ -1115,6 +1162,13 @@ platform {
>>  &tlmm {
>>  	gpio-reserved-ranges = <13 4>, <56 2>;
>>
>> +	cam_mclk1_default: cam-mclk1-default-state {
>> +		pins = "gpio30";
>> +		function = "cam_mclk1";
>> +		drive-strength = <2>;
>> +		bias-disable;
>> +	};
>
>I think I was told before that this can go into the SoC .dtsi
>
>Regards
>Luca

Right, I’ll move it to the SoC .dtsi file.

Best Regards,
Himanshu