[PATCH 0/3] ARM: EXYNOS4: NURI Support NTC

[PATCH 0/3] ARM: EXYNOS4: NURI Support NTC

[MyungJoo Ham]

This series of patches require NTC thermistor support.
NTC thermistor support patch is submitted and told to be applied (lm-sensors) although it is not being found in -next branches just yet.
You can find the patch at:
http://comments.gmane.org/gmane.linux.drivers.sensors/26475

MyungJoo Ham (3):
  Samsung SoC: NTC Thermistor attached to S3C-ADC.
  Samsung SoC: ready to use NTC value inside kernel
  Exynos4 NURI: support for NTC thermistor

 arch/arm/mach-exynos4/mach-nuri.c            |    4 +
 arch/arm/plat-samsung/dev-adc.c              |  119 ++++++++++++++++++++++++++
 arch/arm/plat-samsung/include/plat/adc-ntc.h |   19 ++++
 arch/arm/plat-samsung/include/plat/devs.h    |    1 +
 4 files changed, 143 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/plat-samsung/include/plat/adc-ntc.h

-- 
1.7.4.1

[PATCH 0/3] ARM: EXYNOS4: NURI Support NTC

[MyungJoo Ham]

This series of patches require NTC thermistor support.
NTC thermistor support patch is submitted and told to be applied (lm-sensors) although it is not being found in -next branches just yet.
You can find the patch at:
http://comments.gmane.org/gmane.linux.drivers.sensors/26475

MyungJoo Ham (3):
  Samsung SoC: NTC Thermistor attached to S3C-ADC.
  Samsung SoC: ready to use NTC value inside kernel
  Exynos4 NURI: support for NTC thermistor

 arch/arm/mach-exynos4/mach-nuri.c            |    4 +
 arch/arm/plat-samsung/dev-adc.c              |  119 ++++++++++++++++++++++++++
 arch/arm/plat-samsung/include/plat/adc-ntc.h |   19 ++++
 arch/arm/plat-samsung/include/plat/devs.h    |    1 +
 4 files changed, 143 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/plat-samsung/include/plat/adc-ntc.h

-- 
1.7.4.1

[PATCH 1/3] Samsung SoC: NTC Thermistor attached to S3C-ADC.

[MyungJoo Ham]

This patch allows platform configurations to share information about
the NTC thermistor device attached to ADC port of S3C.

A platform configuration is required to initialize the ADC port for NTC
thermistor by calling "s3c_adc_ntc_init(portnum);" before registering
the NTC thermistor device. Then, registering a platform device,
s3c_device_adc_ntc_thermistor, enables NTC thermistor for the
platform.

The platform data is configured for Exynos4-Nuri, S5PC110-Aquila, and
S5PC110-Goni. However, if a board happens to have different platform
data setting, we will need to provide different platform data either
by providing a function to update the platform data or by providing
another platform data entry.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/plat-samsung/dev-adc.c           |   57 +++++++++++++++++++++++++++++
 arch/arm/plat-samsung/include/plat/devs.h |    1 +
 2 files changed, 58 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-samsung/dev-adc.c b/arch/arm/plat-samsung/dev-adc.c
index 9d903d4..622972c 100644
--- a/arch/arm/plat-samsung/dev-adc.c
+++ b/arch/arm/plat-samsung/dev-adc.c
@@ -9,9 +9,11 @@
  * published by the Free Software Foundation.
 */
 
+#include <linux/err.h>
 #include <linux/kernel.h>
 #include <linux/string.h>
 #include <linux/platform_device.h>
+#include <linux/platform_data/ntc_thermistor.h>
 
 #include <mach/irqs.h>
 #include <mach/map.h>
@@ -44,3 +46,58 @@ struct platform_device s3c_device_adc = {
 	.num_resources	= ARRAY_SIZE(s3c_adc_resource),
 	.resource	= s3c_adc_resource,
 };
+
+/* NTC Thermistor. Attached to S3C-ADC in some Samsung SoC Devices */
+struct platform_device s3c_device_adc_ntc_thermistor;
+static int ntc_adc_num = -EINVAL; /* Uninitialized */
+static struct s3c_adc_client *ntc_adc;
+
+int __init s3c_adc_ntc_init(int port)
+{
+	int err = 0;
+
+	if (port < 0 || port > 9)
+		return -EINVAL;
+	ntc_adc_num = port;
+
+	ntc_adc = s3c_adc_register(&s3c_device_adc_ntc_thermistor, NULL, NULL,
+				   0);
+	if (IS_ERR(ntc_adc)) {
+		err = PTR_ERR(ntc_adc);
+		ntc_adc = NULL;
+		return err;
+	}
+
+	return 0;
+}
+
+static int read_thermistor_uV(void)
+{
+	int val;
+	s64 converted;
+
+	WARN(ntc_adc == NULL || ntc_adc_num < 0,
+	     "S3C NTC-ADC is not initialized for %s.\n", __func__);
+
+	val = s3c_adc_read(ntc_adc, ntc_adc_num);
+
+	converted = 3300000LL * (s64) val;
+	converted >>= 12;
+
+	return converted;
+}
+
+static struct ntc_thermistor_platform_data ntc_adc_pdata = {
+	.read_uV	= read_thermistor_uV,
+	.pullup_uV	= 3300000, /* voltage of vdd for ADC */
+	.pullup_ohm	= 100000,
+	.pulldown_ohm	= 100000,
+	.connect	= NTC_CONNECTED_GROUND,
+};
+
+struct platform_device s3c_device_adc_ntc_thermistor = {
+	.name			= "ncp15wb473",
+	.dev			= {
+		.platform_data = &ntc_adc_pdata,
+	},
+};
diff --git a/arch/arm/plat-samsung/include/plat/devs.h b/arch/arm/plat-samsung/include/plat/devs.h
index 3c87779..f83b1da 100644
--- a/arch/arm/plat-samsung/include/plat/devs.h
+++ b/arch/arm/plat-samsung/include/plat/devs.h
@@ -61,6 +61,7 @@ extern struct platform_device s3c_device_i2c6;
 extern struct platform_device s3c_device_i2c7;
 extern struct platform_device s3c_device_rtc;
 extern struct platform_device s3c_device_adc;
+extern struct platform_device s3c_device_adc_ntc_thermistor;
 extern struct platform_device s3c_device_sdi;
 extern struct platform_device s3c_device_iis;
 extern struct platform_device s3c_device_hwmon;
-- 
1.7.4.1

[PATCH 1/3] Samsung SoC: NTC Thermistor attached to S3C-ADC.

[MyungJoo Ham]

This patch allows platform configurations to share information about
the NTC thermistor device attached to ADC port of S3C.

A platform configuration is required to initialize the ADC port for NTC
thermistor by calling "s3c_adc_ntc_init(portnum);" before registering
the NTC thermistor device. Then, registering a platform device,
s3c_device_adc_ntc_thermistor, enables NTC thermistor for the
platform.

The platform data is configured for Exynos4-Nuri, S5PC110-Aquila, and
S5PC110-Goni. However, if a board happens to have different platform
data setting, we will need to provide different platform data either
by providing a function to update the platform data or by providing
another platform data entry.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/plat-samsung/dev-adc.c           |   57 +++++++++++++++++++++++++++++
 arch/arm/plat-samsung/include/plat/devs.h |    1 +
 2 files changed, 58 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-samsung/dev-adc.c b/arch/arm/plat-samsung/dev-adc.c
index 9d903d4..622972c 100644
--- a/arch/arm/plat-samsung/dev-adc.c
+++ b/arch/arm/plat-samsung/dev-adc.c
@@ -9,9 +9,11 @@
  * published by the Free Software Foundation.
 */
 
+#include <linux/err.h>
 #include <linux/kernel.h>
 #include <linux/string.h>
 #include <linux/platform_device.h>
+#include <linux/platform_data/ntc_thermistor.h>
 
 #include <mach/irqs.h>
 #include <mach/map.h>
@@ -44,3 +46,58 @@ struct platform_device s3c_device_adc = {
 	.num_resources	= ARRAY_SIZE(s3c_adc_resource),
 	.resource	= s3c_adc_resource,
 };
+
+/* NTC Thermistor. Attached to S3C-ADC in some Samsung SoC Devices */
+struct platform_device s3c_device_adc_ntc_thermistor;
+static int ntc_adc_num = -EINVAL; /* Uninitialized */
+static struct s3c_adc_client *ntc_adc;
+
+int __init s3c_adc_ntc_init(int port)
+{
+	int err = 0;
+
+	if (port < 0 || port > 9)
+		return -EINVAL;
+	ntc_adc_num = port;
+
+	ntc_adc = s3c_adc_register(&s3c_device_adc_ntc_thermistor, NULL, NULL,
+				   0);
+	if (IS_ERR(ntc_adc)) {
+		err = PTR_ERR(ntc_adc);
+		ntc_adc = NULL;
+		return err;
+	}
+
+	return 0;
+}
+
+static int read_thermistor_uV(void)
+{
+	int val;
+	s64 converted;
+
+	WARN(ntc_adc == NULL || ntc_adc_num < 0,
+	     "S3C NTC-ADC is not initialized for %s.\n", __func__);
+
+	val = s3c_adc_read(ntc_adc, ntc_adc_num);
+
+	converted = 3300000LL * (s64) val;
+	converted >>= 12;
+
+	return converted;
+}
+
+static struct ntc_thermistor_platform_data ntc_adc_pdata = {
+	.read_uV	= read_thermistor_uV,
+	.pullup_uV	= 3300000, /* voltage of vdd for ADC */
+	.pullup_ohm	= 100000,
+	.pulldown_ohm	= 100000,
+	.connect	= NTC_CONNECTED_GROUND,
+};
+
+struct platform_device s3c_device_adc_ntc_thermistor = {
+	.name			= "ncp15wb473",
+	.dev			= {
+		.platform_data = &ntc_adc_pdata,
+	},
+};
diff --git a/arch/arm/plat-samsung/include/plat/devs.h b/arch/arm/plat-samsung/include/plat/devs.h
index 3c87779..f83b1da 100644
--- a/arch/arm/plat-samsung/include/plat/devs.h
+++ b/arch/arm/plat-samsung/include/plat/devs.h
@@ -61,6 +61,7 @@ extern struct platform_device s3c_device_i2c6;
 extern struct platform_device s3c_device_i2c7;
 extern struct platform_device s3c_device_rtc;
 extern struct platform_device s3c_device_adc;
+extern struct platform_device s3c_device_adc_ntc_thermistor;
 extern struct platform_device s3c_device_sdi;
 extern struct platform_device s3c_device_iis;
 extern struct platform_device s3c_device_hwmon;
-- 
1.7.4.1

[PATCH 2/3] Samsung SoC: ready to use NTC value inside kernel

[MyungJoo Ham]

This patch allows kernel codes to use values from NTC LM-Sensor
driver, which allows SYSFS access only.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/plat-samsung/dev-adc.c              |   62 ++++++++++++++++++++++++++
 arch/arm/plat-samsung/include/plat/adc-ntc.h |   19 ++++++++
 2 files changed, 81 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/plat-samsung/include/plat/adc-ntc.h

diff --git a/arch/arm/plat-samsung/dev-adc.c b/arch/arm/plat-samsung/dev-adc.c
index 622972c..526097a 100644
--- a/arch/arm/plat-samsung/dev-adc.c
+++ b/arch/arm/plat-samsung/dev-adc.c
@@ -22,6 +22,8 @@
 #include <plat/devs.h>
 #include <plat/cpu.h>
 
+#include "../../../fs/sysfs/sysfs.h"
+
 static struct resource s3c_adc_resource[] = {
 	[0] = {
 		.start = SAMSUNG_PA_ADC,
@@ -101,3 +103,63 @@ struct platform_device s3c_device_adc_ntc_thermistor = {
 		.platform_data = &ntc_adc_pdata,
 	},
 };
+
+static struct device_attribute *ntc_attr;
+
+static int init_s3c_adc_ntc_read(void)
+{
+	struct kobject *ntc;
+	struct sysfs_dirent *ntc_d;
+
+	ntc = &s3c_device_adc_ntc_thermistor.dev.kobj;
+	ntc_d = sysfs_get_dirent(ntc->sd, get_ktype(ntc)->namespace(ntc),
+				 "temp1_input");
+	if (!ntc_d || sysfs_type(ntc_d) != SYSFS_KOBJ_ATTR) {
+		dev_err(&s3c_device_adc_ntc_thermistor.dev,
+			"Cannot initialize thermistor dirent info.\n");
+		if (ntc_d)
+			sysfs_put(ntc_d);
+		return -ENODEV;
+	}
+	ntc_attr = container_of(ntc_d->s_attr.attr, struct device_attribute,
+				attr);
+
+	sysfs_put(ntc_d);
+	if (IS_ERR(ntc_attr)) {
+		dev_err(&s3c_device_adc_ntc_thermistor.dev,
+			"Cannot access NTC thermistor.\n");
+		return PTR_ERR(ntc_attr);
+	}
+
+	return 0;
+}
+
+/* A helper function to read values from NTC, in 1/1000 Centigrade */
+int read_s3c_adc_ntc(int *mC)
+{
+	char buf[32];
+	int ret;
+
+	/* init should be done after ADC and NTC are probed */
+	if (ntc_attr == NULL) {
+		ret = init_s3c_adc_ntc_read();
+		if (ret) {
+			if (ntc_attr == NULL)
+				ntc_attr = ERR_PTR(ret);
+			return ret;
+		}
+	}
+
+	if (IS_ERR(ntc_attr))
+		return -ENODEV;
+
+	if (!ntc_attr->show)
+		return -EINVAL;
+
+	ret = ntc_attr->show(&s3c_device_adc_ntc_thermistor.dev, ntc_attr, buf);
+	if (ret < 0)
+		return ret;
+	sscanf(buf, "%d", mC);
+
+	return 0;
+}
diff --git a/arch/arm/plat-samsung/include/plat/adc-ntc.h b/arch/arm/plat-samsung/include/plat/adc-ntc.h
new file mode 100644
index 0000000..3d74118
--- /dev/null
+++ b/arch/arm/plat-samsung/include/plat/adc-ntc.h
@@ -0,0 +1,19 @@
+/* linux/arch/arm/plat-samsung/include/plat/adc-ntc.h
+ *
+ * Copyright (c) 2011 Samsung Electronics Co., Ltd.
+ *		http://www.samsung.com/
+ *
+ * NTC Thermistor attached to Samsung ADC Controller driver information
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+*/
+
+#ifndef __PLAT_ADC_NTC_H
+#define __PLAT_ADC_NTC_H __FILE__
+
+extern void s3c_adc_ntc_init(int port);
+extern int read_s3c_adc_ntc(int *mC);
+
+#endif /* __PLAT_ADC_NTC_H */
-- 
1.7.4.1

[PATCH 2/3] Samsung SoC: ready to use NTC value inside kernel

[MyungJoo Ham]

This patch allows kernel codes to use values from NTC LM-Sensor
driver, which allows SYSFS access only.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/plat-samsung/dev-adc.c              |   62 ++++++++++++++++++++++++++
 arch/arm/plat-samsung/include/plat/adc-ntc.h |   19 ++++++++
 2 files changed, 81 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/plat-samsung/include/plat/adc-ntc.h

diff --git a/arch/arm/plat-samsung/dev-adc.c b/arch/arm/plat-samsung/dev-adc.c
index 622972c..526097a 100644
--- a/arch/arm/plat-samsung/dev-adc.c
+++ b/arch/arm/plat-samsung/dev-adc.c
@@ -22,6 +22,8 @@
 #include <plat/devs.h>
 #include <plat/cpu.h>
 
+#include "../../../fs/sysfs/sysfs.h"
+
 static struct resource s3c_adc_resource[] = {
 	[0] = {
 		.start = SAMSUNG_PA_ADC,
@@ -101,3 +103,63 @@ struct platform_device s3c_device_adc_ntc_thermistor = {
 		.platform_data = &ntc_adc_pdata,
 	},
 };
+
+static struct device_attribute *ntc_attr;
+
+static int init_s3c_adc_ntc_read(void)
+{
+	struct kobject *ntc;
+	struct sysfs_dirent *ntc_d;
+
+	ntc = &s3c_device_adc_ntc_thermistor.dev.kobj;
+	ntc_d = sysfs_get_dirent(ntc->sd, get_ktype(ntc)->namespace(ntc),
+				 "temp1_input");
+	if (!ntc_d || sysfs_type(ntc_d) != SYSFS_KOBJ_ATTR) {
+		dev_err(&s3c_device_adc_ntc_thermistor.dev,
+			"Cannot initialize thermistor dirent info.\n");
+		if (ntc_d)
+			sysfs_put(ntc_d);
+		return -ENODEV;
+	}
+	ntc_attr = container_of(ntc_d->s_attr.attr, struct device_attribute,
+				attr);
+
+	sysfs_put(ntc_d);
+	if (IS_ERR(ntc_attr)) {
+		dev_err(&s3c_device_adc_ntc_thermistor.dev,
+			"Cannot access NTC thermistor.\n");
+		return PTR_ERR(ntc_attr);
+	}
+
+	return 0;
+}
+
+/* A helper function to read values from NTC, in 1/1000 Centigrade */
+int read_s3c_adc_ntc(int *mC)
+{
+	char buf[32];
+	int ret;
+
+	/* init should be done after ADC and NTC are probed */
+	if (ntc_attr == NULL) {
+		ret = init_s3c_adc_ntc_read();
+		if (ret) {
+			if (ntc_attr == NULL)
+				ntc_attr = ERR_PTR(ret);
+			return ret;
+		}
+	}
+
+	if (IS_ERR(ntc_attr))
+		return -ENODEV;
+
+	if (!ntc_attr->show)
+		return -EINVAL;
+
+	ret = ntc_attr->show(&s3c_device_adc_ntc_thermistor.dev, ntc_attr, buf);
+	if (ret < 0)
+		return ret;
+	sscanf(buf, "%d", mC);
+
+	return 0;
+}
diff --git a/arch/arm/plat-samsung/include/plat/adc-ntc.h b/arch/arm/plat-samsung/include/plat/adc-ntc.h
new file mode 100644
index 0000000..3d74118
--- /dev/null
+++ b/arch/arm/plat-samsung/include/plat/adc-ntc.h
@@ -0,0 +1,19 @@
+/* linux/arch/arm/plat-samsung/include/plat/adc-ntc.h
+ *
+ * Copyright (c) 2011 Samsung Electronics Co., Ltd.
+ *		http://www.samsung.com/
+ *
+ * NTC Thermistor attached to Samsung ADC Controller driver information
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+*/
+
+#ifndef __PLAT_ADC_NTC_H
+#define __PLAT_ADC_NTC_H __FILE__
+
+extern void s3c_adc_ntc_init(int port);
+extern int read_s3c_adc_ntc(int *mC);
+
+#endif /* __PLAT_ADC_NTC_H */
-- 
1.7.4.1

Re: [PATCH 2/3] Samsung SoC: ready to use NTC value inside kernel

[Russell King - ARM Linux]

On Thu, Jun 30, 2011 at 05:26:26PM +0900, MyungJoo Ham wrote:
> diff --git a/arch/arm/plat-samsung/dev-adc.c b/arch/arm/plat-samsung/dev-adc.c
> index 622972c..526097a 100644
> --- a/arch/arm/plat-samsung/dev-adc.c
> +++ b/arch/arm/plat-samsung/dev-adc.c
> @@ -22,6 +22,8 @@
>  #include <plat/devs.h>
>  #include <plat/cpu.h>
>  
> +#include "../../../fs/sysfs/sysfs.h"

That is a big hint that you're doing something wrong.

> @@ -101,3 +103,63 @@ struct platform_device s3c_device_adc_ntc_thermistor = {
>  		.platform_data = &ntc_adc_pdata,
>  	},
>  };
> +
> +static struct device_attribute *ntc_attr;
> +
> +static int init_s3c_adc_ntc_read(void)
> +{
> +	struct kobject *ntc;
> +	struct sysfs_dirent *ntc_d;
> +
> +	ntc = &s3c_device_adc_ntc_thermistor.dev.kobj;
> +	ntc_d = sysfs_get_dirent(ntc->sd, get_ktype(ntc)->namespace(ntc),
> +				 "temp1_input");
> +	if (!ntc_d || sysfs_type(ntc_d) != SYSFS_KOBJ_ATTR) {
> +		dev_err(&s3c_device_adc_ntc_thermistor.dev,
> +			"Cannot initialize thermistor dirent info.\n");
> +		if (ntc_d)
> +			sysfs_put(ntc_d);
> +		return -ENODEV;
> +	}
> +	ntc_attr = container_of(ntc_d->s_attr.attr, struct device_attribute,
> +				attr);
> +
> +	sysfs_put(ntc_d);
> +	if (IS_ERR(ntc_attr)) {
> +		dev_err(&s3c_device_adc_ntc_thermistor.dev,
> +			"Cannot access NTC thermistor.\n");
> +		return PTR_ERR(ntc_attr);
> +	}
> +
> +	return 0;
> +}
> +
> +/* A helper function to read values from NTC, in 1/1000 Centigrade */
> +int read_s3c_adc_ntc(int *mC)
> +{
> +	char buf[32];
> +	int ret;
> +
> +	/* init should be done after ADC and NTC are probed */
> +	if (ntc_attr == NULL) {
> +		ret = init_s3c_adc_ntc_read();
> +		if (ret) {
> +			if (ntc_attr == NULL)
> +				ntc_attr = ERR_PTR(ret);
> +			return ret;
> +		}
> +	}
> +
> +	if (IS_ERR(ntc_attr))
> +		return -ENODEV;
> +
> +	if (!ntc_attr->show)
> +		return -EINVAL;
> +
> +	ret = ntc_attr->show(&s3c_device_adc_ntc_thermistor.dev, ntc_attr, buf);
> +	if (ret < 0)
> +		return ret;
> +	sscanf(buf, "%d", mC);
> +
> +	return 0;
> +}

This is wrong on just about every level.  It needs to be rewritten to
avoid poking about in subsystem internal data structures, and you really
should not be sprint-ing a value to only sscanf it later.

Plus, container_of doesn't return a pointer-error code.

You need to come up with a far better way of doing this altogether.

[PATCH 2/3] Samsung SoC: ready to use NTC value inside kernel

[Russell King - ARM Linux]

On Thu, Jun 30, 2011 at 05:26:26PM +0900, MyungJoo Ham wrote:
> diff --git a/arch/arm/plat-samsung/dev-adc.c b/arch/arm/plat-samsung/dev-adc.c
> index 622972c..526097a 100644
> --- a/arch/arm/plat-samsung/dev-adc.c
> +++ b/arch/arm/plat-samsung/dev-adc.c
> @@ -22,6 +22,8 @@
>  #include <plat/devs.h>
>  #include <plat/cpu.h>
>  
> +#include "../../../fs/sysfs/sysfs.h"

That is a big hint that you're doing something wrong.

> @@ -101,3 +103,63 @@ struct platform_device s3c_device_adc_ntc_thermistor = {
>  		.platform_data = &ntc_adc_pdata,
>  	},
>  };
> +
> +static struct device_attribute *ntc_attr;
> +
> +static int init_s3c_adc_ntc_read(void)
> +{
> +	struct kobject *ntc;
> +	struct sysfs_dirent *ntc_d;
> +
> +	ntc = &s3c_device_adc_ntc_thermistor.dev.kobj;
> +	ntc_d = sysfs_get_dirent(ntc->sd, get_ktype(ntc)->namespace(ntc),
> +				 "temp1_input");
> +	if (!ntc_d || sysfs_type(ntc_d) != SYSFS_KOBJ_ATTR) {
> +		dev_err(&s3c_device_adc_ntc_thermistor.dev,
> +			"Cannot initialize thermistor dirent info.\n");
> +		if (ntc_d)
> +			sysfs_put(ntc_d);
> +		return -ENODEV;
> +	}
> +	ntc_attr = container_of(ntc_d->s_attr.attr, struct device_attribute,
> +				attr);
> +
> +	sysfs_put(ntc_d);
> +	if (IS_ERR(ntc_attr)) {
> +		dev_err(&s3c_device_adc_ntc_thermistor.dev,
> +			"Cannot access NTC thermistor.\n");
> +		return PTR_ERR(ntc_attr);
> +	}
> +
> +	return 0;
> +}
> +
> +/* A helper function to read values from NTC, in 1/1000 Centigrade */
> +int read_s3c_adc_ntc(int *mC)
> +{
> +	char buf[32];
> +	int ret;
> +
> +	/* init should be done after ADC and NTC are probed */
> +	if (ntc_attr == NULL) {
> +		ret = init_s3c_adc_ntc_read();
> +		if (ret) {
> +			if (ntc_attr == NULL)
> +				ntc_attr = ERR_PTR(ret);
> +			return ret;
> +		}
> +	}
> +
> +	if (IS_ERR(ntc_attr))
> +		return -ENODEV;
> +
> +	if (!ntc_attr->show)
> +		return -EINVAL;
> +
> +	ret = ntc_attr->show(&s3c_device_adc_ntc_thermistor.dev, ntc_attr, buf);
> +	if (ret < 0)
> +		return ret;
> +	sscanf(buf, "%d", mC);
> +
> +	return 0;
> +}

This is wrong on just about every level.  It needs to be rewritten to
avoid poking about in subsystem internal data structures, and you really
should not be sprint-ing a value to only sscanf it later.

Plus, container_of doesn't return a pointer-error code.

You need to come up with a far better way of doing this altogether.

Re: [PATCH 2/3] Samsung SoC: ready to use NTC value inside kernel

[MyungJoo Ham]

On Thu, Jun 30, 2011 at 6:00 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Thu, Jun 30, 2011 at 05:26:26PM +0900, MyungJoo Ham wrote:
>> diff --git a/arch/arm/plat-samsung/dev-adc.c b/arch/arm/plat-samsung/dev-adc.c
>> index 622972c..526097a 100644
>> --- a/arch/arm/plat-samsung/dev-adc.c
>> +++ b/arch/arm/plat-samsung/dev-adc.c
>> @@ -22,6 +22,8 @@
>>  #include <plat/devs.h>
>>  #include <plat/cpu.h>
>>
>> +#include "../../../fs/sysfs/sysfs.h"
>
> That is a big hint that you're doing something wrong.
>
[]
>
> This is wrong on just about every level.  It needs to be rewritten to
> avoid poking about in subsystem internal data structures, and you really
> should not be sprint-ing a value to only sscanf it later.
>
> Plus, container_of doesn't return a pointer-error code.
>
> You need to come up with a far better way of doing this altogether.
>

Um.. is there any clean and nice way to read values that are exported to sysfs?

Or if the values are only exported through sysfs (HWMON), is the only
clean way to read such values (w/o extending HWMON itself) to seperate
the HWMON device driver into two pieces: a platform driver that
provide values to somewhere sharable in kernel and a HWMON driver that
reads the values and exports them to sysfs?



Thanks.

- MyungJoo

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

[PATCH 2/3] Samsung SoC: ready to use NTC value inside kernel

[MyungJoo Ham]

On Thu, Jun 30, 2011 at 6:00 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Thu, Jun 30, 2011 at 05:26:26PM +0900, MyungJoo Ham wrote:
>> diff --git a/arch/arm/plat-samsung/dev-adc.c b/arch/arm/plat-samsung/dev-adc.c
>> index 622972c..526097a 100644
>> --- a/arch/arm/plat-samsung/dev-adc.c
>> +++ b/arch/arm/plat-samsung/dev-adc.c
>> @@ -22,6 +22,8 @@
>> ?#include <plat/devs.h>
>> ?#include <plat/cpu.h>
>>
>> +#include "../../../fs/sysfs/sysfs.h"
>
> That is a big hint that you're doing something wrong.
>
[]
>
> This is wrong on just about every level. ?It needs to be rewritten to
> avoid poking about in subsystem internal data structures, and you really
> should not be sprint-ing a value to only sscanf it later.
>
> Plus, container_of doesn't return a pointer-error code.
>
> You need to come up with a far better way of doing this altogether.
>

Um.. is there any clean and nice way to read values that are exported to sysfs?

Or if the values are only exported through sysfs (HWMON), is the only
clean way to read such values (w/o extending HWMON itself) to seperate
the HWMON device driver into two pieces: a platform driver that
provide values to somewhere sharable in kernel and a HWMON driver that
reads the values and exports them to sysfs?



Thanks.

- MyungJoo

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

Re: [PATCH 2/3] Samsung SoC: ready to use NTC value inside kernel

[Linus Walleij]

On Fri, Jul 1, 2011 at 2:28 AM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:

> Or if the values are only exported through sysfs (HWMON), is the only
> clean way to read such values (w/o extending HWMON itself) to seperate
> the HWMON device driver into two pieces: a platform driver that
> provide values to somewhere sharable in kernel and a HWMON driver that
> reads the values and exports them to sysfs?

Sounds like you could very well separate out the ADC part and have
functions for reading values off them? Compare
drivers/mfd/ab8500-gpadc.c for example, this will be used by a
yet to be submitted HWMON driver for doing ADC.

There is no clean nice ADC subsystem though, and *that* feels
like a big problem to drivers like this (and no I don't like the AB8500
GPADC living in MFD either) so if you'd like to create a ADC
subsystem just go ahead, there is something in drivers/staging/iio/adc
but that had another problem last time I looked: it was only intended
for userspace control, not in-kernel use.

Linus Walleij

[PATCH 2/3] Samsung SoC: ready to use NTC value inside kernel

[Linus Walleij]

On Fri, Jul 1, 2011 at 2:28 AM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:

> Or if the values are only exported through sysfs (HWMON), is the only
> clean way to read such values (w/o extending HWMON itself) to seperate
> the HWMON device driver into two pieces: a platform driver that
> provide values to somewhere sharable in kernel and a HWMON driver that
> reads the values and exports them to sysfs?

Sounds like you could very well separate out the ADC part and have
functions for reading values off them? Compare
drivers/mfd/ab8500-gpadc.c for example, this will be used by a
yet to be submitted HWMON driver for doing ADC.

There is no clean nice ADC subsystem though, and *that* feels
like a big problem to drivers like this (and no I don't like the AB8500
GPADC living in MFD either) so if you'd like to create a ADC
subsystem just go ahead, there is something in drivers/staging/iio/adc
but that had another problem last time I looked: it was only intended
for userspace control, not in-kernel use.

Linus Walleij

Re: [PATCH 2/3] Samsung SoC: ready to use NTC value inside kernel

[Mark Brown]

On Fri, Jul 01, 2011 at 05:15:05PM +0200, Linus Walleij wrote:

> There is no clean nice ADC subsystem though, and *that* feels
> like a big problem to drivers like this (and no I don't like the AB8500
> GPADC living in MFD either) so if you'd like to create a ADC
> subsystem just go ahead, there is something in drivers/staging/iio/adc
> but that had another problem last time I looked: it was only intended
> for userspace control, not in-kernel use.

The other issue is that it's designed for high volume data and these
AUXADCs tend to be relatively slow.  That said I've had some discussions
with Jonathan about supporting these devices via IIO and he seems to be
coming round to the idea that we could integrate support.  Not sure how
fast IIO is progressing to mainline, though.

[PATCH 2/3] Samsung SoC: ready to use NTC value inside kernel

[Mark Brown]

On Fri, Jul 01, 2011 at 05:15:05PM +0200, Linus Walleij wrote:

> There is no clean nice ADC subsystem though, and *that* feels
> like a big problem to drivers like this (and no I don't like the AB8500
> GPADC living in MFD either) so if you'd like to create a ADC
> subsystem just go ahead, there is something in drivers/staging/iio/adc
> but that had another problem last time I looked: it was only intended
> for userspace control, not in-kernel use.

The other issue is that it's designed for high volume data and these
AUXADCs tend to be relatively slow.  That said I've had some discussions
with Jonathan about supporting these devices via IIO and he seems to be
coming round to the idea that we could integrate support.  Not sure how
fast IIO is progressing to mainline, though.

Re: [PATCH 2/3] Samsung SoC: ready to use NTC value inside kernel

[Arnd Bergmann]

On Friday 01 July 2011, Mark Brown wrote:
> On Fri, Jul 01, 2011 at 05:15:05PM +0200, Linus Walleij wrote:
> 
> > There is no clean nice ADC subsystem though, and that feels
> > like a big problem to drivers like this (and no I don't like the AB8500
> > GPADC living in MFD either) so if you'd like to create a ADC
> > subsystem just go ahead, there is something in drivers/staging/iio/adc
> > but that had another problem last time I looked: it was only intended
> > for userspace control, not in-kernel use.

Agreed.

> The other issue is that it's designed for high volume data and these
> AUXADCs tend to be relatively slow.  That said I've had some discussions
> with Jonathan about supporting these devices via IIO and he seems to be
> coming round to the idea that we could integrate support.  Not sure how
> fast IIO is progressing to mainline, though.

I think IIO is progressing well and I would rather base products on that
while it's in staging than having a random out-of-tree driver. It seems
to fit the purpose of this driver, though not the in-kernel uses that
Linus mentioned above.

We have a bunch of things that seem to me like they should live together:
gpio, pinmux, pwm and adc. There are ongoing discussions about pinmux
and pwm subsystems right now, so it might be the right time to discuss
where we are going with these in general.

With regard to IIO, that could live on a higher level and just provide
a user interface on top of the generic in-kernel abstraction for adc.

	Arnd

[PATCH 2/3] Samsung SoC: ready to use NTC value inside kernel

[Arnd Bergmann]

On Friday 01 July 2011, Mark Brown wrote:
> On Fri, Jul 01, 2011 at 05:15:05PM +0200, Linus Walleij wrote:
> 
> > There is no clean nice ADC subsystem though, and that feels
> > like a big problem to drivers like this (and no I don't like the AB8500
> > GPADC living in MFD either) so if you'd like to create a ADC
> > subsystem just go ahead, there is something in drivers/staging/iio/adc
> > but that had another problem last time I looked: it was only intended
> > for userspace control, not in-kernel use.

Agreed.

> The other issue is that it's designed for high volume data and these
> AUXADCs tend to be relatively slow.  That said I've had some discussions
> with Jonathan about supporting these devices via IIO and he seems to be
> coming round to the idea that we could integrate support.  Not sure how
> fast IIO is progressing to mainline, though.

I think IIO is progressing well and I would rather base products on that
while it's in staging than having a random out-of-tree driver. It seems
to fit the purpose of this driver, though not the in-kernel uses that
Linus mentioned above.

We have a bunch of things that seem to me like they should live together:
gpio, pinmux, pwm and adc. There are ongoing discussions about pinmux
and pwm subsystems right now, so it might be the right time to discuss
where we are going with these in general.

With regard to IIO, that could live on a higher level and just provide
a user interface on top of the generic in-kernel abstraction for adc.

	Arnd

Re: [PATCH 2/3] Samsung SoC: ready to use NTC value inside kernel

[Mark Brown]

On Fri, Jul 01, 2011 at 07:23:13PM +0200, Arnd Bergmann wrote:
> On Friday 01 July 2011, Mark Brown wrote:

> > The other issue is that it's designed for high volume data and these
> > AUXADCs tend to be relatively slow.  That said I've had some discussions
> > with Jonathan about supporting these devices via IIO and he seems to be
> > coming round to the idea that we could integrate support.  Not sure how
> > fast IIO is progressing to mainline, though.

> I think IIO is progressing well and I would rather base products on that
> while it's in staging than having a random out-of-tree driver. It seems

It's certainly progressing well - looks really nice right now.

> to fit the purpose of this driver, though not the in-kernel uses that
> Linus mentioned above.

The in kernel uses are the main usage of this sort of ADC in most
systems - you get a general purpose ADC used for (usually) voltage and
temperature monitoring with a few auxiliary inputs available for random
system specific things but generally not actually used so often (jack
detection stuff is the main use case).  The power supply and hwmon
subsystems are the main users.

> With regard to IIO, that could live on a higher level and just provide
> a user interface on top of the generic in-kernel abstraction for adc.

Yeah, that's roughly the option that Jonathan and myself had been
discussing last time this came up - have the convertor level deliver
events to a pluggable API layer with the IIO userspace be just one of
those plugins.

[PATCH 2/3] Samsung SoC: ready to use NTC value inside kernel

[Mark Brown]

On Fri, Jul 01, 2011 at 07:23:13PM +0200, Arnd Bergmann wrote:
> On Friday 01 July 2011, Mark Brown wrote:

> > The other issue is that it's designed for high volume data and these
> > AUXADCs tend to be relatively slow.  That said I've had some discussions
> > with Jonathan about supporting these devices via IIO and he seems to be
> > coming round to the idea that we could integrate support.  Not sure how
> > fast IIO is progressing to mainline, though.

> I think IIO is progressing well and I would rather base products on that
> while it's in staging than having a random out-of-tree driver. It seems

It's certainly progressing well - looks really nice right now.

> to fit the purpose of this driver, though not the in-kernel uses that
> Linus mentioned above.

The in kernel uses are the main usage of this sort of ADC in most
systems - you get a general purpose ADC used for (usually) voltage and
temperature monitoring with a few auxiliary inputs available for random
system specific things but generally not actually used so often (jack
detection stuff is the main use case).  The power supply and hwmon
subsystems are the main users.

> With regard to IIO, that could live on a higher level and just provide
> a user interface on top of the generic in-kernel abstraction for adc.

Yeah, that's roughly the option that Jonathan and myself had been
discussing last time this came up - have the convertor level deliver
events to a pluggable API layer with the IIO userspace be just one of
those plugins.

[PATCH 3/3] Exynos4 NURI: support for NTC thermistor

[MyungJoo Ham]

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/mach-exynos4/mach-nuri.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-exynos4/mach-nuri.c b/arch/arm/mach-exynos4/mach-nuri.c
index bdf7a5c..cd8bc06 100644
--- a/arch/arm/mach-exynos4/mach-nuri.c
+++ b/arch/arm/mach-exynos4/mach-nuri.c
@@ -40,6 +40,7 @@
 #include <plat/clock.h>
 #include <plat/gpio-cfg.h>
 #include <plat/iic.h>
+#include <plat/adc-ntc.h>
 
 #include <mach/map.h>
 
@@ -1098,6 +1099,7 @@ static struct platform_device *nuri_devices[] __initdata = {
 	&s3c_device_i2c3,
 	&i2c9_gpio,
 	&s3c_device_adc,
+	&s3c_device_adc_ntc_thermistor,
 	&s3c_device_rtc,
 
 	/* NURI Devices */
@@ -1133,6 +1135,8 @@ static void __init nuri_machine_init(void)
 	nuri_ehci_init();
 	clk_xusbxti.rate = 24000000;
 
+	s3c_adc_ntc_init(6); /* NTC Thermistor uses ADC port 6 */
+
 	/* Last */
 	platform_add_devices(nuri_devices, ARRAY_SIZE(nuri_devices));
 }
-- 
1.7.4.1

[PATCH 3/3] Exynos4 NURI: support for NTC thermistor

[MyungJoo Ham]

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/mach-exynos4/mach-nuri.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-exynos4/mach-nuri.c b/arch/arm/mach-exynos4/mach-nuri.c
index bdf7a5c..cd8bc06 100644
--- a/arch/arm/mach-exynos4/mach-nuri.c
+++ b/arch/arm/mach-exynos4/mach-nuri.c
@@ -40,6 +40,7 @@
 #include <plat/clock.h>
 #include <plat/gpio-cfg.h>
 #include <plat/iic.h>
+#include <plat/adc-ntc.h>
 
 #include <mach/map.h>
 
@@ -1098,6 +1099,7 @@ static struct platform_device *nuri_devices[] __initdata = {
 	&s3c_device_i2c3,
 	&i2c9_gpio,
 	&s3c_device_adc,
+	&s3c_device_adc_ntc_thermistor,
 	&s3c_device_rtc,
 
 	/* NURI Devices */
@@ -1133,6 +1135,8 @@ static void __init nuri_machine_init(void)
 	nuri_ehci_init();
 	clk_xusbxti.rate = 24000000;
 
+	s3c_adc_ntc_init(6); /* NTC Thermistor uses ADC port 6 */
+
 	/* Last */
 	platform_add_devices(nuri_devices, ARRAY_SIZE(nuri_devices));
 }
-- 
1.7.4.1