Re: [PATCH V2] hwmon: twl4030: Driver for twl4030 madc module

2010-11-25 Thread J, KEERTHY 
On Thu, Nov 11, 2010 at 5:31 AM, Guenter Roeck
guenter.ro...@ericsson.com wrote:
 On Tue, Nov 09, 2010 at 04:20:57AM -0500, Keerthy wrote:
 Introducing a driver for MADC on TWL4030 powerIC. MADC stands for
 monitoring
 ADC. This driver monitors the real time conversion of analog signals
 like
 battery temperature, battery type, battery level etc. User can also ask
 for
 the conversion on a particular channel using the sysfs nodes.

 Signed-off-by: Keerthy j-keer...@ti.com

 Again, I am not sure if this driver belongs into hwmon, since it is not
 really a hardware monitoring chip but a generic adc. We'll have to sort
 that out.

Since hwmon is the place where most of the ADC drivers are residing.
MADC is also an ADC. We feel that hwmon is the right place.


 Code looks much better than before. Still not a complete review; you
 should
 have a much closer look at error handling. I am sure I missed several
 cases
 where error returns are ignored.

 Thanks,
 Guenter

 ---

 Several people have contributed to this driver on the linux-omap list.

 V2:

 Error path correction in probe function.
 Return checks added.
 the_madc pointer could not be removed. The external drivers will not be
 knowing
 device information of the madc.
 Added another function which takes the channel number alone and returns
 the channel reading if the caller wants TWL4030_MADC_SW2 method for ADC
 conversion.
 IOCTL function is removed.
 Work struct is completely removed since request_threaded_irq is used.

  drivers/hwmon/Kconfig            |    6 +
  drivers/hwmon/Makefile           |    1 +
  drivers/hwmon/twl4030-madc.c     |  573
 ++
  include/linux/i2c/twl4030-madc.h |  118 
  4 files changed, 698 insertions(+), 0 deletions(-)
  create mode 100644 drivers/hwmon/twl4030-madc.c
  create mode 100644 include/linux/i2c/twl4030-madc.h

 V1:

 http://www.mail-archive.com/linux-omap@vger.kernel.org/msg34947.html

 diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
 index a56f6ad..fef75f2 100644
 --- a/drivers/hwmon/Kconfig
 +++ b/drivers/hwmon/Kconfig
 @@ -1171,6 +1171,12 @@ config SENSORS_MC13783_ADC
          help
            Support for the A/D converter on MC13783 PMIC.

 +config SENSORS_TWL4030_MADC
 +       tristate
 +       depends on TWL4030_CORE
 +       help
 +         This driver provides support for TWL4030-MADC.
 +

 Besides adding a description, you might alwo want to move this to the
 other
 TI chips.


I will move this to the other TI Chips.

  if ACPI

  comment ACPI drivers
 diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
 index 2479b3d..a54af22 100644
 --- a/drivers/hwmon/Makefile
 +++ b/drivers/hwmon/Makefile
 @@ -100,6 +100,7 @@ obj-$(CONFIG_SENSORS_THMC50)        += thmc50.o
  obj-$(CONFIG_SENSORS_TMP102)   += tmp102.o
  obj-$(CONFIG_SENSORS_TMP401)   += tmp401.o
  obj-$(CONFIG_SENSORS_TMP421)   += tmp421.o
 +obj-$(CONFIG_SENSORS_TWL4030_MADC)+= twl4030-madc.o
  obj-$(CONFIG_SENSORS_VIA_CPUTEMP)+= via-cputemp.o
  obj-$(CONFIG_SENSORS_VIA686A)  += via686a.o
  obj-$(CONFIG_SENSORS_VT1211)   += vt1211.o
 diff --git a/drivers/hwmon/twl4030-madc.c b/drivers/hwmon/twl4030-madc.c
 new file mode 100644
 index 000..42f7d4a
 --- /dev/null
 +++ b/drivers/hwmon/twl4030-madc.c
 @@ -0,0 +1,573 @@
 +/*
 + *
 + * TWL4030 MADC module driver-This driver monitors the real time
 + * conversion of analog signals like battery temperature,
 + * battery type, battery level etc. User can also ask for the
 conversion on a
 + * particular channel using the sysfs nodes.
 + *
 + * Copyright (C) 2010 Texas Instruments Inc.
 + * J Keerthy j-keer...@ti.com
 + *
 + * Based on twl4030-madc.c
 + * Copyright (C) 2008 Nokia Corporation
 + * Mikko Ylinen mikko.k.yli...@nokia.com
 + *
 + * Amit Kucheria amit.kuche...@canonical.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.
 + *
 + * This program is distributed in the hope that it will be useful, but
 + * WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
 + * General Public License for more details.
 + *
 + * You should have received a copy of the GNU General Public License
 + * along with this program; if not, write to the Free Software
 + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
 + * 02110-1301 USA
 + *
 + */
 +
 +#include linux/init.h
 +#include linux/interrupt.h
 +#include linux/kernel.h
 +#include linux/types.h
 +#include linux/module.h
 +#include linux/delay.h
 +#include linux/fs.h
 +#include linux/platform_device.h
 +#include linux/slab.h
 +#include linux/i2c/twl.h
 +#include linux/i2c/twl4030-madc.h
 +#include linux/sysfs.h
 +#include linux/hwmon.h
 +#include linux/hwmon-sysfs.h
 +#include linux/uaccess.h
 +
 +struct twl4030_madc_data {
 +       struct device *hwmon_dev;
 +      

Re: [PATCH V2] hwmon: twl4030: Driver for twl4030 madc module

2010-11-10 Thread Guenter Roeck 
On Tue, Nov 09, 2010 at 04:20:57AM -0500, Keerthy wrote:
 Introducing a driver for MADC on TWL4030 powerIC. MADC stands for monitoring
 ADC. This driver monitors the real time conversion of analog signals like
 battery temperature, battery type, battery level etc. User can also ask for
 the conversion on a particular channel using the sysfs nodes.
 
 Signed-off-by: Keerthy j-keer...@ti.com

Again, I am not sure if this driver belongs into hwmon, since it is not
really a hardware monitoring chip but a generic adc. We'll have to sort that 
out.

Code looks much better than before. Still not a complete review; you should
have a much closer look at error handling. I am sure I missed several cases
where error returns are ignored.

Thanks,
Guenter

 ---
 
 Several people have contributed to this driver on the linux-omap list.
 
 V2:
 
 Error path correction in probe function.
 Return checks added.
 the_madc pointer could not be removed. The external drivers will not be 
 knowing
 device information of the madc.
 Added another function which takes the channel number alone and returns
 the channel reading if the caller wants TWL4030_MADC_SW2 method for ADC 
 conversion.
 IOCTL function is removed.
 Work struct is completely removed since request_threaded_irq is used.
 
  drivers/hwmon/Kconfig|6 +
  drivers/hwmon/Makefile   |1 +
  drivers/hwmon/twl4030-madc.c |  573 
 ++
  include/linux/i2c/twl4030-madc.h |  118 
  4 files changed, 698 insertions(+), 0 deletions(-)
  create mode 100644 drivers/hwmon/twl4030-madc.c
  create mode 100644 include/linux/i2c/twl4030-madc.h
 
 V1:
 
 http://www.mail-archive.com/linux-omap@vger.kernel.org/msg34947.html
 
 diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
 index a56f6ad..fef75f2 100644
 --- a/drivers/hwmon/Kconfig
 +++ b/drivers/hwmon/Kconfig
 @@ -1171,6 +1171,12 @@ config SENSORS_MC13783_ADC
  help
Support for the A/D converter on MC13783 PMIC.
 
 +config SENSORS_TWL4030_MADC
 +   tristate
 +   depends on TWL4030_CORE
 +   help
 + This driver provides support for TWL4030-MADC.
 +

Besides adding a description, you might alwo want to move this to the other 
TI chips.

  if ACPI
 
  comment ACPI drivers
 diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
 index 2479b3d..a54af22 100644
 --- a/drivers/hwmon/Makefile
 +++ b/drivers/hwmon/Makefile
 @@ -100,6 +100,7 @@ obj-$(CONFIG_SENSORS_THMC50)+= thmc50.o
  obj-$(CONFIG_SENSORS_TMP102)   += tmp102.o
  obj-$(CONFIG_SENSORS_TMP401)   += tmp401.o
  obj-$(CONFIG_SENSORS_TMP421)   += tmp421.o
 +obj-$(CONFIG_SENSORS_TWL4030_MADC)+= twl4030-madc.o
  obj-$(CONFIG_SENSORS_VIA_CPUTEMP)+= via-cputemp.o
  obj-$(CONFIG_SENSORS_VIA686A)  += via686a.o
  obj-$(CONFIG_SENSORS_VT1211)   += vt1211.o
 diff --git a/drivers/hwmon/twl4030-madc.c b/drivers/hwmon/twl4030-madc.c
 new file mode 100644
 index 000..42f7d4a
 --- /dev/null
 +++ b/drivers/hwmon/twl4030-madc.c
 @@ -0,0 +1,573 @@
 +/*
 + *
 + * TWL4030 MADC module driver-This driver monitors the real time
 + * conversion of analog signals like battery temperature,
 + * battery type, battery level etc. User can also ask for the conversion on a
 + * particular channel using the sysfs nodes.
 + *
 + * Copyright (C) 2010 Texas Instruments Inc.
 + * J Keerthy j-keer...@ti.com
 + *
 + * Based on twl4030-madc.c
 + * Copyright (C) 2008 Nokia Corporation
 + * Mikko Ylinen mikko.k.yli...@nokia.com
 + *
 + * Amit Kucheria amit.kuche...@canonical.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.
 + *
 + * This program is distributed in the hope that it will be useful, but
 + * WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
 + * General Public License for more details.
 + *
 + * You should have received a copy of the GNU General Public License
 + * along with this program; if not, write to the Free Software
 + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
 + * 02110-1301 USA
 + *
 + */
 +
 +#include linux/init.h
 +#include linux/interrupt.h
 +#include linux/kernel.h
 +#include linux/types.h
 +#include linux/module.h
 +#include linux/delay.h
 +#include linux/fs.h
 +#include linux/platform_device.h
 +#include linux/slab.h
 +#include linux/i2c/twl.h
 +#include linux/i2c/twl4030-madc.h
 +#include linux/sysfs.h
 +#include linux/hwmon.h
 +#include linux/hwmon-sysfs.h
 +#include linux/uaccess.h
 +
 +struct twl4030_madc_data {
 +   struct device *hwmon_dev;
 +   struct mutex lock;
 +   struct twl4030_madc_request requests[TWL4030_MADC_NUM_METHODS];
 +   int imr;
 +   int isr;
 +};
 +
 +static struct twl4030_madc_data *the_madc;
 +
 +static ssize_t madc_read(struct device *dev,
 +

[PATCH V2] hwmon: twl4030: Driver for twl4030 madc module

2010-11-09 Thread Keerthy 
Introducing a driver for MADC on TWL4030 powerIC. MADC stands for monitoring
ADC. This driver monitors the real time conversion of analog signals like
battery temperature, battery type, battery level etc. User can also ask for
the conversion on a particular channel using the sysfs nodes.

Signed-off-by: Keerthy j-keer...@ti.com
---

Several people have contributed to this driver on the linux-omap list.

V2:

Error path correction in probe function.
Return checks added.
the_madc pointer could not be removed. The external drivers will not be knowing
device information of the madc.
Added another function which takes the channel number alone and returns
the channel reading if the caller wants TWL4030_MADC_SW2 method for ADC 
conversion.
IOCTL function is removed.
Work struct is completely removed since request_threaded_irq is used.

 drivers/hwmon/Kconfig|6 +
 drivers/hwmon/Makefile   |1 +
 drivers/hwmon/twl4030-madc.c |  573 ++
 include/linux/i2c/twl4030-madc.h |  118 
 4 files changed, 698 insertions(+), 0 deletions(-)
 create mode 100644 drivers/hwmon/twl4030-madc.c
 create mode 100644 include/linux/i2c/twl4030-madc.h

V1:

http://www.mail-archive.com/linux-omap@vger.kernel.org/msg34947.html

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index a56f6ad..fef75f2 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1171,6 +1171,12 @@ config SENSORS_MC13783_ADC
 help
   Support for the A/D converter on MC13783 PMIC.
 
+config SENSORS_TWL4030_MADC
+   tristate
+   depends on TWL4030_CORE
+   help
+ This driver provides support for TWL4030-MADC.
+
 if ACPI
 
 comment ACPI drivers
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 2479b3d..a54af22 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -100,6 +100,7 @@ obj-$(CONFIG_SENSORS_THMC50)+= thmc50.o
 obj-$(CONFIG_SENSORS_TMP102)   += tmp102.o
 obj-$(CONFIG_SENSORS_TMP401)   += tmp401.o
 obj-$(CONFIG_SENSORS_TMP421)   += tmp421.o
+obj-$(CONFIG_SENSORS_TWL4030_MADC)+= twl4030-madc.o
 obj-$(CONFIG_SENSORS_VIA_CPUTEMP)+= via-cputemp.o
 obj-$(CONFIG_SENSORS_VIA686A)  += via686a.o
 obj-$(CONFIG_SENSORS_VT1211)   += vt1211.o
diff --git a/drivers/hwmon/twl4030-madc.c b/drivers/hwmon/twl4030-madc.c
new file mode 100644
index 000..42f7d4a
--- /dev/null
+++ b/drivers/hwmon/twl4030-madc.c
@@ -0,0 +1,573 @@
+/*
+ *
+ * TWL4030 MADC module driver-This driver monitors the real time
+ * conversion of analog signals like battery temperature,
+ * battery type, battery level etc. User can also ask for the conversion on a
+ * particular channel using the sysfs nodes.
+ *
+ * Copyright (C) 2010 Texas Instruments Inc.
+ * J Keerthy j-keer...@ti.com
+ *
+ * Based on twl4030-madc.c
+ * Copyright (C) 2008 Nokia Corporation
+ * Mikko Ylinen mikko.k.yli...@nokia.com
+ *
+ * Amit Kucheria amit.kuche...@canonical.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.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ *
+ */
+
+#include linux/init.h
+#include linux/interrupt.h
+#include linux/kernel.h
+#include linux/types.h
+#include linux/module.h
+#include linux/delay.h
+#include linux/fs.h
+#include linux/platform_device.h
+#include linux/slab.h
+#include linux/i2c/twl.h
+#include linux/i2c/twl4030-madc.h
+#include linux/sysfs.h
+#include linux/hwmon.h
+#include linux/hwmon-sysfs.h
+#include linux/uaccess.h
+
+struct twl4030_madc_data {
+   struct device *hwmon_dev;
+   struct mutex lock;
+   struct twl4030_madc_request requests[TWL4030_MADC_NUM_METHODS];
+   int imr;
+   int isr;
+};
+
+static struct twl4030_madc_data *the_madc;
+
+static ssize_t madc_read(struct device *dev,
+struct device_attribute *devattr, char *buf)
+{
+   struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+   struct twl4030_madc_request req;
+   int status;
+   long val;
+
+   req.channels = (1  attr-index);
+   req.method = TWL4030_MADC_SW2;
+   req.func_cb = NULL;
+
+   val = twl4030_madc_conversion(req);
+   if (likely(val = 0))
+   val = req.rbuf[attr-index];
+   else
+   return val;
+   status = sprintf(buf, %ld\n, val);
+   return status;
+}
+
+static
+const struct twl4030_madc_conversion_method twl4030_conversion_methods[] = {
+

RE: [PATCH V2] hwmon: twl4030: Driver for twl4030 madc module

2010-11-09 Thread Anand Gadiyar 
 Introducing a driver for MADC on TWL4030 powerIC. MADC stands for
monitoring
 ADC. This driver monitors the real time conversion of analog signals
like
 battery temperature, battery type, battery level etc. User can also ask
for
 the conversion on a particular channel using the sysfs nodes.

 Signed-off-by: Keerthy j-keer...@ti.com

...


 diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
 index a56f6ad..fef75f2 100644
 --- a/drivers/hwmon/Kconfig
 +++ b/drivers/hwmon/Kconfig
 @@ -1171,6 +1171,12 @@ config SENSORS_MC13783_ADC
  help
Support for the A/D converter on MC13783 PMIC.

 +config SENSORS_TWL4030_MADC
 + tristate

You need to provide some label text here, otherwise this option
will not show up in the kernel configuration tools, and hence
cannot be turned on or off from there.

- Anand
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH V2] hwmon: twl4030: Driver for twl4030 madc module

2010-11-09 Thread J, KEERTHY 
Hello Anand,

-Original Message-
From: Gadiyar, Anand 
Sent: Tuesday, November 09, 2010 7:10 PM
To: J, KEERTHY; lm-sens...@lm-sensors.org; guenter.ro...@ericsson.com; 
sa...@linux.intel.com; kh...@linux-fr.org
Cc: mikko.k.yli...@nokia.com; Balbi, Felipe; amit.kuche...@canonical.com; 
linux-omap@vger.kernel.org; Krishnamoorthy, Balaji T
Subject: RE: [PATCH V2] hwmon: twl4030: Driver for twl4030 madc module

 Introducing a driver for MADC on TWL4030 powerIC. MADC stands for
monitoring
 ADC. This driver monitors the real time conversion of analog signals
like
 battery temperature, battery type, battery level etc. User can also ask
for
 the conversion on a particular channel using the sysfs nodes.

 Signed-off-by: Keerthy j-keer...@ti.com

...


 diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
 index a56f6ad..fef75f2 100644
 --- a/drivers/hwmon/Kconfig
 +++ b/drivers/hwmon/Kconfig
 @@ -1171,6 +1171,12 @@ config SENSORS_MC13783_ADC
  help
Support for the A/D converter on MC13783 PMIC.

 +config SENSORS_TWL4030_MADC
 + tristate

You need to provide some label text here, otherwise this option
will not show up in the kernel configuration tools, and hence
cannot be turned on or off from there.

Ok. I get it. I will add a label.

- Anand

Thanks,
Keerthy
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html