| Message ID | E1dxsSB-0006RM-08@rmk-PC.armlinux.org.uk |
|---|---|
| State | Accepted |
| Headers | show |
| Series | rtc: armada38x: add support for trimming the RTC | expand |
Hi Russell, On 29/09/2017 at 11:23:31 +0100, Russell King wrote: > +/* > + * 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; 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. > +}
On Tue, Oct 03, 2017 at 03:24:33PM +0200, Alexandre Belloni wrote: > Hi Russell, > > On 29/09/2017 at 11:23:31 +0100, Russell King wrote: > > +/* > > + * 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; > > 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.
On 29/09/2017 at 11:23:31 +0100, Russell King wrote: > 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. > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> > --- > drivers/rtc/rtc-armada38x.c | 101 ++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 101 insertions(+) > Applied, thanks.
diff --git a/drivers/rtc/rtc-armada38x.c b/drivers/rtc/rtc-armada38x.c index 21f355c37eab..1e4978c96ffd 100644 --- a/drivers/rtc/rtc-armada38x.c +++ b/drivers/rtc/rtc-armada38x.c @@ -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 = {
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. Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> --- drivers/rtc/rtc-armada38x.c | 101 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 101 insertions(+)