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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [day] [month] [year] [list]
Message-ID: <20200515151056.GQ1634618@smile.fi.intel.com>
Date:   Fri, 15 May 2020 18:10:56 +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,
        Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@...el.com>,
        Charles Keepax <ckeepax@...nsource.cirrus.com>,
        Clement Leger <cleger@...ray.eu>,
        "wuxu.wu" <wuxu.wu@...wei.com>,
        Phil Edworthy <phil.edworthy@...esas.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Jarkko Nikula <jarkko.nikula@...ux.intel.com>,
        linux-spi@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 18/19] spi: dw: Use regset32 DebugFS method to create
 regdump file

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 <andriy.shevchenko@...ux.intel.com>

> 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 | 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


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ