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
