diff options
author | Gabe Black <gabeblack@google.com> | 2013-06-15 20:33:05 -0700 |
---|---|---|
committer | Stefan Reinauer <stefan.reinauer@coreboot.org> | 2013-07-10 21:49:55 +0200 |
commit | fe6406033fe327d4ae408b02efc060b4b421bc03 (patch) | |
tree | 7977185b4d8a2448430ec54b31f983c52c460ad4 /src/mainboard | |
parent | 001056f560dfec46aa98659f318819cce7098e5b (diff) | |
download | coreboot-fe6406033fe327d4ae408b02efc060b4b421bc03.tar.xz |
exynos5250: De-switch-ify the pinmux configuration code.
The pinmux code for the exynos5250 was all bundled into a single, large
function which contained a switch statement that would set up the pins for
different peripherals within the SOC. There was also a "flags" parameter, the
meaning of which, if any, depended on which peripheral was being set up.
There are several problems with that approach. First, the code is inefficient
in both time and space. The caller knows which peripheral it wants to set up,
but that information is encoded in a constant which has to be unpacked within
the function before any action can be taken. If there were a function per
peripheral, that information would be implicit. Also, the compiler and linker
are forced to include the entire function with all its cases even if most of
them are never called. If each peripheral was a function, the unused ones
could be garbage collected.
Second, it would be possible to try to set up a peripheral which that function
doesn't know about, so there has to be additional error checking/handling. If
each peripheral had a function, the fact that there was a function to call at
all would imply that the call would be understood.
Third, the flags parameter is fairly opaque, usually doesn't do anything, and
sometimes has to have multiple values embedded in it. By having separate
functions, you can have only the parameters you actually want, give them
names that make sense, and pass in values directly.
Fourth, having one giant function pretends to be a generic, portable API, but
in reality, the only way it's useful is to call it with constants which are
specific to a particular implementation of that API. It's highly unlikely that
a bit of code will need to set up a peripheral but have no idea what that
peripheral actually is.
Call sights for the prior pinmux API have been updated. Also, pinmux
initialization within the i2c driver was moved to be in the board setup code
where it really probably belongs. The function block that implements the I2C
controller may be shared between multiple SOCs (and in fact is), and those
SOCs may have different pinmuxes (which they do).
Other places this same sort of change can be made are the pinmux code for the
5420, and the clock configuration code for both the 5250 and the 5420.
Change-Id: Ie9133a895e0dd861cb06a6d5f995b8770b6dc8cf
Signed-off-by: Gabe Black <gabeblack@chromium.org>
Reviewed-on: http://review.coreboot.org/3673
Tested-by: build bot (Jenkins)
Reviewed-by: Stefan Reinauer <stefan.reinauer@coreboot.org>
Diffstat (limited to 'src/mainboard')
-rw-r--r-- | src/mainboard/google/snow/mainboard.c | 16 | ||||
-rw-r--r-- | src/mainboard/google/snow/romstage.c | 7 |
2 files changed, 12 insertions, 11 deletions
diff --git a/src/mainboard/google/snow/mainboard.c b/src/mainboard/google/snow/mainboard.c index a0d6a61e0e..5aedac9646 100644 --- a/src/mainboard/google/snow/mainboard.c +++ b/src/mainboard/google/snow/mainboard.c @@ -56,7 +56,7 @@ static enum exynos5_gpio_pin dp_hpd = GPIO_X07; /* active high */ static void exynos_dp_bridge_setup(void) { - exynos_pinmux_config(PERIPH_ID_DPHPD, 0); + exynos_pinmux_dphpd(); gpio_set_value(dp_pd_l, 1); gpio_cfg_pin(dp_pd_l, GPIO_OUTPUT); @@ -179,12 +179,12 @@ static void disable_usb30_pll(void) static void gpio_init(void) { /* Set up the I2C busses. */ - exynos_pinmux_config(PERIPH_ID_I2C0, PINMUX_FLAG_NONE); - exynos_pinmux_config(PERIPH_ID_I2C1, PINMUX_FLAG_NONE); - exynos_pinmux_config(PERIPH_ID_I2C2, PINMUX_FLAG_NONE); - exynos_pinmux_config(PERIPH_ID_I2C3, PINMUX_FLAG_NONE); - exynos_pinmux_config(PERIPH_ID_I2C4, PINMUX_FLAG_NONE); - exynos_pinmux_config(PERIPH_ID_I2C7, PINMUX_FLAG_NONE); + exynos_pinmux_i2c0(); + exynos_pinmux_i2c1(); + exynos_pinmux_i2c2(); + exynos_pinmux_i2c3(); + exynos_pinmux_i2c4(); + exynos_pinmux_i2c7(); /* Set up the GPIOs used to arbitrate for I2C bus 4. */ gpio_set_pull(GPIO_F03, GPIO_PULL_NONE); @@ -199,7 +199,7 @@ static void gpio_init(void) gpio_direction_output(GPIO_X15, 1); /* Set up the I2S busses. */ - exynos_pinmux_config(PERIPH_ID_I2S1, PINMUX_FLAG_NONE); + exynos_pinmux_i2s1(); } /* this happens after cpu_init where exynos resources are set */ diff --git a/src/mainboard/google/snow/romstage.c b/src/mainboard/google/snow/romstage.c index ec810ce8de..8c01536a32 100644 --- a/src/mainboard/google/snow/romstage.c +++ b/src/mainboard/google/snow/romstage.c @@ -50,6 +50,7 @@ static void setup_power(void) power_init(); /* Initialize I2C bus to configure PMIC. */ + exynos_pinmux_i2c0(); i2c_init(0, I2C_0_SPEED, 0x00); printk(BIOS_DEBUG, "%s: Setting up PMIC...\n", __func__); @@ -100,16 +101,16 @@ static void setup_storage(void) } gpio_set_pull(MMC0_GPIO_PIN, GPIO_PULL_NONE); gpio_set_drv(MMC0_GPIO_PIN, GPIO_DRV_4X); - exynos_pinmux_config(PERIPH_ID_SDMMC0, PINMUX_FLAG_8BIT_MODE); + exynos_pinmux_sdmmc0(); /* MMC2: Removable, 4 bit mode, no GPIO. */ clock_set_mshci(PERIPH_ID_SDMMC2); - exynos_pinmux_config(PERIPH_ID_SDMMC2, 0); + exynos_pinmux_sdmmc2(); } static void setup_graphics(void) { - exynos_pinmux_config(PERIPH_ID_DPHPD, 0); + exynos_pinmux_dphpd(); } static void setup_gpio(void) |