v4:
for gpio-msm-v2 patch
* Made summary_irq and tlmm_base part of msm_gpio_dev
* Fixed the bitmap allocation
* Fixed some compile issues with non-ARM
Rohit Vaswani (3):
ARM: msm: Remove gpiomux-v2 and re-organize MSM_GPIOMUX configs
ARM: msm: Remove unused and unmapped MSM_TLMM_BASE for 8x60
gpio: msm: Add device tree and irqdomain support for gpio-msm-v2
.../devicetree/bindings/gpio/gpio-msm.txt | 26 +++
arch/arm/boot/dts/msm8660-surf.dts | 11 +
arch/arm/boot/dts/msm8960-cdp.dts | 11 +
arch/arm/mach-msm/Kconfig | 13 +-
arch/arm/mach-msm/Makefile | 6 +-
arch/arm/mach-msm/gpiomux-8x60.c | 19 --
arch/arm/mach-msm/gpiomux-v2.c | 25 ---
arch/arm/mach-msm/gpiomux-v2.h | 61 ------
arch/arm/mach-msm/gpiomux.c | 15 ++
arch/arm/mach-msm/gpiomux.h | 5 -
arch/arm/mach-msm/include/mach/msm_iomap-8x60.h | 4 -
drivers/gpio/Kconfig | 2 +-
drivers/gpio/gpio-msm-v2.c | 199 ++++++++++++--------
13 files changed, 189 insertions(+), 208 deletions(-)
create mode 100644 Documentation/devicetree/bindings/gpio/gpio-msm.txt
delete mode 100644 arch/arm/mach-msm/gpiomux-8x60.c
delete mode 100644 arch/arm/mach-msm/gpiomux-v2.c
delete mode 100644 arch/arm/mach-msm/gpiomux-v2.h
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
Remove gpiomux-v2 as it's not being used and make way for future improvements.
Signed-off-by: Rohit Vaswani <[email protected]>
---
arch/arm/mach-msm/Kconfig | 13 +++-----
arch/arm/mach-msm/Makefile | 6 +--
arch/arm/mach-msm/gpiomux-8x60.c | 19 ------------
arch/arm/mach-msm/gpiomux-v2.c | 25 ---------------
arch/arm/mach-msm/gpiomux-v2.h | 61 --------------------------------------
arch/arm/mach-msm/gpiomux.c | 15 +++++++++
arch/arm/mach-msm/gpiomux.h | 5 ---
drivers/gpio/gpio-msm-v2.c | 5 +--
8 files changed, 24 insertions(+), 125 deletions(-)
delete mode 100644 arch/arm/mach-msm/gpiomux-8x60.c
delete mode 100644 arch/arm/mach-msm/gpiomux-v2.c
delete mode 100644 arch/arm/mach-msm/gpiomux-v2.h
diff --git a/arch/arm/mach-msm/Kconfig b/arch/arm/mach-msm/Kconfig
index fceb093..614e41e 100644
--- a/arch/arm/mach-msm/Kconfig
+++ b/arch/arm/mach-msm/Kconfig
@@ -48,9 +48,7 @@ config ARCH_MSM8X60
select CPU_V7
select GPIO_MSM_V2
select HAVE_SMP
- select MSM_GPIOMUX
select MSM_SCM if SMP
- select MSM_V2_TLMM
select USE_OF
config ARCH_MSM8960
@@ -58,9 +56,8 @@ config ARCH_MSM8960
select ARM_GIC
select CPU_V7
select HAVE_SMP
- select MSM_GPIOMUX
+ select GPIO_MSM_V2
select MSM_SCM if SMP
- select MSM_V2_TLMM
select USE_OF
config MSM_HAS_DEBUG_UART_HS
@@ -124,10 +121,10 @@ config MSM_SMD
bool
config MSM_GPIOMUX
- bool
-
-config MSM_V2_TLMM
- bool
+ depends on !(ARCH_MSM8X60 || ARCH_MSM8960)
+ bool "MSM V1 TLMM GPIOMUX architecture"
+ help
+ Support for MSM V1 TLMM GPIOMUX architecture.
config MSM_SCM
bool
diff --git a/arch/arm/mach-msm/Makefile b/arch/arm/mach-msm/Makefile
index 17519fa..1a26d04 100644
--- a/arch/arm/mach-msm/Makefile
+++ b/arch/arm/mach-msm/Makefile
@@ -27,7 +27,5 @@ obj-$(CONFIG_ARCH_MSM7X30) += board-msm7x30.o devices-msm7x30.o
obj-$(CONFIG_ARCH_QSD8X50) += board-qsd8x50.o devices-qsd8x50.o
obj-$(CONFIG_ARCH_MSM8X60) += board-dt-8660.o
obj-$(CONFIG_ARCH_MSM8960) += board-dt-8960.o
-
-obj-$(CONFIG_ARCH_MSM7X30) += gpiomux-v1.o gpiomux.o
-obj-$(CONFIG_ARCH_QSD8X50) += gpiomux-8x50.o gpiomux-v1.o gpiomux.o
-obj-$(CONFIG_ARCH_MSM8X60) += gpiomux-8x60.o gpiomux-v2.o gpiomux.o
+obj-$(CONFIG_MSM_GPIOMUX) += gpiomux.o
+obj-$(CONFIG_ARCH_QSD8X50) += gpiomux-8x50.o
diff --git a/arch/arm/mach-msm/gpiomux-8x60.c b/arch/arm/mach-msm/gpiomux-8x60.c
deleted file mode 100644
index 7b380b3..0000000
--- a/arch/arm/mach-msm/gpiomux-8x60.c
+++ /dev/null
@@ -1,19 +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.
- */
-#include "gpiomux.h"
-
-struct msm_gpiomux_config msm_gpiomux_configs[GPIOMUX_NGPIOS] = {};
diff --git a/arch/arm/mach-msm/gpiomux-v2.c b/arch/arm/mach-msm/gpiomux-v2.c
deleted file mode 100644
index 273396d..0000000
--- a/arch/arm/mach-msm/gpiomux-v2.c
+++ /dev/null
@@ -1,25 +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.
- */
-#include <linux/io.h>
-#include <mach/msm_iomap.h>
-#include "gpiomux.h"
-
-void __msm_gpiomux_write(unsigned gpio, gpiomux_config_t val)
-{
- writel(val & ~GPIOMUX_CTL_MASK,
- 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 3bf10e7..0000000
--- a/arch/arm/mach-msm/gpiomux-v2.h
+++ /dev/null
@@ -1,61 +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
-
-#define GPIOMUX_NGPIOS 173
-
-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 53af21a..2b8e2d2 100644
--- a/arch/arm/mach-msm/gpiomux.c
+++ b/arch/arm/mach-msm/gpiomux.c
@@ -17,9 +17,24 @@
#include <linux/module.h>
#include <linux/spinlock.h>
#include "gpiomux.h"
+#include "proc_comm.h"
static DEFINE_SPINLOCK(gpiomux_lock);
+static void __msm_gpiomux_write(unsigned gpio, gpiomux_config_t val)
+{
+ unsigned tlmm_config = (val & ~GPIOMUX_CTL_MASK) |
+ ((gpio & 0x3ff) << 4);
+ unsigned tlmm_disable = 0;
+ int rc;
+
+ rc = msm_proc_comm(PCOM_RPC_GPIO_TLMM_CONFIG_EX,
+ &tlmm_config, &tlmm_disable);
+ if (rc)
+ pr_err("%s: unexpected proc_comm failure %d: %08x %08x\n",
+ __func__, rc, tlmm_config, tlmm_disable);
+}
+
int msm_gpiomux_write(unsigned gpio,
gpiomux_config_t active,
gpiomux_config_t suspended)
diff --git a/arch/arm/mach-msm/gpiomux.h b/arch/arm/mach-msm/gpiomux.h
index 00459f6..8e82f41 100644
--- a/arch/arm/mach-msm/gpiomux.h
+++ b/arch/arm/mach-msm/gpiomux.h
@@ -20,12 +20,7 @@
#include <linux/bitops.h>
#include <linux/errno.h>
#include <mach/msm_gpiomux.h>
-
-#if defined(CONFIG_MSM_V2_TLMM)
-#include "gpiomux-v2.h"
-#else
#include "gpiomux-v1.h"
-#endif
/**
* struct msm_gpiomux_config: gpiomux settings for one gpio line.
diff --git a/drivers/gpio/gpio-msm-v2.c b/drivers/gpio/gpio-msm-v2.c
index dd2edde..75cc821 100644
--- a/drivers/gpio/gpio-msm-v2.c
+++ b/drivers/gpio/gpio-msm-v2.c
@@ -29,7 +29,6 @@
#include <linux/platform_device.h>
#include <linux/spinlock.h>
-#include <mach/msm_gpiomux.h>
#include <mach/msm_iomap.h>
/* Bits of interest in the GPIO_IN_OUT register.
@@ -159,12 +158,12 @@ static int msm_gpio_direction_output(struct gpio_chip *chip,
static int msm_gpio_request(struct gpio_chip *chip, unsigned offset)
{
- return msm_gpiomux_get(chip->base + offset);
+ return 0;
}
static void msm_gpio_free(struct gpio_chip *chip, unsigned offset)
{
- msm_gpiomux_put(chip->base + offset);
+ return;
}
static int msm_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
This cleans up the gpio-msm-v2 driver of all the global define usage.
The number of gpios are now defined in the device tree. This enables
adding irqdomain support as well.
Signed-off-by: Rohit Vaswani <[email protected]>
---
.../devicetree/bindings/gpio/gpio-msm.txt | 26 +++
arch/arm/boot/dts/msm8660-surf.dts | 11 +
arch/arm/boot/dts/msm8960-cdp.dts | 11 +
drivers/gpio/Kconfig | 2 +-
drivers/gpio/gpio-msm-v2.c | 194 ++++++++++++--------
5 files changed, 165 insertions(+), 79 deletions(-)
create mode 100644 Documentation/devicetree/bindings/gpio/gpio-msm.txt
diff --git a/Documentation/devicetree/bindings/gpio/gpio-msm.txt b/Documentation/devicetree/bindings/gpio/gpio-msm.txt
new file mode 100644
index 0000000..ac20e68
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-msm.txt
@@ -0,0 +1,26 @@
+MSM GPIO controller bindings
+
+Required properties:
+- compatible:
+ - "qcom,msm-gpio" for MSM controllers
+- #gpio-cells : Should be two.
+ - first cell is the pin number
+ - second cell is used to specify optional parameters (unused)
+- gpio-controller : Marks the device node as a GPIO controller.
+- #interrupt-cells : Should be 2.
+- interrupt-controller: Mark the device node as an interrupt controller
+- interrupts : Specify the TLMM summary interrupt number
+- ngpio : Specify the number of MSM GPIOs
+
+Example:
+
+ msmgpio: gpio@fd510000 {
+ compatible = "qcom,msm-gpio";
+ gpio-controller;
+ #gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ reg = <0xfd510000 0x4000>;
+ interrupts = <0 208 0>;
+ ngpio = <150>;
+ };
diff --git a/arch/arm/boot/dts/msm8660-surf.dts b/arch/arm/boot/dts/msm8660-surf.dts
index 9bf49b3..8931906 100644
--- a/arch/arm/boot/dts/msm8660-surf.dts
+++ b/arch/arm/boot/dts/msm8660-surf.dts
@@ -26,6 +26,17 @@
cpu-offset = <0x40000>;
};
+ msmgpio: gpio@800000 {
+ compatible = "qcom,msm-gpio";
+ reg = <0x00800000 0x1000>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ ngpio = <173>;
+ interrupts = <0 32 0x4>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
+
serial@19c400000 {
compatible = "qcom,msm-hsuart", "qcom,msm-uart";
reg = <0x19c40000 0x1000>,
diff --git a/arch/arm/boot/dts/msm8960-cdp.dts b/arch/arm/boot/dts/msm8960-cdp.dts
index 2e4d87a..52fe253 100644
--- a/arch/arm/boot/dts/msm8960-cdp.dts
+++ b/arch/arm/boot/dts/msm8960-cdp.dts
@@ -26,6 +26,17 @@
cpu-offset = <0x80000>;
};
+ msmgpio: gpio@fd510000 {
+ compatible = "qcom,msm-gpio";
+ gpio-controller;
+ #gpio-cells = <2>;
+ ngpio = <150>;
+ interrupts = <0 32 0x4>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ reg = <0xfd510000 0x4000>;
+ };
+
serial@19c400000 {
compatible = "qcom,msm-hsuart", "qcom,msm-uart";
reg = <0x16440000 0x1000>,
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 87d5670..6d61a12 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -165,7 +165,7 @@ config GPIO_MSM_V1
config GPIO_MSM_V2
tristate "Qualcomm MSM GPIO v2"
- depends on GPIOLIB && ARCH_MSM
+ depends on GPIOLIB && OF && ARCH_MSM
help
Say yes here to support the GPIO interface on ARM v7 based
Qualcomm MSM chips. Most of the pins on the MSM can be
diff --git a/drivers/gpio/gpio-msm-v2.c b/drivers/gpio/gpio-msm-v2.c
index 75cc821..5a824be 100644
--- a/drivers/gpio/gpio-msm-v2.c
+++ b/drivers/gpio/gpio-msm-v2.c
@@ -19,17 +19,19 @@
#include <linux/bitmap.h>
#include <linux/bitops.h>
+#include <linux/err.h>
#include <linux/gpio.h>
#include <linux/init.h>
#include <linux/interrupt.h>
#include <linux/io.h>
#include <linux/irqchip/chained_irq.h>
#include <linux/irq.h>
+#include <linux/irqdomain.h>
#include <linux/module.h>
+#include <linux/of_address.h>
#include <linux/platform_device.h>
#include <linux/spinlock.h>
-
-#include <mach/msm_iomap.h>
+#include <linux/slab.h>
/* Bits of interest in the GPIO_IN_OUT register.
*/
@@ -76,13 +78,6 @@ enum {
TARGET_PROC_NONE = 7,
};
-
-#define GPIO_INTR_CFG_SU(gpio) (MSM_TLMM_BASE + 0x0400 + (0x04 * (gpio)))
-#define GPIO_CONFIG(gpio) (MSM_TLMM_BASE + 0x1000 + (0x10 * (gpio)))
-#define GPIO_IN_OUT(gpio) (MSM_TLMM_BASE + 0x1004 + (0x10 * (gpio)))
-#define GPIO_INTR_CFG(gpio) (MSM_TLMM_BASE + 0x1008 + (0x10 * (gpio)))
-#define GPIO_INTR_STATUS(gpio) (MSM_TLMM_BASE + 0x100c + (0x10 * (gpio)))
-
/**
* struct msm_gpio_dev: the MSM8660 SoC GPIO device structure
*
@@ -101,11 +96,27 @@ enum {
*/
struct msm_gpio_dev {
struct gpio_chip gpio_chip;
- DECLARE_BITMAP(enabled_irqs, NR_GPIO_IRQS);
- DECLARE_BITMAP(wake_irqs, NR_GPIO_IRQS);
- DECLARE_BITMAP(dual_edge_irqs, NR_GPIO_IRQS);
+ unsigned long *enabled_irqs;
+ unsigned long *wake_irqs;
+ unsigned long *dual_edge_irqs;
+ struct irq_domain *domain;
+ unsigned int summary_irq;
+ void __iomem *msm_tlmm_base;
};
+struct msm_gpio_dev msm_gpio;
+
+#define GPIO_INTR_CFG_SU(gpio) (msm_gpio.msm_tlmm_base + 0x0400 + \
+ (0x04 * (gpio)))
+#define GPIO_CONFIG(gpio) (msm_gpio.msm_tlmm_base + 0x1000 + \
+ (0x10 * (gpio)))
+#define GPIO_IN_OUT(gpio) (msm_gpio.msm_tlmm_base + 0x1004 + \
+ (0x10 * (gpio)))
+#define GPIO_INTR_CFG(gpio) (msm_gpio.msm_tlmm_base + 0x1008 + \
+ (0x10 * (gpio)))
+#define GPIO_INTR_STATUS(gpio) (msm_gpio.msm_tlmm_base + 0x100c + \
+ (0x10 * (gpio)))
+
static DEFINE_SPINLOCK(tlmm_lock);
static inline struct msm_gpio_dev *to_msm_gpio_dev(struct gpio_chip *chip)
@@ -168,27 +179,19 @@ static void msm_gpio_free(struct gpio_chip *chip, unsigned offset)
static int msm_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
{
- return MSM_GPIO_TO_INT(chip->base + offset);
+ struct msm_gpio_dev *g_dev = to_msm_gpio_dev(chip);
+ struct irq_domain *domain = g_dev->domain;
+
+ return irq_create_mapping(domain, offset);
}
static inline int msm_irq_to_gpio(struct gpio_chip *chip, unsigned irq)
{
- return irq - MSM_GPIO_TO_INT(chip->base);
+ struct irq_data *irq_data = irq_get_irq_data(irq);
+
+ return irq_data->hwirq;
}
-static struct msm_gpio_dev msm_gpio = {
- .gpio_chip = {
- .base = 0,
- .ngpio = NR_GPIO_IRQS,
- .direction_input = msm_gpio_direction_input,
- .direction_output = msm_gpio_direction_output,
- .get = msm_gpio_get,
- .set = msm_gpio_set,
- .to_irq = msm_gpio_to_irq,
- .request = msm_gpio_request,
- .free = msm_gpio_free,
- },
-};
/* For dual-edge interrupts in software, since the hardware has no
* such support:
@@ -226,9 +229,9 @@ static void msm_gpio_update_dual_edge_pos(unsigned gpio)
if (intstat || val == val2)
return;
} while (loop_limit-- > 0);
- pr_err("dual-edge irq failed to stabilize, "
+ pr_err("%s: dual-edge irq failed to stabilize, "
"interrupts dropped. %#08x != %#08x\n",
- val, val2);
+ __func__, val, val2);
}
static void msm_gpio_irq_ack(struct irq_data *d)
@@ -312,13 +315,14 @@ static void msm_summary_irq_handler(unsigned int irq, struct irq_desc *desc)
{
unsigned long i;
struct irq_chip *chip = irq_desc_get_chip(desc);
+ int ngpio = msm_gpio.gpio_chip.ngpio;
chained_irq_enter(chip, desc);
- for_each_set_bit(i, msm_gpio.enabled_irqs, NR_GPIO_IRQS) {
+ for_each_set_bit(i, msm_gpio.enabled_irqs, ngpio) {
if (readl(GPIO_INTR_STATUS(i)) & BIT(INTR_STATUS))
- generic_handle_irq(msm_gpio_to_irq(&msm_gpio.gpio_chip,
- i));
+ generic_handle_irq(irq_find_mapping(msm_gpio.domain,
+ i));
}
chained_irq_exit(chip, desc);
@@ -327,15 +331,16 @@ static void msm_summary_irq_handler(unsigned int irq, struct irq_desc *desc)
static int msm_gpio_irq_set_wake(struct irq_data *d, unsigned int on)
{
int gpio = msm_irq_to_gpio(&msm_gpio.gpio_chip, d->irq);
+ int ngpio = msm_gpio.gpio_chip.ngpio;
if (on) {
- if (bitmap_empty(msm_gpio.wake_irqs, NR_GPIO_IRQS))
- irq_set_irq_wake(TLMM_SCSS_SUMMARY_IRQ, 1);
+ if (bitmap_empty(msm_gpio.wake_irqs, ngpio))
+ irq_set_irq_wake(msm_gpio.summary_irq, 1);
set_bit(gpio, msm_gpio.wake_irqs);
} else {
clear_bit(gpio, msm_gpio.wake_irqs);
- if (bitmap_empty(msm_gpio.wake_irqs, NR_GPIO_IRQS))
- irq_set_irq_wake(TLMM_SCSS_SUMMARY_IRQ, 0);
+ if (bitmap_empty(msm_gpio.wake_irqs, ngpio))
+ irq_set_irq_wake(msm_gpio.summary_irq, 0);
}
return 0;
@@ -350,30 +355,88 @@ static struct irq_chip msm_gpio_irq_chip = {
.irq_set_wake = msm_gpio_irq_set_wake,
};
-static int msm_gpio_probe(struct platform_device *dev)
+static struct lock_class_key msm_gpio_lock_class;
+
+static int msm_gpio_irq_domain_map(struct irq_domain *d, unsigned int irq,
+ irq_hw_number_t hwirq)
{
- int i, irq, ret;
+ irq_set_lockdep_class(irq, &msm_gpio_lock_class);
+ irq_set_chip_and_handler(irq, &msm_gpio_irq_chip,
+ handle_level_irq);
+ set_irq_flags(irq, IRQF_VALID);
+
+ return 0;
+}
+
+static const struct irq_domain_ops msm_gpio_irq_domain_ops = {
+ .xlate = irq_domain_xlate_twocell,
+ .map = msm_gpio_irq_domain_map,
+};
+
+static int msm_gpio_probe(struct platform_device *pdev)
+{
+ int ret, ngpio;
+ struct resource *res;
+
+ msm_gpio.gpio_chip.label = pdev->name;
+ msm_gpio.gpio_chip.dev = &pdev->dev;
+ if (!of_property_read_u32(pdev->dev.of_node, "ngpio", &ngpio)) {
+ dev_err(&pdev->dev, "%s: ngpio property missing\n", __func__);
+ return -EINVAL;
+ }
+
+ msm_gpio.gpio_chip.ngpio = ngpio;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ msm_gpio.msm_tlmm_base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(msm_gpio.msm_tlmm_base))
+ return PTR_ERR(msm_gpio.msm_tlmm_base);
+
+ msm_gpio.enabled_irqs = devm_kzalloc(&pdev->dev,
+ sizeof(unsigned long) *
+ BITS_TO_LONGS(ngpio) * 3,
+ GFP_KERNEL);
+ msm_gpio.wake_irqs = &msm_gpio.enabled_irqs[BITS_TO_LONGS(ngpio)];
+ msm_gpio.dual_edge_irqs =
+ &msm_gpio.enabled_irqs[BITS_TO_LONGS(ngpio * 2)];
+
+ msm_gpio.gpio_chip.base = 0;
+ msm_gpio.gpio_chip.direction_input = msm_gpio_direction_input;
+ msm_gpio.gpio_chip.direction_output = msm_gpio_direction_output;
+ msm_gpio.gpio_chip.get = msm_gpio_get;
+ msm_gpio.gpio_chip.set = msm_gpio_set;
+ msm_gpio.gpio_chip.to_irq = msm_gpio_to_irq;
+ msm_gpio.gpio_chip.request = msm_gpio_request;
+ msm_gpio.gpio_chip.free = msm_gpio_free;
- bitmap_zero(msm_gpio.enabled_irqs, NR_GPIO_IRQS);
- bitmap_zero(msm_gpio.wake_irqs, NR_GPIO_IRQS);
- bitmap_zero(msm_gpio.dual_edge_irqs, NR_GPIO_IRQS);
- msm_gpio.gpio_chip.label = dev->name;
ret = gpiochip_add(&msm_gpio.gpio_chip);
- if (ret < 0)
+ if (ret < 0) {
+ dev_err(&pdev->dev, "gpiochip_add failed with error %d\n", ret);
return ret;
+ }
- for (i = 0; i < msm_gpio.gpio_chip.ngpio; ++i) {
- irq = msm_gpio_to_irq(&msm_gpio.gpio_chip, i);
- irq_set_chip_and_handler(irq, &msm_gpio_irq_chip,
- handle_level_irq);
- set_irq_flags(irq, IRQF_VALID);
+ msm_gpio.summary_irq = platform_get_irq(pdev, 0);
+ if (msm_gpio.summary_irq < 0) {
+ dev_err(&pdev->dev, "No Summary irq defined for msmgpio\n");
+ return msm_gpio.summary_irq;
}
- irq_set_chained_handler(TLMM_SCSS_SUMMARY_IRQ,
- msm_summary_irq_handler);
+ msm_gpio.domain = irq_domain_add_linear(pdev->dev.of_node, ngpio,
+ &msm_gpio_irq_domain_ops,
+ &msm_gpio);
+ if (!msm_gpio.domain)
+ return -ENODEV;
+
+ irq_set_chained_handler(msm_gpio.summary_irq, msm_summary_irq_handler);
+
return 0;
}
+static struct of_device_id msm_gpio_of_match[] = {
+ { .compatible = "qcom,msm-gpio", },
+ { },
+};
+
static int msm_gpio_remove(struct platform_device *dev)
{
int ret = gpiochip_remove(&msm_gpio.gpio_chip);
@@ -381,7 +444,7 @@ static int msm_gpio_remove(struct platform_device *dev)
if (ret < 0)
return ret;
- irq_set_handler(TLMM_SCSS_SUMMARY_IRQ, NULL);
+ irq_set_handler(msm_gpio.summary_irq, NULL);
return 0;
}
@@ -392,36 +455,11 @@ static struct platform_driver msm_gpio_driver = {
.driver = {
.name = "msmgpio",
.owner = THIS_MODULE,
+ .of_match_table = msm_gpio_of_match,
},
};
-static struct platform_device msm_device_gpio = {
- .name = "msmgpio",
- .id = -1,
-};
-
-static int __init msm_gpio_init(void)
-{
- int rc;
-
- rc = platform_driver_register(&msm_gpio_driver);
- if (!rc) {
- rc = platform_device_register(&msm_device_gpio);
- if (rc)
- platform_driver_unregister(&msm_gpio_driver);
- }
-
- return rc;
-}
-
-static void __exit msm_gpio_exit(void)
-{
- platform_device_unregister(&msm_device_gpio);
- platform_driver_unregister(&msm_gpio_driver);
-}
-
-postcore_initcall(msm_gpio_init);
-module_exit(msm_gpio_exit);
+module_platform_driver(msm_gpio_driver)
MODULE_AUTHOR("Gregory Bean <[email protected]>");
MODULE_DESCRIPTION("Driver for Qualcomm MSM TLMMv2 SoC GPIOs");
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
MSM_TLMM_BASE is currently not mapped by 8x60. Remove it.
Signed-off-by: Rohit Vaswani <[email protected]>
---
arch/arm/mach-msm/include/mach/msm_iomap-8x60.h | 4 ----
1 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/arch/arm/mach-msm/include/mach/msm_iomap-8x60.h b/arch/arm/mach-msm/include/mach/msm_iomap-8x60.h
index 199372e..cf24b5c 100644
--- a/arch/arm/mach-msm/include/mach/msm_iomap-8x60.h
+++ b/arch/arm/mach-msm/include/mach/msm_iomap-8x60.h
@@ -41,10 +41,6 @@
#define MSM8X60_QGIC_CPU_PHYS 0x02081000
#define MSM8X60_QGIC_CPU_SIZE SZ_4K
-#define MSM_TLMM_BASE IOMEM(0xF0004000)
-#define MSM_TLMM_PHYS 0x00800000
-#define MSM_TLMM_SIZE SZ_16K
-
#define MSM8X60_TMR_PHYS 0x02000000
#define MSM8X60_TMR_SIZE SZ_4K
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
On Sat, Jun 1, 2013 at 1:22 AM, Rohit Vaswani <[email protected]> wrote:
> This cleans up the gpio-msm-v2 driver of all the global define usage.
> The number of gpios are now defined in the device tree. This enables
> adding irqdomain support as well.
>
> Signed-off-by: Rohit Vaswani <[email protected]>
> ---
> .../devicetree/bindings/gpio/gpio-msm.txt | 26 +++
> arch/arm/boot/dts/msm8660-surf.dts | 11 +
> arch/arm/boot/dts/msm8960-cdp.dts | 11 +
> drivers/gpio/Kconfig | 2 +-
> drivers/gpio/gpio-msm-v2.c | 194 ++++++++++++--------
> 5 files changed, 165 insertions(+), 79 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/gpio/gpio-msm.txt
>
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-msm.txt b/Documentation/devicetree/bindings/gpio/gpio-msm.txt
> new file mode 100644
> index 0000000..ac20e68
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-msm.txt
> @@ -0,0 +1,26 @@
> +MSM GPIO controller bindings
> +
> +Required properties:
> +- compatible:
> + - "qcom,msm-gpio" for MSM controllers
> +- #gpio-cells : Should be two.
> + - first cell is the pin number
> + - second cell is used to specify optional parameters (unused)
> +- gpio-controller : Marks the device node as a GPIO controller.
> +- #interrupt-cells : Should be 2.
> +- interrupt-controller: Mark the device node as an interrupt controller
> +- interrupts : Specify the TLMM summary interrupt number
> +- ngpio : Specify the number of MSM GPIOs
> +
> +Example:
> +
> + msmgpio: gpio@fd510000 {
> + compatible = "qcom,msm-gpio";
> + gpio-controller;
> + #gpio-cells = <2>;
> + interrupt-controller;
> + #interrupt-cells = <2>;
> + reg = <0xfd510000 0x4000>;
> + interrupts = <0 208 0>;
> + ngpio = <150>;
> + };
> diff --git a/arch/arm/boot/dts/msm8660-surf.dts b/arch/arm/boot/dts/msm8660-surf.dts
> index 9bf49b3..8931906 100644
> --- a/arch/arm/boot/dts/msm8660-surf.dts
> +++ b/arch/arm/boot/dts/msm8660-surf.dts
> @@ -26,6 +26,17 @@
> cpu-offset = <0x40000>;
> };
>
> + msmgpio: gpio@800000 {
> + compatible = "qcom,msm-gpio";
> + reg = <0x00800000 0x1000>;
> + gpio-controller;
> + #gpio-cells = <2>;
> + ngpio = <173>;
> + interrupts = <0 32 0x4>;
> + interrupt-controller;
> + #interrupt-cells = <2>;
> + };
> +
> serial@19c400000 {
> compatible = "qcom,msm-hsuart", "qcom,msm-uart";
> reg = <0x19c40000 0x1000>,
> diff --git a/arch/arm/boot/dts/msm8960-cdp.dts b/arch/arm/boot/dts/msm8960-cdp.dts
> index 2e4d87a..52fe253 100644
> --- a/arch/arm/boot/dts/msm8960-cdp.dts
> +++ b/arch/arm/boot/dts/msm8960-cdp.dts
> @@ -26,6 +26,17 @@
> cpu-offset = <0x80000>;
> };
>
> + msmgpio: gpio@fd510000 {
> + compatible = "qcom,msm-gpio";
> + gpio-controller;
> + #gpio-cells = <2>;
> + ngpio = <150>;
> + interrupts = <0 32 0x4>;
> + interrupt-controller;
> + #interrupt-cells = <2>;
> + reg = <0xfd510000 0x4000>;
> + };
> +
> serial@19c400000 {
> compatible = "qcom,msm-hsuart", "qcom,msm-uart";
> reg = <0x16440000 0x1000>,
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 87d5670..6d61a12 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -165,7 +165,7 @@ config GPIO_MSM_V1
>
> config GPIO_MSM_V2
> tristate "Qualcomm MSM GPIO v2"
> - depends on GPIOLIB && ARCH_MSM
> + depends on GPIOLIB && OF && ARCH_MSM
> help
> Say yes here to support the GPIO interface on ARM v7 based
> Qualcomm MSM chips. Most of the pins on the MSM can be
> diff --git a/drivers/gpio/gpio-msm-v2.c b/drivers/gpio/gpio-msm-v2.c
> index 75cc821..5a824be 100644
> --- a/drivers/gpio/gpio-msm-v2.c
> +++ b/drivers/gpio/gpio-msm-v2.c
> @@ -19,17 +19,19 @@
>
> #include <linux/bitmap.h>
> #include <linux/bitops.h>
> +#include <linux/err.h>
> #include <linux/gpio.h>
> #include <linux/init.h>
> #include <linux/interrupt.h>
> #include <linux/io.h>
> #include <linux/irqchip/chained_irq.h>
> #include <linux/irq.h>
> +#include <linux/irqdomain.h>
> #include <linux/module.h>
> +#include <linux/of_address.h>
> #include <linux/platform_device.h>
> #include <linux/spinlock.h>
> -
> -#include <mach/msm_iomap.h>
> +#include <linux/slab.h>
>
> /* Bits of interest in the GPIO_IN_OUT register.
> */
> @@ -76,13 +78,6 @@ enum {
> TARGET_PROC_NONE = 7,
> };
>
> -
> -#define GPIO_INTR_CFG_SU(gpio) (MSM_TLMM_BASE + 0x0400 + (0x04 * (gpio)))
> -#define GPIO_CONFIG(gpio) (MSM_TLMM_BASE + 0x1000 + (0x10 * (gpio)))
> -#define GPIO_IN_OUT(gpio) (MSM_TLMM_BASE + 0x1004 + (0x10 * (gpio)))
> -#define GPIO_INTR_CFG(gpio) (MSM_TLMM_BASE + 0x1008 + (0x10 * (gpio)))
> -#define GPIO_INTR_STATUS(gpio) (MSM_TLMM_BASE + 0x100c + (0x10 * (gpio)))
> -
> /**
> * struct msm_gpio_dev: the MSM8660 SoC GPIO device structure
> *
> @@ -101,11 +96,27 @@ enum {
> */
> struct msm_gpio_dev {
> struct gpio_chip gpio_chip;
> - DECLARE_BITMAP(enabled_irqs, NR_GPIO_IRQS);
> - DECLARE_BITMAP(wake_irqs, NR_GPIO_IRQS);
> - DECLARE_BITMAP(dual_edge_irqs, NR_GPIO_IRQS);
> + unsigned long *enabled_irqs;
> + unsigned long *wake_irqs;
> + unsigned long *dual_edge_irqs;
Was there a reason you ignored the comment to leave these bitmaps as
statically allocated?
[...]
> + msm_gpio.enabled_irqs = devm_kzalloc(&pdev->dev,
> + sizeof(unsigned long) *
> + BITS_TO_LONGS(ngpio) * 3,
> + GFP_KERNEL);
> + msm_gpio.wake_irqs = &msm_gpio.enabled_irqs[BITS_TO_LONGS(ngpio)];
> + msm_gpio.dual_edge_irqs =
> + &msm_gpio.enabled_irqs[BITS_TO_LONGS(ngpio * 2)];
I should have just deleted my comment about doing it this way. I was
making the point that one allocation is better that three; but then I
also said that it was better to not allocate at all. Go back to the
statically allocated bitmap array. It is far better than this.
g.
On 6/1/2013 1:09 AM, Grant Likely wrote:
> On Sat, Jun 1, 2013 at 1:22 AM, Rohit Vaswani <[email protected]> wrote:
>> This cleans up the gpio-msm-v2 driver of all the global define usage.
>> The number of gpios are now defined in the device tree. This enables
>> adding irqdomain support as well.
>>
>> Signed-off-by: Rohit Vaswani <[email protected]>
>> ---
>> .../devicetree/bindings/gpio/gpio-msm.txt | 26 +++
>> arch/arm/boot/dts/msm8660-surf.dts | 11 +
>> arch/arm/boot/dts/msm8960-cdp.dts | 11 +
>> drivers/gpio/Kconfig | 2 +-
>> drivers/gpio/gpio-msm-v2.c | 194 ++++++++++++--------
>> 5 files changed, 165 insertions(+), 79 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/gpio/gpio-msm.txt
>>
>> diff --git a/Documentation/devicetree/bindings/gpio/gpio-msm.txt b/Documentation/devicetree/bindings/gpio/gpio-msm.txt
>> new file mode 100644
>> index 0000000..ac20e68
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/gpio/gpio-msm.txt
>> @@ -0,0 +1,26 @@
>> +MSM GPIO controller bindings
>> +
>> +Required properties:
>> +- compatible:
>> + - "qcom,msm-gpio" for MSM controllers
>> +- #gpio-cells : Should be two.
>> + - first cell is the pin number
>> + - second cell is used to specify optional parameters (unused)
>> +- gpio-controller : Marks the device node as a GPIO controller.
>> +- #interrupt-cells : Should be 2.
>> +- interrupt-controller: Mark the device node as an interrupt controller
>> +- interrupts : Specify the TLMM summary interrupt number
>> +- ngpio : Specify the number of MSM GPIOs
>> +
>> +Example:
>> +
>> + msmgpio: gpio@fd510000 {
>> + compatible = "qcom,msm-gpio";
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> + interrupt-controller;
>> + #interrupt-cells = <2>;
>> + reg = <0xfd510000 0x4000>;
>> + interrupts = <0 208 0>;
>> + ngpio = <150>;
>> + };
>> diff --git a/arch/arm/boot/dts/msm8660-surf.dts b/arch/arm/boot/dts/msm8660-surf.dts
>> index 9bf49b3..8931906 100644
>> --- a/arch/arm/boot/dts/msm8660-surf.dts
>> +++ b/arch/arm/boot/dts/msm8660-surf.dts
>> @@ -26,6 +26,17 @@
>> cpu-offset = <0x40000>;
>> };
>>
>> + msmgpio: gpio@800000 {
>> + compatible = "qcom,msm-gpio";
>> + reg = <0x00800000 0x1000>;
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> + ngpio = <173>;
>> + interrupts = <0 32 0x4>;
>> + interrupt-controller;
>> + #interrupt-cells = <2>;
>> + };
>> +
>> serial@19c400000 {
>> compatible = "qcom,msm-hsuart", "qcom,msm-uart";
>> reg = <0x19c40000 0x1000>,
>> diff --git a/arch/arm/boot/dts/msm8960-cdp.dts b/arch/arm/boot/dts/msm8960-cdp.dts
>> index 2e4d87a..52fe253 100644
>> --- a/arch/arm/boot/dts/msm8960-cdp.dts
>> +++ b/arch/arm/boot/dts/msm8960-cdp.dts
>> @@ -26,6 +26,17 @@
>> cpu-offset = <0x80000>;
>> };
>>
>> + msmgpio: gpio@fd510000 {
>> + compatible = "qcom,msm-gpio";
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> + ngpio = <150>;
>> + interrupts = <0 32 0x4>;
>> + interrupt-controller;
>> + #interrupt-cells = <2>;
>> + reg = <0xfd510000 0x4000>;
>> + };
>> +
>> serial@19c400000 {
>> compatible = "qcom,msm-hsuart", "qcom,msm-uart";
>> reg = <0x16440000 0x1000>,
>> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
>> index 87d5670..6d61a12 100644
>> --- a/drivers/gpio/Kconfig
>> +++ b/drivers/gpio/Kconfig
>> @@ -165,7 +165,7 @@ config GPIO_MSM_V1
>>
>> config GPIO_MSM_V2
>> tristate "Qualcomm MSM GPIO v2"
>> - depends on GPIOLIB && ARCH_MSM
>> + depends on GPIOLIB && OF && ARCH_MSM
>> help
>> Say yes here to support the GPIO interface on ARM v7 based
>> Qualcomm MSM chips. Most of the pins on the MSM can be
>> diff --git a/drivers/gpio/gpio-msm-v2.c b/drivers/gpio/gpio-msm-v2.c
>> index 75cc821..5a824be 100644
>> --- a/drivers/gpio/gpio-msm-v2.c
>> +++ b/drivers/gpio/gpio-msm-v2.c
>> @@ -19,17 +19,19 @@
>>
>> #include <linux/bitmap.h>
>> #include <linux/bitops.h>
>> +#include <linux/err.h>
>> #include <linux/gpio.h>
>> #include <linux/init.h>
>> #include <linux/interrupt.h>
>> #include <linux/io.h>
>> #include <linux/irqchip/chained_irq.h>
>> #include <linux/irq.h>
>> +#include <linux/irqdomain.h>
>> #include <linux/module.h>
>> +#include <linux/of_address.h>
>> #include <linux/platform_device.h>
>> #include <linux/spinlock.h>
>> -
>> -#include <mach/msm_iomap.h>
>> +#include <linux/slab.h>
>>
>> /* Bits of interest in the GPIO_IN_OUT register.
>> */
>> @@ -76,13 +78,6 @@ enum {
>> TARGET_PROC_NONE = 7,
>> };
>>
>> -
>> -#define GPIO_INTR_CFG_SU(gpio) (MSM_TLMM_BASE + 0x0400 + (0x04 * (gpio)))
>> -#define GPIO_CONFIG(gpio) (MSM_TLMM_BASE + 0x1000 + (0x10 * (gpio)))
>> -#define GPIO_IN_OUT(gpio) (MSM_TLMM_BASE + 0x1004 + (0x10 * (gpio)))
>> -#define GPIO_INTR_CFG(gpio) (MSM_TLMM_BASE + 0x1008 + (0x10 * (gpio)))
>> -#define GPIO_INTR_STATUS(gpio) (MSM_TLMM_BASE + 0x100c + (0x10 * (gpio)))
>> -
>> /**
>> * struct msm_gpio_dev: the MSM8660 SoC GPIO device structure
>> *
>> @@ -101,11 +96,27 @@ enum {
>> */
>> struct msm_gpio_dev {
>> struct gpio_chip gpio_chip;
>> - DECLARE_BITMAP(enabled_irqs, NR_GPIO_IRQS);
>> - DECLARE_BITMAP(wake_irqs, NR_GPIO_IRQS);
>> - DECLARE_BITMAP(dual_edge_irqs, NR_GPIO_IRQS);
>> + unsigned long *enabled_irqs;
>> + unsigned long *wake_irqs;
>> + unsigned long *dual_edge_irqs;
> Was there a reason you ignored the comment to leave these bitmaps as
> statically allocated?
>
> [...]
>
>> + msm_gpio.enabled_irqs = devm_kzalloc(&pdev->dev,
>> + sizeof(unsigned long) *
>> + BITS_TO_LONGS(ngpio) * 3,
>> + GFP_KERNEL);
>> + msm_gpio.wake_irqs = &msm_gpio.enabled_irqs[BITS_TO_LONGS(ngpio)];
>> + msm_gpio.dual_edge_irqs =
>> + &msm_gpio.enabled_irqs[BITS_TO_LONGS(ngpio * 2)];
> I should have just deleted my comment about doing it this way. I was
> making the point that one allocation is better that three; but then I
> also said that it was better to not allocate at all. Go back to the
> statically allocated bitmap array. It is far better than this.
>
> g.
I agree that DECLARE_BITMAP is the most efficient way, but
DECLARE_BITMAP takes a statically defined number of gpios as an
argument. Since we get the ngpio from device tree, these had to go as
well. Under this scheme, 1 allocation was better than 3 and went ahead
with your suggestion.
Thanks,
Rohit Vaswani
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation
On Sun, 02 Jun 2013 19:31:33 -0700, Rohit Vaswani <[email protected]> wrote:
> On 6/1/2013 1:09 AM, Grant Likely wrote:
> > On Sat, Jun 1, 2013 at 1:22 AM, Rohit Vaswani <[email protected]> wrote:
> >> This cleans up the gpio-msm-v2 driver of all the global define usage.
> >> The number of gpios are now defined in the device tree. This enables
> >> adding irqdomain support as well.
> >>
> >> Signed-off-by: Rohit Vaswani <[email protected]>
> >> @@ -101,11 +96,27 @@ enum {
> >> */
> >> struct msm_gpio_dev {
> >> struct gpio_chip gpio_chip;
> >> - DECLARE_BITMAP(enabled_irqs, NR_GPIO_IRQS);
> >> - DECLARE_BITMAP(wake_irqs, NR_GPIO_IRQS);
> >> - DECLARE_BITMAP(dual_edge_irqs, NR_GPIO_IRQS);
> >> + unsigned long *enabled_irqs;
> >> + unsigned long *wake_irqs;
> >> + unsigned long *dual_edge_irqs;
> > Was there a reason you ignored the comment to leave these bitmaps as
> > statically allocated?
> >
> > [...]
> >
> >> + msm_gpio.enabled_irqs = devm_kzalloc(&pdev->dev,
> >> + sizeof(unsigned long) *
> >> + BITS_TO_LONGS(ngpio) * 3,
> >> + GFP_KERNEL);
> >> + msm_gpio.wake_irqs = &msm_gpio.enabled_irqs[BITS_TO_LONGS(ngpio)];
> >> + msm_gpio.dual_edge_irqs =
> >> + &msm_gpio.enabled_irqs[BITS_TO_LONGS(ngpio * 2)];
> > I should have just deleted my comment about doing it this way. I was
> > making the point that one allocation is better that three; but then I
> > also said that it was better to not allocate at all. Go back to the
> > statically allocated bitmap array. It is far better than this.
> >
> > g.
>
> I agree that DECLARE_BITMAP is the most efficient way, but
> DECLARE_BITMAP takes a statically defined number of gpios as an
> argument. Since we get the ngpio from device tree, these had to go as
> well. Under this scheme, 1 allocation was better than 3 and went ahead
> with your suggestion.
Think about it for a moment; You *know* all the devices that are going
to use the driver. The largest size for ngpios that I see is 173 which
is a mere 18 long words for 3 bitmaps. Even if you were to quadruple
that amount the total for all the bitmasks is 72 long words. You're
optimizing at the wrong place. Changing it to dynamic allocation makes
the code slower, more complicated, and probably doesn't do anything to
save on real memory consumption.
The solution is simple; choose a maximum value of ngpios that the driver
will likely need to work support. If a device appears with more GPIOs,
then the driver should throw a warning and work with the maximum it can
support. Part of enablement for the new device will be increasing the
driver to handle more gpios.
g.
On 6/5/2013 2:38 PM, Grant Likely wrote:
> On Sun, 02 Jun 2013 19:31:33 -0700, Rohit Vaswani <[email protected]> wrote:
>> On 6/1/2013 1:09 AM, Grant Likely wrote:
>>> On Sat, Jun 1, 2013 at 1:22 AM, Rohit Vaswani <[email protected]> wrote:
>>>> This cleans up the gpio-msm-v2 driver of all the global define usage.
>>>> The number of gpios are now defined in the device tree. This enables
>>>> adding irqdomain support as well.
>>>>
>>>> Signed-off-by: Rohit Vaswani <[email protected]>
>>>> @@ -101,11 +96,27 @@ enum {
>>>> */
>>>> struct msm_gpio_dev {
>>>> struct gpio_chip gpio_chip;
>>>> - DECLARE_BITMAP(enabled_irqs, NR_GPIO_IRQS);
>>>> - DECLARE_BITMAP(wake_irqs, NR_GPIO_IRQS);
>>>> - DECLARE_BITMAP(dual_edge_irqs, NR_GPIO_IRQS);
>>>> + unsigned long *enabled_irqs;
>>>> + unsigned long *wake_irqs;
>>>> + unsigned long *dual_edge_irqs;
>>> Was there a reason you ignored the comment to leave these bitmaps as
>>> statically allocated?
>>>
>>> [...]
>>>
>>>> + msm_gpio.enabled_irqs = devm_kzalloc(&pdev->dev,
>>>> + sizeof(unsigned long) *
>>>> + BITS_TO_LONGS(ngpio) * 3,
>>>> + GFP_KERNEL);
>>>> + msm_gpio.wake_irqs = &msm_gpio.enabled_irqs[BITS_TO_LONGS(ngpio)];
>>>> + msm_gpio.dual_edge_irqs =
>>>> + &msm_gpio.enabled_irqs[BITS_TO_LONGS(ngpio * 2)];
>>> I should have just deleted my comment about doing it this way. I was
>>> making the point that one allocation is better that three; but then I
>>> also said that it was better to not allocate at all. Go back to the
>>> statically allocated bitmap array. It is far better than this.
>>>
>>> g.
>> I agree that DECLARE_BITMAP is the most efficient way, but
>> DECLARE_BITMAP takes a statically defined number of gpios as an
>> argument. Since we get the ngpio from device tree, these had to go as
>> well. Under this scheme, 1 allocation was better than 3 and went ahead
>> with your suggestion.
> Think about it for a moment; You *know* all the devices that are going
> to use the driver. The largest size for ngpios that I see is 173 which
> is a mere 18 long words for 3 bitmaps. Even if you were to quadruple
> that amount the total for all the bitmasks is 72 long words. You're
> optimizing at the wrong place. Changing it to dynamic allocation makes
> the code slower, more complicated, and probably doesn't do anything to
> save on real memory consumption.
>
> The solution is simple; choose a maximum value of ngpios that the driver
> will likely need to work support. If a device appears with more GPIOs,
> then the driver should throw a warning and work with the maximum it can
> support. Part of enablement for the new device will be increasing the
> driver to handle more gpios.
>
> g.
Got it.
Thanks,
Rohit Vaswani
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation
On Sat, Jun 1, 2013 at 2:21 AM, Rohit Vaswani <[email protected]> wrote:
> Remove gpiomux-v2 as it's not being used and make way for future improvements.
>
> Signed-off-by: Rohit Vaswani <[email protected]>
> ---
> arch/arm/mach-msm/Kconfig | 13 +++-----
> arch/arm/mach-msm/Makefile | 6 +--
> arch/arm/mach-msm/gpiomux-8x60.c | 19 ------------
> arch/arm/mach-msm/gpiomux-v2.c | 25 ---------------
> arch/arm/mach-msm/gpiomux-v2.h | 61 --------------------------------------
> arch/arm/mach-msm/gpiomux.c | 15 +++++++++
> arch/arm/mach-msm/gpiomux.h | 5 ---
> drivers/gpio/gpio-msm-v2.c | 5 +--
> 8 files changed, 24 insertions(+), 125 deletions(-)
> delete mode 100644 arch/arm/mach-msm/gpiomux-8x60.c
> delete mode 100644 arch/arm/mach-msm/gpiomux-v2.c
> delete mode 100644 arch/arm/mach-msm/gpiomux-v2.h
Acked-by: Linus Walleij <[email protected]>
Yours,
Linus Walleij