| 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
| ||
|
Message-ID: <20200515130305.GA1634618@smile.fi.intel.com>
Date: Fri, 15 May 2020 16:03:05 +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>,
Allison Randal <allison@...utok.net>,
Gareth Williams <gareth.williams.jx@...esas.com>,
Arnd Bergmann <arnd@...db.de>,
Rob Herring <robh+dt@...nel.org>, linux-mips@...r.kernel.org,
devicetree@...r.kernel.org,
Jarkko Nikula <jarkko.nikula@...ux.intel.com>,
Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@...el.com>,
Thomas Gleixner <tglx@...utronix.de>,
Clement Leger <cleger@...ray.eu>,
Xinwei Kong <kong.kongxinwei@...ilicon.com>,
"wuxu.wu" <wuxu.wu@...wei.com>,
Phil Edworthy <phil.edworthy@...esas.com>,
Linus Walleij <linus.walleij@...aro.org>,
linux-spi@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 08/19] spi: dw: Discard dma_width member of the dw_spi
structure
On Fri, May 15, 2020 at 01:47:47PM +0300, Serge Semin wrote:
> This member has exactly the same value as n_bytes of the DW SPI private
> data object, it's calculated at the same point of the transfer method,
> n_bytes isn't changed during the whole transfer, and they even serve for
> the same purpose - keep number of bytes per transfer word, though the
> dma_width is used only to calculate the DMA source/destination addresses
> width, which n_bytes could be also utilized for. Taking all of these
> into account let's replace the dma_width member usage with n_bytes one
> and remove the former.
I've no strong opinion about this.
So, after addressing one issue below,
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: Allison Randal <allison@...utok.net>
> Cc: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
> Cc: Gareth Williams <gareth.williams.jx@...esas.com>
> Cc: Arnd Bergmann <arnd@...db.de>
> Cc: Rob Herring <robh+dt@...nel.org>
> Cc: linux-mips@...r.kernel.org
> Cc: devicetree@...r.kernel.org
>
> ---
>
> Changelog v2:
> - It's a new patch created as a result of more thorough driver study.
> ---
> drivers/spi/spi-dw-mid.c | 10 +++++-----
> drivers/spi/spi-dw.c | 1 -
> drivers/spi/spi-dw.h | 1 -
> 3 files changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/spi/spi-dw-mid.c b/drivers/spi/spi-dw-mid.c
> index be886e22a1b1..ca8813a693d8 100644
> --- a/drivers/spi/spi-dw-mid.c
> +++ b/drivers/spi/spi-dw-mid.c
> @@ -132,10 +132,10 @@ static bool mid_spi_can_dma(struct spi_controller *master,
> return xfer->len > dws->fifo_len;
> }
>
> -static enum dma_slave_buswidth convert_dma_width(u32 dma_width) {
> - if (dma_width == 1)
> +static enum dma_slave_buswidth convert_dma_width(u8 n_bytes) {
It seems somebody (maybe even me) at some point messed up between enum
definition and function that returns an enum.
For what said, { should be on the separate line.
> + if (n_bytes == 1)
> return DMA_SLAVE_BUSWIDTH_1_BYTE;
> - else if (dma_width == 2)
> + else if (n_bytes == 2)
> return DMA_SLAVE_BUSWIDTH_2_BYTES;
>
> return DMA_SLAVE_BUSWIDTH_UNDEFINED;
> @@ -195,7 +195,7 @@ static struct dma_async_tx_descriptor *dw_spi_dma_prepare_tx(struct dw_spi *dws,
> txconf.dst_addr = dws->dma_addr;
> txconf.dst_maxburst = 16;
> txconf.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
> - txconf.dst_addr_width = convert_dma_width(dws->dma_width);
> + txconf.dst_addr_width = convert_dma_width(dws->n_bytes);
> txconf.device_fc = false;
>
> dmaengine_slave_config(dws->txchan, &txconf);
> @@ -268,7 +268,7 @@ static struct dma_async_tx_descriptor *dw_spi_dma_prepare_rx(struct dw_spi *dws,
> rxconf.src_addr = dws->dma_addr;
> rxconf.src_maxburst = 16;
> rxconf.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
> - rxconf.src_addr_width = convert_dma_width(dws->dma_width);
> + rxconf.src_addr_width = convert_dma_width(dws->n_bytes);
> rxconf.device_fc = false;
>
> dmaengine_slave_config(dws->rxchan, &rxconf);
> diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c
> index 450c8218caeb..1edb8cdd11ee 100644
> --- a/drivers/spi/spi-dw.c
> +++ b/drivers/spi/spi-dw.c
> @@ -353,7 +353,6 @@ static int dw_spi_transfer_one(struct spi_controller *master,
> }
>
> dws->n_bytes = DIV_ROUND_UP(transfer->bits_per_word, BITS_PER_BYTE);
> - dws->dma_width = DIV_ROUND_UP(transfer->bits_per_word, BITS_PER_BYTE);
>
> cr0 = dws->update_cr0(master, spi, transfer);
> dw_writel(dws, DW_SPI_CTRLR0, cr0);
> diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
> index b6ab81e0c747..4902f937c3d7 100644
> --- a/drivers/spi/spi-dw.h
> +++ b/drivers/spi/spi-dw.h
> @@ -136,7 +136,6 @@ struct dw_spi {
> void *rx_end;
> int dma_mapped;
> u8 n_bytes; /* current is a 1/2 bytes op */
> - u32 dma_width;
> irqreturn_t (*transfer_handler)(struct dw_spi *dws);
> u32 current_freq; /* frequency in hz */
>
> --
> 2.25.1
>
--
With Best Regards,
Andy Shevchenko
Powered by blists - more mailing lists