* 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