linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/3] dt-bindings: gpio: Add Spreadtrum EIC controller documentation
@ 2018-02-24 10:44 Baolin Wang
  2018-02-24 10:44 ` [PATCH v2 2/3] gpio: Add Spreadtrum EIC driver support Baolin Wang
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Baolin Wang @ 2018-02-24 10:44 UTC (permalink / raw)
  To: linus.walleij, robh+dt, mark.rutland
  Cc: devicetree, linux-kernel, linux-gpio, broonie, baolin.wang,
	andy.shevchenko

This patch adds the device tree bindings for the Spreadtrum EIC
controller. The EIC can be seen as a special type of GPIO, which
can only be used as input mode.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
Changes since v1:
 - Fix some typos and grammar issues.
 - Add more explanation to make things clear.
 - List all device nodes as examples.
---
 .../devicetree/bindings/gpio/gpio-eic-sprd.txt     |   97 ++++++++++++++++++++
 1 file changed, 97 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-eic-sprd.txt

diff --git a/Documentation/devicetree/bindings/gpio/gpio-eic-sprd.txt b/Documentation/devicetree/bindings/gpio/gpio-eic-sprd.txt
new file mode 100644
index 0000000..93d98d0
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-eic-sprd.txt
@@ -0,0 +1,97 @@
+Spreadtrum EIC controller bindings
+
+The EIC is the abbreviation of external interrupt controller, which can
+be used only in input mode. The Spreadtrum platform has 2 EIC controllers,
+one is in digital chip, and another one is in PMIC. The digital chip EIC
+controller contains 4 sub-modules: EIC-debounce, EIC-latch, EIC-async and
+EIC-sync. But the PMIC EIC controller contains only one EIC-debounce sub-
+module.
+
+The EIC-debounce sub-module provides up to 8 source input signal
+connections. A debounce mechanism is used to capture the input signals'
+stable status (millisecond resolution) and a single-trigger mechanism
+is introduced into this sub-module to enhance the input event detection
+reliability. In addition, this sub-module's clock can be shut off
+automatically to reduce power dissipation. Moreover the debounce range
+is from 1ms to 4s with a step size of 1ms. The input signal will be
+ignored if it is asserted for less than 1 ms.
+
+The EIC-latch sub-module is used to latch some special power down signals
+and generate interrupts, since the EIC-latch does not depend on the APB
+clock to capture signals.
+
+The EIC-async sub-module uses a 32kHz clock to capture the short signals
+(microsecond resolution) to generate interrupts by level or edge trigger.
+
+The EIC-sync is similar with GPIO's input function, which is a synchronized
+signal input register. It can generate interrupts by level or edge trigger
+when detecting input signals.
+
+Required properties:
+- compatible: Should be one of the following:
+  "sprd,sc9860-eic-debounce",
+  "sprd,sc9860-eic-latch",
+  "sprd,sc9860-eic-async",
+  "sprd,sc9860-eic-sync",
+  "sprd,sc27xx-eic".
+- reg: Define the base and range of the I/O address space containing
+  the GPIO controller registers.
+- gpio-controller: Marks the device node as a GPIO controller.
+- #gpio-cells: Should be <2>. The first cell is the gpio number and
+  the second cell is used to specify optional parameters.
+- interrupt-controller: Marks the device node as an interrupt controller.
+- #interrupt-cells: Should be <2>. Specifies the number of cells needed
+  to encode interrupt source.
+- interrupts: Should be the port interrupt shared by all the gpios.
+
+Example:
+	eic_debounce: gpio@40210000 {
+		compatible = "sprd,sc9860-eic-debounce";
+		reg = <0 0x40210000 0 0x80>;
+		gpio-controller;
+		#gpio-cells = <2>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+		interrupts = <GIC_SPI 52 IRQ_TYPE_LEVEL_HIGH>;
+	};
+
+	eic_latch: gpio@40210080 {
+		compatible = "sprd,sc9860-eic-latch";
+		reg = <0 0x40210080 0 0x20>;
+		gpio-controller;
+		#gpio-cells = <2>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+		interrupts = <GIC_SPI 52 IRQ_TYPE_LEVEL_HIGH>;
+	};
+
+	eic_async: gpio@402100a0 {
+		compatible = "sprd,sc9860-eic-async";
+		reg = <0 0x402100a0 0 0x20>;
+		gpio-controller;
+		#gpio-cells = <2>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+		interrupts = <GIC_SPI 52 IRQ_TYPE_LEVEL_HIGH>;
+	};
+
+	eic_sync: gpio@402100c0 {
+		compatible = "sprd,sc9860-eic-sync";
+		reg = <0 0x402100c0 0 0x20>;
+		gpio-controller;
+		#gpio-cells = <2>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+		interrupts = <GIC_SPI 52 IRQ_TYPE_LEVEL_HIGH>;
+	};
+
+	pmic_eic: gpio@300 {
+		compatible = "sprd,sc27xx-eic";
+		reg = <0x300>;
+		interrupt-parent = <&sc2731_pmic>;
+		interrupts = <5 IRQ_TYPE_LEVEL_HIGH>;
+		gpio-controller;
+		#gpio-cells = <2>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+	};
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 2/3] gpio: Add Spreadtrum EIC driver support
  2018-02-24 10:44 [PATCH v2 1/3] dt-bindings: gpio: Add Spreadtrum EIC controller documentation Baolin Wang
@ 2018-02-24 10:44 ` Baolin Wang
  2018-03-02  9:53   ` Linus Walleij
  2018-02-24 10:44 ` [PATCH v2 3/3] gpio: Add Spreadtrum PMIC " Baolin Wang
  2018-03-02 22:08 ` [PATCH v2 1/3] dt-bindings: gpio: Add Spreadtrum EIC controller documentation Rob Herring
  2 siblings, 1 reply; 13+ messages in thread
From: Baolin Wang @ 2018-02-24 10:44 UTC (permalink / raw)
  To: linus.walleij, robh+dt, mark.rutland
  Cc: devicetree, linux-kernel, linux-gpio, broonie, baolin.wang,
	andy.shevchenko

The Spreadtrum digital-chip EIC controller has 4 sub-modules: debounce EIC,
latch EIC, async EIC and sync EIC, and each sub-module can has multiple
banks and each bank contains 8 EICs.

Each EIC can only be used as input mode, and has the capability to trigger
interrupts when detecting input signals.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
Changes since v1:
 - Add OF_GPIO dependency.
 - Fix some typos and grammar issues.
 - Simplify the sprd_eic_update() and sprd_eic_read().
 - Rename 'sprd_eic_data' to 'sprd_eic_variant_data'.
 - Rename sprd_eic_group_base() to sprd_eic_offset_base().
 - Split the PMIC EIC support into one separate driver.
 - Return -EIO for sprd_eic_direction_input().
 - Other optimization.
---
 drivers/gpio/Kconfig         |    8 +
 drivers/gpio/Makefile        |    1 +
 drivers/gpio/gpio-eic-sprd.c |  606 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 615 insertions(+)
 create mode 100644 drivers/gpio/gpio-eic-sprd.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index a64b29d..c937407 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -159,6 +159,14 @@ config GPIO_DWAPB
 	  Say Y or M here to build support for the Synopsys DesignWare APB
 	  GPIO block.
 
+config GPIO_EIC_SPRD
+	tristate "Spreadtrum EIC support"
+	depends on ARCH_SPRD || COMPILE_TEST
+	depends on OF_GPIO
+	select GPIOLIB_IRQCHIP
+	help
+	  Say yes here to support Spreadtrum EIC device.
+
 config GPIO_EM
 	tristate "Emma Mobile GPIO"
 	depends on (ARCH_EMEV2 || COMPILE_TEST) && OF_GPIO
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 2e3d320..66eaa9e 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -44,6 +44,7 @@ obj-$(CONFIG_GPIO_DA9055)	+= gpio-da9055.o
 obj-$(CONFIG_GPIO_DAVINCI)	+= gpio-davinci.o
 obj-$(CONFIG_GPIO_DLN2)		+= gpio-dln2.o
 obj-$(CONFIG_GPIO_DWAPB)	+= gpio-dwapb.o
+obj-$(CONFIG_GPIO_EIC_SPRD)	+= gpio-eic-sprd.o
 obj-$(CONFIG_GPIO_EM)		+= gpio-em.o
 obj-$(CONFIG_GPIO_EP93XX)	+= gpio-ep93xx.o
 obj-$(CONFIG_GPIO_ETRAXFS)	+= gpio-etraxfs.o
diff --git a/drivers/gpio/gpio-eic-sprd.c b/drivers/gpio/gpio-eic-sprd.c
new file mode 100644
index 0000000..ce00455e
--- /dev/null
+++ b/drivers/gpio/gpio-eic-sprd.c
@@ -0,0 +1,606 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Spreadtrum Communications Inc.
+ * Copyright (C) 2018 Linaro Ltd.
+ */
+
+#include <linux/bitops.h>
+#include <linux/gpio/driver.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+
+/* EIC registers definition */
+#define SPRD_EIC_DBNC_DATA		0x0
+#define SPRD_EIC_DBNC_DMSK		0x4
+#define SPRD_EIC_DBNC_IEV		0x14
+#define SPRD_EIC_DBNC_IE		0x18
+#define SPRD_EIC_DBNC_RIS		0x1c
+#define SPRD_EIC_DBNC_MIS		0x20
+#define SPRD_EIC_DBNC_IC		0x24
+#define SPRD_EIC_DBNC_TRIG		0x28
+#define SPRD_EIC_DBNC_CTRL0		0x40
+
+#define SPRD_EIC_LATCH_INTEN		0x0
+#define SPRD_EIC_LATCH_INTRAW		0x4
+#define SPRD_EIC_LATCH_INTMSK		0x8
+#define SPRD_EIC_LATCH_INTCLR		0xc
+#define SPRD_EIC_LATCH_INTPOL		0x10
+#define SPRD_EIC_LATCH_INTMODE		0x14
+
+#define SPRD_EIC_ASYNC_INTIE		0x0
+#define SPRD_EIC_ASYNC_INTRAW		0x4
+#define SPRD_EIC_ASYNC_INTMSK		0x8
+#define SPRD_EIC_ASYNC_INTCLR		0xc
+#define SPRD_EIC_ASYNC_INTMODE		0x10
+#define SPRD_EIC_ASYNC_INTBOTH		0x14
+#define SPRD_EIC_ASYNC_INTPOL		0x18
+#define SPRD_EIC_ASYNC_DATA		0x1c
+
+#define SPRD_EIC_SYNC_INTIE		0x0
+#define SPRD_EIC_SYNC_INTRAW		0x4
+#define SPRD_EIC_SYNC_INTMSK		0x8
+#define SPRD_EIC_SYNC_INTCLR		0xc
+#define SPRD_EIC_SYNC_INTMODE		0x10
+#define SPRD_EIC_SYNC_INTBOTH		0x14
+#define SPRD_EIC_SYNC_INTPOL		0x18
+#define SPRD_EIC_SYNC_DATA		0x1c
+
+/*
+ * The digital-chip EIC controller can support maximum 3 banks, and each bank
+ * contains 8 EICs.
+ */
+#define SPRD_EIC_MAX_BANK		3
+#define SPRD_EIC_PER_BANK_NR		8
+#define SPRD_EIC_DATA_MASK		GENMASK(7, 0)
+#define SPRD_EIC_BIT(x)			((x) & (SPRD_EIC_PER_BANK_NR - 1))
+#define SPRD_EIC_DBNC_MASK		GENMASK(11, 0)
+
+/*
+ * The Spreadtrum EIC (external interrupt controller) can be used only in
+ * input mode to generate interrupts if detecting input signals.
+ *
+ * The Spreadtrum digital-chip EIC controller contains 4 sub-modules:
+ * debounce EIC, latch EIC, async EIC and sync EIC,
+ *
+ * The debounce EIC is used to capture the input signals' stable status
+ * (millisecond resolution) and a single-trigger mechanism is introduced
+ * into this sub-module to enhance the input event detection reliability.
+ * The debounce range is from 1ms to 4s with a step size of 1ms.
+ *
+ * The latch EIC is used to latch some special power down signals and
+ * generate interrupts, since the latch EIC does not depend on the APB clock
+ * to capture signals.
+ *
+ * The async EIC uses a 32k clock to capture the short signals (microsecond
+ * resolution) to generate interrupts by level or edge trigger.
+ *
+ * The EIC-sync is similar with GPIO's input function, which is a synchronized
+ * signal input register.
+ */
+enum sprd_eic_type {
+	SPRD_EIC_DEBOUNCE,
+	SPRD_EIC_LATCH,
+	SPRD_EIC_ASYNC,
+	SPRD_EIC_SYNC,
+	SPRD_EIC_MAX,
+};
+
+struct sprd_eic {
+	struct gpio_chip chip;
+	struct irq_chip intc;
+	void __iomem *base[SPRD_EIC_MAX_BANK];
+	enum sprd_eic_type type;
+	spinlock_t lock;
+	int irq;
+};
+
+struct sprd_eic_variant_data {
+	enum sprd_eic_type type;
+	u32 num_eics;
+};
+
+static const char *sprd_eic_label_name[SPRD_EIC_MAX] = {
+	"eic-debounce", "eic-latch", "eic-async",
+	"eic-sync",
+};
+
+static const struct sprd_eic_variant_data sc9860_eic_dbnc_data = {
+	.type = SPRD_EIC_DEBOUNCE,
+	.num_eics = 8,
+};
+
+static const struct sprd_eic_variant_data sc9860_eic_latch_data = {
+	.type = SPRD_EIC_LATCH,
+	.num_eics = 8,
+};
+
+static const struct sprd_eic_variant_data sc9860_eic_async_data = {
+	.type = SPRD_EIC_ASYNC,
+	.num_eics = 8,
+};
+
+static const struct sprd_eic_variant_data sc9860_eic_sync_data = {
+	.type = SPRD_EIC_SYNC,
+	.num_eics = 8,
+};
+
+static inline void __iomem *sprd_eic_offset_base(struct sprd_eic *sprd_eic,
+						 unsigned int bank)
+{
+	if (bank >= SPRD_EIC_MAX_BANK)
+		return NULL;
+
+	return sprd_eic->base[bank];
+}
+
+static void sprd_eic_update(struct gpio_chip *chip, unsigned int offset,
+			    u16 reg, unsigned int val)
+{
+	struct sprd_eic *sprd_eic = gpiochip_get_data(chip);
+	void __iomem *base =
+		sprd_eic_offset_base(sprd_eic, offset / SPRD_EIC_PER_BANK_NR);
+	unsigned long flags;
+	u32 tmp;
+
+	spin_lock_irqsave(&sprd_eic->lock, flags);
+	tmp = readl_relaxed(base + reg);
+
+	if (val)
+		tmp |= BIT(SPRD_EIC_BIT(offset));
+	else
+		tmp &= ~BIT(SPRD_EIC_BIT(offset));
+
+	writel_relaxed(tmp, base + reg);
+	spin_unlock_irqrestore(&sprd_eic->lock, flags);
+}
+
+static int sprd_eic_read(struct gpio_chip *chip, unsigned int offset, u16 reg)
+{
+	struct sprd_eic *sprd_eic = gpiochip_get_data(chip);
+	void __iomem *base =
+		sprd_eic_offset_base(sprd_eic, offset / SPRD_EIC_PER_BANK_NR);
+
+	return !!(readl_relaxed(base + reg) & BIT(SPRD_EIC_BIT(offset)));
+}
+
+static int sprd_eic_request(struct gpio_chip *chip, unsigned int offset)
+{
+	sprd_eic_update(chip, offset, SPRD_EIC_DBNC_DMSK, 1);
+	return 0;
+}
+
+static void sprd_eic_free(struct gpio_chip *chip, unsigned int offset)
+{
+	sprd_eic_update(chip, offset, SPRD_EIC_DBNC_DMSK, 0);
+}
+
+static int sprd_eic_get(struct gpio_chip *chip, unsigned int offset)
+{
+	return sprd_eic_read(chip, offset, SPRD_EIC_DBNC_DATA);
+}
+
+static int sprd_eic_direction_input(struct gpio_chip *chip, unsigned int offset)
+{
+	/* EICs are always input, nothing need to do here. */
+	return 0;
+}
+
+static void sprd_eic_set(struct gpio_chip *chip, unsigned int offset, int value)
+{
+	/* EICs are always input, nothing need to do here. */
+}
+
+static int sprd_eic_set_debounce(struct gpio_chip *chip, unsigned int offset,
+				 unsigned int debounce)
+{
+	struct sprd_eic *sprd_eic = gpiochip_get_data(chip);
+	void __iomem *base =
+		sprd_eic_offset_base(sprd_eic, offset / SPRD_EIC_PER_BANK_NR);
+	u32 reg = SPRD_EIC_DBNC_CTRL0 + SPRD_EIC_BIT(offset) * 0x4;
+	u32 value = readl_relaxed(base + reg) & ~SPRD_EIC_DBNC_MASK;
+
+	value |= debounce / 1000;
+	writel_relaxed(value, base + reg);
+
+	return 0;
+}
+
+static int sprd_eic_set_config(struct gpio_chip *chip, unsigned int offset,
+			       unsigned long config)
+{
+	unsigned long param = pinconf_to_config_param(config);
+	u32 arg = pinconf_to_config_argument(config);
+
+	if (param == PIN_CONFIG_INPUT_DEBOUNCE)
+		return sprd_eic_set_debounce(chip, offset, arg);
+
+	return -ENOTSUPP;
+}
+
+static void sprd_eic_irq_mask(struct irq_data *data)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
+	struct sprd_eic *sprd_eic = gpiochip_get_data(chip);
+	u32 offset = irqd_to_hwirq(data);
+
+	switch (sprd_eic->type) {
+	case SPRD_EIC_DEBOUNCE:
+		sprd_eic_update(chip, offset, SPRD_EIC_DBNC_IE, 0);
+		sprd_eic_update(chip, offset, SPRD_EIC_DBNC_TRIG, 0);
+		break;
+	case SPRD_EIC_LATCH:
+		sprd_eic_update(chip, offset, SPRD_EIC_LATCH_INTEN, 0);
+		break;
+	case SPRD_EIC_ASYNC:
+		sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTIE, 0);
+		break;
+	case SPRD_EIC_SYNC:
+		sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTIE, 0);
+		break;
+	default:
+		dev_err(chip->parent, "Unsupported EIC type.\n");
+	}
+}
+
+static void sprd_eic_irq_unmask(struct irq_data *data)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
+	struct sprd_eic *sprd_eic = gpiochip_get_data(chip);
+	u32 offset = irqd_to_hwirq(data);
+
+	switch (sprd_eic->type) {
+	case SPRD_EIC_DEBOUNCE:
+		sprd_eic_update(chip, offset, SPRD_EIC_DBNC_IE, 1);
+		sprd_eic_update(chip, offset, SPRD_EIC_DBNC_TRIG, 1);
+		break;
+	case SPRD_EIC_LATCH:
+		sprd_eic_update(chip, offset, SPRD_EIC_LATCH_INTEN, 1);
+		break;
+	case SPRD_EIC_ASYNC:
+		sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTIE, 1);
+		break;
+	case SPRD_EIC_SYNC:
+		sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTIE, 1);
+		break;
+	default:
+		dev_err(chip->parent, "Unsupported EIC type.\n");
+	}
+}
+
+static void sprd_eic_irq_ack(struct irq_data *data)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
+	struct sprd_eic *sprd_eic = gpiochip_get_data(chip);
+	u32 offset = irqd_to_hwirq(data);
+
+	switch (sprd_eic->type) {
+	case SPRD_EIC_DEBOUNCE:
+		sprd_eic_update(chip, offset, SPRD_EIC_DBNC_IC, 1);
+		break;
+	case SPRD_EIC_LATCH:
+		sprd_eic_update(chip, offset, SPRD_EIC_LATCH_INTCLR, 1);
+		break;
+	case SPRD_EIC_ASYNC:
+		sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTCLR, 1);
+		break;
+	case SPRD_EIC_SYNC:
+		sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTCLR, 1);
+		break;
+	default:
+		dev_err(chip->parent, "Unsupported EIC type.\n");
+	}
+}
+
+static int sprd_eic_irq_set_type(struct irq_data *data, unsigned int flow_type)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
+	struct sprd_eic *sprd_eic = gpiochip_get_data(chip);
+	u32 offset = irqd_to_hwirq(data);
+
+	switch (sprd_eic->type) {
+	case SPRD_EIC_DEBOUNCE:
+		switch (flow_type) {
+		case IRQ_TYPE_LEVEL_HIGH:
+			sprd_eic_update(chip, offset, SPRD_EIC_DBNC_IEV, 1);
+			break;
+		case IRQ_TYPE_LEVEL_LOW:
+			sprd_eic_update(chip, offset, SPRD_EIC_DBNC_IEV, 0);
+			break;
+		default:
+			return -ENOTSUPP;
+		}
+
+		irq_set_handler_locked(data, handle_level_irq);
+		break;
+	case SPRD_EIC_LATCH:
+		switch (flow_type) {
+		case IRQ_TYPE_LEVEL_HIGH:
+			sprd_eic_update(chip, offset, SPRD_EIC_LATCH_INTPOL, 0);
+			break;
+		case IRQ_TYPE_LEVEL_LOW:
+			sprd_eic_update(chip, offset, SPRD_EIC_LATCH_INTPOL, 1);
+			break;
+		default:
+			return -ENOTSUPP;
+		}
+
+		irq_set_handler_locked(data, handle_level_irq);
+		break;
+	case SPRD_EIC_ASYNC:
+		switch (flow_type) {
+		case IRQ_TYPE_EDGE_RISING:
+			sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTBOTH, 0);
+			sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTMODE, 0);
+			sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTPOL, 1);
+			irq_set_handler_locked(data, handle_edge_irq);
+			break;
+		case IRQ_TYPE_EDGE_FALLING:
+			sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTBOTH, 0);
+			sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTMODE, 0);
+			sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTPOL, 0);
+			irq_set_handler_locked(data, handle_edge_irq);
+			break;
+		case IRQ_TYPE_EDGE_BOTH:
+			sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTBOTH, 1);
+			irq_set_handler_locked(data, handle_edge_irq);
+			break;
+		case IRQ_TYPE_LEVEL_HIGH:
+			sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTBOTH, 0);
+			sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTMODE, 1);
+			sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTPOL, 1);
+			irq_set_handler_locked(data, handle_level_irq);
+			break;
+		case IRQ_TYPE_LEVEL_LOW:
+			sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTBOTH, 0);
+			sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTMODE, 1);
+			sprd_eic_update(chip, offset, SPRD_EIC_ASYNC_INTPOL, 0);
+			irq_set_handler_locked(data, handle_level_irq);
+			break;
+		default:
+			return -ENOTSUPP;
+		}
+		break;
+	case SPRD_EIC_SYNC:
+		switch (flow_type) {
+		case IRQ_TYPE_EDGE_RISING:
+			sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTBOTH, 0);
+			sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTMODE, 0);
+			sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTPOL, 1);
+			irq_set_handler_locked(data, handle_edge_irq);
+			break;
+		case IRQ_TYPE_EDGE_FALLING:
+			sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTBOTH, 0);
+			sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTMODE, 0);
+			sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTPOL, 0);
+			irq_set_handler_locked(data, handle_edge_irq);
+			break;
+		case IRQ_TYPE_EDGE_BOTH:
+			sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTBOTH, 1);
+			irq_set_handler_locked(data, handle_edge_irq);
+			break;
+		case IRQ_TYPE_LEVEL_HIGH:
+			sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTBOTH, 0);
+			sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTMODE, 1);
+			sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTPOL, 1);
+			irq_set_handler_locked(data, handle_level_irq);
+			break;
+		case IRQ_TYPE_LEVEL_LOW:
+			sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTBOTH, 0);
+			sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTMODE, 1);
+			sprd_eic_update(chip, offset, SPRD_EIC_SYNC_INTPOL, 0);
+			irq_set_handler_locked(data, handle_level_irq);
+			break;
+		default:
+			return -ENOTSUPP;
+		}
+	default:
+		dev_err(chip->parent, "Unsupported EIC type.\n");
+		return -ENOTSUPP;
+	}
+
+	return 0;
+}
+
+static int sprd_eic_match_chip_by_type(struct gpio_chip *chip, void *data)
+{
+	enum sprd_eic_type type = *(enum sprd_eic_type *)data;
+
+	return !strcmp(chip->label, sprd_eic_label_name[type]);
+}
+
+static void sprd_eic_handle_one_type(struct gpio_chip *chip)
+{
+	struct sprd_eic *sprd_eic = gpiochip_get_data(chip);
+	u32 bank, n, girq;
+
+	for (bank = 0; bank * SPRD_EIC_PER_BANK_NR < chip->ngpio; bank++) {
+		void __iomem *base = sprd_eic_offset_base(sprd_eic, bank);
+		unsigned long reg;
+
+		switch (sprd_eic->type) {
+		case SPRD_EIC_DEBOUNCE:
+			reg = readl_relaxed(base + SPRD_EIC_DBNC_MIS) &
+				SPRD_EIC_DATA_MASK;
+			break;
+		case SPRD_EIC_LATCH:
+			reg = readl_relaxed(base + SPRD_EIC_LATCH_INTMSK) &
+				SPRD_EIC_DATA_MASK;
+			break;
+		case SPRD_EIC_ASYNC:
+			reg = readl_relaxed(base + SPRD_EIC_ASYNC_INTMSK) &
+				SPRD_EIC_DATA_MASK;
+			break;
+		case SPRD_EIC_SYNC:
+			reg = readl_relaxed(base + SPRD_EIC_SYNC_INTMSK) &
+				SPRD_EIC_DATA_MASK;
+			break;
+		default:
+			dev_err(chip->parent, "Unsupported EIC type.\n");
+			return;
+		}
+
+		for_each_set_bit(n, &reg, SPRD_EIC_PER_BANK_NR) {
+			girq = irq_find_mapping(chip->irq.domain,
+					bank * SPRD_EIC_PER_BANK_NR + n);
+
+			generic_handle_irq(girq);
+		}
+	}
+}
+
+static void sprd_eic_irq_handler(struct irq_desc *desc)
+{
+	struct irq_chip *ic = irq_desc_get_chip(desc);
+	struct gpio_chip *chip;
+	enum sprd_eic_type type;
+
+	chained_irq_enter(ic, desc);
+
+	/*
+	 * Since the digital-chip EIC 4 sub-modules (debounce, latch, async
+	 * and sync) share one same interrupt line, we should iterate each
+	 * EIC module to check if there are EIC interrupts were triggered.
+	 */
+	for (type = SPRD_EIC_DEBOUNCE; type < SPRD_EIC_MAX; type++) {
+		chip = gpiochip_find(&type, sprd_eic_match_chip_by_type);
+		if (!chip)
+			continue;
+
+		sprd_eic_handle_one_type(chip);
+	}
+
+	chained_irq_exit(ic, desc);
+}
+
+static int sprd_eic_probe(struct platform_device *pdev)
+{
+	const struct sprd_eic_variant_data *pdata;
+	struct gpio_irq_chip *irq;
+	struct sprd_eic *sprd_eic;
+	struct resource *res;
+	int ret, i;
+
+	pdata = of_device_get_match_data(&pdev->dev);
+	if (!pdata) {
+		dev_err(&pdev->dev, "No matching driver data found.\n");
+		return -EINVAL;
+	}
+
+	sprd_eic = devm_kzalloc(&pdev->dev, sizeof(*sprd_eic), GFP_KERNEL);
+	if (!sprd_eic)
+		return -ENOMEM;
+
+	spin_lock_init(&sprd_eic->lock);
+	sprd_eic->type = pdata->type;
+
+	sprd_eic->irq = platform_get_irq(pdev, 0);
+	if (sprd_eic->irq < 0) {
+		dev_err(&pdev->dev, "Failed to get EIC interrupt.\n");
+		return sprd_eic->irq;
+	}
+
+	for (i = 0; i < SPRD_EIC_MAX_BANK; i++) {
+		/*
+		 * We can have maximum 3 banks EICs, and each EIC has
+		 * its own base address. But some platform maybe only
+		 * have one bank EIC, thus base[1] and base[2] can be
+		 * optional.
+		 */
+		res = platform_get_resource(pdev, IORESOURCE_MEM, i);
+		if (!res)
+			continue;
+
+		sprd_eic->base[i] = devm_ioremap_resource(&pdev->dev, res);
+		if (IS_ERR(sprd_eic->base[i]))
+			return PTR_ERR(sprd_eic->base[i]);
+	}
+
+	sprd_eic->chip.label = sprd_eic_label_name[sprd_eic->type];
+	sprd_eic->chip.ngpio = pdata->num_eics;
+	sprd_eic->chip.base = -1;
+	sprd_eic->chip.parent = &pdev->dev;
+	sprd_eic->chip.of_node = pdev->dev.of_node;
+	sprd_eic->chip.direction_input = sprd_eic_direction_input;
+	switch (sprd_eic->type) {
+	case SPRD_EIC_DEBOUNCE:
+		sprd_eic->chip.request = sprd_eic_request;
+		sprd_eic->chip.free = sprd_eic_free;
+		sprd_eic->chip.set_config = sprd_eic_set_config;
+		sprd_eic->chip.set = sprd_eic_set;
+		/* fall-through */
+	case SPRD_EIC_ASYNC:
+		/* fall-through */
+	case SPRD_EIC_SYNC:
+		sprd_eic->chip.get = sprd_eic_get;
+		break;
+	case SPRD_EIC_LATCH:
+		/* fall-through */
+	default:
+		break;
+	}
+
+	sprd_eic->intc.name = dev_name(&pdev->dev);
+	sprd_eic->intc.irq_ack = sprd_eic_irq_ack;
+	sprd_eic->intc.irq_mask = sprd_eic_irq_mask;
+	sprd_eic->intc.irq_unmask = sprd_eic_irq_unmask;
+	sprd_eic->intc.irq_set_type = sprd_eic_irq_set_type;
+	sprd_eic->intc.flags = IRQCHIP_SKIP_SET_WAKE;
+
+	irq = &sprd_eic->chip.irq;
+	irq->chip = &sprd_eic->intc;
+	irq->handler = handle_bad_irq;
+	irq->default_type = IRQ_TYPE_NONE;
+	irq->parent_handler = sprd_eic_irq_handler;
+	irq->parent_handler_data = sprd_eic;
+	irq->num_parents = 1;
+	irq->parents = &sprd_eic->irq;
+
+	ret = devm_gpiochip_add_data(&pdev->dev, &sprd_eic->chip, sprd_eic);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Could not register gpiochip %d.\n", ret);
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, sprd_eic);
+	return 0;
+}
+
+static const struct of_device_id sprd_eic_of_match[] = {
+	{
+		.compatible = "sprd,sc9860-eic-debounce",
+		.data = &sc9860_eic_dbnc_data,
+	},
+	{
+		.compatible = "sprd,sc9860-eic-latch",
+		.data = &sc9860_eic_latch_data,
+	},
+	{
+		.compatible = "sprd,sc9860-eic-async",
+		.data = &sc9860_eic_async_data,
+	},
+	{
+		.compatible = "sprd,sc9860-eic-sync",
+		.data = &sc9860_eic_sync_data,
+	},
+	{
+		/* end of list */
+	}
+};
+MODULE_DEVICE_TABLE(of, sprd_eic_of_match);
+
+static struct platform_driver sprd_eic_driver = {
+	.probe = sprd_eic_probe,
+	.driver = {
+		.name = "sprd-eic",
+		.of_match_table	= sprd_eic_of_match,
+	},
+};
+
+module_platform_driver(sprd_eic_driver);
+
+MODULE_DESCRIPTION("Spreadtrum EIC driver");
+MODULE_LICENSE("GPL v2");
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 3/3] gpio: Add Spreadtrum PMIC EIC driver support
  2018-02-24 10:44 [PATCH v2 1/3] dt-bindings: gpio: Add Spreadtrum EIC controller documentation Baolin Wang
  2018-02-24 10:44 ` [PATCH v2 2/3] gpio: Add Spreadtrum EIC driver support Baolin Wang
@ 2018-02-24 10:44 ` Baolin Wang
  2018-02-25 12:19   ` Andy Shevchenko
  2018-03-02 22:08 ` [PATCH v2 1/3] dt-bindings: gpio: Add Spreadtrum EIC controller documentation Rob Herring
  2 siblings, 1 reply; 13+ messages in thread
From: Baolin Wang @ 2018-02-24 10:44 UTC (permalink / raw)
  To: linus.walleij, robh+dt, mark.rutland
  Cc: devicetree, linux-kernel, linux-gpio, broonie, baolin.wang,
	andy.shevchenko

The Spreadtrum PMIC EIC controller contains only one bank of debounce EIC,
and this bank contains 16 EICs. Each EIC can only be used as input mode,
as well as supporting the debounce and the capability to trigger interrupts
when detecting input signals.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
Changes since v1:
 - New patch in v2.
---
 drivers/gpio/Kconfig              |    8 +
 drivers/gpio/Makefile             |    1 +
 drivers/gpio/gpio-pmic-eic-sprd.c |  328 +++++++++++++++++++++++++++++++++++++
 3 files changed, 337 insertions(+)
 create mode 100644 drivers/gpio/gpio-pmic-eic-sprd.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index c937407..f642314 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -380,6 +380,14 @@ config GPIO_PL061
 	help
 	  Say yes here to support the PrimeCell PL061 GPIO device
 
+config GPIO_PMIC_EIC_SPRD
+	tristate "Spreadtrum PMIC EIC support"
+	depends on MFD_SC27XX_PMIC || COMPILE_TEST
+	depends on OF_GPIO
+	select GPIOLIB_IRQCHIP
+	help
+	  Say yes here to support Spreadtrum PMIC EIC device.
+
 config GPIO_PXA
 	bool "PXA GPIO support"
 	depends on ARCH_PXA || ARCH_MMP
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 66eaa9e..50ba64b 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -99,6 +99,7 @@ obj-$(CONFIG_GPIO_PCI_IDIO_16)	+= gpio-pci-idio-16.o
 obj-$(CONFIG_GPIO_PCIE_IDIO_24)	+= gpio-pcie-idio-24.o
 obj-$(CONFIG_GPIO_PISOSR)	+= gpio-pisosr.o
 obj-$(CONFIG_GPIO_PL061)	+= gpio-pl061.o
+obj-$(CONFIG_GPIO_PMIC_EIC_SPRD)	+= gpio-pmic-eic-sprd.o
 obj-$(CONFIG_GPIO_PXA)		+= gpio-pxa.o
 obj-$(CONFIG_GPIO_RC5T583)	+= gpio-rc5t583.o
 obj-$(CONFIG_GPIO_RDC321X)	+= gpio-rdc321x.o
diff --git a/drivers/gpio/gpio-pmic-eic-sprd.c b/drivers/gpio/gpio-pmic-eic-sprd.c
new file mode 100644
index 0000000..ed5d6b2
--- /dev/null
+++ b/drivers/gpio/gpio-pmic-eic-sprd.c
@@ -0,0 +1,328 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Spreadtrum Communications Inc.
+ * Copyright (C) 2018 Linaro Ltd.
+ */
+
+#include <linux/gpio/driver.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+/* EIC registers definition */
+#define SPRD_PMIC_EIC_DATA		0x0
+#define SPRD_PMIC_EIC_DMSK		0x4
+#define SPRD_PMIC_EIC_IEV		0x14
+#define SPRD_PMIC_EIC_IE		0x18
+#define SPRD_PMIC_EIC_RIS		0x1c
+#define SPRD_PMIC_EIC_MIS		0x20
+#define SPRD_PMIC_EIC_IC		0x24
+#define SPRD_PMIC_EIC_TRIG		0x28
+#define SPRD_PMIC_EIC_CTRL0		0x40
+
+/*
+ * The PMIC EIC controller only has one bank, and each bank now can contain
+ * 16 EICs.
+ */
+#define SPRD_PMIC_EIC_PER_BANK_NR	16
+#define SPRD_PMIC_EIC_NR		SPRD_PMIC_EIC_PER_BANK_NR
+#define SPRD_PMIC_EIC_DATA_MASK		GENMASK(15, 0)
+#define SPRD_PMIC_EIC_BIT(x)		((x) & (SPRD_PMIC_EIC_PER_BANK_NR - 1))
+#define SPRD_PMIC_EIC_DBNC_MASK		GENMASK(11, 0)
+
+/*
+ * These registers are modified under the irq bus lock and cached to avoid
+ * unnecessary writes in bus_sync_unlock.
+ */
+enum { REG_IEV, REG_IE, REG_TRIG, CACHE_NR_REGS };
+
+/**
+ * struct sprd_pmic_eic - PMIC EIC controller
+ * @chip: the gpio_chip structure.
+ * @intc: the irq_chip structure.
+ * @regmap: the regmap from the parent device.
+ * @offset: the EIC controller's offset address of the PMIC.
+ * @reg: the array to cache the EIC registers.
+ * @buslock: for bus lock/sync and unlock.
+ * @irq: the interrupt number of the PMIC EIC conteroller.
+ */
+struct sprd_pmic_eic {
+	struct gpio_chip chip;
+	struct irq_chip intc;
+	struct regmap *map;
+	u32 offset;
+	u8 reg[CACHE_NR_REGS];
+	struct mutex buslock;
+	int irq;
+};
+
+static void sprd_pmic_eic_update(struct gpio_chip *chip, unsigned int offset,
+				 u16 reg, unsigned int val)
+{
+	struct sprd_pmic_eic *pmic_eic = gpiochip_get_data(chip);
+	u32 shift = SPRD_PMIC_EIC_BIT(offset);
+
+	regmap_update_bits(pmic_eic->map, pmic_eic->offset + reg,
+			   BIT(shift), val << shift);
+}
+
+static int sprd_pmic_eic_read(struct gpio_chip *chip, unsigned int offset,
+			      u16 reg)
+{
+	struct sprd_pmic_eic *pmic_eic = gpiochip_get_data(chip);
+	u32 value;
+	int ret;
+
+	ret = regmap_read(pmic_eic->map, pmic_eic->offset + reg, &value);
+	if (ret)
+		return ret;
+
+	return !!(value & BIT(SPRD_PMIC_EIC_BIT(offset)));
+}
+
+static int sprd_pmic_eic_request(struct gpio_chip *chip, unsigned int offset)
+{
+	sprd_pmic_eic_update(chip, offset, SPRD_PMIC_EIC_DMSK, 1);
+	return 0;
+}
+
+static void sprd_pmic_eic_free(struct gpio_chip *chip, unsigned int offset)
+{
+	sprd_pmic_eic_update(chip, offset, SPRD_PMIC_EIC_DMSK, 0);
+}
+
+static int sprd_pmic_eic_get(struct gpio_chip *chip, unsigned int offset)
+{
+	return sprd_pmic_eic_read(chip, offset, SPRD_PMIC_EIC_DATA);
+}
+
+static int sprd_pmic_eic_direction_input(struct gpio_chip *chip,
+					 unsigned int offset)
+{
+	/* EICs are always input, nothing need to do here. */
+	return 0;
+}
+
+static void sprd_pmic_eic_set(struct gpio_chip *chip, unsigned int offset,
+			      int value)
+{
+	/* EICs are always input, nothing need to do here. */
+}
+
+static int sprd_pmic_eic_set_debounce(struct gpio_chip *chip,
+				      unsigned int offset,
+				      unsigned int debounce)
+{
+	struct sprd_pmic_eic *pmic_eic = gpiochip_get_data(chip);
+	u32 reg, value;
+	int ret;
+
+	reg = SPRD_PMIC_EIC_CTRL0 + SPRD_PMIC_EIC_BIT(offset) * 0x4;
+	ret = regmap_read(pmic_eic->map, pmic_eic->offset + reg, &value);
+	if (ret)
+		return ret;
+
+	value &= ~SPRD_PMIC_EIC_DBNC_MASK;
+	value |= debounce / 1000;
+	return regmap_write(pmic_eic->map, pmic_eic->offset + reg, value);
+}
+
+static int sprd_pmic_eic_set_config(struct gpio_chip *chip, unsigned int offset,
+				    unsigned long config)
+{
+	unsigned long param = pinconf_to_config_param(config);
+	u32 arg = pinconf_to_config_argument(config);
+
+	if (param == PIN_CONFIG_INPUT_DEBOUNCE)
+		return sprd_pmic_eic_set_debounce(chip, offset, arg);
+
+	return -ENOTSUPP;
+}
+
+static void sprd_pmic_eic_irq_mask(struct irq_data *data)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
+	struct sprd_pmic_eic *pmic_eic = gpiochip_get_data(chip);
+
+	pmic_eic->reg[REG_IE] = 0;
+	pmic_eic->reg[REG_TRIG] = 0;
+}
+
+static void sprd_pmic_eic_irq_unmask(struct irq_data *data)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
+	struct sprd_pmic_eic *pmic_eic = gpiochip_get_data(chip);
+
+	pmic_eic->reg[REG_IE] = 1;
+	pmic_eic->reg[REG_TRIG] = 1;
+}
+
+static int sprd_pmic_eic_irq_set_type(struct irq_data *data,
+				      unsigned int flow_type)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
+	struct sprd_pmic_eic *pmic_eic = gpiochip_get_data(chip);
+
+	switch (flow_type) {
+	case IRQ_TYPE_LEVEL_HIGH:
+		pmic_eic->reg[REG_IEV] = 1;
+		break;
+	case IRQ_TYPE_LEVEL_LOW:
+		pmic_eic->reg[REG_IEV] = 0;
+		break;
+	default:
+		return -ENOTSUPP;
+	}
+
+	irq_set_handler_locked(data, handle_level_irq);
+	return 0;
+}
+
+static void sprd_pmic_eic_bus_lock(struct irq_data *data)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
+	struct sprd_pmic_eic *pmic_eic = gpiochip_get_data(chip);
+
+	mutex_lock(&pmic_eic->buslock);
+}
+
+static void sprd_pmic_eic_bus_sync_unlock(struct irq_data *data)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
+	struct sprd_pmic_eic *pmic_eic = gpiochip_get_data(chip);
+	u32 offset = irqd_to_hwirq(data);
+
+	/* Set irq type */
+	sprd_pmic_eic_update(chip, offset, SPRD_PMIC_EIC_IEV,
+			     pmic_eic->reg[REG_IEV]);
+	/* Set irq unmask */
+	sprd_pmic_eic_update(chip, offset, SPRD_PMIC_EIC_IE,
+			     pmic_eic->reg[REG_IE]);
+	/* Generate trigger start pulse for debounce EIC */
+	sprd_pmic_eic_update(chip, offset, SPRD_PMIC_EIC_TRIG,
+			     pmic_eic->reg[REG_TRIG]);
+
+	mutex_unlock(&pmic_eic->buslock);
+}
+
+static irqreturn_t sprd_pmic_eic_irq_handler(int irq, void *data)
+{
+	struct sprd_pmic_eic *pmic_eic = data;
+	struct gpio_chip *chip = &pmic_eic->chip;
+	u32 n, girq, val;
+	int ret;
+
+	ret = regmap_read(pmic_eic->map, pmic_eic->offset + SPRD_PMIC_EIC_MIS,
+			  &val);
+	if (ret)
+		return IRQ_RETVAL(ret);
+
+	val &= SPRD_PMIC_EIC_DATA_MASK;
+
+	for (n = 0; n < chip->ngpio; n++) {
+		if (!(BIT(n) & val))
+			continue;
+
+		/* Clear the interrupt */
+		sprd_pmic_eic_update(chip, n, SPRD_PMIC_EIC_IC, 1);
+
+		girq = irq_find_mapping(chip->irq.domain, n);
+		handle_nested_irq(girq);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int sprd_pmic_eic_probe(struct platform_device *pdev)
+{
+	struct gpio_irq_chip *irq;
+	struct sprd_pmic_eic *pmic_eic;
+	int ret;
+
+	pmic_eic = devm_kzalloc(&pdev->dev, sizeof(*pmic_eic), GFP_KERNEL);
+	if (!pmic_eic)
+		return -ENOMEM;
+
+	mutex_init(&pmic_eic->buslock);
+
+	pmic_eic->irq = platform_get_irq(pdev, 0);
+	if (pmic_eic->irq < 0) {
+		dev_err(&pdev->dev, "Failed to get PMIC EIC interrupt.\n");
+		return pmic_eic->irq;
+	}
+
+	pmic_eic->map = dev_get_regmap(pdev->dev.parent, NULL);
+	if (!pmic_eic->map)
+		return -ENODEV;
+
+	ret = of_property_read_u32(pdev->dev.of_node, "reg", &pmic_eic->offset);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to get PMIC EIC base address.\n");
+		return ret;
+	}
+
+	ret = devm_request_threaded_irq(&pdev->dev, pmic_eic->irq, NULL,
+					sprd_pmic_eic_irq_handler,
+					IRQF_TRIGGER_LOW |
+					IRQF_ONESHOT | IRQF_NO_SUSPEND,
+					dev_name(&pdev->dev), pmic_eic);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to request PMIC EIC IRQ.\n");
+		return ret;
+	}
+
+	pmic_eic->chip.label = dev_name(&pdev->dev);
+	pmic_eic->chip.ngpio = SPRD_PMIC_EIC_NR;
+	pmic_eic->chip.base = -1;
+	pmic_eic->chip.parent = &pdev->dev;
+	pmic_eic->chip.of_node = pdev->dev.of_node;
+	pmic_eic->chip.direction_input = sprd_pmic_eic_direction_input;
+	pmic_eic->chip.request = sprd_pmic_eic_request;
+	pmic_eic->chip.free = sprd_pmic_eic_free;
+	pmic_eic->chip.set_config = sprd_pmic_eic_set_config;
+	pmic_eic->chip.set = sprd_pmic_eic_set;
+	pmic_eic->chip.get = sprd_pmic_eic_get;
+
+	pmic_eic->intc.name = dev_name(&pdev->dev);
+	pmic_eic->intc.irq_mask = sprd_pmic_eic_irq_mask;
+	pmic_eic->intc.irq_unmask = sprd_pmic_eic_irq_unmask;
+	pmic_eic->intc.irq_set_type = sprd_pmic_eic_irq_set_type;
+	pmic_eic->intc.irq_bus_lock = sprd_pmic_eic_bus_lock;
+	pmic_eic->intc.irq_bus_sync_unlock = sprd_pmic_eic_bus_sync_unlock;
+	pmic_eic->intc.flags = IRQCHIP_SKIP_SET_WAKE;
+
+	irq = &pmic_eic->chip.irq;
+	irq->chip = &pmic_eic->intc;
+	irq->threaded = true;
+
+	ret = devm_gpiochip_add_data(&pdev->dev, &pmic_eic->chip, pmic_eic);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Could not register gpiochip %d.\n", ret);
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, pmic_eic);
+	return 0;
+}
+
+static const struct of_device_id sprd_pmic_eic_of_match[] = {
+	{ .compatible = "sprd,sc27xx-eic", },
+	{ /* end of list */ }
+};
+MODULE_DEVICE_TABLE(of, sprd_pmic_eic_of_match);
+
+static struct platform_driver sprd_pmic_eic_driver = {
+	.probe = sprd_pmic_eic_probe,
+	.driver = {
+		.name = "sprd-pmic-eic",
+		.of_match_table	= sprd_pmic_eic_of_match,
+	},
+};
+
+module_platform_driver(sprd_pmic_eic_driver);
+
+MODULE_DESCRIPTION("Spreadtrum PMIC EIC driver");
+MODULE_LICENSE("GPL v2");
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 3/3] gpio: Add Spreadtrum PMIC EIC driver support
  2018-02-24 10:44 ` [PATCH v2 3/3] gpio: Add Spreadtrum PMIC " Baolin Wang
@ 2018-02-25 12:19   ` Andy Shevchenko
  2018-02-26  3:01     ` Baolin Wang
  0 siblings, 1 reply; 13+ messages in thread
From: Andy Shevchenko @ 2018-02-25 12:19 UTC (permalink / raw)
  To: Baolin Wang
  Cc: Linus Walleij, Rob Herring, Mark Rutland, devicetree,
	Linux Kernel Mailing List, open list:GPIO SUBSYSTEM, Mark Brown

On Sat, Feb 24, 2018 at 12:44 PM, Baolin Wang <baolin.wang@linaro.org> wrote:
> The Spreadtrum PMIC EIC controller contains only one bank of debounce EIC,
> and this bank contains 16 EICs. Each EIC can only be used as input mode,
> as well as supporting the debounce and the capability to trigger interrupts
> when detecting input signals.

> +/*
> + * These registers are modified under the irq bus lock and cached to avoid
> + * unnecessary writes in bus_sync_unlock.
> + */
> +enum { REG_IEV, REG_IE, REG_TRIG, CACHE_NR_REGS };

One item per line.

> +static int sprd_pmic_eic_direction_input(struct gpio_chip *chip,
> +                                        unsigned int offset)
> +{
> +       /* EICs are always input, nothing need to do here. */
> +       return 0;
> +}
> +
> +static void sprd_pmic_eic_set(struct gpio_chip *chip, unsigned int offset,
> +                             int value)
> +{
> +       /* EICs are always input, nothing need to do here. */
> +}

Remove both.

Look at what GPIO core does.

> +       value |= debounce / 1000;

Possible overflow.

> +       for (n = 0; n < chip->ngpio; n++) {
> +               if (!(BIT(n) & val))

for_each_set_bit().

At some point you may need just to go across lib/ in the kernel and
see what we have there.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 3/3] gpio: Add Spreadtrum PMIC EIC driver support
  2018-02-25 12:19   ` Andy Shevchenko
@ 2018-02-26  3:01     ` Baolin Wang
  2018-02-26 12:02       ` Andy Shevchenko
  0 siblings, 1 reply; 13+ messages in thread
From: Baolin Wang @ 2018-02-26  3:01 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Linus Walleij, Rob Herring, Mark Rutland, devicetree,
	Linux Kernel Mailing List, open list:GPIO SUBSYSTEM, Mark Brown

Hi Andy,

On 25 February 2018 at 20:19, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> On Sat, Feb 24, 2018 at 12:44 PM, Baolin Wang <baolin.wang@linaro.org> wrote:
>> The Spreadtrum PMIC EIC controller contains only one bank of debounce EIC,
>> and this bank contains 16 EICs. Each EIC can only be used as input mode,
>> as well as supporting the debounce and the capability to trigger interrupts
>> when detecting input signals.
>
>> +/*
>> + * These registers are modified under the irq bus lock and cached to avoid
>> + * unnecessary writes in bus_sync_unlock.
>> + */
>> +enum { REG_IEV, REG_IE, REG_TRIG, CACHE_NR_REGS };
>
> One item per line.

Sure.

>
>> +static int sprd_pmic_eic_direction_input(struct gpio_chip *chip,
>> +                                        unsigned int offset)
>> +{
>> +       /* EICs are always input, nothing need to do here. */
>> +       return 0;
>> +}
>> +
>> +static void sprd_pmic_eic_set(struct gpio_chip *chip, unsigned int offset,
>> +                             int value)
>> +{
>> +       /* EICs are always input, nothing need to do here. */
>> +}
>
> Remove both.
>
> Look at what GPIO core does.

I've checked the GPIO core, we need the
sprd_pmic_eic_direction_input() returns 0, since user can set GPIOD_IN
flag when requesting one GPIO, otherwise it will return errors.
We also need one dummy sprd_pmic_eic_set() when setting debounce for
one GPIO, otherwise it will return errors.

>
>> +       value |= debounce / 1000;
>
> Possible overflow.

OK. I should & SPRD_PMIC_EIC_DBC_MASK.

>
>> +       for (n = 0; n < chip->ngpio; n++) {
>> +               if (!(BIT(n) & val))
>
> for_each_set_bit().
>
> At some point you may need just to go across lib/ in the kernel and
> see what we have there.

I've considered the for_each_set_bit(), it need one 'unsigned long'
type parameter, but we get the value from regmap is 'u32' type. So we
need one extra conversion from 'u32' to 'unsigned long' like:

unsigned long reg = val;

for_each_set_bit(n, &reg, chip->ngpio) {
        .......
}

If you like this conversion, then I can change to use
for_each_set_bit(). Thanks.

-- 
Baolin.wang
Best Regards

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 3/3] gpio: Add Spreadtrum PMIC EIC driver support
  2018-02-26  3:01     ` Baolin Wang
@ 2018-02-26 12:02       ` Andy Shevchenko
  2018-02-27  2:35         ` Baolin Wang
  0 siblings, 1 reply; 13+ messages in thread
From: Andy Shevchenko @ 2018-02-26 12:02 UTC (permalink / raw)
  To: Baolin Wang
  Cc: Linus Walleij, Rob Herring, Mark Rutland, devicetree,
	Linux Kernel Mailing List, open list:GPIO SUBSYSTEM, Mark Brown

On Mon, Feb 26, 2018 at 5:01 AM, Baolin Wang <baolin.wang@linaro.org> wrote:
> On 25 February 2018 at 20:19, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>> On Sat, Feb 24, 2018 at 12:44 PM, Baolin Wang <baolin.wang@linaro.org> wrote:

>>> +static int sprd_pmic_eic_direction_input(struct gpio_chip *chip,
>>> +                                        unsigned int offset)
>>> +{
>>> +       /* EICs are always input, nothing need to do here. */
>>> +       return 0;
>>> +}
>>> +
>>> +static void sprd_pmic_eic_set(struct gpio_chip *chip, unsigned int offset,
>>> +                             int value)
>>> +{
>>> +       /* EICs are always input, nothing need to do here. */
>>> +}
>>
>> Remove both.
>>
>> Look at what GPIO core does.
>
> I've checked the GPIO core, we need the
> sprd_pmic_eic_direction_input() returns 0, since user can set GPIOD_IN
> flag when requesting one GPIO, otherwise it will return errors.

Right. I thought it depends on presence of direction_output().

> We also need one dummy sprd_pmic_eic_set() when setting debounce for
> one GPIO, otherwise it will return errors.

This is pretty much a "feature" of GPIO framework. It shouldn't
require ->set() by logic if there is no output facility.
OK.

>>> +       for (n = 0; n < chip->ngpio; n++) {
>>> +               if (!(BIT(n) & val))
>>
>> for_each_set_bit().
>>
>> At some point you may need just to go across lib/ in the kernel and
>> see what we have there.
>
> I've considered the for_each_set_bit(), it need one 'unsigned long'
> type parameter, but we get the value from regmap is 'u32' type. So we
> need one extra conversion from 'u32' to 'unsigned long' like:
>
> unsigned long reg = val;
>
> for_each_set_bit(n, &reg, chip->ngpio) {
>         .......
> }
>
> If you like this conversion, then I can change to use
> for_each_set_bit(). Thanks.

Wouldn't it work like

unsigned long val;

...regmap_read(..., &val);

?

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 3/3] gpio: Add Spreadtrum PMIC EIC driver support
  2018-02-26 12:02       ` Andy Shevchenko
@ 2018-02-27  2:35         ` Baolin Wang
  2018-02-27 15:54           ` Andy Shevchenko
  0 siblings, 1 reply; 13+ messages in thread
From: Baolin Wang @ 2018-02-27  2:35 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Linus Walleij, Rob Herring, Mark Rutland, devicetree,
	Linux Kernel Mailing List, open list:GPIO SUBSYSTEM, Mark Brown

On 26 February 2018 at 20:02, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> On Mon, Feb 26, 2018 at 5:01 AM, Baolin Wang <baolin.wang@linaro.org> wrote:
>> On 25 February 2018 at 20:19, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>>> On Sat, Feb 24, 2018 at 12:44 PM, Baolin Wang <baolin.wang@linaro.org> wrote:
>
>>>> +static int sprd_pmic_eic_direction_input(struct gpio_chip *chip,
>>>> +                                        unsigned int offset)
>>>> +{
>>>> +       /* EICs are always input, nothing need to do here. */
>>>> +       return 0;
>>>> +}
>>>> +
>>>> +static void sprd_pmic_eic_set(struct gpio_chip *chip, unsigned int offset,
>>>> +                             int value)
>>>> +{
>>>> +       /* EICs are always input, nothing need to do here. */
>>>> +}
>>>
>>> Remove both.
>>>
>>> Look at what GPIO core does.
>>
>> I've checked the GPIO core, we need the
>> sprd_pmic_eic_direction_input() returns 0, since user can set GPIOD_IN
>> flag when requesting one GPIO, otherwise it will return errors.
>
> Right. I thought it depends on presence of direction_output().
>
>> We also need one dummy sprd_pmic_eic_set() when setting debounce for
>> one GPIO, otherwise it will return errors.
>
> This is pretty much a "feature" of GPIO framework. It shouldn't
> require ->set() by logic if there is no output facility.
> OK.
>
>>>> +       for (n = 0; n < chip->ngpio; n++) {
>>>> +               if (!(BIT(n) & val))
>>>
>>> for_each_set_bit().
>>>
>>> At some point you may need just to go across lib/ in the kernel and
>>> see what we have there.
>>
>> I've considered the for_each_set_bit(), it need one 'unsigned long'
>> type parameter, but we get the value from regmap is 'u32' type. So we
>> need one extra conversion from 'u32' to 'unsigned long' like:
>>
>> unsigned long reg = val;
>>
>> for_each_set_bit(n, &reg, chip->ngpio) {
>>         .......
>> }
>>
>> If you like this conversion, then I can change to use
>> for_each_set_bit(). Thanks.
>
> Wouldn't it work like
>
> unsigned long val;
>
> ...regmap_read(..., &val);
>
> ?

It can not work, regmap_read() expects 'unsigned int *'. But I can
convert it like this:

for_each_set_bit(n, (unsigned long *)&val, chip->ngpio) {
         .......
}

-- 
Baolin.wang
Best Regards

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 3/3] gpio: Add Spreadtrum PMIC EIC driver support
  2018-02-27  2:35         ` Baolin Wang
@ 2018-02-27 15:54           ` Andy Shevchenko
  2018-02-28  2:22             ` Baolin Wang
  0 siblings, 1 reply; 13+ messages in thread
From: Andy Shevchenko @ 2018-02-27 15:54 UTC (permalink / raw)
  To: Baolin Wang
  Cc: Linus Walleij, Rob Herring, Mark Rutland, devicetree,
	Linux Kernel Mailing List, open list:GPIO SUBSYSTEM, Mark Brown

On Tue, Feb 27, 2018 at 4:35 AM, Baolin Wang <baolin.wang@linaro.org> wrote:
> On 26 February 2018 at 20:02, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>> On Mon, Feb 26, 2018 at 5:01 AM, Baolin Wang <baolin.wang@linaro.org> wrote:
>>> On 25 February 2018 at 20:19, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

>>>>> +       for (n = 0; n < chip->ngpio; n++) {
>>>>> +               if (!(BIT(n) & val))
>>>>
>>>> for_each_set_bit().
>>>>
>>>> At some point you may need just to go across lib/ in the kernel and
>>>> see what we have there.
>>>
>>> I've considered the for_each_set_bit(), it need one 'unsigned long'
>>> type parameter, but we get the value from regmap is 'u32' type. So we
>>> need one extra conversion from 'u32' to 'unsigned long' like:
>>>
>>> unsigned long reg = val;
>>>
>>> for_each_set_bit(n, &reg, chip->ngpio) {
>>>         .......
>>> }
>>>
>>> If you like this conversion, then I can change to use
>>> for_each_set_bit(). Thanks.
>>
>> Wouldn't it work like
>>
>> unsigned long val;
>>
>> ...regmap_read(..., &val);
>>
>> ?
>
> It can not work, regmap_read() expects 'unsigned int *'.

Ah, OK, than the temporary variable is a left approach.

> But I can
> convert it like this:
>
> for_each_set_bit(n, (unsigned long *)&val, chip->ngpio) {
>          .......
> }

No, this is a boilerplate for static analyzers and definitely UB.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 3/3] gpio: Add Spreadtrum PMIC EIC driver support
  2018-02-27 15:54           ` Andy Shevchenko
@ 2018-02-28  2:22             ` Baolin Wang
  0 siblings, 0 replies; 13+ messages in thread
From: Baolin Wang @ 2018-02-28  2:22 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Linus Walleij, Rob Herring, Mark Rutland, devicetree,
	Linux Kernel Mailing List, open list:GPIO SUBSYSTEM, Mark Brown

On 27 February 2018 at 23:54, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> On Tue, Feb 27, 2018 at 4:35 AM, Baolin Wang <baolin.wang@linaro.org> wrote:
>> On 26 February 2018 at 20:02, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>>> On Mon, Feb 26, 2018 at 5:01 AM, Baolin Wang <baolin.wang@linaro.org> wrote:
>>>> On 25 February 2018 at 20:19, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>
>>>>>> +       for (n = 0; n < chip->ngpio; n++) {
>>>>>> +               if (!(BIT(n) & val))
>>>>>
>>>>> for_each_set_bit().
>>>>>
>>>>> At some point you may need just to go across lib/ in the kernel and
>>>>> see what we have there.
>>>>
>>>> I've considered the for_each_set_bit(), it need one 'unsigned long'
>>>> type parameter, but we get the value from regmap is 'u32' type. So we
>>>> need one extra conversion from 'u32' to 'unsigned long' like:
>>>>
>>>> unsigned long reg = val;
>>>>
>>>> for_each_set_bit(n, &reg, chip->ngpio) {
>>>>         .......
>>>> }
>>>>
>>>> If you like this conversion, then I can change to use
>>>> for_each_set_bit(). Thanks.
>>>
>>> Wouldn't it work like
>>>
>>> unsigned long val;
>>>
>>> ...regmap_read(..., &val);
>>>
>>> ?
>>
>> It can not work, regmap_read() expects 'unsigned int *'.
>
> Ah, OK, than the temporary variable is a left approach.
>
>> But I can
>> convert it like this:
>>
>> for_each_set_bit(n, (unsigned long *)&val, chip->ngpio) {
>>          .......
>> }
>
> No, this is a boilerplate for static analyzers and definitely UB.

OK. Thanks for pointing this issue out.

-- 
Baolin.wang
Best Regards

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 2/3] gpio: Add Spreadtrum EIC driver support
  2018-02-24 10:44 ` [PATCH v2 2/3] gpio: Add Spreadtrum EIC driver support Baolin Wang
@ 2018-03-02  9:53   ` Linus Walleij
  2018-03-02 10:48     ` Baolin Wang
  0 siblings, 1 reply; 13+ messages in thread
From: Linus Walleij @ 2018-03-02  9:53 UTC (permalink / raw)
  To: Baolin Wang
  Cc: Rob Herring, Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-kernel, open list:GPIO SUBSYSTEM, Mark Brown,
	Andy Shevchenko

On Sat, Feb 24, 2018 at 11:44 AM, Baolin Wang <baolin.wang@linaro.org> wrote:

> The Spreadtrum digital-chip EIC controller has 4 sub-modules: debounce EIC,
> latch EIC, async EIC and sync EIC, and each sub-module can has multiple
> banks and each bank contains 8 EICs.
>
> Each EIC can only be used as input mode, and has the capability to trigger
> interrupts when detecting input signals.
>
> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
> ---
> Changes since v1:
>  - Add OF_GPIO dependency.
>  - Fix some typos and grammar issues.
>  - Simplify the sprd_eic_update() and sprd_eic_read().
>  - Rename 'sprd_eic_data' to 'sprd_eic_variant_data'.
>  - Rename sprd_eic_group_base() to sprd_eic_offset_base().
>  - Split the PMIC EIC support into one separate driver.
>  - Return -EIO for sprd_eic_direction_input().
>  - Other optimization.

This is looking nice!

I guess we agreed with the input maintainer that what
needs to happen is to implement edge trigger emulation
for all variants in the driver, test it with the keys and
then we should be done.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 2/3] gpio: Add Spreadtrum EIC driver support
  2018-03-02  9:53   ` Linus Walleij
@ 2018-03-02 10:48     ` Baolin Wang
  2018-03-02 12:40       ` Linus Walleij
  0 siblings, 1 reply; 13+ messages in thread
From: Baolin Wang @ 2018-03-02 10:48 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Rob Herring, Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-kernel, open list:GPIO SUBSYSTEM, Mark Brown,
	Andy Shevchenko

Hi Linus,

On 2 March 2018 at 17:53, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Sat, Feb 24, 2018 at 11:44 AM, Baolin Wang <baolin.wang@linaro.org> wrote:
>
>> The Spreadtrum digital-chip EIC controller has 4 sub-modules: debounce EIC,
>> latch EIC, async EIC and sync EIC, and each sub-module can has multiple
>> banks and each bank contains 8 EICs.
>>
>> Each EIC can only be used as input mode, and has the capability to trigger
>> interrupts when detecting input signals.
>>
>> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
>> ---
>> Changes since v1:
>>  - Add OF_GPIO dependency.
>>  - Fix some typos and grammar issues.
>>  - Simplify the sprd_eic_update() and sprd_eic_read().
>>  - Rename 'sprd_eic_data' to 'sprd_eic_variant_data'.
>>  - Rename sprd_eic_group_base() to sprd_eic_offset_base().
>>  - Split the PMIC EIC support into one separate driver.
>>  - Return -EIO for sprd_eic_direction_input().
>>  - Other optimization.
>
> This is looking nice!
>
> I guess we agreed with the input maintainer that what
> needs to happen is to implement edge trigger emulation
> for all variants in the driver, test it with the keys and
> then we should be done.

I want to send one separate patch to implement edge trigger emulation
for EIC drivers, since we can have some talks for the implementation.
If there are no other issues for this patch set, I will send out the
V3 after fixing the issues in patch 3 pointed by Andy. Is it OK?

-- 
Baolin.wang
Best Regards

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 2/3] gpio: Add Spreadtrum EIC driver support
  2018-03-02 10:48     ` Baolin Wang
@ 2018-03-02 12:40       ` Linus Walleij
  0 siblings, 0 replies; 13+ messages in thread
From: Linus Walleij @ 2018-03-02 12:40 UTC (permalink / raw)
  To: Baolin Wang
  Cc: Rob Herring, Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-kernel, open list:GPIO SUBSYSTEM, Mark Brown,
	Andy Shevchenko

On Fri, Mar 2, 2018 at 11:48 AM, Baolin Wang <baolin.wang@linaro.org> wrote:
> On 2 March 2018 at 17:53, Linus Walleij <linus.walleij@linaro.org> wrote:

>> This is looking nice!
>>
>> I guess we agreed with the input maintainer that what
>> needs to happen is to implement edge trigger emulation
>> for all variants in the driver, test it with the keys and
>> then we should be done.
>
> I want to send one separate patch to implement edge trigger emulation
> for EIC drivers, since we can have some talks for the implementation.
> If there are no other issues for this patch set, I will send out the
> V3 after fixing the issues in patch 3 pointed by Andy. Is it OK?

OK go for it.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/3] dt-bindings: gpio: Add Spreadtrum EIC controller documentation
  2018-02-24 10:44 [PATCH v2 1/3] dt-bindings: gpio: Add Spreadtrum EIC controller documentation Baolin Wang
  2018-02-24 10:44 ` [PATCH v2 2/3] gpio: Add Spreadtrum EIC driver support Baolin Wang
  2018-02-24 10:44 ` [PATCH v2 3/3] gpio: Add Spreadtrum PMIC " Baolin Wang
@ 2018-03-02 22:08 ` Rob Herring
  2 siblings, 0 replies; 13+ messages in thread
From: Rob Herring @ 2018-03-02 22:08 UTC (permalink / raw)
  To: Baolin Wang
  Cc: linus.walleij, mark.rutland, devicetree, linux-kernel,
	linux-gpio, broonie, andy.shevchenko

On Sat, Feb 24, 2018 at 06:44:18PM +0800, Baolin Wang wrote:
> This patch adds the device tree bindings for the Spreadtrum EIC
> controller. The EIC can be seen as a special type of GPIO, which
> can only be used as input mode.
> 
> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
> ---
> Changes since v1:
>  - Fix some typos and grammar issues.
>  - Add more explanation to make things clear.
>  - List all device nodes as examples.
> ---
>  .../devicetree/bindings/gpio/gpio-eic-sprd.txt     |   97 ++++++++++++++++++++
>  1 file changed, 97 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-eic-sprd.txt

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2018-03-02 22:09 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-24 10:44 [PATCH v2 1/3] dt-bindings: gpio: Add Spreadtrum EIC controller documentation Baolin Wang
2018-02-24 10:44 ` [PATCH v2 2/3] gpio: Add Spreadtrum EIC driver support Baolin Wang
2018-03-02  9:53   ` Linus Walleij
2018-03-02 10:48     ` Baolin Wang
2018-03-02 12:40       ` Linus Walleij
2018-02-24 10:44 ` [PATCH v2 3/3] gpio: Add Spreadtrum PMIC " Baolin Wang
2018-02-25 12:19   ` Andy Shevchenko
2018-02-26  3:01     ` Baolin Wang
2018-02-26 12:02       ` Andy Shevchenko
2018-02-27  2:35         ` Baolin Wang
2018-02-27 15:54           ` Andy Shevchenko
2018-02-28  2:22             ` Baolin Wang
2018-03-02 22:08 ` [PATCH v2 1/3] dt-bindings: gpio: Add Spreadtrum EIC controller documentation Rob Herring

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).