No subject

we need to use to boot. If we set up clocks and pinmuxes in the board
init, then this is not possible. We had a discussion on the list a few
months back about how to minimize this and the answer given was that
we should only init peripherals when they are used.

Looking ahead to fdt, I think the control point for what gets inited
and what the pinmux settings are will be in the device tree. My
understanding of the concept is that drives will look at their fdt
node and init accordingly.

We already have a function:

int tegra2_mmc_init(int dev_index, int bus_width)

Could this not be enhanced to set the pinmux and clocks? It seems from
your patch that the pinmux is always fixed but if not it could be a
parameter.

The same applies in my view to the GPIOs. With this patch in
board_mmc_getcd() we must look up the index and then decide what GPIO
to use. But the GPIO is inited in the board file and could simply be
recorded by the MMC dirver. In fact if we had:

mmc_init_port(unsigned dev_index, enum periph_id mmc_id,
		struct tegra2_mmc *reg, int bus_width,
		int cd_gpio, int wp_gpio)

or similar, then the GPIO setup and access could be handled by the MMC driv=
er.


> ---
> =A0board/nvidia/common/board.c =A0 =A0 =A0| =A0 77 ++++++++++++++++++++++=
++--------------
> =A0board/nvidia/common/board.h =A0 =A0 =A0| =A0 =A08 +++-
> =A0board/nvidia/harmony/harmony.c =A0 | =A0 56 +++++++++++++++++++++++++-=
-
> =A0board/nvidia/seaboard/seaboard.c | =A0 23 +++++++++++
> =A0drivers/mmc/tegra2_mmc.c =A0 =A0 =A0 =A0 | =A0 18 +++++++++
> =A05 files changed, 148 insertions(+), 34 deletions(-)
>
> diff --git a/board/nvidia/common/board.c b/board/nvidia/common/board.c
> index 8033612..6c09a4c 100644
> --- a/board/nvidia/common/board.c
> +++ b/board/nvidia/common/board.c
> @@ -102,20 +102,37 @@ static void pin_mux_uart(void)
>
> =A0#ifdef CONFIG_TEGRA2_MMC
> =A0/*
> - * Routine: clock_init_mmc
> - * Description: init the PLL and clocks for the SDMMC controllers
> + * Routine: clock_init_mmc4
> + * Description: init the PLL and clocks for SDMMC4 controller
> =A0*/
> -static void clock_init_mmc(void)
> +void clock_init_mmc4(void)
> =A0{
> =A0 =A0 =A0 =A0clock_start_periph_pll(PERIPH_ID_SDMMC4, CLOCK_ID_PERIPH, =
20000000);
> +}
> +
> +/*
> + * Routine: clock_init_mmc3
> + * Description: init the PLL and clocks for SDMMC3 controller
> + */
> +void clock_init_mmc3(void)
> +{
> =A0 =A0 =A0 =A0clock_start_periph_pll(PERIPH_ID_SDMMC3, CLOCK_ID_PERIPH, =
20000000);
> =A0}
>
> =A0/*
> - * Routine: pin_mux_mmc
> - * Description: setup the pin muxes/tristate values for the SDMMC(s)
> + * Routine: clock_init_mmc2
> + * Description: init the PLL and clocks for SDMMC2 controller
> =A0*/
> -static void pin_mux_mmc(void)
> +void clock_init_mmc2(void)
> +{
> + =A0 =A0 =A0 clock_start_periph_pll(PERIPH_ID_SDMMC2, CLOCK_ID_PERIPH, 2=
0000000);
> +}
> +
> +/*
> + * Routine: pin_mux_mmc4
> + * Description: setup the pin muxes/tristate values for SDMMC4
> + */
> +void pin_mux_mmc4(void)
> =A0{
> =A0 =A0 =A0 =A0/* SDMMC4: config 3, x8 on 2nd set of pins */
> =A0 =A0 =A0 =A0pinmux_set_func(PINGRP_ATB, PMUX_FUNC_SDIO4);
> @@ -125,7 +142,14 @@ static void pin_mux_mmc(void)
> =A0 =A0 =A0 =A0pinmux_tristate_disable(PINGRP_ATB);
> =A0 =A0 =A0 =A0pinmux_tristate_disable(PINGRP_GMA);
> =A0 =A0 =A0 =A0pinmux_tristate_disable(PINGRP_GME);
> +}
>
> +/*
> + * Routine: pin_mux_mmc3
> + * Description: setup the pin muxes/tristate values for SDMMC3
> + */
> +void pin_mux_mmc3(void)
> +{
> =A0 =A0 =A0 =A0/* SDMMC3: SDIO3_CLK, SDIO3_CMD, SDIO3_DAT[3:0] */
> =A0 =A0 =A0 =A0pinmux_set_func(PINGRP_SDB, PMUX_FUNC_SDIO3);
> =A0 =A0 =A0 =A0pinmux_set_func(PINGRP_SDC, PMUX_FUNC_SDIO3);
> @@ -135,6 +159,25 @@ static void pin_mux_mmc(void)
> =A0 =A0 =A0 =A0pinmux_tristate_disable(PINGRP_SDD);
> =A0 =A0 =A0 =A0pinmux_tristate_disable(PINGRP_SDB);
> =A0}
> +
> +/*
> + * Routine: pin_mux_mmc2
> + * Description: setup the pin muxes/tristate values for SDMMC2
> + */
> +void pin_mux_mmc2(void)
> +{
> + =A0 =A0 =A0 /* SDMMC2: SDIO2_CLK, SDIO2_CMD, SDIO2_DAT[7:0] */
> + =A0 =A0 =A0 pinmux_set_func(PINGRP_DTA, PMUX_FUNC_SDIO2);
> + =A0 =A0 =A0 pinmux_set_func(PINGRP_DTD, PMUX_FUNC_SDIO2);
> +
> + =A0 =A0 =A0 pinmux_tristate_disable(PINGRP_DTA);
> + =A0 =A0 =A0 pinmux_tristate_disable(PINGRP_DTD);
> +
> + =A0 =A0 =A0 /* For power GPIO PI6 */
> + =A0 =A0 =A0 pinmux_tristate_disable(PINGRP_ATA);
> + =A0 =A0 =A0 /* For power GPIO PT3 */
> + =A0 =A0 =A0 pinmux_tristate_disable(PINGRP_DTB);
> +}
> =A0#endif
>
> =A0/*
> @@ -152,28 +195,6 @@ int board_init(void)
> =A0 =A0 =A0 =A0return 0;
> =A0}
>
> -#ifdef CONFIG_TEGRA2_MMC
> -/* this is a weak define that we are overriding */
> -int board_mmc_init(bd_t *bd)
> -{
> - =A0 =A0 =A0 debug("board_mmc_init called\n");
> - =A0 =A0 =A0 /* Enable clocks, muxes, etc. for SDMMC controllers */
> - =A0 =A0 =A0 clock_init_mmc();
> - =A0 =A0 =A0 pin_mux_mmc();
> - =A0 =A0 =A0 gpio_config_mmc();
> -
> - =A0 =A0 =A0 debug("board_mmc_init: init eMMC\n");
> - =A0 =A0 =A0 /* init dev 0, eMMC chip, with 8-bit bus */
> - =A0 =A0 =A0 tegra2_mmc_init(0, 8);

You could pass the GPIOs to these functions. They could init the
clocks also. For pinmux, are there actually any options?

> -
> - =A0 =A0 =A0 debug("board_mmc_init: init SD slot\n");
> - =A0 =A0 =A0 /* init dev 1, SD slot, with 4-bit bus */
> - =A0 =A0 =A0 tegra2_mmc_init(1, 4);
> -
> - =A0 =A0 =A0 return 0;
> -}
> -#endif
> -
> =A0#ifdef CONFIG_BOARD_EARLY_INIT_F
> =A0int board_early_init_f(void)
> =A0{
> diff --git a/board/nvidia/common/board.h b/board/nvidia/common/board.h
> index 344e702..eee475d 100644
> --- a/board/nvidia/common/board.h
> +++ b/board/nvidia/common/board.h
> @@ -26,7 +26,13 @@
>
> =A0void tegra2_start(void);
> =A0void gpio_config_uart(void);
> -void gpio_config_mmc(void);
> +void clock_init_mmc4(void);
> +void clock_init_mmc3(void);
> +void clock_init_mmc2(void);
> +void pin_mux_mmc4(void);
> +void pin_mux_mmc3(void);
> +void pin_mux_mmc2(void);
> =A0int tegra2_mmc_init(int dev_index, int bus_width);
> +int tegra2_mmc_index(struct mmc *mmc);
>
> =A0#endif /* BOARD_H */
> diff --git a/board/nvidia/harmony/harmony.c b/board/nvidia/harmony/harmon=
y.c
> index cbb30d6..228ae2e 100644
> --- a/board/nvidia/harmony/harmony.c
> +++ b/board/nvidia/harmony/harmony.c
> @@ -23,10 +23,13 @@
>
> =A0#include <common.h>
> =A0#include <asm/io.h>
> +#include <asm/arch/clock.h>
> =A0#include <asm/arch/tegra2.h>
> +#include <asm/gpio.h>
> =A0#ifdef CONFIG_TEGRA2_MMC
> =A0#include <mmc.h>
> =A0#endif
> +#include "../common/board.h"
>
> =A0/*
> =A0* Routine: gpio_config_uart
> @@ -43,18 +46,61 @@ void gpio_config_uart(void)
> =A0*/
> =A0void gpio_config_mmc(void)
> =A0{
> - =A0 =A0 =A0 /* Not implemented for now */
> + =A0 =A0 =A0 /* Set SDMMC4 power (GPIO I6) */
> + =A0 =A0 =A0 gpio_request(GPIO_PI6, "SDMMC4 power");
> + =A0 =A0 =A0 gpio_direction_output(GPIO_PI6, 1);
> +
> + =A0 =A0 =A0 /* Config pin as GPI for SDMMC4 Card Detect (GPIO H2) */
> + =A0 =A0 =A0 gpio_request(GPIO_PH2, "SDMMC4 card detect");
> + =A0 =A0 =A0 gpio_direction_input(GPIO_PH2);
> +
> + =A0 =A0 =A0 /* Set SDMMC2 power (GPIO T3) */
> + =A0 =A0 =A0 gpio_request(GPIO_PT3, "SDMMC2 power");
> + =A0 =A0 =A0 gpio_direction_output(GPIO_PT3, 1);
> +
> + =A0 =A0 =A0 /* Config pin as GPI for SDMMC2 Card Detect (GPIO I5) */
> + =A0 =A0 =A0 gpio_request(GPIO_PI5, "SDMMC2 card detect");
> + =A0 =A0 =A0 gpio_direction_input(GPIO_PI5);
> +}
> +
> +/* this is a weak define that we are overriding */
> +int board_mmc_init(bd_t *bd)
> +{
> + =A0 =A0 =A0 debug("board_mmc_init called\n");
> + =A0 =A0 =A0 /* Enable clocks, muxes, etc. for SDMMC controllers */
> + =A0 =A0 =A0 clock_init_mmc4();
> + =A0 =A0 =A0 clock_init_mmc2();
> + =A0 =A0 =A0 pin_mux_mmc4();
> + =A0 =A0 =A0 pin_mux_mmc2();
> + =A0 =A0 =A0 gpio_config_mmc();
> +
> + =A0 =A0 =A0 debug("board_mmc_init: init SD slot J26\n");
> + =A0 =A0 =A0 /* init dev 0, SD slot J26, with 8-bit bus */
> + =A0 =A0 =A0 tegra2_mmc_init(0, 8);
> +
> + =A0 =A0 =A0 debug("board_mmc_init: init SD slot J5\n");
> + =A0 =A0 =A0 /* init dev 2, SD slot J5, with 8-bit bus */
> + =A0 =A0 =A0 tegra2_mmc_init(2, 4);
> +
> + =A0 =A0 =A0 return 0;
> =A0}
>
> =A0/* this is a weak define that we are overriding */
> =A0int board_mmc_getcd(u8 *cd, struct mmc *mmc)
> =A0{
> =A0 =A0 =A0 =A0debug("board_mmc_getcd called\n");
> - =A0 =A0 =A0 /*
> - =A0 =A0 =A0 =A0* Hard-code CD presence for now. Need to add GPIO inputs
> - =A0 =A0 =A0 =A0* for Harmony
> - =A0 =A0 =A0 =A0*/
> =A0 =A0 =A0 =A0*cd =3D 1;
> +
> + =A0 =A0 =A0 if (tegra2_mmc_index(mmc) =3D=3D 0) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* Harmony SDMMC4 =3D SDIO4_CD =3D GPIO_PH2=
 */
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (gpio_get_value(GPIO_PH2))
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 *cd =3D 0;
> + =A0 =A0 =A0 } else {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* Harmony SDMMC2 =3D SDIO2_CD =3D GPIO_PI5=
 */
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (gpio_get_value(GPIO_PI5))
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 *cd =3D 0;
> + =A0 =A0 =A0 }

This doesn't seem very nice - it has to 'reverse calculate' the GPIO -
it means that the GPIO is referenced in gpio_config_mmc() and here,
but in different ways. Better to pass the GPIOs to the driver?

> +
> =A0 =A0 =A0 =A0return 0;
> =A0}
> =A0#endif
> diff --git a/board/nvidia/seaboard/seaboard.c b/board/nvidia/seaboard/sea=
board.c
> index 1310e3d..0667eaa 100644
> --- a/board/nvidia/seaboard/seaboard.c
> +++ b/board/nvidia/seaboard/seaboard.c
> @@ -28,6 +28,7 @@
> =A0#ifdef CONFIG_TEGRA2_MMC
> =A0#include <mmc.h>
> =A0#endif
> +#include "../common/board.h"
>
> =A0/*
> =A0* Routine: gpio_config_uart
> @@ -71,6 +72,28 @@ void gpio_config_mmc(void)
> =A0}
>
> =A0/* this is a weak define that we are overriding */
> +int board_mmc_init(bd_t *bd)
> +{
> + =A0 =A0 =A0 debug("board_mmc_init called\n");
> + =A0 =A0 =A0 /* Enable clocks, muxes, etc. for SDMMC controllers */
> + =A0 =A0 =A0 clock_init_mmc4();
> + =A0 =A0 =A0 clock_init_mmc3();
> + =A0 =A0 =A0 pin_mux_mmc4();
> + =A0 =A0 =A0 pin_mux_mmc3();
> + =A0 =A0 =A0 gpio_config_mmc();
> +
> + =A0 =A0 =A0 debug("board_mmc_init: init eMMC\n");
> + =A0 =A0 =A0 /* init dev 0, eMMC chip, with 8-bit bus */
> + =A0 =A0 =A0 tegra2_mmc_init(0, 8);
> +
> + =A0 =A0 =A0 debug("board_mmc_init: init SD slot\n");
> + =A0 =A0 =A0 /* init dev 1, SD slot, with 4-bit bus */
> + =A0 =A0 =A0 tegra2_mmc_init(1, 4);
> +
> + =A0 =A0 =A0 return 0;
> +}
> +
> +/* this is a weak define that we are overriding */
> =A0int board_mmc_getcd(u8 *cd, struct mmc *mmc)
> =A0{
> =A0 =A0 =A0 =A0debug("board_mmc_getcd called\n");
> diff --git a/drivers/mmc/tegra2_mmc.c b/drivers/mmc/tegra2_mmc.c
> index 9e741f2..223dbf3 100644
> --- a/drivers/mmc/tegra2_mmc.c
> +++ b/drivers/mmc/tegra2_mmc.c
> @@ -478,3 +478,21 @@ int tegra2_mmc_init(int dev_index, int bus_width)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0dev_index, bus_width);
> =A0 =A0 =A0 =A0return tegra2_mmc_initialize(dev_index, bus_width);
> =A0}
> +
> +int tegra2_mmc_index(struct mmc *mmc)
> +{
> + =A0 =A0 =A0 struct mmc_host *host =3D (struct mmc_host *)mmc->priv;
> +
> + =A0 =A0 =A0 switch (host->base) {
> + =A0 =A0 =A0 case TEGRA2_SDMMC3_BASE:
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return 1;
> + =A0 =A0 =A0 case TEGRA2_SDMMC2_BASE:
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return 2;
> + =A0 =A0 =A0 case TEGRA2_SDMMC1_BASE:
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return 3;
> + =A0 =A0 =A0 case TEGRA2_SDMMC4_BASE:
> + =A0 =A0 =A0 default:
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return 0;
> + =A0 =A0 =A0 }
> +}

It seems odd to have to look up the index based on the peripheral
address - can we not store this information?

At the top of the file there is a function:

static inline struct tegra2_mmc *tegra2_get_base_mmc(int dev_index)

which seems to go the other way!

Regards,
Simon


> +
> --
> 1.7.0.4
>
>