[U-Boot] [PATCH V2] tegra: Compulab TrimSlice board support

[Tom Warren]

Igor,

> -----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
> 
> 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 at c5004000 {
> >>> +        status = "disabled";
> >>> +    };
> >>> +
> >>> +    usb at 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