Dear Gilles,

In message 3EFCB679-8DF9-43F2-B3F4-FBC00E578061@gigadevices.com 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 gilles@gigadevices.com Date: Sun, 1 Feb 2015 19:30:01 -0800 Subject: [PATCH] Adding Support for BAV335x boards

Signed-off-by: Gilles Gameiro gilles@gigadevices.com

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