linux-rtc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/5] add quartz load support to NXP rtc drivers
@ 2019-01-08 18:54 Sam Ravnborg
  2019-01-08 18:54 ` [PATCH v2 1/5] devicetree: property-units: Add femtofarads unit Sam Ravnborg
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Sam Ravnborg @ 2019-01-08 18:54 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni, Andrew Jeffery,
	Fabio Estevam, Joel Stanley, Mark Rutland, Rob Herring,
	Russell King, Sascha Hauer, Urs Fässler, Shawn Guo,
	devicetree, linux-kernel, linux-rtc
  Cc: 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(-)

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 1/5] devicetree: property-units: Add femtofarads unit
  2019-01-08 18:54 [PATCH v2 0/5] add quartz load support to NXP rtc drivers Sam Ravnborg
@ 2019-01-08 18:54 ` Sam Ravnborg
  2019-01-15 21:04   ` Rob Herring
  2019-01-08 18:54 ` [PATCH v2 2/5] dt-binding: pcf8523: add xtal load capacitance Sam Ravnborg
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Sam Ravnborg @ 2019-01-08 18:54 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni, Andrew Jeffery,
	Fabio Estevam, Joel Stanley, Mark Rutland, Rob Herring,
	Russell King, Sascha Hauer, Urs Fässler, Shawn Guo,
	devicetree, linux-kernel, linux-rtc
  Cc: Sam Ravnborg, Rob Herring

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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 2/5] dt-binding: pcf8523: add xtal load capacitance
  2019-01-08 18:54 [PATCH v2 0/5] add quartz load support to NXP rtc drivers Sam Ravnborg
  2019-01-08 18:54 ` [PATCH v2 1/5] devicetree: property-units: Add femtofarads unit Sam Ravnborg
@ 2019-01-08 18:54 ` Sam Ravnborg
  2019-01-15 21:06   ` Rob Herring
  2019-01-08 18:54 ` [PATCH v2 3/5] dt-binding: pcf85063: " Sam Ravnborg
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Sam Ravnborg @ 2019-01-08 18:54 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni, Andrew Jeffery,
	Fabio Estevam, Joel Stanley, Mark Rutland, Rob Herring,
	Russell King, Sascha Hauer, Urs Fässler, Shawn Guo,
	devicetree, linux-kernel, linux-rtc
  Cc: Sam Ravnborg, Søren Andersen

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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 3/5] dt-binding: pcf85063: add xtal load capacitance
  2019-01-08 18:54 [PATCH v2 0/5] add quartz load support to NXP rtc drivers Sam Ravnborg
  2019-01-08 18:54 ` [PATCH v2 1/5] devicetree: property-units: Add femtofarads unit Sam Ravnborg
  2019-01-08 18:54 ` [PATCH v2 2/5] dt-binding: pcf8523: add xtal load capacitance Sam Ravnborg
@ 2019-01-08 18:54 ` Sam Ravnborg
  2019-01-15 21:08   ` Rob Herring
  2019-01-08 18:54 ` [PATCH v2 4/5] rtc: pcf8523: set xtal load capacitance from DT Sam Ravnborg
  2019-01-08 18:54 ` [PATCH v2 5/5] rtc: pcf85063: " Sam Ravnborg
  4 siblings, 1 reply; 12+ messages in thread
From: Sam Ravnborg @ 2019-01-08 18:54 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni, Andrew Jeffery,
	Fabio Estevam, Joel Stanley, Mark Rutland, Rob Herring,
	Russell King, Sascha Hauer, Urs Fässler, Shawn Guo,
	devicetree, linux-kernel, linux-rtc
  Cc: Sam Ravnborg, Søren Andersen

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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 4/5] rtc: pcf8523: set xtal load capacitance from DT
  2019-01-08 18:54 [PATCH v2 0/5] add quartz load support to NXP rtc drivers Sam Ravnborg
                   ` (2 preceding siblings ...)
  2019-01-08 18:54 ` [PATCH v2 3/5] dt-binding: pcf85063: " Sam Ravnborg
@ 2019-01-08 18:54 ` Sam Ravnborg
  2019-01-16 21:04   ` Alexandre Belloni
  2019-01-08 18:54 ` [PATCH v2 5/5] rtc: pcf85063: " Sam Ravnborg
  4 siblings, 1 reply; 12+ messages in thread
From: Sam Ravnborg @ 2019-01-08 18:54 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni, Andrew Jeffery,
	Fabio Estevam, Joel Stanley, Mark Rutland, Rob Herring,
	Russell King, Sascha Hauer, Urs Fässler, Shawn Guo,
	devicetree, linux-kernel, linux-rtc
  Cc: 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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 5/5] rtc: pcf85063: set xtal load capacitance from DT
  2019-01-08 18:54 [PATCH v2 0/5] add quartz load support to NXP rtc drivers Sam Ravnborg
                   ` (3 preceding siblings ...)
  2019-01-08 18:54 ` [PATCH v2 4/5] rtc: pcf8523: set xtal load capacitance from DT Sam Ravnborg
@ 2019-01-08 18:54 ` Sam Ravnborg
  2019-01-16 21:11   ` Alexandre Belloni
  4 siblings, 1 reply; 12+ messages in thread
From: Sam Ravnborg @ 2019-01-08 18:54 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni, Andrew Jeffery,
	Fabio Estevam, Joel Stanley, Mark Rutland, Rob Herring,
	Russell King, Sascha Hauer, Urs Fässler, Shawn Guo,
	devicetree, linux-kernel, linux-rtc
  Cc: 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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/5] devicetree: property-units: Add femtofarads unit
  2019-01-08 18:54 ` [PATCH v2 1/5] devicetree: property-units: Add femtofarads unit Sam Ravnborg
@ 2019-01-15 21:04   ` Rob Herring
  0 siblings, 0 replies; 12+ messages in thread
From: Rob Herring @ 2019-01-15 21:04 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: Alessandro Zummo, Alexandre Belloni, Andrew Jeffery,
	Fabio Estevam, Joel Stanley, Mark Rutland, devicetree,
	linux-kernel, linux-rtc, Sam Ravnborg

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>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/5] dt-binding: pcf8523: add xtal load capacitance
  2019-01-08 18:54 ` [PATCH v2 2/5] dt-binding: pcf8523: add xtal load capacitance Sam Ravnborg
@ 2019-01-15 21:06   ` Rob Herring
  0 siblings, 0 replies; 12+ messages in thread
From: Rob Herring @ 2019-01-15 21:06 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: Alessandro Zummo, Alexandre Belloni, Andrew Jeffery,
	Fabio Estevam, Joel Stanley, Mark Rutland, devicetree,
	linux-kernel, linux-rtc, Sam Ravnborg, Søren Andersen

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>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 3/5] dt-binding: pcf85063: add xtal load capacitance
  2019-01-08 18:54 ` [PATCH v2 3/5] dt-binding: pcf85063: " Sam Ravnborg
@ 2019-01-15 21:08   ` Rob Herring
  2019-01-15 22:22     ` Sam Ravnborg
  0 siblings, 1 reply; 12+ messages in thread
From: Rob Herring @ 2019-01-15 21:08 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: Alessandro Zummo, Alexandre Belloni, Andrew Jeffery,
	Fabio Estevam, Joel Stanley, Mark Rutland, Russell King,
	Sascha Hauer, Urs Fässler, Shawn Guo, devicetree,
	linux-kernel, linux-rtc, Søren Andersen

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
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 3/5] dt-binding: pcf85063: add xtal load capacitance
  2019-01-15 21:08   ` Rob Herring
@ 2019-01-15 22:22     ` Sam Ravnborg
  0 siblings, 0 replies; 12+ messages in thread
From: Sam Ravnborg @ 2019-01-15 22:22 UTC (permalink / raw)
  To: Rob Herring
  Cc: Alessandro Zummo, Alexandre Belloni, Andrew Jeffery,
	Fabio Estevam, Joel Stanley, Mark Rutland, Russell King,
	Sascha Hauer, Urs Fässler, Shawn Guo, devicetree,
	linux-kernel, linux-rtc, Søren Andersen

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 4/5] rtc: pcf8523: set xtal load capacitance from DT
  2019-01-08 18:54 ` [PATCH v2 4/5] rtc: pcf8523: set xtal load capacitance from DT Sam Ravnborg
@ 2019-01-16 21:04   ` Alexandre Belloni
  0 siblings, 0 replies; 12+ messages in thread
From: Alexandre Belloni @ 2019-01-16 21:04 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: Alessandro Zummo, Andrew Jeffery, Fabio Estevam, Joel Stanley,
	Mark Rutland, Rob Herring, Russell King, Sascha Hauer,
	Urs Fässler, Shawn Guo, devicetree, linux-kernel, linux-rtc

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 5/5] rtc: pcf85063: set xtal load capacitance from DT
  2019-01-08 18:54 ` [PATCH v2 5/5] rtc: pcf85063: " Sam Ravnborg
@ 2019-01-16 21:11   ` Alexandre Belloni
  0 siblings, 0 replies; 12+ messages in thread
From: Alexandre Belloni @ 2019-01-16 21:11 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: Alessandro Zummo, Andrew Jeffery, Fabio Estevam, Joel Stanley,
	Mark Rutland, Rob Herring, Russell King, Sascha Hauer,
	Urs Fässler, Shawn Guo, devicetree, linux-kernel, linux-rtc

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2019-01-16 21:11 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-08 18:54 [PATCH v2 0/5] add quartz load support to NXP rtc drivers Sam Ravnborg
2019-01-08 18:54 ` [PATCH v2 1/5] devicetree: property-units: Add femtofarads unit Sam Ravnborg
2019-01-15 21:04   ` Rob Herring
2019-01-08 18:54 ` [PATCH v2 2/5] dt-binding: pcf8523: add xtal load capacitance Sam Ravnborg
2019-01-15 21:06   ` Rob Herring
2019-01-08 18:54 ` [PATCH v2 3/5] dt-binding: pcf85063: " Sam Ravnborg
2019-01-15 21:08   ` Rob Herring
2019-01-15 22:22     ` Sam Ravnborg
2019-01-08 18:54 ` [PATCH v2 4/5] rtc: pcf8523: set xtal load capacitance from DT Sam Ravnborg
2019-01-16 21:04   ` Alexandre Belloni
2019-01-08 18:54 ` [PATCH v2 5/5] rtc: pcf85063: " Sam Ravnborg
2019-01-16 21:11   ` Alexandre Belloni

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).