Hook up support for reading the values of the SR_LBAT85 and SR_LBAT75
bits. Translate the former to "battery low", and the latter to
"battery empty or not-present".
Signed-off-by: Rasmus Villemoes <[email protected]>
---
drivers/rtc/rtc-isl12022.c | 41 ++++++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)
diff --git a/drivers/rtc/rtc-isl12022.c b/drivers/rtc/rtc-isl12022.c
index cb8f1d92e116..1b6659a9b33a 100644
--- a/drivers/rtc/rtc-isl12022.c
+++ b/drivers/rtc/rtc-isl12022.c
@@ -203,7 +203,48 @@ static int isl12022_rtc_set_time(struct device *dev, struct rtc_time *tm)
return regmap_bulk_write(regmap, ISL12022_REG_SC, buf, sizeof(buf));
}
+static int isl12022_read_sr(struct regmap *regmap)
+{
+ int ret;
+ u32 val;
+
+ ret = regmap_read(regmap, ISL12022_REG_SR, &val);
+ if (ret < 0)
+ return ret;
+ return val;
+}
+
+static int isl12022_rtc_ioctl(struct device *dev, unsigned int cmd, unsigned long arg)
+{
+ struct regmap *regmap = dev_get_drvdata(dev);
+ u32 user = 0;
+ int ret;
+
+ switch (cmd) {
+ case RTC_VL_READ:
+ ret = isl12022_read_sr(regmap);
+ if (ret < 0)
+ return ret;
+
+ if (ret & ISL12022_SR_LBAT85)
+ user |= RTC_VL_BACKUP_LOW;
+
+ if (ret & ISL12022_SR_LBAT75)
+ user |= RTC_VL_BACKUP_EMPTY;
+
+ return put_user(user, (u32 __user *)arg);
+
+ case RTC_VL_CLR:
+ return regmap_clear_bits(regmap, ISL12022_REG_SR,
+ ISL12022_SR_LBAT85 | ISL12022_SR_LBAT75);
+
+ default:
+ return -ENOIOCTLCMD;
+ }
+}
+
static const struct rtc_class_ops isl12022_rtc_ops = {
+ .ioctl = isl12022_rtc_ioctl,
.read_time = isl12022_rtc_read_time,
.set_time = isl12022_rtc_set_time,
};
--
2.37.2
On 12/06/2023 13:30:55+0200, Rasmus Villemoes wrote:
> Hook up support for reading the values of the SR_LBAT85 and SR_LBAT75
> bits. Translate the former to "battery low", and the latter to
> "battery empty or not-present".
>
> Signed-off-by: Rasmus Villemoes <[email protected]>
> ---
> drivers/rtc/rtc-isl12022.c | 41 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 41 insertions(+)
>
> diff --git a/drivers/rtc/rtc-isl12022.c b/drivers/rtc/rtc-isl12022.c
> index cb8f1d92e116..1b6659a9b33a 100644
> --- a/drivers/rtc/rtc-isl12022.c
> +++ b/drivers/rtc/rtc-isl12022.c
> @@ -203,7 +203,48 @@ static int isl12022_rtc_set_time(struct device *dev, struct rtc_time *tm)
> return regmap_bulk_write(regmap, ISL12022_REG_SC, buf, sizeof(buf));
> }
>
> +static int isl12022_read_sr(struct regmap *regmap)
> +{
> + int ret;
> + u32 val;
> +
> + ret = regmap_read(regmap, ISL12022_REG_SR, &val);
> + if (ret < 0)
> + return ret;
> + return val;
> +}
> +
> +static int isl12022_rtc_ioctl(struct device *dev, unsigned int cmd, unsigned long arg)
> +{
> + struct regmap *regmap = dev_get_drvdata(dev);
> + u32 user = 0;
> + int ret;
> +
> + switch (cmd) {
> + case RTC_VL_READ:
> + ret = isl12022_read_sr(regmap);
> + if (ret < 0)
> + return ret;
> +
> + if (ret & ISL12022_SR_LBAT85)
> + user |= RTC_VL_BACKUP_LOW;
> +
> + if (ret & ISL12022_SR_LBAT75)
> + user |= RTC_VL_BACKUP_EMPTY;
> +
> + return put_user(user, (u32 __user *)arg);
> +
> + case RTC_VL_CLR:
> + return regmap_clear_bits(regmap, ISL12022_REG_SR,
> + ISL12022_SR_LBAT85 | ISL12022_SR_LBAT75);
I'm against using RTC_VL_CLR for this as it deletes important
information (i.e. the date is probably invalid). You should let the RTC
clear the bits once the battery has been changed:
"The LBAT75 bit is set when the
VBAT has dropped below the pre-selected trip level, and will self
clear when the VBAT is above the pre-selected trip level at the
next detection cycle either by manual or automatic trigger."
> +
> + default:
> + return -ENOIOCTLCMD;
> + }
> +}
> +
> static const struct rtc_class_ops isl12022_rtc_ops = {
> + .ioctl = isl12022_rtc_ioctl,
> .read_time = isl12022_rtc_read_time,
> .set_time = isl12022_rtc_set_time,
> };
> --
> 2.37.2
>
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
On Mon, Jun 12, 2023 at 01:30:55PM +0200, Rasmus Villemoes wrote:
> Hook up support for reading the values of the SR_LBAT85 and SR_LBAT75
> bits. Translate the former to "battery low", and the latter to
> "battery empty or not-present".
...
> +static int isl12022_read_sr(struct regmap *regmap)
> +{
> + int ret;
> + u32 val;
> +
> + ret = regmap_read(regmap, ISL12022_REG_SR, &val);
> + if (ret < 0)
> + return ret;
> + return val;
Wondering if the bit 31 is in use with this register (note, I haven't checked
the register width nor datasheet).
> +}
--
With Best Regards,
Andy Shevchenko
On 12/06/2023 18:48:49+0300, Andy Shevchenko wrote:
> On Mon, Jun 12, 2023 at 01:30:55PM +0200, Rasmus Villemoes wrote:
> > Hook up support for reading the values of the SR_LBAT85 and SR_LBAT75
> > bits. Translate the former to "battery low", and the latter to
> > "battery empty or not-present".
>
> ...
>
> > +static int isl12022_read_sr(struct regmap *regmap)
> > +{
> > + int ret;
> > + u32 val;
> > +
> > + ret = regmap_read(regmap, ISL12022_REG_SR, &val);
> > + if (ret < 0)
> > + return ret;
> > + return val;
>
> Wondering if the bit 31 is in use with this register (note, I haven't checked
> the register width nor datasheet).
>
register width is in the driver:
static const struct regmap_config regmap_config = {
.reg_bits = 8,
.val_bits = 8,
.use_single_write = true,
};
> > +}
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
On 12/06/2023 16.07, Alexandre Belloni wrote:
> On 12/06/2023 13:30:55+0200, Rasmus Villemoes wrote:
>> +static int isl12022_rtc_ioctl(struct device *dev, unsigned int cmd, unsigned long arg)
>> +{
>> + struct regmap *regmap = dev_get_drvdata(dev);
>> + u32 user = 0;
>> + int ret;
>> +
>> + switch (cmd) {
>> + case RTC_VL_READ:
>> + ret = isl12022_read_sr(regmap);
>> + if (ret < 0)
>> + return ret;
>> +
>> + if (ret & ISL12022_SR_LBAT85)
>> + user |= RTC_VL_BACKUP_LOW;
>> +
>> + if (ret & ISL12022_SR_LBAT75)
>> + user |= RTC_VL_BACKUP_EMPTY;
>> +
>> + return put_user(user, (u32 __user *)arg);
>> +
>> + case RTC_VL_CLR:
>> + return regmap_clear_bits(regmap, ISL12022_REG_SR,
>> + ISL12022_SR_LBAT85 | ISL12022_SR_LBAT75);
>
> I'm against using RTC_VL_CLR for this as it deletes important
> information (i.e. the date is probably invalid). You should let the RTC
> clear the bits once the battery has been changed:
>
> "The LBAT75 bit is set when the
> VBAT has dropped below the pre-selected trip level, and will self
> clear when the VBAT is above the pre-selected trip level at the
> next detection cycle either by manual or automatic trigger."
Well, the same thing means that the bit would get set again within a
minute after the RTC_VL_CLR, so the information isn't lost as such. I
actually don't understand what RTC_VL_CLR would be for if not this
(though, again, in this case at least it would only have a very
short-lived effect), but I'm perfectly happy to just rip out the
RTC_VL_CLR case.
Rasmus
On 12/06/2023 18.10, Alexandre Belloni wrote:
> On 12/06/2023 18:48:49+0300, Andy Shevchenko wrote:
>> On Mon, Jun 12, 2023 at 01:30:55PM +0200, Rasmus Villemoes wrote:
>>> Hook up support for reading the values of the SR_LBAT85 and SR_LBAT75
>>> bits. Translate the former to "battery low", and the latter to
>>> "battery empty or not-present".
>>
>> ...
>>
>>> +static int isl12022_read_sr(struct regmap *regmap)
>>> +{
>>> + int ret;
>>> + u32 val;
>>> +
>>> + ret = regmap_read(regmap, ISL12022_REG_SR, &val);
>>> + if (ret < 0)
>>> + return ret;
>>> + return val;
>>
>> Wondering if the bit 31 is in use with this register (note, I haven't checked
>> the register width nor datasheet).
>>
>
> register width is in the driver:
>
> static const struct regmap_config regmap_config = {
> .reg_bits = 8,
> .val_bits = 8,
> .use_single_write = true,
> };
Yeah.
But I only factored that out because I wanted to read the SR also in the
isl12022_set_trip_levels() to emit the warning at boot time, but when
that goes away, there's no longer any reason to not just fold this back
into the ioctl() handler.
Rasmus
On 13/06/2023 09:53:03+0200, Rasmus Villemoes wrote:
> On 12/06/2023 18.10, Alexandre Belloni wrote:
> > On 12/06/2023 18:48:49+0300, Andy Shevchenko wrote:
> >> On Mon, Jun 12, 2023 at 01:30:55PM +0200, Rasmus Villemoes wrote:
> >>> Hook up support for reading the values of the SR_LBAT85 and SR_LBAT75
> >>> bits. Translate the former to "battery low", and the latter to
> >>> "battery empty or not-present".
> >>
> >> ...
> >>
> >>> +static int isl12022_read_sr(struct regmap *regmap)
> >>> +{
> >>> + int ret;
> >>> + u32 val;
> >>> +
> >>> + ret = regmap_read(regmap, ISL12022_REG_SR, &val);
> >>> + if (ret < 0)
> >>> + return ret;
> >>> + return val;
> >>
> >> Wondering if the bit 31 is in use with this register (note, I haven't checked
> >> the register width nor datasheet).
> >>
> >
> > register width is in the driver:
> >
> > static const struct regmap_config regmap_config = {
> > .reg_bits = 8,
> > .val_bits = 8,
> > .use_single_write = true,
> > };
>
> Yeah.
>
> But I only factored that out because I wanted to read the SR also in the
> isl12022_set_trip_levels() to emit the warning at boot time, but when
> that goes away, there's no longer any reason to not just fold this back
> into the ioctl() handler.
That would be to clear a not self clearable battery low (but not empty)
flag or a backup voltage switch flag.
>
> Rasmus
>
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com