Re: [PATCH v4 1/2] power: supply: add Qualcomm PMI8998 SMB2 Charger driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Caleb,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on sre-power-supply/for-next]
[also build test WARNING on robh/for-next linus/master v5.19-rc5 next-20220707]
[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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Caleb-Connolly/power-supply-introduce-support-for-the-Qualcomm-smb2-charger/20220707-034307
base:   https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next
config: arm64-allmodconfig (https://download.01.org/0day-ci/archive/20220707/202207072358.2odInqXi-lkp@xxxxxxxxx/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project f553287b588916de09c66e3e32bf75e5060f967f)
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 arm64 cross compiling tool for clang build
        # apt-get install binutils-aarch64-linux-gnu
        # https://github.com/intel-lab-lkp/linux/commit/a6b315467a158024bb1af7fed00c9a5227c9b293
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Caleb-Connolly/power-supply-introduce-support-for-the-Qualcomm-smb2-charger/20220707-034307
        git checkout a6b315467a158024bb1af7fed00c9a5227c9b293
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm64 SHELL=/bin/bash drivers/power/supply/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@xxxxxxxxx>

All warnings (new ones prefixed by >>):

>> drivers/power/supply/qcom_pmi8998_charger.c:425:5: warning: no previous prototype for function 'smb2_get_prop_usb_online' [-Wmissing-prototypes]
   int smb2_get_prop_usb_online(struct smb2_chip *chip, int *val)
       ^
   drivers/power/supply/qcom_pmi8998_charger.c:425:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int smb2_get_prop_usb_online(struct smb2_chip *chip, int *val)
   ^
   static 
>> drivers/power/supply/qcom_pmi8998_charger.c:486:5: warning: no previous prototype for function 'smb2_get_prop_status' [-Wmissing-prototypes]
   int smb2_get_prop_status(struct smb2_chip *chip, int *val)
       ^
   drivers/power/supply/qcom_pmi8998_charger.c:486:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int smb2_get_prop_status(struct smb2_chip *chip, int *val)
   ^
   static 
>> drivers/power/supply/qcom_pmi8998_charger.c:565:6: warning: no previous prototype for function 'smb2_status_change_work' [-Wmissing-prototypes]
   void smb2_status_change_work(struct work_struct *work)
        ^
   drivers/power/supply/qcom_pmi8998_charger.c:565:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   void smb2_status_change_work(struct work_struct *work)
   ^
   static 
>> drivers/power/supply/qcom_pmi8998_charger.c:614:5: warning: no previous prototype for function 'smb2_get_iio_chan' [-Wmissing-prototypes]
   int smb2_get_iio_chan(struct smb2_chip *chip, struct iio_channel *chan,
       ^
   drivers/power/supply/qcom_pmi8998_charger.c:614:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int smb2_get_iio_chan(struct smb2_chip *chip, struct iio_channel *chan,
   ^
   static 
>> drivers/power/supply/qcom_pmi8998_charger.c:635:5: warning: no previous prototype for function 'smb2_get_prop_health' [-Wmissing-prototypes]
   int smb2_get_prop_health(struct smb2_chip *chip, int *val)
       ^
   drivers/power/supply/qcom_pmi8998_charger.c:635:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int smb2_get_prop_health(struct smb2_chip *chip, int *val)
   ^
   static 
>> drivers/power/supply/qcom_pmi8998_charger.c:736:13: warning: no previous prototype for function 'smb2_handle_batt_overvoltage' [-Wmissing-prototypes]
   irqreturn_t smb2_handle_batt_overvoltage(int irq, void *data)
               ^
   drivers/power/supply/qcom_pmi8998_charger.c:736:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   irqreturn_t smb2_handle_batt_overvoltage(int irq, void *data)
   ^
   static 
>> drivers/power/supply/qcom_pmi8998_charger.c:754:13: warning: no previous prototype for function 'smb2_handle_usb_plugin' [-Wmissing-prototypes]
   irqreturn_t smb2_handle_usb_plugin(int irq, void *data)
               ^
   drivers/power/supply/qcom_pmi8998_charger.c:754:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   irqreturn_t smb2_handle_usb_plugin(int irq, void *data)
   ^
   static 
>> drivers/power/supply/qcom_pmi8998_charger.c:766:13: warning: no previous prototype for function 'smb2_handle_usb_icl_change' [-Wmissing-prototypes]
   irqreturn_t smb2_handle_usb_icl_change(int irq, void *data)
               ^
   drivers/power/supply/qcom_pmi8998_charger.c:766:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   irqreturn_t smb2_handle_usb_icl_change(int irq, void *data)
   ^
   static 
>> drivers/power/supply/qcom_pmi8998_charger.c:775:13: warning: no previous prototype for function 'smb2_handle_wdog_bark' [-Wmissing-prototypes]
   irqreturn_t smb2_handle_wdog_bark(int irq, void *data)
               ^
   drivers/power/supply/qcom_pmi8998_charger.c:775:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   irqreturn_t smb2_handle_wdog_bark(int irq, void *data)
   ^
   static 
   drivers/power/supply/qcom_pmi8998_charger.c:896:12: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides]
             .addr = 1950000 / 25000 },
                     ^~~~~~~~~~~~~~~
   drivers/power/supply/qcom_pmi8998_charger.c:894:12: note: previous initialization is here
           { .addr = FAST_CHARGE_CURRENT_CFG,
                     ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/power/supply/qcom_pmi8998_charger.c:76:36: note: expanded from macro 'FAST_CHARGE_CURRENT_CFG'
   #define FAST_CHARGE_CURRENT_CFG                         0x61
                                                           ^~~~
   10 warnings generated.


vim +/smb2_get_prop_usb_online +425 drivers/power/supply/qcom_pmi8998_charger.c

   424	
 > 425	int smb2_get_prop_usb_online(struct smb2_chip *chip, int *val)
   426	{
   427		unsigned int stat;
   428		int rc;
   429	
   430		rc = regmap_read(chip->regmap, chip->base + POWER_PATH_STATUS, &stat);
   431		if (rc < 0) {
   432			dev_err(chip->dev, "Couldn't read POWER_PATH_STATUS! ret=%d\n",
   433				rc);
   434			return rc;
   435		}
   436	
   437		*val = (stat & P_PATH_USE_USBIN_BIT) &&
   438		       (stat & P_PATH_VALID_INPUT_POWER_SOURCE_STS_BIT);
   439		return 0;
   440	}
   441	
   442	/*
   443	 * Qualcomm "automatic power source detection" aka APSD
   444	 * tells us what type of charger we're connected to.
   445	 */
   446	static int smb2_apsd_get_charger_type(struct smb2_chip *chip, int *val)
   447	{
   448		int rc;
   449		unsigned int apsd_stat, stat;
   450		int usb_online;
   451	
   452		rc = smb2_get_prop_usb_online(chip, &usb_online);
   453		if (rc < 0 || !usb_online) {
   454			*val = POWER_SUPPLY_USB_TYPE_UNKNOWN;
   455			return 0;
   456		}
   457	
   458		rc = regmap_read(chip->regmap, chip->base + APSD_STATUS, &apsd_stat);
   459		if (rc < 0) {
   460			dev_err(chip->dev, "Failed to read apsd status, rc = %d", rc);
   461			return rc;
   462		}
   463		if (!(apsd_stat & APSD_DTC_STATUS_DONE_BIT)) {
   464			dev_err(chip->dev, "Apsd not ready");
   465			return -EAGAIN;
   466		}
   467	
   468		rc = regmap_read(chip->regmap, chip->base + APSD_RESULT_STATUS, &stat);
   469		if (rc < 0) {
   470			dev_err(chip->dev, "Failed to read apsd result, rc = %d", rc);
   471			return rc;
   472		}
   473	
   474		stat &= APSD_RESULT_STATUS_MASK;
   475	
   476		if (stat & CDP_CHARGER_BIT)
   477			*val = POWER_SUPPLY_USB_TYPE_CDP;
   478		else if (stat & (DCP_CHARGER_BIT | OCP_CHARGER_BIT | FLOAT_CHARGER_BIT))
   479			*val = POWER_SUPPLY_USB_TYPE_DCP;
   480		else /* SDP_CHARGER_BIT (or others) */
   481			*val = POWER_SUPPLY_USB_TYPE_SDP;
   482	
   483		return 0;
   484	}
   485	
 > 486	int smb2_get_prop_status(struct smb2_chip *chip, int *val)
   487	{
   488		int usb_online_val;
   489		unsigned char stat[2];
   490		int rc;
   491	
   492		rc = smb2_get_prop_usb_online(chip, &usb_online_val);
   493		if (rc < 0) {
   494			dev_err(chip->dev, "Couldn't get usb online property rc = %d\n",
   495				rc);
   496			return rc;
   497		}
   498	
   499		if (!usb_online_val) {
   500			*val = POWER_SUPPLY_STATUS_DISCHARGING;
   501			return rc;
   502		}
   503	
   504		rc = regmap_bulk_read(chip->regmap,
   505				      chip->base + BATTERY_CHARGER_STATUS_1, &stat, 2);
   506		if (rc < 0) {
   507			dev_err(chip->dev, "Failed to read charging status ret=%d\n",
   508				rc);
   509			return rc;
   510		}
   511	
   512		if (stat[1] & CHARGER_ERROR_STATUS_BAT_OV_BIT) {
   513			*val = POWER_SUPPLY_STATUS_NOT_CHARGING;
   514			return 0;
   515		}
   516	
   517		stat[0] = stat[0] & BATTERY_CHARGER_STATUS_MASK;
   518	
   519		switch (stat[0]) {
   520		case TRICKLE_CHARGE:
   521		case PRE_CHARGE:
   522		case FAST_CHARGE:
   523		case FULLON_CHARGE:
   524		case TAPER_CHARGE:
   525			*val = POWER_SUPPLY_STATUS_CHARGING;
   526			return rc;
   527		case DISABLE_CHARGE:
   528			*val = POWER_SUPPLY_STATUS_NOT_CHARGING;
   529			return rc;
   530		case TERMINATE_CHARGE:
   531			*val = POWER_SUPPLY_STATUS_FULL;
   532			return rc;
   533		case INHIBIT_CHARGE:
   534		default:
   535			*val = POWER_SUPPLY_STATUS_UNKNOWN;
   536			return rc;
   537		}
   538	}
   539	
   540	static inline int smb2_get_current_limit(struct smb2_chip *chip,
   541						 unsigned int *val)
   542	{
   543		int rc = regmap_read(chip->regmap, chip->base + ICL_STATUS, val);
   544	
   545		if (rc >= 0)
   546			*val *= 25000;
   547		return rc;
   548	}
   549	
   550	static int smb2_set_current_limit(struct smb2_chip *chip, unsigned int val)
   551	{
   552		unsigned char val_raw;
   553	
   554		if (val > 4800000) {
   555			dev_err(chip->dev,
   556				"Can't set current limit higher than 4800000uA");
   557			return -EINVAL;
   558		}
   559		val_raw = val / 25000;
   560	
   561		return regmap_write(chip->regmap, chip->base + USBIN_CURRENT_LIMIT_CFG,
   562				    val_raw);
   563	}
   564	
 > 565	void smb2_status_change_work(struct work_struct *work)
   566	{
   567		struct smb2_chip *chip =
   568			container_of(work, struct smb2_chip, status_change_work.work);
   569		unsigned int charger_type, current_ua;
   570		int usb_online, count, rc;
   571	
   572		smb2_get_prop_usb_online(chip, &usb_online);
   573		if (usb_online == 0) {
   574			chip->default_curr_limit = 0;
   575			return;
   576		}
   577	
   578		for (count = 0; count < 3; count++) {
   579			dev_dbg(chip->dev, "get charger type retry %d\n", count);
   580			rc = smb2_apsd_get_charger_type(chip, &charger_type);
   581			if (rc == 0)
   582				break;
   583			msleep(100);
   584		}
   585	
   586		if (rc < 0) {
   587			rc = regmap_update_bits(chip->regmap, chip->base + CMD_APSD,
   588						APSD_RERUN_BIT, APSD_RERUN_BIT);
   589			schedule_delayed_work(&chip->status_change_work,
   590					      msecs_to_jiffies(1500));
   591			dev_dbg(chip->dev, "get charger type failed, rerun apsd\n");
   592			return;
   593		}
   594	
   595		switch (charger_type) {
   596		case POWER_SUPPLY_USB_TYPE_CDP:
   597			current_ua = CDP_CURRENT_UA;
   598			break;
   599		case POWER_SUPPLY_USB_TYPE_DCP:
   600			current_ua = DCP_CURRENT_UA;
   601			break;
   602		case POWER_SUPPLY_USB_TYPE_SDP:
   603		default:
   604			current_ua = SDP_CURRENT_UA;
   605			break;
   606		}
   607	
   608		chip->default_curr_limit = current_ua;
   609	
   610		smb2_set_current_limit(chip, current_ua);
   611		power_supply_changed(chip->chg_psy);
   612	}
   613	
 > 614	int smb2_get_iio_chan(struct smb2_chip *chip, struct iio_channel *chan,
   615			      int *val)
   616	{
   617		int rc;
   618		union power_supply_propval status;
   619	
   620		rc = power_supply_get_property(chip->chg_psy, POWER_SUPPLY_PROP_STATUS,
   621					       &status);
   622		if (rc < 0 || status.intval != POWER_SUPPLY_STATUS_CHARGING) {
   623			*val = 0;
   624			return 0;
   625		}
   626	
   627		if (IS_ERR(chan)) {
   628			dev_err(chip->dev, "Failed to chan, err = %li", PTR_ERR(chan));
   629			return PTR_ERR(chan);
   630		}
   631	
   632		return iio_read_channel_processed(chan, val);
   633	}
   634	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux