-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2545/#review5565
-----------------------------------------------------------



src/dev/mc146818.hh
<http://reviews.gem5.org/r/2545/#comment4987>

    no need for virtual, perhaps also choose a different name to avoid 
confusion, schedule()?



src/dev/mc146818.hh
<http://reviews.gem5.org/r/2545/#comment4988>

    can also do it in the initialisation list



src/dev/mc146818.hh
<http://reviews.gem5.org/r/2545/#comment4989>

    no need for virtual
    
    again, I would suggest to name it schedule() or similar



src/dev/mc146818.cc
<http://reviews.gem5.org/r/2545/#comment4990>

    An alternative to have methods on the objects would be to simply call 
schedule(event, curTick() + event.getOffset()); or similar


Overall it looks great.

The summary should start with a keyword and be < 65 char. Have a look at 
http://www.gem5.org/Commit_Access. The list is fairly well hidden. In your case 
I would suggest dev.

Also, the check you do in simulate.py should probably be a patch on its own.

- Andreas Hansson


On Nov. 28, 2014, 9:14 p.m., Cagdas Dirik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/2545/
> -----------------------------------------------------------
> 
> (Updated Nov. 28, 2014, 9:14 p.m.)
> 
> 
> Review request for Default, Andrew Bardsley, Andreas Hansson, and Ali Saidi.
> 
> 
> Repository: gem5
> 
> 
> Description
> -------
> 
> This change includes edits to MC146818 timer to prevent RTC events firing 
> before startup to comply with SimObject initialization call sequence.
> 
> 
> Diffs
> -----
> 
>   src/dev/mc146818.hh UNKNOWN 
>   src/dev/mc146818.cc UNKNOWN 
>   src/dev/x86/cmos.hh UNKNOWN 
>   src/dev/x86/cmos.cc UNKNOWN 
>   src/python/m5/simulate.py UNKNOWN 
> 
> Diff: http://reviews.gem5.org/r/2545/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Cagdas Dirik
> 
>

_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to