Message-ID: <5hnd2wwkbairm36ml7l6sqj2gmm7aonyv7iadbtjm6ew266jd2@fp7f7usc7vmt>
Date: Mon, 5 Jan 2026 11:59:40 +0530
From: Manivannan Sadhasivam <mani@...nel.org>
To: Mrinmay Sarkar <mrinmay.sarkar@....qualcomm.com>
Cc: Bjorn Helgaas <bhelgaas@...gle.com>, 
	Lorenzo Pieralisi <lpieralisi@...nel.org>, Krzysztof Wilczyński <kwilczynski@...nel.org>, 
	Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, 
	Conor Dooley <conor+dt@...nel.org>, Philipp Zabel <p.zabel@...gutronix.de>, 
	Bjorn Andersson <andersson@...nel.org>, linux-arm-msm@...r.kernel.org, linux-pci@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Manivannan Sadhasivam <manivannan.sadhasivam@....qualcomm.com>, Krishna Chaitanya Chundru <krishna.chundru@....qualcomm.com>, 
	quic_vbadigan@...cinc.com, quic_shazhuss@...cinc.com, konrad.dybcio@....qualcomm.com, 
	Rama Krishna <quic_ramkri@...cinc.com>, Ayiluri Naga Rashmi <quic_nayiluri@...cinc.com>, 
	Nitesh Gupta <quic_nitegupt@...cinc.com>
Subject: Re: [PATCH v4 2/2] PCI: qcom-ep: Add support for firmware-managed
 PCIe Endpoint

On Tue, Dec 23, 2025 at 02:46:21PM +0530, Mrinmay Sarkar wrote:
> Some Qualcomm platforms use firmware to manage PCIe resources such as
> clocks, resets, and PHY through the SCMI interface. In these cases,
> the Linux driver should not perform resource enable or disable
> operations directly. Additionally, runtime PM support has been enabled
> to ensure proper power state transitions.
> 
> This commit introduces a `firmware_managed` flag in the Endpoint
> configuration structure. When set, the driver skips resource handling
> and uses generic runtime PM calls to let firmware do resource management.
> 
> A new compatible string is added for SA8255P platforms where firmware
> manages resources.
> 
> Signed-off-by: Mrinmay Sarkar <mrinmay.sarkar@....qualcomm.com>
> ---
>  drivers/pci/controller/dwc/pcie-qcom-ep.c | 82 +++++++++++++++++++++++--------
>  1 file changed, 62 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> index f1bc0ac81a928b928ab3f8cc7bf82558fc430474..3c7c2dc49f928514930f304421197435f391d88b 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> @@ -168,11 +168,13 @@ enum qcom_pcie_ep_link_status {
>   * @hdma_support: HDMA support on this SoC
>   * @override_no_snoop: Override NO_SNOOP attribute in TLP to enable cache snooping
>   * @disable_mhi_ram_parity_check: Disable MHI RAM data parity error check
> + * @firmware_managed: Set if the Endpoint controller is firmware managed
>   */
>  struct qcom_pcie_ep_cfg {
>  	bool hdma_support;
>  	bool override_no_snoop;
>  	bool disable_mhi_ram_parity_check;
> +	bool firmware_managed;
>  };
>  
>  /**
> @@ -377,10 +379,17 @@ static int qcom_pcie_enable_resources(struct qcom_pcie_ep *pcie_ep)
>  
>  static void qcom_pcie_disable_resources(struct qcom_pcie_ep *pcie_ep)
>  {
> -	icc_set_bw(pcie_ep->icc_mem, 0, 0);
> -	phy_power_off(pcie_ep->phy);
> -	phy_exit(pcie_ep->phy);
> -	clk_bulk_disable_unprepare(pcie_ep->num_clks, pcie_ep->clks);
> +	struct device *dev = pcie_ep->pci.dev;
> +	int ret;
> +
> +	pm_runtime_put(dev);
> +

I'd prefer to have the goto to skip the resource handling if 'firmware_managed'
flag is set:

	if (pcie_ep->cfg && pcie_ep->cfg->firmware_managed)
		return;

> +	if (!(pcie_ep->cfg && pcie_ep->cfg->firmware_managed)) {
> +		icc_set_bw(pcie_ep->icc_mem, 0, 0);
> +		phy_power_off(pcie_ep->phy);
> +		phy_exit(pcie_ep->phy);
> +		clk_bulk_disable_unprepare(pcie_ep->num_clks, pcie_ep->clks);
> +	}
>  }
>  
>  static int qcom_pcie_perst_deassert(struct dw_pcie *pci)
> @@ -390,12 +399,22 @@ static int qcom_pcie_perst_deassert(struct dw_pcie *pci)
>  	u32 val, offset;
>  	int ret;
>  
> -	ret = qcom_pcie_enable_resources(pcie_ep);
> -	if (ret) {
> -		dev_err(dev, "Failed to enable resources: %d\n", ret);
> +	ret = pm_runtime_resume_and_get(dev);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to enable endpoint device: %d\n", ret);
>  		return ret;
>  	}
>  
> +	/* Enable resources if Endpoint controller is not firmware-managed */

Same here:

	if (pcie_ep->cfg && pcie_ep->cfg->firmware_managed)
		goto skip_resources_enable;

> +	if (!(pcie_ep->cfg && pcie_ep->cfg->firmware_managed)) {
> +		ret = qcom_pcie_enable_resources(pcie_ep);
> +		if (ret) {
> +			dev_err(dev, "Failed to enable resources: %d\n", ret);
> +			pm_runtime_put(dev);
> +			return ret;
> +		}
> +	}
> +

skip_resources_enable:

>  	/* Perform cleanup that requires refclk */
>  	pci_epc_deinit_notify(pci->ep.epc);
>  	dw_pcie_ep_cleanup(&pci->ep);
> @@ -630,16 +649,6 @@ static int qcom_pcie_ep_get_resources(struct platform_device *pdev,
>  		return ret;
>  	}
>  
> -	pcie_ep->num_clks = devm_clk_bulk_get_all(dev, &pcie_ep->clks);
> -	if (pcie_ep->num_clks < 0) {
> -		dev_err(dev, "Failed to get clocks\n");
> -		return pcie_ep->num_clks;
> -	}
> -
> -	pcie_ep->core_reset = devm_reset_control_get_exclusive(dev, "core");
> -	if (IS_ERR(pcie_ep->core_reset))
> -		return PTR_ERR(pcie_ep->core_reset);
> -
>  	pcie_ep->reset = devm_gpiod_get(dev, "reset", GPIOD_IN);
>  	if (IS_ERR(pcie_ep->reset))
>  		return PTR_ERR(pcie_ep->reset);
> @@ -652,9 +661,22 @@ static int qcom_pcie_ep_get_resources(struct platform_device *pdev,
>  	if (IS_ERR(pcie_ep->phy))
>  		ret = PTR_ERR(pcie_ep->phy);

Though this is an optional resource, I'd prefer to wrap it under
'firmware_managed' so that it will be clear that PHY is not needed in firmware
managed solution.

>  
> -	pcie_ep->icc_mem = devm_of_icc_get(dev, "pcie-mem");
> -	if (IS_ERR(pcie_ep->icc_mem))
> -		ret = PTR_ERR(pcie_ep->icc_mem);
> +	/* Populate resources if Endpoint controller is not firmware-managed */

	if (pcie_ep->cfg && pcie_ep->cfg->firmware_managed)
		return 0;

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Powered by blists - more mailing lists