[PATCH v4 0/3] PCI: dwc: Skip waiting for link up if vendor drivers can detect Link up event

[PATCH v4 0/3] PCI: dwc: Skip waiting for link up if vendor drivers can detect Link up event

[Krishna chaitanya chundru]

If the vendor drivers can detect the Link up event using mechanisms
such as Link up IRQ, then waiting for Link up during probe is not
needed. if the drivers can be notified when the link comes up,
vendor driver can enumerate downstream devices instead of waiting
here, which optimizes the boot time.

So skip waiting for link to be up if the driver supports 'linkup_irq'.

Currently, only Qcom RC driver supports the 'linkup_irq' as it can detect
the Link Up event using its own 'global IRQ' interrupt. So set
'linkup_irq' flag for QCOM drivers.

As part of the PCIe link up event, the ICC and OPP values are updated.

Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
---
Changes in v4:
- change the linkup_irq name to use_linkup_irq a suggested by (bjorn
  andresson)
- update commit text as suggested by bjorn andresson.
- Link to v3: https://lore.kernel.org/r/linux-arm-msm/20241101-remove_wait-v3-0-7accf27f7202@quicinc.com/T/
Changes in v3:
- seperate dwc changes and qcom changes as suggested (mani)
- update commit & comments as suggested (mani & bjorn)
- Link to v2: https://lore.kernel.org/linux-pci/20240920-remove_wait-v2-0-7c0fcb3b581d@quicinc.com/T/
Changes in v2:
- Updated the bypass_link_up_wait name to linkup_irq  & added comment as
  suggested (mani).
- seperated the icc and opp update patch (mani).
- Link to v1: https://lore.kernel.org/r/20240917-remove_wait-v1-1-456d2551bc50@quicinc.com

---
Krishna chaitanya chundru (3):
      PCI: dwc: Skip waiting for link up if vendor drivers can detect Link up event
      PCI: qcom: Set linkup_irq if global IRQ handler is present
      PCI: qcom: Update ICC and OPP values during link up event

 drivers/pci/controller/dwc/pcie-designware-host.c | 10 ++++++++--
 drivers/pci/controller/dwc/pcie-designware.h      |  1 +
 drivers/pci/controller/dwc/pcie-qcom.c            |  7 ++++++-
 3 files changed, 15 insertions(+), 3 deletions(-)
---
base-commit: 624ce1863d33298cd1623e51006147e4076ed2ca
change-id: 20241114-remove_wait1-289beeba1989

Best regards,
-- 
Krishna chaitanya chundru <quic_krichai@quicinc.com>

[PATCH v4 1/3] PCI: dwc: Skip waiting for link up if vendor drivers can detect Link up event

[Krishna chaitanya chundru]

If the vendor drivers can detect the Link up event using mechanisms
such as Link up IRQ and can the driver can enumerate downstream devices
instead of waiting here, then waiting for Link up during probe is not
needed here, which optimizes the boot time.

So skip waiting for link to be up if the driver supports 'linkup_irq'.

Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
---
 drivers/pci/controller/dwc/pcie-designware-host.c | 10 ++++++++--
 drivers/pci/controller/dwc/pcie-designware.h      |  1 +
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 3e41865c7290..c8208a6c03d1 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -530,8 +530,14 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
 			goto err_remove_edma;
 	}
 
-	/* Ignore errors, the link may come up later */
-	dw_pcie_wait_for_link(pci);
+	/*
+	 * Note: The link up delay is skipped only when a link up IRQ is present.
+	 * This flag should not be used to bypass the link up delay for arbitrary
+	 * reasons.
+	 */
+	if (!pp->use_linkup_irq)
+		/* Ignore errors, the link may come up later */
+		dw_pcie_wait_for_link(pci);
 
 	bridge->sysdata = pp;
 
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 347ab74ac35a..1d0ec47e1986 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -379,6 +379,7 @@ struct dw_pcie_rp {
 	bool			use_atu_msg;
 	int			msg_atu_index;
 	struct resource		*msg_res;
+	bool			use_linkup_irq;
 };
 
 struct dw_pcie_ep_ops {

-- 
2.34.1

Re: [PATCH v4 1/3] PCI: dwc: Skip waiting for link up if vendor drivers can detect Link up event

[Manivannan Sadhasivam]

On Fri, Nov 15, 2024 at 04:00:21PM +0530, Krishna chaitanya chundru wrote:
> If the vendor drivers can detect the Link up event using mechanisms
> such as Link up IRQ and can the driver can enumerate downstream devices

"if the driver can..."

> instead of waiting here, then waiting for Link up during probe is not
> needed here, which optimizes the boot time.
> 
> So skip waiting for link to be up if the driver supports 'linkup_irq'.
> 

s/linkup_irq/use_linkup_irq

> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>

With above,

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

- Mani

> ---
>  drivers/pci/controller/dwc/pcie-designware-host.c | 10 ++++++++--
>  drivers/pci/controller/dwc/pcie-designware.h      |  1 +
>  2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 3e41865c7290..c8208a6c03d1 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -530,8 +530,14 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
>  			goto err_remove_edma;
>  	}
>  
> -	/* Ignore errors, the link may come up later */
> -	dw_pcie_wait_for_link(pci);
> +	/*
> +	 * Note: The link up delay is skipped only when a link up IRQ is present.
> +	 * This flag should not be used to bypass the link up delay for arbitrary
> +	 * reasons.
> +	 */
> +	if (!pp->use_linkup_irq)
> +		/* Ignore errors, the link may come up later */
> +		dw_pcie_wait_for_link(pci);
>  
>  	bridge->sysdata = pp;
>  
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 347ab74ac35a..1d0ec47e1986 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -379,6 +379,7 @@ struct dw_pcie_rp {
>  	bool			use_atu_msg;
>  	int			msg_atu_index;
>  	struct resource		*msg_res;
> +	bool			use_linkup_irq;
>  };
>  
>  struct dw_pcie_ep_ops {
> 
> -- 
> 2.34.1
> 

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

[PATCH v4 2/3] PCI: qcom: Set linkup_irq if global IRQ handler is present

[Krishna chaitanya chundru]

In cases where a global IRQ handler is present to manage link up
interrupts, it may not be necessary to wait for the link to be up
during PCI initialization which optimizes the bootup time.

So, set use_linkup_irq flag if global IRQ is present and In order to set
the use_linkup_irq flag before calling dw_pcie_host_init() API, which
waits for link to be up, move platform_get_irq_byname_optional() API
above dw_pcie_host_init().

Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index ef44a82be058..c39d1c55b50e 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -1692,6 +1692,10 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, pcie);
 
+	irq = platform_get_irq_byname_optional(pdev, "global");
+	if (irq > 0)
+		pp->use_linkup_irq = true;
+
 	ret = dw_pcie_host_init(pp);
 	if (ret) {
 		dev_err(dev, "cannot initialize host\n");
@@ -1705,7 +1709,6 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 		goto err_host_deinit;
 	}
 
-	irq = platform_get_irq_byname_optional(pdev, "global");
 	if (irq > 0) {
 		ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
 						qcom_pcie_global_irq_thread,

-- 
2.34.1

Re: [PATCH v4 2/3] PCI: qcom: Set linkup_irq if global IRQ handler is present

[Manivannan Sadhasivam]

On Fri, Nov 15, 2024 at 04:00:22PM +0530, Krishna chaitanya chundru wrote:
> In cases where a global IRQ handler is present to manage link up
> interrupts, it may not be necessary to wait for the link to be up
> during PCI initialization which optimizes the bootup time.
> 
> So, set use_linkup_irq flag if global IRQ is present and In order to set

s/In/in

> the use_linkup_irq flag before calling dw_pcie_host_init() API, which
> waits for link to be up, move platform_get_irq_byname_optional() API

s/API/call

> above dw_pcie_host_init().
> 
> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

- Mani

> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index ef44a82be058..c39d1c55b50e 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -1692,6 +1692,10 @@ static int qcom_pcie_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, pcie);
>  
> +	irq = platform_get_irq_byname_optional(pdev, "global");
> +	if (irq > 0)
> +		pp->use_linkup_irq = true;
> +
>  	ret = dw_pcie_host_init(pp);
>  	if (ret) {
>  		dev_err(dev, "cannot initialize host\n");
> @@ -1705,7 +1709,6 @@ static int qcom_pcie_probe(struct platform_device *pdev)
>  		goto err_host_deinit;
>  	}
>  
> -	irq = platform_get_irq_byname_optional(pdev, "global");
>  	if (irq > 0) {
>  		ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
>  						qcom_pcie_global_irq_thread,
> 
> -- 
> 2.34.1
> 

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

[PATCH v4 3/3] PCI: qcom: Update ICC and OPP values during link up event

[Krishna chaitanya chundru]

As the wait for linkup is removed if there is a global IRQ support,
there is no guarantee that the correct icc and opp votes are updated
as part of probe.

And also global IRQ is being used as hotplug event in case link hasn't
come up as part probe, link up IRQ is the correct place to update the
ICC and OPP votes.

So, as part of the PCIe link up event, update ICC and OPP values.

Fixes: 4581403f6792 ("PCI: qcom: Enumerate endpoints based on Link up event in 'global_irq' interrupt")
Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index c39d1c55b50e..39f5c782e2c3 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -1558,6 +1558,8 @@ static irqreturn_t qcom_pcie_global_irq_thread(int irq, void *data)
 		pci_lock_rescan_remove();
 		pci_rescan_bus(pp->bridge->bus);
 		pci_unlock_rescan_remove();
+
+		qcom_pcie_icc_opp_update(pcie);
 	} else {
 		dev_WARN_ONCE(dev, 1, "Received unknown event. INT_STATUS: 0x%08x\n",
 			      status);

-- 
2.34.1

Re: [PATCH v4 3/3] PCI: qcom: Update ICC and OPP values during link up event

[Manivannan Sadhasivam]

On Fri, Nov 15, 2024 at 04:00:23PM +0530, Krishna chaitanya chundru wrote:
> As the wait for linkup is removed if there is a global IRQ support,
> there is no guarantee that the correct icc and opp votes are updated
> as part of probe.
> 
> And also global IRQ is being used as hotplug event in case link hasn't
> come up as part probe, link up IRQ is the correct place to update the
> ICC and OPP votes.
> 
> So, as part of the PCIe link up event, update ICC and OPP values.
> 

How about,

"The commit added the Link up based enumeration support failed to update the
ICC/OPP vote once link is up. Earlier, the update happens during probe and the
endpoints may or may not be enumerated at that time. So the ICC/OPP vote was not
guaranteed to be accurate. Now with the Link up based enumeration support, the
driver can request the accurate vote based on the PCIe link.

So call qcom_pcie_icc_opp_update() in qcom_pcie_global_irq_thread() after
enumerating the endpoints."

> Fixes: 4581403f6792 ("PCI: qcom: Enumerate endpoints based on Link up event in 'global_irq' interrupt")
> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>

With above,

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

- Mani

> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index c39d1c55b50e..39f5c782e2c3 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -1558,6 +1558,8 @@ static irqreturn_t qcom_pcie_global_irq_thread(int irq, void *data)
>  		pci_lock_rescan_remove();
>  		pci_rescan_bus(pp->bridge->bus);
>  		pci_unlock_rescan_remove();
> +
> +		qcom_pcie_icc_opp_update(pcie);
>  	} else {
>  		dev_WARN_ONCE(dev, 1, "Received unknown event. INT_STATUS: 0x%08x\n",
>  			      status);
> 
> -- 
> 2.34.1
> 

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