* [PATCH v2 0/2] PCI: endpoint: Fix core_init_notifier feature
@ 2021-09-01 5:15 Kunihiko Hayashi
2021-09-01 5:16 ` [PATCH v2 1/2] PCI: endpoint: pci-epf-test: register notifier if only core_init_notifier is enabled Kunihiko Hayashi
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Kunihiko Hayashi @ 2021-09-01 5:15 UTC (permalink / raw)
To: Jingoo Han, Gustavo Pimentel, Lorenzo Pieralisi, Rob Herring,
Krzysztof Wilczyński, Bjorn Helgaas, Kishon Vijay Abraham I
Cc: Xiaowei Bao, Om Prakash Singh, Vidya Sagar, linux-pci,
linux-kernel, Kunihiko Hayashi
This series has two fixes for core_init_notifier feature.
Fix the bug that the driver can't register its notifier function
if core_init_notifier == true and linkup_notifier == false.
If enabling the controller is delayed due to core_init_notifier,
accesses to the controller register should be avoided rather than
enabling the controller.
Changes since v1:
- Add Acked-by and Reviewed-by lines
Kunihiko Hayashi (2):
PCI: endpoint: pci-epf-test: register notifier if only
core_init_notifier is enabled
PCI: designware-ep: Fix the access to DBI/iATU registers before
enabling controller
drivers/pci/controller/dwc/pcie-designware-ep.c | 81 +++++++++++++------------
drivers/pci/endpoint/functions/pci-epf-test.c | 2 +-
2 files changed, 42 insertions(+), 41 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH v2 1/2] PCI: endpoint: pci-epf-test: register notifier if only core_init_notifier is enabled 2021-09-01 5:15 [PATCH v2 0/2] PCI: endpoint: Fix core_init_notifier feature Kunihiko Hayashi @ 2021-09-01 5:16 ` Kunihiko Hayashi 2021-12-03 4:35 ` Kishon Vijay Abraham I 2021-09-01 5:16 ` [PATCH v2 2/2] PCI: designware-ep: Fix the access to DBI/iATU registers before enabling controller Kunihiko Hayashi 2021-09-16 11:30 ` [PATCH v2 0/2] PCI: endpoint: Fix core_init_notifier feature Kunihiko Hayashi 2 siblings, 1 reply; 12+ messages in thread From: Kunihiko Hayashi @ 2021-09-01 5:16 UTC (permalink / raw) To: Jingoo Han, Gustavo Pimentel, Lorenzo Pieralisi, Rob Herring, Krzysztof Wilczyński, Bjorn Helgaas, Kishon Vijay Abraham I Cc: Xiaowei Bao, Om Prakash Singh, Vidya Sagar, linux-pci, linux-kernel, Kunihiko Hayashi Need to register pci_epf_test_notifier function even if only core_init_notifier is enabled. Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com> Acked-by: Om Prakash Singh <omp@nvidia.com> --- drivers/pci/endpoint/functions/pci-epf-test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c index 90d84d3..80456ad 100644 --- a/drivers/pci/endpoint/functions/pci-epf-test.c +++ b/drivers/pci/endpoint/functions/pci-epf-test.c @@ -874,7 +874,7 @@ static int pci_epf_test_bind(struct pci_epf *epf) if (ret) epf_test->dma_supported = false; - if (linkup_notifier) { + if (linkup_notifier || core_init_notifier) { epf->nb.notifier_call = pci_epf_test_notifier; pci_epc_register_notifier(epc, &epf->nb); } else { -- 2.7.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] PCI: endpoint: pci-epf-test: register notifier if only core_init_notifier is enabled 2021-09-01 5:16 ` [PATCH v2 1/2] PCI: endpoint: pci-epf-test: register notifier if only core_init_notifier is enabled Kunihiko Hayashi @ 2021-12-03 4:35 ` Kishon Vijay Abraham I 0 siblings, 0 replies; 12+ messages in thread From: Kishon Vijay Abraham I @ 2021-12-03 4:35 UTC (permalink / raw) To: Kunihiko Hayashi, Jingoo Han, Gustavo Pimentel, Lorenzo Pieralisi, Rob Herring, Krzysztof Wilczyński, Bjorn Helgaas Cc: Xiaowei Bao, Om Prakash Singh, Vidya Sagar, linux-pci, linux-kernel On 01/09/21 10:46 am, Kunihiko Hayashi wrote: > Need to register pci_epf_test_notifier function even if only > core_init_notifier is enabled. > > Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com> > Acked-by: Om Prakash Singh <omp@nvidia.com> Acked-by: Kishon Vijay Abraham I <kishon@ti.com> > --- > drivers/pci/endpoint/functions/pci-epf-test.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c > index 90d84d3..80456ad 100644 > --- a/drivers/pci/endpoint/functions/pci-epf-test.c > +++ b/drivers/pci/endpoint/functions/pci-epf-test.c > @@ -874,7 +874,7 @@ static int pci_epf_test_bind(struct pci_epf *epf) > if (ret) > epf_test->dma_supported = false; > > - if (linkup_notifier) { > + if (linkup_notifier || core_init_notifier) { > epf->nb.notifier_call = pci_epf_test_notifier; > pci_epc_register_notifier(epc, &epf->nb); > } else { > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 2/2] PCI: designware-ep: Fix the access to DBI/iATU registers before enabling controller 2021-09-01 5:15 [PATCH v2 0/2] PCI: endpoint: Fix core_init_notifier feature Kunihiko Hayashi 2021-09-01 5:16 ` [PATCH v2 1/2] PCI: endpoint: pci-epf-test: register notifier if only core_init_notifier is enabled Kunihiko Hayashi @ 2021-09-01 5:16 ` Kunihiko Hayashi 2021-12-03 5:06 ` Kishon Vijay Abraham I 2022-02-10 11:04 ` Manivannan Sadhasivam 2021-09-16 11:30 ` [PATCH v2 0/2] PCI: endpoint: Fix core_init_notifier feature Kunihiko Hayashi 2 siblings, 2 replies; 12+ messages in thread From: Kunihiko Hayashi @ 2021-09-01 5:16 UTC (permalink / raw) To: Jingoo Han, Gustavo Pimentel, Lorenzo Pieralisi, Rob Herring, Krzysztof Wilczyński, Bjorn Helgaas, Kishon Vijay Abraham I Cc: Xiaowei Bao, Om Prakash Singh, Vidya Sagar, linux-pci, linux-kernel, Kunihiko Hayashi The driver using core_init_notifier, e.g. pcie-tegra194.c, runs according to the following sequence: probe() dw_pcie_ep_init() bind() dw_pcie_ep_start() enable_irq() (interrupt occurred) handler() [enable controller] dw_pcie_ep_init_complete() dw_pcie_ep_init_notify() After receiving an interrupt from RC, the handler enables the controller and the controller registers can be accessed. So accessing the registers should do in dw_pcie_ep_init_complete(). Currently dw_pcie_ep_init() has functions dw_iatu_detect() and dw_pcie_ep_find_capability() that include accesses to DWC registers. As a result, accessing the registers before enabling the controller, the access will fail. The function dw_pcie_ep_init() shouldn't have any access to DWC registers if the controller is enabled after calling bind(). This moves access codes to DBI/iATU registers and depending variables from dw_pcie_ep_init() to dw_pcie_ep_init_complete(). Cc: Xiaowei Bao <xiaowei.bao@nxp.com> Cc: Vidya Sagar <vidyas@nvidia.com> Fixes: 6bfc9c3a2c70 ("PCI: designware-ep: Move the function of getting MSI capability forward") Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com> Acked-by: Om Prakash Singh <omp@nvidia.com> Reviewed-by: Vidya Sagar <vidyas@nvidia.com> --- drivers/pci/controller/dwc/pcie-designware-ep.c | 81 +++++++++++++------------ 1 file changed, 41 insertions(+), 40 deletions(-) diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c index 998b698..00ce83c 100644 --- a/drivers/pci/controller/dwc/pcie-designware-ep.c +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c @@ -636,16 +636,56 @@ 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; unsigned int offset; unsigned int nbars; u8 hdr_type; + u8 func_no; + void *addr; u32 reg; int i; + 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 < ep->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); + } + hdr_type = dw_pcie_readb_dbi(pci, PCI_HEADER_TYPE) & PCI_HEADER_TYPE_MASK; if (hdr_type != PCI_HEADER_TYPE_NORMAL) { - dev_err(pci->dev, + dev_err(dev, "PCIe controller is not set to EP mode (hdr_type:0x%x)!\n", hdr_type); return -EIO; @@ -674,8 +714,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); @@ -683,7 +721,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); @@ -705,8 +742,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; @@ -714,26 +749,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); @@ -750,20 +765,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); -- 2.7.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] PCI: designware-ep: Fix the access to DBI/iATU registers before enabling controller 2021-09-01 5:16 ` [PATCH v2 2/2] PCI: designware-ep: Fix the access to DBI/iATU registers before enabling controller Kunihiko Hayashi @ 2021-12-03 5:06 ` Kishon Vijay Abraham I 2021-12-06 11:23 ` Lorenzo Pieralisi 2022-02-10 11:02 ` Manivannan Sadhasivam 2022-02-10 11:04 ` Manivannan Sadhasivam 1 sibling, 2 replies; 12+ messages in thread From: Kishon Vijay Abraham I @ 2021-12-03 5:06 UTC (permalink / raw) To: Kunihiko Hayashi, Jingoo Han, Gustavo Pimentel, Lorenzo Pieralisi, Rob Herring, Krzysztof Wilczyński, Bjorn Helgaas Cc: Xiaowei Bao, Om Prakash Singh, Vidya Sagar, linux-pci, linux-kernel Hi Kunihiko, On 01/09/21 10:46 am, Kunihiko Hayashi wrote: > The driver using core_init_notifier, e.g. pcie-tegra194.c, runs according > to the following sequence: > > probe() > dw_pcie_ep_init() > > bind() > dw_pcie_ep_start() > enable_irq() > > (interrupt occurred) > handler() > [enable controller] > dw_pcie_ep_init_complete() > dw_pcie_ep_init_notify() > > After receiving an interrupt from RC, the handler enables the controller > and the controller registers can be accessed. > So accessing the registers should do in dw_pcie_ep_init_complete(). > > Currently dw_pcie_ep_init() has functions dw_iatu_detect() and > dw_pcie_ep_find_capability() that include accesses to DWC registers. > As a result, accessing the registers before enabling the controller, > the access will fail. > > The function dw_pcie_ep_init() shouldn't have any access to DWC registers > if the controller is enabled after calling bind(). This moves access codes > to DBI/iATU registers and depending variables from dw_pcie_ep_init() to > dw_pcie_ep_init_complete(). Ideally pci_epc_create() should be the last step by the controller driver before handing the control to the core EPC framework. Since after this step the EPC framework can start invoking the epc_ops. Here more stuff is being added to dw_pcie_ep_init_complete() which is required for epc_ops and this could result in aborts for platforms which does not add core_init_notifier. Thanks, Kishon > > Cc: Xiaowei Bao <xiaowei.bao@nxp.com> > Cc: Vidya Sagar <vidyas@nvidia.com> > Fixes: 6bfc9c3a2c70 ("PCI: designware-ep: Move the function of getting MSI capability forward") > Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com> > Acked-by: Om Prakash Singh <omp@nvidia.com> > Reviewed-by: Vidya Sagar <vidyas@nvidia.com> > --- > drivers/pci/controller/dwc/pcie-designware-ep.c | 81 +++++++++++++------------ > 1 file changed, 41 insertions(+), 40 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c > index 998b698..00ce83c 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c > @@ -636,16 +636,56 @@ 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; > unsigned int offset; > unsigned int nbars; > u8 hdr_type; > + u8 func_no; > + void *addr; > u32 reg; > int i; > > + 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 < ep->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); > + } > + > hdr_type = dw_pcie_readb_dbi(pci, PCI_HEADER_TYPE) & > PCI_HEADER_TYPE_MASK; > if (hdr_type != PCI_HEADER_TYPE_NORMAL) { > - dev_err(pci->dev, > + dev_err(dev, > "PCIe controller is not set to EP mode (hdr_type:0x%x)!\n", > hdr_type); > return -EIO; > @@ -674,8 +714,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); > @@ -683,7 +721,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); > > @@ -705,8 +742,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; > @@ -714,26 +749,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); > > @@ -750,20 +765,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); > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] PCI: designware-ep: Fix the access to DBI/iATU registers before enabling controller 2021-12-03 5:06 ` Kishon Vijay Abraham I @ 2021-12-06 11:23 ` Lorenzo Pieralisi 2022-01-05 10:43 ` Kunihiko Hayashi 2022-02-10 11:02 ` Manivannan Sadhasivam 1 sibling, 1 reply; 12+ messages in thread From: Lorenzo Pieralisi @ 2021-12-06 11:23 UTC (permalink / raw) To: Kishon Vijay Abraham I Cc: Kunihiko Hayashi, Jingoo Han, Gustavo Pimentel, Rob Herring, Krzysztof Wilczyński, Bjorn Helgaas, Xiaowei Bao, Om Prakash Singh, Vidya Sagar, linux-pci, linux-kernel On Fri, Dec 03, 2021 at 10:36:00AM +0530, Kishon Vijay Abraham I wrote: > Hi Kunihiko, > > On 01/09/21 10:46 am, Kunihiko Hayashi wrote: > > The driver using core_init_notifier, e.g. pcie-tegra194.c, runs according > > to the following sequence: > > > > probe() > > dw_pcie_ep_init() > > > > bind() > > dw_pcie_ep_start() > > enable_irq() > > > > (interrupt occurred) > > handler() > > [enable controller] > > dw_pcie_ep_init_complete() > > dw_pcie_ep_init_notify() > > > > After receiving an interrupt from RC, the handler enables the controller > > and the controller registers can be accessed. > > So accessing the registers should do in dw_pcie_ep_init_complete(). > > > > Currently dw_pcie_ep_init() has functions dw_iatu_detect() and > > dw_pcie_ep_find_capability() that include accesses to DWC registers. > > As a result, accessing the registers before enabling the controller, > > the access will fail. > > > > The function dw_pcie_ep_init() shouldn't have any access to DWC registers > > if the controller is enabled after calling bind(). This moves access codes > > to DBI/iATU registers and depending variables from dw_pcie_ep_init() to > > dw_pcie_ep_init_complete(). > > Ideally pci_epc_create() should be the last step by the controller > driver before handing the control to the core EPC framework. Since > after this step the EPC framework can start invoking the epc_ops. > > Here more stuff is being added to dw_pcie_ep_init_complete() which is > required for epc_ops and this could result in aborts for platforms > which does not add core_init_notifier. This patch needs rework, I will mark the series as "Changes requested". Lorenzo > > Thanks, > Kishon > > > > > Cc: Xiaowei Bao <xiaowei.bao@nxp.com> > > Cc: Vidya Sagar <vidyas@nvidia.com> > > Fixes: 6bfc9c3a2c70 ("PCI: designware-ep: Move the function of getting MSI capability forward") > > Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com> > > Acked-by: Om Prakash Singh <omp@nvidia.com> > > Reviewed-by: Vidya Sagar <vidyas@nvidia.com> > > --- > > drivers/pci/controller/dwc/pcie-designware-ep.c | 81 +++++++++++++------------ > > 1 file changed, 41 insertions(+), 40 deletions(-) > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c > > index 998b698..00ce83c 100644 > > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c > > @@ -636,16 +636,56 @@ 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; > > unsigned int offset; > > unsigned int nbars; > > u8 hdr_type; > > + u8 func_no; > > + void *addr; > > u32 reg; > > int i; > > > > + 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 < ep->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); > > + } > > + > > hdr_type = dw_pcie_readb_dbi(pci, PCI_HEADER_TYPE) & > > PCI_HEADER_TYPE_MASK; > > if (hdr_type != PCI_HEADER_TYPE_NORMAL) { > > - dev_err(pci->dev, > > + dev_err(dev, > > "PCIe controller is not set to EP mode (hdr_type:0x%x)!\n", > > hdr_type); > > return -EIO; > > @@ -674,8 +714,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); > > @@ -683,7 +721,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); > > > > @@ -705,8 +742,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; > > @@ -714,26 +749,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); > > > > @@ -750,20 +765,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); > > > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] PCI: designware-ep: Fix the access to DBI/iATU registers before enabling controller 2021-12-06 11:23 ` Lorenzo Pieralisi @ 2022-01-05 10:43 ` Kunihiko Hayashi 2022-01-05 15:46 ` Manivannan Sadhasivam 0 siblings, 1 reply; 12+ messages in thread From: Kunihiko Hayashi @ 2022-01-05 10:43 UTC (permalink / raw) To: Lorenzo Pieralisi, Kishon Vijay Abraham I Cc: Jingoo Han, Gustavo Pimentel, Rob Herring, Krzysztof Wilczyński, Bjorn Helgaas, Xiaowei Bao, Om Prakash Singh, Vidya Sagar, linux-pci, linux-kernel Hi Kishon, Lorenzo, Thank you and sorry for late reply. On 2021/12/06 20:23, Lorenzo Pieralisi wrote: > On Fri, Dec 03, 2021 at 10:36:00AM +0530, Kishon Vijay Abraham I wrote: >> Hi Kunihiko, >> >> On 01/09/21 10:46 am, Kunihiko Hayashi wrote: >>> The driver using core_init_notifier, e.g. pcie-tegra194.c, runs > according >>> to the following sequence: >>> >>> probe() >>> dw_pcie_ep_init() >>> >>> bind() >>> dw_pcie_ep_start() >>> enable_irq() >>> >>> (interrupt occurred) >>> handler() >>> [enable controller] >>> dw_pcie_ep_init_complete() >>> dw_pcie_ep_init_notify() >>> >>> After receiving an interrupt from RC, the handler enables the > controller >>> and the controller registers can be accessed. >>> So accessing the registers should do in dw_pcie_ep_init_complete(). >>> >>> Currently dw_pcie_ep_init() has functions dw_iatu_detect() and >>> dw_pcie_ep_find_capability() that include accesses to DWC registers. >>> As a result, accessing the registers before enabling the controller, >>> the access will fail. >>> >>> The function dw_pcie_ep_init() shouldn't have any access to DWC > registers >>> if the controller is enabled after calling bind(). This moves access > codes >>> to DBI/iATU registers and depending variables from dw_pcie_ep_init() > to >>> dw_pcie_ep_init_complete(). >> >> Ideally pci_epc_create() should be the last step by the controller >> driver before handing the control to the core EPC framework. Since >> after this step the EPC framework can start invoking the epc_ops. >> >> Here more stuff is being added to dw_pcie_ep_init_complete() which is >> required for epc_ops and this could result in aborts for platforms >> which does not add core_init_notifier. > > This patch needs rework, I will mark the series as "Changes requested". I understand that relocation of dwc register accesses isn't appropriate, but I couldn't think of any other rework to dwc, and I confirmed pcie-qcom-ep driver using core_init_notifier. In pcie-qcom-ep driver, probe() enables clock and deasserts reset first, and when PERST# interrupt arrives, the handler enables clock and deasserts reset again. So, dw_pcie_ep_init() can access DBI registers. In pcie-tegra194 driver, I think the issue will be solved if probe() also handles clock and reset control. However, the driver has other register access between core_clk, core_apb_rst, and core_rst controls. I think that it's appropriate to leave this fix to the developer at this point. As this patch series, I'll resend 1/2 patch only and expect pcie-tegra194 driver to be fixed. Thank you, --- Best Regards Kunihiko Hayashi ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] PCI: designware-ep: Fix the access to DBI/iATU registers before enabling controller 2022-01-05 10:43 ` Kunihiko Hayashi @ 2022-01-05 15:46 ` Manivannan Sadhasivam 0 siblings, 0 replies; 12+ messages in thread From: Manivannan Sadhasivam @ 2022-01-05 15:46 UTC (permalink / raw) To: Kunihiko Hayashi Cc: Lorenzo Pieralisi, Kishon Vijay Abraham I, Jingoo Han, Gustavo Pimentel, Rob Herring, Krzysztof Wilczyński, Bjorn Helgaas, Xiaowei Bao, Om Prakash Singh, Vidya Sagar, linux-pci, linux-kernel On Wed, Jan 05, 2022 at 07:43:04PM +0900, Kunihiko Hayashi wrote: > Hi Kishon, Lorenzo, > > Thank you and sorry for late reply. > > On 2021/12/06 20:23, Lorenzo Pieralisi wrote: > > On Fri, Dec 03, 2021 at 10:36:00AM +0530, Kishon Vijay Abraham I wrote: > > > Hi Kunihiko, > > > > > > On 01/09/21 10:46 am, Kunihiko Hayashi wrote: > > > > The driver using core_init_notifier, e.g. pcie-tegra194.c, runs > > according > > > > to the following sequence: > > > > > > > > probe() > > > > dw_pcie_ep_init() > > > > > > > > bind() > > > > dw_pcie_ep_start() > > > > enable_irq() > > > > > > > > (interrupt occurred) > > > > handler() > > > > [enable controller] > > > > dw_pcie_ep_init_complete() > > > > dw_pcie_ep_init_notify() > > > > > > > > After receiving an interrupt from RC, the handler enables the > > controller > > > > and the controller registers can be accessed. > > > > So accessing the registers should do in dw_pcie_ep_init_complete(). > > > > > > > > Currently dw_pcie_ep_init() has functions dw_iatu_detect() and > > > > dw_pcie_ep_find_capability() that include accesses to DWC registers. > > > > As a result, accessing the registers before enabling the controller, > > > > the access will fail. > > > > > > > > The function dw_pcie_ep_init() shouldn't have any access to DWC > > registers > > > > if the controller is enabled after calling bind(). This moves access > > codes > > > > to DBI/iATU registers and depending variables from dw_pcie_ep_init() > > to > > > > dw_pcie_ep_init_complete(). > > > > > > Ideally pci_epc_create() should be the last step by the controller > > > driver before handing the control to the core EPC framework. Since > > > after this step the EPC framework can start invoking the epc_ops. > > > > > > Here more stuff is being added to dw_pcie_ep_init_complete() which is > > > required for epc_ops and this could result in aborts for platforms > > > which does not add core_init_notifier. > > > > This patch needs rework, I will mark the series as "Changes requested". > > I understand that relocation of dwc register accesses isn't appropriate, > but I couldn't think of any other rework to dwc, and I confirmed > pcie-qcom-ep driver using core_init_notifier. > > In pcie-qcom-ep driver, probe() enables clock and deasserts reset first, > and when PERST# interrupt arrives, the handler enables clock and deasserts > reset again. So, dw_pcie_ep_init() can access DBI registers. > Yes, only since dw_pcie_ep_init() carries out the DBI accesses, we have enabled clocks and PHY. Moving the DBI accesses to init_complete() removes the need of enabling the resources redundantly. Thanks, Mani > In pcie-tegra194 driver, I think the issue will be solved if probe() also > handles clock and reset control. However, the driver has other register > access between core_clk, core_apb_rst, and core_rst controls. > I think that it's appropriate to leave this fix to the developer at this > point. > > As this patch series, I'll resend 1/2 patch only and expect pcie-tegra194 > driver to be fixed. > > Thank you, > > --- > Best Regards > Kunihiko Hayashi ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] PCI: designware-ep: Fix the access to DBI/iATU registers before enabling controller 2021-12-03 5:06 ` Kishon Vijay Abraham I 2021-12-06 11:23 ` Lorenzo Pieralisi @ 2022-02-10 11:02 ` Manivannan Sadhasivam 1 sibling, 0 replies; 12+ messages in thread From: Manivannan Sadhasivam @ 2022-02-10 11:02 UTC (permalink / raw) To: Kishon Vijay Abraham I Cc: Kunihiko Hayashi, Jingoo Han, Gustavo Pimentel, Lorenzo Pieralisi, Rob Herring, Krzysztof Wilczyński, Bjorn Helgaas, Xiaowei Bao, Om Prakash Singh, Vidya Sagar, linux-pci, linux-kernel Hi Kishon, On Fri, Dec 03, 2021 at 10:36:00AM +0530, Kishon Vijay Abraham I wrote: > Hi Kunihiko, > > On 01/09/21 10:46 am, Kunihiko Hayashi wrote: > > The driver using core_init_notifier, e.g. pcie-tegra194.c, runs according > > to the following sequence: > > > > probe() > > dw_pcie_ep_init() > > > > bind() > > dw_pcie_ep_start() > > enable_irq() > > > > (interrupt occurred) > > handler() > > [enable controller] > > dw_pcie_ep_init_complete() > > dw_pcie_ep_init_notify() > > > > After receiving an interrupt from RC, the handler enables the controller > > and the controller registers can be accessed. > > So accessing the registers should do in dw_pcie_ep_init_complete(). > > > > Currently dw_pcie_ep_init() has functions dw_iatu_detect() and > > dw_pcie_ep_find_capability() that include accesses to DWC registers. > > As a result, accessing the registers before enabling the controller, > > the access will fail. > > > > The function dw_pcie_ep_init() shouldn't have any access to DWC registers > > if the controller is enabled after calling bind(). This moves access codes > > to DBI/iATU registers and depending variables from dw_pcie_ep_init() to > > dw_pcie_ep_init_complete(). > > Ideally pci_epc_create() should be the last step by the controller driver before > handing the control to the core EPC framework. Since after this step the EPC > framework can start invoking the epc_ops. > > Here more stuff is being added to dw_pcie_ep_init_complete() which is required > for epc_ops and this could result in aborts for platforms which does not add > core_init_notifier. > Is there a better way to handle this situation? IMO the existing situation is messy. Assume that if EP is gonna powered separately by an independent power rail not tied to host PCIe domain (that's the typical endpoint device usecase), the EP driver will fail to probe due to PHY link not getting stabilized. So ultimately the board design needs to take care of an extra logic to power the EP device after powering the host properly and that's not ideal. Thanks, Mani > Thanks, > Kishon > > > > > Cc: Xiaowei Bao <xiaowei.bao@nxp.com> > > Cc: Vidya Sagar <vidyas@nvidia.com> > > Fixes: 6bfc9c3a2c70 ("PCI: designware-ep: Move the function of getting MSI capability forward") > > Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com> > > Acked-by: Om Prakash Singh <omp@nvidia.com> > > Reviewed-by: Vidya Sagar <vidyas@nvidia.com> > > --- > > drivers/pci/controller/dwc/pcie-designware-ep.c | 81 +++++++++++++------------ > > 1 file changed, 41 insertions(+), 40 deletions(-) > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c > > index 998b698..00ce83c 100644 > > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c > > @@ -636,16 +636,56 @@ 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; > > unsigned int offset; > > unsigned int nbars; > > u8 hdr_type; > > + u8 func_no; > > + void *addr; > > u32 reg; > > int i; > > > > + 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 < ep->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); > > + } > > + > > hdr_type = dw_pcie_readb_dbi(pci, PCI_HEADER_TYPE) & > > PCI_HEADER_TYPE_MASK; > > if (hdr_type != PCI_HEADER_TYPE_NORMAL) { > > - dev_err(pci->dev, > > + dev_err(dev, > > "PCIe controller is not set to EP mode (hdr_type:0x%x)!\n", > > hdr_type); > > return -EIO; > > @@ -674,8 +714,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); > > @@ -683,7 +721,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); > > > > @@ -705,8 +742,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; > > @@ -714,26 +749,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); > > > > @@ -750,20 +765,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); > > > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] PCI: designware-ep: Fix the access to DBI/iATU registers before enabling controller 2021-09-01 5:16 ` [PATCH v2 2/2] PCI: designware-ep: Fix the access to DBI/iATU registers before enabling controller Kunihiko Hayashi 2021-12-03 5:06 ` Kishon Vijay Abraham I @ 2022-02-10 11:04 ` Manivannan Sadhasivam 1 sibling, 0 replies; 12+ messages in thread From: Manivannan Sadhasivam @ 2022-02-10 11:04 UTC (permalink / raw) To: Kunihiko Hayashi Cc: Jingoo Han, Gustavo Pimentel, Lorenzo Pieralisi, Rob Herring, Krzysztof Wilczyński, Bjorn Helgaas, Kishon Vijay Abraham I, Xiaowei Bao, Om Prakash Singh, Vidya Sagar, linux-pci, linux-kernel On Wed, Sep 01, 2021 at 02:16:01PM +0900, Kunihiko Hayashi wrote: > The driver using core_init_notifier, e.g. pcie-tegra194.c, runs according > to the following sequence: > > probe() > dw_pcie_ep_init() > > bind() > dw_pcie_ep_start() > enable_irq() > > (interrupt occurred) > handler() > [enable controller] > dw_pcie_ep_init_complete() > dw_pcie_ep_init_notify() > > After receiving an interrupt from RC, the handler enables the controller > and the controller registers can be accessed. > So accessing the registers should do in dw_pcie_ep_init_complete(). > > Currently dw_pcie_ep_init() has functions dw_iatu_detect() and > dw_pcie_ep_find_capability() that include accesses to DWC registers. > As a result, accessing the registers before enabling the controller, > the access will fail. > > The function dw_pcie_ep_init() shouldn't have any access to DWC registers > if the controller is enabled after calling bind(). This moves access codes > to DBI/iATU registers and depending variables from dw_pcie_ep_init() to > dw_pcie_ep_init_complete(). > > Cc: Xiaowei Bao <xiaowei.bao@nxp.com> > Cc: Vidya Sagar <vidyas@nvidia.com> > Fixes: 6bfc9c3a2c70 ("PCI: designware-ep: Move the function of getting MSI capability forward") > Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com> > Acked-by: Om Prakash Singh <omp@nvidia.com> > Reviewed-by: Vidya Sagar <vidyas@nvidia.com> > --- > drivers/pci/controller/dwc/pcie-designware-ep.c | 81 +++++++++++++------------ > 1 file changed, 41 insertions(+), 40 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c > index 998b698..00ce83c 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c [...] > 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); > @@ -683,7 +721,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); > > @@ -705,8 +742,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; > @@ -714,26 +749,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); > > @@ -750,20 +765,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); You also need to move ep_init() as it can have DBI access too. Thanks, Mani > > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 0/2] PCI: endpoint: Fix core_init_notifier feature 2021-09-01 5:15 [PATCH v2 0/2] PCI: endpoint: Fix core_init_notifier feature Kunihiko Hayashi 2021-09-01 5:16 ` [PATCH v2 1/2] PCI: endpoint: pci-epf-test: register notifier if only core_init_notifier is enabled Kunihiko Hayashi 2021-09-01 5:16 ` [PATCH v2 2/2] PCI: designware-ep: Fix the access to DBI/iATU registers before enabling controller Kunihiko Hayashi @ 2021-09-16 11:30 ` Kunihiko Hayashi 2021-12-01 15:04 ` Lorenzo Pieralisi 2 siblings, 1 reply; 12+ messages in thread From: Kunihiko Hayashi @ 2021-09-16 11:30 UTC (permalink / raw) To: Jingoo Han, Gustavo Pimentel, Lorenzo Pieralisi, Rob Herring, Krzysztof Wilczyński, Bjorn Helgaas, Kishon Vijay Abraham I Cc: Xiaowei Bao, Om Prakash Singh, Vidya Sagar, linux-pci, linux-kernel Gentle ping, are there any comments about this series? Thank you, On 2021/09/01 14:15, Kunihiko Hayashi wrote: > This series has two fixes for core_init_notifier feature. > > Fix the bug that the driver can't register its notifier function > if core_init_notifier == true and linkup_notifier == false. > > If enabling the controller is delayed due to core_init_notifier, > accesses to the controller register should be avoided rather than > enabling the controller. > > Changes since v1: > - Add Acked-by and Reviewed-by lines > > Kunihiko Hayashi (2): > PCI: endpoint: pci-epf-test: register notifier if only > core_init_notifier is enabled > PCI: designware-ep: Fix the access to DBI/iATU registers before > enabling controller > > drivers/pci/controller/dwc/pcie-designware-ep.c | 81 +++++++++++++------------ > drivers/pci/endpoint/functions/pci-epf-test.c | 2 +- > 2 files changed, 42 insertions(+), 41 deletions(-) > -- --- Best Regards Kunihiko Hayashi ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 0/2] PCI: endpoint: Fix core_init_notifier feature 2021-09-16 11:30 ` [PATCH v2 0/2] PCI: endpoint: Fix core_init_notifier feature Kunihiko Hayashi @ 2021-12-01 15:04 ` Lorenzo Pieralisi 0 siblings, 0 replies; 12+ messages in thread From: Lorenzo Pieralisi @ 2021-12-01 15:04 UTC (permalink / raw) To: Kunihiko Hayashi, kishon Cc: Jingoo Han, Gustavo Pimentel, Rob Herring, Krzysztof Wilczyński, Bjorn Helgaas, Kishon Vijay Abraham I, Xiaowei Bao, Om Prakash Singh, Vidya Sagar, linux-pci, linux-kernel On Thu, Sep 16, 2021 at 08:30:35PM +0900, Kunihiko Hayashi wrote: > Gentle ping, are there any comments about this series? Kishon, can you have a look please ? Thanks, Lorenzo > Thank you, > > On 2021/09/01 14:15, Kunihiko Hayashi wrote: > > This series has two fixes for core_init_notifier feature. > > > > Fix the bug that the driver can't register its notifier function > > if core_init_notifier == true and linkup_notifier == false. > > > > If enabling the controller is delayed due to core_init_notifier, > > accesses to the controller register should be avoided rather than > > enabling the controller. > > > > Changes since v1: > > - Add Acked-by and Reviewed-by lines > > > > Kunihiko Hayashi (2): > > PCI: endpoint: pci-epf-test: register notifier if only > > core_init_notifier is enabled > > PCI: designware-ep: Fix the access to DBI/iATU registers before > > enabling controller > > > > drivers/pci/controller/dwc/pcie-designware-ep.c | 81 +++++++++++++------------ > > drivers/pci/endpoint/functions/pci-epf-test.c | 2 +- > > 2 files changed, 42 insertions(+), 41 deletions(-) > > > > -- > --- > Best Regards > Kunihiko Hayashi ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2022-02-10 11:04 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-09-01 5:15 [PATCH v2 0/2] PCI: endpoint: Fix core_init_notifier feature Kunihiko Hayashi 2021-09-01 5:16 ` [PATCH v2 1/2] PCI: endpoint: pci-epf-test: register notifier if only core_init_notifier is enabled Kunihiko Hayashi 2021-12-03 4:35 ` Kishon Vijay Abraham I 2021-09-01 5:16 ` [PATCH v2 2/2] PCI: designware-ep: Fix the access to DBI/iATU registers before enabling controller Kunihiko Hayashi 2021-12-03 5:06 ` Kishon Vijay Abraham I 2021-12-06 11:23 ` Lorenzo Pieralisi 2022-01-05 10:43 ` Kunihiko Hayashi 2022-01-05 15:46 ` Manivannan Sadhasivam 2022-02-10 11:02 ` Manivannan Sadhasivam 2022-02-10 11:04 ` Manivannan Sadhasivam 2021-09-16 11:30 ` [PATCH v2 0/2] PCI: endpoint: Fix core_init_notifier feature Kunihiko Hayashi 2021-12-01 15:04 ` Lorenzo Pieralisi
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.