Patchwork [U-Boot,2/3] common: rework bouncebuf implementation

login
register
mail settings
Submitter Stephen Warren
Date Nov. 5, 2012, 11:04 p.m.
Message ID <1352156642-7975-2-git-send-email-swarren@wwwdotorg.org>
Download mbox | patch
Permalink /patch/197358/
State Superseded
Headers show

Comments

Stephen Warren - Nov. 5, 2012, 11:04 p.m.
From: Stephen Warren <swarren@nvidia.com>

The current bouncebuf API requires all parameters to be passed to both
bounce_buffer_start() and bounce_buffer_stop(). This works fine when
both functions are called from the same place. However, Tegra's MMC
driver splits the data setup and post-processing steps between two
functions, and passing all that state around separately would be painful.
Modify the bouncebuf API to accept a state structure as a parameter, so
that only a single struct needs to be passed to both APIs.

Also, don't modify the data pointer, but rather store the temporary
buffer in this state struct. This also avoids passing the buffer pointer
all over the place. The bouncebuf code ensures that client code can
always use a single buffer pointer in the state structure, irrespective
of whether a bounce buffer actually had to be allocated.

Finally, store the aligned/rounded length in the state structure too, so
that client code doesn't need to duplicate the size alignment/rounding
code, and hence have to guess at the value it was aligned/rounded to.

Update the MXS MMC driver for this change.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 common/bouncebuf.c   |   41 ++++++++++++++++-------------------------
 drivers/mmc/mxsmmc.c |   25 ++++++++++++++-----------
 include/bouncebuf.h  |   36 +++++++++++++++++++++++-------------
 3 files changed, 53 insertions(+), 49 deletions(-)
Simon Glass - Nov. 5, 2012, 11:54 p.m.
Hi Stephen,

On Mon, Nov 5, 2012 at 3:04 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> From: Stephen Warren <swarren@nvidia.com>
>
> The current bouncebuf API requires all parameters to be passed to both
> bounce_buffer_start() and bounce_buffer_stop(). This works fine when
> both functions are called from the same place. However, Tegra's MMC
> driver splits the data setup and post-processing steps between two
> functions, and passing all that state around separately would be painful.
> Modify the bouncebuf API to accept a state structure as a parameter, so
> that only a single struct needs to be passed to both APIs.
>
> Also, don't modify the data pointer, but rather store the temporary
> buffer in this state struct. This also avoids passing the buffer pointer
> all over the place. The bouncebuf code ensures that client code can
> always use a single buffer pointer in the state structure, irrespective
> of whether a bounce buffer actually had to be allocated.
>
> Finally, store the aligned/rounded length in the state structure too, so
> that client code doesn't need to duplicate the size alignment/rounding
> code, and hence have to guess at the value it was aligned/rounded to.
>
> Update the MXS MMC driver for this change.

I missed this API when it came through. I think your changes improve it a lot.

>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
>  common/bouncebuf.c   |   41 ++++++++++++++++-------------------------
>  drivers/mmc/mxsmmc.c |   25 ++++++++++++++-----------
>  include/bouncebuf.h  |   36 +++++++++++++++++++++++-------------
>  3 files changed, 53 insertions(+), 49 deletions(-)
>
> diff --git a/common/bouncebuf.c b/common/bouncebuf.c
> index ffd3c90..210d70d 100644
> --- a/common/bouncebuf.c
> +++ b/common/bouncebuf.c
> @@ -50,44 +50,35 @@ static int addr_aligned(void *data, size_t len)
>         return 1;
>  }
>
> -int bounce_buffer_start(void **data, size_t len, void **backup, uint8_t flags)
> +int bounce_buffer_start(struct bounce_buffer_state *state, void *data,
> +                       size_t len, uint8_t flags)
>  {
> -       void *tmp;
> -       size_t alen;
> +       state->user_buffer = data;
> +       state->bounce_buffer = data;
> +       state->len = len;
> +       state->len_aligned = roundup(len, ARCH_DMA_MINALIGN);
> +       state->flags = flags;
>
> -       if (addr_aligned(*data, len)) {
> -               *backup = NULL;
> +       if (addr_aligned(data, len))

Maybe consider checking for data == NULL here, and return 0. This
would allow you to remove your 'if (data)' checks in the tegra driver.
Would need to update function description in the header file though.

>                 return 0;
> -       }
> -
> -       alen = roundup(len, ARCH_DMA_MINALIGN);
> -       tmp = memalign(ARCH_DMA_MINALIGN, alen);
>
> -       if (!tmp)
> +       state->bounce_buffer = memalign(ARCH_DMA_MINALIGN, state->len_aligned);
> +       if (!state->bounce_buffer)
>                 return -ENOMEM;
>
> -       if (flags & GEN_BB_READ)
> -               memcpy(tmp, *data, len);
> -
> -       *backup = *data;
> -       *data = tmp;
> +       if (state->flags & GEN_BB_READ)
> +               memcpy(state->bounce_buffer, state->user_buffer, state->len);

I wonder if the dcache handling could be done here (and in the
memcpy() below), perhaps under the control of a new flag. After the
cache code in both drivers seems very similar.

>
>         return 0;
>  }
>
> -int bounce_buffer_stop(void **data, size_t len, void **backup, uint8_t flags)
> +int bounce_buffer_stop(struct bounce_buffer_state *state)
>  {
> -       void *tmp = *data;
> -
> -       /* The buffer was already aligned, since "backup" is NULL. */
> -       if (!*backup)
> +       if (state->bounce_buffer == state->user_buffer)
>                 return 0;
>
> -       if (flags & GEN_BB_WRITE)
> -               memcpy(*backup, *data, len);
> -
> -       *data = *backup;
> -       free(tmp);

Don't you need to keep the free()?

> +       if (state->flags & GEN_BB_WRITE)
> +               memcpy(state->user_buffer, state->bounce_buffer, state->len);
>
>         return 0;
>  }
> diff --git a/drivers/mmc/mxsmmc.c b/drivers/mmc/mxsmmc.c
> index 109acbf..d314d7d 100644
> --- a/drivers/mmc/mxsmmc.c
> +++ b/drivers/mmc/mxsmmc.c
> @@ -96,11 +96,11 @@ static int mxsmmc_send_cmd_pio(struct mxsmmc_priv *priv, struct mmc_data *data)
>  static int mxsmmc_send_cmd_dma(struct mxsmmc_priv *priv, struct mmc_data *data)
>  {
>         uint32_t data_count = data->blocksize * data->blocks;
> -       uint32_t cache_data_count = roundup(data_count, ARCH_DMA_MINALIGN);
>         int dmach;
>         struct mxs_dma_desc *desc = priv->desc;
> -       void *addr, *backup;
> +       void *addr;
>         uint8_t flags;
> +       struct bounce_buffer_state bbstate;
>
>         memset(desc, 0, sizeof(struct mxs_dma_desc));
>         desc->address = (dma_addr_t)desc;
> @@ -115,19 +115,21 @@ static int mxsmmc_send_cmd_dma(struct mxsmmc_priv *priv, struct mmc_data *data)
>                 flags = GEN_BB_READ;
>         }
>
> -       bounce_buffer_start(&addr, data_count, &backup, flags);
> +       bounce_buffer_start(&bbstate, addr, data_count, flags);
>
> -       priv->desc->cmd.address = (dma_addr_t)addr;
> +       priv->desc->cmd.address = (dma_addr_t)bbstate.bounce_buffer;
>
>         if (data->flags & MMC_DATA_WRITE) {
>                 /* Flush data to DRAM so DMA can pick them up */
> -               flush_dcache_range((uint32_t)addr,
> -                       (uint32_t)(addr) + cache_data_count);
> +               flush_dcache_range((uint32_t)bbstate.bounce_buffer,
> +                                       (uint32_t)(bbstate.bounce_buffer) +
> +                                               bbstate.len_aligned);
>         }
>
>         /* Invalidate the area, so no writeback into the RAM races with DMA */
>         invalidate_dcache_range((uint32_t)priv->desc->cmd.address,
> -                       (uint32_t)(priv->desc->cmd.address + cache_data_count));
> +                               (uint32_t)(priv->desc->cmd.address +
> +                                       bbstate.len_aligned));
>
>         priv->desc->cmd.data |= MXS_DMA_DESC_IRQ | MXS_DMA_DESC_DEC_SEM |
>                                 (data_count << MXS_DMA_DESC_BYTES_OFFSET);
> @@ -135,17 +137,18 @@ static int mxsmmc_send_cmd_dma(struct mxsmmc_priv *priv, struct mmc_data *data)
>         dmach = MXS_DMA_CHANNEL_AHB_APBH_SSP0 + priv->id;
>         mxs_dma_desc_append(dmach, priv->desc);
>         if (mxs_dma_go(dmach)) {
> -               bounce_buffer_stop(&addr, data_count, &backup, flags);
> +               bounce_buffer_stop(&bbstate);
>                 return COMM_ERR;
>         }
>
>         /* The data arrived into DRAM, invalidate cache over them */
>         if (data->flags & MMC_DATA_READ) {
> -               invalidate_dcache_range((uint32_t)addr,
> -                       (uint32_t)(addr) + cache_data_count);
> +               invalidate_dcache_range((uint32_t)bbstate.bounce_buffer,
> +                                       (uint32_t)(bbstate.bounce_buffer) +
> +                                               bbstate.len_aligned);
>         }
>
> -       bounce_buffer_stop(&addr, data_count, &backup, flags);
> +       bounce_buffer_stop(&bbstate);
>
>         return 0;
>  }
> diff --git a/include/bouncebuf.h b/include/bouncebuf.h
> index 31021c5..205a1ed 100644
> --- a/include/bouncebuf.h
> +++ b/include/bouncebuf.h
> @@ -52,33 +52,43 @@
>  #define GEN_BB_RW      (GEN_BB_READ | GEN_BB_WRITE)
>
>  #ifdef CONFIG_BOUNCE_BUFFER
> +struct bounce_buffer_state {
> +       void *user_buffer;
> +       void *bounce_buffer;
> +       size_t len;
> +       size_t len_aligned;
> +       uint8_t flags;

Would struct bounce_buffer be better?

Perhaps add member comments?

> +};
> +
>  /**
>   * bounce_buffer_start() -- Start the bounce buffer session
> + * state:      stores state passed between bounce_buffer_{start,stop}
>   * data:       pointer to buffer to be aligned
>   * len:                length of the buffer
> - * backup:     pointer to backup buffer (the original value is stored here if
> - *              needed
>   * flags:      flags describing the transaction, see above.
>   */
> -int bounce_buffer_start(void **data, size_t len, void **backup, uint8_t flags);
> +int bounce_buffer_start(struct bounce_buffer_state *state, void *data,
> +                       size_t len, uint8_t flags);

I would argue that a uint8_t parameter doesn't make a lot of sense.
Why not just unsigned int?

>  /**
>   * bounce_buffer_stop() -- Finish the bounce buffer session
> - * data:       pointer to buffer that was aligned
> - * len:                length of the buffer
> - * backup:     pointer to backup buffer (the original value is stored here if
> - *              needed
> - * flags:      flags describing the transaction, see above.
> + * state:      stores state passed between bounce_buffer_{start,stop}
>   */
> -int bounce_buffer_stop(void **data, size_t len, void **backup, uint8_t flags);
> +int bounce_buffer_stop(struct bounce_buffer_state *state);
>  #else
> -static inline int bounce_buffer_start(void **data, size_t len, void **backup,
> -                                       uint8_t flags)
> +struct bounce_buffer_state {
> +       void *bounce_buffer;
> +       size_t len_aligned;
> +};
> +
> +static inline int bounce_buffer_start(struct bounce_buffer_state *state,
> +                                       void *data, size_t len, uint8_t flags)
>  {
> +       state->bounce_buffer = data;
> +       state->len_aligned = len;

Why do you need to do this if it is just a null implementation?
Perhaps add a comment.

>         return 0;
>  }
>
> -static inline int bounce_buffer_stop(void **data, size_t len, void **backup,
> -                                       uint8_t flags)
> +static inline int bounce_buffer_stop(struct bounce_buffer_state *state)
>  {
>         return 0;
>  }
> --
> 1.7.0.4
>

Regards,
Simon
Stephen Warren - Nov. 6, 2012, 6:44 p.m.
On 11/05/2012 04:54 PM, Simon Glass wrote:
> Hi Stephen,
> 
> On Mon, Nov 5, 2012 at 3:04 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> The current bouncebuf API requires all parameters to be passed to both
>> bounce_buffer_start() and bounce_buffer_stop(). This works fine when
>> both functions are called from the same place. However, Tegra's MMC
>> driver splits the data setup and post-processing steps between two
>> functions, and passing all that state around separately would be painful.
>> Modify the bouncebuf API to accept a state structure as a parameter, so
>> that only a single struct needs to be passed to both APIs.
...
> I wonder if the dcache handling could be done here (and in the
> memcpy() below), perhaps under the control of a new flag. After the
> cache code in both drivers seems very similar.

Yes, that's a good idea. It cross my mind before, but I forgot to follow
up on it and realize that the cache management APIs are actually common,
not CPU-specific.

One question here. The MXS MMC driver contains:

> 	if (data->flags & MMC_DATA_WRITE) {
> 		/* Flush data to DRAM so DMA can pick them up */
> 		flush_dcache_range((uint32_t)bbstate.bounce_buffer,
> 					(uint32_t)(bbstate.bounce_buffer) +
> 						bbstate.len_aligned);
> 	}
> 
> 	/* Invalidate the area, so no writeback into the RAM races with DMA */
> 	invalidate_dcache_range((uint32_t)priv->desc->cmd.address,
> 				(uint32_t)(priv->desc->cmd.address +
> 					bbstate.len_aligned));

It the invalidate_dcache_range() really necessary here? I would assume
that the flush_dcache_range() call marks the cache non-dirty, so there
can no longer be any write-backs to race with the DMA. Certainly, the
Tegra MMC driver doesn't include that invalidate call and appears to
work fine.

I agree with all your comments that I haven't quoted here, and will go
implement them.

>> -static inline int bounce_buffer_start(void **data, size_t len, void **backup,
>> -                                       uint8_t flags)
>> +struct bounce_buffer_state {
>> +       void *bounce_buffer;
>> +       size_t len_aligned;
>> +};
>> +
>> +static inline int bounce_buffer_start(struct bounce_buffer_state *state,
>> +                                       void *data, size_t len, uint8_t flags)
>>  {
>> +       state->bounce_buffer = data;
>> +       state->len_aligned = len;
> 
> Why do you need to do this if it is just a null implementation?
> Perhaps add a comment.

I wondered whether we should simply remove the dummy implementations
completely; it seems that if any MMC driver needs bouncebuf ever, it
always needs it. Hence, there should never be a case where these APIs
are called/referenced, yet CONFIG_BOUNCE_BUFFER is not set. However,
there is such a case right now; sc_sps_1.h enables the MXS MMC driver
but not the bounce buffer. Perhaps I should just send a patch to fix
that, and remove these dummy functions.

Aside from that, the reason these dummy functions need to set fields in
*state right now is that the MXS MMC driver unconditionally accesses
those fields, so they need to exist and contain valid data.
Stephen Warren - Nov. 6, 2012, 7:30 p.m.
On 11/05/2012 04:54 PM, Simon Glass wrote:
> Hi Stephen,
> 
> On Mon, Nov 5, 2012 at 3:04 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> From: Stephen Warren <swarren@nvidia.com>
>>
>> The current bouncebuf API requires all parameters to be passed to both
>> bounce_buffer_start() and bounce_buffer_stop(). This works fine when
>> both functions are called from the same place. However, Tegra's MMC
>> driver splits the data setup and post-processing steps between two
>> functions, and passing all that state around separately would be painful.
>> Modify the bouncebuf API to accept a state structure as a parameter, so
>> that only a single struct needs to be passed to both APIs.

>> diff --git a/common/bouncebuf.c b/common/bouncebuf.c

>> -int bounce_buffer_start(void **data, size_t len, void **backup, uint8_t flags)
>> +int bounce_buffer_start(struct bounce_buffer_state *state, void *data,
>> +                       size_t len, uint8_t flags)
>>  {
>> -       void *tmp;
>> -       size_t alen;
>> +       state->user_buffer = data;
>> +       state->bounce_buffer = data;
>> +       state->len = len;
>> +       state->len_aligned = roundup(len, ARCH_DMA_MINALIGN);
>> +       state->flags = flags;
>>
>> -       if (addr_aligned(*data, len)) {
>> -               *backup = NULL;
>> +       if (addr_aligned(data, len))
> 
> Maybe consider checking for data == NULL here, and return 0. This
> would allow you to remove your 'if (data)' checks in the tegra driver.
> Would need to update function description in the header file though.

That doesn't actually work out. The if (data) test in the Tegra driver
is checking whether a non-NULL "struct mmc_data *data" is passed to
Tegra's mmc_send_cmd(). That value isn't directly passed to
bounce_buffer_start(), but rather data->{src,dest}, which doesn't even
exist if (!data).

Patch

diff --git a/common/bouncebuf.c b/common/bouncebuf.c
index ffd3c90..210d70d 100644
--- a/common/bouncebuf.c
+++ b/common/bouncebuf.c
@@ -50,44 +50,35 @@  static int addr_aligned(void *data, size_t len)
 	return 1;
 }
 
-int bounce_buffer_start(void **data, size_t len, void **backup, uint8_t flags)
+int bounce_buffer_start(struct bounce_buffer_state *state, void *data,
+			size_t len, uint8_t flags)
 {
-	void *tmp;
-	size_t alen;
+	state->user_buffer = data;
+	state->bounce_buffer = data;
+	state->len = len;
+	state->len_aligned = roundup(len, ARCH_DMA_MINALIGN);
+	state->flags = flags;
 
-	if (addr_aligned(*data, len)) {
-		*backup = NULL;
+	if (addr_aligned(data, len))
 		return 0;
-	}
-
-	alen = roundup(len, ARCH_DMA_MINALIGN);
-	tmp = memalign(ARCH_DMA_MINALIGN, alen);
 
-	if (!tmp)
+	state->bounce_buffer = memalign(ARCH_DMA_MINALIGN, state->len_aligned);
+	if (!state->bounce_buffer)
 		return -ENOMEM;
 
-	if (flags & GEN_BB_READ)
-		memcpy(tmp, *data, len);
-
-	*backup = *data;
-	*data = tmp;
+	if (state->flags & GEN_BB_READ)
+		memcpy(state->bounce_buffer, state->user_buffer, state->len);
 
 	return 0;
 }
 
-int bounce_buffer_stop(void **data, size_t len, void **backup, uint8_t flags)
+int bounce_buffer_stop(struct bounce_buffer_state *state)
 {
-	void *tmp = *data;
-
-	/* The buffer was already aligned, since "backup" is NULL. */
-	if (!*backup)
+	if (state->bounce_buffer == state->user_buffer)
 		return 0;
 
-	if (flags & GEN_BB_WRITE)
-		memcpy(*backup, *data, len);
-
-	*data = *backup;
-	free(tmp);
+	if (state->flags & GEN_BB_WRITE)
+		memcpy(state->user_buffer, state->bounce_buffer, state->len);
 
 	return 0;
 }
diff --git a/drivers/mmc/mxsmmc.c b/drivers/mmc/mxsmmc.c
index 109acbf..d314d7d 100644
--- a/drivers/mmc/mxsmmc.c
+++ b/drivers/mmc/mxsmmc.c
@@ -96,11 +96,11 @@  static int mxsmmc_send_cmd_pio(struct mxsmmc_priv *priv, struct mmc_data *data)
 static int mxsmmc_send_cmd_dma(struct mxsmmc_priv *priv, struct mmc_data *data)
 {
 	uint32_t data_count = data->blocksize * data->blocks;
-	uint32_t cache_data_count = roundup(data_count, ARCH_DMA_MINALIGN);
 	int dmach;
 	struct mxs_dma_desc *desc = priv->desc;
-	void *addr, *backup;
+	void *addr;
 	uint8_t flags;
+	struct bounce_buffer_state bbstate;
 
 	memset(desc, 0, sizeof(struct mxs_dma_desc));
 	desc->address = (dma_addr_t)desc;
@@ -115,19 +115,21 @@  static int mxsmmc_send_cmd_dma(struct mxsmmc_priv *priv, struct mmc_data *data)
 		flags = GEN_BB_READ;
 	}
 
-	bounce_buffer_start(&addr, data_count, &backup, flags);
+	bounce_buffer_start(&bbstate, addr, data_count, flags);
 
-	priv->desc->cmd.address = (dma_addr_t)addr;
+	priv->desc->cmd.address = (dma_addr_t)bbstate.bounce_buffer;
 
 	if (data->flags & MMC_DATA_WRITE) {
 		/* Flush data to DRAM so DMA can pick them up */
-		flush_dcache_range((uint32_t)addr,
-			(uint32_t)(addr) + cache_data_count);
+		flush_dcache_range((uint32_t)bbstate.bounce_buffer,
+					(uint32_t)(bbstate.bounce_buffer) +
+						bbstate.len_aligned);
 	}
 
 	/* Invalidate the area, so no writeback into the RAM races with DMA */
 	invalidate_dcache_range((uint32_t)priv->desc->cmd.address,
-			(uint32_t)(priv->desc->cmd.address + cache_data_count));
+				(uint32_t)(priv->desc->cmd.address +
+					bbstate.len_aligned));
 
 	priv->desc->cmd.data |= MXS_DMA_DESC_IRQ | MXS_DMA_DESC_DEC_SEM |
 				(data_count << MXS_DMA_DESC_BYTES_OFFSET);
@@ -135,17 +137,18 @@  static int mxsmmc_send_cmd_dma(struct mxsmmc_priv *priv, struct mmc_data *data)
 	dmach = MXS_DMA_CHANNEL_AHB_APBH_SSP0 + priv->id;
 	mxs_dma_desc_append(dmach, priv->desc);
 	if (mxs_dma_go(dmach)) {
-		bounce_buffer_stop(&addr, data_count, &backup, flags);
+		bounce_buffer_stop(&bbstate);
 		return COMM_ERR;
 	}
 
 	/* The data arrived into DRAM, invalidate cache over them */
 	if (data->flags & MMC_DATA_READ) {
-		invalidate_dcache_range((uint32_t)addr,
-			(uint32_t)(addr) + cache_data_count);
+		invalidate_dcache_range((uint32_t)bbstate.bounce_buffer,
+					(uint32_t)(bbstate.bounce_buffer) +
+						bbstate.len_aligned);
 	}
 
-	bounce_buffer_stop(&addr, data_count, &backup, flags);
+	bounce_buffer_stop(&bbstate);
 
 	return 0;
 }
diff --git a/include/bouncebuf.h b/include/bouncebuf.h
index 31021c5..205a1ed 100644
--- a/include/bouncebuf.h
+++ b/include/bouncebuf.h
@@ -52,33 +52,43 @@ 
 #define GEN_BB_RW	(GEN_BB_READ | GEN_BB_WRITE)
 
 #ifdef CONFIG_BOUNCE_BUFFER
+struct bounce_buffer_state {
+	void *user_buffer;
+	void *bounce_buffer;
+	size_t len;
+	size_t len_aligned;
+	uint8_t flags;
+};
+
 /**
  * bounce_buffer_start() -- Start the bounce buffer session
+ * state:	stores state passed between bounce_buffer_{start,stop}
  * data:	pointer to buffer to be aligned
  * len:		length of the buffer
- * backup:	pointer to backup buffer (the original value is stored here if
- *              needed
  * flags:	flags describing the transaction, see above.
  */
-int bounce_buffer_start(void **data, size_t len, void **backup, uint8_t flags);
+int bounce_buffer_start(struct bounce_buffer_state *state, void *data,
+			size_t len, uint8_t flags);
 /**
  * bounce_buffer_stop() -- Finish the bounce buffer session
- * data:	pointer to buffer that was aligned
- * len:		length of the buffer
- * backup:	pointer to backup buffer (the original value is stored here if
- *              needed
- * flags:	flags describing the transaction, see above.
+ * state:	stores state passed between bounce_buffer_{start,stop}
  */
-int bounce_buffer_stop(void **data, size_t len, void **backup, uint8_t flags);
+int bounce_buffer_stop(struct bounce_buffer_state *state);
 #else
-static inline int bounce_buffer_start(void **data, size_t len, void **backup,
-					uint8_t flags)
+struct bounce_buffer_state {
+	void *bounce_buffer;
+	size_t len_aligned;
+};
+
+static inline int bounce_buffer_start(struct bounce_buffer_state *state,
+					void *data, size_t len, uint8_t flags)
 {
+	state->bounce_buffer = data;
+	state->len_aligned = len;
 	return 0;
 }
 
-static inline int bounce_buffer_stop(void **data, size_t len, void **backup,
-					uint8_t flags)
+static inline int bounce_buffer_stop(struct bounce_buffer_state *state)
 {
 	return 0;
 }