This chip has a control register and can prevent altering saved clock.
Without this patch we could have:
----8<----
(arm)root@pac14:~# date
Tue May 21 03:08:27 MSK 2013
(arm)root@pac14:~# /etc/init.d/hwclock.sh show
Tue May 21 11:13:58 2013 -0.067322 seconds
(arm)root@pac14:~# /etc/init.d/hwclock.sh stop
[info] Saving the system clock.
[info] Hardware Clock updated to Tue May 21 03:09:01 MSK 2013.
(arm)root@pac14:~# /etc/init.d/hwclock.sh show
Tue May 21 11:14:15 2013 -0.624272 seconds
----8<----
Signed-off-by: Sergey Yanovich <[email protected]>
---
drivers/rtc/rtc-ds1302.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/rtc/rtc-ds1302.c b/drivers/rtc/rtc-ds1302.c
index 73eafb9..626aec2 100644
--- a/drivers/rtc/rtc-ds1302.c
+++ b/drivers/rtc/rtc-ds1302.c
@@ -23,8 +23,12 @@
#define RTC_CMD_READ 0x81 /* Read command */
#define RTC_CMD_WRITE 0x80 /* Write command */
+#define RTC_CMD_WRITE_ENABLE 0x00 /* Write enable */
+#define RTC_CMD_WRITE_DISABLE 0x80 /* Write disable */
+
#define RTC_ADDR_RAM0 0x20 /* Address of RAM0 */
#define RTC_ADDR_TCR 0x08 /* Address of trickle charge register */
+#define RTC_ADDR_CTRL 0x07 /* Address of control register */
#define RTC_ADDR_YEAR 0x06 /* Address of year register */
#define RTC_ADDR_DAY 0x05 /* Address of day of week register */
#define RTC_ADDR_MON 0x04 /* Address of month register */
@@ -313,6 +317,7 @@ static int __init ds1302_rtc_probe(struct platform_device *pdev)
return PTR_ERR(rtc);
platform_set_drvdata(pdev, rtc);
+ ds1302_writebyte(RTC_ADDR_CTRL, RTC_CMD_WRITE_ENABLE);
return 0;
}
@@ -321,6 +326,7 @@ static int ds1302_rtc_remove(struct platform_device *pdev)
{
struct rtc_device *rtc = platform_get_drvdata(pdev);
+ ds1302_writebyte(RTC_ADDR_CTRL, RTC_CMD_WRITE_DISABLE);
rtc_device_unregister(rtc);
platform_set_drvdata(pdev, NULL);
--
1.7.10.4
On Tue, 21 May 2013 03:21:30 +0400 Sergey Yanovich <[email protected]> wrote:
> This chip has a control register and can prevent altering saved clock.
> Without this patch we could have:
> ----8<----
> (arm)root@pac14:~# date
> Tue May 21 03:08:27 MSK 2013
> (arm)root@pac14:~# /etc/init.d/hwclock.sh show
> Tue May 21 11:13:58 2013 -0.067322 seconds
> (arm)root@pac14:~# /etc/init.d/hwclock.sh stop
> [info] Saving the system clock.
> [info] Hardware Clock updated to Tue May 21 03:09:01 MSK 2013.
> (arm)root@pac14:~# /etc/init.d/hwclock.sh show
> Tue May 21 11:14:15 2013 -0.624272 seconds
> ----8<----
>
> ...
>
> --- a/drivers/rtc/rtc-ds1302.c
> +++ b/drivers/rtc/rtc-ds1302.c
> @@ -23,8 +23,12 @@
> #define RTC_CMD_READ 0x81 /* Read command */
> #define RTC_CMD_WRITE 0x80 /* Write command */
>
> +#define RTC_CMD_WRITE_ENABLE 0x00 /* Write enable */
> +#define RTC_CMD_WRITE_DISABLE 0x80 /* Write disable */
> +
> #define RTC_ADDR_RAM0 0x20 /* Address of RAM0 */
> #define RTC_ADDR_TCR 0x08 /* Address of trickle charge register */
> +#define RTC_ADDR_CTRL 0x07 /* Address of control register */
> #define RTC_ADDR_YEAR 0x06 /* Address of year register */
> #define RTC_ADDR_DAY 0x05 /* Address of day of week register */
> #define RTC_ADDR_MON 0x04 /* Address of month register */
> @@ -313,6 +317,7 @@ static int __init ds1302_rtc_probe(struct platform_device *pdev)
> return PTR_ERR(rtc);
>
> platform_set_drvdata(pdev, rtc);
> + ds1302_writebyte(RTC_ADDR_CTRL, RTC_CMD_WRITE_ENABLE);
>
> return 0;
> }
> @@ -321,6 +326,7 @@ static int ds1302_rtc_remove(struct platform_device *pdev)
> {
> struct rtc_device *rtc = platform_get_drvdata(pdev);
>
> + ds1302_writebyte(RTC_ADDR_CTRL, RTC_CMD_WRITE_DISABLE);
> rtc_device_unregister(rtc);
> platform_set_drvdata(pdev, NULL);
ds1302_rtc_remove() no longer exists in my tree - it got whittled away
to nothing by
http://ozlabs.org/~akpm/mmots/broken-out/rtc-rtc-ds1302-remove-unnecessary-platform_set_drvdata.patch
and
http://ozlabs.org/~akpm/mmots/broken-out/drivers-rtc-rtc-ds1302c-remove-empty-function.patch
Perhaps it should be re-added for this?
On Wed, 2013-05-29 at 15:53 -0700, Andrew Morton wrote:
> On Tue, 21 May 2013 03:21:30 +0400 Sergey Yanovich <[email protected]> wrote:
> @@ -321,6 +326,7 @@ static int ds1302_rtc_remove(struct platform_device *pdev)
> > {
> > struct rtc_device *rtc = platform_get_drvdata(pdev);
> >
> > + ds1302_writebyte(RTC_ADDR_CTRL, RTC_CMD_WRITE_DISABLE);
> > rtc_device_unregister(rtc);
> > platform_set_drvdata(pdev, NULL);
>
> ds1302_rtc_remove() no longer exists in my tree - it got whittled away
> to nothing by
> http://ozlabs.org/~akpm/mmots/broken-out/rtc-rtc-ds1302-remove-unnecessary-platform_set_drvdata.patch
> and
> http://ozlabs.org/~akpm/mmots/broken-out/drivers-rtc-rtc-ds1302c-remove-empty-function.patch
>
> Perhaps it should be re-added for this?
There are 2 options. I would be happy with either.
1. I've chosen 'probe/remove' to enable/disable write access.
2. Another option is to wrap enable/disable around
ds1302_rtc_set_time().
IIUC, the former saves a few bytes of memory. However, now, when
ds1302_rtc_remove() is gone, the latter looks better. So I could rewrite
the patch either way.
On Thu, 30 May 2013 14:14:42 +0400, Sergey Yanovich <[email protected]>
wrote:
> On Wed, 2013-05-29 at 15:53 -0700, Andrew Morton wrote:
>> On Tue, 21 May 2013 03:21:30 +0400 Sergey Yanovich <[email protected]>
>> wrote:
>> @@ -321,6 +326,7 @@ static int ds1302_rtc_remove(struct platform_device
>> *pdev)
>> > {
>> > struct rtc_device *rtc = platform_get_drvdata(pdev);
>> >
>> > + ds1302_writebyte(RTC_ADDR_CTRL, RTC_CMD_WRITE_DISABLE);
>> > rtc_device_unregister(rtc);
>> > platform_set_drvdata(pdev, NULL);
>>
>> ds1302_rtc_remove() no longer exists in my tree - it got whittled away
>> to nothing by
>>
http://ozlabs.org/~akpm/mmots/broken-out/rtc-rtc-ds1302-remove-unnecessary-platform_set_drvdata.patch
>> and
>>
http://ozlabs.org/~akpm/mmots/broken-out/drivers-rtc-rtc-ds1302c-remove-empty-function.patch
>>
>> Perhaps it should be re-added for this?
>
> There are 2 options. I would be happy with either.
>
> 1. I've chosen 'probe/remove' to enable/disable write access.
>
> 2. Another option is to wrap enable/disable around
> ds1302_rtc_set_time().
>
> IIUC, the former saves a few bytes of memory. However, now, when
> ds1302_rtc_remove() is gone, the latter looks better. So I could rewrite
> the patch either way.
Option two looks actually safer to me, as it ensures that an unexpected
reboot outside of the set_time section doesn't leave write access enabled.
You never know what firmware could do while you're not looking...
M.
--
Who you jivin' with that Cosmik Debris?
>From f1cd048a066b249082752a96abce7d33a0cd4ea3 Mon Sep 17 00:00:00 2001
From: Sergey Yanovich <[email protected]>
Date: Tue, 21 May 2013 03:06:31 +0400
Subject: [PATCH] rtc-ds1302: handle write protection
This chip has a control register and can prevent altering saved clock.
Without this patch we could have:
----8<----
(arm)root@pac14:~# date
Tue May 21 03:08:27 MSK 2013
(arm)root@pac14:~# /etc/init.d/hwclock.sh show
Tue May 21 11:13:58 2013 -0.067322 seconds
(arm)root@pac14:~# /etc/init.d/hwclock.sh stop
[info] Saving the system clock.
[info] Hardware Clock updated to Tue May 21 03:09:01 MSK 2013.
(arm)root@pac14:~# /etc/init.d/hwclock.sh show
Tue May 21 11:14:15 2013 -0.624272 seconds
----8<----
The patch enables write access to rtc before the driver tries to write time
and re-disables when time data is written.
Signed-off-by: Sergey Yanovich <[email protected]>
# Changes to be committed:
---
changes for v2:
- do enable/disable around set_time() instead of in probe()/remove()
drivers/rtc/rtc-ds1302.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/rtc/rtc-ds1302.c b/drivers/rtc/rtc-ds1302.c
index 7533b72..07e8d79 100644
--- a/drivers/rtc/rtc-ds1302.c
+++ b/drivers/rtc/rtc-ds1302.c
@@ -23,8 +23,12 @@
#define RTC_CMD_READ 0x81 /* Read command */
#define RTC_CMD_WRITE 0x80 /* Write command */
+#define RTC_CMD_WRITE_ENABLE 0x00 /* Write enable */
+#define RTC_CMD_WRITE_DISABLE 0x80 /* Write disable */
+
#define RTC_ADDR_RAM0 0x20 /* Address of RAM0 */
#define RTC_ADDR_TCR 0x08 /* Address of trickle charge register */
+#define RTC_ADDR_CTRL 0x07 /* Address of control register */
#define RTC_ADDR_YEAR 0x06 /* Address of year register */
#define RTC_ADDR_DAY 0x05 /* Address of day of week register */
#define RTC_ADDR_MON 0x04 /* Address of month register */
@@ -161,6 +165,7 @@ static int ds1302_rtc_read_time(struct device *dev, struct rtc_time *tm)
static int ds1302_rtc_set_time(struct device *dev, struct rtc_time *tm)
{
+ ds1302_writebyte(RTC_ADDR_CTRL, RTC_CMD_WRITE_ENABLE);
/* Stop RTC */
ds1302_writebyte(RTC_ADDR_SEC, ds1302_readbyte(RTC_ADDR_SEC) | 0x80);
@@ -175,6 +180,8 @@ static int ds1302_rtc_set_time(struct device *dev, struct rtc_time *tm)
/* Start RTC */
ds1302_writebyte(RTC_ADDR_SEC, ds1302_readbyte(RTC_ADDR_SEC) & ~0x80);
+ ds1302_writebyte(RTC_ADDR_CTRL, RTC_CMD_WRITE_DISABLE);
+
return 0;
}
--
1.7.10.4
On Thu, 30 May 2013 20:04:48 +0400, Sergey Yanovich <[email protected]>
wrote:
> From f1cd048a066b249082752a96abce7d33a0cd4ea3 Mon Sep 17 00:00:00 2001
> From: Sergey Yanovich <[email protected]>
> Date: Tue, 21 May 2013 03:06:31 +0400
> Subject: [PATCH] rtc-ds1302: handle write protection
>
> This chip has a control register and can prevent altering saved clock.
> Without this patch we could have:
> ----8<----
> (arm)root@pac14:~# date
> Tue May 21 03:08:27 MSK 2013
> (arm)root@pac14:~# /etc/init.d/hwclock.sh show
> Tue May 21 11:13:58 2013 -0.067322 seconds
> (arm)root@pac14:~# /etc/init.d/hwclock.sh stop
> [info] Saving the system clock.
> [info] Hardware Clock updated to Tue May 21 03:09:01 MSK 2013.
> (arm)root@pac14:~# /etc/init.d/hwclock.sh show
> Tue May 21 11:14:15 2013 -0.624272 seconds
> ----8<----
>
> The patch enables write access to rtc before the driver tries to write
time
> and re-disables when time data is written.
>
> Signed-off-by: Sergey Yanovich <[email protected]>
FWIW,
Acked-by: Marc Zyngier <[email protected]>
M.
--
Who you jivin' with that Cosmik Debris?