| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 8 May 2020 22:30:27 +0300
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: Serge Semin <Sergey.Semin@...kalelectronics.ru>
Cc: Mark Brown <broonie@...nel.org>,
Serge Semin <fancer.lancer@...il.com>,
Georgy Vlasov <Georgy.Vlasov@...kalelectronics.ru>,
Ramil Zaripov <Ramil.Zaripov@...kalelectronics.ru>,
Alexey Malahov <Alexey.Malahov@...kalelectronics.ru>,
Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
Paul Burton <paulburton@...nel.org>,
Ralf Baechle <ralf@...ux-mips.org>,
Arnd Bergmann <arnd@...db.de>,
Allison Randal <allison@...utok.net>,
Gareth Williams <gareth.williams.jx@...esas.com>,
Rob Herring <robh+dt@...nel.org>, linux-mips@...r.kernel.org,
devicetree@...r.kernel.org, fengsheng <fengsheng5@...wei.com>,
Phil Edworthy <phil.edworthy@...esas.com>,
Thomas Gleixner <tglx@...utronix.de>,
Xinwei Kong <kong.kongxinwei@...ilicon.com>,
Aditya Pakki <pakki001@....edu>,
"wuxu.wu" <wuxu.wu@...wei.com>,
Charles Keepax <ckeepax@...nsource.cirrus.com>,
Jarkko Nikula <jarkko.nikula@...ux.intel.com>,
Linus Walleij <linus.walleij@...aro.org>,
linux-spi@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 17/17] spi: dw: Use regset32 DebugFS method to create a
registers file
On Fri, May 08, 2020 at 04:29:42PM +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.
Of course ideal would be use something like regmap API which delivers this as a side effect.
But this also good clean up!
Comments below.
>
> Signed-off-by: Serge Semin <Sergey.Semin@...kalelectronics.ru>
> Cc: Georgy Vlasov <Georgy.Vlasov@...kalelectronics.ru>
> Cc: Ramil Zaripov <Ramil.Zaripov@...kalelectronics.ru>
> Cc: Alexey Malahov <Alexey.Malahov@...kalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@...ha.franken.de>
> Cc: Paul Burton <paulburton@...nel.org>
> Cc: Ralf Baechle <ralf@...ux-mips.org>
> Cc: Arnd Bergmann <arnd@...db.de>
> Cc: Allison Randal <allison@...utok.net>
> Cc: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
> Cc: Gareth Williams <gareth.williams.jx@...esas.com>
> Cc: Rob Herring <robh+dt@...nel.org>
> Cc: linux-mips@...r.kernel.org
> Cc: devicetree@...r.kernel.org
> ---
> drivers/spi/spi-dw.c | 85 +++++++++++++-------------------------------
> drivers/spi/spi-dw.h | 2 ++
> 2 files changed, 27 insertions(+), 60 deletions(-)
>
> diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c
> index 897c998af578..625a5f152e1a 100644
> --- a/drivers/spi/spi-dw.c
> +++ b/drivers/spi/spi-dw.c
> @@ -32,66 +32,28 @@ 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,
> - "CTRL0: \t\t0x%08x\n", dw_readl(dws, DW_SPI_CTRL0));
> - len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len,
> - "CTRL1: \t\t0x%08x\n", dw_readl(dws, DW_SPI_CTRL1));
> - 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_TXFLTR));
> - len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len,
> - "RXFTLR: \t0x%08x\n", dw_readl(dws, DW_SPI_RXFLTR));
> - 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 \
Leave comma here.
> }
>
> -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("CTRL0", DW_SPI_CTRL0),
> + DW_SPI_DBGFS_REG("CTRL1", DW_SPI_CTRL1),
> + 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_TXFLTR),
> + DW_SPI_DBGFS_REG("RXFTLR", DW_SPI_RXFLTR),
> + 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)
> @@ -103,8 +65,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 a6c03ea9013c..bad0eca2efe6 100644
> --- a/drivers/spi/spi-dw.h
> +++ b/drivers/spi/spi-dw.h
> @@ -4,6 +4,7 @@
>
> #include <linux/io.h>
> #include <linux/scatterlist.h>
> +#include <linux/debugfs.h>
Perhaps keep ordered?
>
> /* Register offsets */
> #define DW_SPI_CTRL0 0x00
> @@ -142,6 +143,7 @@ struct dw_spi {
> void *priv;
> #ifdef CONFIG_DEBUG_FS
> struct dentry *debugfs;
> + struct debugfs_regset32 regset;
I'm wondering why we need it here and not simple on the stack?
> #endif
> };
>
> --
> 2.25.1
>
--
With Best Regards,
Andy Shevchenko
Powered by blists - more mailing lists