[PATCH 0/2] OMAP2+: DMA: fix src/dst position reporting

[PATCH 0/2] OMAP2+: DMA: fix src/dst position reporting

[Peter Ujfalusi]

Hello,

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 |   25 ++++++++++++++++++++++---
 1 files changed, 22 insertions(+), 3 deletions(-)

-- 
1.7.7.1

[PATCH 1/2] OMAP2+: DMA: Workaround for invalid source position

[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_start
time.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 arch/arm/plat-omap/dma.c |   15 +++++++++++++--
 1 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/arch/arm/plat-omap/dma.c b/arch/arm/plat-omap/dma.c
index c22217c..38b0d44 100644
--- a/arch/arm/plat-omap/dma.c
+++ b/arch/arm/plat-omap/dma.c
@@ -1024,12 +1024,23 @@ EXPORT_SYMBOL(omap_set_dma_callback);
  */
 dma_addr_t omap_get_dma_src_pos(int lch)
 {
+	u32 cdac;
 	dma_addr_t offset = 0;
 
 	if (cpu_is_omap15xx())
 		offset = p->dma_read(CPC, lch);
-	else
-		offset = p->dma_read(CSAC, lch);
+	else {
+		/*
+		 * 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.
+		 */
+		cdac = p->dma_read(CDAC, lch);
+		if (likely(cdac))
+			offset = p->dma_read(CSAC, lch);
+		else
+			offset = p->dma_read(CSSA, lch);
+	}
 
 	if (IS_DMA_ERRATA(DMA_ERRATA_3_3) && offset == 0)
 		offset = p->dma_read(CSAC, lch);
-- 
1.7.7.1

[PATCH 2/2] OMAP2+: DMA: Workaround for invalid destination position

[Peter Ujfalusi]

If 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.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 arch/arm/plat-omap/dma.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/arch/arm/plat-omap/dma.c b/arch/arm/plat-omap/dma.c
index 38b0d44..9186491 100644
--- a/arch/arm/plat-omap/dma.c
+++ b/arch/arm/plat-omap/dma.c
@@ -1066,8 +1066,16 @@ dma_addr_t omap_get_dma_dst_pos(int lch)
 
 	if (cpu_is_omap15xx())
 		offset = p->dma_read(CPC, lch);
-	else
+	else {
 		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);
+	}
 
 	/*
 	 * omap 3.2/3.3 erratum: sometimes 0 is returned if CSAC/CDAC is
-- 
1.7.7.1

Re: [PATCH 0/2] OMAP2+: DMA: fix src/dst position reporting

[Jarkko Nikula]

On 10/31/2011 04:20 PM, Peter Ujfalusi wrote:
> Hello,
>
> 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 |   25 ++++++++++++++++++++++---
>   1 files changed, 22 insertions(+), 3 deletions(-)
>
Both,

Reviewed-by: Jarkko Nikula <jarkko.nikula@bitmer.com>

Re: [PATCH 1/2] OMAP2+: DMA: Workaround for invalid source position

[Tony Lindgren]

* Peter Ujfalusi <peter.ujfalusi@ti.com> [111031 06:46]:
> 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.
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> ---
>  arch/arm/plat-omap/dma.c |   15 +++++++++++++--
>  1 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/dma.c b/arch/arm/plat-omap/dma.c
> index c22217c..38b0d44 100644
> --- a/arch/arm/plat-omap/dma.c
> +++ b/arch/arm/plat-omap/dma.c
> @@ -1024,12 +1024,23 @@ EXPORT_SYMBOL(omap_set_dma_callback);
>   */
>  dma_addr_t omap_get_dma_src_pos(int lch)
>  {
> +	u32 cdac;
>  	dma_addr_t offset = 0;
>  
>  	if (cpu_is_omap15xx())
>  		offset = p->dma_read(CPC, lch);
> -	else
> -		offset = p->dma_read(CSAC, lch);
> +	else {
> +		/*
> +		 * 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.
> +		 */
> +		cdac = p->dma_read(CDAC, lch);
> +		if (likely(cdac))
> +			offset = p->dma_read(CSAC, lch);
> +		else
> +			offset = p->dma_read(CSSA, lch);
> +	}
>  
>  	if (IS_DMA_ERRATA(DMA_ERRATA_3_3) && offset == 0)
>  		offset = p->dma_read(CSAC, lch);

Should these tests be done only after the errata re-read for both
src and dst patches? Otherwise the errata will not be handled?

Tony

Re: [PATCH 1/2] OMAP2+: DMA: Workaround for invalid source position

[Tony Lindgren]

* Tony Lindgren <tony@atomide.com> [111103 13:53]:
> * Peter Ujfalusi <peter.ujfalusi@ti.com> [111031 06:46]:
> > 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.
> > 
> > Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> > ---
> >  arch/arm/plat-omap/dma.c |   15 +++++++++++++--
> >  1 files changed, 13 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm/plat-omap/dma.c b/arch/arm/plat-omap/dma.c
> > index c22217c..38b0d44 100644
> > --- a/arch/arm/plat-omap/dma.c
> > +++ b/arch/arm/plat-omap/dma.c
> > @@ -1024,12 +1024,23 @@ EXPORT_SYMBOL(omap_set_dma_callback);
> >   */
> >  dma_addr_t omap_get_dma_src_pos(int lch)
> >  {
> > +	u32 cdac;
> >  	dma_addr_t offset = 0;
> >  
> >  	if (cpu_is_omap15xx())
> >  		offset = p->dma_read(CPC, lch);
> > -	else
> > -		offset = p->dma_read(CSAC, lch);
> > +	else {
> > +		/*
> > +		 * 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.
> > +		 */
> > +		cdac = p->dma_read(CDAC, lch);
> > +		if (likely(cdac))
> > +			offset = p->dma_read(CSAC, lch);
> > +		else
> > +			offset = p->dma_read(CSSA, lch);
> > +	}
> >  
> >  	if (IS_DMA_ERRATA(DMA_ERRATA_3_3) && offset == 0)
> >  		offset = p->dma_read(CSAC, lch);
> 
> Should these tests be done only after the errata re-read for both
> src and dst patches? Otherwise the errata will not be handled?

Also, please Cc linux-arm-kernel list too so I don't have to repost :)

Tony