[rtc-linux] [PATCH 0/2] drivers: rtc: rtc-tps65910: Add RTC calibration support

[rtc-linux] [PATCH 0/2] drivers: rtc: rtc-tps65910: Add RTC calibration support

[Vesa Jääskeläinen]

Texas Instrument's TPS65910 has support for compensating RTC crystal
inaccuracies. When enabled every hour RTC counter value will be compensated
with two's complement value.

Vesa J=C3=A4=C3=A4skel=C3=A4inen (2):
  drivers: rtc: rtc-tps65910: Add RTC calibration support
  drivers: rtc: rtc-tps65910: use 'unsigned int' instead of 'unsigned'
    in arguments

 .../ABI/testing/sysfs-bus-i2c-devices-tps65910-rtc |   9 ++
 MAINTAINERS                                        |   1 +
 drivers/rtc/rtc-tps65910.c                         | 123 +++++++++++++++++=
+++-
 include/linux/mfd/tps65910.h                       |   1 +
 4 files changed, 133 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-i2c-devices-tps6591=
0-rtc

--=20
2.1.4

--=20
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---=20
You received this message because you are subscribed to the Google Groups "=
rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an e=
mail to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[rtc-linux] [PATCH 1/2] drivers: rtc: rtc-tps65910: Add RTC calibration support

[Vesa Jääskeläinen]

Texas Instrument's TPS65910 has support for compensating RTC crystal
inaccuracies. When enabled every hour RTC counter value will be compensated
with two's complement value.

Signed-off-by: Vesa J=C3=A4=C3=A4skel=C3=A4inen <vesa.jaaskelainen@vaisala.=
com>
---
 .../ABI/testing/sysfs-bus-i2c-devices-tps65910-rtc |   9 ++
 MAINTAINERS                                        |   1 +
 drivers/rtc/rtc-tps65910.c                         | 120 +++++++++++++++++=
++++
 include/linux/mfd/tps65910.h                       |   1 +
 4 files changed, 131 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-i2c-devices-tps6591=
0-rtc

diff --git a/Documentation/ABI/testing/sysfs-bus-i2c-devices-tps65910-rtc b=
/Documentation/ABI/testing/sysfs-bus-i2c-devices-tps65910-rtc
new file mode 100644
index 0000000..fb97544
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-i2c-devices-tps65910-rtc
@@ -0,0 +1,9 @@
+What:           Attribute for calibrating TPS65910's Real Time Clock
+Date:           Nov 2016
+KernelVersion:  4.10
+Contact:        rtc-linux@googlegroups.com
+Description:    The rtc_calibration attribute allows userspace to calibrat=
e the
+                Real Time Clock found in Texas Instruments TPS65910. Every=
 hour
+                the TPS65910 will correct the RTC's value by adding to it =
the
+                value of this attribute. The range of the attribute is -32=
768
+                to 32766 in units of 30.5 micro-seconds/hour.
diff --git a/MAINTAINERS b/MAINTAINERS
index 851b89b..bb2f094 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10146,6 +10146,7 @@ L:	rtc-linux@googlegroups.com
 Q:	http://patchwork.ozlabs.org/project/rtc-linux/list/
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git
 S:	Maintained
+F:	Documentation/ABI/testing/sysfs-bus-i2c-devices-tps65910-rtc
 F:	Documentation/devicetree/bindings/rtc/
 F:	Documentation/rtc.txt
 F:	drivers/rtc/
diff --git a/drivers/rtc/rtc-tps65910.c b/drivers/rtc/rtc-tps65910.c
index 5a3d53c..e1b0b0c 100644
--- a/drivers/rtc/rtc-tps65910.c
+++ b/drivers/rtc/rtc-tps65910.c
@@ -33,6 +33,9 @@ struct tps65910_rtc {
 /* Total number of RTC registers needed to set time*/
 #define NUM_TIME_REGS	(TPS65910_YEARS - TPS65910_SECONDS + 1)
=20
+/* Total number of RTC registers needed to set compensation registers */
+#define NUM_COMP_REGS	(TPS65910_RTC_COMP_MSB - TPS65910_RTC_COMP_LSB + 1)
+
 static int tps65910_rtc_alarm_irq_enable(struct device *dev, unsigned enab=
led)
 {
 	struct tps65910 *tps =3D dev_get_drvdata(dev->parent);
@@ -187,6 +190,115 @@ static int tps65910_rtc_set_alarm(struct device *dev,=
 struct rtc_wkalrm *alm)
 	return ret;
 }
=20
+static int tps65910_rtc_set_calibration(struct device *dev, int calibratio=
n)
+{
+	unsigned char comp_data[NUM_COMP_REGS];
+	struct tps65910 *tps =3D dev_get_drvdata(dev->parent);
+	s16 value;
+	int ret;
+
+	/*
+	 * TPS65910 uses two's complement 16 bit value for compensation for RTC
+	 * crystal inaccuracies. One time every hour when seconds counter
+	 * increments from 0 to 1 compensation value will be added to internal
+	 * RTC counter value.
+	 *
+	 * Compensation value 0x7FFF is prohibited value.
+	 *
+	 * Valid range for compensation value: [-32768 .. 32766]
+	 */
+	if ((calibration < -32768) || (calibration > 32766)) {
+		dev_err(dev, "RTC calibration value out of range: %d\n",
+			calibration);
+		return -EINVAL;
+	}
+
+	value =3D (s16)calibration;
+
+	comp_data[0] =3D (u16)value & 0xFF;
+	comp_data[1] =3D ((u16)value >> 8) & 0xFF;
+
+	/* Update all the compensation registers in one shot */
+	ret =3D regmap_bulk_write(tps->regmap, TPS65910_RTC_COMP_LSB,
+		comp_data, NUM_COMP_REGS);
+	if (ret < 0) {
+		dev_err(dev, "rtc_set_calibration error: %d\n", ret);
+		return ret;
+	}
+
+	/* Enable automatic compensation */
+	ret =3D regmap_update_bits(tps->regmap, TPS65910_RTC_CTRL,
+		TPS65910_RTC_CTRL_AUTO_COMP, TPS65910_RTC_CTRL_AUTO_COMP);
+	if (ret < 0)
+		dev_err(dev, "auto_comp enable failed with error: %d\n", ret);
+
+	return ret;
+}
+
+static int tps65910_rtc_get_calibration(struct device *dev, int *calibrati=
on)
+{
+	unsigned char comp_data[NUM_COMP_REGS];
+	struct tps65910 *tps =3D dev_get_drvdata(dev->parent);
+	u16 value;
+	int ret;
+
+	ret =3D regmap_bulk_read(tps->regmap, TPS65910_RTC_COMP_LSB, comp_data,
+		NUM_COMP_REGS);
+	if (ret < 0) {
+		dev_err(dev, "rtc_get_calibration error: %d\n", ret);
+		return ret;
+	}
+
+	value =3D (u16)comp_data[0] | ((u16)comp_data[1] << 8);
+
+	*calibration =3D (s16)value;
+
+	return 0;
+}
+
+static ssize_t tps65910_sysfs_store_rtc_calibration(struct device *dev,
+				struct device_attribute *attr,
+				const char *buf, size_t count)
+{
+	int calibration;
+	int ret;
+
+	ret =3D kstrtoint(buf, 0, &calibration);
+	if (ret < 0)
+		return ret;
+
+	ret =3D tps65910_rtc_set_calibration(dev, calibration);
+
+	return (ret < 0) ? ret : count;
+}
+
+static ssize_t tps65910_sysfs_show_rtc_calibration(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	int calibration;
+	int ret;
+
+	ret =3D tps65910_rtc_get_calibration(dev, &calibration);
+	if (ret < 0)
+		return ret;
+
+	return sprintf(buf, "%d\n", calibration);
+}
+
+static DEVICE_ATTR(rtc_calibration, 0644,
+		   tps65910_sysfs_show_rtc_calibration,
+		   tps65910_sysfs_store_rtc_calibration);
+
+static int tps65910_sysfs_rtc_register(struct device *dev)
+{
+	return device_create_file(dev, &dev_attr_rtc_calibration);
+}
+
+static void tps65910_sysfs_rtc_unregister(struct device *dev)
+{
+	device_remove_file(dev, &dev_attr_rtc_calibration);
+}
+
 static irqreturn_t tps65910_rtc_interrupt(int irq, void *rtc)
 {
 	struct device *dev =3D rtc;
@@ -285,6 +397,12 @@ static int tps65910_rtc_probe(struct platform_device *=
pdev)
 		return ret;
 	}
=20
+	ret =3D tps65910_sysfs_rtc_register(&pdev->dev);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Failed to register to sysfs\n");
+		return ret;
+	}
+
 	return 0;
 }
=20
@@ -294,6 +412,8 @@ static int tps65910_rtc_probe(struct platform_device *p=
dev)
  */
 static int tps65910_rtc_remove(struct platform_device *pdev)
 {
+	tps65910_sysfs_rtc_unregister(&pdev->dev);
+
 	tps65910_rtc_alarm_irq_enable(&pdev->dev, 0);
=20
 	return 0;
diff --git a/include/linux/mfd/tps65910.h b/include/linux/mfd/tps65910.h
index 6483a6f..ffb21e7 100644
--- a/include/linux/mfd/tps65910.h
+++ b/include/linux/mfd/tps65910.h
@@ -134,6 +134,7 @@
=20
 /* RTC_CTRL_REG bitfields */
 #define TPS65910_RTC_CTRL_STOP_RTC			0x01 /*0=3Dstop, 1=3Drun */
+#define TPS65910_RTC_CTRL_AUTO_COMP			0x04
 #define TPS65910_RTC_CTRL_GET_TIME			0x40
=20
 /* RTC_STATUS_REG bitfields */
--=20
2.1.4

--=20
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---=20
You received this message because you are subscribed to the Google Groups "=
rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an e=
mail to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[rtc-linux] [PATCH 2/2] drivers: rtc: rtc-tps65910: use 'unsigned int' instead of 'unsigned' in arguments

[Vesa Jääskeläinen]

Signed-off-by: Vesa J=C3=A4=C3=A4skel=C3=A4inen <vesa.jaaskelainen@vaisala.=
com>
---
 drivers/rtc/rtc-tps65910.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-tps65910.c b/drivers/rtc/rtc-tps65910.c
index e1b0b0c..d06c0f7 100644
--- a/drivers/rtc/rtc-tps65910.c
+++ b/drivers/rtc/rtc-tps65910.c
@@ -36,7 +36,8 @@ struct tps65910_rtc {
 /* Total number of RTC registers needed to set compensation registers */
 #define NUM_COMP_REGS	(TPS65910_RTC_COMP_MSB - TPS65910_RTC_COMP_LSB + 1)
=20
-static int tps65910_rtc_alarm_irq_enable(struct device *dev, unsigned enab=
led)
+static int tps65910_rtc_alarm_irq_enable(struct device *dev,
+					 unsigned int enabled)
 {
 	struct tps65910 *tps =3D dev_get_drvdata(dev->parent);
 	u8 val =3D 0;
--=20
2.1.4

--=20
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---=20
You received this message because you are subscribed to the Google Groups "=
rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an e=
mail to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[rtc-linux] Re: [PATCH 1/2] drivers: rtc: rtc-tps65910: Add RTC calibration support

[Alexandre Belloni]

Hi,

On 25/11/2016 at 11:52:17 +0200, Vesa J=C3=A4=C3=A4skel=C3=A4inen wrote :
> Texas Instrument's TPS65910 has support for compensating RTC crystal
> inaccuracies. When enabled every hour RTC counter value will be compensat=
ed
> with two's complement value.
>=20
> Signed-off-by: Vesa J=C3=A4=C3=A4skel=C3=A4inen <vesa.jaaskelainen@vaisal=
a.com>

There is already a sysfs attribute for that kind of calibration
(offset, documented in Documentation/rtc.txt). Can you use that instead?

> ---
>  .../ABI/testing/sysfs-bus-i2c-devices-tps65910-rtc |   9 ++
>  MAINTAINERS                                        |   1 +
>  drivers/rtc/rtc-tps65910.c                         | 120 +++++++++++++++=
++++++
>  include/linux/mfd/tps65910.h                       |   1 +
>  4 files changed, 131 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-i2c-devices-tps65=
910-rtc
>=20
> diff --git a/Documentation/ABI/testing/sysfs-bus-i2c-devices-tps65910-rtc=
 b/Documentation/ABI/testing/sysfs-bus-i2c-devices-tps65910-rtc
> new file mode 100644
> index 0000000..fb97544
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-i2c-devices-tps65910-rtc
> @@ -0,0 +1,9 @@
> +What:           Attribute for calibrating TPS65910's Real Time Clock
> +Date:           Nov 2016
> +KernelVersion:  4.10
> +Contact:        rtc-linux@googlegroups.com
> +Description:    The rtc_calibration attribute allows userspace to calibr=
ate the
> +                Real Time Clock found in Texas Instruments TPS65910. Eve=
ry hour
> +                the TPS65910 will correct the RTC's value by adding to i=
t the
> +                value of this attribute. The range of the attribute is -=
32768
> +                to 32766 in units of 30.5 micro-seconds/hour.
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 851b89b..bb2f094 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10146,6 +10146,7 @@ L:	rtc-linux@googlegroups.com
>  Q:	http://patchwork.ozlabs.org/project/rtc-linux/list/
>  T:	git git://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git
>  S:	Maintained
> +F:	Documentation/ABI/testing/sysfs-bus-i2c-devices-tps65910-rtc
>  F:	Documentation/devicetree/bindings/rtc/
>  F:	Documentation/rtc.txt
>  F:	drivers/rtc/
> diff --git a/drivers/rtc/rtc-tps65910.c b/drivers/rtc/rtc-tps65910.c
> index 5a3d53c..e1b0b0c 100644
> --- a/drivers/rtc/rtc-tps65910.c
> +++ b/drivers/rtc/rtc-tps65910.c
> @@ -33,6 +33,9 @@ struct tps65910_rtc {
>  /* Total number of RTC registers needed to set time*/
>  #define NUM_TIME_REGS	(TPS65910_YEARS - TPS65910_SECONDS + 1)
> =20
> +/* Total number of RTC registers needed to set compensation registers */
> +#define NUM_COMP_REGS	(TPS65910_RTC_COMP_MSB - TPS65910_RTC_COMP_LSB + 1=
)
> +
>  static int tps65910_rtc_alarm_irq_enable(struct device *dev, unsigned en=
abled)
>  {
>  	struct tps65910 *tps =3D dev_get_drvdata(dev->parent);
> @@ -187,6 +190,115 @@ static int tps65910_rtc_set_alarm(struct device *de=
v, struct rtc_wkalrm *alm)
>  	return ret;
>  }
> =20
> +static int tps65910_rtc_set_calibration(struct device *dev, int calibrat=
ion)
> +{
> +	unsigned char comp_data[NUM_COMP_REGS];
> +	struct tps65910 *tps =3D dev_get_drvdata(dev->parent);
> +	s16 value;
> +	int ret;
> +
> +	/*
> +	 * TPS65910 uses two's complement 16 bit value for compensation for RTC
> +	 * crystal inaccuracies. One time every hour when seconds counter
> +	 * increments from 0 to 1 compensation value will be added to internal
> +	 * RTC counter value.
> +	 *
> +	 * Compensation value 0x7FFF is prohibited value.
> +	 *
> +	 * Valid range for compensation value: [-32768 .. 32766]
> +	 */
> +	if ((calibration < -32768) || (calibration > 32766)) {
> +		dev_err(dev, "RTC calibration value out of range: %d\n",
> +			calibration);
> +		return -EINVAL;
> +	}
> +
> +	value =3D (s16)calibration;
> +
> +	comp_data[0] =3D (u16)value & 0xFF;
> +	comp_data[1] =3D ((u16)value >> 8) & 0xFF;
> +
> +	/* Update all the compensation registers in one shot */
> +	ret =3D regmap_bulk_write(tps->regmap, TPS65910_RTC_COMP_LSB,
> +		comp_data, NUM_COMP_REGS);
> +	if (ret < 0) {
> +		dev_err(dev, "rtc_set_calibration error: %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* Enable automatic compensation */
> +	ret =3D regmap_update_bits(tps->regmap, TPS65910_RTC_CTRL,
> +		TPS65910_RTC_CTRL_AUTO_COMP, TPS65910_RTC_CTRL_AUTO_COMP);
> +	if (ret < 0)
> +		dev_err(dev, "auto_comp enable failed with error: %d\n", ret);
> +
> +	return ret;
> +}
> +
> +static int tps65910_rtc_get_calibration(struct device *dev, int *calibra=
tion)
> +{
> +	unsigned char comp_data[NUM_COMP_REGS];
> +	struct tps65910 *tps =3D dev_get_drvdata(dev->parent);
> +	u16 value;
> +	int ret;
> +
> +	ret =3D regmap_bulk_read(tps->regmap, TPS65910_RTC_COMP_LSB, comp_data,
> +		NUM_COMP_REGS);
> +	if (ret < 0) {
> +		dev_err(dev, "rtc_get_calibration error: %d\n", ret);
> +		return ret;
> +	}
> +
> +	value =3D (u16)comp_data[0] | ((u16)comp_data[1] << 8);
> +
> +	*calibration =3D (s16)value;
> +
> +	return 0;
> +}
> +
> +static ssize_t tps65910_sysfs_store_rtc_calibration(struct device *dev,
> +				struct device_attribute *attr,
> +				const char *buf, size_t count)
> +{
> +	int calibration;
> +	int ret;
> +
> +	ret =3D kstrtoint(buf, 0, &calibration);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret =3D tps65910_rtc_set_calibration(dev, calibration);
> +
> +	return (ret < 0) ? ret : count;
> +}
> +
> +static ssize_t tps65910_sysfs_show_rtc_calibration(struct device *dev,
> +				struct device_attribute *attr, char *buf)
> +{
> +	int calibration;
> +	int ret;
> +
> +	ret =3D tps65910_rtc_get_calibration(dev, &calibration);
> +	if (ret < 0)
> +		return ret;
> +
> +	return sprintf(buf, "%d\n", calibration);
> +}
> +
> +static DEVICE_ATTR(rtc_calibration, 0644,
> +		   tps65910_sysfs_show_rtc_calibration,
> +		   tps65910_sysfs_store_rtc_calibration);
> +
> +static int tps65910_sysfs_rtc_register(struct device *dev)
> +{
> +	return device_create_file(dev, &dev_attr_rtc_calibration);
> +}
> +
> +static void tps65910_sysfs_rtc_unregister(struct device *dev)
> +{
> +	device_remove_file(dev, &dev_attr_rtc_calibration);
> +}
> +
>  static irqreturn_t tps65910_rtc_interrupt(int irq, void *rtc)
>  {
>  	struct device *dev =3D rtc;
> @@ -285,6 +397,12 @@ static int tps65910_rtc_probe(struct platform_device=
 *pdev)
>  		return ret;
>  	}
> =20
> +	ret =3D tps65910_sysfs_rtc_register(&pdev->dev);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "Failed to register to sysfs\n");
> +		return ret;
> +	}
> +
>  	return 0;
>  }
> =20
> @@ -294,6 +412,8 @@ static int tps65910_rtc_probe(struct platform_device =
*pdev)
>   */
>  static int tps65910_rtc_remove(struct platform_device *pdev)
>  {
> +	tps65910_sysfs_rtc_unregister(&pdev->dev);
> +
>  	tps65910_rtc_alarm_irq_enable(&pdev->dev, 0);
> =20
>  	return 0;
> diff --git a/include/linux/mfd/tps65910.h b/include/linux/mfd/tps65910.h
> index 6483a6f..ffb21e7 100644
> --- a/include/linux/mfd/tps65910.h
> +++ b/include/linux/mfd/tps65910.h
> @@ -134,6 +134,7 @@
> =20
>  /* RTC_CTRL_REG bitfields */
>  #define TPS65910_RTC_CTRL_STOP_RTC			0x01 /*0=3Dstop, 1=3Drun */
> +#define TPS65910_RTC_CTRL_AUTO_COMP			0x04
>  #define TPS65910_RTC_CTRL_GET_TIME			0x40
> =20
>  /* RTC_STATUS_REG bitfields */
> --=20
> 2.1.4
>=20

--=20
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

--=20
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---=20
You received this message because you are subscribed to the Google Groups "=
rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an e=
mail to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[rtc-linux] Re: [PATCH 1/2] drivers: rtc: rtc-tps65910: Add RTC calibration support

[Vesa Jääskeläinen]

Hi,

On 2016-11-25 12:32, Alexandre Belloni wrote:
> Hi,
>
> On 25/11/2016 at 11:52:17 +0200, Vesa J=C3=A4=C3=A4skel=C3=A4inen wrote :
>> Texas Instrument's TPS65910 has support for compensating RTC crystal
>> inaccuracies. When enabled every hour RTC counter value will be compensa=
ted
>> with two's complement value.
>>
>> Signed-off-by: Vesa J=C3=A4=C3=A4skel=C3=A4inen <vesa.jaaskelainen@vaisa=
la.com>
> There is already a sysfs attribute for that kind of calibration
> (offset, documented in Documentation/rtc.txt). Can you use that instead?

That documentation in rtc.txt states that compensation is per billion=20
ticks. This compensation happens every 32768*60*60  =3D=3D 117964800 ticks.

I modeled this after similar functionality of rtc-ab8500.c which=20
provided similar sysfs file. But its time compensation happens also with=20
different interval.

Due to differences here in functionalities between different chips I=20
decided to go with driver specific compensation value.

Thanks,
Vesa J=C3=A4=C3=A4skel=C3=A4inen

--=20
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---=20
You received this message because you are subscribed to the Google Groups "=
rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an e=
mail to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[rtc-linux] Re: [PATCH 1/2] drivers: rtc: rtc-tps65910: Add RTC calibration support

[Alexandre Belloni]

On 25/11/2016 at 13:20:05 +0200, Vesa J=C3=A4=C3=A4skel=C3=A4inen wrote :
> On 2016-11-25 12:32, Alexandre Belloni wrote:
> > Hi,
> >=20
> > On 25/11/2016 at 11:52:17 +0200, Vesa J=C3=A4=C3=A4skel=C3=A4inen wrote=
 :
> > > Texas Instrument's TPS65910 has support for compensating RTC crystal
> > > inaccuracies. When enabled every hour RTC counter value will be compe=
nsated
> > > with two's complement value.
> > >=20
> > > Signed-off-by: Vesa J=C3=A4=C3=A4skel=C3=A4inen <vesa.jaaskelainen@va=
isala.com>
> > There is already a sysfs attribute for that kind of calibration
> > (offset, documented in Documentation/rtc.txt). Can you use that instead=
?
>=20
> That documentation in rtc.txt states that compensation is per billion tic=
ks.
> This compensation happens every 32768*60*60  =3D=3D 117964800 ticks.
>=20
> I modeled this after similar functionality of rtc-ab8500.c which provided
> similar sysfs file. But its time compensation happens also with different
> interval.
>=20
> Due to differences here in functionalities between different chips I deci=
ded
> to go with driver specific compensation value.
>=20

The functionality is exactly the same, but if I understand correctly,
your resolution is 8.477 ppb is that right ?

--=20
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

--=20
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---=20
You received this message because you are subscribed to the Google Groups "=
rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an e=
mail to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[rtc-linux] Re: [PATCH 1/2] drivers: rtc: rtc-tps65910: Add RTC calibration support

[Vesa Jääskeläinen]

On 2016-11-25 17:37, Alexandre Belloni wrote:
> On 25/11/2016 at 13:20:05 +0200, Vesa J=C3=A4=C3=A4skel=C3=A4inen wrote :
>> On 2016-11-25 12:32, Alexandre Belloni wrote:
>>> Hi,
>>>
>>> On 25/11/2016 at 11:52:17 +0200, Vesa J=C3=A4=C3=A4skel=C3=A4inen wrote=
 :
>>>> Texas Instrument's TPS65910 has support for compensating RTC crystal
>>>> inaccuracies. When enabled every hour RTC counter value will be compen=
sated
>>>> with two's complement value.
>>>>
>>>> Signed-off-by: Vesa J=C3=A4=C3=A4skel=C3=A4inen <vesa.jaaskelainen@vai=
sala.com>
>>> There is already a sysfs attribute for that kind of calibration
>>> (offset, documented in Documentation/rtc.txt). Can you use that instead=
?
>> That documentation in rtc.txt states that compensation is per billion ti=
cks.
>> This compensation happens every 32768*60*60  =3D=3D 117964800 ticks.
>>
>> I modeled this after similar functionality of rtc-ab8500.c which provide=
d
>> similar sysfs file. But its time compensation happens also with differen=
t
>> interval.
>>
>> Due to differences here in functionalities between different chips I dec=
ided
>> to go with driver specific compensation value.
>>
> The functionality is exactly the same, but if I understand correctly,
> your resolution is 8.477 ppb is that right ?

I suppose so. Compensation resolution is the differenting factor here=20
and also at what point of time compensation is happening.

If I would convert this to 1ppb then there would be theoretical accuracy=20
lost, not much but in year it sums up :).

btw. is there existing RTC compensation daemon that would support also=20
temperature based calibration and would internally handle updating=20
'offset' or custom calibration fields?

If I spotted correctly this 'offset' property is only supported by=20
rtc-pcf2123.c. That driver seems to use some kind of dynamic scaling for=20
compensation.

Keeping data in chips native format would enable best possible accuracy=20
and no-one could complain that it is not functioning in best possible way.

Thanks,
Vesa J=C3=A4=C3=A4skel=C3=A4inen

--=20
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---=20
You received this message because you are subscribed to the Google Groups "=
rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an e=
mail to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[rtc-linux] Re: [PATCH 1/2] drivers: rtc: rtc-tps65910: Add RTC calibration support

[Alexandre Belloni]

On 28/11/2016 at 16:42:05 +0200, Vesa J=C3=A4=C3=A4skel=C3=A4inen wrote :
> On 2016-11-25 17:37, Alexandre Belloni wrote:
> > On 25/11/2016 at 13:20:05 +0200, Vesa J=C3=A4=C3=A4skel=C3=A4inen wrote=
 :
> > > On 2016-11-25 12:32, Alexandre Belloni wrote:
> > > > Hi,
> > > >=20
> > > > On 25/11/2016 at 11:52:17 +0200, Vesa J=C3=A4=C3=A4skel=C3=A4inen w=
rote :
> > > > > Texas Instrument's TPS65910 has support for compensating RTC crys=
tal
> > > > > inaccuracies. When enabled every hour RTC counter value will be c=
ompensated
> > > > > with two's complement value.
> > > > >=20
> > > > > Signed-off-by: Vesa J=C3=A4=C3=A4skel=C3=A4inen <vesa.jaaskelaine=
n@vaisala.com>
> > > > There is already a sysfs attribute for that kind of calibration
> > > > (offset, documented in Documentation/rtc.txt). Can you use that ins=
tead?
> > > That documentation in rtc.txt states that compensation is per billion=
 ticks.
> > > This compensation happens every 32768*60*60  =3D=3D 117964800 ticks.
> > >=20
> > > I modeled this after similar functionality of rtc-ab8500.c which prov=
ided
> > > similar sysfs file. But its time compensation happens also with diffe=
rent
> > > interval.
> > >=20
> > > Due to differences here in functionalities between different chips I =
decided
> > > to go with driver specific compensation value.
> > >=20
> > The functionality is exactly the same, but if I understand correctly,
> > your resolution is 8.477 ppb is that right ?
>=20
> I suppose so. Compensation resolution is the differenting factor here and
> also at what point of time compensation is happening.
>=20
> If I would convert this to 1ppb then there would be theoretical accuracy
> lost, not much but in year it sums up :).
>=20

Well, the best your RTC can do is 8.477 ppb so and the maximum drift
you'd get over a year because of this resolution is 133.5ms.
The precision loss by rounding to the closest ppb is 16ms over a year,
way below what you can achieve.

Also, remember that when setting the time, you are doing it at a one
second resolution and you may as well be at the 999th millisecond of
that second before even transferring any data to your RTC.

The unit (ppb) is fine and the magic of it is that you don't actually
care about when the correction happens as long as it happens regularly.

> btw. is there existing RTC compensation daemon that would support also
> temperature based calibration and would internally handle updating 'offse=
t'
> or custom calibration fields?
>=20

Nothing that I'm aware of

> If I spotted correctly this 'offset' property is only supported by
> rtc-pcf2123.c. That driver seems to use some kind of dynamic scaling for
> compensation.
>=20

It is the only one using it at the moment. This RTC has two ways of
specifying the compensation: fine (every 2 hours) or coarse (every
hour). This is handled automatically in the driver.

> Keeping data in chips native format would enable best possible accuracy a=
nd
> no-one could complain that it is not functioning in best possible way.
>

My point is that you don't lose any accuracy by using ppb and
converting in the driver anyway.
Using the native format will prevent anyone from writing a generic
userspace application handling that compensation.

--=20
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

--=20
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---=20
You received this message because you are subscribed to the Google Groups "=
rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an e=
mail to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[rtc-linux] Re: [PATCH 1/2] drivers: rtc: rtc-tps65910: Add RTC calibration support

[Vesa Jääskeläinen]

On 2016-12-01 02:12, Alexandre Belloni wrote:

> On 28/11/2016 at 16:42:05 +0200, Vesa J=C3=A4=C3=A4skel=C3=A4inen wrote :
>> On 2016-11-25 17:37, Alexandre Belloni wrote:
>>> On 25/11/2016 at 13:20:05 +0200, Vesa J=C3=A4=C3=A4skel=C3=A4inen wrote=
 :
>>>> On 2016-11-25 12:32, Alexandre Belloni wrote:
>>>>> Hi,
>>>>>
>>>>> On 25/11/2016 at 11:52:17 +0200, Vesa J=C3=A4=C3=A4skel=C3=A4inen wro=
te :
>>>>>> Texas Instrument's TPS65910 has support for compensating RTC crystal
>>>>>> inaccuracies. When enabled every hour RTC counter value will be comp=
ensated
>>>>>> with two's complement value.
>>>>>>
>>>>>> Signed-off-by: Vesa J=C3=A4=C3=A4skel=C3=A4inen <vesa.jaaskelainen@v=
aisala.com>
>>>>> There is already a sysfs attribute for that kind of calibration
>>>>> (offset, documented in Documentation/rtc.txt). Can you use that inste=
ad?
>>>> That documentation in rtc.txt states that compensation is per billion =
ticks.
>>>> This compensation happens every 32768*60*60  =3D=3D 117964800 ticks.
>>>>
>>>> I modeled this after similar functionality of rtc-ab8500.c which provi=
ded
>>>> similar sysfs file. But its time compensation happens also with differ=
ent
>>>> interval.
>>>>
>>>> Due to differences here in functionalities between different chips I d=
ecided
>>>> to go with driver specific compensation value.
>>>>
>>> The functionality is exactly the same, but if I understand correctly,
>>> your resolution is 8.477 ppb is that right ?
>> I suppose so. Compensation resolution is the differenting factor here an=
d
>> also at what point of time compensation is happening.
>>
>> If I would convert this to 1ppb then there would be theoretical accuracy
>> lost, not much but in year it sums up :).
>>
> Well, the best your RTC can do is 8.477 ppb so and the maximum drift
> you'd get over a year because of this resolution is 133.5ms.
> The precision loss by rounding to the closest ppb is 16ms over a year,
> way below what you can achieve.
>
> Also, remember that when setting the time, you are doing it at a one
> second resolution and you may as well be at the 999th millisecond of
> that second before even transferring any data to your RTC.
>
> The unit (ppb) is fine and the magic of it is that you don't actually
> care about when the correction happens as long as it happens regularly.
>
>> Keeping data in chips native format would enable best possible accuracy =
and
>> no-one could complain that it is not functioning in best possible way.
>>
> My point is that you don't lose any accuracy by using ppb and
> converting in the driver anyway.
> Using the native format will prevent anyone from writing a generic
> userspace application handling that compensation.

I started looking at the how it would change things but I am having a=20
bit hard time with this text:

|offset: The amount which the rtc clock has been adjusted in firmware.=20
Visible only if the driver supports clock offset adjustment. The unit is=20
parts per billion, i.e. The number of clock ticks which are added to or=20
removed from the rtc's base clock per billion ticks. A positive value=20
makes a day pass more slowly, longer, and a negative value makes a day=20
pass more quickly.|


Problem here is that I am having hard time understanding time scale=20
here. Or how it is defined in this text. Then I started to think if=20
using ppb unit then how it could be done.

If I look at in example accuracy calculator from Maxim:
https://www.maximintegrated.com/en/design/tools/calculators/product-design/=
rtc.cfm

This is kinda easy to understand; if you have error 100ppm it means that=20
this error (0.0001) happens in one second. Then you just multiply the=20
error for your time range you are interested in. Eg. 8.64s/day.

Or alternatively if you think in ticks then;

     realticks =3D ticks * (1+100ppm)
     seconds =3D realticks / expected rtc crystal

Based on commit messages I believe ppb usage instead of ppm is just for=20
giving possibility for better compensation factor in integers.

To calibrate RTC you need to measure exact crystal frequency. So lets=20
assume RTC crystal would be here 32768 Hz (which is not exported to user=20
space?). Let's assume measured crystal frequency would be 32771.28 Hz=20
(100ppm). So compensation would need to slow it down by 3.2768 Hz/s.

To calculate this compensation 'offset' value:

     offset =3D (32768 - 32771.28) / 32768 * 10^9    ; where offset is ppb=
=20
value
     offset =3D -100000    ; or -100 ppm

To compensate out this -100ppm one would need to write -100000 to=20
'offset' sysfs file.

Now assuming the error is +100 ppm and with compensation of -100 ppm=20
those would cancel each other out and we get "accurate" time.

Am I correct with this? If so then I suppose this text should be updated=20
to be clearer what it means. Problem I see is part "per billion ticks".=20
And perhaps the sign here?

|offset: The amount which the rtc clock has been adjusted in firmware.=20
Visible only if the driver supports clock offset adjustment. The unit is=20
parts per billion, i.e. The number of billionth seconds which are added=20
to or removed from the rtc's time once per second. A positive value=20
makes a day pass more quickly, faster, and a negative value makes a day=20
pass more slowly.|


In my case it would mean that every hour one would add ticks (negative=20
slows it down) by (32768 - 32771.28) * 3600 =3D=3D -11796.48.

or with 'offset' value with 64 bit signed integer math:

     comp =3D (offset * 32768 * 3600 + 10^9/2) / 10^9
     offset =3D (comp * 10^9 + (32768 * 3600)/2) / (32768 * 3600)

I will prepare next patch with this.

Thanks,
Vesa J=C3=A4=C3=A4skel=C3=A4inen

--=20
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---=20
You received this message because you are subscribed to the Google Groups "=
rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an e=
mail to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.