linux-kernel - Re: [PATCH v4] spi: imx: fix the burst length at DMA mode and CPU mode

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Thu, 01 Feb 2024 21:14:39 +0100
From: Benjamin Bigler <benjamin@...ler.one>
To: carlos.song@....com, broonie@...nel.org, shawnguo@...nel.org,
	s.hauer@...gutronix.de, kernel@...gutronix.de, linux-imx@....com,
	stefanmoring@...il.com
Cc: linux-spi@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4] spi: imx: fix the burst length at DMA mode and CPU
 mode

On Thu, 2024-02-01 at 18:54 +0800, carlos.song@....com wrote:
> From: Carlos Song <carlos.song@....com>
> 
> For DMA mode, the bus width of the DMA is equal to the size of data
> word, so burst length should be configured as bits per word.
> 
> For CPU mode, because of the spi transfer len is in byte, so burst
> length should be configured as bits per byte * spi_imx->count.
> 
> Signed-off-by: Carlos Song <carlos.song@....com>
> Reviewed-by: Clark Wang <xiaoning.wang@....com>
> Fixes: e9b220aeacf1 ("spi: spi-imx: correctly configure burst length when using dma")
> Fixes: 5f66db08cbd3 ("spi: imx: Take in account bits per word instead of assuming 8-bits")
> ---
> Changes for V3:
> - include <linux/bits.h>
> Changes for V4:
> - keep the includes sorted alphabetically.
> ---
>  drivers/spi/spi-imx.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
> index 546cdce525fc..f7990ac2c654 100644
> --- a/drivers/spi/spi-imx.c
> +++ b/drivers/spi/spi-imx.c
> @@ -2,6 +2,7 @@
>  // Copyright 2004-2007 Freescale Semiconductor, Inc. All Rights Reserved.
>  // Copyright (C) 2008 Juergen Beisert
>  
> +#include <linux/bits.h>
>  #include <linux/clk.h>
>  #include <linux/completion.h>
>  #include <linux/delay.h>
> @@ -660,15 +661,14 @@ static int mx51_ecspi_prepare_transfer(struct spi_imx_data *spi_imx,
>  			<< MX51_ECSPI_CTRL_BL_OFFSET;
>  	else {
>  		if (spi_imx->usedma) {
> -			ctrl |= (spi_imx->bits_per_word *
> -				spi_imx_bytes_per_word(spi_imx->bits_per_word) - 1)
> +			ctrl |= (spi_imx->bits_per_word - 1)
>  				<< MX51_ECSPI_CTRL_BL_OFFSET;
>  		} else {
>  			if (spi_imx->count >= MX51_ECSPI_CTRL_MAX_BURST)
> -				ctrl |= (MX51_ECSPI_CTRL_MAX_BURST - 1)
> +				ctrl |= (MX51_ECSPI_CTRL_MAX_BURST * BITS_PER_BYTE - 1)
>  						<< MX51_ECSPI_CTRL_BL_OFFSET;
>  			else
> -				ctrl |= (spi_imx->count * spi_imx->bits_per_word - 1)
> +				ctrl |= (spi_imx->count * BITS_PER_BYTE - 1)
I think that will not work for drivers which dont use bits_per_word=8. 
https://lore.kernel.org/all/20230917164037.29284-1-stefanmoring@gmail.com/
>  						<< MX51_ECSPI_CTRL_BL_OFFSET;
>  		}
>  	}

Best regards,
Benjamin Bigler