Re: [PATCH] power: twl4030_charger: clear IRQs after handling them

2014-05-09 Thread Nishanth Menon
On 05/08/2014 08:03 PM, Tony Lindgren wrote:
> * Nishanth Menon  [140506 17:25]:
>> Subject: [PATCH] power: twl4030_charger: detect battery presence prior to
>>  enabling charger
>>
>> TWL4030's Battery Charger seems to be designed for non-hotpluggable
>> batteries.
>>
>> If battery is not present in the system, BATSTS is always set with the
>> expectation that software will take actions to move to a required safe
>> state (could be power down or disable various charger paths).
>>
>> It does not seem possible even by manipulating the edge detection
>> of the event (using BCIEDR2 register) to have a consistent hotplug
>> handling. This seems to be the result of BATSTS interrupt generated
>> when the thermistor of the battery pack is disconnected from the
>> dedicated ADIN1 pin. Clearing the status just results in the status
>> being regenerated by the monitoring ADC(MADC) and disabling the
>> edges of event just makes hotplug no longer function. The only
>> other option is to disable the detection of the MADC by disabling
>> BCIMFEN4::BATSTSMCHGEN (battery presence detector) - but then, we can
>> never again detect battery reconnection.
>>
>> So, detect battery presence based on precharge(which is hardware
>> automatic state) or default main charger configuration at the time of
>> probe and enable charger logic only if battery was present.
>>
>> Reported-by: Russell King 
>> Signed-off-by: Nishanth Menon 
> 
> Gets rid of the errors for me if CONFIG_CHARGER_TWL4030=y.
> Looks like we don't have that enabled by default in
> omap2plus_defconfig which explain why it's taken so long to
> notice this one:
> 
> Tested-by: Tony Lindgren 
> 
Thanks. I will post this out to the list as formal series.

-- 
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] power: twl4030_charger: clear IRQs after handling them

2014-05-09 Thread Nishanth Menon
On 05/08/2014 08:03 PM, Tony Lindgren wrote:
 * Nishanth Menon n...@ti.com [140506 17:25]:
 Subject: [PATCH] power: twl4030_charger: detect battery presence prior to
  enabling charger

 TWL4030's Battery Charger seems to be designed for non-hotpluggable
 batteries.

 If battery is not present in the system, BATSTS is always set with the
 expectation that software will take actions to move to a required safe
 state (could be power down or disable various charger paths).

 It does not seem possible even by manipulating the edge detection
 of the event (using BCIEDR2 register) to have a consistent hotplug
 handling. This seems to be the result of BATSTS interrupt generated
 when the thermistor of the battery pack is disconnected from the
 dedicated ADIN1 pin. Clearing the status just results in the status
 being regenerated by the monitoring ADC(MADC) and disabling the
 edges of event just makes hotplug no longer function. The only
 other option is to disable the detection of the MADC by disabling
 BCIMFEN4::BATSTSMCHGEN (battery presence detector) - but then, we can
 never again detect battery reconnection.

 So, detect battery presence based on precharge(which is hardware
 automatic state) or default main charger configuration at the time of
 probe and enable charger logic only if battery was present.

 Reported-by: Russell King li...@arm.linux.org.uk
 Signed-off-by: Nishanth Menon n...@ti.com
 
 Gets rid of the errors for me if CONFIG_CHARGER_TWL4030=y.
 Looks like we don't have that enabled by default in
 omap2plus_defconfig which explain why it's taken so long to
 notice this one:
 
 Tested-by: Tony Lindgren t...@atomide.com
 
Thanks. I will post this out to the list as formal series.

-- 
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] power: twl4030_charger: clear IRQs after handling them

2014-05-08 Thread Tony Lindgren
* Nishanth Menon  [140506 17:25]:
> Subject: [PATCH] power: twl4030_charger: detect battery presence prior to
>  enabling charger
> 
> TWL4030's Battery Charger seems to be designed for non-hotpluggable
> batteries.
> 
> If battery is not present in the system, BATSTS is always set with the
> expectation that software will take actions to move to a required safe
> state (could be power down or disable various charger paths).
> 
> It does not seem possible even by manipulating the edge detection
> of the event (using BCIEDR2 register) to have a consistent hotplug
> handling. This seems to be the result of BATSTS interrupt generated
> when the thermistor of the battery pack is disconnected from the
> dedicated ADIN1 pin. Clearing the status just results in the status
> being regenerated by the monitoring ADC(MADC) and disabling the
> edges of event just makes hotplug no longer function. The only
> other option is to disable the detection of the MADC by disabling
> BCIMFEN4::BATSTSMCHGEN (battery presence detector) - but then, we can
> never again detect battery reconnection.
> 
> So, detect battery presence based on precharge(which is hardware
> automatic state) or default main charger configuration at the time of
> probe and enable charger logic only if battery was present.
> 
> Reported-by: Russell King 
> Signed-off-by: Nishanth Menon 

Gets rid of the errors for me if CONFIG_CHARGER_TWL4030=y.
Looks like we don't have that enabled by default in
omap2plus_defconfig which explain why it's taken so long to
notice this one:

Tested-by: Tony Lindgren 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] power: twl4030_charger: clear IRQs after handling them

2014-05-08 Thread Tony Lindgren
* Nishanth Menon n...@ti.com [140506 17:25]:
 Subject: [PATCH] power: twl4030_charger: detect battery presence prior to
  enabling charger
 
 TWL4030's Battery Charger seems to be designed for non-hotpluggable
 batteries.
 
 If battery is not present in the system, BATSTS is always set with the
 expectation that software will take actions to move to a required safe
 state (could be power down or disable various charger paths).
 
 It does not seem possible even by manipulating the edge detection
 of the event (using BCIEDR2 register) to have a consistent hotplug
 handling. This seems to be the result of BATSTS interrupt generated
 when the thermistor of the battery pack is disconnected from the
 dedicated ADIN1 pin. Clearing the status just results in the status
 being regenerated by the monitoring ADC(MADC) and disabling the
 edges of event just makes hotplug no longer function. The only
 other option is to disable the detection of the MADC by disabling
 BCIMFEN4::BATSTSMCHGEN (battery presence detector) - but then, we can
 never again detect battery reconnection.
 
 So, detect battery presence based on precharge(which is hardware
 automatic state) or default main charger configuration at the time of
 probe and enable charger logic only if battery was present.
 
 Reported-by: Russell King li...@arm.linux.org.uk
 Signed-off-by: Nishanth Menon n...@ti.com

Gets rid of the errors for me if CONFIG_CHARGER_TWL4030=y.
Looks like we don't have that enabled by default in
omap2plus_defconfig which explain why it's taken so long to
notice this one:

Tested-by: Tony Lindgren t...@atomide.com
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] power: twl4030_charger: clear IRQs after handling them

2014-05-06 Thread Nishanth Menon
On 16:00-20140425, Felipe Balbi wrote:
> 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  [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 
> > >> ---
> > >>
> > >> 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.

I dug a bit more into this earlier today. It makes a little sense
considering that TWL4030 was created in a world where mobile devices did
not consider battery was hot pluggable entity.. 

Long story short:
a) the BCI interrupt status register is read-on-clear (see
drivers/mfd/twl4030-irq.c set_cor is set to true for bci) - so the patch
https://patchwork.kernel.org/patch/4002371/ is practically a NOP.
b) BATSTS is a result of MADC monitoring the thermistor and is
continually generated. I tried a bunch of tricks that I could come up
with to try to do a hotplug support, but either we have infinite
interrupts (due to regeneration of event) or I loose detection ability.

So, to keep things simple I created the following patch that seems to
handle:
a) no battery connected, powered off charger - bci disabled
b) battery connected, powered off charger - bci active
c) battery connected, no charger - bci active

note: if battery was connected, and booted and we hotplug battery out,
we still have the spam of messages.. that is a seperate patch
(basically do an orderly shutdown - I cant think of anything else we
could possible do there)

Let me know your views.. if this approach is fine, I can post out formal
patches for the same.
-8<
>From 9474c7f37990b97c57b72eed917b9153595133ff Mon Sep 17 00:00:00 2001
From: Nishanth Menon 
Date: Tue, 6 May 2014 19:02:49 -0500
Subject: [PATCH] power: twl4030_charger: detect battery presence prior to
 enabling charger

TWL4030's Battery Charger seems to be designed for non-hotpluggable
batteries.

If battery is not present in the system, BATSTS is always set with the
expectation that software will take actions to move to a required safe
state (could be power down or disable various charger paths).

It does not seem possible even by manipulating the edge detection
of the event (using BCIEDR2 register) to have a consistent hotplug
handling. This seems to be the result of BATSTS interrupt generated
when the thermistor of the battery pack is disconnected from the
dedicated ADIN1 pin. Clearing the status just results in the status
being regenerated by the monitoring ADC(MADC) and disabling the
edges of event just makes hotplug no longer function. The only
other option is to disable the detection of the MADC by disabling
BCIMFEN4::BATSTSMCHGEN (battery presence detector) - but then, we can
never again detect battery reconnection.

So, detect battery presence based on precharge(which is hardware
automatic state) or default main charger configuration at the time of
probe and enable charger logic only if battery was present.

Reported-by: Russell King 
Signed-off-by: Nishanth Menon 
---
 drivers/power/twl4030_charger.c |   43 ++-
 1 file changed, 42 insertions(+), 1 deletion(-)

diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index f141088..14ec220 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -28,10 +28,13 @@
 #define TWL4030_BCIICHG0x08
 #define TWL4030_BCIVAC 0x0a
 #define TWL4030_BCIVBUS0x0c
+#define TWL4030_BCIMFSTS3  0x0F
 #define TWL4030_BCIMFSTS4  0x10
 #define TWL4030_BCICTL10x23
 #define TWL4030_BB_CFG 0x12
 
+#define TWL4030_BCIMFSTS1  0x01
+
 #define TWL4030_BCIAUTOWEN BIT(5)
 #define TWL4030_CONFIG_DONEBIT(4)
 #define TWL4030_BCIAUTOUSB BIT(1)
@@ -52,6 +55,9 @@
 #define TWL4030_BBISEL_500uA   0x02
 #define TWL4030_BBISEL_1000uA  0x03
 
+#define TWL4030_BATSTSPCHG BIT(2)
+#define TWL4030_BATSTSMCHG BIT(6)
+
 /* BCI interrupts */
 #define TWL4030_WOVF   BIT(0) /* Watchdog overflow */
 #define TWL4030_TMOVF  BIT(1) /* Timer overflow */
@@ -145,6 +151,34 @@ static int twl4030bci_read_adc_val(u8 reg)
 }
 
 /*
+ * Check if Battery Pack was present
+ */
+static 

Re: [PATCH] power: twl4030_charger: clear IRQs after handling them

2014-05-06 Thread Nishanth Menon
On 16:00-20140425, Felipe Balbi wrote:
 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 ba...@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 ba...@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.

I dug a bit more into this earlier today. It makes a little sense
considering that TWL4030 was created in a world where mobile devices did
not consider battery was hot pluggable entity.. 

Long story short:
a) the BCI interrupt status register is read-on-clear (see
drivers/mfd/twl4030-irq.c set_cor is set to true for bci) - so the patch
https://patchwork.kernel.org/patch/4002371/ is practically a NOP.
b) BATSTS is a result of MADC monitoring the thermistor and is
continually generated. I tried a bunch of tricks that I could come up
with to try to do a hotplug support, but either we have infinite
interrupts (due to regeneration of event) or I loose detection ability.

So, to keep things simple I created the following patch that seems to
handle:
a) no battery connected, powered off charger - bci disabled
b) battery connected, powered off charger - bci active
c) battery connected, no charger - bci active

note: if battery was connected, and booted and we hotplug battery out,
we still have the spam of messages.. that is a seperate patch
(basically do an orderly shutdown - I cant think of anything else we
could possible do there)

Let me know your views.. if this approach is fine, I can post out formal
patches for the same.
-8
From 9474c7f37990b97c57b72eed917b9153595133ff Mon Sep 17 00:00:00 2001
From: Nishanth Menon n...@ti.com
Date: Tue, 6 May 2014 19:02:49 -0500
Subject: [PATCH] power: twl4030_charger: detect battery presence prior to
 enabling charger

TWL4030's Battery Charger seems to be designed for non-hotpluggable
batteries.

If battery is not present in the system, BATSTS is always set with the
expectation that software will take actions to move to a required safe
state (could be power down or disable various charger paths).

It does not seem possible even by manipulating the edge detection
of the event (using BCIEDR2 register) to have a consistent hotplug
handling. This seems to be the result of BATSTS interrupt generated
when the thermistor of the battery pack is disconnected from the
dedicated ADIN1 pin. Clearing the status just results in the status
being regenerated by the monitoring ADC(MADC) and disabling the
edges of event just makes hotplug no longer function. The only
other option is to disable the detection of the MADC by disabling
BCIMFEN4::BATSTSMCHGEN (battery presence detector) - but then, we can
never again detect battery reconnection.

So, detect battery presence based on precharge(which is hardware
automatic state) or default main charger configuration at the time of
probe and enable charger logic only if battery was present.

Reported-by: Russell King li...@arm.linux.org.uk
Signed-off-by: Nishanth Menon n...@ti.com
---
 drivers/power/twl4030_charger.c |   43 ++-
 1 file changed, 42 insertions(+), 1 deletion(-)

diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index f141088..14ec220 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -28,10 +28,13 @@
 #define TWL4030_BCIICHG0x08
 #define TWL4030_BCIVAC 0x0a
 #define TWL4030_BCIVBUS0x0c
+#define TWL4030_BCIMFSTS3  0x0F
 #define TWL4030_BCIMFSTS4  0x10
 #define TWL4030_BCICTL10x23
 #define TWL4030_BB_CFG 0x12
 
+#define TWL4030_BCIMFSTS1  0x01
+
 #define TWL4030_BCIAUTOWEN BIT(5)
 #define TWL4030_CONFIG_DONEBIT(4)
 #define TWL4030_BCIAUTOUSB BIT(1)
@@ -52,6 +55,9 @@
 #define TWL4030_BBISEL_500uA   0x02
 #define TWL4030_BBISEL_1000uA  0x03
 
+#define TWL4030_BATSTSPCHG BIT(2)
+#define TWL4030_BATSTSMCHG BIT(6)
+
 /* BCI interrupts */
 #define TWL4030_WOVF   BIT(0) /* Watchdog overflow */
 #define TWL4030_TMOVF  BIT(1) /* Timer overflow */
@@ -145,6 +151,34 @@ static int twl4030bci_read_adc_val(u8 reg)
 }
 
 /*
+ * Check if Battery Pack was present
+ */
+static int 

Re: [PATCH] power: twl4030_charger: clear IRQs after handling them

2014-04-25 Thread Felipe Balbi
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  [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 
> >> ---
> >>
> >> 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 ...

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH] power: twl4030_charger: clear IRQs after handling them

2014-04-25 Thread Nishanth Menon
On 04/16/2014 11:35 AM, Tony Lindgren wrote:
> * Felipe Balbi  [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 
>> ---
>>
>> 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.
-- 
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] power: twl4030_charger: clear IRQs after handling them

2014-04-25 Thread Nishanth Menon
On 04/16/2014 11:35 AM, Tony Lindgren wrote:
 * Felipe Balbi ba...@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 ba...@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.
-- 
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] power: twl4030_charger: clear IRQs after handling them

2014-04-25 Thread Felipe Balbi
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 ba...@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 ba...@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 ...

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH] power: twl4030_charger: clear IRQs after handling them

2014-04-16 Thread Tony Lindgren
* Felipe Balbi  [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 
> ---
> 
> 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, ,
> 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, ,
> 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 majord...@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-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] power: twl4030_charger: clear IRQs after handling them

2014-04-16 Thread Tony Lindgren
* Felipe Balbi ba...@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 ba...@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 majord...@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-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/