Re: [PATCH v2 09/11] OMAP: DMA: Implement generic errata handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Manjunath,

On 9/17/2010 7:05 AM, G, Manjunath Kondaiah wrote:
Kevin,

-----Original Message-----
From: G, Manjunath Kondaiah
Sent: Tuesday, September 14, 2010 3:42 PM
To: G, Manjunath Kondaiah; Kevin Hilman
Cc: linux-omap@xxxxxxxxxxxxxxx; Cousson, Benoit; Shilimkar, Santosh
Subject: RE: [PATCH v2 09/11] OMAP: DMA: Implement generic
errata handling

Kevin,

-----Original Message-----
From: linux-omap-owner@xxxxxxxxxxxxxxx
[mailto:linux-omap-owner@xxxxxxxxxxxxxxx] On Behalf Of G, Manjunath
Kondaiah
Sent: Tuesday, September 07, 2010 5:18 PM
To: Kevin Hilman
Cc: linux-omap@xxxxxxxxxxxxxxx; Cousson, Benoit; Shilimkar, Santosh
Subject: RE: [PATCH v2 09/11] OMAP: DMA: Implement generic errata
handling



-----Original Message-----
From: Kevin Hilman [mailto:khilman@xxxxxxxxxxxxxxxxxxx]
Sent: Saturday, September 04, 2010 4:12 AM
To: G, Manjunath Kondaiah
Cc: linux-omap@xxxxxxxxxxxxxxx; Cousson, Benoit;
Shilimkar, Santosh
Subject: Re: [PATCH v2 09/11] OMAP: DMA: Implement generic errata
handling

Manjunatha GK<manjugk@xxxxxx>  writes:

This patch introduces generic way of handling all OMAP
DMA errata's
which are applicable for OMAP1 and OMAP2PLUS processors.

Signed-off-by: Manjunatha GK<manjugk@xxxxxx>
Cc: Benoit Cousson<b-cousson@xxxxxx>
Cc: Kevin Hilman<khilman@xxxxxxxxxxxxxxxxxxx>
Cc: Santosh Shilimkar<santosh.shilimkar@xxxxxx>
---
  arch/arm/mach-omap1/dma.c             |    6 ++++
  arch/arm/mach-omap2/dma.c             |   34
+++++++++++++++++++++++
  arch/arm/plat-omap/dma.c              |   48
++++++++++++++++++--------------
  arch/arm/plat-omap/include/plat/dma.h |    9 ++++++
  4 files changed, 76 insertions(+), 21 deletions(-)

diff --git a/arch/arm/mach-omap1/dma.c
b/arch/arm/mach-omap1/dma.c
index 26ab6e3..615c5f5 100644
--- a/arch/arm/mach-omap1/dma.c
+++ b/arch/arm/mach-omap1/dma.c
@@ -170,6 +170,12 @@ static int __init
omap1_system_dma_init(void)
  		goto exit_device_put;
  	}

+	/* Errata handling for all omap1 plus processors */
+	pdata->errata			= 0;

This isn't needed as you just kzalloc'd pdata.
ok

+	if (cpu_class_is_omap1()&&  !cpu_is_omap15xx())

You don't need cpu_class_is_omap1() as this is OMAP1
specific code.
Ok. Looks like copy, paste issue from plat-omap dma.c. I
will fix it.

+		pdata->errata		|= OMAP3_3_ERRATUM;
+
  	d = pdata->dma_attr;

  	/* Valid attributes for omap1 plus processors
*/ diff --git
a/arch/arm/mach-omap2/dma.c b/arch/arm/mach-omap2/dma.c index
f369bee..8832bd1 100644
--- a/arch/arm/mach-omap2/dma.c
+++ b/arch/arm/mach-omap2/dma.c
@@ -80,6 +80,40 @@ static int __init
omap2_system_dma_init_dev(struct omap_hwmod *oh, void *user)

  	pdata->dma_attr		= (struct omap_dma_dev_attr
*)oh->dev_attr;

+	/* Handling Errata's for all OMAP2PLUS processors */
+	pdata->errata			= 0;

not needed, see above
ok

+	if (cpu_is_omap242x() ||
+		(cpu_is_omap243x()&&   omap_type()<=
OMAP2430_REV_ES1_0))
+		pdata->errata		= DMA_CHAINING_ERRATA;
+
+	/*
+	 * Errata: On ES2.0 BUFFERING disable must be set.
+	 * This will always fail on ES1.0
+	 */
+	if (cpu_is_omap24xx())
+		pdata->errata		|=
DMA_BUFF_DISABLE_ERRATA;
+
+	/*
+	 * Errata: OMAP2: sDMA Channel is not disabled
+	 * after a transaction error. So we explicitely
+	 * disable the channel
+	 */
+	if (cpu_class_is_omap2())
+		pdata->errata		|=
DMA_CH_DISABLE_ERRATA;
+
+	/* Errata: OMAP3 :

fix multi-line comment style
Ok.


+	 * A bug in ROM code leaves IRQ status for channels 0
and 1 uncleared
+	 * after secure sram context save and restore.
Hence we need to
+	 * manually clear those IRQs to avoid spurious
interrupts. This
+	 * affects only secure devices.
+	 */
+	if (cpu_is_omap34xx()&&  (omap_type() !=
OMAP2_DEVICE_TYPE_GP))
+		pdata->errata		|=
DMA_IRQ_STATUS_ERRATA;
+
+	/* Errata3.3: Applicable for all omap2 plus */
+	pdata->errata			|= OMAP3_3_ERRATUM;
+
  	od = omap_device_build(name, 0, oh, pdata,
sizeof(*pdata),
  			omap2_dma_latency,
ARRAY_SIZE(omap2_dma_latency), 0);

diff --git a/arch/arm/plat-omap/dma.c
b/arch/arm/plat-omap/dma.c
index 36c3dde..409586a 100644
--- a/arch/arm/plat-omap/dma.c
+++ b/arch/arm/plat-omap/dma.c
@@ -187,6 +187,25 @@ static inline void set_gdma_dev(int
req, int dev)
  #define set_gdma_dev(req, dev)	do {} while (0)
  #endif

+static void dma_ocpsysconfig_errata(u32 *sys_cf, bool flag)

Please use (or extend) hwmod layer for modifying device SYSCONFIG.

I will check this.

I propose following options for addressing this errata:
Option 1:
Creating 2 new API's with 1 static function in omap_hwmod.c
for handling midle mode errata issues.

static int _get_master_standbymode(oh, u32 *idlemode, u32 v);
int omap_hwmod_set_master_idlemode(struct omap_hwmod *oh, u8
idlemode); int omap_hwmod_get_master_idlemode(struct
omap_hwmod *oh, u32 *idlemode);

In dma driver, the api's will be used as:
omap_hwmod_get_master_idlemode(oh, midle_mode);
omap_hwmod_set_master_idlemode(oh, 1);
/* disable channels which completed data transfer */ ...
/* Restore original standby mode values */
omap_hwmod_set_master_idlemode(oh, *midle_mode)


Option 2:
Create 1 new exported API for modifying sysconfig standby mode bits.
In this case, restoring original standby mode value is
through accessing sysc flags and _sysc_cache values through
oh in DMA driver.

I feel option #1 seems to be cleaner approach since it avoids
accessing oh contents in driver. Let me know if you have
objections for the same.

/Code snippet for option 1 */
static int _get_master_standbymode(oh, u32 *idlemode) {
	u32 mstandby_mask;
	u8 mstandby_shift;

	...

	mstandby_shift = oh->class->sysc->sysc_fields->midle_shift;
	mstandby_mask = (0x3<<  mstandby_shift);
	*idle_mode = ((oh->_sysc_cache)&  mstandby_mask)>>
mstandby_shift;

	return 0;
}

int omap_hwmod_set_master_idlemode(struct omap_hwmod *oh, u8
idlemode) {
	u32 v;
	int retval = 0;
	...
	v = oh->_sysc_cache;

	retval = _set_master_standbymode(oh, idlemode,&v);
	if (!retval)
		_write_sysconfig(v, oh);

	return retval;
}

int omap_hwmod_get_master_idlemode(struct omap_hwmod *oh, u32
*idlemode) {
	u32 v;
	int retval = 0;
	...
	retval = _get_master_standbymode(oh, idlemode);
	return retval;
}


I assume you are ok with option #1. Let me know if you have any
issues/concenrs with above approach. I am in the process of consolidating
all the review comments and addressing all applicable review comments.

Not really, the option #1 will still require you to use the oh pointer, which is supposed to be private to the omap_device.

What is still not clear is why and when you need to change the sysconfig setting.

Do you have a details explanation? Ideally you should try to couple the sysconfig change along with pm_runtime / hwmod state change, then we will be able to handle that smoothly in the framework.

If you cannot do that, you will need to add an omap_device API as well.

Regards,
Benoit

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux