diff mbox

[v2,3/3] power: supply: sbs-battery: Add delay when changing capacity mode bit

Message ID 1499766223-130934-4-git-send-email-preid@electromag.com.au (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Phil Reid July 11, 2017, 9:43 a.m. UTC
At least with the Inspired Energy compatible batteries a delay is required
after setting the capacity mode bit from amp to watts or the reverse.
Setting the bit and then immediately pooling the status register results
in an unknown error being returned in the register. Add the delay results
in and ok status being return. This was also seen when reading the charge
and energy registers where the wrong value was returned for the requested
mode.

Signed-off-by: Phil Reid <preid@electromag.com.au>
---
 drivers/power/supply/sbs-battery.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Michael Heinemann July 11, 2017, 11:31 a.m. UTC | #1
On 2017-07-11 11:43, Phil Reid wrote:
> At least with the Inspired Energy compatible batteries a delay is 
> required
> after setting the capacity mode bit from amp to watts or the reverse.
> Setting the bit and then immediately pooling the status register 
> results
> in an unknown error being returned in the register. Add the delay 
> results
> in and ok status being return. This was also seen when reading the 
> charge
> and energy registers where the wrong value was returned for the 
> requested
> mode.
> 
> Signed-off-by: Phil Reid <preid@electromag.com.au>

Tested-by: Michael Heinemann <committed@heine.so>

> ---
>  drivers/power/supply/sbs-battery.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/power/supply/sbs-battery.c
> b/drivers/power/supply/sbs-battery.c
> index 4594ed1..3473b45 100644
> --- a/drivers/power/supply/sbs-battery.c
> +++ b/drivers/power/supply/sbs-battery.c
> @@ -14,6 +14,7 @@
>   * more details.
>   */
> 
> +#include <linux/delay.h>
>  #include <linux/err.h>
>  #include <linux/gpio/consumer.h>
>  #include <linux/i2c.h>
> @@ -511,6 +512,8 @@ static enum sbs_battery_mode
> sbs_set_battery_mode(struct i2c_client *client,
>  	if (ret < 0)
>  		return ret;
> 
> +	usleep_range(1000, 2000);
> +
>  	return original_val & BATTERY_MODE_MASK;
>  }
Phil Reid July 19, 2017, 2:16 a.m. UTC | #2
On 11/07/2017 19:31, Michael Heinemann wrote:
> On 2017-07-11 11:43, Phil Reid wrote:
>> At least with the Inspired Energy compatible batteries a delay is required
>> after setting the capacity mode bit from amp to watts or the reverse.
>> Setting the bit and then immediately pooling the status register results
>> in an unknown error being returned in the register. Add the delay results
>> in and ok status being return. This was also seen when reading the charge
>> and energy registers where the wrong value was returned for the requested
>> mode.
>>
>> Signed-off-by: Phil Reid <preid@electromag.com.au>
> 
> Tested-by: Michael Heinemann <committed@heine.so>

I received some feedback from inspired energy on this and they confirmed they need a
delay between smbus transactions for their batteries.
This is a deviation from the standard for their batteries only.

Perhaps this should delay should only be added if we detect and inspired energy
battery. Could look at the manufacturer field to determine whether to add it.

WDYR?


> 
>> ---
>>  drivers/power/supply/sbs-battery.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/power/supply/sbs-battery.c
>> b/drivers/power/supply/sbs-battery.c
>> index 4594ed1..3473b45 100644
>> --- a/drivers/power/supply/sbs-battery.c
>> +++ b/drivers/power/supply/sbs-battery.c
>> @@ -14,6 +14,7 @@
>>   * more details.
>>   */
>>
>> +#include <linux/delay.h>
>>  #include <linux/err.h>
>>  #include <linux/gpio/consumer.h>
>>  #include <linux/i2c.h>
>> @@ -511,6 +512,8 @@ static enum sbs_battery_mode
>> sbs_set_battery_mode(struct i2c_client *client,
>>      if (ret < 0)
>>          return ret;
>>
>> +    usleep_range(1000, 2000);
>> +
>>      return original_val & BATTERY_MODE_MASK;
>>  }
> 
>
diff mbox

Patch

diff --git a/drivers/power/supply/sbs-battery.c b/drivers/power/supply/sbs-battery.c
index 4594ed1..3473b45 100644
--- a/drivers/power/supply/sbs-battery.c
+++ b/drivers/power/supply/sbs-battery.c
@@ -14,6 +14,7 @@ 
  * more details.
  */
 
+#include <linux/delay.h>
 #include <linux/err.h>
 #include <linux/gpio/consumer.h>
 #include <linux/i2c.h>
@@ -511,6 +512,8 @@  static enum sbs_battery_mode sbs_set_battery_mode(struct i2c_client *client,
 	if (ret < 0)
 		return ret;
 
+	usleep_range(1000, 2000);
+
 	return original_val & BATTERY_MODE_MASK;
 }