> On 2010-08-14 10:57:53, Ali Saidi wrote: > > src/arch/arm/linux/atag.hh, line 51 > > <http://reviews.m5sim.org/r/165/diff/1/?file=1840#file1840line51> > > > > The max size can be arbitrarily large, so that doesn't work > > particularly well
I don't necessarily agree, but at least it should be a uint8_t * or void * that gets cast to a structure. Just knowing it's a bunch of uint32_ts (and according to that comment it isn't actually?) and knowing some value is at offset such and such is a little too magical. > On 2010-08-14 10:57:53, Ali Saidi wrote: > > src/arch/arm/linux/atag.hh, line 111 > > <http://reviews.m5sim.org/r/165/diff/1/?file=1840#file1840line111> > > > > It's special for each type. It doesn't seem necessary to create a bunch > > of AtagXXXId that is assigned to each. > > The comment below actually already has constants defined just like that, but then they aren't used. If you move that out of the comment and just make it an enum at the top of the file, you haven't net added anything and it'd be a lot clearer. > On 2010-08-14 10:57:53, Ali Saidi wrote: > > src/arch/arm/linux/atag.hh, line 162 > > <http://reviews.m5sim.org/r/165/diff/1/?file=1840#file1840line162> > > > > Perhaps one should read the entire file before beginning to comment on > > it. ;) But one probably won't and shouldn't have to since it's an easy problem to fix. - Gabe ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/165/#review201 ----------------------------------------------------------- On 2010-08-13 09:44:46, Ali Saidi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/165/ > ----------------------------------------------------------- > > (Updated 2010-08-13 09:44:46) > > > Review request for Default. > > > Summary > ------- > > ARM: Add system for ARM/Linux and bootstrapping > > > Diffs > ----- > > src/arch/arm/ArmSystem.py 3c48b2b3cb83 > src/arch/arm/SConscript 3c48b2b3cb83 > src/arch/arm/faults.cc 3c48b2b3cb83 > src/arch/arm/linux/atag.hh PRE-CREATION > src/arch/arm/linux/system.hh PRE-CREATION > src/arch/arm/linux/system.cc PRE-CREATION > src/arch/arm/system.cc 3c48b2b3cb83 > > Diff: http://reviews.m5sim.org/r/165/diff > > > Testing > ------- > > > Thanks, > > Ali > > _______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
