[U-Boot] [PATCH 4/7] Tegra30: Add arch-tegra30 include files

[Stephen Warren]

On 10/02/2012 04:45 PM, Tom Warren wrote:
> Common Tegra files are in arch-tegra, shared between T20 and T30.
> Tegra30-specific headers are in arch-tegra30. Note that some of
> these will be filled in as more T30 support is added (drivers,
> WB/LP0 support, etc.).

> diff --git a/arch/arm/include/asm/arch-tegra/gp_padctrl.h b/arch/arm/include/asm/arch-tegra/gp_padctrl.h

> +/* bit fields definitions for APB_MISC_GP_HIDREV register */
> +#define HIDREV_CHIPID_SHIFT		8
> +#define HIDREV_CHIPID_MASK		(0xff << HIDREV_CHIPID_SHIFT)
> +#define HIDREV_MAJORPREV_SHIFT		4
> +#define HIDREV_MAJORPREV_MASK		(0xf << HIDREV_MAJORPREV_SHIFT)

Patch 1 added the following:

#define GP_HIDREV	0x804

... to arch/arm/cpu/arm720t/tegra-common/cpu.h. I wonder if that line
wouldn't be better place here, alongside the macros that allow you to
use it?

> diff --git a/arch/arm/include/asm/arch-tegra30/emc.h b/arch/arm/include/asm/arch-tegra30/emc.h

This file seems to be:

a) An exact duplicate of the Tegra20 file. Did the register layout
really not change at all? That seems unlikely. If so, shouldn't the file
be shared?

b) Not used by anything in this patch series, so shouldn't be needed.

c) Incorrect; struct emc_ctlr doesn't actually match the register layout
for Tegra20 or Tegra30 (at least, offset 0 is interrupt status in HW in
both chips, not cfg as in the struct). Some fields match though.

> diff --git a/arch/arm/include/asm/arch-tegra30/funcmux.h b/arch/arm/include/asm/arch-tegra30/funcmux.h

> +/**
> + * Select a config for a particular peripheral.
> + *
> + * Each peripheral can operate through a number of configurations,
> + * which are sets of pins that it uses to bring out its signals.
> + * The basic config is 0, and higher numbers indicate different
> + * pinmux settings to bring the peripheral out on other pins,
> + *
> + * This function also disables tristate for the function's pins,
> + * so that they operate in normal mode.
> + *
> + * @param id		Peripheral id
> + * @param config	Configuration to use (FUNCMUX_...), 0 for default
> + * @return 0 if ok, -1 on error (e.g. incorrect id or config)
> + */
> +int funcmux_select(enum periph_id id, int config);

That prototype is identical to Tegra20. Shouldn't there be a common
funcmux.h that defines the prototype, either include from the
SoC-specific file, or itself including the SoC-specific file? This is
important since common code (stuff that's not specific to Tegra20 or
Tegra30) calls this function, so it shouldn't be prototyped in multiple
places (with associated possibility of divergence) even if the
implementation is entirely separate for the two SoCs.

> diff --git a/arch/arm/include/asm/arch-tegra30/gpio.h b/arch/arm/include/asm/arch-tegra30/gpio.h

> + * The Tegra 3x GPIO controller has 246 GPIOS in 8 banks of 4 ports,
> + *  each with 8 GPIOs.

Extra space there after *.

> diff --git a/arch/arm/include/asm/arch-tegra30/hardware.h b/arch/arm/include/asm/arch-tegra30/hardware.h

This file is empty except for comments. Is it really useful? The Tegra20
file is empty too.

> diff --git a/arch/arm/include/asm/arch-tegra30/pinmux-config-common.h b/arch/arm/include/asm/arch-tegra30/pinmux-config-common.h

The content of this file presumably describes Cardhu (which revision?)
Surely it should be in board/nvidia/cardhu/*.c?

> diff --git a/arch/arm/include/asm/arch-tegra30/pinmux.h b/arch/arm/include/asm/arch-tegra30/pinmux.h

> +/*
> + * Functions which can be assigned to each of the pin groups. The values here
> + * bear no relation to the values programmed into pinmux registers and are
> + * purely a convenience. The translation is done through a table search.
> + */
> +enum pmux_func {
> +	PMUX_FUNC_RSVD = 0x8000,
> +	PMUX_FUNC_RSVD0 = PMUX_FUNC_RSVD,
> +	PMUX_FUNC_BAD = 0x4000,		/* Invalid! */
> +	PMUX_FUNC_NONE = 0,

I think all four of those should be deleted.

> +	PMUX_FUNC_I2C,
> +	PMUX_FUNC_I2C1 = PMUX_FUNC_I2C,

I don't think there's a need for duplicate names for any of these.