Skip to contentSkip to conversation

OMAP: omap_device: API to modify SYSCONFIG register

PATCH Superseded Kishon Vijay Abraham I 3 participants 9 messages Jan 31, 2011 – Mar 10, 2011

Subsystems (4)

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

v2 cover OMAP: omap_device: API to modify SYSCONFIG register

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/[email protected]/msg39647.html
[2]: https://patchwork.kernel.org/patch/134371/
[3]: http://www.mail-archive.com/[email protected]/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(-)
#1 lore raw

PATCH v2 1/2 OMAP: hwmod: 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.

Cc Paul Walmsley <[email protected]>
Cc Benoit Cousson <[email protected]>
---
 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(-)
arch/arm/mach-omap2/omap_hwmod.c +36 -0
index e282e35..709543a 100644
@@ -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 *
  *
arch/arm/plat-omap/include/plat/omap_hwmod.h +1 -0
index 1eee85a..76f0274 100644
@@ -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);
Signature · 1 line
1.7.0.4
maintainer
#2 lore raw

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

Quoted 11 lines · 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.

Cc Paul Walmsley <[email protected]>
Cc Benoit Cousson <[email protected]>

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 <[email protected]>
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.

Cc Paul Walmsley <[email protected]>
Cc Benoit Cousson <[email protected]>

[[email protected]: restrict the hwmod states that the autoidle bit can be changed
in; changed function name; dropped "int" from "unsigned int long"]

---
 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(-)
arch/arm/mach-omap2/omap_hwmod.c +36 -0
index 816aeb9..a68a2cf 100644
@@ -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 *
  *
arch/arm/plat-omap/include/plat/omap_hwmod.h +1 -0
index bba2343..98f7f61 100644
@@ -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);
Signature · 1 line
1.7.2.3
#3 lore raw

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

In reply to 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.

---
 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(-)
arch/arm/plat-omap/include/plat/omap_device.h +6 -0
index e4c349f..47ad0ab 100644
@@ -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);
arch/arm/plat-omap/omap_device.c +176 -0

182 lines not on this page · raw message

Signature · 1 line
1.7.0.4
#4 lore raw

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

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.

Quoted 142 lines · KISHON VIJAY

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.

---
  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(-)
arch/arm/plat-omap/include/plat/omap_device.h +6 -0
index e4c349f..47ad0ab 100644
@@ -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);
arch/arm/plat-omap/omap_device.c +105 -0
index 57adb27..da8609a 100644
@@ -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 "

arch/arm/plat-omap/omap_device.c void __iomem *omap_device_get_rt_va(struct omap_device *od) · line 843

Quoted 10 lines
arch/arm/plat-omap/omap_device.c +10 -0
+			"%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.

Quoted 19 lines
arch/arm/plat-omap/omap_device.c +19 -0
+	}
+
+	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;
+

arch/arm/plat-omap/omap_device.c void __iomem *omap_device_get_rt_va(struct omap_device *od) · line 872

Quoted 10 lines
arch/arm/plat-omap/omap_device.c +10 -0
+	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

#5 lore raw

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

Quoted 8 lines · 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.

Quoted 6 lines

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?

Quoted 214 lines · KISHON VIJAY
Quoted 164 lines

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.

---
 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(-)
arch/arm/plat-omap/include/plat/omap_device.h +6 -0
index e4c349f..47ad0ab 100644
@@ -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);

arch/arm/plat-omap/omap_device.c +6 -0
index 57adb27..da8609a 100644
@@ -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().
+ */

arch/arm/plat-omap/omap_device.c +6 -0
+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;
+       }

arch/arm/plat-omap/omap_device.c +15 -0
+
+       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().
+ */

arch/arm/plat-omap/omap_device.c +6 -0
+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;
+       }

arch/arm/plat-omap/omap_device.c +15 -0
+
+       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().
+ */

arch/arm/plat-omap/omap_device.c +6 -0
+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;
+       }

arch/arm/plat-omap/omap_device.c +10 -0
+
+       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().
+ */

arch/arm/plat-omap/omap_device.c +7 -0
+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);

Quoted 8 lines

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

Quoted 24 lines

+       }

arch/arm/plat-omap/omap_device.c +10 -0
+
+       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().
+ */

arch/arm/plat-omap/omap_device.c +6 -0
+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 "

Quoted 8 lines

+                       "%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.

Quoted 1 line

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

OK.

Quoted 4 lines

Benoit

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

#6 lore raw

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

Quoted 12 lines · KISHON VIJAY
Quoted 11 lines · 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.

Quoted 7 lines
Quoted 3 lines

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

Quoted 224 lines
Quoted 215 lines · KISHON VIJAY
Quoted 162 lines

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.

---
  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(-)
arch/arm/plat-omap/include/plat/omap_device.h +6 -0
index e4c349f..47ad0ab 100644
@@ -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);
arch/arm/plat-omap/omap_device.c +6 -0
index 57adb27..da8609a 100644
@@ -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().
+ */

arch/arm/plat-omap/omap_device.c +6 -0
+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 "

arch/arm/plat-omap/omap_device.c +19 -0
+                       "%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().
+ */

arch/arm/plat-omap/omap_device.c +6 -0
+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 "

arch/arm/plat-omap/omap_device.c +19 -0
+                       "%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().
+ */

arch/arm/plat-omap/omap_device.c +6 -0
+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 "

arch/arm/plat-omap/omap_device.c +14 -0
+                       "%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().
+ */

arch/arm/plat-omap/omap_device.c +7 -0
+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 "

arch/arm/plat-omap/omap_device.c void __iomem *omap_device_get_rt_va(struct omap_device *od) · line 843

Quoted 10 lines
arch/arm/plat-omap/omap_device.c +10 -0
+                       "%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.

Quoted 24 lines
arch/arm/plat-omap/omap_device.c +11 -0
+       }
+
+       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().
+ */

arch/arm/plat-omap/omap_device.c +6 -0
+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 "

arch/arm/plat-omap/omap_device.c void __iomem *omap_device_get_rt_va(struct omap_device *od) · line 872

Quoted 8 lines
arch/arm/plat-omap/omap_device.c +8 -0
+                       "%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.

Quoted 1 line

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

   OK.
Quoted 4 lines

Benoit

#7 lore raw
In reply to Kishon Vijay Abraham I

Kevin, Paul, Benoit,

Do you have any comments for this patch series?

-Kishon

Quoted 45 lines · 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/[email protected]/msg39647.html
[2]: https://patchwork.kernel.org/patch/134371/
[3]: http://www.mail-archive.com/[email protected]/msg39615.html

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

Quoted 7 lines

 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#8 lore raw

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

Quoted 60 lines · KISHON VIJAY

Kevin, Paul, Benoit,

Do you have any comments for this patch series?

-Kishon

Quoted 52 lines · 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/[email protected]/msg39647.html
[2]: https://patchwork.kernel.org/patch/134371/
[3]: http://www.mail-archive.com/[email protected]/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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html