Skip to contentSkip to conversation

tegra: Compulab TrimSlice board support

PATCH Superseded Stephen Warren 4 participants 6 messages May 17, 2012 – Jun 1, 2012 Tegra architecture support

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 tegra: Compulab TrimSlice board support

From: Stephen Warren <[email protected]>

---
v2: Fixed unused variable warnings

---
 MAINTAINERS                             |    1 +
 board/compulab/dts/tegra2-trimslice.dts |   57 ++++++++++++++++++++
 board/compulab/trimslice/Makefile       |   49 +++++++++++++++++
 board/compulab/trimslice/trimslice.c    |   73 +++++++++++++++++++++++++
 boards.cfg                              |    1 +
 include/configs/trimslice.h             |   89 +++++++++++++++++++++++++++++++
 6 files changed, 270 insertions(+), 0 deletions(-)
 create mode 100644 board/compulab/dts/tegra2-trimslice.dts
 create mode 100644 board/compulab/trimslice/Makefile
 create mode 100644 board/compulab/trimslice/trimslice.c
 create mode 100644 include/configs/trimslice.h
MAINTAINERS +1 -0
index 7c161db..9d35c0e 100644
@@ -912,6 +912,7 @@ Stephen Warren <[email protected]>
 
 	ventana		Tegra2 (ARM7 & A9 Dual Core)
 	paz00		Tegra2 (ARM7 & A9 Dual Core)
+	trimslice	Tegra2 (ARM7 & A9 Dual Core)
 	whistler	Tegra2 (ARM7 & A9 Dual Core)
 
 Thomas Weber <[email protected]>
board/compulab/dts/tegra2-trimslice.dts new file +57 -0

57 lines not on this page · raw message

board/compulab/trimslice/Makefile new file +49 -0

49 lines not on this page · raw message

board/compulab/trimslice/trimslice.c new file +73 -0

73 lines not on this page · raw message

boards.cfg +1 -0
index a96b7d7..e983b05 100644
@@ -260,6 +260,7 @@ jornada                      arm         sa1100
 plutux                       arm         armv7       plutux              avionic-design tegra2
 medcom                       arm         armv7       medcom              avionic-design tegra2
 paz00                        arm         armv7       paz00               compal         tegra2
+trimslice                    arm         armv7       trimslice           compulab       tegra2
 atngw100                     avr32       at32ap      -                   atmel          at32ap700x
 atstk1002                    avr32       at32ap      atstk1000           atmel          at32ap700x
 atstk1003                    avr32       at32ap      atstk1000           atmel          at32ap700x
include/configs/trimslice.h new file +89 -0

89 lines not on this page · raw message

Signature · 1 line
1.7.0.4
#1 lore raw

PATCH v2 1/1 tegra: Compulab TrimSlice board support

Hi Stephen,

Thanks for doing this!
We highly appreciate your help.
We (Igor and me) have several comments below...

Quoted 82 lines · Stephen Warren

From: Stephen Warren<[email protected]>

---
v2: Fixed unused variable warnings

---
  MAINTAINERS                             |    1 +
  board/compulab/dts/tegra2-trimslice.dts |   57 ++++++++++++++++++++
  board/compulab/trimslice/Makefile       |   49 +++++++++++++++++
  board/compulab/trimslice/trimslice.c    |   73 +++++++++++++++++++++++++
  boards.cfg                              |    1 +
  include/configs/trimslice.h             |   89 +++++++++++++++++++++++++++++++
  6 files changed, 270 insertions(+), 0 deletions(-)

create mode 100644 board/compulab/dts/tegra2-trimslice.dts create mode 100644 board/compulab/trimslice/Makefile create mode 100644 board/compulab/trimslice/trimslice.c create mode 100644 include/configs/trimslice.h

MAINTAINERS +1 -0
index 7c161db..9d35c0e 100644
@@ -912,6 +912,7 @@ Stephen Warren<[email protected]>
 
  	ventana		Tegra2 (ARM7&  A9 Dual Core)
  	paz00		Tegra2 (ARM7&  A9 Dual Core)
+	trimslice	Tegra2 (ARM7&  A9 Dual Core)
  	whistler	Tegra2 (ARM7&  A9 Dual Core)
 
  Thomas Weber<[email protected]>
board/compulab/dts/tegra2-trimslice.dts new file +46 -0
new file mode 100644
index 0000000..c707eb8
@@ -0,0 +1,57 @@
+/dts-v1/;
+
+/include/ ARCH_CPU_DTS
+
+/ {
+	model = "Compulab TrimSlice board";
+	compatible = "compulab,trimslice", "nvidia,tegra20";
+
+	aliases {
+		usb0 = "/usb at c5008000";
+	};
+
+	memory {
+		reg =<0x00000000 0x40000000>;
+	};
+
+	clocks {
+		clk_32k: clk_32k {
+			clock-frequency =<32000>;
+		};
+		osc {
+			clock-frequency =<12000000>;
+		};
+	};
+
+	clock at 60006000 {
+		clocks =<&clk_32k&osc>;
+	};
+
+	serial at 70006000 {
+		clock-frequency =<216000000>;
+	};
+
+	i2c at 7000c000 {
+		status = "disabled";
+	};
+
+	i2c at 7000c400 {
+		status = "disabled";
+	};
+
+	i2c at 7000c500 {
+		status = "disabled";
+	};
+
+	i2c at 7000d000 {

board/compulab/dts/tegra2-trimslice.dts line 56

Quoted 10 lines
board/compulab/dts/tegra2-trimslice.dts new file +10 -0
+		status = "disabled";
+	};
+
+	usb at c5004000 {
+		status = "disabled";
+	};
+
+	usb at c5004000 {
+		status = "disabled";
+	};

This looks like a typo?

Quoted 19 lines
board/compulab/dts/tegra2-trimslice.dts new file +1 -0
+};
board/compulab/trimslice/Makefile new file +12 -0
new file mode 100644
index 0000000..bf624f4
@@ -0,0 +1,49 @@
+#
+#  (C) Copyright 2010-2012
+#  NVIDIA Corporation<www.nvidia.com>
+#
+#
+#  See file CREDITS for list of people who contributed to this
+#  project.
+#
+#  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; either version 2 of
+#  the License, or (at your option) any later version.

board/compulab/trimslice/Makefile line 22

Quoted 10 lines
board/compulab/trimslice/Makefile new file +10 -0
+#
+#  This program is distributed in the hope that it will be useful,
+#  but WITHOUT ANY WARRANTY; without even the implied warranty of
+#  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+#  GNU General Public License for more details.
+#
+#  You should have received a copy of the GNU General Public License
+#  along with this program; if not, write to the Free Software
+#  Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+#  MA 02111-1307 USA

Can we, please not have the postal address here and all other files?

board/compulab/trimslice/Makefile line 35

Quoted 13 lines
board/compulab/trimslice/Makefile new file +13 -0
+#
+
+include $(TOPDIR)/config.mk
+
+ifneq ($(OBJTREE),$(SRCTREE))
+$(shell mkdir -p $(obj)../../nvidia/common)
+endif
+
+LIB	= $(obj)lib$(BOARD).o
+
+COBJS	:= $(BOARD).o
+COBJS	+= ../../nvidia/common/board.o
+

I feel that the common board.c file should be in the common SoC location, like arch/arm/cpu/armv7/tegra2/ ?
In fact there is already a board.c file there, so how about we move the common stuff there?

Quoted 137 lines
board/compulab/trimslice/Makefile new file +14 -0
+SRCS	:= $(COBJS:.o=.c)
+OBJS	:= $(addprefix $(obj),$(COBJS))
+
+$(LIB):	$(obj).depend $(OBJS)
+	$(call cmd_link_o_target, $(OBJS))
+
+#########################################################################
+
+# defines $(obj).depend target
+include $(SRCTREE)/rules.mk
+
+sinclude $(obj).depend
+
+#########################################################################
board/compulab/trimslice/trimslice.c new file +73 -0
new file mode 100644
index 0000000..7167c91
@@ -0,0 +1,73 @@
+/*
+ *  (C) Copyright 2010-2012
+ *  NVIDIA Corporation<www.nvidia.com>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * 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; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include<common.h>
+#include<i2c.h>
+#include<asm/io.h>
+#include<asm/arch/tegra2.h>
+#include<asm/arch/clock.h>
+#include<asm/arch/funcmux.h>
+#include<asm/arch/pinmux.h>
+#include<asm/arch/mmc.h>
+#include<asm/gpio.h>
+#ifdef CONFIG_TEGRA2_MMC
+#include<mmc.h>
+#endif
+
+/*
+ * Routine: gpio_config_uart
+ * Description: Does nothing on TrimSlice - no UART-related GPIOs.
+ */
+void gpio_config_uart(void)
+{
+}
+
+/*
+ * Routine: pin_mux_mmc
+ * Description: setup the pin muxes/tristate values for the SDMMC(s)
+ */
+static void pin_mux_mmc(void)
+{
+	funcmux_select(PERIPH_ID_SDMMC1, FUNCMUX_SDMMC1_SDIO1_4BIT);
+	funcmux_select(PERIPH_ID_SDMMC4, FUNCMUX_SDMMC4_ATB_GMA_4_BIT);
+
+	/* For CD GPIO PP1 */
+	pinmux_tristate_disable(PINGRP_DAP3);
+}
+
+/* this is a weak define that we are overriding */
+int board_mmc_init(bd_t *bd)
+{
+	debug("board_mmc_init called\n");
+
+	/* Enable muxes, etc. for SDMMC controllers */
+	pin_mux_mmc();
+
+	/* init dev 0 (SDMMC4), (micro-SD slot) with 4-bit bus */
+	tegra2_mmc_init(0, 4, -1, GPIO_PP1);
+
+	/* init dev 3 (SDMMC1), (SD slot) with 4-bit bus */
+	tegra2_mmc_init(3, 4, -1, -1);
+
+	return 0;
+}
boards.cfg +1 -0
index a96b7d7..e983b05 100644
@@ -260,6 +260,7 @@ jornada                      arm         sa1100
  plutux                       arm         armv7       plutux              avionic-design tegra2
  medcom                       arm         armv7       medcom              avionic-design tegra2
  paz00                        arm         armv7       paz00               compal         tegra2
+trimslice                    arm         armv7       trimslice           compulab       tegra2
  atngw100                     avr32       at32ap      -                   atmel          at32ap700x
  atstk1002                    avr32       at32ap      atstk1000           atmel          at32ap700x
  atstk1003                    avr32       at32ap      atstk1000           atmel          at32ap700x
include/configs/trimslice.h new file +26 -0
new file mode 100644
index 0000000..dafca5e
@@ -0,0 +1,89 @@
+/*
+ *  (C) Copyright 2010-2012
+ *  NVIDIA Corporation<www.nvidia.com>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * 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; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#ifndef __CONFIG_H
+#define __CONFIG_H
+

include/configs/trimslice.h line 36

Quoted 10 lines
include/configs/trimslice.h new file +10 -0
+#include<asm/sizes.h>
+#include "tegra2-common.h"
+
+/* Enable fdt support for TrimSlice. Flash the image in u-boot-dtb.bin */
+#define CONFIG_DEFAULT_DEVICE_TREE	tegra2-trimslice
+#define CONFIG_OF_CONTROL
+#define CONFIG_OF_SEPARATE
+
+/* High-level configuration options */
+#define V_PROMPT		"Tegra2 (TrimSlice) # "

Can this, please be "TrimSlice #"

include/configs/trimslice.h line 37

Quoted 1 line
include/configs/trimslice.h new file +1 -0
+#define CONFIG_TEGRA2_BOARD_STRING	"NVIDIA Trimslice"

Can this, please be "CompuLab TrimSlice"

include/configs/trimslice.h line 46

Quoted 9 lines
include/configs/trimslice.h new file +9 -0
+
+/* Board-specific serial config */
+#define CONFIG_SERIAL_MULTI
+#define CONFIG_TEGRA2_ENABLE_UARTA
+#define CONFIG_TEGRA2_UARTA_GPU
+#define CONFIG_SYS_NS16550_COM1		NV_PA_APB_UARTA_BASE
+
+#define CONFIG_MACH_TYPE		MACH_TYPE_TRIMSLICE
+#define CONFIG_SYS_BOARD_ODMDATA	0x300c0011 /* lp?, 1GB, UARTA */

In our downstream U-Boot, ODMDATA is 0x300d8011 /* lp1, 1GB, UARTA*/ Are you certain that your value is the correct one?

Quoted 25 lines
include/configs/trimslice.h new file +25 -0
+
+#define CONFIG_BOARD_EARLY_INIT_F
+
+/* I2C */
+#define CONFIG_TEGRA_I2C
+#define CONFIG_SYS_I2C_INIT_BOARD
+#define CONFIG_I2C_MULTI_BUS
+#define CONFIG_SYS_MAX_I2C_BUS		4
+#define CONFIG_SYS_I2C_SPEED		100000
+#define CONFIG_CMD_I2C
+
+/* SD/MMC */
+#define CONFIG_MMC
+#define CONFIG_GENERIC_MMC
+#define CONFIG_TEGRA2_MMC
+#define CONFIG_CMD_MMC
+
+#define CONFIG_DOS_PARTITION
+#define CONFIG_EFI_PARTITION
+#define CONFIG_CMD_EXT2
+#define CONFIG_CMD_FAT
+
+/* Environment not stored */
+#define CONFIG_ENV_IS_NOWHERE
+

include/configs/trimslice.h line 81

Quoted 10 lines
include/configs/trimslice.h new file +10 -0
+/* USB Host support */
+#define CONFIG_USB_EHCI
+#define CONFIG_USB_EHCI_TEGRA
+#define CONFIG_USB_STORAGE
+#define CONFIG_CMD_USB
+
+/* USB networking support */
+#define CONFIG_USB_HOST_ETHER
+#define CONFIG_USB_ETHER_SMSC95XX
+#define CONFIG_USB_ETHER_ASIX

TrimSlice does not have any on-board Ethernet over USB devices. Do you connect them through the USB port? Or is it just a copy/paste from another board?

include/configs/trimslice.h line 89

Quoted 8 lines
include/configs/trimslice.h new file +8 -0
+
+/* General networking support */
+#define CONFIG_CMD_NET
+#define CONFIG_CMD_DHCP
+
+#include "tegra2-common-post.h"
+
+#endif /* __CONFIG_H */

--

Best Regards,

Konstantin

#2 lore raw

PATCH v2 1/1 tegra: Compulab TrimSlice board support

Quoted 19 lines · Konstantin Sinyuk

Hi Stephen,

Thanks for doing this!
We highly appreciate your help.
We (Igor and me) have several comments below...

board/compulab/dts/tegra2-trimslice.dts line 56

Quoted 9 lines · Stephen Warren
board/compulab/dts/tegra2-trimslice.dts new file +7 -0
 
+    usb at c5004000 {
+        status = "disabled";
+    };
+
+    usb at c5004000 {
+        status = "disabled";
+    };
Quoted 1 line

This looks like a typo?

Yes indeed. I'll send a patch to fix that up.

Quoted 8 lines

board/compulab/trimslice/Makefile line 22

Quoted 6 lines
board/compulab/trimslice/Makefile new file +4 -0
 
+#  You should have received a copy of the GNU General Public License
+#  along with this program; if not, write to the Free Software
+#  Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+#  MA 02111-1307 USA

Can we, please not have the postal address here and all other files?

Hmm. Well, all the existing code I'm basing these patches on has that license header. I don't feel strongly enough to change it now this patch is checked in. Feel free to submit a patch to clean this up, but if you do, I'd prefer if you made everything Tegra-related consistent.

Quoted 8 lines
Quoted 8 lines
board/compulab/trimslice/Makefile new file +8 -0
+include $(TOPDIR)/config.mk
+
+ifneq ($(OBJTREE),$(SRCTREE))
+$(shell mkdir -p $(obj)../../nvidia/common)
+endif
+
+LIB    = $(obj)lib$(BOARD).o
+
Quoted 8 lines

board/compulab/trimslice/Makefile line 35

Quoted 3 lines
board/compulab/trimslice/Makefile new file +3 -0
+COBJS    := $(BOARD).o
+COBJS    += ../../nvidia/common/board.o
+

I feel that the common board.c file should be in the common SoC location, like arch/arm/cpu/armv7/tegra2/ ?
In fact there is already a board.c file there, so how about we move the common stuff there?

Yes, the Tegra board support is evolving and may not be optimally structured right now. The logic above is what's needed given the current state. Perhaps this will move to a better location in the future. I don't plan on making that change right now though; I don't want to conflict with other changes I know people are making such as moving a bunch of code around to support a separate SPL.

Quoted 6 lines

include/configs/trimslice.h line 36

Quoted 4 lines
include/configs/trimslice.h +2 -0
 
+/* High-level configuration options */
+#define V_PROMPT        "Tegra2 (TrimSlice) # "

Can this, please be "TrimSlice #"

That wouldn't be consistent with any of the other Tegra boards though. Is there a specific need for the prompt to be different?

Quoted 3 lines
Quoted 1 line

+#define CONFIG_TEGRA2_BOARD_STRING "NVIDIA Trimslice"

Can this, please be "CompuLab TrimSlice"

Sure, that's just something I forgot to update when copying from another board's config file.

Quoted 11 lines

include/configs/trimslice.h line 46

Quoted 8 lines
include/configs/trimslice.h +8 -0
+/* Board-specific serial config */
+#define CONFIG_SERIAL_MULTI
+#define CONFIG_TEGRA2_ENABLE_UARTA
+#define CONFIG_TEGRA2_UARTA_GPU
+#define CONFIG_SYS_NS16550_COM1        NV_PA_APB_UARTA_BASE
+
+#define CONFIG_MACH_TYPE        MACH_TYPE_TRIMSLICE
+#define CONFIG_SYS_BOARD_ODMDATA    0x300c0011 /* lp?, 1GB, UARTA */

In our downstream U-Boot, ODMDATA is 0x300d8011 /* lp1, 1GB, UARTA*/ Are you certain that your value is the correct one?

Yes, 300d8011 is wrong, and you're just getting lucky that it's working for you since nothing actually uses the ODMDATA fields that are wrong.

The "d8" in the ODMDATA means to use UART D as the debug UART. However, TrimSlice uses UART A, and hence needs "c0" here.

Without this change, the mainline kernel config option CONFIG_TEGRA_DEBUG_UART_AUTO_ODMDATA (which affects where DEBUG_LL/earlyprintk output is routed) will not work correctly.

Quoted 8 lines

include/configs/trimslice.h line 81

Quoted 4 lines
include/configs/trimslice.h +4 -0
+/* USB networking support */
+#define CONFIG_USB_HOST_ETHER
+#define CONFIG_USB_ETHER_SMSC95XX
+#define CONFIG_USB_ETHER_ASIX

TrimSlice does not have any on-board Ethernet over USB devices. Do you connect them through the USB port? Or is it just a copy/paste from another board?

A later change has removed the SMSC95XX support since as you say, that chip is not present on the board. However, I actively use an ASIX USB Ethernet dongle with TrimSlice, so do need that config option enabled. Perhaps it can be removed if/when Tegra PCIe support is added to mainline U-Boot, so the built-in Ethernet adapter can be used instead.

#3 lore raw

PATCH v2 1/1 tegra: Compulab TrimSlice board support

Quoted 33 lines · Stephen Warren
Quoted 19 lines · Konstantin Sinyuk

Hi Stephen,

Thanks for doing this!
We highly appreciate your help.
We (Igor and me) have several comments below...

board/compulab/dts/tegra2-trimslice.dts line 56

Quoted 9 lines · Stephen Warren
board/compulab/dts/tegra2-trimslice.dts new file +7 -0
 
+    usb at c5004000 {
+        status = "disabled";
+    };
+
+    usb at c5004000 {
+        status = "disabled";
+    };
Quoted 1 line

This looks like a typo?

Yes indeed. I'll send a patch to fix that up.

Quoted 8 lines

board/compulab/trimslice/Makefile line 22

Quoted 6 lines
board/compulab/trimslice/Makefile new file +4 -0
 
+#  You should have received a copy of the GNU General Public License
+#  along with this program; if not, write to the Free Software
+#  Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+#  MA 02111-1307 USA

Can we, please not have the postal address here and all other files?

Quoted 4 lines

Hmm. Well, all the existing code I'm basing these patches on has that license header. I don't feel strongly enough to change it now this patch is checked in. Feel free to submit a patch to clean this up, but if you do, I'd prefer if you made everything Tegra-related consistent.

Ok, I see, the reason I don't like the postal addresses was explained by Russell King some time ago (I can't find it right now) and was about the postal address is subject to change without prior notice (and it did several times before) and that will leave majority of files with the wrong address... I consider it a bad practice.
But I don't really insist on this.

Now you say, that the patch has already been checked in... Tom, can we have a short notice on this,
may be an email (e.g. Applied, thanks!) to the list, so we know which patches went in and which are still pending?
I know we can look at your repository, but still...

Quoted 18 lines
Quoted 17 lines

board/compulab/trimslice/Makefile line 35

Quoted 12 lines
board/compulab/trimslice/Makefile new file +11 -0
+include $(TOPDIR)/config.mk
+
+ifneq ($(OBJTREE),$(SRCTREE))
+$(shell mkdir -p $(obj)../../nvidia/common)
+endif
+
+LIB    = $(obj)lib$(BOARD).o
+
+COBJS    := $(BOARD).o
+COBJS    += ../../nvidia/common/board.o
+

I feel that the common board.c file should be in the common SoC location, like arch/arm/cpu/armv7/tegra2/ ?
In fact there is already a board.c file there, so how about we move the common stuff there?

Quoted 6 lines

Yes, the Tegra board support is evolving and may not be optimally structured right now. The logic above is what's needed given the current state. Perhaps this will move to a better location in the future. I don't plan on making that change right now though; I don't want to conflict with other changes I know people are making such as moving a bunch of code around to support a separate SPL.

Ok.

Quoted 10 lines
Quoted 7 lines

include/configs/trimslice.h line 36

Quoted 5 lines
include/configs/trimslice.h +2 -0
 
+/* High-level configuration options */
+#define V_PROMPT        "Tegra2 (TrimSlice) # "

Can this, please be "TrimSlice #"

That wouldn't be consistent with any of the other Tegra boards though. Is there a specific need for the prompt to be different?

I see and probably you are right...
On the second thought, it will not be consistent with other CompuLab boards (which are not Tegra based), so this whole consistency thing...
I don't know... Let it be... After all, you have volunteered to maintain it :-)

Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

+#define CONFIG_TEGRA2_BOARD_STRING "NVIDIA Trimslice"

Can this, please be "CompuLab TrimSlice"

Sure, that's just something I forgot to update when copying from another board's config file.

Thanks

Quoted 16 lines
Quoted 12 lines

include/configs/trimslice.h line 46

Quoted 9 lines
include/configs/trimslice.h +8 -0
+/* Board-specific serial config */
+#define CONFIG_SERIAL_MULTI
+#define CONFIG_TEGRA2_ENABLE_UARTA
+#define CONFIG_TEGRA2_UARTA_GPU
+#define CONFIG_SYS_NS16550_COM1        NV_PA_APB_UARTA_BASE
+
+#define CONFIG_MACH_TYPE        MACH_TYPE_TRIMSLICE
+#define CONFIG_SYS_BOARD_ODMDATA    0x300c0011 /* lp?, 1GB, UARTA */

In our downstream U-Boot, ODMDATA is 0x300d8011 /* lp1, 1GB, UARTA*/ Are you certain that your value is the correct one?

Yes, 300d8011 is wrong, and you're just getting lucky that it's working for you since nothing actually uses the ODMDATA fields that are wrong.

Quoted 6 lines

The "d8" in the ODMDATA means to use UART D as the debug UART. However, TrimSlice uses UART A, and hence needs "c0" here.

Without this change, the mainline kernel config option CONFIG_TEGRA_DEBUG_UART_AUTO_ODMDATA (which affects where DEBUG_LL/earlyprintk output is routed) will not work correctly.

Ok, I see, thanks for sharing this information.

Quoted 10 lines
Quoted 9 lines

include/configs/trimslice.h line 81

Quoted 5 lines
include/configs/trimslice.h +4 -0
+/* USB networking support */
+#define CONFIG_USB_HOST_ETHER
+#define CONFIG_USB_ETHER_SMSC95XX
+#define CONFIG_USB_ETHER_ASIX

TrimSlice does not have any on-board Ethernet over USB devices. Do you connect them through the USB port? Or is it just a copy/paste from another board?

Quoted 5 lines

A later change has removed the SMSC95XX support since as you say, that chip is not present on the board. However, I actively use an ASIX USB Ethernet dongle with TrimSlice, so do need that config option enabled. Perhaps it can be removed if/when Tegra PCIe support is added to mainline U-Boot, so the built-in Ethernet adapter can be used instead.

Fare enough.
Thanks for the patches, information and the work you've done!

Signature · 2 lines
Regards,
Igor.
#4 lore raw

PATCH v2 1/1 tegra: Compulab TrimSlice board support

Igor,

Quoted 53 lines

-----Original Message-----
From: Igor Grinberg [mailto:grinberg at compulab.co.il]
Sent: Thursday, May 31, 2012 1:45 AM
To: Stephen Warren
Cc: Konstantin Sinyuk; Tom Warren; u-boot at lists.denx.de
Subject: Re: [U-Boot] [PATCH V2] tegra: Compulab TrimSlice board support

Quoted 33 lines · Stephen Warren
Quoted 19 lines · Konstantin Sinyuk

Hi Stephen,

Thanks for doing this!
We highly appreciate your help.
We (Igor and me) have several comments below...

board/compulab/dts/tegra2-trimslice.dts line 56

Quoted 9 lines · Stephen Warren
board/compulab/dts/tegra2-trimslice.dts new file +7 -0
 
+    usb at c5004000 {
+        status = "disabled";
+    };
+
+    usb at c5004000 {
+        status = "disabled";
+    };
Quoted 1 line

This looks like a typo?

Yes indeed. I'll send a patch to fix that up.

Quoted 8 lines

Review

Quoted 6 lines
(quoted diff) +4 -0
 
+#  You should have received a copy of the GNU General Public
+License #  along with this program; if not, write to the Free
+Software #  Foundation, Inc., 59 Temple Place, Suite 330, Boston, #
+MA 02111-1307 USA

Can we, please not have the postal address here and all other files?

Quoted 4 lines

Hmm. Well, all the existing code I'm basing these patches on has that license header. I don't feel strongly enough to change it now this patch is checked in. Feel free to submit a patch to clean this up, but if you do, I'd prefer if you made everything Tegra-related consistent.

Ok, I see, the reason I don't like the postal addresses was explained by Russell King some time ago (I can't find it right now) and was about the postal address is subject to change without prior notice (and it did several times before) and that will leave majority of files with the wrong address... I consider it a bad practice.
But I don't really insist on this.

Quoted 5 lines

Now you say, that the patch has already been checked in... Tom, can we have a short notice on this, may be an email (e.g. Applied, thanks!) to the list, so we know which patches went in and which are still pending?
I know we can look at your repository, but still...

You'll see a pull request to Albert Aribaud when I've collected enough patches to go into u-boot-tegra/master, and if your patch (or a patch you care about) has been checked in, you'll see it there. I can't keep track of every contributor nor every Tegra board and remember to email them individually when code that affects them has been checked in. I can try to remember to add you to the CC when I send the pull request, but no guarantees. ;)

Also, if you are monitoring the mailing list, you can search for TrimSlice commits/discussions, etc. in each digest, on PatchWork, or gmane/pipermail, etc.

Tom

Quoted 104 lines
Quoted 18 lines
Quoted 17 lines

board/compulab/trimslice/Makefile line 35

Quoted 12 lines
board/compulab/trimslice/Makefile new file +10 -0
+include $(TOPDIR)/config.mk
+
+ifneq ($(OBJTREE),$(SRCTREE))
+$(shell mkdir -p $(obj)../../nvidia/common) endif
+
+LIB    = $(obj)lib$(BOARD).o
+
+COBJS    := $(BOARD).o
+COBJS    += ../../nvidia/common/board.o
+

I feel that the common board.c file should be in the common SoC location, like arch/arm/cpu/armv7/tegra2/ ?
In fact there is already a board.c file there, so how about we move the common stuff there?

Quoted 6 lines

Yes, the Tegra board support is evolving and may not be optimally structured right now. The logic above is what's needed given the current state. Perhaps this will move to a better location in the future. I don't plan on making that change right now though; I don't want to conflict with other changes I know people are making such as moving a bunch of code around to support a separate SPL.

Ok.

Quoted 11 lines
Quoted 8 lines

include/configs/trimslice.h line 36

Quoted 6 lines
include/configs/trimslice.h +2 -0
 
+/* High-level configuration options */
+#define V_PROMPT        "Tegra2 (TrimSlice) # "

Can this, please be "TrimSlice #"

That wouldn't be consistent with any of the other Tegra boards though. Is there a specific need for the prompt to be different?

I see and probably you are right...
On the second thought, it will not be consistent with other CompuLab boards (which are not Tegra based), so this whole consistency thing... I don't know... Let it be... After all, you have volunteered to maintain it :-)

Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

+#define CONFIG_TEGRA2_BOARD_STRING "NVIDIA Trimslice"

Can this, please be "CompuLab TrimSlice"

Sure, that's just something I forgot to update when copying from another board's config file.

Thanks

Quoted 13 lines
Quoted 12 lines

include/configs/trimslice.h line 46

Quoted 9 lines
include/configs/trimslice.h +8 -0
+/* Board-specific serial config */
+#define CONFIG_SERIAL_MULTI
+#define CONFIG_TEGRA2_ENABLE_UARTA
+#define CONFIG_TEGRA2_UARTA_GPU
+#define CONFIG_SYS_NS16550_COM1        NV_PA_APB_UARTA_BASE
+
+#define CONFIG_MACH_TYPE        MACH_TYPE_TRIMSLICE
+#define CONFIG_SYS_BOARD_ODMDATA    0x300c0011 /* lp?, 1GB, UARTA */

In our downstream U-Boot, ODMDATA is 0x300d8011 /* lp1, 1GB, UARTA*/ Are you certain that your value is the correct one?

Quoted 2 lines

Yes, 300d8011 is wrong, and you're just getting lucky that it's working for you since nothing actually uses the ODMDATA fields that are

wrong.

Quoted 7 lines

The "d8" in the ODMDATA means to use UART D as the debug UART. However, TrimSlice uses UART A, and hence needs "c0" here.

Without this change, the mainline kernel config option CONFIG_TEGRA_DEBUG_UART_AUTO_ODMDATA (which affects where DEBUG_LL/earlyprintk output is routed) will not work correctly.

Ok, I see, thanks for sharing this information.

Quoted 10 lines
Quoted 9 lines

include/configs/trimslice.h line 81

Quoted 5 lines
include/configs/trimslice.h +4 -0
+/* USB networking support */
+#define CONFIG_USB_HOST_ETHER
+#define CONFIG_USB_ETHER_SMSC95XX
+#define CONFIG_USB_ETHER_ASIX

TrimSlice does not have any on-board Ethernet over USB devices. Do you connect them through the USB port? Or is it just a copy/paste from another board?

Quoted 5 lines

A later change has removed the SMSC95XX support since as you say, that chip is not present on the board. However, I actively use an ASIX USB Ethernet dongle with TrimSlice, so do need that config option enabled. Perhaps it can be removed if/when Tegra PCIe support is added to mainline U-Boot, so the built-in Ethernet adapter can be used instead.

Fare enough.
Thanks for the patches, information and the work you've done!

--
Regards,
Igor.

Signature · 1 line
nvpublic
#5 lore raw

PATCH v2 1/1 tegra: Compulab TrimSlice board support

Hi Tom,

Quoted 55 lines · Tom Warren

Igor,

Quoted 53 lines

-----Original Message-----
From: Igor Grinberg [mailto:grinberg at compulab.co.il]
Sent: Thursday, May 31, 2012 1:45 AM
To: Stephen Warren
Cc: Konstantin Sinyuk; Tom Warren; u-boot at lists.denx.de
Subject: Re: [U-Boot] [PATCH V2] tegra: Compulab TrimSlice board support

Quoted 33 lines · Stephen Warren
Quoted 19 lines · Konstantin Sinyuk

Hi Stephen,

Thanks for doing this!
We highly appreciate your help.
We (Igor and me) have several comments below...

board/compulab/dts/tegra2-trimslice.dts line 56

Quoted 9 lines · Stephen Warren
board/compulab/dts/tegra2-trimslice.dts new file +7 -0
 
+    usb at c5004000 {
+        status = "disabled";
+    };
+
+    usb at c5004000 {
+        status = "disabled";
+    };
Quoted 1 line

This looks like a typo?

Yes indeed. I'll send a patch to fix that up.

Quoted 8 lines

Review

Quoted 6 lines
(quoted diff) +4 -0
 
+#  You should have received a copy of the GNU General Public
+License #  along with this program; if not, write to the Free
+Software #  Foundation, Inc., 59 Temple Place, Suite 330, Boston, #
+MA 02111-1307 USA

Can we, please not have the postal address here and all other files?

Quoted 4 lines

Hmm. Well, all the existing code I'm basing these patches on has that license header. I don't feel strongly enough to change it now this patch is checked in. Feel free to submit a patch to clean this up, but if you do, I'd prefer if you made everything Tegra-related consistent.

Ok, I see, the reason I don't like the postal addresses was explained by Russell King some time ago (I can't find it right now) and was about the postal address is subject to change without prior notice (and it did several times before) and that will leave majority of files with the wrong address... I consider it a bad practice.
But I don't really insist on this.

Quoted 8 lines
Quoted 5 lines

Now you say, that the patch has already been checked in... Tom, can we have a short notice on this, may be an email (e.g. Applied, thanks!) to the list, so we know which patches went in and which are still pending?
I know we can look at your repository, but still...

You'll see a pull request to Albert Aribaud when I've collected enough patches to go into u-boot-tegra/master, and if your patch (or a patch you care about) has been checked in, you'll see it there.

Yes, but it will also be useful to know when you apply the patch to your tree, but please see below...

Quoted 2 lines

I can't keep track of every contributor nor every Tegra board and remember to email them individually when code that affects them has been checked in.

And that is not what I was asking for...
I'm not asking if you can CC everybody you think is interested in the patch... No!
I'm asking if you can do a "Reply to all" to the patch (or a patch set) saying that the patch has been applied? Many other maintainers do this (not only in U-Boot) and it is really convenient to have that short email.

Quoted 1 line

I can try to remember to add you to the CC when I send the pull request, but no guarantees. ;)

Thanks.

Quoted 2 lines

Also, if you are monitoring the mailing list, you can search for TrimSlice commits/discussions, etc. in each digest, on PatchWork, or gmane/pipermail, etc.

Well, monitoring mailing list will not help, if you don't "reply to all" (or at least reply to the list). I you had replied to the list, I would have no questions... ;-)

Quoted 106 lines

Tom

Quoted 104 lines
Quoted 18 lines
Quoted 17 lines

board/compulab/trimslice/Makefile line 35

Quoted 12 lines
board/compulab/trimslice/Makefile new file +10 -0
+include $(TOPDIR)/config.mk
+
+ifneq ($(OBJTREE),$(SRCTREE))
+$(shell mkdir -p $(obj)../../nvidia/common) endif
+
+LIB    = $(obj)lib$(BOARD).o
+
+COBJS    := $(BOARD).o
+COBJS    += ../../nvidia/common/board.o
+

I feel that the common board.c file should be in the common SoC location, like arch/arm/cpu/armv7/tegra2/ ?
In fact there is already a board.c file there, so how about we move the common stuff there?

Quoted 6 lines

Yes, the Tegra board support is evolving and may not be optimally structured right now. The logic above is what's needed given the current state. Perhaps this will move to a better location in the future. I don't plan on making that change right now though; I don't want to conflict with other changes I know people are making such as moving a bunch of code around to support a separate SPL.

Ok.

Quoted 11 lines
Quoted 8 lines

include/configs/trimslice.h line 36

Quoted 6 lines
include/configs/trimslice.h +2 -0
 
+/* High-level configuration options */
+#define V_PROMPT        "Tegra2 (TrimSlice) # "

Can this, please be "TrimSlice #"

That wouldn't be consistent with any of the other Tegra boards though. Is there a specific need for the prompt to be different?

I see and probably you are right...
On the second thought, it will not be consistent with other CompuLab boards (which are not Tegra based), so this whole consistency thing... I don't know... Let it be... After all, you have volunteered to maintain it :-)

Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

+#define CONFIG_TEGRA2_BOARD_STRING "NVIDIA Trimslice"

Can this, please be "CompuLab TrimSlice"

Sure, that's just something I forgot to update when copying from another board's config file.

Thanks

Quoted 13 lines
Quoted 12 lines

include/configs/trimslice.h line 46

Quoted 9 lines
include/configs/trimslice.h +8 -0
+/* Board-specific serial config */
+#define CONFIG_SERIAL_MULTI
+#define CONFIG_TEGRA2_ENABLE_UARTA
+#define CONFIG_TEGRA2_UARTA_GPU
+#define CONFIG_SYS_NS16550_COM1        NV_PA_APB_UARTA_BASE
+
+#define CONFIG_MACH_TYPE        MACH_TYPE_TRIMSLICE
+#define CONFIG_SYS_BOARD_ODMDATA    0x300c0011 /* lp?, 1GB, UARTA */

In our downstream U-Boot, ODMDATA is 0x300d8011 /* lp1, 1GB, UARTA*/ Are you certain that your value is the correct one?

Quoted 2 lines

Yes, 300d8011 is wrong, and you're just getting lucky that it's working for you since nothing actually uses the ODMDATA fields that are

wrong.

Quoted 7 lines

The "d8" in the ODMDATA means to use UART D as the debug UART. However, TrimSlice uses UART A, and hence needs "c0" here.

Without this change, the mainline kernel config option CONFIG_TEGRA_DEBUG_UART_AUTO_ODMDATA (which affects where DEBUG_LL/earlyprintk output is routed) will not work correctly.

Ok, I see, thanks for sharing this information.

Quoted 10 lines
Quoted 9 lines

include/configs/trimslice.h line 81

Quoted 5 lines
include/configs/trimslice.h +4 -0
+/* USB networking support */
+#define CONFIG_USB_HOST_ETHER
+#define CONFIG_USB_ETHER_SMSC95XX
+#define CONFIG_USB_ETHER_ASIX

TrimSlice does not have any on-board Ethernet over USB devices. Do you connect them through the USB port? Or is it just a copy/paste from another board?

Quoted 5 lines

A later change has removed the SMSC95XX support since as you say, that chip is not present on the board. However, I actively use an ASIX USB Ethernet dongle with TrimSlice, so do need that config option enabled. Perhaps it can be removed if/when Tegra PCIe support is added to mainline U-Boot, so the built-in Ethernet adapter can be used instead.

Fare enough.
Thanks for the patches, information and the work you've done!

--
Regards,
Igor.

Signature · 2 lines
Regards,
Igor.