[Qemu-devel] [PATCH 1/1] mc146818rtc: correct UIP hold length

[Qemu-devel] [PATCH 1/1] mc146818rtc: correct UIP hold length

[James Hogan]

The UIP (update in progress) hold time was set to 8 32.768KHz clock
cycles (around 244uS). However the timing diagram in the datasheet
(Figure 16) shows that the UIP bit is held for both the update cycle
time (either 248uS or 1984uS depending on the clock source), and the
minimum time before update cycle (244uS).

It's clear from periodic_timer_update() that only a 32.768KHz clock
source is expected, so correct the hold time to 244uS + 1984uS = 73
32.768KHz clock cycles.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Andreas Färber <afaerber@suse.de>
Cc: Anthony Liguori <aliguori@us.ibm.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Yang Zhang <yang.z.zhang@intel.com>
---
 hw/timer/mc146818rtc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c
index 3c3baac..6000feb 100644
--- a/hw/timer/mc146818rtc.c
+++ b/hw/timer/mc146818rtc.c
@@ -55,7 +55,7 @@
 
 #define RTC_REINJECT_ON_ACK_COUNT 20
 #define RTC_CLOCK_RATE            32768
-#define UIP_HOLD_LENGTH           (8 * NSEC_PER_SEC / 32768)
+#define UIP_HOLD_LENGTH           (73 * NSEC_PER_SEC / 32768)
 
 #define MC146818_RTC(obj) OBJECT_CHECK(RTCState, (obj), TYPE_MC146818_RTC)
 
@@ -597,7 +597,7 @@ static int update_in_progress(RTCState *s)
     }
 
     guest_nsec = get_guest_rtc_ns(s);
-    /* UIP bit will be set at last 244us of every second. */
+    /* UIP bit will be set at last 1984us + 244us of every second. */
     if ((guest_nsec % NSEC_PER_SEC) >= (NSEC_PER_SEC - UIP_HOLD_LENGTH)) {
         return 1;
     }
-- 
1.8.1.2

Re: [Qemu-devel] [PATCH 1/1] mc146818rtc: correct UIP hold length

[Paolo Bonzini]

Il 14/08/2013 16:10, James Hogan ha scritto:
> The UIP (update in progress) hold time was set to 8 32.768KHz clock
> cycles (around 244uS). However the timing diagram in the datasheet
> (Figure 16) shows that the UIP bit is held for both the update cycle
> time (either 248uS or 1984uS depending on the clock source), and the
> minimum time before update cycle (244uS).
> 
> It's clear from periodic_timer_update() that only a 32.768KHz clock
> source is expected, so correct the hold time to 244uS + 1984uS = 73
> 32.768KHz clock cycles.

I am not sure if this time would actually go from t-244us to t+1984us on
real hardware?  You could measure this using a divider reset and
sampling PF and UF.

The emulation right now does "instant" updates, which elegantly
sidesteps the problem. :)

> Signed-off-by: James Hogan <james.hogan@imgtec.com>
> Cc: Andreas Färber <afaerber@suse.de>
> Cc: Anthony Liguori <aliguori@us.ibm.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Yang Zhang <yang.z.zhang@intel.com>
> ---
>  hw/timer/mc146818rtc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c
> index 3c3baac..6000feb 100644
> --- a/hw/timer/mc146818rtc.c
> +++ b/hw/timer/mc146818rtc.c
> @@ -55,7 +55,7 @@
>  
>  #define RTC_REINJECT_ON_ACK_COUNT 20
>  #define RTC_CLOCK_RATE            32768
> -#define UIP_HOLD_LENGTH           (8 * NSEC_PER_SEC / 32768)
> +#define UIP_HOLD_LENGTH           (73 * NSEC_PER_SEC / 32768)
>  
>  #define MC146818_RTC(obj) OBJECT_CHECK(RTCState, (obj), TYPE_MC146818_RTC)
>  
> @@ -597,7 +597,7 @@ static int update_in_progress(RTCState *s)
>      }
>  
>      guest_nsec = get_guest_rtc_ns(s);
> -    /* UIP bit will be set at last 244us of every second. */
> +    /* UIP bit will be set at last 1984us + 244us of every second. */
>      if ((guest_nsec % NSEC_PER_SEC) >= (NSEC_PER_SEC - UIP_HOLD_LENGTH)) {
>          return 1;
>      }
>