Re: [PATCH v5 3/3] drivers:power:twl4030-charger: add deferred probing for phy and iio
From: Sebastian Reichel <hidden>
Date: 2017-06-07 20:44:50
Also in:
linux-devicetree, linux-omap, lkml
Hi, On Sun, May 21, 2017 at 12:38:18PM +0200, H. Nikolaus Schaller wrote:
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. For implementing this we must reorder code because we can't safely return -EPROBE_DEFER after allocating any devm managed interrupt (it might already/still be enabled without working interrupt handler). So the check for required resources that may abort probing by returning -EPROBE_DEFER, must come first.
This sounds fishy. EPROBE_DEFER should not be different from other error codes in this regard and devm_ requested resources should be free'd on any error. Why is irq handler not working?
quoted hunk ↗ jump to hunk
Signed-off-by: H. Nikolaus Schaller <redacted> --- drivers/power/supply/twl4030_charger.c | 37 ++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 15 deletions(-)diff --git a/drivers/power/supply/twl4030_charger.c b/drivers/power/supply/twl4030_charger.c index 785a07bc4f39..945eabdbbc89 100644 --- a/drivers/power/supply/twl4030_charger.c +++ b/drivers/power/supply/twl4030_charger.c@@ -984,6 +984,28 @@ static int twl4030_bci_probe(struct platform_device *pdev) platform_set_drvdata(pdev, bci); + 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); + if (IS_ERR(bci->transceiver) && + PTR_ERR(bci->transceiver) == -EPROBE_DEFER) + return -EPROBE_DEFER; /* PHY not ready */ + } + } + + bci->channel_vac = iio_channel_get(&pdev->dev, "vac"); + if (IS_ERR(bci->channel_vac)) { + if (PTR_ERR(bci->channel_vac) == -EPROBE_DEFER) + return -EPROBE_DEFER; /* iio not ready */ + dev_warn(&pdev->dev, "could not request vac iio channel"); + bci->channel_vac = NULL; + } +
You should not request non-devm managed iio_channel before devm-managed power-supply. This could be fixed by switching to devm_iio_channel_get(), which also cleans up some code. I suspect, that this is also your IRQ problem, since iio_channel is currently free'd before irqs are free'd, but its used in irq code.
quoted hunk ↗ jump to hunk
bci->ac = devm_power_supply_register(&pdev->dev, &twl4030_bci_ac_desc, NULL); if (IS_ERR(bci->ac)) {@@ -1017,25 +1039,10 @@ static int twl4030_bci_probe(struct platform_device *pdev) return ret; } - bci->channel_vac = 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"); - } - 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 |
-- Sebastian
Attachments
- signature.asc [application/pgp-signature] 833 bytes