Hi,
This seems ok, a few comments below:
On 26/05/2015 at 10:17:40 +0200, r...@dave-tech.it wrote :
> From: Andrea Scian <andrea.sc...@dave.eu>
>
> I'm using PCF2127 in a custom ARM-based board and, by looking into PCF2127
> datasheet
> I've found that, in my understanding, it's wrong to say that the date in
> unreliable
> if BLF (battery low flag) is set but you should use OSF flag (seconds
> register) to
> check if oscillator, for any reason, stopped.
> Battery may be low (usually below 2V5 threshold) but the date may be anyway
> correct
> (tipically date is unreliable when input voltage is below 1V2).
typically -^
> diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
> index 1ee514a..2365788 100644
> --- a/drivers/rtc/rtc-pcf2127.c
> +++ b/drivers/rtc/rtc-pcf2127.c
> @@ -59,7 +59,16 @@ static int pcf2127_get_datetime(struct i2c_client *client,
> struct rtc_time *tm)
> if (buf[PCF2127_REG_CTRL3] & 0x04) {
> pcf2127->voltage_low = 1;
> dev_info(&client->dev,
> - "low voltage detected, date/time is not reliable.\n");
> + "low voltage detected, check/replace RTC battery.\n");
> + }
> +
> + if (buf[PCF2127_REG_SC] & 0x80) {
Maybe use a define instead of 0x80, remember to use the BIT() macro.
> + dev_warn(&client->dev,
> + "oscillator stop detected, date/time is not
> reliable.\n");
I would return -EINVAL here because the result might still pass
rtc_valid_tm() but be outdated.
> + /*
> + * no need clear the flag here,
> + * it will be cleared once the new date is saved
> + */
> }
>
> dev_dbg(&client->dev,
> @@ -112,7 +121,7 @@ static int pcf2127_set_datetime(struct i2c_client
> *client, struct rtc_time *tm)
> buf[i++] = PCF2127_REG_SC;
>
> /* hours, minutes and seconds */
> - buf[i++] = bin2bcd(tm->tm_sec);
> + buf[i++] = bin2bcd(tm->tm_sec); /* this will also clear OFS flag */
> buf[i++] = bin2bcd(tm->tm_min);
> buf[i++] = bin2bcd(tm->tm_hour);
> buf[i++] = bin2bcd(tm->tm_mday);
> @@ -144,7 +153,7 @@ static int pcf2127_rtc_ioctl(struct device *dev,
> switch (cmd) {
> case RTC_VL_READ:
> if (pcf2127->voltage_low)
> - dev_info(dev, "low voltage detected, date/time is not
> reliable.\n");
> + dev_info(dev, "low voltage detected, check/replace
> battery\n");
I would also print a warning about OFS here.
--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
