Re: [PATCH 07/18] dmaengine: bcm2385: drop info parameters

Frank Li
2024-06-05T18:00:19Z

On Fri, May 24, 2024 at 07:26:51PM +0100, Dave Stevenson wrote:
> From: Stefan Wahren <stefan.wahren@i2se.com>
>
> The parameters info and finalextrainfo are platform specific. So drop
> them by generating them within bcm2835_dma_create_cb_chain().

Drop 'info' and 'finalextrainfo' because these can be generated by
bcm2835_dma_create_cb_chain().

>
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> ---
> drivers/dma/bcm2835-dma.c | 83 +++++++++++++++++++--------------------
> 1 file changed, 40 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/dma/bcm2835-dma.c b/drivers/dma/bcm2835-dma.c
> index d6c5a2762a46..e2f9c8692e6b 100644
> --- a/drivers/dma/bcm2835-dma.c
> +++ b/drivers/dma/bcm2835-dma.c
> @@ -287,13 +287,11 @@ static void bcm2835_dma_desc_free(struct virt_dma_desc *vd)
> container_of(vd, struct bcm2835_desc, vd));
> }
>
> -static void bcm2835_dma_create_cb_set_length(
> - struct bcm2835_chan *chan,
> - struct bcm2835_dma_cb *control_block,
> - size_t len,
> - size_t period_len,
> - size_t *total_len,
> - u32 finalextrainfo)
> +static bool
> +bcm2835_dma_create_cb_set_length(struct bcm2835_chan *chan,
> + struct bcm2835_dma_cb *control_block,
> + size_t len, size_t period_len,
> + size_t *total_len)

Can you document this function, what's return value means? look like if
need extrainfo?

> {
> size_t max_len = bcm2835_dma_max_frame_length(chan);
>
> @@ -302,7 +300,7 @@ static void bcm2835_dma_create_cb_set_length(
>
> /* finished if we have no period_length */
> if (!period_len)
> - return;
> + return false;
>
> /*
> * period_len means: that we need to generate
> @@ -316,7 +314,7 @@ static void bcm2835_dma_create_cb_set_length(
> if (*total_len + control_block->length < period_len) {
> /* update number of bytes in this period so far */
> *total_len += control_block->length;
> - return;
> + return false;
> }
>
> /* calculate the length that remains to reach period_length */
> @@ -325,8 +323,7 @@ static void bcm2835_dma_create_cb_set_length(
> /* reset total_length for next period */
> *total_len = 0;
>
> - /* add extrainfo bits in info */
> - control_block->info |= finalextrainfo;
> + return true;
> }
>
> static inline size_t bcm2835_dma_count_frames_for_sg(
> @@ -352,7 +349,6 @@ static inline size_t bcm2835_dma_count_frames_for_sg(
> * @chan: the @dma_chan for which we run this
> * @direction: the direction in which we transfer
> * @cyclic: it is a cyclic transfer
> - * @info: the default info bits to apply per controlblock
> * @frames: number of controlblocks to allocate
> * @src: the src address to assign
> * @dst: the dst address to assign
> @@ -360,22 +356,24 @@ static inline size_t bcm2835_dma_count_frames_for_sg(
> * @period_len: the period length when to apply @finalextrainfo
> * in addition to the last transfer
> * this will also break some control-blocks early
> - * @finalextrainfo: additional bits in last controlblock
> - * (or when period_len is reached in case of cyclic)
> * @gfp: the GFP flag to use for allocation
> + * @flags
> */
> static struct bcm2835_desc *bcm2835_dma_create_cb_chain(
> struct dma_chan *chan, enum dma_transfer_direction direction,
> - bool cyclic, u32 info, u32 finalextrainfo, size_t frames,
> - dma_addr_t src, dma_addr_t dst, size_t buf_len,
> - size_t period_len, gfp_t gfp)
> + bool cyclic, size_t frames, dma_addr_t src, dma_addr_t dst,
> + size_t buf_len, size_t period_len, gfp_t gfp, unsigned long flags)
> {
> + struct bcm2835_dmadev *od = to_bcm2835_dma_dev(chan->device);
> struct bcm2835_chan *c = to_bcm2835_dma_chan(chan);
> size_t len = buf_len, total_len;
> size_t frame;
> struct bcm2835_desc *d;
> struct bcm2835_cb_entry *cb_entry;
> struct bcm2835_dma_cb *control_block;
> + u32 extrainfo = bcm2835_dma_prepare_cb_extra(c, direction, cyclic,
> + false, flags);
> + bool zero_page = false;
>
> if (!frames)
> return NULL;
> @@ -389,6 +387,14 @@ static struct bcm2835_desc *bcm2835_dma_create_cb_chain(
> d->dir = direction;
> d->cyclic = cyclic;
>
> + switch (direction) {
> + case DMA_MEM_TO_MEM:
> + case DMA_DEV_TO_MEM:
> + break;
> + default:
> + zero_page = src == od->zero_page;
> + }
> +
> /*
> * Iterate over all frames, create a control block
> * for each frame and link them together.
> @@ -402,7 +408,8 @@ static struct bcm2835_desc *bcm2835_dma_create_cb_chain(
>
> /* fill in the control block */
> control_block = cb_entry->cb;
> - control_block->info = info;
> + control_block->info = bcm2835_dma_prepare_cb_info(c, direction,
> + zero_page);
> control_block->src = src;
> control_block->dst = dst;
> control_block->stride = 0;
> @@ -410,10 +417,12 @@ static struct bcm2835_desc *bcm2835_dma_create_cb_chain(
> /* set up length in control_block if requested */
> if (buf_len) {
> /* calculate length honoring period_length */
> - bcm2835_dma_create_cb_set_length(
> - c, control_block,
> - len, period_len, &total_len,
> - cyclic ? finalextrainfo : 0);
> + if (bcm2835_dma_create_cb_set_length(c, control_block,
> + len, period_len,
> + &total_len)) {
> + /* add extrainfo bits in info */
> + control_block->info |= extrainfo;
> + }
>
> /* calculate new remaining length */
> len -= control_block->length;
> @@ -434,7 +443,9 @@ static struct bcm2835_desc *bcm2835_dma_create_cb_chain(
> }
>
> /* the last frame requires extra flags */
> - d->cb_list[d->frames - 1].cb->info |= finalextrainfo;
> + extrainfo = bcm2835_dma_prepare_cb_extra(c, direction, cyclic, true,
> + flags);
> + d->cb_list[d->frames - 1].cb->info |= extrainfo;
>
> /* detect a size missmatch */
> if (buf_len && (d->size != buf_len))
> @@ -682,9 +693,6 @@ static struct dma_async_tx_descriptor *bcm2835_dma_prep_dma_memcpy(
> {
> struct bcm2835_chan *c = to_bcm2835_dma_chan(chan);
> struct bcm2835_desc *d;
> - u32 info = bcm2835_dma_prepare_cb_info(c, DMA_MEM_TO_MEM, false);
> - u32 extra = bcm2835_dma_prepare_cb_extra(c, DMA_MEM_TO_MEM, false,
> - true, 0);
> size_t max_len = bcm2835_dma_max_frame_length(c);
> size_t frames;
>
> @@ -696,9 +704,8 @@ static struct dma_async_tx_descriptor *bcm2835_dma_prep_dma_memcpy(
> frames = bcm2835_dma_frames_for_length(len, max_len);
>
> /* allocate the CB chain - this also fills in the pointers */
> - d = bcm2835_dma_create_cb_chain(chan, DMA_MEM_TO_MEM, false,
> - info, extra, frames,
> - src, dst, len, 0, GFP_KERNEL);
> + d = bcm2835_dma_create_cb_chain(chan, DMA_MEM_TO_MEM, false, frames,
> + src, dst, len, 0, GFP_KERNEL, 0);
> if (!d)
> return NULL;
>
> @@ -714,8 +721,6 @@ static struct dma_async_tx_descriptor *bcm2835_dma_prep_slave_sg(
> struct bcm2835_chan *c = to_bcm2835_dma_chan(chan);
> struct bcm2835_desc *d;
> dma_addr_t src = 0, dst = 0;
> - u32 info = bcm2835_dma_prepare_cb_info(c, direction, false);
> - u32 extra = bcm2835_dma_prepare_cb_extra(c, direction, false, true, 0);
> size_t frames;
>
> if (!is_slave_direction(direction)) {
> @@ -738,10 +743,8 @@ static struct dma_async_tx_descriptor *bcm2835_dma_prep_slave_sg(
> frames = bcm2835_dma_count_frames_for_sg(c, sgl, sg_len);
>
> /* allocate the CB chain */
> - d = bcm2835_dma_create_cb_chain(chan, direction, false,
> - info, extra,
> - frames, src, dst, 0, 0,
> - GFP_NOWAIT);
> + d = bcm2835_dma_create_cb_chain(chan, direction, false, frames, src,
> + dst, 0, 0, GFP_NOWAIT, 0);
> if (!d)
> return NULL;
>
> @@ -757,13 +760,9 @@ static struct dma_async_tx_descriptor *bcm2835_dma_prep_dma_cyclic(
> size_t period_len, enum dma_transfer_direction direction,
> unsigned long flags)
> {
> - struct bcm2835_dmadev *od = to_bcm2835_dma_dev(chan->device);
> struct bcm2835_chan *c = to_bcm2835_dma_chan(chan);
> struct bcm2835_desc *d;
> dma_addr_t src, dst;
> - u32 info = bcm2835_dma_prepare_cb_info(c, direction,
> - buf_addr == od->zero_page);
> - u32 extra = bcm2835_dma_prepare_cb_extra(c, direction, true, true, 0);
> size_t max_len = bcm2835_dma_max_frame_length(c);
> size_t frames;
>
> @@ -814,10 +813,8 @@ static struct dma_async_tx_descriptor *bcm2835_dma_prep_dma_cyclic(
> * note that we need to use GFP_NOWAIT, as the ALSA i2s dmaengine
> * implementation calls prep_dma_cyclic with interrupts disabled.
> */
> - d = bcm2835_dma_create_cb_chain(chan, direction, true,
> - info, extra,
> - frames, src, dst, buf_len,
> - period_len, GFP_NOWAIT);
> + d = bcm2835_dma_create_cb_chain(chan, direction, true, frames, src, dst,
> + buf_len, period_len, GFP_NOWAIT, flags);
> if (!d)
> return NULL;
>
> --
> 2.34.1
>