Add dt-bindings for ltc2991 octal i2c voltage, current and temperature
monitor.
Signed-off-by: Antoniu Miclaus <[email protected]>
---
.../bindings/hwmon/adi,ltc2991.yaml | 114 ++++++++++++++++++
1 file changed, 114 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hwmon/adi,ltc2991.yaml
diff --git a/Documentation/devicetree/bindings/hwmon/adi,ltc2991.yaml b/Documentation/devicetree/bindings/hwmon/adi,ltc2991.yaml
new file mode 100644
index 000000000000..6174e0113ef8
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/adi,ltc2991.yaml
@@ -0,0 +1,114 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+
+$id: http://devicetree.org/schemas/hwmon/adi,ltc2991.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices LTC2991 Octal I2C Voltage, Current and Temperature Monitor
+
+maintainers:
+ - Antoniu Miclaus <[email protected]>
+
+description: |
+ The LTC2991 is used to monitor system temperatures, voltages and currents.
+ Through the I2C serial interface, the eight monitors can individually measure
+ supply voltages and can be paired for differential measurements of current
+ sense resistors or temperature sensing transistors.
+
+ Datasheet:
+ https://www.analog.com/en/products/ltc2991.html
+
+properties:
+ compatible:
+ enum:
+ - adi,ltc2991
+
+ reg:
+ maxItems: 1
+
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+ vcc-supply: true
+
+patternProperties:
+ "^channel@[0-3]$":
+ type: object
+ description: |
+ Represents the differential/temperature channels.
+
+ properties:
+ reg:
+ description: |
+ The channel number. LTC2992 can monitor 4 currents/temperatures.
+ items:
+ minimum: 0
+ maximum: 3
+
+ shunt-resistor-mili-ohms:
+ description:
+ The value of curent sense resistor in miliohms. Enables differential
+ input pair.
+
+ temperature-enable:
+ description:
+ Enables temperature readings for a input pair.
+
+required:
+ - compatible
+ - reg
+ - vcc-supply
+
+additionalProperties: false
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ hwmon@48 {
+ compatible = "adi,ltc2991";
+ reg = <0x48>;
+ vcc-supply = <&vcc>;
+ };
+ };
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ hwmon@48 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ compatible = "adi,ltc2991";
+ reg = <0x48>;
+ vcc-supply = <&vcc>;
+
+ channel@0 {
+ reg = <0x0>;
+ shunt-resistor-mili-ohms = <100>;
+ };
+
+ channel@1 {
+ reg = <0x1>;
+ shunt-resistor-mili-ohms = <100>;
+ };
+
+ channel@2 {
+ reg = <0x2>;
+ temperature-enable;
+ };
+
+ channel@3 {
+ reg = <0x3>;
+ temperature-enable;
+ };
+ };
+ };
+...
--
2.42.0
Add support for LTC2991 Octal I2C Voltage, Current, and Temperature
Monitor.
The LTC2991 is used to monitor system temperatures, voltages and
currents. Through the I 2C serial interface, theeight monitors can
individually measure supply voltages and can be paired for
differential measurements of current sense resistors or temperature
sensing transistors. Additional measurements include internal
temperature and internal VCC.
Signed-off-by: Antoniu Miclaus <[email protected]>
---
Documentation/hwmon/index.rst | 1 +
Documentation/hwmon/ltc2991.rst | 43 +++
MAINTAINERS | 8 +
drivers/hwmon/Kconfig | 11 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/ltc2991.c | 490 ++++++++++++++++++++++++++++++++
6 files changed, 554 insertions(+)
create mode 100644 Documentation/hwmon/ltc2991.rst
create mode 100644 drivers/hwmon/ltc2991.c
diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
index 88dadea85cfc..0ec96abe3f7d 100644
--- a/Documentation/hwmon/index.rst
+++ b/Documentation/hwmon/index.rst
@@ -121,6 +121,7 @@ Hardware Monitoring Kernel Drivers
ltc2947
ltc2978
ltc2990
+ ltc2991
ltc3815
ltc4151
ltc4215
diff --git a/Documentation/hwmon/ltc2991.rst b/Documentation/hwmon/ltc2991.rst
new file mode 100644
index 000000000000..9ab29dd85012
--- /dev/null
+++ b/Documentation/hwmon/ltc2991.rst
@@ -0,0 +1,43 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+Kernel driver ltc2991
+=====================
+
+Supported chips:
+
+ * Analog Devices LTC2991
+
+ Prefix: 'ltc2991'
+
+ Addresses scanned: I2C 0x48 - 0x4f
+
+ Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/2991ff.pdf
+
+Authors:
+
+ - Antoniu Miclaus <[email protected]>
+
+
+Description
+-----------
+
+This driver supports hardware monitoring for Analog Devices LTC2991 Octal I2C
+Voltage, Current and Temperature Monitor.
+
+The LTC2991 is used to monitor system temperatures, voltages and currents.
+Through the I2C serial interface, the eight monitors can individually measure
+supply voltages and can be paired for differential measurements of current sense
+resistors or temperature sensing transistors. Additional measurements include
+internal temperatureand internal VCC.
+
+
+sysfs-Interface
+-------------
+
+The following attributes are supported. Limits are read-only.
+
+=============== =================
+inX_input: voltage input
+currX_input: current input
+tempX_input: temperature input
+=============== =================
diff --git a/MAINTAINERS b/MAINTAINERS
index b19995690904..98dd8a8e1f84 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12451,6 +12451,14 @@ F: drivers/hwmon/ltc2947-i2c.c
F: drivers/hwmon/ltc2947-spi.c
F: drivers/hwmon/ltc2947.h
+LTC2991 HARDWARE MONITOR DRIVER
+M: Antoniu Miclaus <[email protected]>
+L: [email protected]
+S: Supported
+W: https://ez.analog.com/linux-software-drivers
+F: Documentation/devicetree/bindings/hwmon/adi,ltc2991.yaml
+F: drivers/hwmon/ltc2991.c
+
LTC2983 IIO TEMPERATURE DRIVER
M: Nuno Sá <[email protected]>
L: [email protected]
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index ec38c8892158..818a67328fcd 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -932,6 +932,17 @@ config SENSORS_LTC2990
This driver can also be built as a module. If so, the module will
be called ltc2990.
+config SENSORS_LTC2991
+ tristate "Analog Devices LTC2991"
+ depends on I2C
+ help
+ If you say yes here you get support for Analog Devices LTC2991
+ Octal I2C Voltage, Current, and Temperature Monitor. The LTC2991
+ supports a combination of voltage, current and temperature monitoring.
+
+ This driver can also be built as a module. If so, the module will
+ be called ltc2991.
+
config SENSORS_LTC2992
tristate "Linear Technology LTC2992"
depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 4ac9452b5430..f324d057535a 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -127,6 +127,7 @@ obj-$(CONFIG_SENSORS_LTC2947) += ltc2947-core.o
obj-$(CONFIG_SENSORS_LTC2947_I2C) += ltc2947-i2c.o
obj-$(CONFIG_SENSORS_LTC2947_SPI) += ltc2947-spi.o
obj-$(CONFIG_SENSORS_LTC2990) += ltc2990.o
+obj-$(CONFIG_SENSORS_LTC2991) += ltc2991.o
obj-$(CONFIG_SENSORS_LTC2992) += ltc2992.o
obj-$(CONFIG_SENSORS_LTC4151) += ltc4151.o
obj-$(CONFIG_SENSORS_LTC4215) += ltc4215.o
diff --git a/drivers/hwmon/ltc2991.c b/drivers/hwmon/ltc2991.c
new file mode 100644
index 000000000000..51a60ca8c24e
--- /dev/null
+++ b/drivers/hwmon/ltc2991.c
@@ -0,0 +1,490 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2023 Analog Devices, Inc.
+ * Author: Antoniu Miclaus <[email protected]>
+ */
+
+#include <linux/bitops.h>
+#include <linux/err.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+
+#define LTC2991_STATUS_LOW 0x00
+#define LTC2991_CH_EN_TRIGGER 0x01
+#define LTC2991_V1_V4_CTRL 0x06
+#define LTC2991_V5_V8_CTRL 0x07
+#define LTC2991_PWM_TH_LSB_T_INT 0x08
+#define LTC2991_PWM_TH_MSB 0x09
+#define LTC2991_CHANNEL_V_MSB(x) (0x0A + ((x) * 2))
+#define LTC2991_CHANNEL_T_MSB(x) (0x0A + ((x) * 4))
+#define LTC2991_CHANNEL_C_MSB(x) (0x0C + ((x) * 4))
+#define LTC2991_T_INT_MSB 0x1A
+#define LTC2991_VCC_MSB 0x1C
+
+#define LTC2991_V7_V8_EN BIT(7)
+#define LTC2991_V5_V6_EN BIT(6)
+#define LTC2991_V3_V4_EN BIT(5)
+#define LTC2991_V1_V2_EN BIT(4)
+#define LTC2991_T_INT_VCC_EN BIT(3)
+
+#define LTC2991_V3_V4_FILT_EN BIT(7)
+#define LTC2991_V3_V4_TEMP_EN BIT(5)
+#define LTC2991_V3_V4_DIFF_EN BIT(4)
+#define LTC2991_V1_V2_FILT_EN BIT(3)
+#define LTC2991_V1_V2_TEMP_EN BIT(1)
+#define LTC2991_V1_V2_DIFF_EN BIT(0)
+
+#define LTC2991_V7_V8_FILT_EN BIT(7)
+#define LTC2991_V7_V8_TEMP_EN BIT(5)
+#define LTC2991_V7_V8_DIFF_EN BIT(4)
+#define LTC2991_V5_V6_FILT_EN BIT(7)
+#define LTC2991_V5_V6_TEMP_EN BIT(5)
+#define LTC2991_V5_V6_DIFF_EN BIT(4)
+
+#define LTC2991_REPEAT_ACQ_EN BIT(4)
+#define LTC2991_T_INT_FILT_EN BIT(3)
+
+#define LTC2991_MAX_CHANNEL 4
+#define LTC2991_T_INT_CH_NR 4
+#define LTC2991_VCC_CH_NR 0
+
+static const char *const label_voltages[] = {
+ "vcc",
+ "voltage1",
+ "voltage2",
+ "voltage3",
+ "voltage4",
+ "voltage5",
+ "voltage6",
+ "voltage7",
+ "voltage8"
+};
+
+static const char *const label_temp[] = {
+ "t1",
+ "t2",
+ "t3",
+ "t4",
+ "t_int"
+};
+
+static const char *const label_curr[] = {
+ "v1-v2",
+ "v3-v4",
+ "v5-v6",
+ "v7-v8"
+};
+
+struct ltc2991_state {
+ struct i2c_client *client;
+ struct regmap *regmap;
+ u32 r_sense_mohm[LTC2991_MAX_CHANNEL];
+ bool temp_en[LTC2991_MAX_CHANNEL];
+};
+
+static int ltc2991_read_reg(struct ltc2991_state *st, u8 addr, u8 reg_len,
+ int *val)
+{
+ u8 regvals[2];
+ int ret;
+ int i;
+
+ ret = regmap_bulk_read(st->regmap, addr, regvals, reg_len);
+ if (ret)
+ return ret;
+
+ *val = 0;
+ for (i = 0; i < reg_len; i++)
+ *val |= regvals[reg_len - i - 1] << (i * 8);
+
+ return 0;
+}
+
+static int ltc2991_get_voltage(struct ltc2991_state *st, u32 reg, long *val)
+{
+ int reg_val, ret, offset = 0;
+
+ ret = ltc2991_read_reg(st, reg, 2, ®_val);
+ if (ret)
+ return ret;
+
+ if (reg == LTC2991_VCC_MSB)
+ /* Vcc 2.5V offset */
+ offset = 2500;
+
+ /* Vx, 305.18uV/LSB */
+ *val = DIV_ROUND_CLOSEST(sign_extend32(reg_val, 14) * 30518,
+ 1000 * 100) + offset;
+
+ return 0;
+}
+
+static int ltc2991_read_in(struct device *dev, u32 attr, int channel, long *val)
+{
+ struct ltc2991_state *st = dev_get_drvdata(dev);
+ u32 reg;
+
+ switch (attr) {
+ case hwmon_in_input:
+ if (channel == LTC2991_VCC_CH_NR)
+ reg = LTC2991_VCC_MSB;
+ else
+ reg = LTC2991_CHANNEL_V_MSB(channel - 1);
+ break;
+ default:
+ return -EOPNOTSUPP;
+ }
+
+ return ltc2991_get_voltage(st, reg, val);
+}
+
+static int ltc2991_get_curr(struct ltc2991_state *st, u32 reg, int channel,
+ long *val)
+{
+ int reg_val, ret;
+
+ ret = ltc2991_read_reg(st, reg, 2, ®_val);
+ if (ret)
+ return ret;
+
+ /* Vx-Vy, 19.075uV/LSB */
+ *val = DIV_ROUND_CLOSEST(sign_extend32(reg_val, 14) * 19075, 1000)
+ / st->r_sense_mohm[channel];
+
+ return 0;
+}
+
+static int ltc2991_read_curr(struct device *dev, u32 attr, int channel,
+ long *val)
+{
+ struct ltc2991_state *st = dev_get_drvdata(dev);
+ u32 reg;
+
+ switch (attr) {
+ case hwmon_curr_input:
+ reg = LTC2991_CHANNEL_C_MSB(channel);
+ break;
+ default:
+ return -EOPNOTSUPP;
+ }
+
+ return ltc2991_get_curr(st, reg, channel, val);
+}
+
+static int ltc2991_get_temp(struct ltc2991_state *st, u32 reg, int channel,
+ long *val)
+{
+ int reg_val, ret;
+
+ ret = ltc2991_read_reg(st, reg, 2, ®_val);
+ if (ret)
+ return ret;
+
+ /* Temp LSB = 0.0625 Degrees */
+ *val = DIV_ROUND_CLOSEST(sign_extend32(reg_val, 12) * 1000, 16);
+
+ return 0;
+}
+
+static int ltc2991_read_temp(struct device *dev, u32 attr, int channel,
+ long *val)
+{
+ struct ltc2991_state *st = dev_get_drvdata(dev);
+ u32 reg;
+
+ switch (attr) {
+ case hwmon_temp_input:
+ if (channel == LTC2991_T_INT_CH_NR)
+ reg = LTC2991_T_INT_MSB;
+ else
+ reg = LTC2991_CHANNEL_T_MSB(channel);
+ break;
+ default:
+ return -EOPNOTSUPP;
+ }
+
+ return ltc2991_get_temp(st, reg, channel, val);
+}
+
+static int ltc2991_read(struct device *dev, enum hwmon_sensor_types type, u32 attr, int channel,
+ long *val)
+{
+ switch (type) {
+ case hwmon_in:
+ return ltc2991_read_in(dev, attr, channel, val);
+ case hwmon_curr:
+ return ltc2991_read_curr(dev, attr, channel, val);
+ case hwmon_temp:
+ return ltc2991_read_temp(dev, attr, channel, val);
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+static umode_t ltc2991_is_visible(const void *data, enum hwmon_sensor_types type, u32 attr,
+ int channel)
+{
+ const struct ltc2991_state *st = data;
+
+ switch (type) {
+ case hwmon_in:
+ switch (attr) {
+ case hwmon_in_input:
+ case hwmon_in_label:
+ return 0444;
+ }
+ break;
+ case hwmon_curr:
+ switch (attr) {
+ case hwmon_curr_input:
+ case hwmon_curr_label:
+ if (st->r_sense_mohm[channel])
+ return 0444;
+ break;
+ }
+ break;
+ case hwmon_temp:
+ switch (attr) {
+ case hwmon_temp_input:
+ case hwmon_temp_label:
+ if (st->temp_en[channel] || channel == LTC2991_T_INT_CH_NR)
+ return 0444;
+ break;
+ }
+ break;
+ default:
+ break;
+ }
+
+ return 0;
+}
+
+static int ltc2991_read_string(struct device *dev, enum hwmon_sensor_types type, u32 attr,
+ int channel, const char **str)
+{
+ switch (type) {
+ case hwmon_temp:
+ *str = label_temp[channel];
+ break;
+ case hwmon_curr:
+ *str = label_curr[channel];
+ break;
+ case hwmon_in:
+ *str = label_voltages[channel];
+ break;
+ default:
+ return -EOPNOTSUPP;
+ }
+
+ return 0;
+}
+
+static const struct hwmon_ops ltc2991_hwmon_ops = {
+ .is_visible = ltc2991_is_visible,
+ .read = ltc2991_read,
+ .read_string = ltc2991_read_string,
+};
+
+static const struct hwmon_channel_info *ltc2991_info[] = {
+ HWMON_CHANNEL_INFO(temp,
+ HWMON_T_INPUT | HWMON_T_LABEL,
+ HWMON_T_INPUT | HWMON_T_LABEL,
+ HWMON_T_INPUT | HWMON_T_LABEL,
+ HWMON_T_INPUT | HWMON_T_LABEL,
+ HWMON_T_INPUT | HWMON_T_LABEL
+ ),
+ HWMON_CHANNEL_INFO(curr,
+ HWMON_C_INPUT | HWMON_C_LABEL,
+ HWMON_C_INPUT | HWMON_C_LABEL,
+ HWMON_C_INPUT | HWMON_C_LABEL,
+ HWMON_C_INPUT | HWMON_C_LABEL
+ ),
+ HWMON_CHANNEL_INFO(in,
+ HWMON_I_INPUT | HWMON_I_LABEL,
+ HWMON_I_INPUT | HWMON_I_LABEL,
+ HWMON_I_INPUT | HWMON_I_LABEL,
+ HWMON_I_INPUT | HWMON_I_LABEL,
+ HWMON_I_INPUT | HWMON_I_LABEL,
+ HWMON_I_INPUT | HWMON_I_LABEL,
+ HWMON_I_INPUT | HWMON_I_LABEL,
+ HWMON_I_INPUT | HWMON_I_LABEL,
+ HWMON_I_INPUT | HWMON_I_LABEL
+ ),
+ NULL
+};
+
+static const struct hwmon_chip_info ltc2991_chip_info = {
+ .ops = <c2991_hwmon_ops,
+ .info = ltc2991_info,
+};
+
+static const struct regmap_config ltc2991_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .max_register = 0x1D,
+};
+
+static int ltc2991_init(struct ltc2991_state *st)
+{
+ struct fwnode_handle *fwnode;
+ struct fwnode_handle *child;
+ int ret;
+ u32 val, addr;
+ u8 v5_v8_reg_data = 0, v1_v4_reg_data = 0;
+
+ ret = devm_regulator_get_enable(&st->client->dev, "vcc");
+ if (ret)
+ return dev_err_probe(&st->client->dev, ret,
+ "failed to enable regulator\n");
+
+ fwnode = dev_fwnode(&st->client->dev);
+
+ fwnode_for_each_available_child_node(fwnode, child) {
+ ret = fwnode_property_read_u32(child, "reg", &addr);
+ if (ret < 0) {
+ fwnode_handle_put(child);
+ return ret;
+ }
+
+ if (addr > 3) {
+ fwnode_handle_put(child);
+ return -EINVAL;
+ }
+
+ ret = fwnode_property_read_u32(child, "shunt-resistor-mili-ohms", &val);
+ if (!ret) {
+ st->r_sense_mohm[addr] = val;
+ switch (addr) {
+ case 0:
+ v1_v4_reg_data |= LTC2991_V1_V2_DIFF_EN;
+ break;
+ case 1:
+ v1_v4_reg_data |= LTC2991_V3_V4_DIFF_EN;
+ break;
+ case 2:
+ v5_v8_reg_data |= LTC2991_V5_V6_DIFF_EN;
+ break;
+ case 3:
+ v5_v8_reg_data |= LTC2991_V7_V8_DIFF_EN;
+ break;
+ default:
+ break;
+ }
+ }
+
+ ret = fwnode_property_read_bool(child, "temperature-enable");
+ if (ret) {
+ st->temp_en[addr] = ret;
+ switch (addr) {
+ case 0:
+ v1_v4_reg_data |= LTC2991_V1_V2_TEMP_EN;
+ break;
+ case 1:
+ v1_v4_reg_data |= LTC2991_V3_V4_TEMP_EN;
+ break;
+ case 2:
+ v5_v8_reg_data |= LTC2991_V5_V6_TEMP_EN;
+ break;
+ case 3:
+ v5_v8_reg_data |= LTC2991_V7_V8_TEMP_EN;
+ break;
+ default:
+ break;
+ }
+ }
+ }
+
+ /* Setup V5-V8 Control register */
+ ret = regmap_write(st->regmap, LTC2991_V5_V8_CTRL, v5_v8_reg_data);
+ if (ret)
+ return dev_err_probe(&st->client->dev, ret,
+ "Error: Failed to set V5-V8 CTRL reg.\n");
+
+ /* Setup V1-V4 Control register */
+ ret = regmap_write(st->regmap, LTC2991_V1_V4_CTRL, v1_v4_reg_data);
+ if (ret)
+ return dev_err_probe(&st->client->dev, ret,
+ "Error: Failed to set V1-V4 CTRL reg.\n");
+
+ /* Setup continuous mode */
+ ret = regmap_write(st->regmap, LTC2991_PWM_TH_LSB_T_INT,
+ LTC2991_REPEAT_ACQ_EN);
+ if (ret)
+ return dev_err_probe(&st->client->dev, ret,
+ "Error: Failed to set contiuous mode.\n");
+
+ /* Enable all channels and trigger conversions */
+ ret = regmap_write(st->regmap, LTC2991_CH_EN_TRIGGER,
+ LTC2991_V7_V8_EN | LTC2991_V5_V6_EN |
+ LTC2991_V3_V4_EN | LTC2991_V1_V2_EN |
+ LTC2991_T_INT_VCC_EN);
+ if (ret)
+ return dev_err_probe(&st->client->dev, ret,
+ "Error: Failed to enable conversions.\n");
+
+ return 0;
+}
+
+static int ltc2991_i2c_probe(struct i2c_client *client)
+{
+ int ret;
+ struct device *hwmon_dev;
+ struct ltc2991_state *st;
+
+ if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_WORD_DATA))
+ return -EOPNOTSUPP;
+
+ st = devm_kzalloc(&client->dev, sizeof(*st), GFP_KERNEL);
+ if (!st)
+ return -ENOMEM;
+
+ st->client = client;
+ st->regmap = devm_regmap_init_i2c(client, <c2991_regmap_config);
+ if (IS_ERR(st->regmap))
+ return PTR_ERR(st->regmap);
+
+ ret = ltc2991_init(st);
+ if (ret)
+ return ret;
+
+ hwmon_dev = devm_hwmon_device_register_with_info(&client->dev,
+ client->name, st,
+ <c2991_chip_info,
+ NULL);
+
+ return PTR_ERR_OR_ZERO(hwmon_dev);
+}
+
+static const struct of_device_id ltc2991_of_match[] = {
+ { .compatible = "adi,ltc2991" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, ltc2991_of_match);
+
+static const struct i2c_device_id ltc2991_i2c_id[] = {
+ { "ltc2991", 0 },
+ {}
+};
+MODULE_DEVICE_TABLE(i2c, ltc2991_i2c_id);
+
+static struct i2c_driver ltc2991_i2c_driver = {
+ .class = I2C_CLASS_HWMON,
+ .driver = {
+ .name = "ltc2991",
+ .of_match_table = ltc2991_of_match,
+ },
+ .probe = ltc2991_i2c_probe,
+ .id_table = ltc2991_i2c_id,
+};
+
+module_i2c_driver(ltc2991_i2c_driver);
+
+MODULE_AUTHOR("Antoniu Miclaus <[email protected]>");
+MODULE_DESCRIPTION("Analog Devices LTC2991 HWMON Driver");
+MODULE_LICENSE("GPL");
--
2.42.0
Hi,
On Tue, Sep 26, 2023 at 05:05:29PM +0300, Antoniu Miclaus wrote:
> Add dt-bindings for ltc2991 octal i2c voltage, current and temperature
> monitor.
>
> Signed-off-by: Antoniu Miclaus <[email protected]>
> ---
> .../bindings/hwmon/adi,ltc2991.yaml | 114 ++++++++++++++++++
> 1 file changed, 114 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/hwmon/adi,ltc2991.yaml
>
> diff --git a/Documentation/devicetree/bindings/hwmon/adi,ltc2991.yaml b/Documentation/devicetree/bindings/hwmon/adi,ltc2991.yaml
> new file mode 100644
> index 000000000000..6174e0113ef8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/adi,ltc2991.yaml
> @@ -0,0 +1,114 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +
> +$id: http://devicetree.org/schemas/hwmon/adi,ltc2991.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices LTC2991 Octal I2C Voltage, Current and Temperature Monitor
> +
> +maintainers:
> + - Antoniu Miclaus <[email protected]>
> +
> +description: |
> + The LTC2991 is used to monitor system temperatures, voltages and currents.
> + Through the I2C serial interface, the eight monitors can individually measure
> + supply voltages and can be paired for differential measurements of current
> + sense resistors or temperature sensing transistors.
> +
> + Datasheet:
> + https://www.analog.com/en/products/ltc2991.html
> +
> +properties:
> + compatible:
> + enum:
> + - adi,ltc2991
if you aren't expecting to add other devices that can share the binding,
make this const: rather than enum:.
> +
> + reg:
> + maxItems: 1
> +
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 0
> +
> + vcc-supply: true
> +
> +patternProperties:
> + "^channel@[0-3]$":
> + type: object
> + description: |
The |s are only needed when you have formatting to preserve.
> + Represents the differential/temperature channels.
> +
> + properties:
> + reg:
> + description: |
> + The channel number. LTC2992 can monitor 4 currents/temperatures.
> + items:
> + minimum: 0
> + maximum: 3
> +
> + shunt-resistor-mili-ohms:
The standard properties here are ohms and micro-ohms. Also, "milli" has
2 ls.
> + description:
> + The value of curent sense resistor in miliohms. Enables differential
> + input pair.
> +
> + temperature-enable:
This seems like a vendor property that should have a vendor prefix?
> + description:
> + Enables temperature readings for a input pair.
TBH, this seems like it is used just to control software behaviour.
Why would you want to actually disable this in DT?
Cheers,
Conor.
> +
> +required:
> + - compatible
> + - reg
> + - vcc-supply
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + hwmon@48 {
> + compatible = "adi,ltc2991";
> + reg = <0x48>;
> + vcc-supply = <&vcc>;
> + };
> + };
> + - |
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + hwmon@48 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + compatible = "adi,ltc2991";
> + reg = <0x48>;
> + vcc-supply = <&vcc>;
> +
> + channel@0 {
> + reg = <0x0>;
> + shunt-resistor-mili-ohms = <100>;
> + };
> +
> + channel@1 {
> + reg = <0x1>;
> + shunt-resistor-mili-ohms = <100>;
> + };
> +
> + channel@2 {
> + reg = <0x2>;
> + temperature-enable;
> + };
> +
> + channel@3 {
> + reg = <0x3>;
> + temperature-enable;
> + };
> + };
> + };
> +...
> --
> 2.42.0
>
Hi Antoniu,
some comments from me...
On Tue, 2023-09-26 at 17:05 +0300, Antoniu Miclaus wrote:
> Add support for LTC2991 Octal I2C Voltage, Current, and Temperature
> Monitor.
>
> The LTC2991 is used to monitor system temperatures, voltages and
> currents. Through the I 2C serial interface, theeight monitors can
> individually measure supply voltages and can be paired for
> differential measurements of current sense resistors or temperature
> sensing transistors. Additional measurements include internal
> temperature and internal VCC.
>
> Signed-off-by: Antoniu Miclaus <[email protected]>
> ---
> Documentation/hwmon/index.rst | 1 +
> Documentation/hwmon/ltc2991.rst | 43 +++
> MAINTAINERS | 8 +
> drivers/hwmon/Kconfig | 11 +
> drivers/hwmon/Makefile | 1 +
> drivers/hwmon/ltc2991.c | 490 ++++++++++++++++++++++++++++++++
> 6 files changed, 554 insertions(+)
> create mode 100644 Documentation/hwmon/ltc2991.rst
> create mode 100644 drivers/hwmon/ltc2991.c
>
> diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
> index 88dadea85cfc..0ec96abe3f7d 100644
> --- a/Documentation/hwmon/index.rst
> +++ b/Documentation/hwmon/index.rst
> @@ -121,6 +121,7 @@ Hardware Monitoring Kernel Drivers
> ltc2947
> ltc2978
> ltc2990
> + ltc2991
> ltc3815
> ltc4151
> ltc4215
> diff --git a/Documentation/hwmon/ltc2991.rst b/Documentation/hwmon/ltc2991.rst
> new file mode 100644
> index 000000000000..9ab29dd85012
> --- /dev/null
> +++ b/Documentation/hwmon/ltc2991.rst
> @@ -0,0 +1,43 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +Kernel driver ltc2991
> +=====================
> +
> +Supported chips:
> +
> + * Analog Devices LTC2991
> +
> + Prefix: 'ltc2991'
> +
> + Addresses scanned: I2C 0x48 - 0x4f
> +
> + Datasheet:
> https://www.analog.com/media/en/technical-documentation/data-sheets/2991ff.pdf
> +
> +Authors:
> +
> + - Antoniu Miclaus <[email protected]>
> +
> +
> +Description
> +-----------
> +
> +This driver supports hardware monitoring for Analog Devices LTC2991 Octal I2C
> +Voltage, Current and Temperature Monitor.
> +
> +The LTC2991 is used to monitor system temperatures, voltages and currents.
> +Through the I2C serial interface, the eight monitors can individually measure
> +supply voltages and can be paired for differential measurements of current sense
> +resistors or temperature sensing transistors. Additional measurements include
> +internal temperatureand internal VCC.
> +
> +
> +sysfs-Interface
> +-------------
> +
> +The following attributes are supported. Limits are read-only.
> +
> +=============== =================
> +inX_input: voltage input
> +currX_input: current input
> +tempX_input: temperature input
> +=============== =================
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b19995690904..98dd8a8e1f84 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -12451,6 +12451,14 @@ F: drivers/hwmon/ltc2947-i2c.c
> F: drivers/hwmon/ltc2947-spi.c
> F: drivers/hwmon/ltc2947.h
>
> +LTC2991 HARDWARE MONITOR DRIVER
> +M: Antoniu Miclaus <[email protected]>
> +L: [email protected]
> +S: Supported
> +W: https://ez.analog.com/linux-software-drivers
> +F: Documentation/devicetree/bindings/hwmon/adi,ltc2991.yaml
> +F: drivers/hwmon/ltc2991.c
> +
> LTC2983 IIO TEMPERATURE DRIVER
> M: Nuno Sá <[email protected]>
> L: [email protected]
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index ec38c8892158..818a67328fcd 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -932,6 +932,17 @@ config SENSORS_LTC2990
> This driver can also be built as a module. If so, the module will
> be called ltc2990.
>
> +config SENSORS_LTC2991
> + tristate "Analog Devices LTC2991"
> + depends on I2C
> + help
> + If you say yes here you get support for Analog Devices LTC2991
> + Octal I2C Voltage, Current, and Temperature Monitor. The LTC2991
> + supports a combination of voltage, current and temperature monitoring.
> +
> + This driver can also be built as a module. If so, the module will
> + be called ltc2991.
> +
> config SENSORS_LTC2992
> tristate "Linear Technology LTC2992"
> depends on I2C
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 4ac9452b5430..f324d057535a 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -127,6 +127,7 @@ obj-$(CONFIG_SENSORS_LTC2947) += ltc2947-core.o
> obj-$(CONFIG_SENSORS_LTC2947_I2C) += ltc2947-i2c.o
> obj-$(CONFIG_SENSORS_LTC2947_SPI) += ltc2947-spi.o
> obj-$(CONFIG_SENSORS_LTC2990) += ltc2990.o
> +obj-$(CONFIG_SENSORS_LTC2991) += ltc2991.o
> obj-$(CONFIG_SENSORS_LTC2992) += ltc2992.o
> obj-$(CONFIG_SENSORS_LTC4151) += ltc4151.o
> obj-$(CONFIG_SENSORS_LTC4215) += ltc4215.o
> diff --git a/drivers/hwmon/ltc2991.c b/drivers/hwmon/ltc2991.c
> new file mode 100644
> index 000000000000..51a60ca8c24e
> --- /dev/null
> +++ b/drivers/hwmon/ltc2991.c
> @@ -0,0 +1,490 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2023 Analog Devices, Inc.
> + * Author: Antoniu Miclaus <[email protected]>
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/err.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/i2c.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +
> +#define LTC2991_STATUS_LOW 0x00
> +#define LTC2991_CH_EN_TRIGGER 0x01
> +#define LTC2991_V1_V4_CTRL 0x06
> +#define LTC2991_V5_V8_CTRL 0x07
> +#define LTC2991_PWM_TH_LSB_T_INT 0x08
> +#define LTC2991_PWM_TH_MSB 0x09
> +#define LTC2991_CHANNEL_V_MSB(x) (0x0A + ((x) * 2))
> +#define LTC2991_CHANNEL_T_MSB(x) (0x0A + ((x) * 4))
> +#define LTC2991_CHANNEL_C_MSB(x) (0x0C + ((x) * 4))
> +#define LTC2991_T_INT_MSB 0x1A
> +#define LTC2991_VCC_MSB 0x1C
> +
> +#define LTC2991_V7_V8_EN BIT(7)
> +#define LTC2991_V5_V6_EN BIT(6)
> +#define LTC2991_V3_V4_EN BIT(5)
> +#define LTC2991_V1_V2_EN BIT(4)
> +#define LTC2991_T_INT_VCC_EN BIT(3)
> +
> +#define LTC2991_V3_V4_FILT_EN BIT(7)
> +#define LTC2991_V3_V4_TEMP_EN BIT(5)
> +#define LTC2991_V3_V4_DIFF_EN BIT(4)
> +#define LTC2991_V1_V2_FILT_EN BIT(3)
> +#define LTC2991_V1_V2_TEMP_EN BIT(1)
> +#define LTC2991_V1_V2_DIFF_EN BIT(0)
> +
> +#define LTC2991_V7_V8_FILT_EN BIT(7)
> +#define LTC2991_V7_V8_TEMP_EN BIT(5)
> +#define LTC2991_V7_V8_DIFF_EN BIT(4)
> +#define LTC2991_V5_V6_FILT_EN BIT(7)
> +#define LTC2991_V5_V6_TEMP_EN BIT(5)
> +#define LTC2991_V5_V6_DIFF_EN BIT(4)
> +
> +#define LTC2991_REPEAT_ACQ_EN BIT(4)
> +#define LTC2991_T_INT_FILT_EN BIT(3)
> +
> +#define LTC2991_MAX_CHANNEL 4
> +#define LTC2991_T_INT_CH_NR 4
> +#define LTC2991_VCC_CH_NR 0
> +
> +static const char *const label_voltages[] = {
> + "vcc",
> + "voltage1",
> + "voltage2",
> + "voltage3",
> + "voltage4",
> + "voltage5",
> + "voltage6",
> + "voltage7",
> + "voltage8"
> +};
> +
> +static const char *const label_temp[] = {
> + "t1",
> + "t2",
> + "t3",
> + "t4",
> + "t_int"
> +};
> +
> +static const char *const label_curr[] = {
> + "v1-v2",
> + "v3-v4",
> + "v5-v6",
> + "v7-v8"
> +};
> +
> +struct ltc2991_state {
> + struct i2c_client *client;
> + struct regmap *regmap;
> + u32 r_sense_mohm[LTC2991_MAX_CHANNEL];
> + bool temp_en[LTC2991_MAX_CHANNEL];
> +};
> +
> +static int ltc2991_read_reg(struct ltc2991_state *st, u8 addr, u8 reg_len,
> + int *val)
> +{
> + u8 regvals[2];
> + int ret;
> + int i;
> +
> + ret = regmap_bulk_read(st->regmap, addr, regvals, reg_len);
> + if (ret)
> + return ret;
> +
> + *val = 0;
> + for (i = 0; i < reg_len; i++)
> + *val |= regvals[reg_len - i - 1] << (i * 8);
Hmm this does not look good :)... You should use proper __be annotations and API...
> +
> + return 0;
> +}
> +
> +static int ltc2991_get_voltage(struct ltc2991_state *st, u32 reg, long *val)
> +{
> + int reg_val, ret, offset = 0;
> +
> + ret = ltc2991_read_reg(st, reg, 2, ®_val);
> + if (ret)
> + return ret;
> +
> + if (reg == LTC2991_VCC_MSB)
> + /* Vcc 2.5V offset */
> + offset = 2500;
> +
> + /* Vx, 305.18uV/LSB */
> + *val = DIV_ROUND_CLOSEST(sign_extend32(reg_val, 14) * 30518,
> + 1000 * 100) + offset;
> +
> + return 0;
> +}
> +
> +static int ltc2991_read_in(struct device *dev, u32 attr, int channel, long *val)
> +{
> + struct ltc2991_state *st = dev_get_drvdata(dev);
> + u32 reg;
> +
> + switch (attr) {
> + case hwmon_in_input:
> + if (channel == LTC2991_VCC_CH_NR)
> + reg = LTC2991_VCC_MSB;
> + else
> + reg = LTC2991_CHANNEL_V_MSB(channel - 1);
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + return ltc2991_get_voltage(st, reg, val);
> +}
> +
> +static int ltc2991_get_curr(struct ltc2991_state *st, u32 reg, int channel,
> + long *val)
> +{
> + int reg_val, ret;
> +
> + ret = ltc2991_read_reg(st, reg, 2, ®_val);
> + if (ret)
> + return ret;
> +
> + /* Vx-Vy, 19.075uV/LSB */
> + *val = DIV_ROUND_CLOSEST(sign_extend32(reg_val, 14) * 19075, 1000)
> + / st->r_sense_mohm[channel];
> +
> + return 0;
> +}
> +
> +static int ltc2991_read_curr(struct device *dev, u32 attr, int channel,
> + long *val)
> +{
> + struct ltc2991_state *st = dev_get_drvdata(dev);
> + u32 reg;
> +
> + switch (attr) {
> + case hwmon_curr_input:
> + reg = LTC2991_CHANNEL_C_MSB(channel);
you could just return...
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + return ltc2991_get_curr(st, reg, channel, val);
> +}
> +
> +static int ltc2991_get_temp(struct ltc2991_state *st, u32 reg, int channel,
> + long *val)
> +{
> + int reg_val, ret;
> +
> + ret = ltc2991_read_reg(st, reg, 2, ®_val);
> + if (ret)
> + return ret;
> +
> + /* Temp LSB = 0.0625 Degrees */
> + *val = DIV_ROUND_CLOSEST(sign_extend32(reg_val, 12) * 1000, 16);
> +
> + return 0;
> +}
> +
> +static int ltc2991_read_temp(struct device *dev, u32 attr, int channel,
> + long *val)
> +{
> + struct ltc2991_state *st = dev_get_drvdata(dev);
> + u32 reg;
> +
> + switch (attr) {
> + case hwmon_temp_input:
> + if (channel == LTC2991_T_INT_CH_NR)
> + reg = LTC2991_T_INT_MSB;
> + else
> + reg = LTC2991_CHANNEL_T_MSB(channel);
nit: you could just return in each of the statements...
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + return ltc2991_get_temp(st, reg, channel, val);
> +}
> +
> +static int ltc2991_read(struct device *dev, enum hwmon_sensor_types type, u32
> attr, int channel,
> + long *val)
> +{
> + switch (type) {
> + case hwmon_in:
> + return ltc2991_read_in(dev, attr, channel, val);
> + case hwmon_curr:
> + return ltc2991_read_curr(dev, attr, channel, val);
> + case hwmon_temp:
> + return ltc2991_read_temp(dev, attr, channel, val);
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static umode_t ltc2991_is_visible(const void *data, enum hwmon_sensor_types type,
> u32 attr,
> + int channel)
> +{
> + const struct ltc2991_state *st = data;
> +
> + switch (type) {
> + case hwmon_in:
> + switch (attr) {
> + case hwmon_in_input:
> + case hwmon_in_label:
> + return 0444;
> + }
> + break;
> + case hwmon_curr:
> + switch (attr) {
> + case hwmon_curr_input:
> + case hwmon_curr_label:
> + if (st->r_sense_mohm[channel])
> + return 0444;
Is this is a real usecase? I mean, not having a rsense? If not, I would just make the
property mandatory...
> + break;
> + }
> + break;
> + case hwmon_temp:
> + switch (attr) {
> + case hwmon_temp_input:
> + case hwmon_temp_label:
> + if (st->temp_en[channel] || channel == LTC2991_T_INT_CH_NR)
> + return 0444;
> + break;
> + }
> + break;
> + default:
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static int ltc2991_read_string(struct device *dev, enum hwmon_sensor_types type,
> u32 attr,
> + int channel, const char **str)
> +{
> + switch (type) {
> + case hwmon_temp:
> + *str = label_temp[channel];
> + break;
> + case hwmon_curr:
> + *str = label_curr[channel];
> + break;
> + case hwmon_in:
> + *str = label_voltages[channel];
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + return 0;
> +}
> +
> +static const struct hwmon_ops ltc2991_hwmon_ops = {
> + .is_visible = ltc2991_is_visible,
> + .read = ltc2991_read,
> + .read_string = ltc2991_read_string,
> +};
> +
> +static const struct hwmon_channel_info *ltc2991_info[] = {
> + HWMON_CHANNEL_INFO(temp,
> + HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_INPUT | HWMON_T_LABEL
> + ),
> + HWMON_CHANNEL_INFO(curr,
> + HWMON_C_INPUT | HWMON_C_LABEL,
> + HWMON_C_INPUT | HWMON_C_LABEL,
> + HWMON_C_INPUT | HWMON_C_LABEL,
> + HWMON_C_INPUT | HWMON_C_LABEL
> + ),
> + HWMON_CHANNEL_INFO(in,
> + HWMON_I_INPUT | HWMON_I_LABEL,
> + HWMON_I_INPUT | HWMON_I_LABEL,
> + HWMON_I_INPUT | HWMON_I_LABEL,
> + HWMON_I_INPUT | HWMON_I_LABEL,
> + HWMON_I_INPUT | HWMON_I_LABEL,
> + HWMON_I_INPUT | HWMON_I_LABEL,
> + HWMON_I_INPUT | HWMON_I_LABEL,
> + HWMON_I_INPUT | HWMON_I_LABEL,
> + HWMON_I_INPUT | HWMON_I_LABEL
> + ),
> + NULL
> +};
> +
> +static const struct hwmon_chip_info ltc2991_chip_info = {
> + .ops = <c2991_hwmon_ops,
> + .info = ltc2991_info,
> +};
> +
> +static const struct regmap_config ltc2991_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .max_register = 0x1D,
> +};
> +
> +static int ltc2991_init(struct ltc2991_state *st)
> +{
> + struct fwnode_handle *fwnode;
> + struct fwnode_handle *child;
> + int ret;
> + u32 val, addr;
> + u8 v5_v8_reg_data = 0, v1_v4_reg_data = 0;
> +
> + ret = devm_regulator_get_enable(&st->client->dev, "vcc");
> + if (ret)
> + return dev_err_probe(&st->client->dev, ret,
> + "failed to enable regulator\n");
> +
> + fwnode = dev_fwnode(&st->client->dev);
> +
> + fwnode_for_each_available_child_node(fwnode, child) {
device_for_each_child_node()
> + ret = fwnode_property_read_u32(child, "reg", &addr);
> + if (ret < 0) {
> + fwnode_handle_put(child);
> + return ret;
> + }
> +
> + if (addr > 3) {
> + fwnode_handle_put(child);
> + return -EINVAL;
> + }
> +
> + ret = fwnode_property_read_u32(child, "shunt-resistor-mili-ohms",
> &val);
> + if (!ret) {
> + st->r_sense_mohm[addr] = val;
> + switch (addr) {
> + case 0:
> + v1_v4_reg_data |= LTC2991_V1_V2_DIFF_EN;
> + break;
> + case 1:
> + v1_v4_reg_data |= LTC2991_V3_V4_DIFF_EN;
> + break;
> + case 2:
> + v5_v8_reg_data |= LTC2991_V5_V6_DIFF_EN;
> + break;
> + case 3:
> + v5_v8_reg_data |= LTC2991_V7_V8_DIFF_EN;
> + break;
> + default:
> + break;
> + }
> + }
> +
> + ret = fwnode_property_read_bool(child, "temperature-enable");
> + if (ret) {
> + st->temp_en[addr] = ret;
> + switch (addr) {
> + case 0:
> + v1_v4_reg_data |= LTC2991_V1_V2_TEMP_EN;
> + break;
> + case 1:
> + v1_v4_reg_data |= LTC2991_V3_V4_TEMP_EN;
> + break;
> + case 2:
> + v5_v8_reg_data |= LTC2991_V5_V6_TEMP_EN;
> + break;
> + case 3:
> + v5_v8_reg_data |= LTC2991_V7_V8_TEMP_EN;
> + break;
> + default:
> + break;
> + }
> + }
> + }
> +
> + /* Setup V5-V8 Control register */
> + ret = regmap_write(st->regmap, LTC2991_V5_V8_CTRL, v5_v8_reg_data);
> + if (ret)
> + return dev_err_probe(&st->client->dev, ret,
> + "Error: Failed to set V5-V8 CTRL reg.\n");
> +
> + /* Setup V1-V4 Control register */
> + ret = regmap_write(st->regmap, LTC2991_V1_V4_CTRL, v1_v4_reg_data);
> + if (ret)
> + return dev_err_probe(&st->client->dev, ret,
> + "Error: Failed to set V1-V4 CTRL reg.\n");
> +
> + /* Setup continuous mode */
> + ret = regmap_write(st->regmap, LTC2991_PWM_TH_LSB_T_INT,
> + LTC2991_REPEAT_ACQ_EN);
> + if (ret)
> + return dev_err_probe(&st->client->dev, ret,
> + "Error: Failed to set contiuous mode.\n");
> +
> + /* Enable all channels and trigger conversions */
> + ret = regmap_write(st->regmap, LTC2991_CH_EN_TRIGGER,
> + LTC2991_V7_V8_EN | LTC2991_V5_V6_EN |
> + LTC2991_V3_V4_EN | LTC2991_V1_V2_EN |
> + LTC2991_T_INT_VCC_EN);
Not sure on the added value of all the comments before doing a configuration...
> + if (ret)
> + return dev_err_probe(&st->client->dev, ret,
> + "Error: Failed to enable conversions.\n");
> +
I would just return regmap_write()...
> + return 0;
> +}
> +
> +static int ltc2991_i2c_probe(struct i2c_client *client)
> +{
> + int ret;
> + struct device *hwmon_dev;
> + struct ltc2991_state *st;
> +
> + if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_WORD_DATA))
> + return -EOPNOTSUPP;
> +
Is the above really needed? Isn't the device also compatible with plain i2c?
> + st = devm_kzalloc(&client->dev, sizeof(*st), GFP_KERNEL);
> + if (!st)
> + return -ENOMEM;
> +
> + st->client = client;
> + st->regmap = devm_regmap_init_i2c(client, <c2991_regmap_config);
> + if (IS_ERR(st->regmap))
> + return PTR_ERR(st->regmap);
> +
> + ret = ltc2991_init(st);
> + if (ret)
> + return ret;
> +
> + hwmon_dev = devm_hwmon_device_register_with_info(&client->dev,
> + client->name, st,
> + <c2991_chip_info,
> + NULL);
> +
> + return PTR_ERR_OR_ZERO(hwmon_dev);
> +}
> +
> +static const struct of_device_id ltc2991_of_match[] = {
> + { .compatible = "adi,ltc2991" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, ltc2991_of_match);
> +
> +static const struct i2c_device_id ltc2991_i2c_id[] = {
> + { "ltc2991", 0 },
> + {}
> +};
> +MODULE_DEVICE_TABLE(i2c, ltc2991_i2c_id);
> +
> +static struct i2c_driver ltc2991_i2c_driver = {
> + .class = I2C_CLASS_HWMON,
curious: why or when is the above needed?
- Nuno Sá
On Thu, Sep 28, 2023 at 09:37:16AM +0200, Nuno S? wrote:
> Hi Antoniu,
>
> some comments from me...
>
> On Tue, 2023-09-26 at 17:05 +0300, Antoniu Miclaus wrote:
> > Add support for LTC2991 Octal I2C Voltage, Current, and Temperature
> > Monitor.
> >
> > The LTC2991 is used to monitor system temperatures, voltages and
> > currents. Through the I 2C serial interface, theeight monitors can
> > individually measure supply voltages and can be paired for
> > differential measurements of current sense resistors or temperature
> > sensing transistors. Additional measurements include internal
> > temperature and internal VCC.
> >
> > Signed-off-by: Antoniu Miclaus <[email protected]>
> > ---
> > ?Documentation/hwmon/index.rst?? |?? 1 +
> > ?Documentation/hwmon/ltc2991.rst |? 43 +++
> > ?MAINTAINERS???????????????????? |?? 8 +
> > ?drivers/hwmon/Kconfig?????????? |? 11 +
> > ?drivers/hwmon/Makefile????????? |?? 1 +
> > ?drivers/hwmon/ltc2991.c???????? | 490 ++++++++++++++++++++++++++++++++
> > ?6 files changed, 554 insertions(+)
> > ?create mode 100644 Documentation/hwmon/ltc2991.rst
> > ?create mode 100644 drivers/hwmon/ltc2991.c
> >
> > diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
> > index 88dadea85cfc..0ec96abe3f7d 100644
> > --- a/Documentation/hwmon/index.rst
> > +++ b/Documentation/hwmon/index.rst
> > @@ -121,6 +121,7 @@ Hardware Monitoring Kernel Drivers
> > ??? ltc2947
> > ??? ltc2978
> > ??? ltc2990
> > +?? ltc2991
> > ??? ltc3815
> > ??? ltc4151
> > ??? ltc4215
> > diff --git a/Documentation/hwmon/ltc2991.rst b/Documentation/hwmon/ltc2991.rst
> > new file mode 100644
> > index 000000000000..9ab29dd85012
> > --- /dev/null
> > +++ b/Documentation/hwmon/ltc2991.rst
> > @@ -0,0 +1,43 @@
> > +.. SPDX-License-Identifier: GPL-2.0
> > +
> > +Kernel driver ltc2991
> > +=====================
> > +
> > +Supported chips:
> > +
> > +? * Analog Devices LTC2991
> > +
> > +??? Prefix: 'ltc2991'
> > +
> > +??? Addresses scanned: I2C 0x48 - 0x4f
> > +
> > +??? Datasheet:
> > https://www.analog.com/media/en/technical-documentation/data-sheets/2991ff.pdf
> > +
> > +Authors:
> > +
> > +? - Antoniu Miclaus <[email protected]>
> > +
> > +
> > +Description
> > +-----------
> > +
> > +This driver supports hardware monitoring for Analog Devices LTC2991 Octal I2C
> > +Voltage, Current and Temperature Monitor.
> > +
> > +The LTC2991 is used to monitor system temperatures, voltages and currents.
> > +Through the I2C serial interface, the eight monitors can individually measure
> > +supply voltages and can be paired for differential measurements of current sense
> > +resistors or temperature sensing transistors. Additional measurements include
> > +internal temperatureand internal VCC.
> > +
> > +
> > +sysfs-Interface
> > +-------------
> > +
> > +The following attributes are supported. Limits are read-only.
> > +
> > +=============== =================
> > +inX_input:????? voltage input
> > +currX_input:??? current input
> > +tempX_input:??? temperature input
> > +=============== =================
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index b19995690904..98dd8a8e1f84 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -12451,6 +12451,14 @@ F:?????drivers/hwmon/ltc2947-i2c.c
> > ?F:?????drivers/hwmon/ltc2947-spi.c
> > ?F:?????drivers/hwmon/ltc2947.h
> > ?
> > +LTC2991 HARDWARE MONITOR DRIVER
> > +M:?????Antoniu Miclaus <[email protected]>
> > +L:[email protected]
> > +S:?????Supported
> > +W:?????https://ez.analog.com/linux-software-drivers
> > +F:?????Documentation/devicetree/bindings/hwmon/adi,ltc2991.yaml
> > +F:?????drivers/hwmon/ltc2991.c
> > +
> > ?LTC2983 IIO TEMPERATURE DRIVER
> > ?M:?????Nuno S? <[email protected]>
> > ?L:[email protected]
> > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> > index ec38c8892158..818a67328fcd 100644
> > --- a/drivers/hwmon/Kconfig
> > +++ b/drivers/hwmon/Kconfig
> > @@ -932,6 +932,17 @@ config SENSORS_LTC2990
> > ????????? This driver can also be built as a module. If so, the module will
> > ????????? be called ltc2990.
> > ?
> > +config SENSORS_LTC2991
> > +???????tristate "Analog Devices LTC2991"
> > +???????depends on I2C
> > +???????help
> > +???????? If you say yes here you get support for Analog Devices LTC2991
> > +???????? Octal I2C Voltage, Current, and Temperature Monitor. The LTC2991
> > +???????? supports a combination of voltage, current and temperature monitoring.
> > +
> > +???????? This driver can also be built as a module. If so, the module will
> > +???????? be called ltc2991.
> > +
> > ?config SENSORS_LTC2992
> > ????????tristate "Linear Technology LTC2992"
> > ????????depends on I2C
> > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> > index 4ac9452b5430..f324d057535a 100644
> > --- a/drivers/hwmon/Makefile
> > +++ b/drivers/hwmon/Makefile
> > @@ -127,6 +127,7 @@ obj-$(CONFIG_SENSORS_LTC2947)???????+= ltc2947-core.o
> > ?obj-$(CONFIG_SENSORS_LTC2947_I2C) += ltc2947-i2c.o
> > ?obj-$(CONFIG_SENSORS_LTC2947_SPI) += ltc2947-spi.o
> > ?obj-$(CONFIG_SENSORS_LTC2990)??+= ltc2990.o
> > +obj-$(CONFIG_SENSORS_LTC2991)??+= ltc2991.o
> > ?obj-$(CONFIG_SENSORS_LTC2992)??+= ltc2992.o
> > ?obj-$(CONFIG_SENSORS_LTC4151)??+= ltc4151.o
> > ?obj-$(CONFIG_SENSORS_LTC4215)??+= ltc4215.o
> > diff --git a/drivers/hwmon/ltc2991.c b/drivers/hwmon/ltc2991.c
> > new file mode 100644
> > index 000000000000..51a60ca8c24e
> > --- /dev/null
> > +++ b/drivers/hwmon/ltc2991.c
> > @@ -0,0 +1,490 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2023 Analog Devices, Inc.
> > + * Author: Antoniu Miclaus <[email protected]>
> > + */
> > +
> > +#include <linux/bitops.h>
> > +#include <linux/err.h>
> > +#include <linux/hwmon.h>
> > +#include <linux/hwmon-sysfs.h>
> > +#include <linux/i2c.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/property.h>
> > +#include <linux/regmap.h>
> > +#include <linux/regulator/consumer.h>
> > +
> > +#define LTC2991_STATUS_LOW?????????????0x00
> > +#define LTC2991_CH_EN_TRIGGER??????????0x01
> > +#define LTC2991_V1_V4_CTRL?????????????0x06
> > +#define LTC2991_V5_V8_CTRL?????????????0x07
> > +#define LTC2991_PWM_TH_LSB_T_INT???????0x08
> > +#define LTC2991_PWM_TH_MSB?????????????0x09
> > +#define LTC2991_CHANNEL_V_MSB(x)???????(0x0A + ((x) * 2))
> > +#define LTC2991_CHANNEL_T_MSB(x)???????(0x0A + ((x) * 4))
> > +#define LTC2991_CHANNEL_C_MSB(x)???????(0x0C + ((x) * 4))
> > +#define LTC2991_T_INT_MSB??????????????0x1A
> > +#define LTC2991_VCC_MSB????????????????????????0x1C
> > +
> > +#define LTC2991_V7_V8_EN???????????????BIT(7)
> > +#define LTC2991_V5_V6_EN???????????????BIT(6)
> > +#define LTC2991_V3_V4_EN???????????????BIT(5)
> > +#define LTC2991_V1_V2_EN???????????????BIT(4)
> > +#define LTC2991_T_INT_VCC_EN???????????BIT(3)
> > +
> > +#define LTC2991_V3_V4_FILT_EN??????????BIT(7)
> > +#define LTC2991_V3_V4_TEMP_EN??????????BIT(5)
> > +#define LTC2991_V3_V4_DIFF_EN??????????BIT(4)
> > +#define LTC2991_V1_V2_FILT_EN??????????BIT(3)
> > +#define LTC2991_V1_V2_TEMP_EN??????????BIT(1)
> > +#define LTC2991_V1_V2_DIFF_EN??????????BIT(0)
> > +
> > +#define LTC2991_V7_V8_FILT_EN??????????BIT(7)
> > +#define LTC2991_V7_V8_TEMP_EN??????????BIT(5)
> > +#define LTC2991_V7_V8_DIFF_EN??????????BIT(4)
> > +#define LTC2991_V5_V6_FILT_EN??????????BIT(7)
> > +#define LTC2991_V5_V6_TEMP_EN??????????BIT(5)
> > +#define LTC2991_V5_V6_DIFF_EN??????????BIT(4)
> > +
> > +#define LTC2991_REPEAT_ACQ_EN??????????BIT(4)
> > +#define LTC2991_T_INT_FILT_EN??????????BIT(3)
> > +
> > +#define LTC2991_MAX_CHANNEL????????????4
> > +#define LTC2991_T_INT_CH_NR????????????4
> > +#define LTC2991_VCC_CH_NR??????????????0
> > +
> > +static const char *const label_voltages[] = {
> > +???????"vcc",
> > +???????"voltage1",
> > +???????"voltage2",
> > +???????"voltage3",
> > +???????"voltage4",
> > +???????"voltage5",
> > +???????"voltage6",
> > +???????"voltage7",
> > +???????"voltage8"
> > +};
> > +
> > +static const char *const label_temp[] = {
> > +???????"t1",
> > +???????"t2",
> > +???????"t3",
> > +???????"t4",
> > +???????"t_int"
> > +};
> > +
> > +static const char *const label_curr[] = {
> > +???????"v1-v2",
> > +???????"v3-v4",
> > +???????"v5-v6",
> > +???????"v7-v8"
> > +};
> > +
> > +struct ltc2991_state {
> > +???????struct i2c_client???????*client;
> > +???????struct regmap???????????*regmap;
> > +???????u32?????????????????????r_sense_mohm[LTC2991_MAX_CHANNEL];
> > +???????bool????????????????????temp_en[LTC2991_MAX_CHANNEL];
> > +};
> > +
> > +static int ltc2991_read_reg(struct ltc2991_state *st, u8 addr, u8 reg_len,
> > +?????????????????????????? int *val)
> > +{
> > +???????u8 regvals[2];
> > +???????int ret;
> > +???????int i;
> > +
> > +???????ret = regmap_bulk_read(st->regmap, addr, regvals, reg_len);
> > +???????if (ret)
> > +???????????????return ret;
> > +
> > +???????*val = 0;
> > +???????for (i = 0; i < reg_len; i++)
> > +???????????????*val |= regvals[reg_len - i - 1] << (i * 8);
>
> Hmm this does not look good :)... You should use proper __be annotations and API...
>
> > +
> > +???????return 0;
> > +}
> > +
> > +static int ltc2991_get_voltage(struct ltc2991_state *st, u32 reg, long *val)
> > +{
> > +???????int reg_val, ret, offset = 0;
> > +
> > +???????ret = ltc2991_read_reg(st, reg, 2, ®_val);
> > +???????if (ret)
> > +???????????????return ret;
> > +
> > +???????if (reg == LTC2991_VCC_MSB)
> > +???????????????/* Vcc 2.5V offset */
> > +???????????????offset = 2500;
> > +
> > +???????/* Vx, 305.18uV/LSB */
> > +???????*val = DIV_ROUND_CLOSEST(sign_extend32(reg_val, 14) * 30518,
> > +??????????????????????????????? 1000 * 100) + offset;
> > +
> > +???????return 0;
> > +}
> > +
> > +static int ltc2991_read_in(struct device *dev, u32 attr, int channel, long *val)
> > +{
> > +???????struct ltc2991_state *st = dev_get_drvdata(dev);
> > +???????u32 reg;
> > +
> > +???????switch (attr) {
> > +???????case hwmon_in_input:
> > +???????????????if (channel == LTC2991_VCC_CH_NR)
> > +???????????????????????reg = LTC2991_VCC_MSB;
> > +???????????????else
> > +???????????????????????reg = LTC2991_CHANNEL_V_MSB(channel - 1);
> > +???????????????break;
> > +???????default:
> > +???????????????return -EOPNOTSUPP;
> > +???????}
> > +
> > +???????return ltc2991_get_voltage(st, reg, val);
> > +}
> > +
> > +static int ltc2991_get_curr(struct ltc2991_state *st, u32 reg, int channel,
> > +?????????????????????????? long *val)
> > +{
> > +???????int reg_val, ret;
> > +
> > +???????ret = ltc2991_read_reg(st, reg, 2, ®_val);
> > +???????if (ret)
> > +???????????????return ret;
> > +
> > +???????/* Vx-Vy, 19.075uV/LSB */
> > +???????*val = DIV_ROUND_CLOSEST(sign_extend32(reg_val, 14) * 19075, 1000)
> > +??????????????????????????????? / st->r_sense_mohm[channel];
> > +
> > +???????return 0;
> > +}
> > +
> > +static int ltc2991_read_curr(struct device *dev, u32 attr, int channel,
> > +??????????????????????????? long *val)
> > +{
> > +???????struct ltc2991_state *st = dev_get_drvdata(dev);
> > +???????u32 reg;
> > +
> > +???????switch (attr) {
> > +???????case hwmon_curr_input:
> > +???????????????reg = LTC2991_CHANNEL_C_MSB(channel);
> you could just return...
> > +???????????????break;
> > +???????default:
> > +???????????????return -EOPNOTSUPP;
> > +???????}
> > +
> > +???????return ltc2991_get_curr(st, reg, channel, val);
> > +}
> > +
> > +static int ltc2991_get_temp(struct ltc2991_state *st, u32 reg, int channel,
> > +?????????????????????????? long *val)
> > +{
> > +???????int reg_val, ret;
> > +
> > +???????ret = ltc2991_read_reg(st, reg, 2, ®_val);
> > +???????if (ret)
> > +???????????????return ret;
> > +
> > +???????/* Temp LSB = 0.0625 Degrees */
> > +???????*val = DIV_ROUND_CLOSEST(sign_extend32(reg_val, 12) * 1000, 16);
> > +
> > +???????return 0;
> > +}
> > +
> > +static int ltc2991_read_temp(struct device *dev, u32 attr, int channel,
> > +??????????????????????????? long *val)
> > +{
> > +???????struct ltc2991_state *st = dev_get_drvdata(dev);
> > +???????u32 reg;
> > +
> > +???????switch (attr) {
> > +???????case hwmon_temp_input:
> > +???????????????if (channel == LTC2991_T_INT_CH_NR)
> > +???????????????????????reg = LTC2991_T_INT_MSB;
> > +???????????????else
> > +???????????????????????reg = LTC2991_CHANNEL_T_MSB(channel);
>
> nit: you could just return in each of the statements...
>
> > +???????????????break;
> > +???????default:
> > +???????????????return -EOPNOTSUPP;
> > +???????}
> > +
> > +???????return ltc2991_get_temp(st, reg, channel, val);
> > +}
> > +
> > +static int ltc2991_read(struct device *dev, enum hwmon_sensor_types type, u32
> > attr, int channel,
> > +???????????????????????long *val)
> > +{
> > +???????switch (type) {
> > +???????case hwmon_in:
> > +???????????????return ltc2991_read_in(dev, attr, channel, val);
> > +???????case hwmon_curr:
> > +???????????????return ltc2991_read_curr(dev, attr, channel, val);
> > +???????case hwmon_temp:
> > +???????????????return ltc2991_read_temp(dev, attr, channel, val);
> > +???????default:
> > +???????????????return -EOPNOTSUPP;
> > +???????}
> > +}
> > +
> > +static umode_t ltc2991_is_visible(const void *data, enum hwmon_sensor_types type,
> > u32 attr,
> > +???????????????????????????????? int channel)
> > +{
> > +???????const struct ltc2991_state *st = data;
> > +
> > +???????switch (type) {
> > +???????case hwmon_in:
> > +???????????????switch (attr) {
> > +???????????????case hwmon_in_input:
> > +???????????????case hwmon_in_label:
> > +???????????????????????return 0444;
> > +???????????????}
> > +???????????????break;
> > +???????case hwmon_curr:
> > +???????????????switch (attr) {
> > +???????????????case hwmon_curr_input:
> > +???????????????case hwmon_curr_label:
> > +???????????????????????if (st->r_sense_mohm[channel])
> > +???????????????????????????????return 0444;
>
> Is this is a real usecase? I mean, not having a rsense? If not, I would just make the
> property mandatory...
>
It doesn't make sense. There should be a default if the property
isn't there, and a property value of 0 should not be accepted.
However, I think the chip only monitors current or voltage or temperature
on each channel, so something is wrong anyway. Figure 1 in the datasheet
shows this pretty well: It consumes all input pins and has
one current sensor, three voltage sensors, two external temperature sensors,
and the internal temperature sensor. That makes a total of 7 sensors,
not all the sensors enabled here. Essentially,
- If voltages are differential, only every other voltage is enabled
- If temperature sensors are enabled, the corresponding output pins
do not monitor voltage or current
- current channels are only enabled if differential voltage measurement
is enabled (or, in other words, enabling a current sensor disables
the second associated voltage sensor)
This must all be reflected here, and the existence of a current sense
resistor should not be the deciding factor to determine if current
sensing is enabled on a set of input pins. That should be explicit.
Per datasheet, chip default is that all channels are configured to
report voltages. Either that or, much better, the current chip configuration
should be the default if there is no devicetree data (because that enables
systems where defaults are set by the rom monitor or bios, and it enables
module test code).
Guenter
On Tue, Sep 26, 2023 at 05:05:30PM +0300, Antoniu Miclaus wrote:
> Add support for LTC2991 Octal I2C Voltage, Current, and Temperature
> Monitor.
>
> The LTC2991 is used to monitor system temperatures, voltages and
> currents. Through the I 2C serial interface, theeight monitors can
> individually measure supply voltages and can be paired for
> differential measurements of current sense resistors or temperature
> sensing transistors. Additional measurements include internal
> temperature and internal VCC.
>
> Signed-off-by: Antoniu Miclaus <[email protected]>
> ---
> Documentation/hwmon/index.rst | 1 +
> Documentation/hwmon/ltc2991.rst | 43 +++
> MAINTAINERS | 8 +
> drivers/hwmon/Kconfig | 11 +
> drivers/hwmon/Makefile | 1 +
> drivers/hwmon/ltc2991.c | 490 ++++++++++++++++++++++++++++++++
> 6 files changed, 554 insertions(+)
> create mode 100644 Documentation/hwmon/ltc2991.rst
> create mode 100644 drivers/hwmon/ltc2991.c
>
> diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
> index 88dadea85cfc..0ec96abe3f7d 100644
> --- a/Documentation/hwmon/index.rst
> +++ b/Documentation/hwmon/index.rst
> @@ -121,6 +121,7 @@ Hardware Monitoring Kernel Drivers
> ltc2947
> ltc2978
> ltc2990
> + ltc2991
> ltc3815
> ltc4151
> ltc4215
> diff --git a/Documentation/hwmon/ltc2991.rst b/Documentation/hwmon/ltc2991.rst
> new file mode 100644
> index 000000000000..9ab29dd85012
> --- /dev/null
> +++ b/Documentation/hwmon/ltc2991.rst
> @@ -0,0 +1,43 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +Kernel driver ltc2991
> +=====================
> +
> +Supported chips:
> +
> + * Analog Devices LTC2991
> +
> + Prefix: 'ltc2991'
> +
> + Addresses scanned: I2C 0x48 - 0x4f
> +
> + Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/2991ff.pdf
> +
> +Authors:
> +
> + - Antoniu Miclaus <[email protected]>
> +
> +
> +Description
> +-----------
> +
> +This driver supports hardware monitoring for Analog Devices LTC2991 Octal I2C
> +Voltage, Current and Temperature Monitor.
> +
> +The LTC2991 is used to monitor system temperatures, voltages and currents.
> +Through the I2C serial interface, the eight monitors can individually measure
> +supply voltages and can be paired for differential measurements of current sense
> +resistors or temperature sensing transistors. Additional measurements include
> +internal temperatureand internal VCC.
> +
> +
> +sysfs-Interface
> +-------------
> +
> +The following attributes are supported. Limits are read-only.
> +
> +=============== =================
> +inX_input: voltage input
> +currX_input: current input
> +tempX_input: temperature input
> +=============== =================
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b19995690904..98dd8a8e1f84 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -12451,6 +12451,14 @@ F: drivers/hwmon/ltc2947-i2c.c
> F: drivers/hwmon/ltc2947-spi.c
> F: drivers/hwmon/ltc2947.h
>
> +LTC2991 HARDWARE MONITOR DRIVER
> +M: Antoniu Miclaus <[email protected]>
> +L: [email protected]
> +S: Supported
> +W: https://ez.analog.com/linux-software-drivers
> +F: Documentation/devicetree/bindings/hwmon/adi,ltc2991.yaml
> +F: drivers/hwmon/ltc2991.c
> +
> LTC2983 IIO TEMPERATURE DRIVER
> M: Nuno S? <[email protected]>
> L: [email protected]
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index ec38c8892158..818a67328fcd 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -932,6 +932,17 @@ config SENSORS_LTC2990
> This driver can also be built as a module. If so, the module will
> be called ltc2990.
>
> +config SENSORS_LTC2991
> + tristate "Analog Devices LTC2991"
> + depends on I2C
> + help
> + If you say yes here you get support for Analog Devices LTC2991
> + Octal I2C Voltage, Current, and Temperature Monitor. The LTC2991
> + supports a combination of voltage, current and temperature monitoring.
> +
> + This driver can also be built as a module. If so, the module will
> + be called ltc2991.
> +
> config SENSORS_LTC2992
> tristate "Linear Technology LTC2992"
> depends on I2C
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 4ac9452b5430..f324d057535a 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -127,6 +127,7 @@ obj-$(CONFIG_SENSORS_LTC2947) += ltc2947-core.o
> obj-$(CONFIG_SENSORS_LTC2947_I2C) += ltc2947-i2c.o
> obj-$(CONFIG_SENSORS_LTC2947_SPI) += ltc2947-spi.o
> obj-$(CONFIG_SENSORS_LTC2990) += ltc2990.o
> +obj-$(CONFIG_SENSORS_LTC2991) += ltc2991.o
> obj-$(CONFIG_SENSORS_LTC2992) += ltc2992.o
> obj-$(CONFIG_SENSORS_LTC4151) += ltc4151.o
> obj-$(CONFIG_SENSORS_LTC4215) += ltc4215.o
> diff --git a/drivers/hwmon/ltc2991.c b/drivers/hwmon/ltc2991.c
> new file mode 100644
> index 000000000000..51a60ca8c24e
> --- /dev/null
> +++ b/drivers/hwmon/ltc2991.c
> @@ -0,0 +1,490 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2023 Analog Devices, Inc.
> + * Author: Antoniu Miclaus <[email protected]>
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/err.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/i2c.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +
> +#define LTC2991_STATUS_LOW 0x00
> +#define LTC2991_CH_EN_TRIGGER 0x01
> +#define LTC2991_V1_V4_CTRL 0x06
> +#define LTC2991_V5_V8_CTRL 0x07
> +#define LTC2991_PWM_TH_LSB_T_INT 0x08
> +#define LTC2991_PWM_TH_MSB 0x09
> +#define LTC2991_CHANNEL_V_MSB(x) (0x0A + ((x) * 2))
> +#define LTC2991_CHANNEL_T_MSB(x) (0x0A + ((x) * 4))
> +#define LTC2991_CHANNEL_C_MSB(x) (0x0C + ((x) * 4))
> +#define LTC2991_T_INT_MSB 0x1A
> +#define LTC2991_VCC_MSB 0x1C
> +
> +#define LTC2991_V7_V8_EN BIT(7)
> +#define LTC2991_V5_V6_EN BIT(6)
> +#define LTC2991_V3_V4_EN BIT(5)
> +#define LTC2991_V1_V2_EN BIT(4)
> +#define LTC2991_T_INT_VCC_EN BIT(3)
> +
> +#define LTC2991_V3_V4_FILT_EN BIT(7)
> +#define LTC2991_V3_V4_TEMP_EN BIT(5)
> +#define LTC2991_V3_V4_DIFF_EN BIT(4)
> +#define LTC2991_V1_V2_FILT_EN BIT(3)
> +#define LTC2991_V1_V2_TEMP_EN BIT(1)
> +#define LTC2991_V1_V2_DIFF_EN BIT(0)
> +
> +#define LTC2991_V7_V8_FILT_EN BIT(7)
> +#define LTC2991_V7_V8_TEMP_EN BIT(5)
> +#define LTC2991_V7_V8_DIFF_EN BIT(4)
> +#define LTC2991_V5_V6_FILT_EN BIT(7)
> +#define LTC2991_V5_V6_TEMP_EN BIT(5)
> +#define LTC2991_V5_V6_DIFF_EN BIT(4)
> +
> +#define LTC2991_REPEAT_ACQ_EN BIT(4)
> +#define LTC2991_T_INT_FILT_EN BIT(3)
> +
> +#define LTC2991_MAX_CHANNEL 4
> +#define LTC2991_T_INT_CH_NR 4
> +#define LTC2991_VCC_CH_NR 0
> +
> +static const char *const label_voltages[] = {
> + "vcc",
> + "voltage1",
> + "voltage2",
> + "voltage3",
> + "voltage4",
> + "voltage5",
> + "voltage6",
> + "voltage7",
> + "voltage8"
> +};
> +
> +static const char *const label_temp[] = {
> + "t1",
> + "t2",
> + "t3",
> + "t4",
> + "t_int"
> +};
> +
> +static const char *const label_curr[] = {
> + "v1-v2",
> + "v3-v4",
> + "v5-v6",
> + "v7-v8"
Those labels are all but pointless. Please drop.
> +};
> +
> +struct ltc2991_state {
> + struct i2c_client *client;
> + struct regmap *regmap;
> + u32 r_sense_mohm[LTC2991_MAX_CHANNEL];
> + bool temp_en[LTC2991_MAX_CHANNEL];
> +};
> +
> +static int ltc2991_read_reg(struct ltc2991_state *st, u8 addr, u8 reg_len,
> + int *val)
> +{
> + u8 regvals[2];
> + int ret;
> + int i;
> +
> + ret = regmap_bulk_read(st->regmap, addr, regvals, reg_len);
> + if (ret)
> + return ret;
> +
> + *val = 0;
> + for (i = 0; i < reg_len; i++)
> + *val |= regvals[reg_len - i - 1] << (i * 8);
> +
> + return 0;
> +}
> +
> +static int ltc2991_get_voltage(struct ltc2991_state *st, u32 reg, long *val)
> +{
> + int reg_val, ret, offset = 0;
> +
> + ret = ltc2991_read_reg(st, reg, 2, ®_val);
> + if (ret)
> + return ret;
> +
> + if (reg == LTC2991_VCC_MSB)
> + /* Vcc 2.5V offset */
> + offset = 2500;
> +
> + /* Vx, 305.18uV/LSB */
> + *val = DIV_ROUND_CLOSEST(sign_extend32(reg_val, 14) * 30518,
> + 1000 * 100) + offset;
> +
> + return 0;
> +}
> +
> +static int ltc2991_read_in(struct device *dev, u32 attr, int channel, long *val)
> +{
> + struct ltc2991_state *st = dev_get_drvdata(dev);
> + u32 reg;
> +
> + switch (attr) {
> + case hwmon_in_input:
> + if (channel == LTC2991_VCC_CH_NR)
> + reg = LTC2991_VCC_MSB;
> + else
> + reg = LTC2991_CHANNEL_V_MSB(channel - 1);
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + return ltc2991_get_voltage(st, reg, val);
I see no value having this code here. WHy not in the case statement above ?
> +}
> +
> +static int ltc2991_get_curr(struct ltc2991_state *st, u32 reg, int channel,
> + long *val)
> +{
> + int reg_val, ret;
> +
> + ret = ltc2991_read_reg(st, reg, 2, ®_val);
> + if (ret)
> + return ret;
> +
> + /* Vx-Vy, 19.075uV/LSB */
> + *val = DIV_ROUND_CLOSEST(sign_extend32(reg_val, 14) * 19075, 1000)
> + / st->r_sense_mohm[channel];
> +
> + return 0;
> +}
> +
> +static int ltc2991_read_curr(struct device *dev, u32 attr, int channel,
> + long *val)
> +{
> + struct ltc2991_state *st = dev_get_drvdata(dev);
> + u32 reg;
> +
> + switch (attr) {
> + case hwmon_curr_input:
> + reg = LTC2991_CHANNEL_C_MSB(channel);
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + return ltc2991_get_curr(st, reg, channel, val);
Even less value than above. That would only make sense if there were
more attributes to read.
> +}
> +
> +static int ltc2991_get_temp(struct ltc2991_state *st, u32 reg, int channel,
> + long *val)
> +{
> + int reg_val, ret;
> +
> + ret = ltc2991_read_reg(st, reg, 2, ®_val);
> + if (ret)
> + return ret;
> +
> + /* Temp LSB = 0.0625 Degrees */
> + *val = DIV_ROUND_CLOSEST(sign_extend32(reg_val, 12) * 1000, 16);
> +
> + return 0;
> +}
> +
> +static int ltc2991_read_temp(struct device *dev, u32 attr, int channel,
> + long *val)
> +{
> + struct ltc2991_state *st = dev_get_drvdata(dev);
> + u32 reg;
> +
> + switch (attr) {
> + case hwmon_temp_input:
> + if (channel == LTC2991_T_INT_CH_NR)
> + reg = LTC2991_T_INT_MSB;
> + else
> + reg = LTC2991_CHANNEL_T_MSB(channel);
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + return ltc2991_get_temp(st, reg, channel, val);
And again.
> +}
> +
> +static int ltc2991_read(struct device *dev, enum hwmon_sensor_types type, u32 attr, int channel,
> + long *val)
> +{
> + switch (type) {
> + case hwmon_in:
> + return ltc2991_read_in(dev, attr, channel, val);
> + case hwmon_curr:
> + return ltc2991_read_curr(dev, attr, channel, val);
> + case hwmon_temp:
> + return ltc2991_read_temp(dev, attr, channel, val);
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static umode_t ltc2991_is_visible(const void *data, enum hwmon_sensor_types type, u32 attr,
> + int channel)
> +{
> + const struct ltc2991_state *st = data;
> +
> + switch (type) {
> + case hwmon_in:
> + switch (attr) {
> + case hwmon_in_input:
> + case hwmon_in_label:
> + return 0444;
> + }
> + break;
> + case hwmon_curr:
> + switch (attr) {
> + case hwmon_curr_input:
> + case hwmon_curr_label:
> + if (st->r_sense_mohm[channel])
> + return 0444;
> + break;
> + }
> + break;
> + case hwmon_temp:
> + switch (attr) {
> + case hwmon_temp_input:
> + case hwmon_temp_label:
> + if (st->temp_en[channel] || channel == LTC2991_T_INT_CH_NR)
> + return 0444;
> + break;
> + }
> + break;
> + default:
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static int ltc2991_read_string(struct device *dev, enum hwmon_sensor_types type, u32 attr,
> + int channel, const char **str)
> +{
> + switch (type) {
> + case hwmon_temp:
> + *str = label_temp[channel];
> + break;
> + case hwmon_curr:
> + *str = label_curr[channel];
> + break;
> + case hwmon_in:
> + *str = label_voltages[channel];
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + return 0;
> +}
> +
> +static const struct hwmon_ops ltc2991_hwmon_ops = {
> + .is_visible = ltc2991_is_visible,
> + .read = ltc2991_read,
> + .read_string = ltc2991_read_string,
> +};
> +
> +static const struct hwmon_channel_info *ltc2991_info[] = {
> + HWMON_CHANNEL_INFO(temp,
> + HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_INPUT | HWMON_T_LABEL
> + ),
> + HWMON_CHANNEL_INFO(curr,
> + HWMON_C_INPUT | HWMON_C_LABEL,
> + HWMON_C_INPUT | HWMON_C_LABEL,
> + HWMON_C_INPUT | HWMON_C_LABEL,
> + HWMON_C_INPUT | HWMON_C_LABEL
> + ),
> + HWMON_CHANNEL_INFO(in,
> + HWMON_I_INPUT | HWMON_I_LABEL,
> + HWMON_I_INPUT | HWMON_I_LABEL,
> + HWMON_I_INPUT | HWMON_I_LABEL,
> + HWMON_I_INPUT | HWMON_I_LABEL,
> + HWMON_I_INPUT | HWMON_I_LABEL,
> + HWMON_I_INPUT | HWMON_I_LABEL,
> + HWMON_I_INPUT | HWMON_I_LABEL,
> + HWMON_I_INPUT | HWMON_I_LABEL,
> + HWMON_I_INPUT | HWMON_I_LABEL
> + ),
> + NULL
> +};
> +
> +static const struct hwmon_chip_info ltc2991_chip_info = {
> + .ops = <c2991_hwmon_ops,
> + .info = ltc2991_info,
> +};
> +
> +static const struct regmap_config ltc2991_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .max_register = 0x1D,
> +};
> +
> +static int ltc2991_init(struct ltc2991_state *st)
> +{
> + struct fwnode_handle *fwnode;
> + struct fwnode_handle *child;
> + int ret;
> + u32 val, addr;
> + u8 v5_v8_reg_data = 0, v1_v4_reg_data = 0;
> +
> + ret = devm_regulator_get_enable(&st->client->dev, "vcc");
The only use of st->client is in this function, which is called from probe.
Please just pass client as parameter (or even &client->dev since that is
what is really used).
> + if (ret)
> + return dev_err_probe(&st->client->dev, ret,
> + "failed to enable regulator\n");
> +
> + fwnode = dev_fwnode(&st->client->dev);
> +
> + fwnode_for_each_available_child_node(fwnode, child) {
> + ret = fwnode_property_read_u32(child, "reg", &addr);
> + if (ret < 0) {
> + fwnode_handle_put(child);
> + return ret;
> + }
> +
> + if (addr > 3) {
> + fwnode_handle_put(child);
> + return -EINVAL;
> + }
> +
> + ret = fwnode_property_read_u32(child, "shunt-resistor-mili-ohms", &val);
> + if (!ret) {
> + st->r_sense_mohm[addr] = val;
shunt resistors may be in fraction of milli-ohms.
> + switch (addr) {
> + case 0:
> + v1_v4_reg_data |= LTC2991_V1_V2_DIFF_EN;
> + break;
> + case 1:
> + v1_v4_reg_data |= LTC2991_V3_V4_DIFF_EN;
> + break;
> + case 2:
> + v5_v8_reg_data |= LTC2991_V5_V6_DIFF_EN;
> + break;
> + case 3:
> + v5_v8_reg_data |= LTC2991_V7_V8_DIFF_EN;
> + break;
> + default:
> + break;
> + }
While the above accepts sense resistor values of 0, it does not
distinguish that from "value not provided". At the very least that needs
an explanation.
> + }
> +
> + ret = fwnode_property_read_bool(child, "temperature-enable");
> + if (ret) {
> + st->temp_en[addr] = ret;
> + switch (addr) {
> + case 0:
> + v1_v4_reg_data |= LTC2991_V1_V2_TEMP_EN;
> + break;
> + case 1:
> + v1_v4_reg_data |= LTC2991_V3_V4_TEMP_EN;
> + break;
> + case 2:
> + v5_v8_reg_data |= LTC2991_V5_V6_TEMP_EN;
> + break;
> + case 3:
> + v5_v8_reg_data |= LTC2991_V7_V8_TEMP_EN;
> + break;
> + default:
> + break;
> + }
> + }
> + }
I am not happy that there are no defaults, meaning the chip can not be used
without devicetree or firmware (acpi) data. This is very undesirable and not
common for hwmon sensors. I would very much prefer the use of defaults
(i.e., existing register values and a default for the shunt resistor).
> +
> + /* Setup V5-V8 Control register */
> + ret = regmap_write(st->regmap, LTC2991_V5_V8_CTRL, v5_v8_reg_data);
> + if (ret)
> + return dev_err_probe(&st->client->dev, ret,
> + "Error: Failed to set V5-V8 CTRL reg.\n");
> +
> + /* Setup V1-V4 Control register */
> + ret = regmap_write(st->regmap, LTC2991_V1_V4_CTRL, v1_v4_reg_data);
> + if (ret)
> + return dev_err_probe(&st->client->dev, ret,
> + "Error: Failed to set V1-V4 CTRL reg.\n");
> +
> + /* Setup continuous mode */
> + ret = regmap_write(st->regmap, LTC2991_PWM_TH_LSB_T_INT,
> + LTC2991_REPEAT_ACQ_EN);
> + if (ret)
> + return dev_err_probe(&st->client->dev, ret,
> + "Error: Failed to set contiuous mode.\n");
> +
> + /* Enable all channels and trigger conversions */
> + ret = regmap_write(st->regmap, LTC2991_CH_EN_TRIGGER,
> + LTC2991_V7_V8_EN | LTC2991_V5_V6_EN |
> + LTC2991_V3_V4_EN | LTC2991_V1_V2_EN |
> + LTC2991_T_INT_VCC_EN);
> + if (ret)
> + return dev_err_probe(&st->client->dev, ret,
> + "Error: Failed to enable conversions.\n");
> +
> + return 0;
> +}
> +
> +static int ltc2991_i2c_probe(struct i2c_client *client)
> +{
> + int ret;
> + struct device *hwmon_dev;
> + struct ltc2991_state *st;
> +
> + if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_WORD_DATA))
> + return -EOPNOTSUPP;
> +
> + st = devm_kzalloc(&client->dev, sizeof(*st), GFP_KERNEL);
> + if (!st)
> + return -ENOMEM;
> +
> + st->client = client;
> + st->regmap = devm_regmap_init_i2c(client, <c2991_regmap_config);
> + if (IS_ERR(st->regmap))
> + return PTR_ERR(st->regmap);
> +
> + ret = ltc2991_init(st);
> + if (ret)
> + return ret;
> +
> + hwmon_dev = devm_hwmon_device_register_with_info(&client->dev,
> + client->name, st,
> + <c2991_chip_info,
> + NULL);
> +
> + return PTR_ERR_OR_ZERO(hwmon_dev);
> +}
> +
> +static const struct of_device_id ltc2991_of_match[] = {
> + { .compatible = "adi,ltc2991" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, ltc2991_of_match);
> +
> +static const struct i2c_device_id ltc2991_i2c_id[] = {
> + { "ltc2991", 0 },
> + {}
> +};
> +MODULE_DEVICE_TABLE(i2c, ltc2991_i2c_id);
> +
> +static struct i2c_driver ltc2991_i2c_driver = {
> + .class = I2C_CLASS_HWMON,
> + .driver = {
> + .name = "ltc2991",
> + .of_match_table = ltc2991_of_match,
> + },
> + .probe = ltc2991_i2c_probe,
> + .id_table = ltc2991_i2c_id,
> +};
> +
> +module_i2c_driver(ltc2991_i2c_driver);
> +
> +MODULE_AUTHOR("Antoniu Miclaus <[email protected]>");
> +MODULE_DESCRIPTION("Analog Devices LTC2991 HWMON Driver");
> +MODULE_LICENSE("GPL");
> --
> 2.42.0
>