diff mbox series

[01/18] ARM: dts: exynos: correct fuel gauge interrupt trigger level on GT-I9100

Message ID 20201210212534.216197-1-krzk@kernel.org
State Accepted
Commit 46799802136670e00498f19898f1635fbc85f583
Headers show
Series [01/18] ARM: dts: exynos: correct fuel gauge interrupt trigger level on GT-I9100 | expand

Commit Message

Krzysztof Kozlowski Dec. 10, 2020, 9:25 p.m. UTC
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 <krzk@kernel.org>
---
 arch/arm/boot/dts/exynos4210-i9100.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Iskren Chernev Dec. 11, 2020, 3:44 p.m. UTC | #1
On 12/11/20 9:47 AM, Krzysztof Kozlowski wrote:
 > On Thu, Dec 10, 2020 at 10:25:34PM +0100, Krzysztof Kozlowski wrote:

 >> 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 <krzk@kernel.org>

 >>

 >> ---

 >>

 >> 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
 >> index c802f664b508..194eb9fe574d 100644

 >> --- 

a/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
 >> +++ 

b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
 >> @@ -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;

 >>      };

 >>

 >> diff --git a/drivers/power/supply/max17040_battery.c 

b/drivers/power/supply/max17040_battery.c
 >> index d956c67d5155..f737de0470de 100644

 >> --- a/drivers/power/supply/max17040_battery.c

 >> +++ b/drivers/power/supply/max17040_battery.c

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

max17040_chip *chip)
 >>

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

Fixing the DT and the drivers will hopefully set a precedent, so future
drivers (when copied/tweaked from existing drivers) will do it right.

Acked-by: Iskren Chernev <iskren.chernev@gmail.com>


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

 >>
Krzysztof Kozlowski March 3, 2021, 6:19 p.m. UTC | #2
On Thu, Dec 10, 2020 at 10:25:17PM +0100, Krzysztof Kozlowski wrote:
> 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 <krzk@kernel.org>
> ---
>  arch/arm/boot/dts/exynos4210-i9100.dts | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Applied 1-10 (Exynos and S5P dts patches).

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/arch/arm/boot/dts/exynos4210-i9100.dts b/arch/arm/boot/dts/exynos4210-i9100.dts
index a0c3bab382ae..e56b64e237d3 100644
--- a/arch/arm/boot/dts/exynos4210-i9100.dts
+++ b/arch/arm/boot/dts/exynos4210-i9100.dts
@@ -136,7 +136,7 @@  battery@36 {
 			compatible = "maxim,max17042";
 
 			interrupt-parent = <&gpx2>;
-			interrupts = <3 IRQ_TYPE_EDGE_FALLING>;
+			interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
 
 			pinctrl-0 = <&max17042_fuel_irq>;
 			pinctrl-names = "default";