* [PATCH 1/8] PCI/DPC: Rename interrupt_event_handler() to dpc_work()
2018-01-26 22:55 [PATCH 0/8] PCI/DPC: Simplify RP PIO logging Bjorn Helgaas
@ 2018-01-26 22:55 ` Bjorn Helgaas
2018-01-26 22:55 ` [PATCH 2/8] PCI/DPC: Add local variable for DPC capability offset Bjorn Helgaas
` (7 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Bjorn Helgaas @ 2018-01-26 22:55 UTC (permalink / raw)
To: Keith Busch; +Cc: linux-pci, Dongdong Liu
From: Bjorn Helgaas <bhelgaas@google.com>
Rename interrupt_event_handler() to dpc_work() so there's more useful
information in stack traces and similar situations. No functional change
intended.
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/pci/pcie/pcie-dpc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/pcie/pcie-dpc.c b/drivers/pci/pcie/pcie-dpc.c
index 0515abd20a48..48e1e3f8dd69 100644
--- a/drivers/pci/pcie/pcie-dpc.c
+++ b/drivers/pci/pcie/pcie-dpc.c
@@ -105,7 +105,7 @@ static void dpc_wait_link_inactive(struct dpc_dev *dpc)
dev_warn(dev, "Link state not disabled for DPC event\n");
}
-static void interrupt_event_handler(struct work_struct *work)
+static void dpc_work(struct work_struct *work)
{
struct dpc_dev *dpc = container_of(work, struct dpc_dev, work);
struct pci_dev *dev, *temp, *pdev = dpc->dev->port;
@@ -319,7 +319,7 @@ static int dpc_probe(struct pcie_device *dev)
dpc->cap_pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_DPC);
dpc->dev = dev;
- INIT_WORK(&dpc->work, interrupt_event_handler);
+ INIT_WORK(&dpc->work, dpc_work);
set_service_data(dev, dpc);
status = devm_request_irq(device, dev->irq, dpc_irq, IRQF_SHARED,
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 2/8] PCI/DPC: Add local variable for DPC capability offset
2018-01-26 22:55 [PATCH 0/8] PCI/DPC: Simplify RP PIO logging Bjorn Helgaas
2018-01-26 22:55 ` [PATCH 1/8] PCI/DPC: Rename interrupt_event_handler() to dpc_work() Bjorn Helgaas
@ 2018-01-26 22:55 ` Bjorn Helgaas
2018-01-26 22:55 ` [PATCH 3/8] PCI/DPC: Rename struct dpc_dev.rp to rp_extensions Bjorn Helgaas
` (6 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Bjorn Helgaas @ 2018-01-26 22:55 UTC (permalink / raw)
To: Keith Busch; +Cc: linux-pci, Dongdong Liu
From: Bjorn Helgaas <bhelgaas@google.com>
Add a local variable for DPC capability offset and replace repeated use of
"dpc->cap_pos" with simply "cap". No functional change intended.
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/pci/pcie/pcie-dpc.c | 65 +++++++++++++++++++------------------------
1 file changed, 29 insertions(+), 36 deletions(-)
diff --git a/drivers/pci/pcie/pcie-dpc.c b/drivers/pci/pcie/pcie-dpc.c
index 48e1e3f8dd69..52702f7b911a 100644
--- a/drivers/pci/pcie/pcie-dpc.c
+++ b/drivers/pci/pcie/pcie-dpc.c
@@ -41,7 +41,7 @@ struct dpc_rp_pio_regs {
struct dpc_dev {
struct pcie_device *dev;
struct work_struct work;
- int cap_pos;
+ u16 cap_pos;
bool rp;
u32 rp_pio_status;
};
@@ -73,13 +73,13 @@ static int dpc_wait_rp_inactive(struct dpc_dev *dpc)
unsigned long timeout = jiffies + HZ;
struct pci_dev *pdev = dpc->dev->port;
struct device *dev = &dpc->dev->device;
- u16 status;
+ u16 cap = dpc->cap_pos, status;
- pci_read_config_word(pdev, dpc->cap_pos + PCI_EXP_DPC_STATUS, &status);
+ pci_read_config_word(pdev, cap + PCI_EXP_DPC_STATUS, &status);
while (status & PCI_EXP_DPC_RP_BUSY &&
!time_after(jiffies, timeout)) {
msleep(10);
- pci_read_config_word(pdev, dpc->cap_pos + PCI_EXP_DPC_STATUS, &status);
+ pci_read_config_word(pdev, cap + PCI_EXP_DPC_STATUS, &status);
}
if (status & PCI_EXP_DPC_RP_BUSY) {
dev_warn(dev, "DPC root port still busy\n");
@@ -110,7 +110,7 @@ static void dpc_work(struct work_struct *work)
struct dpc_dev *dpc = container_of(work, struct dpc_dev, work);
struct pci_dev *dev, *temp, *pdev = dpc->dev->port;
struct pci_bus *parent = pdev->subordinate;
- u16 ctl;
+ u16 cap = dpc->cap_pos, ctl;
pci_lock_rescan_remove();
list_for_each_entry_safe_reverse(dev, temp, &parent->devices,
@@ -129,17 +129,16 @@ static void dpc_work(struct work_struct *work)
if (dpc->rp && dpc_wait_rp_inactive(dpc))
return;
if (dpc->rp && dpc->rp_pio_status) {
- pci_write_config_dword(pdev,
- dpc->cap_pos + PCI_EXP_DPC_RP_PIO_STATUS,
- dpc->rp_pio_status);
+ pci_write_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_STATUS,
+ dpc->rp_pio_status);
dpc->rp_pio_status = 0;
}
- pci_write_config_word(pdev, dpc->cap_pos + PCI_EXP_DPC_STATUS,
+ pci_write_config_word(pdev, cap + PCI_EXP_DPC_STATUS,
PCI_EXP_DPC_STATUS_TRIGGER | PCI_EXP_DPC_STATUS_INTERRUPT);
- pci_read_config_word(pdev, dpc->cap_pos + PCI_EXP_DPC_CTL, &ctl);
- pci_write_config_word(pdev, dpc->cap_pos + PCI_EXP_DPC_CTL,
+ pci_read_config_word(pdev, cap + PCI_EXP_DPC_CTL, &ctl);
+ pci_write_config_word(pdev, cap + PCI_EXP_DPC_CTL,
ctl | PCI_EXP_DPC_CTL_INT_EN);
}
@@ -189,23 +188,22 @@ static void dpc_rp_pio_get_info(struct dpc_dev *dpc,
struct pci_dev *pdev = dpc->dev->port;
struct device *dev = &dpc->dev->device;
int i;
- u16 cap;
- u16 status;
+ u16 cap = dpc->cap_pos, status;
- pci_read_config_dword(pdev, dpc->cap_pos + PCI_EXP_DPC_RP_PIO_STATUS,
+ pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_STATUS,
&rp_pio->status);
- pci_read_config_dword(pdev, dpc->cap_pos + PCI_EXP_DPC_RP_PIO_MASK,
+ pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_MASK,
&rp_pio->mask);
- pci_read_config_dword(pdev, dpc->cap_pos + PCI_EXP_DPC_RP_PIO_SEVERITY,
+ pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_SEVERITY,
&rp_pio->severity);
- pci_read_config_dword(pdev, dpc->cap_pos + PCI_EXP_DPC_RP_PIO_SYSERROR,
+ pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_SYSERROR,
&rp_pio->syserror);
- pci_read_config_dword(pdev, dpc->cap_pos + PCI_EXP_DPC_RP_PIO_EXCEPTION,
+ pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_EXCEPTION,
&rp_pio->exception);
/* Get First Error Pointer */
- pci_read_config_word(pdev, dpc->cap_pos + PCI_EXP_DPC_STATUS, &status);
+ pci_read_config_word(pdev, cap + PCI_EXP_DPC_STATUS, &status);
rp_pio->first_error = (status & 0x1f00) >> 8;
pci_read_config_word(pdev, dpc->cap_pos + PCI_EXP_DPC_CAP, &cap);
@@ -216,27 +214,22 @@ static void dpc_rp_pio_get_info(struct dpc_dev *dpc,
return;
}
- pci_read_config_dword(pdev,
- dpc->cap_pos + PCI_EXP_DPC_RP_PIO_HEADER_LOG,
+ pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_HEADER_LOG,
&rp_pio->header_log.dw0);
- pci_read_config_dword(pdev,
- dpc->cap_pos + PCI_EXP_DPC_RP_PIO_HEADER_LOG + 4,
+ pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_HEADER_LOG + 4,
&rp_pio->header_log.dw1);
- pci_read_config_dword(pdev,
- dpc->cap_pos + PCI_EXP_DPC_RP_PIO_HEADER_LOG + 8,
+ pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_HEADER_LOG + 8,
&rp_pio->header_log.dw2);
- pci_read_config_dword(pdev,
- dpc->cap_pos + PCI_EXP_DPC_RP_PIO_HEADER_LOG + 12,
+ pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_HEADER_LOG + 12,
&rp_pio->header_log.dw3);
if (rp_pio->log_size == 4)
return;
- pci_read_config_dword(pdev,
- dpc->cap_pos + PCI_EXP_DPC_RP_PIO_IMPSPEC_LOG,
+ pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_IMPSPEC_LOG,
&rp_pio->impspec_log);
for (i = 0; i < rp_pio->log_size - 5; i++)
pci_read_config_dword(pdev,
- dpc->cap_pos + PCI_EXP_DPC_RP_PIO_TLPPREFIX_LOG,
+ cap + PCI_EXP_DPC_RP_PIO_TLPPREFIX_LOG,
&rp_pio->tlp_prefix_log[i]);
}
@@ -255,28 +248,28 @@ static irqreturn_t dpc_irq(int irq, void *context)
struct dpc_dev *dpc = (struct dpc_dev *)context;
struct pci_dev *pdev = dpc->dev->port;
struct device *dev = &dpc->dev->device;
- u16 ctl, status, source, reason, ext_reason;
+ u16 cap = dpc->cap_pos, ctl, status, source, reason, ext_reason;
- pci_read_config_word(pdev, dpc->cap_pos + PCI_EXP_DPC_CTL, &ctl);
+ pci_read_config_word(pdev, cap + PCI_EXP_DPC_CTL, &ctl);
if (!(ctl & PCI_EXP_DPC_CTL_INT_EN) || ctl == (u16)(~0))
return IRQ_NONE;
- pci_read_config_word(pdev, dpc->cap_pos + PCI_EXP_DPC_STATUS, &status);
+ pci_read_config_word(pdev, cap + PCI_EXP_DPC_STATUS, &status);
if (!(status & PCI_EXP_DPC_STATUS_INTERRUPT))
return IRQ_NONE;
if (!(status & PCI_EXP_DPC_STATUS_TRIGGER)) {
- pci_write_config_word(pdev, dpc->cap_pos + PCI_EXP_DPC_STATUS,
+ pci_write_config_word(pdev, cap + PCI_EXP_DPC_STATUS,
PCI_EXP_DPC_STATUS_INTERRUPT);
return IRQ_HANDLED;
}
- pci_write_config_word(pdev, dpc->cap_pos + PCI_EXP_DPC_CTL,
+ pci_write_config_word(pdev, cap + PCI_EXP_DPC_CTL,
ctl & ~PCI_EXP_DPC_CTL_INT_EN);
- pci_read_config_word(pdev, dpc->cap_pos + PCI_EXP_DPC_SOURCE_ID,
+ pci_read_config_word(pdev, cap + PCI_EXP_DPC_SOURCE_ID,
&source);
dev_info(dev, "DPC containment event, status:%#06x source:%#06x\n",
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 3/8] PCI/DPC: Rename struct dpc_dev.rp to rp_extensions
2018-01-26 22:55 [PATCH 0/8] PCI/DPC: Simplify RP PIO logging Bjorn Helgaas
2018-01-26 22:55 ` [PATCH 1/8] PCI/DPC: Rename interrupt_event_handler() to dpc_work() Bjorn Helgaas
2018-01-26 22:55 ` [PATCH 2/8] PCI/DPC: Add local variable for DPC capability offset Bjorn Helgaas
@ 2018-01-26 22:55 ` Bjorn Helgaas
2018-01-26 22:56 ` [PATCH 4/8] PCI/DPC: Read RP PIO Log Size once at probe Bjorn Helgaas
` (5 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Bjorn Helgaas @ 2018-01-26 22:55 UTC (permalink / raw)
To: Keith Busch; +Cc: linux-pci, Dongdong Liu
From: Bjorn Helgaas <bhelgaas@google.com>
"rp" is ambiguous: it might mean "this DPC device is a Root Port." But in
fact, it means "this DPC device is a Root Port *and* it supports a set of
DPC Extensions."
Rename "rp" to "rp_extensions" to make this more clear. No functional
change intended.
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/pci/pcie/pcie-dpc.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/pcie/pcie-dpc.c b/drivers/pci/pcie/pcie-dpc.c
index 52702f7b911a..0bbdf513067a 100644
--- a/drivers/pci/pcie/pcie-dpc.c
+++ b/drivers/pci/pcie/pcie-dpc.c
@@ -42,7 +42,7 @@ struct dpc_dev {
struct pcie_device *dev;
struct work_struct work;
u16 cap_pos;
- bool rp;
+ bool rp_extensions;
u32 rp_pio_status;
};
@@ -126,9 +126,9 @@ static void dpc_work(struct work_struct *work)
pci_unlock_rescan_remove();
dpc_wait_link_inactive(dpc);
- if (dpc->rp && dpc_wait_rp_inactive(dpc))
+ if (dpc->rp_extensions && dpc_wait_rp_inactive(dpc))
return;
- if (dpc->rp && dpc->rp_pio_status) {
+ if (dpc->rp_extensions && dpc->rp_pio_status) {
pci_write_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_STATUS,
dpc->rp_pio_status);
dpc->rp_pio_status = 0;
@@ -326,7 +326,7 @@ static int dpc_probe(struct pcie_device *dev)
pci_read_config_word(pdev, dpc->cap_pos + PCI_EXP_DPC_CAP, &cap);
pci_read_config_word(pdev, dpc->cap_pos + PCI_EXP_DPC_CTL, &ctl);
- dpc->rp = (cap & PCI_EXP_DPC_CAP_RP_EXT);
+ dpc->rp_extensions = (cap & PCI_EXP_DPC_CAP_RP_EXT);
ctl = (ctl & 0xfff4) | PCI_EXP_DPC_CTL_EN_NONFATAL | PCI_EXP_DPC_CTL_INT_EN;
pci_write_config_word(pdev, dpc->cap_pos + PCI_EXP_DPC_CTL, ctl);
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 4/8] PCI/DPC: Read RP PIO Log Size once at probe
2018-01-26 22:55 [PATCH 0/8] PCI/DPC: Simplify RP PIO logging Bjorn Helgaas
` (2 preceding siblings ...)
2018-01-26 22:55 ` [PATCH 3/8] PCI/DPC: Rename struct dpc_dev.rp to rp_extensions Bjorn Helgaas
@ 2018-01-26 22:56 ` Bjorn Helgaas
2018-01-26 22:56 ` [PATCH 5/8] PCI/DPC: Process RP PIO details only if RP PIO extensions supported Bjorn Helgaas
` (4 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Bjorn Helgaas @ 2018-01-26 22:56 UTC (permalink / raw)
To: Keith Busch; +Cc: linux-pci, Dongdong Liu
From: Bjorn Helgaas <bhelgaas@google.com>
The RP PIO Log Size is a read-only field in the DPC Capability, so it is
constant and known at probe-time, but previously we read it every time we
processed an RP PIO error.
Read it once in dpc_probe() (if the RP Extensions for DPC are supported)
and remember the size in struct dpc_dev. No functional change intended.
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/pci/pcie/pcie-dpc.c | 29 ++++++++++++++++-------------
1 file changed, 16 insertions(+), 13 deletions(-)
diff --git a/drivers/pci/pcie/pcie-dpc.c b/drivers/pci/pcie/pcie-dpc.c
index 0bbdf513067a..17f60bd4ea5f 100644
--- a/drivers/pci/pcie/pcie-dpc.c
+++ b/drivers/pci/pcie/pcie-dpc.c
@@ -34,7 +34,6 @@ struct dpc_rp_pio_regs {
struct rp_pio_header_log_regs header_log;
u32 impspec_log;
u32 tlp_prefix_log[4];
- u32 log_size;
u16 first_error;
};
@@ -44,6 +43,7 @@ struct dpc_dev {
u16 cap_pos;
bool rp_extensions;
u32 rp_pio_status;
+ u8 rp_log_size;
};
static const char * const rp_pio_error_string[] = {
@@ -173,11 +173,12 @@ static void dpc_rp_pio_print_error(struct dpc_dev *dpc,
}
dpc_rp_pio_print_tlp_header(dev, &rp_pio->header_log);
- if (rp_pio->log_size == 4)
+ if (dpc->rp_log_size == 4)
return;
+
dev_err(dev, "RP PIO ImpSpec Log %#010x\n", rp_pio->impspec_log);
- for (i = 0; i < rp_pio->log_size - 5; i++)
+ for (i = 0; i < dpc->rp_log_size - 5; i++)
dev_err(dev, "TLP Prefix Header: dw%d, %#010x\n", i,
rp_pio->tlp_prefix_log[i]);
}
@@ -186,7 +187,6 @@ static void dpc_rp_pio_get_info(struct dpc_dev *dpc,
struct dpc_rp_pio_regs *rp_pio)
{
struct pci_dev *pdev = dpc->dev->port;
- struct device *dev = &dpc->dev->device;
int i;
u16 cap = dpc->cap_pos, status;
@@ -206,13 +206,8 @@ static void dpc_rp_pio_get_info(struct dpc_dev *dpc,
pci_read_config_word(pdev, cap + PCI_EXP_DPC_STATUS, &status);
rp_pio->first_error = (status & 0x1f00) >> 8;
- pci_read_config_word(pdev, dpc->cap_pos + PCI_EXP_DPC_CAP, &cap);
- rp_pio->log_size = (cap & PCI_EXP_DPC_RP_PIO_LOG_SIZE) >> 8;
- if (rp_pio->log_size < 4 || rp_pio->log_size > 9) {
- dev_err(dev, "RP PIO log size %u is invalid\n",
- rp_pio->log_size);
+ if (dpc->rp_log_size < 4)
return;
- }
pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_HEADER_LOG,
&rp_pio->header_log.dw0);
@@ -222,12 +217,12 @@ static void dpc_rp_pio_get_info(struct dpc_dev *dpc,
&rp_pio->header_log.dw2);
pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_HEADER_LOG + 12,
&rp_pio->header_log.dw3);
- if (rp_pio->log_size == 4)
+ if (dpc->rp_log_size == 4)
return;
pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_IMPSPEC_LOG,
&rp_pio->impspec_log);
- for (i = 0; i < rp_pio->log_size - 5; i++)
+ for (i = 0; i < dpc->rp_log_size - 5; i++)
pci_read_config_dword(pdev,
cap + PCI_EXP_DPC_RP_PIO_TLPPREFIX_LOG,
&rp_pio->tlp_prefix_log[i]);
@@ -327,6 +322,14 @@ static int dpc_probe(struct pcie_device *dev)
pci_read_config_word(pdev, dpc->cap_pos + PCI_EXP_DPC_CTL, &ctl);
dpc->rp_extensions = (cap & PCI_EXP_DPC_CAP_RP_EXT);
+ if (dpc->rp_extensions) {
+ dpc->rp_log_size = (cap & PCI_EXP_DPC_RP_PIO_LOG_SIZE) >> 8;
+ if (dpc->rp_log_size < 4 || dpc->rp_log_size > 9) {
+ dev_err(device, "RP PIO log size %u is invalid\n",
+ dpc->rp_log_size);
+ dpc->rp_log_size = 0;
+ }
+ }
ctl = (ctl & 0xfff4) | PCI_EXP_DPC_CTL_EN_NONFATAL | PCI_EXP_DPC_CTL_INT_EN;
pci_write_config_word(pdev, dpc->cap_pos + PCI_EXP_DPC_CTL, ctl);
@@ -334,7 +337,7 @@ static int dpc_probe(struct pcie_device *dev)
dev_info(device, "DPC error containment capabilities: Int Msg #%d, RPExt%c PoisonedTLP%c SwTrigger%c RP PIO Log %d, DL_ActiveErr%c\n",
cap & PCI_EXP_DPC_IRQ, FLAG(cap, PCI_EXP_DPC_CAP_RP_EXT),
FLAG(cap, PCI_EXP_DPC_CAP_POISONED_TLP),
- FLAG(cap, PCI_EXP_DPC_CAP_SW_TRIGGER), (cap >> 8) & 0xf,
+ FLAG(cap, PCI_EXP_DPC_CAP_SW_TRIGGER), dpc->rp_log_size,
FLAG(cap, PCI_EXP_DPC_CAP_DL_ACTIVE));
return status;
}
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 5/8] PCI/DPC: Process RP PIO details only if RP PIO extensions supported
2018-01-26 22:55 [PATCH 0/8] PCI/DPC: Simplify RP PIO logging Bjorn Helgaas
` (3 preceding siblings ...)
2018-01-26 22:56 ` [PATCH 4/8] PCI/DPC: Read RP PIO Log Size once at probe Bjorn Helgaas
@ 2018-01-26 22:56 ` Bjorn Helgaas
2018-01-26 22:56 ` [PATCH 6/8] PCI/DPC: Consolidate RP PIO get/print functions Bjorn Helgaas
` (3 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Bjorn Helgaas @ 2018-01-26 22:56 UTC (permalink / raw)
To: Keith Busch; +Cc: linux-pci, Dongdong Liu
From: Bjorn Helgaas <bhelgaas@google.com>
The RP PIO registers (status, mask, severity, etc) are only implemented if
the "RP Extensions for DPC" bit is set in the DPC Capabilities register.
Previously we called dpc_process_rp_pio_error(), which reads and decodes
those RP PIO registers, whenever the DPC Status register indicated an "RP
PIO error" (Trigger Reason == 3 and Trigger Reason Extension == 0).
It does seem reasonable to assume that DPC Status would only indicate an RP
PIO error if the RP extensions are supported, but PCIe r4.0, sec 7.9.15.4,
is actually not explicit about that: it does not say "Trigger Reason
Extension == 0 is valid only for Root Ports that support RP Extensions for
DPC."
Check whether the RP Extensions for DPC are supported before trying to read
the RP PIO registers.
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/pci/pcie/pcie-dpc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/pcie/pcie-dpc.c b/drivers/pci/pcie/pcie-dpc.c
index 17f60bd4ea5f..a6b8d1496322 100644
--- a/drivers/pci/pcie/pcie-dpc.c
+++ b/drivers/pci/pcie/pcie-dpc.c
@@ -281,7 +281,7 @@ static irqreturn_t dpc_irq(int irq, void *context)
(ext_reason == 1) ? "software trigger" :
"reserved error");
/* show RP PIO error detail information */
- if (reason == 3 && ext_reason == 0)
+ if (dpc->rp_extensions && reason == 3 && ext_reason == 0)
dpc_process_rp_pio_error(dpc);
schedule_work(&dpc->work);
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 6/8] PCI/DPC: Consolidate RP PIO get/print functions
2018-01-26 22:55 [PATCH 0/8] PCI/DPC: Simplify RP PIO logging Bjorn Helgaas
` (4 preceding siblings ...)
2018-01-26 22:56 ` [PATCH 5/8] PCI/DPC: Process RP PIO details only if RP PIO extensions supported Bjorn Helgaas
@ 2018-01-26 22:56 ` Bjorn Helgaas
2018-01-30 0:42 ` okaya
2018-01-26 22:56 ` [PATCH 7/8] PCI/DPC: Add and use DPC Status register field definitions Bjorn Helgaas
` (2 subsequent siblings)
8 siblings, 1 reply; 12+ messages in thread
From: Bjorn Helgaas @ 2018-01-26 22:56 UTC (permalink / raw)
To: Keith Busch; +Cc: linux-pci, Dongdong Liu
From: Bjorn Helgaas <bhelgaas@google.com>
Previously we defined a structure for RP PIO log information, allocated it
on the stack, called one function to fill it in from DPC registers, and
called another to print it out.
Simplify this by dropping the structure and printing the error information
as soon as we read it from the DPC capability. This way we don't need a
structure, there's one function instead of four, and everything we do with
the register information is in one place instead of being split between
functions.
No functional change intended.
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/pci/pcie/pcie-dpc.c | 132 +++++++++++++------------------------------
1 file changed, 39 insertions(+), 93 deletions(-)
diff --git a/drivers/pci/pcie/pcie-dpc.c b/drivers/pci/pcie/pcie-dpc.c
index a6b8d1496322..06d3af112580 100644
--- a/drivers/pci/pcie/pcie-dpc.c
+++ b/drivers/pci/pcie/pcie-dpc.c
@@ -17,26 +17,6 @@
#include "../pci.h"
#include "aer/aerdrv.h"
-struct rp_pio_header_log_regs {
- u32 dw0;
- u32 dw1;
- u32 dw2;
- u32 dw3;
-};
-
-struct dpc_rp_pio_regs {
- u32 status;
- u32 mask;
- u32 severity;
- u32 syserror;
- u32 exception;
-
- struct rp_pio_header_log_regs header_log;
- u32 impspec_log;
- u32 tlp_prefix_log[4];
- u16 first_error;
-};
-
struct dpc_dev {
struct pcie_device *dev;
struct work_struct work;
@@ -142,100 +122,66 @@ static void dpc_work(struct work_struct *work)
ctl | PCI_EXP_DPC_CTL_INT_EN);
}
-static void dpc_rp_pio_print_tlp_header(struct device *dev,
- struct rp_pio_header_log_regs *t)
-{
- dev_err(dev, "TLP Header: %#010x %#010x %#010x %#010x\n",
- t->dw0, t->dw1, t->dw2, t->dw3);
-}
-
-static void dpc_rp_pio_print_error(struct dpc_dev *dpc,
- struct dpc_rp_pio_regs *rp_pio)
+static void dpc_process_rp_pio_error(struct dpc_dev *dpc)
{
struct device *dev = &dpc->dev->device;
+ struct pci_dev *pdev = dpc->dev->port;
+ u16 cap = dpc->cap_pos;
+ u32 status, mask;
+ u32 sev, syserr, exc;
+ u16 dpc_status, first_error;
int i;
- u32 status;
+ u32 dw0, dw1, dw2, dw3;
+ u32 log;
+ u32 prefix;
+ pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_STATUS, &status);
+ pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_MASK, &mask);
dev_err(dev, "rp_pio_status: %#010x, rp_pio_mask: %#010x\n",
- rp_pio->status, rp_pio->mask);
+ status, mask);
+ dpc->rp_pio_status = status;
+
+ pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_SEVERITY, &sev);
+ pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_SYSERROR, &syserr);
+ pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_EXCEPTION, &exc);
dev_err(dev, "RP PIO severity=%#010x, syserror=%#010x, exception=%#010x\n",
- rp_pio->severity, rp_pio->syserror, rp_pio->exception);
+ sev, syserr, exc);
- status = (rp_pio->status & ~rp_pio->mask);
+ /* Get First Error Pointer */
+ pci_read_config_word(pdev, cap + PCI_EXP_DPC_STATUS, &dpc_status);
+ first_error = (dpc_status & 0x1f00) >> 8;
+ status &= ~mask;
for (i = 0; i < ARRAY_SIZE(rp_pio_error_string); i++) {
- if (!(status & (1 << i)))
- continue;
-
- dev_err(dev, "[%2d] %s%s\n", i, rp_pio_error_string[i],
- rp_pio->first_error == i ? " (First)" : "");
+ if (status & (1 << i))
+ dev_err(dev, "[%2d] %s%s\n", i, rp_pio_error_string[i],
+ first_error == i ? " (First)" : "");
}
- dpc_rp_pio_print_tlp_header(dev, &rp_pio->header_log);
- if (dpc->rp_log_size == 4)
- return;
-
- dev_err(dev, "RP PIO ImpSpec Log %#010x\n", rp_pio->impspec_log);
-
- for (i = 0; i < dpc->rp_log_size - 5; i++)
- dev_err(dev, "TLP Prefix Header: dw%d, %#010x\n", i,
- rp_pio->tlp_prefix_log[i]);
-}
-
-static void dpc_rp_pio_get_info(struct dpc_dev *dpc,
- struct dpc_rp_pio_regs *rp_pio)
-{
- struct pci_dev *pdev = dpc->dev->port;
- int i;
- u16 cap = dpc->cap_pos, status;
-
- pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_STATUS,
- &rp_pio->status);
- pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_MASK,
- &rp_pio->mask);
-
- pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_SEVERITY,
- &rp_pio->severity);
- pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_SYSERROR,
- &rp_pio->syserror);
- pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_EXCEPTION,
- &rp_pio->exception);
-
- /* Get First Error Pointer */
- pci_read_config_word(pdev, cap + PCI_EXP_DPC_STATUS, &status);
- rp_pio->first_error = (status & 0x1f00) >> 8;
-
if (dpc->rp_log_size < 4)
return;
-
pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_HEADER_LOG,
- &rp_pio->header_log.dw0);
+ &dw0);
pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_HEADER_LOG + 4,
- &rp_pio->header_log.dw1);
+ &dw1);
pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_HEADER_LOG + 8,
- &rp_pio->header_log.dw2);
+ &dw2);
pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_HEADER_LOG + 12,
- &rp_pio->header_log.dw3);
- if (dpc->rp_log_size == 4)
+ &dw3);
+ dev_err(dev, "TLP Header: %#010x %#010x %#010x %#010x\n",
+ dw0, dw1, dw2, dw3);
+
+ if (dpc->rp_log_size < 5)
return;
+ pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_IMPSPEC_LOG, &log);
+ dev_err(dev, "RP PIO ImpSpec Log %#010x\n", log);
- pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_IMPSPEC_LOG,
- &rp_pio->impspec_log);
- for (i = 0; i < dpc->rp_log_size - 5; i++)
+ for (i = 0; i < dpc->rp_log_size - 5; i++) {
pci_read_config_dword(pdev,
- cap + PCI_EXP_DPC_RP_PIO_TLPPREFIX_LOG,
- &rp_pio->tlp_prefix_log[i]);
-}
-
-static void dpc_process_rp_pio_error(struct dpc_dev *dpc)
-{
- struct dpc_rp_pio_regs rp_pio_regs;
-
- dpc_rp_pio_get_info(dpc, &rp_pio_regs);
- dpc_rp_pio_print_error(dpc, &rp_pio_regs);
-
- dpc->rp_pio_status = rp_pio_regs.status;
+ cap + PCI_EXP_DPC_RP_PIO_TLPPREFIX_LOG, &prefix);
+ dev_err(dev, "TLP Prefix Header: dw%d, %#010x\n", i, prefix);
+ }
}
static irqreturn_t dpc_irq(int irq, void *context)
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 6/8] PCI/DPC: Consolidate RP PIO get/print functions
2018-01-26 22:56 ` [PATCH 6/8] PCI/DPC: Consolidate RP PIO get/print functions Bjorn Helgaas
@ 2018-01-30 0:42 ` okaya
2018-01-30 18:11 ` Bjorn Helgaas
0 siblings, 1 reply; 12+ messages in thread
From: okaya @ 2018-01-30 0:42 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: Keith Busch, linux-pci, Dongdong Liu, linux-pci-owner
On 2018-01-26 17:56, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
>
> Previously we defined a structure for RP PIO log information, allocated
> it
> on the stack, called one function to fill it in from DPC registers, and
> called another to print it out.
>
> Simplify this by dropping the structure and printing the error
> information
> as soon as we read it from the DPC capability. This way we don't need
> a
> structure, there's one function instead of four, and everything we do
> with
> the register information is in one place instead of being split between
> functions.
>
> No functional change intended.
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
> drivers/pci/pcie/pcie-dpc.c | 132
> +++++++++++++------------------------------
> 1 file changed, 39 insertions(+), 93 deletions(-)
>
> diff --git a/drivers/pci/pcie/pcie-dpc.c b/drivers/pci/pcie/pcie-dpc.c
> index a6b8d1496322..06d3af112580 100644
> --- a/drivers/pci/pcie/pcie-dpc.c
> +++ b/drivers/pci/pcie/pcie-dpc.c
> @@ -17,26 +17,6 @@
> #include "../pci.h"
> #include "aer/aerdrv.h"
>
> -struct rp_pio_header_log_regs {
> - u32 dw0;
> - u32 dw1;
> - u32 dw2;
> - u32 dw3;
> -};
> -
> -struct dpc_rp_pio_regs {
> - u32 status;
> - u32 mask;
> - u32 severity;
> - u32 syserror;
> - u32 exception;
> -
> - struct rp_pio_header_log_regs header_log;
> - u32 impspec_log;
> - u32 tlp_prefix_log[4];
> - u16 first_error;
> -};
> -
> struct dpc_dev {
> struct pcie_device *dev;
> struct work_struct work;
> @@ -142,100 +122,66 @@ static void dpc_work(struct work_struct *work)
> ctl | PCI_EXP_DPC_CTL_INT_EN);
> }
>
> -static void dpc_rp_pio_print_tlp_header(struct device *dev,
> - struct rp_pio_header_log_regs *t)
> -{
> - dev_err(dev, "TLP Header: %#010x %#010x %#010x %#010x\n",
> - t->dw0, t->dw1, t->dw2, t->dw3);
> -}
> -
> -static void dpc_rp_pio_print_error(struct dpc_dev *dpc,
> - struct dpc_rp_pio_regs *rp_pio)
> +static void dpc_process_rp_pio_error(struct dpc_dev *dpc)
> {
> struct device *dev = &dpc->dev->device;
> + struct pci_dev *pdev = dpc->dev->port;
> + u16 cap = dpc->cap_pos;
> + u32 status, mask;
> + u32 sev, syserr, exc;
> + u16 dpc_status, first_error;
> int i;
> - u32 status;
> + u32 dw0, dw1, dw2, dw3;
> + u32 log;
> + u32 prefix;
>
> + pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_STATUS,
> &status);
> + pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_MASK, &mask);
> dev_err(dev, "rp_pio_status: %#010x, rp_pio_mask: %#010x\n",
> - rp_pio->status, rp_pio->mask);
> + status, mask);
>
> + dpc->rp_pio_status = status;
> +
> + pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_SEVERITY, &sev);
> + pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_SYSERROR,
> &syserr);
> + pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_EXCEPTION,
> &exc);
> dev_err(dev, "RP PIO severity=%#010x, syserror=%#010x,
> exception=%#010x\n",
> - rp_pio->severity, rp_pio->syserror, rp_pio->exception);
> + sev, syserr, exc);
>
> - status = (rp_pio->status & ~rp_pio->mask);
> + /* Get First Error Pointer */
> + pci_read_config_word(pdev, cap + PCI_EXP_DPC_STATUS, &dpc_status);
> + first_error = (dpc_status & 0x1f00) >> 8;
This didn't look like a simple change here.
>
> + status &= ~mask;
> for (i = 0; i < ARRAY_SIZE(rp_pio_error_string); i++) {
> - if (!(status & (1 << i)))
> - continue;
> -
> - dev_err(dev, "[%2d] %s%s\n", i, rp_pio_error_string[i],
> - rp_pio->first_error == i ? " (First)" : "");
> + if (status & (1 << i))
> + dev_err(dev, "[%2d] %s%s\n", i, rp_pio_error_string[i],
> + first_error == i ? " (First)" : "");
> }
>
> - dpc_rp_pio_print_tlp_header(dev, &rp_pio->header_log);
> - if (dpc->rp_log_size == 4)
> - return;
> -
> - dev_err(dev, "RP PIO ImpSpec Log %#010x\n", rp_pio->impspec_log);
> -
> - for (i = 0; i < dpc->rp_log_size - 5; i++)
> - dev_err(dev, "TLP Prefix Header: dw%d, %#010x\n", i,
> - rp_pio->tlp_prefix_log[i]);
> -}
> -
> -static void dpc_rp_pio_get_info(struct dpc_dev *dpc,
> - struct dpc_rp_pio_regs *rp_pio)
> -{
> - struct pci_dev *pdev = dpc->dev->port;
> - int i;
> - u16 cap = dpc->cap_pos, status;
> -
> - pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_STATUS,
> - &rp_pio->status);
> - pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_MASK,
> - &rp_pio->mask);
> -
> - pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_SEVERITY,
> - &rp_pio->severity);
> - pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_SYSERROR,
> - &rp_pio->syserror);
> - pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_EXCEPTION,
> - &rp_pio->exception);
> -
> - /* Get First Error Pointer */
> - pci_read_config_word(pdev, cap + PCI_EXP_DPC_STATUS, &status);
> - rp_pio->first_error = (status & 0x1f00) >> 8;
> -
> if (dpc->rp_log_size < 4)
> return;
> -
> pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_HEADER_LOG,
> - &rp_pio->header_log.dw0);
> + &dw0);
> pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_HEADER_LOG + 4,
> - &rp_pio->header_log.dw1);
> + &dw1);
> pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_HEADER_LOG + 8,
> - &rp_pio->header_log.dw2);
> + &dw2);
> pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_HEADER_LOG + 12,
> - &rp_pio->header_log.dw3);
> - if (dpc->rp_log_size == 4)
> + &dw3);
> + dev_err(dev, "TLP Header: %#010x %#010x %#010x %#010x\n",
> + dw0, dw1, dw2, dw3);
> +
> + if (dpc->rp_log_size < 5)
> return;
> + pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_IMPSPEC_LOG,
> &log);
> + dev_err(dev, "RP PIO ImpSpec Log %#010x\n", log);
>
> - pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_IMPSPEC_LOG,
> - &rp_pio->impspec_log);
> - for (i = 0; i < dpc->rp_log_size - 5; i++)
> + for (i = 0; i < dpc->rp_log_size - 5; i++) {
> pci_read_config_dword(pdev,
> - cap + PCI_EXP_DPC_RP_PIO_TLPPREFIX_LOG,
> - &rp_pio->tlp_prefix_log[i]);
> -}
> -
> -static void dpc_process_rp_pio_error(struct dpc_dev *dpc)
> -{
> - struct dpc_rp_pio_regs rp_pio_regs;
> -
> - dpc_rp_pio_get_info(dpc, &rp_pio_regs);
> - dpc_rp_pio_print_error(dpc, &rp_pio_regs);
> -
> - dpc->rp_pio_status = rp_pio_regs.status;
> + cap + PCI_EXP_DPC_RP_PIO_TLPPREFIX_LOG, &prefix);
> + dev_err(dev, "TLP Prefix Header: dw%d, %#010x\n", i, prefix);
> + }
> }
>
> static irqreturn_t dpc_irq(int irq, void *context)
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 6/8] PCI/DPC: Consolidate RP PIO get/print functions
2018-01-30 0:42 ` okaya
@ 2018-01-30 18:11 ` Bjorn Helgaas
0 siblings, 0 replies; 12+ messages in thread
From: Bjorn Helgaas @ 2018-01-30 18:11 UTC (permalink / raw)
To: okaya; +Cc: Keith Busch, linux-pci, Dongdong Liu, linux-pci-owner
On Mon, Jan 29, 2018 at 07:42:54PM -0500, okaya@codeaurora.org wrote:
> On 2018-01-26 17:56, Bjorn Helgaas wrote:
> >From: Bjorn Helgaas <bhelgaas@google.com>
> >
> >Previously we defined a structure for RP PIO log information,
> >allocated it
> >on the stack, called one function to fill it in from DPC registers, and
> >called another to print it out.
> >
> >Simplify this by dropping the structure and printing the error
> >information
> >as soon as we read it from the DPC capability. This way we don't
> >need a
> >structure, there's one function instead of four, and everything we
> >do with
> >the register information is in one place instead of being split between
> >functions.
> >
> >No functional change intended.
> >
> >Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> >---
> > drivers/pci/pcie/pcie-dpc.c | 132
> >+++++++++++++------------------------------
> > 1 file changed, 39 insertions(+), 93 deletions(-)
> >
> >diff --git a/drivers/pci/pcie/pcie-dpc.c b/drivers/pci/pcie/pcie-dpc.c
> >index a6b8d1496322..06d3af112580 100644
> >--- a/drivers/pci/pcie/pcie-dpc.c
> >+++ b/drivers/pci/pcie/pcie-dpc.c
> >@@ -17,26 +17,6 @@
> > #include "../pci.h"
> > #include "aer/aerdrv.h"
> >
> >-struct rp_pio_header_log_regs {
> >- u32 dw0;
> >- u32 dw1;
> >- u32 dw2;
> >- u32 dw3;
> >-};
> >-
> >-struct dpc_rp_pio_regs {
> >- u32 status;
> >- u32 mask;
> >- u32 severity;
> >- u32 syserror;
> >- u32 exception;
> >-
> >- struct rp_pio_header_log_regs header_log;
> >- u32 impspec_log;
> >- u32 tlp_prefix_log[4];
> >- u16 first_error;
> >-};
> >-
> > struct dpc_dev {
> > struct pcie_device *dev;
> > struct work_struct work;
> >@@ -142,100 +122,66 @@ static void dpc_work(struct work_struct *work)
> > ctl | PCI_EXP_DPC_CTL_INT_EN);
> > }
> >
> >-static void dpc_rp_pio_print_tlp_header(struct device *dev,
> >- struct rp_pio_header_log_regs *t)
> >-{
> >- dev_err(dev, "TLP Header: %#010x %#010x %#010x %#010x\n",
> >- t->dw0, t->dw1, t->dw2, t->dw3);
> >-}
> >-
> >-static void dpc_rp_pio_print_error(struct dpc_dev *dpc,
> >- struct dpc_rp_pio_regs *rp_pio)
> >+static void dpc_process_rp_pio_error(struct dpc_dev *dpc)
> > {
> > struct device *dev = &dpc->dev->device;
> >+ struct pci_dev *pdev = dpc->dev->port;
> >+ u16 cap = dpc->cap_pos;
> >+ u32 status, mask;
> >+ u32 sev, syserr, exc;
> >+ u16 dpc_status, first_error;
> > int i;
> >- u32 status;
> >+ u32 dw0, dw1, dw2, dw3;
> >+ u32 log;
> >+ u32 prefix;
> >
> >+ pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_STATUS,
> >&status);
> >+ pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_MASK, &mask);
> > dev_err(dev, "rp_pio_status: %#010x, rp_pio_mask: %#010x\n",
> >- rp_pio->status, rp_pio->mask);
> >+ status, mask);
> >
> >+ dpc->rp_pio_status = status;
> >+
> >+ pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_SEVERITY, &sev);
> >+ pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_SYSERROR,
> >&syserr);
> >+ pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_EXCEPTION,
> >&exc);
> > dev_err(dev, "RP PIO severity=%#010x, syserror=%#010x,
> >exception=%#010x\n",
> >- rp_pio->severity, rp_pio->syserror, rp_pio->exception);
> >+ sev, syserr, exc);
> >
> >- status = (rp_pio->status & ~rp_pio->mask);
> >+ /* Get First Error Pointer */
> >+ pci_read_config_word(pdev, cap + PCI_EXP_DPC_STATUS, &dpc_status);
> >+ first_error = (dpc_status & 0x1f00) >> 8;
>
> This didn't look like a simple change here.
The diff is ugly, for sure. I'll split it up and you can see what
you think.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 7/8] PCI/DPC: Add and use DPC Status register field definitions
2018-01-26 22:55 [PATCH 0/8] PCI/DPC: Simplify RP PIO logging Bjorn Helgaas
` (5 preceding siblings ...)
2018-01-26 22:56 ` [PATCH 6/8] PCI/DPC: Consolidate RP PIO get/print functions Bjorn Helgaas
@ 2018-01-26 22:56 ` Bjorn Helgaas
2018-01-26 22:56 ` [PATCH 8/8] PCI/DPC: Reformat DPC register definitions Bjorn Helgaas
2018-01-26 23:15 ` [PATCH 0/8] PCI/DPC: Simplify RP PIO logging Keith Busch
8 siblings, 0 replies; 12+ messages in thread
From: Bjorn Helgaas @ 2018-01-26 22:56 UTC (permalink / raw)
To: Keith Busch; +Cc: linux-pci, Dongdong Liu
From: Bjorn Helgaas <bhelgaas@google.com>
Add definitions for DPC Status register fields and use them in the code.
No functional change intended.
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/pci/pcie/pcie-dpc.c | 4 ++--
include/uapi/linux/pci_regs.h | 2 ++
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/pcie/pcie-dpc.c b/drivers/pci/pcie/pcie-dpc.c
index 06d3af112580..014f6590cd79 100644
--- a/drivers/pci/pcie/pcie-dpc.c
+++ b/drivers/pci/pcie/pcie-dpc.c
@@ -216,8 +216,8 @@ static irqreturn_t dpc_irq(int irq, void *context)
dev_info(dev, "DPC containment event, status:%#06x source:%#06x\n",
status, source);
- reason = (status >> 1) & 0x3;
- ext_reason = (status >> 5) & 0x3;
+ reason = (status & PCI_EXP_DPC_STATUS_TRIGGER_RSN) >> 1;
+ ext_reason = (status & PCI_EXP_DPC_STATUS_TRIGGER_RSN_EXT) >> 5;
dev_warn(dev, "DPC %s detected, remove downstream devices\n",
(reason == 0) ? "unmasked uncorrectable error" :
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index 70c2b2ade048..970a0dc535c4 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -979,8 +979,10 @@
#define PCI_EXP_DPC_STATUS 8 /* DPC Status */
#define PCI_EXP_DPC_STATUS_TRIGGER 0x01 /* Trigger Status */
+#define PCI_EXP_DPC_STATUS_TRIGGER_RSN 0x06 /* Trigger Reason */
#define PCI_EXP_DPC_STATUS_INTERRUPT 0x08 /* Interrupt Status */
#define PCI_EXP_DPC_RP_BUSY 0x10 /* Root Port Busy */
+#define PCI_EXP_DPC_STATUS_TRIGGER_RSN_EXT 0x60 /* Trig Reason Extension */
#define PCI_EXP_DPC_SOURCE_ID 10 /* DPC Source Identifier */
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 8/8] PCI/DPC: Reformat DPC register definitions
2018-01-26 22:55 [PATCH 0/8] PCI/DPC: Simplify RP PIO logging Bjorn Helgaas
` (6 preceding siblings ...)
2018-01-26 22:56 ` [PATCH 7/8] PCI/DPC: Add and use DPC Status register field definitions Bjorn Helgaas
@ 2018-01-26 22:56 ` Bjorn Helgaas
2018-01-26 23:15 ` [PATCH 0/8] PCI/DPC: Simplify RP PIO logging Keith Busch
8 siblings, 0 replies; 12+ messages in thread
From: Bjorn Helgaas @ 2018-01-26 22:56 UTC (permalink / raw)
To: Keith Busch; +Cc: linux-pci, Dongdong Liu
From: Bjorn Helgaas <bhelgaas@google.com>
Reformat DPC register definitions to follow the convention that register
field masks indicate the register width, e.g., a field of a 16-bit register
uses a mask of 4 hex digits, with leading zeros included as needed.
No functional change intended.
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
include/uapi/linux/pci_regs.h | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index 970a0dc535c4..66d71461d2f0 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -966,28 +966,28 @@
/* Downstream Port Containment */
#define PCI_EXP_DPC_CAP 4 /* DPC Capability */
-#define PCI_EXP_DPC_IRQ 0x1f /* DPC Interrupt Message Number */
-#define PCI_EXP_DPC_CAP_RP_EXT 0x20 /* Root Port Extensions for DPC */
-#define PCI_EXP_DPC_CAP_POISONED_TLP 0x40 /* Poisoned TLP Egress Blocking Supported */
-#define PCI_EXP_DPC_CAP_SW_TRIGGER 0x80 /* Software Triggering Supported */
-#define PCI_EXP_DPC_RP_PIO_LOG_SIZE 0xF00 /* RP PIO log size */
+#define PCI_EXP_DPC_IRQ 0x001F /* Interrupt Message Number */
+#define PCI_EXP_DPC_CAP_RP_EXT 0x0020 /* Root Port Extensions */
+#define PCI_EXP_DPC_CAP_POISONED_TLP 0x0040 /* Poisoned TLP Egress Blocking Supported */
+#define PCI_EXP_DPC_CAP_SW_TRIGGER 0x0080 /* Software Triggering Supported */
+#define PCI_EXP_DPC_RP_PIO_LOG_SIZE 0x0F00 /* RP PIO Log Size */
#define PCI_EXP_DPC_CAP_DL_ACTIVE 0x1000 /* ERR_COR signal on DL_Active supported */
#define PCI_EXP_DPC_CTL 6 /* DPC control */
-#define PCI_EXP_DPC_CTL_EN_NONFATAL 0x02 /* Enable trigger on ERR_NONFATAL message */
-#define PCI_EXP_DPC_CTL_INT_EN 0x08 /* DPC Interrupt Enable */
+#define PCI_EXP_DPC_CTL_EN_NONFATAL 0x0002 /* Enable trigger on ERR_NONFATAL message */
+#define PCI_EXP_DPC_CTL_INT_EN 0x0008 /* DPC Interrupt Enable */
#define PCI_EXP_DPC_STATUS 8 /* DPC Status */
-#define PCI_EXP_DPC_STATUS_TRIGGER 0x01 /* Trigger Status */
-#define PCI_EXP_DPC_STATUS_TRIGGER_RSN 0x06 /* Trigger Reason */
-#define PCI_EXP_DPC_STATUS_INTERRUPT 0x08 /* Interrupt Status */
-#define PCI_EXP_DPC_RP_BUSY 0x10 /* Root Port Busy */
-#define PCI_EXP_DPC_STATUS_TRIGGER_RSN_EXT 0x60 /* Trig Reason Extension */
+#define PCI_EXP_DPC_STATUS_TRIGGER 0x0001 /* Trigger Status */
+#define PCI_EXP_DPC_STATUS_TRIGGER_RSN 0x0006 /* Trigger Reason */
+#define PCI_EXP_DPC_STATUS_INTERRUPT 0x0008 /* Interrupt Status */
+#define PCI_EXP_DPC_RP_BUSY 0x0010 /* Root Port Busy */
+#define PCI_EXP_DPC_STATUS_TRIGGER_RSN_EXT 0x0060 /* Trig Reason Extension */
#define PCI_EXP_DPC_SOURCE_ID 10 /* DPC Source Identifier */
#define PCI_EXP_DPC_RP_PIO_STATUS 0x0C /* RP PIO Status */
-#define PCI_EXP_DPC_RP_PIO_MASK 0x10 /* RP PIO MASK */
+#define PCI_EXP_DPC_RP_PIO_MASK 0x10 /* RP PIO Mask */
#define PCI_EXP_DPC_RP_PIO_SEVERITY 0x14 /* RP PIO Severity */
#define PCI_EXP_DPC_RP_PIO_SYSERROR 0x18 /* RP PIO SysError */
#define PCI_EXP_DPC_RP_PIO_EXCEPTION 0x1C /* RP PIO Exception */
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 0/8] PCI/DPC: Simplify RP PIO logging
2018-01-26 22:55 [PATCH 0/8] PCI/DPC: Simplify RP PIO logging Bjorn Helgaas
` (7 preceding siblings ...)
2018-01-26 22:56 ` [PATCH 8/8] PCI/DPC: Reformat DPC register definitions Bjorn Helgaas
@ 2018-01-26 23:15 ` Keith Busch
8 siblings, 0 replies; 12+ messages in thread
From: Keith Busch @ 2018-01-26 23:15 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: linux-pci, Dongdong Liu
On Fri, Jan 26, 2018 at 04:55:30PM -0600, Bjorn Helgaas wrote:
> These are almost all just cleanups.
>
> The only behavior change I intend is this: If a port does not support
> the "RP Extensions for DPC" and it reports an "RP PIO error", we
> previously read the RP PIO log registers. I don't know if that's a
> legal situation, but I couldn't find an explicit prohibition in the
> spec.
>
> Anyway, the rest is all cleanups and simplifications that are not
> intended to change any behavior.
>
> These are currently on my pci/dpc branch [1] on top of patches 3 and 5
> from your v2 series [2], Keith:
>
> 6b9045b34b57 PCI/DPC: Fix interrupt message number print
> eed85ff4c0da PCI/DPC: Enable DPC only if AER is available
>
> Your patch 1 ("PCI/AER: Return correct value when AER is not
> supported") is on pci/aer. Patches 2 and 4 go together and I think
> you're still working on those.
>
> You don't need to worry about integrating your work with these
> patches; I'm just kibbitzing and will drop these completely if you
> don't like them, or I'll take care of putting the pieces back together
> if you do.
No problem at all. I happened to pull your tree a few moments before you
posted this series, so I had a head start reviewing it. I'm happy with
the cleanups, and this doesn't trip up the updates I'm working on. I'm ok
to base the remaining DPC updates on this if you want to move it forward:
Reviewed-by: Keith Busch <keith.busch@intel.com>
I do need to merge pci/dpc with pci/aer to complete the rest of my DPC
updates, and there is a trivial merge conflict, but no biggie.
^ permalink raw reply [flat|nested] 12+ messages in thread