| Message ID | 0eeffa7a6ab8e4213f518e2caf6982d2b77dc0b2.1497432355.git.hns@goldelico.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On Wed, Jun 14, 2017 at 11:25:55AM +0200, H. Nikolaus Schaller wrote: > This avoids a potential race if irqs are enabled and triggered too early > before the worker is properly set up. > > Suggested-by: Grygorii Strashko <grygorii.strashko@ti.com> > Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com> > --- > drivers/power/supply/twl4030_charger.c | 28 ++++++++++++++-------------- > 1 file changed, 14 insertions(+), 14 deletions(-) > > diff --git a/drivers/power/supply/twl4030_charger.c b/drivers/power/supply/twl4030_charger.c > index 1fbbe0cc216a..3bebeecb4a1f 100644 > --- a/drivers/power/supply/twl4030_charger.c > +++ b/drivers/power/supply/twl4030_charger.c > @@ -984,6 +984,16 @@ 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); > + } > + The notifier is not that much different from an irq. I see the call can access at least the iio channel (so iio channel should be registered first). I suspect worker and power-supply should also be initialized/registered first. Best location seems to be directly before requesting the irqs. > bci->channel_vac = devm_iio_channel_get(&pdev->dev, "vac"); > if (IS_ERR(bci->channel_vac)) { > bci->channel_vac = NULL; > @@ -1006,6 +1016,10 @@ 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; You should configure the notifier block *before* registering it. > ret = devm_request_threaded_irq(&pdev->dev, bci->irq_chg, NULL, > twl4030_charger_interrupt, IRQF_ONESHOT, pdev->name, > bci); > @@ -1023,20 +1037,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); -- Sebastian
diff --git a/drivers/power/supply/twl4030_charger.c b/drivers/power/supply/twl4030_charger.c index 1fbbe0cc216a..3bebeecb4a1f 100644 --- a/drivers/power/supply/twl4030_charger.c +++ b/drivers/power/supply/twl4030_charger.c @@ -984,6 +984,16 @@ 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); + } + bci->channel_vac = devm_iio_channel_get(&pdev->dev, "vac"); if (IS_ERR(bci->channel_vac)) { bci->channel_vac = NULL; @@ -1006,6 +1016,10 @@ 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; ret = devm_request_threaded_irq(&pdev->dev, bci->irq_chg, NULL, twl4030_charger_interrupt, IRQF_ONESHOT, pdev->name, bci); @@ -1023,20 +1037,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);
This avoids a potential race if irqs are enabled and triggered too early before the worker is properly set up. Suggested-by: Grygorii Strashko <grygorii.strashko@ti.com> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com> --- drivers/power/supply/twl4030_charger.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-)