* [PATCH V2] hwmon: twl4030: Driver for twl4030 madc module
@ 2010-11-09 9:32 ` Keerthy
0 siblings, 0 replies; 10+ messages in thread
From: Keerthy @ 2010-11-09 9:20 UTC (permalink / raw)
To: lm-sensors, guenter.roeck, sameo, khali
Cc: mikko.k.ylinen, balbi, amit.kucheria, linux-omap, balajitk,
j-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-keerthy@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 0000000..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-keerthy@ti.com>
+ *
+ * Based on twl4030-madc.c
+ * Copyright (C) 2008 Nokia Corporation
+ * Mikko Ylinen <mikko.k.ylinen@nokia.com>
+ *
+ * Amit Kucheria <amit.kucheria@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[] = {
+ [TWL4030_MADC_RT] = {
+ .sel = TWL4030_MADC_RTSELECT_LSB,
+ .avg = TWL4030_MADC_RTAVERAGE_LSB,
+ .rbase = TWL4030_MADC_RTCH0_LSB,
+ },
+ [TWL4030_MADC_SW1] = {
+ .sel = TWL4030_MADC_SW1SELECT_LSB,
+ .avg = TWL4030_MADC_SW1AVERAGE_LSB,
+ .rbase = TWL4030_MADC_GPCH0_LSB,
+ .ctrl = TWL4030_MADC_CTRL_SW1,
+ },
+ [TWL4030_MADC_SW2] = {
+ .sel = TWL4030_MADC_SW2SELECT_LSB,
+ .avg = TWL4030_MADC_SW2AVERAGE_LSB,
+ .rbase = TWL4030_MADC_GPCH0_LSB,
+ .ctrl = TWL4030_MADC_CTRL_SW2,
+ },
+};
+
+static int twl4030_madc_read(struct twl4030_madc_data *madc, u8 reg)
+{
+ int ret;
+ u8 val;
+
+ ret = twl_i2c_read_u8(TWL4030_MODULE_MADC, &val, reg);
+ if (ret) {
+ dev_dbg(madc->hwmon_dev, "unable to read register 0x%X\n", reg);
+ return ret;
+ }
+
+ return val;
+}
+
+static void twl4030_madc_write(struct twl4030_madc_data *madc, u8 reg, u8 val)
+{
+ int ret;
+
+ ret = twl_i2c_write_u8(TWL4030_MODULE_MADC, val, reg);
+ if (ret)
+ dev_err(madc->hwmon_dev,
+ "unable to write register 0x%X\n", reg);
+}
+
+static int twl4030_madc_channel_raw_read(struct twl4030_madc_data *madc, u8 reg)
+{
+ u8 msb, lsb;
+
+ /*
+ * For each ADC channel, we have MSB and LSB register pair. MSB address
+ * is always LSB address+1. reg parameter is the addr of LSB register
+ */
+ msb = twl4030_madc_read(madc, reg + 1);
+ lsb = twl4030_madc_read(madc, reg);
+
+ return (int)(((msb << 8) | lsb) >> 6);
+}
+
+static int twl4030_madc_read_channels(struct twl4030_madc_data *madc,
+ u8 reg_base, u16 channels, int *buf)
+{
+ int count = 0;
+ u8 reg, i;
+
+ if (unlikely(!buf))
+ return 0;
+
+ for (i = 0; i < TWL4030_MADC_MAX_CHANNELS; i++) {
+ if (channels & (1 << i)) {
+ reg = reg_base + 2 * i;
+ buf[i] = twl4030_madc_channel_raw_read(madc, reg);
+ count++;
+ }
+ }
+ return count;
+}
+
+static void twl4030_madc_enable_irq(struct twl4030_madc_data *madc, int id)
+{
+ u8 val;
+
+ val = twl4030_madc_read(madc, madc->imr);
+ val &= ~(1 << id);
+ twl4030_madc_write(madc, madc->imr, val);
+}
+
+static void twl4030_madc_disable_irq(struct twl4030_madc_data *madc, int id)
+{
+ u8 val;
+
+ val = twl4030_madc_read(madc, madc->imr);
+ val |= (1 << id);
+ twl4030_madc_write(madc, madc->imr, val);
+}
+
+static irqreturn_t twl4030_madc_irq_handler(int irq, void *_madc)
+{
+ struct twl4030_madc_data *madc = _madc;
+ const struct twl4030_madc_conversion_method *method;
+ u8 isr_val, imr_val;
+ int i, len;
+ struct twl4030_madc_request *r;
+
+ isr_val = twl4030_madc_read(madc, madc->isr);
+ imr_val = twl4030_madc_read(madc, madc->imr);
+
+ isr_val &= ~imr_val;
+
+ for (i = 0; i < TWL4030_MADC_NUM_METHODS; i++) {
+
+ if (!(isr_val & (1 << i)))
+ continue;
+
+ twl4030_madc_disable_irq(madc, i);
+ madc->requests[i].result_pending = 1;
+ }
+ mutex_lock(&madc->lock);
+ for (i = 0; i < TWL4030_MADC_NUM_METHODS; i++) {
+
+ r = &madc->requests[i];
+
+ /* No pending results for this method, move to next one */
+ if (!r->result_pending)
+ continue;
+
+ method = &twl4030_conversion_methods[r->method];
+
+ /* Read results */
+ len = twl4030_madc_read_channels(madc, method->rbase,
+ r->channels, r->rbuf);
+ /* Return results to caller */
+ if (r->func_cb != NULL) {
+ r->func_cb(len, r->channels, r->rbuf);
+ r->func_cb = NULL;
+ }
+
+ /* Free request */
+ r->result_pending = 0;
+ r->active = 0;
+ }
+
+ mutex_unlock(&madc->lock);
+
+ return IRQ_HANDLED;
+}
+
+static int twl4030_madc_set_irq(struct twl4030_madc_data *madc,
+ struct twl4030_madc_request *req)
+{
+ struct twl4030_madc_request *p;
+
+ p = &madc->requests[req->method];
+
+ memcpy(p, req, sizeof *req);
+
+ twl4030_madc_enable_irq(madc, req->method);
+
+ return 0;
+}
+
+static inline void twl4030_madc_start_conversion(struct twl4030_madc_data *madc,
+ int conv_method)
+{
+ const struct twl4030_madc_conversion_method *method;
+
+ method = &twl4030_conversion_methods[conv_method];
+
+ switch (conv_method) {
+ case TWL4030_MADC_SW1:
+ case TWL4030_MADC_SW2:
+ twl4030_madc_write(madc, method->ctrl, TWL4030_MADC_SW_START);
+ break;
+ case TWL4030_MADC_RT:
+ default:
+ break;
+ }
+}
+
+static int twl4030_madc_wait_conversion_ready(struct twl4030_madc_data *madc,
+ unsigned int timeout_ms,
+ u8 status_reg)
+{
+ unsigned long timeout;
+
+ timeout = jiffies + msecs_to_jiffies(timeout_ms);
+ do {
+ u8 reg;
+
+ reg = twl4030_madc_read(madc, status_reg);
+ if (!(reg & TWL4030_MADC_BUSY) && (reg & TWL4030_MADC_EOC_SW))
+ return 0;
+ } while (!time_after(jiffies, timeout));
+
+ return -EAGAIN;
+}
+
+int twl4030_madc_conversion(struct twl4030_madc_request *req)
+{
+ const struct twl4030_madc_conversion_method *method;
+ u8 ch_msb, ch_lsb;
+ int ret;
+
+ if (unlikely(!req))
+ return -EINVAL;
+
+ mutex_lock(&the_madc->lock);
+
+ if (req->method < TWL4030_MADC_RT || req->method > TWL4030_MADC_SW2) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ /* Do we have a conversion request ongoing */
+ if (the_madc->requests[req->method].active) {
+ ret = -EBUSY;
+ goto out;
+ }
+
+ ch_msb = (req->channels >> 8) & 0xff;
+ ch_lsb = req->channels & 0xff;
+
+ method = &twl4030_conversion_methods[req->method];
+
+ /* Select channels to be converted */
+ twl4030_madc_write(the_madc, method->sel + 1, ch_msb);
+ twl4030_madc_write(the_madc, method->sel, ch_lsb);
+
+ /* Select averaging for all channels if do_avg is set */
+ if (req->do_avg) {
+ twl4030_madc_write(the_madc, method->avg + 1, ch_msb);
+ twl4030_madc_write(the_madc, method->avg, ch_lsb);
+ }
+
+ if (req->type == TWL4030_MADC_IRQ_ONESHOT && req->func_cb != NULL) {
+ twl4030_madc_set_irq(the_madc, req);
+ twl4030_madc_start_conversion(the_madc, req->method);
+ the_madc->requests[req->method].active = 1;
+ ret = 0;
+ goto out;
+ }
+
+ /* With RT method we should not be here anymore */
+ if (req->method == TWL4030_MADC_RT) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ twl4030_madc_start_conversion(the_madc, req->method);
+ the_madc->requests[req->method].active = 1;
+
+ /* Wait until conversion is ready (ctrl register returns EOC) */
+ ret = twl4030_madc_wait_conversion_ready(the_madc, 5, method->ctrl);
+ if (ret) {
+ dev_dbg(the_madc->hwmon_dev, "conversion timeout!\n");
+ the_madc->requests[req->method].active = 0;
+ goto out;
+ }
+
+ ret = twl4030_madc_read_channels(the_madc, method->rbase, req->channels,
+ req->rbuf);
+
+ the_madc->requests[req->method].active = 0;
+
+out:
+ mutex_unlock(&the_madc->lock);
+
+ return ret;
+}
+EXPORT_SYMBOL(twl4030_madc_conversion);
+
+/*
+ * Return channel value
+ * Or < 0 on failure.
+ */
+static int twl4030_get_madc_conversion(int channel_no)
+{
+ struct twl4030_madc_request req;
+ int temp = 0;
+ int ret;
+
+ req.channels = (1 << channel_no);
+ req.method = TWL4030_MADC_SW2;
+ req.active = 0;
+ req.func_cb = NULL;
+ ret = twl4030_madc_conversion(&req);
+ if (ret < 0)
+ return ret;
+
+ if (req.rbuf[channel_no] > 0)
+ temp = req.rbuf[channel_no];
+
+ return temp;
+}
+EXPORT_SYMBOL(twl4030_get_madc_conversion);
+
+static void twl4030_madc_set_current_generator(struct twl4030_madc_data *madc,
+ int chan, int on)
+{
+ int ret;
+ u8 regval;
+
+ ret = twl_i2c_read_u8(TWL4030_MODULE_MAIN_CHARGE,
+ ®val, TWL4030_BCI_BCICTL1);
+ if (ret) {
+ dev_dbg(madc->hwmon_dev,
+ "unable to read register 0x%X\n", TWL4030_BCI_BCICTL1);
+ }
+ if (on)
+ regval |= chan ? TWL4030_BCI_ITHEN : TWL4030_BCI_TYPEN;
+ else
+ regval &= chan ? ~TWL4030_BCI_ITHEN : ~TWL4030_BCI_TYPEN;
+ ret = twl_i2c_write_u8(TWL4030_MODULE_MAIN_CHARGE,
+ regval, TWL4030_BCI_BCICTL1);
+ if (ret)
+ dev_err(madc->hwmon_dev,
+ "unable to write register 0x%X\n", TWL4030_BCI_BCICTL1);
+
+}
+
+static void twl4030_madc_set_power(struct twl4030_madc_data *madc, int on)
+{
+ u8 regval;
+
+ regval = twl4030_madc_read(madc, TWL4030_MADC_CTRL1);
+ if (on)
+ regval |= TWL4030_MADC_MADCON;
+ else
+ regval &= ~TWL4030_MADC_MADCON;
+ twl4030_madc_write(madc, TWL4030_MADC_CTRL1, regval);
+}
+
+static SENSOR_DEVICE_ATTR(in0_input, S_IRUGO, madc_read, NULL, 0);
+static SENSOR_DEVICE_ATTR(in1_input, S_IRUGO, madc_read, NULL, 1);
+static SENSOR_DEVICE_ATTR(in2_input, S_IRUGO, madc_read, NULL, 2);
+static SENSOR_DEVICE_ATTR(in3_input, S_IRUGO, madc_read, NULL, 3);
+static SENSOR_DEVICE_ATTR(in4_input, S_IRUGO, madc_read, NULL, 4);
+static SENSOR_DEVICE_ATTR(in5_input, S_IRUGO, madc_read, NULL, 5);
+static SENSOR_DEVICE_ATTR(in6_input, S_IRUGO, madc_read, NULL, 6);
+static SENSOR_DEVICE_ATTR(in7_input, S_IRUGO, madc_read, NULL, 7);
+static SENSOR_DEVICE_ATTR(in8_input, S_IRUGO, madc_read, NULL, 8);
+static SENSOR_DEVICE_ATTR(in9_input, S_IRUGO, madc_read, NULL, 9);
+static SENSOR_DEVICE_ATTR(in10_input, S_IRUGO, madc_read, NULL, 10);
+static SENSOR_DEVICE_ATTR(in11_input, S_IRUGO, madc_read, NULL, 11);
+static SENSOR_DEVICE_ATTR(in12_input, S_IRUGO, madc_read, NULL, 12);
+static SENSOR_DEVICE_ATTR(in13_input, S_IRUGO, madc_read, NULL, 13);
+static SENSOR_DEVICE_ATTR(in14_input, S_IRUGO, madc_read, NULL, 14);
+static SENSOR_DEVICE_ATTR(in15_input, S_IRUGO, madc_read, NULL, 15);
+
+static struct attribute *twl4030_madc_attributes[] = {
+ &sensor_dev_attr_in0_input.dev_attr.attr,
+ &sensor_dev_attr_in1_input.dev_attr.attr,
+ &sensor_dev_attr_in2_input.dev_attr.attr,
+ &sensor_dev_attr_in3_input.dev_attr.attr,
+ &sensor_dev_attr_in4_input.dev_attr.attr,
+ &sensor_dev_attr_in5_input.dev_attr.attr,
+ &sensor_dev_attr_in6_input.dev_attr.attr,
+ &sensor_dev_attr_in7_input.dev_attr.attr,
+ &sensor_dev_attr_in8_input.dev_attr.attr,
+ &sensor_dev_attr_in9_input.dev_attr.attr,
+ &sensor_dev_attr_in10_input.dev_attr.attr,
+ &sensor_dev_attr_in11_input.dev_attr.attr,
+ &sensor_dev_attr_in12_input.dev_attr.attr,
+ &sensor_dev_attr_in13_input.dev_attr.attr,
+ &sensor_dev_attr_in14_input.dev_attr.attr,
+ &sensor_dev_attr_in15_input.dev_attr.attr,
+ NULL
+};
+
+static const struct attribute_group twl4030_madc_group = {
+ .attrs = twl4030_madc_attributes,
+};
+
+static int __devinit twl4030_madc_probe(struct platform_device *pdev)
+{
+ struct twl4030_madc_data *madc;
+ struct twl4030_madc_platform_data *pdata = pdev->dev.platform_data;
+ int ret;
+ int status;
+ u8 regval;
+
+ madc = kzalloc(sizeof *madc, GFP_KERNEL);
+ if (!madc)
+ return -ENOMEM;
+
+ if (!pdata) {
+ dev_dbg(&pdev->dev, "platform_data not available\n");
+ ret = -EINVAL;
+ goto err_misc;
+ }
+ madc->imr = (pdata->irq_line == 1) ?
+ TWL4030_MADC_IMR1 : TWL4030_MADC_IMR2;
+ madc->isr = (pdata->irq_line == 1) ?
+ TWL4030_MADC_ISR1 : TWL4030_MADC_ISR2;
+ twl4030_madc_set_power(madc, 1);
+
+ twl4030_madc_set_current_generator(madc, 0, 1);
+
+ ret = twl_i2c_read_u8(TWL4030_MODULE_MAIN_CHARGE,
+ ®val, TWL4030_BCI_BCICTL1);
+ if (ret) {
+ dev_err(&pdev->dev,
+ "unable to read register 0x%X\n", TWL4030_BCI_BCICTL1);
+ }
+ regval |= TWL4030_BCI_MESBAT;
+
+ ret = twl_i2c_write_u8(TWL4030_MODULE_MAIN_CHARGE,
+ regval, TWL4030_BCI_BCICTL1);
+ if (ret) {
+ dev_err(&pdev->dev,
+ "unable to write register 0x%X\n", TWL4030_BCI_BCICTL1);
+ }
+
+ platform_set_drvdata(pdev, madc);
+ mutex_init(&madc->lock);
+
+ ret = request_threaded_irq(platform_get_irq(pdev, 0), NULL,
+ twl4030_madc_irq_handler,
+ IRQF_TRIGGER_RISING, "twl4030_madc", madc);
+ if (ret) {
+ dev_dbg(&pdev->dev, "could not request irq\n");
+ goto err_irq;
+ }
+
+ ret = sysfs_create_group(&pdev->dev.kobj, &twl4030_madc_group);
+ if (ret)
+ goto err_sysfs;
+
+ madc->hwmon_dev = hwmon_device_register(&pdev->dev);
+ if (IS_ERR(madc->hwmon_dev)) {
+ dev_err(&pdev->dev, "hwmon_device_register failed.\n");
+ status = PTR_ERR(madc->hwmon_dev);
+ goto err_reg;
+ }
+
+ the_madc = madc;
+ return 0;
+
+err_reg:
+ hwmon_device_unregister(&pdev->dev);
+err_sysfs:
+ sysfs_remove_group(&pdev->dev.kobj, &twl4030_madc_group);
+err_irq:
+ free_irq(platform_get_irq(pdev, 0), madc);
+ platform_set_drvdata(pdev, NULL);
+ twl4030_madc_set_power(madc, 0);
+ twl4030_madc_set_current_generator(madc, 0, 0);
+err_misc:
+ kfree(madc);
+
+ return ret;
+}
+
+static int __devexit twl4030_madc_remove(struct platform_device *pdev)
+{
+ struct twl4030_madc_data *madc = platform_get_drvdata(pdev);
+
+ hwmon_device_unregister(&pdev->dev);
+ sysfs_remove_group(&pdev->dev.kobj, &twl4030_madc_group);
+ free_irq(platform_get_irq(pdev, 0), madc);
+ platform_set_drvdata(pdev, NULL);
+ twl4030_madc_set_current_generator(madc, 0, 0);
+ twl4030_madc_set_power(madc, 0);
+ kfree(madc);
+
+ return 0;
+}
+
+static struct platform_driver twl4030_madc_driver = {
+ .probe = twl4030_madc_probe,
+ .remove = __exit_p(twl4030_madc_remove),
+ .driver = {
+ .name = "twl4030_madc",
+ .owner = THIS_MODULE,
+ },
+};
+
+static int __init twl4030_madc_init(void)
+{
+ return platform_driver_register(&twl4030_madc_driver);
+}
+
+module_init(twl4030_madc_init);
+
+static void __exit twl4030_madc_exit(void)
+{
+ platform_driver_unregister(&twl4030_madc_driver);
+}
+
+module_exit(twl4030_madc_exit);
+
+MODULE_DESCRIPTION("TWL4030 ADC driver");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("J Keerthy");
diff --git a/include/linux/i2c/twl4030-madc.h b/include/linux/i2c/twl4030-madc.h
new file mode 100644
index 0000000..0e4e227
--- /dev/null
+++ b/include/linux/i2c/twl4030-madc.h
@@ -0,0 +1,118 @@
+/*
+ * 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
+ *
+ */
+
+#ifndef _TWL4030_MADC_H
+#define _TWL4030_MADC_H
+
+struct twl4030_madc_conversion_method {
+ u8 sel;
+ u8 avg;
+ u8 rbase;
+ u8 ctrl;
+};
+
+#define TWL4030_MADC_MAX_CHANNELS 16
+
+struct twl4030_madc_request {
+ u16 channels;
+ u16 do_avg;
+ u16 method;
+ u16 type;
+ bool active;
+ bool result_pending;
+ int rbuf[TWL4030_MADC_MAX_CHANNELS];
+ void (*func_cb)(int len, int channels, int *buf);
+};
+
+enum conversion_methods {
+ TWL4030_MADC_RT,
+ TWL4030_MADC_SW1,
+ TWL4030_MADC_SW2,
+ TWL4030_MADC_NUM_METHODS
+};
+
+enum sample_type {
+ TWL4030_MADC_WAIT,
+ TWL4030_MADC_IRQ_ONESHOT,
+ TWL4030_MADC_IRQ_REARM
+};
+
+#define TWL4030_MADC_CTRL1 0x00
+#define TWL4030_MADC_CTRL2 0x01
+
+#define TWL4030_MADC_RTSELECT_LSB 0x02
+#define TWL4030_MADC_SW1SELECT_LSB 0x06
+#define TWL4030_MADC_SW2SELECT_LSB 0x0A
+
+#define TWL4030_MADC_RTAVERAGE_LSB 0x04
+#define TWL4030_MADC_SW1AVERAGE_LSB 0x08
+#define TWL4030_MADC_SW2AVERAGE_LSB 0x0C
+
+#define TWL4030_MADC_CTRL_SW1 0x12
+#define TWL4030_MADC_CTRL_SW2 0x13
+
+#define TWL4030_MADC_RTCH0_LSB 0x17
+#define TWL4030_MADC_GPCH0_LSB 0x37
+
+#define TWL4030_MADC_MADCON (1 << 0) /* MADC power on */
+#define TWL4030_MADC_BUSY (1 << 0) /* MADC busy */
+#define TWL4030_MADC_EOC_SW (1 << 1) /* MADC conversion completion */
+#define TWL4030_MADC_SW_START (1 << 5) /* MADC SWx start conversion */
+
+#define TWL4030_MADC_ADCIN0 (1 << 0)
+#define TWL4030_MADC_ADCIN1 (1 << 1)
+#define TWL4030_MADC_ADCIN2 (1 << 2)
+#define TWL4030_MADC_ADCIN3 (1 << 3)
+#define TWL4030_MADC_ADCIN4 (1 << 4)
+#define TWL4030_MADC_ADCIN5 (1 << 5)
+#define TWL4030_MADC_ADCIN6 (1 << 6)
+#define TWL4030_MADC_ADCIN7 (1 << 7)
+#define TWL4030_MADC_ADCIN8 (1 << 8)
+#define TWL4030_MADC_ADCIN9 (1 << 9)
+#define TWL4030_MADC_ADCIN10 (1 << 10)
+#define TWL4030_MADC_ADCIN11 (1 << 11)
+#define TWL4030_MADC_ADCIN12 (1 << 12)
+#define TWL4030_MADC_ADCIN13 (1 << 13)
+#define TWL4030_MADC_ADCIN14 (1 << 14)
+#define TWL4030_MADC_ADCIN15 (1 << 15)
+
+/* Fixed channels */
+#define TWL4030_MADC_BTEMP TWL4030_MADC_ADCIN1
+#define TWL4030_MADC_VBUS TWL4030_MADC_ADCIN8
+#define TWL4030_MADC_VBKB TWL4030_MADC_ADCIN9
+#define TWL4030_MADC_ICHG TWL4030_MADC_ADCIN10
+#define TWL4030_MADC_VCHG TWL4030_MADC_ADCIN11
+#define TWL4030_MADC_VBAT TWL4030_MADC_ADCIN12
+
+#define TWL4030_BCI_BCICTL1 0x23
+#define TWL4030_BCI_MESBAT (1 << 1)
+#define TWL4030_BCI_TYPEN (1 << 4)
+#define TWL4030_BCI_ITHEN (1 << 3)
+
+#define TWL4030_MADC_IOC_MAGIC '`'
+#define TWL4030_MADC_IOCX_ADC_RAW_READ _IO(TWL4030_MADC_IOC_MAGIC, 0)
+
+struct twl4030_madc_user_parms {
+ int channel;
+ int average;
+ int status;
+ u16 result;
+};
+
+int twl4030_madc_conversion(struct twl4030_madc_request *conv);
+
+#endif
--
1.7.0.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [lm-sensors] [PATCH V2] hwmon: twl4030: Driver for twl4030 madc
@ 2010-11-09 9:32 ` Keerthy
0 siblings, 0 replies; 10+ messages in thread
From: Keerthy @ 2010-11-09 9:32 UTC (permalink / raw)
To: lm-sensors, guenter.roeck, sameo, khali
Cc: mikko.k.ylinen, balbi, amit.kucheria, linux-omap, balajitk,
j-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-keerthy@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 0000000..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-keerthy@ti.com>
+ *
+ * Based on twl4030-madc.c
+ * Copyright (C) 2008 Nokia Corporation
+ * Mikko Ylinen <mikko.k.ylinen@nokia.com>
+ *
+ * Amit Kucheria <amit.kucheria@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[] = {
+ [TWL4030_MADC_RT] = {
+ .sel = TWL4030_MADC_RTSELECT_LSB,
+ .avg = TWL4030_MADC_RTAVERAGE_LSB,
+ .rbase = TWL4030_MADC_RTCH0_LSB,
+ },
+ [TWL4030_MADC_SW1] = {
+ .sel = TWL4030_MADC_SW1SELECT_LSB,
+ .avg = TWL4030_MADC_SW1AVERAGE_LSB,
+ .rbase = TWL4030_MADC_GPCH0_LSB,
+ .ctrl = TWL4030_MADC_CTRL_SW1,
+ },
+ [TWL4030_MADC_SW2] = {
+ .sel = TWL4030_MADC_SW2SELECT_LSB,
+ .avg = TWL4030_MADC_SW2AVERAGE_LSB,
+ .rbase = TWL4030_MADC_GPCH0_LSB,
+ .ctrl = TWL4030_MADC_CTRL_SW2,
+ },
+};
+
+static int twl4030_madc_read(struct twl4030_madc_data *madc, u8 reg)
+{
+ int ret;
+ u8 val;
+
+ ret = twl_i2c_read_u8(TWL4030_MODULE_MADC, &val, reg);
+ if (ret) {
+ dev_dbg(madc->hwmon_dev, "unable to read register 0x%X\n", reg);
+ return ret;
+ }
+
+ return val;
+}
+
+static void twl4030_madc_write(struct twl4030_madc_data *madc, u8 reg, u8 val)
+{
+ int ret;
+
+ ret = twl_i2c_write_u8(TWL4030_MODULE_MADC, val, reg);
+ if (ret)
+ dev_err(madc->hwmon_dev,
+ "unable to write register 0x%X\n", reg);
+}
+
+static int twl4030_madc_channel_raw_read(struct twl4030_madc_data *madc, u8 reg)
+{
+ u8 msb, lsb;
+
+ /*
+ * For each ADC channel, we have MSB and LSB register pair. MSB address
+ * is always LSB address+1. reg parameter is the addr of LSB register
+ */
+ msb = twl4030_madc_read(madc, reg + 1);
+ lsb = twl4030_madc_read(madc, reg);
+
+ return (int)(((msb << 8) | lsb) >> 6);
+}
+
+static int twl4030_madc_read_channels(struct twl4030_madc_data *madc,
+ u8 reg_base, u16 channels, int *buf)
+{
+ int count = 0;
+ u8 reg, i;
+
+ if (unlikely(!buf))
+ return 0;
+
+ for (i = 0; i < TWL4030_MADC_MAX_CHANNELS; i++) {
+ if (channels & (1 << i)) {
+ reg = reg_base + 2 * i;
+ buf[i] = twl4030_madc_channel_raw_read(madc, reg);
+ count++;
+ }
+ }
+ return count;
+}
+
+static void twl4030_madc_enable_irq(struct twl4030_madc_data *madc, int id)
+{
+ u8 val;
+
+ val = twl4030_madc_read(madc, madc->imr);
+ val &= ~(1 << id);
+ twl4030_madc_write(madc, madc->imr, val);
+}
+
+static void twl4030_madc_disable_irq(struct twl4030_madc_data *madc, int id)
+{
+ u8 val;
+
+ val = twl4030_madc_read(madc, madc->imr);
+ val |= (1 << id);
+ twl4030_madc_write(madc, madc->imr, val);
+}
+
+static irqreturn_t twl4030_madc_irq_handler(int irq, void *_madc)
+{
+ struct twl4030_madc_data *madc = _madc;
+ const struct twl4030_madc_conversion_method *method;
+ u8 isr_val, imr_val;
+ int i, len;
+ struct twl4030_madc_request *r;
+
+ isr_val = twl4030_madc_read(madc, madc->isr);
+ imr_val = twl4030_madc_read(madc, madc->imr);
+
+ isr_val &= ~imr_val;
+
+ for (i = 0; i < TWL4030_MADC_NUM_METHODS; i++) {
+
+ if (!(isr_val & (1 << i)))
+ continue;
+
+ twl4030_madc_disable_irq(madc, i);
+ madc->requests[i].result_pending = 1;
+ }
+ mutex_lock(&madc->lock);
+ for (i = 0; i < TWL4030_MADC_NUM_METHODS; i++) {
+
+ r = &madc->requests[i];
+
+ /* No pending results for this method, move to next one */
+ if (!r->result_pending)
+ continue;
+
+ method = &twl4030_conversion_methods[r->method];
+
+ /* Read results */
+ len = twl4030_madc_read_channels(madc, method->rbase,
+ r->channels, r->rbuf);
+ /* Return results to caller */
+ if (r->func_cb != NULL) {
+ r->func_cb(len, r->channels, r->rbuf);
+ r->func_cb = NULL;
+ }
+
+ /* Free request */
+ r->result_pending = 0;
+ r->active = 0;
+ }
+
+ mutex_unlock(&madc->lock);
+
+ return IRQ_HANDLED;
+}
+
+static int twl4030_madc_set_irq(struct twl4030_madc_data *madc,
+ struct twl4030_madc_request *req)
+{
+ struct twl4030_madc_request *p;
+
+ p = &madc->requests[req->method];
+
+ memcpy(p, req, sizeof *req);
+
+ twl4030_madc_enable_irq(madc, req->method);
+
+ return 0;
+}
+
+static inline void twl4030_madc_start_conversion(struct twl4030_madc_data *madc,
+ int conv_method)
+{
+ const struct twl4030_madc_conversion_method *method;
+
+ method = &twl4030_conversion_methods[conv_method];
+
+ switch (conv_method) {
+ case TWL4030_MADC_SW1:
+ case TWL4030_MADC_SW2:
+ twl4030_madc_write(madc, method->ctrl, TWL4030_MADC_SW_START);
+ break;
+ case TWL4030_MADC_RT:
+ default:
+ break;
+ }
+}
+
+static int twl4030_madc_wait_conversion_ready(struct twl4030_madc_data *madc,
+ unsigned int timeout_ms,
+ u8 status_reg)
+{
+ unsigned long timeout;
+
+ timeout = jiffies + msecs_to_jiffies(timeout_ms);
+ do {
+ u8 reg;
+
+ reg = twl4030_madc_read(madc, status_reg);
+ if (!(reg & TWL4030_MADC_BUSY) && (reg & TWL4030_MADC_EOC_SW))
+ return 0;
+ } while (!time_after(jiffies, timeout));
+
+ return -EAGAIN;
+}
+
+int twl4030_madc_conversion(struct twl4030_madc_request *req)
+{
+ const struct twl4030_madc_conversion_method *method;
+ u8 ch_msb, ch_lsb;
+ int ret;
+
+ if (unlikely(!req))
+ return -EINVAL;
+
+ mutex_lock(&the_madc->lock);
+
+ if (req->method < TWL4030_MADC_RT || req->method > TWL4030_MADC_SW2) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ /* Do we have a conversion request ongoing */
+ if (the_madc->requests[req->method].active) {
+ ret = -EBUSY;
+ goto out;
+ }
+
+ ch_msb = (req->channels >> 8) & 0xff;
+ ch_lsb = req->channels & 0xff;
+
+ method = &twl4030_conversion_methods[req->method];
+
+ /* Select channels to be converted */
+ twl4030_madc_write(the_madc, method->sel + 1, ch_msb);
+ twl4030_madc_write(the_madc, method->sel, ch_lsb);
+
+ /* Select averaging for all channels if do_avg is set */
+ if (req->do_avg) {
+ twl4030_madc_write(the_madc, method->avg + 1, ch_msb);
+ twl4030_madc_write(the_madc, method->avg, ch_lsb);
+ }
+
+ if (req->type = TWL4030_MADC_IRQ_ONESHOT && req->func_cb != NULL) {
+ twl4030_madc_set_irq(the_madc, req);
+ twl4030_madc_start_conversion(the_madc, req->method);
+ the_madc->requests[req->method].active = 1;
+ ret = 0;
+ goto out;
+ }
+
+ /* With RT method we should not be here anymore */
+ if (req->method = TWL4030_MADC_RT) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ twl4030_madc_start_conversion(the_madc, req->method);
+ the_madc->requests[req->method].active = 1;
+
+ /* Wait until conversion is ready (ctrl register returns EOC) */
+ ret = twl4030_madc_wait_conversion_ready(the_madc, 5, method->ctrl);
+ if (ret) {
+ dev_dbg(the_madc->hwmon_dev, "conversion timeout!\n");
+ the_madc->requests[req->method].active = 0;
+ goto out;
+ }
+
+ ret = twl4030_madc_read_channels(the_madc, method->rbase, req->channels,
+ req->rbuf);
+
+ the_madc->requests[req->method].active = 0;
+
+out:
+ mutex_unlock(&the_madc->lock);
+
+ return ret;
+}
+EXPORT_SYMBOL(twl4030_madc_conversion);
+
+/*
+ * Return channel value
+ * Or < 0 on failure.
+ */
+static int twl4030_get_madc_conversion(int channel_no)
+{
+ struct twl4030_madc_request req;
+ int temp = 0;
+ int ret;
+
+ req.channels = (1 << channel_no);
+ req.method = TWL4030_MADC_SW2;
+ req.active = 0;
+ req.func_cb = NULL;
+ ret = twl4030_madc_conversion(&req);
+ if (ret < 0)
+ return ret;
+
+ if (req.rbuf[channel_no] > 0)
+ temp = req.rbuf[channel_no];
+
+ return temp;
+}
+EXPORT_SYMBOL(twl4030_get_madc_conversion);
+
+static void twl4030_madc_set_current_generator(struct twl4030_madc_data *madc,
+ int chan, int on)
+{
+ int ret;
+ u8 regval;
+
+ ret = twl_i2c_read_u8(TWL4030_MODULE_MAIN_CHARGE,
+ ®val, TWL4030_BCI_BCICTL1);
+ if (ret) {
+ dev_dbg(madc->hwmon_dev,
+ "unable to read register 0x%X\n", TWL4030_BCI_BCICTL1);
+ }
+ if (on)
+ regval |= chan ? TWL4030_BCI_ITHEN : TWL4030_BCI_TYPEN;
+ else
+ regval &= chan ? ~TWL4030_BCI_ITHEN : ~TWL4030_BCI_TYPEN;
+ ret = twl_i2c_write_u8(TWL4030_MODULE_MAIN_CHARGE,
+ regval, TWL4030_BCI_BCICTL1);
+ if (ret)
+ dev_err(madc->hwmon_dev,
+ "unable to write register 0x%X\n", TWL4030_BCI_BCICTL1);
+
+}
+
+static void twl4030_madc_set_power(struct twl4030_madc_data *madc, int on)
+{
+ u8 regval;
+
+ regval = twl4030_madc_read(madc, TWL4030_MADC_CTRL1);
+ if (on)
+ regval |= TWL4030_MADC_MADCON;
+ else
+ regval &= ~TWL4030_MADC_MADCON;
+ twl4030_madc_write(madc, TWL4030_MADC_CTRL1, regval);
+}
+
+static SENSOR_DEVICE_ATTR(in0_input, S_IRUGO, madc_read, NULL, 0);
+static SENSOR_DEVICE_ATTR(in1_input, S_IRUGO, madc_read, NULL, 1);
+static SENSOR_DEVICE_ATTR(in2_input, S_IRUGO, madc_read, NULL, 2);
+static SENSOR_DEVICE_ATTR(in3_input, S_IRUGO, madc_read, NULL, 3);
+static SENSOR_DEVICE_ATTR(in4_input, S_IRUGO, madc_read, NULL, 4);
+static SENSOR_DEVICE_ATTR(in5_input, S_IRUGO, madc_read, NULL, 5);
+static SENSOR_DEVICE_ATTR(in6_input, S_IRUGO, madc_read, NULL, 6);
+static SENSOR_DEVICE_ATTR(in7_input, S_IRUGO, madc_read, NULL, 7);
+static SENSOR_DEVICE_ATTR(in8_input, S_IRUGO, madc_read, NULL, 8);
+static SENSOR_DEVICE_ATTR(in9_input, S_IRUGO, madc_read, NULL, 9);
+static SENSOR_DEVICE_ATTR(in10_input, S_IRUGO, madc_read, NULL, 10);
+static SENSOR_DEVICE_ATTR(in11_input, S_IRUGO, madc_read, NULL, 11);
+static SENSOR_DEVICE_ATTR(in12_input, S_IRUGO, madc_read, NULL, 12);
+static SENSOR_DEVICE_ATTR(in13_input, S_IRUGO, madc_read, NULL, 13);
+static SENSOR_DEVICE_ATTR(in14_input, S_IRUGO, madc_read, NULL, 14);
+static SENSOR_DEVICE_ATTR(in15_input, S_IRUGO, madc_read, NULL, 15);
+
+static struct attribute *twl4030_madc_attributes[] = {
+ &sensor_dev_attr_in0_input.dev_attr.attr,
+ &sensor_dev_attr_in1_input.dev_attr.attr,
+ &sensor_dev_attr_in2_input.dev_attr.attr,
+ &sensor_dev_attr_in3_input.dev_attr.attr,
+ &sensor_dev_attr_in4_input.dev_attr.attr,
+ &sensor_dev_attr_in5_input.dev_attr.attr,
+ &sensor_dev_attr_in6_input.dev_attr.attr,
+ &sensor_dev_attr_in7_input.dev_attr.attr,
+ &sensor_dev_attr_in8_input.dev_attr.attr,
+ &sensor_dev_attr_in9_input.dev_attr.attr,
+ &sensor_dev_attr_in10_input.dev_attr.attr,
+ &sensor_dev_attr_in11_input.dev_attr.attr,
+ &sensor_dev_attr_in12_input.dev_attr.attr,
+ &sensor_dev_attr_in13_input.dev_attr.attr,
+ &sensor_dev_attr_in14_input.dev_attr.attr,
+ &sensor_dev_attr_in15_input.dev_attr.attr,
+ NULL
+};
+
+static const struct attribute_group twl4030_madc_group = {
+ .attrs = twl4030_madc_attributes,
+};
+
+static int __devinit twl4030_madc_probe(struct platform_device *pdev)
+{
+ struct twl4030_madc_data *madc;
+ struct twl4030_madc_platform_data *pdata = pdev->dev.platform_data;
+ int ret;
+ int status;
+ u8 regval;
+
+ madc = kzalloc(sizeof *madc, GFP_KERNEL);
+ if (!madc)
+ return -ENOMEM;
+
+ if (!pdata) {
+ dev_dbg(&pdev->dev, "platform_data not available\n");
+ ret = -EINVAL;
+ goto err_misc;
+ }
+ madc->imr = (pdata->irq_line = 1) ?
+ TWL4030_MADC_IMR1 : TWL4030_MADC_IMR2;
+ madc->isr = (pdata->irq_line = 1) ?
+ TWL4030_MADC_ISR1 : TWL4030_MADC_ISR2;
+ twl4030_madc_set_power(madc, 1);
+
+ twl4030_madc_set_current_generator(madc, 0, 1);
+
+ ret = twl_i2c_read_u8(TWL4030_MODULE_MAIN_CHARGE,
+ ®val, TWL4030_BCI_BCICTL1);
+ if (ret) {
+ dev_err(&pdev->dev,
+ "unable to read register 0x%X\n", TWL4030_BCI_BCICTL1);
+ }
+ regval |= TWL4030_BCI_MESBAT;
+
+ ret = twl_i2c_write_u8(TWL4030_MODULE_MAIN_CHARGE,
+ regval, TWL4030_BCI_BCICTL1);
+ if (ret) {
+ dev_err(&pdev->dev,
+ "unable to write register 0x%X\n", TWL4030_BCI_BCICTL1);
+ }
+
+ platform_set_drvdata(pdev, madc);
+ mutex_init(&madc->lock);
+
+ ret = request_threaded_irq(platform_get_irq(pdev, 0), NULL,
+ twl4030_madc_irq_handler,
+ IRQF_TRIGGER_RISING, "twl4030_madc", madc);
+ if (ret) {
+ dev_dbg(&pdev->dev, "could not request irq\n");
+ goto err_irq;
+ }
+
+ ret = sysfs_create_group(&pdev->dev.kobj, &twl4030_madc_group);
+ if (ret)
+ goto err_sysfs;
+
+ madc->hwmon_dev = hwmon_device_register(&pdev->dev);
+ if (IS_ERR(madc->hwmon_dev)) {
+ dev_err(&pdev->dev, "hwmon_device_register failed.\n");
+ status = PTR_ERR(madc->hwmon_dev);
+ goto err_reg;
+ }
+
+ the_madc = madc;
+ return 0;
+
+err_reg:
+ hwmon_device_unregister(&pdev->dev);
+err_sysfs:
+ sysfs_remove_group(&pdev->dev.kobj, &twl4030_madc_group);
+err_irq:
+ free_irq(platform_get_irq(pdev, 0), madc);
+ platform_set_drvdata(pdev, NULL);
+ twl4030_madc_set_power(madc, 0);
+ twl4030_madc_set_current_generator(madc, 0, 0);
+err_misc:
+ kfree(madc);
+
+ return ret;
+}
+
+static int __devexit twl4030_madc_remove(struct platform_device *pdev)
+{
+ struct twl4030_madc_data *madc = platform_get_drvdata(pdev);
+
+ hwmon_device_unregister(&pdev->dev);
+ sysfs_remove_group(&pdev->dev.kobj, &twl4030_madc_group);
+ free_irq(platform_get_irq(pdev, 0), madc);
+ platform_set_drvdata(pdev, NULL);
+ twl4030_madc_set_current_generator(madc, 0, 0);
+ twl4030_madc_set_power(madc, 0);
+ kfree(madc);
+
+ return 0;
+}
+
+static struct platform_driver twl4030_madc_driver = {
+ .probe = twl4030_madc_probe,
+ .remove = __exit_p(twl4030_madc_remove),
+ .driver = {
+ .name = "twl4030_madc",
+ .owner = THIS_MODULE,
+ },
+};
+
+static int __init twl4030_madc_init(void)
+{
+ return platform_driver_register(&twl4030_madc_driver);
+}
+
+module_init(twl4030_madc_init);
+
+static void __exit twl4030_madc_exit(void)
+{
+ platform_driver_unregister(&twl4030_madc_driver);
+}
+
+module_exit(twl4030_madc_exit);
+
+MODULE_DESCRIPTION("TWL4030 ADC driver");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("J Keerthy");
diff --git a/include/linux/i2c/twl4030-madc.h b/include/linux/i2c/twl4030-madc.h
new file mode 100644
index 0000000..0e4e227
--- /dev/null
+++ b/include/linux/i2c/twl4030-madc.h
@@ -0,0 +1,118 @@
+/*
+ * 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
+ *
+ */
+
+#ifndef _TWL4030_MADC_H
+#define _TWL4030_MADC_H
+
+struct twl4030_madc_conversion_method {
+ u8 sel;
+ u8 avg;
+ u8 rbase;
+ u8 ctrl;
+};
+
+#define TWL4030_MADC_MAX_CHANNELS 16
+
+struct twl4030_madc_request {
+ u16 channels;
+ u16 do_avg;
+ u16 method;
+ u16 type;
+ bool active;
+ bool result_pending;
+ int rbuf[TWL4030_MADC_MAX_CHANNELS];
+ void (*func_cb)(int len, int channels, int *buf);
+};
+
+enum conversion_methods {
+ TWL4030_MADC_RT,
+ TWL4030_MADC_SW1,
+ TWL4030_MADC_SW2,
+ TWL4030_MADC_NUM_METHODS
+};
+
+enum sample_type {
+ TWL4030_MADC_WAIT,
+ TWL4030_MADC_IRQ_ONESHOT,
+ TWL4030_MADC_IRQ_REARM
+};
+
+#define TWL4030_MADC_CTRL1 0x00
+#define TWL4030_MADC_CTRL2 0x01
+
+#define TWL4030_MADC_RTSELECT_LSB 0x02
+#define TWL4030_MADC_SW1SELECT_LSB 0x06
+#define TWL4030_MADC_SW2SELECT_LSB 0x0A
+
+#define TWL4030_MADC_RTAVERAGE_LSB 0x04
+#define TWL4030_MADC_SW1AVERAGE_LSB 0x08
+#define TWL4030_MADC_SW2AVERAGE_LSB 0x0C
+
+#define TWL4030_MADC_CTRL_SW1 0x12
+#define TWL4030_MADC_CTRL_SW2 0x13
+
+#define TWL4030_MADC_RTCH0_LSB 0x17
+#define TWL4030_MADC_GPCH0_LSB 0x37
+
+#define TWL4030_MADC_MADCON (1 << 0) /* MADC power on */
+#define TWL4030_MADC_BUSY (1 << 0) /* MADC busy */
+#define TWL4030_MADC_EOC_SW (1 << 1) /* MADC conversion completion */
+#define TWL4030_MADC_SW_START (1 << 5) /* MADC SWx start conversion */
+
+#define TWL4030_MADC_ADCIN0 (1 << 0)
+#define TWL4030_MADC_ADCIN1 (1 << 1)
+#define TWL4030_MADC_ADCIN2 (1 << 2)
+#define TWL4030_MADC_ADCIN3 (1 << 3)
+#define TWL4030_MADC_ADCIN4 (1 << 4)
+#define TWL4030_MADC_ADCIN5 (1 << 5)
+#define TWL4030_MADC_ADCIN6 (1 << 6)
+#define TWL4030_MADC_ADCIN7 (1 << 7)
+#define TWL4030_MADC_ADCIN8 (1 << 8)
+#define TWL4030_MADC_ADCIN9 (1 << 9)
+#define TWL4030_MADC_ADCIN10 (1 << 10)
+#define TWL4030_MADC_ADCIN11 (1 << 11)
+#define TWL4030_MADC_ADCIN12 (1 << 12)
+#define TWL4030_MADC_ADCIN13 (1 << 13)
+#define TWL4030_MADC_ADCIN14 (1 << 14)
+#define TWL4030_MADC_ADCIN15 (1 << 15)
+
+/* Fixed channels */
+#define TWL4030_MADC_BTEMP TWL4030_MADC_ADCIN1
+#define TWL4030_MADC_VBUS TWL4030_MADC_ADCIN8
+#define TWL4030_MADC_VBKB TWL4030_MADC_ADCIN9
+#define TWL4030_MADC_ICHG TWL4030_MADC_ADCIN10
+#define TWL4030_MADC_VCHG TWL4030_MADC_ADCIN11
+#define TWL4030_MADC_VBAT TWL4030_MADC_ADCIN12
+
+#define TWL4030_BCI_BCICTL1 0x23
+#define TWL4030_BCI_MESBAT (1 << 1)
+#define TWL4030_BCI_TYPEN (1 << 4)
+#define TWL4030_BCI_ITHEN (1 << 3)
+
+#define TWL4030_MADC_IOC_MAGIC '`'
+#define TWL4030_MADC_IOCX_ADC_RAW_READ _IO(TWL4030_MADC_IOC_MAGIC, 0)
+
+struct twl4030_madc_user_parms {
+ int channel;
+ int average;
+ int status;
+ u16 result;
+};
+
+int twl4030_madc_conversion(struct twl4030_madc_request *conv);
+
+#endif
--
1.7.0.4
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply related [flat|nested] 10+ messages in thread
* RE: [PATCH V2] hwmon: twl4030: Driver for twl4030 madc module
2010-11-09 9:32 ` [lm-sensors] [PATCH V2] hwmon: twl4030: Driver for twl4030 madc Keerthy
@ 2010-11-09 13:52 ` Anand Gadiyar
-1 siblings, 0 replies; 10+ messages in thread
From: Anand Gadiyar @ 2010-11-09 13:40 UTC (permalink / raw)
To: J KEERTHY, lm-sensors, guenter.roeck, sameo, khali
Cc: mikko.k.ylinen, Felipe Balbi, amit.kucheria, linux-omap,
Balaji T Krishnamoorthy
> 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-keerthy@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
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH V2] hwmon: twl4030: Driver for twl4030 madc module
2010-11-09 13:52 ` [lm-sensors] [PATCH V2] hwmon: twl4030: Driver for twl4030 madc Anand Gadiyar
@ 2010-11-09 13:58 ` J, KEERTHY
-1 siblings, 0 replies; 10+ messages in thread
From: J, KEERTHY @ 2010-11-09 13:46 UTC (permalink / raw)
To: Gadiyar, Anand, lm-sensors@lm-sensors.org,
guenter.roeck@ericsson.com, sameo@linux.intel.com
Cc: mikko.k.ylinen@nokia.com, Balbi, Felipe,
amit.kucheria@canonical.com, linux-omap@vger.kernel.org,
Krishnamoorthy, Balaji T
Hello Anand,
-----Original Message-----
From: Gadiyar, Anand
Sent: Tuesday, November 09, 2010 7:10 PM
To: J, KEERTHY; lm-sensors@lm-sensors.org; guenter.roeck@ericsson.com; sameo@linux.intel.com; khali@linux-fr.org
Cc: mikko.k.ylinen@nokia.com; Balbi, Felipe; amit.kucheria@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-keerthy@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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [lm-sensors] [PATCH V2] hwmon: twl4030: Driver for twl4030 madc
@ 2010-11-09 13:52 ` Anand Gadiyar
0 siblings, 0 replies; 10+ messages in thread
From: Anand Gadiyar @ 2010-11-09 13:52 UTC (permalink / raw)
To: J KEERTHY, lm-sensors, guenter.roeck, sameo, khali
Cc: mikko.k.ylinen, Felipe Balbi, amit.kucheria, linux-omap,
Balaji T Krishnamoorthy
> 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-keerthy@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
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [lm-sensors] [PATCH V2] hwmon: twl4030: Driver for twl4030 madc
@ 2010-11-09 13:58 ` J, KEERTHY
0 siblings, 0 replies; 10+ messages in thread
From: J, KEERTHY @ 2010-11-09 13:58 UTC (permalink / raw)
To: Gadiyar, Anand, lm-sensors@lm-sensors.org,
guenter.roeck@ericsson.com, sameo@linux.intel.com
Cc: mikko.k.ylinen@nokia.com, Balbi, Felipe,
amit.kucheria@canonical.com, linux-omap@vger.kernel.org,
Krishnamoorthy, Balaji T
Hello Anand,
-----Original Message-----
From: Gadiyar, Anand
Sent: Tuesday, November 09, 2010 7:10 PM
To: J, KEERTHY; lm-sensors@lm-sensors.org; guenter.roeck@ericsson.com; sameo@linux.intel.com; khali@linux-fr.org
Cc: mikko.k.ylinen@nokia.com; Balbi, Felipe; amit.kucheria@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-keerthy@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
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V2] hwmon: twl4030: Driver for twl4030 madc module
2010-11-09 9:32 ` [lm-sensors] [PATCH V2] hwmon: twl4030: Driver for twl4030 madc Keerthy
@ 2010-11-11 0:01 ` Guenter Roeck
-1 siblings, 0 replies; 10+ messages in thread
From: Guenter Roeck @ 2010-11-11 0:01 UTC (permalink / raw)
To: Keerthy
Cc: lm-sensors@lm-sensors.org, sameo@linux.intel.com,
khali@linux-fr.org, mikko.k.ylinen@nokia.com, balbi@ti.com,
amit.kucheria@canonical.com, linux-omap@vger.kernel.org,
balajitk@ti.com
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-keerthy@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 0000000..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-keerthy@ti.com>
> + *
> + * Based on twl4030-madc.c
> + * Copyright (C) 2008 Nokia Corporation
> + * Mikko Ylinen <mikko.k.ylinen@nokia.com>
> + *
> + * Amit Kucheria <amit.kucheria@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;
if (unlikely(val < 0))
return val;
return sprintf(buf, "%ld\n", req.rbuf[attr->index]);
should do and would be simpler. status is not needed at all.
> +}
> +
> +static
> +const struct twl4030_madc_conversion_method twl4030_conversion_methods[] = {
> + [TWL4030_MADC_RT] = {
> + .sel = TWL4030_MADC_RTSELECT_LSB,
> + .avg = TWL4030_MADC_RTAVERAGE_LSB,
> + .rbase = TWL4030_MADC_RTCH0_LSB,
> + },
> + [TWL4030_MADC_SW1] = {
> + .sel = TWL4030_MADC_SW1SELECT_LSB,
> + .avg = TWL4030_MADC_SW1AVERAGE_LSB,
> + .rbase = TWL4030_MADC_GPCH0_LSB,
> + .ctrl = TWL4030_MADC_CTRL_SW1,
> + },
> + [TWL4030_MADC_SW2] = {
> + .sel = TWL4030_MADC_SW2SELECT_LSB,
> + .avg = TWL4030_MADC_SW2AVERAGE_LSB,
> + .rbase = TWL4030_MADC_GPCH0_LSB,
> + .ctrl = TWL4030_MADC_CTRL_SW2,
> + },
> +};
> +
> +static int twl4030_madc_read(struct twl4030_madc_data *madc, u8 reg)
> +{
> + int ret;
> + u8 val;
> +
> + ret = twl_i2c_read_u8(TWL4030_MODULE_MADC, &val, reg);
> + if (ret) {
> + dev_dbg(madc->hwmon_dev, "unable to read register 0x%X\n", reg);
> + return ret;
> + }
> +
> + return val;
> +}
> +
> +static void twl4030_madc_write(struct twl4030_madc_data *madc, u8 reg, u8 val)
> +{
> + int ret;
> +
> + ret = twl_i2c_write_u8(TWL4030_MODULE_MADC, val, reg);
> + if (ret)
> + dev_err(madc->hwmon_dev,
> + "unable to write register 0x%X\n", reg);
> +}
Looking into other uses of twl_i2c_write_u8(), most check and return errors.
Not sure if it is a good idea to ignore all errors as it is done here. There
should at least some comment explaining that and why it is done.
> +
> +static int twl4030_madc_channel_raw_read(struct twl4030_madc_data *madc, u8 reg)
> +{
> + u8 msb, lsb;
> +
> + /*
> + * For each ADC channel, we have MSB and LSB register pair. MSB address
> + * is always LSB address+1. reg parameter is the addr of LSB register
> + */
> + msb = twl4030_madc_read(madc, reg + 1);
> + lsb = twl4030_madc_read(madc, reg);
> +
Ignoring returned errors and combining them into a random value doesn't make much sense.
> + return (int)(((msb << 8) | lsb) >> 6);
> +}
> +
> +static int twl4030_madc_read_channels(struct twl4030_madc_data *madc,
> + u8 reg_base, u16 channels, int *buf)
> +{
> + int count = 0;
> + u8 reg, i;
> +
> + if (unlikely(!buf))
> + return 0;
> +
I don't see how this can ever be NULL.
> + for (i = 0; i < TWL4030_MADC_MAX_CHANNELS; i++) {
> + if (channels & (1 << i)) {
> + reg = reg_base + 2 * i;
> + buf[i] = twl4030_madc_channel_raw_read(madc, reg);
Content of buf will be more or less random after errors.
> + count++;
> + }
> + }
> + return count;
> +}
> +
> +static void twl4030_madc_enable_irq(struct twl4030_madc_data *madc, int id)
> +{
> + u8 val;
> +
> + val = twl4030_madc_read(madc, madc->imr);
What if twl4030_madc_read() returned an error ? Try to write a random value ?
> + val &= ~(1 << id);
> + twl4030_madc_write(madc, madc->imr, val);
> +}
> +
> +static void twl4030_madc_disable_irq(struct twl4030_madc_data *madc, int id)
> +{
> + u8 val;
> +
> + val = twl4030_madc_read(madc, madc->imr);
Returned error ignored.
> + val |= (1 << id);
> + twl4030_madc_write(madc, madc->imr, val);
> +}
> +
> +static irqreturn_t twl4030_madc_irq_handler(int irq, void *_madc)
> +{
> + struct twl4030_madc_data *madc = _madc;
> + const struct twl4030_madc_conversion_method *method;
> + u8 isr_val, imr_val;
> + int i, len;
> + struct twl4030_madc_request *r;
> +
> + isr_val = twl4030_madc_read(madc, madc->isr);
> + imr_val = twl4030_madc_read(madc, madc->imr);
> +
Returned errors ignored.
> + isr_val &= ~imr_val;
> +
> + for (i = 0; i < TWL4030_MADC_NUM_METHODS; i++) {
> +
> + if (!(isr_val & (1 << i)))
> + continue;
> +
> + twl4030_madc_disable_irq(madc, i);
> + madc->requests[i].result_pending = 1;
> + }
> + mutex_lock(&madc->lock);
> + for (i = 0; i < TWL4030_MADC_NUM_METHODS; i++) {
> +
> + r = &madc->requests[i];
> +
> + /* No pending results for this method, move to next one */
> + if (!r->result_pending)
> + continue;
> +
> + method = &twl4030_conversion_methods[r->method];
> +
> + /* Read results */
> + len = twl4030_madc_read_channels(madc, method->rbase,
> + r->channels, r->rbuf);
> + /* Return results to caller */
> + if (r->func_cb != NULL) {
> + r->func_cb(len, r->channels, r->rbuf);
> + r->func_cb = NULL;
> + }
> +
> + /* Free request */
> + r->result_pending = 0;
> + r->active = 0;
> + }
> +
> + mutex_unlock(&madc->lock);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int twl4030_madc_set_irq(struct twl4030_madc_data *madc,
> + struct twl4030_madc_request *req)
> +{
> + struct twl4030_madc_request *p;
> +
> + p = &madc->requests[req->method];
> +
> + memcpy(p, req, sizeof *req);
> +
> + twl4030_madc_enable_irq(madc, req->method);
> +
> + return 0;
> +}
> +
> +static inline void twl4030_madc_start_conversion(struct twl4030_madc_data *madc,
> + int conv_method)
> +{
> + const struct twl4030_madc_conversion_method *method;
> +
> + method = &twl4030_conversion_methods[conv_method];
> +
> + switch (conv_method) {
> + case TWL4030_MADC_SW1:
> + case TWL4030_MADC_SW2:
> + twl4030_madc_write(madc, method->ctrl, TWL4030_MADC_SW_START);
> + break;
> + case TWL4030_MADC_RT:
> + default:
> + break;
> + }
> +}
> +
> +static int twl4030_madc_wait_conversion_ready(struct twl4030_madc_data *madc,
> + unsigned int timeout_ms,
> + u8 status_reg)
> +{
> + unsigned long timeout;
> +
> + timeout = jiffies + msecs_to_jiffies(timeout_ms);
> + do {
> + u8 reg;
> +
> + reg = twl4030_madc_read(madc, status_reg);
Returned error ignored.
> + if (!(reg & TWL4030_MADC_BUSY) && (reg & TWL4030_MADC_EOC_SW))
> + return 0;
> + } while (!time_after(jiffies, timeout));
> +
> + return -EAGAIN;
> +}
> +
> +int twl4030_madc_conversion(struct twl4030_madc_request *req)
> +{
> + const struct twl4030_madc_conversion_method *method;
> + u8 ch_msb, ch_lsb;
> + int ret;
> +
> + if (unlikely(!req))
> + return -EINVAL;
> +
> + mutex_lock(&the_madc->lock);
> +
> + if (req->method < TWL4030_MADC_RT || req->method > TWL4030_MADC_SW2) {
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + /* Do we have a conversion request ongoing */
> + if (the_madc->requests[req->method].active) {
> + ret = -EBUSY;
> + goto out;
> + }
> +
> + ch_msb = (req->channels >> 8) & 0xff;
> + ch_lsb = req->channels & 0xff;
> +
> + method = &twl4030_conversion_methods[req->method];
> +
> + /* Select channels to be converted */
> + twl4030_madc_write(the_madc, method->sel + 1, ch_msb);
> + twl4030_madc_write(the_madc, method->sel, ch_lsb);
> +
> + /* Select averaging for all channels if do_avg is set */
> + if (req->do_avg) {
> + twl4030_madc_write(the_madc, method->avg + 1, ch_msb);
> + twl4030_madc_write(the_madc, method->avg, ch_lsb);
> + }
> +
> + if (req->type == TWL4030_MADC_IRQ_ONESHOT && req->func_cb != NULL) {
> + twl4030_madc_set_irq(the_madc, req);
> + twl4030_madc_start_conversion(the_madc, req->method);
> + the_madc->requests[req->method].active = 1;
> + ret = 0;
> + goto out;
> + }
> +
> + /* With RT method we should not be here anymore */
> + if (req->method == TWL4030_MADC_RT) {
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + twl4030_madc_start_conversion(the_madc, req->method);
> + the_madc->requests[req->method].active = 1;
> +
> + /* Wait until conversion is ready (ctrl register returns EOC) */
> + ret = twl4030_madc_wait_conversion_ready(the_madc, 5, method->ctrl);
> + if (ret) {
> + dev_dbg(the_madc->hwmon_dev, "conversion timeout!\n");
> + the_madc->requests[req->method].active = 0;
> + goto out;
> + }
> +
> + ret = twl4030_madc_read_channels(the_madc, method->rbase, req->channels,
> + req->rbuf);
> +
> + the_madc->requests[req->method].active = 0;
> +
> +out:
> + mutex_unlock(&the_madc->lock);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(twl4030_madc_conversion);
> +
> +/*
> + * Return channel value
> + * Or < 0 on failure.
> + */
> +static int twl4030_get_madc_conversion(int channel_no)
> +{
> + struct twl4030_madc_request req;
> + int temp = 0;
> + int ret;
> +
> + req.channels = (1 << channel_no);
> + req.method = TWL4030_MADC_SW2;
> + req.active = 0;
> + req.func_cb = NULL;
> + ret = twl4030_madc_conversion(&req);
> + if (ret < 0)
> + return ret;
> +
> + if (req.rbuf[channel_no] > 0)
> + temp = req.rbuf[channel_no];
> +
> + return temp;
> +}
> +EXPORT_SYMBOL(twl4030_get_madc_conversion);
> +
EXPORT_SYMBOL and static declaration is a contradiction.
Actually, this function is not used anywhere. Does this compile without warning ?
> +static void twl4030_madc_set_current_generator(struct twl4030_madc_data *madc,
> + int chan, int on)
> +{
> + int ret;
> + u8 regval;
> +
> + ret = twl_i2c_read_u8(TWL4030_MODULE_MAIN_CHARGE,
> + ®val, TWL4030_BCI_BCICTL1);
> + if (ret) {
> + dev_dbg(madc->hwmon_dev,
> + "unable to read register 0x%X\n", TWL4030_BCI_BCICTL1);
> + }
You detect the error ... then go ahead and use the random regval anyway.
> + if (on)
> + regval |= chan ? TWL4030_BCI_ITHEN : TWL4030_BCI_TYPEN;
> + else
> + regval &= chan ? ~TWL4030_BCI_ITHEN : ~TWL4030_BCI_TYPEN;
> + ret = twl_i2c_write_u8(TWL4030_MODULE_MAIN_CHARGE,
> + regval, TWL4030_BCI_BCICTL1);
> + if (ret)
> + dev_err(madc->hwmon_dev,
> + "unable to write register 0x%X\n", TWL4030_BCI_BCICTL1);
> +
Unnecessary blank line.
> +}
> +
> +static void twl4030_madc_set_power(struct twl4030_madc_data *madc, int on)
> +{
> + u8 regval;
> +
> + regval = twl4030_madc_read(madc, TWL4030_MADC_CTRL1);
Returned error ignored.
> + if (on)
> + regval |= TWL4030_MADC_MADCON;
> + else
> + regval &= ~TWL4030_MADC_MADCON;
> + twl4030_madc_write(madc, TWL4030_MADC_CTRL1, regval);
> +}
> +
> +static SENSOR_DEVICE_ATTR(in0_input, S_IRUGO, madc_read, NULL, 0);
> +static SENSOR_DEVICE_ATTR(in1_input, S_IRUGO, madc_read, NULL, 1);
> +static SENSOR_DEVICE_ATTR(in2_input, S_IRUGO, madc_read, NULL, 2);
> +static SENSOR_DEVICE_ATTR(in3_input, S_IRUGO, madc_read, NULL, 3);
> +static SENSOR_DEVICE_ATTR(in4_input, S_IRUGO, madc_read, NULL, 4);
> +static SENSOR_DEVICE_ATTR(in5_input, S_IRUGO, madc_read, NULL, 5);
> +static SENSOR_DEVICE_ATTR(in6_input, S_IRUGO, madc_read, NULL, 6);
> +static SENSOR_DEVICE_ATTR(in7_input, S_IRUGO, madc_read, NULL, 7);
> +static SENSOR_DEVICE_ATTR(in8_input, S_IRUGO, madc_read, NULL, 8);
> +static SENSOR_DEVICE_ATTR(in9_input, S_IRUGO, madc_read, NULL, 9);
> +static SENSOR_DEVICE_ATTR(in10_input, S_IRUGO, madc_read, NULL, 10);
> +static SENSOR_DEVICE_ATTR(in11_input, S_IRUGO, madc_read, NULL, 11);
> +static SENSOR_DEVICE_ATTR(in12_input, S_IRUGO, madc_read, NULL, 12);
> +static SENSOR_DEVICE_ATTR(in13_input, S_IRUGO, madc_read, NULL, 13);
> +static SENSOR_DEVICE_ATTR(in14_input, S_IRUGO, madc_read, NULL, 14);
> +static SENSOR_DEVICE_ATTR(in15_input, S_IRUGO, madc_read, NULL, 15);
> +
> +static struct attribute *twl4030_madc_attributes[] = {
> + &sensor_dev_attr_in0_input.dev_attr.attr,
> + &sensor_dev_attr_in1_input.dev_attr.attr,
> + &sensor_dev_attr_in2_input.dev_attr.attr,
> + &sensor_dev_attr_in3_input.dev_attr.attr,
> + &sensor_dev_attr_in4_input.dev_attr.attr,
> + &sensor_dev_attr_in5_input.dev_attr.attr,
> + &sensor_dev_attr_in6_input.dev_attr.attr,
> + &sensor_dev_attr_in7_input.dev_attr.attr,
> + &sensor_dev_attr_in8_input.dev_attr.attr,
> + &sensor_dev_attr_in9_input.dev_attr.attr,
> + &sensor_dev_attr_in10_input.dev_attr.attr,
> + &sensor_dev_attr_in11_input.dev_attr.attr,
> + &sensor_dev_attr_in12_input.dev_attr.attr,
> + &sensor_dev_attr_in13_input.dev_attr.attr,
> + &sensor_dev_attr_in14_input.dev_attr.attr,
> + &sensor_dev_attr_in15_input.dev_attr.attr,
> + NULL
> +};
> +
> +static const struct attribute_group twl4030_madc_group = {
> + .attrs = twl4030_madc_attributes,
> +};
> +
> +static int __devinit twl4030_madc_probe(struct platform_device *pdev)
> +{
> + struct twl4030_madc_data *madc;
> + struct twl4030_madc_platform_data *pdata = pdev->dev.platform_data;
> + int ret;
> + int status;
> + u8 regval;
> +
> + madc = kzalloc(sizeof *madc, GFP_KERNEL);
> + if (!madc)
> + return -ENOMEM;
> +
> + if (!pdata) {
> + dev_dbg(&pdev->dev, "platform_data not available\n");
> + ret = -EINVAL;
> + goto err_misc;
> + }
You can check that before allocating memory.
> + madc->imr = (pdata->irq_line == 1) ?
> + TWL4030_MADC_IMR1 : TWL4030_MADC_IMR2;
> + madc->isr = (pdata->irq_line == 1) ?
> + TWL4030_MADC_ISR1 : TWL4030_MADC_ISR2;
> + twl4030_madc_set_power(madc, 1);
> +
> + twl4030_madc_set_current_generator(madc, 0, 1);
> +
> + ret = twl_i2c_read_u8(TWL4030_MODULE_MAIN_CHARGE,
> + ®val, TWL4030_BCI_BCICTL1);
> + if (ret) {
> + dev_err(&pdev->dev,
> + "unable to read register 0x%X\n", TWL4030_BCI_BCICTL1);
No action ?
> + }
> + regval |= TWL4030_BCI_MESBAT;
> +
If the read failed above, you are going to write random data.
Not sure if that makes sense.
> + ret = twl_i2c_write_u8(TWL4030_MODULE_MAIN_CHARGE,
> + regval, TWL4030_BCI_BCICTL1);
> + if (ret) {
> + dev_err(&pdev->dev,
> + "unable to write register 0x%X\n", TWL4030_BCI_BCICTL1);
No action ?
It might make sense to bail out if register read/writes fail.
> + }
> +
> + platform_set_drvdata(pdev, madc);
> + mutex_init(&madc->lock);
> +
> + ret = request_threaded_irq(platform_get_irq(pdev, 0), NULL,
> + twl4030_madc_irq_handler,
> + IRQF_TRIGGER_RISING, "twl4030_madc", madc);
> + if (ret) {
> + dev_dbg(&pdev->dev, "could not request irq\n");
> + goto err_irq;
Jump to release irq for which the request failed.
> + }
> +
> + ret = sysfs_create_group(&pdev->dev.kobj, &twl4030_madc_group);
> + if (ret)
> + goto err_sysfs;
Jump to remove group which wasn't added.
> +
> + madc->hwmon_dev = hwmon_device_register(&pdev->dev);
> + if (IS_ERR(madc->hwmon_dev)) {
> + dev_err(&pdev->dev, "hwmon_device_register failed.\n");
> + status = PTR_ERR(madc->hwmon_dev);
> + goto err_reg;
This jumps to err_reg where you try to unregister the device for which registration failed.
> + }
> +
> + the_madc = madc;
> + return 0;
> +
> +err_reg:
> + hwmon_device_unregister(&pdev->dev);
> +err_sysfs:
> + sysfs_remove_group(&pdev->dev.kobj, &twl4030_madc_group);
> +err_irq:
> + free_irq(platform_get_irq(pdev, 0), madc);
> + platform_set_drvdata(pdev, NULL);
> + twl4030_madc_set_power(madc, 0);
> + twl4030_madc_set_current_generator(madc, 0, 0);
> +err_misc:
> + kfree(madc);
> +
> + return ret;
> +}
> +
> +static int __devexit twl4030_madc_remove(struct platform_device *pdev)
> +{
> + struct twl4030_madc_data *madc = platform_get_drvdata(pdev);
> +
> + hwmon_device_unregister(&pdev->dev);
> + sysfs_remove_group(&pdev->dev.kobj, &twl4030_madc_group);
> + free_irq(platform_get_irq(pdev, 0), madc);
> + platform_set_drvdata(pdev, NULL);
> + twl4030_madc_set_current_generator(madc, 0, 0);
> + twl4030_madc_set_power(madc, 0);
> + kfree(madc);
> +
> + return 0;
> +}
> +
> +static struct platform_driver twl4030_madc_driver = {
> + .probe = twl4030_madc_probe,
> + .remove = __exit_p(twl4030_madc_remove),
> + .driver = {
> + .name = "twl4030_madc",
> + .owner = THIS_MODULE,
> + },
> +};
> +
> +static int __init twl4030_madc_init(void)
> +{
> + return platform_driver_register(&twl4030_madc_driver);
> +}
> +
> +module_init(twl4030_madc_init);
> +
> +static void __exit twl4030_madc_exit(void)
> +{
> + platform_driver_unregister(&twl4030_madc_driver);
> +}
> +
> +module_exit(twl4030_madc_exit);
> +
> +MODULE_DESCRIPTION("TWL4030 ADC driver");
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("J Keerthy");
> diff --git a/include/linux/i2c/twl4030-madc.h b/include/linux/i2c/twl4030-madc.h
> new file mode 100644
> index 0000000..0e4e227
> --- /dev/null
> +++ b/include/linux/i2c/twl4030-madc.h
> @@ -0,0 +1,118 @@
> +/*
> + * 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
> + *
> + */
> +
> +#ifndef _TWL4030_MADC_H
> +#define _TWL4030_MADC_H
> +
> +struct twl4030_madc_conversion_method {
> + u8 sel;
> + u8 avg;
> + u8 rbase;
> + u8 ctrl;
> +};
> +
> +#define TWL4030_MADC_MAX_CHANNELS 16
> +
> +struct twl4030_madc_request {
> + u16 channels;
> + u16 do_avg;
> + u16 method;
> + u16 type;
> + bool active;
> + bool result_pending;
> + int rbuf[TWL4030_MADC_MAX_CHANNELS];
> + void (*func_cb)(int len, int channels, int *buf);
> +};
> +
> +enum conversion_methods {
> + TWL4030_MADC_RT,
> + TWL4030_MADC_SW1,
> + TWL4030_MADC_SW2,
> + TWL4030_MADC_NUM_METHODS
> +};
> +
> +enum sample_type {
> + TWL4030_MADC_WAIT,
> + TWL4030_MADC_IRQ_ONESHOT,
> + TWL4030_MADC_IRQ_REARM
> +};
> +
> +#define TWL4030_MADC_CTRL1 0x00
> +#define TWL4030_MADC_CTRL2 0x01
> +
> +#define TWL4030_MADC_RTSELECT_LSB 0x02
> +#define TWL4030_MADC_SW1SELECT_LSB 0x06
> +#define TWL4030_MADC_SW2SELECT_LSB 0x0A
> +
> +#define TWL4030_MADC_RTAVERAGE_LSB 0x04
> +#define TWL4030_MADC_SW1AVERAGE_LSB 0x08
> +#define TWL4030_MADC_SW2AVERAGE_LSB 0x0C
> +
> +#define TWL4030_MADC_CTRL_SW1 0x12
> +#define TWL4030_MADC_CTRL_SW2 0x13
> +
> +#define TWL4030_MADC_RTCH0_LSB 0x17
> +#define TWL4030_MADC_GPCH0_LSB 0x37
> +
> +#define TWL4030_MADC_MADCON (1 << 0) /* MADC power on */
> +#define TWL4030_MADC_BUSY (1 << 0) /* MADC busy */
> +#define TWL4030_MADC_EOC_SW (1 << 1) /* MADC conversion completion */
> +#define TWL4030_MADC_SW_START (1 << 5) /* MADC SWx start conversion */
> +
Formatting looks off here, and I am sure lines are > 80 columns.
> +#define TWL4030_MADC_ADCIN0 (1 << 0)
> +#define TWL4030_MADC_ADCIN1 (1 << 1)
> +#define TWL4030_MADC_ADCIN2 (1 << 2)
> +#define TWL4030_MADC_ADCIN3 (1 << 3)
> +#define TWL4030_MADC_ADCIN4 (1 << 4)
> +#define TWL4030_MADC_ADCIN5 (1 << 5)
> +#define TWL4030_MADC_ADCIN6 (1 << 6)
> +#define TWL4030_MADC_ADCIN7 (1 << 7)
> +#define TWL4030_MADC_ADCIN8 (1 << 8)
> +#define TWL4030_MADC_ADCIN9 (1 << 9)
> +#define TWL4030_MADC_ADCIN10 (1 << 10)
> +#define TWL4030_MADC_ADCIN11 (1 << 11)
> +#define TWL4030_MADC_ADCIN12 (1 << 12)
> +#define TWL4030_MADC_ADCIN13 (1 << 13)
> +#define TWL4030_MADC_ADCIN14 (1 << 14)
> +#define TWL4030_MADC_ADCIN15 (1 << 15)
> +
Please no tabs after #define.
> +/* Fixed channels */
> +#define TWL4030_MADC_BTEMP TWL4030_MADC_ADCIN1
> +#define TWL4030_MADC_VBUS TWL4030_MADC_ADCIN8
> +#define TWL4030_MADC_VBKB TWL4030_MADC_ADCIN9
> +#define TWL4030_MADC_ICHG TWL4030_MADC_ADCIN10
> +#define TWL4030_MADC_VCHG TWL4030_MADC_ADCIN11
> +#define TWL4030_MADC_VBAT TWL4030_MADC_ADCIN12
> +
> +#define TWL4030_BCI_BCICTL1 0x23
> +#define TWL4030_BCI_MESBAT (1 << 1)
> +#define TWL4030_BCI_TYPEN (1 << 4)
> +#define TWL4030_BCI_ITHEN (1 << 3)
> +
> +#define TWL4030_MADC_IOC_MAGIC '`'
> +#define TWL4030_MADC_IOCX_ADC_RAW_READ _IO(TWL4030_MADC_IOC_MAGIC, 0)
> +
> +struct twl4030_madc_user_parms {
> + int channel;
> + int average;
> + int status;
> + u16 result;
> +};
> +
> +int twl4030_madc_conversion(struct twl4030_madc_request *conv);
> +
> +#endif
> --
> 1.7.0.4
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [lm-sensors] [PATCH V2] hwmon: twl4030: Driver for twl4030 madc
@ 2010-11-11 0:01 ` Guenter Roeck
0 siblings, 0 replies; 10+ messages in thread
From: Guenter Roeck @ 2010-11-11 0:01 UTC (permalink / raw)
To: Keerthy
Cc: lm-sensors@lm-sensors.org, sameo@linux.intel.com,
khali@linux-fr.org, mikko.k.ylinen@nokia.com, balbi@ti.com,
amit.kucheria@canonical.com, linux-omap@vger.kernel.org,
balajitk@ti.com
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-keerthy@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 0000000..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-keerthy@ti.com>
> + *
> + * Based on twl4030-madc.c
> + * Copyright (C) 2008 Nokia Corporation
> + * Mikko Ylinen <mikko.k.ylinen@nokia.com>
> + *
> + * Amit Kucheria <amit.kucheria@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;
if (unlikely(val < 0))
return val;
return sprintf(buf, "%ld\n", req.rbuf[attr->index]);
should do and would be simpler. status is not needed at all.
> +}
> +
> +static
> +const struct twl4030_madc_conversion_method twl4030_conversion_methods[] = {
> + [TWL4030_MADC_RT] = {
> + .sel = TWL4030_MADC_RTSELECT_LSB,
> + .avg = TWL4030_MADC_RTAVERAGE_LSB,
> + .rbase = TWL4030_MADC_RTCH0_LSB,
> + },
> + [TWL4030_MADC_SW1] = {
> + .sel = TWL4030_MADC_SW1SELECT_LSB,
> + .avg = TWL4030_MADC_SW1AVERAGE_LSB,
> + .rbase = TWL4030_MADC_GPCH0_LSB,
> + .ctrl = TWL4030_MADC_CTRL_SW1,
> + },
> + [TWL4030_MADC_SW2] = {
> + .sel = TWL4030_MADC_SW2SELECT_LSB,
> + .avg = TWL4030_MADC_SW2AVERAGE_LSB,
> + .rbase = TWL4030_MADC_GPCH0_LSB,
> + .ctrl = TWL4030_MADC_CTRL_SW2,
> + },
> +};
> +
> +static int twl4030_madc_read(struct twl4030_madc_data *madc, u8 reg)
> +{
> + int ret;
> + u8 val;
> +
> + ret = twl_i2c_read_u8(TWL4030_MODULE_MADC, &val, reg);
> + if (ret) {
> + dev_dbg(madc->hwmon_dev, "unable to read register 0x%X\n", reg);
> + return ret;
> + }
> +
> + return val;
> +}
> +
> +static void twl4030_madc_write(struct twl4030_madc_data *madc, u8 reg, u8 val)
> +{
> + int ret;
> +
> + ret = twl_i2c_write_u8(TWL4030_MODULE_MADC, val, reg);
> + if (ret)
> + dev_err(madc->hwmon_dev,
> + "unable to write register 0x%X\n", reg);
> +}
Looking into other uses of twl_i2c_write_u8(), most check and return errors.
Not sure if it is a good idea to ignore all errors as it is done here. There
should at least some comment explaining that and why it is done.
> +
> +static int twl4030_madc_channel_raw_read(struct twl4030_madc_data *madc, u8 reg)
> +{
> + u8 msb, lsb;
> +
> + /*
> + * For each ADC channel, we have MSB and LSB register pair. MSB address
> + * is always LSB address+1. reg parameter is the addr of LSB register
> + */
> + msb = twl4030_madc_read(madc, reg + 1);
> + lsb = twl4030_madc_read(madc, reg);
> +
Ignoring returned errors and combining them into a random value doesn't make much sense.
> + return (int)(((msb << 8) | lsb) >> 6);
> +}
> +
> +static int twl4030_madc_read_channels(struct twl4030_madc_data *madc,
> + u8 reg_base, u16 channels, int *buf)
> +{
> + int count = 0;
> + u8 reg, i;
> +
> + if (unlikely(!buf))
> + return 0;
> +
I don't see how this can ever be NULL.
> + for (i = 0; i < TWL4030_MADC_MAX_CHANNELS; i++) {
> + if (channels & (1 << i)) {
> + reg = reg_base + 2 * i;
> + buf[i] = twl4030_madc_channel_raw_read(madc, reg);
Content of buf will be more or less random after errors.
> + count++;
> + }
> + }
> + return count;
> +}
> +
> +static void twl4030_madc_enable_irq(struct twl4030_madc_data *madc, int id)
> +{
> + u8 val;
> +
> + val = twl4030_madc_read(madc, madc->imr);
What if twl4030_madc_read() returned an error ? Try to write a random value ?
> + val &= ~(1 << id);
> + twl4030_madc_write(madc, madc->imr, val);
> +}
> +
> +static void twl4030_madc_disable_irq(struct twl4030_madc_data *madc, int id)
> +{
> + u8 val;
> +
> + val = twl4030_madc_read(madc, madc->imr);
Returned error ignored.
> + val |= (1 << id);
> + twl4030_madc_write(madc, madc->imr, val);
> +}
> +
> +static irqreturn_t twl4030_madc_irq_handler(int irq, void *_madc)
> +{
> + struct twl4030_madc_data *madc = _madc;
> + const struct twl4030_madc_conversion_method *method;
> + u8 isr_val, imr_val;
> + int i, len;
> + struct twl4030_madc_request *r;
> +
> + isr_val = twl4030_madc_read(madc, madc->isr);
> + imr_val = twl4030_madc_read(madc, madc->imr);
> +
Returned errors ignored.
> + isr_val &= ~imr_val;
> +
> + for (i = 0; i < TWL4030_MADC_NUM_METHODS; i++) {
> +
> + if (!(isr_val & (1 << i)))
> + continue;
> +
> + twl4030_madc_disable_irq(madc, i);
> + madc->requests[i].result_pending = 1;
> + }
> + mutex_lock(&madc->lock);
> + for (i = 0; i < TWL4030_MADC_NUM_METHODS; i++) {
> +
> + r = &madc->requests[i];
> +
> + /* No pending results for this method, move to next one */
> + if (!r->result_pending)
> + continue;
> +
> + method = &twl4030_conversion_methods[r->method];
> +
> + /* Read results */
> + len = twl4030_madc_read_channels(madc, method->rbase,
> + r->channels, r->rbuf);
> + /* Return results to caller */
> + if (r->func_cb != NULL) {
> + r->func_cb(len, r->channels, r->rbuf);
> + r->func_cb = NULL;
> + }
> +
> + /* Free request */
> + r->result_pending = 0;
> + r->active = 0;
> + }
> +
> + mutex_unlock(&madc->lock);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int twl4030_madc_set_irq(struct twl4030_madc_data *madc,
> + struct twl4030_madc_request *req)
> +{
> + struct twl4030_madc_request *p;
> +
> + p = &madc->requests[req->method];
> +
> + memcpy(p, req, sizeof *req);
> +
> + twl4030_madc_enable_irq(madc, req->method);
> +
> + return 0;
> +}
> +
> +static inline void twl4030_madc_start_conversion(struct twl4030_madc_data *madc,
> + int conv_method)
> +{
> + const struct twl4030_madc_conversion_method *method;
> +
> + method = &twl4030_conversion_methods[conv_method];
> +
> + switch (conv_method) {
> + case TWL4030_MADC_SW1:
> + case TWL4030_MADC_SW2:
> + twl4030_madc_write(madc, method->ctrl, TWL4030_MADC_SW_START);
> + break;
> + case TWL4030_MADC_RT:
> + default:
> + break;
> + }
> +}
> +
> +static int twl4030_madc_wait_conversion_ready(struct twl4030_madc_data *madc,
> + unsigned int timeout_ms,
> + u8 status_reg)
> +{
> + unsigned long timeout;
> +
> + timeout = jiffies + msecs_to_jiffies(timeout_ms);
> + do {
> + u8 reg;
> +
> + reg = twl4030_madc_read(madc, status_reg);
Returned error ignored.
> + if (!(reg & TWL4030_MADC_BUSY) && (reg & TWL4030_MADC_EOC_SW))
> + return 0;
> + } while (!time_after(jiffies, timeout));
> +
> + return -EAGAIN;
> +}
> +
> +int twl4030_madc_conversion(struct twl4030_madc_request *req)
> +{
> + const struct twl4030_madc_conversion_method *method;
> + u8 ch_msb, ch_lsb;
> + int ret;
> +
> + if (unlikely(!req))
> + return -EINVAL;
> +
> + mutex_lock(&the_madc->lock);
> +
> + if (req->method < TWL4030_MADC_RT || req->method > TWL4030_MADC_SW2) {
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + /* Do we have a conversion request ongoing */
> + if (the_madc->requests[req->method].active) {
> + ret = -EBUSY;
> + goto out;
> + }
> +
> + ch_msb = (req->channels >> 8) & 0xff;
> + ch_lsb = req->channels & 0xff;
> +
> + method = &twl4030_conversion_methods[req->method];
> +
> + /* Select channels to be converted */
> + twl4030_madc_write(the_madc, method->sel + 1, ch_msb);
> + twl4030_madc_write(the_madc, method->sel, ch_lsb);
> +
> + /* Select averaging for all channels if do_avg is set */
> + if (req->do_avg) {
> + twl4030_madc_write(the_madc, method->avg + 1, ch_msb);
> + twl4030_madc_write(the_madc, method->avg, ch_lsb);
> + }
> +
> + if (req->type = TWL4030_MADC_IRQ_ONESHOT && req->func_cb != NULL) {
> + twl4030_madc_set_irq(the_madc, req);
> + twl4030_madc_start_conversion(the_madc, req->method);
> + the_madc->requests[req->method].active = 1;
> + ret = 0;
> + goto out;
> + }
> +
> + /* With RT method we should not be here anymore */
> + if (req->method = TWL4030_MADC_RT) {
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + twl4030_madc_start_conversion(the_madc, req->method);
> + the_madc->requests[req->method].active = 1;
> +
> + /* Wait until conversion is ready (ctrl register returns EOC) */
> + ret = twl4030_madc_wait_conversion_ready(the_madc, 5, method->ctrl);
> + if (ret) {
> + dev_dbg(the_madc->hwmon_dev, "conversion timeout!\n");
> + the_madc->requests[req->method].active = 0;
> + goto out;
> + }
> +
> + ret = twl4030_madc_read_channels(the_madc, method->rbase, req->channels,
> + req->rbuf);
> +
> + the_madc->requests[req->method].active = 0;
> +
> +out:
> + mutex_unlock(&the_madc->lock);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(twl4030_madc_conversion);
> +
> +/*
> + * Return channel value
> + * Or < 0 on failure.
> + */
> +static int twl4030_get_madc_conversion(int channel_no)
> +{
> + struct twl4030_madc_request req;
> + int temp = 0;
> + int ret;
> +
> + req.channels = (1 << channel_no);
> + req.method = TWL4030_MADC_SW2;
> + req.active = 0;
> + req.func_cb = NULL;
> + ret = twl4030_madc_conversion(&req);
> + if (ret < 0)
> + return ret;
> +
> + if (req.rbuf[channel_no] > 0)
> + temp = req.rbuf[channel_no];
> +
> + return temp;
> +}
> +EXPORT_SYMBOL(twl4030_get_madc_conversion);
> +
EXPORT_SYMBOL and static declaration is a contradiction.
Actually, this function is not used anywhere. Does this compile without warning ?
> +static void twl4030_madc_set_current_generator(struct twl4030_madc_data *madc,
> + int chan, int on)
> +{
> + int ret;
> + u8 regval;
> +
> + ret = twl_i2c_read_u8(TWL4030_MODULE_MAIN_CHARGE,
> + ®val, TWL4030_BCI_BCICTL1);
> + if (ret) {
> + dev_dbg(madc->hwmon_dev,
> + "unable to read register 0x%X\n", TWL4030_BCI_BCICTL1);
> + }
You detect the error ... then go ahead and use the random regval anyway.
> + if (on)
> + regval |= chan ? TWL4030_BCI_ITHEN : TWL4030_BCI_TYPEN;
> + else
> + regval &= chan ? ~TWL4030_BCI_ITHEN : ~TWL4030_BCI_TYPEN;
> + ret = twl_i2c_write_u8(TWL4030_MODULE_MAIN_CHARGE,
> + regval, TWL4030_BCI_BCICTL1);
> + if (ret)
> + dev_err(madc->hwmon_dev,
> + "unable to write register 0x%X\n", TWL4030_BCI_BCICTL1);
> +
Unnecessary blank line.
> +}
> +
> +static void twl4030_madc_set_power(struct twl4030_madc_data *madc, int on)
> +{
> + u8 regval;
> +
> + regval = twl4030_madc_read(madc, TWL4030_MADC_CTRL1);
Returned error ignored.
> + if (on)
> + regval |= TWL4030_MADC_MADCON;
> + else
> + regval &= ~TWL4030_MADC_MADCON;
> + twl4030_madc_write(madc, TWL4030_MADC_CTRL1, regval);
> +}
> +
> +static SENSOR_DEVICE_ATTR(in0_input, S_IRUGO, madc_read, NULL, 0);
> +static SENSOR_DEVICE_ATTR(in1_input, S_IRUGO, madc_read, NULL, 1);
> +static SENSOR_DEVICE_ATTR(in2_input, S_IRUGO, madc_read, NULL, 2);
> +static SENSOR_DEVICE_ATTR(in3_input, S_IRUGO, madc_read, NULL, 3);
> +static SENSOR_DEVICE_ATTR(in4_input, S_IRUGO, madc_read, NULL, 4);
> +static SENSOR_DEVICE_ATTR(in5_input, S_IRUGO, madc_read, NULL, 5);
> +static SENSOR_DEVICE_ATTR(in6_input, S_IRUGO, madc_read, NULL, 6);
> +static SENSOR_DEVICE_ATTR(in7_input, S_IRUGO, madc_read, NULL, 7);
> +static SENSOR_DEVICE_ATTR(in8_input, S_IRUGO, madc_read, NULL, 8);
> +static SENSOR_DEVICE_ATTR(in9_input, S_IRUGO, madc_read, NULL, 9);
> +static SENSOR_DEVICE_ATTR(in10_input, S_IRUGO, madc_read, NULL, 10);
> +static SENSOR_DEVICE_ATTR(in11_input, S_IRUGO, madc_read, NULL, 11);
> +static SENSOR_DEVICE_ATTR(in12_input, S_IRUGO, madc_read, NULL, 12);
> +static SENSOR_DEVICE_ATTR(in13_input, S_IRUGO, madc_read, NULL, 13);
> +static SENSOR_DEVICE_ATTR(in14_input, S_IRUGO, madc_read, NULL, 14);
> +static SENSOR_DEVICE_ATTR(in15_input, S_IRUGO, madc_read, NULL, 15);
> +
> +static struct attribute *twl4030_madc_attributes[] = {
> + &sensor_dev_attr_in0_input.dev_attr.attr,
> + &sensor_dev_attr_in1_input.dev_attr.attr,
> + &sensor_dev_attr_in2_input.dev_attr.attr,
> + &sensor_dev_attr_in3_input.dev_attr.attr,
> + &sensor_dev_attr_in4_input.dev_attr.attr,
> + &sensor_dev_attr_in5_input.dev_attr.attr,
> + &sensor_dev_attr_in6_input.dev_attr.attr,
> + &sensor_dev_attr_in7_input.dev_attr.attr,
> + &sensor_dev_attr_in8_input.dev_attr.attr,
> + &sensor_dev_attr_in9_input.dev_attr.attr,
> + &sensor_dev_attr_in10_input.dev_attr.attr,
> + &sensor_dev_attr_in11_input.dev_attr.attr,
> + &sensor_dev_attr_in12_input.dev_attr.attr,
> + &sensor_dev_attr_in13_input.dev_attr.attr,
> + &sensor_dev_attr_in14_input.dev_attr.attr,
> + &sensor_dev_attr_in15_input.dev_attr.attr,
> + NULL
> +};
> +
> +static const struct attribute_group twl4030_madc_group = {
> + .attrs = twl4030_madc_attributes,
> +};
> +
> +static int __devinit twl4030_madc_probe(struct platform_device *pdev)
> +{
> + struct twl4030_madc_data *madc;
> + struct twl4030_madc_platform_data *pdata = pdev->dev.platform_data;
> + int ret;
> + int status;
> + u8 regval;
> +
> + madc = kzalloc(sizeof *madc, GFP_KERNEL);
> + if (!madc)
> + return -ENOMEM;
> +
> + if (!pdata) {
> + dev_dbg(&pdev->dev, "platform_data not available\n");
> + ret = -EINVAL;
> + goto err_misc;
> + }
You can check that before allocating memory.
> + madc->imr = (pdata->irq_line = 1) ?
> + TWL4030_MADC_IMR1 : TWL4030_MADC_IMR2;
> + madc->isr = (pdata->irq_line = 1) ?
> + TWL4030_MADC_ISR1 : TWL4030_MADC_ISR2;
> + twl4030_madc_set_power(madc, 1);
> +
> + twl4030_madc_set_current_generator(madc, 0, 1);
> +
> + ret = twl_i2c_read_u8(TWL4030_MODULE_MAIN_CHARGE,
> + ®val, TWL4030_BCI_BCICTL1);
> + if (ret) {
> + dev_err(&pdev->dev,
> + "unable to read register 0x%X\n", TWL4030_BCI_BCICTL1);
No action ?
> + }
> + regval |= TWL4030_BCI_MESBAT;
> +
If the read failed above, you are going to write random data.
Not sure if that makes sense.
> + ret = twl_i2c_write_u8(TWL4030_MODULE_MAIN_CHARGE,
> + regval, TWL4030_BCI_BCICTL1);
> + if (ret) {
> + dev_err(&pdev->dev,
> + "unable to write register 0x%X\n", TWL4030_BCI_BCICTL1);
No action ?
It might make sense to bail out if register read/writes fail.
> + }
> +
> + platform_set_drvdata(pdev, madc);
> + mutex_init(&madc->lock);
> +
> + ret = request_threaded_irq(platform_get_irq(pdev, 0), NULL,
> + twl4030_madc_irq_handler,
> + IRQF_TRIGGER_RISING, "twl4030_madc", madc);
> + if (ret) {
> + dev_dbg(&pdev->dev, "could not request irq\n");
> + goto err_irq;
Jump to release irq for which the request failed.
> + }
> +
> + ret = sysfs_create_group(&pdev->dev.kobj, &twl4030_madc_group);
> + if (ret)
> + goto err_sysfs;
Jump to remove group which wasn't added.
> +
> + madc->hwmon_dev = hwmon_device_register(&pdev->dev);
> + if (IS_ERR(madc->hwmon_dev)) {
> + dev_err(&pdev->dev, "hwmon_device_register failed.\n");
> + status = PTR_ERR(madc->hwmon_dev);
> + goto err_reg;
This jumps to err_reg where you try to unregister the device for which registration failed.
> + }
> +
> + the_madc = madc;
> + return 0;
> +
> +err_reg:
> + hwmon_device_unregister(&pdev->dev);
> +err_sysfs:
> + sysfs_remove_group(&pdev->dev.kobj, &twl4030_madc_group);
> +err_irq:
> + free_irq(platform_get_irq(pdev, 0), madc);
> + platform_set_drvdata(pdev, NULL);
> + twl4030_madc_set_power(madc, 0);
> + twl4030_madc_set_current_generator(madc, 0, 0);
> +err_misc:
> + kfree(madc);
> +
> + return ret;
> +}
> +
> +static int __devexit twl4030_madc_remove(struct platform_device *pdev)
> +{
> + struct twl4030_madc_data *madc = platform_get_drvdata(pdev);
> +
> + hwmon_device_unregister(&pdev->dev);
> + sysfs_remove_group(&pdev->dev.kobj, &twl4030_madc_group);
> + free_irq(platform_get_irq(pdev, 0), madc);
> + platform_set_drvdata(pdev, NULL);
> + twl4030_madc_set_current_generator(madc, 0, 0);
> + twl4030_madc_set_power(madc, 0);
> + kfree(madc);
> +
> + return 0;
> +}
> +
> +static struct platform_driver twl4030_madc_driver = {
> + .probe = twl4030_madc_probe,
> + .remove = __exit_p(twl4030_madc_remove),
> + .driver = {
> + .name = "twl4030_madc",
> + .owner = THIS_MODULE,
> + },
> +};
> +
> +static int __init twl4030_madc_init(void)
> +{
> + return platform_driver_register(&twl4030_madc_driver);
> +}
> +
> +module_init(twl4030_madc_init);
> +
> +static void __exit twl4030_madc_exit(void)
> +{
> + platform_driver_unregister(&twl4030_madc_driver);
> +}
> +
> +module_exit(twl4030_madc_exit);
> +
> +MODULE_DESCRIPTION("TWL4030 ADC driver");
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("J Keerthy");
> diff --git a/include/linux/i2c/twl4030-madc.h b/include/linux/i2c/twl4030-madc.h
> new file mode 100644
> index 0000000..0e4e227
> --- /dev/null
> +++ b/include/linux/i2c/twl4030-madc.h
> @@ -0,0 +1,118 @@
> +/*
> + * 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
> + *
> + */
> +
> +#ifndef _TWL4030_MADC_H
> +#define _TWL4030_MADC_H
> +
> +struct twl4030_madc_conversion_method {
> + u8 sel;
> + u8 avg;
> + u8 rbase;
> + u8 ctrl;
> +};
> +
> +#define TWL4030_MADC_MAX_CHANNELS 16
> +
> +struct twl4030_madc_request {
> + u16 channels;
> + u16 do_avg;
> + u16 method;
> + u16 type;
> + bool active;
> + bool result_pending;
> + int rbuf[TWL4030_MADC_MAX_CHANNELS];
> + void (*func_cb)(int len, int channels, int *buf);
> +};
> +
> +enum conversion_methods {
> + TWL4030_MADC_RT,
> + TWL4030_MADC_SW1,
> + TWL4030_MADC_SW2,
> + TWL4030_MADC_NUM_METHODS
> +};
> +
> +enum sample_type {
> + TWL4030_MADC_WAIT,
> + TWL4030_MADC_IRQ_ONESHOT,
> + TWL4030_MADC_IRQ_REARM
> +};
> +
> +#define TWL4030_MADC_CTRL1 0x00
> +#define TWL4030_MADC_CTRL2 0x01
> +
> +#define TWL4030_MADC_RTSELECT_LSB 0x02
> +#define TWL4030_MADC_SW1SELECT_LSB 0x06
> +#define TWL4030_MADC_SW2SELECT_LSB 0x0A
> +
> +#define TWL4030_MADC_RTAVERAGE_LSB 0x04
> +#define TWL4030_MADC_SW1AVERAGE_LSB 0x08
> +#define TWL4030_MADC_SW2AVERAGE_LSB 0x0C
> +
> +#define TWL4030_MADC_CTRL_SW1 0x12
> +#define TWL4030_MADC_CTRL_SW2 0x13
> +
> +#define TWL4030_MADC_RTCH0_LSB 0x17
> +#define TWL4030_MADC_GPCH0_LSB 0x37
> +
> +#define TWL4030_MADC_MADCON (1 << 0) /* MADC power on */
> +#define TWL4030_MADC_BUSY (1 << 0) /* MADC busy */
> +#define TWL4030_MADC_EOC_SW (1 << 1) /* MADC conversion completion */
> +#define TWL4030_MADC_SW_START (1 << 5) /* MADC SWx start conversion */
> +
Formatting looks off here, and I am sure lines are > 80 columns.
> +#define TWL4030_MADC_ADCIN0 (1 << 0)
> +#define TWL4030_MADC_ADCIN1 (1 << 1)
> +#define TWL4030_MADC_ADCIN2 (1 << 2)
> +#define TWL4030_MADC_ADCIN3 (1 << 3)
> +#define TWL4030_MADC_ADCIN4 (1 << 4)
> +#define TWL4030_MADC_ADCIN5 (1 << 5)
> +#define TWL4030_MADC_ADCIN6 (1 << 6)
> +#define TWL4030_MADC_ADCIN7 (1 << 7)
> +#define TWL4030_MADC_ADCIN8 (1 << 8)
> +#define TWL4030_MADC_ADCIN9 (1 << 9)
> +#define TWL4030_MADC_ADCIN10 (1 << 10)
> +#define TWL4030_MADC_ADCIN11 (1 << 11)
> +#define TWL4030_MADC_ADCIN12 (1 << 12)
> +#define TWL4030_MADC_ADCIN13 (1 << 13)
> +#define TWL4030_MADC_ADCIN14 (1 << 14)
> +#define TWL4030_MADC_ADCIN15 (1 << 15)
> +
Please no tabs after #define.
> +/* Fixed channels */
> +#define TWL4030_MADC_BTEMP TWL4030_MADC_ADCIN1
> +#define TWL4030_MADC_VBUS TWL4030_MADC_ADCIN8
> +#define TWL4030_MADC_VBKB TWL4030_MADC_ADCIN9
> +#define TWL4030_MADC_ICHG TWL4030_MADC_ADCIN10
> +#define TWL4030_MADC_VCHG TWL4030_MADC_ADCIN11
> +#define TWL4030_MADC_VBAT TWL4030_MADC_ADCIN12
> +
> +#define TWL4030_BCI_BCICTL1 0x23
> +#define TWL4030_BCI_MESBAT (1 << 1)
> +#define TWL4030_BCI_TYPEN (1 << 4)
> +#define TWL4030_BCI_ITHEN (1 << 3)
> +
> +#define TWL4030_MADC_IOC_MAGIC '`'
> +#define TWL4030_MADC_IOCX_ADC_RAW_READ _IO(TWL4030_MADC_IOC_MAGIC, 0)
> +
> +struct twl4030_madc_user_parms {
> + int channel;
> + int average;
> + int status;
> + u16 result;
> +};
> +
> +int twl4030_madc_conversion(struct twl4030_madc_request *conv);
> +
> +#endif
> --
> 1.7.0.4
>
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V2] hwmon: twl4030: Driver for twl4030 madc module
[not found] ` <AANLkTiniRxNoNn9bSR+PN8B=m6=YVWS8Na_vJUdi1QhT@mail.gmail.com>
@ 2010-11-26 6:44 ` J, KEERTHY
0 siblings, 0 replies; 10+ messages in thread
From: J, KEERTHY @ 2010-11-26 6:32 UTC (permalink / raw)
To: guenter.roeck
Cc: lm-sensors, sameo, khali, mikko.k.ylinen, Balbi, Felipe,
amit.kucheria, linux-omap, Krishnamoorthy, Balaji T
On Thu, Nov 11, 2010 at 5:31 AM, Guenter Roeck
<guenter.roeck@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-keerthy@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 0000000..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-keerthy@ti.com>
>> + *
>> + * Based on twl4030-madc.c
>> + * Copyright (C) 2008 Nokia Corporation
>> + * Mikko Ylinen <mikko.k.ylinen@nokia.com>
>> + *
>> + * Amit Kucheria <amit.kucheria@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;
>
>
> if (unlikely(val < 0))
> return val;
>
> return sprintf(buf, "%ld\n", req.rbuf[attr->index]);
>
> should do and would be simpler. status is not needed at all.
>
Ok i will remove.
>> +}
>> +
>> +static
>> +const struct twl4030_madc_conversion_method
> twl4030_conversion_methods[] = {
>> + [TWL4030_MADC_RT] = {
>> + .sel = TWL4030_MADC_RTSELECT_LSB,
>> + .avg = TWL4030_MADC_RTAVERAGE_LSB,
>> + .rbase = TWL4030_MADC_RTCH0_LSB,
>> + },
>> + [TWL4030_MADC_SW1] = {
>> + .sel = TWL4030_MADC_SW1SELECT_LSB,
>> + .avg = TWL4030_MADC_SW1AVERAGE_LSB,
>> + .rbase = TWL4030_MADC_GPCH0_LSB,
>> + .ctrl = TWL4030_MADC_CTRL_SW1,
>> + },
>> + [TWL4030_MADC_SW2] = {
>> + .sel = TWL4030_MADC_SW2SELECT_LSB,
>> + .avg = TWL4030_MADC_SW2AVERAGE_LSB,
>> + .rbase = TWL4030_MADC_GPCH0_LSB,
>> + .ctrl = TWL4030_MADC_CTRL_SW2,
>> + },
>> +};
>> +
>> +static int twl4030_madc_read(struct twl4030_madc_data *madc, u8 reg)
>> +{
>> + int ret;
>> + u8 val;
>> +
>> + ret = twl_i2c_read_u8(TWL4030_MODULE_MADC, &val, reg);
>> + if (ret) {
>> + dev_dbg(madc->hwmon_dev, "unable to read register
> 0x%X\n", reg);
>> + return ret;
>> + }
>> +
>> + return val;
>> +}
>> +
>> +static void twl4030_madc_write(struct twl4030_madc_data *madc, u8 reg,
> u8 val)
>> +{
>> + int ret;
>> +
>> + ret = twl_i2c_write_u8(TWL4030_MODULE_MADC, val, reg);
>> + if (ret)
>> + dev_err(madc->hwmon_dev,
>> + "unable to write register 0x%X\n", reg);
>> +}
>
> Looking into other uses of twl_i2c_write_u8(), most check and return
> errors.
> Not sure if it is a good idea to ignore all errors as it is done here.
> There
> should at least some comment explaining that and why it is done.
>
Ok. Error check will be in place.
>> +
>> +static int twl4030_madc_channel_raw_read(struct twl4030_madc_data
> *madc, u8 reg)
>> +{
>> + u8 msb, lsb;
>> +
>> + /*
>> + * For each ADC channel, we have MSB and LSB register pair. MSB
> address
>> + * is always LSB address+1. reg parameter is the addr of LSB
> register
>> + */
>> + msb = twl4030_madc_read(madc, reg + 1);
>> + lsb = twl4030_madc_read(madc, reg);
>> +
> Ignoring returned errors and combining them into a random value doesn't
> make much sense.
Return checks will be added.
>
>> + return (int)(((msb << 8) | lsb) >> 6);
>> +}
>> +
>> +static int twl4030_madc_read_channels(struct twl4030_madc_data *madc,
>> + u8 reg_base, u16 channels, int
> *buf)
>> +{
>> + int count = 0;
>> + u8 reg, i;
>> +
>> + if (unlikely(!buf))
>> + return 0;
>> +
> I don't see how this can ever be NULL.
Ok i will remove it.
>
>> + for (i = 0; i < TWL4030_MADC_MAX_CHANNELS; i++) {
>> + if (channels & (1 << i)) {
>> + reg = reg_base + 2 * i;
>> + buf[i] = twl4030_madc_channel_raw_read(madc,
> reg);
>
> Content of buf will be more or less random after errors.
Ok i will add a return check for every iteration.
>
>> + count++;
>> + }
>> + }
>> + return count;
>> +}
>> +
>> +static void twl4030_madc_enable_irq(struct twl4030_madc_data *madc, int
> id)
>> +{
>> + u8 val;
>> +
>> + val = twl4030_madc_read(madc, madc->imr);
>
> What if twl4030_madc_read() returned an error ? Try to write a random
> value ?
>
>> + val &= ~(1 << id);
>> + twl4030_madc_write(madc, madc->imr, val);
>> +}
>> +
>> +static void twl4030_madc_disable_irq(struct twl4030_madc_data *madc,
> int id)
>> +{
>> + u8 val;
>> +
>> + val = twl4030_madc_read(madc, madc->imr);
>
> Returned error ignored.
Return checks will be added for read followed by writes.
>
>> + val |= (1 << id);
>> + twl4030_madc_write(madc, madc->imr, val);
>> +}
>> +
>> +static irqreturn_t twl4030_madc_irq_handler(int irq, void *_madc)
>> +{
>> + struct twl4030_madc_data *madc = _madc;
>> + const struct twl4030_madc_conversion_method *method;
>> + u8 isr_val, imr_val;
>> + int i, len;
>> + struct twl4030_madc_request *r;
>> +
>> + isr_val = twl4030_madc_read(madc, madc->isr);
>> + imr_val = twl4030_madc_read(madc, madc->imr);
>> +
> Returned errors ignored.
I will add a check.
>
>> + isr_val &= ~imr_val;
>> +
>> + for (i = 0; i < TWL4030_MADC_NUM_METHODS; i++) {
>> +
>> + if (!(isr_val & (1 << i)))
>> + continue;
>> +
>> + twl4030_madc_disable_irq(madc, i);
>> + madc->requests[i].result_pending = 1;
>> + }
>> + mutex_lock(&madc->lock);
>> + for (i = 0; i < TWL4030_MADC_NUM_METHODS; i++) {
>> +
>> + r = &madc->requests[i];
>> +
>> + /* No pending results for this method, move to next one
> */
>> + if (!r->result_pending)
>> + continue;
>> +
>> + method = &twl4030_conversion_methods[r->method];
>> +
>> + /* Read results */
>> + len = twl4030_madc_read_channels(madc, method->rbase,
>> + r->channels, r->rbuf);
>> + /* Return results to caller */
>> + if (r->func_cb != NULL) {
>> + r->func_cb(len, r->channels, r->rbuf);
>> + r->func_cb = NULL;
>> + }
>> +
>> + /* Free request */
>> + r->result_pending = 0;
>> + r->active = 0;
>> + }
>> +
>> + mutex_unlock(&madc->lock);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static int twl4030_madc_set_irq(struct twl4030_madc_data *madc,
>> + struct twl4030_madc_request *req)
>> +{
>> + struct twl4030_madc_request *p;
>> +
>> + p = &madc->requests[req->method];
>> +
>> + memcpy(p, req, sizeof *req);
>> +
>> + twl4030_madc_enable_irq(madc, req->method);
>> +
>> + return 0;
>> +}
>> +
>> +static inline void twl4030_madc_start_conversion(struct
> twl4030_madc_data *madc,
>> + int conv_method)
>> +{
>> + const struct twl4030_madc_conversion_method *method;
>> +
>> + method = &twl4030_conversion_methods[conv_method];
>> +
>> + switch (conv_method) {
>> + case TWL4030_MADC_SW1:
>> + case TWL4030_MADC_SW2:
>> + twl4030_madc_write(madc, method->ctrl,
> TWL4030_MADC_SW_START);
>> + break;
>> + case TWL4030_MADC_RT:
>> + default:
>> + break;
>> + }
>> +}
>> +
>> +static int twl4030_madc_wait_conversion_ready(struct twl4030_madc_data
> *madc,
>> + unsigned int timeout_ms,
>> + u8 status_reg)
>> +{
>> + unsigned long timeout;
>> +
>> + timeout = jiffies + msecs_to_jiffies(timeout_ms);
>> + do {
>> + u8 reg;
>> +
>> + reg = twl4030_madc_read(madc, status_reg);
>
> Returned error ignored.
Return check will be addded.
>
>> + if (!(reg & TWL4030_MADC_BUSY) && (reg &
> TWL4030_MADC_EOC_SW))
>> + return 0;
>> + } while (!time_after(jiffies, timeout));
>> +
>> + return -EAGAIN;
>> +}
>> +
>> +int twl4030_madc_conversion(struct twl4030_madc_request *req)
>> +{
>> + const struct twl4030_madc_conversion_method *method;
>> + u8 ch_msb, ch_lsb;
>> + int ret;
>> +
>> + if (unlikely(!req))
>> + return -EINVAL;
>> +
>> + mutex_lock(&the_madc->lock);
>> +
>> + if (req->method < TWL4030_MADC_RT || req->method >
> TWL4030_MADC_SW2) {
>> + ret = -EINVAL;
>> + goto out;
>> + }
>> +
>> + /* Do we have a conversion request ongoing */
>> + if (the_madc->requests[req->method].active) {
>> + ret = -EBUSY;
>> + goto out;
>> + }
>> +
>> + ch_msb = (req->channels >> 8) & 0xff;
>> + ch_lsb = req->channels & 0xff;
>> +
>> + method = &twl4030_conversion_methods[req->method];
>> +
>> + /* Select channels to be converted */
>> + twl4030_madc_write(the_madc, method->sel + 1, ch_msb);
>> + twl4030_madc_write(the_madc, method->sel, ch_lsb);
>> +
>> + /* Select averaging for all channels if do_avg is set */
>> + if (req->do_avg) {
>> + twl4030_madc_write(the_madc, method->avg + 1, ch_msb);
>> + twl4030_madc_write(the_madc, method->avg, ch_lsb);
>> + }
>> +
>> + if (req->type == TWL4030_MADC_IRQ_ONESHOT && req->func_cb !=
> NULL) {
>> + twl4030_madc_set_irq(the_madc, req);
>> + twl4030_madc_start_conversion(the_madc, req->method);
>> + the_madc->requests[req->method].active = 1;
>> + ret = 0;
>> + goto out;
>> + }
>> +
>> + /* With RT method we should not be here anymore */
>> + if (req->method == TWL4030_MADC_RT) {
>> + ret = -EINVAL;
>> + goto out;
>> + }
>> +
>> + twl4030_madc_start_conversion(the_madc, req->method);
>> + the_madc->requests[req->method].active = 1;
>> +
>> + /* Wait until conversion is ready (ctrl register returns EOC) */
>> + ret = twl4030_madc_wait_conversion_ready(the_madc, 5,
> method->ctrl);
>> + if (ret) {
>> + dev_dbg(the_madc->hwmon_dev, "conversion timeout!\n");
>> + the_madc->requests[req->method].active = 0;
>> + goto out;
>> + }
>> +
>> + ret = twl4030_madc_read_channels(the_madc, method->rbase,
> req->channels,
>> + req->rbuf);
>> +
>> + the_madc->requests[req->method].active = 0;
>> +
>> +out:
>> + mutex_unlock(&the_madc->lock);
>> +
>> + return ret;
>> +}
>> +EXPORT_SYMBOL(twl4030_madc_conversion);
>> +
>> +/*
>> + * Return channel value
>> + * Or < 0 on failure.
>> + */
>> +static int twl4030_get_madc_conversion(int channel_no)
>> +{
>> + struct twl4030_madc_request req;
>> + int temp = 0;
>> + int ret;
>> +
>> + req.channels = (1 << channel_no);
>> + req.method = TWL4030_MADC_SW2;
>> + req.active = 0;
>> + req.func_cb = NULL;
>> + ret = twl4030_madc_conversion(&req);
>> + if (ret < 0)
>> + return ret;
>> +
>> + if (req.rbuf[channel_no] > 0)
>> + temp = req.rbuf[channel_no];
>> +
>> + return temp;
>> +}
>> +EXPORT_SYMBOL(twl4030_get_madc_conversion);
>> +
>
> EXPORT_SYMBOL and static declaration is a contradiction.
> Actually, this function is not used anywhere. Does this compile without
> warning ?
I will add the function declaration in a header file.
>
>> +static void twl4030_madc_set_current_generator(struct twl4030_madc_data
> *madc,
>> + int chan, int on)
>> +{
>> + int ret;
>> + u8 regval;
>> +
>> + ret = twl_i2c_read_u8(TWL4030_MODULE_MAIN_CHARGE,
>> + ®val, TWL4030_BCI_BCICTL1);
>> + if (ret) {
>> + dev_dbg(madc->hwmon_dev,
>> + "unable to read register 0x%X\n",
> TWL4030_BCI_BCICTL1);
>> + }
> You detect the error ... then go ahead and use the random regval anyway.
>
I will add a return value check.
>> + if (on)
>> + regval |= chan ? TWL4030_BCI_ITHEN : TWL4030_BCI_TYPEN;
>> + else
>> + regval &= chan ? ~TWL4030_BCI_ITHEN :
> ~TWL4030_BCI_TYPEN;
>> + ret = twl_i2c_write_u8(TWL4030_MODULE_MAIN_CHARGE,
>> + regval, TWL4030_BCI_BCICTL1);
>> + if (ret)
>> + dev_err(madc->hwmon_dev,
>> + "unable to write register 0x%X\n",
> TWL4030_BCI_BCICTL1);
>> +
> Unnecessary blank line.
I will reomve it.
>
>> +}
>> +
>> +static void twl4030_madc_set_power(struct twl4030_madc_data *madc, int
> on)
>> +{
>> + u8 regval;
>> +
>> + regval = twl4030_madc_read(madc, TWL4030_MADC_CTRL1);
>
> Returned error ignored.
Return check will be added.
>
>> + if (on)
>> + regval |= TWL4030_MADC_MADCON;
>> + else
>> + regval &= ~TWL4030_MADC_MADCON;
>> + twl4030_madc_write(madc, TWL4030_MADC_CTRL1, regval);
>> +}
>> +
>> +static SENSOR_DEVICE_ATTR(in0_input, S_IRUGO, madc_read, NULL, 0);
>> +static SENSOR_DEVICE_ATTR(in1_input, S_IRUGO, madc_read, NULL, 1);
>> +static SENSOR_DEVICE_ATTR(in2_input, S_IRUGO, madc_read, NULL, 2);
>> +static SENSOR_DEVICE_ATTR(in3_input, S_IRUGO, madc_read, NULL, 3);
>> +static SENSOR_DEVICE_ATTR(in4_input, S_IRUGO, madc_read, NULL, 4);
>> +static SENSOR_DEVICE_ATTR(in5_input, S_IRUGO, madc_read, NULL, 5);
>> +static SENSOR_DEVICE_ATTR(in6_input, S_IRUGO, madc_read, NULL, 6);
>> +static SENSOR_DEVICE_ATTR(in7_input, S_IRUGO, madc_read, NULL, 7);
>> +static SENSOR_DEVICE_ATTR(in8_input, S_IRUGO, madc_read, NULL, 8);
>> +static SENSOR_DEVICE_ATTR(in9_input, S_IRUGO, madc_read, NULL, 9);
>> +static SENSOR_DEVICE_ATTR(in10_input, S_IRUGO, madc_read, NULL, 10);
>> +static SENSOR_DEVICE_ATTR(in11_input, S_IRUGO, madc_read, NULL, 11);
>> +static SENSOR_DEVICE_ATTR(in12_input, S_IRUGO, madc_read, NULL, 12);
>> +static SENSOR_DEVICE_ATTR(in13_input, S_IRUGO, madc_read, NULL, 13);
>> +static SENSOR_DEVICE_ATTR(in14_input, S_IRUGO, madc_read, NULL, 14);
>> +static SENSOR_DEVICE_ATTR(in15_input, S_IRUGO, madc_read, NULL, 15);
>> +
>> +static struct attribute *twl4030_madc_attributes[] = {
>> + &sensor_dev_attr_in0_input.dev_attr.attr,
>> + &sensor_dev_attr_in1_input.dev_attr.attr,
>> + &sensor_dev_attr_in2_input.dev_attr.attr,
>> + &sensor_dev_attr_in3_input.dev_attr.attr,
>> + &sensor_dev_attr_in4_input.dev_attr.attr,
>> + &sensor_dev_attr_in5_input.dev_attr.attr,
>> + &sensor_dev_attr_in6_input.dev_attr.attr,
>> + &sensor_dev_attr_in7_input.dev_attr.attr,
>> + &sensor_dev_attr_in8_input.dev_attr.attr,
>> + &sensor_dev_attr_in9_input.dev_attr.attr,
>> + &sensor_dev_attr_in10_input.dev_attr.attr,
>> + &sensor_dev_attr_in11_input.dev_attr.attr,
>> + &sensor_dev_attr_in12_input.dev_attr.attr,
>> + &sensor_dev_attr_in13_input.dev_attr.attr,
>> + &sensor_dev_attr_in14_input.dev_attr.attr,
>> + &sensor_dev_attr_in15_input.dev_attr.attr,
>> + NULL
>> +};
>> +
>> +static const struct attribute_group twl4030_madc_group = {
>> + .attrs = twl4030_madc_attributes,
>> +};
>> +
>> +static int __devinit twl4030_madc_probe(struct platform_device *pdev)
>> +{
>> + struct twl4030_madc_data *madc;
>> + struct twl4030_madc_platform_data *pdata =
> pdev->dev.platform_data;
>> + int ret;
>> + int status;
>> + u8 regval;
>> +
>> + madc = kzalloc(sizeof *madc, GFP_KERNEL);
>> + if (!madc)
>> + return -ENOMEM;
>> +
>> + if (!pdata) {
>> + dev_dbg(&pdev->dev, "platform_data not available\n");
>> + ret = -EINVAL;
>> + goto err_misc;
>> + }
> You can check that before allocating memory.
Ok i will do that.
>
>> + madc->imr = (pdata->irq_line == 1) ?
>> + TWL4030_MADC_IMR1 : TWL4030_MADC_IMR2;
>> + madc->isr = (pdata->irq_line == 1) ?
>> + TWL4030_MADC_ISR1 : TWL4030_MADC_ISR2;
>> + twl4030_madc_set_power(madc, 1);
>> +
>> + twl4030_madc_set_current_generator(madc, 0, 1);
>> +
>> + ret = twl_i2c_read_u8(TWL4030_MODULE_MAIN_CHARGE,
>> + ®val, TWL4030_BCI_BCICTL1);
>> + if (ret) {
>> + dev_err(&pdev->dev,
>> + "unable to read register 0x%X\n",
> TWL4030_BCI_BCICTL1);
>
> No action ?
Ok. If read fails subsequent write will not make sense i will return an error.
>
>> + }
>> + regval |= TWL4030_BCI_MESBAT;
>> +
> If the read failed above, you are going to write random data.
> Not sure if that makes sense.
Ok i will return an error.
>
>> + ret = twl_i2c_write_u8(TWL4030_MODULE_MAIN_CHARGE,
>> + regval, TWL4030_BCI_BCICTL1);
>> + if (ret) {
>> + dev_err(&pdev->dev,
>> + "unable to write register 0x%X\n",
> TWL4030_BCI_BCICTL1);
>
> No action ?
>
> It might make sense to bail out if register read/writes fail.
Ok i will return error if it fails.
>
>> + }
>> +
>> + platform_set_drvdata(pdev, madc);
>> + mutex_init(&madc->lock);
>> +
>> + ret = request_threaded_irq(platform_get_irq(pdev, 0), NULL,
>> + twl4030_madc_irq_handler,
>> + IRQF_TRIGGER_RISING, "twl4030_madc",
> madc);
>> + if (ret) {
>> + dev_dbg(&pdev->dev, "could not request irq\n");
>> + goto err_irq;
>
> Jump to release irq for which the request failed.
Ok i will revert only the previously succeeded steps.
>
>> + }
>> +
>> + ret = sysfs_create_group(&pdev->dev.kobj, &twl4030_madc_group);
>> + if (ret)
>> + goto err_sysfs;
>
> Jump to remove group which wasn't added.
Ok i will revert only the previously succeeded steps.
>
>> +
>> + madc->hwmon_dev = hwmon_device_register(&pdev->dev);
>> + if (IS_ERR(madc->hwmon_dev)) {
>> + dev_err(&pdev->dev, "hwmon_device_register failed.\n");
>> + status = PTR_ERR(madc->hwmon_dev);
>> + goto err_reg;
>
> This jumps to err_reg where you try to unregister the device for which
> registration failed.
>
Ok i will revert only the previously succeeded steps.
>> + }
>> +
>> + the_madc = madc;
>> + return 0;
>> +
>> +err_reg:
>> + hwmon_device_unregister(&pdev->dev);
>> +err_sysfs:
>> + sysfs_remove_group(&pdev->dev.kobj, &twl4030_madc_group);
>> +err_irq:
>> + free_irq(platform_get_irq(pdev, 0), madc);
>> + platform_set_drvdata(pdev, NULL);
>> + twl4030_madc_set_power(madc, 0);
>> + twl4030_madc_set_current_generator(madc, 0, 0);
>> +err_misc:
>> + kfree(madc);
>> +
>> + return ret;
>> +}
>> +
>> +static int __devexit twl4030_madc_remove(struct platform_device *pdev)
>> +{
>> + struct twl4030_madc_data *madc = platform_get_drvdata(pdev);
>> +
>> + hwmon_device_unregister(&pdev->dev);
>> + sysfs_remove_group(&pdev->dev.kobj, &twl4030_madc_group);
>> + free_irq(platform_get_irq(pdev, 0), madc);
>> + platform_set_drvdata(pdev, NULL);
>> + twl4030_madc_set_current_generator(madc, 0, 0);
>> + twl4030_madc_set_power(madc, 0);
>> + kfree(madc);
>> +
>> + return 0;
>> +}
>> +
>> +static struct platform_driver twl4030_madc_driver = {
>> + .probe = twl4030_madc_probe,
>> + .remove = __exit_p(twl4030_madc_remove),
>> + .driver = {
>> + .name = "twl4030_madc",
>> + .owner = THIS_MODULE,
>> + },
>> +};
>> +
>> +static int __init twl4030_madc_init(void)
>> +{
>> + return platform_driver_register(&twl4030_madc_driver);
>> +}
>> +
>> +module_init(twl4030_madc_init);
>> +
>> +static void __exit twl4030_madc_exit(void)
>> +{
>> + platform_driver_unregister(&twl4030_madc_driver);
>> +}
>> +
>> +module_exit(twl4030_madc_exit);
>> +
>> +MODULE_DESCRIPTION("TWL4030 ADC driver");
>> +MODULE_LICENSE("GPL");
>> +MODULE_AUTHOR("J Keerthy");
>> diff --git a/include/linux/i2c/twl4030-madc.h
> b/include/linux/i2c/twl4030-madc.h
>> new file mode 100644
>> index 0000000..0e4e227
>> --- /dev/null
>> +++ b/include/linux/i2c/twl4030-madc.h
>> @@ -0,0 +1,118 @@
>> +/*
>> + * 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
>> + *
>> + */
>> +
>> +#ifndef _TWL4030_MADC_H
>> +#define _TWL4030_MADC_H
>> +
>> +struct twl4030_madc_conversion_method {
>> + u8 sel;
>> + u8 avg;
>> + u8 rbase;
>> + u8 ctrl;
>> +};
>> +
>> +#define TWL4030_MADC_MAX_CHANNELS 16
>> +
>> +struct twl4030_madc_request {
>> + u16 channels;
>> + u16 do_avg;
>> + u16 method;
>> + u16 type;
>> + bool active;
>> + bool result_pending;
>> + int rbuf[TWL4030_MADC_MAX_CHANNELS];
>> + void (*func_cb)(int len, int channels, int *buf);
>> +};
>> +
>> +enum conversion_methods {
>> + TWL4030_MADC_RT,
>> + TWL4030_MADC_SW1,
>> + TWL4030_MADC_SW2,
>> + TWL4030_MADC_NUM_METHODS
>> +};
>> +
>> +enum sample_type {
>> + TWL4030_MADC_WAIT,
>> + TWL4030_MADC_IRQ_ONESHOT,
>> + TWL4030_MADC_IRQ_REARM
>> +};
>> +
>> +#define TWL4030_MADC_CTRL1 0x00
>> +#define TWL4030_MADC_CTRL2 0x01
>> +
>> +#define TWL4030_MADC_RTSELECT_LSB 0x02
>> +#define TWL4030_MADC_SW1SELECT_LSB 0x06
>> +#define TWL4030_MADC_SW2SELECT_LSB 0x0A
>> +
>> +#define TWL4030_MADC_RTAVERAGE_LSB 0x04
>> +#define TWL4030_MADC_SW1AVERAGE_LSB 0x08
>> +#define TWL4030_MADC_SW2AVERAGE_LSB 0x0C
>> +
>> +#define TWL4030_MADC_CTRL_SW1 0x12
>> +#define TWL4030_MADC_CTRL_SW2 0x13
>> +
>> +#define TWL4030_MADC_RTCH0_LSB 0x17
>> +#define TWL4030_MADC_GPCH0_LSB 0x37
>> +
>> +#define TWL4030_MADC_MADCON (1 << 0) /* MADC power on
> */
>> +#define TWL4030_MADC_BUSY (1 << 0) /* MADC busy */
>> +#define TWL4030_MADC_EOC_SW (1 << 1) /* MADC
> conversion completion */
>> +#define TWL4030_MADC_SW_START (1 << 5) /* MADC SWx start
> conversion */
>> +
> Formatting looks off here, and I am sure lines are > 80 columns.
>
Yes formatting is off. I will format it.
>> +#define TWL4030_MADC_ADCIN0 (1 << 0)
>> +#define TWL4030_MADC_ADCIN1 (1 << 1)
>> +#define TWL4030_MADC_ADCIN2 (1 << 2)
>> +#define TWL4030_MADC_ADCIN3 (1 << 3)
>> +#define TWL4030_MADC_ADCIN4 (1 << 4)
>> +#define TWL4030_MADC_ADCIN5 (1 << 5)
>> +#define TWL4030_MADC_ADCIN6 (1 << 6)
>> +#define TWL4030_MADC_ADCIN7 (1 << 7)
>> +#define TWL4030_MADC_ADCIN8 (1 << 8)
>> +#define TWL4030_MADC_ADCIN9 (1 << 9)
>> +#define TWL4030_MADC_ADCIN10 (1 << 10)
>> +#define TWL4030_MADC_ADCIN11 (1 << 11)
>> +#define TWL4030_MADC_ADCIN12 (1 << 12)
>> +#define TWL4030_MADC_ADCIN13 (1 << 13)
>> +#define TWL4030_MADC_ADCIN14 (1 << 14)
>> +#define TWL4030_MADC_ADCIN15 (1 << 15)
>> +
> Please no tabs after #define.
>
Ok i will remove the tabs.
>> +/* Fixed channels */
>> +#define TWL4030_MADC_BTEMP TWL4030_MADC_ADCIN1
>> +#define TWL4030_MADC_VBUS TWL4030_MADC_ADCIN8
>> +#define TWL4030_MADC_VBKB TWL4030_MADC_ADCIN9
>> +#define TWL4030_MADC_ICHG TWL4030_MADC_ADCIN10
>> +#define TWL4030_MADC_VCHG TWL4030_MADC_ADCIN11
>> +#define TWL4030_MADC_VBAT TWL4030_MADC_ADCIN12
>> +
>> +#define TWL4030_BCI_BCICTL1 0x23
>> +#define TWL4030_BCI_MESBAT (1 << 1)
>> +#define TWL4030_BCI_TYPEN (1 << 4)
>> +#define TWL4030_BCI_ITHEN (1 << 3)
>> +
>> +#define TWL4030_MADC_IOC_MAGIC '`'
>> +#define TWL4030_MADC_IOCX_ADC_RAW_READ
> _IO(TWL4030_MADC_IOC_MAGIC, 0)
>> +
>> +struct twl4030_madc_user_parms {
>> + int channel;
>> + int average;
>> + int status;
>> + u16 result;
>> +};
>> +
>> +int twl4030_madc_conversion(struct twl4030_madc_request *conv);
>> +
>> +#endif
>> --
>> 1.7.0.4
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [lm-sensors] [PATCH V2] hwmon: twl4030: Driver for twl4030 madc
@ 2010-11-26 6:44 ` J, KEERTHY
0 siblings, 0 replies; 10+ messages in thread
From: J, KEERTHY @ 2010-11-26 6:44 UTC (permalink / raw)
To: guenter.roeck
Cc: lm-sensors, sameo, khali, mikko.k.ylinen, Balbi, Felipe,
amit.kucheria, linux-omap, Krishnamoorthy, Balaji T
On Thu, Nov 11, 2010 at 5:31 AM, Guenter Roeck
<guenter.roeck@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-keerthy@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 0000000..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-keerthy@ti.com>
>> + *
>> + * Based on twl4030-madc.c
>> + * Copyright (C) 2008 Nokia Corporation
>> + * Mikko Ylinen <mikko.k.ylinen@nokia.com>
>> + *
>> + * Amit Kucheria <amit.kucheria@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;
>
>
> if (unlikely(val < 0))
> return val;
>
> return sprintf(buf, "%ld\n", req.rbuf[attr->index]);
>
> should do and would be simpler. status is not needed at all.
>
Ok i will remove.
>> +}
>> +
>> +static
>> +const struct twl4030_madc_conversion_method
> twl4030_conversion_methods[] = {
>> + [TWL4030_MADC_RT] = {
>> + .sel = TWL4030_MADC_RTSELECT_LSB,
>> + .avg = TWL4030_MADC_RTAVERAGE_LSB,
>> + .rbase = TWL4030_MADC_RTCH0_LSB,
>> + },
>> + [TWL4030_MADC_SW1] = {
>> + .sel = TWL4030_MADC_SW1SELECT_LSB,
>> + .avg = TWL4030_MADC_SW1AVERAGE_LSB,
>> + .rbase = TWL4030_MADC_GPCH0_LSB,
>> + .ctrl = TWL4030_MADC_CTRL_SW1,
>> + },
>> + [TWL4030_MADC_SW2] = {
>> + .sel = TWL4030_MADC_SW2SELECT_LSB,
>> + .avg = TWL4030_MADC_SW2AVERAGE_LSB,
>> + .rbase = TWL4030_MADC_GPCH0_LSB,
>> + .ctrl = TWL4030_MADC_CTRL_SW2,
>> + },
>> +};
>> +
>> +static int twl4030_madc_read(struct twl4030_madc_data *madc, u8 reg)
>> +{
>> + int ret;
>> + u8 val;
>> +
>> + ret = twl_i2c_read_u8(TWL4030_MODULE_MADC, &val, reg);
>> + if (ret) {
>> + dev_dbg(madc->hwmon_dev, "unable to read register
> 0x%X\n", reg);
>> + return ret;
>> + }
>> +
>> + return val;
>> +}
>> +
>> +static void twl4030_madc_write(struct twl4030_madc_data *madc, u8 reg,
> u8 val)
>> +{
>> + int ret;
>> +
>> + ret = twl_i2c_write_u8(TWL4030_MODULE_MADC, val, reg);
>> + if (ret)
>> + dev_err(madc->hwmon_dev,
>> + "unable to write register 0x%X\n", reg);
>> +}
>
> Looking into other uses of twl_i2c_write_u8(), most check and return
> errors.
> Not sure if it is a good idea to ignore all errors as it is done here.
> There
> should at least some comment explaining that and why it is done.
>
Ok. Error check will be in place.
>> +
>> +static int twl4030_madc_channel_raw_read(struct twl4030_madc_data
> *madc, u8 reg)
>> +{
>> + u8 msb, lsb;
>> +
>> + /*
>> + * For each ADC channel, we have MSB and LSB register pair. MSB
> address
>> + * is always LSB address+1. reg parameter is the addr of LSB
> register
>> + */
>> + msb = twl4030_madc_read(madc, reg + 1);
>> + lsb = twl4030_madc_read(madc, reg);
>> +
> Ignoring returned errors and combining them into a random value doesn't
> make much sense.
Return checks will be added.
>
>> + return (int)(((msb << 8) | lsb) >> 6);
>> +}
>> +
>> +static int twl4030_madc_read_channels(struct twl4030_madc_data *madc,
>> + u8 reg_base, u16 channels, int
> *buf)
>> +{
>> + int count = 0;
>> + u8 reg, i;
>> +
>> + if (unlikely(!buf))
>> + return 0;
>> +
> I don't see how this can ever be NULL.
Ok i will remove it.
>
>> + for (i = 0; i < TWL4030_MADC_MAX_CHANNELS; i++) {
>> + if (channels & (1 << i)) {
>> + reg = reg_base + 2 * i;
>> + buf[i] = twl4030_madc_channel_raw_read(madc,
> reg);
>
> Content of buf will be more or less random after errors.
Ok i will add a return check for every iteration.
>
>> + count++;
>> + }
>> + }
>> + return count;
>> +}
>> +
>> +static void twl4030_madc_enable_irq(struct twl4030_madc_data *madc, int
> id)
>> +{
>> + u8 val;
>> +
>> + val = twl4030_madc_read(madc, madc->imr);
>
> What if twl4030_madc_read() returned an error ? Try to write a random
> value ?
>
>> + val &= ~(1 << id);
>> + twl4030_madc_write(madc, madc->imr, val);
>> +}
>> +
>> +static void twl4030_madc_disable_irq(struct twl4030_madc_data *madc,
> int id)
>> +{
>> + u8 val;
>> +
>> + val = twl4030_madc_read(madc, madc->imr);
>
> Returned error ignored.
Return checks will be added for read followed by writes.
>
>> + val |= (1 << id);
>> + twl4030_madc_write(madc, madc->imr, val);
>> +}
>> +
>> +static irqreturn_t twl4030_madc_irq_handler(int irq, void *_madc)
>> +{
>> + struct twl4030_madc_data *madc = _madc;
>> + const struct twl4030_madc_conversion_method *method;
>> + u8 isr_val, imr_val;
>> + int i, len;
>> + struct twl4030_madc_request *r;
>> +
>> + isr_val = twl4030_madc_read(madc, madc->isr);
>> + imr_val = twl4030_madc_read(madc, madc->imr);
>> +
> Returned errors ignored.
I will add a check.
>
>> + isr_val &= ~imr_val;
>> +
>> + for (i = 0; i < TWL4030_MADC_NUM_METHODS; i++) {
>> +
>> + if (!(isr_val & (1 << i)))
>> + continue;
>> +
>> + twl4030_madc_disable_irq(madc, i);
>> + madc->requests[i].result_pending = 1;
>> + }
>> + mutex_lock(&madc->lock);
>> + for (i = 0; i < TWL4030_MADC_NUM_METHODS; i++) {
>> +
>> + r = &madc->requests[i];
>> +
>> + /* No pending results for this method, move to next one
> */
>> + if (!r->result_pending)
>> + continue;
>> +
>> + method = &twl4030_conversion_methods[r->method];
>> +
>> + /* Read results */
>> + len = twl4030_madc_read_channels(madc, method->rbase,
>> + r->channels, r->rbuf);
>> + /* Return results to caller */
>> + if (r->func_cb != NULL) {
>> + r->func_cb(len, r->channels, r->rbuf);
>> + r->func_cb = NULL;
>> + }
>> +
>> + /* Free request */
>> + r->result_pending = 0;
>> + r->active = 0;
>> + }
>> +
>> + mutex_unlock(&madc->lock);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static int twl4030_madc_set_irq(struct twl4030_madc_data *madc,
>> + struct twl4030_madc_request *req)
>> +{
>> + struct twl4030_madc_request *p;
>> +
>> + p = &madc->requests[req->method];
>> +
>> + memcpy(p, req, sizeof *req);
>> +
>> + twl4030_madc_enable_irq(madc, req->method);
>> +
>> + return 0;
>> +}
>> +
>> +static inline void twl4030_madc_start_conversion(struct
> twl4030_madc_data *madc,
>> + int conv_method)
>> +{
>> + const struct twl4030_madc_conversion_method *method;
>> +
>> + method = &twl4030_conversion_methods[conv_method];
>> +
>> + switch (conv_method) {
>> + case TWL4030_MADC_SW1:
>> + case TWL4030_MADC_SW2:
>> + twl4030_madc_write(madc, method->ctrl,
> TWL4030_MADC_SW_START);
>> + break;
>> + case TWL4030_MADC_RT:
>> + default:
>> + break;
>> + }
>> +}
>> +
>> +static int twl4030_madc_wait_conversion_ready(struct twl4030_madc_data
> *madc,
>> + unsigned int timeout_ms,
>> + u8 status_reg)
>> +{
>> + unsigned long timeout;
>> +
>> + timeout = jiffies + msecs_to_jiffies(timeout_ms);
>> + do {
>> + u8 reg;
>> +
>> + reg = twl4030_madc_read(madc, status_reg);
>
> Returned error ignored.
Return check will be addded.
>
>> + if (!(reg & TWL4030_MADC_BUSY) && (reg &
> TWL4030_MADC_EOC_SW))
>> + return 0;
>> + } while (!time_after(jiffies, timeout));
>> +
>> + return -EAGAIN;
>> +}
>> +
>> +int twl4030_madc_conversion(struct twl4030_madc_request *req)
>> +{
>> + const struct twl4030_madc_conversion_method *method;
>> + u8 ch_msb, ch_lsb;
>> + int ret;
>> +
>> + if (unlikely(!req))
>> + return -EINVAL;
>> +
>> + mutex_lock(&the_madc->lock);
>> +
>> + if (req->method < TWL4030_MADC_RT || req->method >
> TWL4030_MADC_SW2) {
>> + ret = -EINVAL;
>> + goto out;
>> + }
>> +
>> + /* Do we have a conversion request ongoing */
>> + if (the_madc->requests[req->method].active) {
>> + ret = -EBUSY;
>> + goto out;
>> + }
>> +
>> + ch_msb = (req->channels >> 8) & 0xff;
>> + ch_lsb = req->channels & 0xff;
>> +
>> + method = &twl4030_conversion_methods[req->method];
>> +
>> + /* Select channels to be converted */
>> + twl4030_madc_write(the_madc, method->sel + 1, ch_msb);
>> + twl4030_madc_write(the_madc, method->sel, ch_lsb);
>> +
>> + /* Select averaging for all channels if do_avg is set */
>> + if (req->do_avg) {
>> + twl4030_madc_write(the_madc, method->avg + 1, ch_msb);
>> + twl4030_madc_write(the_madc, method->avg, ch_lsb);
>> + }
>> +
>> + if (req->type = TWL4030_MADC_IRQ_ONESHOT && req->func_cb !> NULL) {
>> + twl4030_madc_set_irq(the_madc, req);
>> + twl4030_madc_start_conversion(the_madc, req->method);
>> + the_madc->requests[req->method].active = 1;
>> + ret = 0;
>> + goto out;
>> + }
>> +
>> + /* With RT method we should not be here anymore */
>> + if (req->method = TWL4030_MADC_RT) {
>> + ret = -EINVAL;
>> + goto out;
>> + }
>> +
>> + twl4030_madc_start_conversion(the_madc, req->method);
>> + the_madc->requests[req->method].active = 1;
>> +
>> + /* Wait until conversion is ready (ctrl register returns EOC) */
>> + ret = twl4030_madc_wait_conversion_ready(the_madc, 5,
> method->ctrl);
>> + if (ret) {
>> + dev_dbg(the_madc->hwmon_dev, "conversion timeout!\n");
>> + the_madc->requests[req->method].active = 0;
>> + goto out;
>> + }
>> +
>> + ret = twl4030_madc_read_channels(the_madc, method->rbase,
> req->channels,
>> + req->rbuf);
>> +
>> + the_madc->requests[req->method].active = 0;
>> +
>> +out:
>> + mutex_unlock(&the_madc->lock);
>> +
>> + return ret;
>> +}
>> +EXPORT_SYMBOL(twl4030_madc_conversion);
>> +
>> +/*
>> + * Return channel value
>> + * Or < 0 on failure.
>> + */
>> +static int twl4030_get_madc_conversion(int channel_no)
>> +{
>> + struct twl4030_madc_request req;
>> + int temp = 0;
>> + int ret;
>> +
>> + req.channels = (1 << channel_no);
>> + req.method = TWL4030_MADC_SW2;
>> + req.active = 0;
>> + req.func_cb = NULL;
>> + ret = twl4030_madc_conversion(&req);
>> + if (ret < 0)
>> + return ret;
>> +
>> + if (req.rbuf[channel_no] > 0)
>> + temp = req.rbuf[channel_no];
>> +
>> + return temp;
>> +}
>> +EXPORT_SYMBOL(twl4030_get_madc_conversion);
>> +
>
> EXPORT_SYMBOL and static declaration is a contradiction.
> Actually, this function is not used anywhere. Does this compile without
> warning ?
I will add the function declaration in a header file.
>
>> +static void twl4030_madc_set_current_generator(struct twl4030_madc_data
> *madc,
>> + int chan, int on)
>> +{
>> + int ret;
>> + u8 regval;
>> +
>> + ret = twl_i2c_read_u8(TWL4030_MODULE_MAIN_CHARGE,
>> + ®val, TWL4030_BCI_BCICTL1);
>> + if (ret) {
>> + dev_dbg(madc->hwmon_dev,
>> + "unable to read register 0x%X\n",
> TWL4030_BCI_BCICTL1);
>> + }
> You detect the error ... then go ahead and use the random regval anyway.
>
I will add a return value check.
>> + if (on)
>> + regval |= chan ? TWL4030_BCI_ITHEN : TWL4030_BCI_TYPEN;
>> + else
>> + regval &= chan ? ~TWL4030_BCI_ITHEN :
> ~TWL4030_BCI_TYPEN;
>> + ret = twl_i2c_write_u8(TWL4030_MODULE_MAIN_CHARGE,
>> + regval, TWL4030_BCI_BCICTL1);
>> + if (ret)
>> + dev_err(madc->hwmon_dev,
>> + "unable to write register 0x%X\n",
> TWL4030_BCI_BCICTL1);
>> +
> Unnecessary blank line.
I will reomve it.
>
>> +}
>> +
>> +static void twl4030_madc_set_power(struct twl4030_madc_data *madc, int
> on)
>> +{
>> + u8 regval;
>> +
>> + regval = twl4030_madc_read(madc, TWL4030_MADC_CTRL1);
>
> Returned error ignored.
Return check will be added.
>
>> + if (on)
>> + regval |= TWL4030_MADC_MADCON;
>> + else
>> + regval &= ~TWL4030_MADC_MADCON;
>> + twl4030_madc_write(madc, TWL4030_MADC_CTRL1, regval);
>> +}
>> +
>> +static SENSOR_DEVICE_ATTR(in0_input, S_IRUGO, madc_read, NULL, 0);
>> +static SENSOR_DEVICE_ATTR(in1_input, S_IRUGO, madc_read, NULL, 1);
>> +static SENSOR_DEVICE_ATTR(in2_input, S_IRUGO, madc_read, NULL, 2);
>> +static SENSOR_DEVICE_ATTR(in3_input, S_IRUGO, madc_read, NULL, 3);
>> +static SENSOR_DEVICE_ATTR(in4_input, S_IRUGO, madc_read, NULL, 4);
>> +static SENSOR_DEVICE_ATTR(in5_input, S_IRUGO, madc_read, NULL, 5);
>> +static SENSOR_DEVICE_ATTR(in6_input, S_IRUGO, madc_read, NULL, 6);
>> +static SENSOR_DEVICE_ATTR(in7_input, S_IRUGO, madc_read, NULL, 7);
>> +static SENSOR_DEVICE_ATTR(in8_input, S_IRUGO, madc_read, NULL, 8);
>> +static SENSOR_DEVICE_ATTR(in9_input, S_IRUGO, madc_read, NULL, 9);
>> +static SENSOR_DEVICE_ATTR(in10_input, S_IRUGO, madc_read, NULL, 10);
>> +static SENSOR_DEVICE_ATTR(in11_input, S_IRUGO, madc_read, NULL, 11);
>> +static SENSOR_DEVICE_ATTR(in12_input, S_IRUGO, madc_read, NULL, 12);
>> +static SENSOR_DEVICE_ATTR(in13_input, S_IRUGO, madc_read, NULL, 13);
>> +static SENSOR_DEVICE_ATTR(in14_input, S_IRUGO, madc_read, NULL, 14);
>> +static SENSOR_DEVICE_ATTR(in15_input, S_IRUGO, madc_read, NULL, 15);
>> +
>> +static struct attribute *twl4030_madc_attributes[] = {
>> + &sensor_dev_attr_in0_input.dev_attr.attr,
>> + &sensor_dev_attr_in1_input.dev_attr.attr,
>> + &sensor_dev_attr_in2_input.dev_attr.attr,
>> + &sensor_dev_attr_in3_input.dev_attr.attr,
>> + &sensor_dev_attr_in4_input.dev_attr.attr,
>> + &sensor_dev_attr_in5_input.dev_attr.attr,
>> + &sensor_dev_attr_in6_input.dev_attr.attr,
>> + &sensor_dev_attr_in7_input.dev_attr.attr,
>> + &sensor_dev_attr_in8_input.dev_attr.attr,
>> + &sensor_dev_attr_in9_input.dev_attr.attr,
>> + &sensor_dev_attr_in10_input.dev_attr.attr,
>> + &sensor_dev_attr_in11_input.dev_attr.attr,
>> + &sensor_dev_attr_in12_input.dev_attr.attr,
>> + &sensor_dev_attr_in13_input.dev_attr.attr,
>> + &sensor_dev_attr_in14_input.dev_attr.attr,
>> + &sensor_dev_attr_in15_input.dev_attr.attr,
>> + NULL
>> +};
>> +
>> +static const struct attribute_group twl4030_madc_group = {
>> + .attrs = twl4030_madc_attributes,
>> +};
>> +
>> +static int __devinit twl4030_madc_probe(struct platform_device *pdev)
>> +{
>> + struct twl4030_madc_data *madc;
>> + struct twl4030_madc_platform_data *pdata > pdev->dev.platform_data;
>> + int ret;
>> + int status;
>> + u8 regval;
>> +
>> + madc = kzalloc(sizeof *madc, GFP_KERNEL);
>> + if (!madc)
>> + return -ENOMEM;
>> +
>> + if (!pdata) {
>> + dev_dbg(&pdev->dev, "platform_data not available\n");
>> + ret = -EINVAL;
>> + goto err_misc;
>> + }
> You can check that before allocating memory.
Ok i will do that.
>
>> + madc->imr = (pdata->irq_line = 1) ?
>> + TWL4030_MADC_IMR1 : TWL4030_MADC_IMR2;
>> + madc->isr = (pdata->irq_line = 1) ?
>> + TWL4030_MADC_ISR1 : TWL4030_MADC_ISR2;
>> + twl4030_madc_set_power(madc, 1);
>> +
>> + twl4030_madc_set_current_generator(madc, 0, 1);
>> +
>> + ret = twl_i2c_read_u8(TWL4030_MODULE_MAIN_CHARGE,
>> + ®val, TWL4030_BCI_BCICTL1);
>> + if (ret) {
>> + dev_err(&pdev->dev,
>> + "unable to read register 0x%X\n",
> TWL4030_BCI_BCICTL1);
>
> No action ?
Ok. If read fails subsequent write will not make sense i will return an error.
>
>> + }
>> + regval |= TWL4030_BCI_MESBAT;
>> +
> If the read failed above, you are going to write random data.
> Not sure if that makes sense.
Ok i will return an error.
>
>> + ret = twl_i2c_write_u8(TWL4030_MODULE_MAIN_CHARGE,
>> + regval, TWL4030_BCI_BCICTL1);
>> + if (ret) {
>> + dev_err(&pdev->dev,
>> + "unable to write register 0x%X\n",
> TWL4030_BCI_BCICTL1);
>
> No action ?
>
> It might make sense to bail out if register read/writes fail.
Ok i will return error if it fails.
>
>> + }
>> +
>> + platform_set_drvdata(pdev, madc);
>> + mutex_init(&madc->lock);
>> +
>> + ret = request_threaded_irq(platform_get_irq(pdev, 0), NULL,
>> + twl4030_madc_irq_handler,
>> + IRQF_TRIGGER_RISING, "twl4030_madc",
> madc);
>> + if (ret) {
>> + dev_dbg(&pdev->dev, "could not request irq\n");
>> + goto err_irq;
>
> Jump to release irq for which the request failed.
Ok i will revert only the previously succeeded steps.
>
>> + }
>> +
>> + ret = sysfs_create_group(&pdev->dev.kobj, &twl4030_madc_group);
>> + if (ret)
>> + goto err_sysfs;
>
> Jump to remove group which wasn't added.
Ok i will revert only the previously succeeded steps.
>
>> +
>> + madc->hwmon_dev = hwmon_device_register(&pdev->dev);
>> + if (IS_ERR(madc->hwmon_dev)) {
>> + dev_err(&pdev->dev, "hwmon_device_register failed.\n");
>> + status = PTR_ERR(madc->hwmon_dev);
>> + goto err_reg;
>
> This jumps to err_reg where you try to unregister the device for which
> registration failed.
>
Ok i will revert only the previously succeeded steps.
>> + }
>> +
>> + the_madc = madc;
>> + return 0;
>> +
>> +err_reg:
>> + hwmon_device_unregister(&pdev->dev);
>> +err_sysfs:
>> + sysfs_remove_group(&pdev->dev.kobj, &twl4030_madc_group);
>> +err_irq:
>> + free_irq(platform_get_irq(pdev, 0), madc);
>> + platform_set_drvdata(pdev, NULL);
>> + twl4030_madc_set_power(madc, 0);
>> + twl4030_madc_set_current_generator(madc, 0, 0);
>> +err_misc:
>> + kfree(madc);
>> +
>> + return ret;
>> +}
>> +
>> +static int __devexit twl4030_madc_remove(struct platform_device *pdev)
>> +{
>> + struct twl4030_madc_data *madc = platform_get_drvdata(pdev);
>> +
>> + hwmon_device_unregister(&pdev->dev);
>> + sysfs_remove_group(&pdev->dev.kobj, &twl4030_madc_group);
>> + free_irq(platform_get_irq(pdev, 0), madc);
>> + platform_set_drvdata(pdev, NULL);
>> + twl4030_madc_set_current_generator(madc, 0, 0);
>> + twl4030_madc_set_power(madc, 0);
>> + kfree(madc);
>> +
>> + return 0;
>> +}
>> +
>> +static struct platform_driver twl4030_madc_driver = {
>> + .probe = twl4030_madc_probe,
>> + .remove = __exit_p(twl4030_madc_remove),
>> + .driver = {
>> + .name = "twl4030_madc",
>> + .owner = THIS_MODULE,
>> + },
>> +};
>> +
>> +static int __init twl4030_madc_init(void)
>> +{
>> + return platform_driver_register(&twl4030_madc_driver);
>> +}
>> +
>> +module_init(twl4030_madc_init);
>> +
>> +static void __exit twl4030_madc_exit(void)
>> +{
>> + platform_driver_unregister(&twl4030_madc_driver);
>> +}
>> +
>> +module_exit(twl4030_madc_exit);
>> +
>> +MODULE_DESCRIPTION("TWL4030 ADC driver");
>> +MODULE_LICENSE("GPL");
>> +MODULE_AUTHOR("J Keerthy");
>> diff --git a/include/linux/i2c/twl4030-madc.h
> b/include/linux/i2c/twl4030-madc.h
>> new file mode 100644
>> index 0000000..0e4e227
>> --- /dev/null
>> +++ b/include/linux/i2c/twl4030-madc.h
>> @@ -0,0 +1,118 @@
>> +/*
>> + * 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
>> + *
>> + */
>> +
>> +#ifndef _TWL4030_MADC_H
>> +#define _TWL4030_MADC_H
>> +
>> +struct twl4030_madc_conversion_method {
>> + u8 sel;
>> + u8 avg;
>> + u8 rbase;
>> + u8 ctrl;
>> +};
>> +
>> +#define TWL4030_MADC_MAX_CHANNELS 16
>> +
>> +struct twl4030_madc_request {
>> + u16 channels;
>> + u16 do_avg;
>> + u16 method;
>> + u16 type;
>> + bool active;
>> + bool result_pending;
>> + int rbuf[TWL4030_MADC_MAX_CHANNELS];
>> + void (*func_cb)(int len, int channels, int *buf);
>> +};
>> +
>> +enum conversion_methods {
>> + TWL4030_MADC_RT,
>> + TWL4030_MADC_SW1,
>> + TWL4030_MADC_SW2,
>> + TWL4030_MADC_NUM_METHODS
>> +};
>> +
>> +enum sample_type {
>> + TWL4030_MADC_WAIT,
>> + TWL4030_MADC_IRQ_ONESHOT,
>> + TWL4030_MADC_IRQ_REARM
>> +};
>> +
>> +#define TWL4030_MADC_CTRL1 0x00
>> +#define TWL4030_MADC_CTRL2 0x01
>> +
>> +#define TWL4030_MADC_RTSELECT_LSB 0x02
>> +#define TWL4030_MADC_SW1SELECT_LSB 0x06
>> +#define TWL4030_MADC_SW2SELECT_LSB 0x0A
>> +
>> +#define TWL4030_MADC_RTAVERAGE_LSB 0x04
>> +#define TWL4030_MADC_SW1AVERAGE_LSB 0x08
>> +#define TWL4030_MADC_SW2AVERAGE_LSB 0x0C
>> +
>> +#define TWL4030_MADC_CTRL_SW1 0x12
>> +#define TWL4030_MADC_CTRL_SW2 0x13
>> +
>> +#define TWL4030_MADC_RTCH0_LSB 0x17
>> +#define TWL4030_MADC_GPCH0_LSB 0x37
>> +
>> +#define TWL4030_MADC_MADCON (1 << 0) /* MADC power on
> */
>> +#define TWL4030_MADC_BUSY (1 << 0) /* MADC busy */
>> +#define TWL4030_MADC_EOC_SW (1 << 1) /* MADC
> conversion completion */
>> +#define TWL4030_MADC_SW_START (1 << 5) /* MADC SWx start
> conversion */
>> +
> Formatting looks off here, and I am sure lines are > 80 columns.
>
Yes formatting is off. I will format it.
>> +#define TWL4030_MADC_ADCIN0 (1 << 0)
>> +#define TWL4030_MADC_ADCIN1 (1 << 1)
>> +#define TWL4030_MADC_ADCIN2 (1 << 2)
>> +#define TWL4030_MADC_ADCIN3 (1 << 3)
>> +#define TWL4030_MADC_ADCIN4 (1 << 4)
>> +#define TWL4030_MADC_ADCIN5 (1 << 5)
>> +#define TWL4030_MADC_ADCIN6 (1 << 6)
>> +#define TWL4030_MADC_ADCIN7 (1 << 7)
>> +#define TWL4030_MADC_ADCIN8 (1 << 8)
>> +#define TWL4030_MADC_ADCIN9 (1 << 9)
>> +#define TWL4030_MADC_ADCIN10 (1 << 10)
>> +#define TWL4030_MADC_ADCIN11 (1 << 11)
>> +#define TWL4030_MADC_ADCIN12 (1 << 12)
>> +#define TWL4030_MADC_ADCIN13 (1 << 13)
>> +#define TWL4030_MADC_ADCIN14 (1 << 14)
>> +#define TWL4030_MADC_ADCIN15 (1 << 15)
>> +
> Please no tabs after #define.
>
Ok i will remove the tabs.
>> +/* Fixed channels */
>> +#define TWL4030_MADC_BTEMP TWL4030_MADC_ADCIN1
>> +#define TWL4030_MADC_VBUS TWL4030_MADC_ADCIN8
>> +#define TWL4030_MADC_VBKB TWL4030_MADC_ADCIN9
>> +#define TWL4030_MADC_ICHG TWL4030_MADC_ADCIN10
>> +#define TWL4030_MADC_VCHG TWL4030_MADC_ADCIN11
>> +#define TWL4030_MADC_VBAT TWL4030_MADC_ADCIN12
>> +
>> +#define TWL4030_BCI_BCICTL1 0x23
>> +#define TWL4030_BCI_MESBAT (1 << 1)
>> +#define TWL4030_BCI_TYPEN (1 << 4)
>> +#define TWL4030_BCI_ITHEN (1 << 3)
>> +
>> +#define TWL4030_MADC_IOC_MAGIC '`'
>> +#define TWL4030_MADC_IOCX_ADC_RAW_READ
> _IO(TWL4030_MADC_IOC_MAGIC, 0)
>> +
>> +struct twl4030_madc_user_parms {
>> + int channel;
>> + int average;
>> + int status;
>> + u16 result;
>> +};
>> +
>> +int twl4030_madc_conversion(struct twl4030_madc_request *conv);
>> +
>> +#endif
>> --
>> 1.7.0.4
>>
>
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2010-11-26 6:44 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-11-09 9:20 [PATCH V2] hwmon: twl4030: Driver for twl4030 madc module Keerthy
2010-11-09 9:32 ` [lm-sensors] [PATCH V2] hwmon: twl4030: Driver for twl4030 madc Keerthy
2010-11-09 13:40 ` [PATCH V2] hwmon: twl4030: Driver for twl4030 madc module Anand Gadiyar
2010-11-09 13:52 ` [lm-sensors] [PATCH V2] hwmon: twl4030: Driver for twl4030 madc Anand Gadiyar
2010-11-09 13:46 ` [PATCH V2] hwmon: twl4030: Driver for twl4030 madc module J, KEERTHY
2010-11-09 13:58 ` [lm-sensors] [PATCH V2] hwmon: twl4030: Driver for twl4030 madc J, KEERTHY
2010-11-11 0:01 ` [PATCH V2] hwmon: twl4030: Driver for twl4030 madc module Guenter Roeck
2010-11-11 0:01 ` [lm-sensors] [PATCH V2] hwmon: twl4030: Driver for twl4030 madc Guenter Roeck
[not found] ` <AANLkTiniRxNoNn9bSR+PN8B=m6=YVWS8Na_vJUdi1QhT@mail.gmail.com>
2010-11-26 6:32 ` [PATCH V2] hwmon: twl4030: Driver for twl4030 madc module J, KEERTHY
2010-11-26 6:44 ` [lm-sensors] [PATCH V2] hwmon: twl4030: Driver for twl4030 madc J, KEERTHY
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.