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

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

Comments

Zhang, Yang Z - March 2, 2012, 6:59 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
Marcelo Tosatti - March 9, 2012, 1:35 a.m.
On Fri, Mar 02, 2012 at 06:59:04AM +0000, 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.
> 
> 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:

Even though its not correct accordingly to the manual, guests can update
format and expect it to be reflected to time byte reads.

commit 51e08f3e4b8a3b6d27fde9a9e75c8fa32eaa72d0
Author: Aurelien Jarno <aurelien@aurel32.net>
Date:   Tue Jan 25 11:55:15 2011 +0100

mc146818rtc: update registers after a format change
    
For some unknown reason, the MIPS kernel briefly changes the RTC to
binary mode during boot, switch back to BCD mode and read the time. As
the registers are updated only every second, they may still be in the
old format when they are read.

This patch forces a register update immediately after a format
change (BCD/binary or 12/24H). This avoid long fsck during boot due to
time wrap.
    
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
Zhang, Yang Z - March 9, 2012, 1:54 a.m.
> -----Original Message-----
> From: Marcelo Tosatti [mailto:mtosatti@redhat.com]
> Sent: Friday, March 09, 2012 9:36 AM
> > ---
> >  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:
> 
> Even though its not correct accordingly to the manual, guests can update
> format and expect it to be reflected to time byte reads.
> 
> commit 51e08f3e4b8a3b6d27fde9a9e75c8fa32eaa72d0
> Author: Aurelien Jarno <aurelien@aurel32.net>
> Date:   Tue Jan 25 11:55:15 2011 +0100
> 
> mc146818rtc: update registers after a format change
> 
> For some unknown reason, the MIPS kernel briefly changes the RTC to
> binary mode during boot, switch back to BCD mode and read the time. As
> the registers are updated only every second, they may still be in the
> old format when they are read.
> 
> This patch forces a register update immediately after a format
> change (BCD/binary or 12/24H). This avoid long fsck during boot due to
> time wrap.
> 
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> 
This is obvious a wrong fixing. Even for real RTC, changing date mode will not change the internal time register. So the MIPS kernel also should not boot up with real RTC. But it only fail in qemu, this means he not found the culprit. This just a workaround.


best regards
yang
Zhang, Yang Z - March 9, 2012, 1:58 a.m.
I think the better fixing is to update the cmos before reading the RTC. And in my patch, it will do it.

best regards
yang


> -----Original Message-----
> From: Zhang, Yang Z
> Sent: Friday, March 09, 2012 9:54 AM
> To: Marcelo Tosatti
> Cc: qemu-devel@nongnu.org; Jan Kiszka; kvm@vger.kernel.org;
> aliguori@us.ibm.com; Paolo Bonzini
> Subject: RE: [PATCH v3 1/7] RTC: Remove the logic to update time format when
> DM bit changed
> 
> > -----Original Message-----
> > From: Marcelo Tosatti [mailto:mtosatti@redhat.com]
> > Sent: Friday, March 09, 2012 9:36 AM
> > > ---
> > >  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:
> >
> > Even though its not correct accordingly to the manual, guests can update
> > format and expect it to be reflected to time byte reads.
> >
> > commit 51e08f3e4b8a3b6d27fde9a9e75c8fa32eaa72d0
> > Author: Aurelien Jarno <aurelien@aurel32.net>
> > Date:   Tue Jan 25 11:55:15 2011 +0100
> >
> > mc146818rtc: update registers after a format change
> >
> > For some unknown reason, the MIPS kernel briefly changes the RTC to
> > binary mode during boot, switch back to BCD mode and read the time. As
> > the registers are updated only every second, they may still be in the
> > old format when they are read.
> >
> > This patch forces a register update immediately after a format
> > change (BCD/binary or 12/24H). This avoid long fsck during boot due to
> > time wrap.
> >
> > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> >
> This is obvious a wrong fixing. Even for real RTC, changing date mode will not
> change the internal time register. So the MIPS kernel also should not boot up
> with real RTC. But it only fail in qemu, this means he not found the culprit. This
> just a workaround.
> 
> 
> 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: