RE: [PATCH v3 02/13] OMAP: DMA: Introduce errata handling feature
> -Original Message- > From: Kevin Hilman [mailto:khil...@deeprootsystems.com] > Sent: Wednesday, November 10, 2010 9:56 PM > To: G, Manjunath Kondaiah > Cc: linux-omap@vger.kernel.org; > linux-arm-ker...@lists.infradead.org; Peter Ujfalusi; > Cousson, Benoit; Shilimkar, Santosh > Subject: Re: [PATCH v3 02/13] OMAP: DMA: Introduce errata > handling feature > > "G, Manjunath Kondaiah" writes: > > [...] > > >> > +if (omap_type() == OMAP3430_REV_ES1_0) > >> > +SET_DMA_ERRATA(DMA_ERRATA_i88); > >> > + > >> > +/* > >> > + * Errata 3.2/3.3: sometimes 0 is returned if > CSAC/CDAC is > >> > >> This doesn't look like a unique errata identifier. Chapter 3 > >> is usually > >> the 'Cautions' section, and I don't listed in either the > 34xx (v5.9), > >> 36xx (v1.3) or 44xx (v1.3) errata docs under this number, and > >> don't see > >> that number in either of the 24xx errata docs either. > >> Please clarify. > > > > Even I tried to find this info from errata docs prior to migrating > > all errata's to generic errata handling. This code was > exising earlier, > > hence I retained it "as is", no documentation exists for > this errata. > > > > Hmm, OK. > > According to the code, it seems to affect every IP execpt > 1510, yet it's > not documented in any of the errata docs. > > I wonder what happens if it is removed? This errata work around is used in two API's: 1. omap_get_dma_src_pos 2. omap_get_dma_dst_pos which are used in sound and usb drivers. we can run sound and usb unit tests which are using above two API's and if there no issues, drop this errata work around? -Manjunath -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 02/13] OMAP: DMA: Introduce errata handling feature
"G, Manjunath Kondaiah" writes: [...] >> > + if (omap_type() == OMAP3430_REV_ES1_0) >> > + SET_DMA_ERRATA(DMA_ERRATA_i88); >> > + >> > + /* >> > + * Errata 3.2/3.3: sometimes 0 is returned if CSAC/CDAC is >> >> This doesn't look like a unique errata identifier. Chapter 3 >> is usually >> the 'Cautions' section, and I don't listed in either the 34xx (v5.9), >> 36xx (v1.3) or 44xx (v1.3) errata docs under this number, and >> don't see >> that number in either of the 24xx errata docs either. >> Please clarify. > > Even I tried to find this info from errata docs prior to migrating > all errata's to generic errata handling. This code was exising earlier, > hence I retained it "as is", no documentation exists for this errata. > Hmm, OK. According to the code, it seems to affect every IP execpt 1510, yet it's not documented in any of the errata docs. I wonder what happens if it is removed? Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v3 02/13] OMAP: DMA: Introduce errata handling feature
> -Original Message- > From: Kevin Hilman [mailto:khil...@deeprootsystems.com] > Sent: Wednesday, November 10, 2010 3:43 AM > To: G, Manjunath Kondaiah > Cc: linux-omap@vger.kernel.org; > linux-arm-ker...@lists.infradead.org; Peter Ujfalusi; > Cousson, Benoit; Shilimkar, Santosh > Subject: Re: [PATCH v3 02/13] OMAP: DMA: Introduce errata > handling feature > > "G, Manjunath Kondaiah" writes: > > > Implement errata handling to use flags instead of cpu_is_* > > and cpu_class_* in the code. > > > > The errata flags are initialized at init time and during runtime > > we are using the errata variable (via the IS_DMA_ERRATA macro) > > to execute the required errata workaround. > > > > Reused errata handling patch from Peter Ujfalusi > > > https://patchwork.kernel.org/patch/231191/ > > When starting from someone else's work, would be very helpful to > reviewers (and original authors) if you summarized what you > changed/fixed/updated etc. ok. I will update patch summary. > > > Signed-off-by: G, Manjunath Kondaiah > > Cc: Peter Ujfalusi > > Cc: Benoit Cousson > > Cc: Kevin Hilman > > Cc: Santosh Shilimkar > > --- > > arch/arm/plat-omap/dma.c | 134 > ++-- [...] > > + if (omap_type() == OMAP3430_REV_ES1_0) > > + SET_DMA_ERRATA(DMA_ERRATA_i88); > > + > > + /* > > +* Errata 3.2/3.3: sometimes 0 is returned if CSAC/CDAC is > > This doesn't look like a unique errata identifier. Chapter 3 > is usually > the 'Cautions' section, and I don't listed in either the 34xx (v5.9), > 36xx (v1.3) or 44xx (v1.3) errata docs under this number, and > don't see > that number in either of the 24xx errata docs either. > Please clarify. Even I tried to find this info from errata docs prior to migrating all errata's to generic errata handling. This code was exising earlier, hence I retained it "as is", no documentation exists for this errata. > > > +* read before the DMA controller finished disabling > the channel. > > +*/ > > + if (!cpu_is_omap15xx()) > > + SET_DMA_ERRATA(DMA_ERRATA_3_3); > > +} > > + > > > /* > */ > > > > static int __init omap_init_dma(void) > > @@ -2481,6 +2528,9 @@ static int __init omap_init_dma(void) > > } > > } > > > > + /* Configure errata handling for all omap's */ > > + configure_dma_errata(); > > + > > return 0; > > > > out_free: > > diff --git a/arch/arm/plat-omap/include/plat/dma.h > b/arch/arm/plat-omap/include/plat/dma.h > > index 27578f3..5e28d26 100644 > > --- a/arch/arm/plat-omap/include/plat/dma.h > > +++ b/arch/arm/plat-omap/include/plat/dma.h > > @@ -285,6 +285,17 @@ > > #define DMA_CH_PRIO_HIGH 0x1 > > #define DMA_CH_PRIO_LOW0x0 /* Def */ > > > > +/* Errata handling */ > > +#define IS_DMA_ERRATA(id) (errata &= (id)) > > Why the '&=' here? I guess it should just be a '&' The '=' > was not in > Peter's original. correct. There was bug in peter's original patch for SET_DMA_ERRATA(id) it was | instead |=, while fixing set_dma_errata, accidentally, I have changed is_dma_errata also. Will fix it. > > > +#define SET_DMA_ERRATA(id) (errata |= (id)) > > + > > +#define DMA_ERRATA_IFRAME_BUFFERING(1 << 0) > > +#define DMA_ERRATA_PARALLEL_CHANNELS (1 << 1) > > +#define DMA_ERRATA_i378(1 << 2) > > +#define DMA_ERRATA_i541(1 << 3) > > +#define DMA_ERRATA_i88 (1 << 4) > > +#define DMA_ERRATA_3_3 (1 << 5) > > Please use BIT() ok. -Manjunath-- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 02/13] OMAP: DMA: Introduce errata handling feature
"G, Manjunath Kondaiah" writes:
> Implement errata handling to use flags instead of cpu_is_*
> and cpu_class_* in the code.
>
> The errata flags are initialized at init time and during runtime
> we are using the errata variable (via the IS_DMA_ERRATA macro)
> to execute the required errata workaround.
>
> Reused errata handling patch from Peter Ujfalusi
> https://patchwork.kernel.org/patch/231191/
When starting from someone else's work, would be very helpful to
reviewers (and original authors) if you summarized what you
changed/fixed/updated etc.
> Signed-off-by: G, Manjunath Kondaiah
> Cc: Peter Ujfalusi
> Cc: Benoit Cousson
> Cc: Kevin Hilman
> Cc: Santosh Shilimkar
> ---
> arch/arm/plat-omap/dma.c | 134 ++--
> arch/arm/plat-omap/include/plat/dma.h | 11 +++
> 2 files changed, 103 insertions(+), 42 deletions(-)
>
> diff --git a/arch/arm/plat-omap/dma.c b/arch/arm/plat-omap/dma.c
> index 77241e2..0ff82d0 100644
> --- a/arch/arm/plat-omap/dma.c
> +++ b/arch/arm/plat-omap/dma.c
> @@ -195,6 +195,7 @@ enum { DMA_CHAIN_STARTED, DMA_CHAIN_NOTSTARTED };
> #define OMAP_FUNC_MUX_ARM_BASE (0xfffe1000 + 0xec)
>
> static int enable_1510_mode;
> +static u32 errata;
>
> static struct omap_dma_global_context_registers {
> u32 dma_irqenable_l0;
> @@ -1216,12 +1217,8 @@ void omap_start_dma(int lch)
>
> cur_lch = next_lch;
> } while (next_lch != -1);
> - } else if (cpu_is_omap242x() ||
> - (cpu_is_omap243x() && omap_type() <= OMAP2430_REV_ES1_0)) {
> -
> - /* Errata: Need to write lch even if not using chaining */
> + } else if (IS_DMA_ERRATA(DMA_ERRATA_PARALLEL_CHANNELS))
> dma_write(lch, CLNK_CTRL2, lch);
> - }
>
> omap_enable_channel_irq(lch);
>
> @@ -1231,17 +1228,8 @@ void omap_start_dma(int lch)
> dma_write(l, CCR1, lch);
> } else {
> l = dma_read(CCR2, lch);
> - /*
> - * Errata: Inter Frame DMA buffering issue (All OMAP2420 and
> - * OMAP2430ES1.0): DMA will wrongly buffer elements if packing
> - * and bursting is enabled. This might result in data gets
> - * stalled in FIFO at the end of the block.
> - * Workaround: DMA channels must have BUFFERING_DISABLED bit
> - * set to guarantee no data will stay in the DMA FIFO in case
> - * inter frame buffering occurs.
> - */
> - if (cpu_is_omap2420() || (cpu_is_omap2430() &&
> - (omap_type() == OMAP2430_REV_ES1_0)))
> +
> + if (IS_DMA_ERRATA(DMA_ERRATA_IFRAME_BUFFERING))
> l |= OMAP_DMA_CCR_BUFFERING_DISABLE;
>
> l |= OMAP_DMA_CCR_EN;
> @@ -1253,14 +1241,14 @@ EXPORT_SYMBOL(omap_start_dma);
>
> void omap_stop_dma(int lch)
> {
> - u32 l;
> + u32 l = 0;
>
> /* Disable all interrupts on the channel */
> if (cpu_class_is_omap1())
> dma_write(0, CICR1, lch);
>
> - /* OMAP3 Errata i541: sDMA FIFO draining does not finish */
> - if (cpu_is_omap34xx() && (l & OMAP_DMA_CCR_SEL_SRC_DST_SYNC)) {
> + if (IS_DMA_ERRATA(DMA_ERRATA_i541) &&
> + (l & OMAP_DMA_CCR_SEL_SRC_DST_SYNC)) {
> int i = 0;
> u32 sys_cf;
>
> @@ -1367,11 +1355,7 @@ dma_addr_t omap_get_dma_src_pos(int lch)
> else
> offset = dma_read(CSAC2, 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 (IS_DMA_ERRATA(DMA_ERRATA_3_3) && offset == 0) {
> if (cpu_class_is_omap1())
> offset = dma_read(CSAC1, lch);
> else
> @@ -1966,7 +1950,7 @@ int omap_stop_dma_chain_transfers(int chain_id)
> {
> int *channels;
> u32 l, i;
> - u32 sys_cf;
> + u32 sys_cf = 0;
>
> /* Check for input params */
> if (unlikely((chain_id < 0 || chain_id >= dma_lch_count))) {
> @@ -1981,15 +1965,13 @@ int omap_stop_dma_chain_transfers(int chain_id)
> }
> channels = dma_linked_lch[chain_id].linked_dmach_q;
>
> - /*
> - * DMA Errata:
> - * Special programming model needed to disable DMA before end of block
> - */
> - sys_cf = dma_read(OCP_SYSCONFIG, 0);
> - l = sys_cf;
> - /* Middle mode reg set no Standby */
> - l &= ~((1 << 12)|(1 << 13));
> - dma_write(l, OCP_SYSCONFIG, 0);
> + if (IS_DMA_ERRATA(DMA_ERRATA_i88)) {
> + sys_cf = dma_read(OCP_SYSCONFIG, 0);
> + l = sys_cf;
> + /* Middle mode reg set no Standby */
> + l &= ~((1 << 12)|(1 << 13));
> + dma_write(l, OCP_SYSCONFIG, 0);
> + }
>
> for (i = 0; i < dma_linked_lch[chain_i
