* [PATCH v2 1/3] drivers:power:twl4030-charger: fix problem with EPROBE_DEFER
2015-11-02 11:27 [PATCH v2 0/3] Fixes for twl4030 charger H. Nikolaus Schaller
@ 2015-11-02 11:27 ` H. Nikolaus Schaller
2015-11-02 11:27 ` [PATCH v2 2/3] drivers:power:twl4030-charger: don't return after allocating irq H. Nikolaus Schaller
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: H. Nikolaus Schaller @ 2015-11-02 11:27 UTC (permalink / raw)
To: Gražvydas Ignotas, Sebastian Reichel,
Dmitry Eremin-Solenikov, David Woodhouse, Andreas Kemnade
Cc: linux-pm, gta04-owner, linux-kernel, H. Nikolaus Schaller
If the phy-twol4030-usb driver is compiled as a module,
devm_usb_get_phy_by_node() may return -EPROBE_DEFER in which
case we should also defer probing of the twl4030 charger instead of
turning USB charging off (forever or until next reboot) further down in
code.
Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
drivers/power/twl4030_charger.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index 74f2d3f..00697e9 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -1057,9 +1057,13 @@ static int twl4030_bci_probe(struct platform_device *pdev)
phynode = of_find_compatible_node(bci->dev->of_node->parent,
NULL, "ti,twl4030-usb");
- if (phynode)
+ if (phynode) {
bci->transceiver = devm_usb_get_phy_by_node(
bci->dev, phynode, &bci->usb_nb);
+ if (IS_ERR(bci->transceiver) &&
+ PTR_ERR(bci->transceiver) == -EPROBE_DEFER)
+ return -EPROBE_DEFER; /* PHY not ready */
+ }
}
/* Enable interrupts now. */
--
2.5.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v2 2/3] drivers:power:twl4030-charger: don't return after allocating irq
2015-11-02 11:27 [PATCH v2 0/3] Fixes for twl4030 charger H. Nikolaus Schaller
2015-11-02 11:27 ` [PATCH v2 1/3] drivers:power:twl4030-charger: fix problem with EPROBE_DEFER H. Nikolaus Schaller
@ 2015-11-02 11:27 ` H. Nikolaus Schaller
2015-11-02 11:27 ` [PATCH v2 3/3] drivers:power:twl4030-charger: don't check if battery is present H. Nikolaus Schaller
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: H. Nikolaus Schaller @ 2015-11-02 11:27 UTC (permalink / raw)
To: Gražvydas Ignotas, Sebastian Reichel,
Dmitry Eremin-Solenikov, David Woodhouse, Andreas Kemnade
Cc: linux-pm, gta04-owner, linux-kernel, H. Nikolaus Schaller
It appears that simply returning with error status (especially
-EPROBE_DEFER) is very dangerous *after* allocating devm managed
interrupts.
See discussion of potential issues: https://lkml.org/lkml/2013/2/22/65
The result is that the boot process hangs if both, phy-twl4030-usb and this
twl4030_charger driver are compiled as modules. This has been observed
on gta04 and openpandora boards.
So we move the search and check that the twl4030-phy transceiver link is
available before we request to set up the interrupts.
Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
drivers/power/twl4030_charger.c | 34 +++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index 00697e9..05d693d 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -1031,23 +1031,6 @@ static int twl4030_bci_probe(struct platform_device *pdev)
return ret;
}
- ret = devm_request_threaded_irq(&pdev->dev, bci->irq_chg, NULL,
- twl4030_charger_interrupt, IRQF_ONESHOT, pdev->name,
- bci);
- if (ret < 0) {
- dev_err(&pdev->dev, "could not request irq %d, status %d\n",
- bci->irq_chg, ret);
- return ret;
- }
-
- ret = devm_request_threaded_irq(&pdev->dev, bci->irq_bci, NULL,
- twl4030_bci_interrupt, IRQF_ONESHOT, pdev->name, bci);
- if (ret < 0) {
- dev_err(&pdev->dev, "could not request irq %d, status %d\n",
- bci->irq_bci, ret);
- return ret;
- }
-
INIT_WORK(&bci->work, twl4030_bci_usb_work);
INIT_DELAYED_WORK(&bci->current_worker, twl4030_current_worker);
@@ -1066,6 +1049,23 @@ static int twl4030_bci_probe(struct platform_device *pdev)
}
}
+ ret = devm_request_threaded_irq(&pdev->dev, bci->irq_chg, NULL,
+ twl4030_charger_interrupt, IRQF_ONESHOT, pdev->name,
+ bci);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "could not request irq %d, status %d\n",
+ bci->irq_chg, ret);
+ return ret;
+ }
+
+ ret = devm_request_threaded_irq(&pdev->dev, bci->irq_bci, NULL,
+ twl4030_bci_interrupt, IRQF_ONESHOT, pdev->name, bci);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "could not request irq %d, status %d\n",
+ bci->irq_bci, ret);
+ return ret;
+ }
+
/* Enable interrupts now. */
reg = ~(u32)(TWL4030_ICHGLOW | TWL4030_ICHGEOC | TWL4030_TBATOR2 |
TWL4030_TBATOR1 | TWL4030_BATSTS);
--
2.5.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v2 3/3] drivers:power:twl4030-charger: don't check if battery is present
2015-11-02 11:27 [PATCH v2 0/3] Fixes for twl4030 charger H. Nikolaus Schaller
2015-11-02 11:27 ` [PATCH v2 1/3] drivers:power:twl4030-charger: fix problem with EPROBE_DEFER H. Nikolaus Schaller
2015-11-02 11:27 ` [PATCH v2 2/3] drivers:power:twl4030-charger: don't return after allocating irq H. Nikolaus Schaller
@ 2015-11-02 11:27 ` H. Nikolaus Schaller
2015-11-13 10:28 ` [PATCH v2 0/3] Fixes for twl4030 charger H. Nikolaus Schaller
2015-12-04 23:54 ` Sebastian Reichel
4 siblings, 0 replies; 7+ messages in thread
From: H. Nikolaus Schaller @ 2015-11-02 11:27 UTC (permalink / raw)
To: Gražvydas Ignotas, Sebastian Reichel,
Dmitry Eremin-Solenikov, David Woodhouse, Andreas Kemnade
Cc: linux-pm, gta04-owner, linux-kernel, H. Nikolaus Schaller
We can't assume that the battery is present after probing (it can
sometimes be removed/hot swapped by the user while device
(e.g. GTA04 or OpenPanodra) is operated through external AC
or USB power).
So it makes no sense to check for this situation (only) during probe.
Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
drivers/power/twl4030_charger.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index 05d693d..6fbdf24 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -1006,13 +1006,6 @@ static int twl4030_bci_probe(struct platform_device *pdev)
bci->irq_chg = platform_get_irq(pdev, 0);
bci->irq_bci = platform_get_irq(pdev, 1);
- /* Only proceed further *IF* battery is physically present */
- ret = twl4030_is_battery_present(bci);
- if (ret) {
- dev_crit(&pdev->dev, "Battery was not detected:%d\n", ret);
- return ret;
- }
-
platform_set_drvdata(pdev, bci);
bci->ac = devm_power_supply_register(&pdev->dev, &twl4030_bci_ac_desc,
--
2.5.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2 0/3] Fixes for twl4030 charger
2015-11-02 11:27 [PATCH v2 0/3] Fixes for twl4030 charger H. Nikolaus Schaller
` (2 preceding siblings ...)
2015-11-02 11:27 ` [PATCH v2 3/3] drivers:power:twl4030-charger: don't check if battery is present H. Nikolaus Schaller
@ 2015-11-13 10:28 ` H. Nikolaus Schaller
2015-11-23 9:27 ` [Gta04-owner] " H. Nikolaus Schaller
2015-12-04 23:54 ` Sebastian Reichel
4 siblings, 1 reply; 7+ messages in thread
From: H. Nikolaus Schaller @ 2015-11-13 10:28 UTC (permalink / raw)
To: Gražvydas Ignotas, Sebastian Reichel,
Dmitry Eremin-Solenikov, David Woodhouse, Andreas Kemnade
Cc: linux-pm, gta04-owner, linux-kernel
ping.
Am 02.11.2015 um 12:27 schrieb H. Nikolaus Schaller <hns@goldelico.com>:
> Changes V2:
> * worked in comments by Nishanth Menon <nm@ti.com>
> * added another patch which solves a probing/boot stall problem (irq allocation vs. -EPROBE_DEFER)
>
> V1:
> 4.3-rc1 introduced a new charger driver for the twl4030.
>
> While making it operable and testing on GTA04 and OpenPandora
> we have found some issues.
>
>
> H. Nikolaus Schaller (3):
> drivers:power:twl4030-charger: fix problem with EPROBE_DEFER
> drivers:power:twl4030-charger: don't return after allocating irq
> drivers:power:twl4030-charger: don't check if battery is present
>
> drivers/power/twl4030_charger.c | 39 ++++++++++++++++++---------------------
> 1 file changed, 18 insertions(+), 21 deletions(-)
>
> --
> 2.5.1
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Gta04-owner] [PATCH v2 0/3] Fixes for twl4030 charger
2015-11-13 10:28 ` [PATCH v2 0/3] Fixes for twl4030 charger H. Nikolaus Schaller
@ 2015-11-23 9:27 ` H. Nikolaus Schaller
0 siblings, 0 replies; 7+ messages in thread
From: H. Nikolaus Schaller @ 2015-11-23 9:27 UTC (permalink / raw)
To: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse
Cc: Andreas Kemnade, LKML, Linux PM mailing list,
Gražvydas Ignotas
ping.
Am 13.11.2015 um 11:28 schrieb H. Nikolaus Schaller <hns@goldelico.com>:
> ping.
>
> Am 02.11.2015 um 12:27 schrieb H. Nikolaus Schaller <hns@goldelico.com>:
>
>> Changes V2:
>> * worked in comments by Nishanth Menon <nm@ti.com>
>> * added another patch which solves a probing/boot stall problem (irq allocation vs. -EPROBE_DEFER)
>>
>> V1:
>> 4.3-rc1 introduced a new charger driver for the twl4030.
>>
>> While making it operable and testing on GTA04 and OpenPandora
>> we have found some issues.
>>
>>
>> H. Nikolaus Schaller (3):
>> drivers:power:twl4030-charger: fix problem with EPROBE_DEFER
>> drivers:power:twl4030-charger: don't return after allocating irq
>> drivers:power:twl4030-charger: don't check if battery is present
>>
>> drivers/power/twl4030_charger.c | 39 ++++++++++++++++++---------------------
>> 1 file changed, 18 insertions(+), 21 deletions(-)
>>
>> --
>> 2.5.1
>>
>
> _______________________________________________
> Gta04-owner mailing list
> Gta04-owner@goldelico.com
> http://lists.goldelico.com/mailman/listinfo.cgi/gta04-owner
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 0/3] Fixes for twl4030 charger
2015-11-02 11:27 [PATCH v2 0/3] Fixes for twl4030 charger H. Nikolaus Schaller
` (3 preceding siblings ...)
2015-11-13 10:28 ` [PATCH v2 0/3] Fixes for twl4030 charger H. Nikolaus Schaller
@ 2015-12-04 23:54 ` Sebastian Reichel
4 siblings, 0 replies; 7+ messages in thread
From: Sebastian Reichel @ 2015-12-04 23:54 UTC (permalink / raw)
To: H. Nikolaus Schaller
Cc: Gražvydas Ignotas, Dmitry Eremin-Solenikov, David Woodhouse,
Andreas Kemnade, linux-pm, gta04-owner, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1103 bytes --]
Hi Nikolaus,
On Mon, Nov 02, 2015 at 12:27:33PM +0100, H. Nikolaus Schaller wrote:
> Changes V2:
> * worked in comments by Nishanth Menon <nm@ti.com>
> * added another patch which solves a probing/boot stall problem (irq allocation vs. -EPROBE_DEFER)
>
> V1:
> 4.3-rc1 introduced a new charger driver for the twl4030.
>
> While making it operable and testing on GTA04 and OpenPandora
> we have found some issues.
>
>
> H. Nikolaus Schaller (3):
> drivers:power:twl4030-charger: fix problem with EPROBE_DEFER
> drivers:power:twl4030-charger: don't return after allocating irq
> drivers:power:twl4030-charger: don't check if battery is present
PATCH 1 and 2 should be rebased to my current next branch.
They definetly do not take 2202e1fc5a into account. Also
take into account, that iio_channel_get() is not managed,
so you will have to free the channel, if your return an
error after it.
PATCH 3 introduces a new warning:
drivers/power/twl4030_charger.c:211:12: warning: ‘twl4030_is_battery_present’ defined but not used [-Wunused-function]
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread