Re: [PATCH v6 2/5] gdb: Add OpenRISC or1k and or1knd target support

[Stafford Horne <shorne at gmail dot com>]

On Tue, May 02, 2017 at 03:32:53PM +0100, Yao Qi wrote:
> Stafford Horne <shorne@gmail.com> writes:
> 
> > +@kindex target remote
> > +@item target remote
> > +
> > +Connect to a remote OpenRISC 1000 target.  This is supported by
> > +@dfn{Or1ksim}, the OpenRISC 1000 architectural simulator as well as QEMU,
> > +Verilator and Icarus Verilog simulators.  @dfn{Remote serial protocol}
> > +servers, such as OpenOCD, are also available to drive various hardware
> > +implementations via JTAG.
> > +
> > +Example: @code{target remote :51000}
> 
> Do we need to document "target remote"? which is not specific to any
> particular configurations.

I understand this is nothing new, but this tells to the users some extra
details about using target remote for OpenRISC (example which platforms
support it).  Also, this is consistent with some other targets like
Microblaze.

> > +
> > +@kindex target sim
> > +@item target sim
> > +
> > +Runs the builtin CPU simulator which can run very basic
> > +programs but does not support most hardware functions like MMU.
> > +For more complex use cases the user is advised to run an external
> > +target, and connect using @samp{target remote}.
> > +
> > +Example: @code{target sim}
> > +
> > +@end table
> > +
> >  @node PowerPC Embedded
> >  @subsection PowerPC Embedded
> >  
> > @@ -41088,6 +41131,7 @@ registers using the capitalization used in the description.
> >  * M68K Features::
> >  * NDS32 Features::
> >  * Nios II Features::
> > +* OpenRISC 1000 Features::
> >  * PowerPC Features::
> >  * S/390 and System z Features::
> >  * Sparc Features::
> > @@ -41374,6 +41418,32 @@ targets.  It should contain the 32 core registers (@samp{zero},
> >  @samp{pc}, and the 16 control registers (@samp{status} through
> >  @samp{mpuacc}).
> >  
> > +@node OpenRISC 1000 Features
> > +@subsection Openrisc 1000 Features
> > +@cindex target descriptions, OpenRISC 1000 features
> > +
> > +The @samp{org.gnu.gdb.or1k.group0} feature is required for OpenRISC 1000
> > +targets.  It should contain the 32 general purpose registers (@samp{r0}
> > +through @samp{r31}), @samp{ppc}, @samp{npc} and @samp{sr}.
> > +
> > +Along with the default reggroups like @samp{system} and @samp{general}
> > +provided by @value{GDBN}, OpenRISC targets can use the following reggroups
> > +to group their many registers:
> > +
> > +@smallexample
> > + Group      Type
> > + immu       user
> > + dmmu       user
> > + icache     user
> > + dcache     user
> > + pic        user
> > + timer      user
> > + power      user
> > + perf       user
> > + mac        user
> > + debug      user
> > +@end smallexample
> > +
> 
> Why do you need to document the reggroups?

These register groups can be used by the target description features.  If
not documented one would have to look into the code.  In general arbitrary
groups are not allowed by features.  This is also related to patch 1/5.

> > +
> > +extern initialize_file_ftype _initialize_or1k_tdep; /* -Wmissing-prototypes */
> > +
> 
> This is no longer needed, because GDB is moved to C++.
> 
> Otherwise, the code is good to me.
> 
> -- 
> Yao (齐尧)