Implement support for using the values given in the
isil,battery-trip-levels-microvolt property to set appropriate values
in the VB85TP/VB75TP bits in the PWR_VBAT register.
Signed-off-by: Rasmus Villemoes <[email protected]>
---
drivers/rtc/rtc-isl12022.c | 39 ++++++++++++++++++++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/drivers/rtc/rtc-isl12022.c b/drivers/rtc/rtc-isl12022.c
index ebd66b835cef..6a757f0a4736 100644
--- a/drivers/rtc/rtc-isl12022.c
+++ b/drivers/rtc/rtc-isl12022.c
@@ -9,6 +9,7 @@
*/
#include <linux/bcd.h>
+#include <linux/bitfield.h>
#include <linux/err.h>
#include <linux/hwmon.h>
#include <linux/i2c.h>
@@ -31,6 +32,8 @@
#define ISL12022_REG_SR 0x07
#define ISL12022_REG_INT 0x08
+#define ISL12022_REG_PWR_VBAT 0x0a
+
#define ISL12022_REG_BETA 0x0d
#define ISL12022_REG_TEMP_L 0x28
@@ -42,6 +45,9 @@
#define ISL12022_INT_WRTC (1 << 6)
+#define ISL12022_REG_VB85_MASK GENMASK(5, 3)
+#define ISL12022_REG_VB75_MASK GENMASK(2, 0)
+
#define ISL12022_BETA_TSE (1 << 7)
static umode_t isl12022_hwmon_is_visible(const void *data,
@@ -209,6 +215,38 @@ static const struct regmap_config regmap_config = {
.use_single_write = true,
};
+static const u32 trip_levels[2][7] = {
+ { 2125000, 2295000, 2550000, 2805000, 3060000, 4250000, 4675000 },
+ { 1875000, 2025000, 2250000, 2475000, 2700000, 3750000, 4125000 },
+};
+
+static void isl12022_set_trip_levels(struct device *dev)
+{
+ struct regmap *regmap = dev_get_drvdata(dev);
+ u32 levels[2] = {0, 0};
+ int ret, i, j, x[2];
+ u8 val, mask;
+
+ device_property_read_u32_array(dev, "isil,battery-trip-levels-microvolt",
+ levels, 2);
+
+ for (i = 0; i < 2; i++) {
+ for (j = 0; j < ARRAY_SIZE(trip_levels[i]) - 1; j++) {
+ if (levels[i] <= trip_levels[i][j])
+ break;
+ }
+ x[i] = j;
+ }
+
+ val = FIELD_PREP(ISL12022_REG_VB85_MASK, x[0]) |
+ FIELD_PREP(ISL12022_REG_VB75_MASK, x[1]);
+ mask = ISL12022_REG_VB85_MASK | ISL12022_REG_VB75_MASK;
+
+ ret = regmap_update_bits(regmap, ISL12022_REG_PWR_VBAT, mask, val);
+ if (ret)
+ dev_warn(dev, "unable to set battery alarm levels: %d\n", ret);
+}
+
static int isl12022_probe(struct i2c_client *client)
{
struct rtc_device *rtc;
@@ -225,6 +263,7 @@ static int isl12022_probe(struct i2c_client *client)
dev_set_drvdata(&client->dev, regmap);
+ isl12022_set_trip_levels(&client->dev);
isl12022_hwmon_register(&client->dev);
rtc = devm_rtc_allocate_device(&client->dev);
--
2.37.2
On Thu, Jun 15, 2023 at 12:58:22PM +0200, Rasmus Villemoes wrote:
> Implement support for using the values given in the
> isil,battery-trip-levels-microvolt property to set appropriate values
> in the VB85TP/VB75TP bits in the PWR_VBAT register.
A few nit-picks below.
...
> +static void isl12022_set_trip_levels(struct device *dev)
> +{
> + struct regmap *regmap = dev_get_drvdata(dev);
> + u32 levels[2] = {0, 0};
A nit, 0, 0 is not needed, {} will do the job.
> + int ret, i, j, x[2];
> + u8 val, mask;
BUILD_BUG_ON(ARRAY_SIZE(x) != ARRAY_SIZE(levels)) ?
> + device_property_read_u32_array(dev, "isil,battery-trip-levels-microvolt",
> + levels, 2);
A nit, ARRAY_SIZE(levels) ?
> + for (i = 0; i < 2; i++) {
ARRAY_SIZE(x) ?
> + for (j = 0; j < ARRAY_SIZE(trip_levels[i]) - 1; j++) {
> + if (levels[i] <= trip_levels[i][j])
> + break;
> + }
> + x[i] = j;
> + }
> +
> + val = FIELD_PREP(ISL12022_REG_VB85_MASK, x[0]) |
> + FIELD_PREP(ISL12022_REG_VB75_MASK, x[1]);
> + mask = ISL12022_REG_VB85_MASK | ISL12022_REG_VB75_MASK;
> +
> + ret = regmap_update_bits(regmap, ISL12022_REG_PWR_VBAT, mask, val);
> + if (ret)
> + dev_warn(dev, "unable to set battery alarm levels: %d\n", ret);
> +}
--
With Best Regards,
Andy Shevchenko
On 15/06/2023 13.11, Andy Shevchenko wrote:
> On Thu, Jun 15, 2023 at 12:58:22PM +0200, Rasmus Villemoes wrote:
>> +static void isl12022_set_trip_levels(struct device *dev)
>> +{
>> + struct regmap *regmap = dev_get_drvdata(dev);
>> + u32 levels[2] = {0, 0};
>
> A nit, 0, 0 is not needed, {} will do the job.
So? I'm not code-golfing, and here I really want to initialize to 0 (or
any value lower than the first entries in the trip_levels[] arrays so
that, lacking the DT property, the code ends up using what are the chip
reset defaults).
>> + int ret, i, j, x[2];
>> + u8 val, mask;
>
> BUILD_BUG_ON(ARRAY_SIZE(x) != ARRAY_SIZE(levels)) ?
BUILD_BUG_ON doesn't make sense when these things are declared within a
few lines of each other _and_ since they're sized based on properties of
the hardware we're dealing with, nobody would ever have a reason to
change either. So no, that would IMO make it harder to read, because one
would stop and think "why is this obvious thing asserted?".
>> + device_property_read_u32_array(dev, "isil,battery-trip-levels-microvolt",
>> + levels, 2);
>
> A nit, ARRAY_SIZE(levels) ?
>
>> + for (i = 0; i < 2; i++) {
>
> ARRAY_SIZE(x) ?
I considered that, but really didn't think it improved readability. I'll
defer to Alexandre on whether to change this.
Rasmus