Re: [PATCH 10/12] PCI: dwc-plat: Simplify the probe method return value handling

[Manivannan Sadhasivam]

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
>