* [patch v2 0/2] Add support for Maxim MAX6621 temperature sensor device
@ 2017-08-30 21:55 ` Vadim Pasternak
0 siblings, 0 replies; 9+ messages in thread
From: Vadim Pasternak @ 2017-08-30 21:55 UTC (permalink / raw)
To: linux, robh+dt; +Cc: linux-hwmon, devicetree, jiri, Vadim Pasternak
This patchset:
- support for MAX6621 temperature sensor;
- binding documentation for this device;
Vadim Pasternak (2):
hwmon: Driver for Maxim MAX6621 temperature sensor
dt-bindings: hwmon: add binding documentation for max6621 device
.../devicetree/bindings/hwmon/max6621.txt | 14 +
drivers/hwmon/Kconfig | 14 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/max6621.c | 484 +++++++++++++++++++++
4 files changed, 513 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hwmon/max6621.txt
create mode 100644 drivers/hwmon/max6621.c
--
2.1.4
^ permalink raw reply [flat|nested] 9+ messages in thread
* [patch v2 0/2] Add support for Maxim MAX6621 temperature sensor device
@ 2017-08-30 21:55 ` Vadim Pasternak
0 siblings, 0 replies; 9+ messages in thread
From: Vadim Pasternak @ 2017-08-30 21:55 UTC (permalink / raw)
To: linux-0h96xk9xTtrk1uMJSBkQmQ, robh+dt-DgEjT+Ai2ygdnm+yROfE0A
Cc: linux-hwmon-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, jiri-rHqAuBHg3fBzbRFIqnYvSA,
Vadim Pasternak
This patchset:
- support for MAX6621 temperature sensor;
- binding documentation for this device;
Vadim Pasternak (2):
hwmon: Driver for Maxim MAX6621 temperature sensor
dt-bindings: hwmon: add binding documentation for max6621 device
.../devicetree/bindings/hwmon/max6621.txt | 14 +
drivers/hwmon/Kconfig | 14 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/max6621.c | 484 +++++++++++++++++++++
4 files changed, 513 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hwmon/max6621.txt
create mode 100644 drivers/hwmon/max6621.c
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 9+ messages in thread
* [patch v2 1/2] hwmon: Driver for Maxim MAX6621 temperature sensor
2017-08-30 21:55 ` Vadim Pasternak
(?)
@ 2017-08-30 21:55 ` Vadim Pasternak
2017-09-01 15:30 ` [v2,1/2] " Guenter Roeck
-1 siblings, 1 reply; 9+ messages in thread
From: Vadim Pasternak @ 2017-08-30 21:55 UTC (permalink / raw)
To: linux, robh+dt; +Cc: linux-hwmon, devicetree, jiri, Vadim Pasternak
MAX6621 is a PECI-to-I2C translator provides an efficient, low-cost
solution for PECI-to-SMBus/I2C protocol conversion. It allows reading the
temperature from the PECI-compliant host directly from up to four
PECI-enabled CPUs.
Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
---
v1->v2
Comments pointed out by Guenter:
- arrange includes in alphabetic order;
- add include for bitops.h;
- remove MAX6621_REG_NON_WRITEABLE_REG;
- drop temp_min, temp_max, temp_reset_history attributes;
- remove redundant braces in max6621_verify_reg_data;
- fix return code in max6621_verify_reg_data;
- not report channels which are not physically connected;
- use u32 for regval in max6621_read and max6621_write;
- provide in temprature offset in milli-degrees C;
- drop hwmon_temp_fault attribute;
- drop activation point setting in CONFIG2 reg, leave it for user space;
Comments pointed out by Jiri:
- fixe device name;
Fixes added by Vadim:
- modify names in max6621_temp_labels;
- add hwmon_temp_alarm attribute and hwmon_temp_crit attributes per
socket;
- fix defines for MIN and MAX temperature;
---
drivers/hwmon/Kconfig | 14 ++
drivers/hwmon/Makefile | 1 +
drivers/hwmon/max6621.c | 484 ++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 499 insertions(+)
create mode 100644 drivers/hwmon/max6621.c
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 5b9a61f..325d161 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -855,6 +855,20 @@ tristate "MAX31722 temperature sensor"
This driver can also be built as a module. If so, the module
will be called max31722.
+config SENSORS_MAX6621
+ tristate "Maxim MAX6621 sensor chip"
+ depends on I2C
+ select REGMAP_I2C
+ help
+ If you say yes here you get support for MAX6621 sensor chip.
+ MAX6621 is a PECI-to-I2C translator provides an efficient,
+ low-cost solution for PECI-to-SMBus/I2C protocol conversion.
+ It allows reading the temperature from the PECI-compliant
+ host directly from up to four PECI-enabled CPUs.
+
+ This driver can also be built as a module. If so, the module
+ will be called max6621.
+
config SENSORS_MAX6639
tristate "Maxim MAX6639 sensor chip"
depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index d4641a9..43333cb 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -116,6 +116,7 @@ obj-$(CONFIG_SENSORS_MAX1619) += max1619.o
obj-$(CONFIG_SENSORS_MAX1668) += max1668.o
obj-$(CONFIG_SENSORS_MAX197) += max197.o
obj-$(CONFIG_SENSORS_MAX31722) += max31722.o
+obj-$(CONFIG_SENSORS_MAX6621) += max6621.o
obj-$(CONFIG_SENSORS_MAX6639) += max6639.o
obj-$(CONFIG_SENSORS_MAX6642) += max6642.o
obj-$(CONFIG_SENSORS_MAX6650) += max6650.o
diff --git a/drivers/hwmon/max6621.c b/drivers/hwmon/max6621.c
new file mode 100644
index 0000000..2ad7fad
--- /dev/null
+++ b/drivers/hwmon/max6621.c
@@ -0,0 +1,484 @@
+/*
+ * Hardware monitoring driver for Maxim MAX6621
+ *
+ * Copyright (c) 2017 Mellanox Technologies. All rights reserved.
+ * Copyright (c) 2017 Vadim Pasternak <vadimp@mellanox.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/bitops.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/of_device.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+
+#define MAX6621_DRV_NAME "max6621"
+#define MAX6621_TEMP_INPUT_REG_NUM 9
+#define MAX6621_TEMP_INPUT_MIN -20000
+#define MAX6621_TEMP_INPUT_MAX 120000
+
+#define MAX6621_TEMP_S0D0_REG 0x00
+#define MAX6621_TEMP_S0D1_REG 0x01
+#define MAX6621_TEMP_S1D0_REG 0x02
+#define MAX6621_TEMP_S1D1_REG 0x03
+#define MAX6621_TEMP_S2D0_REG 0x04
+#define MAX6621_TEMP_S2D1_REG 0x05
+#define MAX6621_TEMP_S3D0_REG 0x06
+#define MAX6621_TEMP_S3D1_REG 0x07
+#define MAX6621_TEMP_MAX_REG 0x08
+#define MAX6621_TEMP_ALERT_CAUSE 0x0b
+#define MAX6621_TEMP_MAX_ADDR_REG 0x0a
+#define MAX6621_CONFIG0_REG 0x0c
+#define MAX6621_CONFIG1_REG 0x0d
+#define MAX6621_CONFIG2_REG 0x0e
+#define MAX6621_CONFIG3_REG 0x0f
+#define MAX6621_TEMP_S0_ALERT_REG 0x10
+#define MAX6621_TEMP_S1_ALERT_REG 0x11
+#define MAX6621_TEMP_S2_ALERT_REG 0x12
+#define MAX6621_TEMP_S3_ALERT_REG 0x13
+#define MAX6621_CLEAR_ALERT_REG 0x15
+#define MAX6621_REG_MAX (MAX6621_CLEAR_ALERT_REG + 1)
+#define MAX6621_REG_TEMP_SHIFT 0x06
+
+#define MAX6621_ENABLE_TEMP_ALERTS_BIT 4
+#define MAX6621_ENABLE_I2C_CRC_BIT 5
+#define MAX6621_ENABLE_ALTERNATE_DATA 6
+#define MAX6621_ENABLE_LOCKUP_TO 7
+#define MAX6621_ENABLE_S0D0_BIT 8
+#define MAX6621_ENABLE_S0D1_BIT 9
+#define MAX6621_ENABLE_S1D0_BIT 10
+#define MAX6621_ENABLE_S1D1_BIT 11
+#define MAX6621_ENABLE_S2D0_BIT 12
+#define MAX6621_ENABLE_S2D1_BIT 13
+#define MAX6621_ENABLE_S3D0_BIT 14
+#define MAX6621_ENABLE_S3D1_BIT 15
+#define MAX6621_ENABLE_TEMP_ALL GENMASK(MAX6621_ENABLE_S3D1_BIT, \
+ MAX6621_ENABLE_S0D0_BIT)
+#define MAX6621_POLL_DELAY_MASK 0x5
+#define MAX6621_CONFIG0_INIT (MAX6621_ENABLE_TEMP_ALL | \
+ BIT(MAX6621_ENABLE_LOCKUP_TO) | \
+ BIT(MAX6621_ENABLE_I2C_CRC_BIT) | \
+ MAX6621_POLL_DELAY_MASK)
+
+/* Error codes */
+#define MAX6621_TRAN_FAILED 0x8100 /*
+ * PECI transaction failed for more
+ * than the configured number of
+ * consecutive retries.
+ */
+#define MAX6621_POOL_DIS 0x8101 /*
+ * Polling disabled for requested
+ * socket/domain.
+ */
+#define MAX6621_POOL_UNCOMPLETE 0x8102 /*
+ * First poll not yet completed for
+ * requested socket/domain (on
+ * startup).
+ */
+#define MAX6621_SD_DIS 0x8103 /*
+ * Read maximum temperature requested,
+ * but no sockets/domains enabled or
+ * all enabled sockets/domains have
+ * errors; or read maximum temperature
+ * address requested, but read maximum
+ * temperature was not called.
+ */
+#define MAX6621_ALERT_DIS 0x8104 /*
+ * Get alert socket/domain requested,
+ * but no alert active.
+ */
+#define MAX6621_PECI_ERR_MIN 0x8000 /* Intel spec PECI error min value. */
+#define MAX6621_PECI_ERR_MAX 0x80ff /* Intel spec PECI error max value. */
+
+static const u32 max6621_temp_regs[] = {
+ MAX6621_TEMP_MAX_REG, MAX6621_TEMP_S0D0_REG, MAX6621_TEMP_S1D0_REG,
+ MAX6621_TEMP_S2D0_REG, MAX6621_TEMP_S3D0_REG, MAX6621_TEMP_S0D1_REG,
+ MAX6621_TEMP_S1D1_REG, MAX6621_TEMP_S2D1_REG, MAX6621_TEMP_S3D1_REG,
+};
+
+static const char *const max6621_temp_labels[] = {
+ "maximum",
+ "socket0_0",
+ "socket1_0",
+ "socket2_0",
+ "socket3_0",
+ "socket0_1",
+ "socket1_1",
+ "socket2_1",
+ "socket3_1",
+};
+
+static const int max6621_temp_alert_chan2reg[] = {
+ MAX6621_CLEAR_ALERT_REG,
+ MAX6621_TEMP_S0_ALERT_REG,
+ MAX6621_TEMP_S1_ALERT_REG,
+ MAX6621_TEMP_S2_ALERT_REG,
+ MAX6621_TEMP_S3_ALERT_REG,
+};
+
+/**
+ * struct max6621_data - private data:
+ *
+ * @client: I2C client;
+ * @regmap: register map handle;
+ * @temp_offset: offset that is added to all temperature return;
+ * @input_chan2reg: mapping from channel to register;
+ */
+struct max6621_data {
+ struct i2c_client *client;
+ struct regmap *regmap;
+ u16 temp_offset;
+ int input_chan2reg[MAX6621_TEMP_INPUT_REG_NUM + 1];
+};
+
+static long max6621_temp_mc2reg(long val)
+{
+ return (val / 1000L) << MAX6621_REG_TEMP_SHIFT;
+}
+
+static umode_t
+max6621_is_visible(const void *data, enum hwmon_sensor_types type, u32 attr,
+ int channel)
+{
+ /* Skip channels which are not physically conncted. */
+ if (((struct max6621_data *)data)->input_chan2reg[channel] < 0)
+ return 0;
+
+ switch (type) {
+ case hwmon_temp:
+ switch (attr) {
+ case hwmon_temp_input:
+ return 0444;
+ case hwmon_temp_label:
+ case hwmon_temp_offset:
+ case hwmon_temp_crit:
+ case hwmon_temp_alarm:
+ return 0644;
+ default:
+ break;
+ }
+
+ default:
+ break;
+ }
+
+ return 0;
+}
+
+static int max6621_verify_reg_data(struct device *dev, int regval)
+{
+ if (regval >= MAX6621_PECI_ERR_MIN && regval <=
+ MAX6621_PECI_ERR_MAX) {
+ dev_dbg(dev, "PECI error code - err 0x%04x.\n",
+ regval);
+
+ return -EINVAL;
+ }
+
+ switch (regval) {
+ case MAX6621_TRAN_FAILED:
+ dev_dbg(dev, "PECI transaction failed - err 0x%04x.\n",
+ regval);
+ break;
+ case MAX6621_POOL_DIS:
+ dev_dbg(dev, "Polling disabled - err 0x%04x.\n", regval);
+ break;
+ case MAX6621_POOL_UNCOMPLETE:
+ dev_dbg(dev, "First poll not completed on startup - err 0x%04x.\n",
+ regval);
+ break;
+ case MAX6621_SD_DIS:
+ dev_dbg(dev, "Resource is disabled - err 0x%04x.\n", regval);
+ break;
+ case MAX6621_ALERT_DIS:
+ dev_dbg(dev, "No alert active - err 0x%04x.\n", regval);
+ break;
+ default:
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
+static int
+max6621_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
+ int channel, long *val)
+{
+ struct max6621_data *data = dev_get_drvdata(dev);
+ u32 regval;
+ int reg;
+ s8 temp;
+ int ret;
+
+ switch (type) {
+ case hwmon_temp:
+ switch (attr) {
+ case hwmon_temp_input:
+ reg = data->input_chan2reg[channel];
+ ret = regmap_read(data->regmap, reg, ®val);
+ if (ret)
+ return ret;
+
+ ret = max6621_verify_reg_data(dev, regval);
+ if (ret)
+ return ret;
+
+ temp = (regval >> MAX6621_REG_TEMP_SHIFT);
+ *val = temp * 1000L;
+
+ break;
+ case hwmon_temp_offset:
+ *val = (data->temp_offset >> MAX6621_REG_TEMP_SHIFT) *
+ 1000L;
+
+ break;
+ case hwmon_temp_crit:
+ reg = max6621_temp_alert_chan2reg[channel];
+ ret = regmap_read(data->regmap, reg, ®val);
+ if (ret)
+ return ret;
+
+ ret = max6621_verify_reg_data(dev, regval);
+ if (ret)
+ return ret;
+
+ *val = regval * 1000L;
+
+ break;
+ case hwmon_temp_alarm:
+ ret = regmap_read(data->regmap,
+ MAX6621_TEMP_ALERT_CAUSE,
+ ®val);
+ if (ret)
+ return ret;
+
+ ret = max6621_verify_reg_data(dev, regval);
+ if (ret < 0)
+ return ret;
+
+ *val = !!regval;
+
+ break;
+ default:
+ return -EOPNOTSUPP;
+ }
+ break;
+
+ default:
+ return -EOPNOTSUPP;
+ }
+
+ return 0;
+}
+
+static int
+max6621_write(struct device *dev, enum hwmon_sensor_types type, u32 attr,
+ int channel, long val)
+{
+ struct max6621_data *data = dev_get_drvdata(dev);
+ u32 reg;
+
+ switch (type) {
+ case hwmon_temp:
+ switch (attr) {
+ case hwmon_temp_offset:
+ /* Clamp to allowed range to prevent overflow. */
+ val = clamp_val(val, MAX6621_TEMP_INPUT_MIN,
+ MAX6621_TEMP_INPUT_MAX);
+ val = max6621_temp_mc2reg(val);
+ if (data->temp_offset != val) {
+ data->temp_offset = val;
+ return regmap_write(data->regmap,
+ MAX6621_CONFIG2_REG, val);
+ }
+
+ return 0;
+ case hwmon_temp_crit:
+ reg = max6621_temp_alert_chan2reg[channel];
+ /* Clamp to allowed range to prevent overflow. */
+ val = clamp_val(val, MAX6621_TEMP_INPUT_MIN,
+ MAX6621_TEMP_INPUT_MAX);
+ val = val / 1000L;
+
+ return regmap_write(data->regmap, reg, val);
+ case hwmon_temp_alarm:
+ /*
+ * Use i2c_smbus_write_byte, since
+ * MAX6621_CLEAR_ALERT_REG access expects send-byte
+ * smbus protocol for clearing alert.
+ */
+ return i2c_smbus_write_byte(data->client,
+ MAX6621_CLEAR_ALERT_REG);
+ default:
+ return -EOPNOTSUPP;
+ }
+ break;
+
+ default:
+ return -EOPNOTSUPP;
+ }
+
+ return -EOPNOTSUPP;
+}
+
+static int
+max6621_read_string(struct device *dev, enum hwmon_sensor_types type, u32 attr,
+ int channel, const char **str)
+{
+ switch (type) {
+ case hwmon_temp:
+ switch (attr) {
+ case hwmon_temp_label:
+ *str = max6621_temp_labels[channel];
+ return 0;
+ default:
+ return -EOPNOTSUPP;
+ }
+ break;
+ default:
+ return -EOPNOTSUPP;
+ }
+
+ return -EOPNOTSUPP;
+}
+
+static const struct regmap_config max6621_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 16,
+ .max_register = MAX6621_REG_MAX,
+ .val_format_endian = REGMAP_ENDIAN_LITTLE,
+};
+
+static u32 max6621_chip_config[] = {
+ HWMON_C_REGISTER_TZ,
+ 0
+};
+
+static const struct hwmon_channel_info max6621_chip = {
+ .type = hwmon_chip,
+ .config = max6621_chip_config,
+};
+
+static const u32 max6621_temp_config[] = {
+ HWMON_T_INPUT | HWMON_T_ALARM | HWMON_T_LABEL | HWMON_T_OFFSET,
+ HWMON_T_INPUT | HWMON_T_CRIT | HWMON_T_LABEL,
+ HWMON_T_INPUT | HWMON_T_CRIT | HWMON_T_LABEL,
+ HWMON_T_INPUT | HWMON_T_CRIT | HWMON_T_LABEL,
+ HWMON_T_INPUT | HWMON_T_CRIT,
+ HWMON_T_INPUT | HWMON_T_LABEL,
+ HWMON_T_INPUT | HWMON_T_LABEL,
+ HWMON_T_INPUT | HWMON_T_LABEL,
+ HWMON_T_INPUT | HWMON_T_LABEL,
+ 0
+};
+
+static const struct hwmon_channel_info max6621_temp = {
+ .type = hwmon_temp,
+ .config = max6621_temp_config,
+};
+
+static const struct hwmon_channel_info *max6621_info[] = {
+ &max6621_chip,
+ &max6621_temp,
+ NULL
+};
+
+static const struct hwmon_ops max6621_hwmon_ops = {
+ .read = max6621_read,
+ .write = max6621_write,
+ .read_string = max6621_read_string,
+ .is_visible = max6621_is_visible,
+};
+
+static const struct hwmon_chip_info max6621_chip_info = {
+ .ops = &max6621_hwmon_ops,
+ .info = max6621_info,
+};
+
+static int max6621_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct device *dev = &client->dev;
+ struct max6621_data *data;
+ struct device *hwmon_dev;
+ int i;
+ int ret;
+
+ data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ data->regmap = devm_regmap_init_i2c(client, &max6621_regmap_config);
+ if (IS_ERR(data->regmap))
+ return PTR_ERR(data->regmap);
+
+ i2c_set_clientdata(client, data);
+ data->client = client;
+
+ /* Set CONFIG0 register masking temperature alerts and PEC. */
+ ret = regmap_write(data->regmap, MAX6621_CONFIG0_REG,
+ MAX6621_CONFIG0_INIT);
+ if (ret)
+ return ret;
+
+ /* Verify which temperature input registers are enabled. */
+ for (i = 0; i < MAX6621_TEMP_INPUT_REG_NUM; i++) {
+ ret = i2c_smbus_read_word_data(client, max6621_temp_regs[i]);
+ if (ret < 0)
+ return ret;
+ ret = max6621_verify_reg_data(dev, ret);
+ if (ret) {
+ data->input_chan2reg[i] = -1;
+ continue;
+ }
+
+ data->input_chan2reg[i] = max6621_temp_regs[i];
+ }
+
+ hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name,
+ data,
+ &max6621_chip_info,
+ NULL);
+
+ return PTR_ERR_OR_ZERO(hwmon_dev);
+}
+
+static const struct i2c_device_id max6621_id[] = {
+ { MAX6621_DRV_NAME, 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, max6621_id);
+
+static const struct of_device_id max6621_of_match[] = {
+ { .compatible = "maxim,max6621" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, max6621_of_match);
+
+static struct i2c_driver max6621_driver = {
+ .class = I2C_CLASS_HWMON,
+ .driver = {
+ .name = MAX6621_DRV_NAME,
+ .of_match_table = of_match_ptr(max6621_of_match),
+ },
+ .probe = max6621_probe,
+ .id_table = max6621_id,
+};
+
+module_i2c_driver(max6621_driver);
+
+MODULE_AUTHOR("Vadim Pasternak <vadimp@mellanox.com>");
+MODULE_DESCRIPTION("Driver for Maxim MAX6621");
+MODULE_LICENSE("GPL");
--
2.1.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [patch v2 2/2] dt-bindings: hwmon: add binding documentation for max6621 device
@ 2017-08-30 21:55 ` Vadim Pasternak
0 siblings, 0 replies; 9+ messages in thread
From: Vadim Pasternak @ 2017-08-30 21:55 UTC (permalink / raw)
To: linux, robh+dt; +Cc: linux-hwmon, devicetree, jiri, Vadim Pasternak
Add binding document for Maxim MAX6621 temperature sensor device.
Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
---
Documentation/devicetree/bindings/hwmon/max6621.txt | 14 ++++++++++++++
1 file changed, 14 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hwmon/max6621.txt
diff --git a/Documentation/devicetree/bindings/hwmon/max6621.txt b/Documentation/devicetree/bindings/hwmon/max6621.txt
new file mode 100644
index 0000000..c1f7e95
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/max6621.txt
@@ -0,0 +1,14 @@
+MAX6621 temperature sensor
+-------------------------
+
+This device supports I2C only.
+
+Requires node properties:
+- compatible : "maxim,max6621"
+- reg : the I2C address of the device.
+
+Example:
+ max6621@2b {
+ compatible = "maxim,max6621";
+ reg = <0x2b>;
+ };
--
2.1.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [patch v2 2/2] dt-bindings: hwmon: add binding documentation for max6621 device
@ 2017-08-30 21:55 ` Vadim Pasternak
0 siblings, 0 replies; 9+ messages in thread
From: Vadim Pasternak @ 2017-08-30 21:55 UTC (permalink / raw)
To: linux-0h96xk9xTtrk1uMJSBkQmQ, robh+dt-DgEjT+Ai2ygdnm+yROfE0A
Cc: linux-hwmon-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, jiri-rHqAuBHg3fBzbRFIqnYvSA,
Vadim Pasternak
Add binding document for Maxim MAX6621 temperature sensor device.
Signed-off-by: Vadim Pasternak <vadimp-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
Documentation/devicetree/bindings/hwmon/max6621.txt | 14 ++++++++++++++
1 file changed, 14 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hwmon/max6621.txt
diff --git a/Documentation/devicetree/bindings/hwmon/max6621.txt b/Documentation/devicetree/bindings/hwmon/max6621.txt
new file mode 100644
index 0000000..c1f7e95
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/max6621.txt
@@ -0,0 +1,14 @@
+MAX6621 temperature sensor
+-------------------------
+
+This device supports I2C only.
+
+Requires node properties:
+- compatible : "maxim,max6621"
+- reg : the I2C address of the device.
+
+Example:
+ max6621@2b {
+ compatible = "maxim,max6621";
+ reg = <0x2b>;
+ };
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [v2,1/2] hwmon: Driver for Maxim MAX6621 temperature sensor
2017-08-30 21:55 ` [patch v2 1/2] hwmon: Driver for Maxim MAX6621 temperature sensor Vadim Pasternak
@ 2017-09-01 15:30 ` Guenter Roeck
0 siblings, 0 replies; 9+ messages in thread
From: Guenter Roeck @ 2017-09-01 15:30 UTC (permalink / raw)
To: Vadim Pasternak; +Cc: robh+dt, linux-hwmon, devicetree, jiri
On Wed, Aug 30, 2017 at 09:55:56PM +0000, Vadim Pasternak wrote:
> MAX6621 is a PECI-to-I2C translator provides an efficient, low-cost
> solution for PECI-to-SMBus/I2C protocol conversion. It allows reading the
> temperature from the PECI-compliant host directly from up to four
> PECI-enabled CPUs.
>
> Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
> ---
> v1->v2
> Comments pointed out by Guenter:
> - arrange includes in alphabetic order;
> - add include for bitops.h;
> - remove MAX6621_REG_NON_WRITEABLE_REG;
> - drop temp_min, temp_max, temp_reset_history attributes;
> - remove redundant braces in max6621_verify_reg_data;
> - fix return code in max6621_verify_reg_data;
> - not report channels which are not physically connected;
> - use u32 for regval in max6621_read and max6621_write;
> - provide in temprature offset in milli-degrees C;
> - drop hwmon_temp_fault attribute;
> - drop activation point setting in CONFIG2 reg, leave it for user space;
> Comments pointed out by Jiri:
> - fixe device name;
> Fixes added by Vadim:
> - modify names in max6621_temp_labels;
> - add hwmon_temp_alarm attribute and hwmon_temp_crit attributes per
> socket;
> - fix defines for MIN and MAX temperature;
> ---
> drivers/hwmon/Kconfig | 14 ++
> drivers/hwmon/Makefile | 1 +
> drivers/hwmon/max6621.c | 484 ++++++++++++++++++++++++++++++++++++++++++++++++
Please also provide Documentation/hwmon/max6621 to describe the various
attributes.
>
> 3 files changed, 499 insertions(+)
> create mode 100644 drivers/hwmon/max6621.c
>
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 5b9a61f..325d161 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -855,6 +855,20 @@ tristate "MAX31722 temperature sensor"
> This driver can also be built as a module. If so, the module
> will be called max31722.
>
> +config SENSORS_MAX6621
> + tristate "Maxim MAX6621 sensor chip"
> + depends on I2C
> + select REGMAP_I2C
> + help
> + If you say yes here you get support for MAX6621 sensor chip.
> + MAX6621 is a PECI-to-I2C translator provides an efficient,
> + low-cost solution for PECI-to-SMBus/I2C protocol conversion.
> + It allows reading the temperature from the PECI-compliant
> + host directly from up to four PECI-enabled CPUs.
> +
> + This driver can also be built as a module. If so, the module
> + will be called max6621.
> +
> config SENSORS_MAX6639
> tristate "Maxim MAX6639 sensor chip"
> depends on I2C
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index d4641a9..43333cb 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -116,6 +116,7 @@ obj-$(CONFIG_SENSORS_MAX1619) += max1619.o
> obj-$(CONFIG_SENSORS_MAX1668) += max1668.o
> obj-$(CONFIG_SENSORS_MAX197) += max197.o
> obj-$(CONFIG_SENSORS_MAX31722) += max31722.o
> +obj-$(CONFIG_SENSORS_MAX6621) += max6621.o
> obj-$(CONFIG_SENSORS_MAX6639) += max6639.o
> obj-$(CONFIG_SENSORS_MAX6642) += max6642.o
> obj-$(CONFIG_SENSORS_MAX6650) += max6650.o
> diff --git a/drivers/hwmon/max6621.c b/drivers/hwmon/max6621.c
> new file mode 100644
> index 0000000..2ad7fad
> --- /dev/null
> +++ b/drivers/hwmon/max6621.c
> @@ -0,0 +1,484 @@
> +/*
> + * Hardware monitoring driver for Maxim MAX6621
> + *
> + * Copyright (c) 2017 Mellanox Technologies. All rights reserved.
> + * Copyright (c) 2017 Vadim Pasternak <vadimp@mellanox.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/of_device.h>
> +#include <linux/module.h>
Not really alphabetical.
> +#include <linux/regmap.h>
> +
> +#define MAX6621_DRV_NAME "max6621"
> +#define MAX6621_TEMP_INPUT_REG_NUM 9
> +#define MAX6621_TEMP_INPUT_MIN -20000
> +#define MAX6621_TEMP_INPUT_MAX 120000
> +
> +#define MAX6621_TEMP_S0D0_REG 0x00
> +#define MAX6621_TEMP_S0D1_REG 0x01
> +#define MAX6621_TEMP_S1D0_REG 0x02
> +#define MAX6621_TEMP_S1D1_REG 0x03
> +#define MAX6621_TEMP_S2D0_REG 0x04
> +#define MAX6621_TEMP_S2D1_REG 0x05
> +#define MAX6621_TEMP_S3D0_REG 0x06
> +#define MAX6621_TEMP_S3D1_REG 0x07
> +#define MAX6621_TEMP_MAX_REG 0x08
> +#define MAX6621_TEMP_ALERT_CAUSE 0x0b
> +#define MAX6621_TEMP_MAX_ADDR_REG 0x0a
> +#define MAX6621_CONFIG0_REG 0x0c
> +#define MAX6621_CONFIG1_REG 0x0d
> +#define MAX6621_CONFIG2_REG 0x0e
> +#define MAX6621_CONFIG3_REG 0x0f
> +#define MAX6621_TEMP_S0_ALERT_REG 0x10
> +#define MAX6621_TEMP_S1_ALERT_REG 0x11
> +#define MAX6621_TEMP_S2_ALERT_REG 0x12
> +#define MAX6621_TEMP_S3_ALERT_REG 0x13
> +#define MAX6621_CLEAR_ALERT_REG 0x15
> +#define MAX6621_REG_MAX (MAX6621_CLEAR_ALERT_REG + 1)
> +#define MAX6621_REG_TEMP_SHIFT 0x06
> +
> +#define MAX6621_ENABLE_TEMP_ALERTS_BIT 4
> +#define MAX6621_ENABLE_I2C_CRC_BIT 5
> +#define MAX6621_ENABLE_ALTERNATE_DATA 6
> +#define MAX6621_ENABLE_LOCKUP_TO 7
> +#define MAX6621_ENABLE_S0D0_BIT 8
> +#define MAX6621_ENABLE_S0D1_BIT 9
> +#define MAX6621_ENABLE_S1D0_BIT 10
> +#define MAX6621_ENABLE_S1D1_BIT 11
> +#define MAX6621_ENABLE_S2D0_BIT 12
> +#define MAX6621_ENABLE_S2D1_BIT 13
> +#define MAX6621_ENABLE_S3D0_BIT 14
> +#define MAX6621_ENABLE_S3D1_BIT 15
> +#define MAX6621_ENABLE_TEMP_ALL GENMASK(MAX6621_ENABLE_S3D1_BIT, \
> + MAX6621_ENABLE_S0D0_BIT)
> +#define MAX6621_POLL_DELAY_MASK 0x5
> +#define MAX6621_CONFIG0_INIT (MAX6621_ENABLE_TEMP_ALL | \
> + BIT(MAX6621_ENABLE_LOCKUP_TO) | \
> + BIT(MAX6621_ENABLE_I2C_CRC_BIT) | \
> + MAX6621_POLL_DELAY_MASK)
> +
> +/* Error codes */
> +#define MAX6621_TRAN_FAILED 0x8100 /*
> + * PECI transaction failed for more
> + * than the configured number of
> + * consecutive retries.
> + */
> +#define MAX6621_POOL_DIS 0x8101 /*
> + * Polling disabled for requested
> + * socket/domain.
> + */
> +#define MAX6621_POOL_UNCOMPLETE 0x8102 /*
> + * First poll not yet completed for
> + * requested socket/domain (on
> + * startup).
> + */
> +#define MAX6621_SD_DIS 0x8103 /*
> + * Read maximum temperature requested,
> + * but no sockets/domains enabled or
> + * all enabled sockets/domains have
> + * errors; or read maximum temperature
> + * address requested, but read maximum
> + * temperature was not called.
> + */
> +#define MAX6621_ALERT_DIS 0x8104 /*
> + * Get alert socket/domain requested,
> + * but no alert active.
> + */
> +#define MAX6621_PECI_ERR_MIN 0x8000 /* Intel spec PECI error min value. */
> +#define MAX6621_PECI_ERR_MAX 0x80ff /* Intel spec PECI error max value. */
> +
> +static const u32 max6621_temp_regs[] = {
> + MAX6621_TEMP_MAX_REG, MAX6621_TEMP_S0D0_REG, MAX6621_TEMP_S1D0_REG,
> + MAX6621_TEMP_S2D0_REG, MAX6621_TEMP_S3D0_REG, MAX6621_TEMP_S0D1_REG,
> + MAX6621_TEMP_S1D1_REG, MAX6621_TEMP_S2D1_REG, MAX6621_TEMP_S3D1_REG,
> +};
> +
> +static const char *const max6621_temp_labels[] = {
> + "maximum",
> + "socket0_0",
> + "socket1_0",
> + "socket2_0",
> + "socket3_0",
> + "socket0_1",
> + "socket1_1",
> + "socket2_1",
> + "socket3_1",
> +};
> +
> +static const int max6621_temp_alert_chan2reg[] = {
> + MAX6621_CLEAR_ALERT_REG,
Writing temp1_crit is supposed to clear alerts ? That won't work
because the 1st channel does not have a critical temperature limit
configured. Besides, even if it worked, it would be highly unusual
and a misuse of the ABI.
> + MAX6621_TEMP_S0_ALERT_REG,
> + MAX6621_TEMP_S1_ALERT_REG,
> + MAX6621_TEMP_S2_ALERT_REG,
> + MAX6621_TEMP_S3_ALERT_REG,
> +};
> +
> +/**
> + * struct max6621_data - private data:
> + *
> + * @client: I2C client;
> + * @regmap: register map handle;
> + * @temp_offset: offset that is added to all temperature return;
> + * @input_chan2reg: mapping from channel to register;
> + */
> +struct max6621_data {
> + struct i2c_client *client;
> + struct regmap *regmap;
> + u16 temp_offset;
Unless I am missing something, this is not initialized, meaning it will return 0
until written, no matter what the chip is actually configured for.
I would suggest to use regmap, and use regmap for caching the value
if you want to avoid re-reading it. At the very least you should read
the value when instantiating the driver.
> + int input_chan2reg[MAX6621_TEMP_INPUT_REG_NUM + 1];
> +};
> +
> +static long max6621_temp_mc2reg(long val)
> +{
> + return (val / 1000L) << MAX6621_REG_TEMP_SHIFT;
> +}
> +
> +static umode_t
> +max6621_is_visible(const void *data, enum hwmon_sensor_types type, u32 attr,
> + int channel)
> +{
> + /* Skip channels which are not physically conncted. */
> + if (((struct max6621_data *)data)->input_chan2reg[channel] < 0)
> + return 0;
> +
> + switch (type) {
> + case hwmon_temp:
> + switch (attr) {
> + case hwmon_temp_input:
> + return 0444;
> + case hwmon_temp_label:
temp_label is supposed to be writeable ? Not that it works,
but that is not the idea with labels. Those are supposed to
be constant. For dynamic labels use sensors3.conf.
> + case hwmon_temp_offset:
> + case hwmon_temp_crit:
> + case hwmon_temp_alarm:
> + return 0644;
> + default:
> + break;
> + }
> +
> + default:
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static int max6621_verify_reg_data(struct device *dev, int regval)
> +{
> + if (regval >= MAX6621_PECI_ERR_MIN && regval <=
> + MAX6621_PECI_ERR_MAX) {
Personally I prefer something like
if (regval >= MAX6621_PECI_ERR_MIN &&
regval <= MAX6621_PECI_ERR_MAX) {
for line splits like that; I think it is much easier to read.
> + dev_dbg(dev, "PECI error code - err 0x%04x.\n",
> + regval);
> +
> + return -EINVAL;
> + }
> +
> + switch (regval) {
> + case MAX6621_TRAN_FAILED:
> + dev_dbg(dev, "PECI transaction failed - err 0x%04x.\n",
> + regval);
> + break;
> + case MAX6621_POOL_DIS:
> + dev_dbg(dev, "Polling disabled - err 0x%04x.\n", regval);
> + break;
> + case MAX6621_POOL_UNCOMPLETE:
> + dev_dbg(dev, "First poll not completed on startup - err 0x%04x.\n",
> + regval);
> + break;
> + case MAX6621_SD_DIS:
> + dev_dbg(dev, "Resource is disabled - err 0x%04x.\n", regval);
> + break;
> + case MAX6621_ALERT_DIS:
> + dev_dbg(dev, "No alert active - err 0x%04x.\n", regval);
> + break;
> + default:
> + return 0;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int
> +max6621_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
> + int channel, long *val)
> +{
> + struct max6621_data *data = dev_get_drvdata(dev);
> + u32 regval;
> + int reg;
> + s8 temp;
> + int ret;
> +
> + switch (type) {
> + case hwmon_temp:
> + switch (attr) {
> + case hwmon_temp_input:
> + reg = data->input_chan2reg[channel];
> + ret = regmap_read(data->regmap, reg, ®val);
> + if (ret)
> + return ret;
> +
> + ret = max6621_verify_reg_data(dev, regval);
> + if (ret)
> + return ret;
> +
> + temp = (regval >> MAX6621_REG_TEMP_SHIFT);
> + *val = temp * 1000L;
Not that I understand the returned temperature value in the datasheet,
but is 8 bit really sufficient to hold the value ? Maybe it is; just
wondering.
> +
> + break;
> + case hwmon_temp_offset:
> + *val = (data->temp_offset >> MAX6621_REG_TEMP_SHIFT) *
> + 1000L;
> +
> + break;
> + case hwmon_temp_crit:
> + reg = max6621_temp_alert_chan2reg[channel];
> + ret = regmap_read(data->regmap, reg, ®val);
> + if (ret)
> + return ret;
> +
> + ret = max6621_verify_reg_data(dev, regval);
> + if (ret)
> + return ret;
> +
> + *val = regval * 1000L;
> +
> + break;
> + case hwmon_temp_alarm:
Since the temperature limits are declared as _crit attributes,
hwmon_temp_crit_alarm may be more appropriate here.
> + ret = regmap_read(data->regmap,
> + MAX6621_TEMP_ALERT_CAUSE,
> + ®val);
> + if (ret)
> + return ret;
> +
> + ret = max6621_verify_reg_data(dev, regval);
> + if (ret < 0)
> + return ret;
> +
> + *val = !!regval;
> +
I don't think that works as intended. The return value is supposed to be 0 or 1.
However, if there is no alert, presumably reading MAX6621_TEMP_ALERT_CAUSE
should result in MAX6621_ALERT_DIS, which would abort above since
max6621_verify_reg_data() would return an error.
I see that the alarm is associated with channel 0, the maximum temperature.
Guess it is acceptable, though you might consider alarm attributes associated
with individual channels. After all, reading MAX6621_TEMP_ALERT_CAUSE returns
the alert channel, so you could associate it with the requested channel.
Next question is when to clear the alert. Currently this is done explitly by
writing into the alarm attribute. Presumably the alert will be set again if
the alert condition still applies, so it might be better to clear it
automatically after reading MAX6621_TEMP_ALERT_CAUSE. Just a thought.
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> + break;
> +
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + return 0;
> +}
> +
> +static int
> +max6621_write(struct device *dev, enum hwmon_sensor_types type, u32 attr,
> + int channel, long val)
> +{
> + struct max6621_data *data = dev_get_drvdata(dev);
> + u32 reg;
> +
> + switch (type) {
> + case hwmon_temp:
> + switch (attr) {
> + case hwmon_temp_offset:
> + /* Clamp to allowed range to prevent overflow. */
> + val = clamp_val(val, MAX6621_TEMP_INPUT_MIN,
> + MAX6621_TEMP_INPUT_MAX);
> + val = max6621_temp_mc2reg(val);
> + if (data->temp_offset != val) {
> + data->temp_offset = val;
> + return regmap_write(data->regmap,
> + MAX6621_CONFIG2_REG, val);
> + }
> +
> + return 0;
> + case hwmon_temp_crit:
> + reg = max6621_temp_alert_chan2reg[channel];
> + /* Clamp to allowed range to prevent overflow. */
> + val = clamp_val(val, MAX6621_TEMP_INPUT_MIN,
> + MAX6621_TEMP_INPUT_MAX);
> + val = val / 1000L;
> +
> + return regmap_write(data->regmap, reg, val);
> + case hwmon_temp_alarm:
> + /*
> + * Use i2c_smbus_write_byte, since
> + * MAX6621_CLEAR_ALERT_REG access expects send-byte
> + * smbus protocol for clearing alert.
> + */
> + return i2c_smbus_write_byte(data->client,
> + MAX6621_CLEAR_ALERT_REG);
> + default:
> + return -EOPNOTSUPP;
> + }
> + break;
> +
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + return -EOPNOTSUPP;
> +}
> +
> +static int
> +max6621_read_string(struct device *dev, enum hwmon_sensor_types type, u32 attr,
> + int channel, const char **str)
> +{
> + switch (type) {
> + case hwmon_temp:
> + switch (attr) {
> + case hwmon_temp_label:
> + *str = max6621_temp_labels[channel];
> + return 0;
> + default:
> + return -EOPNOTSUPP;
> + }
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + return -EOPNOTSUPP;
> +}
> +
> +static const struct regmap_config max6621_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 16,
> + .max_register = MAX6621_REG_MAX,
> + .val_format_endian = REGMAP_ENDIAN_LITTLE,
regmap lets you specify which registers are static and which registers
can change on their own. It might make sense to use that capability
to avoid unnecessary reads on configuration registers, limit registers,
and the temperature offset register.
> +};
> +
> +static u32 max6621_chip_config[] = {
> + HWMON_C_REGISTER_TZ,
> + 0
> +};
> +
> +static const struct hwmon_channel_info max6621_chip = {
> + .type = hwmon_chip,
> + .config = max6621_chip_config,
> +};
> +
> +static const u32 max6621_temp_config[] = {
> + HWMON_T_INPUT | HWMON_T_ALARM | HWMON_T_LABEL | HWMON_T_OFFSET,
> + HWMON_T_INPUT | HWMON_T_CRIT | HWMON_T_LABEL,
> + HWMON_T_INPUT | HWMON_T_CRIT | HWMON_T_LABEL,
> + HWMON_T_INPUT | HWMON_T_CRIT | HWMON_T_LABEL,
> + HWMON_T_INPUT | HWMON_T_CRIT,
> + HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_INPUT | HWMON_T_LABEL,
> + 0
> +};
> +
> +static const struct hwmon_channel_info max6621_temp = {
> + .type = hwmon_temp,
> + .config = max6621_temp_config,
> +};
> +
> +static const struct hwmon_channel_info *max6621_info[] = {
> + &max6621_chip,
> + &max6621_temp,
> + NULL
> +};
> +
> +static const struct hwmon_ops max6621_hwmon_ops = {
> + .read = max6621_read,
> + .write = max6621_write,
> + .read_string = max6621_read_string,
> + .is_visible = max6621_is_visible,
> +};
> +
> +static const struct hwmon_chip_info max6621_chip_info = {
> + .ops = &max6621_hwmon_ops,
> + .info = max6621_info,
> +};
> +
> +static int max6621_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct device *dev = &client->dev;
> + struct max6621_data *data;
> + struct device *hwmon_dev;
> + int i;
> + int ret;
> +
> + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + data->regmap = devm_regmap_init_i2c(client, &max6621_regmap_config);
> + if (IS_ERR(data->regmap))
> + return PTR_ERR(data->regmap);
> +
> + i2c_set_clientdata(client, data);
> + data->client = client;
> +
> + /* Set CONFIG0 register masking temperature alerts and PEC. */
> + ret = regmap_write(data->regmap, MAX6621_CONFIG0_REG,
> + MAX6621_CONFIG0_INIT);
> + if (ret)
> + return ret;
> +
> + /* Verify which temperature input registers are enabled. */
> + for (i = 0; i < MAX6621_TEMP_INPUT_REG_NUM; i++) {
> + ret = i2c_smbus_read_word_data(client, max6621_temp_regs[i]);
> + if (ret < 0)
> + return ret;
> + ret = max6621_verify_reg_data(dev, ret);
> + if (ret) {
> + data->input_chan2reg[i] = -1;
> + continue;
> + }
> +
> + data->input_chan2reg[i] = max6621_temp_regs[i];
> + }
> +
> + hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name,
> + data,
> + &max6621_chip_info,
> + NULL);
> +
> + return PTR_ERR_OR_ZERO(hwmon_dev);
> +}
> +
> +static const struct i2c_device_id max6621_id[] = {
> + { MAX6621_DRV_NAME, 0 },
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, max6621_id);
> +
> +static const struct of_device_id max6621_of_match[] = {
> + { .compatible = "maxim,max6621" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, max6621_of_match);
> +
> +static struct i2c_driver max6621_driver = {
> + .class = I2C_CLASS_HWMON,
> + .driver = {
> + .name = MAX6621_DRV_NAME,
> + .of_match_table = of_match_ptr(max6621_of_match),
> + },
> + .probe = max6621_probe,
> + .id_table = max6621_id,
> +};
> +
> +module_i2c_driver(max6621_driver);
> +
> +MODULE_AUTHOR("Vadim Pasternak <vadimp@mellanox.com>");
> +MODULE_DESCRIPTION("Driver for Maxim MAX6621");
> +MODULE_LICENSE("GPL");
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [v2,1/2] hwmon: Driver for Maxim MAX6621 temperature sensor
2017-10-03 14:58 ` Guenter Roeck
(?)
@ 2017-10-03 15:23 ` Vadim Pasternak
-1 siblings, 0 replies; 9+ messages in thread
From: Vadim Pasternak @ 2017-10-03 15:23 UTC (permalink / raw)
To: Guenter Roeck; +Cc: robh+dt, linux-hwmon, devicetree, jiri
> -----Original Message-----
> From: Guenter Roeck [mailto:groeck7@gmail.com] On Behalf Of Guenter
> Roeck
> Sent: Tuesday, October 03, 2017 5:59 PM
> To: Vadim Pasternak <vadimp@mellanox.com>
> Cc: robh+dt@kernel.org; linux-hwmon@vger.kernel.org;
> devicetree@vger.kernel.org; jiri@resnulli.us
> Subject: Re: [v2,1/2] hwmon: Driver for Maxim MAX6621 temperature sensor
>
> On Mon, Sep 11, 2017 at 10:45:24AM +0000, Vadim Pasternak wrote:
> > MAX6621 is a PECI-to-I2C translator provides an efficient, low-cost
> > solution for PECI-to-SMBus/I2C protocol conversion. It allows reading
> > the temperature from the PECI-compliant host directly from up to four
> > PECI-enabled CPUs.
> >
Hi Guenter,
Very sorry, it should be v4. I'll resend v4.
I also got ack from Rob for dts file, but This with suggestion to put it in
trivial-devices.txt instead.
I will resend v4 updated with Rob's comment.
Really sorry for this confusion.
Thanks,
Vadim.
>
>
> I am confused by this patch. It was sent after v3, effectively undoing that
> version. Please explain.
>
> Guenter
>
> > Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
> > ---
> > v1->v2
> > Comments pointed out by Guenter:
> > - arrange includes in alphabetic order;
> > - add include for bitops.h;
> > - remove MAX6621_REG_NON_WRITEABLE_REG;
> > - drop temp_min, temp_max, temp_reset_history attributes;
> > - remove redundant braces in max6621_verify_reg_data;
> > - fix return code in max6621_verify_reg_data;
> > - not report channels which are not physically connected;
> > - use u32 for regval in max6621_read and max6621_write;
> > - provide in temprature offset in milli-degrees C;
> > - drop hwmon_temp_fault attribute;
> > - drop activation point setting in CONFIG2 reg, leave it for user
> > space; Comments pointed out by Jiri:
> > - fixe device name;
> > Fixes added by Vadim:
> > - modify names in max6621_temp_labels;
> > - add hwmon_temp_alarm attribute and hwmon_temp_crit attributes per
> > socket;
> > - fix defines for MIN and MAX temperature;
> > ---
> > drivers/hwmon/Kconfig | 14 ++
> > drivers/hwmon/Makefile | 1 +
> > drivers/hwmon/max6621.c | 484
> > ++++++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 499 insertions(+)
> > create mode 100644 drivers/hwmon/max6621.c
> >
> > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index
> > 5b9a61f..325d161 100644
> > --- a/drivers/hwmon/Kconfig
> > +++ b/drivers/hwmon/Kconfig
> > @@ -855,6 +855,20 @@ tristate "MAX31722 temperature sensor"
> > This driver can also be built as a module. If so, the module
> > will be called max31722.
> >
> > +config SENSORS_MAX6621
> > + tristate "Maxim MAX6621 sensor chip"
> > + depends on I2C
> > + select REGMAP_I2C
> > + help
> > + If you say yes here you get support for MAX6621 sensor chip.
> > + MAX6621 is a PECI-to-I2C translator provides an efficient,
> > + low-cost solution for PECI-to-SMBus/I2C protocol conversion.
> > + It allows reading the temperature from the PECI-compliant
> > + host directly from up to four PECI-enabled CPUs.
> > +
> > + This driver can also be built as a module. If so, the module
> > + will be called max6621.
> > +
> > config SENSORS_MAX6639
> > tristate "Maxim MAX6639 sensor chip"
> > depends on I2C
> > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index
> > d4641a9..43333cb 100644
> > --- a/drivers/hwmon/Makefile
> > +++ b/drivers/hwmon/Makefile
> > @@ -116,6 +116,7 @@ obj-$(CONFIG_SENSORS_MAX1619) += max1619.o
> > obj-$(CONFIG_SENSORS_MAX1668) += max1668.o
> > obj-$(CONFIG_SENSORS_MAX197) += max197.o
> > obj-$(CONFIG_SENSORS_MAX31722) += max31722.o
> > +obj-$(CONFIG_SENSORS_MAX6621) += max6621.o
> > obj-$(CONFIG_SENSORS_MAX6639) += max6639.o
> > obj-$(CONFIG_SENSORS_MAX6642) += max6642.o
> > obj-$(CONFIG_SENSORS_MAX6650) += max6650.o
> > diff --git a/drivers/hwmon/max6621.c b/drivers/hwmon/max6621.c new
> > file mode 100644 index 0000000..2ad7fad
> > --- /dev/null
> > +++ b/drivers/hwmon/max6621.c
> > @@ -0,0 +1,484 @@
> > +/*
> > + * Hardware monitoring driver for Maxim MAX6621
> > + *
> > + * Copyright (c) 2017 Mellanox Technologies. All rights reserved.
> > + * Copyright (c) 2017 Vadim Pasternak <vadimp@mellanox.com>
> > + *
> > + * This program is free software; you can redistribute it and/or
> > +modify
> > + * it under the terms of the GNU General Public License as published
> > +by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include <linux/bitops.h>
> > +#include <linux/hwmon.h>
> > +#include <linux/hwmon-sysfs.h>
> > +#include <linux/i2c.h>
> > +#include <linux/init.h>
> > +#include <linux/of_device.h>
> > +#include <linux/module.h>
> > +#include <linux/regmap.h>
> > +
> > +#define MAX6621_DRV_NAME "max6621"
> > +#define MAX6621_TEMP_INPUT_REG_NUM 9
> > +#define MAX6621_TEMP_INPUT_MIN -20000
> > +#define MAX6621_TEMP_INPUT_MAX 120000
> > +
> > +#define MAX6621_TEMP_S0D0_REG 0x00
> > +#define MAX6621_TEMP_S0D1_REG 0x01
> > +#define MAX6621_TEMP_S1D0_REG 0x02
> > +#define MAX6621_TEMP_S1D1_REG 0x03
> > +#define MAX6621_TEMP_S2D0_REG 0x04
> > +#define MAX6621_TEMP_S2D1_REG 0x05
> > +#define MAX6621_TEMP_S3D0_REG 0x06
> > +#define MAX6621_TEMP_S3D1_REG 0x07
> > +#define MAX6621_TEMP_MAX_REG 0x08
> > +#define MAX6621_TEMP_ALERT_CAUSE 0x0b
> > +#define MAX6621_TEMP_MAX_ADDR_REG 0x0a
> > +#define MAX6621_CONFIG0_REG 0x0c
> > +#define MAX6621_CONFIG1_REG 0x0d
> > +#define MAX6621_CONFIG2_REG 0x0e
> > +#define MAX6621_CONFIG3_REG 0x0f
> > +#define MAX6621_TEMP_S0_ALERT_REG 0x10
> > +#define MAX6621_TEMP_S1_ALERT_REG 0x11
> > +#define MAX6621_TEMP_S2_ALERT_REG 0x12
> > +#define MAX6621_TEMP_S3_ALERT_REG 0x13
> > +#define MAX6621_CLEAR_ALERT_REG 0x15
> > +#define MAX6621_REG_MAX
> (MAX6621_CLEAR_ALERT_REG + 1)
> > +#define MAX6621_REG_TEMP_SHIFT 0x06
> > +
> > +#define MAX6621_ENABLE_TEMP_ALERTS_BIT 4
> > +#define MAX6621_ENABLE_I2C_CRC_BIT 5
> > +#define MAX6621_ENABLE_ALTERNATE_DATA 6
> > +#define MAX6621_ENABLE_LOCKUP_TO 7
> > +#define MAX6621_ENABLE_S0D0_BIT 8
> > +#define MAX6621_ENABLE_S0D1_BIT 9
> > +#define MAX6621_ENABLE_S1D0_BIT 10
> > +#define MAX6621_ENABLE_S1D1_BIT 11
> > +#define MAX6621_ENABLE_S2D0_BIT 12
> > +#define MAX6621_ENABLE_S2D1_BIT 13
> > +#define MAX6621_ENABLE_S3D0_BIT 14
> > +#define MAX6621_ENABLE_S3D1_BIT 15
> > +#define MAX6621_ENABLE_TEMP_ALL
> GENMASK(MAX6621_ENABLE_S3D1_BIT, \
> > + MAX6621_ENABLE_S0D0_BIT)
> > +#define MAX6621_POLL_DELAY_MASK 0x5
> > +#define MAX6621_CONFIG0_INIT
> (MAX6621_ENABLE_TEMP_ALL | \
> > + BIT(MAX6621_ENABLE_LOCKUP_TO)
> | \
> > + BIT(MAX6621_ENABLE_I2C_CRC_BIT)
> | \
> > + MAX6621_POLL_DELAY_MASK)
> > +
> > +/* Error codes */
> > +#define MAX6621_TRAN_FAILED 0x8100 /*
> > + * PECI transaction failed for more
> > + * than the configured number of
> > + * consecutive retries.
> > + */
> > +#define MAX6621_POOL_DIS 0x8101 /*
> > + * Polling disabled for requested
> > + * socket/domain.
> > + */
> > +#define MAX6621_POOL_UNCOMPLETE 0x8102 /*
> > + * First poll not yet completed for
> > + * requested socket/domain (on
> > + * startup).
> > + */
> > +#define MAX6621_SD_DIS 0x8103 /*
> > + * Read maximum temperature
> requested,
> > + * but no sockets/domains enabled
> or
> > + * all enabled sockets/domains have
> > + * errors; or read maximum
> temperature
> > + * address requested, but read
> maximum
> > + * temperature was not called.
> > + */
> > +#define MAX6621_ALERT_DIS 0x8104 /*
> > + * Get alert socket/domain
> requested,
> > + * but no alert active.
> > + */
> > +#define MAX6621_PECI_ERR_MIN 0x8000 /* Intel spec PECI error min
> value. */
> > +#define MAX6621_PECI_ERR_MAX 0x80ff /* Intel spec PECI error max
> value. */
> > +
> > +static const u32 max6621_temp_regs[] = {
> > + MAX6621_TEMP_MAX_REG, MAX6621_TEMP_S0D0_REG,
> MAX6621_TEMP_S1D0_REG,
> > + MAX6621_TEMP_S2D0_REG, MAX6621_TEMP_S3D0_REG,
> MAX6621_TEMP_S0D1_REG,
> > + MAX6621_TEMP_S1D1_REG, MAX6621_TEMP_S2D1_REG,
> MAX6621_TEMP_S3D1_REG,
> > +};
> > +
> > +static const char *const max6621_temp_labels[] = {
> > + "maximum",
> > + "socket0_0",
> > + "socket1_0",
> > + "socket2_0",
> > + "socket3_0",
> > + "socket0_1",
> > + "socket1_1",
> > + "socket2_1",
> > + "socket3_1",
> > +};
> > +
> > +static const int max6621_temp_alert_chan2reg[] = {
> > + MAX6621_CLEAR_ALERT_REG,
> > + MAX6621_TEMP_S0_ALERT_REG,
> > + MAX6621_TEMP_S1_ALERT_REG,
> > + MAX6621_TEMP_S2_ALERT_REG,
> > + MAX6621_TEMP_S3_ALERT_REG,
> > +};
> > +
> > +/**
> > + * struct max6621_data - private data:
> > + *
> > + * @client: I2C client;
> > + * @regmap: register map handle;
> > + * @temp_offset: offset that is added to all temperature return;
> > + * @input_chan2reg: mapping from channel to register; */ struct
> > +max6621_data {
> > + struct i2c_client *client;
> > + struct regmap *regmap;
> > + u16 temp_offset;
> > + int
> input_chan2reg[MAX6621_TEMP_INPUT_REG_NUM + 1];
> > +};
> > +
> > +static long max6621_temp_mc2reg(long val) {
> > + return (val / 1000L) << MAX6621_REG_TEMP_SHIFT; }
> > +
> > +static umode_t
> > +max6621_is_visible(const void *data, enum hwmon_sensor_types type,
> u32 attr,
> > + int channel)
> > +{
> > + /* Skip channels which are not physically conncted. */
> > + if (((struct max6621_data *)data)->input_chan2reg[channel] < 0)
> > + return 0;
> > +
> > + switch (type) {
> > + case hwmon_temp:
> > + switch (attr) {
> > + case hwmon_temp_input:
> > + return 0444;
> > + case hwmon_temp_label:
> > + case hwmon_temp_offset:
> > + case hwmon_temp_crit:
> > + case hwmon_temp_alarm:
> > + return 0644;
> > + default:
> > + break;
> > + }
> > +
> > + default:
> > + break;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int max6621_verify_reg_data(struct device *dev, int regval) {
> > + if (regval >= MAX6621_PECI_ERR_MIN && regval <=
> > + MAX6621_PECI_ERR_MAX) {
> > + dev_dbg(dev, "PECI error code - err 0x%04x.\n",
> > + regval);
> > +
> > + return -EINVAL;
> > + }
> > +
> > + switch (regval) {
> > + case MAX6621_TRAN_FAILED:
> > + dev_dbg(dev, "PECI transaction failed - err 0x%04x.\n",
> > + regval);
> > + break;
> > + case MAX6621_POOL_DIS:
> > + dev_dbg(dev, "Polling disabled - err 0x%04x.\n", regval);
> > + break;
> > + case MAX6621_POOL_UNCOMPLETE:
> > + dev_dbg(dev, "First poll not completed on startup - err
> 0x%04x.\n",
> > + regval);
> > + break;
> > + case MAX6621_SD_DIS:
> > + dev_dbg(dev, "Resource is disabled - err 0x%04x.\n", regval);
> > + break;
> > + case MAX6621_ALERT_DIS:
> > + dev_dbg(dev, "No alert active - err 0x%04x.\n", regval);
> > + break;
> > + default:
> > + return 0;
> > + }
> > +
> > + return -EINVAL;
> > +}
> > +
> > +static int
> > +max6621_read(struct device *dev, enum hwmon_sensor_types type, u32
> attr,
> > + int channel, long *val)
> > +{
> > + struct max6621_data *data = dev_get_drvdata(dev);
> > + u32 regval;
> > + int reg;
> > + s8 temp;
> > + int ret;
> > +
> > + switch (type) {
> > + case hwmon_temp:
> > + switch (attr) {
> > + case hwmon_temp_input:
> > + reg = data->input_chan2reg[channel];
> > + ret = regmap_read(data->regmap, reg, ®val);
> > + if (ret)
> > + return ret;
> > +
> > + ret = max6621_verify_reg_data(dev, regval);
> > + if (ret)
> > + return ret;
> > +
> > + temp = (regval >> MAX6621_REG_TEMP_SHIFT);
> > + *val = temp * 1000L;
> > +
> > + break;
> > + case hwmon_temp_offset:
> > + *val = (data->temp_offset >>
> MAX6621_REG_TEMP_SHIFT) *
> > + 1000L;
> > +
> > + break;
> > + case hwmon_temp_crit:
> > + reg = max6621_temp_alert_chan2reg[channel];
> > + ret = regmap_read(data->regmap, reg, ®val);
> > + if (ret)
> > + return ret;
> > +
> > + ret = max6621_verify_reg_data(dev, regval);
> > + if (ret)
> > + return ret;
> > +
> > + *val = regval * 1000L;
> > +
> > + break;
> > + case hwmon_temp_alarm:
> > + ret = regmap_read(data->regmap,
> > + MAX6621_TEMP_ALERT_CAUSE,
> > + ®val);
> > + if (ret)
> > + return ret;
> > +
> > + ret = max6621_verify_reg_data(dev, regval);
> > + if (ret < 0)
> > + return ret;
> > +
> > + *val = !!regval;
> > +
> > + break;
> > + default:
> > + return -EOPNOTSUPP;
> > + }
> > + break;
> > +
> > + default:
> > + return -EOPNOTSUPP;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int
> > +max6621_write(struct device *dev, enum hwmon_sensor_types type, u32
> attr,
> > + int channel, long val)
> > +{
> > + struct max6621_data *data = dev_get_drvdata(dev);
> > + u32 reg;
> > +
> > + switch (type) {
> > + case hwmon_temp:
> > + switch (attr) {
> > + case hwmon_temp_offset:
> > + /* Clamp to allowed range to prevent overflow. */
> > + val = clamp_val(val, MAX6621_TEMP_INPUT_MIN,
> > + MAX6621_TEMP_INPUT_MAX);
> > + val = max6621_temp_mc2reg(val);
> > + if (data->temp_offset != val) {
> > + data->temp_offset = val;
> > + return regmap_write(data->regmap,
> > + MAX6621_CONFIG2_REG,
> val);
> > + }
> > +
> > + return 0;
> > + case hwmon_temp_crit:
> > + reg = max6621_temp_alert_chan2reg[channel];
> > + /* Clamp to allowed range to prevent overflow. */
> > + val = clamp_val(val, MAX6621_TEMP_INPUT_MIN,
> > + MAX6621_TEMP_INPUT_MAX);
> > + val = val / 1000L;
> > +
> > + return regmap_write(data->regmap, reg, val);
> > + case hwmon_temp_alarm:
> > + /*
> > + * Use i2c_smbus_write_byte, since
> > + * MAX6621_CLEAR_ALERT_REG access expects send-
> byte
> > + * smbus protocol for clearing alert.
> > + */
> > + return i2c_smbus_write_byte(data->client,
> > +
> MAX6621_CLEAR_ALERT_REG);
> > + default:
> > + return -EOPNOTSUPP;
> > + }
> > + break;
> > +
> > + default:
> > + return -EOPNOTSUPP;
> > + }
> > +
> > + return -EOPNOTSUPP;
> > +}
> > +
> > +static int
> > +max6621_read_string(struct device *dev, enum hwmon_sensor_types
> type, u32 attr,
> > + int channel, const char **str)
> > +{
> > + switch (type) {
> > + case hwmon_temp:
> > + switch (attr) {
> > + case hwmon_temp_label:
> > + *str = max6621_temp_labels[channel];
> > + return 0;
> > + default:
> > + return -EOPNOTSUPP;
> > + }
> > + break;
> > + default:
> > + return -EOPNOTSUPP;
> > + }
> > +
> > + return -EOPNOTSUPP;
> > +}
> > +
> > +static const struct regmap_config max6621_regmap_config = {
> > + .reg_bits = 8,
> > + .val_bits = 16,
> > + .max_register = MAX6621_REG_MAX,
> > + .val_format_endian = REGMAP_ENDIAN_LITTLE, };
> > +
> > +static u32 max6621_chip_config[] = {
> > + HWMON_C_REGISTER_TZ,
> > + 0
> > +};
> > +
> > +static const struct hwmon_channel_info max6621_chip = {
> > + .type = hwmon_chip,
> > + .config = max6621_chip_config,
> > +};
> > +
> > +static const u32 max6621_temp_config[] = {
> > + HWMON_T_INPUT | HWMON_T_ALARM | HWMON_T_LABEL |
> HWMON_T_OFFSET,
> > + HWMON_T_INPUT | HWMON_T_CRIT | HWMON_T_LABEL,
> > + HWMON_T_INPUT | HWMON_T_CRIT | HWMON_T_LABEL,
> > + HWMON_T_INPUT | HWMON_T_CRIT | HWMON_T_LABEL,
> > + HWMON_T_INPUT | HWMON_T_CRIT,
> > + HWMON_T_INPUT | HWMON_T_LABEL,
> > + HWMON_T_INPUT | HWMON_T_LABEL,
> > + HWMON_T_INPUT | HWMON_T_LABEL,
> > + HWMON_T_INPUT | HWMON_T_LABEL,
> > + 0
> > +};
> > +
> > +static const struct hwmon_channel_info max6621_temp = {
> > + .type = hwmon_temp,
> > + .config = max6621_temp_config,
> > +};
> > +
> > +static const struct hwmon_channel_info *max6621_info[] = {
> > + &max6621_chip,
> > + &max6621_temp,
> > + NULL
> > +};
> > +
> > +static const struct hwmon_ops max6621_hwmon_ops = {
> > + .read = max6621_read,
> > + .write = max6621_write,
> > + .read_string = max6621_read_string,
> > + .is_visible = max6621_is_visible,
> > +};
> > +
> > +static const struct hwmon_chip_info max6621_chip_info = {
> > + .ops = &max6621_hwmon_ops,
> > + .info = max6621_info,
> > +};
> > +
> > +static int max6621_probe(struct i2c_client *client,
> > + const struct i2c_device_id *id)
> > +{
> > + struct device *dev = &client->dev;
> > + struct max6621_data *data;
> > + struct device *hwmon_dev;
> > + int i;
> > + int ret;
> > +
> > + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> > + if (!data)
> > + return -ENOMEM;
> > +
> > + data->regmap = devm_regmap_init_i2c(client,
> &max6621_regmap_config);
> > + if (IS_ERR(data->regmap))
> > + return PTR_ERR(data->regmap);
> > +
> > + i2c_set_clientdata(client, data);
> > + data->client = client;
> > +
> > + /* Set CONFIG0 register masking temperature alerts and PEC. */
> > + ret = regmap_write(data->regmap, MAX6621_CONFIG0_REG,
> > + MAX6621_CONFIG0_INIT);
> > + if (ret)
> > + return ret;
> > +
> > + /* Verify which temperature input registers are enabled. */
> > + for (i = 0; i < MAX6621_TEMP_INPUT_REG_NUM; i++) {
> > + ret = i2c_smbus_read_word_data(client,
> max6621_temp_regs[i]);
> > + if (ret < 0)
> > + return ret;
> > + ret = max6621_verify_reg_data(dev, ret);
> > + if (ret) {
> > + data->input_chan2reg[i] = -1;
> > + continue;
> > + }
> > +
> > + data->input_chan2reg[i] = max6621_temp_regs[i];
> > + }
> > +
> > + hwmon_dev = devm_hwmon_device_register_with_info(dev, client-
> >name,
> > + data,
> > +
> &max6621_chip_info,
> > + NULL);
> > +
> > + return PTR_ERR_OR_ZERO(hwmon_dev);
> > +}
> > +
> > +static const struct i2c_device_id max6621_id[] = {
> > + { MAX6621_DRV_NAME, 0 },
> > + { }
> > +};
> > +MODULE_DEVICE_TABLE(i2c, max6621_id);
> > +
> > +static const struct of_device_id max6621_of_match[] = {
> > + { .compatible = "maxim,max6621" },
> > + { }
> > +};
> > +MODULE_DEVICE_TABLE(of, max6621_of_match);
> > +
> > +static struct i2c_driver max6621_driver = {
> > + .class = I2C_CLASS_HWMON,
> > + .driver = {
> > + .name = MAX6621_DRV_NAME,
> > + .of_match_table = of_match_ptr(max6621_of_match),
> > + },
> > + .probe = max6621_probe,
> > + .id_table = max6621_id,
> > +};
> > +
> > +module_i2c_driver(max6621_driver);
> > +
> > +MODULE_AUTHOR("Vadim Pasternak <vadimp@mellanox.com>");
> > +MODULE_DESCRIPTION("Driver for Maxim MAX6621");
> > +MODULE_LICENSE("GPL");
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [v2,1/2] hwmon: Driver for Maxim MAX6621 temperature sensor
@ 2017-10-03 14:58 ` Guenter Roeck
0 siblings, 0 replies; 9+ messages in thread
From: Guenter Roeck @ 2017-10-03 14:58 UTC (permalink / raw)
To: Vadim Pasternak; +Cc: robh+dt, linux-hwmon, devicetree, jiri
On Mon, Sep 11, 2017 at 10:45:24AM +0000, Vadim Pasternak wrote:
> MAX6621 is a PECI-to-I2C translator provides an efficient, low-cost
> solution for PECI-to-SMBus/I2C protocol conversion. It allows reading the
> temperature from the PECI-compliant host directly from up to four
> PECI-enabled CPUs.
>
I am confused by this patch. It was sent after v3, effectively
undoing that version. Please explain.
Guenter
> Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
> ---
> v1->v2
> Comments pointed out by Guenter:
> - arrange includes in alphabetic order;
> - add include for bitops.h;
> - remove MAX6621_REG_NON_WRITEABLE_REG;
> - drop temp_min, temp_max, temp_reset_history attributes;
> - remove redundant braces in max6621_verify_reg_data;
> - fix return code in max6621_verify_reg_data;
> - not report channels which are not physically connected;
> - use u32 for regval in max6621_read and max6621_write;
> - provide in temprature offset in milli-degrees C;
> - drop hwmon_temp_fault attribute;
> - drop activation point setting in CONFIG2 reg, leave it for user space;
> Comments pointed out by Jiri:
> - fixe device name;
> Fixes added by Vadim:
> - modify names in max6621_temp_labels;
> - add hwmon_temp_alarm attribute and hwmon_temp_crit attributes per
> socket;
> - fix defines for MIN and MAX temperature;
> ---
> drivers/hwmon/Kconfig | 14 ++
> drivers/hwmon/Makefile | 1 +
> drivers/hwmon/max6621.c | 484 ++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 499 insertions(+)
> create mode 100644 drivers/hwmon/max6621.c
>
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 5b9a61f..325d161 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -855,6 +855,20 @@ tristate "MAX31722 temperature sensor"
> This driver can also be built as a module. If so, the module
> will be called max31722.
>
> +config SENSORS_MAX6621
> + tristate "Maxim MAX6621 sensor chip"
> + depends on I2C
> + select REGMAP_I2C
> + help
> + If you say yes here you get support for MAX6621 sensor chip.
> + MAX6621 is a PECI-to-I2C translator provides an efficient,
> + low-cost solution for PECI-to-SMBus/I2C protocol conversion.
> + It allows reading the temperature from the PECI-compliant
> + host directly from up to four PECI-enabled CPUs.
> +
> + This driver can also be built as a module. If so, the module
> + will be called max6621.
> +
> config SENSORS_MAX6639
> tristate "Maxim MAX6639 sensor chip"
> depends on I2C
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index d4641a9..43333cb 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -116,6 +116,7 @@ obj-$(CONFIG_SENSORS_MAX1619) += max1619.o
> obj-$(CONFIG_SENSORS_MAX1668) += max1668.o
> obj-$(CONFIG_SENSORS_MAX197) += max197.o
> obj-$(CONFIG_SENSORS_MAX31722) += max31722.o
> +obj-$(CONFIG_SENSORS_MAX6621) += max6621.o
> obj-$(CONFIG_SENSORS_MAX6639) += max6639.o
> obj-$(CONFIG_SENSORS_MAX6642) += max6642.o
> obj-$(CONFIG_SENSORS_MAX6650) += max6650.o
> diff --git a/drivers/hwmon/max6621.c b/drivers/hwmon/max6621.c
> new file mode 100644
> index 0000000..2ad7fad
> --- /dev/null
> +++ b/drivers/hwmon/max6621.c
> @@ -0,0 +1,484 @@
> +/*
> + * Hardware monitoring driver for Maxim MAX6621
> + *
> + * Copyright (c) 2017 Mellanox Technologies. All rights reserved.
> + * Copyright (c) 2017 Vadim Pasternak <vadimp@mellanox.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/of_device.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +
> +#define MAX6621_DRV_NAME "max6621"
> +#define MAX6621_TEMP_INPUT_REG_NUM 9
> +#define MAX6621_TEMP_INPUT_MIN -20000
> +#define MAX6621_TEMP_INPUT_MAX 120000
> +
> +#define MAX6621_TEMP_S0D0_REG 0x00
> +#define MAX6621_TEMP_S0D1_REG 0x01
> +#define MAX6621_TEMP_S1D0_REG 0x02
> +#define MAX6621_TEMP_S1D1_REG 0x03
> +#define MAX6621_TEMP_S2D0_REG 0x04
> +#define MAX6621_TEMP_S2D1_REG 0x05
> +#define MAX6621_TEMP_S3D0_REG 0x06
> +#define MAX6621_TEMP_S3D1_REG 0x07
> +#define MAX6621_TEMP_MAX_REG 0x08
> +#define MAX6621_TEMP_ALERT_CAUSE 0x0b
> +#define MAX6621_TEMP_MAX_ADDR_REG 0x0a
> +#define MAX6621_CONFIG0_REG 0x0c
> +#define MAX6621_CONFIG1_REG 0x0d
> +#define MAX6621_CONFIG2_REG 0x0e
> +#define MAX6621_CONFIG3_REG 0x0f
> +#define MAX6621_TEMP_S0_ALERT_REG 0x10
> +#define MAX6621_TEMP_S1_ALERT_REG 0x11
> +#define MAX6621_TEMP_S2_ALERT_REG 0x12
> +#define MAX6621_TEMP_S3_ALERT_REG 0x13
> +#define MAX6621_CLEAR_ALERT_REG 0x15
> +#define MAX6621_REG_MAX (MAX6621_CLEAR_ALERT_REG + 1)
> +#define MAX6621_REG_TEMP_SHIFT 0x06
> +
> +#define MAX6621_ENABLE_TEMP_ALERTS_BIT 4
> +#define MAX6621_ENABLE_I2C_CRC_BIT 5
> +#define MAX6621_ENABLE_ALTERNATE_DATA 6
> +#define MAX6621_ENABLE_LOCKUP_TO 7
> +#define MAX6621_ENABLE_S0D0_BIT 8
> +#define MAX6621_ENABLE_S0D1_BIT 9
> +#define MAX6621_ENABLE_S1D0_BIT 10
> +#define MAX6621_ENABLE_S1D1_BIT 11
> +#define MAX6621_ENABLE_S2D0_BIT 12
> +#define MAX6621_ENABLE_S2D1_BIT 13
> +#define MAX6621_ENABLE_S3D0_BIT 14
> +#define MAX6621_ENABLE_S3D1_BIT 15
> +#define MAX6621_ENABLE_TEMP_ALL GENMASK(MAX6621_ENABLE_S3D1_BIT, \
> + MAX6621_ENABLE_S0D0_BIT)
> +#define MAX6621_POLL_DELAY_MASK 0x5
> +#define MAX6621_CONFIG0_INIT (MAX6621_ENABLE_TEMP_ALL | \
> + BIT(MAX6621_ENABLE_LOCKUP_TO) | \
> + BIT(MAX6621_ENABLE_I2C_CRC_BIT) | \
> + MAX6621_POLL_DELAY_MASK)
> +
> +/* Error codes */
> +#define MAX6621_TRAN_FAILED 0x8100 /*
> + * PECI transaction failed for more
> + * than the configured number of
> + * consecutive retries.
> + */
> +#define MAX6621_POOL_DIS 0x8101 /*
> + * Polling disabled for requested
> + * socket/domain.
> + */
> +#define MAX6621_POOL_UNCOMPLETE 0x8102 /*
> + * First poll not yet completed for
> + * requested socket/domain (on
> + * startup).
> + */
> +#define MAX6621_SD_DIS 0x8103 /*
> + * Read maximum temperature requested,
> + * but no sockets/domains enabled or
> + * all enabled sockets/domains have
> + * errors; or read maximum temperature
> + * address requested, but read maximum
> + * temperature was not called.
> + */
> +#define MAX6621_ALERT_DIS 0x8104 /*
> + * Get alert socket/domain requested,
> + * but no alert active.
> + */
> +#define MAX6621_PECI_ERR_MIN 0x8000 /* Intel spec PECI error min value. */
> +#define MAX6621_PECI_ERR_MAX 0x80ff /* Intel spec PECI error max value. */
> +
> +static const u32 max6621_temp_regs[] = {
> + MAX6621_TEMP_MAX_REG, MAX6621_TEMP_S0D0_REG, MAX6621_TEMP_S1D0_REG,
> + MAX6621_TEMP_S2D0_REG, MAX6621_TEMP_S3D0_REG, MAX6621_TEMP_S0D1_REG,
> + MAX6621_TEMP_S1D1_REG, MAX6621_TEMP_S2D1_REG, MAX6621_TEMP_S3D1_REG,
> +};
> +
> +static const char *const max6621_temp_labels[] = {
> + "maximum",
> + "socket0_0",
> + "socket1_0",
> + "socket2_0",
> + "socket3_0",
> + "socket0_1",
> + "socket1_1",
> + "socket2_1",
> + "socket3_1",
> +};
> +
> +static const int max6621_temp_alert_chan2reg[] = {
> + MAX6621_CLEAR_ALERT_REG,
> + MAX6621_TEMP_S0_ALERT_REG,
> + MAX6621_TEMP_S1_ALERT_REG,
> + MAX6621_TEMP_S2_ALERT_REG,
> + MAX6621_TEMP_S3_ALERT_REG,
> +};
> +
> +/**
> + * struct max6621_data - private data:
> + *
> + * @client: I2C client;
> + * @regmap: register map handle;
> + * @temp_offset: offset that is added to all temperature return;
> + * @input_chan2reg: mapping from channel to register;
> + */
> +struct max6621_data {
> + struct i2c_client *client;
> + struct regmap *regmap;
> + u16 temp_offset;
> + int input_chan2reg[MAX6621_TEMP_INPUT_REG_NUM + 1];
> +};
> +
> +static long max6621_temp_mc2reg(long val)
> +{
> + return (val / 1000L) << MAX6621_REG_TEMP_SHIFT;
> +}
> +
> +static umode_t
> +max6621_is_visible(const void *data, enum hwmon_sensor_types type, u32 attr,
> + int channel)
> +{
> + /* Skip channels which are not physically conncted. */
> + if (((struct max6621_data *)data)->input_chan2reg[channel] < 0)
> + return 0;
> +
> + switch (type) {
> + case hwmon_temp:
> + switch (attr) {
> + case hwmon_temp_input:
> + return 0444;
> + case hwmon_temp_label:
> + case hwmon_temp_offset:
> + case hwmon_temp_crit:
> + case hwmon_temp_alarm:
> + return 0644;
> + default:
> + break;
> + }
> +
> + default:
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static int max6621_verify_reg_data(struct device *dev, int regval)
> +{
> + if (regval >= MAX6621_PECI_ERR_MIN && regval <=
> + MAX6621_PECI_ERR_MAX) {
> + dev_dbg(dev, "PECI error code - err 0x%04x.\n",
> + regval);
> +
> + return -EINVAL;
> + }
> +
> + switch (regval) {
> + case MAX6621_TRAN_FAILED:
> + dev_dbg(dev, "PECI transaction failed - err 0x%04x.\n",
> + regval);
> + break;
> + case MAX6621_POOL_DIS:
> + dev_dbg(dev, "Polling disabled - err 0x%04x.\n", regval);
> + break;
> + case MAX6621_POOL_UNCOMPLETE:
> + dev_dbg(dev, "First poll not completed on startup - err 0x%04x.\n",
> + regval);
> + break;
> + case MAX6621_SD_DIS:
> + dev_dbg(dev, "Resource is disabled - err 0x%04x.\n", regval);
> + break;
> + case MAX6621_ALERT_DIS:
> + dev_dbg(dev, "No alert active - err 0x%04x.\n", regval);
> + break;
> + default:
> + return 0;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int
> +max6621_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
> + int channel, long *val)
> +{
> + struct max6621_data *data = dev_get_drvdata(dev);
> + u32 regval;
> + int reg;
> + s8 temp;
> + int ret;
> +
> + switch (type) {
> + case hwmon_temp:
> + switch (attr) {
> + case hwmon_temp_input:
> + reg = data->input_chan2reg[channel];
> + ret = regmap_read(data->regmap, reg, ®val);
> + if (ret)
> + return ret;
> +
> + ret = max6621_verify_reg_data(dev, regval);
> + if (ret)
> + return ret;
> +
> + temp = (regval >> MAX6621_REG_TEMP_SHIFT);
> + *val = temp * 1000L;
> +
> + break;
> + case hwmon_temp_offset:
> + *val = (data->temp_offset >> MAX6621_REG_TEMP_SHIFT) *
> + 1000L;
> +
> + break;
> + case hwmon_temp_crit:
> + reg = max6621_temp_alert_chan2reg[channel];
> + ret = regmap_read(data->regmap, reg, ®val);
> + if (ret)
> + return ret;
> +
> + ret = max6621_verify_reg_data(dev, regval);
> + if (ret)
> + return ret;
> +
> + *val = regval * 1000L;
> +
> + break;
> + case hwmon_temp_alarm:
> + ret = regmap_read(data->regmap,
> + MAX6621_TEMP_ALERT_CAUSE,
> + ®val);
> + if (ret)
> + return ret;
> +
> + ret = max6621_verify_reg_data(dev, regval);
> + if (ret < 0)
> + return ret;
> +
> + *val = !!regval;
> +
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> + break;
> +
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + return 0;
> +}
> +
> +static int
> +max6621_write(struct device *dev, enum hwmon_sensor_types type, u32 attr,
> + int channel, long val)
> +{
> + struct max6621_data *data = dev_get_drvdata(dev);
> + u32 reg;
> +
> + switch (type) {
> + case hwmon_temp:
> + switch (attr) {
> + case hwmon_temp_offset:
> + /* Clamp to allowed range to prevent overflow. */
> + val = clamp_val(val, MAX6621_TEMP_INPUT_MIN,
> + MAX6621_TEMP_INPUT_MAX);
> + val = max6621_temp_mc2reg(val);
> + if (data->temp_offset != val) {
> + data->temp_offset = val;
> + return regmap_write(data->regmap,
> + MAX6621_CONFIG2_REG, val);
> + }
> +
> + return 0;
> + case hwmon_temp_crit:
> + reg = max6621_temp_alert_chan2reg[channel];
> + /* Clamp to allowed range to prevent overflow. */
> + val = clamp_val(val, MAX6621_TEMP_INPUT_MIN,
> + MAX6621_TEMP_INPUT_MAX);
> + val = val / 1000L;
> +
> + return regmap_write(data->regmap, reg, val);
> + case hwmon_temp_alarm:
> + /*
> + * Use i2c_smbus_write_byte, since
> + * MAX6621_CLEAR_ALERT_REG access expects send-byte
> + * smbus protocol for clearing alert.
> + */
> + return i2c_smbus_write_byte(data->client,
> + MAX6621_CLEAR_ALERT_REG);
> + default:
> + return -EOPNOTSUPP;
> + }
> + break;
> +
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + return -EOPNOTSUPP;
> +}
> +
> +static int
> +max6621_read_string(struct device *dev, enum hwmon_sensor_types type, u32 attr,
> + int channel, const char **str)
> +{
> + switch (type) {
> + case hwmon_temp:
> + switch (attr) {
> + case hwmon_temp_label:
> + *str = max6621_temp_labels[channel];
> + return 0;
> + default:
> + return -EOPNOTSUPP;
> + }
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + return -EOPNOTSUPP;
> +}
> +
> +static const struct regmap_config max6621_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 16,
> + .max_register = MAX6621_REG_MAX,
> + .val_format_endian = REGMAP_ENDIAN_LITTLE,
> +};
> +
> +static u32 max6621_chip_config[] = {
> + HWMON_C_REGISTER_TZ,
> + 0
> +};
> +
> +static const struct hwmon_channel_info max6621_chip = {
> + .type = hwmon_chip,
> + .config = max6621_chip_config,
> +};
> +
> +static const u32 max6621_temp_config[] = {
> + HWMON_T_INPUT | HWMON_T_ALARM | HWMON_T_LABEL | HWMON_T_OFFSET,
> + HWMON_T_INPUT | HWMON_T_CRIT | HWMON_T_LABEL,
> + HWMON_T_INPUT | HWMON_T_CRIT | HWMON_T_LABEL,
> + HWMON_T_INPUT | HWMON_T_CRIT | HWMON_T_LABEL,
> + HWMON_T_INPUT | HWMON_T_CRIT,
> + HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_INPUT | HWMON_T_LABEL,
> + 0
> +};
> +
> +static const struct hwmon_channel_info max6621_temp = {
> + .type = hwmon_temp,
> + .config = max6621_temp_config,
> +};
> +
> +static const struct hwmon_channel_info *max6621_info[] = {
> + &max6621_chip,
> + &max6621_temp,
> + NULL
> +};
> +
> +static const struct hwmon_ops max6621_hwmon_ops = {
> + .read = max6621_read,
> + .write = max6621_write,
> + .read_string = max6621_read_string,
> + .is_visible = max6621_is_visible,
> +};
> +
> +static const struct hwmon_chip_info max6621_chip_info = {
> + .ops = &max6621_hwmon_ops,
> + .info = max6621_info,
> +};
> +
> +static int max6621_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct device *dev = &client->dev;
> + struct max6621_data *data;
> + struct device *hwmon_dev;
> + int i;
> + int ret;
> +
> + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + data->regmap = devm_regmap_init_i2c(client, &max6621_regmap_config);
> + if (IS_ERR(data->regmap))
> + return PTR_ERR(data->regmap);
> +
> + i2c_set_clientdata(client, data);
> + data->client = client;
> +
> + /* Set CONFIG0 register masking temperature alerts and PEC. */
> + ret = regmap_write(data->regmap, MAX6621_CONFIG0_REG,
> + MAX6621_CONFIG0_INIT);
> + if (ret)
> + return ret;
> +
> + /* Verify which temperature input registers are enabled. */
> + for (i = 0; i < MAX6621_TEMP_INPUT_REG_NUM; i++) {
> + ret = i2c_smbus_read_word_data(client, max6621_temp_regs[i]);
> + if (ret < 0)
> + return ret;
> + ret = max6621_verify_reg_data(dev, ret);
> + if (ret) {
> + data->input_chan2reg[i] = -1;
> + continue;
> + }
> +
> + data->input_chan2reg[i] = max6621_temp_regs[i];
> + }
> +
> + hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name,
> + data,
> + &max6621_chip_info,
> + NULL);
> +
> + return PTR_ERR_OR_ZERO(hwmon_dev);
> +}
> +
> +static const struct i2c_device_id max6621_id[] = {
> + { MAX6621_DRV_NAME, 0 },
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, max6621_id);
> +
> +static const struct of_device_id max6621_of_match[] = {
> + { .compatible = "maxim,max6621" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, max6621_of_match);
> +
> +static struct i2c_driver max6621_driver = {
> + .class = I2C_CLASS_HWMON,
> + .driver = {
> + .name = MAX6621_DRV_NAME,
> + .of_match_table = of_match_ptr(max6621_of_match),
> + },
> + .probe = max6621_probe,
> + .id_table = max6621_id,
> +};
> +
> +module_i2c_driver(max6621_driver);
> +
> +MODULE_AUTHOR("Vadim Pasternak <vadimp@mellanox.com>");
> +MODULE_DESCRIPTION("Driver for Maxim MAX6621");
> +MODULE_LICENSE("GPL");
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [v2,1/2] hwmon: Driver for Maxim MAX6621 temperature sensor
@ 2017-10-03 14:58 ` Guenter Roeck
0 siblings, 0 replies; 9+ messages in thread
From: Guenter Roeck @ 2017-10-03 14:58 UTC (permalink / raw)
To: Vadim Pasternak
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
linux-hwmon-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, jiri-rHqAuBHg3fBzbRFIqnYvSA
On Mon, Sep 11, 2017 at 10:45:24AM +0000, Vadim Pasternak wrote:
> MAX6621 is a PECI-to-I2C translator provides an efficient, low-cost
> solution for PECI-to-SMBus/I2C protocol conversion. It allows reading the
> temperature from the PECI-compliant host directly from up to four
> PECI-enabled CPUs.
>
I am confused by this patch. It was sent after v3, effectively
undoing that version. Please explain.
Guenter
> Signed-off-by: Vadim Pasternak <vadimp-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> ---
> v1->v2
> Comments pointed out by Guenter:
> - arrange includes in alphabetic order;
> - add include for bitops.h;
> - remove MAX6621_REG_NON_WRITEABLE_REG;
> - drop temp_min, temp_max, temp_reset_history attributes;
> - remove redundant braces in max6621_verify_reg_data;
> - fix return code in max6621_verify_reg_data;
> - not report channels which are not physically connected;
> - use u32 for regval in max6621_read and max6621_write;
> - provide in temprature offset in milli-degrees C;
> - drop hwmon_temp_fault attribute;
> - drop activation point setting in CONFIG2 reg, leave it for user space;
> Comments pointed out by Jiri:
> - fixe device name;
> Fixes added by Vadim:
> - modify names in max6621_temp_labels;
> - add hwmon_temp_alarm attribute and hwmon_temp_crit attributes per
> socket;
> - fix defines for MIN and MAX temperature;
> ---
> drivers/hwmon/Kconfig | 14 ++
> drivers/hwmon/Makefile | 1 +
> drivers/hwmon/max6621.c | 484 ++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 499 insertions(+)
> create mode 100644 drivers/hwmon/max6621.c
>
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 5b9a61f..325d161 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -855,6 +855,20 @@ tristate "MAX31722 temperature sensor"
> This driver can also be built as a module. If so, the module
> will be called max31722.
>
> +config SENSORS_MAX6621
> + tristate "Maxim MAX6621 sensor chip"
> + depends on I2C
> + select REGMAP_I2C
> + help
> + If you say yes here you get support for MAX6621 sensor chip.
> + MAX6621 is a PECI-to-I2C translator provides an efficient,
> + low-cost solution for PECI-to-SMBus/I2C protocol conversion.
> + It allows reading the temperature from the PECI-compliant
> + host directly from up to four PECI-enabled CPUs.
> +
> + This driver can also be built as a module. If so, the module
> + will be called max6621.
> +
> config SENSORS_MAX6639
> tristate "Maxim MAX6639 sensor chip"
> depends on I2C
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index d4641a9..43333cb 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -116,6 +116,7 @@ obj-$(CONFIG_SENSORS_MAX1619) += max1619.o
> obj-$(CONFIG_SENSORS_MAX1668) += max1668.o
> obj-$(CONFIG_SENSORS_MAX197) += max197.o
> obj-$(CONFIG_SENSORS_MAX31722) += max31722.o
> +obj-$(CONFIG_SENSORS_MAX6621) += max6621.o
> obj-$(CONFIG_SENSORS_MAX6639) += max6639.o
> obj-$(CONFIG_SENSORS_MAX6642) += max6642.o
> obj-$(CONFIG_SENSORS_MAX6650) += max6650.o
> diff --git a/drivers/hwmon/max6621.c b/drivers/hwmon/max6621.c
> new file mode 100644
> index 0000000..2ad7fad
> --- /dev/null
> +++ b/drivers/hwmon/max6621.c
> @@ -0,0 +1,484 @@
> +/*
> + * Hardware monitoring driver for Maxim MAX6621
> + *
> + * Copyright (c) 2017 Mellanox Technologies. All rights reserved.
> + * Copyright (c) 2017 Vadim Pasternak <vadimp-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> + *
> + * 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.
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/of_device.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +
> +#define MAX6621_DRV_NAME "max6621"
> +#define MAX6621_TEMP_INPUT_REG_NUM 9
> +#define MAX6621_TEMP_INPUT_MIN -20000
> +#define MAX6621_TEMP_INPUT_MAX 120000
> +
> +#define MAX6621_TEMP_S0D0_REG 0x00
> +#define MAX6621_TEMP_S0D1_REG 0x01
> +#define MAX6621_TEMP_S1D0_REG 0x02
> +#define MAX6621_TEMP_S1D1_REG 0x03
> +#define MAX6621_TEMP_S2D0_REG 0x04
> +#define MAX6621_TEMP_S2D1_REG 0x05
> +#define MAX6621_TEMP_S3D0_REG 0x06
> +#define MAX6621_TEMP_S3D1_REG 0x07
> +#define MAX6621_TEMP_MAX_REG 0x08
> +#define MAX6621_TEMP_ALERT_CAUSE 0x0b
> +#define MAX6621_TEMP_MAX_ADDR_REG 0x0a
> +#define MAX6621_CONFIG0_REG 0x0c
> +#define MAX6621_CONFIG1_REG 0x0d
> +#define MAX6621_CONFIG2_REG 0x0e
> +#define MAX6621_CONFIG3_REG 0x0f
> +#define MAX6621_TEMP_S0_ALERT_REG 0x10
> +#define MAX6621_TEMP_S1_ALERT_REG 0x11
> +#define MAX6621_TEMP_S2_ALERT_REG 0x12
> +#define MAX6621_TEMP_S3_ALERT_REG 0x13
> +#define MAX6621_CLEAR_ALERT_REG 0x15
> +#define MAX6621_REG_MAX (MAX6621_CLEAR_ALERT_REG + 1)
> +#define MAX6621_REG_TEMP_SHIFT 0x06
> +
> +#define MAX6621_ENABLE_TEMP_ALERTS_BIT 4
> +#define MAX6621_ENABLE_I2C_CRC_BIT 5
> +#define MAX6621_ENABLE_ALTERNATE_DATA 6
> +#define MAX6621_ENABLE_LOCKUP_TO 7
> +#define MAX6621_ENABLE_S0D0_BIT 8
> +#define MAX6621_ENABLE_S0D1_BIT 9
> +#define MAX6621_ENABLE_S1D0_BIT 10
> +#define MAX6621_ENABLE_S1D1_BIT 11
> +#define MAX6621_ENABLE_S2D0_BIT 12
> +#define MAX6621_ENABLE_S2D1_BIT 13
> +#define MAX6621_ENABLE_S3D0_BIT 14
> +#define MAX6621_ENABLE_S3D1_BIT 15
> +#define MAX6621_ENABLE_TEMP_ALL GENMASK(MAX6621_ENABLE_S3D1_BIT, \
> + MAX6621_ENABLE_S0D0_BIT)
> +#define MAX6621_POLL_DELAY_MASK 0x5
> +#define MAX6621_CONFIG0_INIT (MAX6621_ENABLE_TEMP_ALL | \
> + BIT(MAX6621_ENABLE_LOCKUP_TO) | \
> + BIT(MAX6621_ENABLE_I2C_CRC_BIT) | \
> + MAX6621_POLL_DELAY_MASK)
> +
> +/* Error codes */
> +#define MAX6621_TRAN_FAILED 0x8100 /*
> + * PECI transaction failed for more
> + * than the configured number of
> + * consecutive retries.
> + */
> +#define MAX6621_POOL_DIS 0x8101 /*
> + * Polling disabled for requested
> + * socket/domain.
> + */
> +#define MAX6621_POOL_UNCOMPLETE 0x8102 /*
> + * First poll not yet completed for
> + * requested socket/domain (on
> + * startup).
> + */
> +#define MAX6621_SD_DIS 0x8103 /*
> + * Read maximum temperature requested,
> + * but no sockets/domains enabled or
> + * all enabled sockets/domains have
> + * errors; or read maximum temperature
> + * address requested, but read maximum
> + * temperature was not called.
> + */
> +#define MAX6621_ALERT_DIS 0x8104 /*
> + * Get alert socket/domain requested,
> + * but no alert active.
> + */
> +#define MAX6621_PECI_ERR_MIN 0x8000 /* Intel spec PECI error min value. */
> +#define MAX6621_PECI_ERR_MAX 0x80ff /* Intel spec PECI error max value. */
> +
> +static const u32 max6621_temp_regs[] = {
> + MAX6621_TEMP_MAX_REG, MAX6621_TEMP_S0D0_REG, MAX6621_TEMP_S1D0_REG,
> + MAX6621_TEMP_S2D0_REG, MAX6621_TEMP_S3D0_REG, MAX6621_TEMP_S0D1_REG,
> + MAX6621_TEMP_S1D1_REG, MAX6621_TEMP_S2D1_REG, MAX6621_TEMP_S3D1_REG,
> +};
> +
> +static const char *const max6621_temp_labels[] = {
> + "maximum",
> + "socket0_0",
> + "socket1_0",
> + "socket2_0",
> + "socket3_0",
> + "socket0_1",
> + "socket1_1",
> + "socket2_1",
> + "socket3_1",
> +};
> +
> +static const int max6621_temp_alert_chan2reg[] = {
> + MAX6621_CLEAR_ALERT_REG,
> + MAX6621_TEMP_S0_ALERT_REG,
> + MAX6621_TEMP_S1_ALERT_REG,
> + MAX6621_TEMP_S2_ALERT_REG,
> + MAX6621_TEMP_S3_ALERT_REG,
> +};
> +
> +/**
> + * struct max6621_data - private data:
> + *
> + * @client: I2C client;
> + * @regmap: register map handle;
> + * @temp_offset: offset that is added to all temperature return;
> + * @input_chan2reg: mapping from channel to register;
> + */
> +struct max6621_data {
> + struct i2c_client *client;
> + struct regmap *regmap;
> + u16 temp_offset;
> + int input_chan2reg[MAX6621_TEMP_INPUT_REG_NUM + 1];
> +};
> +
> +static long max6621_temp_mc2reg(long val)
> +{
> + return (val / 1000L) << MAX6621_REG_TEMP_SHIFT;
> +}
> +
> +static umode_t
> +max6621_is_visible(const void *data, enum hwmon_sensor_types type, u32 attr,
> + int channel)
> +{
> + /* Skip channels which are not physically conncted. */
> + if (((struct max6621_data *)data)->input_chan2reg[channel] < 0)
> + return 0;
> +
> + switch (type) {
> + case hwmon_temp:
> + switch (attr) {
> + case hwmon_temp_input:
> + return 0444;
> + case hwmon_temp_label:
> + case hwmon_temp_offset:
> + case hwmon_temp_crit:
> + case hwmon_temp_alarm:
> + return 0644;
> + default:
> + break;
> + }
> +
> + default:
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static int max6621_verify_reg_data(struct device *dev, int regval)
> +{
> + if (regval >= MAX6621_PECI_ERR_MIN && regval <=
> + MAX6621_PECI_ERR_MAX) {
> + dev_dbg(dev, "PECI error code - err 0x%04x.\n",
> + regval);
> +
> + return -EINVAL;
> + }
> +
> + switch (regval) {
> + case MAX6621_TRAN_FAILED:
> + dev_dbg(dev, "PECI transaction failed - err 0x%04x.\n",
> + regval);
> + break;
> + case MAX6621_POOL_DIS:
> + dev_dbg(dev, "Polling disabled - err 0x%04x.\n", regval);
> + break;
> + case MAX6621_POOL_UNCOMPLETE:
> + dev_dbg(dev, "First poll not completed on startup - err 0x%04x.\n",
> + regval);
> + break;
> + case MAX6621_SD_DIS:
> + dev_dbg(dev, "Resource is disabled - err 0x%04x.\n", regval);
> + break;
> + case MAX6621_ALERT_DIS:
> + dev_dbg(dev, "No alert active - err 0x%04x.\n", regval);
> + break;
> + default:
> + return 0;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int
> +max6621_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
> + int channel, long *val)
> +{
> + struct max6621_data *data = dev_get_drvdata(dev);
> + u32 regval;
> + int reg;
> + s8 temp;
> + int ret;
> +
> + switch (type) {
> + case hwmon_temp:
> + switch (attr) {
> + case hwmon_temp_input:
> + reg = data->input_chan2reg[channel];
> + ret = regmap_read(data->regmap, reg, ®val);
> + if (ret)
> + return ret;
> +
> + ret = max6621_verify_reg_data(dev, regval);
> + if (ret)
> + return ret;
> +
> + temp = (regval >> MAX6621_REG_TEMP_SHIFT);
> + *val = temp * 1000L;
> +
> + break;
> + case hwmon_temp_offset:
> + *val = (data->temp_offset >> MAX6621_REG_TEMP_SHIFT) *
> + 1000L;
> +
> + break;
> + case hwmon_temp_crit:
> + reg = max6621_temp_alert_chan2reg[channel];
> + ret = regmap_read(data->regmap, reg, ®val);
> + if (ret)
> + return ret;
> +
> + ret = max6621_verify_reg_data(dev, regval);
> + if (ret)
> + return ret;
> +
> + *val = regval * 1000L;
> +
> + break;
> + case hwmon_temp_alarm:
> + ret = regmap_read(data->regmap,
> + MAX6621_TEMP_ALERT_CAUSE,
> + ®val);
> + if (ret)
> + return ret;
> +
> + ret = max6621_verify_reg_data(dev, regval);
> + if (ret < 0)
> + return ret;
> +
> + *val = !!regval;
> +
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> + break;
> +
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + return 0;
> +}
> +
> +static int
> +max6621_write(struct device *dev, enum hwmon_sensor_types type, u32 attr,
> + int channel, long val)
> +{
> + struct max6621_data *data = dev_get_drvdata(dev);
> + u32 reg;
> +
> + switch (type) {
> + case hwmon_temp:
> + switch (attr) {
> + case hwmon_temp_offset:
> + /* Clamp to allowed range to prevent overflow. */
> + val = clamp_val(val, MAX6621_TEMP_INPUT_MIN,
> + MAX6621_TEMP_INPUT_MAX);
> + val = max6621_temp_mc2reg(val);
> + if (data->temp_offset != val) {
> + data->temp_offset = val;
> + return regmap_write(data->regmap,
> + MAX6621_CONFIG2_REG, val);
> + }
> +
> + return 0;
> + case hwmon_temp_crit:
> + reg = max6621_temp_alert_chan2reg[channel];
> + /* Clamp to allowed range to prevent overflow. */
> + val = clamp_val(val, MAX6621_TEMP_INPUT_MIN,
> + MAX6621_TEMP_INPUT_MAX);
> + val = val / 1000L;
> +
> + return regmap_write(data->regmap, reg, val);
> + case hwmon_temp_alarm:
> + /*
> + * Use i2c_smbus_write_byte, since
> + * MAX6621_CLEAR_ALERT_REG access expects send-byte
> + * smbus protocol for clearing alert.
> + */
> + return i2c_smbus_write_byte(data->client,
> + MAX6621_CLEAR_ALERT_REG);
> + default:
> + return -EOPNOTSUPP;
> + }
> + break;
> +
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + return -EOPNOTSUPP;
> +}
> +
> +static int
> +max6621_read_string(struct device *dev, enum hwmon_sensor_types type, u32 attr,
> + int channel, const char **str)
> +{
> + switch (type) {
> + case hwmon_temp:
> + switch (attr) {
> + case hwmon_temp_label:
> + *str = max6621_temp_labels[channel];
> + return 0;
> + default:
> + return -EOPNOTSUPP;
> + }
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + return -EOPNOTSUPP;
> +}
> +
> +static const struct regmap_config max6621_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 16,
> + .max_register = MAX6621_REG_MAX,
> + .val_format_endian = REGMAP_ENDIAN_LITTLE,
> +};
> +
> +static u32 max6621_chip_config[] = {
> + HWMON_C_REGISTER_TZ,
> + 0
> +};
> +
> +static const struct hwmon_channel_info max6621_chip = {
> + .type = hwmon_chip,
> + .config = max6621_chip_config,
> +};
> +
> +static const u32 max6621_temp_config[] = {
> + HWMON_T_INPUT | HWMON_T_ALARM | HWMON_T_LABEL | HWMON_T_OFFSET,
> + HWMON_T_INPUT | HWMON_T_CRIT | HWMON_T_LABEL,
> + HWMON_T_INPUT | HWMON_T_CRIT | HWMON_T_LABEL,
> + HWMON_T_INPUT | HWMON_T_CRIT | HWMON_T_LABEL,
> + HWMON_T_INPUT | HWMON_T_CRIT,
> + HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_INPUT | HWMON_T_LABEL,
> + 0
> +};
> +
> +static const struct hwmon_channel_info max6621_temp = {
> + .type = hwmon_temp,
> + .config = max6621_temp_config,
> +};
> +
> +static const struct hwmon_channel_info *max6621_info[] = {
> + &max6621_chip,
> + &max6621_temp,
> + NULL
> +};
> +
> +static const struct hwmon_ops max6621_hwmon_ops = {
> + .read = max6621_read,
> + .write = max6621_write,
> + .read_string = max6621_read_string,
> + .is_visible = max6621_is_visible,
> +};
> +
> +static const struct hwmon_chip_info max6621_chip_info = {
> + .ops = &max6621_hwmon_ops,
> + .info = max6621_info,
> +};
> +
> +static int max6621_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct device *dev = &client->dev;
> + struct max6621_data *data;
> + struct device *hwmon_dev;
> + int i;
> + int ret;
> +
> + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + data->regmap = devm_regmap_init_i2c(client, &max6621_regmap_config);
> + if (IS_ERR(data->regmap))
> + return PTR_ERR(data->regmap);
> +
> + i2c_set_clientdata(client, data);
> + data->client = client;
> +
> + /* Set CONFIG0 register masking temperature alerts and PEC. */
> + ret = regmap_write(data->regmap, MAX6621_CONFIG0_REG,
> + MAX6621_CONFIG0_INIT);
> + if (ret)
> + return ret;
> +
> + /* Verify which temperature input registers are enabled. */
> + for (i = 0; i < MAX6621_TEMP_INPUT_REG_NUM; i++) {
> + ret = i2c_smbus_read_word_data(client, max6621_temp_regs[i]);
> + if (ret < 0)
> + return ret;
> + ret = max6621_verify_reg_data(dev, ret);
> + if (ret) {
> + data->input_chan2reg[i] = -1;
> + continue;
> + }
> +
> + data->input_chan2reg[i] = max6621_temp_regs[i];
> + }
> +
> + hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name,
> + data,
> + &max6621_chip_info,
> + NULL);
> +
> + return PTR_ERR_OR_ZERO(hwmon_dev);
> +}
> +
> +static const struct i2c_device_id max6621_id[] = {
> + { MAX6621_DRV_NAME, 0 },
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, max6621_id);
> +
> +static const struct of_device_id max6621_of_match[] = {
> + { .compatible = "maxim,max6621" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, max6621_of_match);
> +
> +static struct i2c_driver max6621_driver = {
> + .class = I2C_CLASS_HWMON,
> + .driver = {
> + .name = MAX6621_DRV_NAME,
> + .of_match_table = of_match_ptr(max6621_of_match),
> + },
> + .probe = max6621_probe,
> + .id_table = max6621_id,
> +};
> +
> +module_i2c_driver(max6621_driver);
> +
> +MODULE_AUTHOR("Vadim Pasternak <vadimp-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>");
> +MODULE_DESCRIPTION("Driver for Maxim MAX6621");
> +MODULE_LICENSE("GPL");
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-10-03 15:23 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-30 21:55 [patch v2 0/2] Add support for Maxim MAX6621 temperature sensor device Vadim Pasternak
2017-08-30 21:55 ` Vadim Pasternak
2017-08-30 21:55 ` [patch v2 1/2] hwmon: Driver for Maxim MAX6621 temperature sensor Vadim Pasternak
2017-09-01 15:30 ` [v2,1/2] " Guenter Roeck
2017-08-30 21:55 ` [patch v2 2/2] dt-bindings: hwmon: add binding documentation for max6621 device Vadim Pasternak
2017-08-30 21:55 ` Vadim Pasternak
2017-09-11 10:45 [patch v2 1/2] hwmon: Driver for Maxim MAX6621 temperature sensor Vadim Pasternak
2017-10-03 14:58 ` [v2,1/2] " Guenter Roeck
2017-10-03 14:58 ` Guenter Roeck
2017-10-03 15:23 ` Vadim Pasternak
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.