On Fri, Feb 25, 2011 at 12:19 PM, Rohit Vaswani<rvaswani@xxxxxxxxxxxxxx> wrote:diff --git a/arch/arm/mach-msm/board-qsd8x50.c
b/arch/arm/mach-msm/board-qsd8x50.c
index 33ab1fe..d665b0e 100644
--- a/arch/arm/mach-msm/board-qsd8x50.c
+++ b/arch/arm/mach-msm/board-qsd8x50.c
@@ -38,19 +38,26 @@
#include "devices.h"
#include "gpiomux.h"
-#define UART3_SUSPENDED (GPIOMUX_DRV_2MA | GPIOMUX_PULL_DOWN |\
- GPIOMUX_FUNC_1 | GPIOMUX_VALID)
+static struct gpiomux_setting uart3_suspended = {
+ .drv = GPIOMUX_DRV_2MA,
+ .pull = GPIOMUX_PULL_DOWN,
+ .func = GPIOMUX_FUNC_1,
+};
extern struct sys_timer msm_timer;
-struct msm_gpiomux_config qsd8x50_uart3_configs[] __initdata = {
+struct msm_gpiomux_config qsd8x50_uart3_configs[] = {
{
.gpio = 86, /* UART3 RX */
- .suspended = UART3_SUSPENDED,
+ .settings = {
+ [GPIOMUX_SUSPENDED] =&uart3_suspended,
+ },
},
{
.gpio = 87, /* UART3 TX */
- .suspended = UART3_SUSPENDED,
+ .settings = {
+ [GPIOMUX_SUSPENDED] =&uart3_suspended,
+ },
},
};
I think this new interface is way too verbose and will quickly get
unwieldy for configurations that have more than a few pins. For
instance, imagine what the above would look like when muxing a 24bit
LCD pin list...
How about adding a "bool valid" to gpiomux_setting, and convert the
"sets" array to an array of settings and not pointers to settings.
This will allow us to do (in gpiomux.h):
struct msm_gpiomux_rec {
struct gpiomux_setting sets[GPIOMUX_NSETTINGS];
int ref;
};
struct gpiomux_setting {
enum gpiomux_func func;
enum gpiomux_drv drv;
enum gpiomux_pull pull;
bool valid;
};
This way, I can do something like (very rough):
#define GPIOMUX_SET(func,drv,pull) { \
.func = GPIOMUX_##func, \
.drv = GPIOMUX_##drv, \
.pull = GPIOMUX_##pull, \
.valid = true, \
}
#define GPIOMUX_SET_NONE { .valid = false, }
#define GPIOMUX_CFG(g, active, suspended) { \
.gpio = g, \
.sets = { \
[GPIOMUX_ACTIVE] = active, \
[GPIOMUX_SUSPENDED] = suspended, \
}, \
}
This will then allow me to define the uart3 pinmuxing in my board file
as follows:
struct msm_gpiomux_rec uart3_mux_cfg[] = {
GPIOMUX_CFG(86, GPIOMUX_SET_NONE,
GPIOMUX_SET(FUNC_1, DRV_2MA, PULL_DOWN)),
GPIOMUX_CFG(87, GPIOMUX_SET_NONE,
GPIOMUX_SET(FUNC_1, DRV_2MA, PULL_DOWN)),
};
Thoughts?