linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/3] gpio / pinctrl: cherryview: Fix missing events from EC
@ 2016-09-20 12:15 Mika Westerberg
  2016-09-20 12:15 ` [PATCH v3 1/3] gpiolib: Make it possible to exclude GPIOs from IRQ domain Mika Westerberg
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Mika Westerberg @ 2016-09-20 12:15 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Marc Zyngier, Thomas Gleixner, Phidias Chiang, Anisse Astier,
	Heikki Krogerus, Yu C Chen, Mika Westerberg, linux-gpio,
	linux-kernel

Hi,

Up until now systems having Intel Cherryview/Braswell might lose GPEs
(General Purpose Events) from EC (Embedded Controller) because the pinctrl
driver masks all interrupt sources at probe time. I tried to fix this
already in bcb48cca23ec ("pinctrl: cherryview: Do not mask all interrupts
in probe") but it resulted that the irq core masked all the interrupts
because now we pass handle_bad_irq() as default handler for the irqchip.

After reading again the hardware spec, I think I finally understand the
problem correctly. In summary for southwest and north communities only the
first 8 (or 16) wires can be used to generate interrupts. Rest are reserved
for GPEs.

We fix this by excluding these only GPE capable wires from the IRQ domain
of the gpiochip.

This first follows what LinusW suggested and adds irq_valid_mask for each
gpiochip and then converts pinctrl-cherryview to use it.

Previous version of the patches and the discussion around this issue can be
found here:

  v2: http://www.spinics.net/lists/linux-gpio/msg16760.html
  v1: https://lkml.org/lkml/2015/5/22/111

Changes from v2:
  - Add irq_need_valid_mask flag that if set will make gpiochip_add_data()
    allocate irq_valid_mask for the chip.
  - Drop gpiochip_irqchip_exclude_irq() in favor of directly using
    set_/clear_bit() in drivers.
  - Use likely() annotation in gpiochip_irqchip_irq_valid().
  - Changed ordering of last two patches because there is no dependency
    anymore.

Changes from v1:
  - Only allocate irq_valid_mask when needed
  - Provide gpiochip_irqchip_exclude_irq() helper which allows drivers to
    select which GPIOs to exclude.
  - Use ->nrirqs in chv_gpio_irq_handler()
  - Added patch to convert the driver to use devm_gpiochip_add_data() so
    we can just return if gpiochip_irqchip_exclude_irq() fails (and also
    this simplifies the driver).

Mika Westerberg (3):
  gpiolib: Make it possible to exclude GPIOs from IRQ domain
  pinctrl: cherryview: Do not add all southwest and north GPIOs to IRQ domain
  pinctrl: cherryview: Convert to use devm_gpiochip_add_data()

 Documentation/gpio/driver.txt              |  6 +++
 drivers/gpio/gpiolib.c                     | 66 ++++++++++++++++++++++++++++--
 drivers/pinctrl/intel/pinctrl-cherryview.c | 55 ++++++++++++++++---------
 include/linux/gpio/driver.h                |  6 +++
 4 files changed, 111 insertions(+), 22 deletions(-)

-- 
2.9.3


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

* [PATCH v3 1/3] gpiolib: Make it possible to exclude GPIOs from IRQ domain
  2016-09-20 12:15 [PATCH v3 0/3] gpio / pinctrl: cherryview: Fix missing events from EC Mika Westerberg
@ 2016-09-20 12:15 ` Mika Westerberg
  2016-09-23 12:47   ` Linus Walleij
  2016-09-20 12:15 ` [PATCH v3 2/3] pinctrl: cherryview: Do not add all southwest and north GPIOs to " Mika Westerberg
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Mika Westerberg @ 2016-09-20 12:15 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Marc Zyngier, Thomas Gleixner, Phidias Chiang, Anisse Astier,
	Heikki Krogerus, Yu C Chen, Mika Westerberg, linux-gpio,
	linux-kernel

When using GPIO irqchip helpers to setup irqchip for a gpiolib based
driver, it is not possible to select which GPIOs to add to the IRQ domain.
Instead it just adds all GPIOs which is not always desired. For example
there might be GPIOs that for some reason cannot generated normal
interrupts at all.

To support this we add a flag irq_need_valid_mask to struct gpio_chip. When
this flag is set the core allocates irq_valid_mask that holds one bit for
each GPIO the chip has. By default all bits are set but drivers can
manipulate this using set_bit() and clear_bit() accordingly.

Then when gpiochip_irqchip_add() is called, this mask is checked and all
GPIOs with bit is set are added to the IRQ domain created for the GPIO
chip.

Suggested-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 Documentation/gpio/driver.txt |  6 ++++
 drivers/gpio/gpiolib.c        | 66 +++++++++++++++++++++++++++++++++++++++++--
 include/linux/gpio/driver.h   |  6 ++++
 3 files changed, 75 insertions(+), 3 deletions(-)

diff --git a/Documentation/gpio/driver.txt b/Documentation/gpio/driver.txt
index 6cb35a78eff4..368d5a294d89 100644
--- a/Documentation/gpio/driver.txt
+++ b/Documentation/gpio/driver.txt
@@ -262,6 +262,12 @@ symbol:
   to the container using container_of().
   (See Documentation/driver-model/design-patterns.txt)
 
+  If there is a need to exclude certain GPIOs from the IRQ domain, one can
+  set .irq_need_valid_mask of the gpiochip before gpiochip_add_data() is
+  called. This allocates .irq_valid_mask with as many bits set as there are
+  GPIOs in the chip. Drivers can exclude GPIOs by clearing bits from this
+  mask. The mask must be filled in before gpiochip_irqchip_add() is called.
+
 * gpiochip_set_chained_irqchip(): sets up a chained irq handler for a
   gpio_chip from a parent IRQ and passes the struct gpio_chip* as handler
   data. (Notice handler data, since the irqchip data is likely used by the
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 53ff25ac66d8..23be4daefeed 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -71,6 +71,8 @@ LIST_HEAD(gpio_devices);
 
 static void gpiochip_free_hogs(struct gpio_chip *chip);
 static void gpiochip_irqchip_remove(struct gpio_chip *gpiochip);
+static int gpiochip_irqchip_init_valid_mask(struct gpio_chip *gpiochip);
+static void gpiochip_irqchip_free_valid_mask(struct gpio_chip *gpiochip);
 
 static bool gpiolib_initialized;
 
@@ -1167,6 +1169,10 @@ int gpiochip_add_data(struct gpio_chip *chip, void *data)
 	if (status)
 		goto err_remove_from_list;
 
+	status = gpiochip_irqchip_init_valid_mask(chip);
+	if (status)
+		goto err_remove_from_list;
+
 	status = of_gpiochip_add(chip);
 	if (status)
 		goto err_remove_chip;
@@ -1192,6 +1198,7 @@ err_remove_chip:
 	acpi_gpiochip_remove(chip);
 	gpiochip_free_hogs(chip);
 	of_gpiochip_remove(chip);
+	gpiochip_irqchip_free_valid_mask(chip);
 err_remove_from_list:
 	spin_lock_irqsave(&gpio_lock, flags);
 	list_del(&gdev->list);
@@ -1401,6 +1408,40 @@ static struct gpio_chip *find_chip_by_name(const char *name)
  * The following is irqchip helper code for gpiochips.
  */
 
+static int gpiochip_irqchip_init_valid_mask(struct gpio_chip *gpiochip)
+{
+	int i;
+
+	if (!gpiochip->irq_need_valid_mask)
+		return 0;
+
+	gpiochip->irq_valid_mask = kcalloc(BITS_TO_LONGS(gpiochip->ngpio),
+					   sizeof(long), GFP_KERNEL);
+	if (!gpiochip->irq_valid_mask)
+		return -ENOMEM;
+
+	/* Assume by default all GPIOs are valid */
+	for (i = 0; i < gpiochip->ngpio; i++)
+		set_bit(i, gpiochip->irq_valid_mask);
+
+	return 0;
+}
+
+static void gpiochip_irqchip_free_valid_mask(struct gpio_chip *gpiochip)
+{
+	kfree(gpiochip->irq_valid_mask);
+	gpiochip->irq_valid_mask = NULL;
+}
+
+static bool gpiochip_irqchip_irq_valid(const struct gpio_chip *gpiochip,
+				       unsigned int offset)
+{
+	/* No mask means all valid */
+	if (likely(!gpiochip->irq_valid_mask))
+		return true;
+	return test_bit(offset, gpiochip->irq_valid_mask);
+}
+
 /**
  * gpiochip_set_chained_irqchip() - sets a chained irqchip to a gpiochip
  * @gpiochip: the gpiochip to set the irqchip chain to
@@ -1442,9 +1483,12 @@ void gpiochip_set_chained_irqchip(struct gpio_chip *gpiochip,
 	}
 
 	/* Set the parent IRQ for all affected IRQs */
-	for (offset = 0; offset < gpiochip->ngpio; offset++)
+	for (offset = 0; offset < gpiochip->ngpio; offset++) {
+		if (!gpiochip_irqchip_irq_valid(gpiochip, offset))
+			continue;
 		irq_set_parent(irq_find_mapping(gpiochip->irqdomain, offset),
 			       parent_irq);
+	}
 }
 EXPORT_SYMBOL_GPL(gpiochip_set_chained_irqchip);
 
@@ -1551,9 +1595,12 @@ static void gpiochip_irqchip_remove(struct gpio_chip *gpiochip)
 
 	/* Remove all IRQ mappings and delete the domain */
 	if (gpiochip->irqdomain) {
-		for (offset = 0; offset < gpiochip->ngpio; offset++)
+		for (offset = 0; offset < gpiochip->ngpio; offset++) {
+			if (!gpiochip_irqchip_irq_valid(gpiochip, offset))
+				continue;
 			irq_dispose_mapping(
 				irq_find_mapping(gpiochip->irqdomain, offset));
+		}
 		irq_domain_remove(gpiochip->irqdomain);
 	}
 
@@ -1562,6 +1609,8 @@ static void gpiochip_irqchip_remove(struct gpio_chip *gpiochip)
 		gpiochip->irqchip->irq_release_resources = NULL;
 		gpiochip->irqchip = NULL;
 	}
+
+	gpiochip_irqchip_free_valid_mask(gpiochip);
 }
 
 /**
@@ -1597,6 +1646,7 @@ int _gpiochip_irqchip_add(struct gpio_chip *gpiochip,
 			  struct lock_class_key *lock_key)
 {
 	struct device_node *of_node;
+	bool irq_base_set = false;
 	unsigned int offset;
 	unsigned irq_base = 0;
 
@@ -1646,13 +1696,17 @@ int _gpiochip_irqchip_add(struct gpio_chip *gpiochip,
 	 * necessary to allocate descriptors for all IRQs.
 	 */
 	for (offset = 0; offset < gpiochip->ngpio; offset++) {
+		if (!gpiochip_irqchip_irq_valid(gpiochip, offset))
+			continue;
 		irq_base = irq_create_mapping(gpiochip->irqdomain, offset);
-		if (offset == 0)
+		if (!irq_base_set) {
 			/*
 			 * Store the base into the gpiochip to be used when
 			 * unmapping the irqs.
 			 */
 			gpiochip->irq_base = irq_base;
+			irq_base_set = true;
+		}
 	}
 
 	acpi_gpiochip_request_interrupts(gpiochip);
@@ -1664,6 +1718,12 @@ EXPORT_SYMBOL_GPL(_gpiochip_irqchip_add);
 #else /* CONFIG_GPIOLIB_IRQCHIP */
 
 static void gpiochip_irqchip_remove(struct gpio_chip *gpiochip) {}
+static inline int gpiochip_irqchip_init_valid_mask(struct gpio_chip *gpiochip)
+{
+	return 0;
+}
+static inline void gpiochip_irqchip_free_valid_mask(struct gpio_chip *gpiochip)
+{ }
 
 #endif /* CONFIG_GPIOLIB_IRQCHIP */
 
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index 50882e09289b..420b837f2aa7 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -112,6 +112,10 @@ enum single_ended_mode {
  *	initialization, provided by GPIO driver
  * @irq_parent: GPIO IRQ chip parent/bank linux irq number,
  *	provided by GPIO driver
+ * @irq_need_valid_mask: If set core allocates @irq_valid_mask with all
+ *	bits set to one
+ * @irq_valid_mask: If not %NULL holds bitmask of GPIOs which are valid to
+ *	be included in IRQ domain of the chip
  * @lock_key: per GPIO IRQ chip lockdep class
  *
  * A gpio_chip can help platforms abstract various sources of GPIOs so
@@ -190,6 +194,8 @@ struct gpio_chip {
 	irq_flow_handler_t	irq_handler;
 	unsigned int		irq_default_type;
 	int			irq_parent;
+	bool			irq_need_valid_mask;
+	unsigned long		*irq_valid_mask;
 	struct lock_class_key	*lock_key;
 #endif
 
-- 
2.9.3


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

* [PATCH v3 2/3] pinctrl: cherryview: Do not add all southwest and north GPIOs to IRQ domain
  2016-09-20 12:15 [PATCH v3 0/3] gpio / pinctrl: cherryview: Fix missing events from EC Mika Westerberg
  2016-09-20 12:15 ` [PATCH v3 1/3] gpiolib: Make it possible to exclude GPIOs from IRQ domain Mika Westerberg
@ 2016-09-20 12:15 ` Mika Westerberg
  2016-09-23 12:58   ` Linus Walleij
  2016-09-20 12:15 ` [PATCH v3 3/3] pinctrl: cherryview: Convert to use devm_gpiochip_add_data() Mika Westerberg
  2016-10-11  6:23 ` [PATCH v3 0/3] gpio / pinctrl: cherryview: Fix missing events from EC Phidias Chiang
  3 siblings, 1 reply; 9+ messages in thread
From: Mika Westerberg @ 2016-09-20 12:15 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Marc Zyngier, Thomas Gleixner, Phidias Chiang, Anisse Astier,
	Heikki Krogerus, Yu C Chen, Mika Westerberg, linux-gpio,
	linux-kernel

It turns out that for north and southwest communities, they can only
generate GPIO interrupts for lower 8 interrupts (IntSel value). The upper
part (8-15) can only generate GPEs (General Purpose Events).

Now the reason why EC events such as pressing hotkeys does not work if we
mask all the interrupts is that in order to generate either interrupts or
GPEs the INTMASK register must have that particular interrupt unmasked. In
case of GPEs the CPU does not trigger normal interrupt (and thus the GPIO
driver does not see it) but instead it causes SCI (System Control
Interrupt) to be triggered with the GPE in question set.

To make this all work as expected we only add those GPIOs to the IRQ domain
that can actually generate interrupts (IntSel value 0-7) and skip others.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/pinctrl/intel/pinctrl-cherryview.c | 34 +++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c
index 0fe8fad25e4d..d23be3a2fb35 100644
--- a/drivers/pinctrl/intel/pinctrl-cherryview.c
+++ b/drivers/pinctrl/intel/pinctrl-cherryview.c
@@ -134,6 +134,7 @@ struct chv_gpio_pinrange {
  * @gpio_ranges: An array of GPIO ranges in this community
  * @ngpio_ranges: Number of GPIO ranges
  * @ngpios: Total number of GPIOs in this community
+ * @nirqs: Total number of IRQs this community can generate
  */
 struct chv_community {
 	const char *uid;
@@ -146,6 +147,7 @@ struct chv_community {
 	const struct chv_gpio_pinrange *gpio_ranges;
 	size_t ngpio_ranges;
 	size_t ngpios;
+	size_t nirqs;
 };
 
 struct chv_pin_context {
@@ -396,6 +398,12 @@ static const struct chv_community southwest_community = {
 	.gpio_ranges = southwest_gpio_ranges,
 	.ngpio_ranges = ARRAY_SIZE(southwest_gpio_ranges),
 	.ngpios = ARRAY_SIZE(southwest_pins),
+	/*
+	 * Southwest community can benerate GPIO interrupts only for the
+	 * first 8 interrupts. The upper half (8-15) can only be used to
+	 * trigger GPEs.
+	 */
+	.nirqs = 8,
 };
 
 static const struct pinctrl_pin_desc north_pins[] = {
@@ -479,6 +487,12 @@ static const struct chv_community north_community = {
 	.gpio_ranges = north_gpio_ranges,
 	.ngpio_ranges = ARRAY_SIZE(north_gpio_ranges),
 	.ngpios = ARRAY_SIZE(north_pins),
+	/*
+	 * North community can benerate GPIO interrupts only for the first
+	 * 8 interrupts. The upper half (8-15) can only be used to trigger
+	 * GPEs.
+	 */
+	.nirqs = 8,
 };
 
 static const struct pinctrl_pin_desc east_pins[] = {
@@ -521,6 +535,7 @@ static const struct chv_community east_community = {
 	.gpio_ranges = east_gpio_ranges,
 	.ngpio_ranges = ARRAY_SIZE(east_gpio_ranges),
 	.ngpios = ARRAY_SIZE(east_pins),
+	.nirqs = 16,
 };
 
 static const struct pinctrl_pin_desc southeast_pins[] = {
@@ -646,6 +661,7 @@ static const struct chv_community southeast_community = {
 	.gpio_ranges = southeast_gpio_ranges,
 	.ngpio_ranges = ARRAY_SIZE(southeast_gpio_ranges),
 	.ngpios = ARRAY_SIZE(southeast_pins),
+	.nirqs = 16,
 };
 
 static const struct chv_community *chv_communities[] = {
@@ -1497,7 +1513,7 @@ static void chv_gpio_irq_handler(struct irq_desc *desc)
 	chained_irq_enter(chip, desc);
 
 	pending = readl(pctrl->regs + CHV_INTSTAT);
-	for_each_set_bit(intr_line, &pending, 16) {
+	for_each_set_bit(intr_line, &pending, pctrl->community->nirqs) {
 		unsigned irq, offset;
 
 		offset = pctrl->intr_lines[intr_line];
@@ -1520,6 +1536,7 @@ static int chv_gpio_probe(struct chv_pinctrl *pctrl, int irq)
 	chip->label = dev_name(pctrl->dev);
 	chip->parent = pctrl->dev;
 	chip->base = -1;
+	chip->irq_need_valid_mask = true;
 
 	ret = gpiochip_add_data(chip, pctrl);
 	if (ret) {
@@ -1539,6 +1556,21 @@ static int chv_gpio_probe(struct chv_pinctrl *pctrl, int irq)
 		offset += range->npins;
 	}
 
+	/* Do not add GPIOs that can only generate GPEs to the IRQ domain */
+	for (i = 0; i < pctrl->community->npins; i++) {
+		const struct pinctrl_pin_desc *desc;
+		u32 intsel;
+
+		desc = &pctrl->community->pins[i];
+
+		intsel = readl(chv_padreg(pctrl, desc->number, CHV_PADCTRL0));
+		intsel &= CHV_PADCTRL0_INTSEL_MASK;
+		intsel >>= CHV_PADCTRL0_INTSEL_SHIFT;
+
+		if (intsel >= pctrl->community->nirqs)
+			clear_bit(i, chip->irq_valid_mask);
+	}
+
 	/* Clear all interrupts */
 	chv_writel(0xffff, pctrl->regs + CHV_INTSTAT);
 
-- 
2.9.3

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

* [PATCH v3 3/3] pinctrl: cherryview: Convert to use devm_gpiochip_add_data()
  2016-09-20 12:15 [PATCH v3 0/3] gpio / pinctrl: cherryview: Fix missing events from EC Mika Westerberg
  2016-09-20 12:15 ` [PATCH v3 1/3] gpiolib: Make it possible to exclude GPIOs from IRQ domain Mika Westerberg
  2016-09-20 12:15 ` [PATCH v3 2/3] pinctrl: cherryview: Do not add all southwest and north GPIOs to " Mika Westerberg
@ 2016-09-20 12:15 ` Mika Westerberg
  2016-09-23 13:00   ` Linus Walleij
  2016-10-11  6:23 ` [PATCH v3 0/3] gpio / pinctrl: cherryview: Fix missing events from EC Phidias Chiang
  3 siblings, 1 reply; 9+ messages in thread
From: Mika Westerberg @ 2016-09-20 12:15 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Marc Zyngier, Thomas Gleixner, Phidias Chiang, Anisse Astier,
	Heikki Krogerus, Yu C Chen, Mika Westerberg, linux-gpio,
	linux-kernel

This simplifies the error handling and allows us to drop the whole
chv_pinctrl_remove() function.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/pinctrl/intel/pinctrl-cherryview.c | 21 +++------------------
 1 file changed, 3 insertions(+), 18 deletions(-)

diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c
index d23be3a2fb35..30389f4ccab4 100644
--- a/drivers/pinctrl/intel/pinctrl-cherryview.c
+++ b/drivers/pinctrl/intel/pinctrl-cherryview.c
@@ -1538,7 +1538,7 @@ static int chv_gpio_probe(struct chv_pinctrl *pctrl, int irq)
 	chip->base = -1;
 	chip->irq_need_valid_mask = true;
 
-	ret = gpiochip_add_data(chip, pctrl);
+	ret = devm_gpiochip_add_data(pctrl->dev, chip, pctrl);
 	if (ret) {
 		dev_err(pctrl->dev, "Failed to register gpiochip\n");
 		return ret;
@@ -1550,7 +1550,7 @@ static int chv_gpio_probe(struct chv_pinctrl *pctrl, int irq)
 					     range->base, range->npins);
 		if (ret) {
 			dev_err(pctrl->dev, "failed to add GPIO pin range\n");
-			goto fail;
+			return ret;
 		}
 
 		offset += range->npins;
@@ -1578,17 +1578,12 @@ static int chv_gpio_probe(struct chv_pinctrl *pctrl, int irq)
 				   handle_bad_irq, IRQ_TYPE_NONE);
 	if (ret) {
 		dev_err(pctrl->dev, "failed to add IRQ chip\n");
-		goto fail;
+		return ret;
 	}
 
 	gpiochip_set_chained_irqchip(chip, &chv_gpio_irqchip, irq,
 				     chv_gpio_irq_handler);
 	return 0;
-
-fail:
-	gpiochip_remove(chip);
-
-	return ret;
 }
 
 static int chv_pinctrl_probe(struct platform_device *pdev)
@@ -1656,15 +1651,6 @@ static int chv_pinctrl_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int chv_pinctrl_remove(struct platform_device *pdev)
-{
-	struct chv_pinctrl *pctrl = platform_get_drvdata(pdev);
-
-	gpiochip_remove(&pctrl->chip);
-
-	return 0;
-}
-
 #ifdef CONFIG_PM_SLEEP
 static int chv_pinctrl_suspend(struct device *dev)
 {
@@ -1761,7 +1747,6 @@ MODULE_DEVICE_TABLE(acpi, chv_pinctrl_acpi_match);
 
 static struct platform_driver chv_pinctrl_driver = {
 	.probe = chv_pinctrl_probe,
-	.remove = chv_pinctrl_remove,
 	.driver = {
 		.name = "cherryview-pinctrl",
 		.pm = &chv_pinctrl_pm_ops,
-- 
2.9.3


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

* Re: [PATCH v3 1/3] gpiolib: Make it possible to exclude GPIOs from IRQ domain
  2016-09-20 12:15 ` [PATCH v3 1/3] gpiolib: Make it possible to exclude GPIOs from IRQ domain Mika Westerberg
@ 2016-09-23 12:47   ` Linus Walleij
  0 siblings, 0 replies; 9+ messages in thread
From: Linus Walleij @ 2016-09-23 12:47 UTC (permalink / raw)
  To: Mika Westerberg
  Cc: Marc Zyngier, Thomas Gleixner, Phidias Chiang, Anisse Astier,
	Heikki Krogerus, Yu C Chen, linux-gpio@vger.kernel.org,
	linux-kernel@vger.kernel.org

On Tue, Sep 20, 2016 at 2:15 PM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:

> When using GPIO irqchip helpers to setup irqchip for a gpiolib based
> driver, it is not possible to select which GPIOs to add to the IRQ domain.
> Instead it just adds all GPIOs which is not always desired. For example
> there might be GPIOs that for some reason cannot generated normal
> interrupts at all.
>
> To support this we add a flag irq_need_valid_mask to struct gpio_chip. When
> this flag is set the core allocates irq_valid_mask that holds one bit for
> each GPIO the chip has. By default all bits are set but drivers can
> manipulate this using set_bit() and clear_bit() accordingly.
>
> Then when gpiochip_irqchip_add() is called, this mask is checked and all
> GPIOs with bit is set are added to the IRQ domain created for the GPIO
> chip.
>
> Suggested-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Yeps, exactly like this!

Patch applied.

Yours,
Linus Walleij

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

* Re: [PATCH v3 2/3] pinctrl: cherryview: Do not add all southwest and north GPIOs to IRQ domain
  2016-09-20 12:15 ` [PATCH v3 2/3] pinctrl: cherryview: Do not add all southwest and north GPIOs to " Mika Westerberg
@ 2016-09-23 12:58   ` Linus Walleij
  2016-09-23 18:04     ` Mika Westerberg
  0 siblings, 1 reply; 9+ messages in thread
From: Linus Walleij @ 2016-09-23 12:58 UTC (permalink / raw)
  To: Mika Westerberg
  Cc: Marc Zyngier, Thomas Gleixner, Phidias Chiang, Anisse Astier,
	Heikki Krogerus, Yu C Chen, linux-gpio@vger.kernel.org,
	linux-kernel@vger.kernel.org

On Tue, Sep 20, 2016 at 2:15 PM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:

> It turns out that for north and southwest communities, they can only
> generate GPIO interrupts for lower 8 interrupts (IntSel value). The upper
> part (8-15) can only generate GPEs (General Purpose Events).
>
> Now the reason why EC events such as pressing hotkeys does not work if we
> mask all the interrupts is that in order to generate either interrupts or
> GPEs the INTMASK register must have that particular interrupt unmasked. In
> case of GPEs the CPU does not trigger normal interrupt (and thus the GPIO
> driver does not see it) but instead it causes SCI (System Control
> Interrupt) to be triggered with the GPE in question set.
>
> To make this all work as expected we only add those GPIOs to the IRQ domain
> that can actually generate interrupts (IntSel value 0-7) and skip others.
>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Patch applied, had to merge in the recent fix from -rc6 first but
after that it applied cleanly. Check the result please!

Yours,
Linus Walleij

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

* Re: [PATCH v3 3/3] pinctrl: cherryview: Convert to use devm_gpiochip_add_data()
  2016-09-20 12:15 ` [PATCH v3 3/3] pinctrl: cherryview: Convert to use devm_gpiochip_add_data() Mika Westerberg
@ 2016-09-23 13:00   ` Linus Walleij
  0 siblings, 0 replies; 9+ messages in thread
From: Linus Walleij @ 2016-09-23 13:00 UTC (permalink / raw)
  To: Mika Westerberg
  Cc: Marc Zyngier, Thomas Gleixner, Phidias Chiang, Anisse Astier,
	Heikki Krogerus, Yu C Chen, linux-gpio@vger.kernel.org,
	linux-kernel@vger.kernel.org

On Tue, Sep 20, 2016 at 2:15 PM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:

> This simplifies the error handling and allows us to drop the whole
> chv_pinctrl_remove() function.
>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Patch applied!

Yours,
Linus Walleij

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

* Re: [PATCH v3 2/3] pinctrl: cherryview: Do not add all southwest and north GPIOs to IRQ domain
  2016-09-23 12:58   ` Linus Walleij
@ 2016-09-23 18:04     ` Mika Westerberg
  0 siblings, 0 replies; 9+ messages in thread
From: Mika Westerberg @ 2016-09-23 18:04 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Marc Zyngier, Thomas Gleixner, Phidias Chiang, Anisse Astier,
	Heikki Krogerus, Yu C Chen, linux-gpio@vger.kernel.org,
	linux-kernel@vger.kernel.org

On Fri, Sep 23, 2016 at 02:58:47PM +0200, Linus Walleij wrote:
> On Tue, Sep 20, 2016 at 2:15 PM, Mika Westerberg
> <mika.westerberg@linux.intel.com> wrote:
> 
> > It turns out that for north and southwest communities, they can only
> > generate GPIO interrupts for lower 8 interrupts (IntSel value). The upper
> > part (8-15) can only generate GPEs (General Purpose Events).
> >
> > Now the reason why EC events such as pressing hotkeys does not work if we
> > mask all the interrupts is that in order to generate either interrupts or
> > GPEs the INTMASK register must have that particular interrupt unmasked. In
> > case of GPEs the CPU does not trigger normal interrupt (and thus the GPIO
> > driver does not see it) but instead it causes SCI (System Control
> > Interrupt) to be triggered with the GPE in question set.
> >
> > To make this all work as expected we only add those GPIOs to the IRQ domain
> > that can actually generate interrupts (IntSel value 0-7) and skip others.
> >
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> 
> Patch applied, had to merge in the recent fix from -rc6 first but
> after that it applied cleanly. Check the result please!

Looks good, thanks.

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

* Re: [PATCH v3 0/3] gpio / pinctrl: cherryview: Fix missing events from EC
  2016-09-20 12:15 [PATCH v3 0/3] gpio / pinctrl: cherryview: Fix missing events from EC Mika Westerberg
                   ` (2 preceding siblings ...)
  2016-09-20 12:15 ` [PATCH v3 3/3] pinctrl: cherryview: Convert to use devm_gpiochip_add_data() Mika Westerberg
@ 2016-10-11  6:23 ` Phidias Chiang
  3 siblings, 0 replies; 9+ messages in thread
From: Phidias Chiang @ 2016-10-11  6:23 UTC (permalink / raw)
  To: Mika Westerberg
  Cc: Linus Walleij, Marc Zyngier, Thomas Gleixner, Anisse Astier,
	Heikki Krogerus, Yu C Chen, linux-gpio, linux-kernel

Sorry for late response, the fix works like a charm :), thank you very
much!

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

end of thread, other threads:[~2016-10-11  6:23 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-20 12:15 [PATCH v3 0/3] gpio / pinctrl: cherryview: Fix missing events from EC Mika Westerberg
2016-09-20 12:15 ` [PATCH v3 1/3] gpiolib: Make it possible to exclude GPIOs from IRQ domain Mika Westerberg
2016-09-23 12:47   ` Linus Walleij
2016-09-20 12:15 ` [PATCH v3 2/3] pinctrl: cherryview: Do not add all southwest and north GPIOs to " Mika Westerberg
2016-09-23 12:58   ` Linus Walleij
2016-09-23 18:04     ` Mika Westerberg
2016-09-20 12:15 ` [PATCH v3 3/3] pinctrl: cherryview: Convert to use devm_gpiochip_add_data() Mika Westerberg
2016-09-23 13:00   ` Linus Walleij
2016-10-11  6:23 ` [PATCH v3 0/3] gpio / pinctrl: cherryview: Fix missing events from EC Phidias Chiang

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).