Re: [PATCH 1/7] phy: qcom-mipi-csi2: fix -EEXIST probe failure on single power-domain SoCs

[Bryan O'Donoghue <bod@kernel.org>]

On 17/10/2026 00:54, Anusha Arun Nandi wrote:
> sa8775p, qcs8300 and sc7280 each list exactly one power-domain ("top")
> on the CSI2 PHY node. For a device with a single power-domain phandle,
> the generic platform-bus code (platform_probe() ->
> dev_pm_domain_attach()) auto-attaches it via genpd_dev_pm_attach()
> before the driver's own probe() runs. phy_qcom_mipi_csi2_probe() then
> unconditionally calls devm_pm_domain_attach_list(), which fails with
> -EEXIST because dev->pm_domain is already set, and the driver treats
> that as fatal:
> 
>    qcom-mipi-csi2-phy ace6000.phy: error -EEXIST: Failed to attach
>    power-domain list
>    qcom-mipi-csi2-phy ace6000.phy: probe with driver qcom-mipi-csi2-phy
>    failed with error -17
> 
> x1e80100 is unaffected: it lists three power-domains, so the generic
> code does not auto-attach any of them (auto-attach only applies to
> single-power-domain devices); the driver's explicit attach is the
> only one that runs.
> 
> For the single power-domain case, call dev_pm_domain_attach() instead
> of devm_pm_domain_attach_list(): it returns early when dev->pm_domain
> is already set, so csi2phy->pd_list is left NULL rather than built up
> by hand. The three call sites that walk pd_list for genpd
> performance-state scaling now guard against a NULL pd_list, which is
> harmless since scaling is unused (scaled = false) on every SoC that has
> a single "top" domain.

Fix implies bug but, this patch really adds support for one power-domain 
cases.

I think the patch title should be something like "Add support for single 
power-domain case"

or if you believe it is a bugfix level change add

Fixes:
Cc: stable@vger.kernel.org

My own preference is "Add support" since the driver hasn't been designed 
to support single PD cases thus far but, OTOH you could make an argument 
that that is in fact a design bug.

> Signed-off-by: Anusha Arun Nandi <anusha.nandi@oss.qualcomm.com>
> ---
>   .../phy/qualcomm/phy-qcom-mipi-csi2-core.c    | 24 +++++++++++++------
>   1 file changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c
> index c0a4fa20eaf0..00285be9dd64 100644
> --- a/drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c
> +++ b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c
> @@ -43,7 +43,7 @@ phy_qcom_mipi_csi2_set_clock_rates(struct mipi_csi2phy_device *csi2phy,
>   	}
> 
>   	pstate = 0;
> -	for (i = 0; i < csi2phy->pd_list->num_pds; i++) {
> +	for (i = 0; csi2phy->pd_list && i < csi2phy->pd_list->num_pds; i++) {
>   		unsigned int perf;
> 
>   		if (!csi2phy->soc_cfg->genpds[i].scaled)
> @@ -174,7 +174,7 @@ static int phy_qcom_mipi_csi2_power_on(struct phy *phy)
>   unset_rate:
>   	dev_pm_opp_set_rate(csi2phy->dev, 0);
> 
> -	for (i = 0; i < csi2phy->pd_list->num_pds; i++) {
> +	for (i = 0; csi2phy->pd_list && i < csi2phy->pd_list->num_pds; i++) {
>   		if (!csi2phy->soc_cfg->genpds[i].scaled)
>   			continue;
> 
> @@ -204,7 +204,7 @@ static int phy_qcom_mipi_csi2_power_off(struct phy *phy)
> 
>   	dev_pm_opp_set_rate(csi2phy->dev, 0);
> 
> -	for (i = 0; i < csi2phy->pd_list->num_pds; i++) {
> +	for (i = 0; csi2phy->pd_list && i < csi2phy->pd_list->num_pds; i++) {
>   		if (!csi2phy->soc_cfg->genpds[i].scaled)
>   			continue;
> 
> @@ -233,6 +233,19 @@ static int phy_qcom_mipi_csi2_attach_pm_domains(struct mipi_csi2phy_device *csi2
>   	const char **pd_names;
>   	int i;
> 
> +	/*
> +	 * A device with a single power-domain entry in the devicetree is
> +	 * already attached by the generic platform-bus code before probe()
> +	 * runs (genpd_dev_pm_attach()), so calling devm_pm_domain_attach_list()
> +	 * here would fail with -EEXIST. dev_pm_domain_attach() returns early
> +	 * in that case (dev->pm_domain already set) instead of leaving
> +	 * csi2phy->pd_list unset; this is fine since pd_list is only used for
> +	 * performance-state scaling, which is unused ("scaled = false") on
> +	 * every SoC that has a single "top" power-domain.
> +	 */

This is alot of comment - looks like LLM babble to me. I think it can be 
dropped the code is pretty obvious, if num_genpds == 1 do this other 
thing here.

> +	if (csi2phy->soc_cfg->num_genpds == 1)
> +		return dev_pm_domain_attach(csi2phy->dev, 0);
> +
>   	pd_names = devm_kzalloc(csi2phy->dev,
>   				sizeof(char *) * csi2phy->soc_cfg->num_genpds,
>   				GFP_KERNEL);
> @@ -369,11 +382,8 @@ static int phy_qcom_mipi_csi2_probe(struct platform_device *pdev)
>   		return ret;
> 
>   	ret = phy_qcom_mipi_csi2_attach_pm_domains(csi2phy);
> -	if (ret < 0 || csi2phy->pd_list == NULL) {
> -		if (ret == 0)
> -			ret = -ENODEV;
> +	if (ret < 0)
>   		return dev_err_probe(dev, ret, "Failed to attach power-domain list\n");
> -	}
> 
>   	ret = devm_pm_runtime_enable(dev);
>   	if (ret < 0)
> --
> 2.34.1