Skip to contentSkip to conversation

PCI: Small improvements for pci-bridge-emul and mvebu

PATCH Superseded Pali Rohár 4 participants 33 messages Dec 21, 2021 – Feb 3, 2022

Subsystems (4)
lore
Share & tools
lore

Pali Rohár

author maintainer
#0 lore raw

cover PCI: Small improvements for pci-bridge-emul and mvebu

This patch series contains small improvements for pci-bridge-emul and mvebu drivers. This patch series is based on top of the patches:
https://lore.kernel.org/linux-pci/[email protected]/

Pali Rohár (11):
MAINTAINERS: Add Pali Rohár as pci-mvebu.c maintainer
PCI: pci-bridge-emul: Make struct pci_bridge_emul_ops as const
PCI: pci-bridge-emul: Rename PCI_BRIDGE_EMUL_NO_PREFETCHABLE_BAR to
PCI_BRIDGE_EMUL_NO_PREFMEM_FORWARD
PCI: pci-bridge-emul: Add support for new flag
PCI_BRIDGE_EMUL_NO_IO_FORWARD
PCI: mvebu: Add help string for CONFIG_PCI_MVEBU option
PCI: mvebu: Remove duplicate nports assignment
PCI: mvebu: Set PCI_BRIDGE_EMUL_NO_IO_FORWARD when IO is unsupported
PCI: mvebu: Properly initialize vendor, device and revision of
emulated bridge
PCI: mvebu: Update comment for PCI_EXP_LNKCAP register on emulated
bridge
PCI: mvebu: Update comment for PCI_EXP_LNKCTL register on emulated
bridge
PCI: mvebu: Fix reporting Data Link Layer Link Active on emulated
bridge

 MAINTAINERS                           |  1 +
 drivers/pci/controller/Kconfig        |  4 ++
 drivers/pci/controller/pci-aardvark.c |  2 +-
 drivers/pci/controller/pci-mvebu.c    | 64 ++++++++++++---------------
 drivers/pci/pci-bridge-emul.c         | 11 ++++-
 drivers/pci/pci-bridge-emul.h         |  5 ++-
 6 files changed, 48 insertions(+), 39 deletions(-)
Signature · 1 line
2.20.1
author maintainer
#1 lore raw

PATCH 1/11 MAINTAINERS: Add Pali Rohár as pci-mvebu.c maintainer

---
I discussed with Thomas and he ask me for taking maintenance of pci-mvebu.c driver.

---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)
MAINTAINERS +1 -0
index 7a2345ce8521..24527789d933 100644
@@ -14617,6 +14617,7 @@ F:	drivers/pci/controller/mobiveil/pcie-mobiveil*
 
 PCI DRIVER FOR MVEBU (Marvell Armada 370 and Armada XP SOC support)
 M:	Thomas Petazzoni <[email protected]>
+M:	Pali Rohár <[email protected]>
 L:	[email protected]
 L:	[email protected] (moderated for non-subscribers)
 S:	Maintained
Signature · 1 line
2.20.1
maintainer Acked-by on v1 1/11
#2 lore raw

Re: [PATCH 01/11] MAINTAINERS: Add Pali Rohár as pci-mvebu.c maintainer

Quoted 1 line · On Tue, 21 Dec 2021 15:14:45 +0100 Pali Rohár <[email protected]> wrote:

Thanks a lot Pali for volunteering to maintain this driver!

Thomas

Signature · 3 lines
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
author maintainer
#3 lore raw

PATCH 2/11 PCI: pci-bridge-emul: Make struct pci_bridge_emul_ops as const

In reply to Pali Rohár

It is read-only constant structure, so properly mark it with const keyword.

---
 drivers/pci/controller/pci-aardvark.c | 2 +-
 drivers/pci/controller/pci-mvebu.c    | 2 +-
 drivers/pci/pci-bridge-emul.h         | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)
drivers/pci/controller/pci-aardvark.c +1 -1
index 62fc55f2ed40..1fa6fe1e022a 100644
@@ -928,7 +928,7 @@ advk_pci_bridge_emul_pcie_conf_write(struct pci_bridge_emul *bridge,
 	}
 }
 
-static struct pci_bridge_emul_ops advk_pci_bridge_emul_ops = {
+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,
drivers/pci/controller/pci-mvebu.c +1 -1
index 68aa94a258ff..2ecc1ab12249 100644
@@ -709,7 +709,7 @@ mvebu_pci_bridge_emul_pcie_conf_write(struct pci_bridge_emul *bridge,
 	}
 }
 
-static struct pci_bridge_emul_ops mvebu_pci_bridge_emul_ops = {
+static const struct pci_bridge_emul_ops mvebu_pci_bridge_emul_ops = {
 	.read_base = mvebu_pci_bridge_emul_base_conf_read,
 	.write_base = mvebu_pci_bridge_emul_base_conf_write,
 	.read_pcie = mvebu_pci_bridge_emul_pcie_conf_read,
drivers/pci/pci-bridge-emul.h +1 -1
index 49bbd37ee318..0690b6369755 100644
@@ -112,7 +112,7 @@ struct pci_bridge_reg_behavior;
 struct pci_bridge_emul {
 	struct pci_bridge_emul_conf conf;
 	struct pci_bridge_emul_pcie_conf pcie_conf;
-	struct pci_bridge_emul_ops *ops;
+	const struct pci_bridge_emul_ops *ops;
 	struct pci_bridge_reg_behavior *pci_regs_behavior;
 	struct pci_bridge_reg_behavior *pcie_cap_regs_behavior;
 	void *data;
Signature · 1 line
2.20.1
maintainer Acked-by on v1 2/11
#4 lore raw

Re: [PATCH 02/11] PCI: pci-bridge-emul: Make struct pci_bridge_emul_ops as const

Quoted 3 lines · On Tue, 21 Dec 2021 15:14:46 +0100 Pali Rohár <[email protected]> wrote:

It is read-only constant structure, so properly mark it with const keyword.

Signature · 3 lines
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
author maintainer
#5 lore raw

PATCH 3/11 PCI: pci-bridge-emul: Rename PCI_BRIDGE_EMUL_NO_PREFETCHABLE_BAR to PCI_BRIDGE_EMUL_NO_PREFMEM_FORWARD

In reply to Pali Rohár

This flag describe whether PCI bridge supports forwarding of prefetchable memory requests in given range between primary and secondary buses. It does not specify if bridge has support for prefetchable memory BAR (moreover this pci-bridge-emul.c driver does not provide support for BARs).

So change name of this flag to be less misleading.

---
 drivers/pci/controller/pci-mvebu.c | 2 +-
 drivers/pci/pci-bridge-emul.c      | 2 +-
 drivers/pci/pci-bridge-emul.h      | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)
drivers/pci/controller/pci-mvebu.c +1 -1
index 2ecc1ab12249..2e10ade660a1 100644
@@ -747,7 +747,7 @@ static int mvebu_pci_bridge_emul_init(struct mvebu_pcie_port *port)
 	bridge->data = port;
 	bridge->ops = &mvebu_pci_bridge_emul_ops;
 
-	return pci_bridge_emul_init(bridge, PCI_BRIDGE_EMUL_NO_PREFETCHABLE_BAR);
+	return pci_bridge_emul_init(bridge, PCI_BRIDGE_EMUL_NO_PREFMEM_FORWARD);
 }
 
 static inline struct mvebu_pcie *sys_to_pcie(struct pci_sys_data *sys)
drivers/pci/pci-bridge-emul.c +1 -1
index 79b947528455..432b1bec2e22 100644
@@ -373,7 +373,7 @@ int pci_bridge_emul_init(struct pci_bridge_emul *bridge,
 			~(BIT(10) << 16);
 	}
 
-	if (flags & PCI_BRIDGE_EMUL_NO_PREFETCHABLE_BAR) {
+	if (flags & PCI_BRIDGE_EMUL_NO_PREFMEM_FORWARD) {
 		bridge->pci_regs_behavior[PCI_PREF_MEMORY_BASE / 4].ro = ~0;
 		bridge->pci_regs_behavior[PCI_PREF_MEMORY_BASE / 4].rw = 0;
 	}
drivers/pci/pci-bridge-emul.h +1 -1
index 0690b6369755..88877ebefbac 100644
@@ -120,7 +120,7 @@ struct pci_bridge_emul {
 };
 
 enum {
-	PCI_BRIDGE_EMUL_NO_PREFETCHABLE_BAR = BIT(0),
+	PCI_BRIDGE_EMUL_NO_PREFMEM_FORWARD = BIT(0),
 };
 
 int pci_bridge_emul_init(struct pci_bridge_emul *bridge,
Signature · 1 line
2.20.1
maintainer
#6 lore raw

Re: [PATCH 03/11] PCI: pci-bridge-emul: Rename PCI_BRIDGE_EMUL_NO_PREFETCHABLE_BAR to PCI_BRIDGE_EMUL_NO_PREFMEM_FORWARD

drivers/pci/pci-bridge-emul.h struct pci_bridge_emul { · line 124

Quoted 4 lines · On Tue, 21 Dec 2021 15:14:47 +0100 Pali Rohár <[email protected]> wrote:

enum {

drivers/pci/pci-bridge-emul.h +1 -1
-	PCI_BRIDGE_EMUL_NO_PREFETCHABLE_BAR = BIT(0),
+	PCI_BRIDGE_EMUL_NO_PREFMEM_FORWARD = BIT(0),
 };

Documenting the flag here would be useful IMO, just copy/paste what you have in your commit log. Can be done later, of course.

Thomas

Signature · 3 lines
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
author maintainer
#7 lore raw

PATCH 4/11 PCI: pci-bridge-emul: Add support for new flag PCI_BRIDGE_EMUL_NO_IO_FORWARD

In reply to Pali Rohár

Like PCI_BRIDGE_EMUL_NO_PREFMEM_FORWARD, this new flag specifies that emulated PCI bridge does not support forwarding of IO requests in given range between primary and secondary buses. This flag should be used as argument for pci_bridge_emul_init() for hardware setup without IO support.

Setting this flag cause that IO base and limit registers are read-only.

---
 drivers/pci/pci-bridge-emul.c | 9 +++++++++
 drivers/pci/pci-bridge-emul.h | 1 +
 2 files changed, 10 insertions(+)
drivers/pci/pci-bridge-emul.c +9 -0
index 432b1bec2e22..033bbeb99176 100644
@@ -378,6 +378,15 @@ int pci_bridge_emul_init(struct pci_bridge_emul *bridge,
 		bridge->pci_regs_behavior[PCI_PREF_MEMORY_BASE / 4].rw = 0;
 	}
 
+	if (flags & PCI_BRIDGE_EMUL_NO_IO_FORWARD) {
+		bridge->pci_regs_behavior[PCI_COMMAND / 4].ro |= PCI_COMMAND_IO;
+		bridge->pci_regs_behavior[PCI_COMMAND / 4].rw &= ~PCI_COMMAND_IO;
+		bridge->pci_regs_behavior[PCI_IO_BASE / 4].ro |= GENMASK(15, 0);
+		bridge->pci_regs_behavior[PCI_IO_BASE / 4].rw &= ~GENMASK(15, 0);
+		bridge->pci_regs_behavior[PCI_IO_BASE_UPPER16 / 4].ro = ~0;
+		bridge->pci_regs_behavior[PCI_IO_BASE_UPPER16 / 4].rw = 0;
+	}
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(pci_bridge_emul_init);
drivers/pci/pci-bridge-emul.h +1 -0
index 88877ebefbac..ab33609c598b 100644
@@ -121,6 +121,7 @@ struct pci_bridge_emul {
 
 enum {
 	PCI_BRIDGE_EMUL_NO_PREFMEM_FORWARD = BIT(0),
+	PCI_BRIDGE_EMUL_NO_IO_FORWARD = BIT(1),
 };
 
 int pci_bridge_emul_init(struct pci_bridge_emul *bridge,
Signature · 1 line
2.20.1
maintainer
#8 lore raw

Re: [PATCH 04/11] PCI: pci-bridge-emul: Add support for new flag PCI_BRIDGE_EMUL_NO_IO_FORWARD

Quoted 8 lines · On Tue, 21 Dec 2021 15:14:48 +0100 Pali Rohár <[email protected]> wrote:

Like PCI_BRIDGE_EMUL_NO_PREFMEM_FORWARD, this new flag specifies that emulated PCI bridge does not support forwarding of IO requests in given range between primary and secondary buses. This flag should be used as argument for pci_bridge_emul_init() for hardware setup without IO support.

Setting this flag cause that IO base and limit registers are read-only.

Looks good. As said on PATCH 03/11, perhaps just a snippet of documentation in pci-bridge-emul.h to describe the semantic of the new flag.

Thomas

Signature · 3 lines
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
author maintainer
#9 lore raw

PATCH 5/11 PCI: mvebu: Add help string for CONFIG_PCI_MVEBU option

In reply to Pali Rohár

There is no description for CONFIG_PCI_MVEBU option. Add it.

---
 drivers/pci/controller/Kconfig | 4 ++++
 1 file changed, 4 insertions(+)
drivers/pci/controller/Kconfig +4 -0
index 67189bcd5d89..534b446f2cf0 100644
@@ -10,6 +10,10 @@ config PCI_MVEBU
 	depends on ARM
 	depends on OF
 	select PCI_BRIDGE_EMUL
+	help
+	 Add support for Marvell EBU PCIe controller. This PCIe controller
+	 is used on 32-bit Marvell ARM SoCs: Dove, Kirkwood, Armada 370,
+	 Armada XP, Armada 375, Armada 38x and Armada 39x.
 
 config PCI_AARDVARK
 	tristate "Aardvark PCIe controller"
Signature · 1 line
2.20.1
maintainer Acked-by on v1 5/11
#10 lore raw

Re: [PATCH 05/11] PCI: mvebu: Add help string for CONFIG_PCI_MVEBU option

Quoted 3 lines · On Tue, 21 Dec 2021 15:14:49 +0100 Pali Rohár <[email protected]> wrote:

There is no description for CONFIG_PCI_MVEBU option. Add it.

Signature · 3 lines
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
author maintainer
#11 lore raw

PATCH 6/11 PCI: mvebu: Remove duplicate nports assignment

In reply to Pali Rohár

Member pcie->nports is initialized to correct value before the previous for-loop. There is not need to initialize it more times.

---
 drivers/pci/controller/pci-mvebu.c | 2 --
 1 file changed, 2 deletions(-)
drivers/pci/controller/pci-mvebu.c +0 -2
index 2e10ade660a1..016f709b3067 100644
@@ -1337,8 +1337,6 @@ static int mvebu_pcie_probe(struct platform_device *pdev)
 		mvebu_pcie_set_local_bus_nr(port, 0);
 	}
 
-	pcie->nports = i;
-
 	bridge->sysdata = pcie;
 	bridge->ops = &mvebu_pcie_ops;
 	bridge->align_resource = mvebu_pcie_align_resource;
Signature · 1 line
2.20.1
maintainer Acked-by on v1 6/11
#12 lore raw

Re: [PATCH 06/11] PCI: mvebu: Remove duplicate nports assignment

Quoted 4 lines · On Tue, 21 Dec 2021 15:14:50 +0100 Pali Rohár <[email protected]> wrote:

Member pcie->nports is initialized to correct value before the previous for-loop. There is not need to initialize it more times.

Signature · 3 lines
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
author maintainer
#13 lore raw

PATCH 7/11 PCI: mvebu: Set PCI_BRIDGE_EMUL_NO_IO_FORWARD when IO is unsupported

In reply to Pali Rohár

This will make PCI bridge to return zeros when accessing IO base and limit registers, as required by PCIe base specification.

This allows to remove adhoc checks around mvebu_pcie_handle_iobase_change() function for unsupported IO ranges. PCI_BRIDGE_EMUL_NO_IO_FORWARD ensures that there will be no non-zeros write to IO registers when IO is not supported.

---
 drivers/pci/controller/pci-mvebu.c | 29 ++++++++++-------------------
 1 file changed, 10 insertions(+), 19 deletions(-)
drivers/pci/controller/pci-mvebu.c +10 -19
index 016f709b3067..551f55af5226 100644
@@ -438,12 +438,6 @@ static int mvebu_pcie_handle_iobase_change(struct mvebu_pcie_port *port)
 		return mvebu_pcie_set_window(port, port->io_target, port->io_attr,
 					     &desired, &port->iowin);
 
-	if (!mvebu_has_ioport(port)) {
-		dev_WARN(&port->pcie->pdev->dev,
-			 "Attempt to set IO when IO is disabled\n");
-		return -EOPNOTSUPP;
-	}
-
 	/*
 	 * We read the PCI-to-PCI bridge emulated registers, and
 	 * calculate the base address and size of the address decoding
@@ -599,24 +593,18 @@ mvebu_pci_bridge_emul_base_conf_write(struct pci_bridge_emul *bridge,
 
 	switch (reg) {
 	case PCI_COMMAND:
-		if (!mvebu_has_ioport(port)) {
-			conf->command = cpu_to_le16(
-				le16_to_cpu(conf->command) & ~PCI_COMMAND_IO);
-			new &= ~PCI_COMMAND_IO;
-		}
-
 		mvebu_writel(port, new, PCIE_CMD_OFF);
 		break;
 
 	case PCI_IO_BASE:
-		if ((mask & 0xffff) && mvebu_pcie_handle_iobase_change(port)) {
+		if ((mask & 0xffff) && mvebu_has_ioport(port) &&
+		    mvebu_pcie_handle_iobase_change(port)) {
 			/* On error disable IO range */
 			conf->iobase &= ~0xf0;
 			conf->iolimit &= ~0xf0;
+			conf->iobase |= 0xf0;
 			conf->iobaseupper = cpu_to_le16(0x0000);
 			conf->iolimitupper = cpu_to_le16(0x0000);
-			if (mvebu_has_ioport(port))
-				conf->iobase |= 0xf0;
 		}
 		break;
 
@@ -630,14 +618,14 @@ mvebu_pci_bridge_emul_base_conf_write(struct pci_bridge_emul *bridge,
 		break;
 
 	case PCI_IO_BASE_UPPER16:
-		if (mvebu_pcie_handle_iobase_change(port)) {
+		if (mvebu_has_ioport(port) &&
+		    mvebu_pcie_handle_iobase_change(port)) {
 			/* On error disable IO range */
 			conf->iobase &= ~0xf0;
 			conf->iolimit &= ~0xf0;
+			conf->iobase |= 0xf0;
 			conf->iobaseupper = cpu_to_le16(0x0000);
 			conf->iolimitupper = cpu_to_le16(0x0000);
-			if (mvebu_has_ioport(port))
-				conf->iobase |= 0xf0;
 		}
 		break;
 
@@ -722,6 +710,7 @@ static const struct pci_bridge_emul_ops mvebu_pci_bridge_emul_ops = {
  */
 static int mvebu_pci_bridge_emul_init(struct mvebu_pcie_port *port)
 {
+	unsigned int bridge_flags = PCI_BRIDGE_EMUL_NO_PREFMEM_FORWARD;
 	struct pci_bridge_emul *bridge = &port->bridge;
 	u32 pcie_cap = mvebu_readl(port, PCIE_CAP_PCIEXP);
 	u8 pcie_cap_ver = ((pcie_cap >> 16) & PCI_EXP_FLAGS_VERS);
@@ -735,6 +724,8 @@ static int mvebu_pci_bridge_emul_init(struct mvebu_pcie_port *port)
 		/* We support 32 bits I/O addressing */
 		bridge->conf.iobase = PCI_IO_RANGE_TYPE_32;
 		bridge->conf.iolimit = PCI_IO_RANGE_TYPE_32;
+	} else {
+		bridge_flags |= PCI_BRIDGE_EMUL_NO_IO_FORWARD;
 	}
 
 	/*
@@ -747,7 +738,7 @@ static int mvebu_pci_bridge_emul_init(struct mvebu_pcie_port *port)
 	bridge->data = port;
 	bridge->ops = &mvebu_pci_bridge_emul_ops;
 
-	return pci_bridge_emul_init(bridge, PCI_BRIDGE_EMUL_NO_PREFMEM_FORWARD);
+	return pci_bridge_emul_init(bridge, bridge_flags);
 }
 
 static inline struct mvebu_pcie *sys_to_pcie(struct pci_sys_data *sys)
Signature · 1 line
2.20.1
author maintainer
#14 lore raw

PATCH 8/11 PCI: mvebu: Properly initialize vendor, device and revision of emulated bridge

In reply to Pali Rohár

With this change also PCI vendor id is read from mvebu registers.

---
 drivers/pci/controller/pci-mvebu.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
drivers/pci/controller/pci-mvebu.c +5 -4
index 551f55af5226..94ef00b6d697 100644
@@ -712,13 +712,14 @@ static int mvebu_pci_bridge_emul_init(struct mvebu_pcie_port *port)
 {
 	unsigned int bridge_flags = PCI_BRIDGE_EMUL_NO_PREFMEM_FORWARD;
 	struct pci_bridge_emul *bridge = &port->bridge;
+	u32 dev_id = mvebu_readl(port, PCIE_DEV_ID_OFF);
+	u32 dev_rev = mvebu_readl(port, PCIE_DEV_REV_OFF);
 	u32 pcie_cap = mvebu_readl(port, PCIE_CAP_PCIEXP);
 	u8 pcie_cap_ver = ((pcie_cap >> 16) & PCI_EXP_FLAGS_VERS);
 
-	bridge->conf.vendor = PCI_VENDOR_ID_MARVELL;
-	bridge->conf.device = mvebu_readl(port, PCIE_DEV_ID_OFF) >> 16;
-	bridge->conf.class_revision =
-		mvebu_readl(port, PCIE_DEV_REV_OFF) & 0xff;
+	bridge->conf.vendor = cpu_to_le16(dev_id & 0xffff);
+	bridge->conf.device = cpu_to_le16(dev_id >> 16);
+	bridge->conf.class_revision = cpu_to_le32(dev_rev & 0xff);
 
 	if (mvebu_has_ioport(port)) {
 		/* We support 32 bits I/O addressing */
Signature · 1 line
2.20.1
author maintainer
#15 lore raw

PATCH 9/11 PCI: mvebu: Update comment for PCI_EXP_LNKCAP register on emulated bridge

In reply to Pali Rohár

Reason for clearing this bit is because mvebu hw returns incorrectly this bit set to 1.

---
 drivers/pci/controller/pci-mvebu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/pci/controller/pci-mvebu.c +2 -2
index 94ef00b6d697..1aac65977b97 100644
@@ -546,8 +546,8 @@ mvebu_pci_bridge_emul_pcie_conf_read(struct pci_bridge_emul *bridge,
 
 	case PCI_EXP_LNKCAP:
 		/*
-		 * PCIe requires the clock power management capability to be
-		 * hard-wired to zero for downstream ports
+		 * PCIe requires that the Clock Power Management capability bit
+		 * is hard-wired to zero for downstream ports but HW returns 1.
 		 */
 		*value = mvebu_readl(port, PCIE_CAP_PCIEXP + PCI_EXP_LNKCAP) &
 			 ~PCI_EXP_LNKCAP_CLKPM;
Signature · 1 line
2.20.1
author maintainer
#16 lore raw

PATCH 10/11 PCI: mvebu: Update comment for PCI_EXP_LNKCTL register on emulated bridge

In reply to Pali Rohár

Logic and code for clearing PCI_EXP_LNKCTL_CLKREQ_EN bit is correct, but comment describing it is misleading. PCI_EXP_LNKCTL_CLKREQ_EN bit should be hardwired to zero but mvebu hw allows to change it.

---
 drivers/pci/controller/pci-mvebu.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)
drivers/pci/controller/pci-mvebu.c +3 -4
index 1aac65977b97..dffa330de174 100644
@@ -663,10 +663,9 @@ mvebu_pci_bridge_emul_pcie_conf_write(struct pci_bridge_emul *bridge,
 
 	case PCI_EXP_LNKCTL:
 		/*
-		 * If we don't support CLKREQ, we must ensure that the
-		 * CLKREQ enable bit always reads zero.  Since we haven't
-		 * had this capability, and it's dependent on board wiring,
-		 * disable it for the time being.
+		 * PCIe requires that the Enable Clock Power Management bit
+		 * is hard-wired to zero for downstream ports but HW allows
+		 * to change it.
 		 */
 		new &= ~PCI_EXP_LNKCTL_CLKREQ_EN;
 
Signature · 1 line
2.20.1
author maintainer
#17 lore raw

PATCH 11/11 PCI: mvebu: Fix reporting Data Link Layer Link Active on emulated bridge

In reply to Pali Rohár

Add support for reporting PCI_EXP_LNKSTA_DLLLA bit in Link Control register on emulated bridge via PCIE_STAT_OFF reg. Function mvebu_pcie_link_up() already parses this register and returns if Data Link is Active or not.

Also correctly indicate DLLLA capability via PCI_EXP_LNKCAP_DLLLARC bit in Link Control Capability register which is required for reporting DLLLA bit.

---
 drivers/pci/controller/pci-mvebu.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)
drivers/pci/controller/pci-mvebu.c +8 -3
index dffa330de174..a075ba26cff1 100644
@@ -548,13 +548,18 @@ mvebu_pci_bridge_emul_pcie_conf_read(struct pci_bridge_emul *bridge,
 		/*
 		 * PCIe requires that the Clock Power Management capability bit
 		 * is hard-wired to zero for downstream ports but HW returns 1.
+		 * Additionally enable Data Link Layer Link Active Reporting
+		 * Capable bit as DL_Active indication is provided too.
 		 */
-		*value = mvebu_readl(port, PCIE_CAP_PCIEXP + PCI_EXP_LNKCAP) &
-			 ~PCI_EXP_LNKCAP_CLKPM;
+		*value = (mvebu_readl(port, PCIE_CAP_PCIEXP + PCI_EXP_LNKCAP) &
+			  ~PCI_EXP_LNKCAP_CLKPM) | PCI_EXP_LNKCAP_DLLLARC;
 		break;
 
 	case PCI_EXP_LNKCTL:
-		*value = mvebu_readl(port, PCIE_CAP_PCIEXP + PCI_EXP_LNKCTL);
+		/* DL_Active indication is provided via PCIE_STAT_OFF */
+		*value = mvebu_readl(port, PCIE_CAP_PCIEXP + PCI_EXP_LNKCTL) |
+			 (mvebu_pcie_link_up(port) ?
+			  (PCI_EXP_LNKSTA_DLLLA << 16) : 0);
 		break;
 
 	case PCI_EXP_SLTCTL:
Signature · 1 line
2.20.1
author maintainer
#18 lore raw

v2 cover PCI: Small improvements for pci-bridge-emul and mvebu

In reply to Pali Rohár

This patch series contains small improvements for pci-bridge-emul and mvebu drivers. This patch series is based on top of the patches:
https://lore.kernel.org/linux-pci/[email protected]/
(which are now in pci/mvebu branch)

In V2 was added comment into code explaining PCI_BRIDGE_EMUL_NO_PREFMEM_FORWARD and PCI_BRIDGE_EMUL_NO_IO_FORWARD flags.

Pali Rohár (11):
MAINTAINERS: Add Pali Rohár as pci-mvebu.c maintainer
PCI: pci-bridge-emul: Make struct pci_bridge_emul_ops as const
PCI: pci-bridge-emul: Rename PCI_BRIDGE_EMUL_NO_PREFETCHABLE_BAR to
PCI_BRIDGE_EMUL_NO_PREFMEM_FORWARD
PCI: pci-bridge-emul: Add support for new flag
PCI_BRIDGE_EMUL_NO_IO_FORWARD
PCI: mvebu: Add help string for CONFIG_PCI_MVEBU option
PCI: mvebu: Remove duplicate nports assignment
PCI: mvebu: Set PCI_BRIDGE_EMUL_NO_IO_FORWARD when IO is unsupported
PCI: mvebu: Properly initialize vendor, device and revision of
emulated bridge
PCI: mvebu: Update comment for PCI_EXP_LNKCAP register on emulated
bridge
PCI: mvebu: Update comment for PCI_EXP_LNKCTL register on emulated
bridge
PCI: mvebu: Fix reporting Data Link Layer Link Active on emulated
bridge

 MAINTAINERS                           |  1 +
 drivers/pci/controller/Kconfig        |  4 ++
 drivers/pci/controller/pci-aardvark.c |  2 +-
 drivers/pci/controller/pci-mvebu.c    | 64 ++++++++++++---------------
 drivers/pci/pci-bridge-emul.c         | 11 ++++-
 drivers/pci/pci-bridge-emul.h         | 14 +++++-
 6 files changed, 57 insertions(+), 39 deletions(-)
Signature · 1 line
2.20.1
author maintainer
#19 lore raw

PATCH v2 1/11 MAINTAINERS: Add Pali Rohár as pci-mvebu.c maintainer

---
I discussed with Thomas and he ask me for taking maintenance of pci-mvebu.c driver.

---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)
MAINTAINERS +1 -0
index 7a2345ce8521..24527789d933 100644
@@ -14617,6 +14617,7 @@ F:	drivers/pci/controller/mobiveil/pcie-mobiveil*
 
 PCI DRIVER FOR MVEBU (Marvell Armada 370 and Armada XP SOC support)
 M:	Thomas Petazzoni <[email protected]>
+M:	Pali Rohár <[email protected]>
 L:	[email protected]
 L:	[email protected] (moderated for non-subscribers)
 S:	Maintained
Signature · 1 line
2.20.1
author maintainer
#20 lore raw

PATCH v2 2/11 PCI: pci-bridge-emul: Make struct pci_bridge_emul_ops as const

In reply to Pali Rohár

It is read-only constant structure, so properly mark it with const keyword.

---
 drivers/pci/controller/pci-aardvark.c | 2 +-
 drivers/pci/controller/pci-mvebu.c    | 2 +-
 drivers/pci/pci-bridge-emul.h         | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)
drivers/pci/controller/pci-aardvark.c +1 -1
index 62fc55f2ed40..1fa6fe1e022a 100644
@@ -928,7 +928,7 @@ advk_pci_bridge_emul_pcie_conf_write(struct pci_bridge_emul *bridge,
 	}
 }
 
-static struct pci_bridge_emul_ops advk_pci_bridge_emul_ops = {
+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,
drivers/pci/controller/pci-mvebu.c +1 -1
index 68aa94a258ff..2ecc1ab12249 100644
@@ -709,7 +709,7 @@ mvebu_pci_bridge_emul_pcie_conf_write(struct pci_bridge_emul *bridge,
 	}
 }
 
-static struct pci_bridge_emul_ops mvebu_pci_bridge_emul_ops = {
+static const struct pci_bridge_emul_ops mvebu_pci_bridge_emul_ops = {
 	.read_base = mvebu_pci_bridge_emul_base_conf_read,
 	.write_base = mvebu_pci_bridge_emul_base_conf_write,
 	.read_pcie = mvebu_pci_bridge_emul_pcie_conf_read,
drivers/pci/pci-bridge-emul.h +1 -1
index 49bbd37ee318..0690b6369755 100644
@@ -112,7 +112,7 @@ struct pci_bridge_reg_behavior;
 struct pci_bridge_emul {
 	struct pci_bridge_emul_conf conf;
 	struct pci_bridge_emul_pcie_conf pcie_conf;
-	struct pci_bridge_emul_ops *ops;
+	const struct pci_bridge_emul_ops *ops;
 	struct pci_bridge_reg_behavior *pci_regs_behavior;
 	struct pci_bridge_reg_behavior *pcie_cap_regs_behavior;
 	void *data;
Signature · 1 line
2.20.1
author maintainer
#21 lore raw

PATCH v2 3/11 PCI: pci-bridge-emul: Rename PCI_BRIDGE_EMUL_NO_PREFETCHABLE_BAR to PCI_BRIDGE_EMUL_NO_PREFMEM_FORWARD

In reply to Pali Rohár

This flag describe whether PCI bridge supports forwarding of prefetchable memory requests in given range between primary and secondary buses. It does not specify if bridge has support for prefetchable memory BAR (moreover this pci-bridge-emul.c driver does not provide support for BARs).

So change name of this flag to be less misleading and add comment.

---
Changes in v2:
* Add comment into code.

---
 drivers/pci/controller/pci-mvebu.c | 2 +-
 drivers/pci/pci-bridge-emul.c      | 2 +-
 drivers/pci/pci-bridge-emul.h      | 6 +++++-
 3 files changed, 7 insertions(+), 3 deletions(-)
drivers/pci/controller/pci-mvebu.c +1 -1
index 2ecc1ab12249..2e10ade660a1 100644
@@ -747,7 +747,7 @@ static int mvebu_pci_bridge_emul_init(struct mvebu_pcie_port *port)
 	bridge->data = port;
 	bridge->ops = &mvebu_pci_bridge_emul_ops;
 
-	return pci_bridge_emul_init(bridge, PCI_BRIDGE_EMUL_NO_PREFETCHABLE_BAR);
+	return pci_bridge_emul_init(bridge, PCI_BRIDGE_EMUL_NO_PREFMEM_FORWARD);
 }
 
 static inline struct mvebu_pcie *sys_to_pcie(struct pci_sys_data *sys)
drivers/pci/pci-bridge-emul.c +1 -1
index 79b947528455..432b1bec2e22 100644
@@ -373,7 +373,7 @@ int pci_bridge_emul_init(struct pci_bridge_emul *bridge,
 			~(BIT(10) << 16);
 	}
 
-	if (flags & PCI_BRIDGE_EMUL_NO_PREFETCHABLE_BAR) {
+	if (flags & PCI_BRIDGE_EMUL_NO_PREFMEM_FORWARD) {
 		bridge->pci_regs_behavior[PCI_PREF_MEMORY_BASE / 4].ro = ~0;
 		bridge->pci_regs_behavior[PCI_PREF_MEMORY_BASE / 4].rw = 0;
 	}
drivers/pci/pci-bridge-emul.h +5 -1
index 0690b6369755..087b5fa01bcf 100644
@@ -120,7 +120,11 @@ struct pci_bridge_emul {
 };
 
 enum {
-	PCI_BRIDGE_EMUL_NO_PREFETCHABLE_BAR = BIT(0),
+	/*
+	 * PCI bridge does not support forwarding of prefetchable memory
+	 * requests between primary and secondary buses.
+	 */
+	PCI_BRIDGE_EMUL_NO_PREFMEM_FORWARD = BIT(0),
 };
 
 int pci_bridge_emul_init(struct pci_bridge_emul *bridge,
Signature · 1 line
2.20.1
author maintainer
#22 lore raw

PATCH v2 4/11 PCI: pci-bridge-emul: Add support for new flag PCI_BRIDGE_EMUL_NO_IO_FORWARD

In reply to Pali Rohár

Like PCI_BRIDGE_EMUL_NO_PREFMEM_FORWARD, this new flag specifies that emulated PCI bridge does not support forwarding of IO requests in given range between primary and secondary buses. This flag should be used as argument for pci_bridge_emul_init() for hardware setup without IO support.

Setting this flag cause that IO base and limit registers are read-only.

---
Changes in v2:
* Add comment into code.

---
 drivers/pci/pci-bridge-emul.c | 9 +++++++++
 drivers/pci/pci-bridge-emul.h | 6 ++++++
 2 files changed, 15 insertions(+)
drivers/pci/pci-bridge-emul.c +9 -0
index 432b1bec2e22..033bbeb99176 100644
@@ -378,6 +378,15 @@ int pci_bridge_emul_init(struct pci_bridge_emul *bridge,
 		bridge->pci_regs_behavior[PCI_PREF_MEMORY_BASE / 4].rw = 0;
 	}
 
+	if (flags & PCI_BRIDGE_EMUL_NO_IO_FORWARD) {
+		bridge->pci_regs_behavior[PCI_COMMAND / 4].ro |= PCI_COMMAND_IO;
+		bridge->pci_regs_behavior[PCI_COMMAND / 4].rw &= ~PCI_COMMAND_IO;
+		bridge->pci_regs_behavior[PCI_IO_BASE / 4].ro |= GENMASK(15, 0);
+		bridge->pci_regs_behavior[PCI_IO_BASE / 4].rw &= ~GENMASK(15, 0);
+		bridge->pci_regs_behavior[PCI_IO_BASE_UPPER16 / 4].ro = ~0;
+		bridge->pci_regs_behavior[PCI_IO_BASE_UPPER16 / 4].rw = 0;
+	}
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(pci_bridge_emul_init);
drivers/pci/pci-bridge-emul.h +6 -0
index 087b5fa01bcf..4953274cac18 100644
@@ -125,6 +125,12 @@ enum {
 	 * requests between primary and secondary buses.
 	 */
 	PCI_BRIDGE_EMUL_NO_PREFMEM_FORWARD = BIT(0),
+
+	/*
+	 * PCI bridge does not support forwarding of IO requests between
+	 * primary and secondary buses.
+	 */
+	PCI_BRIDGE_EMUL_NO_IO_FORWARD = BIT(1),
 };
 
 int pci_bridge_emul_init(struct pci_bridge_emul *bridge,
Signature · 1 line
2.20.1
author maintainer
#23 lore raw

PATCH v2 5/11 PCI: mvebu: Add help string for CONFIG_PCI_MVEBU option

In reply to Pali Rohár

There is no description for CONFIG_PCI_MVEBU option. Add it.

---
 drivers/pci/controller/Kconfig | 4 ++++
 1 file changed, 4 insertions(+)
drivers/pci/controller/Kconfig +4 -0
index 67189bcd5d89..534b446f2cf0 100644
@@ -10,6 +10,10 @@ config PCI_MVEBU
 	depends on ARM
 	depends on OF
 	select PCI_BRIDGE_EMUL
+	help
+	 Add support for Marvell EBU PCIe controller. This PCIe controller
+	 is used on 32-bit Marvell ARM SoCs: Dove, Kirkwood, Armada 370,
+	 Armada XP, Armada 375, Armada 38x and Armada 39x.
 
 config PCI_AARDVARK
 	tristate "Aardvark PCIe controller"
Signature · 1 line
2.20.1
author maintainer
#24 lore raw

PATCH v2 6/11 PCI: mvebu: Remove duplicate nports assignment

In reply to Pali Rohár

Member pcie->nports is initialized to correct value before the previous for-loop. There is not need to initialize it more times.

---
 drivers/pci/controller/pci-mvebu.c | 2 --
 1 file changed, 2 deletions(-)
drivers/pci/controller/pci-mvebu.c +0 -2
index 2e10ade660a1..016f709b3067 100644
@@ -1337,8 +1337,6 @@ static int mvebu_pcie_probe(struct platform_device *pdev)
 		mvebu_pcie_set_local_bus_nr(port, 0);
 	}
 
-	pcie->nports = i;
-
 	bridge->sysdata = pcie;
 	bridge->ops = &mvebu_pcie_ops;
 	bridge->align_resource = mvebu_pcie_align_resource;
Signature · 1 line
2.20.1
author maintainer
#25 lore raw

PATCH v2 7/11 PCI: mvebu: Set PCI_BRIDGE_EMUL_NO_IO_FORWARD when IO is unsupported

In reply to Pali Rohár

This will make PCI bridge to return zeros when accessing IO base and limit registers, as required by PCIe base specification.

This allows to remove adhoc checks around mvebu_pcie_handle_iobase_change() function for unsupported IO ranges. PCI_BRIDGE_EMUL_NO_IO_FORWARD ensures that there will be no non-zeros write to IO registers when IO is not supported.

---
 drivers/pci/controller/pci-mvebu.c | 29 ++++++++++-------------------
 1 file changed, 10 insertions(+), 19 deletions(-)
drivers/pci/controller/pci-mvebu.c +10 -19
index 016f709b3067..551f55af5226 100644
@@ -438,12 +438,6 @@ static int mvebu_pcie_handle_iobase_change(struct mvebu_pcie_port *port)
 		return mvebu_pcie_set_window(port, port->io_target, port->io_attr,
 					     &desired, &port->iowin);
 
-	if (!mvebu_has_ioport(port)) {
-		dev_WARN(&port->pcie->pdev->dev,
-			 "Attempt to set IO when IO is disabled\n");
-		return -EOPNOTSUPP;
-	}
-
 	/*
 	 * We read the PCI-to-PCI bridge emulated registers, and
 	 * calculate the base address and size of the address decoding
@@ -599,24 +593,18 @@ mvebu_pci_bridge_emul_base_conf_write(struct pci_bridge_emul *bridge,
 
 	switch (reg) {
 	case PCI_COMMAND:
-		if (!mvebu_has_ioport(port)) {
-			conf->command = cpu_to_le16(
-				le16_to_cpu(conf->command) & ~PCI_COMMAND_IO);
-			new &= ~PCI_COMMAND_IO;
-		}
-
 		mvebu_writel(port, new, PCIE_CMD_OFF);
 		break;
 
 	case PCI_IO_BASE:
-		if ((mask & 0xffff) && mvebu_pcie_handle_iobase_change(port)) {
+		if ((mask & 0xffff) && mvebu_has_ioport(port) &&
+		    mvebu_pcie_handle_iobase_change(port)) {
 			/* On error disable IO range */
 			conf->iobase &= ~0xf0;
 			conf->iolimit &= ~0xf0;
+			conf->iobase |= 0xf0;
 			conf->iobaseupper = cpu_to_le16(0x0000);
 			conf->iolimitupper = cpu_to_le16(0x0000);
-			if (mvebu_has_ioport(port))
-				conf->iobase |= 0xf0;
 		}
 		break;
 
@@ -630,14 +618,14 @@ mvebu_pci_bridge_emul_base_conf_write(struct pci_bridge_emul *bridge,
 		break;
 
 	case PCI_IO_BASE_UPPER16:
-		if (mvebu_pcie_handle_iobase_change(port)) {
+		if (mvebu_has_ioport(port) &&
+		    mvebu_pcie_handle_iobase_change(port)) {
 			/* On error disable IO range */
 			conf->iobase &= ~0xf0;
 			conf->iolimit &= ~0xf0;
+			conf->iobase |= 0xf0;
 			conf->iobaseupper = cpu_to_le16(0x0000);
 			conf->iolimitupper = cpu_to_le16(0x0000);
-			if (mvebu_has_ioport(port))
-				conf->iobase |= 0xf0;
 		}
 		break;
 
@@ -722,6 +710,7 @@ static const struct pci_bridge_emul_ops mvebu_pci_bridge_emul_ops = {
  */
 static int mvebu_pci_bridge_emul_init(struct mvebu_pcie_port *port)
 {
+	unsigned int bridge_flags = PCI_BRIDGE_EMUL_NO_PREFMEM_FORWARD;
 	struct pci_bridge_emul *bridge = &port->bridge;
 	u32 pcie_cap = mvebu_readl(port, PCIE_CAP_PCIEXP);
 	u8 pcie_cap_ver = ((pcie_cap >> 16) & PCI_EXP_FLAGS_VERS);
@@ -735,6 +724,8 @@ static int mvebu_pci_bridge_emul_init(struct mvebu_pcie_port *port)
 		/* We support 32 bits I/O addressing */
 		bridge->conf.iobase = PCI_IO_RANGE_TYPE_32;
 		bridge->conf.iolimit = PCI_IO_RANGE_TYPE_32;
+	} else {
+		bridge_flags |= PCI_BRIDGE_EMUL_NO_IO_FORWARD;
 	}
 
 	/*
@@ -747,7 +738,7 @@ static int mvebu_pci_bridge_emul_init(struct mvebu_pcie_port *port)
 	bridge->data = port;
 	bridge->ops = &mvebu_pci_bridge_emul_ops;
 
-	return pci_bridge_emul_init(bridge, PCI_BRIDGE_EMUL_NO_PREFMEM_FORWARD);
+	return pci_bridge_emul_init(bridge, bridge_flags);
 }
 
 static inline struct mvebu_pcie *sys_to_pcie(struct pci_sys_data *sys)
Signature · 1 line
2.20.1
author maintainer
#26 lore raw

PATCH v2 8/11 PCI: mvebu: Properly initialize vendor, device and revision of emulated bridge

In reply to Pali Rohár

With this change also PCI vendor id is read from mvebu registers.

---
 drivers/pci/controller/pci-mvebu.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
drivers/pci/controller/pci-mvebu.c +5 -4
index 551f55af5226..94ef00b6d697 100644
@@ -712,13 +712,14 @@ static int mvebu_pci_bridge_emul_init(struct mvebu_pcie_port *port)
 {
 	unsigned int bridge_flags = PCI_BRIDGE_EMUL_NO_PREFMEM_FORWARD;
 	struct pci_bridge_emul *bridge = &port->bridge;
+	u32 dev_id = mvebu_readl(port, PCIE_DEV_ID_OFF);
+	u32 dev_rev = mvebu_readl(port, PCIE_DEV_REV_OFF);
 	u32 pcie_cap = mvebu_readl(port, PCIE_CAP_PCIEXP);
 	u8 pcie_cap_ver = ((pcie_cap >> 16) & PCI_EXP_FLAGS_VERS);
 
-	bridge->conf.vendor = PCI_VENDOR_ID_MARVELL;
-	bridge->conf.device = mvebu_readl(port, PCIE_DEV_ID_OFF) >> 16;
-	bridge->conf.class_revision =
-		mvebu_readl(port, PCIE_DEV_REV_OFF) & 0xff;
+	bridge->conf.vendor = cpu_to_le16(dev_id & 0xffff);
+	bridge->conf.device = cpu_to_le16(dev_id >> 16);
+	bridge->conf.class_revision = cpu_to_le32(dev_rev & 0xff);
 
 	if (mvebu_has_ioport(port)) {
 		/* We support 32 bits I/O addressing */
Signature · 1 line
2.20.1
author maintainer
#27 lore raw

PATCH v2 9/11 PCI: mvebu: Update comment for PCI_EXP_LNKCAP register on emulated bridge

In reply to Pali Rohár

Reason for clearing this bit is because mvebu hw returns incorrectly this bit set to 1.

---
 drivers/pci/controller/pci-mvebu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/pci/controller/pci-mvebu.c +2 -2
index 94ef00b6d697..1aac65977b97 100644
@@ -546,8 +546,8 @@ mvebu_pci_bridge_emul_pcie_conf_read(struct pci_bridge_emul *bridge,
 
 	case PCI_EXP_LNKCAP:
 		/*
-		 * PCIe requires the clock power management capability to be
-		 * hard-wired to zero for downstream ports
+		 * PCIe requires that the Clock Power Management capability bit
+		 * is hard-wired to zero for downstream ports but HW returns 1.
 		 */
 		*value = mvebu_readl(port, PCIE_CAP_PCIEXP + PCI_EXP_LNKCAP) &
 			 ~PCI_EXP_LNKCAP_CLKPM;
Signature · 1 line
2.20.1
author maintainer
#28 lore raw

PATCH v2 10/11 PCI: mvebu: Update comment for PCI_EXP_LNKCTL register on emulated bridge

In reply to Pali Rohár

Logic and code for clearing PCI_EXP_LNKCTL_CLKREQ_EN bit is correct, but comment describing it is misleading. PCI_EXP_LNKCTL_CLKREQ_EN bit should be hardwired to zero but mvebu hw allows to change it.

---
 drivers/pci/controller/pci-mvebu.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)
drivers/pci/controller/pci-mvebu.c +3 -4
index 1aac65977b97..dffa330de174 100644
@@ -663,10 +663,9 @@ mvebu_pci_bridge_emul_pcie_conf_write(struct pci_bridge_emul *bridge,
 
 	case PCI_EXP_LNKCTL:
 		/*
-		 * If we don't support CLKREQ, we must ensure that the
-		 * CLKREQ enable bit always reads zero.  Since we haven't
-		 * had this capability, and it's dependent on board wiring,
-		 * disable it for the time being.
+		 * PCIe requires that the Enable Clock Power Management bit
+		 * is hard-wired to zero for downstream ports but HW allows
+		 * to change it.
 		 */
 		new &= ~PCI_EXP_LNKCTL_CLKREQ_EN;
 
Signature · 1 line
2.20.1
author maintainer
#29 lore raw

PATCH v2 11/11 PCI: mvebu: Fix reporting Data Link Layer Link Active on emulated bridge

In reply to Pali Rohár

Add support for reporting PCI_EXP_LNKSTA_DLLLA bit in Link Control register on emulated bridge via PCIE_STAT_OFF reg. Function mvebu_pcie_link_up() already parses this register and returns if Data Link is Active or not.

Also correctly indicate DLLLA capability via PCI_EXP_LNKCAP_DLLLARC bit in Link Control Capability register which is required for reporting DLLLA bit.

---
 drivers/pci/controller/pci-mvebu.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)
drivers/pci/controller/pci-mvebu.c +8 -3
index dffa330de174..a075ba26cff1 100644
@@ -548,13 +548,18 @@ mvebu_pci_bridge_emul_pcie_conf_read(struct pci_bridge_emul *bridge,
 		/*
 		 * PCIe requires that the Clock Power Management capability bit
 		 * is hard-wired to zero for downstream ports but HW returns 1.
+		 * Additionally enable Data Link Layer Link Active Reporting
+		 * Capable bit as DL_Active indication is provided too.
 		 */
-		*value = mvebu_readl(port, PCIE_CAP_PCIEXP + PCI_EXP_LNKCAP) &
-			 ~PCI_EXP_LNKCAP_CLKPM;
+		*value = (mvebu_readl(port, PCIE_CAP_PCIEXP + PCI_EXP_LNKCAP) &
+			  ~PCI_EXP_LNKCAP_CLKPM) | PCI_EXP_LNKCAP_DLLLARC;
 		break;
 
 	case PCI_EXP_LNKCTL:
-		*value = mvebu_readl(port, PCIE_CAP_PCIEXP + PCI_EXP_LNKCTL);
+		/* DL_Active indication is provided via PCIE_STAT_OFF */
+		*value = mvebu_readl(port, PCIE_CAP_PCIEXP + PCI_EXP_LNKCTL) |
+			 (mvebu_pcie_link_up(port) ?
+			  (PCI_EXP_LNKSTA_DLLLA << 16) : 0);
 		break;
 
 	case PCI_EXP_SLTCTL:
Signature · 1 line
2.20.1
author maintainer
#30 lore raw
In reply to Pali Rohár

Hello Lorenzo, could you please review these patches? There are here without any new comments for month.

Quoted 38 lines · On Tuesday 04 January 2022 16:35:18 Pali Rohár wrote:

This patch series contains small improvements for pci-bridge-emul and mvebu drivers. This patch series is based on top of the patches:
https://lore.kernel.org/linux-pci/[email protected]/
(which are now in pci/mvebu branch)

In V2 was added comment into code explaining PCI_BRIDGE_EMUL_NO_PREFMEM_FORWARD and PCI_BRIDGE_EMUL_NO_IO_FORWARD flags.

Pali Rohár (11):
MAINTAINERS: Add Pali Rohár as pci-mvebu.c maintainer
PCI: pci-bridge-emul: Make struct pci_bridge_emul_ops as const
PCI: pci-bridge-emul: Rename PCI_BRIDGE_EMUL_NO_PREFETCHABLE_BAR to
PCI_BRIDGE_EMUL_NO_PREFMEM_FORWARD
PCI: pci-bridge-emul: Add support for new flag
PCI_BRIDGE_EMUL_NO_IO_FORWARD
PCI: mvebu: Add help string for CONFIG_PCI_MVEBU option
PCI: mvebu: Remove duplicate nports assignment
PCI: mvebu: Set PCI_BRIDGE_EMUL_NO_IO_FORWARD when IO is unsupported
PCI: mvebu: Properly initialize vendor, device and revision of
emulated bridge
PCI: mvebu: Update comment for PCI_EXP_LNKCAP register on emulated
bridge
PCI: mvebu: Update comment for PCI_EXP_LNKCTL register on emulated
bridge
PCI: mvebu: Fix reporting Data Link Layer Link Active on emulated
bridge

 MAINTAINERS                           |  1 +
 drivers/pci/controller/Kconfig        |  4 ++
 drivers/pci/controller/pci-aardvark.c |  2 +-
 drivers/pci/controller/pci-mvebu.c    | 64 ++++++++++++---------------
 drivers/pci/pci-bridge-emul.c         | 11 ++++-
 drivers/pci/pci-bridge-emul.h         | 14 +++++-
 6 files changed, 57 insertions(+), 39 deletions(-)
Signature · 1 line
2.20.1
reviewer Reviewed-by on the v2 series
#31 lore raw
In reply to Pali Rohár
Quoted 18 lines · Pali Rohár

This patch series contains small improvements for pci-bridge-emul and mvebu drivers. This patch series is based on top of the patches:
https://lore.kernel.org/linux-pci/[email protected]/
(which are now in pci/mvebu branch)

In V2 was added comment into code explaining PCI_BRIDGE_EMUL_NO_PREFMEM_FORWARD and PCI_BRIDGE_EMUL_NO_IO_FORWARD flags.

Pali Rohár (11):
MAINTAINERS: Add Pali Rohár as pci-mvebu.c maintainer
PCI: pci-bridge-emul: Make struct pci_bridge_emul_ops as const
PCI: pci-bridge-emul: Rename PCI_BRIDGE_EMUL_NO_PREFETCHABLE_BAR to
PCI_BRIDGE_EMUL_NO_PREFMEM_FORWARD
PCI: pci-bridge-emul: Add support for new flag
PCI_BRIDGE_EMUL_NO_IO_FORWARD
PCI: mvebu: Add help string for CONFIG_PCI_MVEBU option
PCI: mvebu: Remove duplicate nports assignment
PCI: mvebu: Set PCI_BRIDGE_EMUL_NO_IO_FORWARD when IO is unsupported

Quoted 8 lines

PCI: mvebu: Properly initialize vendor, device and revision of
emulated bridge
PCI: mvebu: Update comment for PCI_EXP_LNKCAP register on emulated
bridge
PCI: mvebu: Update comment for PCI_EXP_LNKCTL register on emulated
bridge
PCI: mvebu: Fix reporting Data Link Layer Link Active on emulated
bridge

For the series,

In the future, please do not thread new versions with the old version of the series.

Rob

#32 lore raw
In reply to Pali Rohár
Quoted 9 lines · Pali Rohár

This patch series contains small improvements for pci-bridge-emul and mvebu drivers. This patch series is based on top of the patches:
https://lore.kernel.org/linux-pci/[email protected]/
(which are now in pci/mvebu branch)

In V2 was added comment into code explaining PCI_BRIDGE_EMUL_NO_PREFMEM_FORWARD and PCI_BRIDGE_EMUL_NO_IO_FORWARD flags.

[...]

Applied to pci/mvebu, thanks!

[01/11] MAINTAINERS: Add Pali Rohár as pci-mvebu.c maintainer

        https://git.kernel.org/lpieralisi/pci/c/e621e106a1
[02/11] PCI: pci-bridge-emul: Make struct pci_bridge_emul_ops as const
        https://git.kernel.org/lpieralisi/pci/c/64a70f521e
[03/11] PCI: pci-bridge-emul: Rename PCI_BRIDGE_EMUL_NO_PREFETCHABLE_BAR to PCI_BRIDGE_EMUL_NO_PREFMEM_FORWARD
        https://git.kernel.org/lpieralisi/pci/c/d3f332b568
[04/11] PCI: pci-bridge-emul: Add support for new flag PCI_BRIDGE_EMUL_NO_IO_FORWARD
        https://git.kernel.org/lpieralisi/pci/c/05241c1365
[05/11] PCI: mvebu: Add help string for CONFIG_PCI_MVEBU option
        https://git.kernel.org/lpieralisi/pci/c/7f09a4b195
[06/11] PCI: mvebu: Remove duplicate nports assignment
        https://git.kernel.org/lpieralisi/pci/c/7a02acdb5d
[07/11] PCI: mvebu: Set PCI_BRIDGE_EMUL_NO_IO_FORWARD when IO is unsupported
        https://git.kernel.org/lpieralisi/pci/c/5c88ed7985
[08/11] PCI: mvebu: Properly initialize vendor, device and revision of emulated bridge
        https://git.kernel.org/lpieralisi/pci/c/16038ebb0f
[09/11] PCI: mvebu: Update comment for PCI_EXP_LNKCAP register on emulated bridge
        https://git.kernel.org/lpieralisi/pci/c/d76a6ed096
[10/11] PCI: mvebu: Update comment for PCI_EXP_LNKCTL register on emulated bridge
        https://git.kernel.org/lpieralisi/pci/c/c94ea32c0d
[11/11] PCI: mvebu: Fix reporting Data Link Layer Link Active on emulated bridge
        https://git.kernel.org/lpieralisi/pci/c/c3bd7dc553

Thanks,
Lorenzo