[PATCH 2/2] hwmon: twl4030: Hwmon Driver for TWL4030 MADC

[PATCH 2/2] hwmon: twl4030: Hwmon Driver for TWL4030 MADC

[Keerthy]

This driver exposes the sysfs nodes of the TWL4030 MADC module.
All the channel values are expressed in terms of mV. Channel 13
and channel 14 are reserved. There are channels which represent
temperature and current. Even they output raw voltage in mV.

Signed-off-by: Keerthy <j-keerthy@ti.com>
---

The previous discussion concluded in keeping the generic ADC
functionality and the hwmon separate. The discussion can be found here:

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

 Documentation/hwmon/twl4030-madc-hwmon |   45 +++++++++
 drivers/hwmon/Kconfig                  |   10 ++
 drivers/hwmon/Makefile                 |    1 +
 drivers/hwmon/twl4030-madc-hwmon.c     |  155 ++++++++++++++++++++++++++++++++
 4 files changed, 211 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/hwmon/twl4030-madc-hwmon
 create mode 100644 drivers/hwmon/twl4030-madc-hwmon.c

diff --git a/Documentation/hwmon/twl4030-madc-hwmon b/Documentation/hwmon/twl4030-madc-hwmon
new file mode 100644
index 0000000..2cf1cc2
--- /dev/null
+++ b/Documentation/hwmon/twl4030-madc-hwmon
@@ -0,0 +1,45 @@
+Kernel driver twl4030-madc
+=========================
+
+Supported chips:
+	* Texas Instruments TWL4030
+	Prefix: 'twl4030-madc'
+
+
+Authors:
+	J Keerthy <j-keerthy@ti.com>
+
+Description
+-----------
+
+The Texas Instruments TWL4030 is a Power Management and Audio Circuit. Among
+other things it contains a 10-bit A/D converter MADC. The converter has 16
+channels which can be used in different modes.
+
+
+See this table for the meaning of the different channels
+
+Channel Signal
+------------------------------------------
+0	Battery type(BTYPE)
+1	BCI: Battery temperature (BTEMP)
+2	GP analog input
+3	GP analog input
+4	GP analog input
+5	GP analog input
+6	GP analog input
+7	GP analog input
+8	BCI: VBUS voltage(VBUS)
+9	Backup Battery voltage (VBKP)
+10	BCI: Battery charger current (ICHG)
+11	BCI: Battery charger voltage (VCHG)
+12	BCI: Main battery voltage (VBAT)
+13	Reserved
+14	Reserved
+15	VRUSB Supply/Speaker left/Speaker right polarization level
+
+
+The Sysfs nodes will represent the voltage in the units of mV,
+the temperature channel shows the converted raw voltage in mV.
+The Battery charging current channel represents raw voltage mV.
+Channel 13 and 14 are reserved.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 773e484..11ddde3 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -940,6 +940,16 @@ config SENSORS_TMP421
 	  This driver can also be built as a module.  If so, the module
 	  will be called tmp421.
 
+config SENSORS_TWL4030_MADC
+	tristate "Texas Instruments TWL4030 MADC Hwmon"
+	depends on TWL4030_MADC
+	help
+	This driver provides hwmon support for triton TWL4030-MADC.
+	This driver can be built as part of kernel or can be built
+	as a module.
+		This driver exposes the various voltage values to
+	the user space.
+
 config SENSORS_VIA_CPUTEMP
 	tristate "VIA CPU temperature sensor"
 	depends on X86
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index dde02d9..bc7d740 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -102,6 +102,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-hwmon.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-hwmon.c b/drivers/hwmon/twl4030-madc-hwmon.c
new file mode 100644
index 0000000..4a61e8a
--- /dev/null
+++ b/drivers/hwmon/twl4030-madc-hwmon.c
@@ -0,0 +1,155 @@
+/*
+ *
+ * TWL4030 MADC Hwmon driver-This driver monitors the real time
+ * conversion of analog signals like battery temperature,
+ * battery type, battery level etc. User can ask for the conversion on a
+ * particular channel using the sysfs nodes.
+ *
+ * Copyright (C) 2010 Texas Instruments Incorporated - http://www.ti.com/
+ * J Keerthy <j-keerthy@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * 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/platform_device.h>
+#include <linux/i2c/twl.h>
+#include <linux/i2c/twl4030-madc.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+
+/*
+ * sysfs hook function
+ */
+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;
+	long val;
+
+	req.channels = (1 << attr->index);
+	req.method = TWL4030_MADC_SW2;
+	req.func_cb = NULL;
+	val = twl4030_madc_conversion(&req);
+	if (val >= 0)
+		val = req.rbuf[attr->index];
+	else
+		return val;
+
+	return sprintf(buf, "%ld\n", val);
+}
+
+/* sysfs nodes to read individual channels from user side */
+static SENSOR_DEVICE_ATTR(in0_battery_type, S_IRUGO, madc_read, NULL, 0);
+static SENSOR_DEVICE_ATTR(in1_battery_temp, 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_vbus, S_IRUGO, madc_read, NULL, 8);
+static SENSOR_DEVICE_ATTR(in9_vbkp, S_IRUGO, madc_read, NULL, 9);
+static SENSOR_DEVICE_ATTR(in10_battery_charger_current,
+			S_IRUGO, madc_read, NULL, 10);
+static SENSOR_DEVICE_ATTR(in11_battery_charger_voltage,
+			S_IRUGO, madc_read, NULL, 11);
+static SENSOR_DEVICE_ATTR(in12_main_battery_voltage,
+			S_IRUGO, madc_read, NULL, 12);
+static SENSOR_DEVICE_ATTR(in15_input, S_IRUGO, madc_read, NULL, 13);
+
+static struct attribute *twl4030_madc_attributes[] = {
+	&sensor_dev_attr_in0_battery_type.dev_attr.attr,
+	&sensor_dev_attr_in1_battery_temp.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_vbus.dev_attr.attr,
+	&sensor_dev_attr_in9_vbkp.dev_attr.attr,
+	&sensor_dev_attr_in10_battery_charger_current.dev_attr.attr,
+	&sensor_dev_attr_in11_battery_charger_voltage.dev_attr.attr,
+	&sensor_dev_attr_in12_main_battery_voltage.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_hwmon_probe(struct platform_device *pdev)
+{
+	int ret;
+	int status;
+	struct device *hwmon;
+
+	ret = sysfs_create_group(&pdev->dev.kobj, &twl4030_madc_group);
+	if (ret)
+		goto err_sysfs;
+	hwmon = hwmon_device_register(&pdev->dev);
+	if (IS_ERR(hwmon)) {
+		dev_err(&pdev->dev, "hwmon_device_register failed.\n");
+		status = PTR_ERR(hwmon);
+		goto err_reg;
+	}
+
+	return 0;
+
+err_reg:
+	sysfs_remove_group(&pdev->dev.kobj, &twl4030_madc_group);
+err_sysfs:
+
+	return ret;
+}
+
+static int __devexit twl4030_madc_hwmon_remove(struct platform_device *pdev)
+{
+	hwmon_device_unregister(&pdev->dev);
+	sysfs_remove_group(&pdev->dev.kobj, &twl4030_madc_group);
+
+	return 0;
+}
+
+static struct platform_driver twl4030_madc_hwmon_driver = {
+	.probe = twl4030_madc_hwmon_probe,
+	.remove = __exit_p(twl4030_madc_hwmon_remove),
+	.driver = {
+		   .name = "twl4030_madc_hwmon",
+		   .owner = THIS_MODULE,
+		   },
+};
+
+static int __init twl4030_madc_hwmon_init(void)
+{
+	return platform_driver_register(&twl4030_madc_hwmon_driver);
+}
+
+module_init(twl4030_madc_hwmon_init);
+
+static void __exit twl4030_madc_hwmon_exit(void)
+{
+	platform_driver_unregister(&twl4030_madc_hwmon_driver);
+}
+
+module_exit(twl4030_madc_hwmon_exit);
+
+MODULE_DESCRIPTION("TWL4030 ADC Hwmon driver");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("J Keerthy");
+MODULE_ALIAS("twl4030_madc_hwmon");
-- 
1.7.0.4

[PATCH 2/2] hwmon: twl4030: Hwmon Driver for TWL4030 MADC

[Keerthy]

This driver exposes the sysfs nodes of the TWL4030 MADC module.
All the channel values are expressed in terms of mV. Channel 13
and channel 14 are reserved. There are channels which represent
temperature and current. Even they output raw voltage in mV.

Signed-off-by: Keerthy <j-keerthy@ti.com>
---

The previous discussion concluded in keeping the generic ADC
functionality and the hwmon separate. The discussion can be found here:

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

 Documentation/hwmon/twl4030-madc-hwmon |   45 +++++++++
 drivers/hwmon/Kconfig                  |   10 ++
 drivers/hwmon/Makefile                 |    1 +
 drivers/hwmon/twl4030-madc-hwmon.c     |  155 ++++++++++++++++++++++++++++++++
 4 files changed, 211 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/hwmon/twl4030-madc-hwmon
 create mode 100644 drivers/hwmon/twl4030-madc-hwmon.c

diff --git a/Documentation/hwmon/twl4030-madc-hwmon b/Documentation/hwmon/twl4030-madc-hwmon
new file mode 100644
index 0000000..2cf1cc2
--- /dev/null
+++ b/Documentation/hwmon/twl4030-madc-hwmon
@@ -0,0 +1,45 @@
+Kernel driver twl4030-madc
+=========================
+
+Supported chips:
+	* Texas Instruments TWL4030
+	Prefix: 'twl4030-madc'
+
+
+Authors:
+	J Keerthy <j-keerthy@ti.com>
+
+Description
+-----------
+
+The Texas Instruments TWL4030 is a Power Management and Audio Circuit. Among
+other things it contains a 10-bit A/D converter MADC. The converter has 16
+channels which can be used in different modes.
+
+
+See this table for the meaning of the different channels
+
+Channel Signal
+------------------------------------------
+0	Battery type(BTYPE)
+1	BCI: Battery temperature (BTEMP)
+2	GP analog input
+3	GP analog input
+4	GP analog input
+5	GP analog input
+6	GP analog input
+7	GP analog input
+8	BCI: VBUS voltage(VBUS)
+9	Backup Battery voltage (VBKP)
+10	BCI: Battery charger current (ICHG)
+11	BCI: Battery charger voltage (VCHG)
+12	BCI: Main battery voltage (VBAT)
+13	Reserved
+14	Reserved
+15	VRUSB Supply/Speaker left/Speaker right polarization level
+
+
+The Sysfs nodes will represent the voltage in the units of mV,
+the temperature channel shows the converted raw voltage in mV.
+The Battery charging current channel represents raw voltage mV.
+Channel 13 and 14 are reserved.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 773e484..11ddde3 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -940,6 +940,16 @@ config SENSORS_TMP421
 	  This driver can also be built as a module.  If so, the module
 	  will be called tmp421.
 
+config SENSORS_TWL4030_MADC
+	tristate "Texas Instruments TWL4030 MADC Hwmon"
+	depends on TWL4030_MADC
+	help
+	This driver provides hwmon support for triton TWL4030-MADC.
+	This driver can be built as part of kernel or can be built
+	as a module.
+		This driver exposes the various voltage values to
+	the user space.
+
 config SENSORS_VIA_CPUTEMP
 	tristate "VIA CPU temperature sensor"
 	depends on X86
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index dde02d9..bc7d740 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -102,6 +102,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-hwmon.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-hwmon.c b/drivers/hwmon/twl4030-madc-hwmon.c
new file mode 100644
index 0000000..4a61e8a
--- /dev/null
+++ b/drivers/hwmon/twl4030-madc-hwmon.c
@@ -0,0 +1,155 @@
+/*
+ *
+ * TWL4030 MADC Hwmon driver-This driver monitors the real time
+ * conversion of analog signals like battery temperature,
+ * battery type, battery level etc. User can ask for the conversion on a
+ * particular channel using the sysfs nodes.
+ *
+ * Copyright (C) 2010 Texas Instruments Incorporated - http://www.ti.com/
+ * J Keerthy <j-keerthy@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * 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/platform_device.h>
+#include <linux/i2c/twl.h>
+#include <linux/i2c/twl4030-madc.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+
+/*
+ * sysfs hook function
+ */
+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;
+	long val;
+
+	req.channels = (1 << attr->index);
+	req.method = TWL4030_MADC_SW2;
+	req.func_cb = NULL;
+	val = twl4030_madc_conversion(&req);
+	if (val >= 0)
+		val = req.rbuf[attr->index];
+	else
+		return val;
+
+	return sprintf(buf, "%ld\n", val);
+}
+
+/* sysfs nodes to read individual channels from user side */
+static SENSOR_DEVICE_ATTR(in0_battery_type, S_IRUGO, madc_read, NULL, 0);
+static SENSOR_DEVICE_ATTR(in1_battery_temp, 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_vbus, S_IRUGO, madc_read, NULL, 8);
+static SENSOR_DEVICE_ATTR(in9_vbkp, S_IRUGO, madc_read, NULL, 9);
+static SENSOR_DEVICE_ATTR(in10_battery_charger_current,
+			S_IRUGO, madc_read, NULL, 10);
+static SENSOR_DEVICE_ATTR(in11_battery_charger_voltage,
+			S_IRUGO, madc_read, NULL, 11);
+static SENSOR_DEVICE_ATTR(in12_main_battery_voltage,
+			S_IRUGO, madc_read, NULL, 12);
+static SENSOR_DEVICE_ATTR(in15_input, S_IRUGO, madc_read, NULL, 15);
+
+static struct attribute *twl4030_madc_attributes[] = {
+	&sensor_dev_attr_in0_battery_type.dev_attr.attr,
+	&sensor_dev_attr_in1_battery_temp.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_vbus.dev_attr.attr,
+	&sensor_dev_attr_in9_vbkp.dev_attr.attr,
+	&sensor_dev_attr_in10_battery_charger_current.dev_attr.attr,
+	&sensor_dev_attr_in11_battery_charger_voltage.dev_attr.attr,
+	&sensor_dev_attr_in12_main_battery_voltage.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_hwmon_probe(struct platform_device *pdev)
+{
+	int ret;
+	int status;
+	struct device *hwmon;
+
+	ret = sysfs_create_group(&pdev->dev.kobj, &twl4030_madc_group);
+	if (ret)
+		goto err_sysfs;
+	hwmon = hwmon_device_register(&pdev->dev);
+	if (IS_ERR(hwmon)) {
+		dev_err(&pdev->dev, "hwmon_device_register failed.\n");
+		status = PTR_ERR(hwmon);
+		goto err_reg;
+	}
+
+	return 0;
+
+err_reg:
+	sysfs_remove_group(&pdev->dev.kobj, &twl4030_madc_group);
+err_sysfs:
+
+	return ret;
+}
+
+static int __devexit twl4030_madc_hwmon_remove(struct platform_device *pdev)
+{
+	hwmon_device_unregister(&pdev->dev);
+	sysfs_remove_group(&pdev->dev.kobj, &twl4030_madc_group);
+
+	return 0;
+}
+
+static struct platform_driver twl4030_madc_hwmon_driver = {
+	.probe = twl4030_madc_hwmon_probe,
+	.remove = __exit_p(twl4030_madc_hwmon_remove),
+	.driver = {
+		   .name = "twl4030_madc_hwmon",
+		   .owner = THIS_MODULE,
+		   },
+};
+
+static int __init twl4030_madc_hwmon_init(void)
+{
+	return platform_driver_register(&twl4030_madc_hwmon_driver);
+}
+
+module_init(twl4030_madc_hwmon_init);
+
+static void __exit twl4030_madc_hwmon_exit(void)
+{
+	platform_driver_unregister(&twl4030_madc_hwmon_driver);
+}
+
+module_exit(twl4030_madc_hwmon_exit);
+
+MODULE_DESCRIPTION("TWL4030 ADC Hwmon driver");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("J Keerthy");
+MODULE_ALIAS("twl4030_madc_hwmon");
-- 
1.7.0.4

Re: [PATCH 2/2] hwmon: twl4030: Hwmon Driver for TWL4030 MADC

[Guenter Roeck]

On Wed, Feb 16, 2011 at 07:56:57AM -0500, Keerthy wrote:
> This driver exposes the sysfs nodes of the TWL4030 MADC module.
> All the channel values are expressed in terms of mV. Channel 13
> and channel 14 are reserved. There are channels which represent
> temperature and current. Even they output raw voltage in mV.
> 
Why ?

> Signed-off-by: Keerthy <j-keerthy@ti.com>
> ---
> 
> The previous discussion concluded in keeping the generic ADC
> functionality and the hwmon separate. The discussion can be found here:
> 
> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg41805.html
> 
>  Documentation/hwmon/twl4030-madc-hwmon |   45 +++++++++
>  drivers/hwmon/Kconfig                  |   10 ++
>  drivers/hwmon/Makefile                 |    1 +
>  drivers/hwmon/twl4030-madc-hwmon.c     |  155 ++++++++++++++++++++++++++++++++
>  4 files changed, 211 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/hwmon/twl4030-madc-hwmon
>  create mode 100644 drivers/hwmon/twl4030-madc-hwmon.c
> 
> diff --git a/Documentation/hwmon/twl4030-madc-hwmon b/Documentation/hwmon/twl4030-madc-hwmon
> new file mode 100644
> index 0000000..2cf1cc2
> --- /dev/null
> +++ b/Documentation/hwmon/twl4030-madc-hwmon
> @@ -0,0 +1,45 @@
> +Kernel driver twl4030-madc
> +=========================
> +
> +Supported chips:
> +	* Texas Instruments TWL4030
> +	Prefix: 'twl4030-madc'
> +
> +
> +Authors:
> +	J Keerthy <j-keerthy@ti.com>
> +
> +Description
> +-----------
> +
> +The Texas Instruments TWL4030 is a Power Management and Audio Circuit. Among
> +other things it contains a 10-bit A/D converter MADC. The converter has 16
> +channels which can be used in different modes.
> +
> +
> +See this table for the meaning of the different channels
> +
> +Channel Signal
> +------------------------------------------
> +0	Battery type(BTYPE)
> +1	BCI: Battery temperature (BTEMP)
> +2	GP analog input
> +3	GP analog input
> +4	GP analog input
> +5	GP analog input
> +6	GP analog input
> +7	GP analog input
> +8	BCI: VBUS voltage(VBUS)
> +9	Backup Battery voltage (VBKP)
> +10	BCI: Battery charger current (ICHG)
> +11	BCI: Battery charger voltage (VCHG)
> +12	BCI: Main battery voltage (VBAT)
> +13	Reserved
> +14	Reserved
> +15	VRUSB Supply/Speaker left/Speaker right polarization level
> +
> +
> +The Sysfs nodes will represent the voltage in the units of mV,
> +the temperature channel shows the converted raw voltage in mV.
> +The Battery charging current channel represents raw voltage mV.

Doesn't really make sense to me to export values known to be current and temperature
as voltages. You'll have to add a lot more information for that to make sense.

> +Channel 13 and 14 are reserved.

You already said that above.

> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 773e484..11ddde3 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -940,6 +940,16 @@ config SENSORS_TMP421
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called tmp421.
>  
> +config SENSORS_TWL4030_MADC
> +	tristate "Texas Instruments TWL4030 MADC Hwmon"
> +	depends on TWL4030_MADC
> +	help
> +	This driver provides hwmon support for triton TWL4030-MADC.
> +	This driver can be built as part of kernel or can be built
> +	as a module.

	Kernel is obvious. Please stick with the usual text

	"This driver can also be built as a module.  If so, the module
         will be called XXXX"

At least please tell users how the module is named.

> +		This driver exposes the various voltage values to
> +	the user space.
> +
Not sure if this provides any value. Either case, it should be before "built as module".

>  config SENSORS_VIA_CPUTEMP
>  	tristate "VIA CPU temperature sensor"
>  	depends on X86
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index dde02d9..bc7d740 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -102,6 +102,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-hwmon.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-hwmon.c b/drivers/hwmon/twl4030-madc-hwmon.c
> new file mode 100644
> index 0000000..4a61e8a
> --- /dev/null
> +++ b/drivers/hwmon/twl4030-madc-hwmon.c
> @@ -0,0 +1,155 @@
> +/*
> + *
> + * TWL4030 MADC Hwmon driver-This driver monitors the real time
> + * conversion of analog signals like battery temperature,
> + * battery type, battery level etc. User can ask for the conversion on a
> + * particular channel using the sysfs nodes.
> + *
> + * Copyright (C) 2010 Texas Instruments Incorporated - http://www.ti.com/

2011 ?

> + * J Keerthy <j-keerthy@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * version 2 as published by the Free Software Foundation.
> + *
> + * 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/platform_device.h>
> +#include <linux/i2c/twl.h>
> +#include <linux/i2c/twl4030-madc.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +
> +/*
> + * sysfs hook function
> + */
> +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;
> +	long val;
> +
> +	req.channels = (1 << attr->index);
> +	req.method = TWL4030_MADC_SW2;
> +	req.func_cb = NULL;
> +	val = twl4030_madc_conversion(&req);
> +	if (val >= 0)
> +		val = req.rbuf[attr->index];
> +	else
> +		return val;
> +
Seems to be a bit complicated.

	if (val < 0)
		return val;

	return sprintf(buf, "%ld\n", req.rbuf[attr->index]);

would be more straightforward.

> +	return sprintf(buf, "%ld\n", val);
> +}
> +
> +/* sysfs nodes to read individual channels from user side */
> +static SENSOR_DEVICE_ATTR(in0_battery_type, S_IRUGO, madc_read, NULL, 0);
> +static SENSOR_DEVICE_ATTR(in1_battery_temp, S_IRUGO, madc_read, NULL, 1);

I don't know why people always try to introduce new sysfs attributes
for no good reason. Use a label if you want to describe the sensor.
And use tempX_input for temperatures, and currX_input for currents.

I am not inclined to accept drivers introducing new sysfs attributes,
unless the new attribute has either been discussed and added to the ABI,
or if a _really_ good reason for the non-standard attribute is provided.
Neither is the case here. 

> +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_vbus, S_IRUGO, madc_read, NULL, 8);
> +static SENSOR_DEVICE_ATTR(in9_vbkp, S_IRUGO, madc_read, NULL, 9);
> +static SENSOR_DEVICE_ATTR(in10_battery_charger_current,
> +			S_IRUGO, madc_read, NULL, 10);
> +static SENSOR_DEVICE_ATTR(in11_battery_charger_voltage,
> +			S_IRUGO, madc_read, NULL, 11);
> +static SENSOR_DEVICE_ATTR(in12_main_battery_voltage,
> +			S_IRUGO, madc_read, NULL, 12);

... and again ...

> +static SENSOR_DEVICE_ATTR(in15_input, S_IRUGO, madc_read, NULL, 15);
> +

Number sequence doesn't have to match chip channel number.
Sure you want to leave a hole here ?

Either case, you should describe in the documentation how chip channels
match attribute names.

> +static struct attribute *twl4030_madc_attributes[] = {
> +	&sensor_dev_attr_in0_battery_type.dev_attr.attr,
> +	&sensor_dev_attr_in1_battery_temp.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_vbus.dev_attr.attr,
> +	&sensor_dev_attr_in9_vbkp.dev_attr.attr,
> +	&sensor_dev_attr_in10_battery_charger_current.dev_attr.attr,
> +	&sensor_dev_attr_in11_battery_charger_voltage.dev_attr.attr,
> +	&sensor_dev_attr_in12_main_battery_voltage.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_hwmon_probe(struct platform_device *pdev)
> +{
> +	int ret;
> +	int status;
> +	struct device *hwmon;
> +
> +	ret = sysfs_create_group(&pdev->dev.kobj, &twl4030_madc_group);
> +	if (ret)
> +		goto err_sysfs;
> +	hwmon = hwmon_device_register(&pdev->dev);
> +	if (IS_ERR(hwmon)) {
> +		dev_err(&pdev->dev, "hwmon_device_register failed.\n");
> +		status = PTR_ERR(hwmon);
> +		goto err_reg;
> +	}
> +
> +	return 0;
> +
> +err_reg:
> +	sysfs_remove_group(&pdev->dev.kobj, &twl4030_madc_group);
> +err_sysfs:
> +
> +	return ret;
> +}
> +
> +static int __devexit twl4030_madc_hwmon_remove(struct platform_device *pdev)
> +{
> +	hwmon_device_unregister(&pdev->dev);
> +	sysfs_remove_group(&pdev->dev.kobj, &twl4030_madc_group);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver twl4030_madc_hwmon_driver = {
> +	.probe = twl4030_madc_hwmon_probe,
> +	.remove = __exit_p(twl4030_madc_hwmon_remove),
> +	.driver = {
> +		   .name = "twl4030_madc_hwmon",
> +		   .owner = THIS_MODULE,
> +		   },
> +};
> +
> +static int __init twl4030_madc_hwmon_init(void)
> +{
> +	return platform_driver_register(&twl4030_madc_hwmon_driver);
> +}
> +
> +module_init(twl4030_madc_hwmon_init);
> +
> +static void __exit twl4030_madc_hwmon_exit(void)
> +{
> +	platform_driver_unregister(&twl4030_madc_hwmon_driver);
> +}
> +
> +module_exit(twl4030_madc_hwmon_exit);
> +
> +MODULE_DESCRIPTION("TWL4030 ADC Hwmon driver");
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("J Keerthy");
> +MODULE_ALIAS("twl4030_madc_hwmon");
> -- 
> 1.7.0.4
> 

Re: [PATCH 2/2] hwmon: twl4030: Hwmon Driver for TWL4030 MADC

[J, KEERTHY]

Hello Guenter,

On Wed, Feb 16, 2011 at 9:39 PM, Guenter Roeck
<guenter.roeck@ericsson.com> wrote:
> On Wed, Feb 16, 2011 at 07:56:57AM -0500, Keerthy wrote:
>> This driver exposes the sysfs nodes of the TWL4030 MADC module.
>> All the channel values are expressed in terms of mV. Channel 13
>> and channel 14 are reserved. There are channels which represent
>> temperature and current. Even they output raw voltage in mV.
>>
> Why ?

The conversion to current and temperature in case of MADC depends
on a register in the battery module. Hence battery driver can expose the
converted value. So providing the raw voltage here.
>
>> Signed-off-by: Keerthy <j-keerthy@ti.com>
>> ---
>>
>> The previous discussion concluded in keeping the generic ADC
>> functionality and the hwmon separate. The discussion can be found here:
>>
>> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg41805.html
>>
>>  Documentation/hwmon/twl4030-madc-hwmon |   45 +++++++++
>>  drivers/hwmon/Kconfig                  |   10 ++
>>  drivers/hwmon/Makefile                 |    1 +
>>  drivers/hwmon/twl4030-madc-hwmon.c     |  155 ++++++++++++++++++++++++++++++++
>>  4 files changed, 211 insertions(+), 0 deletions(-)
>>  create mode 100644 Documentation/hwmon/twl4030-madc-hwmon
>>  create mode 100644 drivers/hwmon/twl4030-madc-hwmon.c
>>
>> diff --git a/Documentation/hwmon/twl4030-madc-hwmon b/Documentation/hwmon/twl4030-madc-hwmon
>> new file mode 100644
>> index 0000000..2cf1cc2
>> --- /dev/null
>> +++ b/Documentation/hwmon/twl4030-madc-hwmon
>> @@ -0,0 +1,45 @@
>> +Kernel driver twl4030-madc
>> +=========================
>> +
>> +Supported chips:
>> +     * Texas Instruments TWL4030
>> +     Prefix: 'twl4030-madc'
>> +
>> +
>> +Authors:
>> +     J Keerthy <j-keerthy@ti.com>
>> +
>> +Description
>> +-----------
>> +
>> +The Texas Instruments TWL4030 is a Power Management and Audio Circuit. Among
>> +other things it contains a 10-bit A/D converter MADC. The converter has 16
>> +channels which can be used in different modes.
>> +
>> +
>> +See this table for the meaning of the different channels
>> +
>> +Channel Signal
>> +------------------------------------------
>> +0    Battery type(BTYPE)
>> +1    BCI: Battery temperature (BTEMP)
>> +2    GP analog input
>> +3    GP analog input
>> +4    GP analog input
>> +5    GP analog input
>> +6    GP analog input
>> +7    GP analog input
>> +8    BCI: VBUS voltage(VBUS)
>> +9    Backup Battery voltage (VBKP)
>> +10   BCI: Battery charger current (ICHG)
>> +11   BCI: Battery charger voltage (VCHG)
>> +12   BCI: Main battery voltage (VBAT)
>> +13   Reserved
>> +14   Reserved
>> +15   VRUSB Supply/Speaker left/Speaker right polarization level
>> +
>> +
>> +The Sysfs nodes will represent the voltage in the units of mV,
>> +the temperature channel shows the converted raw voltage in mV.
>> +The Battery charging current channel represents raw voltage mV.
>
> Doesn't really make sense to me to export values known to be current and temperature
> as voltages. You'll have to add a lot more information for that to make sense.
>

As mentioned earlier the raw voltage to current conversion depends on the CGAIN
bit of the BCICTL1 battery register. Hence i preferrd not to include
the conversion
here.

>> +Channel 13 and 14 are reserved.
>
> You already said that above.

Ok i will remove this.
>
>> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
>> index 773e484..11ddde3 100644
>> --- a/drivers/hwmon/Kconfig
>> +++ b/drivers/hwmon/Kconfig
>> @@ -940,6 +940,16 @@ config SENSORS_TMP421
>>         This driver can also be built as a module.  If so, the module
>>         will be called tmp421.
>>
>> +config SENSORS_TWL4030_MADC
>> +     tristate "Texas Instruments TWL4030 MADC Hwmon"
>> +     depends on TWL4030_MADC
>> +     help
>> +     This driver provides hwmon support for triton TWL4030-MADC.
>> +     This driver can be built as part of kernel or can be built
>> +     as a module.
>
>        Kernel is obvious. Please stick with the usual text
>

Ok.
>        "This driver can also be built as a module.  If so, the module
>         will be called XXXX"
>
> At least please tell users how the module is named.
>

Ok. I will add it.
>> +             This driver exposes the various voltage values to
>> +     the user space.
>> +
> Not sure if this provides any value. Either case, it should be before "built as module".
>

Ok.
>>  config SENSORS_VIA_CPUTEMP
>>       tristate "VIA CPU temperature sensor"
>>       depends on X86
>> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
>> index dde02d9..bc7d740 100644
>> --- a/drivers/hwmon/Makefile
>> +++ b/drivers/hwmon/Makefile
>> @@ -102,6 +102,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-hwmon.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-hwmon.c b/drivers/hwmon/twl4030-madc-hwmon.c
>> new file mode 100644
>> index 0000000..4a61e8a
>> --- /dev/null
>> +++ b/drivers/hwmon/twl4030-madc-hwmon.c
>> @@ -0,0 +1,155 @@
>> +/*
>> + *
>> + * TWL4030 MADC Hwmon driver-This driver monitors the real time
>> + * conversion of analog signals like battery temperature,
>> + * battery type, battery level etc. User can ask for the conversion on a
>> + * particular channel using the sysfs nodes.
>> + *
>> + * Copyright (C) 2010 Texas Instruments Incorporated - http://www.ti.com/
>
> 2011 ?

Oops missed it. I will correct it.
>
>> + * J Keerthy <j-keerthy@ti.com>
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * version 2 as published by the Free Software Foundation.
>> + *
>> + * 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/platform_device.h>
>> +#include <linux/i2c/twl.h>
>> +#include <linux/i2c/twl4030-madc.h>
>> +#include <linux/hwmon.h>
>> +#include <linux/hwmon-sysfs.h>
>> +
>> +/*
>> + * sysfs hook function
>> + */
>> +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;
>> +     long val;
>> +
>> +     req.channels = (1 << attr->index);
>> +     req.method = TWL4030_MADC_SW2;
>> +     req.func_cb = NULL;
>> +     val = twl4030_madc_conversion(&req);
>> +     if (val >= 0)
>> +             val = req.rbuf[attr->index];
>> +     else
>> +             return val;
>> +
> Seems to be a bit complicated.
>
>        if (val < 0)
>                return val;
>
>        return sprintf(buf, "%ld\n", req.rbuf[attr->index]);
>
> would be more straightforward.
>

Ok.
>> +     return sprintf(buf, "%ld\n", val);
>> +}
>> +
>> +/* sysfs nodes to read individual channels from user side */
>> +static SENSOR_DEVICE_ATTR(in0_battery_type, S_IRUGO, madc_read, NULL, 0);
>> +static SENSOR_DEVICE_ATTR(in1_battery_temp, S_IRUGO, madc_read, NULL, 1);
>
> I don't know why people always try to introduce new sysfs attributes
> for no good reason. Use a label if you want to describe the sensor.
> And use tempX_input for temperatures, and currX_input for currents.
>

Since i was providing the raw voltages i stuck to inX_ naming for even
the cuurent and
temperature attributes. I will change them.

> I am not inclined to accept drivers introducing new sysfs attributes,
> unless the new attribute has either been discussed and added to the ABI,
> or if a _really_ good reason for the non-standard attribute is provided.
> Neither is the case here.
>

Ok. I get the point.
>> +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_vbus, S_IRUGO, madc_read, NULL, 8);
>> +static SENSOR_DEVICE_ATTR(in9_vbkp, S_IRUGO, madc_read, NULL, 9);
>> +static SENSOR_DEVICE_ATTR(in10_battery_charger_current,
>> +                     S_IRUGO, madc_read, NULL, 10);
>> +static SENSOR_DEVICE_ATTR(in11_battery_charger_voltage,
>> +                     S_IRUGO, madc_read, NULL, 11);
>> +static SENSOR_DEVICE_ATTR(in12_main_battery_voltage,
>> +                     S_IRUGO, madc_read, NULL, 12);
>
> ... and again ...

I will correct it.
>
>> +static SENSOR_DEVICE_ATTR(in15_input, S_IRUGO, madc_read, NULL, 15);
>> +
>
> Number sequence doesn't have to match chip channel number.
> Sure you want to leave a hole here ?

13, 14 channels are reserved. Hence left the gap. Followed with the
chip channel numbers.
>
> Either case, you should describe in the documentation how chip channels
> match attribute names.
>

Ok i will add a description.
>> +static struct attribute *twl4030_madc_attributes[] = {
>> +     &sensor_dev_attr_in0_battery_type.dev_attr.attr,
>> +     &sensor_dev_attr_in1_battery_temp.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_vbus.dev_attr.attr,
>> +     &sensor_dev_attr_in9_vbkp.dev_attr.attr,
>> +     &sensor_dev_attr_in10_battery_charger_current.dev_attr.attr,
>> +     &sensor_dev_attr_in11_battery_charger_voltage.dev_attr.attr,
>> +     &sensor_dev_attr_in12_main_battery_voltage.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_hwmon_probe(struct platform_device *pdev)
>> +{
>> +     int ret;
>> +     int status;
>> +     struct device *hwmon;
>> +
>> +     ret = sysfs_create_group(&pdev->dev.kobj, &twl4030_madc_group);
>> +     if (ret)
>> +             goto err_sysfs;
>> +     hwmon = hwmon_device_register(&pdev->dev);
>> +     if (IS_ERR(hwmon)) {
>> +             dev_err(&pdev->dev, "hwmon_device_register failed.\n");
>> +             status = PTR_ERR(hwmon);
>> +             goto err_reg;
>> +     }
>> +
>> +     return 0;
>> +
>> +err_reg:
>> +     sysfs_remove_group(&pdev->dev.kobj, &twl4030_madc_group);
>> +err_sysfs:
>> +
>> +     return ret;
>> +}
>> +
>> +static int __devexit twl4030_madc_hwmon_remove(struct platform_device *pdev)
>> +{
>> +     hwmon_device_unregister(&pdev->dev);
>> +     sysfs_remove_group(&pdev->dev.kobj, &twl4030_madc_group);
>> +
>> +     return 0;
>> +}
>> +
>> +static struct platform_driver twl4030_madc_hwmon_driver = {
>> +     .probe = twl4030_madc_hwmon_probe,
>> +     .remove = __exit_p(twl4030_madc_hwmon_remove),
>> +     .driver = {
>> +                .name = "twl4030_madc_hwmon",
>> +                .owner = THIS_MODULE,
>> +                },
>> +};
>> +
>> +static int __init twl4030_madc_hwmon_init(void)
>> +{
>> +     return platform_driver_register(&twl4030_madc_hwmon_driver);
>> +}
>> +
>> +module_init(twl4030_madc_hwmon_init);
>> +
>> +static void __exit twl4030_madc_hwmon_exit(void)
>> +{
>> +     platform_driver_unregister(&twl4030_madc_hwmon_driver);
>> +}
>> +
>> +module_exit(twl4030_madc_hwmon_exit);
>> +
>> +MODULE_DESCRIPTION("TWL4030 ADC Hwmon driver");
>> +MODULE_LICENSE("GPL");
>> +MODULE_AUTHOR("J Keerthy");
>> +MODULE_ALIAS("twl4030_madc_hwmon");
>> --
>> 1.7.0.4
>>
>



-- 
Regards and Thanks,
Keerthy
--
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

Re: [PATCH 2/2] hwmon: twl4030: Hwmon Driver for TWL4030 MADC

[Guenter Roeck]

On Wed, Feb 16, 2011 at 12:59:52PM -0500, J, KEERTHY wrote:
> Hello Guenter,
> 
> On Wed, Feb 16, 2011 at 9:39 PM, Guenter Roeck
> <guenter.roeck@ericsson.com> wrote:
> > On Wed, Feb 16, 2011 at 07:56:57AM -0500, Keerthy wrote:
> >> This driver exposes the sysfs nodes of the TWL4030 MADC module.
> >> All the channel values are expressed in terms of mV. Channel 13
> >> and channel 14 are reserved. There are channels which represent
> >> temperature and current. Even they output raw voltage in mV.
> >>
> > Why ?
> 
> The conversion to current and temperature in case of MADC depends
> on a register in the battery module. Hence battery driver can expose the
> converted value. So providing the raw voltage here.

Not a good reason.

You could create an API to let you retrieve the register values.
You could read the respective registers directly.
You could provide the register values in platform data.
If there will always be just one instance of the driver, you could provide
the register values via module parameters.
You could present the raw temperature/current values and correct it
with sensors3.conf.

Either case, I don't think it is a good idea to present known temperatures
or currents as voltage.

Guenter