Re: [PATCH 08/13] OMAP2+: powerdomain: control power domains next state
On Thu, Jul 28, 2011 at 10:30:15AM +0200, jean.pi...@newoldbits.com wrote:
...
+int pwrdm_set_wkup_lat_constraint(struct powerdomain *pwrdm, void *cookie,
+ long min_latency)
+{
+ struct pwrdm_wkup_constraints_entry *user = NULL;
+ struct pwrdm_wkup_constraints_entry *tmp_user, *new_user;
+ int ret = 0, free_new_user = 0, free_node = 0;
+ long value = 0;
+ unsigned long flags;
+
+ pr_debug(powerdomain: %s: pwrdm %s, cookie=0x%p, min_latency=%ld\n,
+ __func__, pwrdm-name, cookie, min_latency);
+
+ new_user = kzalloc(sizeof(struct pwrdm_wkup_constraints_entry),
+GFP_KERNEL);
+ if (!new_user) {
+ pr_err(%s: FATAL ERROR: kzalloc failed\n, __func__);
+ return -ENOMEM;
+ }
+
+ spin_lock_irqsave(pwrdm-wkup_lat_plist_lock, flags);
+
+ /* Check if there already is a constraint for cookie */
+ plist_for_each_entry(tmp_user, pwrdm-wkup_lat_plist_head, node) {
+ if (tmp_user-cookie == cookie) {
+ user = tmp_user;
+ free_new_user = 1;
+ break;
+ }
+ }
+
+ if (min_latency != PM_QOS_DEV_LAT_DEFAULT_VALUE) {
+ /* If nothing to update, job done */
+ if (user (user-node.prio == min_latency))
+ goto exit_ok;
+
+ if (!user) {
+ /* Add new entry to the list */
+ user = new_user;
+ user-cookie = cookie;
+ } else {
+ /* Update existing entry */
+ plist_del(user-node, pwrdm-wkup_lat_plist_head);
+ }
+
+ plist_node_init(user-node, min_latency);
+ plist_add(user-node, pwrdm-wkup_lat_plist_head);
+ } else {
+ /* Remove the constraint from the list */
+ if (!user) {
+ pr_err(%s: Error: no prior constraint to release\n,
+__func__);
+ ret = -EINVAL;
+ goto exit_error;
+ }
+
+ plist_del(user-node, pwrdm-wkup_lat_plist_head);
+ free_node = 1;
All min_latency != PM_QOS_DEV_LAT_DEFAULT_VALUE paths need
free_new_user = 1. (Or maybe change the logic to check user !=
new_user and free new_user if so.)
+ }
+
+exit_ok:
+ /* Find the strongest constraint from the list */
+ if (!plist_head_empty(pwrdm-wkup_lat_plist_head))
+ value = plist_first(pwrdm-wkup_lat_plist_head)-prio;
+
+ spin_unlock_irqrestore(pwrdm-wkup_lat_plist_lock, flags);
+
+ if (free_node)
+ kfree(user);
+
+ if (free_new_user)
+ kfree(new_user);
+
+ /* Apply the constraint to the pwrdm */
+ pr_debug(powerdomain: %s: pwrdm %s, value=%ld\n,
+ __func__, pwrdm-name, value);
+ pwrdm_wakeuplat_update_pwrst(pwrdm, value);
+
+ return 0;
+
+exit_error:
+ spin_unlock_irqrestore(pwrdm-wkup_lat_plist_lock, flags);
Need:
kfree(new_user);
+ return ret;
+}
Todd
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/13] OMAP PM: early init of the pwrdms states
On Thu, Jul 28, 2011 at 10:30:14AM +0200, jean.pi...@newoldbits.com wrote: From: Jean Pihet j-pi...@ti.com The powerdomains next states are initialized in pwrdms_setup as a late_initcall. Because the PM QoS devices constraint can be requested early in the boot sequence, the power domains next states can be overwritten by pwrdms_setup. This patch fixes it by initializing the power domains next states early at boot, so that the constraints can be applied. Later in the pwrdms_setup function the currently programmed next states are re-used as next state values. Applies to OMAP3 and OMAP4. Tested on OMAP3 Beagleboard and OMAP4 Pandaboard in RET/OFF using wake-up latency constraints on MPU, CORE and PER. Signed-off-by: Jean Pihet j-pi...@ti.com --- ... diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c index 9af0847..63c3e7a 100644 --- a/arch/arm/mach-omap2/powerdomain.c +++ b/arch/arm/mach-omap2/powerdomain.c @@ -108,6 +108,9 @@ static int _pwrdm_register(struct powerdomain *pwrdm) pwrdm-state = pwrdm_read_pwrst(pwrdm); pwrdm-state_counter[pwrdm-state] = 1; + /* Early init of the next power state */ + pwrdm_set_next_pwrst(pwrdm, PWRDM_POWER_RET); + Wanted to check that it's OK to initialize the next state of a power domain to RETENTION early in the boot sequence. I believe patches have been previously discussed that set the state to ON to ensure the domain doesn't go to a lower state, and possibly lose context, before the PM subsystem is setup to handle it? Not sure, thought maybe worth a doublecheck. Todd -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v3 00/13] PM QoS: add a per-device latency constraints class
Mark, On Thu, Jul 28, 2011 at 3:14 PM, mark gross markgr...@thegnar.org wrote: On Thu, Jul 28, 2011 at 10:30:07AM +0200, jean.pi...@newoldbits.com wrote: From: Jean Pihet j-pi...@ti.com This patch set is in an RFC state, for review and comments. High level implementation: ... 7. Misc fixes to improve code readability: . rename of the PM QoS implementation file from pm_qos_params.[ch] to pm_qos.[ch] I picked the name for the file as pm_qos_params over pm_qos because I wanted to make it implicitly clear that this was not an full QOS implementation. True QOS carries expectations similar to real time and as the infrastructure is closer to good intentioned than even best effort and offers no notification when the QOS request is not able to be met and really doesn't implement a true QOS at all, (it just provides the parameter interface for part of one its missing the notification interface when the service level is not met and I think a few other things.) So I wanted to have it named a bit different from just pm_qos. This said I'm not supper attached to the naming of the modules. If folks want to change it I wouldn't complain (too much anyway;). Ok got the idea. I do not know what name to chose though. As suggested previously the name pm_qos_params does not reflect the implementation, that is why I renamed it. --mark PS I'll look at the rest of the patches tomorrow, this time for real as I'm about to have more free time to focus on non-work stuff :) Thanks you for reviewing! FWIW this write up sounds interesting. Hope it is readable ;p Regards, Jean -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/13] OMAP2+: powerdomain: control power domains next state
Todd,
On Fri, Jul 29, 2011 at 9:59 AM, Todd Poynor toddpoy...@google.com wrote:
On Thu, Jul 28, 2011 at 10:30:15AM +0200, jean.pi...@newoldbits.com wrote:
...
+int pwrdm_set_wkup_lat_constraint(struct powerdomain *pwrdm, void *cookie,
+ long min_latency)
+{
+ struct pwrdm_wkup_constraints_entry *user = NULL;
+ struct pwrdm_wkup_constraints_entry *tmp_user, *new_user;
+ int ret = 0, free_new_user = 0, free_node = 0;
+ long value = 0;
+ unsigned long flags;
+
+ pr_debug(powerdomain: %s: pwrdm %s, cookie=0x%p, min_latency=%ld\n,
+ __func__, pwrdm-name, cookie, min_latency);
+
+ new_user = kzalloc(sizeof(struct pwrdm_wkup_constraints_entry),
+ GFP_KERNEL);
+ if (!new_user) {
+ pr_err(%s: FATAL ERROR: kzalloc failed\n, __func__);
+ return -ENOMEM;
+ }
+
+ spin_lock_irqsave(pwrdm-wkup_lat_plist_lock, flags);
+
+ /* Check if there already is a constraint for cookie */
+ plist_for_each_entry(tmp_user, pwrdm-wkup_lat_plist_head, node) {
+ if (tmp_user-cookie == cookie) {
+ user = tmp_user;
+ free_new_user = 1;
+ break;
+ }
+ }
+
+ if (min_latency != PM_QOS_DEV_LAT_DEFAULT_VALUE) {
+ /* If nothing to update, job done */
+ if (user (user-node.prio == min_latency))
+ goto exit_ok;
+
+ if (!user) {
+ /* Add new entry to the list */
+ user = new_user;
+ user-cookie = cookie;
+ } else {
+ /* Update existing entry */
+ plist_del(user-node, pwrdm-wkup_lat_plist_head);
+ }
+
+ plist_node_init(user-node, min_latency);
+ plist_add(user-node, pwrdm-wkup_lat_plist_head);
+ } else {
+ /* Remove the constraint from the list */
+ if (!user) {
+ pr_err(%s: Error: no prior constraint to release\n,
+ __func__);
+ ret = -EINVAL;
+ goto exit_error;
+ }
+
+ plist_del(user-node, pwrdm-wkup_lat_plist_head);
+ free_node = 1;
All min_latency != PM_QOS_DEV_LAT_DEFAULT_VALUE paths need
free_new_user = 1.
free_new_user = 1 is only needed if no existing constraint has been
found, i.e. user stays at NULL. This is implemented in the check for
an existing constraint (plist_for_each_entry(...)).
(Or maybe change the logic to check user !=
new_user and free new_user if so.)
That could be done as well.
+ }
+
+exit_ok:
+ /* Find the strongest constraint from the list */
+ if (!plist_head_empty(pwrdm-wkup_lat_plist_head))
+ value = plist_first(pwrdm-wkup_lat_plist_head)-prio;
+
+ spin_unlock_irqrestore(pwrdm-wkup_lat_plist_lock, flags);
+
+ if (free_node)
+ kfree(user);
+
+ if (free_new_user)
+ kfree(new_user);
+
+ /* Apply the constraint to the pwrdm */
+ pr_debug(powerdomain: %s: pwrdm %s, value=%ld\n,
+ __func__, pwrdm-name, value);
+ pwrdm_wakeuplat_update_pwrst(pwrdm, value);
+
+ return 0;
+
+exit_error:
+ spin_unlock_irqrestore(pwrdm-wkup_lat_plist_lock, flags);
Need:
kfree(new_user);
Correct!
BTW I have a new version (patch here below) that improves the cases
with latency = PM_QOS_DEV_LAT_DEFAULT_VALUE. This will come in v4
after testing.
+ return ret;
+}
Todd
Thanks for reviewing!
Regards,
Jean
---
Patch for cases with latency = PM_QOS_DEV_LAT_DEFAULT_VALUE:
diff --git a/arch/arm/mach-omap2/powerdomain.c
b/arch/arm/mach-omap2/powerdomain.c
index c0f48ab..2e9379b 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -206,7 +206,7 @@ static int _pwrdm_post_transition_cb(struct
powerdomain *pwrdm, void *unused)
* pwrdm_wakeuplat_update_pwrst - Update power domain power state if needed
* @pwrdm: struct powerdomain * to which requesting device belongs to.
* @min_latency: the allowed wake-up latency for the given power domain. A
- * value of 0 means 'no constraint' on the pwrdm.
+ * value of PM_QOS_DEV_LAT_DEFAULT_VALUE means 'no constraint' on the pwrdm.
*
* Finds the power domain next power state that fulfills the constraint.
* Programs a new target state if it is different from current power state.
@@ -232,7 +232,7 @@ static int pwrdm_wakeuplat_update_pwrst(struct
powerdomain *pwrdm,
/* Find power state with wakeup latency minimum constraint */
for (new_state = 0x0; new_state PWRDM_MAX_PWRSTS; new_state++) {
- if (min_latency == 0 ||
+ if (min_latency == PM_QOS_DEV_LAT_DEFAULT_VALUE ||
pwrdm-wakeup_lat[new_state] = min_latency)
Re: [PATCH 07/13] OMAP PM: early init of the pwrdms states
On Fri, Jul 29, 2011 at 10:08 AM, Todd Poynor toddpoy...@google.com wrote: On Thu, Jul 28, 2011 at 10:30:14AM +0200, jean.pi...@newoldbits.com wrote: From: Jean Pihet j-pi...@ti.com The powerdomains next states are initialized in pwrdms_setup as a late_initcall. Because the PM QoS devices constraint can be requested early in the boot sequence, the power domains next states can be overwritten by pwrdms_setup. This patch fixes it by initializing the power domains next states early at boot, so that the constraints can be applied. Later in the pwrdms_setup function the currently programmed next states are re-used as next state values. Applies to OMAP3 and OMAP4. Tested on OMAP3 Beagleboard and OMAP4 Pandaboard in RET/OFF using wake-up latency constraints on MPU, CORE and PER. Signed-off-by: Jean Pihet j-pi...@ti.com --- ... diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c index 9af0847..63c3e7a 100644 --- a/arch/arm/mach-omap2/powerdomain.c +++ b/arch/arm/mach-omap2/powerdomain.c @@ -108,6 +108,9 @@ static int _pwrdm_register(struct powerdomain *pwrdm) pwrdm-state = pwrdm_read_pwrst(pwrdm); pwrdm-state_counter[pwrdm-state] = 1; + /* Early init of the next power state */ + pwrdm_set_next_pwrst(pwrdm, PWRDM_POWER_RET); + Wanted to check that it's OK to initialize the next state of a power domain to RETENTION early in the boot sequence. I believe patches have been previously discussed that set the state to ON to ensure the domain doesn't go to a lower state, and possibly lose context, before the PM subsystem is setup to handle it? Not sure, thought maybe worth a doublecheck. Indeed I need to check the behavior for OMAP3 4 which seem to initialize the pwrdm states differently. BTW the patch that inits all pwrdms to ON is not yet in l-o master that is why I (lazily) submitted this one for now. Todd Jean -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: USB Ethernet gadget doesn't work with DM3730
Hi, On Thu, Jul 14, 2011 at 01:34:54PM +0200, Enric Balletbò i Serra wrote: Playing with USB ethernet gadget on IGEP boards with mainline kernel (Linux 3.0.0-rc7) I observed one strange behavior. The ethernet gadget works with one board with OMAP3530 but it doesn't with another one with DM3730. The log looks like this: root@igep00x0:~# modprobe g_ether [ 51.420257] g_ether gadget: using random host ethernet address [ 51.427886] usb0: MAC da:93:4b:2d:55:e9 [ 51.431915] usb0: HOST MAC 42:47:83:70:46:b6 [ 51.436706] g_ether gadget: Ethernet Gadget, version: Memorial Day 2008 [ 51.443725] g_ether gadget: g_ether ready [ 51.447906] musb-hdrc musb-hdrc: MUSB HDRC host driver [ 51.455871] musb-hdrc musb-hdrc: new USB bus registered, assigned bus number 1 [ 51.464141] usb usb1: New USB device found, idVendor=1d6b, idProduct=0002 [ 51.471313] usb usb1: New USB device strings: Mfr=3, Product=2, SerialNumber=1 [ 51.478912] usb usb1: Product: MUSB HDRC host driver [ 51.484161] usb usb1: Manufacturer: Linux 3.0.0-rc7-00027-g8d86e5f musb-hcd [ 51.491485] usb usb1: SerialNumber: musb-hdrc [ 51.498321] hub 1-0:1.0: USB hub found [ 51.502410] hub 1-0:1.0: 1 port detected root@igep00x0:~# ifconfig usb0 192.168.7.2 usb0 Link encap:Ethernet HWaddr DA:93:4B:2D:55:E9 inet addr:192.168.7.2 Bcast:192.168.7.255 Mask:255.255.255.0 UP BROADCAST RUNNING MULTICAST MTU:1500 Metric:1 RX packets:0 errors:0 dropped:0 overruns:0 frame:0 TX packets:0 errors:0 dropped:0 overruns:0 carrier:0 collisions:0 txqueuelen:1000 RX bytes:0 (0.0 B) TX bytes:0 (0.0 B) root@igep00x0:~# ping 192.168.7.1 PING 192.168.7.1 (192.168.7.1): 56 data bytes With DM3730 I can't ping the other side (the same configuration with OMAP3530 works without problems). Any clue on this ? DMA problem ?? Can you check if PIO-only works ? -- balbi signature.asc Description: Digital signature
Re: [PATCHv4 2/4] regulator: omap smps regulator driver
On Thu, Jul 28, 2011 at 02:48:57PM +0300, Tero Kristo wrote: OMAP SMPS regulator driver provides access to OMAP voltage processor controlled regulators. These include VDD_MPU and VDD_CORE for OMAP3 and additionally VDD_IVA for OMAP4. SMPS regulators use the OMAP voltage layer for the actual voltage regulation operations. Signed-off-by: Tero Kristo t-kri...@ti.com Acked-by: Mark Brown broo...@opensource.wolfsonmicro.com -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v2]: Issues implementing clock handling mechanism within UART driver
Hi, On Thu, Jul 28, 2011 at 02:59:55PM +0530, Govindraj.R wrote: Proposal: 1. For the UART, follow the current approach of locking the console in Idle/Suspend path before cutting the clock but using pm_runtime_putsync. That is, continue using the prepare/resume Idle calls in idle path. I believe you should be using -prepare() to prevent that any other work on UART won't trigger a console_write() right now. Maybe only queueing the work for after -complete() or, maybe, just ignoring the work and loosing some prints, dunno. 2. Other Approach would be adding conditions to debug prints from omap_device/ omap_hwmod/clock_framework avoid calling these debug prints for uart. Or Even a debug macro that would not debug prints if the context is from uart. yeah, that might work but then again, if we were using 8250.c, would we have this limitation ?? I think that, maybe, your best call would be to capture console_write() calls into an internal temporary buffer on -prepare() and flush it once -complete() is called ? BTW, where are the patches ? :-) -- balbi signature.asc Description: Digital signature
Re: [Q] No message from Kernel (Howto start debug?)
2011/7/29 Tapani Utriainen tap...@technexion.com:
On Thu, 28 Jul 2011 16:18:51 +0200
Arno Steffen arno.stef...@googlemail.com wrote:
That has been good points. I've tried both, but with no result so far.
Try appending earlyprintk=${console} to your bootargs (where ${console}
is your console string, e.g. ttyO0,115200n8 )
---
Tapani Utriainen, PhD
Senior Software Engineer
TechNexion Ltd, www.technexion.com
Tried this without a change. I really think this is a machine-id problem.
So although I have done same changes for my board as in my previous 33
kernel, it might be not enough to cover the machine-id. Maybe I can
overule this for test to find out the source of this misbehaviour.
The problem is already before the first debug messages are coming.
Sricharan is right, first the message uncompress has to be printed.
Thanks
- Arno
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv5 1/3] OMAP: I2C: Reset support
Under some error conditions the i2c driver may do a reset.
Adding a reset field and support in the platform.
Signed-off-by: Shubhrajyoti D shubhrajy...@ti.com
---
arch/arm/plat-omap/i2c.c | 18 ++
include/linux/i2c-omap.h |1 +
2 files changed, 19 insertions(+), 0 deletions(-)
diff --git a/arch/arm/plat-omap/i2c.c b/arch/arm/plat-omap/i2c.c
index 2388b8e..be36cac 100644
--- a/arch/arm/plat-omap/i2c.c
+++ b/arch/arm/plat-omap/i2c.c
@@ -146,6 +146,22 @@ static struct omap_device_pm_latency omap_i2c_latency[] = {
.flags = OMAP_DEVICE_LATENCY_AUTO_ADJUST,
},
};
+/**
+ * omap2_i2c_reset - reset the omap i2c module.
+ * @dev: struct device*
+ */
+
+static int omap2_i2c_reset(struct device *dev)
+{
+ int r = 0;
+ struct platform_device *pdev = to_platform_device(dev);
+ struct omap_device *odev = to_omap_device(pdev);
+ struct omap_hwmod *oh;
+
+ oh = odev-hwmods[0];
+ r = omap_hwmod_reset(oh);
+ return r;
+}
static inline int omap2_i2c_add_bus(int bus_id)
{
@@ -187,6 +203,8 @@ static inline int omap2_i2c_add_bus(int bus_id)
*/
if (cpu_is_omap34xx())
pdata-set_mpu_wkup_lat = omap_pm_set_max_mpu_wakeup_lat_compat;
+
+ pdata-device_reset = omap2_i2c_reset;
od = omap_device_build(name, bus_id, oh, pdata,
sizeof(struct omap_i2c_bus_platform_data),
omap_i2c_latency, ARRAY_SIZE(omap_i2c_latency), 0);
diff --git a/include/linux/i2c-omap.h b/include/linux/i2c-omap.h
index 98ae49b..8aa91b6 100644
--- a/include/linux/i2c-omap.h
+++ b/include/linux/i2c-omap.h
@@ -38,6 +38,7 @@ struct omap_i2c_bus_platform_data {
int (*device_enable) (struct platform_device *pdev);
int (*device_shutdown) (struct platform_device *pdev);
int (*device_idle) (struct platform_device *pdev);
+ int (*device_reset) (struct device *dev);
};
#endif
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv5 3/3] OMAP: I2C: Remove the SYSC register definition
The SYSC register should not accessed in the driver removing the
define from the driver.
Also clean up the syscstate from the omap_i2c_dev struct.
Acked-by: Santosh Shilimkar santosh.shilim...@ti.com
Signed-off-by: Shubhrajyoti D shubhrajy...@ti.com
---
drivers/i2c/busses/i2c-omap.c |5 -
1 files changed, 0 insertions(+), 5 deletions(-)
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index d8f4470..d05efe7 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -63,7 +63,6 @@ enum {
OMAP_I2C_BUF_REG,
OMAP_I2C_CNT_REG,
OMAP_I2C_DATA_REG,
- OMAP_I2C_SYSC_REG,
OMAP_I2C_CON_REG,
OMAP_I2C_OA_REG,
OMAP_I2C_SA_REG,
@@ -187,7 +186,6 @@ struct omap_i2c_dev {
u16 scllstate;
u16 sclhstate;
u16 bufstate;
- u16 syscstate;
u16 westate;
u16 errata;
};
@@ -202,7 +200,6 @@ const static u8 reg_map_ip_v1[] = {
[OMAP_I2C_BUF_REG] = 0x05,
[OMAP_I2C_CNT_REG] = 0x06,
[OMAP_I2C_DATA_REG] = 0x07,
- [OMAP_I2C_SYSC_REG] = 0x08,
[OMAP_I2C_CON_REG] = 0x09,
[OMAP_I2C_OA_REG] = 0x0a,
[OMAP_I2C_SA_REG] = 0x0b,
@@ -223,7 +220,6 @@ const static u8 reg_map_ip_v2[] = {
[OMAP_I2C_BUF_REG] = 0x94,
[OMAP_I2C_CNT_REG] = 0x98,
[OMAP_I2C_DATA_REG] = 0x9c,
- [OMAP_I2C_SYSC_REG] = 0x20,
[OMAP_I2C_CON_REG] = 0xa4,
[OMAP_I2C_OA_REG] = 0xa8,
[OMAP_I2C_SA_REG] = 0xac,
@@ -264,7 +260,6 @@ static void omap_i2c_unidle(struct omap_i2c_dev *dev)
omap_i2c_write_reg(dev, OMAP_I2C_SCLL_REG, dev-scllstate);
omap_i2c_write_reg(dev, OMAP_I2C_SCLH_REG, dev-sclhstate);
omap_i2c_write_reg(dev, OMAP_I2C_BUF_REG, dev-bufstate);
- omap_i2c_write_reg(dev, OMAP_I2C_SYSC_REG, dev-syscstate);
omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, dev-westate);
omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, OMAP_I2C_CON_EN);
}
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv5 2/3] OMAP: I2C: Remove the reset in the init path
- The reset in the driver at init is not needed anymore as the
hwmod framework takes care of reseting it.
- Reset is removed from omap_i2c_init, which was called
not only during probe, but also after time out and error handling.
device_reset were added in those places to effect the reset.
- Earlier the hwmod SYSC settings were over-written in the driver.
Removing the same and letting the hwmod take care of the settings.
- Clean up the SYSS_RESETDONE_MASK macro as it is no longer needed.
- Clean up the SYSCONFIG SYSC bit defination macros.
- Fix the typos in wakeup.
Signed-off-by: Shubhrajyoti D shubhrajy...@ti.com
---
drivers/i2c/busses/i2c-omap.c | 83 +++--
1 files changed, 22 insertions(+), 61 deletions(-)
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 4e3256f..d8f4470 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -155,19 +155,6 @@ enum {
#define OMAP_I2C_SYSTEST_SDA_O (1 0)/* SDA line drive out */
#endif
-/* OCP_SYSSTATUS bit definitions */
-#define SYSS_RESETDONE_MASK(1 0)
-
-/* OCP_SYSCONFIG bit definitions */
-#define SYSC_CLOCKACTIVITY_MASK(0x3 8)
-#define SYSC_SIDLEMODE_MASK(0x3 3)
-#define SYSC_ENAWAKEUP_MASK(1 2)
-#define SYSC_SOFTRESET_MASK(1 1)
-#define SYSC_AUTOIDLE_MASK (1 0)
-
-#define SYSC_IDLEMODE_SMART0x2
-#define SYSC_CLOCKACTIVITY_FCLK0x2
-
/* Errata definitions */
#define I2C_OMAP_ERRATA_I207 (1 0)
#define I2C_OMAP3_1P153(1 1)
@@ -182,6 +169,7 @@ struct omap_i2c_dev {
u32 latency;/* maximum mpu wkup latency */
void(*set_mpu_wkup_lat)(struct device *dev,
long latency);
+ int (*device_reset)(struct device *dev);
u32 speed; /* Speed of bus in Khz */
u16 cmd_err;
u8 *buf;
@@ -317,60 +305,22 @@ static int omap_i2c_init(struct omap_i2c_dev *dev)
u16 psc = 0, scll = 0, sclh = 0, buf = 0;
u16 fsscll = 0, fssclh = 0, hsscll = 0, hssclh = 0;
unsigned long fclk_rate = 1200;
- unsigned long timeout;
unsigned long internal_clk = 0;
struct clk *fclk;
struct omap_i2c_bus_platform_data *pdata;
pdata = dev-dev-platform_data;
- if (dev-rev = OMAP_I2C_OMAP1_REV_2) {
- /* Disable I2C controller before soft reset */
- omap_i2c_write_reg(dev, OMAP_I2C_CON_REG,
- omap_i2c_read_reg(dev, OMAP_I2C_CON_REG)
- ~(OMAP_I2C_CON_EN));
-
- omap_i2c_write_reg(dev, OMAP_I2C_SYSC_REG, SYSC_SOFTRESET_MASK);
- /* For some reason we need to set the EN bit before the
-* reset done bit gets set. */
- timeout = jiffies + OMAP_I2C_TIMEOUT;
- omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, OMAP_I2C_CON_EN);
- while (!(omap_i2c_read_reg(dev, OMAP_I2C_SYSS_REG)
-SYSS_RESETDONE_MASK)) {
- if (time_after(jiffies, timeout)) {
- dev_warn(dev-dev, timeout waiting
- for controller reset\n);
- return -ETIMEDOUT;
- }
- msleep(1);
- }
-
- /* SYSC register is cleared by the reset; rewrite it */
- if (dev-rev == OMAP_I2C_REV_ON_2430) {
-
- omap_i2c_write_reg(dev, OMAP_I2C_SYSC_REG,
- SYSC_AUTOIDLE_MASK);
-
- } else if (dev-rev = OMAP_I2C_REV_ON_3430) {
- dev-syscstate = SYSC_AUTOIDLE_MASK;
- dev-syscstate |= SYSC_ENAWAKEUP_MASK;
- dev-syscstate |= (SYSC_IDLEMODE_SMART
- __ffs(SYSC_SIDLEMODE_MASK));
- dev-syscstate |= (SYSC_CLOCKACTIVITY_FCLK
- __ffs(SYSC_CLOCKACTIVITY_MASK));
-
- omap_i2c_write_reg(dev, OMAP_I2C_SYSC_REG,
- dev-syscstate);
- /*
-* Enabling all wakup sources to stop I2C freezing on
-* WFI instruction.
-* REVISIT: Some wkup sources might not be needed.
-*/
- dev-westate = OMAP_I2C_WE_ALL;
- if (dev-rev OMAP_I2C_REV_ON_3530_4430)
- omap_i2c_write_reg(dev, OMAP_I2C_WE_REG,
- dev-westate);
-
[PATCHv5 0/3] I2C driver updates
The series attempts to do the following - The reset should not be done in the driver have support for the same. - Remove the sysc register access in the driver. version 2 - Fix the indentation. - Restore in the error path is not needed as we are doing a init. version 3 - Combine the patch 1 and 2 as i2c-omap.h isn't a generic header version 4/5 - Making the changelogs more descriptive. - Rebasing to the wip/i2c branch Acknowledge Balaji ,Santosh and Kevin for the review comments. Tested on sdp4430 and on omap3430 Has dependency on the following patch https://patchwork.kernel.org/patch/904582/ Shubhrajyoti D (3): OMAP: I2C: Reset support OMAP: I2C: Remove the reset in the init path OMAP: I2C: Remove the SYSC register definition arch/arm/plat-omap/i2c.c | 18 drivers/i2c/busses/i2c-omap.c | 88 ++-- include/linux/i2c-omap.h |1 + 3 files changed, 41 insertions(+), 66 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4 REPOST 18/20] gpio/omap: use pm-runtime framework
[...] Looking at omap_gpio_mod_init() it seems like much of its processing could probably be done once at probe time (or at pm_runtime_get_sync time) as well, namely setting the IRQ enable masks. This must be called at the beginning of bank access to get reset state. Otherwise, it would contain settings related to previous bank access. What state may have changed between the last time a pin was released from the bank and the next time a pin is requested in the bank? Prior to this patch, omap_gpio_mod_init() is called once at probe time, and initializing irqenable masks etc. isn't reset at the beginning of bank access, if I understand correctly. Call to *_gpio_mod_init() is overhead if a client driver request/release same pin(s) and no other pins in the same bank are used by others. This is just a special case. We normally expect multiple multiple clients Access pins within the bank. Here *_mod_init() would be called just once. If a bank was used temporarily just during boot it is fair to reset it Using *_mod_init() by new client driver. If it's not actually necessary to do this on each first request to the bank then it's not a large overhead or anything, but want to make sure the PM operations being performed are correct. This patch is basically to runtime PM enable the GPIO bank when a pin is requested, and disable when no pin is requested. If other actions beyond the runtime PM enable are needed, now that a runtime PM disable is added, then it calls into question whether the enable method is missing something. More troubling are the PM actions performed in addition to pm_runtime_get_sync... Your concern is valid. But overhead is not likely from *_gpio_request/free() for reason explained. We have inherent overhead in runtime callbacks. But as I said, we can avoid Many of the operations when bank is accessed the first time. Ungating the interface clock, enabling wakeup, setting smart idle for the module, and enabling the (old-style) system clock seem like either one-time actions at probe, or a part of the pm_runtime_get_sync callback. Yes... sysconfig related settings are done by hwmod framework. Debounce clocks are enabled in callback. Or is there some other reason these power management actions should be taken each time a GPIO is requested in the block (when none were previously requested), after the runtime PM get_sync callback, but not as part of the get_sync callback? If so, what caused the smart idle setting to be lost, or the interface clock gated, if not the pm_runtime_put_sync? I am not sure which smart idle setting you are referring to. That's part of what the magic constant in this statement is doing: __raw_writew(0x0014, bank-base + OMAP1610_GPIO_SYSCONFIG); Ok, now this is moved to init and is called only once. Most stuffs done in *_get_sync() callback can skip first time. Omap_gpio_mod_init() does resetting irqenable settings to reset value. This should not be part of callback. To be clear, it seems like resetting irqenable settings should be a one-time action at probe time. The power management actions such as enabling debounce clocks, setting module PRCM idle protocol behavior, and ungating interface clocks seem like they should either be one-time probe actions, or a part of the runtime PM callbacks, or balanced with corresponding actions when the last pin in the bank is released. Else what caused the interface clock to gate, or the slave idle control to change, or the debounce clock to be cut? The only thing added here that would do that is the pm_runtime_put_sync. If it can cause those actions then do other calls to pm_runtime_get_sync need to fix these up? Debounce clk enable/disable has to be associated with *_get/put_sync(). Idle mode setting and interface clock gating is a one time operation. This is done during init. We can however look at optimization of the Callbacks. Anyhow, mainly wanted to point that out as a possible optimization. It's pretty unlikely there's an actual problem here. Ok, thanks. -- Tarun Todd -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4 REPOST 18/20] gpio/omap: use pm-runtime framework
[...] To be clear, it seems like resetting irqenable settings should be a one-time action at probe time. The power management actions such as Looking at it again, well we could probably avoid *_mod_init() in request. I will test and confirm. Thanks. -- Tarun [...] Anyhow, mainly wanted to point that out as a possible optimization. It's pretty unlikely there's an actual problem here. Todd -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v2]: Issues implementing clock handling mechanism within UART driver
On Fri, Jul 29, 2011 at 3:25 PM, Felipe Balbi ba...@ti.com wrote: Hi, Thanks for replying. On Thu, Jul 28, 2011 at 02:59:55PM +0530, Govindraj.R wrote: Proposal: 1. For the UART, follow the current approach of locking the console in Idle/Suspend path before cutting the clock but using pm_runtime_putsync. That is, continue using the prepare/resume Idle calls in idle path. I believe you should be using -prepare() to prevent that any other work on UART won't trigger a console_write() right now. Maybe only queueing the work for after -complete() or, maybe, just ignoring the work and loosing some prints, dunno. Yes true, for suspend path but for Idle path we don't have any such mechanism. 2. Other Approach would be adding conditions to debug prints from omap_device/ omap_hwmod/clock_framework avoid calling these debug prints for uart. Or Even a debug macro that would not debug prints if the context is from uart. yeah, that might work but then again, if we were using 8250.c, would we have this limitation ?? Its not necessary which driver we use, its about clock handling mechanism where it has to be done. if we add clock handling mechanism into the driver, from driver context handling clocks + managing the printks for the same uart port is a issue, due to reasons said earlier. I think that, maybe, your best call would be to capture console_write() calls into an internal temporary buffer on -prepare() and flush it once -complete() is called ? IIUC -prepare and - complete are only for suspend path when we do system wide suspend for normal idle path get_sync and put_sycn we dont have any -prepare or - complete where we can buffer the contents and flush later. BTW, where are the patches ? :-) I have done clock gating in idle path integrating irq chaining patches. hosted in gitorious here[1]. Was consolidating whether this approach is OK, or Are there any other approaches that I should consider? -- Thanks, Govindraj.R [1]: https://gitorious.org/runtime_3-0/runtime_3-0/commits/wip_irqchn -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHV2 3/5] OMAP: I2C: Remove the reset in the init path
On Thursday 21 July 2011 04:57 PM, Santosh Shilimkar wrote:
Thanks for your review.
On 7/21/2011 4:39 PM, Shubhrajyoti D wrote:
snip
+/*
+ * Enabling all wakup sources to stop I2C freezing on
+ * WFI instruction.
+ * REVISIT: Some wkup sources might not be needed.
+ */
Surely not related to your patch. But the 'REVISIT:' caught
my attention. Is the comment still valid.
Yes I will look and optimise the settings. Obviously all of them may not
be needed.
Will get back on this.
Also I see that we are not writing it for OMAP_I2C_REV_ON_3530_4430
I will send a patch correcting the same.
+dev-westate = OMAP_I2C_WE_ALL;
+if (dev-rev OMAP_I2C_REV_ON_3530_4430)
Space if (dev-rev OMAP_I2C_REV_ON_3530_4430)
+omap_i2c_write_reg(dev, OMAP_I2C_WE_REG,
+dev-westate);
}
omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, 0);
@@ -612,6 +572,11 @@ static int omap_i2c_xfer_msg(struct i2c_adapter
*adap,
return r;
if (r == 0) {
dev_err(dev-dev, controller timed out\n);
+if (dev-device_reset != NULL) {
+r = dev-device_reset(dev-dev);
+if (r 0)
ditto
+dev_err(dev-dev, reset failed\n);
+}
omap_i2c_init(dev);
return -ETIMEDOUT;
}
@@ -622,6 +587,11 @@ static int omap_i2c_xfer_msg(struct i2c_adapter
*adap,
/* We have an error */
if (dev-cmd_err (OMAP_I2C_STAT_AL | OMAP_I2C_STAT_ROVR |
You can fix this one as well.
OMAP_I2C_STAT_XUDF)) {
+if (dev-device_reset != NULL) {
+r = dev-device_reset(dev-dev);
+if (r 0)
here too.
+dev_err(dev-dev, reset failed\n);
+}
omap_i2c_init(dev);
return -EIO;
}
@@ -1024,6 +994,7 @@ omap_i2c_probe(struct platform_device *pdev)
if (pdata != NULL) {
speed = pdata-clkrate;
dev-set_mpu_wkup_lat = pdata-set_mpu_wkup_lat;
+dev-device_reset = pdata-device_reset;
} else {
speed = 100;/* Default speed */
dev-set_mpu_wkup_lat = NULL;
Regards
Santosh
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v2]: Issues implementing clock handling mechanism within UART driver
Hi, On Fri, Jul 29, 2011 at 04:54:44PM +0530, Govindraj wrote: On Fri, Jul 29, 2011 at 3:25 PM, Felipe Balbi ba...@ti.com wrote: Hi, Thanks for replying. On Thu, Jul 28, 2011 at 02:59:55PM +0530, Govindraj.R wrote: Proposal: 1. For the UART, follow the current approach of locking the console in Idle/Suspend path before cutting the clock but using pm_runtime_putsync. That is, continue using the prepare/resume Idle calls in idle path. I believe you should be using -prepare() to prevent that any other work on UART won't trigger a console_write() right now. Maybe only queueing the work for after -complete() or, maybe, just ignoring the work and loosing some prints, dunno. Yes true, for suspend path but for Idle path we don't have any such mechanism. aha... good point ;-) I have done clock gating in idle path integrating irq chaining patches. hosted in gitorious here[1]. Was consolidating whether this approach is OK, or Are there any other approaches that I should consider? why don't you also start queueing writes after the first call to runtime_suspend() and flush them after the first call runtime_resume()?? -- balbi signature.asc Description: Digital signature
[PATCH] OMAP4: I2C: Enable the wakeup in I2C_WE
Currently for OMAP4 the I2C_WE is not programmed. This patch enables the programming for OMAP4. Reported-by: Santosh Shilimkar santosh.shilim...@ti.com Signed-off-by: Shubhrajyoti D shubhrajy...@ti.com --- TODO: Currently all the wakeup sources are enabled. There is scope of optimising the same. Will revisit it. Rebased on Kevin's wip/i2c branch Tested on OMAP4430. drivers/i2c/busses/i2c-omap.c |5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index d05efe7..18cc0af 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -313,9 +313,8 @@ static int omap_i2c_init(struct omap_i2c_dev *dev) * REVISIT: Some wakeup sources might not be needed. */ dev-westate = OMAP_I2C_WE_ALL; - if (dev-rev OMAP_I2C_REV_ON_3530_4430) - omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, - dev-westate); + omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, + dev-westate); } omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, 0); -- 1.7.1 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v2]: Issues implementing clock handling mechanism within UART driver
On Fri, Jul 29, 2011 at 5:07 PM, Felipe Balbi ba...@ti.com wrote: Hi, On Fri, Jul 29, 2011 at 04:54:44PM +0530, Govindraj wrote: On Fri, Jul 29, 2011 at 3:25 PM, Felipe Balbi ba...@ti.com wrote: Hi, Thanks for replying. On Thu, Jul 28, 2011 at 02:59:55PM +0530, Govindraj.R wrote: Proposal: 1. For the UART, follow the current approach of locking the console in Idle/Suspend path before cutting the clock but using pm_runtime_putsync. That is, continue using the prepare/resume Idle calls in idle path. I believe you should be using -prepare() to prevent that any other work on UART won't trigger a console_write() right now. Maybe only queueing the work for after -complete() or, maybe, just ignoring the work and loosing some prints, dunno. Yes true, for suspend path but for Idle path we don't have any such mechanism. aha... good point ;-) I have done clock gating in idle path integrating irq chaining patches. hosted in gitorious here[1]. Was consolidating whether this approach is OK, or Are there any other approaches that I should consider? why don't you also start queueing writes after the first call to runtime_suspend() and flush them after the first call runtime_resume()?? Yes fine, But there are scenarios even before first runtime_suspend happens, ex: uart_port_configure - get_sync - pm_generic_runtime_resume (omap_device_enable in this case) debug printk - console_write - get_sync. there are numerous such scenarios where we end from runtime context to runtime api context again, or jumping from one uart port operation to uart print operation. So either we should not have those prints from pm_generic layers or suppress them(seems pretty much a problem for a clean design within the driver using console_lock/unlock for every get_sync, and for runtime_put we cannot suppress the prints as it gets scheduled later) or if other folks who really need those prints form pm_generic* layers to debug and analysis then we have no other choice rather control the clk_enable/disable from outside driver code in idle path. -- Thanks Govindraj.R *pm_generic layers for omap, referring to omap_device/omap_hwmod layers which does low level clock handling. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] OMAP4: I2C: Enable the wakeup in I2C_WE
Hi, On Fri, Jul 29, 2011 at 05:18:42PM +0530, Shubhrajyoti D wrote: Currently for OMAP4 the I2C_WE is not programmed. This patch enables the programming for OMAP4. Reported-by: Santosh Shilimkar santosh.shilim...@ti.com Signed-off-by: Shubhrajyoti D shubhrajy...@ti.com --- TODO: Currently all the wakeup sources are enabled. There is scope of optimising the same. Will revisit it. Rebased on Kevin's wip/i2c branch Tested on OMAP4430. drivers/i2c/busses/i2c-omap.c |5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index d05efe7..18cc0af 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -313,9 +313,8 @@ static int omap_i2c_init(struct omap_i2c_dev *dev) * REVISIT: Some wakeup sources might not be needed. */ dev-westate = OMAP_I2C_WE_ALL; - if (dev-rev OMAP_I2C_REV_ON_3530_4430) - omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, - dev-westate); + omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, + dev-westate); this is also enabling for 3530, have you tested there too ?? On the other hand, this looks like it's fixing a bogus change on commit a3a7acbcc3df4e9ecc12aa1fc435534d74ebbdf4 (I2C: OMAP2+: address confused probed version naming) specifically on the hunk below [1]: @@ -379,7 +379,9 @@ static int omap_i2c_init(struct omap_i2c_dev *dev) * REVISIT: Some wkup sources might not be needed. */ dev-westate = OMAP_I2C_WE_ALL; - omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, dev-westate); + if (dev-rev OMAP_I2C_REV_ON_3530_4430) + omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, + dev-westate); } } omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, 0); if that's the case, you should either update your commit log stating that this is a fix to a bug introduced by that commit, or fold this patch in the same. You should also Cc the patch author to clarify why the dev-rev check was added. Andy, can you clarify why you added the revision check which didn't exist before ? [1] http://git.kernel.org/?p=linux/kernel/git/khilman/linux-omap-pm.git;a=commitdiff;h=a3a7acbcc3df4e9ecc12aa1fc435534d74ebbdf4 -- balbi signature.asc Description: Digital signature
Re: [RFC v2]: Issues implementing clock handling mechanism within UART driver
Hi, On Fri, Jul 29, 2011 at 05:29:12PM +0530, Govindraj wrote: Yes fine, But there are scenarios even before first runtime_suspend happens, ex: uart_port_configure - get_sync - pm_generic_runtime_resume (omap_device_enable in this case) debug printk - console_write - get_sync. there are numerous such scenarios where we end from runtime context to runtime api context again, or jumping from one uart port operation to uart print operation. calling pm_runtime_get_sync() should not be a problem. It should only increase the usage counters... This sounds like a race condition on the driver, no ? What you're experiencing, if I understood correctly, is a deadlock ? In that case, can you try to track the locking mechanism on the omap-serial driver to try to find if there isn't anything obviously wrong ? So either we should not have those prints from pm_generic layers or suppress them(seems pretty much a problem for a clean design within the driver using console_lock/unlock for every get_sync, and for runtime_put we cannot suppress the prints as it gets scheduled later) or if other folks who really need those prints form pm_generic* layers to debug and analysis then we have no other choice rather control the clk_enable/disable from outside driver code in idle path. yeah, none of these would be nice :-( I think this needs more debugging to be sure what's exactly going on. What's exactly causing the deadlock ? Which lock is held and never released ? -- balbi signature.asc Description: Digital signature
Re: [PATCH] OMAP4: I2C: Enable the wakeup in I2C_WE
Hi, On Fri, Jul 29, 2011 at 01:28:12PM +0100, Andy Green (林安廸) wrote: On 07/29/2011 01:07 PM, Somebody in the thread at some point said: Hi - - omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, dev-westate); + if (dev-rev OMAP_I2C_REV_ON_3530_4430) + omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, + dev-westate); Andy, can you clarify why you added the revision check which didn't exist before ? [1] http://git.kernel.org/?p=linux/kernel/git/khilman/linux-omap-pm.git;a=commitdiff;h=a3a7acbcc3df4e9ecc12aa1fc435534d74ebbdf4 At the time I wrote the patches back in March, the code there was different: there was a pre-extant test avoiding that line on 4430, and the patch is simply converting it to the new scheme. You can see it here: http://permalink.gmane.org/gmane.linux.ports.arm.omap/54940 @@ -379,7 +379,7 @@ static int omap_i2c_init(struct omap_i2c_dev *dev) * REVISIT: Some wkup sources might not be needed. */ dev-westate = OMAP_I2C_WE_ALL; - if (dev-rev OMAP_I2C_REV_ON_4430) + if (dev-rev OMAP_I2C_REV_ON_3530_4430) omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, dev-westate); } I guess since March and before this got committed for 3.1, someone got a patch in first removing the test, so when my patchset was uplevelled for commit against 3.1-rc this conflict was dealt with by re-introducing the test. Long story short, it's there from me as a mechanical 1:1 renaming action as part of the fix that 3530 and 4430 (different) IPs return the same rev number. Despite how it now looks I didn't add it, so if Shubhrajyoti has reasons to think it should be gone again I have nothing against that at all. yeah, looks like a bad conflict resolution. Shubhrajyoti, care to respin the patch and update commit log stating that it is fixing a bad conflict resolution or something ? -- balbi signature.asc Description: Digital signature
[PATCH v2] OMAP3: NAND: Adding NAND support and specifying NAND partitions.
This patch adds the NAND support on OMAP3EVM board and also allocates
five partitions on NAND.
Referred to file: arch/arm/mach-omap2/board-omap3beagle.c
Signed-off-by: Vaibhav Hiremath hvaib...@ti.com
Signed-off-by: Sanjeev Premi pr...@ti.com
Signed-off-by: Hrishikesh Bhandiwad hrishikes...@ti.com
---
arch/arm/mach-omap2/board-omap3evm.c | 37 ++
1 files changed, 37 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-omap2/board-omap3evm.c
b/arch/arm/mach-omap2/board-omap3evm.c
index b4d4346..3ac96d4 100644
--- a/arch/arm/mach-omap2/board-omap3evm.c
+++ b/arch/arm/mach-omap2/board-omap3evm.c
@@ -30,6 +30,9 @@
#include linux/usb/otg.h
#include linux/smsc911x.h
+#include linux/mtd/partitions.h
+#include linux/mtd/nand.h
+
#include linux/wl12xx.h
#include linux/regulator/fixed.h
#include linux/regulator/machine.h
@@ -101,6 +104,37 @@ static void __init omap3_evm_get_revision(void)
}
}
+static struct mtd_partition omap3evm_nand_partitions[] = {
+ /* All the partition sizes are listed in terms of NAND block size */
+ {
+ .name = X-Loader,
+ .offset = 0,
+ .size = 4 * NAND_BLOCK_SIZE,
+ .mask_flags = MTD_WRITEABLE,/* force read-only */
+ },
+ {
+ .name = U-Boot,
+ .offset = 0x8,
+ .size = 15 * NAND_BLOCK_SIZE,
+ .mask_flags = MTD_WRITEABLE,/* force read-only */
+ },
+ {
+ .name = U-Boot Env,
+ .offset = 0x26,
+ .size = 1 * NAND_BLOCK_SIZE,
+ },
+ {
+ .name = Kernel,
+ .offset = 0x28,
+ .size = 32 * NAND_BLOCK_SIZE,
+ },
+ {
+ .name = File System,
+ .offset = 0x68,
+ .size = MTDPART_SIZ_FULL,
+ },
+};
+
#if defined(CONFIG_SMSC911X) || defined(CONFIG_SMSC911X_MODULE)
#include plat/gpmc-smsc911x.h
@@ -696,6 +730,9 @@ static void __init omap3_evm_init(void)
omap_serial_init();
+ omap_nand_flash_init(NAND_BUSWIDTH_16, omap3evm_nand_partitions,
+ ARRAY_SIZE(omap3evm_nand_partitions));
+
/* OMAP3EVM uses ISP1504 phy and so register nop transceiver */
usb_nop_xceiv_register();
--
1.6.2.4
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] OMAP4: I2C: Enable the wakeup in I2C_WE
On 07/29/2011 01:07 PM, Somebody in the thread at some point said: Hi - - omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, dev-westate); + if (dev-rev OMAP_I2C_REV_ON_3530_4430) + omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, + dev-westate); Andy, can you clarify why you added the revision check which didn't exist before ? [1] http://git.kernel.org/?p=linux/kernel/git/khilman/linux-omap-pm.git;a=commitdiff;h=a3a7acbcc3df4e9ecc12aa1fc435534d74ebbdf4 At the time I wrote the patches back in March, the code there was different: there was a pre-extant test avoiding that line on 4430, and the patch is simply converting it to the new scheme. You can see it here: http://permalink.gmane.org/gmane.linux.ports.arm.omap/54940 @@ -379,7 +379,7 @@ static int omap_i2c_init(struct omap_i2c_dev *dev) * REVISIT: Some wkup sources might not be needed. */ dev-westate = OMAP_I2C_WE_ALL; - if (dev-rev OMAP_I2C_REV_ON_4430) + if (dev-rev OMAP_I2C_REV_ON_3530_4430) omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, dev-westate); } I guess since March and before this got committed for 3.1, someone got a patch in first removing the test, so when my patchset was uplevelled for commit against 3.1-rc this conflict was dealt with by re-introducing the test. Long story short, it's there from me as a mechanical 1:1 renaming action as part of the fix that 3530 and 4430 (different) IPs return the same rev number. Despite how it now looks I didn't add it, so if Shubhrajyoti has reasons to think it should be gone again I have nothing against that at all. -Andy -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v2]: Issues implementing clock handling mechanism within UART driver
On Fri, Jul 29, 2011 at 5:49 PM, Felipe Balbi ba...@ti.com wrote: Hi, On Fri, Jul 29, 2011 at 05:29:12PM +0530, Govindraj wrote: Yes fine, But there are scenarios even before first runtime_suspend happens, ex: uart_port_configure - get_sync - pm_generic_runtime_resume (omap_device_enable in this case) debug printk - console_write - get_sync. there are numerous such scenarios where we end from runtime context to runtime api context again, or jumping from one uart port operation to uart print operation. calling pm_runtime_get_sync() should not be a problem. It should only increase the usage counters... This sounds like a race condition on the driver, no ? Actually when we call a API to enable clocks we except the internals of API to just enable clocks and return. *Clock enable API should not cause or trigger to do a _device_driver_operation_ even before enabling clocks of the device-driver which called it* for uart context get_sync can land me to uart driver back even before enabling the uart clocks due to printks. What you're experiencing, if I understood correctly, is a deadlock ? In that case, can you try to track the locking mechanism on the omap-serial driver to try to find if there isn't anything obviously wrong ? Yes deadlocks. due to entering from runtime context to runtime context or entering from uart_port_operation to uart_console_write ops. There are already port locks used extensively within the uart driver to secure a port operation. But cannot secure a port operation while using clock_enable API. since clock enable API can land the control back to uart_console_write operation.. So either we should not have those prints from pm_generic layers or suppress them(seems pretty much a problem for a clean design within the driver using console_lock/unlock for every get_sync, and for runtime_put we cannot suppress the prints as it gets scheduled later) or if other folks who really need those prints form pm_generic* layers to debug and analysis then we have no other choice rather control the clk_enable/disable from outside driver code in idle path. yeah, none of these would be nice :-( I think this needs more debugging to be sure what's exactly going on. What's exactly causing the deadlock ? Which lock is held and never released ? I had done some investigations, from scenarios it simply boils down to fact to handle clock within uart driver, uart driver expects clock enable API* used to just enable uart clocks but rather not trigger a _uart_ops_ within which kind of unacceptable from the uart_driver context. -- Thanks, Govindraj.R *clock enable API can be any API used to enable clocks like get_sync/ omap_device_enable/clock_enable etc. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-pm] Runtime PM discussion notes
Hi! Actually, it just occurred to me that if we're waiting for a system timer and can hand that off to a suitable timer in the PMIC then we can do a suspend to RAM for the deep idle state from the hardware point of view. Yep. At LinuxCon Cambridge two years ago, we had a discussion about whether it would be possible to enter ACPI S-states from CPUIdle (or some idle governor) on Intel chips. If I remember correctly, the conclusion was that ACPI always disables the screen/backlight, so it would only be useful for situations where that was acceptable. Well, auto suspending when screensaver is active would still be useful. (And IIRC some machines kept screen on when in S-state unless driver powered it down... but that might be S1. The reason why you can't enter ACPI S-states from CPUidle is because you need to go out of the idle loop to execute some ACPI-specific stuff. Which is not even specific to Intel chips, but to ACPI in general. The code was little tricky/unclean, but it worked for me at one point... I called it sleepy linux. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] OMAP4: I2C: Enable the wakeup in I2C_WE
On Friday 29 July 2011 06:07 PM, Felipe Balbi wrote: Hi, On Fri, Jul 29, 2011 at 01:28:12PM +0100, Andy Green (林安廸) wrote: On 07/29/2011 01:07 PM, Somebody in the thread at some point said: Hi - - omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, dev-westate); + if (dev-rev OMAP_I2C_REV_ON_3530_4430) + omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, + dev-westate); Andy, can you clarify why you added the revision check which didn't exist before ? [1] http://git.kernel.org/?p=linux/kernel/git/khilman/linux-omap-pm.git;a=commitdiff;h=a3a7acbcc3df4e9ecc12aa1fc435534d74ebbdf4 At the time I wrote the patches back in March, the code there was different: there was a pre-extant test avoiding that line on 4430, and the patch is simply converting it to the new scheme. You can see it here: http://permalink.gmane.org/gmane.linux.ports.arm.omap/54940 @@ -379,7 +379,7 @@ static int omap_i2c_init(struct omap_i2c_dev *dev) * REVISIT: Some wkup sources might not be needed. */ dev-westate = OMAP_I2C_WE_ALL; - if (dev-rev OMAP_I2C_REV_ON_4430) + if (dev-rev OMAP_I2C_REV_ON_3530_4430) omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, dev-westate); } I guess since March and before this got committed for 3.1, someone got a patch in first removing the test, so when my patchset was uplevelled for commit against 3.1-rc this conflict was dealt with by re-introducing the test. Long story short, it's there from me as a mechanical 1:1 renaming action as part of the fix that 3530 and 4430 (different) IPs return the same rev number. Despite how it now looks I didn't add it, so if Shubhrajyoti has reasons to think it should be gone again I have nothing against that at all. yeah, looks like a bad conflict resolution. Shubhrajyoti, care to respin the patch and update commit log stating that it is fixing a bad conflict resolution or something ? I wasn't aware of the conflict resolution part. Actually came across this piece of code as per the discussion on the reset implementation patch will update the changelogs. How about, From: Shubhrajyoti Dshubhrajy...@ti.com Currently for OMAP4 the I2C_WE is not programmed. This patch enables the programming for OMAP4. Fixes a conflict resolution of Andy's patches. Reported-by: Santosh Shilimkarsantosh.shilim...@ti.com Signed-off-by: Shubhrajyoti Dshubhrajy...@ti.com --- TODO: Currently all the wakeup sources are enabled. There is scope of optimising the same. Will revisit it. Rebased on Kevin's wip/i2c branch Tested on OMAP4430. drivers/i2c/busses/i2c-omap.c |5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index d05efe7..18cc0af 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -313,9 +313,8 @@ static int omap_i2c_init(struct omap_i2c_dev *dev) * REVISIT: Some wakeup sources might not be needed. */ dev-westate = OMAP_I2C_WE_ALL; - if (dev-rev OMAP_I2C_REV_ON_3530_4430) - omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, - dev-westate); + omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, + dev-westate); } omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, 0); -- 1.7.1 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 2/7] OMAP3: beagle: don't touch omap_device internals
On 08:31-20110728, Menon, Nishanth wrote:
On Thu, Jul 28, 2011 at 07:57, Cousson, Benoit b-cous...@ti.com wrote:
[...]
diff --git a/arch/arm/mach-omap2/omap_hwmod.c
b/arch/arm/mach-omap2/omap_hwmod.c
index 293fa6c..77d01a2 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -142,6 +142,7 @@
#include powerdomain.h
#includeplat/clock.h
#includeplat/omap_hwmod.h
+#includeplat/omap_device.h
I'd rather put that code inside the omap_device.c instead of here.
The omap_device layer is on top of the omap_hwmod.
In order to minimize the dependencies from the low HW description layer to
the omap_device layer, you should maybe define a omap_device_from_hwmod()
function or something similar.
Thanks for the review. will check on this.
That being said, do we really need to get the device from the hwmod name?
Cannot we use the device name instead?
I do not know all the usecases, that why I'm asking.
mpu.0 , are the device names - which probably lets me walk the kernel
data structrues instead of omap database to get to the right device,
Vs using the common names like mpu make things a little easier to
deal with from driver perspective.
as an example, some of the dev_driver_string(dev):dev_name(dev) (in
bracket hwmod name) I collected from OMAP4 are:
platform:mpu.0 (mpu)
platform:l3_main_1.0 ('l3_main_1)
pvrsrvkm:pvrsrvkm.0 (gpu)
rpres:fdif.0 (fdif)
omap_hsi:omap_hsi.0 (hsi)
platform:iss.0 (iss)
etc..
I mean I have'nt been keeping track of the device tree discussions so
dont know if this function could be better done - but I think I agree
with the overall idea that instead of spawning off get_xyz_device() we
need to have some uniform approach to get to the device scaling
silicon - I hoped we could consider the hwmod database/what ever
replacing it to do that.
following are a couple of reference patches how this could be done
with omap_device
From f03490456e24f72ca5272303c95a6f0b212494d5 Mon Sep 17 00:00:00 2001
From: Nishanth Menon n...@ti.com
Date: Wed, 27 Jul 2011 15:02:32 -0500
Subject: [PATCH 1/2] OMAP: PM: omap_device: add omap_hwmod_name_get_odev
An API which translates a standard hwmod name to corresponding
omap_device is useful for drivers when they need to look up the
device associated with a hwmod name to map back into the device
structure pointers. These ideally should be used by drivers in
mach directory. Using a generic hwmod name like gpu instead of
the actual device name which could change in the future, allows
us to:
a) Could in effect help replace apis such as omap2_get_mpuss_device,
omap2_get_iva_device, omap2_get_l3_device, omap4_get_dsp_device,
etc..
b) Scale to more devices rather than be restricted to named functions
c) Simplify driver's platform_data from passing additional fields
all doing the same thing with different function pointer names
just for accessing a different device name.
Change-Id: Ib42d22b4a929982c87a79866e3d7dc6e41073a6c
Signed-off-by: Nishanth Menon n...@ti.com
---
arch/arm/plat-omap/include/plat/omap_device.h |1 +
arch/arm/plat-omap/omap_device.c | 32 +
2 files changed, 33 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 70d31d0..7a3c046 100644
--- a/arch/arm/plat-omap/include/plat/omap_device.h
+++ b/arch/arm/plat-omap/include/plat/omap_device.h
@@ -102,6 +102,7 @@ int omap_device_register(struct omap_device *od);
int omap_early_device_register(struct omap_device *od);
void __iomem *omap_device_get_rt_va(struct omap_device *od);
+struct omap_device *omap_hwmod_name_get_odev(const char *oh_name);
/* OMAP PM interface */
int omap_device_align_pm_lat(struct platform_device *pdev,
diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
index 92b4496..21df532 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -780,6 +780,38 @@ void __iomem *omap_device_get_rt_va(struct omap_device *od)
return omap_hwmod_get_mpu_rt_va(od-hwmods[0]);
}
+/**
+ * omap_hwmod_name_get_odev() - convert a hwmod name to omap_device pointer
+ * @oh_name: name of the hwmod device
+ *
+ * returns back a struct omap_device * pointer associated with a hwmod
+ * device represented by a hwmod_name
+ */
+struct omap_device *omap_hwmod_name_get_odev(const char *oh_name)
+{
+ struct omap_hwmod *oh;
+
+ if (!oh_name) {
+ WARN(1, %s: no hwmod name!\n, __func__);
+ return ERR_PTR(-EINVAL);
+ }
+
+ oh = omap_hwmod_lookup(oh_name);
+ if (IS_ERR_OR_NULL(oh)) {
+ WARN(1, %s: no hwmod for %s\n, __func__,
+ oh_name);
+ return ERR_PTR(oh ? PTR_ERR(oh) : -ENODEV);
+ }
+ if (IS_ERR_OR_NULL(oh-od)) {
+ WARN(1, %s: no omap_device for %s\n, __func__,
+
Re: [RFC v2]: Issues implementing clock handling mechanism within UART driver
Hi,
On Fri, Jul 29, 2011 at 06:28:32PM +0530, Govindraj wrote:
On Fri, Jul 29, 2011 at 5:49 PM, Felipe Balbi ba...@ti.com wrote:
On Fri, Jul 29, 2011 at 05:29:12PM +0530, Govindraj wrote:
Yes fine, But there are scenarios even before first runtime_suspend
happens,
ex: uart_port_configure - get_sync - pm_generic_runtime_resume
(omap_device_enable in this case) debug printk - console_write -
get_sync.
there are numerous such scenarios where we end from runtime context
to runtime api context again, or jumping from one uart port operation
to uart print operation.
calling pm_runtime_get_sync() should not be a problem. It should only
increase the usage counters... This sounds like a race condition on the
driver, no ?
Actually when we call a API to enable clocks we except the internals of API
to just enable clocks and return.
*Clock enable API should not cause or trigger to do a
_device_driver_operation_
even before enabling clocks of the device-driver which called it*
for uart context get_sync can land me to uart driver back
even before enabling the uart clocks due to printks.
only if _you_ have prints or _your_ runtime_*() calls, no ?
Let's say omap_hwmod.c wants to do a print:
- printk()
- pm_runtime_get_sync
- console_write
- pm_runtim_put
now, if you have a printk() on your runtime_resume() before you enable
the clocks, then I can see why you would deadlock:
- pm_runtime_get_sync
- omap_serial_runtime_resume
- printk
- pm_runtime_get_sync
- omap_serial_runtime_resume
- printk
- pm_runtime_get_sync
.
maybe I'm missing something, but can you add a stack dump on your
-runtime_resume and -runtime_suspend methods, just so we try to figure
out who's to blame here ?
What you're experiencing, if I understood correctly, is a deadlock ? In
that case, can you try to track the locking mechanism on the omap-serial
driver to try to find if there isn't anything obviously wrong ?
Yes deadlocks. due to entering from runtime context to runtime context
or entering from uart_port_operation to uart_console_write ops.
There are already port locks used extensively within the uart driver
to secure a port operation.
But cannot secure a port operation while using clock_enable API.
since clock enable API can land the control back to uart_console_write
operation..
but in that case, if clock isn't enabled, why don't you just ignore the
print and enable the clock ?? Just return 0 and continue with
clk_enable() ??
So either we should not have those prints from pm_generic layers or
suppress
them(seems pretty much a problem for a clean design within the driver
using console_lock/unlock for every get_sync, and for
runtime_put we cannot suppress the prints as it gets scheduled later)
or if other folks who really need those prints form pm_generic* layers
to debug and analysis then we have no other choice rather control
the clk_enable/disable from outside driver code in idle path.
yeah, none of these would be nice :-(
I think this needs more debugging to be sure what's exactly going on.
What's exactly causing the deadlock ? Which lock is held and never
released ?
I had done some investigations, from scenarios it simply boils down to fact
to handle clock within uart driver, uart driver expects clock enable API* used
to just enable uart clocks but rather not trigger a _uart_ops_ within which
kind of unacceptable from the uart_driver context.
ok, now I see what you mean:
113 static int _omap_device_activate(struct omap_device *od, u8 ignore_lat)
114 {
115 struct timespec a, b, c;
116
117 pr_debug(omap_device: %s: activating\n, od-pdev.name);
118
119 while (od-pm_lat_level 0) {
120 struct omap_device_pm_latency *odpl;
121 unsigned long long act_lat = 0;
122
123 od-pm_lat_level--;
124
125 odpl = od-pm_lats + od-pm_lat_level;
126
127 if (!ignore_lat
128 (od-dev_wakeup_lat = od-_dev_wakeup_lat_limit))
129 break;
130
131 read_persistent_clock(a);
132
133 /* XXX check return code */
134 odpl-activate_func(od);
135
136 read_persistent_clock(b);
137
138 c = timespec_sub(b, a);
139 act_lat = timespec_to_ns(c);
140
141 pr_debug(omap_device: %s: pm_lat %d: activate: elapsed
time
142 %llu nsec\n, od-pdev.name, od-pm_lat_level,
143 act_lat);
144
145 if (act_lat odpl-activate_lat) {
146 odpl-activate_lat_worst = act_lat;
147 if (odpl-flags OMAP_DEVICE_LATENCY_AUTO_ADJUST) {
148 odpl-activate_lat = act_lat;
149 pr_warning(omap_device: %s.%d: new
Re: [PATCH] OMAP4: I2C: Enable the wakeup in I2C_WE
Hi, On Fri, Jul 29, 2011 at 07:11:34PM +0530, Shubhrajyoti wrote: On Friday 29 July 2011 06:07 PM, Felipe Balbi wrote: Hi, On Fri, Jul 29, 2011 at 01:28:12PM +0100, Andy Green (林安廸) wrote: On 07/29/2011 01:07 PM, Somebody in the thread at some point said: Hi - - omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, dev-westate); + if (dev-rev OMAP_I2C_REV_ON_3530_4430) + omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, + dev-westate); Andy, can you clarify why you added the revision check which didn't exist before ? [1] http://git.kernel.org/?p=linux/kernel/git/khilman/linux-omap-pm.git;a=commitdiff;h=a3a7acbcc3df4e9ecc12aa1fc435534d74ebbdf4 At the time I wrote the patches back in March, the code there was different: there was a pre-extant test avoiding that line on 4430, and the patch is simply converting it to the new scheme. You can see it here: http://permalink.gmane.org/gmane.linux.ports.arm.omap/54940 @@ -379,7 +379,7 @@ static int omap_i2c_init(struct omap_i2c_dev *dev) * REVISIT: Some wkup sources might not be needed. */ dev-westate = OMAP_I2C_WE_ALL; - if (dev-rev OMAP_I2C_REV_ON_4430) + if (dev-rev OMAP_I2C_REV_ON_3530_4430) omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, dev-westate); } I guess since March and before this got committed for 3.1, someone got a patch in first removing the test, so when my patchset was uplevelled for commit against 3.1-rc this conflict was dealt with by re-introducing the test. Long story short, it's there from me as a mechanical 1:1 renaming action as part of the fix that 3530 and 4430 (different) IPs return the same rev number. Despite how it now looks I didn't add it, so if Shubhrajyoti has reasons to think it should be gone again I have nothing against that at all. yeah, looks like a bad conflict resolution. Shubhrajyoti, care to respin the patch and update commit log stating that it is fixing a bad conflict resolution or something ? I wasn't aware of the conflict resolution part. Actually came across this piece of code as per the discussion on the reset implementation patch will update the changelogs. How about, From: Shubhrajyoti Dshubhrajy...@ti.com Currently for OMAP4 the I2C_WE is not programmed. This patch enables the programming for OMAP4. Fixes a conflict resolution of Andy's patches. I think you need to be a bit more verbose here ;-) Describe what happened and point to the commit number and mailing list archives for references. Imagine someone else reads this commit half a year from now, will s/he have enough information to understand the background of this patch ? -- balbi signature.asc Description: Digital signature
Re: [RFC/PATCH 2/7] OMAP3: beagle: don't touch omap_device internals
hi,
On Fri, Jul 29, 2011 at 08:49:34AM -0500, Nishanth Menon wrote:
From f03490456e24f72ca5272303c95a6f0b212494d5 Mon Sep 17 00:00:00 2001
From: Nishanth Menon n...@ti.com
Date: Wed, 27 Jul 2011 15:02:32 -0500
Subject: [PATCH 1/2] OMAP: PM: omap_device: add omap_hwmod_name_get_odev
An API which translates a standard hwmod name to corresponding
omap_device is useful for drivers when they need to look up the
device associated with a hwmod name to map back into the device
structure pointers. These ideally should be used by drivers in
mach directory. Using a generic hwmod name like gpu instead of
the actual device name which could change in the future, allows
us to:
a) Could in effect help replace apis such as omap2_get_mpuss_device,
omap2_get_iva_device, omap2_get_l3_device, omap4_get_dsp_device,
etc..
b) Scale to more devices rather than be restricted to named functions
c) Simplify driver's platform_data from passing additional fields
all doing the same thing with different function pointer names
just for accessing a different device name.
Change-Id: Ib42d22b4a929982c87a79866e3d7dc6e41073a6c
Signed-off-by: Nishanth Menon n...@ti.com
---
arch/arm/plat-omap/include/plat/omap_device.h |1 +
arch/arm/plat-omap/omap_device.c | 32
+
2 files changed, 33 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 70d31d0..7a3c046 100644
--- a/arch/arm/plat-omap/include/plat/omap_device.h
+++ b/arch/arm/plat-omap/include/plat/omap_device.h
@@ -102,6 +102,7 @@ int omap_device_register(struct omap_device *od);
int omap_early_device_register(struct omap_device *od);
void __iomem *omap_device_get_rt_va(struct omap_device *od);
+struct omap_device *omap_hwmod_name_get_odev(const char *oh_name);
/* OMAP PM interface */
int omap_device_align_pm_lat(struct platform_device *pdev,
diff --git a/arch/arm/plat-omap/omap_device.c
b/arch/arm/plat-omap/omap_device.c
index 92b4496..21df532 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -780,6 +780,38 @@ void __iomem *omap_device_get_rt_va(struct omap_device
*od)
return omap_hwmod_get_mpu_rt_va(od-hwmods[0]);
}
+/**
+ * omap_hwmod_name_get_odev() - convert a hwmod name to omap_device pointer
+ * @oh_name: name of the hwmod device
+ *
+ * returns back a struct omap_device * pointer associated with a hwmod
+ * device represented by a hwmod_name
+ */
+struct omap_device *omap_hwmod_name_get_odev(const char *oh_name)
+{
+ struct omap_hwmod *oh;
+
+ if (!oh_name) {
+ WARN(1, %s: no hwmod name!\n, __func__);
+ return ERR_PTR(-EINVAL);
+ }
+
+ oh = omap_hwmod_lookup(oh_name);
+ if (IS_ERR_OR_NULL(oh)) {
+ WARN(1, %s: no hwmod for %s\n, __func__,
+ oh_name);
+ return ERR_PTR(oh ? PTR_ERR(oh) : -ENODEV);
+ }
+ if (IS_ERR_OR_NULL(oh-od)) {
+ WARN(1, %s: no omap_device for %s\n, __func__,
+ oh_name);
+ return ERR_PTR(oh-od ? PTR_ERR(oh-od) : -ENODEV);
+ }
+
+ return oh-od;
+}
+EXPORT_SYMBOL(omap_hwmod_name_get_odev);
maybe EXPORT_SYMBOL_GPL() ?? Not sure we want non-GPL code to access
this ;-)
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH] OMAP4: I2C: Enable the wakeup in I2C_WE
Hi, On Fri, Jul 29, 2011 at 07:33:39PM +0530, Datta, Shubhrajyoti wrote: On Fri, Jul 29, 2011 at 7:11 PM, Shubhrajyoti [1]shubhrajy...@ti.com wrote: On Friday 29 July 2011 06:07 PM, Felipe Balbi wrote: Hi, On Fri, Jul 29, 2011 at 01:28:12PM +0100, Andy Green (林安廸) wrote: On 07/29/2011 01:07 PM, Somebody in the thread at some point said: Hi - - omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, dev-westate); + if (dev-rev OMAP_I2C_REV_ON_3530_4430) + omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, + dev-westate); Andy, can you clarify why you added the revision check which didn't exist before ? [1] [2]http://git.kernel.org/?p=linux/kernel/git/khilman/linux-omap-pm.git;a=commitdiff;h=a3a7acbcc3df4e9ecc12aa1fc435534d74ebbdf4 At the time I wrote the patches back in March, the code there was different: there was a pre-extant test avoiding that line on 4430, and the patch is simply converting it to the new scheme. You can see it here: [3]http://permalink.gmane.org/gmane.linux.ports.arm.omap/54940 @@ -379,7 +379,7 @@ static int omap_i2c_init(struct omap_i2c_dev *dev) * REVISIT: Some wkup sources might not be needed. */ dev-westate = OMAP_I2C_WE_ALL; - if (dev-rev OMAP_I2C_REV_ON_4430) + if (dev-rev OMAP_I2C_REV_ON_3530_4430) omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, dev-westate); } I guess since March and before this got committed for 3.1, someone got a patch in first removing the test, so when my patchset was uplevelled for commit against 3.1-rc this conflict was dealt with by re-introducing the test. Long story short, it's there from me as a mechanical 1:1 renaming action as part of the fix that 3530 and 4430 (different) IPs return the same rev number. Despite how it now looks I didn't add it, so if Shubhrajyoti has reasons to think it should be gone again I have nothing against that at all. yeah, looks like a bad conflict resolution. Shubhrajyoti, care to respin the patch and update commit log stating that it is fixing a bad conflict resolution or something ? I wasn't aware of the conflict resolution part. Actually came across this piece of code as per the discussion on the reset implementation patch will update the changelogs. How about, Earlier mail got corrupted resending this is much worse. What mail client are you using ? Maybe there are some tips on Documentation/email-clients.txt -- balbi signature.asc Description: Digital signature
Re: [RFC/PATCH v3 00/13] PM QoS: add a per-device latency constraints class
On Fri, Jul 29, 2011 at 10:37:52AM +0200, Jean Pihet wrote: Mark, On Thu, Jul 28, 2011 at 3:14 PM, mark gross markgr...@thegnar.org wrote: On Thu, Jul 28, 2011 at 10:30:07AM +0200, jean.pi...@newoldbits.com wrote: From: Jean Pihet j-pi...@ti.com This patch set is in an RFC state, for review and comments. High level implementation: ... 7. Misc fixes to improve code readability: . rename of the PM QoS implementation file from pm_qos_params.[ch] to pm_qos.[ch] I picked the name for the file as pm_qos_params over pm_qos because I wanted to make it implicitly clear that this was not an full QOS implementation. True QOS carries expectations similar to real time and as the infrastructure is closer to good intentioned than even best effort and offers no notification when the QOS request is not able to be met and really doesn't implement a true QOS at all, (it just provides the parameter interface for part of one its missing the notification interface when the service level is not met and I think a few other things.) So I wanted to have it named a bit different from just pm_qos. This said I'm not supper attached to the naming of the modules. If folks want to change it I wouldn't complain (too much anyway;). Ok got the idea. I do not know what name to chose though. As suggested previously the name pm_qos_params does not reflect the implementation, that is why I renamed it. I must have missed the part where the name doesn't reflect the implementation was talked about. I look at the interface and I see parameters all over the place and a small bit of notification. --mark. --mark PS I'll look at the rest of the patches tomorrow, this time for real as I'm about to have more free time to focus on non-work stuff :) Thanks you for reviewing! FWIW this write up sounds interesting. Hope it is readable ;p Regards, Jean -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v2]: Issues implementing clock handling mechanism within UART driver
On Fri, Jul 29, 2011 at 7:32 PM, Felipe Balbi ba...@ti.com wrote:
Hi,
On Fri, Jul 29, 2011 at 06:28:32PM +0530, Govindraj wrote:
On Fri, Jul 29, 2011 at 5:49 PM, Felipe Balbi ba...@ti.com wrote:
On Fri, Jul 29, 2011 at 05:29:12PM +0530, Govindraj wrote:
Yes fine, But there are scenarios even before first runtime_suspend
happens,
ex: uart_port_configure - get_sync - pm_generic_runtime_resume
(omap_device_enable in this case) debug printk - console_write -
get_sync.
there are numerous such scenarios where we end from runtime context
to runtime api context again, or jumping from one uart port operation
to uart print operation.
calling pm_runtime_get_sync() should not be a problem. It should only
increase the usage counters... This sounds like a race condition on the
driver, no ?
Actually when we call a API to enable clocks we except the internals of API
to just enable clocks and return.
*Clock enable API should not cause or trigger to do a
_device_driver_operation_
even before enabling clocks of the device-driver which called it*
for uart context get_sync can land me to uart driver back
even before enabling the uart clocks due to printks.
only if _you_ have prints or _your_ runtime_*() calls, no ?
Let's say omap_hwmod.c wants to do a print:
- printk()
- pm_runtime_get_sync
- console_write
- pm_runtim_put
now, if you have a printk() on your runtime_resume() before you enable
the clocks, then I can see why you would deadlock:
- pm_runtime_get_sync
- omap_serial_runtime_resume
- printk
- pm_runtime_get_sync
- omap_serial_runtime_resume
- printk
- pm_runtime_get_sync
.
maybe I'm missing something, but can you add a stack dump on your
-runtime_resume and -runtime_suspend methods, just so we try to figure
out who's to blame here ?
What you're experiencing, if I understood correctly, is a deadlock ? In
that case, can you try to track the locking mechanism on the omap-serial
driver to try to find if there isn't anything obviously wrong ?
Yes deadlocks. due to entering from runtime context to runtime context
or entering from uart_port_operation to uart_console_write ops.
There are already port locks used extensively within the uart driver
to secure a port operation.
But cannot secure a port operation while using clock_enable API.
since clock enable API can land the control back to uart_console_write
operation..
but in that case, if clock isn't enabled, why don't you just ignore the
print and enable the clock ?? Just return 0 and continue with
clk_enable() ??
So either we should not have those prints from pm_generic layers or
suppress
them(seems pretty much a problem for a clean design within the driver
using console_lock/unlock for every get_sync, and for
runtime_put we cannot suppress the prints as it gets scheduled later)
or if other folks who really need those prints form pm_generic* layers
to debug and analysis then we have no other choice rather control
the clk_enable/disable from outside driver code in idle path.
yeah, none of these would be nice :-(
I think this needs more debugging to be sure what's exactly going on.
What's exactly causing the deadlock ? Which lock is held and never
released ?
I had done some investigations, from scenarios it simply boils down to fact
to handle clock within uart driver, uart driver expects clock enable API*
used
to just enable uart clocks but rather not trigger a _uart_ops_ within which
kind of unacceptable from the uart_driver context.
ok, now I see what you mean:
113 static int _omap_device_activate(struct omap_device *od, u8 ignore_lat)
114 {
115 struct timespec a, b, c;
116
117 pr_debug(omap_device: %s: activating\n, od-pdev.name);
118
119 while (od-pm_lat_level 0) {
120 struct omap_device_pm_latency *odpl;
121 unsigned long long act_lat = 0;
122
123 od-pm_lat_level--;
124
125 odpl = od-pm_lats + od-pm_lat_level;
126
127 if (!ignore_lat
128 (od-dev_wakeup_lat = od-_dev_wakeup_lat_limit))
129 break;
130
131 read_persistent_clock(a);
132
133 /* XXX check return code */
134 odpl-activate_func(od);
135
136 read_persistent_clock(b);
137
138 c = timespec_sub(b, a);
139 act_lat = timespec_to_ns(c);
140
141 pr_debug(omap_device: %s: pm_lat %d: activate: elapsed
time
142 %llu nsec\n, od-pdev.name, od-pm_lat_level,
143 act_lat);
144
145 if (act_lat odpl-activate_lat) {
146 odpl-activate_lat_worst = act_lat;
147 if (odpl-flags
OMAP_DEVICE_LATENCY_AUTO_ADJUST) {
148
[PATCHv2] OMAP4: I2C: Enable the wakeup in I2C_WE
Currently for OMAP4 the I2C_WE is not programmed. This patch enables the programming for OMAP4. This patch fixes a bad conflict resolution. This effectively restores the following commit Commit 120bdaa47[i2c-omap: Program I2C_WE on OMAP4 to enable i2c wakeup] which got changed by Commit a3a7acbc[I2C: OMAP2+: address confused probed version naming] Reviewed-by: Felipe Balbi ba...@ti.com Reported-by: Santosh Shilimkar santosh.shilim...@ti.com Signed-off-by: Shubhrajyoti D shubhrajy...@ti.com --- TODO: Currently all the wakeup sources are enabled. There is scope of optimising the same. Will revisit it. Rebased on Kevin's wip/i2c branch Tested on OMAP4430. drivers/i2c/busses/i2c-omap.c |5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index d05efe7..18cc0af 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -313,9 +313,8 @@ static int omap_i2c_init(struct omap_i2c_dev *dev) * REVISIT: Some wakeup sources might not be needed. */ dev-westate = OMAP_I2C_WE_ALL; - if (dev-rev OMAP_I2C_REV_ON_3530_4430) - omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, - dev-westate); + omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, + dev-westate); } omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, 0); -- 1.7.1 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/13] OMAP2+: powerdomain: control power domains next state
On Fri, Jul 29, 2011 at 10:47:43AM +0200, Jean Pihet wrote:
On Fri, Jul 29, 2011 at 9:59 AM, Todd Poynor toddpoy...@google.com wrote:
...
All min_latency != PM_QOS_DEV_LAT_DEFAULT_VALUE paths need
free_new_user = 1.
free_new_user = 1 is only needed if no existing constraint has been
found, i.e. user stays at NULL. This is implemented in the check for
an existing constraint (plist_for_each_entry(...)).
Oops, I meant to say it applies in all cases where min_latency ==
PM_QOS_DEV_LAT_DEFAULT_VALUE, since the new node is only used for
adding a constraint (and no existing constraint found). I'd suggest
something like:
if (min_latency != PM_QOS_DEV_LAT_DEFAULT_VALUE) {
new_user = kzalloc(sizeof(struct pwrdm_wkup_constraints_entry),
GFP_KERNEL);
check NULL return
free_new_user = 1;
}
and then set free_new_user = 0 only if no existing constraint is found
for the add case. Because it's easy to miss cases where the
allocated memory needs to be freed when that's not the default, and you
might as well skip the allocate on a constraint removal. Pretty minor
point, though.
Todd
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-pm] Runtime PM discussion notes
On Friday, July 29, 2011, Pavel Machek wrote: Hi! Actually, it just occurred to me that if we're waiting for a system timer and can hand that off to a suitable timer in the PMIC then we can do a suspend to RAM for the deep idle state from the hardware point of view. Yep. At LinuxCon Cambridge two years ago, we had a discussion about whether it would be possible to enter ACPI S-states from CPUIdle (or some idle governor) on Intel chips. If I remember correctly, the conclusion was that ACPI always disables the screen/backlight, so it would only be useful for situations where that was acceptable. Well, auto suspending when screensaver is active would still be useful. (And IIRC some machines kept screen on when in S-state unless driver powered it down... but that might be S1. The reason why you can't enter ACPI S-states from CPUidle is because you need to go out of the idle loop to execute some ACPI-specific stuff. Which is not even specific to Intel chips, but to ACPI in general. The code was little tricky/unclean, but it worked for me at one point... I called it sleepy linux. Yes, you can find a system where it might kind of work (just because _PTS is empty or something like this). Is it going to work in general? No way. So please let's turn into something at least _theoretically_ viable. Thanks, Rafael -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v3 00/13] PM QoS: add a per-device latency constraints class
On Thursday, July 28, 2011, jean.pi...@newoldbits.com wrote: From: Jean Pihet j-pi...@ti.com This patch set is in an RFC state, for review and comments. High level implementation: 1. Add a new PM QoS class for device wake-up constraints (PM_QOS_DEV_LATENCY). . Define a pm_qos_constraints struct for the storage of the constraints list and associated values (target_value, default_value, type ...). . Update the pm_qos_object struct with the information related to a PM QoS class: ptr to constraints list, notifer ptr, name ... . Each PM QoS class statically declare objects for pm_qos_object and pm_qos_constraints. The only exception is the devices constraints, cf. below. . The device constraints class is statically declaring a pm_qos_object. The pm_qos_constraints are per-device and so are embedded into the device struct. The new class is available from kernel drivers and shall be made available to user space through a per-device sysfs entry. User space API to come as a subsequent patch. 2. Added a notification of device insertion/removal from the device PM framework to PM QoS. This allows to init/de-init the per-device constraints list upon device insertion and removal. RFC state for comments and review, lightly tested 3. Make the pm_qos_add_request API more generic by using a struct pm_qos_parameters parameter. This allows easy extension in the future. 4. Upon a change of the aggregated constraint value in the PM_QOS_DEV_LATENCY class a notification chain mechanism is used to take action on the system. This is the proposed way to have PM QoS and the platform dependant code to interact with each other, cf. 5 below. The notification mechanism now passes the constraint request struct ptr in order for the notifier callback to have access to the full set of constraint data, e.g. the struct device ptr. 5. cpuidle interaction with the OMAP3 cpuidle handler Since cpuidle is a CPU centric framework it decides the MPU next power state based on the MPU exit_latency and target_residency figures. The rest of the power domains get their next power state programmed from the PM_QOS_DEV_LATENCY class of the PM QoS framework, via the device wake-up latency constraints callback to the OMAP_PM_CONSTRAINTS framework. Note: the exit_latency and target_residency figures of the MPU include the MPU itself and the peripherals needed for the MPU to execute instructions (e.g. main memory, caches, IRQ controller, MMU etc). Some of those peripherals can belong to other power domains than the MPU subsystem and so the corresponding latencies must be included in those figures. 6. Update the pm_qos_add_request callers to the generic API 7. Misc fixes to improve code readability: . rename of the PM QoS implementation file from pm_qos_params.[ch] to pm_qos.[ch] . rename of fields names (request, list, constraints, class), . simplification of the in-kernel PM QoS API implementation. The main logic part is now implemented in the update_target function. Questions: 1. per-device user-space API: since sysfs does not provide open/close callbacks it is not possible to support multiple and simultaneous users of the per-device sysfs entry. A user-space constraints aggregation application is needed in case of multiple constraints for a device. Is this the way to go? I'd say so. Thanks, Rafael -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v3 00/13] PM QoS: add a per-device latency constraints class
On Friday, July 29, 2011, mark gross wrote: On Fri, Jul 29, 2011 at 10:37:52AM +0200, Jean Pihet wrote: Mark, On Thu, Jul 28, 2011 at 3:14 PM, mark gross markgr...@thegnar.org wrote: On Thu, Jul 28, 2011 at 10:30:07AM +0200, jean.pi...@newoldbits.com wrote: From: Jean Pihet j-pi...@ti.com This patch set is in an RFC state, for review and comments. High level implementation: ... 7. Misc fixes to improve code readability: . rename of the PM QoS implementation file from pm_qos_params.[ch] to pm_qos.[ch] I picked the name for the file as pm_qos_params over pm_qos because I wanted to make it implicitly clear that this was not an full QOS implementation. True QOS carries expectations similar to real time and as the infrastructure is closer to good intentioned than even best effort and offers no notification when the QOS request is not able to be met and really doesn't implement a true QOS at all, (it just provides the parameter interface for part of one its missing the notification interface when the service level is not met and I think a few other things.) So I wanted to have it named a bit different from just pm_qos. This said I'm not supper attached to the naming of the modules. If folks want to change it I wouldn't complain (too much anyway;). Ok got the idea. I do not know what name to chose though. As suggested previously the name pm_qos_params does not reflect the implementation, that is why I renamed it. I must have missed the part where the name doesn't reflect the implementation was talked about. I look at the interface and I see parameters all over the place and a small bit of notification. The interface is for specifying PM QoS requirements or constraints that may or may not be regarded as parameters, depending on ones definition of the last term. Moreover, the code not only implements the interface, but also defines data structures and API to be used throughout the kernel which is quite a bit more than just parameters. In my not so humble opinion the name isn't good and the .c file should be in kernel/power in the first place. I would call it simply qos.c (the fact that _right_ _now_ it's not a full QoS implementation doesn't matter, because it may become one in the future). Thanks, Rafael -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/13] PM: add a per-device wake-up latency constraints plist
On Thursday, July 28, 2011, jean.pi...@newoldbits.com wrote:
From: Jean Pihet j-pi...@ti.com
Add the field latency_constraints in the struct dev_pm_info
and the initialization of the plist in device_pm_init.
This enables the implementation of per-device constraints in
PM QoS.
Signed-off-by: Jean Pihet j-pi...@ti.com
This one looks good to me.
Thanks,
Rafael
---
drivers/base/power/main.c |1 +
include/linux/pm.h|2 ++
2 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index 06f09bf..dad2eb9 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -97,6 +97,7 @@ void device_pm_add(struct device *dev)
dev_name(dev-parent));
list_add_tail(dev-power.entry, dpm_list);
mutex_unlock(dpm_list_mtx);
+ plist_head_init(dev-power.latency_constraints, dev-power.lock);
}
/**
diff --git a/include/linux/pm.h b/include/linux/pm.h
index 411e4f4..23c85f1 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -22,6 +22,7 @@
#define _LINUX_PM_H
#include linux/list.h
+#include linux/plist.h
#include linux/workqueue.h
#include linux/spinlock.h
#include linux/wait.h
@@ -463,6 +464,7 @@ struct dev_pm_info {
unsigned long accounting_timestamp;
void*subsys_data; /* Owned by the subsystem. */
#endif
+ struct plist_head latency_constraints;
};
extern void update_pm_runtime_accounting(struct device *dev);
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/13] PM: QoS: rename pm_qos_params files to pm_qos
On Thursday, July 28, 2011, jean.pi...@newoldbits.com wrote: From: Jean Pihet j-pi...@ti.com The PM QoS implementation files are better named kernel/pm_qos.c and include/linux/pm_qos.h. Signed-off-by: Jean Pihet j-pi...@ti.com --- arch/arm/mach-msm/clock.c |2 +- drivers/acpi/processor_idle.c |2 +- drivers/cpuidle/cpuidle.c |2 +- drivers/cpuidle/governors/ladder.c |2 +- drivers/cpuidle/governors/menu.c |2 +- drivers/media/video/via-camera.c |2 +- drivers/net/e1000e/netdev.c|2 +- drivers/net/wireless/ipw2x00/ipw2100.c |2 +- drivers/staging/msm/lcdc.c |2 +- drivers/staging/msm/tvenc.c|2 +- include/linux/netdevice.h |2 +- include/linux/pm_qos.h | 38 +++ include/linux/pm_qos_params.h | 38 --- include/sound/pcm.h|2 +- kernel/Makefile|2 +- kernel/pm_qos.c| 481 kernel/pm_qos_params.c | 481 net/mac80211/main.c|2 +- net/mac80211/mlme.c|2 +- net/mac80211/scan.c|2 +- sound/core/pcm_native.c|2 +- 21 files changed, 536 insertions(+), 536 deletions(-) create mode 100644 include/linux/pm_qos.h delete mode 100644 include/linux/pm_qos_params.h That I agree with. create mode 100644 kernel/pm_qos.c delete mode 100644 kernel/pm_qos_params.c As I said, please move that file to kernel/power and call it qos.c. That said the device interface should be located in drivers/base/power to follow our current conventions. Thanks, Rafael -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/13] PM: QoS: extend the in-kernel API with per-device latency constraints
On Thursday, July 28, 2011, jean.pi...@newoldbits.com wrote:
From: Jean Pihet j-pi...@ti.com
Extend the PM QoS kernel API:
- add a new PM QoS class PM_QOS_DEV_LATENCY for device wake-up latency
constraints
- make the pm_qos_add_request API more generic by using a parameter of
type struct pm_qos_parameters
- minor clean-ups and rename of struct fields:
. rename pm_qos_class to class and pm_qos_req to req in internal code
. consistenly use req and params as the API parameters
. rename struct pm_qos_request_list to struct pm_qos_request
- update the in-kernel API callers to the new API
There should be more about the motivation in the changelog. It says
what the patch is doing, but it doesn't say a word of _why_ it is done in
the first place.
Second, you're renaming a lot of things in the same patch that makes
functional changes. This is confusing and makes it very difficult to
understand what's going on. Please use separate patches to rename
things, when possible, and avoid renaming things that don't need to be
renamed.
Signed-off-by: Jean Pihet j-pi...@ti.com
---
arch/arm/plat-omap/i2c.c | 20 -
drivers/i2c/busses/i2c-omap.c | 35 ++---
drivers/media/video/via-camera.c |7 +-
drivers/net/e1000e/netdev.c|9 ++-
drivers/net/wireless/ipw2x00/ipw2100.c |8 +-
include/linux/netdevice.h |2 +-
include/linux/pm_qos.h | 39 ++
include/sound/pcm.h|2 +-
kernel/pm_qos.c| 130 +--
sound/core/pcm_native.c|8 ++-
10 files changed, 141 insertions(+), 119 deletions(-)
I'm going to comment the core changes this time.
...
diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
index 9cc0224..a2e4409 100644
--- a/include/linux/pm_qos.h
+++ b/include/linux/pm_qos.h
@@ -8,31 +8,40 @@
#include linux/notifier.h
#include linux/miscdevice.h
-#define PM_QOS_RESERVED 0
-#define PM_QOS_CPU_DMA_LATENCY 1
-#define PM_QOS_NETWORK_LATENCY 2
-#define PM_QOS_NETWORK_THROUGHPUT 3
+#define PM_QOS_RESERVED 0
+#define PM_QOS_CPU_DMA_LATENCY 1
+#define PM_QOS_DEV_LATENCY 2
+#define PM_QOS_NETWORK_LATENCY 3
+#define PM_QOS_NETWORK_THROUGHPUT4
-#define PM_QOS_NUM_CLASSES 4
+#define PM_QOS_NUM_CLASSES 5
#define PM_QOS_DEFAULT_VALUE -1
#define PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE (2000 * USEC_PER_SEC)
+#define PM_QOS_DEV_LAT_DEFAULT_VALUE 0
#define PM_QOS_NETWORK_LAT_DEFAULT_VALUE (2000 * USEC_PER_SEC)
#define PM_QOS_NETWORK_THROUGHPUT_DEFAULT_VALUE 0
-struct pm_qos_request_list {
+struct pm_qos_request {
This renaming should go in a separate patch, really.
struct plist_node list;
- int pm_qos_class;
+ int class;
This renaming doesn't seem to be necessary. Moreover, it's confusing,
because there already is struct class (for device classes) and a member
called class in struct device and they aren't related to this one.
+ struct device *dev;
};
-void pm_qos_add_request(struct pm_qos_request_list *l, int pm_qos_class, s32
value);
-void pm_qos_update_request(struct pm_qos_request_list *pm_qos_req,
- s32 new_value);
-void pm_qos_remove_request(struct pm_qos_request_list *pm_qos_req);
+struct pm_qos_parameters {
+ int class;
+ struct device *dev;
+ s32 value;
+};
What exactly is the dev member needed for?
+
+void pm_qos_add_request(struct pm_qos_request *req,
+ struct pm_qos_parameters *params);
+void pm_qos_update_request(struct pm_qos_request *req, s32 new_value);
+void pm_qos_remove_request(struct pm_qos_request *req);
-int pm_qos_request(int pm_qos_class);
-int pm_qos_add_notifier(int pm_qos_class, struct notifier_block *notifier);
-int pm_qos_remove_notifier(int pm_qos_class, struct notifier_block
*notifier);
-int pm_qos_request_active(struct pm_qos_request_list *req);
+int pm_qos_request(int class);
+int pm_qos_add_notifier(int class, struct notifier_block *notifier);
+int pm_qos_remove_notifier(int class, struct notifier_block *notifier);
+int pm_qos_request_active(struct pm_qos_request *req);
#endif
diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index 1204f17..d3b068f 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -373,7 +373,7 @@ struct snd_pcm_substream {
int number;
char name[32]; /* substream name */
int stream; /* stream (direction) */
- struct pm_qos_request_list latency_pm_qos_req; /* pm_qos request */
+ struct pm_qos_request latency_pm_qos_req; /* pm_qos request */
size_t buffer_bytes_max;/* limit ring buffer size */
struct snd_dma_buffer dma_buffer;
unsigned int dma_buf_id;
diff --git a/kernel/pm_qos.c b/kernel/pm_qos.c
index
Re: [RFC/PATCH 2/7] OMAP3: beagle: don't touch omap_device internals
On Fri, Jul 29, 2011 at 09:05, Felipe Balbi ba...@ti.com wrote: +} +EXPORT_SYMBOL(omap_hwmod_name_get_odev); maybe EXPORT_SYMBOL_GPL() ?? Not sure we want non-GPL code to access this ;-) Sure.. but is this the way we want to go? if yes, I can post the series in a formal way to the list. Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/7] decouple platform_device from omap_device
G, Manjunath Kondaiah manj...@ti.com writes: On Wed, Jul 27, 2011 at 02:45:33PM -0700, Hilman, Kevin wrote: Hi Manjunath, On Wed, Jul 27, 2011 at 7:04 AM, G, Manjunath Kondaiah manj...@ti.com wrote: Kevin, On Thu, Jul 21, 2011 at 04:52:10PM -0700, Kevin Hilman wrote: Here's a first whack, proof-of-concept on how we could start to decouple the platform_device from an omap_device. The main RFC is in the last patch, and everything leading up to it are misc. omap_device cleanups that make the last patch cleaner and clearer. It's really the last patch that does the decoupling. This will be necessary if we're going to decouple the platform_device creation from the omap_device/omap_hwmod creation etc. This patch leaves them both done in omap_device_build(), but shows that they can be decoupled. Can you pls mention baseline used for these patches? I tried applying on latest mainline, v3.0 and git://git.pwsan.com/linux-2.6 prcm-devel-3.1 Oops, sorry. I forgot to mention it. Due to some misc. dependencies (mainly on Beagle board file), I've temporarily based it on the for-next branch of the arm-soc tree[1] since that has everything already queued for the next merge window. I also based it on the omap_device patch I posted which changes the pr_* prints to dev_*. For convenience, I've pushed this series to the 'wip/od-devres' branch of my tree[2]. Sorry for the confusion, Kevin [1] git://git.kernel.org/pub/scm/linux/kernel/git/arm/linux-arm-soc.git [2] git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-omap-pm.git This series results in boot crash. However, it boots fine without this series. I have not tried to debug but it looks like from boot log, the res structure has invalid entries in mmc probe. As mentioned by Grant in 7/7, the scope of devres is not lifetime hence it needs to be fixed. This limitation was also clearly described in the changelog of patch 7/7. However, I don't think that problem should cause a problem on boot, only a from the driver core (which, suprisingly, I don't see in your bootlog.) I suspect your crash is because you're not also testing with the MMC runtime PM series that is merging for v3.1. Kevin -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
