Re: [PATCH 4/4] power_supply: bq24261 charger driver

[Jenny Tc <jenny.tc@intel.com>]

On Fri, Feb 21, 2014 at 03:44:00PM +0100, Pavel Machek wrote:
> Hi!
> 
> > > > +static inline int bq24261_set_cv(struct bq24261_charger *chip, int cv)
> > > > +{
> > > > +	int bat_volt;
> > > > +	int ret;
> > > > +	u8 reg_val;
> > > > +	u8 vindpm_val = 0x0;
> > > > +
> > > > +	/*
> > > > +	* Setting VINDPM value as per the battery voltage
> > > > +	*  VBatt           Vindpm     Register Setting
> > > > +	*  < 3.7v           4.2v       0x0 (default)
> > > > +	*  3.71v - 3.96v    4.36v      0x2
> > > > +	*  > 3.96v          4.6v       0x5
> > > > +	*/
> > > > +	ret = get_battery_voltage(&bat_volt);
> > > > +	if (ret) {
> > > > +		dev_err(&chip->client->dev,
> > > > +			"Error getting battery voltage!!\n");
> > > > +	} else {
> > > 
> > > You forget the error value and continue anyway.
> > 
> > On error, throw the error and program default VINDPM value.
> 
> Is it good idea to attempt charging when we can't read battery
> voltage?

This function decides the VINDPM setting and doesn't enable charging.
VINDPM setting is used to ensure minimum input voltage and thereby allow to
charge with low power charging source. If the voltage read fails, then the
default VINDPM value 0x0 will be programmed and the input voltage may go down
as low as 4.2V. The charging/not charging decision is taken by power supply
charger driver and not by the chip driver. The worst case impact would be that
charging may happen with a low charge current at high battery voltages,
but doesn't compromise safety at all. 

-Jenny