[PATCH V1] PCI: tegra194: Fix probe path for Endpoint mode

[PATCH V1] PCI: tegra194: Fix probe path for Endpoint mode

[Vidya Sagar]

Tegra194 PCIe probe path is taking failure path in success case for
Endpoint mode. Return success from the switch case instead of going
into the failure path.

Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
---
 drivers/pci/controller/dwc/pcie-tegra194.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
index 4bba31502ce1..1a8178dc899a 100644
--- a/drivers/pci/controller/dwc/pcie-tegra194.c
+++ b/drivers/pci/controller/dwc/pcie-tegra194.c
@@ -2273,11 +2273,14 @@ static int tegra_pcie_dw_probe(struct platform_device *pdev)
 		ret = tegra_pcie_config_ep(pcie, pdev);
 		if (ret < 0)
 			goto fail;
+		else
+			return 0;
 		break;
 
 	default:
 		dev_err(dev, "Invalid PCIe device type %d\n",
 			pcie->of_data->mode);
+		ret = -EINVAL;
 	}
 
 fail:
-- 
2.25.1

Re: [PATCH V1] PCI: tegra194: Fix probe path for Endpoint mode

[Jon Hunter]

On 01/04/2024 16:01, Vidya Sagar wrote:
> Tegra194 PCIe probe path is taking failure path in success case for
> Endpoint mode. Return success from the switch case instead of going
> into the failure path.
> 
> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>

Fixes: c57247f940e8 ("PCI: tegra: Add support for PCIe endpoint mode in Tegra194")

> ---
>   drivers/pci/controller/dwc/pcie-tegra194.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
> index 4bba31502ce1..1a8178dc899a 100644
> --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> @@ -2273,11 +2273,14 @@ static int tegra_pcie_dw_probe(struct platform_device *pdev)
>   		ret = tegra_pcie_config_ep(pcie, pdev);
>   		if (ret < 0)
>   			goto fail;
> +		else
> +			return 0;
>   		break;
>   
>   	default:
>   		dev_err(dev, "Invalid PCIe device type %d\n",
>   			pcie->of_data->mode);
> +		ret = -EINVAL;
>   	}
>   
>   fail:


Otherwise ...

Reviewed-by: Jon Hunter <jonathanh@nvidia.com>

Jon

-- 
nvpublic

[PATCH V2] PCI: tegra194: Fix probe path for Endpoint mode

[Vidya Sagar]

Tegra194 PCIe probe path is taking failure path in success case for
Endpoint mode. Return success from the switch case instead of going
into the failure path.

Fixes: c57247f940e8 ("PCI: tegra: Add support for PCIe endpoint mode in Tegra194")
Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
Reviewed-by: Jon Hunter <jonathanh@nvidia.com>
---
v2:
* Added 'Fixes' and 'Reviewed-by' from Jon Hunter

 drivers/pci/controller/dwc/pcie-tegra194.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
index 4bba31502ce1..1a8178dc899a 100644
--- a/drivers/pci/controller/dwc/pcie-tegra194.c
+++ b/drivers/pci/controller/dwc/pcie-tegra194.c
@@ -2273,11 +2273,14 @@ static int tegra_pcie_dw_probe(struct platform_device *pdev)
 		ret = tegra_pcie_config_ep(pcie, pdev);
 		if (ret < 0)
 			goto fail;
+		else
+			return 0;
 		break;
 
 	default:
 		dev_err(dev, "Invalid PCIe device type %d\n",
 			pcie->of_data->mode);
+		ret = -EINVAL;
 	}
 
 fail:
-- 
2.25.1

Re: [PATCH V2] PCI: tegra194: Fix probe path for Endpoint mode

[Bjorn Helgaas]

On Mon, Apr 08, 2024 at 03:00:53PM +0530, Vidya Sagar wrote:
> Tegra194 PCIe probe path is taking failure path in success case for
> Endpoint mode. Return success from the switch case instead of going
> into the failure path.
> 
> Fixes: c57247f940e8 ("PCI: tegra: Add support for PCIe endpoint mode in Tegra194")
> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
> Reviewed-by: Jon Hunter <jonathanh@nvidia.com>
> ---
> v2:
> * Added 'Fixes' and 'Reviewed-by' from Jon Hunter
> 
>  drivers/pci/controller/dwc/pcie-tegra194.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
> index 4bba31502ce1..1a8178dc899a 100644
> --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> @@ -2273,11 +2273,14 @@ static int tegra_pcie_dw_probe(struct platform_device *pdev)
>  		ret = tegra_pcie_config_ep(pcie, pdev);
>  		if (ret < 0)
>  			goto fail;
> +		else
> +			return 0;

Wow, how did you ever notice this?  It looks like this path would
previously have returned "ret" (which was most likely 0 for success)
but with an extra tegra_bpmp_put() that we shouldn't have done.

Eagle eyes!

>  		break;
>  
>  	default:
>  		dev_err(dev, "Invalid PCIe device type %d\n",
>  			pcie->of_data->mode);
> +		ret = -EINVAL;
>  	}
>  
>  fail:
> -- 
> 2.25.1
> 

Re: [PATCH V2] PCI: tegra194: Fix probe path for Endpoint mode

[Vidya Sagar]

It was Jon Hunter (jonathanh@nvidia.com), who identified the issue.
Credits to him.
I'm merely upstreaming the fix.

Thanks,
Vidya Sagar

On 13-04-2024 00:44, Bjorn Helgaas wrote:
> External email: Use caution opening links or attachments
>
>
> On Mon, Apr 08, 2024 at 03:00:53PM +0530, Vidya Sagar wrote:
>> Tegra194 PCIe probe path is taking failure path in success case for
>> Endpoint mode. Return success from the switch case instead of going
>> into the failure path.
>>
>> Fixes: c57247f940e8 ("PCI: tegra: Add support for PCIe endpoint mode in Tegra194")
>> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
>> Reviewed-by: Jon Hunter <jonathanh@nvidia.com>
>> ---
>> v2:
>> * Added 'Fixes' and 'Reviewed-by' from Jon Hunter
>>
>>   drivers/pci/controller/dwc/pcie-tegra194.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
>> index 4bba31502ce1..1a8178dc899a 100644
>> --- a/drivers/pci/controller/dwc/pcie-tegra194.c
>> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
>> @@ -2273,11 +2273,14 @@ static int tegra_pcie_dw_probe(struct platform_device *pdev)
>>                ret = tegra_pcie_config_ep(pcie, pdev);
>>                if (ret < 0)
>>                        goto fail;
>> +             else
>> +                     return 0;
> Wow, how did you ever notice this?  It looks like this path would
> previously have returned "ret" (which was most likely 0 for success)
> but with an extra tegra_bpmp_put() that we shouldn't have done.
>
> Eagle eyes!
>
>>                break;
>>
>>        default:
>>                dev_err(dev, "Invalid PCIe device type %d\n",
>>                        pcie->of_data->mode);
>> +             ret = -EINVAL;
>>        }
>>
>>   fail:
>> --
>> 2.25.1
>>

Re: [PATCH V2] PCI: tegra194: Fix probe path for Endpoint mode

[Manivannan Sadhasivam]

On Mon, Apr 08, 2024 at 03:00:53PM +0530, Vidya Sagar wrote:
> Tegra194 PCIe probe path is taking failure path in success case for
> Endpoint mode. Return success from the switch case instead of going
> into the failure path.
> 
> Fixes: c57247f940e8 ("PCI: tegra: Add support for PCIe endpoint mode in Tegra194")
> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
> Reviewed-by: Jon Hunter <jonathanh@nvidia.com>
> ---
> v2:
> * Added 'Fixes' and 'Reviewed-by' from Jon Hunter
> 
>  drivers/pci/controller/dwc/pcie-tegra194.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
> index 4bba31502ce1..1a8178dc899a 100644
> --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> @@ -2273,11 +2273,14 @@ static int tegra_pcie_dw_probe(struct platform_device *pdev)
>  		ret = tegra_pcie_config_ep(pcie, pdev);
>  		if (ret < 0)
>  			goto fail;
> +		else
> +			return 0;

This pattern is prone to error just like how it was before this patch. You
should just return 0 at the end of the function for success case and direct all
failure cases to 'fail' label:

```
diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
index 1f7b662cb8e1..f410aae0ee7f 100644
--- a/drivers/pci/controller/dwc/pcie-tegra194.c
+++ b/drivers/pci/controller/dwc/pcie-tegra194.c
@@ -2236,9 +2236,8 @@ static int tegra_pcie_dw_probe(struct platform_device *pdev)
        pcie->icc_path = devm_of_icc_get(&pdev->dev, "write");
        ret = PTR_ERR_OR_ZERO(pcie->icc_path);
        if (ret) {
-               tegra_bpmp_put(pcie->bpmp);
                dev_err_probe(&pdev->dev, ret, "failed to get write interconnect\n");
-               return ret;
+               goto fail;
        }
 
        switch (pcie->of_data->mode) {
@@ -2254,8 +2253,6 @@ static int tegra_pcie_dw_probe(struct platform_device *pdev)
                ret = tegra_pcie_config_rp(pcie);
                if (ret && ret != -ENOMEDIUM)
                        goto fail;
-               else
-                       return 0;
                break;
 
        case DW_PCIE_EP_TYPE:
@@ -2278,8 +2275,11 @@ static int tegra_pcie_dw_probe(struct platform_device *pdev)
        default:
                dev_err(dev, "Invalid PCIe device type %d\n",
                        pcie->of_data->mode);
+               goto fail;
        }
 
+       return 0;
+
 fail:
        tegra_bpmp_put(pcie->bpmp);
        return ret;
```

- Mani

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

Re: [PATCH V2] PCI: tegra194: Fix probe path for Endpoint mode

[Krzysztof Wilczyński]

Hello,

> Tegra194 PCIe probe path is taking failure path in success case for
> Endpoint mode. Return success from the switch case instead of going
> into the failure path.

Applied to controller/tegra194, thank you!

[1/1] PCI: tegra194: Fix probe path for Endpoint mode
      https://git.kernel.org/pci/pci/c/19326006a21d

	Krzysztof