On 5/30/24 01:06, Amna Waseem wrote:
On 5/29/24 16:07, Guenter Roeck wrote:
On 5/28/24 23:07, Amna Waseem wrote:
The INA230 has an Alert pin which is asserted when the alert
function selected in the Mask/Enable register exceeds the
value programmed into the Alert Limit register. Assertion is based
on the Alert Polarity Bit (APOL, bit 1 of the Mask/Enable register).
It is default set to value 0 i.e Normal (active-low open collector).
However, hardware can be designed in such a way that expects Alert pin
to become active high if a user-defined threshold in Alert limit
register has been exceeded. This patch adds a way to pass alert polarity
value to the driver via device tree.
Signed-off-by: Amna Waseem <Amna.Waseem@xxxxxxxx>
---
drivers/hwmon/ina2xx.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/drivers/hwmon/ina2xx.c b/drivers/hwmon/ina2xx.c
index d8415d1f21fc..b58e795bdc8f 100644
--- a/drivers/hwmon/ina2xx.c
+++ b/drivers/hwmon/ina2xx.c
@@ -73,6 +73,9 @@
#define INA226_READ_AVG(reg) (((reg) & INA226_AVG_RD_MASK) >> 9)
#define INA226_SHIFT_AVG(val) ((val) << 9)
+#define INA226_ALERT_POLARITY_MASK 0x0002
+#define INA226_SHIFT_ALERT_POLARITY(val) ((val) << 1)
+
/* bit number of alert functions in Mask/Enable Register */
#define INA226_SHUNT_OVER_VOLTAGE_BIT 15
#define INA226_SHUNT_UNDER_VOLTAGE_BIT 14
@@ -178,6 +181,23 @@ static u16 ina226_interval_to_reg(int interval)
return INA226_SHIFT_AVG(avg_bits);
}
+static int ina2xx_set_alert_polarity(struct ina2xx_data *data,
+ unsigned long val)
+{
+ int ret;
+
+ if (val > INT_MAX || !(val == 0 || val == 1))
if (val != 0 && val !=1)
would be sufficient and much easier to understand.
Agreed.
+ return -EINVAL;
+
+ mutex_lock(&data->config_lock);
Pointless lock.
+ ret = regmap_update_bits(data->regmap, INA226_MASK_ENABLE,
+ INA226_ALERT_POLARITY_MASK,
+ INA226_SHIFT_ALERT_POLARITY(val));
+
+ mutex_unlock(&data->config_lock);
+ return ret;
+}
+
/*
* Calibration register is set to the best value, which eliminates
* truncation errors on calculating current register in hardware.
@@ -659,6 +679,14 @@ static int ina2xx_probe(struct i2c_client *client)
if (ret)
return dev_err_probe(dev, ret, "failed to enable vs regulator\n");
+ if (!of_property_read_u32(dev->of_node, "alert-polarity", &val)) {
+ ret = ina2xx_set_alert_polarity(data, val);
+ if (ret < 0)
+ return dev_err_probe(
+ dev, ret,
+ "failed to set APOL bit of Enable/Mask register\n");
+ }
INA219 and INA220 do not support alert pin configuration (or, naturally,
the mask register in the first place). This will need to be validated.
Guenter
Would "of_property_read_bool" be sufficient to check whether the property exists or not for different chips? It means that if INA219 and INA220 are being used, they will not have a property "alert-polarity" defined in their devicetree so of_property_read_bool will return false and nothing will happen for these chips.
No, that would not be sufficient, because the non-existence of the property
also has a meaning and still should be used to set the polarity.
You have two options: Not evaluate the property in the first place if
the chip doesn't support it, or return an error if the property exists
but the chip does not support it. Personally I'd go the easy route;
something like
if (chip supports it) {
ret = ina2xx_set_alert_polarity(dev, regmap);
if (ret)
return dev_err_probe(...);
}
and evaluate the property in ina2xx_set_alert_polarity().
Guenter
