Re: [PATCH] PCI: designware-ep: Move DBI access to init_complete if notifier is used

From: Vidya Sagar
Date: Fri Jul 08 2022 - 06:19:32 EST

Next message: Jiaxin Yu: "Re: [PATCH] ASoC: mediatek: mt8186: Remove condition with no effect"
Previous message: Stephen Rothwell: "linux-next: Tree for Jul 8"
Next in thread: Kunihiko Hayashi: "Re: [PATCH] PCI: designware-ep: Move DBI access to init_complete if notifier is used"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

I have a minor comment. Otherwise this patch looks fine.
I verified it on Tegra194 and it works as expected.

Reviewed-by: Vidya Sagar <vidyas@xxxxxxxxxx>
Tested-by: Vidya Sagar <vidyas@xxxxxxxxxx>

On 7/7/2022 10:27 PM, Manivannan Sadhasivam wrote:

External email: Use caution opening links or attachments

On Wed, Apr 27, 2022 at 03:48:26PM +0530, Manivannan Sadhasivam wrote:

On Wed, Mar 30, 2022 at 11:35:15AM +0530, Manivannan Sadhasivam wrote:

For controllers supporting the CORE_INIT notifier, the resources are
supposed to be enabled in the init_complete function. Currently,
these controllers are enabling the resources during probe time due to
the DBI access happens in dw_pcie_ep_init().

This creates the dependency with the host PCIe controller since the
resource enablement like PHY depends on host PCIe to be up. For the
standalone endpoint usecase, this would never work. So let's move all DBI
access to init_complete function if CORE_INIT notifier is used. For the
controllers those doesn't support this notifier, this change is a NO-OP.

Cc: Kunihiko Hayashi <hayashi.kunihiko@xxxxxxxxxxxxx>
Cc: Om Prakash Singh <omp@xxxxxxxxxx>
Cc: Vidya Sagar <vidyas@xxxxxxxxxx>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>

A gentle ping on this patch!

Ping again!

Thanks,
Mani

Thanks,
Mani

---
.../pci/controller/dwc/pcie-designware-ep.c | 138 ++++++++++++------
drivers/pci/controller/dwc/pcie-designware.h | 1 +
2 files changed, 94 insertions(+), 45 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index 0eda8236c125..fb2bf4bf5ba0 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -636,6 +636,63 @@ static unsigned int dw_pcie_ep_find_ext_capability(struct dw_pcie *pci, int cap)
return 0;
}

+static int dw_pcie_iatu_config(struct dw_pcie_ep *ep)

How about using dw_pcie_ep_iatu_config() name? Just added 'ep' to be in sync with other API names in this file.

+{
+ struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+ struct device *dev = pci->dev;
+ void *addr;
+
+ 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;
+
+ return 0;
+}
+
+static int dw_pcie_ep_func_init(struct dw_pcie_ep *ep)
+{
+ struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+ struct dw_pcie_ep_func *ep_func;
+ struct pci_epc *epc = ep->epc;
+ struct device *dev = pci->dev;
+ u8 func_no;
+
+ 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);
+ }
+
+ return 0;
+}
+
int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep)
{
struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
@@ -643,7 +700,22 @@ int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep)
unsigned int nbars;
u8 hdr_type;
u32 reg;
- int i;
+ int ret, i;
+
+ if (ep->core_init_notifier) {
+ ret = dw_pcie_iatu_config(ep);
+ if (ret)
+ return ret;
+ }
+
+ if (ep->core_init_notifier) {
+ ret = dw_pcie_ep_func_init(ep);
+ if (ret)
+ return ret;
+
+ if (ep->ops->ep_init)
+ ep->ops->ep_init(ep);
+ }

hdr_type = dw_pcie_readb_dbi(pci, PCI_HEADER_TYPE) &
PCI_HEADER_TYPE_MASK;
@@ -677,8 +749,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 +756,12 @@ 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;
+
+ if (ep->ops->get_features) {
+ epc_features = ep->ops->get_features(ep);
+ if (epc_features->core_init_notifier)
+ ep->core_init_notifier = true;
+ }

INIT_LIST_HEAD(&ep->func_list);

@@ -708,7 +783,11 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
}
}

- dw_pcie_iatu_detect(pci);
+ if (!ep->core_init_notifier) {
+ ret = dw_pcie_iatu_config(ep);
+ if (ret)
+ return ret;
+ }

res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "addr_space");
if (!res)
@@ -717,26 +796,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,23 +812,15 @@ 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);
+ if (!ep->core_init_notifier) {
+ ret = dw_pcie_ep_func_init(ep);
+ if (ret)
+ return ret;

- list_add_tail(&ep_func->list, &ep->func_list);
+ if (ep->ops->ep_init)
+ ep->ops->ep_init(ep);
}

- if (ep->ops->ep_init)
- ep->ops->ep_init(ep);
-
ret = pci_epc_mem_init(epc, ep->phys_base, ep->addr_size,
ep->page_size);
if (ret < 0) {
@@ -784,12 +835,9 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
return -ENOMEM;
}

- if (ep->ops->get_features) {
- epc_features = ep->ops->get_features(ep);
- if (epc_features->core_init_notifier)
- return 0;
- }
+ if (!ep->core_init_notifier)
+ return dw_pcie_ep_init_complete(ep);

- return dw_pcie_ep_init_complete(ep);
+ return 0;
}
EXPORT_SYMBOL_GPL(dw_pcie_ep_init);
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 7d6e9b7576be..aadb14159df7 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -242,6 +242,7 @@ struct dw_pcie_ep {
void __iomem *msi_mem;
phys_addr_t msi_mem_phys;
struct pci_epf_bar *epf_bar[PCI_STD_NUM_BARS];
+ bool core_init_notifier;
};

struct dw_pcie_ops {
--
2.25.1

--
மணிவண்ணன் சதாசிவம்

Next message: Jiaxin Yu: "Re: [PATCH] ASoC: mediatek: mt8186: Remove condition with no effect"
Previous message: Stephen Rothwell: "linux-next: Tree for Jul 8"
Next in thread: Kunihiko Hayashi: "Re: [PATCH] PCI: designware-ep: Move DBI access to init_complete if notifier is used"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]