* [PATCH 1/2] rtc: hym8563: Read the valid flag directly instead of caching it
@ 2019-12-12 10:36 Paul Kocialkowski
2019-12-12 10:36 ` [PATCH 2/2] rtc: hym8563: Return -EINVAL if the time is known to be invalid Paul Kocialkowski
2019-12-12 10:43 ` [PATCH 1/2] rtc: hym8563: Read the valid flag directly instead of caching it Paul Kocialkowski
0 siblings, 2 replies; 3+ messages in thread
From: Paul Kocialkowski @ 2019-12-12 10:36 UTC (permalink / raw)
To: linux-rtc, linux-kernel
Cc: Alessandro Zummo, Alexandre Belloni, Thomas Petazzoni, Paul Kocialkowski
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@bootlin.com>
---
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) {
+ 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
^ permalink raw reply related [flat|nested] 3+ messages in thread* [PATCH 2/2] rtc: hym8563: Return -EINVAL if the time is known to be invalid
2019-12-12 10:36 [PATCH 1/2] rtc: hym8563: Read the valid flag directly instead of caching it Paul Kocialkowski
@ 2019-12-12 10:36 ` Paul Kocialkowski
2019-12-12 10:43 ` [PATCH 1/2] rtc: hym8563: Read the valid flag directly instead of caching it Paul Kocialkowski
1 sibling, 0 replies; 3+ messages in thread
From: Paul Kocialkowski @ 2019-12-12 10:36 UTC (permalink / raw)
To: linux-rtc, linux-kernel
Cc: Alessandro Zummo, Alexandre Belloni, Thomas Petazzoni, Paul Kocialkowski
The current code returns -EPERM when the voltage loss bit is set.
Since the bit indicates that the time value is not valid, return
-EINVAL instead, which is the appropriate error code for this
situation.
Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
drivers/rtc/rtc-hym8563.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/rtc/rtc-hym8563.c b/drivers/rtc/rtc-hym8563.c
index 584a7815f246..e547a892b692 100644
--- a/drivers/rtc/rtc-hym8563.c
+++ b/drivers/rtc/rtc-hym8563.c
@@ -101,7 +101,7 @@ static int hym8563_rtc_read_time(struct device *dev, struct rtc_time *tm)
if (ret & HYM8563_SEC_VL) {
dev_warn(&client->dev,
"no valid clock/calendar values available\n");
- return -EPERM;
+ return -EINVAL;
}
tm->tm_sec = bcd2bin(buf[0] & HYM8563_SEC_MASK);
--
2.24.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH 1/2] rtc: hym8563: Read the valid flag directly instead of caching it
2019-12-12 10:36 [PATCH 1/2] rtc: hym8563: Read the valid flag directly instead of caching it Paul Kocialkowski
2019-12-12 10:36 ` [PATCH 2/2] rtc: hym8563: Return -EINVAL if the time is known to be invalid Paul Kocialkowski
@ 2019-12-12 10:43 ` Paul Kocialkowski
1 sibling, 0 replies; 3+ messages in thread
From: Paul Kocialkowski @ 2019-12-12 10:43 UTC (permalink / raw)
To: linux-rtc, linux-kernel
Cc: Alessandro Zummo, Alexandre Belloni, Thomas Petazzoni
[-- Attachment #1: Type: text/plain, Size: 2683 bytes --]
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@bootlin.com>
> ---
> 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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-12-12 10:43 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-12 10:36 [PATCH 1/2] rtc: hym8563: Read the valid flag directly instead of caching it Paul Kocialkowski
2019-12-12 10:36 ` [PATCH 2/2] rtc: hym8563: Return -EINVAL if the time is known to be invalid Paul Kocialkowski
2019-12-12 10:43 ` [PATCH 1/2] rtc: hym8563: Read the valid flag directly instead of caching it Paul Kocialkowski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).