* [PATCH v7 1/2] power: supply: twl4030-charger: move irq allocation to just before irqs are enabled
2017-06-19 8:41 [PATCH v7 0/2] More fixes for twl4030 charger H. Nikolaus Schaller
@ 2017-06-19 8:41 ` H. Nikolaus Schaller
2017-07-03 12:55 ` Sebastian Reichel
2017-06-19 8:41 ` [PATCH v7 2/2] power: supply: twl4030-charger: add deferred probing for phy and iio H. Nikolaus Schaller
2017-07-01 11:49 ` [PATCH v7 0/2] More fixes for twl4030 charger H. Nikolaus Schaller
2 siblings, 1 reply; 7+ messages in thread
From: H. Nikolaus Schaller @ 2017-06-19 8:41 UTC (permalink / raw)
To: Grygorii Strashko, NeilBrown, Rob Herring, Mark Rutland,
Russell King, Sebastian Reichel, Marek Belisko,
H. Nikolaus Schaller
Cc: linux-kernel, devicetree, linux-pm, letux-kernel, notasas, linux-omap
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.
Suggested-by: Grygorii Strashko <grygorii.strashko@ti.com>
Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
drivers/power/supply/twl4030_charger.c | 29 +++++++++++++++--------------
1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/drivers/power/supply/twl4030_charger.c b/drivers/power/supply/twl4030_charger.c
index 1fbbe0cc216a..9d974f1e3957 100644
--- a/drivers/power/supply/twl4030_charger.c
+++ b/drivers/power/supply/twl4030_charger.c
@@ -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->transceiver = devm_usb_get_phy_by_node(
+ bci->dev, phynode, &bci->usb_nb);
+ bci->usb_nb.notifier_call = twl4030_bci_usb_ncb;
+ }
+ }
+
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);
--
2.12.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v7 1/2] power: supply: twl4030-charger: move irq allocation to just before irqs are enabled
2017-06-19 8:41 ` [PATCH v7 1/2] power: supply: twl4030-charger: move irq allocation to just before irqs are enabled H. Nikolaus Schaller
@ 2017-07-03 12:55 ` Sebastian Reichel
2017-07-03 16:36 ` H. Nikolaus Schaller
0 siblings, 1 reply; 7+ messages in thread
From: Sebastian Reichel @ 2017-07-03 12:55 UTC (permalink / raw)
To: H. Nikolaus Schaller
Cc: Grygorii Strashko, NeilBrown, Rob Herring, Mark Rutland,
Russell King, Marek Belisko, linux-kernel, devicetree, linux-pm,
letux-kernel, notasas, linux-omap
[-- Attachment #1: Type: text/plain, Size: 1636 bytes --]
Hi,
On Mon, Jun 19, 2017 at 10:41:45AM +0200, H. Nikolaus Schaller wrote:
> 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.
>
> Suggested-by: Grygorii Strashko <grygorii.strashko@ti.com>
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> ---
> drivers/power/supply/twl4030_charger.c | 29 +++++++++++++++--------------
> 1 file changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/power/supply/twl4030_charger.c b/drivers/power/supply/twl4030_charger.c
> index 1fbbe0cc216a..9d974f1e3957 100644
> --- a/drivers/power/supply/twl4030_charger.c
> +++ b/drivers/power/supply/twl4030_charger.c
> @@ -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->transceiver = devm_usb_get_phy_by_node(
> + bci->dev, phynode, &bci->usb_nb);
> + bci->usb_nb.notifier_call = twl4030_bci_usb_ncb;
those two should be the other way around.
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v7 1/2] power: supply: twl4030-charger: move irq allocation to just before irqs are enabled
2017-07-03 12:55 ` Sebastian Reichel
@ 2017-07-03 16:36 ` H. Nikolaus Schaller
0 siblings, 0 replies; 7+ messages in thread
From: H. Nikolaus Schaller @ 2017-07-03 16:36 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Grygorii Strashko, NeilBrown, Rob Herring, Mark Rutland,
Russell King, Marek Belisko, linux-kernel, devicetree, linux-pm,
letux-kernel, notasas, linux-omap
[-- Attachment #1: Type: text/plain, Size: 1882 bytes --]
Hi,
> Am 03.07.2017 um 14:55 schrieb Sebastian Reichel <sebastian.reichel@collabora.co.uk>:
>
> Hi,
>
> On Mon, Jun 19, 2017 at 10:41:45AM +0200, H. Nikolaus Schaller wrote:
>> 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.
>>
>> Suggested-by: Grygorii Strashko <grygorii.strashko@ti.com>
>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>> ---
>> drivers/power/supply/twl4030_charger.c | 29 +++++++++++++++--------------
>> 1 file changed, 15 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/power/supply/twl4030_charger.c b/drivers/power/supply/twl4030_charger.c
>> index 1fbbe0cc216a..9d974f1e3957 100644
>> --- a/drivers/power/supply/twl4030_charger.c
>> +++ b/drivers/power/supply/twl4030_charger.c
>> @@ -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->transceiver = devm_usb_get_phy_by_node(
>> + bci->dev, phynode, &bci->usb_nb);
>> + bci->usb_nb.notifier_call = twl4030_bci_usb_ncb;
>
> those two should be the other way around.
Seems to work as well (and avoids another potential race condition).
v8 coming instantly.
BR and thanks,
Nikolaus
[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v7 2/2] power: supply: twl4030-charger: add deferred probing for phy and iio
2017-06-19 8:41 [PATCH v7 0/2] More fixes for twl4030 charger H. Nikolaus Schaller
2017-06-19 8:41 ` [PATCH v7 1/2] power: supply: twl4030-charger: move irq allocation to just before irqs are enabled H. Nikolaus Schaller
@ 2017-06-19 8:41 ` H. Nikolaus Schaller
2017-07-03 12:59 ` Sebastian Reichel
2017-07-01 11:49 ` [PATCH v7 0/2] More fixes for twl4030 charger H. Nikolaus Schaller
2 siblings, 1 reply; 7+ messages in thread
From: H. Nikolaus Schaller @ 2017-06-19 8:41 UTC (permalink / raw)
To: Grygorii Strashko, NeilBrown, Rob Herring, Mark Rutland,
Russell King, Sebastian Reichel, Marek Belisko,
H. Nikolaus Schaller
Cc: linux-kernel, devicetree, linux-pm, letux-kernel, notasas, linux-omap
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.
Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
drivers/power/supply/twl4030_charger.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/power/supply/twl4030_charger.c b/drivers/power/supply/twl4030_charger.c
index 9d974f1e3957..0d2125e4f735 100644
--- a/drivers/power/supply/twl4030_charger.c
+++ b/drivers/power/supply/twl4030_charger.c
@@ -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->transceiver = devm_usb_get_phy_by_node(
bci->dev, phynode, &bci->usb_nb);
bci->usb_nb.notifier_call = twl4030_bci_usb_ncb;
+ 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;
+ }
}
}
--
2.12.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v7 2/2] power: supply: twl4030-charger: add deferred probing for phy and iio
2017-06-19 8:41 ` [PATCH v7 2/2] power: supply: twl4030-charger: add deferred probing for phy and iio H. Nikolaus Schaller
@ 2017-07-03 12:59 ` Sebastian Reichel
0 siblings, 0 replies; 7+ messages in thread
From: Sebastian Reichel @ 2017-07-03 12:59 UTC (permalink / raw)
To: H. Nikolaus Schaller
Cc: Grygorii Strashko, NeilBrown, Rob Herring, Mark Rutland,
Russell King, Marek Belisko, linux-kernel, devicetree, linux-pm,
letux-kernel, notasas, linux-omap
[-- Attachment #1: Type: text/plain, Size: 2020 bytes --]
Hi,
On Mon, Jun 19, 2017 at 10:41:46AM +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.
>
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
Looks fine to me.
-- Sebastian
> ---
> drivers/power/supply/twl4030_charger.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/power/supply/twl4030_charger.c b/drivers/power/supply/twl4030_charger.c
> index 9d974f1e3957..0d2125e4f735 100644
> --- a/drivers/power/supply/twl4030_charger.c
> +++ b/drivers/power/supply/twl4030_charger.c
> @@ -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->transceiver = devm_usb_get_phy_by_node(
> bci->dev, phynode, &bci->usb_nb);
> bci->usb_nb.notifier_call = twl4030_bci_usb_ncb;
> + 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;
> + }
> }
> }
>
> --
> 2.12.2
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v7 0/2] More fixes for twl4030 charger
2017-06-19 8:41 [PATCH v7 0/2] More fixes for twl4030 charger H. Nikolaus Schaller
2017-06-19 8:41 ` [PATCH v7 1/2] power: supply: twl4030-charger: move irq allocation to just before irqs are enabled H. Nikolaus Schaller
2017-06-19 8:41 ` [PATCH v7 2/2] power: supply: twl4030-charger: add deferred probing for phy and iio H. Nikolaus Schaller
@ 2017-07-01 11:49 ` H. Nikolaus Schaller
2 siblings, 0 replies; 7+ messages in thread
From: H. Nikolaus Schaller @ 2017-07-01 11:49 UTC (permalink / raw)
To: Sebastian Reichel
Cc: LKML, devicetree, Linux PM mailing list,
Discussions about the Letux Kernel, Grazvydas Ignotas,
linux-omap, Nikolaus Schaller, Marek Belisko, Russell King,
Mark Rutland, Rob Herring, NeilBrown, Grygorii Strashko
Hi Sebastian,
> Am 19.06.2017 um 10:41 schrieb H. Nikolaus Schaller <hns@goldelico.com>:
>
> 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)
Did you find time to review the updated v7 patches?
I have not seen them arrive in linux-next.
BR and thanks,
Nikolaus
^ permalink raw reply [flat|nested] 7+ messages in thread