lkml.org 
[lkml]   [2019]   [Feb]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] rtc: rs5c372: Fix reading from rtc when the oscillator got interrupted.
Hi,

On 11/01/2019 13:15:39+0000, Oliver.Rohe@wago.com wrote:
> When the oscillator of the rtc gets interrupted,
> e.g. due to an empty battery, reading from the rtc will now return an error
> and the oscillator bit will be cleared, once the rtc is successfully reset.
>
> Signed-off-by: Oliver Rohe <oliver.rohe@wago.com>
> ---
> drivers/rtc/rtc-rs5c372.c | 60 ++++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 52 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/rtc/rtc-rs5c372.c b/drivers/rtc/rtc-rs5c372.c
> index ff35dce..91d1475 100644
> --- a/drivers/rtc/rtc-rs5c372.c
> +++ b/drivers/rtc/rtc-rs5c372.c
> @@ -52,8 +52,10 @@
> # define RS5C_CTRL1_CT4 (4 << 0) /* 1 Hz level irq */
> #define RS5C_REG_CTRL2 15
> # define RS5C372_CTRL2_24 (1 << 5)
> -# define R2x2x_CTRL2_XSTP (1 << 5) /* only if R2x2x */
> -# define RS5C_CTRL2_XSTP (1 << 4) /* only if !R2x2x */
> +# define RS5C_CTRL2_XSTP (1 << 4) /* only if !R2x2x */

This should probably use tabs instead of spaces

> +# define R2x2x_CTRL2_VDET (1 << 6) /* only if R2x2x */
> +# define R2x2x_CTRL2_XSTP (1 << 5) /* only if R2x2x */
> +# define R2x2x_CTRL2_PON (1 << 4) /* only if R2x2x */
> # define RS5C_CTRL2_CTFG (1 << 2)
> # define RS5C_CTRL2_AAFG (1 << 1) /* or WAFG */
> # define RS5C_CTRL2_BAFG (1 << 0) /* or DAFG */
> @@ -212,10 +214,31 @@ static int rs5c372_rtc_read_time(struct device *dev, struct rtc_time *tm)
> struct i2c_client *client = to_i2c_client(dev);
> struct rs5c372 *rs5c = i2c_get_clientdata(client);
> int status = rs5c_get_regs(rs5c);
> + unsigned char ctrl2 = rs5c->regs[RS5C_REG_CTRL2];
>
> if (status < 0)
> return status;
>
> + switch (rs5c->type) {
> + case rtc_r2025sd:
> + case rtc_r2221tl:
> + if (ctrl2 & R2x2x_CTRL2_VDET)
> + dev_warn(&client->dev, "rtc battery voltage drop below threshold detected.\n");

VDET doesn't mean anything specific regarding timekeeping so I wouldn't warn here.

> + if (ctrl2 & R2x2x_CTRL2_PON)
> + dev_warn(&client->dev, "rtc battery voltage drop to zero detected.\n");

You should return -EINVAL directly here

> + if ((rs5c->type == rtc_r2025sd && !(ctrl2 & R2x2x_CTRL2_XSTP)) ||
> + (rs5c->type == rtc_r2221tl && (ctrl2 & R2x2x_CTRL2_XSTP))) {
> + dev_warn(&client->dev, "rtc oscillator interruption detected. Please reset the rtc clock.\n");
> + return -EIO;

-EINVAL is the correct error code here.

> + }
> + break;
> + default:
> + if (ctrl2 & RS5C_CTRL2_XSTP) {
> + dev_warn(&client->dev, "rtc oscillator interruption detected. Please reset the rtc clock.\n");
> + return -EIO;

and here.


--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
\
 
 \ /
  Last update: 2019-02-05 22:56    [W:0.073 / U:14.984 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site