Re: [PATCH 2/3] regulator: max77693: Also manipulate the fast charge current

[Krzysztof Kozlowski]

On 09/27/2016 07:51 PM, Wolfgang Wiedmeyer wrote:
> 
> Mark Brown writes:
> 
>> On Tue, Sep 27, 2016 at 03:50:42PM +0200, Wolfgang Wiedmeyer wrote:
>>
>>> When charging is disabled (e.g. by removing the USB cable) the charge
>>> current is not reset to zero. So if I expose the current by the
>>> CURRENT_NOW property, it incorrectly reports the current that was set
>>> when charging was enabled, although there is no charging going on
>>> anymore. So I felt the need to update the charge current every time the
>>> charger gets enabled or disabled.
>>> Initially, the charge current is set to zero, so I think it needs to be
>>> set at least at the beginning to enable charging.
>>
>> Are you sure that the register value you're looking at is the actual
>> charge current right now and not just the maximum that the charger will
>> try to use depending on the conditions (supply available, battery
>> state...)?  It seems like you're acting as though it's the latter but
>> that's not what the chip is doing.
> 
> I was looking at the vendor code that was released for the Galaxy S3 and
> there the same register gets accessed for getting the current for
> the CURRENT_NOW property [1] and for setting the current [2]. So is this
> probably the wrong use of the CURRENT_NOW property because not the
> actual charge current is read but the maximum value that was
> set?

Yes, reading from this register will give only information about
currently set charge current. Not the real current.

Best regards,
Krzysztof

Re: [PATCH 2/3] regulator: max77693: Also manipulate the fast charge current

[Krzysztof Kozlowski]

On 09/27/2016 07:51 PM, Wolfgang Wiedmeyer wrote:
> 
> Mark Brown writes:
> 
>> On Tue, Sep 27, 2016 at 03:50:42PM +0200, Wolfgang Wiedmeyer wrote:
>>
>>> When charging is disabled (e.g. by removing the USB cable) the charge
>>> current is not reset to zero. So if I expose the current by the
>>> CURRENT_NOW property, it incorrectly reports the current that was set
>>> when charging was enabled, although there is no charging going on
>>> anymore. So I felt the need to update the charge current every time the
>>> charger gets enabled or disabled.
>>> Initially, the charge current is set to zero, so I think it needs to be
>>> set at least at the beginning to enable charging.
>>
>> Are you sure that the register value you're looking at is the actual
>> charge current right now and not just the maximum that the charger will
>> try to use depending on the conditions (supply available, battery
>> state...)?  It seems like you're acting as though it's the latter but
>> that's not what the chip is doing.
> 
> I was looking at the vendor code that was released for the Galaxy S3 and
> there the same register gets accessed for getting the current for
> the CURRENT_NOW property [1] and for setting the current [2]. So is this
> probably the wrong use of the CURRENT_NOW property because not the
> actual charge current is read but the maximum value that was
> set?

Yes, reading from this register will give only information about
currently set charge current. Not the real current.

Best regards,
Krzysztof

Re: [PATCH 2/3] regulator: max77693: Also manipulate the fast charge current

[Wolfgang Wiedmeyer]

Mark Brown writes:

> On Tue, Sep 27, 2016 at 03:50:42PM +0200, Wolfgang Wiedmeyer wrote:
>
>> When charging is disabled (e.g. by removing the USB cable) the charge
>> current is not reset to zero. So if I expose the current by the
>> CURRENT_NOW property, it incorrectly reports the current that was set
>> when charging was enabled, although there is no charging going on
>> anymore. So I felt the need to update the charge current every time the
>> charger gets enabled or disabled.
>> Initially, the charge current is set to zero, so I think it needs to be
>> set at least at the beginning to enable charging.
>
> Are you sure that the register value you're looking at is the actual
> charge current right now and not just the maximum that the charger will
> try to use depending on the conditions (supply available, battery
> state...)?  It seems like you're acting as though it's the latter but
> that's not what the chip is doing.

I was looking at the vendor code that was released for the Galaxy S3 and
there the same register gets accessed for getting the current for
the CURRENT_NOW property [1] and for setting the current [2]. So is this
probably the wrong use of the CURRENT_NOW property because not the
actual charge current is read but the maximum value that was
set? Unfortunately, I don't have access to the datasheet and I didn't
find it online so I don't know where the actual current can be
accessed.

Thanks,
Wolfgang

[1] 
https://code.fossencdi.org/kernel_samsung_smdk4412.git/tree/drivers/battery/max77693_charger.c#n531

[2] 
https://code.fossencdi.org/kernel_samsung_smdk4412.git/tree/drivers/battery/max77693_charger.c#n552

-- 
Website: https://fossencdi.org
OpenPGP: 0F30 D1A0 2F73 F70A 6FEE  048E 5816 A24C 1075 7FC4
Key download: https://wiedmeyer.de/keys/ww.asc


signature.asc
Description: PGP signature

Re: [PATCH 2/3] regulator: max77693: Also manipulate the fast charge current

[Wolfgang Wiedmeyer]

Mark Brown writes:

> On Tue, Sep 27, 2016 at 03:50:42PM +0200, Wolfgang Wiedmeyer wrote:
>
>> When charging is disabled (e.g. by removing the USB cable) the charge
>> current is not reset to zero. So if I expose the current by the
>> CURRENT_NOW property, it incorrectly reports the current that was set
>> when charging was enabled, although there is no charging going on
>> anymore. So I felt the need to update the charge current every time the
>> charger gets enabled or disabled.
>> Initially, the charge current is set to zero, so I think it needs to be
>> set at least at the beginning to enable charging.
>
> Are you sure that the register value you're looking at is the actual
> charge current right now and not just the maximum that the charger will
> try to use depending on the conditions (supply available, battery
> state...)?  It seems like you're acting as though it's the latter but
> that's not what the chip is doing.

I was looking at the vendor code that was released for the Galaxy S3 and
there the same register gets accessed for getting the current for
the CURRENT_NOW property [1] and for setting the current [2]. So is this
probably the wrong use of the CURRENT_NOW property because not the
actual charge current is read but the maximum value that was
set? Unfortunately, I don't have access to the datasheet and I didn't
find it online so I don't know where the actual current can be
accessed.

Thanks,
Wolfgang

[1] 
https://code.fossencdi.org/kernel_samsung_smdk4412.git/tree/drivers/battery/max77693_charger.c#n531

[2] 
https://code.fossencdi.org/kernel_samsung_smdk4412.git/tree/drivers/battery/max77693_charger.c#n552

-- 
Website: https://fossencdi.org
OpenPGP: 0F30 D1A0 2F73 F70A 6FEE  048E 5816 A24C 1075 7FC4
Key download: https://wiedmeyer.de/keys/ww.asc


signature.asc
Description: PGP signature

Re: [PATCH 2/3] regulator: max77693: Also manipulate the fast charge current

[Mark Brown]

On Tue, Sep 27, 2016 at 03:50:42PM +0200, Wolfgang Wiedmeyer wrote:

> When charging is disabled (e.g. by removing the USB cable) the charge
> current is not reset to zero. So if I expose the current by the
> CURRENT_NOW property, it incorrectly reports the current that was set
> when charging was enabled, although there is no charging going on
> anymore. So I felt the need to update the charge current every time the
> charger gets enabled or disabled.
> Initially, the charge current is set to zero, so I think it needs to be
> set at least at the beginning to enable charging.

Are you sure that the register value you're looking at is the actual
charge current right now and not just the maximum that the charger will
try to use depending on the conditions (supply available, battery
state...)?  It seems like you're acting as though it's the latter but
that's not what the chip is doing.


signature.asc
Description: PGP signature

Re: [PATCH 2/3] regulator: max77693: Also manipulate the fast charge current

[Mark Brown]

On Tue, Sep 27, 2016 at 03:50:42PM +0200, Wolfgang Wiedmeyer wrote:

> When charging is disabled (e.g. by removing the USB cable) the charge
> current is not reset to zero. So if I expose the current by the
> CURRENT_NOW property, it incorrectly reports the current that was set
> when charging was enabled, although there is no charging going on
> anymore. So I felt the need to update the charge current every time the
> charger gets enabled or disabled.
> Initially, the charge current is set to zero, so I think it needs to be
> set at least at the beginning to enable charging.

Are you sure that the register value you're looking at is the actual
charge current right now and not just the maximum that the charger will
try to use depending on the conditions (supply available, battery
state...)?  It seems like you're acting as though it's the latter but
that's not what the chip is doing.


signature.asc
Description: PGP signature

Re: [PATCH 2/3] regulator: max77693: Also manipulate the fast charge current

[Wolfgang Wiedmeyer]

Krzysztof Kozlowski writes:

> On Tue, Sep 27, 2016 at 01:31:09AM +0200, Wolfgang Wiedmeyer wrote:
>> For MAX77693, the fast charge current also needs to be manipulated for
>> proper charging. The fast charge current is only set in the case of
>> the MAX77693 type, as the MAX77843 properly manipulates the fast
>> charge current.
>
> Are you sure it has to be manipulated? Some time I didn't dig into this.
> Now I looked at the datasheet and it says that usually there is no need
> for changing the charge current during the operation. Maxim recommends
> to setting it to a maximum safe value for the battery. The device will
> manage charge current on its own.
>
> However I agree that the charge current should be set... maybe once, to
> a maximum value appropriate for battery. Probably this should be done
> by max77693_charger in max77693_dt_init().

When charging is disabled (e.g. by removing the USB cable) the charge
current is not reset to zero. So if I expose the current by the
CURRENT_NOW property, it incorrectly reports the current that was set
when charging was enabled, although there is no charging going on
anymore. So I felt the need to update the charge current every time the
charger gets enabled or disabled.
Initially, the charge current is set to zero, so I think it needs to be
set at least at the beginning to enable charging.

Thanks,
Wolfgang

> Best regards,
> Krzysztof
>
>
>> The fast charge current is set to the next possible value below the
>> maximum input current.
>
>
>> 
>> Signed-off-by: Wolfgang Wiedmeyer 
>> ---
>>  drivers/regulator/max77693-regulator.c | 45 
>> +++---
>>  1 file changed, 42 insertions(+), 3 deletions(-)
>> 
>> diff --git a/drivers/regulator/max77693-regulator.c 
>> b/drivers/regulator/max77693-regulator.c
>> index cfbb951..e2f7584 100644
>> --- a/drivers/regulator/max77693-regulator.c
>> +++ b/drivers/regulator/max77693-regulator.c
>> @@ -54,14 +54,19 @@ struct chg_reg_data {
>>  unsigned int linear_mask;
>>  unsigned int uA_step;
>>  unsigned int min_sel;
>> +
>> +bool set_fast;
>> +unsigned int fast_reg;
>> +unsigned int fast_mask;
>>  };
>>  
>>  /*
>>   * MAX77693 CHARGER regulator - Min : 20mA, Max : 2580mA, step : 20mA
>>   * 0x00, 0x01, 0x2, 0x03= 60 mA
>>   * 0x04 ~ 0x7E  = (60 + (X - 3) * 20) mA
>> - * Actually for MAX77693 the driver manipulates the maximum input current,
>> - * not the fast charge current (output). This should be fixed.
>> + * Actually for MAX77693 the driver manipulates the maximum input current
>> + * and the fast charge current (output) because the fast charge current
>> + * is not set.
>>   *
>>   * On MAX77843 the calculation formula is the same (except values).
>>   * Fortunately it properly manipulates the fast charge current.
>> @@ -100,6 +105,8 @@ static int max77693_chg_set_current_limit(struct 
>> regulator_dev *rdev,
>>  const struct chg_reg_data *reg_data = rdev_get_drvdata(rdev);
>>  unsigned int chg_min_uA = rdev->constraints->min_uA;
>>  int sel = 0;
>> +unsigned int data;
>> +int ret;
>>  
>>  while (chg_min_uA + reg_data->uA_step * sel < min_uA)
>>  sel++;
>> @@ -110,7 +117,35 @@ static int max77693_chg_set_current_limit(struct 
>> regulator_dev *rdev,
>>  /* the first four codes for charger current are all 60mA */
>>  sel += reg_data->min_sel;
>>  
>> -return regmap_write(rdev->regmap, reg_data->linear_reg, sel);
>> +ret = regmap_write(rdev->regmap, reg_data->linear_reg, sel);
>> +if (ret < 0)
>> +return ret;
>> +
>> +if (reg_data->set_fast) {
>> +/* disable fast charge if minimum value */
>> +if (sel == reg_data->min_sel)
>> +data = 0;
>> +else {
>> +/*
>> + * set the fast charge current to the closest value
>> + * below the input current
>> + */
>> +ret = regmap_read(rdev->regmap, reg_data->fast_reg,
>> +  );
>> +if (ret < 0)
>> +return ret;
>> +
>> +sel *= reg_data->uA_step / 1000; /* convert to mA */
>> +data &= ~reg_data->fast_mask;
>> +data |= sel * 10 / 333; /* 0.1A/3 steps */
>> +}
>> +
>> +ret = regmap_write(rdev->regmap, reg_data->fast_reg, data);
>> +if (ret < 0)
>> +return ret;
>> +}
>> +
>> +return 0;
>>  }
>>  /* end of CHARGER regulator ops */
>>  
>> @@ -197,6 +232,9 @@ static const struct chg_reg_data max77693_chg_reg_data = 
>> {
>>  .linear_mask= CHG_CNFG_09_CHGIN_ILIM_MASK,
>>  .uA_step= 2,
>>  .min_sel= 3,
>> +.set_fast   = true,
>> +.fast_reg   = MAX77693_CHG_REG_CHG_CNFG_02,
>> +.fast_mask  = CHG_CNFG_02_CC_MASK,
>> 

Re: [PATCH 2/3] regulator: max77693: Also manipulate the fast charge current

[Wolfgang Wiedmeyer]

Krzysztof Kozlowski writes:

> On Tue, Sep 27, 2016 at 01:31:09AM +0200, Wolfgang Wiedmeyer wrote:
>> For MAX77693, the fast charge current also needs to be manipulated for
>> proper charging. The fast charge current is only set in the case of
>> the MAX77693 type, as the MAX77843 properly manipulates the fast
>> charge current.
>
> Are you sure it has to be manipulated? Some time I didn't dig into this.
> Now I looked at the datasheet and it says that usually there is no need
> for changing the charge current during the operation. Maxim recommends
> to setting it to a maximum safe value for the battery. The device will
> manage charge current on its own.
>
> However I agree that the charge current should be set... maybe once, to
> a maximum value appropriate for battery. Probably this should be done
> by max77693_charger in max77693_dt_init().

When charging is disabled (e.g. by removing the USB cable) the charge
current is not reset to zero. So if I expose the current by the
CURRENT_NOW property, it incorrectly reports the current that was set
when charging was enabled, although there is no charging going on
anymore. So I felt the need to update the charge current every time the
charger gets enabled or disabled.
Initially, the charge current is set to zero, so I think it needs to be
set at least at the beginning to enable charging.

Thanks,
Wolfgang

> Best regards,
> Krzysztof
>
>
>> The fast charge current is set to the next possible value below the
>> maximum input current.
>
>
>> 
>> Signed-off-by: Wolfgang Wiedmeyer 
>> ---
>>  drivers/regulator/max77693-regulator.c | 45 
>> +++---
>>  1 file changed, 42 insertions(+), 3 deletions(-)
>> 
>> diff --git a/drivers/regulator/max77693-regulator.c 
>> b/drivers/regulator/max77693-regulator.c
>> index cfbb951..e2f7584 100644
>> --- a/drivers/regulator/max77693-regulator.c
>> +++ b/drivers/regulator/max77693-regulator.c
>> @@ -54,14 +54,19 @@ struct chg_reg_data {
>>  unsigned int linear_mask;
>>  unsigned int uA_step;
>>  unsigned int min_sel;
>> +
>> +bool set_fast;
>> +unsigned int fast_reg;
>> +unsigned int fast_mask;
>>  };
>>  
>>  /*
>>   * MAX77693 CHARGER regulator - Min : 20mA, Max : 2580mA, step : 20mA
>>   * 0x00, 0x01, 0x2, 0x03= 60 mA
>>   * 0x04 ~ 0x7E  = (60 + (X - 3) * 20) mA
>> - * Actually for MAX77693 the driver manipulates the maximum input current,
>> - * not the fast charge current (output). This should be fixed.
>> + * Actually for MAX77693 the driver manipulates the maximum input current
>> + * and the fast charge current (output) because the fast charge current
>> + * is not set.
>>   *
>>   * On MAX77843 the calculation formula is the same (except values).
>>   * Fortunately it properly manipulates the fast charge current.
>> @@ -100,6 +105,8 @@ static int max77693_chg_set_current_limit(struct 
>> regulator_dev *rdev,
>>  const struct chg_reg_data *reg_data = rdev_get_drvdata(rdev);
>>  unsigned int chg_min_uA = rdev->constraints->min_uA;
>>  int sel = 0;
>> +unsigned int data;
>> +int ret;
>>  
>>  while (chg_min_uA + reg_data->uA_step * sel < min_uA)
>>  sel++;
>> @@ -110,7 +117,35 @@ static int max77693_chg_set_current_limit(struct 
>> regulator_dev *rdev,
>>  /* the first four codes for charger current are all 60mA */
>>  sel += reg_data->min_sel;
>>  
>> -return regmap_write(rdev->regmap, reg_data->linear_reg, sel);
>> +ret = regmap_write(rdev->regmap, reg_data->linear_reg, sel);
>> +if (ret < 0)
>> +return ret;
>> +
>> +if (reg_data->set_fast) {
>> +/* disable fast charge if minimum value */
>> +if (sel == reg_data->min_sel)
>> +data = 0;
>> +else {
>> +/*
>> + * set the fast charge current to the closest value
>> + * below the input current
>> + */
>> +ret = regmap_read(rdev->regmap, reg_data->fast_reg,
>> +  );
>> +if (ret < 0)
>> +return ret;
>> +
>> +sel *= reg_data->uA_step / 1000; /* convert to mA */
>> +data &= ~reg_data->fast_mask;
>> +data |= sel * 10 / 333; /* 0.1A/3 steps */
>> +}
>> +
>> +ret = regmap_write(rdev->regmap, reg_data->fast_reg, data);
>> +if (ret < 0)
>> +return ret;
>> +}
>> +
>> +return 0;
>>  }
>>  /* end of CHARGER regulator ops */
>>  
>> @@ -197,6 +232,9 @@ static const struct chg_reg_data max77693_chg_reg_data = 
>> {
>>  .linear_mask= CHG_CNFG_09_CHGIN_ILIM_MASK,
>>  .uA_step= 2,
>>  .min_sel= 3,
>> +.set_fast   = true,
>> +.fast_reg   = MAX77693_CHG_REG_CHG_CNFG_02,
>> +.fast_mask  = CHG_CNFG_02_CC_MASK,
>>  };
>>  
>>  #define  

Re: [PATCH 2/3] regulator: max77693: Also manipulate the fast charge current

[Krzysztof Kozlowski]

On Tue, Sep 27, 2016 at 01:31:09AM +0200, Wolfgang Wiedmeyer wrote:
> For MAX77693, the fast charge current also needs to be manipulated for
> proper charging. The fast charge current is only set in the case of
> the MAX77693 type, as the MAX77843 properly manipulates the fast
> charge current.

Are you sure it has to be manipulated? Some time I didn't dig into this.
Now I looked at the datasheet and it says that usually there is no need
for changing the charge current during the operation. Maxim recommends
to setting it to a maximum safe value for the battery. The device will
manage charge current on its own.

However I agree that the charge current should be set... maybe once, to
a maximum value appropriate for battery. Probably this should be done
by max77693_charger in max77693_dt_init().

Best regards,
Krzysztof


> The fast charge current is set to the next possible value below the
> maximum input current.


> 
> Signed-off-by: Wolfgang Wiedmeyer 
> ---
>  drivers/regulator/max77693-regulator.c | 45 
> +++---
>  1 file changed, 42 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/regulator/max77693-regulator.c 
> b/drivers/regulator/max77693-regulator.c
> index cfbb951..e2f7584 100644
> --- a/drivers/regulator/max77693-regulator.c
> +++ b/drivers/regulator/max77693-regulator.c
> @@ -54,14 +54,19 @@ struct chg_reg_data {
>   unsigned int linear_mask;
>   unsigned int uA_step;
>   unsigned int min_sel;
> +
> + bool set_fast;
> + unsigned int fast_reg;
> + unsigned int fast_mask;
>  };
>  
>  /*
>   * MAX77693 CHARGER regulator - Min : 20mA, Max : 2580mA, step : 20mA
>   * 0x00, 0x01, 0x2, 0x03 = 60 mA
>   * 0x04 ~ 0x7E   = (60 + (X - 3) * 20) mA
> - * Actually for MAX77693 the driver manipulates the maximum input current,
> - * not the fast charge current (output). This should be fixed.
> + * Actually for MAX77693 the driver manipulates the maximum input current
> + * and the fast charge current (output) because the fast charge current
> + * is not set.
>   *
>   * On MAX77843 the calculation formula is the same (except values).
>   * Fortunately it properly manipulates the fast charge current.
> @@ -100,6 +105,8 @@ static int max77693_chg_set_current_limit(struct 
> regulator_dev *rdev,
>   const struct chg_reg_data *reg_data = rdev_get_drvdata(rdev);
>   unsigned int chg_min_uA = rdev->constraints->min_uA;
>   int sel = 0;
> + unsigned int data;
> + int ret;
>  
>   while (chg_min_uA + reg_data->uA_step * sel < min_uA)
>   sel++;
> @@ -110,7 +117,35 @@ static int max77693_chg_set_current_limit(struct 
> regulator_dev *rdev,
>   /* the first four codes for charger current are all 60mA */
>   sel += reg_data->min_sel;
>  
> - return regmap_write(rdev->regmap, reg_data->linear_reg, sel);
> + ret = regmap_write(rdev->regmap, reg_data->linear_reg, sel);
> + if (ret < 0)
> + return ret;
> +
> + if (reg_data->set_fast) {
> + /* disable fast charge if minimum value */
> + if (sel == reg_data->min_sel)
> + data = 0;
> + else {
> + /*
> +  * set the fast charge current to the closest value
> +  * below the input current
> +  */
> + ret = regmap_read(rdev->regmap, reg_data->fast_reg,
> +   );
> + if (ret < 0)
> + return ret;
> +
> + sel *= reg_data->uA_step / 1000; /* convert to mA */
> + data &= ~reg_data->fast_mask;
> + data |= sel * 10 / 333; /* 0.1A/3 steps */
> + }
> +
> + ret = regmap_write(rdev->regmap, reg_data->fast_reg, data);
> + if (ret < 0)
> + return ret;
> + }
> +
> + return 0;
>  }
>  /* end of CHARGER regulator ops */
>  
> @@ -197,6 +232,9 @@ static const struct chg_reg_data max77693_chg_reg_data = {
>   .linear_mask= CHG_CNFG_09_CHGIN_ILIM_MASK,
>   .uA_step= 2,
>   .min_sel= 3,
> + .set_fast   = true,
> + .fast_reg   = MAX77693_CHG_REG_CHG_CNFG_02,
> + .fast_mask  = CHG_CNFG_02_CC_MASK,
>  };
>  
>  #define  max77843_regulator_desc_esafeout(num)   {   
> \
> @@ -237,6 +275,7 @@ static const struct chg_reg_data max77843_chg_reg_data = {
>   .linear_mask= MAX77843_CHG_FAST_CHG_CURRENT_MASK,
>   .uA_step= MAX77843_CHG_FAST_CHG_CURRENT_STEP,
>   .min_sel= 2,
> + .set_fast   = false,
>  };
>  
>  static int max77693_pmic_probe(struct platform_device *pdev)
> -- 
> 2.8.0.rc3
> 

Re: [PATCH 2/3] regulator: max77693: Also manipulate the fast charge current

[Krzysztof Kozlowski]

On Tue, Sep 27, 2016 at 01:31:09AM +0200, Wolfgang Wiedmeyer wrote:
> For MAX77693, the fast charge current also needs to be manipulated for
> proper charging. The fast charge current is only set in the case of
> the MAX77693 type, as the MAX77843 properly manipulates the fast
> charge current.

Are you sure it has to be manipulated? Some time I didn't dig into this.
Now I looked at the datasheet and it says that usually there is no need
for changing the charge current during the operation. Maxim recommends
to setting it to a maximum safe value for the battery. The device will
manage charge current on its own.

However I agree that the charge current should be set... maybe once, to
a maximum value appropriate for battery. Probably this should be done
by max77693_charger in max77693_dt_init().

Best regards,
Krzysztof


> The fast charge current is set to the next possible value below the
> maximum input current.


> 
> Signed-off-by: Wolfgang Wiedmeyer 
> ---
>  drivers/regulator/max77693-regulator.c | 45 
> +++---
>  1 file changed, 42 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/regulator/max77693-regulator.c 
> b/drivers/regulator/max77693-regulator.c
> index cfbb951..e2f7584 100644
> --- a/drivers/regulator/max77693-regulator.c
> +++ b/drivers/regulator/max77693-regulator.c
> @@ -54,14 +54,19 @@ struct chg_reg_data {
>   unsigned int linear_mask;
>   unsigned int uA_step;
>   unsigned int min_sel;
> +
> + bool set_fast;
> + unsigned int fast_reg;
> + unsigned int fast_mask;
>  };
>  
>  /*
>   * MAX77693 CHARGER regulator - Min : 20mA, Max : 2580mA, step : 20mA
>   * 0x00, 0x01, 0x2, 0x03 = 60 mA
>   * 0x04 ~ 0x7E   = (60 + (X - 3) * 20) mA
> - * Actually for MAX77693 the driver manipulates the maximum input current,
> - * not the fast charge current (output). This should be fixed.
> + * Actually for MAX77693 the driver manipulates the maximum input current
> + * and the fast charge current (output) because the fast charge current
> + * is not set.
>   *
>   * On MAX77843 the calculation formula is the same (except values).
>   * Fortunately it properly manipulates the fast charge current.
> @@ -100,6 +105,8 @@ static int max77693_chg_set_current_limit(struct 
> regulator_dev *rdev,
>   const struct chg_reg_data *reg_data = rdev_get_drvdata(rdev);
>   unsigned int chg_min_uA = rdev->constraints->min_uA;
>   int sel = 0;
> + unsigned int data;
> + int ret;
>  
>   while (chg_min_uA + reg_data->uA_step * sel < min_uA)
>   sel++;
> @@ -110,7 +117,35 @@ static int max77693_chg_set_current_limit(struct 
> regulator_dev *rdev,
>   /* the first four codes for charger current are all 60mA */
>   sel += reg_data->min_sel;
>  
> - return regmap_write(rdev->regmap, reg_data->linear_reg, sel);
> + ret = regmap_write(rdev->regmap, reg_data->linear_reg, sel);
> + if (ret < 0)
> + return ret;
> +
> + if (reg_data->set_fast) {
> + /* disable fast charge if minimum value */
> + if (sel == reg_data->min_sel)
> + data = 0;
> + else {
> + /*
> +  * set the fast charge current to the closest value
> +  * below the input current
> +  */
> + ret = regmap_read(rdev->regmap, reg_data->fast_reg,
> +   );
> + if (ret < 0)
> + return ret;
> +
> + sel *= reg_data->uA_step / 1000; /* convert to mA */
> + data &= ~reg_data->fast_mask;
> + data |= sel * 10 / 333; /* 0.1A/3 steps */
> + }
> +
> + ret = regmap_write(rdev->regmap, reg_data->fast_reg, data);
> + if (ret < 0)
> + return ret;
> + }
> +
> + return 0;
>  }
>  /* end of CHARGER regulator ops */
>  
> @@ -197,6 +232,9 @@ static const struct chg_reg_data max77693_chg_reg_data = {
>   .linear_mask= CHG_CNFG_09_CHGIN_ILIM_MASK,
>   .uA_step= 2,
>   .min_sel= 3,
> + .set_fast   = true,
> + .fast_reg   = MAX77693_CHG_REG_CHG_CNFG_02,
> + .fast_mask  = CHG_CNFG_02_CC_MASK,
>  };
>  
>  #define  max77843_regulator_desc_esafeout(num)   {   
> \
> @@ -237,6 +275,7 @@ static const struct chg_reg_data max77843_chg_reg_data = {
>   .linear_mask= MAX77843_CHG_FAST_CHG_CURRENT_MASK,
>   .uA_step= MAX77843_CHG_FAST_CHG_CURRENT_STEP,
>   .min_sel= 2,
> + .set_fast   = false,
>  };
>  
>  static int max77693_pmic_probe(struct platform_device *pdev)
> -- 
> 2.8.0.rc3
> 

[PATCH 2/3] regulator: max77693: Also manipulate the fast charge current

[Wolfgang Wiedmeyer]

For MAX77693, the fast charge current also needs to be manipulated for
proper charging. The fast charge current is only set in the case of
the MAX77693 type, as the MAX77843 properly manipulates the fast
charge current.
The fast charge current is set to the next possible value below the
maximum input current.

Signed-off-by: Wolfgang Wiedmeyer 
---
 drivers/regulator/max77693-regulator.c | 45 +++---
 1 file changed, 42 insertions(+), 3 deletions(-)

diff --git a/drivers/regulator/max77693-regulator.c 
b/drivers/regulator/max77693-regulator.c
index cfbb951..e2f7584 100644
--- a/drivers/regulator/max77693-regulator.c
+++ b/drivers/regulator/max77693-regulator.c
@@ -54,14 +54,19 @@ struct chg_reg_data {
unsigned int linear_mask;
unsigned int uA_step;
unsigned int min_sel;
+
+   bool set_fast;
+   unsigned int fast_reg;
+   unsigned int fast_mask;
 };
 
 /*
  * MAX77693 CHARGER regulator - Min : 20mA, Max : 2580mA, step : 20mA
  * 0x00, 0x01, 0x2, 0x03   = 60 mA
  * 0x04 ~ 0x7E = (60 + (X - 3) * 20) mA
- * Actually for MAX77693 the driver manipulates the maximum input current,
- * not the fast charge current (output). This should be fixed.
+ * Actually for MAX77693 the driver manipulates the maximum input current
+ * and the fast charge current (output) because the fast charge current
+ * is not set.
  *
  * On MAX77843 the calculation formula is the same (except values).
  * Fortunately it properly manipulates the fast charge current.
@@ -100,6 +105,8 @@ static int max77693_chg_set_current_limit(struct 
regulator_dev *rdev,
const struct chg_reg_data *reg_data = rdev_get_drvdata(rdev);
unsigned int chg_min_uA = rdev->constraints->min_uA;
int sel = 0;
+   unsigned int data;
+   int ret;
 
while (chg_min_uA + reg_data->uA_step * sel < min_uA)
sel++;
@@ -110,7 +117,35 @@ static int max77693_chg_set_current_limit(struct 
regulator_dev *rdev,
/* the first four codes for charger current are all 60mA */
sel += reg_data->min_sel;
 
-   return regmap_write(rdev->regmap, reg_data->linear_reg, sel);
+   ret = regmap_write(rdev->regmap, reg_data->linear_reg, sel);
+   if (ret < 0)
+   return ret;
+
+   if (reg_data->set_fast) {
+   /* disable fast charge if minimum value */
+   if (sel == reg_data->min_sel)
+   data = 0;
+   else {
+   /*
+* set the fast charge current to the closest value
+* below the input current
+*/
+   ret = regmap_read(rdev->regmap, reg_data->fast_reg,
+ );
+   if (ret < 0)
+   return ret;
+
+   sel *= reg_data->uA_step / 1000; /* convert to mA */
+   data &= ~reg_data->fast_mask;
+   data |= sel * 10 / 333; /* 0.1A/3 steps */
+   }
+
+   ret = regmap_write(rdev->regmap, reg_data->fast_reg, data);
+   if (ret < 0)
+   return ret;
+   }
+
+   return 0;
 }
 /* end of CHARGER regulator ops */
 
@@ -197,6 +232,9 @@ static const struct chg_reg_data max77693_chg_reg_data = {
.linear_mask= CHG_CNFG_09_CHGIN_ILIM_MASK,
.uA_step= 2,
.min_sel= 3,
+   .set_fast   = true,
+   .fast_reg   = MAX77693_CHG_REG_CHG_CNFG_02,
+   .fast_mask  = CHG_CNFG_02_CC_MASK,
 };
 
 #definemax77843_regulator_desc_esafeout(num)   {   
\
@@ -237,6 +275,7 @@ static const struct chg_reg_data max77843_chg_reg_data = {
.linear_mask= MAX77843_CHG_FAST_CHG_CURRENT_MASK,
.uA_step= MAX77843_CHG_FAST_CHG_CURRENT_STEP,
.min_sel= 2,
+   .set_fast   = false,
 };
 
 static int max77693_pmic_probe(struct platform_device *pdev)
-- 
2.8.0.rc3

[PATCH 2/3] regulator: max77693: Also manipulate the fast charge current

[Wolfgang Wiedmeyer]

For MAX77693, the fast charge current also needs to be manipulated for
proper charging. The fast charge current is only set in the case of
the MAX77693 type, as the MAX77843 properly manipulates the fast
charge current.
The fast charge current is set to the next possible value below the
maximum input current.

Signed-off-by: Wolfgang Wiedmeyer 
---
 drivers/regulator/max77693-regulator.c | 45 +++---
 1 file changed, 42 insertions(+), 3 deletions(-)

diff --git a/drivers/regulator/max77693-regulator.c 
b/drivers/regulator/max77693-regulator.c
index cfbb951..e2f7584 100644
--- a/drivers/regulator/max77693-regulator.c
+++ b/drivers/regulator/max77693-regulator.c
@@ -54,14 +54,19 @@ struct chg_reg_data {
unsigned int linear_mask;
unsigned int uA_step;
unsigned int min_sel;
+
+   bool set_fast;
+   unsigned int fast_reg;
+   unsigned int fast_mask;
 };
 
 /*
  * MAX77693 CHARGER regulator - Min : 20mA, Max : 2580mA, step : 20mA
  * 0x00, 0x01, 0x2, 0x03   = 60 mA
  * 0x04 ~ 0x7E = (60 + (X - 3) * 20) mA
- * Actually for MAX77693 the driver manipulates the maximum input current,
- * not the fast charge current (output). This should be fixed.
+ * Actually for MAX77693 the driver manipulates the maximum input current
+ * and the fast charge current (output) because the fast charge current
+ * is not set.
  *
  * On MAX77843 the calculation formula is the same (except values).
  * Fortunately it properly manipulates the fast charge current.
@@ -100,6 +105,8 @@ static int max77693_chg_set_current_limit(struct 
regulator_dev *rdev,
const struct chg_reg_data *reg_data = rdev_get_drvdata(rdev);
unsigned int chg_min_uA = rdev->constraints->min_uA;
int sel = 0;
+   unsigned int data;
+   int ret;
 
while (chg_min_uA + reg_data->uA_step * sel < min_uA)
sel++;
@@ -110,7 +117,35 @@ static int max77693_chg_set_current_limit(struct 
regulator_dev *rdev,
/* the first four codes for charger current are all 60mA */
sel += reg_data->min_sel;
 
-   return regmap_write(rdev->regmap, reg_data->linear_reg, sel);
+   ret = regmap_write(rdev->regmap, reg_data->linear_reg, sel);
+   if (ret < 0)
+   return ret;
+
+   if (reg_data->set_fast) {
+   /* disable fast charge if minimum value */
+   if (sel == reg_data->min_sel)
+   data = 0;
+   else {
+   /*
+* set the fast charge current to the closest value
+* below the input current
+*/
+   ret = regmap_read(rdev->regmap, reg_data->fast_reg,
+ );
+   if (ret < 0)
+   return ret;
+
+   sel *= reg_data->uA_step / 1000; /* convert to mA */
+   data &= ~reg_data->fast_mask;
+   data |= sel * 10 / 333; /* 0.1A/3 steps */
+   }
+
+   ret = regmap_write(rdev->regmap, reg_data->fast_reg, data);
+   if (ret < 0)
+   return ret;
+   }
+
+   return 0;
 }
 /* end of CHARGER regulator ops */
 
@@ -197,6 +232,9 @@ static const struct chg_reg_data max77693_chg_reg_data = {
.linear_mask= CHG_CNFG_09_CHGIN_ILIM_MASK,
.uA_step= 2,
.min_sel= 3,
+   .set_fast   = true,
+   .fast_reg   = MAX77693_CHG_REG_CHG_CNFG_02,
+   .fast_mask  = CHG_CNFG_02_CC_MASK,
 };
 
 #definemax77843_regulator_desc_esafeout(num)   {   
\
@@ -237,6 +275,7 @@ static const struct chg_reg_data max77843_chg_reg_data = {
.linear_mask= MAX77843_CHG_FAST_CHG_CURRENT_MASK,
.uA_step= MAX77843_CHG_FAST_CHG_CURRENT_STEP,
.min_sel= 2,
+   .set_fast   = false,
 };
 
 static int max77693_pmic_probe(struct platform_device *pdev)
-- 
2.8.0.rc3