Re: [PATCH/RFC 2/4] hwmon: PMBus device driver
On 06/28/10 22:56, Guenter Roeck wrote:
Hi Guenter,
A few questions and inital comments...
Ouch the pmbus spec is tricky to follow!
> Signed-off-by: Guenter Roeck
> ---
> drivers/hwmon/Kconfig | 12 +
> drivers/hwmon/Makefile |1 +
> drivers/hwmon/pmbus.c | 1227
>
> drivers/hwmon/pmbus.h | 209
> 4 files changed, 1449 insertions(+), 0 deletions(-)
> create mode 100644 drivers/hwmon/pmbus.c
> create mode 100644 drivers/hwmon/pmbus.h
>
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index e19cf8e..8d53cf7 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -702,6 +702,18 @@ config SENSORS_PCF8591
> These devices are hard to detect and rarely found on mainstream
> hardware. If unsure, say N.
>
> +config SENSORS_PMBUS
> + tristate "PMBus devices"
> + depends on I2C && EXPERIMENTAL
> + default n
> + help
> + If you say yes here you get hardware monitoring support for various
> + PMBus devices, including but not limited to BMR45x, LTC2978, MAX16064,
> + MAX8688, and UCD92xx.
> +
> + This driver can also be built as a module. If so, the module will
> + be called pmbus.
> +
> config SENSORS_SHT15
> tristate "Sensiron humidity and temperature sensors. SHT15 and compat."
> depends on GENERIC_GPIO
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 2138ceb..88b043e 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -83,6 +83,7 @@ obj-$(CONFIG_SENSORS_MC13783_ADC)+= mc13783-adc.o
> obj-$(CONFIG_SENSORS_PC87360)+= pc87360.o
> obj-$(CONFIG_SENSORS_PC87427)+= pc87427.o
> obj-$(CONFIG_SENSORS_PCF8591)+= pcf8591.o
> +obj-$(CONFIG_SENSORS_PMBUS) += pmbus.o
> obj-$(CONFIG_SENSORS_S3C)+= s3c-hwmon.o
> obj-$(CONFIG_SENSORS_SHT15) += sht15.o
> obj-$(CONFIG_SENSORS_SIS5595)+= sis5595.o
> diff --git a/drivers/hwmon/pmbus.c b/drivers/hwmon/pmbus.c
> new file mode 100644
> index 000..418ee2c
> --- /dev/null
> +++ b/drivers/hwmon/pmbus.c
> @@ -0,0 +1,1227 @@
> +/*
> + * Hardware monitoring driver for PMBus devices
> + *
> + * Copyright (C) 2010 Ericsson AB.
> + *
> + * 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., 675 Mass Ave, Cambridge, MA 02139, USA.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include "pmbus.h"
> +
> +#define PMBUS_SENSORS64
> +#define PMBUS_BOOLEANS 64
> +#define PMBUS_LABELS 32
> +#define PMBUS_NUM_ATTR (PMBUS_BOOLEANS + PMBUS_SENSORS + PMBUS_LABELS)
> +#define PMBUS_PAGES 8
> +
> +static int pages;
> +module_param(pages, int, 0);
> +MODULE_PARM_DESC(pages, "Number of sensor pages");
Why is this a module parameter? If you do it like this
you will be overriding page count for all devices in the system.
Is that the intent?
> +
> +enum chips { ltc2978, max16064, max8688, pmbus, ucd9220, ucd9240 };
> +
> +enum pmbus_sensor_classes {
> + PSC_VOLTAGE = 0,
> + PSC_TEMPERATURE,
> + PSC_CURRENT,
> + PSC_POWER,
Perhaps name this PSC_MAX_LABEL or similar to indicate it is just here to
allow the number of possible classes to be established.
> + SENSOR_CLASSES
> +};
> +
> +struct pmbus_config {
> + int pages; /* Total number of pages (= output sensors) */
> + bool direct;/* true if device uses direct data format */
> + /*
> + * Support one set of coefficients for each sensor type
> + * Used for chips providing data in direct mode.
> + */
> + int m[SENSOR_CLASSES]; /* mantissa for direct data format */
> + int b[SENSOR_CLASSES]; /* offset */
> + int R[SENSOR_CLASSES]; /* exponent */
> +};
> +
Can we name these something to do with PB to cut down on chance of name
clashes.
> +#define HAVE_STATUS_VOUT (1<<0)
> +#define HAVE_STATUS_IOUT (1<<1)
> +#define HAVE_STATUS_INPUT(1<<2)
> +#define HAVE_STATUS_TEMP (1<<3)
> +
> +#define PB_STATUS_BASE 0
> +#define PB_STATUS_VOUT_BASE (PB_STATUS_BASE + PMBUS_PAGES)
> +#define PB_STATUS_IOUT_BASE (PB_STATUS_VOUT_BASE + PMBUS_PAGES)
> +#define PB_STATUS_INPUT_BASE (PB_STATUS_IOUT_BASE + PMBUS_PAGES)
> +#define PB_STATUS_TEMP_BASE (PB_STATUS_INPUT_BASE + 1)
>
[PATCH/RFC 2/4] hwmon: PMBus device driver
Signed-off-by: Guenter Roeck
---
drivers/hwmon/Kconfig | 12 +
drivers/hwmon/Makefile |1 +
drivers/hwmon/pmbus.c | 1227
drivers/hwmon/pmbus.h | 209
4 files changed, 1449 insertions(+), 0 deletions(-)
create mode 100644 drivers/hwmon/pmbus.c
create mode 100644 drivers/hwmon/pmbus.h
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index e19cf8e..8d53cf7 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -702,6 +702,18 @@ config SENSORS_PCF8591
These devices are hard to detect and rarely found on mainstream
hardware. If unsure, say N.
+config SENSORS_PMBUS
+ tristate "PMBus devices"
+ depends on I2C && EXPERIMENTAL
+ default n
+ help
+ If you say yes here you get hardware monitoring support for various
+ PMBus devices, including but not limited to BMR45x, LTC2978, MAX16064,
+ MAX8688, and UCD92xx.
+
+ This driver can also be built as a module. If so, the module will
+ be called pmbus.
+
config SENSORS_SHT15
tristate "Sensiron humidity and temperature sensors. SHT15 and compat."
depends on GENERIC_GPIO
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 2138ceb..88b043e 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -83,6 +83,7 @@ obj-$(CONFIG_SENSORS_MC13783_ADC)+= mc13783-adc.o
obj-$(CONFIG_SENSORS_PC87360) += pc87360.o
obj-$(CONFIG_SENSORS_PC87427) += pc87427.o
obj-$(CONFIG_SENSORS_PCF8591) += pcf8591.o
+obj-$(CONFIG_SENSORS_PMBUS)+= pmbus.o
obj-$(CONFIG_SENSORS_S3C) += s3c-hwmon.o
obj-$(CONFIG_SENSORS_SHT15)+= sht15.o
obj-$(CONFIG_SENSORS_SIS5595) += sis5595.o
diff --git a/drivers/hwmon/pmbus.c b/drivers/hwmon/pmbus.c
new file mode 100644
index 000..418ee2c
--- /dev/null
+++ b/drivers/hwmon/pmbus.c
@@ -0,0 +1,1227 @@
+/*
+ * Hardware monitoring driver for PMBus devices
+ *
+ * Copyright (C) 2010 Ericsson AB.
+ *
+ * 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., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include "pmbus.h"
+
+#define PMBUS_SENSORS 64
+#define PMBUS_BOOLEANS 64
+#define PMBUS_LABELS 32
+#define PMBUS_NUM_ATTR (PMBUS_BOOLEANS + PMBUS_SENSORS + PMBUS_LABELS)
+#define PMBUS_PAGES8
+
+static int pages;
+module_param(pages, int, 0);
+MODULE_PARM_DESC(pages, "Number of sensor pages");
+
+enum chips { ltc2978, max16064, max8688, pmbus, ucd9220, ucd9240 };
+
+enum pmbus_sensor_classes {
+ PSC_VOLTAGE = 0,
+ PSC_TEMPERATURE,
+ PSC_CURRENT,
+ PSC_POWER,
+ SENSOR_CLASSES
+};
+
+struct pmbus_config {
+ int pages; /* Total number of pages (= output sensors) */
+ bool direct;/* true if device uses direct data format */
+ /*
+* Support one set of coefficients for each sensor type
+* Used for chips providing data in direct mode.
+*/
+ int m[SENSOR_CLASSES]; /* mantissa for direct data format */
+ int b[SENSOR_CLASSES]; /* offset */
+ int R[SENSOR_CLASSES]; /* exponent */
+};
+
+#define HAVE_STATUS_VOUT (1<<0)
+#define HAVE_STATUS_IOUT (1<<1)
+#define HAVE_STATUS_INPUT (1<<2)
+#define HAVE_STATUS_TEMP (1<<3)
+
+#define PB_STATUS_BASE 0
+#define PB_STATUS_VOUT_BASE(PB_STATUS_BASE + PMBUS_PAGES)
+#define PB_STATUS_IOUT_BASE(PB_STATUS_VOUT_BASE + PMBUS_PAGES)
+#define PB_STATUS_INPUT_BASE (PB_STATUS_IOUT_BASE + PMBUS_PAGES)
+#define PB_STATUS_TEMP_BASE(PB_STATUS_INPUT_BASE + 1)
+
+struct pmbus_sensor {
+ char name[I2C_NAME_SIZE]; /* sysfs sensor name */
+ u8 page;/* page number */
+ u8 reg; /* register */
+ enum pmbus_sensor_classes class;/* sensor class */
+ bool update;/* runtime sensor update needed */
+};
+
+struct pmbus_boolean {
+ char name[I2C_NAME_SIZE]; /* sysfs boolean name */
+};
+
+struct pmbus_label {
+ char name[I2C_NAME_SIZE]; /* sysfs label name */
+ char label[I2C_NAME_SIZE]; /* label */
+};
+
+struct pmbus_data {
+ struct device *hwmon_dev;
+ enum chips type;
+
+ bool direct;
+ int pages;
+
+
