* [lm-sensors] [PATCH v4] drivers/hwmon NTC Thermistor Initial
@ 2011-06-08 11:35 Donggeun Kim
2011-06-08 15:10 ` Guenter Roeck
2011-06-09 5:38 ` Donggeun Kim
0 siblings, 2 replies; 3+ messages in thread
From: Donggeun Kim @ 2011-06-08 11:35 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: Donggeun Kim <dg77.kim@samsung.com>
Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: KyungMin Park <kyungmin.park@samsung.com>
--
Updates
v4
- put in alphabetic order in Kconfig/Makefile
- handle additional error cases
- remove the exported function
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 | 101 ++++++++++
drivers/hwmon/Kconfig | 13 ++
drivers/hwmon/Makefile | 1 +
drivers/hwmon/ntc.c | 469 +++++++++++++++++++++++++++++++++++++++++++++++
include/linux/ntc.h | 52 ++++++
5 files changed, 636 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..6c682dd
--- /dev/null
+++ b/Documentation/hwmon/ntc
@@ -0,0 +1,101 @@
+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.
+
+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 16db83c..ebf0b74 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -767,6 +767,19 @@ config SENSORS_MAX6650
This driver can also be built as a module. If so, the module
will be called max6650.
+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, this driver supports
+ NCP15WB473, NCP18WB473, NCP21WB473, NCP03WB473, and NCP15WL333.
+
+ This driver can also be built as a module. If so, the module
+ will be called ntc-thermistor.
+
config SENSORS_PC87360
tristate "National Semiconductor PC87360 family"
select HWMON_VID
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 28061cf..2b9a36a 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -91,6 +91,7 @@ obj-$(CONFIG_SENSORS_MAX6639) += max6639.o
obj-$(CONFIG_SENSORS_MAX6642) += max6642.o
obj-$(CONFIG_SENSORS_MAX6650) += max6650.o
obj-$(CONFIG_SENSORS_MC13783_ADC)+= mc13783-adc.o
+obj-$(CONFIG_SENSORS_NTC_THERMISTOR) += ntc.o
obj-$(CONFIG_SENSORS_PC87360) += pc87360.o
obj-$(CONFIG_SENSORS_PC87427) += pc87427.o
obj-$(CONFIG_SENSORS_PCF8591) += pcf8591.o
diff --git a/drivers/hwmon/ntc.c b/drivers/hwmon/ntc.c
new file mode 100644
index 0000000..532e3d3
--- /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 *temp)
+{
+ int low, high;
+ int ret;
+
+ ret = lookup_comp(data, ohm, &low, &high);
+ if (ret) {
+ /* Unable to use linear approximation */
+ if (low != -1)
+ *temp = data->comp[low].temp_C * 1000;
+ else if (high != -1)
+ *temp = data->comp[high].temp_C * 1000;
+ else
+ return -EINVAL;
+ } else
+ *temp = 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);
+
+ return 0;
+}
+
+static int _ntc_thermistor_read(struct ntc_data *data, int *temp)
+{
+ int ret;
+
+ if (data->pdata->read_ohm)
+ ret = get_temp_mC(data, data->pdata->read_ohm(), temp);
+
+ if (data->pdata->read_uV)
+ ret = get_temp_mC(data, get_ohm_of_thermistor(data,
+ data->pdata->read_uV()), temp);
+ if (ret) {
+ dev_dbg(data->dev, "Sensor reading function not available.\n");
+ return -EINVAL;
+ } else
+ return 0;
+}
+
+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)
+{
+ struct ntc_data *data = dev_get_drvdata(dev);
+ int temp, ret;
+
+ ret = _ntc_thermistor_read(data, &temp);
+ if (ret)
+ return sprintf(buf, "%d\n", ret);
+ 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.
+ */
+ 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->maxtemp != INT_MIN)) {
+ dev_err(data->dev, "error in compensation table\n");
+ ret = -EINVAL;
+ goto err;
+ }
+
+ 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");
+MODULE_ALIAS("ntc-thermistor");
diff --git a/include/linux/ntc.h b/include/linux/ntc.h
new file mode 100644
index 0000000..e194c6b
--- /dev/null
+++ b/include/linux/ntc.h
@@ -0,0 +1,52 @@
+/*
+ * 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);
+};
+
+#endif /* _LINUX_NTC_H */
--
1.7.4.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] 3+ messages in thread
* Re: [lm-sensors] [PATCH v4] drivers/hwmon NTC Thermistor Initial
2011-06-08 11:35 [lm-sensors] [PATCH v4] drivers/hwmon NTC Thermistor Initial Donggeun Kim
@ 2011-06-08 15:10 ` Guenter Roeck
2011-06-09 5:38 ` Donggeun Kim
1 sibling, 0 replies; 3+ messages in thread
From: Guenter Roeck @ 2011-06-08 15:10 UTC (permalink / raw)
To: lm-sensors
On Wed, Jun 08, 2011 at 07:35:11AM -0400, Donggeun Kim 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: Donggeun Kim <dg77.kim@samsung.com>
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> Signed-off-by: KyungMin Park <kyungmin.park@samsung.com>
>
> --
> Updates
> v4
> - put in alphabetic order in Kconfig/Makefile
> - handle additional error cases
> - remove the exported function
> 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 | 101 ++++++++++
> drivers/hwmon/Kconfig | 13 ++
> drivers/hwmon/Makefile | 1 +
> drivers/hwmon/ntc.c | 469 +++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/ntc.h | 52 ++++++
> 5 files changed, 636 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..6c682dd
> --- /dev/null
> +++ b/Documentation/hwmon/ntc
> @@ -0,0 +1,101 @@
> +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.
> +
> +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 16db83c..ebf0b74 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -767,6 +767,19 @@ config SENSORS_MAX6650
> This driver can also be built as a module. If so, the module
> will be called max6650.
>
> +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, this driver supports
> + NCP15WB473, NCP18WB473, NCP21WB473, NCP03WB473, and NCP15WL333.
> +
> + This driver can also be built as a module. If so, the module
> + will be called ntc-thermistor.
> +
> config SENSORS_PC87360
> tristate "National Semiconductor PC87360 family"
> select HWMON_VID
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 28061cf..2b9a36a 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -91,6 +91,7 @@ obj-$(CONFIG_SENSORS_MAX6639) += max6639.o
> obj-$(CONFIG_SENSORS_MAX6642) += max6642.o
> obj-$(CONFIG_SENSORS_MAX6650) += max6650.o
> obj-$(CONFIG_SENSORS_MC13783_ADC)+= mc13783-adc.o
> +obj-$(CONFIG_SENSORS_NTC_THERMISTOR) += ntc.o
> obj-$(CONFIG_SENSORS_PC87360) += pc87360.o
> obj-$(CONFIG_SENSORS_PC87427) += pc87427.o
> obj-$(CONFIG_SENSORS_PCF8591) += pcf8591.o
> diff --git a/drivers/hwmon/ntc.c b/drivers/hwmon/ntc.c
> new file mode 100644
> index 0000000..532e3d3
> --- /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;
else statement is not needed here.
> + }
> + 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;
This implementation is quite expensive. Can you consider using binary search
instead ?
> + }
> + *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 *temp)
> +{
> + int low, high;
> + int ret;
> +
> + ret = lookup_comp(data, ohm, &low, &high);
> + if (ret) {
> + /* Unable to use linear approximation */
> + if (low != -1)
> + *temp = data->comp[low].temp_C * 1000;
> + else if (high != -1)
> + *temp = data->comp[high].temp_C * 1000;
> + else
> + return -EINVAL;
You want to return ret here. Sure, it is the same, but that may change and you want
to return the original error, not replace it.
> + } else
> + *temp = 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);
> +
Please use { } in the else statement as well (CodingStyle, chapter 3).
> + return 0;
> +}
> +
> +static int _ntc_thermistor_read(struct ntc_data *data, int *temp)
> +{
> + int ret;
> +
> + if (data->pdata->read_ohm)
> + ret = get_temp_mC(data, data->pdata->read_ohm(), temp);
> +
> + if (data->pdata->read_uV)
> + ret = get_temp_mC(data, get_ohm_of_thermistor(data,
> + data->pdata->read_uV()), temp);
This overrides the original return value if set. The case should never happen,
so you might want to use else above. Besides, either read_ohm or read_uV must be set,
or ret won't even be initialized. A simple if/else (without checking if read_uV
is actually set) should be sufficient here. And you can improve readability with
int ret;
unsigned int ohm;
if (data->pdata->read_ohm)
ohm = data->pdata->read_ohm();
else
ohm = get_ohm_of_thermistor(data, data->pdata->read_uV());
ret = get_temp_mC(data, ohm, temp);
One concern I have is that read_ohm() and read_uV() don't return errors. This may be true
for the current back-end driver, but may not be true for future drivers. I would suggest
to declare the function return codes as "int" and support (and check) error return values.
> + if (ret) {
> + dev_dbg(data->dev, "Sensor reading function not available.\n");
> + return -EINVAL;
> + } else
> + return 0;
else is not needed here.
> +}
> +
> +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)
> +{
> + struct ntc_data *data = dev_get_drvdata(dev);
> + int temp, ret;
> +
> + ret = _ntc_thermistor_read(data, &temp);
> + if (ret)
> + return sprintf(buf, "%d\n", ret);
You want to return ret here, not the error code converted into a string.
> + 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,
Do _min and _max have any real value here ? What happens if a temperature is outside the
reported range ? Seems all you do is to report the lowest/highest temperatures supported
by the conversion functions, but there is no alarm associated with it nor does it really
have a practical effect.
In other words, all it does is to report the temperature range supported by the driver
for a given chip, which isn't the idea. I would suggest to drop those attributes.
> + &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.
> + */
> + 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->maxtemp != INT_MIN)) {
> + dev_err(data->dev, "error in compensation table\n");
> + ret = -EINVAL;
> + goto err;
> + }
> +
> + 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 */
Odd comment, doesn't provide any value. Please remove.
> + sysfs_remove_group(&data->dev->kobj, &ntc_attr_group);
> +
> + kfree(data);
> +
> + platform_set_drvdata(pdev, NULL);
> +
You want to set this to NULL prior to releasing the memory.
> + 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");
> +MODULE_ALIAS("ntc-thermistor");
> diff --git a/include/linux/ntc.h b/include/linux/ntc.h
> new file mode 100644
> index 0000000..e194c6b
> --- /dev/null
> +++ b/include/linux/ntc.h
> @@ -0,0 +1,52 @@
> +/*
> + * 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);
> +};
> +
> +#endif /* _LINUX_NTC_H */
> --
> 1.7.4.1
>
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [lm-sensors] [PATCH v4] drivers/hwmon NTC Thermistor Initial
2011-06-08 11:35 [lm-sensors] [PATCH v4] drivers/hwmon NTC Thermistor Initial Donggeun Kim
2011-06-08 15:10 ` Guenter Roeck
@ 2011-06-09 5:38 ` Donggeun Kim
1 sibling, 0 replies; 3+ messages in thread
From: Donggeun Kim @ 2011-06-09 5:38 UTC (permalink / raw)
To: lm-sensors
On 2011년 06월 09일 00:10, Guenter Roeck wrote:
(snip)
>
> else statement is not needed here.
>
(snip)
>
> This implementation is quite expensive. Can you consider using binary search
> instead ?
>
I will use binary search at the next version.
(snip)
>
> You want to return ret here. Sure, it is the same, but that may change and you want
> to return the original error, not replace it.
>
(snip)
> Please use { } in the else statement as well (CodingStyle, chapter 3).
>
(snip)
>
> This overrides the original return value if set. The case should never happen,
> so you might want to use else above. Besides, either read_ohm or read_uV must be set,
> or ret won't even be initialized. A simple if/else (without checking if read_uV
> is actually set) should be sufficient here. And you can improve readability with
>
> int ret;
> unsigned int ohm;
>
> if (data->pdata->read_ohm)
> ohm = data->pdata->read_ohm();
> else
> ohm = get_ohm_of_thermistor(data, data->pdata->read_uV());
>
> ret = get_temp_mC(data, ohm, temp);
>
> One concern I have is that read_ohm() and read_uV() don't return errors. This may be true
> for the current back-end driver, but may not be true for future drivers. I would suggest
> to declare the function return codes as "int" and support (and check) error return values.
>
OK, I will fix it.
(snip)
>
> else is not needed here.
>
(snip)
>
> You want to return ret here, not the error code converted into a string.
>
(snip)
>
> Do _min and _max have any real value here ? What happens if a temperature is outside the
> reported range ? Seems all you do is to report the lowest/highest temperatures supported
> by the conversion functions, but there is no alarm associated with it nor does it really
> have a practical effect.
> In other words, all it does is to report the temperature range supported by the driver
> for a given chip, which isn't the idea. I would suggest to drop those attributes.
>
(snip)
>
> Odd comment, doesn't provide any value. Please remove.
>
(snip)
> You want to set this to NULL prior to releasing the memory.
>
I will follow all suggestions at the next patch.
Thank you.
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-06-09 5:38 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-06-08 11:35 [lm-sensors] [PATCH v4] drivers/hwmon NTC Thermistor Initial Donggeun Kim
2011-06-08 15:10 ` Guenter Roeck
2011-06-09 5:38 ` Donggeun Kim
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.