Skip to contentSkip to conversation

arm: msm: Add gpiolib support for MSM7200A-family chips.

PATCH In review Gregory Bean 2 participants 6 messages Jun 16, 2010 – Jun 17, 2010

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

cover arm: msm: Add gpiolib support for MSM7200A-family chips.

Hi folks:

Here is a proposed driver which adds gpio and irq support to all currently-supported arm-msm SoCs. Feedback is greatly appreciated. This is the second submission after it was decided that this code belongs in mach-msm instead of the generic drivers/gpio. It did receive some reviewing while submitted there. For completeness, I include those notes here as well:

First-submission revision history:
v2: Cleaned up some housekeeping noise in probe and remove methods.
v3: Incorporated feedback from [email protected].
v4: Incorporated feedback from [email protected].
v5: Incorporated feedback from [email protected], moved code
for resubmission.

Gregory Bean (2):
arm: msm: Add gpiolib support for MSM7200A-family chips.
arm: msm: Add irq support to msm7200a-gpio.

 arch/arm/mach-msm/Kconfig         |    7 +
 arch/arm/mach-msm/Makefile        |    2 +-
 arch/arm/mach-msm/msm7200a-gpio.c |  353 +++++++++++++++++++++++++++++++++++++
 arch/arm/mach-msm/msm7200a-gpio.h |   87 +++++++++
 4 files changed, 448 insertions(+), 1 deletions(-)
 create mode 100644 arch/arm/mach-msm/msm7200a-gpio.c
 create mode 100644 arch/arm/mach-msm/msm7200a-gpio.h

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

#1 lore raw

PATCH 1/2 arm: msm: Add gpiolib support for MSM7200A-family chips.

Add support for uniprocessor MSM chips whose TLMM/GPIO design is the same as the MSM7200A.
This includes, but is not necessarily limited to, the:
MSM7200A, MSM7x25, MSM7x27, MSM7x30, QSD8x50, QSD8x50A

---
 arch/arm/mach-msm/Kconfig         |    7 ++
 arch/arm/mach-msm/Makefile        |    2 +-
 arch/arm/mach-msm/msm7200a-gpio.c |  187 +++++++++++++++++++++++++++++++++++++
 arch/arm/mach-msm/msm7200a-gpio.h |   62 ++++++++++++
 4 files changed, 257 insertions(+), 1 deletions(-)
 create mode 100644 arch/arm/mach-msm/msm7200a-gpio.c
 create mode 100644 arch/arm/mach-msm/msm7200a-gpio.h
arch/arm/mach-msm/Kconfig +7 -0
index 47264a7..d919b90 100644
@@ -106,4 +106,11 @@ config MSM_SMD_PKG3
 config MSM_SMD
 	bool
 
+config MSM7200A_GPIO
+	tristate "Qualcomm MSM7200A SoC GPIO support"
+	depends on GPIOLIB
+	help
+	  Say yes here to support GPIO functionality on Qualcomm's
+	  MSM chipsets which descend from the MSM7200a:
+	  MSM7x01(a), MSM7x25, MSM7x27, MSM7x30, QSD8x50(a).
 endif
arch/arm/mach-msm/Makefile +1 -1
index 66677f0..50af063 100644
@@ -19,4 +19,4 @@ obj-$(CONFIG_MACH_TROUT) += board-trout.o devices-msm7x00.o
 obj-$(CONFIG_MACH_HALIBUT) += board-halibut.o devices-msm7x00.o
 obj-$(CONFIG_ARCH_MSM7X30) += board-msm7x30.o devices-msm7x30.o
 obj-$(CONFIG_ARCH_QSD8X50) += board-qsd8x50.o devices-qsd8x50.o
-
+obj-$(CONFIG_MSM7200A_GPIO) += msm7200a-gpio.o
arch/arm/mach-msm/msm7200a-gpio.c new file +187 -0

187 lines not on this page · raw message

arch/arm/mach-msm/msm7200a-gpio.h new file +62 -0

62 lines not on this page · raw message

Signature · 5 lines
1.7.0.4

--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
#2 lore raw
Quoted 20 lines · Gregory Bean

Add support for uniprocessor MSM chips whose TLMM/GPIO design is the same as the MSM7200A.
This includes, but is not necessarily limited to, the:
MSM7200A, MSM7x25, MSM7x27, MSM7x30, QSD8x50, QSD8x50A

---
 arch/arm/mach-msm/Kconfig         |    7 ++
 arch/arm/mach-msm/Makefile        |    2 +-
 arch/arm/mach-msm/msm7200a-gpio.c |  187 +++++++++++++++++++++++++++++++++++++
 arch/arm/mach-msm/msm7200a-gpio.h |   62 ++++++++++++
 4 files changed, 257 insertions(+), 1 deletions(-)
 create mode 100644 arch/arm/mach-msm/msm7200a-gpio.c
 create mode 100644 arch/arm/mach-msm/msm7200a-gpio.h
arch/arm/mach-msm/Kconfig +0 -0
index 47264a7..d919b90 100644

arch/arm/mach-msm/Kconfig config MSM_SMD_PKG3 · line 115

Quoted 10 lines
arch/arm/mach-msm/Kconfig +7 -0
@@ -106,4 +106,11 @@ config MSM_SMD_PKG3
 config MSM_SMD
 	bool
 
+config MSM7200A_GPIO
+	tristate "Qualcomm MSM7200A SoC GPIO support"
+	depends on GPIOLIB
+	help
+	  Say yes here to support GPIO functionality on Qualcomm's
+	  MSM chipsets which descend from the MSM7200a:
+	  MSM7x01(a), MSM7x25, MSM7x27, MSM7x30, QSD8x50(a).

Can't we drop this? It's always going to be on right ?

Daniel

author
#3 lore raw
Quoted 9 lines

arch/arm/mach-msm/Kconfig config MSM_SMD_PKG3 · line 115

Quoted 7 lines
arch/arm/mach-msm/Kconfig +7 -0
+config MSM7200A_GPIO
+	tristate "Qualcomm MSM7200A SoC GPIO support"
+	depends on GPIOLIB
+	help
+	  Say yes here to support GPIO functionality on Qualcomm's
+	  MSM chipsets which descend from the MSM7200a:
+	  MSM7x01(a), MSM7x25, MSM7x27, MSM7x30, QSD8x50(a).

Can't we drop this? It's always going to be on right ?

Not on all targets. There will be other drivers for chips from other lines. However, I would think that it could be entirely reasonable to lose the Kconfig and just hard-wire it in based on ARCH, if you like. Is that preferred?

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

#4 lore raw
Quoted 10 lines · Greg Bean
Quoted 9 lines

arch/arm/mach-msm/Kconfig config MSM_SMD_PKG3 · line 115

Quoted 7 lines
arch/arm/mach-msm/Kconfig +7 -0
+config MSM7200A_GPIO
+	tristate "Qualcomm MSM7200A SoC GPIO support"
+	depends on GPIOLIB
+	help
+	  Say yes here to support GPIO functionality on Qualcomm's
+	  MSM chipsets which descend from the MSM7200a:
+	  MSM7x01(a), MSM7x25, MSM7x27, MSM7x30, QSD8x50(a).

Can't we drop this? It's always going to be on right ?

Quoted 4 lines

Not on all targets. There will be other drivers for chips from other lines. However, I would think that it could be entirely reasonable to lose the Kconfig and just hard-wire it in based on ARCH, if you like. Is that preferred?

Yeah, it's better not to have Kconfig options like this since it's easier for the user if we just encode this into options we know they have to select already.

So for example they pick 8x50 via Kconfig, so we select for them the right stuff for 8x50 .. Otherwise we have to assume they know what to select which may not be the case, plus it could be a pain to go selecting all the related Kconfig options for low level stuff.

If you respin this please re-send it privately just to me.

Daniel

#5 lore raw

PATCH 2/2 arm: msm: Add irq support to msm7200a-gpio.

In reply to Gregory Bean
---
 arch/arm/mach-msm/msm7200a-gpio.c |  168 ++++++++++++++++++++++++++++++++++++-
 arch/arm/mach-msm/msm7200a-gpio.h |   25 ++++++
 2 files changed, 192 insertions(+), 1 deletions(-)
arch/arm/mach-msm/msm7200a-gpio.c +167 -1
index d24768d..1f9d53c 100644
@@ -23,6 +23,8 @@
 #include <linux/kernel.h>
 #include <linux/gpio.h>
 #include <linux/io.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
@@ -31,6 +33,9 @@
 struct msm_gpio_dev {
 	struct gpio_chip		gpio_chip;
 	spinlock_t			lock;
+	unsigned			irq_base;
+	unsigned			irq_summary;
+	bool				latch_level_irqs;
 	struct msm7200a_gpio_regs	regs;
 };
 
@@ -110,11 +115,130 @@ static void gpio_chip_set(struct gpio_chip *chip, unsigned offset, int value)
 	spin_unlock_irqrestore(&msm_gpio->lock, irq_flags);
 }
 
+static int gpio_chip_to_irq(struct gpio_chip *chip, unsigned offset)
+{
+	struct msm_gpio_dev *msm_gpio = to_msm_gpio_dev(chip);
+	return msm_gpio->irq_base + offset;
+}
+
+static void forget_level_irq(struct msm_gpio_dev *msm_gpio, unsigned offset)
+{
+	if (!msm_gpio->latch_level_irqs) {
+		unsigned v = readl(msm_gpio->regs.int_edge);
+		unsigned b = BIT(offset);
+
+		if (!(v & b))
+			writel(b, msm_gpio->regs.int_clear);
+	}
+}
+
+static void msm_gpio_irq_mask(unsigned int irq)
+{
+	unsigned long irq_flags;
+	struct msm_gpio_dev *msm_gpio = get_irq_chip_data(irq);
+	unsigned offset = irq - msm_gpio->irq_base;
+
+	spin_lock_irqsave(&msm_gpio->lock, irq_flags);
+	forget_level_irq(msm_gpio, offset);
+	clr_gpio_bit(offset, msm_gpio->regs.int_en);
+	spin_unlock_irqrestore(&msm_gpio->lock, irq_flags);
+}
+
+static void msm_gpio_irq_unmask(unsigned int irq)
+{
+	unsigned long irq_flags;
+	struct msm_gpio_dev *msm_gpio = get_irq_chip_data(irq);
+	unsigned offset = irq - msm_gpio->irq_base;
+
+	spin_lock_irqsave(&msm_gpio->lock, irq_flags);
+	forget_level_irq(msm_gpio, offset);
+	set_gpio_bit(offset, msm_gpio->regs.int_en);
+	spin_unlock_irqrestore(&msm_gpio->lock, irq_flags);
+}
+
+static int msm_gpio_irq_set_type(unsigned int irq, unsigned int flow_type)
+{
+	unsigned long irq_flags;
+	struct msm_gpio_dev *msm_gpio = get_irq_chip_data(irq);
+	unsigned offset = irq - msm_gpio->irq_base;
+
+	if ((flow_type & IRQ_TYPE_EDGE_BOTH) ==	IRQ_TYPE_EDGE_BOTH)
+		return -EINVAL;
+
+	if ((flow_type & (IRQF_TRIGGER_HIGH | IRQF_TRIGGER_LOW)) ==
+		(IRQF_TRIGGER_HIGH | IRQF_TRIGGER_LOW))
+		return -EINVAL;
+
+	spin_lock_irqsave(&msm_gpio->lock, irq_flags);
+
+	if (flow_type & IRQ_TYPE_EDGE_BOTH) {
+		set_gpio_bit(offset, msm_gpio->regs.int_edge);
+		irq_desc[irq].handle_irq = handle_edge_irq;
+	} else {
+		clr_gpio_bit(offset, msm_gpio->regs.int_edge);
+		irq_desc[irq].handle_irq = handle_level_irq;
+	}
+
+	if (flow_type & (IRQF_TRIGGER_HIGH | IRQF_TRIGGER_RISING))
+		set_gpio_bit(offset, msm_gpio->regs.int_pos);
+	else
+		clr_gpio_bit(offset, msm_gpio->regs.int_pos);
+
+	spin_unlock_irqrestore(&msm_gpio->lock, irq_flags);
+
+	return 0;
+}
+
+static void msm_gpio_irq_mask_ack(unsigned int irq)
+{
+	msm_gpio_irq_mask(irq);
+}
+
+static irqreturn_t msm_gpio_irq_handler(int irq, void *dev)
+{
+	struct msm_gpio_dev *msm_gpio = dev;
+	unsigned e, s, triggered_irqs;
+	int b;
+
+	/*
+	 * The int_status register latches trigger events whether or not
+	 * the gpio line is enabled as an interrupt source.  Therefore,
+	 * the set of pins which defines the interrupts which need to fire
+	 * is the intersection of int_status and int_en - int_status
+	 * alone provides an incomplete picture.
+	 */
+	spin_lock(&msm_gpio->lock);
+	s = readl(msm_gpio->regs.int_status);
+	e = readl(msm_gpio->regs.int_en);
+	triggered_irqs = s & e;
+	if (triggered_irqs)
+		writel(triggered_irqs, msm_gpio->regs.int_clear);
+	spin_unlock(&msm_gpio->lock);
+
+	if (!triggered_irqs)
+		return IRQ_NONE;
+
+	while (triggered_irqs) {
+		b = ffs(triggered_irqs) - 1;
+		triggered_irqs &= ~BIT(b);
+		generic_handle_irq(msm_gpio->irq_base + b);
+	}
+	return IRQ_HANDLED;
+}
+
+static struct irq_chip msm_gpio_irq_chip = {
+	.name			= "msm_gpio",
+	.mask			= msm_gpio_irq_mask,
+	.mask_ack		= msm_gpio_irq_mask_ack,
+	.unmask			= msm_gpio_irq_unmask,
+	.set_type		= msm_gpio_irq_set_type,
+};
+
 static int __devinit msm_gpio_probe(struct platform_device *dev)
 {
 	struct msm_gpio_dev *msm_gpio;
 	struct msm7200a_gpio_platform_data *pdata = dev->dev.platform_data;
-	int ret;
+	int i, irq, ret;
 
 	if (!pdata)
 		return -EINVAL;
@@ -134,12 +258,53 @@ static int __devinit msm_gpio_probe(struct platform_device *dev)
 	msm_gpio->gpio_chip.direction_output = gpio_chip_direction_output;
 	msm_gpio->gpio_chip.get              = gpio_chip_get;
 	msm_gpio->gpio_chip.set              = gpio_chip_set;
+	msm_gpio->gpio_chip.to_irq           = gpio_chip_to_irq;
+	msm_gpio->irq_base                   = pdata->irq_base;
+	msm_gpio->irq_summary                = pdata->irq_summary;
+	msm_gpio->latch_level_irqs           = pdata->latch_level_irqs;
 
 	ret = gpiochip_add(&msm_gpio->gpio_chip);
 	if (ret < 0)
 		goto err_post_malloc;
 
+	for (i = 0; i < msm_gpio->gpio_chip.ngpio; ++i) {
+		irq = msm_gpio->irq_base + i;
+		set_irq_chip_data(irq, msm_gpio);
+		set_irq_chip(irq, &msm_gpio_irq_chip);
+		set_irq_handler(irq, handle_level_irq);
+		set_irq_flags(irq, IRQF_VALID);
+	}
+
+	/*
+	 * We use a level-triggered interrupt because of the nature
+	 * of the shared GPIO-group interrupt.
+	 *
+	 * Many GPIO chips may be sharing the same group IRQ line, and
+	 * it is possible for GPIO interrupt to re-occur while the system
+	 * is still servicing the group interrupt associated with it.
+	 * The group IRQ line would not de-assert and re-assert, and
+	 * we'd get no second edge to cause the group IRQ to be handled again.
+	 *
+	 * Using a level interrupt guarantees that the group IRQ handlers
+	 * will continue to be called as long as any GPIO chip in the group
+	 * is asserting, even if the condition began while the group
+	 * handler was in mid-pass.
+	 */
+	ret = request_irq(msm_gpio->irq_summary,
+			  msm_gpio_irq_handler,
+			  IRQF_SHARED | IRQF_TRIGGER_HIGH,
+			  dev_name(&dev->dev),
+			  msm_gpio);
+	if (ret < 0)
+		goto err_post_gpiochip_add;
+
 	return ret;
+err_post_gpiochip_add:
+	/*
+	 * Under no circumstances should a line be held on a gpiochip
+	 * which hasn't finished probing.
+	 */
+	BUG_ON(gpiochip_remove(&msm_gpio->gpio_chip) < 0);
 err_post_malloc:
 	platform_set_drvdata(dev, NULL);
 	kfree(msm_gpio);
@@ -154,6 +319,7 @@ static int __devexit msm_gpio_remove(struct platform_device *dev)
 	if (ret < 0)
 		return ret;
 
+	free_irq(msm_gpio->irq_summary, msm_gpio);
 	kfree(msm_gpio);
 
 	return 0;
arch/arm/mach-msm/msm7200a-gpio.h +25 -0
index c44d16b..317c1fd 100644
@@ -34,6 +34,11 @@
  * @in:		GPIO_IN_n
  * @out:	GPIO_OUT_n
  * @oe:		GPIO_OE_n
+ * @int_status:	GPIO_INT_STATUS_n
+ * @int_clear:	GPIO_INT_CLEAR_n
+ * @int_en:	GPIO_INT_EN_n
+ * @int_edge:	GPIO_INT_EDGE_n
+ * @int_pos:	GPIO_INT_POS_n
  *
  * Registers are not guaranteed to be packed in memory, or even
  * located in a predictable pattern.
@@ -42,6 +47,11 @@ struct msm7200a_gpio_regs {
 	void __iomem *in;
 	void __iomem *out;
 	void __iomem *oe;
+	void __iomem *int_status;
+	void __iomem *int_clear;
+	void __iomem *int_en;
+	void __iomem *int_edge;
+	void __iomem *int_pos;
 };
 
 /**
@@ -50,12 +60,27 @@ struct msm7200a_gpio_regs {
  *		directly to gpio_chip.base.
  * @ngpio:	The number of gpio lines to be managed by the device.
  *		Must be <= 32.  Corresponds directly to gpio_chip.ngpio.
+ * @irq_base:	The first irq to be assigned to the device.  The gpio
+ *		at 'gpio_base' will be assigned irq 'irq_base',
+ *		gpio 'gpio_base + 1' will receive irq 'irq_base + 1',
+ *		and so on.
+ * @irq_summary:	The summary irq line which will be used by the device
+ *			to notify the kernel when an interrupt occurs on
+ *			any of its gpio lines.  Most MSM SoCs have more
+ *			than one gpio device sharing each of these.
+ * @latch_level_irqs:	The MSM gpio hardware latches level interrupts,
+ *			which is atypical.  Setting this flag to false
+ *			makes the driver compensate for this and produce
+ *			the traditional unlatched behavior for level irqs.
  * @regs:	Addresses of the registers which control the gpios
  *		to be managed by the device.
  */
 struct msm7200a_gpio_platform_data {
 	unsigned gpio_base;
 	unsigned ngpio;
+	unsigned irq_base;
+	unsigned irq_summary;
+	bool latch_level_irqs;
 	struct msm7200a_gpio_regs regs;
 };
 
Signature · 5 lines
1.7.0.4

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