2023-05-23 17:30:51

by Peter Rosin

Subject: [PATCH v2 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

Changes since v1 [1], after comments from Tudor Ambarus:

Patch 1/2:
- Don't convert to inline functions.
- Cc stable

Patch 2/2:
- Extend the field instead of killing "too big" field values.
- Add Fixes and R-b tags.
- Cc stable

[1] https://lore.kernel.org/lkml/[email protected]/


Peter Rosin (2):
dmaengine: at_hdmac: Repair bitfield macros for peripheral ID handling
dmaengine: at_hdmac: Extend the Flow Controller bitfield to three bits

drivers/dma/at_hdmac.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)

--
2.20.1

2023-05-23 17:32:06

by Peter Rosin

Subject: [PATCH v2 1/2] dmaengine: at_hdmac: Repair bitfield macros for peripheral ID handling

The MSB part of the peripheral IDs need to go into the ATC_SRC_PER_MSB
and ATC_DST_PER_MSB fields. Not the LSB part.

This fixes a severe regression for TSE-850 devices (compatible
axentia,tse850v3) where output to the audio I2S codec (the main
purpose of the device) simply do not work.

Fixes: d8840a7edcf0 ("dmaengine: at_hdmac: Use bitfield access macros")
Cc: [email protected]
Signed-off-by: Peter Rosin <[email protected]>
---
drivers/dma/at_hdmac.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
index 8858470246e1..6362013b90df 100644
--- a/drivers/dma/at_hdmac.c
+++ b/drivers/dma/at_hdmac.c
@@ -153,8 +153,6 @@
#define ATC_AUTO BIT(31) /* Auto multiple buffer tx enable */

/* Bitfields in CFG */
-#define ATC_PER_MSB(h) ((0x30U & (h)) >> 4) /* Extract most significant bits of a handshaking identifier */
-
#define ATC_SRC_PER GENMASK(3, 0) /* Channel src rq associated with periph handshaking ifc h */
#define ATC_DST_PER GENMASK(7, 4) /* Channel dst rq associated with periph handshaking ifc h */
#define ATC_SRC_REP BIT(8) /* Source Replay Mod */
@@ -181,10 +179,15 @@
#define ATC_DPIP_HOLE GENMASK(15, 0)
#define ATC_DPIP_BOUNDARY GENMASK(25, 16)

-#define ATC_SRC_PER_ID(id) (FIELD_PREP(ATC_SRC_PER_MSB, (id)) | \
- FIELD_PREP(ATC_SRC_PER, (id)))
-#define ATC_DST_PER_ID(id) (FIELD_PREP(ATC_DST_PER_MSB, (id)) | \
- FIELD_PREP(ATC_DST_PER, (id)))
+#define ATC_PER_MSB GENMASK(5, 4) /* Extract MSBs of a handshaking identifier */
+#define ATC_SRC_PER_ID(id) \
+ ({ typeof(id) _id = (id); \
+ FIELD_PREP(ATC_SRC_PER_MSB, FIELD_GET(ATC_PER_MSB, _id)) | \
+ FIELD_PREP(ATC_SRC_PER, _id); })
+#define ATC_DST_PER_ID(id) \
+ ({ typeof(id) _id = (id); \
+ FIELD_PREP(ATC_DST_PER_MSB, FIELD_GET(ATC_PER_MSB, _id)) | \
+ FIELD_PREP(ATC_DST_PER, _id); })



--
2.20.1

2023-05-24 06:17:34

by Vinod Koul

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

On 23-05-23, 19:19, Peter Rosin wrote:
> 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.

Applied, thanks

--
~Vinod