| Message ID | 20190905130336.10651-1-marex@denx.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [V2] rtc: ds1307: Enable battery backup on RX8130 | expand |
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; > } >
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; >> } >> > >
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; >>> } >>> >> >> > >
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.
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.
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
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
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; }
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(+)