diff mbox series

PCI: qcom: Use OPP only if the platform supports it

Message ID 20240722131128.32470-1-manivannan.sadhasivam@linaro.org (mailing list archive)
State Accepted
Delegated to: Krzysztof Wilczyński
Headers show
Series PCI: qcom: Use OPP only if the platform supports it | expand

Commit Message

Manivannan Sadhasivam July 22, 2024, 1:11 p.m. UTC
With commit 5b6272e0efd5 ("PCI: qcom: Add OPP support to scale
performance"), OPP was used to control the interconnect and power domains
if the platform supported OPP. Also to maintain the backward compatibility
with platforms not supporting OPP but just ICC, the above mentioned commit
assumed that if ICC was not available on the platform, it would resort to
OPP.

Unfortunately, some old platforms don't support either ICC or OPP. So on
those platforms, resorting to OPP in the absence of ICC throws below errors
from OPP core during suspend and resume:

qcom-pcie 1c08000.pcie: dev_pm_opp_set_opp: device opp doesn't exist
qcom-pcie 1c08000.pcie: _find_key: OPP table not found (-19)

Also, it doesn't make sense to invoke the OPP APIs when OPP is not
supported by the platform at all. So let's use a flag to identify whether
OPP is supported by the platform or not and use it to control invoking the
OPP APIs.

Fixes: 5b6272e0efd5 ("PCI: qcom: Add OPP support to scale performance")
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

Mayank Rana July 22, 2024, 9:47 p.m. UTC | #1
Hi Mani

On 7/22/2024 6:11 AM, Manivannan Sadhasivam wrote:
> With commit 5b6272e0efd5 ("PCI: qcom: Add OPP support to scale
> performance"), OPP was used to control the interconnect and power domains
> if the platform supported OPP. Also to maintain the backward compatibility
> with platforms not supporting OPP but just ICC, the above mentioned commit
> assumed that if ICC was not available on the platform, it would resort to
> OPP.
> 
> Unfortunately, some old platforms don't support either ICC or OPP. So on
> those platforms, resorting to OPP in the absence of ICC throws below errors
> from OPP core during suspend and resume:
> 
> qcom-pcie 1c08000.pcie: dev_pm_opp_set_opp: device opp doesn't exist
> qcom-pcie 1c08000.pcie: _find_key: OPP table not found (-19)
> 
> Also, it doesn't make sense to invoke the OPP APIs when OPP is not
> supported by the platform at all. So let's use a flag to identify whether
> OPP is supported by the platform or not and use it to control invoking the
> OPP APIs.
> 
> Fixes: 5b6272e0efd5 ("PCI: qcom: Add OPP support to scale performance")
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>   drivers/pci/controller/dwc/pcie-qcom.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 0180edf3310e..6f953e32d990 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -261,6 +261,7 @@ struct qcom_pcie {
>   	const struct qcom_pcie_cfg *cfg;
>   	struct dentry *debugfs;
>   	bool suspended;
> +	bool use_pm_opp;
>   };
>   
>   #define to_qcom_pcie(x)		dev_get_drvdata((x)->dev)
> @@ -1433,7 +1434,7 @@ static void qcom_pcie_icc_opp_update(struct qcom_pcie *pcie)
>   			dev_err(pci->dev, "Failed to set bandwidth for PCIe-MEM interconnect path: %d\n",
>   				ret);
>   		}
> -	} else {
> +	} else if (pcie->use_pm_opp) {
>   		freq_mbps = pcie_dev_speed_mbps(pcie_link_speed[speed]);
>   		if (freq_mbps < 0)
>   			return;
> @@ -1592,6 +1593,8 @@ static int qcom_pcie_probe(struct platform_device *pdev)
>   				      max_freq);
>   			goto err_pm_runtime_put;
>   		}
> +
> +		pcie->use_pm_opp = true;
>   	} else {
>   		/* Skip ICC init if OPP is supported as it is handled by OPP */
>   		ret = qcom_pcie_icc_init(pcie);
> @@ -1683,7 +1686,7 @@ static int qcom_pcie_suspend_noirq(struct device *dev)
>   		if (ret)
>   			dev_err(dev, "Failed to disable CPU-PCIe interconnect path: %d\n", ret);
>   
> -		if (!pcie->icc_mem)
> +		if (pcie->use_pm_opp)
>   			dev_pm_opp_set_opp(pcie->pci->dev, NULL);
>   	}
>   	return ret;
 >
Reviewed-by: Mayank Rana <quic_mrana@quicinc.com>

Regards,
Mayank
Krzysztof Wilczyński Aug. 13, 2024, 8:24 p.m. UTC | #2
Hello,

> With commit 5b6272e0efd5 ("PCI: qcom: Add OPP support to scale
> performance"), OPP was used to control the interconnect and power domains
> if the platform supported OPP. Also to maintain the backward compatibility
> with platforms not supporting OPP but just ICC, the above mentioned commit
> assumed that if ICC was not available on the platform, it would resort to
> OPP.
> 
> Unfortunately, some old platforms don't support either ICC or OPP. So on
> those platforms, resorting to OPP in the absence of ICC throws below errors
> from OPP core during suspend and resume:
> 
> qcom-pcie 1c08000.pcie: dev_pm_opp_set_opp: device opp doesn't exist
> qcom-pcie 1c08000.pcie: _find_key: OPP table not found (-19)
> 
> Also, it doesn't make sense to invoke the OPP APIs when OPP is not
> supported by the platform at all. So let's use a flag to identify whether
> OPP is supported by the platform or not and use it to control invoking the
> OPP APIs.

Applied to controller/qcom, thank you!

[1/1] PCI: qcom: Use OPP only if the platform supports it
      https://git.kernel.org/pci/pci/c/d0fa8ca89100

	Krzysztof
Krzysztof Wilczyński Sept. 1, 2024, 4:29 p.m. UTC | #3
[...]
> Applied to controller/qcom, thank you!

Bjorn included the patch as part of his recent Pull Request with assorted
PCI tree fixes, as such, I removed this patch from the branch, since it's
upstream already.

	Krzysztof
diff mbox series

Patch

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 0180edf3310e..6f953e32d990 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -261,6 +261,7 @@  struct qcom_pcie {
 	const struct qcom_pcie_cfg *cfg;
 	struct dentry *debugfs;
 	bool suspended;
+	bool use_pm_opp;
 };
 
 #define to_qcom_pcie(x)		dev_get_drvdata((x)->dev)
@@ -1433,7 +1434,7 @@  static void qcom_pcie_icc_opp_update(struct qcom_pcie *pcie)
 			dev_err(pci->dev, "Failed to set bandwidth for PCIe-MEM interconnect path: %d\n",
 				ret);
 		}
-	} else {
+	} else if (pcie->use_pm_opp) {
 		freq_mbps = pcie_dev_speed_mbps(pcie_link_speed[speed]);
 		if (freq_mbps < 0)
 			return;
@@ -1592,6 +1593,8 @@  static int qcom_pcie_probe(struct platform_device *pdev)
 				      max_freq);
 			goto err_pm_runtime_put;
 		}
+
+		pcie->use_pm_opp = true;
 	} else {
 		/* Skip ICC init if OPP is supported as it is handled by OPP */
 		ret = qcom_pcie_icc_init(pcie);
@@ -1683,7 +1686,7 @@  static int qcom_pcie_suspend_noirq(struct device *dev)
 		if (ret)
 			dev_err(dev, "Failed to disable CPU-PCIe interconnect path: %d\n", ret);
 
-		if (!pcie->icc_mem)
+		if (pcie->use_pm_opp)
 			dev_pm_opp_set_opp(pcie->pci->dev, NULL);
 	}
 	return ret;