Re: [PATCH v3 1/3] arm64: dts: qcom: lemans: add CSI2 D-PHY common-phy providers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] arm64: dts: qcom: lemans: undocumented qcom,sa8775p-csi2-phy compatible
- [Medium] arm64: dts: qcom: lemans: single power-domain in csiphy0..3 causes -EEXIST
--

commit 03beee1895ec9b19424b0689817a5a264d09433f
Author: Anusha Arun Nandi <anusha.nandi@oss.qualcomm.com>

arm64: dts: qcom: lemans: add CSI2 D-PHY common-phy providers

This commit adds four standalone qcom,sa8775p-csi2-phy generic-PHY provider
nodes (csiphy0..3) and their shared OPP table to lemans.dtsi so boards can
wire camera sensors through the non-legacy CAMSS PHY path.

> diff --git a/arch/arm64/boot/dts/qcom/lemans.dtsi b/arch/arm64/boot/dts/qcom/lemans.dtsi
> index 1fdb72fa55184..6909dfa06d867 100644
> --- a/arch/arm64/boot/dts/qcom/lemans.dtsi
> +++ b/arch/arm64/boot/dts/qcom/lemans.dtsi

[ ... ]

> @@ -5348,6 +5356,134 @@ port@3 {
>  			};
>  		};
>  
> +		csiphy0: phy@ac9c000 {
> +			compatible = "qcom,sa8775p-csi2-phy";

[Severity: Medium]
As reported by Sashiko on v1 [1], is there a devicetree binding update and
driver support accompanying qcom,sa8775p-csi2-phy (used on csiphy0..csiphy3)?

In Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml, only
qcom,x1e80100-csi2-phy and qcom,glymur-csi2-phy are documented, so dtbs_check
will fail on these nodes.

In addition, phy_qcom_mipi_csi2_of_match_table in
drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c only matches
qcom,x1e80100-csi2-phy:

static const struct of_device_id phy_qcom_mipi_csi2_of_match_table[] = {
	{ .compatible	= "qcom,x1e80100-csi2-phy", .data = &mipi_csi2_dphy_4nm_x1e },
	{ }
};

When a board or overlay enables one of csiphy0..csiphy3 and wires a sensor
endpoint through it, camss_probe() -> camss_init_subdevices() ->
camss_detect_legacy_phy() in drivers/media/platform/qcom/camss/camss.c sees
the remote port parent node named phy and sets camss->legacy_phy = false.
Then msm_csiphy_subdev_init() in
drivers/media/platform/qcom/camss/camss-csiphy.c calls
devm_phy_get_by_of_node(dev, remote):

	csiphy->phy = devm_phy_get_by_of_node(dev, remote);
	of_node_put(remote);
	if (IS_ERR(csiphy->phy)) {
		ret = PTR_ERR(csiphy->phy);
		goto done;
	}

Without a driver matching qcom,sa8775p-csi2-phy to register a PHY provider
(and a corresponding 5nm v1.3.0 PHY configuration in
drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c), won't
devm_phy_get_by_of_node() return -EPROBE_DEFER indefinitely?

> +			reg = <0x0 0x0ac9c000 0x0 0x2000>;
> +
> +			clocks = <&camcc CAM_CC_CSIPHY0_CLK>,
> +				 <&camcc CAM_CC_CSI0PHYTIMER_CLK>;
> +			clock-names = "core",
> +				      "timer";
> +
> +			interrupts = <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>;
> +
> +			operating-points-v2 = <&csiphy_opp_table>;
> +
> +			power-domains = <&camcc CAM_CC_TITAN_TOP_GDSC>;
> +			power-domain-names = "top";

[Severity: Medium]
As reported by Sashiko on v2 [2], will phy_qcom_mipi_csi2_probe() fail with
-EEXIST when power-domains only contains a single entry on csiphy0..csiphy3?

When a platform device has a single power-domains entry in DT,
platform_probe() -> dev_pm_domain_attach() -> genpd_dev_pm_attach() in
drivers/pmdomain/core.c attaches that domain directly to dev->pm_domain before
the driver's probe callback runs:

	if (of_count_phandle_with_args(dev->of_node, "power-domains",
				       "#power-domain-cells") != 1)
		return 0;

	return __genpd_dev_pm_attach(dev, dev, 0, 1, true);

When phy_qcom_mipi_csi2_probe() in
drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c subsequently calls
phy_qcom_mipi_csi2_attach_pm_domains() -> devm_pm_domain_attach_list() ->
dev_pm_domain_attach_list() in drivers/base/power/common.c:

	if (dev->pm_domain)
		return -EEXIST;

dev_pm_domain_attach_list() returns -EEXIST because dev->pm_domain is already
set, causing phy_qcom_mipi_csi2_probe() to fail. Furthermore, both
Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml and
drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c expect three power domains
(top, mmcx, mx) and three clocks (core, timer, ahb).

[1] https://lore.kernel.org/r/sashiko-outbox-175063@kernel.org
[2] https://lore.kernel.org/r/sashiko-outbox-196105@kernel.org

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106030212.2811027-1-anusha.nandi@oss.qualcomm.com?part=1