| Message ID | FD897F46D140444CAB8DC80B08F0742B0185AC6ABB@PFDE-MX11.EU.P-F.BIZ |
|---|---|
| State | Changes Requested |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
| Series | spi: cadence_spi_apb: fix using bouncebuf with writeback dcache | expand |
On 11/15/2017 03:17 PM, Goldschmidt Simon wrote: > Bounce buffer may be used for CPU-only transfers (this is currently > the case for cadence_qspi). However, in this case, invalidating the > data cache might throw away copied data that is still in the cache > only. > > To make CPU-only transfers work with bouncebuf (but still take > advantage of having aligned buffers), a new flag 'GEN_BB_NODMA' is > introduced. If this flag is set, cache flushing/invalidation is > skipped and only the alignment part of bouncebuf is active. > > Signed-off-by: Simon Goldschmidt <sgoldschmidt@de.pepperl-fuchs.com> Why don't you just fix the cache operations in the driver ? This bounce-buffer for only CPU operations is just wasting CPU cycles and degrades performance and I just cannot find a good reason for it. > --- > common/bouncebuf.c | 9 +++++---- > include/bouncebuf.h | 9 +++++++++ > 2 files changed, 14 insertions(+), 4 deletions(-) > > diff --git a/common/bouncebuf.c b/common/bouncebuf.c > index 054d9e0302..0d18477f13 100644 > --- a/common/bouncebuf.c > +++ b/common/bouncebuf.c > @@ -55,16 +55,17 @@ int bounce_buffer_start(struct bounce_buffer *state, void *data, > * Flush data to RAM so DMA reads can pick it up, > * and any CPU writebacks don't race with DMA writes > */ > - flush_dcache_range((unsigned long)state->bounce_buffer, > - (unsigned long)(state->bounce_buffer) + > - state->len_aligned); > + if (!(state->flags & GEN_BB_NODMA)) > + flush_dcache_range((unsigned long)state->bounce_buffer, > + (unsigned long)(state->bounce_buffer) + > + state->len_aligned); > > return 0; > } > > int bounce_buffer_stop(struct bounce_buffer *state) > { > - if (state->flags & GEN_BB_WRITE) { > + if ((state->flags & (GEN_BB_WRITE|GEN_BB_NODMA)) == GEN_BB_WRITE) { > /* Invalidate cache so that CPU can see any newly DMA'd data */ > invalidate_dcache_range((unsigned long)state->bounce_buffer, > (unsigned long)(state->bounce_buffer) + > diff --git a/include/bouncebuf.h b/include/bouncebuf.h > index 5ffa99bc13..c6720b3b2e 100644 > --- a/include/bouncebuf.h > +++ b/include/bouncebuf.h > @@ -37,6 +37,15 @@ > */ > #define GEN_BB_RW (GEN_BB_READ | GEN_BB_WRITE) > > +/* > + * GEN_BB_NODMA -- Data is read/written by CPU only (no DMA), so no cache > + * flushing and invalidating is required. Not passing this for GEN_BB_WRITE > + * transfers done by the CPU (not DMA) may result in invalid data as the data > + * written into the cache is lost by invalidating the dcache after the transfer > + * (unless a writethrough cache is used). > + */ > +#define GEN_BB_NODMA (1 << 2) > + > struct bounce_buffer { > /* Copy of data parameter passed to start() */ > void *user_buffer; >
diff --git a/common/bouncebuf.c b/common/bouncebuf.c index 054d9e0302..0d18477f13 100644 --- a/common/bouncebuf.c +++ b/common/bouncebuf.c @@ -55,16 +55,17 @@ int bounce_buffer_start(struct bounce_buffer *state, void *data, * Flush data to RAM so DMA reads can pick it up, * and any CPU writebacks don't race with DMA writes */ - flush_dcache_range((unsigned long)state->bounce_buffer, - (unsigned long)(state->bounce_buffer) + - state->len_aligned); + if (!(state->flags & GEN_BB_NODMA)) + flush_dcache_range((unsigned long)state->bounce_buffer, + (unsigned long)(state->bounce_buffer) + + state->len_aligned); return 0; } int bounce_buffer_stop(struct bounce_buffer *state) { - if (state->flags & GEN_BB_WRITE) { + if ((state->flags & (GEN_BB_WRITE|GEN_BB_NODMA)) == GEN_BB_WRITE) { /* Invalidate cache so that CPU can see any newly DMA'd data */ invalidate_dcache_range((unsigned long)state->bounce_buffer, (unsigned long)(state->bounce_buffer) + diff --git a/include/bouncebuf.h b/include/bouncebuf.h index 5ffa99bc13..c6720b3b2e 100644 --- a/include/bouncebuf.h +++ b/include/bouncebuf.h @@ -37,6 +37,15 @@ */ #define GEN_BB_RW (GEN_BB_READ | GEN_BB_WRITE) +/* + * GEN_BB_NODMA -- Data is read/written by CPU only (no DMA), so no cache + * flushing and invalidating is required. Not passing this for GEN_BB_WRITE + * transfers done by the CPU (not DMA) may result in invalid data as the data + * written into the cache is lost by invalidating the dcache after the transfer + * (unless a writethrough cache is used). + */ +#define GEN_BB_NODMA (1 << 2) + struct bounce_buffer { /* Copy of data parameter passed to start() */ void *user_buffer;
Bounce buffer may be used for CPU-only transfers (this is currently the case for cadence_qspi). However, in this case, invalidating the data cache might throw away copied data that is still in the cache only. To make CPU-only transfers work with bouncebuf (but still take advantage of having aligned buffers), a new flag 'GEN_BB_NODMA' is introduced. If this flag is set, cache flushing/invalidation is skipped and only the alignment part of bouncebuf is active. Signed-off-by: Simon Goldschmidt <sgoldschmidt@de.pepperl-fuchs.com> --- common/bouncebuf.c | 9 +++++---- include/bouncebuf.h | 9 +++++++++ 2 files changed, 14 insertions(+), 4 deletions(-)