Re: [PATCH] I2C: Fix for suspend/resume issue in i2c-core
On Sat, 4 Sep 2010 01:10:10 +0200, Rafael J. Wysocki wrote: On Friday, September 03, 2010, Sripathy, Vishwanath wrote: We are seeing one more issue even after making this fix. In summary, after first suspend/resume, CPU Idle no longer works since i2c module is active. Basically when the system resumed from the suspend, dpm layer (dpm_resume_end) calls device_resume which intern calls i2c_device_pm_resume (among many other calls). i2c_device_pm_resume calls pm_runtime_set_active which brings device to Active state and increases child_count of it's parent. Since the device is active and also it's parent child_count is non 0, these modules will prevent corresponding clock domains to go idle. This will break CPU Idle. This issue happens even if the module was in idle state before performing suspend/resume. In summary, the flow is 1. i2c module is idle (let's assume system is idle) 2. system suspend is initiated by user 3. i2c_device_pm_suspend gets called which tries to idle i2c, but it's already idled. 4. system is suspended 5. system resumed (because of user event/timers) 6. dpm layer will call i2c_device_pm_resume 7. i2c_device_pm_resume will enable i2c device by calling pm_runtime_set_active So at the end of suspend/resume, a device that was idled before is getting enabled unnecessarily at the end. SO I am just wondering is there any real need to call pm_runtime_set_active in resume and pm_runtime_set_suspened in suspend functions? I just tried suspend/resume and CPU Idle after removing these calls in i2c and it seems to function perfectly fine on OMAP4. Your analysis appears to be entirely correct. So, instead of applying the $subject patch it might be better to remove the block that calls pm_runtime_set_active(dev) from i2c_device_pm_resume(). Are there any objections? No objection. Just send an updated patch and I'll be happy to apply it. -- Jean Delvare -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V2] I2C: Fix for suspend/resume issue in i2c-core
In current i2c core driver, call to pm_runtime_set_active from
i2c_device_pm_resume
will unconditionally enable i2c module and increment child count of the parent.
Because of this, in CPU Idle path, i2c does not idle, preventing Core to
enter retention. Also i2c module will not be suspended upon system suspend as
pm_runtime_set_suspended is not called from i2c_device_pm_suspend.
This issue is fixed by removing pm_runtime_set_active call from resume path
which
is not necessary.
This fix has been tested on OMAP4430.
Signed-off-by: Partha Basak p-bas...@ti.com
Signed-off-by: Vishwanath BS vishwanath...@ti.com
Cc: Rafael J. Wysocki r...@sisk.pl
Cc: Kevin Hilman khil...@deeprootsystems.com
Cc: Ben Dooks ben-li...@fluff.org
Cc: Jean Delvare kh...@linux-fr.org
---
drivers/i2c/i2c-core.c |6 --
1 files changed, 0 insertions(+), 6 deletions(-)
mode change 100644 = 100755 drivers/i2c/i2c-core.c
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
old mode 100644
new mode 100755
index 6649176..13927d5
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -216,12 +216,6 @@ static int i2c_device_pm_resume(struct device *dev)
else
ret = i2c_legacy_resume(dev);
- if (!ret) {
- pm_runtime_disable(dev);
- pm_runtime_set_active(dev);
- pm_runtime_enable(dev);
- }
-
return ret;
}
--
1.7.0.4
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2] I2C: Fix for suspend/resume issue in i2c-core
Hi Vishwa,
I just have a couple of minors comments:
On 9/4/2010 10:48 AM, Sripathy, Vishwanath wrote:
In current i2c core driver, call to pm_runtime_set_active from
i2c_device_pm_resume
will unconditionally enable i2c module and increment child count of the parent.
Because of this, in CPU Idle path, i2c does not idle, preventing Core to
enter retention. Also i2c module will not be suspended upon system suspend as
pm_runtime_set_suspended is not called from i2c_device_pm_suspend.
This issue is fixed by removing pm_runtime_set_active call from resume path
which
Unnecessary spaces: -^ and ^
is not necessary.
This fix has been tested on OMAP4430.
Signed-off-by: Partha Basakp-bas...@ti.com
Signed-off-by: Vishwanath BSvishwanath...@ti.com
Cc: Rafael J. Wysockir...@sisk.pl
Cc: Kevin Hilmankhil...@deeprootsystems.com
Cc: Ben Dooksben-li...@fluff.org
Cc: Jean Delvarekh...@linux-fr.org
---
drivers/i2c/i2c-core.c |6 --
1 files changed, 0 insertions(+), 6 deletions(-)
mode change 100644 = 100755 drivers/i2c/i2c-core.c
You should probably not have to change the file mode.
Regards,
Benoit
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
old mode 100644
new mode 100755
index 6649176..13927d5
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -216,12 +216,6 @@ static int i2c_device_pm_resume(struct device *dev)
else
ret = i2c_legacy_resume(dev);
- if (!ret) {
- pm_runtime_disable(dev);
- pm_runtime_set_active(dev);
- pm_runtime_enable(dev);
- }
-
return ret;
}
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2] I2C: Fix for suspend/resume issue in i2c-core
On Sat, 4 Sep 2010 14:18:01 +0530, Vishwanath BS wrote:
In current i2c core driver, call to pm_runtime_set_active from
i2c_device_pm_resume
will unconditionally enable i2c module and increment child count of the
parent.
Because of this, in CPU Idle path, i2c does not idle, preventing Core to
enter retention. Also i2c module will not be suspended upon system suspend as
pm_runtime_set_suspended is not called from i2c_device_pm_suspend.
This issue is fixed by removing pm_runtime_set_active call from resume path
which
is not necessary.
This fix has been tested on OMAP4430.
Signed-off-by: Partha Basak p-bas...@ti.com
Signed-off-by: Vishwanath BS vishwanath...@ti.com
Cc: Rafael J. Wysocki r...@sisk.pl
Cc: Kevin Hilman khil...@deeprootsystems.com
Cc: Ben Dooks ben-li...@fluff.org
Cc: Jean Delvare kh...@linux-fr.org
---
drivers/i2c/i2c-core.c |6 --
1 files changed, 0 insertions(+), 6 deletions(-)
mode change 100644 = 100755 drivers/i2c/i2c-core.c
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
old mode 100644
new mode 100755
index 6649176..13927d5
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -216,12 +216,6 @@ static int i2c_device_pm_resume(struct device *dev)
else
ret = i2c_legacy_resume(dev);
- if (!ret) {
- pm_runtime_disable(dev);
- pm_runtime_set_active(dev);
- pm_runtime_enable(dev);
- }
-
return ret;
}
Applied, thanks.
--
Jean Delvare
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] i2c-davinci: Fix use of default platform data if none supplied.
There is a bug in the i2c-davinci device init routine that attempts
to use default platform data if none is supplied (e.g., is NULL).
This patch fixes the bug.
Signed-off-by: Michael Williamson michael.william...@criticallink.com
---
drivers/i2c/busses/i2c-davinci.c |8 +---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index c87..6d4eeeb 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -235,10 +235,12 @@ static void i2c_davinci_calc_clk_dividers(struct
davinci_i2c_dev *dev)
*/
static int i2c_davinci_init(struct davinci_i2c_dev *dev)
{
- struct davinci_i2c_platform_data *pdata = dev-dev-platform_data;
+ struct davinci_i2c_platform_data *pdata;
- if (!pdata)
- pdata = davinci_i2c_platform_data_default;
+ if (!dev-dev-platform_data)
+ dev-dev-platform_data = davinci_i2c_platform_data_default;
+
+ pdata = dev-dev-platform_data;
/* put I2C into reset */
davinci_i2c_reset_ctrl(dev, 0);
--
1.7.0.4
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] iio: ak8975: Add Ak8975 magnetometer sensor
On 09/03/10 00:40, ac...@nvidia.com wrote: From: Andrew Chew ac...@nvidia.com This is for the Asahi Kasei AK8975 3-axis magnetometer. It resides within the magnetometer section of the IIO subsystem, and implements the raw magn_x,y,z attributes, as well as x,y,z factory calibration attributes. Signed-off-by: Andrew Chew ac...@nvidia.com Hi Andrew, Looks pretty clean. A nice little driver. Anyhow inline you will find: * A few nitpicking points about eating errors. Lots of drivers in kernel do it, but lets not encourage even more. * Couple of other cleanup suggestions (none of which are vital, but might be worth doing if you are respinning anyway) I'm particularly intrigued to know if the sannity check for existence of the device can ever fail... I'll take a final look after you have done the abi related changes. Thanks, Jonathan p.s. As Andrew said, please make it clear which version of a patch this is, typically make the subject [Patch 1/1 V2] iio: ak8975 ... And some brief comments to say what has changed from the previous version are a great time saver for reviewers. --- drivers/staging/iio/magnetometer/Kconfig | 11 + drivers/staging/iio/magnetometer/Makefile |1 + drivers/staging/iio/magnetometer/ak8975.c | 521 + 3 files changed, 533 insertions(+), 0 deletions(-) create mode 100644 drivers/staging/iio/magnetometer/ak8975.c diff --git a/drivers/staging/iio/magnetometer/Kconfig b/drivers/staging/iio/magnetometer/Kconfig index d014450..00e1204 100644 --- a/drivers/staging/iio/magnetometer/Kconfig +++ b/drivers/staging/iio/magnetometer/Kconfig @@ -3,6 +3,17 @@ # comment Magnetometer sensors +config SENSORS_AK8975 + tristate Asahi Kasei AK8975 3-Axis Magnetometer + default n + depends on I2C + help + Say yes here to build support for Asahi Kasei AK8975 3-Axis + Magnetometer. + + To compile this driver as a module, choose M here: the module + will be called ak8975. + config SENSORS_HMC5843 tristate Honeywell HMC5843 3-Axis Magnetometer depends on I2C diff --git a/drivers/staging/iio/magnetometer/Makefile b/drivers/staging/iio/magnetometer/Makefile index f9bfb2e..e3dbaa4 100644 --- a/drivers/staging/iio/magnetometer/Makefile +++ b/drivers/staging/iio/magnetometer/Makefile @@ -2,4 +2,5 @@ # Makefile for industrial I/O Magnetometer sensors # +obj-$(CONFIG_SENSORS_AK8975) := ak8975.o obj-$(CONFIG_SENSORS_HMC5843)+= hmc5843.o diff --git a/drivers/staging/iio/magnetometer/ak8975.c b/drivers/staging/iio/magnetometer/ak8975.c new file mode 100644 index 000..ca39840 --- /dev/null +++ b/drivers/staging/iio/magnetometer/ak8975.c @@ -0,0 +1,521 @@ +/* + * A sensor driver for the magnetometer AK8975. + * + * Magnetic compass sensor driver for monitoring magnetic flux information. + * + * Copyright (c) 2010, NVIDIA Corporation. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +#include linux/module.h +#include linux/kernel.h +#include linux/slab.h +#include linux/i2c.h +#include linux/err.h +#include linux/mutex.h +#include linux/delay.h + +#include linux/gpio.h + +#include ../iio.h +#include magnet.h + +/* + * Register definitions, as well as various shifts and masks to get at the + * individual fields of the registers. + */ +#define AK8975_REG_WIA 0x00 +#define AK8975_DEVICE_ID 0x48 + +#define AK8975_REG_INFO 0x01 + +#define AK8975_REG_ST1 0x02 +#define AK8975_REG_ST1_DRDY_SHIFT0 +#define AK8975_REG_ST1_DRDY_MASK (1 AK8975_REG_ST1_DRDY_SHIFT) + +#define AK8975_REG_HXL 0x03 +#define AK8975_REG_HXH 0x04 +#define AK8975_REG_HYL 0x05 +#define AK8975_REG_HYH 0x06 +#define AK8975_REG_HZL 0x07 +#define AK8975_REG_HZH 0x08 +#define AK8975_REG_ST2 0x09 +#define AK8975_REG_ST2_DERR_SHIFT2 +#define AK8975_REG_ST2_DERR_MASK (1 AK8975_REG_ST2_DERR_SHIFT) + +#define AK8975_REG_ST2_HOFL_SHIFT3 +#define AK8975_REG_ST2_HOFL_MASK (1
RE: [PATCH v2] power supply: add driver for TI BQ20Z75 gas gauge IC
Mark, have you had a chance to double check my changes based on the comments you made? Thanks, Rhyland -Original Message- From: Anton Vorontsov [mailto:cbouatmai...@gmail.com] Sent: Thursday, September 02, 2010 10:48 AM To: Rhyland Klein Cc: linux-i2c@vger.kernel.org; linux-ker...@vger.kernel.org; o...@lixom.net; Andrew Chew; kh...@linux-fr.org; broo...@opensource.wolfsonmicro.com Subject: Re: [PATCH v2] power supply: add driver for TI BQ20Z75 gas gauge IC On Thu, Sep 02, 2010 at 10:29:46AM -0700, rkl...@nvidia.com wrote: From: Rhyland Klein rkl...@nvidia.com this driver depends on I2C and uses SMBUS for communication with the host. Addressed comments from reviews by Mark Brown and Jean Delvare. * Cleaned up whitespace and alignment issues * changed return codes to more appropriate values * change Kconfig option name to be consistent with existing devices * removed global struct and moved to device specific data * changed printk to dev_dbg The driver looks good to me. But I'll wait a day or two for Jean's and Mark's Acked-by or Reviewed-by tags before applying, just to give them some credit for reviwing this driver. Thanks! -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2
