RTC control register should be enabled in the process of initliazing.
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
drivers/rtc/rtc-pl031.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
@@ -345,10 +346,11 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)dev_dbg(&adev->dev,"designer ID = 0x%02x\n",amba_manf(adev));dev_dbg(&adev->dev,"revision = 0x%01x\n",amba_rev(adev));+data=readl(ldata->base+RTC_CR);/* Enable the clockwatch on ST Variants */if(vendor->clockwatch)-writel(readl(ldata->base+RTC_CR)|RTC_CR_CWEN,-ldata->base+RTC_CR);+data|=RTC_CR_CWEN;+writel(data|RTC_CR_EN,ldata->base+RTC_CR);/**OnSTPL031variants,theRTCresetvaluedoesnotprovidecorrect
@@ -345,10 +346,11 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)dev_dbg(&adev->dev,"designer ID = 0x%02x\n",amba_manf(adev));dev_dbg(&adev->dev,"revision = 0x%01x\n",amba_rev(adev));+data=readl(ldata->base+RTC_CR);/* Enable the clockwatch on ST Variants */if(vendor->clockwatch)-writel(readl(ldata->base+RTC_CR)|RTC_CR_CWEN,-ldata->base+RTC_CR);+data|=RTC_CR_CWEN;+writel(data|RTC_CR_EN,ldata->base+RTC_CR);
Does this patch fix some user-visible misbehaviour? If so, please
fully describe that misbehaviour.
@@ -345,10 +346,11 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)dev_dbg(&adev->dev,"designer ID = 0x%02x\n",amba_manf(adev));dev_dbg(&adev->dev,"revision = 0x%01x\n",amba_rev(adev));+data=readl(ldata->base+RTC_CR);/* Enable the clockwatch on ST Variants */if(vendor->clockwatch)-writel(readl(ldata->base+RTC_CR)|RTC_CR_CWEN,-ldata->base+RTC_CR);+data|=RTC_CR_CWEN;+writel(data|RTC_CR_EN,ldata->base+RTC_CR);
Does this patch fix some user-visible misbehaviour? If so, please
fully describe that misbehaviour.
Hi Andrew,
I copy the description from rtc pl031 user manual (page 33 of
DDI0224.pdf) in below.
RTCCR is a 1-bit control register. When HIGH, the counter enable
signal is asserted to
enable the counter. Table 3-5 shows the bit assignments for the RTCCR register.
Table 3-5 RTCCR register
-----------------------------------------------------------------------------------------------------------------------
Bits Name Type Function
31:1 - Read/write Reserved. Read
unpredictable. Should
be written as 0.
0 RTC start Read/write If set to 1, the
RTC is enabled. Once it is
enabled, any writes to this bit have no
effect
on the RTC until a system reset.
A read
returns the status of the RTC.
-----------------------------------------------------------------------------------------------------------------------
From this document, RTCCR must be enabled before usage. Without this
patch, I really
failed to enable RTC in Hisilicon Hi3620 SoC. It results that the
register mapping section
in RTC is always read as zero. So I doubt that ST guys may already
enable this register
in bootloader. So they won't meet this issue.
Best Regards
Haojian
On Fri, 1 Feb 2013 09:32:59 +0800
Haojian Zhuang [off-list ref] wrote:
Without this
patch, I really
failed to enable RTC in Hisilicon Hi3620 SoC. It results that the
register mapping section
in RTC is always read as zero. So I doubt that ST guys may already
enable this register
in bootloader. So they won't meet this issue.
OK, thanks, that sounds pretty serious so I tagged the patch for
backporting into -stable kernels as well.
Hi Haojian, sorry for taking too long to reply...
On Wed, Jan 30, 2013 at 2:04 AM, Haojian Zhuang
[off-list ref] wrote:
RTC control register should be enabled in the process of initliazing.
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
(...)
+ data = readl(ldata->base + RTC_CR);
/* Enable the clockwatch on ST Variants */
if (vendor->clockwatch)
- writel(readl(ldata->base + RTC_CR) | RTC_CR_CWEN,
- ldata->base + RTC_CR);
+ data |= RTC_CR_CWEN;
+ writel(data | RTC_CR_EN, ldata->base + RTC_CR);
This last line is *not* OK on the ST Variant. In our hardware that bit
is part of the clock divider, which means it will affect our timekeeping.
Do you want me to submit a follow-up patch?
Yours,
Linus Walleij
On 4 February 2013 20:25, Linus Walleij [off-list ref] wrote:
Hi Haojian, sorry for taking too long to reply...
On Wed, Jan 30, 2013 at 2:04 AM, Haojian Zhuang
[off-list ref] wrote:
quoted
RTC control register should be enabled in the process of initliazing.
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
(...)
quoted
+ data = readl(ldata->base + RTC_CR);
/* Enable the clockwatch on ST Variants */
if (vendor->clockwatch)
- writel(readl(ldata->base + RTC_CR) | RTC_CR_CWEN,
- ldata->base + RTC_CR);
+ data |= RTC_CR_CWEN;
+ writel(data | RTC_CR_EN, ldata->base + RTC_CR);
This last line is *not* OK on the ST Variant. In our hardware that bit
is part of the clock divider, which means it will affect our timekeeping.
Do you want me to submit a follow-up patch?
Yours,
Linus Walleij
I prefer you can submit a follow up patch. And I think that you can
use a compatible
name to distinguish from original "arm,rtc-pl031". Then we can get two
branches to
handle the difference in the probe function. What's your opinion?
Regards
Haojian
On Mon, Feb 4, 2013 at 1:34 PM, Haojian Zhuang
[off-list ref] wrote:
I prefer you can submit a follow up patch.
OK!
And I think that you can
use a compatible
name to distinguish from original "arm,rtc-pl031". Then we can get two
branches to
handle the difference in the probe function. What's your opinion?
No that is wrong for PrimeCells.
PrimeCells have special ID numbers that we use to identify the
different variants already.
Yours,
Linus Walleij