On Monday 25 June 2012 14:02:38 Dmitry Bondar wrote:
> C6X (C6000) is Texas Instruments architecture of fixed and floating-point
> DSPs. This patch add basic support.
> Many of code in arch/c6x/include/asm come from c6x-linux project
> (http://linux-c6x.org)

you should also update README and doc/README.standalone with some notes about 
the C6X architecture.  just search for "blackfin" for the areas you should fill 
in.

> --- /dev/null
> +++ b/arch/c6x/config.mk
> @@ -0,0 +1 @@
> +CROSS_COMPILE ?= c6x-uclinux-

missing intro comment block w/copyright/licnese.  look at any other arch's 
config.mk to see an example.

you should prob also add a default CONFIG_STANDALONE_LOAD_ADDR

> --- /dev/null
> +++ b/arch/c6x/include/asm/global_data.h
>
> +#ifndef      __ASM_GBL_DATA_H

should be space after the ifndef, not a tab

> +typedef      struct  global_data {
> +     bd_t            *bd;
> +     unsigned long   flags;
> +     unsigned long   baudrate;
> +     unsigned long   have_console;   /* serial_init() was called */
> +#ifdef CONFIG_PRE_CONSOLE_BUFFER
> +     unsigned long   precon_buf_idx; /* Pre-Console buffer index */
> +#endif
> +     unsigned long   env_addr;       /* Address  of Environment struct */
> +     unsigned long   env_valid;      /* Checksum of Environment valid? */
> +     unsigned long   fb_base;        /* base address of frame buffer */
> +
> +     /* "static data" needed by most of timer.c (like on ARM platform) */
> +     unsigned long   timer_rate_hz;
> +     unsigned long   tbl;
> +     unsigned long   tbu;
> +     unsigned long long      timer_reset_value;
> +     unsigned long   lastinc;
> +
> +     phys_size_t     ram_size;       /* RAM size */
> +#if 0
> +     unsigned long   relocaddr;      /* Start address of U-Boot in RAM */
> +     phys_size_t     ram_size;       /* RAM size */
> +     unsigned long   mon_len;        /* monitor len */
> +     unsigned long   irq_sp;         /* irq stack pointer */
> +     unsigned long   start_addr_sp;  /* start_addr_stackpointer */
> +     unsigned long   reloc_off;
> +#endif

delete dead code.  you should also kill any fields you don't actually need 
rather than just copying from another arch.  the 
timer_rate_hz/tbl/tbu/timer_reset_value/lastinc look suspicious to me.

> --- /dev/null
> +++ b/arch/c6x/include/asm/sizes.h

do you actually need this ?  i don't think you do, so kill it.  if you have 
some file that includes it, fix it and get rid of this file.

> --- /dev/null
> +++ b/arch/c6x/include/asm/u-boot-c6x.h

please rename to sections.h

> +#ifndef _U_BOOT_C6X_H_
> +#define _U_BOOT_C6X_H_ 1

no need to define to 1

> --- /dev/null
> +++ b/arch/c6x/include/asm/u-boot.h
>
> +typedef struct bd_info {
> +     int             bi_baudrate;    /* serial console baudrate */
> +     unsigned long   bi_ip_addr;     /* IP Address */
> +     ulong           bi_arch_number; /* unique id for this board */
> +     ulong           bi_boot_params; /* where this board expects params */
> +     unsigned long   bi_arm_freq; /* arm frequency */
> +     unsigned long   bi_dsp_freq; /* dsp core frequency */
> +     unsigned long   bi_ddr_freq; /* ddr frequency */
> +     struct                          /* RAM configuration */
> +     {
> +             ulong start;
> +             ulong size;
> +     } bi_dram[CONFIG_NR_DRAM_BANKS];
> +} bd_t;

again, please go through here and delete members you don't actually need

> +/* For image.h:image_check_target_arch() */
> +#define IH_ARCH_DEFAULT IH_ARCH_ARM

brrrr-wrong

> --- /dev/null
> +++ b/arch/c6x/lib/board.c
>
> +#ifdef CONFIG_STATUS_LED
> +#include <status_led.h>
> +#endif

no need to conditionalize the include

> +static int display_banner(void)
> +{
> +     printf("\n\n%s\n\n", version_string);

just call display_options()

> +     WATCHDOG_RESET();
> +/*   interrupt_init();*/

delete dead code

> +int do_reset(cmd_tbl_t *cmdtp, int flag, int argc,
> +     char *const argv[])
> +{
> +     reset_cpu(0);
> +     return 0;
> +}
> +

delete trailing newlines

> --- /dev/null
> +++ b/arch/c6x/lib/bootm.c
>
> +int disable_interrupts()

(void)

> +void enable_interrupts()

(void)

> --- /dev/null
> +++ b/arch/c6x/lib/memcmp.c
>
> +int memcmp(const void *cs, const void *ct, size_t count)
> +{
> +     const unsigned char *su1, *su2;
> +
> +     for (su1 = cs, su2 = ct; 0 < count; ++su1, ++su2, count--)
> +             if (*su1 != *su2)
> +                     return (*su1 < *su2) ? -1 : 1;
> +     return 0;
> +}
>
> --- /dev/null
> +++ b/arch/c6x/lib/memmove.c
>
> +void *memmove(void *s1, const void *s2, size_t n)
> +{
> +     register char *s = (char *) s1;
> +     register const char *p = (const char *) s2;
> +
> +     if (p >= s) {
> +             return memcpy(s, p, n);
> +     } else {
> +             while (n) {
> +                     --n;
> +                     s[n] = p[n];
> +             }
> +     }
> +     return s1;
> +}
>
> --- /dev/null
> +++ b/arch/c6x/lib/memset.c
>
> +void *memset(void *mem, register int ch, register size_t length)
> +{
> +     register char *m = (char *)mem - 1;
> +     while (length--)
> +             *++m = ch;
> +     return mem;
> +}

i can't see how these would be any better than the ones we already have in 
lib/string.c.  i say delete these files and don't bother.
-mike

Attachment: signature.asc
Description: This is a digitally signed message part.

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to