rtc: ds1307: Enable battery backup on RX8130
diff mbox series

Message ID 20190628002151.4925-1-marex@denx.de
State Superseded
Headers show
Series
  • rtc: ds1307: Enable battery backup on RX8130
Related show

Commit Message

Marek Vasut June 28, 2019, 12:21 a.m. UTC
The battery backup can be disabled on this RTC, e.g. if populated right
out of production. Force the battery backup bit on to enable it.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Arnaud Ebalard <arno@natisbad.org>
Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
 drivers/rtc/rtc-ds1307.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Marek Vasut Aug. 7, 2019, 11:12 a.m. UTC | #1
On 6/28/19 2:21 AM, Marek Vasut wrote:
> The battery backup can be disabled on this RTC, e.g. if populated right
> out of production. Force the battery backup bit on to enable it.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Arnaud Ebalard <arno@natisbad.org>
> Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> ---
>  drivers/rtc/rtc-ds1307.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
> index 1f7e8aefc1eb..4af00cac1eff 100644
> --- a/drivers/rtc/rtc-ds1307.c
> +++ b/drivers/rtc/rtc-ds1307.c
> @@ -121,6 +121,8 @@ enum ds_type {
>  #define RX8130_REG_FLAG_AF		BIT(3)
>  #define RX8130_REG_CONTROL0		0x1e
>  #define RX8130_REG_CONTROL0_AIE		BIT(3)
> +#define RX8130_REG_CONTROL1		0x0f
> +#define RX8130_REG_CONTROL1_INIEN	BIT(4)
>  
>  #define MCP794XX_REG_CONTROL		0x07
>  #	define MCP794XX_BIT_ALM0_EN	0x10
> @@ -1750,6 +1752,11 @@ static int ds1307_probe(struct i2c_client *client,
>  				     DS1307_REG_HOUR << 4 | 0x08, hour);
>  		}
>  		break;
> +	case rx_8130:
> +		/* make sure that the backup battery is enabled */
> +		regmap_write(ds1307->regmap, chip->offset + RX8130_REG_CONTROL1,
> +			     RX8130_REG_CONTROL1_INIEN);
> +		break;
>  	default:
>  		break;
>  	}
> 

Bump ?
Bastian Krause Aug. 30, 2019, 10:42 a.m. UTC | #2
Hi Marek,

On 6/28/19 2:21 AM, Marek Vasut wrote:
> The battery backup can be disabled on this RTC, e.g. if populated right
> out of production. Force the battery backup bit on to enable it.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Arnaud Ebalard <arno@natisbad.org>
> Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> ---
>  drivers/rtc/rtc-ds1307.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
> index 1f7e8aefc1eb..4af00cac1eff 100644
> --- a/drivers/rtc/rtc-ds1307.c
> +++ b/drivers/rtc/rtc-ds1307.c
> @@ -121,6 +121,8 @@ enum ds_type {
>  #define RX8130_REG_FLAG_AF		BIT(3)
>  #define RX8130_REG_CONTROL0		0x1e
>  #define RX8130_REG_CONTROL0_AIE		BIT(3)
> +#define RX8130_REG_CONTROL1		0x0f

I think it is surprising that RX8130_REG_CONTROL0 is defined absolute
and RX8130_REG_CONTROL1 must be used with chip offset. Having this
defined consistently would be nice.

> +#define RX8130_REG_CONTROL1_INIEN	BIT(4)
>  
>  #define MCP794XX_REG_CONTROL		0x07
>  #	define MCP794XX_BIT_ALM0_EN	0x10
> @@ -1750,6 +1752,11 @@ static int ds1307_probe(struct i2c_client *client,
>  				     DS1307_REG_HOUR << 4 | 0x08, hour);
>  		}
>  		break;
> +	case rx_8130:
> +		/* make sure that the backup battery is enabled */
> +		regmap_write(ds1307->regmap, chip->offset + RX8130_REG_CONTROL1,
> +			     RX8130_REG_CONTROL1_INIEN);
> +		break;

Quick note: if a rechargeable battery or supercap is used the CHGEN bit
must be set also.

Regards,
Bastian

>  	default:
>  		break;
>  	}
> 
>
Marek Vasut Sept. 5, 2019, 1 p.m. UTC | #3
On 8/30/19 12:42 PM, Bastian Krause wrote:
> Hi Marek,

Hi,

> On 6/28/19 2:21 AM, Marek Vasut wrote:
>> The battery backup can be disabled on this RTC, e.g. if populated right
>> out of production. Force the battery backup bit on to enable it.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> Cc: Arnaud Ebalard <arno@natisbad.org>
>> Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
>> ---
>>  drivers/rtc/rtc-ds1307.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
>> index 1f7e8aefc1eb..4af00cac1eff 100644
>> --- a/drivers/rtc/rtc-ds1307.c
>> +++ b/drivers/rtc/rtc-ds1307.c
>> @@ -121,6 +121,8 @@ enum ds_type {
>>  #define RX8130_REG_FLAG_AF		BIT(3)
>>  #define RX8130_REG_CONTROL0		0x1e
>>  #define RX8130_REG_CONTROL0_AIE		BIT(3)
>> +#define RX8130_REG_CONTROL1		0x0f
> 
> I think it is surprising that RX8130_REG_CONTROL0 is defined absolute
> and RX8130_REG_CONTROL1 must be used with chip offset. Having this
> defined consistently would be nice.

It's because this old patch was rebased from before the regmap changes
were even in.

>> +#define RX8130_REG_CONTROL1_INIEN	BIT(4)
>>  
>>  #define MCP794XX_REG_CONTROL		0x07
>>  #	define MCP794XX_BIT_ALM0_EN	0x10
>> @@ -1750,6 +1752,11 @@ static int ds1307_probe(struct i2c_client *client,
>>  				     DS1307_REG_HOUR << 4 | 0x08, hour);
>>  		}
>>  		break;
>> +	case rx_8130:
>> +		/* make sure that the backup battery is enabled */
>> +		regmap_write(ds1307->regmap, chip->offset + RX8130_REG_CONTROL1,
>> +			     RX8130_REG_CONTROL1_INIEN);
>> +		break;
> 
> Quick note: if a rechargeable battery or supercap is used the CHGEN bit
> must be set also.

By rechargable battery, you mean coin cell one ?
Bastian Krause Sept. 9, 2019, 1:33 p.m. UTC | #4
On 9/5/19 3:00 PM, Marek Vasut wrote:
> On 8/30/19 12:42 PM, Bastian Krause wrote:
>> On 6/28/19 2:21 AM, Marek Vasut wrote:
>>> The battery backup can be disabled on this RTC, e.g. if populated right
>>> out of production. Force the battery backup bit on to enable it.
>>>
>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>> Cc: Arnaud Ebalard <arno@natisbad.org>
>>> Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
>>> ---
>>>  drivers/rtc/rtc-ds1307.c | 7 +++++++
>>>  1 file changed, 7 insertions(+)
>>>
>>> diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
>>> index 1f7e8aefc1eb..4af00cac1eff 100644
>>> --- a/drivers/rtc/rtc-ds1307.c
>>> +++ b/drivers/rtc/rtc-ds1307.c
>>> @@ -121,6 +121,8 @@ enum ds_type {
>>>  #define RX8130_REG_FLAG_AF		BIT(3)
>>>  #define RX8130_REG_CONTROL0		0x1e
>>>  #define RX8130_REG_CONTROL0_AIE		BIT(3)
>>> +#define RX8130_REG_CONTROL1		0x0f
>>
>> I think it is surprising that RX8130_REG_CONTROL0 is defined absolute
>> and RX8130_REG_CONTROL1 must be used with chip offset. Having this
>> defined consistently would be nice.
> 
> It's because this old patch was rebased from before the regmap changes
> were even in.
> 
>>> +#define RX8130_REG_CONTROL1_INIEN	BIT(4)
>>>  
>>>  #define MCP794XX_REG_CONTROL		0x07
>>>  #	define MCP794XX_BIT_ALM0_EN	0x10
>>> @@ -1750,6 +1752,11 @@ static int ds1307_probe(struct i2c_client *client,
>>>  				     DS1307_REG_HOUR << 4 | 0x08, hour);
>>>  		}
>>>  		break;
>>> +	case rx_8130:
>>> +		/* make sure that the backup battery is enabled */
>>> +		regmap_write(ds1307->regmap, chip->offset + RX8130_REG_CONTROL1,
>>> +			     RX8130_REG_CONTROL1_INIEN);
>>> +		break;
>>
>> Quick note: if a rechargeable battery or supercap is used the CHGEN bit
>> must be set also.
> 
> By rechargable battery, you mean coin cell one ?

Yes, some rechargeable lithium coin/button cell. Or in my case a ELNA
0.2F 3.3V supercap.

Regards,
Bastian

Patch
diff mbox series

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 1f7e8aefc1eb..4af00cac1eff 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -121,6 +121,8 @@  enum ds_type {
 #define RX8130_REG_FLAG_AF		BIT(3)
 #define RX8130_REG_CONTROL0		0x1e
 #define RX8130_REG_CONTROL0_AIE		BIT(3)
+#define RX8130_REG_CONTROL1		0x0f
+#define RX8130_REG_CONTROL1_INIEN	BIT(4)
 
 #define MCP794XX_REG_CONTROL		0x07
 #	define MCP794XX_BIT_ALM0_EN	0x10
@@ -1750,6 +1752,11 @@  static int ds1307_probe(struct i2c_client *client,
 				     DS1307_REG_HOUR << 4 | 0x08, hour);
 		}
 		break;
+	case rx_8130:
+		/* make sure that the backup battery is enabled */
+		regmap_write(ds1307->regmap, chip->offset + RX8130_REG_CONTROL1,
+			     RX8130_REG_CONTROL1_INIEN);
+		break;
 	default:
 		break;
 	}