[PATCH 2/2] rtc-tps65910: enable RTC power domain on initialization

[PATCH 2/2] rtc-tps65910: enable RTC power domain on initialization

[Kim, Milo]

 Enabling RTC HW block depends on the default value of TPS65910 register.
 In some mode, RTC block is disabled by default.(eg. AM3517 Craneboard)
 In this case, RTC_PWDN(RTC power down) bit should be cleared to enable
 the RTC HW block.

 This patch also works in case that RTC block is active by default,
 because there is no side effect even if the bit is updated again.

 Tested on AM3517 Craneboard.

Signed-off-by: Milo(Woogyom) Kim <milo.kim@ti.com>
---
 drivers/rtc/rtc-tps65910.c |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/rtc/rtc-tps65910.c b/drivers/rtc/rtc-tps65910.c
index e8d44bc..b4d9f02 100644
--- a/drivers/rtc/rtc-tps65910.c
+++ b/drivers/rtc/rtc-tps65910.c
@@ -247,6 +247,13 @@ static int __devinit tps65910_rtc_probe(struct platform_device *pdev)
 		return ret;
 
 	dev_dbg(&pdev->dev, "Enabling rtc-tps65910.\n");
+
+	/* Enable RTC digital power domain */
+	ret = regmap_update_bits(tps65910->regmap, TPS65910_DEVCTRL,
+		DEVCTRL_RTC_PWDN_MASK, 0 << DEVCTRL_RTC_PWDN_SHIFT);
+	if (ret < 0)
+		return ret;
+
 	rtc_reg = TPS65910_RTC_CTRL_STOP_RTC;
 	ret = regmap_write(tps65910->regmap, TPS65910_RTC_CTRL, rtc_reg);
 	if (ret < 0)
-- 
1.7.9.5


Best Regards,
Milo

RE: [PATCH 2/2] rtc-tps65910: enable RTC power domain on initialization

[Venu Byravarasu]

> -----Original Message-----
> From: Kim, Milo [mailto:Milo.Kim@ti.com]
> Sent: Thursday, November 29, 2012 2:18 PM
> To: Andrew Morton
> Cc: Samuel Ortiz; a.zummo@towertech.it; swarren@wwwdotorg.org; Venu
> Byravarasu; Sivaram Nair; linux-kernel@vger.kernel.org
> Subject: [PATCH 2/2] rtc-tps65910: enable RTC power domain on initialization
> 
>  Enabling RTC HW block depends on the default value of TPS65910 register.
>  In some mode, RTC block is disabled by default.(eg. AM3517 Craneboard)
>  In this case, RTC_PWDN(RTC power down) bit should be cleared to enable
>  the RTC HW block.

>From the description of RTC_PWDN bit of DEVCTRL_REG in TPS65910 data sheet
it is very evident that the default value of RTC_PWDN is 0.

Probably on "AM3517 Craneboard", some code is running prior to the RTC driver
which might be writing 1 on to this bit. IMO you must disable that write operation
instead of just writing default value into a register.

> 
>  This patch also works in case that RTC block is active by default,
>  because there is no side effect even if the bit is updated again.
> 
>  Tested on AM3517 Craneboard.
> 
> Signed-off-by: Milo(Woogyom) Kim <milo.kim@ti.com>
> ---
>  drivers/rtc/rtc-tps65910.c |    7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/rtc/rtc-tps65910.c b/drivers/rtc/rtc-tps65910.c
> index e8d44bc..b4d9f02 100644
> --- a/drivers/rtc/rtc-tps65910.c
> +++ b/drivers/rtc/rtc-tps65910.c
> @@ -247,6 +247,13 @@ static int __devinit tps65910_rtc_probe(struct
> platform_device *pdev)
>  		return ret;
> 
>  	dev_dbg(&pdev->dev, "Enabling rtc-tps65910.\n");
> +
> +	/* Enable RTC digital power domain */
> +	ret = regmap_update_bits(tps65910->regmap, TPS65910_DEVCTRL,
> +		DEVCTRL_RTC_PWDN_MASK, 0 <<
> DEVCTRL_RTC_PWDN_SHIFT);
> +	if (ret < 0)
> +		return ret;
> +
>  	rtc_reg = TPS65910_RTC_CTRL_STOP_RTC;
>  	ret = regmap_write(tps65910->regmap, TPS65910_RTC_CTRL,
> rtc_reg);
>  	if (ret < 0)
> --
> 1.7.9.5
> 
> 
> Best Regards,
> Milo
> 

RE: [PATCH 2/2] rtc-tps65910: enable RTC power domain on initialization

[Kim, Milo]

Hi Venu

> >  Enabling RTC HW block depends on the default value of TPS65910
> register.
> >  In some mode, RTC block is disabled by default.(eg. AM3517
> Craneboard)
> >  In this case, RTC_PWDN(RTC power down) bit should be cleared to
> enable
> >  the RTC HW block.
> 
> From the description of RTC_PWDN bit of DEVCTRL_REG in TPS65910 data
> sheet
> it is very evident that the default value of RTC_PWDN is 0.

According to the datasheet(http://www.ti.com/lit/ds/swcs046q/swcs046q.pdf),
the default value RTC_PWDN is 1 which means power down.

The default values are loaded from the EEPROM with BOOT_MODE 0,1 pin connection.
The RTC is disabled by default when BOOT_MODE = 00.
The Craneboard has the BOOT_MODE 00.

You may have other EEPROM settings,
however the official datasheet shows the RTC block is off by default.

Could you check the silicon version number? (0x80 register - JTAGVERNUM_REG)
In my case, the read value is 0x01.

> Probably on "AM3517 Craneboard", some code is running prior to the RTC
> driver
> which might be writing 1 on to this bit. IMO you must disable that
> write operation
> instead of just writing default value into a register.

Thank you for your comment.
I've read the DEVCTRL register (0x3F) in mfd tps65910 driver
as soon as the regmap registration is done, however RTC_PWDN is always 1.
That means the RTC is disabled by default.

Best Regards,
Milo

Re: [PATCH 2/2] rtc-tps65910: enable RTC power domain on initialization

[Andrew Morton]

On Thu, 29 Nov 2012 23:11:37 +0000
"Kim, Milo" <Milo.Kim@ti.com> wrote:

> Hi Venu
> 
> > >  Enabling RTC HW block depends on the default value of TPS65910
> > register.
> > >  In some mode, RTC block is disabled by default.(eg. AM3517
> > Craneboard)
> > >  In this case, RTC_PWDN(RTC power down) bit should be cleared to
> > enable
> > >  the RTC HW block.
> > 
> > From the description of RTC_PWDN bit of DEVCTRL_REG in TPS65910 data
> > sheet
> > it is very evident that the default value of RTC_PWDN is 0.
> 
> According to the datasheet(http://www.ti.com/lit/ds/swcs046q/swcs046q.pdf),
> the default value RTC_PWDN is 1 which means power down.
> 
> The default values are loaded from the EEPROM with BOOT_MODE 0,1 pin connection.
> The RTC is disabled by default when BOOT_MODE = 00.
> The Craneboard has the BOOT_MODE 00.
> 
> You may have other EEPROM settings,
> however the official datasheet shows the RTC block is off by default.
> 
> Could you check the silicon version number? (0x80 register - JTAGVERNUM_REG)
> In my case, the read value is 0x01.
> 
> > Probably on "AM3517 Craneboard", some code is running prior to the RTC
> > driver
> > which might be writing 1 on to this bit. IMO you must disable that
> > write operation
> > instead of just writing default value into a register.
> 
> Thank you for your comment.
> I've read the DEVCTRL register (0x3F) in mfd tps65910 driver
> as soon as the regmap registration is done, however RTC_PWDN is always 1.
> That means the RTC is disabled by default.
> 

I've merged this patch into -mm for 3.8 along with a note-to-self that
there might still be open issues.  Venu, please let us know if the
above settles things?

RE: [PATCH 2/2] rtc-tps65910: enable RTC power domain on initialization

[Venu Byravarasu]

> -----Original Message-----
> From: Andrew Morton [mailto:akpm@linux-foundation.org]
> Sent: Friday, November 30, 2012 5:44 AM
> To: Kim, Milo
> Cc: Venu Byravarasu; Samuel Ortiz; a.zummo@towertech.it;
> swarren@wwwdotorg.org; Sivaram Nair; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 2/2] rtc-tps65910: enable RTC power domain on
> initialization
> 
> On Thu, 29 Nov 2012 23:11:37 +0000
> "Kim, Milo" <Milo.Kim@ti.com> wrote:
> 
> > Hi Venu
> >

> >
> 
> I've merged this patch into -mm for 3.8 along with a note-to-self that
> there might still be open issues.  Venu, please let us know if the
> above settles things?

Hi Andrew,

Above change is fine & I added my ACK to the change.
Due to recent change in the data sheet, which I'm not aware of, made
my previous comment.

Sorry for the confusion created.

Thanks,
Venu

RE: [PATCH 2/2] rtc-tps65910: enable RTC power domain on initialization

[Venu Byravarasu]

> -----Original Message-----
> From: Kim, Milo [mailto:Milo.Kim@ti.com]
> Sent: Friday, November 30, 2012 4:42 AM
> To: Venu Byravarasu
> Cc: Andrew Morton; Samuel Ortiz; a.zummo@towertech.it;
> swarren@wwwdotorg.org; Sivaram Nair; linux-kernel@vger.kernel.org
> Subject: RE: [PATCH 2/2] rtc-tps65910: enable RTC power domain on
> initialization
> 
> Hi Venu
> 
> > >  Enabling RTC HW block depends on the default value of TPS65910
> > register.
> > >  In some mode, RTC block is disabled by default.(eg. AM3517
> > Craneboard)
> > >  In this case, RTC_PWDN(RTC power down) bit should be cleared to
> > enable
> > >  the RTC HW block.
> >
> > From the description of RTC_PWDN bit of DEVCTRL_REG in TPS65910 data
> > sheet
> > it is very evident that the default value of RTC_PWDN is 0.
> 
> According to the
> datasheet(http://www.ti.com/lit/ds/swcs046q/swcs046q.pdf),
> the default value RTC_PWDN is 1 which means power down.

As per the data sheet you pointed, the change you made is correct.
Seems the data sheet got updated recently, as it was shown in the
data sheet top section "REVISED SEPTEMBER 2012".

My data sheet has this field as "REVISED FEBRUARY 2011".

So, here I add my ack to this change.
Acked-by: Venu Byravarasu <vbyravarasu@nvidia.com> 

> 
> The default values are loaded from the EEPROM with BOOT_MODE 0,1 pin
> connection.
> The RTC is disabled by default when BOOT_MODE = 00.
> The Craneboard has the BOOT_MODE 00.
> 
> You may have other EEPROM settings,
> however the official datasheet shows the RTC block is off by default.
> 
> Could you check the silicon version number? (0x80 register -
> JTAGVERNUM_REG)
> In my case, the read value is 0x01.
> 
> > Probably on "AM3517 Craneboard", some code is running prior to the RTC
> > driver
> > which might be writing 1 on to this bit. IMO you must disable that
> > write operation
> > instead of just writing default value into a register.
> 
> Thank you for your comment.
> I've read the DEVCTRL register (0x3F) in mfd tps65910 driver
> as soon as the regmap registration is done, however RTC_PWDN is always 1.
> That means the RTC is disabled by default.
> 
> Best Regards,
> Milo