power: supply: charger-manager: Fix info message in check_charging_duration()

PATCH In review Gustavo A. R. Silva 4 participants 5 messages Sep 2, 2020 – Sep 3, 2020 Power supply

Share & tools

Gustavo A. R. Silva

author

2020-09-02 16:17 UTC #0 lore raw

PATCH v2 1/1 power: supply: charger-manager: Fix info message in check_charging_duration()

A few months ago, commit e132fc6bb89b ("power: supply: charger-manager: Make decisions focussed on battery status") changed the expression in the if statement from "duration > desc->discharging_max_duration_ms" to "duration > desc->charging_max_duration_ms", but the arguments for dev_info() were left unchanged. Apparently, due to a copy-paste error.

Fix this by using the proper arguments for dev_info().

Also, while there, replace "exceed" with "exceeds", for both messages.

Addresses-Coverity-ID: 1496803 ("Copy-paste error")

Fixes e132fc6bb89b ("power: supply: charger-manager: Make decisions focussed on battery status")

Signed-off-by Gustavo A. R. Silva <[email protected]>

---
Changes in v2:
- Replace "exceed" with "exceeds"

 drivers/power/supply/charger-manager.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

drivers/power/supply/charger-manager.c +3 -3

index 07992821e252..a6d5dbd55e37 100644
@@ -464,7 +464,7 @@ static int check_charging_duration(struct charger_manager *cm)
 		duration = curr - cm->charging_start_time;
 
 		if (duration > desc->charging_max_duration_ms) {
-			dev_info(cm->dev, "Charging duration exceed %ums\n",
+			dev_info(cm->dev, "Charging duration exceeds %ums\n",
 				 desc->charging_max_duration_ms);
 			ret = true;
 		}

@@ -472,8 +472,8 @@ static int check_charging_duration(struct charger_manager *cm)
 		duration = curr - cm->charging_end_time;
 
 		if (duration > desc->charging_max_duration_ms) {
-			dev_info(cm->dev, "Discharging duration exceed %ums\n",
-				 desc->discharging_max_duration_ms);
+			dev_info(cm->dev, "Charging duration exceeds %ums\n",
+				 desc->charging_max_duration_ms);
 			ret = true;
 		}
 	}

Signature · 1 line

2.27.0

Randy Dunlap

2020-09-02 16:29 UTC #1 lore raw

Quoted 34 lines · Gustavo A. R. Silva

A few months ago, commit e132fc6bb89b ("power: supply: charger-manager: Make decisions focussed on battery status") changed the expression in the if statement from "duration > desc->discharging_max_duration_ms" to "duration > desc->charging_max_duration_ms", but the arguments for dev_info() were left unchanged. Apparently, due to a copy-paste error.
Fix this by using the proper arguments for dev_info().
Also, while there, replace "exceed" with "exceeds", for both messages.
Addresses-Coverity-ID: 1496803 ("Copy-paste error")
Fixes e132fc6bb89b ("power: supply: charger-manager: Make decisions focussed on battery status")
Signed-off-by Gustavo A. R. Silva <[email protected]>
---
Changes in v2:
- Replace "exceed" with "exceeds"
 drivers/power/supply/charger-manager.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/power/supply/charger-manager.c +1 -1
index 07992821e252..a6d5dbd55e37 100644
@@ -464,7 +464,7 @@ static int check_charging_duration(struct charger_manager *cm)
 		duration = curr - cm->charging_start_time;
 
 		if (duration > desc->charging_max_duration_ms) {
-			dev_info(cm->dev, "Charging duration exceed %ums\n",
+			dev_info(cm->dev, "Charging duration exceeds %ums\n",
 				 desc->charging_max_duration_ms);
 			ret = true;
 		}

drivers/power/supply/charger-manager.c static int check_charging_duration(struct charger_manager *cm) · line 479

Quoted 10 lines

drivers/power/supply/charger-manager.c +2 -2

@@ -472,8 +472,8 @@ static int check_charging_duration(struct charger_manager *cm)
 		duration = curr - cm->charging_end_time;
 
 		if (duration > desc->charging_max_duration_ms) {
-			dev_info(cm->dev, "Discharging duration exceed %ums\n",
-				 desc->discharging_max_duration_ms);
+			dev_info(cm->dev, "Charging duration exceeds %ums\n",
+				 desc->charging_max_duration_ms);
 			ret = true;
 		}
 	}

Hi,

It looks to me like the second block (else if) should be about discharging, not charging, more like Colin King's patch had it:

https://lore.kernel.org/lkml/[email protected]/

but I don't know this code.

Signature · 1 line

~Randy

Gustavo A. R. Silva

author

2020-09-02 16:37 UTC #2 lore raw

Quoted 48 lines · Randy Dunlap

Quoted 34 lines · Gustavo A. R. Silva
A few months ago, commit e132fc6bb89b ("power: supply: charger-manager: Make decisions focussed on battery status") changed the expression in the if statement from "duration > desc->discharging_max_duration_ms" to "duration > desc->charging_max_duration_ms", but the arguments for dev_info() were left unchanged. Apparently, due to a copy-paste error.
Fix this by using the proper arguments for dev_info().
Also, while there, replace "exceed" with "exceeds", for both messages.
Addresses-Coverity-ID: 1496803 ("Copy-paste error")
Fixes e132fc6bb89b ("power: supply: charger-manager: Make decisions focussed on battery status")
Signed-off-by Gustavo A. R. Silva <[email protected]>
---
Changes in v2:
- Replace "exceed" with "exceeds"
 drivers/power/supply/charger-manager.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/power/supply/charger-manager.c +1 -1
index 07992821e252..a6d5dbd55e37 100644
@@ -464,7 +464,7 @@ static int check_charging_duration(struct charger_manager *cm)
 		duration = curr - cm->charging_start_time;
 
 		if (duration > desc->charging_max_duration_ms) {
-			dev_info(cm->dev, "Charging duration exceed %ums\n",
+			dev_info(cm->dev, "Charging duration exceeds %ums\n",
 				 desc->charging_max_duration_ms);
 			ret = true;
 		}
drivers/power/supply/charger-manager.c static int check_charging_duration(struct charger_manager *cm) · line 479
Quoted 10 lines
drivers/power/supply/charger-manager.c +2 -2
@@ -472,8 +472,8 @@ static int check_charging_duration(struct charger_manager *cm)
 		duration = curr - cm->charging_end_time;
 
 		if (duration > desc->charging_max_duration_ms) {
-			dev_info(cm->dev, "Discharging duration exceed %ums\n",
-				 desc->discharging_max_duration_ms);
+			dev_info(cm->dev, "Charging duration exceeds %ums\n",
+				 desc->charging_max_duration_ms);
 			ret = true;
 		}
 	}
Hi,

Quoted 3 lines

It looks to me like the second block (else if) should be about discharging, not charging, more like Colin King's patch had it:

I had the same impression for a moment, but what makes me think this is more about charging than discharging, is this line:

471 } else if (cm->battery_status == POWER_SUPPLY_STATUS_NOT_CHARGING) {

which was introduced by the same commit:

e132fc6bb89b ("power: supply: charger-manager: Make decisions focussed on battery status")

let's find out... :)

Thanks
--
Gustavo

Colin Ian King

2020-09-02 16:46 UTC #3 lore raw

Quoted 56 lines · Gustavo A. R. Silva

Quoted 48 lines · Randy Dunlap
Quoted 34 lines · Gustavo A. R. Silva
A few months ago, commit e132fc6bb89b ("power: supply: charger-manager: Make decisions focussed on battery status") changed the expression in the if statement from "duration > desc->discharging_max_duration_ms" to "duration > desc->charging_max_duration_ms", but the arguments for dev_info() were left unchanged. Apparently, due to a copy-paste error.
Fix this by using the proper arguments for dev_info().
Also, while there, replace "exceed" with "exceeds", for both messages.
Addresses-Coverity-ID: 1496803 ("Copy-paste error")
Fixes e132fc6bb89b ("power: supply: charger-manager: Make decisions focussed on battery status")
Signed-off-by Gustavo A. R. Silva <[email protected]>
---
Changes in v2:
- Replace "exceed" with "exceeds"
 drivers/power/supply/charger-manager.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/power/supply/charger-manager.c +1 -1
index 07992821e252..a6d5dbd55e37 100644
@@ -464,7 +464,7 @@ static int check_charging_duration(struct charger_manager *cm)
 		duration = curr - cm->charging_start_time;
 
 		if (duration > desc->charging_max_duration_ms) {
-			dev_info(cm->dev, "Charging duration exceed %ums\n",
+			dev_info(cm->dev, "Charging duration exceeds %ums\n",
 				 desc->charging_max_duration_ms);
 			ret = true;
 		}
drivers/power/supply/charger-manager.c static int check_charging_duration(struct charger_manager *cm) · line 479
Quoted 10 lines
drivers/power/supply/charger-manager.c +2 -2
@@ -472,8 +472,8 @@ static int check_charging_duration(struct charger_manager *cm)
 		duration = curr - cm->charging_end_time;
 
 		if (duration > desc->charging_max_duration_ms) {
-			dev_info(cm->dev, "Discharging duration exceed %ums\n",
-				 desc->discharging_max_duration_ms);
+			dev_info(cm->dev, "Charging duration exceeds %ums\n",
+				 desc->charging_max_duration_ms);
 			ret = true;
 		}
 	}
Hi,
Quoted 3 lines
It looks to me like the second block (else if) should be about discharging, not charging, more like Colin King's patch had it:
I had the same impression for a moment, but what makes me think this is more about charging than discharging, is this line:

Quoted 7 lines

471 } else if (cm->battery_status == POWER_SUPPLY_STATUS_NOT_CHARGING) {
which was introduced by the same commit:
e132fc6bb89b ("power: supply: charger-manager: Make decisions focussed on battery status")
let's find out... :)

It's a 50/50 bet :-)

Quoted 5 lines

Thanks
--
Gustavo

Jonathan Bakker

2020-09-03 01:22 UTC #4 lore raw

Quoted 59 lines · Colin Ian King

Quoted 58 lines · Gustavo A. R. Silva
Quoted 48 lines · Randy Dunlap
Quoted 34 lines · Gustavo A. R. Silva
A few months ago, commit e132fc6bb89b ("power: supply: charger-manager: Make decisions focussed on battery status") changed the expression in the if statement from "duration > desc->discharging_max_duration_ms" to "duration > desc->charging_max_duration_ms", but the arguments for dev_info() were left unchanged. Apparently, due to a copy-paste error.
Fix this by using the proper arguments for dev_info().
Also, while there, replace "exceed" with "exceeds", for both messages.
Addresses-Coverity-ID: 1496803 ("Copy-paste error")
Fixes e132fc6bb89b ("power: supply: charger-manager: Make decisions focussed on battery status")
Signed-off-by Gustavo A. R. Silva <[email protected]>
---
Changes in v2:
- Replace "exceed" with "exceeds"
 drivers/power/supply/charger-manager.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/power/supply/charger-manager.c +1 -1
index 07992821e252..a6d5dbd55e37 100644
@@ -464,7 +464,7 @@ static int check_charging_duration(struct charger_manager *cm)
 		duration = curr - cm->charging_start_time;
 
 		if (duration > desc->charging_max_duration_ms) {
-			dev_info(cm->dev, "Charging duration exceed %ums\n",
+			dev_info(cm->dev, "Charging duration exceeds %ums\n",
 				 desc->charging_max_duration_ms);
 			ret = true;
 		}
drivers/power/supply/charger-manager.c static int check_charging_duration(struct charger_manager *cm) · line 479
Quoted 10 lines
drivers/power/supply/charger-manager.c +2 -2
@@ -472,8 +472,8 @@ static int check_charging_duration(struct charger_manager *cm)
 		duration = curr - cm->charging_end_time;
 
 		if (duration > desc->charging_max_duration_ms) {
-			dev_info(cm->dev, "Discharging duration exceed %ums\n",
-				 desc->discharging_max_duration_ms);
+			dev_info(cm->dev, "Charging duration exceeds %ums\n",
+				 desc->charging_max_duration_ms);
 			ret = true;
 		}
 	}
Hi,
Quoted 3 lines
It looks to me like the second block (else if) should be about discharging, not charging, more like Colin King's patch had it:
I had the same impression for a moment, but what makes me think this is more about charging than discharging, is this line:
471 } else if (cm->battery_status == POWER_SUPPLY_STATUS_NOT_CHARGING) {

Quoted 7 lines

Quoted 5 lines
which was introduced by the same commit:
e132fc6bb89b ("power: supply: charger-manager: Make decisions focussed on battery status")
let's find out... :)
It's a 50/50 bet :-)

I believe it should be as Colin King's patch had it, ie

- if (duration > desc->charging_max_duration_ms) {
+ if (duration > desc->discharging_max_duration_ms) {

The battery_status as POWER_SUPPLY_STATUS_NOT_CHARGING only occurs when we would be charging, except that we're above or below the allowable temperature readings. This retains the logic of prior to e132fc6bb89b ("power: supply: charger-manager: Make decisions focussed on battery status")

Plus, this is the only place discharging_max_duration_ms is actually used. It appears to be the time that the battery can discharge (while being plugged in but out of temperature range) before restarting charging is tried (which will probably then fail on the next monitor session due to being above temp).

Thanks,
Jonathan

Quoted 7 lines

Quoted 6 lines
Thanks
--
Gustavo