Re: PATCH: Support armv6-m in libgloss

[Jeff Johnston <jjohnstn at redhat dot com>]

On 07/04/2011 05:36 AM, Bin Cheng wrote:
> > -----Original Message-----
> > From: newlib-owner@sourceware.org [mailto:newlib-owner@sourceware.org]
> > On Behalf Of Can Finner
> > Sent: Thursday, June 30, 2011 9:54 AM
> > To: newlib@sources.redhat.com
> > Cc: Bin Cheng; nickc@redhat.com
> > Subject: Re: PATCH: Support armv6-m in libgloss
> >
> > > Hi,
> > > The attached patch supports armv6-m in libgloss/arm, which is thumb
> > only
> > > arch and have not been supported in libgloss yet.
> > > This patch makes following changes:
> > >
> > > -   arm unified assembly language(UAL) is now used globally in
> > libgloss,
> > > rather than in thumb2 startup codes only;
> > > -   use flag setting variant of some instructions, this is required
> > by
> > > rewriting pre-UAL to UAL codes;
> > >      Though it might change the behavior of some of these instructions,
> > it
> > > won't change program behavior, since all instructions depend on flags
> > have
> > > its own direct preceding flag setting instructions.
> > > -   change in functionality for AngelSWI_Reason_HeapInfo semihosting,
> > we no
> > > longer set sp/sl if values loaded are ZERO. Please refer to comments
> > at line
> > > 127 of this patch and section 8.3.9 of ARM document
> > > "DUI0203J_rvct_developer_guide" for more information;
> > > -   fix a semihosting bug in crt0.S involving armv6-m/armv7-m, please
> > see
> > > line 106-117 of this patch, and go to website
> > > http://sources.redhat.com/ml/newlib/2011/msg00240.html and
> > > http://sources.redhat.com/ml/newlib/2011/msg00257.html for more
> > discussions;
> > > I have run arm-none-eabi-gcc/g++/gdb "make check" on
> > > arm9/arm9_thumb/cortex-m3, the results seems sane. Also I ran some
> > > benchmarks on cortex-m0/m3 boards successfully.
> > >
> > > Please review this patch and apply it if OK.
> > >
> > >
> > > 2011-06-24  Chengbin<bin.cheng@arm.com>
> > >
> > > libgloss/
> > >                 * arm/crt0.S: Support armv6-m processors in libgloss.
> > >                 * arm/swi.h: Likewise.
> > >                 * arm/trap.S: Likewise.
> > >                 * arm/redboot-crt0.S: Likewise.
> > >                 * arm/linux-crt0.c: Likewise.
> > >                 * arm/arm.h: New.
> >
> > Hi,
> > Could somebody help review this patch?
> >
> > BTW, the arm.com email address wasn't subscribed in mail list before,
> > so have to reply with this address to keep the messages in sequence.
> > Sorry for disturbing.
>
> Hi,
>
> I sent the message and patch about one week ago and got no comment on it. As
> for the patch, should I do anything else to get it reviewed and approved?
> More generally, I am new to newlib list, and wondering there are some rules
> on how to contribute, how patches get reviewed, etc.?
> Your advice would be very helpful. Thanks very much.
>
> Regards.

Hi Bin,

Generally you post to the mailing list.  In most cases, Corinna and 
myself review patches and there are a number of others who are 
well-versed in many areas of newlib who will comment when they see 
issues or have concerns.

In this case, I am not an arm expert and the code is shared by a large 
number of configurations so it is real easy to cause breakage.  This 
change modifies code in the shared path so I defer to the arm experts to 
comment.  Nick is one of them and you cc'd him from the start.

When a patch receives no comment, the right thing to do is what you have 
done and to ping the list.  CC'ing others who have committed changes in 
the same area is a good idea as well which you have done.

I will try to contact Nick directly and ask if he is available to review 
the patch.

Regards,

-- Jeff J.

>