Skip to contentSkip to conversation

rtc: armada38x: add support for trimming the RTC

PATCH New Russell King 3 participants 4 messages Sep 29, 2017 – Oct 25, 2017

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 rtc: armada38x: add support for trimming the RTC

Add support for trimming the RTC using the offset mechanism. This RTC supports two modes: low update mode and high update mode. Low update mode has finer precision than high update mode, so we use the low mode where possible.

---
 drivers/rtc/rtc-armada38x.c | 101 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 101 insertions(+)
drivers/rtc/rtc-armada38x.c +101 -0
index 21f355c37eab..1e4978c96ffd 100644
@@ -28,6 +28,8 @@
 #define RTC_IRQ_AL_EN		    BIT(0)
 #define RTC_IRQ_FREQ_EN		    BIT(1)
 #define RTC_IRQ_FREQ_1HZ	    BIT(2)
+#define RTC_CCR		    0x18
+#define RTC_CCR_MODE		    BIT(15)
 
 #define RTC_TIME	    0xC
 #define RTC_ALARM1	    0x10
@@ -343,18 +345,117 @@ static irqreturn_t armada38x_rtc_alarm_irq(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
+/*
+ * The information given in the Armada 388 functional spec is complex.
+ * They give two different formulas for calculating the offset value,
+ * but when considering "Offset" as an 8-bit signed integer, they both
+ * reduce down to (we shall rename "Offset" as "val" here):
+ *
+ *   val = (f_ideal / f_measured - 1) / resolution   where f_ideal = 32768
+ *
+ * Converting to time, f = 1/t:
+ *   val = (t_measured / t_ideal - 1) / resolution   where t_ideal = 1/32768
+ *
+ *   =>  t_measured / t_ideal = val * resolution + 1
+ *
+ * "offset" in the RTC interface is defined as:
+ *   t = t0 * (1 + offset * 1e-9)
+ * where t is the desired period, t0 is the measured period with a zero
+ * offset, which is t_measured above. With t0 = t_measured and t = t_ideal,
+ *   offset = (t_ideal / t_measured - 1) / 1e-9
+ *
+ *   => t_ideal / t_measured = offset * 1e-9 + 1
+ *
+ * so:
+ *
+ *   offset * 1e-9 + 1 = 1 / (val * resolution + 1)
+ *
+ * We want "resolution" to be an integer, so resolution = R * 1e-9, giving
+ *   offset = 1e18 / (val * R + 1e9) - 1e9
+ *   val = (1e18 / (offset + 1e9) - 1e9) / R
+ * with a common transformation:
+ *   f(x) = 1e18 / (x + 1e9) - 1e9
+ *   offset = f(val * R)
+ *   val = f(offset) / R
+ *
+ * Armada 38x supports two modes, fine mode (954ppb) and coarse mode (3815ppb).
+ */
+static long armada38x_ppb_convert(long ppb)
+{
+	long div = ppb + 1000000000L;
+
+	return div_s64(1000000000000000000LL + div / 2, div) - 1000000000L;
+}
+
+static int armada38x_rtc_read_offset(struct device *dev, long *offset)
+{
+	struct armada38x_rtc *rtc = dev_get_drvdata(dev);
+	unsigned long ccr, flags;
+	long ppb_cor;
+
+	spin_lock_irqsave(&rtc->lock, flags);
+	ccr = rtc->data->read_rtc_reg(rtc, RTC_CCR);
+	spin_unlock_irqrestore(&rtc->lock, flags);
+
+	ppb_cor = (ccr & RTC_CCR_MODE ? 3815 : 954) * (s8)ccr;
+	/* ppb_cor + 1000000000L can never be zero */
+	*offset = armada38x_ppb_convert(ppb_cor);
+
+	return 0;
+}
+
+static int armada38x_rtc_set_offset(struct device *dev, long offset)
+{
+	struct armada38x_rtc *rtc = dev_get_drvdata(dev);
+	unsigned long ccr = 0;
+	long ppb_cor, off;
+
+	/*
+	 * The maximum ppb_cor is -128 * 3815 .. 127 * 3815, but we
+	 * need to clamp the input.  This equates to -484270 .. 488558.
+	 * Not only is this to stop out of range "off" but also to
+	 * avoid the division by zero in armada38x_ppb_convert().
+	 */
+	offset = clamp(offset, -484270L, 488558L);
+
+	ppb_cor = armada38x_ppb_convert(offset);
+
+	/*
+	 * Use low update mode where possible, which gives a better
+	 * resolution of correction.
+	 */
+	off = DIV_ROUND_CLOSEST(ppb_cor, 954);
+	if (off > 127 || off < -128) {
+		ccr = RTC_CCR_MODE;
+		off = DIV_ROUND_CLOSEST(ppb_cor, 3815);
+	}
+
+	/*
+	 * Armada 388 requires a bit pattern in bits 14..8 depending on
+	 * the sign bit: { 0, ~S, S, S, S, S, S }
+	 */
+	ccr |= (off & 0x3fff) ^ 0x2000;
+	rtc_delayed_write(ccr, rtc, RTC_CCR);
+
+	return 0;
+}
+
 static const struct rtc_class_ops armada38x_rtc_ops = {
 	.read_time = armada38x_rtc_read_time,
 	.set_time = armada38x_rtc_set_time,
 	.read_alarm = armada38x_rtc_read_alarm,
 	.set_alarm = armada38x_rtc_set_alarm,
 	.alarm_irq_enable = armada38x_rtc_alarm_irq_enable,
+	.read_offset = armada38x_rtc_read_offset,
+	.set_offset = armada38x_rtc_set_offset,
 };
 
 static const struct rtc_class_ops armada38x_rtc_ops_noirq = {
 	.read_time = armada38x_rtc_read_time,
 	.set_time = armada38x_rtc_set_time,
 	.read_alarm = armada38x_rtc_read_alarm,
+	.read_offset = armada38x_rtc_read_offset,
+	.set_offset = armada38x_rtc_set_offset,
 };
 
 static const struct armada38x_rtc_data armada38x_data = {
Signature · 1 line
2.7.4
#1 lore raw

PATCH 1/1 rtc: armada38x: add support for trimming the RTC

Hi Russell,

Quoted 30 lines · Russell King
drivers/rtc/rtc-armada38x.c +30 -0
+/*
+ * The information given in the Armada 388 functional spec is complex.
+ * They give two different formulas for calculating the offset value,
+ * but when considering "Offset" as an 8-bit signed integer, they both
+ * reduce down to (we shall rename "Offset" as "val" here):
+ *
+ *   val = (f_ideal / f_measured - 1) / resolution   where f_ideal = 32768
+ *
+ * Converting to time, f = 1/t:
+ *   val = (t_measured / t_ideal - 1) / resolution   where t_ideal = 1/32768
+ *
+ *   =>  t_measured / t_ideal = val * resolution + 1
+ *
+ * "offset" in the RTC interface is defined as:
+ *   t = t0 * (1 + offset * 1e-9)
+ * where t is the desired period, t0 is the measured period with a zero
+ * offset, which is t_measured above. With t0 = t_measured and t = t_ideal,
+ *   offset = (t_ideal / t_measured - 1) / 1e-9
+ *
+ *   => t_ideal / t_measured = offset * 1e-9 + 1
+ *
+ * so:
+ *
+ *   offset * 1e-9 + 1 = 1 / (val * resolution + 1)
+ *
+ * We want "resolution" to be an integer, so resolution = R * 1e-9, giving
+ *   offset = 1e18 / (val * R + 1e9) - 1e9
+ *   val = (1e18 / (offset + 1e9) - 1e9) / R
+ * with a common transformation:
+ *   f(x) = 1e18 / (x + 1e9) - 1e9

drivers/rtc/rtc-armada38x.c static irqreturn_t armada38x_rtc_alarm_irq(int irq, void *data) · line 387

Quoted 10 lines
drivers/rtc/rtc-armada38x.c +10 -0
+ *   offset = f(val * R)
+ *   val = f(offset) / R
+ *
+ * Armada 38x supports two modes, fine mode (954ppb) and coarse mode (3815ppb).
+ */
+static long armada38x_ppb_convert(long ppb)
+{
+	long div = ppb + 1000000000L;
+
+	return div_s64(1000000000000000000LL + div / 2, div) - 1000000000L;

The previous comment is perfect but it was not completely obvious to me why you are adding div / 2. Maybe you can add a small comment.

Quoted 1 line

+}

Signature · 3 lines
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
#2 lore raw

PATCH 1/1 rtc: armada38x: add support for trimming the RTC

Quoted 38 lines · Alexandre Belloni

Hi Russell,

Quoted 35 lines · Russell King

+/*
+ * The information given in the Armada 388 functional spec is complex.
+ * They give two different formulas for calculating the offset value,
+ * but when considering "Offset" as an 8-bit signed integer, they both
+ * reduce down to (we shall rename "Offset" as "val" here):
+ *
+ * val = (f_ideal / f_measured - 1) / resolution where f_ideal = 32768
+ *
+ * Converting to time, f = 1/t:
+ * val = (t_measured / t_ideal - 1) / resolution where t_ideal = 1/32768
+ *
+ * => t_measured / t_ideal = val * resolution + 1
+ *
+ * "offset" in the RTC interface is defined as:
+ * t = t0 * (1 + offset * 1e-9)
+ * where t is the desired period, t0 is the measured period with a zero
+ * offset, which is t_measured above. With t0 = t_measured and t = t_ideal,
+ * offset = (t_ideal / t_measured - 1) / 1e-9
+ *
+ * => t_ideal / t_measured = offset * 1e-9 + 1
+ *
+ * so:
+ *
+ * offset * 1e-9 + 1 = 1 / (val * resolution + 1)
+ *
+ * We want "resolution" to be an integer, so resolution = R * 1e-9, giving
+ * offset = 1e18 / (val * R + 1e9) - 1e9
+ * val = (1e18 / (offset + 1e9) - 1e9) / R
+ * with a common transformation:

+ *   f(x) = 1e18 / (x + 1e9) - 1e9
+ *   offset = f(val * R)
+ *   val = f(offset) / R

+ *
+ * Armada 38x supports two modes, fine mode (954ppb) and coarse mode (3815ppb).
+ */

Quoted 8 lines

drivers/rtc/rtc-armada38x.c static irqreturn_t armada38x_rtc_alarm_irq(int irq, void *data) · line 387

Quoted 5 lines
drivers/rtc/rtc-armada38x.c +5 -0
+static long armada38x_ppb_convert(long ppb)
+{
+	long div = ppb + 1000000000L;
+
+	return div_s64(1000000000000000000LL + div / 2, div) - 1000000000L;

The previous comment is perfect but it was not completely obvious to me why you are adding div / 2. Maybe you can add a small comment.

It's the standard way to turn a regular integer division into one which rounds-to-closest - I consider it to be one of the basics which every programmer should know, and therefore should be obvious.

Integer division in the form of:

	r = (a + b / 2) / b

should be part of the programmers basic knowledge set.

Signature · 3 lines
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
#3 lore raw

PATCH 1/1 rtc: armada38x: add support for trimming the RTC

In reply to Russell King
Quoted 10 lines · Russell King

Add support for trimming the RTC using the offset mechanism. This RTC supports two modes: low update mode and high update mode. Low update mode has finer precision than high update mode, so we use the low mode where possible.

---
 drivers/rtc/rtc-armada38x.c | 101 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 101 insertions(+)

Applied, thanks.

Signature · 3 lines
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com