[PATCH v2 0/2] OMAP: omap_device: API to modify SYSCONFIG register

[PATCH v2 0/2] OMAP: omap_device: API to modify SYSCONFIG register

[Kishon Vijay Abraham I]

Certain peripherals require autoidle bits to be disabled before performing
some operations. This patch series provides APIs in omap_device layer to
modify the SYSCONFIG register.

Since current implementation of PM run time framework does not support
changing sysconfig settings during middle of the on going operation,
these APIs will support the same.

For e.g McBSP 2 and 3 in OMAP3 has sidetone feature which requires
autoidle to be disabled before starting the sidetone.

McBSP also requires the SYSCONFIG to be in NOIDLE when ELEMENTSYNCH
mode is selected for DMA operation.

Created on top of linux OMAP master (linux-omap-2.6 :master)
Tested on OMAP4430, OMAP3430 and OMAP2430 SDP boards. Verified that this patch
series does not break the OMAP1 build.

V2:
Mutex is replaced with spinlock.

V1:
* Creates 3 separate API's to change the idle mode to NOIDLE, SMARTIDLE
 and FORCEIDLE and one more API to change the idlemode to default value
 based on the hwmod flag. This change is done to align with the discussion
 on [3]

* Added hwmod mutex in omap hwmod APIs that modifies SYSCONFIG register.

* omap_hwmod_set_slave_idlemode() is not modified to take true/false kind-of
argument since 3 states are associated with SIDLE bits (force, no and smart).

These changes were made to align with Benoit's and Paul's comments for a
similar patch written by Manjunath [1] for changing MSTANDBY bits.

The discussions that happened for the RFC patch can be found at [2]

[1]: http://www.mail-archive.com/linux-omap@vger.kernel.org/msg39647.html
[2]: https://patchwork.kernel.org/patch/134371/
[3]: http://www.mail-archive.com/linux-omap@vger.kernel.org/msg39615.html

Kishon Vijay Abraham I (2):
  OMAP: hwmod: API to handle autoidle mode
  OMAP: omap_device: API to modify AUTOIDLE and SMARTIDLE bits

 arch/arm/mach-omap2/omap_hwmod.c              |   36 +++++
 arch/arm/plat-omap/include/plat/omap_device.h |    6 +
 arch/arm/plat-omap/include/plat/omap_hwmod.h  |    1 +
 arch/arm/plat-omap/omap_device.c              |  176 +++++++++++++++++++++++++
 4 files changed, 219 insertions(+), 0 deletions(-)

[PATCH v2 1/2] OMAP: hwmod: API to handle autoidle mode

[Kishon Vijay Abraham I]

Create a new API that forms a wrapper to _set_module_autoidle()
to modify the AUTOIDLE bit.

This API is intended to be used by drivers that requires direct
manipulation of the AUTOIDLE bits in SYSCONFIG register.
McBSP driver requires autoidle bit to be enabled/disabled while
using sidetone feature.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Benoit Cousson <b-cousson@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c             |   36 ++++++++++++++++++++++++++
 arch/arm/plat-omap/include/plat/omap_hwmod.h |    1 +
 2 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index e282e35..709543a 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -1288,6 +1288,42 @@ static int _idle(struct omap_hwmod *oh)
 }
 
 /**
+ * omap_hwmod_set_slave_autoidle - set the hwmod's OCP slave autoidle
+ * @oh: struct omap_hwmod *
+ * @autoidle: desired AUTOIDLE bitfield value (0 or 1)
+ *
+ * Sets the IP block's OCP slave autoidle in hardware, and updates our
+ * local copy. Intended to be used by drivers that requires
+ * direct manipulation of the AUTOIDLE bits.
+ * Returns -EINVAL if @oh is null, or passes along the return value
+ * from _set_module_autoidle().
+ *
+ * Any users of this function should be scrutinized carefully.
+ */
+int omap_hwmod_set_slave_autoidle(struct omap_hwmod *oh, u8 autoidle)
+{
+	u32 v;
+	int retval = 0;
+	unsigned int long flags;
+
+	if (!oh)
+		return -EINVAL;
+
+	spin_lock_irqsave(&oh->_lock, flags);
+
+	v = oh->_sysc_cache;
+
+	retval = _set_module_autoidle(oh, autoidle, &v);
+
+	if (!retval)
+		_write_sysconfig(v, oh);
+
+	spin_unlock_irqrestore(&oh->_lock, flags);
+
+	return retval;
+}
+
+/**
  * _shutdown - shutdown an omap_hwmod
  * @oh: struct omap_hwmod *
  *
diff --git a/arch/arm/plat-omap/include/plat/omap_hwmod.h b/arch/arm/plat-omap/include/plat/omap_hwmod.h
index 1eee85a..76f0274 100644
--- a/arch/arm/plat-omap/include/plat/omap_hwmod.h
+++ b/arch/arm/plat-omap/include/plat/omap_hwmod.h
@@ -555,6 +555,7 @@ int omap_hwmod_enable_clocks(struct omap_hwmod *oh);
 int omap_hwmod_disable_clocks(struct omap_hwmod *oh);
 
 int omap_hwmod_set_slave_idlemode(struct omap_hwmod *oh, u8 idlemode);
+int omap_hwmod_set_slave_autoidle(struct omap_hwmod *oh, u8 autoidle);
 
 int omap_hwmod_reset(struct omap_hwmod *oh);
 void omap_hwmod_ocp_barrier(struct omap_hwmod *oh);
-- 
1.7.0.4

Re: [PATCH v2 1/2] OMAP: hwmod: API to handle autoidle mode

[Paul Walmsley]

On Mon, 31 Jan 2011, Kishon Vijay Abraham I wrote:

> Create a new API that forms a wrapper to _set_module_autoidle()
> to modify the AUTOIDLE bit.
> 
> This API is intended to be used by drivers that requires direct
> manipulation of the AUTOIDLE bits in SYSCONFIG register.
> McBSP driver requires autoidle bit to be enabled/disabled while
> using sidetone feature.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> Cc: Paul Walmsley <paul@pwsan.com>
> Cc: Benoit Cousson <b-cousson@ti.com>

I've made some changes to this patch.

I don't think this bit is slave-specific.  For example 34xx TRM Rev. ZH 
Table 15-155 "DISPC_SYSCONFIG" refers to both the L3 and L4 interface 
clock gating strategy.  So I've renamed the function to 
omap_hwmod_set_ocp_autoidle().

Also I think the only valid hwmod state that this function could be called 
in is ENABLED, so I've restricted its use to that state.  Please advise if 
this is not correct.

I've also cleaned up a few minor issues with the code and comments.  I'm 
not entirely sure this function the best approach going forward, but it 
should do for now.

Updated patch below, queued for 2.6.39,


- Paul


From: Kishon Vijay Abraham I <kishon@ti.com>
Date: Mon, 31 Jan 2011 12:34:47 +0000
Subject: [PATCH] OMAP2+: hwmod: add API to handle autoidle mode

Create a new API that forms a wrapper to _set_module_autoidle()
to modify the AUTOIDLE bit.

This API is intended to be used by drivers that requires direct
manipulation of the AUTOIDLE bits in SYSCONFIG register.
McBSP driver requires autoidle bit to be enabled/disabled while
using sidetone feature.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Benoit Cousson <b-cousson@ti.com>
[paul@pwsan.com: restrict the hwmod states that the autoidle bit can be changed
 in; changed function name; dropped "int" from "unsigned int long"]
Signed-off-by: Paul Walmsley <paul@pwsan.com>
---
 arch/arm/mach-omap2/omap_hwmod.c             |   36 ++++++++++++++++++++++++++
 arch/arm/plat-omap/include/plat/omap_hwmod.h |    1 +
 2 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 816aeb9..a68a2cf 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -1288,6 +1288,42 @@ static int _idle(struct omap_hwmod *oh)
 }
 
 /**
+ * omap_hwmod_set_ocp_autoidle - set the hwmod's OCP autoidle bit
+ * @oh: struct omap_hwmod *
+ * @autoidle: desired AUTOIDLE bitfield value (0 or 1)
+ *
+ * Sets the IP block's OCP autoidle bit in hardware, and updates our
+ * local copy. Intended to be used by drivers that require
+ * direct manipulation of the AUTOIDLE bits.
+ * Returns -EINVAL if @oh is null or is not in the ENABLED state, or passes
+ * along the return value from _set_module_autoidle().
+ *
+ * Any users of this function should be scrutinized carefully.
+ */
+int omap_hwmod_set_ocp_autoidle(struct omap_hwmod *oh, u8 autoidle)
+{
+	u32 v;
+	int retval = 0;
+	unsigned long flags;
+
+	if (!oh || oh->_state != _HWMOD_STATE_ENABLED)
+		return -EINVAL;
+
+	spin_lock_irqsave(&oh->_lock, flags);
+
+	v = oh->_sysc_cache;
+
+	retval = _set_module_autoidle(oh, autoidle, &v);
+
+	if (!retval)
+		_write_sysconfig(v, oh);
+
+	spin_unlock_irqrestore(&oh->_lock, flags);
+
+	return retval;
+}
+
+/**
  * _shutdown - shutdown an omap_hwmod
  * @oh: struct omap_hwmod *
  *
diff --git a/arch/arm/plat-omap/include/plat/omap_hwmod.h b/arch/arm/plat-omap/include/plat/omap_hwmod.h
index bba2343..98f7f61 100644
--- a/arch/arm/plat-omap/include/plat/omap_hwmod.h
+++ b/arch/arm/plat-omap/include/plat/omap_hwmod.h
@@ -555,6 +555,7 @@ int omap_hwmod_enable_clocks(struct omap_hwmod *oh);
 int omap_hwmod_disable_clocks(struct omap_hwmod *oh);
 
 int omap_hwmod_set_slave_idlemode(struct omap_hwmod *oh, u8 idlemode);
+int omap_hwmod_set_ocp_autoidle(struct omap_hwmod *oh, u8 autoidle);
 
 int omap_hwmod_reset(struct omap_hwmod *oh);
 void omap_hwmod_ocp_barrier(struct omap_hwmod *oh);
-- 
1.7.2.3

[PATCH v2 2/2] OMAP: omap_device: API to modify AUTOIDLE and SMARTIDLE bits

[Kishon Vijay Abraham I]

Provide APIs to be used by the driver in order to modify AUTOIDLE
and SIDLE bits. These APIs in turn call hwmod APIs to modify the
SYSCONFIG register.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
Signed-off-by: Benoit Cousson <b-cousson@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
---
 arch/arm/plat-omap/include/plat/omap_device.h |    6 +
 arch/arm/plat-omap/omap_device.c              |  176 +++++++++++++++++++++++++
 2 files changed, 182 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-omap/include/plat/omap_device.h b/arch/arm/plat-omap/include/plat/omap_device.h
index e4c349f..47ad0ab 100644
--- a/arch/arm/plat-omap/include/plat/omap_device.h
+++ b/arch/arm/plat-omap/include/plat/omap_device.h
@@ -110,6 +110,12 @@ struct powerdomain *omap_device_get_pwrdm(struct omap_device *od);
 u32 omap_device_get_context_loss_count(struct platform_device *pdev);
 
 /* Other */
+int omap_device_noidle(struct omap_device *od);
+int omap_device_smartidle(struct omap_device *od);
+int omap_device_forceidle(struct omap_device *od);
+int omap_device_default_idle(struct omap_device *od);
+int omap_device_enable_autoidle(struct omap_device *od);
+int omap_device_disable_autoidle(struct omap_device *od);
 
 int omap_device_idle_hwmods(struct omap_device *od);
 int omap_device_enable_hwmods(struct omap_device *od);
diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
index 57adb27..da8609a 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -726,6 +726,182 @@ void __iomem *omap_device_get_rt_va(struct omap_device *od)
 	return omap_hwmod_get_mpu_rt_va(od->hwmods[0]);
 }
 
+/**
+ * omap_device_noidle - set the device's slave idlemode to no idle
+ * @od: struct omap_device *
+ *
+ * Sets the IP block's OCP slave idlemode in hardware to no idle.
+ * Intended to be used by drivers that have some erratum that requires direct
+ * manipulation of the SIDLEMODE bits.  Returns -EINVAL if @od is in invalid
+ * state, or passes along the return value from
+ * omap_hwmod_set_slave_idlemode().
+ */
+int omap_device_noidle(struct omap_device *od)
+{
+	int retval = 0, i;
+
+	if (od->_state != OMAP_DEVICE_STATE_ENABLED) {
+		WARN(1, "omap_device: %s.%d: %s() called from invalid state "
+			"%d\n", od->pdev.name, od->pdev.id, __func__,
+			od->_state);
+		return -EINVAL;
+	}
+
+	for (i = 0; i < od->hwmods_cnt; i++)
+		retval |= omap_hwmod_set_slave_idlemode(od->hwmods[i],
+					HWMOD_IDLEMODE_NO);
+
+	return retval;
+}
+
+
+/**
+ * omap_device_smartidle - set the device's slave idlemode to smart idle
+ * @od: struct omap_device *
+ *
+ * Sets the IP block's OCP slave idlemode in hardware to smart idle.
+ * Intended to be used by drivers that have some erratum that requires direct
+ * manipulation of the SIDLEMODE bits.  Returns -EINVAL if @od is in invalid
+ * state, or passes along the return value from
+ * omap_hwmod_set_slave_idlemode().
+ */
+int omap_device_smartidle(struct omap_device *od)
+{
+	int retval = 0, i;
+
+	if (od->_state != OMAP_DEVICE_STATE_ENABLED) {
+		WARN(1, "omap_device: %s.%d: %s() called from invalid state "
+			"%d\n", od->pdev.name, od->pdev.id, __func__,
+			od->_state);
+		return -EINVAL;
+	}
+
+	for (i = 0; i < od->hwmods_cnt; i++)
+		retval |= omap_hwmod_set_slave_idlemode(od->hwmods[i],
+					HWMOD_IDLEMODE_SMART);
+
+	return retval;
+}
+
+
+/**
+ * omap_device_forceidle - set the device's slave idlemode to force idle
+ * @od: struct omap_device *
+ *
+ * Sets the IP block's OCP slave idlemode in hardware to force idle.
+ * Intended to be used by drivers that have some erratum that requires direct
+ * manipulation of the SIDLEMODE bits.  Returns -EINVAL if @od is in invalid
+ * state, or passes along the return value from
+ * omap_hwmod_set_slave_idlemode().
+ */
+int omap_device_forceidle(struct omap_device *od)
+{
+	int retval = 0, i;
+
+	if (od->_state != OMAP_DEVICE_STATE_ENABLED) {
+		WARN(1, "omap_device: %s.%d: %s() called from invalid state "
+			"%d\n", od->pdev.name, od->pdev.id, __func__,
+			od->_state);
+		return -EINVAL;
+	}
+
+	for (i = 0; i < od->hwmods_cnt; i++)
+		retval |= omap_hwmod_set_slave_idlemode(od->hwmods[i],
+					HWMOD_IDLEMODE_FORCE);
+
+	return retval;
+}
+
+/**
+ * omap_device_default_idle - set the device's slave idlemode to no idle or
+ * smart idle based on the hwmod flag
+ * @od: struct omap_device *
+ *
+ * Sets the IP block's OCP slave idlemode in hardware to no idle or smart idle
+ * depending on status of the flag HWMOD_SWSUP_SIDLE.
+ * Intended to be used by drivers that have some erratum that requires direct
+ * manipulation of the SIDLEMODE bits.  Returns -EINVAL if @od is in invalid
+ * state, or passes along the return value from
+ * omap_hwmod_set_slave_idlemode().
+ */
+int omap_device_default_idle(struct omap_device *od)
+{
+	int retval = 0, i;
+	u8 idlemode;
+
+	if (od->_state != OMAP_DEVICE_STATE_ENABLED) {
+		WARN(1, "omap_device: %s.%d: %s() called from invalid state "
+			"%d\n", od->pdev.name, od->pdev.id, __func__,
+		od->_state);
+		return -EINVAL;
+	}
+
+	for (i = 0; i < od->hwmods_cnt; i++) {
+		idlemode = (od->hwmods[i]->flags & HWMOD_SWSUP_SIDLE) ?
+				HWMOD_IDLEMODE_NO : HWMOD_IDLEMODE_SMART;
+		retval |= omap_hwmod_set_slave_idlemode(od->hwmods[i],
+				idlemode);
+	}
+
+	return retval;
+}
+
+/*
+ * omap_device_enable_autoidle - enable the device's OCP slave autoidle
+ * @od: struct omap_device *
+ *
+ * Sets the IP block's OCP slave autoidle in hardware.
+ * Intended to be used by drivers that have some erratum that requires direct
+ * manipulation of the AUTOIDLE bits.  Returns -EINVAL if @od is in invalid
+ * state, or passes along the return value from
+ * omap_hwmod_set_slave_autoidle().
+ */
+int omap_device_enable_autoidle(struct omap_device *od)
+{
+	int retval = 0, i;
+
+	if (od->_state != OMAP_DEVICE_STATE_ENABLED) {
+		WARN(1, "omap_device: %s.%d: %s() called from invalid state "
+			"%d\n", od->pdev.name, od->pdev.id, __func__,
+			od->_state);
+		return -EINVAL;
+	}
+
+	for (i = 0; i < od->hwmods_cnt; i++)
+		retval |= omap_hwmod_set_slave_autoidle(od->hwmods[i],
+					true);
+
+	return retval;
+}
+
+/*
+ * omap_device_disable_autoidle - disable the device's OCP slave autoidle
+ * @od: struct omap_device *
+ *
+ * Disables the IP block's OCP slave autoidle in hardware.
+ * Intended to be used by drivers that have some erratum that requires direct
+ * manipulation of the AUTOIDLE bits.  Returns -EINVAL if @od is in invalid
+ * state, or passes along the return value from
+ * omap_hwmod_set_slave_autoidle().
+ */
+int omap_device_disable_autoidle(struct omap_device *od)
+{
+	int retval = 0, i;
+
+	if (od->_state != OMAP_DEVICE_STATE_ENABLED) {
+		WARN(1, "omap_device: %s.%d: %s() called from invalid state "
+			"%d\n", od->pdev.name, od->pdev.id, __func__,
+			od->_state);
+		return -EINVAL;
+	}
+
+	for (i = 0; i < od->hwmods_cnt; i++)
+		retval |= omap_hwmod_set_slave_autoidle(od->hwmods[i],
+					false);
+
+	return retval;
+}
+
 /*
  * Public functions intended for use in omap_device_pm_latency
  * .activate_func and .deactivate_func function pointers
-- 
1.7.0.4

Re: [PATCH v2 2/2] OMAP: omap_device: API to modify AUTOIDLE and SMARTIDLE bits

[Cousson, Benoit]

Hi Kishon,

Sorry for this late reply.

I'm fine with this omap_device API which is well aligned with the 
discussion we had. I just have few minor comments about the split 
omap_hwmod / omap_device.

To summarize, you should not hack directly hwmod internal attributes 
from the upper layer.

You should simply expose the same number of API at hwmod level instead 
of relying on a single omap_hwmod_set_slave_idlemode with miscellaneous 
parameters.

On 1/31/2011 1:34 PM, ABRAHAM, KISHON VIJAY wrote:
> Provide APIs to be used by the driver in order to modify AUTOIDLE
> and SIDLE bits. These APIs in turn call hwmod APIs to modify the
> SYSCONFIG register.
>
> Signed-off-by: Kishon Vijay Abraham I<kishon@ti.com>
> Signed-off-by: Benoit Cousson<b-cousson@ti.com>
> Cc: Paul Walmsley<paul@pwsan.com>
> ---
>   arch/arm/plat-omap/include/plat/omap_device.h |    6 +
>   arch/arm/plat-omap/omap_device.c              |  176 +++++++++++++++++++++++++
>   2 files changed, 182 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/plat-omap/include/plat/omap_device.h b/arch/arm/plat-omap/include/plat/omap_device.h
> index e4c349f..47ad0ab 100644
> --- a/arch/arm/plat-omap/include/plat/omap_device.h
> +++ b/arch/arm/plat-omap/include/plat/omap_device.h
> @@ -110,6 +110,12 @@ struct powerdomain *omap_device_get_pwrdm(struct omap_device *od);
>   u32 omap_device_get_context_loss_count(struct platform_device *pdev);
>
>   /* Other */
> +int omap_device_noidle(struct omap_device *od);
> +int omap_device_smartidle(struct omap_device *od);
> +int omap_device_forceidle(struct omap_device *od);
> +int omap_device_default_idle(struct omap_device *od);
> +int omap_device_enable_autoidle(struct omap_device *od);
> +int omap_device_disable_autoidle(struct omap_device *od);
>
>   int omap_device_idle_hwmods(struct omap_device *od);
>   int omap_device_enable_hwmods(struct omap_device *od);
> diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
> index 57adb27..da8609a 100644
> --- a/arch/arm/plat-omap/omap_device.c
> +++ b/arch/arm/plat-omap/omap_device.c
> @@ -726,6 +726,182 @@ void __iomem *omap_device_get_rt_va(struct omap_device *od)
>   	return omap_hwmod_get_mpu_rt_va(od->hwmods[0]);
>   }
>
> +/**
> + * omap_device_noidle - set the device's slave idlemode to no idle
> + * @od: struct omap_device *
> + *
> + * Sets the IP block's OCP slave idlemode in hardware to no idle.
> + * Intended to be used by drivers that have some erratum that requires direct
> + * manipulation of the SIDLEMODE bits.  Returns -EINVAL if @od is in invalid
> + * state, or passes along the return value from
> + * omap_hwmod_set_slave_idlemode().
> + */
> +int omap_device_noidle(struct omap_device *od)
> +{
> +	int retval = 0, i;
> +
> +	if (od->_state != OMAP_DEVICE_STATE_ENABLED) {
> +		WARN(1, "omap_device: %s.%d: %s() called from invalid state "
> +			"%d\n", od->pdev.name, od->pdev.id, __func__,
> +			od->_state);
> +		return -EINVAL;
> +	}
> +
> +	for (i = 0; i<  od->hwmods_cnt; i++)
> +		retval |= omap_hwmod_set_slave_idlemode(od->hwmods[i],
> +					HWMOD_IDLEMODE_NO);
> +
> +	return retval;
> +}
> +
> +
> +/**
> + * omap_device_smartidle - set the device's slave idlemode to smart idle
> + * @od: struct omap_device *
> + *
> + * Sets the IP block's OCP slave idlemode in hardware to smart idle.
> + * Intended to be used by drivers that have some erratum that requires direct
> + * manipulation of the SIDLEMODE bits.  Returns -EINVAL if @od is in invalid
> + * state, or passes along the return value from
> + * omap_hwmod_set_slave_idlemode().
> + */
> +int omap_device_smartidle(struct omap_device *od)
> +{
> +	int retval = 0, i;
> +
> +	if (od->_state != OMAP_DEVICE_STATE_ENABLED) {
> +		WARN(1, "omap_device: %s.%d: %s() called from invalid state "
> +			"%d\n", od->pdev.name, od->pdev.id, __func__,
> +			od->_state);
> +		return -EINVAL;
> +	}
> +
> +	for (i = 0; i<  od->hwmods_cnt; i++)
> +		retval |= omap_hwmod_set_slave_idlemode(od->hwmods[i],
> +					HWMOD_IDLEMODE_SMART);
> +
> +	return retval;
> +}
> +
> +
> +/**
> + * omap_device_forceidle - set the device's slave idlemode to force idle
> + * @od: struct omap_device *
> + *
> + * Sets the IP block's OCP slave idlemode in hardware to force idle.
> + * Intended to be used by drivers that have some erratum that requires direct
> + * manipulation of the SIDLEMODE bits.  Returns -EINVAL if @od is in invalid
> + * state, or passes along the return value from
> + * omap_hwmod_set_slave_idlemode().
> + */
> +int omap_device_forceidle(struct omap_device *od)
> +{
> +	int retval = 0, i;
> +
> +	if (od->_state != OMAP_DEVICE_STATE_ENABLED) {
> +		WARN(1, "omap_device: %s.%d: %s() called from invalid state "
> +			"%d\n", od->pdev.name, od->pdev.id, __func__,
> +			od->_state);
> +		return -EINVAL;
> +	}
> +
> +	for (i = 0; i<  od->hwmods_cnt; i++)
> +		retval |= omap_hwmod_set_slave_idlemode(od->hwmods[i],
> +					HWMOD_IDLEMODE_FORCE);
> +
> +	return retval;
> +}
> +
> +/**
> + * omap_device_default_idle - set the device's slave idlemode to no idle or
> + * smart idle based on the hwmod flag
> + * @od: struct omap_device *
> + *
> + * Sets the IP block's OCP slave idlemode in hardware to no idle or smart idle
> + * depending on status of the flag HWMOD_SWSUP_SIDLE.
> + * Intended to be used by drivers that have some erratum that requires direct
> + * manipulation of the SIDLEMODE bits.  Returns -EINVAL if @od is in invalid
> + * state, or passes along the return value from
> + * omap_hwmod_set_slave_idlemode().
> + */
> +int omap_device_default_idle(struct omap_device *od)
> +{
> +	int retval = 0, i;
> +	u8 idlemode;
> +
> +	if (od->_state != OMAP_DEVICE_STATE_ENABLED) {
> +		WARN(1, "omap_device: %s.%d: %s() called from invalid state "
> +			"%d\n", od->pdev.name, od->pdev.id, __func__,
> +		od->_state);
> +		return -EINVAL;
> +	}
> +
> +	for (i = 0; i<  od->hwmods_cnt; i++) {
> +		idlemode = (od->hwmods[i]->flags&  HWMOD_SWSUP_SIDLE) ?
> +				HWMOD_IDLEMODE_NO : HWMOD_IDLEMODE_SMART;
> +		retval |= omap_hwmod_set_slave_idlemode(od->hwmods[i],
> +				idlemode);

That one is the worst :-) You should definitively not hack directly 
hwmod internal attributes from the upper layer.

> +	}
> +
> +	return retval;
> +}
> +
> +/*
> + * omap_device_enable_autoidle - enable the device's OCP slave autoidle
> + * @od: struct omap_device *
> + *
> + * Sets the IP block's OCP slave autoidle in hardware.
> + * Intended to be used by drivers that have some erratum that requires direct
> + * manipulation of the AUTOIDLE bits.  Returns -EINVAL if @od is in invalid
> + * state, or passes along the return value from
> + * omap_hwmod_set_slave_autoidle().
> + */
> +int omap_device_enable_autoidle(struct omap_device *od)
> +{
> +	int retval = 0, i;
> +
> +	if (od->_state != OMAP_DEVICE_STATE_ENABLED) {
> +		WARN(1, "omap_device: %s.%d: %s() called from invalid state "
> +			"%d\n", od->pdev.name, od->pdev.id, __func__,
> +			od->_state);
> +		return -EINVAL;
> +	}
> +
> +	for (i = 0; i<  od->hwmods_cnt; i++)
> +		retval |= omap_hwmod_set_slave_autoidle(od->hwmods[i],
> +					true);

You are using the same API to send either HWMOD_XXX flags or boolean, 
and that's quite confusing.

As I said, you'd better duplicate these APIs at hwmod level.

Benoit

Re: [PATCH v2 2/2] OMAP: omap_device: API to modify AUTOIDLE and SMARTIDLE bits

[ABRAHAM, KISHON VIJAY]

On Tue, Mar 1, 2011 at 4:47 PM, Cousson, Benoit <b-cousson@ti.com> wrote:
> Hi Kishon,
>
> Sorry for this late reply.
>
> I'm fine with this omap_device API which is well aligned with the discussion
> we had. I just have few minor comments about the split omap_hwmod /
> omap_device.
>
> To summarize, you should not hack directly hwmod internal attributes from
> the upper layer.
>
> You should simply expose the same number of API at hwmod level instead of
> relying on a single omap_hwmod_set_slave_idlemode with miscellaneous
> parameters.

  Should it then be like one-one mapping between omap_device APIs and
  omap_hwmod APIs, or have a single omap_device API, that takes a additional
  parameter and based on the parameter, call the appropriate omap_hwmod API?

>
> On 1/31/2011 1:34 PM, ABRAHAM, KISHON VIJAY wrote:
>>
>> Provide APIs to be used by the driver in order to modify AUTOIDLE
>> and SIDLE bits. These APIs in turn call hwmod APIs to modify the
>> SYSCONFIG register.
>>
>> Signed-off-by: Kishon Vijay Abraham I<kishon@ti.com>
>> Signed-off-by: Benoit Cousson<b-cousson@ti.com>
>> Cc: Paul Walmsley<paul@pwsan.com>
>> ---
>>  arch/arm/plat-omap/include/plat/omap_device.h |    6 +
>>  arch/arm/plat-omap/omap_device.c              |  176
>> +++++++++++++++++++++++++
>>  2 files changed, 182 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/plat-omap/include/plat/omap_device.h
>> b/arch/arm/plat-omap/include/plat/omap_device.h
>> index e4c349f..47ad0ab 100644
>> --- a/arch/arm/plat-omap/include/plat/omap_device.h
>> +++ b/arch/arm/plat-omap/include/plat/omap_device.h
>> @@ -110,6 +110,12 @@ struct powerdomain *omap_device_get_pwrdm(struct
>> omap_device *od);
>>  u32 omap_device_get_context_loss_count(struct platform_device *pdev);
>>
>>  /* Other */
>> +int omap_device_noidle(struct omap_device *od);
>> +int omap_device_smartidle(struct omap_device *od);
>> +int omap_device_forceidle(struct omap_device *od);
>> +int omap_device_default_idle(struct omap_device *od);
>> +int omap_device_enable_autoidle(struct omap_device *od);
>> +int omap_device_disable_autoidle(struct omap_device *od);
>>
>>  int omap_device_idle_hwmods(struct omap_device *od);
>>  int omap_device_enable_hwmods(struct omap_device *od);
>> diff --git a/arch/arm/plat-omap/omap_device.c
>> b/arch/arm/plat-omap/omap_device.c
>> index 57adb27..da8609a 100644
>> --- a/arch/arm/plat-omap/omap_device.c
>> +++ b/arch/arm/plat-omap/omap_device.c
>> @@ -726,6 +726,182 @@ void __iomem *omap_device_get_rt_va(struct
>> omap_device *od)
>>        return omap_hwmod_get_mpu_rt_va(od->hwmods[0]);
>>  }
>>
>> +/**
>> + * omap_device_noidle - set the device's slave idlemode to no idle
>> + * @od: struct omap_device *
>> + *
>> + * Sets the IP block's OCP slave idlemode in hardware to no idle.
>> + * Intended to be used by drivers that have some erratum that requires
>> direct
>> + * manipulation of the SIDLEMODE bits.  Returns -EINVAL if @od is in
>> invalid
>> + * state, or passes along the return value from
>> + * omap_hwmod_set_slave_idlemode().
>> + */
>> +int omap_device_noidle(struct omap_device *od)
>> +{
>> +       int retval = 0, i;
>> +
>> +       if (od->_state != OMAP_DEVICE_STATE_ENABLED) {
>> +               WARN(1, "omap_device: %s.%d: %s() called from invalid
>> state "
>> +                       "%d\n", od->pdev.name, od->pdev.id, __func__,
>> +                       od->_state);
>> +               return -EINVAL;
>> +       }
>> +
>> +       for (i = 0; i<  od->hwmods_cnt; i++)
>> +               retval |= omap_hwmod_set_slave_idlemode(od->hwmods[i],
>> +                                       HWMOD_IDLEMODE_NO);
>> +
>> +       return retval;
>> +}
>> +
>> +
>> +/**
>> + * omap_device_smartidle - set the device's slave idlemode to smart idle
>> + * @od: struct omap_device *
>> + *
>> + * Sets the IP block's OCP slave idlemode in hardware to smart idle.
>> + * Intended to be used by drivers that have some erratum that requires
>> direct
>> + * manipulation of the SIDLEMODE bits.  Returns -EINVAL if @od is in
>> invalid
>> + * state, or passes along the return value from
>> + * omap_hwmod_set_slave_idlemode().
>> + */
>> +int omap_device_smartidle(struct omap_device *od)
>> +{
>> +       int retval = 0, i;
>> +
>> +       if (od->_state != OMAP_DEVICE_STATE_ENABLED) {
>> +               WARN(1, "omap_device: %s.%d: %s() called from invalid
>> state "
>> +                       "%d\n", od->pdev.name, od->pdev.id, __func__,
>> +                       od->_state);
>> +               return -EINVAL;
>> +       }
>> +
>> +       for (i = 0; i<  od->hwmods_cnt; i++)
>> +               retval |= omap_hwmod_set_slave_idlemode(od->hwmods[i],
>> +                                       HWMOD_IDLEMODE_SMART);
>> +
>> +       return retval;
>> +}
>> +
>> +
>> +/**
>> + * omap_device_forceidle - set the device's slave idlemode to force idle
>> + * @od: struct omap_device *
>> + *
>> + * Sets the IP block's OCP slave idlemode in hardware to force idle.
>> + * Intended to be used by drivers that have some erratum that requires
>> direct
>> + * manipulation of the SIDLEMODE bits.  Returns -EINVAL if @od is in
>> invalid
>> + * state, or passes along the return value from
>> + * omap_hwmod_set_slave_idlemode().
>> + */
>> +int omap_device_forceidle(struct omap_device *od)
>> +{
>> +       int retval = 0, i;
>> +
>> +       if (od->_state != OMAP_DEVICE_STATE_ENABLED) {
>> +               WARN(1, "omap_device: %s.%d: %s() called from invalid
>> state "
>> +                       "%d\n", od->pdev.name, od->pdev.id, __func__,
>> +                       od->_state);
>> +               return -EINVAL;
>> +       }
>> +
>> +       for (i = 0; i<  od->hwmods_cnt; i++)
>> +               retval |= omap_hwmod_set_slave_idlemode(od->hwmods[i],
>> +                                       HWMOD_IDLEMODE_FORCE);
>> +
>> +       return retval;
>> +}
>> +
>> +/**
>> + * omap_device_default_idle - set the device's slave idlemode to no idle
>> or
>> + * smart idle based on the hwmod flag
>> + * @od: struct omap_device *
>> + *
>> + * Sets the IP block's OCP slave idlemode in hardware to no idle or smart
>> idle
>> + * depending on status of the flag HWMOD_SWSUP_SIDLE.
>> + * Intended to be used by drivers that have some erratum that requires
>> direct
>> + * manipulation of the SIDLEMODE bits.  Returns -EINVAL if @od is in
>> invalid
>> + * state, or passes along the return value from
>> + * omap_hwmod_set_slave_idlemode().
>> + */
>> +int omap_device_default_idle(struct omap_device *od)
>> +{
>> +       int retval = 0, i;
>> +       u8 idlemode;
>> +
>> +       if (od->_state != OMAP_DEVICE_STATE_ENABLED) {
>> +               WARN(1, "omap_device: %s.%d: %s() called from invalid
>> state "
>> +                       "%d\n", od->pdev.name, od->pdev.id, __func__,
>> +               od->_state);
>> +               return -EINVAL;
>> +       }
>> +
>> +       for (i = 0; i<  od->hwmods_cnt; i++) {
>> +               idlemode = (od->hwmods[i]->flags&  HWMOD_SWSUP_SIDLE) ?
>> +                               HWMOD_IDLEMODE_NO : HWMOD_IDLEMODE_SMART;
>> +               retval |= omap_hwmod_set_slave_idlemode(od->hwmods[i],
>> +                               idlemode);
>
> That one is the worst :-) You should definitively not hack directly hwmod
> internal attributes from the upper layer.
>
>> +       }
>> +
>> +       return retval;
>> +}
>> +
>> +/*
>> + * omap_device_enable_autoidle - enable the device's OCP slave autoidle
>> + * @od: struct omap_device *
>> + *
>> + * Sets the IP block's OCP slave autoidle in hardware.
>> + * Intended to be used by drivers that have some erratum that requires
>> direct
>> + * manipulation of the AUTOIDLE bits.  Returns -EINVAL if @od is in
>> invalid
>> + * state, or passes along the return value from
>> + * omap_hwmod_set_slave_autoidle().
>> + */
>> +int omap_device_enable_autoidle(struct omap_device *od)
>> +{
>> +       int retval = 0, i;
>> +
>> +       if (od->_state != OMAP_DEVICE_STATE_ENABLED) {
>> +               WARN(1, "omap_device: %s.%d: %s() called from invalid
>> state "
>> +                       "%d\n", od->pdev.name, od->pdev.id, __func__,
>> +                       od->_state);
>> +               return -EINVAL;
>> +       }
>> +
>> +       for (i = 0; i<  od->hwmods_cnt; i++)
>> +               retval |= omap_hwmod_set_slave_autoidle(od->hwmods[i],
>> +                                       true);
>
> You are using the same API to send either HWMOD_XXX flags or boolean, and
> that's quite confusing.
>
> As I said, you'd better duplicate these APIs at hwmod level.

  OK.

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

Re: [PATCH v2 2/2] OMAP: omap_device: API to modify AUTOIDLE and SMARTIDLE bits

[Cousson, Benoit]

On 3/1/2011 12:27 PM, ABRAHAM, KISHON VIJAY wrote:
> On Tue, Mar 1, 2011 at 4:47 PM, Cousson, Benoit<b-cousson@ti.com>  wrote:
>> Hi Kishon,
>>
>> Sorry for this late reply.
>>
>> I'm fine with this omap_device API which is well aligned with the discussion
>> we had. I just have few minor comments about the split omap_hwmod /
>> omap_device.
>>
>> To summarize, you should not hack directly hwmod internal attributes from
>> the upper layer.
>>
>> You should simply expose the same number of API at hwmod level instead of
>> relying on a single omap_hwmod_set_slave_idlemode with miscellaneous
>> parameters.
>
>    Should it then be like one-one mapping between omap_device APIs and
>    omap_hwmod APIs, or have a single omap_device API, that takes a additional
>    parameter and based on the parameter, call the appropriate omap_hwmod API?

A one-to-one is the best thing to do for my point of view.

Benoit

>
>>
>> On 1/31/2011 1:34 PM, ABRAHAM, KISHON VIJAY wrote:
>>>
>>> Provide APIs to be used by the driver in order to modify AUTOIDLE
>>> and SIDLE bits. These APIs in turn call hwmod APIs to modify the
>>> SYSCONFIG register.
>>>
>>> Signed-off-by: Kishon Vijay Abraham I<kishon@ti.com>
>>> Signed-off-by: Benoit Cousson<b-cousson@ti.com>
>>> Cc: Paul Walmsley<paul@pwsan.com>
>>> ---
>>>   arch/arm/plat-omap/include/plat/omap_device.h |    6 +
>>>   arch/arm/plat-omap/omap_device.c              |  176
>>> +++++++++++++++++++++++++
>>>   2 files changed, 182 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/arch/arm/plat-omap/include/plat/omap_device.h
>>> b/arch/arm/plat-omap/include/plat/omap_device.h
>>> index e4c349f..47ad0ab 100644
>>> --- a/arch/arm/plat-omap/include/plat/omap_device.h
>>> +++ b/arch/arm/plat-omap/include/plat/omap_device.h
>>> @@ -110,6 +110,12 @@ struct powerdomain *omap_device_get_pwrdm(struct
>>> omap_device *od);
>>>   u32 omap_device_get_context_loss_count(struct platform_device *pdev);
>>>
>>>   /* Other */
>>> +int omap_device_noidle(struct omap_device *od);
>>> +int omap_device_smartidle(struct omap_device *od);
>>> +int omap_device_forceidle(struct omap_device *od);
>>> +int omap_device_default_idle(struct omap_device *od);
>>> +int omap_device_enable_autoidle(struct omap_device *od);
>>> +int omap_device_disable_autoidle(struct omap_device *od);
>>>
>>>   int omap_device_idle_hwmods(struct omap_device *od);
>>>   int omap_device_enable_hwmods(struct omap_device *od);
>>> diff --git a/arch/arm/plat-omap/omap_device.c
>>> b/arch/arm/plat-omap/omap_device.c
>>> index 57adb27..da8609a 100644
>>> --- a/arch/arm/plat-omap/omap_device.c
>>> +++ b/arch/arm/plat-omap/omap_device.c
>>> @@ -726,6 +726,182 @@ void __iomem *omap_device_get_rt_va(struct
>>> omap_device *od)
>>>         return omap_hwmod_get_mpu_rt_va(od->hwmods[0]);
>>>   }
>>>
>>> +/**
>>> + * omap_device_noidle - set the device's slave idlemode to no idle
>>> + * @od: struct omap_device *
>>> + *
>>> + * Sets the IP block's OCP slave idlemode in hardware to no idle.
>>> + * Intended to be used by drivers that have some erratum that requires
>>> direct
>>> + * manipulation of the SIDLEMODE bits.  Returns -EINVAL if @od is in
>>> invalid
>>> + * state, or passes along the return value from
>>> + * omap_hwmod_set_slave_idlemode().
>>> + */
>>> +int omap_device_noidle(struct omap_device *od)
>>> +{
>>> +       int retval = 0, i;
>>> +
>>> +       if (od->_state != OMAP_DEVICE_STATE_ENABLED) {
>>> +               WARN(1, "omap_device: %s.%d: %s() called from invalid
>>> state "
>>> +                       "%d\n", od->pdev.name, od->pdev.id, __func__,
>>> +                       od->_state);
>>> +               return -EINVAL;
>>> +       }
>>> +
>>> +       for (i = 0; i<    od->hwmods_cnt; i++)
>>> +               retval |= omap_hwmod_set_slave_idlemode(od->hwmods[i],
>>> +                                       HWMOD_IDLEMODE_NO);
>>> +
>>> +       return retval;
>>> +}
>>> +
>>> +
>>> +/**
>>> + * omap_device_smartidle - set the device's slave idlemode to smart idle
>>> + * @od: struct omap_device *
>>> + *
>>> + * Sets the IP block's OCP slave idlemode in hardware to smart idle.
>>> + * Intended to be used by drivers that have some erratum that requires
>>> direct
>>> + * manipulation of the SIDLEMODE bits.  Returns -EINVAL if @od is in
>>> invalid
>>> + * state, or passes along the return value from
>>> + * omap_hwmod_set_slave_idlemode().
>>> + */
>>> +int omap_device_smartidle(struct omap_device *od)
>>> +{
>>> +       int retval = 0, i;
>>> +
>>> +       if (od->_state != OMAP_DEVICE_STATE_ENABLED) {
>>> +               WARN(1, "omap_device: %s.%d: %s() called from invalid
>>> state "
>>> +                       "%d\n", od->pdev.name, od->pdev.id, __func__,
>>> +                       od->_state);
>>> +               return -EINVAL;
>>> +       }
>>> +
>>> +       for (i = 0; i<    od->hwmods_cnt; i++)
>>> +               retval |= omap_hwmod_set_slave_idlemode(od->hwmods[i],
>>> +                                       HWMOD_IDLEMODE_SMART);
>>> +
>>> +       return retval;
>>> +}
>>> +
>>> +
>>> +/**
>>> + * omap_device_forceidle - set the device's slave idlemode to force idle
>>> + * @od: struct omap_device *
>>> + *
>>> + * Sets the IP block's OCP slave idlemode in hardware to force idle.
>>> + * Intended to be used by drivers that have some erratum that requires
>>> direct
>>> + * manipulation of the SIDLEMODE bits.  Returns -EINVAL if @od is in
>>> invalid
>>> + * state, or passes along the return value from
>>> + * omap_hwmod_set_slave_idlemode().
>>> + */
>>> +int omap_device_forceidle(struct omap_device *od)
>>> +{
>>> +       int retval = 0, i;
>>> +
>>> +       if (od->_state != OMAP_DEVICE_STATE_ENABLED) {
>>> +               WARN(1, "omap_device: %s.%d: %s() called from invalid
>>> state "
>>> +                       "%d\n", od->pdev.name, od->pdev.id, __func__,
>>> +                       od->_state);
>>> +               return -EINVAL;
>>> +       }
>>> +
>>> +       for (i = 0; i<    od->hwmods_cnt; i++)
>>> +               retval |= omap_hwmod_set_slave_idlemode(od->hwmods[i],
>>> +                                       HWMOD_IDLEMODE_FORCE);
>>> +
>>> +       return retval;
>>> +}
>>> +
>>> +/**
>>> + * omap_device_default_idle - set the device's slave idlemode to no idle
>>> or
>>> + * smart idle based on the hwmod flag
>>> + * @od: struct omap_device *
>>> + *
>>> + * Sets the IP block's OCP slave idlemode in hardware to no idle or smart
>>> idle
>>> + * depending on status of the flag HWMOD_SWSUP_SIDLE.
>>> + * Intended to be used by drivers that have some erratum that requires
>>> direct
>>> + * manipulation of the SIDLEMODE bits.  Returns -EINVAL if @od is in
>>> invalid
>>> + * state, or passes along the return value from
>>> + * omap_hwmod_set_slave_idlemode().
>>> + */
>>> +int omap_device_default_idle(struct omap_device *od)
>>> +{
>>> +       int retval = 0, i;
>>> +       u8 idlemode;
>>> +
>>> +       if (od->_state != OMAP_DEVICE_STATE_ENABLED) {
>>> +               WARN(1, "omap_device: %s.%d: %s() called from invalid
>>> state "
>>> +                       "%d\n", od->pdev.name, od->pdev.id, __func__,
>>> +               od->_state);
>>> +               return -EINVAL;
>>> +       }
>>> +
>>> +       for (i = 0; i<    od->hwmods_cnt; i++) {
>>> +               idlemode = (od->hwmods[i]->flags&    HWMOD_SWSUP_SIDLE) ?
>>> +                               HWMOD_IDLEMODE_NO : HWMOD_IDLEMODE_SMART;
>>> +               retval |= omap_hwmod_set_slave_idlemode(od->hwmods[i],
>>> +                               idlemode);
>>
>> That one is the worst :-) You should definitively not hack directly hwmod
>> internal attributes from the upper layer.
>>
>>> +       }
>>> +
>>> +       return retval;
>>> +}
>>> +
>>> +/*
>>> + * omap_device_enable_autoidle - enable the device's OCP slave autoidle
>>> + * @od: struct omap_device *
>>> + *
>>> + * Sets the IP block's OCP slave autoidle in hardware.
>>> + * Intended to be used by drivers that have some erratum that requires
>>> direct
>>> + * manipulation of the AUTOIDLE bits.  Returns -EINVAL if @od is in
>>> invalid
>>> + * state, or passes along the return value from
>>> + * omap_hwmod_set_slave_autoidle().
>>> + */
>>> +int omap_device_enable_autoidle(struct omap_device *od)
>>> +{
>>> +       int retval = 0, i;
>>> +
>>> +       if (od->_state != OMAP_DEVICE_STATE_ENABLED) {
>>> +               WARN(1, "omap_device: %s.%d: %s() called from invalid
>>> state "
>>> +                       "%d\n", od->pdev.name, od->pdev.id, __func__,
>>> +                       od->_state);
>>> +               return -EINVAL;
>>> +       }
>>> +
>>> +       for (i = 0; i<    od->hwmods_cnt; i++)
>>> +               retval |= omap_hwmod_set_slave_autoidle(od->hwmods[i],
>>> +                                       true);
>>
>> You are using the same API to send either HWMOD_XXX flags or boolean, and
>> that's quite confusing.
>>
>> As I said, you'd better duplicate these APIs at hwmod level.
>
>    OK.
>
>>
>> Benoit
>>
>>
>

Re: [PATCH v2 0/2] OMAP: omap_device: API to modify SYSCONFIG register

[ABRAHAM, KISHON VIJAY]

Kevin, Paul, Benoit,

Do you have any comments for this patch series?

-Kishon

On Mon, Jan 31, 2011 at 6:04 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote:
> Certain peripherals require autoidle bits to be disabled before performing
> some operations. This patch series provides APIs in omap_device layer to
> modify the SYSCONFIG register.
>
> Since current implementation of PM run time framework does not support
> changing sysconfig settings during middle of the on going operation,
> these APIs will support the same.
>
> For e.g McBSP 2 and 3 in OMAP3 has sidetone feature which requires
> autoidle to be disabled before starting the sidetone.
>
> McBSP also requires the SYSCONFIG to be in NOIDLE when ELEMENTSYNCH
> mode is selected for DMA operation.
>
> Created on top of linux OMAP master (linux-omap-2.6 :master)
> Tested on OMAP4430, OMAP3430 and OMAP2430 SDP boards. Verified that this patch
> series does not break the OMAP1 build.
>
> V2:
> Mutex is replaced with spinlock.
>
> V1:
> * Creates 3 separate API's to change the idle mode to NOIDLE, SMARTIDLE
>  and FORCEIDLE and one more API to change the idlemode to default value
>  based on the hwmod flag. This change is done to align with the discussion
>  on [3]
>
> * Added hwmod mutex in omap hwmod APIs that modifies SYSCONFIG register.
>
> * omap_hwmod_set_slave_idlemode() is not modified to take true/false kind-of
> argument since 3 states are associated with SIDLE bits (force, no and smart).
>
> These changes were made to align with Benoit's and Paul's comments for a
> similar patch written by Manjunath [1] for changing MSTANDBY bits.
>
> The discussions that happened for the RFC patch can be found at [2]
>
> [1]: http://www.mail-archive.com/linux-omap@vger.kernel.org/msg39647.html
> [2]: https://patchwork.kernel.org/patch/134371/
> [3]: http://www.mail-archive.com/linux-omap@vger.kernel.org/msg39615.html
>
> Kishon Vijay Abraham I (2):
>  OMAP: hwmod: API to handle autoidle mode
>  OMAP: omap_device: API to modify AUTOIDLE and SMARTIDLE bits
>
>  arch/arm/mach-omap2/omap_hwmod.c              |   36 +++++
>  arch/arm/plat-omap/include/plat/omap_device.h |    6 +
>  arch/arm/plat-omap/include/plat/omap_hwmod.h  |    1 +
>  arch/arm/plat-omap/omap_device.c              |  176 +++++++++++++++++++++++++
>  4 files changed, 219 insertions(+), 0 deletions(-)
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/2] OMAP: omap_device: API to modify SYSCONFIG register

[ABRAHAM, KISHON VIJAY]

Hi Kevin, Paul, Benoit,

Do you have any comments for this patch series? This series is
necessary for McBSP hwmod and
PM runtime patch series to be merged.

Regards
Kishon

On Tue, Feb 15, 2011 at 12:08 PM, ABRAHAM, KISHON VIJAY <kishon@ti.com> wrote:
> Kevin, Paul, Benoit,
>
> Do you have any comments for this patch series?
>
> -Kishon
>
> On Mon, Jan 31, 2011 at 6:04 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote:
>> Certain peripherals require autoidle bits to be disabled before performing
>> some operations. This patch series provides APIs in omap_device layer to
>> modify the SYSCONFIG register.
>>
>> Since current implementation of PM run time framework does not support
>> changing sysconfig settings during middle of the on going operation,
>> these APIs will support the same.
>>
>> For e.g McBSP 2 and 3 in OMAP3 has sidetone feature which requires
>> autoidle to be disabled before starting the sidetone.
>>
>> McBSP also requires the SYSCONFIG to be in NOIDLE when ELEMENTSYNCH
>> mode is selected for DMA operation.
>>
>> Created on top of linux OMAP master (linux-omap-2.6 :master)
>> Tested on OMAP4430, OMAP3430 and OMAP2430 SDP boards. Verified that this patch
>> series does not break the OMAP1 build.
>>
>> V2:
>> Mutex is replaced with spinlock.
>>
>> V1:
>> * Creates 3 separate API's to change the idle mode to NOIDLE, SMARTIDLE
>>  and FORCEIDLE and one more API to change the idlemode to default value
>>  based on the hwmod flag. This change is done to align with the discussion
>>  on [3]
>>
>> * Added hwmod mutex in omap hwmod APIs that modifies SYSCONFIG register.
>>
>> * omap_hwmod_set_slave_idlemode() is not modified to take true/false kind-of
>> argument since 3 states are associated with SIDLE bits (force, no and smart).
>>
>> These changes were made to align with Benoit's and Paul's comments for a
>> similar patch written by Manjunath [1] for changing MSTANDBY bits.
>>
>> The discussions that happened for the RFC patch can be found at [2]
>>
>> [1]: http://www.mail-archive.com/linux-omap@vger.kernel.org/msg39647.html
>> [2]: https://patchwork.kernel.org/patch/134371/
>> [3]: http://www.mail-archive.com/linux-omap@vger.kernel.org/msg39615.html
>>
>> Kishon Vijay Abraham I (2):
>>  OMAP: hwmod: API to handle autoidle mode
>>  OMAP: omap_device: API to modify AUTOIDLE and SMARTIDLE bits
>>
>>  arch/arm/mach-omap2/omap_hwmod.c              |   36 +++++
>>  arch/arm/plat-omap/include/plat/omap_device.h |    6 +
>>  arch/arm/plat-omap/include/plat/omap_hwmod.h  |    1 +
>>  arch/arm/plat-omap/omap_device.c              |  176 +++++++++++++++++++++++++
>>  4 files changed, 219 insertions(+), 0 deletions(-)
>>
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html