diff mbox

power: twl4030_charger: clear IRQs after handling them

Message ID 1397661299-31342-1-git-send-email-balbi@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Felipe Balbi April 16, 2014, 3:14 p.m. UTC 
TRM says we *must* write 1 to each bit we're handling
in order to clear the IRQ status and bring IRQ line
low. This patch implements that.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---

Russell, I don't have HW to test, but this should
solve the problem you saw when not using battery
with Zoom board. Let me know if it doesn't.

cheers

 drivers/power/twl4030_charger.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

Comments

Tony Lindgren April 16, 2014, 4:35 p.m. UTC | #1 
* Felipe Balbi <balbi@ti.com> [140416 08:18]:
> TRM says we *must* write 1 to each bit we're handling
> in order to clear the IRQ status and bring IRQ line
> low. This patch implements that.
> 
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> ---
> 
> Russell, I don't have HW to test, but this should
> solve the problem you saw when not using battery
> with Zoom board. Let me know if it doesn't.

BTW, looks like we're enabling BCI automatically in twl4030.dtsi
while the legacy booting does not have TWL_COMMON_PDATA_BCI
enabled for LDP. Anyways, for LDP BCI should be enabled for
sure, that's the only way to power at least the earlier LDP
revisions.

>  drivers/power/twl4030_charger.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
> index f141088..b090842 100644
> --- a/drivers/power/twl4030_charger.c
> +++ b/drivers/power/twl4030_charger.c
> @@ -301,12 +301,24 @@ static irqreturn_t twl4030_bci_interrupt(int irq, void *arg)
>  	ret = twl_i2c_read_u8(TWL4030_MODULE_INTERRUPTS, &irqs1,
>  			      TWL4030_INTERRUPTS_BCIISR1A);
>  	if (ret < 0)
> -		return IRQ_HANDLED;
> +		return IRQ_NONE;
> +
> +	/* clear IRQs early */
> +	ret = twl_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, irqs1,
> +			TWL4030_INTERRUPTS_BCIISR1A);
> +	if (ret < 0)
> +		return IRQ_NONE;
>  
>  	ret = twl_i2c_read_u8(TWL4030_MODULE_INTERRUPTS, &irqs2,
>  			      TWL4030_INTERRUPTS_BCIISR2A);
>  	if (ret < 0)
> -		return IRQ_HANDLED;
> +		return IRQ_NONE;
> +
> +	/* clear IRQs early */
> +	ret = twl_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, irqs2,
> +			      TWL4030_INTERRUPTS_BCIISR2A);
> +	if (ret < 0)
> +		return IRQ_NONE;
>  
>  	dev_dbg(bci->dev, "BCI irq %02x %02x\n", irqs2, irqs1);
>  
> -- 
> 1.9.1.286.g5172cb3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Nishanth Menon April 25, 2014, 8:58 p.m. UTC | #2 
On 04/16/2014 11:35 AM, Tony Lindgren wrote:
> * Felipe Balbi <balbi@ti.com> [140416 08:18]:
>> TRM says we *must* write 1 to each bit we're handling
>> in order to clear the IRQ status and bring IRQ line
>> low. This patch implements that.
>>
>> Signed-off-by: Felipe Balbi <balbi@ti.com>
>> ---
>>
>> Russell, I don't have HW to test, but this should
>> solve the problem you saw when not using battery
>> with Zoom board. Let me know if it doesn't.
> 
> BTW, looks like we're enabling BCI automatically in twl4030.dtsi
> while the legacy booting does not have TWL_COMMON_PDATA_BCI
> enabled for LDP. Anyways, for LDP BCI should be enabled for
> sure, that's the only way to power at least the earlier LDP
> revisions.
> 
I picked up https://patchwork.kernel.org/patch/4002371/ for testing.

Unfortunately, it does not seem to work in my tests:

ldp-board-no-battery uses upstream u-boot in a chainloaded
configuration + does not have battery plugged in, instead, just J58
(charger) is connected - this is plugged on a remote farm setup for
developers to work with.

ldp-board-with-battery uses legacy bootloader with a battery simulator.

omap2plus_defconfig + CONFIG_CHARGER_TWL4030 enabled.

Result:
 v3-15-rc2-ldp-board-no-battery:  Boot FAIL:
http://slexy.org/raw/s20riTwu04

 v3-15-rc2-ldp-board-with-battery:  Boot PASS:
http://slexy.org/raw/s2elKdJpI7

 v3-15-rc2-with-felipe-fix-ldp-board-no-battery:  Boot FAIL:
http://slexy.org/raw/s20lKp2o0s


This patch should probably also acknowledge that Russell reported this
issue.
Felipe Balbi April 25, 2014, 9 p.m. UTC | #3 
On Fri, Apr 25, 2014 at 03:58:10PM -0500, Nishanth Menon wrote:
> On 04/16/2014 11:35 AM, Tony Lindgren wrote:
> > * Felipe Balbi <balbi@ti.com> [140416 08:18]:
> >> TRM says we *must* write 1 to each bit we're handling
> >> in order to clear the IRQ status and bring IRQ line
> >> low. This patch implements that.
> >>
> >> Signed-off-by: Felipe Balbi <balbi@ti.com>
> >> ---
> >>
> >> Russell, I don't have HW to test, but this should
> >> solve the problem you saw when not using battery
> >> with Zoom board. Let me know if it doesn't.
> > 
> > BTW, looks like we're enabling BCI automatically in twl4030.dtsi
> > while the legacy booting does not have TWL_COMMON_PDATA_BCI
> > enabled for LDP. Anyways, for LDP BCI should be enabled for
> > sure, that's the only way to power at least the earlier LDP
> > revisions.
> > 
> I picked up https://patchwork.kernel.org/patch/4002371/ for testing.
> 
> Unfortunately, it does not seem to work in my tests:

alright, I'll have a look after I get this other issue out of the way.
Probably not until next week.

> This patch should probably also acknowledge that Russell reported this
> issue.

sure I can add that, although it doesn't solve anything as it is ...
diff mbox

Patch

diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index f141088..b090842 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -301,12 +301,24 @@  static irqreturn_t twl4030_bci_interrupt(int irq, void *arg)
 	ret = twl_i2c_read_u8(TWL4030_MODULE_INTERRUPTS, &irqs1,
 			      TWL4030_INTERRUPTS_BCIISR1A);
 	if (ret < 0)
-		return IRQ_HANDLED;
+		return IRQ_NONE;
+
+	/* clear IRQs early */
+	ret = twl_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, irqs1,
+			TWL4030_INTERRUPTS_BCIISR1A);
+	if (ret < 0)
+		return IRQ_NONE;
 
 	ret = twl_i2c_read_u8(TWL4030_MODULE_INTERRUPTS, &irqs2,
 			      TWL4030_INTERRUPTS_BCIISR2A);
 	if (ret < 0)
-		return IRQ_HANDLED;
+		return IRQ_NONE;
+
+	/* clear IRQs early */
+	ret = twl_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, irqs2,
+			      TWL4030_INTERRUPTS_BCIISR2A);
+	if (ret < 0)
+		return IRQ_NONE;
 
 	dev_dbg(bci->dev, "BCI irq %02x %02x\n", irqs2, irqs1);