Messages in this thread | | | Date | Wed, 2 Aug 2023 14:19:26 +0100 | Subject | Re: drivers/power/supply/qcom_pmi8998_charger.c:565 smb2_status_change_work() error: uninitialized symbol 'usb_online'. | From | Caleb Connolly <> |
| |
On 28/07/2023 06:45, kernel test robot wrote: > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master > head: 57012c57536f8814dec92e74197ee96c3498d24e > commit: 8648aeb5d7b70e13264ff5f444f22081d37d4670 power: supply: add Qualcomm PMI8998 SMB2 Charger driver > config: arm-randconfig-m041-20230727 (https://download.01.org/0day-ci/archive/20230728/202307280638.556PrzIS-lkp@intel.com/config) > compiler: arm-linux-gnueabi-gcc (GCC) 12.3.0 > reproduce: (https://download.01.org/0day-ci/archive/20230728/202307280638.556PrzIS-lkp@intel.com/reproduce) > > 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 <error27@gmail.com> > | Closes: https://lore.kernel.org/r/202307280638.556PrzIS-lkp@intel.com/ > > smatch warnings: > drivers/power/supply/qcom_pmi8998_charger.c:565 smb2_status_change_work() error: uninitialized symbol 'usb_online'.
Hi, thanks for the report. > > vim +/usb_online +565 drivers/power/supply/qcom_pmi8998_charger.c > > 8648aeb5d7b70e Caleb Connolly 2023-05-26 556 static void smb2_status_change_work(struct work_struct *work) > 8648aeb5d7b70e Caleb Connolly 2023-05-26 557 { > 8648aeb5d7b70e Caleb Connolly 2023-05-26 558 unsigned int charger_type, current_ua; > 8648aeb5d7b70e Caleb Connolly 2023-05-26 559 int usb_online, count, rc; > 8648aeb5d7b70e Caleb Connolly 2023-05-26 560 struct smb2_chip *chip; > 8648aeb5d7b70e Caleb Connolly 2023-05-26 561 > 8648aeb5d7b70e Caleb Connolly 2023-05-26 562 chip = container_of(work, struct smb2_chip, status_change_work.work); > 8648aeb5d7b70e Caleb Connolly 2023-05-26 563 > 8648aeb5d7b70e Caleb Connolly 2023-05-26 564 smb2_get_prop_usb_online(chip, &usb_online); > > This can only happen if regmap_read() fails, and in real life they > can't actually fail can they? We can't really recover if regmap > breaks so in that situation this uninitialized variable would be the > least of our concerns. Right?
In this case, the driver is for a peripheral on the SPMI bus, a read failing is extremely unlikely but under some conditions like bandwidth constraints it could happen. Though admittedly there are likely bigger issues to deal with in that situation heh.
It's a trivial fix so I'll send a patch over. > > So what I could do is just delete the regmap_read error paths from > the DB. I just add these two lines to smatch_data/db/kernel.delete.return_states > > regmap_read (-22) > regmap_read (-4095)-(-1) > > 8648aeb5d7b70e Caleb Connolly 2023-05-26 @565 if (!usb_online) > ^^^^^^^^^^ > > 8648aeb5d7b70e Caleb Connolly 2023-05-26 566 return; > 8648aeb5d7b70e Caleb Connolly 2023-05-26 567 > 8648aeb5d7b70e Caleb Connolly 2023-05-26 568 for (count = 0; count < 3; count++) { > 8648aeb5d7b70e Caleb Connolly 2023-05-26 569 dev_dbg(chip->dev, "get charger type retry %d\n", count); >
-- // Caleb (they/them)
| |