* [PATCH] hwmon: (ina2xx) Implement alert functions
@ 2020-05-04 22:42 Alex Qiu
2020-05-04 23:54 ` Guenter Roeck
0 siblings, 1 reply; 6+ messages in thread
From: Alex Qiu @ 2020-05-04 22:42 UTC (permalink / raw)
To: Guenter Roeck; +Cc: linux-hwmon, Alex Qiu
Implement alert functions for INA226, INA230 and INA231. Expose 06h
Mask/Enable and 07h Alert Limit registers via alert setting and alarm
files.
Signed-off-by: Alex Qiu <xqiu@google.com>
---
Documentation/hwmon/ina2xx.rst | 19 ++++
drivers/hwmon/ina2xx.c | 174 +++++++++++++++++++++++++++++++++
2 files changed, 193 insertions(+)
diff --git a/Documentation/hwmon/ina2xx.rst b/Documentation/hwmon/ina2xx.rst
index 94b9a260c518..ed81f5416331 100644
--- a/Documentation/hwmon/ina2xx.rst
+++ b/Documentation/hwmon/ina2xx.rst
@@ -99,6 +99,25 @@ Sysfs entries for ina226, ina230 and ina231 only
------------------------------------------------
======================= ====================================================
+in0_lcrit Critical low shunt voltage
+in0_crit Critical high shunt voltage
+in0_lcrit_alarm Shunt voltage critical low alarm
+in0_crit_alarm Shunt voltage critical high alarm
+in1_lcrit Critical low bus voltage
+in1_crit Critical high bus voltage
+in1_lcrit_alarm Bus voltage critical low alarm
+in1_crit_alarm Bus voltage critical high alarm
+power1_crit Critical high power
+power1_crit_alarm Power critical high alarm
update_interval data conversion time; affects number of samples used
to average results for shunt and bus voltages.
======================= ====================================================
+
+.. note::
+
+ - Configure `shunt_resistor` before configure `power1_crit`, because power
+ value is calculated based on `shunt_resistor` set.
+ - Because of the underlying register implementation, only one `*crit` setting
+ and its `alarm` can be active. Writing to one `*crit` setting clears other
+ `*crit` settings and alarms. Writing 0 to any `*crit` setting clears all
+ `*crit` settings and alarms.
diff --git a/drivers/hwmon/ina2xx.c b/drivers/hwmon/ina2xx.c
index e9e78c0b7212..0ce54e0a071d 100644
--- a/drivers/hwmon/ina2xx.c
+++ b/drivers/hwmon/ina2xx.c
@@ -74,6 +74,17 @@
#define INA226_READ_AVG(reg) (((reg) & INA226_AVG_RD_MASK) >> 9)
#define INA226_SHIFT_AVG(val) ((val) << 9)
+/* bit number of alert functions in Mask/Enable Register */
+#define INA226_SHUNT_OVER_VOLTAGE_BIT 15
+#define INA226_SHUNT_UNDER_VOLTAGE_BIT 14
+#define INA226_BUS_OVER_VOLTAGE_BIT 13
+#define INA226_BUS_UNDER_VOLTAGE_BIT 12
+#define INA226_POWER_OVER_LIMIT_BIT 11
+
+/* bit mask for alert config bits of Mask/Enable Register */
+#define INA226_ALERT_CONFIG_MASK 0xFC00
+#define INA226_ALERT_FUNCTION_FLAG BIT(4)
+
/* common attrs, ina226 attrs and NULL */
#define INA2XX_MAX_ATTRIBUTE_GROUPS 3
@@ -303,6 +314,136 @@ static ssize_t ina2xx_value_show(struct device *dev,
ina2xx_get_value(data, attr->index, regval));
}
+static int ina226_reg_to_alert(struct ina2xx_data *data, u8 bit, u16 regval)
+{
+ int get_value_reg_map;
+
+ switch (bit) {
+ case INA226_SHUNT_OVER_VOLTAGE_BIT:
+ case INA226_SHUNT_UNDER_VOLTAGE_BIT:
+ get_value_reg_map = INA2XX_SHUNT_VOLTAGE;
+ break;
+ case INA226_BUS_OVER_VOLTAGE_BIT:
+ case INA226_BUS_UNDER_VOLTAGE_BIT:
+ get_value_reg_map = INA2XX_BUS_VOLTAGE;
+ break;
+ case INA226_POWER_OVER_LIMIT_BIT:
+ get_value_reg_map = INA2XX_POWER;
+ break;
+ default:
+ /* programmer goofed */
+ WARN_ON_ONCE(1);
+ return 0;
+ }
+
+ return ina2xx_get_value(data, get_value_reg_map, regval);
+}
+
+/*
+ * Turns alert limit values into register values.
+ * Opposite of the formula in ina2xx_get_value().
+ */
+static s16 ina226_alert_to_reg(struct ina2xx_data *data, u8 bit, int val)
+{
+ switch (bit) {
+ case INA226_SHUNT_OVER_VOLTAGE_BIT:
+ case INA226_SHUNT_UNDER_VOLTAGE_BIT:
+ val *= data->config->shunt_div;
+ return clamp_val(val, SHRT_MIN, SHRT_MAX);
+ case INA226_BUS_OVER_VOLTAGE_BIT:
+ case INA226_BUS_UNDER_VOLTAGE_BIT:
+ val = (val * 1000) << data->config->bus_voltage_shift;
+ val = DIV_ROUND_CLOSEST(val, data->config->bus_voltage_lsb);
+ return clamp_val(val, 0, SHRT_MAX);
+ case INA226_POWER_OVER_LIMIT_BIT:
+ val = DIV_ROUND_CLOSEST(val, data->power_lsb_uW);
+ return clamp_val(val, 0, USHRT_MAX);
+ default:
+ /* programmer goofed */
+ WARN_ON_ONCE(1);
+ return 0;
+ }
+}
+
+static ssize_t ina226_alert_show(struct device *dev,
+ struct device_attribute *da, char *buf)
+{
+ struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
+ struct ina2xx_data *data = dev_get_drvdata(dev);
+ int regval;
+ int val = 0;
+ int ret;
+
+ ret = regmap_read(data->regmap, INA226_MASK_ENABLE, ®val);
+ if (ret)
+ return ret;
+
+ if (regval & BIT(attr->index)) {
+ ret = regmap_read(data->regmap, INA226_ALERT_LIMIT, ®val);
+ if (ret)
+ return ret;
+ val = ina226_reg_to_alert(data, attr->index, regval);
+ }
+
+ return snprintf(buf, PAGE_SIZE, "%d\n", val);
+}
+
+static ssize_t ina226_alert_store(struct device *dev,
+ struct device_attribute *da,
+ const char *buf, size_t count)
+{
+ struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
+ struct ina2xx_data *data = dev_get_drvdata(dev);
+ unsigned long val;
+ int ret;
+
+ ret = kstrtoul(buf, 10, &val);
+ if (ret < 0)
+ return ret;
+
+ /*
+ * Clear all alerts first to avoid accidentally triggering ALERT pin
+ * due to register write sequence. Then, only enable the alert
+ * if the value is non-zero.
+ */
+ ret = regmap_update_bits(data->regmap, INA226_MASK_ENABLE,
+ INA226_ALERT_CONFIG_MASK, 0);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_write(data->regmap, INA226_ALERT_LIMIT,
+ ina226_alert_to_reg(data, attr->index, val));
+ if (ret < 0)
+ return ret;
+
+ if (val != 0) {
+ ret = regmap_update_bits(data->regmap, INA226_MASK_ENABLE,
+ INA226_ALERT_CONFIG_MASK,
+ BIT(attr->index));
+ if (ret < 0)
+ return ret;
+ }
+ return count;
+}
+
+static ssize_t ina226_alarm_show(struct device *dev,
+ struct device_attribute *da, char *buf)
+{
+ struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
+ struct ina2xx_data *data = dev_get_drvdata(dev);
+ int regval;
+ int alarm = 0;
+ int ret;
+
+ ret = regmap_read(data->regmap, INA226_MASK_ENABLE, ®val);
+ if (ret)
+ return ret;
+
+ alarm = (regval & BIT(attr->index)) &&
+ (regval & INA226_ALERT_FUNCTION_FLAG);
+ return snprintf(buf, PAGE_SIZE, "%d\n", alarm);
+}
+
/*
* In order to keep calibration register value fixed, the product
* of current_lsb and shunt_resistor should also be fixed and equal
@@ -392,15 +533,38 @@ static ssize_t ina226_interval_show(struct device *dev,
/* shunt voltage */
static SENSOR_DEVICE_ATTR_RO(in0_input, ina2xx_value, INA2XX_SHUNT_VOLTAGE);
+/* shunt voltage over/under voltage alert setting and alarm */
+static SENSOR_DEVICE_ATTR_RW(in0_crit, ina226_alert,
+ INA226_SHUNT_OVER_VOLTAGE_BIT);
+static SENSOR_DEVICE_ATTR_RW(in0_lcrit, ina226_alert,
+ INA226_SHUNT_UNDER_VOLTAGE_BIT);
+static SENSOR_DEVICE_ATTR_RO(in0_crit_alarm, ina226_alarm,
+ INA226_SHUNT_OVER_VOLTAGE_BIT);
+static SENSOR_DEVICE_ATTR_RO(in0_lcrit_alarm, ina226_alarm,
+ INA226_SHUNT_UNDER_VOLTAGE_BIT);
/* bus voltage */
static SENSOR_DEVICE_ATTR_RO(in1_input, ina2xx_value, INA2XX_BUS_VOLTAGE);
+/* bus voltage over/under voltage alert setting and alarm */
+static SENSOR_DEVICE_ATTR_RW(in1_crit, ina226_alert,
+ INA226_BUS_OVER_VOLTAGE_BIT);
+static SENSOR_DEVICE_ATTR_RW(in1_lcrit, ina226_alert,
+ INA226_BUS_UNDER_VOLTAGE_BIT);
+static SENSOR_DEVICE_ATTR_RO(in1_crit_alarm, ina226_alarm,
+ INA226_BUS_OVER_VOLTAGE_BIT);
+static SENSOR_DEVICE_ATTR_RO(in1_lcrit_alarm, ina226_alarm,
+ INA226_BUS_UNDER_VOLTAGE_BIT);
/* calculated current */
static SENSOR_DEVICE_ATTR_RO(curr1_input, ina2xx_value, INA2XX_CURRENT);
/* calculated power */
static SENSOR_DEVICE_ATTR_RO(power1_input, ina2xx_value, INA2XX_POWER);
+/* over-limit power alert setting and alarm */
+static SENSOR_DEVICE_ATTR_RW(power1_crit, ina226_alert,
+ INA226_POWER_OVER_LIMIT_BIT);
+static SENSOR_DEVICE_ATTR_RO(power1_crit_alarm, ina226_alarm,
+ INA226_POWER_OVER_LIMIT_BIT);
/* shunt resistance */
static SENSOR_DEVICE_ATTR_RW(shunt_resistor, ina2xx_shunt, INA2XX_CALIBRATION);
@@ -423,6 +587,16 @@ static const struct attribute_group ina2xx_group = {
};
static struct attribute *ina226_attrs[] = {
+ &sensor_dev_attr_in0_crit.dev_attr.attr,
+ &sensor_dev_attr_in0_lcrit.dev_attr.attr,
+ &sensor_dev_attr_in0_crit_alarm.dev_attr.attr,
+ &sensor_dev_attr_in0_lcrit_alarm.dev_attr.attr,
+ &sensor_dev_attr_in1_crit.dev_attr.attr,
+ &sensor_dev_attr_in1_lcrit.dev_attr.attr,
+ &sensor_dev_attr_in1_crit_alarm.dev_attr.attr,
+ &sensor_dev_attr_in1_lcrit_alarm.dev_attr.attr,
+ &sensor_dev_attr_power1_crit.dev_attr.attr,
+ &sensor_dev_attr_power1_crit_alarm.dev_attr.attr,
&sensor_dev_attr_update_interval.dev_attr.attr,
NULL,
};
--
2.26.2.526.g744177e7f7-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] hwmon: (ina2xx) Implement alert functions
2020-05-04 22:42 [PATCH] hwmon: (ina2xx) Implement alert functions Alex Qiu
@ 2020-05-04 23:54 ` Guenter Roeck
2020-05-05 0:56 ` Alex Qiu
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Guenter Roeck @ 2020-05-04 23:54 UTC (permalink / raw)
To: Alex Qiu; +Cc: linux-hwmon
On Mon, May 04, 2020 at 03:42:47PM -0700, Alex Qiu wrote:
> Implement alert functions for INA226, INA230 and INA231. Expose 06h
> Mask/Enable and 07h Alert Limit registers via alert setting and alarm
> files.
>
> Signed-off-by: Alex Qiu <xqiu@google.com>
> ---
> Documentation/hwmon/ina2xx.rst | 19 ++++
> drivers/hwmon/ina2xx.c | 174 +++++++++++++++++++++++++++++++++
> 2 files changed, 193 insertions(+)
>
> diff --git a/Documentation/hwmon/ina2xx.rst b/Documentation/hwmon/ina2xx.rst
> index 94b9a260c518..ed81f5416331 100644
> --- a/Documentation/hwmon/ina2xx.rst
> +++ b/Documentation/hwmon/ina2xx.rst
> @@ -99,6 +99,25 @@ Sysfs entries for ina226, ina230 and ina231 only
> ------------------------------------------------
>
> ======================= ====================================================
> +in0_lcrit Critical low shunt voltage
> +in0_crit Critical high shunt voltage
> +in0_lcrit_alarm Shunt voltage critical low alarm
> +in0_crit_alarm Shunt voltage critical high alarm
> +in1_lcrit Critical low bus voltage
> +in1_crit Critical high bus voltage
> +in1_lcrit_alarm Bus voltage critical low alarm
> +in1_crit_alarm Bus voltage critical high alarm
> +power1_crit Critical high power
> +power1_crit_alarm Power critical high alarm
> update_interval data conversion time; affects number of samples used
> to average results for shunt and bus voltages.
> ======================= ====================================================
> +
> +.. note::
> +
> + - Configure `shunt_resistor` before configure `power1_crit`, because power
> + value is calculated based on `shunt_resistor` set.
> + - Because of the underlying register implementation, only one `*crit` setting
> + and its `alarm` can be active. Writing to one `*crit` setting clears other
> + `*crit` settings and alarms. Writing 0 to any `*crit` setting clears all
> + `*crit` settings and alarms.
> diff --git a/drivers/hwmon/ina2xx.c b/drivers/hwmon/ina2xx.c
> index e9e78c0b7212..0ce54e0a071d 100644
> --- a/drivers/hwmon/ina2xx.c
> +++ b/drivers/hwmon/ina2xx.c
> @@ -74,6 +74,17 @@
> #define INA226_READ_AVG(reg) (((reg) & INA226_AVG_RD_MASK) >> 9)
> #define INA226_SHIFT_AVG(val) ((val) << 9)
>
> +/* bit number of alert functions in Mask/Enable Register */
> +#define INA226_SHUNT_OVER_VOLTAGE_BIT 15
> +#define INA226_SHUNT_UNDER_VOLTAGE_BIT 14
> +#define INA226_BUS_OVER_VOLTAGE_BIT 13
> +#define INA226_BUS_UNDER_VOLTAGE_BIT 12
> +#define INA226_POWER_OVER_LIMIT_BIT 11
> +
> +/* bit mask for alert config bits of Mask/Enable Register */
> +#define INA226_ALERT_CONFIG_MASK 0xFC00
> +#define INA226_ALERT_FUNCTION_FLAG BIT(4)
> +
> /* common attrs, ina226 attrs and NULL */
> #define INA2XX_MAX_ATTRIBUTE_GROUPS 3
>
> @@ -303,6 +314,136 @@ static ssize_t ina2xx_value_show(struct device *dev,
> ina2xx_get_value(data, attr->index, regval));
> }
>
> +static int ina226_reg_to_alert(struct ina2xx_data *data, u8 bit, u16 regval)
> +{
> + int get_value_reg_map;
"reg" is really good enough as variable name here.
> +
> + switch (bit) {
> + case INA226_SHUNT_OVER_VOLTAGE_BIT:
> + case INA226_SHUNT_UNDER_VOLTAGE_BIT:
> + get_value_reg_map = INA2XX_SHUNT_VOLTAGE;
> + break;
> + case INA226_BUS_OVER_VOLTAGE_BIT:
> + case INA226_BUS_UNDER_VOLTAGE_BIT:
> + get_value_reg_map = INA2XX_BUS_VOLTAGE;
> + break;
> + case INA226_POWER_OVER_LIMIT_BIT:
> + get_value_reg_map = INA2XX_POWER;
> + break;
> + default:
> + /* programmer goofed */
> + WARN_ON_ONCE(1);
> + return 0;
> + }
> +
> + return ina2xx_get_value(data, get_value_reg_map, regval);
> +}
> +
> +/*
> + * Turns alert limit values into register values.
> + * Opposite of the formula in ina2xx_get_value().
> + */
> +static s16 ina226_alert_to_reg(struct ina2xx_data *data, u8 bit, int val)
> +{
> + switch (bit) {
> + case INA226_SHUNT_OVER_VOLTAGE_BIT:
> + case INA226_SHUNT_UNDER_VOLTAGE_BIT:
> + val *= data->config->shunt_div;
> + return clamp_val(val, SHRT_MIN, SHRT_MAX);
> + case INA226_BUS_OVER_VOLTAGE_BIT:
> + case INA226_BUS_UNDER_VOLTAGE_BIT:
> + val = (val * 1000) << data->config->bus_voltage_shift;
> + val = DIV_ROUND_CLOSEST(val, data->config->bus_voltage_lsb);
> + return clamp_val(val, 0, SHRT_MAX);
> + case INA226_POWER_OVER_LIMIT_BIT:
> + val = DIV_ROUND_CLOSEST(val, data->power_lsb_uW);
> + return clamp_val(val, 0, USHRT_MAX);
> + default:
> + /* programmer goofed */
> + WARN_ON_ONCE(1);
> + return 0;
> + }
> +}
> +
> +static ssize_t ina226_alert_show(struct device *dev,
> + struct device_attribute *da, char *buf)
> +{
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> + struct ina2xx_data *data = dev_get_drvdata(dev);
> + int regval;
> + int val = 0;
> + int ret;
> +
> + ret = regmap_read(data->regmap, INA226_MASK_ENABLE, ®val);
> + if (ret)
> + return ret;
> +
> + if (regval & BIT(attr->index)) {
> + ret = regmap_read(data->regmap, INA226_ALERT_LIMIT, ®val);
> + if (ret)
> + return ret;
> + val = ina226_reg_to_alert(data, attr->index, regval);
> + }
This will need mutex protection. Otherwise the active bit can change in
the write function between the two reads.
> +
> + return snprintf(buf, PAGE_SIZE, "%d\n", val);
> +}
> +
> +static ssize_t ina226_alert_store(struct device *dev,
> + struct device_attribute *da,
> + const char *buf, size_t count)
> +{
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> + struct ina2xx_data *data = dev_get_drvdata(dev);
> + unsigned long val;
> + int ret;
> +
> + ret = kstrtoul(buf, 10, &val);
> + if (ret < 0)
> + return ret;
> +
> + /*
> + * Clear all alerts first to avoid accidentally triggering ALERT pin
> + * due to register write sequence. Then, only enable the alert
> + * if the value is non-zero.
> + */
> + ret = regmap_update_bits(data->regmap, INA226_MASK_ENABLE,
> + INA226_ALERT_CONFIG_MASK, 0);
> + if (ret < 0)
> + return ret;
> +
> + ret = regmap_write(data->regmap, INA226_ALERT_LIMIT,
> + ina226_alert_to_reg(data, attr->index, val));
> + if (ret < 0)
> + return ret;
> +
> + if (val != 0) {
> + ret = regmap_update_bits(data->regmap, INA226_MASK_ENABLE,
> + INA226_ALERT_CONFIG_MASK,
> + BIT(attr->index));
> + if (ret < 0)
> + return ret;
> + }
Same here; the write sequence needs to be mutex protected.
> + return count;
> +}
> +
> +static ssize_t ina226_alarm_show(struct device *dev,
> + struct device_attribute *da, char *buf)
> +{
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> + struct ina2xx_data *data = dev_get_drvdata(dev);
> + int regval;
> + int alarm = 0;
> + int ret;
> +
> + ret = regmap_read(data->regmap, INA226_MASK_ENABLE, ®val);
> + if (ret)
> + return ret;
> +
> + alarm = (regval & BIT(attr->index)) &&
> + (regval & INA226_ALERT_FUNCTION_FLAG);
> + return snprintf(buf, PAGE_SIZE, "%d\n", alarm);
> +}
> +
> /*
> * In order to keep calibration register value fixed, the product
> * of current_lsb and shunt_resistor should also be fixed and equal
> @@ -392,15 +533,38 @@ static ssize_t ina226_interval_show(struct device *dev,
>
> /* shunt voltage */
> static SENSOR_DEVICE_ATTR_RO(in0_input, ina2xx_value, INA2XX_SHUNT_VOLTAGE);
> +/* shunt voltage over/under voltage alert setting and alarm */
> +static SENSOR_DEVICE_ATTR_RW(in0_crit, ina226_alert,
> + INA226_SHUNT_OVER_VOLTAGE_BIT);
> +static SENSOR_DEVICE_ATTR_RW(in0_lcrit, ina226_alert,
> + INA226_SHUNT_UNDER_VOLTAGE_BIT);
> +static SENSOR_DEVICE_ATTR_RO(in0_crit_alarm, ina226_alarm,
> + INA226_SHUNT_OVER_VOLTAGE_BIT);
> +static SENSOR_DEVICE_ATTR_RO(in0_lcrit_alarm, ina226_alarm,
> + INA226_SHUNT_UNDER_VOLTAGE_BIT);
>
> /* bus voltage */
> static SENSOR_DEVICE_ATTR_RO(in1_input, ina2xx_value, INA2XX_BUS_VOLTAGE);
> +/* bus voltage over/under voltage alert setting and alarm */
> +static SENSOR_DEVICE_ATTR_RW(in1_crit, ina226_alert,
> + INA226_BUS_OVER_VOLTAGE_BIT);
> +static SENSOR_DEVICE_ATTR_RW(in1_lcrit, ina226_alert,
> + INA226_BUS_UNDER_VOLTAGE_BIT);
> +static SENSOR_DEVICE_ATTR_RO(in1_crit_alarm, ina226_alarm,
> + INA226_BUS_OVER_VOLTAGE_BIT);
> +static SENSOR_DEVICE_ATTR_RO(in1_lcrit_alarm, ina226_alarm,
> + INA226_BUS_UNDER_VOLTAGE_BIT);
>
> /* calculated current */
> static SENSOR_DEVICE_ATTR_RO(curr1_input, ina2xx_value, INA2XX_CURRENT);
>
> /* calculated power */
> static SENSOR_DEVICE_ATTR_RO(power1_input, ina2xx_value, INA2XX_POWER);
> +/* over-limit power alert setting and alarm */
> +static SENSOR_DEVICE_ATTR_RW(power1_crit, ina226_alert,
> + INA226_POWER_OVER_LIMIT_BIT);
> +static SENSOR_DEVICE_ATTR_RO(power1_crit_alarm, ina226_alarm,
> + INA226_POWER_OVER_LIMIT_BIT);
>
> /* shunt resistance */
> static SENSOR_DEVICE_ATTR_RW(shunt_resistor, ina2xx_shunt, INA2XX_CALIBRATION);
> @@ -423,6 +587,16 @@ static const struct attribute_group ina2xx_group = {
> };
>
> static struct attribute *ina226_attrs[] = {
> + &sensor_dev_attr_in0_crit.dev_attr.attr,
> + &sensor_dev_attr_in0_lcrit.dev_attr.attr,
> + &sensor_dev_attr_in0_crit_alarm.dev_attr.attr,
> + &sensor_dev_attr_in0_lcrit_alarm.dev_attr.attr,
> + &sensor_dev_attr_in1_crit.dev_attr.attr,
> + &sensor_dev_attr_in1_lcrit.dev_attr.attr,
> + &sensor_dev_attr_in1_crit_alarm.dev_attr.attr,
> + &sensor_dev_attr_in1_lcrit_alarm.dev_attr.attr,
> + &sensor_dev_attr_power1_crit.dev_attr.attr,
> + &sensor_dev_attr_power1_crit_alarm.dev_attr.attr,
> &sensor_dev_attr_update_interval.dev_attr.attr,
> NULL,
> };
> --
> 2.26.2.526.g744177e7f7-goog
>
^ permalink raw reply [flat|nested] 6+ messages in thread* (no subject)
2020-05-04 23:54 ` Guenter Roeck
@ 2020-05-05 0:56 ` Alex Qiu
2020-05-05 0:59 ` Alex Qiu
2020-05-05 0:59 ` [PATCH v2] hwmon: (ina2xx) Implement alert functions Alex Qiu
2 siblings, 0 replies; 6+ messages in thread
From: Alex Qiu @ 2020-05-05 0:56 UTC (permalink / raw)
To: Guenter Roeck; +Cc: linux-hwmon
Hi Guenter,
Thanks for pointing these issues out! I wrapped them up with config_lock, since the shunt resistor value in memory is needed to calculate the power reading, and it's also guarded by the same lock. Thx.
^ permalink raw reply [flat|nested] 6+ messages in thread
* (no subject)
2020-05-04 23:54 ` Guenter Roeck
2020-05-05 0:56 ` Alex Qiu
@ 2020-05-05 0:59 ` Alex Qiu
2020-05-05 0:59 ` [PATCH v2] hwmon: (ina2xx) Implement alert functions Alex Qiu
2 siblings, 0 replies; 6+ messages in thread
From: Alex Qiu @ 2020-05-05 0:59 UTC (permalink / raw)
To: Guenter Roeck; +Cc: linux-hwmon
Hi Guenter,
Thanks for pointing these issues out! I wrapped them up with config_lock, since the shunt resistor value in memory is needed to calculate the power reading, and it's also guarded by the same lock. Thx.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] hwmon: (ina2xx) Implement alert functions
2020-05-04 23:54 ` Guenter Roeck
2020-05-05 0:56 ` Alex Qiu
2020-05-05 0:59 ` Alex Qiu
@ 2020-05-05 0:59 ` Alex Qiu
2020-05-05 0:59 ` Alex Qiu
2 siblings, 1 reply; 6+ messages in thread
From: Alex Qiu @ 2020-05-05 0:59 UTC (permalink / raw)
To: Guenter Roeck; +Cc: linux-hwmon
Hi Guenter,
Thanks for pointing these issues out! I wrapped them up with config_lock, since the shunt resistor value in memory is needed to calculate the power reading, and it's also guarded by the same lock. Thx.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] hwmon: (ina2xx) Implement alert functions
2020-05-05 0:59 ` [PATCH v2] hwmon: (ina2xx) Implement alert functions Alex Qiu
@ 2020-05-05 0:59 ` Alex Qiu
0 siblings, 0 replies; 6+ messages in thread
From: Alex Qiu @ 2020-05-05 0:59 UTC (permalink / raw)
To: Guenter Roeck; +Cc: linux-hwmon, Alex Qiu
Implement alert functions for INA226, INA230 and INA231. Expose 06h
Mask/Enable and 07h Alert Limit registers via alert setting and alarm
files.
Signed-off-by: Alex Qiu <xqiu@google.com>
---
Documentation/hwmon/ina2xx.rst | 19 ++++
drivers/hwmon/ina2xx.c | 183 +++++++++++++++++++++++++++++++++
2 files changed, 202 insertions(+)
diff --git a/Documentation/hwmon/ina2xx.rst b/Documentation/hwmon/ina2xx.rst
index 94b9a260c518..ed81f5416331 100644
--- a/Documentation/hwmon/ina2xx.rst
+++ b/Documentation/hwmon/ina2xx.rst
@@ -99,6 +99,25 @@ Sysfs entries for ina226, ina230 and ina231 only
------------------------------------------------
======================= ====================================================
+in0_lcrit Critical low shunt voltage
+in0_crit Critical high shunt voltage
+in0_lcrit_alarm Shunt voltage critical low alarm
+in0_crit_alarm Shunt voltage critical high alarm
+in1_lcrit Critical low bus voltage
+in1_crit Critical high bus voltage
+in1_lcrit_alarm Bus voltage critical low alarm
+in1_crit_alarm Bus voltage critical high alarm
+power1_crit Critical high power
+power1_crit_alarm Power critical high alarm
update_interval data conversion time; affects number of samples used
to average results for shunt and bus voltages.
======================= ====================================================
+
+.. note::
+
+ - Configure `shunt_resistor` before configure `power1_crit`, because power
+ value is calculated based on `shunt_resistor` set.
+ - Because of the underlying register implementation, only one `*crit` setting
+ and its `alarm` can be active. Writing to one `*crit` setting clears other
+ `*crit` settings and alarms. Writing 0 to any `*crit` setting clears all
+ `*crit` settings and alarms.
diff --git a/drivers/hwmon/ina2xx.c b/drivers/hwmon/ina2xx.c
index e9e78c0b7212..55d474ec7c35 100644
--- a/drivers/hwmon/ina2xx.c
+++ b/drivers/hwmon/ina2xx.c
@@ -74,6 +74,17 @@
#define INA226_READ_AVG(reg) (((reg) & INA226_AVG_RD_MASK) >> 9)
#define INA226_SHIFT_AVG(val) ((val) << 9)
+/* bit number of alert functions in Mask/Enable Register */
+#define INA226_SHUNT_OVER_VOLTAGE_BIT 15
+#define INA226_SHUNT_UNDER_VOLTAGE_BIT 14
+#define INA226_BUS_OVER_VOLTAGE_BIT 13
+#define INA226_BUS_UNDER_VOLTAGE_BIT 12
+#define INA226_POWER_OVER_LIMIT_BIT 11
+
+/* bit mask for alert config bits of Mask/Enable Register */
+#define INA226_ALERT_CONFIG_MASK 0xFC00
+#define INA226_ALERT_FUNCTION_FLAG BIT(4)
+
/* common attrs, ina226 attrs and NULL */
#define INA2XX_MAX_ATTRIBUTE_GROUPS 3
@@ -303,6 +314,145 @@ static ssize_t ina2xx_value_show(struct device *dev,
ina2xx_get_value(data, attr->index, regval));
}
+static int ina226_reg_to_alert(struct ina2xx_data *data, u8 bit, u16 regval)
+{
+ int reg;
+
+ switch (bit) {
+ case INA226_SHUNT_OVER_VOLTAGE_BIT:
+ case INA226_SHUNT_UNDER_VOLTAGE_BIT:
+ reg = INA2XX_SHUNT_VOLTAGE;
+ break;
+ case INA226_BUS_OVER_VOLTAGE_BIT:
+ case INA226_BUS_UNDER_VOLTAGE_BIT:
+ reg = INA2XX_BUS_VOLTAGE;
+ break;
+ case INA226_POWER_OVER_LIMIT_BIT:
+ reg = INA2XX_POWER;
+ break;
+ default:
+ /* programmer goofed */
+ WARN_ON_ONCE(1);
+ return 0;
+ }
+
+ return ina2xx_get_value(data, reg, regval);
+}
+
+/*
+ * Turns alert limit values into register values.
+ * Opposite of the formula in ina2xx_get_value().
+ */
+static s16 ina226_alert_to_reg(struct ina2xx_data *data, u8 bit, int val)
+{
+ switch (bit) {
+ case INA226_SHUNT_OVER_VOLTAGE_BIT:
+ case INA226_SHUNT_UNDER_VOLTAGE_BIT:
+ val *= data->config->shunt_div;
+ return clamp_val(val, SHRT_MIN, SHRT_MAX);
+ case INA226_BUS_OVER_VOLTAGE_BIT:
+ case INA226_BUS_UNDER_VOLTAGE_BIT:
+ val = (val * 1000) << data->config->bus_voltage_shift;
+ val = DIV_ROUND_CLOSEST(val, data->config->bus_voltage_lsb);
+ return clamp_val(val, 0, SHRT_MAX);
+ case INA226_POWER_OVER_LIMIT_BIT:
+ val = DIV_ROUND_CLOSEST(val, data->power_lsb_uW);
+ return clamp_val(val, 0, USHRT_MAX);
+ default:
+ /* programmer goofed */
+ WARN_ON_ONCE(1);
+ return 0;
+ }
+}
+
+static ssize_t ina226_alert_show(struct device *dev,
+ struct device_attribute *da, char *buf)
+{
+ struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
+ struct ina2xx_data *data = dev_get_drvdata(dev);
+ int regval;
+ int val = 0;
+ int ret;
+
+ mutex_lock(&data->config_lock);
+ ret = regmap_read(data->regmap, INA226_MASK_ENABLE, ®val);
+ if (ret)
+ goto abort;
+
+ if (regval & BIT(attr->index)) {
+ ret = regmap_read(data->regmap, INA226_ALERT_LIMIT, ®val);
+ if (ret)
+ goto abort;
+ val = ina226_reg_to_alert(data, attr->index, regval);
+ }
+
+ ret = snprintf(buf, PAGE_SIZE, "%d\n", val);
+abort:
+ mutex_unlock(&data->config_lock);
+ return ret;
+}
+
+static ssize_t ina226_alert_store(struct device *dev,
+ struct device_attribute *da,
+ const char *buf, size_t count)
+{
+ struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
+ struct ina2xx_data *data = dev_get_drvdata(dev);
+ unsigned long val;
+ int ret;
+
+ ret = kstrtoul(buf, 10, &val);
+ if (ret < 0)
+ return ret;
+
+ /*
+ * Clear all alerts first to avoid accidentally triggering ALERT pin
+ * due to register write sequence. Then, only enable the alert
+ * if the value is non-zero.
+ */
+ mutex_lock(&data->config_lock);
+ ret = regmap_update_bits(data->regmap, INA226_MASK_ENABLE,
+ INA226_ALERT_CONFIG_MASK, 0);
+ if (ret < 0)
+ goto abort;
+
+ ret = regmap_write(data->regmap, INA226_ALERT_LIMIT,
+ ina226_alert_to_reg(data, attr->index, val));
+ if (ret < 0)
+ goto abort;
+
+ if (val != 0) {
+ ret = regmap_update_bits(data->regmap, INA226_MASK_ENABLE,
+ INA226_ALERT_CONFIG_MASK,
+ BIT(attr->index));
+ if (ret < 0)
+ goto abort;
+ }
+
+ ret = count;
+abort:
+ mutex_unlock(&data->config_lock);
+ return ret;
+}
+
+static ssize_t ina226_alarm_show(struct device *dev,
+ struct device_attribute *da, char *buf)
+{
+ struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
+ struct ina2xx_data *data = dev_get_drvdata(dev);
+ int regval;
+ int alarm = 0;
+ int ret;
+
+ ret = regmap_read(data->regmap, INA226_MASK_ENABLE, ®val);
+ if (ret)
+ return ret;
+
+ alarm = (regval & BIT(attr->index)) &&
+ (regval & INA226_ALERT_FUNCTION_FLAG);
+ return snprintf(buf, PAGE_SIZE, "%d\n", alarm);
+}
+
/*
* In order to keep calibration register value fixed, the product
* of current_lsb and shunt_resistor should also be fixed and equal
@@ -392,15 +542,38 @@ static ssize_t ina226_interval_show(struct device *dev,
/* shunt voltage */
static SENSOR_DEVICE_ATTR_RO(in0_input, ina2xx_value, INA2XX_SHUNT_VOLTAGE);
+/* shunt voltage over/under voltage alert setting and alarm */
+static SENSOR_DEVICE_ATTR_RW(in0_crit, ina226_alert,
+ INA226_SHUNT_OVER_VOLTAGE_BIT);
+static SENSOR_DEVICE_ATTR_RW(in0_lcrit, ina226_alert,
+ INA226_SHUNT_UNDER_VOLTAGE_BIT);
+static SENSOR_DEVICE_ATTR_RO(in0_crit_alarm, ina226_alarm,
+ INA226_SHUNT_OVER_VOLTAGE_BIT);
+static SENSOR_DEVICE_ATTR_RO(in0_lcrit_alarm, ina226_alarm,
+ INA226_SHUNT_UNDER_VOLTAGE_BIT);
/* bus voltage */
static SENSOR_DEVICE_ATTR_RO(in1_input, ina2xx_value, INA2XX_BUS_VOLTAGE);
+/* bus voltage over/under voltage alert setting and alarm */
+static SENSOR_DEVICE_ATTR_RW(in1_crit, ina226_alert,
+ INA226_BUS_OVER_VOLTAGE_BIT);
+static SENSOR_DEVICE_ATTR_RW(in1_lcrit, ina226_alert,
+ INA226_BUS_UNDER_VOLTAGE_BIT);
+static SENSOR_DEVICE_ATTR_RO(in1_crit_alarm, ina226_alarm,
+ INA226_BUS_OVER_VOLTAGE_BIT);
+static SENSOR_DEVICE_ATTR_RO(in1_lcrit_alarm, ina226_alarm,
+ INA226_BUS_UNDER_VOLTAGE_BIT);
/* calculated current */
static SENSOR_DEVICE_ATTR_RO(curr1_input, ina2xx_value, INA2XX_CURRENT);
/* calculated power */
static SENSOR_DEVICE_ATTR_RO(power1_input, ina2xx_value, INA2XX_POWER);
+/* over-limit power alert setting and alarm */
+static SENSOR_DEVICE_ATTR_RW(power1_crit, ina226_alert,
+ INA226_POWER_OVER_LIMIT_BIT);
+static SENSOR_DEVICE_ATTR_RO(power1_crit_alarm, ina226_alarm,
+ INA226_POWER_OVER_LIMIT_BIT);
/* shunt resistance */
static SENSOR_DEVICE_ATTR_RW(shunt_resistor, ina2xx_shunt, INA2XX_CALIBRATION);
@@ -423,6 +596,16 @@ static const struct attribute_group ina2xx_group = {
};
static struct attribute *ina226_attrs[] = {
+ &sensor_dev_attr_in0_crit.dev_attr.attr,
+ &sensor_dev_attr_in0_lcrit.dev_attr.attr,
+ &sensor_dev_attr_in0_crit_alarm.dev_attr.attr,
+ &sensor_dev_attr_in0_lcrit_alarm.dev_attr.attr,
+ &sensor_dev_attr_in1_crit.dev_attr.attr,
+ &sensor_dev_attr_in1_lcrit.dev_attr.attr,
+ &sensor_dev_attr_in1_crit_alarm.dev_attr.attr,
+ &sensor_dev_attr_in1_lcrit_alarm.dev_attr.attr,
+ &sensor_dev_attr_power1_crit.dev_attr.attr,
+ &sensor_dev_attr_power1_crit_alarm.dev_attr.attr,
&sensor_dev_attr_update_interval.dev_attr.attr,
NULL,
};
--
2.26.2.526.g744177e7f7-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-05-05 1:00 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-04 22:42 [PATCH] hwmon: (ina2xx) Implement alert functions Alex Qiu
2020-05-04 23:54 ` Guenter Roeck
2020-05-05 0:56 ` Alex Qiu
2020-05-05 0:59 ` Alex Qiu
2020-05-05 0:59 ` [PATCH v2] hwmon: (ina2xx) Implement alert functions Alex Qiu
2020-05-05 0:59 ` Alex Qiu
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).