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