* [PATCH 0/3] LTC2947 support
@ 2019-09-24 12:49 Nuno Sá
2019-09-24 12:49 ` [PATCH 1/3] hwmon: Fix HWMON_P_MIN_ALARM mask Nuno Sá
` (3 more replies)
0 siblings, 4 replies; 21+ messages in thread
From: Nuno Sá @ 2019-09-24 12:49 UTC (permalink / raw)
To: linux-hwmon, devicetree, linux-doc
Cc: Rob Herring, Mark Rutland, Guenter Roeck, Jean Delvare, Jonathan Corbet
This series adds support for the LTC2947 hwmon device. The device
supports power[1-*]_min_alarm, so that it makes use of
the HWMON_P_MIN_ALARM mask. This brings me to the first patch, which is a
fix on the hwmon subsystem for the HWMON_P_MIN_ALARM mask. It was defining
the same mask as HWMON_P_MAX_ALARM.
The rest of the series is the usual for a new device.
Nuno Sá (3):
hwmon: Fix HWMON_P_MIN_ALARM mask
hwmon: Add support for ltc2947
dt-bindings: iio: Add ltc2947 documentation
.../bindings/hwmon/adi,ltc2947.yaml | 101 ++
Documentation/hwmon/ltc2947.rst | 110 ++
MAINTAINERS | 11 +
drivers/hwmon/Kconfig | 27 +
drivers/hwmon/Makefile | 3 +
drivers/hwmon/ltc2947-core.c | 1421 +++++++++++++++++
drivers/hwmon/ltc2947-i2c.c | 49 +
drivers/hwmon/ltc2947-spi.c | 50 +
drivers/hwmon/ltc2947.h | 12 +
include/linux/hwmon.h | 2 +-
10 files changed, 1785 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/hwmon/adi,ltc2947.yaml
create mode 100644 Documentation/hwmon/ltc2947.rst
create mode 100644 drivers/hwmon/ltc2947-core.c
create mode 100644 drivers/hwmon/ltc2947-i2c.c
create mode 100644 drivers/hwmon/ltc2947-spi.c
create mode 100644 drivers/hwmon/ltc2947.h
--
2.23.0
^ permalink raw reply [flat|nested] 21+ messages in thread* [PATCH 1/3] hwmon: Fix HWMON_P_MIN_ALARM mask 2019-09-24 12:49 [PATCH 0/3] LTC2947 support Nuno Sá @ 2019-09-24 12:49 ` Nuno Sá 2019-10-02 13:37 ` Guenter Roeck 2019-09-24 12:49 ` [PATCH 2/3] hwmon: Add support for ltc2947 Nuno Sá ` (2 subsequent siblings) 3 siblings, 1 reply; 21+ messages in thread From: Nuno Sá @ 2019-09-24 12:49 UTC (permalink / raw) To: linux-hwmon, devicetree, linux-doc Cc: Rob Herring, Mark Rutland, Guenter Roeck, Jean Delvare, Jonathan Corbet, stable Both HWMON_P_MIN_ALARM and HWMON_P_MAX_ALARM were using BIT(hwmon_power_max_alarm). Fixes: aa7f29b07c870 ("hwmon: Add support for power min, lcrit, min_alarm and lcrit_alarm") CC: <stable@vger.kernel.org> Signed-off-by: Nuno Sá <nuno.sa@analog.com> --- include/linux/hwmon.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/hwmon.h b/include/linux/hwmon.h index 04c36b7a61dd..72579168189d 100644 --- a/include/linux/hwmon.h +++ b/include/linux/hwmon.h @@ -235,7 +235,7 @@ enum hwmon_power_attributes { #define HWMON_P_LABEL BIT(hwmon_power_label) #define HWMON_P_ALARM BIT(hwmon_power_alarm) #define HWMON_P_CAP_ALARM BIT(hwmon_power_cap_alarm) -#define HWMON_P_MIN_ALARM BIT(hwmon_power_max_alarm) +#define HWMON_P_MIN_ALARM BIT(hwmon_power_min_alarm) #define HWMON_P_MAX_ALARM BIT(hwmon_power_max_alarm) #define HWMON_P_LCRIT_ALARM BIT(hwmon_power_lcrit_alarm) #define HWMON_P_CRIT_ALARM BIT(hwmon_power_crit_alarm) -- 2.23.0 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 1/3] hwmon: Fix HWMON_P_MIN_ALARM mask 2019-09-24 12:49 ` [PATCH 1/3] hwmon: Fix HWMON_P_MIN_ALARM mask Nuno Sá @ 2019-10-02 13:37 ` Guenter Roeck 0 siblings, 0 replies; 21+ messages in thread From: Guenter Roeck @ 2019-10-02 13:37 UTC (permalink / raw) To: Nuno Sá Cc: linux-hwmon, devicetree, linux-doc, Rob Herring, Mark Rutland, Jean Delvare, Jonathan Corbet, stable On Tue, Sep 24, 2019 at 02:49:43PM +0200, Nuno Sá wrote: > Both HWMON_P_MIN_ALARM and HWMON_P_MAX_ALARM were using > BIT(hwmon_power_max_alarm). > > Fixes: aa7f29b07c870 ("hwmon: Add support for power min, lcrit, min_alarm and lcrit_alarm") > CC: <stable@vger.kernel.org> > Signed-off-by: Nuno Sá <nuno.sa@analog.com> Applied. Thanks, Guenter > --- > include/linux/hwmon.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/linux/hwmon.h b/include/linux/hwmon.h > index 04c36b7a61dd..72579168189d 100644 > --- a/include/linux/hwmon.h > +++ b/include/linux/hwmon.h > @@ -235,7 +235,7 @@ enum hwmon_power_attributes { > #define HWMON_P_LABEL BIT(hwmon_power_label) > #define HWMON_P_ALARM BIT(hwmon_power_alarm) > #define HWMON_P_CAP_ALARM BIT(hwmon_power_cap_alarm) > -#define HWMON_P_MIN_ALARM BIT(hwmon_power_max_alarm) > +#define HWMON_P_MIN_ALARM BIT(hwmon_power_min_alarm) > #define HWMON_P_MAX_ALARM BIT(hwmon_power_max_alarm) > #define HWMON_P_LCRIT_ALARM BIT(hwmon_power_lcrit_alarm) > #define HWMON_P_CRIT_ALARM BIT(hwmon_power_crit_alarm) ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/3] hwmon: Add support for ltc2947 2019-09-24 12:49 [PATCH 0/3] LTC2947 support Nuno Sá 2019-09-24 12:49 ` [PATCH 1/3] hwmon: Fix HWMON_P_MIN_ALARM mask Nuno Sá @ 2019-09-24 12:49 ` Nuno Sá 2019-10-03 4:14 ` Guenter Roeck 2019-09-24 12:49 ` [PATCH 3/3] dt-bindings: iio: Add ltc2947 documentation Nuno Sá 2019-09-26 10:17 ` [PATCH 0/3] LTC2947 support Sa, Nuno 3 siblings, 1 reply; 21+ messages in thread From: Nuno Sá @ 2019-09-24 12:49 UTC (permalink / raw) To: linux-hwmon, devicetree, linux-doc Cc: Rob Herring, Mark Rutland, Guenter Roeck, Jean Delvare, Jonathan Corbet The ltc2947 is a high precision power and energy monitor with an internal sense resistor supporting up to +/- 30A. Three internal no Latency ADCs ensure accurate measurement of voltage and current, while high-bandwidth analog multiplication of voltage and current provides accurate power measurement in a wide range of applications. Internal or external clocking options enable precise charge and energy measurements. Signed-off-by: Nuno Sá <nuno.sa@analog.com> --- Documentation/hwmon/ltc2947.rst | 110 +++ MAINTAINERS | 10 + drivers/hwmon/Kconfig | 27 + drivers/hwmon/Makefile | 3 + drivers/hwmon/ltc2947-core.c | 1421 +++++++++++++++++++++++++++++++ drivers/hwmon/ltc2947-i2c.c | 49 ++ drivers/hwmon/ltc2947-spi.c | 50 ++ drivers/hwmon/ltc2947.h | 12 + 8 files changed, 1682 insertions(+) create mode 100644 Documentation/hwmon/ltc2947.rst create mode 100644 drivers/hwmon/ltc2947-core.c create mode 100644 drivers/hwmon/ltc2947-i2c.c create mode 100644 drivers/hwmon/ltc2947-spi.c create mode 100644 drivers/hwmon/ltc2947.h diff --git a/Documentation/hwmon/ltc2947.rst b/Documentation/hwmon/ltc2947.rst new file mode 100644 index 000000000000..2dcf7487076d --- /dev/null +++ b/Documentation/hwmon/ltc2947.rst @@ -0,0 +1,110 @@ +Kernel drivers ltc2947-i2c and ltc2947-spi +========================================= + +Supported chips: + * Analog Devices LTC2947 + Prefix: 'ltc2947' + Addresses scanned: - + Datasheet: + https://www.analog.com/media/en/technical-documentation/data-sheets/LTC2947.pdf + +Author: Nuno Sa <nuno.sa@analog.com> + +Description +___________ + +The LTC2947 is a high precision power and energy monitor that measures current, +voltage, power, temperature, charge and energy. The device supports both SPI +and I2C depending on the chip configuration. +The device also measures accumulated quantities as energy. It has two banks of +register's to read/set energy related values. These banks can be configured +independently to have setups like: energy1 accumulates always and enrgy2 only +accumulates if current is positive (to check battery charging efficiency for +example). The device also supports a GPIO pin that can be configured as output +to control a fan as a function of measured temperature. Then, the GPIO becomes +active as soon as a temperature reading is higher than a defined threshold. The +temp2 channel is used to control this thresholds and to read the respective +alarms. + +Sysfs entries +_____________ + +The following attributes are supported. Limits are read-write, reset_history +is write-only and all the other attributes are read-only. + +in0_input VP-VM voltage (mV). +in0_min Undervoltage threshold +in0_max Overvoltage threshold +in0_lowest Lowest measured voltage +in0_highest Highest measured voltage +in0_reset_history Write 1 to reset in1 history +in0_min_alarm Undervoltage alarm +in0_max_alarm Overvoltage alarm +in0_fault Fault value +in0_label Channel label (VP-VM) + +in1_input DVCC voltage (mV) +in1_min Undervoltage threshold +in1_max Overvoltage threshold +in1_lowest Lowest measured voltage +in1_highest Highest measured voltage +in1_reset_history Write 1 to reset in2 history +in1_min_alarm Undervoltage alarm +in1_max_alarm Overvoltage alarm +in1_fault Fault value +in1_label Channel label (DVCC) + +curr1_input IP-IM Sense current (mA) +curr1_min Undercurrent threshold +curr1_max Overcurrent threshold +curr1_lowest Lowest measured current +curr1_highest Highest measured current +curr1_reset_history Write 1 to reset curr1 history +curr1_min_alarm Undercurrent alarm +curr1_max_alarm Overcurrent alarm +curr1_fault Fault value +curr1_label Channel label (IP-IM) + +power1_input Power (in uW) +power1_min Low power threshold +power1_max High power threshold +power1_input_lowest Historical minimum power use +power1_input_highest Historical maximum power use +power1_reset_history Write 1 to reset power1 history +power1_min_alarm Low power alarm +power1_max_alarm High power alarm +power1_fault Fault value +power1_label Channel label (Power) + +temp1_input Chip Temperature (in milliC) +temp1_min Low temperature threshold +temp1_max High temperature threshold +temp1_input_lowest Historical minimum temperature use +temp1_input_highest Historical maximum temperature use +temp1_reset_history Write 1 to reset temp1 history +temp1_min_alarm Low temperature alarm +temp1_max_alarm High temperature alarm +temp1_fault Fault value +temp1_label Channel label (Ambient) + +temp2_min Low temperature threshold for fan control +temp2_max High temperature threshold for fan control +temp2_min_alarm Low temperature fan control alarm +temp2_max_alarm High temperature fan control alarm +temp2_label Channel label (TEMPFAN) + +energy1_input Measured energy over time (in microJoule) +energy1_max High energy threshold +energy1_max_alarm High energy alarm +energy1_min Low energy threshold +energy1_min_alarm Low energy alarm +energy1_overflow_alarm Energy1 register is about to overflow +energy1_fault Fault value + +energy2_input Measured energy over time (in microJoule) +energy2_max High energy threshold +energy2_max_alarm High energy alarm +energy2_min Low energy threshold +energy2_min_alarm Low energy alarm +energy2_overflow_alarm Energy2 register is about to overflow +energy2_fault Fault value diff --git a/MAINTAINERS b/MAINTAINERS index c7035ce2460b..889f38c1c930 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -9496,6 +9496,16 @@ S: Maintained F: Documentation/hwmon/ltc4261.rst F: drivers/hwmon/ltc4261.c +LTC2947 HARDWARE MONITOR DRIVER +M: Nuno Sá <nuno.sa@analog.com> +W: http://ez.analog.com/community/linux-device-drivers +L: linux-hwmon@vger.kernel.org +S: Supported +F: drivers/hwmon/ltc2947-core.c +F: drivers/hwmon/ltc2947-spi.c +F: drivers/hwmon/ltc2947-i2c.c +F: drivers/hwmon/ltc2947.h + LTC4306 I2C MULTIPLEXER DRIVER M: Michael Hennerich <michael.hennerich@analog.com> W: http://ez.analog.com/community/linux-device-drivers diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index 2ca5668bdb62..ad35d625b6d3 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -737,6 +737,33 @@ config SENSORS_LTC2945 This driver can also be built as a module. If so, the module will be called ltc2945. +config SENSORS_LTC2947 + tristate + +config SENSORS_LTC2947_I2C + tristate "Analog Devices LTC2947 High Precision Power and Energy Monitor over I2C" + depends on I2C + select REGMAP_I2C + select SENSORS_LTC2947 + help + If you say yes here you get support for Linear Technology LTC2947 + I2C High Precision Power and Energy Monitor + + This driver can also be built as a module. If so, the module will + be called ltc2947-i2c. + +config SENSORS_LTC2947_SPI + tristate "Analog Devices LTC2947 High Precision Power and Energy Monitor over SPI" + depends on SPI_MASTER + select REGMAP_SPI + select SENSORS_LTC2947 + help + If you say yes here you get support for Linear Technology LTC2947 + SPI High Precision Power and Energy Monitor + + This driver can also be built as a module. If so, the module will + be called ltc2947-spi. + config SENSORS_LTC2990 tristate "Linear Technology LTC2990" depends on I2C diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index c86ce4d3d36b..b54a4cec3203 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -107,6 +107,9 @@ obj-$(CONFIG_SENSORS_LM95234) += lm95234.o obj-$(CONFIG_SENSORS_LM95241) += lm95241.o obj-$(CONFIG_SENSORS_LM95245) += lm95245.o obj-$(CONFIG_SENSORS_LTC2945) += ltc2945.o +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_LTC4151) += ltc4151.o obj-$(CONFIG_SENSORS_LTC4215) += ltc4215.o diff --git a/drivers/hwmon/ltc2947-core.c b/drivers/hwmon/ltc2947-core.c new file mode 100644 index 000000000000..350b2b76467c --- /dev/null +++ b/drivers/hwmon/ltc2947-core.c @@ -0,0 +1,1421 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Analog Devices LTC2947 high precision power and energy monitor + * + * Copyright 2019 Analog Devices Inc. + */ +#include <linux/bitfield.h> +#include <linux/clk.h> +#include <linux/device.h> +#include <linux/hwmon.h> +#include <linux/hwmon-sysfs.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/regmap.h> + +#include "ltc2947.h" + +/* register's */ +#define LTC2947_REG_PAGE_CTRL 0xFF +#define LTC2947_REG_CTRL 0xF0 +#define LTC2947_REG_TBCTL 0xE9 +#define LTC2947_CONT_MODE_MASK BIT(3) +#define LTC2947_CONT_MODE(x) FIELD_PREP(LTC2947_CONT_MODE_MASK, x) +#define LTC2947_PRE_MASK GENMASK(2, 0) +#define LTC2947_PRE(x) FIELD_PREP(LTC2947_PRE_MASK, x) +#define LTC2947_DIV_MASK GENMASK(7, 3) +#define LTC2947_DIV(x) FIELD_PREP(LTC2947_DIV_MASK, x) +#define LTC2947_SHUTDOWN_MASK BIT(0) +#define LTC2947_REG_ACCUM_POL 0xE1 +#define LTC2947_ACCUM_POL_1_MASK GENMASK(1, 0) +#define LTC2947_ACCUM_POL_1(x) FIELD_PREP(LTC2947_ACCUM_POL_1_MASK, x) +#define LTC2947_ACCUM_POL_2_MASK GENMASK(3, 2) +#define LTC2947_ACCUM_POL_2(x) FIELD_PREP(LTC2947_ACCUM_POL_2_MASK, x) +#define LTC2947_REG_ACCUM_DEADBAND 0xE4 +#define LTC2947_REG_GPIOSTATCTL 0x67 +#define LTC2947_GPIO_EN_MASK BIT(0) +#define LTC2947_GPIO_EN(x) FIELD_PREP(LTC2947_GPIO_EN_MASK, x) +#define LTC2947_GPIO_FAN_EN_MASK BIT(6) +#define LTC2947_GPIO_FAN_EN(x) FIELD_PREP(LTC2947_GPIO_FAN_EN_MASK, x) +#define LTC2947_GPIO_FAN_POL_MASK BIT(7) +#define LTC2947_GPIO_FAN_POL(x) FIELD_PREP(LTC2947_GPIO_FAN_POL_MASK, x) +#define LTC2947_REG_GPIO_ACCUM 0xE3 +/* 200Khz */ +#define LTC2947_CLK_MIN 200000 +/* 25Mhz */ +#define LTC2947_CLK_MAX 25000000 +#define PAGE0 0 +#define PAGE1 1 +/* Voltage registers */ +#define LTC2947_REG_VOLTAGE 0xA0 +#define LTC2947_REG_VOLTAGE_MAX 0x50 +#define LTC2947_REG_VOLTAGE_MIN 0x52 +#define LTC2947_REG_VOLTAGE_THRE_H 0x90 +#define LTC2947_REG_VOLTAGE_THRE_L 0x92 +#define LTC2947_REG_DVCC 0xA4 +#define LTC2947_REG_DVCC_MAX 0x58 +#define LTC2947_REG_DVCC_MIN 0x5A +#define LTC2947_REG_DVCC_THRE_H 0x98 +#define LTC2947_REG_DVCC_THRE_L 0x9A +#define LTC2947_VOLTAGE_GEN_CHAN 0 +#define LTC2947_VOLTAGE_DVCC_CHAN 1 +/* Current registers */ +#define LTC2947_REG_CURRENT 0x90 +#define LTC2947_REG_CURRENT_MAX 0x40 +#define LTC2947_REG_CURRENT_MIN 0x42 +#define LTC2947_REG_CURRENT_THRE_H 0x80 +#define LTC2947_REG_CURRENT_THRE_L 0x82 +/* Power registers */ +#define LTC2947_REG_POWER 0x93 +#define LTC2947_REG_POWER_MAX 0x44 +#define LTC2947_REG_POWER_MIN 0x46 +#define LTC2947_REG_POWER_THRE_H 0x84 +#define LTC2947_REG_POWER_THRE_L 0x86 +/* Temperature registers */ +#define LTC2947_REG_TEMP 0xA2 +#define LTC2947_REG_TEMP_MAX 0x54 +#define LTC2947_REG_TEMP_MIN 0x56 +#define LTC2947_REG_TEMP_THRE_H 0x94 +#define LTC2947_REG_TEMP_THRE_L 0x96 +#define LTC2947_REG_TEMP_FAN_THRE_H 0x9C +#define LTC2947_REG_TEMP_FAN_THRE_L 0x9E +#define LTC2947_TEMP_FAN_CHAN 1 +/* Energy registers */ +#define LTC2947_REG_ENERGY1 0x06 +#define LTC2947_REG_ENERGY1_THRE_H 0x10 +#define LTC2947_REG_ENERGY1_THRE_L 0x16 +#define LTC2947_REG_ENERGY2 0x16 +#define LTC2947_REG_ENERGY2_THRE_H 0x30 +#define LTC2947_REG_ENERGY2_THRE_L 0x36 +#define ENERGY_MIN 0xFFFF800000000000LL +#define ENERGY_MAX 0x00007FFFFFFFFFFFLL +/* Status/Alarm/Overflow registers */ +#define LTC2947_REG_STATUS 0x80 +#define LTC2947_REG_STATVT 0x81 +#define LTC2947_REG_STATIP 0x82 +#define LTC2947_REG_STATC 0x83 +#define LTC2947_REG_STATE 0x84 +#define LTC2947_REG_STATCEOF 0x85 +#define LTC2947_REG_STATVDVCC 0x87 + +#define LTC2947_ALERTS_SIZE (LTC2947_REG_STATVDVCC - LTC2947_REG_STATUS) +#define LTC2947_UPDATE_VAL_MASK BIT(4) +#define LTC2947_MAX_VOLTAGE_MASK BIT(0) +#define LTC2947_MIN_VOLTAGE_MASK BIT(1) +#define LTC2947_MAX_CURRENT_MASK BIT(0) +#define LTC2947_MIN_CURRENT_MASK BIT(1) +#define LTC2947_MAX_POWER_MASK BIT(2) +#define LTC2947_MIN_POWER_MASK BIT(3) +#define LTC2947_MAX_TEMP_MASK BIT(2) +#define LTC2947_MIN_TEMP_MASK BIT(3) +#define LTC2947_MAX_TEMP_FAN_MASK BIT(4) +#define LTC2947_MIN_TEMP_FAN_MASK BIT(5) +#define LTC2947_SINGLE_SHOT_MASK BIT(2) +#define LTC2947_MAX_ENERGY1_MASK BIT(0) +#define LTC2947_MIN_ENERGY1_MASK BIT(1) +#define LTC2947_MAX_ENERGY2_MASK BIT(2) +#define LTC2947_MIN_ENERGY2_MASK BIT(3) +#define LTC2947_MIN_ENERGY1_O_MASK BIT(4) +#define LTC2947_MIN_ENERGY2_O_MASK BIT(5) +#define LTC2947_ADCERR_MASK BIT(5) +/* + * For accumulated values there's a fault if the ADC conversions are invalid + * (ADCERR) or if there is an overflow of the internal timebase register + * (which indicates invalid TBCTL configuration). + */ +#define LTC2947_ENERGY_FAULT_MASK GENMASK(6, 5) + +struct ltc2947_data { + struct regmap *map; + struct device *dev; + /* + * The mutex is needed because the device has 2 memory pages. When + * reading/writing the correct page needs to be set so that, the + * complete sequence select_page->read/write needs to be protected. + */ + struct mutex lock; + u32 lsb_energy; + bool reset; + bool gpio_out; +}; + +/* used for raw sysfs entries */ +enum { + LTC2947_POWER_INPUT, + LTC2947_POWER_THRE_L, + LTC2947_POWER_THRE_H, + LTC2947_POWER_HIGHEST, + LTC2947_POWER_LOWEST, + LTC2947_ENERGY1_INPUT, + LTC2947_ENERGY1_THRE_H, + LTC2947_ENERGY1_THRE_L, + LTC2947_ENERGY1_MAX_ALARM, + LTC2947_ENERGY1_MIN_ALARM, + LTC2947_ENERGY2_INPUT, + LTC2947_ENERGY2_THRE_H, + LTC2947_ENERGY2_THRE_L, + LTC2947_ENERGY2_MAX_ALARM, + LTC2947_ENERGY2_MIN_ALARM, + LTC2947_ENERGY1_OVERF_ALARM, + LTC2947_ENERGY2_OVERF_ALARM, + LTC2947_FAULT = LTC2947_ADCERR_MASK, + LTC2947_ENERGY_FAULT = LTC2947_ENERGY_FAULT_MASK, +}; + +static int __ltc2947_val_read16(const struct ltc2947_data *st, const u8 reg, + u64 *val) +{ + __be16 __val = 0; + int ret; + + ret = regmap_bulk_read(st->map, reg, &__val, 2); + if (ret) + return ret; + + *val = be16_to_cpu(__val); + + return 0; +} + +static int __ltc2947_val_read24(const struct ltc2947_data *st, const u8 reg, + u64 *val) +{ + __be32 __val = 0; + int ret; + + ret = regmap_bulk_read(st->map, reg, &__val, 3); + if (ret) + return ret; + + *val = be32_to_cpu(__val) >> 8; + + return 0; +} + +static int __ltc2947_val_read64(const struct ltc2947_data *st, const u8 reg, + u64 *val) +{ + __be64 __val = 0; + int ret; + + ret = regmap_bulk_read(st->map, reg, &__val, 6); + if (ret) + return ret; + + *val = be64_to_cpu(__val) >> 16; + + return 0; +} + +static int ltc2947_val_read(struct ltc2947_data *st, const u8 reg, + const u8 page, const size_t size, s64 *val) +{ + int ret; + u64 __val = 0; + + mutex_lock(&st->lock); + + if (st->reset) { + mutex_unlock(&st->lock); + return -EPERM; + } + + ret = regmap_write(st->map, LTC2947_REG_PAGE_CTRL, page); + if (ret) { + mutex_unlock(&st->lock); + return ret; + } + + dev_dbg(st->dev, "Read val, reg:%02X, p:%d sz:%zu\n", reg, page, + size); + + switch (size) { + case 2: + ret = __ltc2947_val_read16(st, reg, &__val); + break; + case 3: + ret = __ltc2947_val_read24(st, reg, &__val); + break; + case 6: + ret = __ltc2947_val_read64(st, reg, &__val); + break; + default: + dev_err(st->dev, "Invalid size(%zu) to read", size); + ret = -EINVAL; + break; + } + + mutex_unlock(&st->lock); + + if (ret) + return ret; + + *val = sign_extend64(__val, (8 * size) - 1); + + dev_dbg(st->dev, "Got s:%lld, u:%016llX\n", *val, __val); + + return 0; +} + +static int __ltc2947_val_write64(const struct ltc2947_data *st, const u8 reg, + const u64 val) +{ + __be64 __val; + + __val = cpu_to_be64(val << 16); + return regmap_bulk_write(st->map, reg, &__val, 6); +} + +static int __ltc2947_val_write16(const struct ltc2947_data *st, const u8 reg, + const u16 val) +{ + __be16 __val; + + __val = cpu_to_be16(val); + return regmap_bulk_write(st->map, reg, &__val, 2); +} + +static int ltc2947_val_write(struct ltc2947_data *st, const u8 reg, + const u8 page, const size_t size, const u64 val) +{ + int ret; + + mutex_lock(&st->lock); + /* + * Do not allow channel readings if device is in sleep state. + * A read/write on the spi/i2c bus would bring the device prematurely + * out of sleep. + */ + if (st->reset) { + mutex_unlock(&st->lock); + return -EPERM; + } + /* set device on correct page */ + ret = regmap_write(st->map, LTC2947_REG_PAGE_CTRL, page); + if (ret) { + mutex_unlock(&st->lock); + return ret; + } + + dev_dbg(st->dev, "Write val, r:%02X, p:%d, sz:%zu, val:%016llX\n", + reg, page, size, val); + + switch (size) { + case 2: + ret = __ltc2947_val_write16(st, reg, val); + break; + case 6: + ret = __ltc2947_val_write64(st, reg, val); + break; + default: + dev_err(st->dev, "Invalid size(%zu) to write", size); + ret = -EINVAL; + break; + } + + mutex_unlock(&st->lock); + + return ret; +} + +static int ltc2947_reset_history(struct ltc2947_data *st, const u8 reg_h, + const u8 reg_l) +{ + int ret; + /* + * let's reset the tracking register's. Tracking register's have all + * 2 bytes size + */ + ret = ltc2947_val_write(st, reg_h, PAGE0, 2, 0x8000U); + if (ret) + return ret; + + return ltc2947_val_write(st, reg_l, PAGE0, 2, 0x7FFFU); +} + +static int ltc2947_alarm_read(struct ltc2947_data *st, const u8 reg, + const u32 mask, long *val) +{ + u8 offset = reg - LTC2947_REG_STATUS; + /* +1 to include status reg */ + char alarms[LTC2947_ALERTS_SIZE + 1]; + int ret = 0; + + memset(alarms, 0, sizeof(alarms)); + + mutex_lock(&st->lock); + + if (st->reset) { + ret = -EPERM; + goto unlock; + } + + ret = regmap_write(st->map, LTC2947_REG_PAGE_CTRL, PAGE0); + if (ret) + goto unlock; + + dev_dbg(st->dev, "Read alarm, reg:%02X, mask:%02X\n", reg, mask); + /* + * As stated in the datasheet, when Threshold and Overflow registers + * are used, the status and all alert registers must be read in one + * multi-byte transaction. + */ + ret = regmap_bulk_read(st->map, LTC2947_REG_STATUS, alarms, + sizeof(alarms)); + if (ret) + goto unlock; + + /* get the alarm */ + *val = !!(alarms[offset] & mask); +unlock: + mutex_unlock(&st->lock); + return ret; +} + +static ssize_t ltc2947_set_value(struct device *dev, + struct device_attribute *da, + const char *buf, size_t count) +{ + struct ltc2947_data *st = dev_get_drvdata(dev); + struct sensor_device_attribute *attr = to_sensor_dev_attr(da); + int ret; + u8 reg, page = PAGE1; + s64 val = 0; + + ret = kstrtoll(buf, 10, &val); + if (ret) { + dev_err(st->dev, "Failed to convert the value\n"); + return ret; + } + + switch (attr->index) { + case LTC2947_POWER_THRE_H: + val = clamp_val(val, SHRT_MIN, SHRT_MAX); + ret = ltc2947_val_write(st, LTC2947_REG_POWER_THRE_H, PAGE1, 2, + div_s64(val, 200000)); + return ret ? ret : count; + case LTC2947_POWER_THRE_L: + val = clamp_val(val, SHRT_MIN, SHRT_MAX); + ret = ltc2947_val_write(st, LTC2947_REG_POWER_THRE_L, PAGE1, 2, + div_s64(val, 200000)); + return ret ? ret : count; + case LTC2947_ENERGY1_THRE_H: + reg = LTC2947_REG_ENERGY1_THRE_H; + break; + case LTC2947_ENERGY1_THRE_L: + reg = LTC2947_REG_ENERGY1_THRE_L; + break; + case LTC2947_ENERGY2_THRE_H: + reg = LTC2947_REG_ENERGY2_THRE_H; + break; + case LTC2947_ENERGY2_THRE_L: + reg = LTC2947_REG_ENERGY2_THRE_L; + break; + case LTC2947_ENERGY1_INPUT: + reg = LTC2947_REG_ENERGY1; + page = PAGE0; + break; + case LTC2947_ENERGY2_INPUT: + reg = LTC2947_REG_ENERGY2; + page = PAGE0; + break; + default: + return -ENOTSUPP; + } + + val = clamp_val(val, ENERGY_MIN, ENERGY_MAX); + /* we are losing the fractional part here... */ + val = div_s64(val * 1000, st->lsb_energy); + + ret = ltc2947_val_write(st, reg, page, 6, val); + + return ret ? ret : count; +} + +static ssize_t ltc2947_show_value(struct device *dev, + struct device_attribute *da, char *buf) +{ + struct ltc2947_data *st = dev_get_drvdata(dev); + struct sensor_device_attribute *attr = to_sensor_dev_attr(da); + int ret; + s64 val = 0; + + switch (attr->index) { + case LTC2947_POWER_INPUT: + ret = ltc2947_val_read(st, LTC2947_REG_POWER, PAGE0, 3, &val); + return ret ? ret : sprintf(buf, "%lld\n", val * 50000); + case LTC2947_POWER_THRE_H: + ret = ltc2947_val_read(st, LTC2947_REG_POWER_THRE_H, PAGE1, 2, + &val); + return ret ? ret : sprintf(buf, "%lld\n", val * 200000); + case LTC2947_POWER_THRE_L: + ret = ltc2947_val_read(st, LTC2947_REG_POWER_THRE_L, PAGE1, 2, + &val); + return ret ? ret : sprintf(buf, "%lld\n", val * 200000); + case LTC2947_POWER_HIGHEST: + ret = ltc2947_val_read(st, LTC2947_REG_POWER_MAX, PAGE0, 2, + &val); + return ret ? ret : sprintf(buf, "%lld\n", val * 200000); + case LTC2947_POWER_LOWEST: + ret = ltc2947_val_read(st, LTC2947_REG_POWER_MIN, PAGE0, 2, + &val); + return ret ? ret : sprintf(buf, "%lld\n", val * 200000); + case LTC2947_ENERGY1_THRE_H: + ret = ltc2947_val_read(st, LTC2947_REG_ENERGY1_THRE_H, PAGE1, 6, + &val); + break; + case LTC2947_ENERGY1_THRE_L: + ret = ltc2947_val_read(st, LTC2947_REG_ENERGY1_THRE_L, PAGE1, 6, + &val); + break; + case LTC2947_ENERGY2_THRE_H: + ret = ltc2947_val_read(st, LTC2947_REG_ENERGY2_THRE_H, PAGE1, 6, + &val); + break; + case LTC2947_ENERGY2_THRE_L: + ret = ltc2947_val_read(st, LTC2947_REG_ENERGY2_THRE_L, PAGE1, 6, + &val); + break; + case LTC2947_ENERGY1_INPUT: + ret = ltc2947_val_read(st, LTC2947_REG_ENERGY1, PAGE0, 6, &val); + break; + case LTC2947_ENERGY2_INPUT: + ret = ltc2947_val_read(st, LTC2947_REG_ENERGY2, PAGE0, 6, &val); + break; + default: + return -EINVAL; + } + + /* if we got here, must be an energy reading... */ + if (ret) + return ret; + + /* value in microJoule. st->lsb_energy was multiplied by 10E9 */ + val = div_s64(val * st->lsb_energy, 1000); + + return sprintf(buf, "%lld\n", val); +} + +static ssize_t ltc2947_show_alert(struct device *dev, + struct device_attribute *da, char *buf) +{ + struct ltc2947_data *st = dev_get_drvdata(dev); + struct sensor_device_attribute *attr = to_sensor_dev_attr(da); + long alert; + int ret = 0; + + switch (attr->index) { + case LTC2947_ENERGY1_MAX_ALARM: + ret = ltc2947_alarm_read(st, LTC2947_REG_STATE, + LTC2947_MAX_ENERGY1_MASK, &alert); + break; + case LTC2947_ENERGY1_MIN_ALARM: + ret = ltc2947_alarm_read(st, LTC2947_REG_STATE, + LTC2947_MIN_ENERGY1_MASK, &alert); + break; + case LTC2947_ENERGY2_MAX_ALARM: + ret = ltc2947_alarm_read(st, LTC2947_REG_STATE, + LTC2947_MAX_ENERGY2_MASK, &alert); + break; + case LTC2947_ENERGY2_MIN_ALARM: + ret = ltc2947_alarm_read(st, LTC2947_REG_STATE, + LTC2947_MIN_ENERGY2_MASK, &alert); + break; + case LTC2947_ENERGY1_OVERF_ALARM: + ret = ltc2947_alarm_read(st, LTC2947_REG_STATCEOF, + LTC2947_MIN_ENERGY1_O_MASK, &alert); + break; + case LTC2947_ENERGY2_OVERF_ALARM: + ret = ltc2947_alarm_read(st, LTC2947_REG_STATCEOF, + LTC2947_MIN_ENERGY2_O_MASK, &alert); + break; + case LTC2947_FAULT: + case LTC2947_ENERGY_FAULT: + ret = ltc2947_alarm_read(st, LTC2947_REG_STATUS, attr->index, + &alert); + break; + default: + return -EINVAL; + } + + return ret ? ret : sprintf(buf, "%li\n", alert); +} + +static int ltc2947_read_temp(struct device *dev, const u32 attr, long *val, + const int channel) +{ + int ret; + struct ltc2947_data *st = dev_get_drvdata(dev); + s64 __val = 0; + + if (channel < 0 || channel > LTC2947_TEMP_FAN_CHAN) { + dev_err(st->dev, "Invalid chan%d for temperature", channel); + return -EINVAL; + } + + switch (attr) { + case hwmon_temp_input: + ret = ltc2947_val_read(st, LTC2947_REG_TEMP, PAGE0, 2, &__val); + break; + case hwmon_temp_highest: + ret = ltc2947_val_read(st, LTC2947_REG_TEMP_MAX, PAGE0, 2, + &__val); + break; + case hwmon_temp_lowest: + ret = ltc2947_val_read(st, LTC2947_REG_TEMP_MIN, PAGE0, 2, + &__val); + break; + case hwmon_temp_max_alarm: + if (channel == LTC2947_TEMP_FAN_CHAN) + return ltc2947_alarm_read(st, LTC2947_REG_STATVT, + LTC2947_MAX_TEMP_FAN_MASK, + val); + else + return ltc2947_alarm_read(st, LTC2947_REG_STATVT, + LTC2947_MAX_TEMP_MASK, val); + case hwmon_temp_min_alarm: + if (channel == LTC2947_TEMP_FAN_CHAN) + return ltc2947_alarm_read(st, LTC2947_REG_STATVT, + LTC2947_MIN_TEMP_FAN_MASK, + val); + else + return ltc2947_alarm_read(st, LTC2947_REG_STATVT, + LTC2947_MIN_TEMP_MASK, val); + case hwmon_temp_max: + if (channel == LTC2947_TEMP_FAN_CHAN) + ret = ltc2947_val_read(st, LTC2947_REG_TEMP_FAN_THRE_H, + PAGE1, 2, &__val); + else + ret = ltc2947_val_read(st, LTC2947_REG_TEMP_THRE_H, + PAGE1, 2, &__val); + break; + case hwmon_temp_min: + if (channel == LTC2947_TEMP_FAN_CHAN) + ret = ltc2947_val_read(st, LTC2947_REG_TEMP_FAN_THRE_L, + PAGE1, 2, &__val); + else + ret = ltc2947_val_read(st, LTC2947_REG_TEMP_THRE_L, + PAGE1, 2, &__val); + break; + default: + return -ENOTSUPP; + } + + if (ret) + return ret; + + /* in milidegrees celcius, temp is given by: */ + *val = (__val * 204) + 550; + + return 0; +} + +static int ltc2947_read_power(struct device *dev, const u32 attr, long *val) +{ + struct ltc2947_data *st = dev_get_drvdata(dev); + + switch (attr) { + case hwmon_power_max_alarm: + return ltc2947_alarm_read(st, LTC2947_REG_STATIP, + LTC2947_MAX_POWER_MASK, val); + case hwmon_power_min_alarm: + return ltc2947_alarm_read(st, LTC2947_REG_STATIP, + LTC2947_MIN_POWER_MASK, val); + default: + return -ENOTSUPP; + } + + return 0; +} + +static int ltc2947_read_curr(struct device *dev, const u32 attr, long *val) +{ + struct ltc2947_data *st = dev_get_drvdata(dev); + int ret; + u8 lsb = 12; /* in mA */ + s64 __val = 0; + + switch (attr) { + case hwmon_curr_input: + ret = ltc2947_val_read(st, LTC2947_REG_CURRENT, PAGE0, 3, + &__val); + lsb = 3; + break; + case hwmon_curr_highest: + ret = ltc2947_val_read(st, LTC2947_REG_CURRENT_MAX, PAGE0, 2, + &__val); + break; + case hwmon_curr_lowest: + ret = ltc2947_val_read(st, LTC2947_REG_CURRENT_MIN, PAGE0, 2, + &__val); + break; + case hwmon_curr_max_alarm: + return ltc2947_alarm_read(st, LTC2947_REG_STATIP, + LTC2947_MAX_CURRENT_MASK, val); + case hwmon_curr_min_alarm: + return ltc2947_alarm_read(st, LTC2947_REG_STATIP, + LTC2947_MIN_CURRENT_MASK, val); + case hwmon_curr_max: + ret = ltc2947_val_read(st, LTC2947_REG_CURRENT_THRE_H, PAGE1, 2, + &__val); + break; + case hwmon_curr_min: + ret = ltc2947_val_read(st, LTC2947_REG_CURRENT_THRE_L, PAGE1, 2, + &__val); + break; + default: + return -ENOTSUPP; + } + + if (ret) + return ret; + + *val = __val * lsb; + + return 0; +} + +static int ltc2947_read_in(struct device *dev, const u32 attr, long *val, + const int channel) +{ + struct ltc2947_data *st = dev_get_drvdata(dev); + int ret; + u8 lsb = 2; /* in mV */ + s64 __val = 0; + + if (channel < 0 || channel > LTC2947_VOLTAGE_DVCC_CHAN) { + dev_err(st->dev, "Invalid chan%d for voltage", channel); + return -EINVAL; + } + + switch (attr) { + case hwmon_in_input: + if (channel == LTC2947_VOLTAGE_DVCC_CHAN) { + ret = ltc2947_val_read(st, LTC2947_REG_DVCC, PAGE0, 2, + &__val); + lsb = 145; + } else { + ret = ltc2947_val_read(st, LTC2947_REG_VOLTAGE, PAGE0, + 2, &__val); + } + break; + case hwmon_in_highest: + if (channel == LTC2947_VOLTAGE_DVCC_CHAN) { + ret = ltc2947_val_read(st, LTC2947_REG_DVCC_MAX, PAGE0, + 2, &__val); + lsb = 145; + } else { + ret = ltc2947_val_read(st, LTC2947_REG_VOLTAGE_MAX, + PAGE0, 2, &__val); + } + break; + case hwmon_in_lowest: + if (channel == LTC2947_VOLTAGE_DVCC_CHAN) { + ret = ltc2947_val_read(st, LTC2947_REG_DVCC_MIN, PAGE0, + 2, &__val); + lsb = 145; + } else { + ret = ltc2947_val_read(st, LTC2947_REG_VOLTAGE_MIN, + PAGE0, 2, &__val); + } + break; + case hwmon_in_max_alarm: + if (channel == LTC2947_VOLTAGE_DVCC_CHAN) + return ltc2947_alarm_read(st, LTC2947_REG_STATVDVCC, + LTC2947_MAX_VOLTAGE_MASK, + val); + else + return ltc2947_alarm_read(st, LTC2947_REG_STATVT, + LTC2947_MAX_VOLTAGE_MASK, + val); + case hwmon_in_min_alarm: + if (channel == LTC2947_VOLTAGE_DVCC_CHAN) + return ltc2947_alarm_read(st, LTC2947_REG_STATVDVCC, + LTC2947_MIN_VOLTAGE_MASK, + val); + else + return ltc2947_alarm_read(st, LTC2947_REG_STATVT, + LTC2947_MIN_VOLTAGE_MASK, + val); + case hwmon_in_max: + if (channel == LTC2947_VOLTAGE_DVCC_CHAN) { + ret = ltc2947_val_read(st, LTC2947_REG_DVCC_THRE_H, + PAGE1, 2, &__val); + lsb = 145; + } else { + ret = ltc2947_val_read(st, LTC2947_REG_VOLTAGE_THRE_H, + PAGE1, 2, &__val); + } + break; + case hwmon_in_min: + if (channel == LTC2947_VOLTAGE_DVCC_CHAN) { + ret = ltc2947_val_read(st, LTC2947_REG_DVCC_THRE_L, + PAGE1, 2, &__val); + lsb = 145; + } else { + ret = ltc2947_val_read(st, LTC2947_REG_VOLTAGE_THRE_L, + PAGE1, 2, &__val); + } + break; + default: + return -ENOTSUPP; + } + + if (ret) + return ret; + + *val = __val * lsb; + + return 0; +} + +static int ltc2947_read(struct device *dev, enum hwmon_sensor_types type, + u32 attr, int channel, long *val) +{ + switch (type) { + case hwmon_in: + return ltc2947_read_in(dev, attr, val, channel); + case hwmon_curr: + return ltc2947_read_curr(dev, attr, val); + case hwmon_power: + return ltc2947_read_power(dev, attr, val); + case hwmon_temp: + return ltc2947_read_temp(dev, attr, val, channel); + default: + return -ENOTSUPP; + } +} + +static int ltc2947_write_temp(struct device *dev, const u32 attr, + long val, const int channel) +{ + struct ltc2947_data *st = dev_get_drvdata(dev); + + if (channel < 0 || channel > LTC2947_TEMP_FAN_CHAN) { + dev_err(st->dev, "Invalid chan%d for temperature", channel); + return -EINVAL; + } + + switch (attr) { + case hwmon_temp_reset_history: + if (val != 1) + return -EINVAL; + return ltc2947_reset_history(st, LTC2947_REG_TEMP_MAX, + LTC2947_REG_TEMP_MIN); + case hwmon_temp_max: + val = clamp_val(val, SHRT_MIN, SHRT_MAX); + if (channel == LTC2947_TEMP_FAN_CHAN) { + if (!st->gpio_out) + return -ENOTSUPP; + + return ltc2947_val_write(st, + LTC2947_REG_TEMP_FAN_THRE_H, PAGE1, 2, + DIV_ROUND_CLOSEST(val - 550, 204)); + } else { + return ltc2947_val_write(st, LTC2947_REG_TEMP_THRE_H, + PAGE1, 2, + DIV_ROUND_CLOSEST(val - 550, 204)); + } + case hwmon_temp_min: + val = clamp_val(val, SHRT_MIN, SHRT_MAX); + if (channel == LTC2947_TEMP_FAN_CHAN) { + if (!st->gpio_out) + return -ENOTSUPP; + + return ltc2947_val_write(st, + LTC2947_REG_TEMP_FAN_THRE_L, PAGE1, 2, + DIV_ROUND_CLOSEST(val - 550, 204)); + } else { + return ltc2947_val_write(st, LTC2947_REG_TEMP_THRE_L, + PAGE1, 2, + DIV_ROUND_CLOSEST(val - 550, 204)); + } + default: + return -ENOTSUPP; + } +} + +static int ltc2947_write_power(struct device *dev, const u32 attr, + long val) +{ + struct ltc2947_data *st = dev_get_drvdata(dev); + + switch (attr) { + case hwmon_power_reset_history: + if (val != 1) + return -EINVAL; + return ltc2947_reset_history(st, LTC2947_REG_POWER_MAX, + LTC2947_REG_POWER_MIN); + default: + return -ENOTSUPP; + } +} + +static int ltc2947_write_curr(struct device *dev, const u32 attr, + long val) +{ + struct ltc2947_data *st = dev_get_drvdata(dev); + + switch (attr) { + case hwmon_curr_reset_history: + if (val != 1) + return -EINVAL; + return ltc2947_reset_history(st, LTC2947_REG_CURRENT_MAX, + LTC2947_REG_CURRENT_MIN); + case hwmon_curr_max: + val = clamp_val(val, SHRT_MIN, SHRT_MAX); + return ltc2947_val_write(st, LTC2947_REG_CURRENT_THRE_H, PAGE1, + 2, DIV_ROUND_CLOSEST(val, 12)); + case hwmon_curr_min: + val = clamp_val(val, SHRT_MIN, SHRT_MAX); + return ltc2947_val_write(st, LTC2947_REG_CURRENT_THRE_L, PAGE1, + 2, DIV_ROUND_CLOSEST(val, 12)); + default: + return -ENOTSUPP; + } +} + +static int ltc2947_write_in(struct device *dev, const u32 attr, long val, + const int channel) +{ + struct ltc2947_data *st = dev_get_drvdata(dev); + + if (channel > LTC2947_VOLTAGE_DVCC_CHAN) { + dev_err(st->dev, "Invalid chan%d for voltage", channel); + return -EINVAL; + } + + switch (attr) { + case hwmon_in_reset_history: + if (val != 1) + return -EINVAL; + + if (channel == LTC2947_VOLTAGE_DVCC_CHAN) + return ltc2947_reset_history(st, LTC2947_REG_DVCC_MAX, + LTC2947_REG_DVCC_MIN); + else + return ltc2947_reset_history(st, + LTC2947_REG_VOLTAGE_MAX, + LTC2947_REG_VOLTAGE_MIN); + case hwmon_in_max: + val = clamp_val(val, SHRT_MIN, SHRT_MAX); + + if (channel == LTC2947_VOLTAGE_DVCC_CHAN) + return ltc2947_val_write(st, LTC2947_REG_DVCC_THRE_H, + PAGE1, 2, + DIV_ROUND_CLOSEST(val, 145)); + else + return ltc2947_val_write(st, LTC2947_REG_VOLTAGE_THRE_H, + PAGE1, 2, + DIV_ROUND_CLOSEST(val, 2)); + case hwmon_in_min: + val = clamp_val(val, SHRT_MIN, SHRT_MAX); + + if (channel == LTC2947_VOLTAGE_DVCC_CHAN) + return ltc2947_val_write(st, LTC2947_REG_DVCC_THRE_L, + PAGE1, 2, + DIV_ROUND_CLOSEST(val, 145)); + else + return ltc2947_val_write(st, LTC2947_REG_VOLTAGE_THRE_L, + PAGE1, 2, + DIV_ROUND_CLOSEST(val, 2)); + default: + return -ENOTSUPP; + } +} + +static int ltc2947_write(struct device *dev, + enum hwmon_sensor_types type, + u32 attr, int channel, long val) +{ + switch (type) { + case hwmon_in: + return ltc2947_write_in(dev, attr, val, channel); + case hwmon_curr: + return ltc2947_write_curr(dev, attr, val); + case hwmon_power: + return ltc2947_write_power(dev, attr, val); + case hwmon_temp: + return ltc2947_write_temp(dev, attr, val, channel); + default: + return -ENOTSUPP; + } +} + +static int ltc2947_read_labels(struct device *dev, + enum hwmon_sensor_types type, + u32 attr, int channel, const char **str) +{ + switch (type) { + case hwmon_in: + if (channel == LTC2947_VOLTAGE_DVCC_CHAN) + *str = "DVCC"; + else + *str = "VP-VM"; + return 0; + case hwmon_curr: + *str = "IP-IM"; + return 0; + case hwmon_temp: + if (channel == LTC2947_TEMP_FAN_CHAN) + *str = "TEMPFAN"; + else + *str = "Ambient"; + return 0; + case hwmon_power: + *str = "Power"; + return 0; + default: + return -ENOTSUPP; + } +} + +static int ltc2947_in_is_visible(const u32 attr) +{ + switch (attr) { + case hwmon_in_input: + case hwmon_in_highest: + case hwmon_in_lowest: + case hwmon_in_max_alarm: + case hwmon_in_min_alarm: + case hwmon_in_label: + return 0444; + case hwmon_in_reset_history: + return 0200; + case hwmon_in_max: + case hwmon_in_min: + return 0644; + default: + return 0; + } +} + +static int ltc2947_curr_is_visible(const u32 attr) +{ + switch (attr) { + case hwmon_curr_input: + case hwmon_curr_highest: + case hwmon_curr_lowest: + case hwmon_curr_max_alarm: + case hwmon_curr_min_alarm: + case hwmon_curr_label: + return 0444; + case hwmon_curr_reset_history: + return 0200; + case hwmon_curr_max: + case hwmon_curr_min: + return 0644; + default: + return 0; + } +} + +static int ltc2947_power_is_visible(const u32 attr) +{ + switch (attr) { + case hwmon_power_label: + case hwmon_power_max_alarm: + case hwmon_power_min_alarm: + return 0444; + case hwmon_power_reset_history: + return 0200; + default: + return 0; + } +} + +static int ltc2947_temp_is_visible(const u32 attr) +{ + switch (attr) { + case hwmon_temp_input: + case hwmon_temp_highest: + case hwmon_temp_lowest: + case hwmon_temp_max_alarm: + case hwmon_temp_min_alarm: + case hwmon_temp_label: + return 0444; + case hwmon_temp_reset_history: + return 0200; + case hwmon_temp_max: + case hwmon_temp_min: + return 0644; + default: + return 0; + } +} + +static umode_t ltc2947_is_visible(const void *data, + enum hwmon_sensor_types type, + u32 attr, int channel) +{ + switch (type) { + case hwmon_in: + return ltc2947_in_is_visible(attr); + case hwmon_curr: + return ltc2947_curr_is_visible(attr); + case hwmon_power: + return ltc2947_power_is_visible(attr); + case hwmon_temp: + return ltc2947_temp_is_visible(attr); + default: + return 0; + } +} + +static const struct hwmon_channel_info *ltc2947_info[] = { + HWMON_CHANNEL_INFO(in, + HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_HIGHEST | + HWMON_I_MAX | HWMON_I_MIN | HWMON_I_RESET_HISTORY | + HWMON_I_MIN_ALARM | HWMON_I_MAX_ALARM | + HWMON_I_LABEL, + HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_HIGHEST | + HWMON_I_MAX | HWMON_I_MIN | HWMON_I_RESET_HISTORY | + HWMON_I_MIN_ALARM | HWMON_I_MAX_ALARM | + HWMON_I_LABEL), + HWMON_CHANNEL_INFO(curr, + HWMON_C_INPUT | HWMON_C_LOWEST | HWMON_C_HIGHEST | + HWMON_C_MAX | HWMON_C_MIN | HWMON_C_RESET_HISTORY | + HWMON_C_MIN_ALARM | HWMON_C_MAX_ALARM | + HWMON_C_LABEL), + HWMON_CHANNEL_INFO(power, + HWMON_P_RESET_HISTORY | HWMON_P_MAX_ALARM | + HWMON_P_MIN_ALARM | HWMON_P_LABEL), + HWMON_CHANNEL_INFO(temp, + HWMON_T_INPUT | HWMON_T_LOWEST | HWMON_T_HIGHEST | + HWMON_T_MAX | HWMON_T_MIN | HWMON_T_RESET_HISTORY | + HWMON_T_MIN_ALARM | HWMON_T_MAX_ALARM | + HWMON_T_LABEL, + HWMON_T_MAX_ALARM | HWMON_T_MIN_ALARM | HWMON_T_MAX | + HWMON_T_MIN | HWMON_T_LABEL), + NULL +}; + +static const struct hwmon_ops ltc2947_hwmon_ops = { + .is_visible = ltc2947_is_visible, + .read = ltc2947_read, + .write = ltc2947_write, + .read_string = ltc2947_read_labels, +}; + +static const struct hwmon_chip_info ltc2947_chip_info = { + .ops = <c2947_hwmon_ops, + .info = ltc2947_info, +}; + +/* power attributes - need u64 value */ +static SENSOR_DEVICE_ATTR(power1_input, 0444, ltc2947_show_value, + ltc2947_set_value, LTC2947_POWER_INPUT); +static SENSOR_DEVICE_ATTR(power1_max, 0644, ltc2947_show_value, + ltc2947_set_value, LTC2947_POWER_THRE_H); +static SENSOR_DEVICE_ATTR(power1_min, 0644, ltc2947_show_value, + ltc2947_set_value, LTC2947_POWER_THRE_L); +static SENSOR_DEVICE_ATTR(power1_input_highest, 0444, ltc2947_show_value, + ltc2947_set_value, LTC2947_POWER_HIGHEST); +static SENSOR_DEVICE_ATTR(power1_input_lowest, 0444, ltc2947_show_value, + ltc2947_set_value, LTC2947_POWER_LOWEST); +/* energy attributes */ +static SENSOR_DEVICE_ATTR(energy1_input, 0444, ltc2947_show_value, NULL, + LTC2947_ENERGY1_INPUT); +static SENSOR_DEVICE_ATTR(energy1_max, 0644, ltc2947_show_value, + ltc2947_set_value, LTC2947_ENERGY1_THRE_H); +static SENSOR_DEVICE_ATTR(energy1_min, 0644, ltc2947_show_value, + ltc2947_set_value, LTC2947_ENERGY1_THRE_L); +static SENSOR_DEVICE_ATTR(energy1_max_alarm, 0444, ltc2947_show_alert, NULL, + LTC2947_ENERGY1_MAX_ALARM); +static SENSOR_DEVICE_ATTR(energy1_min_alarm, 0444, ltc2947_show_alert, NULL, + LTC2947_ENERGY1_MIN_ALARM); +static SENSOR_DEVICE_ATTR(energy2_input, 0444, ltc2947_show_value, NULL, + LTC2947_ENERGY2_INPUT); +static SENSOR_DEVICE_ATTR(energy2_max, 0644, ltc2947_show_value, + ltc2947_set_value, LTC2947_ENERGY2_THRE_H); +static SENSOR_DEVICE_ATTR(energy2_min, 0644, ltc2947_show_value, + ltc2947_set_value, LTC2947_ENERGY2_THRE_L); +static SENSOR_DEVICE_ATTR(energy2_max_alarm, 0444, ltc2947_show_alert, NULL, + LTC2947_ENERGY2_MAX_ALARM); +static SENSOR_DEVICE_ATTR(energy2_min_alarm, 0444, ltc2947_show_alert, NULL, + LTC2947_ENERGY2_MIN_ALARM); +/* + * Overflow attributes indicate that the readings of the accumulated result + * registers are invalid. Furthermore it should indicate invalid TBCTL settings + */ +static SENSOR_DEVICE_ATTR(energy1_overflow_alarm, 0444, ltc2947_show_alert, + NULL, LTC2947_ENERGY1_OVERF_ALARM); +static SENSOR_DEVICE_ATTR(energy2_overflow_alarm, 0444, ltc2947_show_alert, + NULL, LTC2947_ENERGY2_OVERF_ALARM); + +/* + * Fault attributes indicate that the readings in the respective channel are + * not to be trusted + */ +static SENSOR_DEVICE_ATTR(energy1_fault, 0444, ltc2947_show_alert, + NULL, LTC2947_ENERGY_FAULT); +static SENSOR_DEVICE_ATTR(energy2_fault, 0444, ltc2947_show_alert, + NULL, LTC2947_ENERGY_FAULT); +static SENSOR_DEVICE_ATTR(in0_fault, 0444, ltc2947_show_alert, + NULL, LTC2947_FAULT); +static SENSOR_DEVICE_ATTR(in1_fault, 0444, ltc2947_show_alert, + NULL, LTC2947_FAULT); +static SENSOR_DEVICE_ATTR(curr1_fault, 0444, ltc2947_show_alert, + NULL, LTC2947_FAULT); +static SENSOR_DEVICE_ATTR(temp1_fault, 0444, ltc2947_show_alert, + NULL, LTC2947_FAULT); +static SENSOR_DEVICE_ATTR(power1_fault, 0444, ltc2947_show_alert, + NULL, LTC2947_FAULT); + +static struct attribute *ltc2947_attrs[] = { + &sensor_dev_attr_in0_fault.dev_attr.attr, + &sensor_dev_attr_in1_fault.dev_attr.attr, + &sensor_dev_attr_curr1_fault.dev_attr.attr, + &sensor_dev_attr_temp1_fault.dev_attr.attr, + &sensor_dev_attr_power1_input.dev_attr.attr, + &sensor_dev_attr_power1_max.dev_attr.attr, + &sensor_dev_attr_power1_min.dev_attr.attr, + &sensor_dev_attr_power1_input_highest.dev_attr.attr, + &sensor_dev_attr_power1_input_lowest.dev_attr.attr, + &sensor_dev_attr_power1_fault.dev_attr.attr, + &sensor_dev_attr_energy1_input.dev_attr.attr, + &sensor_dev_attr_energy1_max.dev_attr.attr, + &sensor_dev_attr_energy1_min.dev_attr.attr, + &sensor_dev_attr_energy1_max_alarm.dev_attr.attr, + &sensor_dev_attr_energy1_min_alarm.dev_attr.attr, + &sensor_dev_attr_energy2_input.dev_attr.attr, + &sensor_dev_attr_energy2_max.dev_attr.attr, + &sensor_dev_attr_energy2_min.dev_attr.attr, + &sensor_dev_attr_energy2_max_alarm.dev_attr.attr, + &sensor_dev_attr_energy2_min_alarm.dev_attr.attr, + &sensor_dev_attr_energy1_overflow_alarm.dev_attr.attr, + &sensor_dev_attr_energy2_overflow_alarm.dev_attr.attr, + &sensor_dev_attr_energy1_fault.dev_attr.attr, + &sensor_dev_attr_energy2_fault.dev_attr.attr, + NULL, +}; +ATTRIBUTE_GROUPS(ltc2947); + +static void ltc2947_clk_disable(void *data) +{ + struct clk *extclk = data; + + clk_disable_unprepare(extclk); +} + +static int ltc2947_setup(struct ltc2947_data *st) +{ + int ret; + struct clk *extclk; + u32 dummy, deadband, pol; + u32 accum[2]; + + /* clear status register by reading it */ + ret = regmap_read(st->map, LTC2947_REG_STATUS, &dummy); + if (ret) + return ret; + + /* check external clock presence */ + extclk = devm_clk_get(st->dev, NULL); + if (!IS_ERR(extclk)) { + unsigned long rate_hz; + u8 pre = 0, div, tbctl; + u64 aux; + + /* let's calculate and set the right valus in TBCTL */ + rate_hz = clk_get_rate(extclk); + if (rate_hz < LTC2947_CLK_MIN || rate_hz > LTC2947_CLK_MAX) { + dev_err(st->dev, "Invalid rate:%lu for external clock", + rate_hz); + return -EINVAL; + } + + ret = clk_prepare_enable(extclk); + if (ret) + return ret; + + ret = devm_add_action_or_reset(st->dev, ltc2947_clk_disable, + extclk); + if (ret) + return ret; + /* as in table 1 of the datasheet */ + if (rate_hz >= LTC2947_CLK_MIN && rate_hz <= 1000000) + pre = 0; + else if (rate_hz > 1000000 && rate_hz <= 2000000) + pre = 1; + else if (rate_hz > 2000000 && rate_hz <= 4000000) + pre = 2; + else if (rate_hz > 4000000 && rate_hz <= 8000000) + pre = 3; + else if (rate_hz > 8000000 && rate_hz <= 16000000) + pre = 4; + else if (rate_hz > 16000000 && rate_hz <= LTC2947_CLK_MAX) + pre = 5; + /* + * Div is given by: + * floor(fref / (2^PRE * 32768)) + */ + div = rate_hz / ((1 << pre) * 32768); + tbctl = LTC2947_PRE(pre) | LTC2947_DIV(div); + + ret = regmap_write(st->map, LTC2947_REG_TBCTL, tbctl); + if (ret) + return ret; + /* + * The energy lsb is given by (in W*s): + * 06416 * (1/fref) * 2^PRE * (DIV + 1) + * The value is multiplied by 10E9 + */ + aux = (div + 1) * ((1 << pre) * 641600000ULL); + st->lsb_energy = DIV_ROUND_CLOSEST_ULL(aux, rate_hz); + } else { + /* 19.89E-6 * 10E9 */ + st->lsb_energy = 19890; + } + ret = of_property_read_u32_array(st->dev->of_node, + "adi,accumulator-ctl-pol", accum, + ARRAY_SIZE(accum)); + if (!ret) { + u32 accum_reg = LTC2947_ACCUM_POL_1(accum[0]) | + LTC2947_ACCUM_POL_2(accum[1]); + + ret = regmap_write(st->map, LTC2947_REG_ACCUM_POL, accum_reg); + if (ret) + return ret; + } + ret = of_property_read_u32(st->dev->of_node, + "adi,accumulation-deadband-microamp", + &deadband); + if (!ret) { + /* the LSB is the same as the current, so 3mA */ + ret = regmap_write(st->map, LTC2947_REG_ACCUM_DEADBAND, + deadband / (1000 * 3)); + if (ret) + return ret; + } + /* check gpio cfg */ + ret = of_property_read_u32(st->dev->of_node, "adi,gpio-out-pol", &pol); + if (!ret) { + /* setup GPIO as output */ + u32 gpio_ctl = LTC2947_GPIO_EN(1) | LTC2947_GPIO_FAN_EN(1) | + LTC2947_GPIO_FAN_POL(pol); + + st->gpio_out = true; + ret = regmap_write(st->map, LTC2947_REG_GPIOSTATCTL, gpio_ctl); + if (ret) + return ret; + } + ret = of_property_read_u32_array(st->dev->of_node, "adi,gpio-in-accum", + accum, ARRAY_SIZE(accum)); + if (!ret) { + /* + * Setup the accum options. The gpioctl is already defined as + * input by default. + */ + u32 accum_val = LTC2947_ACCUM_POL_1(accum[0]) | + LTC2947_ACCUM_POL_2(accum[1]); + + if (st->gpio_out) { + dev_err(st->dev, + "Cannot have input gpio config if already configured as output"); + return -EINVAL; + } + + ret = regmap_write(st->map, LTC2947_REG_GPIO_ACCUM, accum_val); + if (ret) + return ret; + } + + /* set continuos mode */ + return regmap_update_bits(st->map, LTC2947_REG_CTRL, + LTC2947_CONT_MODE_MASK, LTC2947_CONT_MODE(1)); +} + +int ltc2947_core_probe(struct regmap *map, const char *name) +{ + struct ltc2947_data *st; + struct device *dev = regmap_get_device(map); + struct device *hwmon; + int ret; + + st = devm_kzalloc(dev, sizeof(*st), GFP_KERNEL); + if (!st) + return -ENOMEM; + + st->map = map; + st->dev = dev; + dev_set_drvdata(dev, st); + mutex_init(&st->lock); + + ret = ltc2947_setup(st); + if (ret) + return ret; + + hwmon = devm_hwmon_device_register_with_info(dev, name, st, + <c2947_chip_info, + ltc2947_groups); + return PTR_ERR_OR_ZERO(hwmon); +} +EXPORT_SYMBOL_GPL(ltc2947_core_probe); + +static int __maybe_unused ltc2947_resume(struct device *dev) +{ + struct ltc2947_data *st = dev_get_drvdata(dev); + u32 ctrl = 0; + int ret; + + mutex_lock(&st->lock); + /* dummy read to wake the device */ + ret = regmap_read(st->map, LTC2947_REG_CTRL, &ctrl); + if (ret) + goto unlock; + + /* + * Wait for the device. It takes 100ms to wake up so, 10ms extra + * should be enough. + */ + msleep(110); + ret = regmap_read(st->map, LTC2947_REG_CTRL, &ctrl); + if (ret) + goto unlock; + /* ctrl should be 0 */ + if (ctrl != 0) { + dev_err(st->dev, "Device failed to wake up, ctl:%02X\n", ctrl); + ret = -ETIMEDOUT; + goto unlock; + } + + st->reset = false; + /* set continuous mode */ + ret = regmap_update_bits(st->map, LTC2947_REG_CTRL, + LTC2947_CONT_MODE_MASK, LTC2947_CONT_MODE(1)); +unlock: + mutex_unlock(&st->lock); + return ret; +} + +static int __maybe_unused ltc2947_suspend(struct device *dev) +{ + struct ltc2947_data *st = dev_get_drvdata(dev); + int ret; + + mutex_lock(&st->lock); + ret = regmap_update_bits(st->map, LTC2947_REG_CTRL, + LTC2947_SHUTDOWN_MASK, 1); + if (ret) + goto unlock; + + st->reset = true; +unlock: + mutex_unlock(&st->lock); + return ret; +} + +SIMPLE_DEV_PM_OPS(ltc2947_pm_ops, ltc2947_suspend, ltc2947_resume); +EXPORT_SYMBOL_GPL(ltc2947_pm_ops); + +const struct of_device_id ltc2947_of_match[] = { + { .compatible = "adi,ltc2947" }, + {} +}; +EXPORT_SYMBOL_GPL(ltc2947_of_match); +MODULE_DEVICE_TABLE(of, ltc2947_of_match); + +MODULE_AUTHOR("Nuno Sa <nuno.sa@analog.com>"); +MODULE_DESCRIPTION("LTC2947 power and energy monitor core driver"); +MODULE_LICENSE("GPL"); diff --git a/drivers/hwmon/ltc2947-i2c.c b/drivers/hwmon/ltc2947-i2c.c new file mode 100644 index 000000000000..cf6074b110ae --- /dev/null +++ b/drivers/hwmon/ltc2947-i2c.c @@ -0,0 +1,49 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Analog Devices LTC2947 high precision power and energy monitor over I2C + * + * Copyright 2019 Analog Devices Inc. + */ +#include <linux/i2c.h> +#include <linux/module.h> +#include <linux/regmap.h> + +#include "ltc2947.h" + +static const struct regmap_config ltc2947_regmap_config = { + .reg_bits = 8, + .val_bits = 8, +}; + +static int ltc2947_probe(struct i2c_client *i2c, + const struct i2c_device_id *id) +{ + struct regmap *map; + + map = devm_regmap_init_i2c(i2c, <c2947_regmap_config); + if (IS_ERR(map)) + return PTR_ERR(map); + + return ltc2947_core_probe(map, i2c->name); +} + +static const struct i2c_device_id ltc2947_id[] = { + {"ltc2947", 0}, + {} +}; +MODULE_DEVICE_TABLE(i2c, ltc2947_id); + +static struct i2c_driver ltc2947_driver = { + .driver = { + .name = "ltc2947", + .of_match_table = ltc2947_of_match, + .pm = <c2947_pm_ops, + }, + .probe = ltc2947_probe, + .id_table = ltc2947_id, +}; +module_i2c_driver(ltc2947_driver); + +MODULE_AUTHOR("Nuno Sa <nuno.sa@analog.com>"); +MODULE_DESCRIPTION("LTC2947 I2C power and energy monitor driver"); +MODULE_LICENSE("GPL"); diff --git a/drivers/hwmon/ltc2947-spi.c b/drivers/hwmon/ltc2947-spi.c new file mode 100644 index 000000000000..c24ca569db1b --- /dev/null +++ b/drivers/hwmon/ltc2947-spi.c @@ -0,0 +1,50 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Analog Devices LTC2947 high precision power and energy monitor over SPI + * + * Copyright 2019 Analog Devices Inc. + */ +#include <linux/module.h> +#include <linux/of.h> +#include <linux/regmap.h> +#include <linux/spi/spi.h> + +#include "ltc2947.h" + +static const struct regmap_config ltc2947_regmap_config = { + .reg_bits = 16, + .val_bits = 8, + .read_flag_mask = BIT(0), +}; + +static int ltc2947_probe(struct spi_device *spi) +{ + struct regmap *map; + + map = devm_regmap_init_spi(spi, <c2947_regmap_config); + if (IS_ERR(map)) + return PTR_ERR(map); + + return ltc2947_core_probe(map, spi_get_device_id(spi)->name); +} + +static const struct spi_device_id ltc2947_id[] = { + {"ltc2947", 0}, + {} +}; +MODULE_DEVICE_TABLE(spi, ltc2947_id); + +static struct spi_driver ltc2947_driver = { + .driver = { + .name = "ltc2947", + .of_match_table = ltc2947_of_match, + .pm = <c2947_pm_ops, + }, + .probe = ltc2947_probe, + .id_table = ltc2947_id, +}; +module_spi_driver(ltc2947_driver); + +MODULE_AUTHOR("Nuno Sa <nuno.sa@analog.com>"); +MODULE_DESCRIPTION("LTC2947 SPI power and energy monitor driver"); +MODULE_LICENSE("GPL"); diff --git a/drivers/hwmon/ltc2947.h b/drivers/hwmon/ltc2947.h new file mode 100644 index 000000000000..5b8ff81a3dba --- /dev/null +++ b/drivers/hwmon/ltc2947.h @@ -0,0 +1,12 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _LINUX_LTC2947_H +#define _LINUX_LTC2947_H + +struct regmap; + +extern const struct of_device_id ltc2947_of_match[]; +extern const struct dev_pm_ops ltc2947_pm_ops; + +int ltc2947_core_probe(struct regmap *map, const char *name); + +#endif -- 2.23.0 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 2/3] hwmon: Add support for ltc2947 2019-09-24 12:49 ` [PATCH 2/3] hwmon: Add support for ltc2947 Nuno Sá @ 2019-10-03 4:14 ` Guenter Roeck 2019-10-04 7:45 ` Sa, Nuno 0 siblings, 1 reply; 21+ messages in thread From: Guenter Roeck @ 2019-10-03 4:14 UTC (permalink / raw) To: Nuno Sá Cc: linux-hwmon, devicetree, linux-doc, Rob Herring, Mark Rutland, Jean Delvare, Jonathan Corbet On Tue, Sep 24, 2019 at 02:49:44PM +0200, Nuno Sá wrote: > The ltc2947 is a high precision power and energy monitor with an > internal sense resistor supporting up to +/- 30A. Three internal no > Latency ADCs ensure accurate measurement of voltage and current, while > high-bandwidth analog multiplication of voltage and current provides > accurate power measurement in a wide range of applications. Internal or > external clocking options enable precise charge and energy measurements. > > Signed-off-by: Nuno Sá <nuno.sa@analog.com> > --- > Documentation/hwmon/ltc2947.rst | 110 +++ > MAINTAINERS | 10 + > drivers/hwmon/Kconfig | 27 + > drivers/hwmon/Makefile | 3 + > drivers/hwmon/ltc2947-core.c | 1421 +++++++++++++++++++++++++++++++ > drivers/hwmon/ltc2947-i2c.c | 49 ++ > drivers/hwmon/ltc2947-spi.c | 50 ++ > drivers/hwmon/ltc2947.h | 12 + > 8 files changed, 1682 insertions(+) > create mode 100644 Documentation/hwmon/ltc2947.rst > create mode 100644 drivers/hwmon/ltc2947-core.c > create mode 100644 drivers/hwmon/ltc2947-i2c.c > create mode 100644 drivers/hwmon/ltc2947-spi.c > create mode 100644 drivers/hwmon/ltc2947.h > > diff --git a/Documentation/hwmon/ltc2947.rst b/Documentation/hwmon/ltc2947.rst > new file mode 100644 > index 000000000000..2dcf7487076d > --- /dev/null > +++ b/Documentation/hwmon/ltc2947.rst > @@ -0,0 +1,110 @@ > +Kernel drivers ltc2947-i2c and ltc2947-spi > +========================================= > + > +Supported chips: > + * Analog Devices LTC2947 > + Prefix: 'ltc2947' > + Addresses scanned: - > + Datasheet: > + https://www.analog.com/media/en/technical-documentation/data-sheets/LTC2947.pdf > + > +Author: Nuno Sa <nuno.sa@analog.com> > + > +Description > +___________ > + > +The LTC2947 is a high precision power and energy monitor that measures current, > +voltage, power, temperature, charge and energy. The device supports both SPI > +and I2C depending on the chip configuration. > +The device also measures accumulated quantities as energy. It has two banks of > +register's to read/set energy related values. These banks can be configured > +independently to have setups like: energy1 accumulates always and enrgy2 only > +accumulates if current is positive (to check battery charging efficiency for > +example). The device also supports a GPIO pin that can be configured as output > +to control a fan as a function of measured temperature. Then, the GPIO becomes > +active as soon as a temperature reading is higher than a defined threshold. The > +temp2 channel is used to control this thresholds and to read the respective > +alarms. > + > +Sysfs entries > +_____________ > + > +The following attributes are supported. Limits are read-write, reset_history > +is write-only and all the other attributes are read-only. > + > +in0_input VP-VM voltage (mV). > +in0_min Undervoltage threshold > +in0_max Overvoltage threshold > +in0_lowest Lowest measured voltage > +in0_highest Highest measured voltage > +in0_reset_history Write 1 to reset in1 history > +in0_min_alarm Undervoltage alarm > +in0_max_alarm Overvoltage alarm > +in0_fault Fault value This is a non-standard attribute. What exactly does it reflect ? > +in0_label Channel label (VP-VM) > + > +in1_input DVCC voltage (mV) > +in1_min Undervoltage threshold > +in1_max Overvoltage threshold > +in1_lowest Lowest measured voltage > +in1_highest Highest measured voltage > +in1_reset_history Write 1 to reset in2 history > +in1_min_alarm Undervoltage alarm > +in1_max_alarm Overvoltage alarm > +in1_fault Fault value > +in1_label Channel label (DVCC) > + > +curr1_input IP-IM Sense current (mA) > +curr1_min Undercurrent threshold > +curr1_max Overcurrent threshold > +curr1_lowest Lowest measured current > +curr1_highest Highest measured current > +curr1_reset_history Write 1 to reset curr1 history > +curr1_min_alarm Undercurrent alarm > +curr1_max_alarm Overcurrent alarm > +curr1_fault Fault value > +curr1_label Channel label (IP-IM) > + > +power1_input Power (in uW) > +power1_min Low power threshold > +power1_max High power threshold > +power1_input_lowest Historical minimum power use > +power1_input_highest Historical maximum power use > +power1_reset_history Write 1 to reset power1 history > +power1_min_alarm Low power alarm > +power1_max_alarm High power alarm > +power1_fault Fault value > +power1_label Channel label (Power) > + > +temp1_input Chip Temperature (in milliC) > +temp1_min Low temperature threshold > +temp1_max High temperature threshold > +temp1_input_lowest Historical minimum temperature use > +temp1_input_highest Historical maximum temperature use > +temp1_reset_history Write 1 to reset temp1 history > +temp1_min_alarm Low temperature alarm > +temp1_max_alarm High temperature alarm > +temp1_fault Fault value > +temp1_label Channel label (Ambient) > + > +temp2_min Low temperature threshold for fan control > +temp2_max High temperature threshold for fan control > +temp2_min_alarm Low temperature fan control alarm > +temp2_max_alarm High temperature fan control alarm > +temp2_label Channel label (TEMPFAN) > + > +energy1_input Measured energy over time (in microJoule) > +energy1_max High energy threshold > +energy1_max_alarm High energy alarm > +energy1_min Low energy threshold > +energy1_min_alarm Low energy alarm > +energy1_overflow_alarm Energy1 register is about to overflow > +energy1_fault Fault value > + > +energy2_input Measured energy over time (in microJoule) > +energy2_max High energy threshold > +energy2_max_alarm High energy alarm > +energy2_min Low energy threshold > +energy2_min_alarm Low energy alarm > +energy2_overflow_alarm Energy2 register is about to overflow > +energy2_fault Fault value > diff --git a/MAINTAINERS b/MAINTAINERS > index c7035ce2460b..889f38c1c930 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -9496,6 +9496,16 @@ S: Maintained > F: Documentation/hwmon/ltc4261.rst > F: drivers/hwmon/ltc4261.c > > +LTC2947 HARDWARE MONITOR DRIVER > +M: Nuno Sá <nuno.sa@analog.com> > +W: http://ez.analog.com/community/linux-device-drivers > +L: linux-hwmon@vger.kernel.org > +S: Supported > +F: drivers/hwmon/ltc2947-core.c > +F: drivers/hwmon/ltc2947-spi.c > +F: drivers/hwmon/ltc2947-i2c.c > +F: drivers/hwmon/ltc2947.h > + > LTC4306 I2C MULTIPLEXER DRIVER > M: Michael Hennerich <michael.hennerich@analog.com> > W: http://ez.analog.com/community/linux-device-drivers > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig > index 2ca5668bdb62..ad35d625b6d3 100644 > --- a/drivers/hwmon/Kconfig > +++ b/drivers/hwmon/Kconfig > @@ -737,6 +737,33 @@ config SENSORS_LTC2945 > This driver can also be built as a module. If so, the module will > be called ltc2945. > > +config SENSORS_LTC2947 > + tristate > + > +config SENSORS_LTC2947_I2C > + tristate "Analog Devices LTC2947 High Precision Power and Energy Monitor over I2C" > + depends on I2C > + select REGMAP_I2C > + select SENSORS_LTC2947 > + help > + If you say yes here you get support for Linear Technology LTC2947 > + I2C High Precision Power and Energy Monitor > + > + This driver can also be built as a module. If so, the module will > + be called ltc2947-i2c. > + > +config SENSORS_LTC2947_SPI > + tristate "Analog Devices LTC2947 High Precision Power and Energy Monitor over SPI" > + depends on SPI_MASTER > + select REGMAP_SPI > + select SENSORS_LTC2947 > + help > + If you say yes here you get support for Linear Technology LTC2947 > + SPI High Precision Power and Energy Monitor > + > + This driver can also be built as a module. If so, the module will > + be called ltc2947-spi. > + > config SENSORS_LTC2990 > tristate "Linear Technology LTC2990" > depends on I2C > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile > index c86ce4d3d36b..b54a4cec3203 100644 > --- a/drivers/hwmon/Makefile > +++ b/drivers/hwmon/Makefile > @@ -107,6 +107,9 @@ obj-$(CONFIG_SENSORS_LM95234) += lm95234.o > obj-$(CONFIG_SENSORS_LM95241) += lm95241.o > obj-$(CONFIG_SENSORS_LM95245) += lm95245.o > obj-$(CONFIG_SENSORS_LTC2945) += ltc2945.o > +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_LTC4151) += ltc4151.o > obj-$(CONFIG_SENSORS_LTC4215) += ltc4215.o > diff --git a/drivers/hwmon/ltc2947-core.c b/drivers/hwmon/ltc2947-core.c > new file mode 100644 > index 000000000000..350b2b76467c > --- /dev/null > +++ b/drivers/hwmon/ltc2947-core.c > @@ -0,0 +1,1421 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Analog Devices LTC2947 high precision power and energy monitor > + * > + * Copyright 2019 Analog Devices Inc. > + */ > +#include <linux/bitfield.h> BIT() is defined in linux/bits.h. > +#include <linux/clk.h> > +#include <linux/device.h> > +#include <linux/hwmon.h> > +#include <linux/hwmon-sysfs.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/regmap.h> > + > +#include "ltc2947.h" > + > +/* register's */ > +#define LTC2947_REG_PAGE_CTRL 0xFF > +#define LTC2947_REG_CTRL 0xF0 > +#define LTC2947_REG_TBCTL 0xE9 > +#define LTC2947_CONT_MODE_MASK BIT(3) > +#define LTC2947_CONT_MODE(x) FIELD_PREP(LTC2947_CONT_MODE_MASK, x) > +#define LTC2947_PRE_MASK GENMASK(2, 0) > +#define LTC2947_PRE(x) FIELD_PREP(LTC2947_PRE_MASK, x) > +#define LTC2947_DIV_MASK GENMASK(7, 3) > +#define LTC2947_DIV(x) FIELD_PREP(LTC2947_DIV_MASK, x) > +#define LTC2947_SHUTDOWN_MASK BIT(0) > +#define LTC2947_REG_ACCUM_POL 0xE1 > +#define LTC2947_ACCUM_POL_1_MASK GENMASK(1, 0) > +#define LTC2947_ACCUM_POL_1(x) FIELD_PREP(LTC2947_ACCUM_POL_1_MASK, x) > +#define LTC2947_ACCUM_POL_2_MASK GENMASK(3, 2) > +#define LTC2947_ACCUM_POL_2(x) FIELD_PREP(LTC2947_ACCUM_POL_2_MASK, x) > +#define LTC2947_REG_ACCUM_DEADBAND 0xE4 > +#define LTC2947_REG_GPIOSTATCTL 0x67 > +#define LTC2947_GPIO_EN_MASK BIT(0) > +#define LTC2947_GPIO_EN(x) FIELD_PREP(LTC2947_GPIO_EN_MASK, x) > +#define LTC2947_GPIO_FAN_EN_MASK BIT(6) > +#define LTC2947_GPIO_FAN_EN(x) FIELD_PREP(LTC2947_GPIO_FAN_EN_MASK, x) > +#define LTC2947_GPIO_FAN_POL_MASK BIT(7) > +#define LTC2947_GPIO_FAN_POL(x) FIELD_PREP(LTC2947_GPIO_FAN_POL_MASK, x) > +#define LTC2947_REG_GPIO_ACCUM 0xE3 > +/* 200Khz */ > +#define LTC2947_CLK_MIN 200000 > +/* 25Mhz */ > +#define LTC2947_CLK_MAX 25000000 > +#define PAGE0 0 > +#define PAGE1 1 > +/* Voltage registers */ > +#define LTC2947_REG_VOLTAGE 0xA0 > +#define LTC2947_REG_VOLTAGE_MAX 0x50 > +#define LTC2947_REG_VOLTAGE_MIN 0x52 > +#define LTC2947_REG_VOLTAGE_THRE_H 0x90 > +#define LTC2947_REG_VOLTAGE_THRE_L 0x92 > +#define LTC2947_REG_DVCC 0xA4 > +#define LTC2947_REG_DVCC_MAX 0x58 > +#define LTC2947_REG_DVCC_MIN 0x5A > +#define LTC2947_REG_DVCC_THRE_H 0x98 > +#define LTC2947_REG_DVCC_THRE_L 0x9A > +#define LTC2947_VOLTAGE_GEN_CHAN 0 > +#define LTC2947_VOLTAGE_DVCC_CHAN 1 > +/* Current registers */ > +#define LTC2947_REG_CURRENT 0x90 > +#define LTC2947_REG_CURRENT_MAX 0x40 > +#define LTC2947_REG_CURRENT_MIN 0x42 > +#define LTC2947_REG_CURRENT_THRE_H 0x80 > +#define LTC2947_REG_CURRENT_THRE_L 0x82 > +/* Power registers */ > +#define LTC2947_REG_POWER 0x93 > +#define LTC2947_REG_POWER_MAX 0x44 > +#define LTC2947_REG_POWER_MIN 0x46 > +#define LTC2947_REG_POWER_THRE_H 0x84 > +#define LTC2947_REG_POWER_THRE_L 0x86 > +/* Temperature registers */ > +#define LTC2947_REG_TEMP 0xA2 > +#define LTC2947_REG_TEMP_MAX 0x54 > +#define LTC2947_REG_TEMP_MIN 0x56 > +#define LTC2947_REG_TEMP_THRE_H 0x94 > +#define LTC2947_REG_TEMP_THRE_L 0x96 > +#define LTC2947_REG_TEMP_FAN_THRE_H 0x9C > +#define LTC2947_REG_TEMP_FAN_THRE_L 0x9E > +#define LTC2947_TEMP_FAN_CHAN 1 > +/* Energy registers */ > +#define LTC2947_REG_ENERGY1 0x06 > +#define LTC2947_REG_ENERGY1_THRE_H 0x10 > +#define LTC2947_REG_ENERGY1_THRE_L 0x16 > +#define LTC2947_REG_ENERGY2 0x16 > +#define LTC2947_REG_ENERGY2_THRE_H 0x30 > +#define LTC2947_REG_ENERGY2_THRE_L 0x36 > +#define ENERGY_MIN 0xFFFF800000000000LL > +#define ENERGY_MAX 0x00007FFFFFFFFFFFLL > +/* Status/Alarm/Overflow registers */ > +#define LTC2947_REG_STATUS 0x80 > +#define LTC2947_REG_STATVT 0x81 > +#define LTC2947_REG_STATIP 0x82 > +#define LTC2947_REG_STATC 0x83 > +#define LTC2947_REG_STATE 0x84 > +#define LTC2947_REG_STATCEOF 0x85 > +#define LTC2947_REG_STATVDVCC 0x87 > + > +#define LTC2947_ALERTS_SIZE (LTC2947_REG_STATVDVCC - LTC2947_REG_STATUS) > +#define LTC2947_UPDATE_VAL_MASK BIT(4) > +#define LTC2947_MAX_VOLTAGE_MASK BIT(0) > +#define LTC2947_MIN_VOLTAGE_MASK BIT(1) > +#define LTC2947_MAX_CURRENT_MASK BIT(0) > +#define LTC2947_MIN_CURRENT_MASK BIT(1) > +#define LTC2947_MAX_POWER_MASK BIT(2) > +#define LTC2947_MIN_POWER_MASK BIT(3) > +#define LTC2947_MAX_TEMP_MASK BIT(2) > +#define LTC2947_MIN_TEMP_MASK BIT(3) > +#define LTC2947_MAX_TEMP_FAN_MASK BIT(4) > +#define LTC2947_MIN_TEMP_FAN_MASK BIT(5) > +#define LTC2947_SINGLE_SHOT_MASK BIT(2) > +#define LTC2947_MAX_ENERGY1_MASK BIT(0) > +#define LTC2947_MIN_ENERGY1_MASK BIT(1) > +#define LTC2947_MAX_ENERGY2_MASK BIT(2) > +#define LTC2947_MIN_ENERGY2_MASK BIT(3) > +#define LTC2947_MIN_ENERGY1_O_MASK BIT(4) > +#define LTC2947_MIN_ENERGY2_O_MASK BIT(5) > +#define LTC2947_ADCERR_MASK BIT(5) > +/* > + * For accumulated values there's a fault if the ADC conversions are invalid > + * (ADCERR) or if there is an overflow of the internal timebase register > + * (which indicates invalid TBCTL configuration). > + */ > +#define LTC2947_ENERGY_FAULT_MASK GENMASK(6, 5) > + > +struct ltc2947_data { > + struct regmap *map; > + struct device *dev; > + /* > + * The mutex is needed because the device has 2 memory pages. When > + * reading/writing the correct page needs to be set so that, the > + * complete sequence select_page->read/write needs to be protected. > + */ > + struct mutex lock; > + u32 lsb_energy; > + bool reset; > + bool gpio_out; > +}; > + > +/* used for raw sysfs entries */ > +enum { > + LTC2947_POWER_INPUT, > + LTC2947_POWER_THRE_L, > + LTC2947_POWER_THRE_H, > + LTC2947_POWER_HIGHEST, > + LTC2947_POWER_LOWEST, > + LTC2947_ENERGY1_INPUT, > + LTC2947_ENERGY1_THRE_H, > + LTC2947_ENERGY1_THRE_L, > + LTC2947_ENERGY1_MAX_ALARM, > + LTC2947_ENERGY1_MIN_ALARM, > + LTC2947_ENERGY2_INPUT, > + LTC2947_ENERGY2_THRE_H, > + LTC2947_ENERGY2_THRE_L, > + LTC2947_ENERGY2_MAX_ALARM, > + LTC2947_ENERGY2_MIN_ALARM, > + LTC2947_ENERGY1_OVERF_ALARM, > + LTC2947_ENERGY2_OVERF_ALARM, > + LTC2947_FAULT = LTC2947_ADCERR_MASK, > + LTC2947_ENERGY_FAULT = LTC2947_ENERGY_FAULT_MASK, > +}; > + > +static int __ltc2947_val_read16(const struct ltc2947_data *st, const u8 reg, > + u64 *val) > +{ > + __be16 __val = 0; > + int ret; > + > + ret = regmap_bulk_read(st->map, reg, &__val, 2); > + if (ret) > + return ret; > + > + *val = be16_to_cpu(__val); > + > + return 0; > +} > + > +static int __ltc2947_val_read24(const struct ltc2947_data *st, const u8 reg, > + u64 *val) > +{ > + __be32 __val = 0; > + int ret; > + > + ret = regmap_bulk_read(st->map, reg, &__val, 3); > + if (ret) > + return ret; > + > + *val = be32_to_cpu(__val) >> 8; > + > + return 0; > +} > + > +static int __ltc2947_val_read64(const struct ltc2947_data *st, const u8 reg, > + u64 *val) > +{ > + __be64 __val = 0; > + int ret; > + > + ret = regmap_bulk_read(st->map, reg, &__val, 6); > + if (ret) > + return ret; > + > + *val = be64_to_cpu(__val) >> 16; > + > + return 0; > +} > + > +static int ltc2947_val_read(struct ltc2947_data *st, const u8 reg, > + const u8 page, const size_t size, s64 *val) > +{ > + int ret; > + u64 __val = 0; > + > + mutex_lock(&st->lock); > + > + if (st->reset) { > + mutex_unlock(&st->lock); > + return -EPERM; Not sure what the error here should be, but EPERM is not correct. Under which conditions would this function be called while in suspend ? > + } > + > + ret = regmap_write(st->map, LTC2947_REG_PAGE_CTRL, page); > + if (ret) { > + mutex_unlock(&st->lock); > + return ret; > + } > + > + dev_dbg(st->dev, "Read val, reg:%02X, p:%d sz:%zu\n", reg, page, > + size); > + > + switch (size) { > + case 2: > + ret = __ltc2947_val_read16(st, reg, &__val); > + break; > + case 3: > + ret = __ltc2947_val_read24(st, reg, &__val); > + break; > + case 6: > + ret = __ltc2947_val_read64(st, reg, &__val); > + break; > + default: > + dev_err(st->dev, "Invalid size(%zu) to read", size); Following the code flow, this is dead code. An error message seems pointless. > + ret = -EINVAL; > + break; > + } > + > + mutex_unlock(&st->lock); > + > + if (ret) > + return ret; > + > + *val = sign_extend64(__val, (8 * size) - 1); > + > + dev_dbg(st->dev, "Got s:%lld, u:%016llX\n", *val, __val); > + > + return 0; > +} > + > +static int __ltc2947_val_write64(const struct ltc2947_data *st, const u8 reg, > + const u64 val) > +{ > + __be64 __val; > + > + __val = cpu_to_be64(val << 16); > + return regmap_bulk_write(st->map, reg, &__val, 6); > +} > + > +static int __ltc2947_val_write16(const struct ltc2947_data *st, const u8 reg, > + const u16 val) > +{ > + __be16 __val; > + > + __val = cpu_to_be16(val); > + return regmap_bulk_write(st->map, reg, &__val, 2); > +} > + > +static int ltc2947_val_write(struct ltc2947_data *st, const u8 reg, > + const u8 page, const size_t size, const u64 val) > +{ > + int ret; > + > + mutex_lock(&st->lock); > + /* > + * Do not allow channel readings if device is in sleep state. > + * A read/write on the spi/i2c bus would bring the device prematurely > + * out of sleep. > + */ > + if (st->reset) { > + mutex_unlock(&st->lock); > + return -EPERM; > + } > + /* set device on correct page */ > + ret = regmap_write(st->map, LTC2947_REG_PAGE_CTRL, page); > + if (ret) { > + mutex_unlock(&st->lock); > + return ret; > + } > + > + dev_dbg(st->dev, "Write val, r:%02X, p:%d, sz:%zu, val:%016llX\n", > + reg, page, size, val); > + > + switch (size) { > + case 2: > + ret = __ltc2947_val_write16(st, reg, val); > + break; > + case 6: > + ret = __ltc2947_val_write64(st, reg, val); > + break; > + default: > + dev_err(st->dev, "Invalid size(%zu) to write", size); > + ret = -EINVAL; > + break; > + } > + > + mutex_unlock(&st->lock); > + > + return ret; > +} > + > +static int ltc2947_reset_history(struct ltc2947_data *st, const u8 reg_h, > + const u8 reg_l) > +{ > + int ret; > + /* > + * let's reset the tracking register's. Tracking register's have all > + * 2 bytes size > + */ > + ret = ltc2947_val_write(st, reg_h, PAGE0, 2, 0x8000U); > + if (ret) > + return ret; > + > + return ltc2947_val_write(st, reg_l, PAGE0, 2, 0x7FFFU); > +} > + > +static int ltc2947_alarm_read(struct ltc2947_data *st, const u8 reg, > + const u32 mask, long *val) > +{ > + u8 offset = reg - LTC2947_REG_STATUS; > + /* +1 to include status reg */ > + char alarms[LTC2947_ALERTS_SIZE + 1]; > + int ret = 0; > + > + memset(alarms, 0, sizeof(alarms)); > + > + mutex_lock(&st->lock); > + > + if (st->reset) { > + ret = -EPERM; > + goto unlock; > + } > + > + ret = regmap_write(st->map, LTC2947_REG_PAGE_CTRL, PAGE0); > + if (ret) > + goto unlock; > + > + dev_dbg(st->dev, "Read alarm, reg:%02X, mask:%02X\n", reg, mask); > + /* > + * As stated in the datasheet, when Threshold and Overflow registers > + * are used, the status and all alert registers must be read in one > + * multi-byte transaction. > + */ > + ret = regmap_bulk_read(st->map, LTC2947_REG_STATUS, alarms, > + sizeof(alarms)); > + if (ret) > + goto unlock; > + > + /* get the alarm */ > + *val = !!(alarms[offset] & mask); > +unlock: > + mutex_unlock(&st->lock); > + return ret; > +} > + > +static ssize_t ltc2947_set_value(struct device *dev, > + struct device_attribute *da, > + const char *buf, size_t count) > +{ > + struct ltc2947_data *st = dev_get_drvdata(dev); > + struct sensor_device_attribute *attr = to_sensor_dev_attr(da); > + int ret; > + u8 reg, page = PAGE1; > + s64 val = 0; > + > + ret = kstrtoll(buf, 10, &val); > + if (ret) { > + dev_err(st->dev, "Failed to convert the value\n"); > + return ret; > + } > + > + switch (attr->index) { > + case LTC2947_POWER_THRE_H: > + val = clamp_val(val, SHRT_MIN, SHRT_MAX); > + ret = ltc2947_val_write(st, LTC2947_REG_POWER_THRE_H, PAGE1, 2, > + div_s64(val, 200000)); > + return ret ? ret : count; > + case LTC2947_POWER_THRE_L: > + val = clamp_val(val, SHRT_MIN, SHRT_MAX); > + ret = ltc2947_val_write(st, LTC2947_REG_POWER_THRE_L, PAGE1, 2, > + div_s64(val, 200000)); > + return ret ? ret : count; > + case LTC2947_ENERGY1_THRE_H: > + reg = LTC2947_REG_ENERGY1_THRE_H; > + break; > + case LTC2947_ENERGY1_THRE_L: > + reg = LTC2947_REG_ENERGY1_THRE_L; > + break; > + case LTC2947_ENERGY2_THRE_H: > + reg = LTC2947_REG_ENERGY2_THRE_H; > + break; > + case LTC2947_ENERGY2_THRE_L: > + reg = LTC2947_REG_ENERGY2_THRE_L; > + break; > + case LTC2947_ENERGY1_INPUT: > + reg = LTC2947_REG_ENERGY1; > + page = PAGE0; > + break; > + case LTC2947_ENERGY2_INPUT: > + reg = LTC2947_REG_ENERGY2; > + page = PAGE0; > + break; > + default: > + return -ENOTSUPP; > + } > + > + val = clamp_val(val, ENERGY_MIN, ENERGY_MAX); > + /* we are losing the fractional part here... */ > + val = div_s64(val * 1000, st->lsb_energy); > + > + ret = ltc2947_val_write(st, reg, page, 6, val); > + > + return ret ? ret : count; > +} > + > +static ssize_t ltc2947_show_value(struct device *dev, > + struct device_attribute *da, char *buf) > +{ > + struct ltc2947_data *st = dev_get_drvdata(dev); > + struct sensor_device_attribute *attr = to_sensor_dev_attr(da); > + int ret; > + s64 val = 0; > + > + switch (attr->index) { > + case LTC2947_POWER_INPUT: > + ret = ltc2947_val_read(st, LTC2947_REG_POWER, PAGE0, 3, &val); > + return ret ? ret : sprintf(buf, "%lld\n", val * 50000); > + case LTC2947_POWER_THRE_H: > + ret = ltc2947_val_read(st, LTC2947_REG_POWER_THRE_H, PAGE1, 2, > + &val); > + return ret ? ret : sprintf(buf, "%lld\n", val * 200000); > + case LTC2947_POWER_THRE_L: > + ret = ltc2947_val_read(st, LTC2947_REG_POWER_THRE_L, PAGE1, 2, > + &val); > + return ret ? ret : sprintf(buf, "%lld\n", val * 200000); > + case LTC2947_POWER_HIGHEST: > + ret = ltc2947_val_read(st, LTC2947_REG_POWER_MAX, PAGE0, 2, > + &val); > + return ret ? ret : sprintf(buf, "%lld\n", val * 200000); > + case LTC2947_POWER_LOWEST: > + ret = ltc2947_val_read(st, LTC2947_REG_POWER_MIN, PAGE0, 2, > + &val); > + return ret ? ret : sprintf(buf, "%lld\n", val * 200000); > + case LTC2947_ENERGY1_THRE_H: > + ret = ltc2947_val_read(st, LTC2947_REG_ENERGY1_THRE_H, PAGE1, 6, > + &val); > + break; > + case LTC2947_ENERGY1_THRE_L: > + ret = ltc2947_val_read(st, LTC2947_REG_ENERGY1_THRE_L, PAGE1, 6, > + &val); > + break; > + case LTC2947_ENERGY2_THRE_H: > + ret = ltc2947_val_read(st, LTC2947_REG_ENERGY2_THRE_H, PAGE1, 6, > + &val); > + break; > + case LTC2947_ENERGY2_THRE_L: > + ret = ltc2947_val_read(st, LTC2947_REG_ENERGY2_THRE_L, PAGE1, 6, > + &val); > + break; > + case LTC2947_ENERGY1_INPUT: > + ret = ltc2947_val_read(st, LTC2947_REG_ENERGY1, PAGE0, 6, &val); > + break; > + case LTC2947_ENERGY2_INPUT: > + ret = ltc2947_val_read(st, LTC2947_REG_ENERGY2, PAGE0, 6, &val); > + break; > + default: > + return -EINVAL; > + } > + > + /* if we got here, must be an energy reading... */ > + if (ret) > + return ret; > + > + /* value in microJoule. st->lsb_energy was multiplied by 10E9 */ > + val = div_s64(val * st->lsb_energy, 1000); > + > + return sprintf(buf, "%lld\n", val); > +} > + > +static ssize_t ltc2947_show_alert(struct device *dev, > + struct device_attribute *da, char *buf) > +{ > + struct ltc2947_data *st = dev_get_drvdata(dev); > + struct sensor_device_attribute *attr = to_sensor_dev_attr(da); > + long alert; > + int ret = 0; > + > + switch (attr->index) { > + case LTC2947_ENERGY1_MAX_ALARM: > + ret = ltc2947_alarm_read(st, LTC2947_REG_STATE, > + LTC2947_MAX_ENERGY1_MASK, &alert); > + break; > + case LTC2947_ENERGY1_MIN_ALARM: > + ret = ltc2947_alarm_read(st, LTC2947_REG_STATE, > + LTC2947_MIN_ENERGY1_MASK, &alert); > + break; > + case LTC2947_ENERGY2_MAX_ALARM: > + ret = ltc2947_alarm_read(st, LTC2947_REG_STATE, > + LTC2947_MAX_ENERGY2_MASK, &alert); > + break; > + case LTC2947_ENERGY2_MIN_ALARM: > + ret = ltc2947_alarm_read(st, LTC2947_REG_STATE, > + LTC2947_MIN_ENERGY2_MASK, &alert); > + break; > + case LTC2947_ENERGY1_OVERF_ALARM: > + ret = ltc2947_alarm_read(st, LTC2947_REG_STATCEOF, > + LTC2947_MIN_ENERGY1_O_MASK, &alert); > + break; > + case LTC2947_ENERGY2_OVERF_ALARM: > + ret = ltc2947_alarm_read(st, LTC2947_REG_STATCEOF, > + LTC2947_MIN_ENERGY2_O_MASK, &alert); > + break; > + case LTC2947_FAULT: > + case LTC2947_ENERGY_FAULT: > + ret = ltc2947_alarm_read(st, LTC2947_REG_STATUS, attr->index, > + &alert); > + break; > + default: > + return -EINVAL; > + } > + > + return ret ? ret : sprintf(buf, "%li\n", alert); > +} > + > +static int ltc2947_read_temp(struct device *dev, const u32 attr, long *val, > + const int channel) > +{ > + int ret; > + struct ltc2947_data *st = dev_get_drvdata(dev); > + s64 __val = 0; > + > + if (channel < 0 || channel > LTC2947_TEMP_FAN_CHAN) { > + dev_err(st->dev, "Invalid chan%d for temperature", channel); > + return -EINVAL; > + } > + > + switch (attr) { > + case hwmon_temp_input: > + ret = ltc2947_val_read(st, LTC2947_REG_TEMP, PAGE0, 2, &__val); > + break; > + case hwmon_temp_highest: > + ret = ltc2947_val_read(st, LTC2947_REG_TEMP_MAX, PAGE0, 2, > + &__val); > + break; > + case hwmon_temp_lowest: > + ret = ltc2947_val_read(st, LTC2947_REG_TEMP_MIN, PAGE0, 2, > + &__val); > + break; > + case hwmon_temp_max_alarm: > + if (channel == LTC2947_TEMP_FAN_CHAN) > + return ltc2947_alarm_read(st, LTC2947_REG_STATVT, > + LTC2947_MAX_TEMP_FAN_MASK, > + val); > + else > + return ltc2947_alarm_read(st, LTC2947_REG_STATVT, > + LTC2947_MAX_TEMP_MASK, val); > + case hwmon_temp_min_alarm: > + if (channel == LTC2947_TEMP_FAN_CHAN) > + return ltc2947_alarm_read(st, LTC2947_REG_STATVT, > + LTC2947_MIN_TEMP_FAN_MASK, > + val); > + else > + return ltc2947_alarm_read(st, LTC2947_REG_STATVT, > + LTC2947_MIN_TEMP_MASK, val); > + case hwmon_temp_max: > + if (channel == LTC2947_TEMP_FAN_CHAN) > + ret = ltc2947_val_read(st, LTC2947_REG_TEMP_FAN_THRE_H, > + PAGE1, 2, &__val); > + else > + ret = ltc2947_val_read(st, LTC2947_REG_TEMP_THRE_H, > + PAGE1, 2, &__val); > + break; > + case hwmon_temp_min: > + if (channel == LTC2947_TEMP_FAN_CHAN) > + ret = ltc2947_val_read(st, LTC2947_REG_TEMP_FAN_THRE_L, > + PAGE1, 2, &__val); > + else > + ret = ltc2947_val_read(st, LTC2947_REG_TEMP_THRE_L, > + PAGE1, 2, &__val); > + break; > + default: > + return -ENOTSUPP; > + } > + > + if (ret) > + return ret; > + > + /* in milidegrees celcius, temp is given by: */ > + *val = (__val * 204) + 550; > + > + return 0; > +} > + > +static int ltc2947_read_power(struct device *dev, const u32 attr, long *val) > +{ > + struct ltc2947_data *st = dev_get_drvdata(dev); > + > + switch (attr) { > + case hwmon_power_max_alarm: > + return ltc2947_alarm_read(st, LTC2947_REG_STATIP, > + LTC2947_MAX_POWER_MASK, val); > + case hwmon_power_min_alarm: > + return ltc2947_alarm_read(st, LTC2947_REG_STATIP, > + LTC2947_MIN_POWER_MASK, val); > + default: > + return -ENOTSUPP; > + } > + > + return 0; > +} > + > +static int ltc2947_read_curr(struct device *dev, const u32 attr, long *val) > +{ > + struct ltc2947_data *st = dev_get_drvdata(dev); > + int ret; > + u8 lsb = 12; /* in mA */ > + s64 __val = 0; > + > + switch (attr) { > + case hwmon_curr_input: > + ret = ltc2947_val_read(st, LTC2947_REG_CURRENT, PAGE0, 3, > + &__val); > + lsb = 3; > + break; > + case hwmon_curr_highest: > + ret = ltc2947_val_read(st, LTC2947_REG_CURRENT_MAX, PAGE0, 2, > + &__val); > + break; > + case hwmon_curr_lowest: > + ret = ltc2947_val_read(st, LTC2947_REG_CURRENT_MIN, PAGE0, 2, > + &__val); > + break; > + case hwmon_curr_max_alarm: > + return ltc2947_alarm_read(st, LTC2947_REG_STATIP, > + LTC2947_MAX_CURRENT_MASK, val); > + case hwmon_curr_min_alarm: > + return ltc2947_alarm_read(st, LTC2947_REG_STATIP, > + LTC2947_MIN_CURRENT_MASK, val); > + case hwmon_curr_max: > + ret = ltc2947_val_read(st, LTC2947_REG_CURRENT_THRE_H, PAGE1, 2, > + &__val); > + break; > + case hwmon_curr_min: > + ret = ltc2947_val_read(st, LTC2947_REG_CURRENT_THRE_L, PAGE1, 2, > + &__val); > + break; > + default: > + return -ENOTSUPP; > + } > + > + if (ret) > + return ret; > + > + *val = __val * lsb; > + > + return 0; > +} > + > +static int ltc2947_read_in(struct device *dev, const u32 attr, long *val, > + const int channel) > +{ > + struct ltc2947_data *st = dev_get_drvdata(dev); > + int ret; > + u8 lsb = 2; /* in mV */ > + s64 __val = 0; > + > + if (channel < 0 || channel > LTC2947_VOLTAGE_DVCC_CHAN) { > + dev_err(st->dev, "Invalid chan%d for voltage", channel); > + return -EINVAL; > + } > + > + switch (attr) { > + case hwmon_in_input: > + if (channel == LTC2947_VOLTAGE_DVCC_CHAN) { > + ret = ltc2947_val_read(st, LTC2947_REG_DVCC, PAGE0, 2, > + &__val); > + lsb = 145; > + } else { > + ret = ltc2947_val_read(st, LTC2947_REG_VOLTAGE, PAGE0, > + 2, &__val); > + } > + break; > + case hwmon_in_highest: > + if (channel == LTC2947_VOLTAGE_DVCC_CHAN) { > + ret = ltc2947_val_read(st, LTC2947_REG_DVCC_MAX, PAGE0, > + 2, &__val); > + lsb = 145; > + } else { > + ret = ltc2947_val_read(st, LTC2947_REG_VOLTAGE_MAX, > + PAGE0, 2, &__val); > + } > + break; > + case hwmon_in_lowest: > + if (channel == LTC2947_VOLTAGE_DVCC_CHAN) { > + ret = ltc2947_val_read(st, LTC2947_REG_DVCC_MIN, PAGE0, > + 2, &__val); > + lsb = 145; > + } else { > + ret = ltc2947_val_read(st, LTC2947_REG_VOLTAGE_MIN, > + PAGE0, 2, &__val); > + } > + break; > + case hwmon_in_max_alarm: > + if (channel == LTC2947_VOLTAGE_DVCC_CHAN) > + return ltc2947_alarm_read(st, LTC2947_REG_STATVDVCC, > + LTC2947_MAX_VOLTAGE_MASK, > + val); > + else > + return ltc2947_alarm_read(st, LTC2947_REG_STATVT, > + LTC2947_MAX_VOLTAGE_MASK, > + val); > + case hwmon_in_min_alarm: > + if (channel == LTC2947_VOLTAGE_DVCC_CHAN) > + return ltc2947_alarm_read(st, LTC2947_REG_STATVDVCC, > + LTC2947_MIN_VOLTAGE_MASK, > + val); > + else > + return ltc2947_alarm_read(st, LTC2947_REG_STATVT, > + LTC2947_MIN_VOLTAGE_MASK, > + val); > + case hwmon_in_max: > + if (channel == LTC2947_VOLTAGE_DVCC_CHAN) { > + ret = ltc2947_val_read(st, LTC2947_REG_DVCC_THRE_H, > + PAGE1, 2, &__val); > + lsb = 145; > + } else { > + ret = ltc2947_val_read(st, LTC2947_REG_VOLTAGE_THRE_H, > + PAGE1, 2, &__val); > + } > + break; > + case hwmon_in_min: > + if (channel == LTC2947_VOLTAGE_DVCC_CHAN) { > + ret = ltc2947_val_read(st, LTC2947_REG_DVCC_THRE_L, > + PAGE1, 2, &__val); > + lsb = 145; > + } else { > + ret = ltc2947_val_read(st, LTC2947_REG_VOLTAGE_THRE_L, > + PAGE1, 2, &__val); > + } > + break; > + default: > + return -ENOTSUPP; > + } > + > + if (ret) > + return ret; > + > + *val = __val * lsb; > + > + return 0; > +} > + > +static int ltc2947_read(struct device *dev, enum hwmon_sensor_types type, > + u32 attr, int channel, long *val) > +{ > + switch (type) { > + case hwmon_in: > + return ltc2947_read_in(dev, attr, val, channel); > + case hwmon_curr: > + return ltc2947_read_curr(dev, attr, val); > + case hwmon_power: > + return ltc2947_read_power(dev, attr, val); > + case hwmon_temp: > + return ltc2947_read_temp(dev, attr, val, channel); > + default: > + return -ENOTSUPP; > + } > +} > + > +static int ltc2947_write_temp(struct device *dev, const u32 attr, > + long val, const int channel) > +{ > + struct ltc2947_data *st = dev_get_drvdata(dev); > + > + if (channel < 0 || channel > LTC2947_TEMP_FAN_CHAN) { > + dev_err(st->dev, "Invalid chan%d for temperature", channel); > + return -EINVAL; > + } > + > + switch (attr) { > + case hwmon_temp_reset_history: > + if (val != 1) > + return -EINVAL; > + return ltc2947_reset_history(st, LTC2947_REG_TEMP_MAX, > + LTC2947_REG_TEMP_MIN); > + case hwmon_temp_max: > + val = clamp_val(val, SHRT_MIN, SHRT_MAX); > + if (channel == LTC2947_TEMP_FAN_CHAN) { > + if (!st->gpio_out) > + return -ENOTSUPP; > + > + return ltc2947_val_write(st, > + LTC2947_REG_TEMP_FAN_THRE_H, PAGE1, 2, > + DIV_ROUND_CLOSEST(val - 550, 204)); > + } else { > + return ltc2947_val_write(st, LTC2947_REG_TEMP_THRE_H, > + PAGE1, 2, > + DIV_ROUND_CLOSEST(val - 550, 204)); > + } > + case hwmon_temp_min: > + val = clamp_val(val, SHRT_MIN, SHRT_MAX); > + if (channel == LTC2947_TEMP_FAN_CHAN) { > + if (!st->gpio_out) > + return -ENOTSUPP; > + > + return ltc2947_val_write(st, > + LTC2947_REG_TEMP_FAN_THRE_L, PAGE1, 2, > + DIV_ROUND_CLOSEST(val - 550, 204)); > + } else { > + return ltc2947_val_write(st, LTC2947_REG_TEMP_THRE_L, > + PAGE1, 2, > + DIV_ROUND_CLOSEST(val - 550, 204)); > + } > + default: > + return -ENOTSUPP; > + } > +} > + > +static int ltc2947_write_power(struct device *dev, const u32 attr, > + long val) > +{ > + struct ltc2947_data *st = dev_get_drvdata(dev); > + > + switch (attr) { > + case hwmon_power_reset_history: > + if (val != 1) > + return -EINVAL; > + return ltc2947_reset_history(st, LTC2947_REG_POWER_MAX, > + LTC2947_REG_POWER_MIN); > + default: > + return -ENOTSUPP; > + } > +} > + > +static int ltc2947_write_curr(struct device *dev, const u32 attr, > + long val) > +{ > + struct ltc2947_data *st = dev_get_drvdata(dev); > + > + switch (attr) { > + case hwmon_curr_reset_history: > + if (val != 1) > + return -EINVAL; > + return ltc2947_reset_history(st, LTC2947_REG_CURRENT_MAX, > + LTC2947_REG_CURRENT_MIN); > + case hwmon_curr_max: > + val = clamp_val(val, SHRT_MIN, SHRT_MAX); > + return ltc2947_val_write(st, LTC2947_REG_CURRENT_THRE_H, PAGE1, > + 2, DIV_ROUND_CLOSEST(val, 12)); > + case hwmon_curr_min: > + val = clamp_val(val, SHRT_MIN, SHRT_MAX); > + return ltc2947_val_write(st, LTC2947_REG_CURRENT_THRE_L, PAGE1, > + 2, DIV_ROUND_CLOSEST(val, 12)); > + default: > + return -ENOTSUPP; > + } > +} > + > +static int ltc2947_write_in(struct device *dev, const u32 attr, long val, > + const int channel) > +{ > + struct ltc2947_data *st = dev_get_drvdata(dev); > + > + if (channel > LTC2947_VOLTAGE_DVCC_CHAN) { > + dev_err(st->dev, "Invalid chan%d for voltage", channel); > + return -EINVAL; > + } > + > + switch (attr) { > + case hwmon_in_reset_history: > + if (val != 1) > + return -EINVAL; > + > + if (channel == LTC2947_VOLTAGE_DVCC_CHAN) > + return ltc2947_reset_history(st, LTC2947_REG_DVCC_MAX, > + LTC2947_REG_DVCC_MIN); > + else > + return ltc2947_reset_history(st, > + LTC2947_REG_VOLTAGE_MAX, > + LTC2947_REG_VOLTAGE_MIN); > + case hwmon_in_max: > + val = clamp_val(val, SHRT_MIN, SHRT_MAX); > + > + if (channel == LTC2947_VOLTAGE_DVCC_CHAN) > + return ltc2947_val_write(st, LTC2947_REG_DVCC_THRE_H, > + PAGE1, 2, > + DIV_ROUND_CLOSEST(val, 145)); > + else > + return ltc2947_val_write(st, LTC2947_REG_VOLTAGE_THRE_H, > + PAGE1, 2, > + DIV_ROUND_CLOSEST(val, 2)); > + case hwmon_in_min: > + val = clamp_val(val, SHRT_MIN, SHRT_MAX); > + > + if (channel == LTC2947_VOLTAGE_DVCC_CHAN) > + return ltc2947_val_write(st, LTC2947_REG_DVCC_THRE_L, > + PAGE1, 2, > + DIV_ROUND_CLOSEST(val, 145)); > + else > + return ltc2947_val_write(st, LTC2947_REG_VOLTAGE_THRE_L, > + PAGE1, 2, > + DIV_ROUND_CLOSEST(val, 2)); > + default: > + return -ENOTSUPP; > + } > +} > + > +static int ltc2947_write(struct device *dev, > + enum hwmon_sensor_types type, > + u32 attr, int channel, long val) > +{ > + switch (type) { > + case hwmon_in: > + return ltc2947_write_in(dev, attr, val, channel); > + case hwmon_curr: > + return ltc2947_write_curr(dev, attr, val); > + case hwmon_power: > + return ltc2947_write_power(dev, attr, val); > + case hwmon_temp: > + return ltc2947_write_temp(dev, attr, val, channel); > + default: > + return -ENOTSUPP; > + } > +} > + > +static int ltc2947_read_labels(struct device *dev, > + enum hwmon_sensor_types type, > + u32 attr, int channel, const char **str) > +{ > + switch (type) { > + case hwmon_in: > + if (channel == LTC2947_VOLTAGE_DVCC_CHAN) > + *str = "DVCC"; > + else > + *str = "VP-VM"; > + return 0; > + case hwmon_curr: > + *str = "IP-IM"; > + return 0; > + case hwmon_temp: > + if (channel == LTC2947_TEMP_FAN_CHAN) > + *str = "TEMPFAN"; > + else > + *str = "Ambient"; > + return 0; > + case hwmon_power: > + *str = "Power"; > + return 0; > + default: > + return -ENOTSUPP; > + } > +} > + > +static int ltc2947_in_is_visible(const u32 attr) > +{ > + switch (attr) { > + case hwmon_in_input: > + case hwmon_in_highest: > + case hwmon_in_lowest: > + case hwmon_in_max_alarm: > + case hwmon_in_min_alarm: > + case hwmon_in_label: > + return 0444; > + case hwmon_in_reset_history: > + return 0200; > + case hwmon_in_max: > + case hwmon_in_min: > + return 0644; > + default: > + return 0; > + } > +} > + > +static int ltc2947_curr_is_visible(const u32 attr) > +{ > + switch (attr) { > + case hwmon_curr_input: > + case hwmon_curr_highest: > + case hwmon_curr_lowest: > + case hwmon_curr_max_alarm: > + case hwmon_curr_min_alarm: > + case hwmon_curr_label: > + return 0444; > + case hwmon_curr_reset_history: > + return 0200; > + case hwmon_curr_max: > + case hwmon_curr_min: > + return 0644; > + default: > + return 0; > + } > +} > + > +static int ltc2947_power_is_visible(const u32 attr) > +{ > + switch (attr) { > + case hwmon_power_label: > + case hwmon_power_max_alarm: > + case hwmon_power_min_alarm: > + return 0444; > + case hwmon_power_reset_history: > + return 0200; > + default: > + return 0; > + } > +} > + > +static int ltc2947_temp_is_visible(const u32 attr) > +{ > + switch (attr) { > + case hwmon_temp_input: > + case hwmon_temp_highest: > + case hwmon_temp_lowest: > + case hwmon_temp_max_alarm: > + case hwmon_temp_min_alarm: > + case hwmon_temp_label: > + return 0444; > + case hwmon_temp_reset_history: > + return 0200; > + case hwmon_temp_max: > + case hwmon_temp_min: > + return 0644; > + default: > + return 0; > + } > +} > + > +static umode_t ltc2947_is_visible(const void *data, > + enum hwmon_sensor_types type, > + u32 attr, int channel) > +{ > + switch (type) { > + case hwmon_in: > + return ltc2947_in_is_visible(attr); > + case hwmon_curr: > + return ltc2947_curr_is_visible(attr); > + case hwmon_power: > + return ltc2947_power_is_visible(attr); > + case hwmon_temp: > + return ltc2947_temp_is_visible(attr); > + default: > + return 0; > + } > +} > + > +static const struct hwmon_channel_info *ltc2947_info[] = { > + HWMON_CHANNEL_INFO(in, > + HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_HIGHEST | > + HWMON_I_MAX | HWMON_I_MIN | HWMON_I_RESET_HISTORY | > + HWMON_I_MIN_ALARM | HWMON_I_MAX_ALARM | > + HWMON_I_LABEL, > + HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_HIGHEST | > + HWMON_I_MAX | HWMON_I_MIN | HWMON_I_RESET_HISTORY | > + HWMON_I_MIN_ALARM | HWMON_I_MAX_ALARM | > + HWMON_I_LABEL), > + HWMON_CHANNEL_INFO(curr, > + HWMON_C_INPUT | HWMON_C_LOWEST | HWMON_C_HIGHEST | > + HWMON_C_MAX | HWMON_C_MIN | HWMON_C_RESET_HISTORY | > + HWMON_C_MIN_ALARM | HWMON_C_MAX_ALARM | > + HWMON_C_LABEL), > + HWMON_CHANNEL_INFO(power, > + HWMON_P_RESET_HISTORY | HWMON_P_MAX_ALARM | > + HWMON_P_MIN_ALARM | HWMON_P_LABEL), > + HWMON_CHANNEL_INFO(temp, > + HWMON_T_INPUT | HWMON_T_LOWEST | HWMON_T_HIGHEST | > + HWMON_T_MAX | HWMON_T_MIN | HWMON_T_RESET_HISTORY | > + HWMON_T_MIN_ALARM | HWMON_T_MAX_ALARM | > + HWMON_T_LABEL, > + HWMON_T_MAX_ALARM | HWMON_T_MIN_ALARM | HWMON_T_MAX | > + HWMON_T_MIN | HWMON_T_LABEL), > + NULL > +}; > + > +static const struct hwmon_ops ltc2947_hwmon_ops = { > + .is_visible = ltc2947_is_visible, > + .read = ltc2947_read, > + .write = ltc2947_write, > + .read_string = ltc2947_read_labels, > +}; > + > +static const struct hwmon_chip_info ltc2947_chip_info = { > + .ops = <c2947_hwmon_ops, > + .info = ltc2947_info, > +}; > + > +/* power attributes - need u64 value */ > +static SENSOR_DEVICE_ATTR(power1_input, 0444, ltc2947_show_value, > + ltc2947_set_value, LTC2947_POWER_INPUT); > +static SENSOR_DEVICE_ATTR(power1_max, 0644, ltc2947_show_value, > + ltc2947_set_value, LTC2947_POWER_THRE_H); > +static SENSOR_DEVICE_ATTR(power1_min, 0644, ltc2947_show_value, > + ltc2947_set_value, LTC2947_POWER_THRE_L); > +static SENSOR_DEVICE_ATTR(power1_input_highest, 0444, ltc2947_show_value, > + ltc2947_set_value, LTC2947_POWER_HIGHEST); > +static SENSOR_DEVICE_ATTR(power1_input_lowest, 0444, ltc2947_show_value, > + ltc2947_set_value, LTC2947_POWER_LOWEST); > +/* energy attributes */ > +static SENSOR_DEVICE_ATTR(energy1_input, 0444, ltc2947_show_value, NULL, > + LTC2947_ENERGY1_INPUT); > +static SENSOR_DEVICE_ATTR(energy1_max, 0644, ltc2947_show_value, > + ltc2947_set_value, LTC2947_ENERGY1_THRE_H); > +static SENSOR_DEVICE_ATTR(energy1_min, 0644, ltc2947_show_value, > + ltc2947_set_value, LTC2947_ENERGY1_THRE_L); > +static SENSOR_DEVICE_ATTR(energy1_max_alarm, 0444, ltc2947_show_alert, NULL, > + LTC2947_ENERGY1_MAX_ALARM); > +static SENSOR_DEVICE_ATTR(energy1_min_alarm, 0444, ltc2947_show_alert, NULL, > + LTC2947_ENERGY1_MIN_ALARM); > +static SENSOR_DEVICE_ATTR(energy2_input, 0444, ltc2947_show_value, NULL, > + LTC2947_ENERGY2_INPUT); > +static SENSOR_DEVICE_ATTR(energy2_max, 0644, ltc2947_show_value, > + ltc2947_set_value, LTC2947_ENERGY2_THRE_H); > +static SENSOR_DEVICE_ATTR(energy2_min, 0644, ltc2947_show_value, > + ltc2947_set_value, LTC2947_ENERGY2_THRE_L); > +static SENSOR_DEVICE_ATTR(energy2_max_alarm, 0444, ltc2947_show_alert, NULL, > + LTC2947_ENERGY2_MAX_ALARM); > +static SENSOR_DEVICE_ATTR(energy2_min_alarm, 0444, ltc2947_show_alert, NULL, > + LTC2947_ENERGY2_MIN_ALARM); > +/* > + * Overflow attributes indicate that the readings of the accumulated result > + * registers are invalid. Furthermore it should indicate invalid TBCTL settings > + */ > +static SENSOR_DEVICE_ATTR(energy1_overflow_alarm, 0444, ltc2947_show_alert, > + NULL, LTC2947_ENERGY1_OVERF_ALARM); > +static SENSOR_DEVICE_ATTR(energy2_overflow_alarm, 0444, ltc2947_show_alert, > + NULL, LTC2947_ENERGY2_OVERF_ALARM); > + > +/* > + * Fault attributes indicate that the readings in the respective channel are > + * not to be trusted > + */ > +static SENSOR_DEVICE_ATTR(energy1_fault, 0444, ltc2947_show_alert, > + NULL, LTC2947_ENERGY_FAULT); > +static SENSOR_DEVICE_ATTR(energy2_fault, 0444, ltc2947_show_alert, > + NULL, LTC2947_ENERGY_FAULT); > +static SENSOR_DEVICE_ATTR(in0_fault, 0444, ltc2947_show_alert, > + NULL, LTC2947_FAULT); > +static SENSOR_DEVICE_ATTR(in1_fault, 0444, ltc2947_show_alert, > + NULL, LTC2947_FAULT); > +static SENSOR_DEVICE_ATTR(curr1_fault, 0444, ltc2947_show_alert, > + NULL, LTC2947_FAULT); > +static SENSOR_DEVICE_ATTR(temp1_fault, 0444, ltc2947_show_alert, > + NULL, LTC2947_FAULT); > +static SENSOR_DEVICE_ATTR(power1_fault, 0444, ltc2947_show_alert, > + NULL, LTC2947_FAULT); > + > +static struct attribute *ltc2947_attrs[] = { > + &sensor_dev_attr_in0_fault.dev_attr.attr, > + &sensor_dev_attr_in1_fault.dev_attr.attr, > + &sensor_dev_attr_curr1_fault.dev_attr.attr, > + &sensor_dev_attr_temp1_fault.dev_attr.attr, > + &sensor_dev_attr_power1_input.dev_attr.attr, > + &sensor_dev_attr_power1_max.dev_attr.attr, > + &sensor_dev_attr_power1_min.dev_attr.attr, > + &sensor_dev_attr_power1_input_highest.dev_attr.attr, > + &sensor_dev_attr_power1_input_lowest.dev_attr.attr, > + &sensor_dev_attr_power1_fault.dev_attr.attr, > + &sensor_dev_attr_energy1_input.dev_attr.attr, > + &sensor_dev_attr_energy1_max.dev_attr.attr, > + &sensor_dev_attr_energy1_min.dev_attr.attr, > + &sensor_dev_attr_energy1_max_alarm.dev_attr.attr, > + &sensor_dev_attr_energy1_min_alarm.dev_attr.attr, > + &sensor_dev_attr_energy2_input.dev_attr.attr, > + &sensor_dev_attr_energy2_max.dev_attr.attr, > + &sensor_dev_attr_energy2_min.dev_attr.attr, > + &sensor_dev_attr_energy2_max_alarm.dev_attr.attr, > + &sensor_dev_attr_energy2_min_alarm.dev_attr.attr, > + &sensor_dev_attr_energy1_overflow_alarm.dev_attr.attr, > + &sensor_dev_attr_energy2_overflow_alarm.dev_attr.attr, > + &sensor_dev_attr_energy1_fault.dev_attr.attr, > + &sensor_dev_attr_energy2_fault.dev_attr.attr, Some of those are non-standard attributes. You'll have to explain each in detail, especially why it makes sense to provide such attributes to the user and why you can't use standard attributes instead. Also, for the _fault attributes, I don't entirely see the point. If the fault bit is set, ADC readings are not valid because supply voltage is low. This means that ADC register reads will be invalid. What is the point of having a non-standard attribute - which likely will be ignored - instead of returning an error when an attempt is made to read an ADC value ? Others, like energy1_input, or most of the power attributes, are standard attributes. Please explain the reasoning for not using the standard API for those. I am stoppping with the review at this point. We will have to discuss all non-standard attributes first. Guenter > + NULL, > +}; > +ATTRIBUTE_GROUPS(ltc2947); > + > +static void ltc2947_clk_disable(void *data) > +{ > + struct clk *extclk = data; > + > + clk_disable_unprepare(extclk); > +} > + > +static int ltc2947_setup(struct ltc2947_data *st) > +{ > + int ret; > + struct clk *extclk; > + u32 dummy, deadband, pol; > + u32 accum[2]; > + > + /* clear status register by reading it */ > + ret = regmap_read(st->map, LTC2947_REG_STATUS, &dummy); > + if (ret) > + return ret; > + > + /* check external clock presence */ > + extclk = devm_clk_get(st->dev, NULL); > + if (!IS_ERR(extclk)) { > + unsigned long rate_hz; > + u8 pre = 0, div, tbctl; > + u64 aux; > + > + /* let's calculate and set the right valus in TBCTL */ > + rate_hz = clk_get_rate(extclk); > + if (rate_hz < LTC2947_CLK_MIN || rate_hz > LTC2947_CLK_MAX) { > + dev_err(st->dev, "Invalid rate:%lu for external clock", > + rate_hz); > + return -EINVAL; > + } > + > + ret = clk_prepare_enable(extclk); > + if (ret) > + return ret; > + > + ret = devm_add_action_or_reset(st->dev, ltc2947_clk_disable, > + extclk); > + if (ret) > + return ret; > + /* as in table 1 of the datasheet */ > + if (rate_hz >= LTC2947_CLK_MIN && rate_hz <= 1000000) > + pre = 0; > + else if (rate_hz > 1000000 && rate_hz <= 2000000) > + pre = 1; > + else if (rate_hz > 2000000 && rate_hz <= 4000000) > + pre = 2; > + else if (rate_hz > 4000000 && rate_hz <= 8000000) > + pre = 3; > + else if (rate_hz > 8000000 && rate_hz <= 16000000) > + pre = 4; > + else if (rate_hz > 16000000 && rate_hz <= LTC2947_CLK_MAX) > + pre = 5; > + /* > + * Div is given by: > + * floor(fref / (2^PRE * 32768)) > + */ > + div = rate_hz / ((1 << pre) * 32768); > + tbctl = LTC2947_PRE(pre) | LTC2947_DIV(div); > + > + ret = regmap_write(st->map, LTC2947_REG_TBCTL, tbctl); > + if (ret) > + return ret; > + /* > + * The energy lsb is given by (in W*s): > + * 06416 * (1/fref) * 2^PRE * (DIV + 1) > + * The value is multiplied by 10E9 > + */ > + aux = (div + 1) * ((1 << pre) * 641600000ULL); > + st->lsb_energy = DIV_ROUND_CLOSEST_ULL(aux, rate_hz); > + } else { > + /* 19.89E-6 * 10E9 */ > + st->lsb_energy = 19890; > + } > + ret = of_property_read_u32_array(st->dev->of_node, > + "adi,accumulator-ctl-pol", accum, > + ARRAY_SIZE(accum)); > + if (!ret) { > + u32 accum_reg = LTC2947_ACCUM_POL_1(accum[0]) | > + LTC2947_ACCUM_POL_2(accum[1]); > + > + ret = regmap_write(st->map, LTC2947_REG_ACCUM_POL, accum_reg); > + if (ret) > + return ret; > + } > + ret = of_property_read_u32(st->dev->of_node, > + "adi,accumulation-deadband-microamp", > + &deadband); > + if (!ret) { > + /* the LSB is the same as the current, so 3mA */ > + ret = regmap_write(st->map, LTC2947_REG_ACCUM_DEADBAND, > + deadband / (1000 * 3)); > + if (ret) > + return ret; > + } > + /* check gpio cfg */ > + ret = of_property_read_u32(st->dev->of_node, "adi,gpio-out-pol", &pol); > + if (!ret) { > + /* setup GPIO as output */ > + u32 gpio_ctl = LTC2947_GPIO_EN(1) | LTC2947_GPIO_FAN_EN(1) | > + LTC2947_GPIO_FAN_POL(pol); > + > + st->gpio_out = true; > + ret = regmap_write(st->map, LTC2947_REG_GPIOSTATCTL, gpio_ctl); > + if (ret) > + return ret; > + } > + ret = of_property_read_u32_array(st->dev->of_node, "adi,gpio-in-accum", > + accum, ARRAY_SIZE(accum)); > + if (!ret) { > + /* > + * Setup the accum options. The gpioctl is already defined as > + * input by default. > + */ > + u32 accum_val = LTC2947_ACCUM_POL_1(accum[0]) | > + LTC2947_ACCUM_POL_2(accum[1]); > + > + if (st->gpio_out) { > + dev_err(st->dev, > + "Cannot have input gpio config if already configured as output"); > + return -EINVAL; > + } > + > + ret = regmap_write(st->map, LTC2947_REG_GPIO_ACCUM, accum_val); > + if (ret) > + return ret; > + } > + > + /* set continuos mode */ > + return regmap_update_bits(st->map, LTC2947_REG_CTRL, > + LTC2947_CONT_MODE_MASK, LTC2947_CONT_MODE(1)); > +} > + > +int ltc2947_core_probe(struct regmap *map, const char *name) > +{ > + struct ltc2947_data *st; > + struct device *dev = regmap_get_device(map); > + struct device *hwmon; > + int ret; > + > + st = devm_kzalloc(dev, sizeof(*st), GFP_KERNEL); > + if (!st) > + return -ENOMEM; > + > + st->map = map; > + st->dev = dev; > + dev_set_drvdata(dev, st); > + mutex_init(&st->lock); > + > + ret = ltc2947_setup(st); > + if (ret) > + return ret; > + > + hwmon = devm_hwmon_device_register_with_info(dev, name, st, > + <c2947_chip_info, > + ltc2947_groups); > + return PTR_ERR_OR_ZERO(hwmon); > +} > +EXPORT_SYMBOL_GPL(ltc2947_core_probe); > + > +static int __maybe_unused ltc2947_resume(struct device *dev) > +{ > + struct ltc2947_data *st = dev_get_drvdata(dev); > + u32 ctrl = 0; > + int ret; > + > + mutex_lock(&st->lock); > + /* dummy read to wake the device */ > + ret = regmap_read(st->map, LTC2947_REG_CTRL, &ctrl); > + if (ret) > + goto unlock; > + > + /* > + * Wait for the device. It takes 100ms to wake up so, 10ms extra > + * should be enough. > + */ > + msleep(110); > + ret = regmap_read(st->map, LTC2947_REG_CTRL, &ctrl); > + if (ret) > + goto unlock; > + /* ctrl should be 0 */ > + if (ctrl != 0) { > + dev_err(st->dev, "Device failed to wake up, ctl:%02X\n", ctrl); > + ret = -ETIMEDOUT; > + goto unlock; > + } > + > + st->reset = false; > + /* set continuous mode */ > + ret = regmap_update_bits(st->map, LTC2947_REG_CTRL, > + LTC2947_CONT_MODE_MASK, LTC2947_CONT_MODE(1)); > +unlock: > + mutex_unlock(&st->lock); > + return ret; > +} > + > +static int __maybe_unused ltc2947_suspend(struct device *dev) > +{ > + struct ltc2947_data *st = dev_get_drvdata(dev); > + int ret; > + > + mutex_lock(&st->lock); > + ret = regmap_update_bits(st->map, LTC2947_REG_CTRL, > + LTC2947_SHUTDOWN_MASK, 1); > + if (ret) > + goto unlock; > + > + st->reset = true; > +unlock: > + mutex_unlock(&st->lock); > + return ret; > +} > + > +SIMPLE_DEV_PM_OPS(ltc2947_pm_ops, ltc2947_suspend, ltc2947_resume); > +EXPORT_SYMBOL_GPL(ltc2947_pm_ops); > + > +const struct of_device_id ltc2947_of_match[] = { > + { .compatible = "adi,ltc2947" }, > + {} > +}; > +EXPORT_SYMBOL_GPL(ltc2947_of_match); > +MODULE_DEVICE_TABLE(of, ltc2947_of_match); > + > +MODULE_AUTHOR("Nuno Sa <nuno.sa@analog.com>"); > +MODULE_DESCRIPTION("LTC2947 power and energy monitor core driver"); > +MODULE_LICENSE("GPL"); > diff --git a/drivers/hwmon/ltc2947-i2c.c b/drivers/hwmon/ltc2947-i2c.c > new file mode 100644 > index 000000000000..cf6074b110ae > --- /dev/null > +++ b/drivers/hwmon/ltc2947-i2c.c > @@ -0,0 +1,49 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Analog Devices LTC2947 high precision power and energy monitor over I2C > + * > + * Copyright 2019 Analog Devices Inc. > + */ > +#include <linux/i2c.h> > +#include <linux/module.h> > +#include <linux/regmap.h> > + > +#include "ltc2947.h" > + > +static const struct regmap_config ltc2947_regmap_config = { > + .reg_bits = 8, > + .val_bits = 8, > +}; > + > +static int ltc2947_probe(struct i2c_client *i2c, > + const struct i2c_device_id *id) > +{ > + struct regmap *map; > + > + map = devm_regmap_init_i2c(i2c, <c2947_regmap_config); > + if (IS_ERR(map)) > + return PTR_ERR(map); > + > + return ltc2947_core_probe(map, i2c->name); > +} > + > +static const struct i2c_device_id ltc2947_id[] = { > + {"ltc2947", 0}, > + {} > +}; > +MODULE_DEVICE_TABLE(i2c, ltc2947_id); > + > +static struct i2c_driver ltc2947_driver = { > + .driver = { > + .name = "ltc2947", > + .of_match_table = ltc2947_of_match, > + .pm = <c2947_pm_ops, > + }, > + .probe = ltc2947_probe, > + .id_table = ltc2947_id, > +}; > +module_i2c_driver(ltc2947_driver); > + > +MODULE_AUTHOR("Nuno Sa <nuno.sa@analog.com>"); > +MODULE_DESCRIPTION("LTC2947 I2C power and energy monitor driver"); > +MODULE_LICENSE("GPL"); > diff --git a/drivers/hwmon/ltc2947-spi.c b/drivers/hwmon/ltc2947-spi.c > new file mode 100644 > index 000000000000..c24ca569db1b > --- /dev/null > +++ b/drivers/hwmon/ltc2947-spi.c > @@ -0,0 +1,50 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Analog Devices LTC2947 high precision power and energy monitor over SPI > + * > + * Copyright 2019 Analog Devices Inc. > + */ > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/regmap.h> > +#include <linux/spi/spi.h> > + > +#include "ltc2947.h" > + > +static const struct regmap_config ltc2947_regmap_config = { > + .reg_bits = 16, > + .val_bits = 8, > + .read_flag_mask = BIT(0), > +}; > + > +static int ltc2947_probe(struct spi_device *spi) > +{ > + struct regmap *map; > + > + map = devm_regmap_init_spi(spi, <c2947_regmap_config); > + if (IS_ERR(map)) > + return PTR_ERR(map); > + > + return ltc2947_core_probe(map, spi_get_device_id(spi)->name); > +} > + > +static const struct spi_device_id ltc2947_id[] = { > + {"ltc2947", 0}, > + {} > +}; > +MODULE_DEVICE_TABLE(spi, ltc2947_id); > + > +static struct spi_driver ltc2947_driver = { > + .driver = { > + .name = "ltc2947", > + .of_match_table = ltc2947_of_match, > + .pm = <c2947_pm_ops, > + }, > + .probe = ltc2947_probe, > + .id_table = ltc2947_id, > +}; > +module_spi_driver(ltc2947_driver); > + > +MODULE_AUTHOR("Nuno Sa <nuno.sa@analog.com>"); > +MODULE_DESCRIPTION("LTC2947 SPI power and energy monitor driver"); > +MODULE_LICENSE("GPL"); > diff --git a/drivers/hwmon/ltc2947.h b/drivers/hwmon/ltc2947.h > new file mode 100644 > index 000000000000..5b8ff81a3dba > --- /dev/null > +++ b/drivers/hwmon/ltc2947.h > @@ -0,0 +1,12 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef _LINUX_LTC2947_H > +#define _LINUX_LTC2947_H > + > +struct regmap; > + > +extern const struct of_device_id ltc2947_of_match[]; > +extern const struct dev_pm_ops ltc2947_pm_ops; > + > +int ltc2947_core_probe(struct regmap *map, const char *name); > + > +#endif ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/3] hwmon: Add support for ltc2947 2019-10-03 4:14 ` Guenter Roeck @ 2019-10-04 7:45 ` Sa, Nuno 2019-10-04 15:06 ` Guenter Roeck 0 siblings, 1 reply; 21+ messages in thread From: Sa, Nuno @ 2019-10-04 7:45 UTC (permalink / raw) To: linux Cc: corbet, linux-hwmon, devicetree, linux-doc, mark.rutland, robh+dt, jdelvare On Wed, 2019-10-02 at 21:14 -0700, Guenter Roeck wrote: > > On Tue, Sep 24, 2019 at 02:49:44PM +0200, Nuno Sá wrote: > > The ltc2947 is a high precision power and energy monitor with an > > internal sense resistor supporting up to +/- 30A. Three internal no > > Latency ADCs ensure accurate measurement of voltage and current, > > while > > high-bandwidth analog multiplication of voltage and current > > provides > > accurate power measurement in a wide range of applications. > > Internal or > > external clocking options enable precise charge and energy > > measurements. > > > > Signed-off-by: Nuno Sá <nuno.sa@analog.com> > > --- > > Documentation/hwmon/ltc2947.rst | 110 +++ > > MAINTAINERS | 10 + > > drivers/hwmon/Kconfig | 27 + > > drivers/hwmon/Makefile | 3 + > > drivers/hwmon/ltc2947-core.c | 1421 > > +++++++++++++++++++++++++++++++ > > drivers/hwmon/ltc2947-i2c.c | 49 ++ > > drivers/hwmon/ltc2947-spi.c | 50 ++ > > drivers/hwmon/ltc2947.h | 12 + > > 8 files changed, 1682 insertions(+) > > create mode 100644 Documentation/hwmon/ltc2947.rst > > create mode 100644 drivers/hwmon/ltc2947-core.c > > create mode 100644 drivers/hwmon/ltc2947-i2c.c > > create mode 100644 drivers/hwmon/ltc2947-spi.c > > create mode 100644 drivers/hwmon/ltc2947.h > > > > diff --git a/Documentation/hwmon/ltc2947.rst > > b/Documentation/hwmon/ltc2947.rst > > new file mode 100644 > > index 000000000000..2dcf7487076d > > --- /dev/null > > +++ b/Documentation/hwmon/ltc2947.rst > > @@ -0,0 +1,110 @@ > > +Kernel drivers ltc2947-i2c and ltc2947-spi > > +========================================= > > + > > +Supported chips: > > + * Analog Devices LTC2947 > > + Prefix: 'ltc2947' > > + Addresses scanned: - > > + Datasheet: > > + > > https://www.analog.com/media/en/technical-documentation/data-sheets/LTC2947.pdf > > + > > +Author: Nuno Sa <nuno.sa@analog.com> > > + > > +Description > > +___________ > > + > > +The LTC2947 is a high precision power and energy monitor that > > measures current, > > +voltage, power, temperature, charge and energy. The device > > supports both SPI > > +and I2C depending on the chip configuration. > > +The device also measures accumulated quantities as energy. It has > > two banks of > > +register's to read/set energy related values. These banks can be > > configured > > +independently to have setups like: energy1 accumulates always and > > enrgy2 only > > +accumulates if current is positive (to check battery charging > > efficiency for > > +example). The device also supports a GPIO pin that can be > > configured as output > > +to control a fan as a function of measured temperature. Then, the > > GPIO becomes > > +active as soon as a temperature reading is higher than a defined > > threshold. The > > +temp2 channel is used to control this thresholds and to read the > > respective > > +alarms. > > + > > +Sysfs entries > > +_____________ > > + > > +The following attributes are supported. Limits are read-write, > > reset_history > > +is write-only and all the other attributes are read-only. > > + > > +in0_input VP-VM voltage (mV). > > +in0_min Undervoltage threshold > > +in0_max Overvoltage threshold > > +in0_lowest Lowest measured voltage > > +in0_highest Highest measured voltage > > +in0_reset_history Write 1 to reset in1 history > > +in0_min_alarm Undervoltage alarm > > +in0_max_alarm Overvoltage alarm > > +in0_fault Fault value > > This is a non-standard attribute. What exactly does it reflect ? > > +in0_label Channel label (VP-VM) > > + > > +in1_input DVCC voltage (mV) > > +in1_min Undervoltage threshold > > +in1_max Overvoltage threshold > > +in1_lowest Lowest measured voltage > > +in1_highest Highest measured voltage > > +in1_reset_history Write 1 to reset in2 history > > +in1_min_alarm Undervoltage alarm > > +in1_max_alarm Overvoltage alarm > > +in1_fault Fault value > > +in1_label Channel label (DVCC) > > + > > +curr1_input IP-IM Sense current (mA) > > +curr1_min Undercurrent threshold > > +curr1_max Overcurrent threshold > > +curr1_lowest Lowest measured current > > +curr1_highest Highest measured current > > +curr1_reset_history Write 1 to reset curr1 history > > +curr1_min_alarm Undercurrent alarm > > +curr1_max_alarm Overcurrent alarm > > +curr1_fault Fault value > > +curr1_label Channel label (IP-IM) > > + > > +power1_input Power (in uW) > > +power1_min Low power threshold > > +power1_max High power threshold > > +power1_input_lowest Historical minimum power use > > +power1_input_highest Historical maximum power use > > +power1_reset_history Write 1 to reset power1 history > > +power1_min_alarm Low power alarm > > +power1_max_alarm High power alarm > > +power1_fault Fault value > > +power1_label Channel label (Power) > > + > > +temp1_input Chip Temperature (in milliC) > > +temp1_min Low temperature threshold > > +temp1_max High temperature threshold > > +temp1_input_lowest Historical minimum temperature use > > +temp1_input_highest Historical maximum temperature use > > +temp1_reset_history Write 1 to reset temp1 history > > +temp1_min_alarm Low temperature alarm > > +temp1_max_alarm High temperature alarm > > +temp1_fault Fault value > > +temp1_label Channel label (Ambient) > > + > > +temp2_min Low temperature threshold for fan control > > +temp2_max High temperature threshold for fan control > > +temp2_min_alarm Low temperature fan control alarm > > +temp2_max_alarm High temperature fan control alarm > > +temp2_label Channel label (TEMPFAN) > > + > > +energy1_input Measured energy over time (in > > microJoule) > > +energy1_max High energy threshold > > +energy1_max_alarm High energy alarm > > +energy1_min Low energy threshold > > +energy1_min_alarm Low energy alarm > > +energy1_overflow_alarm Energy1 register is about to overflow > > +energy1_fault Fault value > > + > > +energy2_input Measured energy over time (in > > microJoule) > > +energy2_max High energy threshold > > +energy2_max_alarm High energy alarm > > +energy2_min Low energy threshold > > +energy2_min_alarm Low energy alarm > > +energy2_overflow_alarm Energy2 register is about to overflow > > +energy2_fault Fault value > > diff --git a/MAINTAINERS b/MAINTAINERS > > index c7035ce2460b..889f38c1c930 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -9496,6 +9496,16 @@ S: Maintained > > F: Documentation/hwmon/ltc4261.rst > > F: drivers/hwmon/ltc4261.c > > > > +LTC2947 HARDWARE MONITOR DRIVER > > +M: Nuno Sá <nuno.sa@analog.com> > > +W: http://ez.analog.com/community/linux-device-drivers > > +L: linux-hwmon@vger.kernel.org > > +S: Supported > > +F: drivers/hwmon/ltc2947-core.c > > +F: drivers/hwmon/ltc2947-spi.c > > +F: drivers/hwmon/ltc2947-i2c.c > > +F: drivers/hwmon/ltc2947.h > > + > > LTC4306 I2C MULTIPLEXER DRIVER > > M: Michael Hennerich <michael.hennerich@analog.com> > > W: http://ez.analog.com/community/linux-device-drivers > > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig > > index 2ca5668bdb62..ad35d625b6d3 100644 > > --- a/drivers/hwmon/Kconfig > > +++ b/drivers/hwmon/Kconfig > > @@ -737,6 +737,33 @@ config SENSORS_LTC2945 > > This driver can also be built as a module. If so, the module > > will > > be called ltc2945. > > > > +config SENSORS_LTC2947 > > + tristate > > + > > +config SENSORS_LTC2947_I2C > > + tristate "Analog Devices LTC2947 High Precision Power and > > Energy Monitor over I2C" > > + depends on I2C > > + select REGMAP_I2C > > + select SENSORS_LTC2947 > > + help > > + If you say yes here you get support for Linear Technology > > LTC2947 > > + I2C High Precision Power and Energy Monitor > > + > > + This driver can also be built as a module. If so, the module > > will > > + be called ltc2947-i2c. > > + > > +config SENSORS_LTC2947_SPI > > + tristate "Analog Devices LTC2947 High Precision Power and > > Energy Monitor over SPI" > > + depends on SPI_MASTER > > + select REGMAP_SPI > > + select SENSORS_LTC2947 > > + help > > + If you say yes here you get support for Linear Technology > > LTC2947 > > + SPI High Precision Power and Energy Monitor > > + > > + This driver can also be built as a module. If so, the module > > will > > + be called ltc2947-spi. > > + > > config SENSORS_LTC2990 > > tristate "Linear Technology LTC2990" > > depends on I2C > > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile > > index c86ce4d3d36b..b54a4cec3203 100644 > > --- a/drivers/hwmon/Makefile > > +++ b/drivers/hwmon/Makefile > > @@ -107,6 +107,9 @@ obj-$(CONFIG_SENSORS_LM95234) += lm95234.o > > obj-$(CONFIG_SENSORS_LM95241) += lm95241.o > > obj-$(CONFIG_SENSORS_LM95245) += lm95245.o > > obj-$(CONFIG_SENSORS_LTC2945) += ltc2945.o > > +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_LTC4151) += ltc4151.o > > obj-$(CONFIG_SENSORS_LTC4215) += ltc4215.o > > diff --git a/drivers/hwmon/ltc2947-core.c b/drivers/hwmon/ltc2947- > > core.c > > new file mode 100644 > > index 000000000000..350b2b76467c > > --- /dev/null > > +++ b/drivers/hwmon/ltc2947-core.c > > @@ -0,0 +1,1421 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Analog Devices LTC2947 high precision power and energy monitor > > + * > > + * Copyright 2019 Analog Devices Inc. > > + */ > > +#include <linux/bitfield.h> > > BIT() is defined in linux/bits.h. ack. > > +#include <linux/clk.h> > > +#include <linux/device.h> > > +#include <linux/hwmon.h> > > +#include <linux/hwmon-sysfs.h> > > +#include <linux/module.h> > > +#include <linux/of.h> > > +#include <linux/regmap.h> > > + > > +#include "ltc2947.h" > > + > > +/* register's */ > > +#define LTC2947_REG_PAGE_CTRL 0xFF > > +#define LTC2947_REG_CTRL 0xF0 > > +#define LTC2947_REG_TBCTL 0xE9 > > +#define LTC2947_CONT_MODE_MASK BIT(3) > > +#define LTC2947_CONT_MODE(x) FIELD_PREP(LTC2947_CONT > > _MODE_MASK, x) > > +#define LTC2947_PRE_MASK GENMASK(2, 0) > > +#define LTC2947_PRE(x) FIELD_PREP(LTC2947_PRE_ > > MASK, x) > > +#define LTC2947_DIV_MASK GENMASK(7, 3) > > +#define LTC2947_DIV(x) FIELD_PREP(LTC2947_DIV_ > > MASK, x) > > +#define LTC2947_SHUTDOWN_MASK BIT(0) > > +#define LTC2947_REG_ACCUM_POL 0xE1 > > +#define LTC2947_ACCUM_POL_1_MASK GENMASK(1, 0) > > +#define LTC2947_ACCUM_POL_1(x) FIELD_PREP(LTC2947_ACCU > > M_POL_1_MASK, x) > > +#define LTC2947_ACCUM_POL_2_MASK GENMASK(3, 2) > > +#define LTC2947_ACCUM_POL_2(x) FIELD_PREP(LTC2947_ACCU > > M_POL_2_MASK, x) > > +#define LTC2947_REG_ACCUM_DEADBAND 0xE4 > > +#define LTC2947_REG_GPIOSTATCTL 0x67 > > +#define LTC2947_GPIO_EN_MASK BIT(0) > > +#define LTC2947_GPIO_EN(x) FIELD_PREP(LTC2947_GPIO_EN_MASK > > , x) > > +#define LTC2947_GPIO_FAN_EN_MASK BIT(6) > > +#define LTC2947_GPIO_FAN_EN(x) FIELD_PREP(LTC2947_GPIO > > _FAN_EN_MASK, x) > > +#define LTC2947_GPIO_FAN_POL_MASK BIT(7) > > +#define LTC2947_GPIO_FAN_POL(x) FIELD_PREP(LTC2947_GPIO > > _FAN_POL_MASK, x) > > +#define LTC2947_REG_GPIO_ACCUM 0xE3 > > +/* 200Khz */ > > +#define LTC2947_CLK_MIN 200000 > > +/* 25Mhz */ > > +#define LTC2947_CLK_MAX 25000000 > > +#define PAGE0 0 > > +#define PAGE1 1 > > +/* Voltage registers */ > > +#define LTC2947_REG_VOLTAGE 0xA0 > > +#define LTC2947_REG_VOLTAGE_MAX 0x50 > > +#define LTC2947_REG_VOLTAGE_MIN 0x52 > > +#define LTC2947_REG_VOLTAGE_THRE_H 0x90 > > +#define LTC2947_REG_VOLTAGE_THRE_L 0x92 > > +#define LTC2947_REG_DVCC 0xA4 > > +#define LTC2947_REG_DVCC_MAX 0x58 > > +#define LTC2947_REG_DVCC_MIN 0x5A > > +#define LTC2947_REG_DVCC_THRE_H 0x98 > > +#define LTC2947_REG_DVCC_THRE_L 0x9A > > +#define LTC2947_VOLTAGE_GEN_CHAN 0 > > +#define LTC2947_VOLTAGE_DVCC_CHAN 1 > > +/* Current registers */ > > +#define LTC2947_REG_CURRENT 0x90 > > +#define LTC2947_REG_CURRENT_MAX 0x40 > > +#define LTC2947_REG_CURRENT_MIN 0x42 > > +#define LTC2947_REG_CURRENT_THRE_H 0x80 > > +#define LTC2947_REG_CURRENT_THRE_L 0x82 > > +/* Power registers */ > > +#define LTC2947_REG_POWER 0x93 > > +#define LTC2947_REG_POWER_MAX 0x44 > > +#define LTC2947_REG_POWER_MIN 0x46 > > +#define LTC2947_REG_POWER_THRE_H 0x84 > > +#define LTC2947_REG_POWER_THRE_L 0x86 > > +/* Temperature registers */ > > +#define LTC2947_REG_TEMP 0xA2 > > +#define LTC2947_REG_TEMP_MAX 0x54 > > +#define LTC2947_REG_TEMP_MIN 0x56 > > +#define LTC2947_REG_TEMP_THRE_H 0x94 > > +#define LTC2947_REG_TEMP_THRE_L 0x96 > > +#define LTC2947_REG_TEMP_FAN_THRE_H 0x9C > > +#define LTC2947_REG_TEMP_FAN_THRE_L 0x9E > > +#define LTC2947_TEMP_FAN_CHAN 1 > > +/* Energy registers */ > > +#define LTC2947_REG_ENERGY1 0x06 > > +#define LTC2947_REG_ENERGY1_THRE_H 0x10 > > +#define LTC2947_REG_ENERGY1_THRE_L 0x16 > > +#define LTC2947_REG_ENERGY2 0x16 > > +#define LTC2947_REG_ENERGY2_THRE_H 0x30 > > +#define LTC2947_REG_ENERGY2_THRE_L 0x36 > > +#define ENERGY_MIN 0xFFFF800000000000LL > > +#define ENERGY_MAX 0x00007FFFFFFFFFFFLL > > +/* Status/Alarm/Overflow registers */ > > +#define LTC2947_REG_STATUS 0x80 > > +#define LTC2947_REG_STATVT 0x81 > > +#define LTC2947_REG_STATIP 0x82 > > +#define LTC2947_REG_STATC 0x83 > > +#define LTC2947_REG_STATE 0x84 > > +#define LTC2947_REG_STATCEOF 0x85 > > +#define LTC2947_REG_STATVDVCC 0x87 > > + > > +#define LTC2947_ALERTS_SIZE (LTC2947_REG_STATVDVCC - > > LTC2947_REG_STATUS) > > +#define LTC2947_UPDATE_VAL_MASK BIT(4) > > +#define LTC2947_MAX_VOLTAGE_MASK BIT(0) > > +#define LTC2947_MIN_VOLTAGE_MASK BIT(1) > > +#define LTC2947_MAX_CURRENT_MASK BIT(0) > > +#define LTC2947_MIN_CURRENT_MASK BIT(1) > > +#define LTC2947_MAX_POWER_MASK BIT(2) > > +#define LTC2947_MIN_POWER_MASK BIT(3) > > +#define LTC2947_MAX_TEMP_MASK BIT(2) > > +#define LTC2947_MIN_TEMP_MASK BIT(3) > > +#define LTC2947_MAX_TEMP_FAN_MASK BIT(4) > > +#define LTC2947_MIN_TEMP_FAN_MASK BIT(5) > > +#define LTC2947_SINGLE_SHOT_MASK BIT(2) > > +#define LTC2947_MAX_ENERGY1_MASK BIT(0) > > +#define LTC2947_MIN_ENERGY1_MASK BIT(1) > > +#define LTC2947_MAX_ENERGY2_MASK BIT(2) > > +#define LTC2947_MIN_ENERGY2_MASK BIT(3) > > +#define LTC2947_MIN_ENERGY1_O_MASK BIT(4) > > +#define LTC2947_MIN_ENERGY2_O_MASK BIT(5) > > +#define LTC2947_ADCERR_MASK BIT(5) > > +/* > > + * For accumulated values there's a fault if the ADC conversions > > are invalid > > + * (ADCERR) or if there is an overflow of the internal timebase > > register > > + * (which indicates invalid TBCTL configuration). > > + */ > > +#define LTC2947_ENERGY_FAULT_MASK GENMASK(6, 5) > > + > > +struct ltc2947_data { > > + struct regmap *map; > > + struct device *dev; > > + /* > > + * The mutex is needed because the device has 2 memory pages. > > When > > + * reading/writing the correct page needs to be set so that, > > the > > + * complete sequence select_page->read/write needs to be > > protected. > > + */ > > + struct mutex lock; > > + u32 lsb_energy; > > + bool reset; > > + bool gpio_out; > > +}; > > + > > +/* used for raw sysfs entries */ > > +enum { > > + LTC2947_POWER_INPUT, > > + LTC2947_POWER_THRE_L, > > + LTC2947_POWER_THRE_H, > > + LTC2947_POWER_HIGHEST, > > + LTC2947_POWER_LOWEST, > > + LTC2947_ENERGY1_INPUT, > > + LTC2947_ENERGY1_THRE_H, > > + LTC2947_ENERGY1_THRE_L, > > + LTC2947_ENERGY1_MAX_ALARM, > > + LTC2947_ENERGY1_MIN_ALARM, > > + LTC2947_ENERGY2_INPUT, > > + LTC2947_ENERGY2_THRE_H, > > + LTC2947_ENERGY2_THRE_L, > > + LTC2947_ENERGY2_MAX_ALARM, > > + LTC2947_ENERGY2_MIN_ALARM, > > + LTC2947_ENERGY1_OVERF_ALARM, > > + LTC2947_ENERGY2_OVERF_ALARM, > > + LTC2947_FAULT = LTC2947_ADCERR_MASK, > > + LTC2947_ENERGY_FAULT = LTC2947_ENERGY_FAULT_MASK, > > +}; > > + > > +static int __ltc2947_val_read16(const struct ltc2947_data *st, > > const u8 reg, > > + u64 *val) > > +{ > > + __be16 __val = 0; > > + int ret; > > + > > + ret = regmap_bulk_read(st->map, reg, &__val, 2); > > + if (ret) > > + return ret; > > + > > + *val = be16_to_cpu(__val); > > + > > + return 0; > > +} > > + > > +static int __ltc2947_val_read24(const struct ltc2947_data *st, > > const u8 reg, > > + u64 *val) > > +{ > > + __be32 __val = 0; > > + int ret; > > + > > + ret = regmap_bulk_read(st->map, reg, &__val, 3); > > + if (ret) > > + return ret; > > + > > + *val = be32_to_cpu(__val) >> 8; > > + > > + return 0; > > +} > > + > > +static int __ltc2947_val_read64(const struct ltc2947_data *st, > > const u8 reg, > > + u64 *val) > > +{ > > + __be64 __val = 0; > > + int ret; > > + > > + ret = regmap_bulk_read(st->map, reg, &__val, 6); > > + if (ret) > > + return ret; > > + > > + *val = be64_to_cpu(__val) >> 16; > > + > > + return 0; > > +} > > + > > +static int ltc2947_val_read(struct ltc2947_data *st, const u8 reg, > > + const u8 page, const size_t size, s64 *val) > > +{ > > + int ret; > > + u64 __val = 0; > > + > > + mutex_lock(&st->lock); > > + > > + if (st->reset) { > > + mutex_unlock(&st->lock); > > + return -EPERM; > > Not sure what the error here should be, but EPERM is not correct. > > Under which conditions would this function be called while in > suspend ? Honestly, this is more like a sanity check. I'm not sure if we can get here in suspend mode. Don't userland apps can still run in suspend? I guess so but I'm not 100% sure on this. Do you have any recommendation for the error here? > > + } > > + > > + ret = regmap_write(st->map, LTC2947_REG_PAGE_CTRL, page); > > + if (ret) { > > + mutex_unlock(&st->lock); > > + return ret; > > + } > > + > > + dev_dbg(st->dev, "Read val, reg:%02X, p:%d sz:%zu\n", reg, > > page, > > + size); > > + > > + switch (size) { > > + case 2: > > + ret = __ltc2947_val_read16(st, reg, &__val); > > + break; > > + case 3: > > + ret = __ltc2947_val_read24(st, reg, &__val); > > + break; > > + case 6: > > + ret = __ltc2947_val_read64(st, reg, &__val); > > + break; > > + default: > > + dev_err(st->dev, "Invalid size(%zu) to read", size); > > Following the code flow, this is dead code. An error message > seems pointless. Will remove the `dev_err` call. > > + ret = -EINVAL; > > + break; > > + } > > + > > + mutex_unlock(&st->lock); > > + > > + if (ret) > > + return ret; > > + > > + *val = sign_extend64(__val, (8 * size) - 1); > > + > > + dev_dbg(st->dev, "Got s:%lld, u:%016llX\n", *val, __val); > > + > > + return 0; > > +} > > + > > +static int __ltc2947_val_write64(const struct ltc2947_data *st, > > const u8 reg, > > + const u64 val) > > +{ > > + __be64 __val; > > + > > + __val = cpu_to_be64(val << 16); > > + return regmap_bulk_write(st->map, reg, &__val, 6); > > +} > > + > > +static int __ltc2947_val_write16(const struct ltc2947_data *st, > > const u8 reg, > > + const u16 val) > > +{ > > + __be16 __val; > > + > > + __val = cpu_to_be16(val); > > + return regmap_bulk_write(st->map, reg, &__val, 2); > > +} > > + > > +static int ltc2947_val_write(struct ltc2947_data *st, const u8 > > reg, > > + const u8 page, const size_t size, const > > u64 val) > > +{ > > + int ret; > > + > > + mutex_lock(&st->lock); > > + /* > > + * Do not allow channel readings if device is in sleep state. > > + * A read/write on the spi/i2c bus would bring the device > > prematurely > > + * out of sleep. > > + */ > > + if (st->reset) { > > + mutex_unlock(&st->lock); > > + return -EPERM; > > + } > > + /* set device on correct page */ > > + ret = regmap_write(st->map, LTC2947_REG_PAGE_CTRL, page); > > + if (ret) { > > + mutex_unlock(&st->lock); > > + return ret; > > + } > > + > > + dev_dbg(st->dev, "Write val, r:%02X, p:%d, sz:%zu, > > val:%016llX\n", > > + reg, page, size, val); > > + > > + switch (size) { > > + case 2: > > + ret = __ltc2947_val_write16(st, reg, val); > > + break; > > + case 6: > > + ret = __ltc2947_val_write64(st, reg, val); > > + break; > > + default: > > + dev_err(st->dev, "Invalid size(%zu) to write", size); > > + ret = -EINVAL; > > + break; > > + } > > + > > + mutex_unlock(&st->lock); > > + > > + return ret; > > +} > > + > > +static int ltc2947_reset_history(struct ltc2947_data *st, const u8 > > reg_h, > > + const u8 reg_l) > > +{ > > + int ret; > > + /* > > + * let's reset the tracking register's. Tracking register's > > have all > > + * 2 bytes size > > + */ > > + ret = ltc2947_val_write(st, reg_h, PAGE0, 2, 0x8000U); > > + if (ret) > > + return ret; > > + > > + return ltc2947_val_write(st, reg_l, PAGE0, 2, 0x7FFFU); > > +} > > + > > +static int ltc2947_alarm_read(struct ltc2947_data *st, const u8 > > reg, > > + const u32 mask, long *val) > > +{ > > + u8 offset = reg - LTC2947_REG_STATUS; > > + /* +1 to include status reg */ > > + char alarms[LTC2947_ALERTS_SIZE + 1]; > > + int ret = 0; > > + > > + memset(alarms, 0, sizeof(alarms)); > > + > > + mutex_lock(&st->lock); > > + > > + if (st->reset) { > > + ret = -EPERM; > > + goto unlock; > > + } > > + > > + ret = regmap_write(st->map, LTC2947_REG_PAGE_CTRL, PAGE0); > > + if (ret) > > + goto unlock; > > + > > + dev_dbg(st->dev, "Read alarm, reg:%02X, mask:%02X\n", reg, > > mask); > > + /* > > + * As stated in the datasheet, when Threshold and Overflow > > registers > > + * are used, the status and all alert registers must be read in > > one > > + * multi-byte transaction. > > + */ > > + ret = regmap_bulk_read(st->map, LTC2947_REG_STATUS, alarms, > > + sizeof(alarms)); > > + if (ret) > > + goto unlock; > > + > > + /* get the alarm */ > > + *val = !!(alarms[offset] & mask); > > +unlock: > > + mutex_unlock(&st->lock); > > + return ret; > > +} > > + > > +static ssize_t ltc2947_set_value(struct device *dev, > > + struct device_attribute *da, > > + const char *buf, size_t count) > > +{ > > + struct ltc2947_data *st = dev_get_drvdata(dev); > > + struct sensor_device_attribute *attr = to_sensor_dev_attr(da); > > + int ret; > > + u8 reg, page = PAGE1; > > + s64 val = 0; > > + > > + ret = kstrtoll(buf, 10, &val); > > + if (ret) { > > + dev_err(st->dev, "Failed to convert the value\n"); > > + return ret; > > + } > > + > > + switch (attr->index) { > > + case LTC2947_POWER_THRE_H: > > + val = clamp_val(val, SHRT_MIN, SHRT_MAX); > > + ret = ltc2947_val_write(st, LTC2947_REG_POWER_THRE_H, > > PAGE1, 2, > > + div_s64(val, 200000)); > > + return ret ? ret : count; > > + case LTC2947_POWER_THRE_L: > > + val = clamp_val(val, SHRT_MIN, SHRT_MAX); > > + ret = ltc2947_val_write(st, LTC2947_REG_POWER_THRE_L, > > PAGE1, 2, > > + div_s64(val, 200000)); > > + return ret ? ret : count; > > + case LTC2947_ENERGY1_THRE_H: > > + reg = LTC2947_REG_ENERGY1_THRE_H; > > + break; > > + case LTC2947_ENERGY1_THRE_L: > > + reg = LTC2947_REG_ENERGY1_THRE_L; > > + break; > > + case LTC2947_ENERGY2_THRE_H: > > + reg = LTC2947_REG_ENERGY2_THRE_H; > > + break; > > + case LTC2947_ENERGY2_THRE_L: > > + reg = LTC2947_REG_ENERGY2_THRE_L; > > + break; > > + case LTC2947_ENERGY1_INPUT: > > + reg = LTC2947_REG_ENERGY1; > > + page = PAGE0; > > + break; > > + case LTC2947_ENERGY2_INPUT: > > + reg = LTC2947_REG_ENERGY2; > > + page = PAGE0; > > + break; > > + default: > > + return -ENOTSUPP; > > + } > > + > > + val = clamp_val(val, ENERGY_MIN, ENERGY_MAX); > > + /* we are losing the fractional part here... */ > > + val = div_s64(val * 1000, st->lsb_energy); > > + > > + ret = ltc2947_val_write(st, reg, page, 6, val); > > + > > + return ret ? ret : count; > > +} > > + > > +static ssize_t ltc2947_show_value(struct device *dev, > > + struct device_attribute *da, char > > *buf) > > +{ > > + struct ltc2947_data *st = dev_get_drvdata(dev); > > + struct sensor_device_attribute *attr = to_sensor_dev_attr(da); > > + int ret; > > + s64 val = 0; > > + > > + switch (attr->index) { > > + case LTC2947_POWER_INPUT: > > + ret = ltc2947_val_read(st, LTC2947_REG_POWER, PAGE0, 3, > > &val); > > + return ret ? ret : sprintf(buf, "%lld\n", val * 50000); > > + case LTC2947_POWER_THRE_H: > > + ret = ltc2947_val_read(st, LTC2947_REG_POWER_THRE_H, > > PAGE1, 2, > > + &val); > > + return ret ? ret : sprintf(buf, "%lld\n", val * > > 200000); > > + case LTC2947_POWER_THRE_L: > > + ret = ltc2947_val_read(st, LTC2947_REG_POWER_THRE_L, > > PAGE1, 2, > > + &val); > > + return ret ? ret : sprintf(buf, "%lld\n", val * > > 200000); > > + case LTC2947_POWER_HIGHEST: > > + ret = ltc2947_val_read(st, LTC2947_REG_POWER_MAX, > > PAGE0, 2, > > + &val); > > + return ret ? ret : sprintf(buf, "%lld\n", val * > > 200000); > > + case LTC2947_POWER_LOWEST: > > + ret = ltc2947_val_read(st, LTC2947_REG_POWER_MIN, > > PAGE0, 2, > > + &val); > > + return ret ? ret : sprintf(buf, "%lld\n", val * > > 200000); > > + case LTC2947_ENERGY1_THRE_H: > > + ret = ltc2947_val_read(st, LTC2947_REG_ENERGY1_THRE_H, > > PAGE1, 6, > > + &val); > > + break; > > + case LTC2947_ENERGY1_THRE_L: > > + ret = ltc2947_val_read(st, LTC2947_REG_ENERGY1_THRE_L, > > PAGE1, 6, > > + &val); > > + break; > > + case LTC2947_ENERGY2_THRE_H: > > + ret = ltc2947_val_read(st, LTC2947_REG_ENERGY2_THRE_H, > > PAGE1, 6, > > + &val); > > + break; > > + case LTC2947_ENERGY2_THRE_L: > > + ret = ltc2947_val_read(st, LTC2947_REG_ENERGY2_THRE_L, > > PAGE1, 6, > > + &val); > > + break; > > + case LTC2947_ENERGY1_INPUT: > > + ret = ltc2947_val_read(st, LTC2947_REG_ENERGY1, PAGE0, > > 6, &val); > > + break; > > + case LTC2947_ENERGY2_INPUT: > > + ret = ltc2947_val_read(st, LTC2947_REG_ENERGY2, PAGE0, > > 6, &val); > > + break; > > + default: > > + return -EINVAL; > > + } > > + > > + /* if we got here, must be an energy reading... */ > > + if (ret) > > + return ret; > > + > > + /* value in microJoule. st->lsb_energy was multiplied by 10E9 > > */ > > + val = div_s64(val * st->lsb_energy, 1000); > > + > > + return sprintf(buf, "%lld\n", val); > > +} > > + > > +static ssize_t ltc2947_show_alert(struct device *dev, > > + struct device_attribute *da, char > > *buf) > > +{ > > + struct ltc2947_data *st = dev_get_drvdata(dev); > > + struct sensor_device_attribute *attr = to_sensor_dev_attr(da); > > + long alert; > > + int ret = 0; > > + > > + switch (attr->index) { > > + case LTC2947_ENERGY1_MAX_ALARM: > > + ret = ltc2947_alarm_read(st, LTC2947_REG_STATE, > > + LTC2947_MAX_ENERGY1_MASK, > > &alert); > > + break; > > + case LTC2947_ENERGY1_MIN_ALARM: > > + ret = ltc2947_alarm_read(st, LTC2947_REG_STATE, > > + LTC2947_MIN_ENERGY1_MASK, > > &alert); > > + break; > > + case LTC2947_ENERGY2_MAX_ALARM: > > + ret = ltc2947_alarm_read(st, LTC2947_REG_STATE, > > + LTC2947_MAX_ENERGY2_MASK, > > &alert); > > + break; > > + case LTC2947_ENERGY2_MIN_ALARM: > > + ret = ltc2947_alarm_read(st, LTC2947_REG_STATE, > > + LTC2947_MIN_ENERGY2_MASK, > > &alert); > > + break; > > + case LTC2947_ENERGY1_OVERF_ALARM: > > + ret = ltc2947_alarm_read(st, LTC2947_REG_STATCEOF, > > + LTC2947_MIN_ENERGY1_O_MASK, > > &alert); > > + break; > > + case LTC2947_ENERGY2_OVERF_ALARM: > > + ret = ltc2947_alarm_read(st, LTC2947_REG_STATCEOF, > > + LTC2947_MIN_ENERGY2_O_MASK, > > &alert); > > + break; > > + case LTC2947_FAULT: > > + case LTC2947_ENERGY_FAULT: > > + ret = ltc2947_alarm_read(st, LTC2947_REG_STATUS, attr- > > >index, > > + &alert); > > + break; > > + default: > > + return -EINVAL; > > + } > > + > > + return ret ? ret : sprintf(buf, "%li\n", alert); > > +} > > + > > +static int ltc2947_read_temp(struct device *dev, const u32 attr, > > long *val, > > + const int channel) > > +{ > > + int ret; > > + struct ltc2947_data *st = dev_get_drvdata(dev); > > + s64 __val = 0; > > + > > + if (channel < 0 || channel > LTC2947_TEMP_FAN_CHAN) { > > + dev_err(st->dev, "Invalid chan%d for temperature", > > channel); > > + return -EINVAL; > > + } > > + > > + switch (attr) { > > + case hwmon_temp_input: > > + ret = ltc2947_val_read(st, LTC2947_REG_TEMP, PAGE0, 2, > > &__val); > > + break; > > + case hwmon_temp_highest: > > + ret = ltc2947_val_read(st, LTC2947_REG_TEMP_MAX, PAGE0, > > 2, > > + &__val); > > + break; > > + case hwmon_temp_lowest: > > + ret = ltc2947_val_read(st, LTC2947_REG_TEMP_MIN, PAGE0, > > 2, > > + &__val); > > + break; > > + case hwmon_temp_max_alarm: > > + if (channel == LTC2947_TEMP_FAN_CHAN) > > + return ltc2947_alarm_read(st, > > LTC2947_REG_STATVT, > > + LTC2947_MAX_TEMP_FAN_ > > MASK, > > + val); > > + else > > + return ltc2947_alarm_read(st, > > LTC2947_REG_STATVT, > > + LTC2947_MAX_TEMP_MASK > > , val); > > + case hwmon_temp_min_alarm: > > + if (channel == LTC2947_TEMP_FAN_CHAN) > > + return ltc2947_alarm_read(st, > > LTC2947_REG_STATVT, > > + LTC2947_MIN_TEMP_FAN > > _MASK, > > + val); > > + else > > + return ltc2947_alarm_read(st, > > LTC2947_REG_STATVT, > > + LTC2947_MIN_TEMP_MAS > > K, val); > > + case hwmon_temp_max: > > + if (channel == LTC2947_TEMP_FAN_CHAN) > > + ret = ltc2947_val_read(st, > > LTC2947_REG_TEMP_FAN_THRE_H, > > + PAGE1, 2, &__val); > > + else > > + ret = ltc2947_val_read(st, > > LTC2947_REG_TEMP_THRE_H, > > + PAGE1, 2, &__val); > > + break; > > + case hwmon_temp_min: > > + if (channel == LTC2947_TEMP_FAN_CHAN) > > + ret = ltc2947_val_read(st, > > LTC2947_REG_TEMP_FAN_THRE_L, > > + PAGE1, 2, &__val); > > + else > > + ret = ltc2947_val_read(st, > > LTC2947_REG_TEMP_THRE_L, > > + PAGE1, 2, &__val); > > + break; > > + default: > > + return -ENOTSUPP; > > + } > > + > > + if (ret) > > + return ret; > > + > > + /* in milidegrees celcius, temp is given by: */ > > + *val = (__val * 204) + 550; > > + > > + return 0; > > +} > > + > > +static int ltc2947_read_power(struct device *dev, const u32 attr, > > long *val) > > +{ > > + struct ltc2947_data *st = dev_get_drvdata(dev); > > + > > + switch (attr) { > > + case hwmon_power_max_alarm: > > + return ltc2947_alarm_read(st, LTC2947_REG_STATIP, > > + LTC2947_MAX_POWER_MASK, val); > > + case hwmon_power_min_alarm: > > + return ltc2947_alarm_read(st, LTC2947_REG_STATIP, > > + LTC2947_MIN_POWER_MASK, val); > > + default: > > + return -ENOTSUPP; > > + } > > + > > + return 0; > > +} > > + > > +static int ltc2947_read_curr(struct device *dev, const u32 attr, > > long *val) > > +{ > > + struct ltc2947_data *st = dev_get_drvdata(dev); > > + int ret; > > + u8 lsb = 12; /* in mA */ > > + s64 __val = 0; > > + > > + switch (attr) { > > + case hwmon_curr_input: > > + ret = ltc2947_val_read(st, LTC2947_REG_CURRENT, PAGE0, > > 3, > > + &__val); > > + lsb = 3; > > + break; > > + case hwmon_curr_highest: > > + ret = ltc2947_val_read(st, LTC2947_REG_CURRENT_MAX, > > PAGE0, 2, > > + &__val); > > + break; > > + case hwmon_curr_lowest: > > + ret = ltc2947_val_read(st, LTC2947_REG_CURRENT_MIN, > > PAGE0, 2, > > + &__val); > > + break; > > + case hwmon_curr_max_alarm: > > + return ltc2947_alarm_read(st, LTC2947_REG_STATIP, > > + LTC2947_MAX_CURRENT_MASK, > > val); > > + case hwmon_curr_min_alarm: > > + return ltc2947_alarm_read(st, LTC2947_REG_STATIP, > > + LTC2947_MIN_CURRENT_MASK, > > val); > > + case hwmon_curr_max: > > + ret = ltc2947_val_read(st, LTC2947_REG_CURRENT_THRE_H, > > PAGE1, 2, > > + &__val); > > + break; > > + case hwmon_curr_min: > > + ret = ltc2947_val_read(st, LTC2947_REG_CURRENT_THRE_L, > > PAGE1, 2, > > + &__val); > > + break; > > + default: > > + return -ENOTSUPP; > > + } > > + > > + if (ret) > > + return ret; > > + > > + *val = __val * lsb; > > + > > + return 0; > > +} > > + > > +static int ltc2947_read_in(struct device *dev, const u32 attr, > > long *val, > > + const int channel) > > +{ > > + struct ltc2947_data *st = dev_get_drvdata(dev); > > + int ret; > > + u8 lsb = 2; /* in mV */ > > + s64 __val = 0; > > + > > + if (channel < 0 || channel > LTC2947_VOLTAGE_DVCC_CHAN) { > > + dev_err(st->dev, "Invalid chan%d for voltage", > > channel); > > + return -EINVAL; > > + } > > + > > + switch (attr) { > > + case hwmon_in_input: > > + if (channel == LTC2947_VOLTAGE_DVCC_CHAN) { > > + ret = ltc2947_val_read(st, LTC2947_REG_DVCC, > > PAGE0, 2, > > + &__val); > > + lsb = 145; > > + } else { > > + ret = ltc2947_val_read(st, LTC2947_REG_VOLTAGE, > > PAGE0, > > + 2, &__val); > > + } > > + break; > > + case hwmon_in_highest: > > + if (channel == LTC2947_VOLTAGE_DVCC_CHAN) { > > + ret = ltc2947_val_read(st, > > LTC2947_REG_DVCC_MAX, PAGE0, > > + 2, &__val); > > + lsb = 145; > > + } else { > > + ret = ltc2947_val_read(st, > > LTC2947_REG_VOLTAGE_MAX, > > + PAGE0, 2, &__val); > > + } > > + break; > > + case hwmon_in_lowest: > > + if (channel == LTC2947_VOLTAGE_DVCC_CHAN) { > > + ret = ltc2947_val_read(st, > > LTC2947_REG_DVCC_MIN, PAGE0, > > + 2, &__val); > > + lsb = 145; > > + } else { > > + ret = ltc2947_val_read(st, > > LTC2947_REG_VOLTAGE_MIN, > > + PAGE0, 2, &__val); > > + } > > + break; > > + case hwmon_in_max_alarm: > > + if (channel == LTC2947_VOLTAGE_DVCC_CHAN) > > + return ltc2947_alarm_read(st, > > LTC2947_REG_STATVDVCC, > > + LTC2947_MAX_VOLTAGE_M > > ASK, > > + val); > > + else > > + return ltc2947_alarm_read(st, > > LTC2947_REG_STATVT, > > + LTC2947_MAX_VOLTAGE_M > > ASK, > > + val); > > + case hwmon_in_min_alarm: > > + if (channel == LTC2947_VOLTAGE_DVCC_CHAN) > > + return ltc2947_alarm_read(st, > > LTC2947_REG_STATVDVCC, > > + LTC2947_MIN_VOLTAGE_M > > ASK, > > + val); > > + else > > + return ltc2947_alarm_read(st, > > LTC2947_REG_STATVT, > > + LTC2947_MIN_VOLTAGE_M > > ASK, > > + val); > > + case hwmon_in_max: > > + if (channel == LTC2947_VOLTAGE_DVCC_CHAN) { > > + ret = ltc2947_val_read(st, > > LTC2947_REG_DVCC_THRE_H, > > + PAGE1, 2, &__val); > > + lsb = 145; > > + } else { > > + ret = ltc2947_val_read(st, > > LTC2947_REG_VOLTAGE_THRE_H, > > + PAGE1, 2, &__val); > > + } > > + break; > > + case hwmon_in_min: > > + if (channel == LTC2947_VOLTAGE_DVCC_CHAN) { > > + ret = ltc2947_val_read(st, > > LTC2947_REG_DVCC_THRE_L, > > + PAGE1, 2, &__val); > > + lsb = 145; > > + } else { > > + ret = ltc2947_val_read(st, > > LTC2947_REG_VOLTAGE_THRE_L, > > + PAGE1, 2, &__val); > > + } > > + break; > > + default: > > + return -ENOTSUPP; > > + } > > + > > + if (ret) > > + return ret; > > + > > + *val = __val * lsb; > > + > > + return 0; > > +} > > + > > +static int ltc2947_read(struct device *dev, enum > > hwmon_sensor_types type, > > + u32 attr, int channel, long *val) > > +{ > > + switch (type) { > > + case hwmon_in: > > + return ltc2947_read_in(dev, attr, val, channel); > > + case hwmon_curr: > > + return ltc2947_read_curr(dev, attr, val); > > + case hwmon_power: > > + return ltc2947_read_power(dev, attr, val); > > + case hwmon_temp: > > + return ltc2947_read_temp(dev, attr, val, channel); > > + default: > > + return -ENOTSUPP; > > + } > > +} > > + > > +static int ltc2947_write_temp(struct device *dev, const u32 attr, > > + long val, const int channel) > > +{ > > + struct ltc2947_data *st = dev_get_drvdata(dev); > > + > > + if (channel < 0 || channel > LTC2947_TEMP_FAN_CHAN) { > > + dev_err(st->dev, "Invalid chan%d for temperature", > > channel); > > + return -EINVAL; > > + } > > + > > + switch (attr) { > > + case hwmon_temp_reset_history: > > + if (val != 1) > > + return -EINVAL; > > + return ltc2947_reset_history(st, LTC2947_REG_TEMP_MAX, > > + LTC2947_REG_TEMP_MIN); > > + case hwmon_temp_max: > > + val = clamp_val(val, SHRT_MIN, SHRT_MAX); > > + if (channel == LTC2947_TEMP_FAN_CHAN) { > > + if (!st->gpio_out) > > + return -ENOTSUPP; > > + > > + return ltc2947_val_write(st, > > + LTC2947_REG_TEMP_FAN_THRE_H, > > PAGE1, 2, > > + DIV_ROUND_CLOSEST(val - 550, > > 204)); > > + } else { > > + return ltc2947_val_write(st, > > LTC2947_REG_TEMP_THRE_H, > > + PAGE1, 2, > > + DIV_ROUND_CLOSEST(val - 550, > > 204)); > > + } > > + case hwmon_temp_min: > > + val = clamp_val(val, SHRT_MIN, SHRT_MAX); > > + if (channel == LTC2947_TEMP_FAN_CHAN) { > > + if (!st->gpio_out) > > + return -ENOTSUPP; > > + > > + return ltc2947_val_write(st, > > + LTC2947_REG_TEMP_FAN_THRE_L, > > PAGE1, 2, > > + DIV_ROUND_CLOSEST(val - 550, > > 204)); > > + } else { > > + return ltc2947_val_write(st, > > LTC2947_REG_TEMP_THRE_L, > > + PAGE1, 2, > > + DIV_ROUND_CLOSEST(val - 550, > > 204)); > > + } > > + default: > > + return -ENOTSUPP; > > + } > > +} > > + > > +static int ltc2947_write_power(struct device *dev, const u32 attr, > > + long val) > > +{ > > + struct ltc2947_data *st = dev_get_drvdata(dev); > > + > > + switch (attr) { > > + case hwmon_power_reset_history: > > + if (val != 1) > > + return -EINVAL; > > + return ltc2947_reset_history(st, LTC2947_REG_POWER_MAX, > > + LTC2947_REG_POWER_MIN); > > + default: > > + return -ENOTSUPP; > > + } > > +} > > + > > +static int ltc2947_write_curr(struct device *dev, const u32 attr, > > + long val) > > +{ > > + struct ltc2947_data *st = dev_get_drvdata(dev); > > + > > + switch (attr) { > > + case hwmon_curr_reset_history: > > + if (val != 1) > > + return -EINVAL; > > + return ltc2947_reset_history(st, > > LTC2947_REG_CURRENT_MAX, > > + LTC2947_REG_CURRENT_MIN); > > + case hwmon_curr_max: > > + val = clamp_val(val, SHRT_MIN, SHRT_MAX); > > + return ltc2947_val_write(st, > > LTC2947_REG_CURRENT_THRE_H, PAGE1, > > + 2, DIV_ROUND_CLOSEST(val, > > 12)); > > + case hwmon_curr_min: > > + val = clamp_val(val, SHRT_MIN, SHRT_MAX); > > + return ltc2947_val_write(st, > > LTC2947_REG_CURRENT_THRE_L, PAGE1, > > + 2, DIV_ROUND_CLOSEST(val, > > 12)); > > + default: > > + return -ENOTSUPP; > > + } > > +} > > + > > +static int ltc2947_write_in(struct device *dev, const u32 attr, > > long val, > > + const int channel) > > +{ > > + struct ltc2947_data *st = dev_get_drvdata(dev); > > + > > + if (channel > LTC2947_VOLTAGE_DVCC_CHAN) { > > + dev_err(st->dev, "Invalid chan%d for voltage", > > channel); > > + return -EINVAL; > > + } > > + > > + switch (attr) { > > + case hwmon_in_reset_history: > > + if (val != 1) > > + return -EINVAL; > > + > > + if (channel == LTC2947_VOLTAGE_DVCC_CHAN) > > + return ltc2947_reset_history(st, > > LTC2947_REG_DVCC_MAX, > > + LTC2947_REG_DVCC_M > > IN); > > + else > > + return ltc2947_reset_history(st, > > + LTC2947_REG_VOLTAG > > E_MAX, > > + LTC2947_REG_VOLTAG > > E_MIN); > > + case hwmon_in_max: > > + val = clamp_val(val, SHRT_MIN, SHRT_MAX); > > + > > + if (channel == LTC2947_VOLTAGE_DVCC_CHAN) > > + return ltc2947_val_write(st, > > LTC2947_REG_DVCC_THRE_H, > > + PAGE1, 2, > > + DIV_ROUND_CLOSEST(val, > > 145)); > > + else > > + return ltc2947_val_write(st, > > LTC2947_REG_VOLTAGE_THRE_H, > > + PAGE1, 2, > > + DIV_ROUND_CLOSEST(val, > > 2)); > > + case hwmon_in_min: > > + val = clamp_val(val, SHRT_MIN, SHRT_MAX); > > + > > + if (channel == LTC2947_VOLTAGE_DVCC_CHAN) > > + return ltc2947_val_write(st, > > LTC2947_REG_DVCC_THRE_L, > > + PAGE1, 2, > > + DIV_ROUND_CLOSEST(val, > > 145)); > > + else > > + return ltc2947_val_write(st, > > LTC2947_REG_VOLTAGE_THRE_L, > > + PAGE1, 2, > > + DIV_ROUND_CLOSEST(val, > > 2)); > > + default: > > + return -ENOTSUPP; > > + } > > +} > > + > > +static int ltc2947_write(struct device *dev, > > + enum hwmon_sensor_types type, > > + u32 attr, int channel, long val) > > +{ > > + switch (type) { > > + case hwmon_in: > > + return ltc2947_write_in(dev, attr, val, channel); > > + case hwmon_curr: > > + return ltc2947_write_curr(dev, attr, val); > > + case hwmon_power: > > + return ltc2947_write_power(dev, attr, val); > > + case hwmon_temp: > > + return ltc2947_write_temp(dev, attr, val, channel); > > + default: > > + return -ENOTSUPP; > > + } > > +} > > + > > +static int ltc2947_read_labels(struct device *dev, > > + enum hwmon_sensor_types type, > > + u32 attr, int channel, const char **str) > > +{ > > + switch (type) { > > + case hwmon_in: > > + if (channel == LTC2947_VOLTAGE_DVCC_CHAN) > > + *str = "DVCC"; > > + else > > + *str = "VP-VM"; > > + return 0; > > + case hwmon_curr: > > + *str = "IP-IM"; > > + return 0; > > + case hwmon_temp: > > + if (channel == LTC2947_TEMP_FAN_CHAN) > > + *str = "TEMPFAN"; > > + else > > + *str = "Ambient"; > > + return 0; > > + case hwmon_power: > > + *str = "Power"; > > + return 0; > > + default: > > + return -ENOTSUPP; > > + } > > +} > > + > > +static int ltc2947_in_is_visible(const u32 attr) > > +{ > > + switch (attr) { > > + case hwmon_in_input: > > + case hwmon_in_highest: > > + case hwmon_in_lowest: > > + case hwmon_in_max_alarm: > > + case hwmon_in_min_alarm: > > + case hwmon_in_label: > > + return 0444; > > + case hwmon_in_reset_history: > > + return 0200; > > + case hwmon_in_max: > > + case hwmon_in_min: > > + return 0644; > > + default: > > + return 0; > > + } > > +} > > + > > +static int ltc2947_curr_is_visible(const u32 attr) > > +{ > > + switch (attr) { > > + case hwmon_curr_input: > > + case hwmon_curr_highest: > > + case hwmon_curr_lowest: > > + case hwmon_curr_max_alarm: > > + case hwmon_curr_min_alarm: > > + case hwmon_curr_label: > > + return 0444; > > + case hwmon_curr_reset_history: > > + return 0200; > > + case hwmon_curr_max: > > + case hwmon_curr_min: > > + return 0644; > > + default: > > + return 0; > > + } > > +} > > + > > +static int ltc2947_power_is_visible(const u32 attr) > > +{ > > + switch (attr) { > > + case hwmon_power_label: > > + case hwmon_power_max_alarm: > > + case hwmon_power_min_alarm: > > + return 0444; > > + case hwmon_power_reset_history: > > + return 0200; > > + default: > > + return 0; > > + } > > +} > > + > > +static int ltc2947_temp_is_visible(const u32 attr) > > +{ > > + switch (attr) { > > + case hwmon_temp_input: > > + case hwmon_temp_highest: > > + case hwmon_temp_lowest: > > + case hwmon_temp_max_alarm: > > + case hwmon_temp_min_alarm: > > + case hwmon_temp_label: > > + return 0444; > > + case hwmon_temp_reset_history: > > + return 0200; > > + case hwmon_temp_max: > > + case hwmon_temp_min: > > + return 0644; > > + default: > > + return 0; > > + } > > +} > > + > > +static umode_t ltc2947_is_visible(const void *data, > > + enum hwmon_sensor_types type, > > + u32 attr, int channel) > > +{ > > + switch (type) { > > + case hwmon_in: > > + return ltc2947_in_is_visible(attr); > > + case hwmon_curr: > > + return ltc2947_curr_is_visible(attr); > > + case hwmon_power: > > + return ltc2947_power_is_visible(attr); > > + case hwmon_temp: > > + return ltc2947_temp_is_visible(attr); > > + default: > > + return 0; > > + } > > +} > > + > > +static const struct hwmon_channel_info *ltc2947_info[] = { > > + HWMON_CHANNEL_INFO(in, > > + HWMON_I_INPUT | HWMON_I_LOWEST | > > HWMON_I_HIGHEST | > > + HWMON_I_MAX | HWMON_I_MIN | > > HWMON_I_RESET_HISTORY | > > + HWMON_I_MIN_ALARM | HWMON_I_MAX_ALARM | > > + HWMON_I_LABEL, > > + HWMON_I_INPUT | HWMON_I_LOWEST | > > HWMON_I_HIGHEST | > > + HWMON_I_MAX | HWMON_I_MIN | > > HWMON_I_RESET_HISTORY | > > + HWMON_I_MIN_ALARM | HWMON_I_MAX_ALARM | > > + HWMON_I_LABEL), > > + HWMON_CHANNEL_INFO(curr, > > + HWMON_C_INPUT | HWMON_C_LOWEST | > > HWMON_C_HIGHEST | > > + HWMON_C_MAX | HWMON_C_MIN | > > HWMON_C_RESET_HISTORY | > > + HWMON_C_MIN_ALARM | HWMON_C_MAX_ALARM | > > + HWMON_C_LABEL), > > + HWMON_CHANNEL_INFO(power, > > + HWMON_P_RESET_HISTORY | HWMON_P_MAX_ALARM | > > + HWMON_P_MIN_ALARM | HWMON_P_LABEL), > > + HWMON_CHANNEL_INFO(temp, > > + HWMON_T_INPUT | HWMON_T_LOWEST | > > HWMON_T_HIGHEST | > > + HWMON_T_MAX | HWMON_T_MIN | > > HWMON_T_RESET_HISTORY | > > + HWMON_T_MIN_ALARM | HWMON_T_MAX_ALARM | > > + HWMON_T_LABEL, > > + HWMON_T_MAX_ALARM | HWMON_T_MIN_ALARM | > > HWMON_T_MAX | > > + HWMON_T_MIN | HWMON_T_LABEL), > > + NULL > > +}; > > + > > +static const struct hwmon_ops ltc2947_hwmon_ops = { > > + .is_visible = ltc2947_is_visible, > > + .read = ltc2947_read, > > + .write = ltc2947_write, > > + .read_string = ltc2947_read_labels, > > +}; > > + > > +static const struct hwmon_chip_info ltc2947_chip_info = { > > + .ops = <c2947_hwmon_ops, > > + .info = ltc2947_info, > > +}; > > + > > +/* power attributes - need u64 value */ > > +static SENSOR_DEVICE_ATTR(power1_input, 0444, ltc2947_show_value, > > + ltc2947_set_value, LTC2947_POWER_INPUT); > > +static SENSOR_DEVICE_ATTR(power1_max, 0644, ltc2947_show_value, > > + ltc2947_set_value, LTC2947_POWER_THRE_H); > > +static SENSOR_DEVICE_ATTR(power1_min, 0644, ltc2947_show_value, > > + ltc2947_set_value, LTC2947_POWER_THRE_L); > > +static SENSOR_DEVICE_ATTR(power1_input_highest, 0444, > > ltc2947_show_value, > > + ltc2947_set_value, LTC2947_POWER_HIGHEST); > > +static SENSOR_DEVICE_ATTR(power1_input_lowest, 0444, > > ltc2947_show_value, > > + ltc2947_set_value, LTC2947_POWER_LOWEST); > > +/* energy attributes */ > > +static SENSOR_DEVICE_ATTR(energy1_input, 0444, ltc2947_show_value, > > NULL, > > + LTC2947_ENERGY1_INPUT); > > +static SENSOR_DEVICE_ATTR(energy1_max, 0644, ltc2947_show_value, > > + ltc2947_set_value, LTC2947_ENERGY1_THRE_H); > > +static SENSOR_DEVICE_ATTR(energy1_min, 0644, ltc2947_show_value, > > + ltc2947_set_value, LTC2947_ENERGY1_THRE_L); > > +static SENSOR_DEVICE_ATTR(energy1_max_alarm, 0444, > > ltc2947_show_alert, NULL, > > + LTC2947_ENERGY1_MAX_ALARM); > > +static SENSOR_DEVICE_ATTR(energy1_min_alarm, 0444, > > ltc2947_show_alert, NULL, > > + LTC2947_ENERGY1_MIN_ALARM); > > +static SENSOR_DEVICE_ATTR(energy2_input, 0444, ltc2947_show_value, > > NULL, > > + LTC2947_ENERGY2_INPUT); > > +static SENSOR_DEVICE_ATTR(energy2_max, 0644, ltc2947_show_value, > > + ltc2947_set_value, LTC2947_ENERGY2_THRE_H); > > +static SENSOR_DEVICE_ATTR(energy2_min, 0644, ltc2947_show_value, > > + ltc2947_set_value, LTC2947_ENERGY2_THRE_L); > > +static SENSOR_DEVICE_ATTR(energy2_max_alarm, 0444, > > ltc2947_show_alert, NULL, > > + LTC2947_ENERGY2_MAX_ALARM); > > +static SENSOR_DEVICE_ATTR(energy2_min_alarm, 0444, > > ltc2947_show_alert, NULL, > > + LTC2947_ENERGY2_MIN_ALARM); > > +/* > > + * Overflow attributes indicate that the readings of the > > accumulated result > > + * registers are invalid. Furthermore it should indicate invalid > > TBCTL settings > > + */ > > +static SENSOR_DEVICE_ATTR(energy1_overflow_alarm, 0444, > > ltc2947_show_alert, > > + NULL, LTC2947_ENERGY1_OVERF_ALARM); > > +static SENSOR_DEVICE_ATTR(energy2_overflow_alarm, 0444, > > ltc2947_show_alert, > > + NULL, LTC2947_ENERGY2_OVERF_ALARM); > > + > > +/* > > + * Fault attributes indicate that the readings in the respective > > channel are > > + * not to be trusted > > + */ > > +static SENSOR_DEVICE_ATTR(energy1_fault, 0444, ltc2947_show_alert, > > + NULL, LTC2947_ENERGY_FAULT); > > +static SENSOR_DEVICE_ATTR(energy2_fault, 0444, ltc2947_show_alert, > > + NULL, LTC2947_ENERGY_FAULT); > > +static SENSOR_DEVICE_ATTR(in0_fault, 0444, ltc2947_show_alert, > > + NULL, LTC2947_FAULT); > > +static SENSOR_DEVICE_ATTR(in1_fault, 0444, ltc2947_show_alert, > > + NULL, LTC2947_FAULT); > > +static SENSOR_DEVICE_ATTR(curr1_fault, 0444, ltc2947_show_alert, > > + NULL, LTC2947_FAULT); > > +static SENSOR_DEVICE_ATTR(temp1_fault, 0444, ltc2947_show_alert, > > + NULL, LTC2947_FAULT); > > +static SENSOR_DEVICE_ATTR(power1_fault, 0444, ltc2947_show_alert, > > + NULL, LTC2947_FAULT); > > + > > +static struct attribute *ltc2947_attrs[] = { > > + &sensor_dev_attr_in0_fault.dev_attr.attr, > > + &sensor_dev_attr_in1_fault.dev_attr.attr, > > + &sensor_dev_attr_curr1_fault.dev_attr.attr, > > + &sensor_dev_attr_temp1_fault.dev_attr.attr, > > + &sensor_dev_attr_power1_input.dev_attr.attr, > > + &sensor_dev_attr_power1_max.dev_attr.attr, > > + &sensor_dev_attr_power1_min.dev_attr.attr, > > + &sensor_dev_attr_power1_input_highest.dev_attr.attr, > > + &sensor_dev_attr_power1_input_lowest.dev_attr.attr, > > + &sensor_dev_attr_power1_fault.dev_attr.attr, > > + &sensor_dev_attr_energy1_input.dev_attr.attr, > > + &sensor_dev_attr_energy1_max.dev_attr.attr, > > + &sensor_dev_attr_energy1_min.dev_attr.attr, > > + &sensor_dev_attr_energy1_max_alarm.dev_attr.attr, > > + &sensor_dev_attr_energy1_min_alarm.dev_attr.attr, > > + &sensor_dev_attr_energy2_input.dev_attr.attr, > > + &sensor_dev_attr_energy2_max.dev_attr.attr, > > + &sensor_dev_attr_energy2_min.dev_attr.attr, > > + &sensor_dev_attr_energy2_max_alarm.dev_attr.attr, > > + &sensor_dev_attr_energy2_min_alarm.dev_attr.attr, > > + &sensor_dev_attr_energy1_overflow_alarm.dev_attr.attr, > > + &sensor_dev_attr_energy2_overflow_alarm.dev_attr.attr, These overflow attributes are kind of an alarm for the energy ones. It tells that the energy registers are about to overflow. I guess that some application can easily find out the maximum values supported on these registers and implement whatever logic they want in the app itself. So, if you prefer I can just drop this ones? > > + &sensor_dev_attr_energy1_fault.dev_attr.attr, > > + &sensor_dev_attr_energy2_fault.dev_attr.attr, > > Some of those are non-standard attributes. You'll have > to explain each in detail, especially why it makes sense > to provide such attributes to the user and why you can't > use standard attributes instead. Also, for the _fault > attributes, I don't entirely see the point. If the fault bit > is set, ADC readings are not valid because supply voltage > is low. This means that ADC register reads will be invalid. > What is the point of having a non-standard attribute - which > likely will be ignored - instead of returning an error when > an attempt is made to read an ADC value ? I was also not sure on this *_fault attributes. They are there to tell that the readings are invalid. Now that I think about it, I'm not sure if it even makes sense to return error if this bit is set. The part is in continuous mode so, it might happen that we have the fault bit set for a short time but afterwards things go normal and the bit will still be set until we read it. So my point is, we might be returning error for a conversion that happened way before our current reading. Any suggestion here? Would you be fine if I just drop this attributes? > Others, like energy1_input, or most of the power attributes, > are standard attributes. Please explain the reasoning for > not using the standard API for those. This ones were because we need 64bit variables. For energy, the part also supports the alarms, max and min attributes so I included them. > I am stoppping with the review at this point. We will have > to discuss all non-standard attributes first. > > Guenter > > > + NULL, > > +}; > > +ATTRIBUTE_GROUPS(ltc2947); > > + > > +static void ltc2947_clk_disable(void *data) > > +{ > > + struct clk *extclk = data; > > + > > + clk_disable_unprepare(extclk); > > +} > > + > > +static int ltc2947_setup(struct ltc2947_data *st) > > +{ > > + int ret; > > + struct clk *extclk; > > + u32 dummy, deadband, pol; > > + u32 accum[2]; > > + > > + /* clear status register by reading it */ > > + ret = regmap_read(st->map, LTC2947_REG_STATUS, &dummy); > > + if (ret) > > + return ret; > > + > > + /* check external clock presence */ > > + extclk = devm_clk_get(st->dev, NULL); > > + if (!IS_ERR(extclk)) { > > + unsigned long rate_hz; > > + u8 pre = 0, div, tbctl; > > + u64 aux; > > + > > + /* let's calculate and set the right valus in TBCTL */ > > + rate_hz = clk_get_rate(extclk); > > + if (rate_hz < LTC2947_CLK_MIN || rate_hz > > > LTC2947_CLK_MAX) { > > + dev_err(st->dev, "Invalid rate:%lu for external > > clock", > > + rate_hz); > > + return -EINVAL; > > + } > > + > > + ret = clk_prepare_enable(extclk); > > + if (ret) > > + return ret; > > + > > + ret = devm_add_action_or_reset(st->dev, > > ltc2947_clk_disable, > > + extclk); > > + if (ret) > > + return ret; > > + /* as in table 1 of the datasheet */ > > + if (rate_hz >= LTC2947_CLK_MIN && rate_hz <= 1000000) > > + pre = 0; > > + else if (rate_hz > 1000000 && rate_hz <= 2000000) > > + pre = 1; > > + else if (rate_hz > 2000000 && rate_hz <= 4000000) > > + pre = 2; > > + else if (rate_hz > 4000000 && rate_hz <= 8000000) > > + pre = 3; > > + else if (rate_hz > 8000000 && rate_hz <= 16000000) > > + pre = 4; > > + else if (rate_hz > 16000000 && rate_hz <= > > LTC2947_CLK_MAX) > > + pre = 5; > > + /* > > + * Div is given by: > > + * floor(fref / (2^PRE * 32768)) > > + */ > > + div = rate_hz / ((1 << pre) * 32768); > > + tbctl = LTC2947_PRE(pre) | LTC2947_DIV(div); > > + > > + ret = regmap_write(st->map, LTC2947_REG_TBCTL, tbctl); > > + if (ret) > > + return ret; > > + /* > > + * The energy lsb is given by (in W*s): > > + * 06416 * (1/fref) * 2^PRE * (DIV + 1) > > + * The value is multiplied by 10E9 > > + */ > > + aux = (div + 1) * ((1 << pre) * 641600000ULL); > > + st->lsb_energy = DIV_ROUND_CLOSEST_ULL(aux, rate_hz); > > + } else { > > + /* 19.89E-6 * 10E9 */ > > + st->lsb_energy = 19890; > > + } > > + ret = of_property_read_u32_array(st->dev->of_node, > > + "adi,accumulator-ctl-pol", > > accum, > > + ARRAY_SIZE(accum)); > > + if (!ret) { > > + u32 accum_reg = LTC2947_ACCUM_POL_1(accum[0]) | > > + LTC2947_ACCUM_POL_2(accum[1]); > > + > > + ret = regmap_write(st->map, LTC2947_REG_ACCUM_POL, > > accum_reg); > > + if (ret) > > + return ret; > > + } > > + ret = of_property_read_u32(st->dev->of_node, > > + "adi,accumulation-deadband- > > microamp", > > + &deadband); > > + if (!ret) { > > + /* the LSB is the same as the current, so 3mA */ > > + ret = regmap_write(st->map, LTC2947_REG_ACCUM_DEADBAND, > > + deadband / (1000 * 3)); > > + if (ret) > > + return ret; > > + } > > + /* check gpio cfg */ > > + ret = of_property_read_u32(st->dev->of_node, "adi,gpio-out- > > pol", &pol); > > + if (!ret) { > > + /* setup GPIO as output */ > > + u32 gpio_ctl = LTC2947_GPIO_EN(1) | > > LTC2947_GPIO_FAN_EN(1) | > > + LTC2947_GPIO_FAN_POL(pol); > > + > > + st->gpio_out = true; > > + ret = regmap_write(st->map, LTC2947_REG_GPIOSTATCTL, > > gpio_ctl); > > + if (ret) > > + return ret; > > + } > > + ret = of_property_read_u32_array(st->dev->of_node, "adi,gpio- > > in-accum", > > + accum, ARRAY_SIZE(accum)); > > + if (!ret) { > > + /* > > + * Setup the accum options. The gpioctl is already > > defined as > > + * input by default. > > + */ > > + u32 accum_val = LTC2947_ACCUM_POL_1(accum[0]) | > > + LTC2947_ACCUM_POL_2(accum[1]); > > + > > + if (st->gpio_out) { > > + dev_err(st->dev, > > + "Cannot have input gpio config if > > already configured as output"); > > + return -EINVAL; > > + } > > + > > + ret = regmap_write(st->map, LTC2947_REG_GPIO_ACCUM, > > accum_val); > > + if (ret) > > + return ret; > > + } > > + > > + /* set continuos mode */ > > + return regmap_update_bits(st->map, LTC2947_REG_CTRL, > > + LTC2947_CONT_MODE_MASK, > > LTC2947_CONT_MODE(1)); > > +} > > + > > +int ltc2947_core_probe(struct regmap *map, const char *name) > > +{ > > + struct ltc2947_data *st; > > + struct device *dev = regmap_get_device(map); > > + struct device *hwmon; > > + int ret; > > + > > + st = devm_kzalloc(dev, sizeof(*st), GFP_KERNEL); > > + if (!st) > > + return -ENOMEM; > > + > > + st->map = map; > > + st->dev = dev; > > + dev_set_drvdata(dev, st); > > + mutex_init(&st->lock); > > + > > + ret = ltc2947_setup(st); > > + if (ret) > > + return ret; > > + > > + hwmon = devm_hwmon_device_register_with_info(dev, name, st, > > + <c2947_chip_info > > , > > + ltc2947_groups); > > + return PTR_ERR_OR_ZERO(hwmon); > > +} > > +EXPORT_SYMBOL_GPL(ltc2947_core_probe); > > + > > +static int __maybe_unused ltc2947_resume(struct device *dev) > > +{ > > + struct ltc2947_data *st = dev_get_drvdata(dev); > > + u32 ctrl = 0; > > + int ret; > > + > > + mutex_lock(&st->lock); > > + /* dummy read to wake the device */ > > + ret = regmap_read(st->map, LTC2947_REG_CTRL, &ctrl); > > + if (ret) > > + goto unlock; > > + > > + /* > > + * Wait for the device. It takes 100ms to wake up so, 10ms > > extra > > + * should be enough. > > + */ > > + msleep(110); > > + ret = regmap_read(st->map, LTC2947_REG_CTRL, &ctrl); > > + if (ret) > > + goto unlock; > > + /* ctrl should be 0 */ > > + if (ctrl != 0) { > > + dev_err(st->dev, "Device failed to wake up, > > ctl:%02X\n", ctrl); > > + ret = -ETIMEDOUT; > > + goto unlock; > > + } > > + > > + st->reset = false; > > + /* set continuous mode */ > > + ret = regmap_update_bits(st->map, LTC2947_REG_CTRL, > > + LTC2947_CONT_MODE_MASK, > > LTC2947_CONT_MODE(1)); > > +unlock: > > + mutex_unlock(&st->lock); > > + return ret; > > +} > > + > > +static int __maybe_unused ltc2947_suspend(struct device *dev) > > +{ > > + struct ltc2947_data *st = dev_get_drvdata(dev); > > + int ret; > > + > > + mutex_lock(&st->lock); > > + ret = regmap_update_bits(st->map, LTC2947_REG_CTRL, > > + LTC2947_SHUTDOWN_MASK, 1); > > + if (ret) > > + goto unlock; > > + > > + st->reset = true; > > +unlock: > > + mutex_unlock(&st->lock); > > + return ret; > > +} > > + > > +SIMPLE_DEV_PM_OPS(ltc2947_pm_ops, ltc2947_suspend, > > ltc2947_resume); > > +EXPORT_SYMBOL_GPL(ltc2947_pm_ops); > > + > > +const struct of_device_id ltc2947_of_match[] = { > > + { .compatible = "adi,ltc2947" }, > > + {} > > +}; > > +EXPORT_SYMBOL_GPL(ltc2947_of_match); > > +MODULE_DEVICE_TABLE(of, ltc2947_of_match); > > + > > +MODULE_AUTHOR("Nuno Sa <nuno.sa@analog.com>"); > > +MODULE_DESCRIPTION("LTC2947 power and energy monitor core > > driver"); > > +MODULE_LICENSE("GPL"); > > diff --git a/drivers/hwmon/ltc2947-i2c.c b/drivers/hwmon/ltc2947- > > i2c.c > > new file mode 100644 > > index 000000000000..cf6074b110ae > > --- /dev/null > > +++ b/drivers/hwmon/ltc2947-i2c.c > > @@ -0,0 +1,49 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Analog Devices LTC2947 high precision power and energy monitor > > over I2C > > + * > > + * Copyright 2019 Analog Devices Inc. > > + */ > > +#include <linux/i2c.h> > > +#include <linux/module.h> > > +#include <linux/regmap.h> > > + > > +#include "ltc2947.h" > > + > > +static const struct regmap_config ltc2947_regmap_config = { > > + .reg_bits = 8, > > + .val_bits = 8, > > +}; > > + > > +static int ltc2947_probe(struct i2c_client *i2c, > > + const struct i2c_device_id *id) > > +{ > > + struct regmap *map; > > + > > + map = devm_regmap_init_i2c(i2c, <c2947_regmap_config); > > + if (IS_ERR(map)) > > + return PTR_ERR(map); > > + > > + return ltc2947_core_probe(map, i2c->name); > > +} > > + > > +static const struct i2c_device_id ltc2947_id[] = { > > + {"ltc2947", 0}, > > + {} > > +}; > > +MODULE_DEVICE_TABLE(i2c, ltc2947_id); > > + > > +static struct i2c_driver ltc2947_driver = { > > + .driver = { > > + .name = "ltc2947", > > + .of_match_table = ltc2947_of_match, > > + .pm = <c2947_pm_ops, > > + }, > > + .probe = ltc2947_probe, > > + .id_table = ltc2947_id, > > +}; > > +module_i2c_driver(ltc2947_driver); > > + > > +MODULE_AUTHOR("Nuno Sa <nuno.sa@analog.com>"); > > +MODULE_DESCRIPTION("LTC2947 I2C power and energy monitor driver"); > > +MODULE_LICENSE("GPL"); > > diff --git a/drivers/hwmon/ltc2947-spi.c b/drivers/hwmon/ltc2947- > > spi.c > > new file mode 100644 > > index 000000000000..c24ca569db1b > > --- /dev/null > > +++ b/drivers/hwmon/ltc2947-spi.c > > @@ -0,0 +1,50 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Analog Devices LTC2947 high precision power and energy monitor > > over SPI > > + * > > + * Copyright 2019 Analog Devices Inc. > > + */ > > +#include <linux/module.h> > > +#include <linux/of.h> > > +#include <linux/regmap.h> > > +#include <linux/spi/spi.h> > > + > > +#include "ltc2947.h" > > + > > +static const struct regmap_config ltc2947_regmap_config = { > > + .reg_bits = 16, > > + .val_bits = 8, > > + .read_flag_mask = BIT(0), > > +}; > > + > > +static int ltc2947_probe(struct spi_device *spi) > > +{ > > + struct regmap *map; > > + > > + map = devm_regmap_init_spi(spi, <c2947_regmap_config); > > + if (IS_ERR(map)) > > + return PTR_ERR(map); > > + > > + return ltc2947_core_probe(map, spi_get_device_id(spi)->name); > > +} > > + > > +static const struct spi_device_id ltc2947_id[] = { > > + {"ltc2947", 0}, > > + {} > > +}; > > +MODULE_DEVICE_TABLE(spi, ltc2947_id); > > + > > +static struct spi_driver ltc2947_driver = { > > + .driver = { > > + .name = "ltc2947", > > + .of_match_table = ltc2947_of_match, > > + .pm = <c2947_pm_ops, > > + }, > > + .probe = ltc2947_probe, > > + .id_table = ltc2947_id, > > +}; > > +module_spi_driver(ltc2947_driver); > > + > > +MODULE_AUTHOR("Nuno Sa <nuno.sa@analog.com>"); > > +MODULE_DESCRIPTION("LTC2947 SPI power and energy monitor driver"); > > +MODULE_LICENSE("GPL"); > > diff --git a/drivers/hwmon/ltc2947.h b/drivers/hwmon/ltc2947.h > > new file mode 100644 > > index 000000000000..5b8ff81a3dba > > --- /dev/null > > +++ b/drivers/hwmon/ltc2947.h > > @@ -0,0 +1,12 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +#ifndef _LINUX_LTC2947_H > > +#define _LINUX_LTC2947_H > > + > > +struct regmap; > > + > > +extern const struct of_device_id ltc2947_of_match[]; > > +extern const struct dev_pm_ops ltc2947_pm_ops; > > + > > +int ltc2947_core_probe(struct regmap *map, const char *name); > > + > > +#endif ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/3] hwmon: Add support for ltc2947 2019-10-04 7:45 ` Sa, Nuno @ 2019-10-04 15:06 ` Guenter Roeck 2019-10-07 12:25 ` Sa, Nuno 0 siblings, 1 reply; 21+ messages in thread From: Guenter Roeck @ 2019-10-04 15:06 UTC (permalink / raw) To: Sa, Nuno Cc: corbet, linux-hwmon, devicetree, linux-doc, mark.rutland, robh+dt, jdelvare On Fri, Oct 04, 2019 at 07:45:07AM +0000, Sa, Nuno wrote: [ ... ] > > > +static int ltc2947_val_read(struct ltc2947_data *st, const u8 reg, > > > + const u8 page, const size_t size, s64 *val) > > > +{ > > > + int ret; > > > + u64 __val = 0; > > > + > > > + mutex_lock(&st->lock); > > > + On a side note, suspend code is supposed to be atomic, If this lock is held, the process or thread holding it will likely stall suspend since it won't run. At the very least, this would have to be a trylock, with suspend failing if the lock can not be acquired. > > > + if (st->reset) { > > > + mutex_unlock(&st->lock); > > > + return -EPERM; > > > > Not sure what the error here should be, but EPERM is not correct. > > > > Under which conditions would this function be called while in > > suspend ? > > Honestly, this is more like a sanity check. I'm not sure if we can get > here in suspend mode. Don't userland apps can still run in suspend? I > guess so but I'm not 100% sure on this. Do you have any recommendation > for the error here? > Sorry, I won't accept "just in case" code. Having said that, please inform yourself how suspend works. Userland code has to be stopped before any hardware can be disabled. Similar, hardware has to be re-enabled before userland code can resume. Otherwise the kernel would crash all over the place. In many cases, disabling the hardware means that trying to access hardware registers will cause an acess fault. [...] > > > + > > > +static struct attribute *ltc2947_attrs[] = { > > > + &sensor_dev_attr_in0_fault.dev_attr.attr, > > > + &sensor_dev_attr_in1_fault.dev_attr.attr, > > > + &sensor_dev_attr_curr1_fault.dev_attr.attr, > > > + &sensor_dev_attr_temp1_fault.dev_attr.attr, > > > + &sensor_dev_attr_power1_input.dev_attr.attr, > > > + &sensor_dev_attr_power1_max.dev_attr.attr, > > > + &sensor_dev_attr_power1_min.dev_attr.attr, > > > + &sensor_dev_attr_power1_input_highest.dev_attr.attr, > > > + &sensor_dev_attr_power1_input_lowest.dev_attr.attr, > > > + &sensor_dev_attr_power1_fault.dev_attr.attr, > > > + &sensor_dev_attr_energy1_input.dev_attr.attr, > > > + &sensor_dev_attr_energy1_max.dev_attr.attr, > > > + &sensor_dev_attr_energy1_min.dev_attr.attr, > > > + &sensor_dev_attr_energy1_max_alarm.dev_attr.attr, > > > + &sensor_dev_attr_energy1_min_alarm.dev_attr.attr, > > > + &sensor_dev_attr_energy2_input.dev_attr.attr, > > > + &sensor_dev_attr_energy2_max.dev_attr.attr, > > > + &sensor_dev_attr_energy2_min.dev_attr.attr, > > > + &sensor_dev_attr_energy2_max_alarm.dev_attr.attr, > > > + &sensor_dev_attr_energy2_min_alarm.dev_attr.attr, > > > + &sensor_dev_attr_energy1_overflow_alarm.dev_attr.attr, > > > + &sensor_dev_attr_energy2_overflow_alarm.dev_attr.attr, > > These overflow attributes are kind of an alarm for the energy ones. It > tells that the energy registers are about to overflow. I guess that > some application can easily find out the maximum values supported on > these registers and implement whatever logic they want in the app > itself. So, if you prefer I can just drop this ones? > I understand the overflow use case. However, the mere presence of min/max attributes for energy attributes suggests that this is not the min/max use case for hwmon attributes. There is no "minimum" or "maximum" energy for an accumulating value. Such attributes only make sense in an application abler to measure available energy (eg a battery controller). I'll have to read the chip specification to understand the intended use case. > > > + &sensor_dev_attr_energy1_fault.dev_attr.attr, > > > + &sensor_dev_attr_energy2_fault.dev_attr.attr, > > > > Some of those are non-standard attributes. You'll have > > to explain each in detail, especially why it makes sense > > to provide such attributes to the user and why you can't > > use standard attributes instead. Also, for the _fault > > attributes, I don't entirely see the point. If the fault bit > > is set, ADC readings are not valid because supply voltage > > is low. This means that ADC register reads will be invalid. > > What is the point of having a non-standard attribute - which > > likely will be ignored - instead of returning an error when > > an attempt is made to read an ADC value ? > > I was also not sure on this *_fault attributes. They are there to tell > that the readings are invalid. Now that I think about it, I'm not sure > if it even makes sense to return error if this bit is set. The part is > in continuous mode so, it might happen that we have the fault bit set > for a short time but afterwards things go normal and the bit will still > be set until we read it. So my point is, we might be returning error > for a conversion that happened way before our current reading. Any > suggestion here? Would you be fine if I just drop this attributes? > It sounds like "fault" means something like "one of the past readings was wrong, but I don't know which one and I don't know if the wrong value was ever read by user space". Sorry, I fail to see what value those attributes are supposed to have for the user. At best it could mean "please re-read the associated attrribute", but that could as well be accomplished without userspace action if it is really needed. Also, per datasheet, it looks like the fault bit is set of the chip voltage is too low. If that happens, the system has a severe problem which can not be resolved with an attribute visible to userspace. > > Others, like energy1_input, or most of the power attributes, > > are standard attributes. Please explain the reasoning for > > not using the standard API for those. > > This ones were because we need 64bit variables. For energy, the part > also supports the alarms, max and min attributes so I included them. > I can to some degree the logic for energy attributes, but do you really have an application where the chip is used on a 32-bit system and monitors power larger than 2kW ? If you do have such a use case, we'll need to enhance the API to reflect it. Thanks, Guenter ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/3] hwmon: Add support for ltc2947 2019-10-04 15:06 ` Guenter Roeck @ 2019-10-07 12:25 ` Sa, Nuno 2019-10-07 12:44 ` Guenter Roeck 0 siblings, 1 reply; 21+ messages in thread From: Sa, Nuno @ 2019-10-07 12:25 UTC (permalink / raw) To: linux Cc: jdelvare, corbet, linux-hwmon, devicetree, mark.rutland, linux-doc, robh+dt On Fri, 2019-10-04 at 08:06 -0700, Guenter Roeck wrote: > > On Fri, Oct 04, 2019 at 07:45:07AM +0000, Sa, Nuno wrote: > [ ... ] > > > > +static int ltc2947_val_read(struct ltc2947_data *st, const u8 > > > > reg, > > > > + const u8 page, const size_t size, > > > > s64 *val) > > > > +{ > > > > + int ret; > > > > + u64 __val = 0; > > > > + > > > > + mutex_lock(&st->lock); > > > > + > > On a side note, suspend code is supposed to be atomic, > If this lock is held, the process or thread holding it > will likely stall suspend since it won't run. > At the very least, this would have to be a trylock, > with suspend failing if the lock can not be acquired. Got it. Even more, Now I don't see the point of having the mutex in the PM callbacks (though I saw other drivers doing this). As you said, at the very least the trylock should be used since a frozen task might have the mutex locked. Either way, I will drop both the flag and the call to mutex_* functions is suspend()/resume(). > > > > + if (st->reset) { > > > > + mutex_unlock(&st->lock); > > > > + return -EPERM; > > > > > > Not sure what the error here should be, but EPERM is not correct. > > > > > > Under which conditions would this function be called while in > > > suspend ? > > > > Honestly, this is more like a sanity check. I'm not sure if we can > > get > > here in suspend mode. Don't userland apps can still run in suspend? > > I > > guess so but I'm not 100% sure on this. Do you have any > > recommendation > > for the error here? > > > Sorry, I won't accept "just in case" code. > > Having said that, please inform yourself how suspend works. Userland > code > has to be stopped before any hardware can be disabled. Similar, > hardware > has to be re-enabled before userland code can resume. > Otherwise the kernel would crash all over the place. In many cases, > disabling the hardware means that trying to access hardware registers > will cause an acess fault. Yes, you are right. I did checked the suspend code and all userland tasks and kthreads are stopped before the suspend() callback is called for the HW devices. > [...] > > > > > + > > > > +static struct attribute *ltc2947_attrs[] = { > > > > + &sensor_dev_attr_in0_fault.dev_attr.attr, > > > > + &sensor_dev_attr_in1_fault.dev_attr.attr, > > > > + &sensor_dev_attr_curr1_fault.dev_attr.attr, > > > > + &sensor_dev_attr_temp1_fault.dev_attr.attr, > > > > + &sensor_dev_attr_power1_input.dev_attr.attr, > > > > + &sensor_dev_attr_power1_max.dev_attr.attr, > > > > + &sensor_dev_attr_power1_min.dev_attr.attr, > > > > + &sensor_dev_attr_power1_input_highest.dev_attr.attr, > > > > + &sensor_dev_attr_power1_input_lowest.dev_attr.attr, > > > > + &sensor_dev_attr_power1_fault.dev_attr.attr, > > > > + &sensor_dev_attr_energy1_input.dev_attr.attr, > > > > + &sensor_dev_attr_energy1_max.dev_attr.attr, > > > > + &sensor_dev_attr_energy1_min.dev_attr.attr, > > > > + &sensor_dev_attr_energy1_max_alarm.dev_attr.attr, > > > > + &sensor_dev_attr_energy1_min_alarm.dev_attr.attr, > > > > + &sensor_dev_attr_energy2_input.dev_attr.attr, > > > > + &sensor_dev_attr_energy2_max.dev_attr.attr, > > > > + &sensor_dev_attr_energy2_min.dev_attr.attr, > > > > + &sensor_dev_attr_energy2_max_alarm.dev_attr.attr, > > > > + &sensor_dev_attr_energy2_min_alarm.dev_attr.attr, > > > > + &sensor_dev_attr_energy1_overflow_alarm.dev_attr.attr, > > > > + &sensor_dev_attr_energy2_overflow_alarm.dev_attr.attr, > > > > These overflow attributes are kind of an alarm for the energy ones. > > It > > tells that the energy registers are about to overflow. I guess that > > some application can easily find out the maximum values supported > > on > > these registers and implement whatever logic they want in the app > > itself. So, if you prefer I can just drop this ones? > > > I understand the overflow use case. However, the mere presence > of min/max attributes for energy attributes suggests that this > is not the min/max use case for hwmon attributes. There is no > "minimum" > or "maximum" energy for an accumulating value. Such attributes > only make sense in an application abler to measure available > energy (eg a battery controller). I'll have to read the chip > specification to understand the intended use case. Should I just drop the overflow attributes? I think the part can be used to check battery charging efficiency for example (though I guess we would need to also support the Charge register's). > > > > + &sensor_dev_attr_energy1_fault.dev_attr.attr, > > > > + &sensor_dev_attr_energy2_fault.dev_attr.attr, > > > > > > Some of those are non-standard attributes. You'll have > > > to explain each in detail, especially why it makes sense > > > to provide such attributes to the user and why you can't > > > use standard attributes instead. Also, for the _fault > > > attributes, I don't entirely see the point. If the fault bit > > > is set, ADC readings are not valid because supply voltage > > > is low. This means that ADC register reads will be invalid. > > > What is the point of having a non-standard attribute - which > > > likely will be ignored - instead of returning an error when > > > an attempt is made to read an ADC value ? > > > > I was also not sure on this *_fault attributes. They are there to > > tell > > that the readings are invalid. Now that I think about it, I'm not > > sure > > if it even makes sense to return error if this bit is set. The part > > is > > in continuous mode so, it might happen that we have the fault bit > > set > > for a short time but afterwards things go normal and the bit will > > still > > be set until we read it. So my point is, we might be returning > > error > > for a conversion that happened way before our current reading. Any > > suggestion here? Would you be fine if I just drop this attributes? > > > > It sounds like "fault" means something like "one of the past readings > was wrong, but I don't know which one and I don't know if the wrong > value was ever read by user space". Sorry, I fail to see what value > those attributes are supposed to have for the user. At best it could > mean "please re-read the associated attrribute", but that could as > well > be accomplished without userspace action if it is really needed. > Also, per datasheet, it looks like the fault bit is set of the chip > voltage is too low. If that happens, the system has a severe problem > which can not be resolved with an attribute visible to userspace. I will drop the fault attributes. > > > Others, like energy1_input, or most of the power attributes, > > > are standard attributes. Please explain the reasoning for > > > not using the standard API for those. > > > > This ones were because we need 64bit variables. For energy, the > > part > > also supports the alarms, max and min attributes so I included > > them. > > > I can to some degree the logic for energy attributes, but do you > really > have an application where the chip is used on a 32-bit system and > monitors power larger than 2kW ? Hmm, I looked again at the chip specification and unless I'm missing something obvious the part can only measure +/- 30A and 0-15V giving us +/- 450W which definitely fits in a long variable. The only thing that will be truncated is the min/max values. The part, by default, has this value to 0x7fff and 0x8000 which times 200000uW (part scale) will be truncated. Now, we can argue that this max/min values are not real and the user is expected to write this attributes with some meaningful values? How do you suggest to proceed? Should I just use standard attributes for power? > If you do have such a use case, we'll need to enhance the API > to reflect it. > > Thanks, > Guenter ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/3] hwmon: Add support for ltc2947 2019-10-07 12:25 ` Sa, Nuno @ 2019-10-07 12:44 ` Guenter Roeck 2019-10-07 14:51 ` Sa, Nuno 0 siblings, 1 reply; 21+ messages in thread From: Guenter Roeck @ 2019-10-07 12:44 UTC (permalink / raw) To: Sa, Nuno Cc: jdelvare, corbet, linux-hwmon, devicetree, mark.rutland, linux-doc, robh+dt On 10/7/19 5:25 AM, Sa, Nuno wrote: > On Fri, 2019-10-04 at 08:06 -0700, Guenter Roeck wrote: >> >> On Fri, Oct 04, 2019 at 07:45:07AM +0000, Sa, Nuno wrote: >> [ ... ] >>>>> +static int ltc2947_val_read(struct ltc2947_data *st, const u8 >>>>> reg, >>>>> + const u8 page, const size_t size, >>>>> s64 *val) >>>>> +{ >>>>> + int ret; >>>>> + u64 __val = 0; >>>>> + >>>>> + mutex_lock(&st->lock); >>>>> + >> >> On a side note, suspend code is supposed to be atomic, >> If this lock is held, the process or thread holding it >> will likely stall suspend since it won't run. >> At the very least, this would have to be a trylock, >> with suspend failing if the lock can not be acquired. > > Got it. Even more, Now I don't see the point of having the mutex in the > PM callbacks (though I saw other drivers doing this). As you said, at > the very least the trylock should be used since a frozen task might > have the mutex locked. Either way, I will drop both the flag and the > call to mutex_* functions is suspend()/resume(). > >>>>> + if (st->reset) { >>>>> + mutex_unlock(&st->lock); >>>>> + return -EPERM; >>>> >>>> Not sure what the error here should be, but EPERM is not correct. >>>> >>>> Under which conditions would this function be called while in >>>> suspend ? >>> >>> Honestly, this is more like a sanity check. I'm not sure if we can >>> get >>> here in suspend mode. Don't userland apps can still run in suspend? >>> I >>> guess so but I'm not 100% sure on this. Do you have any >>> recommendation >>> for the error here? >>> >> Sorry, I won't accept "just in case" code. >> >> Having said that, please inform yourself how suspend works. Userland >> code >> has to be stopped before any hardware can be disabled. Similar, >> hardware >> has to be re-enabled before userland code can resume. >> Otherwise the kernel would crash all over the place. In many cases, >> disabling the hardware means that trying to access hardware registers >> will cause an acess fault. > > Yes, you are right. I did checked the suspend code and all userland > tasks and kthreads are stopped before the suspend() callback is called > for the HW devices. > >> [...] >> >>>>> + >>>>> +static struct attribute *ltc2947_attrs[] = { >>>>> + &sensor_dev_attr_in0_fault.dev_attr.attr, >>>>> + &sensor_dev_attr_in1_fault.dev_attr.attr, >>>>> + &sensor_dev_attr_curr1_fault.dev_attr.attr, >>>>> + &sensor_dev_attr_temp1_fault.dev_attr.attr, >>>>> + &sensor_dev_attr_power1_input.dev_attr.attr, >>>>> + &sensor_dev_attr_power1_max.dev_attr.attr, >>>>> + &sensor_dev_attr_power1_min.dev_attr.attr, >>>>> + &sensor_dev_attr_power1_input_highest.dev_attr.attr, >>>>> + &sensor_dev_attr_power1_input_lowest.dev_attr.attr, >>>>> + &sensor_dev_attr_power1_fault.dev_attr.attr, >>>>> + &sensor_dev_attr_energy1_input.dev_attr.attr, >>>>> + &sensor_dev_attr_energy1_max.dev_attr.attr, >>>>> + &sensor_dev_attr_energy1_min.dev_attr.attr, >>>>> + &sensor_dev_attr_energy1_max_alarm.dev_attr.attr, >>>>> + &sensor_dev_attr_energy1_min_alarm.dev_attr.attr, >>>>> + &sensor_dev_attr_energy2_input.dev_attr.attr, >>>>> + &sensor_dev_attr_energy2_max.dev_attr.attr, >>>>> + &sensor_dev_attr_energy2_min.dev_attr.attr, >>>>> + &sensor_dev_attr_energy2_max_alarm.dev_attr.attr, >>>>> + &sensor_dev_attr_energy2_min_alarm.dev_attr.attr, >>>>> + &sensor_dev_attr_energy1_overflow_alarm.dev_attr.attr, >>>>> + &sensor_dev_attr_energy2_overflow_alarm.dev_attr.attr, >>> >>> These overflow attributes are kind of an alarm for the energy ones. >>> It >>> tells that the energy registers are about to overflow. I guess that >>> some application can easily find out the maximum values supported >>> on >>> these registers and implement whatever logic they want in the app >>> itself. So, if you prefer I can just drop this ones? >>> >> I understand the overflow use case. However, the mere presence >> of min/max attributes for energy attributes suggests that this >> is not the min/max use case for hwmon attributes. There is no >> "minimum" >> or "maximum" energy for an accumulating value. Such attributes >> only make sense in an application abler to measure available >> energy (eg a battery controller). I'll have to read the chip >> specification to understand the intended use case. > > Should I just drop the overflow attributes? I think the part can be > used to check battery charging efficiency for example (though I guess > we would need to also support the Charge register's). > If chip is (or can be) used as charger, it should register as such. Note my question was the energy limit attributes, not the overflow attributes. >>>>> + &sensor_dev_attr_energy1_fault.dev_attr.attr, >>>>> + &sensor_dev_attr_energy2_fault.dev_attr.attr, >>>> >>>> Some of those are non-standard attributes. You'll have >>>> to explain each in detail, especially why it makes sense >>>> to provide such attributes to the user and why you can't >>>> use standard attributes instead. Also, for the _fault >>>> attributes, I don't entirely see the point. If the fault bit >>>> is set, ADC readings are not valid because supply voltage >>>> is low. This means that ADC register reads will be invalid. >>>> What is the point of having a non-standard attribute - which >>>> likely will be ignored - instead of returning an error when >>>> an attempt is made to read an ADC value ? >>> >>> I was also not sure on this *_fault attributes. They are there to >>> tell >>> that the readings are invalid. Now that I think about it, I'm not >>> sure >>> if it even makes sense to return error if this bit is set. The part >>> is >>> in continuous mode so, it might happen that we have the fault bit >>> set >>> for a short time but afterwards things go normal and the bit will >>> still >>> be set until we read it. So my point is, we might be returning >>> error >>> for a conversion that happened way before our current reading. Any >>> suggestion here? Would you be fine if I just drop this attributes? >>> >> >> It sounds like "fault" means something like "one of the past readings >> was wrong, but I don't know which one and I don't know if the wrong >> value was ever read by user space". Sorry, I fail to see what value >> those attributes are supposed to have for the user. At best it could >> mean "please re-read the associated attrribute", but that could as >> well >> be accomplished without userspace action if it is really needed. >> Also, per datasheet, it looks like the fault bit is set of the chip >> voltage is too low. If that happens, the system has a severe problem >> which can not be resolved with an attribute visible to userspace. > > I will drop the fault attributes. > >>>> Others, like energy1_input, or most of the power attributes, >>>> are standard attributes. Please explain the reasoning for >>>> not using the standard API for those. >>> >>> This ones were because we need 64bit variables. For energy, the >>> part >>> also supports the alarms, max and min attributes so I included >>> them. >>> >> I can to some degree the logic for energy attributes, but do you >> really >> have an application where the chip is used on a 32-bit system and >> monitors power larger than 2kW ? > > Hmm, I looked again at the chip specification and unless I'm missing > something obvious the part can only measure +/- 30A and 0-15V giving us > +/- 450W which definitely fits in a long variable. The only thing that > will be truncated is the min/max values. The part, by default, has this > value to 0x7fff and 0x8000 which times 200000uW (part scale) will be > truncated. Now, we can argue that this max/min values are not real and > the user is expected to write this attributes with some meaningful > values? How do you suggest to proceed? Should I just use standard > attributes for power? > How about detecting the overflow on read and just report the maximum supported value ? Or, alternatively, initialize the register with the maximum supported value. Guenter ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/3] hwmon: Add support for ltc2947 2019-10-07 12:44 ` Guenter Roeck @ 2019-10-07 14:51 ` Sa, Nuno 2019-10-10 7:13 ` Sa, Nuno 0 siblings, 1 reply; 21+ messages in thread From: Sa, Nuno @ 2019-10-07 14:51 UTC (permalink / raw) To: linux Cc: linux-doc, corbet, linux-hwmon, devicetree, mark.rutland, jdelvare, robh+dt On Mon, 2019-10-07 at 05:44 -0700, Guenter Roeck wrote: > > On 10/7/19 5:25 AM, Sa, Nuno wrote: > > On Fri, 2019-10-04 at 08:06 -0700, Guenter Roeck wrote: > > > On Fri, Oct 04, 2019 at 07:45:07AM +0000, Sa, Nuno wrote: > > > [ ... ] > > > > > > +static int ltc2947_val_read(struct ltc2947_data *st, const > > > > > > u8 > > > > > > reg, > > > > > > + const u8 page, const size_t size, > > > > > > s64 *val) > > > > > > +{ > > > > > > + int ret; > > > > > > + u64 __val = 0; > > > > > > + > > > > > > + mutex_lock(&st->lock); > > > > > > + > > > > > > On a side note, suspend code is supposed to be atomic, > > > If this lock is held, the process or thread holding it > > > will likely stall suspend since it won't run. > > > At the very least, this would have to be a trylock, > > > with suspend failing if the lock can not be acquired. > > > > Got it. Even more, Now I don't see the point of having the mutex in > > the > > PM callbacks (though I saw other drivers doing this). As you said, > > at > > the very least the trylock should be used since a frozen task might > > have the mutex locked. Either way, I will drop both the flag and > > the > > call to mutex_* functions is suspend()/resume(). > > > > > > > > + if (st->reset) { > > > > > > + mutex_unlock(&st->lock); > > > > > > + return -EPERM; > > > > > > > > > > Not sure what the error here should be, but EPERM is not > > > > > correct. > > > > > > > > > > Under which conditions would this function be called while in > > > > > suspend ? > > > > > > > > Honestly, this is more like a sanity check. I'm not sure if we > > > > can > > > > get > > > > here in suspend mode. Don't userland apps can still run in > > > > suspend? > > > > I > > > > guess so but I'm not 100% sure on this. Do you have any > > > > recommendation > > > > for the error here? > > > > > > > Sorry, I won't accept "just in case" code. > > > > > > Having said that, please inform yourself how suspend works. > > > Userland > > > code > > > has to be stopped before any hardware can be disabled. Similar, > > > hardware > > > has to be re-enabled before userland code can resume. > > > Otherwise the kernel would crash all over the place. In many > > > cases, > > > disabling the hardware means that trying to access hardware > > > registers > > > will cause an acess fault. > > > > Yes, you are right. I did checked the suspend code and all userland > > tasks and kthreads are stopped before the suspend() callback is > > called > > for the HW devices. > > > > > [...] > > > > > > > > > + > > > > > > +static struct attribute *ltc2947_attrs[] = { > > > > > > + &sensor_dev_attr_in0_fault.dev_attr.attr, > > > > > > + &sensor_dev_attr_in1_fault.dev_attr.attr, > > > > > > + &sensor_dev_attr_curr1_fault.dev_attr.attr, > > > > > > + &sensor_dev_attr_temp1_fault.dev_attr.attr, > > > > > > + &sensor_dev_attr_power1_input.dev_attr.attr, > > > > > > + &sensor_dev_attr_power1_max.dev_attr.attr, > > > > > > + &sensor_dev_attr_power1_min.dev_attr.attr, > > > > > > + &sensor_dev_attr_power1_input_highest.dev_attr.attr, > > > > > > + &sensor_dev_attr_power1_input_lowest.dev_attr.attr, > > > > > > + &sensor_dev_attr_power1_fault.dev_attr.attr, > > > > > > + &sensor_dev_attr_energy1_input.dev_attr.attr, > > > > > > + &sensor_dev_attr_energy1_max.dev_attr.attr, > > > > > > + &sensor_dev_attr_energy1_min.dev_attr.attr, > > > > > > + &sensor_dev_attr_energy1_max_alarm.dev_attr.attr, > > > > > > + &sensor_dev_attr_energy1_min_alarm.dev_attr.attr, > > > > > > + &sensor_dev_attr_energy2_input.dev_attr.attr, > > > > > > + &sensor_dev_attr_energy2_max.dev_attr.attr, > > > > > > + &sensor_dev_attr_energy2_min.dev_attr.attr, > > > > > > + &sensor_dev_attr_energy2_max_alarm.dev_attr.attr, > > > > > > + &sensor_dev_attr_energy2_min_alarm.dev_attr.attr, > > > > > > + &sensor_dev_attr_energy1_overflow_alarm.dev_attr.attr, > > > > > > + &sensor_dev_attr_energy2_overflow_alarm.dev_attr.attr, > > > > > > > > These overflow attributes are kind of an alarm for the energy > > > > ones. > > > > It > > > > tells that the energy registers are about to overflow. I guess > > > > that > > > > some application can easily find out the maximum values > > > > supported > > > > on > > > > these registers and implement whatever logic they want in the > > > > app > > > > itself. So, if you prefer I can just drop this ones? > > > > > > > I understand the overflow use case. However, the mere presence > > > of min/max attributes for energy attributes suggests that this > > > is not the min/max use case for hwmon attributes. There is no > > > "minimum" > > > or "maximum" energy for an accumulating value. Such attributes > > > only make sense in an application abler to measure available > > > energy (eg a battery controller). I'll have to read the chip > > > specification to understand the intended use case. > > > > Should I just drop the overflow attributes? I think the part can be > > used to check battery charging efficiency for example (though I > > guess > > we would need to also support the Charge register's). > > > > If chip is (or can be) used as charger, it should register as such. > Note my question was the energy limit attributes, not the overflow > attributes. I don't think it can be used as a charger but it can also measure charge (integrating the measured current over time). As they are not standard attributes I did not included this on the driver (I sent a query on this before starting the driver - https://marc.info/?l=linux-hwmon&m=156507711612877&w=2). I see your point about energy and having maximum and minimum for an accumulated value. Honestly, looking at the chip specification I cannot see the intended use case for this. Maybe for monitoring/characterizing batteries since there are some controls on these accumulated values (we can set the part to accumulate only when current is positive for example). I will do some internal querying to see if I can find out more on this. > > > > > > + &sensor_dev_attr_energy1_fault.dev_attr.attr, > > > > > > + &sensor_dev_attr_energy2_fault.dev_attr.attr, > > > > > > > > > > Some of those are non-standard attributes. You'll have > > > > > to explain each in detail, especially why it makes sense > > > > > to provide such attributes to the user and why you can't > > > > > use standard attributes instead. Also, for the _fault > > > > > attributes, I don't entirely see the point. If the fault bit > > > > > is set, ADC readings are not valid because supply voltage > > > > > is low. This means that ADC register reads will be invalid. > > > > > What is the point of having a non-standard attribute - which > > > > > likely will be ignored - instead of returning an error when > > > > > an attempt is made to read an ADC value ? > > > > > > > > I was also not sure on this *_fault attributes. They are there > > > > to > > > > tell > > > > that the readings are invalid. Now that I think about it, I'm > > > > not > > > > sure > > > > if it even makes sense to return error if this bit is set. The > > > > part > > > > is > > > > in continuous mode so, it might happen that we have the fault > > > > bit > > > > set > > > > for a short time but afterwards things go normal and the bit > > > > will > > > > still > > > > be set until we read it. So my point is, we might be returning > > > > error > > > > for a conversion that happened way before our current reading. > > > > Any > > > > suggestion here? Would you be fine if I just drop this > > > > attributes? > > > > > > > > > > It sounds like "fault" means something like "one of the past > > > readings > > > was wrong, but I don't know which one and I don't know if the > > > wrong > > > value was ever read by user space". Sorry, I fail to see what > > > value > > > those attributes are supposed to have for the user. At best it > > > could > > > mean "please re-read the associated attrribute", but that could > > > as > > > well > > > be accomplished without userspace action if it is really needed. > > > Also, per datasheet, it looks like the fault bit is set of the > > > chip > > > voltage is too low. If that happens, the system has a severe > > > problem > > > which can not be resolved with an attribute visible to userspace. > > > > I will drop the fault attributes. > > > > > > > Others, like energy1_input, or most of the power attributes, > > > > > are standard attributes. Please explain the reasoning for > > > > > not using the standard API for those. > > > > > > > > This ones were because we need 64bit variables. For energy, the > > > > part > > > > also supports the alarms, max and min attributes so I included > > > > them. > > > > > > > I can to some degree the logic for energy attributes, but do you > > > really > > > have an application where the chip is used on a 32-bit system and > > > monitors power larger than 2kW ? > > > > Hmm, I looked again at the chip specification and unless I'm > > missing > > something obvious the part can only measure +/- 30A and 0-15V > > giving us > > +/- 450W which definitely fits in a long variable. The only thing > > that > > will be truncated is the min/max values. The part, by default, has > > this > > value to 0x7fff and 0x8000 which times 200000uW (part scale) will > > be > > truncated. Now, we can argue that this max/min values are not real > > and > > the user is expected to write this attributes with some meaningful > > values? How do you suggest to proceed? Should I just use standard > > attributes for power? > > > How about detecting the overflow on read and just report the maximum > supported value ? Or, alternatively, initialize the register with the > maximum supported value. Sounds good. I will initialize the register's on the setup() phase. > Guenter ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/3] hwmon: Add support for ltc2947 2019-10-07 14:51 ` Sa, Nuno @ 2019-10-10 7:13 ` Sa, Nuno 2019-10-10 15:21 ` Guenter Roeck 0 siblings, 1 reply; 21+ messages in thread From: Sa, Nuno @ 2019-10-10 7:13 UTC (permalink / raw) To: linux Cc: robh+dt, corbet, linux-doc, devicetree, linux-hwmon, jdelvare, mark.rutland Hi Guenter, I got some feedback about energy max/min... On Mon, 2019-10-07 at 14:51 +0000, Sa, Nuno wrote: > On Mon, 2019-10-07 at 05:44 -0700, Guenter Roeck wrote: > > On 10/7/19 5:25 AM, Sa, Nuno wrote: > > > On Fri, 2019-10-04 at 08:06 -0700, Guenter Roeck wrote: > > > > On Fri, Oct 04, 2019 at 07:45:07AM +0000, Sa, Nuno wrote: > > > > [ ... ] > > > > > > > +static int ltc2947_val_read(struct ltc2947_data *st, > > > > > > > const > > > > > > > u8 > > > > > > > reg, > > > > > > > + const u8 page, const size_t size, > > > > > > > s64 *val) > > > > > > > +{ > > > > > > > + int ret; > > > > > > > + u64 __val = 0; > > > > > > > + > > > > > > > + mutex_lock(&st->lock); > > > > > > > + > > > > > > > > On a side note, suspend code is supposed to be atomic, > > > > If this lock is held, the process or thread holding it > > > > will likely stall suspend since it won't run. > > > > At the very least, this would have to be a trylock, > > > > with suspend failing if the lock can not be acquired. > > > > > > Got it. Even more, Now I don't see the point of having the mutex > > > in > > > the > > > PM callbacks (though I saw other drivers doing this). As you > > > said, > > > at > > > the very least the trylock should be used since a frozen task > > > might > > > have the mutex locked. Either way, I will drop both the flag and > > > the > > > call to mutex_* functions is suspend()/resume(). > > > > > > > > > > + if (st->reset) { > > > > > > > + mutex_unlock(&st->lock); > > > > > > > + return -EPERM; > > > > > > > > > > > > Not sure what the error here should be, but EPERM is not > > > > > > correct. > > > > > > > > > > > > Under which conditions would this function be called while > > > > > > in > > > > > > suspend ? > > > > > > > > > > Honestly, this is more like a sanity check. I'm not sure if > > > > > we > > > > > can > > > > > get > > > > > here in suspend mode. Don't userland apps can still run in > > > > > suspend? > > > > > I > > > > > guess so but I'm not 100% sure on this. Do you have any > > > > > recommendation > > > > > for the error here? > > > > > > > > > Sorry, I won't accept "just in case" code. > > > > > > > > Having said that, please inform yourself how suspend works. > > > > Userland > > > > code > > > > has to be stopped before any hardware can be disabled. Similar, > > > > hardware > > > > has to be re-enabled before userland code can resume. > > > > Otherwise the kernel would crash all over the place. In many > > > > cases, > > > > disabling the hardware means that trying to access hardware > > > > registers > > > > will cause an acess fault. > > > > > > Yes, you are right. I did checked the suspend code and all > > > userland > > > tasks and kthreads are stopped before the suspend() callback is > > > called > > > for the HW devices. > > > > > > > [...] > > > > > > > > > > > + > > > > > > > +static struct attribute *ltc2947_attrs[] = { > > > > > > > + &sensor_dev_attr_in0_fault.dev_attr.attr, > > > > > > > + &sensor_dev_attr_in1_fault.dev_attr.attr, > > > > > > > + &sensor_dev_attr_curr1_fault.dev_attr.attr, > > > > > > > + &sensor_dev_attr_temp1_fault.dev_attr.attr, > > > > > > > + &sensor_dev_attr_power1_input.dev_attr.attr, > > > > > > > + &sensor_dev_attr_power1_max.dev_attr.attr, > > > > > > > + &sensor_dev_attr_power1_min.dev_attr.attr, > > > > > > > + &sensor_dev_attr_power1_input_highest.dev_attr.attr, > > > > > > > + &sensor_dev_attr_power1_input_lowest.dev_attr.attr, > > > > > > > + &sensor_dev_attr_power1_fault.dev_attr.attr, > > > > > > > + &sensor_dev_attr_energy1_input.dev_attr.attr, > > > > > > > + &sensor_dev_attr_energy1_max.dev_attr.attr, > > > > > > > + &sensor_dev_attr_energy1_min.dev_attr.attr, > > > > > > > + &sensor_dev_attr_energy1_max_alarm.dev_attr.attr, > > > > > > > + &sensor_dev_attr_energy1_min_alarm.dev_attr.attr, > > > > > > > + &sensor_dev_attr_energy2_input.dev_attr.attr, > > > > > > > + &sensor_dev_attr_energy2_max.dev_attr.attr, > > > > > > > + &sensor_dev_attr_energy2_min.dev_attr.attr, > > > > > > > + &sensor_dev_attr_energy2_max_alarm.dev_attr.attr, > > > > > > > + &sensor_dev_attr_energy2_min_alarm.dev_attr.attr, > > > > > > > + &sensor_dev_attr_energy1_overflow_alarm.dev_attr.attr, > > > > > > > + &sensor_dev_attr_energy2_overflow_alarm.dev_attr.attr, > > > > > > > > > > These overflow attributes are kind of an alarm for the energy > > > > > ones. > > > > > It > > > > > tells that the energy registers are about to overflow. I > > > > > guess > > > > > that > > > > > some application can easily find out the maximum values > > > > > supported > > > > > on > > > > > these registers and implement whatever logic they want in the > > > > > app > > > > > itself. So, if you prefer I can just drop this ones? > > > > > > > > > I understand the overflow use case. However, the mere presence > > > > of min/max attributes for energy attributes suggests that this > > > > is not the min/max use case for hwmon attributes. There is no > > > > "minimum" > > > > or "maximum" energy for an accumulating value. Such attributes > > > > only make sense in an application abler to measure available > > > > energy (eg a battery controller). I'll have to read the chip > > > > specification to understand the intended use case. > > > > > > Should I just drop the overflow attributes? I think the part can > > > be > > > used to check battery charging efficiency for example (though I > > > guess > > > we would need to also support the Charge register's). > > > > > > > If chip is (or can be) used as charger, it should register as such. > > Note my question was the energy limit attributes, not the overflow > > attributes. > > I don't think it can be used as a charger but it can also measure > charge (integrating the measured current over time). As they are not > standard attributes I did not included this on the driver (I sent a > query on this before starting the driver - > https://marc.info/?l=linux-hwmon&m=156507711612877&w=2). > > I see your point about energy and having maximum and minimum for an > accumulated value. Honestly, looking at the chip specification I > cannot > see the intended use case for this. Maybe for > monitoring/characterizing > batteries since there are some controls on these accumulated values > (we > can set the part to accumulate only when current is positive for > example). > I will do some internal querying to see if I can find out more on > this. > Quoting the reply I had: "As the LTC2947 is bi-directional, the most likely use for the Min/Max Energy thresholds are for monitoring a battery being charged or discharged. A limit could be set based around the battery's storage capacity so that the battery is protected from being overcharging or fully drained." So, I'm not sure if this is a valid use case for hwmon subsystem. I'm aware there's also the power subsystem. So let me know what do you prefer here. Should I just report energyX_input attributes? Or can we keep the min/max? Nuno Sá ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/3] hwmon: Add support for ltc2947 2019-10-10 7:13 ` Sa, Nuno @ 2019-10-10 15:21 ` Guenter Roeck 2019-10-11 6:59 ` Sa, Nuno 0 siblings, 1 reply; 21+ messages in thread From: Guenter Roeck @ 2019-10-10 15:21 UTC (permalink / raw) To: Sa, Nuno Cc: robh+dt, corbet, linux-doc, devicetree, linux-hwmon, jdelvare, mark.rutland On Thu, Oct 10, 2019 at 07:13:06AM +0000, Sa, Nuno wrote: > > Quoting the reply I had: > > "As the LTC2947 is bi-directional, the most likely use for the Min/Max > Energy thresholds are for monitoring a battery being charged or > discharged. > A limit could be set based around the battery's storage capacity so > that the battery is protected from being overcharging or fully > drained." > > So, I'm not sure if this is a valid use case for hwmon subsystem. I'm > aware there's also the power subsystem. So let me know what do you > prefer here. Should I just report energyX_input attributes? Or can we > keep the min/max? > This isn't really a hwmon use case. Having such min/max attributes would just be confusing. Guenter ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/3] hwmon: Add support for ltc2947 2019-10-10 15:21 ` Guenter Roeck @ 2019-10-11 6:59 ` Sa, Nuno 0 siblings, 0 replies; 21+ messages in thread From: Sa, Nuno @ 2019-10-11 6:59 UTC (permalink / raw) To: linux Cc: mark.rutland, corbet, linux-doc, devicetree, linux-hwmon, robh+dt, jdelvare On Thu, 2019-10-10 at 08:21 -0700, Guenter Roeck wrote: > [External] > > On Thu, Oct 10, 2019 at 07:13:06AM +0000, Sa, Nuno wrote: > > Quoting the reply I had: > > > > "As the LTC2947 is bi-directional, the most likely use for the > > Min/Max > > Energy thresholds are for monitoring a battery being charged or > > discharged. > > A limit could be set based around the battery's storage capacity so > > that the battery is protected from being overcharging or fully > > drained." > > > > So, I'm not sure if this is a valid use case for hwmon subsystem. > > I'm > > aware there's also the power subsystem. So let me know what do you > > prefer here. Should I just report energyX_input attributes? Or can > > we > > keep the min/max? > > > This isn't really a hwmon use case. Having such min/max attributes > would > just be confusing. > Got it. Then I will remove min/max and the alarm attributes for energy and just leave the standard ones. When I have the time I might prepare a patch which turns this device into an mfd and adds a new driver in the power subsystem to monitor charge and energy so that it can cover this use case. I guess that would make sense. Nuno Sá ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 3/3] dt-bindings: iio: Add ltc2947 documentation 2019-09-24 12:49 [PATCH 0/3] LTC2947 support Nuno Sá 2019-09-24 12:49 ` [PATCH 1/3] hwmon: Fix HWMON_P_MIN_ALARM mask Nuno Sá 2019-09-24 12:49 ` [PATCH 2/3] hwmon: Add support for ltc2947 Nuno Sá @ 2019-09-24 12:49 ` Nuno Sá 2019-10-02 14:19 ` Rob Herring 2019-09-26 10:17 ` [PATCH 0/3] LTC2947 support Sa, Nuno 3 siblings, 1 reply; 21+ messages in thread From: Nuno Sá @ 2019-09-24 12:49 UTC (permalink / raw) To: linux-hwmon, devicetree, linux-doc Cc: Rob Herring, Mark Rutland, Guenter Roeck, Jean Delvare, Jonathan Corbet Document the LTC2947 device devicetree bindings. Signed-off-by: Nuno Sá <nuno.sa@analog.com> --- .../bindings/hwmon/adi,ltc2947.yaml | 101 ++++++++++++++++++ MAINTAINERS | 1 + 2 files changed, 102 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/adi,ltc2947.yaml diff --git a/Documentation/devicetree/bindings/hwmon/adi,ltc2947.yaml b/Documentation/devicetree/bindings/hwmon/adi,ltc2947.yaml new file mode 100644 index 000000000000..2ea0187421d4 --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/adi,ltc2947.yaml @@ -0,0 +1,101 @@ +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/bindings/hwmon/adi,ltc2947.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Analog Devices LTC2947 high precision power and energy monitor + +maintainers: + - Nuno Sá <nuno.sa@analog.com> + +description: | + Analog Devices LTC2947 high precision power and energy monitor over SPI or I2C. + + https://www.analog.com/media/en/technical-documentation/data-sheets/LTC2947.pdf + +properties: + compatible: + enum: + - adi,ltc2947 + + reg: + maxItems: 1 + + clocks: + description: + The LTC2947 uses either a trimmed internal oscillator or an external clock + as the time base for determining the integration period to represent time, + charge and energy. When an external clock is used, this property must be + set accordingly. + maxItems: 1 + + adi,accumulator-ctl-pol: + description: + This property controls the polarity of current that is accumulated to + calculate charge and energy so that, they can be only accumulated for + positive current for example. Since there are two sets of registers for + the accumulated values, this entry can also have two items which sets + energy1/charge1 and energy2/charger2 respectively. Check table 12 of the + datasheet for more information on the supported options. + allOf: + - $ref: /schemas/types.yaml#/definitions/uint32-array + - enum: [0, 1, 2, 3] + - minItems: 2 + - maxItems: 2 + default: [0, 0] + + adi,accumulation-deadband-microamp: + description: + This property controls the Accumulation Dead band which allows to set the + level of current below which no accumulation takes place. + allOf: + - $ref: /schemas/types.yaml#/definitions/uint32 + - maximum: 255 + default: 0 + + adi,gpio-out-pol: + description: + This property controls the GPIO polarity. Setting it to one makes the GPIO + active high, setting it to zero makets it active low. When this property + is present, the GPIO is automatically configured as output and set to + control a fan as a function of measured temperature. + allOf: + - $ref: /schemas/types.yaml#/definitions/uint32 + - enum: [0, 1] + default: 0 + + adi,gpio-in-accum: + description: + When set, this property sets the GPIO as input. It is then used to control + the accumulation of charge, energy and time. This function can be + enabled/configured separately for each of the two sets of accumulation + registers. Check table 13 of the datasheet for more information on the + supported options. This property cannot be used together with + adi,gpio-out-pol. + allOf: + - $ref: /schemas/types.yaml#/definitions/uint32-array + - enum: [0, 1, 2] + - minItems: 2 + - maxItems: 2 + default: [0, 0] + +required: + - compatible + - reg + + +examples: + - | + spi0 { + #address-cells = <1>; + #size-cells = <0>; + + ltc2947_spi: ltc2947@0 { + compatible = "adi,ltc2947"; + reg = <0>; + /* accumulation takes place always for energ1/charge1. */ + /* accumulation only on positive current for energy2/charge2. */ + adi,accumulator-ctl-pol = <0 1>; + }; + }; +... diff --git a/MAINTAINERS b/MAINTAINERS index 889f38c1c930..820bdde2044b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -9505,6 +9505,7 @@ F: drivers/hwmon/ltc2947-core.c F: drivers/hwmon/ltc2947-spi.c F: drivers/hwmon/ltc2947-i2c.c F: drivers/hwmon/ltc2947.h +F: Documentation/devicetree/bindings/hwmon/adi,ltc2947.yaml LTC4306 I2C MULTIPLEXER DRIVER M: Michael Hennerich <michael.hennerich@analog.com> -- 2.23.0 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 3/3] dt-bindings: iio: Add ltc2947 documentation 2019-09-24 12:49 ` [PATCH 3/3] dt-bindings: iio: Add ltc2947 documentation Nuno Sá @ 2019-10-02 14:19 ` Rob Herring 2019-10-02 15:09 ` Sa, Nuno 0 siblings, 1 reply; 21+ messages in thread From: Rob Herring @ 2019-10-02 14:19 UTC (permalink / raw) To: Nuno Sá Cc: linux-hwmon, devicetree, linux-doc, Mark Rutland, Guenter Roeck, Jean Delvare, Jonathan Corbet On Tue, Sep 24, 2019 at 02:49:45PM +0200, Nuno Sá wrote: > Document the LTC2947 device devicetree bindings. > > Signed-off-by: Nuno Sá <nuno.sa@analog.com> > --- > .../bindings/hwmon/adi,ltc2947.yaml | 101 ++++++++++++++++++ > MAINTAINERS | 1 + > 2 files changed, 102 insertions(+) > create mode 100644 Documentation/devicetree/bindings/hwmon/adi,ltc2947.yaml > > diff --git a/Documentation/devicetree/bindings/hwmon/adi,ltc2947.yaml b/Documentation/devicetree/bindings/hwmon/adi,ltc2947.yaml > new file mode 100644 > index 000000000000..2ea0187421d4 > --- /dev/null > +++ b/Documentation/devicetree/bindings/hwmon/adi,ltc2947.yaml > @@ -0,0 +1,101 @@ Missing license. Please make new bindings (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/bindings/hwmon/adi,ltc2947.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Analog Devices LTC2947 high precision power and energy monitor > + > +maintainers: > + - Nuno Sá <nuno.sa@analog.com> > + > +description: | > + Analog Devices LTC2947 high precision power and energy monitor over SPI or I2C. > + > + https://www.analog.com/media/en/technical-documentation/data-sheets/LTC2947.pdf > + > +properties: > + compatible: > + enum: > + - adi,ltc2947 > + > + reg: > + maxItems: 1 > + > + clocks: > + description: > + The LTC2947 uses either a trimmed internal oscillator or an external clock > + as the time base for determining the integration period to represent time, > + charge and energy. When an external clock is used, this property must be > + set accordingly. > + maxItems: 1 > + > + adi,accumulator-ctl-pol: > + description: > + This property controls the polarity of current that is accumulated to > + calculate charge and energy so that, they can be only accumulated for > + positive current for example. Since there are two sets of registers for > + the accumulated values, this entry can also have two items which sets > + energy1/charge1 and energy2/charger2 respectively. Check table 12 of the > + datasheet for more information on the supported options. > + allOf: > + - $ref: /schemas/types.yaml#/definitions/uint32-array > + - enum: [0, 1, 2, 3] > + - minItems: 2 > + - maxItems: 2 > + default: [0, 0] This should be: allOf: - $ref: ... items: enum: [0, 1, 2, 3] default: 0 minItems: 2 maxItems: 2 > + > + adi,accumulation-deadband-microamp: > + description: > + This property controls the Accumulation Dead band which allows to set the > + level of current below which no accumulation takes place. > + allOf: > + - $ref: /schemas/types.yaml#/definitions/uint32 > + - maximum: 255 maximum should be at same indent as allOf. Or default should be at the same level as maximum (under a single '-' list entry). > + default: 0 > + > + adi,gpio-out-pol: > + description: > + This property controls the GPIO polarity. Setting it to one makes the GPIO > + active high, setting it to zero makets it active low. When this property > + is present, the GPIO is automatically configured as output and set to > + control a fan as a function of measured temperature. > + allOf: > + - $ref: /schemas/types.yaml#/definitions/uint32 > + - enum: [0, 1] > + default: 0 Same here. > + > + adi,gpio-in-accum: > + description: > + When set, this property sets the GPIO as input. It is then used to control > + the accumulation of charge, energy and time. This function can be > + enabled/configured separately for each of the two sets of accumulation > + registers. Check table 13 of the datasheet for more information on the > + supported options. This property cannot be used together with > + adi,gpio-out-pol. > + allOf: > + - $ref: /schemas/types.yaml#/definitions/uint32-array > + - enum: [0, 1, 2] > + - minItems: 2 > + - maxItems: 2 > + default: [0, 0] Similar here. > + > +required: > + - compatible > + - reg > + > + > +examples: > + - | > + spi0 { Just 'spi' > + #address-cells = <1>; > + #size-cells = <0>; > + > + ltc2947_spi: ltc2947@0 { > + compatible = "adi,ltc2947"; > + reg = <0>; > + /* accumulation takes place always for energ1/charge1. */ > + /* accumulation only on positive current for energy2/charge2. */ > + adi,accumulator-ctl-pol = <0 1>; > + }; > + }; > +... > diff --git a/MAINTAINERS b/MAINTAINERS > index 889f38c1c930..820bdde2044b 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -9505,6 +9505,7 @@ F: drivers/hwmon/ltc2947-core.c > F: drivers/hwmon/ltc2947-spi.c > F: drivers/hwmon/ltc2947-i2c.c > F: drivers/hwmon/ltc2947.h > +F: Documentation/devicetree/bindings/hwmon/adi,ltc2947.yaml > > LTC4306 I2C MULTIPLEXER DRIVER > M: Michael Hennerich <michael.hennerich@analog.com> > -- > 2.23.0 > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/3] dt-bindings: iio: Add ltc2947 documentation 2019-10-02 14:19 ` Rob Herring @ 2019-10-02 15:09 ` Sa, Nuno 2019-10-02 19:06 ` Rob Herring 0 siblings, 1 reply; 21+ messages in thread From: Sa, Nuno @ 2019-10-02 15:09 UTC (permalink / raw) To: robh Cc: linux, corbet, linux-hwmon, devicetree, linux-doc, mark.rutland, jdelvare On Wed, 2019-10-02 at 09:19 -0500, Rob Herring wrote: > > On Tue, Sep 24, 2019 at 02:49:45PM +0200, Nuno Sá wrote: > > Document the LTC2947 device devicetree bindings. > > > > Signed-off-by: Nuno Sá <nuno.sa@analog.com> > > --- > > .../bindings/hwmon/adi,ltc2947.yaml | 101 > > ++++++++++++++++++ > > MAINTAINERS | 1 + > > 2 files changed, 102 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/hwmon/adi,ltc2947.yaml > > > > diff --git > > a/Documentation/devicetree/bindings/hwmon/adi,ltc2947.yaml > > b/Documentation/devicetree/bindings/hwmon/adi,ltc2947.yaml > > new file mode 100644 > > index 000000000000..2ea0187421d4 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/hwmon/adi,ltc2947.yaml > > @@ -0,0 +1,101 @@ > > Missing license. Please make new bindings (GPL-2.0-only OR BSD-2- > Clause) ack. > > +%YAML 1.2 > > +--- > > +$id: > > http://devicetree.org/schemas/bindings/hwmon/adi,ltc2947.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Analog Devices LTC2947 high precision power and energy > > monitor > > + > > +maintainers: > > + - Nuno Sá <nuno.sa@analog.com> > > + > > +description: | > > + Analog Devices LTC2947 high precision power and energy monitor > > over SPI or I2C. > > + > > + > > https://www.analog.com/media/en/technical-documentation/data-sheets/LTC2947.pdf > > + > > +properties: > > + compatible: > > + enum: > > + - adi,ltc2947 > > + > > + reg: > > + maxItems: 1 > > + > > + clocks: > > + description: > > + The LTC2947 uses either a trimmed internal oscillator or an > > external clock > > + as the time base for determining the integration period to > > represent time, > > + charge and energy. When an external clock is used, this > > property must be > > + set accordingly. > > + maxItems: 1 > > + > > + adi,accumulator-ctl-pol: > > + description: > > + This property controls the polarity of current that is > > accumulated to > > + calculate charge and energy so that, they can be only > > accumulated for > > + positive current for example. Since there are two sets of > > registers for > > + the accumulated values, this entry can also have two items > > which sets > > + energy1/charge1 and energy2/charger2 respectively. Check > > table 12 of the > > + datasheet for more information on the supported options. > > + allOf: > > + - $ref: /schemas/types.yaml#/definitions/uint32-array > > + - enum: [0, 1, 2, 3] > > + - minItems: 2 > > + - maxItems: 2 > > + default: [0, 0] > > This should be: > > allOf: > - $ref: ... > items: > enum: [0, 1, 2, 3] > default: 0 > minItems: 2 > maxItems: 2 I was expecting this already. When upstreaming another device, I realized some errors with `make dt_binding_check` when evaluating the example. Just for my understanding, is this also incorrect? allOf: - $ref: /schemas/types.yaml#/definitions/uint32-array - minItems: 2 maxItems: 2 items: enum: [0, 1, 2, 3] default: 0 I used a similar pattern in another device that is also waiting for review :). > > + > > + adi,accumulation-deadband-microamp: > > + description: > > + This property controls the Accumulation Dead band which > > allows to set the > > + level of current below which no accumulation takes place. > > + allOf: > > + - $ref: /schemas/types.yaml#/definitions/uint32 > > + - maximum: 255 > > maximum should be at same indent as allOf. Or default should be at > the > same level as maximum (under a single '-' list entry). So it should be? allOf: - $ref: /schemas/types.yaml#/definitions/uint32 - maximum: 255 default: 0 > > + default: 0 > > + > > + adi,gpio-out-pol: > > + description: > > + This property controls the GPIO polarity. Setting it to one > > makes the GPIO > > + active high, setting it to zero makets it active low. When > > this property > > + is present, the GPIO is automatically configured as output > > and set to > > + control a fan as a function of measured temperature. > > + allOf: > > + - $ref: /schemas/types.yaml#/definitions/uint32 > > + - enum: [0, 1] > > + default: 0 > > Same here. > > > + > > + adi,gpio-in-accum: > > + description: > > + When set, this property sets the GPIO as input. It is then > > used to control > > + the accumulation of charge, energy and time. This function > > can be > > + enabled/configured separately for each of the two sets of > > accumulation > > + registers. Check table 13 of the datasheet for more > > information on the > > + supported options. This property cannot be used together > > with > > + adi,gpio-out-pol. > > + allOf: > > + - $ref: /schemas/types.yaml#/definitions/uint32-array > > + - enum: [0, 1, 2] > > + - minItems: 2 > > + - maxItems: 2 > > + default: [0, 0] > > Similar here. > > > + > > +required: > > + - compatible > > + - reg > > + > > + > > +examples: > > + - | > > + spi0 { > > Just 'spi' ack. > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + ltc2947_spi: ltc2947@0 { > > + compatible = "adi,ltc2947"; > > + reg = <0>; > > + /* accumulation takes place always for > > energ1/charge1. */ > > + /* accumulation only on positive current for > > energy2/charge2. */ > > + adi,accumulator-ctl-pol = <0 1>; > > + }; > > + }; > > +... > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 889f38c1c930..820bdde2044b 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -9505,6 +9505,7 @@ F: drivers/hwmon/ltc2947-core.c > > F: drivers/hwmon/ltc2947-spi.c > > F: drivers/hwmon/ltc2947-i2c.c > > F: drivers/hwmon/ltc2947.h > > +F: Documentation/devicetree/bindings/hwmon/adi,ltc2947.yaml > > > > LTC4306 I2C MULTIPLEXER DRIVER > > M: Michael Hennerich <michael.hennerich@analog.com> > > -- > > 2.23.0 > > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/3] dt-bindings: iio: Add ltc2947 documentation 2019-10-02 15:09 ` Sa, Nuno @ 2019-10-02 19:06 ` Rob Herring 2019-10-04 14:58 ` Sa, Nuno 0 siblings, 1 reply; 21+ messages in thread From: Rob Herring @ 2019-10-02 19:06 UTC (permalink / raw) To: Sa, Nuno Cc: linux, corbet, linux-hwmon, devicetree, linux-doc, mark.rutland, jdelvare On Wed, Oct 2, 2019 at 10:09 AM Sa, Nuno <Nuno.Sa@analog.com> wrote: > > On Wed, 2019-10-02 at 09:19 -0500, Rob Herring wrote: > > > > On Tue, Sep 24, 2019 at 02:49:45PM +0200, Nuno Sá wrote: > > > Document the LTC2947 device devicetree bindings. > > > > > > Signed-off-by: Nuno Sá <nuno.sa@analog.com> > > > --- > > > .../bindings/hwmon/adi,ltc2947.yaml | 101 > > > ++++++++++++++++++ > > > MAINTAINERS | 1 + > > > 2 files changed, 102 insertions(+) > > > create mode 100644 > > > Documentation/devicetree/bindings/hwmon/adi,ltc2947.yaml > > > > > > diff --git > > > a/Documentation/devicetree/bindings/hwmon/adi,ltc2947.yaml > > > b/Documentation/devicetree/bindings/hwmon/adi,ltc2947.yaml > > > new file mode 100644 > > > index 000000000000..2ea0187421d4 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/hwmon/adi,ltc2947.yaml > > > @@ -0,0 +1,101 @@ > > > > Missing license. Please make new bindings (GPL-2.0-only OR BSD-2- > > Clause) > > ack. > > > > +%YAML 1.2 > > > +--- > > > +$id: > > > http://devicetree.org/schemas/bindings/hwmon/adi,ltc2947.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: Analog Devices LTC2947 high precision power and energy > > > monitor > > > + > > > +maintainers: > > > + - Nuno Sá <nuno.sa@analog.com> > > > + > > > +description: | > > > + Analog Devices LTC2947 high precision power and energy monitor > > > over SPI or I2C. > > > + > > > + > > > https://www.analog.com/media/en/technical-documentation/data-sheets/LTC2947.pdf > > > + > > > +properties: > > > + compatible: > > > + enum: > > > + - adi,ltc2947 > > > + > > > + reg: > > > + maxItems: 1 > > > + > > > + clocks: > > > + description: > > > + The LTC2947 uses either a trimmed internal oscillator or an > > > external clock > > > + as the time base for determining the integration period to > > > represent time, > > > + charge and energy. When an external clock is used, this > > > property must be > > > + set accordingly. > > > + maxItems: 1 > > > + > > > + adi,accumulator-ctl-pol: > > > + description: > > > + This property controls the polarity of current that is > > > accumulated to > > > + calculate charge and energy so that, they can be only > > > accumulated for > > > + positive current for example. Since there are two sets of > > > registers for > > > + the accumulated values, this entry can also have two items > > > which sets > > > + energy1/charge1 and energy2/charger2 respectively. Check > > > table 12 of the > > > + datasheet for more information on the supported options. > > > + allOf: > > > + - $ref: /schemas/types.yaml#/definitions/uint32-array > > > + - enum: [0, 1, 2, 3] > > > + - minItems: 2 > > > + - maxItems: 2 > > > + default: [0, 0] > > > > This should be: > > > > allOf: > > - $ref: ... > > items: > > enum: [0, 1, 2, 3] > > default: 0 > > minItems: 2 > > maxItems: 2 > > I was expecting this already. When upstreaming another device, I > realized some errors with `make dt_binding_check` when evaluating the > example. Just for my understanding, is this also incorrect? The allOf structure like this also works. You'll get better error messages if schemas are not under an allOf. > allOf: > - $ref: /schemas/types.yaml#/definitions/uint32-array > - minItems: 2 > maxItems: 2 > items: > enum: [0, 1, 2, 3] > default: 0 However, 'default' needs to be indented under 'items'. > > I used a similar pattern in another device that is also waiting for > review :). > > > + > > > + adi,accumulation-deadband-microamp: > > > + description: > > > + This property controls the Accumulation Dead band which > > > allows to set the > > > + level of current below which no accumulation takes place. > > > + allOf: > > > + - $ref: /schemas/types.yaml#/definitions/uint32 > > > + - maximum: 255 > > > > maximum should be at same indent as allOf. Or default should be at > > the > > same level as maximum (under a single '-' list entry). > > So it should be? > > allOf: > - $ref: /schemas/types.yaml#/definitions/uint32 > - maximum: 255 > default: 0 Yes. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/3] dt-bindings: iio: Add ltc2947 documentation 2019-10-02 19:06 ` Rob Herring @ 2019-10-04 14:58 ` Sa, Nuno 2019-10-04 15:23 ` Rob Herring 0 siblings, 1 reply; 21+ messages in thread From: Sa, Nuno @ 2019-10-04 14:58 UTC (permalink / raw) To: robh Cc: linux, corbet, linux-hwmon, devicetree, mark.rutland, linux-doc, jdelvare Hi Rob, Just wanted to clarify something which I don't know if is something I'm doing wrong or if it is some issue. On Wed, 2019-10-02 at 14:06 -0500, Rob Herring wrote: > > On Wed, Oct 2, 2019 at 10:09 AM Sa, Nuno <Nuno.Sa@analog.com> wrote: > > On Wed, 2019-10-02 at 09:19 -0500, Rob Herring wrote: > > > On Tue, Sep 24, 2019 at 02:49:45PM +0200, Nuno Sá wrote: > > > > Document the LTC2947 device devicetree bindings. > > > > > > > > Signed-off-by: Nuno Sá <nuno.sa@analog.com> > > > > --- > > > > .../bindings/hwmon/adi,ltc2947.yaml | 101 > > > > ++++++++++++++++++ > > > > MAINTAINERS | 1 + > > > > 2 files changed, 102 insertions(+) > > > > create mode 100644 > > > > Documentation/devicetree/bindings/hwmon/adi,ltc2947.yaml > > > > > > > > diff --git > > > > a/Documentation/devicetree/bindings/hwmon/adi,ltc2947.yaml > > > > b/Documentation/devicetree/bindings/hwmon/adi,ltc2947.yaml > > > > new file mode 100644 > > > > index 000000000000..2ea0187421d4 > > > > --- /dev/null > > > > +++ b/Documentation/devicetree/bindings/hwmon/adi,ltc2947.yaml > > > > @@ -0,0 +1,101 @@ > > > > > > Missing license. Please make new bindings (GPL-2.0-only OR BSD-2- > > > Clause) > > > > ack. > > > > > > +%YAML 1.2 > > > > +--- > > > > +$id: > > > > http://devicetree.org/schemas/bindings/hwmon/adi,ltc2947.yaml# > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > + > > > > +title: Analog Devices LTC2947 high precision power and energy > > > > monitor > > > > + > > > > +maintainers: > > > > + - Nuno Sá <nuno.sa@analog.com> > > > > + > > > > +description: | > > > > + Analog Devices LTC2947 high precision power and energy > > > > monitor > > > > over SPI or I2C. > > > > + > > > > + > > > > https://www.analog.com/media/en/technical-documentation/data-sheets/LTC2947.pdf > > > > + > > > > +properties: > > > > + compatible: > > > > + enum: > > > > + - adi,ltc2947 > > > > + > > > > + reg: > > > > + maxItems: 1 > > > > + > > > > + clocks: > > > > + description: > > > > + The LTC2947 uses either a trimmed internal oscillator or > > > > an > > > > external clock > > > > + as the time base for determining the integration period > > > > to > > > > represent time, > > > > + charge and energy. When an external clock is used, this > > > > property must be > > > > + set accordingly. > > > > + maxItems: 1 > > > > + > > > > + adi,accumulator-ctl-pol: > > > > + description: > > > > + This property controls the polarity of current that is > > > > accumulated to > > > > + calculate charge and energy so that, they can be only > > > > accumulated for > > > > + positive current for example. Since there are two sets > > > > of > > > > registers for > > > > + the accumulated values, this entry can also have two > > > > items > > > > which sets > > > > + energy1/charge1 and energy2/charger2 respectively. Check > > > > table 12 of the > > > > + datasheet for more information on the supported options. > > > > + allOf: > > > > + - $ref: /schemas/types.yaml#/definitions/uint32-array > > > > + - enum: [0, 1, 2, 3] > > > > + - minItems: 2 > > > > + - maxItems: 2 > > > > + default: [0, 0] > > > > > > This should be: > > > > > > allOf: > > > - $ref: ... > > > items: > > > enum: [0, 1, 2, 3] > > > default: 0 > > > minItems: 2 > > > maxItems: 2 When trying the above I get: "ltc2947@0: adi,accumulator-ctl-pol:0: [0, 1] is not valid under any of the given schemas" > > I was expecting this already. When upstreaming another device, I > > realized some errors with `make dt_binding_check` when evaluating > > the > > example. Just for my understanding, is this also incorrect? > > The allOf structure like this also works. You'll get better error > messages if schemas are not under an allOf. > > > allOf: > > - $ref: /schemas/types.yaml#/definitions/uint32-array > > - minItems: 2 > > maxItems: 2 > > items: > > enum: [0, 1, 2, 3] > > default: 0 > > However, 'default' needs to be indented under 'items'. In this form, I don't get any warning/error when checking the dts example. > > I used a similar pattern in another device that is also waiting for > > review :). > > > > + > > > > + adi,accumulation-deadband-microamp: > > > > + description: > > > > + This property controls the Accumulation Dead band which > > > > allows to set the > > > > + level of current below which no accumulation takes > > > > place. > > > > + allOf: > > > > + - $ref: /schemas/types.yaml#/definitions/uint32 > > > > + - maximum: 255 > > > > > > maximum should be at same indent as allOf. Or default should be > > > at > > > the > > > same level as maximum (under a single '-' list entry). > > > > So it should be? > > > > allOf: > > - $ref: /schemas/types.yaml#/definitions/uint32 > > - maximum: 255 > > default: 0 > > Yes. Regards, Nuno Sá ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/3] dt-bindings: iio: Add ltc2947 documentation 2019-10-04 14:58 ` Sa, Nuno @ 2019-10-04 15:23 ` Rob Herring 2019-10-08 14:20 ` Sa, Nuno 0 siblings, 1 reply; 21+ messages in thread From: Rob Herring @ 2019-10-04 15:23 UTC (permalink / raw) To: Sa, Nuno Cc: linux, corbet, linux-hwmon, devicetree, mark.rutland, linux-doc, jdelvare On Fri, Oct 4, 2019 at 9:59 AM Sa, Nuno <Nuno.Sa@analog.com> wrote: > > Hi Rob, > > Just wanted to clarify something which I don't know if is something I'm > doing wrong or if it is some issue. > > On Wed, 2019-10-02 at 14:06 -0500, Rob Herring wrote: > > > > On Wed, Oct 2, 2019 at 10:09 AM Sa, Nuno <Nuno.Sa@analog.com> wrote: > > > On Wed, 2019-10-02 at 09:19 -0500, Rob Herring wrote: > > > > On Tue, Sep 24, 2019 at 02:49:45PM +0200, Nuno Sá wrote: > > > > > Document the LTC2947 device devicetree bindings. > > > > > > > > > > Signed-off-by: Nuno Sá <nuno.sa@analog.com> > > > > > --- > > > > > .../bindings/hwmon/adi,ltc2947.yaml | 101 > > > > > ++++++++++++++++++ > > > > > MAINTAINERS | 1 + > > > > > 2 files changed, 102 insertions(+) > > > > > create mode 100644 > > > > > Documentation/devicetree/bindings/hwmon/adi,ltc2947.yaml > > > > > > > > > > diff --git > > > > > a/Documentation/devicetree/bindings/hwmon/adi,ltc2947.yaml > > > > > b/Documentation/devicetree/bindings/hwmon/adi,ltc2947.yaml > > > > > new file mode 100644 > > > > > index 000000000000..2ea0187421d4 > > > > > --- /dev/null > > > > > +++ b/Documentation/devicetree/bindings/hwmon/adi,ltc2947.yaml > > > > > @@ -0,0 +1,101 @@ > > > > > > > > Missing license. Please make new bindings (GPL-2.0-only OR BSD-2- > > > > Clause) > > > > > > ack. > > > > > > > > +%YAML 1.2 > > > > > +--- > > > > > +$id: > > > > > http://devicetree.org/schemas/bindings/hwmon/adi,ltc2947.yaml# > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > > + > > > > > +title: Analog Devices LTC2947 high precision power and energy > > > > > monitor > > > > > + > > > > > +maintainers: > > > > > + - Nuno Sá <nuno.sa@analog.com> > > > > > + > > > > > +description: | > > > > > + Analog Devices LTC2947 high precision power and energy > > > > > monitor > > > > > over SPI or I2C. > > > > > + > > > > > + > > > > > https://www.analog.com/media/en/technical-documentation/data-sheets/LTC2947.pdf > > > > > + > > > > > +properties: > > > > > + compatible: > > > > > + enum: > > > > > + - adi,ltc2947 > > > > > + > > > > > + reg: > > > > > + maxItems: 1 > > > > > + > > > > > + clocks: > > > > > + description: > > > > > + The LTC2947 uses either a trimmed internal oscillator or > > > > > an > > > > > external clock > > > > > + as the time base for determining the integration period > > > > > to > > > > > represent time, > > > > > + charge and energy. When an external clock is used, this > > > > > property must be > > > > > + set accordingly. > > > > > + maxItems: 1 > > > > > + > > > > > + adi,accumulator-ctl-pol: > > > > > + description: > > > > > + This property controls the polarity of current that is > > > > > accumulated to > > > > > + calculate charge and energy so that, they can be only > > > > > accumulated for > > > > > + positive current for example. Since there are two sets > > > > > of > > > > > registers for > > > > > + the accumulated values, this entry can also have two > > > > > items > > > > > which sets > > > > > + energy1/charge1 and energy2/charger2 respectively. Check > > > > > table 12 of the > > > > > + datasheet for more information on the supported options. > > > > > + allOf: > > > > > + - $ref: /schemas/types.yaml#/definitions/uint32-array > > > > > + - enum: [0, 1, 2, 3] > > > > > + - minItems: 2 > > > > > + - maxItems: 2 > > > > > + default: [0, 0] > > > > > > > > This should be: > > > > > > > > allOf: > > > > - $ref: ... > > > > items: > > > > enum: [0, 1, 2, 3] > > > > default: 0 > > > > minItems: 2 > > > > maxItems: 2 > > When trying the above I get: > > "ltc2947@0: adi,accumulator-ctl-pol:0: [0, 1] is not valid under any of > the given schemas" Is dtschema up to date with the latest. I think I fixed this case recently, though with the wonderful json-schema errors it is hard to tell. Rob ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/3] dt-bindings: iio: Add ltc2947 documentation 2019-10-04 15:23 ` Rob Herring @ 2019-10-08 14:20 ` Sa, Nuno 0 siblings, 0 replies; 21+ messages in thread From: Sa, Nuno @ 2019-10-08 14:20 UTC (permalink / raw) To: robh Cc: linux, corbet, linux-hwmon, devicetree, mark.rutland, linux-doc, jdelvare On Fri, 2019-10-04 at 10:23 -0500, Rob Herring wrote: > > On Fri, Oct 4, 2019 at 9:59 AM Sa, Nuno <Nuno.Sa@analog.com> wrote: > > Hi Rob, > > > > Just wanted to clarify something which I don't know if is something > > I'm > > doing wrong or if it is some issue. > > > > On Wed, 2019-10-02 at 14:06 -0500, Rob Herring wrote: > > > On Wed, Oct 2, 2019 at 10:09 AM Sa, Nuno <Nuno.Sa@analog.com> > > > wrote: > > > > On Wed, 2019-10-02 at 09:19 -0500, Rob Herring wrote: > > > > > On Tue, Sep 24, 2019 at 02:49:45PM +0200, Nuno Sá wrote: > > > > > > Document the LTC2947 device devicetree bindings. > > > > > > > > > > > > Signed-off-by: Nuno Sá <nuno.sa@analog.com> > > > > > > --- > > > > > > .../bindings/hwmon/adi,ltc2947.yaml | 101 > > > > > > ++++++++++++++++++ > > > > > > MAINTAINERS | 1 + > > > > > > 2 files changed, 102 insertions(+) > > > > > > create mode 100644 > > > > > > Documentation/devicetree/bindings/hwmon/adi,ltc2947.yaml > > > > > > > > > > > > diff --git > > > > > > a/Documentation/devicetree/bindings/hwmon/adi,ltc2947.yaml > > > > > > b/Documentation/devicetree/bindings/hwmon/adi,ltc2947.yaml > > > > > > new file mode 100644 > > > > > > index 000000000000..2ea0187421d4 > > > > > > --- /dev/null > > > > > > +++ > > > > > > b/Documentation/devicetree/bindings/hwmon/adi,ltc2947.yaml > > > > > > @@ -0,0 +1,101 @@ > > > > > > > > > > Missing license. Please make new bindings (GPL-2.0-only OR > > > > > BSD-2- > > > > > Clause) > > > > > > > > ack. > > > > > > > > > > +%YAML 1.2 > > > > > > +--- > > > > > > +$id: > > > > > > http://devicetree.org/schemas/bindings/hwmon/adi,ltc2947.yaml# > > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > > > + > > > > > > +title: Analog Devices LTC2947 high precision power and > > > > > > energy > > > > > > monitor > > > > > > + > > > > > > +maintainers: > > > > > > + - Nuno Sá <nuno.sa@analog.com> > > > > > > + > > > > > > +description: | > > > > > > + Analog Devices LTC2947 high precision power and energy > > > > > > monitor > > > > > > over SPI or I2C. > > > > > > + > > > > > > + > > > > > > https://www.analog.com/media/en/technical-documentation/data-sheets/LTC2947.pdf > > > > > > + > > > > > > +properties: > > > > > > + compatible: > > > > > > + enum: > > > > > > + - adi,ltc2947 > > > > > > + > > > > > > + reg: > > > > > > + maxItems: 1 > > > > > > + > > > > > > + clocks: > > > > > > + description: > > > > > > + The LTC2947 uses either a trimmed internal > > > > > > oscillator or > > > > > > an > > > > > > external clock > > > > > > + as the time base for determining the integration > > > > > > period > > > > > > to > > > > > > represent time, > > > > > > + charge and energy. When an external clock is used, > > > > > > this > > > > > > property must be > > > > > > + set accordingly. > > > > > > + maxItems: 1 > > > > > > + > > > > > > + adi,accumulator-ctl-pol: > > > > > > + description: > > > > > > + This property controls the polarity of current that > > > > > > is > > > > > > accumulated to > > > > > > + calculate charge and energy so that, they can be > > > > > > only > > > > > > accumulated for > > > > > > + positive current for example. Since there are two > > > > > > sets > > > > > > of > > > > > > registers for > > > > > > + the accumulated values, this entry can also have two > > > > > > items > > > > > > which sets > > > > > > + energy1/charge1 and energy2/charger2 respectively. > > > > > > Check > > > > > > table 12 of the > > > > > > + datasheet for more information on the supported > > > > > > options. > > > > > > + allOf: > > > > > > + - $ref: /schemas/types.yaml#/definitions/uint32- > > > > > > array > > > > > > + - enum: [0, 1, 2, 3] > > > > > > + - minItems: 2 > > > > > > + - maxItems: 2 > > > > > > + default: [0, 0] > > > > > > > > > > This should be: > > > > > > > > > > allOf: > > > > > - $ref: ... > > > > > items: > > > > > enum: [0, 1, 2, 3] > > > > > default: 0 > > > > > minItems: 2 > > > > > maxItems: 2 > > > > When trying the above I get: > > > > "ltc2947@0: adi,accumulator-ctl-pol:0: [0, 1] is not valid under > > any of > > the given schemas" > > Is dtschema up to date with the latest. I think I fixed this case > recently, though with the wonderful json-schema errors it is hard to > tell. With latest dtschema works. Thanks! Nuno Sa > Rob -- Analog Devices GmbH Otl-Aicher Strasse 60-64 80807 München Sitz der Gesellschaft München, Registergericht München HRB 40368, Geschäftsführer: Stefan Steyerl, Thomas Edward Cribben, Michael Paul Sondel ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] LTC2947 support 2019-09-24 12:49 [PATCH 0/3] LTC2947 support Nuno Sá ` (2 preceding siblings ...) 2019-09-24 12:49 ` [PATCH 3/3] dt-bindings: iio: Add ltc2947 documentation Nuno Sá @ 2019-09-26 10:17 ` Sa, Nuno 3 siblings, 0 replies; 21+ messages in thread From: Sa, Nuno @ 2019-09-26 10:17 UTC (permalink / raw) To: linux-hwmon, devicetree, linux-doc Cc: linux, corbet, mark.rutland, robh+dt, jdelvare On Tue, 2019-09-24 at 14:49 +0200, Nuno Sá wrote: > > This series adds support for the LTC2947 hwmon device. The device > supports power[1-*]_min_alarm, so that it makes use of > the HWMON_P_MIN_ALARM mask. This brings me to the first patch, which > is a > fix on the hwmon subsystem for the HWMON_P_MIN_ALARM mask. It was > defining > the same mask as HWMON_P_MAX_ALARM. > The rest of the series is the usual for a new device. > > Nuno Sá (3): > hwmon: Fix HWMON_P_MIN_ALARM mask > hwmon: Add support for ltc2947 > dt-bindings: iio: Add ltc2947 documentation Ok, I will wait for further reviews, but the dt-bindings patch definitely need to be renamed to *dt-bindings: hwmon* :(. > > .../bindings/hwmon/adi,ltc2947.yaml | 101 ++ > Documentation/hwmon/ltc2947.rst | 110 ++ > MAINTAINERS | 11 + > drivers/hwmon/Kconfig | 27 + > drivers/hwmon/Makefile | 3 + > drivers/hwmon/ltc2947-core.c | 1421 > +++++++++++++++++ > drivers/hwmon/ltc2947-i2c.c | 49 + > drivers/hwmon/ltc2947-spi.c | 50 + > drivers/hwmon/ltc2947.h | 12 + > include/linux/hwmon.h | 2 +- > 10 files changed, 1785 insertions(+), 1 deletion(-) > create mode 100644 > Documentation/devicetree/bindings/hwmon/adi,ltc2947.yaml > create mode 100644 Documentation/hwmon/ltc2947.rst > create mode 100644 drivers/hwmon/ltc2947-core.c > create mode 100644 drivers/hwmon/ltc2947-i2c.c > create mode 100644 drivers/hwmon/ltc2947-spi.c > create mode 100644 drivers/hwmon/ltc2947.h > ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2019-10-11 7:00 UTC | newest] Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-09-24 12:49 [PATCH 0/3] LTC2947 support Nuno Sá 2019-09-24 12:49 ` [PATCH 1/3] hwmon: Fix HWMON_P_MIN_ALARM mask Nuno Sá 2019-10-02 13:37 ` Guenter Roeck 2019-09-24 12:49 ` [PATCH 2/3] hwmon: Add support for ltc2947 Nuno Sá 2019-10-03 4:14 ` Guenter Roeck 2019-10-04 7:45 ` Sa, Nuno 2019-10-04 15:06 ` Guenter Roeck 2019-10-07 12:25 ` Sa, Nuno 2019-10-07 12:44 ` Guenter Roeck 2019-10-07 14:51 ` Sa, Nuno 2019-10-10 7:13 ` Sa, Nuno 2019-10-10 15:21 ` Guenter Roeck 2019-10-11 6:59 ` Sa, Nuno 2019-09-24 12:49 ` [PATCH 3/3] dt-bindings: iio: Add ltc2947 documentation Nuno Sá 2019-10-02 14:19 ` Rob Herring 2019-10-02 15:09 ` Sa, Nuno 2019-10-02 19:06 ` Rob Herring 2019-10-04 14:58 ` Sa, Nuno 2019-10-04 15:23 ` Rob Herring 2019-10-08 14:20 ` Sa, Nuno 2019-09-26 10:17 ` [PATCH 0/3] LTC2947 support Sa, Nuno
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).