Discussion:
[PATCH v6 0/4] pcie-designware: add iATU unroll feature
(too old to reply)
Bjorn Helgaas
2016-08-17 20:00:02 UTC
Permalink
Original description from Joao:

The new DWC PCIe Core version (4.80) implements iATU in a different
way. This new mechanism is called iATU Unroll Mode. The Core still
supports the "old" mechanism calling it Legacy Mode if configured to
do so, but the standard way will be using Unroll. This patch adds
the necessary support for the mechanism and makes some minor
improvements to the existent one.

I made a few changes from the v5 series Joao posted, so I'm posting
the patches I applied as this v6.

changes v5->v6:
- Add a patch to change the signature of dw_pcie_readl_rc() from
this:
void dw_pcie_readl_rc(struct pcie_port *pp, u32 reg, u32 *val)
to this:
u32 dw_pcie_readl_rc(struct pcie_port *pp, u32 reg)
Note that this also affects Exynos.
- Move loop checking for iATU enable to a separate patch.
- Change iATU register definitions to be offsets (not register
numbers). This is to match the existing style of the pre-unroll
definitions.
- Rename dw_pcie_get_atu_mode() and related things to
dw_pcie_iatu_unroll_enabled() so they're more descriptive.

---

Bjorn Helgaas (1):
PCI: designware: Return data directly from dw_pcie_readl_rc()

Joao Pinto (3):
PCI: designware: Move link wait definitions to .c file
PCI: designware: Wait for iATU enable
PCI: designware: Add iATU Unroll feature


drivers/pci/host/pci-exynos.c | 9 ++
drivers/pci/host/pcie-designware.c | 138 ++++++++++++++++++++++++++++++------
drivers/pci/host/pcie-designware.h | 9 +-
3 files changed, 123 insertions(+), 33 deletions(-)
Bjorn Helgaas
2016-08-17 20:10:02 UTC
Permalink
From: Joao Pinto <***@synopsys.com>

Move the link wait sleep definitions to the .c file as suggested by
Jisheng Zhang in a previous patch.

Signed-off-by: Joao Pinto <***@synopsys.com>
Signed-off-by: Bjorn Helgaas <***@google.com>
CC: Jisheng Zhang <***@marvell.com>
---
drivers/pci/host/pcie-designware.c | 5 +++++
drivers/pci/host/pcie-designware.h | 5 -----
2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 1f6bd6d..e99f56e 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -26,6 +26,11 @@

#include "pcie-designware.h"

+/* Parameters for the waiting for link up routine */
+#define LINK_WAIT_MAX_RETRIES 10
+#define LINK_WAIT_USLEEP_MIN 90000
+#define LINK_WAIT_USLEEP_MAX 100000
+
/* Synopsis specific PCIE configuration registers */
#define PCIE_PORT_LINK_CONTROL 0x710
#define PORT_LINK_MODE_MASK (0x3f << 16)
diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
index 74a8fc6..285e1ed 100644
--- a/drivers/pci/host/pcie-designware.h
+++ b/drivers/pci/host/pcie-designware.h
@@ -22,11 +22,6 @@
#define MAX_MSI_IRQS 32
#define MAX_MSI_CTRLS (MAX_MSI_IRQS / 32)

-/* Parameters for the waiting for link up routine */
-#define LINK_WAIT_MAX_RETRIES 10
-#define LINK_WAIT_USLEEP_MIN 90000
-#define LINK_WAIT_USLEEP_MAX 100000
-
struct pcie_port {
struct device *dev;
u8 root_bus_nr;
Bjorn Helgaas
2016-08-17 20:10:02 UTC
Permalink
From: Joao Pinto <***@synopsys.com>

Add a loop with timeout to make sure the iATU is really enabled before
subsequent config and I/O accesses.

[bhelgaas: split to separate patch, use dev_err() instead of dev_dbg()]
Signed-off-by: Joao Pinto <***@synopsys.com>
Signed-off-by: Bjorn Helgaas <***@google.com>
---
drivers/pci/host/pcie-designware.c | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index e99f56e..947fac3 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -31,7 +31,12 @@
#define LINK_WAIT_USLEEP_MIN 90000
#define LINK_WAIT_USLEEP_MAX 100000

-/* Synopsis specific PCIE configuration registers */
+/* Parameters for the waiting for iATU enabled routine */
+#define LINK_WAIT_MAX_IATU_RETRIES 5
+#define LINK_WAIT_IATU_MIN 9000
+#define LINK_WAIT_IATU_MAX 10000
+
+/* Synopsys-specific PCIe configuration registers */
#define PCIE_PORT_LINK_CONTROL 0x710
#define PORT_LINK_MODE_MASK (0x3f << 16)
#define PORT_LINK_MODE_1_LANES (0x1 << 16)
@@ -157,7 +162,7 @@ static int dw_pcie_wr_own_conf(struct pcie_port *pp, int where, int size,
static void dw_pcie_prog_outbound_atu(struct pcie_port *pp, int index,
int type, u64 cpu_addr, u64 pci_addr, u32 size)
{
- u32 val;
+ u32 retries, val;

dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | index,
PCIE_ATU_VIEWPORT);
@@ -174,7 +179,14 @@ static void dw_pcie_prog_outbound_atu(struct pcie_port *pp, int index,
* Make sure ATU enable takes effect before any subsequent config
* and I/O accesses.
*/
- val = dw_pcie_readl_rc(pp, PCIE_ATU_CR2);
+ for (retries = 0; retries < LINK_WAIT_MAX_IATU_RETRIES; retries++) {
+ val = dw_pcie_readl_rc(pp, PCIE_ATU_CR2);
+ if (val == PCIE_ATU_ENABLE)
+ return;
+
+ usleep_range(LINK_WAIT_IATU_MIN, LINK_WAIT_IATU_MAX);
+ }
+ dev_err(pp->dev, "iATU is not being enabled\n");
}

static struct irq_chip dw_msi_irq_chip = {
Bjorn Helgaas
2016-08-17 20:10:02 UTC
Permalink
From: Joao Pinto <***@synopsys.com>

Add support for the new iATU Unroll mechanism that will be used from Core
version 4.80. The new Cores can support either iATU Unroll or the "old"
iATU method, now called Legacy Mode. The driver is perfectly capable of
performing well for both.

[bhelgaas: split ATU enable timeout to separate patch]
Signed-off-by: Joao Pinto <***@synopsys.com>
Signed-off-by: Bjorn Helgaas <***@google.com>
---
drivers/pci/host/pcie-designware.c | 97 ++++++++++++++++++++++++++++++++----
drivers/pci/host/pcie-designware.h | 1
2 files changed, 87 insertions(+), 11 deletions(-)

diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 947fac3..e489b18 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -80,6 +80,21 @@
#define PCIE_ATU_FUNC(x) (((x) & 0x7) << 16)
#define PCIE_ATU_UPPER_TARGET 0x91C

+/*
+ * iATU Unroll-specific register definitions
+ * From 4.80 core version the address translation will be made by unroll
+ */
+#define PCIE_ATU_UNR_REGION_CTRL1 0x00
+#define PCIE_ATU_UNR_REGION_CTRL2 0x04
+#define PCIE_ATU_UNR_LOWER_BASE 0x08
+#define PCIE_ATU_UNR_UPPER_BASE 0x0C
+#define PCIE_ATU_UNR_LIMIT 0x10
+#define PCIE_ATU_UNR_LOWER_TARGET 0x14
+#define PCIE_ATU_UNR_UPPER_TARGET 0x18
+
+/* Register address builder */
+#define PCIE_GET_ATU_OUTB_UNR_REG_OFFSET(region) ((0x3 << 20) | (region << 9)
+
/* PCIe Port Logic registers */
#define PLR_OFFSET 0x700
#define PCIE_PHY_DEBUG_R1 (PLR_OFFSET + 0x2c)
@@ -141,6 +156,27 @@ static inline void dw_pcie_writel_rc(struct pcie_port *pp, u32 val, u32 reg)
writel(val, pp->dbi_base + reg);
}

+static inline u32 dw_pcie_readl_unroll(struct pcie_port *pp, u32 index, u32 reg)
+{
+ u32 offset = PCIE_GET_ATU_OUTB_UNR_REG_OFFSET(index);
+
+ if (pp->ops->readl_rc)
+ return pp->ops->readl_rc(pp, pp->dbi_base + offset + reg);
+
+ return readl(pp->dbi_base + offset + reg);
+}
+
+static inline void dw_pcie_writel_unroll(struct pcie_port *pp, u32 index,
+ u32 val, u32 reg)
+{
+ u32 offset = PCIE_GET_ATU_OUTB_UNR_REG_OFFSET(index);
+
+ if (pp->ops->writel_rc)
+ pp->ops->writel_rc(pp, val, pp->dbi_base + offset + reg);
+ else
+ writel(val, pp->dbi_base + offset + reg);
+}
+
static int dw_pcie_rd_own_conf(struct pcie_port *pp, int where, int size,
u32 *val)
{
@@ -164,23 +200,49 @@ static void dw_pcie_prog_outbound_atu(struct pcie_port *pp, int index,
{
u32 retries, val;

- dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | index,
- PCIE_ATU_VIEWPORT);
- dw_pcie_writel_rc(pp, lower_32_bits(cpu_addr), PCIE_ATU_LOWER_BASE);
- dw_pcie_writel_rc(pp, upper_32_bits(cpu_addr), PCIE_ATU_UPPER_BASE);
- dw_pcie_writel_rc(pp, lower_32_bits(cpu_addr + size - 1),
- PCIE_ATU_LIMIT);
- dw_pcie_writel_rc(pp, lower_32_bits(pci_addr), PCIE_ATU_LOWER_TARGET);
- dw_pcie_writel_rc(pp, upper_32_bits(pci_addr), PCIE_ATU_UPPER_TARGET);
- dw_pcie_writel_rc(pp, type, PCIE_ATU_CR1);
- dw_pcie_writel_rc(pp, PCIE_ATU_ENABLE, PCIE_ATU_CR2);
+ if (pp->iatu_unroll_enabled) {
+ dw_pcie_writel_unroll(pp, index,
+ lower_32_bits(cpu_addr), PCIE_ATU_UNR_LOWER_BASE);
+ dw_pcie_writel_unroll(pp, index,
+ upper_32_bits(cpu_addr), PCIE_ATU_UNR_UPPER_BASE);
+ dw_pcie_writel_unroll(pp, index,
+ lower_32_bits(cpu_addr + size - 1), PCIE_ATU_UNR_LIMIT);
+ dw_pcie_writel_unroll(pp, index,
+ lower_32_bits(pci_addr), PCIE_ATU_UNR_LOWER_TARGET);
+ dw_pcie_writel_unroll(pp, index,
+ upper_32_bits(pci_addr), PCIE_ATU_UNR_UPPER_TARGET);
+ dw_pcie_writel_unroll(pp, index,
+ type, PCIE_ATU_UNR_REGION_CTRL1);
+ dw_pcie_writel_unroll(pp, index,
+ PCIE_ATU_ENABLE, PCIE_ATU_UNR_REGION_CTRL2);
+ } else {
+ dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | index,
+ PCIE_ATU_VIEWPORT);
+ dw_pcie_writel_rc(pp, lower_32_bits(cpu_addr),
+ PCIE_ATU_LOWER_BASE);
+ dw_pcie_writel_rc(pp, upper_32_bits(cpu_addr),
+ PCIE_ATU_UPPER_BASE);
+ dw_pcie_writel_rc(pp, lower_32_bits(cpu_addr + size - 1),
+ PCIE_ATU_LIMIT);
+ dw_pcie_writel_rc(pp, lower_32_bits(pci_addr),
+ PCIE_ATU_LOWER_TARGET);
+ dw_pcie_writel_rc(pp, upper_32_bits(pci_addr),
+ PCIE_ATU_UPPER_TARGET);
+ dw_pcie_writel_rc(pp, type, PCIE_ATU_CR1);
+ dw_pcie_writel_rc(pp, PCIE_ATU_ENABLE, PCIE_ATU_CR2);
+ }

/*
* Make sure ATU enable takes effect before any subsequent config
* and I/O accesses.
*/
for (retries = 0; retries < LINK_WAIT_MAX_IATU_RETRIES; retries++) {
- val = dw_pcie_readl_rc(pp, PCIE_ATU_CR2);
+ if (pp->iatu_unroll_enabled)
+ val = dw_pcie_readl_unroll(pp, index,
+ PCIE_ATU_UNR_REGION_CTRL2);
+ else
+ val = dw_pcie_readl_rc(pp, PCIE_ATU_CR2);
+
if (val == PCIE_ATU_ENABLE)
return;

@@ -445,6 +507,17 @@ static const struct irq_domain_ops msi_domain_ops = {
.map = dw_pcie_msi_map,
};

+static u8 dw_pcie_iatu_unroll_enabled(struct pcie_port *pp)
+{
+ u32 val = 0;
+
+ dw_pcie_readl_rc(pp, PCIE_ATU_VIEWPORT, &val);
+ if (val == 0xffffffff)
+ return 1;
+
+ return 0;
+}
+
int dw_pcie_host_init(struct pcie_port *pp)
{
struct device_node *np = pp->dev->of_node;
@@ -561,6 +634,8 @@ int dw_pcie_host_init(struct pcie_port *pp)
}
}

+ pp->iatu_unroll_enabled = dw_pcie_iatu_unroll_enabled(pp);
+
if (pp->ops->host_init)
pp->ops->host_init(pp);

diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
index 285e1ed..e74cc4a 100644
--- a/drivers/pci/host/pcie-designware.h
+++ b/drivers/pci/host/pcie-designware.h
@@ -48,6 +48,7 @@ struct pcie_port {
int msi_irq;
struct irq_domain *irq_domain;
unsigned long msi_data;
+ u8 iatu_unroll_enabled;
DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS);
};
kbuild test robot
2016-08-17 20:50:01 UTC
Permalink
Hi Joao,

[auto build test ERROR on pci/next]
[also build test ERROR on v4.8-rc2 next-20160817]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Bjorn-Helgaas/PCI-designware-Return-data-directly-from-dw_pcie_readl_rc/20160818-040531
base: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: x86_64-randconfig-x015-201633 (attached as .config)
compiler: gcc-6 (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
drivers/pci/host/pcie-designware.c:161:54: error: expected ')' before ';' token
u32 offset = PCIE_GET_ATU_OUTB_UNR_REG_OFFSET(index);
^
drivers/pci/host/pcie-designware.c:167:1: error: expected ',' or ';' before '}' token
}
^
drivers/pci/host/pcie-designware.c:169:20: error: invalid storage class for function 'dw_pcie_writel_unroll'
static inline void dw_pcie_writel_unroll(struct pcie_port *pp, u32 index,
^~~~~~~~~~~~~~~~~~~~~
drivers/pci/host/pcie-designware.c: In function 'dw_pcie_writel_unroll':
drivers/pci/host/pcie-designware.c:172:54: error: expected ')' before ';' token
u32 offset = PCIE_GET_ATU_OUTB_UNR_REG_OFFSET(index);
^
drivers/pci/host/pcie-designware.c:178:1: error: expected ',' or ';' before '}' token
}
^
drivers/pci/host/pcie-designware.c:180:12: error: invalid storage class for function 'dw_pcie_rd_own_conf'
static int dw_pcie_rd_own_conf(struct pcie_port *pp, int where, int size,
^~~~~~~~~~~~~~~~~~~
drivers/pci/host/pcie-designware.c:189:12: error: invalid storage class for function 'dw_pcie_wr_own_conf'
static int dw_pcie_wr_own_conf(struct pcie_port *pp, int where, int size,
^~~~~~~~~~~~~~~~~~~
drivers/pci/host/pcie-designware.c:198:13: error: invalid storage class for function 'dw_pcie_prog_outbound_atu'
static void dw_pcie_prog_outbound_atu(struct pcie_port *pp, int index,
^~~~~~~~~~~~~~~~~~~~~~~~~
drivers/pci/host/pcie-designware.c:304:13: error: invalid storage class for function 'dw_pcie_msi_clear_irq'
static void dw_pcie_msi_clear_irq(struct pcie_port *pp, int irq)
^~~~~~~~~~~~~~~~~~~~~
drivers/pci/host/pcie-designware.c:315:13: error: invalid storage class for function 'clear_irq_range'
static void clear_irq_range(struct pcie_port *pp, unsigned int irq_base,
^~~~~~~~~~~~~~~
drivers/pci/host/pcie-designware.c:332:13: error: invalid storage class for function 'dw_pcie_msi_set_irq'
static void dw_pcie_msi_set_irq(struct pcie_port *pp, int irq)
^~~~~~~~~~~~~~~~~~~
drivers/pci/host/pcie-designware.c:343:12: error: invalid storage class for function 'assign_irq'
static int assign_irq(int no_irqs, struct msi_desc *desc, int *pos)
^~~~~~~~~~
drivers/pci/host/pcie-designware.c:387:13: error: invalid storage class for function 'dw_msi_setup_msg'
static void dw_msi_setup_msg(struct pcie_port *pp, unsigned int irq, u32 pos)
^~~~~~~~~~~~~~~~
drivers/pci/host/pcie-designware.c:408:12: error: invalid storage class for function 'dw_msi_setup_irq'
static int dw_msi_setup_irq(struct msi_controller *chip, struct pci_dev *pdev,
^~~~~~~~~~~~~~~~
drivers/pci/host/pcie-designware.c:426:12: error: invalid storage class for function 'dw_msi_setup_irqs'
static int dw_msi_setup_irqs(struct msi_controller *chip, struct pci_dev *pdev,
^~~~~~~~~~~~~~~~~
drivers/pci/host/pcie-designware.c:453:13: error: invalid storage class for function 'dw_msi_teardown_irq'
static void dw_msi_teardown_irq(struct msi_controller *chip, unsigned int irq)
^~~~~~~~~~~~~~~~~~~
drivers/pci/host/pcie-designware.c:463:15: error: initializer element is not constant
.setup_irq = dw_msi_setup_irq,
^~~~~~~~~~~~~~~~
drivers/pci/host/pcie-designware.c:463:15: note: (near initialization for 'dw_pcie_msi_chip.setup_irq')
drivers/pci/host/pcie-designware.c:464:16: error: initializer element is not constant
.setup_irqs = dw_msi_setup_irqs,
^~~~~~~~~~~~~~~~~
drivers/pci/host/pcie-designware.c:464:16: note: (near initialization for 'dw_pcie_msi_chip.setup_irqs')
drivers/pci/host/pcie-designware.c:465:18: error: initializer element is not constant
.teardown_irq = dw_msi_teardown_irq,
^~~~~~~~~~~~~~~~~~~
drivers/pci/host/pcie-designware.c:465:18: note: (near initialization for 'dw_pcie_msi_chip.teardown_irq')
drivers/pci/host/pcie-designware.c:497:12: error: invalid storage class for function 'dw_pcie_msi_map'
static int dw_pcie_msi_map(struct irq_domain *domain, unsigned int irq,
^~~~~~~~~~~~~~~
drivers/pci/host/pcie-designware.c:507:9: error: initializer element is not constant
.map = dw_pcie_msi_map,
^~~~~~~~~~~~~~~
drivers/pci/host/pcie-designware.c:507:9: note: (near initialization for 'msi_domain_ops.map')
drivers/pci/host/pcie-designware.c:510:11: error: invalid storage class for function 'dw_pcie_iatu_unroll_enabled'
static u8 dw_pcie_iatu_unroll_enabled(struct pcie_port *pp)
^~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/pci/host/pcie-designware.c:514:2: error: too many arguments to function 'dw_pcie_readl_rc'
dw_pcie_readl_rc(pp, PCIE_ATU_VIEWPORT, &val);
^~~~~~~~~~~~~~~~
drivers/pci/host/pcie-designware.c:143:19: note: declared here
static inline u32 dw_pcie_readl_rc(struct pcie_port *pp, u32 reg)
^~~~~~~~~~~~~~~~
drivers/pci/host/pcie-designware.c:678:12: error: invalid storage class for function 'dw_pcie_rd_other_conf'
static int dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus,
^~~~~~~~~~~~~~~~~~~~~
drivers/pci/host/pcie-designware.c:715:12: error: invalid storage class for function 'dw_pcie_wr_other_conf'
static int dw_pcie_wr_other_conf(struct pcie_port *pp, struct pci_bus *bus,
^~~~~~~~~~~~~~~~~~~~~

vim +161 drivers/pci/host/pcie-designware.c

137 else
138 return PCIBIOS_BAD_REGISTER_NUMBER;
139
140 return PCIBIOS_SUCCESSFUL;
141 }
142
143 static inline u32 dw_pcie_readl_rc(struct pcie_port *pp, u32 reg)
144 {
145 if (pp->ops->readl_rc)
146 return pp->ops->readl_rc(pp, pp->dbi_base + reg);
147
148 return readl(pp->dbi_base + reg);
149 }
150
151 static inline void dw_pcie_writel_rc(struct pcie_port *pp, u32 val, u32 reg)
152 {
153 if (pp->ops->writel_rc)
154 pp->ops->writel_rc(pp, val, pp->dbi_base + reg);
155 else
156 writel(val, pp->dbi_base + reg);
157 }
158
159 static inline u32 dw_pcie_readl_unroll(struct pcie_port *pp, u32 index, u32 reg)
160 {
161 u32 offset = PCIE_GET_ATU_OUTB_UNR_REG_OFFSET(index);
162
163 if (pp->ops->readl_rc)
164 return pp->ops->readl_rc(pp, pp->dbi_base + offset + reg);
165
166 return readl(pp->dbi_base + offset + reg);
167 }
168
169 static inline void dw_pcie_writel_unroll(struct pcie_port *pp, u32 index,
170 u32 val, u32 reg)
171 {
172 u32 offset = PCIE_GET_ATU_OUTB_UNR_REG_OFFSET(index);
173
174 if (pp->ops->writel_rc)
175 pp->ops->writel_rc(pp, val, pp->dbi_base + offset + reg);
176 else
177 writel(val, pp->dbi_base + offset + reg);
178 }
179
180 static int dw_pcie_rd_own_conf(struct pcie_port *pp, int where, int size,
181 u32 *val)
182 {
183 if (pp->ops->rd_own_conf)
184 return pp->ops->rd_own_conf(pp, where, size, val);
185
186 return dw_pcie_cfg_read(pp->dbi_base + where, size, val);
187 }
188
189 static int dw_pcie_wr_own_conf(struct pcie_port *pp, int where, int size,
190 u32 val)
191 {
192 if (pp->ops->wr_own_conf)
193 return pp->ops->wr_own_conf(pp, where, size, val);
194
195 return dw_pcie_cfg_write(pp->dbi_base + where, size, val);
196 }
197
198 static void dw_pcie_prog_outbound_atu(struct pcie_port *pp, int index,
199 int type, u64 cpu_addr, u64 pci_addr, u32 size)
200 {
201 u32 retries, val;
202
203 if (pp->iatu_unroll_enabled) {
204 dw_pcie_writel_unroll(pp, index,
205 lower_32_bits(cpu_addr), PCIE_ATU_UNR_LOWER_BASE);
206 dw_pcie_writel_unroll(pp, index,
207 upper_32_bits(cpu_addr), PCIE_ATU_UNR_UPPER_BASE);
208 dw_pcie_writel_unroll(pp, index,
209 lower_32_bits(cpu_addr + size - 1), PCIE_ATU_UNR_LIMIT);
210 dw_pcie_writel_unroll(pp, index,
211 lower_32_bits(pci_addr), PCIE_ATU_UNR_LOWER_TARGET);
212 dw_pcie_writel_unroll(pp, index,
213 upper_32_bits(pci_addr), PCIE_ATU_UNR_UPPER_TARGET);
214 dw_pcie_writel_unroll(pp, index,
215 type, PCIE_ATU_UNR_REGION_CTRL1);
216 dw_pcie_writel_unroll(pp, index,
217 PCIE_ATU_ENABLE, PCIE_ATU_UNR_REGION_CTRL2);
218 } else {
219 dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | index,
220 PCIE_ATU_VIEWPORT);
221 dw_pcie_writel_rc(pp, lower_32_bits(cpu_addr),
222 PCIE_ATU_LOWER_BASE);
223 dw_pcie_writel_rc(pp, upper_32_bits(cpu_addr),
224 PCIE_ATU_UPPER_BASE);
225 dw_pcie_writel_rc(pp, lower_32_bits(cpu_addr + size - 1),
226 PCIE_ATU_LIMIT);
227 dw_pcie_writel_rc(pp, lower_32_bits(pci_addr),
228 PCIE_ATU_LOWER_TARGET);
229 dw_pcie_writel_rc(pp, upper_32_bits(pci_addr),
230 PCIE_ATU_UPPER_TARGET);
231 dw_pcie_writel_rc(pp, type, PCIE_ATU_CR1);
232 dw_pcie_writel_rc(pp, PCIE_ATU_ENABLE, PCIE_ATU_CR2);
233 }
234
235 /*
236 * Make sure ATU enable takes effect before any subsequent config
237 * and I/O accesses.
238 */
239 for (retries = 0; retries < LINK_WAIT_MAX_IATU_RETRIES; retries++) {
240 if (pp->iatu_unroll_enabled)
241 val = dw_pcie_readl_unroll(pp, index,
242 PCIE_ATU_UNR_REGION_CTRL2);
243 else
244 val = dw_pcie_readl_rc(pp, PCIE_ATU_CR2);
245
246 if (val == PCIE_ATU_ENABLE)
247 return;
248
249 usleep_range(LINK_WAIT_IATU_MIN, LINK_WAIT_IATU_MAX);
250 }
251 dev_err(pp->dev, "iATU is not being enabled\n");
252 }
253
254 static struct irq_chip dw_msi_irq_chip = {
255 .name = "PCI-MSI",
256 .irq_enable = pci_msi_unmask_irq,
257 .irq_disable = pci_msi_mask_irq,
258 .irq_mask = pci_msi_mask_irq,
259 .irq_unmask = pci_msi_unmask_irq,
260 };
261
262 /* MSI int handler */
263 irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
264 {
265 unsigned long val;
266 int i, pos, irq;
267 irqreturn_t ret = IRQ_NONE;
268
269 for (i = 0; i < MAX_MSI_CTRLS; i++) {
270 dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_STATUS + i * 12, 4,
271 (u32 *)&val);
272 if (val) {
273 ret = IRQ_HANDLED;
274 pos = 0;
275 while ((pos = find_next_bit(&val, 32, pos)) != 32) {
276 irq = irq_find_mapping(pp->irq_domain,
277 i * 32 + pos);
278 dw_pcie_wr_own_conf(pp,
279 PCIE_MSI_INTR0_STATUS + i * 12,
280 4, 1 << pos);
281 generic_handle_irq(irq);
282 pos++;
283 }
284 }
285 }
286
287 return ret;
288 }
289
290 void dw_pcie_msi_init(struct pcie_port *pp)
291 {
292 u64 msi_target;
293
294 pp->msi_data = __get_free_pages(GFP_KERNEL, 0);
295 msi_target = virt_to_phys((void *)pp->msi_data);
296
297 /* program the msi_data */
298 dw_pcie_wr_own_conf(pp, PCIE_MSI_ADDR_LO, 4,
299 (u32)(msi_target & 0xffffffff));
300 dw_pcie_wr_own_conf(pp, PCIE_MSI_ADDR_HI, 4,
301 (u32)(msi_target >> 32 & 0xffffffff));
302 }
303
304 static void dw_pcie_msi_clear_irq(struct pcie_port *pp, int irq)
305 {
306 unsigned int res, bit, val;
307
308 res = (irq / 32) * 12;
309 bit = irq % 32;
310 dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4, &val);
311 val &= ~(1 << bit);
312 dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4, val);
313 }
314
315 static void clear_irq_range(struct pcie_port *pp, unsigned int irq_base,
316 unsigned int nvec, unsigned int pos)
317 {
318 unsigned int i;
319
320 for (i = 0; i < nvec; i++) {
321 irq_set_msi_desc_off(irq_base, i, NULL);
322 /* Disable corresponding interrupt on MSI controller */
323 if (pp->ops->msi_clear_irq)
324 pp->ops->msi_clear_irq(pp, pos + i);
325 else
326 dw_pcie_msi_clear_irq(pp, pos + i);
327 }
328
329 bitmap_release_region(pp->msi_irq_in_use, pos, order_base_2(nvec));
330 }
331
332 static void dw_pcie_msi_set_irq(struct pcie_port *pp, int irq)
333 {
334 unsigned int res, bit, val;
335
336 res = (irq / 32) * 12;
337 bit = irq % 32;
338 dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4, &val);
339 val |= 1 << bit;
340 dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4, val);
341 }
342
343 static int assign_irq(int no_irqs, struct msi_desc *desc, int *pos)
344 {
345 int irq, pos0, i;
346 struct pcie_port *pp = (struct pcie_port *) msi_desc_to_pci_sysdata(desc);

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Tim Harvey
2016-11-16 00:30:02 UTC
Permalink
Post by Bjorn Helgaas
Add support for the new iATU Unroll mechanism that will be used from Core
version 4.80. The new Cores can support either iATU Unroll or the "old"
iATU method, now called Legacy Mode. The driver is perfectly capable of
performing well for both.
[bhelgaas: split ATU enable timeout to separate patch]
---
drivers/pci/host/pcie-designware.c | 97 ++++++++++++++++++++++++++++++++----
drivers/pci/host/pcie-designware.h | 1
2 files changed, 87 insertions(+), 11 deletions(-)
diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 947fac3..e489b18 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -80,6 +80,21 @@
#define PCIE_ATU_FUNC(x) (((x) & 0x7) << 16)
#define PCIE_ATU_UPPER_TARGET 0x91C
+/*
+ * iATU Unroll-specific register definitions
+ * From 4.80 core version the address translation will be made by unroll
+ */
+#define PCIE_ATU_UNR_REGION_CTRL1 0x00
+#define PCIE_ATU_UNR_REGION_CTRL2 0x04
+#define PCIE_ATU_UNR_LOWER_BASE 0x08
+#define PCIE_ATU_UNR_UPPER_BASE 0x0C
+#define PCIE_ATU_UNR_LIMIT 0x10
+#define PCIE_ATU_UNR_LOWER_TARGET 0x14
+#define PCIE_ATU_UNR_UPPER_TARGET 0x18
+
+/* Register address builder */
+#define PCIE_GET_ATU_OUTB_UNR_REG_OFFSET(region) ((0x3 << 20) | (region << 9)
+
/* PCIe Port Logic registers */
#define PLR_OFFSET 0x700
#define PCIE_PHY_DEBUG_R1 (PLR_OFFSET + 0x2c)
@@ -141,6 +156,27 @@ static inline void dw_pcie_writel_rc(struct pcie_port *pp, u32 val, u32 reg)
writel(val, pp->dbi_base + reg);
}
+static inline u32 dw_pcie_readl_unroll(struct pcie_port *pp, u32 index, u32 reg)
+{
+ u32 offset = PCIE_GET_ATU_OUTB_UNR_REG_OFFSET(index);
+
+ if (pp->ops->readl_rc)
+ return pp->ops->readl_rc(pp, pp->dbi_base + offset + reg);
+
+ return readl(pp->dbi_base + offset + reg);
+}
+
+static inline void dw_pcie_writel_unroll(struct pcie_port *pp, u32 index,
+ u32 val, u32 reg)
+{
+ u32 offset = PCIE_GET_ATU_OUTB_UNR_REG_OFFSET(index);
+
+ if (pp->ops->writel_rc)
+ pp->ops->writel_rc(pp, val, pp->dbi_base + offset + reg);
+ else
+ writel(val, pp->dbi_base + offset + reg);
+}
+
static int dw_pcie_rd_own_conf(struct pcie_port *pp, int where, int size,
u32 *val)
{
@@ -164,23 +200,49 @@ static void dw_pcie_prog_outbound_atu(struct pcie_port *pp, int index,
{
u32 retries, val;
- dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | index,
- PCIE_ATU_VIEWPORT);
- dw_pcie_writel_rc(pp, lower_32_bits(cpu_addr), PCIE_ATU_LOWER_BASE);
- dw_pcie_writel_rc(pp, upper_32_bits(cpu_addr), PCIE_ATU_UPPER_BASE);
- dw_pcie_writel_rc(pp, lower_32_bits(cpu_addr + size - 1),
- PCIE_ATU_LIMIT);
- dw_pcie_writel_rc(pp, lower_32_bits(pci_addr), PCIE_ATU_LOWER_TARGET);
- dw_pcie_writel_rc(pp, upper_32_bits(pci_addr), PCIE_ATU_UPPER_TARGET);
- dw_pcie_writel_rc(pp, type, PCIE_ATU_CR1);
- dw_pcie_writel_rc(pp, PCIE_ATU_ENABLE, PCIE_ATU_CR2);
+ if (pp->iatu_unroll_enabled) {
+ dw_pcie_writel_unroll(pp, index,
+ lower_32_bits(cpu_addr), PCIE_ATU_UNR_LOWER_BASE);
+ dw_pcie_writel_unroll(pp, index,
+ upper_32_bits(cpu_addr), PCIE_ATU_UNR_UPPER_BASE);
+ dw_pcie_writel_unroll(pp, index,
+ lower_32_bits(cpu_addr + size - 1), PCIE_ATU_UNR_LIMIT);
+ dw_pcie_writel_unroll(pp, index,
+ lower_32_bits(pci_addr), PCIE_ATU_UNR_LOWER_TARGET);
+ dw_pcie_writel_unroll(pp, index,
+ upper_32_bits(pci_addr), PCIE_ATU_UNR_UPPER_TARGET);
+ dw_pcie_writel_unroll(pp, index,
+ type, PCIE_ATU_UNR_REGION_CTRL1);
+ dw_pcie_writel_unroll(pp, index,
+ PCIE_ATU_ENABLE, PCIE_ATU_UNR_REGION_CTRL2);
+ } else {
+ dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | index,
+ PCIE_ATU_VIEWPORT);
+ dw_pcie_writel_rc(pp, lower_32_bits(cpu_addr),
+ PCIE_ATU_LOWER_BASE);
+ dw_pcie_writel_rc(pp, upper_32_bits(cpu_addr),
+ PCIE_ATU_UPPER_BASE);
+ dw_pcie_writel_rc(pp, lower_32_bits(cpu_addr + size - 1),
+ PCIE_ATU_LIMIT);
+ dw_pcie_writel_rc(pp, lower_32_bits(pci_addr),
+ PCIE_ATU_LOWER_TARGET);
+ dw_pcie_writel_rc(pp, upper_32_bits(pci_addr),
+ PCIE_ATU_UPPER_TARGET);
+ dw_pcie_writel_rc(pp, type, PCIE_ATU_CR1);
+ dw_pcie_writel_rc(pp, PCIE_ATU_ENABLE, PCIE_ATU_CR2);
+ }
/*
* Make sure ATU enable takes effect before any subsequent config
* and I/O accesses.
*/
for (retries = 0; retries < LINK_WAIT_MAX_IATU_RETRIES; retries++) {
- val = dw_pcie_readl_rc(pp, PCIE_ATU_CR2);
+ if (pp->iatu_unroll_enabled)
+ val = dw_pcie_readl_unroll(pp, index,
+ PCIE_ATU_UNR_REGION_CTRL2);
+ else
+ val = dw_pcie_readl_rc(pp, PCIE_ATU_CR2);
+
if (val == PCIE_ATU_ENABLE)
return;
@@ -445,6 +507,17 @@ static const struct irq_domain_ops msi_domain_ops = {
.map = dw_pcie_msi_map,
};
+static u8 dw_pcie_iatu_unroll_enabled(struct pcie_port *pp)
+{
+ u32 val = 0;
+
+ dw_pcie_readl_rc(pp, PCIE_ATU_VIEWPORT, &val);
+ if (val == 0xffffffff)
+ return 1;
+
+ return 0;
+}
+
int dw_pcie_host_init(struct pcie_port *pp)
{
struct device_node *np = pp->dev->of_node;
@@ -561,6 +634,8 @@ int dw_pcie_host_init(struct pcie_port *pp)
}
}
+ pp->iatu_unroll_enabled = dw_pcie_iatu_unroll_enabled(pp);
+
if (pp->ops->host_init)
pp->ops->host_init(pp);
diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
index 285e1ed..e74cc4a 100644
--- a/drivers/pci/host/pcie-designware.h
+++ b/drivers/pci/host/pcie-designware.h
@@ -48,6 +48,7 @@ struct pcie_port {
int msi_irq;
struct irq_domain *irq_domain;
unsigned long msi_data;
+ u8 iatu_unroll_enabled;
DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS);
};
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
More majordomo info at http://vger.kernel.org/majordomo-info.html
Bjorn / Joao,

This patch (from commit a0601a47053714eecec726aea5ebcd829f817497) is
causing a kernel regression in 4.9. I can no longer boot the kernel on
boards that have a PCIe switch downstream from the IMX6 (ie many of
the Gateworks Ventana boards) unless PCI is enabled in the bootloader
(which is not recommended as that causes PCI link in the kernel to
occasionally fail).

Regards,

Tim
Fabio Estevam
2016-11-16 00:40:02 UTC
Permalink
Hi Tim,
Post by Tim Harvey
Bjorn / Joao,
This patch (from commit a0601a47053714eecec726aea5ebcd829f817497) is
causing a kernel regression in 4.9. I can no longer boot the kernel on
boards that have a PCIe switch downstream from the IMX6 (ie many of
the Gateworks Ventana boards) unless PCI is enabled in the bootloader
(which is not recommended as that causes PCI link in the kernel to
occasionally fail).
This has been fixed in 4.9-rc4 by the following commit:

commit 416379f9ebded501eda882e6af0a7aafc1866700
Author: Niklas Cassel <***@axis.com>
Date: Fri Oct 14 23:54:55 2016 +0200

PCI: designware: Check for iATU unroll support after initializing host

dw_pcie_iatu_unroll_enabled() reads a dbi_base register. Reading any
dbi_base register before pp->ops->host_init has been called causes
"imprecise external abort" on platforms like ARTPEC-6, where the PCIe
module is disabled at boot and first enabled in pp->ops->host_init. Move
dw_pcie_iatu_unroll_enabled() to dw_pcie_setup_rc(), since it is after
pp->ops->host_init, but before pp->iatu_unroll_enabled is actually used.

Fixes: a0601a470537 ("PCI: designware: Add iATU Unroll feature")
Tested-by: James Le Cuirot <***@gentoo.org>
Signed-off-by: Niklas Cassel <***@axis.com>
Signed-off-by: Bjorn Helgaas <***@google.com>
Acked-by: Joao Pinto <***@synopsys.com>
Acked-by: Olof Johansson <***@lixom.net>
Tim Harvey
2016-11-16 16:20:01 UTC
Permalink
Post by Fabio Estevam
Hi Tim,
Post by Tim Harvey
Bjorn / Joao,
This patch (from commit a0601a47053714eecec726aea5ebcd829f817497) is
causing a kernel regression in 4.9. I can no longer boot the kernel on
boards that have a PCIe switch downstream from the IMX6 (ie many of
the Gateworks Ventana boards) unless PCI is enabled in the bootloader
(which is not recommended as that causes PCI link in the kernel to
occasionally fail).
commit 416379f9ebded501eda882e6af0a7aafc1866700
Date: Fri Oct 14 23:54:55 2016 +0200
PCI: designware: Check for iATU unroll support after initializing host
dw_pcie_iatu_unroll_enabled() reads a dbi_base register. Reading any
dbi_base register before pp->ops->host_init has been called causes
"imprecise external abort" on platforms like ARTPEC-6, where the PCIe
module is disabled at boot and first enabled in pp->ops->host_init. Move
dw_pcie_iatu_unroll_enabled() to dw_pcie_setup_rc(), since it is after
pp->ops->host_init, but before pp->iatu_unroll_enabled is actually used.
Fixes: a0601a470537 ("PCI: designware: Add iATU Unroll feature")
Fabio,

That does indeed resolve the issue!

Thanks,

Tim

Bjorn Helgaas
2016-08-17 20:10:03 UTC
Permalink
dw_pcie_readl_rc() reads a u32 value. Previously we stored that value in
space supplied by the caller. Return the u32 value directly instead.

This makes the calling code read better and makes it obvious that the
caller need not initialize the storage. In the following example it isn't
clear whether "val" is initialized before being used:

dw_pcie_readl_rc(pp, PCI_COMMAND, &val);
if (val & PCI_COMMAND_MEMORY)
...

No functional change intended.

Signed-off-by: Bjorn Helgaas <***@google.com>
---
drivers/pci/host/pci-exynos.c | 9 ++++++---
drivers/pci/host/pcie-designware.c | 20 ++++++++++----------
drivers/pci/host/pcie-designware.h | 3 +--
3 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/drivers/pci/host/pci-exynos.c b/drivers/pci/host/pci-exynos.c
index 2199761..490f8b6 100644
--- a/drivers/pci/host/pci-exynos.c
+++ b/drivers/pci/host/pci-exynos.c
@@ -425,12 +425,15 @@ static void exynos_pcie_enable_interrupts(struct pcie_port *pp)
exynos_pcie_msi_init(pp);
}

-static inline void exynos_pcie_readl_rc(struct pcie_port *pp,
- void __iomem *dbi_base, u32 *val)
+static inline u32 exynos_pcie_readl_rc(struct pcie_port *pp,
+ void __iomem *dbi_base)
{
+ u32 val;
+
exynos_pcie_sideband_dbi_r_mode(pp, true);
- *val = readl(dbi_base);
+ val = readl(dbi_base);
exynos_pcie_sideband_dbi_r_mode(pp, false);
+ return val;
}

static inline void exynos_pcie_writel_rc(struct pcie_port *pp,
diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 12afce1..1f6bd6d 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -115,12 +115,12 @@ int dw_pcie_cfg_write(void __iomem *addr, int size, u32 val)
return PCIBIOS_SUCCESSFUL;
}

-static inline void dw_pcie_readl_rc(struct pcie_port *pp, u32 reg, u32 *val)
+static inline u32 dw_pcie_readl_rc(struct pcie_port *pp, u32 reg)
{
if (pp->ops->readl_rc)
- pp->ops->readl_rc(pp, pp->dbi_base + reg, val);
- else
- *val = readl(pp->dbi_base + reg);
+ return pp->ops->readl_rc(pp, pp->dbi_base + reg);
+
+ return readl(pp->dbi_base + reg);
}

static inline void dw_pcie_writel_rc(struct pcie_port *pp, u32 val, u32 reg)
@@ -169,7 +169,7 @@ static void dw_pcie_prog_outbound_atu(struct pcie_port *pp, int index,
* Make sure ATU enable takes effect before any subsequent config
* and I/O accesses.
*/
- dw_pcie_readl_rc(pp, PCIE_ATU_CR2, &val);
+ val = dw_pcie_readl_rc(pp, PCIE_ATU_CR2);
}

static struct irq_chip dw_msi_irq_chip = {
@@ -720,7 +720,7 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
u32 val;

/* set the number of lanes */
- dw_pcie_readl_rc(pp, PCIE_PORT_LINK_CONTROL, &val);
+ val = dw_pcie_readl_rc(pp, PCIE_PORT_LINK_CONTROL);
val &= ~PORT_LINK_MODE_MASK;
switch (pp->lanes) {
case 1:
@@ -742,7 +742,7 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
dw_pcie_writel_rc(pp, val, PCIE_PORT_LINK_CONTROL);

/* set link width speed control register */
- dw_pcie_readl_rc(pp, PCIE_LINK_WIDTH_SPEED_CONTROL, &val);
+ val = dw_pcie_readl_rc(pp, PCIE_LINK_WIDTH_SPEED_CONTROL);
val &= ~PORT_LOGIC_LINK_WIDTH_MASK;
switch (pp->lanes) {
case 1:
@@ -765,19 +765,19 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
dw_pcie_writel_rc(pp, 0x00000000, PCI_BASE_ADDRESS_1);

/* setup interrupt pins */
- dw_pcie_readl_rc(pp, PCI_INTERRUPT_LINE, &val);
+ val = dw_pcie_readl_rc(pp, PCI_INTERRUPT_LINE);
val &= 0xffff00ff;
val |= 0x00000100;
dw_pcie_writel_rc(pp, val, PCI_INTERRUPT_LINE);

/* setup bus numbers */
- dw_pcie_readl_rc(pp, PCI_PRIMARY_BUS, &val);
+ val = dw_pcie_readl_rc(pp, PCI_PRIMARY_BUS);
val &= 0xff000000;
val |= 0x00010100;
dw_pcie_writel_rc(pp, val, PCI_PRIMARY_BUS);

/* setup command register */
- dw_pcie_readl_rc(pp, PCI_COMMAND, &val);
+ val = dw_pcie_readl_rc(pp, PCI_COMMAND);
val &= 0xffff0000;
val |= PCI_COMMAND_IO | PCI_COMMAND_MEMORY |
PCI_COMMAND_MASTER | PCI_COMMAND_SERR;
diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
index f437f9b..74a8fc6 100644
--- a/drivers/pci/host/pcie-designware.h
+++ b/drivers/pci/host/pcie-designware.h
@@ -57,8 +57,7 @@ struct pcie_port {
};

struct pcie_host_ops {
- void (*readl_rc)(struct pcie_port *pp,
- void __iomem *dbi_base, u32 *val);
+ u32 (*readl_rc)(struct pcie_port *pp, void __iomem *dbi_base);
void (*writel_rc)(struct pcie_port *pp,
u32 val, void __iomem *dbi_base);
int (*rd_own_conf)(struct pcie_port *pp, int where, int size, u32 *val);
Loading...