Re: [PATCH v7 06/12] arm64: dts: qcom: x1e80100-crd: Add ov08x40 RGB sensor on CSIPHY4

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] arm64: dts: qcom: x1e80100-crd: Broken CAMSS OF graph and missing PHY regulators
--

commit ffc60037283c2ea9b513225f3dfd676a7b8869ad
Author: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

arm64: dts: qcom: x1e80100-crd: Add ov08x40 RGB sensor on CSIPHY4
    
Define ov08x40 on cci1_i2c1. The RGB sensor appears on the AON CCI pins
connected to CSIPHY4 in four lane mode.

> diff --git a/arch/arm64/boot/dts/qcom/x1-crd.dtsi b/arch/arm64/boot/dts/qcom/x1-crd.dtsi
> index 47cfda908c515..a98f5c5b623bc 100644
> --- a/arch/arm64/boot/dts/qcom/x1-crd.dtsi
> +++ b/arch/arm64/boot/dts/qcom/x1-crd.dtsi

[ ... ]

> @@ -933,6 +935,70 @@ &gpu {
>  	status = "okay";
>  };
>  
> +&camss {
> +	status = "okay";
> +};

[Severity: High]
Will the upstream driver fail to manage the CSIPHY block since this node
is missing the vdd-csiphy-0p8-supply and vdd-csiphy-1p2-supply properties?

The upstream camss.c driver manages up the CSIPHY block natively and
requires its regulators to be defined on the camss node itself.

> +
> +&camss_csiphy4_in_port {
> +	camss_csiphy4_inep: endpoint {
> +		data-lanes = <1 2 3 4>;
> +		remote-endpoint = <&csiphy4_out_ep>;
> +	};
> +};

[Severity: High]
Is it intended to connect the CAMSS port to the intermediate PHY node
instead of directly to the sensor here?

> +
> +&cci1 {
> +	status = "okay";
> +};
> +
> +&cci1_i2c1 {
> +	camera@36 {

[ ... ]

> +		port {
> +			ov08x40_out_ep: endpoint {
> +				data-lanes = <1 2 3 4>;
> +				link-frequencies = /bits/ 64 <400000000>;
> +				remote-endpoint = <&csiphy4_in_ep>;
> +			};
> +		};
> +	};
> +};

[Severity: High]
Does routing the sensor output to the intermediate PHY node instead of the
CAMSS node break sensor discovery?

The CAMSS driver discovers sensors by iterating over its own ports and
expecting remote endpoints to be V4L2 subdevices.

> +
> +&csiphy4 {
> +	vdda-0p9-supply = <&vreg_l2c_0p8>;
> +	vdda-1p2-supply = <&vreg_l1c_1p2>;
> +
> +	status = "okay";
> +};

[Severity: High]
Because there is no upstream Linux driver for this standalone CSIPHY node,
will these regulators ever be requested and enabled?

> +
> +&csiphy4_in_port {
> +	csiphy4_in_ep: endpoint {
> +		bus-type = <MEDIA_BUS_TYPE_CSI2_DPHY>;
> +		data-lanes = <1 2 3 4>;
> +		remote-endpoint = <&ov08x40_out_ep>;
> +	};
> +};
> +
> +&csiphy4_out_port {
> +	csiphy4_out_ep: endpoint {
> +		remote-endpoint = <&camss_csiphy4_inep>;
> +	};
> +};

[Severity: High]
Do these intermediate OF graph links between the sensor, the split CSIPHY,
and CAMSS prevent the V4L2 async notifier from registering the subdevice?

The CAMSS V4L2 async notifier in camss_parse_ports() might wait indefinitely
for a non-existent csiphy4 subdevice to register, preventing pipeline probe.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260917-x1e-camss-csi2-phy-dtsi-v7-0-1a63eb35838b@linaro.org?part=6