[U-Boot] [PATCH v3 1/4] Add support new arch: c6x

Mike Frysinger
2012-07-19T03:53:54Z

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120718/6039e63b/attachment.pgp>