On 02/25/2011 05:20 PM, Dima Zavin wrote:
On Fri, Feb 25, 2011 at 12:19 PM, Rohit Vaswani<[email protected]> 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?
I haven't read this GPIO code thoroughly, but by looking just at the
diff, I think you can still have these type of macros with the structure
definition Rohit chose. I have no opinion one which struct definition is
better (not enough context). Just trying to help with writing helper macros.
The trick is to use pointers to anonymous struct. A very rough macro:
#define GPIOMUX_SET(f, d, p) \
&(struct gpiomux_setting) {
.func = f,
.drv = d,
.pull = p,
}
#define GPIOMUX_CFG(g, active, suspended) { \
.gpio = g,
.settings = {
[ACTIVE] = active,
[SUSPENDED] = suspended,
}
}
struct msm_gpiomux_config foo_bar[] = {
GPIOMUX_CFG(10, GPIOMUX_SET(FUNC, 2MA, PULL_UP), NULL),
GPIOMUX_CFG(11, GPIOMUX_SET(FUNC, 2MA, PULL_UP), NULL),
};
I'm certain the pointer to anonymous struct stuff works. You might have
to tweak the macros a bit though. Hope this help.
-Saravana
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
