public inbox for linux-pm@vger.kernel.org
 help / color / mirror / Atom feed
* [bug report] power: supply: max77759: add charger driver
@ 2026-04-10 10:14 Dan Carpenter
  2026-04-15 19:55 ` Amit Sunil Dhamne
  0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2026-04-10 10:14 UTC (permalink / raw)
  To: Amit Sunil Dhamne; +Cc: linux-pm

Hello Amit Sunil Dhamne,

Commit 70d7dd27f6dc ("power: supply: max77759: add charger driver")
from Mar 25, 2026 (linux-next), leads to the following Smatch static
checker warning:

	drivers/power/supply/max77759_charger.c:117 get_online()
	info: return a literal instead of 'ret'

drivers/power/supply/max77759_charger.c
    97  static int charger_input_valid(struct max77759_charger *chg)
    98  {
    99          u32 val;
   100          int ret;
   101  
   102          ret = regmap_read(chg->regmap, MAX77759_CHGR_REG_CHG_INT_OK, &val);
   103          if (ret)
   104                  return ret;
   105  
   106          return (val & MAX77759_CHGR_REG_CHG_INT_CHG) &&
   107                  (val & MAX77759_CHGR_REG_CHG_INT_CHGIN);
   108  }
   109  
   110  static int get_online(struct max77759_charger *chg)
   111  {
   112          u32 val;
   113          int ret;
   114  
   115          ret = charger_input_valid(chg);
   116          if (ret <= 0)
   117                  return ret;

This needs some comments.  From the naming, we would assume
charger_input_valid() returns true for valid and false for invalid.

Based on reading the code get_online() return true/false as well
but what does it mean?  false means offline and true means online?
In which sense is this a get_ function?  I'm so confused.

   118  
   119          ret = regmap_read(chg->regmap, MAX77759_CHGR_REG_CHG_DETAILS_02, &val);
   120          if (ret)
   121                  return ret;
   122  
   123          guard(mutex)(&chg->lock);
   124  
   125          return (val & MAX77759_CHGR_REG_CHG_DETAILS_02_CHGIN_STS) &&
   126                  (chg->mode == MAX77759_CHGR_MODE_CHG_BUCK_ON);
   127  }

This email is a free service from the Smatch-CI project [smatch.sf.net].

regards,
dan carpenter

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

* Re: [bug report] power: supply: max77759: add charger driver
  2026-04-10 10:14 [bug report] power: supply: max77759: add charger driver Dan Carpenter
@ 2026-04-15 19:55 ` Amit Sunil Dhamne
  0 siblings, 0 replies; 2+ messages in thread
From: Amit Sunil Dhamne @ 2026-04-15 19:55 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: linux-pm

Hi Dan,

Thanks for running the static checker.

On 4/10/26 3:14 AM, Dan Carpenter wrote:
> Hello Amit Sunil Dhamne,
>
> Commit 70d7dd27f6dc ("power: supply: max77759: add charger driver")
> from Mar 25, 2026 (linux-next), leads to the following Smatch static
> checker warning:
>
> 	drivers/power/supply/max77759_charger.c:117 get_online()
> 	info: return a literal instead of 'ret'
>
> drivers/power/supply/max77759_charger.c
>      97  static int charger_input_valid(struct max77759_charger *chg)
>      98  {
>      99          u32 val;
>     100          int ret;
>     101
>     102          ret = regmap_read(chg->regmap, MAX77759_CHGR_REG_CHG_INT_OK, &val);
>     103          if (ret)
>     104                  return ret;
>     105
>     106          return (val & MAX77759_CHGR_REG_CHG_INT_CHG) &&
>     107                  (val & MAX77759_CHGR_REG_CHG_INT_CHGIN);
>     108  }
>     109
>     110  static int get_online(struct max77759_charger *chg)
>     111  {
>     112          u32 val;
>     113          int ret;
>     114
>     115          ret = charger_input_valid(chg);
>     116          if (ret <= 0)
>     117                  return ret;
>
> This needs some comments.  From the naming, we would assume

Sure, I can add a comment.


> charger_input_valid() returns true for valid and false for invalid.
>
> Based on reading the code get_online() return true/false as well
> but what does it mean?  false means offline and true means online?
> In which sense is this a get_ function?  I'm so confused.

You're correct, a value of 0 would mean offline, 1 would mean online and 
a failure to read the hardware register would cause it to return with 
the appropriate error code.

This is a get_ function because it is a internal helper function to get 
the "POWER_SUPPLY_PROP_ONLINE" property.


BR,

Amit

>
>     118
>     119          ret = regmap_read(chg->regmap, MAX77759_CHGR_REG_CHG_DETAILS_02, &val);
>     120          if (ret)
>     121                  return ret;
>     122
>     123          guard(mutex)(&chg->lock);
>     124
>     125          return (val & MAX77759_CHGR_REG_CHG_DETAILS_02_CHGIN_STS) &&
>     126                  (chg->mode == MAX77759_CHGR_MODE_CHG_BUCK_ON);
>     127  }
>
> This email is a free service from the Smatch-CI project [smatch.sf.net].
>
> regards,
> dan carpenter

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

end of thread, other threads:[~2026-04-15 19:55 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-10 10:14 [bug report] power: supply: max77759: add charger driver Dan Carpenter
2026-04-15 19:55 ` Amit Sunil Dhamne

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