OMAP2+: DMA: fix src/dst position reporting
Subsystems (2)
v2 cover OMAP2+: DMA: fix src/dst position reporting
Hello,
Changes since v1:
- Move the check after the ERRATA_3_3 re-read test
If the user asks for the sDMA current position before the first data has been transmitted (before the first DMA request has been generated), the reported position is not valid:
src position: CSAC is uninitialized
dst position: CDAC is 0
The return values in both case considered invalid. This sitation can be identified by checking if the CDAC register is 0 (it is initialized to 0 in omap_dam_start call). In this case return the programmed source/destination address.
The affected omap_get_dma_src_pos/omap_get_dma_dst_pos functions are used by the audio stack mainly for checking the current position of the audio stream.
Regards,
Peter
---
Peter Ujfalusi (2):
OMAP2+: DMA: Workaround for invalid source position OMAP2+: DMA: Workaround for invalid destination position
arch/arm/plat-omap/dma.c | 22 +++++++++++++++++++++- 1 files changed, 21 insertions(+), 1 deletions(-)
Signature · 1 line
1.7.7.2
PATCH v2 1/2 OMAP2+: DMA: Workaround for invalid source position
If the DMA source position has been asked before the first actual data transfer has been done, the CSAC register does not contain valid information.
We can identify this situation by checking the CDAC register:
CDAC != 0 indicates that the DMA transfer on the channel has been started already.
When CDAC == 0 we can not trust the CSAC value since it has not been updated, and can contain random number. Return the start address in case the DMA has not jet started.
Note: The CDAC register has been initialized to 0 at dma_start time.
--- arch/arm/plat-omap/dma.c | 12 ++++++++++++ 1 files changed, 12 insertions(+), 0 deletions(-)
arch/arm/plat-omap/dma.c +12 -0
@@ -1034,6 +1034,18 @@ dma_addr_t omap_get_dma_src_pos(int lch)if (IS_DMA_ERRATA (DMA_ERRATA_3_3 ) && offset ==0 ) offset = p->dma_read (CSAC , lch); +if (!cpu_is_omap15xx ()) { +/* +* CDAC == 0 indicates that the DMA transfer on the channel has +* not been started (no data has been transferred so far). +* Return the programmed source start address in this case. +*/ +if (likely (p->dma_read (CDAC , lch))) + offset = p->dma_read (CSAC , lch); +else + offset = p->dma_read (CSSA , lch); + } +if (cpu_class_is_omap1 ()) offset |= (p->dma_read (CSSA , lch) &0xFFFF0000 );
Signature · 1 line
1.7.7.2
PATCH v2 1/2 OMAP2+: DMA: Workaround for invalid source position
Quoted 29 lines · Peter Ujfalusi
If the DMA source position has been asked before the first actual data transfer has been done, the CSAC register does not contain valid information.
We can identify this situation by checking the CDAC register:
CDAC != 0 indicates that the DMA transfer on the channel has been started already.
When CDAC == 0 we can not trust the CSAC value since it has not been updated, and can contain random number. Return the start address in case the DMA has not jet started.Note: The CDAC register has been initialized to 0 at
dma_starttime.Signed-off-by Peter Ujfalusi<[email protected]>--- arch/arm/plat-omap/dma.c | 12 ++++++++++++ 1 files changed, 12 insertions(+), 0 deletions(-)arch/arm/plat-omap/dma.c +2
-0@@ -1034,6 +1034,18 @@ dma_addr_t omap_get_dma_src_pos(int lch)if (IS_DMA_ERRATA (DMA_ERRATA_3_3 )&& offset ==0 ) offset = p->dma_read (CSAC , lch); +if (!cpu_is_omap15xx ()) { +/*
arch/arm/plat-omap/dma.c dma_addr_t omap_get_dma_src_pos(int lch) · line 1048
Quoted 10 lines
arch/arm/plat-omap/dma.c +10
-0+* CDAC == 0 indicates that the DMA transfer on the channel has +* not been started (no data has been transferred so far). +* Return the programmed source start address in this case. +*/ +if (likely (p->dma_read (CDAC , lch))) + offset = p->dma_read (CSAC , lch); +else + offset = p->dma_read (CSSA , lch); + } +
I think this is enough:
if (unlikely(p->dma_read(CDAC, lch) == 0))
offset = p->dma_read(CSSA, lch);
I suppose offset is ok for normal case as it is already read (twise) above.
Signature · 1 line
Jarkko
PATCH v2 1/2 OMAP2+: DMA: Workaround for invalid source position
Quoted 43 lines · Jarkko Nikula
Quoted 29 lines · Peter Ujfalusi
If the DMA source position has been asked before the first actual data transfer has been done, the CSAC register does not contain valid information.
We can identify this situation by checking the CDAC register:
CDAC != 0 indicates that the DMA transfer on the channel has been started already.
When CDAC == 0 we can not trust the CSAC value since it has not been updated, and can contain random number. Return the start address in case the DMA has not jet started.Note: The CDAC register has been initialized to 0 at
dma_starttime.Signed-off-by Peter Ujfalusi<[email protected]>---
arch/arm/plat-omap/dma.c| 12 ++++++++++++1 files changed, 12 insertions(+), 0 deletions(-)arch/arm/plat-omap/dma.c +2
-0@@ -1034,6 +1034,18 @@ dma_addr_t omap_get_dma_src_pos(int lch)if (IS_DMA_ERRATA (DMA_ERRATA_3_3 )&& offset ==0 ) offset = p->dma_read (CSAC , lch); +if (!cpu_is_omap15xx ()) { +/* arch/arm/plat-omap/dma.c dma_addr_t omap_get_dma_src_pos(int lch) · line 1048
Quoted 10 lines
arch/arm/plat-omap/dma.c +10
-0+* CDAC == 0 indicates that the DMA transfer on the channel has +* not been started (no data has been transferred so far). +* Return the programmed source start address in this case. +*/ +if (likely (p->dma_read (CDAC , lch))) + offset = p->dma_read (CSAC , lch); +else + offset = p->dma_read (CSSA , lch); + } +I think this is enough:
Quoted 5 lines
if (
unlikely(p->dma_read(CDAC, lch) == 0))
offset =p->dma_read(CSSA, lch);I suppose offset is ok for normal case as it is already read (twise) above.
Or actually my proposal could have a race if CDAC changes between CSAC read and CDAC read. In that case it's better to re-read CSAC as your patch does after CDAC test and give to both:
PATCH v2 1/2 OMAP2+: DMA: Workaround for invalid source position
Quoted 47 lines · On Thursday 10 November 2011 15:02:04 Jarkko Nikula wrote:
Quoted 46 lines · Jarkko Nikula
Quoted 29 lines · Peter Ujfalusi
If the DMA source position has been asked before the first actual data transfer has been done, the CSAC register does not contain valid information.
We can identify this situation by checking the CDAC register:
CDAC != 0 indicates that the DMA transfer on the channel has been started already.
When CDAC == 0 we can not trust the CSAC value since it has not been updated, and can contain random number. Return the start address in case the DMA has not jet started.Note: The CDAC register has been initialized to 0 at
dma_starttime.Signed-off-by Peter Ujfalusi<[email protected]>---
arch/arm/plat-omap/dma.c| 12 ++++++++++++1 files changed, 12 insertions(+), 0 deletions(-)arch/arm/plat-omap/dma.c +2
-0@@ -1034,6 +1034,18 @@ dma_addr_t omap_get_dma_src_pos(int lch)if (IS_DMA_ERRATA (DMA_ERRATA_3_3 )&& offset ==0 ) offset = p->dma_read (CSAC , lch); +if (!cpu_is_omap15xx ()) { +/* arch/arm/plat-omap/dma.c dma_addr_t omap_get_dma_src_pos(int lch) · line 1048
Quoted 10 lines
arch/arm/plat-omap/dma.c +10
-0+* CDAC == 0 indicates that the DMA transfer on the channel has +* not been started (no data has been transferred so far). +* Return the programmed source start address in this case. +*/ +if (likely (p->dma_read (CDAC , lch))) + offset = p->dma_read (CSAC , lch); +else + offset = p->dma_read (CSSA , lch); + } +I think this is enough:
if (
unlikely(p->dma_read(CDAC, lch) == 0))
offset =p->dma_read(CSSA, lch);
Quoted 7 lines
Quoted 2 lines
I suppose offset is ok for normal case as it is already read (twise) above.
Or actually my proposal could have a race if CDAC changes between CSAC read and CDAC read. In that case it's better to re-read CSAC as your patch does after CDAC test and give to both:
Reviewed-by Jarkko Nikula <[email protected]>
Tony, have you taken this patch? I failed to find it in the l-o tree...
--
Péter
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v2 1/2 OMAP2+: DMA: Workaround for invalid source position
Quoted 11 lines · P?ter Ujfalusi
Quoted 10 lines · On Thursday 10 November 2011 15:02:04 Jarkko Nikula wrote:
Quoted 9 lines · Jarkko Nikula
On 11/07/2011 11:33 AM, Peter Ujfalusi wrote:
I think this is enough:
if (
unlikely(p->dma_read(CDAC, lch) == 0))
offset =p->dma_read(CSSA, lch);I suppose offset is ok for normal case as it is already read (twise) above.
Quoted 7 lines
Quoted 5 lines
Or actually my proposal could have a race if CDAC changes between CSAC read and CDAC read. In that case it's better to re-read CSAC as your patch does after CDAC test and give to both:
Reviewed-by Jarkko Nikula <[email protected]>Tony, have you taken this patch? I failed to find it in the l-o tree...
Sorry for the delay, applying both into fixes-non-critical.
Tony
PATCH v2 2/2 OMAP2+: DMA: Workaround for invalid destination position
In reply to Peter UjfalusiIf the DMA destination position has been asked before the first actual data transfer has been done, the CDAC register still contains 0 (it is initialized to 0 at omsp_dma_start).
If CDAC == 0, return the programmed start address.
--- arch/arm/plat-omap/dma.c | 10 +++++++++- 1 files changed, 9 insertions(+), 1 deletions(-)
arch/arm/plat-omap/dma.c +9 -1
@@ -1074,8 +1074,16 @@ dma_addr_t omap_get_dma_dst_pos(int lch)* omap 3.2/3.3 erratum: sometimes 0 is returned if CSAC/CDAC is * read before the DMA controller finished disabling the channel. */ -if (!cpu_is_omap15xx () && offset ==0 ) +if (!cpu_is_omap15xx () && offset ==0 ) { offset = p->dma_read (CDAC , lch); +/* +* CDAC == 0 indicates that the DMA transfer on the channel has +* not been started (no data has been transferred so far). +* Return the programmed destination start address in this case. +*/ +if (unlikely (!offset)) + offset = p->dma_read (CDSA , lch); + }if (cpu_class_is_omap1 ()) offset |= (p->dma_read (CDSA , lch) &0xFFFF0000 );
Signature · 1 line
1.7.7.2