bcm2835-dma: fix cyclic DMA setup

The current implementation is broken in several ways:

The number of frames was calculated wrong, which results in
one frame less than requested. eg when requesting a 96000 bytes
buffer split into 4 periods of 24000 bytes the code would allocate
3 frames: the first two with 24000 bytes size and the third one
with 48000 bytes.

This not only results in the cyclic callback being called at other
times than requested but can also lead to frames larger than what
the DMA hardware can support. eg when requesting a 192000 bytes buffer
split into 4 periods of 48000 bytes each (well within the limit
of what the DMA hardware supports) it would allocate 2 48000 byte
frames and one 96000 byte frame - the latter larger than what the
hardware supports.

Frames larger than the hardware limit are the reason for clicking
playback and the previous fix of using a smaller limit for
cyclic transfers only worked around the underlying bug.

Another issue is the way the code tried to handle period sizes larger
than what the hardware supports. It results in cyclic callbacks
not at the requested period length but in callbacks at the hardware
limit boundaries.

The simple fix for these issues is to reject requests that can't
be handled by the hardware.

The maximum period length in bcm2835-i2s has been fixed (set to
64k-4 bytes, the limit of the lite DMA channels) so the cyclic
DMA setup code should never be called with period lenght larger
than the hardware limit, thus period splitting is not neccessary
at all.

Signed-off-by: Matthias Reichl <[email protected]>

Author: HiassofT

drivers/dma/bcm2835-dma.c

@@ -144,12 +144,6 @@ struct bcm2835_desc {
  */
 #define MAX_LITE_TRANSFER	(SZ_64K - 4)
 
-/*
- * Transfers larger than 32k cause issues with the bcm2708-i2s driver,
- * so limit transfer size to 32k as bcm2708-dmaengine did.
- */
-#define MAX_CYCLIC_LITE_TRANSFER	SZ_32K
-
 static inline struct bcm2835_dmadev *to_bcm2835_dma_dev(struct dma_device *d)
 {
 	return container_of(d, struct bcm2835_dmadev, ddev);
@@ -385,6 +379,15 @@ static struct dma_async_tx_descriptor *bcm2835_dma_prep_dma_cyclic(
 	unsigned int frame, max_size;
 	int i;
 
+	if (!buf_len || !period_len)
+		return NULL;
+
+	if (buf_len % period_len) {
+		dev_err(chan->device->dev,
+			 "Buffer length should be a multiple of period\n");
+		return NULL;
+	}
+
 	/* Grab configuration */
 	if (!is_slave_direction(direction)) {
 		dev_err(chan->device->dev, "%s: bad direction?\n", __func__);
@@ -410,19 +413,26 @@ static struct dma_async_tx_descriptor *bcm2835_dma_prep_dma_cyclic(
 		return NULL;
 	}
 
+	if (c->ch >= 8) /* LITE channel */
+		max_size = MAX_LITE_TRANSFER;
+	else
+		max_size = MAX_NORMAL_TRANSFER;
+
+	if (period_len > max_size) {
+		dev_err(chan->device->dev,
+			"Period length %d larger than maximum %d\n",
+			period_len, max_size);
+		return NULL;
+	}
+
 	/* Now allocate and setup the descriptor. */
 	d = kzalloc(sizeof(*d), GFP_NOWAIT);
 	if (!d)
 		return NULL;
 
 	d->c = c;
 	d->dir = direction;
-	if (c->ch >= 8) /* LITE channel */
-		max_size = MAX_CYCLIC_LITE_TRANSFER;
-	else
-		max_size = MAX_NORMAL_TRANSFER;
-	period_len = min(period_len, max_size);
-	d->frames = (buf_len - 1) / (period_len + 1);
+	d->frames = buf_len / period_len;
 
 	d->cb_list = kcalloc(d->frames, sizeof(*d->cb_list), GFP_KERNEL);
 	if (!d->cb_list) {
@@ -470,11 +480,7 @@ static struct dma_async_tx_descriptor *bcm2835_dma_prep_dma_cyclic(
 				BCM2835_DMA_PER_MAP(c->dreq);
 
 		/* Length of a frame */
-		if (frame != d->frames - 1)
-			control_block->length = period_len;
-		else
-			control_block->length = buf_len - (d->frames - 1) *
-						period_len;
+		control_block->length = period_len;
 		d->size += control_block->length;
 
 		/*