Dear Gilles, In message <[email protected]> you wrote: > > I just added support for the BAV335x OEM boards = > http://birdland.com/oem/bav335x-network-processor/ > > This patch was generated with the patman tool which reported no checks, = > warnings nor errors. > > The patch is relative to the v2015.01 tag.
THis being a version 2 of the patch, you should have marked it as such in the Subject, and provides a Changelog. > Content-Disposition: attachment; > filename=0001-Adding-Support-for-BAV335x-boards.patch > Content-Type: application/octet-stream; > name="0001-Adding-Support-for-BAV335x-boards.patch" > Content-Transfer-Encoding: quoted-printable Also, I asked you before NOT to send patches as MIME attachments. Please make sure to send them inline, ideally using git-send-email. This being a TI based board, you should have sent it to the TI custodian. > From a4d163542a7acbcc1ac5f520f41966f373b9f307 Mon Sep 17 00:00:00 2001 > From: Gilles Gameiro <[email protected]> > Date: Sun, 1 Feb 2015 19:30:01 -0800 > Subject: [PATCH] Adding Support for BAV335x boards > > Signed-off-by: Gilles Gameiro <[email protected]> Please add a description of the board to the commit message. > @@ -969,6 +982,7 @@ source "board/taskit/stamp9g20/Kconfig" > source "board/tbs/tbs2910/Kconfig" > source "board/ti/am335x/Kconfig" > source "board/ti/am43xx/Kconfig" > +source "board/birdland/bav335x/Kconfig" > source "board/ti/ti814x/Kconfig" > source "board/ti/ti816x/Kconfig" > source "board/ti/tnetv107xevm/Kconfig" Please keep lists sorted. > +config CONS_INDEX > + int "UART used for console" > + range 1 6 > + default 1 > + help > + The AM335x SoC has a total of 6 UARTs (UART0 to UART5 as referenced > + in documentation, etc) available to it. Depending on your specific > + board you may want something other than UART0 as for example the IDK > + uses UART3 so enter 4 here. Is this useful and correct information for your board? > --- /dev/null > +++ b/board/birdland/bav335x/Makefile > @@ -0,0 +1,11 @@ > +# > +# Makefile > +# > +# Copyright (C) 2012-2014, Birdland Audio - http://birdland.com/oem > +# SPDX ID missing. > +++ b/board/birdland/bav335x/README > @@ -0,0 +1,31 @@ > +Summary > +======= SPDX ID missing. Please fix globally for all new files. > +static __maybe_unused struct ctrl_dev *cdev = > + (struct ctrl_dev *)CTRL_DEVICE_BASE; > + > + > + > +/* Excessive white space - please use a single blank line. Please fix globally. > + if (header.version[1] == 'A') { > + if (debug) > + puts("=== Detected Board model BAV335x Rev.A"); Use debug() instead. Please fix globally. ... > +void sdram_init(void) > +{ > + config_ddr(400, &ioregs_bonelt, > + &ddr3_bav335x_data, > + &ddr3_bav335x_cmd_ctrl_data, > + &ddr3_bav335x_emif_reg_data, 0); > +} > +#endif It might be useful to add a comment so it is easier to see which "#if" this belongs to. > + /* Default manufacturing address; used when no EE or invalid */ > + n = 0; > + mac_addr[0] = 0; > + mac_addr[1] = 0x20; > + mac_addr[2] = 0x18; > + mac_addr[3] = 0x1C; > + mac_addr[4] = 0x00; > + mac_addr[5] = 0x01; Please never, never ever hard-wire MAC addresses into U-Boot. Also, the 002018 prefix appears to be owned by Cis Technology Inc.; do you have the rights to use it? > --- /dev/null > +++ b/board/birdland/bav335x/mux.c > @@ -0,0 +1,206 @@ > +/* > + * mux.c > + * > + * Copyright (c) 2012-2014 Birdland Audio - http://birdland.com/oem > + * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com/ > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License as > + * published by the Free Software Foundation version 2. > + * > + * This program is distributed "as is" WITHOUT ANY WARRANTY of any > + * kind, whether express or implied; without even the implied warranty > + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. Please use SPDX ID instead. What exactly is the difference in this file compared to board/ti/am335x/mux.c ? > +/* > + * The AM335x GP EVM, if daughter card(s) are connected, can have 8 > + * different profiles. These profiles determine what peripherals are > + * valid and need pinmux to be configured. > + */ Is this relevant to your board? > diff --git a/board/birdland/bav335x/u-boot.lds > b/board/birdland/bav335x/u-boot.lds > new file mode 100644 > index 0000000..fc80f21 > --- /dev/null > +++ b/board/birdland/bav335x/u-boot.lds Do you really need a private linker script? Why? > + /* > + * Deprecated: this MMU section is used by pxa at present but > + * should not be used by new boards/CPUs. > + */ Did you read that? > --- /dev/null > +++ b/include/configs/bav335x.h > @@ -0,0 +1,633 @@ > +/* > + * bav335x.h > + * > + * Copyright (c) 2012-2014 Birdland Audio - http://birdland.com/oem > + * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com/ > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License as > + * published by the Free Software Foundation version 2. > + * > + * This program is distributed "as is" WITHOUT ANY WARRANTY of any > + * kind, whether express or implied; without even the implied warranty > + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. SPDX ID .... > +#define MACH_TYPE_TIAM335EVM 3589 /* Until the next sync */ > +#define CONFIG_MACH_TYPE MACH_TYPE_TIAM335EVM But this is NOT a TI AM335X EVM board, it is your own machine. You should not hijack foreign IDs. > +/* Always 128 KiB env size */ > +#define CONFIG_ENV_SIZE (128 << 10) This makes no sense. You will NEVER need that much. If you make the environment so big without need, it will only slow down booting. > +#define CONFIG_EXTRA_ENV_SETTINGS \ > +DEFAULT_LINUX_BOOT_ENV \ > +"boot_fdt=try\0" \ > +"bootpart=0:2\0" \ > +"bootdir=\0" \ ... Please use proper indentation. > +#define MTDPARTS_DEFAULT \ > + "mtdparts=nand.0:" \ > + "128k(NAND.SPL)," \ > + "128k(NAND.SPL.backup1)," \ > + "128k(NAND.SPL.backup2)," \ > + "128k(NAND.SPL.backup3)," \ > + "256k(NAND.u-boot-spl-os)," \ > + "1m(NAND.u-boot)," \ > + "128k(NAND.u-boot-env)," \ > + "128k(NAND.u-boot-env.backup1)," \ > + "8m(NAND.kernel)," \ > + "-(NAND.rootfs)" ... > +/* > + * Default to using SPI for environment, etc. > + * 0x000000 - 0x020000 : SPL (128KiB) > + * 0x020000 - 0x0A0000 : U-Boot (512KiB) > + * 0x0A0000 - 0x0BFFFF : First copy of U-Boot Environment (128KiB) > + * 0x0C0000 - 0x0DFFFF : Second copy of U-Boot Environment (128KiB) > + * 0x0E0000 - 0x442000 : Linux Kernel > + * 0x442000 - 0x800000 : Userland > + */ Are these settings in any way related and/or kept in sync? > +#define CONFIG_ENV_OFFSET (768 << 10) /* 768 KiB in */ > +#define CONFIG_ENV_OFFSET_REDUND (896 << 10) /* 896 KiB in */ Hm... 768kB = 0xC0000, 896 kB = 0xE0000 This is not what you wrote before??? > +#define MTDIDS_DEFAULT "nor0=m25p80-flash.0" > +#define MTDPARTS_DEFAULT "mtdparts=m25p80-flash.0:128k(SPL)," \ > + "512k(u-boot),128k(u-boot-env1)," \ > + "128k(u-boot-env2),3464k(kernel)," \ > + "-(rootfs)" See MTDPARTS_DEFAULT further above? What happens if you have CONFIG_NAND and CONFIG_SPI_BOOT both set? > +/* > + * NOR Size = 16 MiB > + * Number of Sectors/Blocks = 128 > + * Sector Size = 128 KiB > + * Word length = 16 bits > + * Default layout: > + * 0x000000 - 0x07FFFF : U-Boot (512 KiB) > + * 0x080000 - 0x09FFFF : First copy of U-Boot Environment (128 KiB) > + * 0x0A0000 - 0x0BFFFF : Second copy of U-Boot Environment (128 KiB) > + * 0x0C0000 - 0x4BFFFF : Linux Kernel (4 MiB) > + * 0x4C0000 - 0xFFFFFF : Userland (11 MiB + 256 KiB) Oh, yet another set of settings... This config file is a bit too complicated, don't you think so? I recommend to perform a serious clean-up. As is, it is unreadable and unmaintainable. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: [email protected] The White Rabbit put on his spectacles. "Where shall I begin, please your Majesty ?" he asked. "Begin at the beginning,", the King said, very gravely, "and go on till you come to the end: then stop." -- Lewis Carroll _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
