Re: [PATCH v2 3/3] lp8788-charger: fix wrong ADC conversion

2012-11-17 Thread Anton Vorontsov
On Fri, Nov 02, 2012 at 10:09:20AM +, Jonathan Cameron wrote:
[...]
> > Looks good to me, fwiw:
> > 
> > Reviewed-by Lars-Peter Clausen 
>
> On this stuff as far as I'm concerned Lars' approval is fine but
> for what it's worth this all looks fine to me.
> 
> Acked-by: Jonathan Cameron 

All three applied, thanks a lot!
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2 3/3] lp8788-charger: fix wrong ADC conversion

2012-11-17 Thread Anton Vorontsov
On Fri, Nov 02, 2012 at 10:09:20AM +, Jonathan Cameron wrote:
[...]
  Looks good to me, fwiw:
  
  Reviewed-by Lars-Peter Clausen l...@metafoo.de

 On this stuff as far as I'm concerned Lars' approval is fine but
 for what it's worth this all looks fine to me.
 
 Acked-by: Jonathan Cameron ji...@kernel.org

All three applied, thanks a lot!
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2 3/3] lp8788-charger: fix wrong ADC conversion

2012-11-02 Thread Jonathan Cameron
On 10/19/2012 01:45 PM, Lars-Peter Clausen wrote:
> On 10/19/2012 02:12 AM, Kim, Milo wrote:
>>  To get the battery voltage and temperature, IIO ADC functions are used.
>>  LP8788 ADC driver provides RAW and SCALE channel information.
>>  This patch fixes wrong ADC result.
>>
>>  Patch v2.
>>  Use simple iio_read_channel_processed() function rather than
>>  iio_read_channel_raw() and _scale().
>>
>>  Fix the result type of ADC function as a signed integer.
>>  Because power_supply_propval.intval and the return value of
>>  iio_read_channel_processed() are a signed integer,
>>  'unsigned int' are replaced with 'int'.
>>
>>  Patch v1.
>>  Fix wrong ADC results using iio_read_channel_raw() and _scale().
>>
>> Signed-off-by: Milo(Woogyom) Kim 
> 
> Looks good to me, fwiw:
> 
> Reviewed-by Lars-Peter Clausen 
On this stuff as far as I'm concerned Lars' approval is fine but
for what it's worth this all looks fine to me.

Acked-by: Jonathan Cameron 

> 
> But there is one issue, but this is not necessarily related to this patch,
> more inline.
> 
>> ---
>>  drivers/power/lp8788-charger.c |   26 +++---
>>  1 file changed, 7 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/power/lp8788-charger.c b/drivers/power/lp8788-charger.c
>> index 02fc9ab..f18ec8f 100644
>> --- a/drivers/power/lp8788-charger.c
>> +++ b/drivers/power/lp8788-charger.c
>> @@ -235,25 +235,14 @@ static int lp8788_get_battery_present(struct 
>> lp8788_charger *pchg,
>>  return 0;
>>  }
>>  
> [...]
>>  static int lp8788_get_battery_voltage(struct lp8788_charger *pchg,
>> @@ -268,7 +257,7 @@ static int lp8788_get_battery_capacity(struct 
>> lp8788_charger *pchg,
>>  struct lp8788 *lp = pchg->lp;
>>  struct lp8788_charger_platform_data *pdata = pchg->pdata;
>>  unsigned int max_vbatt;
>> -unsigned int vbatt;
>> +int vbatt;
>>  enum lp8788_charging_state state;
>>  u8 data;
>>  int ret;
>> @@ -304,19 +293,18 @@ static int lp8788_get_battery_temperature(struct 
>> lp8788_charger *pchg,
>>  union power_supply_propval *val)
>>  {
>>  struct iio_channel *channel = pchg->chan[LP8788_BATT_TEMP];
>> -int scaleint;
>> -int scalepart;
>> +int result;
>>  int ret;
>>  
>>  if (!channel)
>>  return -EINVAL;
>>  
>> -ret = iio_read_channel_scale(channel, , );
>> -if (ret != IIO_VAL_INT_PLUS_MICRO)
>> +ret = iio_read_channel_processed(channel, );
>> +if (ret < 0)
>>  return -EINVAL;
>>  
>>  /* unit: 0.1 'C */
>> -val->intval = (scaleint + scalepart * 100) / 100;
>> +val->intval = result * 10;
> 
> IIO reports temperatures in milli degree Celsius. So it should be multiplied
> by 100 to get tenth degree like the power supply framework expects it. But
> this might be a issue in your IIO driver reporting the wrong scale.
> 
> - Lars
> 
>>  
>>  return 0;
>>  }
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2 3/3] lp8788-charger: fix wrong ADC conversion

2012-11-02 Thread Jonathan Cameron
On 10/19/2012 01:45 PM, Lars-Peter Clausen wrote:
 On 10/19/2012 02:12 AM, Kim, Milo wrote:
  To get the battery voltage and temperature, IIO ADC functions are used.
  LP8788 ADC driver provides RAW and SCALE channel information.
  This patch fixes wrong ADC result.

  Patch v2.
  Use simple iio_read_channel_processed() function rather than
  iio_read_channel_raw() and _scale().

  Fix the result type of ADC function as a signed integer.
  Because power_supply_propval.intval and the return value of
  iio_read_channel_processed() are a signed integer,
  'unsigned int' are replaced with 'int'.

  Patch v1.
  Fix wrong ADC results using iio_read_channel_raw() and _scale().

 Signed-off-by: Milo(Woogyom) Kim milo@ti.com
 
 Looks good to me, fwiw:
 
 Reviewed-by Lars-Peter Clausen l...@metafoo.de
On this stuff as far as I'm concerned Lars' approval is fine but
for what it's worth this all looks fine to me.

Acked-by: Jonathan Cameron ji...@kernel.org

 
 But there is one issue, but this is not necessarily related to this patch,
 more inline.
 
 ---
  drivers/power/lp8788-charger.c |   26 +++---
  1 file changed, 7 insertions(+), 19 deletions(-)

 diff --git a/drivers/power/lp8788-charger.c b/drivers/power/lp8788-charger.c
 index 02fc9ab..f18ec8f 100644
 --- a/drivers/power/lp8788-charger.c
 +++ b/drivers/power/lp8788-charger.c
 @@ -235,25 +235,14 @@ static int lp8788_get_battery_present(struct 
 lp8788_charger *pchg,
  return 0;
  }
  
 [...]
  static int lp8788_get_battery_voltage(struct lp8788_charger *pchg,
 @@ -268,7 +257,7 @@ static int lp8788_get_battery_capacity(struct 
 lp8788_charger *pchg,
  struct lp8788 *lp = pchg-lp;
  struct lp8788_charger_platform_data *pdata = pchg-pdata;
  unsigned int max_vbatt;
 -unsigned int vbatt;
 +int vbatt;
  enum lp8788_charging_state state;
  u8 data;
  int ret;
 @@ -304,19 +293,18 @@ static int lp8788_get_battery_temperature(struct 
 lp8788_charger *pchg,
  union power_supply_propval *val)
  {
  struct iio_channel *channel = pchg-chan[LP8788_BATT_TEMP];
 -int scaleint;
 -int scalepart;
 +int result;
  int ret;
  
  if (!channel)
  return -EINVAL;
  
 -ret = iio_read_channel_scale(channel, scaleint, scalepart);
 -if (ret != IIO_VAL_INT_PLUS_MICRO)
 +ret = iio_read_channel_processed(channel, result);
 +if (ret  0)
  return -EINVAL;
  
  /* unit: 0.1 'C */
 -val-intval = (scaleint + scalepart * 100) / 100;
 +val-intval = result * 10;
 
 IIO reports temperatures in milli degree Celsius. So it should be multiplied
 by 100 to get tenth degree like the power supply framework expects it. But
 this might be a issue in your IIO driver reporting the wrong scale.
 
 - Lars
 
  
  return 0;
  }
 
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2 3/3] lp8788-charger: fix wrong ADC conversion

2012-10-19 Thread Lars-Peter Clausen
On 10/19/2012 02:12 AM, Kim, Milo wrote:
>  To get the battery voltage and temperature, IIO ADC functions are used.
>  LP8788 ADC driver provides RAW and SCALE channel information.
>  This patch fixes wrong ADC result.
> 
>  Patch v2.
>  Use simple iio_read_channel_processed() function rather than
>  iio_read_channel_raw() and _scale().
> 
>  Fix the result type of ADC function as a signed integer.
>  Because power_supply_propval.intval and the return value of
>  iio_read_channel_processed() are a signed integer,
>  'unsigned int' are replaced with 'int'.
> 
>  Patch v1.
>  Fix wrong ADC results using iio_read_channel_raw() and _scale().
> 
> Signed-off-by: Milo(Woogyom) Kim 

Looks good to me, fwiw:

Reviewed-by Lars-Peter Clausen 

But there is one issue, but this is not necessarily related to this patch,
more inline.

> ---
>  drivers/power/lp8788-charger.c |   26 +++---
>  1 file changed, 7 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/power/lp8788-charger.c b/drivers/power/lp8788-charger.c
> index 02fc9ab..f18ec8f 100644
> --- a/drivers/power/lp8788-charger.c
> +++ b/drivers/power/lp8788-charger.c
> @@ -235,25 +235,14 @@ static int lp8788_get_battery_present(struct 
> lp8788_charger *pchg,
>   return 0;
>  }
>  
[...]
>  static int lp8788_get_battery_voltage(struct lp8788_charger *pchg,
> @@ -268,7 +257,7 @@ static int lp8788_get_battery_capacity(struct 
> lp8788_charger *pchg,
>   struct lp8788 *lp = pchg->lp;
>   struct lp8788_charger_platform_data *pdata = pchg->pdata;
>   unsigned int max_vbatt;
> - unsigned int vbatt;
> + int vbatt;
>   enum lp8788_charging_state state;
>   u8 data;
>   int ret;
> @@ -304,19 +293,18 @@ static int lp8788_get_battery_temperature(struct 
> lp8788_charger *pchg,
>   union power_supply_propval *val)
>  {
>   struct iio_channel *channel = pchg->chan[LP8788_BATT_TEMP];
> - int scaleint;
> - int scalepart;
> + int result;
>   int ret;
>  
>   if (!channel)
>   return -EINVAL;
>  
> - ret = iio_read_channel_scale(channel, , );
> - if (ret != IIO_VAL_INT_PLUS_MICRO)
> + ret = iio_read_channel_processed(channel, );
> + if (ret < 0)
>   return -EINVAL;
>  
>   /* unit: 0.1 'C */
> - val->intval = (scaleint + scalepart * 100) / 100;
> + val->intval = result * 10;

IIO reports temperatures in milli degree Celsius. So it should be multiplied
by 100 to get tenth degree like the power supply framework expects it. But
this might be a issue in your IIO driver reporting the wrong scale.

- Lars

>  
>   return 0;
>  }

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2 3/3] lp8788-charger: fix wrong ADC conversion

2012-10-19 Thread Lars-Peter Clausen
On 10/19/2012 02:12 AM, Kim, Milo wrote:
  To get the battery voltage and temperature, IIO ADC functions are used.
  LP8788 ADC driver provides RAW and SCALE channel information.
  This patch fixes wrong ADC result.
 
  Patch v2.
  Use simple iio_read_channel_processed() function rather than
  iio_read_channel_raw() and _scale().
 
  Fix the result type of ADC function as a signed integer.
  Because power_supply_propval.intval and the return value of
  iio_read_channel_processed() are a signed integer,
  'unsigned int' are replaced with 'int'.
 
  Patch v1.
  Fix wrong ADC results using iio_read_channel_raw() and _scale().
 
 Signed-off-by: Milo(Woogyom) Kim milo@ti.com

Looks good to me, fwiw:

Reviewed-by Lars-Peter Clausen l...@metafoo.de

But there is one issue, but this is not necessarily related to this patch,
more inline.

 ---
  drivers/power/lp8788-charger.c |   26 +++---
  1 file changed, 7 insertions(+), 19 deletions(-)
 
 diff --git a/drivers/power/lp8788-charger.c b/drivers/power/lp8788-charger.c
 index 02fc9ab..f18ec8f 100644
 --- a/drivers/power/lp8788-charger.c
 +++ b/drivers/power/lp8788-charger.c
 @@ -235,25 +235,14 @@ static int lp8788_get_battery_present(struct 
 lp8788_charger *pchg,
   return 0;
  }
  
[...]
  static int lp8788_get_battery_voltage(struct lp8788_charger *pchg,
 @@ -268,7 +257,7 @@ static int lp8788_get_battery_capacity(struct 
 lp8788_charger *pchg,
   struct lp8788 *lp = pchg-lp;
   struct lp8788_charger_platform_data *pdata = pchg-pdata;
   unsigned int max_vbatt;
 - unsigned int vbatt;
 + int vbatt;
   enum lp8788_charging_state state;
   u8 data;
   int ret;
 @@ -304,19 +293,18 @@ static int lp8788_get_battery_temperature(struct 
 lp8788_charger *pchg,
   union power_supply_propval *val)
  {
   struct iio_channel *channel = pchg-chan[LP8788_BATT_TEMP];
 - int scaleint;
 - int scalepart;
 + int result;
   int ret;
  
   if (!channel)
   return -EINVAL;
  
 - ret = iio_read_channel_scale(channel, scaleint, scalepart);
 - if (ret != IIO_VAL_INT_PLUS_MICRO)
 + ret = iio_read_channel_processed(channel, result);
 + if (ret  0)
   return -EINVAL;
  
   /* unit: 0.1 'C */
 - val-intval = (scaleint + scalepart * 100) / 100;
 + val-intval = result * 10;

IIO reports temperatures in milli degree Celsius. So it should be multiplied
by 100 to get tenth degree like the power supply framework expects it. But
this might be a issue in your IIO driver reporting the wrong scale.

- Lars

  
   return 0;
  }

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v2 3/3] lp8788-charger: fix wrong ADC conversion

2012-10-18 Thread Kim, Milo
 To get the battery voltage and temperature, IIO ADC functions are used.
 LP8788 ADC driver provides RAW and SCALE channel information.
 This patch fixes wrong ADC result.

 Patch v2.
 Use simple iio_read_channel_processed() function rather than
 iio_read_channel_raw() and _scale().

 Fix the result type of ADC function as a signed integer.
 Because power_supply_propval.intval and the return value of
 iio_read_channel_processed() are a signed integer,
 'unsigned int' are replaced with 'int'.

 Patch v1.
 Fix wrong ADC results using iio_read_channel_raw() and _scale().

Signed-off-by: Milo(Woogyom) Kim 
---
 drivers/power/lp8788-charger.c |   26 +++---
 1 file changed, 7 insertions(+), 19 deletions(-)

diff --git a/drivers/power/lp8788-charger.c b/drivers/power/lp8788-charger.c
index 02fc9ab..f18ec8f 100644
--- a/drivers/power/lp8788-charger.c
+++ b/drivers/power/lp8788-charger.c
@@ -235,25 +235,14 @@ static int lp8788_get_battery_present(struct 
lp8788_charger *pchg,
return 0;
 }
 
-static int lp8788_get_vbatt_adc(struct lp8788_charger *pchg,
-   unsigned int *result)
+static int lp8788_get_vbatt_adc(struct lp8788_charger *pchg, int *result)
 {
struct iio_channel *channel = pchg->chan[LP8788_VBATT];
-   int scaleint;
-   int scalepart;
-   int ret;
 
if (!channel)
return -EINVAL;
 
-   ret = iio_read_channel_scale(channel, , );
-   if (ret != IIO_VAL_INT_PLUS_MICRO)
-   return -EINVAL;
-
-   /* unit: mV */
-   *result = (scaleint + scalepart * 100) / 1000;
-
-   return 0;
+   return iio_read_channel_processed(channel, result);
 }
 
 static int lp8788_get_battery_voltage(struct lp8788_charger *pchg,
@@ -268,7 +257,7 @@ static int lp8788_get_battery_capacity(struct 
lp8788_charger *pchg,
struct lp8788 *lp = pchg->lp;
struct lp8788_charger_platform_data *pdata = pchg->pdata;
unsigned int max_vbatt;
-   unsigned int vbatt;
+   int vbatt;
enum lp8788_charging_state state;
u8 data;
int ret;
@@ -304,19 +293,18 @@ static int lp8788_get_battery_temperature(struct 
lp8788_charger *pchg,
union power_supply_propval *val)
 {
struct iio_channel *channel = pchg->chan[LP8788_BATT_TEMP];
-   int scaleint;
-   int scalepart;
+   int result;
int ret;
 
if (!channel)
return -EINVAL;
 
-   ret = iio_read_channel_scale(channel, , );
-   if (ret != IIO_VAL_INT_PLUS_MICRO)
+   ret = iio_read_channel_processed(channel, );
+   if (ret < 0)
return -EINVAL;
 
/* unit: 0.1 'C */
-   val->intval = (scaleint + scalepart * 100) / 100;
+   val->intval = result * 10;
 
return 0;
 }
-- 
1.7.9.5


Best Regards,
Milo


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v2 3/3] lp8788-charger: fix wrong ADC conversion

2012-10-18 Thread Kim, Milo
 To get the battery voltage and temperature, IIO ADC functions are used.
 LP8788 ADC driver provides RAW and SCALE channel information.
 This patch fixes wrong ADC result.

 Patch v2.
 Use simple iio_read_channel_processed() function rather than
 iio_read_channel_raw() and _scale().

 Fix the result type of ADC function as a signed integer.
 Because power_supply_propval.intval and the return value of
 iio_read_channel_processed() are a signed integer,
 'unsigned int' are replaced with 'int'.

 Patch v1.
 Fix wrong ADC results using iio_read_channel_raw() and _scale().

Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
 drivers/power/lp8788-charger.c |   26 +++---
 1 file changed, 7 insertions(+), 19 deletions(-)

diff --git a/drivers/power/lp8788-charger.c b/drivers/power/lp8788-charger.c
index 02fc9ab..f18ec8f 100644
--- a/drivers/power/lp8788-charger.c
+++ b/drivers/power/lp8788-charger.c
@@ -235,25 +235,14 @@ static int lp8788_get_battery_present(struct 
lp8788_charger *pchg,
return 0;
 }
 
-static int lp8788_get_vbatt_adc(struct lp8788_charger *pchg,
-   unsigned int *result)
+static int lp8788_get_vbatt_adc(struct lp8788_charger *pchg, int *result)
 {
struct iio_channel *channel = pchg-chan[LP8788_VBATT];
-   int scaleint;
-   int scalepart;
-   int ret;
 
if (!channel)
return -EINVAL;
 
-   ret = iio_read_channel_scale(channel, scaleint, scalepart);
-   if (ret != IIO_VAL_INT_PLUS_MICRO)
-   return -EINVAL;
-
-   /* unit: mV */
-   *result = (scaleint + scalepart * 100) / 1000;
-
-   return 0;
+   return iio_read_channel_processed(channel, result);
 }
 
 static int lp8788_get_battery_voltage(struct lp8788_charger *pchg,
@@ -268,7 +257,7 @@ static int lp8788_get_battery_capacity(struct 
lp8788_charger *pchg,
struct lp8788 *lp = pchg-lp;
struct lp8788_charger_platform_data *pdata = pchg-pdata;
unsigned int max_vbatt;
-   unsigned int vbatt;
+   int vbatt;
enum lp8788_charging_state state;
u8 data;
int ret;
@@ -304,19 +293,18 @@ static int lp8788_get_battery_temperature(struct 
lp8788_charger *pchg,
union power_supply_propval *val)
 {
struct iio_channel *channel = pchg-chan[LP8788_BATT_TEMP];
-   int scaleint;
-   int scalepart;
+   int result;
int ret;
 
if (!channel)
return -EINVAL;
 
-   ret = iio_read_channel_scale(channel, scaleint, scalepart);
-   if (ret != IIO_VAL_INT_PLUS_MICRO)
+   ret = iio_read_channel_processed(channel, result);
+   if (ret  0)
return -EINVAL;
 
/* unit: 0.1 'C */
-   val-intval = (scaleint + scalepart * 100) / 100;
+   val-intval = result * 10;
 
return 0;
 }
-- 
1.7.9.5


Best Regards,
Milo


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/