From mboxrd@z Thu Jan  1 00:00:00 1970
From: Julian Scheel <julian@jusst.de>
Date: Tue, 23 Feb 2016 20:47:14 +0100
Subject: [U-Boot] [PATCH 2/2] Add support the Avionic Design Meerkat COM
 and Kein Baseboard
In-Reply-To: <56CC9EE6.9070107@wwwdotorg.org>
References: <1456232346-3968-1-git-send-email-julian@jusst.de>
 <1456232346-3968-3-git-send-email-julian@jusst.de>
 <56CC9EE6.9070107@wwwdotorg.org>
Message-ID: <56CCB742.1000404@jusst.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On 23.02.16 19:03, Stephen Warren wrote:
> On 02/23/2016 05:59 AM, Julian Scheel wrote:
>> Add support for platforms based on the Meerkat COM module. Includes
>> support
>> for the minimal reference platform called Kein Baseboard, which in
>> fact is
>> sufficient to run most existing Meerkat carriers.
>
>> diff --git a/arch/arm/dts/tegra124-meerkat.dtsi
>> b/arch/arm/dts/tegra124-meerkat.dtsi
>
>> +    aliases {
>> +        i2c0 = "/i2c at 7000c000";
>> +        i2c1 = "/i2c at 7000c400";
>> +        i2c2 = "/i2c at 7000c500";
>> +        i2c3 = "/i2c at 7000c700";
>> +        i2c4 = "/i2c at 7000d000";
>> +        i2c5 = "/i2c at 7000d100";
>
> /i2c at 7000d000 is alias i2c0 on all other Tegra boards that have I2C
> aliases.

See topic mail, I think we can change that.

>> diff --git a/arch/arm/mach-tegra/tegra124/Kconfig
>> b/arch/arm/mach-tegra/tegra124/Kconfig
>
>> +config TARGET_KBB
>> +    bool "Avionic Design Kein Baseboard"
>> +    select CPU_V7_HAS_NONSEC if !SPL_BUILD
>> +    select CPU_V7_HAS_VIRT if !SPL_BUILD
>> +    help
>> +      Minimal baseboard for Avionic Design Meerkat COM. Should work
>> with most
>> +      baseboard that follow the reference design.
>
> I hope "KBB" doesn't turn out to be too generic and cause conflicts. I
> might suggest TARGET_AD_KBB. However, I suppose we can just rename this
> if it becomes a problem later, so perhaps it isn't worth fixing.

I think changing to TARGET_KEIN_BASEBOARD might be the best? Matches the 
directory name and is unlikely to conflict with anything... Only a bit 
bulky.

>> diff --git a/board/avionic-design/common/meerkat.c
>> b/board/avionic-design/common/meerkat.c
>
>> +int tegra_pcie_board_init(void)
>
>> +    err = as3722_gpio_configure(pmic, 1, AS3722_GPIO_OUTPUT_VDDH |
>> +                         AS3722_GPIO_INVERT);
>> +    if (err < 0) {
>> +        error("failed to configure GPIO#1 as output: %d\n", err);
>> +        return err;
>> +    }
>> +
>> +    err = as3722_gpio_direction_output(pmic, 2, 1);
>> +    if (err < 0) {
>> +        error("failed to set GPIO#2 high: %d\n", err);
>> +        return err;
>> +    }
>
> Those two GPIO manipulations should likely be removed; see the recent
> change to similar code in jetson-tk1.c.

Ack. I think this code was shamelessly stolen from jetson-tk1.c without 
rethinking, anyway.

>> diff --git a/board/avionic-design/common/pinmux-config-meerkat.h
>> b/board/avionic-design/common/pinmux-config-meerkat.h
>
> I'd like to see the pinmux config for this board added to
> https://github.com/NVIDIA/tegra-pinmux-scripts
>
> That way, if we want to support other SW stacks and/or e.g. change the
> structure of these pinmux tables, we can simply re-generate this file
> with no issue.
>
> Was this file auto-generated using a downstream version of
> tegra-pinmux-scripts? There's no comment indicating it was, and the most
> recent tegra-pinmux-scripts does add such a comment...

In fact this pinmux is handcrafted and we do not maintain that excel 
sheet which tegra-pinmux-scripts uses.
I'll start a discussion internally if we could provide it with 
reasonable effort.

>> diff --git a/board/avionic-design/kein-baseboard/Kconfig
>> b/board/avionic-design/kein-baseboard/Kconfig
>
>> +if TARGET_KBB
>
> It'd be nice if this directory name matched the Kconfig symbol.

See above.

>> diff --git a/configs/kein-baseboard_defconfig
>> b/configs/kein-baseboard_defconfig
>
>> diff --git a/include/configs/kein-baseboard.h
>> b/include/configs/kein-baseboard.h
>
>> +#define CONFIG_USB_MAX_CONTROLLER_COUNT    3
>
> That define shouldn't be necessary any more, following the conversion of
> Tegra to DM USB.

Ack, didn't spot that when rebasing to recent u-boot.

>> +#define CONFIG_CMD_PCI_ENUM
>
> That define shouldn't be necessary any more, following the conversion of
> Tegra to DM PCI.

Ack again.

Thanks for the super fast review :)
-Julian