ARM: dts: exynos: correct fuel gauge interrupt trigger level on GT-I9100

The Maxim fuel gauge datasheets describe the interrupt line as active low with a requirement of acknowledge from the CPU. The falling edge interrupt will mostly work but it's not correct.

Fixes e8614292cd41 ("ARM: dts: Add Maxim 77693 fuel gauge node for exynos4412-trats2")

Signed-off-by Krzysztof Kozlowski <[email protected]>

---
 arch/arm/boot/dts/exynos4412-midas.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/arm/boot/dts/exynos4412-midas.dtsi +1 -1

index 111c32bae02c..b8b75dc81aa1 100644
@@ -221,7 +221,7 @@ i2c_max77693_fuel: i2c-gpio-3 {
 		fuel-gauge@36 {
 			compatible = "maxim,max17047";
 			interrupt-parent = <&gpx2>;
-			interrupts = <3 IRQ_TYPE_EDGE_FALLING>;
+			interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
 			pinctrl-names = "default";
 			pinctrl-0 = <&max77693_fuel_irq>;
 			reg = <0x36>;

Signature · 1 line

2.25.1

The Maxim fuel gauge datasheets describe the interrupt line as active low with a requirement of acknowledge from the CPU.  The falling edge interrupt will mostly work but it's not correct.

Fixes 45dfa741df86 ("ARM: dts: qcom: msm8974-lge-nexus5: Add fuel gauge")

Signed-off-by Krzysztof Kozlowski <[email protected]>

---
  arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts | 2 +-

  1 file changed, 1 insertion(+), 1 deletion(-)

arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts +0 -0

index e769f638f205..4c6f54aa9f66 100644

The Maxim fuel gauge datasheets describe the interrupt line as active low with a requirement of acknowledge from the CPU.  The falling edge interrupt will mostly work but it's not correct.

Fixes da8d46992e67 ("ARM: dts: qcom: msm8974-klte: Add fuel gauge")

Signed-off-by Krzysztof Kozlowski <[email protected]>

---
  arch/arm/boot/dts/qcom-msm8974-samsung-klte.dts | 2 +-

  1 file changed, 1 insertion(+), 1 deletion(-)

arch/arm/boot/dts/qcom-msm8974-samsung-klte.dts +0 -0

index 97352de91314..64a3fdb79539 100644

Interrupt line can be configured on different hardware in different way, even inverted. Therefore driver should not enforce specific trigger type - edge falling - but instead rely on Devicetree to configure it.

The Maxim 77686 datasheet describes the interrupt line as active low with a requirement of acknowledge from the CPU therefore the edge falling is not correct.

The interrupt line is shared between PMIC and RTC driver, so using level sensitive interrupt is here especially important to avoid races. With an edge configuration in case if first PMIC signals interrupt followed shortly after by the RTC, the interrupt might not be yet cleared/acked thus the second one would not be noticed.

Signed-off-by Krzysztof Kozlowski <[email protected]>

---

This patch should wait till DTS changes are merged, as it relies on proper Devicetree.

---
 Documentation/devicetree/bindings/clock/maxim,max77686.txt | 4 ++--
 Documentation/devicetree/bindings/mfd/max77686.txt         | 2 +-
 Documentation/devicetree/bindings/regulator/max77686.txt   | 2 +-
 drivers/mfd/max77686.c                                     | 3 +--
 4 files changed, 5 insertions(+), 6 deletions(-)

Interrupt line can be configured on different hardware in different way, even inverted. Therefore driver should not enforce specific trigger type - edge falling - but instead rely on Devicetree to configure it.

The Maxim 77693 datasheet describes the interrupt line as active low with a requirement of acknowledge from the CPU therefore the edge falling is not correct.

The interrupt line is shared between PMIC and RTC driver, so using level sensitive interrupt is here especially important to avoid races. With an edge configuration in case if first PMIC signals interrupt followed shortly after by the RTC, the interrupt might not be yet cleared/acked thus the second one would not be noticed.

Signed-off-by Krzysztof Kozlowski <[email protected]>

---

This patch should wait till DTS changes are merged, as it relies on proper Devicetree.

---
 Documentation/devicetree/bindings/mfd/max77693.txt |  2 +-
 drivers/mfd/max77693.c                             | 12 ++++--------
 2 files changed, 5 insertions(+), 9 deletions(-)

Interrupt line can be configured on different hardware in different way, even inverted. Therefore driver should not enforce specific trigger type - edge falling - but instead rely on Devicetree to configure it.

The Maxim 14577/77836 datasheets describe the interrupt line as active low with a requirement of acknowledge from the CPU therefore the edge falling is not correct.

The interrupt line is shared between PMIC and charger driver, so using level sensitive interrupt is here especially important to avoid races. With an edge configuration in case if first PMIC signals interrupt followed shortly after by the RTC, the interrupt might not be yet cleared/acked thus the second one would not be noticed.

Signed-off-by Krzysztof Kozlowski <[email protected]>

---

This patch should wait till DTS changes are merged, as it relies on proper Devicetree.

---
 Documentation/devicetree/bindings/mfd/max14577.txt | 4 ++--
 drivers/mfd/max14577.c                             | 6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

(I shortened the Cc)

Hi,

Quoted 12 lines · Krzysztof Kozlowski

[...]
index d956c67d5155..f737de0470de 100644

drivers/power/supply/max17040_battery.c +1 -1

@@ -367,7 +367,7 @@ static int max17040_enable_alert_irq(struct max17040_chip *chip)
 
 	flags = IRQF_TRIGGER_FALLING | IRQF_ONESHOT;
 	ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
-					max17040_thread_handler, flags,
+					max17040_thread_handler, IRQF_ONESHOT,
 					chip->battery->desc->name, chip);
 

drivers/power/supply/max17040_battery.c static int max17040_enable_alert_irq(struct max17040_chip *chip) · line 373

Quoted 1 line

drivers/power/supply/max17040_battery.c +0 -0

 	return ret;

'flags' is unused after this change.

Interrupt line can be configured on different hardware in different way, even inverted. Therefore driver should not enforce specific trigger type - edge falling - but instead rely on Devicetree to configure it.

The Maxim 14577/77836 datasheets describe the interrupt line as active low with a requirement of acknowledge from the CPU therefore the edge falling is not correct.

Signed-off-by Krzysztof Kozlowski <[email protected]>

---

This patch should wait till DTS changes are merged, as it relies on proper Devicetree.

---
 .../devicetree/bindings/power/supply/max17040_battery.txt       | 2 +-
 drivers/power/supply/max17040_battery.c                         | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Documentation/devicetree/bindings/power/supply/max17040_battery.txt +1 -1

index c802f664b508..194eb9fe574d 100644
@@ -39,7 +39,7 @@ Example:
 		reg = <0x36>;
 		maxim,alert-low-soc-level = <10>;
 		interrupt-parent = <&gpio7>;
-		interrupts = <2 IRQ_TYPE_EDGE_FALLING>;
+		interrupts = <2 IRQ_TYPE_LEVEL_LOW>;
 		wakeup-source;
 	};
 

drivers/power/supply/max17040_battery.c +0 -0

index d956c67d5155..f737de0470de 100644

drivers/power/supply/max17040_battery.c +1 -1

 	ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
-					max17040_thread_handler, flags,
+					max17040_thread_handler, IRQF_ONESHOT,
 					chip->battery->desc->name, chip);
 
 	return ret;

Signature · 1 line

2.25.1

Quoted 38 lines · Krzysztof Kozlowski

Interrupt line can be configured on different hardware in different way, even inverted.  Therefore driver should not enforce specific trigger type - edge falling - but instead rely on Devicetree to configure it.

The Maxim 14577/77836 datasheets describe the interrupt line as active low with a requirement of acknowledge from the CPU therefore the edge falling is not correct.

Signed-off-by Krzysztof Kozlowski <[email protected]>

---

This patch should wait till DTS changes are merged, as it relies on proper Devicetree.
---
.../devicetree/bindings/power/supply/max17040_battery.txt       | 2 +- drivers/power/supply/max17040_battery.c                         | 2 +-

  2 files changed, 2 insertions(+), 2 deletions(-)

Documentation/devicetree/bindings/power/supply/max17040_battery.txt +1 -1

index c802f664b508..194eb9fe574d 100644
@@ -39,7 +39,7 @@ Example:
          reg = <0x36>;
          maxim,alert-low-soc-level = <10>;
          interrupt-parent = <&gpio7>;
-        interrupts = <2 IRQ_TYPE_EDGE_FALLING>;
+        interrupts = <2 IRQ_TYPE_LEVEL_LOW>;
          wakeup-source;
      };
 

drivers/power/supply/max17040_battery.c +0 -0

index d956c67d5155..f737de0470de 100644
@@ -367,7 +367,7 @@ static int max17040_enable_alert_irq(struct max17040_chip *chip)
 

drivers/power/supply/max17040_battery.c static int max17040_enable_alert_irq(struct max17040_chip *chip) · line 368

Quoted 1 line

drivers/power/supply/max17040_battery.c +0 -0

      flags = IRQF_TRIGGER_FALLING | IRQF_ONESHOT;

This has to be removed. I will fix it in v2.

Best regards,
Krzysztof

Quoted 9 lines

drivers/power/supply/max17040_battery.c +1 -2

      ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
-                    max17040_thread_handler, flags,
+                    max17040_thread_handler, IRQF_ONESHOT,
                      chip->battery->desc->name, chip);
 
      return ret;
--

2.25.1

Quoted 13 lines · Krzysztof Kozlowski

Quoted 12 lines · Krzysztof Kozlowski

Interrupt line can be configured on different hardware in different way, even inverted.  Therefore driver should not enforce specific trigger type - edge falling - but instead rely on Devicetree to configure it.

The Maxim 14577/77836 datasheets describe the interrupt line as active low with a requirement of acknowledge from the CPU therefore the edge falling is not correct.

Signed-off-by Krzysztof Kozlowski <[email protected]>

---

Quoted 8 lines

Quoted 8 lines

This patch should wait till DTS changes are merged, as it relies on proper Devicetree.
---
.../devicetree/bindings/power/supply/max17040_battery.txt       | 2 +- drivers/power/supply/max17040_battery.c                         | 2 +-

  2 files changed, 2 insertions(+), 2 deletions(-)

diff --git

a/Documentation/devicetree/bindings/power/supply/max17040_battery.txt b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt

Quoted 12 lines

Quoted 12 lines

index c802f664b508..194eb9fe574d 100644

Documentation/devicetree/bindings/power/supply/max17040_battery.txt +1 -1

@@ -39,7 +39,7 @@ Example:
          reg = <0x36>;
          maxim,alert-low-soc-level = <10>;
          interrupt-parent = <&gpio7>;
-        interrupts = <2 IRQ_TYPE_EDGE_FALLING>;
+        interrupts = <2 IRQ_TYPE_LEVEL_LOW>;
          wakeup-source;
      };
 

Quoted 1 line

Review

Quoted 1 line

(quoted diff) +0 -0

b/drivers/power/supply/max17040_battery.c

Quoted 4 lines

drivers/power/supply/max17040_battery.c static int max17040_enable_alert_irq(struct

Quoted 4 lines

index d956c67d5155..f737de0470de 100644

drivers/power/supply/max17040_battery.c +0 -0

@@ -367,7 +367,7 @@ static int max17040_enable_alert_irq(struct

max17040_chip *chip)

Quoted 7 lines

drivers/power/supply/max17040_battery.c static int max17040_enable_alert_irq(struct max17040_chip *chip) · line 368

Quoted 2 lines

drivers/power/supply/max17040_battery.c +0 -0

      flags = IRQF_TRIGGER_FALLING | IRQF_ONESHOT;

This has to be removed. I will fix it in v2.

Best regards,
Krzysztof

I removed the IRQF_TRIGGER_FALLING, tweaked the DT as per the DT patch, and it worked on the samsung klte.

I don't understand how the DT irq flag ends up being used by the kernel. It is never explicitly read from DT or passed to interrupt API, only i2c->irq, which is a pure int.

The Maxim fuel gauge datasheets describe the interrupt line as active low with a requirement of acknowledge from the CPU. The falling edge interrupt will mostly work but it's not correct.

Fixes 8620cc2f99b7 ("ARM: dts: exynos: Add devicetree file for the Galaxy S2")

Signed-off-by Krzysztof Kozlowski <[email protected]>

---
 arch/arm/boot/dts/exynos4210-i9100.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)