All of lore.kernel.org
 help / color / mirror / Atom feed
* [NEW DRIVER V2 6/7] DA9058 HWMON driver
@ 2012-08-05 20:43 ` Anthony Olech
  0 siblings, 0 replies; 8+ messages in thread
From: Anthony Olech @ 2012-08-05 20:43 UTC (permalink / raw)
  To: Guenter Roeck, Jean Delvare
  Cc: Mark Brown, Randy Dunlop, lm-sensors, LKML, David Dajun Chen

This is the HWMON component driver of the Dialog DA9058 PMIC.
This driver is just one component of the whole DA9058 PMIC driver.
It depends on the core DA9058 MFD driver.

Signed-off-by: Anthony Olech <anthony.olech.opensource@diasemi.com>
Signed-off-by: David Dajun Chen <david.chen@diasemi.com>
---
 drivers/hwmon/Kconfig        |   10 +
 drivers/hwmon/Makefile       |    1 +
 drivers/hwmon/da9058-hwmon.c |  390 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 401 insertions(+), 0 deletions(-)
 create mode 100644 drivers/hwmon/da9058-hwmon.c

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 6f1d167..0986f43 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -294,6 +294,16 @@ config SENSORS_ATXP1
 	  This driver can also be built as a module.  If so, the module
 	  will be called atxp1.
 
+config SENSORS_DA9058
+	tristate "Dialog Semiconductor DA9058 ADC"
+	depends on I2C
+	help
+	  If you say yes here you get support for ADC on the Dialog
+	  Semiconductor DA9058 PMIC.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called da9058-hwmon.
+
 config SENSORS_DS620
 	tristate "Dallas Semiconductor DS620"
 	depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index e1eeac1..be99572 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -43,6 +43,7 @@ obj-$(CONFIG_SENSORS_ASC7621)	+= asc7621.o
 obj-$(CONFIG_SENSORS_ATXP1)	+= atxp1.o
 obj-$(CONFIG_SENSORS_CORETEMP)	+= coretemp.o
 obj-$(CONFIG_SENSORS_DME1737)	+= dme1737.o
+obj-$(CONFIG_SENSORS_DA9058)	+= da9058-hwmon.o
 obj-$(CONFIG_SENSORS_DS620)	+= ds620.o
 obj-$(CONFIG_SENSORS_DS1621)	+= ds1621.o
 obj-$(CONFIG_SENSORS_EMC1403)	+= emc1403.o
diff --git a/drivers/hwmon/da9058-hwmon.c b/drivers/hwmon/da9058-hwmon.c
new file mode 100644
index 0000000..a1475e4
--- /dev/null
+++ b/drivers/hwmon/da9058-hwmon.c
@@ -0,0 +1,390 @@
+/*
+ *  Copyright (C) 2012 Dialog Semiconductor Ltd.
+ *
+ *  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.
+ *
+ */
+
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/mfd/core.h>
+
+#include <linux/mfd/da9058/version.h>
+#include <linux/mfd/da9058/registers.h>
+#include <linux/mfd/da9058/core.h>
+#include <linux/mfd/da9058/hwmon.h>
+
+struct da9058_hwmon {
+	struct da9058	*da9058;
+	struct platform_device *pdev;
+
+	struct device	*class_device;
+	struct mutex	hwmon_lock;
+	int use_automatic_adc;
+	int temp_adc_resistance;
+	int vf_adc_resistance;
+};
+
+static ssize_t da9058_read_tbat(struct device *dev,
+				struct device_attribute *devattr, char *buf)
+{
+	struct da9058_hwmon *hwmon = dev_get_drvdata(dev);
+	int voltage; /* x000 .. xFFF = 0 .. 2500 mV */
+	int ret;
+
+	ret = da9058_adc_read(hwmon->da9058, DA9058_ADCMAN_MUXSEL_TEMP,
+					hwmon->use_automatic_adc, &voltage);
+	if (ret)
+		return ret;
+
+	return sprintf(buf, "%d\n", voltage * 2500 / 0xFFF);
+}
+
+static ssize_t da9058_read_vbat(struct device *dev,
+				struct device_attribute *devattr, char *buf)
+{
+	struct da9058_hwmon *hwmon = dev_get_drvdata(dev);
+	int voltage; /* x000 .. xFFF = 2500 .. 4500 mV */
+	int ret;
+
+	ret = da9058_adc_read(hwmon->da9058, DA9058_ADCMAN_MUXSEL_VBAT,
+					hwmon->use_automatic_adc, &voltage);
+	if (ret)
+		return ret;
+
+	return sprintf(buf, "%d\n", 2500 + voltage * 2000 / 0xFFF);
+}
+
+static ssize_t da9058_read_misc_channel(struct device *dev,
+					struct device_attribute *devattr,
+					char *buf)
+{
+	struct da9058_hwmon *hwmon = dev_get_drvdata(dev);
+	int channel = to_sensor_dev_attr(devattr)->index;
+	int voltage; /* xFFF .. x800 = 0 .. 2500 mV */
+	int ret;
+
+	ret = da9058_adc_read(hwmon->da9058, channel,
+				hwmon->use_automatic_adc, &voltage);
+	if (ret)
+		return ret;
+
+	return sprintf(buf, "%d\n", (0xFFF - voltage) * 2500 / 0x7FF);
+}
+
+/*
+ *  The algorithm for converting the value is
+ *  Degrees celsius = 1.708 * (TJUNC_RES - T_OFFSET) - 108.8
+ *  T_OFFSET is a trim value used to improve accuracy of the result
+ */
+static ssize_t da9058_read_tjunc(struct device *dev,
+				struct device_attribute *devattr, char *buf)
+{
+	struct da9058_hwmon *hwmon = dev_get_drvdata(dev);
+	int tjunc;
+	unsigned int toffset;
+	int ret;
+
+	ret = da9058_reg_read(hwmon->da9058, DA9058_TOFFSET_REG, &toffset);
+	if (ret < 0)
+		return ret;
+
+	ret = da9058_adc_read(hwmon->da9058, DA9058_ADCMAN_MUXSEL_TJUNC,
+					hwmon->use_automatic_adc, &tjunc);
+	if (ret < 0)
+		return ret;
+
+	return sprintf(buf, "%d\n", 1708 * (tjunc - (int)toffset) - 108800);
+}
+
+static ssize_t da9058_read_vfpin(struct device *dev,
+				struct device_attribute *devattr, char *buf)
+{
+	struct da9058_hwmon *hwmon = dev_get_drvdata(dev);
+	int voltage; /* x000 .. xFFF = 0 .. 4095 mV */
+	int ret;
+
+	ret = da9058_adc_read(hwmon->da9058, DA9058_ADCMAN_MUXSEL_VF,
+					hwmon->use_automatic_adc, &voltage);
+	if (ret)
+		return ret;
+
+	return sprintf(buf, "%d\n", voltage);
+}
+
+static ssize_t da9058_vfpin_mode(struct device *dev,
+				struct device_attribute *devattr, char *buf)
+{
+	struct da9058_hwmon *hwmon = dev_get_drvdata(dev);
+	int ret;
+	unsigned int mode;
+
+	ret = da9058_reg_read(hwmon->da9058, DA9058_ADCCONT_REG, &mode);
+	if (ret)
+		return ret;
+
+	if (mode & DA9058_ADCCONT_VFISRCEN) {
+		if (hwmon->vf_adc_resistance)
+			return sprintf(buf, "resistance\n");
+		else
+			return sprintf(buf,
+				"resistance - but should be voltage\n");
+	} else {
+		if (hwmon->vf_adc_resistance)
+			return sprintf(buf,
+				"voltage - but should be resistance\n");
+		else
+			return sprintf(buf, "voltage\n");
+	}
+}
+
+static ssize_t da9058_get_adc_mode(struct device *dev,
+					struct device_attribute *devattr,
+					char *buf)
+{
+	struct da9058_hwmon *hwmon = dev_get_drvdata(dev);
+	unsigned int mode;
+	int ret;
+
+	ret = da9058_reg_read(hwmon->da9058, DA9058_ADCCONT_REG, &mode);
+	if (ret)
+		return ret;
+
+	if (mode & DA9058_ADCCONT_AUTOADCEN) {
+		if (hwmon->use_automatic_adc)
+			return sprintf(buf, "automatic\n");
+		else
+			return sprintf(buf,
+					"automatic - but should be manual\n");
+	} else {
+		if (hwmon->use_automatic_adc)
+			return sprintf(buf,
+					"manual - but should be automatic\n");
+		else
+			return sprintf(buf, "manual\n");
+	}
+}
+
+static ssize_t da9058_set_adc_mode(struct device *dev,
+					struct device_attribute *devattr,
+					const char *buf, size_t count)
+{
+	struct da9058_hwmon *hwmon = dev_get_drvdata(dev);
+
+	if (*buf == 'A' || *buf == 'a') {
+		if (hwmon->use_automatic_adc) {
+			dev_info(&hwmon->pdev->dev,
+					"already in AUTOMATIC adc_mode\n");
+			return -EINVAL;
+		} else {
+			int ret;
+			unsigned int mode = DA9058_ADCCONT_AUTOADCEN |
+					DA9058_ADCCONT_TEMPISRCEN |
+					DA9058_ADCCONT_AUTOVBATEN |
+					DA9058_ADCCONT_AUTOVFEN |
+					DA9058_ADCCONT_AUTOAINEN;
+
+			if (hwmon->vf_adc_resistance)
+				mode |= DA9058_ADCCONT_VFISRCEN;
+
+			hwmon->use_automatic_adc = 1;
+			ret = da9058_reg_write(hwmon->da9058,
+						DA9058_ADCCONT_REG, mode);
+			if (ret)
+				return ret;
+			return count;
+		}
+
+	} else if (*buf == 'M' || *buf == 'm') {
+		if (hwmon->use_automatic_adc) {
+			int ret;
+			unsigned int mode = 0;
+
+			if (hwmon->temp_adc_resistance)
+				mode |= DA9058_ADCCONT_TEMPISRCEN;
+			if (hwmon->vf_adc_resistance)
+				mode |= DA9058_ADCCONT_VFISRCEN;
+
+			hwmon->use_automatic_adc = 0;
+			ret = da9058_reg_write(hwmon->da9058,
+						DA9058_ADCCONT_REG, mode);
+			if (ret)
+				return ret;
+			return count;
+		} else {
+			dev_info(&hwmon->pdev->dev,
+				"already in MANUAL adc_mode\n");
+			return -EINVAL;
+		}
+	} else {
+		dev_err(&hwmon->pdev->dev,
+			"adc_mode should be AUTOMATIC or MANUAL not '%s'",
+				buf);
+		return -EINVAL;
+	}
+}
+
+static ssize_t da9058_hwmon_show_name(struct device *dev,
+					struct device_attribute *devattr,
+					char *buf)
+{
+	return sprintf(buf, "da9058-hwmon\n");
+}
+
+static SENSOR_DEVICE_ATTR(vbat_mV, S_IRUGO, da9058_read_vbat, NULL, 0);
+static SENSOR_DEVICE_ATTR(adc_mV, S_IRUGO, da9058_read_misc_channel, NULL,
+				DA9058_ADCMAN_MUXSEL_ADCIN);
+static SENSOR_DEVICE_ATTR(vfpin_mV, S_IRUGO, da9058_read_vfpin, NULL, 0);
+static SENSOR_DEVICE_ATTR(vfpin_mode, S_IRUGO, da9058_vfpin_mode, NULL, 0);
+static SENSOR_DEVICE_ATTR(tbat_mV, S_IRUGO, da9058_read_tbat, NULL, 0);
+static SENSOR_DEVICE_ATTR(tjunc_in, S_IRUGO, da9058_read_tjunc, NULL, 0);
+static SENSOR_DEVICE_ATTR(adc_mode, S_IWUSR | S_IRUGO, da9058_get_adc_mode,
+				da9058_set_adc_mode, 0);
+
+static DEVICE_ATTR(name, S_IRUGO, da9058_hwmon_show_name, NULL);
+
+static struct attribute *da9058_attr[] = {
+	&dev_attr_name.attr,
+	&sensor_dev_attr_adc_mode.dev_attr.attr,
+	&sensor_dev_attr_vbat_mV.dev_attr.attr,
+	&sensor_dev_attr_adc_mV.dev_attr.attr,
+	&sensor_dev_attr_vfpin_mV.dev_attr.attr,
+	&sensor_dev_attr_vfpin_mode.dev_attr.attr,
+	&sensor_dev_attr_tbat_mV.dev_attr.attr,
+	&sensor_dev_attr_tjunc_in.dev_attr.attr,
+	NULL
+};
+
+/*
+ *  the attributes will appear in /sys/class/hwmon/hwmon0/device
+ */
+
+static const struct attribute_group da9058_attr_group = {.attrs = da9058_attr};
+
+static int __devinit da9058_hwmon_probe(struct platform_device *pdev)
+{
+	struct da9058 *da9058 = dev_get_drvdata(pdev->dev.parent);
+	const struct mfd_cell *cell = mfd_get_cell(pdev);
+	struct da9058_hwmon_pdata *hwmon_pdata;
+	struct da9058_hwmon *hwmon;
+	int ret;
+
+	if (cell == NULL) {
+		ret = -ENODEV;
+		goto exit;
+	}
+
+	hwmon_pdata = cell->platform_data;
+
+	if (hwmon_pdata == NULL) {
+		ret = -EINVAL;
+		goto exit;
+	}
+
+	if (hwmon_pdata->use_automatic_adc &
+				!hwmon_pdata->temp_adc_resistance) {
+		ret = -EINVAL; /* impossible setting */
+		goto exit;
+	}
+
+	dev_info(&pdev->dev, "HWMON uses %s ADC conversions\n",
+		hwmon_pdata->use_automatic_adc ? "automatic" : "manual");
+
+	hwmon = devm_kzalloc(&pdev->dev, sizeof(struct da9058_hwmon),
+				GFP_KERNEL);
+	if (!hwmon) {
+		ret = -ENOMEM;
+		goto exit;
+	}
+
+	platform_set_drvdata(pdev, hwmon);
+
+	hwmon->da9058 = da9058;
+	hwmon->pdev = pdev;
+	hwmon->use_automatic_adc = hwmon_pdata->use_automatic_adc;
+	hwmon->temp_adc_resistance = hwmon_pdata->temp_adc_resistance;
+	hwmon->vf_adc_resistance = hwmon_pdata->vf_adc_resistance;
+
+	if (hwmon->use_automatic_adc) {
+		unsigned int mode = DA9058_ADCCONT_AUTOADCEN |
+				DA9058_ADCCONT_TEMPISRCEN |
+				DA9058_ADCCONT_AUTOVBATEN |
+				DA9058_ADCCONT_AUTOVFEN |
+				DA9058_ADCCONT_AUTOAINEN;
+
+		if (hwmon->vf_adc_resistance)
+			mode |= DA9058_ADCCONT_VFISRCEN;
+
+		ret = da9058_reg_write(da9058, DA9058_ADCCONT_REG, mode);
+		if (ret)
+			goto err1;
+	} else {
+		unsigned int mode = 0;
+
+		if (hwmon->temp_adc_resistance)
+			mode |= DA9058_ADCCONT_TEMPISRCEN;
+		if (hwmon->vf_adc_resistance)
+			mode |= DA9058_ADCCONT_VFISRCEN;
+
+		ret = da9058_reg_write(da9058, DA9058_ADCCONT_REG, mode);
+		if (ret)
+			goto err1;
+	}
+
+	mutex_init(&hwmon->hwmon_lock);
+
+	ret = sysfs_create_group(&pdev->dev.kobj, &da9058_attr_group);
+	if (ret)
+		goto err1;
+
+	hwmon->class_device = hwmon_device_register(&pdev->dev);
+	if (IS_ERR(hwmon->class_device)) {
+		ret = PTR_ERR(hwmon->class_device);
+		goto err2;
+	}
+
+	goto exit;
+
+err2:
+	sysfs_remove_group(&pdev->dev.kobj, &da9058_attr_group);
+err1:
+exit:
+	return ret;
+}
+
+static int __devexit da9058_hwmon_remove(struct platform_device *pdev)
+{
+	struct da9058_hwmon *hwmon = platform_get_drvdata(pdev);
+
+	hwmon_device_unregister(hwmon->class_device);
+	sysfs_remove_group(&pdev->dev.kobj, &da9058_attr_group);
+
+	return 0;
+}
+
+static struct platform_driver da9058_hwmon_driver = {
+	.probe = da9058_hwmon_probe,
+	.remove = __devexit_p(da9058_hwmon_remove),
+	.driver = {
+		.name = "da9058-hwmon",
+		.owner = THIS_MODULE,
+	},
+};
+
+module_platform_driver(da9058_hwmon_driver);
+
+MODULE_DESCRIPTION("Dialog DA9058 PMIC HardWare Monitor Driver");
+MODULE_AUTHOR("Anthony Olech <Anthony.Olech@diasemi.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:da9058-hwmon");
-- 
end-of-patch for NEW DRIVER V2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [lm-sensors] [NEW DRIVER V2 6/7] DA9058 HWMON driver
@ 2012-08-05 20:43 ` Anthony Olech
  0 siblings, 0 replies; 8+ messages in thread
From: Anthony Olech @ 2012-08-05 20:43 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: Mark Brown

This is the HWMON component driver of the Dialog DA9058 PMIC.
This driver is just one component of the whole DA9058 PMIC driver.
It depends on the core DA9058 MFD driver.

Signed-off-by: Anthony Olech <anthony.olech.opensource@diasemi.com>
Signed-off-by: David Dajun Chen <david.chen@diasemi.com>
---
 drivers/hwmon/Kconfig        |   10 +
 drivers/hwmon/Makefile       |    1 +
 drivers/hwmon/da9058-hwmon.c |  390 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 401 insertions(+), 0 deletions(-)
 create mode 100644 drivers/hwmon/da9058-hwmon.c

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 6f1d167..0986f43 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -294,6 +294,16 @@ config SENSORS_ATXP1
 	  This driver can also be built as a module.  If so, the module
 	  will be called atxp1.
 
+config SENSORS_DA9058
+	tristate "Dialog Semiconductor DA9058 ADC"
+	depends on I2C
+	help
+	  If you say yes here you get support for ADC on the Dialog
+	  Semiconductor DA9058 PMIC.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called da9058-hwmon.
+
 config SENSORS_DS620
 	tristate "Dallas Semiconductor DS620"
 	depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index e1eeac1..be99572 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -43,6 +43,7 @@ obj-$(CONFIG_SENSORS_ASC7621)	+= asc7621.o
 obj-$(CONFIG_SENSORS_ATXP1)	+= atxp1.o
 obj-$(CONFIG_SENSORS_CORETEMP)	+= coretemp.o
 obj-$(CONFIG_SENSORS_DME1737)	+= dme1737.o
+obj-$(CONFIG_SENSORS_DA9058)	+= da9058-hwmon.o
 obj-$(CONFIG_SENSORS_DS620)	+= ds620.o
 obj-$(CONFIG_SENSORS_DS1621)	+= ds1621.o
 obj-$(CONFIG_SENSORS_EMC1403)	+= emc1403.o
diff --git a/drivers/hwmon/da9058-hwmon.c b/drivers/hwmon/da9058-hwmon.c
new file mode 100644
index 0000000..a1475e4
--- /dev/null
+++ b/drivers/hwmon/da9058-hwmon.c
@@ -0,0 +1,390 @@
+/*
+ *  Copyright (C) 2012 Dialog Semiconductor Ltd.
+ *
+ *  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.
+ *
+ */
+
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/mfd/core.h>
+
+#include <linux/mfd/da9058/version.h>
+#include <linux/mfd/da9058/registers.h>
+#include <linux/mfd/da9058/core.h>
+#include <linux/mfd/da9058/hwmon.h>
+
+struct da9058_hwmon {
+	struct da9058	*da9058;
+	struct platform_device *pdev;
+
+	struct device	*class_device;
+	struct mutex	hwmon_lock;
+	int use_automatic_adc;
+	int temp_adc_resistance;
+	int vf_adc_resistance;
+};
+
+static ssize_t da9058_read_tbat(struct device *dev,
+				struct device_attribute *devattr, char *buf)
+{
+	struct da9058_hwmon *hwmon = dev_get_drvdata(dev);
+	int voltage; /* x000 .. xFFF = 0 .. 2500 mV */
+	int ret;
+
+	ret = da9058_adc_read(hwmon->da9058, DA9058_ADCMAN_MUXSEL_TEMP,
+					hwmon->use_automatic_adc, &voltage);
+	if (ret)
+		return ret;
+
+	return sprintf(buf, "%d\n", voltage * 2500 / 0xFFF);
+}
+
+static ssize_t da9058_read_vbat(struct device *dev,
+				struct device_attribute *devattr, char *buf)
+{
+	struct da9058_hwmon *hwmon = dev_get_drvdata(dev);
+	int voltage; /* x000 .. xFFF = 2500 .. 4500 mV */
+	int ret;
+
+	ret = da9058_adc_read(hwmon->da9058, DA9058_ADCMAN_MUXSEL_VBAT,
+					hwmon->use_automatic_adc, &voltage);
+	if (ret)
+		return ret;
+
+	return sprintf(buf, "%d\n", 2500 + voltage * 2000 / 0xFFF);
+}
+
+static ssize_t da9058_read_misc_channel(struct device *dev,
+					struct device_attribute *devattr,
+					char *buf)
+{
+	struct da9058_hwmon *hwmon = dev_get_drvdata(dev);
+	int channel = to_sensor_dev_attr(devattr)->index;
+	int voltage; /* xFFF .. x800 = 0 .. 2500 mV */
+	int ret;
+
+	ret = da9058_adc_read(hwmon->da9058, channel,
+				hwmon->use_automatic_adc, &voltage);
+	if (ret)
+		return ret;
+
+	return sprintf(buf, "%d\n", (0xFFF - voltage) * 2500 / 0x7FF);
+}
+
+/*
+ *  The algorithm for converting the value is
+ *  Degrees celsius = 1.708 * (TJUNC_RES - T_OFFSET) - 108.8
+ *  T_OFFSET is a trim value used to improve accuracy of the result
+ */
+static ssize_t da9058_read_tjunc(struct device *dev,
+				struct device_attribute *devattr, char *buf)
+{
+	struct da9058_hwmon *hwmon = dev_get_drvdata(dev);
+	int tjunc;
+	unsigned int toffset;
+	int ret;
+
+	ret = da9058_reg_read(hwmon->da9058, DA9058_TOFFSET_REG, &toffset);
+	if (ret < 0)
+		return ret;
+
+	ret = da9058_adc_read(hwmon->da9058, DA9058_ADCMAN_MUXSEL_TJUNC,
+					hwmon->use_automatic_adc, &tjunc);
+	if (ret < 0)
+		return ret;
+
+	return sprintf(buf, "%d\n", 1708 * (tjunc - (int)toffset) - 108800);
+}
+
+static ssize_t da9058_read_vfpin(struct device *dev,
+				struct device_attribute *devattr, char *buf)
+{
+	struct da9058_hwmon *hwmon = dev_get_drvdata(dev);
+	int voltage; /* x000 .. xFFF = 0 .. 4095 mV */
+	int ret;
+
+	ret = da9058_adc_read(hwmon->da9058, DA9058_ADCMAN_MUXSEL_VF,
+					hwmon->use_automatic_adc, &voltage);
+	if (ret)
+		return ret;
+
+	return sprintf(buf, "%d\n", voltage);
+}
+
+static ssize_t da9058_vfpin_mode(struct device *dev,
+				struct device_attribute *devattr, char *buf)
+{
+	struct da9058_hwmon *hwmon = dev_get_drvdata(dev);
+	int ret;
+	unsigned int mode;
+
+	ret = da9058_reg_read(hwmon->da9058, DA9058_ADCCONT_REG, &mode);
+	if (ret)
+		return ret;
+
+	if (mode & DA9058_ADCCONT_VFISRCEN) {
+		if (hwmon->vf_adc_resistance)
+			return sprintf(buf, "resistance\n");
+		else
+			return sprintf(buf,
+				"resistance - but should be voltage\n");
+	} else {
+		if (hwmon->vf_adc_resistance)
+			return sprintf(buf,
+				"voltage - but should be resistance\n");
+		else
+			return sprintf(buf, "voltage\n");
+	}
+}
+
+static ssize_t da9058_get_adc_mode(struct device *dev,
+					struct device_attribute *devattr,
+					char *buf)
+{
+	struct da9058_hwmon *hwmon = dev_get_drvdata(dev);
+	unsigned int mode;
+	int ret;
+
+	ret = da9058_reg_read(hwmon->da9058, DA9058_ADCCONT_REG, &mode);
+	if (ret)
+		return ret;
+
+	if (mode & DA9058_ADCCONT_AUTOADCEN) {
+		if (hwmon->use_automatic_adc)
+			return sprintf(buf, "automatic\n");
+		else
+			return sprintf(buf,
+					"automatic - but should be manual\n");
+	} else {
+		if (hwmon->use_automatic_adc)
+			return sprintf(buf,
+					"manual - but should be automatic\n");
+		else
+			return sprintf(buf, "manual\n");
+	}
+}
+
+static ssize_t da9058_set_adc_mode(struct device *dev,
+					struct device_attribute *devattr,
+					const char *buf, size_t count)
+{
+	struct da9058_hwmon *hwmon = dev_get_drvdata(dev);
+
+	if (*buf = 'A' || *buf = 'a') {
+		if (hwmon->use_automatic_adc) {
+			dev_info(&hwmon->pdev->dev,
+					"already in AUTOMATIC adc_mode\n");
+			return -EINVAL;
+		} else {
+			int ret;
+			unsigned int mode = DA9058_ADCCONT_AUTOADCEN |
+					DA9058_ADCCONT_TEMPISRCEN |
+					DA9058_ADCCONT_AUTOVBATEN |
+					DA9058_ADCCONT_AUTOVFEN |
+					DA9058_ADCCONT_AUTOAINEN;
+
+			if (hwmon->vf_adc_resistance)
+				mode |= DA9058_ADCCONT_VFISRCEN;
+
+			hwmon->use_automatic_adc = 1;
+			ret = da9058_reg_write(hwmon->da9058,
+						DA9058_ADCCONT_REG, mode);
+			if (ret)
+				return ret;
+			return count;
+		}
+
+	} else if (*buf = 'M' || *buf = 'm') {
+		if (hwmon->use_automatic_adc) {
+			int ret;
+			unsigned int mode = 0;
+
+			if (hwmon->temp_adc_resistance)
+				mode |= DA9058_ADCCONT_TEMPISRCEN;
+			if (hwmon->vf_adc_resistance)
+				mode |= DA9058_ADCCONT_VFISRCEN;
+
+			hwmon->use_automatic_adc = 0;
+			ret = da9058_reg_write(hwmon->da9058,
+						DA9058_ADCCONT_REG, mode);
+			if (ret)
+				return ret;
+			return count;
+		} else {
+			dev_info(&hwmon->pdev->dev,
+				"already in MANUAL adc_mode\n");
+			return -EINVAL;
+		}
+	} else {
+		dev_err(&hwmon->pdev->dev,
+			"adc_mode should be AUTOMATIC or MANUAL not '%s'",
+				buf);
+		return -EINVAL;
+	}
+}
+
+static ssize_t da9058_hwmon_show_name(struct device *dev,
+					struct device_attribute *devattr,
+					char *buf)
+{
+	return sprintf(buf, "da9058-hwmon\n");
+}
+
+static SENSOR_DEVICE_ATTR(vbat_mV, S_IRUGO, da9058_read_vbat, NULL, 0);
+static SENSOR_DEVICE_ATTR(adc_mV, S_IRUGO, da9058_read_misc_channel, NULL,
+				DA9058_ADCMAN_MUXSEL_ADCIN);
+static SENSOR_DEVICE_ATTR(vfpin_mV, S_IRUGO, da9058_read_vfpin, NULL, 0);
+static SENSOR_DEVICE_ATTR(vfpin_mode, S_IRUGO, da9058_vfpin_mode, NULL, 0);
+static SENSOR_DEVICE_ATTR(tbat_mV, S_IRUGO, da9058_read_tbat, NULL, 0);
+static SENSOR_DEVICE_ATTR(tjunc_in, S_IRUGO, da9058_read_tjunc, NULL, 0);
+static SENSOR_DEVICE_ATTR(adc_mode, S_IWUSR | S_IRUGO, da9058_get_adc_mode,
+				da9058_set_adc_mode, 0);
+
+static DEVICE_ATTR(name, S_IRUGO, da9058_hwmon_show_name, NULL);
+
+static struct attribute *da9058_attr[] = {
+	&dev_attr_name.attr,
+	&sensor_dev_attr_adc_mode.dev_attr.attr,
+	&sensor_dev_attr_vbat_mV.dev_attr.attr,
+	&sensor_dev_attr_adc_mV.dev_attr.attr,
+	&sensor_dev_attr_vfpin_mV.dev_attr.attr,
+	&sensor_dev_attr_vfpin_mode.dev_attr.attr,
+	&sensor_dev_attr_tbat_mV.dev_attr.attr,
+	&sensor_dev_attr_tjunc_in.dev_attr.attr,
+	NULL
+};
+
+/*
+ *  the attributes will appear in /sys/class/hwmon/hwmon0/device
+ */
+
+static const struct attribute_group da9058_attr_group = {.attrs = da9058_attr};
+
+static int __devinit da9058_hwmon_probe(struct platform_device *pdev)
+{
+	struct da9058 *da9058 = dev_get_drvdata(pdev->dev.parent);
+	const struct mfd_cell *cell = mfd_get_cell(pdev);
+	struct da9058_hwmon_pdata *hwmon_pdata;
+	struct da9058_hwmon *hwmon;
+	int ret;
+
+	if (cell = NULL) {
+		ret = -ENODEV;
+		goto exit;
+	}
+
+	hwmon_pdata = cell->platform_data;
+
+	if (hwmon_pdata = NULL) {
+		ret = -EINVAL;
+		goto exit;
+	}
+
+	if (hwmon_pdata->use_automatic_adc &
+				!hwmon_pdata->temp_adc_resistance) {
+		ret = -EINVAL; /* impossible setting */
+		goto exit;
+	}
+
+	dev_info(&pdev->dev, "HWMON uses %s ADC conversions\n",
+		hwmon_pdata->use_automatic_adc ? "automatic" : "manual");
+
+	hwmon = devm_kzalloc(&pdev->dev, sizeof(struct da9058_hwmon),
+				GFP_KERNEL);
+	if (!hwmon) {
+		ret = -ENOMEM;
+		goto exit;
+	}
+
+	platform_set_drvdata(pdev, hwmon);
+
+	hwmon->da9058 = da9058;
+	hwmon->pdev = pdev;
+	hwmon->use_automatic_adc = hwmon_pdata->use_automatic_adc;
+	hwmon->temp_adc_resistance = hwmon_pdata->temp_adc_resistance;
+	hwmon->vf_adc_resistance = hwmon_pdata->vf_adc_resistance;
+
+	if (hwmon->use_automatic_adc) {
+		unsigned int mode = DA9058_ADCCONT_AUTOADCEN |
+				DA9058_ADCCONT_TEMPISRCEN |
+				DA9058_ADCCONT_AUTOVBATEN |
+				DA9058_ADCCONT_AUTOVFEN |
+				DA9058_ADCCONT_AUTOAINEN;
+
+		if (hwmon->vf_adc_resistance)
+			mode |= DA9058_ADCCONT_VFISRCEN;
+
+		ret = da9058_reg_write(da9058, DA9058_ADCCONT_REG, mode);
+		if (ret)
+			goto err1;
+	} else {
+		unsigned int mode = 0;
+
+		if (hwmon->temp_adc_resistance)
+			mode |= DA9058_ADCCONT_TEMPISRCEN;
+		if (hwmon->vf_adc_resistance)
+			mode |= DA9058_ADCCONT_VFISRCEN;
+
+		ret = da9058_reg_write(da9058, DA9058_ADCCONT_REG, mode);
+		if (ret)
+			goto err1;
+	}
+
+	mutex_init(&hwmon->hwmon_lock);
+
+	ret = sysfs_create_group(&pdev->dev.kobj, &da9058_attr_group);
+	if (ret)
+		goto err1;
+
+	hwmon->class_device = hwmon_device_register(&pdev->dev);
+	if (IS_ERR(hwmon->class_device)) {
+		ret = PTR_ERR(hwmon->class_device);
+		goto err2;
+	}
+
+	goto exit;
+
+err2:
+	sysfs_remove_group(&pdev->dev.kobj, &da9058_attr_group);
+err1:
+exit:
+	return ret;
+}
+
+static int __devexit da9058_hwmon_remove(struct platform_device *pdev)
+{
+	struct da9058_hwmon *hwmon = platform_get_drvdata(pdev);
+
+	hwmon_device_unregister(hwmon->class_device);
+	sysfs_remove_group(&pdev->dev.kobj, &da9058_attr_group);
+
+	return 0;
+}
+
+static struct platform_driver da9058_hwmon_driver = {
+	.probe = da9058_hwmon_probe,
+	.remove = __devexit_p(da9058_hwmon_remove),
+	.driver = {
+		.name = "da9058-hwmon",
+		.owner = THIS_MODULE,
+	},
+};
+
+module_platform_driver(da9058_hwmon_driver);
+
+MODULE_DESCRIPTION("Dialog DA9058 PMIC HardWare Monitor Driver");
+MODULE_AUTHOR("Anthony Olech <Anthony.Olech@diasemi.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:da9058-hwmon");
-- 
end-of-patch for NEW DRIVER V2


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [lm-sensors] [NEW DRIVER V2 6/7] DA9058 HWMON driver
  2012-08-05 20:43 ` [lm-sensors] " Anthony Olech
@ 2012-08-06 17:39   ` Guenter Roeck
  -1 siblings, 0 replies; 8+ messages in thread
From: Guenter Roeck @ 2012-08-06 17:39 UTC (permalink / raw)
  To: Anthony Olech
  Cc: Guenter Roeck, Jean Delvare, Randy Dunlop, Mark Brown,
	David Dajun Chen, LKML, lm-sensors

On Sun, Aug 05, 2012 at 09:43:44PM +0100, Anthony Olech wrote:
> This is the HWMON component driver of the Dialog DA9058 PMIC.
> This driver is just one component of the whole DA9058 PMIC driver.
> It depends on the core DA9058 MFD driver.
> 
> Signed-off-by: Anthony Olech <anthony.olech.opensource@diasemi.com>
> Signed-off-by: David Dajun Chen <david.chen@diasemi.com>

[ ... ]

> +static SENSOR_DEVICE_ATTR(vbat_mV, S_IRUGO, da9058_read_vbat, NULL, 0);
> +static SENSOR_DEVICE_ATTR(adc_mV, S_IRUGO, da9058_read_misc_channel, NULL,
> +				DA9058_ADCMAN_MUXSEL_ADCIN);
> +static SENSOR_DEVICE_ATTR(vfpin_mV, S_IRUGO, da9058_read_vfpin, NULL, 0);
> +static SENSOR_DEVICE_ATTR(vfpin_mode, S_IRUGO, da9058_vfpin_mode, NULL, 0);
> +static SENSOR_DEVICE_ATTR(tbat_mV, S_IRUGO, da9058_read_tbat, NULL, 0);
> +static SENSOR_DEVICE_ATTR(tjunc_in, S_IRUGO, da9058_read_tjunc, NULL, 0);
> +static SENSOR_DEVICE_ATTR(adc_mode, S_IWUSR | S_IRUGO, da9058_get_adc_mode,
> +				da9058_set_adc_mode, 0);

Please use standard sysfs attribute names for temperature and voltage attributes.
For configuration (XXX_mode), please use devicetreee and/or platform data, not
sysfs attributes.

Thanks,
Guenter

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [lm-sensors] [NEW DRIVER V2 6/7] DA9058 HWMON driver
@ 2012-08-06 17:39   ` Guenter Roeck
  0 siblings, 0 replies; 8+ messages in thread
From: Guenter Roeck @ 2012-08-06 17:39 UTC (permalink / raw)
  To: Anthony Olech
  Cc: Guenter Roeck, Jean Delvare, Randy Dunlop, Mark Brown,
	David Dajun Chen, LKML, lm-sensors

On Sun, Aug 05, 2012 at 09:43:44PM +0100, Anthony Olech wrote:
> This is the HWMON component driver of the Dialog DA9058 PMIC.
> This driver is just one component of the whole DA9058 PMIC driver.
> It depends on the core DA9058 MFD driver.
> 
> Signed-off-by: Anthony Olech <anthony.olech.opensource@diasemi.com>
> Signed-off-by: David Dajun Chen <david.chen@diasemi.com>

[ ... ]

> +static SENSOR_DEVICE_ATTR(vbat_mV, S_IRUGO, da9058_read_vbat, NULL, 0);
> +static SENSOR_DEVICE_ATTR(adc_mV, S_IRUGO, da9058_read_misc_channel, NULL,
> +				DA9058_ADCMAN_MUXSEL_ADCIN);
> +static SENSOR_DEVICE_ATTR(vfpin_mV, S_IRUGO, da9058_read_vfpin, NULL, 0);
> +static SENSOR_DEVICE_ATTR(vfpin_mode, S_IRUGO, da9058_vfpin_mode, NULL, 0);
> +static SENSOR_DEVICE_ATTR(tbat_mV, S_IRUGO, da9058_read_tbat, NULL, 0);
> +static SENSOR_DEVICE_ATTR(tjunc_in, S_IRUGO, da9058_read_tjunc, NULL, 0);
> +static SENSOR_DEVICE_ATTR(adc_mode, S_IWUSR | S_IRUGO, da9058_get_adc_mode,
> +				da9058_set_adc_mode, 0);

Please use standard sysfs attribute names for temperature and voltage attributes.
For configuration (XXX_mode), please use devicetreee and/or platform data, not
sysfs attributes.

Thanks,
Guenter

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [lm-sensors] [NEW DRIVER V2 6/7] DA9058 HWMON driver
  2012-08-06 17:39   ` Guenter Roeck
@ 2012-08-07 12:10     ` Opensource [Anthony Olech]
  -1 siblings, 0 replies; 8+ messages in thread
From: Opensource [Anthony Olech] @ 2012-08-07 12:10 UTC (permalink / raw)
  To: Guenter Roeck, Opensource [Anthony Olech]
  Cc: Guenter Roeck, Jean Delvare, Randy Dunlop, Mark Brown,
	David Dajun Chen, LKML, lm-sensors

> -----Original Message-----
> From: Guenter Roeck [mailto:linux@roeck-us.net]
> Sent: 06 August 2012 18:40
> To: Opensource [Anthony Olech]
> Cc: Guenter Roeck; Jean Delvare; Randy Dunlop; Mark Brown; David Dajun
> Chen; LKML; lm-sensors@lm-sensors.org
> Subject: Re: [lm-sensors] [NEW DRIVER V2 6/7] DA9058 HWMON driver
> On Sun, Aug 05, 2012 at 09:43:44PM +0100, Anthony Olech wrote:
> > This is the HWMON component driver of the Dialog DA9058 PMIC.
> > This driver is just one component of the whole DA9058 PMIC driver.
> > It depends on the core DA9058 MFD driver.
> > Signed-off-by: Anthony Olech <anthony.olech.opensource@diasemi.com>
> > Signed-off-by: David Dajun Chen <david.chen@diasemi.com>
> [ ... ]
> > +static SENSOR_DEVICE_ATTR(vbat_mV, S_IRUGO, da9058_read_vbat, NULL,
> > +0); static SENSOR_DEVICE_ATTR(adc_mV, S_IRUGO,
> da9058_read_misc_channel, NULL,
> > +				DA9058_ADCMAN_MUXSEL_ADCIN);
> > +static SENSOR_DEVICE_ATTR(vfpin_mV, S_IRUGO, da9058_read_vfpin,
> NULL,
> > +0); static SENSOR_DEVICE_ATTR(vfpin_mode, S_IRUGO,
> da9058_vfpin_mode,
> > +NULL, 0); static SENSOR_DEVICE_ATTR(tbat_mV, S_IRUGO,
> > +da9058_read_tbat, NULL, 0); static SENSOR_DEVICE_ATTR(tjunc_in,
> > +S_IRUGO, da9058_read_tjunc, NULL, 0); static
> SENSOR_DEVICE_ATTR(adc_mode, S_IWUSR | S_IRUGO,
> da9058_get_adc_mode,
> > +				da9058_set_adc_mode, 0);
> Please use standard sysfs attribute names for temperature and voltage attributes.

I could not find a naming convention, so I will try to abstract one from all the
HWMON driver that have your name in them. I noted when searching that
I missed out a file in also that Documentation/hwmon. I will correct both
issues in my next submission attempt.

> For configuration (XXX_mode), please use devicetreee and/or platform data,
> not sysfs attributes.

As far as I can see both devicetreee and platform data allow configuration
data to be passed into the driver at "probe" time, they don't allow an operating
mode to be changed dynamically. That is what I thought sysfs allowed. Thus
your comments seem to imply that you do not want to allow the mode to be
changed dynamically. If that is the case then I can remove the dynamic mode
setting, leaving it fixed by platform data.

Thanks,
Tony Olech

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [lm-sensors] [NEW DRIVER V2 6/7] DA9058 HWMON driver
@ 2012-08-07 12:10     ` Opensource [Anthony Olech]
  0 siblings, 0 replies; 8+ messages in thread
From: Opensource [Anthony Olech] @ 2012-08-07 12:10 UTC (permalink / raw)
  To: Guenter Roeck, Opensource [Anthony Olech]
  Cc: Guenter Roeck, Jean Delvare, Randy Dunlop, Mark Brown,
	David Dajun Chen, LKML, lm-sensors

> -----Original Message-----
> From: Guenter Roeck [mailto:linux@roeck-us.net]
> Sent: 06 August 2012 18:40
> To: Opensource [Anthony Olech]
> Cc: Guenter Roeck; Jean Delvare; Randy Dunlop; Mark Brown; David Dajun
> Chen; LKML; lm-sensors@lm-sensors.org
> Subject: Re: [lm-sensors] [NEW DRIVER V2 6/7] DA9058 HWMON driver
> On Sun, Aug 05, 2012 at 09:43:44PM +0100, Anthony Olech wrote:
> > This is the HWMON component driver of the Dialog DA9058 PMIC.
> > This driver is just one component of the whole DA9058 PMIC driver.
> > It depends on the core DA9058 MFD driver.
> > Signed-off-by: Anthony Olech <anthony.olech.opensource@diasemi.com>
> > Signed-off-by: David Dajun Chen <david.chen@diasemi.com>
> [ ... ]
> > +static SENSOR_DEVICE_ATTR(vbat_mV, S_IRUGO, da9058_read_vbat, NULL,
> > +0); static SENSOR_DEVICE_ATTR(adc_mV, S_IRUGO,
> da9058_read_misc_channel, NULL,
> > +				DA9058_ADCMAN_MUXSEL_ADCIN);
> > +static SENSOR_DEVICE_ATTR(vfpin_mV, S_IRUGO, da9058_read_vfpin,
> NULL,
> > +0); static SENSOR_DEVICE_ATTR(vfpin_mode, S_IRUGO,
> da9058_vfpin_mode,
> > +NULL, 0); static SENSOR_DEVICE_ATTR(tbat_mV, S_IRUGO,
> > +da9058_read_tbat, NULL, 0); static SENSOR_DEVICE_ATTR(tjunc_in,
> > +S_IRUGO, da9058_read_tjunc, NULL, 0); static
> SENSOR_DEVICE_ATTR(adc_mode, S_IWUSR | S_IRUGO,
> da9058_get_adc_mode,
> > +				da9058_set_adc_mode, 0);
> Please use standard sysfs attribute names for temperature and voltage attributes.

I could not find a naming convention, so I will try to abstract one from all the
HWMON driver that have your name in them. I noted when searching that
I missed out a file in also that Documentation/hwmon. I will correct both
issues in my next submission attempt.

> For configuration (XXX_mode), please use devicetreee and/or platform data,
> not sysfs attributes.

As far as I can see both devicetreee and platform data allow configuration
data to be passed into the driver at "probe" time, they don't allow an operating
mode to be changed dynamically. That is what I thought sysfs allowed. Thus
your comments seem to imply that you do not want to allow the mode to be
changed dynamically. If that is the case then I can remove the dynamic mode
setting, leaving it fixed by platform data.

Thanks,
Tony Olech

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [lm-sensors] [NEW DRIVER V2 6/7] DA9058 HWMON driver
  2012-08-07 12:10     ` Opensource [Anthony Olech]
@ 2012-08-07 15:09       ` Guenter Roeck
  -1 siblings, 0 replies; 8+ messages in thread
From: Guenter Roeck @ 2012-08-07 15:09 UTC (permalink / raw)
  To: Opensource [Anthony Olech]
  Cc: Guenter Roeck, Jean Delvare, Randy Dunlop, Mark Brown,
	David Dajun Chen, LKML, lm-sensors

On Tue, Aug 07, 2012 at 12:10:08PM +0000, Opensource [Anthony Olech] wrote:
> > -----Original Message-----
> > From: Guenter Roeck [mailto:linux@roeck-us.net]
> > Sent: 06 August 2012 18:40
> > To: Opensource [Anthony Olech]
> > Cc: Guenter Roeck; Jean Delvare; Randy Dunlop; Mark Brown; David Dajun
> > Chen; LKML; lm-sensors@lm-sensors.org
> > Subject: Re: [lm-sensors] [NEW DRIVER V2 6/7] DA9058 HWMON driver
> > On Sun, Aug 05, 2012 at 09:43:44PM +0100, Anthony Olech wrote:
> > > This is the HWMON component driver of the Dialog DA9058 PMIC.
> > > This driver is just one component of the whole DA9058 PMIC driver.
> > > It depends on the core DA9058 MFD driver.
> > > Signed-off-by: Anthony Olech <anthony.olech.opensource@diasemi.com>
> > > Signed-off-by: David Dajun Chen <david.chen@diasemi.com>
> > [ ... ]
> > > +static SENSOR_DEVICE_ATTR(vbat_mV, S_IRUGO, da9058_read_vbat, NULL,
> > > +0); static SENSOR_DEVICE_ATTR(adc_mV, S_IRUGO,
> > da9058_read_misc_channel, NULL,
> > > +				DA9058_ADCMAN_MUXSEL_ADCIN);
> > > +static SENSOR_DEVICE_ATTR(vfpin_mV, S_IRUGO, da9058_read_vfpin,
> > NULL,
> > > +0); static SENSOR_DEVICE_ATTR(vfpin_mode, S_IRUGO,
> > da9058_vfpin_mode,
> > > +NULL, 0); static SENSOR_DEVICE_ATTR(tbat_mV, S_IRUGO,
> > > +da9058_read_tbat, NULL, 0); static SENSOR_DEVICE_ATTR(tjunc_in,
> > > +S_IRUGO, da9058_read_tjunc, NULL, 0); static
> > SENSOR_DEVICE_ATTR(adc_mode, S_IWUSR | S_IRUGO,
> > da9058_get_adc_mode,
> > > +				da9058_set_adc_mode, 0);
> > Please use standard sysfs attribute names for temperature and voltage attributes.
> 
> I could not find a naming convention, so I will try to abstract one from all the

Documentation/hwmon/sysfs-interface might be a good start.

> HWMON driver that have your name in them. I noted when searching that
> I missed out a file in also that Documentation/hwmon. I will correct both
> issues in my next submission attempt.
> 
> > For configuration (XXX_mode), please use devicetreee and/or platform data,
> > not sysfs attributes.
> 
> As far as I can see both devicetreee and platform data allow configuration
> data to be passed into the driver at "probe" time, they don't allow an operating
> mode to be changed dynamically. That is what I thought sysfs allowed. Thus
> your comments seem to imply that you do not want to allow the mode to be
> changed dynamically. If that is the case then I can remove the dynamic mode

Correct.

> setting, leaving it fixed by platform data.
> 

I would also suggest to read and follow Documentation/hwmon/submitting-patches.

Thanks,
Guenter

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [lm-sensors] [NEW DRIVER V2 6/7] DA9058 HWMON driver
@ 2012-08-07 15:09       ` Guenter Roeck
  0 siblings, 0 replies; 8+ messages in thread
From: Guenter Roeck @ 2012-08-07 15:09 UTC (permalink / raw)
  To: Opensource [Anthony Olech]
  Cc: Guenter Roeck, Jean Delvare, Randy Dunlop, Mark Brown,
	David Dajun Chen, LKML, lm-sensors

On Tue, Aug 07, 2012 at 12:10:08PM +0000, Opensource [Anthony Olech] wrote:
> > -----Original Message-----
> > From: Guenter Roeck [mailto:linux@roeck-us.net]
> > Sent: 06 August 2012 18:40
> > To: Opensource [Anthony Olech]
> > Cc: Guenter Roeck; Jean Delvare; Randy Dunlop; Mark Brown; David Dajun
> > Chen; LKML; lm-sensors@lm-sensors.org
> > Subject: Re: [lm-sensors] [NEW DRIVER V2 6/7] DA9058 HWMON driver
> > On Sun, Aug 05, 2012 at 09:43:44PM +0100, Anthony Olech wrote:
> > > This is the HWMON component driver of the Dialog DA9058 PMIC.
> > > This driver is just one component of the whole DA9058 PMIC driver.
> > > It depends on the core DA9058 MFD driver.
> > > Signed-off-by: Anthony Olech <anthony.olech.opensource@diasemi.com>
> > > Signed-off-by: David Dajun Chen <david.chen@diasemi.com>
> > [ ... ]
> > > +static SENSOR_DEVICE_ATTR(vbat_mV, S_IRUGO, da9058_read_vbat, NULL,
> > > +0); static SENSOR_DEVICE_ATTR(adc_mV, S_IRUGO,
> > da9058_read_misc_channel, NULL,
> > > +				DA9058_ADCMAN_MUXSEL_ADCIN);
> > > +static SENSOR_DEVICE_ATTR(vfpin_mV, S_IRUGO, da9058_read_vfpin,
> > NULL,
> > > +0); static SENSOR_DEVICE_ATTR(vfpin_mode, S_IRUGO,
> > da9058_vfpin_mode,
> > > +NULL, 0); static SENSOR_DEVICE_ATTR(tbat_mV, S_IRUGO,
> > > +da9058_read_tbat, NULL, 0); static SENSOR_DEVICE_ATTR(tjunc_in,
> > > +S_IRUGO, da9058_read_tjunc, NULL, 0); static
> > SENSOR_DEVICE_ATTR(adc_mode, S_IWUSR | S_IRUGO,
> > da9058_get_adc_mode,
> > > +				da9058_set_adc_mode, 0);
> > Please use standard sysfs attribute names for temperature and voltage attributes.
> 
> I could not find a naming convention, so I will try to abstract one from all the

Documentation/hwmon/sysfs-interface might be a good start.

> HWMON driver that have your name in them. I noted when searching that
> I missed out a file in also that Documentation/hwmon. I will correct both
> issues in my next submission attempt.
> 
> > For configuration (XXX_mode), please use devicetreee and/or platform data,
> > not sysfs attributes.
> 
> As far as I can see both devicetreee and platform data allow configuration
> data to be passed into the driver at "probe" time, they don't allow an operating
> mode to be changed dynamically. That is what I thought sysfs allowed. Thus
> your comments seem to imply that you do not want to allow the mode to be
> changed dynamically. If that is the case then I can remove the dynamic mode

Correct.

> setting, leaving it fixed by platform data.
> 

I would also suggest to read and follow Documentation/hwmon/submitting-patches.

Thanks,
Guenter

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2012-08-07 15:09 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-05 20:43 [NEW DRIVER V2 6/7] DA9058 HWMON driver Anthony Olech
2012-08-05 20:43 ` [lm-sensors] " Anthony Olech
2012-08-06 17:39 ` Guenter Roeck
2012-08-06 17:39   ` Guenter Roeck
2012-08-07 12:10   ` Opensource [Anthony Olech]
2012-08-07 12:10     ` Opensource [Anthony Olech]
2012-08-07 15:09     ` Guenter Roeck
2012-08-07 15:09       ` Guenter Roeck

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.