Re: pcfrtc(4) fix

2020-04-27 Thread Mark Kettenis
> Date: Sat, 25 Apr 2020 00:41:38 +0200 (CEST)
> From: Mark Kettenis 
> 
> The chip will set the OSF flag whenever the internal oscillator stops
> running.  That happens for example wen the battry runs out of juice.
> The idea as that we can check this flag to decide whether we should
> trust the time in the chip.  The driver tries to implement that but
> fails because it always clears the flag in the attach function.
> 
> Diff below fixes that by leaving the flag alone in the attach
> function, but clearing it when we set the time.
> 
> ok?

ping?

> Index: dev/i2c/pcf8523.c
> ===
> RCS file: /cvs/src/sys/dev/i2c/pcf8523.c,v
> retrieving revision 1.3
> diff -u -p -r1.3 pcf8523.c
> --- dev/i2c/pcf8523.c 20 May 2016 20:33:53 -  1.3
> +++ dev/i2c/pcf8523.c 24 Apr 2020 22:37:20 -
> @@ -165,13 +165,6 @@ pcfrtc_attach(struct device *parent, str
>   /* Report battery status. */
>   reg = pcfrtc_reg_read(sc, PCF8523_CONTROL3);
>   printf(": battery %s\n", (reg & PCF8523_CONTROL3_BLF) ? "low" : "ok");
> -
> - /* Clear OS flag.  */
> - reg = pcfrtc_reg_read(sc, PCF8523_SECONDS);
> - if (reg & PCF8523_SECONDS_OS) {
> - reg &= ~PCF8523_SECONDS_OS;
> - pcfrtc_reg_write(sc, PCF8523_SECONDS, reg);
> - }
>  }
>  
>  int
> @@ -194,11 +187,19 @@ pcfrtc_settime(struct todr_chip_handle *
>  {
>   struct pcfrtc_softc *sc = ch->cookie;
>   struct clock_ymdhms dt;
> + uint8_t reg;
>  
>   clock_secs_to_ymdhms(tv->tv_sec, &dt);
> -
>   if (pcfrtc_clock_write(sc, &dt) == 0)
>   return (-1);
> +
> + /* Clear OS flag.  */
> + reg = pcfrtc_reg_read(sc, PCF8523_SECONDS);
> + if (reg & PCF8523_SECONDS_OS) {
> + reg &= ~PCF8523_SECONDS_OS;
> + pcfrtc_reg_write(sc, PCF8523_SECONDS, reg);
> + }
> +
>   return (0);
>  }
>  
> 
> 



pcfrtc(4) fix

2020-04-24 Thread Mark Kettenis
The chip will set the OSF flag whenever the internal oscillator stops
running.  That happens for example wen the battry runs out of juice.
The idea as that we can check this flag to decide whether we should
trust the time in the chip.  The driver tries to implement that but
fails because it always clears the flag in the attach function.

Diff below fixes that by leaving the flag alone in the attach
function, but clearing it when we set the time.

ok?


Index: dev/i2c/pcf8523.c
===
RCS file: /cvs/src/sys/dev/i2c/pcf8523.c,v
retrieving revision 1.3
diff -u -p -r1.3 pcf8523.c
--- dev/i2c/pcf8523.c   20 May 2016 20:33:53 -  1.3
+++ dev/i2c/pcf8523.c   24 Apr 2020 22:37:20 -
@@ -165,13 +165,6 @@ pcfrtc_attach(struct device *parent, str
/* Report battery status. */
reg = pcfrtc_reg_read(sc, PCF8523_CONTROL3);
printf(": battery %s\n", (reg & PCF8523_CONTROL3_BLF) ? "low" : "ok");
-
-   /* Clear OS flag.  */
-   reg = pcfrtc_reg_read(sc, PCF8523_SECONDS);
-   if (reg & PCF8523_SECONDS_OS) {
-   reg &= ~PCF8523_SECONDS_OS;
-   pcfrtc_reg_write(sc, PCF8523_SECONDS, reg);
-   }
 }
 
 int
@@ -194,11 +187,19 @@ pcfrtc_settime(struct todr_chip_handle *
 {
struct pcfrtc_softc *sc = ch->cookie;
struct clock_ymdhms dt;
+   uint8_t reg;
 
clock_secs_to_ymdhms(tv->tv_sec, &dt);
-
if (pcfrtc_clock_write(sc, &dt) == 0)
return (-1);
+
+   /* Clear OS flag.  */
+   reg = pcfrtc_reg_read(sc, PCF8523_SECONDS);
+   if (reg & PCF8523_SECONDS_OS) {
+   reg &= ~PCF8523_SECONDS_OS;
+   pcfrtc_reg_write(sc, PCF8523_SECONDS, reg);
+   }
+
return (0);
 }