Skip to contentSkip to conversation

More fixes for twl4030 charger

PATCH In review H. Nikolaus Schaller 2 participants 4 messages Jul 3, 2017 Power supply

lore
Share & tools
lore
#0 lore raw

v8 cover More fixes for twl4030 charger

Changes V78:
* further fix for wrong notifier initialization sequence (suggested by Sebastian Reichel)

2017-06-19 10:41:47: Changes V7:

* removed already merged patches from patch set
* further fix for potentially wrong irq and notifier initialization sequence (suggested by Sebastian Reichel)
* NULLify transceiver pointer in case of errors (suggested by Sebastian Reichel)

2017-06-14 11:25:59: Changes V6:

* split up -EPROBE_DEFER and irq allocation patch into small steps:
* convert iio to devm allocation (by Sebsatian Reichel)
* fix potentially wrong initialization sequence (by Grygorii Strashko)
* add -EPROBE_DEFER

2017-05-21 12:38:24: Changes V5:

* reworked max_current removal patch (comments by Sebastian Reichel)
* resubmit missing madc connection for AC power detection
* resubmit patch for irq allocation and -EPROBE_DEFER
* rebased on 4.12-rc1

Changes V4:
* resent commit (original one did contain material not upstream)

2017-04-14 21:29:39: 2017-04-14 20:26:00: Changes V3:

* worked in comments by Sebsatian Reichel
* clarifications of some commit messages
* rebased on v4.11rc-6

It took a while (18 months) until we propose an updated patch for upstream...

2015-11-02 12:27:39: 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. This patch set fixes some issues.

While making twl4030 changes from 4.3 operable we have found some issues during testing on GTA04 and OpenPandora.

H. Nikolaus Schaller (2):
power: supply: twl4030-charger: move irq allocation to just before
irqs are enabled
power: supply: twl4030-charger: add deferred probing for phy and iio

 drivers/power/supply/twl4030_charger.c | 43 ++++++++++++++++++++++------------
 1 file changed, 28 insertions(+), 15 deletions(-)
Signature · 1 line
2.12.2
#1 lore raw

PATCH v8 1/2 power: supply: twl4030-charger: move irq allocation to just before irqs are enabled

And initialize workers and notifiers as soon as possible.

This avoids a potential race if irqs are enabled and triggered too early before the worker is properly set up.

---
 drivers/power/supply/twl4030_charger.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)
drivers/power/supply/twl4030_charger.c +15 -14
index 1fbbe0cc216a..fde6f07ba8c3 100644
@@ -984,12 +984,27 @@ static int twl4030_bci_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, bci);
 
+	INIT_WORK(&bci->work, twl4030_bci_usb_work);
+	INIT_DELAYED_WORK(&bci->current_worker, twl4030_current_worker);
+
 	bci->channel_vac = devm_iio_channel_get(&pdev->dev, "vac");
 	if (IS_ERR(bci->channel_vac)) {
 		bci->channel_vac = NULL;
 		dev_warn(&pdev->dev, "could not request vac iio channel");
 	}
 
+	if (bci->dev->of_node) {
+		struct device_node *phynode;
+
+		phynode = of_find_compatible_node(bci->dev->of_node->parent,
+						  NULL, "ti,twl4030-usb");
+		if (phynode) {
+			bci->usb_nb.notifier_call = twl4030_bci_usb_ncb;
+			bci->transceiver = devm_usb_get_phy_by_node(
+				bci->dev, phynode, &bci->usb_nb);
+		}
+	}
+
 	bci->ac = devm_power_supply_register(&pdev->dev, &twl4030_bci_ac_desc,
 					     NULL);
 	if (IS_ERR(bci->ac)) {
@@ -1023,20 +1038,6 @@ static int twl4030_bci_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	INIT_WORK(&bci->work, twl4030_bci_usb_work);
-	INIT_DELAYED_WORK(&bci->current_worker, twl4030_current_worker);
-
-	bci->usb_nb.notifier_call = twl4030_bci_usb_ncb;
-	if (bci->dev->of_node) {
-		struct device_node *phynode;
-
-		phynode = of_find_compatible_node(bci->dev->of_node->parent,
-						  NULL, "ti,twl4030-usb");
-		if (phynode)
-			bci->transceiver = devm_usb_get_phy_by_node(
-				bci->dev, phynode, &bci->usb_nb);
-	}
-
 	/* Enable interrupts now. */
 	reg = ~(u32)(TWL4030_ICHGLOW | TWL4030_ICHGEOC | TWL4030_TBATOR2 |
 		TWL4030_TBATOR1 | TWL4030_BATSTS);
Signature · 1 line
2.12.2
#2 lore raw

PATCH v8 2/2 power: supply: twl4030-charger: add deferred probing for phy and iio

In reply to H. Nikolaus Schaller

This fixes an issue if both this twl4030_charger driver and phy-twl4030-usb are compiled as modules and loaded in random order.

It has been observed on GTA04 and OpenPandora devices that in worst case the boot process hangs and in best case the AC detection fails with a warning.

Therefore we add deferred probing checks for the usb_phy and the iio channel for AC detection.

---
 drivers/power/supply/twl4030_charger.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)
drivers/power/supply/twl4030_charger.c +13 -1
index fde6f07ba8c3..86fe3aca2b1c 100644
@@ -989,8 +989,12 @@ static int twl4030_bci_probe(struct platform_device *pdev)
 
 	bci->channel_vac = devm_iio_channel_get(&pdev->dev, "vac");
 	if (IS_ERR(bci->channel_vac)) {
+		ret = PTR_ERR(bci->channel_vac);
+		if (ret == -EPROBE_DEFER)
+			return ret;	/* iio not ready */
+		dev_warn(&pdev->dev, "could not request vac iio channel (%d)",
+			ret);
 		bci->channel_vac = NULL;
-		dev_warn(&pdev->dev, "could not request vac iio channel");
 	}
 
 	if (bci->dev->of_node) {
@@ -1002,6 +1006,14 @@ static int twl4030_bci_probe(struct platform_device *pdev)
 			bci->usb_nb.notifier_call = twl4030_bci_usb_ncb;
 			bci->transceiver = devm_usb_get_phy_by_node(
 				bci->dev, phynode, &bci->usb_nb);
+			if (IS_ERR(bci->transceiver)) {
+				ret = PTR_ERR(bci->transceiver);
+				if (ret == -EPROBE_DEFER)
+					return ret;	/* phy not ready */
+				dev_warn(&pdev->dev, "could not request transceiver (%d)",
+					ret);
+				bci->transceiver = NULL;
+			}
 		}
 	}
 
Signature · 1 line
2.12.2
#3 lore raw
In reply to H. Nikolaus Schaller

Hi,

Thanks, queued. I will send these in 2nd pull-request end of next week, so that they get some time in linux-next.

-- Sebastian

Quoted 52 lines · H. Nikolaus Schaller

Changes V78:
* further fix for wrong notifier initialization sequence (suggested by Sebastian Reichel)

2017-06-19 10:41:47: Changes V7:

* removed already merged patches from patch set
* further fix for potentially wrong irq and notifier initialization sequence (suggested by Sebastian Reichel)
* NULLify transceiver pointer in case of errors (suggested by Sebastian Reichel)

2017-06-14 11:25:59: Changes V6:

* split up -EPROBE_DEFER and irq allocation patch into small steps:
* convert iio to devm allocation (by Sebsatian Reichel)
* fix potentially wrong initialization sequence (by Grygorii Strashko)
* add -EPROBE_DEFER

2017-05-21 12:38:24: Changes V5:

* reworked max_current removal patch (comments by Sebastian Reichel)
* resubmit missing madc connection for AC power detection
* resubmit patch for irq allocation and -EPROBE_DEFER
* rebased on 4.12-rc1

Changes V4:
* resent commit (original one did contain material not upstream)

2017-04-14 21:29:39: 2017-04-14 20:26:00: Changes V3:

* worked in comments by Sebsatian Reichel
* clarifications of some commit messages
* rebased on v4.11rc-6

It took a while (18 months) until we propose an updated patch for upstream...

2015-11-02 12:27:39: 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. This patch set fixes some issues.

While making twl4030 changes from 4.3 operable we have found some issues during testing on GTA04 and OpenPandora.

H. Nikolaus Schaller (2):
power: supply: twl4030-charger: move irq allocation to just before
irqs are enabled
power: supply: twl4030-charger: add deferred probing for phy and iio

 drivers/power/supply/twl4030_charger.c | 43 ++++++++++++++++++++++------------
 1 file changed, 28 insertions(+), 15 deletions(-)
Signature · 1 line
2.12.2