From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13464 invoked by alias); 25 Sep 2009 09:15:13 -0000 Received: (qmail 13453 invoked by uid 22791); 25 Sep 2009 09:15:11 -0000 X-SWARE-Spam-Status: No, hits=0.4 required=5.0 tests=AWL,BAYES_00,J_CHICKENPOX_14,RCVD_IN_JMF_BL,SPF_SOFTFAIL X-Spam-Check-By: sourceware.org Received: from mtaout3.012.net.il (HELO mtaout3.012.net.il) (84.95.2.7) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 25 Sep 2009 09:15:03 +0000 Received: from conversion-daemon.i_mtaout3.012.net.il by i_mtaout3.012.net.il (HyperSendmail v2004.12) id <0KQI00J00S9LZ300@i_mtaout3.012.net.il> for gdb-patches@sourceware.org; Fri, 25 Sep 2009 12:15:00 +0300 (IDT) Received: from HOME-C4E4A596F7 ([87.70.48.81]) by i_mtaout3.012.net.il (HyperSendmail v2004.12) with ESMTPA id <0KQI00ACOSCZ0X50@i_mtaout3.012.net.il>; Fri, 25 Sep 2009 12:15:00 +0300 (IDT) Date: Fri, 25 Sep 2009 09:15:00 -0000 From: Eli Zaretskii Subject: Re: [PATCH] Add support for Xilinx MicroBlaze In-reply-to: <4ABA69CE.5040707@eagercon.com> To: Michael Eager Cc: gdb-patches@sourceware.org Reply-to: Eli Zaretskii Message-id: <83y6o3tmjl.fsf@gnu.org> References: <4ABA69CE.5040707@eagercon.com> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2009-09/txt/msg00782.txt.bz2 > Date: Wed, 23 Sep 2009 11:32:46 -0700 > From: Michael Eager > > 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. > * 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, > + µblaze_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.