On Sat, 2012-12-22 at 01:00 +0100, Franck Jullien wrote: 
> I'm currently working on GDB updates.
> The idea is to get rid of the readspr/writespr commands to access spr 
> registers.

Hi Franck,

This is a great piece of work. I've taken a first look through your
patches, and here are some comments.

> As a matter of fact those commands use qRcmd RSP packets to get data from
> the GDB client but this is not compliant with the RSP protocol.

> This is not a really big deal when you are working with openrisc specific
> gdb clients versions. However, if we want to get our OpenOCD port to be
> mainstream, we can't use this trick.

Just being picky, but qRcmd is compliant with RSP - the whole point of
qRcmd is to allow the client to make arbitrary communication with the
server. I think the point is that it would be much better if we could
avoid needing custom commands in the server. It also sounds like OpenOCD
only supports a restricted subset of RSP.

Where I have seen qRcmd used is to provide additional commands in the
GDB client to access facilities like flashing an FPGA. Looks like those
architectures would not be able to use OpenOCD.

> This series, implement spr access using p/P RSP commands and registers
> definition using remote target description file.

I am very interested in this, because I'm working on another
architecture with a large auxiliary register set. I hadn't heard of
restricting the range of registers that GDB can access using g/G packets
and using p/P packets for other registers.

Have you run this idea past the GDB mailing list? While GDB generally
uses g/G to cache registers, it sometimes uses p/P as well, so you'll
need to make sure p/P still work with the GPRs. If the GDB community
agree this works, I'd like to add it to the GDB internals manual to help
future implementers of architectures with auxiliary registers.

> This patch keeps compatibility with "classic" gdb clients.
> 
> I already modified OpenOCD to send the target descriptor file and get/set
> spr registers using p/P packets (https://github.com/Franck79/openOCD,
> auto_tdesc branch, work in progress).
> 
> As a matter of fact, OpenOCD generates the target descriptor file at run time
> based on the target registers (we can still tells it tu use an existing file,
> this is just a TCL config).
> So adding a register (and handle it in gdb) is very easy. We could imagine a
> simple TCL command to add a register definition if we don't want to recompile
> OpenOCD nor GBD...

What happens to GDB users who are not connecting to an OpenOCD target?
Where do they get their register description? I see other architectures
providing feature sets in the gdb/features directory. Should there be a
fallback to allow a feature set XML file to be supplied to the client?

One big omission. You haven't provided ChangeLog entries. GNU coding
standards compliant ChangeLog's are an absolute must have if this code
is ever to be committed to the FSF. OpenRISC specific entries should go
into ChangeLog.or32 files, which are merged into the main ChangeLog for
FSF submission.

(I know the argument that git logs replace ChangeLogs, but a) the FSF
set the rules and b) I've yet to see any git log usage with the
discipline of GNU compliant ChangeLogs. The best approach is to write
your ChangeLog entries and paste those into your git log).

On a secondary note, some of your patches don't seem to follow GNU
layout rules for indentation. You can use the "indent" command to apply
GNU rules automatically to your source.

Final thought. How do the regression results compare. If you get any
tests that pass with the old approach and fail with the new, they will
be important to investigate.

Best wishes,


Jeremy

> Franck Jullien (3):
>   target-description: add tdesc/feature functions
>   or32: set gdbarch num regs to gpr + spr regs
>   or32: add target descriptor support
> 
>  gdb/or32-tdep.c           |  160 
> +++++++++++++++++++++++++++++++++++++++++++--
>  gdb/or32-tdep.h           |    4 +-
>  gdb/target-descriptions.c |   69 +++++++++++++++++++
>  gdb/target-descriptions.h |   24 +++++++
>  4 files changed, 251 insertions(+), 6 deletions(-)
> 
> _______________________________________________
> Openrisc mailing list
> [email protected]
> http://lists.opencores.org/listinfo/openrisc

-- 
Tel:      +44 (1590) 610184
Cell:     +44 (7970) 676050
SkypeID: jeremybennett
Email:   [email protected]
Web:     www.embecosm.com

_______________________________________________
OpenRISC mailing list
[email protected]
http://lists.openrisc.net/listinfo/openrisc

Reply via email to