RFA: Blackfin Binutils Port Submission

[Nick Clifton]

Hi Catherine,

> Please let me know if it's okay to commit.

It is.  (Very sorry for taking so long to review this patch).

There are a couple of points however:

> 	* config.sub (bfin): New.
> 	* configure.in (bfin-*-*): New.

Changes to the toplevel configure.in file should be approved by the gdb 
and gcc projects as well, and then applied to both the sourceware 
version and the gcc version.  Changes to the config.sub file need to be 
submitted to the config project <config-patches@gnu.org> for approval.

> 	* Makefile.in: Regenerated.
> 	* aclocal.m4: Regenerated.

There is no need to include regenerated files in the actual patch 
itself.  It just takes up needless room.

---

I also have a few minor comments about pieces of the patch itself:

 > bfd/reloc.c
 > + ENUMDOC
 > +   ADI Blackfin 16 bit immediate absolute reloc

Please could you add a full stop to the end of comments.  It helps to 
make the generated comment slightly more conformant to the GNU Coding 
standard.

 > + /* ADI Blackfin 16 bit immediate absolute reloc  */

---

Also in a couple of places you have an unnecessary capitalized second 
letter in a comment, eg:

 > +   ADI Blackfin 16 bit immediate absolute reloc HIgher 16 bits

This should really be:

   +   ADI Blackfin 16 bit immediate absolute reloc Higher 16 bits

or maybe even:

   +   ADI Blackfin 16 bit immediate absolute reloc higher 16 bits

unless the word "Higher" has particular significance to the Blackfin port ?

---

 > bfd/cpu-bfin.c
 > +   ADI Blackfin 16 bit immediate absolute reloc HIgher 16 bits

Please could the comments in this file be converted to conform the GNU 
Coding Standard ?  ie with a capitial first letter, a terminating full 
stop and a couple of spaces before the closing */.

In fact as a general point, all of the comments in your patch should be 
checked to make sure that they conform to the standard.

---

 > bfd/elf32-bfin.c+ static bfd_reloc_status_type bfin_bfd_reloc
 > +   PARAMS ((bfd *, arelent *, asymbol *, PTR, asection *, bfd *, 
char **));

Since we are now only coding for ISO C90 conformance there is no need to 
use the PARAMS macro, and in fact its use is discouraged.  In fact most 
of the static function prototypes in this file can be removed providing 
that you reorganise the code so that a static function is defined before 
it is used.

---

 > gas/config/bfin-aux.h
 > + /* bfin-aux.h ADI Blackfin Header file for gas^M

It appears that this file contains extraneous carriage returns at the 
end of every line.  (Explicitly shown above as ^M).


Cheers
   Nick