RE: [Patch, Microblaze]: Added support of shr and slr regs, little endian breakpoints,backtrace support and communicate with larger blocks.

[Ajit Kumar Agarwal <ajit dot kumar dot agarwal at xilinx dot com>]

Sorry for resending again as earlier mail has company disclaimer info so that it wont be sent to gdb-patches list.

Hello Joel:

> This patch added the support of shr and slr regs and little endian 
> breakpoints, backtrace support without debug information and 
> communicate larger blocks with debugging agent.

>>Thanks for the patch. My first comment is that you need to split the patches into small individual patches instead of submitting everything in one bulk patch. This will facilitate review, >>thus increasing the chances of getting this patch in sooner rather than later. And it's also a good principle to follow in case we want to revert one part of the change.

Thanks for the review comments. I will split the patch and send it across.
> 
> ChangeLog:
>  2014-05-20 Ajit Agarwal <ajitkum@xilinx.com>
> 
>  * gdb/gdbserver/Makefile.in (microblaze-linux.c) : New target

>>Not sure if this intended or not, but the entry should be indented with a tab. No space before the ':'. And all sentences should end with a period (as well as starting with a capital letter >>as you did here).

The comment here is for the ChangeLog written or the coding comment in the file Makefile.in.  Sorry to ask this!!
> 
>  * gdb/microblaze-tdep.c
>    (microblaze_register_names): Added the rshr and rslr register names.
>    (microblaze_breakpoint_from_pc): Added Declaration of byte_order

>>The indentation for all lines should be one tab, so the subsequent lines should not be over indented.  Please also join the first and second lines, as there is no need to skip to the next >>line here.

Ditto.

>    and break_insn_le. Check of byte order by BFD_ENDIAN_BIG
>    (microblaze_alloc_frame_cache):  Initialize saved_sp
>    (microblaze_analyze_prologue):  Do a block read to minimize the
>    transaction with debug agent. Use of target_read_memory. Freeing
>    the block read.
>    (microblaze_frame_cache): use of microblaze_analyze_prologue and
>    assigning the register_offsets.
>    (microblaze_frame_prev_register): Use of frame_unwind_got_constant.
>    Check of regnum with MICROBLAZE_SP_REGNUM ,MICROBLAZE_R19_REGNUM,
>    MICROBLAZE_PC_REGNUM.
> 
>    * gdb/microblaze-tdep.h
>    (microblaze_frame_cache): Addition of fields saved_sp,modification of
>    register_offsets.
>    (microblaze_reg_num): Addition of fields MICROBLAZE_SLR_REGNUM and
>    MICROBLAZE_SHR_REGNUM.
>    (MICROBLAZE_BREAKPOINT_LE): New Macro.
> 
>    * gdb/regformats/reg-microblaze.dat: New Register Data files for Microblaze.

>>Can you also make sure that, once re-indented properly, none of the lines exceed 74 characters? It's a soft limit, so you can exceed it a bit if it helps, but the absolute hard limit is 80 >>characters.

In the above changes in the file gdb/microblaze-tdep.c or gdb/microblaze-tdep.h  none of the lines exceed the 80 characters or not even 74 characters? 
 
>>We'll try to review the patches themselves once they have been split.
I will surely split the patch.

>>I hope I don't sound too hard. The GDB Coding Style, which is derived from the GNU Coding Style, can take a while to learn.


--
Joel