Re: [PATCH v2 1/3] sh: dma: Fix dma channel offset calculation

[John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>]

On Sat, 2023-05-27 at 18:44 +0200, Artur Rojek wrote:
> Various SoCs of the SH3, SH4 and SH4A family, which use this driver,
> feature a differing number of DMA channels, which can be distributed
> between up to two DMAC modules. Existing implementation fails to
> correctly accommodate for all those variations, resulting in wrong
> channel offset calculations and leading to kernel panics.
> 
> Rewrite dma_base_addr() in order to properly calculate channel offsets
> in a DMAC module. Fix dmaor_read_reg() and dmaor_write_reg(), so that
> the correct DMAC module base is selected for the DMAOR register.
> 
> Fixes: 7f47c7189b3e8f19 ("sh: dma: More legacy cpu dma chainsawing.")
> Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
> ---
> 
> v2: also handle differing numbers of DMAC modules and channels
> 
>  arch/sh/drivers/dma/dma-sh.c | 37 +++++++++++++++++++++++-------------
>  1 file changed, 24 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/sh/drivers/dma/dma-sh.c b/arch/sh/drivers/dma/dma-sh.c
> index 96c626c2cd0a..306fba1564e5 100644
> --- a/arch/sh/drivers/dma/dma-sh.c
> +++ b/arch/sh/drivers/dma/dma-sh.c
> @@ -18,6 +18,18 @@
>  #include <cpu/dma-register.h>
>  #include <cpu/dma.h>
>  
> +/*
> + * Some of the SoCs feature two DMAC modules. In such a case, the channels are
> + * distributed equally among them.
> + */
> +#ifdef	SH_DMAC_BASE1
> +#define	SH_DMAC_NR_MD_CH	(CONFIG_NR_ONCHIP_DMA_CHANNELS / 2)
> +#else
> +#define	SH_DMAC_NR_MD_CH	CONFIG_NR_ONCHIP_DMA_CHANNELS
> +#endif
> +
> +#define	SH_DMAC_CH_SZ		0x10
> +
>  /*
>   * Define the default configuration for dual address memory-memory transfer.
>   * The 0x400 value represents auto-request, external->external.
> @@ -29,7 +41,7 @@ static unsigned long dma_find_base(unsigned int chan)
>  	unsigned long base = SH_DMAC_BASE0;
>  
>  #ifdef SH_DMAC_BASE1
> -	if (chan >= 6)
> +	if (chan >= SH_DMAC_NR_MD_CH)
>  		base = SH_DMAC_BASE1;
>  #endif
>  
> @@ -40,13 +52,13 @@ static unsigned long dma_base_addr(unsigned int chan)
>  {
>  	unsigned long base = dma_find_base(chan);
>  
> -	/* Normalize offset calculation */
> -	if (chan >= 9)
> -		chan -= 6;
> -	if (chan >= 4)
> -		base += 0x10;
> +	chan = (chan % SH_DMAC_NR_MD_CH) * SH_DMAC_CH_SZ;
> +
> +	/* DMAOR is placed inside the channel register space. Step over it. */
> +	if (chan >= DMAOR)
> +		base += SH_DMAC_CH_SZ;
>  
> -	return base + (chan * 0x10);
> +	return base + chan;
>  }
>  
>  #ifdef CONFIG_SH_DMA_IRQ_MULTI
> @@ -250,12 +262,11 @@ static int sh_dmac_get_dma_residue(struct dma_channel *chan)
>  #define NR_DMAOR	1
>  #endif
>  
> -/*
> - * DMAOR bases are broken out amongst channel groups. DMAOR0 manages
> - * channels 0 - 5, DMAOR1 6 - 11 (optional).
> - */
> -#define dmaor_read_reg(n)		__raw_readw(dma_find_base((n)*6))
> -#define dmaor_write_reg(n, data)	__raw_writew(data, dma_find_base(n)*6)
> +#define dmaor_read_reg(n)		__raw_readw(dma_find_base((n) * \
> +						    SH_DMAC_NR_MD_CH) + DMAOR)
> +#define dmaor_write_reg(n, data)	__raw_writew(data, \
> +						     dma_find_base((n) * \
> +						     SH_DMAC_NR_MD_CH) + DMAOR)
>  
>  static inline int dmaor_reset(int no)
>  {

Reviewed-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913