From: Guenter Roeck <guenter.roeck@ericsson.com>
To: lm-sensors@vger.kernel.org
Subject: Re: [lm-sensors] [PATCH v4] drivers/hwmon NTC Thermistor Initial
Date: Wed, 08 Jun 2011 15:10:11 +0000 [thread overview]
Message-ID: <20110608151011.GA7292@ericsson.com> (raw)
In-Reply-To: <1307532911-2185-1-git-send-email-dg77.kim@samsung.com>
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
next prev parent reply other threads:[~2011-06-08 15:10 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-06-08 11:35 [lm-sensors] [PATCH v4] drivers/hwmon NTC Thermistor Initial Donggeun Kim
2011-06-08 15:10 ` Guenter Roeck [this message]
2011-06-09 5:38 ` Donggeun Kim
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20110608151011.GA7292@ericsson.com \
--to=guenter.roeck@ericsson.com \
--cc=lm-sensors@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.