linux-kernel - Re: [PATCH] power: supply: 88pm860x_battery array

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1508996478.10651.28.camel@perches.com>
Date:   Wed, 25 Oct 2017 22:41:18 -0700
From:   Joe Perches <joe@...ches.com>
To:     "winton.liu" <18502523564@....com>, sre@...nel.org
Cc:     linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] power: supply: 88pm860x_battery array_soc first number
 is in mV

On Thu, 2017-10-26 at 13:21 +0800, winton.liu wrote:
> Fix wrong comments of array_soc description.
> First number is mV not mAh.
[]
> diff --git a/drivers/power/supply/88pm860x_battery.c b/drivers/power/supply/88pm860x_battery.c
[]
> @@ -123,7 +123,7 @@ struct ccnt {
>  
>  /*
>   * State of Charge.
> - * The first number is mAh(=3.6C), and the second number is percent point.
> + * The first number is mV, and the second number is percent point.
>   */
>  static int array_soc[][2] = {
>  	{4170, 100}, {4154, 99}, {4136, 98}, {4122, 97}, {4107, 96},

OK, but why not change the declaration to a struct
and make it obvious?

Also, the array or struct should be const.

Perhaps:
---
 drivers/power/supply/88pm860x_battery.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/power/supply/88pm860x_battery.c b/drivers/power/supply/88pm860x_battery.c
index 63c57dc82ac1..973c5f7b07ba 100644
--- a/drivers/power/supply/88pm860x_battery.c
+++ b/drivers/power/supply/88pm860x_battery.c
@@ -123,9 +123,12 @@ struct ccnt {
 
 /*
  * State of Charge.
- * The first number is mAh(=3.6C), and the second number is percent point.
+ * The first number is mV(=3.6C), and the second number is percent point.
  */
-static int array_soc[][2] = {
+static const struct {
+	u16 mv;
+	u8 percent;
+} array_soc[] = {
 	{4170, 100}, {4154, 99}, {4136, 98}, {4122, 97}, {4107, 96},
 	{4102, 95}, {4088, 94}, {4081, 93}, {4070, 92}, {4060, 91},
 	{4053, 90}, {4044, 89}, {4035, 88}, {4028, 87}, {4019, 86},
@@ -388,14 +391,14 @@ static int calc_soc(struct pm860x_battery_info *info, int state, int *soc)
 		return ret;
 
 	count = ARRAY_SIZE(array_soc);
-	if (ocv < array_soc[count - 1][0]) {
+	if (ocv < array_soc[count - 1].mv) {
 		*soc = 0;
 		return 0;
 	}
 
 	for (i = 0; i < count; i++) {
-		if (ocv >= array_soc[i][0]) {
-			*soc = array_soc[i][1];
+		if (ocv >= array_soc[i].mv) {
+			*soc = array_soc[i].percent;
 			break;
 		}
 	}