On Thu, Mar 24, 2022 at 04:25:21AM +0300, Serge Semin wrote:
> The whole switch-case-logic implemented in the DWC PCIe RC/EP probe
> procedure doesn't seem well thought through. First of all the ret variable
> is unused in the EP-case and is only partly involved in the RC-case of the
> switch-case statement, which unnecessary complicates the code. Secondly
> the probe method will return zero if an unknown mode is detected. That is
> improbable situation since the OF-device data is initialized only with
> valid modes, but such code is still wrong at least from maintainability
> point of view. So let's convert the switch-case part of the probe function
> to being more coherent. We suggest to use the local ret variable to
> preserve the status of the case-clauses and return its value from the
> probe procedure after the work is done.
>
> Signed-off-by: Serge Semin <Sergey.Semin@xxxxxxxxxxxxxxxxxxxx>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
Thanks,
Mani
> ---
> drivers/pci/controller/dwc/pcie-designware-plat.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-plat.c b/drivers/pci/controller/dwc/pcie-designware-plat.c
> index 0c5de87d3cc6..fea785096261 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-plat.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-plat.c
> @@ -153,20 +153,21 @@ static int dw_plat_pcie_probe(struct platform_device *pdev)
> return -ENODEV;
>
> ret = dw_plat_add_pcie_port(dw_plat_pcie, pdev);
> - if (ret < 0)
> - return ret;
> break;
> case DW_PCIE_EP_TYPE:
> if (!IS_ENABLED(CONFIG_PCIE_DW_PLAT_EP))
> return -ENODEV;
>
> pci->ep.ops = &pcie_ep_ops;
> - return dw_pcie_ep_init(&pci->ep);
> + ret = dw_pcie_ep_init(&pci->ep);
> + break;
> default:
> dev_err(dev, "INVALID device type %d\n", dw_plat_pcie->mode);
> + ret = -EINVAL;
> + break;
> }
>
> - return 0;
> + return ret;
> }
>
> static const struct dw_plat_pcie_of_data dw_plat_pcie_rc_of_data = {
> --
> 2.35.1
>
