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

Thierry Reding
2014-01-24T15:20:09Z

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>