linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/18] PCI: aardvark controller changes BATCH 5
@ 2022-02-20 19:33 Marek Behún
  2022-02-20 19:33 ` [PATCH 01/18] PCI: pci-bridge-emul: Re-arrange register tests Marek Behún
                   ` (21 more replies)
  0 siblings, 22 replies; 44+ messages in thread
From: Marek Behún @ 2022-02-20 19:33 UTC (permalink / raw)
  To: Lorenzo Pieralisi, Bjorn Helgaas
  Cc: Krzysztof Wilczyński, Marc Zyngier, pali, linux-pci,
	linux-arm-kernel, Gregory CLEMENT, Marek Behún

Hello Lorenzo, Krzysztof,

here comes batch 5 of changes for Aardvark PCIe controller.

This batch
- adds support for AER
- adds support for DLLSC and hotplug interrupt
- adds support for sending slot power limit message
- adds enabling/disabling PCIe clock
- adds suspend support
- optimizes link training by adding it into separate worker
- optimizes GPIO resetting by asserting it only if it wasn't asserted
  already

Marek

Marek Behún (1):
  arm64: dts: marvell: armada-37xx: Add clock to PCIe node

Miquel Raynal (2):
  PCI: aardvark: Add clock support
  PCI: aardvark: Add suspend to RAM support

Pali Rohár (13):
  PCI: aardvark: Add support for AER registers on emulated bridge
  PCI: Add PCI_EXP_SLTCAP_*_SHIFT macros
  PCI: aardvark: Fix reporting Slot capabilities on emulated bridge
  PCI: pciehp: Enable DLLSC interrupt only if supported
  PCI: pciehp: Enable Command Completed Interrupt only if supported
  PCI: aardvark: Add support for DLLSC and hotplug interrupt
  PCI: Add PCI_EXP_SLTCTL_ASPL_DISABLE macro
  PCI: Add function for parsing `slot-power-limit-milliwatt` DT property
  dt-bindings: PCI: aardvark: Describe slot-power-limit-milliwatt
  PCI: aardvark: Send Set_Slot_Power_Limit message
  arm64: dts: armada-3720-turris-mox: Define slot-power-limit-milliwatt
    for PCIe
  PCI: aardvark: Run link training in separate worker
  PCI: aardvark: Optimize PCIe card reset via GPIO

Russell King (2):
  PCI: pci-bridge-emul: Re-arrange register tests
  PCI: pci-bridge-emul: Add support for PCIe extended capabilities

 .../devicetree/bindings/pci/aardvark-pci.txt  |   2 +
 .../dts/marvell/armada-3720-turris-mox.dts    |   1 +
 arch/arm64/boot/dts/marvell/armada-37xx.dtsi  |   1 +
 drivers/pci/controller/pci-aardvark.c         | 380 ++++++++++++++++--
 drivers/pci/hotplug/pciehp_hpc.c              |  34 +-
 drivers/pci/hotplug/pnv_php.c                 |  13 +-
 drivers/pci/of.c                              |  64 +++
 drivers/pci/pci-bridge-emul.c                 | 130 +++---
 drivers/pci/pci-bridge-emul.h                 |  15 +
 drivers/pci/pci.h                             |  15 +
 include/uapi/linux/pci_regs.h                 |   4 +
 11 files changed, 565 insertions(+), 94 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 44+ messages in thread

* [PATCH 01/18] PCI: pci-bridge-emul: Re-arrange register tests
  2022-02-20 19:33 [PATCH 00/18] PCI: aardvark controller changes BATCH 5 Marek Behún
@ 2022-02-20 19:33 ` Marek Behún
  2022-02-20 19:33 ` [PATCH 02/18] PCI: pci-bridge-emul: Add support for PCIe extended capabilities Marek Behún
                   ` (20 subsequent siblings)
  21 siblings, 0 replies; 44+ messages in thread
From: Marek Behún @ 2022-02-20 19:33 UTC (permalink / raw)
  To: Lorenzo Pieralisi, Bjorn Helgaas
  Cc: Krzysztof Wilczyński, Marc Zyngier, pali, linux-pci,
	linux-arm-kernel, Gregory CLEMENT, Russell King,
	Marek Behún

From: Russell King <rmk+kernel@armlinux.org.uk>

Re-arrange the tests for which sets of registers are being accessed so that
it is easier to add further regions later. No functional change.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
[pali: Fix reading old value in pci_bridge_emul_conf_write]
Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Marek Behún <kabel@kernel.org>
---
 drivers/pci/pci-bridge-emul.c | 61 ++++++++++++++++++-----------------
 1 file changed, 31 insertions(+), 30 deletions(-)

diff --git a/drivers/pci/pci-bridge-emul.c b/drivers/pci/pci-bridge-emul.c
index a16f9e30099e..a956408834d6 100644
--- a/drivers/pci/pci-bridge-emul.c
+++ b/drivers/pci/pci-bridge-emul.c
@@ -422,25 +422,25 @@ int pci_bridge_emul_conf_read(struct pci_bridge_emul *bridge, int where,
 	__le32 *cfgspace;
 	const struct pci_bridge_reg_behavior *behavior;
 
-	if (bridge->has_pcie && reg >= PCI_CAP_PCIE_END) {
-		*value = 0;
-		return PCIBIOS_SUCCESSFUL;
-	}
-
-	if (!bridge->has_pcie && reg >= PCI_BRIDGE_CONF_END) {
+	if (reg < PCI_BRIDGE_CONF_END) {
+		/* Emulated PCI space */
+		read_op = bridge->ops->read_base;
+		cfgspace = (__le32 *) &bridge->conf;
+		behavior = bridge->pci_regs_behavior;
+	} else if (!bridge->has_pcie) {
+		/* PCIe space is not implemented, and no PCI capabilities */
 		*value = 0;
 		return PCIBIOS_SUCCESSFUL;
-	}
-
-	if (bridge->has_pcie && reg >= PCI_CAP_PCIE_START) {
+	} else if (reg < PCI_CAP_PCIE_END) {
+		/* Our emulated PCIe capability */
 		reg -= PCI_CAP_PCIE_START;
 		read_op = bridge->ops->read_pcie;
 		cfgspace = (__le32 *) &bridge->pcie_conf;
 		behavior = bridge->pcie_cap_regs_behavior;
 	} else {
-		read_op = bridge->ops->read_base;
-		cfgspace = (__le32 *) &bridge->conf;
-		behavior = bridge->pci_regs_behavior;
+		/* Beyond our PCIe space */
+		*value = 0;
+		return PCIBIOS_SUCCESSFUL;
 	}
 
 	if (read_op)
@@ -484,11 +484,27 @@ int pci_bridge_emul_conf_write(struct pci_bridge_emul *bridge, int where,
 	__le32 *cfgspace;
 	const struct pci_bridge_reg_behavior *behavior;
 
-	if (bridge->has_pcie && reg >= PCI_CAP_PCIE_END)
-		return PCIBIOS_SUCCESSFUL;
+	ret = pci_bridge_emul_conf_read(bridge, reg, 4, &old);
+	if (ret != PCIBIOS_SUCCESSFUL)
+		return ret;
 
-	if (!bridge->has_pcie && reg >= PCI_BRIDGE_CONF_END)
+	if (reg < PCI_BRIDGE_CONF_END) {
+		/* Emulated PCI space */
+		write_op = bridge->ops->write_base;
+		cfgspace = (__le32 *) &bridge->conf;
+		behavior = bridge->pci_regs_behavior;
+	} else if (!bridge->has_pcie) {
+		/* PCIe space is not implemented, and no PCI capabilities */
 		return PCIBIOS_SUCCESSFUL;
+	} else if (reg < PCI_CAP_PCIE_END) {
+		/* Our emulated PCIe capability */
+		reg -= PCI_CAP_PCIE_START;
+		write_op = bridge->ops->write_pcie;
+		cfgspace = (__le32 *) &bridge->pcie_conf;
+		behavior = bridge->pcie_cap_regs_behavior;
+	} else {
+		return PCIBIOS_SUCCESSFUL;
+	}
 
 	shift = (where & 0x3) * 8;
 
@@ -501,21 +517,6 @@ int pci_bridge_emul_conf_write(struct pci_bridge_emul *bridge, int where,
 	else
 		return PCIBIOS_BAD_REGISTER_NUMBER;
 
-	ret = pci_bridge_emul_conf_read(bridge, reg, 4, &old);
-	if (ret != PCIBIOS_SUCCESSFUL)
-		return ret;
-
-	if (bridge->has_pcie && reg >= PCI_CAP_PCIE_START) {
-		reg -= PCI_CAP_PCIE_START;
-		write_op = bridge->ops->write_pcie;
-		cfgspace = (__le32 *) &bridge->pcie_conf;
-		behavior = bridge->pcie_cap_regs_behavior;
-	} else {
-		write_op = bridge->ops->write_base;
-		cfgspace = (__le32 *) &bridge->conf;
-		behavior = bridge->pci_regs_behavior;
-	}
-
 	/* Keep all bits, except the RW bits */
 	new = old & (~mask | ~behavior[reg / 4].rw);
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 02/18] PCI: pci-bridge-emul: Add support for PCIe extended capabilities
  2022-02-20 19:33 [PATCH 00/18] PCI: aardvark controller changes BATCH 5 Marek Behún
  2022-02-20 19:33 ` [PATCH 01/18] PCI: pci-bridge-emul: Re-arrange register tests Marek Behún
@ 2022-02-20 19:33 ` Marek Behún
  2022-02-20 19:33 ` [PATCH 03/18] PCI: aardvark: Add support for AER registers on emulated bridge Marek Behún
                   ` (19 subsequent siblings)
  21 siblings, 0 replies; 44+ messages in thread
From: Marek Behún @ 2022-02-20 19:33 UTC (permalink / raw)
  To: Lorenzo Pieralisi, Bjorn Helgaas
  Cc: Krzysztof Wilczyński, Marc Zyngier, pali, linux-pci,
	linux-arm-kernel, Gregory CLEMENT, Russell King,
	Marek Behún

From: Russell King <rmk+kernel@armlinux.org.uk>

Add support for PCIe extended capabilities, which we just redirect to the
emulating driver.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
[pali: Fix writing new value with W1C bits]
Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Marek Behún <kabel@kernel.org>
---
 drivers/pci/pci-bridge-emul.c | 77 +++++++++++++++++++++++------------
 drivers/pci/pci-bridge-emul.h | 15 +++++++
 2 files changed, 67 insertions(+), 25 deletions(-)

diff --git a/drivers/pci/pci-bridge-emul.c b/drivers/pci/pci-bridge-emul.c
index a956408834d6..c4b9837006ff 100644
--- a/drivers/pci/pci-bridge-emul.c
+++ b/drivers/pci/pci-bridge-emul.c
@@ -437,10 +437,16 @@ int pci_bridge_emul_conf_read(struct pci_bridge_emul *bridge, int where,
 		read_op = bridge->ops->read_pcie;
 		cfgspace = (__le32 *) &bridge->pcie_conf;
 		behavior = bridge->pcie_cap_regs_behavior;
-	} else {
-		/* Beyond our PCIe space */
+	} else if (reg < PCI_CFG_SPACE_SIZE) {
+		/* Rest of PCI space not implemented */
 		*value = 0;
 		return PCIBIOS_SUCCESSFUL;
+	} else {
+		/* PCIe extended capability space */
+		reg -= PCI_CFG_SPACE_SIZE;
+		read_op = bridge->ops->read_ext;
+		cfgspace = NULL;
+		behavior = NULL;
 	}
 
 	if (read_op)
@@ -448,15 +454,20 @@ int pci_bridge_emul_conf_read(struct pci_bridge_emul *bridge, int where,
 	else
 		ret = PCI_BRIDGE_EMUL_NOT_HANDLED;
 
-	if (ret == PCI_BRIDGE_EMUL_NOT_HANDLED)
-		*value = le32_to_cpu(cfgspace[reg / 4]);
+	if (ret == PCI_BRIDGE_EMUL_NOT_HANDLED) {
+		if (cfgspace)
+			*value = le32_to_cpu(cfgspace[reg / 4]);
+		else
+			*value = 0;
+	}
 
 	/*
 	 * Make sure we never return any reserved bit with a value
 	 * different from 0.
 	 */
-	*value &= behavior[reg / 4].ro | behavior[reg / 4].rw |
-		  behavior[reg / 4].w1c;
+	if (behavior)
+		*value &= behavior[reg / 4].ro | behavior[reg / 4].rw |
+			  behavior[reg / 4].w1c;
 
 	if (size == 1)
 		*value = (*value >> (8 * (where & 3))) & 0xff;
@@ -502,8 +513,15 @@ int pci_bridge_emul_conf_write(struct pci_bridge_emul *bridge, int where,
 		write_op = bridge->ops->write_pcie;
 		cfgspace = (__le32 *) &bridge->pcie_conf;
 		behavior = bridge->pcie_cap_regs_behavior;
-	} else {
+	} else if (reg < PCI_CFG_SPACE_SIZE) {
+		/* Rest of PCI space not implemented */
 		return PCIBIOS_SUCCESSFUL;
+	} else {
+		/* PCIe extended capability space */
+		reg -= PCI_CFG_SPACE_SIZE;
+		write_op = bridge->ops->write_ext;
+		cfgspace = NULL;
+		behavior = NULL;
 	}
 
 	shift = (where & 0x3) * 8;
@@ -517,29 +535,38 @@ int pci_bridge_emul_conf_write(struct pci_bridge_emul *bridge, int where,
 	else
 		return PCIBIOS_BAD_REGISTER_NUMBER;
 
-	/* Keep all bits, except the RW bits */
-	new = old & (~mask | ~behavior[reg / 4].rw);
+	if (behavior) {
+		/* Keep all bits, except the RW bits */
+		new = old & (~mask | ~behavior[reg / 4].rw);
 
-	/* Update the value of the RW bits */
-	new |= (value << shift) & (behavior[reg / 4].rw & mask);
+		/* Update the value of the RW bits */
+		new |= (value << shift) & (behavior[reg / 4].rw & mask);
 
-	/* Clear the W1C bits */
-	new &= ~((value << shift) & (behavior[reg / 4].w1c & mask));
+		/* Clear the W1C bits */
+		new &= ~((value << shift) & (behavior[reg / 4].w1c & mask));
+	} else {
+		new = old & ~mask;
+		new |= (value << shift) & mask;
+	}
 
-	/* Save the new value with the cleared W1C bits into the cfgspace */
-	cfgspace[reg / 4] = cpu_to_le32(new);
+	if (cfgspace) {
+		/* Save the new value with the cleared W1C bits into the cfgspace */
+		cfgspace[reg / 4] = cpu_to_le32(new);
+	}
 
-	/*
-	 * Clear the W1C bits not specified by the write mask, so that the
-	 * write_op() does not clear them.
-	 */
-	new &= ~(behavior[reg / 4].w1c & ~mask);
+	if (behavior) {
+		/*
+		 * Clear the W1C bits not specified by the write mask, so that the
+		 * write_op() does not clear them.
+		 */
+		new &= ~(behavior[reg / 4].w1c & ~mask);
 
-	/*
-	 * Set the W1C bits specified by the write mask, so that write_op()
-	 * knows about that they are to be cleared.
-	 */
-	new |= (value << shift) & (behavior[reg / 4].w1c & mask);
+		/*
+		 * Set the W1C bits specified by the write mask, so that write_op()
+		 * knows about that they are to be cleared.
+		 */
+		new |= (value << shift) & (behavior[reg / 4].w1c & mask);
+	}
 
 	if (write_op)
 		write_op(bridge, reg, old, new, mask);
diff --git a/drivers/pci/pci-bridge-emul.h b/drivers/pci/pci-bridge-emul.h
index 4953274cac18..6b5f75b2ad02 100644
--- a/drivers/pci/pci-bridge-emul.h
+++ b/drivers/pci/pci-bridge-emul.h
@@ -90,6 +90,14 @@ struct pci_bridge_emul_ops {
 	 */
 	pci_bridge_emul_read_status_t (*read_pcie)(struct pci_bridge_emul *bridge,
 						   int reg, u32 *value);
+
+	/*
+	 * Same as ->read_base(), except it is for reading from the
+	 * PCIe extended capability configuration space.
+	 */
+	pci_bridge_emul_read_status_t (*read_ext)(struct pci_bridge_emul *bridge,
+						  int reg, u32 *value);
+
 	/*
 	 * Called when writing to the regular PCI bridge configuration
 	 * space. old is the current value, new is the new value being
@@ -105,6 +113,13 @@ struct pci_bridge_emul_ops {
 	 */
 	void (*write_pcie)(struct pci_bridge_emul *bridge, int reg,
 			   u32 old, u32 new, u32 mask);
+
+	/*
+	 * Same as ->write_base(), except it is for writing from the
+	 * PCIe extended capability configuration space.
+	 */
+	void (*write_ext)(struct pci_bridge_emul *bridge, int reg,
+			  u32 old, u32 new, u32 mask);
 };
 
 struct pci_bridge_reg_behavior;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 03/18] PCI: aardvark: Add support for AER registers on emulated bridge
  2022-02-20 19:33 [PATCH 00/18] PCI: aardvark controller changes BATCH 5 Marek Behún
  2022-02-20 19:33 ` [PATCH 01/18] PCI: pci-bridge-emul: Re-arrange register tests Marek Behún
  2022-02-20 19:33 ` [PATCH 02/18] PCI: pci-bridge-emul: Add support for PCIe extended capabilities Marek Behún
@ 2022-02-20 19:33 ` Marek Behún
  2022-02-20 19:33 ` [PATCH 04/18] PCI: Add PCI_EXP_SLTCAP_*_SHIFT macros Marek Behún
                   ` (18 subsequent siblings)
  21 siblings, 0 replies; 44+ messages in thread
From: Marek Behún @ 2022-02-20 19:33 UTC (permalink / raw)
  To: Lorenzo Pieralisi, Bjorn Helgaas
  Cc: Krzysztof Wilczyński, Marc Zyngier, pali, linux-pci,
	linux-arm-kernel, Gregory CLEMENT, Marek Behún

From: Pali Rohár <pali@kernel.org>

Aardvark controller supports Advanced Error Reporting configuration
registers.

Export these registers on the emulated root bridge via the new .read_ext
and .write_ext methods.

Note that in the Advanced Error Reporting Capability header the offset
to the next Extended Capability header is set, but it is not documented
in Armada 3700 Functional Specification. Since this change adds support
only for Advanced Error Reporting, explicitly clear PCI_EXT_CAP_NEXT
bits in AER capability header.

Now the pcieport driver correctly detects AER support and allows PCIe
AER driver to start receiving ERR interrupts. Kernel log now says:

    [    4.358401] pcieport 0000:00:00.0: AER: enabled with IRQ 52

Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Marek Behún <kabel@kernel.org>
---
 drivers/pci/controller/pci-aardvark.c | 77 +++++++++++++++++++++++++++
 1 file changed, 77 insertions(+)

diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
index 7621c9344a2b..01dd530e1b5f 100644
--- a/drivers/pci/controller/pci-aardvark.c
+++ b/drivers/pci/controller/pci-aardvark.c
@@ -33,6 +33,7 @@
 #define PCIE_CORE_CMD_STATUS_REG				0x4
 #define PCIE_CORE_DEV_REV_REG					0x8
 #define PCIE_CORE_PCIEXP_CAP					0xc0
+#define PCIE_CORE_PCIERR_CAP					0x100
 #define PCIE_CORE_ERR_CAPCTL_REG				0x118
 #define     PCIE_CORE_ERR_CAPCTL_ECRC_CHK_TX			BIT(5)
 #define     PCIE_CORE_ERR_CAPCTL_ECRC_CHK_TX_EN			BIT(6)
@@ -945,11 +946,87 @@ advk_pci_bridge_emul_pcie_conf_write(struct pci_bridge_emul *bridge,
 	}
 }
 
+static pci_bridge_emul_read_status_t
+advk_pci_bridge_emul_ext_conf_read(struct pci_bridge_emul *bridge,
+				   int reg, u32 *value)
+{
+	struct advk_pcie *pcie = bridge->data;
+
+	switch (reg) {
+	case 0:
+		*value = advk_readl(pcie, PCIE_CORE_PCIERR_CAP + reg);
+		/*
+		 * PCI_EXT_CAP_NEXT bits are set to offset 0x150, but Armada
+		 * 3700 Functional Specification does not document registers
+		 * at those addresses.
+		 * Thus we clear PCI_EXT_CAP_NEXT bits to make Advanced Error
+		 * Reporting Capability header the last of Extended
+		 * Capabilities. (If we obtain documentation for those
+		 * registers in the future, this can be changed.)
+		 */
+		*value &= 0x000fffff;
+		return PCI_BRIDGE_EMUL_HANDLED;
+
+	case PCI_ERR_UNCOR_STATUS:
+	case PCI_ERR_UNCOR_MASK:
+	case PCI_ERR_UNCOR_SEVER:
+	case PCI_ERR_COR_STATUS:
+	case PCI_ERR_COR_MASK:
+	case PCI_ERR_CAP:
+	case PCI_ERR_HEADER_LOG + 0:
+	case PCI_ERR_HEADER_LOG + 4:
+	case PCI_ERR_HEADER_LOG + 8:
+	case PCI_ERR_HEADER_LOG + 12:
+	case PCI_ERR_ROOT_COMMAND:
+	case PCI_ERR_ROOT_STATUS:
+	case PCI_ERR_ROOT_ERR_SRC:
+		*value = advk_readl(pcie, PCIE_CORE_PCIERR_CAP + reg);
+		return PCI_BRIDGE_EMUL_HANDLED;
+
+	default:
+		return PCI_BRIDGE_EMUL_NOT_HANDLED;
+	}
+}
+
+static void
+advk_pci_bridge_emul_ext_conf_write(struct pci_bridge_emul *bridge,
+				    int reg, u32 old, u32 new, u32 mask)
+{
+	struct advk_pcie *pcie = bridge->data;
+
+	switch (reg) {
+	/* These are W1C registers, so clear other bits */
+	case PCI_ERR_UNCOR_STATUS:
+	case PCI_ERR_COR_STATUS:
+	case PCI_ERR_ROOT_STATUS:
+		new &= mask;
+		fallthrough;
+
+	case PCI_ERR_UNCOR_MASK:
+	case PCI_ERR_UNCOR_SEVER:
+	case PCI_ERR_COR_MASK:
+	case PCI_ERR_CAP:
+	case PCI_ERR_HEADER_LOG + 0:
+	case PCI_ERR_HEADER_LOG + 4:
+	case PCI_ERR_HEADER_LOG + 8:
+	case PCI_ERR_HEADER_LOG + 12:
+	case PCI_ERR_ROOT_COMMAND:
+	case PCI_ERR_ROOT_ERR_SRC:
+		advk_writel(pcie, new, PCIE_CORE_PCIERR_CAP + reg);
+		break;
+
+	default:
+		break;
+	}
+}
+
 static const struct pci_bridge_emul_ops advk_pci_bridge_emul_ops = {
 	.read_base = advk_pci_bridge_emul_base_conf_read,
 	.write_base = advk_pci_bridge_emul_base_conf_write,
 	.read_pcie = advk_pci_bridge_emul_pcie_conf_read,
 	.write_pcie = advk_pci_bridge_emul_pcie_conf_write,
+	.read_ext = advk_pci_bridge_emul_ext_conf_read,
+	.write_ext = advk_pci_bridge_emul_ext_conf_write,
 };
 
 /*
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 04/18] PCI: Add PCI_EXP_SLTCAP_*_SHIFT macros
  2022-02-20 19:33 [PATCH 00/18] PCI: aardvark controller changes BATCH 5 Marek Behún
                   ` (2 preceding siblings ...)
  2022-02-20 19:33 ` [PATCH 03/18] PCI: aardvark: Add support for AER registers on emulated bridge Marek Behún
@ 2022-02-20 19:33 ` Marek Behún
  2022-04-28 11:09   ` Lorenzo Pieralisi
  2022-05-18 19:23   ` Bjorn Helgaas
  2022-02-20 19:33 ` [PATCH 05/18] PCI: aardvark: Fix reporting Slot capabilities on emulated bridge Marek Behún
                   ` (17 subsequent siblings)
  21 siblings, 2 replies; 44+ messages in thread
From: Marek Behún @ 2022-02-20 19:33 UTC (permalink / raw)
  To: Lorenzo Pieralisi, Bjorn Helgaas
  Cc: Krzysztof Wilczyński, Marc Zyngier, pali, linux-pci,
	linux-arm-kernel, Gregory CLEMENT, Marek Behún

From: Pali Rohár <pali@kernel.org>

These macros allows to easily compose and extract Slot Power Limit and
Physical Slot Number values from Slot Capability Register.

Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Marek Behún <kabel@kernel.org>
---
 include/uapi/linux/pci_regs.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index bee1a9ed6e66..d825e17e448c 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -591,10 +591,13 @@
 #define  PCI_EXP_SLTCAP_HPS	0x00000020 /* Hot-Plug Surprise */
 #define  PCI_EXP_SLTCAP_HPC	0x00000040 /* Hot-Plug Capable */
 #define  PCI_EXP_SLTCAP_SPLV	0x00007f80 /* Slot Power Limit Value */
+#define  PCI_EXP_SLTCAP_SPLV_SHIFT	7  /* Slot Power Limit Value shift */
 #define  PCI_EXP_SLTCAP_SPLS	0x00018000 /* Slot Power Limit Scale */
+#define  PCI_EXP_SLTCAP_SPLS_SHIFT	15 /* Slot Power Limit Scale shift */
 #define  PCI_EXP_SLTCAP_EIP	0x00020000 /* Electromechanical Interlock Present */
 #define  PCI_EXP_SLTCAP_NCCS	0x00040000 /* No Command Completed Support */
 #define  PCI_EXP_SLTCAP_PSN	0xfff80000 /* Physical Slot Number */
+#define  PCI_EXP_SLTCAP_PSN_SHIFT	19 /* Physical Slot Number shift */
 #define PCI_EXP_SLTCTL		0x18	/* Slot Control */
 #define  PCI_EXP_SLTCTL_ABPE	0x0001	/* Attention Button Pressed Enable */
 #define  PCI_EXP_SLTCTL_PFDE	0x0002	/* Power Fault Detected Enable */
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 05/18] PCI: aardvark: Fix reporting Slot capabilities on emulated bridge
  2022-02-20 19:33 [PATCH 00/18] PCI: aardvark controller changes BATCH 5 Marek Behún
                   ` (3 preceding siblings ...)
  2022-02-20 19:33 ` [PATCH 04/18] PCI: Add PCI_EXP_SLTCAP_*_SHIFT macros Marek Behún
@ 2022-02-20 19:33 ` Marek Behún
  2022-02-20 19:33 ` [PATCH 06/18] PCI: pciehp: Enable DLLSC interrupt only if supported Marek Behún
                   ` (16 subsequent siblings)
  21 siblings, 0 replies; 44+ messages in thread
From: Marek Behún @ 2022-02-20 19:33 UTC (permalink / raw)
  To: Lorenzo Pieralisi, Bjorn Helgaas
  Cc: Krzysztof Wilczyński, Marc Zyngier, pali, linux-pci,
	linux-arm-kernel, Gregory CLEMENT, Marek Behún

From: Pali Rohár <pali@kernel.org>

Slot capabilities are currently not reported because emulated bridge
does not report the PCI_EXP_FLAGS_SLOT flag.

Set PCI_EXP_FLAGS_SLOT to let the kernel know that PCI_EXP_SLT*
registers are supported.

Move setting of PCI_EXP_SLTCTL register from "dynamic" pcie_conf_read
function to static buffer as it is only statically filled the
PCI_EXP_SLTSTA_PDS flag and dynamic read callback is not needed for this
register.

Set Presence State Bit to 1 since there is no support for unplugging the
card and there is currently no platform able to detect presence of
a card - in such a case the bit needs to be set to 1.

Finally correctly set Physical Slot Number to 1 since there is only one
port and zero value is reserved for ports within the same silicon as
Root Port which is not our case for Aardvark HW.

Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Marek Behún <kabel@kernel.org>
---
 drivers/pci/controller/pci-aardvark.c | 31 +++++++++++++++++++--------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
index 01dd530e1b5f..c80c78505bfa 100644
--- a/drivers/pci/controller/pci-aardvark.c
+++ b/drivers/pci/controller/pci-aardvark.c
@@ -859,14 +859,11 @@ advk_pci_bridge_emul_pcie_conf_read(struct pci_bridge_emul *bridge,
 
 
 	switch (reg) {
-	case PCI_EXP_SLTCTL:
-		*value = PCI_EXP_SLTSTA_PDS << 16;
-		return PCI_BRIDGE_EMUL_HANDLED;
-
 	/*
-	 * PCI_EXP_RTCTL and PCI_EXP_RTSTA are also supported, but do not need
-	 * to be handled here, because their values are stored in emulated
-	 * config space buffer, and we read them from there when needed.
+	 * PCI_EXP_SLTCAP, PCI_EXP_SLTCTL, PCI_EXP_RTCTL and PCI_EXP_RTSTA are
+	 * also supported, but do not need to be handled here, because their
+	 * values are stored in emulated config space buffer, and we read them
+	 * from there when needed.
 	 */
 
 	case PCI_EXP_LNKCAP: {
@@ -1055,8 +1052,24 @@ static int advk_sw_pci_bridge_init(struct advk_pcie *pcie)
 	/* Support interrupt A for MSI feature */
 	bridge->conf.intpin = PCI_INTERRUPT_INTA;
 
-	/* Aardvark HW provides PCIe Capability structure in version 2 */
-	bridge->pcie_conf.cap = cpu_to_le16(2);
+	/*
+	 * Aardvark HW provides PCIe Capability structure in version 2 and
+	 * indicate slot support, which is emulated.
+	 */
+	bridge->pcie_conf.cap = cpu_to_le16(2 | PCI_EXP_FLAGS_SLOT);
+
+	/*
+	 * Set Presence Detect State bit permanently since there is no support
+	 * for unplugging the card nor detecting whether it is plugged. (If a
+	 * platform exists in the future that supports it, via a GPIO for
+	 * example, it should be implemented via this bit.)
+	 *
+	 * Set physical slot number to 1 since there is only one port and zero
+	 * value is reserved for ports within the same silicon as Root Port
+	 * which is not our case.
+	 */
+	bridge->pcie_conf.slotcap = cpu_to_le32(1 << PCI_EXP_SLTCAP_PSN_SHIFT);
+	bridge->pcie_conf.slotsta = cpu_to_le16(PCI_EXP_SLTSTA_PDS);
 
 	/* Indicates supports for Completion Retry Status */
 	bridge->pcie_conf.rootcap = cpu_to_le16(PCI_EXP_RTCAP_CRSVIS);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 06/18] PCI: pciehp: Enable DLLSC interrupt only if supported
  2022-02-20 19:33 [PATCH 00/18] PCI: aardvark controller changes BATCH 5 Marek Behún
                   ` (4 preceding siblings ...)
  2022-02-20 19:33 ` [PATCH 05/18] PCI: aardvark: Fix reporting Slot capabilities on emulated bridge Marek Behún
@ 2022-02-20 19:33 ` Marek Behún
  2022-05-09  3:42   ` Lukas Wunner
  2022-02-20 19:33 ` [PATCH 07/18] PCI: pciehp: Enable Command Completed Interrupt " Marek Behún
                   ` (15 subsequent siblings)
  21 siblings, 1 reply; 44+ messages in thread
From: Marek Behún @ 2022-02-20 19:33 UTC (permalink / raw)
  To: Lorenzo Pieralisi, Bjorn Helgaas
  Cc: Krzysztof Wilczyński, Marc Zyngier, pali, linux-pci,
	linux-arm-kernel, Gregory CLEMENT, Marek Behún

From: Pali Rohár <pali@kernel.org>

Don't enable Data Link Layer State Changed interrupt if it isn't
supported.

Data Link Layer Link Active Reporting Capable bit in Link Capabilities
register indicates if Data Link Layer State Changed Enable is supported.

Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Marek Behún <kabel@kernel.org>
---
 drivers/pci/hotplug/pciehp_hpc.c | 30 +++++++++++++++++++++++-------
 drivers/pci/hotplug/pnv_php.c    | 13 +++++++++----
 2 files changed, 32 insertions(+), 11 deletions(-)

diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
index 040ae076ec0e..373bb396fe22 100644
--- a/drivers/pci/hotplug/pciehp_hpc.c
+++ b/drivers/pci/hotplug/pciehp_hpc.c
@@ -788,6 +788,7 @@ static int pciehp_poll(void *data)
 static void pcie_enable_notification(struct controller *ctrl)
 {
 	u16 cmd, mask;
+	u32 link_cap;
 
 	/*
 	 * TBD: Power fault detected software notification support.
@@ -800,12 +801,17 @@ static void pcie_enable_notification(struct controller *ctrl)
 	 * next power fault detected interrupt was notified again.
 	 */
 
+	pcie_capability_read_dword(ctrl_dev(ctrl), PCI_EXP_LNKCAP, &link_cap);
+
 	/*
-	 * Always enable link events: thus link-up and link-down shall
-	 * always be treated as hotplug and unplug respectively. Enable
-	 * presence detect only if Attention Button is not present.
+	 * Enable link events if their support is indicated in Link Capability
+	 * register: thus link-up and link-down shall always be treated as
+	 * hotplug and unplug respectively. Enable presence detect only if
+	 * Attention Button is not present.
 	 */
-	cmd = PCI_EXP_SLTCTL_DLLSCE;
+	cmd = 0;
+	if (link_cap & PCI_EXP_LNKCAP_DLLLARC)
+		cmd |= PCI_EXP_SLTCTL_DLLSCE;
 	if (ATTN_BUTTN(ctrl))
 		cmd |= PCI_EXP_SLTCTL_ABPE;
 	else
@@ -844,9 +850,14 @@ void pcie_clear_hotplug_events(struct controller *ctrl)
 
 void pcie_enable_interrupt(struct controller *ctrl)
 {
+	u32 link_cap;
 	u16 mask;
 
-	mask = PCI_EXP_SLTCTL_HPIE | PCI_EXP_SLTCTL_DLLSCE;
+	pcie_capability_read_dword(ctrl_dev(ctrl), PCI_EXP_LNKCAP, &link_cap);
+
+	mask = PCI_EXP_SLTCTL_HPIE;
+	if (link_cap & PCI_EXP_LNKCAP_DLLLARC)
+		mask |= PCI_EXP_SLTCTL_DLLSCE;
 	pcie_write_cmd(ctrl, mask, mask);
 }
 
@@ -904,19 +915,24 @@ int pciehp_reset_slot(struct hotplug_slot *hotplug_slot, bool probe)
 	struct controller *ctrl = to_ctrl(hotplug_slot);
 	struct pci_dev *pdev = ctrl_dev(ctrl);
 	u16 stat_mask = 0, ctrl_mask = 0;
+	u32 link_cap;
 	int rc;
 
 	if (probe)
 		return 0;
 
+	pcie_capability_read_dword(pdev, PCI_EXP_LNKCAP, &link_cap);
+
 	down_write_nested(&ctrl->reset_lock, ctrl->depth);
 
 	if (!ATTN_BUTTN(ctrl)) {
 		ctrl_mask |= PCI_EXP_SLTCTL_PDCE;
 		stat_mask |= PCI_EXP_SLTSTA_PDC;
 	}
-	ctrl_mask |= PCI_EXP_SLTCTL_DLLSCE;
-	stat_mask |= PCI_EXP_SLTSTA_DLLSC;
+	if (link_cap & PCI_EXP_LNKCAP_DLLLARC) {
+		ctrl_mask |= PCI_EXP_SLTCTL_DLLSCE;
+		stat_mask |= PCI_EXP_SLTSTA_DLLSC;
+	}
 
 	pcie_write_cmd(ctrl, 0, ctrl_mask);
 	ctrl_dbg(ctrl, "%s: SLOTCTRL %x write cmd %x\n", __func__,
diff --git a/drivers/pci/hotplug/pnv_php.c b/drivers/pci/hotplug/pnv_php.c
index f4c2e6e01be0..e05e8460eb2c 100644
--- a/drivers/pci/hotplug/pnv_php.c
+++ b/drivers/pci/hotplug/pnv_php.c
@@ -840,6 +840,7 @@ static void pnv_php_init_irq(struct pnv_php_slot *php_slot, int irq)
 	struct pci_dev *pdev = php_slot->pdev;
 	u32 broken_pdc = 0;
 	u16 sts, ctrl;
+	u32 link_cap;
 	int ret;
 
 	/* Allocate workqueue */
@@ -873,17 +874,21 @@ static void pnv_php_init_irq(struct pnv_php_slot *php_slot, int irq)
 		return;
 	}
 
+	pcie_capability_read_dword(pdev, PCI_EXP_LNKCAP, &link_cap);
+
 	/* Enable the interrupts */
 	pcie_capability_read_word(pdev, PCI_EXP_SLTCTL, &ctrl);
 	if (php_slot->flags & PNV_PHP_FLAG_BROKEN_PDC) {
 		ctrl &= ~PCI_EXP_SLTCTL_PDCE;
-		ctrl |= (PCI_EXP_SLTCTL_HPIE |
-			 PCI_EXP_SLTCTL_DLLSCE);
+		ctrl |= PCI_EXP_SLTCTL_HPIE;
 	} else {
 		ctrl |= (PCI_EXP_SLTCTL_HPIE |
-			 PCI_EXP_SLTCTL_PDCE |
-			 PCI_EXP_SLTCTL_DLLSCE);
+			 PCI_EXP_SLTCTL_PDCE);
 	}
+	if (link_cap & PCI_EXP_LNKCAP_DLLLARC)
+		ctrl |= PCI_EXP_SLTCTL_DLLSCE;
+	else
+		ctrl &= ~PCI_EXP_SLTCTL_DLLSCE;
 	pcie_capability_write_word(pdev, PCI_EXP_SLTCTL, ctrl);
 
 	/* The interrupt is initialized successfully when @irq is valid */
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 07/18] PCI: pciehp: Enable Command Completed Interrupt only if supported
  2022-02-20 19:33 [PATCH 00/18] PCI: aardvark controller changes BATCH 5 Marek Behún
                   ` (5 preceding siblings ...)
  2022-02-20 19:33 ` [PATCH 06/18] PCI: pciehp: Enable DLLSC interrupt only if supported Marek Behún
@ 2022-02-20 19:33 ` Marek Behún
  2022-05-09  4:01   ` Lukas Wunner
  2022-02-20 19:33 ` [PATCH 08/18] PCI: aardvark: Add support for DLLSC and hotplug interrupt Marek Behún
                   ` (14 subsequent siblings)
  21 siblings, 1 reply; 44+ messages in thread
From: Marek Behún @ 2022-02-20 19:33 UTC (permalink / raw)
  To: Lorenzo Pieralisi, Bjorn Helgaas
  Cc: Krzysztof Wilczyński, Marc Zyngier, pali, linux-pci,
	linux-arm-kernel, Gregory CLEMENT, Marek Behún

From: Pali Rohár <pali@kernel.org>

The No Command Completed Support bit in the Slot Capabilities register
indicates whether Command Completed Interrupt Enable is unsupported.

Enable this interrupt only in the case it is supported.

Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Marek Behún <kabel@kernel.org>
---
 drivers/pci/hotplug/pciehp_hpc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
index 373bb396fe22..838eb6cc3ec7 100644
--- a/drivers/pci/hotplug/pciehp_hpc.c
+++ b/drivers/pci/hotplug/pciehp_hpc.c
@@ -817,7 +817,9 @@ static void pcie_enable_notification(struct controller *ctrl)
 	else
 		cmd |= PCI_EXP_SLTCTL_PDCE;
 	if (!pciehp_poll_mode)
-		cmd |= PCI_EXP_SLTCTL_HPIE | PCI_EXP_SLTCTL_CCIE;
+		cmd |= PCI_EXP_SLTCTL_HPIE;
+	if (!pciehp_poll_mode && !NO_CMD_CMPL(ctrl))
+		cmd |= PCI_EXP_SLTCTL_CCIE;
 
 	mask = (PCI_EXP_SLTCTL_PDCE | PCI_EXP_SLTCTL_ABPE |
 		PCI_EXP_SLTCTL_PFDE |
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 08/18] PCI: aardvark: Add support for DLLSC and hotplug interrupt
  2022-02-20 19:33 [PATCH 00/18] PCI: aardvark controller changes BATCH 5 Marek Behún
                   ` (6 preceding siblings ...)
  2022-02-20 19:33 ` [PATCH 07/18] PCI: pciehp: Enable Command Completed Interrupt " Marek Behún
@ 2022-02-20 19:33 ` Marek Behún
  2022-02-20 19:33 ` [PATCH 09/18] PCI: Add PCI_EXP_SLTCTL_ASPL_DISABLE macro Marek Behún
                   ` (13 subsequent siblings)
  21 siblings, 0 replies; 44+ messages in thread
From: Marek Behún @ 2022-02-20 19:33 UTC (permalink / raw)
  To: Lorenzo Pieralisi, Bjorn Helgaas
  Cc: Krzysztof Wilczyński, Marc Zyngier, pali, linux-pci,
	linux-arm-kernel, Gregory CLEMENT, Marek Behún

From: Pali Rohár <pali@kernel.org>

Add support for Data Link Layer State Change in the emulated slot
registers and hotplug interrupt via the emulated root bridge.

Link down state change can be implemented because Aardvark supports Link
Down event interrupt. Use it for signaling that Data Link Layer Link is
not active anymore via Hot-Plug Interrupt on emulated root bridge.

Link up interrupt is not available on Aardvark, but we check for whether
link is up in the advk_pcie_link_up() function. By triggering Hot-Plug
Interrupt from this function we achieve Link up event, so long as the
function is called (which it is after probe and when rescanning).
Although it is not ideal, it is better than nothing.

Since advk_pcie_link_up() is not called from interrupt handler, we
cannot call generic_handle_domain_irq() from it directly. Instead create
a TIMER_IRQSAFE timer and trigger it from advk_pcie_link_up().

(We haven't been able to find any documentation for a Link Up interrupt
 on Aardvark, but it is possible there is one, in some undocumented
 register. If we manage to find this information, this can be
 rewritten.)

Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Marek Behún <kabel@kernel.org>
---
 drivers/pci/controller/pci-aardvark.c | 100 ++++++++++++++++++++++++--
 1 file changed, 96 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
index c80c78505bfa..62bb0308b9f7 100644
--- a/drivers/pci/controller/pci-aardvark.c
+++ b/drivers/pci/controller/pci-aardvark.c
@@ -24,6 +24,7 @@
 #include <linux/of_address.h>
 #include <linux/of_gpio.h>
 #include <linux/of_pci.h>
+#include <linux/timer.h>
 
 #include "../pci.h"
 #include "../pci-bridge-emul.h"
@@ -99,6 +100,7 @@
 #define PCIE_MSG_PM_PME_MASK			BIT(7)
 #define PCIE_ISR0_MASK_REG			(CONTROL_BASE_ADDR + 0x44)
 #define     PCIE_ISR0_MSI_INT_PENDING		BIT(24)
+#define     PCIE_ISR0_LINK_DOWN			BIT(1)
 #define     PCIE_ISR0_CORR_ERR			BIT(11)
 #define     PCIE_ISR0_NFAT_ERR			BIT(12)
 #define     PCIE_ISR0_FAT_ERR			BIT(13)
@@ -284,6 +286,8 @@ struct advk_pcie {
 	DECLARE_BITMAP(msi_used, MSI_IRQ_NUM);
 	struct mutex msi_used_lock;
 	int link_gen;
+	bool link_was_up;
+	struct timer_list link_irq_timer;
 	struct pci_bridge_emul bridge;
 	struct gpio_desc *reset_gpio;
 	struct phy *phy;
@@ -313,7 +317,24 @@ static inline bool advk_pcie_link_up(struct advk_pcie *pcie)
 {
 	/* check if LTSSM is in normal operation - some L* state */
 	u8 ltssm_state = advk_pcie_ltssm_state(pcie);
-	return ltssm_state >= LTSSM_L0 && ltssm_state < LTSSM_DISABLED;
+	bool link_is_up;
+	u16 slotsta;
+
+	link_is_up = ltssm_state >= LTSSM_L0 && ltssm_state < LTSSM_DISABLED;
+
+	if (link_is_up && !pcie->link_was_up) {
+		dev_info(&pcie->pdev->dev, "link up\n");
+
+		pcie->link_was_up = true;
+
+		slotsta = le16_to_cpu(pcie->bridge.pcie_conf.slotsta);
+		slotsta |= PCI_EXP_SLTSTA_DLLSC;
+		pcie->bridge.pcie_conf.slotsta = cpu_to_le16(slotsta);
+
+		mod_timer(&pcie->link_irq_timer, jiffies + 1);
+	}
+
+	return link_is_up;
 }
 
 static inline bool advk_pcie_link_active(struct advk_pcie *pcie)
@@ -442,8 +463,6 @@ static void advk_pcie_train_link(struct advk_pcie *pcie)
 	ret = advk_pcie_wait_for_link(pcie);
 	if (ret < 0)
 		dev_err(dev, "link never came up\n");
-	else
-		dev_info(dev, "link up\n");
 }
 
 /*
@@ -592,6 +611,11 @@ static void advk_pcie_setup_hw(struct advk_pcie *pcie)
 	reg &= ~PCIE_ISR0_MSI_INT_PENDING;
 	advk_writel(pcie, reg, PCIE_ISR0_MASK_REG);
 
+	/* Unmask Link Down interrupt */
+	reg = advk_readl(pcie, PCIE_ISR0_MASK_REG);
+	reg &= ~PCIE_ISR0_LINK_DOWN;
+	advk_writel(pcie, reg, PCIE_ISR0_MASK_REG);
+
 	/* Unmask PME interrupt for processing of PME requester */
 	reg = advk_readl(pcie, PCIE_ISR0_MASK_REG);
 	reg &= ~PCIE_MSG_PM_PME_MASK;
@@ -918,6 +942,14 @@ advk_pci_bridge_emul_pcie_conf_write(struct pci_bridge_emul *bridge,
 			advk_pcie_wait_for_retrain(pcie);
 		break;
 
+	case PCI_EXP_SLTCTL: {
+		u16 slotctl = le16_to_cpu(bridge->pcie_conf.slotctl);
+		/* Only emulation of HPIE and DLLSCE bits is provided */
+		slotctl &= PCI_EXP_SLTCTL_HPIE | PCI_EXP_SLTCTL_DLLSCE;
+		bridge->pcie_conf.slotctl = cpu_to_le16(slotctl);
+		break;
+	}
+
 	case PCI_EXP_RTCTL: {
 		u16 rootctl = le16_to_cpu(bridge->pcie_conf.rootctl);
 		/* Only emulation of PMEIE and CRSSVE bits is provided */
@@ -1033,6 +1065,7 @@ static const struct pci_bridge_emul_ops advk_pci_bridge_emul_ops = {
 static int advk_sw_pci_bridge_init(struct advk_pcie *pcie)
 {
 	struct pci_bridge_emul *bridge = &pcie->bridge;
+	u32 slotcap;
 
 	bridge->conf.vendor =
 		cpu_to_le16(advk_readl(pcie, PCIE_CORE_DEV_ID_REG) & 0xffff);
@@ -1059,6 +1092,13 @@ static int advk_sw_pci_bridge_init(struct advk_pcie *pcie)
 	bridge->pcie_conf.cap = cpu_to_le16(2 | PCI_EXP_FLAGS_SLOT);
 
 	/*
+	 * Mark bridge as Hot Plug Capable since this is the way how to enable
+	 * delivering of Data Link Layer State Change interrupts.
+	 *
+	 * Set No Command Completed Support because bridge does not support
+	 * Command Completed Interrupt. Every command is executed immediately
+	 * without any delay.
+	 *
 	 * Set Presence Detect State bit permanently since there is no support
 	 * for unplugging the card nor detecting whether it is plugged. (If a
 	 * platform exists in the future that supports it, via a GPIO for
@@ -1068,7 +1108,9 @@ static int advk_sw_pci_bridge_init(struct advk_pcie *pcie)
 	 * value is reserved for ports within the same silicon as Root Port
 	 * which is not our case.
 	 */
-	bridge->pcie_conf.slotcap = cpu_to_le32(1 << PCI_EXP_SLTCAP_PSN_SHIFT);
+	slotcap = PCI_EXP_SLTCAP_NCCS | PCI_EXP_SLTCAP_HPC |
+		  (1 << PCI_EXP_SLTCAP_PSN_SHIFT);
+	bridge->pcie_conf.slotcap = cpu_to_le32(slotcap);
 	bridge->pcie_conf.slotsta = cpu_to_le16(PCI_EXP_SLTSTA_PDS);
 
 	/* Indicates supports for Completion Retry Status */
@@ -1565,6 +1607,24 @@ static void advk_pcie_remove_rp_irq_domain(struct advk_pcie *pcie)
 	irq_domain_remove(pcie->rp_irq_domain);
 }
 
+static void advk_pcie_link_irq_handler(struct timer_list *timer)
+{
+	struct advk_pcie *pcie = from_timer(pcie, timer, link_irq_timer);
+	u16 slotctl;
+
+	slotctl = le16_to_cpu(pcie->bridge.pcie_conf.slotctl);
+	if (!(slotctl & PCI_EXP_SLTCTL_DLLSCE) ||
+	    !(slotctl & PCI_EXP_SLTCTL_HPIE))
+		return;
+
+	/*
+	 * Aardvark HW returns zero for PCI_EXP_FLAGS_IRQ, so use PCIe
+	 * interrupt 0
+	 */
+	if (generic_handle_domain_irq(pcie->rp_irq_domain, 0) == -EINVAL)
+		dev_err_ratelimited(&pcie->pdev->dev, "unhandled HP IRQ\n");
+}
+
 static void advk_pcie_handle_pme(struct advk_pcie *pcie)
 {
 	u32 requester = advk_readl(pcie, PCIE_MSG_LOG_REG) >> 16;
@@ -1616,6 +1676,7 @@ static void advk_pcie_handle_int(struct advk_pcie *pcie)
 {
 	u32 isr0_val, isr0_mask, isr0_status;
 	u32 isr1_val, isr1_mask, isr1_status;
+	u16 slotsta;
 	int i;
 
 	isr0_val = advk_readl(pcie, PCIE_ISR0_REG);
@@ -1642,6 +1703,26 @@ static void advk_pcie_handle_int(struct advk_pcie *pcie)
 			dev_err_ratelimited(&pcie->pdev->dev, "unhandled ERR IRQ\n");
 	}
 
+	/* Process Link Down interrupt as HP IRQ */
+	if (isr0_status & PCIE_ISR0_LINK_DOWN) {
+		advk_writel(pcie, PCIE_ISR0_LINK_DOWN, PCIE_ISR0_REG);
+
+		dev_info(&pcie->pdev->dev, "link down\n");
+
+		pcie->link_was_up = false;
+
+		slotsta = le16_to_cpu(pcie->bridge.pcie_conf.slotsta);
+		slotsta |= PCI_EXP_SLTSTA_DLLSC;
+		pcie->bridge.pcie_conf.slotsta = cpu_to_le16(slotsta);
+
+		/*
+		 * Deactivate timer and call advk_pcie_link_irq_handler()
+		 * function directly as we are in the interrupt context.
+		 */
+		del_timer_sync(&pcie->link_irq_timer);
+		advk_pcie_link_irq_handler(&pcie->link_irq_timer);
+	}
+
 	/* Process MSI interrupts */
 	if (isr0_status & PCIE_ISR0_MSI_INT_PENDING)
 		advk_pcie_handle_msi(pcie);
@@ -1877,6 +1958,14 @@ static int advk_pcie_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	/*
+	 * generic_handle_domain_irq() expects local IRQs to be disabled since
+	 * normally it is called from interrupt context, so use TIMER_IRQSAFE
+	 * flag for this link_irq_timer.
+	 */
+	timer_setup(&pcie->link_irq_timer, advk_pcie_link_irq_handler,
+		    TIMER_IRQSAFE);
+
 	advk_pcie_setup_hw(pcie);
 
 	ret = advk_sw_pci_bridge_init(pcie);
@@ -1971,6 +2060,9 @@ static int advk_pcie_remove(struct platform_device *pdev)
 	advk_pcie_remove_msi_irq_domain(pcie);
 	advk_pcie_remove_irq_domain(pcie);
 
+	/* Deactivate link event timer */
+	del_timer_sync(&pcie->link_irq_timer);
+
 	/* Free config space for emulated root bridge */
 	pci_bridge_emul_cleanup(&pcie->bridge);
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 09/18] PCI: Add PCI_EXP_SLTCTL_ASPL_DISABLE macro
  2022-02-20 19:33 [PATCH 00/18] PCI: aardvark controller changes BATCH 5 Marek Behún
                   ` (7 preceding siblings ...)
  2022-02-20 19:33 ` [PATCH 08/18] PCI: aardvark: Add support for DLLSC and hotplug interrupt Marek Behún
@ 2022-02-20 19:33 ` Marek Behún
  2022-02-20 19:33 ` [PATCH 10/18] PCI: Add function for parsing `slot-power-limit-milliwatt` DT property Marek Behún
                   ` (12 subsequent siblings)
  21 siblings, 0 replies; 44+ messages in thread
From: Marek Behún @ 2022-02-20 19:33 UTC (permalink / raw)
  To: Lorenzo Pieralisi, Bjorn Helgaas
  Cc: Krzysztof Wilczyński, Marc Zyngier, pali, linux-pci,
	linux-arm-kernel, Gregory CLEMENT, Marek Behún

From: Pali Rohár <pali@kernel.org>

Add macro defining Auto Slot Power Limit Disable bit in Slot Control
Register.

Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Marek Behún <kabel@kernel.org>
---
 include/uapi/linux/pci_regs.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index d825e17e448c..3fc9a4cac630 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -619,6 +619,7 @@
 #define  PCI_EXP_SLTCTL_PWR_OFF        0x0400 /* Power Off */
 #define  PCI_EXP_SLTCTL_EIC	0x0800	/* Electromechanical Interlock Control */
 #define  PCI_EXP_SLTCTL_DLLSCE	0x1000	/* Data Link Layer State Changed Enable */
+#define  PCI_EXP_SLTCTL_ASPL_DISABLE	0x2000 /* Auto Slot Power Limit Disable */
 #define  PCI_EXP_SLTCTL_IBPD_DISABLE	0x4000 /* In-band PD disable */
 #define PCI_EXP_SLTSTA		0x1a	/* Slot Status */
 #define  PCI_EXP_SLTSTA_ABP	0x0001	/* Attention Button Pressed */
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 10/18] PCI: Add function for parsing `slot-power-limit-milliwatt` DT property
  2022-02-20 19:33 [PATCH 00/18] PCI: aardvark controller changes BATCH 5 Marek Behún
                   ` (8 preceding siblings ...)
  2022-02-20 19:33 ` [PATCH 09/18] PCI: Add PCI_EXP_SLTCTL_ASPL_DISABLE macro Marek Behún
@ 2022-02-20 19:33 ` Marek Behún
  2022-02-20 19:33 ` [PATCH 11/18] dt-bindings: PCI: aardvark: Describe slot-power-limit-milliwatt Marek Behún
                   ` (11 subsequent siblings)
  21 siblings, 0 replies; 44+ messages in thread
From: Marek Behún @ 2022-02-20 19:33 UTC (permalink / raw)
  To: Lorenzo Pieralisi, Bjorn Helgaas
  Cc: Krzysztof Wilczyński, Marc Zyngier, pali, linux-pci,
	linux-arm-kernel, Gregory CLEMENT, Marek Behún

From: Pali Rohár <pali@kernel.org>

Add function of_pci_get_slot_power_limit(), which parses the
`slot-power-limit-milliwatt` DT property, returning the value in
milliwatts and in format ready for the PCIe Slot Capabilities Register.

Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Marek Behún <kabel@kernel.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/of.c  | 64 +++++++++++++++++++++++++++++++++++++++++++++++
 drivers/pci/pci.h | 15 +++++++++++
 2 files changed, 79 insertions(+)

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index cb2e8351c2cc..2b0c0a3641a8 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -633,3 +633,67 @@ int of_pci_get_max_link_speed(struct device_node *node)
 	return max_link_speed;
 }
 EXPORT_SYMBOL_GPL(of_pci_get_max_link_speed);
+
+/**
+ * of_pci_get_slot_power_limit - Parses the "slot-power-limit-milliwatt"
+ *				 property.
+ *
+ * @node: device tree node with the slot power limit information
+ * @slot_power_limit_value: pointer where the value should be stored in PCIe
+ *			    Slot Capabilities Register format
+ * @slot_power_limit_scale: pointer where the scale should be stored in PCIe
+ *			    Slot Capabilities Register format
+ *
+ * Returns the slot power limit in milliwatts and if @slot_power_limit_value
+ * and @slot_power_limit_scale pointers are non-NULL, fills in the value and
+ * scale in format used by PCIe Slot Capabilities Register.
+ *
+ * If the property is not found or is invalid, returns 0.
+ */
+u32 of_pci_get_slot_power_limit(struct device_node *node,
+				u8 *slot_power_limit_value,
+				u8 *slot_power_limit_scale)
+{
+	u32 slot_power_limit;
+	u8 value, scale;
+
+	if (of_property_read_u32(node, "slot-power-limit-milliwatt",
+				 &slot_power_limit))
+		slot_power_limit = 0;
+
+	/* Calculate Slot Power Limit Value and Slot Power Limit Scale */
+	if (slot_power_limit == 0) {
+		value = 0x00;
+		scale = 0;
+	} else if (slot_power_limit <= 255) {
+		value = slot_power_limit;
+		scale = 3;
+	} else if (slot_power_limit <= 255*10) {
+		value = slot_power_limit / 10;
+		scale = 2;
+	} else if (slot_power_limit <= 255*100) {
+		value = slot_power_limit / 100;
+		scale = 1;
+	} else if (slot_power_limit <= 239*1000) {
+		value = slot_power_limit / 1000;
+		scale = 0;
+	} else if (slot_power_limit <= 250*1000) {
+		value = 0xF0;
+		scale = 0;
+	} else if (slot_power_limit <= 275*1000) {
+		value = 0xF1;
+		scale = 0;
+	} else {
+		value = 0xF2;
+		scale = 0;
+	}
+
+	if (slot_power_limit_value)
+		*slot_power_limit_value = value;
+
+	if (slot_power_limit_scale)
+		*slot_power_limit_scale = scale;
+
+	return slot_power_limit;
+}
+EXPORT_SYMBOL_GPL(of_pci_get_slot_power_limit);
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 3d60cabde1a1..e10cdec6c56e 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -627,6 +627,9 @@ struct device_node;
 int of_pci_parse_bus_range(struct device_node *node, struct resource *res);
 int of_get_pci_domain_nr(struct device_node *node);
 int of_pci_get_max_link_speed(struct device_node *node);
+u32 of_pci_get_slot_power_limit(struct device_node *node,
+				u8 *slot_power_limit_value,
+				u8 *slot_power_limit_scale);
 void pci_set_of_node(struct pci_dev *dev);
 void pci_release_of_node(struct pci_dev *dev);
 void pci_set_bus_of_node(struct pci_bus *bus);
@@ -653,6 +656,18 @@ of_pci_get_max_link_speed(struct device_node *node)
 	return -EINVAL;
 }
 
+static inline u32
+of_pci_get_slot_power_limit(struct device_node *node,
+			    u8 *slot_power_limit_value,
+			    u8 *slot_power_limit_scale)
+{
+	if (slot_power_limit_value)
+		*slot_power_limit_value = 0;
+	if (slot_power_limit_scale)
+		*slot_power_limit_scale = 0;
+	return 0;
+}
+
 static inline void pci_set_of_node(struct pci_dev *dev) { }
 static inline void pci_release_of_node(struct pci_dev *dev) { }
 static inline void pci_set_bus_of_node(struct pci_bus *bus) { }
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 11/18] dt-bindings: PCI: aardvark: Describe slot-power-limit-milliwatt
  2022-02-20 19:33 [PATCH 00/18] PCI: aardvark controller changes BATCH 5 Marek Behún
                   ` (9 preceding siblings ...)
  2022-02-20 19:33 ` [PATCH 10/18] PCI: Add function for parsing `slot-power-limit-milliwatt` DT property Marek Behún
@ 2022-02-20 19:33 ` Marek Behún
  2022-02-20 19:33 ` [PATCH 12/18] PCI: aardvark: Send Set_Slot_Power_Limit message Marek Behún
                   ` (10 subsequent siblings)
  21 siblings, 0 replies; 44+ messages in thread
From: Marek Behún @ 2022-02-20 19:33 UTC (permalink / raw)
  To: Lorenzo Pieralisi, Bjorn Helgaas
  Cc: Krzysztof Wilczyński, Marc Zyngier, pali, linux-pci,
	linux-arm-kernel, Gregory CLEMENT, Marek Behún

From: Pali Rohár <pali@kernel.org>

Add description of the "slot-power-limit-milliwatt" property to the
Aardvark controller binding.

Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Marek Behún <kabel@kernel.org>
---
 Documentation/devicetree/bindings/pci/aardvark-pci.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/aardvark-pci.txt b/Documentation/devicetree/bindings/pci/aardvark-pci.txt
index 2b8ca920a7fa..0405870d2fa0 100644
--- a/Documentation/devicetree/bindings/pci/aardvark-pci.txt
+++ b/Documentation/devicetree/bindings/pci/aardvark-pci.txt
@@ -20,6 +20,7 @@ contain the following properties:
    define the mapping of the PCIe interface to interrupt numbers.
  - bus-range: PCI bus numbers covered
  - phys: the PCIe PHY handle
+ - slot-power-limit-milliwatt: see pci-bus.yaml in dtschema
  - max-link-speed: see pci.txt
  - reset-gpios: see pci.txt
 
@@ -52,6 +53,7 @@ Example:
 				<0 0 0 3 &pcie_intc 2>,
 				<0 0 0 4 &pcie_intc 3>;
 		phys = <&comphy1 0>;
+		slot-power-limit-milliwatt = <10000>;
 		pcie_intc: interrupt-controller {
 			interrupt-controller;
 			#interrupt-cells = <1>;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 12/18] PCI: aardvark: Send Set_Slot_Power_Limit message
  2022-02-20 19:33 [PATCH 00/18] PCI: aardvark controller changes BATCH 5 Marek Behún
                   ` (10 preceding siblings ...)
  2022-02-20 19:33 ` [PATCH 11/18] dt-bindings: PCI: aardvark: Describe slot-power-limit-milliwatt Marek Behún
@ 2022-02-20 19:33 ` Marek Behún
  2022-02-20 19:33 ` [PATCH 13/18] arm64: dts: armada-3720-turris-mox: Define slot-power-limit-milliwatt for PCIe Marek Behún
                   ` (9 subsequent siblings)
  21 siblings, 0 replies; 44+ messages in thread
From: Marek Behún @ 2022-02-20 19:33 UTC (permalink / raw)
  To: Lorenzo Pieralisi, Bjorn Helgaas
  Cc: Krzysztof Wilczyński, Marc Zyngier, pali, linux-pci,
	linux-arm-kernel, Gregory CLEMENT, Marek Behún

From: Pali Rohár <pali@kernel.org>

Emulate Slot PowerLimit Scale and Value bits in the Slot Capabilities
register of the emulated bridge and if slot power limit value is
defined, send that Set_Slot_Power_Limit message via Message Generation
Control Register in Link Up handler on link up event.

Slot power limit value is read from device-tree property
'slot-power-limit-milliwatt'. If this property is not specified, we
treat it as "Slot Capabilities register has not yet been initialized".

According to PCIe Base specification 3.0, when transitioning from a
non-DL_Up Status to a DL_Up Status, the Port must initiate the
transmission of a Set_Slot_Power_Limit Message to the other component
on the Link to convey the value programmed in the Slot Power Limit
Scale and Value fields of the Slot Capabilities register. This
transmission is optional if the Slot Capabilities register has not
yet been initialized.

Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Marek Behún <kabel@kernel.org>
---
 drivers/pci/controller/pci-aardvark.c | 52 ++++++++++++++++++++++++---
 1 file changed, 48 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
index 62bb0308b9f7..41127a26c5bc 100644
--- a/drivers/pci/controller/pci-aardvark.c
+++ b/drivers/pci/controller/pci-aardvark.c
@@ -212,6 +212,11 @@ enum {
 };
 
 #define VENDOR_ID_REG				(LMI_BASE_ADDR + 0x44)
+#define PME_MSG_GEN_CTRL			(LMI_BASE_ADDR + 0x220)
+#define     SEND_SET_SLOT_POWER_LIMIT		BIT(13)
+#define     SEND_PME_TURN_OFF			BIT(14)
+#define     SLOT_POWER_LIMIT_DATA_SHIFT		16
+#define     SLOT_POWER_LIMIT_DATA_MASK		GENMASK(25, 16)
 
 /* PCIe core controller registers */
 #define CTRL_CORE_BASE_ADDR			0x18000
@@ -285,6 +290,8 @@ struct advk_pcie {
 	raw_spinlock_t msi_irq_lock;
 	DECLARE_BITMAP(msi_used, MSI_IRQ_NUM);
 	struct mutex msi_used_lock;
+	u8 slot_power_limit_value;
+	u8 slot_power_limit_scale;
 	int link_gen;
 	bool link_was_up;
 	struct timer_list link_irq_timer;
@@ -317,8 +324,9 @@ static inline bool advk_pcie_link_up(struct advk_pcie *pcie)
 {
 	/* check if LTSSM is in normal operation - some L* state */
 	u8 ltssm_state = advk_pcie_ltssm_state(pcie);
+	u16 slotsta, slotctl;
+	u32 slotpwr, val;
 	bool link_is_up;
-	u16 slotsta;
 
 	link_is_up = ltssm_state >= LTSSM_L0 && ltssm_state < LTSSM_DISABLED;
 
@@ -332,6 +340,28 @@ static inline bool advk_pcie_link_up(struct advk_pcie *pcie)
 		pcie->bridge.pcie_conf.slotsta = cpu_to_le16(slotsta);
 
 		mod_timer(&pcie->link_irq_timer, jiffies + 1);
+
+		/*
+		 * According to PCIe Base specification 3.0, when transitioning
+		 * from a non-DL_Up Status to a DL_Up Status, the Port must
+		 * initiate the transmission of a Set_Slot_Power_Limit Message
+		 * to the other component on the Link to convey the value
+		 * programmed in the Slot Power Limit Scale and Value fields of
+		 * the Slot Capabilities register. This transmission is optional
+		 * if the Slot Capabilities register has not yet been
+		 * initialized.
+		 */
+		slotctl = le16_to_cpu(pcie->bridge.pcie_conf.slotctl);
+		slotpwr = (le32_to_cpu(pcie->bridge.pcie_conf.slotcap) &
+			   (PCI_EXP_SLTCAP_SPLV | PCI_EXP_SLTCAP_SPLS)) >>
+			  PCI_EXP_SLTCAP_SPLV_SHIFT;
+		if (!(slotctl & PCI_EXP_SLTCTL_ASPL_DISABLE) && slotpwr) {
+			val = advk_readl(pcie, PME_MSG_GEN_CTRL);
+			val &= ~SLOT_POWER_LIMIT_DATA_MASK;
+			val |= slotpwr << SLOT_POWER_LIMIT_DATA_SHIFT;
+			val |= SEND_SET_SLOT_POWER_LIMIT;
+			advk_writel(pcie, val, PME_MSG_GEN_CTRL);
+		}
 	}
 
 	return link_is_up;
@@ -944,8 +974,9 @@ advk_pci_bridge_emul_pcie_conf_write(struct pci_bridge_emul *bridge,
 
 	case PCI_EXP_SLTCTL: {
 		u16 slotctl = le16_to_cpu(bridge->pcie_conf.slotctl);
-		/* Only emulation of HPIE and DLLSCE bits is provided */
-		slotctl &= PCI_EXP_SLTCTL_HPIE | PCI_EXP_SLTCTL_DLLSCE;
+		/* Only emulation of HPIE, DLLSCE and ASPLD bits is provided */
+		slotctl &= PCI_EXP_SLTCTL_HPIE | PCI_EXP_SLTCTL_DLLSCE |
+			   PCI_EXP_SLTCTL_ASPL_DISABLE;
 		bridge->pcie_conf.slotctl = cpu_to_le16(slotctl);
 		break;
 	}
@@ -1107,9 +1138,13 @@ static int advk_sw_pci_bridge_init(struct advk_pcie *pcie)
 	 * Set physical slot number to 1 since there is only one port and zero
 	 * value is reserved for ports within the same silicon as Root Port
 	 * which is not our case.
+	 *
+	 * Set slot power limit.
 	 */
 	slotcap = PCI_EXP_SLTCAP_NCCS | PCI_EXP_SLTCAP_HPC |
-		  (1 << PCI_EXP_SLTCAP_PSN_SHIFT);
+		  (1 << PCI_EXP_SLTCAP_PSN_SHIFT) |
+		  (pcie->slot_power_limit_value << PCI_EXP_SLTCAP_SPLV_SHIFT) |
+		  (pcie->slot_power_limit_scale << PCI_EXP_SLTCAP_SPLS_SHIFT);
 	bridge->pcie_conf.slotcap = cpu_to_le32(slotcap);
 	bridge->pcie_conf.slotsta = cpu_to_le16(PCI_EXP_SLTSTA_PDS);
 
@@ -1842,6 +1877,7 @@ static int advk_pcie_probe(struct platform_device *pdev)
 	struct advk_pcie *pcie;
 	struct pci_host_bridge *bridge;
 	struct resource_entry *entry;
+	u32 slot_power_limit;
 	int ret;
 
 	bridge = devm_pci_alloc_host_bridge(dev, sizeof(struct advk_pcie));
@@ -1954,6 +1990,14 @@ static int advk_pcie_probe(struct platform_device *pdev)
 	else
 		pcie->link_gen = ret;
 
+	slot_power_limit = of_pci_get_slot_power_limit(dev->of_node,
+						       &pcie->slot_power_limit_value,
+						       &pcie->slot_power_limit_scale);
+	if (slot_power_limit)
+		dev_info(dev, "Slot Power Limit: %u.%uW\n",
+			 slot_power_limit / 1000,
+			 (slot_power_limit / 100) % 10);
+
 	ret = advk_pcie_setup_phy(pcie);
 	if (ret)
 		return ret;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 13/18] arm64: dts: armada-3720-turris-mox: Define slot-power-limit-milliwatt for PCIe
  2022-02-20 19:33 [PATCH 00/18] PCI: aardvark controller changes BATCH 5 Marek Behún
                   ` (11 preceding siblings ...)
  2022-02-20 19:33 ` [PATCH 12/18] PCI: aardvark: Send Set_Slot_Power_Limit message Marek Behún
@ 2022-02-20 19:33 ` Marek Behún
  2022-02-20 19:33 ` [PATCH 14/18] PCI: aardvark: Add clock support Marek Behún
                   ` (8 subsequent siblings)
  21 siblings, 0 replies; 44+ messages in thread
From: Marek Behún @ 2022-02-20 19:33 UTC (permalink / raw)
  To: Lorenzo Pieralisi, Bjorn Helgaas
  Cc: Krzysztof Wilczyński, Marc Zyngier, pali, linux-pci,
	linux-arm-kernel, Gregory CLEMENT, Marek Behún

From: Pali Rohár <pali@kernel.org>

PCIe Slot Power Limit on Turris Mox is 10W.

Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Marek Behún <kabel@kernel.org>
---
 arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
index 04da07ae4420..6dca28d7f764 100644
--- a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
+++ b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
@@ -135,6 +135,7 @@ &pcie0 {
 	pinctrl-0 = <&pcie_reset_pins &pcie_clkreq_pins>;
 	status = "okay";
 	reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>;
+	slot-power-limit-milliwatt = <10000>;
 	/*
 	 * U-Boot port for Turris Mox has a bug which always expects that "ranges" DT property
 	 * contains exactly 2 ranges with 3 (child) address cells, 2 (parent) address cells and
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 14/18] PCI: aardvark: Add clock support
  2022-02-20 19:33 [PATCH 00/18] PCI: aardvark controller changes BATCH 5 Marek Behún
                   ` (12 preceding siblings ...)
  2022-02-20 19:33 ` [PATCH 13/18] arm64: dts: armada-3720-turris-mox: Define slot-power-limit-milliwatt for PCIe Marek Behún
@ 2022-02-20 19:33 ` Marek Behún
  2022-02-20 19:33 ` [PATCH 15/18] arm64: dts: marvell: armada-37xx: Add clock to PCIe node Marek Behún
                   ` (7 subsequent siblings)
  21 siblings, 0 replies; 44+ messages in thread
From: Marek Behún @ 2022-02-20 19:33 UTC (permalink / raw)
  To: Lorenzo Pieralisi, Bjorn Helgaas
  Cc: Krzysztof Wilczyński, Marc Zyngier, pali, linux-pci,
	linux-arm-kernel, Gregory CLEMENT, Miquel Raynal,
	Marek Behún

From: Miquel Raynal <miquel.raynal@bootlin.com>

The IP relies on a gated clock. When we will add S2RAM support, this
clock will need to be resumed before any PCIe registers are
accessed. Add support for this clock.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Marek Behún <kabel@kernel.org>
---
 drivers/pci/controller/pci-aardvark.c | 32 +++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
index 41127a26c5bc..3b51f47abd72 100644
--- a/drivers/pci/controller/pci-aardvark.c
+++ b/drivers/pci/controller/pci-aardvark.c
@@ -8,6 +8,7 @@
  * Author: Hezi Shahmoon <hezi.shahmoon@marvell.com>
  */
 
+#include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/gpio/consumer.h>
 #include <linux/interrupt.h>
@@ -297,6 +298,7 @@ struct advk_pcie {
 	struct timer_list link_irq_timer;
 	struct pci_bridge_emul bridge;
 	struct gpio_desc *reset_gpio;
+	struct clk *clk;
 	struct phy *phy;
 };
 
@@ -1813,6 +1815,29 @@ static int advk_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
 		return of_irq_parse_and_map_pci(dev, slot, pin);
 }
 
+static int advk_pcie_setup_clk(struct advk_pcie *pcie)
+{
+	struct device *dev = &pcie->pdev->dev;
+	int ret;
+
+	pcie->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(pcie->clk) && (PTR_ERR(pcie->clk) == -EPROBE_DEFER))
+		return PTR_ERR(pcie->clk);
+
+	/* Old bindings miss the clock handle */
+	if (IS_ERR(pcie->clk)) {
+		dev_warn(dev, "Clock unavailable (%ld)\n", PTR_ERR(pcie->clk));
+		pcie->clk = NULL;
+		return 0;
+	}
+
+	ret = clk_prepare_enable(pcie->clk);
+	if (ret)
+		dev_err(dev, "Clock initialization failed (%d)\n", ret);
+
+	return ret;
+}
+
 static void advk_pcie_disable_phy(struct advk_pcie *pcie)
 {
 	phy_power_off(pcie->phy);
@@ -1998,6 +2023,10 @@ static int advk_pcie_probe(struct platform_device *pdev)
 			 slot_power_limit / 1000,
 			 (slot_power_limit / 100) % 10);
 
+	ret = advk_pcie_setup_clk(pcie);
+	if (ret)
+		return ret;
+
 	ret = advk_pcie_setup_phy(pcie);
 	if (ret)
 		return ret;
@@ -2126,6 +2155,9 @@ static int advk_pcie_remove(struct platform_device *pdev)
 	/* Disable phy */
 	advk_pcie_disable_phy(pcie);
 
+	/* Disable clock */
+	clk_disable_unprepare(pcie->clk);
+
 	return 0;
 }
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 15/18] arm64: dts: marvell: armada-37xx: Add clock to PCIe node
  2022-02-20 19:33 [PATCH 00/18] PCI: aardvark controller changes BATCH 5 Marek Behún
                   ` (13 preceding siblings ...)
  2022-02-20 19:33 ` [PATCH 14/18] PCI: aardvark: Add clock support Marek Behún
@ 2022-02-20 19:33 ` Marek Behún
  2022-02-28 15:52   ` Gregory CLEMENT
  2022-02-20 19:33 ` [PATCH 16/18] PCI: aardvark: Add suspend to RAM support Marek Behún
                   ` (6 subsequent siblings)
  21 siblings, 1 reply; 44+ messages in thread
From: Marek Behún @ 2022-02-20 19:33 UTC (permalink / raw)
  To: Lorenzo Pieralisi, Bjorn Helgaas
  Cc: Krzysztof Wilczyński, Marc Zyngier, pali, linux-pci,
	linux-arm-kernel, Gregory CLEMENT, Marek Behún

The clock binding documents PCIe clock for a long time already. Add
clock phande into the PCIe node.

Signed-off-by: Marek Behún <kabel@kernel.org>
---
 arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
index 673f4906eef9..c0de8d10e58c 100644
--- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
@@ -489,6 +489,7 @@ pcie0: pcie@d0070000 {
 			bus-range = <0x00 0xff>;
 			interrupts = <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>;
 			#interrupt-cells = <1>;
+			clocks = <&sb_periph_clk 13>;
 			msi-parent = <&pcie0>;
 			msi-controller;
 			/*
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 16/18] PCI: aardvark: Add suspend to RAM support
  2022-02-20 19:33 [PATCH 00/18] PCI: aardvark controller changes BATCH 5 Marek Behún
                   ` (14 preceding siblings ...)
  2022-02-20 19:33 ` [PATCH 15/18] arm64: dts: marvell: armada-37xx: Add clock to PCIe node Marek Behún
@ 2022-02-20 19:33 ` Marek Behún
  2022-04-12 11:14   ` Lorenzo Pieralisi
  2022-02-20 19:33 ` [PATCH 17/18] PCI: aardvark: Run link training in separate worker Marek Behún
                   ` (5 subsequent siblings)
  21 siblings, 1 reply; 44+ messages in thread
From: Marek Behún @ 2022-02-20 19:33 UTC (permalink / raw)
  To: Lorenzo Pieralisi, Bjorn Helgaas
  Cc: Krzysztof Wilczyński, Marc Zyngier, pali, linux-pci,
	linux-arm-kernel, Gregory CLEMENT, Miquel Raynal,
	Marek Behún

From: Miquel Raynal <miquel.raynal@bootlin.com>

Add suspend and resume callbacks. The priority of these are
"_noirq()", to workaround early access to the registers done by the
PCI core through the ->read()/->write() callbacks at resume time.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Marek Behún <kabel@kernel.org>
---
 drivers/pci/controller/pci-aardvark.c | 39 +++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
index 3b51f47abd72..8c9ac7766ac7 100644
--- a/drivers/pci/controller/pci-aardvark.c
+++ b/drivers/pci/controller/pci-aardvark.c
@@ -1896,6 +1896,44 @@ static int advk_pcie_setup_phy(struct advk_pcie *pcie)
 	return ret;
 }
 
+static int __maybe_unused advk_pcie_suspend(struct device *dev)
+{
+	struct advk_pcie *pcie = dev_get_drvdata(dev);
+
+	advk_pcie_disable_phy(pcie);
+
+	clk_disable_unprepare(pcie->clk);
+
+	return 0;
+}
+
+static int __maybe_unused advk_pcie_resume(struct device *dev)
+{
+	struct advk_pcie *pcie = dev_get_drvdata(dev);
+	int ret;
+
+	ret = clk_prepare_enable(pcie->clk);
+	if (ret)
+		return ret;
+
+	ret = advk_pcie_enable_phy(pcie);
+	if (ret)
+		return ret;
+
+	advk_pcie_setup_hw(pcie);
+
+	return 0;
+}
+
+/*
+ * The PCI core will try to reconfigure the bus quite early in the resume path.
+ * We must use the _noirq() alternatives to ensure the controller is ready when
+ * the core uses the ->read()/->write() callbacks.
+ */
+static const struct dev_pm_ops advk_pcie_dev_pm_ops = {
+	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(advk_pcie_suspend, advk_pcie_resume)
+};
+
 static int advk_pcie_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -2171,6 +2209,7 @@ static struct platform_driver advk_pcie_driver = {
 	.driver = {
 		.name = "advk-pcie",
 		.of_match_table = advk_pcie_of_match_table,
+		.pm = &advk_pcie_dev_pm_ops,
 	},
 	.probe = advk_pcie_probe,
 	.remove = advk_pcie_remove,
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 17/18] PCI: aardvark: Run link training in separate worker
  2022-02-20 19:33 [PATCH 00/18] PCI: aardvark controller changes BATCH 5 Marek Behún
                   ` (15 preceding siblings ...)
  2022-02-20 19:33 ` [PATCH 16/18] PCI: aardvark: Add suspend to RAM support Marek Behún
@ 2022-02-20 19:33 ` Marek Behún
  2022-04-12 15:25   ` Lorenzo Pieralisi
  2022-02-20 19:33 ` [PATCH 18/18] PCI: aardvark: Optimize PCIe card reset via GPIO Marek Behún
                   ` (4 subsequent siblings)
  21 siblings, 1 reply; 44+ messages in thread
From: Marek Behún @ 2022-02-20 19:33 UTC (permalink / raw)
  To: Lorenzo Pieralisi, Bjorn Helgaas
  Cc: Krzysztof Wilczyński, Marc Zyngier, pali, linux-pci,
	linux-arm-kernel, Gregory CLEMENT, Marek Behún

From: Pali Rohár <pali@kernel.org>

Link training and PCIe card reset routines in Aardvark contain several
delays, resulting in rather slow PCIe card probing. The worst case is
when there is no card connected: the driver tries link training at all
possible speeds and waits until all timers expire.

Since probe methods for all system devices are called sequentially, this
results in noticeably longer boot time.

Move card reset and link training code from driver probe function into
a separate worker, so that kernel can do something different while the
driver is waiting during reset or training.

On ESPRESSObin and Turris MOX this decreases boot time by 0.4s with
plugged PCIe card and by 2.2s if no card is connected.

Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Marek Behún <kabel@kernel.org>
---
 drivers/pci/controller/pci-aardvark.c | 42 ++++++++++++++++++---------
 1 file changed, 28 insertions(+), 14 deletions(-)

diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
index 8c9ac7766ac7..056f49d0e3a4 100644
--- a/drivers/pci/controller/pci-aardvark.c
+++ b/drivers/pci/controller/pci-aardvark.c
@@ -26,6 +26,7 @@
 #include <linux/of_gpio.h>
 #include <linux/of_pci.h>
 #include <linux/timer.h>
+#include <linux/workqueue.h>
 
 #include "../pci.h"
 #include "../pci-bridge-emul.h"
@@ -296,6 +297,8 @@ struct advk_pcie {
 	int link_gen;
 	bool link_was_up;
 	struct timer_list link_irq_timer;
+	struct delayed_work probe_card_work;
+	bool host_bridge_probed;
 	struct pci_bridge_emul bridge;
 	struct gpio_desc *reset_gpio;
 	struct clk *clk;
@@ -497,6 +500,21 @@ static void advk_pcie_train_link(struct advk_pcie *pcie)
 		dev_err(dev, "link never came up\n");
 }
 
+static void advk_pcie_probe_card_work(struct work_struct *work)
+{
+	struct delayed_work *dwork = container_of(work, struct delayed_work,
+						  work);
+	struct advk_pcie *pcie = container_of(dwork, struct advk_pcie,
+					      probe_card_work);
+	struct pci_host_bridge *bridge = pci_host_bridge_from_priv(pcie);
+	int ret;
+
+	advk_pcie_train_link(pcie);
+	ret = pci_host_probe(bridge);
+	if (!ret)
+		pcie->host_bridge_probed = true;
+}
+
 /*
  * Set PCIe address window register which could be used for memory
  * mapping.
@@ -701,8 +719,6 @@ static void advk_pcie_setup_hw(struct advk_pcie *pcie)
 	/* Disable remaining PCIe outbound windows */
 	for (i = pcie->wins_count; i < OB_WIN_COUNT; i++)
 		advk_pcie_disable_ob_win(pcie, i);
-
-	advk_pcie_train_link(pcie);
 }
 
 static int advk_pcie_check_pio_status(struct advk_pcie *pcie, bool allow_crs, u32 *val)
@@ -2112,14 +2128,8 @@ static int advk_pcie_probe(struct platform_device *pdev)
 	bridge->ops = &advk_pcie_ops;
 	bridge->map_irq = advk_pcie_map_irq;
 
-	ret = pci_host_probe(bridge);
-	if (ret < 0) {
-		irq_set_chained_handler_and_data(pcie->irq, NULL, NULL);
-		advk_pcie_remove_rp_irq_domain(pcie);
-		advk_pcie_remove_msi_irq_domain(pcie);
-		advk_pcie_remove_irq_domain(pcie);
-		return ret;
-	}
+	INIT_DELAYED_WORK(&pcie->probe_card_work, advk_pcie_probe_card_work);
+	schedule_delayed_work(&pcie->probe_card_work, 1);
 
 	return 0;
 }
@@ -2131,11 +2141,15 @@ static int advk_pcie_remove(struct platform_device *pdev)
 	u32 val;
 	int i;
 
+	cancel_delayed_work_sync(&pcie->probe_card_work);
+
 	/* Remove PCI bus with all devices */
-	pci_lock_rescan_remove();
-	pci_stop_root_bus(bridge->bus);
-	pci_remove_root_bus(bridge->bus);
-	pci_unlock_rescan_remove();
+	if (pcie->host_bridge_probed) {
+		pci_lock_rescan_remove();
+		pci_stop_root_bus(bridge->bus);
+		pci_remove_root_bus(bridge->bus);
+		pci_unlock_rescan_remove();
+	}
 
 	/* Disable Root Bridge I/O space, memory space and bus mastering */
 	val = advk_readl(pcie, PCIE_CORE_CMD_STATUS_REG);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 18/18] PCI: aardvark: Optimize PCIe card reset via GPIO
  2022-02-20 19:33 [PATCH 00/18] PCI: aardvark controller changes BATCH 5 Marek Behún
                   ` (16 preceding siblings ...)
  2022-02-20 19:33 ` [PATCH 17/18] PCI: aardvark: Run link training in separate worker Marek Behún
@ 2022-02-20 19:33 ` Marek Behún
  2022-04-11 15:36 ` [PATCH 00/18] PCI: aardvark controller changes BATCH 5 Lorenzo Pieralisi
                   ` (3 subsequent siblings)
  21 siblings, 0 replies; 44+ messages in thread
From: Marek Behún @ 2022-02-20 19:33 UTC (permalink / raw)
  To: Lorenzo Pieralisi, Bjorn Helgaas
  Cc: Krzysztof Wilczyński, Marc Zyngier, pali, linux-pci,
	linux-arm-kernel, Gregory CLEMENT, Marek Behún

From: Pali Rohár <pali@kernel.org>

Currently we always assert reset GPIO (if it is available) for ~10ms.
But if the GPIO was asserted before driver probe (by bootloader for
example), we do not need to wait these 10ms.

Assert reset GPIO for 10ms only if it wasn't asserted.

We need to get the GPIO with flag GPIOD_FLAGS_BIT_DIR_OUT instead of
GPIOD_OUT_LOW, so that it's value isn't changed when getting it.

Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Marek Behún <kabel@kernel.org>
---
 drivers/pci/controller/pci-aardvark.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
index 056f49d0e3a4..80eb6e98923f 100644
--- a/drivers/pci/controller/pci-aardvark.c
+++ b/drivers/pci/controller/pci-aardvark.c
@@ -431,10 +431,17 @@ static void advk_pcie_issue_perst(struct advk_pcie *pcie)
 	if (!pcie->reset_gpio)
 		return;
 
-	/* 10ms delay is needed for some cards */
-	dev_info(&pcie->pdev->dev, "issuing PERST via reset GPIO for 10ms\n");
-	gpiod_set_value_cansleep(pcie->reset_gpio, 1);
-	usleep_range(10000, 11000);
+	/*
+	 * Assert PERST# for at least 10ms if it wasn't asserted yet (it could
+	 * have been asserted by bootloader or by GPIO driver, for example).
+	 */
+	if (!gpiod_get_value(pcie->reset_gpio)) {
+		dev_info(&pcie->pdev->dev, "issuing PERST via reset GPIO for 10ms\n");
+		gpiod_set_value_cansleep(pcie->reset_gpio, 1);
+		usleep_range(10000, 11000);
+	}
+
+	/* De-assert PERST# */
 	gpiod_set_value_cansleep(pcie->reset_gpio, 0);
 }
 
@@ -2049,7 +2056,7 @@ static int advk_pcie_probe(struct platform_device *pdev)
 
 	pcie->reset_gpio = devm_gpiod_get_from_of_node(dev, dev->of_node,
 						       "reset-gpios", 0,
-						       GPIOD_OUT_LOW,
+						       GPIOD_FLAGS_BIT_DIR_OUT,
 						       "pcie1-reset");
 	ret = PTR_ERR_OR_ZERO(pcie->reset_gpio);
 	if (ret) {
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* Re: [PATCH 15/18] arm64: dts: marvell: armada-37xx: Add clock to PCIe node
  2022-02-20 19:33 ` [PATCH 15/18] arm64: dts: marvell: armada-37xx: Add clock to PCIe node Marek Behún
@ 2022-02-28 15:52   ` Gregory CLEMENT
  0 siblings, 0 replies; 44+ messages in thread
From: Gregory CLEMENT @ 2022-02-28 15:52 UTC (permalink / raw)
  To: Marek Behún, Lorenzo Pieralisi, Bjorn Helgaas
  Cc: Krzysztof Wilczyński, Marc Zyngier, pali, linux-pci,
	linux-arm-kernel, Marek Behún

Marek Behún <kabel@kernel.org> writes:

> The clock binding documents PCIe clock for a long time already. Add
> clock phande into the PCIe node.
>
> Signed-off-by: Marek Behún <kabel@kernel.org>

Applied on mvebu/arm64

Thanks,

Gregory

> ---
>  arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> index 673f4906eef9..c0de8d10e58c 100644
> --- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> +++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> @@ -489,6 +489,7 @@ pcie0: pcie@d0070000 {
>  			bus-range = <0x00 0xff>;
>  			interrupts = <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>;
>  			#interrupt-cells = <1>;
> +			clocks = <&sb_periph_clk 13>;
>  			msi-parent = <&pcie0>;
>  			msi-controller;
>  			/*
> -- 
> 2.34.1
>

-- 
Gregory Clement, Bootlin
Embedded Linux and Kernel engineering
http://bootlin.com

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 00/18] PCI: aardvark controller changes BATCH 5
  2022-02-20 19:33 [PATCH 00/18] PCI: aardvark controller changes BATCH 5 Marek Behún
                   ` (17 preceding siblings ...)
  2022-02-20 19:33 ` [PATCH 18/18] PCI: aardvark: Optimize PCIe card reset via GPIO Marek Behún
@ 2022-04-11 15:36 ` Lorenzo Pieralisi
  2022-04-11 16:53   ` Pali Rohár
  2022-05-13 10:33 ` Lorenzo Pieralisi
                   ` (2 subsequent siblings)
  21 siblings, 1 reply; 44+ messages in thread
From: Lorenzo Pieralisi @ 2022-04-11 15:36 UTC (permalink / raw)
  To: Marek Behún
  Cc: Bjorn Helgaas, Krzysztof Wilczyński, Marc Zyngier, pali,
	linux-pci, linux-arm-kernel, Gregory CLEMENT

On Sun, Feb 20, 2022 at 08:33:28PM +0100, Marek Behún wrote:
> Hello Lorenzo, Krzysztof,
> 
> here comes batch 5 of changes for Aardvark PCIe controller.
> 
> This batch
> - adds support for AER
> - adds support for DLLSC and hotplug interrupt
> - adds support for sending slot power limit message
> - adds enabling/disabling PCIe clock
> - adds suspend support
> - optimizes link training by adding it into separate worker
> - optimizes GPIO resetting by asserting it only if it wasn't asserted
>   already
> 
> Marek

Hi Marek,

I noticed Pali posted patches [9,11] separately:

https://lore.kernel.org/linux-pci/20220325093827.4983-1-pali@kernel.org

I will review the rest of the series - when it comes to merging patches
we will handle how to apply them.

Lorenzo

> Marek Behún (1):
>   arm64: dts: marvell: armada-37xx: Add clock to PCIe node
> 
> Miquel Raynal (2):
>   PCI: aardvark: Add clock support
>   PCI: aardvark: Add suspend to RAM support
> 
> Pali Rohár (13):
>   PCI: aardvark: Add support for AER registers on emulated bridge
>   PCI: Add PCI_EXP_SLTCAP_*_SHIFT macros
>   PCI: aardvark: Fix reporting Slot capabilities on emulated bridge
>   PCI: pciehp: Enable DLLSC interrupt only if supported
>   PCI: pciehp: Enable Command Completed Interrupt only if supported
>   PCI: aardvark: Add support for DLLSC and hotplug interrupt
>   PCI: Add PCI_EXP_SLTCTL_ASPL_DISABLE macro
>   PCI: Add function for parsing `slot-power-limit-milliwatt` DT property
>   dt-bindings: PCI: aardvark: Describe slot-power-limit-milliwatt
>   PCI: aardvark: Send Set_Slot_Power_Limit message
>   arm64: dts: armada-3720-turris-mox: Define slot-power-limit-milliwatt
>     for PCIe
>   PCI: aardvark: Run link training in separate worker
>   PCI: aardvark: Optimize PCIe card reset via GPIO
> 
> Russell King (2):
>   PCI: pci-bridge-emul: Re-arrange register tests
>   PCI: pci-bridge-emul: Add support for PCIe extended capabilities
> 
>  .../devicetree/bindings/pci/aardvark-pci.txt  |   2 +
>  .../dts/marvell/armada-3720-turris-mox.dts    |   1 +
>  arch/arm64/boot/dts/marvell/armada-37xx.dtsi  |   1 +
>  drivers/pci/controller/pci-aardvark.c         | 380 ++++++++++++++++--
>  drivers/pci/hotplug/pciehp_hpc.c              |  34 +-
>  drivers/pci/hotplug/pnv_php.c                 |  13 +-
>  drivers/pci/of.c                              |  64 +++
>  drivers/pci/pci-bridge-emul.c                 | 130 +++---
>  drivers/pci/pci-bridge-emul.h                 |  15 +
>  drivers/pci/pci.h                             |  15 +
>  include/uapi/linux/pci_regs.h                 |   4 +
>  11 files changed, 565 insertions(+), 94 deletions(-)
> 
> -- 
> 2.34.1
> 

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 00/18] PCI: aardvark controller changes BATCH 5
  2022-04-11 15:36 ` [PATCH 00/18] PCI: aardvark controller changes BATCH 5 Lorenzo Pieralisi
@ 2022-04-11 16:53   ` Pali Rohár
  0 siblings, 0 replies; 44+ messages in thread
From: Pali Rohár @ 2022-04-11 16:53 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: Marek Behún, Bjorn Helgaas, Krzysztof Wilczyński,
	Marc Zyngier, linux-pci, linux-arm-kernel, Gregory CLEMENT

On Monday 11 April 2022 16:36:43 Lorenzo Pieralisi wrote:
> On Sun, Feb 20, 2022 at 08:33:28PM +0100, Marek Behún wrote:
> > Hello Lorenzo, Krzysztof,
> > 
> > here comes batch 5 of changes for Aardvark PCIe controller.
> > 
> > This batch
> > - adds support for AER
> > - adds support for DLLSC and hotplug interrupt
> > - adds support for sending slot power limit message
> > - adds enabling/disabling PCIe clock
> > - adds suspend support
> > - optimizes link training by adding it into separate worker
> > - optimizes GPIO resetting by asserting it only if it wasn't asserted
> >   already
> > 
> > Marek
> 
> Hi Marek,
> 
> I noticed Pali posted patches [9,11] separately:
> 
> https://lore.kernel.org/linux-pci/20220325093827.4983-1-pali@kernel.org

Patches 9,10,11 from this patch series are also in above mvebu patch
series as they are required for other patch in above patch series.

Above patch series is v3 and I'm planning to send v4 to address all
review issues.

So basically patches 9,10,11 in this patch series should be replaced
with new version.

> I will review the rest of the series - when it comes to merging patches
> we will handle how to apply them.
> 
> Lorenzo
> 
> > Marek Behún (1):
> >   arm64: dts: marvell: armada-37xx: Add clock to PCIe node
> > 
> > Miquel Raynal (2):
> >   PCI: aardvark: Add clock support
> >   PCI: aardvark: Add suspend to RAM support
> > 
> > Pali Rohár (13):
> >   PCI: aardvark: Add support for AER registers on emulated bridge
> >   PCI: Add PCI_EXP_SLTCAP_*_SHIFT macros
> >   PCI: aardvark: Fix reporting Slot capabilities on emulated bridge
> >   PCI: pciehp: Enable DLLSC interrupt only if supported
> >   PCI: pciehp: Enable Command Completed Interrupt only if supported
> >   PCI: aardvark: Add support for DLLSC and hotplug interrupt
> >   PCI: Add PCI_EXP_SLTCTL_ASPL_DISABLE macro
> >   PCI: Add function for parsing `slot-power-limit-milliwatt` DT property
> >   dt-bindings: PCI: aardvark: Describe slot-power-limit-milliwatt
> >   PCI: aardvark: Send Set_Slot_Power_Limit message
> >   arm64: dts: armada-3720-turris-mox: Define slot-power-limit-milliwatt
> >     for PCIe
> >   PCI: aardvark: Run link training in separate worker
> >   PCI: aardvark: Optimize PCIe card reset via GPIO
> > 
> > Russell King (2):
> >   PCI: pci-bridge-emul: Re-arrange register tests
> >   PCI: pci-bridge-emul: Add support for PCIe extended capabilities
> > 
> >  .../devicetree/bindings/pci/aardvark-pci.txt  |   2 +
> >  .../dts/marvell/armada-3720-turris-mox.dts    |   1 +
> >  arch/arm64/boot/dts/marvell/armada-37xx.dtsi  |   1 +
> >  drivers/pci/controller/pci-aardvark.c         | 380 ++++++++++++++++--
> >  drivers/pci/hotplug/pciehp_hpc.c              |  34 +-
> >  drivers/pci/hotplug/pnv_php.c                 |  13 +-
> >  drivers/pci/of.c                              |  64 +++
> >  drivers/pci/pci-bridge-emul.c                 | 130 +++---
> >  drivers/pci/pci-bridge-emul.h                 |  15 +
> >  drivers/pci/pci.h                             |  15 +
> >  include/uapi/linux/pci_regs.h                 |   4 +
> >  11 files changed, 565 insertions(+), 94 deletions(-)
> > 
> > -- 
> > 2.34.1
> > 

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 16/18] PCI: aardvark: Add suspend to RAM support
  2022-02-20 19:33 ` [PATCH 16/18] PCI: aardvark: Add suspend to RAM support Marek Behún
@ 2022-04-12 11:14   ` Lorenzo Pieralisi
  0 siblings, 0 replies; 44+ messages in thread
From: Lorenzo Pieralisi @ 2022-04-12 11:14 UTC (permalink / raw)
  To: Marek Behún
  Cc: Bjorn Helgaas, Krzysztof Wilczyński, Marc Zyngier, pali,
	linux-pci, linux-arm-kernel, Gregory CLEMENT, Miquel Raynal,
	rafael

[+Rafael]

On Sun, Feb 20, 2022 at 08:33:44PM +0100, Marek Behún wrote:
> From: Miquel Raynal <miquel.raynal@bootlin.com>
> 
> Add suspend and resume callbacks. The priority of these are
> "_noirq()", to workaround early access to the registers done by the
> PCI core through the ->read()/->write() callbacks at resume time.

This commit log should be rewritten to clarify it. IIUC, you are hooking
up the _noirq() callbacks to take advantage of callbacks ordering rather
than needing the IRQ disabled per-se.

Thread here:
https://lore.kernel.org/linux-pci/20220220193346.23789-17-kabel@kernel.org

I'd ask Rafael please if this is common practice or a workaround and
whether that's how it should be done.

Thanks,
Lorenzo

> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Signed-off-by: Marek Behún <kabel@kernel.org>
> ---
>  drivers/pci/controller/pci-aardvark.c | 39 +++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
> 
> diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
> index 3b51f47abd72..8c9ac7766ac7 100644
> --- a/drivers/pci/controller/pci-aardvark.c
> +++ b/drivers/pci/controller/pci-aardvark.c
> @@ -1896,6 +1896,44 @@ static int advk_pcie_setup_phy(struct advk_pcie *pcie)
>  	return ret;
>  }
>  
> +static int __maybe_unused advk_pcie_suspend(struct device *dev)
> +{
> +	struct advk_pcie *pcie = dev_get_drvdata(dev);
> +
> +	advk_pcie_disable_phy(pcie);
> +
> +	clk_disable_unprepare(pcie->clk);
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused advk_pcie_resume(struct device *dev)
> +{
> +	struct advk_pcie *pcie = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = clk_prepare_enable(pcie->clk);
> +	if (ret)
> +		return ret;
> +
> +	ret = advk_pcie_enable_phy(pcie);
> +	if (ret)
> +		return ret;
> +
> +	advk_pcie_setup_hw(pcie);
> +
> +	return 0;
> +}
> +
> +/*
> + * The PCI core will try to reconfigure the bus quite early in the resume path.
> + * We must use the _noirq() alternatives to ensure the controller is ready when
> + * the core uses the ->read()/->write() callbacks.
> + */
> +static const struct dev_pm_ops advk_pcie_dev_pm_ops = {
> +	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(advk_pcie_suspend, advk_pcie_resume)
> +};
> +
>  static int advk_pcie_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -2171,6 +2209,7 @@ static struct platform_driver advk_pcie_driver = {
>  	.driver = {
>  		.name = "advk-pcie",
>  		.of_match_table = advk_pcie_of_match_table,
> +		.pm = &advk_pcie_dev_pm_ops,
>  	},
>  	.probe = advk_pcie_probe,
>  	.remove = advk_pcie_remove,
> -- 
> 2.34.1
> 

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 17/18] PCI: aardvark: Run link training in separate worker
  2022-02-20 19:33 ` [PATCH 17/18] PCI: aardvark: Run link training in separate worker Marek Behún
@ 2022-04-12 15:25   ` Lorenzo Pieralisi
  2022-04-12 17:55     ` Pali Rohár
  0 siblings, 1 reply; 44+ messages in thread
From: Lorenzo Pieralisi @ 2022-04-12 15:25 UTC (permalink / raw)
  To: Marek Behún
  Cc: Bjorn Helgaas, Krzysztof Wilczyński, Marc Zyngier, pali,
	linux-pci, linux-arm-kernel, Gregory CLEMENT

On Sun, Feb 20, 2022 at 08:33:45PM +0100, Marek Behún wrote:
> From: Pali Rohár <pali@kernel.org>
> 
> Link training and PCIe card reset routines in Aardvark contain several
> delays, resulting in rather slow PCIe card probing. The worst case is
> when there is no card connected: the driver tries link training at all
> possible speeds and waits until all timers expire.
> 
> Since probe methods for all system devices are called sequentially, this
> results in noticeably longer boot time.
> 
> Move card reset and link training code from driver probe function into
> a separate worker, so that kernel can do something different while the
> driver is waiting during reset or training.
> 
> On ESPRESSObin and Turris MOX this decreases boot time by 0.4s with
> plugged PCIe card and by 2.2s if no card is connected.

I believe this is what the PROBE_PREFER_ASYNCHRONOUS flag in
struct device_driver.probe_type flag is there for unless I am
missing something obvious here.

Can you give it a try and report back please ?

Thanks,
Lorenzo

> Signed-off-by: Pali Rohár <pali@kernel.org>
> Signed-off-by: Marek Behún <kabel@kernel.org>
> ---
>  drivers/pci/controller/pci-aardvark.c | 42 ++++++++++++++++++---------
>  1 file changed, 28 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
> index 8c9ac7766ac7..056f49d0e3a4 100644
> --- a/drivers/pci/controller/pci-aardvark.c
> +++ b/drivers/pci/controller/pci-aardvark.c
> @@ -26,6 +26,7 @@
>  #include <linux/of_gpio.h>
>  #include <linux/of_pci.h>
>  #include <linux/timer.h>
> +#include <linux/workqueue.h>
>  
>  #include "../pci.h"
>  #include "../pci-bridge-emul.h"
> @@ -296,6 +297,8 @@ struct advk_pcie {
>  	int link_gen;
>  	bool link_was_up;
>  	struct timer_list link_irq_timer;
> +	struct delayed_work probe_card_work;
> +	bool host_bridge_probed;
>  	struct pci_bridge_emul bridge;
>  	struct gpio_desc *reset_gpio;
>  	struct clk *clk;
> @@ -497,6 +500,21 @@ static void advk_pcie_train_link(struct advk_pcie *pcie)
>  		dev_err(dev, "link never came up\n");
>  }
>  
> +static void advk_pcie_probe_card_work(struct work_struct *work)
> +{
> +	struct delayed_work *dwork = container_of(work, struct delayed_work,
> +						  work);
> +	struct advk_pcie *pcie = container_of(dwork, struct advk_pcie,
> +					      probe_card_work);
> +	struct pci_host_bridge *bridge = pci_host_bridge_from_priv(pcie);
> +	int ret;
> +
> +	advk_pcie_train_link(pcie);
> +	ret = pci_host_probe(bridge);
> +	if (!ret)
> +		pcie->host_bridge_probed = true;
> +}
> +
>  /*
>   * Set PCIe address window register which could be used for memory
>   * mapping.
> @@ -701,8 +719,6 @@ static void advk_pcie_setup_hw(struct advk_pcie *pcie)
>  	/* Disable remaining PCIe outbound windows */
>  	for (i = pcie->wins_count; i < OB_WIN_COUNT; i++)
>  		advk_pcie_disable_ob_win(pcie, i);
> -
> -	advk_pcie_train_link(pcie);
>  }
>  
>  static int advk_pcie_check_pio_status(struct advk_pcie *pcie, bool allow_crs, u32 *val)
> @@ -2112,14 +2128,8 @@ static int advk_pcie_probe(struct platform_device *pdev)
>  	bridge->ops = &advk_pcie_ops;
>  	bridge->map_irq = advk_pcie_map_irq;
>  
> -	ret = pci_host_probe(bridge);
> -	if (ret < 0) {
> -		irq_set_chained_handler_and_data(pcie->irq, NULL, NULL);
> -		advk_pcie_remove_rp_irq_domain(pcie);
> -		advk_pcie_remove_msi_irq_domain(pcie);
> -		advk_pcie_remove_irq_domain(pcie);
> -		return ret;
> -	}
> +	INIT_DELAYED_WORK(&pcie->probe_card_work, advk_pcie_probe_card_work);
> +	schedule_delayed_work(&pcie->probe_card_work, 1);
>  
>  	return 0;
>  }
> @@ -2131,11 +2141,15 @@ static int advk_pcie_remove(struct platform_device *pdev)
>  	u32 val;
>  	int i;
>  
> +	cancel_delayed_work_sync(&pcie->probe_card_work);
> +
>  	/* Remove PCI bus with all devices */
> -	pci_lock_rescan_remove();
> -	pci_stop_root_bus(bridge->bus);
> -	pci_remove_root_bus(bridge->bus);
> -	pci_unlock_rescan_remove();
> +	if (pcie->host_bridge_probed) {
> +		pci_lock_rescan_remove();
> +		pci_stop_root_bus(bridge->bus);
> +		pci_remove_root_bus(bridge->bus);
> +		pci_unlock_rescan_remove();
> +	}
>  
>  	/* Disable Root Bridge I/O space, memory space and bus mastering */
>  	val = advk_readl(pcie, PCIE_CORE_CMD_STATUS_REG);
> -- 
> 2.34.1
> 

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 17/18] PCI: aardvark: Run link training in separate worker
  2022-04-12 15:25   ` Lorenzo Pieralisi
@ 2022-04-12 17:55     ` Pali Rohár
  2022-04-13  9:16       ` Lorenzo Pieralisi
  0 siblings, 1 reply; 44+ messages in thread
From: Pali Rohár @ 2022-04-12 17:55 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: Marek Behún, Bjorn Helgaas, Krzysztof Wilczyński,
	Marc Zyngier, linux-pci, linux-arm-kernel, Gregory CLEMENT

On Tuesday 12 April 2022 16:25:24 Lorenzo Pieralisi wrote:
> On Sun, Feb 20, 2022 at 08:33:45PM +0100, Marek Behún wrote:
> > From: Pali Rohár <pali@kernel.org>
> > 
> > Link training and PCIe card reset routines in Aardvark contain several
> > delays, resulting in rather slow PCIe card probing. The worst case is
> > when there is no card connected: the driver tries link training at all
> > possible speeds and waits until all timers expire.
> > 
> > Since probe methods for all system devices are called sequentially, this
> > results in noticeably longer boot time.
> > 
> > Move card reset and link training code from driver probe function into
> > a separate worker, so that kernel can do something different while the
> > driver is waiting during reset or training.
> > 
> > On ESPRESSObin and Turris MOX this decreases boot time by 0.4s with
> > plugged PCIe card and by 2.2s if no card is connected.
> 
> I believe this is what the PROBE_PREFER_ASYNCHRONOUS flag in
> struct device_driver.probe_type flag is there for unless I am
> missing something obvious here.
> 
> Can you give it a try and report back please ?

Hello Lorenzo.

During testing patches 17 and 18 I saw that following race condition
https://lore.kernel.org/linux-pci/20210407144146.rl7x2h5l2cc3escy@pali/
(which cause kernel oops) was triggered more often.

I'm not sure if above race condition was fully fixed by the last
Krzysztof's patches or there is also other issue which cause oops.

As both patches 17 and 18 are just optimizations, I would suggest to
skip it for now, until all these issues are resolved or verified that
they are not triggered anymore.

I guess that at this time we can look at PROBE_PREFER_ASYNCHRONOUS flag
and decide how to implement this optimization.

Do you agree, or do you have other opinion?

> Thanks,
> Lorenzo
> 
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > Signed-off-by: Marek Behún <kabel@kernel.org>
> > ---
> >  drivers/pci/controller/pci-aardvark.c | 42 ++++++++++++++++++---------
> >  1 file changed, 28 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
> > index 8c9ac7766ac7..056f49d0e3a4 100644
> > --- a/drivers/pci/controller/pci-aardvark.c
> > +++ b/drivers/pci/controller/pci-aardvark.c
> > @@ -26,6 +26,7 @@
> >  #include <linux/of_gpio.h>
> >  #include <linux/of_pci.h>
> >  #include <linux/timer.h>
> > +#include <linux/workqueue.h>
> >  
> >  #include "../pci.h"
> >  #include "../pci-bridge-emul.h"
> > @@ -296,6 +297,8 @@ struct advk_pcie {
> >  	int link_gen;
> >  	bool link_was_up;
> >  	struct timer_list link_irq_timer;
> > +	struct delayed_work probe_card_work;
> > +	bool host_bridge_probed;
> >  	struct pci_bridge_emul bridge;
> >  	struct gpio_desc *reset_gpio;
> >  	struct clk *clk;
> > @@ -497,6 +500,21 @@ static void advk_pcie_train_link(struct advk_pcie *pcie)
> >  		dev_err(dev, "link never came up\n");
> >  }
> >  
> > +static void advk_pcie_probe_card_work(struct work_struct *work)
> > +{
> > +	struct delayed_work *dwork = container_of(work, struct delayed_work,
> > +						  work);
> > +	struct advk_pcie *pcie = container_of(dwork, struct advk_pcie,
> > +					      probe_card_work);
> > +	struct pci_host_bridge *bridge = pci_host_bridge_from_priv(pcie);
> > +	int ret;
> > +
> > +	advk_pcie_train_link(pcie);
> > +	ret = pci_host_probe(bridge);
> > +	if (!ret)
> > +		pcie->host_bridge_probed = true;
> > +}
> > +
> >  /*
> >   * Set PCIe address window register which could be used for memory
> >   * mapping.
> > @@ -701,8 +719,6 @@ static void advk_pcie_setup_hw(struct advk_pcie *pcie)
> >  	/* Disable remaining PCIe outbound windows */
> >  	for (i = pcie->wins_count; i < OB_WIN_COUNT; i++)
> >  		advk_pcie_disable_ob_win(pcie, i);
> > -
> > -	advk_pcie_train_link(pcie);
> >  }
> >  
> >  static int advk_pcie_check_pio_status(struct advk_pcie *pcie, bool allow_crs, u32 *val)
> > @@ -2112,14 +2128,8 @@ static int advk_pcie_probe(struct platform_device *pdev)
> >  	bridge->ops = &advk_pcie_ops;
> >  	bridge->map_irq = advk_pcie_map_irq;
> >  
> > -	ret = pci_host_probe(bridge);
> > -	if (ret < 0) {
> > -		irq_set_chained_handler_and_data(pcie->irq, NULL, NULL);
> > -		advk_pcie_remove_rp_irq_domain(pcie);
> > -		advk_pcie_remove_msi_irq_domain(pcie);
> > -		advk_pcie_remove_irq_domain(pcie);
> > -		return ret;
> > -	}
> > +	INIT_DELAYED_WORK(&pcie->probe_card_work, advk_pcie_probe_card_work);
> > +	schedule_delayed_work(&pcie->probe_card_work, 1);
> >  
> >  	return 0;
> >  }
> > @@ -2131,11 +2141,15 @@ static int advk_pcie_remove(struct platform_device *pdev)
> >  	u32 val;
> >  	int i;
> >  
> > +	cancel_delayed_work_sync(&pcie->probe_card_work);
> > +
> >  	/* Remove PCI bus with all devices */
> > -	pci_lock_rescan_remove();
> > -	pci_stop_root_bus(bridge->bus);
> > -	pci_remove_root_bus(bridge->bus);
> > -	pci_unlock_rescan_remove();
> > +	if (pcie->host_bridge_probed) {
> > +		pci_lock_rescan_remove();
> > +		pci_stop_root_bus(bridge->bus);
> > +		pci_remove_root_bus(bridge->bus);
> > +		pci_unlock_rescan_remove();
> > +	}
> >  
> >  	/* Disable Root Bridge I/O space, memory space and bus mastering */
> >  	val = advk_readl(pcie, PCIE_CORE_CMD_STATUS_REG);
> > -- 
> > 2.34.1
> > 

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 17/18] PCI: aardvark: Run link training in separate worker
  2022-04-12 17:55     ` Pali Rohár
@ 2022-04-13  9:16       ` Lorenzo Pieralisi
  2022-05-04 14:02         ` Marek Behún
  0 siblings, 1 reply; 44+ messages in thread
From: Lorenzo Pieralisi @ 2022-04-13  9:16 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Marek Behún, Bjorn Helgaas, Krzysztof Wilczyński,
	Marc Zyngier, linux-pci, linux-arm-kernel, Gregory CLEMENT

On Tue, Apr 12, 2022 at 07:55:41PM +0200, Pali Rohár wrote:
> On Tuesday 12 April 2022 16:25:24 Lorenzo Pieralisi wrote:
> > On Sun, Feb 20, 2022 at 08:33:45PM +0100, Marek Behún wrote:
> > > From: Pali Rohár <pali@kernel.org>
> > > 
> > > Link training and PCIe card reset routines in Aardvark contain several
> > > delays, resulting in rather slow PCIe card probing. The worst case is
> > > when there is no card connected: the driver tries link training at all
> > > possible speeds and waits until all timers expire.
> > > 
> > > Since probe methods for all system devices are called sequentially, this
> > > results in noticeably longer boot time.
> > > 
> > > Move card reset and link training code from driver probe function into
> > > a separate worker, so that kernel can do something different while the
> > > driver is waiting during reset or training.
> > > 
> > > On ESPRESSObin and Turris MOX this decreases boot time by 0.4s with
> > > plugged PCIe card and by 2.2s if no card is connected.
> > 
> > I believe this is what the PROBE_PREFER_ASYNCHRONOUS flag in
> > struct device_driver.probe_type flag is there for unless I am
> > missing something obvious here.
> > 
> > Can you give it a try and report back please ?
> 
> Hello Lorenzo.
> 
> During testing patches 17 and 18 I saw that following race condition
> https://lore.kernel.org/linux-pci/20210407144146.rl7x2h5l2cc3escy@pali/
> (which cause kernel oops) was triggered more often.
> 
> I'm not sure if above race condition was fully fixed by the last
> Krzysztof's patches or there is also other issue which cause oops.
> 
> As both patches 17 and 18 are just optimizations, I would suggest to
> skip it for now, until all these issues are resolved or verified that
> they are not triggered anymore.
> 
> I guess that at this time we can look at PROBE_PREFER_ASYNCHRONOUS flag
> and decide how to implement this optimization.
> 
> Do you agree, or do you have other opinion?

It is fine by me - I will consider other patches in the series.

Lorenzo

> > Thanks,
> > Lorenzo
> > 
> > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > Signed-off-by: Marek Behún <kabel@kernel.org>
> > > ---
> > >  drivers/pci/controller/pci-aardvark.c | 42 ++++++++++++++++++---------
> > >  1 file changed, 28 insertions(+), 14 deletions(-)
> > > 
> > > diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
> > > index 8c9ac7766ac7..056f49d0e3a4 100644
> > > --- a/drivers/pci/controller/pci-aardvark.c
> > > +++ b/drivers/pci/controller/pci-aardvark.c
> > > @@ -26,6 +26,7 @@
> > >  #include <linux/of_gpio.h>
> > >  #include <linux/of_pci.h>
> > >  #include <linux/timer.h>
> > > +#include <linux/workqueue.h>
> > >  
> > >  #include "../pci.h"
> > >  #include "../pci-bridge-emul.h"
> > > @@ -296,6 +297,8 @@ struct advk_pcie {
> > >  	int link_gen;
> > >  	bool link_was_up;
> > >  	struct timer_list link_irq_timer;
> > > +	struct delayed_work probe_card_work;
> > > +	bool host_bridge_probed;
> > >  	struct pci_bridge_emul bridge;
> > >  	struct gpio_desc *reset_gpio;
> > >  	struct clk *clk;
> > > @@ -497,6 +500,21 @@ static void advk_pcie_train_link(struct advk_pcie *pcie)
> > >  		dev_err(dev, "link never came up\n");
> > >  }
> > >  
> > > +static void advk_pcie_probe_card_work(struct work_struct *work)
> > > +{
> > > +	struct delayed_work *dwork = container_of(work, struct delayed_work,
> > > +						  work);
> > > +	struct advk_pcie *pcie = container_of(dwork, struct advk_pcie,
> > > +					      probe_card_work);
> > > +	struct pci_host_bridge *bridge = pci_host_bridge_from_priv(pcie);
> > > +	int ret;
> > > +
> > > +	advk_pcie_train_link(pcie);
> > > +	ret = pci_host_probe(bridge);
> > > +	if (!ret)
> > > +		pcie->host_bridge_probed = true;
> > > +}
> > > +
> > >  /*
> > >   * Set PCIe address window register which could be used for memory
> > >   * mapping.
> > > @@ -701,8 +719,6 @@ static void advk_pcie_setup_hw(struct advk_pcie *pcie)
> > >  	/* Disable remaining PCIe outbound windows */
> > >  	for (i = pcie->wins_count; i < OB_WIN_COUNT; i++)
> > >  		advk_pcie_disable_ob_win(pcie, i);
> > > -
> > > -	advk_pcie_train_link(pcie);
> > >  }
> > >  
> > >  static int advk_pcie_check_pio_status(struct advk_pcie *pcie, bool allow_crs, u32 *val)
> > > @@ -2112,14 +2128,8 @@ static int advk_pcie_probe(struct platform_device *pdev)
> > >  	bridge->ops = &advk_pcie_ops;
> > >  	bridge->map_irq = advk_pcie_map_irq;
> > >  
> > > -	ret = pci_host_probe(bridge);
> > > -	if (ret < 0) {
> > > -		irq_set_chained_handler_and_data(pcie->irq, NULL, NULL);
> > > -		advk_pcie_remove_rp_irq_domain(pcie);
> > > -		advk_pcie_remove_msi_irq_domain(pcie);
> > > -		advk_pcie_remove_irq_domain(pcie);
> > > -		return ret;
> > > -	}
> > > +	INIT_DELAYED_WORK(&pcie->probe_card_work, advk_pcie_probe_card_work);
> > > +	schedule_delayed_work(&pcie->probe_card_work, 1);
> > >  
> > >  	return 0;
> > >  }
> > > @@ -2131,11 +2141,15 @@ static int advk_pcie_remove(struct platform_device *pdev)
> > >  	u32 val;
> > >  	int i;
> > >  
> > > +	cancel_delayed_work_sync(&pcie->probe_card_work);
> > > +
> > >  	/* Remove PCI bus with all devices */
> > > -	pci_lock_rescan_remove();
> > > -	pci_stop_root_bus(bridge->bus);
> > > -	pci_remove_root_bus(bridge->bus);
> > > -	pci_unlock_rescan_remove();
> > > +	if (pcie->host_bridge_probed) {
> > > +		pci_lock_rescan_remove();
> > > +		pci_stop_root_bus(bridge->bus);
> > > +		pci_remove_root_bus(bridge->bus);
> > > +		pci_unlock_rescan_remove();
> > > +	}
> > >  
> > >  	/* Disable Root Bridge I/O space, memory space and bus mastering */
> > >  	val = advk_readl(pcie, PCIE_CORE_CMD_STATUS_REG);
> > > -- 
> > > 2.34.1
> > > 

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 04/18] PCI: Add PCI_EXP_SLTCAP_*_SHIFT macros
  2022-02-20 19:33 ` [PATCH 04/18] PCI: Add PCI_EXP_SLTCAP_*_SHIFT macros Marek Behún
@ 2022-04-28 11:09   ` Lorenzo Pieralisi
  2022-04-28 11:16     ` Pali Rohár
  2022-05-18 19:23   ` Bjorn Helgaas
  1 sibling, 1 reply; 44+ messages in thread
From: Lorenzo Pieralisi @ 2022-04-28 11:09 UTC (permalink / raw)
  To: Marek Behún
  Cc: Bjorn Helgaas, Krzysztof Wilczyński, Marc Zyngier, pali,
	linux-pci, linux-arm-kernel, Gregory CLEMENT

On Sun, Feb 20, 2022 at 08:33:32PM +0100, Marek Behún wrote:
> From: Pali Rohár <pali@kernel.org>
> 
> These macros allows to easily compose and extract Slot Power Limit and
> Physical Slot Number values from Slot Capability Register.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Signed-off-by: Marek Behún <kabel@kernel.org>
> ---
>  include/uapi/linux/pci_regs.h | 3 +++
>  1 file changed, 3 insertions(+)

This patch can be dropped, correct ?

Thanks,
Lorenzo

> 
> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> index bee1a9ed6e66..d825e17e448c 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -591,10 +591,13 @@
>  #define  PCI_EXP_SLTCAP_HPS	0x00000020 /* Hot-Plug Surprise */
>  #define  PCI_EXP_SLTCAP_HPC	0x00000040 /* Hot-Plug Capable */
>  #define  PCI_EXP_SLTCAP_SPLV	0x00007f80 /* Slot Power Limit Value */
> +#define  PCI_EXP_SLTCAP_SPLV_SHIFT	7  /* Slot Power Limit Value shift */
>  #define  PCI_EXP_SLTCAP_SPLS	0x00018000 /* Slot Power Limit Scale */
> +#define  PCI_EXP_SLTCAP_SPLS_SHIFT	15 /* Slot Power Limit Scale shift */
>  #define  PCI_EXP_SLTCAP_EIP	0x00020000 /* Electromechanical Interlock Present */
>  #define  PCI_EXP_SLTCAP_NCCS	0x00040000 /* No Command Completed Support */
>  #define  PCI_EXP_SLTCAP_PSN	0xfff80000 /* Physical Slot Number */
> +#define  PCI_EXP_SLTCAP_PSN_SHIFT	19 /* Physical Slot Number shift */
>  #define PCI_EXP_SLTCTL		0x18	/* Slot Control */
>  #define  PCI_EXP_SLTCTL_ABPE	0x0001	/* Attention Button Pressed Enable */
>  #define  PCI_EXP_SLTCTL_PFDE	0x0002	/* Power Fault Detected Enable */
> -- 
> 2.34.1
> 

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 04/18] PCI: Add PCI_EXP_SLTCAP_*_SHIFT macros
  2022-04-28 11:09   ` Lorenzo Pieralisi
@ 2022-04-28 11:16     ` Pali Rohár
  0 siblings, 0 replies; 44+ messages in thread
From: Pali Rohár @ 2022-04-28 11:16 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: Marek Behún, Bjorn Helgaas, Krzysztof Wilczyński,
	Marc Zyngier, linux-pci, linux-arm-kernel, Gregory CLEMENT

On Thursday 28 April 2022 12:09:26 Lorenzo Pieralisi wrote:
> On Sun, Feb 20, 2022 at 08:33:32PM +0100, Marek Behún wrote:
> > From: Pali Rohár <pali@kernel.org>
> > 
> > These macros allows to easily compose and extract Slot Power Limit and
> > Physical Slot Number values from Slot Capability Register.
> > 
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > Signed-off-by: Marek Behún <kabel@kernel.org>
> > ---
> >  include/uapi/linux/pci_regs.h | 3 +++
> >  1 file changed, 3 insertions(+)
> 
> This patch can be dropped, correct ?

Yes!

And note that 'slot-power-limit-milliwatt' DT property patch you took
into pci/power-slot branch.

> Thanks,
> Lorenzo
> 
> > 
> > diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> > index bee1a9ed6e66..d825e17e448c 100644
> > --- a/include/uapi/linux/pci_regs.h
> > +++ b/include/uapi/linux/pci_regs.h
> > @@ -591,10 +591,13 @@
> >  #define  PCI_EXP_SLTCAP_HPS	0x00000020 /* Hot-Plug Surprise */
> >  #define  PCI_EXP_SLTCAP_HPC	0x00000040 /* Hot-Plug Capable */
> >  #define  PCI_EXP_SLTCAP_SPLV	0x00007f80 /* Slot Power Limit Value */
> > +#define  PCI_EXP_SLTCAP_SPLV_SHIFT	7  /* Slot Power Limit Value shift */
> >  #define  PCI_EXP_SLTCAP_SPLS	0x00018000 /* Slot Power Limit Scale */
> > +#define  PCI_EXP_SLTCAP_SPLS_SHIFT	15 /* Slot Power Limit Scale shift */
> >  #define  PCI_EXP_SLTCAP_EIP	0x00020000 /* Electromechanical Interlock Present */
> >  #define  PCI_EXP_SLTCAP_NCCS	0x00040000 /* No Command Completed Support */
> >  #define  PCI_EXP_SLTCAP_PSN	0xfff80000 /* Physical Slot Number */
> > +#define  PCI_EXP_SLTCAP_PSN_SHIFT	19 /* Physical Slot Number shift */
> >  #define PCI_EXP_SLTCTL		0x18	/* Slot Control */
> >  #define  PCI_EXP_SLTCTL_ABPE	0x0001	/* Attention Button Pressed Enable */
> >  #define  PCI_EXP_SLTCTL_PFDE	0x0002	/* Power Fault Detected Enable */
> > -- 
> > 2.34.1
> > 

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 17/18] PCI: aardvark: Run link training in separate worker
  2022-04-13  9:16       ` Lorenzo Pieralisi
@ 2022-05-04 14:02         ` Marek Behún
  0 siblings, 0 replies; 44+ messages in thread
From: Marek Behún @ 2022-05-04 14:02 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: Pali Rohár, Bjorn Helgaas, linux-pci, linux-arm-kernel

On Wed, 13 Apr 2022 10:16:03 +0100
Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote:

> 
> It is fine by me - I will consider other patches in the series.
> 
> Lorenzo

Hello Lorenzo,

did you have time to look into the other patches?

Thanks,

Marek

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 06/18] PCI: pciehp: Enable DLLSC interrupt only if supported
  2022-02-20 19:33 ` [PATCH 06/18] PCI: pciehp: Enable DLLSC interrupt only if supported Marek Behún
@ 2022-05-09  3:42   ` Lukas Wunner
  2022-05-13 16:57     ` Pali Rohár
  0 siblings, 1 reply; 44+ messages in thread
From: Lukas Wunner @ 2022-05-09  3:42 UTC (permalink / raw)
  To: Marek Behún
  Cc: Lorenzo Pieralisi, Bjorn Helgaas, Krzysztof Wilczy??ski,
	Marc Zyngier, pali, linux-pci, linux-arm-kernel, Gregory CLEMENT

On Sun, Feb 20, 2022 at 08:33:34PM +0100, Marek Behún wrote:
> --- a/drivers/pci/hotplug/pciehp_hpc.c
> +++ b/drivers/pci/hotplug/pciehp_hpc.c
> @@ -788,6 +788,7 @@ static int pciehp_poll(void *data)
> @@ -800,12 +801,17 @@ static void pcie_enable_notification(struct controller *ctrl)
>  	 * next power fault detected interrupt was notified again.
>  	 */
>  
> +	pcie_capability_read_dword(ctrl_dev(ctrl), PCI_EXP_LNKCAP, &link_cap);
>  	/*
> -	 * Always enable link events: thus link-up and link-down shall
> -	 * always be treated as hotplug and unplug respectively. Enable
> -	 * presence detect only if Attention Button is not present.
> +	 * Enable link events if their support is indicated in Link Capability
> +	 * register: thus link-up and link-down shall always be treated as
> +	 * hotplug and unplug respectively. Enable presence detect only if
> +	 * Attention Button is not present.
>  	 */
> -	cmd = PCI_EXP_SLTCTL_DLLSCE;
> +	cmd = 0;
> +	if (link_cap & PCI_EXP_LNKCAP_DLLLARC)
> +		cmd |= PCI_EXP_SLTCTL_DLLSCE;

The Data Link Layer Link Active Reporting Capable bit is cached
in ctrl_dev(ctrl)->link_active_reporting.  Please use that
instead of re-reading it from the register.

According to PCIe r6.0, sec. 7.5.3.6, "For a hot-plug capable
Downstream Port [...], this bit must be hardwired to 1b."

That has been part of the spec since PCIe r1.1, sec. 7.8.6.

PCIe r1.0 did not contain the sentence because it did not support
DLLLARC (it defined bit 20 as RsvdP).

In other words, what you're doing here is add support for PCIe r1.0.
I'm not opposed to that, but I'd assume that aardvark supports a
more recent spec version.  More likely it doesn't comply with the spec?

What is the user-visible issue that you're experiencing without this
commit?  Does aardvark somehow misbehave if the DLLLARC bit is set to 1?
Since the bit is RsvdP, setting it shouldn't have any negative side
effects.


> --- a/drivers/pci/hotplug/pnv_php.c
> +++ b/drivers/pci/hotplug/pnv_php.c
> @@ -840,6 +840,7 @@ static void pnv_php_init_irq(struct pnv_php_slot *php_slot, int irq)
>  	struct pci_dev *pdev = php_slot->pdev;
>  	u32 broken_pdc = 0;
>  	u16 sts, ctrl;
> +	u32 link_cap;
>  	int ret;
>  
>  	/* Allocate workqueue */

pnv_php.c is a driver for PowerNV, yet this patch is for a series
targeting an ARM PCIe controller.  That doesn't make sense,
changes to pnv_php.c seem wrong here.

Thanks,

Lukas

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 07/18] PCI: pciehp: Enable Command Completed Interrupt only if supported
  2022-02-20 19:33 ` [PATCH 07/18] PCI: pciehp: Enable Command Completed Interrupt " Marek Behún
@ 2022-05-09  4:01   ` Lukas Wunner
  2022-05-13 16:59     ` Pali Rohár
  0 siblings, 1 reply; 44+ messages in thread
From: Lukas Wunner @ 2022-05-09  4:01 UTC (permalink / raw)
  To: Marek Behún
  Cc: Lorenzo Pieralisi, Bjorn Helgaas, Krzysztof Wilczy??ski,
	Marc Zyngier, pali, linux-pci, linux-arm-kernel, Gregory CLEMENT

On Sun, Feb 20, 2022 at 08:33:35PM +0100, Marek Behún wrote:
> The No Command Completed Support bit in the Slot Capabilities register
> indicates whether Command Completed Interrupt Enable is unsupported.
> 
> Enable this interrupt only in the case it is supported.
[...]
> --- a/drivers/pci/hotplug/pciehp_hpc.c
> +++ b/drivers/pci/hotplug/pciehp_hpc.c
> @@ -817,7 +817,9 @@ static void pcie_enable_notification(struct controller *ctrl)
>  	else
>  		cmd |= PCI_EXP_SLTCTL_PDCE;
>  	if (!pciehp_poll_mode)
> -		cmd |= PCI_EXP_SLTCTL_HPIE | PCI_EXP_SLTCTL_CCIE;
> +		cmd |= PCI_EXP_SLTCTL_HPIE;
> +	if (!pciehp_poll_mode && !NO_CMD_CMPL(ctrl))
> +		cmd |= PCI_EXP_SLTCTL_CCIE;

Looks okay to me in principle, I'm just wondering why this change is
necessary, i.e. what issue are you seeing without it?

Thanks,

Lukas

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 00/18] PCI: aardvark controller changes BATCH 5
  2022-02-20 19:33 [PATCH 00/18] PCI: aardvark controller changes BATCH 5 Marek Behún
                   ` (18 preceding siblings ...)
  2022-04-11 15:36 ` [PATCH 00/18] PCI: aardvark controller changes BATCH 5 Lorenzo Pieralisi
@ 2022-05-13 10:33 ` Lorenzo Pieralisi
  2022-05-13 16:48   ` Pali Rohár
  2022-05-18 15:54 ` (subset) " Lorenzo Pieralisi
  2022-08-16 16:25 ` Lorenzo Pieralisi
  21 siblings, 1 reply; 44+ messages in thread
From: Lorenzo Pieralisi @ 2022-05-13 10:33 UTC (permalink / raw)
  To: Marek Behún
  Cc: Bjorn Helgaas, Krzysztof Wilczyński, Marc Zyngier, pali,
	linux-pci, linux-arm-kernel, Gregory CLEMENT

On Sun, Feb 20, 2022 at 08:33:28PM +0100, Marek Behún wrote:
> Hello Lorenzo, Krzysztof,
> 
> here comes batch 5 of changes for Aardvark PCIe controller.
> 
> This batch
> - adds support for AER
> - adds support for DLLSC and hotplug interrupt
> - adds support for sending slot power limit message
> - adds enabling/disabling PCIe clock
> - adds suspend support
> - optimizes link training by adding it into separate worker
> - optimizes GPIO resetting by asserting it only if it wasn't asserted
>   already

I had a look and I can take patches 3 and 5 to cut the delta further,
please let me know if that helps.

Thanks,
Lorenzo

> Marek
> 
> Marek Behún (1):
>   arm64: dts: marvell: armada-37xx: Add clock to PCIe node
> 
> Miquel Raynal (2):
>   PCI: aardvark: Add clock support
>   PCI: aardvark: Add suspend to RAM support
> 
> Pali Rohár (13):
>   PCI: aardvark: Add support for AER registers on emulated bridge
>   PCI: Add PCI_EXP_SLTCAP_*_SHIFT macros
>   PCI: aardvark: Fix reporting Slot capabilities on emulated bridge
>   PCI: pciehp: Enable DLLSC interrupt only if supported
>   PCI: pciehp: Enable Command Completed Interrupt only if supported
>   PCI: aardvark: Add support for DLLSC and hotplug interrupt
>   PCI: Add PCI_EXP_SLTCTL_ASPL_DISABLE macro
>   PCI: Add function for parsing `slot-power-limit-milliwatt` DT property
>   dt-bindings: PCI: aardvark: Describe slot-power-limit-milliwatt
>   PCI: aardvark: Send Set_Slot_Power_Limit message
>   arm64: dts: armada-3720-turris-mox: Define slot-power-limit-milliwatt
>     for PCIe
>   PCI: aardvark: Run link training in separate worker
>   PCI: aardvark: Optimize PCIe card reset via GPIO
> 
> Russell King (2):
>   PCI: pci-bridge-emul: Re-arrange register tests
>   PCI: pci-bridge-emul: Add support for PCIe extended capabilities
> 
>  .../devicetree/bindings/pci/aardvark-pci.txt  |   2 +
>  .../dts/marvell/armada-3720-turris-mox.dts    |   1 +
>  arch/arm64/boot/dts/marvell/armada-37xx.dtsi  |   1 +
>  drivers/pci/controller/pci-aardvark.c         | 380 ++++++++++++++++--
>  drivers/pci/hotplug/pciehp_hpc.c              |  34 +-
>  drivers/pci/hotplug/pnv_php.c                 |  13 +-
>  drivers/pci/of.c                              |  64 +++
>  drivers/pci/pci-bridge-emul.c                 | 130 +++---
>  drivers/pci/pci-bridge-emul.h                 |  15 +
>  drivers/pci/pci.h                             |  15 +
>  include/uapi/linux/pci_regs.h                 |   4 +
>  11 files changed, 565 insertions(+), 94 deletions(-)
> 
> -- 
> 2.34.1
> 

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 00/18] PCI: aardvark controller changes BATCH 5
  2022-05-13 10:33 ` Lorenzo Pieralisi
@ 2022-05-13 16:48   ` Pali Rohár
  0 siblings, 0 replies; 44+ messages in thread
From: Pali Rohár @ 2022-05-13 16:48 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: Marek Behún, Bjorn Helgaas, Krzysztof Wilczyński,
	Marc Zyngier, linux-pci, linux-arm-kernel, Gregory CLEMENT

On Friday 13 May 2022 11:33:28 Lorenzo Pieralisi wrote:
> On Sun, Feb 20, 2022 at 08:33:28PM +0100, Marek Behún wrote:
> > Hello Lorenzo, Krzysztof,
> > 
> > here comes batch 5 of changes for Aardvark PCIe controller.
> > 
> > This batch
> > - adds support for AER
> > - adds support for DLLSC and hotplug interrupt
> > - adds support for sending slot power limit message
> > - adds enabling/disabling PCIe clock
> > - adds suspend support
> > - optimizes link training by adding it into separate worker
> > - optimizes GPIO resetting by asserting it only if it wasn't asserted
> >   already
> 
> I had a look and I can take patches 3 and 5 to cut the delta further,
> please let me know if that helps.

Hello! Hopefully Marek would be back in week or more. Meanwhile patches
3 and 5 should be fine to take.

> Thanks,
> Lorenzo
> 
> > Marek
> > 
> > Marek Behún (1):
> >   arm64: dts: marvell: armada-37xx: Add clock to PCIe node
> > 
> > Miquel Raynal (2):
> >   PCI: aardvark: Add clock support
> >   PCI: aardvark: Add suspend to RAM support
> > 
> > Pali Rohár (13):
> >   PCI: aardvark: Add support for AER registers on emulated bridge
> >   PCI: Add PCI_EXP_SLTCAP_*_SHIFT macros
> >   PCI: aardvark: Fix reporting Slot capabilities on emulated bridge
> >   PCI: pciehp: Enable DLLSC interrupt only if supported
> >   PCI: pciehp: Enable Command Completed Interrupt only if supported
> >   PCI: aardvark: Add support for DLLSC and hotplug interrupt
> >   PCI: Add PCI_EXP_SLTCTL_ASPL_DISABLE macro
> >   PCI: Add function for parsing `slot-power-limit-milliwatt` DT property
> >   dt-bindings: PCI: aardvark: Describe slot-power-limit-milliwatt
> >   PCI: aardvark: Send Set_Slot_Power_Limit message
> >   arm64: dts: armada-3720-turris-mox: Define slot-power-limit-milliwatt
> >     for PCIe
> >   PCI: aardvark: Run link training in separate worker
> >   PCI: aardvark: Optimize PCIe card reset via GPIO
> > 
> > Russell King (2):
> >   PCI: pci-bridge-emul: Re-arrange register tests
> >   PCI: pci-bridge-emul: Add support for PCIe extended capabilities
> > 
> >  .../devicetree/bindings/pci/aardvark-pci.txt  |   2 +
> >  .../dts/marvell/armada-3720-turris-mox.dts    |   1 +
> >  arch/arm64/boot/dts/marvell/armada-37xx.dtsi  |   1 +
> >  drivers/pci/controller/pci-aardvark.c         | 380 ++++++++++++++++--
> >  drivers/pci/hotplug/pciehp_hpc.c              |  34 +-
> >  drivers/pci/hotplug/pnv_php.c                 |  13 +-
> >  drivers/pci/of.c                              |  64 +++
> >  drivers/pci/pci-bridge-emul.c                 | 130 +++---
> >  drivers/pci/pci-bridge-emul.h                 |  15 +
> >  drivers/pci/pci.h                             |  15 +
> >  include/uapi/linux/pci_regs.h                 |   4 +
> >  11 files changed, 565 insertions(+), 94 deletions(-)
> > 
> > -- 
> > 2.34.1
> > 

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 06/18] PCI: pciehp: Enable DLLSC interrupt only if supported
  2022-05-09  3:42   ` Lukas Wunner
@ 2022-05-13 16:57     ` Pali Rohár
  2022-05-14  9:14       ` Lukas Wunner
  0 siblings, 1 reply; 44+ messages in thread
From: Pali Rohár @ 2022-05-13 16:57 UTC (permalink / raw)
  To: Lukas Wunner
  Cc: Marek Behún, Lorenzo Pieralisi, Bjorn Helgaas,
	Krzysztof Wilczy??ski, Marc Zyngier, linux-pci, linux-arm-kernel,
	Gregory CLEMENT

On Monday 09 May 2022 05:42:16 Lukas Wunner wrote:
> On Sun, Feb 20, 2022 at 08:33:34PM +0100, Marek Behún wrote:
> > --- a/drivers/pci/hotplug/pciehp_hpc.c
> > +++ b/drivers/pci/hotplug/pciehp_hpc.c
> > @@ -788,6 +788,7 @@ static int pciehp_poll(void *data)
> > @@ -800,12 +801,17 @@ static void pcie_enable_notification(struct controller *ctrl)
> >  	 * next power fault detected interrupt was notified again.
> >  	 */
> >  
> > +	pcie_capability_read_dword(ctrl_dev(ctrl), PCI_EXP_LNKCAP, &link_cap);
> >  	/*
> > -	 * Always enable link events: thus link-up and link-down shall
> > -	 * always be treated as hotplug and unplug respectively. Enable
> > -	 * presence detect only if Attention Button is not present.
> > +	 * Enable link events if their support is indicated in Link Capability
> > +	 * register: thus link-up and link-down shall always be treated as
> > +	 * hotplug and unplug respectively. Enable presence detect only if
> > +	 * Attention Button is not present.
> >  	 */
> > -	cmd = PCI_EXP_SLTCTL_DLLSCE;
> > +	cmd = 0;
> > +	if (link_cap & PCI_EXP_LNKCAP_DLLLARC)
> > +		cmd |= PCI_EXP_SLTCTL_DLLSCE;
> 
> The Data Link Layer Link Active Reporting Capable bit is cached
> in ctrl_dev(ctrl)->link_active_reporting.  Please use that
> instead of re-reading it from the register.
> 
> According to PCIe r6.0, sec. 7.5.3.6, "For a hot-plug capable
> Downstream Port [...], this bit must be hardwired to 1b."
> 
> That has been part of the spec since PCIe r1.1, sec. 7.8.6.
> 
> PCIe r1.0 did not contain the sentence because it did not support
> DLLLARC (it defined bit 20 as RsvdP).
> 
> In other words, what you're doing here is add support for PCIe r1.0.
> I'm not opposed to that, but I'd assume that aardvark supports a
> more recent spec version.  More likely it doesn't comply with the spec?
> 
> What is the user-visible issue that you're experiencing without this
> commit?  Does aardvark somehow misbehave if the DLLLARC bit is set to 1?
> Since the bit is RsvdP, setting it shouldn't have any negative side
> effects.

I will let fixing those issues to Marek.

To answer your questions: Config space of Aardvark Root Port does not
conform to PCIe base spec. It does not implement DLLLARC, nor DLLSCE and
lot of other bits. Plus it has Type 0 header (not Type 1). And due to
these reasons, pci-aardvark.c driver implements "emulation" of the
Root Port and implements some of the functionality via custom aardvark
registers. So Root Port would be presented to kernel and also to
userspace as PCI Bridge device with Type 1 header and with PCIe
registers required by linux kernel.

During my testing of kernel hotplug code last year, I figured out that
kernel was waiting for event which never happened. And so it was needed
to "fix" kernel to not try to enable DLLSCE because it did nothing.

I asked more times Marvell for Aardvark documentation, so I could fix
these issues, but I have never received any response for it.

> 
> > --- a/drivers/pci/hotplug/pnv_php.c
> > +++ b/drivers/pci/hotplug/pnv_php.c
> > @@ -840,6 +840,7 @@ static void pnv_php_init_irq(struct pnv_php_slot *php_slot, int irq)
> >  	struct pci_dev *pdev = php_slot->pdev;
> >  	u32 broken_pdc = 0;
> >  	u16 sts, ctrl;
> > +	u32 link_cap;
> >  	int ret;
> >  
> >  	/* Allocate workqueue */
> 
> pnv_php.c is a driver for PowerNV, yet this patch is for a series
> targeting an ARM PCIe controller.  That doesn't make sense,
> changes to pnv_php.c seem wrong here.
> 
> Thanks,
> 
> Lukas

At time when I prepared this patch (year ago) I changed that DLLSCE
pattern in all places because it looked like copy+paste code which
should be fixed too.

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 07/18] PCI: pciehp: Enable Command Completed Interrupt only if supported
  2022-05-09  4:01   ` Lukas Wunner
@ 2022-05-13 16:59     ` Pali Rohár
  0 siblings, 0 replies; 44+ messages in thread
From: Pali Rohár @ 2022-05-13 16:59 UTC (permalink / raw)
  To: Lukas Wunner
  Cc: Marek Behún, Lorenzo Pieralisi, Bjorn Helgaas,
	Krzysztof Wilczy??ski, Marc Zyngier, linux-pci, linux-arm-kernel,
	Gregory CLEMENT

On Monday 09 May 2022 06:01:39 Lukas Wunner wrote:
> On Sun, Feb 20, 2022 at 08:33:35PM +0100, Marek Behún wrote:
> > The No Command Completed Support bit in the Slot Capabilities register
> > indicates whether Command Completed Interrupt Enable is unsupported.
> > 
> > Enable this interrupt only in the case it is supported.
> [...]
> > --- a/drivers/pci/hotplug/pciehp_hpc.c
> > +++ b/drivers/pci/hotplug/pciehp_hpc.c
> > @@ -817,7 +817,9 @@ static void pcie_enable_notification(struct controller *ctrl)
> >  	else
> >  		cmd |= PCI_EXP_SLTCTL_PDCE;
> >  	if (!pciehp_poll_mode)
> > -		cmd |= PCI_EXP_SLTCTL_HPIE | PCI_EXP_SLTCTL_CCIE;
> > +		cmd |= PCI_EXP_SLTCTL_HPIE;
> > +	if (!pciehp_poll_mode && !NO_CMD_CMPL(ctrl))
> > +		cmd |= PCI_EXP_SLTCTL_CCIE;
> 
> Looks okay to me in principle, I'm just wondering why this change is
> necessary, i.e. what issue are you seeing without it?
> 
> Thanks,
> 
> Lukas

This is that case which I described in previous email. Kernel was
waiting for completion, but if (emulated) Root Port does not support
completion event then there were timeouts.

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 06/18] PCI: pciehp: Enable DLLSC interrupt only if supported
  2022-05-13 16:57     ` Pali Rohár
@ 2022-05-14  9:14       ` Lukas Wunner
  2022-08-18 12:22         ` Marek Behún
  0 siblings, 1 reply; 44+ messages in thread
From: Lukas Wunner @ 2022-05-14  9:14 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Marek Behún, Lorenzo Pieralisi, Bjorn Helgaas,
	Krzysztof Wilczy??ski, Marc Zyngier, linux-pci, linux-arm-kernel,
	Gregory CLEMENT

On Fri, May 13, 2022 at 06:57:29PM +0200, Pali Rohár wrote:
> To answer your questions: Config space of Aardvark Root Port does not
> conform to PCIe base spec. It does not implement DLLLARC, nor DLLSCE and
> lot of other bits. Plus it has Type 0 header (not Type 1). And due to
> these reasons, pci-aardvark.c driver implements "emulation" of the
> Root Port and implements some of the functionality via custom aardvark
> registers. So Root Port would be presented to kernel and also to
> userspace as PCI Bridge device with Type 1 header and with PCIe
> registers required by linux kernel.
> 
> During my testing of kernel hotplug code last year, I figured out that
> kernel was waiting for event which never happened. And so it was needed
> to "fix" kernel to not try to enable DLLSCE because it did nothing.

Could you please reproduce this and add the following on the command line:

  log_buf_len=10M pciehp.pciehp_debug=1 dyndbg="file pciehp* +p"
  ignore_loglevel

Then open a bug at bugzilla.kernel.org, attach full dmesg output
as well as full "lspci -vv" output and send the bugzilla link to me.

(Obviously, revert patches 6 and 7 when trying to reproduce it.)

So a PDC event should be sufficient to bring the slot up or down,
a DLLSC event should not be necessary.  Enabling DLLSC should not
make a difference on a controller which doesn't support it.
I just double-checked the code and I do not see where we'd wait
for a DLLSC event which never comes.

Don't worry, if we come to the conclusion that your proposed fix
is the only solution, I'm fine with it, but at this point I'd
like to get a better understanding what is really going on.
Perhaps there is some other issue in pciehp that this patch
just papers over.  Once you provide the dmesg debug output
I'll be able to analyze that.

Thanks!

Lukas

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: (subset) [PATCH 00/18] PCI: aardvark controller changes BATCH 5
  2022-02-20 19:33 [PATCH 00/18] PCI: aardvark controller changes BATCH 5 Marek Behún
                   ` (19 preceding siblings ...)
  2022-05-13 10:33 ` Lorenzo Pieralisi
@ 2022-05-18 15:54 ` Lorenzo Pieralisi
  2022-08-16 16:25 ` Lorenzo Pieralisi
  21 siblings, 0 replies; 44+ messages in thread
From: Lorenzo Pieralisi @ 2022-05-18 15:54 UTC (permalink / raw)
  To: Bjorn Helgaas, Marek Behún
  Cc: Lorenzo Pieralisi, pali, Krzysztof Wilczyński, linux-pci,
	Gregory CLEMENT, linux-arm-kernel, Marc Zyngier

On Sun, 20 Feb 2022 20:33:28 +0100, Marek Behún wrote:
> here comes batch 5 of changes for Aardvark PCIe controller.
> 
> This batch
> - adds support for AER
> - adds support for DLLSC and hotplug interrupt
> - adds support for sending slot power limit message
> - adds enabling/disabling PCIe clock
> - adds suspend support
> - optimizes link training by adding it into separate worker
> - optimizes GPIO resetting by asserting it only if it wasn't asserted
>   already
> 
> [...]

Applied to pci/aardvark, thanks!

[03/18] PCI: aardvark: Add support for AER registers on emulated bridge
        https://git.kernel.org/lpieralisi/pci/c/06228c0ae2
[05/18] PCI: aardvark: Fix reporting Slot capabilities on emulated bridge
        https://git.kernel.org/lpieralisi/pci/c/bf8dd34079

Thanks,
Lorenzo

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 04/18] PCI: Add PCI_EXP_SLTCAP_*_SHIFT macros
  2022-02-20 19:33 ` [PATCH 04/18] PCI: Add PCI_EXP_SLTCAP_*_SHIFT macros Marek Behún
  2022-04-28 11:09   ` Lorenzo Pieralisi
@ 2022-05-18 19:23   ` Bjorn Helgaas
  2022-05-18 19:26     ` Pali Rohár
  1 sibling, 1 reply; 44+ messages in thread
From: Bjorn Helgaas @ 2022-05-18 19:23 UTC (permalink / raw)
  To: Marek Behún
  Cc: Lorenzo Pieralisi, Krzysztof Wilczyński, Marc Zyngier, pali,
	linux-pci, linux-arm-kernel, Gregory CLEMENT

On Sun, Feb 20, 2022 at 08:33:32PM +0100, Marek Behún wrote:
> From: Pali Rohár <pali@kernel.org>
> 
> These macros allows to easily compose and extract Slot Power Limit and
> Physical Slot Number values from Slot Capability Register.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Signed-off-by: Marek Behún <kabel@kernel.org>

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

We talked about using FIELD_PREP() / FIELD_GET(), which I think would
remove the need for the *_SHIFT macros.  But we can always do that
later.

> ---
>  include/uapi/linux/pci_regs.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> index bee1a9ed6e66..d825e17e448c 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -591,10 +591,13 @@
>  #define  PCI_EXP_SLTCAP_HPS	0x00000020 /* Hot-Plug Surprise */
>  #define  PCI_EXP_SLTCAP_HPC	0x00000040 /* Hot-Plug Capable */
>  #define  PCI_EXP_SLTCAP_SPLV	0x00007f80 /* Slot Power Limit Value */
> +#define  PCI_EXP_SLTCAP_SPLV_SHIFT	7  /* Slot Power Limit Value shift */
>  #define  PCI_EXP_SLTCAP_SPLS	0x00018000 /* Slot Power Limit Scale */
> +#define  PCI_EXP_SLTCAP_SPLS_SHIFT	15 /* Slot Power Limit Scale shift */
>  #define  PCI_EXP_SLTCAP_EIP	0x00020000 /* Electromechanical Interlock Present */
>  #define  PCI_EXP_SLTCAP_NCCS	0x00040000 /* No Command Completed Support */
>  #define  PCI_EXP_SLTCAP_PSN	0xfff80000 /* Physical Slot Number */
> +#define  PCI_EXP_SLTCAP_PSN_SHIFT	19 /* Physical Slot Number shift */
>  #define PCI_EXP_SLTCTL		0x18	/* Slot Control */
>  #define  PCI_EXP_SLTCTL_ABPE	0x0001	/* Attention Button Pressed Enable */
>  #define  PCI_EXP_SLTCTL_PFDE	0x0002	/* Power Fault Detected Enable */
> -- 
> 2.34.1
> 

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 04/18] PCI: Add PCI_EXP_SLTCAP_*_SHIFT macros
  2022-05-18 19:23   ` Bjorn Helgaas
@ 2022-05-18 19:26     ` Pali Rohár
  2022-05-18 20:05       ` Marek Behún
  0 siblings, 1 reply; 44+ messages in thread
From: Pali Rohár @ 2022-05-18 19:26 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Marek Behún, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Marc Zyngier, linux-pci, linux-arm-kernel, Gregory CLEMENT

On Wednesday 18 May 2022 14:23:22 Bjorn Helgaas wrote:
> On Sun, Feb 20, 2022 at 08:33:32PM +0100, Marek Behún wrote:
> > From: Pali Rohár <pali@kernel.org>
> > 
> > These macros allows to easily compose and extract Slot Power Limit and
> > Physical Slot Number values from Slot Capability Register.
> > 
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > Signed-off-by: Marek Behún <kabel@kernel.org>
> 
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> 
> We talked about using FIELD_PREP() / FIELD_GET(), which I think would
> remove the need for the *_SHIFT macros.  But we can always do that
> later.

I have already done this for pci-mvebu.c driver and patch was merged:
https://lore.kernel.org/linux-pci/20220412094946.27069-5-pali@kernel.org/

IIRC we have decided to not use those *_SHIFT macros as it would flood
public uapi file and from this file we cannot remove macros due to
userspace backward compatibility.

> > ---
> >  include/uapi/linux/pci_regs.h | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> > index bee1a9ed6e66..d825e17e448c 100644
> > --- a/include/uapi/linux/pci_regs.h
> > +++ b/include/uapi/linux/pci_regs.h
> > @@ -591,10 +591,13 @@
> >  #define  PCI_EXP_SLTCAP_HPS	0x00000020 /* Hot-Plug Surprise */
> >  #define  PCI_EXP_SLTCAP_HPC	0x00000040 /* Hot-Plug Capable */
> >  #define  PCI_EXP_SLTCAP_SPLV	0x00007f80 /* Slot Power Limit Value */
> > +#define  PCI_EXP_SLTCAP_SPLV_SHIFT	7  /* Slot Power Limit Value shift */
> >  #define  PCI_EXP_SLTCAP_SPLS	0x00018000 /* Slot Power Limit Scale */
> > +#define  PCI_EXP_SLTCAP_SPLS_SHIFT	15 /* Slot Power Limit Scale shift */
> >  #define  PCI_EXP_SLTCAP_EIP	0x00020000 /* Electromechanical Interlock Present */
> >  #define  PCI_EXP_SLTCAP_NCCS	0x00040000 /* No Command Completed Support */
> >  #define  PCI_EXP_SLTCAP_PSN	0xfff80000 /* Physical Slot Number */
> > +#define  PCI_EXP_SLTCAP_PSN_SHIFT	19 /* Physical Slot Number shift */
> >  #define PCI_EXP_SLTCTL		0x18	/* Slot Control */
> >  #define  PCI_EXP_SLTCTL_ABPE	0x0001	/* Attention Button Pressed Enable */
> >  #define  PCI_EXP_SLTCTL_PFDE	0x0002	/* Power Fault Detected Enable */
> > -- 
> > 2.34.1
> > 

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 04/18] PCI: Add PCI_EXP_SLTCAP_*_SHIFT macros
  2022-05-18 19:26     ` Pali Rohár
@ 2022-05-18 20:05       ` Marek Behún
  2022-05-18 20:27         ` Bjorn Helgaas
  0 siblings, 1 reply; 44+ messages in thread
From: Marek Behún @ 2022-05-18 20:05 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Marc Zyngier, linux-pci, linux-arm-kernel, Gregory CLEMENT

On Wed, 18 May 2022 21:26:00 +0200
Pali Rohár <pali@kernel.org> wrote:

> On Wednesday 18 May 2022 14:23:22 Bjorn Helgaas wrote:
> > On Sun, Feb 20, 2022 at 08:33:32PM +0100, Marek Behún wrote:  
> > > From: Pali Rohár <pali@kernel.org>
> > > 
> > > These macros allows to easily compose and extract Slot Power Limit and
> > > Physical Slot Number values from Slot Capability Register.
> > > 
> > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > Signed-off-by: Marek Behún <kabel@kernel.org>  
> > 
> > Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> > 
> > We talked about using FIELD_PREP() / FIELD_GET(), which I think would
> > remove the need for the *_SHIFT macros.  But we can always do that
> > later.  
> 
> I have already done this for pci-mvebu.c driver and patch was merged:
> https://lore.kernel.org/linux-pci/20220412094946.27069-5-pali@kernel.org/
> 
> IIRC we have decided to not use those *_SHIFT macros as it would flood
> public uapi file and from this file we cannot remove macros due to
> userspace backward compatibility.

So should I change patch 5 of the aardvark batch 5 to also use
FIELD_GET(), so that this patch is not needed?

Marek

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 04/18] PCI: Add PCI_EXP_SLTCAP_*_SHIFT macros
  2022-05-18 20:05       ` Marek Behún
@ 2022-05-18 20:27         ` Bjorn Helgaas
  0 siblings, 0 replies; 44+ messages in thread
From: Bjorn Helgaas @ 2022-05-18 20:27 UTC (permalink / raw)
  To: Marek Behún
  Cc: Pali Rohár, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Marc Zyngier, linux-pci, linux-arm-kernel, Gregory CLEMENT

On Wed, May 18, 2022 at 10:05:56PM +0200, Marek Behún wrote:
> On Wed, 18 May 2022 21:26:00 +0200
> Pali Rohár <pali@kernel.org> wrote:
> 
> > On Wednesday 18 May 2022 14:23:22 Bjorn Helgaas wrote:
> > > On Sun, Feb 20, 2022 at 08:33:32PM +0100, Marek Behún wrote:  
> > > > From: Pali Rohár <pali@kernel.org>
> > > > 
> > > > These macros allows to easily compose and extract Slot Power Limit and
> > > > Physical Slot Number values from Slot Capability Register.
> > > > 
> > > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > > Signed-off-by: Marek Behún <kabel@kernel.org>  
> > > 
> > > Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> > > 
> > > We talked about using FIELD_PREP() / FIELD_GET(), which I think would
> > > remove the need for the *_SHIFT macros.  But we can always do that
> > > later.  
> > 
> > I have already done this for pci-mvebu.c driver and patch was merged:
> > https://lore.kernel.org/linux-pci/20220412094946.27069-5-pali@kernel.org/
> > 
> > IIRC we have decided to not use those *_SHIFT macros as it would flood
> > public uapi file and from this file we cannot remove macros due to
> > userspace backward compatibility.
> 
> So should I change patch 5 of the aardvark batch 5 to also use
> FIELD_GET(), so that this patch is not needed?

That would be awesome if you could.  I forgot about the backward
compatibility issue with include/uapi/linux/pci_regs.h

Bjorn

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 00/18] PCI: aardvark controller changes BATCH 5
  2022-02-20 19:33 [PATCH 00/18] PCI: aardvark controller changes BATCH 5 Marek Behún
                   ` (20 preceding siblings ...)
  2022-05-18 15:54 ` (subset) " Lorenzo Pieralisi
@ 2022-08-16 16:25 ` Lorenzo Pieralisi
  2022-08-18 13:56   ` Marek Behún
  21 siblings, 1 reply; 44+ messages in thread
From: Lorenzo Pieralisi @ 2022-08-16 16:25 UTC (permalink / raw)
  To: Marek Behún
  Cc: Lorenzo Pieralisi, Bjorn Helgaas, Krzysztof Wilczyński,
	Marc Zyngier, pali, linux-pci, linux-arm-kernel, Gregory CLEMENT

On Sun, Feb 20, 2022 at 08:33:28PM +0100, Marek Behún wrote:
> Hello Lorenzo, Krzysztof,
> 
> here comes batch 5 of changes for Aardvark PCIe controller.
> 
> This batch
> - adds support for AER
> - adds support for DLLSC and hotplug interrupt
> - adds support for sending slot power limit message
> - adds enabling/disabling PCIe clock
> - adds suspend support
> - optimizes link training by adding it into separate worker
> - optimizes GPIO resetting by asserting it only if it wasn't asserted
>   already
> 
> Marek
> 
> Marek Behún (1):
>   arm64: dts: marvell: armada-37xx: Add clock to PCIe node
> 
> Miquel Raynal (2):
>   PCI: aardvark: Add clock support
>   PCI: aardvark: Add suspend to RAM support
> 
> Pali Rohár (13):
>   PCI: aardvark: Add support for AER registers on emulated bridge
>   PCI: Add PCI_EXP_SLTCAP_*_SHIFT macros
>   PCI: aardvark: Fix reporting Slot capabilities on emulated bridge
>   PCI: pciehp: Enable DLLSC interrupt only if supported
>   PCI: pciehp: Enable Command Completed Interrupt only if supported
>   PCI: aardvark: Add support for DLLSC and hotplug interrupt
>   PCI: Add PCI_EXP_SLTCTL_ASPL_DISABLE macro
>   PCI: Add function for parsing `slot-power-limit-milliwatt` DT property
>   dt-bindings: PCI: aardvark: Describe slot-power-limit-milliwatt
>   PCI: aardvark: Send Set_Slot_Power_Limit message
>   arm64: dts: armada-3720-turris-mox: Define slot-power-limit-milliwatt
>     for PCIe
>   PCI: aardvark: Run link training in separate worker
>   PCI: aardvark: Optimize PCIe card reset via GPIO
> 
> Russell King (2):
>   PCI: pci-bridge-emul: Re-arrange register tests
>   PCI: pci-bridge-emul: Add support for PCIe extended capabilities
> 
>  .../devicetree/bindings/pci/aardvark-pci.txt  |   2 +
>  .../dts/marvell/armada-3720-turris-mox.dts    |   1 +
>  arch/arm64/boot/dts/marvell/armada-37xx.dtsi  |   1 +
>  drivers/pci/controller/pci-aardvark.c         | 380 ++++++++++++++++--
>  drivers/pci/hotplug/pciehp_hpc.c              |  34 +-
>  drivers/pci/hotplug/pnv_php.c                 |  13 +-
>  drivers/pci/of.c                              |  64 +++
>  drivers/pci/pci-bridge-emul.c                 | 130 +++---
>  drivers/pci/pci-bridge-emul.h                 |  15 +
>  drivers/pci/pci.h                             |  15 +
>  include/uapi/linux/pci_regs.h                 |   4 +
>  11 files changed, 565 insertions(+), 94 deletions(-)

Hi Marek,

back from a hiatus, just catching up with threads. According to
patchwork patches {8,14,16} of this series are still to be
reviewed; may I ask you please an update on the status and we
will take it from there.

Thanks,
Lorenzo

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 06/18] PCI: pciehp: Enable DLLSC interrupt only if supported
  2022-05-14  9:14       ` Lukas Wunner
@ 2022-08-18 12:22         ` Marek Behún
  0 siblings, 0 replies; 44+ messages in thread
From: Marek Behún @ 2022-08-18 12:22 UTC (permalink / raw)
  To: Lukas Wunner
  Cc: Pali Rohár, Lorenzo Pieralisi, Bjorn Helgaas,
	Krzysztof Wilczy??ski, Marc Zyngier, linux-pci, linux-arm-kernel,
	Gregory CLEMENT

On Sat, 14 May 2022 11:14:00 +0200
Lukas Wunner <lukas@wunner.de> wrote:

> On Fri, May 13, 2022 at 06:57:29PM +0200, Pali Rohár wrote:
> > To answer your questions: Config space of Aardvark Root Port does not
> > conform to PCIe base spec. It does not implement DLLLARC, nor DLLSCE and
> > lot of other bits. Plus it has Type 0 header (not Type 1). And due to
> > these reasons, pci-aardvark.c driver implements "emulation" of the
> > Root Port and implements some of the functionality via custom aardvark
> > registers. So Root Port would be presented to kernel and also to
> > userspace as PCI Bridge device with Type 1 header and with PCIe
> > registers required by linux kernel.
> > 
> > During my testing of kernel hotplug code last year, I figured out that
> > kernel was waiting for event which never happened. And so it was needed
> > to "fix" kernel to not try to enable DLLSCE because it did nothing.  
> 
> Could you please reproduce this and add the following on the command line:
> 
>   log_buf_len=10M pciehp.pciehp_debug=1 dyndbg="file pciehp* +p"
>   ignore_loglevel
> 
> Then open a bug at bugzilla.kernel.org, attach full dmesg output
> as well as full "lspci -vv" output and send the bugzilla link to me.
> 
> (Obviously, revert patches 6 and 7 when trying to reproduce it.)
> 
> So a PDC event should be sufficient to bring the slot up or down,
> a DLLSC event should not be necessary.  Enabling DLLSC should not
> make a difference on a controller which doesn't support it.
> I just double-checked the code and I do not see where we'd wait
> for a DLLSC event which never comes.
> 
> Don't worry, if we come to the conclusion that your proposed fix
> is the only solution, I'm fine with it, but at this point I'd
> like to get a better understanding what is really going on.
> Perhaps there is some other issue in pciehp that this patch
> just papers over.  Once you provide the dmesg debug output
> I'll be able to analyze that.

Dear Lukas,

we have tried to reproduce the bug where kernel was waiting for an
event which never happend, the bug that Pali remembered from his
work on the pciehp code.

We have concluded that it doesn't concert the DLLSC patch (06/18), only
the Command Completed Interrupt patch (07/18), and even there it seems
that the patch is not needed to trigger the bug: it seems that when
Pali was studying the bug, he did two things:
1. he made enabling Command Completed Interrupt conditional on NCCS bit
   not  set
2. he made the aardvark driver report NCCS bit via emulated bridge.

It turns out that only the second thing is needed, since the pciehp
code checks NCCS bit in pcie_wait_cmd() and does not wait for the
interrupt if NCCS is set.

Anyway we still think that both patches make sense, at least so that an
interrupt isn't reported as enabled and not supported at once when
dumping the configuration space.

So I will resend these patches with updated commit messages.

Marek

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 00/18] PCI: aardvark controller changes BATCH 5
  2022-08-16 16:25 ` Lorenzo Pieralisi
@ 2022-08-18 13:56   ` Marek Behún
  0 siblings, 0 replies; 44+ messages in thread
From: Marek Behún @ 2022-08-18 13:56 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: Lorenzo Pieralisi, Bjorn Helgaas, Krzysztof Wilczyński,
	Marc Zyngier, pali, linux-pci, linux-arm-kernel, Gregory CLEMENT

On Tue, 16 Aug 2022 18:25:48 +0200
Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:

> Hi Marek,
> 
> back from a hiatus, just catching up with threads. According to
> patchwork patches {8,14,16} of this series are still to be
> reviewed; may I ask you please an update on the status and we
> will take it from there.

Sent batch 6 :)

^ permalink raw reply	[flat|nested] 44+ messages in thread

end of thread, other threads:[~2022-08-18 13:57 UTC | newest]

Thread overview: 44+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-20 19:33 [PATCH 00/18] PCI: aardvark controller changes BATCH 5 Marek Behún
2022-02-20 19:33 ` [PATCH 01/18] PCI: pci-bridge-emul: Re-arrange register tests Marek Behún
2022-02-20 19:33 ` [PATCH 02/18] PCI: pci-bridge-emul: Add support for PCIe extended capabilities Marek Behún
2022-02-20 19:33 ` [PATCH 03/18] PCI: aardvark: Add support for AER registers on emulated bridge Marek Behún
2022-02-20 19:33 ` [PATCH 04/18] PCI: Add PCI_EXP_SLTCAP_*_SHIFT macros Marek Behún
2022-04-28 11:09   ` Lorenzo Pieralisi
2022-04-28 11:16     ` Pali Rohár
2022-05-18 19:23   ` Bjorn Helgaas
2022-05-18 19:26     ` Pali Rohár
2022-05-18 20:05       ` Marek Behún
2022-05-18 20:27         ` Bjorn Helgaas
2022-02-20 19:33 ` [PATCH 05/18] PCI: aardvark: Fix reporting Slot capabilities on emulated bridge Marek Behún
2022-02-20 19:33 ` [PATCH 06/18] PCI: pciehp: Enable DLLSC interrupt only if supported Marek Behún
2022-05-09  3:42   ` Lukas Wunner
2022-05-13 16:57     ` Pali Rohár
2022-05-14  9:14       ` Lukas Wunner
2022-08-18 12:22         ` Marek Behún
2022-02-20 19:33 ` [PATCH 07/18] PCI: pciehp: Enable Command Completed Interrupt " Marek Behún
2022-05-09  4:01   ` Lukas Wunner
2022-05-13 16:59     ` Pali Rohár
2022-02-20 19:33 ` [PATCH 08/18] PCI: aardvark: Add support for DLLSC and hotplug interrupt Marek Behún
2022-02-20 19:33 ` [PATCH 09/18] PCI: Add PCI_EXP_SLTCTL_ASPL_DISABLE macro Marek Behún
2022-02-20 19:33 ` [PATCH 10/18] PCI: Add function for parsing `slot-power-limit-milliwatt` DT property Marek Behún
2022-02-20 19:33 ` [PATCH 11/18] dt-bindings: PCI: aardvark: Describe slot-power-limit-milliwatt Marek Behún
2022-02-20 19:33 ` [PATCH 12/18] PCI: aardvark: Send Set_Slot_Power_Limit message Marek Behún
2022-02-20 19:33 ` [PATCH 13/18] arm64: dts: armada-3720-turris-mox: Define slot-power-limit-milliwatt for PCIe Marek Behún
2022-02-20 19:33 ` [PATCH 14/18] PCI: aardvark: Add clock support Marek Behún
2022-02-20 19:33 ` [PATCH 15/18] arm64: dts: marvell: armada-37xx: Add clock to PCIe node Marek Behún
2022-02-28 15:52   ` Gregory CLEMENT
2022-02-20 19:33 ` [PATCH 16/18] PCI: aardvark: Add suspend to RAM support Marek Behún
2022-04-12 11:14   ` Lorenzo Pieralisi
2022-02-20 19:33 ` [PATCH 17/18] PCI: aardvark: Run link training in separate worker Marek Behún
2022-04-12 15:25   ` Lorenzo Pieralisi
2022-04-12 17:55     ` Pali Rohár
2022-04-13  9:16       ` Lorenzo Pieralisi
2022-05-04 14:02         ` Marek Behún
2022-02-20 19:33 ` [PATCH 18/18] PCI: aardvark: Optimize PCIe card reset via GPIO Marek Behún
2022-04-11 15:36 ` [PATCH 00/18] PCI: aardvark controller changes BATCH 5 Lorenzo Pieralisi
2022-04-11 16:53   ` Pali Rohár
2022-05-13 10:33 ` Lorenzo Pieralisi
2022-05-13 16:48   ` Pali Rohár
2022-05-18 15:54 ` (subset) " Lorenzo Pieralisi
2022-08-16 16:25 ` Lorenzo Pieralisi
2022-08-18 13:56   ` Marek Behún

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).