From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22747 invoked by alias); 20 May 2014 14:15:23 -0000 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 Received: (qmail 22736 invoked by uid 89); 20 May 2014 14:15:22 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.1 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 X-HELO: na01-bn1-obe.outbound.protection.outlook.com Received: from mail-bn1blp0188.outbound.protection.outlook.com (HELO na01-bn1-obe.outbound.protection.outlook.com) (207.46.163.188) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Tue, 20 May 2014 14:15:19 +0000 Received: from BL2FFO11FD027.protection.gbl (10.173.160.33) by BL2FFO11HUB005.protection.gbl (10.173.160.225) with Microsoft SMTP Server (TLS) id 15.0.949.9; Tue, 20 May 2014 14:15:15 +0000 Received: from xsj-gw1 (149.199.60.83) by BL2FFO11FD027.mail.protection.outlook.com (10.173.161.106) with Microsoft SMTP Server id 15.0.949.9 via Frontend Transport; Tue, 20 May 2014 14:15:15 +0000 Received: from unknown-38-66.xilinx.com ([149.199.38.66] helo=xsj-smtp1) by xsj-gw1 with esmtp (Exim 4.63) (envelope-from ) id 1Wmkp1-0002Rd-8J; Tue, 20 May 2014 07:15:15 -0700 From: Ajit Kumar Agarwal To: Joel Brobecker CC: "gdb-patches@sourceware.org" , Michael Eager , Vinod Kathail , Vidhumouli Hunsigida , Nagaraju Mekala Subject: RE: [Patch, Microblaze]: Added support of shr and slr regs, little endian breakpoints,backtrace support and communicate with larger blocks. Date: Tue, 20 May 2014 14:15:00 -0000 References: <57db2c05-1b1a-472b-9182-5ce078e8805a@BY2FFO11FD055.protection.gbl> <20140520131332.GF22822@adacore.com> Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-RCIS-Action: ALLOW Message-ID: X-EOPAttributedMessage: 0 X-Forefront-Antispam-Report: CIP:149.199.60.83;CTRY:US;IPV:NLI;IPV:NLI;EFV:NLI;SFV:NSPM;SFS:(6009001)(438001)(377424004)(54534003)(189002)(199002)(51704005)(80022001)(64706001)(44976005)(74316001)(92726001)(20776003)(47776003)(31966008)(74662001)(74502001)(83072002)(85852003)(97756001)(23726002)(33646001)(50986999)(21056001)(551934003)(76482001)(19580405001)(54356999)(77982001)(19580395003)(86362001)(83322001)(99396002)(50466002)(4396001)(70736001)(76176999)(46406003)(46102001)(81542001)(87936001)(92566001)(79102001)(2656002)(31696002)(81342001)(53416003)(142933001)(23106003);DIR:OUT;SFP:;SCL:1;SRVR:BL2FFO11HUB005;H:xsj-gw1;FPR:;MLV:sfv;PTR:unknown-60-83.xilinx.com;MX:1;A:1;LANG:en; X-OriginatorOrg: xilinx.onmicrosoft.com X-Forefront-PRVS: 02176E2458 Received-SPF: Pass (: domain of xilinx.com designates 149.199.60.83 as permitted sender) receiver=; client-ip=149.199.60.83; helo=xsj-gw1; Authentication-Results: spf=pass (sender IP is 149.199.60.83) smtp.mailfrom=ajit.kumar.agarwal@xilinx.com; X-SW-Source: 2014-05/txt/msg00421.txt.bz2 Sorry for resending again as earlier mail has company disclaimer info so th= at it wont be sent to gdb-patches list. Hello Joel: > This patch added the support of shr and slr regs and little endian=20 > breakpoints, backtrace support without debug information and=20 > communicate larger blocks with debugging agent. >>Thanks for the patch. My first comment is that you need to split the patc= hes into small individual patches instead of submitting everything in one b= ulk patch. This will facilitate review, >>thus increasing the chances of ge= tting this patch in sooner rather than later. And it's also a good principl= e 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. >=20 > ChangeLog: > 2014-05-20 Ajit Agarwal >=20 > * 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!! >=20 > * 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. >=20 > * 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. >=20 > * gdb/regformats/reg-microblaze.dat: New Register Data files for Micro= blaze. >>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-td= ep.h none of the lines exceed the 80 characters or not even 74 characters?= =20 =20 >>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 fro= m the GNU Coding Style, can take a while to learn. -- Joel