[PATCH V2] rtc: ds1307: Enable battery backup on RX8130

[PATCH V2] rtc: ds1307: Enable battery backup on RX8130

[Marek Vasut]

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>
Cc: Bastian Krause <bst@pengutronix.de>
---
V2: Drop the custom offset, let regmap handle that
---
 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..f2d1e59478c2 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		0x1f
+#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, RX8130_REG_CONTROL1,
+			     RX8130_REG_CONTROL1_INIEN);
+		break;
 	default:
 		break;
 	}
-- 
2.23.0.rc1

Re: [PATCH V2] rtc: ds1307: Enable battery backup on RX8130

[Bastian Krause]

On 9/5/19 3:03 PM, 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>
> Cc: Bastian Krause <bst@pengutronix.de>

Reviewed-by: Bastian Krause <bst@pengutronix.de>

Regards,
Bastian

> ---
> V2: Drop the custom offset, let regmap handle that
> ---
>  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..f2d1e59478c2 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		0x1f
> +#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, RX8130_REG_CONTROL1,
> +			     RX8130_REG_CONTROL1_INIEN);
> +		break;
>  	default:
>  		break;
>  	}
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH V2] rtc: ds1307: Enable battery backup on RX8130

[Marek Vasut]

On 11/21/19 9:09 AM, Bastian Krause wrote:
> On 9/5/19 3:03 PM, 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>
>> Cc: Bastian Krause <bst@pengutronix.de>
> 
> Reviewed-by: Bastian Krause <bst@pengutronix.de>
> 

I recall there was some comment about setting BIT(5) as well,
RX8130_REG_CONTROL1_CHGEN , can you check that ?

> 
>> ---
>> V2: Drop the custom offset, let regmap handle that
>> ---
>>  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..f2d1e59478c2 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		0x1f
>> +#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, RX8130_REG_CONTROL1,
>> +			     RX8130_REG_CONTROL1_INIEN);
>> +		break;
>>  	default:
>>  		break;
>>  	}
>>
> 
> 


-- 
Best regards,
Marek Vasut

Re: [PATCH V2] rtc: ds1307: Enable battery backup on RX8130

[Bastian Krause]

On 11/21/19 9:14 AM, Marek Vasut wrote:
> On 11/21/19 9:09 AM, Bastian Krause wrote:
>> On 9/5/19 3:03 PM, 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>
>>> Cc: Bastian Krause <bst@pengutronix.de>
>>
>> Reviewed-by: Bastian Krause <bst@pengutronix.de>
>>
> 
> I recall there was some comment about setting BIT(5) as well,
> RX8130_REG_CONTROL1_CHGEN , can you check that ?

RX8130_REG_CONTROL1_CHGEN decides whether the battery or the supercap
should be charged or not. I think this patch is okay as is. I'll send a
follow-up patch which will set RX8130_REG_CONTROL1_CHGEN depending on a
new dt-binding "epson,backup-battery-chargeable" once this one is applied.

Regards,
Bastian

> 
>>
>>> ---
>>> V2: Drop the custom offset, let regmap handle that
>>> ---
>>>  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..f2d1e59478c2 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		0x1f
>>> +#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, RX8130_REG_CONTROL1,
>>> +			     RX8130_REG_CONTROL1_INIEN);
>>> +		break;
>>>  	default:
>>>  		break;
>>>  	}
>>>
>>
>>
> 
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH V2] rtc: ds1307: Enable battery backup on RX8130

[Marek Vasut]

On 11/21/19 9:21 AM, Bastian Krause wrote:
> 
> On 11/21/19 9:14 AM, Marek Vasut wrote:
>> On 11/21/19 9:09 AM, Bastian Krause wrote:
>>> On 9/5/19 3:03 PM, 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>
>>>> Cc: Bastian Krause <bst@pengutronix.de>
>>>
>>> Reviewed-by: Bastian Krause <bst@pengutronix.de>
>>>
>>
>> I recall there was some comment about setting BIT(5) as well,
>> RX8130_REG_CONTROL1_CHGEN , can you check that ?
> 
> RX8130_REG_CONTROL1_CHGEN decides whether the battery or the supercap
> should be charged or not. I think this patch is okay as is. I'll send a
> follow-up patch which will set RX8130_REG_CONTROL1_CHGEN depending on a
> new dt-binding "epson,backup-battery-chargeable" once this one is applied.

Even better, thanks.

Re: [PATCH V2] rtc: ds1307: Enable battery backup on RX8130

[Alexandre Belloni]

On 21/11/2019 09:21:49+0100, Bastian Krause wrote:
> 
> On 11/21/19 9:14 AM, Marek Vasut wrote:
> > On 11/21/19 9:09 AM, Bastian Krause wrote:
> >> On 9/5/19 3:03 PM, 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>
> >>> Cc: Bastian Krause <bst@pengutronix.de>
> >>
> >> Reviewed-by: Bastian Krause <bst@pengutronix.de>
> >>
> > 
> > I recall there was some comment about setting BIT(5) as well,
> > RX8130_REG_CONTROL1_CHGEN , can you check that ?
> 
> RX8130_REG_CONTROL1_CHGEN decides whether the battery or the supercap
> should be charged or not. I think this patch is okay as is. I'll send a
> follow-up patch which will set RX8130_REG_CONTROL1_CHGEN depending on a
> new dt-binding "epson,backup-battery-chargeable" once this one is applied.
> 

You need to have a generic RTC property, either reuse
trickle-diode-disable (I know the name is a bit unfortunate but that is
waht we have) or have a new property stating that the auxiliary voltage
is chargeable. using battery in the name is probably not wise because
this may as well be a supercap.

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH V2] rtc: ds1307: Enable battery backup on RX8130

[Bastian Krause]

On 11/21/19 5:39 PM, Alexandre Belloni wrote:
> On 21/11/2019 09:21:49+0100, Bastian Krause wrote:
>>
>> On 11/21/19 9:14 AM, Marek Vasut wrote:
>>> On 11/21/19 9:09 AM, Bastian Krause wrote:
>>>> On 9/5/19 3:03 PM, 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>
>>>>> Cc: Bastian Krause <bst@pengutronix.de>
>>>>
>>>> Reviewed-by: Bastian Krause <bst@pengutronix.de>
>>>>
>>>
>>> I recall there was some comment about setting BIT(5) as well,
>>> RX8130_REG_CONTROL1_CHGEN , can you check that ?
>>
>> RX8130_REG_CONTROL1_CHGEN decides whether the battery or the supercap
>> should be charged or not. I think this patch is okay as is. I'll send a
>> follow-up patch which will set RX8130_REG_CONTROL1_CHGEN depending on a
>> new dt-binding "epson,backup-battery-chargeable" once this one is applied.
>>
> 
> You need to have a generic RTC property, either reuse
> trickle-diode-disable (I know the name is a bit unfortunate but that is
> waht we have) or have a new property stating that the auxiliary voltage
> is chargeable. using battery in the name is probably not wise because
> this may as well be a supercap.

Alright, thanks for the suggestion. I will incorporate into the patch.

Regards,
Bastian

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH V2] rtc: ds1307: Enable battery backup on RX8130

[Bastian Krause]

On 11/21/19 9:09 AM, Bastian Krause wrote:
> On 9/5/19 3:03 PM, 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>
>> Cc: Bastian Krause <bst@pengutronix.de>
> 
> Reviewed-by: Bastian Krause <bst@pengutronix.de>

Gentle ping.

Regards,
Bastian

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |