[PATCH v2] iio: adc: add new lp8788 adc driver

2012-08-10 Thread Kim, Milo
Patch v2.
(a) Use iio_priv() for private data rather than allocating data
(b) Support raw and scale inferface for iio consumer
(c) Make inline function for lp8788_adc_read_raw()
(d) For better readability, use fixed number for shift and mask
rather than getting bits from channel scan type
(e) Clean up the iio channel spec macro and use LP8788_CHAN(id, type) macro

Signed-off-by: Milo(Woogyom) Kim 
---
 drivers/iio/adc/Kconfig  |6 +
 drivers/iio/adc/Makefile |1 +
 drivers/iio/adc/lp8788_adc.c |  223 ++
 3 files changed, 230 insertions(+), 0 deletions(-)
 create mode 100644 drivers/iio/adc/lp8788_adc.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 8a78b4f..30c06ed 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -22,4 +22,10 @@ config AT91_ADC
help
  Say yes here to build support for Atmel AT91 ADC.
 
+config LP8788_ADC
+   bool "LP8788 ADC driver"
+   depends on MFD_LP8788
+   help
+ Say yes here to build support for TI LP8788 ADC.
+
 endmenu
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 52eec25..72f9a76 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -4,3 +4,4 @@
 
 obj-$(CONFIG_AD7266) += ad7266.o
 obj-$(CONFIG_AT91_ADC) += at91_adc.o
+obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o
diff --git a/drivers/iio/adc/lp8788_adc.c b/drivers/iio/adc/lp8788_adc.c
new file mode 100644
index 000..7265080
--- /dev/null
+++ b/drivers/iio/adc/lp8788_adc.c
@@ -0,0 +1,223 @@
+/*
+ * TI LP8788 MFD - ADC driver
+ *
+ * Copyright 2012 Texas Instruments
+ *
+ * Author: Milo(Woogyom) Kim 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* register address */
+#define LP8788_ADC_CONF0x60
+#define LP8788_ADC_RAW 0x61
+#define LP8788_ADC_DONE0x63
+
+#define START_ADC_CHANNEL  LPADC_VBATT_5P5
+#define END_ADC_CHANNELLPADC_MAX
+#define ADC_CONV_START 1
+#define ADC_CONV_DELAY_US  100
+
+struct lp8788_adc {
+   struct lp8788 *lp;
+};
+
+static const int adc_const[LPADC_MAX] = {
+   [LPADC_VBATT_5P5] = 1343,
+   [LPADC_VIN_CHG]   = 3052,
+   [LPADC_IBATT] = 610,
+   [LPADC_IC_TEMP]   = 610,
+   [LPADC_VBATT_6P0] = 1465,
+   [LPADC_VBATT_5P0] = 1221,
+   [LPADC_ADC1]  = 610,
+   [LPADC_ADC2]  = 610,
+   [LPADC_VDD]   = 1025,
+   [LPADC_VCOIN] = 757,
+   [LPADC_VDD_LDO]   = 610,
+   [LPADC_ADC3]  = 610,
+   [LPADC_ADC4]  = 610,
+};
+
+static inline int lp8788_adc_read_raw(struct iio_dev *indio_dev,
+   struct iio_chan_spec const *chan,
+   int *val, int *val2, long mask)
+{
+   struct lp8788_adc *adc = iio_priv(indio_dev);
+   int retry = 5;
+   unsigned int msb, lsb, result;
+   u8 data, rawdata[2];
+   int size = ARRAY_SIZE(rawdata);
+   enum lp8788_adc_id id = chan->channel;
+
+   data = (id << 1) | ADC_CONV_START;
+   if (lp8788_write_byte(adc->lp, LP8788_ADC_CONF, data))
+   goto err;
+
+   /* retry until adc conversion is done */
+   data = 0;
+   while (retry--) {
+   udelay(ADC_CONV_DELAY_US);
+
+   if (lp8788_read_byte(adc->lp, LP8788_ADC_DONE, &data))
+   goto err;
+
+   /* conversion done */
+   if (data)
+   break;
+   }
+
+   if (lp8788_read_multi_bytes(adc->lp, LP8788_ADC_RAW, rawdata, size))
+   goto err;
+
+   msb = (rawdata[0] << 4) & 0x0ff0;
+   lsb = (rawdata[1] >> 4) & 0x000f;
+   result = msb | lsb;
+
+   switch (mask) {
+   case IIO_CHAN_INFO_RAW:
+   *val = result;
+   return IIO_VAL_INT;
+   case IIO_CHAN_INFO_SCALE:
+   *val = adc_const[id] * ((result * 1000 + 500) / 1000);
+   *val2 = 0;
+   return IIO_VAL_INT_PLUS_MICRO;
+   default:
+   break;
+   }
+
+err:
+   return -EINVAL;
+}
+
+static const struct iio_info lp8788_adc_info = {
+   .read_raw = &lp8788_adc_read_raw,
+   .driver_module = THIS_MODULE,
+};
+
+#define LP8788_CHAN(_id, _type) {  \
+   .type = _type,  \
+   .indexed = 1,   \
+   .channel = LPADC_##_id, \
+   .info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT |   \
+   IIO_CHAN_INFO_SCALE_SEPARATE_BIT,   \
+   .address = LP8788_ADC_RAW,  

Re: [PATCH v2] iio: adc: add new lp8788 adc driver

2012-08-14 Thread Jonathan Cameron
On 08/10/2012 08:06 AM, Kim, Milo wrote:
> Patch v2.
> (a) Use iio_priv() for private data rather than allocating data
> (b) Support raw and scale inferface for iio consumer
> (c) Make inline function for lp8788_adc_read_raw()
> (d) For better readability, use fixed number for shift and mask
> rather than getting bits from channel scan type
> (e) Clean up the iio channel spec macro and use LP8788_CHAN(id, type) macro
>
One last thing.. You are still allocating the wrong size with iio_device_alloc.


> Signed-off-by: Milo(Woogyom) Kim 
> ---
>  drivers/iio/adc/Kconfig  |6 +
>  drivers/iio/adc/Makefile |1 +
>  drivers/iio/adc/lp8788_adc.c |  223 
> ++
>  3 files changed, 230 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/iio/adc/lp8788_adc.c
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 8a78b4f..30c06ed 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -22,4 +22,10 @@ config AT91_ADC
>   help
> Say yes here to build support for Atmel AT91 ADC.
>
> +config LP8788_ADC
> + bool "LP8788 ADC driver"
> + depends on MFD_LP8788
> + help
> +   Say yes here to build support for TI LP8788 ADC.
> +
>  endmenu
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 52eec25..72f9a76 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -4,3 +4,4 @@
>
>  obj-$(CONFIG_AD7266) += ad7266.o
>  obj-$(CONFIG_AT91_ADC) += at91_adc.o
> +obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o
> diff --git a/drivers/iio/adc/lp8788_adc.c b/drivers/iio/adc/lp8788_adc.c
> new file mode 100644
> index 000..7265080
> --- /dev/null
> +++ b/drivers/iio/adc/lp8788_adc.c
> @@ -0,0 +1,223 @@
> +/*
> + * TI LP8788 MFD - ADC driver
> + *
> + * Copyright 2012 Texas Instruments
> + *
> + * Author: Milo(Woogyom) Kim 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
nitpick of the day. Unnecessary blank line.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/* register address */
> +#define LP8788_ADC_CONF  0x60
> +#define LP8788_ADC_RAW   0x61
> +#define LP8788_ADC_DONE  0x63
> +
> +#define START_ADC_CHANNELLPADC_VBATT_5P5
> +#define END_ADC_CHANNEL  LPADC_MAX
> +#define ADC_CONV_START   1
> +#define ADC_CONV_DELAY_US100
> +
> +struct lp8788_adc {
> + struct lp8788 *lp;
> +};
> +
> +static const int adc_const[LPADC_MAX] = {
> + [LPADC_VBATT_5P5] = 1343,
> + [LPADC_VIN_CHG]   = 3052,
> + [LPADC_IBATT] = 610,
> + [LPADC_IC_TEMP]   = 610,
> + [LPADC_VBATT_6P0] = 1465,
> + [LPADC_VBATT_5P0] = 1221,
> + [LPADC_ADC1]  = 610,
> + [LPADC_ADC2]  = 610,
> + [LPADC_VDD]   = 1025,
> + [LPADC_VCOIN] = 757,
> + [LPADC_VDD_LDO]   = 610,
> + [LPADC_ADC3]  = 610,
> + [LPADC_ADC4]  = 610,
> +};
> +
> +static inline int lp8788_adc_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + struct lp8788_adc *adc = iio_priv(indio_dev);
> + int retry = 5;
> + unsigned int msb, lsb, result;
> + u8 data, rawdata[2];
> + int size = ARRAY_SIZE(rawdata);
> + enum lp8788_adc_id id = chan->channel;
> +
> + data = (id << 1) | ADC_CONV_START;
> + if (lp8788_write_byte(adc->lp, LP8788_ADC_CONF, data))
> + goto err;
> +
> + /* retry until adc conversion is done */
> + data = 0;
> + while (retry--) {
> + udelay(ADC_CONV_DELAY_US);
> +
> + if (lp8788_read_byte(adc->lp, LP8788_ADC_DONE, &data))
> + goto err;
> +
> + /* conversion done */
> + if (data)
> + break;
> + }
> +
> + if (lp8788_read_multi_bytes(adc->lp, LP8788_ADC_RAW, rawdata, size))
> + goto err;
> +
> + msb = (rawdata[0] << 4) & 0x0ff0;
> + lsb = (rawdata[1] >> 4) & 0x000f;
> + result = msb | lsb;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + *val = result;
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_SCALE:
> + *val = adc_const[id] * ((result * 1000 + 500) / 1000);
> + *val2 = 0;
> + return IIO_VAL_INT_PLUS_MICRO;
> + default:
> + break;
> + }
> +
> +err:
> + return -EINVAL;
> +}
> +
> +static const struct iio_info lp8788_adc_info = {
> + .read_raw = &lp8788_adc_read_raw,
> + .driver_module = THIS_MODULE,
> +};
> +
> +#define LP8788_CHAN(_id, _type) {\
> + .type = _type,  

Re: [PATCH v2] iio: adc: add new lp8788 adc driver

2012-08-15 Thread Lars-Peter Clausen
On 08/10/2012 09:06 AM, Kim, Milo wrote:
> [...]
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + *val = result;
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_SCALE:
> + *val = adc_const[id] * ((result * 1000 + 500) / 1000);

This looks wrong. The IIO_CHAN_INFO_SCALE attribute is the factor by which
IIO_CHAN_INFO_RAW needs to be multiplied to get the value in the proper unit,
which is specified in the IIO ABI spec. E.g. milli volts for voltages.

What you return here seems to be the IIO_CHAN_INFO_PROCESSED attribute. Which
basically is raw * scale.

> + *val2 = 0;
> + return IIO_VAL_INT_PLUS_MICRO;
> + default:
> + break;
> + }
> +
> +err:
> + return -EINVAL;
> +}
> +
> [...]
> +}
> +
> +static struct iio_chan_spec lp8788_adc_channels[] = {

const

> + [LPADC_VBATT_5P5] = LP8788_CHAN(VBATT_5P5, IIO_VOLTAGE),
> + [LPADC_VIN_CHG]   = LP8788_CHAN(VIN_CHG, IIO_VOLTAGE),
> + [LPADC_IBATT] = LP8788_CHAN(IBATT, IIO_CURRENT),
> + [LPADC_IC_TEMP]   = LP8788_CHAN(IC_TEMP, IIO_TEMP),
> + [LPADC_VBATT_6P0] = LP8788_CHAN(VBATT_6P0, IIO_VOLTAGE),
> + [LPADC_VBATT_5P0] = LP8788_CHAN(VBATT_5P0, IIO_VOLTAGE),
> + [LPADC_ADC1]  = LP8788_CHAN(ADC1, IIO_VOLTAGE),
> + [LPADC_ADC2]  = LP8788_CHAN(ADC2, IIO_VOLTAGE),
> + [LPADC_VDD]   = LP8788_CHAN(VDD, IIO_VOLTAGE),
> + [LPADC_VCOIN] = LP8788_CHAN(VCOIN, IIO_VOLTAGE),
> + [LPADC_VDD_LDO]   = LP8788_CHAN(VDD_LDO, IIO_VOLTAGE),
> + [LPADC_ADC3]  = LP8788_CHAN(ADC3, IIO_VOLTAGE),
> + [LPADC_ADC4]  = LP8788_CHAN(ADC4, IIO_VOLTAGE),
> +};
> +


--
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] iio: adc: add new lp8788 adc driver

2012-08-15 Thread Kim, Milo
> > +   switch (mask) {
> > +   case IIO_CHAN_INFO_RAW:
> > +   *val = result;
> > +   return IIO_VAL_INT;
> > +   case IIO_CHAN_INFO_SCALE:
> > +   *val = adc_const[id] * ((result * 1000 + 500) / 1000);
> 
> This looks wrong. The IIO_CHAN_INFO_SCALE attribute is the factor by
> which
> IIO_CHAN_INFO_RAW needs to be multiplied to get the value in the proper
> unit,
> which is specified in the IIO ABI spec. E.g. milli volts for voltages.
> 
> What you return here seems to be the IIO_CHAN_INFO_PROCESSED attribute.
> Which
> basically is raw * scale.

Thanks a lot for your review.

Any way to get the result with offset value in the iio-consumer side?
What I need is as below.

  result = raw * scale + offset

At this moment, there are two apis() for reading the iio channel
- iio_read_channel_raw() and iio_read_channel_scale().

Does it sound good if I add iio_read_channel_offset() consumer api
using IIO_CHAN_INFO_OFFSET?

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/


RE: [PATCH v2] iio: adc: add new lp8788 adc driver

2012-08-15 Thread Jonathan Cameron


"Kim, Milo"  wrote:

>> > +  switch (mask) {
>> > +  case IIO_CHAN_INFO_RAW:
>> > +  *val = result;
>> > +  return IIO_VAL_INT;
>> > +  case IIO_CHAN_INFO_SCALE:
>> > +  *val = adc_const[id] * ((result * 1000 + 500) / 1000);
>> 
>> This looks wrong. The IIO_CHAN_INFO_SCALE attribute is the factor by
>> which
>> IIO_CHAN_INFO_RAW needs to be multiplied to get the value in the
>proper
>> unit,
>> which is specified in the IIO ABI spec. E.g. milli volts for
>voltages.
>> 
>> What you return here seems to be the IIO_CHAN_INFO_PROCESSED
>attribute.
>> Which
>> basically is raw * scale.
>
>Thanks a lot for your review.
>
>Any way to get the result with offset value in the iio-consumer side?
>What I need is as below.
>
>  result = raw * scale + offset
>
>At this moment, there are two apis() for reading the iio channel
>- iio_read_channel_raw() and iio_read_channel_scale().
>
>Does it sound good if I add iio_read_channel_offset() consumer api
>using IIO_CHAN_INFO_OFFSET?

Yes. Please add that.
>
>Best Regards,
>Milo

-- 
Sent from my Android phone with K-9 Mail. Please excuse my brevity.
--
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/