Re: [RFC+PATCH] RTC calibration
On Tue 2007-09-11 18:04:06, Dag-Erling Sm?rgrav wrote: > Clemens Koller <[EMAIL PROTECTED]> writes: > > Dag-Erling Sm?rgrav <[EMAIL PROTECTED]> writes: > > > Without knowing exacly which chip is present, there is no way for the > > > userland calibration tool to know how big a difference a calibration > > > step makes. > > I am not talking about the calibration algorithm and it's quality. > > Neither am I. > > > I am talking about _how_ the calibration register is addressed from > > userspace. It's a simple register, some bits at address 7 and I would > > expect to read/modify/write registers to do all the things you want > > to do. Register access in userspace doesn't put any limitation > > to applications. > > It requires the application to know the hardware intimately. > > Calibration of the M41T11 is implemented using the lower 6 bits of > register 7; this is not necessarily the case for other existing or > future chips. The driver should know the hardware. > Let's say I normalize this to [-128;127]; an application that tried to > speed up the clock would waste several hours increasing the > calibration value from 0 to 1, 2, 3 before seeing an effect after > increasing it to 4. And how do I normalize the assymetric range of > the M41T11? So you normalize it to -32;32 range, and tell application (using ioctl) that range is -32;32? Or you just -EINVAL if it goes out of range? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC+PATCH] RTC calibration
Arne Georg Gleditsch <[EMAIL PROTECTED]> writes: > Dag-Erling Smørgrav <[EMAIL PROTECTED]> writes: > > + case RTC_SPEED_UP: > > + err = rtc_speed_up(rtc); > > + break; > > + > > + case RTC_SLOW_DOWN: > > + err = rtc_speed_up(rtc); > > + break; > This doesn't look quite right. rtc_slow_down, surely? Uh, yes. ENOCOFFEE. DES -- Dag-Erling Smørgrav Senior Software Developer Linpro AS - www.linpro.no - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC+PATCH] RTC calibration
Dag-Erling Smørgrav <[EMAIL PROTECTED]> writes: > + case RTC_SPEED_UP: > + err = rtc_speed_up(rtc); > + break; > + > + case RTC_SLOW_DOWN: > + err = rtc_speed_up(rtc); > + break; This doesn't look quite right. rtc_slow_down, surely? -- Arne. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC+PATCH] RTC calibration
Dag-Erling Smørgrav schrieb: > Clemens Koller <[EMAIL PROTECTED]> writes: >> I am talking about _how_ the calibration register is addressed from >> userspace. It's a simple register, some bits at address 7 and I would >> expect to read/modify/write registers to do all the things you want >> to do. Register access in userspace doesn't put any limitation >> to applications. > > It requires the application to know the hardware intimately. That's right... there is no need to put that into the kernel and hide this trivial functionality from userspace. > Calibration of the M41T11 is implemented using the lower 6 bits of > register 7; this is not necessarily the case for other existing or > future chips. I've read the datasheet. Your driver is specific for the M41T11 chip as mentioned in your first mail, isn't it? If any future driver comes up with 8 bits you wouldn't need to change a generic interface to read/modify/write these 8 bits, right? > Let's say I normalize this to [-128;127]; Why do any normalization in the driver? That's what userspace can do in any way it might be necessary to do. >> Having only incs and decs without getting the actual value back seems >> to be an absolutely unnecessary limitation here. >> You cannot get the current value back to see if it's i.e. in saturation in >> a way that it doesn't make sense to inc/decrement it further or in bigger steps >> or reset it to zero... > > The driver will return EINVAL if you try to increment or decrement the > calibration register beyond its limits. That behaviour seems also odd to me... I can increment/decrement by an unknown number and then I get an EINVAL. And I cannot reset it to some default value easily. I still don't see any reason to implement relative changes to an otherwise unknown value if it's possible to give absolute values to work with. Well, that's my opinion, my five cents... I don't want to get into a lenghtly discussion... I just used common sense how a interface to a register might look like and your way of a relative manipulation just looks very uncommon to me, having seen lot's of drivers. Best regards, Clemens Koller __ R&D Imaging Devices Anagramm GmbH Rupert-Mayer-Straße 45/1 Linhof Werksgelände D-81379 München Tel.089-741518-50 Fax 089-741518-19 http://www.anagramm-technology.com - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC+PATCH] RTC calibration
Mark Gross <[EMAIL PROTECTED]> writes: > Is it necessary to change the rtc interface? Can't you detect within > your m48t59 driver that you are getting consistent time adjustments up > or down from whats in the rtc and infer it needs speeding up or slowing > down? [I didn't notice this when I first replied] Because of the RTC's low resolution and of how the calibration is implemented in the chip, you need to sample the clock at intervals of several hours in order to determine the direction of the required adjustment. Repeatedly setting the RTC to the correct time will actually prevent correct calibration. DES -- Dag-Erling Smørgrav Senior Software Developer Linpro AS - www.linpro.no - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC+PATCH] RTC calibration
Clemens Koller <[EMAIL PROTECTED]> writes: > Dag-Erling Smørgrav <[EMAIL PROTECTED]> writes: > > Without knowing exacly which chip is present, there is no way for the > > userland calibration tool to know how big a difference a calibration > > step makes. > I am not talking about the calibration algorithm and it's quality. Neither am I. > I am talking about _how_ the calibration register is addressed from > userspace. It's a simple register, some bits at address 7 and I would > expect to read/modify/write registers to do all the things you want > to do. Register access in userspace doesn't put any limitation > to applications. It requires the application to know the hardware intimately. Calibration of the M41T11 is implemented using the lower 6 bits of register 7; this is not necessarily the case for other existing or future chips. Let's say I normalize this to [-128;127]; an application that tried to speed up the clock would waste several hours increasing the calibration value from 0 to 1, 2, 3 before seeing an effect after increasing it to 4. And how do I normalize the assymetric range of the M41T11? > Having only incs and decs without getting the actual value back seems > to be an absolutely unnecessary limitation here. > You cannot get the current value back to see if it's i.e. in saturation in > a way that it doesn't make sense to inc/decrement it further or in bigger > steps > or reset it to zero... The driver will return EINVAL if you try to increment or decrement the calibration register beyond its limits. DES -- Dag-Erling Smørgrav Senior Software Developer Linpro AS - www.linpro.no - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC+PATCH] RTC calibration
Mark Gross <[EMAIL PROTECTED]> writes: > Why not use NTP? Because NTP is a completely different beast. NTP is used to keep a software clock synchronized with a remote source. This software clock is usually implemented as a software PLL on top of a high-resolution hardware clock, with the remote clock serving as frequency reference. What I need to do, however, is calibrate a low-resolution hardware clock using a trusted reference (which could very well be a software clock maintained by NTP). DES -- Dag-Erling Smørgrav Senior Software Developer Linpro AS - www.linpro.no - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC+PATCH] RTC calibration
Dag-Erling Smørgrav schrieb: > Clemens Koller <[EMAIL PROTECTED]> writes: >> It looks odd to me to do only differential up and down adjustments. >> I would prefer read_calibration_register and write_calibration_register >> access and let the userspace decide how much it wants to >> increment/decrement the register. > > Without knowing exacly which chip is present, there is no way for the > userland calibration tool to know how big a difference a calibration > step makes. I am not talking about the calibration algorithm and it's quality. I am talking about _how_ the calibration register is addressed from userspace. It's a simple register, some bits at address 7 and I would expect to read/modify/write registers to do all the things you want to do. Register access in userspace doesn't put any limitation to applications. Having only incs and decs without getting the actual value back seems to be an absolutely unnecessary limitation here. You cannot get the current value back to see if it's i.e. in saturation in a way that it doesn't make sense to inc/decrement it further or in bigger steps or reset it to zero... Regards, Clemens Koller __ R&D Imaging Devices Anagramm GmbH Rupert-Mayer-Straße 45/1 Linhof Werksgelände D-81379 München Tel.089-741518-50 Fax 089-741518-19 http://www.anagramm-technology.com - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC+PATCH] RTC calibration
On Tue, Sep 11, 2007 at 03:48:53PM +0200, Dag-Erling Smørgrav wrote:
> The STMicroelectronics M41T11 is an RTC chip which is similar in most
> ways to the Dallas/Maxim DS1307 or DS1337, except that it supports
> software calibration:
>
> http://www.st.com/stonline/books/pdf/docs/6103.pdf
>
> I want to use this calibration feature in an embedded device I'm
> working on at TANDBERG Telecom AS. The basic idea is that when the
> device is connected to the network and an accurate reference clock is
> available, a userland process will periodically measure the divergence
> between the reference clock and the RTC, set the RTC to the correct
> time and step the calibration register in the correct direction to
> compensate for the observed drift.
>
> The purpose of this is to reduce the extent to which the RTC will
> drift when the device is switched off. Obviously it won't be perfect,
> as the temperature will be lower when the device is off than when it
> is being calibrated, but the crystal's accuracy curve is reasonably
> flat across the relevant temperature range.
>
> Since there is no way to distinguish the M41T11 from its many cousins,
> I've forked a new rtc-m41t11 driver from rtc-ds1307, stripped some
> irrelevant code, cleaned up the rest, and added support for the
> calibration register.
>
> In order to access this from userland, I've added two ioctls,
> (RTC_SPEED_UP and RTC_SPEED_DOWN) with corresponding functions in
> drivers/rtc/interface.c (rtc_speed_up() and rtc_speed_down()) and
> corresponding slots in rtc_class_ops (speed_up and speed_down).
I wonder if you can plug into the ntp code somehow to do this
automatically?
> These ioctls etc. take no parameters: speed_up will increase the speed
> of the RTC by some unspecified small amount and slow_down will
> decrease it by some unspecified small amount. The application must
> simply measure, tweak, wait, measure, tweak, wait etc. until the RTC
> is within epsilon of the reference - where epsilon is some number that
> takes into account the precision of the RTC and of the reference
> clock, system call overhead, etc. Since the RTC usually has a 1s
> precision, epsilon is likely to be ±1s in practice.
Sounds like a simplified version of ntp to me.
>
> My hope is that this interface is simple enough and general enough to
> be usable by other RTC chips that may also support calibration, and
> that both design and code are of sufficiently quality to make it into
> the mainline at some point.
>
> Patch follows my sig. Comments of all kinds are welcome.
Why not use NTP?
>
> DES
> --
> Dag-Erling Smørgrav
> Senior Software Developer
> Linpro AS - www.linpro.no
>
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index ff9e35c..07e5182 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -206,6 +206,15 @@ config RTC_DRV_PCF8583
> This driver can also be built as a module. If so, the module
> will be called rtc-pcf8583.
>
> +config RTC_DRV_M41T11
> + tristate "ST M41T11"
> + depends on RTC_CLASS && I2C
> + help
> + If you say yes here you get support for the ST M41T11 RTC chip.
> +
> + This driver can also be built as a module. If so, the module
> + will be called rtc-m41t11.
> +
> config RTC_DRV_M41T80
> tristate "ST M41T80 series RTC"
> help
> diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
> index d3a33aa..468aa0c 100644
> --- a/drivers/rtc/Makefile
> +++ b/drivers/rtc/Makefile
> @@ -28,6 +28,7 @@ obj-$(CONFIG_RTC_DRV_DS1672)+= rtc-ds1672.o
> obj-$(CONFIG_RTC_DRV_DS1742) += rtc-ds1742.o
> obj-$(CONFIG_RTC_DRV_EP93XX) += rtc-ep93xx.o
> obj-$(CONFIG_RTC_DRV_ISL1208)+= rtc-isl1208.o
> +obj-$(CONFIG_RTC_DRV_M41T11) += rtc-m41t11.o
> obj-$(CONFIG_RTC_DRV_M41T80) += rtc-m41t80.o
> obj-$(CONFIG_RTC_DRV_M48T59) += rtc-m48t59.o
> obj-$(CONFIG_RTC_DRV_M48T86) += rtc-m48t86.o
> diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
> index ad66c6e..189afb4 100644
> --- a/drivers/rtc/interface.c
> +++ b/drivers/rtc/interface.c
> @@ -272,3 +272,43 @@ int rtc_irq_set_freq(struct rtc_device *rtc, struct
> rtc_task *task, int freq)
> return err;
> }
> EXPORT_SYMBOL_GPL(rtc_irq_set_freq);
Is it necessary to change the rtc interface? Can't you detect within
your m48t59 driver that you are getting consistent time adjustments up
or down from whats in the rtc and infer it needs speeding up or slowing
down?
Something like :
within the *_get_time function compute time delta, if 0 +
> +int rtc_speed_up(struct rtc_device *rtc)
> +{
> + int err = 0;
> +
> + err = mutex_lock_interruptible(&rtc->ops_lock);
> + if (err)
> + return -EBUSY;
> +
> + if (!rtc->ops)
> + err = -ENODEV;
> + else if (!rtc->ops->speed_up)
> + err = -EINVAL;
> + else
> + err = rtc->ops->speed_up(rtc->dev.parent);
> +
> + mutex_unlock(&rtc->ops_lock);
> + return err;
> +}
> +E
Re: [RFC+PATCH] RTC calibration
Clemens Koller <[EMAIL PROTECTED]> writes: > It looks odd to me to do only differential up and down adjustments. > I would prefer read_calibration_register and write_calibration_register > access and let the userspace decide how much it wants to > increment/decrement the register. Without knowing exacly which chip is present, there is no way for the userland calibration tool to know how big a difference a calibration step makes. I don't see the value in telling the caller that the current calibration value is N when it actually has no idea what N means, what the allowable range is, if the scale is linear, logarithmic, exponential, discontinuous... The M41T11, for instance, has a calibration register that runs from -31 to +31, with negative values are multiples of ~2.034 ppm while positive values are multiples of ~4.068 ppm. Therefore, I prefer to leave it to the driver author to decide what a reasonable step in either direction is. The only consequence for the userland calibration tool is that the RTC may take longer to converge on hardware that has smaller calibration steps. > Or - do you adjust your date also after you changed your battery > with +1567days,7h,34m,12sec instead of telling it's 11th Sept. 2007, 4:33pm ? No, the calibration process should start by setting the RTC to the same time as the reference clock once it becomes available, then waiting for a sufficient amount of time (at least three or four hours to begin with, then progressively longer as the divergence decreases) before measuring and adjusting. Alternatively, if you don't want to set the RTC, you measure the delta, wait sufficiently long, measure the delta again, and compare the two deltas (this method is actually exactly the same as the one outlined above, except that above the initial delta is 0) DES -- Dag-Erling Smørgrav Senior Software Developer Linpro AS - www.linpro.no - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC+PATCH] RTC calibration
Dag-Erling Smørgrav schrieb: > The STMicroelectronics M41T11 is an RTC chip which is similar in most > ways to the Dallas/Maxim DS1307 or DS1337, except that it supports > software calibration: > > http://www.st.com/stonline/books/pdf/docs/6103.pdf > > I want to use this calibration feature [...] > [...] > In order to access this from userland, I've added two ioctls, > (RTC_SPEED_UP and RTC_SPEED_DOWN) with corresponding functions in > drivers/rtc/interface.c (rtc_speed_up() and rtc_speed_down()) and > corresponding slots in rtc_class_ops (speed_up and speed_down). It looks odd to me to do only differential up and down adjustments. I would prefer read_calibration_register and write_calibration_register access and let the userspace decide how much it wants to increment/decrement the register. Or - do you adjust your date also after you changed your battery with +1567days,7h,34m,12sec instead of telling it's 11th Sept. 2007, 4:33pm ? Regards, Clemens Koller __ R&D Imaging Devices Anagramm GmbH Rupert-Mayer-Straße 45/1 Linhof Werksgelände D-81379 München Tel.089-741518-50 Fax 089-741518-19 http://www.anagramm-technology.com - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC+PATCH] RTC calibration
The STMicroelectronics M41T11 is an RTC chip which is similar in most
ways to the Dallas/Maxim DS1307 or DS1337, except that it supports
software calibration:
http://www.st.com/stonline/books/pdf/docs/6103.pdf
I want to use this calibration feature in an embedded device I'm
working on at TANDBERG Telecom AS. The basic idea is that when the
device is connected to the network and an accurate reference clock is
available, a userland process will periodically measure the divergence
between the reference clock and the RTC, set the RTC to the correct
time and step the calibration register in the correct direction to
compensate for the observed drift.
The purpose of this is to reduce the extent to which the RTC will
drift when the device is switched off. Obviously it won't be perfect,
as the temperature will be lower when the device is off than when it
is being calibrated, but the crystal's accuracy curve is reasonably
flat across the relevant temperature range.
Since there is no way to distinguish the M41T11 from its many cousins,
I've forked a new rtc-m41t11 driver from rtc-ds1307, stripped some
irrelevant code, cleaned up the rest, and added support for the
calibration register.
In order to access this from userland, I've added two ioctls,
(RTC_SPEED_UP and RTC_SPEED_DOWN) with corresponding functions in
drivers/rtc/interface.c (rtc_speed_up() and rtc_speed_down()) and
corresponding slots in rtc_class_ops (speed_up and speed_down).
These ioctls etc. take no parameters: speed_up will increase the speed
of the RTC by some unspecified small amount and slow_down will
decrease it by some unspecified small amount. The application must
simply measure, tweak, wait, measure, tweak, wait etc. until the RTC
is within epsilon of the reference - where epsilon is some number that
takes into account the precision of the RTC and of the reference
clock, system call overhead, etc. Since the RTC usually has a 1s
precision, epsilon is likely to be ±1s in practice.
My hope is that this interface is simple enough and general enough to
be usable by other RTC chips that may also support calibration, and
that both design and code are of sufficiently quality to make it into
the mainline at some point.
Patch follows my sig. Comments of all kinds are welcome.
DES
--
Dag-Erling Smørgrav
Senior Software Developer
Linpro AS - www.linpro.no
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index ff9e35c..07e5182 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -206,6 +206,15 @@ config RTC_DRV_PCF8583
This driver can also be built as a module. If so, the module
will be called rtc-pcf8583.
+config RTC_DRV_M41T11
+ tristate "ST M41T11"
+ depends on RTC_CLASS && I2C
+ help
+ If you say yes here you get support for the ST M41T11 RTC chip.
+
+ This driver can also be built as a module. If so, the module
+ will be called rtc-m41t11.
+
config RTC_DRV_M41T80
tristate "ST M41T80 series RTC"
help
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index d3a33aa..468aa0c 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -28,6 +28,7 @@ obj-$(CONFIG_RTC_DRV_DS1672) += rtc-ds1672.o
obj-$(CONFIG_RTC_DRV_DS1742) += rtc-ds1742.o
obj-$(CONFIG_RTC_DRV_EP93XX) += rtc-ep93xx.o
obj-$(CONFIG_RTC_DRV_ISL1208) += rtc-isl1208.o
+obj-$(CONFIG_RTC_DRV_M41T11) += rtc-m41t11.o
obj-$(CONFIG_RTC_DRV_M41T80) += rtc-m41t80.o
obj-$(CONFIG_RTC_DRV_M48T59) += rtc-m48t59.o
obj-$(CONFIG_RTC_DRV_M48T86) += rtc-m48t86.o
diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
index ad66c6e..189afb4 100644
--- a/drivers/rtc/interface.c
+++ b/drivers/rtc/interface.c
@@ -272,3 +272,43 @@ int rtc_irq_set_freq(struct rtc_device *rtc, struct
rtc_task *task, int freq)
return err;
}
EXPORT_SYMBOL_GPL(rtc_irq_set_freq);
+
+int rtc_speed_up(struct rtc_device *rtc)
+{
+ int err = 0;
+
+ err = mutex_lock_interruptible(&rtc->ops_lock);
+ if (err)
+ return -EBUSY;
+
+ if (!rtc->ops)
+ err = -ENODEV;
+ else if (!rtc->ops->speed_up)
+ err = -EINVAL;
+ else
+ err = rtc->ops->speed_up(rtc->dev.parent);
+
+ mutex_unlock(&rtc->ops_lock);
+ return err;
+}
+EXPORT_SYMBOL_GPL(rtc_speed_up);
+
+int rtc_slow_down(struct rtc_device *rtc)
+{
+ int err = 0;
+
+ err = mutex_lock_interruptible(&rtc->ops_lock);
+ if (err)
+ return -EBUSY;
+
+ if (!rtc->ops)
+ err = -ENODEV;
+ else if (!rtc->ops->slow_down)
+ err = -EINVAL;
+ else
+ err = rtc->ops->slow_down(rtc->dev.parent);
+
+ mutex_unlock(&rtc->ops_lock);
+ return err;
+}
+EXPORT_SYMBOL_GPL(rtc_slow_down);
diff --git a/drivers/rtc/rtc-dev.c b/drivers/rtc/rtc-dev.c
index 005fff3..d35f907 100644
--- a/drivers/rtc/rtc-dev.c
+++ b/drivers/rtc/rtc-dev.c
@@ -223,6 +223,8 @@ static int rtc_dev_i
