This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
RE: [Patch] Microblaze: Port of Linux gdbserver
- From: Ajit Kumar Agarwal <ajit dot kumar dot agarwal at xilinx dot com>
- To: Michael Eager <eager at eagerm dot com>, Michael Eager <eager at eagercon dot com>, Pedro Alves <palves at redhat dot com>, Joel Brobecker <brobecker at adacore dot com>
- Cc: "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>, Vinod Kathail <vinodk at xilinx dot com>, Vidhumouli Hunsigida <vidhum at xilinx dot com>, "Nagaraju Mekala" <nmekala at xilinx dot com>
- Date: Mon, 13 Oct 2014 16:00:11 +0000
- Subject: RE: [Patch] Microblaze: Port of Linux gdbserver
- Authentication-results: sourceware.org; auth=none
- Authentication-results: spf=pass (sender IP is 149.199.60.83) smtp dot mailfrom=ajit dot kumar dot agarwal at xilinx dot com;
- References: <25de23b98e054fd291ea232d10f2800c at BN1BFFO11FD018 dot protection dot gbl> <5436B7D0 dot 9060004 at eagercon dot com> <d0a23ff7601a4dbcab91d5b680bc2e13 at BY2FFO11FD017 dot protection dot gbl> <54371D57 dot 2030605 at eagerm dot com>
-----Original Message-----
From: Michael Eager [mailto:eager@eagerm.com]
Sent: Friday, October 10, 2014 5:12 AM
To: Ajit Kumar Agarwal; Michael Eager; Pedro Alves; Joel Brobecker
Cc: gdb-patches@sourceware.org; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
Subject: Re: [Patch] Microblaze: Port of Linux gdbserver
On 10/09/14 11:54, Ajit Kumar Agarwal wrote:
>
> To send the patches after incorporating the comments, Is there any other way of sending the patches without top post?
After you address comments, include the changelog at the end and attach the patch (unless it is just a few lines). That way we can tell how you responded to each comment.
> +#define microblaze_breakpoint_len 4
>
>>> Use CAPS for macros.
>
> The MIPS and the ARM gdbserver code does not use the CAPS for the above macro defined.
>Let's follow the GNU coding standard, even if some other targets haven't.
https://gcc.gnu.org/ml/gcc-patches/2000-09/msg00901.html
https://gcc.gnu.org/ml/gcc-patches/2000-09/msg00912.html
Here is the mailing list archive that mentions when not to use ALL_CAPS for Macros.
> + (*the_target->read_memory) (where, (unsigned char *) &insn, 4);
> +
> + if (insn == microblaze_breakpoint)
>
>>> Why use the explicit length rather than the macro you just defined?
>>> Why not use sizeof (insn)?
>
> To match up with the MIPS target and ARM target they have not used the macro defined. In the Mips 4 is used and in the ARM target for the THUMB_ARM 2 is used and for the ARM Mode code 4 is used.
>Let's follow good coding practice, even if there have been lapses in the past.
Unless there is some particular relevance to instruction length on MIPS or ARM/Thumb, let's stick to what is relevant to MicroBlaze.
Ok.
> Pedro:
>> I'd much prefer if we had that patch in the tree before accepting further patches that tweak things around register names, etc. Could you send that (as an independent patch, in a new thread).
>
>>> Please address issues with previous patches before moving on to submit dependent patches.
>
> I have already send the patch related to the above Pedro's comment. I have also send the patch after incorporating the Pedro feedback comments.
>I haven't seen this patch. Please let me know when you posted it, or send me a link to it in the mailing list archive.
>If you submit a patch which depends on previously submitted patches which have not been accepted, the new patch will not be accepted.
Please don't submit dependent patches until all prior prerequisite patches are accepted.
http://sourceware-org.1504.n7.nabble.com/PATCH-Microblaze-Reject-invalid-target-descriptions-td285201.html
> Pedro:
>> diff --git a/gdb/regformats/microblaze-with-stack-protect.dat
> ...
>> Please send a preparatory, independent, patch that updates
>> features/Makefile instead and generates this file, in a new thread,
>> with self-contained description, following the
>> checklist:
>> https://sourceware.org/gdb/wiki/ContributionChecklist
>
>>> Preparatory means that the patch should be submitted before the current patch.
>
> I will be sending this patch soon.
OK. As mentioned above, please do not resubmit this patch until that patch is submitted and accepted.
http://sourceware-org.1504.n7.nabble.com/PATCH-Microblaze-Replace-microblaze-expedite-from-pc-to-rpc-td285649.html
Thanks & Regards
Ajit
--
Michael Eager eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306 650-325-8077