All of lore.kernel.org
 help / color / mirror / Atom feed
From: Julian Scheel <julian@jusst.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 2/2] Add support the Avionic Design Meerkat COM and Kein Baseboard
Date: Tue, 23 Feb 2016 20:47:14 +0100	[thread overview]
Message-ID: <56CCB742.1000404@jusst.de> (raw)
In-Reply-To: <56CC9EE6.9070107@wwwdotorg.org>

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

  reply	other threads:[~2016-02-23 19:47 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-23 12:59 [U-Boot] [PATCH 0/2] tegra124: Add support for Avionic Design platform Julian Scheel
2016-02-23 12:59 ` [U-Boot] [PATCH 1/2] power: as3722: Allow using on any i2c bus with any address Julian Scheel
2016-02-23 17:51   ` Stephen Warren
2016-02-23 12:59 ` [U-Boot] [PATCH 2/2] Add support the Avionic Design Meerkat COM and Kein Baseboard Julian Scheel
2016-02-23 18:03   ` Stephen Warren
2016-02-23 19:47     ` Julian Scheel [this message]
2016-02-29 10:20     ` Julian Scheel
2016-02-29 16:48       ` Stephen Warren
2016-03-02 14:40         ` Thierry Reding
2016-02-23 17:49 ` [U-Boot] [PATCH 0/2] tegra124: Add support for Avionic Design platform Stephen Warren
2016-02-23 19:40   ` Julian Scheel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56CCB742.1000404@jusst.de \
    --to=julian@jusst.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.