Add basic Tegra30 (T30) support

Signed-off-by Tom Warren <[email protected]>

Quoted 1 line · Tom Warren

Signed-off-by Tom Warren <[email protected]>

A few things:
- I see some #define FOO[space][space]val that should be [tab]

- I didn't checkpatch.pl this (nor the whole series) but please do and let us know if it's clean or why the warnings are false positives.

- My preference is to bring in includes and C files and Makefiles and so on all at once, when each is needed / useful. YMMV and not a big deal.
- But please make sure that you aren't adding defines / structs / etc that aren't used at some point by the end of the series at least. Removing (and correcting!) structs and defines was one of the things I had to do on am33xx. If it wasn't added until the corresponding driver work was being pushed, it'd have saved me some time.

--
Tom

Tom,

Quoted 6 lines · Tom Rini

Quoted 1 line · Tom Warren

Signed-off-by Tom Warren <[email protected]>

A few things:
- I see some #define FOO[space][space]val that should be [tab]

Probably copied over from Tegra20 files. I'll turn on whitespace highlighting in my editor and fix 'em up.

Quoted 2 lines

- I didn't checkpatch.pl this (nor the whole series) but please do and let us know if it's clean or why the warnings are false positives.

I always run checkpath before submitting. I'll put a notice to that affect in the next version. Checkpatch ran clean w/only 1 false-positive about 'macros with complex values should be enclosed in parenthesis' for the "#define CONFIG_DEFAULT_DEVICE_TREE tegra30-cardhu" line in cardhu.h.

Quoted 9 lines

- My preference is to bring in includes and C files and Makefiles and so on all at once, when each is needed / useful. YMMV and not a big deal.
- But please make sure that you aren't adding defines / structs / etc that aren't used at some point by the end of the series at least. Removing (and correcting!) structs and defines was one of the things I had to do on am33xx. If it wasn't added until the corresponding driver work was being pushed, it'd have saved me some time.

Sure, and that's good advice. I took a couple of passes during the port to try and remove vestigial and/or useless/unsupported files, features, and code, but I'm sure I missed some (as Stephen has already pointed out). I'll address those in V2.

Thanks,

Tom

Quoted 2 lines

--
Tom

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Hi Tom,

Quoted 32 lines · Tom Warren

Tom,

Quoted 6 lines · Tom Rini

Quoted 1 line · Tom Warren

Signed-off-by Tom Warren <[email protected]>

A few things:
- I see some #define FOO[space][space]val that should be [tab]

Probably copied over from Tegra20 files. I'll turn on whitespace highlighting in my editor and fix 'em up.

Quoted 2 lines

- I didn't checkpatch.pl this (nor the whole series) but please do and let us know if it's clean or why the warnings are false positives.

I always run checkpath before submitting. I'll put a notice to that affect in the next version. Checkpatch ran clean w/only 1 false-positive about 'macros with complex values should be enclosed in parenthesis' for the "#define CONFIG_DEFAULT_DEVICE_TREE tegra30-cardhu" line in cardhu.h.

Quoted 9 lines

- My preference is to bring in includes and C files and Makefiles and so on all at once, when each is needed / useful. YMMV and not a big deal.
- But please make sure that you aren't adding defines / structs / etc that aren't used at some point by the end of the series at least. Removing (and correcting!) structs and defines was one of the things I had to do on am33xx. If it wasn't added until the corresponding driver work was being pushed, it'd have saved me some time.

Quoted 4 lines

Sure, and that's good advice. I took a couple of passes during the port to try and remove vestigial and/or useless/unsupported files, features, and code, but I'm sure I missed some (as Stephen has already pointed out). I'll address those in V2.

Congrats on getting this out. It is a lot of work!

In the Chromium tree, we have an arch-tegra directory where a lot of the common code lives. It sounds like you are going to split that out a bit which is good. I suspect you may also want a few patches to move quite a bit of the code in arch/arm/include/arch-tegra20 to arch/arm/include/arch-tegra.

Regards,
Simon

Quoted 10 lines

Thanks,

Tom

Quoted 2 lines

--
Tom

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Signed-off-by Tom Warren <[email protected]>

arch/arm/include/asm/arch-tegra30/clk_rst.h +3 -0

+/* The clocks supported by the hardware */
+enum periph_id {
+	PERIPH_ID_FIRST,

arch/arm/include/asm/arch-tegra30/clk_rst.h +8 -0

+/*
+ * Clock peripheral IDs which sadly don't match up with PERIPH_ID. we want
+ * callers to use the PERIPH_ID for all access to peripheral clocks to avoid
+ * confusion bewteen PERIPH_ID_... and PERIPHC_...
+ *
+ * We don't call this CLOCK_PERIPH_ID or PERIPH_CLOCK_ID as it would just be
+ * confusing.
+ */

+void clock_enable(enum periph_id clkid);

arch/arm/include/asm/arch-tegra30/funcmux.h +21 -0

 
+/* Configs supported by the func mux */
+enum {
+	FUNCMUX_DEFAULT = 0,	/* default config */
+
+	/* UART configs */
+	FUNCMUX_UART1_ULPI_UART2 = 0,
+	FUNCMUX_UART2_IRDA = 0,
+	FUNCMUX_UART4_GMC = 0,
+
+	/* I2C configs */
+	FUNCMUX_DVC_I2CP = 0,
+	FUNCMUX_I2C1_RM = 0,
+	FUNCMUX_I2C2_DDC = 0,
+	FUNCMUX_I2C2_PTA,
+	FUNCMUX_I2C3_DTF = 0,
+
+	/* SDMMC configs */
+	FUNCMUX_SDMMC1_SDIO1_4BIT = 0,
+	FUNCMUX_SDMMC2_DTA_DTD_8BIT = 0,
+	FUNCMUX_SDMMC3_SDB_4BIT = 0,
+	FUNCMUX_SDMMC3_SDB_SLXA_8BIT,

arch/arm/include/asm/arch-tegra30/gp_padctrl.h +26 -0

 
+/* APB_MISC_GP and padctrl registers */
+struct apb_misc_gp_ctlr {
+	u32	modereg;	/* 0x00: APB_MISC_GP_MODEREG */
+	u32	hidrev;		/* 0x04: APB_MISC_GP_HIDREV */
+	u32	reserved0[22];	/* 0x08 - 0x5C: */
+	u32	emu_revid;	/* 0x60: APB_MISC_GP_EMU_REVID */
+	u32	xactor_scratch;	/* 0x64: APB_MISC_GP_XACTOR_SCRATCH */
+	u32	aocfg1;		/* 0x68: APB_MISC_GP_AOCFG1PADCTRL */
+	u32	aocfg2;		/* 0x6c: APB_MISC_GP_AOCFG2PADCTRL */
+	u32	atcfg1;		/* 0x70: APB_MISC_GP_ATCFG1PADCTRL */
+	u32	atcfg2;		/* 0x74: APB_MISC_GP_ATCFG2PADCTRL */
+	u32	cdevcfg1;	/* 0x78: APB_MISC_GP_CDEV1CFGPADCTRL */
+	u32	cdevcfg2;	/* 0x7C: APB_MISC_GP_CDEV2CFGPADCTRL */
+	u32	csuscfg;	/* 0x80: APB_MISC_GP_CSUSCFGPADCTRL */
+	u32	dap1cfg;	/* 0x84: APB_MISC_GP_DAP1CFGPADCTRL */
+	u32	dap2cfg;	/* 0x88: APB_MISC_GP_DAP2CFGPADCTRL */
+	u32	dap3cfg;	/* 0x8C: APB_MISC_GP_DAP3CFGPADCTRL */
+	u32	dap4cfg;	/* 0x90: APB_MISC_GP_DAP4CFGPADCTRL */
+	u32	dbgcfg;		/* 0x94: APB_MISC_GP_DBGCFGPADCTRL */
+	u32	lcdcfg1;	/* 0x98: APB_MISC_GP_LCDCFG1PADCTRL */
+	u32	lcdcfg2;	/* 0x9C: APB_MISC_GP_LCDCFG2PADCTRL */
+	u32	sdmmc2_cfg;	/* 0xA0: APB_MISC_GP_SDMMC2CFGPADCTRL */
+	u32	sdmmc3_cfg;	/* 0xA4: APB_MISC_GP_SDMMC3CFGPADCTRL */
+	u32	spicfg;		/* 0xA8: APB_MISC_GP_SPICFGPADCTRL */
+	u32	uaacfg;		/* 0xAC: APB_MISC_GP_UAACFGPADCTRL */
+	u32	uabcfg;		/* 0xB0: APB_MISC_GP_UABCFGPADCTRL */

arch/arm/include/asm/arch-tegra30/pinmux.h +3 -0

 
+/*
+ * Pin groups which we adjust. There are three basic attributes of each pin
+ * group which use this enum:

arch/arm/include/asm/arch-tegra30/pinmux.h +9 -0

+/* t30 pin drive group and pin mux registers */
+#define PDRIVE_PINGROUP_OFFSET	(0x868 >> 2)
+#define PMUX_OFFSET	((0x3000 >> 2) - PDRIVE_PINGROUP_OFFSET - \
+				PDRIVE_PINGROUP_COUNT)
+struct pmux_tri_ctlr {
+	uint pmt_reserved0;		/* ABP_MISC_PP_ reserved offset 00 */
+	uint pmt_reserved1;		/* ABP_MISC_PP_ reserved offset 04 */
+	uint pmt_strap_opt_a;		/* _STRAPPING_OPT_A_0, offset 08   */
+	uint pmt_reserved2;		/* ABP_MISC_PP_ reserved offset 0C */

+/**
+ * Configuure a list of pin groups

Quoted 1 line · Tom Warren

Signed-off-by Tom Warren <[email protected]>

Hmm. This is rather large to review, but I tried to at least glance through it all and spot obvious issues...

arch/arm/include/asm/arch-tegra30/ap30.h line 50

Quoted 4 lines

arch/arm/include/asm/arch-tegra30/ap30.h +2 -0

 
+/* T30 Base physical address of SDRAM. */
+#define T30_BASE_PA_SDRAM      0x00000000

That should be 0x80000000. The fact anything still works with this issue implies this constant isn't used anywhere? Aha, I see NV_PA_SDRAM_BASE defined later with the correct value...

arch/arm/include/asm/arch-tegra30/ap30.h line 56

Quoted 3 lines

arch/arm/include/asm/arch-tegra30/ap30.h +2 -0

+/* T30 Base physical address of flash. */
+#define T30_BASE_PA_NOR_FLASH  0xD0000000

I don't know where NOR actually is mapped, but it can't be there; that's in the middle of SDRAM (2G..4G-1)

arch/arm/include/asm/arch-tegra30/board.h line 25

Quoted 5 lines

arch/arm/include/asm/arch-tegra30/board.h +2 -0

 
+#ifndef _TEGRA_BOARD_H_
+#define _TEGRA_BOARD_H_

I wonder if include guards shouldn't say TEGRA30 not just TEGRA. That would avoid any conflict between the different arch/arm/include/asm/arch-tegra* directories. Sure, that's unlikely right now, but if we really continue to push device tree, maybe we'll end up with a unified Tegra20/Tegra30 bootloader image, and need to include both.

arch/arm/include/asm/arch-tegra30/clk_rst.h line 25

Quoted 5 lines

arch/arm/include/asm/arch-tegra30/clk_rst.h +2 -0

 
+#ifndef _CLK_RST_H_
+#define _CLK_RST_H_

Hmm, and the naming format isn't consistent.

Quoted 4 lines

arch/arm/include/asm/arch-tegra30/clk_rst.h +3 -0

+/* The clocks supported by the hardware */
+enum periph_id {
+     PERIPH_ID_FIRST,

...

Quoted 18 lines

arch/arm/include/asm/arch-tegra30/clk_rst.h +18 -0

+/*
+ * Clock peripheral IDs which sadly don't match up with PERIPH_ID. we want
+ * callers to use the PERIPH_ID for all access to peripheral clocks to avoid
+ * confusion bewteen PERIPH_ID_... and PERIPHC_...
+ *
+ * We don't call this CLOCK_PERIPH_ID or PERIPH_CLOCK_ID as it would just be
+ * confusing.
+ */
+enum periphc_internal_id {
+     /* 0x00 */
+     PERIPHC_I2S1,
+     PERIPHC_I2S2,
+     PERIPHC_SPDIF_OUT,
+     PERIPHC_SPDIF_IN,
+     PERIPHC_PWM,
+     PERIPHC_05h,
+     PERIPHC_SBC2,
+     PERIPHC_SBC3,

...

Can you make sure that one/both (whichever is appropriate) of these line up with the proposed Linux kernel Tegra30 clock binding clock IDs. I'm not sure if the Tegra30 proposed binding has been published yet, but Prashant Gaikwad can email you the patch off-list if you need.

Quoted 2 lines

+void clock_enable(enum periph_id clkid);

Hmm. I would have expected all the prototypes to be in a common header between Tegra20 and Tegra30?

Quoted 24 lines

arch/arm/include/asm/arch-tegra30/funcmux.h +21 -0

 
+/* Configs supported by the func mux */
+enum {
+     FUNCMUX_DEFAULT = 0,    /* default config */
+
+     /* UART configs */
+     FUNCMUX_UART1_ULPI_UART2 = 0,
+     FUNCMUX_UART2_IRDA = 0,
+     FUNCMUX_UART4_GMC = 0,
+
+     /* I2C configs */
+     FUNCMUX_DVC_I2CP = 0,
+     FUNCMUX_I2C1_RM = 0,
+     FUNCMUX_I2C2_DDC = 0,
+     FUNCMUX_I2C2_PTA,
+     FUNCMUX_I2C3_DTF = 0,
+
+     /* SDMMC configs */
+     FUNCMUX_SDMMC1_SDIO1_4BIT = 0,
+     FUNCMUX_SDMMC2_DTA_DTD_8BIT = 0,
+     FUNCMUX_SDMMC3_SDB_4BIT = 0,
+     FUNCMUX_SDMMC3_SDB_SLXA_8BIT,

arch/arm/include/asm/arch-tegra30/funcmux.h line 58

Quoted 10 lines

arch/arm/include/asm/arch-tegra30/funcmux.h +10 -0

+     FUNCMUX_SDMMC4_ATC_ATD_8BIT = 0,
+     FUNCMUX_SDMMC4_ATB_GMA_4_BIT,
+     FUNCMUX_SDMMC4_ATB_GMA_GME_8_BIT,
+
+     /* USB configs */
+     FUNCMUX_USB2_ULPI = 0,
+
+     /* Serial Flash configs */
+     FUNCMUX_SPI1_GMC_GMD = 0,
+};

Those all look like Tegra20 options. Tegra30 doesn't have mux pin groups (muxing is per-pin now), so names like DTA, DTD, SDB, SLXA, ATB, GMA, ... above don't make any sense (they're Tegra20 pin group names).

In turn, I wonder if a funcmux.h-style API even makes sense for Tegra30, since the number of legal configurations is probably too large to usefully enumerate, but that is perhaps another question.

Quoted 34 lines

arch/arm/include/asm/arch-tegra30/gp_padctrl.h +31 -0

 
+/* APB_MISC_GP and padctrl registers */
+struct apb_misc_gp_ctlr {
+     u32     modereg;        /* 0x00: APB_MISC_GP_MODEREG */
+     u32     hidrev;         /* 0x04: APB_MISC_GP_HIDREV */
+     u32     reserved0[22];  /* 0x08 - 0x5C: */
+     u32     emu_revid;      /* 0x60: APB_MISC_GP_EMU_REVID */
+     u32     xactor_scratch; /* 0x64: APB_MISC_GP_XACTOR_SCRATCH */
+     u32     aocfg1;         /* 0x68: APB_MISC_GP_AOCFG1PADCTRL */
+     u32     aocfg2;         /* 0x6c: APB_MISC_GP_AOCFG2PADCTRL */
+     u32     atcfg1;         /* 0x70: APB_MISC_GP_ATCFG1PADCTRL */
+     u32     atcfg2;         /* 0x74: APB_MISC_GP_ATCFG2PADCTRL */
+     u32     cdevcfg1;       /* 0x78: APB_MISC_GP_CDEV1CFGPADCTRL */
+     u32     cdevcfg2;       /* 0x7C: APB_MISC_GP_CDEV2CFGPADCTRL */
+     u32     csuscfg;        /* 0x80: APB_MISC_GP_CSUSCFGPADCTRL */
+     u32     dap1cfg;        /* 0x84: APB_MISC_GP_DAP1CFGPADCTRL */
+     u32     dap2cfg;        /* 0x88: APB_MISC_GP_DAP2CFGPADCTRL */
+     u32     dap3cfg;        /* 0x8C: APB_MISC_GP_DAP3CFGPADCTRL */
+     u32     dap4cfg;        /* 0x90: APB_MISC_GP_DAP4CFGPADCTRL */
+     u32     dbgcfg;         /* 0x94: APB_MISC_GP_DBGCFGPADCTRL */
+     u32     lcdcfg1;        /* 0x98: APB_MISC_GP_LCDCFG1PADCTRL */
+     u32     lcdcfg2;        /* 0x9C: APB_MISC_GP_LCDCFG2PADCTRL */
+     u32     sdmmc2_cfg;     /* 0xA0: APB_MISC_GP_SDMMC2CFGPADCTRL */
+     u32     sdmmc3_cfg;     /* 0xA4: APB_MISC_GP_SDMMC3CFGPADCTRL */
+     u32     spicfg;         /* 0xA8: APB_MISC_GP_SPICFGPADCTRL */
+     u32     uaacfg;         /* 0xAC: APB_MISC_GP_UAACFGPADCTRL */
+     u32     uabcfg;         /* 0xB0: APB_MISC_GP_UABCFGPADCTRL */
+     u32     uart2cfg;       /* 0xB4: APB_MISC_GP_UART2CFGPADCTRL */
+     u32     uart3cfg;       /* 0xB8: APB_MISC_GP_UART3CFGPADCTRL */
+     u32     vicfg1;         /* 0xBC: APB_MISC_GP_VICFG1PADCTRL */
+     u32     vicfg2;         /* 0xC0: APB_MISC_GP_VICFG2PADCTRL */
+     u32     xm2cfga;        /* 0xC4: APB_MISC_GP_XM2CFGAPADCTRL */

arch/arm/include/asm/arch-tegra30/gp_padctrl.h line 62

Quoted 5 lines

arch/arm/include/asm/arch-tegra30/gp_padctrl.h +5 -0

+     u32     xm2cfgc;        /* 0xC8: APB_MISC_GP_XM2CFGCPADCTRL */
+     u32     xm2cfgd;        /* 0xCC: APB_MISC_GP_XM2CFGDPADCTRL */
+     u32     xm2clkcfg;      /* 0xD0: APB_MISC_GP_XM2CLKCFGPADCTRL */
+     u32     memcomp;        /* 0xD4: APB_MISC_GP_MEMCOMPPADCTRL */
+};

That is the Tegra20 layout. It's change for Tegra30. The set of fields is probably even quite different.

arch/arm/include/asm/arch-tegra30/gp_padctrl.h line 71

Quoted 9 lines

arch/arm/include/asm/arch-tegra30/gp_padctrl.h +8 -0

+/* 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)
+
+/* CHIPID field returned from APB_MISC_GP_HIDREV register */
+#define CHIPID_TEGRA20                               0x20

There should be a Tegra30 define here, and since that register is common between Tegra20 and Tegra30, I'd expect this to be in a common header.

arch/arm/include/asm/arch-tegra30/mmc.h

Quoted 4 lines

arch/arm/include/asm/arch-tegra30/mmc.h +1 -0

 
+int tegra_mmc_init(int dev_index, int bus_width, int pwr_gpio, int cd_gpio);

Wouldn't this be common too?

Quoted 6 lines

arch/arm/include/asm/arch-tegra30/pinmux.h +3 -0

 
+/*
+ * Pin groups which we adjust. There are three basic attributes of each pin
+ * group which use this enum:

arch/arm/include/asm/arch-tegra30/pinmux.h line 39

Quoted 10 lines

arch/arm/include/asm/arch-tegra30/pinmux.h +10 -0

+ *
+ *   - function
+ *   - pullup / pulldown
+ *   - tristate or normal
+ */
+enum pmux_pingrp {
+     PINGRP_ULPI_DATA0 = 0,  /* offset 0x3000 */
+     PINGRP_ULPI_DATA1,
+     PINGRP_ULPI_DATA2,
+     PINGRP_ULPI_DATA3,

Hmmm. This enum appears to have been picked based on register order in the pin controller which I suppose is fine.

However, that order doesn't match the GPIO order, so if you ever want function gpio_request(n) to program both the GPIO controller and pin controller, then you'll need a table to convert between the two sets of IDs. For better or worse, the device tree binding for the Tegra30 pin controller lists the names based on GPIO ID order, and hence if the bindings are ever converted to integer-based rather than string-based, would probably number the pins based on GPIO order too.

I guess this means that either way you'll need a mapping table, either from gpio-or-binding-id to pinmux register, or from this enum to pinmux-register.

So I guess that means this enum is fine - it's just something to watch out for.

Quoted 12 lines

arch/arm/include/asm/arch-tegra30/pinmux.h +11 -0

+/*
+ * 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_AHB_CLK,
+     PMUX_FUNC_APB_CLK,
+     PMUX_FUNC_AUDIO_SYNC,
+     PMUX_FUNC_CRT,
+     PMUX_FUNC_DAP1,

...

Could you possibly update the order of this enum to match the Linux kernel; see drivers/pinctrl/pinctrl-tegra30.c. Again, such an order would be more likely to match any integer-based device-tree pinmux binding.

Re-ordering this shouldn't be an issue since as the comment says there must be a conversion table anyway.

Quoted 15 lines

arch/arm/include/asm/arch-tegra30/pinmux.h +15 -0

+/* t30 pin drive group and pin mux registers */
+#define PDRIVE_PINGROUP_OFFSET       (0x868 >> 2)
+#define PMUX_OFFSET  ((0x3000 >> 2) - PDRIVE_PINGROUP_OFFSET - \
+                             PDRIVE_PINGROUP_COUNT)
+struct pmux_tri_ctlr {
+     uint pmt_reserved0;             /* ABP_MISC_PP_ reserved offset 00 */
+     uint pmt_reserved1;             /* ABP_MISC_PP_ reserved offset 04 */
+     uint pmt_strap_opt_a;           /* _STRAPPING_OPT_A_0, offset 08   */
+     uint pmt_reserved2;             /* ABP_MISC_PP_ reserved offset 0C */
+     uint pmt_reserved3;             /* ABP_MISC_PP_ reserved offset 10 */
+     uint pmt_reserved4[4];          /* _TRI_STATE_REG_A/B/C/D in t20 */
+     uint pmt_cfg_ctl;               /* _CONFIG_CTL_0, offset 24        */
+
+     uint pmt_reserved[528];         /* ABP_MISC_PP_ reserved offs 28-864 */
+

arch/arm/include/asm/arch-tegra30/pinmux.h line 562

Quoted 4 lines

arch/arm/include/asm/arch-tegra30/pinmux.h +4 -0

+     uint pmt_drive[PDRIVE_PINGROUP_COUNT];  /* pin drive grps offs 868 */
+     uint pmt_reserved5[PMUX_OFFSET];        /* offset 0x3000 */
+     uint pmt_ctl[PINGRP_COUNT];     /* pin mux/pupd/tristate regs  */
+};

Isn't pmc_ctrl at 0x3000; the second comment above appears misleading. Also, PMUX_OFFSET isn't right; the offset is 0x3000; it's more like RESERVED5_SIZE.

Quoted 3 lines

+/**
+ * Configuure a list of pin groups

Typo above.

arch/arm/include/asm/arch-tegra30/pmu.h line 28

Quoted 5 lines

arch/arm/include/asm/arch-tegra30/pmu.h +2 -0

 
+/* Set core and CPU voltages to nominal levels */
+int pmu_set_nominal(void);

That also seems common. I'll stop pointing this issue out.

Quoted 4 lines

arch/arm/include/asm/arch-tegra30/tegra30.h +1 -0

 
+/* These are the available SKUs (product types) for Tegra */

arch/arm/include/asm/arch-tegra30/tegra30.h line 86

Quoted 10 lines

arch/arm/include/asm/arch-tegra30/tegra30.h +10 -0

+enum {
+     SKU_ID_T20              = 0x8,
+     SKU_ID_T25SE            = 0x14,
+     SKU_ID_AP25             = 0x17,
+     SKU_ID_T25              = 0x18,
+     SKU_ID_AP25E            = 0x1b,
+     SKU_ID_T25E             = 0x1c,
+
+     SKU_ID_T30              = 0x81, /* Cardhu value */
+};

There's little point defining Tegra20-specific values unless this header is shared between the two SoCs. Same for:

arch/arm/include/asm/arch-tegra30/tegra30.h line 103

Quoted 10 lines

arch/arm/include/asm/arch-tegra30/tegra30.h +10 -0

+enum {
+     TEGRA_SOC_T20,
+     TEGRA_SOC_T25,
+     TEGRA_SOC_T30,
+     TEGRA_SOC_T30_408MHZ,   /* A T30 with faster PLLP */
+     TEGRA_SOC2_SLOW,        /* T2x needs to run at slow clock initially */
+
+     TEGRA_SOC_COUNT,
+     TEGRA_SOC_UNKNOWN       = -1,
+};

arch/arm/include/asm/arch-tegra30/warmboot.h

Quoted 1 line

arch/arm/include/asm/arch-tegra30/warmboot.h +0 -0

At a quick glance, this header appears identical to Tegra20, yet given the differences between sleep modes on the two SoCs, I'd expect to see quite a few differences. I know a lot of extra PMC scratch registers were added on Tegra30 in order to support the sleep mode code, and hence it works quite differently...

Overall, it might help identify problems if you passed the "-C" option to "git format-patch"; that would show up any headers that had simply been copied from Tegra20 without necessary modifications.

arch/arm/cpu/arm720t/tegra30/cpu.c +9 -0

+static void enable_cpu_power_rail(void)
+{
+	struct pmc_ctlr *pmc = (struct pmc_ctlr *)NV_PA_PMC_BASE;
+	u32 reg;
+
+	debug("enable_cpu_power_rail entry\n");
+	reg = readl(&pmc->pmc_cntrl);
+	reg |= CPUPWRREQ_OE;
+	writel(reg, &pmc->pmc_cntrl);

On 09/12/2012 04:10 PM, Tom Warren wrote:

Patch descriptions would be nice.

arch/arm/cpu/arm720t/tegra30/cpu.c

Quoted 2 lines

arch/arm/cpu/arm720t/tegra30/cpu.c +0 -0

There's quite a bit of Tegra20-support in this file. Can this file be shared with Tegra20 rather than forked and enhanced?

arch/arm/cpu/arm720t/tegra30/cpu.c line 154

Quoted 6 lines

arch/arm/cpu/arm720t/tegra30/cpu.c +6 -0

+/* Returns 1 if the current CPU executing is a Cortex-A9, else 0 */
+int cpu_is_cortexa9(void)
+{
+     u32 id = readb(NV_PA_PG_UP_BASE + PG_UP_TAG_0);
+     return id == (PG_UP_TAG_0_PID_CPU & 0xff);
+}

Hmm. Given this is support for the AVP/COP running SPL, shouldn't this always be true? I thought Allen's SPL patches had cleaned this up.

Quoted 16 lines

arch/arm/cpu/arm720t/tegra30/cpu.c +15 -0

+static void enable_cpu_power_rail(void)
+{
+     struct pmc_ctlr *pmc = (struct pmc_ctlr *)NV_PA_PMC_BASE;
+     u32 reg;
+
+     debug("enable_cpu_power_rail entry\n");
+     reg = readl(&pmc->pmc_cntrl);
+     reg |= CPUPWRREQ_OE;
+     writel(reg, &pmc->pmc_cntrl);
+
+     /*
+      * Pulse PWRREQ via I2C.  We need to find out what this is
+      * doing, tidy up the code and maybe find a better place for it.
+      */
+     tegra_i2c_ll_write_addr(0x005a, 0x0002);

arch/arm/cpu/arm720t/tegra30/cpu.c line 380

Quoted 4 lines

arch/arm/cpu/arm720t/tegra30/cpu.c +4 -0

+     tegra_i2c_ll_write_data(0x2328, 0x0a02);
+     udelay(1000);
+     tegra_i2c_ll_write_data(0x0127, 0x0a02);
+     udelay(10 * 1000);

Those functions access the DVC I2C controller's register space, so presumably they're doing I2C accesses. Not all boards use the same PMIC, so it seems like we really do need to factor this out.

arch/arm/cpu/arm720t/tegra30/cpu.c line 390

Quoted 10 lines

arch/arm/cpu/arm720t/tegra30/cpu.c +9 -0

+     /*
+      * The TI PMU65861C needs a 3.75ms delay between enabling
+      * the power rail and enabling the CPU clock.  This delay
+      * between SM1EN and SM1 is for switching time + the ramp
+      * up of the voltage to the CPU (VDD_CPU from PMU). We use 0xf00 as
+      * is is ARM-friendly (can fit in a single ARMv4T mov immmediate
+      * instruction).
+      */
+     udelay(3840);

The Cardhu board at least does not use the TPS65861. At the very least the comment isn't quite right. Is this code needed?

arch/arm/cpu/arm720t/tegra30/spl.c line 72

Quoted 12 lines

arch/arm/cpu/arm720t/tegra30/spl.c +10 -0

 
+void board_init_f(ulong dummy)
+{
+     board_init_uart_f();
+
+     /* Initialize periph GPIOs */
+#ifdef CONFIG_SPI_UART_SWITCH
+     gpio_early_init_uart();
+#else
+     gpio_config_uart();
+#endif

Didn't we have patches to get rid of that mess and just use the same function consistently across all boards, or was that only discussed and never actually implemented?

Quoted 17 lines

arch/arm/cpu/arm720t/tegra30/spl.c line 72

Quoted 13 lines

arch/arm/cpu/arm720t/tegra30/spl.c +10 -0

 
+void board_init_f(ulong dummy)
+{
+     board_init_uart_f();
+
+     /* Initialize periph GPIOs */
+#ifdef CONFIG_SPI_UART_SWITCH
+     gpio_early_init_uart();
+#else
+     gpio_config_uart();
+#endif

Didn't we have patches to get rid of that mess and just use the same function consistently across all boards, or was that only discussed and never actually implemented?

Someone on the list talked about a cleanup (Lucas? Thierry?), but, AFAIK, that never happened, or I would have put it in tegra/next. At the very least, it's not needed in Tegra30/spl.c, so I'll remove it/clean it up.

Hi Tom,

Am Donnerstag, den 13.09.2012, 14:00 -0700 schrieb Tom Warren:
[...]

Quoted 18 lines

Quoted 17 lines

arch/arm/cpu/arm720t/tegra30/spl.c line 72

Quoted 13 lines

arch/arm/cpu/arm720t/tegra30/spl.c +10 -0

 
+void board_init_f(ulong dummy)
+{
+     board_init_uart_f();
+
+     /* Initialize periph GPIOs */
+#ifdef CONFIG_SPI_UART_SWITCH
+     gpio_early_init_uart();
+#else
+     gpio_config_uart();
+#endif

Didn't we have patches to get rid of that mess and just use the same function consistently across all boards, or was that only discussed and never actually implemented?

Quoted 4 lines

Someone on the list talked about a cleanup (Lucas? Thierry?), but, AFAIK, that never happened, or I would have put it in tegra/next. At the very least, it's not needed in Tegra30/spl.c, so I'll remove it/clean it up.

I did the cleanup and you in fact missed it in the last round of tegra/next. Patch is called "tegra20: rework UART GPIO handling" and is already acked-by Simon Glass.

Thanks,
Lucas

Signed-off-by Tom Warren <[email protected]>

---
 arch/arm/cpu/arm720t/tegra30/Makefile  |   48 +++
 arch/arm/cpu/arm720t/tegra30/board.h   |   25 ++
 arch/arm/cpu/arm720t/tegra30/config.mk |   26 ++
 arch/arm/cpu/arm720t/tegra30/cpu.c     |  570 ++++++++++++++++++++++++++++++++
 arch/arm/cpu/arm720t/tegra30/cpu.h     |   65 ++++
 arch/arm/cpu/arm720t/tegra30/spl.c     |  132 ++++++++

Hi Tom,

Quoted 8 lines · Tom Warren

Signed-off-by Tom Warren <[email protected]>

---
 arch/arm/cpu/arm720t/tegra30/Makefile  |   48 +++
 arch/arm/cpu/arm720t/tegra30/board.h   |   25 ++
 arch/arm/cpu/arm720t/tegra30/config.mk |   26 ++
 arch/arm/cpu/arm720t/tegra30/cpu.c     |  570 ++++++++++++++++++++++++++++++++
 arch/arm/cpu/arm720t/tegra30/cpu.h     |   65 ++++
 arch/arm/cpu/arm720t/tegra30/spl.c     |  132 ++++++++

It certainly has complicated your work, with the AVP arm720t refactor going in before these patches.

I feel that quite a bit of the code here should perhaps go to arch/arm/cpu/arm720t/tegra-common or similar, so that you can share it with tegra30.

Regards,
Simon

arch/arm/cpu/armv7/tegra30/cmd_enterrcm.c

Quoted 1 line · Tom Warren

arch/arm/cpu/armv7/tegra30/cmd_enterrcm.c +0 -0

This whole file is definitely common with Tegra20.

Stephen,

Quoted 5 lines · Stephen Warren

arch/arm/cpu/armv7/tegra30/cmd_enterrcm.c

Quoted 1 line · Tom Warren

arch/arm/cpu/armv7/tegra30/cmd_enterrcm.c +0 -0

This whole file is definitely common with Tegra20.

I'm going through your previous comments, but I'll just reply quickly to this one since it needs clearing up.

The intent of this first series of patches for Tegra30 was just to get to the command prompt on T30 in the quickest way, while impacting Tegra20 code as little as possible. Hence, I used Tegra20 files to create a Tegra30 build, and as I ported it to T30 HW, I tried to eliminate what I could that I knew for sure was T20-specific and not useful. But I've made no effort to combine common files/code in this initial pass. I think it's much easier to understand and review these files as a separate SoC build, rather than having to parse common/combined files and code. I intend to do the combination/common-izing of the TegraXX builds once I have a reasonable T30 build in u-boot-tegra, perhaps even before I start porting the drivers. But this is the initial approach I took. Hopefully it'll be an acceptable course - I'd hate to have to back-track.

Quoted 12 lines · Tom Warren

Stephen,

Quoted 5 lines · Stephen Warren

arch/arm/cpu/armv7/tegra30/cmd_enterrcm.c

Quoted 1 line · Tom Warren

arch/arm/cpu/armv7/tegra30/cmd_enterrcm.c +0 -0

This whole file is definitely common with Tegra20.

I'm going through your previous comments, but I'll just reply quickly to this one since it needs clearing up.

Quoted 14 lines

The intent of this first series of patches for Tegra30 was just to get to the command prompt on T30 in the quickest way, while impacting Tegra20 code as little as possible. Hence, I used Tegra20 files to create a Tegra30 build, and as I ported it to T30 HW, I tried to eliminate what I could that I knew for sure was T20-specific and not useful. But I've made no effort to combine common files/code in this initial pass. I think it's much easier to understand and review these files as a separate SoC build, rather than having to parse common/combined files and code. I intend to do the combination/common-izing of the TegraXX builds once I have a reasonable T30 build in u-boot-tegra, perhaps even before I start porting the drivers. But this is the initial approach I took. Hopefully it'll be an acceptable course - I'd hate to have to back-track.

To be honest, it seems like the patch to add the Tegra30 deltas to the existing Tegra20 code would be massively smaller than duplicating all of Tegra20 as Tegra30 and applying those same changes. In the kernel, we have both Tegra20 and Tegra30 support with run-time differentiation, and the number of places where we have to do something different is not that large at all. With the current patch series, there's a huge amount of code to wade through, so spotting any places that haven't been updated for Tegra30, or weren't intended to be updated yet, is somewhat painful.

Quoted 28 lines · Stephen Warren

Quoted 12 lines · Tom Warren

Stephen,

Quoted 5 lines · Stephen Warren

arch/arm/cpu/armv7/tegra30/cmd_enterrcm.c

Quoted 1 line · Tom Warren

arch/arm/cpu/armv7/tegra30/cmd_enterrcm.c +0 -0

This whole file is definitely common with Tegra20.

I'm going through your previous comments, but I'll just reply quickly to this one since it needs clearing up.

Quoted 14 lines

The intent of this first series of patches for Tegra30 was just to get to the command prompt on T30 in the quickest way, while impacting Tegra20 code as little as possible. Hence, I used Tegra20 files to create a Tegra30 build, and as I ported it to T30 HW, I tried to eliminate what I could that I knew for sure was T20-specific and not useful. But I've made no effort to combine common files/code in this initial pass. I think it's much easier to understand and review these files as a separate SoC build, rather than having to parse common/combined files and code. I intend to do the combination/common-izing of the TegraXX builds once I have a reasonable T30 build in u-boot-tegra, perhaps even before I start porting the drivers. But this is the initial approach I took. Hopefully it'll be an acceptable course - I'd hate to have to back-track.

Quoted 8 lines

To be honest, it seems like the patch to add the Tegra30 deltas to the existing Tegra20 code would be massively smaller than duplicating all of Tegra20 as Tegra30 and applying those same changes. In the kernel, we have both Tegra20 and Tegra30 support with run-time differentiation, and the number of places where we have to do something different is not that large at all. With the current patch series, there's a huge amount of code to wade through, so spotting any places that haven't been updated for Tegra30, or weren't intended to be updated yet, is somewhat painful.

Since we know that the delta can be small, yes, let's just do this right the first time (or so). incremental moves, additions and we can work out run-vs-build time a little further down the road.

--
Tom

Tom,

Quoted 38 lines · Tom Rini

Quoted 28 lines · Stephen Warren

Quoted 12 lines · Tom Warren

Stephen,

Quoted 5 lines · Stephen Warren

arch/arm/cpu/armv7/tegra30/cmd_enterrcm.c

Quoted 1 line · Tom Warren

arch/arm/cpu/armv7/tegra30/cmd_enterrcm.c +0 -0

This whole file is definitely common with Tegra20.

I'm going through your previous comments, but I'll just reply quickly to this one since it needs clearing up.

Quoted 14 lines

The intent of this first series of patches for Tegra30 was just to get to the command prompt on T30 in the quickest way, while impacting Tegra20 code as little as possible. Hence, I used Tegra20 files to create a Tegra30 build, and as I ported it to T30 HW, I tried to eliminate what I could that I knew for sure was T20-specific and not useful. But I've made no effort to combine common files/code in this initial pass. I think it's much easier to understand and review these files as a separate SoC build, rather than having to parse common/combined files and code. I intend to do the combination/common-izing of the TegraXX builds once I have a reasonable T30 build in u-boot-tegra, perhaps even before I start porting the drivers. But this is the initial approach I took. Hopefully it'll be an acceptable course - I'd hate to have to back-track.

Quoted 8 lines

To be honest, it seems like the patch to add the Tegra30 deltas to the existing Tegra20 code would be massively smaller than duplicating all of Tegra20 as Tegra30 and applying those same changes. In the kernel, we have both Tegra20 and Tegra30 support with run-time differentiation, and the number of places where we have to do something different is not that large at all. With the current patch series, there's a huge amount of code to wade through, so spotting any places that haven't been updated for Tegra30, or weren't intended to be updated yet, is somewhat painful.

Quoted 3 lines

Since we know that the delta can be small, yes, let's just do this right the first time (or so). incremental moves, additions and we can work out run-vs-build time a little further down the road.

Sorry, Tom. I'm not clear on exactly which way you'd like to see this go.

Are you advising that I re-cast this patchset as a set of common Tegra files/code, with deltas/diffs for the Tegra30 changes? That implies, I think, that I first have to do a patchset that re-orgs Tegra20 code into common code, and then submit a smaller version of this patchset that is just deltas for Tegra30. That means that I'll be touching everyone's Tegra20 code, and will need Ack's from all the T20 vendors before I can move forward w/T30 code.

The other approach, which is still a 2-(or more)-patchset process, is to continue with this patchset for T30, with corrections as per review, and then immediately work on a 'merge-to-common-code' set of patches to common-ize Tegra20/30. That way Tegra20 is unaffected, I can keep moving forward, and I think the end result will be the same as the approach above.

I can see value in both approaches, and it shouldn't surprise you that I'd favor the 2nd approach, since it's less chaotic for me. Let me know what you think,

Quoted 47 lines · Tom Warren

Tom,

Quoted 38 lines · Tom Rini

Quoted 28 lines · Stephen Warren

Quoted 12 lines · Tom Warren

Stephen,

Quoted 5 lines · Stephen Warren

arch/arm/cpu/armv7/tegra30/cmd_enterrcm.c

Quoted 1 line · Tom Warren

arch/arm/cpu/armv7/tegra30/cmd_enterrcm.c +0 -0

This whole file is definitely common with Tegra20.

I'm going through your previous comments, but I'll just reply quickly to this one since it needs clearing up.

Quoted 14 lines

The intent of this first series of patches for Tegra30 was just to get to the command prompt on T30 in the quickest way, while impacting Tegra20 code as little as possible. Hence, I used Tegra20 files to create a Tegra30 build, and as I ported it to T30 HW, I tried to eliminate what I could that I knew for sure was T20-specific and not useful. But I've made no effort to combine common files/code in this initial pass. I think it's much easier to understand and review these files as a separate SoC build, rather than having to parse common/combined files and code. I intend to do the combination/common-izing of the TegraXX builds once I have a reasonable T30 build in u-boot-tegra, perhaps even before I start porting the drivers. But this is the initial approach I took. Hopefully it'll be an acceptable course - I'd hate to have to back-track.

Quoted 8 lines

To be honest, it seems like the patch to add the Tegra30 deltas to the existing Tegra20 code would be massively smaller than duplicating all of Tegra20 as Tegra30 and applying those same changes. In the kernel, we have both Tegra20 and Tegra30 support with run-time differentiation, and the number of places where we have to do something different is not that large at all. With the current patch series, there's a huge amount of code to wade through, so spotting any places that haven't been updated for Tegra30, or weren't intended to be updated yet, is somewhat painful.

Quoted 3 lines

Since we know that the delta can be small, yes, let's just do this right the first time (or so). incremental moves, additions and we can work out run-vs-build time a little further down the road.

Sorry, Tom. I'm not clear on exactly which way you'd like to see this go.

Quoted 7 lines

Are you advising that I re-cast this patchset as a set of common Tegra files/code, with deltas/diffs for the Tegra30 changes? That implies, I think, that I first have to do a patchset that re-orgs Tegra20 code into common code, and then submit a smaller version of this patchset that is just deltas for Tegra30. That means that I'll be touching everyone's Tegra20 code, and will need Ack's from all the T20 vendors before I can move forward w/T30 code.

As far as I'm conerend to do a:
git mv arch/arm/cpu/armv7/tegra20/cmd_enterrcm.c arch/arm/cpu/armv7/tegra-common/cmd_enterrcm.c (just looking at top of tree mainline) needs just the overall Tegra maintainer to Ack. The Custodians page says that's you, and so long as you MAKEALL -s tegra20 before and after, that's good to go. By that same token, breaking out a hypothetical set of common functions from tegra20/usb.c into tegra-common/usb.c and leaving the T20 specific bits in tegra20/usb.c and adding tegra30/usb.c later just needs you and MAKEALL happy.

Quoted 6 lines

The other approach, which is still a 2-(or more)-patchset process, is to continue with this patchset for T30, with corrections as per review, and then immediately work on a 'merge-to-common-code' set of patches to common-ize Tegra20/30. That way Tegra20 is unaffected, I can keep moving forward, and I think the end result will be the same as the approach above.

As has been noted, when you copy files you get a lot of re-review and it's hard to tell what's new and what's not. It IS good to post what you have posted and say "please check areas X/Y/Z, it's new code". But reviewing code that will be dropped as soon as you switch to the 'merge-to-common-code' branch being the reviewed one is hard, especially when folks with less Tegra background try and read the patches for general issues.

Quoted 3 lines

I can see value in both approaches, and it shouldn't surprise you that I'd favor the 2nd approach, since it's less chaotic for me. Let me know what you think,

Well, I'd argue that since you're going to need to do the 'merge-to-common-code' path at some point, it's going to save you work to do that now rather than fixup issues in two places. And again, if you don't change the code, just where the code is, MAKEALL will catch your problems for you.

--
Tom

Tom,

Quoted 85 lines · Tom Rini

Quoted 47 lines · Tom Warren

Tom,

Quoted 38 lines · Tom Rini

Quoted 28 lines · Stephen Warren

Quoted 12 lines · Tom Warren

Stephen,

Quoted 5 lines · Stephen Warren

arch/arm/cpu/armv7/tegra30/cmd_enterrcm.c

Quoted 1 line · Tom Warren

arch/arm/cpu/armv7/tegra30/cmd_enterrcm.c +0 -0

This whole file is definitely common with Tegra20.

I'm going through your previous comments, but I'll just reply quickly to this one since it needs clearing up.

Quoted 14 lines

The intent of this first series of patches for Tegra30 was just to get to the command prompt on T30 in the quickest way, while impacting Tegra20 code as little as possible. Hence, I used Tegra20 files to create a Tegra30 build, and as I ported it to T30 HW, I tried to eliminate what I could that I knew for sure was T20-specific and not useful. But I've made no effort to combine common files/code in this initial pass. I think it's much easier to understand and review these files as a separate SoC build, rather than having to parse common/combined files and code. I intend to do the combination/common-izing of the TegraXX builds once I have a reasonable T30 build in u-boot-tegra, perhaps even before I start porting the drivers. But this is the initial approach I took. Hopefully it'll be an acceptable course - I'd hate to have to back-track.

Quoted 8 lines

To be honest, it seems like the patch to add the Tegra30 deltas to the existing Tegra20 code would be massively smaller than duplicating all of Tegra20 as Tegra30 and applying those same changes. In the kernel, we have both Tegra20 and Tegra30 support with run-time differentiation, and the number of places where we have to do something different is not that large at all. With the current patch series, there's a huge amount of code to wade through, so spotting any places that haven't been updated for Tegra30, or weren't intended to be updated yet, is somewhat painful.

Quoted 3 lines

Since we know that the delta can be small, yes, let's just do this right the first time (or so). incremental moves, additions and we can work out run-vs-build time a little further down the road.

Sorry, Tom. I'm not clear on exactly which way you'd like to see this go.

Quoted 7 lines

Are you advising that I re-cast this patchset as a set of common Tegra files/code, with deltas/diffs for the Tegra30 changes? That implies, I think, that I first have to do a patchset that re-orgs Tegra20 code into common code, and then submit a smaller version of this patchset that is just deltas for Tegra30. That means that I'll be touching everyone's Tegra20 code, and will need Ack's from all the T20 vendors before I can move forward w/T30 code.

As far as I'm conerend to do a:
git mv arch/arm/cpu/armv7/tegra20/cmd_enterrcm.c arch/arm/cpu/armv7/tegra-common/cmd_enterrcm.c (just looking at top of tree mainline) needs just the overall Tegra maintainer to Ack. The Custodians page says that's you, and so long as you MAKEALL -s tegra20 before and after, that's good to go. By that same token, breaking out a hypothetical set of common functions from tegra20/usb.c into tegra-common/usb.c and leaving the T20 specific bits in tegra20/usb.c and adding tegra30/usb.c later just needs you and MAKEALL happy.

Quoted 6 lines

The other approach, which is still a 2-(or more)-patchset process, is to continue with this patchset for T30, with corrections as per review, and then immediately work on a 'merge-to-common-code' set of patches to common-ize Tegra20/30. That way Tegra20 is unaffected, I can keep moving forward, and I think the end result will be the same as the approach above.

As has been noted, when you copy files you get a lot of re-review and it's hard to tell what's new and what's not. It IS good to post what you have posted and say "please check areas X/Y/Z, it's new code". But reviewing code that will be dropped as soon as you switch to the 'merge-to-common-code' branch being the reviewed one is hard, especially when folks with less Tegra background try and read the patches for general issues.

Quoted 3 lines

I can see value in both approaches, and it shouldn't surprise you that I'd favor the 2nd approach, since it's less chaotic for me. Let me know what you think,

Quoted 5 lines

Well, I'd argue that since you're going to need to do the 'merge-to-common-code' path at some point, it's going to save you work to do that now rather than fixup issues in two places. And again, if you don't change the code, just where the code is, MAKEALL will catch your problems for you.

Moving the code via git as you pointed out above is fine, but how do I hook 'tegra-common' into the boards.cfg/Makefile/MAKEALL process? I'd need a 'tegra-common' subdir for arm720t, armv7 and an 'arch-tegra' subdir for arch/arm/include/asm/. The arch-tegra common subdir is easy, as I can just have the <asm/arch/blah.h> file do an 'include <asm/arch-tegra/blah.h> when the include file is 100% common and flesh it out when it's SoC-specific. But I don't see how I can build the SPL and 'normal' U-Boot sections via boards.cfg w/common code, without having a SPL.C that says 'include ../tegra-common/spl.c', which seems messy. Not to mention that we have arch/arm/cpu/tegra20-common and ../tegra30-common, too. How do you do it on OMAP?

arch/arm/cpu/tegra30-common/board.c +12 -0

 
+unsigned int query_sdram_size(void)
+{
+	struct pmc_ctlr *const pmc = (struct pmc_ctlr *)NV_PA_PMC_BASE;
+	u32 reg;
+
+	reg = readl(&pmc->pmc_scratch20);
+	debug("pmc->pmc_scratch20 (ODMData) = 0x%08x\n", reg);
+
+	/* bits 31:28 in OdmData are used for RAM size  */
+	switch ((reg) >> 28) {
+	case 1:
+		return 0x10000000;	/* 256 MB */

arch/arm/cpu/tegra30-common/clock.c +10 -0

 
+/*
+ * This array translates a periph_id to a periphc_internal_id
+ *
+ * Not present/matched up:
+ *	uint vi_sensor;	 _VI_SENSOR_0,		0x1A8
+ *	SPDIF - which is both 0x08 and 0x0c
+ *
+ */
+#define NONE(name) (-1)
+#define OFFSET(name, value) PERIPHC_ ## name

arch/arm/cpu/tegra30-common/clock.c +30 -0

+#ifdef CONFIG_OF_CONTROL
+/*
+ * Convert a device tree clock ID to our peripheral ID. They are mostly
+ * the same but we are very cautious so we check that a valid clock ID is
+ * provided.
+ *
+ * @param clk_id	Clock ID according to tegra20 device tree binding
+ * @return peripheral ID, or PERIPH_ID_NONE if the clock ID is invalid
+ */
+static enum periph_id clk_id_to_periph_id(int clk_id)
+{
+	if (clk_id > 95)
+		return PERIPH_ID_NONE;
+
+	switch (clk_id) {
+	case 1:
+	case 2:
+	case 7:
+	case 10:
+	case 20:
+	case 30:
+	case 35:
+	case 49:
+	case 56:
+	case 74:
+	case 76:
+	case 77:
+	case 78:
+	case 79:
+	case 80:

arch/arm/cpu/tegra30-common/funcmux.c +7 -0

 
+int funcmux_select(enum periph_id id, int config)
+{
+	int bad_config = config != FUNCMUX_DEFAULT;
+
+	switch (id) {
+	case PERIPH_ID_UART1:
+		switch (config) {

Signed-off-by Tom Warren <[email protected]>

---
 arch/arm/cpu/tegra30-common/Makefile        |   51 ++
 arch/arm/cpu/tegra30-common/ap30.c          |   98 +++
 arch/arm/cpu/tegra30-common/board.c         |  141 ++++
 arch/arm/cpu/tegra30-common/clock.c         | 1099 +++++++++++++++++++++++++++

board/nvidia/cardhu/cardhu.c +9 -0

+#ifdef CONFIG_TEGRA_MMC
+/*
+ * Routine: pin_mux_mmc
+ * Description: setup the pin muxes/tristate values for the SDMMC(s)
+ */
+static void pin_mux_mmc(void)
+{
+	funcmux_select(PERIPH_ID_SDMMC4, FUNCMUX_SDMMC4_ATB_GMA_GME_8_BIT);
+	funcmux_select(PERIPH_ID_SDMMC3, FUNCMUX_SDMMC3_SDB_4BIT);

Signed-off-by Tom Warren <[email protected]>

---
 board/nvidia/cardhu/Makefile               |   48 ++++
 board/nvidia/cardhu/cardhu.c               |   87 +++++++
 board/nvidia/cardhu/pinmux-config-common.h |  346 ++++++++++++++++++++++++++++
 3 files changed, 481 insertions(+), 0 deletions(-)
 create mode 100644 board/nvidia/cardhu/Makefile
 create mode 100644 board/nvidia/cardhu/cardhu.c
 create mode 100644 board/nvidia/cardhu/pinmux-config-common.h

board/nvidia/cardhu/Makefile new file +48 -0

new file mode 100644
index 0000000..a910577
@@ -0,0 +1,48 @@
+#
+#  (C) Copyright 2010-2012
+#  NVIDIA Corporation <www.nvidia.com>
+#
+#
+#  See file CREDITS for list of people who contributed to this
+#  project.
+#
+#  This program is free software; you can redistribute it and/or
+#  modify it under the terms of the GNU General Public License as
+#  published by the Free Software Foundation; either version 2 of
+#  the License, or (at your option) any later version.
+#
+#  This program is distributed in the hope that it will be useful,
+#  but WITHOUT ANY WARRANTY; without even the implied warranty of
+#  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+#  GNU General Public License for more details.
+#
+#  You should have received a copy of the GNU General Public License
+#  along with this program; if not, write to the Free Software
+#  Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+#  MA 02111-1307 USA
+#
+
+include $(TOPDIR)/config.mk
+
+ifneq ($(OBJTREE),$(SRCTREE))
+$(shell mkdir -p $(obj)../common)
+endif
+
+LIB    = $(obj)lib$(BOARD).o
+
+COBJS  := $(BOARD).o
+
+SRCS   := $(COBJS:.o=.c)
+OBJS   := $(addprefix $(obj),$(COBJS))
+
+$(LIB):        $(obj).depend $(OBJS)
+       $(call cmd_link_o_target, $(OBJS))
+
+#########################################################################
+
+# defines $(obj).depend target
+include $(SRCTREE)/rules.mk
+
+sinclude $(obj).depend
+
+#########################################################################

board/nvidia/cardhu/cardhu.c new file +87 -0

new file mode 100644
index 0000000..f907906
@@ -0,0 +1,87 @@
+/*
+ *  (C) Copyright 2010-2012
+ *  NVIDIA Corporation <www.nvidia.com>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <common.h>
+#include <asm/io.h>
+#include <asm/arch/tegra30.h>
+#include <asm/arch/clock.h>
+#include <asm/arch/funcmux.h>
+#include <asm/arch/pinmux.h>
+#include <asm/arch/mmc.h>
+#include <asm/gpio.h>
+#ifdef CONFIG_TEGRA_MMC
+#include <mmc.h>
+#endif
+
+/*
+ * Routine: gpio_config_uart
+ * Description: Does nothing on Cardhu - no conflict w/SPI.
+ */
+void gpio_config_uart(void)
+{
+}
+
+#ifdef CONFIG_TEGRA_MMC
+/*
+ * Routine: pin_mux_mmc
+ * Description: setup the pin muxes/tristate values for the SDMMC(s)
+ */
+static void pin_mux_mmc(void)
+{
+       funcmux_select(PERIPH_ID_SDMMC4, FUNCMUX_SDMMC4_ATB_GMA_GME_8_BIT);
+       funcmux_select(PERIPH_ID_SDMMC3, FUNCMUX_SDMMC3_SDB_4BIT);
+
+       /* For power GPIO PI6 */
+       pinmux_tristate_disable(PINGRP_ATA);
+       /* For CD GPIO PI5 */
+       pinmux_tristate_disable(PINGRP_ATC);
+}
+
+/* this is a weak define that we are overriding */
+int board_mmc_init(bd_t *bd)
+{
+       debug("board_mmc_init called\n");
+
+       /* Enable muxes, etc. for SDMMC controllers */
+       pin_mux_mmc();
+
+       debug("board_mmc_init: init eMMC\n");
+       /* init dev 0, eMMC chip, with 4-bit bus */
+       /* The board has an 8-bit bus, but 8-bit doesn't work yet */
+       tegra_mmc_init(0, 4, -1, -1);
+
+       debug("board_mmc_init: init SD slot\n");
+       /* init dev 1, SD slot, with 4-bit bus */
+       tegra_mmc_init(1, 4, GPIO_PI6, GPIO_PI5);
+
+       return 0;
+}
+#endif
+
+#ifdef CONFIG_USB_EHCI_TEGRA
+void pin_mux_usb(void)
+{
+       /* For USB's GPIO PD0. For now, since we have no pinmux in fdt */
+       pinmux_tristate_disable(PINGRP_SLXK);
+}
+#endif

board/nvidia/cardhu/pinmux-config-common.h new file +55 -0

new file mode 100644
index 0000000..20583c9
@@ -0,0 +1,346 @@
+/*
+ *     (C) Copyright 2010-2012
+ * NVIDIA Corporation <www.nvidia.com>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#ifndef PINMUX_CONFIG_COMMON_H
+#define PINMUX_CONFIG_COMMON_H
+
+#define DEFAULT_PINMUX(_pingroup, _mux, _pull, _tri, _io)      \
+       {                                                       \
+               .pingroup       = PINGRP_##_pingroup,           \
+               .func           = PMUX_FUNC_##_mux,             \
+               .pull           = PMUX_PULL_##_pull,            \
+               .tristate       = PMUX_TRI_##_tri,              \
+               .io             = PMUX_PIN_##_io,               \
+               .lock           = PMUX_PIN_LOCK_DEFAULT,        \
+               .od             = PMUX_PIN_OD_DEFAULT,          \
+               .ioreset        = PMUX_PIN_IO_RESET_DEFAULT,    \
+       }
+
+#define I2C_PINMUX(_pingroup, _mux, _pull, _tri, _io, _lock, _od) \
+       {                                                       \
+               .pingroup       = PINGRP_##_pingroup,           \
+               .func           = PMUX_FUNC_##_mux,             \
+               .pull           = PMUX_PULL_##_pull,            \
+               .tristate       = PMUX_TRI_##_tri,              \
+               .io             = PMUX_PIN_##_io,               \
+               .lock           = PMUX_PIN_LOCK_##_lock,        \
+               .od             = PMUX_PIN_OD_##_od,            \
+               .ioreset        = PMUX_PIN_IO_RESET_DEFAULT,    \
+       }
+
+#define LV_PINMUX(_pingroup, _mux, _pull, _tri, _io, _lock, _ioreset) \
+       {                                                       \
+               .pingroup       = PINGRP_##_pingroup,           \
+               .func           = PMUX_FUNC_##_mux,             \
+               .pull           = PMUX_PULL_##_pull,            \

+#define CONFIG_SYS_LOAD_ADDR (0x80A00800) /* default */

Quoted 1 line · Tom Warren

include/configs/tegra30-common.h +0 -0

include/configs/tegra30-common.h line 34

Quoted 7 lines

include/configs/tegra30-common.h +7 -0

+/*
+ * QUOTE(m) will evaluate to a string version of the value of the macro m
+ * passed in.  The extra level of indirection here is to first evaluate the
+ * macro m before applying the quoting operator.
+ */
+#define QUOTE_(m)       #m
+#define QUOTE(m)        QUOTE_(m)

Seems like a good candidate for a common header (not Tegra-common, but across all of U-Boot).

include/configs/tegra30-common.h line 90

Quoted 2 lines

include/configs/tegra30-common.h +2 -0

+#define CONFIG_SYS_BAUDRATE_TABLE	{4800, 9600, 19200, 38400, 57600,\
+					115200}

IIRC, that's the default now and can be removed. There may be other variables the comment applies to - Tom Rini would know better since he pointed them out in my RPi patches.

Quoted 14 lines · Tom Warren

include/configs/tegra30-common.h line 34

Quoted 10 lines

include/configs/tegra30-common.h +2 -0

 
+/*
+ * QUOTE(m) will evaluate to a string version of the value of the macro

m + * passed in. The extra level of indirection here is to first evaluate the + * macro m before applying the quoting operator.
+ */

include/configs/tegra30-common.h +2 -0

+#define QUOTE_(m)       #m
+#define QUOTE(m)        QUOTE_(m)

Seems like a good candidate for a common header (not Tegra-common, but across all of U-Boot).

This is __stringify() that Marek needs to address Wolfgang's comment on still, iirc.

Quoted 6 lines

include/configs/tegra30-common.h line 90

Quoted 2 lines

include/configs/tegra30-common.h +2 -0

+#define CONFIG_SYS_BAUDRATE_TABLE	{4800, 9600, 19200, 38400, 57600,\
+					115200}

IIRC, that's the default now and can be removed. There may be other variables the comment applies to - Tom Rini would know better since he pointed them out in my RPi patches.

That and the hush prompt one if it's there need to go.

Signed-off-by Tom Warren <[email protected]>

---
 include/configs/cardhu.h         |   52 +++++++++++
 include/configs/tegra30-common.h |  188 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 240 insertions(+), 0 deletions(-)
 create mode 100644 include/configs/cardhu.h
 create mode 100644 include/configs/tegra30-common.h

include/configs/cardhu.h new file +52 -0

new file mode 100644
index 0000000..bfadbff
@@ -0,0 +1,52 @@
+/*
+ *  (C) Copyright 2010-2012
+ *  NVIDIA Corporation <www.nvidia.com>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#ifndef __CONFIG_H
+#define __CONFIG_H
+
+#include <asm/sizes.h>
+
+#include "tegra30-common.h"
+
+/* Enable fdt support for Cardhu. Flash the image in u-boot-dtb.bin */
+#define CONFIG_DEFAULT_DEVICE_TREE     tegra30-cardhu
+#define CONFIG_OF_CONTROL
+#define CONFIG_OF_SEPARATE
+
+/* High-level configuration options */
+#define V_PROMPT               "Tegra30 (Cardhu) # "
+#define CONFIG_TEGRA_BOARD_STRING      "NVIDIA Cardhu"
+
+/* Board-specific serial config */
+#define CONFIG_SERIAL_MULTI
+#define CONFIG_TEGRA_ENABLE_UARTA
+#define CONFIG_SYS_NS16550_COM1                NV_PA_APB_UARTA_BASE
+
+#define CONFIG_MACH_TYPE               MACH_TYPE_CARDHU
+
+#define CONFIG_BOARD_EARLY_INIT_F
+#define CONFIG_ENV_IS_NOWHERE
+
+#include "tegra-common-post.h"
+
+#endif /* __CONFIG_H */

include/configs/tegra30-common.h new file +45 -0

new file mode 100644
index 0000000..b23823d
@@ -0,0 +1,188 @@
+/*
+ *  (C) Copyright 2010-2012
+ *  NVIDIA Corporation <www.nvidia.com>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#ifndef __TEGRA30_COMMON_H
+#define __TEGRA30_COMMON_H
+#include <asm/sizes.h>
+
+/*
+ * QUOTE(m) will evaluate to a string version of the value of the macro m
+ * passed in.  The extra level of indirection here is to first evaluate the
+ * macro m before applying the quoting operator.
+ */
+#define QUOTE_(m)       #m
+#define QUOTE(m)        QUOTE_(m)
+
+/*
+ * High Level Configuration Options
+ */
+#define CONFIG_ARMCORTEXA9             /* This is an ARM V7 CPU core */
+#define CONFIG_TEGRA30                 /* in a NVidia Tegra30 core */
+#define CONFIG_TEGRA                   /* which is a Tegra generic machine */
+#define CONFIG_SYS_L2CACHE_OFF         /* No L2 cache */
+
+#define CONFIG_SYS_CACHELINE_SIZE      32
+

Hi Tom,

Quoted 123 lines · Tom Warren

Signed-off-by Tom Warren <[email protected]>

---
 include/configs/cardhu.h         |   52 +++++++++++
 include/configs/tegra30-common.h |  188 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 240 insertions(+), 0 deletions(-)
 create mode 100644 include/configs/cardhu.h
 create mode 100644 include/configs/tegra30-common.h

include/configs/cardhu.h new file +52 -0

new file mode 100644
index 0000000..bfadbff
@@ -0,0 +1,52 @@
+/*
+ *  (C) Copyright 2010-2012
+ *  NVIDIA Corporation <www.nvidia.com>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#ifndef __CONFIG_H
+#define __CONFIG_H
+
+#include <asm/sizes.h>
+
+#include "tegra30-common.h"
+
+/* Enable fdt support for Cardhu. Flash the image in u-boot-dtb.bin */
+#define CONFIG_DEFAULT_DEVICE_TREE     tegra30-cardhu
+#define CONFIG_OF_CONTROL
+#define CONFIG_OF_SEPARATE
+
+/* High-level configuration options */
+#define V_PROMPT               "Tegra30 (Cardhu) # "
+#define CONFIG_TEGRA_BOARD_STRING      "NVIDIA Cardhu"
+
+/* Board-specific serial config */
+#define CONFIG_SERIAL_MULTI
+#define CONFIG_TEGRA_ENABLE_UARTA
+#define CONFIG_SYS_NS16550_COM1                NV_PA_APB_UARTA_BASE
+
+#define CONFIG_MACH_TYPE               MACH_TYPE_CARDHU
+
+#define CONFIG_BOARD_EARLY_INIT_F
+#define CONFIG_ENV_IS_NOWHERE
+
+#include "tegra-common-post.h"
+
+#endif /* __CONFIG_H */

include/configs/tegra30-common.h new file +51 -0

new file mode 100644
index 0000000..b23823d
@@ -0,0 +1,188 @@
+/*
+ *  (C) Copyright 2010-2012
+ *  NVIDIA Corporation <www.nvidia.com>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#ifndef __TEGRA30_COMMON_H
+#define __TEGRA30_COMMON_H
+#include <asm/sizes.h>
+
+/*
+ * QUOTE(m) will evaluate to a string version of the value of the macro m
+ * passed in.  The extra level of indirection here is to first evaluate the
+ * macro m before applying the quoting operator.
+ */
+#define QUOTE_(m)       #m
+#define QUOTE(m)        QUOTE_(m)
+
+/*
+ * High Level Configuration Options
+ */
+#define CONFIG_ARMCORTEXA9             /* This is an ARM V7 CPU core */
+#define CONFIG_TEGRA30                 /* in a NVidia Tegra30 core */
+#define CONFIG_TEGRA                   /* which is a Tegra generic machine */
+#define CONFIG_SYS_L2CACHE_OFF         /* No L2 cache */
+
+#define CONFIG_SYS_CACHELINE_SIZE      32
+
+#include <asm/arch/tegra30.h>          /* get chip and board defs */
+
+/*
+ * Display CPU and Board information
+ */
+#define CONFIG_DISPLAY_CPUINFO

include/configs/tegra30-common.h line 55

Quoted 4 lines

include/configs/tegra30-common.h new file +4 -0

+#define CONFIG_DISPLAY_BOARDINFO
+
+#define CONFIG_CMDLINE_TAG             /* enable passing of ATAGs */
+#define CONFIG_OF_LIBFDT               /* enable passing of devicetree */

Do you think there should be a tegra-common.h, included by tegra20-common and tegra30-common? That was the approach we talk in the Chromium tree.

Regards,
Simon

Signed-off-by Tom Warren <[email protected]>

---
 board/nvidia/common/board.c |   27 ++++++++++++++++++++++++++-
 1 files changed, 26 insertions(+), 1 deletions(-)

board/nvidia/common/board.c +4 -0

+/*
+ * Routine: pinmux_init
+ * Description: Do individual peripheral pinmux configs
+ */

Quoted 4 lines · Tom Warren

Signed-off-by Tom Warren <[email protected]>

---
 board/nvidia/common/board.c |   27 ++++++++++++++++++++++++++-
 1 files changed, 26 insertions(+), 1 deletions(-)

Common code:-) :-) But ...

board/nvidia/common/board.c static void power_det_init(void) · line 96

Quoted 5 lines

board/nvidia/common/board.c +3 -0

 
+#ifdef CONFIG_TEGRA30
+#include "../cardhu/pinmux-config-common.h"
+#endif

Not all Tegra30 will be Cardhu...

Given this is really board-specific, shouldn't the following be an empty weak definition:

Quoted 4 lines

board/nvidia/common/board.c +4 -0

+/*
+ * Routine: pinmux_init
+ * Description: Do individual peripheral pinmux configs
+ */

board/nvidia/common/board.c static void power_det_init(void) · line 111

Quoted 10 lines

board/nvidia/common/board.c +10 -0

+static void pinmux_init(void)
+{
+#if defined(CONFIG_TEGRA30)
+     pinmux_config_table(tegra3_pinmux_common,
+             ARRAY_SIZE(tegra3_pinmux_common));
+
+     pinmux_config_table(unused_pins_lowpower,
+             ARRAY_SIZE(unused_pins_lowpower));
+#endif
+}

... and the function be overridden in board files as needed?

If we are moving to a model of a single function that sets up the entire pin mux at boot (which seems fine to me, and could eventually be driven by DT if it happened late enough), then it seems like we wouldn't need e.g. pin_mux_mmc() or pin_mux_usb() any more.

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Hi,

Quoted 36 lines · Stephen Warren

Quoted 4 lines · Tom Warren

Signed-off-by Tom Warren <[email protected]>

---
 board/nvidia/common/board.c |   27 ++++++++++++++++++++++++++-
 1 files changed, 26 insertions(+), 1 deletions(-)

Common code:-) :-) But ...

board/nvidia/common/board.c static void power_det_init(void) · line 96

Quoted 5 lines

board/nvidia/common/board.c +3 -0

 
+#ifdef CONFIG_TEGRA30
+#include "../cardhu/pinmux-config-common.h"
+#endif

Not all Tegra30 will be Cardhu...

Given this is really board-specific, shouldn't the following be an empty weak definition:

Quoted 4 lines

board/nvidia/common/board.c +4 -0

+/*
+ * Routine: pinmux_init
+ * Description: Do individual peripheral pinmux configs
+ */

board/nvidia/common/board.c static void power_det_init(void) · line 111

Quoted 10 lines

board/nvidia/common/board.c +10 -0

+static void pinmux_init(void)
+{
+#if defined(CONFIG_TEGRA30)
+     pinmux_config_table(tegra3_pinmux_common,
+             ARRAY_SIZE(tegra3_pinmux_common));
+
+     pinmux_config_table(unused_pins_lowpower,
+             ARRAY_SIZE(unused_pins_lowpower));
+#endif
+}

... and the function be overridden in board files as needed?

Quoted 4 lines

If we are moving to a model of a single function that sets up the entire pin mux at boot (which seems fine to me, and could eventually be driven by DT if it happened late enough), then it seems like we wouldn't need e.g. pin_mux_mmc() or pin_mux_usb() any more.

While the fdt may eventually remove this discussion, I don't think forcing a one-time pinmux init is the best idea. Some peripherals will not be needed on every boot (e.g. normally boot from eMMC unless USB is available). Some peripherals may want to change their config based on run-time settings (although this is unlikely I suppose, particularly if we have the fdt).

Regards,
Simon

Quoted 4 lines

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

As far as I'm aware, an FDT pinmux for Tegra (which is on my plate, but quite a bit behind T30) would be essentially the same deal - one large list of mux settings per build/board.