Igor, > -----Original Message----- > From: Igor Grinberg [mailto:grinb...@compulab.co.il] > Sent: Thursday, May 31, 2012 1:45 AM > To: Stephen Warren > Cc: Konstantin Sinyuk; Tom Warren; u-boot@lists.denx.de > Subject: Re: [U-Boot] [PATCH V2] tegra: Compulab TrimSlice board support > > On 05/30/12 19:22, Stephen Warren wrote: > > On 05/30/2012 09:35 AM, Konstantin Sinyuk wrote: > >> Hi Stephen, > >> > >> Thanks for doing this! > >> We highly appreciate your help. > >> We (Igor and me) have several comments below... > >> > >> > >> On 05/17/2012 07:03 PM, Stephen Warren wrote: > > > >>> diff --git a/board/compulab/dts/tegra2-trimslice.dts > > > >>> + usb@c5004000 { > >>> + status = "disabled"; > >>> + }; > >>> + > >>> + usb@c5004000 { > >>> + status = "disabled"; > >>> + }; > >> > >> This looks like a typo? > > > > Yes indeed. I'll send a patch to fix that up. > > > >>> diff --git a/board/compulab/trimslice/Makefile > > > >>> +# 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. > > 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...
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 > > > > >>> +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? > > > > 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. > > > > >>> diff --git a/include/configs/trimslice.h > >>> b/include/configs/trimslice.h > > > >>> +/* 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 > :-) > > > > >>> +#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 > > > > >>> +/* 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. > > Ok, I see, thanks for sharing this information. > > > > >>> +/* 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. > > Fare enough. > Thanks for the patches, information and the work you've done! > > > -- > Regards, > Igor. -- nvpublic _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
