[U-Boot] [PATCHv5 4/4] Add support the Avionic Design Meerkat COM and Kein Baseboard

[Julian Scheel]

On 06.09.16 19:15, Stephen Warren wrote:
> On 09/05/2016 07:29 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
>
>> @@ -0,0 +1,409 @@
>> +
>> +#include "tegra124.dtsi"
>
> There's an unnecessary blank line at the top of the file.

Ack.

>> diff --git a/board/avionic-design/common/meerkat.c
>> b/board/avionic-design/common/meerkat.c
>
>> +void pinmux_init(void)
>> +{
>> +    pinmux_set_tristate_input_clamping();
>
> That should be pinmux_clear_tristate_input_clamping();

Ack.

> gpio_config_table() is missing here.

If I recall correctly we did remove it for some reason. I need to check 
our internal history.

>> +    pinmux_config_pingrp_table(meerkat_pingrps,
>> +                   ARRAY_SIZE(meerkat_pingrps));
>> +
>> +    pinmux_config_drvgrp_table(meerkat_drvgrps,
>> +                   ARRAY_SIZE(meerkat_drvgrps));
>
> pinmux_config_mipipadctrlgrp_table() is missing here.
>
> Most/all of these are related to not using the latest
> tegra-pinmux-scripts to generate the pin config table; see the comments
> on that below.
>> diff --git a/board/avionic-design/common/pinmux-config-meerkat.h
>> b/board/avionic-design/common/pinmux-config-meerkat.h
>
>> +/*
>> + * Copyright (c) 2014, NVIDIA CORPORATION. All rights reserved.
>> + * Copyright (c) 2015, Avionic Design GmbH
>> + *
>> + * SPDX-License-Identifier: GPL-2.0+
>> + */
>
> Can you please also add support for this board to tegra-pinmux-scripts,
> so that anyone can generate this file? That will also allow you to
> re-generate the file using the latest version of tegra-pinmux-scripts
> which will add (a) the missing "this file is auto-generated" notice, (b)
> the GPIO initialization table, (c) the MIPI padctl initialization table.

I'll discuss this internally. We have never used those scripts at all, 
but did handwrite this code, based from Jetson code.

>> diff --git a/configs/kein-baseboard_defconfig
>> b/configs/kein-baseboard_defconfig
>
>> +CONFIG_CMD_EXT4=y
>
> Relative to Jetson TK1, CONFIG_CMD_EXT4_WRITE is missing. Was that
> deliberate? I'd rather keep all the Tegra configs as similar as
> possible, at least in the upstream code.

We can add it.

>> +CONFIG_DM_I2C_COMPAT=y
>
> That's not required on any Tegra board these days. Is it necessary?

Good question, probably not. I'll check and remove.

>> +CONFIG_E1000=y
>
> I notice that CONFIG_CMD_MII isn't present, yet Ethernet is. For
> consistency with other Tegra boards, does it make sense to add
> CONFIG_CMD_MII?

We can add it.

>> +CONFIG_USB_STORAGE=y
>
> For Jetson TK1, this is defined in include/configs/jetson-tk1.h. I'd
> expect the two board configs to work the same way.

Actually we have moved this to defconfig, as we decided that all options 
that are Kconfig options by now shall be selectable via Kconfig by the user.

> CONFIG_USE_PRIVATE_LIBGCC=y is missing.

Ack.

Thanks for the review :)
-Julian