[U-Boot] [PATCH 1/1] socfpga: Consolidating reset code into reset_manager.c. Also separating reset configuration for virtual target and real hardware Cyclone V development kit

[Chin Liang See]

Hi Pavel,

On Mon, 2013-07-01 at 12:46 +0200, ZY - pavel wrote:
> Hi!
> 
> > > > @@ -21,6 +21,7 @@
> > > >  void reset_cpu(ulong addr);
> > > >  void reset_deassert_peripherals_handoff(void);
> > > > 
> > > > +#if defined(CONFIG_SOCFPGA_VIRTUAL_TARGET)
> > > >  struct socfpga_reset_manager {
> > > >      u32    padding1;
> > > >      u32    ctrl;
> > > > @@ -31,7 +32,23 @@ struct socfpga_reset_manager {
> > > >      u32    per2_mod_reset;
> > > >      u32    brg_mod_reset;
> > > >  };
> > > > +#else
> > > > +struct socfpga_reset_manager {
> > > > +    u32    status;
> > > > +    u32    ctrl;
> > > > +    u32    counts;
> > > > +    u32    padding1;
> > > > +    u32    mpu_mod_reset;
> > > > +    u32    per_mod_reset;
> > > > +    u32    per2_mod_reset;
> > > > +    u32    brg_mod_reset;
> > > > +};
> > > > +#endif
> > > > 
> > > 
> > > Is it really needed to have two definitions of the struct? AFAICT,
> > > structures are same, except that some padding fields have names on
> > > real hardware. Thus, if we simply use "real-hardware" version on the
> > > emulator, it should work. Perhaps with some comments "this is not
> > > emulated on virtual target"...?
> > 
> > We decided to leave the Virtual Platform code support within existing
> > code. We need to do that as we have some discrepancy between the real
> > hardware and the virtual platform. But this is only applicable for
> > Altera specific IP. :)
> 
> That is okay... But notice that structure is same on both real
> hardware and virtual platform... (Just some fields have "paddingX"
> instead of name on virtual platform). If you remove the #ifdef it will
> work just fine.
> 
> (You could add /* this is unimplemented on virtual platform */, or
> maybe even per-field ifdef. It will still be more readable.)

Oh.. I got your point now :)
Its a good suggestion and let me do it for next revision.

Chin Liang

> 
> Thanks,
> 									Pavel