* [PATCH 0/2] Fixes for twl4030 charger @ 2015-10-29 17:01 H. Nikolaus Schaller 2015-10-29 17:01 ` [PATCH 1/2] drivers:power:twl4030-charger: fix problem with EPROBE_DEFER H. Nikolaus Schaller 2015-10-29 17:01 ` [PATCH 2/2] drivers:power:twl4030-charger: don't check if battery is present H. Nikolaus Schaller 0 siblings, 2 replies; 7+ messages in thread From: H. Nikolaus Schaller @ 2015-10-29 17:01 UTC (permalink / raw) To: Gražvydas Ignotas, Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse Cc: linux-pm, gta04-owner, H. Nikolaus Schaller 4.3-rc1 introduced new charger driver for the twl4030. While making it operable and testing on GTA04 and OpenPandora we have found some issues. H. Nikolaus Schaller (2): drivers:power:twl4030-charger: fix problem with EPROBE_DEFER drivers:power:twl4030-charger: don't check if battery is present drivers/power/twl4030_charger.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) -- 2.5.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] drivers:power:twl4030-charger: fix problem with EPROBE_DEFER 2015-10-29 17:01 [PATCH 0/2] Fixes for twl4030 charger H. Nikolaus Schaller @ 2015-10-29 17:01 ` H. Nikolaus Schaller 2015-10-29 19:52 ` Nishanth Menon 2015-10-29 17:01 ` [PATCH 2/2] drivers:power:twl4030-charger: don't check if battery is present H. Nikolaus Schaller 1 sibling, 1 reply; 7+ messages in thread From: H. Nikolaus Schaller @ 2015-10-29 17:01 UTC (permalink / raw) To: Gražvydas Ignotas, Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse Cc: linux-pm, gta04-owner, H. Nikolaus Schaller devm_usb_get_phy_by_node() may return -EPROBE_DEFER in which case we should also defer probing of the twl4030 charger instead of turing USB charging off (forever). 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 423e886..859991f 100644 --- a/drivers/power/twl4030_charger.c +++ b/drivers/power/twl4030_charger.c @@ -1059,9 +1059,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
* Re: [PATCH 1/2] drivers:power:twl4030-charger: fix problem with EPROBE_DEFER 2015-10-29 17:01 ` [PATCH 1/2] drivers:power:twl4030-charger: fix problem with EPROBE_DEFER H. Nikolaus Schaller @ 2015-10-29 19:52 ` Nishanth Menon 2015-10-30 6:01 ` H. Nikolaus Schaller 0 siblings, 1 reply; 7+ messages in thread From: Nishanth Menon @ 2015-10-29 19:52 UTC (permalink / raw) To: H. Nikolaus Schaller, Gražvydas Ignotas, Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse Cc: linux-pm, gta04-owner On 10/29/2015 12:01 PM, H. Nikolaus Schaller wrote: > devm_usb_get_phy_by_node() may return -EPROBE_DEFER in which > case we should also defer probing of the twl4030 charger > instead of turing USB charging off (forever). s/turing/turning > > 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 423e886..859991f 100644 > --- a/drivers/power/twl4030_charger.c > +++ b/drivers/power/twl4030_charger.c > @@ -1059,9 +1059,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) I might align this with the IS_ERR > + return -EPROBE_DEFER; /* PHY not ready */ > + } > } > > /* Enable interrupts now. */ > Otherwise it looks good to me. -- Regards, Nishanth Menon ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] drivers:power:twl4030-charger: fix problem with EPROBE_DEFER 2015-10-29 19:52 ` Nishanth Menon @ 2015-10-30 6:01 ` H. Nikolaus Schaller 0 siblings, 0 replies; 7+ messages in thread From: H. Nikolaus Schaller @ 2015-10-30 6:01 UTC (permalink / raw) To: Nishanth Menon Cc: Gražvydas Ignotas, Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse, linux-pm, gta04-owner Am 29.10.2015 um 20:52 schrieb Nishanth Menon <nm@ti.com>: > On 10/29/2015 12:01 PM, H. Nikolaus Schaller wrote: >> devm_usb_get_phy_by_node() may return -EPROBE_DEFER in which >> case we should also defer probing of the twl4030 charger >> instead of turing USB charging off (forever). > > s/turing/turning Thanks for noting this. > >> >> 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 423e886..859991f 100644 >> --- a/drivers/power/twl4030_charger.c >> +++ b/drivers/power/twl4030_charger.c >> @@ -1059,9 +1059,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) > I might align this with the IS_ERR Ok. > >> + return -EPROBE_DEFER; /* PHY not ready */ >> + } >> } >> >> /* Enable interrupts now. */ >> > > Otherwise it looks good to me. I will update for v2. BR, Nikolaus Schaller igitt ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] drivers:power:twl4030-charger: don't check if battery is present 2015-10-29 17:01 [PATCH 0/2] Fixes for twl4030 charger H. Nikolaus Schaller 2015-10-29 17:01 ` [PATCH 1/2] drivers:power:twl4030-charger: fix problem with EPROBE_DEFER H. Nikolaus Schaller @ 2015-10-29 17:01 ` H. Nikolaus Schaller 2015-10-29 19:50 ` Nishanth Menon 1 sibling, 1 reply; 7+ messages in thread From: H. Nikolaus Schaller @ 2015-10-29 17:01 UTC (permalink / raw) To: Gražvydas Ignotas, Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse Cc: linux-pm, gta04-owner, H. Nikolaus Schaller We can't assume that the battery is present after probing (it can usually be removed while device is operated through external AC or USB power). So it makes no sense to check for it 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 859991f..e232453 100644 --- a/drivers/power/twl4030_charger.c +++ b/drivers/power/twl4030_charger.c @@ -1008,13 +1008,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 2/2] drivers:power:twl4030-charger: don't check if battery is present 2015-10-29 17:01 ` [PATCH 2/2] drivers:power:twl4030-charger: don't check if battery is present H. Nikolaus Schaller @ 2015-10-29 19:50 ` Nishanth Menon 2015-10-30 6:14 ` H. Nikolaus Schaller 0 siblings, 1 reply; 7+ messages in thread From: Nishanth Menon @ 2015-10-29 19:50 UTC (permalink / raw) To: H. Nikolaus Schaller, Gražvydas Ignotas, Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse Cc: linux-pm, gta04-owner On 10/29/2015 12:01 PM, H. Nikolaus Schaller wrote: > We can't assume that the battery is present after probing (it can > usually be removed while device is operated through external AC > or USB power). So it makes no sense to check for it during probe. Do you mean hot plug battery? you sure twl4030 is capable of dealing with that :) ? we ran into all kinds of issues with LDP3430 trying to make that logic work, finally came to the conclusion that the TWL4030 as it stands on LDP cannot just do that. > > 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 859991f..e232453 100644 > --- a/drivers/power/twl4030_charger.c > +++ b/drivers/power/twl4030_charger.c > @@ -1008,13 +1008,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, > -- Regards, Nishanth Menon ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] drivers:power:twl4030-charger: don't check if battery is present 2015-10-29 19:50 ` Nishanth Menon @ 2015-10-30 6:14 ` H. Nikolaus Schaller 0 siblings, 0 replies; 7+ messages in thread From: H. Nikolaus Schaller @ 2015-10-30 6:14 UTC (permalink / raw) To: Nishanth Menon Cc: Gražvydas Ignotas, Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse, linux-pm, gta04-owner Am 29.10.2015 um 20:50 schrieb Nishanth Menon <nm@ti.com>: > On 10/29/2015 12:01 PM, H. Nikolaus Schaller wrote: >> We can't assume that the battery is present after probing (it can >> usually be removed while device is operated through external AC >> or USB power). So it makes no sense to check for it during probe. > > > Do you mean hot plug battery? Yes. > you sure twl4030 is capable of dealing > with that :) ? I am not sure about the twl4030 but with the tps65950 used in the GTA04, it worked always in the past years with the old drivers (before 4.3-rc1 because we have not tested the new ones well enough). > we ran into all kinds of issues with LDP3430 trying to > make that logic work, finally came to the conclusion that the TWL4030 > as it stands on LDP cannot just do that. One issue is of course that AC or USB must provide enough current to operate the board stand-alone. I.e. charging with high enough max_current must be enabled. A typical scenario I was using is to enable e.g. 800 mA max_current for USB, connect the device to some laptop or USB wall charger and then replace the battery. This works with full and empty replacement batteries. Well, it should not be too empty so that the inrush-current is not too much for the USB supply. What we can't do is to boot from power-off without battery. In that case the BCI isn't enabled for high enough current to operate the OMAP. But as soon as the BCI enables enough current, it worked. My main observation is that you can't make sure by software that the user is not trying to hot-swap the battery. I.e. it can still happen after probe time. This is why I think this check doesn't solve a problem. So if there are problems with battery removed, we must find a solution in the driver to handle that situation. And not only check once at boot time. BR, NIkolaus Schaller > >> >> 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 859991f..e232453 100644 >> --- a/drivers/power/twl4030_charger.c >> +++ b/drivers/power/twl4030_charger.c >> @@ -1008,13 +1008,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, >> > > > -- > Regards, > Nishanth Menon ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-10-30 6:14 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-10-29 17:01 [PATCH 0/2] Fixes for twl4030 charger H. Nikolaus Schaller 2015-10-29 17:01 ` [PATCH 1/2] drivers:power:twl4030-charger: fix problem with EPROBE_DEFER H. Nikolaus Schaller 2015-10-29 19:52 ` Nishanth Menon 2015-10-30 6:01 ` H. Nikolaus Schaller 2015-10-29 17:01 ` [PATCH 2/2] drivers:power:twl4030-charger: don't check if battery is present H. Nikolaus Schaller 2015-10-29 19:50 ` Nishanth Menon 2015-10-30 6:14 ` H. Nikolaus Schaller
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).