All of lore.kernel.org
 help / color / mirror / Atom feed
* [lm-sensors] [PATCH v3] drivers/hwmon NTC Thermistor Initial
@ 2010-12-20 10:48 MyungJoo Ham
  2010-12-23 20:34 ` Guenter Roeck
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: MyungJoo Ham @ 2010-12-20 10:48 UTC (permalink / raw)
  To: lm-sensors

This patch adds support for NTC Thermistor series. In this release, the
following thermistors are supported: NCP15WB473, NCP18WB473, NCP03WB473,
and NCP15WL333. This driver is based on the datasheet of MURATA.

The driver in the patch does conversion from the raw ADC value
(either voltage or resistence) to temperature. In order to use
voltage values as input, the circuit schematics should be provided
with the platform data. A compensation table for each type of thermistor
is provided for the conversion.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: KyungMin Park <kyungmin.park@samsung.com>

--
Updates
v3
- style fixes
- removed symbol exports
- avoid divide-by-zero
- omitted files added (Kconfig/Makefile)
- removed unnecessary codes
- return error values, not default values for errors.
v2
- added Documentation/hwmon/ntc
- followed hwmon sysfs
- detached the monitoring service
(thank you for the comments, Guenter Roeck)
---
 Documentation/hwmon/ntc |  103 +++++++++++
 drivers/hwmon/Kconfig   |   10 +
 drivers/hwmon/Makefile  |    1 +
 drivers/hwmon/ntc.c     |  469 +++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/ntc.h     |   54 ++++++
 5 files changed, 637 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/hwmon/ntc
 create mode 100644 drivers/hwmon/ntc.c
 create mode 100644 include/linux/ntc.h

diff --git a/Documentation/hwmon/ntc b/Documentation/hwmon/ntc
new file mode 100644
index 0000000..cdb3a44
--- /dev/null
+++ b/Documentation/hwmon/ntc
@@ -0,0 +1,103 @@
+Kernel driver ntc
+========+
+Supported thermistors:
+* Murata NTC Thermistors "Chip" Type
+  Prefix: 'ncp'
+  NCPxxWB473: NCP15WB473, NCP18WB473, NCP21WB473, NCP03WB473
+  NCPxxWL333: NCP15WL333
+  Datasheet: Publicly available at Murata
+
+Other NTC thermistors can be supported simply by adding compensation
+tables; e.g., NCP15WL333 support is added by the table ncpXXwl333.
+
+Authors:
+	MyungJoo Ham <myungjoo.ham@samsung.com>
+
+Description
+-----------
+
+The NTC thermistor is a simple thermistor that requires users to provide the
+resistance and lookup the corresponding compensation table to get the
+temperature input.
+
+The NTC driver provides lookup tables with a linear approximation function
+and four circuit models with an option not to use any of the four models.
+
+The four circuit models provided are:
+
+	$: resister, [TH]: the thermistor
+
+ 1. connect = NTC_CONNECTED_POSITIVE, pullup_ohm > 0
+
+   [pullup_uV]
+       |    |
+      [TH]  $ (pullup_ohm)
+       |    |
+       +----+-----------------------[read_uV]
+       |
+       $ (pulldown_ohm)
+       |
+      --- (ground)
+
+ 2. connect = NTC_CONNECTED_POSITIVE, pullup_ohm = 0 (not-connected)
+
+   [pullup_uV]
+       |
+      [TH]
+       |
+       +----------------------------[read_uV]
+       |
+       $ (pulldown_ohm)
+       |
+      --- (ground)
+
+ 3. connect = NTC_CONNECTED_GROUND, pulldown_ohm > 0
+
+   [pullup_uV]
+       |
+       $ (pullup_ohm)
+       |
+       +----+-----------------------[read_uV]
+       |    |
+      [TH]  $ (pulldown_ohm)
+       |    |
+      -------- (ground)
+
+ 4. connect = NTC_CONNECTED_GROUND, pulldown_ohm = 0 (not-connected)
+
+   [pullup_uV]
+       |
+       $ (pullup_ohm)
+       |
+       +----------------------------[read_uV]
+       |
+      [TH]
+       |
+      --- (ground)
+
+When one of the four circuit models is used, read_uV, pullup_uV, pullup_ohm,
+pulldown_ohm, and connect should be provided. When none of the four models
+are suitable or the user can get the resistance directly, the user should
+provide read_ohm and _not_ provide the others.
+
+If there is an error, the temperature value returned will be INT_MIN.
+
+Sysfs Interface
+---------------
+name		the mandatory global attribute, the thermistor name.
+
+temp1_type	always 4 (thermistor)
+		RO
+
+temp1_max	the highest temperature possible from the compensation table.
+		RO
+
+temp1_min	the lowest temperature possible from the compensation table.
+		RO
+
+temp1_input	measure the temperature and provide the measured value.
+		(reading this file initiates the reading procedure.)
+		RO
+
+Note that each NTC thermistor has only _one_ thermistor; thus, only temp1 exists.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index a56f6ad..336490e 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1171,6 +1171,16 @@ config SENSORS_MC13783_ADC
         help
           Support for the A/D converter on MC13783 PMIC.
 
+config SENSORS_NTC_THERMISTOR
+	tristate "NTC thermistor support"
+	help
+	  This driver supports NTC thermistors sensor reading and its
+	  interpretation. The driver can also monitor the temperature and
+	  send notifications about the temperature.
+
+	  Currently, among those many NTC thermistors, this driver supports
+	  NCP15WB473, NCP18WB473, NCP21WB473, NCP03WB473, and NCP15WL333.
+
 if ACPI
 
 comment "ACPI drivers"
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 2479b3d..382c130 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -109,6 +109,7 @@ obj-$(CONFIG_SENSORS_W83L785TS)	+= w83l785ts.o
 obj-$(CONFIG_SENSORS_W83L786NG)	+= w83l786ng.o
 obj-$(CONFIG_SENSORS_WM831X)	+= wm831x-hwmon.o
 obj-$(CONFIG_SENSORS_WM8350)	+= wm8350-hwmon.o
+obj-$(CONFIG_SENSORS_NTC_THERMISTOR)	+= ntc.o
 
 ifeq ($(CONFIG_HWMON_DEBUG_CHIP),y)
 EXTRA_CFLAGS += -DDEBUG
diff --git a/drivers/hwmon/ntc.c b/drivers/hwmon/ntc.c
new file mode 100644
index 0000000..365f6fc
--- /dev/null
+++ b/drivers/hwmon/ntc.c
@@ -0,0 +1,469 @@
+/*
+ * ntc.c - NTC Thermistors
+ *
+ *  Copyright (C) 2010 Samsung Electronics
+ *  MyungJoo Ham <myungjoo.ham@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ *
+ */
+
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/pm_runtime.h>
+#include <linux/math64.h>
+#include <linux/platform_device.h>
+#include <linux/err.h>
+
+#include <linux/ntc.h>
+
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+
+struct ntc_compensation {
+	int		temp_C;
+	unsigned int	ohm;
+};
+
+/*
+ * A compsentation table should be sorted by the values of .ohm. Either
+ * ascending or descending order is fine.
+ * The following compensation tables are from the specification of Murata NTC
+ * Thermistors Datasheet
+ */
+const struct ntc_compensation ncpXXwb473[] = {
+	{ .temp_C	= -40, .ohm	= 1747920 },
+	{ .temp_C	= -35, .ohm	= 1245428 },
+	{ .temp_C	= -30, .ohm	= 898485 },
+	{ .temp_C	= -25, .ohm	= 655802 },
+	{ .temp_C	= -20, .ohm	= 483954 },
+	{ .temp_C	= -15, .ohm	= 360850 },
+	{ .temp_C	= -10, .ohm	= 271697 },
+	{ .temp_C	= -5, .ohm	= 206463 },
+	{ .temp_C	= 0, .ohm	= 158214 },
+	{ .temp_C	= 5, .ohm	= 122259 },
+	{ .temp_C	= 10, .ohm	= 95227 },
+	{ .temp_C	= 15, .ohm	= 74730 },
+	{ .temp_C	= 20, .ohm	= 59065 },
+	{ .temp_C	= 25, .ohm	= 47000 },
+	{ .temp_C	= 30, .ohm	= 37643 },
+	{ .temp_C	= 35, .ohm	= 30334 },
+	{ .temp_C	= 40, .ohm	= 24591 },
+	{ .temp_C	= 45, .ohm	= 20048 },
+	{ .temp_C	= 50, .ohm	= 16433 },
+	{ .temp_C	= 55, .ohm	= 13539 },
+	{ .temp_C	= 60, .ohm	= 11209 },
+	{ .temp_C	= 65, .ohm	= 9328 },
+	{ .temp_C	= 70, .ohm	= 7798 },
+	{ .temp_C	= 75, .ohm	= 6544 },
+	{ .temp_C	= 80, .ohm	= 5518 },
+	{ .temp_C	= 85, .ohm	= 4674 },
+	{ .temp_C	= 90, .ohm	= 3972 },
+	{ .temp_C	= 95, .ohm	= 3388 },
+	{ .temp_C	= 100, .ohm	= 2902 },
+	{ .temp_C	= 105, .ohm	= 2494 },
+	{ .temp_C	= 110, .ohm	= 2150 },
+	{ .temp_C	= 115, .ohm	= 1860 },
+	{ .temp_C	= 120, .ohm	= 1615 },
+	{ .temp_C	= 125, .ohm	= 1406 },
+};
+const struct ntc_compensation ncpXXwl333[] = {
+	{ .temp_C	= -40, .ohm	= 1610154 },
+	{ .temp_C	= -35, .ohm	= 1130850 },
+	{ .temp_C	= -30, .ohm	= 802609 },
+	{ .temp_C	= -25, .ohm	= 575385 },
+	{ .temp_C	= -20, .ohm	= 416464 },
+	{ .temp_C	= -15, .ohm	= 304219 },
+	{ .temp_C	= -10, .ohm	= 224193 },
+	{ .temp_C	= -5, .ohm	= 166623 },
+	{ .temp_C	= 0, .ohm	= 124850 },
+	{ .temp_C	= 5, .ohm	= 94287 },
+	{ .temp_C	= 10, .ohm	= 71747 },
+	{ .temp_C	= 15, .ohm	= 54996 },
+	{ .temp_C	= 20, .ohm	= 42455 },
+	{ .temp_C	= 25, .ohm	= 33000 },
+	{ .temp_C	= 30, .ohm	= 25822 },
+	{ .temp_C	= 35, .ohm	= 20335 },
+	{ .temp_C	= 40, .ohm	= 16115 },
+	{ .temp_C	= 45, .ohm	= 12849 },
+	{ .temp_C	= 50, .ohm	= 10306 },
+	{ .temp_C	= 55, .ohm	= 8314 },
+	{ .temp_C	= 60, .ohm	= 6746 },
+	{ .temp_C	= 65, .ohm	= 5503 },
+	{ .temp_C	= 70, .ohm	= 4513 },
+	{ .temp_C	= 75, .ohm	= 3721 },
+	{ .temp_C	= 80, .ohm	= 3084 },
+	{ .temp_C	= 85, .ohm	= 2569 },
+	{ .temp_C	= 90, .ohm	= 2151 },
+	{ .temp_C	= 95, .ohm	= 1809 },
+	{ .temp_C	= 100, .ohm	= 1529 },
+	{ .temp_C	= 105, .ohm	= 1299 },
+	{ .temp_C	= 110, .ohm	= 1108 },
+	{ .temp_C	= 115, .ohm	= 949 },
+	{ .temp_C	= 120, .ohm	= 817 },
+	{ .temp_C	= 125, .ohm	= 707 },
+};
+
+struct ntc_data {
+	struct device *hwmon_dev;
+	struct ntc_thermistor_platform_data *pdata;
+	const struct ntc_compensation *comp;
+	struct device *dev;
+	int n_comp;
+	char name[PLATFORM_NAME_SIZE];
+	int maxtemp;
+	int mintemp;
+};
+
+static inline u64 div64_u64_safe(u64 dividend, u64 divisor)
+{
+	if (divisor = 0 && dividend = 0)
+		return 0;
+	if (divisor = 0)
+		return UINT_MAX;
+	return div64_u64(dividend, divisor);
+}
+
+static unsigned int get_ohm_of_thermistor(struct ntc_data *data,
+		unsigned int uV)
+{
+	struct ntc_thermistor_platform_data *pdata = data->pdata;
+	u64 mV = uV / 1000;
+	u64 pmV = pdata->pullup_uV / 1000;
+	u64 N, puO, pdO;
+	puO = pdata->pullup_ohm;
+	pdO = pdata->pulldown_ohm;
+
+	if (mV = 0) {
+		if (pdata->connect = NTC_CONNECTED_POSITIVE)
+			return UINT_MAX;
+		else
+			return 0;
+	}
+	if (mV >= pmV)
+		return (pdata->connect = NTC_CONNECTED_POSITIVE) ?
+			0 : UINT_MAX;
+
+	if (pdata->connect = NTC_CONNECTED_POSITIVE && puO = 0)
+		N = div64_u64_safe(pdO * (pmV - mV), mV);
+	else if (pdata->connect = NTC_CONNECTED_GROUND && pdO = 0)
+		N = div64_u64_safe(puO * mV, pmV - mV);
+	else if (pdata->connect = NTC_CONNECTED_POSITIVE)
+		N = div64_u64_safe(pdO * puO * (pmV - mV),
+				puO * mV - pdO * (pmV - mV));
+	else
+		N = div64_u64_safe(pdO * puO * mV, pdO * (pmV - mV) - puO * mV);
+
+	return (unsigned int) N;
+}
+
+static int lookup_comp(struct ntc_data *data,
+		unsigned int ohm, int *i_low, int *i_high)
+{
+	/*  greatest lower bound and least upper bound */
+	int glb = -1, lub = -1;
+	unsigned int glb_ohm = 0, lub_ohm = 0;
+	int i;
+
+	for (i = 0; i < data->n_comp; i++) {
+		if (data->comp[i].ohm <= ohm &&
+		    (glb = -1 || glb_ohm < data->comp[i].ohm)) {
+			glb = i;
+			glb_ohm = data->comp[i].ohm;
+		}
+
+		if (data->comp[i].ohm >= ohm &&
+		    (lub = -1 || lub_ohm > data->comp[i].ohm)) {
+			lub = i;
+			lub_ohm = data->comp[i].ohm;
+		}
+
+		/*
+		 * If the compensation table is sorted in any direction,
+		 * having both glb and lub means that we have both values
+		 * valid.
+		 */
+		if (glb != -1 && lub != -1)
+			break;
+	}
+	*i_low = glb;
+	*i_high = lub;
+
+	if (glb = -1 || lub = -1)
+		return -EINVAL;
+	return 0;
+}
+
+static int get_temp_mC(struct ntc_data *data, unsigned int ohm)
+{
+	int low, high;
+	int ret;
+
+	ret = lookup_comp(data, ohm, &low, &high);
+	if (ret) {
+		/* Unable to use linear approximation */
+		if (low != -1)
+			return data->comp[low].temp_C * 1000;
+		if (high != -1)
+			return data->comp[high].temp_C * 1000;
+
+		pr_warn("ntc thermistor: data not valid: %dohm.\n", ohm);
+		return INT_MIN;
+	}
+
+	return data->comp[low].temp_C * 1000 +
+		((data->comp[high].temp_C - data->comp[low].temp_C) * 1000 *
+		((int)ohm - (int)data->comp[low].ohm)) /
+		((int)data->comp[high].ohm - (int)data->comp[low].ohm);
+}
+
+static int _ntc_thermistor_read(struct ntc_data *data)
+{
+	int temp = INT_MIN;
+
+	if (data->pdata->read_ohm)
+		temp = get_temp_mC(data, data->pdata->read_ohm());
+
+	if (data->pdata->read_uV)
+		temp = get_temp_mC(data, get_ohm_of_thermistor(data,
+					data->pdata->read_uV()));
+	if (temp != INT_MIN)
+		return temp;
+
+	dev_warn(data->dev, "Sensor reading function not available.\n");
+	return INT_MIN;
+}
+
+int ntc_thermistor_read(struct device *dev)
+{
+	return _ntc_thermistor_read(dev_get_drvdata(dev));
+}
+
+static ssize_t ntc_show_name(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct ntc_data *data = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%s\n", data->name);
+}
+
+static ssize_t ntc_show_type(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	return sprintf(buf, "4\n");
+}
+
+static ssize_t ntc_show_temp_max(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct ntc_data *data = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%d\n", data->maxtemp);
+}
+
+static ssize_t ntc_show_temp_min(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct ntc_data *data = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%d\n", data->mintemp);
+}
+
+static ssize_t ntc_show_temp(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	int temp = ntc_thermistor_read(dev);
+
+	return sprintf(buf, "%d\n", temp);
+}
+
+static SENSOR_DEVICE_ATTR(temp1_type, S_IRUGO, ntc_show_type, NULL, 0);
+static SENSOR_DEVICE_ATTR(temp1_max, S_IRUGO, ntc_show_temp_max, NULL, 0);
+static SENSOR_DEVICE_ATTR(temp1_min, S_IRUGO, ntc_show_temp_min, NULL, 0);
+static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, ntc_show_temp, NULL, 0);
+static DEVICE_ATTR(name, S_IRUGO, ntc_show_name, NULL);
+
+static struct attribute *ntc_attributes[] = {
+	&dev_attr_name.attr,
+	&sensor_dev_attr_temp1_type.dev_attr.attr,
+	&sensor_dev_attr_temp1_max.dev_attr.attr,
+	&sensor_dev_attr_temp1_min.dev_attr.attr,
+	&sensor_dev_attr_temp1_input.dev_attr.attr,
+	NULL,
+};
+
+static const struct attribute_group ntc_attr_group = {
+	.attrs = ntc_attributes,
+};
+
+static int __devinit ntc_thermistor_probe(struct platform_device *pdev)
+{
+	struct ntc_data *data;
+	struct ntc_thermistor_platform_data *pdata = pdev->dev.platform_data;
+	int ret = 0, i;
+
+	if (!pdata) {
+		dev_err(&pdev->dev, "No platform init data supplied.\n");
+		return -ENODEV;
+	}
+
+	/* Either one of the two is required. */
+	if (!pdata->read_uV && !pdata->read_ohm) {
+		dev_err(&pdev->dev, "Both read_uV and read_ohm missing."
+				"Need either one of the two.\n");
+		return -EINVAL;
+	}
+
+	if (pdata->read_uV && pdata->read_ohm) {
+		dev_warn(&pdev->dev, "Only one of read_uV and read_ohm "
+				"is needed; ignoring read_uV.\n");
+		pdata->read_uV = NULL;
+	}
+
+	if (pdata->read_uV && (pdata->pullup_uV = 0 ||
+				(pdata->pullup_ohm = 0 && pdata->connect =
+				 NTC_CONNECTED_GROUND) ||
+				(pdata->pulldown_ohm = 0 && pdata->connect =
+				 NTC_CONNECTED_POSITIVE) ||
+				(pdata->connect != NTC_CONNECTED_POSITIVE &&
+				 pdata->connect != NTC_CONNECTED_GROUND))) {
+		dev_err(&pdev->dev, "Required data to use read_uV not "
+				"supplied.\n");
+		return -EINVAL;
+	}
+
+	data = kzalloc(sizeof(struct ntc_data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->dev = &pdev->dev;
+	data->pdata = pdata;
+	strncpy(data->name, pdev->id_entry->name, PLATFORM_NAME_SIZE);
+
+	switch (pdev->id_entry->driver_data) {
+	case TYPE_NCPXXWB473:
+		data->comp = ncpXXwb473;
+		data->n_comp = ARRAY_SIZE(ncpXXwb473);
+		break;
+	case TYPE_NCPXXWL333:
+		data->comp = ncpXXwl333;
+		data->n_comp = ARRAY_SIZE(ncpXXwl333);
+		break;
+	default:
+		dev_err(&pdev->dev, "Unknown device type: %lu(%s)\n",
+				pdev->id_entry->driver_data,
+				pdev->id_entry->name);
+		ret = -EINVAL;
+		goto err;
+	}
+
+	data->mintemp = INT_MAX;
+	data->maxtemp = INT_MIN;
+
+	/*
+	 * The compensation table is sorted by ohm value. Although it is
+	 * normally assumed that the temperature value monotonically
+	 * increases or decreases with the ohm value, but there could be
+	 * different thermistors. However, if we can assume so, we only need
+	 * to look up the first and the last values.
+	 */
+	for (i = 0; i < data->n_comp; i++) {
+		if (data->comp[i].temp_C > data->maxtemp)
+			data->maxtemp = data->comp[i].temp_C;
+		if (data->comp[i].temp_C < data->mintemp)
+			data->mintemp = data->comp[i].temp_C;
+	}
+
+	if (data->mintemp != INT_MAX)
+		data->mintemp *= 1000;
+	if (data->maxtemp != INT_MIN)
+		data->maxtemp *= 1000;
+
+	platform_set_drvdata(pdev, data);
+
+	ret = sysfs_create_group(&data->dev->kobj, &ntc_attr_group);
+	if (ret) {
+		dev_err(data->dev, "unable to create sysfs files\n");
+		goto err;
+	}
+
+	data->hwmon_dev = hwmon_device_register(data->dev);
+	if (IS_ERR_OR_NULL(data->hwmon_dev)) {
+		dev_err(data->dev, "unable to register as hwmon device.\n");
+		ret = -EINVAL;
+		goto err_after_sysfs;
+	}
+
+	dev_info(&pdev->dev, "Thermistor %s:%d (type: %s/%lu) successfully probed.\n",
+			pdev->name, pdev->id, pdev->id_entry->name,
+			pdev->id_entry->driver_data);
+	return 0;
+err_after_sysfs:
+	sysfs_remove_group(&data->dev->kobj, &ntc_attr_group);
+err:
+	kfree(data);
+	return ret;
+}
+
+static int __devexit ntc_thermistor_remove(struct platform_device *pdev)
+{
+	struct ntc_data *data = platform_get_drvdata(pdev);
+
+	hwmon_device_unregister(data->hwmon_dev); /* Root of unload warning */
+	sysfs_remove_group(&data->dev->kobj, &ntc_attr_group);
+
+	kfree(data);
+
+	platform_set_drvdata(pdev, NULL);
+
+	return 0;
+}
+
+static const struct platform_device_id ntc_thermistor_id[] = {
+	{ "ncp15wb473", TYPE_NCPXXWB473 },
+	{ "ncp18wb473", TYPE_NCPXXWB473 },
+	{ "ncp21wb473", TYPE_NCPXXWB473 },
+	{ "ncp03wb473", TYPE_NCPXXWB473 },
+	{ "ncp15wl333", TYPE_NCPXXWL333 },
+	{ },
+};
+
+static struct platform_driver ntc_thermistor_driver = {
+	.driver = {
+		.name = "ntc-thermistor",
+		.owner = THIS_MODULE,
+	},
+	.probe = ntc_thermistor_probe,
+	.remove = __devexit_p(ntc_thermistor_remove),
+	.id_table = ntc_thermistor_id,
+};
+
+static int __init ntc_thermistor_init(void)
+{
+	return platform_driver_register(&ntc_thermistor_driver);
+}
+
+module_init(ntc_thermistor_init);
+
+static void __exit ntc_thermistor_cleanup(void)
+{
+	platform_driver_unregister(&ntc_thermistor_driver);
+}
+
+module_exit(ntc_thermistor_cleanup);
+
+MODULE_DESCRIPTION("NTC Thermistor Driver");
+MODULE_AUTHOR("MyungJoo Ham <myungjoo.ham@samsung.com>");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/ntc.h b/include/linux/ntc.h
new file mode 100644
index 0000000..5e853d0
--- /dev/null
+++ b/include/linux/ntc.h
@@ -0,0 +1,54 @@
+/*
+ * ntc.h - NTC Thermistors
+ *
+ *  Copyright (C) 2010 Samsung Electronics
+ *  MyungJoo Ham <myungjoo.ham@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+#ifndef _LINUX_NTC_H
+#define _LINUX_NTC_H
+
+enum ntc_thermistor_type {
+	TYPE_NCPXXWB473,
+	TYPE_NCPXXWL333,
+};
+
+struct ntc_thermistor_platform_data {
+	/*
+	 * One (not both) of read_uV and read_ohm should be provided and only
+	 * one of the two should be provided.
+	 *
+	 * pullup_uV, pullup_ohm, pulldown_ohm, and connect are required to use
+	 * read_uV()
+	 *
+	 * How to setup pullup_ohm, pulldown_ohm, and connect is
+	 * described at Documentation/hwmon/ntc
+	 *
+	 * pullup/down_ohm: 0 for infinite / not-connected
+	 */
+	unsigned int (*read_uV)(void);
+	unsigned int pullup_uV;
+
+	unsigned int pullup_ohm;
+	unsigned int pulldown_ohm;
+	enum { NTC_CONNECTED_POSITIVE, NTC_CONNECTED_GROUND } connect;
+
+	unsigned int (*read_ohm)(void);
+};
+
+extern int ntc_thermistor_read(struct device *dev);
+
+#endif /* _LINUX_NTC_H */
-- 
1.7.1


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

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

* Re: [lm-sensors] [PATCH v3] drivers/hwmon NTC Thermistor Initial
  2010-12-20 10:48 [lm-sensors] [PATCH v3] drivers/hwmon NTC Thermistor Initial MyungJoo Ham
@ 2010-12-23 20:34 ` Guenter Roeck
  2010-12-24  1:36 ` MyungJoo Ham
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Guenter Roeck @ 2010-12-23 20:34 UTC (permalink / raw)
  To: lm-sensors

On Mon, Dec 20, 2010 at 05:48:42AM -0500, MyungJoo Ham wrote:
> This patch adds support for NTC Thermistor series. In this release, the
> following thermistors are supported: NCP15WB473, NCP18WB473, NCP03WB473,
> and NCP15WL333. This driver is based on the datasheet of MURATA.
> 
> The driver in the patch does conversion from the raw ADC value
> (either voltage or resistence) to temperature. In order to use
> voltage values as input, the circuit schematics should be provided
> with the platform data. A compensation table for each type of thermistor
> is provided for the conversion.
> 
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> Signed-off-by: KyungMin Park <kyungmin.park@samsung.com>
> 
> --
> Updates
> v3
> - style fixes
> - removed symbol exports
> - avoid divide-by-zero
> - omitted files added (Kconfig/Makefile)
> - removed unnecessary codes
> - return error values, not default values for errors.
> v2
> - added Documentation/hwmon/ntc
> - followed hwmon sysfs
> - detached the monitoring service
> (thank you for the comments, Guenter Roeck)
> ---
>  Documentation/hwmon/ntc |  103 +++++++++++
>  drivers/hwmon/Kconfig   |   10 +
>  drivers/hwmon/Makefile  |    1 +
>  drivers/hwmon/ntc.c     |  469 +++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/ntc.h     |   54 ++++++
>  5 files changed, 637 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/hwmon/ntc
>  create mode 100644 drivers/hwmon/ntc.c
>  create mode 100644 include/linux/ntc.h
> 
> diff --git a/Documentation/hwmon/ntc b/Documentation/hwmon/ntc
> new file mode 100644
> index 0000000..cdb3a44
> --- /dev/null
> +++ b/Documentation/hwmon/ntc
> @@ -0,0 +1,103 @@
> +Kernel driver ntc
> +========> +
> +Supported thermistors:
> +* Murata NTC Thermistors "Chip" Type
> +  Prefix: 'ncp'
> +  NCPxxWB473: NCP15WB473, NCP18WB473, NCP21WB473, NCP03WB473
> +  NCPxxWL333: NCP15WL333
> +  Datasheet: Publicly available at Murata
> +
> +Other NTC thermistors can be supported simply by adding compensation
> +tables; e.g., NCP15WL333 support is added by the table ncpXXwl333.
> +
> +Authors:
> +       MyungJoo Ham <myungjoo.ham@samsung.com>
> +
> +Description
> +-----------
> +
> +The NTC thermistor is a simple thermistor that requires users to provide the
> +resistance and lookup the corresponding compensation table to get the
> +temperature input.
> +
> +The NTC driver provides lookup tables with a linear approximation function
> +and four circuit models with an option not to use any of the four models.
> +
> +The four circuit models provided are:
> +
> +       $: resister, [TH]: the thermistor
> +
> + 1. connect = NTC_CONNECTED_POSITIVE, pullup_ohm > 0
> +
> +   [pullup_uV]
> +       |    |
> +      [TH]  $ (pullup_ohm)
> +       |    |
> +       +----+-----------------------[read_uV]
> +       |
> +       $ (pulldown_ohm)
> +       |
> +      --- (ground)
> +
> + 2. connect = NTC_CONNECTED_POSITIVE, pullup_ohm = 0 (not-connected)
> +
> +   [pullup_uV]
> +       |
> +      [TH]
> +       |
> +       +----------------------------[read_uV]
> +       |
> +       $ (pulldown_ohm)
> +       |
> +      --- (ground)
> +
> + 3. connect = NTC_CONNECTED_GROUND, pulldown_ohm > 0
> +
> +   [pullup_uV]
> +       |
> +       $ (pullup_ohm)
> +       |
> +       +----+-----------------------[read_uV]
> +       |    |
> +      [TH]  $ (pulldown_ohm)
> +       |    |
> +      -------- (ground)
> +
> + 4. connect = NTC_CONNECTED_GROUND, pulldown_ohm = 0 (not-connected)
> +
> +   [pullup_uV]
> +       |
> +       $ (pullup_ohm)
> +       |
> +       +----------------------------[read_uV]
> +       |
> +      [TH]
> +       |
> +      --- (ground)
> +
> +When one of the four circuit models is used, read_uV, pullup_uV, pullup_ohm,
> +pulldown_ohm, and connect should be provided. When none of the four models
> +are suitable or the user can get the resistance directly, the user should
> +provide read_ohm and _not_ provide the others.
> +
> +If there is an error, the temperature value returned will be INT_MIN.
> +
> +Sysfs Interface
> +---------------
> +name           the mandatory global attribute, the thermistor name.
> +
> +temp1_type     always 4 (thermistor)
> +               RO
> +
> +temp1_max      the highest temperature possible from the compensation table.
> +               RO
> +
> +temp1_min      the lowest temperature possible from the compensation table.
> +               RO
> +
> +temp1_input    measure the temperature and provide the measured value.
> +               (reading this file initiates the reading procedure.)
> +               RO
> +
> +Note that each NTC thermistor has only _one_ thermistor; thus, only temp1 exists.
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index a56f6ad..336490e 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -1171,6 +1171,16 @@ config SENSORS_MC13783_ADC
>          help
>            Support for the A/D converter on MC13783 PMIC.
> 
> +config SENSORS_NTC_THERMISTOR

Should be in alphabetic order, ie before SENSORS_PC87360.

> +       tristate "NTC thermistor support"
> +       help
> +         This driver supports NTC thermistors sensor reading and its
> +         interpretation. The driver can also monitor the temperature and
> +         send notifications about the temperature.
> +
> +         Currently, among those many NTC thermistors, this driver supports
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Text does not provide value. Please remove.

> +         NCP15WB473, NCP18WB473, NCP21WB473, NCP03WB473, and NCP15WL333.
> +

Please add text showing module name if built as module.

>  if ACPI
> 
>  comment "ACPI drivers"
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 2479b3d..382c130 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -109,6 +109,7 @@ obj-$(CONFIG_SENSORS_W83L785TS)     += w83l785ts.o
>  obj-$(CONFIG_SENSORS_W83L786NG)        += w83l786ng.o
>  obj-$(CONFIG_SENSORS_WM831X)   += wm831x-hwmon.o
>  obj-$(CONFIG_SENSORS_WM8350)   += wm8350-hwmon.o
> +obj-$(CONFIG_SENSORS_NTC_THERMISTOR)   += ntc.o
> 
Please put in alphabetic order (before PC87360).

>  ifeq ($(CONFIG_HWMON_DEBUG_CHIP),y)
>  EXTRA_CFLAGS += -DDEBUG
> diff --git a/drivers/hwmon/ntc.c b/drivers/hwmon/ntc.c
> new file mode 100644
> index 0000000..365f6fc
> --- /dev/null
> +++ b/drivers/hwmon/ntc.c
> @@ -0,0 +1,469 @@
> +/*
> + * ntc.c - NTC Thermistors
> + *
> + *  Copyright (C) 2010 Samsung Electronics
> + *  MyungJoo Ham <myungjoo.ham@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * 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., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> + *
> + */
> +
> +#include <linux/slab.h>
> +#include <linux/module.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/math64.h>
> +#include <linux/platform_device.h>
> +#include <linux/err.h>
> +
> +#include <linux/ntc.h>
> +
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +
> +struct ntc_compensation {
> +       int             temp_C;
> +       unsigned int    ohm;
> +};
> +
> +/*
> + * A compsentation table should be sorted by the values of .ohm. Either
> + * ascending or descending order is fine.
> + * The following compensation tables are from the specification of Murata NTC
> + * Thermistors Datasheet
> + */
> +const struct ntc_compensation ncpXXwb473[] = {
> +       { .temp_C       = -40, .ohm     = 1747920 },
> +       { .temp_C       = -35, .ohm     = 1245428 },
> +       { .temp_C       = -30, .ohm     = 898485 },
> +       { .temp_C       = -25, .ohm     = 655802 },
> +       { .temp_C       = -20, .ohm     = 483954 },
> +       { .temp_C       = -15, .ohm     = 360850 },
> +       { .temp_C       = -10, .ohm     = 271697 },
> +       { .temp_C       = -5, .ohm      = 206463 },
> +       { .temp_C       = 0, .ohm       = 158214 },
> +       { .temp_C       = 5, .ohm       = 122259 },
> +       { .temp_C       = 10, .ohm      = 95227 },
> +       { .temp_C       = 15, .ohm      = 74730 },
> +       { .temp_C       = 20, .ohm      = 59065 },
> +       { .temp_C       = 25, .ohm      = 47000 },
> +       { .temp_C       = 30, .ohm      = 37643 },
> +       { .temp_C       = 35, .ohm      = 30334 },
> +       { .temp_C       = 40, .ohm      = 24591 },
> +       { .temp_C       = 45, .ohm      = 20048 },
> +       { .temp_C       = 50, .ohm      = 16433 },
> +       { .temp_C       = 55, .ohm      = 13539 },
> +       { .temp_C       = 60, .ohm      = 11209 },
> +       { .temp_C       = 65, .ohm      = 9328 },
> +       { .temp_C       = 70, .ohm      = 7798 },
> +       { .temp_C       = 75, .ohm      = 6544 },
> +       { .temp_C       = 80, .ohm      = 5518 },
> +       { .temp_C       = 85, .ohm      = 4674 },
> +       { .temp_C       = 90, .ohm      = 3972 },
> +       { .temp_C       = 95, .ohm      = 3388 },
> +       { .temp_C       = 100, .ohm     = 2902 },
> +       { .temp_C       = 105, .ohm     = 2494 },
> +       { .temp_C       = 110, .ohm     = 2150 },
> +       { .temp_C       = 115, .ohm     = 1860 },
> +       { .temp_C       = 120, .ohm     = 1615 },
> +       { .temp_C       = 125, .ohm     = 1406 },
> +};
> +const struct ntc_compensation ncpXXwl333[] = {
> +       { .temp_C       = -40, .ohm     = 1610154 },
> +       { .temp_C       = -35, .ohm     = 1130850 },
> +       { .temp_C       = -30, .ohm     = 802609 },
> +       { .temp_C       = -25, .ohm     = 575385 },
> +       { .temp_C       = -20, .ohm     = 416464 },
> +       { .temp_C       = -15, .ohm     = 304219 },
> +       { .temp_C       = -10, .ohm     = 224193 },
> +       { .temp_C       = -5, .ohm      = 166623 },
> +       { .temp_C       = 0, .ohm       = 124850 },
> +       { .temp_C       = 5, .ohm       = 94287 },
> +       { .temp_C       = 10, .ohm      = 71747 },
> +       { .temp_C       = 15, .ohm      = 54996 },
> +       { .temp_C       = 20, .ohm      = 42455 },
> +       { .temp_C       = 25, .ohm      = 33000 },
> +       { .temp_C       = 30, .ohm      = 25822 },
> +       { .temp_C       = 35, .ohm      = 20335 },
> +       { .temp_C       = 40, .ohm      = 16115 },
> +       { .temp_C       = 45, .ohm      = 12849 },
> +       { .temp_C       = 50, .ohm      = 10306 },
> +       { .temp_C       = 55, .ohm      = 8314 },
> +       { .temp_C       = 60, .ohm      = 6746 },
> +       { .temp_C       = 65, .ohm      = 5503 },
> +       { .temp_C       = 70, .ohm      = 4513 },
> +       { .temp_C       = 75, .ohm      = 3721 },
> +       { .temp_C       = 80, .ohm      = 3084 },
> +       { .temp_C       = 85, .ohm      = 2569 },
> +       { .temp_C       = 90, .ohm      = 2151 },
> +       { .temp_C       = 95, .ohm      = 1809 },
> +       { .temp_C       = 100, .ohm     = 1529 },
> +       { .temp_C       = 105, .ohm     = 1299 },
> +       { .temp_C       = 110, .ohm     = 1108 },
> +       { .temp_C       = 115, .ohm     = 949 },
> +       { .temp_C       = 120, .ohm     = 817 },
> +       { .temp_C       = 125, .ohm     = 707 },
> +};
> +
> +struct ntc_data {
> +       struct device *hwmon_dev;
> +       struct ntc_thermistor_platform_data *pdata;
> +       const struct ntc_compensation *comp;
> +       struct device *dev;
> +       int n_comp;
> +       char name[PLATFORM_NAME_SIZE];
> +       int maxtemp;
> +       int mintemp;
> +};
> +
> +static inline u64 div64_u64_safe(u64 dividend, u64 divisor)
> +{
> +       if (divisor = 0 && dividend = 0)
> +               return 0;
> +       if (divisor = 0)
> +               return UINT_MAX;
> +       return div64_u64(dividend, divisor);
> +}
> +
> +static unsigned int get_ohm_of_thermistor(struct ntc_data *data,
> +               unsigned int uV)
> +{
> +       struct ntc_thermistor_platform_data *pdata = data->pdata;
> +       u64 mV = uV / 1000;
> +       u64 pmV = pdata->pullup_uV / 1000;
> +       u64 N, puO, pdO;
> +       puO = pdata->pullup_ohm;
> +       pdO = pdata->pulldown_ohm;
> +
> +       if (mV = 0) {
> +               if (pdata->connect = NTC_CONNECTED_POSITIVE)
> +                       return UINT_MAX;
> +               else
> +                       return 0;
> +       }
> +       if (mV >= pmV)
> +               return (pdata->connect = NTC_CONNECTED_POSITIVE) ?
> +                       0 : UINT_MAX;
> +
> +       if (pdata->connect = NTC_CONNECTED_POSITIVE && puO = 0)
> +               N = div64_u64_safe(pdO * (pmV - mV), mV);
> +       else if (pdata->connect = NTC_CONNECTED_GROUND && pdO = 0)
> +               N = div64_u64_safe(puO * mV, pmV - mV);
> +       else if (pdata->connect = NTC_CONNECTED_POSITIVE)
> +               N = div64_u64_safe(pdO * puO * (pmV - mV),
> +                               puO * mV - pdO * (pmV - mV));
> +       else
> +               N = div64_u64_safe(pdO * puO * mV, pdO * (pmV - mV) - puO * mV);
> +
> +       return (unsigned int) N;
> +}
> +
> +static int lookup_comp(struct ntc_data *data,
> +               unsigned int ohm, int *i_low, int *i_high)
> +{
> +       /*  greatest lower bound and least upper bound */
> +       int glb = -1, lub = -1;
> +       unsigned int glb_ohm = 0, lub_ohm = 0;
> +       int i;
> +
> +       for (i = 0; i < data->n_comp; i++) {
> +               if (data->comp[i].ohm <= ohm &&
> +                   (glb = -1 || glb_ohm < data->comp[i].ohm)) {
> +                       glb = i;
> +                       glb_ohm = data->comp[i].ohm;
> +               }
> +
> +               if (data->comp[i].ohm >= ohm &&
> +                   (lub = -1 || lub_ohm > data->comp[i].ohm)) {
> +                       lub = i;
> +                       lub_ohm = data->comp[i].ohm;
> +               }
> +
> +               /*
> +                * If the compensation table is sorted in any direction,
> +                * having both glb and lub means that we have both values
> +                * valid.
> +                */
> +               if (glb != -1 && lub != -1)
> +                       break;
> +       }
> +       *i_low = glb;
> +       *i_high = lub;
> +
> +       if (glb = -1 || lub = -1)
> +               return -EINVAL;
> +       return 0;
> +}
> +
> +static int get_temp_mC(struct ntc_data *data, unsigned int ohm)
> +{
> +       int low, high;
> +       int ret;
> +
> +       ret = lookup_comp(data, ohm, &low, &high);
> +       if (ret) {
> +               /* Unable to use linear approximation */
> +               if (low != -1)
> +                       return data->comp[low].temp_C * 1000;
> +               if (high != -1)
> +                       return data->comp[high].temp_C * 1000;
> +
> +               pr_warn("ntc thermistor: data not valid: %dohm.\n", ohm);

Still not returning an error.

> +               return INT_MIN;
> +       }
> +
> +       return data->comp[low].temp_C * 1000 +
> +               ((data->comp[high].temp_C - data->comp[low].temp_C) * 1000 *
> +               ((int)ohm - (int)data->comp[low].ohm)) /
> +               ((int)data->comp[high].ohm - (int)data->comp[low].ohm);
> +}
> +
> +static int _ntc_thermistor_read(struct ntc_data *data)
> +{
> +       int temp = INT_MIN;
> +
> +       if (data->pdata->read_ohm)
> +               temp = get_temp_mC(data, data->pdata->read_ohm());
> +
> +       if (data->pdata->read_uV)
> +               temp = get_temp_mC(data, get_ohm_of_thermistor(data,
> +                                       data->pdata->read_uV()));
> +       if (temp != INT_MIN)
> +               return temp;
> +
> +       dev_warn(data->dev, "Sensor reading function not available.\n");

Still not returning an error.

> +       return INT_MIN;
> +}
> +
> +int ntc_thermistor_read(struct device *dev)
> +{
> +       return _ntc_thermistor_read(dev_get_drvdata(dev));
> +}

Function should be removed since it is no longer exported.

> +
> +static ssize_t ntc_show_name(struct device *dev,
> +               struct device_attribute *attr, char *buf)
> +{
> +       struct ntc_data *data = dev_get_drvdata(dev);
> +
> +       return sprintf(buf, "%s\n", data->name);
> +}
> +
> +static ssize_t ntc_show_type(struct device *dev,
> +               struct device_attribute *attr, char *buf)
> +{
> +       return sprintf(buf, "4\n");
> +}
> +
> +static ssize_t ntc_show_temp_max(struct device *dev,
> +               struct device_attribute *attr, char *buf)
> +{
> +       struct ntc_data *data = dev_get_drvdata(dev);
> +
> +       return sprintf(buf, "%d\n", data->maxtemp);
> +}
> +
> +static ssize_t ntc_show_temp_min(struct device *dev,
> +               struct device_attribute *attr, char *buf)
> +{
> +       struct ntc_data *data = dev_get_drvdata(dev);
> +
> +       return sprintf(buf, "%d\n", data->mintemp);
> +}
> +
> +static ssize_t ntc_show_temp(struct device *dev,
> +               struct device_attribute *attr, char *buf)
> +{
> +       int temp = ntc_thermistor_read(dev);
> +
> +       return sprintf(buf, "%d\n", temp);
> +}
> +
> +static SENSOR_DEVICE_ATTR(temp1_type, S_IRUGO, ntc_show_type, NULL, 0);
> +static SENSOR_DEVICE_ATTR(temp1_max, S_IRUGO, ntc_show_temp_max, NULL, 0);
> +static SENSOR_DEVICE_ATTR(temp1_min, S_IRUGO, ntc_show_temp_min, NULL, 0);
> +static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, ntc_show_temp, NULL, 0);
> +static DEVICE_ATTR(name, S_IRUGO, ntc_show_name, NULL);
> +
> +static struct attribute *ntc_attributes[] = {
> +       &dev_attr_name.attr,
> +       &sensor_dev_attr_temp1_type.dev_attr.attr,
> +       &sensor_dev_attr_temp1_max.dev_attr.attr,
> +       &sensor_dev_attr_temp1_min.dev_attr.attr,
> +       &sensor_dev_attr_temp1_input.dev_attr.attr,
> +       NULL,
> +};
> +
> +static const struct attribute_group ntc_attr_group = {
> +       .attrs = ntc_attributes,
> +};
> +
> +static int __devinit ntc_thermistor_probe(struct platform_device *pdev)
> +{
> +       struct ntc_data *data;
> +       struct ntc_thermistor_platform_data *pdata = pdev->dev.platform_data;
> +       int ret = 0, i;
> +
> +       if (!pdata) {
> +               dev_err(&pdev->dev, "No platform init data supplied.\n");
> +               return -ENODEV;
> +       }
> +
> +       /* Either one of the two is required. */
> +       if (!pdata->read_uV && !pdata->read_ohm) {
> +               dev_err(&pdev->dev, "Both read_uV and read_ohm missing."
> +                               "Need either one of the two.\n");
> +               return -EINVAL;
> +       }
> +
> +       if (pdata->read_uV && pdata->read_ohm) {
> +               dev_warn(&pdev->dev, "Only one of read_uV and read_ohm "
> +                               "is needed; ignoring read_uV.\n");
> +               pdata->read_uV = NULL;
> +       }
> +
> +       if (pdata->read_uV && (pdata->pullup_uV = 0 ||
> +                               (pdata->pullup_ohm = 0 && pdata->connect =
> +                                NTC_CONNECTED_GROUND) ||
> +                               (pdata->pulldown_ohm = 0 && pdata->connect =
> +                                NTC_CONNECTED_POSITIVE) ||
> +                               (pdata->connect != NTC_CONNECTED_POSITIVE &&
> +                                pdata->connect != NTC_CONNECTED_GROUND))) {
> +               dev_err(&pdev->dev, "Required data to use read_uV not "
> +                               "supplied.\n");
> +               return -EINVAL;
> +       }
> +
> +       data = kzalloc(sizeof(struct ntc_data), GFP_KERNEL);
> +       if (!data)
> +               return -ENOMEM;
> +
> +       data->dev = &pdev->dev;
> +       data->pdata = pdata;
> +       strncpy(data->name, pdev->id_entry->name, PLATFORM_NAME_SIZE);
> +
> +       switch (pdev->id_entry->driver_data) {
> +       case TYPE_NCPXXWB473:
> +               data->comp = ncpXXwb473;
> +               data->n_comp = ARRAY_SIZE(ncpXXwb473);
> +               break;
> +       case TYPE_NCPXXWL333:
> +               data->comp = ncpXXwl333;
> +               data->n_comp = ARRAY_SIZE(ncpXXwl333);
> +               break;
> +       default:
> +               dev_err(&pdev->dev, "Unknown device type: %lu(%s)\n",
> +                               pdev->id_entry->driver_data,
> +                               pdev->id_entry->name);
> +               ret = -EINVAL;
> +               goto err;
> +       }
> +
> +       data->mintemp = INT_MAX;
> +       data->maxtemp = INT_MIN;
> +
> +       /*
> +        * The compensation table is sorted by ohm value. Although it is
> +        * normally assumed that the temperature value monotonically
> +        * increases or decreases with the ohm value, but there could be
> +        * different thermistors. However, if we can assume so, we only need
> +        * to look up the first and the last values.

The last sentence isn't really valuable. Either only look up first and last values
only, and explain why you do it and if and when the code might have to change,
or just explain why you loop through the table even if you don't need to right now.

> +        */
> +       for (i = 0; i < data->n_comp; i++) {
> +               if (data->comp[i].temp_C > data->maxtemp)
> +                       data->maxtemp = data->comp[i].temp_C;
> +               if (data->comp[i].temp_C < data->mintemp)
> +                       data->mintemp = data->comp[i].temp_C;
> +       }
> +
> +       if (data->mintemp != INT_MAX)
> +               data->mintemp *= 1000;
> +       if (data->maxtemp != INT_MIN)
> +               data->maxtemp *= 1000;
> +
Doesn't this indicate a logical error in the compensation tables ? If so, you might
want to do something better than just report INT_MAX for the minimum temperature
and INT_MIN for the maximum temperature (which doesn't really make sense).
Maybe have the probe function return an error and fail to install.

> +       platform_set_drvdata(pdev, data);
> +
> +       ret = sysfs_create_group(&data->dev->kobj, &ntc_attr_group);
> +       if (ret) {
> +               dev_err(data->dev, "unable to create sysfs files\n");
> +               goto err;
> +       }
> +
> +       data->hwmon_dev = hwmon_device_register(data->dev);
> +       if (IS_ERR_OR_NULL(data->hwmon_dev)) {
> +               dev_err(data->dev, "unable to register as hwmon device.\n");
> +               ret = -EINVAL;
> +               goto err_after_sysfs;
> +       }
> +
> +       dev_info(&pdev->dev, "Thermistor %s:%d (type: %s/%lu) successfully probed.\n",
> +                       pdev->name, pdev->id, pdev->id_entry->name,
> +                       pdev->id_entry->driver_data);
> +       return 0;
> +err_after_sysfs:
> +       sysfs_remove_group(&data->dev->kobj, &ntc_attr_group);
> +err:
> +       kfree(data);
> +       return ret;
> +}
> +
> +static int __devexit ntc_thermistor_remove(struct platform_device *pdev)
> +{
> +       struct ntc_data *data = platform_get_drvdata(pdev);
> +
> +       hwmon_device_unregister(data->hwmon_dev); /* Root of unload warning */
> +       sysfs_remove_group(&data->dev->kobj, &ntc_attr_group);
> +
> +       kfree(data);
> +
> +       platform_set_drvdata(pdev, NULL);
> +
> +       return 0;
> +}
> +
> +static const struct platform_device_id ntc_thermistor_id[] = {
> +       { "ncp15wb473", TYPE_NCPXXWB473 },
> +       { "ncp18wb473", TYPE_NCPXXWB473 },
> +       { "ncp21wb473", TYPE_NCPXXWB473 },
> +       { "ncp03wb473", TYPE_NCPXXWB473 },
> +       { "ncp15wl333", TYPE_NCPXXWL333 },
> +       { },
> +};
> +
> +static struct platform_driver ntc_thermistor_driver = {
> +       .driver = {
> +               .name = "ntc-thermistor",
> +               .owner = THIS_MODULE,
> +       },
> +       .probe = ntc_thermistor_probe,
> +       .remove = __devexit_p(ntc_thermistor_remove),
> +       .id_table = ntc_thermistor_id,
> +};
> +
> +static int __init ntc_thermistor_init(void)
> +{
> +       return platform_driver_register(&ntc_thermistor_driver);
> +}
> +
> +module_init(ntc_thermistor_init);
> +
> +static void __exit ntc_thermistor_cleanup(void)
> +{
> +       platform_driver_unregister(&ntc_thermistor_driver);
> +}
> +
> +module_exit(ntc_thermistor_cleanup);
> +
> +MODULE_DESCRIPTION("NTC Thermistor Driver");
> +MODULE_AUTHOR("MyungJoo Ham <myungjoo.ham@samsung.com>");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/ntc.h b/include/linux/ntc.h
> new file mode 100644
> index 0000000..5e853d0
> --- /dev/null
> +++ b/include/linux/ntc.h
> @@ -0,0 +1,54 @@
> +/*
> + * ntc.h - NTC Thermistors
> + *
> + *  Copyright (C) 2010 Samsung Electronics
> + *  MyungJoo Ham <myungjoo.ham@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * 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., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> + */
> +#ifndef _LINUX_NTC_H
> +#define _LINUX_NTC_H
> +
> +enum ntc_thermistor_type {
> +       TYPE_NCPXXWB473,
> +       TYPE_NCPXXWL333,
> +};
> +
> +struct ntc_thermistor_platform_data {
> +       /*
> +        * One (not both) of read_uV and read_ohm should be provided and only
> +        * one of the two should be provided.
> +        *
> +        * pullup_uV, pullup_ohm, pulldown_ohm, and connect are required to use
> +        * read_uV()
> +        *
> +        * How to setup pullup_ohm, pulldown_ohm, and connect is
> +        * described at Documentation/hwmon/ntc
> +        *
> +        * pullup/down_ohm: 0 for infinite / not-connected
> +        */
> +       unsigned int (*read_uV)(void);
> +       unsigned int pullup_uV;
> +
> +       unsigned int pullup_ohm;
> +       unsigned int pulldown_ohm;
> +       enum { NTC_CONNECTED_POSITIVE, NTC_CONNECTED_GROUND } connect;
> +
> +       unsigned int (*read_ohm)(void);
> +};
> +
> +extern int ntc_thermistor_read(struct device *dev);
> +
No longer exported, thus the declaration (and the function) should be removed.

> +#endif /* _LINUX_NTC_H */
> --
> 1.7.1
> 

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

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

* Re: [lm-sensors] [PATCH v3] drivers/hwmon NTC Thermistor Initial
  2010-12-20 10:48 [lm-sensors] [PATCH v3] drivers/hwmon NTC Thermistor Initial MyungJoo Ham
  2010-12-23 20:34 ` Guenter Roeck
@ 2010-12-24  1:36 ` MyungJoo Ham
  2010-12-24  2:08 ` Guenter Roeck
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: MyungJoo Ham @ 2010-12-24  1:36 UTC (permalink / raw)
  To: lm-sensors

On Fri, Dec 24, 2010 at 5:34 AM, Guenter Roeck
<guenter.roeck@ericsson.com> wrote:
>

(snip)

>> +config SENSORS_NTC_THERMISTOR
>
> Should be in alphabetic order, ie before SENSORS_PC87360.
>

(snip)

>                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Text does not provide value. Please remove.
>

(snip)

>
> Please add text showing module name if built as module.
>

(sinp)

> Please put in alphabetic order (before PC87360).
>

(snip)

>
> Still not returning an error.
>

(snip)

>
> Still not returning an error.
>

Those will be done at the next patch. Thanks.

>> +       return INT_MIN;
>> +}
>> +
>> +int ntc_thermistor_read(struct device *dev)
>> +{
>> +       return _ntc_thermistor_read(dev_get_drvdata(dev));
>> +}
>
> Function should be removed since it is no longer exported.
>

I have been considering to use this function at the board file in the
kernel space.
However, if it is removed, the driver may have a weird callback (a) or
the board file
may need to access sysfs filesystem and depends on sysfs (b).

(a): add "int (*read_thermistor)(struct platform_device *pdev)" at
struct ntc_thermistor_platform_data (in include/linux/ntc.h) and let
ntc_thermistor_probe provide callback thru pdata back to the caller of
platform_device_register().

(b): read "/sysfs/devices/platform/blahblah/temp1_input"
and keep /sysfs mounted always.

Are these approaches fine?

(snip)

>> +
>> +       /*
>> +        * The compensation table is sorted by ohm value. Although it is
>> +        * normally assumed that the temperature value monotonically
>> +        * increases or decreases with the ohm value, but there could be
>> +        * different thermistors. However, if we can assume so, we only need
>> +        * to look up the first and the last values.
>
> The last sentence isn't really valuable. Either only look up first and last values
> only, and explain why you do it and if and when the code might have to change,
> or just explain why you loop through the table even if you don't need to right now.

Got it.

>
>> +        */
>> +       for (i = 0; i < data->n_comp; i++) {
>> +               if (data->comp[i].temp_C > data->maxtemp)
>> +                       data->maxtemp = data->comp[i].temp_C;
>> +               if (data->comp[i].temp_C < data->mintemp)
>> +                       data->mintemp = data->comp[i].temp_C;
>> +       }
>> +
>> +       if (data->mintemp != INT_MAX)
>> +               data->mintemp *= 1000;
>> +       if (data->maxtemp != INT_MIN)
>> +               data->maxtemp *= 1000;
>> +
> Doesn't this indicate a logical error in the compensation tables ? If so, you might
> want to do something better than just report INT_MAX for the minimum temperature
> and INT_MIN for the maximum temperature (which doesn't really make sense).
> Maybe have the probe function return an error and fail to install.

Ok, I'll let it report an error for these instances as well.

>
(snip)
>

Thank you so much.



Happy Holidays!

- MyungJoo

-- 
MyungJoo Ham (함명주), Ph.D.
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: 82-10-6714-2858

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

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

* Re: [lm-sensors] [PATCH v3] drivers/hwmon NTC Thermistor Initial
  2010-12-20 10:48 [lm-sensors] [PATCH v3] drivers/hwmon NTC Thermistor Initial MyungJoo Ham
  2010-12-23 20:34 ` Guenter Roeck
  2010-12-24  1:36 ` MyungJoo Ham
@ 2010-12-24  2:08 ` Guenter Roeck
  2010-12-24  2:37 ` 함명주
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Guenter Roeck @ 2010-12-24  2:08 UTC (permalink / raw)
  To: lm-sensors

On Thu, Dec 23, 2010 at 08:36:04PM -0500, MyungJoo Ham wrote:
[ ... ]
> >> +int ntc_thermistor_read(struct device *dev)
> >> +{
> >> +       return _ntc_thermistor_read(dev_get_drvdata(dev));
> >> +}
> >
> > Function should be removed since it is no longer exported.
> >
> 
> I have been considering to use this function at the board file in the
> kernel space.

That means you would have to export the function, and we are back to square one,
ie you really use a hwmon driver to provide functions to a non-hwmon driver.

> However, if it is removed, the driver may have a weird callback (a) or
> the board file
> may need to access sysfs filesystem and depends on sysfs (b).
> 
> (a): add "int (*read_thermistor)(struct platform_device *pdev)" at
> struct ntc_thermistor_platform_data (in include/linux/ntc.h) and let
> ntc_thermistor_probe provide callback thru pdata back to the caller of
> platform_device_register().
> 
> (b): read "/sysfs/devices/platform/blahblah/temp1_input"
> and keep /sysfs mounted always.
> 
> Are these approaches fine?
> 
Again, problem is that in order to have this functionality, you force 
hwmon to be active for the board you are talking about. This is,
no matter how you do it, not the correct approach. Maybe someone wants to
build a kernel with no hwmon support. Then what ? This won't work.

If the adc values need to be used by non-hwmon driver(s), there should be 
an independent driver providing adc values, a hwmon driver to convert the 
adc values as appropriate via the hwmon sysfs ABI, and other drivers as needed,
such as your board driver, doing whatever they want to do with the adc values.

Thanks,
Guenter

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

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

* Re: [lm-sensors] [PATCH v3] drivers/hwmon NTC Thermistor Initial
  2010-12-20 10:48 [lm-sensors] [PATCH v3] drivers/hwmon NTC Thermistor Initial MyungJoo Ham
                   ` (2 preceding siblings ...)
  2010-12-24  2:08 ` Guenter Roeck
@ 2010-12-24  2:37 ` 함명주
  2010-12-24  3:09 ` Guenter Roeck
  2010-12-24  5:42 ` Guenter Roeck
  5 siblings, 0 replies; 7+ messages in thread
From: 함명주 @ 2010-12-24  2:37 UTC (permalink / raw)
  To: lm-sensors


Sender : Guenter Roeck<guenter.roeck@ericsson.com>
> On Thu, Dec 23, 2010 at 08:36:04PM -0500, MyungJoo Ham wrote:
> [ ... ]
> > >> +int ntc_thermistor_read(struct device *dev)
> > >> +{
> > >> + ? ? ? return _ntc_thermistor_read(dev_get_drvdata(dev));
> > >> +}
> > >
> > > Function should be removed since it is no longer exported.
> > >
> > 
> > I have been considering to use this function at the board file in the
> > kernel space.
> 
> That means you would have to export the function, and we are back to square one,
> ie you really use a hwmon driver to provide functions to a non-hwmon driver.
> 

Yes, this hwmon driver is used by another (board file) when CONFIG_HWMON and
CONFIG_NTC_.... is "y". Otherwise, the code that requires hwmon driver is not
compiled and a dummy is used instead. Thus, we don't need EXPORT anyway.

> > However, if it is removed, the driver may have a weird callback (a) or
> > the board file
> > may need to access sysfs filesystem and depends on sysfs (b).
> > 
> > (a): add "int (*read_thermistor)(struct platform_device *pdev)" at
> > struct ntc_thermistor_platform_data (in include/linux/ntc.h) and let
> > ntc_thermistor_probe provide callback thru pdata back to the caller of
> > platform_device_register().
> > 
> > (b): read "/sysfs/devices/platform/blahblah/temp1_input"
> > and keep /sysfs mounted always.
> > 
> > Are these approaches fine?
> > 
> Again, problem is that in order to have this functionality, you force 
> hwmon to be active for the board you are talking about. This is,
> no matter how you do it, not the correct approach. Maybe someone wants to
> build a kernel with no hwmon support. Then what ? This won't work.
> 
> If the adc values need to be used by non-hwmon driver(s), there should be 
> an independent driver providing adc values, a hwmon driver to convert the 
> adc values as appropriate via the hwmon sysfs ABI, and other drivers as needed,
> such as your board driver, doing whatever they want to do with the adc values.

If hwmon and ntc is not active (not y), it won't simply use it. The user will lose
related features, but will be able to run the kernel anyway.

For example, in case (a), we may either skip (by using some #if) registering or
handle error from platform_driver_register properly; just don't start the service/function
that requires ntc. In case (b), it would be almost same with the latter method of case (a).
 
> 
> Thanks,
> Guenter
> 


Cheers!
MyungJoo

 MyungJoo Ham («‘∏Ì¡÷)
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: +82-10-6714-2858 / office: +82-31-279-8033
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

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

* Re: [lm-sensors] [PATCH v3] drivers/hwmon NTC Thermistor Initial
  2010-12-20 10:48 [lm-sensors] [PATCH v3] drivers/hwmon NTC Thermistor Initial MyungJoo Ham
                   ` (3 preceding siblings ...)
  2010-12-24  2:37 ` 함명주
@ 2010-12-24  3:09 ` Guenter Roeck
  2010-12-24  5:42 ` Guenter Roeck
  5 siblings, 0 replies; 7+ messages in thread
From: Guenter Roeck @ 2010-12-24  3:09 UTC (permalink / raw)
  To: lm-sensors

On Thu, Dec 23, 2010 at 09:37:37PM -0500, 함명주 wrote:
> 
> Sender : Guenter Roeck<guenter.roeck@ericsson.com>
> > On Thu, Dec 23, 2010 at 08:36:04PM -0500, MyungJoo Ham wrote:
> > [ ... ]
> > > >> +int ntc_thermistor_read(struct device *dev)
> > > >> +{
> > > >> + ? ? ? return _ntc_thermistor_read(dev_get_drvdata(dev));
> > > >> +}
> > > >
> > > > Function should be removed since it is no longer exported.
> > > >
> > > 
> > > I have been considering to use this function at the board file in the
> > > kernel space.
> > 
> > That means you would have to export the function, and we are back to square one,
> > ie you really use a hwmon driver to provide functions to a non-hwmon driver.
> > 
> 
> Yes, this hwmon driver is used by another (board file) when CONFIG_HWMON and
> CONFIG_NTC_.... is "y". Otherwise, the code that requires hwmon driver is not
> compiled and a dummy is used instead. Thus, we don't need EXPORT anyway.
> 
Have you tried to compile the driver as module ?

Guenter

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

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

* Re: [lm-sensors] [PATCH v3] drivers/hwmon NTC Thermistor Initial
  2010-12-20 10:48 [lm-sensors] [PATCH v3] drivers/hwmon NTC Thermistor Initial MyungJoo Ham
                   ` (4 preceding siblings ...)
  2010-12-24  3:09 ` Guenter Roeck
@ 2010-12-24  5:42 ` Guenter Roeck
  5 siblings, 0 replies; 7+ messages in thread
From: Guenter Roeck @ 2010-12-24  5:42 UTC (permalink / raw)
  To: lm-sensors

On Thu, Dec 23, 2010 at 09:37:37PM -0500, 함명주 wrote:
> 
> Sender : Guenter Roeck<guenter.roeck@ericsson.com>
> > On Thu, Dec 23, 2010 at 08:36:04PM -0500, MyungJoo Ham wrote:
> > [ ... ]
> > > >> +int ntc_thermistor_read(struct device *dev)
> > > >> +{
> > > >> + ? ? ? return _ntc_thermistor_read(dev_get_drvdata(dev));
> > > >> +}
> > > >
> > > > Function should be removed since it is no longer exported.
> > > >
> > > 
> > > I have been considering to use this function at the board file in the
> > > kernel space.
> > 
> > That means you would have to export the function, and we are back to square one,
> > ie you really use a hwmon driver to provide functions to a non-hwmon driver.
> > 
> 
> Yes, this hwmon driver is used by another (board file) when CONFIG_HWMON and
> CONFIG_NTC_.... is "y". Otherwise, the code that requires hwmon driver is not
> compiled and a dummy is used instead. Thus, we don't need EXPORT anyway.
> 
After browsing through the kernel source - such a dependency on CONFIG_HWMON
would be the first in the kernel. I think such a dependency is broken, and
I won't be the one to approve it.

Guenter

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

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

end of thread, other threads:[~2010-12-24  5:42 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-12-20 10:48 [lm-sensors] [PATCH v3] drivers/hwmon NTC Thermistor Initial MyungJoo Ham
2010-12-23 20:34 ` Guenter Roeck
2010-12-24  1:36 ` MyungJoo Ham
2010-12-24  2:08 ` Guenter Roeck
2010-12-24  2:37 ` 함명주
2010-12-24  3:09 ` Guenter Roeck
2010-12-24  5:42 ` Guenter Roeck

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.