Messages in this thread | | | Date | Thu, 7 Jul 2022 22:01:30 +0530 | Subject | Re: [PATCH V1] PCI: designware-ep: Fix DBI access before core init | From | Vidya Sagar <> |
| |
On 7/7/2022 9:30 PM, Bjorn Helgaas wrote: > External email: Use caution opening links or attachments > > > [+cc Serge] > > On Thu, Jul 07, 2022 at 02:39:08PM +0530, Vidya Sagar wrote: >> Hi, >> Anyone has review comments for this change? >> Without this change, Tegra194's endpoint mode is effectively broken. > > Did Tegra194 endpoint mode ever work, or has this always been broken. > Wondering if there's a Fixes: or stable tag we should add. Also cc'd > Serge since he's been doing similar work on dwc.
Yes. The endpoint mode of Tegra194 did work particularly after the commit ac37dde72177 ("PCI: dwc: Add API to notify core initialization completion") There are many changes that got merged after this and hence couldn't really point to one change to quote in Fixes: tag.
Thanks & Regards, Vidya Sagar
> >> On 6/22/2022 9:31 AM, Vidya Sagar wrote: >>> Platforms that cannot support their core initialization without the >>> reference clock from the host, implement the feature 'core_init_notifier' >>> to indicate the DesignWare sub-system about when their core is getting >>> initialized. Any accesses to the core (Ex:- DBI) would result in system >>> hang in such systems (Ex:- tegra194). This patch moves any access to the >>> core to dw_pcie_ep_init_complete() API which is effectively called only >>> after the core initialization. >>> >>> Signed-off-by: Vidya Sagar <vidyas@nvidia.com> >>> --- >>> .../pci/controller/dwc/pcie-designware-ep.c | 88 +++++++++++-------- >>> 1 file changed, 49 insertions(+), 39 deletions(-) >>> >>> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c >>> index 0eda8236c125..9feec720175f 100644 >>> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c >>> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c >>> @@ -639,9 +639,14 @@ 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; >>> unsigned int nbars; >>> u8 hdr_type; >>> + u8 func_no; >>> + void *addr; >>> u32 reg; >>> int i; >>> @@ -654,6 +659,42 @@ int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep) >>> return -EIO; >>> } >>> + dw_pcie_iatu_detect(pci); >>> + >>> + ep->ib_window_map = devm_kcalloc(dev, >>> + BITS_TO_LONGS(pci->num_ib_windows), >>> + sizeof(long), >>> + GFP_KERNEL); >>> + if (!ep->ib_window_map) >>> + return -ENOMEM; >>> + >>> + ep->ob_window_map = devm_kcalloc(dev, >>> + BITS_TO_LONGS(pci->num_ob_windows), >>> + sizeof(long), >>> + 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; >>> + >>> + 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); >>> + } >>> + >>> offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR); >>> dw_pcie_dbi_ro_wr_en(pci); >>> @@ -677,8 +718,6 @@ 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); >>> @@ -686,7 +725,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); >>> @@ -708,8 +746,6 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep) >>> } >>> } >>> - dw_pcie_iatu_detect(pci); >>> - >>> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "addr_space"); >>> if (!res) >>> return -EINVAL; >>> @@ -717,26 +753,6 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep) >>> ep->phys_base = res->start; >>> ep->addr_size = resource_size(res); >>> - ep->ib_window_map = devm_kcalloc(dev, >>> - BITS_TO_LONGS(pci->num_ib_windows), >>> - sizeof(long), >>> - GFP_KERNEL); >>> - if (!ep->ib_window_map) >>> - return -ENOMEM; >>> - >>> - ep->ob_window_map = devm_kcalloc(dev, >>> - BITS_TO_LONGS(pci->num_ob_windows), >>> - sizeof(long), >>> - 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; >>> - >>> if (pci->link_gen < 1) >>> pci->link_gen = of_pci_get_max_link_speed(np); >>> @@ -753,20 +769,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->ep_init) >>> ep->ops->ep_init(ep); >>> @@ -790,6 +792,14 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep) >>> 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 hard hang. >>> + */ >>> + >>> return dw_pcie_ep_init_complete(ep); >>> } >>> EXPORT_SYMBOL_GPL(dw_pcie_ep_init); >>>
| |