On Sat, Feb 24, 2024 at 12:24:10PM +0530, Manivannan Sadhasivam wrote:
> The DWC glue drivers requiring an active reference clock from the PCIe host
> for initializing their PCIe EP core, set a flag called 'core_init_notifier'
> to let DWC driver know that these drivers need a special attention during
> initialization. In these drivers, access to the hw registers (like DBI)
> before receiving the active refclk from host will result in access failure
> and also could cause a whole system hang.
> 
> But the current DWC EP driver doesn't honor the requirements of the drivers
> setting 'core_init_notifier' flag and tries to access the DBI registers
> during dw_pcie_ep_init(). This causes the system hang for glue drivers such
> as Tegra194 and Qcom EP as they depend on refclk from host and have set the
> above mentioned flag.
> 
> To workaround this issue, users of the affected platforms have to maintain
> the dependency with the PCIe host by booting the PCIe EP after host boot.
> But this won't provide a good user experience, since PCIe EP is _one_ of
> the features of those platforms and it doesn't make sense to delay the
> whole platform booting due to PCIe requiring active refclk.
> 
> So to fix this issue, let's move all the DBI access from
> dw_pcie_ep_init() in the DWC EP driver to the dw_pcie_ep_init_complete()
> API. This API will only be called by the drivers setting
> 'core_init_notifier' flag once refclk is received from host. For the rest
> of the drivers that gets the refclk locally, this API will be called
> within dw_pcie_ep_init().
> 
> Fixes: e966f7390da9 ("PCI: dwc: Refactor core initialization code for EP 
> mode")
> Co-developed-by: Vidya Sagar <vid...@nvidia.com>
> Signed-off-by: Vidya Sagar <vid...@nvidia.com>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasi...@linaro.org>

Reviewed-by: Frank Li <frank...@nxp.com>

> ---
>  drivers/pci/controller/dwc/pcie-designware-ep.c | 120 
> ++++++++++++++----------
>  1 file changed, 71 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c 
> b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 1205bfba8310..99d66b0fa59b 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -606,11 +606,16 @@ static unsigned int 
> dw_pcie_ep_find_ext_capability(struct dw_pcie *pci, int cap)
>  int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep)
>  {
>       struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> +     struct dw_pcie_ep_func *ep_func;
> +     struct device *dev = pci->dev;
> +     struct pci_epc *epc = ep->epc;
>       unsigned int offset, ptm_cap_base;
>       unsigned int nbars;
>       u8 hdr_type;
> +     u8 func_no;
> +     int i, ret;
> +     void *addr;
>       u32 reg;
> -     int i;
>  
>       hdr_type = dw_pcie_readb_dbi(pci, PCI_HEADER_TYPE) &
>                  PCI_HEADER_TYPE_MASK;
> @@ -621,6 +626,58 @@ int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep)
>               return -EIO;
>       }
>  
> +     dw_pcie_version_detect(pci);
> +
> +     dw_pcie_iatu_detect(pci);
> +
> +     ret = dw_pcie_edma_detect(pci);
> +     if (ret)
> +             return ret;
> +
> +     if (!ep->ib_window_map) {
> +             ep->ib_window_map = devm_bitmap_zalloc(dev, pci->num_ib_windows,
> +                                                    GFP_KERNEL);
> +             if (!ep->ib_window_map)
> +                     goto err_remove_edma;
> +     }
> +
> +     if (!ep->ob_window_map) {
> +             ep->ob_window_map = devm_bitmap_zalloc(dev, pci->num_ob_windows,
> +                                                    GFP_KERNEL);
> +             if (!ep->ob_window_map)
> +                     goto err_remove_edma;
> +     }
> +
> +     if (!ep->outbound_addr) {
> +             addr = devm_kcalloc(dev, pci->num_ob_windows, 
> sizeof(phys_addr_t),
> +                                 GFP_KERNEL);
> +             if (!addr)
> +                     goto err_remove_edma;
> +             ep->outbound_addr = addr;
> +     }
> +
> +     for (func_no = 0; func_no < epc->max_functions; func_no++) {
> +
> +             ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
> +             if (ep_func)
> +                     continue;
> +
> +             ep_func = devm_kzalloc(dev, sizeof(*ep_func), GFP_KERNEL);
> +             if (!ep_func)
> +                     goto err_remove_edma;
> +
> +             ep_func->func_no = func_no;
> +             ep_func->msi_cap = dw_pcie_ep_find_capability(ep, func_no,
> +                                                           PCI_CAP_ID_MSI);
> +             ep_func->msix_cap = dw_pcie_ep_find_capability(ep, func_no,
> +                                                            PCI_CAP_ID_MSIX);
> +
> +             list_add_tail(&ep_func->list, &ep->func_list);
> +     }
> +
> +     if (ep->ops->init)
> +             ep->ops->init(ep);
> +
>       offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR);
>       ptm_cap_base = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_PTM);
>  
> @@ -655,14 +712,17 @@ int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep)
>       dw_pcie_dbi_ro_wr_dis(pci);
>  
>       return 0;
> +
> +err_remove_edma:
> +     dw_pcie_edma_remove(pci);
> +
> +     return ret;
>  }
>  EXPORT_SYMBOL_GPL(dw_pcie_ep_init_complete);
>  
>  int dw_pcie_ep_init(struct dw_pcie_ep *ep)
>  {
>       int ret;
> -     void *addr;
> -     u8 func_no;
>       struct resource *res;
>       struct pci_epc *epc;
>       struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> @@ -670,7 +730,6 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
>       struct platform_device *pdev = to_platform_device(dev);
>       struct device_node *np = dev->of_node;
>       const struct pci_epc_features *epc_features;
> -     struct dw_pcie_ep_func *ep_func;
>  
>       INIT_LIST_HEAD(&ep->func_list);
>  
> @@ -688,26 +747,6 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
>       if (ep->ops->pre_init)
>               ep->ops->pre_init(ep);
>  
> -     dw_pcie_version_detect(pci);
> -
> -     dw_pcie_iatu_detect(pci);
> -
> -     ep->ib_window_map = devm_bitmap_zalloc(dev, pci->num_ib_windows,
> -                                            GFP_KERNEL);
> -     if (!ep->ib_window_map)
> -             return -ENOMEM;
> -
> -     ep->ob_window_map = devm_bitmap_zalloc(dev, pci->num_ob_windows,
> -                                            GFP_KERNEL);
> -     if (!ep->ob_window_map)
> -             return -ENOMEM;
> -
> -     addr = devm_kcalloc(dev, pci->num_ob_windows, sizeof(phys_addr_t),
> -                         GFP_KERNEL);
> -     if (!addr)
> -             return -ENOMEM;
> -     ep->outbound_addr = addr;
> -
>       epc = devm_pci_epc_create(dev, &epc_ops);
>       if (IS_ERR(epc)) {
>               dev_err(dev, "Failed to create epc device\n");
> @@ -721,23 +760,6 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
>       if (ret < 0)
>               epc->max_functions = 1;
>  
> -     for (func_no = 0; func_no < epc->max_functions; func_no++) {
> -             ep_func = devm_kzalloc(dev, sizeof(*ep_func), GFP_KERNEL);
> -             if (!ep_func)
> -                     return -ENOMEM;
> -
> -             ep_func->func_no = func_no;
> -             ep_func->msi_cap = dw_pcie_ep_find_capability(ep, func_no,
> -                                                           PCI_CAP_ID_MSI);
> -             ep_func->msix_cap = dw_pcie_ep_find_capability(ep, func_no,
> -                                                            PCI_CAP_ID_MSIX);
> -
> -             list_add_tail(&ep_func->list, &ep->func_list);
> -     }
> -
> -     if (ep->ops->init)
> -             ep->ops->init(ep);
> -
>       ret = pci_epc_mem_init(epc, ep->phys_base, ep->addr_size,
>                              ep->page_size);
>       if (ret < 0) {
> @@ -753,25 +775,25 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
>               goto err_exit_epc_mem;
>       }
>  
> -     ret = dw_pcie_edma_detect(pci);
> -     if (ret)
> -             goto err_free_epc_mem;
> -
>       if (ep->ops->get_features) {
>               epc_features = ep->ops->get_features(ep);
>               if (epc_features->core_init_notifier)
>                       return 0;
>       }
>  
> +     /*
> +      * NOTE:- Avoid accessing the hardware (Ex:- DBI space) before this
> +      * step as platforms that implement 'core_init_notifier' feature may
> +      * not have the hardware ready (i.e. core initialized) for access
> +      * (Ex: tegra194). Any hardware access on such platforms result
> +      * in system hang.
> +      */
>       ret = dw_pcie_ep_init_complete(ep);
>       if (ret)
> -             goto err_remove_edma;
> +             goto err_free_epc_mem;
>  
>       return 0;
>  
> -err_remove_edma:
> -     dw_pcie_edma_remove(pci);
> -
>  err_free_epc_mem:
>       pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem,
>                             epc->mem->window.page_size);
> 
> -- 
> 2.25.1
> 

Reply via email to