[v3] PCI: dwc: Skip waiting for link up if vendor drivers can detect Link up event

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

Posted by Krishna chaitanya chundru 3 weeks, 2 days ago

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 linkup_irq flag if global IRQ is present and In order to set the
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..474b7525442d 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->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 v3 2/3] PCI: qcom: Set linkup_irq if global IRQ handler is present

Posted by Bjorn Andersson 3 weeks, 2 days ago

On Fri, Nov 01, 2024 at 05:04:13PM GMT, 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 linkup_irq flag if global IRQ is present and In order to set the
> 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..474b7525442d 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->linkup_irq = true;

This seems to only ever being used in dw_pcie_host_init(), would it make
sense to use a argument to the function to pass the parameter instead of
stashing it in the persistent data structure?

Regards,
Bjorn

> +
>  	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 v3 2/3] PCI: qcom: Set linkup_irq if global IRQ handler is present

Posted by Krishna Chaitanya Chundru 2 weeks, 6 days ago


On 11/1/2024 9:00 PM, Bjorn Andersson wrote:
> On Fri, Nov 01, 2024 at 05:04:13PM GMT, 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 linkup_irq flag if global IRQ is present and In order to set the
>> 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..474b7525442d 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->linkup_irq = true;
> 
> This seems to only ever being used in dw_pcie_host_init(), would it make
> sense to use a argument to the function to pass the parameter instead of
> stashing it in the persistent data structure?
> 
dw_pcie_host_init() API is being used by multiple vendors under
drivers/pci/controller/dwc/* it may not be ideal to change the argument
here.

- Krishna Chaitanya.
> Regards,
> Bjorn
> 
>> +
>>   	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
>>
>>