[U-Boot] [PATCH v4 1/4] tegra2: Move MMC clock initialization into MMC driver

This centralizes knowledge of MMC clocking into the MMC driver. This also removes clock setup from the board files, which will simplify later changes that modify the Harmony board to support the correct set of MMC controllers.
Signed-off-by: Stephen Warren swarren@nvidia.com Acked-by: Simon Glass sjg@chromium.org Tested-by: Simon Glass sjg@chromium.org --- board/nvidia/common/board.c | 13 +------------ drivers/mmc/tegra2_mmc.c | 12 +++++++++--- 2 files changed, 10 insertions(+), 15 deletions(-)
diff --git a/board/nvidia/common/board.c b/board/nvidia/common/board.c index d13537d..370a259 100644 --- a/board/nvidia/common/board.c +++ b/board/nvidia/common/board.c @@ -102,16 +102,6 @@ static void pin_mux_uart(void)
#ifdef CONFIG_TEGRA2_MMC /* - * Routine: clock_init_mmc - * Description: init the PLL and clocks for the SDMMC controllers - */ -static void clock_init_mmc(void) -{ - clock_start_periph_pll(PERIPH_ID_SDMMC4, CLOCK_ID_PERIPH, 20000000); - clock_start_periph_pll(PERIPH_ID_SDMMC3, CLOCK_ID_PERIPH, 20000000); -} - -/* * Routine: pin_mux_mmc * Description: setup the pin muxes/tristate values for the SDMMC(s) */ @@ -157,8 +147,7 @@ int board_init(void) int board_mmc_init(bd_t *bd) { debug("board_mmc_init called\n"); - /* Enable clocks, muxes, etc. for SDMMC controllers */ - clock_init_mmc(); + /* Enable muxes, etc. for SDMMC controllers */ pin_mux_mmc(); gpio_config_mmc();
diff --git a/drivers/mmc/tegra2_mmc.c b/drivers/mmc/tegra2_mmc.c index 9e741f2..78b1190 100644 --- a/drivers/mmc/tegra2_mmc.c +++ b/drivers/mmc/tegra2_mmc.c @@ -435,14 +435,22 @@ static int mmc_core_init(struct mmc *mmc)
static int tegra2_mmc_initialize(int dev_index, int bus_width) { + struct mmc_host *host; struct mmc *mmc;
debug(" mmc_initialize called\n");
+ host = &mmc_host[dev_index]; + + host->clock = 0; + tegra2_get_setup(host, dev_index); + + clock_start_periph_pll(host->mmc_id, CLOCK_ID_PERIPH, 20000000); + mmc = &mmc_dev[dev_index];
sprintf(mmc->name, "Tegra2 SD/MMC"); - mmc->priv = &mmc_host[dev_index]; + mmc->priv = host; mmc->send_cmd = mmc_send_cmd; mmc->set_ios = mmc_set_ios; mmc->init = mmc_core_init; @@ -465,8 +473,6 @@ static int tegra2_mmc_initialize(int dev_index, int bus_width) mmc->f_min = 375000; mmc->f_max = 48000000;
- mmc_host[dev_index].clock = 0; - tegra2_get_setup(&mmc_host[dev_index], dev_index); mmc_register(mmc);
return 0;

For Seaboard, this is mostly a cut/paste of board_mmc_init() and pin_mux_mmc() into seaboard.c; pin_mux_mmc() was modified to add some missing pinmux_tristate_disable calls for the GPIOs.
For Harmony, those functions were modified to configure SDMMC2 (index 2) instead of SDMMC3 (index 1), since that's what is present on the board.
However, harmony.c is still missing the required GPIO setup, so neither port is likely to function correctly yet. This will be fixed in the next change.
v4: Include board.h to prototype tegra2_mmc_init().
Signed-off-by: Stephen Warren swarren@nvidia.com Tested-by: Simon Glass sjg@chromium.org --- board/nvidia/common/board.c | 52 ----------------------------------- board/nvidia/harmony/harmony.c | 56 ++++++++++++++++++++++++++++++++++++++ board/nvidia/seaboard/seaboard.c | 53 +++++++++++++++++++++++++++++++++++ 3 files changed, 109 insertions(+), 52 deletions(-)
diff --git a/board/nvidia/common/board.c b/board/nvidia/common/board.c index 370a259..0f12de2 100644 --- a/board/nvidia/common/board.c +++ b/board/nvidia/common/board.c @@ -33,10 +33,6 @@ #include <asm/arch/uart.h> #include "board.h"
-#ifdef CONFIG_TEGRA2_MMC -#include <mmc.h> -#endif - DECLARE_GLOBAL_DATA_PTR;
const struct tegra2_sysinfo sysinfo = { @@ -100,33 +96,6 @@ static void pin_mux_uart(void) #endif /* CONFIG_TEGRA2_ENABLE_UARTD */ }
-#ifdef CONFIG_TEGRA2_MMC -/* - * Routine: pin_mux_mmc - * Description: setup the pin muxes/tristate values for the SDMMC(s) - */ -static void pin_mux_mmc(void) -{ - /* SDMMC4: config 3, x8 on 2nd set of pins */ - pinmux_set_func(PINGRP_ATB, PMUX_FUNC_SDIO4); - pinmux_set_func(PINGRP_GMA, PMUX_FUNC_SDIO4); - pinmux_set_func(PINGRP_GME, PMUX_FUNC_SDIO4); - - pinmux_tristate_disable(PINGRP_ATB); - pinmux_tristate_disable(PINGRP_GMA); - pinmux_tristate_disable(PINGRP_GME); - - /* SDMMC3: SDIO3_CLK, SDIO3_CMD, SDIO3_DAT[3:0] */ - pinmux_set_func(PINGRP_SDB, PMUX_FUNC_SDIO3); - pinmux_set_func(PINGRP_SDC, PMUX_FUNC_SDIO3); - pinmux_set_func(PINGRP_SDD, PMUX_FUNC_SDIO3); - - pinmux_tristate_disable(PINGRP_SDC); - pinmux_tristate_disable(PINGRP_SDD); - pinmux_tristate_disable(PINGRP_SDB); -} -#endif - /* * Routine: board_init * Description: Early hardware init. @@ -142,27 +111,6 @@ int board_init(void) return 0; }
-#ifdef CONFIG_TEGRA2_MMC -/* 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(); - gpio_config_mmc(); - - debug("board_mmc_init: init eMMC\n"); - /* init dev 0, eMMC chip, with 4-bit bus */ - tegra2_mmc_init(0, 4); - - debug("board_mmc_init: init SD slot\n"); - /* init dev 1, SD slot, with 4-bit bus */ - tegra2_mmc_init(1, 4); - - return 0; -} -#endif - #ifdef CONFIG_BOARD_EARLY_INIT_F int board_early_init_f(void) { diff --git a/board/nvidia/harmony/harmony.c b/board/nvidia/harmony/harmony.c index cbb30d6..f2c3867 100644 --- a/board/nvidia/harmony/harmony.c +++ b/board/nvidia/harmony/harmony.c @@ -24,9 +24,11 @@ #include <common.h> #include <asm/io.h> #include <asm/arch/tegra2.h> +#include <asm/arch/pinmux.h> #ifdef CONFIG_TEGRA2_MMC #include <mmc.h> #endif +#include "../common/board.h"
/* * Routine: gpio_config_uart @@ -38,6 +40,39 @@ void gpio_config_uart(void)
#ifdef CONFIG_TEGRA2_MMC /* + * Routine: pin_mux_mmc + * Description: setup the pin muxes/tristate values for the SDMMC(s) + */ +static void pin_mux_mmc(void) +{ + /* SDMMC4: config 3, x8 on 2nd set of pins */ + pinmux_set_func(PINGRP_ATB, PMUX_FUNC_SDIO4); + pinmux_set_func(PINGRP_GMA, PMUX_FUNC_SDIO4); + pinmux_set_func(PINGRP_GME, PMUX_FUNC_SDIO4); + + pinmux_tristate_disable(PINGRP_ATB); + pinmux_tristate_disable(PINGRP_GMA); + pinmux_tristate_disable(PINGRP_GME); + + /* For power GPIO PI6 */ + pinmux_tristate_disable(PINGRP_ATA); + /* For CD GPIO PH2 */ + pinmux_tristate_disable(PINGRP_ATD); + + /* SDMMC2: SDIO2_CLK, SDIO2_CMD, SDIO2_DAT[7:0] */ + pinmux_set_func(PINGRP_DTA, PMUX_FUNC_SDIO2); + pinmux_set_func(PINGRP_DTD, PMUX_FUNC_SDIO2); + + pinmux_tristate_disable(PINGRP_DTA); + pinmux_tristate_disable(PINGRP_DTD); + + /* For power GPIO PT3 */ + pinmux_tristate_disable(PINGRP_DTB); + /* For CD GPIO PI5 */ + pinmux_tristate_disable(PINGRP_ATC); +} + +/* * Routine: gpio_config_mmc * Description: Set GPIOs for SD card */ @@ -47,6 +82,27 @@ void gpio_config_mmc(void) }
/* 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(); + gpio_config_mmc(); + + debug("board_mmc_init: init SD slot J26\n"); + /* init dev 0, SD slot J26, with 4-bit bus */ + /* The board has an 8-bit bus, but 8-bit doesn't work yet */ + tegra2_mmc_init(0, 4); + + debug("board_mmc_init: init SD slot J5\n"); + /* init dev 2, SD slot J5, with 4-bit bus */ + tegra2_mmc_init(2, 4); + + return 0; +} + +/* this is a weak define that we are overriding */ int board_mmc_getcd(u8 *cd, struct mmc *mmc) { debug("board_mmc_getcd called\n"); diff --git a/board/nvidia/seaboard/seaboard.c b/board/nvidia/seaboard/seaboard.c index bc67d0f..22a0e69 100644 --- a/board/nvidia/seaboard/seaboard.c +++ b/board/nvidia/seaboard/seaboard.c @@ -24,10 +24,12 @@ #include <common.h> #include <asm/io.h> #include <asm/arch/tegra2.h> +#include <asm/arch/pinmux.h> #include <asm/gpio.h> #ifdef CONFIG_TEGRA2_MMC #include <mmc.h> #endif +#include "../common/board.h"
/* * Routine: gpio_config_uart @@ -56,6 +58,36 @@ void gpio_config_uart(void)
#ifdef CONFIG_TEGRA2_MMC /* + * Routine: pin_mux_mmc + * Description: setup the pin muxes/tristate values for the SDMMC(s) + */ +static void pin_mux_mmc(void) +{ + /* SDMMC4: config 3, x8 on 2nd set of pins */ + pinmux_set_func(PINGRP_ATB, PMUX_FUNC_SDIO4); + pinmux_set_func(PINGRP_GMA, PMUX_FUNC_SDIO4); + pinmux_set_func(PINGRP_GME, PMUX_FUNC_SDIO4); + + pinmux_tristate_disable(PINGRP_ATB); + pinmux_tristate_disable(PINGRP_GMA); + pinmux_tristate_disable(PINGRP_GME); + + /* SDMMC3: SDIO3_CLK, SDIO3_CMD, SDIO3_DAT[3:0] */ + pinmux_set_func(PINGRP_SDB, PMUX_FUNC_SDIO3); + pinmux_set_func(PINGRP_SDC, PMUX_FUNC_SDIO3); + pinmux_set_func(PINGRP_SDD, PMUX_FUNC_SDIO3); + + pinmux_tristate_disable(PINGRP_SDC); + pinmux_tristate_disable(PINGRP_SDD); + pinmux_tristate_disable(PINGRP_SDB); + + /* For power GPIO PI6 */ + pinmux_tristate_disable(PINGRP_ATA); + /* For CD GPIO PI5 */ + pinmux_tristate_disable(PINGRP_ATC); +} + +/* * Routine: gpio_config_mmc * Description: Set GPIOs for SDMMC3 SDIO slot. */ @@ -69,6 +101,27 @@ void gpio_config_mmc(void) }
/* 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(); + gpio_config_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 */ + tegra2_mmc_init(0, 4); + + debug("board_mmc_init: init SD slot\n"); + /* init dev 1, SD slot, with 4-bit bus */ + tegra2_mmc_init(1, 4); + + return 0; +} + +/* this is a weak define that we are overriding */ int board_mmc_getcd(u8 *cd, struct mmc *mmc) { debug("board_mmc_getcd called\n");

Pass the GPIO numbers for power and card detect to tegra2_mmc_init(), and modify that function to perform all required GPIO initialization. This removes the need for board files to perform these operations.
Move board_mmc_getcd() into tegra2_mmc.c now that the driver knows which GPIOs to use.
Update affected call-sites in seaboard.c and harmony.c. Note that this change should make all SD ports work on Harmony, since the required GPIO setup is now being performed.
v4: Fix prototype of tegra2_mmc_init() in board.h to match driver change. Remove prototype of gpio_config_mmc() from board.h
Signed-off-by: Stephen Warren swarren@nvidia.com Tested-by: Simon Glass sjg@chromium.org --- board/nvidia/common/board.h | 3 +- board/nvidia/harmony/harmony.c | 27 ++--------------------- board/nvidia/seaboard/seaboard.c | 33 +---------------------------- drivers/mmc/tegra2_mmc.c | 42 ++++++++++++++++++++++++++++++++----- drivers/mmc/tegra2_mmc.h | 4 ++- 5 files changed, 45 insertions(+), 64 deletions(-)
diff --git a/board/nvidia/common/board.h b/board/nvidia/common/board.h index 344e702..35acbca 100644 --- a/board/nvidia/common/board.h +++ b/board/nvidia/common/board.h @@ -26,7 +26,6 @@
void tegra2_start(void); void gpio_config_uart(void); -void gpio_config_mmc(void); -int tegra2_mmc_init(int dev_index, int bus_width); +int tegra2_mmc_init(int dev_index, int bus_width, int pwr_gpio, int cd_gpio);
#endif /* BOARD_H */ diff --git a/board/nvidia/harmony/harmony.c b/board/nvidia/harmony/harmony.c index f2c3867..3cbe820 100644 --- a/board/nvidia/harmony/harmony.c +++ b/board/nvidia/harmony/harmony.c @@ -25,6 +25,7 @@ #include <asm/io.h> #include <asm/arch/tegra2.h> #include <asm/arch/pinmux.h> +#include <asm/gpio.h> #ifdef CONFIG_TEGRA2_MMC #include <mmc.h> #endif @@ -72,15 +73,6 @@ static void pin_mux_mmc(void) pinmux_tristate_disable(PINGRP_ATC); }
-/* - * Routine: gpio_config_mmc - * Description: Set GPIOs for SD card - */ -void gpio_config_mmc(void) -{ - /* Not implemented for now */ -} - /* this is a weak define that we are overriding */ int board_mmc_init(bd_t *bd) { @@ -88,29 +80,16 @@ int board_mmc_init(bd_t *bd)
/* Enable muxes, etc. for SDMMC controllers */ pin_mux_mmc(); - gpio_config_mmc();
debug("board_mmc_init: init SD slot J26\n"); /* init dev 0, SD slot J26, with 4-bit bus */ /* The board has an 8-bit bus, but 8-bit doesn't work yet */ - tegra2_mmc_init(0, 4); + tegra2_mmc_init(0, 4, GPIO_PI6, GPIO_PH2);
debug("board_mmc_init: init SD slot J5\n"); /* init dev 2, SD slot J5, with 4-bit bus */ - tegra2_mmc_init(2, 4); + tegra2_mmc_init(2, 4, GPIO_PT3, GPIO_PI5);
return 0; } - -/* this is a weak define that we are overriding */ -int board_mmc_getcd(u8 *cd, struct mmc *mmc) -{ - debug("board_mmc_getcd called\n"); - /* - * Hard-code CD presence for now. Need to add GPIO inputs - * for Harmony - */ - *cd = 1; - return 0; -} #endif diff --git a/board/nvidia/seaboard/seaboard.c b/board/nvidia/seaboard/seaboard.c index 22a0e69..356d616 100644 --- a/board/nvidia/seaboard/seaboard.c +++ b/board/nvidia/seaboard/seaboard.c @@ -87,19 +87,6 @@ static void pin_mux_mmc(void) pinmux_tristate_disable(PINGRP_ATC); }
-/* - * Routine: gpio_config_mmc - * Description: Set GPIOs for SDMMC3 SDIO slot. - */ -void gpio_config_mmc(void) -{ - /* Set EN_VDDIO_SD (GPIO I6) */ - gpio_direction_output(GPIO_PI6, 1); - - /* Config pin as GPI for Card Detect (GPIO I5) */ - gpio_direction_input(GPIO_PI5); -} - /* this is a weak define that we are overriding */ int board_mmc_init(bd_t *bd) { @@ -107,31 +94,15 @@ int board_mmc_init(bd_t *bd)
/* Enable muxes, etc. for SDMMC controllers */ pin_mux_mmc(); - gpio_config_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 */ - tegra2_mmc_init(0, 4); + tegra2_mmc_init(0, 4, -1, -1);
debug("board_mmc_init: init SD slot\n"); /* init dev 1, SD slot, with 4-bit bus */ - tegra2_mmc_init(1, 4); - - return 0; -} - -/* this is a weak define that we are overriding */ -int board_mmc_getcd(u8 *cd, struct mmc *mmc) -{ - debug("board_mmc_getcd called\n"); - *cd = 1; /* Assume card is inserted, or eMMC */ - - if (IS_SD(mmc)) { - /* Seaboard SDMMC3 = SDIO3_CD = GPIO_PI5 */ - if (gpio_get_value(GPIO_PI5)) - *cd = 0; - } + tegra2_mmc_init(1, 4, GPIO_PI6, GPIO_PI5);
return 0; } diff --git a/drivers/mmc/tegra2_mmc.c b/drivers/mmc/tegra2_mmc.c index 78b1190..3de9c5d 100644 --- a/drivers/mmc/tegra2_mmc.c +++ b/drivers/mmc/tegra2_mmc.c @@ -21,6 +21,7 @@
#include <common.h> #include <mmc.h> +#include <asm/gpio.h> #include <asm/io.h> #include <asm/arch/clk_rst.h> #include <asm/arch/clock.h> @@ -433,20 +434,37 @@ static int mmc_core_init(struct mmc *mmc) return 0; }
-static int tegra2_mmc_initialize(int dev_index, int bus_width) +int tegra2_mmc_init(int dev_index, int bus_width, int pwr_gpio, int cd_gpio) { struct mmc_host *host; + char gpusage[12]; /* "SD/MMCn PWR" or "SD/MMCn CD" */ struct mmc *mmc;
- debug(" mmc_initialize called\n"); + debug(" tegra2_mmc_init: index %d, bus width %d " + "pwr_gpio %d cd_gpio %d\n", + dev_index, bus_width, pwr_gpio, cd_gpio);
host = &mmc_host[dev_index];
host->clock = 0; + host->pwr_gpio = pwr_gpio; + host->cd_gpio = cd_gpio; tegra2_get_setup(host, dev_index);
clock_start_periph_pll(host->mmc_id, CLOCK_ID_PERIPH, 20000000);
+ if (host->pwr_gpio >= 0) { + sprintf(gpusage, "SD/MMC%d PWR", dev_index); + gpio_request(host->pwr_gpio, gpusage); + gpio_direction_output(host->pwr_gpio, 1); + } + + if (host->cd_gpio >= 0) { + sprintf(gpusage, "SD/MMC%d CD", dev_index); + gpio_request(host->cd_gpio, gpusage); + gpio_direction_input(host->cd_gpio); + } + mmc = &mmc_dev[dev_index];
sprintf(mmc->name, "Tegra2 SD/MMC"); @@ -478,9 +496,21 @@ static int tegra2_mmc_initialize(int dev_index, int bus_width) return 0; }
-int tegra2_mmc_init(int dev_index, int bus_width) +/* this is a weak define that we are overriding */ +int board_mmc_getcd(u8 *cd, struct mmc *mmc) { - debug(" tegra2_mmc_init: index %d, bus width %d\n", - dev_index, bus_width); - return tegra2_mmc_initialize(dev_index, bus_width); + struct mmc_host *host = (struct mmc_host *)mmc->priv; + + debug("board_mmc_getcd called\n"); + + *cd = 1; /* Assume card is inserted, or eMMC */ + + if (IS_SD(mmc)) { + if (host->cd_gpio >= 0) { + if (gpio_get_value(host->cd_gpio)) + *cd = 0; + } + } + + return 0; } diff --git a/drivers/mmc/tegra2_mmc.h b/drivers/mmc/tegra2_mmc.h index 28698e0..72f587b 100644 --- a/drivers/mmc/tegra2_mmc.h +++ b/drivers/mmc/tegra2_mmc.h @@ -74,9 +74,11 @@ struct mmc_host { unsigned int clock; /* Current clock (MHz) */ unsigned int base; /* Base address, SDMMC1/2/3/4 */ enum periph_id mmc_id; /* Peripheral ID: PERIPH_ID_... */ + int pwr_gpio; /* Power GPIO */ + int cd_gpio; /* Change Detect GPIO */ };
-int tegra2_mmc_init(int dev_index, int bus_width); +int tegra2_mmc_init(int dev_index, int bus_width, int pwr_gpio, int cd_gpio);
#endif /* __ASSEMBLY__ */ #endif /* __TEGRA2_MMC_H_ */

* Stephen Warren wrote:
Pass the GPIO numbers for power and card detect to tegra2_mmc_init(), and modify that function to perform all required GPIO initialization. This removes the need for board files to perform these operations.
Move board_mmc_getcd() into tegra2_mmc.c now that the driver knows which GPIOs to use.
Update affected call-sites in seaboard.c and harmony.c. Note that this change should make all SD ports work on Harmony, since the required GPIO setup is now being performed.
v4: Fix prototype of tegra2_mmc_init() in board.h to match driver change. Remove prototype of gpio_config_mmc() from board.h
Signed-off-by: Stephen Warren swarren@nvidia.com Tested-by: Simon Glass sjg@chromium.org
board/nvidia/common/board.h | 3 +- board/nvidia/harmony/harmony.c | 27 ++--------------------- board/nvidia/seaboard/seaboard.c | 33 +---------------------------- drivers/mmc/tegra2_mmc.c | 42 ++++++++++++++++++++++++++++++++----- drivers/mmc/tegra2_mmc.h | 4 ++- 5 files changed, 45 insertions(+), 64 deletions(-)
diff --git a/board/nvidia/common/board.h b/board/nvidia/common/board.h index 344e702..35acbca 100644 --- a/board/nvidia/common/board.h +++ b/board/nvidia/common/board.h @@ -26,7 +26,6 @@
void tegra2_start(void); void gpio_config_uart(void); -void gpio_config_mmc(void); -int tegra2_mmc_init(int dev_index, int bus_width); +int tegra2_mmc_init(int dev_index, int bus_width, int pwr_gpio, int cd_gpio);
[...]
Can we move tegra2_start() and tegra2_mmc_init() into a common Tegra2 header instead of a board-specific one, please? Both functions are not implemented below board/nvidia/common, so there is no need to make their prototype only visible for board/nvidia. Otherwise other boards will have to duplicate this prototype and need to keep it in sync.
tegra2_start() can probably go into arch/arm/include/asm/arch-tegra2/tegra2.h and tegra2_mmc_init() could for example go into a new mmc.h in the same directory.
Thierry

Thierry Reding wrote at Wednesday, November 16, 2011 1:29 PM:
- Stephen Warren wrote:
Pass the GPIO numbers for power and card detect to tegra2_mmc_init(), and modify that function to perform all required GPIO initialization. This removes the need for board files to perform these operations.
Move board_mmc_getcd() into tegra2_mmc.c now that the driver knows which GPIOs to use.
Update affected call-sites in seaboard.c and harmony.c. Note that this change should make all SD ports work on Harmony, since the required GPIO setup is now being performed.
...
diff --git a/board/nvidia/common/board.h b/board/nvidia/common/board.h
...
void tegra2_start(void); void gpio_config_uart(void); -void gpio_config_mmc(void); -int tegra2_mmc_init(int dev_index, int bus_width); +int tegra2_mmc_init(int dev_index, int bus_width, int pwr_gpio, int cd_gpio);
[...]
Can we move tegra2_start() and tegra2_mmc_init() into a common Tegra2 header instead of a board-specific one, please? Both functions are not implemented below board/nvidia/common, so there is no need to make their prototype only visible for board/nvidia. Otherwise other boards will have to duplicate this prototype and need to keep it in sync.
tegra2_start() can probably go into arch/arm/include/asm/arch-tegra2/tegra2.h and tegra2_mmc_init() could for example go into a new mmc.h in the same directory.
I'd certainly be fine with that.
It's probably simplest if you include that change in your patchset though; my patchset doesn't change the location of these functions, so I'd have to introduce another patch for that.

Hi Thierry,
On Wed, Nov 16, 2011 at 1:05 PM, Stephen Warren swarren@nvidia.com wrote:
Thierry Reding wrote at Wednesday, November 16, 2011 1:29 PM:
- Stephen Warren wrote:
Pass the GPIO numbers for power and card detect to tegra2_mmc_init(), and modify that function to perform all required GPIO initialization. This removes the need for board files to perform these operations.
Move board_mmc_getcd() into tegra2_mmc.c now that the driver knows which GPIOs to use.
Update affected call-sites in seaboard.c and harmony.c. Note that this change should make all SD ports work on Harmony, since the required GPIO setup is now being performed.
...
diff --git a/board/nvidia/common/board.h b/board/nvidia/common/board.h
...
void tegra2_start(void); void gpio_config_uart(void); -void gpio_config_mmc(void); -int tegra2_mmc_init(int dev_index, int bus_width); +int tegra2_mmc_init(int dev_index, int bus_width, int pwr_gpio, int cd_gpio);
[...]
Can we move tegra2_start() and tegra2_mmc_init() into a common Tegra2 header instead of a board-specific one, please? Both functions are not implemented below board/nvidia/common, so there is no need to make their prototype only visible for board/nvidia. Otherwise other boards will have to duplicate this prototype and need to keep it in sync.
tegra2_start() can probably go into arch/arm/include/asm/arch-tegra2/tegra2.h
That header file is supported to just be for #defines. How about ap20.h? Anyway, sorry to say that this has been refactored and simplified already. Patches are upstream - it's just that they are not yet applied. Please see my patchwork if you want to base on top of those as well (yes I know this is a pain, sorry).
Regards, Simon
and tegra2_mmc_init() could for example go into a new mmc.h in the same directory.
I'd certainly be fine with that.
It's probably simplest if you include that change in your patchset though; my patchset doesn't change the location of these functions, so I'd have to introduce another patch for that.
-- nvpublic

Ventana is a board which is very similar to Seaboard. Support it by re-using board/nvidia/seaboard/seaboard.c with minor run-time conditionals.
Signed-off-by: Stephen Warren swarren@nvidia.com --- board/nvidia/seaboard/seaboard.c | 11 ++++++- board/nvidia/ventana/Makefile | 55 ++++++++++++++++++++++++++++++++++++++ boards.cfg | 1 + include/configs/ventana.h | 55 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 120 insertions(+), 2 deletions(-) create mode 100644 board/nvidia/ventana/Makefile create mode 100644 include/configs/ventana.h
diff --git a/board/nvidia/seaboard/seaboard.c b/board/nvidia/seaboard/seaboard.c index 356d616..013f1d0 100644 --- a/board/nvidia/seaboard/seaboard.c +++ b/board/nvidia/seaboard/seaboard.c @@ -32,10 +32,10 @@ #include "../common/board.h"
/* - * Routine: gpio_config_uart + * Routine: gpio_config_uart_seaboard * Description: Force GPIO_PI3 low on Seaboard so UART4 works. */ -void gpio_config_uart(void) +void gpio_config_uart_seaboard(void) { int gp = GPIO_PI3; struct gpio_ctlr *gpio = (struct gpio_ctlr *)NV_PA_GPIO_BASE; @@ -56,6 +56,13 @@ void gpio_config_uart(void) writel(val, &bank->gpio_dir_out[GPIO_PORT(gp)]); }
+void gpio_config_uart(void) +{ + if (machine_is_ventana()) + return; + gpio_config_uart_seaboard(); +} + #ifdef CONFIG_TEGRA2_MMC /* * Routine: pin_mux_mmc diff --git a/board/nvidia/ventana/Makefile b/board/nvidia/ventana/Makefile new file mode 100644 index 0000000..029673f --- /dev/null +++ b/board/nvidia/ventana/Makefile @@ -0,0 +1,55 @@ +# +# (C) Copyright 2010,2011 +# 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 += ../seaboard/seaboard.o +COBJS += ../common/board.o + +SRCS := $(COBJS:.o=.c) +OBJS := $(addprefix $(obj),$(COBJS)) + +$(LIB): $(obj).depend $(OBJS) + $(AR) $(ARFLAGS) $@ $(OBJS) + +clean: + rm -f $(OBJS) + +distclean: clean + rm -f $(LIB) core *.bak $(obj).depend + +######################################################################### + +# defines $(obj).depend target +include $(SRCTREE)/rules.mk + +sinclude $(obj).depend + +######################################################################### diff --git a/boards.cfg b/boards.cfg index 65482ac..9211c0a 100644 --- a/boards.cfg +++ b/boards.cfg @@ -192,6 +192,7 @@ s5pc210_universal arm armv7 universal_c210 samsung smdkv310 arm armv7 smdkv310 samsung s5pc2xx harmony arm armv7 harmony nvidia tegra2 seaboard arm armv7 seaboard nvidia tegra2 +ventana arm armv7 ventana nvidia tegra2 u8500_href arm armv7 u8500 st-ericsson u8500 actux1_4_16 arm ixp actux1 - - actux1:FLASH2X2 actux1_8_16 arm ixp actux1 - - actux1:FLASH1X8 diff --git a/include/configs/ventana.h b/include/configs/ventana.h new file mode 100644 index 0000000..afd6ff6 --- /dev/null +++ b/include/configs/ventana.h @@ -0,0 +1,55 @@ +/* + * (C) Copyright 2010,2011 + * 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 "tegra2-common.h" + +/* High-level configuration options */ +#define TEGRA2_SYSMEM "mem=384M@0M nvmem=128M@384M mem=512M@512M" +#define V_PROMPT "Tegra2 (Ventana) # " +#define CONFIG_TEGRA2_BOARD_STRING "NVIDIA Ventana" + +/* Board-specific serial config */ +#define CONFIG_SERIAL_MULTI +#define CONFIG_TEGRA2_ENABLE_UARTD +#define CONFIG_SYS_NS16550_COM1 NV_PA_APB_UARTD_BASE + +#define CONFIG_MACH_TYPE MACH_TYPE_VENTANA +#define CONFIG_SYS_BOARD_ODMDATA 0x300d8011 /* lp1, 1GB */ + +#define CONFIG_BOARD_EARLY_INIT_F + +/* SD/MMC */ +#define CONFIG_MMC +#define CONFIG_GENERIC_MMC +#define CONFIG_TEGRA2_MMC +#define CONFIG_CMD_MMC + +#define CONFIG_DOS_PARTITION +#define CONFIG_EFI_PARTITION +#define CONFIG_CMD_EXT2 +#define CONFIG_CMD_FAT +#endif /* __CONFIG_H */

On Wednesday 12 October 2011 11:18:47 Stephen Warren wrote:
--- /dev/null +++ b/board/nvidia/ventana/Makefile
+$(LIB): $(obj).depend $(OBJS)
- $(AR) $(ARFLAGS) $@ $(OBJS)
your build file is out of date. this needs to use the cmd link target. seem just about any other board Makefile for an example.
+clean:
- rm -f $(OBJS)
+distclean: clean
- rm -f $(LIB) core *.bak $(obj).depend
unused ... just punt them -mike

Mike Frysinger wrote at Wednesday, October 12, 2011 9:27 AM:
On Wednesday 12 October 2011 11:18:47 Stephen Warren wrote:
--- /dev/null +++ b/board/nvidia/ventana/Makefile
+$(LIB): $(obj).depend $(OBJS)
- $(AR) $(ARFLAGS) $@ $(OBJS)
your build file is out of date. this needs to use the cmd link target. seem just about any other board Makefile for an example.
Which repo/branch contains the updated files?
I'm using git://git.denx.de/u-boot.git master, and this new Makefile is identical to existing board/nvidia/{harmony,seaboard}/Makefile. I guess looking at some other board vendors though, you mean that I should make the following change:
$(LIB): $(obj).depend $(OBJS) - $(call cmd_link_o_target, $(OBJS)) + $(AR) $(ARFLAGS) $@ $(OBJS)
If so, I'll also include a patch to update the Harmony/Seaboard files first.
+clean:
- rm -f $(OBJS)
+distclean: clean
- rm -f $(LIB) core *.bak $(obj).depend
unused ... just punt them
These rules appear to be present in at least 80-90% of other board files. Has there simply not been a pass to remove them everywhere yet?
Thanks for the feedback.

On Wednesday 12 October 2011 12:11:23 Stephen Warren wrote:
Mike Frysinger wrote at Wednesday, October 12, 2011 9:27 AM:
On Wednesday 12 October 2011 11:18:47 Stephen Warren wrote:
--- /dev/null +++ b/board/nvidia/ventana/Makefile
+$(LIB): $(obj).depend $(OBJS)
- $(AR) $(ARFLAGS) $@ $(OBJS)
your build file is out of date. this needs to use the cmd link target. seem just about any other board Makefile for an example.
Which repo/branch contains the updated files?
I'm using git://git.denx.de/u-boot.git master, and this new Makefile is identical to existing board/nvidia/{harmony,seaboard}/Makefile. I guess looking at some other board vendors though, you mean that I should make the following change:
$(LIB): $(obj).depend $(OBJS)
- $(call cmd_link_o_target, $(OBJS))
- $(AR) $(ARFLAGS) $@ $(OBJS)
your patch is reverted :). if other boards are using $(AR), it's because during review people didn't notice. calling $(AR) is wrong now. please post patches for those boards too :).
+clean:
- rm -f $(OBJS)
+distclean: clean
- rm -f $(LIB) core *.bak $(obj).depend
unused ... just punt them
These rules appear to be present in at least 80-90% of other board files. Has there simply not been a pass to remove them everywhere yet?
i've posted a big patch to do that already, but it hasn't yet been merged -mike

Mike Frysinger wrote at Wednesday, October 12, 2011 11:02 AM:
On Wednesday 12 October 2011 12:11:23 Stephen Warren wrote:
Mike Frysinger wrote at Wednesday, October 12, 2011 9:27 AM:
On Wednesday 12 October 2011 11:18:47 Stephen Warren wrote:
...
+clean:
- rm -f $(OBJS)
+distclean: clean
- rm -f $(LIB) core *.bak $(obj).depend
unused ... just punt them
These rules appear to be present in at least 80-90% of other board files. Has there simply not been a pass to remove them everywhere yet?
i've posted a big patch to do that already, but it hasn't yet been merged -mike
I found it at: http://lists.denx.de/pipermail/u-boot/2011-September/101648.html
It's unclear from the discussion there if the patch was accepted and is simply waiting to be merged, or if it was rejected. I'm a little hesitant to apply the same patch to the new Ventana board file it the linked patch isn't actually going to be accepted. Can you comment on this?
Thanks.

On Wednesday 12 October 2011 13:18:10 Stephen Warren wrote:
Mike Frysinger wrote at Wednesday, October 12, 2011 11:02 AM:
On Wednesday 12 October 2011 12:11:23 Stephen Warren wrote:
Mike Frysinger wrote at Wednesday, October 12, 2011 9:27 AM:
On Wednesday 12 October 2011 11:18:47 Stephen Warren wrote:
+clean:
- rm -f $(OBJS)
+distclean: clean
- rm -f $(LIB) core *.bak $(obj).depend
unused ... just punt them
These rules appear to be present in at least 80-90% of other board files. Has there simply not been a pass to remove them everywhere yet?
i've posted a big patch to do that already, but it hasn't yet been merged -mike
I found it at: http://lists.denx.de/pipermail/u-boot/2011-September/101648.html
It's unclear from the discussion there if the patch was accepted and is simply waiting to be merged, or if it was rejected. I'm a little hesitant to apply the same patch to the new Ventana board file it the linked patch isn't actually going to be accepted. Can you comment on this?
obviously i think it's just a matter of time ;). important point being that there are boards already without these targets and they build/clean just fine since these sub-targets never get used. -mike

Dear Mike Frysinger,
In message 201110121345.35351.vapier@gentoo.org you wrote:
i've posted a big patch to do that already, but it hasn't yet been merged -mike
I found it at: http://lists.denx.de/pipermail/u-boot/2011-September/101648.html
It's unclear from the discussion there if the patch was accepted and is simply waiting to be merged, or if it was rejected. I'm a little hesitant to apply the same patch to the new Ventana board file it the linked patch isn't actually going to be accepted. Can you comment on this?
obviously i think it's just a matter of time ;). important point being that there are boards already without these targets and they build/clean just fine since these sub-targets never get used.
I don't have this patch on my list. My interpretation was we skip it?
Best regards,
Wolfgang Denk

On Wednesday 12 October 2011 14:38:18 Wolfgang Denk wrote:
Mike Frysinger wrote:
i've posted a big patch to do that already, but it hasn't yet been merged -mike
I found it at: http://lists.denx.de/pipermail/u-boot/2011-September/101648.html
It's unclear from the discussion there if the patch was accepted and is simply waiting to be merged, or if it was rejected. I'm a little hesitant to apply the same patch to the new Ventana board file it the linked patch isn't actually going to be accepted. Can you comment on this?
obviously i think it's just a matter of time ;). important point being that there are boards already without these targets and they build/clean just fine since these sub-targets never get used.
I don't have this patch on my list. My interpretation was we skip it?
it's dead/inconsistent code (as i showed), and i don't think using it gains us anything. for the few edge cases where arches/boards are adding custom targets to the top level "clean" target, i think that better solved by moving to a kbuild style system where the targets are defined in the subdirs and the top level clean target simply removes the summation of all those variables. -mike

Dear Mike Frysinger,
In message 201110121515.59543.vapier@gentoo.org you wrote:
I don't have this patch on my list. My interpretation was we skip it?
it's dead/inconsistent code (as i showed), and i don't think using it gains us anything. for the few edge cases where arches/boards are adding custom targets to the top level "clean" target, i think that better solved by moving to a kbuild style system where the targets are defined in the subdirs and the top level clean target simply removes the summation of all those variables.
Is the patch still OK, or is rebasing needed?
Best regards,
Wolfgang Denk

On Wednesday 12 October 2011 15:29:34 Wolfgang Denk wrote:
Mike Frysinger wrote:
I don't have this patch on my list. My interpretation was we skip it?
it's dead/inconsistent code (as i showed), and i don't think using it gains us anything. for the few edge cases where arches/boards are adding custom targets to the top level "clean" target, i think that better solved by moving to a kbuild style system where the targets are defined in the subdirs and the top level clean target simply removes the summation of all those variables.
Is the patch still OK, or is rebasing needed?
i'll check tonight to see if more boards were added since i last posted -mike
participants (5)
-
Mike Frysinger
-
Simon Glass
-
Stephen Warren
-
Thierry Reding
-
Wolfgang Denk