2010-11-18 19:52:58

by Gregory Bean

[permalink] [raw]
Subject: [PATCH v4 1/2] msm: gpio: Add v2 gpio support to MSM SoCs.

Beginning with the MSM8x60, the hardware block responsible for gpio
support changes. Provide gpiolib support for the new v2 architecture.

Cc: Baruch Siach <[email protected]>
Cc: Pavan Kondeti <[email protected]>
Signed-off-by: Gregory Bean <[email protected]>
---
v4 - miscellaneous cleanup, per [email protected]
v3 - miscellaneous cleanup, per [email protected]
v2 - miscellaneous cleanup, per [email protected]

arch/arm/mach-msm/Makefile | 4 +-
arch/arm/mach-msm/gpio-v2.c | 163 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 166 insertions(+), 1 deletions(-)
create mode 100644 arch/arm/mach-msm/gpio-v2.c

diff --git a/arch/arm/mach-msm/Makefile b/arch/arm/mach-msm/Makefile
index b5a7b07..4a1a7ea 100644
--- a/arch/arm/mach-msm/Makefile
+++ b/arch/arm/mach-msm/Makefile
@@ -28,6 +28,8 @@ obj-$(CONFIG_ARCH_MSM8X60) += board-msm8x60.o
obj-$(CONFIG_ARCH_MSM7X30) += gpiomux-7x30.o 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
-ifndef CONFIG_MSM_V2_TLMM
+ifdef CONFIG_MSM_V2_TLMM
+obj-y += gpio-v2.o
+else
obj-y += gpio.o
endif
diff --git a/arch/arm/mach-msm/gpio-v2.c b/arch/arm/mach-msm/gpio-v2.c
new file mode 100644
index 0000000..d907af6
--- /dev/null
+++ b/arch/arm/mach-msm/gpio-v2.c
@@ -0,0 +1,163 @@
+/* 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/gpio.h>
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+#include <mach/msm_iomap.h>
+#include "gpiomux.h"
+
+/* Bits of interest in the GPIO_IN_OUT register.
+ */
+enum {
+ GPIO_IN_BIT = 0,
+ GPIO_OUT_BIT = 1
+};
+
+/* Bits of interest in the GPIO_CFG register.
+ */
+enum {
+ GPIO_OE_BIT = 9,
+};
+
+#define GPIO_CONFIG(gpio) (MSM_TLMM_BASE + 0x1000 + (0x10 * (gpio)))
+#define GPIO_IN_OUT(gpio) (MSM_TLMM_BASE + 0x1004 + (0x10 * (gpio)))
+
+static DEFINE_SPINLOCK(tlmm_lock);
+
+static int msm_gpio_get(struct gpio_chip *chip, unsigned offset)
+{
+ return readl(GPIO_IN_OUT(offset)) & BIT(GPIO_IN_BIT);
+}
+
+static void msm_gpio_set(struct gpio_chip *chip, unsigned offset, int val)
+{
+ writel(val ? BIT(GPIO_OUT_BIT) : 0, GPIO_IN_OUT(offset));
+}
+
+static int msm_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
+{
+ unsigned long irq_flags;
+
+ spin_lock_irqsave(&tlmm_lock, irq_flags);
+ writel(readl(GPIO_CONFIG(offset)) & ~BIT(GPIO_OE_BIT),
+ GPIO_CONFIG(offset));
+ spin_unlock_irqrestore(&tlmm_lock, irq_flags);
+ return 0;
+}
+
+static int msm_gpio_direction_output(struct gpio_chip *chip,
+ unsigned offset,
+ int val)
+{
+ unsigned long irq_flags;
+
+ spin_lock_irqsave(&tlmm_lock, irq_flags);
+ msm_gpio_set(chip, offset, val);
+ writel(readl(GPIO_CONFIG(offset)) | BIT(GPIO_OE_BIT),
+ GPIO_CONFIG(offset));
+ spin_unlock_irqrestore(&tlmm_lock, irq_flags);
+ return 0;
+}
+
+static int msm_gpio_request(struct gpio_chip *chip, unsigned offset)
+{
+ return msm_gpiomux_get(chip->base + offset);
+}
+
+static void msm_gpio_free(struct gpio_chip *chip, unsigned offset)
+{
+ msm_gpiomux_put(chip->base + offset);
+}
+
+static struct gpio_chip msm_gpio = {
+ .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,
+ .request = msm_gpio_request,
+ .free = msm_gpio_free,
+};
+
+static int __devinit msm_gpio_probe(struct platform_device *dev)
+{
+ int ret;
+
+ msm_gpio.label = dev->name;
+ ret = gpiochip_add(&msm_gpio);
+
+ return ret;
+}
+
+static int __devexit msm_gpio_remove(struct platform_device *dev)
+{
+ int ret = gpiochip_remove(&msm_gpio);
+
+ if (ret < 0)
+ return ret;
+
+ set_irq_handler(TLMM_SCSS_SUMMARY_IRQ, NULL);
+
+ return 0;
+}
+
+static struct platform_driver msm_gpio_driver = {
+ .probe = msm_gpio_probe,
+ .remove = __devexit_p(msm_gpio_remove),
+ .driver = {
+ .name = "msmgpio",
+ .owner = THIS_MODULE,
+ },
+};
+
+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_AUTHOR("Gregory Bean <[email protected]>");
+MODULE_DESCRIPTION("Driver for Qualcomm MSM TLMMv2 SoC GPIOs");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:msmgpio");
--
1.7.0.4

--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.


2010-11-18 19:52:57

by Gregory Bean

[permalink] [raw]
Subject: [PATCH v4 2/2] msm: gpio: Add irq support to v2 gpiolib.

Complete the MSM v2 gpio subsystem by adding irq_chip.

Signed-off-by: Gregory Bean <[email protected]>
---
v4 - miscellaneous cleanup to patch 1/2, per [email protected]
v3 - miscellaneous cleanup to patch 1/2, per [email protected]
v2 - miscellaneous cleanup to patch 1/2, per [email protected]

arch/arm/mach-msm/gpio-v2.c | 344 ++++++++++++++++++++++++++++++++++++++++---
1 files changed, 326 insertions(+), 18 deletions(-)

diff --git a/arch/arm/mach-msm/gpio-v2.c b/arch/arm/mach-msm/gpio-v2.c
index d907af6..926e3d0 100644
--- a/arch/arm/mach-msm/gpio-v2.c
+++ b/arch/arm/mach-msm/gpio-v2.c
@@ -15,7 +15,11 @@
* 02110-1301, USA.
*
*/
+#include <linux/bitmap.h>
+#include <linux/bitops.h>
#include <linux/gpio.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
#include <linux/io.h>
#include <linux/irq.h>
#include <linux/module.h>
@@ -31,17 +35,97 @@ enum {
GPIO_OUT_BIT = 1
};

+/* Bits of interest in the GPIO_INTR_STATUS register.
+ */
+enum {
+ INTR_STATUS_BIT = 0,
+};
+
/* Bits of interest in the GPIO_CFG register.
*/
enum {
GPIO_OE_BIT = 9,
};

+/* Bits of interest in the GPIO_INTR_CFG register.
+ */
+enum {
+ INTR_ENABLE_BIT = 0,
+ INTR_POL_CTL_BIT = 1,
+ INTR_DECT_CTL_BIT = 2,
+ INTR_RAW_STATUS_EN_BIT = 3,
+};
+
+/* Codes of interest in GPIO_INTR_CFG_SU.
+ */
+enum {
+ TARGET_PROC_SCORPION = 4,
+ TARGET_PROC_NONE = 7,
+};
+
+/*
+ * When a GPIO triggers, two separate decisions are made, controlled
+ * by two separate flags.
+ *
+ * - First, INTR_RAW_STATUS_EN controls whether or not the GPIO_INTR_STATUS
+ * register for that GPIO will be updated to reflect the triggering of that
+ * gpio. If this bit is 0, this register will not be updated.
+ * - Second, INTR_ENABLE controls whether an interrupt is triggered.
+ *
+ * If INTR_ENABLE is set and INTR_RAW_STATUS_EN is NOT set, an interrupt
+ * can be triggered but the status register will not reflect it.
+ */
+#define INTR_RAW_STATUS_EN BIT(INTR_RAW_STATUS_EN_BIT)
+#define INTR_ENABLE BIT(INTR_ENABLE_BIT)
+#define INTR_DECT_CTL_EDGE BIT(INTR_DECT_CTL_BIT)
+#define INTR_POL_CTL_HI BIT(INTR_POL_CTL_BIT)
+
+#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
+ *
+ * @enabled_irqs: a bitmap used to optimize the summary-irq handler. By
+ * keeping track of which gpios are unmasked as irq sources, we avoid
+ * having to do readl calls on hundreds of iomapped registers each time
+ * the summary interrupt fires in order to locate the active interrupts.
+ *
+ * @wake_irqs: a bitmap for tracking which interrupt lines are enabled
+ * as wakeup sources. When the device is suspended, interrupts which are
+ * not wakeup sources are disabled.
+ *
+ * @dual_edge_irqs: a bitmap used to track which irqs are configured
+ * as dual-edge, as this is not supported by the hardware and requires
+ * some special handling in the driver.
+ */
+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);
+};

static DEFINE_SPINLOCK(tlmm_lock);

+static inline struct msm_gpio_dev *to_msm_gpio_dev(struct gpio_chip *chip)
+{
+ return container_of(chip, struct msm_gpio_dev, gpio_chip);
+}
+
+static inline void set_gpio_bits(unsigned n, void __iomem *reg)
+{
+ writel(readl(reg) | n, reg);
+}
+
+static inline void clr_gpio_bits(unsigned n, void __iomem *reg)
+{
+ writel(readl(reg) & ~n, reg);
+}
+
static int msm_gpio_get(struct gpio_chip *chip, unsigned offset)
{
return readl(GPIO_IN_OUT(offset)) & BIT(GPIO_IN_BIT);
@@ -57,8 +141,7 @@ static int msm_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
unsigned long irq_flags;

spin_lock_irqsave(&tlmm_lock, irq_flags);
- writel(readl(GPIO_CONFIG(offset)) & ~BIT(GPIO_OE_BIT),
- GPIO_CONFIG(offset));
+ clr_gpio_bits(BIT(GPIO_OE_BIT), GPIO_CONFIG(offset));
spin_unlock_irqrestore(&tlmm_lock, irq_flags);
return 0;
}
@@ -71,8 +154,7 @@ static int msm_gpio_direction_output(struct gpio_chip *chip,

spin_lock_irqsave(&tlmm_lock, irq_flags);
msm_gpio_set(chip, offset, val);
- writel(readl(GPIO_CONFIG(offset)) | BIT(GPIO_OE_BIT),
- GPIO_CONFIG(offset));
+ set_gpio_bits(BIT(GPIO_OE_BIT), GPIO_CONFIG(offset));
spin_unlock_irqrestore(&tlmm_lock, irq_flags);
return 0;
}
@@ -87,30 +169,215 @@ static void msm_gpio_free(struct gpio_chip *chip, unsigned offset)
msm_gpiomux_put(chip->base + offset);
}

-static struct gpio_chip msm_gpio = {
- .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,
- .request = msm_gpio_request,
- .free = msm_gpio_free,
+static int msm_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
+{
+ return MSM_GPIO_TO_INT(offset - chip->base);
+}
+
+static inline int msm_irq_to_gpio(struct gpio_chip *chip, unsigned irq)
+{
+ return irq - MSM_GPIO_TO_INT(chip->base);
+}
+
+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:
+ *
+ * At appropriate moments, this function may be called to flip the polarity
+ * settings of both-edge irq lines to try and catch the next edge.
+ *
+ * The attempt is considered successful if:
+ * - the status bit goes high, indicating that an edge was caught, or
+ * - the input value of the gpio doesn't change during the attempt.
+ * If the value changes twice during the process, that would cause the first
+ * test to fail but would force the second, as two opposite
+ * transitions would cause a detection no matter the polarity setting.
+ *
+ * The do-loop tries to sledge-hammer closed the timing hole between
+ * the initial value-read and the polarity-write - if the line value changes
+ * during that window, an interrupt is lost, the new polarity setting is
+ * incorrect, and the first success test will fail, causing a retry.
+ *
+ * Algorithm comes from Google's msmgpio driver, see mach-msm/gpio.c.
+ */
+static void msm_gpio_update_dual_edge_pos(unsigned gpio)
+{
+ int loop_limit = 100;
+ unsigned val, val2, intstat;
+
+ do {
+ val = readl(GPIO_IN_OUT(gpio)) & BIT(GPIO_IN_BIT);
+ if (val)
+ clr_gpio_bits(INTR_POL_CTL_HI, GPIO_INTR_CFG(gpio));
+ else
+ set_gpio_bits(INTR_POL_CTL_HI, GPIO_INTR_CFG(gpio));
+ val2 = readl(GPIO_IN_OUT(gpio)) & BIT(GPIO_IN_BIT);
+ intstat = readl(GPIO_INTR_STATUS(gpio)) & BIT(INTR_STATUS_BIT);
+ if (intstat || val == val2)
+ return;
+ } while (loop_limit-- > 0);
+ pr_err("%s: dual-edge irq failed to stabilize, "
+ "interrupts dropped. %#08x != %#08x\n",
+ __func__, val, val2);
+}
+
+static void msm_gpio_irq_ack(unsigned int irq)
+{
+ int gpio = msm_irq_to_gpio(&msm_gpio.gpio_chip, irq);
+
+ writel(BIT(INTR_STATUS_BIT), GPIO_INTR_STATUS(gpio));
+ if (test_bit(gpio, msm_gpio.dual_edge_irqs))
+ msm_gpio_update_dual_edge_pos(gpio);
+}
+
+static void msm_gpio_irq_mask(unsigned int irq)
+{
+ int gpio = msm_irq_to_gpio(&msm_gpio.gpio_chip, irq);
+ unsigned long irq_flags;
+
+ spin_lock_irqsave(&tlmm_lock, irq_flags);
+ writel(TARGET_PROC_NONE, GPIO_INTR_CFG_SU(gpio));
+ clr_gpio_bits(INTR_RAW_STATUS_EN | INTR_ENABLE, GPIO_INTR_CFG(gpio));
+ __clear_bit(gpio, msm_gpio.enabled_irqs);
+ spin_unlock_irqrestore(&tlmm_lock, irq_flags);
+}
+
+static void msm_gpio_irq_unmask(unsigned int irq)
+{
+ int gpio = msm_irq_to_gpio(&msm_gpio.gpio_chip, irq);
+ unsigned long irq_flags;
+
+ spin_lock_irqsave(&tlmm_lock, irq_flags);
+ __set_bit(gpio, msm_gpio.enabled_irqs);
+ set_gpio_bits(INTR_RAW_STATUS_EN | INTR_ENABLE, GPIO_INTR_CFG(gpio));
+ writel(TARGET_PROC_SCORPION, GPIO_INTR_CFG_SU(gpio));
+ spin_unlock_irqrestore(&tlmm_lock, irq_flags);
+}
+
+static int msm_gpio_irq_set_type(unsigned int irq, unsigned int flow_type)
+{
+ int gpio = msm_irq_to_gpio(&msm_gpio.gpio_chip, irq);
+ unsigned long irq_flags;
+ uint32_t bits;
+
+ spin_lock_irqsave(&tlmm_lock, irq_flags);
+
+ bits = readl(GPIO_INTR_CFG(gpio));
+
+ if (flow_type & IRQ_TYPE_EDGE_BOTH) {
+ bits |= INTR_DECT_CTL_EDGE;
+ irq_desc[irq].handle_irq = handle_edge_irq;
+ if ((flow_type & IRQ_TYPE_EDGE_BOTH) == IRQ_TYPE_EDGE_BOTH)
+ __set_bit(gpio, msm_gpio.dual_edge_irqs);
+ else
+ __clear_bit(gpio, msm_gpio.dual_edge_irqs);
+ } else {
+ bits &= ~INTR_DECT_CTL_EDGE;
+ irq_desc[irq].handle_irq = handle_level_irq;
+ __clear_bit(gpio, msm_gpio.dual_edge_irqs);
+ }
+
+ if (flow_type & (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_LEVEL_HIGH))
+ bits |= INTR_POL_CTL_HI;
+ else
+ bits &= ~INTR_POL_CTL_HI;
+
+ writel(bits, GPIO_INTR_CFG(gpio));
+
+ if ((flow_type & IRQ_TYPE_EDGE_BOTH) == IRQ_TYPE_EDGE_BOTH)
+ msm_gpio_update_dual_edge_pos(gpio);
+
+ spin_unlock_irqrestore(&tlmm_lock, irq_flags);
+
+ return 0;
+}
+
+/*
+ * When the summary IRQ is raised, any number of GPIO lines may be high.
+ * It is the job of the summary handler to find all those GPIO lines
+ * which have been set as summary IRQ lines and which are triggered,
+ * and to call their interrupt handlers.
+ */
+static void msm_summary_irq_handler(unsigned int irq, struct irq_desc *desc)
+{
+ unsigned long i;
+
+ for (i = find_first_bit(msm_gpio.enabled_irqs, NR_GPIO_IRQS);
+ i < NR_GPIO_IRQS;
+ i = find_next_bit(msm_gpio.enabled_irqs, NR_GPIO_IRQS, i + 1)) {
+ if (readl(GPIO_INTR_STATUS(i)) & BIT(INTR_STATUS_BIT))
+ generic_handle_irq(msm_gpio_to_irq(&msm_gpio.gpio_chip,
+ i));
+ }
+ desc->chip->ack(irq);
+}
+
+static int msm_gpio_irq_set_wake(unsigned int irq, unsigned int on)
+{
+ int gpio = msm_irq_to_gpio(&msm_gpio.gpio_chip, irq);
+
+ if (on) {
+ if (bitmap_empty(msm_gpio.wake_irqs, NR_GPIO_IRQS))
+ set_irq_wake(TLMM_SCSS_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))
+ set_irq_wake(TLMM_SCSS_SUMMARY_IRQ, 0);
+ }
+
+ return 0;
+}
+
+static struct irq_chip msm_gpio_irq_chip = {
+ .name = "msmgpio",
+ .mask = msm_gpio_irq_mask,
+ .unmask = msm_gpio_irq_unmask,
+ .ack = msm_gpio_irq_ack,
+ .set_type = msm_gpio_irq_set_type,
+ .set_wake = msm_gpio_irq_set_wake,
};

static int __devinit msm_gpio_probe(struct platform_device *dev)
{
- int ret;
+ int i, irq, ret;

- msm_gpio.label = dev->name;
- ret = gpiochip_add(&msm_gpio);
+ 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)
+ return ret;

- return ret;
+ for (i = 0; i < msm_gpio.gpio_chip.ngpio; ++i) {
+ irq = msm_gpio_to_irq(&msm_gpio.gpio_chip, i);
+ set_irq_chip(irq, &msm_gpio_irq_chip);
+ set_irq_handler(irq, handle_level_irq);
+ set_irq_flags(irq, IRQF_VALID);
+ }
+
+ set_irq_chained_handler(TLMM_SCSS_SUMMARY_IRQ,
+ msm_summary_irq_handler);
+ return 0;
}

static int __devexit msm_gpio_remove(struct platform_device *dev)
{
- int ret = gpiochip_remove(&msm_gpio);
+ int ret = gpiochip_remove(&msm_gpio.gpio_chip);

if (ret < 0)
return ret;
@@ -120,12 +387,53 @@ static int __devexit msm_gpio_remove(struct platform_device *dev)
return 0;
}

+#ifdef CONFIG_PM
+static int msm_gpio_suspend_noirq(struct device *dev)
+{
+ unsigned long irq_flags;
+ unsigned long i;
+
+ spin_lock_irqsave(&tlmm_lock, irq_flags);
+ for_each_set_bit(i, msm_gpio.enabled_irqs, NR_GPIO_IRQS) {
+ if (!test_bit(i, msm_gpio.wake_irqs))
+ writel(TARGET_PROC_NONE, GPIO_INTR_CFG_SU(i));
+ }
+ spin_unlock_irqrestore(&tlmm_lock, irq_flags);
+ return 0;
+}
+
+static int msm_gpio_resume_noirq(struct device *dev)
+{
+ unsigned long irq_flags;
+ unsigned long i;
+
+ spin_lock_irqsave(&tlmm_lock, irq_flags);
+ for_each_set_bit(i, msm_gpio.enabled_irqs, NR_GPIO_IRQS)
+ writel(TARGET_PROC_SCORPION, GPIO_INTR_CFG_SU(i));
+ spin_unlock_irqrestore(&tlmm_lock, irq_flags);
+ return 0;
+}
+#else
+#define msm_gpio_suspend_noirq NULL
+#define msm_gpio_resume_noirq NULL
+#endif
+
+static const struct dev_pm_ops msm_gpio_dev_pm_ops = {
+ .suspend_noirq = msm_gpio_suspend_noirq,
+ .resume_noirq = msm_gpio_resume_noirq,
+ .freeze_noirq = msm_gpio_suspend_noirq,
+ .thaw_noirq = msm_gpio_resume_noirq,
+ .poweroff_noirq = msm_gpio_suspend_noirq,
+ .restore_noirq = msm_gpio_resume_noirq,
+};
+
static struct platform_driver msm_gpio_driver = {
.probe = msm_gpio_probe,
.remove = __devexit_p(msm_gpio_remove),
.driver = {
.name = "msmgpio",
.owner = THIS_MODULE,
+ .pm = &msm_gpio_dev_pm_ops,
},
};

--
1.7.0.4

--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

2010-11-18 19:56:46

by Janakiram Sistla

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] msm: gpio: Add v2 gpio support to MSM SoCs.

On Thu, Nov 18, 2010 at 11:52 AM, Gregory Bean <[email protected]> wrote:
> Beginning with the MSM8x60, the hardware block responsible for gpio
> support changes. ?Provide gpiolib support for the new v2 architecture.
>
> Cc: Baruch Siach <[email protected]>
> Cc: Pavan Kondeti <[email protected]>
> Signed-off-by: Gregory Bean <[email protected]>
> ---
> ?v4 - miscellaneous cleanup, per [email protected]
> ?v3 - miscellaneous cleanup, per [email protected]
> ?v2 - miscellaneous cleanup, per [email protected]
>
> ?arch/arm/mach-msm/Makefile ?| ? ?4 +-
> ?arch/arm/mach-msm/gpio-v2.c | ?163 +++++++++++++++++++++++++++++++++++++++++++
[Ram] Can you let me know how this naming convention gpio-v2??
> ?2 files changed, 166 insertions(+), 1 deletions(-)
> ?create mode 100644 arch/arm/mach-msm/gpio-v2.c
>
> diff --git a/arch/arm/mach-msm/Makefile b/arch/arm/mach-msm/Makefile
> index b5a7b07..4a1a7ea 100644
> --- a/arch/arm/mach-msm/Makefile
> +++ b/arch/arm/mach-msm/Makefile
> @@ -28,6 +28,8 @@ obj-$(CONFIG_ARCH_MSM8X60) += board-msm8x60.o
> ?obj-$(CONFIG_ARCH_MSM7X30) += gpiomux-7x30.o 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
> -ifndef CONFIG_MSM_V2_TLMM
> +ifdef CONFIG_MSM_V2_TLMM
> +obj-y ?+= gpio-v2.o
> +else
> ?obj-y ?+= gpio.o
> ?endif
> diff --git a/arch/arm/mach-msm/gpio-v2.c b/arch/arm/mach-msm/gpio-v2.c
> new file mode 100644
> index 0000000..d907af6
> --- /dev/null
> +++ b/arch/arm/mach-msm/gpio-v2.c
> @@ -0,0 +1,163 @@
> +/* 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/gpio.h>
> +#include <linux/io.h>
> +#include <linux/irq.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/spinlock.h>
> +#include <mach/msm_iomap.h>
> +#include "gpiomux.h"
> +
> +/* Bits of interest in the GPIO_IN_OUT register.
> + */
> +enum {
> + ? ? ? GPIO_IN_BIT ?= 0,
> + ? ? ? GPIO_OUT_BIT = 1
> +};
> +
> +/* Bits of interest in the GPIO_CFG register.
> + */
> +enum {
> + ? ? ? GPIO_OE_BIT = 9,
> +};
> +
> +#define GPIO_CONFIG(gpio) ? ? ? ? (MSM_TLMM_BASE + 0x1000 + (0x10 * (gpio)))
> +#define GPIO_IN_OUT(gpio) ? ? ? ? (MSM_TLMM_BASE + 0x1004 + (0x10 * (gpio)))
> +
> +static DEFINE_SPINLOCK(tlmm_lock);
> +
> +static int msm_gpio_get(struct gpio_chip *chip, unsigned offset)
> +{
> + ? ? ? return readl(GPIO_IN_OUT(offset)) & BIT(GPIO_IN_BIT);
> +}
> +
> +static void msm_gpio_set(struct gpio_chip *chip, unsigned offset, int val)
> +{
> + ? ? ? writel(val ? BIT(GPIO_OUT_BIT) : 0, GPIO_IN_OUT(offset));
> +}
> +
> +static int msm_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
> +{
> + ? ? ? unsigned long irq_flags;
> +
> + ? ? ? spin_lock_irqsave(&tlmm_lock, irq_flags);
> + ? ? ? writel(readl(GPIO_CONFIG(offset)) & ~BIT(GPIO_OE_BIT),
> + ? ? ? ? ? ? ? GPIO_CONFIG(offset));
> + ? ? ? spin_unlock_irqrestore(&tlmm_lock, irq_flags);
> + ? ? ? return 0;
> +}
> +
> +static int msm_gpio_direction_output(struct gpio_chip *chip,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? unsigned offset,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? int val)
> +{
> + ? ? ? unsigned long irq_flags;
> +
> + ? ? ? spin_lock_irqsave(&tlmm_lock, irq_flags);
> + ? ? ? msm_gpio_set(chip, offset, val);
> + ? ? ? writel(readl(GPIO_CONFIG(offset)) | BIT(GPIO_OE_BIT),
> + ? ? ? ? ? ? ? GPIO_CONFIG(offset));
> + ? ? ? spin_unlock_irqrestore(&tlmm_lock, irq_flags);
> + ? ? ? return 0;
> +}
> +
> +static int msm_gpio_request(struct gpio_chip *chip, unsigned offset)
> +{
> + ? ? ? return msm_gpiomux_get(chip->base + offset);
> +}
> +
> +static void msm_gpio_free(struct gpio_chip *chip, unsigned offset)
> +{
> + ? ? ? msm_gpiomux_put(chip->base + offset);
> +}
> +
> +static struct gpio_chip msm_gpio = {
> + ? ? ? .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,
> + ? ? ? .request ? ? ? ? ?= msm_gpio_request,
> + ? ? ? .free ? ? ? ? ? ? = msm_gpio_free,
> +};
> +
> +static int __devinit msm_gpio_probe(struct platform_device *dev)
> +{
> + ? ? ? int ret;
> +
> + ? ? ? msm_gpio.label = dev->name;
> + ? ? ? ret = gpiochip_add(&msm_gpio);
> +
> + ? ? ? return ret;
> +}
> +
> +static int __devexit msm_gpio_remove(struct platform_device *dev)
> +{
> + ? ? ? int ret = gpiochip_remove(&msm_gpio);
> +
> + ? ? ? if (ret < 0)
> + ? ? ? ? ? ? ? return ret;
> +
> + ? ? ? set_irq_handler(TLMM_SCSS_SUMMARY_IRQ, NULL);
> +
> + ? ? ? return 0;
> +}
> +
> +static struct platform_driver msm_gpio_driver = {
> + ? ? ? .probe = msm_gpio_probe,
> + ? ? ? .remove = __devexit_p(msm_gpio_remove),
> + ? ? ? .driver = {
> + ? ? ? ? ? ? ? .name = "msmgpio",
> + ? ? ? ? ? ? ? .owner = THIS_MODULE,
> + ? ? ? },
> +};
> +
> +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_AUTHOR("Gregory Bean <[email protected]>");
> +MODULE_DESCRIPTION("Driver for Qualcomm MSM TLMMv2 SoC GPIOs");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:msmgpio");
> --
> 1.7.0.4
>
> --
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at ?http://www.tux.org/lkml/
>

2010-11-18 20:00:00

by Janakiram Sistla

[permalink] [raw]
Subject: Re: [PATCH v4 2/2] msm: gpio: Add irq support to v2 gpiolib.

On Thu, Nov 18, 2010 at 11:52 AM, Gregory Bean <[email protected]> wrote:
> Complete the MSM v2 gpio subsystem by adding irq_chip.
>
> Signed-off-by: Gregory Bean <[email protected]>
> ---
> ?v4 - miscellaneous cleanup to patch 1/2, per [email protected]
> ?v3 - miscellaneous cleanup to patch 1/2, per [email protected]
> ?v2 - miscellaneous cleanup to patch 1/2, per [email protected]
>
> ?arch/arm/mach-msm/gpio-v2.c | ?344 ++++++++++++++++++++++++++++++++++++++++---
[Ram] Do you mean to MSM have v1 and v2 gpio hardware ??? can not we
have one file under mach-msm as gpio.c??
> ?1 files changed, 326 insertions(+), 18 deletions(-)
>
> diff --git a/arch/arm/mach-msm/gpio-v2.c b/arch/arm/mach-msm/gpio-v2.c
> index d907af6..926e3d0 100644
> --- a/arch/arm/mach-msm/gpio-v2.c
> +++ b/arch/arm/mach-msm/gpio-v2.c
> @@ -15,7 +15,11 @@
> ?* 02110-1301, USA.
> ?*
> ?*/
> +#include <linux/bitmap.h>
> +#include <linux/bitops.h>
> ?#include <linux/gpio.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> ?#include <linux/io.h>
> ?#include <linux/irq.h>
> ?#include <linux/module.h>
> @@ -31,17 +35,97 @@ enum {
> ? ? ? ?GPIO_OUT_BIT = 1
> ?};
>
> +/* Bits of interest in the GPIO_INTR_STATUS register.
> + */
> +enum {
> + ? ? ? INTR_STATUS_BIT = 0,
> +};
> +
> ?/* Bits of interest in the GPIO_CFG register.
> ?*/
> ?enum {
> ? ? ? ?GPIO_OE_BIT = 9,
> ?};
>
> +/* Bits of interest in the GPIO_INTR_CFG register.
> + */
> +enum {
> + ? ? ? INTR_ENABLE_BIT ? ? ? ?= 0,
> + ? ? ? INTR_POL_CTL_BIT ? ? ? = 1,
> + ? ? ? INTR_DECT_CTL_BIT ? ? ?= 2,
> + ? ? ? INTR_RAW_STATUS_EN_BIT = 3,
> +};
> +
> +/* Codes of interest in GPIO_INTR_CFG_SU.
> + */
> +enum {
> + ? ? ? TARGET_PROC_SCORPION = 4,
> + ? ? ? TARGET_PROC_NONE ? ? = 7,
> +};
> +
> +/*
> + * When a GPIO triggers, two separate decisions are made, controlled
> + * by two separate flags.
> + *
> + * - First, INTR_RAW_STATUS_EN controls whether or not the GPIO_INTR_STATUS
> + * register for that GPIO will be updated to reflect the triggering of that
> + * gpio. ?If this bit is 0, this register will not be updated.
> + * - Second, INTR_ENABLE controls whether an interrupt is triggered.
> + *
> + * If INTR_ENABLE is set and INTR_RAW_STATUS_EN is NOT set, an interrupt
> + * can be triggered but the status register will not reflect it.
> + */
> +#define INTR_RAW_STATUS_EN BIT(INTR_RAW_STATUS_EN_BIT)
> +#define INTR_ENABLE ? ? ? ?BIT(INTR_ENABLE_BIT)
> +#define INTR_DECT_CTL_EDGE BIT(INTR_DECT_CTL_BIT)
> +#define INTR_POL_CTL_HI ? ?BIT(INTR_POL_CTL_BIT)
> +
> +#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
> + *
> + * @enabled_irqs: a bitmap used to optimize the summary-irq handler. ?By
> + * keeping track of which gpios are unmasked as irq sources, we avoid
> + * having to do readl calls on hundreds of iomapped registers each time
> + * the summary interrupt fires in order to locate the active interrupts.
> + *
> + * @wake_irqs: a bitmap for tracking which interrupt lines are enabled
> + * as wakeup sources. ?When the device is suspended, interrupts which are
> + * not wakeup sources are disabled.
> + *
> + * @dual_edge_irqs: a bitmap used to track which irqs are configured
> + * as dual-edge, as this is not supported by the hardware and requires
> + * some special handling in the driver.
> + */
> +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);
> +};
>
> ?static DEFINE_SPINLOCK(tlmm_lock);
>
> +static inline struct msm_gpio_dev *to_msm_gpio_dev(struct gpio_chip *chip)
> +{
> + ? ? ? return container_of(chip, struct msm_gpio_dev, gpio_chip);
> +}
> +
> +static inline void set_gpio_bits(unsigned n, void __iomem *reg)
> +{
> + ? ? ? writel(readl(reg) | n, reg);
> +}
> +
> +static inline void clr_gpio_bits(unsigned n, void __iomem *reg)
> +{
> + ? ? ? writel(readl(reg) & ~n, reg);
> +}
> +
> ?static int msm_gpio_get(struct gpio_chip *chip, unsigned offset)
> ?{
> ? ? ? ?return readl(GPIO_IN_OUT(offset)) & BIT(GPIO_IN_BIT);
> @@ -57,8 +141,7 @@ static int msm_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
> ? ? ? ?unsigned long irq_flags;
>
> ? ? ? ?spin_lock_irqsave(&tlmm_lock, irq_flags);
> - ? ? ? writel(readl(GPIO_CONFIG(offset)) & ~BIT(GPIO_OE_BIT),
> - ? ? ? ? ? ? ? GPIO_CONFIG(offset));
> + ? ? ? clr_gpio_bits(BIT(GPIO_OE_BIT), GPIO_CONFIG(offset));
> ? ? ? ?spin_unlock_irqrestore(&tlmm_lock, irq_flags);
> ? ? ? ?return 0;
> ?}
> @@ -71,8 +154,7 @@ static int msm_gpio_direction_output(struct gpio_chip *chip,
>
> ? ? ? ?spin_lock_irqsave(&tlmm_lock, irq_flags);
> ? ? ? ?msm_gpio_set(chip, offset, val);
> - ? ? ? writel(readl(GPIO_CONFIG(offset)) | BIT(GPIO_OE_BIT),
> - ? ? ? ? ? ? ? GPIO_CONFIG(offset));
> + ? ? ? set_gpio_bits(BIT(GPIO_OE_BIT), GPIO_CONFIG(offset));
> ? ? ? ?spin_unlock_irqrestore(&tlmm_lock, irq_flags);
> ? ? ? ?return 0;
> ?}
> @@ -87,30 +169,215 @@ static void msm_gpio_free(struct gpio_chip *chip, unsigned offset)
> ? ? ? ?msm_gpiomux_put(chip->base + offset);
> ?}
>
> -static struct gpio_chip msm_gpio = {
> - ? ? ? .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,
> - ? ? ? .request ? ? ? ? ?= msm_gpio_request,
> - ? ? ? .free ? ? ? ? ? ? = msm_gpio_free,
> +static int msm_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
> +{
> + ? ? ? return MSM_GPIO_TO_INT(offset - chip->base);
> +}
> +
> +static inline int msm_irq_to_gpio(struct gpio_chip *chip, unsigned irq)
> +{
> + ? ? ? return irq - MSM_GPIO_TO_INT(chip->base);
> +}
> +
> +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:
> + *
> + * At appropriate moments, this function may be called to flip the polarity
> + * settings of both-edge irq lines to try and catch the next edge.
> + *
> + * The attempt is considered successful if:
> + * - the status bit goes high, indicating that an edge was caught, or
> + * - the input value of the gpio doesn't change during the attempt.
> + * If the value changes twice during the process, that would cause the first
> + * test to fail but would force the second, as two opposite
> + * transitions would cause a detection no matter the polarity setting.
> + *
> + * The do-loop tries to sledge-hammer closed the timing hole between
> + * the initial value-read and the polarity-write - if the line value changes
> + * during that window, an interrupt is lost, the new polarity setting is
> + * incorrect, and the first success test will fail, causing a retry.
> + *
> + * Algorithm comes from Google's msmgpio driver, see mach-msm/gpio.c.
> + */
> +static void msm_gpio_update_dual_edge_pos(unsigned gpio)
> +{
> + ? ? ? int loop_limit = 100;
> + ? ? ? unsigned val, val2, intstat;
> +
> + ? ? ? do {
> + ? ? ? ? ? ? ? val = readl(GPIO_IN_OUT(gpio)) & BIT(GPIO_IN_BIT);
> + ? ? ? ? ? ? ? if (val)
> + ? ? ? ? ? ? ? ? ? ? ? clr_gpio_bits(INTR_POL_CTL_HI, GPIO_INTR_CFG(gpio));
> + ? ? ? ? ? ? ? else
> + ? ? ? ? ? ? ? ? ? ? ? set_gpio_bits(INTR_POL_CTL_HI, GPIO_INTR_CFG(gpio));
> + ? ? ? ? ? ? ? val2 = readl(GPIO_IN_OUT(gpio)) & BIT(GPIO_IN_BIT);
> + ? ? ? ? ? ? ? intstat = readl(GPIO_INTR_STATUS(gpio)) & BIT(INTR_STATUS_BIT);
> + ? ? ? ? ? ? ? if (intstat || val == val2)
> + ? ? ? ? ? ? ? ? ? ? ? return;
> + ? ? ? } while (loop_limit-- > 0);
> + ? ? ? pr_err("%s: dual-edge irq failed to stabilize, "
> + ? ? ? ? ? ? ?"interrupts dropped. %#08x != %#08x\n",
> + ? ? ? ? ? ? ?__func__, val, val2);
> +}
> +
> +static void msm_gpio_irq_ack(unsigned int irq)
> +{
> + ? ? ? int gpio = msm_irq_to_gpio(&msm_gpio.gpio_chip, irq);
> +
> + ? ? ? writel(BIT(INTR_STATUS_BIT), GPIO_INTR_STATUS(gpio));
> + ? ? ? if (test_bit(gpio, msm_gpio.dual_edge_irqs))
> + ? ? ? ? ? ? ? msm_gpio_update_dual_edge_pos(gpio);
> +}
> +
> +static void msm_gpio_irq_mask(unsigned int irq)
> +{
> + ? ? ? int gpio = msm_irq_to_gpio(&msm_gpio.gpio_chip, irq);
> + ? ? ? unsigned long irq_flags;
> +
> + ? ? ? spin_lock_irqsave(&tlmm_lock, irq_flags);
> + ? ? ? writel(TARGET_PROC_NONE, GPIO_INTR_CFG_SU(gpio));
> + ? ? ? clr_gpio_bits(INTR_RAW_STATUS_EN | INTR_ENABLE, GPIO_INTR_CFG(gpio));
> + ? ? ? __clear_bit(gpio, msm_gpio.enabled_irqs);
> + ? ? ? spin_unlock_irqrestore(&tlmm_lock, irq_flags);
> +}
> +
> +static void msm_gpio_irq_unmask(unsigned int irq)
> +{
> + ? ? ? int gpio = msm_irq_to_gpio(&msm_gpio.gpio_chip, irq);
> + ? ? ? unsigned long irq_flags;
> +
> + ? ? ? spin_lock_irqsave(&tlmm_lock, irq_flags);
> + ? ? ? __set_bit(gpio, msm_gpio.enabled_irqs);
> + ? ? ? set_gpio_bits(INTR_RAW_STATUS_EN | INTR_ENABLE, GPIO_INTR_CFG(gpio));
> + ? ? ? writel(TARGET_PROC_SCORPION, GPIO_INTR_CFG_SU(gpio));
> + ? ? ? spin_unlock_irqrestore(&tlmm_lock, irq_flags);
> +}
> +
> +static int msm_gpio_irq_set_type(unsigned int irq, unsigned int flow_type)
> +{
> + ? ? ? int gpio = msm_irq_to_gpio(&msm_gpio.gpio_chip, irq);
> + ? ? ? unsigned long irq_flags;
> + ? ? ? uint32_t bits;
> +
> + ? ? ? spin_lock_irqsave(&tlmm_lock, irq_flags);
> +
> + ? ? ? bits = readl(GPIO_INTR_CFG(gpio));
> +
> + ? ? ? if (flow_type & IRQ_TYPE_EDGE_BOTH) {
> + ? ? ? ? ? ? ? bits |= INTR_DECT_CTL_EDGE;
> + ? ? ? ? ? ? ? irq_desc[irq].handle_irq = handle_edge_irq;
> + ? ? ? ? ? ? ? if ((flow_type & IRQ_TYPE_EDGE_BOTH) == IRQ_TYPE_EDGE_BOTH)
> + ? ? ? ? ? ? ? ? ? ? ? __set_bit(gpio, msm_gpio.dual_edge_irqs);
> + ? ? ? ? ? ? ? else
> + ? ? ? ? ? ? ? ? ? ? ? __clear_bit(gpio, msm_gpio.dual_edge_irqs);
> + ? ? ? } else {
> + ? ? ? ? ? ? ? bits &= ~INTR_DECT_CTL_EDGE;
> + ? ? ? ? ? ? ? irq_desc[irq].handle_irq = handle_level_irq;
> + ? ? ? ? ? ? ? __clear_bit(gpio, msm_gpio.dual_edge_irqs);
> + ? ? ? }
> +
> + ? ? ? if (flow_type & (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_LEVEL_HIGH))
> + ? ? ? ? ? ? ? bits |= INTR_POL_CTL_HI;
> + ? ? ? else
> + ? ? ? ? ? ? ? bits &= ~INTR_POL_CTL_HI;
> +
> + ? ? ? writel(bits, GPIO_INTR_CFG(gpio));
> +
> + ? ? ? if ((flow_type & IRQ_TYPE_EDGE_BOTH) == IRQ_TYPE_EDGE_BOTH)
> + ? ? ? ? ? ? ? msm_gpio_update_dual_edge_pos(gpio);
> +
> + ? ? ? spin_unlock_irqrestore(&tlmm_lock, irq_flags);
> +
> + ? ? ? return 0;
> +}
> +
> +/*
> + * When the summary IRQ is raised, any number of GPIO lines may be high.
> + * It is the job of the summary handler to find all those GPIO lines
> + * which have been set as summary IRQ lines and which are triggered,
> + * and to call their interrupt handlers.
> + */
> +static void msm_summary_irq_handler(unsigned int irq, struct irq_desc *desc)
> +{
> + ? ? ? unsigned long i;
> +
> + ? ? ? for (i = find_first_bit(msm_gpio.enabled_irqs, NR_GPIO_IRQS);
> + ? ? ? ? ? ?i < NR_GPIO_IRQS;
> + ? ? ? ? ? ?i = find_next_bit(msm_gpio.enabled_irqs, NR_GPIO_IRQS, i + 1)) {
> + ? ? ? ? ? ? ? if (readl(GPIO_INTR_STATUS(i)) & BIT(INTR_STATUS_BIT))
> + ? ? ? ? ? ? ? ? ? ? ? generic_handle_irq(msm_gpio_to_irq(&msm_gpio.gpio_chip,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?i));
> + ? ? ? }
> + ? ? ? desc->chip->ack(irq);
> +}
> +
> +static int msm_gpio_irq_set_wake(unsigned int irq, unsigned int on)
> +{
> + ? ? ? int gpio = msm_irq_to_gpio(&msm_gpio.gpio_chip, irq);
> +
> + ? ? ? if (on) {
> + ? ? ? ? ? ? ? if (bitmap_empty(msm_gpio.wake_irqs, NR_GPIO_IRQS))
> + ? ? ? ? ? ? ? ? ? ? ? set_irq_wake(TLMM_SCSS_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))
> + ? ? ? ? ? ? ? ? ? ? ? set_irq_wake(TLMM_SCSS_SUMMARY_IRQ, 0);
> + ? ? ? }
> +
> + ? ? ? return 0;
> +}
> +
> +static struct irq_chip msm_gpio_irq_chip = {
> + ? ? ? .name ? ? ? ? ? = "msmgpio",
> + ? ? ? .mask ? ? ? ? ? = msm_gpio_irq_mask,
> + ? ? ? .unmask ? ? ? ? = msm_gpio_irq_unmask,
> + ? ? ? .ack ? ? ? ? ? ?= msm_gpio_irq_ack,
> + ? ? ? .set_type ? ? ? = msm_gpio_irq_set_type,
> + ? ? ? .set_wake ? ? ? = msm_gpio_irq_set_wake,
> ?};
>
> ?static int __devinit msm_gpio_probe(struct platform_device *dev)
> ?{
> - ? ? ? int ret;
> + ? ? ? int i, irq, ret;
>
> - ? ? ? msm_gpio.label = dev->name;
> - ? ? ? ret = gpiochip_add(&msm_gpio);
> + ? ? ? 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)
> + ? ? ? ? ? ? ? return ret;
>
> - ? ? ? return ret;
> + ? ? ? for (i = 0; i < msm_gpio.gpio_chip.ngpio; ++i) {
> + ? ? ? ? ? ? ? irq = msm_gpio_to_irq(&msm_gpio.gpio_chip, i);
> + ? ? ? ? ? ? ? set_irq_chip(irq, &msm_gpio_irq_chip);
> + ? ? ? ? ? ? ? set_irq_handler(irq, handle_level_irq);
> + ? ? ? ? ? ? ? set_irq_flags(irq, IRQF_VALID);
> + ? ? ? }
> +
> + ? ? ? set_irq_chained_handler(TLMM_SCSS_SUMMARY_IRQ,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? msm_summary_irq_handler);
> + ? ? ? return 0;
> ?}
>
> ?static int __devexit msm_gpio_remove(struct platform_device *dev)
> ?{
> - ? ? ? int ret = gpiochip_remove(&msm_gpio);
> + ? ? ? int ret = gpiochip_remove(&msm_gpio.gpio_chip);
>
> ? ? ? ?if (ret < 0)
> ? ? ? ? ? ? ? ?return ret;
> @@ -120,12 +387,53 @@ static int __devexit msm_gpio_remove(struct platform_device *dev)
> ? ? ? ?return 0;
> ?}
>
> +#ifdef CONFIG_PM
> +static int msm_gpio_suspend_noirq(struct device *dev)
> +{
> + ? ? ? unsigned long irq_flags;
> + ? ? ? unsigned long i;
> +
> + ? ? ? spin_lock_irqsave(&tlmm_lock, irq_flags);
> + ? ? ? for_each_set_bit(i, msm_gpio.enabled_irqs, NR_GPIO_IRQS) {
> + ? ? ? ? ? ? ? if (!test_bit(i, msm_gpio.wake_irqs))
> + ? ? ? ? ? ? ? ? ? ? ? writel(TARGET_PROC_NONE, GPIO_INTR_CFG_SU(i));
> + ? ? ? }
> + ? ? ? spin_unlock_irqrestore(&tlmm_lock, irq_flags);
> + ? ? ? return 0;
> +}
> +
> +static int msm_gpio_resume_noirq(struct device *dev)
> +{
> + ? ? ? unsigned long irq_flags;
> + ? ? ? unsigned long i;
> +
> + ? ? ? spin_lock_irqsave(&tlmm_lock, irq_flags);
> + ? ? ? for_each_set_bit(i, msm_gpio.enabled_irqs, NR_GPIO_IRQS)
> + ? ? ? ? ? ? ? writel(TARGET_PROC_SCORPION, GPIO_INTR_CFG_SU(i));
> + ? ? ? spin_unlock_irqrestore(&tlmm_lock, irq_flags);
> + ? ? ? return 0;
> +}
> +#else
> +#define msm_gpio_suspend_noirq NULL
> +#define msm_gpio_resume_noirq NULL
> +#endif
> +
> +static const struct dev_pm_ops msm_gpio_dev_pm_ops = {
> + ? ? ? .suspend_noirq ?= msm_gpio_suspend_noirq,
> + ? ? ? .resume_noirq ? = msm_gpio_resume_noirq,
> + ? ? ? .freeze_noirq ? = msm_gpio_suspend_noirq,
> + ? ? ? .thaw_noirq ? ? = msm_gpio_resume_noirq,
> + ? ? ? .poweroff_noirq = msm_gpio_suspend_noirq,
> + ? ? ? .restore_noirq ?= msm_gpio_resume_noirq,
> +};
> +
> ?static struct platform_driver msm_gpio_driver = {
> ? ? ? ?.probe = msm_gpio_probe,
> ? ? ? ?.remove = __devexit_p(msm_gpio_remove),
> ? ? ? ?.driver = {
> ? ? ? ? ? ? ? ?.name = "msmgpio",
> ? ? ? ? ? ? ? ?.owner = THIS_MODULE,
> + ? ? ? ? ? ? ? .pm = &msm_gpio_dev_pm_ops,
> ? ? ? ?},
> ?};
>
> --
> 1.7.0.4
>
> --
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at ?http://www.tux.org/lkml/
>

2010-11-18 20:30:59

by Gregory Bean

[permalink] [raw]
Subject: Re: [PATCH v4 2/2] msm: gpio: Add irq support to v2 gpiolib.

On Thu, 18 Nov 2010, Janakiram Sistla wrote:

> Can you let me know how this naming convention gpio-v2??
>
> Do you mean to MSM have v1 and v2 gpio hardware ??? can not we
> have one file under mach-msm as gpio.c??

The MSM8x60 is the first MSM chip to carry the second-generation, or v2,
GPIO hardware block. This block is not backward compatible with the
v1 block found on older MSMs, so the two drivers cannot be elegantly merged.

--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

2010-11-18 20:39:35

by Janakiram Sistla

[permalink] [raw]
Subject: Re: [PATCH v4 2/2] msm: gpio: Add irq support to v2 gpiolib.

On Thu, Nov 18, 2010 at 12:30 PM, Gregory Bean <[email protected]> wrote:
> On Thu, 18 Nov 2010, Janakiram Sistla wrote:
>
>> Can you let me know how this naming convention gpio-v2??
>>
>> Do you mean to MSM have v1 and v2 gpio hardware ??? can not we
>> have one file under mach-msm as gpio.c??
>
> The MSM8x60 is the first MSM chip to carry the second-generation, or v2,
> GPIO hardware block. ?This block is not backward compatible with the
> v1 block found on older MSMs, so the two drivers cannot be elegantly merged.
so this is applicable to 8x60 and coming chip sets.I basically have a
question,How come a a new person under stand the chipset uses gpio-v1
or gpiov2??
> --
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
>

2010-11-18 20:59:14

by Gregory Bean

[permalink] [raw]
Subject: Re: [PATCH v4 2/2] msm: gpio: Add irq support to v2 gpiolib.

>>> Do you mean to MSM have v1 and v2 gpio hardware ??? can not we
>>> have one file under mach-msm as gpio.c??
>>
>> The MSM8x60 is the first MSM chip to carry the second-generation, or v2,
>> GPIO hardware block. ?This block is not backward compatible with the
>> v1 block found on older MSMs, so the two drivers cannot be elegantly merged.
>
> so this is applicable to 8x60 and coming chip sets.I basically have a
> question,How come a a new person under stand the chipset uses gpio-v1
> or gpiov2??

Targets with the v2 block define CONFIG_MSM_V2_TLMM.

--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

2010-11-18 22:02:13

by Janakiram Sistla

[permalink] [raw]
Subject: Re: [PATCH v4 2/2] msm: gpio: Add irq support to v2 gpiolib.

On Thu, Nov 18, 2010 at 12:59 PM, Gregory Bean <[email protected]> wrote:
>>>> Do you mean to MSM have v1 and v2 gpio hardware ??? can not we
>>>> have one file under mach-msm as gpio.c??
>>>
>>> The MSM8x60 is the first MSM chip to carry the second-generation, or v2,
>>> GPIO hardware block. ?This block is not backward compatible with the
>>> v1 block found on older MSMs, so the two drivers cannot be elegantly
>>> merged.
>>
>> so this is applicable to 8x60 and coming chip sets.I basically have a
>> question,How come a a new person under stand the ?chipset uses gpio-v1
>> or gpiov2??
>
> Targets with the v2 block define CONFIG_MSM_V2_TLMM.
So in that case we can name it as gpio-tlmm.c ??
> --
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

2010-11-19 18:37:49

by Gregory Bean

[permalink] [raw]
Subject: Re: [PATCH v4 2/2] msm: gpio: Add irq support to v2 gpiolib.

On Thu, 18 Nov 2010, Janakiram Sistla wrote:

>> Targets with the v2 block define CONFIG_MSM_V2_TLMM.
>
> So in that case we can name it as gpio-tlmm.c ??

No, not and have things be any clearer.

All MSM chips have a TLMM block. The older chips have
version 1 (v1) of the TLMM block. The newer chips have v2.

--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

2010-11-19 18:49:49

by Janakiram Sistla

[permalink] [raw]
Subject: Re: [PATCH v4 2/2] msm: gpio: Add irq support to v2 gpiolib.

On Fri, Nov 19, 2010 at 10:37 AM, Gregory Bean <[email protected]> wrote:
> On Thu, 18 Nov 2010, Janakiram Sistla wrote:
>
>>> Targets with the v2 block define CONFIG_MSM_V2_TLMM.
>>
>> So in that case we can name ?it as gpio-tlmm.c ??
>
> No, not and have things be any clearer.
>
> All MSM chips have a TLMM block. ?The older chips have
> version 1 (v1) of the TLMM block. ?The newer chips have v2.
I think that we should differentiate the new gpio infrastructure with
appropriate naming that makes the diff between v1 and v2
> --
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
>

2010-11-19 19:00:36

by Gregory Bean

[permalink] [raw]
Subject: Re: [PATCH v4 2/2] msm: gpio: Add irq support to v2 gpiolib.

>>>> Targets with the v2 block define CONFIG_MSM_V2_TLMM.
>>>
>>> So in that case we can name ?it as gpio-tlmm.c ??
>>
>> No, not and have things be any clearer.
>>
>> All MSM chips have a TLMM block. ?The older chips have
>> version 1 (v1) of the TLMM block. ?The newer chips have v2.
>
> I think that we should differentiate the new gpio infrastructure with
> appropriate naming that makes the diff between v1 and v2

What do you suggest? All MSM SoCs have a TLMM block. The block itself
carries no model or revision name, and is not tied specifically to any
particular SoC, except for the fact that the second generation of this block
happened to appear at the same time as the MSM8x60.

gpio.c is already in wide use for v1 systems, and is well-establised
in the android mainlines. Changing without extremely good cause would not
go pleasantly.

Since gpio.c is v1, we used gpio-v2.c for v2.

--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

2010-11-19 19:09:09

by Janakiram Sistla

[permalink] [raw]
Subject: Re: [PATCH v4 2/2] msm: gpio: Add irq support to v2 gpiolib.

On Fri, Nov 19, 2010 at 11:00 AM, Gregory Bean <[email protected]> wrote:
>>>>> Targets with the v2 block define CONFIG_MSM_V2_TLMM.
>>>>
>>>> So in that case we can name ?it as gpio-tlmm.c ??
>>>
>>> No, not and have things be any clearer.
>>>
>>> All MSM chips have a TLMM block. ?The older chips have
>>> version 1 (v1) of the TLMM block. ?The newer chips have v2.
>>
>> I think that we should differentiate the new gpio infrastructure with
>> appropriate naming that makes the diff between v1 and v2
>
> What do you suggest? ?All MSM SoCs have a TLMM block. ?The block itself
> carries no model or revision name, and is not tied specifically to any
> particular SoC, except for the fact that the second generation of this block
> happened to appear at the same time as the MSM8x60.
>
> gpio.c is already in wide use for v1 systems, and is well-establised
> in the android mainlines. ?Changing without extremely good cause would not
> go pleasantly.
>
> Since gpio.c is v1, we used gpio-v2.c for v2.
Greg, i agree your point ,but i think v2 differs from v1 with a new
features distinguished.Also there might be a intention behind going
for v2 that might resolve some hardware erratas in v1.
what if again a new generation comes then we will name it gpio-v3.c ??
>
> --
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

2010-11-19 20:27:27

by Gregory Bean

[permalink] [raw]
Subject: Re: [PATCH v4 2/2] msm: gpio: Add irq support to v2 gpiolib.

>> Since gpio.c is v1, we used gpio-v2.c for v2.
>
> Greg, i agree your point ,but i think v2 differs from v1 with a new
> features distinguished.Also there might be a intention behind going
> for v2 that might resolve some hardware erratas in v1.

V2 is not an extension to or erratas to v1, it is almost
completely different. Any erratas to v1 would be released as v1.1 (I suspect).

> what if again a new generation comes then we will name it gpio-v3.c ??

I assume so.

--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

2010-11-23 17:52:05

by Daniel Walker

[permalink] [raw]
Subject: Re: [PATCH v4 2/2] msm: gpio: Add irq support to v2 gpiolib.

On Thu, 2010-11-18 at 11:52 -0800, Gregory Bean wrote:
> Complete the MSM v2 gpio subsystem by adding irq_chip.
>

It looks like your actually doing more than what stated here. Like your
adding pm related structure which shouldn't be needed for just the
irq_chip.

more comments below,

> Signed-off-by: Gregory Bean <[email protected]>
> ---
> v4 - miscellaneous cleanup to patch 1/2, per [email protected]
> v3 - miscellaneous cleanup to patch 1/2, per [email protected]
> v2 - miscellaneous cleanup to patch 1/2, per [email protected]
>
> arch/arm/mach-msm/gpio-v2.c | 344 ++++++++++++++++++++++++++++++++++++++++---
> 1 files changed, 326 insertions(+), 18 deletions(-)
>
> diff --git a/arch/arm/mach-msm/gpio-v2.c b/arch/arm/mach-msm/gpio-v2.c
> index d907af6..926e3d0 100644
> --- a/arch/arm/mach-msm/gpio-v2.c
> +++ b/arch/arm/mach-msm/gpio-v2.c
> @@ -15,7 +15,11 @@
> * 02110-1301, USA.
> *
> */
> +#include <linux/bitmap.h>
> +#include <linux/bitops.h>
> #include <linux/gpio.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> #include <linux/io.h>
> #include <linux/irq.h>
> #include <linux/module.h>
> @@ -31,17 +35,97 @@ enum {
> GPIO_OUT_BIT = 1
> };
>
> +/* Bits of interest in the GPIO_INTR_STATUS register.
> + */
> +enum {
> + INTR_STATUS_BIT = 0,
> +};
> +
> /* Bits of interest in the GPIO_CFG register.
> */
> enum {
> GPIO_OE_BIT = 9,
> };
>
> +/* Bits of interest in the GPIO_INTR_CFG register.
> + */
> +enum {
> + INTR_ENABLE_BIT = 0,
> + INTR_POL_CTL_BIT = 1,
> + INTR_DECT_CTL_BIT = 2,
> + INTR_RAW_STATUS_EN_BIT = 3,
> +};
> +
> +/* Codes of interest in GPIO_INTR_CFG_SU.
> + */
> +enum {
> + TARGET_PROC_SCORPION = 4,
> + TARGET_PROC_NONE = 7,
> +};
> +
> +/*
> + * When a GPIO triggers, two separate decisions are made, controlled
> + * by two separate flags.
> + *
> + * - First, INTR_RAW_STATUS_EN controls whether or not the GPIO_INTR_STATUS
> + * register for that GPIO will be updated to reflect the triggering of that
> + * gpio. If this bit is 0, this register will not be updated.
> + * - Second, INTR_ENABLE controls whether an interrupt is triggered.
> + *
> + * If INTR_ENABLE is set and INTR_RAW_STATUS_EN is NOT set, an interrupt
> + * can be triggered but the status register will not reflect it.
> + */
> +#define INTR_RAW_STATUS_EN BIT(INTR_RAW_STATUS_EN_BIT)
> +#define INTR_ENABLE BIT(INTR_ENABLE_BIT)
> +#define INTR_DECT_CTL_EDGE BIT(INTR_DECT_CTL_BIT)
> +#define INTR_POL_CTL_HI BIT(INTR_POL_CTL_BIT

This is a little bit inconsistent .. Your using BIT() in this case, but
there are other cases where you define which bit and use BIT() in the
code.

> +#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
> + *
> + * @enabled_irqs: a bitmap used to optimize the summary-irq handler. By
> + * keeping track of which gpios are unmasked as irq sources, we avoid
> + * having to do readl calls on hundreds of iomapped registers each time
> + * the summary interrupt fires in order to locate the active interrupts.
> + *
> + * @wake_irqs: a bitmap for tracking which interrupt lines are enabled
> + * as wakeup sources. When the device is suspended, interrupts which are
> + * not wakeup sources are disabled.
> + *
> + * @dual_edge_irqs: a bitmap used to track which irqs are configured
> + * as dual-edge, as this is not supported by the hardware and requires
> + * some special handling in the driver.
> + */
> +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);
> +};
>
> static DEFINE_SPINLOCK(tlmm_lock);
>
> +static inline struct msm_gpio_dev *to_msm_gpio_dev(struct gpio_chip *chip)
> +{
> + return container_of(chip, struct msm_gpio_dev, gpio_chip);
> +}
> +
> +static inline void set_gpio_bits(unsigned n, void __iomem *reg)
> +{
> + writel(readl(reg) | n, reg);
> +}
> +
> +static inline void clr_gpio_bits(unsigned n, void __iomem *reg)
> +{
> + writel(readl(reg) & ~n, reg);
> +}

It seems these functions actually accept output from BIT(). It would be
safer to force these to accept the bit number then use BIT() inside this
function to translate. That way you wouldn't use "unsigned n" for the
argument you would use a named enum for the argument.

> static int msm_gpio_get(struct gpio_chip *chip, unsigned offset)
> {
> return readl(GPIO_IN_OUT(offset)) & BIT(GPIO_IN_BIT);
> @@ -57,8 +141,7 @@ static int msm_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
> unsigned long irq_flags;
>
> spin_lock_irqsave(&tlmm_lock, irq_flags);
> - writel(readl(GPIO_CONFIG(offset)) & ~BIT(GPIO_OE_BIT),
> - GPIO_CONFIG(offset));
> + clr_gpio_bits(BIT(GPIO_OE_BIT), GPIO_CONFIG(offset));

so in this case you just send in GPIO_OE_BIT , but you also need
GPIO_OE_BIT to be part of the same enum as all the other bits sent to
clr_gpio_bits/set_gpio_bits. I'd just use "clear_gpio_bits" for the
name, doesn't look like there's a good reason to shorten it.

> spin_unlock_irqrestore(&tlmm_lock, irq_flags);
> return 0;
> }
> @@ -71,8 +154,7 @@ static int msm_gpio_direction_output(struct gpio_chip *chip,
>
> spin_lock_irqsave(&tlmm_lock, irq_flags);
> msm_gpio_set(chip, offset, val);
> - writel(readl(GPIO_CONFIG(offset)) | BIT(GPIO_OE_BIT),
> - GPIO_CONFIG(offset));
> + set_gpio_bits(BIT(GPIO_OE_BIT), GPIO_CONFIG(offset));
> spin_unlock_irqrestore(&tlmm_lock, irq_flags);
> return 0;
> }
> @@ -87,30 +169,215 @@ static void msm_gpio_free(struct gpio_chip *chip, unsigned offset)
> msm_gpiomux_put(chip->base + offset);
> }
>
> -static struct gpio_chip msm_gpio = {
> - .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,
> - .request = msm_gpio_request,
> - .free = msm_gpio_free,
> +static int msm_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
> +{
> + return MSM_GPIO_TO_INT(offset - chip->base);
> +}

Not chip->base + offset?

> +
> +static inline int msm_irq_to_gpio(struct gpio_chip *chip, unsigned irq)
> +{
> + return irq - MSM_GPIO_TO_INT(chip->base);
> +}
> +
> +static struct msm_gpio_dev msm_gpio = {
> + .gpio_chip = {
> + .base = 0,

I guess it's fine to do "offset - chip->base" if base is always zero,
but why do subtraction at all.

> + .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:
> + *
> + * At appropriate moments, this function may be called to flip the polarity
> + * settings of both-edge irq lines to try and catch the next edge.
> + *
> + * The attempt is considered successful if:
> + * - the status bit goes high, indicating that an edge was caught, or
> + * - the input value of the gpio doesn't change during the attempt.
> + * If the value changes twice during the process, that would cause the first
> + * test to fail but would force the second, as two opposite
> + * transitions would cause a detection no matter the polarity setting.
> + *
> + * The do-loop tries to sledge-hammer closed the timing hole between
> + * the initial value-read and the polarity-write - if the line value changes
> + * during that window, an interrupt is lost, the new polarity setting is
> + * incorrect, and the first success test will fail, causing a retry.
> + *
> + * Algorithm comes from Google's msmgpio driver, see mach-msm/gpio.c.
> + */
> +static void msm_gpio_update_dual_edge_pos(unsigned gpio)
> +{
> + int loop_limit = 100;
> + unsigned val, val2, intstat;
> +
> + do {
> + val = readl(GPIO_IN_OUT(gpio)) & BIT(GPIO_IN_BIT);
> + if (val)
> + clr_gpio_bits(INTR_POL_CTL_HI, GPIO_INTR_CFG(gpio));
> + else
> + set_gpio_bits(INTR_POL_CTL_HI, GPIO_INTR_CFG(gpio));
> + val2 = readl(GPIO_IN_OUT(gpio)) & BIT(GPIO_IN_BIT);
> + intstat = readl(GPIO_INTR_STATUS(gpio)) & BIT(INTR_STATUS_BIT);
> + if (intstat || val == val2)
> + return;
> + } while (loop_limit-- > 0);
> + pr_err("%s: dual-edge irq failed to stabilize, "
> + "interrupts dropped. %#08x != %#08x\n",
> + __func__, val, val2);

You could set pr_fmt so it automatically adds the __func__ prefix.
Assuming you planned on adding more of these.

> +}
> +
> +static void msm_gpio_irq_ack(unsigned int irq)
> +{
> + int gpio = msm_irq_to_gpio(&msm_gpio.gpio_chip, irq);
> +
> + writel(BIT(INTR_STATUS_BIT), GPIO_INTR_STATUS(gpio));
> + if (test_bit(gpio, msm_gpio.dual_edge_irqs))
> + msm_gpio_update_dual_edge_pos(gpio);
> +}
> +
> +static void msm_gpio_irq_mask(unsigned int irq)
> +{
> + int gpio = msm_irq_to_gpio(&msm_gpio.gpio_chip, irq);
> + unsigned long irq_flags;
> +
> + spin_lock_irqsave(&tlmm_lock, irq_flags);
> + writel(TARGET_PROC_NONE, GPIO_INTR_CFG_SU(gpio));
> + clr_gpio_bits(INTR_RAW_STATUS_EN | INTR_ENABLE, GPIO_INTR_CFG(gpio));
> + __clear_bit(gpio, msm_gpio.enabled_irqs);
> + spin_unlock_irqrestore(&tlmm_lock, irq_flags);
> +}
> +
> +static void msm_gpio_irq_unmask(unsigned int irq)
> +{
> + int gpio = msm_irq_to_gpio(&msm_gpio.gpio_chip, irq);
> + unsigned long irq_flags;
> +
> + spin_lock_irqsave(&tlmm_lock, irq_flags);
> + __set_bit(gpio, msm_gpio.enabled_irqs);
> + set_gpio_bits(INTR_RAW_STATUS_EN | INTR_ENABLE, GPIO_INTR_CFG(gpio));

I's just break this into two calls, or make another helper that to set
that accepts the mask and have set_gpio_bits call that. This here you
would just use the other helper. like set_gpio_bits calls
set_gpio_bits_mask() and you call the mask version here.

> + writel(TARGET_PROC_SCORPION, GPIO_INTR_CFG_SU(gpio));
> + spin_unlock_irqrestore(&tlmm_lock, irq_flags);
> +}
> +
> +static int msm_gpio_irq_set_type(unsigned int irq, unsigned int flow_type)
> +{
> + int gpio = msm_irq_to_gpio(&msm_gpio.gpio_chip, irq);
> + unsigned long irq_flags;
> + uint32_t bits;
> +
> + spin_lock_irqsave(&tlmm_lock, irq_flags);
> +
> + bits = readl(GPIO_INTR_CFG(gpio));
> +
> + if (flow_type & IRQ_TYPE_EDGE_BOTH) {
> + bits |= INTR_DECT_CTL_EDGE;
> + irq_desc[irq].handle_irq = handle_edge_irq;
> + if ((flow_type & IRQ_TYPE_EDGE_BOTH) == IRQ_TYPE_EDGE_BOTH)
> + __set_bit(gpio, msm_gpio.dual_edge_irqs);
> + else
> + __clear_bit(gpio, msm_gpio.dual_edge_irqs);
> + } else {
> + bits &= ~INTR_DECT_CTL_EDGE;
> + irq_desc[irq].handle_irq = handle_level_irq;
> + __clear_bit(gpio, msm_gpio.dual_edge_irqs);
> + }
> +
> + if (flow_type & (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_LEVEL_HIGH))
> + bits |= INTR_POL_CTL_HI;
> + else
> + bits &= ~INTR_POL_CTL_HI;
> +
> + writel(bits, GPIO_INTR_CFG(gpio));
> +
> + if ((flow_type & IRQ_TYPE_EDGE_BOTH) == IRQ_TYPE_EDGE_BOTH)
> + msm_gpio_update_dual_edge_pos(gpio);
> +
> + spin_unlock_irqrestore(&tlmm_lock, irq_flags);
> +
> + return 0;
> +}
> +
> +/*
> + * When the summary IRQ is raised, any number of GPIO lines may be high.
> + * It is the job of the summary handler to find all those GPIO lines
> + * which have been set as summary IRQ lines and which are triggered,
> + * and to call their interrupt handlers.
> + */
> +static void msm_summary_irq_handler(unsigned int irq, struct irq_desc *desc)
> +{
> + unsigned long i;
> +
> + for (i = find_first_bit(msm_gpio.enabled_irqs, NR_GPIO_IRQS);
> + i < NR_GPIO_IRQS;
> + i = find_next_bit(msm_gpio.enabled_irqs, NR_GPIO_IRQS, i + 1)) {
> + if (readl(GPIO_INTR_STATUS(i)) & BIT(INTR_STATUS_BIT))
> + generic_handle_irq(msm_gpio_to_irq(&msm_gpio.gpio_chip,
> + i));
> + }
> + desc->chip->ack(irq);
> +}
> +
> +static int msm_gpio_irq_set_wake(unsigned int irq, unsigned int on)
> +{
> + int gpio = msm_irq_to_gpio(&msm_gpio.gpio_chip, irq);
> +
> + if (on) {
> + if (bitmap_empty(msm_gpio.wake_irqs, NR_GPIO_IRQS))
> + set_irq_wake(TLMM_SCSS_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))
> + set_irq_wake(TLMM_SCSS_SUMMARY_IRQ, 0);
> + }
> +
> + return 0;
> +}
> +
> +static struct irq_chip msm_gpio_irq_chip = {
> + .name = "msmgpio",
> + .mask = msm_gpio_irq_mask,
> + .unmask = msm_gpio_irq_unmask,
> + .ack = msm_gpio_irq_ack,
> + .set_type = msm_gpio_irq_set_type,
> + .set_wake = msm_gpio_irq_set_wake,
> };
>
> static int __devinit msm_gpio_probe(struct platform_device *dev)
> {
> - int ret;
> + int i, irq, ret;
>
> - msm_gpio.label = dev->name;
> - ret = gpiochip_add(&msm_gpio);
> + 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)
> + return ret;
>
> - return ret;
> + for (i = 0; i < msm_gpio.gpio_chip.ngpio; ++i) {
> + irq = msm_gpio_to_irq(&msm_gpio.gpio_chip, i);
> + set_irq_chip(irq, &msm_gpio_irq_chip);
> + set_irq_handler(irq, handle_level_irq);
> + set_irq_flags(irq, IRQF_VALID);
> + }
> +
> + set_irq_chained_handler(TLMM_SCSS_SUMMARY_IRQ,
> + msm_summary_irq_handler);
> + return 0;
> }
>
> static int __devexit msm_gpio_remove(struct platform_device *dev)
> {
> - int ret = gpiochip_remove(&msm_gpio);
> + int ret = gpiochip_remove(&msm_gpio.gpio_chip);
>
> if (ret < 0)
> return ret;
> @@ -120,12 +387,53 @@ static int __devexit msm_gpio_remove(struct platform_device *dev)
> return 0;
> }
>
> +#ifdef CONFIG_PM

Then this I'd do a whole other patch for. Can this get used yet tho?

Daniel

--
Sent by an consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum.

2010-11-23 23:08:09

by Gregory Bean

[permalink] [raw]
Subject: Re: [PATCH v4 2/2] msm: gpio: Add irq support to v2 gpiolib.

>> +static inline void set_gpio_bits(unsigned n, void __iomem *reg)
>> +{
>> + writel(readl(reg) | n, reg);
>> +}
>> +
>> +static inline void clr_gpio_bits(unsigned n, void __iomem *reg)
>> +{
>> + writel(readl(reg) & ~n, reg);
>> +}
>
> It seems these functions actually accept output from BIT(). It would be
> safer to force these to accept the bit number then use BIT() inside this
> function to translate. That way you wouldn't use "unsigned n" for the
> argument you would use a named enum for the argument.

I don't think that will work well, because there are cases where we want
to set or clear more than one bit at a time. Making these functions
take a bit number as an argument would restrict them to setting or clearing
only one bit at a time, forcing users to call them multiple times to set
or clear more than one bit, meaning lots of readl & writel calls for
compount bit-changes.

>> +static struct msm_gpio_dev msm_gpio = {
>> + .gpio_chip = {
>> + .base = 0,
>
> I guess it's fine to do "offset - chip->base" if base is always zero,
> but why do subtraction at all.

If the chip is ever moved, not accounting for the base would produce an error.
I know that 'speculative coding' is frowned upon, but isn't removing an
addition (as you pointed out, the subtraction is a bug) because this instance
of the chip is at offset zero a little over the top?

>> + set_gpio_bits(INTR_RAW_STATUS_EN | INTR_ENABLE, GPIO_INTR_CFG(gpio));
>
> I's just break this into two calls, or make another helper that to set
> that accepts the mask and have set_gpio_bits call that. This here you
> would just use the other helper. like set_gpio_bits calls
> set_gpio_bits_mask() and you call the mask version here.

Why make two readl/writel call pairs, or have one version of a helper which
can set a single bit and another version which can set more than one
at a time? That seems really complicated.

--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

2010-11-24 00:44:33

by Daniel Walker

[permalink] [raw]
Subject: Re: [PATCH v4 2/2] msm: gpio: Add irq support to v2 gpiolib.

On Tue, 2010-11-23 at 15:08 -0800, Gregory Bean wrote:
> >> +static inline void set_gpio_bits(unsigned n, void __iomem *reg)
> >> +{
> >> + writel(readl(reg) | n, reg);
> >> +}
> >> +
> >> +static inline void clr_gpio_bits(unsigned n, void __iomem *reg)
> >> +{
> >> + writel(readl(reg) & ~n, reg);
> >> +}
> >
> > It seems these functions actually accept output from BIT(). It would be
> > safer to force these to accept the bit number then use BIT() inside this
> > function to translate. That way you wouldn't use "unsigned n" for the
> > argument you would use a named enum for the argument.
>
> I don't think that will work well, because there are cases where we want
> to set or clear more than one bit at a time. Making these functions
> take a bit number as an argument would restrict them to setting or clearing
> only one bit at a time, forcing users to call them multiple times to set
> or clear more than one bit, meaning lots of readl & writel calls for
> compount bit-changes.

How often will you do the multiple bit changes tho?

> >> +static struct msm_gpio_dev msm_gpio = {
> >> + .gpio_chip = {
> >> + .base = 0,
> >
> > I guess it's fine to do "offset - chip->base" if base is always zero,
> > but why do subtraction at all.
>
> If the chip is ever moved, not accounting for the base would produce an error.
> I know that 'speculative coding' is frowned upon, but isn't removing an
> addition (as you pointed out, the subtraction is a bug) because this instance
> of the chip is at offset zero a little over the top?

Up to you what to do here .. I don't care if you have it be redundant
addition, you might want to comment on it tho.

> >> + set_gpio_bits(INTR_RAW_STATUS_EN | INTR_ENABLE, GPIO_INTR_CFG(gpio));
> >
> > I's just break this into two calls, or make another helper that to set
> > that accepts the mask and have set_gpio_bits call that. This here you
> > would just use the other helper. like set_gpio_bits calls
> > set_gpio_bits_mask() and you call the mask version here.
>
> Why make two readl/writel call pairs, or have one version of a helper which
> can set a single bit and another version which can set more than one
> at a time? That seems really complicated.

The benefit is that you would have type checking on the values sent in.
You would get that from using the enum to define the argument. With just
using unsigned int you can send in any value you want (even invalid
ones).

Daniel

--
Sent by an consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum.