Hi Gabe,

I'm probably missing something here, but all the mucking about with TZ
caught my attention as something that would be nice to avoid.
Wouldn't it work to keep curTime as a time_t and just increment it on
every tick event, then call setTime(*gmtime(&curTime)) only when
you're about to touch clock_data in readData() or writeData()?

In fact, you could get rid of the union altogether as a member (but
keep the type) and just create a local instance on each call to
readData() or writeData() that's dynamically populated from the time_t
curTime.  (OK, on closer inspection that's not quite so clean since
you do need to store the *_alrm fields separately.  So it probably is
easier just to keep the member, but treat all the fields other than
*_alrm as temporaries.)

I think you're also missing the step of updating curTime if clock_data
is changed in writeData(), independent of whether curTime is a struct
tm or a time_t.

BTW, I wanted to congratulate you on all the recent x86 progress,,,
this is looking awesome.  I take it you're fixing the RTC because
you're now getting far enough to notice that it's not advancing?

Steve

On Thu, Aug 20, 2009 at 9:20 AM, Gabe Black<[email protected]> wrote:
> changeset ade9a088bb14 in /z/repo/m5
> details: http://repo.m5sim.org/m5?cmd=changeset;node=ade9a088bb14
> description:
>        X86: Make the real time clock actually keep track of time.
>
> diffstat:
>
> 2 files changed, 103 insertions(+), 19 deletions(-)
> src/dev/mc146818.cc |   96 ++++++++++++++++++++++++++++++++++++++++-----------
> src/dev/mc146818.hh |   26 +++++++++++++
>
> diffs (184 lines):
>
> diff -r de112a8ac3d8 -r ade9a088bb14 src/dev/mc146818.cc
> --- a/src/dev/mc146818.cc       Thu Aug 20 00:42:14 2009 -0700
> +++ b/src/dev/mc146818.cc       Thu Aug 20 00:42:43 2009 -0700
> @@ -43,28 +43,20 @@
>
>  using namespace std;
>
> -MC146818::MC146818(EventManager *em, const string &n, const struct tm time,
> -                   bool bcd, Tick frequency)
> -    : EventManager(em), _name(n), event(this, frequency)
> +static uint8_t
> +bcdize(uint8_t val)
>  {
> -    memset(clock_data, 0, sizeof(clock_data));
> -    stat_regA = RTCA_32768HZ | RTCA_1024HZ;
> -    stat_regB = RTCB_PRDC_IE | RTCB_24HR;
> -    if (!bcd)
> -        stat_regB |= RTCB_BIN;
> +    uint8_t result;
> +    result = val % 10;
> +    result += (val / 10) << 4;
> +    return result;
> +}
>
> +void
> +MC146818::setTime(const struct tm time)
> +{
> +    curTime = time;
>     year = time.tm_year;
> -
> -    if (bcd) {
> -        // The datasheet says that the year field can be either BCD or
> -        // years since 1900.  Linux seems to be happy with years since
> -        // 1900.
> -        year = year % 100;
> -        int tens = year / 10;
> -        int ones = year % 10;
> -        year = (tens << 4) + ones;
> -    }
> -
>     // Unix is 0-11 for month, data seet says start at 1
>     mon = time.tm_mon + 1;
>     mday = time.tm_mday;
> @@ -75,6 +67,30 @@
>     // Datasheet says 1 is sunday
>     wday = time.tm_wday + 1;
>
> +    if (!(stat_regB & RTCB_BIN)) {
> +        // The datasheet says that the year field can be either BCD or
> +        // years since 1900.  Linux seems to be happy with years since
> +        // 1900.
> +        year = bcdize(year % 100);
> +        mon = bcdize(mon);
> +        mday = bcdize(mday);
> +        hour = bcdize(hour);
> +        min = bcdize(min);
> +        sec = bcdize(sec);
> +    }
> +}
> +
> +MC146818::MC146818(EventManager *em, const string &n, const struct tm time,
> +                   bool bcd, Tick frequency)
> +    : EventManager(em), _name(n), event(this, frequency), tickEvent(this)
> +{
> +    memset(clock_data, 0, sizeof(clock_data));
> +    stat_regA = RTCA_32768HZ | RTCA_1024HZ;
> +    stat_regB = RTCB_PRDC_IE | RTCB_24HR;
> +    if (!bcd)
> +        stat_regB |= RTCB_BIN;
> +
> +    setTime(time);
>     DPRINTFN("Real-time clock set to %s", asctime(&time));
>  }
>
> @@ -141,6 +157,34 @@
>     }
>  }
>
> +static time_t
> +mkutctime(struct tm *time)
> +{
> +    time_t ret;
> +    char *tz;
> +
> +    tz = getenv("TZ");
> +    setenv("TZ", "", 1);
> +    tzset();
> +    ret = mktime(time);
> +    if (tz)
> +        setenv("TZ", tz, 1);
> +    else
> +        unsetenv("TZ");
> +    tzset();
> +    return ret;
> +}
> +
> +void
> +MC146818::tickClock()
> +{
> +    if (stat_regB & RTCB_NO_UPDT)
> +        return;
> +    time_t calTime = mkutctime(&curTime);
> +    calTime++;
> +    setTime(*gmtime(&calTime));
> +}
> +
>  void
>  MC146818::serialize(const string &base, ostream &os)
>  {
> @@ -190,3 +234,17 @@
>  {
>     return "RTC interrupt";
>  }
> +
> +void
> +MC146818::RTCTickEvent::process()
> +{
> +    DPRINTF(MC146818, "RTC clock tick\n");
> +    parent->schedule(this, curTick + Clock::Int::s);
> +    parent->tickClock();
> +}
> +
> +const char *
> +MC146818::RTCTickEvent::description() const
> +{
> +    return "RTC clock tick";
> +}
> diff -r de112a8ac3d8 -r ade9a088bb14 src/dev/mc146818.hh
> --- a/src/dev/mc146818.hh       Thu Aug 20 00:42:14 2009 -0700
> +++ b/src/dev/mc146818.hh       Thu Aug 20 00:42:43 2009 -0700
> @@ -64,6 +64,23 @@
>         virtual const char *description() const;
>     };
>
> +    /** Event for RTC periodic interrupt */
> +    struct RTCTickEvent : public Event
> +    {
> +        MC146818 * parent;
> +
> +        RTCTickEvent(MC146818 * _parent) : parent(_parent)
> +        {
> +            parent->schedule(this, curTick + Clock::Int::s);
> +        }
> +
> +        /** Event process to occur at interrupt*/
> +        void process();
> +
> +        /** Event description */
> +        const char *description() const;
> +    };
> +
>   private:
>     std::string _name;
>     const std::string &name() const { return _name; }
> @@ -71,6 +88,9 @@
>     /** RTC periodic interrupt event */
>     RTCEvent event;
>
> +    /** RTC tick event */
> +    RTCTickEvent tickEvent;
> +
>     /** Data for real-time clock function */
>     union {
>         uint8_t clock_data[10];
> @@ -89,6 +109,10 @@
>         };
>     };
>
> +    struct tm curTime;
> +
> +    void setTime(const struct tm time);
> +
>     /** RTC status register A */
>     uint8_t stat_regA;
>
> @@ -106,6 +130,8 @@
>     /** RTC read data */
>     uint8_t readData(const uint8_t addr);
>
> +    void tickClock();
> +
>     /**
>       * Serialize this object to the given output stream.
>       * @param base The base name of the counter object.
> _______________________________________________
> m5-dev mailing list
> [email protected]
> http://m5sim.org/mailman/listinfo/m5-dev
>
_______________________________________________
m5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/m5-dev

Reply via email to