Skip to contentSkip to conversation

mfd: mc13xxx: Set the irq type.

PATCH Superseded Magnus Lilja 3 participants 7 messages Dec 30, 2016 – Jul 25, 2017

Subsystems (2)
Stable branch Multifunction devices (MFD)
lore
Share & tools
lore

Magnus Lilja

author
#0 lore raw

PATCH v2 1/1 mfd: mc13xxx: Set the irq type.

Commit 10f9edaeaa30 ("mfd: mc13xxx: Use regmap irq framework for interrupts") removed the passing of the IRQF_TRIGGER_HIGH flag when registering the interrupt.
This commit fixes that problem by setting the IRQF_TRIGGER_HIGH flag in case no irq type is set via irqd framework (e.g. device tree). In the latter case the irq flag from irqd is used.

Tested on i.MX31 PDK hardware.

Fixes 10f9edaeaa30 ("mfd: mc13xxx: Use regmap irq framework for interrupts")
Cc <[email protected]> # 3.18.x
Cc Lee Jones <[email protected]>
Signed-off-by Magnus Lilja <[email protected]>

---
Changes from v1 (which was part of a patch series):
- Now uses irqd_-functions to check if irq type is defined
- Added Fixes: and Cc: to stable kernel.

 drivers/mfd/mc13xxx-core.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/mfd/mc13xxx-core.c +7 -1
index d7f54e4..e1757ea 100644
@@ -15,6 +15,7 @@
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/mfd/core.h>
+#include <linux/irq.h>
 
 #include "mc13xxx.h"
 

@@ -410,6 +411,7 @@ int mc13xxx_common_init(struct device *dev)
 	struct mc13xxx *mc13xxx = dev_get_drvdata(dev);
 	u32 revision;
 	int i, ret;
+	unsigned int flags;
 
 	mc13xxx->dev = dev;
 

@@ -440,7 +442,11 @@ int mc13xxx_common_init(struct device *dev)
 	mc13xxx->irq_chip.irqs = mc13xxx->irqs;
 	mc13xxx->irq_chip.num_irqs = ARRAY_SIZE(mc13xxx->irqs);
 
-	ret = regmap_add_irq_chip(mc13xxx->regmap, mc13xxx->irq, IRQF_ONESHOT,
+	flags = irqd_get_trigger_type(irq_get_irq_data(mc13xxx->irq));
+	flags = (flags == IRQ_TYPE_NONE) ? IRQF_TRIGGER_HIGH : flags;
+
+	ret = regmap_add_irq_chip(mc13xxx->regmap, mc13xxx->irq,
+				  IRQF_ONESHOT | flags,
 				  0, &mc13xxx->irq_chip, &mc13xxx->irq_data);
 	if (ret)
 		return ret;
Signature · 1 line
2.7.4
#1 lore raw

PATCH v2 1/1 mfd: mc13xxx: Set the irq type.

Quoted 7 lines · Magnus Lilja

Commit 10f9edaeaa30 ("mfd: mc13xxx: Use regmap irq framework for interrupts") removed the passing of the IRQF_TRIGGER_HIGH flag when registering the interrupt.
This commit fixes that problem by setting the IRQF_TRIGGER_HIGH flag in case no irq type is set via irqd framework (e.g. device tree). In the latter case the irq flag from irqd is used.

Quoted 6 lines

Tested on i.MX31 PDK hardware.

Fixes 10f9edaeaa30 ("mfd: mc13xxx: Use regmap irq framework for interrupts")
Cc <[email protected]> # 3.18.x
Cc Lee Jones <[email protected]>
Signed-off-by Magnus Lilja <[email protected]>
Reviewed-by Fabio Estevam <[email protected]>
#2 lore raw

PATCH v2 1/1 mfd: mc13xxx: Set the irq type.

In reply to Magnus Lilja
Quoted 6 lines · Magnus Lilja

Commit 10f9edaeaa30 ("mfd: mc13xxx: Use regmap irq framework for interrupts") removed the passing of the IRQF_TRIGGER_HIGH flag when registering the interrupt.
This commit fixes that problem by setting the IRQF_TRIGGER_HIGH flag in case no irq type is set via irqd framework (e.g. device tree). In the latter case the irq flag from irqd is used.

This looks like a hack.

Why can't you set the trigger type in Device Tree instead?

Quoted 47 lines

Tested on i.MX31 PDK hardware.

Fixes 10f9edaeaa30 ("mfd: mc13xxx: Use regmap irq framework for interrupts")
Cc <[email protected]> # 3.18.x
Cc Lee Jones <[email protected]>
Signed-off-by Magnus Lilja <[email protected]>

---
Changes from v1 (which was part of a patch series):
- Now uses irqd_-functions to check if irq type is defined
- Added Fixes: and Cc: to stable kernel.

 drivers/mfd/mc13xxx-core.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/mfd/mc13xxx-core.c +7 -1
index d7f54e4..e1757ea 100644
@@ -15,6 +15,7 @@
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/mfd/core.h>
+#include <linux/irq.h>
 
 #include "mc13xxx.h"
 

@@ -410,6 +411,7 @@ int mc13xxx_common_init(struct device *dev)
 	struct mc13xxx *mc13xxx = dev_get_drvdata(dev);
 	u32 revision;
 	int i, ret;
+	unsigned int flags;
 
 	mc13xxx->dev = dev;
 

@@ -440,7 +442,11 @@ int mc13xxx_common_init(struct device *dev)
 	mc13xxx->irq_chip.irqs = mc13xxx->irqs;
 	mc13xxx->irq_chip.num_irqs = ARRAY_SIZE(mc13xxx->irqs);
 
-	ret = regmap_add_irq_chip(mc13xxx->regmap, mc13xxx->irq, IRQF_ONESHOT,
+	flags = irqd_get_trigger_type(irq_get_irq_data(mc13xxx->irq));
+	flags = (flags == IRQ_TYPE_NONE) ? IRQF_TRIGGER_HIGH : flags;
+
+	ret = regmap_add_irq_chip(mc13xxx->regmap, mc13xxx->irq,
+				  IRQF_ONESHOT | flags,
 				  0, &mc13xxx->irq_chip, &mc13xxx->irq_data);
 	if (ret)
 		return ret;
Signature · 4 lines
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
author
#3 lore raw

PATCH v2 1/1 mfd: mc13xxx: Set the irq type.

Hi,

Quoted 9 lines · Lee Jones
Quoted 6 lines · Magnus Lilja

Commit 10f9edaeaa30 ("mfd: mc13xxx: Use regmap irq framework for interrupts") removed the passing of the IRQF_TRIGGER_HIGH flag when registering the interrupt.
This commit fixes that problem by setting the IRQF_TRIGGER_HIGH flag in case no irq type is set via irqd framework (e.g. device tree). In the latter case the irq flag from irqd is used.

Quoted 3 lines

This looks like a hack.

Why can't you set the trigger type in Device Tree instead?

The i.MX31 PDK board has not, like many (all?) i.MX31 boards, not been converted to use device tree yet. I think there is work in progress in this area. However, as the IRQF_TRIGGER problem also affects several stable kernel series (since 3.18.x) I thought it was worthwhile to fix this.

Regards, Magnus

Quoted 53 lines
Quoted 37 lines

Tested on i.MX31 PDK hardware.

Fixes 10f9edaeaa30 ("mfd: mc13xxx: Use regmap irq framework for interrupts")
Cc <[email protected]> # 3.18.x
Cc Lee Jones <[email protected]>
Signed-off-by Magnus Lilja <[email protected]>

---
Changes from v1 (which was part of a patch series):
- Now uses irqd_-functions to check if irq type is defined
- Added Fixes: and Cc: to stable kernel.

 drivers/mfd/mc13xxx-core.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/mfd/mc13xxx-core.c +2 -0
index d7f54e4..e1757ea 100644
@@ -15,6 +15,7 @@
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/mfd/core.h>
+#include <linux/irq.h>
 
 #include "mc13xxx.h"
 

@@ -410,6 +411,7 @@ int mc13xxx_common_init(struct device *dev)
      struct mc13xxx *mc13xxx = dev_get_drvdata(dev);
      u32 revision;
      int i, ret;
+     unsigned int flags;
 
      mc13xxx->dev = dev;
 

@@ -440,7 +442,11 @@ int mc13xxx_common_init(struct device *dev)
      mc13xxx->irq_chip.irqs = mc13xxx->irqs;
      mc13xxx->irq_chip.num_irqs = ARRAY_SIZE(mc13xxx->irqs);

drivers/mfd/mc13xxx-core.c int mc13xxx_common_init(struct device *dev) · line 452

Quoted 10 lines
drivers/mfd/mc13xxx-core.c +5 -1
 
-     ret = regmap_add_irq_chip(mc13xxx->regmap, mc13xxx->irq, IRQF_ONESHOT,
+     flags = irqd_get_trigger_type(irq_get_irq_data(mc13xxx->irq));
+     flags = (flags == IRQ_TYPE_NONE) ? IRQF_TRIGGER_HIGH : flags;
+
+     ret = regmap_add_irq_chip(mc13xxx->regmap, mc13xxx->irq,
+                               IRQF_ONESHOT | flags,
                                0, &mc13xxx->irq_chip, &mc13xxx->irq_data);
      if (ret)
              return ret;

--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

#4 lore raw

PATCH v2 1/1 mfd: mc13xxx: Set the irq type.

Thomas,

Quoted 12 lines · Magnus Lilja
Quoted 11 lines · Lee Jones
Quoted 6 lines · Magnus Lilja

Commit 10f9edaeaa30 ("mfd: mc13xxx: Use regmap irq framework for interrupts") removed the passing of the IRQF_TRIGGER_HIGH flag when registering the interrupt.
This commit fixes that problem by setting the IRQF_TRIGGER_HIGH flag in case no irq type is set via irqd framework (e.g. device tree). In the latter case the irq flag from irqd is used.

This looks like a hack.

Quoted 7 lines
Quoted 1 line

Why can't you set the trigger type in Device Tree instead?

The i.MX31 PDK board has not, like many (all?) i.MX31 boards, not been converted to use device tree yet. I think there is work in progress in this area. However, as the IRQF_TRIGGER problem also affects several stable kernel series (since 3.18.x) I thought it was worthwhile to fix this.

I would like Thomas' advice on this.

Quoted 48 lines
Quoted 48 lines
Quoted 47 lines

Tested on i.MX31 PDK hardware.

Fixes 10f9edaeaa30 ("mfd: mc13xxx: Use regmap irq framework for interrupts")
Cc <[email protected]> # 3.18.x
Cc Lee Jones <[email protected]>
Signed-off-by Magnus Lilja <[email protected]>

---
Changes from v1 (which was part of a patch series):
- Now uses irqd_-functions to check if irq type is defined
- Added Fixes: and Cc: to stable kernel.

 drivers/mfd/mc13xxx-core.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/mfd/mc13xxx-core.c +7 -1
index d7f54e4..e1757ea 100644
@@ -15,6 +15,7 @@
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/mfd/core.h>
+#include <linux/irq.h>
 
 #include "mc13xxx.h"
 

@@ -410,6 +411,7 @@ int mc13xxx_common_init(struct device *dev)
      struct mc13xxx *mc13xxx = dev_get_drvdata(dev);
      u32 revision;
      int i, ret;
+     unsigned int flags;
 
      mc13xxx->dev = dev;
 

@@ -440,7 +442,11 @@ int mc13xxx_common_init(struct device *dev)
      mc13xxx->irq_chip.irqs = mc13xxx->irqs;
      mc13xxx->irq_chip.num_irqs = ARRAY_SIZE(mc13xxx->irqs);
 
-     ret = regmap_add_irq_chip(mc13xxx->regmap, mc13xxx->irq, IRQF_ONESHOT,
+     flags = irqd_get_trigger_type(irq_get_irq_data(mc13xxx->irq));
+     flags = (flags == IRQ_TYPE_NONE) ? IRQF_TRIGGER_HIGH : flags;
+
+     ret = regmap_add_irq_chip(mc13xxx->regmap, mc13xxx->irq,
+                               IRQF_ONESHOT | flags,
                                0, &mc13xxx->irq_chip, &mc13xxx->irq_data);
      if (ret)
              return ret;
Signature · 4 lines
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
author
#5 lore raw

PATCH v2 1/1 mfd: mc13xxx: Set the irq type.

Lee, Thomas,

I don't think I saw an answer from Thomas in this matter.

( http://lists.infradead.org/pipermail/linux-arm-kernel/2017-January/476699.html
)

Regards, Magnus

Quoted 79 lines · Lee Jones

Thomas,

Quoted 12 lines · Magnus Lilja
Quoted 11 lines · Lee Jones
Quoted 6 lines · Magnus Lilja

Commit 10f9edaeaa30 ("mfd: mc13xxx: Use regmap irq framework for interrupts") removed the passing of the IRQF_TRIGGER_HIGH flag when registering the interrupt.
This commit fixes that problem by setting the IRQF_TRIGGER_HIGH flag in case no irq type is set via irqd framework (e.g. device tree). In the latter case the irq flag from irqd is used.

This looks like a hack.

Quoted 7 lines
Quoted 1 line

Why can't you set the trigger type in Device Tree instead?

The i.MX31 PDK board has not, like many (all?) i.MX31 boards, not been converted to use device tree yet. I think there is work in progress in this area. However, as the IRQF_TRIGGER problem also affects several stable kernel series (since 3.18.x) I thought it was worthwhile to fix this.

I would like Thomas' advice on this.

Quoted 38 lines
Quoted 38 lines
Quoted 38 lines

Tested on i.MX31 PDK hardware.

Fixes 10f9edaeaa30 ("mfd: mc13xxx: Use regmap irq framework for interrupts")
Cc <[email protected]> # 3.18.x
Cc Lee Jones <[email protected]>
Signed-off-by Magnus Lilja <[email protected]>

---
Changes from v1 (which was part of a patch series):
- Now uses irqd_-functions to check if irq type is defined
- Added Fixes: and Cc: to stable kernel.

 drivers/mfd/mc13xxx-core.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/mfd/mc13xxx-core.c +2 -0
index d7f54e4..e1757ea 100644
@@ -15,6 +15,7 @@
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/mfd/core.h>
+#include <linux/irq.h>
 
 #include "mc13xxx.h"
 

@@ -410,6 +411,7 @@ int mc13xxx_common_init(struct device *dev)
      struct mc13xxx *mc13xxx = dev_get_drvdata(dev);
      u32 revision;
      int i, ret;
+     unsigned int flags;
 
      mc13xxx->dev = dev;
 

@@ -440,7 +442,11 @@ int mc13xxx_common_init(struct device *dev)
      mc13xxx->irq_chip.irqs = mc13xxx->irqs;
      mc13xxx->irq_chip.num_irqs = ARRAY_SIZE(mc13xxx->irqs);
Quoted 10 lines
Quoted 10 lines

drivers/mfd/mc13xxx-core.c int mc13xxx_common_init(struct device *dev) · line 452

Quoted 9 lines
drivers/mfd/mc13xxx-core.c +5 -1
-     ret = regmap_add_irq_chip(mc13xxx->regmap, mc13xxx->irq, IRQF_ONESHOT,
+     flags = irqd_get_trigger_type(irq_get_irq_data(mc13xxx->irq));
+     flags = (flags == IRQ_TYPE_NONE) ? IRQF_TRIGGER_HIGH : flags;
+
+     ret = regmap_add_irq_chip(mc13xxx->regmap, mc13xxx->irq,
+                               IRQF_ONESHOT | flags,
                                0, &mc13xxx->irq_chip, &mc13xxx->irq_data);
      if (ret)
              return ret;

--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

#6 lore raw

PATCH v2 1/1 mfd: mc13xxx: Set the irq type.

Quoted 6 lines · Magnus Lilja

Lee, Thomas,

I don't think I saw an answer from Thomas in this matter.

( http://lists.infradead.org/pipermail/linux-arm-kernel/2017-January/476699.html
)

No, me either.

Hopefully he will reply now.

Quoted 75 lines · Lee Jones
Quoted 74 lines

Thomas,

Quoted 12 lines · Magnus Lilja
Quoted 11 lines · Lee Jones
Quoted 6 lines · Magnus Lilja

Commit 10f9edaeaa30 ("mfd: mc13xxx: Use regmap irq framework for interrupts") removed the passing of the IRQF_TRIGGER_HIGH flag when registering the interrupt.
This commit fixes that problem by setting the IRQF_TRIGGER_HIGH flag in case no irq type is set via irqd framework (e.g. device tree). In the latter case the irq flag from irqd is used.

This looks like a hack.

Quoted 7 lines
Quoted 1 line

Why can't you set the trigger type in Device Tree instead?

The i.MX31 PDK board has not, like many (all?) i.MX31 boards, not been converted to use device tree yet. I think there is work in progress in this area. However, as the IRQF_TRIGGER problem also affects several stable kernel series (since 3.18.x) I thought it was worthwhile to fix this.

I would like Thomas' advice on this.

Quoted 48 lines
Quoted 48 lines
Quoted 47 lines

Tested on i.MX31 PDK hardware.

Fixes 10f9edaeaa30 ("mfd: mc13xxx: Use regmap irq framework for interrupts")
Cc <[email protected]> # 3.18.x
Cc Lee Jones <[email protected]>
Signed-off-by Magnus Lilja <[email protected]>

---
Changes from v1 (which was part of a patch series):
- Now uses irqd_-functions to check if irq type is defined
- Added Fixes: and Cc: to stable kernel.

 drivers/mfd/mc13xxx-core.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/mfd/mc13xxx-core.c +7 -1
index d7f54e4..e1757ea 100644
@@ -15,6 +15,7 @@
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/mfd/core.h>
+#include <linux/irq.h>
 
 #include "mc13xxx.h"
 

@@ -410,6 +411,7 @@ int mc13xxx_common_init(struct device *dev)
      struct mc13xxx *mc13xxx = dev_get_drvdata(dev);
      u32 revision;
      int i, ret;
+     unsigned int flags;
 
      mc13xxx->dev = dev;
 

@@ -440,7 +442,11 @@ int mc13xxx_common_init(struct device *dev)
      mc13xxx->irq_chip.irqs = mc13xxx->irqs;
      mc13xxx->irq_chip.num_irqs = ARRAY_SIZE(mc13xxx->irqs);
 
-     ret = regmap_add_irq_chip(mc13xxx->regmap, mc13xxx->irq, IRQF_ONESHOT,
+     flags = irqd_get_trigger_type(irq_get_irq_data(mc13xxx->irq));
+     flags = (flags == IRQ_TYPE_NONE) ? IRQF_TRIGGER_HIGH : flags;
+
+     ret = regmap_add_irq_chip(mc13xxx->regmap, mc13xxx->irq,
+                               IRQF_ONESHOT | flags,
                                0, &mc13xxx->irq_chip, &mc13xxx->irq_data);
      if (ret)
              return ret;
Signature · 4 lines
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog