On Fri, May 15, 2020 at 01:47:57PM +0300, Serge Semin wrote: > DebugFS kernel interface provides a dedicated method to create the > registers dump file. Use it instead of creating a generic DebugFS > file with manually written read callback function.
With below nit addressed, Reviewed-by: Andy Shevchenko <[email protected]> > Signed-off-by: Serge Semin <[email protected]> > Cc: Georgy Vlasov <[email protected]> > Cc: Ramil Zaripov <[email protected]> > Cc: Alexey Malahov <[email protected]> > Cc: Thomas Bogendoerfer <[email protected]> > Cc: Paul Burton <[email protected]> > Cc: Ralf Baechle <[email protected]> > Cc: Arnd Bergmann <[email protected]> > Cc: Allison Randal <[email protected]> > Cc: Andy Shevchenko <[email protected]> > Cc: Gareth Williams <[email protected]> > Cc: Rob Herring <[email protected]> > Cc: [email protected] > Cc: [email protected] > --- > drivers/spi/spi-dw.c | 86 ++++++++++++++------------------------------ > drivers/spi/spi-dw.h | 2 ++ > 2 files changed, 28 insertions(+), 60 deletions(-) > > diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c > index 31607b40147d..bb470cff40d3 100644 > --- a/drivers/spi/spi-dw.c > +++ b/drivers/spi/spi-dw.c > @@ -29,66 +29,29 @@ struct chip_data { > }; > > #ifdef CONFIG_DEBUG_FS > -#define SPI_REGS_BUFSIZE 1024 > -static ssize_t dw_spi_show_regs(struct file *file, char __user *user_buf, > - size_t count, loff_t *ppos) > -{ > - struct dw_spi *dws = file->private_data; > - char *buf; > - u32 len = 0; > - ssize_t ret; > - > - buf = kzalloc(SPI_REGS_BUFSIZE, GFP_KERNEL); > - if (!buf) > - return 0; > - > - len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len, > - "%s registers:\n", dev_name(&dws->master->dev)); > - len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len, > - "=================================\n"); > - len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len, > - "CTRLR0: \t0x%08x\n", dw_readl(dws, DW_SPI_CTRLR0)); > - len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len, > - "CTRLR1: \t0x%08x\n", dw_readl(dws, DW_SPI_CTRLR1)); > - len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len, > - "SSIENR: \t0x%08x\n", dw_readl(dws, DW_SPI_SSIENR)); > - len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len, > - "SER: \t\t0x%08x\n", dw_readl(dws, DW_SPI_SER)); > - len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len, > - "BAUDR: \t\t0x%08x\n", dw_readl(dws, DW_SPI_BAUDR)); > - len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len, > - "TXFTLR: \t0x%08x\n", dw_readl(dws, DW_SPI_TXFTLR)); > - len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len, > - "RXFTLR: \t0x%08x\n", dw_readl(dws, DW_SPI_RXFTLR)); > - len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len, > - "TXFLR: \t\t0x%08x\n", dw_readl(dws, DW_SPI_TXFLR)); > - len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len, > - "RXFLR: \t\t0x%08x\n", dw_readl(dws, DW_SPI_RXFLR)); > - len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len, > - "SR: \t\t0x%08x\n", dw_readl(dws, DW_SPI_SR)); > - len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len, > - "IMR: \t\t0x%08x\n", dw_readl(dws, DW_SPI_IMR)); > - len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len, > - "ISR: \t\t0x%08x\n", dw_readl(dws, DW_SPI_ISR)); > - len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len, > - "DMACR: \t\t0x%08x\n", dw_readl(dws, DW_SPI_DMACR)); > - len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len, > - "DMATDLR: \t0x%08x\n", dw_readl(dws, DW_SPI_DMATDLR)); > - len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len, > - "DMARDLR: \t0x%08x\n", dw_readl(dws, DW_SPI_DMARDLR)); > - len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len, > - "=================================\n"); > - > - ret = simple_read_from_buffer(user_buf, count, ppos, buf, len); > - kfree(buf); > - return ret; > + > +#define DW_SPI_DBGFS_REG(_name, _off) \ > +{ \ > + .name = _name, \ > + .offset = _off \ As previously discussed (did I miss your answer?) the comma at the end leaves better pattern for maintenance prospective. > } > > -static const struct file_operations dw_spi_regs_ops = { > - .owner = THIS_MODULE, > - .open = simple_open, > - .read = dw_spi_show_regs, > - .llseek = default_llseek, > +static const struct debugfs_reg32 dw_spi_dbgfs_regs[] = { > + DW_SPI_DBGFS_REG("CTRLR0", DW_SPI_CTRLR0), > + DW_SPI_DBGFS_REG("CTRLR1", DW_SPI_CTRLR1), > + DW_SPI_DBGFS_REG("SSIENR", DW_SPI_SSIENR), > + DW_SPI_DBGFS_REG("SER", DW_SPI_SER), > + DW_SPI_DBGFS_REG("BAUDR", DW_SPI_BAUDR), > + DW_SPI_DBGFS_REG("TXFTLR", DW_SPI_TXFTLR), > + DW_SPI_DBGFS_REG("RXFTLR", DW_SPI_RXFTLR), > + DW_SPI_DBGFS_REG("TXFLR", DW_SPI_TXFLR), > + DW_SPI_DBGFS_REG("RXFLR", DW_SPI_RXFLR), > + DW_SPI_DBGFS_REG("SR", DW_SPI_SR), > + DW_SPI_DBGFS_REG("IMR", DW_SPI_IMR), > + DW_SPI_DBGFS_REG("ISR", DW_SPI_ISR), > + DW_SPI_DBGFS_REG("DMACR", DW_SPI_DMACR), > + DW_SPI_DBGFS_REG("DMATDLR", DW_SPI_DMATDLR), > + DW_SPI_DBGFS_REG("DMARDLR", DW_SPI_DMARDLR) > }; > > static int dw_spi_debugfs_init(struct dw_spi *dws) > @@ -100,8 +63,11 @@ static int dw_spi_debugfs_init(struct dw_spi *dws) > if (!dws->debugfs) > return -ENOMEM; > > - debugfs_create_file("registers", S_IFREG | S_IRUGO, > - dws->debugfs, (void *)dws, &dw_spi_regs_ops); > + dws->regset.regs = dw_spi_dbgfs_regs; > + dws->regset.nregs = ARRAY_SIZE(dw_spi_dbgfs_regs); > + dws->regset.base = dws->regs; > + debugfs_create_regset32("registers", 0400, dws->debugfs, &dws->regset); > + > return 0; > } > > diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h > index 24462b0c65cb..4adce6da6013 100644 > --- a/drivers/spi/spi-dw.h > +++ b/drivers/spi/spi-dw.h > @@ -2,6 +2,7 @@ > #ifndef DW_SPI_HEADER_H > #define DW_SPI_HEADER_H > > +#include <linux/debugfs.h> > #include <linux/irqreturn.h> > #include <linux/io.h> > #include <linux/scatterlist.h> > @@ -150,6 +151,7 @@ struct dw_spi { > > #ifdef CONFIG_DEBUG_FS > struct dentry *debugfs; > + struct debugfs_regset32 regset; > #endif > }; > > -- > 2.25.1 > -- With Best Regards, Andy Shevchenko
