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 <[email protected]>
---
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,
+ },
},
};
diff --git a/arch/arm/mach-msm/gpiomux-v1.c b/arch/arm/mach-msm/gpiomux-v1.c
index 27de2ab..a9525c2 100644
--- a/arch/arm/mach-msm/gpiomux-v1.c
+++ b/arch/arm/mach-msm/gpiomux-v1.c
@@ -18,13 +18,16 @@
#include "gpiomux.h"
#include "proc_comm.h"
-void __msm_gpiomux_write(unsigned gpio, gpiomux_config_t val)
+void __msm_gpiomux_write(unsigned gpio, struct gpiomux_setting val)
{
- unsigned tlmm_config = (val & ~GPIOMUX_CTL_MASK) |
- ((gpio & 0x3ff) << 4);
+ unsigned tlmm_config;
unsigned tlmm_disable = 0;
int rc;
+ tlmm_config = (val.drv << 17) |
+ (val.pull << 15) |
+ ((gpio & 0x3ff) << 4) |
+ val.func;
rc = msm_proc_comm(PCOM_RPC_GPIO_TLMM_CONFIG_EX,
&tlmm_config, &tlmm_disable);
if (rc)
diff --git a/arch/arm/mach-msm/gpiomux-v1.h b/arch/arm/mach-msm/gpiomux-v1.h
deleted file mode 100644
index 96ad5fa..0000000
--- a/arch/arm/mach-msm/gpiomux-v1.h
+++ /dev/null
@@ -1,59 +0,0 @@
-/* Copyright (c) 2010, Code Aurora Forum. All rights reserved.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 and
- * only version 2 as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
- * 02110-1301, USA.
- */
-#ifndef __ARCH_ARM_MACH_MSM_GPIOMUX_V1_H
-#define __ARCH_ARM_MACH_MSM_GPIOMUX_V1_H
-
-typedef u32 gpiomux_config_t;
-
-enum {
- GPIOMUX_DRV_2MA = 0UL << 17,
- GPIOMUX_DRV_4MA = 1UL << 17,
- GPIOMUX_DRV_6MA = 2UL << 17,
- GPIOMUX_DRV_8MA = 3UL << 17,
- GPIOMUX_DRV_10MA = 4UL << 17,
- GPIOMUX_DRV_12MA = 5UL << 17,
- GPIOMUX_DRV_14MA = 6UL << 17,
- GPIOMUX_DRV_16MA = 7UL << 17,
-};
-
-enum {
- GPIOMUX_FUNC_GPIO = 0UL,
- GPIOMUX_FUNC_1 = 1UL,
- GPIOMUX_FUNC_2 = 2UL,
- GPIOMUX_FUNC_3 = 3UL,
- GPIOMUX_FUNC_4 = 4UL,
- GPIOMUX_FUNC_5 = 5UL,
- GPIOMUX_FUNC_6 = 6UL,
- GPIOMUX_FUNC_7 = 7UL,
- GPIOMUX_FUNC_8 = 8UL,
- GPIOMUX_FUNC_9 = 9UL,
- GPIOMUX_FUNC_A = 10UL,
- GPIOMUX_FUNC_B = 11UL,
- GPIOMUX_FUNC_C = 12UL,
- GPIOMUX_FUNC_D = 13UL,
- GPIOMUX_FUNC_E = 14UL,
- GPIOMUX_FUNC_F = 15UL,
-};
-
-enum {
- GPIOMUX_PULL_NONE = 0UL << 15,
- GPIOMUX_PULL_DOWN = 1UL << 15,
- GPIOMUX_PULL_KEEPER = 2UL << 15,
- GPIOMUX_PULL_UP = 3UL << 15,
-};
-
-#endif
diff --git a/arch/arm/mach-msm/gpiomux-v2.c b/arch/arm/mach-msm/gpiomux-v2.c
index 273396d..0a04d60 100644
--- a/arch/arm/mach-msm/gpiomux-v2.c
+++ b/arch/arm/mach-msm/gpiomux-v2.c
@@ -18,8 +18,10 @@
#include <mach/msm_iomap.h>
#include "gpiomux.h"
-void __msm_gpiomux_write(unsigned gpio, gpiomux_config_t val)
+void __msm_gpiomux_write(unsigned gpio, struct gpiomux_setting val)
{
- writel(val & ~GPIOMUX_CTL_MASK,
- MSM_TLMM_BASE + 0x1000 + (0x10 * gpio));
+ uint32_t bits;
+
+ bits = (val.drv << 6) | (val.func << 2) | val.pull;
+ writel(bits, MSM_TLMM_BASE + 0x1000 + (0x10 * gpio));
}
diff --git a/arch/arm/mach-msm/gpiomux-v2.h b/arch/arm/mach-msm/gpiomux-v2.h
deleted file mode 100644
index a7dec1ea..0000000
--- a/arch/arm/mach-msm/gpiomux-v2.h
+++ /dev/null
@@ -1,59 +0,0 @@
-/* Copyright (c) 2010, Code Aurora Forum. All rights reserved.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 and
- * only version 2 as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
- * 02110-1301, USA.
- */
-#ifndef __ARCH_ARM_MACH_MSM_GPIOMUX_V2_H
-#define __ARCH_ARM_MACH_MSM_GPIOMUX_V2_H
-
-typedef u16 gpiomux_config_t;
-
-enum {
- GPIOMUX_DRV_2MA = 0UL << 6,
- GPIOMUX_DRV_4MA = 1UL << 6,
- GPIOMUX_DRV_6MA = 2UL << 6,
- GPIOMUX_DRV_8MA = 3UL << 6,
- GPIOMUX_DRV_10MA = 4UL << 6,
- GPIOMUX_DRV_12MA = 5UL << 6,
- GPIOMUX_DRV_14MA = 6UL << 6,
- GPIOMUX_DRV_16MA = 7UL << 6,
-};
-
-enum {
- GPIOMUX_FUNC_GPIO = 0UL << 2,
- GPIOMUX_FUNC_1 = 1UL << 2,
- GPIOMUX_FUNC_2 = 2UL << 2,
- GPIOMUX_FUNC_3 = 3UL << 2,
- GPIOMUX_FUNC_4 = 4UL << 2,
- GPIOMUX_FUNC_5 = 5UL << 2,
- GPIOMUX_FUNC_6 = 6UL << 2,
- GPIOMUX_FUNC_7 = 7UL << 2,
- GPIOMUX_FUNC_8 = 8UL << 2,
- GPIOMUX_FUNC_9 = 9UL << 2,
- GPIOMUX_FUNC_A = 10UL << 2,
- GPIOMUX_FUNC_B = 11UL << 2,
- GPIOMUX_FUNC_C = 12UL << 2,
- GPIOMUX_FUNC_D = 13UL << 2,
- GPIOMUX_FUNC_E = 14UL << 2,
- GPIOMUX_FUNC_F = 15UL << 2,
-};
-
-enum {
- GPIOMUX_PULL_NONE = 0UL,
- GPIOMUX_PULL_DOWN = 1UL,
- GPIOMUX_PULL_KEEPER = 2UL,
- GPIOMUX_PULL_UP = 3UL,
-};
-
-#endif
diff --git a/arch/arm/mach-msm/gpiomux.c b/arch/arm/mach-msm/gpiomux.c
index 9ef9864..1ca26ec 100644
--- a/arch/arm/mach-msm/gpiomux.c
+++ b/arch/arm/mach-msm/gpiomux.c
@@ -20,21 +20,21 @@
#include "gpiomux.h"
struct msm_gpiomux_rec {
- gpiomux_config_t active;
- gpiomux_config_t suspended;
- int ref;
+ struct gpiomux_setting *sets[GPIOMUX_NSETTINGS];
+ int ref;
};
static DEFINE_SPINLOCK(gpiomux_lock);
static struct msm_gpiomux_rec *msm_gpiomux_recs;
+static struct gpiomux_setting *msm_gpiomux_sets;
static unsigned msm_gpiomux_ngpio;
-int msm_gpiomux_write(unsigned gpio,
- gpiomux_config_t active,
- gpiomux_config_t suspended)
+int msm_gpiomux_write(unsigned gpio, enum msm_gpiomux_setting which,
+ struct gpiomux_setting *setting)
{
- struct msm_gpiomux_rec *cfg = msm_gpiomux_recs + gpio;
+ struct msm_gpiomux_rec *rec = msm_gpiomux_recs + gpio;
+ unsigned set_slot = gpio * GPIOMUX_NSETTINGS + which;
unsigned long irq_flags;
- gpiomux_config_t setting;
+ struct gpiomux_setting *new_set;
if (!msm_gpiomux_recs)
return -EFAULT;
@@ -44,15 +44,17 @@ int msm_gpiomux_write(unsigned gpio,
spin_lock_irqsave(&gpiomux_lock, irq_flags);
- if (active & GPIOMUX_VALID)
- cfg->active = active;
-
- if (suspended & GPIOMUX_VALID)
- cfg->suspended = suspended;
+ if (setting) {
+ msm_gpiomux_sets[set_slot] = *setting;
+ rec->sets[which] = &msm_gpiomux_sets[set_slot];
+ } else {
+ rec->sets[which] = NULL;
+ }
- setting = cfg->ref ? active : suspended;
- if (setting & GPIOMUX_VALID)
- __msm_gpiomux_write(gpio, setting);
+ new_set = rec->ref ? rec->sets[GPIOMUX_ACTIVE] :
+ rec->sets[GPIOMUX_SUSPENDED];
+ if (new_set)
+ __msm_gpiomux_write(gpio, *new_set);
spin_unlock_irqrestore(&gpiomux_lock, irq_flags);
return 0;
@@ -61,7 +63,7 @@ EXPORT_SYMBOL(msm_gpiomux_write);
int msm_gpiomux_get(unsigned gpio)
{
- struct msm_gpiomux_rec *cfg = msm_gpiomux_recs + gpio;
+ struct msm_gpiomux_rec *rec = msm_gpiomux_recs + gpio;
unsigned long irq_flags;
if (!msm_gpiomux_recs)
@@ -71,8 +73,8 @@ int msm_gpiomux_get(unsigned gpio)
return -EINVAL;
spin_lock_irqsave(&gpiomux_lock, irq_flags);
- if (cfg->ref++ == 0 && cfg->active & GPIOMUX_VALID)
- __msm_gpiomux_write(gpio, cfg->active);
+ if (rec->ref++ == 0 && rec->sets[GPIOMUX_ACTIVE])
+ __msm_gpiomux_write(gpio, *rec->sets[GPIOMUX_ACTIVE]);
spin_unlock_irqrestore(&gpiomux_lock, irq_flags);
return 0;
}
@@ -80,7 +82,7 @@ EXPORT_SYMBOL(msm_gpiomux_get);
int msm_gpiomux_put(unsigned gpio)
{
- struct msm_gpiomux_rec *cfg = msm_gpiomux_recs + gpio;
+ struct msm_gpiomux_rec *rec = msm_gpiomux_recs + gpio;
unsigned long irq_flags;
if (!msm_gpiomux_recs)
@@ -90,9 +92,9 @@ int msm_gpiomux_put(unsigned gpio)
return -EINVAL;
spin_lock_irqsave(&gpiomux_lock, irq_flags);
- BUG_ON(cfg->ref == 0);
- if (--cfg->ref == 0 && cfg->suspended & GPIOMUX_VALID)
- __msm_gpiomux_write(gpio, cfg->suspended);
+ BUG_ON(rec->ref == 0);
+ if (--rec->ref == 0 && rec->sets[GPIOMUX_SUSPENDED])
+ __msm_gpiomux_write(gpio, *rec->sets[GPIOMUX_SUSPENDED]);
spin_unlock_irqrestore(&gpiomux_lock, irq_flags);
return 0;
}
@@ -111,6 +113,17 @@ int msm_gpiomux_init(size_t ngpio)
if (!msm_gpiomux_recs)
return -ENOMEM;
+ /* There is no need to zero this memory, as clients will be blindly
+ * installing settings on top of it.
+ */
+ msm_gpiomux_sets = kmalloc(sizeof(struct gpiomux_setting) * ngpio *
+ GPIOMUX_NSETTINGS, GFP_KERNEL);
+ if (!msm_gpiomux_sets) {
+ kfree(msm_gpiomux_recs);
+ msm_gpiomux_recs = NULL;
+ return -ENOMEM;
+ }
+
msm_gpiomux_ngpio = ngpio;
return 0;
@@ -119,18 +132,16 @@ EXPORT_SYMBOL(msm_gpiomux_init);
void msm_gpiomux_install(struct msm_gpiomux_config *configs, unsigned
nconfigs)
{
- unsigned n;
+ unsigned c, s;
int rc;
- if (!msm_gpiomux_recs)
- return;
-
- for (n = 0; n < nconfigs; ++n) {
- rc = msm_gpiomux_write(configs[n].gpio,
- configs[n].active,
- configs[n].suspended);
- if (rc)
- pr_err("%s: write failure: %d\n", __func__, rc);
+ for (c = 0; c < nconfigs; ++c) {
+ for (s = 0; s < GPIOMUX_NSETTINGS; ++s) {
+ rc = msm_gpiomux_write(configs[c].gpio, s,
+ configs[c].settings[s]);
+ if (rc)
+ pr_err("%s: write failure: %d\n", __func__, rc);
+ }
}
}
EXPORT_SYMBOL(msm_gpiomux_install);
diff --git a/arch/arm/mach-msm/gpiomux.h b/arch/arm/mach-msm/gpiomux.h
index 38bf511..bd8d6c2 100644
--- a/arch/arm/mach-msm/gpiomux.h
+++ b/arch/arm/mach-msm/gpiomux.h
@@ -20,44 +20,73 @@
#include <linux/bitops.h>
#include <linux/errno.h>
-#if defined(CONFIG_MSM_V2_TLMM)
-#include "gpiomux-v2.h"
-#else
-#include "gpiomux-v1.h"
-#endif
+enum msm_gpiomux_setting {
+ GPIOMUX_ACTIVE = 0,
+ GPIOMUX_SUSPENDED,
+ GPIOMUX_NSETTINGS
+};
+
+enum gpiomux_drv {
+ GPIOMUX_DRV_2MA = 0,
+ GPIOMUX_DRV_4MA,
+ GPIOMUX_DRV_6MA,
+ GPIOMUX_DRV_8MA,
+ GPIOMUX_DRV_10MA,
+ GPIOMUX_DRV_12MA,
+ GPIOMUX_DRV_14MA,
+ GPIOMUX_DRV_16MA,
+};
+
+enum gpiomux_func {
+ GPIOMUX_FUNC_GPIO = 0,
+ GPIOMUX_FUNC_1,
+ GPIOMUX_FUNC_2,
+ GPIOMUX_FUNC_3,
+ GPIOMUX_FUNC_4,
+ GPIOMUX_FUNC_5,
+ GPIOMUX_FUNC_6,
+ GPIOMUX_FUNC_7,
+ GPIOMUX_FUNC_8,
+ GPIOMUX_FUNC_9,
+ GPIOMUX_FUNC_A,
+ GPIOMUX_FUNC_B,
+ GPIOMUX_FUNC_C,
+ GPIOMUX_FUNC_D,
+ GPIOMUX_FUNC_E,
+ GPIOMUX_FUNC_F,
+};
+
+enum gpiomux_pull {
+ GPIOMUX_PULL_NONE = 0,
+ GPIOMUX_PULL_DOWN,
+ GPIOMUX_PULL_KEEPER,
+ GPIOMUX_PULL_UP,
+};
+
+struct gpiomux_setting {
+ enum gpiomux_func func;
+ enum gpiomux_drv drv;
+ enum gpiomux_pull pull;
+};
/**
* struct msm_gpiomux_config: gpiomux settings for one gpio line.
*
- * A complete gpiomux config is the bitwise-or of a drive-strength,
+ * A complete gpiomux setting is the combination of a drive-strength,
* function, and pull. For functions other than GPIO, the OE
* is hard-wired according to the function. For GPIO mode,
* OE is controlled by gpiolib.
*
- * Available settings differ by target; see the gpiomux header
- * specific to your target arch for available configurations.
- *
* @gpio: The index number of the gpio being described.
- * @active: The configuration to be installed when the line is
- * active, or its reference count is > 0.
- * @suspended: The configuration to be installed when the line
- * is suspended, or its reference count is 0.
+ * @settings: The settings to be installed, specifically:
+ * GPIOMUX_ACTIVE: The setting to be installed when the
+ * line is active, or its reference count is > 0.
+ * GPIOMUX_SUSPENDED: The setting to be installed when
+ * the line is suspended, or its reference count is 0.
*/
struct msm_gpiomux_config {
- unsigned gpio;
- gpiomux_config_t active;
- gpiomux_config_t suspended;
-};
-
-/**
- * @GPIOMUX_VALID: If set, the config field contains 'good data'.
- * The absence of this bit will prevent the gpiomux
- * system from applying the configuration under all
- * circumstances.
- */
-enum {
- GPIOMUX_VALID = BIT(sizeof(gpiomux_config_t) * BITS_PER_BYTE - 1),
- GPIOMUX_CTL_MASK = GPIOMUX_VALID,
+ unsigned gpio;
+ struct gpiomux_setting *settings[GPIOMUX_NSETTINGS];
};
#ifdef CONFIG_MSM_GPIOMUX
@@ -79,12 +108,10 @@ int __must_check msm_gpiomux_get(unsigned gpio);
/* Decrement a gpio's reference count, possibly suspending the line. */
int msm_gpiomux_put(unsigned gpio);
-/* Install a new configuration to the gpio line. To avoid overwriting
- * a configuration, leave the VALID bit out.
+/* Install a new setting in a gpio. To erase a slot, use NULL.
*/
-int msm_gpiomux_write(unsigned gpio,
- gpiomux_config_t active,
- gpiomux_config_t suspended);
+int msm_gpiomux_write(unsigned gpio, enum msm_gpiomux_setting which,
+ struct gpiomux_setting *setting);
/* Architecture-internal function for use by the framework only.
* This function can assume the following:
@@ -94,7 +121,7 @@ int msm_gpiomux_write(unsigned gpio,
* This function is not for public consumption. External users
* should use msm_gpiomux_write.
*/
-void __msm_gpiomux_write(unsigned gpio, gpiomux_config_t val);
+void __msm_gpiomux_write(unsigned gpio, struct gpiomux_setting val);
#else
static inline int msm_gpiomux_init(size_t ngpio)
{
@@ -115,8 +142,7 @@ static inline int msm_gpiomux_put(unsigned gpio)
}
static inline int msm_gpiomux_write(unsigned gpio,
- gpiomux_config_t active,
- gpiomux_config_t suspended)
+ enum msm_gpiomux_setting which, struct gpiomux_setting *setting)
{
return -ENOSYS;
}
--
1.7.3.3
Thanks,
Rohit Vaswani
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
On Fri, Feb 25, 2011 at 12:19 PM, Rohit Vaswani <[email protected]> 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 <[email protected]>
> ---
> ?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
>
> diff --git a/arch/arm/mach-msm/gpiomux-v1.c b/arch/arm/mach-msm/gpiomux-v1.c
> index 27de2ab..a9525c2 100644
> --- a/arch/arm/mach-msm/gpiomux-v1.c
> +++ b/arch/arm/mach-msm/gpiomux-v1.c
> @@ -18,13 +18,16 @@
> ?#include "gpiomux.h"
> ?#include "proc_comm.h"
>
> -void __msm_gpiomux_write(unsigned gpio, gpiomux_config_t val)
> +void __msm_gpiomux_write(unsigned gpio, struct gpiomux_setting val)
> ?{
> - ? ?unsigned tlmm_config ?= (val & ~GPIOMUX_CTL_MASK) |
> - ? ? ? ? ? ? ? ?((gpio & 0x3ff) << 4);
> + ? ?unsigned tlmm_config;
> ? ? unsigned tlmm_disable = 0;
> ? ? int rc;
>
> + ? ?tlmm_config ?= (val.drv << 17) |
> + ? ? ? ?(val.pull << 15) |
> + ? ? ? ?((gpio & 0x3ff) << 4) |
> + ? ? ? ?val.func;
> ? ? rc = msm_proc_comm(PCOM_RPC_GPIO_TLMM_CONFIG_EX,
> &tlmm_config, &tlmm_disable);
> ? ? if (rc)
> diff --git a/arch/arm/mach-msm/gpiomux-v1.h b/arch/arm/mach-msm/gpiomux-v1.h
> deleted file mode 100644
> index 96ad5fa..0000000
> --- a/arch/arm/mach-msm/gpiomux-v1.h
> +++ /dev/null
> @@ -1,59 +0,0 @@
> -/* Copyright (c) 2010, Code Aurora Forum. All rights reserved.
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License version 2 and
> - * only version 2 as published by the Free Software Foundation.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. ?See the
> - * GNU General Public License for more details.
> - *
> - * You should have received a copy of the GNU General Public License
> - * along with this program; if not, write to the Free Software
> - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> - * 02110-1301, USA.
> - */
> -#ifndef __ARCH_ARM_MACH_MSM_GPIOMUX_V1_H
> -#define __ARCH_ARM_MACH_MSM_GPIOMUX_V1_H
> -
> -typedef u32 gpiomux_config_t;
> -
> -enum {
> - ? ?GPIOMUX_DRV_2MA ?= 0UL << 17,
> - ? ?GPIOMUX_DRV_4MA ?= 1UL << 17,
> - ? ?GPIOMUX_DRV_6MA ?= 2UL << 17,
> - ? ?GPIOMUX_DRV_8MA ?= 3UL << 17,
> - ? ?GPIOMUX_DRV_10MA = 4UL << 17,
> - ? ?GPIOMUX_DRV_12MA = 5UL << 17,
> - ? ?GPIOMUX_DRV_14MA = 6UL << 17,
> - ? ?GPIOMUX_DRV_16MA = 7UL << 17,
> -};
> -
> -enum {
> - ? ?GPIOMUX_FUNC_GPIO = 0UL,
> - ? ?GPIOMUX_FUNC_1 ? ?= 1UL,
> - ? ?GPIOMUX_FUNC_2 ? ?= 2UL,
> - ? ?GPIOMUX_FUNC_3 ? ?= 3UL,
> - ? ?GPIOMUX_FUNC_4 ? ?= 4UL,
> - ? ?GPIOMUX_FUNC_5 ? ?= 5UL,
> - ? ?GPIOMUX_FUNC_6 ? ?= 6UL,
> - ? ?GPIOMUX_FUNC_7 ? ?= 7UL,
> - ? ?GPIOMUX_FUNC_8 ? ?= 8UL,
> - ? ?GPIOMUX_FUNC_9 ? ?= 9UL,
> - ? ?GPIOMUX_FUNC_A ? ?= 10UL,
> - ? ?GPIOMUX_FUNC_B ? ?= 11UL,
> - ? ?GPIOMUX_FUNC_C ? ?= 12UL,
> - ? ?GPIOMUX_FUNC_D ? ?= 13UL,
> - ? ?GPIOMUX_FUNC_E ? ?= 14UL,
> - ? ?GPIOMUX_FUNC_F ? ?= 15UL,
> -};
> -
> -enum {
> - ? ?GPIOMUX_PULL_NONE ? = 0UL << 15,
> - ? ?GPIOMUX_PULL_DOWN ? = 1UL << 15,
> - ? ?GPIOMUX_PULL_KEEPER = 2UL << 15,
> - ? ?GPIOMUX_PULL_UP ? ? = 3UL << 15,
> -};
> -
> -#endif
> diff --git a/arch/arm/mach-msm/gpiomux-v2.c b/arch/arm/mach-msm/gpiomux-v2.c
> index 273396d..0a04d60 100644
> --- a/arch/arm/mach-msm/gpiomux-v2.c
> +++ b/arch/arm/mach-msm/gpiomux-v2.c
> @@ -18,8 +18,10 @@
> ?#include <mach/msm_iomap.h>
> ?#include "gpiomux.h"
>
> -void __msm_gpiomux_write(unsigned gpio, gpiomux_config_t val)
> +void __msm_gpiomux_write(unsigned gpio, struct gpiomux_setting val)
> ?{
> - ? ?writel(val & ~GPIOMUX_CTL_MASK,
> - ? ? ? ? ? MSM_TLMM_BASE + 0x1000 + (0x10 * gpio));
> + ? ?uint32_t bits;
> +
> + ? ?bits = (val.drv << 6) | (val.func << 2) | val.pull;
> + ? ?writel(bits, MSM_TLMM_BASE + 0x1000 + (0x10 * gpio));
> ?}
> diff --git a/arch/arm/mach-msm/gpiomux-v2.h b/arch/arm/mach-msm/gpiomux-v2.h
> deleted file mode 100644
> index a7dec1ea..0000000
> --- a/arch/arm/mach-msm/gpiomux-v2.h
> +++ /dev/null
> @@ -1,59 +0,0 @@
> -/* Copyright (c) 2010, Code Aurora Forum. All rights reserved.
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License version 2 and
> - * only version 2 as published by the Free Software Foundation.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. ?See the
> - * GNU General Public License for more details.
> - *
> - * You should have received a copy of the GNU General Public License
> - * along with this program; if not, write to the Free Software
> - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> - * 02110-1301, USA.
> - */
> -#ifndef __ARCH_ARM_MACH_MSM_GPIOMUX_V2_H
> -#define __ARCH_ARM_MACH_MSM_GPIOMUX_V2_H
> -
> -typedef u16 gpiomux_config_t;
> -
> -enum {
> - ? ?GPIOMUX_DRV_2MA ?= 0UL << 6,
> - ? ?GPIOMUX_DRV_4MA ?= 1UL << 6,
> - ? ?GPIOMUX_DRV_6MA ?= 2UL << 6,
> - ? ?GPIOMUX_DRV_8MA ?= 3UL << 6,
> - ? ?GPIOMUX_DRV_10MA = 4UL << 6,
> - ? ?GPIOMUX_DRV_12MA = 5UL << 6,
> - ? ?GPIOMUX_DRV_14MA = 6UL << 6,
> - ? ?GPIOMUX_DRV_16MA = 7UL << 6,
> -};
> -
> -enum {
> - ? ?GPIOMUX_FUNC_GPIO = 0UL << 2,
> - ? ?GPIOMUX_FUNC_1 ? ?= 1UL << 2,
> - ? ?GPIOMUX_FUNC_2 ? ?= 2UL << 2,
> - ? ?GPIOMUX_FUNC_3 ? ?= 3UL << 2,
> - ? ?GPIOMUX_FUNC_4 ? ?= 4UL << 2,
> - ? ?GPIOMUX_FUNC_5 ? ?= 5UL << 2,
> - ? ?GPIOMUX_FUNC_6 ? ?= 6UL << 2,
> - ? ?GPIOMUX_FUNC_7 ? ?= 7UL << 2,
> - ? ?GPIOMUX_FUNC_8 ? ?= 8UL << 2,
> - ? ?GPIOMUX_FUNC_9 ? ?= 9UL << 2,
> - ? ?GPIOMUX_FUNC_A ? ?= 10UL << 2,
> - ? ?GPIOMUX_FUNC_B ? ?= 11UL << 2,
> - ? ?GPIOMUX_FUNC_C ? ?= 12UL << 2,
> - ? ?GPIOMUX_FUNC_D ? ?= 13UL << 2,
> - ? ?GPIOMUX_FUNC_E ? ?= 14UL << 2,
> - ? ?GPIOMUX_FUNC_F ? ?= 15UL << 2,
> -};
> -
> -enum {
> - ? ?GPIOMUX_PULL_NONE ? = 0UL,
> - ? ?GPIOMUX_PULL_DOWN ? = 1UL,
> - ? ?GPIOMUX_PULL_KEEPER = 2UL,
> - ? ?GPIOMUX_PULL_UP ? ? = 3UL,
> -};
> -
> -#endif
> diff --git a/arch/arm/mach-msm/gpiomux.c b/arch/arm/mach-msm/gpiomux.c
> index 9ef9864..1ca26ec 100644
> --- a/arch/arm/mach-msm/gpiomux.c
> +++ b/arch/arm/mach-msm/gpiomux.c
> @@ -20,21 +20,21 @@
> ?#include "gpiomux.h"
>
> ?struct msm_gpiomux_rec {
> - ? ?gpiomux_config_t active;
> - ? ?gpiomux_config_t suspended;
> - ? ?int ? ? ? ? ? ? ?ref;
> + ? ?struct gpiomux_setting *sets[GPIOMUX_NSETTINGS];
> + ? ?int ref;
> ?};
> ?static DEFINE_SPINLOCK(gpiomux_lock);
> ?static struct msm_gpiomux_rec *msm_gpiomux_recs;
> +static struct gpiomux_setting *msm_gpiomux_sets;
> ?static unsigned msm_gpiomux_ngpio;
>
> -int msm_gpiomux_write(unsigned gpio,
> - ? ? ? ? ? ? ?gpiomux_config_t active,
> - ? ? ? ? ? ? ?gpiomux_config_t suspended)
> +int msm_gpiomux_write(unsigned gpio, enum msm_gpiomux_setting which,
> + ? ?struct gpiomux_setting *setting)
> ?{
> - ? ?struct msm_gpiomux_rec *cfg = msm_gpiomux_recs + gpio;
> + ? ?struct msm_gpiomux_rec *rec = msm_gpiomux_recs + gpio;
> + ? ?unsigned set_slot = gpio * GPIOMUX_NSETTINGS + which;
> ? ? unsigned long irq_flags;
> - ? ?gpiomux_config_t setting;
> + ? ?struct gpiomux_setting *new_set;
>
> ? ? if (!msm_gpiomux_recs)
> ? ? ? ? return -EFAULT;
> @@ -44,15 +44,17 @@ int msm_gpiomux_write(unsigned gpio,
>
> ? ? spin_lock_irqsave(&gpiomux_lock, irq_flags);
>
> - ? ?if (active & GPIOMUX_VALID)
> - ? ? ? ?cfg->active = active;
> -
> - ? ?if (suspended & GPIOMUX_VALID)
> - ? ? ? ?cfg->suspended = suspended;
> + ? ?if (setting) {
> + ? ? ? ?msm_gpiomux_sets[set_slot] = *setting;
> + ? ? ? ?rec->sets[which] = &msm_gpiomux_sets[set_slot];
> + ? ?} else {
> + ? ? ? ?rec->sets[which] = NULL;
> + ? ?}
>
> - ? ?setting = cfg->ref ? active : suspended;
> - ? ?if (setting & GPIOMUX_VALID)
> - ? ? ? ?__msm_gpiomux_write(gpio, setting);
> + ? ?new_set = rec->ref ? rec->sets[GPIOMUX_ACTIVE] :
> + ? ? ? ?rec->sets[GPIOMUX_SUSPENDED];
> + ? ?if (new_set)
> + ? ? ? ?__msm_gpiomux_write(gpio, *new_set);
>
> ? ? spin_unlock_irqrestore(&gpiomux_lock, irq_flags);
> ? ? return 0;
> @@ -61,7 +63,7 @@ EXPORT_SYMBOL(msm_gpiomux_write);
>
> ?int msm_gpiomux_get(unsigned gpio)
> ?{
> - ? ?struct msm_gpiomux_rec *cfg = msm_gpiomux_recs + gpio;
> + ? ?struct msm_gpiomux_rec *rec = msm_gpiomux_recs + gpio;
> ? ? unsigned long irq_flags;
>
> ? ? if (!msm_gpiomux_recs)
> @@ -71,8 +73,8 @@ int msm_gpiomux_get(unsigned gpio)
> ? ? ? ? return -EINVAL;
>
> ? ? spin_lock_irqsave(&gpiomux_lock, irq_flags);
> - ? ?if (cfg->ref++ == 0 && cfg->active & GPIOMUX_VALID)
> - ? ? ? ?__msm_gpiomux_write(gpio, cfg->active);
> + ? ?if (rec->ref++ == 0 && rec->sets[GPIOMUX_ACTIVE])
> + ? ? ? ?__msm_gpiomux_write(gpio, *rec->sets[GPIOMUX_ACTIVE]);
> ? ? spin_unlock_irqrestore(&gpiomux_lock, irq_flags);
> ? ? return 0;
> ?}
> @@ -80,7 +82,7 @@ EXPORT_SYMBOL(msm_gpiomux_get);
>
> ?int msm_gpiomux_put(unsigned gpio)
> ?{
> - ? ?struct msm_gpiomux_rec *cfg = msm_gpiomux_recs + gpio;
> + ? ?struct msm_gpiomux_rec *rec = msm_gpiomux_recs + gpio;
> ? ? unsigned long irq_flags;
>
> ? ? if (!msm_gpiomux_recs)
> @@ -90,9 +92,9 @@ int msm_gpiomux_put(unsigned gpio)
> ? ? ? ? return -EINVAL;
>
> ? ? spin_lock_irqsave(&gpiomux_lock, irq_flags);
> - ? ?BUG_ON(cfg->ref == 0);
> - ? ?if (--cfg->ref == 0 && cfg->suspended & GPIOMUX_VALID)
> - ? ? ? ?__msm_gpiomux_write(gpio, cfg->suspended);
> + ? ?BUG_ON(rec->ref == 0);
> + ? ?if (--rec->ref == 0 && rec->sets[GPIOMUX_SUSPENDED])
> + ? ? ? ?__msm_gpiomux_write(gpio, *rec->sets[GPIOMUX_SUSPENDED]);
> ? ? spin_unlock_irqrestore(&gpiomux_lock, irq_flags);
> ? ? return 0;
> ?}
> @@ -111,6 +113,17 @@ int msm_gpiomux_init(size_t ngpio)
> ? ? if (!msm_gpiomux_recs)
> ? ? ? ? return -ENOMEM;
>
> + ? ?/* There is no need to zero this memory, as clients will be blindly
> + ? ? * installing settings on top of it.
> + ? ? */
> + ? ?msm_gpiomux_sets = kmalloc(sizeof(struct gpiomux_setting) * ngpio *
> + ? ? ? ?GPIOMUX_NSETTINGS, GFP_KERNEL);
> + ? ?if (!msm_gpiomux_sets) {
> + ? ? ? ?kfree(msm_gpiomux_recs);
> + ? ? ? ?msm_gpiomux_recs = NULL;
> + ? ? ? ?return -ENOMEM;
> + ? ?}
> +
> ? ? msm_gpiomux_ngpio = ngpio;
>
> ? ? return 0;
> @@ -119,18 +132,16 @@ EXPORT_SYMBOL(msm_gpiomux_init);
>
> ?void msm_gpiomux_install(struct msm_gpiomux_config *configs, unsigned
> nconfigs)
> ?{
> - ? ?unsigned n;
> + ? ?unsigned c, s;
> ? ? int rc;
>
> - ? ?if (!msm_gpiomux_recs)
> - ? ? ? ?return;
> -
> - ? ?for (n = 0; n < nconfigs; ++n) {
> - ? ? ? ?rc = msm_gpiomux_write(configs[n].gpio,
> - ? ? ? ? ? ? ? ? ? ? ? configs[n].active,
> - ? ? ? ? ? ? ? ? ? ? ? configs[n].suspended);
> - ? ? ? ?if (rc)
> - ? ? ? ? ? ?pr_err("%s: write failure: %d\n", __func__, rc);
> + ? ?for (c = 0; c < nconfigs; ++c) {
> + ? ? ? ?for (s = 0; s < GPIOMUX_NSETTINGS; ++s) {
> + ? ? ? ? ? ?rc = msm_gpiomux_write(configs[c].gpio, s,
> + ? ? ? ? ? ? ? ?configs[c].settings[s]);
> + ? ? ? ? ? ?if (rc)
> + ? ? ? ? ? ? ? ?pr_err("%s: write failure: %d\n", __func__, rc);
> + ? ? ? ?}
> ? ? }
> ?}
> ?EXPORT_SYMBOL(msm_gpiomux_install);
> diff --git a/arch/arm/mach-msm/gpiomux.h b/arch/arm/mach-msm/gpiomux.h
> index 38bf511..bd8d6c2 100644
> --- a/arch/arm/mach-msm/gpiomux.h
> +++ b/arch/arm/mach-msm/gpiomux.h
> @@ -20,44 +20,73 @@
> ?#include <linux/bitops.h>
> ?#include <linux/errno.h>
>
> -#if defined(CONFIG_MSM_V2_TLMM)
> -#include "gpiomux-v2.h"
> -#else
> -#include "gpiomux-v1.h"
> -#endif
> +enum msm_gpiomux_setting {
> + ? ?GPIOMUX_ACTIVE = 0,
> + ? ?GPIOMUX_SUSPENDED,
> + ? ?GPIOMUX_NSETTINGS
> +};
> +
> +enum gpiomux_drv {
> + ? ?GPIOMUX_DRV_2MA = 0,
> + ? ?GPIOMUX_DRV_4MA,
> + ? ?GPIOMUX_DRV_6MA,
> + ? ?GPIOMUX_DRV_8MA,
> + ? ?GPIOMUX_DRV_10MA,
> + ? ?GPIOMUX_DRV_12MA,
> + ? ?GPIOMUX_DRV_14MA,
> + ? ?GPIOMUX_DRV_16MA,
> +};
> +
> +enum gpiomux_func {
> + ? ?GPIOMUX_FUNC_GPIO = 0,
> + ? ?GPIOMUX_FUNC_1,
> + ? ?GPIOMUX_FUNC_2,
> + ? ?GPIOMUX_FUNC_3,
> + ? ?GPIOMUX_FUNC_4,
> + ? ?GPIOMUX_FUNC_5,
> + ? ?GPIOMUX_FUNC_6,
> + ? ?GPIOMUX_FUNC_7,
> + ? ?GPIOMUX_FUNC_8,
> + ? ?GPIOMUX_FUNC_9,
> + ? ?GPIOMUX_FUNC_A,
> + ? ?GPIOMUX_FUNC_B,
> + ? ?GPIOMUX_FUNC_C,
> + ? ?GPIOMUX_FUNC_D,
> + ? ?GPIOMUX_FUNC_E,
> + ? ?GPIOMUX_FUNC_F,
> +};
> +
> +enum gpiomux_pull {
> + ? ?GPIOMUX_PULL_NONE = 0,
> + ? ?GPIOMUX_PULL_DOWN,
> + ? ?GPIOMUX_PULL_KEEPER,
> + ? ?GPIOMUX_PULL_UP,
> +};
> +
> +struct gpiomux_setting {
> + ? ?enum gpiomux_func func;
> + ? ?enum gpiomux_drv ?drv;
> + ? ?enum gpiomux_pull pull;
> +};
>
> ?/**
> ?* struct msm_gpiomux_config: gpiomux settings for one gpio line.
> ?*
> - * A complete gpiomux config is the bitwise-or of a drive-strength,
> + * A complete gpiomux setting is the combination of a drive-strength,
> ?* function, and pull. ?For functions other than GPIO, the OE
> ?* is hard-wired according to the function. ?For GPIO mode,
> ?* OE is controlled by gpiolib.
> ?*
> - * Available settings differ by target; see the gpiomux header
> - * specific to your target arch for available configurations.
> - *
> ?* @gpio: The index number of the gpio being described.
> - * @active: The configuration to be installed when the line is
> - * active, or its reference count is > 0.
> - * @suspended: The configuration to be installed when the line
> - * is suspended, or its reference count is 0.
> + * @settings: The settings to be installed, specifically:
> + * ? ? ? ? ? GPIOMUX_ACTIVE: The setting to be installed when the
> + * ? ? ? ? ? line is active, or its reference count is > 0.
> + * ? ? ? ? ? GPIOMUX_SUSPENDED: The setting to be installed when
> + * ? ? ? ? ? the line is suspended, or its reference count is 0.
> ?*/
> ?struct msm_gpiomux_config {
> - ? ?unsigned ? ? ? ? gpio;
> - ? ?gpiomux_config_t active;
> - ? ?gpiomux_config_t suspended;
> -};
> -
> -/**
> - * @GPIOMUX_VALID: ? ?If set, the config field contains 'good data'.
> - * ? ? ? ? ? ? ? ? ? ? ?The absence of this bit will prevent the gpiomux
> - * ? ? ? ? ? ?system from applying the configuration under all
> - * ? ? ? ? ? ?circumstances.
> - */
> -enum {
> - ? ?GPIOMUX_VALID ? ? = BIT(sizeof(gpiomux_config_t) * BITS_PER_BYTE - 1),
> - ? ?GPIOMUX_CTL_MASK = GPIOMUX_VALID,
> + ? ?unsigned gpio;
> + ? ?struct gpiomux_setting *settings[GPIOMUX_NSETTINGS];
> ?};
>
> ?#ifdef CONFIG_MSM_GPIOMUX
> @@ -79,12 +108,10 @@ int __must_check msm_gpiomux_get(unsigned gpio);
> ?/* Decrement a gpio's reference count, possibly suspending the line. */
> ?int msm_gpiomux_put(unsigned gpio);
>
> -/* Install a new configuration to the gpio line. ?To avoid overwriting
> - * a configuration, leave the VALID bit out.
> +/* Install a new setting in a gpio. ?To erase a slot, use NULL.
> ?*/
> -int msm_gpiomux_write(unsigned gpio,
> - ? ? ? ? ? ? ?gpiomux_config_t active,
> - ? ? ? ? ? ? ?gpiomux_config_t suspended);
> +int msm_gpiomux_write(unsigned gpio, enum msm_gpiomux_setting which,
> + ? ?struct gpiomux_setting *setting);
>
> ?/* Architecture-internal function for use by the framework only.
> ?* This function can assume the following:
> @@ -94,7 +121,7 @@ int msm_gpiomux_write(unsigned gpio,
> ?* This function is not for public consumption. ?External users
> ?* should use msm_gpiomux_write.
> ?*/
> -void __msm_gpiomux_write(unsigned gpio, gpiomux_config_t val);
> +void __msm_gpiomux_write(unsigned gpio, struct gpiomux_setting val);
> ?#else
> ?static inline int msm_gpiomux_init(size_t ngpio)
> ?{
> @@ -115,8 +142,7 @@ static inline int msm_gpiomux_put(unsigned gpio)
> ?}
>
> ?static inline int msm_gpiomux_write(unsigned gpio,
> - ? ? ? ? ? ? ? ? ? ?gpiomux_config_t active,
> - ? ? ? ? ? ? ? ? ? ?gpiomux_config_t suspended)
> + ? ?enum msm_gpiomux_setting which, struct gpiomux_setting *setting)
> ?{
> ? ? return -ENOSYS;
> ?}
> --
> 1.7.3.3
>
>
> Thanks,
> Rohit Vaswani
>
> --
> Sent by an employee of the Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
>
>
On 2/25/2011 5:20 PM, Dima Zavin wrote:
> On Fri, Feb 25, 2011 at 12:19 PM, Rohit Vaswani<[email protected]> 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<[email protected]>
>> ---
>> 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
>
Thanks for you feedback Dima. I agree that the above change makes the
pin configurations verbose - but its neither too verbose to so as to
become cumbersome to use nor typically unmanageable. We did think about
adding a valid/invalid bool to the configurations - but the fact was
that they it simply did not belong there. The gpiomux configurations are
never "invalid" as such. Another important part to this decision was
that - with adding another variable as a part of the gpiomux
configurations meant that we were transferring responsibility of
maintaining something that belongs to the "gpiomux internals" to the
user. We felt that having the user to append a valid/invalid flag with
every configuration to was not a step in the right direction.
On the contrary, the explicit structures improved readability and kept
things pretty simple. Thinking about it more - it doesn't even make it
verbose - but it just may seem that way because we expand it into a
structure instead of hiding behind a macro.
Also, we currently use this same design for a number of GPIOs far
greater than 24 and my/our experience has never been bothersome.
Hope that answers your query.
Thanks,
Rohit Vaswani
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
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.
Good point, ptr to anon struct would work.
I would still very much like to see the helper macros be added though
as it really would be very helpful in keeping the pinmuxing tidy in
the board files. Doing it as a separate patch is fine.
Other than the addition of helper macros, ack.
--Dima
On Fri, Feb 25, 2011 at 9:40 PM, Saravana Kannan <[email protected]> wrote:
> 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.
>
On 03/01/2011 06:25 PM, Dima Zavin wrote:
> Good point, ptr to anon struct would work.
Glad to know the suggestion helped.
> I would still very much like to see the helper macros be added though
> as it really would be very helpful in keeping the pinmuxing tidy in
> the board files. Doing it as a separate patch is fine.
>
> Other than the addition of helper macros, ack.
Rohit, would you mind adding the helper macros that use pointer to anon
struct to simplify the GPIO config population in the board files? You
can contact me offline if you need any help with it.
Thanks,
Saravana
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.