[PATCH v2 0/8] rtc: expand charge support, implement rx8130 charging

[PATCH v2 0/8] rtc: expand charge support, implement rx8130 charging

[Bastian Krause]

In order to preserve previous RTC charging behavior while allowing to
add new charging configurations this series adds the dt property
aux-voltage-chargeable as a uint enum. It supersedes the
trickle-diode-disable flag.

Then the ds1307 driver's charging infrastructure is generalized:

- support charging on 'aux-voltage-chargeable = <1>'
- keep the previous charge default per chip
- make trickle-resistor-ohms optional for charging
- apply DS13XX_TRICKLE_CHARGER_MAGIC only conditionally

This preparatory work allows to enable Epson's RX8130 backup
battery and make it chargeable when 'aux-voltage-chargeable = <1>' is
given.

Regards,
Bastian

Bastian Krause (8):
  dt-bindings: rtc: let aux-voltage-chargeable supersede
    trickle-diode-disable
  dt-bindings: rtc: ds1307: let aux-voltage-chargeable supersede
    trickle-diode-disable
  dt-bindings: rtc: ds1307: add rx8130 aux-voltage-chargeable support
  rtc: ds1307: apply DS13XX_TRICKLE_CHARGER_MAGIC only conditionally
  rtc: ds1307: introduce requires_trickle_resistor per chip
  rtc: ds1307: store previous charge default per chip
  rtc: ds1307: consider aux-voltage-chargeable
  rtc: ds1307: enable rx8130's backup battery, make it chargeable
    optionally

 .../devicetree/bindings/rtc/rtc-ds1307.txt    |  9 ++-
 .../devicetree/bindings/rtc/rtc.yaml          | 10 ++++
 drivers/rtc/rtc-ds1307.c                      | 58 +++++++++++++++++--
 3 files changed, 71 insertions(+), 6 deletions(-)

-- 
2.28.0

[PATCH v2 1/8] dt-bindings: rtc: let aux-voltage-chargeable supersede trickle-diode-disable

[Bastian Krause]

Some RTCs can be equipped with a chargeable battery or supercap.
Every RTC allowing this whose driver's implement it are charged by
default. To disable this the trickle-diode-disable flag exists.

If a driver did not support charging and some time later one wants to
add that feature, there is currently no way to do it without breaking
dt backwards compatibility. RTCs on boards without the
trickle-diode-disable flag in their device tree would suddenly charge
their battery/supercap which is a change in behavior.

Change that by introducing aux-voltage-chargeable, not as a flag but as
a uint32 enum allowing to set "do not charge" (0) or "charge" (1). This
dt property is optional, so we can now distinguish these cases.

Care must be taken to support the old behavior for device trees without
aux-voltage-chargeable nonetheless to stay compatible.

Signed-off-by: Bastian Krause <bst@pengutronix.de>
---
Changes since v1:
- fixed dt_binding_check error by adding " |" after "description:"

v1 notes:
In a previous series aux-voltage-chargeable was added as a ds1307 dt
property. Discussions lead to turning that into a generic rtc dt
property:
https://lore.kernel.org/linux-rtc/98fa7181-3ebe-d7c3-cfac-fee841c81e15@pengutronix.de/T/
---
 Documentation/devicetree/bindings/rtc/rtc.yaml | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/rtc/rtc.yaml b/Documentation/devicetree/bindings/rtc/rtc.yaml
index ee237b2ed66a..6b8a11325691 100644
--- a/Documentation/devicetree/bindings/rtc/rtc.yaml
+++ b/Documentation/devicetree/bindings/rtc/rtc.yaml
@@ -17,6 +17,15 @@ properties:
   $nodename:
     pattern: "^rtc(@.*|-[0-9a-f])*$"
 
+  aux-voltage-chargeable:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [0, 1]
+    description: |
+      Tells whether the battery/supercap of the RTC (if any) is
+      chargeable or not:
+      0: not chargeable
+      1: chargeable
+
   quartz-load-femtofarads:
     $ref: /schemas/types.yaml#/definitions/uint32
     description:
@@ -35,6 +44,7 @@ properties:
     description:
       Do not use internal trickle charger diode. Should be given if
       internal trickle charger diode should be disabled.
+    deprecated: true
 
   trickle-resistor-ohms:
     $ref: /schemas/types.yaml#/definitions/uint32
-- 
2.28.0

Re: [PATCH v2 1/8] dt-bindings: rtc: let aux-voltage-chargeable supersede trickle-diode-disable

[Rob Herring]

On Thu, 17 Sep 2020 20:32:39 +0200, Bastian Krause wrote:
> Some RTCs can be equipped with a chargeable battery or supercap.
> Every RTC allowing this whose driver's implement it are charged by
> default. To disable this the trickle-diode-disable flag exists.
> 
> If a driver did not support charging and some time later one wants to
> add that feature, there is currently no way to do it without breaking
> dt backwards compatibility. RTCs on boards without the
> trickle-diode-disable flag in their device tree would suddenly charge
> their battery/supercap which is a change in behavior.
> 
> Change that by introducing aux-voltage-chargeable, not as a flag but as
> a uint32 enum allowing to set "do not charge" (0) or "charge" (1). This
> dt property is optional, so we can now distinguish these cases.
> 
> Care must be taken to support the old behavior for device trees without
> aux-voltage-chargeable nonetheless to stay compatible.
> 
> Signed-off-by: Bastian Krause <bst@pengutronix.de>
> ---
> Changes since v1:
> - fixed dt_binding_check error by adding " |" after "description:"
> 
> v1 notes:
> In a previous series aux-voltage-chargeable was added as a ds1307 dt
> property. Discussions lead to turning that into a generic rtc dt
> property:
> https://lore.kernel.org/linux-rtc/98fa7181-3ebe-d7c3-cfac-fee841c81e15@pengutronix.de/T/
> ---
>  Documentation/devicetree/bindings/rtc/rtc.yaml | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

[PATCH v2 2/8] dt-bindings: rtc: ds1307: let aux-voltage-chargeable supersede trickle-diode-disable

[Bastian Krause]

trickle-diode-disable is deprecated, so reflect that in the driver's
binding and add the new aux-voltage-chargeable.

Signed-off-by: Bastian Krause <bst@pengutronix.de>
Reviewed-by: Rob Herring <robh@kernel.org>
---
Changes since v1:
- added Rob Herring's Reviewed-by

v1 notes:
Previous version:
https://lore.kernel.org/linux-rtc/20200415163701.21989-2-bst@pengutronix.de/
---
 Documentation/devicetree/bindings/rtc/rtc-ds1307.txt | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/rtc/rtc-ds1307.txt b/Documentation/devicetree/bindings/rtc/rtc-ds1307.txt
index 66f0a31ae9ce..08ea9734da80 100644
--- a/Documentation/devicetree/bindings/rtc/rtc-ds1307.txt
+++ b/Documentation/devicetree/bindings/rtc/rtc-ds1307.txt
@@ -31,9 +31,16 @@ Optional properties:
 	Selected resistor for trickle charger
 	Possible values are 250, 2000, 4000
 	Should be given if trickle charger should be enabled
-- trickle-diode-disable : ds1339, ds1340 and ds 1388 only
+- aux-voltage-chargeable: ds1339, ds1340 and ds1388 only
+	Tells whether the battery/supercap of the RTC (if any) is
+	chargeable or not.
+	Possible values are 0 (not chargeable), 1 (chargeable)
+
+Deprecated properties:
+- trickle-diode-disable : ds1339, ds1340 and ds1388 only
 	Do not use internal trickle charger diode
 	Should be given if internal trickle charger diode should be disabled
+	(superseded by aux-voltage-chargeable)
 
 Example:
 	ds1339: rtc@68 {
-- 
2.28.0

[PATCH v2 3/8] dt-bindings: rtc: ds1307: add rx8130 aux-voltage-chargeable support

[Bastian Krause]

Epson's RX8130 was not charged before. A related patch will allow
optional charging.

Signed-off-by: Bastian Krause <bst@pengutronix.de>
Reviewed-by: Rob Herring <robh@kernel.org>
---
Changes since v1:
- added Rob Herring's Reviewed-by

v1 notes:
Previous version:
https://lore.kernel.org/linux-rtc/20200415163701.21989-2-bst@pengutronix.de/
---
 Documentation/devicetree/bindings/rtc/rtc-ds1307.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/rtc/rtc-ds1307.txt b/Documentation/devicetree/bindings/rtc/rtc-ds1307.txt
index 08ea9734da80..36f610bb051e 100644
--- a/Documentation/devicetree/bindings/rtc/rtc-ds1307.txt
+++ b/Documentation/devicetree/bindings/rtc/rtc-ds1307.txt
@@ -31,7 +31,7 @@ Optional properties:
 	Selected resistor for trickle charger
 	Possible values are 250, 2000, 4000
 	Should be given if trickle charger should be enabled
-- aux-voltage-chargeable: ds1339, ds1340 and ds1388 only
+- aux-voltage-chargeable: ds1339, ds1340, ds1388 and rx8130 only
 	Tells whether the battery/supercap of the RTC (if any) is
 	chargeable or not.
 	Possible values are 0 (not chargeable), 1 (chargeable)
-- 
2.28.0

[PATCH v2 4/8] rtc: ds1307: apply DS13XX_TRICKLE_CHARGER_MAGIC only conditionally

[Bastian Krause]

DS13XX_TRICKLE_CHARGER_MAGIC sets the trickle-charge select (TCS) bits
(7..4). The datasheet of Maxim Integrated's DS1339 [1] for instance
reads:

"To prevent accidental enabling, only a pattern on 1010 enables the
trickle charger. All other patterns disable the trickle charger."

Since not all RTCs connected to a backup battery or supercap use these
bits DS13XX_TRICKLE_CHARGER_MAGIC should not get applied for all charger
setups unconditionally.
Epson's RX8130 is such an example: Instead of TCS bits "SMPTSEL1",
"SMPTSEL0",  "CHGEN" and "INIEN" are expected as bit 7..4.

DS1339 and DS1340 are currently the only RTCs in the ds1307 driver that
apply DS13XX_TRICKLE_CHARGER_MAGIC to their setup register value. So
apply DS13XX_TRICKLE_CHARGER_MAGIC in do_trickle_setup_ds1339() which
is used by both RTCs.

[1] https://datasheets.maximintegrated.com/en/ds/DS1339-DS1339U.pdf
[2] https://support.epson.biz/td/api/doc_check.php?dl=app_RX8130CE

Signed-off-by: Bastian Krause <bst@pengutronix.de>
---
No changes since v1.

v1 notes:
No previous version.
---
 drivers/rtc/rtc-ds1307.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 54c85cdd019d..ff5242d10b21 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -507,6 +507,8 @@ static u8 do_trickle_setup_ds1339(struct ds1307 *ds1307, u32 ohms, bool diode)
 	u8 setup = (diode) ? DS1307_TRICKLE_CHARGER_DIODE :
 		DS1307_TRICKLE_CHARGER_NO_DIODE;
 
+	setup |= DS13XX_TRICKLE_CHARGER_MAGIC;
+
 	switch (ohms) {
 	case 250:
 		setup |= DS1307_TRICKLE_CHARGER_250_OHM;
@@ -1758,7 +1760,6 @@ static int ds1307_probe(struct i2c_client *client,
 		trickle_charger_setup = pdata->trickle_charger_setup;
 
 	if (trickle_charger_setup && chip->trickle_charger_reg) {
-		trickle_charger_setup |= DS13XX_TRICKLE_CHARGER_MAGIC;
 		dev_dbg(ds1307->dev,
 			"writing trickle charger info 0x%x to 0x%x\n",
 			trickle_charger_setup, chip->trickle_charger_reg);
-- 
2.28.0

[PATCH v2 5/8] rtc: ds1307: introduce requires_trickle_resistor per chip

[Bastian Krause]

Make trickle-resistor-ohms optional for charging setups that do not
require specifying ROUT bits (specifying the resistor value between Vcc
and Vbackup). In order to allow specifying that, introduce
requires_trickle_resistor per chip.

Signed-off-by: Bastian Krause <bst@pengutronix.de>
---
No changes since v1.

v1 notes:
No previous version.
---
 drivers/rtc/rtc-ds1307.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index ff5242d10b21..7983438b677a 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -190,6 +190,10 @@ struct chip_desc {
 	u16			trickle_charger_reg;
 	u8			(*do_trickle_setup)(struct ds1307 *, u32,
 						    bool);
+	/* Does the RTC require trickle-resistor-ohms to select the value of
+	 * the resistor between Vcc and Vbackup?
+	 */
+	bool			requires_trickle_resistor;
 };
 
 static const struct chip_desc chips[last_ds_type];
@@ -981,6 +985,7 @@ static const struct chip_desc chips[last_ds_type] = {
 		.bbsqi_bit	= DS1339_BIT_BBSQI,
 		.trickle_charger_reg = 0x10,
 		.do_trickle_setup = &do_trickle_setup_ds1339,
+		.requires_trickle_resistor = true,
 	},
 	[ds_1340] = {
 		.century_reg	= DS1307_REG_HOUR,
@@ -988,6 +993,7 @@ static const struct chip_desc chips[last_ds_type] = {
 		.century_bit	= DS1340_BIT_CENTURY,
 		.do_trickle_setup = &do_trickle_setup_ds1339,
 		.trickle_charger_reg = 0x08,
+		.requires_trickle_resistor = true,
 	},
 	[ds_1341] = {
 		.century_reg	= DS1307_REG_MONTH,
@@ -1302,7 +1308,7 @@ static u8 ds1307_trickle_init(struct ds1307 *ds1307,
 		return 0;
 
 	if (device_property_read_u32(ds1307->dev, "trickle-resistor-ohms",
-				     &ohms))
+				     &ohms) && chip->requires_trickle_resistor)
 		return 0;
 
 	if (device_property_read_bool(ds1307->dev, "trickle-diode-disable"))
-- 
2.28.0

[PATCH v2 6/8] rtc: ds1307: store previous charge default per chip

[Bastian Krause]

Some RTC's batteries and supercaps were charged by default until now.
In contrast other RTCs allow charging but the driver did not configure
them to do so until now. These must not be charged by default to stay
backwards compatible.

In order to do that, store the charge default per chip.

Signed-off-by: Bastian Krause <bst@pengutronix.de>
---
No changes since v1.

v1 notes:
No previous version.
---
 drivers/rtc/rtc-ds1307.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 7983438b677a..270621e1c9cf 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -194,6 +194,11 @@ struct chip_desc {
 	 * the resistor between Vcc and Vbackup?
 	 */
 	bool			requires_trickle_resistor;
+	/* Some RTC's batteries and supercaps were charged by default, others
+	 * allow charging but were not configured previously to do so.
+	 * Remember this behavior to stay backwards compatible.
+	 */
+	bool			charge_default;
 };
 
 static const struct chip_desc chips[last_ds_type];
@@ -986,6 +991,7 @@ static const struct chip_desc chips[last_ds_type] = {
 		.trickle_charger_reg = 0x10,
 		.do_trickle_setup = &do_trickle_setup_ds1339,
 		.requires_trickle_resistor = true,
+		.charge_default = true,
 	},
 	[ds_1340] = {
 		.century_reg	= DS1307_REG_HOUR,
@@ -994,6 +1000,7 @@ static const struct chip_desc chips[last_ds_type] = {
 		.do_trickle_setup = &do_trickle_setup_ds1339,
 		.trickle_charger_reg = 0x08,
 		.requires_trickle_resistor = true,
+		.charge_default = true,
 	},
 	[ds_1341] = {
 		.century_reg	= DS1307_REG_MONTH,
@@ -1302,7 +1309,7 @@ static u8 ds1307_trickle_init(struct ds1307 *ds1307,
 			      const struct chip_desc *chip)
 {
 	u32 ohms;
-	bool diode = true;
+	bool diode = chip->charge_default;
 
 	if (!chip->do_trickle_setup)
 		return 0;
-- 
2.28.0

[PATCH v2 7/8] rtc: ds1307: consider aux-voltage-chargeable

[Bastian Krause]

Prefer aux-voltage-chargeable over trickle-diode-disable and set diode
accordingly. This is then passed to the chip's appropriate charge setup
function.

Signed-off-by: Bastian Krause <bst@pengutronix.de>
---
No changes since v1.

v1 notes:
This is the discussion that lead to this patch:
https://lore.kernel.org/linux-rtc/a492b6a0-b41c-a088-3ba1-f1448a074b34@pengutronix.de/
---
 drivers/rtc/rtc-ds1307.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 270621e1c9cf..9bf1822a989f 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -1308,7 +1308,7 @@ static int ds1307_nvram_write(void *priv, unsigned int offset, void *val,
 static u8 ds1307_trickle_init(struct ds1307 *ds1307,
 			      const struct chip_desc *chip)
 {
-	u32 ohms;
+	u32 ohms, chargeable;
 	bool diode = chip->charge_default;
 
 	if (!chip->do_trickle_setup)
@@ -1318,8 +1318,27 @@ static u8 ds1307_trickle_init(struct ds1307 *ds1307,
 				     &ohms) && chip->requires_trickle_resistor)
 		return 0;
 
-	if (device_property_read_bool(ds1307->dev, "trickle-diode-disable"))
+	/* aux-voltage-chargeable takes precedence over the deprecated
+	 * trickle-diode-disable
+	 */
+	if (!device_property_read_u32(ds1307->dev, "aux-voltage-chargeable",
+				     &chargeable)) {
+		switch (chargeable) {
+		case 0:
+			diode = false;
+			break;
+		case 1:
+			diode = true;
+			break;
+		default:
+			dev_warn(ds1307->dev,
+				 "unsupported aux-voltage-chargeable value\n");
+			break;
+		}
+	} else if (device_property_read_bool(ds1307->dev,
+					     "trickle-diode-disable")) {
 		diode = false;
+	}
 
 	return chip->do_trickle_setup(ds1307, ohms, diode);
 }
-- 
2.28.0

[PATCH v2 8/8] rtc: ds1307: enable rx8130's backup battery, make it chargeable optionally

[Bastian Krause]

The ds1307 charger infrastructure now allows to add a rx8130 charger
setup that..

- does not depend on trickle-resistor-ohms
- does not use DS13XX_TRICKLE_CHARGER_MAGIC trickle-charge select (TCS)
  bits
- keeps previous no-charge behavior for device trees without
  aux-voltage-chargeable

Make that happen.

Signed-off-by: Bastian Krause <bst@pengutronix.de>
---
No changes since v1.

v1 notes:
Based on:
- https://lore.kernel.org/linux-rtc/20200415163701.21989-1-bst@pengutronix.de/
- https://lore.kernel.org/linux-rtc/20200415163701.21989-3-bst@pengutronix.de/

Changes since then:
- use chager_reg (called trickle_charger_reg before patch 4/8)
- use charger setup function to set backup battery enable bit, charge
  bit optionally (introduced by patch 5/8)
---
 drivers/rtc/rtc-ds1307.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 9bf1822a989f..1fe0c2df2578 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -122,6 +122,9 @@ 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 RX8130_REG_CONTROL1_CHGEN	BIT(5)
 
 #define MCP794XX_REG_CONTROL		0x07
 #	define MCP794XX_BIT_ALM0_EN	0x10
@@ -536,6 +539,16 @@ static u8 do_trickle_setup_ds1339(struct ds1307 *ds1307, u32 ohms, bool diode)
 	return setup;
 }
 
+static u8 do_trickle_setup_rx8130(struct ds1307 *ds1307, u32 ohms, bool diode)
+{
+	/* make sure that the backup battery is enabled */
+	u8 setup = RX8130_REG_CONTROL1_INIEN;
+	if (diode)
+		setup |= RX8130_REG_CONTROL1_CHGEN;
+
+	return setup;
+}
+
 static irqreturn_t rx8130_irq(int irq, void *dev_id)
 {
 	struct ds1307           *ds1307 = dev_id;
@@ -1024,6 +1037,8 @@ static const struct chip_desc chips[last_ds_type] = {
 		.offset		= 0x10,
 		.irq_handler = rx8130_irq,
 		.rtc_ops = &rx8130_rtc_ops,
+		.trickle_charger_reg = RX8130_REG_CONTROL1,
+		.do_trickle_setup = &do_trickle_setup_rx8130,
 	},
 	[m41t0] = {
 		.rtc_ops	= &m41txx_rtc_ops,
-- 
2.28.0

Re: [PATCH v2 0/8] rtc: expand charge support, implement rx8130 charging

[Alexandre Belloni]

On Thu, 17 Sep 2020 20:32:38 +0200, Bastian Krause wrote:
> In order to preserve previous RTC charging behavior while allowing to
> add new charging configurations this series adds the dt property
> aux-voltage-chargeable as a uint enum. It supersedes the
> trickle-diode-disable flag.
> 
> Then the ds1307 driver's charging infrastructure is generalized:
> 
> [...]

Applied, thanks!

[1/8] dt-bindings: rtc: let aux-voltage-chargeable supersede trickle-diode-disable
      commit: 965fe1ceea05de735f63d1c1f90f0310fc830a8d
[2/8] dt-bindings: rtc: ds1307: let aux-voltage-chargeable supersede trickle-diode-disable
      commit: c53cee48670cc623a42e49319a261798d8608b0e
[3/8] dt-bindings: rtc: ds1307: add rx8130 aux-voltage-chargeable support
      commit: 40d58c9742286ef95ef8dc5885d4043224490c82
[4/8] rtc: ds1307: apply DS13XX_TRICKLE_CHARGER_MAGIC only conditionally
      commit: 462eb736db3db76899022e4e4db788a7b6efbe09
[5/8] rtc: ds1307: introduce requires_trickle_resistor per chip
      commit: 1b5b6af788ae59ee73e3f3230dbfa4f0e31d8d18
[6/8] rtc: ds1307: store previous charge default per chip
      commit: 95a74cbb21a2431dd2fd8918fa26113629b6e13e
[7/8] rtc: ds1307: consider aux-voltage-chargeable
      commit: 0874734e09af5cc05439dbe2c8ff704f14d679f5
[8/8] rtc: ds1307: enable rx8130's backup battery, make it chargeable optionally
      commit: 0026f1604c9ba1ae8108d4977da0366c283552bc

Best regards,
-- 
Alexandre Belloni <alexandre.belloni@bootlin.com>