[PATCHv1 0/2] Convert rx51-battery to IIO API and add DT support

[PATCHv1 0/2] Convert rx51-battery to IIO API and add DT support

[Sebastian Reichel]

Hi,

This is PATCHv1 for converting rx51-battery to the IIO API
and adding DT support. The patchset compiles and has been
tested on my Nokia N900. It depends on another patchset
converting twl4030-madc to the IIO API:

https://lkml.org/lkml/2014/2/25/627

-- Sebastian

Sebastian Reichel (2):
  rx51_battery: convert to iio consumer
  Documentation: DT: Document rx51-battery binding

 .../devicetree/bindings/power/rx51-battery.txt     | 25 ++++++++
 drivers/power/rx51_battery.c                       | 68 ++++++++++++++--------
 2 files changed, 70 insertions(+), 23 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/power/rx51-battery.txt

-- 
1.8.5.3

[PATCHv1 1/2] rx51_battery: convert to iio consumer

[Sebastian Reichel]

Update rx51-battery driver to use the new IIO API of
twl4030-madc and add DT support.

Signed-off-by: Sebastian Reichel <sre@debian.org>
---
 drivers/power/rx51_battery.c | 68 +++++++++++++++++++++++++++++---------------
 1 file changed, 45 insertions(+), 23 deletions(-)

diff --git a/drivers/power/rx51_battery.c b/drivers/power/rx51_battery.c
index 1bc5857..f7cb58e 100644
--- a/drivers/power/rx51_battery.c
+++ b/drivers/power/rx51_battery.c
@@ -24,34 +24,27 @@
 #include <linux/power_supply.h>
 #include <linux/slab.h>
 #include <linux/i2c/twl4030-madc.h>
-
-/* RX51 specific channels */
-#define TWL4030_MADC_BTEMP_RX51	TWL4030_MADC_ADCIN0
-#define TWL4030_MADC_BCI_RX51	TWL4030_MADC_ADCIN4
+#include <linux/iio/consumer.h>
+#include <linux/of.h>
 
 struct rx51_device_info {
 	struct device *dev;
 	struct power_supply bat;
+	struct iio_channel *channel_temp;
+	struct iio_channel *channel_bsi;
+	struct iio_channel *channel_vbat;
 };
 
 /*
  * Read ADCIN channel value, code copied from maemo kernel
  */
-static int rx51_battery_read_adc(int channel)
+static int rx51_battery_read_adc(struct iio_channel *channel)
 {
-	struct twl4030_madc_request req;
-
-	req.channels = channel;
-	req.do_avg = 1;
-	req.method = TWL4030_MADC_SW1;
-	req.func_cb = NULL;
-	req.type = TWL4030_MADC_WAIT;
-	req.raw = true;
-
-	if (twl4030_madc_conversion(&req) <= 0)
-		return -ENODATA;
-
-	return req.rbuf[ffs(channel) - 1];
+	int val, err;
+	err = iio_read_channel_average_raw(channel, &val);
+	if (err < 0)
+		return err;
+	return val;
 }
 
 /*
@@ -60,10 +53,12 @@ static int rx51_battery_read_adc(int channel)
  */
 static int rx51_battery_read_voltage(struct rx51_device_info *di)
 {
-	int voltage = rx51_battery_read_adc(TWL4030_MADC_VBAT);
+	int voltage = rx51_battery_read_adc(di->channel_vbat);
 
-	if (voltage < 0)
+	if (voltage < 0) {
+		dev_err(di->dev, "Could not read ADC: %d\n", voltage);
 		return voltage;
+	}
 
 	return 1000 * (10000 * voltage / 1705);
 }
@@ -112,7 +107,10 @@ static int rx51_battery_read_temperature(struct rx51_device_info *di)
 {
 	int min = 0;
 	int max = ARRAY_SIZE(rx51_temp_table2) - 1;
-	int raw = rx51_battery_read_adc(TWL4030_MADC_BTEMP_RX51);
+	int raw = rx51_battery_read_adc(di->channel_temp);
+
+	if (raw < 0)
+		dev_err(di->dev, "Could not read ADC: %d\n", raw);
 
 	/* Zero and negative values are undefined */
 	if (raw <= 0)
@@ -146,10 +144,12 @@ static int rx51_battery_read_temperature(struct rx51_device_info *di)
  */
 static int rx51_battery_read_capacity(struct rx51_device_info *di)
 {
-	int capacity = rx51_battery_read_adc(TWL4030_MADC_BCI_RX51);
+	int capacity = rx51_battery_read_adc(di->channel_bsi);
 
-	if (capacity < 0)
+	if (capacity < 0) {
+		dev_err(di->dev, "Could not read ADC: %d\n", capacity);
 		return capacity;
+	}
 
 	return 1280 * (1200 * capacity)/(1024 - capacity);
 }
@@ -213,12 +213,25 @@ static int rx51_battery_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, di);
 
+	di->dev = &pdev->dev;
 	di->bat.name = dev_name(&pdev->dev);
 	di->bat.type = POWER_SUPPLY_TYPE_BATTERY;
 	di->bat.properties = rx51_battery_props;
 	di->bat.num_properties = ARRAY_SIZE(rx51_battery_props);
 	di->bat.get_property = rx51_battery_get_property;
 
+	di->channel_temp = iio_channel_get(di->dev, "temp");
+	if (IS_ERR(di->channel_temp))
+		return PTR_ERR(di->channel_temp);
+
+	di->channel_bsi  = iio_channel_get(di->dev, "bsi");
+	if (IS_ERR(di->channel_bsi))
+		return PTR_ERR(di->channel_bsi);
+
+	di->channel_vbat = iio_channel_get(di->dev, "vbat");
+	if (IS_ERR(di->channel_vbat))
+		return PTR_ERR(di->channel_vbat);
+
 	ret = power_supply_register(di->dev, &di->bat);
 	if (ret)
 		return ret;
@@ -235,12 +248,21 @@ static int rx51_battery_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_OF
+static const struct of_device_id n900_battery_of_match[] = {
+	{.compatible = "nokia,n900-battery", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, n900_battery_of_match);
+#endif
+
 static struct platform_driver rx51_battery_driver = {
 	.probe = rx51_battery_probe,
 	.remove = rx51_battery_remove,
 	.driver = {
 		.name = "rx51-battery",
 		.owner = THIS_MODULE,
+		.of_match_table = of_match_ptr(n900_battery_of_match),
 	},
 };
 module_platform_driver(rx51_battery_driver);
-- 
1.8.5.3

Re: [PATCHv1 1/2] rx51_battery: convert to iio consumer

[Belisko Marek]

Hi Sebastian,

On Wed, Feb 26, 2014 at 1:46 AM, Sebastian Reichel <sre-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org> wrote:
> Update rx51-battery driver to use the new IIO API of
> twl4030-madc and add DT support.
>
> Signed-off-by: Sebastian Reichel <sre-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>
> ---
>  drivers/power/rx51_battery.c | 68 +++++++++++++++++++++++++++++---------------
>  1 file changed, 45 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/power/rx51_battery.c b/drivers/power/rx51_battery.c
> index 1bc5857..f7cb58e 100644
> --- a/drivers/power/rx51_battery.c
> +++ b/drivers/power/rx51_battery.c
> @@ -24,34 +24,27 @@
>  #include <linux/power_supply.h>
>  #include <linux/slab.h>
>  #include <linux/i2c/twl4030-madc.h>
> -
> -/* RX51 specific channels */
> -#define TWL4030_MADC_BTEMP_RX51        TWL4030_MADC_ADCIN0
> -#define TWL4030_MADC_BCI_RX51  TWL4030_MADC_ADCIN4
> +#include <linux/iio/consumer.h>
> +#include <linux/of.h>
>
>  struct rx51_device_info {
>         struct device *dev;
>         struct power_supply bat;
> +       struct iio_channel *channel_temp;
> +       struct iio_channel *channel_bsi;
> +       struct iio_channel *channel_vbat;
>  };
>
>  /*
>   * Read ADCIN channel value, code copied from maemo kernel
>   */
> -static int rx51_battery_read_adc(int channel)
> +static int rx51_battery_read_adc(struct iio_channel *channel)
>  {
> -       struct twl4030_madc_request req;
> -
> -       req.channels = channel;
> -       req.do_avg = 1;
> -       req.method = TWL4030_MADC_SW1;
> -       req.func_cb = NULL;
> -       req.type = TWL4030_MADC_WAIT;
> -       req.raw = true;
> -
> -       if (twl4030_madc_conversion(&req) <= 0)
> -               return -ENODATA;
> -
> -       return req.rbuf[ffs(channel) - 1];
> +       int val, err;
> +       err = iio_read_channel_average_raw(channel, &val);
Where this function comes from? I cannot find it in current linux-next
(only iio_read_channel_raw()).
Am I missing some patches? Thx. BTW when I convert to iio consumer and
use DT some of values work
but some of them just report 0 :( (I don't have latest twl4030-madc
iio patches).
> +       if (err < 0)
> +               return err;
> +       return val;
>  }
>
>  /*
> @@ -60,10 +53,12 @@ static int rx51_battery_read_adc(int channel)
>   */
>  static int rx51_battery_read_voltage(struct rx51_device_info *di)
>  {
> -       int voltage = rx51_battery_read_adc(TWL4030_MADC_VBAT);
> +       int voltage = rx51_battery_read_adc(di->channel_vbat);
>
> -       if (voltage < 0)
> +       if (voltage < 0) {
> +               dev_err(di->dev, "Could not read ADC: %d\n", voltage);
>                 return voltage;
> +       }
>
>         return 1000 * (10000 * voltage / 1705);
>  }
> @@ -112,7 +107,10 @@ static int rx51_battery_read_temperature(struct rx51_device_info *di)
>  {
>         int min = 0;
>         int max = ARRAY_SIZE(rx51_temp_table2) - 1;
> -       int raw = rx51_battery_read_adc(TWL4030_MADC_BTEMP_RX51);
> +       int raw = rx51_battery_read_adc(di->channel_temp);
> +
> +       if (raw < 0)
> +               dev_err(di->dev, "Could not read ADC: %d\n", raw);
>
>         /* Zero and negative values are undefined */
>         if (raw <= 0)
> @@ -146,10 +144,12 @@ static int rx51_battery_read_temperature(struct rx51_device_info *di)
>   */
>  static int rx51_battery_read_capacity(struct rx51_device_info *di)
>  {
> -       int capacity = rx51_battery_read_adc(TWL4030_MADC_BCI_RX51);
> +       int capacity = rx51_battery_read_adc(di->channel_bsi);
>
> -       if (capacity < 0)
> +       if (capacity < 0) {
> +               dev_err(di->dev, "Could not read ADC: %d\n", capacity);
>                 return capacity;
> +       }
>
>         return 1280 * (1200 * capacity)/(1024 - capacity);
>  }
> @@ -213,12 +213,25 @@ static int rx51_battery_probe(struct platform_device *pdev)
>
>         platform_set_drvdata(pdev, di);
>
> +       di->dev = &pdev->dev;
>         di->bat.name = dev_name(&pdev->dev);
>         di->bat.type = POWER_SUPPLY_TYPE_BATTERY;
>         di->bat.properties = rx51_battery_props;
>         di->bat.num_properties = ARRAY_SIZE(rx51_battery_props);
>         di->bat.get_property = rx51_battery_get_property;
>
> +       di->channel_temp = iio_channel_get(di->dev, "temp");
> +       if (IS_ERR(di->channel_temp))
> +               return PTR_ERR(di->channel_temp);
> +
> +       di->channel_bsi  = iio_channel_get(di->dev, "bsi");
> +       if (IS_ERR(di->channel_bsi))
> +               return PTR_ERR(di->channel_bsi);
> +
> +       di->channel_vbat = iio_channel_get(di->dev, "vbat");
> +       if (IS_ERR(di->channel_vbat))
> +               return PTR_ERR(di->channel_vbat);
> +
>         ret = power_supply_register(di->dev, &di->bat);
>         if (ret)
>                 return ret;
> @@ -235,12 +248,21 @@ static int rx51_battery_remove(struct platform_device *pdev)
>         return 0;
>  }
>
> +#ifdef CONFIG_OF
> +static const struct of_device_id n900_battery_of_match[] = {
> +       {.compatible = "nokia,n900-battery", },
> +       { },
> +};
> +MODULE_DEVICE_TABLE(of, n900_battery_of_match);
> +#endif
> +
>  static struct platform_driver rx51_battery_driver = {
>         .probe = rx51_battery_probe,
>         .remove = rx51_battery_remove,
>         .driver = {
>                 .name = "rx51-battery",
>                 .owner = THIS_MODULE,
> +               .of_match_table = of_match_ptr(n900_battery_of_match),
>         },
>  };
>  module_platform_driver(rx51_battery_driver);
> --
> 1.8.5.3
>

BR,

marek

-- 
as simple and primitive as possible
-------------------------------------------------
Marek Belisko - OPEN-NANDRA
Freelance Developer

Ruska Nova Ves 219 | Presov, 08005 Slovak Republic
Tel: +421 915 052 184
skype: marekwhite
twitter: #opennandra
web: http://open-nandra.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv1 1/2] rx51_battery: convert to iio consumer

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 833 bytes --]

Hi,

On Wed, Feb 26, 2014 at 10:43:40PM +0100, Belisko Marek wrote:
> [...]
> > +       int val, err;
> > +       err = iio_read_channel_average_raw(channel, &val);
> Where this function comes from? I cannot find it in current linux-next
> (only iio_read_channel_raw()). Am I missing some patches? Thx.

Ah right. I planned to send a patch adding this function together
with the rx51-battery patchset, but it seems I forgot to include it.

Sorry for the inconvenience. I will send it out as a separate patch
now.

> BTW when I convert to iio consumer and use DT some of values work
> but some of them just report 0 :( (I don't have latest twl4030-madc
> iio patches).

Can you retry with the twl4030-madc iio patch from today? The
older patchsets, which do not contain the "tested on real hw"
note are slightly broken.

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCHv1 1/2] rx51_battery: convert to iio consumer

[Belisko Marek]

Hi Sebastian,

On Wed, Feb 26, 2014 at 10:54 PM, Sebastian Reichel <sre@debian.org> wrote:
> Hi,
>
> On Wed, Feb 26, 2014 at 10:43:40PM +0100, Belisko Marek wrote:
>> [...]
>> > +       int val, err;
>> > +       err = iio_read_channel_average_raw(channel, &val);
>> Where this function comes from? I cannot find it in current linux-next
>> (only iio_read_channel_raw()). Am I missing some patches? Thx.
>
> Ah right. I planned to send a patch adding this function together
> with the rx51-battery patchset, but it seems I forgot to include it.
>
> Sorry for the inconvenience. I will send it out as a separate patch
> now.
>
>> BTW when I convert to iio consumer and use DT some of values work
>> but some of them just report 0 :( (I don't have latest twl4030-madc
>> iio patches).
>
> Can you retry with the twl4030-madc iio patch from today? The
> older patchsets, which do not contain the "tested on real hw"
> note are slightly broken.
Well I've tried and it's worse :). I got during booting:
[    2.218383] ERROR: could not get IIO channel /battery:temp(0)
[    2.224639] platform battery.4: Driver twl4030_madc_battery
requests probe deferral
Not sure if it's just error or warning but temp is always reported as
0 (and also other values in sysfs).

My patches ca be found here:
https://patchwork.kernel.org/patch/3735981/
https://patchwork.kernel.org/patch/3735961/
https://patchwork.kernel.org/patch/3735941/

>
> -- Sebastian

BR,

marek

-- 
as simple and primitive as possible
-------------------------------------------------
Marek Belisko - OPEN-NANDRA
Freelance Developer

Ruska Nova Ves 219 | Presov, 08005 Slovak Republic
Tel: +421 915 052 184
skype: marekwhite
twitter: #opennandra
web: http://open-nandra.com

Re: [PATCHv1 1/2] rx51_battery: convert to iio consumer

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 571 bytes --]

On Thu, Feb 27, 2014 at 10:34:35PM +0100, Belisko Marek wrote:
> Well I've tried and it's worse :). I got during booting:
> [    2.218383] ERROR: could not get IIO channel /battery:temp(0)
> [    2.224639] platform battery.4: Driver twl4030_madc_battery
> requests probe deferral
> Not sure if it's just error or warning but temp is always reported as
> 0 (and also other values in sysfs).

This is an error, which basically means, that twl4030-madc has not
yet been loaded. Do you get proper values when you use the old madc
API with the patchset applied?

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCHv1 1/2] rx51_battery: convert to iio consumer

[Belisko Marek]

Hi Sebastian,

On Fri, Feb 28, 2014 at 3:05 AM, Sebastian Reichel <sre@debian.org> wrote:
> On Thu, Feb 27, 2014 at 10:34:35PM +0100, Belisko Marek wrote:
>> Well I've tried and it's worse :). I got during booting:
>> [    2.218383] ERROR: could not get IIO channel /battery:temp(0)
>> [    2.224639] platform battery.4: Driver twl4030_madc_battery
>> requests probe deferral
>> Not sure if it's just error or warning but temp is always reported as
>> 0 (and also other values in sysfs).
>
> This is an error, which basically means, that twl4030-madc has not
> yet been loaded. Do you get proper values when you use the old madc
> API with the patchset applied?
It works without converting to iio consumer (at least I get some
reasonable values).
With conversion it fails with above error. I recheck (add printk to
iio twl4030-madc) that
madc driver is loaded. Could this be that twl4030_madc_battery is
loaded earlier then
twl4030_madc and than it fails to get iio channels?
>
> -- Sebastian

BR,

marek

-- 
as simple and primitive as possible
-------------------------------------------------
Marek Belisko - OPEN-NANDRA
Freelance Developer

Ruska Nova Ves 219 | Presov, 08005 Slovak Republic
Tel: +421 915 052 184
skype: marekwhite
twitter: #opennandra
web: http://open-nandra.com

Re: [PATCHv1 1/2] rx51_battery: convert to iio consumer

[Belisko Marek]

On Fri, Feb 28, 2014 at 9:32 PM, Belisko Marek <marek.belisko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Hi Sebastian,
>
> On Fri, Feb 28, 2014 at 3:05 AM, Sebastian Reichel <sre-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org> wrote:
>> On Thu, Feb 27, 2014 at 10:34:35PM +0100, Belisko Marek wrote:
>>> Well I've tried and it's worse :). I got during booting:
>>> [    2.218383] ERROR: could not get IIO channel /battery:temp(0)
>>> [    2.224639] platform battery.4: Driver twl4030_madc_battery
>>> requests probe deferral
>>> Not sure if it's just error or warning but temp is always reported as
>>> 0 (and also other values in sysfs).
>>
>> This is an error, which basically means, that twl4030-madc has not
>> yet been loaded. Do you get proper values when you use the old madc
>> API with the patchset applied?
> It works without converting to iio consumer (at least I get some
> reasonable values).
> With conversion it fails with above error. I recheck (add printk to
> iio twl4030-madc) that
> madc driver is loaded. Could this be that twl4030_madc_battery is
> loaded earlier then
> twl4030_madc and than it fails to get iio channels?
Hmm I wasn't far away from truth in previous email. I think that channel doesn't
exists because twl4030_madc driver isn't loaded yet and that is the
reason for deferral probe
message in log. So it seems after some time it is loaded correctly but
not working. No ideas.
>>
>> -- Sebastian
>
> BR,
>
> marek
>
> --
> as simple and primitive as possible
> -------------------------------------------------
> Marek Belisko - OPEN-NANDRA
> Freelance Developer
>
> Ruska Nova Ves 219 | Presov, 08005 Slovak Republic
> Tel: +421 915 052 184
> skype: marekwhite
> twitter: #opennandra
> web: http://open-nandra.com

BR,

marek

-- 
as simple and primitive as possible
-------------------------------------------------
Marek Belisko - OPEN-NANDRA
Freelance Developer

Ruska Nova Ves 219 | Presov, 08005 Slovak Republic
Tel: +421 915 052 184
skype: marekwhite
twitter: #opennandra
web: http://open-nandra.com

Re: [PATCHv1 1/2] rx51_battery: convert to iio consumer

[Jonathan Cameron]

On 26/02/14 00:46, Sebastian Reichel wrote:
> Update rx51-battery driver to use the new IIO API of
> twl4030-madc and add DT support.
>
> Signed-off-by: Sebastian Reichel <sre-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>
The error handling needs tidying up.
Otherwise this looks fine to me.  Note that you (really me)
may get some grief over the DT bindings.  Theoretically we have
been planning to rewrite those entirely for some time...
> ---
>   drivers/power/rx51_battery.c | 68 +++++++++++++++++++++++++++++---------------
>   1 file changed, 45 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/power/rx51_battery.c b/drivers/power/rx51_battery.c
> index 1bc5857..f7cb58e 100644
> --- a/drivers/power/rx51_battery.c
> +++ b/drivers/power/rx51_battery.c
> @@ -24,34 +24,27 @@
>   #include <linux/power_supply.h>
>   #include <linux/slab.h>
>   #include <linux/i2c/twl4030-madc.h>
> -
> -/* RX51 specific channels */
> -#define TWL4030_MADC_BTEMP_RX51	TWL4030_MADC_ADCIN0
> -#define TWL4030_MADC_BCI_RX51	TWL4030_MADC_ADCIN4
> +#include <linux/iio/consumer.h>
> +#include <linux/of.h>
>
>   struct rx51_device_info {
>   	struct device *dev;
>   	struct power_supply bat;
> +	struct iio_channel *channel_temp;
> +	struct iio_channel *channel_bsi;
> +	struct iio_channel *channel_vbat;
>   };
>
>   /*
>    * Read ADCIN channel value, code copied from maemo kernel
>    */
> -static int rx51_battery_read_adc(int channel)
> +static int rx51_battery_read_adc(struct iio_channel *channel)
>   {
> -	struct twl4030_madc_request req;
> -
> -	req.channels = channel;
> -	req.do_avg = 1;
> -	req.method = TWL4030_MADC_SW1;
> -	req.func_cb = NULL;
> -	req.type = TWL4030_MADC_WAIT;
> -	req.raw = true;
> -
> -	if (twl4030_madc_conversion(&req) <= 0)
> -		return -ENODATA;
> -
> -	return req.rbuf[ffs(channel) - 1];
> +	int val, err;
> +	err = iio_read_channel_average_raw(channel, &val);
> +	if (err < 0)
> +		return err;
> +	return val;
>   }
>
>   /*
> @@ -60,10 +53,12 @@ static int rx51_battery_read_adc(int channel)
>    */
>   static int rx51_battery_read_voltage(struct rx51_device_info *di)
>   {
> -	int voltage = rx51_battery_read_adc(TWL4030_MADC_VBAT);
> +	int voltage = rx51_battery_read_adc(di->channel_vbat);
>
> -	if (voltage < 0)
> +	if (voltage < 0) {
> +		dev_err(di->dev, "Could not read ADC: %d\n", voltage);
>   		return voltage;
> +	}
>
>   	return 1000 * (10000 * voltage / 1705);
>   }
> @@ -112,7 +107,10 @@ static int rx51_battery_read_temperature(struct rx51_device_info *di)
>   {
>   	int min = 0;
>   	int max = ARRAY_SIZE(rx51_temp_table2) - 1;
> -	int raw = rx51_battery_read_adc(TWL4030_MADC_BTEMP_RX51);
> +	int raw = rx51_battery_read_adc(di->channel_temp);
> +
> +	if (raw < 0)
> +		dev_err(di->dev, "Could not read ADC: %d\n", raw);
>
>   	/* Zero and negative values are undefined */
>   	if (raw <= 0)
> @@ -146,10 +144,12 @@ static int rx51_battery_read_temperature(struct rx51_device_info *di)
>    */
>   static int rx51_battery_read_capacity(struct rx51_device_info *di)
>   {
> -	int capacity = rx51_battery_read_adc(TWL4030_MADC_BCI_RX51);
> +	int capacity = rx51_battery_read_adc(di->channel_bsi);
>
> -	if (capacity < 0)
> +	if (capacity < 0) {
> +		dev_err(di->dev, "Could not read ADC: %d\n", capacity);
>   		return capacity;
> +	}
>
>   	return 1280 * (1200 * capacity)/(1024 - capacity);
>   }
> @@ -213,12 +213,25 @@ static int rx51_battery_probe(struct platform_device *pdev)
>
>   	platform_set_drvdata(pdev, di);
>
> +	di->dev = &pdev->dev;
>   	di->bat.name = dev_name(&pdev->dev);
>   	di->bat.type = POWER_SUPPLY_TYPE_BATTERY;
>   	di->bat.properties = rx51_battery_props;
>   	di->bat.num_properties = ARRAY_SIZE(rx51_battery_props);
>   	di->bat.get_property = rx51_battery_get_property;
>
> +	di->channel_temp = iio_channel_get(di->dev, "temp");
> +	if (IS_ERR(di->channel_temp))
> +		return PTR_ERR(di->channel_temp);
> +
> +	di->channel_bsi  = iio_channel_get(di->dev, "bsi");
> +	if (IS_ERR(di->channel_bsi))
> +		return PTR_ERR(di->channel_bsi);
You need to unwind the iio_channel_get that did succeed if we get here.
Otherwise references to the iio driver will still be held despite
this driver failing to probe.

> +
> +	di->channel_vbat = iio_channel_get(di->dev, "vbat");
> +	if (IS_ERR(di->channel_vbat))
> +		return PTR_ERR(di->channel_vbat);
> +
>   	ret = power_supply_register(di->dev, &di->bat);
>   	if (ret)
>   		return ret;
> @@ -235,12 +248,21 @@ static int rx51_battery_remove(struct platform_device *pdev)
>   	return 0;
>   }
>
> +#ifdef CONFIG_OF
> +static const struct of_device_id n900_battery_of_match[] = {
> +	{.compatible = "nokia,n900-battery", },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, n900_battery_of_match);
> +#endif
> +
>   static struct platform_driver rx51_battery_driver = {
>   	.probe = rx51_battery_probe,
>   	.remove = rx51_battery_remove,
>   	.driver = {
>   		.name = "rx51-battery",
>   		.owner = THIS_MODULE,
> +		.of_match_table = of_match_ptr(n900_battery_of_match),
>   	},
>   };
>   module_platform_driver(rx51_battery_driver);
>

Re: [PATCHv1 1/2] rx51_battery: convert to iio consumer

[Jonathan Cameron]

On 01/03/14 11:17, Jonathan Cameron wrote:
> On 26/02/14 00:46, Sebastian Reichel wrote:
>> Update rx51-battery driver to use the new IIO API of
>> twl4030-madc and add DT support.
>>
>> Signed-off-by: Sebastian Reichel <sre-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>
> The error handling needs tidying up.
> Otherwise this looks fine to me.  Note that you (really me)
> may get some grief over the DT bindings.  Theoretically we have
> been planning to rewrite those entirely for some time...

It's a longer term point, but it does rather feel like it ought
to be possible to write a generic battery driver sometime down
the line where any conversion characteristics would be in the device
tree.  Definitely a question for the future though!

>> ---
>>   drivers/power/rx51_battery.c | 68 +++++++++++++++++++++++++++++---------------
>>   1 file changed, 45 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/power/rx51_battery.c b/drivers/power/rx51_battery.c
>> index 1bc5857..f7cb58e 100644
>> --- a/drivers/power/rx51_battery.c
>> +++ b/drivers/power/rx51_battery.c
>> @@ -24,34 +24,27 @@
>>   #include <linux/power_supply.h>
>>   #include <linux/slab.h>
>>   #include <linux/i2c/twl4030-madc.h>
>> -
>> -/* RX51 specific channels */
>> -#define TWL4030_MADC_BTEMP_RX51    TWL4030_MADC_ADCIN0
>> -#define TWL4030_MADC_BCI_RX51    TWL4030_MADC_ADCIN4
>> +#include <linux/iio/consumer.h>
>> +#include <linux/of.h>
>>
>>   struct rx51_device_info {
>>       struct device *dev;
>>       struct power_supply bat;
>> +    struct iio_channel *channel_temp;
>> +    struct iio_channel *channel_bsi;
>> +    struct iio_channel *channel_vbat;
>>   };
>>
>>   /*
>>    * Read ADCIN channel value, code copied from maemo kernel
>>    */
>> -static int rx51_battery_read_adc(int channel)
>> +static int rx51_battery_read_adc(struct iio_channel *channel)
>>   {
>> -    struct twl4030_madc_request req;
>> -
>> -    req.channels = channel;
>> -    req.do_avg = 1;
>> -    req.method = TWL4030_MADC_SW1;
>> -    req.func_cb = NULL;
>> -    req.type = TWL4030_MADC_WAIT;
>> -    req.raw = true;
>> -
>> -    if (twl4030_madc_conversion(&req) <= 0)
>> -        return -ENODATA;
>> -
>> -    return req.rbuf[ffs(channel) - 1];
>> +    int val, err;
>> +    err = iio_read_channel_average_raw(channel, &val);
>> +    if (err < 0)
>> +        return err;
>> +    return val;
>>   }
>>
>>   /*
>> @@ -60,10 +53,12 @@ static int rx51_battery_read_adc(int channel)
>>    */
>>   static int rx51_battery_read_voltage(struct rx51_device_info *di)
>>   {
>> -    int voltage = rx51_battery_read_adc(TWL4030_MADC_VBAT);
>> +    int voltage = rx51_battery_read_adc(di->channel_vbat);
>>
>> -    if (voltage < 0)
>> +    if (voltage < 0) {
>> +        dev_err(di->dev, "Could not read ADC: %d\n", voltage);
>>           return voltage;
>> +    }
>>
>>       return 1000 * (10000 * voltage / 1705);
>>   }
>> @@ -112,7 +107,10 @@ static int rx51_battery_read_temperature(struct rx51_device_info *di)
>>   {
>>       int min = 0;
>>       int max = ARRAY_SIZE(rx51_temp_table2) - 1;
>> -    int raw = rx51_battery_read_adc(TWL4030_MADC_BTEMP_RX51);
>> +    int raw = rx51_battery_read_adc(di->channel_temp);
>> +
>> +    if (raw < 0)
>> +        dev_err(di->dev, "Could not read ADC: %d\n", raw);
>>
>>       /* Zero and negative values are undefined */
>>       if (raw <= 0)
>> @@ -146,10 +144,12 @@ static int rx51_battery_read_temperature(struct rx51_device_info *di)
>>    */
>>   static int rx51_battery_read_capacity(struct rx51_device_info *di)
>>   {
>> -    int capacity = rx51_battery_read_adc(TWL4030_MADC_BCI_RX51);
>> +    int capacity = rx51_battery_read_adc(di->channel_bsi);
>>
>> -    if (capacity < 0)
>> +    if (capacity < 0) {
>> +        dev_err(di->dev, "Could not read ADC: %d\n", capacity);
>>           return capacity;
>> +    }
>>
>>       return 1280 * (1200 * capacity)/(1024 - capacity);
>>   }
>> @@ -213,12 +213,25 @@ static int rx51_battery_probe(struct platform_device *pdev)
>>
>>       platform_set_drvdata(pdev, di);
>>
>> +    di->dev = &pdev->dev;
>>       di->bat.name = dev_name(&pdev->dev);
>>       di->bat.type = POWER_SUPPLY_TYPE_BATTERY;
>>       di->bat.properties = rx51_battery_props;
>>       di->bat.num_properties = ARRAY_SIZE(rx51_battery_props);
>>       di->bat.get_property = rx51_battery_get_property;
>>
>> +    di->channel_temp = iio_channel_get(di->dev, "temp");
>> +    if (IS_ERR(di->channel_temp))
>> +        return PTR_ERR(di->channel_temp);
>> +
>> +    di->channel_bsi  = iio_channel_get(di->dev, "bsi");
>> +    if (IS_ERR(di->channel_bsi))
>> +        return PTR_ERR(di->channel_bsi);
> You need to unwind the iio_channel_get that did succeed if we get here.
> Otherwise references to the iio driver will still be held despite
> this driver failing to probe.
>
>> +
>> +    di->channel_vbat = iio_channel_get(di->dev, "vbat");
>> +    if (IS_ERR(di->channel_vbat))
>> +        return PTR_ERR(di->channel_vbat);
>> +
>>       ret = power_supply_register(di->dev, &di->bat);
>>       if (ret)
>>           return ret;
>> @@ -235,12 +248,21 @@ static int rx51_battery_remove(struct platform_device *pdev)
>>       return 0;
>>   }
>>
>> +#ifdef CONFIG_OF
>> +static const struct of_device_id n900_battery_of_match[] = {
>> +    {.compatible = "nokia,n900-battery", },
>> +    { },
>> +};
>> +MODULE_DEVICE_TABLE(of, n900_battery_of_match);
>> +#endif
>> +
>>   static struct platform_driver rx51_battery_driver = {
>>       .probe = rx51_battery_probe,
>>       .remove = rx51_battery_remove,
>>       .driver = {
>>           .name = "rx51-battery",
>>           .owner = THIS_MODULE,
>> +        .of_match_table = of_match_ptr(n900_battery_of_match),
>>       },
>>   };
>>   module_platform_driver(rx51_battery_driver);
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv1 2/2] Documentation: DT: Document rx51-battery binding

[Sebastian Reichel]

Add devicetree binding documentation for rx51-battery,
which is a simple A/D converter consumer.

Signed-off-by: Sebastian Reichel <sre-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>
---
 .../devicetree/bindings/power/rx51-battery.txt     | 25 ++++++++++++++++++++++
 1 file changed, 25 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/rx51-battery.txt

diff --git a/Documentation/devicetree/bindings/power/rx51-battery.txt b/Documentation/devicetree/bindings/power/rx51-battery.txt
new file mode 100644
index 0000000..9043845
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/rx51-battery.txt
@@ -0,0 +1,25 @@
+Binding for Nokia N900 battery
+
+The Nokia N900 battery status can be read via the TWL4030's A/D converter.
+
+Required properties:
+- compatible: Should contain one of the following:
+ * "nokia,n900-battery"
+- io-channels: Should contain IIO channel specifiers
+               for each element in io-channel-names.
+- io-channel-names: Should contain the following values:
+ * "temp" - The ADC channel for temperature reading
+ * "bsi"  - The ADC channel for battery size identification
+ * "vbat" - The ADC channel to measure the battery voltage
+
+Example from Nokia N900:
+
+battery: n900-battery {
+	compatible = "nokia,n900-battery";
+	io-channels = <&twl4030_madc 0>,
+		      <&twl4030_madc 4>,
+		      <&twl4030_madc 12>;
+	io-channel-names = "temp",
+			   "bsi",
+			   "vbat";
+};
-- 
1.8.5.3

Re: [PATCHv1 0/2] Convert rx51-battery to IIO API and add DT support

[Pali Rohár]

Hi!

2014-02-26 1:46 GMT+01:00 Sebastian Reichel <sre-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>:
> Hi,
>
> This is PATCHv1 for converting rx51-battery to the IIO API
> and adding DT support. The patchset compiles and has been
> tested on my Nokia N900. It depends on another patchset
> converting twl4030-madc to the IIO API:
>
> https://lkml.org/lkml/2014/2/25/627
>
> -- Sebastian
>
> Sebastian Reichel (2):
>   rx51_battery: convert to iio consumer
>   Documentation: DT: Document rx51-battery binding
>
>  .../devicetree/bindings/power/rx51-battery.txt     | 25 ++++++++
>  drivers/power/rx51_battery.c                       | 68 ++++++++++++++--------
>  2 files changed, 70 insertions(+), 23 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/power/rx51-battery.txt
>
> --
> 1.8.5.3
>

Thanks for patch!

I would like to ask other kernel developers what do you think about
moving ADC channel numbers from rx51_battery.ko driver code to DT.
Driver rx51_battery.ko is platform specific for Nokia RX-51 (N900) so
it is usefull only for this one device.

Before this patch all driver data (look-up tables, adc channel
numbers, etc...) were in driver code. Now after this patch adc channel
numbers were moved to DT. What do you think? It is better to have all
data in one place (driver code) or some in DT and some in driver code?

For me it does not make sense to move these numbers to DT, because
driver is rx51 device specific and chaning it in DT does not make
sense. And I think it is better to have add driver data in one place
and not in two...

Sebastian already wrote me that this is normal to have numbers in DT
and other code in driver. But I think that driver which can be used
only in one device (so specified only in one DT file) does not need to
have configuration (via DT or board files).

Or do you think that driver specified only for one device needs to
have ADC numbers configuration via DT?

-- 
Pali Rohár
pali.rohar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org

Re: [PATCHv1 0/2] Convert rx51-battery to IIO API and add DT support

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 2803 bytes --]

Hi Pali,

On Wed, Feb 26, 2014 at 08:40:54AM +0100, Pali Rohár wrote:
> I would like to ask other kernel developers what do you think about
> moving ADC channel numbers from rx51_battery.ko driver code to DT.
> Driver rx51_battery.ko is platform specific for Nokia RX-51 (N900) so
> it is usefull only for this one device.
> 
> Before this patch all driver data (look-up tables, adc channel
> numbers, etc...) were in driver code. Now after this patch adc channel
> numbers were moved to DT. What do you think? It is better to have all
> data in one place (driver code) or some in DT and some in driver code?
> 
> For me it does not make sense to move these numbers to DT, because
> driver is rx51 device specific and chaning it in DT does not make
> sense. And I think it is better to have add driver data in one place
> and not in two...
> 
> Sebastian already wrote me that this is normal to have numbers in DT
> and other code in driver. But I think that driver which can be used
> only in one device (so specified only in one DT file) does not need to
> have configuration (via DT or board files).
>
> Or do you think that driver specified only for one device needs to
> have ADC numbers configuration via DT?

I think the problem is, that you think of ADC channel numbers as
configuration data. This means you think of rx51-battery as an
alternative driver for the twl4030-madc.

I think of rx51-battery as its own platform device, which makes use
of the ADC similar to a button making use of a GPIO chip.
For me the ADC channel numbers are not configuration data, but an
inter-device resource reference, like e.g. GPIO references.
This is exactly the data you would put into the device tree.

Now let's take the rx51-audio device as another example for an n900
specific device (not yet in mainline kernel). It does not need ADC
channels, but GPIO lines:

	sound: n900-audio {
		/* ... some more references ... */
		nokia,tvout-selection-gpio = <&gpio2 8 GPIO_ACTIVE_HIGH>; /* 40 */
		nokia,jack-detection-gpio = <&gpio6 17 GPIO_ACTIVE_HIGH>; /* 177 */
		nokia,eci-switch-gpio = <&gpio6 22 GPIO_ACTIVE_HIGH>; /* 182 */
		nokia,speaker-amplifier-gpio = <&twl_gpio 7 GPIO_ACTIVE_HIGH>;
	};

Since GPIO numbers are not guaranteed to be consistent in DT boot
mode at least the GPIO chip must be referenced. Do you really think
it's a good idea to leave out the exact GPIO number just because
the driver knows it needs the 8th GPIO from the second GPIO chip?
IMHO this is really ugly, since it splits the information, which
GPIO is used into two parts - one living in the DT and one living
in the driver with no advantage at all. So it does make sense to
specify inter-device resources via DT even for platform specific
devices.

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCHv2 0/2] Convert rx51-battery to IIO API and add DT support

[Sebastian Reichel]

Hi,

This is PATCHv2 for converting rx51-battery to the IIO API
and adding DT support. The patchset compiles and has been
tested on my Nokia N900. It depends on another patchset
converting twl4030-madc to the IIO API [0].

Changes since PATCHv1 [1]:
 * Release IIO channels on device removal
 * Release IIO channels when errors occur in device probe

[0] https://lkml.org/lkml/2014/3/1/105
[1] Convert rx51-battery to IIO API and add DT support

-- Sebastian

Sebastian Reichel (2):
  rx51_battery: convert to iio consumer
  Documentation: DT: Document rx51-battery binding

 .../devicetree/bindings/power/rx51-battery.txt     | 25 ++++++
 drivers/power/rx51_battery.c                       | 90 ++++++++++++++++------
 2 files changed, 91 insertions(+), 24 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/power/rx51-battery.txt

-- 
1.9.0

[PATCHv2 1/2] rx51_battery: convert to iio consumer

[Sebastian Reichel]

Update rx51-battery driver to use the new IIO API of
twl4030-madc and add DT support.

Signed-off-by: Sebastian Reichel <sre@debian.org>
---
 drivers/power/rx51_battery.c | 90 ++++++++++++++++++++++++++++++++------------
 1 file changed, 66 insertions(+), 24 deletions(-)

diff --git a/drivers/power/rx51_battery.c b/drivers/power/rx51_battery.c
index 1bc5857..d5a2acf 100644
--- a/drivers/power/rx51_battery.c
+++ b/drivers/power/rx51_battery.c
@@ -24,34 +24,27 @@
 #include <linux/power_supply.h>
 #include <linux/slab.h>
 #include <linux/i2c/twl4030-madc.h>
-
-/* RX51 specific channels */
-#define TWL4030_MADC_BTEMP_RX51	TWL4030_MADC_ADCIN0
-#define TWL4030_MADC_BCI_RX51	TWL4030_MADC_ADCIN4
+#include <linux/iio/consumer.h>
+#include <linux/of.h>
 
 struct rx51_device_info {
 	struct device *dev;
 	struct power_supply bat;
+	struct iio_channel *channel_temp;
+	struct iio_channel *channel_bsi;
+	struct iio_channel *channel_vbat;
 };
 
 /*
  * Read ADCIN channel value, code copied from maemo kernel
  */
-static int rx51_battery_read_adc(int channel)
+static int rx51_battery_read_adc(struct iio_channel *channel)
 {
-	struct twl4030_madc_request req;
-
-	req.channels = channel;
-	req.do_avg = 1;
-	req.method = TWL4030_MADC_SW1;
-	req.func_cb = NULL;
-	req.type = TWL4030_MADC_WAIT;
-	req.raw = true;
-
-	if (twl4030_madc_conversion(&req) <= 0)
-		return -ENODATA;
-
-	return req.rbuf[ffs(channel) - 1];
+	int val, err;
+	err = iio_read_channel_average_raw(channel, &val);
+	if (err < 0)
+		return err;
+	return val;
 }
 
 /*
@@ -60,10 +53,12 @@ static int rx51_battery_read_adc(int channel)
  */
 static int rx51_battery_read_voltage(struct rx51_device_info *di)
 {
-	int voltage = rx51_battery_read_adc(TWL4030_MADC_VBAT);
+	int voltage = rx51_battery_read_adc(di->channel_vbat);
 
-	if (voltage < 0)
+	if (voltage < 0) {
+		dev_err(di->dev, "Could not read ADC: %d\n", voltage);
 		return voltage;
+	}
 
 	return 1000 * (10000 * voltage / 1705);
 }
@@ -112,7 +107,10 @@ static int rx51_battery_read_temperature(struct rx51_device_info *di)
 {
 	int min = 0;
 	int max = ARRAY_SIZE(rx51_temp_table2) - 1;
-	int raw = rx51_battery_read_adc(TWL4030_MADC_BTEMP_RX51);
+	int raw = rx51_battery_read_adc(di->channel_temp);
+
+	if (raw < 0)
+		dev_err(di->dev, "Could not read ADC: %d\n", raw);
 
 	/* Zero and negative values are undefined */
 	if (raw <= 0)
@@ -146,10 +144,12 @@ static int rx51_battery_read_temperature(struct rx51_device_info *di)
  */
 static int rx51_battery_read_capacity(struct rx51_device_info *di)
 {
-	int capacity = rx51_battery_read_adc(TWL4030_MADC_BCI_RX51);
+	int capacity = rx51_battery_read_adc(di->channel_bsi);
 
-	if (capacity < 0)
+	if (capacity < 0) {
+		dev_err(di->dev, "Could not read ADC: %d\n", capacity);
 		return capacity;
+	}
 
 	return 1280 * (1200 * capacity)/(1024 - capacity);
 }
@@ -213,17 +213,46 @@ static int rx51_battery_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, di);
 
+	di->dev = &pdev->dev;
 	di->bat.name = dev_name(&pdev->dev);
 	di->bat.type = POWER_SUPPLY_TYPE_BATTERY;
 	di->bat.properties = rx51_battery_props;
 	di->bat.num_properties = ARRAY_SIZE(rx51_battery_props);
 	di->bat.get_property = rx51_battery_get_property;
 
+	di->channel_temp = iio_channel_get(di->dev, "temp");
+	if (IS_ERR(di->channel_temp)) {
+		ret = PTR_ERR(di->channel_temp);
+		goto error;
+	}
+
+	di->channel_bsi  = iio_channel_get(di->dev, "bsi");
+	if (IS_ERR(di->channel_bsi)) {
+		ret = PTR_ERR(di->channel_bsi);
+		goto error_channel_temp;
+	}
+
+	di->channel_vbat = iio_channel_get(di->dev, "vbat");
+	if (IS_ERR(di->channel_vbat)) {
+		ret = PTR_ERR(di->channel_vbat);
+		goto error_channel_bsi;
+	}
+
 	ret = power_supply_register(di->dev, &di->bat);
 	if (ret)
-		return ret;
+		goto error_channel_vbat;
 
 	return 0;
+
+error_channel_vbat:
+	iio_channel_release(di->channel_vbat);
+error_channel_bsi:
+	iio_channel_release(di->channel_bsi);
+error_channel_temp:
+	iio_channel_release(di->channel_temp);
+error:
+
+	return ret;
 }
 
 static int rx51_battery_remove(struct platform_device *pdev)
@@ -232,15 +261,28 @@ static int rx51_battery_remove(struct platform_device *pdev)
 
 	power_supply_unregister(&di->bat);
 
+	iio_channel_release(di->channel_vbat);
+	iio_channel_release(di->channel_bsi);
+	iio_channel_release(di->channel_temp);
+
 	return 0;
 }
 
+#ifdef CONFIG_OF
+static const struct of_device_id n900_battery_of_match[] = {
+	{.compatible = "nokia,n900-battery", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, n900_battery_of_match);
+#endif
+
 static struct platform_driver rx51_battery_driver = {
 	.probe = rx51_battery_probe,
 	.remove = rx51_battery_remove,
 	.driver = {
 		.name = "rx51-battery",
 		.owner = THIS_MODULE,
+		.of_match_table = of_match_ptr(n900_battery_of_match),
 	},
 };
 module_platform_driver(rx51_battery_driver);
-- 
1.9.0

Re: [PATCHv2 1/2] rx51_battery: convert to iio consumer

[Jonathan Cameron]

On 01/03/14 20:22, Sebastian Reichel wrote:
> Update rx51-battery driver to use the new IIO API of
> twl4030-madc and add DT support.
>
> Signed-off-by: Sebastian Reichel <sre-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>
This looks fine to me.  I'd love to see a more generic solution, but
we can work on that another time.

Acked-by: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
>   drivers/power/rx51_battery.c | 90 ++++++++++++++++++++++++++++++++------------
>   1 file changed, 66 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/power/rx51_battery.c b/drivers/power/rx51_battery.c
> index 1bc5857..d5a2acf 100644
> --- a/drivers/power/rx51_battery.c
> +++ b/drivers/power/rx51_battery.c
> @@ -24,34 +24,27 @@
>   #include <linux/power_supply.h>
>   #include <linux/slab.h>
>   #include <linux/i2c/twl4030-madc.h>
> -
> -/* RX51 specific channels */
> -#define TWL4030_MADC_BTEMP_RX51	TWL4030_MADC_ADCIN0
> -#define TWL4030_MADC_BCI_RX51	TWL4030_MADC_ADCIN4
> +#include <linux/iio/consumer.h>
> +#include <linux/of.h>
>
>   struct rx51_device_info {
>   	struct device *dev;
>   	struct power_supply bat;
> +	struct iio_channel *channel_temp;
> +	struct iio_channel *channel_bsi;
> +	struct iio_channel *channel_vbat;
>   };
>
>   /*
>    * Read ADCIN channel value, code copied from maemo kernel
>    */
> -static int rx51_battery_read_adc(int channel)
> +static int rx51_battery_read_adc(struct iio_channel *channel)
>   {
> -	struct twl4030_madc_request req;
> -
> -	req.channels = channel;
> -	req.do_avg = 1;
> -	req.method = TWL4030_MADC_SW1;
> -	req.func_cb = NULL;
> -	req.type = TWL4030_MADC_WAIT;
> -	req.raw = true;
> -
> -	if (twl4030_madc_conversion(&req) <= 0)
> -		return -ENODATA;
> -
> -	return req.rbuf[ffs(channel) - 1];
> +	int val, err;
> +	err = iio_read_channel_average_raw(channel, &val);
> +	if (err < 0)
> +		return err;
> +	return val;
>   }
>
>   /*
> @@ -60,10 +53,12 @@ static int rx51_battery_read_adc(int channel)
>    */
>   static int rx51_battery_read_voltage(struct rx51_device_info *di)
>   {
> -	int voltage = rx51_battery_read_adc(TWL4030_MADC_VBAT);
> +	int voltage = rx51_battery_read_adc(di->channel_vbat);
>
> -	if (voltage < 0)
> +	if (voltage < 0) {
> +		dev_err(di->dev, "Could not read ADC: %d\n", voltage);
>   		return voltage;
> +	}
>
>   	return 1000 * (10000 * voltage / 1705);
>   }
> @@ -112,7 +107,10 @@ static int rx51_battery_read_temperature(struct rx51_device_info *di)
>   {
>   	int min = 0;
>   	int max = ARRAY_SIZE(rx51_temp_table2) - 1;
> -	int raw = rx51_battery_read_adc(TWL4030_MADC_BTEMP_RX51);
> +	int raw = rx51_battery_read_adc(di->channel_temp);
> +
> +	if (raw < 0)
> +		dev_err(di->dev, "Could not read ADC: %d\n", raw);
>
>   	/* Zero and negative values are undefined */
>   	if (raw <= 0)
> @@ -146,10 +144,12 @@ static int rx51_battery_read_temperature(struct rx51_device_info *di)
>    */
>   static int rx51_battery_read_capacity(struct rx51_device_info *di)
>   {
> -	int capacity = rx51_battery_read_adc(TWL4030_MADC_BCI_RX51);
> +	int capacity = rx51_battery_read_adc(di->channel_bsi);
>
> -	if (capacity < 0)
> +	if (capacity < 0) {
> +		dev_err(di->dev, "Could not read ADC: %d\n", capacity);
>   		return capacity;
> +	}
>
>   	return 1280 * (1200 * capacity)/(1024 - capacity);
>   }
> @@ -213,17 +213,46 @@ static int rx51_battery_probe(struct platform_device *pdev)
>
>   	platform_set_drvdata(pdev, di);
>
> +	di->dev = &pdev->dev;
>   	di->bat.name = dev_name(&pdev->dev);
>   	di->bat.type = POWER_SUPPLY_TYPE_BATTERY;
>   	di->bat.properties = rx51_battery_props;
>   	di->bat.num_properties = ARRAY_SIZE(rx51_battery_props);
>   	di->bat.get_property = rx51_battery_get_property;
>
> +	di->channel_temp = iio_channel_get(di->dev, "temp");
> +	if (IS_ERR(di->channel_temp)) {
> +		ret = PTR_ERR(di->channel_temp);
> +		goto error;
> +	}
> +
> +	di->channel_bsi  = iio_channel_get(di->dev, "bsi");
> +	if (IS_ERR(di->channel_bsi)) {
> +		ret = PTR_ERR(di->channel_bsi);
> +		goto error_channel_temp;
> +	}
> +
> +	di->channel_vbat = iio_channel_get(di->dev, "vbat");
> +	if (IS_ERR(di->channel_vbat)) {
> +		ret = PTR_ERR(di->channel_vbat);
> +		goto error_channel_bsi;
> +	}
> +
>   	ret = power_supply_register(di->dev, &di->bat);
>   	if (ret)
> -		return ret;
> +		goto error_channel_vbat;
>
>   	return 0;
> +
> +error_channel_vbat:
> +	iio_channel_release(di->channel_vbat);
> +error_channel_bsi:
> +	iio_channel_release(di->channel_bsi);
> +error_channel_temp:
> +	iio_channel_release(di->channel_temp);
> +error:
> +
> +	return ret;
>   }
>
>   static int rx51_battery_remove(struct platform_device *pdev)
> @@ -232,15 +261,28 @@ static int rx51_battery_remove(struct platform_device *pdev)
>
>   	power_supply_unregister(&di->bat);
>
> +	iio_channel_release(di->channel_vbat);
> +	iio_channel_release(di->channel_bsi);
> +	iio_channel_release(di->channel_temp);
> +
>   	return 0;
>   }
>
> +#ifdef CONFIG_OF
> +static const struct of_device_id n900_battery_of_match[] = {
> +	{.compatible = "nokia,n900-battery", },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, n900_battery_of_match);
> +#endif
> +
>   static struct platform_driver rx51_battery_driver = {
>   	.probe = rx51_battery_probe,
>   	.remove = rx51_battery_remove,
>   	.driver = {
>   		.name = "rx51-battery",
>   		.owner = THIS_MODULE,
> +		.of_match_table = of_match_ptr(n900_battery_of_match),
>   	},
>   };
>   module_platform_driver(rx51_battery_driver);
>

Re: [PATCHv2 1/2] rx51_battery: convert to iio consumer

[Pavel Machek]

On Sat 2014-03-29 11:09:45, Jonathan Cameron wrote:
> On 01/03/14 20:22, Sebastian Reichel wrote:
> >Update rx51-battery driver to use the new IIO API of
> >twl4030-madc and add DT support.
> >
> >Signed-off-by: Sebastian Reichel <sre@debian.org>
> This looks fine to me.  I'd love to see a more generic solution, but
> we can work on that another time.
> 
> Acked-by: Jonathan Cameron <jic23@kernel.org>

Reviewed-by: Pavel Machek <pavel@ucw.cz>

I don't see this patch in 3.15-rc nor -next. What is needed to push
this patch?


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCHv2 1/2] rx51_battery: convert to iio consumer

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1086 bytes --]

On Sun, Apr 20, 2014 at 02:08:23PM +0200, Pavel Machek wrote:
> On Sat 2014-03-29 11:09:45, Jonathan Cameron wrote:
> > On 01/03/14 20:22, Sebastian Reichel wrote:
> > >Update rx51-battery driver to use the new IIO API of
> > >twl4030-madc and add DT support.
> > >
> > >Signed-off-by: Sebastian Reichel <sre-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>
> > This looks fine to me.  I'd love to see a more generic solution, but
> > we can work on that another time.
> > 
> > Acked-by: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> 
> Reviewed-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
> 
> I don't see this patch in 3.15-rc nor -next. What is needed to push
> this patch?

The problem is, that the power subsystem maintainers seem to be too
busy with real life. The last change in their git [0] was 2014-02-01
and I haven't seen any mail from them on the mailinglist since about
the same time. I have just queried his status @ Google+ (there were
some life-signs there).

[0] http://git.infradead.org/battery-2.6.git/

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCHv2 1/2] rx51_battery: convert to iio consumer

[Pavel Machek]

On Wed 2014-04-23 18:09:37, Sebastian Reichel wrote:
> On Sun, Apr 20, 2014 at 02:08:23PM +0200, Pavel Machek wrote:
> > On Sat 2014-03-29 11:09:45, Jonathan Cameron wrote:
> > > On 01/03/14 20:22, Sebastian Reichel wrote:
> > > >Update rx51-battery driver to use the new IIO API of
> > > >twl4030-madc and add DT support.
> > > >
> > > >Signed-off-by: Sebastian Reichel <sre-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>
> > > This looks fine to me.  I'd love to see a more generic solution, but
> > > we can work on that another time.
> > > 
> > > Acked-by: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > 
> > Reviewed-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
> > 
> > I don't see this patch in 3.15-rc nor -next. What is needed to push
> > this patch?
> 
> The problem is, that the power subsystem maintainers seem to be too
> busy with real life. The last change in their git [0] was 2014-02-01
> and I haven't seen any mail from them on the mailinglist since about
> the same time. I have just queried his status @ Google+ (there were
> some life-signs there).

If there is stilll no life out there, you may want to ask
Rafael W. to queue the patches...

-- 
(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 devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2 1/2] rx51_battery: convert to iio consumer

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 870 bytes --]

On Sat, Jun 14, 2014 at 10:32:32AM +0200, Pavel Machek wrote:
> > The problem is, that the power subsystem maintainers seem to be too
> > busy with real life. The last change in their git [0] was 2014-02-01
> > and I haven't seen any mail from them on the mailinglist since about
> > the same time. I have just queried his status @ Google+ (there were
> > some life-signs there).
> 
> If there is still no life out there, you may want to ask
> Rafael W. to queue the patches...

Don't worry, I haven't forgotten the patches. I don't think they
make it into 3.16, but they will go into 3.17. For now I have
put them into a dev branch on [0] and requested to take over the
power supply subsystem (see thread [1] on LKML).

[0] https://git.kernel.org/cgit/linux/kernel/git/sre/linux-power-supply.git
[1] https://lkml.org/lkml/2014/5/16/504

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCHv2 2/2] Documentation: DT: Document rx51-battery binding

[Sebastian Reichel]

Add devicetree binding documentation for rx51-battery,
which is a simple A/D converter consumer.

Signed-off-by: Sebastian Reichel <sre-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>
---
 .../devicetree/bindings/power/rx51-battery.txt     | 25 ++++++++++++++++++++++
 1 file changed, 25 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/rx51-battery.txt

diff --git a/Documentation/devicetree/bindings/power/rx51-battery.txt b/Documentation/devicetree/bindings/power/rx51-battery.txt
new file mode 100644
index 0000000..9043845
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/rx51-battery.txt
@@ -0,0 +1,25 @@
+Binding for Nokia N900 battery
+
+The Nokia N900 battery status can be read via the TWL4030's A/D converter.
+
+Required properties:
+- compatible: Should contain one of the following:
+ * "nokia,n900-battery"
+- io-channels: Should contain IIO channel specifiers
+               for each element in io-channel-names.
+- io-channel-names: Should contain the following values:
+ * "temp" - The ADC channel for temperature reading
+ * "bsi"  - The ADC channel for battery size identification
+ * "vbat" - The ADC channel to measure the battery voltage
+
+Example from Nokia N900:
+
+battery: n900-battery {
+	compatible = "nokia,n900-battery";
+	io-channels = <&twl4030_madc 0>,
+		      <&twl4030_madc 4>,
+		      <&twl4030_madc 12>;
+	io-channel-names = "temp",
+			   "bsi",
+			   "vbat";
+};
-- 
1.9.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2 2/2] Documentation: DT: Document rx51-battery binding

[Jonathan Cameron]

On 01/03/14 20:22, Sebastian Reichel wrote:
> Add devicetree binding documentation for rx51-battery,
> which is a simple A/D converter consumer.
>
> Signed-off-by: Sebastian Reichel <sre-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>
Acked-by: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Clearly this set has some rather large dependencies, but its good to go
once those are in place.

Jonathan
> ---
>   .../devicetree/bindings/power/rx51-battery.txt     | 25 ++++++++++++++++++++++
>   1 file changed, 25 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/power/rx51-battery.txt
>
> diff --git a/Documentation/devicetree/bindings/power/rx51-battery.txt b/Documentation/devicetree/bindings/power/rx51-battery.txt
> new file mode 100644
> index 0000000..9043845
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/rx51-battery.txt
> @@ -0,0 +1,25 @@
> +Binding for Nokia N900 battery
> +
> +The Nokia N900 battery status can be read via the TWL4030's A/D converter.
> +
> +Required properties:
> +- compatible: Should contain one of the following:
> + * "nokia,n900-battery"
> +- io-channels: Should contain IIO channel specifiers
> +               for each element in io-channel-names.
> +- io-channel-names: Should contain the following values:
> + * "temp" - The ADC channel for temperature reading
> + * "bsi"  - The ADC channel for battery size identification
> + * "vbat" - The ADC channel to measure the battery voltage
> +
> +Example from Nokia N900:
> +
> +battery: n900-battery {
> +	compatible = "nokia,n900-battery";
> +	io-channels = <&twl4030_madc 0>,
> +		      <&twl4030_madc 4>,
> +		      <&twl4030_madc 12>;
> +	io-channel-names = "temp",
> +			   "bsi",
> +			   "vbat";
> +};
>

Re: [PATCHv2 2/2] Documentation: DT: Document rx51-battery binding

[Pavel Machek]

On Sat 2014-03-01 21:22:45, Sebastian Reichel wrote:
> Add devicetree binding documentation for rx51-battery,
> which is a simple A/D converter consumer.
> 
> Signed-off-by: Sebastian Reichel <sre-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>

Reviewed-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>

-- 
(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 devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html