Messages in this thread | | | Date | Mon, 12 Jun 2023 14:12:01 -0700 | From | Dan Williams <> | Subject | RE: [PATCH v5 21/26] cxl/pci: Update CXL error logging to use RAS register address |
| |
Terry Bowman wrote: > The CXL error handler currently only logs endpoint RAS status. The CXL > topology includes several components providing RAS details to be logged > during error handling.[1] Update the current handler's RAS logging to use a > RAS register address. This will allow for adding support to log other CXL > component's RAS details in the future. > > [1] CXL3.0 Table 8-22 CXL_Capability_ID Assignment > > Co-developed-by: Robert Richter <rrichter@amd.com> > Signed-off-by: Robert Richter <rrichter@amd.com> > Signed-off-by: Terry Bowman <terry.bowman@amd.com> > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- > drivers/cxl/core/pci.c | 42 ++++++++++++++++++++++++++++++------------ > 1 file changed, 30 insertions(+), 12 deletions(-) > > diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c > index 67f4ab6daa34..def6ee5ab4f5 100644 > --- a/drivers/cxl/core/pci.c > +++ b/drivers/cxl/core/pci.c > @@ -665,32 +665,36 @@ void read_cdat_data(struct cxl_port *port) > } > EXPORT_SYMBOL_NS_GPL(read_cdat_data, CXL); > > -void cxl_cor_error_detected(struct pci_dev *pdev) > +static void __cxl_log_correctable_ras(struct cxl_dev_state *cxlds, > + void __iomem *ras_base) > { > - struct cxl_dev_state *cxlds = pci_get_drvdata(pdev); > void __iomem *addr; > u32 status; > > - if (!cxlds->regs.ras) > + if (!ras_base) > return; > > - addr = cxlds->regs.ras + CXL_RAS_CORRECTABLE_STATUS_OFFSET; > + addr = ras_base + CXL_RAS_CORRECTABLE_STATUS_OFFSET; > status = readl(addr); > if (status & CXL_RAS_CORRECTABLE_STATUS_MASK) { > writel(status & CXL_RAS_CORRECTABLE_STATUS_MASK, addr); > trace_cxl_aer_correctable_error(cxlds->cxlmd, status); > } > } > -EXPORT_SYMBOL_NS_GPL(cxl_cor_error_detected, CXL); > + > +static void cxl_log_correctable_ras_endpoint(struct cxl_dev_state *cxlds) > +{ > + return __cxl_log_correctable_ras(cxlds, cxlds->regs.ras); > +}
As I review patch 24 it leads me back here to grumble about some naming choices. We now have:
cxl_cor_error_detected() __cxl_log_correctable_ras() cxl_report_and_clear()
Which of those clear the status? What is the difference between "report" and a "log"? I don't much care what name gets chosen but these three functions at least need to give the impression they were written with a common vision. The term "handle" would not surprise me in the names of functions that emit messages and clear register status bits.
| |