[PATCH v2 0/5] add quartz load support to NXP rtc drivers

[PATCH v2 0/5] add quartz load support to NXP rtc drivers

[Sam Ravnborg]

pcf8523 hardcode the quartz load to 12.5pF
pcf85063 uses the reset default of 7pF

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.
The fix is tested on a proprietary board for both
RTC variants.

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 (5):
      devicetree: property-units: Add femtofarads unit
      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      |  2 --
 drivers/rtc/rtc-pcf85063.c                         | 39 ++++++++++++++++++++++
 drivers/rtc/rtc-pcf8523.c                          | 28 +++++++++++-----
 6 files changed, 96 insertions(+), 10 deletions(-)

[PATCH v2 1/5] 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>
Cc: 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

Re: [PATCH v2 1/5] devicetree: property-units: Add femtofarads unit

[Rob Herring]

On Tue,  8 Jan 2019 19:54:10 +0100, Sam Ravnborg wrote:
> 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>
> Cc: 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(+)
> 

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

[PATCH v2 2/5] 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>
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/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..214ac5d26df7
--- /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: pcf8523@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 7c8da6926095..e07b15d151ac 100644
--- a/Documentation/devicetree/bindings/rtc/rtc.txt
+++ b/Documentation/devicetree/bindings/rtc/rtc.txt
@@ -51,7 +51,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

Re: [PATCH v2 2/5] dt-binding: pcf8523: add xtal load capacitance

[Rob Herring]

On Tue,  8 Jan 2019 19:54:11 +0100, Sam Ravnborg wrote:
> 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>
> 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/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
> 

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

[PATCH v2 3/5] 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..ad5e8eaa370e
--- /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: pcf85063@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 e07b15d151ac..efac6dc5b914 100644
--- a/Documentation/devicetree/bindings/rtc/rtc.txt
+++ b/Documentation/devicetree/bindings/rtc/rtc.txt
@@ -52,7 +52,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

Re: [PATCH v2 3/5] dt-binding: pcf85063: add xtal load capacitance

[Rob Herring]

On Tue, Jan 08, 2019 at 07:54:12PM +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
> 
> diff --git a/Documentation/devicetree/bindings/rtc/nxp,pcf85063.txt b/Documentation/devicetree/bindings/rtc/nxp,pcf85063.txt
> new file mode 100644
> index 000000000000..ad5e8eaa370e
> --- /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),

Probably should put this in rtc.txt so it can be shared.

> +  expressed in femto Farad (fF). Valid values are 7000 and 12500.
> +  Default value (if no value is specified) is 7000fF.
> +
> +Example:
> +
> +pcf85063: pcf85063@51 {

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 e07b15d151ac..efac6dc5b914 100644
> --- a/Documentation/devicetree/bindings/rtc/rtc.txt
> +++ b/Documentation/devicetree/bindings/rtc/rtc.txt
> @@ -52,7 +52,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
> 

Re: [PATCH v2 3/5] dt-binding: pcf85063: add xtal load capacitance

[Sam Ravnborg]

Hi Rob.

> > +
> > +Optional property:
> > +- quartz-load-femtofarads: The capacitive load of the quartz(x-tal),
> 
> Probably should put this in rtc.txt so it can be shared.

Good point - will do and respin the patchset.

> 
> > +  expressed in femto Farad (fF). Valid values are 7000 and 12500.
> > +  Default value (if no value is specified) is 7000fF.
> > +
> > +Example:
> > +
> > +pcf85063: pcf85063@51 {
> 
> rtc@51

OK, will change.

	Sam

[PATCH v2 4/5] 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..dbbc00b53b50 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

Re: [PATCH v2 4/5] rtc: pcf8523: set xtal load capacitance from DT

[Alexandre Belloni]

On 08/01/2019 19:54:13+0100, Sam Ravnborg wrote:
> 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..dbbc00b53b50 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);

This alignment is not correct, as you will be respinning for the DT doc,
please fix ;)

> +		/* 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);
Ditto

>  
>  	err = pcf8523_set_pm(client, 0);
>  	if (err < 0)
> -- 
> 2.12.0
> 

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

[PATCH v2 5/5] 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..0c0b6030627e 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 v2 5/5] rtc: pcf85063: set xtal load capacitance from DT

[Alexandre Belloni]

On 08/01/2019 19:54:14+0100, Sam Ravnborg wrote:
> 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..0c0b6030627e 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);

Alignment is not correct

> +		/* 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);

This should be using more tabs.

As you can see, I don't have much to add and I'm quite pleased with the
series.

As Rob pointed out, quartz-load-femtofarads should be added in rtc.txt

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