2023-02-15 12:43:18

by Christophe JAILLET

Subject: [PATCH] power: supply: rt9467: Fix rt9467_run_aicl()

It is spurious to bail-out on a wait_for_completion_timeout() call that
does NOT timeout.

Reverse the logic to return -ETIMEDOUT instead, in case of tiemout.

Fixes: 6f7f70e3a8dd ("power: supply: rt9467: Add Richtek RT9467 charger driver")
Signed-off-by: Christophe JAILLET <[email protected]>
---
Compile tested only.

This is spurious, because if I'm right, it means that it does not work!
Testing on a real hardware is really welcomed.
---
drivers/power/supply/rt9467-charger.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/power/supply/rt9467-charger.c b/drivers/power/supply/rt9467-charger.c
index 96ad0d7d3af4..bb737fd1cca6 100644
--- a/drivers/power/supply/rt9467-charger.c
+++ b/drivers/power/supply/rt9467-charger.c
@@ -598,8 +598,8 @@ static int rt9467_run_aicl(struct rt9467_chg_data *data)

reinit_completion(&data->aicl_done);
ret = wait_for_completion_timeout(&data->aicl_done, msecs_to_jiffies(3500));
- if (ret)
- return ret;
+ if (ret == 0)
+ return -ETIMEDOUT;

ret = rt9467_get_value_from_ranges(data, F_IAICR, RT9467_RANGE_IAICR, &aicr_get);
if (ret) {
--
2.34.1

2023-02-17 05:51:39

by ChiYuan Huang

Subject: Re: [PATCH] power: supply: rt9467: Fix rt9467_run_aicl()

Hi, Christophe:
On Wed, Feb 15, 2023 at 01:43:04PM +0100, Christophe JAILLET wrote:
> It is spurious to bail-out on a wait_for_completion_timeout() call that
> does NOT timeout.
>
> Reverse the logic to return -ETIMEDOUT instead, in case of tiemout.
>
Thanks. It's really our coding issue.

Reviewed-by: ChiYuan Huang <[email protected]>
> Fixes: 6f7f70e3a8dd ("power: supply: rt9467: Add Richtek RT9467 charger driver")
> Signed-off-by: Christophe JAILLET <[email protected]>
> ---
> Compile tested only.
>
> This is spurious, because if I'm right, it means that it does not work!
> Testing on a real hardware is really welcomed.
> ---
> drivers/power/supply/rt9467-charger.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/power/supply/rt9467-charger.c b/drivers/power/supply/rt9467-charger.c
> index 96ad0d7d3af4..bb737fd1cca6 100644
> --- a/drivers/power/supply/rt9467-charger.c
> +++ b/drivers/power/supply/rt9467-charger.c
> @@ -598,8 +598,8 @@ static int rt9467_run_aicl(struct rt9467_chg_data *data)
>
> reinit_completion(&data->aicl_done);
> ret = wait_for_completion_timeout(&data->aicl_done, msecs_to_jiffies(3500));
> - if (ret)
> - return ret;
> + if (ret == 0)
> + return -ETIMEDOUT;
>
> ret = rt9467_get_value_from_ranges(data, F_IAICR, RT9467_RANGE_IAICR, &aicr_get);
> if (ret) {
> --
> 2.34.1
>

2023-09-14 21:46:05

by Sebastian Reichel

Subject: Re: [PATCH] power: supply: rt9467: Fix rt9467_run_aicl()

On Wed, 15 Feb 2023 13:43:04 +0100, Christophe JAILLET wrote:
> It is spurious to bail-out on a wait_for_completion_timeout() call that
> does NOT timeout.
>
> Reverse the logic to return -ETIMEDOUT instead, in case of tiemout.
>
>

Applied, thanks!

[1/1] power: supply: rt9467: Fix rt9467_run_aicl()
commit: cba320408d631422fef0ad8407954fb9d6f8f650

Best regards,
--
Sebastian Reichel <[email protected]>