[bug report] regulator: MAX8997/8966 support

public inbox for kernel-janitors@vger.kernel.org
 help / color / mirror / Atom feed

* [bug report] regulator: MAX8997/8966 support
@ 2017-04-28 14:11 ` Dan Carpenter
  2017-05-08  5:45   ` [PATCH] regulator: max8997/8966: fix charger cv voltage set bug MyungJoo Ham
  0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2017-04-28 14:11 UTC (permalink / raw)
  To: kernel-janitors

Hello MyungJoo Ham,

The patch bd6ca2cf50fb: "regulator: MAX8997/8966 support" from Mar
11, 2011, leads to the following static checker warning:

	drivers/regulator/max8997-regulator.c:435 max8997_set_voltage_charger_cv()
	warn: dead code because of 'max_uV >= 4000000' and 'max_uV < 4000000'

drivers/regulator/max8997-regulator.c
   411  static int max8997_set_voltage_charger_cv(struct regulator_dev *rdev,
   412                  int min_uV, int max_uV, unsigned *selector)
   413  {
   414          struct max8997_data *max8997 = rdev_get_drvdata(rdev);
   415          struct i2c_client *i2c = max8997->iodev->i2c;
   416          int rid = rdev_get_id(rdev);
   417          int lb, ub;
   418          int reg, shift = 0, mask, ret = 0;
   419          u8 val = 0x0;
   420  
   421          if (rid != MAX8997_CHARGER_CV)
   422                  return -EINVAL;
   423  
   424          ret = max8997_get_voltage_register(rdev, &reg, &shift, &mask);
   425          if (ret)
   426                  return ret;
   427  
   428          if (max_uV < 4000000 || min_uV > 4350000)
                    ^^^^^^^^^^^^^^^^
We know that max_uV >= 4000000 because of this check.

   429                  return -EINVAL;
   430  
   431          if (min_uV <= 4000000) {
   432                  if (max_uV >= 4000000)
                            ^^^^^^^^^^^^^^^^^
So this check is redundent, we will always return -EINVAL here.

   433                          return -EINVAL;
   434                  else
   435                          val = 0x1;
                                ^^^^^^^^^
Dead code.

   436          } else if (min_uV <= 4200000 && max_uV >= 4200000)
   437                  val = 0x0;
   438          else {
   439                  lb = (min_uV - 4000001) / 20000 + 2;
   440                  ub = (max_uV - 4000000) / 20000 + 1;
   441  
   442                  if (lb > ub)
   443                          return -EINVAL;
   444  
   445                  if (lb < 0xf)
   446                          val = lb;
   447                  else {
   448                          if (ub >= 0xf)
   449                                  val = 0xf;
   450                          else
   451                                  return -EINVAL;
   452                  }
   453          }

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] regulator: max8997/8966: fix charger cv voltage set bug
  2017-04-28 14:11 ` [bug report] regulator: MAX8997/8966 support Dan Carpenter
@ 2017-05-08  5:45   ` MyungJoo Ham
  2017-05-08  9:14     ` Bartlomiej Zolnierkiewicz
                       ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: MyungJoo Ham @ 2017-05-08  5:45 UTC (permalink / raw)
  To: Dan Carpenter, lgirdwood@gmail.com, broonie@kernel.org,
	Chanwoo Choi, krzk@kernel.org, Bartlomiej Zolnierkiewicz
  Cc: kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org


When min charger-CV is <= 4.0V and max charger-CV is >= 4.0V,
we can use 4.00V as CV (register value = 0x1).`

The original code had a typo that wrote ">=" (max_uV >= 4000000),
which should've been "<", which is not necessary anyway
as mentioned by Dan Carpenter.

Reported-By: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
---
 drivers/regulator/max8997-regulator.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/regulator/max8997-regulator.c b/drivers/regulator/max8997-regulator.c
index efabc0e..559b9ac 100644
--- a/drivers/regulator/max8997-regulator.c
+++ b/drivers/regulator/max8997-regulator.c
@@ -428,12 +428,9 @@ static int max8997_set_voltage_charger_cv(struct regulator_dev *rdev,
 	if (max_uV < 4000000 || min_uV > 4350000)
 		return -EINVAL;
 
-	if (min_uV <= 4000000) {
-		if (max_uV >= 4000000)
-			return -EINVAL;
-		else
-			val = 0x1;
-	} else if (min_uV <= 4200000 && max_uV >= 4200000)
+	if (min_uV <= 4000000)
+		val = 0x1;
+	else if (min_uV <= 4200000 && max_uV >= 4200000)
 		val = 0x0;
 	else {
 		lb = (min_uV - 4000001) / 20000 + 2;
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] regulator: max8997/8966: fix charger cv voltage set bug
  2017-05-08  5:45   ` [PATCH] regulator: max8997/8966: fix charger cv voltage set bug MyungJoo Ham
@ 2017-05-08  9:14     ` Bartlomiej Zolnierkiewicz
  2017-05-09 23:33     ` Chanwoo Choi
  2017-05-15  8:04     ` Applied "regulator: max8997/8966: fix charger cv voltage set bug" to the regulator tree Mark Brown
  2 siblings, 0 replies; 5+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2017-05-08  9:14 UTC (permalink / raw)
  To: myungjoo.ham
  Cc: Dan Carpenter, lgirdwood@gmail.com, broonie@kernel.org,
	Chanwoo Choi, krzk@kernel.org, kernel-janitors@vger.kernel.org,
	linux-kernel@vger.kernel.org

On Monday, May 08, 2017 05:45:44 AM MyungJoo Ham wrote:
> 
> When min charger-CV is <= 4.0V and max charger-CV is >= 4.0V,
> we can use 4.00V as CV (register value = 0x1).`
> 
> The original code had a typo that wrote ">=" (max_uV >= 4000000),
> which should've been "<", which is not necessary anyway
> as mentioned by Dan Carpenter.
> 
> Reported-By: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>

Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] regulator: max8997/8966: fix charger cv voltage set bug
  2017-05-08  5:45   ` [PATCH] regulator: max8997/8966: fix charger cv voltage set bug MyungJoo Ham
  2017-05-08  9:14     ` Bartlomiej Zolnierkiewicz
@ 2017-05-09 23:33     ` Chanwoo Choi
  2017-05-15  8:04     ` Applied "regulator: max8997/8966: fix charger cv voltage set bug" to the regulator tree Mark Brown
  2 siblings, 0 replies; 5+ messages in thread
From: Chanwoo Choi @ 2017-05-09 23:33 UTC (permalink / raw)
  To: myungjoo.ham, Dan Carpenter, lgirdwood@gmail.com,
	broonie@kernel.org, krzk@kernel.org, Bartlomiej Zolnierkiewicz
  Cc: kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org

Hi,

On 2017년 05월 08일 14:45, MyungJoo Ham wrote:
> 
> When min charger-CV is <= 4.0V and max charger-CV is >= 4.0V,
> we can use 4.00V as CV (register value = 0x1).`
> 
> The original code had a typo that wrote ">=" (max_uV >= 4000000),
> which should've been "<", which is not necessary anyway
> as mentioned by Dan Carpenter.
> 
> Reported-By: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> ---
>  drivers/regulator/max8997-regulator.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/regulator/max8997-regulator.c b/drivers/regulator/max8997-regulator.c
> index efabc0e..559b9ac 100644
> --- a/drivers/regulator/max8997-regulator.c
> +++ b/drivers/regulator/max8997-regulator.c
> @@ -428,12 +428,9 @@ static int max8997_set_voltage_charger_cv(struct regulator_dev *rdev,
>  	if (max_uV < 4000000 || min_uV > 4350000)
>  		return -EINVAL;
>  
> -	if (min_uV <= 4000000) {
> -		if (max_uV >= 4000000)
> -			return -EINVAL;
> -		else
> -			val = 0x1;
> -	} else if (min_uV <= 4200000 && max_uV >= 4200000)
> +	if (min_uV <= 4000000)
> +		val = 0x1;
> +	else if (min_uV <= 4200000 && max_uV >= 4200000)
>  		val = 0x0;
>  	else {
>  		lb = (min_uV - 4000001) / 20000 + 2;
> 

Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Applied "regulator: max8997/8966: fix charger cv voltage set bug" to the regulator tree
  2017-05-08  5:45   ` [PATCH] regulator: max8997/8966: fix charger cv voltage set bug MyungJoo Ham
  2017-05-08  9:14     ` Bartlomiej Zolnierkiewicz
  2017-05-09 23:33     ` Chanwoo Choi
@ 2017-05-15  8:04     ` Mark Brown
  2 siblings, 0 replies; 5+ messages in thread
From: Mark Brown @ 2017-05-15  8:04 UTC (permalink / raw)
  To: MyungJoo Ham; +Cc: Mark Brown, Dan Carpenter, lgirdwood@gmail.com

The patch

   regulator: max8997/8966: fix charger cv voltage set bug

has been applied to the regulator tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

From f74521ca578f38daa3e800efde7fdb2ac3ba76ef Mon Sep 17 00:00:00 2001
From: MyungJoo Ham <myungjoo.ham@samsung.com>
Date: Mon, 8 May 2017 05:45:44 +0000
Subject: [PATCH] regulator: max8997/8966: fix charger cv voltage set bug

When min charger-CV is <= 4.0V and max charger-CV is >= 4.0V,
we can use 4.00V as CV (register value = 0x1).`

The original code had a typo that wrote ">=" (max_uV >= 4000000),
which should've been "<", which is not necessary anyway
as mentioned by Dan Carpenter.

Reported-By: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/regulator/max8997-regulator.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/regulator/max8997-regulator.c b/drivers/regulator/max8997-regulator.c
index efabc0ea0e96..559b9ac45404 100644
--- a/drivers/regulator/max8997-regulator.c
+++ b/drivers/regulator/max8997-regulator.c
@@ -428,12 +428,9 @@ static int max8997_set_voltage_charger_cv(struct regulator_dev *rdev,
 	if (max_uV < 4000000 || min_uV > 4350000)
 		return -EINVAL;
 
-	if (min_uV <= 4000000) {
-		if (max_uV >= 4000000)
-			return -EINVAL;
-		else
-			val = 0x1;
-	} else if (min_uV <= 4200000 && max_uV >= 4200000)
+	if (min_uV <= 4000000)
+		val = 0x1;
+	else if (min_uV <= 4200000 && max_uV >= 4200000)
 		val = 0x0;
 	else {
 		lb = (min_uV - 4000001) / 20000 + 2;
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-05-15  8:04 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <CGME20170428141118epcas2p42c864cf574cc6fa3ffedbef22029e3ef@epcms1p6>
2017-04-28 14:11 ` [bug report] regulator: MAX8997/8966 support Dan Carpenter
2017-05-08  5:45   ` [PATCH] regulator: max8997/8966: fix charger cv voltage set bug MyungJoo Ham
2017-05-08  9:14     ` Bartlomiej Zolnierkiewicz
2017-05-09 23:33     ` Chanwoo Choi
2017-05-15  8:04     ` Applied "regulator: max8997/8966: fix charger cv voltage set bug" to the regulator tree Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox