2023-05-23 13:10:23

by Peter Rosin

Subject: [PATCH 0/2] dmaengine: at_hdmac: Regression fix and cleanup

Hi!

I found a regression in DMA handling on one of our SAMA5D3 boards.

While combing through the regressing commit, a found two unrelated
strange things. The first is the actually problematic change. The
second is a number of suspect defines, that I fail to see how they
can ever do any good.

Cheers,
Peter

Peter Rosin (2):
dmaengine: at_hdmac: Repair bitfield macros for peripheral ID handling
dmaengine: at_hdmac: Remove unused field values that do not fit

drivers/dma/at_hdmac.c | 39 ++++++++++++++++++++++++++++-----------
1 file changed, 28 insertions(+), 11 deletions(-)

--
2.20.1

2023-05-23 13:11:39

by Peter Rosin

Subject: [PATCH 2/2] dmaengine: at_hdmac: Remove unused field values that do not fit

The values are not used, and they do not fit in the intended
register field (ATC_FC, 2 bits wide). Kill 'em all.

Signed-off-by: Peter Rosin <[email protected]>
---
drivers/dma/at_hdmac.c | 4 ----
1 file changed, 4 deletions(-)

diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
index 6f352160bc3b..51d0a0c7aaf7 100644
--- a/drivers/dma/at_hdmac.c
+++ b/drivers/dma/at_hdmac.c
@@ -137,10 +137,6 @@
#define ATC_FC_MEM2PER 0x1 /* Mem-to-Periph (DMA) */
#define ATC_FC_PER2MEM 0x2 /* Periph-to-Mem (DMA) */
#define ATC_FC_PER2PER 0x3 /* Periph-to-Periph (DMA) */
-#define ATC_FC_PER2MEM_PER 0x4 /* Periph-to-Mem (Peripheral) */
-#define ATC_FC_MEM2PER_PER 0x5 /* Mem-to-Periph (Peripheral) */
-#define ATC_FC_PER2PER_SRCPER 0x6 /* Periph-to-Periph (Src Peripheral) */
-#define ATC_FC_PER2PER_DSTPER 0x7 /* Periph-to-Periph (Dst Peripheral) */
#define ATC_SRC_ADDR_MODE GENMASK(25, 24)
#define ATC_SRC_ADDR_MODE_INCR 0x0 /* Incrementing Mode */
#define ATC_SRC_ADDR_MODE_DECR 0x1 /* Decrementing Mode */
--
2.20.1

2023-05-23 14:33:35

by Tudor Ambarus

Subject: Re: [PATCH 2/2] dmaengine: at_hdmac: Remove unused field values that do not fit

Hi, Peter,

On 5/23/23 13:42, Peter Rosin wrote:
> The values are not used, and they do not fit in the intended
> register field (ATC_FC, 2 bits wide). Kill 'em all.
>
Actually ATC_FC should be set to GENMASK(23, 21) and keep the other
definitions, see:
http://ww1.microchip.com/downloads/en/devicedoc/Atmel-6438-32-bit-ARM926-Embedded-Microprocessor-SAM9G45_Datasheet.pdf

After you update the patch you should also add:
Fixes: d8840a7edcf0 ("dmaengine: at_hdmac: Use bitfield access macros")
Cc: [email protected]
so that it gets backported to Linux-stable.

Feel free to add my R-b tag after addressing the comments:
Reviewed-by: Tudor Ambarus <[email protected]>

> Signed-off-by: Peter Rosin <[email protected]>
> ---
> drivers/dma/at_hdmac.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
> index 6f352160bc3b..51d0a0c7aaf7 100644
> --- a/drivers/dma/at_hdmac.c
> +++ b/drivers/dma/at_hdmac.c
> @@ -137,10 +137,6 @@
> #define ATC_FC_MEM2PER 0x1 /* Mem-to-Periph (DMA) */
> #define ATC_FC_PER2MEM 0x2 /* Periph-to-Mem (DMA) */
> #define ATC_FC_PER2PER 0x3 /* Periph-to-Periph (DMA) */
> -#define ATC_FC_PER2MEM_PER 0x4 /* Periph-to-Mem (Peripheral) */
> -#define ATC_FC_MEM2PER_PER 0x5 /* Mem-to-Periph (Peripheral) */
> -#define ATC_FC_PER2PER_SRCPER 0x6 /* Periph-to-Periph (Src Peripheral) */
> -#define ATC_FC_PER2PER_DSTPER 0x7 /* Periph-to-Periph (Dst Peripheral) */
> #define ATC_SRC_ADDR_MODE GENMASK(25, 24)
> #define ATC_SRC_ADDR_MODE_INCR 0x0 /* Incrementing Mode */
> #define ATC_SRC_ADDR_MODE_DECR 0x1 /* Decrementing Mode */