* [PATCH] rtc: rs5c372: r2221: fix to use the correct XSTP bit
@ 2019-01-09 10:59 Oliver.Rohe
2019-01-11 10:05 ` Alexandre Belloni
2019-02-05 21:57 ` Alexandre Belloni
0 siblings, 2 replies; 4+ messages in thread
From: Oliver.Rohe @ 2019-01-09 10:59 UTC (permalink / raw)
To: a.zummo, alexandre.belloni; +Cc: linux-rtc, linux-kernel, Oliver.Rohe
The Ricoh chips have slightly different register layouts
and the r2221 chip uses bit 5 as the oscillator halt sensor bit.
Signed-off-by: Olive Rohe <oliver.rohe@wago.com>
---
drivers/rtc/rtc-rs5c372.c | 32 +++++++++++++++++++++-----------
1 file changed, 21 insertions(+), 11 deletions(-)
diff --git a/drivers/rtc/rtc-rs5c372.c b/drivers/rtc/rtc-rs5c372.c
index c503832..ff35dce 100644
--- a/drivers/rtc/rtc-rs5c372.c
+++ b/drivers/rtc/rtc-rs5c372.c
@@ -52,8 +52,8 @@
# define RS5C_CTRL1_CT4 (4 << 0) /* 1 Hz level irq */
#define RS5C_REG_CTRL2 15
# define RS5C372_CTRL2_24 (1 << 5)
-# define R2025_CTRL2_XST (1 << 5)
-# define RS5C_CTRL2_XSTP (1 << 4) /* only if !R2025S/D */
+# define R2x2x_CTRL2_XSTP (1 << 5) /* only if R2x2x */
+# define RS5C_CTRL2_XSTP (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 */
@@ -519,20 +519,30 @@ static int rs5c_oscillator_setup(struct rs5c372 *rs5c372)
unsigned char buf[2];
int addr, i, ret = 0;
- if (rs5c372->type == rtc_r2025sd) {
- if (rs5c372->regs[RS5C_REG_CTRL2] & R2025_CTRL2_XST)
+ addr = RS5C_ADDR(RS5C_REG_CTRL1);
+ buf[0] = rs5c372->regs[RS5C_REG_CTRL1];
+ buf[1] = rs5c372->regs[RS5C_REG_CTRL2];
+
+ /* handle xstp bit */
+ switch (rs5c372->type) {
+ case rtc_r2025sd:
+ if (buf[1] & R2x2x_CTRL2_XSTP)
return ret;
- rs5c372->regs[RS5C_REG_CTRL2] |= R2025_CTRL2_XST;
- } else {
- if (!(rs5c372->regs[RS5C_REG_CTRL2] & RS5C_CTRL2_XSTP))
+ rs5c372->regs[RS5C_REG_CTRL2] |= R2x2x_CTRL2_XSTP;
+ break;
+ case rtc_r2221tl:
+ if (!(buf[1] & R2x2x_CTRL2_XSTP))
+ return ret;
+ rs5c372->regs[RS5C_REG_CTRL2] &= ~R2x2x_CTRL2_XSTP;
+ break;
+
+ default:
+ if (!(buf[1] & RS5C_CTRL2_XSTP))
return ret;
rs5c372->regs[RS5C_REG_CTRL2] &= ~RS5C_CTRL2_XSTP;
+ break;
}
- addr = RS5C_ADDR(RS5C_REG_CTRL1);
- buf[0] = rs5c372->regs[RS5C_REG_CTRL1];
- buf[1] = rs5c372->regs[RS5C_REG_CTRL2];
-
/* use 24hr mode */
switch (rs5c372->type) {
case rtc_rs5c372a:
--
2.7.4
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] rtc: rs5c372: r2221: fix to use the correct XSTP bit
2019-01-09 10:59 [PATCH] rtc: rs5c372: r2221: fix to use the correct XSTP bit Oliver.Rohe
@ 2019-01-11 10:05 ` Alexandre Belloni
2019-01-11 10:39 ` Oliver.Rohe
2019-02-05 21:57 ` Alexandre Belloni
1 sibling, 1 reply; 4+ messages in thread
From: Alexandre Belloni @ 2019-01-11 10:05 UTC (permalink / raw)
To: Oliver.Rohe; +Cc: a.zummo, linux-rtc, linux-kernel
Hello,
On 09/01/2019 10:59:40+0000, Oliver.Rohe@wago.com wrote:
> The Ricoh chips have slightly different register layouts
> and the r2221 chip uses bit 5 as the oscillator halt sensor bit.
>
> Signed-off-by: Olive Rohe <oliver.rohe@wago.com>
> ---
> drivers/rtc/rtc-rs5c372.c | 32 +++++++++++++++++++++-----------
> 1 file changed, 21 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/rtc/rtc-rs5c372.c b/drivers/rtc/rtc-rs5c372.c
> index c503832..ff35dce 100644
> --- a/drivers/rtc/rtc-rs5c372.c
> +++ b/drivers/rtc/rtc-rs5c372.c
> @@ -52,8 +52,8 @@
> # define RS5C_CTRL1_CT4 (4 << 0) /* 1 Hz level irq */
> #define RS5C_REG_CTRL2 15
> # define RS5C372_CTRL2_24 (1 << 5)
> -# define R2025_CTRL2_XST (1 << 5)
> -# define RS5C_CTRL2_XSTP (1 << 4) /* only if !R2025S/D */
> +# define R2x2x_CTRL2_XSTP (1 << 5) /* only if R2x2x */
I wouldn't rename that define as later on, it may be used for RTCs that
doesn't match R2x2x (as is the case for RV5C387_CTRL1_24 for example).
the bit doesn't even have the same meaning on R2025 and R2221.
> +# define RS5C_CTRL2_XSTP (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 */
> @@ -519,20 +519,30 @@ static int rs5c_oscillator_setup(struct rs5c372 *rs5c372)
> unsigned char buf[2];
> int addr, i, ret = 0;
>
> - if (rs5c372->type == rtc_r2025sd) {
> - if (rs5c372->regs[RS5C_REG_CTRL2] & R2025_CTRL2_XST)
> + addr = RS5C_ADDR(RS5C_REG_CTRL1);
> + buf[0] = rs5c372->regs[RS5C_REG_CTRL1];
> + buf[1] = rs5c372->regs[RS5C_REG_CTRL2];
> +
> + /* handle xstp bit */
> + switch (rs5c372->type) {
> + case rtc_r2025sd:
> + if (buf[1] & R2x2x_CTRL2_XSTP)
> return ret;
> - rs5c372->regs[RS5C_REG_CTRL2] |= R2025_CTRL2_XST;
> - } else {
> - if (!(rs5c372->regs[RS5C_REG_CTRL2] & RS5C_CTRL2_XSTP))
> + rs5c372->regs[RS5C_REG_CTRL2] |= R2x2x_CTRL2_XSTP;
> + break;
> + case rtc_r2221tl:
> + if (!(buf[1] & R2x2x_CTRL2_XSTP))
> + return ret;
> + rs5c372->regs[RS5C_REG_CTRL2] &= ~R2x2x_CTRL2_XSTP;
> + break;
> +
> + default:
> + if (!(buf[1] & RS5C_CTRL2_XSTP))
> return ret;
> rs5c372->regs[RS5C_REG_CTRL2] &= ~RS5C_CTRL2_XSTP;
> + break;
> }
>
Note that this is actually quite bad to restart the oscillator on probe.
The best would be to do that only in rs5c372_rtc_set_time once you know
the set time is good. then you can return -EINVAL from
rs5c372_rtc_read_time when you know the oscillator is stopped so
userspace know the RTC time is bad. This could be done as a follw up
patch.
There is also more work needed to clean up that driver if you have time
and are willing to.
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] rtc: rs5c372: r2221: fix to use the correct XSTP bit
2019-01-11 10:05 ` Alexandre Belloni
@ 2019-01-11 10:39 ` Oliver.Rohe
0 siblings, 0 replies; 4+ messages in thread
From: Oliver.Rohe @ 2019-01-11 10:39 UTC (permalink / raw)
To: alexandre.belloni; +Cc: a.zummo, linux-rtc, linux-kernel
Hi Alexandre,
On 11.01.19 11:05, Alexandre Belloni wrote:
> Hello,
>
> On 09/01/2019 10:59:40+0000, Oliver.Rohe@wago.com wrote:
>> The Ricoh chips have slightly different register layouts
>> and the r2221 chip uses bit 5 as the oscillator halt sensor bit.
>>
>> Signed-off-by: Olive Rohe <oliver.rohe@wago.com>
>> ---
>> drivers/rtc/rtc-rs5c372.c | 32 +++++++++++++++++++++-----------
>> 1 file changed, 21 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/rtc/rtc-rs5c372.c b/drivers/rtc/rtc-rs5c372.c
>> index c503832..ff35dce 100644
>> --- a/drivers/rtc/rtc-rs5c372.c
>> +++ b/drivers/rtc/rtc-rs5c372.c
>> @@ -52,8 +52,8 @@
>> # define RS5C_CTRL1_CT4 (4 << 0) /* 1 Hz level irq */
>> #define RS5C_REG_CTRL2 15
>> # define RS5C372_CTRL2_24 (1 << 5)
>> -# define R2025_CTRL2_XST (1 << 5)
>> -# define RS5C_CTRL2_XSTP (1 << 4) /* only if !R2025S/D */
>> +# define R2x2x_CTRL2_XSTP (1 << 5) /* only if R2x2x */
>
> I wouldn't rename that define as later on, it may be used for RTCs that
> doesn't match R2x2x (as is the case for RV5C387_CTRL1_24 for example).
> the bit doesn't even have the same meaning on R2025 and R2221.
Yes, but R2025 and R2221 have a few registers in common (PON, VDET). The meaning
of XST and XSTP I think are the same, even though they have flipped logic.
>
>> +# define RS5C_CTRL2_XSTP (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 */
>> @@ -519,20 +519,30 @@ static int rs5c_oscillator_setup(struct rs5c372 *rs5c372)
>> unsigned char buf[2];
>> int addr, i, ret = 0;
>>
>> - if (rs5c372->type == rtc_r2025sd) {
>> - if (rs5c372->regs[RS5C_REG_CTRL2] & R2025_CTRL2_XST)
>> + addr = RS5C_ADDR(RS5C_REG_CTRL1);
>> + buf[0] = rs5c372->regs[RS5C_REG_CTRL1];
>> + buf[1] = rs5c372->regs[RS5C_REG_CTRL2];
>> +
>> + /* handle xstp bit */
>> + switch (rs5c372->type) {
>> + case rtc_r2025sd:
>> + if (buf[1] & R2x2x_CTRL2_XSTP)
>> return ret;
>> - rs5c372->regs[RS5C_REG_CTRL2] |= R2025_CTRL2_XST;
>> - } else {
>> - if (!(rs5c372->regs[RS5C_REG_CTRL2] & RS5C_CTRL2_XSTP))
>> + rs5c372->regs[RS5C_REG_CTRL2] |= R2x2x_CTRL2_XSTP;
>> + break;
>> + case rtc_r2221tl:
>> + if (!(buf[1] & R2x2x_CTRL2_XSTP))
>> + return ret;
>> + rs5c372->regs[RS5C_REG_CTRL2] &= ~R2x2x_CTRL2_XSTP;
>> + break;
>> +
>> + default:
>> + if (!(buf[1] & RS5C_CTRL2_XSTP))
>> return ret;
>> rs5c372->regs[RS5C_REG_CTRL2] &= ~RS5C_CTRL2_XSTP;
>> + break;
>> }
>>
>
> Note that this is actually quite bad to restart the oscillator on probe.
> The best would be to do that only in rs5c372_rtc_set_time once you know
> the set time is good. then you can return -EINVAL from
> rs5c372_rtc_read_time when you know the oscillator is stopped so
> userspace know the RTC time is bad. This could be done as a follw up
> patch.
Yes, I agree. I have another patch that does exactly what you suggest. We reset
the different bits (XSTP, PON, VDET) in set time and return an error in read
time if the xstp bit is set. I will send it to you.
> There is also more work needed to clean up that driver if you have time
> and are willing to.
Yes sure. With some help, or examples from you guys I can do that.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] rtc: rs5c372: r2221: fix to use the correct XSTP bit
2019-01-09 10:59 [PATCH] rtc: rs5c372: r2221: fix to use the correct XSTP bit Oliver.Rohe
2019-01-11 10:05 ` Alexandre Belloni
@ 2019-02-05 21:57 ` Alexandre Belloni
1 sibling, 0 replies; 4+ messages in thread
From: Alexandre Belloni @ 2019-02-05 21:57 UTC (permalink / raw)
To: Oliver.Rohe; +Cc: a.zummo, linux-rtc, linux-kernel
On 09/01/2019 10:59:40+0000, Oliver.Rohe@wago.com wrote:
> The Ricoh chips have slightly different register layouts
> and the r2221 chip uses bit 5 as the oscillator halt sensor bit.
>
> Signed-off-by: Olive Rohe <oliver.rohe@wago.com>
> ---
> drivers/rtc/rtc-rs5c372.c | 32 +++++++++++++++++++++-----------
> 1 file changed, 21 insertions(+), 11 deletions(-)
>
Applied, thanks.
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-02-05 21:57 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-09 10:59 [PATCH] rtc: rs5c372: r2221: fix to use the correct XSTP bit Oliver.Rohe
2019-01-11 10:05 ` Alexandre Belloni
2019-01-11 10:39 ` Oliver.Rohe
2019-02-05 21:57 ` Alexandre Belloni
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).