| Message ID | 2ed01020fa8a135c36dbaa871095ded47d926507.1676464968.git.christophe.jaillet@wanadoo.fr |
|---|---|
| State | Accepted |
| Commit | cba320408d631422fef0ad8407954fb9d6f8f650 |
| Headers | show |
| Series | power: supply: rt9467: Fix rt9467_run_aicl() | expand |
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 <cy_huang@richtek.com> > Fixes: 6f7f70e3a8dd ("power: supply: rt9467: Add Richtek RT9467 charger driver") > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > --- > 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 >
Le 17/02/2023 à 06:51, ChiYuan Huang a écrit : > 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 <cy_huang@richtek.com> Hi, polite reminder. This patch is still not in -next. CJ >> Fixes: 6f7f70e3a8dd ("power: supply: rt9467: Add Richtek RT9467 charger driver") >> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> >> --- >> 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 >> >
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,
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) {
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 <christophe.jaillet@wanadoo.fr> --- 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(-)