linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 2/3] iio: adc: add new lp8788 adc driver
@ 2012-08-09  8:22 Kim, Milo
  2012-08-09 10:15 ` Jonathan Cameron
  0 siblings, 1 reply; 3+ messages in thread
From: Kim, Milo @ 2012-08-09  8:22 UTC (permalink / raw)
  To: jic23; +Cc: linux-kernel, linux-iio, sameo

TI LP8788 has ADC function.
The result of LP878 ADC is used in the LP8788 power supply driver.
(such like getting the battery voltage, temperature and etc)

Signed-off-by: Milo(Woogyom) Kim <milo.kim@ti.com>
---
 drivers/iio/adc/Kconfig      |    6 +
 drivers/iio/adc/Makefile     |    1 +
 drivers/iio/adc/lp8788_adc.c |  240 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 247 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 0000000..30767d5
--- /dev/null
+++ b/drivers/iio/adc/lp8788_adc.c
@@ -0,0 +1,240 @@
+/*
+ * TI LP8788 MFD - ADC driver
+ *
+ * Copyright 2012 Texas Instruments
+ *
+ * Author: Milo(Woogyom) Kim <milo.kim@ti.com>
+ *
+ * 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 <linux/module.h>
+#include <linux/slab.h>
+#include <linux/err.h>
+#include <linux/platform_device.h>
+#include <linux/delay.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/driver.h>
+#include <linux/mfd/lp8788.h>
+
+/* register address */
+#define LP8788_ADC_CONF			0x60
+#define LP8788_ADC_RAW			0x61
+#define LP8788_ADC_DONE			0x63
+
+#define START_ADC_CHANNEL		LPADC_VBATT_5P5
+#define END_ADC_CHANNEL			LPADC_MAX
+#define ADC_CONV_START			1
+#define ADC_CONV_DELAY_US		100
+
+struct lp8788_adc {
+	struct lp8788 *lp;
+	struct iio_dev *indio_dev;
+};
+
+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 unsigned int _get_adc_micro_unit(enum lp8788_adc_id id,
+					unsigned int adc_result)
+{
+	return adc_const[id] * ((adc_result * 1000 + 500) / 1000);
+}
+
+static 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;
+	u8 data, rawdata[2], shift;
+	int size = ARRAY_SIZE(rawdata);
+
+	if (mask != IIO_CHAN_INFO_RAW)
+		return -EINVAL;
+
+	data = (chan->channel << 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;
+
+	shift = chan->scan_type.shift;
+	msb = (rawdata[0] << shift) & 0x00000ff0;
+	lsb = (rawdata[1] >> shift) & 0x0000000f;
+	*val = _get_adc_micro_unit(chan->channel, msb | lsb);
+
+	return IIO_VAL_INT;
+err:
+	return -EINVAL;
+}
+
+static const struct iio_info lp8788_adc_info = {
+	.read_raw = &lp8788_adc_read_raw,
+	.driver_module = THIS_MODULE,
+};
+
+#define LP8788_COMMON_CH(id)					\
+		.indexed = 1,					\
+		.output = 1,					\
+		.channel = LPADC_##id,				\
+		.info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT,	\
+		.address = LP8788_ADC_RAW,			\
+		.scan_type = IIO_ST('u', 8, 12, 4),		\
+		.scan_index = 1,				\
+		.datasheet_name = #id,				\
+
+#define LP8788_V_CHAN(id)	{				\
+		.type = IIO_VOLTAGE,				\
+		LP8788_COMMON_CH(id)				\
+}
+
+#define LP8788_I_CHAN(id)	{				\
+		.type = IIO_CURRENT,				\
+		LP8788_COMMON_CH(id)				\
+}
+
+#define LP8788_T_CHAN(id)	{				\
+		.type = IIO_TEMP,				\
+		LP8788_COMMON_CH(id)				\
+}
+
+static struct iio_chan_spec lp8788_adc_channels[] = {
+	[LPADC_VBATT_5P5] = LP8788_V_CHAN(VBATT_5P5),
+	[LPADC_VIN_CHG]   = LP8788_V_CHAN(VIN_CHG),
+	[LPADC_IBATT]     = LP8788_I_CHAN(IBATT),
+	[LPADC_IC_TEMP]   = LP8788_T_CHAN(IC_TEMP),
+	[LPADC_VBATT_6P0] = LP8788_V_CHAN(VBATT_6P0),
+	[LPADC_VBATT_5P0] = LP8788_V_CHAN(VBATT_5P0),
+	[LPADC_ADC1]      = LP8788_V_CHAN(ADC1),
+	[LPADC_ADC2]      = LP8788_V_CHAN(ADC2),
+	[LPADC_VDD]       = LP8788_V_CHAN(VDD),
+	[LPADC_VCOIN]     = LP8788_V_CHAN(VCOIN),
+	[LPADC_VDD_LDO]   = LP8788_V_CHAN(VDD_LDO),
+	[LPADC_ADC3]      = LP8788_V_CHAN(ADC3),
+	[LPADC_ADC4]      = LP8788_V_CHAN(ADC4),
+};
+
+static int __devinit lp8788_adc_probe(struct platform_device *pdev)
+{
+	struct lp8788 *lp = dev_get_drvdata(pdev->dev.parent);
+	struct iio_map *map;
+	struct lp8788_adc *adc;
+	struct iio_dev *indio_dev;
+	int i, ret;
+
+	adc = devm_kzalloc(lp->dev, sizeof(struct lp8788_adc), GFP_KERNEL);
+	if (!adc)
+		return -ENOMEM;
+
+	indio_dev = iio_device_alloc(sizeof(struct iio_dev));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	adc = iio_priv(indio_dev);
+	adc->lp = lp;
+	adc->indio_dev = indio_dev;
+	platform_set_drvdata(pdev, adc);
+
+	if (lp->pdata) {
+		for (i = START_ADC_CHANNEL; i < END_ADC_CHANNEL ; i++) {
+			map = lp->pdata->adc_pdata[i];
+			ret = iio_map_array_register(indio_dev, map);
+			if (ret) {
+				dev_err(lp->dev, "iio map err: %d\n", ret);
+				goto err_iio_map_array;
+			}
+		}
+	}
+
+	indio_dev->dev.parent = lp->dev;
+	indio_dev->name = pdev->name;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->info = &lp8788_adc_info;
+	indio_dev->channels = lp8788_adc_channels;
+	indio_dev->num_channels = ARRAY_SIZE(lp8788_adc_channels);
+
+	ret = iio_device_register(indio_dev);
+	if (ret) {
+		dev_err(lp->dev, "iio dev register err: %d\n", ret);
+		goto err_iio_register;
+	}
+
+	return 0;
+
+err_iio_map_array:
+	while (--i >= START_ADC_CHANNEL) {
+		map = lp->pdata->adc_pdata[i];
+		iio_map_array_unregister(indio_dev, map);
+	}
+err_iio_register:
+	iio_device_free(indio_dev);
+	return ret;
+}
+
+static int __devexit lp8788_adc_remove(struct platform_device *pdev)
+{
+	struct lp8788_adc *adc = platform_get_drvdata(pdev);
+	struct lp8788 *lp = adc->lp;
+	struct iio_dev *indio_dev = adc->indio_dev;
+	struct iio_map *map;
+	int i;
+
+	if (lp->pdata) {
+		for (i = START_ADC_CHANNEL; i < END_ADC_CHANNEL ; i++) {
+			map = lp->pdata->adc_pdata[i];
+			iio_map_array_unregister(indio_dev, map);
+		}
+	}
+
+	iio_device_unregister(indio_dev);
+	iio_device_free(indio_dev);
+
+	return 0;
+}
+
+static struct platform_driver lp8788_adc_driver = {
+	.probe = lp8788_adc_probe,
+	.remove = __devexit_p(lp8788_adc_remove),
+	.driver = {
+		.name = LP8788_DEV_ADC,
+		.owner = THIS_MODULE,
+	},
+};
+module_platform_driver(lp8788_adc_driver);
+
+MODULE_DESCRIPTION("Texas Instruments LP8788 ADC Driver");
+MODULE_AUTHOR("Milo Kim");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:lp8788-adc");
-- 
1.7.2.5

Best Regards,
Milo



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH 2/3] iio: adc: add new lp8788 adc driver
  2012-08-09  8:22 [PATCH 2/3] iio: adc: add new lp8788 adc driver Kim, Milo
@ 2012-08-09 10:15 ` Jonathan Cameron
  2012-08-10  7:07   ` Kim, Milo
  0 siblings, 1 reply; 3+ messages in thread
From: Jonathan Cameron @ 2012-08-09 10:15 UTC (permalink / raw)
  To: Kim, Milo; +Cc: jic23, linux-kernel, linux-iio, sameo

On 08/09/2012 09:22 AM, Kim, Milo wrote:
> TI LP8788 has ADC function.
> The result of LP878 ADC is used in the LP8788 power supply driver.
> (such like getting the battery voltage, temperature and etc)
> 
Hi,

This is mostly fine though things have gotten a little confused
wrt to the handling iio_priv in the probe and remove so that
needs cleaning up.  A few other minor bits inline.

Thanks,

Jonathan
> Signed-off-by: Milo(Woogyom) Kim <milo.kim@ti.com>
> ---
>  drivers/iio/adc/Kconfig      |    6 +
>  drivers/iio/adc/Makefile     |    1 +
>  drivers/iio/adc/lp8788_adc.c |  240 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 247 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 0000000..30767d5
> --- /dev/null
> +++ b/drivers/iio/adc/lp8788_adc.c
> @@ -0,0 +1,240 @@
> +/*
> + * TI LP8788 MFD - ADC driver
> + *
> + * Copyright 2012 Texas Instruments
> + *
> + * Author: Milo(Woogyom) Kim <milo.kim@ti.com>
> + *
> + * 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 <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/err.h>
> +#include <linux/platform_device.h>
> +#include <linux/delay.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/driver.h>
> +#include <linux/mfd/lp8788.h>
> +
> +/* register address */
> +#define LP8788_ADC_CONF			0x60
> +#define LP8788_ADC_RAW			0x61
> +#define LP8788_ADC_DONE			0x63
> +
> +#define START_ADC_CHANNEL		LPADC_VBATT_5P5
> +#define END_ADC_CHANNEL			LPADC_MAX
> +#define ADC_CONV_START			1
> +#define ADC_CONV_DELAY_US		100
> +
> +struct lp8788_adc {
> +	struct lp8788 *lp;

As explained below I would drop this pointer.
The iio_priv with the suggested changes just stores a
pointer to lp.
> +	struct iio_dev *indio_dev;
> +};
> +
> +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 unsigned int _get_adc_micro_unit(enum lp8788_adc_id id,
> +					unsigned int adc_result)
> +{
> +	return adc_const[id] * ((adc_result * 1000 + 500) / 1000);
> +}
You haven't marked the channels as processed (e.g. converted into correct units)
so I would do these as chan_info elements (scale and offset)
> +
> +static 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;
> +	u8 data, rawdata[2], shift;
> +	int size = ARRAY_SIZE(rawdata);
only used in one place, so do it inline.

> +
> +	if (mask != IIO_CHAN_INFO_RAW)
> +		return -EINVAL;
> +
> +	data = (chan->channel << 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;
> +
This is a little odd. You establish the shift from the channel type, but
then have the masks hard coded. I'd just hard code the lot to
make it slightly easier to read

> +	shift = chan->scan_type.shift;
> +	msb = (rawdata[0] << shift) & 0x00000ff0;
> +	lsb = (rawdata[1] >> shift) & 0x0000000f;
> +	*val = _get_adc_micro_unit(chan->channel, msb | lsb);
> +
> +	return IIO_VAL_INT;
> +err:
> +	return -EINVAL;
> +}
> +
> +static const struct iio_info lp8788_adc_info = {
> +	.read_raw = &lp8788_adc_read_raw,
> +	.driver_module = THIS_MODULE,
> +};
> +
> +#define LP8788_COMMON_CH(id)					\
> +		.indexed = 1,					\
> +		.output = 1,					\
err. these aren't all outputs... Actually I'm not sure any of them are...
> +		.channel = LPADC_##id,				\
> +		.info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT,	\
> +		.address = LP8788_ADC_RAW,			\
> +		.scan_type = IIO_ST('u', 8, 12, 4),		\
> +		.scan_index = 1,				\
> +		.datasheet_name = #id,				\
Given how you are using this macro I'd just make it take an additional
parameter then you can effectively drop all the ones below.
So have
#define LP8788_CHAN(id, type)

> +
> +#define LP8788_V_CHAN(id)	{				\
> +		.type = IIO_VOLTAGE,				\
> +		LP8788_COMMON_CH(id)				\
> +}
> +
> +#define LP8788_I_CHAN(id)	{				\
> +		.type = IIO_CURRENT,				\
> +		LP8788_COMMON_CH(id)				\
> +}
> +
> +#define LP8788_T_CHAN(id)	{				\
> +		.type = IIO_TEMP,				\
> +		LP8788_COMMON_CH(id)				\
> +}
> +
> +static struct iio_chan_spec lp8788_adc_channels[] = {
> +	[LPADC_VBATT_5P5] = LP8788_V_CHAN(VBATT_5P5),
> +	[LPADC_VIN_CHG]   = LP8788_V_CHAN(VIN_CHG),
> +	[LPADC_IBATT]     = LP8788_I_CHAN(IBATT),
> +	[LPADC_IC_TEMP]   = LP8788_T_CHAN(IC_TEMP),
> +	[LPADC_VBATT_6P0] = LP8788_V_CHAN(VBATT_6P0),
> +	[LPADC_VBATT_5P0] = LP8788_V_CHAN(VBATT_5P0),
> +	[LPADC_ADC1]      = LP8788_V_CHAN(ADC1),
> +	[LPADC_ADC2]      = LP8788_V_CHAN(ADC2),
> +	[LPADC_VDD]       = LP8788_V_CHAN(VDD),
> +	[LPADC_VCOIN]     = LP8788_V_CHAN(VCOIN),
> +	[LPADC_VDD_LDO]   = LP8788_V_CHAN(VDD_LDO),
> +	[LPADC_ADC3]      = LP8788_V_CHAN(ADC3),
> +	[LPADC_ADC4]      = LP8788_V_CHAN(ADC4),
> +};
> +
> +static int __devinit lp8788_adc_probe(struct platform_device *pdev)
> +{
> +	struct lp8788 *lp = dev_get_drvdata(pdev->dev.parent);
> +	struct iio_map *map;
> +	struct lp8788_adc *adc;
> +	struct iio_dev *indio_dev;
> +	int i, ret;
> +
> +	adc = devm_kzalloc(lp->dev, sizeof(struct lp8788_adc), GFP_KERNEL);
> +	if (!adc)
> +		return -ENOMEM;
> +
This has me confused. The parameter is for private data (e.g. lp8788_adc here).
It already allocates enough space for a struct iio_dev.   If you want to do
it as here, you'll need to have the parameter as 0.
> +	indio_dev = iio_device_alloc(sizeof(struct iio_dev));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	adc = iio_priv(indio_dev);
That's just set it to point at a region sized for a struct iio_dev.

What I think you meant to do was drop the adc = devm_kzalloc
and do
indio_dev = iio_device_alloc(sizeof(*adc));
> +	adc->lp = lp;
> +	adc->indio_dev = indio_dev;
As a general rule we avoid this sort of reflection back. It's normally
not necessary given the indio_dev should almost always be available...

> +	platform_set_drvdata(pdev, adc);
set that to be the indio_dev rather than adc and you won't need the
adc->indio_dev pointer.
> +
> +	if (lp->pdata) {
> +		for (i = START_ADC_CHANNEL; i < END_ADC_CHANNEL ; i++) {
> +			map = lp->pdata->adc_pdata[i];
> +			ret = iio_map_array_register(indio_dev, map);
> +			if (ret) {
> +				dev_err(lp->dev, "iio map err: %d\n", ret);
> +				goto err_iio_map_array;
> +			}
> +		}
> +	}
> +
> +	indio_dev->dev.parent = lp->dev;
> +	indio_dev->name = pdev->name;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->info = &lp8788_adc_info;
> +	indio_dev->channels = lp8788_adc_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(lp8788_adc_channels);
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret) {
> +		dev_err(lp->dev, "iio dev register err: %d\n", ret);
> +		goto err_iio_register;
> +	}
> +
> +	return 0;
> +
> +err_iio_map_array:
> +	while (--i >= START_ADC_CHANNEL) {
> +		map = lp->pdata->adc_pdata[i];
> +		iio_map_array_unregister(indio_dev, map);
> +	}
> +err_iio_register:
> +	iio_device_free(indio_dev);
> +	return ret;
> +}
> +
> +static int __devexit lp8788_adc_remove(struct platform_device *pdev)
> +{
> +	struct lp8788_adc *adc = platform_get_drvdata(pdev);
> +	struct lp8788 *lp = adc->lp;
if the platform data is set to the indio_dev this isn't needed and
adc can retrieved via iio_priv.
> +	struct iio_dev *indio_dev = adc->indio_dev;
> +	struct iio_map *map;
> +	int i;
> +
> +	if (lp->pdata) {
> +		for (i = START_ADC_CHANNEL; i < END_ADC_CHANNEL ; i++) {
> +			map = lp->pdata->adc_pdata[i];
> +			iio_map_array_unregister(indio_dev, map);
> +		}
> +	}
> +
> +	iio_device_unregister(indio_dev);
> +	iio_device_free(indio_dev);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver lp8788_adc_driver = {
> +	.probe = lp8788_adc_probe,
> +	.remove = __devexit_p(lp8788_adc_remove),
> +	.driver = {
> +		.name = LP8788_DEV_ADC,
> +		.owner = THIS_MODULE,
> +	},
> +};
> +module_platform_driver(lp8788_adc_driver);
> +
> +MODULE_DESCRIPTION("Texas Instruments LP8788 ADC Driver");
> +MODULE_AUTHOR("Milo Kim");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:lp8788-adc");
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* RE: [PATCH 2/3] iio: adc: add new lp8788 adc driver
  2012-08-09 10:15 ` Jonathan Cameron
@ 2012-08-10  7:07   ` Kim, Milo
  0 siblings, 0 replies; 3+ messages in thread
From: Kim, Milo @ 2012-08-10  7:07 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: jic23, linux-kernel, linux-iio, sameo

> This is mostly fine though things have gotten a little confused
> wrt to the handling iio_priv in the probe and remove so that
> needs cleaning up.  A few other minor bits inline.
> 
> Thanks,
> 
> Jonathan

Thanks a lot for detailed review.
Patch v2 has been sent.
Title: [PATCH v2] iio: adc: add new lp8788 adc driver

Thank you.

Best Regards,
Milo




^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2012-08-10  7:09 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-09  8:22 [PATCH 2/3] iio: adc: add new lp8788 adc driver Kim, Milo
2012-08-09 10:15 ` Jonathan Cameron
2012-08-10  7:07   ` Kim, Milo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).