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

[rtc-linux] [PATCH v2 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

 drivers/rtc/rtc-tps65910.c   | 135 +++++++++++++++++++++++++++++++++++++++=
+++-
 include/linux/mfd/tps65910.h |   1 +
 2 files changed, 135 insertions(+), 1 deletion(-)

--=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 v2 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>
---
 drivers/rtc/rtc-tps65910.c   | 132 +++++++++++++++++++++++++++++++++++++++=
++++
 include/linux/mfd/tps65910.h |   1 +
 2 files changed, 133 insertions(+)

diff --git a/drivers/rtc/rtc-tps65910.c b/drivers/rtc/rtc-tps65910.c
index 5a3d53c..aafa716 100644
--- a/drivers/rtc/rtc-tps65910.c
+++ b/drivers/rtc/rtc-tps65910.c
@@ -21,6 +21,7 @@
 #include <linux/types.h>
 #include <linux/rtc.h>
 #include <linux/bcd.h>
+#include <linux/math64.h>
 #include <linux/platform_device.h>
 #include <linux/interrupt.h>
 #include <linux/mfd/tps65910.h>
@@ -33,6 +34,19 @@ 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)
+
+/* Min and max values supported with 'offset' interface (swapped sign) */
+#define MIN_OFFSET	(-277761)
+#define MAX_OFFSET	(277778)
+
+/* Number of ticks per hour */
+#define TICKS_PER_HOUR	(32768 * 3600)
+
+/* Multiplier for ppb conversions */
+#define PPB_MULT	(1000000000LL)
+
 static int tps65910_rtc_alarm_irq_enable(struct device *dev, unsigned enab=
led)
 {
 	struct tps65910 *tps =3D dev_get_drvdata(dev->parent);
@@ -187,6 +201,122 @@ 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 int tps65910_read_offset(struct device *dev, long *offset)
+{
+	int calibration;
+	s64 tmp;
+	int ret;
+
+	ret =3D tps65910_rtc_get_calibration(dev, &calibration);
+	if (ret < 0)
+		return ret;
+
+	/* Convert from RTC calibration register format to ppb format */
+	tmp =3D calibration * (s64)PPB_MULT;
+	if (tmp < 0)
+		tmp -=3D TICKS_PER_HOUR / 2LL;
+	else
+		tmp +=3D TICKS_PER_HOUR / 2LL;
+	tmp =3D div_s64(tmp, TICKS_PER_HOUR);
+
+	/* Offset value operates in negative way, so swap sign */
+	*offset =3D (long)-tmp;
+
+	return 0;
+}
+
+static int tps65910_set_offset(struct device *dev, long offset)
+{
+	int calibration;
+	s64 tmp;
+	int ret;
+
+	/* Make sure offset value is within supported range */
+	if (offset < MIN_OFFSET || offset > MAX_OFFSET)
+		return -ERANGE;
+
+	/* Convert from ppb format to RTC calibration register format */
+	tmp =3D offset * (s64)TICKS_PER_HOUR;
+	if (tmp < 0)
+		tmp -=3D PPB_MULT / 2LL;
+	else
+		tmp +=3D PPB_MULT / 2LL;
+	tmp =3D div_s64(tmp, PPB_MULT);
+
+	/* Offset value operates in negative way, so swap sign */
+	calibration =3D (int)-tmp;
+
+	ret =3D tps65910_rtc_set_calibration(dev, calibration);
+
+	return ret;
+}
+
 static irqreturn_t tps65910_rtc_interrupt(int irq, void *rtc)
 {
 	struct device *dev =3D rtc;
@@ -219,6 +349,8 @@ static const struct rtc_class_ops tps65910_rtc_ops =3D =
{
 	.read_alarm	=3D tps65910_rtc_read_alarm,
 	.set_alarm	=3D tps65910_rtc_set_alarm,
 	.alarm_irq_enable =3D tps65910_rtc_alarm_irq_enable,
+	.read_offset	=3D tps65910_read_offset,
+	.set_offset	=3D tps65910_set_offset,
 };
=20
 static int tps65910_rtc_probe(struct platform_device *pdev)
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 v2 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 aafa716..a86b79a 100644
--- a/drivers/rtc/rtc-tps65910.c
+++ b/drivers/rtc/rtc-tps65910.c
@@ -47,7 +47,8 @@ struct tps65910_rtc {
 /* Multiplier for ppb conversions */
 #define PPB_MULT	(1000000000LL)
=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 v2 0/2] drivers: rtc: rtc-tps65910: Add RTC calibration support

[Vesa Jääskeläinen]

On 2016-12-10 23:50, 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.
>
> 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
>
>   drivers/rtc/rtc-tps65910.c   | 135 ++++++++++++++++++++++++++++++++++++=
++++++-
>   include/linux/mfd/tps65910.h |   1 +
>   2 files changed, 135 insertions(+), 1 deletion(-)
>
This version now uses 'offset' sysfs file for calibration purposes.

It supports rounding so if one calculates absolute 'offset' compensation=20
value then it should give "optimal" calibration.

I believe one should calculate compensation value like:

offset =3D ((measured_freq/32768)-1)*10^9

So if meaured_freq =3D=3D 32771.2768, then matching offset =3D=3D 100000.
Or if mesured_freq =3D=3D 32764.7232, then matching offset =3D=3D -100000.

'offset' compensation value is following existing rtc.txt definition=20
meaning positive values causes time to slowdown and negative values to=20
fasten up. Which I believe should be other way around to be more=20
logical. This is easy to swap if I just understood it wrongly.

'offset' value calculation assumes "common" understanding how ppm values=20
are used, eg. it is negated compensation value (expect now positive).

Eg. if RTC crystal is off by +100ppm then 'offset' value must be 100ppm=20
(write 100000 to 'offset').

I believe offset documentation in rtc.txt should be clarified on what=20
value means.

'offset' value has limits due to RTC chip itself, if out-of-limits value=20
is given -ERANGE is returned.

Due to RTC chip's implementation -- it loses RTC compensation enable bit=20
on reset, so compensation value must be written on every boot at least=20
once to be active.

When compensation is not enabled "cat offset" returns value. There is=20
currently no way to know from users space (without i2cget -f) whether=20
compensation is enabled or not. If this should not be the case it can be=20
easily fixed, -EINVAL might do the trick.

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 v2 1/2] drivers: rtc: rtc-tps65910: Add RTC calibration support

[Lee Jones]

On Sat, 10 Dec 2016, 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>
> ---
>  drivers/rtc/rtc-tps65910.c   | 132 +++++++++++++++++++++++++++++++++++++=
++++++
>  include/linux/mfd/tps65910.h |   1 +

For the MFD change:

Acked-by: Lee Jones <lee.jones@linaro.org>

>  2 files changed, 133 insertions(+)
>=20
> diff --git a/drivers/rtc/rtc-tps65910.c b/drivers/rtc/rtc-tps65910.c
> index 5a3d53c..aafa716 100644
> --- a/drivers/rtc/rtc-tps65910.c
> +++ b/drivers/rtc/rtc-tps65910.c
> @@ -21,6 +21,7 @@
>  #include <linux/types.h>
>  #include <linux/rtc.h>
>  #include <linux/bcd.h>
> +#include <linux/math64.h>
>  #include <linux/platform_device.h>
>  #include <linux/interrupt.h>
>  #include <linux/mfd/tps65910.h>
> @@ -33,6 +34,19 @@ 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=
)
> +
> +/* Min and max values supported with 'offset' interface (swapped sign) *=
/
> +#define MIN_OFFSET	(-277761)
> +#define MAX_OFFSET	(277778)
> +
> +/* Number of ticks per hour */
> +#define TICKS_PER_HOUR	(32768 * 3600)
> +
> +/* Multiplier for ppb conversions */
> +#define PPB_MULT	(1000000000LL)
> +
>  static int tps65910_rtc_alarm_irq_enable(struct device *dev, unsigned en=
abled)
>  {
>  	struct tps65910 *tps =3D dev_get_drvdata(dev->parent);
> @@ -187,6 +201,122 @@ 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 int tps65910_read_offset(struct device *dev, long *offset)
> +{
> +	int calibration;
> +	s64 tmp;
> +	int ret;
> +
> +	ret =3D tps65910_rtc_get_calibration(dev, &calibration);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Convert from RTC calibration register format to ppb format */
> +	tmp =3D calibration * (s64)PPB_MULT;
> +	if (tmp < 0)
> +		tmp -=3D TICKS_PER_HOUR / 2LL;
> +	else
> +		tmp +=3D TICKS_PER_HOUR / 2LL;
> +	tmp =3D div_s64(tmp, TICKS_PER_HOUR);
> +
> +	/* Offset value operates in negative way, so swap sign */
> +	*offset =3D (long)-tmp;
> +
> +	return 0;
> +}
> +
> +static int tps65910_set_offset(struct device *dev, long offset)
> +{
> +	int calibration;
> +	s64 tmp;
> +	int ret;
> +
> +	/* Make sure offset value is within supported range */
> +	if (offset < MIN_OFFSET || offset > MAX_OFFSET)
> +		return -ERANGE;
> +
> +	/* Convert from ppb format to RTC calibration register format */
> +	tmp =3D offset * (s64)TICKS_PER_HOUR;
> +	if (tmp < 0)
> +		tmp -=3D PPB_MULT / 2LL;
> +	else
> +		tmp +=3D PPB_MULT / 2LL;
> +	tmp =3D div_s64(tmp, PPB_MULT);
> +
> +	/* Offset value operates in negative way, so swap sign */
> +	calibration =3D (int)-tmp;
> +
> +	ret =3D tps65910_rtc_set_calibration(dev, calibration);
> +
> +	return ret;
> +}
> +
>  static irqreturn_t tps65910_rtc_interrupt(int irq, void *rtc)
>  {
>  	struct device *dev =3D rtc;
> @@ -219,6 +349,8 @@ static const struct rtc_class_ops tps65910_rtc_ops =
=3D {
>  	.read_alarm	=3D tps65910_rtc_read_alarm,
>  	.set_alarm	=3D tps65910_rtc_set_alarm,
>  	.alarm_irq_enable =3D tps65910_rtc_alarm_irq_enable,
> +	.read_offset	=3D tps65910_read_offset,
> +	.set_offset	=3D tps65910_set_offset,
>  };
> =20
>  static int tps65910_rtc_probe(struct platform_device *pdev)
> 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
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org =E2=94=82 Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

--=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 v2 0/2] drivers: rtc: rtc-tps65910: Add RTC calibration support

[Alexandre Belloni]

On 11/12/2016 at 00:21:02 +0200, Vesa J=C3=A4=C3=A4skel=C3=A4inen wrote :
>=20
> On 2016-12-10 23:50, 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 compens=
ated
> > with two's complement value.
> >=20
> > 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
> >=20
> >   drivers/rtc/rtc-tps65910.c   | 135 ++++++++++++++++++++++++++++++++++=
++++++++-
> >   include/linux/mfd/tps65910.h |   1 +
> >   2 files changed, 135 insertions(+), 1 deletion(-)
> >=20
> This version now uses 'offset' sysfs file for calibration purposes.
>=20
> It supports rounding so if one calculates absolute 'offset' compensation
> value then it should give "optimal" calibration.
>=20
> I believe one should calculate compensation value like:
>=20
> offset =3D ((measured_freq/32768)-1)*10^9
>=20
> So if meaured_freq =3D=3D 32771.2768, then matching offset =3D=3D 100000.
> Or if mesured_freq =3D=3D 32764.7232, then matching offset =3D=3D -100000=
.
>=20
> 'offset' compensation value is following existing rtc.txt definition mean=
ing
> positive values causes time to slowdown and negative values to fasten up.
> Which I believe should be other way around to be more logical. This is ea=
sy
> to swap if I just understood it wrongly.
>=20

The hardware vendors are using positive offsets to slow the RTC and
negative offsets to hasten it so that's what we exposed to userspace as
it avoids a conversion.

> 'offset' value calculation assumes "common" understanding how ppm values =
are
> used, eg. it is negated compensation value (expect now positive).
>=20
> Eg. if RTC crystal is off by +100ppm then 'offset' value must be 100ppm
> (write 100000 to 'offset').
>=20
> I believe offset documentation in rtc.txt should be clarified on what val=
ue
> means.
>=20
> 'offset' value has limits due to RTC chip itself, if out-of-limits value =
is
> given -ERANGE is returned.
>=20

That's fine.

> Due to RTC chip's implementation -- it loses RTC compensation enable bit =
on
> reset, so compensation value must be written on every boot at least once =
to
> be active.
>=20
> When compensation is not enabled "cat offset" returns value. There is
> currently no way to know from users space (without i2cget -f) whether
> compensation is enabled or not. If this should not be the case it can be
> easily fixed, -EINVAL might do the trick.
>=20

If compensation is not enabled, simply return 0 so userspace actually
has a way to know it is not currently in effect. Anyway, you'll have to
write a value to enable it again.
You are probably already able to calculate that value anyway so I'm not
sure of the benefit of using the RTC to remember it across resets.


--=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 v2 0/2] drivers: rtc: rtc-tps65910: Add RTC calibration support

[Alexandre Belloni]

On 19/12/2016 at 01:38:54 +0100, Alexandre Belloni wrote :
> On 11/12/2016 at 00:21:02 +0200, Vesa J=C3=A4=C3=A4skel=C3=A4inen wrote :
> >=20
> > On 2016-12-10 23:50, 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
> > > 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 'unsigne=
d'
> > >      in arguments
> > >=20
> > >   drivers/rtc/rtc-tps65910.c   | 135 ++++++++++++++++++++++++++++++++=
++++++++++-
> > >   include/linux/mfd/tps65910.h |   1 +
> > >   2 files changed, 135 insertions(+), 1 deletion(-)
> > >=20
> > This version now uses 'offset' sysfs file for calibration purposes.
> >=20
> > It supports rounding so if one calculates absolute 'offset' compensatio=
n
> > value then it should give "optimal" calibration.
> >=20
> > I believe one should calculate compensation value like:
> >=20
> > offset =3D ((measured_freq/32768)-1)*10^9
> >=20
> > So if meaured_freq =3D=3D 32771.2768, then matching offset =3D=3D 10000=
0.
> > Or if mesured_freq =3D=3D 32764.7232, then matching offset =3D=3D -1000=
00.
> >=20
> > 'offset' compensation value is following existing rtc.txt definition me=
aning
> > positive values causes time to slowdown and negative values to fasten u=
p.
> > Which I believe should be other way around to be more logical. This is =
easy
> > to swap if I just understood it wrongly.
> >=20
>=20
> The hardware vendors are using positive offsets to slow the RTC and
> negative offsets to hasten it so that's what we exposed to userspace as
> it avoids a conversion.
>=20

To be clear, that is unfortunately not the case for the TPS65910.

Usually, the offset is related to a number of clock cycles (or ticks)
that are added in a second (slowing the RTC). On the tps65910, it is the
number that is added to the counter (hastening the RTC).

One or the other would feel backward for someone anyway. I'll try to
clarify that in the documentation.


> > 'offset' value calculation assumes "common" understanding how ppm value=
s are
> > used, eg. it is negated compensation value (expect now positive).
> >=20
> > Eg. if RTC crystal is off by +100ppm then 'offset' value must be 100ppm
> > (write 100000 to 'offset').
> >=20
> > I believe offset documentation in rtc.txt should be clarified on what v=
alue
> > means.
> >=20
> > 'offset' value has limits due to RTC chip itself, if out-of-limits valu=
e is
> > given -ERANGE is returned.
> >=20
>=20
> That's fine.
>=20
> > Due to RTC chip's implementation -- it loses RTC compensation enable bi=
t on
> > reset, so compensation value must be written on every boot at least onc=
e to
> > be active.
> >=20
> > When compensation is not enabled "cat offset" returns value. There is
> > currently no way to know from users space (without i2cget -f) whether
> > compensation is enabled or not. If this should not be the case it can b=
e
> > easily fixed, -EINVAL might do the trick.
> >=20
>=20
> If compensation is not enabled, simply return 0 so userspace actually
> has a way to know it is not currently in effect. Anyway, you'll have to
> write a value to enable it again.
> You are probably already able to calculate that value anyway so I'm not
> sure of the benefit of using the RTC to remember it across resets.
>=20

After reviewing the patch, it is fine apart from that part.

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

--=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 v2 2/2] drivers: rtc: rtc-tps65910: use 'unsigned int' instead of 'unsigned' in arguments

[Alexandre Belloni]

Hi,

On 10/12/2016 at 23:50:44 +0200, Vesa J=C3=A4=C3=A4skel=C3=A4inen wrote :

Please add a short commit message here.

> Signed-off-by: Vesa J=C3=A4=C3=A4skel=C3=A4inen <vesa.jaaskelainen@vaisal=
a.com>
> ---
>  drivers/rtc/rtc-tps65910.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>=20
> diff --git a/drivers/rtc/rtc-tps65910.c b/drivers/rtc/rtc-tps65910.c
> index aafa716..a86b79a 100644
> --- a/drivers/rtc/rtc-tps65910.c
> +++ b/drivers/rtc/rtc-tps65910.c
> @@ -47,7 +47,8 @@ struct tps65910_rtc {
>  /* Multiplier for ppb conversions */
>  #define PPB_MULT	(1000000000LL)
> =20
> -static int tps65910_rtc_alarm_irq_enable(struct device *dev, unsigned en=
abled)
> +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

--=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 v2 1/2] drivers: rtc: rtc-tps65910: Add RTC calibration support

[Vesa Jääskeläinen]

On 2016-12-12 09:44, Lee Jones wrote:
> On Sat, 10 Dec 2016, 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>
>> ---
>>   drivers/rtc/rtc-tps65910.c   | 132 +++++++++++++++++++++++++++++++++++=
++++++++
>>   include/linux/mfd/tps65910.h |   1 +
> For the MFD change:
>
> Acked-by: Lee Jones <lee.jones@linaro.org>

Thanks for the review. Added Acked-by to v3.

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 v2 0/2] drivers: rtc: rtc-tps65910: Add RTC calibration support

[Vesa Jääskeläinen]

On 2016-12-19 02:53, Alexandre Belloni wrote:
> On 19/12/2016 at 01:38:54 +0100, Alexandre Belloni wrote :
>> On 11/12/2016 at 00:21:02 +0200, Vesa J=C3=A4=C3=A4skel=C3=A4inen wrote =
:
>>> On 2016-12-10 23:50, Vesa J=C3=A4=C3=A4skel=C3=A4inen wrote:
>>> Due to RTC chip's implementation -- it loses RTC compensation enable bi=
t on
>>> reset, so compensation value must be written on every boot at least onc=
e to
>>> be active.
>>>
>>> When compensation is not enabled "cat offset" returns value. There is
>>> currently no way to know from users space (without i2cget -f) whether
>>> compensation is enabled or not. If this should not be the case it can b=
e
>>> easily fixed, -EINVAL might do the trick.
>>>
>> If compensation is not enabled, simply return 0 so userspace actually
>> has a way to know it is not currently in effect. Anyway, you'll have to
>> write a value to enable it again.
>> You are probably already able to calculate that value anyway so I'm not
>> sure of the benefit of using the RTC to remember it across resets.
>>
> After reviewing the patch, it is fine apart from that part.
>

Sent patch v3 to address that issue and improved commit message for=20
cleanup patch.

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.