[U-Boot] [PATCH V2 07/13] ARM: tegra: add/edit headers for Tegra124

[Thierry Reding <thierry.reding@gmail.com>]

On Thu, Jan 23, 2014 at 05:42:54PM -0700, Stephen Warren wrote:
[...]
> diff --git a/arch/arm/include/asm/arch-tegra/clk_rst.h b/arch/arm/include/asm/arch-tegra/clk_rst.h
[...]
> @@ -105,10 +128,10 @@ struct clk_rst_ctlr {
>  	uint crc_clk_cpug_cmplx;	/* _CLK_CPUG_CMPLX_0,       0x378 */
>  	uint crc_clk_cpulp_cmplx;	/* _CLK_CPULP_CMPLX_0,      0x37C */
>  	uint crc_cpu_softrst_ctrl;	/* _CPU_SOFTRST_CTRL_0,     0x380 */
> -	uint crc_cpu_softrst_ctrl1;	/* _CPU_SOFTRST_CTR1L_0,    0x384 */
> +	uint crc_cpu_softrst_ctrl1;	/* _CPU_SOFTRST_CTRL1_0,    0x384 */
>  	uint crc_cpu_softrst_ctrl2;	/* _CPU_SOFTRST_CTRL2_0,    0x388 */
>  	uint crc_reserved33[9];		/* _reserved_33,        0x38c-3ac */
> -	uint crc_clk_src_vw[TEGRA_CLK_SOURCES_VW]; /* _G3D2_0..., 0x3b0-0x42c */
> +	uint crc_clk_src_vw[TEGRA_CLK_SOURCES_VW];	/* 0x3B0-0x42C */

While at it, perhaps also make the offsets in the comment for
crc_reserved33 also all caps?

> @@ -142,6 +165,47 @@ struct clk_rst_ctlr {
[...]
> +	uint crs_reserved_50[7];	/* _reserved_50, 0x4CC-0x4E4 */
[...]
> +	uint crc_reserved51[1];		/* _reserved_51,     0x538 */
[...]
> +	uint crc_reserved52[1];		/* _reserved_52,     0x554 */
[...]
> +	uint crc_reserved60[40];	/* _reserved_60,     0x560 - 0x5FC */

Spacing in these is all somewhat inconsistent.

> +/* CLK_RST_CONTROLLER_RST_CPUxx_CMPLX_CLR 0x344 */
> +#define CLR_CPURESET0			(1 << 0)
> +#define CLR_CPURESET1			(1 << 1)
> +#define CLR_CPURESET2			(1 << 2)
> +#define CLR_CPURESET3			(1 << 3)
[...]
> +/* CLK_RST_CONTROLLER_CLK_CPU_CMPLX_CLR 0x34c */
> +#define CLR_CPU3_CLK_STP		(1 << 11)
> +#define CLR_CPU2_CLK_STP		(1 << 10)
> +#define CLR_CPU1_CLK_STP		(1 << 9)
> +#define CLR_CPU0_CLK_STP		(1 << 8)

Sometimes these are listed in order of increasing bit positions, other
times they are in order of decreasing bit positions.

> diff --git a/arch/arm/include/asm/arch-tegra/pmc.h b/arch/arm/include/asm/arch-tegra/pmc.h
[...]
> +/* APBDEV_PMC_PWRGATE_TOGGLE_0 0x30 */
> +#define PWRGATE_TOGGLE_PARTID_CRAIL		0
> +#define PWRGATE_TOGGLE_PARTID_TD		1
> +#define PWRGATE_TOGGLE_PARTID_VE		2

  +#define PWRGATE_TOGGLE_PARTID_PCX		3

> +#define PWRGATE_TOGGLE_PARTID_VDE		4
> +#define PWRGATE_TOGGLE_PARTID_L2C		5
> +#define PWRGATE_TOGGLE_PARTID_MPE		6
> +#define PWRGATE_TOGGLE_PARTID_HEG		7

  +#define PWRGATE_TOGGLE_PARTID_SAX		8

> +#define PWRGATE_TOGGLE_PARTID_CE1		9
> +#define PWRGATE_TOGGLE_PARTID_CE2		10
> +#define PWRGATE_TOGGLE_PARTID_CE3		11
> +#define PWRGATE_TOGGLE_PARTID_CELP		12
> +#define PWRGATE_TOGGLE_PARTID_CE0		14
> +#define PWRGATE_TOGGLE_PARTID_C0NC		15
> +#define PWRGATE_TOGGLE_PARTID_C1NC		16

  +#define PWRGATE_TOGGLE_PARTID_SOR		17

> +#define PWRGATE_TOGGLE_PARTID_DIS		18
> +#define PWRGATE_TOGGLE_PARTID_DISB		19
> +#define PWRGATE_TOGGLE_PARTID_XUSBA		20
> +#define PWRGATE_TOGGLE_PARTID_XUSBB		21
> +#define PWRGATE_TOGGLE_PARTID_XUSBC		22

  +#define PWRGATE_TOGGLE_PARTID_VIC		23
  +#define PWRGATE_TOGGLE_PARTID_IRAM		24

> +/* APBDEV_PMC_PWRGATE_STATUS_0 0x38 */
> +#define PWRGATE_STATUS_CRAIL_ENABLE		(1 << 0)
> +#define PWRGATE_STATUS_TD_ENABLE		(1 << 1)
> +#define PWRGATE_STATUS_VE_ENABLE		(1 << 2)

  +#define PWRGATE_STATUS_PCX_ENABLE		(1 << 3)

> +#define PWRGATE_STATUS_VDE_ENABLE		(1 << 4)
> +#define PWRGATE_STATUS_L2C_ENABLE		(1 << 5)
> +#define PWRGATE_STATUS_MPE_ENABLE		(1 << 6)
> +#define PWRGATE_STATUS_HEG_ENABLE		(1 << 7)

  +#define PWRGATE_STATUS_SAX_ENABLE		(1 << 8)

> +#define PWRGATE_STATUS_CE1_ENABLE		(1 << 9)
> +#define PWRGATE_STATUS_CE2_ENABLE		(1 << 10)
> +#define PWRGATE_STATUS_CE3_ENABLE		(1 << 11)
> +#define PWRGATE_STATUS_CELP_ENABLE		(1 << 12)
> +#define PWRGATE_STATUS_CE0_ENABLE		(1 << 14)
> +#define PWRGATE_STATUS_C0NC_ENABLE		(1 << 15)
> +#define PWRGATE_STATUS_C1NC_ENABLE		(1 << 16)

  +#define PWRGATE_STATUS_SOR_ENABLE		(1 << 17)

> +#define PWRGATE_STATUS_DIS_ENABLE		(1 << 18)
> +#define PWRGATE_STATUS_DISB_ENABLE		(1 << 19)
> +#define PWRGATE_STATUS_XUSBA_ENABLE		(1 << 20)
> +#define PWRGATE_STATUS_XUSBB_ENABLE		(1 << 21)
> +#define PWRGATE_STATUS_XUSBC_ENABLE		(1 << 22)

  +#define PWRGATE_STATUS_VIC_ENABLE		(1 << 23)
  +#define PWRGATE_STATUS_IRAM_ENABLE		(1 << 24)

> diff --git a/arch/arm/include/asm/arch-tegra124/clock-tables.h b/arch/arm/include/asm/arch-tegra124/clock-tables.h
> +/* The PLLs supported by the hardware */
> +enum clock_id {
> +	CLOCK_ID_FIRST,
> +	CLOCK_ID_CGENERAL = CLOCK_ID_FIRST,
> +	CLOCK_ID_MEMORY,
> +	CLOCK_ID_PERIPH,
> +	CLOCK_ID_AUDIO,
> +	CLOCK_ID_USB,
> +	CLOCK_ID_DISPLAY,
> +
> +	/* now the simple ones */
> +	CLOCK_ID_FIRST_SIMPLE,
> +	CLOCK_ID_XCPU = CLOCK_ID_FIRST_SIMPLE,
> +	CLOCK_ID_EPCI,
> +	CLOCK_ID_SFROM32KHZ,
> +
> +	/* These are the base clocks (inputs to the Tegra SOC) */

Nit: SOC -> SoC

> +	CLOCK_ID_32KHZ,
> +	CLOCK_ID_OSC,
> +
> +	CLOCK_ID_COUNT,	/* number of PLLs */
> +
> +	/*
> +	 * These are clock ids that are used in table clock_source[][]

Nit: ids -> IDs

> diff --git a/arch/arm/include/asm/arch-tegra124/clock.h b/arch/arm/include/asm/arch-tegra124/clock.h
[...]
> +/* Tegra124 clock control functions */

There aren't actually any functions in this header file. =P

> +#ifndef _TEGRA124_CLOCK_H_
> +#define _TEGRA124_CLOCK_H_
> +
> +#include <asm/arch-tegra/clock.h>
> +
> +/* CLK_RST_CONTROLLER_OSC_CTRL_0 */
> +#define OSC_FREQ_SHIFT          28
> +#define OSC_FREQ_MASK           (0xF << OSC_FREQ_SHIFT)
> +
> +#endif	/* _TEGRA124_CLOCK_H_ */

> diff --git a/arch/arm/include/asm/arch-tegra124/gpio.h b/arch/arm/include/asm/arch-tegra124/gpio.h
[...]
> +enum gpio_pin {
> +	GPIO_PA0 = 0,	/* pin 0 */
[...]
> +	GPIO_PFF7,	/* pin 255 */
> +};

Perhaps this should be converted to something similar to what we have in
the kernel? We essentially duplicate this list for every SoC generation.

> diff --git a/arch/arm/include/asm/arch-tegra124/spl.h b/arch/arm/include/asm/arch-tegra124/spl.h
[...]
> +#ifndef	_ARCH_TEGRA_SPL_H_
> +#define	_ARCH_TEGRA_SPL_H_

Nit: I don't think any of the other include guards use a tab, but this
one does.

> +
> +#define BOOT_DEVICE_RAM         1
> +
> +#endif

Also all the other headers have a comment with the identifier on this
last line.

Mostly these are just minor nits and I don't feel too strongly about
them. But I'd like to see at least the powergate partition table
completed with the missing entries.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140124/7da5ef3f/attachment.pgp>