Fixes for twl4030 charger
v2 cover Fixes for twl4030 charger
Changes V2:
* worked in comments by Nishanth Menon <[email protected]>
* 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(-)
Signature · 1 line
2.5.1
PATCH v2 1/3 drivers:power:twl4030-charger: fix problem with EPROBE_DEFER
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.
--- drivers/power/twl4030_charger.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
drivers/power/twl4030_charger.c +5 -1
@@ -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. */
Signature · 1 line
2.5.1
PATCH v2 2/3 drivers:power:twl4030-charger: don't return after allocating irq
In reply to H. Nikolaus SchallerIt 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.
--- drivers/power/twl4030_charger.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-)
drivers/power/twl4030_charger.c +17 -17
@@ -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 );
Signature · 1 line
2.5.1
PATCH v2 3/3 drivers:power:twl4030-charger: don't check if battery is present
In reply to H. Nikolaus SchallerWe 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.
--- drivers/power/twl4030_charger.c | 7 ------- 1 file changed, 7 deletions(-)
drivers/power/twl4030_charger.c +0 -7
@@ -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,
Signature · 1 line
2.5.1
ping.
Am 02.11.2015 um 12:27 schrieb H. Nikolaus Schaller <[email protected]>:
Quoted 22 lines
Changes V2:
* worked in comments by Nishanth Menon <[email protected]>
* 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 withEPROBE_DEFERdrivers: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(-)Signature · 1 line
2.5.1
ping.
Am 13.11.2015 um 11:28 schrieb H. Nikolaus Schaller <[email protected]>:
Quoted 31 lines
ping.
Am 02.11.2015 um 12:27 schrieb H. Nikolaus Schaller <[email protected]>:
Quoted 11 lines
Changes V2:
* worked in comments by Nishanth Menon <[email protected]>
* 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.
Quoted 11 lines
H. Nikolaus Schaller (3):
drivers:power:twl4030-charger: fix problem withEPROBE_DEFERdrivers: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(-)Signature · 1 line
2.5.1_______________________________________________
Gta04-owner mailing list
[email protected]
http://lists.goldelico.com/mailman/listinfo.cgi/gta04-owner
Hi Nikolaus,
Quoted 7 lines · H. Nikolaus Schaller
Changes V2:
* worked in comments by Nishanth Menon <[email protected]>
* 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.
Quoted 8 lines
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 withEPROBE_DEFERdrivers: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