Re: [PATCH v2 2/9] PCI/DPC: Prefix dmesg logs with PCIe service name
On Thu, May 02, 2019 at 10:59:39PM -0500, Frederick Lawler wrote: > Prefix dmesg logs with PCIe service name. The important thing about this patch is not so much that it adds a prefix (actually, it basically *moves* the prefix from the driver name ("dpc") to being part of the messages ("DPC:")), but that it changes the logging from being associated with the pcie_device to the pci_dev. I think the message change will be something like this (which I would include in the commit log): - dpc :80:10.0:pcie008: DPC error containment capabilities: ... + pcieport :80:10.0: DPC: error containment capabilities: ... with a subject like: PCI/DPC: Log messages with pci_dev, not pcie_device The above example assumes you drop the extra "DPC" as Keith suggested, which I think I agree with. Otherwise we'd have: + pcieport :80:10.0: DPC: DPC error containment capabilities: ... which is a little redundant. You could even include a link like: Link: https://lore.kernel.org/lkml/20190308180149.gd214...@google.com > Signed-off-by: Frederick Lawler > --- > drivers/pci/pcie/dpc.c | 37 ++--- > 1 file changed, 18 insertions(+), 19 deletions(-) > > diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c > index 7b77754a82de..934391c91c23 100644 > --- a/drivers/pci/pcie/dpc.c > +++ b/drivers/pci/pcie/dpc.c > @@ -6,6 +6,8 @@ > * Copyright (C) 2016 Intel Corp. > */ > > +#define dev_fmt(fmt) "DPC: " fmt > + > #include > #include > #include > @@ -100,7 +102,6 @@ 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 cap = dpc->cap_pos, status; > > pci_read_config_word(pdev, cap + PCI_EXP_DPC_STATUS, &status); > @@ -110,7 +111,7 @@ static int dpc_wait_rp_inactive(struct dpc_dev *dpc) > 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"); > + pci_warn(pdev, "DPC root port still busy\n"); > return -EBUSY; > } > return 0; > @@ -148,7 +149,6 @@ static pci_ers_result_t dpc_reset_link(struct pci_dev > *pdev) > > 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, dpc_status, first_error; > u32 status, mask, sev, syserr, exc, dw0, dw1, dw2, dw3, log, prefix; > @@ -156,13 +156,13 @@ static void dpc_process_rp_pio_error(struct dpc_dev > *dpc) > > 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", > + pci_err(pdev, "rp_pio_status: %#010x, rp_pio_mask: %#010x\n", > status, mask); > > 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", > + pci_err(pdev, "RP PIO severity=%#010x, syserror=%#010x, > exception=%#010x\n", > sev, syserr, exc); > > /* Get First Error Pointer */ > @@ -171,7 +171,7 @@ static void dpc_process_rp_pio_error(struct dpc_dev *dpc) > > for (i = 0; i < ARRAY_SIZE(rp_pio_error_string); i++) { > if ((status & ~mask) & (1 << i)) > - dev_err(dev, "[%2d] %s%s\n", i, rp_pio_error_string[i], > + pci_err(pdev, "[%2d] %s%s\n", i, rp_pio_error_string[i], > first_error == i ? " (First)" : ""); > } > > @@ -185,18 +185,18 @@ static void dpc_process_rp_pio_error(struct dpc_dev > *dpc) > &dw2); > pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_HEADER_LOG + 12, > &dw3); > - dev_err(dev, "TLP Header: %#010x %#010x %#010x %#010x\n", > + pci_err(pdev, "TLP Header: %#010x %#010x %#010x %#010x\n", > dw0, dw1, dw2, dw3); > > if (dpc->rp_log_size < 5) > goto clear_status; > 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_err(pdev, "RP PIO ImpSpec Log %#010x\n", log); > > for (i = 0; i < dpc->rp_log_size - 5; i++) { > pci_read_config_dword(pdev, > cap + PCI_EXP_DPC_RP_PIO_TLPPREFIX_LOG, &prefix); > - dev_err(dev, "TLP Prefix Header: dw%d, %#010x\n", i, prefix); > + pci_err(pdev, "TLP Prefix Header: dw%d, %#010x\n", i, prefix); > } > c
Re: [PATCH v2 2/9] PCI/DPC: Prefix dmesg logs with PCIe service name
On Thu, May 02, 2019 at 08:59:39PM -0700, Frederick Lawler wrote: > +#define dev_fmt(fmt) "DPC: " fmt > + > @@ -110,7 +111,7 @@ static int dpc_wait_rp_inactive(struct dpc_dev *dpc) > + pci_warn(pdev, "DPC root port still busy\n"); > @@ -229,18 +229,17 @@ static irqreturn_t dpc_handler(int irq, void *context) > + pci_warn(pdev, "DPC %s detected\n", > @@ -328,11 +327,11 @@ static int dpc_probe(struct pcie_device *dev) > + pci_info(pdev, "DPC error containment capabilities: Int Msg #%d, > RPExt%c PoisonedTLP%c SwTrigger%c RP PIO Log %d, DL_ActiveErr%c\n", Since you've already prefixed each print with "DPC: ", the extra "DPC" in the above prints is redundant.
[PATCH v2 2/9] PCI/DPC: Prefix dmesg logs with PCIe service name
Prefix dmesg logs with PCIe service name. Signed-off-by: Frederick Lawler --- drivers/pci/pcie/dpc.c | 37 ++--- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c index 7b77754a82de..934391c91c23 100644 --- a/drivers/pci/pcie/dpc.c +++ b/drivers/pci/pcie/dpc.c @@ -6,6 +6,8 @@ * Copyright (C) 2016 Intel Corp. */ +#define dev_fmt(fmt) "DPC: " fmt + #include #include #include @@ -100,7 +102,6 @@ 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 cap = dpc->cap_pos, status; pci_read_config_word(pdev, cap + PCI_EXP_DPC_STATUS, &status); @@ -110,7 +111,7 @@ static int dpc_wait_rp_inactive(struct dpc_dev *dpc) 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"); + pci_warn(pdev, "DPC root port still busy\n"); return -EBUSY; } return 0; @@ -148,7 +149,6 @@ static pci_ers_result_t dpc_reset_link(struct pci_dev *pdev) 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, dpc_status, first_error; u32 status, mask, sev, syserr, exc, dw0, dw1, dw2, dw3, log, prefix; @@ -156,13 +156,13 @@ static void dpc_process_rp_pio_error(struct dpc_dev *dpc) 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", + pci_err(pdev, "rp_pio_status: %#010x, rp_pio_mask: %#010x\n", status, mask); 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", + pci_err(pdev, "RP PIO severity=%#010x, syserror=%#010x, exception=%#010x\n", sev, syserr, exc); /* Get First Error Pointer */ @@ -171,7 +171,7 @@ static void dpc_process_rp_pio_error(struct dpc_dev *dpc) for (i = 0; i < ARRAY_SIZE(rp_pio_error_string); i++) { if ((status & ~mask) & (1 << i)) - dev_err(dev, "[%2d] %s%s\n", i, rp_pio_error_string[i], + pci_err(pdev, "[%2d] %s%s\n", i, rp_pio_error_string[i], first_error == i ? " (First)" : ""); } @@ -185,18 +185,18 @@ static void dpc_process_rp_pio_error(struct dpc_dev *dpc) &dw2); pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_HEADER_LOG + 12, &dw3); - dev_err(dev, "TLP Header: %#010x %#010x %#010x %#010x\n", + pci_err(pdev, "TLP Header: %#010x %#010x %#010x %#010x\n", dw0, dw1, dw2, dw3); if (dpc->rp_log_size < 5) goto clear_status; 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_err(pdev, "RP PIO ImpSpec Log %#010x\n", log); for (i = 0; i < dpc->rp_log_size - 5; i++) { pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_TLPPREFIX_LOG, &prefix); - dev_err(dev, "TLP Prefix Header: dw%d, %#010x\n", i, prefix); + pci_err(pdev, "TLP Prefix Header: dw%d, %#010x\n", i, prefix); } clear_status: pci_write_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_STATUS, status); @@ -229,18 +229,17 @@ static irqreturn_t dpc_handler(int irq, void *context) struct aer_err_info info; struct dpc_dev *dpc = context; struct pci_dev *pdev = dpc->dev->port; - struct device *dev = &dpc->dev->device; u16 cap = dpc->cap_pos, status, source, reason, ext_reason; pci_read_config_word(pdev, cap + PCI_EXP_DPC_STATUS, &status); pci_read_config_word(pdev, cap + PCI_EXP_DPC_SOURCE_ID, &source); - dev_info(dev, "DPC containment event, status:%#06x source:%#06x\n", + pci_info(pdev, "DPC containment event, status:%#06x source:%#06x\n", status, source); 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\n", + pci_warn(pdev, "DPC %s detected\n", (reason == 0) ? "unmasked uncorrectable error" : (reason == 1) ? "ERR_NONFATAL"