| Message ID | 1308622033-2521-5-git-send-email-myungjoo.ham@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jun 21, 2011 at 11:07:13AM +0900, MyungJoo Ham wrote: > +#include <linux/platform_data/ntc_thermistor.h> This doesn't appear to be in mainline. > +/* NTC Thermistor */ > +static struct platform_device nuri_ncp15wb473_thermistor; > +static int read_thermistor_uV(void) Blank line between these two. > +{ > + static struct s3c_adc_client *adc; > + int val; > + s64 converted; > + > + if (!adc) { > + adc = s3c_adc_register(&nuri_ncp15wb473_thermistor, > + NULL, NULL, 0); > + if (IS_ERR_OR_NULL(adc)) { > + pr_err("%s: Cannot get adc.\n", __func__); > + return adc ? PTR_ERR(adc) : -ENODEV; > + } > + } Why not do this in an initcall or in the device registration? This looks like working around a limitation of the ntc_thermistor driver which should be fixed as part of a mainline merge for that. > + > + if (IS_ERR_OR_NULL(adc)) > + return adc ? PTR_ERR(adc) : -ENODEV; > + > + val = s3c_adc_read(adc, 6); > + > + converted = 3300000LL * (s64) val; > + converted >>= 12; > + > + pr_emerg("%s: %d -> %llduV\n", __func__, val, converted); This looks like debug that was left in by mistake.
On Tue, Jun 21, 2011 at 7:50 PM, Mark Brown <broonie@opensource.wolfsonmicro.com> wrote: > On Tue, Jun 21, 2011 at 11:07:13AM +0900, MyungJoo Ham wrote: > >> +#include <linux/platform_data/ntc_thermistor.h> > > This doesn't appear to be in mainline. Right, not just yet. I should've added comment about the patch just being applied to "next". I've added it as it was told to be applied to "next" in lm-sensors by Guenter Roeck yesterday. http://comments.gmane.org/gmane.linux.drivers.sensors/26475 Should I surround them with "#ifdef CONFIG_NTC_THERMISTOR"? > >> +/* NTC Thermistor */ >> +static struct platform_device nuri_ncp15wb473_thermistor; >> +static int read_thermistor_uV(void) > > Blank line between these two. > got it. >> +{ >> + static struct s3c_adc_client *adc; >> + int val; >> + s64 converted; >> + >> + if (!adc) { >> + adc = s3c_adc_register(&nuri_ncp15wb473_thermistor, >> + NULL, NULL, 0); >> + if (IS_ERR_OR_NULL(adc)) { >> + pr_err("%s: Cannot get adc.\n", __func__); >> + return adc ? PTR_ERR(adc) : -ENODEV; >> + } >> + } > > Why not do this in an initcall or in the device registration? This > looks like working around a limitation of the ntc_thermistor driver > which should be fixed as part of a mainline merge for that. Um... I think I can put this away to a common plat-samsung/dev-ntc.c as I've seen many S5PC110/S5PV210/Exynos4 devices using NTC attached to the CPU's ADC ports. However, I won't be able to put that part into the NTC driver because using S3C-ADC is plat-samsung specific method and NTC thermistor itself is a general thermistor device not specific to S3C. Maybe, I can do so after we've got a common ADC framework. > >> + >> + if (IS_ERR_OR_NULL(adc)) >> + return adc ? PTR_ERR(adc) : -ENODEV; >> + >> + val = s3c_adc_read(adc, 6); >> + >> + converted = 3300000LL * (s64) val; >> + converted >>= 12; >> + >> + pr_emerg("%s: %d -> %llduV\n", __func__, val, converted); > > This looks like debug that was left in by mistake. > Oh.. yes.. thanks. I need to remove that.
On Wed, Jun 22, 2011 at 02:00:37PM +0900, MyungJoo Ham wrote: > On Tue, Jun 21, 2011 at 7:50 PM, Mark Brown > >> +#include <linux/platform_data/ntc_thermistor.h> > > This doesn't appear to be in mainline. > Right, not just yet. I should've added comment about the patch just > being applied to "next". I was actually looking at -next rather than Linus' tree there... > Should I surround them with "#ifdef CONFIG_NTC_THERMISTOR"? I guess, or just wait until it gets merged. > > Why not do this in an initcall or in the device registration? This > > looks like working around a limitation of the ntc_thermistor driver > > which should be fixed as part of a mainline merge for that. > Um... I think I can put this away to a common plat-samsung/dev-ntc.c > as I've seen many S5PC110/S5PV210/Exynos4 devices using NTC attached > to the CPU's ADC ports. Yes, but it's not just the location of the code - I was mostly noticing the fact that it was doing something that's clearly supposed to be a probe() method in the main read function. It'd be better if there were an explicit probe() and corresponding remove() interface. I also note that this callback based stuff won't work terribly well with device tree. > However, I won't be able to put that part into the NTC driver because > using S3C-ADC is plat-samsung specific method and NTC thermistor > itself is a general thermistor device not specific to S3C. Maybe, I > can do so after we've got a common ADC framework. Yes, we really need to get that ADC stuff sorted soon :(
diff --git a/arch/arm/mach-exynos4/mach-nuri.c b/arch/arm/mach-exynos4/mach-nuri.c index 51f55b4..dc6fc36 100644 --- a/arch/arm/mach-exynos4/mach-nuri.c +++ b/arch/arm/mach-exynos4/mach-nuri.c @@ -16,6 +16,7 @@ #include <linux/i2c-gpio.h> #include <linux/gpio_keys.h> #include <linux/gpio.h> +#include <linux/platform_data/ntc_thermistor.h> #include <linux/power/max17042_battery.h> #include <linux/regulator/machine.h> #include <linux/regulator/fixed.h> @@ -1006,6 +1007,50 @@ static void __init nuri_ehci_init(void) s5p_ehci_set_platdata(pdata); } +/* NTC Thermistor */ +static struct platform_device nuri_ncp15wb473_thermistor; +static int read_thermistor_uV(void) +{ + static struct s3c_adc_client *adc; + int val; + s64 converted; + + if (!adc) { + adc = s3c_adc_register(&nuri_ncp15wb473_thermistor, + NULL, NULL, 0); + if (IS_ERR_OR_NULL(adc)) { + pr_err("%s: Cannot get adc.\n", __func__); + return adc ? PTR_ERR(adc) : -ENODEV; + } + } + + if (IS_ERR_OR_NULL(adc)) + return adc ? PTR_ERR(adc) : -ENODEV; + + val = s3c_adc_read(adc, 6); + + converted = 3300000LL * (s64) val; + converted >>= 12; + + pr_emerg("%s: %d -> %llduV\n", __func__, val, converted); + return converted; +} + +static struct ntc_thermistor_platform_data ncp15wb473_pdata = { + .read_uV = read_thermistor_uV, + .pullup_uV = 3300000, /* VADC_3.3V_C210 */ + .pullup_ohm = 100000, /* R613 in SLP 7 0105 */ + .pulldown_ohm = 100000, /* R615 in SLP 7 0105 */ + .connect = NTC_CONNECTED_GROUND, +}; + +static struct platform_device nuri_ncp15wb473_thermistor = { + .name = "ncp15wb473", + .dev = { + .platform_data = &ncp15wb473_pdata, + }, +}; + static struct platform_device *nuri_devices[] __initdata = { /* Samsung Platform Devices */ &emmc_fixed_voltage, @@ -1024,6 +1069,7 @@ static struct platform_device *nuri_devices[] __initdata = { &nuri_gpio_keys, &nuri_lcd_device, &nuri_backlight_device, + &nuri_ncp15wb473_thermistor, }; static void __init nuri_map_io(void)
Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com> --- arch/arm/mach-exynos4/mach-nuri.c | 46 +++++++++++++++++++++++++++++++++++++ 1 files changed, 46 insertions(+), 0 deletions(-)