RE: [PATCH v2] power supply: add driver for TI BQ20Z75 gas gauge IC

2010-09-04 Thread Rhyland Klein
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 
> 
> 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


Re: [PATCH 1/1] iio: ak8975: Add Ak8975 magnetometer sensor

2010-09-04 Thread Jonathan Cameron
On 09/03/10 00:40, ac...@nvidia.com wrote:
> From: Andrew Chew 
> 
> 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 

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 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +
> +#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 << AK8975_REG_S

[PATCH] i2c-davinci: Fix use of default platform data if none supplied.

2010-09-04 Thread Michael Williamson
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 
---
 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 V2] I2C: Fix for suspend/resume issue in i2c-core

2010-09-04 Thread Jean Delvare
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 
> Signed-off-by: Vishwanath BS 
> 
> Cc: Rafael J. Wysocki 
> Cc: Kevin Hilman 
> Cc: Ben Dooks 
> Cc: Jean Delvare 
> ---
>  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


Re: [PATCH V2] I2C: Fix for suspend/resume issue in i2c-core

2010-09-04 Thread Cousson, Benoit

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 Basak
Signed-off-by: Vishwanath BS

Cc: Rafael J. Wysocki
Cc: Kevin Hilman
Cc: Ben Dooks
Cc: Jean Delvare
---
  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


[PATCH V2] I2C: Fix for suspend/resume issue in i2c-core

2010-09-04 Thread Vishwanath BS
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 
Signed-off-by: Vishwanath BS 

Cc: Rafael J. Wysocki 
Cc: Kevin Hilman 
Cc: Ben Dooks 
Cc: Jean Delvare 
---
 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