On Fri, Feb 25, 2011 at 12:19 PM, Rohit Vaswani<rvaswani@xxxxxxxxxxxxxx> wrote:
Remove GPIOMUX_VALID, simplify the API, and absorb the complexity
of determining which gpiomux configs are used and which are
not back into the gpiomux implementation where it belongs.
Allow NULL settings to be represented appropriately.
The configuration enumerations represent a single abstraction
and were needlessly split. As such, collapse the conflicting
abstractions into a consistent one and move the implementation
complexity down into the implementation (where it belongs) and
away from the interface (where it doesn't belong).
Signed-off-by: Rohit Vaswani<rvaswani@xxxxxxxxxxxxxx>
---
arch/arm/mach-msm/board-msm7x30.c | 23 +++++++--
arch/arm/mach-msm/board-qsd8x50.c | 17 +++++--
arch/arm/mach-msm/gpiomux-v1.c | 9 ++-
arch/arm/mach-msm/gpiomux-v1.h | 59 ----------------------
arch/arm/mach-msm/gpiomux-v2.c | 8 ++-
arch/arm/mach-msm/gpiomux-v2.h | 59 ----------------------
arch/arm/mach-msm/gpiomux.c | 77 +++++++++++++++++-------------
arch/arm/mach-msm/gpiomux.h | 96
+++++++++++++++++++++++-------------
8 files changed, 145 insertions(+), 203 deletions(-)
delete mode 100644 arch/arm/mach-msm/gpiomux-v1.h
delete mode 100644 arch/arm/mach-msm/gpiomux-v2.h
diff --git a/arch/arm/mach-msm/board-msm7x30.c
b/arch/arm/mach-msm/board-msm7x30.c
index 59b91de..4223329 100644
--- a/arch/arm/mach-msm/board-msm7x30.c
+++ b/arch/arm/mach-msm/board-msm7x30.c
@@ -39,8 +39,11 @@
#include "gpiomux.h"
#include "proc_comm.h"
-#define UART2_SUSPENDED (GPIOMUX_DRV_2MA | GPIOMUX_PULL_DOWN |\
- GPIOMUX_FUNC_2 | GPIOMUX_VALID)
+static struct gpiomux_setting uart2_suspended = {
+ .func = GPIOMUX_FUNC_2,
+ .drv = GPIOMUX_DRV_2MA,
+ .pull = GPIOMUX_PULL_DOWN,
+};
extern struct sys_timer msm_timer;
@@ -59,19 +62,27 @@ static struct msm_otg_platform_data msm_otg_pdata = {
struct msm_gpiomux_config msm7x30_uart2_configs[] __initdata = {
{
.gpio = 49, /* UART2 RFR */
- .suspended = UART2_SUSPENDED,
+ .settings = {
+ [GPIOMUX_SUSPENDED] =&uart2_suspended,
+ },
},
{
.gpio = 50, /* UART2 CTS */
- .suspended = UART2_SUSPENDED,
+ .settings = {
+ [GPIOMUX_SUSPENDED] =&uart2_suspended,
+ },
},
{
.gpio = 51, /* UART2 RX */
- .suspended = UART2_SUSPENDED,
+ .settings = {
+ [GPIOMUX_SUSPENDED] =&uart2_suspended,
+ },
},
{
.gpio = 52, /* UART2 TX */
- .suspended = UART2_SUSPENDED,
+ .settings = {
+ [GPIOMUX_SUSPENDED] =&uart2_suspended,
+ },
},
};
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?
--Dima