Patchwork [v4,1/7] RTC: Remove the logic to update time format when DM bit changed

login
register
mail settings
Submitter Zhang, Yang Z
Date March 19, 2012, 6:13 a.m.
Message ID <A9667DDFB95DB7438FA9D7D576C3D87E09E731@SHSMSX101.ccr.corp.intel.com>
Download mbox | patch
Permalink /patch/147544/
State New
Headers show

Comments

Zhang, Yang Z - March 19, 2012, 6:13 a.m.
Change DM(date mode) and 24/12 control bit don't affect the internal registers. It only indicates what format is using for those registers. So we don't need to update time format when it is modified.

Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
---
 hw/mc146818rtc.c |   10 +---------
 1 files changed, 1 insertions(+), 9 deletions(-)

--
1.7.1
Stefano Stabellini - March 20, 2012, 2:04 p.m.
On Mon, 19 Mar 2012, Zhang, Yang Z wrote:
> Change DM(date mode) and 24/12 control bit don't affect the internal registers. It only indicates what format is using for those registers. So we don't need to update time format when it is modified.

That might be true, but if the user changes format, then issues a read
RTC_SECONDS, isn't he going to get the old format, unless we call
rtc_copy_date here?



> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
> ---
>  hw/mc146818rtc.c |   10 +---------
>  1 files changed, 1 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
> index a46fdfc..9b49cbc 100644
> --- a/hw/mc146818rtc.c
> +++ b/hw/mc146818rtc.c
> @@ -252,15 +252,7 @@ static void cmos_ioport_write(void *opaque, uint32_t addr, uint32_t data)
>                      rtc_set_time(s);
>                  }
>              }
> -            if (((s->cmos_data[RTC_REG_B] ^ data) & (REG_B_DM | REG_B_24H)) &&
> -                !(data & REG_B_SET)) {
> -                /* If the time format has changed and not in set mode,
> -                   update the registers immediately. */
> -                s->cmos_data[RTC_REG_B] = data;
> -                rtc_copy_date(s);
> -            } else {
> -                s->cmos_data[RTC_REG_B] = data;
> -            }
> +            s->cmos_data[RTC_REG_B] = data;
>              rtc_timer_update(s, qemu_get_clock_ns(rtc_clock));
>              break;
>          case RTC_REG_C:
> --
> 1.7.1
>
Zhang, Yang Z - March 21, 2012, 5:47 a.m.
> -----Original Message-----
> From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com]
> Sent: Tuesday, March 20, 2012 10:05 PM
> To: Zhang, Yang Z
> Cc: qemu-devel@nongnu.org; Paolo Bonzini; aliguori@us.ibm.com;
> kvm@vger.kernel.org
> Subject: Re: [Qemu-devel] [PATCH v4 1/7] RTC: Remove the logic to update time
> format when DM bit changed
> 
> On Mon, 19 Mar 2012, Zhang, Yang Z wrote:
> > Change DM(date mode) and 24/12 control bit don't affect the internal registers.
> It only indicates what format is using for those registers. So we don't need to
> update time format when it is modified.
> 
> That might be true, but if the user changes format, then issues a read
> RTC_SECONDS, isn't he going to get the old format, unless we call
> rtc_copy_date here?
You are right. The right fixing is updating the cmos before reading rtc not after changing the format. In my patch, it will update the cmos before reading, so we don't need this logic again. 

best regards
yang

Patch

diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
index a46fdfc..9b49cbc 100644
--- a/hw/mc146818rtc.c
+++ b/hw/mc146818rtc.c
@@ -252,15 +252,7 @@  static void cmos_ioport_write(void *opaque, uint32_t addr, uint32_t data)
                     rtc_set_time(s);
                 }
             }
-            if (((s->cmos_data[RTC_REG_B] ^ data) & (REG_B_DM | REG_B_24H)) &&
-                !(data & REG_B_SET)) {
-                /* If the time format has changed and not in set mode,
-                   update the registers immediately. */
-                s->cmos_data[RTC_REG_B] = data;
-                rtc_copy_date(s);
-            } else {
-                s->cmos_data[RTC_REG_B] = data;
-            }
+            s->cmos_data[RTC_REG_B] = data;
             rtc_timer_update(s, qemu_get_clock_ns(rtc_clock));
             break;
         case RTC_REG_C: