| Message ID | 20240408093053.3948634-1-vidyas@nvidia.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Krzysztof Wilczyński |
| Headers | show |
| Series | [V2] PCI: tegra194: Fix probe path for Endpoint mode | expand |
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 >
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 >>
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
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:
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(+)