[PATCH v8 0/3] ACPI: Add irq_type to gpio interrupt

[PATCH v8 0/3] ACPI: Add irq_type to gpio interrupt

[Christophe Ricard]

Hi,

ACPI probing method does not retrieve irq_type from a gpio interrupt declared
with GpioInt as it is done with devicetree probing. In other terms, irq_get_trigger_type
will always send back 0.

Those 3 patches propose a way to retrieve the correct interrupt polarity/type from a GpioInt
acpi declaration when using irq_get_trigger_type.

Since v4 after Mika and Andy's feedbacks:
- acpi_gpio_info active_low field got renamed polarity. Note that polarity/active_low is having sense
also when a gpio is not mapped as an interrupt but as an output (For example: check gpiod_set_value
in drivers/gpio/gpiolib.c).
- acpi_gsi_get_irq_type is now renamed acpi_dev_get_irq_type and moved to drivers/acpi/resource.c
- The irq described in the acpi spi slave node is now set once we fall back to retrieve normal interrupt.

Since v5 after Andy's feedbacks:
- Make sure we are not using u* type but pure C type for non hardware value.

Since v6:
- Patches now applies on top of Raphael tree tag linux-next

Since v7:
- Added "Acked-by: Mika Westerberg <mika.westerberg-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>" to patch 1.
- Added "Acked-by: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>" to patch 3.
- Patch 3 got reworked following Mika's feedback.
- Patch 2 added few comment in acpi_find_gpio following Mika's feedback.

Best Regards
Christophe 

Christophe Ricard (3):
  acpi: Rename acpi_gsi_get_irq_type to acpi_dev_get_irq_type and export
    symbol
  ACPI / gpio: Add irq_type when a gpio is used as an interrupt
  ACPI / spi: attach gpio irq from acpi description to spi device

 drivers/acpi/gsi.c          | 21 +--------------------
 drivers/acpi/resource.c     | 26 ++++++++++++++++++++++++++
 drivers/gpio/gpiolib-acpi.c | 33 +++++++++++++++++++++++++++------
 drivers/gpio/gpiolib.c      |  4 ++--
 drivers/gpio/gpiolib.h      |  3 ++-
 drivers/spi/spi.c           |  3 +++
 include/linux/acpi.h        |  1 +
 7 files changed, 62 insertions(+), 29 deletions(-)

-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v8 1/3] acpi: Rename acpi_gsi_get_irq_type to acpi_dev_get_irq_type and export symbol

[Christophe Ricard]

acpi_gsi_get_irq_type could be use out of gsi purpose.

Rename and make it available as a resource function.

Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Christophe Ricard <christophe-h.ricard@st.com>
---
 drivers/acpi/gsi.c      | 21 +--------------------
 drivers/acpi/resource.c | 26 ++++++++++++++++++++++++++
 include/linux/acpi.h    |  1 +
 3 files changed, 28 insertions(+), 20 deletions(-)

diff --git a/drivers/acpi/gsi.c b/drivers/acpi/gsi.c
index fa4585a..ee9e0f2 100644
--- a/drivers/acpi/gsi.c
+++ b/drivers/acpi/gsi.c
@@ -17,25 +17,6 @@ enum acpi_irq_model_id acpi_irq_model;
 
 static struct fwnode_handle *acpi_gsi_domain_id;
 
-static unsigned int acpi_gsi_get_irq_type(int trigger, int polarity)
-{
-	switch (polarity) {
-	case ACPI_ACTIVE_LOW:
-		return trigger == ACPI_EDGE_SENSITIVE ?
-		       IRQ_TYPE_EDGE_FALLING :
-		       IRQ_TYPE_LEVEL_LOW;
-	case ACPI_ACTIVE_HIGH:
-		return trigger == ACPI_EDGE_SENSITIVE ?
-		       IRQ_TYPE_EDGE_RISING :
-		       IRQ_TYPE_LEVEL_HIGH;
-	case ACPI_ACTIVE_BOTH:
-		if (trigger == ACPI_EDGE_SENSITIVE)
-			return IRQ_TYPE_EDGE_BOTH;
-	default:
-		return IRQ_TYPE_NONE;
-	}
-}
-
 /**
  * acpi_gsi_to_irq() - Retrieve the linux irq number for a given GSI
  * @gsi: GSI IRQ number to map
@@ -82,7 +63,7 @@ int acpi_register_gsi(struct device *dev, u32 gsi, int trigger,
 
 	fwspec.fwnode = acpi_gsi_domain_id;
 	fwspec.param[0] = gsi;
-	fwspec.param[1] = acpi_gsi_get_irq_type(trigger, polarity);
+	fwspec.param[1] = acpi_dev_get_irq_type(trigger, polarity);
 	fwspec.param_count = 2;
 
 	return irq_create_fwspec_mapping(&fwspec);
diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
index cdc5c25..d02fd53 100644
--- a/drivers/acpi/resource.c
+++ b/drivers/acpi/resource.c
@@ -23,6 +23,7 @@
 #include <linux/export.h>
 #include <linux/ioport.h>
 #include <linux/slab.h>
+#include <linux/irq.h>
 
 #ifdef CONFIG_X86
 #define valid_IRQ(i) (((i) != 0) && ((i) != 2))
@@ -336,6 +337,31 @@ unsigned long acpi_dev_irq_flags(u8 triggering, u8 polarity, u8 shareable)
 }
 EXPORT_SYMBOL_GPL(acpi_dev_irq_flags);
 
+/**
+ * acpi_dev_get_irq_type - Determine irq type.
+ * @triggering: Triggering type as provided by ACPI.
+ * @polarity: Interrupt polarity as provided by ACPI.
+ */
+unsigned int acpi_dev_get_irq_type(int triggering, int polarity)
+{
+	switch (polarity) {
+	case ACPI_ACTIVE_LOW:
+		return triggering == ACPI_EDGE_SENSITIVE ?
+		       IRQ_TYPE_EDGE_FALLING :
+		       IRQ_TYPE_LEVEL_LOW;
+	case ACPI_ACTIVE_HIGH:
+		return triggering == ACPI_EDGE_SENSITIVE ?
+		       IRQ_TYPE_EDGE_RISING :
+		       IRQ_TYPE_LEVEL_HIGH;
+	case ACPI_ACTIVE_BOTH:
+		if (triggering == ACPI_EDGE_SENSITIVE)
+			return IRQ_TYPE_EDGE_BOTH;
+	default:
+		return IRQ_TYPE_NONE;
+	}
+}
+EXPORT_SYMBOL_GPL(acpi_dev_get_irq_type);
+
 static void acpi_dev_irqresource_disabled(struct resource *res, u32 gsi)
 {
 	res->start = gsi;
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 1991aea..42adb80 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -318,6 +318,7 @@ bool acpi_dev_resource_address_space(struct acpi_resource *ares,
 bool acpi_dev_resource_ext_address_space(struct acpi_resource *ares,
 					 struct resource_win *win);
 unsigned long acpi_dev_irq_flags(u8 triggering, u8 polarity, u8 shareable);
+unsigned int acpi_dev_get_irq_type(int triggering, int polarity);
 bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
 				 struct resource *res);
 
-- 
2.1.4

[PATCH v8 2/3] ACPI / gpio: Add irq_type when a gpio is used as an interrupt

[Christophe Ricard]

When a gpio is used as an interrupt in acpi, the irq_type was not
available for device driver.

Make available polarity and triggering information in acpi_find_gpio by
renaming acpi_gpio_info field active_low  to polarity and adding triggering
field (edge/level).
For sanity, in gpiolib.c replace info.active_low by
"info.polarity == GPIO_ACTIVE_LOW".

Set the irq_type if necessary in acpi_dev_gpio_irq_get.

Signed-off-by: Christophe Ricard <christophe-h.ricard@st.com>
---
 drivers/gpio/gpiolib-acpi.c | 33 +++++++++++++++++++++++++++------
 drivers/gpio/gpiolib.c      |  4 ++--
 drivers/gpio/gpiolib.h      |  3 ++-
 3 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 16a7b68..cbbb67a 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -417,10 +417,15 @@ static int acpi_find_gpio(struct acpi_resource *ares, void *data)
 		 * ActiveLow is only specified for GpioInt resource. If
 		 * GpioIo is used then the only way to set the flag is
 		 * to use _DSD "gpios" property.
+		 * Note: we expect here:
+		 * - ACPI_ACTIVE_LOW == GPIO_ACTIVE_LOW
+		 * - ACPI_ACTIVE_HIGH == GPIO_ACTIVE_HIGH
 		 */
-		if (lookup->info.gpioint)
-			lookup->info.active_low =
-				agpio->polarity == ACPI_ACTIVE_LOW;
+		if (lookup->info.gpioint) {
+			lookup->info.polarity = agpio->polarity;
+			lookup->info.triggering = agpio->triggering;
+		}
+
 	}
 
 	return 1;
@@ -447,7 +452,7 @@ static int acpi_gpio_resource_lookup(struct acpi_gpio_lookup *lookup,
 	if (info) {
 		*info = lookup->info;
 		if (lookup->active_low)
-			info->active_low = lookup->active_low;
+			info->polarity = lookup->active_low;
 	}
 	return 0;
 }
@@ -595,6 +600,7 @@ struct gpio_desc *acpi_node_get_gpiod(struct fwnode_handle *fwnode,
 int acpi_dev_gpio_irq_get(struct acpi_device *adev, int index)
 {
 	int idx, i;
+	unsigned int irq_flags;
 
 	for (i = 0, idx = 0; idx <= index; i++) {
 		struct acpi_gpio_info info;
@@ -603,8 +609,23 @@ int acpi_dev_gpio_irq_get(struct acpi_device *adev, int index)
 		desc = acpi_get_gpiod_by_index(adev, NULL, i, &info);
 		if (IS_ERR(desc))
 			break;
-		if (info.gpioint && idx++ == index)
-			return gpiod_to_irq(desc);
+		if (info.gpioint && idx++ == index) {
+			int irq = gpiod_to_irq(desc);
+
+			if (irq < 0)
+				return irq;
+
+			irq_flags = acpi_dev_get_irq_type(info.triggering,
+							  info.polarity);
+
+			/* Set type if specified and different than the current one */
+			if (irq_flags != IRQ_TYPE_NONE &&
+			    irq_flags != irq_get_trigger_type(irq))
+				irq_set_irq_type(irq, irq_flags);
+
+			return irq;
+		}
+
 	}
 	return -ENOENT;
 }
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 2a91f32..5b67177 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1873,7 +1873,7 @@ static struct gpio_desc *acpi_find_gpio(struct device *dev, const char *con_id,
 			return desc;
 	}
 
-	if (info.active_low)
+	if (info.polarity == GPIO_ACTIVE_LOW)
 		*flags |= GPIO_ACTIVE_LOW;
 
 	return desc;
@@ -2211,7 +2211,7 @@ struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode,
 
 		desc = acpi_node_get_gpiod(fwnode, propname, 0, &info);
 		if (!IS_ERR(desc))
-			active_low = info.active_low;
+			active_low = info.polarity == GPIO_ACTIVE_LOW;
 	}
 
 	if (IS_ERR(desc))
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index 98ab08c..5ac3b88 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -26,7 +26,8 @@ struct acpi_device;
  */
 struct acpi_gpio_info {
 	bool gpioint;
-	bool active_low;
+	int polarity;
+	int triggering;
 };
 
 /* gpio suffixes used for ACPI and device tree lookup */
-- 
2.1.4

Re: [PATCH v8 2/3] ACPI / gpio: Add irq_type when a gpio is used as an interrupt

[Mika Westerberg]

On Wed, Dec 23, 2015 at 11:25:34PM +0100, Christophe Ricard wrote:
> When a gpio is used as an interrupt in acpi, the irq_type was not
> available for device driver.
> 
> Make available polarity and triggering information in acpi_find_gpio by
> renaming acpi_gpio_info field active_low  to polarity and adding triggering
> field (edge/level).
> For sanity, in gpiolib.c replace info.active_low by
> "info.polarity == GPIO_ACTIVE_LOW".
> 
> Set the irq_type if necessary in acpi_dev_gpio_irq_get.
> 
> Signed-off-by: Christophe Ricard <christophe-h.ricard-qxv4g6HH51o@public.gmane.org>

Acked-by: Mika Westerberg <mika.westerberg-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v8 3/3] ACPI / spi: attach gpio irq from acpi description to spi device

[Christophe Ricard]

spi->irq was ignoring GpioInt property setting it to -1.
acpi_dev_gpio_irq_get returns and configure the slave irq according to
the acpi slave node description.
It is now inline with devicetree behavior.

Acked-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Christophe Ricard <christophe-h.ricard@st.com>
---
 drivers/spi/spi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 2b0a8ec..f1723bb 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1623,6 +1623,9 @@ static acpi_status acpi_spi_add_device(acpi_handle handle, u32 level,
 		return AE_OK;
 	}
 
+	if (spi->irq < 0)
+		spi->irq = acpi_dev_gpio_irq_get(adev, 0);
+
 	adev->power.flags.ignore_parent = true;
 	strlcpy(spi->modalias, acpi_device_hid(adev), sizeof(spi->modalias));
 	if (spi_add_device(spi)) {
-- 
2.1.4

Re: [PATCH v8 3/3] ACPI / spi: attach gpio irq from acpi description to spi device

[Mika Westerberg]

On Wed, Dec 23, 2015 at 11:25:35PM +0100, Christophe Ricard wrote:
> spi->irq was ignoring GpioInt property setting it to -1.
> acpi_dev_gpio_irq_get returns and configure the slave irq according to
> the acpi slave node description.
> It is now inline with devicetree behavior.
> 
> Acked-by: Mark Brown <broonie@kernel.org>
> Signed-off-by: Christophe Ricard <christophe-h.ricard@st.com>

Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Re: [PATCH v8 0/3] ACPI: Add irq_type to gpio interrupt

[Christophe Ricard]

Hi,

Do you expect any other update from me ?
Is there Anything else needed to this patchset to hit 4.5 release ?

Best Regards
Christophe

On 23/12/2015 23:25, Christophe Ricard wrote:
> Hi,
>
> ACPI probing method does not retrieve irq_type from a gpio interrupt declared
> with GpioInt as it is done with devicetree probing. In other terms, irq_get_trigger_type
> will always send back 0.
>
> Those 3 patches propose a way to retrieve the correct interrupt polarity/type from a GpioInt
> acpi declaration when using irq_get_trigger_type.
>
> Since v4 after Mika and Andy's feedbacks:
> - acpi_gpio_info active_low field got renamed polarity. Note that polarity/active_low is having sense
> also when a gpio is not mapped as an interrupt but as an output (For example: check gpiod_set_value
> in drivers/gpio/gpiolib.c).
> - acpi_gsi_get_irq_type is now renamed acpi_dev_get_irq_type and moved to drivers/acpi/resource.c
> - The irq described in the acpi spi slave node is now set once we fall back to retrieve normal interrupt.
>
> Since v5 after Andy's feedbacks:
> - Make sure we are not using u* type but pure C type for non hardware value.
>
> Since v6:
> - Patches now applies on top of Raphael tree tag linux-next
>
> Since v7:
> - Added "Acked-by: Mika Westerberg <mika.westerberg-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>" to patch 1.
> - Added "Acked-by: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>" to patch 3.
> - Patch 3 got reworked following Mika's feedback.
> - Patch 2 added few comment in acpi_find_gpio following Mika's feedback.
>
> Best Regards
> Christophe
>
> Christophe Ricard (3):
>    acpi: Rename acpi_gsi_get_irq_type to acpi_dev_get_irq_type and export
>      symbol
>    ACPI / gpio: Add irq_type when a gpio is used as an interrupt
>    ACPI / spi: attach gpio irq from acpi description to spi device
>
>   drivers/acpi/gsi.c          | 21 +--------------------
>   drivers/acpi/resource.c     | 26 ++++++++++++++++++++++++++
>   drivers/gpio/gpiolib-acpi.c | 33 +++++++++++++++++++++++++++------
>   drivers/gpio/gpiolib.c      |  4 ++--
>   drivers/gpio/gpiolib.h      |  3 ++-
>   drivers/spi/spi.c           |  3 +++
>   include/linux/acpi.h        |  1 +
>   7 files changed, 62 insertions(+), 29 deletions(-)
>

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 0/3] ACPI: Add irq_type to gpio interrupt

[Rafael J. Wysocki]

On Wednesday, December 30, 2015 06:40:21 PM Christophe Ricard wrote:
> Hi,
> 
> Do you expect any other update from me ?
> Is there Anything else needed to this patchset to hit 4.5 release ?

No.

I'm going to add it to my queue for 4.5 shortly.

Thanks,
Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html