Hi,
On Thu 12 Dec 19, 11:36, Paul Kocialkowski wrote:
> The RTC has a valid bit in the seconds register that indicates whether
> power was lost since the pevious time set. This bit is currently read
> once at probe time, cached and updated with set_time.
>
> Howeever, caching the bit may prevent detecting power loss at runtime
> (which can happen if the RTC's supply is distinct from the the platform's).
>
> Writing the seconds register when setting time will clear the bit,
> so there should be no downside in reading the bit directly instead of
> caching it.
>
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@xxxxxxxxxxx>
> ---
> drivers/rtc/rtc-hym8563.c | 17 +++++++----------
> 1 file changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/rtc/rtc-hym8563.c b/drivers/rtc/rtc-hym8563.c
> index 443f6d05ce29..584a7815f246 100644
> --- a/drivers/rtc/rtc-hym8563.c
> +++ b/drivers/rtc/rtc-hym8563.c
> @@ -78,7 +78,6 @@
> struct hym8563 {
> struct i2c_client *client;
> struct rtc_device *rtc;
> - bool valid;
> #ifdef CONFIG_COMMON_CLK
> struct clk_hw clkout_hw;
> #endif
> @@ -95,15 +94,16 @@ static int hym8563_rtc_read_time(struct device *dev, struct rtc_time *tm)
> u8 buf[7];
> int ret;
>
> - if (!hym8563->valid) {
> - dev_warn(&client->dev, "no valid clock/calendar values available\n");
> - return -EPERM;
> - }
> -
> ret = i2c_smbus_read_i2c_block_data(client, HYM8563_SEC, 7, buf);
> if (ret < 0)
> return ret;
>
> + if (ret & HYM8563_SEC_VL) {
Ouch, I should be checking buf[0] here. Sorry about that, will send v2.
Cheers,
Paul
> + dev_warn(&client->dev,
> + "no valid clock/calendar values available\n");
> + return -EPERM;
> + }
> +
> tm->tm_sec = bcd2bin(buf[0] & HYM8563_SEC_MASK);
> tm->tm_min = bcd2bin(buf[1] & HYM8563_MIN_MASK);
> tm->tm_hour = bcd2bin(buf[2] & HYM8563_HOUR_MASK);
> @@ -157,8 +157,6 @@ static int hym8563_rtc_set_time(struct device *dev, struct rtc_time *tm)
> if (ret < 0)
> return ret;
>
> - hym8563->valid = true;
> -
> return 0;
> }
>
> @@ -556,9 +554,8 @@ static int hym8563_probe(struct i2c_client *client,
> if (ret < 0)
> return ret;
>
> - hym8563->valid = !(ret & HYM8563_SEC_VL);
> dev_dbg(&client->dev, "rtc information is %s\n",
> - hym8563->valid ? "valid" : "invalid");
> + (ret & HYM8563_SEC_VL) ? "invalid" : "valid");
>
> hym8563->rtc = devm_rtc_device_register(&client->dev, client->name,
> &hym8563_rtc_ops, THIS_MODULE);
> --
> 2.24.0
>
--
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
Attachment:
signature.asc
Description: PGP signature
