2024-04-01 15:02:39

by Vidya Sagar

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

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 <[email protected]>
---
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

2024-04-02 12:43:09

by Jon Hunter

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

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 <[email protected]>

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 <[email protected]>

Jon

--
nvpublic

2024-04-08 09:31:41

by Vidya Sagar

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

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 <[email protected]>
Reviewed-by: Jon Hunter <[email protected]>
---
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

2024-04-12 19:14:23

by Bjorn Helgaas

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

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 <[email protected]>
> Reviewed-by: Jon Hunter <[email protected]>
> ---
> 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
>

2024-04-15 11:23:23

by Vidya Sagar

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

It was Jon Hunter ([email protected]), 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 <[email protected]>
>> Reviewed-by: Jon Hunter <[email protected]>
>> ---
>> 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
>>

2024-04-15 13:53:18

by Manivannan Sadhasivam

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

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 <[email protected]>
> Reviewed-by: Jon Hunter <[email protected]>
> ---
> 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

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