From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25011 invoked by alias); 13 Mar 2003 11:40:10 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 24931 invoked from network); 13 Mar 2003 11:40:09 -0000 Received: from unknown (HELO fw-cam.cambridge.arm.com) (193.131.176.3) by sources.redhat.com with SMTP; 13 Mar 2003 11:40:09 -0000 Received: by fw-cam.cambridge.arm.com; id LAA20748; Thu, 13 Mar 2003 11:40:08 GMT Received: from unknown(172.16.1.2) by fw-cam.cambridge.arm.com via smap (V5.5) id xma019925; Thu, 13 Mar 03 11:39:04 GMT Received: from pc960.cambridge.arm.com (pc960.cambridge.arm.com [10.1.205.4]) by cam-admin0.cambridge.arm.com (8.9.3/8.9.3) with ESMTP id LAA02278; Thu, 13 Mar 2003 11:39:04 GMT Received: from pc960.cambridge.arm.com (rearnsha@localhost) by pc960.cambridge.arm.com (8.11.6/8.9.3) with ESMTP id h2DBd3t23717; Thu, 13 Mar 2003 11:39:03 GMT Message-Id: <200303131139.h2DBd3t23717@pc960.cambridge.arm.com> X-Authentication-Warning: pc960.cambridge.arm.com: rearnsha owned process doing -bs To: Vadim Lebedev cc: gdb-patches@sources.redhat.com, Richard.Earnshaw@arm.com Reply-To: Richard.Earnshaw@arm.com Organization: ARM Ltd. X-Telephone: +44 1223 400569 (direct+voicemail), +44 1223 400400 (switchbd) X-Fax: +44 1223 400410 X-Address: ARM Ltd., 110 Fulbourn Road, Cherry Hinton, Cambridge CB1 9NJ. Subject: Re: patch to add semihosting control for remote arm targets In-reply-to: Your message of "Wed, 12 Mar 2003 13:07:42 +0100." <200303121207.h2CC7gJ09861@goldunix1001.propagation.net> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Date: Thu, 13 Mar 2003 11:40:00 -0000 From: Richard Earnshaw X-SW-Source: 2003-03/txt/msg00301.txt.bz2 > > When debugging remote arm targets using devices such as JEENI from epitools > the semihosting function slows execution by the target of SWI instruction > nearly 100-fold. This is especially annoying when you try to debug linux > kernel on the target as it uses SWI to implement system calls. > The attached patch adds a set rdisemihosting {on/off/1/0/true/false} command > to the gdb which allows one to enable or disable semihosting suppport. > the command shoud be executed before target rdi .... command > The patch is against gdb 5.3 but i think will apply to older versions cleanly > too. > > The idea is a good one, but there are several problems with this patch. 1) The formatting does not conform to GNU coding standards. (use gdb_indent.sh) 2) I think we should have a "set rdi" command for the two rdi features (in the same way that we have set debug). Heartbeat and semihosting should be sub-variables of that command. 3) Ideally, it should be possible to turn off semihosting after we have connected to the target. Do you have a copyright assignment on file? Other comments are interspersed below. > Wed Mar 12 13:00:00 2003 Vadim Lebedev (vadim at 7chips.com) > > * remote-rdi.c add set rdisemihosting command > Formatting. Please follow ChangeLog conventions. > +/* target has semihosting enabled */ Formatting. Capital letter at the start. Full stop and two spaces at the end before the close-comment. > if (rslt != RDIError_NoError) > - { > - printf_filtered ("RDI_info: %s\n", rdi_error_message (rslt)); > - } > + { > + printf_filtered ("RDI_info: %s\n", rdi_error_message (rslt)); > + } Indentation -- why have you changed it? > + > + arg1 = rdi_semihosting ? 1 : 0; > + > + rslt = angel_RDI_info(RDISemiHosting_SetState, &arg1, &arg2); > + if (rslt != RDIError_NoError) > + { > + printf_filtered ("RDI_info: %s\n", rdi_error_message (rslt)); > + } Indentation. > > + > arg1 = (unsigned long) ""; > rslt = angel_RDI_info (RDISet_Cmdline, &arg1, &arg2); > if (rslt != RDIError_NoError) > @@ -1051,14 +1064,24 @@ _initialize_remote_rdi (void) > &setlist, &showlist); > > add_setshow_boolean_cmd > - ("rdiheartbeat", no_class, &rdi_heartbeat, > - "Set enable for ADP heartbeat packets.\n" > - "I don't know why you would want this. If you enable them,\n" > - "it will confuse ARM and EPI JTAG interface boxes as well\n" > - "as the Angel Monitor.\n", > - "Show enable for ADP heartbeat packets.\n", > - NULL, NULL, > - &setlist, &showlist); > + ("rdiheartbeat", no_class, &rdi_heartbeat, > + "Set enable for ADP heartbeat packets.\n" > + "I don't know why you would want this. If you enable them,\n" > + "it will confuse ARM and EPI JTAG interface boxes as well\n" > + "as the Angel Monitor.\n", > + "Show enable for ADP heartbeat packets.\n", > + NULL, NULL, > + &setlist, &showlist); Indentation. > + > + add_setshow_boolean_cmd > + ("rdisemihosting", no_class, &rdi_semihosting, > + "Set semihosting support.\n" > + "A true value activates semihosting false value deactivates it.\n", Unnecessary repetition. "A true value activates semihosting.\n" is sufficient. > + "Show enable for semihosting.\n", > + NULL, NULL, > + &setlist, &showlist); > + > + Indetation -- delete unnecessary blank lines. R.