[PATCH v3 0/6] add quartz load support to NXP rtc drivers

[PATCH v3 0/6] add quartz load support to NXP rtc drivers

[Sam Ravnborg]

Respin of patchset on top of v5.0-rc2 with feedback addressed (thanks!)

Introduce a new generic property "quartz-load-femtofarad"
to specify the quartz load.
The default value is selected to match the current Linux
drivers, so there are no behavior changes if a binding do not
specify the quarts-load.
The unit is femtofarads because several RTC's define the quarts load
in steps of 0.5 pF.

The two drivers rtc-pcf8523 and rtc-pcf85063 are both
updated to use the new binding.
Previously pcf8523 hardcoded the quartz load to 12.5pF, and
pcf85063 used the reset default of 7pF.
With this patchset it can now be specified in the DT,
if the defaults are not OK.

The patch-set is tested on a proprietary board for both
RTC variants (v2 variant only as there was only trivial changes in v3).

	Sam

v3:
- Document generic property "quartz-load-femtofarads" in rtc.txt (Rob)
- Fix indent several places - ups (Alexandre)
- Use the simpler name "rtc" in DT examples (Rob)
- Collected "Reviewed by's"

v2:
- Introduce a generic property "quartz-load-femtofarads"
- Add femtofarads to property-units.txt
- Make the changes backward compatible
- Reduced logging
- Warn, when we continue with a default value
- Tested, by Søren Andersen, on real HW
- Rebased on top of v5.0-rc1

        Sam

Sam Ravnborg (6):
      devicetree: property-units: Add femtofarads unit
      dt-bindings: rtc: Add quartz-load-femtofarads property
      dt-binding: pcf8523: add xtal load capacitance
      dt-binding: pcf85063: add xtal load capacitance
      rtc: pcf8523: set xtal load capacitance from DT
      rtc: pcf85063: set xtal load capacitance from DT

 .../devicetree/bindings/property-units.txt         |  1 +
 .../devicetree/bindings/rtc/nxp,pcf85063.txt       | 18 ++++++++++
 .../devicetree/bindings/rtc/nxp,pcf8523.txt        | 18 ++++++++++
 Documentation/devicetree/bindings/rtc/rtc.txt      | 18 +++++-----
 drivers/rtc/rtc-pcf85063.c                         | 39 ++++++++++++++++++++++
 drivers/rtc/rtc-pcf8523.c                          | 28 +++++++++++-----
 6 files changed, 106 insertions(+), 16 deletions(-)

[PATCH v3 1/6] devicetree: property-units: Add femtofarads unit

[Sam Ravnborg]

When dealing with capacitance of 0.5 pF then
a smaller unit is preferred.
Add femtofarads to deal with this.

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 Documentation/devicetree/bindings/property-units.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/property-units.txt b/Documentation/devicetree/bindings/property-units.txt
index 45ce054d844d..bfd33734faca 100644
--- a/Documentation/devicetree/bindings/property-units.txt
+++ b/Documentation/devicetree/bindings/property-units.txt
@@ -31,6 +31,7 @@ Electricity
 -microwatt-hours: micro Watt-hours
 -microvolt	: micro volts
 -picofarads	: picofarads
+-femtofarads	: femtofarads
 
 Temperature
 ----------------------------------------
-- 
2.12.0

[PATCH v3 2/6] dt-bindings: rtc: Add quartz-load-femtofarads property

[Sam Ravnborg]

The quartz-load-femtofarads are relevant for
several users. Add it as a common property in rtc.
Note that valid values and default values must be documented.

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Suggested-by: Rob Herring <robh+dt@kernel.org>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
---
 Documentation/devicetree/bindings/rtc/rtc.txt | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/rtc/rtc.txt b/Documentation/devicetree/bindings/rtc/rtc.txt
index 7c8da6926095..41eb335ea816 100644
--- a/Documentation/devicetree/bindings/rtc/rtc.txt
+++ b/Documentation/devicetree/bindings/rtc/rtc.txt
@@ -21,12 +21,16 @@ Optional properties
 The following properties may not be supported by all drivers. However, if a
 driver wants to support one of the below features, it should adapt the bindings
 below.
-- trickle-resistor-ohms : Selected resistor for trickle charger. Should be given
-                          if trickle charger should be enabled
-- trickle-diode-disable : Do not use internal trickle charger diode Should be
-                          given if internal trickle charger diode should be
-                          disabled
-- wakeup-source :         Enables wake up of host system on alarm
+- trickle-resistor-ohms :   Selected resistor for trickle charger. Should be given
+                            if trickle charger should be enabled
+- trickle-diode-disable :   Do not use internal trickle charger diode Should be
+                            given if internal trickle charger diode should be
+                            disabled
+- wakeup-source :           Enables wake up of host system on alarm
+- quartz-load-femtofarads : The capacitive load of the quartz(x-tal),
+                            expressed in femto Farad (fF).
+                            The default value shall be listed (if optional),
+                            and likewise all valid values.
 
 Trivial RTCs
 ------------
-- 
2.12.0

[PATCH v3 3/6] dt-binding: pcf8523: add xtal load capacitance

[Sam Ravnborg]

The NXP pcf8523 supports two different xtal load capacitance
- 7000fF  (7pF)    HW default
- 12500fF (12.5pF) Minimum power consumption, driver default

To obtain a precise RTC the pcf8523 must be configured
with the correct capacitance load of the xtal.

Add a property to specify the xtal capacitance load.
The default value matches that of the current Linux driver.

With a dedicated binding remove the entry in rtc.txt

Signed-off-by: Søren Andersen <san@skov.dk>
Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Mark Rutland <mark.rutland@arm.com>
---
 Documentation/devicetree/bindings/rtc/nxp,pcf8523.txt | 18 ++++++++++++++++++
 Documentation/devicetree/bindings/rtc/rtc.txt         |  1 -
 2 files changed, 18 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/rtc/nxp,pcf8523.txt

diff --git a/Documentation/devicetree/bindings/rtc/nxp,pcf8523.txt b/Documentation/devicetree/bindings/rtc/nxp,pcf8523.txt
new file mode 100644
index 000000000000..0b1080c60f63
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/nxp,pcf8523.txt
@@ -0,0 +1,18 @@
+* NXP PCF8523 Real Time Clock
+
+Required properties:
+- compatible: Should contain "nxp,pcf8523".
+- reg: I2C address for chip.
+
+Optional property:
+- quartz-load-femtofarads: The capacitive load of the quartz(x-tal),
+  expressed in femto Farad (fF). Valid values are 7000 and 12500.
+  Default value (if no value is specified) is 12500fF.
+
+Example:
+
+pcf8523: rtc@68 {
+	compatible = "nxp,pcf8523";
+	reg = <0x68>;
+	quartz-load-femtofarads = <7000>;
+};
diff --git a/Documentation/devicetree/bindings/rtc/rtc.txt b/Documentation/devicetree/bindings/rtc/rtc.txt
index 41eb335ea816..6d726a801d21 100644
--- a/Documentation/devicetree/bindings/rtc/rtc.txt
+++ b/Documentation/devicetree/bindings/rtc/rtc.txt
@@ -55,7 +55,6 @@ isil,isl12022		Intersil ISL12022 Real-time Clock
 microcrystal,rv3029	Real Time Clock Module with I2C-Bus
 nxp,pcf2127		Real-time clock
 nxp,pcf2129		Real-time clock
-nxp,pcf8523		Real-time Clock
 nxp,pcf8563		Real-time clock/calendar
 nxp,pcf85063		Tiny Real-Time Clock
 pericom,pt7c4338	Real-time Clock Module
-- 
2.12.0

[PATCH v3 4/6] dt-binding: pcf85063: add xtal load capacitance

[Sam Ravnborg]

The NXP pcf85063 supports two different xtal load capacitance
- 7000fF  (7pF)    HW default, Linux driver default
- 12500fF (12.5pF) Minimum power consumption

To obtain a precise RTC the pcf85063 must be configured
with the correct capacitance load of the xtal.

Add a property to specify the xtal capacitance load.
The default value matches that of the current Linux driver.

With a dedicated binding remove the entry in rtc.txt

Signed-off-by: Søren Andersen <san@skov.dk>
Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Urs Fässler <urs.fassler@bbv.ch>
---
 Documentation/devicetree/bindings/rtc/nxp,pcf85063.txt | 18 ++++++++++++++++++
 Documentation/devicetree/bindings/rtc/rtc.txt          |  1 -
 2 files changed, 18 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/rtc/nxp,pcf85063.txt

diff --git a/Documentation/devicetree/bindings/rtc/nxp,pcf85063.txt b/Documentation/devicetree/bindings/rtc/nxp,pcf85063.txt
new file mode 100644
index 000000000000..d3e380ad712d
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/nxp,pcf85063.txt
@@ -0,0 +1,18 @@
+* NXP PCF85063 Real Time Clock
+
+Required properties:
+- compatible: Should contain "nxp,pcf85063".
+- reg: I2C address for chip.
+
+Optional property:
+- quartz-load-femtofarads: The capacitive load of the quartz(x-tal),
+  expressed in femto Farad (fF). Valid values are 7000 and 12500.
+  Default value (if no value is specified) is 7000fF.
+
+Example:
+
+pcf85063: rtc@51 {
+	compatible = "nxp,pcf85063";
+	reg = <0x51>;
+	quartz-load-femtofarads = <12500>;
+};
diff --git a/Documentation/devicetree/bindings/rtc/rtc.txt b/Documentation/devicetree/bindings/rtc/rtc.txt
index 6d726a801d21..460c503e86c6 100644
--- a/Documentation/devicetree/bindings/rtc/rtc.txt
+++ b/Documentation/devicetree/bindings/rtc/rtc.txt
@@ -56,7 +56,6 @@ microcrystal,rv3029	Real Time Clock Module with I2C-Bus
 nxp,pcf2127		Real-time clock
 nxp,pcf2129		Real-time clock
 nxp,pcf8563		Real-time clock/calendar
-nxp,pcf85063		Tiny Real-Time Clock
 pericom,pt7c4338	Real-time Clock Module
 ricoh,r2025sd		I2C bus SERIAL INTERFACE REAL-TIME CLOCK IC
 ricoh,r2221tl		I2C bus SERIAL INTERFACE REAL-TIME CLOCK IC
-- 
2.12.0

[PATCH v3 5/6] rtc: pcf8523: set xtal load capacitance from DT

[Sam Ravnborg]

Add support for specifying the xtal load capacitance in the DT node.
The pcf8523 supports xtal load capacitance of 7pF or 12.5pF.
If the rtc has the wrong configuration the time will
drift several hours/week.

The driver use the default value 12.5pF.

The DT may specify either 7000fF or 12500fF.
(The DT uses femto Farad to avoid decimal numbers).
Other values are warned and the driver uses the default value.

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 drivers/rtc/rtc-pcf8523.c | 28 ++++++++++++++++++++--------
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/rtc/rtc-pcf8523.c b/drivers/rtc/rtc-pcf8523.c
index 3fcd2cbafc84..2e03021f15d1 100644
--- a/drivers/rtc/rtc-pcf8523.c
+++ b/drivers/rtc/rtc-pcf8523.c
@@ -97,8 +97,9 @@ static int pcf8523_voltage_low(struct i2c_client *client)
 	return !!(value & REG_CONTROL3_BLF);
 }
 
-static int pcf8523_select_capacitance(struct i2c_client *client, bool high)
+static int pcf8523_load_capacitance(struct i2c_client *client)
 {
+	u32 load;
 	u8 value;
 	int err;
 
@@ -106,14 +107,24 @@ static int pcf8523_select_capacitance(struct i2c_client *client, bool high)
 	if (err < 0)
 		return err;
 
-	if (!high)
-		value &= ~REG_CONTROL1_CAP_SEL;
-	else
+	load = 12500;
+	of_property_read_u32(client->dev.of_node, "quartz-load-femtofarads",
+			     &load);
+
+	switch (load) {
+	default:
+		dev_warn(&client->dev, "Unknown quartz-load-femtofarads value: %d. Assuming 12500",
+			 load);
+		/* fall through */
+	case 12500:
 		value |= REG_CONTROL1_CAP_SEL;
+		break;
+	case 7000:
+		value &= ~REG_CONTROL1_CAP_SEL;
+		break;
+	}
 
 	err = pcf8523_write(client, REG_CONTROL1, value);
-	if (err < 0)
-		return err;
 
 	return err;
 }
@@ -347,9 +358,10 @@ static int pcf8523_probe(struct i2c_client *client,
 	if (!pcf)
 		return -ENOMEM;
 
-	err = pcf8523_select_capacitance(client, true);
+	err = pcf8523_load_capacitance(client);
 	if (err < 0)
-		return err;
+		dev_warn(&client->dev, "failed to set xtal load capacitance: %d",
+			 err);
 
 	err = pcf8523_set_pm(client, 0);
 	if (err < 0)
-- 
2.12.0

[PATCH v3 6/6] rtc: pcf85063: set xtal load capacitance from DT

[Sam Ravnborg]

Add support for specifying the xtal load capacitance in the DT node.
The pcf85063 supports xtal load capacitance of 7pF or 12.5pF.
If the rtc has the wrong configuration the time will
drift several hours/week.

The driver use the default value 7pF.

The DT may specify either 7000fF or 12500fF.
(The DT uses femto Farad to avoid decimal numbers).
Other values are warned and the driver uses the default value.

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Urs Fässler <urs.fassler@bbv.ch>
---
 drivers/rtc/rtc-pcf85063.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/drivers/rtc/rtc-pcf85063.c b/drivers/rtc/rtc-pcf85063.c
index 283c2335b01b..b79af6e96292 100644
--- a/drivers/rtc/rtc-pcf85063.c
+++ b/drivers/rtc/rtc-pcf85063.c
@@ -27,6 +27,7 @@
 */
 
 #define PCF85063_REG_CTRL1		0x00 /* status */
+#define PCF85063_REG_CTRL1_CAP_SEL	BIT(0)
 #define PCF85063_REG_CTRL1_STOP		BIT(5)
 #define PCF85063_REG_CTRL2		0x01
 
@@ -180,6 +181,39 @@ static const struct rtc_class_ops pcf85063_rtc_ops = {
 	.set_time	= pcf85063_rtc_set_time
 };
 
+static int pcf85063_load_capacitance(struct i2c_client *client)
+{
+	u32 load;
+	int rc;
+	u8 reg;
+
+	rc = i2c_smbus_read_byte_data(client, PCF85063_REG_CTRL1);
+	if (rc < 0)
+		return rc;
+
+	reg = rc;
+	load = 7000;
+	of_property_read_u32(client->dev.of_node, "quartz-load-femtofarads",
+			     &load);
+
+	switch (load) {
+	default:
+		dev_warn(&client->dev, "Unknown quartz-load-femtofarads value: %d. Assuming 7000",
+			 load);
+		/* fall through */
+	case 7000:
+		reg &= ~PCF85063_REG_CTRL1_CAP_SEL;
+		break;
+	case 12500:
+		reg |= PCF85063_REG_CTRL1_CAP_SEL;
+		break;
+	}
+
+	rc = i2c_smbus_write_byte_data(client, PCF85063_REG_CTRL1, reg);
+
+	return rc;
+}
+
 static int pcf85063_probe(struct i2c_client *client,
 				const struct i2c_device_id *id)
 {
@@ -197,6 +231,11 @@ static int pcf85063_probe(struct i2c_client *client,
 		return err;
 	}
 
+	err = pcf85063_load_capacitance(client);
+	if (err < 0)
+		dev_warn(&client->dev, "failed to set xtal load capacitance: %d",
+			 err);
+
 	rtc = devm_rtc_device_register(&client->dev,
 				       pcf85063_driver.driver.name,
 				       &pcf85063_rtc_ops, THIS_MODULE);
-- 
2.12.0

Re: [PATCH v3 2/6] dt-bindings: rtc: Add quartz-load-femtofarads property

[Rob Herring]

On Sat, 19 Jan 2019 10:00:27 +0100, Sam Ravnborg wrote:
> The quartz-load-femtofarads are relevant for
> several users. Add it as a common property in rtc.
> Note that valid values and default values must be documented.
> 
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> Suggested-by: Rob Herring <robh+dt@kernel.org>
> Cc: Alessandro Zummo <a.zummo@towertech.it>
> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> ---
>  Documentation/devicetree/bindings/rtc/rtc.txt | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 

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

Re: [PATCH v3 4/6] dt-binding: pcf85063: add xtal load capacitance

[Rob Herring]

On Sat, 19 Jan 2019 10:00:29 +0100, Sam Ravnborg wrote:
> The NXP pcf85063 supports two different xtal load capacitance
> - 7000fF  (7pF)    HW default, Linux driver default
> - 12500fF (12.5pF) Minimum power consumption
> 
> To obtain a precise RTC the pcf85063 must be configured
> with the correct capacitance load of the xtal.
> 
> Add a property to specify the xtal capacitance load.
> The default value matches that of the current Linux driver.
> 
> With a dedicated binding remove the entry in rtc.txt
> 
> Signed-off-by: Søren Andersen <san@skov.dk>
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> Cc: Alessandro Zummo <a.zummo@towertech.it>
> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Urs Fässler <urs.fassler@bbv.ch>
> ---
>  Documentation/devicetree/bindings/rtc/nxp,pcf85063.txt | 18 ++++++++++++++++++
>  Documentation/devicetree/bindings/rtc/rtc.txt          |  1 -
>  2 files changed, 18 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/rtc/nxp,pcf85063.txt
> 

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

Re: [PATCH v3 0/6] add quartz load support to NXP rtc drivers

[Alexandre Belloni]

On 19/01/2019 10:00:25+0100, Sam Ravnborg wrote:
> Respin of patchset on top of v5.0-rc2 with feedback addressed (thanks!)
> 
> Introduce a new generic property "quartz-load-femtofarad"
> to specify the quartz load.
> The default value is selected to match the current Linux
> drivers, so there are no behavior changes if a binding do not
> specify the quarts-load.
> The unit is femtofarads because several RTC's define the quarts load
> in steps of 0.5 pF.
> 
> The two drivers rtc-pcf8523 and rtc-pcf85063 are both
> updated to use the new binding.
> Previously pcf8523 hardcoded the quartz load to 12.5pF, and
> pcf85063 used the reset default of 7pF.
> With this patchset it can now be specified in the DT,
> if the defaults are not OK.
> 
> The patch-set is tested on a proprietary board for both
> RTC variants (v2 variant only as there was only trivial changes in v3).
> 
> 	Sam
> 
> v3:
> - Document generic property "quartz-load-femtofarads" in rtc.txt (Rob)
> - Fix indent several places - ups (Alexandre)
> - Use the simpler name "rtc" in DT examples (Rob)
> - Collected "Reviewed by's"
> 
> v2:
> - Introduce a generic property "quartz-load-femtofarads"
> - Add femtofarads to property-units.txt
> - Make the changes backward compatible
> - Reduced logging
> - Warn, when we continue with a default value
> - Tested, by Søren Andersen, on real HW
> - Rebased on top of v5.0-rc1
> 
>         Sam
> 
> Sam Ravnborg (6):
>       devicetree: property-units: Add femtofarads unit
>       dt-bindings: rtc: Add quartz-load-femtofarads property
>       dt-binding: pcf8523: add xtal load capacitance
>       dt-binding: pcf85063: add xtal load capacitance
>       rtc: pcf8523: set xtal load capacitance from DT
>       rtc: pcf85063: set xtal load capacitance from DT
> 

All applied, thanks.

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