Re: [PATCH V6 3/5] power: supply: bq257xx: Add support for BQ257XX charger

[Dan Carpenter <dan.carpenter@linaro.org>]

On Mon, Aug 18, 2025 at 11:43:00AM -0500, Chris Morgan wrote:
> On Mon, Aug 18, 2025 at 10:26:06AM -0500, Chris Morgan wrote:
> > On Mon, Aug 18, 2025 at 11:22:35AM +0300, Dan Carpenter wrote:
> > > Hi Chris,
> > > 
> > > kernel test robot noticed the following build warnings:
> > > 
> > > https://git-scm.com/docs/git-format-patch#_base_tree_information]
> > > 
> > > url:    https://github.com/intel-lab-lkp/linux/commits/Chris-Morgan/dt-bindings-mfd-ti-bq25703a-Add-TI-BQ25703A-Charger/20250813-054704
> > > base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next
> > > patch link:    https://lore.kernel.org/r/20250812214300.123129-4-macroalpha82%40gmail.com
> > > patch subject: [PATCH V6 3/5] power: supply: bq257xx: Add support for BQ257XX charger
> > > config: parisc-randconfig-r072-20250818 (https://download.01.org/0day-ci/archive/20250818/202508181503.GrRD2T4C-lkp@intel.com/config)
> > > compiler: hppa-linux-gcc (GCC) 8.5.0
> > > 
> > > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > > the same patch/commit), kindly add following tags
> > > | Reported-by: kernel test robot <lkp@intel.com>
> > > | Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> > > | Closes: https://lore.kernel.org/r/202508181503.GrRD2T4C-lkp@intel.com/
> > > 
> > > smatch warnings:
> > > drivers/power/supply/bq257xx_charger.c:392 bq25703_hw_init() warn: potential ! vs ~ typo
> > > 
> > > vim +392 drivers/power/supply/bq257xx_charger.c
> > > 
> > > 7f3b6f1e51a925 Chris Morgan 2025-08-12  365  static int bq25703_hw_init(struct bq257xx_chg *pdata)
> > > 7f3b6f1e51a925 Chris Morgan 2025-08-12  366  {
> > > 7f3b6f1e51a925 Chris Morgan 2025-08-12  367  	struct regmap *regmap = pdata->bq->regmap;
> > > 7f3b6f1e51a925 Chris Morgan 2025-08-12  368  	int ret = 0;
> > > 7f3b6f1e51a925 Chris Morgan 2025-08-12  369  
> > > 7f3b6f1e51a925 Chris Morgan 2025-08-12  370  	regmap_update_bits(regmap, BQ25703_CHARGE_OPTION_0,
> > > 7f3b6f1e51a925 Chris Morgan 2025-08-12  371  			   BQ25703_WDTMR_ADJ_MASK,
> > > 7f3b6f1e51a925 Chris Morgan 2025-08-12  372  			   FIELD_PREP(BQ25703_WDTMR_ADJ_MASK,
> > > 7f3b6f1e51a925 Chris Morgan 2025-08-12  373  			   BQ25703_WDTMR_DISABLE));
> > > 7f3b6f1e51a925 Chris Morgan 2025-08-12  374  
> > > 7f3b6f1e51a925 Chris Morgan 2025-08-12  375  	ret = pdata->chip->bq257xx_set_ichg(pdata, pdata->ichg_max);
> > > 7f3b6f1e51a925 Chris Morgan 2025-08-12  376  	if (ret)
> > > 7f3b6f1e51a925 Chris Morgan 2025-08-12  377  		return ret;
> > > 7f3b6f1e51a925 Chris Morgan 2025-08-12  378  
> > > 7f3b6f1e51a925 Chris Morgan 2025-08-12  379  	ret = pdata->chip->bq257xx_set_vbatreg(pdata, pdata->vbat_max);
> > > 7f3b6f1e51a925 Chris Morgan 2025-08-12  380  	if (ret)
> > > 7f3b6f1e51a925 Chris Morgan 2025-08-12  381  		return ret;
> > > 7f3b6f1e51a925 Chris Morgan 2025-08-12  382  
> > > 7f3b6f1e51a925 Chris Morgan 2025-08-12  383  	ret = bq25703_set_min_vsys(pdata, pdata->vsys_min);
> > > 7f3b6f1e51a925 Chris Morgan 2025-08-12  384  	if (ret)
> > > 7f3b6f1e51a925 Chris Morgan 2025-08-12  385  		return ret;
> > > 7f3b6f1e51a925 Chris Morgan 2025-08-12  386  
> > > 7f3b6f1e51a925 Chris Morgan 2025-08-12  387  	ret = pdata->chip->bq257xx_set_iindpm(pdata, pdata->iindpm_max);
> > > 7f3b6f1e51a925 Chris Morgan 2025-08-12  388  	if (ret)
> > > 7f3b6f1e51a925 Chris Morgan 2025-08-12  389  		return ret;
> > > 7f3b6f1e51a925 Chris Morgan 2025-08-12  390  
> > > 7f3b6f1e51a925 Chris Morgan 2025-08-12  391  	regmap_update_bits(regmap, BQ25703_CHARGE_OPTION_0,
> > > 7f3b6f1e51a925 Chris Morgan 2025-08-12 @392  			   BQ25703_EN_LWPWR, !BQ25703_EN_LWPWR);
> > > 
> > > Yeah.  This really looks like it should be bitwise negate ~ instead of
> > > logical negate !.
> > 
> > Since BQ25703_EN_LWPWR is defined as 1 and I want to write 0 here (to
> > disable low power mode) I was under the assumption either should work.
> > That said, I'll happily switch to the bitwise negate (~) and resubmit.
> 
> Sorry, hadn't drank my morning coffee yet before I sent the last
> message and should have attempted it first before replying. When I
> make the requested change I get a compile time error of "conversion
> from ‘long unsigned int’ to ‘unsigned int’ changes value from
> ‘18446744073709518847’ to ‘4294934527’". What I'm really trying to
> accomplish here is to simply write a 0 to turn off low power mode, and
> do so in a manner that's easy to understand. I think since a bitwise
> negate gives a compile error and a logical negate throws a smatch
> warning, the most sensible thing to do is simply express this as a 0
> instead of a defined value and write a comment why I'm doing that.
> 

Yeah...  Or you could do "#define BQ25703_DISABLE_LWPWR 0".

regards,
dan carpenter