Re: [EXTERNAL] Re: [PATCH v8 2/2] power: supply: bq256xx: Introduce the BQ256XX charger driver
Sebastian, On 1/5/21 8:18 AM, Sebastian Reichel wrote: Hi Ricardo, On Tue, Jan 05, 2021 at 11:24:18AM +0800, kernel test robot wrote: Hi Ricardo, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on power-supply/for-next] [also build test WARNING on robh/for-next v5.11-rc2 next-20210104] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Ricardo-Rivera-Matos/Introduce-the-BQ256XX-family-of-chargers/20210105-043028 base: https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next config: powerpc64-randconfig-r034-20210105 (attached as .config) compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 5c951623bc8965fa1e89660f2f5f4a2944e4981a) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install powerpc64 cross compiling tool for clang build # apt-get install binutils-powerpc64-linux-gnu # https://github.com/0day-ci/linux/commit/82436c2c6d99c4effb187bbd09b47c4dc59a1f3d git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Ricardo-Rivera-Matos/Introduce-the-BQ256XX-family-of-chargers/20210105-043028 git checkout 82436c2c6d99c4effb187bbd09b47c4dc59a1f3d # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=powerpc64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All warnings (new ones prefixed by >>): drivers/power/supply/bq256xx_charger.c:1644:29: warning: variable 'psy_cfg' is uninitialized when used here [-Wuninitialized] ret = bq256xx_parse_dt(bq, psy_cfg, dev); ^~~ drivers/power/supply/bq256xx_charger.c:1618:37: note: initialize the variable 'psy_cfg' to silence this warning struct power_supply_config *psy_cfg; ^ = NULL bah, I missed this serious issue during review :( FWIW the compiler provided wrong solution. It would result in dereferencing a NULL pointer afterwards since you never allocate any memory for psy_cfg. You could of course allocate memory for it, but power_supply_config is only needed during device registration. Proper solution is to initialize it as variable instead of pointer, so that it ends up on the stack. Also you should initialize it with {} to make sure any fields not explicitly configured are 0. ACK drivers/power/supply/bq256xx_charger.c:1720:36: warning: unused variable 'bq256xx_acpi_match' [-Wunused-const-variable] static const struct acpi_device_id bq256xx_acpi_match[] = { For this one just change .acpi_match_table = ACPI_PTR(bq256xx_acpi_match), into .acpi_match_table = bq256xx_acpi_match, ACK 2 warnings generated. vim +/bq256xx_acpi_match +1720 drivers/power/supply/bq256xx_charger.c 1719 1720static const struct acpi_device_id bq256xx_acpi_match[] = { 1721 { "bq25600", BQ25600 }, 1722 { "bq25600d", BQ25600D }, 1723 { "bq25601", BQ25601 }, 1724 { "bq25601d", BQ25601D }, 1725 { "bq25611d", BQ25611D }, 1726 { "bq25618", BQ25618 }, 1727 { "bq25619", BQ25619 }, 1728 {}, 1729 }; 1730 MODULE_DEVICE_TABLE(acpi, bq256xx_acpi_match); 1731 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org Thanks, -- Sebastian Thanks for the feedback on this! Ricardo
Re: [PATCH v8 2/2] power: supply: bq256xx: Introduce the BQ256XX charger driver
Hi Ricardo, On Tue, Jan 05, 2021 at 11:24:18AM +0800, kernel test robot wrote: > Hi Ricardo, > > Thank you for the patch! Perhaps something to improve: > > [auto build test WARNING on power-supply/for-next] > [also build test WARNING on robh/for-next v5.11-rc2 next-20210104] > [If your patch is applied to the wrong git tree, kindly drop us a note. > And when submitting patch, we suggest to use '--base' as documented in > https://git-scm.com/docs/git-format-patch] > > url: > https://github.com/0day-ci/linux/commits/Ricardo-Rivera-Matos/Introduce-the-BQ256XX-family-of-chargers/20210105-043028 > base: > https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git > for-next > config: powerpc64-randconfig-r034-20210105 (attached as .config) > compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project > 5c951623bc8965fa1e89660f2f5f4a2944e4981a) > reproduce (this is a W=1 build): > wget > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O > ~/bin/make.cross > chmod +x ~/bin/make.cross > # install powerpc64 cross compiling tool for clang build > # apt-get install binutils-powerpc64-linux-gnu > # > https://github.com/0day-ci/linux/commit/82436c2c6d99c4effb187bbd09b47c4dc59a1f3d > git remote add linux-review https://github.com/0day-ci/linux > git fetch --no-tags linux-review > Ricardo-Rivera-Matos/Introduce-the-BQ256XX-family-of-chargers/20210105-043028 > git checkout 82436c2c6d99c4effb187bbd09b47c4dc59a1f3d > # save the attached .config to linux build tree > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross > ARCH=powerpc64 > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot > > All warnings (new ones prefixed by >>): > >drivers/power/supply/bq256xx_charger.c:1644:29: warning: variable > 'psy_cfg' is uninitialized when used here [-Wuninitialized] >ret = bq256xx_parse_dt(bq, psy_cfg, dev); > ^~~ >drivers/power/supply/bq256xx_charger.c:1618:37: note: initialize the > variable 'psy_cfg' to silence this warning >struct power_supply_config *psy_cfg; > ^ >= NULL bah, I missed this serious issue during review :( FWIW the compiler provided wrong solution. It would result in dereferencing a NULL pointer afterwards since you never allocate any memory for psy_cfg. You could of course allocate memory for it, but power_supply_config is only needed during device registration. Proper solution is to initialize it as variable instead of pointer, so that it ends up on the stack. Also you should initialize it with {} to make sure any fields not explicitly configured are 0. > >> drivers/power/supply/bq256xx_charger.c:1720:36: warning: unused variable > >> 'bq256xx_acpi_match' [-Wunused-const-variable] >static const struct acpi_device_id bq256xx_acpi_match[] = { For this one just change .acpi_match_table = ACPI_PTR(bq256xx_acpi_match), into .acpi_match_table = bq256xx_acpi_match, >2 warnings generated. > > > vim +/bq256xx_acpi_match +1720 drivers/power/supply/bq256xx_charger.c > > 1719 > > 1720static const struct acpi_device_id bq256xx_acpi_match[] = { > 1721{ "bq25600", BQ25600 }, > 1722{ "bq25600d", BQ25600D }, > 1723{ "bq25601", BQ25601 }, > 1724{ "bq25601d", BQ25601D }, > 1725{ "bq25611d", BQ25611D }, > 1726{ "bq25618", BQ25618 }, > 1727{ "bq25619", BQ25619 }, > 1728{}, > 1729}; > 1730MODULE_DEVICE_TABLE(acpi, bq256xx_acpi_match); > 1731 > > --- > 0-DAY CI Kernel Test Service, Intel Corporation > https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org Thanks, -- Sebastian signature.asc Description: PGP signature
Re: [PATCH v8 2/2] power: supply: bq256xx: Introduce the BQ256XX charger driver
Hi Ricardo, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on power-supply/for-next] [also build test WARNING on robh/for-next v5.11-rc2 next-20210104] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Ricardo-Rivera-Matos/Introduce-the-BQ256XX-family-of-chargers/20210105-043028 base: https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next config: s390-randconfig-r005-20210105 (attached as .config) compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 5c951623bc8965fa1e89660f2f5f4a2944e4981a) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install s390 cross compiling tool for clang build # apt-get install binutils-s390x-linux-gnu # https://github.com/0day-ci/linux/commit/82436c2c6d99c4effb187bbd09b47c4dc59a1f3d git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Ricardo-Rivera-Matos/Introduce-the-BQ256XX-family-of-chargers/20210105-043028 git checkout 82436c2c6d99c4effb187bbd09b47c4dc59a1f3d # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=s390 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All warnings (new ones prefixed by >>): include/uapi/linux/swab.h:19:12: note: expanded from macro '___constant_swab32' (((__u32)(x) & (__u32)0x00ffUL) << 24) |\ ^ In file included from drivers/power/supply/bq256xx_charger.c:13: In file included from include/linux/regmap.h:20: In file included from include/linux/iopoll.h:14: In file included from include/linux/io.h:13: In file included from arch/s390/include/asm/io.h:80: include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr)); ~~ ^ include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu' #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x)) ^ include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32' ___constant_swab32(x) : \ ^ include/uapi/linux/swab.h:20:12: note: expanded from macro '___constant_swab32' (((__u32)(x) & (__u32)0xff00UL) << 8) |\ ^ In file included from drivers/power/supply/bq256xx_charger.c:13: In file included from include/linux/regmap.h:20: In file included from include/linux/iopoll.h:14: In file included from include/linux/io.h:13: In file included from arch/s390/include/asm/io.h:80: include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr)); ~~ ^ include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu' #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x)) ^ include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32' ___constant_swab32(x) : \ ^ include/uapi/linux/swab.h:21:12: note: expanded from macro '___constant_swab32' (((__u32)(x) & (__u32)0x00ffUL) >> 8) |\ ^ In file included from drivers/power/supply/bq256xx_charger.c:13: In file included from include/linux/regmap.h:20: In file included from include/linux/iopoll.h:14: In file included from include/linux/io.h:13: In file included from arch/s390/include/asm/io.h:80: include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr)); ~~ ^ include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu' #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x)) ^ include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32'
Re: [PATCH v8 2/2] power: supply: bq256xx: Introduce the BQ256XX charger driver
Hi Ricardo, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on power-supply/for-next] [also build test WARNING on robh/for-next v5.11-rc2 next-20210104] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Ricardo-Rivera-Matos/Introduce-the-BQ256XX-family-of-chargers/20210105-043028 base: https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next config: powerpc64-randconfig-r034-20210105 (attached as .config) compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 5c951623bc8965fa1e89660f2f5f4a2944e4981a) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install powerpc64 cross compiling tool for clang build # apt-get install binutils-powerpc64-linux-gnu # https://github.com/0day-ci/linux/commit/82436c2c6d99c4effb187bbd09b47c4dc59a1f3d git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Ricardo-Rivera-Matos/Introduce-the-BQ256XX-family-of-chargers/20210105-043028 git checkout 82436c2c6d99c4effb187bbd09b47c4dc59a1f3d # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=powerpc64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All warnings (new ones prefixed by >>): drivers/power/supply/bq256xx_charger.c:1644:29: warning: variable 'psy_cfg' is uninitialized when used here [-Wuninitialized] ret = bq256xx_parse_dt(bq, psy_cfg, dev); ^~~ drivers/power/supply/bq256xx_charger.c:1618:37: note: initialize the variable 'psy_cfg' to silence this warning struct power_supply_config *psy_cfg; ^ = NULL >> drivers/power/supply/bq256xx_charger.c:1720:36: warning: unused variable >> 'bq256xx_acpi_match' [-Wunused-const-variable] static const struct acpi_device_id bq256xx_acpi_match[] = { ^ 2 warnings generated. vim +/bq256xx_acpi_match +1720 drivers/power/supply/bq256xx_charger.c 1719 > 1720 static const struct acpi_device_id bq256xx_acpi_match[] = { 1721 { "bq25600", BQ25600 }, 1722 { "bq25600d", BQ25600D }, 1723 { "bq25601", BQ25601 }, 1724 { "bq25601d", BQ25601D }, 1725 { "bq25611d", BQ25611D }, 1726 { "bq25618", BQ25618 }, 1727 { "bq25619", BQ25619 }, 1728 {}, 1729 }; 1730 MODULE_DEVICE_TABLE(acpi, bq256xx_acpi_match); 1731 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip
[PATCH v8 2/2] power: supply: bq256xx: Introduce the BQ256XX charger driver
The BQ256XX family of devices are highly integrated buck chargers for single cell batteries. Signed-off-by: Ricardo Rivera-Matos --- drivers/power/supply/Kconfig | 11 + drivers/power/supply/Makefile |1 + drivers/power/supply/bq256xx_charger.c | 1745 3 files changed, 1757 insertions(+) create mode 100644 drivers/power/supply/bq256xx_charger.c diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig index eec646c568b7..cedef501e683 100644 --- a/drivers/power/supply/Kconfig +++ b/drivers/power/supply/Kconfig @@ -645,6 +645,17 @@ config CHARGER_BQ25980 Say Y to enable support for the TI BQ25980, BQ25975 and BQ25960 series of fast battery chargers. +config CHARGER_BQ256XX + tristate "TI BQ256XX battery charger driver" + depends on I2C + depends on GPIOLIB || COMPILE_TEST + select REGMAP_I2C + help + Say Y to enable support for the TI BQ256XX battery chargers. The + BQ256XX family of devices are highly-integrated, switch-mode battery + charge management and system power path management devices for single + cell Li-ion and Li-polymer batteries. + config CHARGER_SMB347 tristate "Summit Microelectronics SMB3XX Battery Charger" depends on I2C diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile index dd4b86318cd9..ae322b1da1ed 100644 --- a/drivers/power/supply/Makefile +++ b/drivers/power/supply/Makefile @@ -85,6 +85,7 @@ obj-$(CONFIG_CHARGER_BQ24735) += bq24735-charger.o obj-$(CONFIG_CHARGER_BQ2515X) += bq2515x_charger.o obj-$(CONFIG_CHARGER_BQ25890) += bq25890_charger.o obj-$(CONFIG_CHARGER_BQ25980) += bq25980_charger.o +obj-$(CONFIG_CHARGER_BQ256XX) += bq256xx_charger.o obj-$(CONFIG_CHARGER_SMB347) += smb347-charger.o obj-$(CONFIG_CHARGER_TPS65090) += tps65090-charger.o obj-$(CONFIG_CHARGER_TPS65217) += tps65217_charger.o diff --git a/drivers/power/supply/bq256xx_charger.c b/drivers/power/supply/bq256xx_charger.c new file mode 100644 index ..a6aa0ab2315f --- /dev/null +++ b/drivers/power/supply/bq256xx_charger.c @@ -0,0 +1,1745 @@ +// SPDX-License-Identifier: GPL-2.0 +// BQ256XX Battery Charger Driver +// Copyright (C) 2020 Texas Instruments Incorporated - http://www.ti.com/ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define BQ256XX_MANUFACTURER "Texas Instruments" + +#define BQ256XX_INPUT_CURRENT_LIMIT0x00 +#define BQ256XX_CHARGER_CONTROL_0 0x01 +#define BQ256XX_CHARGE_CURRENT_LIMIT 0x02 +#define BQ256XX_PRECHG_AND_TERM_CURR_LIM 0x03 +#define BQ256XX_BATTERY_VOLTAGE_LIMIT 0x04 +#define BQ256XX_CHARGER_CONTROL_1 0x05 +#define BQ256XX_CHARGER_CONTROL_2 0x06 +#define BQ256XX_CHARGER_CONTROL_3 0x07 +#define BQ256XX_CHARGER_STATUS_0 0x08 +#define BQ256XX_CHARGER_STATUS_1 0x09 +#define BQ256XX_CHARGER_STATUS_2 0x0a +#define BQ256XX_PART_INFORMATION 0x0b +#define BQ256XX_CHARGER_CONTROL_4 0x0c + +#define BQ256XX_IINDPM_MASKGENMASK(4, 0) +#define BQ256XX_IINDPM_STEP_uA 10 +#define BQ256XX_IINDPM_OFFSET_uA 10 +#define BQ256XX_IINDPM_MIN_uA 10 +#define BQ256XX_IINDPM_MAX_uA 320 +#define BQ256XX_IINDPM_DEF_uA 240 + +#define BQ256XX_VINDPM_MASKGENMASK(3, 0) +#define BQ256XX_VINDPM_STEP_uV 10 +#define BQ256XX_VINDPM_OFFSET_uV 390 +#define BQ256XX_VINDPM_MIN_uV 390 +#define BQ256XX_VINDPM_MAX_uV 540 +#define BQ256XX_VINDPM_DEF_uV 450 + +#define BQ256XX_VBATREG_MASK GENMASK(7, 3) +#define BQ2560X_VBATREG_STEP_uV32000 +#define BQ2560X_VBATREG_OFFSET_uV 3856000 +#define BQ2560X_VBATREG_MIN_uV 3856000 +#define BQ2560X_VBATREG_MAX_uV 4624000 +#define BQ2560X_VBATREG_DEF_uV 4208000 +#define BQ25601D_VBATREG_OFFSET_uV 3847000 +#define BQ25601D_VBATREG_MIN_uV3847000 +#define BQ25601D_VBATREG_MAX_uV4615000 +#define BQ25601D_VBATREG_DEF_uV4199000 +#define BQ2561X_VBATREG_STEP_uV1 +#define BQ25611D_VBATREG_MIN_uV3494000 +#define BQ25611D_VBATREG_MAX_uV451 +#define BQ25611D_VBATREG_DEF_uV419 +#define BQ25618_VBATREG_MIN_uV 3504000 +#define BQ25618_VBATREG_MAX_uV 450 +#define BQ25618_VBATREG_DEF_uV 420 +#define BQ256XX_VBATREG_BIT_SHIFT 3 +#define BQ2561X_VBATREG_THRESH 0x8 +#define BQ25611D_VBATREG_THRESH_uV 429 +#define BQ25618_VBATREG_THRESH_uV 430 + +#define BQ256XX_ITERM_MASK GENMASK(3, 0) +#define BQ256XX_ITERM_STEP_uA 6