-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/387/#review645
-----------------------------------------------------------



src/dev/intel_8254_timer.cc
<http://reviews.m5sim.org/r/387/#comment866>

    This seems to be a bit different since you're fixing existing serialization 
code instead of adding new code, but it's small enough that it's not a huge 
deal to slip it in there. Ideally you'd separate it out, though.



src/dev/x86/i8042.cc
<http://reviews.m5sim.org/r/387/#comment869>

    __data is a top secret internal component of the bit union mechanism, hence 
it's double leading underscores. I think if you use the macro bare gcc won't 
cooperate, and if you cast it the name gets messed up. You'll have to declare a 
temporary variable and serialize/unserialize that. It's a pain, but it doesn't 
break encapsulation.



src/dev/x86/i8042.cc
<http://reviews.m5sim.org/r/387/#comment870>

    Why aren't you using the SERIALIZE_SCALAR, etc., macros for these? Granted 
macros are gross generally, but using them would be consistent with all the 
other code and fit with what most or all of these lines are doing perfectly.



src/dev/x86/i8042.cc
<http://reviews.m5sim.org/r/387/#comment868>

    This stuff that goes with the base PS2Device class should go in serialize 
functions declared there. Then you won't have two copies of it, and it makes 
the code more modular.



src/dev/x86/i8042.cc
<http://reviews.m5sim.org/r/387/#comment867>

    Use new and delete.



src/dev/x86/i82094aa.cc
<http://reviews.m5sim.org/r/387/#comment871>

    Hmm. Apparently you can cast inside a SERIALIZE_*? If that works that's 
what you should do with the bituions. Cast them to their underlying type (the 
type of __data) and it may all magically work. Or maybe not if it's using a 
reference or something.



src/dev/x86/speaker.cc
<http://reviews.m5sim.org/r/387/#comment872>

    See above about bit unions.


- Gabe


On 2011-01-06 16:00:01, Brad Beckmann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/387/
> -----------------------------------------------------------
> 
> (Updated 2011-01-06 16:00:01)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> -------
> 
> x86: Add checkpointing capability to devices
> 
> Add checkpointing capability to the Intel 8254 timer, CMOS, I8042,
> PS2 Keyboard and Mouse, I82094AA, I8237, I8254, I8259, and speaker
> devices
> 
> 
> Diffs
> -----
> 
>   src/dev/intel_8254_timer.cc 9f9e10967912 
>   src/dev/x86/cmos.hh 9f9e10967912 
>   src/dev/x86/cmos.cc 9f9e10967912 
>   src/dev/x86/i8042.hh 9f9e10967912 
>   src/dev/x86/i8042.cc 9f9e10967912 
>   src/dev/x86/i82094aa.hh 9f9e10967912 
>   src/dev/x86/i82094aa.cc 9f9e10967912 
>   src/dev/x86/i8237.hh 9f9e10967912 
>   src/dev/x86/i8237.cc 9f9e10967912 
>   src/dev/x86/i8254.hh 9f9e10967912 
>   src/dev/x86/i8254.cc 9f9e10967912 
>   src/dev/x86/i8259.hh 9f9e10967912 
>   src/dev/x86/i8259.cc 9f9e10967912 
>   src/dev/x86/speaker.hh 9f9e10967912 
>   src/dev/x86/speaker.cc 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/387/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Brad
> 
>

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

Reply via email to