This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Add support for Xilinx MicroBlaze


Eli Zaretskii wrote:
Date: Wed, 23 Sep 2009 11:32:46 -0700
From: Michael Eager <eager@eagercon.com>

Attached is a revised patch which adds support for the
Xilinx MicroBlaze.  I believe that I've addressed all
of the previous comments.

Thanks. And sorry for the delay ion reviewing your patch.

Thanks for the review. I'll fix these issues and resubmit. Sorry this is taking so much back-and-forth.


   * microblaze-linux-tdep.c: New.
   * microblaze-rom.c: New.
   * microblaze-tdep.c: New.
   * microblaze-tdep.h: New.

Please add the necessary tweaks for these new files to gdb/config/djgpp/fnchange.lst. They will clash after 8+3 truncation.

+@code{xmd} uses port @code{1234}. (While it is possible to change
                                   ^^
Two spaces between sentences, please.

+@item target remote XMD-HOST:1234
+Use this command to connect to the target if it is connected to @code{xmd}
+running on a different system named @code{XMD-HOST}.

Please use "@var{xmd-host}" instead of a literal upper-case XMD-HOST. @var is what should be used in Texinfo for identifiers that stand for something else, in this case an actual host name.

+  /* printf("microblaze_analyze_prologue (pc = 0x%8.8x, "
+	    "current_pc = 0x%8.8x, cache = 0x%8.8x)\n",
+	    (int) pc, (int) current_pc, (int) cache);	*/

I believe we don't like dead code in GDB sources.


+ /* Spill register */

Comments should have a period and 2 blanks at their end.


+	  cache->register_offsets[rd] = imm - cache->framesize;
+		/* reg spilled after updating */

Strange formatting of a comment, and indentation seems wrong.


+	  /* We have a frame pointer.  Note
+	     the register which is acting as the frame pointer. */

Please reformat whitespace here: the first line is broken too early.


+ switch (TYPE_LENGTH(type))

I believe GNU standards call for a space before the left paren.


+      default:
+	printf_filtered("Fatal error: unsupported return value size "
+			"requested (%s @ %d)\n", __FILE__, __LINE__);

Shouldn't you call `fatal' here?


In any case, messages presented to the user should be in _(), to be
translatable (the debug messages are exempt from this rule).

+ /* Debug this files internals. */
+ add_setshow_zinteger_cmd ("microblaze", class_maintenance,
+ &microblaze_debug, _("\
+Set microblaze debugging."), _("\
+Show microblaze debugging."), _("\
+When non-zero, microblaze specific debugging is enabled."),
+ NULL,
+ NULL, + &setdebuglist, &showdebuglist);

This command should be documented in the user manual.


--- gdb/gdb/NEWS	2009-09-06 10:14:42.000000000 -0700
+++ mb-gdb/gdb/NEWS	2009-09-23 10:41:55.000000000 -0700
@@ -442,6 +442,11 @@ Lattice Mico32                  lm32-*
 x86 DICOS			i[34567]86-*-dicos*
 x86_64 DICOS		        x86_64-*-dicos*
 S+core 3			score-*-*
+Xilinx MicroBlaze		microblaze-*-*
+
+* New Simulators
+
+Xilinx MicroBlaze		microblaze

The patch for NEWS is fine.


Thanks.



--
Michael Eager	 eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]