Re: [PATCH 01/12] PCI: dwc: Stop link in the host init error and de-initialization

[Manivannan Sadhasivam]

On Thu, Mar 24, 2022 at 04:25:12AM +0300, Serge Semin wrote:
> It's logically correct to undo everything what was done in case of an
> error is discovered or in the corresponding cleanup counterpart. Otherwise
> the host controller will be left in an undetermined state. Seeing the link
> is set up in the Host-initialization method it will be right to
> de-activate it there in the cleanup-on-error block and stop the link in
> the antagonistic routine - dw_pcie_host_deinit(). The link de-activation
> is a platform-specific thing and is supposed to be implemented in the
> framework of the dw_pcie_ops.stop_link() operation.
> 
> Fixes: 886a9c134755 ("PCI: dwc: Move link handling into common code")
> Signed-off-by: Serge Semin <Sergey.Semin@xxxxxxxxxxxxxxxxxxxx>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>

Thanks,
Mani

> ---
>  .../pci/controller/dwc/pcie-designware-host.c    | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index f4755f3a03be..a03619a30c20 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -414,8 +414,14 @@ int dw_pcie_host_init(struct pcie_port *pp)
>  	bridge->sysdata = pp;
>  
>  	ret = pci_host_probe(bridge);
> -	if (!ret)
> -		return 0;
> +	if (ret)
> +		goto err_stop_link;
> +
> +	return 0;
> +
> +err_stop_link:
> +	if (pci->ops && pci->ops->stop_link)
> +		pci->ops->stop_link(pci);
>  
>  err_free_msi:
>  	if (pp->has_msi_ctrl)
> @@ -426,8 +432,14 @@ EXPORT_SYMBOL_GPL(dw_pcie_host_init);
>  
>  void dw_pcie_host_deinit(struct pcie_port *pp)
>  {
> +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +
>  	pci_stop_root_bus(pp->bridge->bus);
>  	pci_remove_root_bus(pp->bridge->bus);
> +
> +	if (pci->ops && pci->ops->stop_link)
> +		pci->ops->stop_link(pci);
> +
>  	if (pp->has_msi_ctrl)
>  		dw_pcie_free_msi(pp);
>  }
> -- 
> 2.35.1
>