linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] PCI Bridge Emulation changes for v5.8
@ 2020-04-14 20:30 Jon Derrick
  2020-04-14 20:30 ` [PATCH 1/5] PCI: pci-bridge-emul: Fix PCIe bit conflicts Jon Derrick
                   ` (4 more replies)
  0 siblings, 5 replies; 14+ messages in thread
From: Jon Derrick @ 2020-04-14 20:30 UTC (permalink / raw)
  To: linux-pci; +Cc: Bjorn Helgaas, Thomas Petazzoni, Russell King, Jon Derrick

Hi Bjorn,

These are the changes for pci-bridge-emul as applies to your origin/next.

The first two patches are fixes for existing issues.
The third patch is the conversion to GENMASK() and BIT() for clarity.
The fourth adds the missing bit definitions from PCIe 4.0 and PCIe 5.0.
The fifth patch optimizes away the unnecessary reserved member.

Jon Derrick (5):
  PCI: pci-bridge-emul: Fix PCIe bit conflicts
  PCI: pci-bridge-emul: Fix Root Cap/Status comment
  PCI: pci-bridge-emul: Convert to GENMASK and BIT
  PCI: pci-bridge-emul: Update for PCIe 5.0 r1.0
  PCI: pci-bridge-emul: Eliminate the 'reserved' member

 drivers/pci/pci-bridge-emul.c | 78 +++++++++++++++++------------------
 1 file changed, 37 insertions(+), 41 deletions(-)

-- 
2.18.1


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

* [PATCH 1/5] PCI: pci-bridge-emul: Fix PCIe bit conflicts
  2020-04-14 20:30 [PATCH 0/5] PCI Bridge Emulation changes for v5.8 Jon Derrick
@ 2020-04-14 20:30 ` Jon Derrick
  2020-05-07 19:48   ` Rob Herring
  2020-04-14 20:30 ` [PATCH 2/5] PCI: pci-bridge-emul: Fix Root Cap/Status comment Jon Derrick
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 14+ messages in thread
From: Jon Derrick @ 2020-04-14 20:30 UTC (permalink / raw)
  To: linux-pci; +Cc: Bjorn Helgaas, Thomas Petazzoni, Russell King, Jon Derrick

This patch fixes two bit conflicts in the pci-bridge-emul driver:

1. Bit 3 of Device Status (19 of Device Control) is marked as both
   Write-1-to-Clear and Read-Only. It should be Write-1-to-Clear.
   The Read-Only and Reserved bitmasks are shifted by 1 bit due to this
   error.

2. Bit 12 of Slot Control is marked as both Read-Write and Reserved.
   It should be Read-Write.

Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
---
 drivers/pci/pci-bridge-emul.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/pci-bridge-emul.c b/drivers/pci/pci-bridge-emul.c
index 4f4f54bc732e..faa414655f33 100644
--- a/drivers/pci/pci-bridge-emul.c
+++ b/drivers/pci/pci-bridge-emul.c
@@ -185,8 +185,8 @@ static const struct pci_bridge_reg_behavior pcie_cap_regs_behavior[] = {
 		 * RO, the rest is reserved
 		 */
 		.w1c = GENMASK(19, 16),
-		.ro = GENMASK(20, 19),
-		.rsvd = GENMASK(31, 21),
+		.ro = GENMASK(21, 20),
+		.rsvd = GENMASK(31, 22),
 	},
 
 	[PCI_EXP_LNKCAP / 4] = {
@@ -226,7 +226,7 @@ static const struct pci_bridge_reg_behavior pcie_cap_regs_behavior[] = {
 			PCI_EXP_SLTSTA_CC | PCI_EXP_SLTSTA_DLLSC) << 16,
 		.ro = (PCI_EXP_SLTSTA_MRLSS | PCI_EXP_SLTSTA_PDS |
 		       PCI_EXP_SLTSTA_EIS) << 16,
-		.rsvd = GENMASK(15, 12) | (GENMASK(15, 9) << 16),
+		.rsvd = GENMASK(15, 13) | (GENMASK(15, 9) << 16),
 	},
 
 	[PCI_EXP_RTCTL / 4] = {
-- 
2.18.1


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

* [PATCH 2/5] PCI: pci-bridge-emul: Fix Root Cap/Status comment
  2020-04-14 20:30 [PATCH 0/5] PCI Bridge Emulation changes for v5.8 Jon Derrick
  2020-04-14 20:30 ` [PATCH 1/5] PCI: pci-bridge-emul: Fix PCIe bit conflicts Jon Derrick
@ 2020-04-14 20:30 ` Jon Derrick
  2020-05-07 19:48   ` Rob Herring
  2020-04-14 20:30 ` [PATCH 3/5] PCI: pci-bridge-emul: Convert to GENMASK and BIT Jon Derrick
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 14+ messages in thread
From: Jon Derrick @ 2020-04-14 20:30 UTC (permalink / raw)
  To: linux-pci; +Cc: Bjorn Helgaas, Thomas Petazzoni, Russell King, Jon Derrick

The upper 16-bits of Root Control contain the Root Capabilities
register. The code instead describes the Root Status register in the
upper 16-bits, although it uses the correct bit definition for Root
Capabilities, and for Root Status in the next definition.

Fix this comment and add a comment describing the Root Status register.

Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
---
 drivers/pci/pci-bridge-emul.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pci-bridge-emul.c b/drivers/pci/pci-bridge-emul.c
index faa414655f33..c00c30ffb198 100644
--- a/drivers/pci/pci-bridge-emul.c
+++ b/drivers/pci/pci-bridge-emul.c
@@ -234,7 +234,7 @@ static const struct pci_bridge_reg_behavior pcie_cap_regs_behavior[] = {
 		 * Root control has bits [4:0] RW, the rest is
 		 * reserved.
 		 *
-		 * Root status has bit 0 RO, the rest is reserved.
+		 * Root capabilities has bit 0 RO, the rest is reserved.
 		 */
 		.rw = (PCI_EXP_RTCTL_SECEE | PCI_EXP_RTCTL_SENFEE |
 		       PCI_EXP_RTCTL_SEFEE | PCI_EXP_RTCTL_PMEIE |
@@ -244,6 +244,10 @@ static const struct pci_bridge_reg_behavior pcie_cap_regs_behavior[] = {
 	},
 
 	[PCI_EXP_RTSTA / 4] = {
+		/*
+		 * Root status has bits 17 and [15:0] RO, bit 16 W1C, the rest
+		 * is reserved.
+		 */
 		.ro = GENMASK(15, 0) | PCI_EXP_RTSTA_PENDING,
 		.w1c = PCI_EXP_RTSTA_PME,
 		.rsvd = GENMASK(31, 18),
-- 
2.18.1


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

* [PATCH 3/5] PCI: pci-bridge-emul: Convert to GENMASK and BIT
  2020-04-14 20:30 [PATCH 0/5] PCI Bridge Emulation changes for v5.8 Jon Derrick
  2020-04-14 20:30 ` [PATCH 1/5] PCI: pci-bridge-emul: Fix PCIe bit conflicts Jon Derrick
  2020-04-14 20:30 ` [PATCH 2/5] PCI: pci-bridge-emul: Fix Root Cap/Status comment Jon Derrick
@ 2020-04-14 20:30 ` Jon Derrick
  2020-04-16  7:30   ` Christoph Hellwig
  2020-04-14 20:30 ` [PATCH 4/5] PCI: pci-bridge-emul: Update for PCIe 5.0 r1.0 Jon Derrick
  2020-04-14 20:30 ` [PATCH 5/5] PCI: pci-bridge-emul: Eliminate the 'reserved' member Jon Derrick
  4 siblings, 1 reply; 14+ messages in thread
From: Jon Derrick @ 2020-04-14 20:30 UTC (permalink / raw)
  To: linux-pci; +Cc: Bjorn Helgaas, Thomas Petazzoni, Russell King, Jon Derrick

In order to make pci-bridge-emul easier to keep up-to-date with new PCIe
features, convert all named register bits to GENMASK and BIT pairs. This
patch doesn't alter any of the PCI configuration space as these bits are
fully defined.

Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
---
 drivers/pci/pci-bridge-emul.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/pci/pci-bridge-emul.c b/drivers/pci/pci-bridge-emul.c
index c00c30ffb198..bbcccadca85e 100644
--- a/drivers/pci/pci-bridge-emul.c
+++ b/drivers/pci/pci-bridge-emul.c
@@ -221,11 +221,8 @@ static const struct pci_bridge_reg_behavior pcie_cap_regs_behavior[] = {
 		 * as reserved bits.
 		 */
 		.rw = GENMASK(12, 0),
-		.w1c = (PCI_EXP_SLTSTA_ABP | PCI_EXP_SLTSTA_PFD |
-			PCI_EXP_SLTSTA_MRLSC | PCI_EXP_SLTSTA_PDC |
-			PCI_EXP_SLTSTA_CC | PCI_EXP_SLTSTA_DLLSC) << 16,
-		.ro = (PCI_EXP_SLTSTA_MRLSS | PCI_EXP_SLTSTA_PDS |
-		       PCI_EXP_SLTSTA_EIS) << 16,
+		.w1c = (BIT(8) | GENMASK(4, 0)) << 16,
+		.ro = GENMASK(7, 5) << 16,
 		.rsvd = GENMASK(15, 13) | (GENMASK(15, 9) << 16),
 	},
 
@@ -236,10 +233,8 @@ static const struct pci_bridge_reg_behavior pcie_cap_regs_behavior[] = {
 		 *
 		 * Root capabilities has bit 0 RO, the rest is reserved.
 		 */
-		.rw = (PCI_EXP_RTCTL_SECEE | PCI_EXP_RTCTL_SENFEE |
-		       PCI_EXP_RTCTL_SEFEE | PCI_EXP_RTCTL_PMEIE |
-		       PCI_EXP_RTCTL_CRSSVE),
-		.ro = PCI_EXP_RTCAP_CRSVIS << 16,
+		.rw = GENMASK(4, 0),
+		.ro = BIT(0) << 16,
 		.rsvd = GENMASK(15, 5) | (GENMASK(15, 1) << 16),
 	},
 
@@ -248,8 +243,8 @@ static const struct pci_bridge_reg_behavior pcie_cap_regs_behavior[] = {
 		 * Root status has bits 17 and [15:0] RO, bit 16 W1C, the rest
 		 * is reserved.
 		 */
-		.ro = GENMASK(15, 0) | PCI_EXP_RTSTA_PENDING,
-		.w1c = PCI_EXP_RTSTA_PME,
+		.ro = BIT(17) | GENMASK(15, 0),
+		.w1c = BIT(16),
 		.rsvd = GENMASK(31, 18),
 	},
 };
-- 
2.18.1


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

* [PATCH 4/5] PCI: pci-bridge-emul: Update for PCIe 5.0 r1.0
  2020-04-14 20:30 [PATCH 0/5] PCI Bridge Emulation changes for v5.8 Jon Derrick
                   ` (2 preceding siblings ...)
  2020-04-14 20:30 ` [PATCH 3/5] PCI: pci-bridge-emul: Convert to GENMASK and BIT Jon Derrick
@ 2020-04-14 20:30 ` Jon Derrick
  2020-05-07 19:49   ` Rob Herring
  2020-04-14 20:30 ` [PATCH 5/5] PCI: pci-bridge-emul: Eliminate the 'reserved' member Jon Derrick
  4 siblings, 1 reply; 14+ messages in thread
From: Jon Derrick @ 2020-04-14 20:30 UTC (permalink / raw)
  To: linux-pci; +Cc: Bjorn Helgaas, Thomas Petazzoni, Russell King, Jon Derrick

Add missing bits from PCIe 4.0 and updates for PCIe 5.0 r1.0.

PCIe 4.0:
Device Status bit 6 - W1C - Emergency Power Reduction Detected
Link Control bits 15:14 - RW - DRS Signaling Control
Slot Control bit 13 - RW - Auto Slow Power Limit Disable

PCIe 5.0:
Slot Control bit 14 - RW - In-Band PD Disable

Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
---
 drivers/pci/pci-bridge-emul.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/drivers/pci/pci-bridge-emul.c b/drivers/pci/pci-bridge-emul.c
index bbcccadca85e..5c0dffa601f3 100644
--- a/drivers/pci/pci-bridge-emul.c
+++ b/drivers/pci/pci-bridge-emul.c
@@ -181,12 +181,12 @@ static const struct pci_bridge_reg_behavior pcie_cap_regs_behavior[] = {
 		.rw = GENMASK(15, 0),
 
 		/*
-		 * Device status register has 4 bits W1C, then 2 bits
-		 * RO, the rest is reserved
+		 * Device status register has bits 6 and [3:0] W1C, [5:4] RO,
+		 * the rest is reserved
 		 */
-		.w1c = GENMASK(19, 16),
-		.ro = GENMASK(21, 20),
-		.rsvd = GENMASK(31, 22),
+		.w1c = (BIT(6) | GENMASK(3, 0)) << 16,
+		.ro = GENMASK(5, 4) << 16,
+		.rsvd = GENMASK(15, 7) << 16,
 	},
 
 	[PCI_EXP_LNKCAP / 4] = {
@@ -197,15 +197,16 @@ static const struct pci_bridge_reg_behavior pcie_cap_regs_behavior[] = {
 
 	[PCI_EXP_LNKCTL / 4] = {
 		/*
-		 * Link control has bits [1:0] and [11:3] RW, the
-		 * other bits are reserved.
-		 * Link status has bits [13:0] RO, and bits [14:15]
+		 * Link control has bits [15:14], [11:3] and [1:0] RW, the
+		 * rest is reserved.
+		 *
+		 * Link status has bits [13:0] RO, and bits [15:14]
 		 * W1C.
 		 */
-		.rw = GENMASK(11, 3) | GENMASK(1, 0),
+		.rw = GENMASK(15, 14) | GENMASK(11, 3) | GENMASK(1, 0),
 		.ro = GENMASK(13, 0) << 16,
 		.w1c = GENMASK(15, 14) << 16,
-		.rsvd = GENMASK(15, 12) | BIT(2),
+		.rsvd = GENMASK(13, 12) | BIT(2),
 	},
 
 	[PCI_EXP_SLTCAP / 4] = {
@@ -214,16 +215,16 @@ static const struct pci_bridge_reg_behavior pcie_cap_regs_behavior[] = {
 
 	[PCI_EXP_SLTCTL / 4] = {
 		/*
-		 * Slot control has bits [12:0] RW, the rest is
+		 * Slot control has bits [14:0] RW, the rest is
 		 * reserved.
 		 *
-		 * Slot status has a mix of W1C and RO bits, as well
-		 * as reserved bits.
+		 * Slot status has bits 8 and [4:0] W1C, bits [7:5] RO, the
+		 * rest is reserved.
 		 */
-		.rw = GENMASK(12, 0),
+		.rw = GENMASK(14, 0),
 		.w1c = (BIT(8) | GENMASK(4, 0)) << 16,
 		.ro = GENMASK(7, 5) << 16,
-		.rsvd = GENMASK(15, 13) | (GENMASK(15, 9) << 16),
+		.rsvd = BIT(15) | (GENMASK(15, 9) << 16),
 	},
 
 	[PCI_EXP_RTCTL / 4] = {
-- 
2.18.1


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

* [PATCH 5/5] PCI: pci-bridge-emul: Eliminate the 'reserved' member
  2020-04-14 20:30 [PATCH 0/5] PCI Bridge Emulation changes for v5.8 Jon Derrick
                   ` (3 preceding siblings ...)
  2020-04-14 20:30 ` [PATCH 4/5] PCI: pci-bridge-emul: Update for PCIe 5.0 r1.0 Jon Derrick
@ 2020-04-14 20:30 ` Jon Derrick
  2020-05-07 20:00   ` Rob Herring
  4 siblings, 1 reply; 14+ messages in thread
From: Jon Derrick @ 2020-04-14 20:30 UTC (permalink / raw)
  To: linux-pci; +Cc: Bjorn Helgaas, Thomas Petazzoni, Russell King, Jon Derrick

Per PCIe 5.0 r1.0, Terms and Acronyms, Page 80:

  Reserved register fields must be read only and must return 0 (all 0's
  for multi-bit fields) when read. Reserved encodings for register and
  packet fields must not be used. Any implementation dependence on a
  Reserved field value or encoding will result in an implementation that
  is not PCI Express-compliant.

This patch ensures reads will return 0 for any bit not in the Read-Only,
Read-Write, or Write-1-to-Clear bitmasks.

Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
---
 drivers/pci/pci-bridge-emul.c | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/drivers/pci/pci-bridge-emul.c b/drivers/pci/pci-bridge-emul.c
index 5c0dffa601f3..aa563c8fd81e 100644
--- a/drivers/pci/pci-bridge-emul.c
+++ b/drivers/pci/pci-bridge-emul.c
@@ -24,6 +24,17 @@
 #define PCI_CAP_PCIE_START	PCI_BRIDGE_CONF_END
 #define PCI_CAP_PCIE_END	(PCI_CAP_PCIE_START + PCI_EXP_SLTSTA2 + 2)
 
+/**
+ * struct pci_bridge_reg_behavior - register bits behaviors
+ * @ro:		Read-Only bits
+ * @rw:		Read-Write bits
+ * @w1c:	Write-1-to-Clear bits
+ *
+ * Reads and Writes will be filtered by specified behavior. All other bits not
+ * declared are assumed 'Reserved' and will return 0 on reads, per PCIe 5.0:
+ * "Reserved register fields must be read only and must return 0 (all 0's for
+ * multi-bit fields) when read".
+ */
 struct pci_bridge_reg_behavior {
 	/* Read-only bits */
 	u32 ro;
@@ -33,9 +44,6 @@ struct pci_bridge_reg_behavior {
 
 	/* Write-1-to-clear bits */
 	u32 w1c;
-
-	/* Reserved bits (hardwired to 0) */
-	u32 rsvd;
 };
 
 static const struct pci_bridge_reg_behavior pci_regs_behavior[] = {
@@ -49,7 +57,6 @@ static const struct pci_bridge_reg_behavior pci_regs_behavior[] = {
 			PCI_COMMAND_FAST_BACK) |
 		       (PCI_STATUS_CAP_LIST | PCI_STATUS_66MHZ |
 			PCI_STATUS_FAST_BACK | PCI_STATUS_DEVSEL_MASK) << 16),
-		.rsvd = GENMASK(15, 10) | ((BIT(6) | GENMASK(3, 0)) << 16),
 		.w1c = PCI_STATUS_ERROR_BITS << 16,
 	},
 	[PCI_CLASS_REVISION / 4] = { .ro = ~0 },
@@ -96,8 +103,6 @@ static const struct pci_bridge_reg_behavior pci_regs_behavior[] = {
 		       GENMASK(11, 8) | GENMASK(3, 0)),
 
 		.w1c = PCI_STATUS_ERROR_BITS << 16,
-
-		.rsvd = ((BIT(6) | GENMASK(4, 0)) << 16),
 	},
 
 	[PCI_MEMORY_BASE / 4] = {
@@ -130,12 +135,10 @@ static const struct pci_bridge_reg_behavior pci_regs_behavior[] = {
 
 	[PCI_CAPABILITY_LIST / 4] = {
 		.ro = GENMASK(7, 0),
-		.rsvd = GENMASK(31, 8),
 	},
 
 	[PCI_ROM_ADDRESS1 / 4] = {
 		.rw = GENMASK(31, 11) | BIT(0),
-		.rsvd = GENMASK(10, 1),
 	},
 
 	/*
@@ -158,8 +161,6 @@ static const struct pci_bridge_reg_behavior pci_regs_behavior[] = {
 		.ro = (GENMASK(15, 8) | ((PCI_BRIDGE_CTL_FAST_BACK) << 16)),
 
 		.w1c = BIT(10) << 16,
-
-		.rsvd = (GENMASK(15, 12) | BIT(4)) << 16,
 	},
 };
 
@@ -186,13 +187,11 @@ static const struct pci_bridge_reg_behavior pcie_cap_regs_behavior[] = {
 		 */
 		.w1c = (BIT(6) | GENMASK(3, 0)) << 16,
 		.ro = GENMASK(5, 4) << 16,
-		.rsvd = GENMASK(15, 7) << 16,
 	},
 
 	[PCI_EXP_LNKCAP / 4] = {
 		/* All bits are RO, except bit 23 which is reserved */
 		.ro = lower_32_bits(~BIT(23)),
-		.rsvd = BIT(23),
 	},
 
 	[PCI_EXP_LNKCTL / 4] = {
@@ -206,7 +205,6 @@ static const struct pci_bridge_reg_behavior pcie_cap_regs_behavior[] = {
 		.rw = GENMASK(15, 14) | GENMASK(11, 3) | GENMASK(1, 0),
 		.ro = GENMASK(13, 0) << 16,
 		.w1c = GENMASK(15, 14) << 16,
-		.rsvd = GENMASK(13, 12) | BIT(2),
 	},
 
 	[PCI_EXP_SLTCAP / 4] = {
@@ -224,7 +222,6 @@ static const struct pci_bridge_reg_behavior pcie_cap_regs_behavior[] = {
 		.rw = GENMASK(14, 0),
 		.w1c = (BIT(8) | GENMASK(4, 0)) << 16,
 		.ro = GENMASK(7, 5) << 16,
-		.rsvd = BIT(15) | (GENMASK(15, 9) << 16),
 	},
 
 	[PCI_EXP_RTCTL / 4] = {
@@ -236,7 +233,6 @@ static const struct pci_bridge_reg_behavior pcie_cap_regs_behavior[] = {
 		 */
 		.rw = GENMASK(4, 0),
 		.ro = BIT(0) << 16,
-		.rsvd = GENMASK(15, 5) | (GENMASK(15, 1) << 16),
 	},
 
 	[PCI_EXP_RTSTA / 4] = {
@@ -246,7 +242,6 @@ static const struct pci_bridge_reg_behavior pcie_cap_regs_behavior[] = {
 		 */
 		.ro = BIT(17) | GENMASK(15, 0),
 		.w1c = BIT(16),
-		.rsvd = GENMASK(31, 18),
 	},
 };
 
@@ -354,7 +349,8 @@ int pci_bridge_emul_conf_read(struct pci_bridge_emul *bridge, int where,
 	 * Make sure we never return any reserved bit with a value
 	 * different from 0.
 	 */
-	*value &= ~behavior[reg / 4].rsvd;
+	*value &= behavior[reg / 4].ro | behavior[reg / 4].rw |
+		  behavior[reg / 4].w1c;
 
 	if (size == 1)
 		*value = (*value >> (8 * (where & 3))) & 0xff;
-- 
2.18.1


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

* Re: [PATCH 3/5] PCI: pci-bridge-emul: Convert to GENMASK and BIT
  2020-04-14 20:30 ` [PATCH 3/5] PCI: pci-bridge-emul: Convert to GENMASK and BIT Jon Derrick
@ 2020-04-16  7:30   ` Christoph Hellwig
  2020-04-16 14:35     ` Derrick, Jonathan
  0 siblings, 1 reply; 14+ messages in thread
From: Christoph Hellwig @ 2020-04-16  7:30 UTC (permalink / raw)
  To: Jon Derrick; +Cc: linux-pci, Bjorn Helgaas, Thomas Petazzoni, Russell King

On Tue, Apr 14, 2020 at 04:30:03PM -0400, Jon Derrick wrote:
> In order to make pci-bridge-emul easier to keep up-to-date with new PCIe
> features, convert all named register bits to GENMASK and BIT pairs. This
> patch doesn't alter any of the PCI configuration space as these bits are
> fully defined.
> 
> Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
> ---
>  drivers/pci/pci-bridge-emul.c | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/pci/pci-bridge-emul.c b/drivers/pci/pci-bridge-emul.c
> index c00c30ffb198..bbcccadca85e 100644
> --- a/drivers/pci/pci-bridge-emul.c
> +++ b/drivers/pci/pci-bridge-emul.c
> @@ -221,11 +221,8 @@ static const struct pci_bridge_reg_behavior pcie_cap_regs_behavior[] = {
>  		 * as reserved bits.
>  		 */
>  		.rw = GENMASK(12, 0),
> -		.w1c = (PCI_EXP_SLTSTA_ABP | PCI_EXP_SLTSTA_PFD |
> -			PCI_EXP_SLTSTA_MRLSC | PCI_EXP_SLTSTA_PDC |
> -			PCI_EXP_SLTSTA_CC | PCI_EXP_SLTSTA_DLLSC) << 16,
> -		.ro = (PCI_EXP_SLTSTA_MRLSS | PCI_EXP_SLTSTA_PDS |
> -		       PCI_EXP_SLTSTA_EIS) << 16,
> +		.w1c = (BIT(8) | GENMASK(4, 0)) << 16,
> +		.ro = GENMASK(7, 5) << 16,

FYI, I find the previous version a lot more readable.  Or rather I find
it readable while the new one looks like intentionally obsfucated
garbage to me.

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

* Re: [PATCH 3/5] PCI: pci-bridge-emul: Convert to GENMASK and BIT
  2020-04-16  7:30   ` Christoph Hellwig
@ 2020-04-16 14:35     ` Derrick, Jonathan
  2020-05-11 10:06       ` Lorenzo Pieralisi
  0 siblings, 1 reply; 14+ messages in thread
From: Derrick, Jonathan @ 2020-04-16 14:35 UTC (permalink / raw)
  To: hch; +Cc: linux-pci, helgaas, thomas.petazzoni, rmk+kernel

On Thu, 2020-04-16 at 00:30 -0700, Christoph Hellwig wrote:
> On Tue, Apr 14, 2020 at 04:30:03PM -0400, Jon Derrick wrote:
> > In order to make pci-bridge-emul easier to keep up-to-date with new PCIe
> > features, convert all named register bits to GENMASK and BIT pairs. This
> > patch doesn't alter any of the PCI configuration space as these bits are
> > fully defined.
> > 
> > Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
> > ---
> >  drivers/pci/pci-bridge-emul.c | 17 ++++++-----------
> >  1 file changed, 6 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/pci/pci-bridge-emul.c b/drivers/pci/pci-bridge-emul.c
> > index c00c30ffb198..bbcccadca85e 100644
> > --- a/drivers/pci/pci-bridge-emul.c
> > +++ b/drivers/pci/pci-bridge-emul.c
> > @@ -221,11 +221,8 @@ static const struct pci_bridge_reg_behavior pcie_cap_regs_behavior[] = {
> >  		 * as reserved bits.
> >  		 */
> >  		.rw = GENMASK(12, 0),
> > -		.w1c = (PCI_EXP_SLTSTA_ABP | PCI_EXP_SLTSTA_PFD |
> > -			PCI_EXP_SLTSTA_MRLSC | PCI_EXP_SLTSTA_PDC |
> > -			PCI_EXP_SLTSTA_CC | PCI_EXP_SLTSTA_DLLSC) << 16,
> > -		.ro = (PCI_EXP_SLTSTA_MRLSS | PCI_EXP_SLTSTA_PDS |
> > -		       PCI_EXP_SLTSTA_EIS) << 16,
> > +		.w1c = (BIT(8) | GENMASK(4, 0)) << 16,
> > +		.ro = GENMASK(7, 5) << 16,
> 
> FYI, I find the previous version a lot more readable.  Or rather I find
> it readable while the new one looks like intentionally obsfucated
> garbage to me.

Well I guess that's entirely subjective. But I do think if all the
existing BIT and GENMASK were converted to named registers instead, it
would be a lot easier to overlook mistakes.

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

* Re: [PATCH 1/5] PCI: pci-bridge-emul: Fix PCIe bit conflicts
  2020-04-14 20:30 ` [PATCH 1/5] PCI: pci-bridge-emul: Fix PCIe bit conflicts Jon Derrick
@ 2020-05-07 19:48   ` Rob Herring
  0 siblings, 0 replies; 14+ messages in thread
From: Rob Herring @ 2020-05-07 19:48 UTC (permalink / raw)
  To: Jon Derrick
  Cc: linux-pci, Bjorn Helgaas, Thomas Petazzoni, Russell King, Jon Derrick

On Tue, 14 Apr 2020 16:30:01 -0400, Jon Derrick wrote:
> This patch fixes two bit conflicts in the pci-bridge-emul driver:
> 
> 1. Bit 3 of Device Status (19 of Device Control) is marked as both
>    Write-1-to-Clear and Read-Only. It should be Write-1-to-Clear.
>    The Read-Only and Reserved bitmasks are shifted by 1 bit due to this
>    error.
> 
> 2. Bit 12 of Slot Control is marked as both Read-Write and Reserved.
>    It should be Read-Write.
> 
> Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
> ---
>  drivers/pci/pci-bridge-emul.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 

Acked-by: Rob Herring <robh@kernel.org>

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

* Re: [PATCH 2/5] PCI: pci-bridge-emul: Fix Root Cap/Status comment
  2020-04-14 20:30 ` [PATCH 2/5] PCI: pci-bridge-emul: Fix Root Cap/Status comment Jon Derrick
@ 2020-05-07 19:48   ` Rob Herring
  0 siblings, 0 replies; 14+ messages in thread
From: Rob Herring @ 2020-05-07 19:48 UTC (permalink / raw)
  To: Jon Derrick
  Cc: linux-pci, Bjorn Helgaas, Thomas Petazzoni, Russell King, Jon Derrick

On Tue, 14 Apr 2020 16:30:02 -0400, Jon Derrick wrote:
> The upper 16-bits of Root Control contain the Root Capabilities
> register. The code instead describes the Root Status register in the
> upper 16-bits, although it uses the correct bit definition for Root
> Capabilities, and for Root Status in the next definition.
> 
> Fix this comment and add a comment describing the Root Status register.
> 
> Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
> ---
>  drivers/pci/pci-bridge-emul.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 

Acked-by: Rob Herring <robh@kernel.org>

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

* Re: [PATCH 4/5] PCI: pci-bridge-emul: Update for PCIe 5.0 r1.0
  2020-04-14 20:30 ` [PATCH 4/5] PCI: pci-bridge-emul: Update for PCIe 5.0 r1.0 Jon Derrick
@ 2020-05-07 19:49   ` Rob Herring
  0 siblings, 0 replies; 14+ messages in thread
From: Rob Herring @ 2020-05-07 19:49 UTC (permalink / raw)
  To: Jon Derrick
  Cc: linux-pci, Bjorn Helgaas, Thomas Petazzoni, Russell King, Jon Derrick

On Tue, 14 Apr 2020 16:30:04 -0400, Jon Derrick wrote:
> Add missing bits from PCIe 4.0 and updates for PCIe 5.0 r1.0.
> 
> PCIe 4.0:
> Device Status bit 6 - W1C - Emergency Power Reduction Detected
> Link Control bits 15:14 - RW - DRS Signaling Control
> Slot Control bit 13 - RW - Auto Slow Power Limit Disable
> 
> PCIe 5.0:
> Slot Control bit 14 - RW - In-Band PD Disable
> 
> Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
> ---
>  drivers/pci/pci-bridge-emul.c | 31 ++++++++++++++++---------------
>  1 file changed, 16 insertions(+), 15 deletions(-)
> 

Acked-by: Rob Herring <robh@kernel.org>

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

* Re: [PATCH 5/5] PCI: pci-bridge-emul: Eliminate the 'reserved' member
  2020-04-14 20:30 ` [PATCH 5/5] PCI: pci-bridge-emul: Eliminate the 'reserved' member Jon Derrick
@ 2020-05-07 20:00   ` Rob Herring
  0 siblings, 0 replies; 14+ messages in thread
From: Rob Herring @ 2020-05-07 20:00 UTC (permalink / raw)
  To: Jon Derrick
  Cc: linux-pci, Bjorn Helgaas, Thomas Petazzoni, Russell King, Jon Derrick

On Tue, 14 Apr 2020 16:30:05 -0400, Jon Derrick wrote:
> Per PCIe 5.0 r1.0, Terms and Acronyms, Page 80:
> 
>   Reserved register fields must be read only and must return 0 (all 0's
>   for multi-bit fields) when read. Reserved encodings for register and
>   packet fields must not be used. Any implementation dependence on a
>   Reserved field value or encoding will result in an implementation that
>   is not PCI Express-compliant.
> 
> This patch ensures reads will return 0 for any bit not in the Read-Only,
> Read-Write, or Write-1-to-Clear bitmasks.
> 
> Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
> ---
>  drivers/pci/pci-bridge-emul.c | 30 +++++++++++++-----------------
>  1 file changed, 13 insertions(+), 17 deletions(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

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

* Re: [PATCH 3/5] PCI: pci-bridge-emul: Convert to GENMASK and BIT
  2020-04-16 14:35     ` Derrick, Jonathan
@ 2020-05-11 10:06       ` Lorenzo Pieralisi
  2020-05-11 15:11         ` Derrick, Jonathan
  0 siblings, 1 reply; 14+ messages in thread
From: Lorenzo Pieralisi @ 2020-05-11 10:06 UTC (permalink / raw)
  To: Derrick, Jonathan; +Cc: hch, linux-pci, helgaas, thomas.petazzoni, rmk+kernel

On Thu, Apr 16, 2020 at 02:35:59PM +0000, Derrick, Jonathan wrote:
> On Thu, 2020-04-16 at 00:30 -0700, Christoph Hellwig wrote:
> > On Tue, Apr 14, 2020 at 04:30:03PM -0400, Jon Derrick wrote:
> > > In order to make pci-bridge-emul easier to keep up-to-date with new PCIe
> > > features, convert all named register bits to GENMASK and BIT pairs. This
> > > patch doesn't alter any of the PCI configuration space as these bits are
> > > fully defined.
> > > 
> > > Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
> > > ---
> > >  drivers/pci/pci-bridge-emul.c | 17 ++++++-----------
> > >  1 file changed, 6 insertions(+), 11 deletions(-)
> > > 
> > > diff --git a/drivers/pci/pci-bridge-emul.c b/drivers/pci/pci-bridge-emul.c
> > > index c00c30ffb198..bbcccadca85e 100644
> > > --- a/drivers/pci/pci-bridge-emul.c
> > > +++ b/drivers/pci/pci-bridge-emul.c
> > > @@ -221,11 +221,8 @@ static const struct pci_bridge_reg_behavior pcie_cap_regs_behavior[] = {
> > >  		 * as reserved bits.
> > >  		 */
> > >  		.rw = GENMASK(12, 0),
> > > -		.w1c = (PCI_EXP_SLTSTA_ABP | PCI_EXP_SLTSTA_PFD |
> > > -			PCI_EXP_SLTSTA_MRLSC | PCI_EXP_SLTSTA_PDC |
> > > -			PCI_EXP_SLTSTA_CC | PCI_EXP_SLTSTA_DLLSC) << 16,
> > > -		.ro = (PCI_EXP_SLTSTA_MRLSS | PCI_EXP_SLTSTA_PDS |
> > > -		       PCI_EXP_SLTSTA_EIS) << 16,
> > > +		.w1c = (BIT(8) | GENMASK(4, 0)) << 16,
> > > +		.ro = GENMASK(7, 5) << 16,
> > 
> > FYI, I find the previous version a lot more readable.  Or rather I find
> > it readable while the new one looks like intentionally obsfucated
> > garbage to me.
> 
> Well I guess that's entirely subjective. But I do think if all the
> existing BIT and GENMASK were converted to named registers instead, it
> would be a lot easier to overlook mistakes.

Hi Jon,

where are we with this patch ? If we drop it I assume you will have
to rebase subsequent patches - I do think Christoph has a point here.

Thanks,
Lorenzo

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

* Re: [PATCH 3/5] PCI: pci-bridge-emul: Convert to GENMASK and BIT
  2020-05-11 10:06       ` Lorenzo Pieralisi
@ 2020-05-11 15:11         ` Derrick, Jonathan
  0 siblings, 0 replies; 14+ messages in thread
From: Derrick, Jonathan @ 2020-05-11 15:11 UTC (permalink / raw)
  To: lorenzo.pieralisi; +Cc: linux-pci, helgaas, hch, thomas.petazzoni, rmk+kernel

On Mon, 2020-05-11 at 11:06 +0100, Lorenzo Pieralisi wrote:
> On Thu, Apr 16, 2020 at 02:35:59PM +0000, Derrick, Jonathan wrote:
> > On Thu, 2020-04-16 at 00:30 -0700, Christoph Hellwig wrote:
> > > On Tue, Apr 14, 2020 at 04:30:03PM -0400, Jon Derrick wrote:
> > > > In order to make pci-bridge-emul easier to keep up-to-date with new PCIe
> > > > features, convert all named register bits to GENMASK and BIT pairs. This
> > > > patch doesn't alter any of the PCI configuration space as these bits are
> > > > fully defined.
> > > > 
> > > > Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
> > > > ---
> > > >  drivers/pci/pci-bridge-emul.c | 17 ++++++-----------
> > > >  1 file changed, 6 insertions(+), 11 deletions(-)
> > > > 
> > > > diff --git a/drivers/pci/pci-bridge-emul.c b/drivers/pci/pci-bridge-emul.c
> > > > index c00c30ffb198..bbcccadca85e 100644
> > > > --- a/drivers/pci/pci-bridge-emul.c
> > > > +++ b/drivers/pci/pci-bridge-emul.c
> > > > @@ -221,11 +221,8 @@ static const struct pci_bridge_reg_behavior pcie_cap_regs_behavior[] = {
> > > >  		 * as reserved bits.
> > > >  		 */
> > > >  		.rw = GENMASK(12, 0),
> > > > -		.w1c = (PCI_EXP_SLTSTA_ABP | PCI_EXP_SLTSTA_PFD |
> > > > -			PCI_EXP_SLTSTA_MRLSC | PCI_EXP_SLTSTA_PDC |
> > > > -			PCI_EXP_SLTSTA_CC | PCI_EXP_SLTSTA_DLLSC) << 16,
> > > > -		.ro = (PCI_EXP_SLTSTA_MRLSS | PCI_EXP_SLTSTA_PDS |
> > > > -		       PCI_EXP_SLTSTA_EIS) << 16,
> > > > +		.w1c = (BIT(8) | GENMASK(4, 0)) << 16,
> > > > +		.ro = GENMASK(7, 5) << 16,
> > > 
> > > FYI, I find the previous version a lot more readable.  Or rather I find
> > > it readable while the new one looks like intentionally obsfucated
> > > garbage to me.
> > 
> > Well I guess that's entirely subjective. But I do think if all the
> > existing BIT and GENMASK were converted to named registers instead, it
> > would be a lot easier to overlook mistakes.
> 
> Hi Jon,
> 
> where are we with this patch ? If we drop it I assume you will have
> to rebase subsequent patches - I do think Christoph has a point here.
> 
> Thanks,
> Lorenzo

I'll post v2 without 3 in a bit

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

end of thread, other threads:[~2020-05-11 15:12 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-14 20:30 [PATCH 0/5] PCI Bridge Emulation changes for v5.8 Jon Derrick
2020-04-14 20:30 ` [PATCH 1/5] PCI: pci-bridge-emul: Fix PCIe bit conflicts Jon Derrick
2020-05-07 19:48   ` Rob Herring
2020-04-14 20:30 ` [PATCH 2/5] PCI: pci-bridge-emul: Fix Root Cap/Status comment Jon Derrick
2020-05-07 19:48   ` Rob Herring
2020-04-14 20:30 ` [PATCH 3/5] PCI: pci-bridge-emul: Convert to GENMASK and BIT Jon Derrick
2020-04-16  7:30   ` Christoph Hellwig
2020-04-16 14:35     ` Derrick, Jonathan
2020-05-11 10:06       ` Lorenzo Pieralisi
2020-05-11 15:11         ` Derrick, Jonathan
2020-04-14 20:30 ` [PATCH 4/5] PCI: pci-bridge-emul: Update for PCIe 5.0 r1.0 Jon Derrick
2020-05-07 19:49   ` Rob Herring
2020-04-14 20:30 ` [PATCH 5/5] PCI: pci-bridge-emul: Eliminate the 'reserved' member Jon Derrick
2020-05-07 20:00   ` Rob Herring

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