Re: [PATCH v2 5/5] PCI: dwc: Move dw_pcie_msi_init() from each users to designware host

2021-03-10 Thread Jisheng Zhang 
On Sun, 7 Mar 2021 23:10:12 +0100 Krzysztof Wilczyński  wrote:

> 
> 
> Hi,
> 
> > Currently, dw_pcie_msi_init() allocates and maps page for msi, then
> > program the PCIE_MSI_ADDR_LO and PCIE_MSI_ADDR_HI. The Root Complex
> > may lose power during suspend-to-RAM, so when we resume, we want to
> > redo the latter but not the former. If designware based driver (for
> > example, pcie-tegra194.c) calls dw_pcie_msi_init() in resume path, the
> > previous msi page will be leaked. From another side, except
> > pci-dra7xx.c we can move the dw_pcie_msi_init() from each users to
> > designware host, I.E move the msi page allocation and mapping to
> > dw_pcie_host_init() and move the PCIE_MSI_ADDR_* programming to
> > dw_pcie_setup_rc(). After this moving, we solve the msi page leakage
> > as well.  
> [...]
> 
> A small nitpick.  All the "designware" should be "DesignWare" both in
> the commit message and the subject.  Similarly, "msi" would be "MSI",
> and "I.E" would become "i.e.,".  If you ever sent another version of the
> patch, that is.
> 

Hi

This series was dropped and useless after Rob's excellent dw pcie code clean
up.

Thanks

Re: [PATCH v2 5/5] PCI: dwc: Move dw_pcie_msi_init() from each users to designware host

2021-03-07 Thread Krzysztof Wilczyński 
Hi,

> Currently, dw_pcie_msi_init() allocates and maps page for msi, then
> program the PCIE_MSI_ADDR_LO and PCIE_MSI_ADDR_HI. The Root Complex
> may lose power during suspend-to-RAM, so when we resume, we want to
> redo the latter but not the former. If designware based driver (for
> example, pcie-tegra194.c) calls dw_pcie_msi_init() in resume path, the
> previous msi page will be leaked. From another side, except
> pci-dra7xx.c we can move the dw_pcie_msi_init() from each users to
> designware host, I.E move the msi page allocation and mapping to
> dw_pcie_host_init() and move the PCIE_MSI_ADDR_* programming to
> dw_pcie_setup_rc(). After this moving, we solve the msi page leakage
> as well.
[...]

A small nitpick.  All the "designware" should be "DesignWare" both in
the commit message and the subject.  Similarly, "msi" would be "MSI",
and "I.E" would become "i.e.,".  If you ever sent another version of the
patch, that is.

See the following for reference:
  
https://lore.kernel.org/linux-pci/20171026223701.ga25...@bhelgaas-glaptop.roam.corp.google.com/

Krzysztof

Re: [PATCH v2 5/5] PCI: dwc: Move dw_pcie_msi_init() from each users to designware host

2020-10-07 Thread Vidya Sagar 




On 9/24/2020 4:37 PM, Jisheng Zhang wrote:

External email: Use caution opening links or attachments


Currently, dw_pcie_msi_init() allocates and maps page for msi, then
program the PCIE_MSI_ADDR_LO and PCIE_MSI_ADDR_HI. The Root Complex
may lose power during suspend-to-RAM, so when we resume, we want to
redo the latter but not the former. If designware based driver (for
example, pcie-tegra194.c) calls dw_pcie_msi_init() in resume path, the
previous msi page will be leaked. From another side, except
pci-dra7xx.c we can move the dw_pcie_msi_init() from each users to
designware host, I.E move the msi page allocation and mapping to
dw_pcie_host_init() and move the PCIE_MSI_ADDR_* programming to
dw_pcie_setup_rc(). After this moving, we solve the msi page leakage
as well.

Signed-off-by: Jisheng Zhang 
---
  drivers/pci/controller/dwc/pci-dra7xx.c   |  1 +
  drivers/pci/controller/dwc/pci-exynos.c   |  2 --
  drivers/pci/controller/dwc/pci-imx6.c |  3 ---
  drivers/pci/controller/dwc/pci-meson.c|  8 ---
  drivers/pci/controller/dwc/pcie-artpec6.c | 10 
  .../pci/controller/dwc/pcie-designware-host.c | 24 ---
  .../pci/controller/dwc/pcie-designware-plat.c |  3 ---
  drivers/pci/controller/dwc/pcie-designware.h  |  5 
  drivers/pci/controller/dwc/pcie-histb.c   |  3 ---
  drivers/pci/controller/dwc/pcie-kirin.c   |  3 ---
  drivers/pci/controller/dwc/pcie-qcom.c|  3 ---
  drivers/pci/controller/dwc/pcie-spear13xx.c   |  1 -
  drivers/pci/controller/dwc/pcie-tegra194.c|  2 --
  drivers/pci/controller/dwc/pcie-uniphier.c|  9 +--
  14 files changed, 22 insertions(+), 55 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c 
b/drivers/pci/controller/dwc/pci-dra7xx.c
index dc387724cf08..d8b74389e353 100644
--- a/drivers/pci/controller/dwc/pci-dra7xx.c
+++ b/drivers/pci/controller/dwc/pci-dra7xx.c
@@ -210,6 +210,7 @@ static int dra7xx_pcie_host_init(struct pcie_port *pp)
 dra7xx_pcie_establish_link(pci);
 dw_pcie_wait_for_link(pci);
 dw_pcie_msi_init(pp);
+   dw_pcie_msi_config(pp);
 dra7xx_pcie_enable_interrupts(dra7xx);

 return 0;
diff --git a/drivers/pci/controller/dwc/pci-exynos.c 
b/drivers/pci/controller/dwc/pci-exynos.c
index 8d82c43ae299..9cca0ce79777 100644
--- a/drivers/pci/controller/dwc/pci-exynos.c
+++ b/drivers/pci/controller/dwc/pci-exynos.c
@@ -298,8 +298,6 @@ static void exynos_pcie_msi_init(struct exynos_pcie *ep)
 struct pcie_port *pp = >pp;
 u32 val;

-   dw_pcie_msi_init(pp);
-
 /* enable MSI interrupt */
 val = exynos_pcie_readl(ep->mem_res->elbi_base, PCIE_IRQ_EN_LEVEL);
 val |= IRQ_MSI_ENABLE;
diff --git a/drivers/pci/controller/dwc/pci-imx6.c 
b/drivers/pci/controller/dwc/pci-imx6.c
index 5fef2613b223..dba6e351e3df 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -848,9 +848,6 @@ static int imx6_pcie_host_init(struct pcie_port *pp)
 dw_pcie_setup_rc(pp);
 imx6_pcie_establish_link(imx6_pcie);

-   if (IS_ENABLED(CONFIG_PCI_MSI))
-   dw_pcie_msi_init(pp);
-
 return 0;
  }

diff --git a/drivers/pci/controller/dwc/pci-meson.c 
b/drivers/pci/controller/dwc/pci-meson.c
index 4f183b96afbb..cd0d9dd8dd61 100644
--- a/drivers/pci/controller/dwc/pci-meson.c
+++ b/drivers/pci/controller/dwc/pci-meson.c
@@ -377,12 +377,6 @@ static int meson_pcie_establish_link(struct meson_pcie *mp)
 return dw_pcie_wait_for_link(pci);
  }

-static void meson_pcie_enable_interrupts(struct meson_pcie *mp)
-{
-   if (IS_ENABLED(CONFIG_PCI_MSI))
-   dw_pcie_msi_init(>pci.pp);
-}
-
  static int meson_pcie_rd_own_conf(struct pcie_port *pp, int where, int size,
   u32 *val)
  {
@@ -467,8 +461,6 @@ static int meson_pcie_host_init(struct pcie_port *pp)
 if (ret)
 return ret;

-   meson_pcie_enable_interrupts(mp);
-
 return 0;
  }

diff --git a/drivers/pci/controller/dwc/pcie-artpec6.c 
b/drivers/pci/controller/dwc/pcie-artpec6.c
index 97d50bb50f06..af1e6bb28e7a 100644
--- a/drivers/pci/controller/dwc/pcie-artpec6.c
+++ b/drivers/pci/controller/dwc/pcie-artpec6.c
@@ -346,15 +346,6 @@ static void artpec6_pcie_deassert_core_reset(struct 
artpec6_pcie *artpec6_pcie)
 usleep_range(100, 200);
  }

-static void artpec6_pcie_enable_interrupts(struct artpec6_pcie *artpec6_pcie)
-{
-   struct dw_pcie *pci = artpec6_pcie->pci;
-   struct pcie_port *pp = >pp;
-
-   if (IS_ENABLED(CONFIG_PCI_MSI))
-   dw_pcie_msi_init(pp);
-}
-
  static int artpec6_pcie_host_init(struct pcie_port *pp)
  {
 struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
@@ -368,7 +359,6 @@ static int artpec6_pcie_host_init(struct pcie_port *pp)
 dw_pcie_setup_rc(pp);
 artpec6_pcie_establish_link(pci);
 dw_pcie_wait_for_link(pci);
-

[PATCH v2 5/5] PCI: dwc: Move dw_pcie_msi_init() from each users to designware host

2020-09-24 Thread Jisheng Zhang 
Currently, dw_pcie_msi_init() allocates and maps page for msi, then
program the PCIE_MSI_ADDR_LO and PCIE_MSI_ADDR_HI. The Root Complex
may lose power during suspend-to-RAM, so when we resume, we want to
redo the latter but not the former. If designware based driver (for
example, pcie-tegra194.c) calls dw_pcie_msi_init() in resume path, the
previous msi page will be leaked. From another side, except
pci-dra7xx.c we can move the dw_pcie_msi_init() from each users to
designware host, I.E move the msi page allocation and mapping to
dw_pcie_host_init() and move the PCIE_MSI_ADDR_* programming to
dw_pcie_setup_rc(). After this moving, we solve the msi page leakage
as well.

Signed-off-by: Jisheng Zhang 
---
 drivers/pci/controller/dwc/pci-dra7xx.c   |  1 +
 drivers/pci/controller/dwc/pci-exynos.c   |  2 --
 drivers/pci/controller/dwc/pci-imx6.c |  3 ---
 drivers/pci/controller/dwc/pci-meson.c|  8 ---
 drivers/pci/controller/dwc/pcie-artpec6.c | 10 
 .../pci/controller/dwc/pcie-designware-host.c | 24 ---
 .../pci/controller/dwc/pcie-designware-plat.c |  3 ---
 drivers/pci/controller/dwc/pcie-designware.h  |  5 
 drivers/pci/controller/dwc/pcie-histb.c   |  3 ---
 drivers/pci/controller/dwc/pcie-kirin.c   |  3 ---
 drivers/pci/controller/dwc/pcie-qcom.c|  3 ---
 drivers/pci/controller/dwc/pcie-spear13xx.c   |  1 -
 drivers/pci/controller/dwc/pcie-tegra194.c|  2 --
 drivers/pci/controller/dwc/pcie-uniphier.c|  9 +--
 14 files changed, 22 insertions(+), 55 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c 
b/drivers/pci/controller/dwc/pci-dra7xx.c
index dc387724cf08..d8b74389e353 100644
--- a/drivers/pci/controller/dwc/pci-dra7xx.c
+++ b/drivers/pci/controller/dwc/pci-dra7xx.c
@@ -210,6 +210,7 @@ static int dra7xx_pcie_host_init(struct pcie_port *pp)
dra7xx_pcie_establish_link(pci);
dw_pcie_wait_for_link(pci);
dw_pcie_msi_init(pp);
+   dw_pcie_msi_config(pp);
dra7xx_pcie_enable_interrupts(dra7xx);
 
return 0;
diff --git a/drivers/pci/controller/dwc/pci-exynos.c 
b/drivers/pci/controller/dwc/pci-exynos.c
index 8d82c43ae299..9cca0ce79777 100644
--- a/drivers/pci/controller/dwc/pci-exynos.c
+++ b/drivers/pci/controller/dwc/pci-exynos.c
@@ -298,8 +298,6 @@ static void exynos_pcie_msi_init(struct exynos_pcie *ep)
struct pcie_port *pp = >pp;
u32 val;
 
-   dw_pcie_msi_init(pp);
-
/* enable MSI interrupt */
val = exynos_pcie_readl(ep->mem_res->elbi_base, PCIE_IRQ_EN_LEVEL);
val |= IRQ_MSI_ENABLE;
diff --git a/drivers/pci/controller/dwc/pci-imx6.c 
b/drivers/pci/controller/dwc/pci-imx6.c
index 5fef2613b223..dba6e351e3df 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -848,9 +848,6 @@ static int imx6_pcie_host_init(struct pcie_port *pp)
dw_pcie_setup_rc(pp);
imx6_pcie_establish_link(imx6_pcie);
 
-   if (IS_ENABLED(CONFIG_PCI_MSI))
-   dw_pcie_msi_init(pp);
-
return 0;
 }
 
diff --git a/drivers/pci/controller/dwc/pci-meson.c 
b/drivers/pci/controller/dwc/pci-meson.c
index 4f183b96afbb..cd0d9dd8dd61 100644
--- a/drivers/pci/controller/dwc/pci-meson.c
+++ b/drivers/pci/controller/dwc/pci-meson.c
@@ -377,12 +377,6 @@ static int meson_pcie_establish_link(struct meson_pcie *mp)
return dw_pcie_wait_for_link(pci);
 }
 
-static void meson_pcie_enable_interrupts(struct meson_pcie *mp)
-{
-   if (IS_ENABLED(CONFIG_PCI_MSI))
-   dw_pcie_msi_init(>pci.pp);
-}
-
 static int meson_pcie_rd_own_conf(struct pcie_port *pp, int where, int size,
  u32 *val)
 {
@@ -467,8 +461,6 @@ static int meson_pcie_host_init(struct pcie_port *pp)
if (ret)
return ret;
 
-   meson_pcie_enable_interrupts(mp);
-
return 0;
 }
 
diff --git a/drivers/pci/controller/dwc/pcie-artpec6.c 
b/drivers/pci/controller/dwc/pcie-artpec6.c
index 97d50bb50f06..af1e6bb28e7a 100644
--- a/drivers/pci/controller/dwc/pcie-artpec6.c
+++ b/drivers/pci/controller/dwc/pcie-artpec6.c
@@ -346,15 +346,6 @@ static void artpec6_pcie_deassert_core_reset(struct 
artpec6_pcie *artpec6_pcie)
usleep_range(100, 200);
 }
 
-static void artpec6_pcie_enable_interrupts(struct artpec6_pcie *artpec6_pcie)
-{
-   struct dw_pcie *pci = artpec6_pcie->pci;
-   struct pcie_port *pp = >pp;
-
-   if (IS_ENABLED(CONFIG_PCI_MSI))
-   dw_pcie_msi_init(pp);
-}
-
 static int artpec6_pcie_host_init(struct pcie_port *pp)
 {
struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
@@ -368,7 +359,6 @@ static int artpec6_pcie_host_init(struct pcie_port *pp)
dw_pcie_setup_rc(pp);
artpec6_pcie_establish_link(pci);
dw_pcie_wait_for_link(pci);
-   artpec6_pcie_enable_interrupts(artpec6_pcie);
 
return 0;
 }
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c