[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

[Pavel Machek]

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.)

Thanks,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html