linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] PCI: endpoint: Fix core_init_notifier feature
@ 2021-07-27 23:45 Kunihiko Hayashi
  2021-07-27 23:45 ` [PATCH 1/2] PCI: endpoint: pci-epf-test: register notifier if only core_init_notifier is enabled Kunihiko Hayashi
  2021-07-27 23:45 ` [PATCH 2/2] PCI: designware-ep: Fix the access to DBI/iATU registers before enabling controller Kunihiko Hayashi
  0 siblings, 2 replies; 6+ messages in thread
From: Kunihiko Hayashi @ 2021-07-27 23:45 UTC (permalink / raw)
  To: Vidya Sagar, Jingoo Han, Gustavo Pimentel, Lorenzo Pieralisi,
	Rob Herring, Krzysztof Wilczyński, Bjorn Helgaas,
	Kishon Vijay Abraham I
  Cc: 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.

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] 6+ messages in thread

* [PATCH 1/2] PCI: endpoint: pci-epf-test: register notifier if only core_init_notifier is enabled
  2021-07-27 23:45 [PATCH 0/2] PCI: endpoint: Fix core_init_notifier feature Kunihiko Hayashi
@ 2021-07-27 23:45 ` Kunihiko Hayashi
  2021-07-28  2:05   ` Om Prakash Singh
  2021-07-27 23:45 ` [PATCH 2/2] PCI: designware-ep: Fix the access to DBI/iATU registers before enabling controller Kunihiko Hayashi
  1 sibling, 1 reply; 6+ messages in thread
From: Kunihiko Hayashi @ 2021-07-27 23:45 UTC (permalink / raw)
  To: Vidya Sagar, Jingoo Han, Gustavo Pimentel, Lorenzo Pieralisi,
	Rob Herring, Krzysztof Wilczyński, Bjorn Helgaas,
	Kishon Vijay Abraham I
  Cc: 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>
---
 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 d2708ca..73833a4 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -864,7 +864,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] 6+ messages in thread

* [PATCH 2/2] PCI: designware-ep: Fix the access to DBI/iATU registers before enabling controller
  2021-07-27 23:45 [PATCH 0/2] PCI: endpoint: Fix core_init_notifier feature Kunihiko Hayashi
  2021-07-27 23:45 ` [PATCH 1/2] PCI: endpoint: pci-epf-test: register notifier if only core_init_notifier is enabled Kunihiko Hayashi
@ 2021-07-27 23:45 ` Kunihiko Hayashi
  2021-07-28  1:45   ` Om Prakash Singh
  2021-08-03 16:08   ` Vidya Sagar
  1 sibling, 2 replies; 6+ messages in thread
From: Kunihiko Hayashi @ 2021-07-27 23:45 UTC (permalink / raw)
  To: Vidya Sagar, Jingoo Han, Gustavo Pimentel, Lorenzo Pieralisi,
	Rob Herring, Krzysztof Wilczyński, Bjorn Helgaas,
	Kishon Vijay Abraham I
  Cc: linux-pci, linux-kernel, Kunihiko Hayashi, Xiaowei Bao

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>
---
 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 8d028a8..f0c93d7 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] 6+ messages in thread

* Re: [PATCH 2/2] PCI: designware-ep: Fix the access to DBI/iATU registers before enabling controller
  2021-07-27 23:45 ` [PATCH 2/2] PCI: designware-ep: Fix the access to DBI/iATU registers before enabling controller Kunihiko Hayashi
@ 2021-07-28  1:45   ` Om Prakash Singh
  2021-08-03 16:08   ` Vidya Sagar
  1 sibling, 0 replies; 6+ messages in thread
From: Om Prakash Singh @ 2021-07-28  1:45 UTC (permalink / raw)
  To: Kunihiko Hayashi, Vidya Sagar, Jingoo Han, Gustavo Pimentel,
	Lorenzo Pieralisi, Rob Herring, Krzysztof Wilczyński,
	Bjorn Helgaas, Kishon Vijay Abraham I
  Cc: linux-pci, linux-kernel, Xiaowei Bao


Acked-by: Om Prakash Singh <omp@nvidia.com>


On 7/28/2021 5:15 AM, Kunihiko Hayashi wrote:
> External email: Use caution opening links or attachments
> 
> 
> 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>
> ---
>   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 8d028a8..f0c93d7 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	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] PCI: endpoint: pci-epf-test: register notifier if only core_init_notifier is enabled
  2021-07-27 23:45 ` [PATCH 1/2] PCI: endpoint: pci-epf-test: register notifier if only core_init_notifier is enabled Kunihiko Hayashi
@ 2021-07-28  2:05   ` Om Prakash Singh
  0 siblings, 0 replies; 6+ messages in thread
From: Om Prakash Singh @ 2021-07-28  2:05 UTC (permalink / raw)
  To: Kunihiko Hayashi, Vidya Sagar, Jingoo Han, Gustavo Pimentel,
	Lorenzo Pieralisi, Rob Herring, Krzysztof Wilczyński,
	Bjorn Helgaas, Kishon Vijay Abraham I
  Cc: linux-pci, linux-kernel


Acked-by: Om Prakash Singh <omp@nvidia.com>


On 7/28/2021 5:15 AM, Kunihiko Hayashi wrote:
> External email: Use caution opening links or attachments
> 
> 
> 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>
> ---
>   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 d2708ca..73833a4 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> @@ -864,7 +864,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	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] PCI: designware-ep: Fix the access to DBI/iATU registers before enabling controller
  2021-07-27 23:45 ` [PATCH 2/2] PCI: designware-ep: Fix the access to DBI/iATU registers before enabling controller Kunihiko Hayashi
  2021-07-28  1:45   ` Om Prakash Singh
@ 2021-08-03 16:08   ` Vidya Sagar
  1 sibling, 0 replies; 6+ messages in thread
From: Vidya Sagar @ 2021-08-03 16:08 UTC (permalink / raw)
  To: Kunihiko Hayashi, Jingoo Han, Gustavo Pimentel,
	Lorenzo Pieralisi, Rob Herring, Krzysztof Wilczyński,
	Bjorn Helgaas, Kishon Vijay Abraham I
  Cc: linux-pci, linux-kernel, Xiaowei Bao

Reviewed-by: Vidya Sagar <vidyas@nvidia.com>

On 7/28/2021 5:15 AM, Kunihiko Hayashi wrote:
> External email: Use caution opening links or attachments
> 
> 
> 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>
> ---
>   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 8d028a8..f0c93d7 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	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-08-03 16:08 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-27 23:45 [PATCH 0/2] PCI: endpoint: Fix core_init_notifier feature Kunihiko Hayashi
2021-07-27 23:45 ` [PATCH 1/2] PCI: endpoint: pci-epf-test: register notifier if only core_init_notifier is enabled Kunihiko Hayashi
2021-07-28  2:05   ` Om Prakash Singh
2021-07-27 23:45 ` [PATCH 2/2] PCI: designware-ep: Fix the access to DBI/iATU registers before enabling controller Kunihiko Hayashi
2021-07-28  1:45   ` Om Prakash Singh
2021-08-03 16:08   ` Vidya Sagar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).