phy: qcom-mipi-csi2: Add support for single power-domain SoCs

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:

1. qcom-mipi-csi2-phy ace6000.phy: error -EEXIST: Failed to attach power-domain list
2. 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.

devm_pm_domain_attach_list() returns the number of attached power-domains on success, so a bare "ret < 0" check in probe() is not enough to catch the case where it succeeds without actually attaching any domains. phy_qcom_mipi_csi2_attach_pm_domains() now also checks for that case directly and reports -ENODEV, keeping probe()'s own check unchanged.

Reviewed-by Jigarkumar Zala <[email protected]>

Signed-off-by Anusha Arun Nandi <[email protected]>

---
 .../phy/qualcomm/phy-qcom-mipi-csi2-core.c    | 25 +++++++++++--------
 1 file changed, 15 insertions(+), 10 deletions(-)

drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c +14 -6

index c0a4fa20eaf0..269232499fca 100644
@@ -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;
 

@@ -231,7 +231,11 @@ static int phy_qcom_mipi_csi2_attach_pm_domains(struct mipi_csi2phy_device *csi2
 {
 	struct dev_pm_domain_attach_data pd_data = { 0 };
 	const char **pd_names;
-	int i;
+	int i, ret;
+
+	/* Single power-domain devices are already attached by the bus core. */
+	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,

@@ -245,8 +249,12 @@ static int phy_qcom_mipi_csi2_attach_pm_domains(struct mipi_csi2phy_device *csi2
 	pd_data.pd_names = pd_names;
 	pd_data.num_pd_names = csi2phy->soc_cfg->num_genpds;
 
-	return devm_pm_domain_attach_list(csi2phy->dev, &pd_data,
-					  &csi2phy->pd_list);
+	ret = devm_pm_domain_attach_list(csi2phy->dev, &pd_data,
+					 &csi2phy->pd_list);
+	if (ret < 0)
+		return ret;
+
+	return 0;
 }
 
 static int phy_qcom_mipi_csi2_parse_routing(struct mipi_csi2phy_device *csi2phy)

Quoted 100 lines · Anusha Arun Nandi

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:

1. qcom-mipi-csi2-phy ace6000.phy: error -EEXIST: Failed to attach power-domain list
2. 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.

devm_pm_domain_attach_list() returns the number of attached power-domains on success, so a bare "ret < 0" check in probe() is not enough to catch the case where it succeeds without actually attaching any domains. phy_qcom_mipi_csi2_attach_pm_domains() now also checks for that case directly and reports -ENODEV, keeping probe()'s own check unchanged.

Reviewed-by Jigarkumar Zala <[email protected]>

Signed-off-by Anusha Arun Nandi <[email protected]>

---
 .../phy/qualcomm/phy-qcom-mipi-csi2-core.c    | 25 +++++++++++--------
 1 file changed, 15 insertions(+), 10 deletions(-)

drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c +14 -6

index c0a4fa20eaf0..269232499fca 100644
@@ -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;
 

@@ -231,7 +231,11 @@ static int phy_qcom_mipi_csi2_attach_pm_domains(struct mipi_csi2phy_device *csi2
 {
 	struct dev_pm_domain_attach_data pd_data = { 0 };
 	const char **pd_names;
-	int i;
+	int i, ret;
+
+	/* Single power-domain devices are already attached by the bus core. */
+	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,

@@ -245,8 +249,12 @@ static int phy_qcom_mipi_csi2_attach_pm_domains(struct mipi_csi2phy_device *csi2
 	pd_data.pd_names = pd_names;
 	pd_data.num_pd_names = csi2phy->soc_cfg->num_genpds;
 
-	return devm_pm_domain_attach_list(csi2phy->dev, &pd_data,
-					  &csi2phy->pd_list);
+	ret = devm_pm_domain_attach_list(csi2phy->dev, &pd_data,
+					 &csi2phy->pd_list);
+	if (ret < 0)
+		return ret;
+
+	return 0;
 }
 
 static int phy_qcom_mipi_csi2_parse_routing(struct mipi_csi2phy_device *csi2phy)

drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c static int phy_qcom_mipi_csi2_probe(struct platform_device *pdev) · line 380

Quoted 7 lines

drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c +1 -3

@@ -369,11 +377,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)

I thought you said you were going to fix this ?

Once done you may add

Reviewed-by Bryan O'Donoghue <[email protected]>

But not until ;)