[v3,1/6] power: supply: max17040: Use regmap i2c

Message ID 20200624155633.3557401-2-iskren.chernev@gmail.com
State New
Headers show
Series
  • [v3,1/6] power: supply: max17040: Use regmap i2c
Related show

Commit Message

Iskren Chernev June 24, 2020, 3:56 p.m.
Rewrite i2c operations from i2c client read/write to regmap i2c. As
a result, most private functions now accept the private driver data
instead of an i2c client pointer.

Signed-off-by: Iskren Chernev <iskren.chernev@gmail.com>
---
 drivers/power/supply/Kconfig            |   1 +
 drivers/power/supply/max17040_battery.c | 250 +++++++++++-------------
 2 files changed, 110 insertions(+), 141 deletions(-)

Comments

Sebastian Reichel Aug. 28, 2020, 5:44 p.m. | #1
Hi,

On Wed, Jun 24, 2020 at 06:56:28PM +0300, Iskren Chernev wrote:
> Rewrite i2c operations from i2c client read/write to regmap i2c. As

> a result, most private functions now accept the private driver data

> instead of an i2c client pointer.

> 

> Signed-off-by: Iskren Chernev <iskren.chernev@gmail.com>

> ---


This still does two things: converting the driver to devm_
and using regmap. Please split into two patches. Otherwise
the changes look good, but the patchset needs to be rebased.

-- Sebastian

>  drivers/power/supply/Kconfig            |   1 +

>  drivers/power/supply/max17040_battery.c | 250 +++++++++++-------------

>  2 files changed, 110 insertions(+), 141 deletions(-)

> 

> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig

> index 44d3c8512fb8d..12ca79952768f 100644

> --- a/drivers/power/supply/Kconfig

> +++ b/drivers/power/supply/Kconfig

> @@ -367,6 +367,7 @@ config AXP288_FUEL_GAUGE

>  config BATTERY_MAX17040

>  	tristate "Maxim MAX17040 Fuel Gauge"

>  	depends on I2C

> +	select REGMAP_I2C

>  	help

>  	  MAX17040 is fuel-gauge systems for lithium-ion (Li+) batteries

>  	  in handheld and portable equipment. The MAX17040 is configured

> diff --git a/drivers/power/supply/max17040_battery.c b/drivers/power/supply/max17040_battery.c

> index 48aa44665e2f1..678241fcc2548 100644

> --- a/drivers/power/supply/max17040_battery.c

> +++ b/drivers/power/supply/max17040_battery.c

> @@ -16,32 +16,30 @@

>  #include <linux/interrupt.h>

>  #include <linux/power_supply.h>

>  #include <linux/max17040_battery.h>

> +#include <linux/regmap.h>

>  #include <linux/slab.h>

>  

>  #define MAX17040_VCELL	0x02

>  #define MAX17040_SOC	0x04

>  #define MAX17040_MODE	0x06

>  #define MAX17040_VER	0x08

> -#define MAX17040_RCOMP	0x0C

> +#define MAX17040_CONFIG	0x0C

>  #define MAX17040_CMD	0xFE

>  

>  

>  #define MAX17040_DELAY		1000

>  #define MAX17040_BATTERY_FULL	95

>  

> -#define MAX17040_ATHD_MASK		0xFFC0

> +#define MAX17040_ATHD_MASK		0x3f

>  #define MAX17040_ATHD_DEFAULT_POWER_UP	4

>  

>  struct max17040_chip {

>  	struct i2c_client		*client;

> +	struct regmap			*regmap;

>  	struct delayed_work		work;

>  	struct power_supply		*battery;

>  	struct max17040_platform_data	*pdata;

>  

> -	/* State Of Connect */

> -	int online;

> -	/* battery voltage */

> -	int vcell;

>  	/* battery capacity */

>  	int soc;

>  	/* State Of Charge */

> @@ -50,135 +48,68 @@ struct max17040_chip {

>  	u32 low_soc_alert;

>  };

>  

> -static int max17040_get_property(struct power_supply *psy,

> -			    enum power_supply_property psp,

> -			    union power_supply_propval *val)

> -{

> -	struct max17040_chip *chip = power_supply_get_drvdata(psy);

> -

> -	switch (psp) {

> -	case POWER_SUPPLY_PROP_STATUS:

> -		val->intval = chip->status;

> -		break;

> -	case POWER_SUPPLY_PROP_ONLINE:

> -		val->intval = chip->online;

> -		break;

> -	case POWER_SUPPLY_PROP_VOLTAGE_NOW:

> -		val->intval = chip->vcell;

> -		break;

> -	case POWER_SUPPLY_PROP_CAPACITY:

> -		val->intval = chip->soc;

> -		break;

> -	default:

> -		return -EINVAL;

> -	}

> -	return 0;

> -}

> -

> -static int max17040_write_reg(struct i2c_client *client, int reg, u16 value)

> -{

> -	int ret;

> -

> -	ret = i2c_smbus_write_word_swapped(client, reg, value);

> -

> -	if (ret < 0)

> -		dev_err(&client->dev, "%s: err %d\n", __func__, ret);

> -

> -	return ret;

> -}

> -

> -static int max17040_read_reg(struct i2c_client *client, int reg)

> -{

> -	int ret;

> -

> -	ret = i2c_smbus_read_word_swapped(client, reg);

> -

> -	if (ret < 0)

> -		dev_err(&client->dev, "%s: err %d\n", __func__, ret);

> -

> -	return ret;

> -}

> -

> -static void max17040_reset(struct i2c_client *client)

> +static int max17040_reset(struct max17040_chip *chip)

>  {

> -	max17040_write_reg(client, MAX17040_CMD, 0x0054);

> +	return regmap_write(chip->regmap, MAX17040_CMD, 0x0054);

>  }

>  

> -static int max17040_set_low_soc_alert(struct i2c_client *client, u32 level)

> +static int max17040_set_low_soc_alert(struct max17040_chip *chip, u32 level)

>  {

> -	int ret;

> -	u16 data;

> -

>  	level = 32 - level;

> -	data = max17040_read_reg(client, MAX17040_RCOMP);

> -	/* clear the alrt bit and set LSb 5 bits */

> -	data &= MAX17040_ATHD_MASK;

> -	data |= level;

> -	ret = max17040_write_reg(client, MAX17040_RCOMP, data);

> -

> -	return ret;

> +	return regmap_update_bits(chip->regmap, MAX17040_CONFIG,

> +			MAX17040_ATHD_MASK, level);

>  }

>  

> -static void max17040_get_vcell(struct i2c_client *client)

> +static int max17040_get_vcell(struct max17040_chip *chip)

>  {

> -	struct max17040_chip *chip = i2c_get_clientdata(client);

> -	u16 vcell;

> +	u32 vcell;

>  

> -	vcell = max17040_read_reg(client, MAX17040_VCELL);

> +	regmap_read(chip->regmap, MAX17040_VCELL, &vcell);

>  

> -	chip->vcell = (vcell >> 4) * 1250;

> +	return (vcell >> 4) * 1250;

>  }

>  

> -static void max17040_get_soc(struct i2c_client *client)

> +static int max17040_get_soc(struct max17040_chip *chip)

>  {

> -	struct max17040_chip *chip = i2c_get_clientdata(client);

> -	u16 soc;

> +	u32 soc;

>  

> -	soc = max17040_read_reg(client, MAX17040_SOC);

> +	regmap_read(chip->regmap, MAX17040_SOC, &soc);

>  

> -	chip->soc = (soc >> 8);

> +	return soc >> 8;

>  }

>  

> -static void max17040_get_version(struct i2c_client *client)

> +static int max17040_get_version(struct max17040_chip *chip)

>  {

> -	u16 version;

> +	int ret;

> +	u32 version;

>  

> -	version = max17040_read_reg(client, MAX17040_VER);

> +	ret = regmap_read(chip->regmap, MAX17040_VER, &version);

>  

> -	dev_info(&client->dev, "MAX17040 Fuel-Gauge Ver 0x%x\n", version);

> +	return ret ? ret : version;

>  }

>  

> -static void max17040_get_online(struct i2c_client *client)

> +static int max17040_get_online(struct max17040_chip *chip)

>  {

> -	struct max17040_chip *chip = i2c_get_clientdata(client);

> -

> -	if (chip->pdata && chip->pdata->battery_online)

> -		chip->online = chip->pdata->battery_online();

> -	else

> -		chip->online = 1;

> +	return chip->pdata && chip->pdata->battery_online ?

> +		chip->pdata->battery_online() : 1;

>  }

>  

> -static void max17040_get_status(struct i2c_client *client)

> +static int max17040_get_status(struct max17040_chip *chip)

>  {

> -	struct max17040_chip *chip = i2c_get_clientdata(client);

> -

>  	if (!chip->pdata || !chip->pdata->charger_online

> -			|| !chip->pdata->charger_enable) {

> -		chip->status = POWER_SUPPLY_STATUS_UNKNOWN;

> -		return;

> -	}

> +			|| !chip->pdata->charger_enable)

> +		return POWER_SUPPLY_STATUS_UNKNOWN;

> +

> +	if (max17040_get_soc(chip) > MAX17040_BATTERY_FULL)

> +		return POWER_SUPPLY_STATUS_FULL;

>  

> -	if (chip->pdata->charger_online()) {

> +	if (chip->pdata->charger_online())

>  		if (chip->pdata->charger_enable())

> -			chip->status = POWER_SUPPLY_STATUS_CHARGING;

> +			return POWER_SUPPLY_STATUS_CHARGING;

>  		else

> -			chip->status = POWER_SUPPLY_STATUS_NOT_CHARGING;

> -	} else {

> -		chip->status = POWER_SUPPLY_STATUS_DISCHARGING;

> -	}

> -

> -	if (chip->soc > MAX17040_BATTERY_FULL)

> -		chip->status = POWER_SUPPLY_STATUS_FULL;

> +			return POWER_SUPPLY_STATUS_NOT_CHARGING;

> +	else

> +		return POWER_SUPPLY_STATUS_DISCHARGING;

>  }

>  

>  static int max17040_get_of_data(struct max17040_chip *chip)

> @@ -190,18 +121,31 @@ static int max17040_get_of_data(struct max17040_chip *chip)

>  				 "maxim,alert-low-soc-level",

>  				 &chip->low_soc_alert);

>  

> -	if (chip->low_soc_alert <= 0 || chip->low_soc_alert >= 33)

> +	if (chip->low_soc_alert <= 0 || chip->low_soc_alert >= 33) {

> +		dev_err(dev, "maxim,alert-low-soc-level out of bounds\n");

>  		return -EINVAL;

> +	}

>  

>  	return 0;

>  }

>  

> -static void max17040_check_changes(struct i2c_client *client)

> +static void max17040_check_changes(struct max17040_chip *chip)

> +{

> +	chip->soc = max17040_get_soc(chip);

> +	chip->status = max17040_get_status(chip);

> +}

> +

> +static void max17040_queue_work(struct max17040_chip *chip)

> +{

> +	queue_delayed_work(system_power_efficient_wq, &chip->work,

> +			   MAX17040_DELAY);

> +}

> +

> +static void max17040_stop_work(void *data)

>  {

> -	max17040_get_vcell(client);

> -	max17040_get_soc(client);

> -	max17040_get_online(client);

> -	max17040_get_status(client);

> +	struct max17040_chip *chip = data;

> +

> +	cancel_delayed_work_sync(&chip->work);

>  }

>  

>  static void max17040_work(struct work_struct *work)

> @@ -214,30 +158,29 @@ static void max17040_work(struct work_struct *work)

>  	/* store SOC and status to check changes */

>  	last_soc = chip->soc;

>  	last_status = chip->status;

> -	max17040_check_changes(chip->client);

> +	max17040_check_changes(chip);

>  

>  	/* check changes and send uevent */

>  	if (last_soc != chip->soc || last_status != chip->status)

>  		power_supply_changed(chip->battery);

>  

> -	queue_delayed_work(system_power_efficient_wq, &chip->work,

> -			   MAX17040_DELAY);

> +	max17040_queue_work(chip);

>  }

>  

>  static irqreturn_t max17040_thread_handler(int id, void *dev)

>  {

>  	struct max17040_chip *chip = dev;

> -	struct i2c_client *client = chip->client;

>  

> -	dev_warn(&client->dev, "IRQ: Alert battery low level");

> +	dev_warn(&chip->client->dev, "IRQ: Alert battery low level");

> +

>  	/* read registers */

> -	max17040_check_changes(chip->client);

> +	max17040_check_changes(chip);

>  

>  	/* send uevent */

>  	power_supply_changed(chip->battery);

>  

>  	/* reset alert bit */

> -	max17040_set_low_soc_alert(client, chip->low_soc_alert);

> +	max17040_set_low_soc_alert(chip, chip->low_soc_alert);

>  

>  	return IRQ_HANDLED;

>  }

> @@ -256,6 +199,38 @@ static int max17040_enable_alert_irq(struct max17040_chip *chip)

>  	return ret;

>  }

>  

> +static int max17040_get_property(struct power_supply *psy,

> +			    enum power_supply_property psp,

> +			    union power_supply_propval *val)

> +{

> +	struct max17040_chip *chip = power_supply_get_drvdata(psy);

> +

> +	switch (psp) {

> +	case POWER_SUPPLY_PROP_STATUS:

> +		val->intval = max17040_get_status(chip);

> +		break;

> +	case POWER_SUPPLY_PROP_ONLINE:

> +		val->intval = max17040_get_online(chip);

> +		break;

> +	case POWER_SUPPLY_PROP_VOLTAGE_NOW:

> +		val->intval = max17040_get_vcell(chip);

> +		break;

> +	case POWER_SUPPLY_PROP_CAPACITY:

> +		val->intval = max17040_get_soc(chip);

> +		break;

> +	default:

> +		return -EINVAL;

> +	}

> +	return 0;

> +}

> +

> +static const struct regmap_config max17040_regmap = {

> +	.reg_bits	= 8,

> +	.reg_stride	= 2,

> +	.val_bits	= 16,

> +	.val_format_endian = REGMAP_ENDIAN_BIG,

> +};

> +

>  static enum power_supply_property max17040_battery_props[] = {

>  	POWER_SUPPLY_PROP_STATUS,

>  	POWER_SUPPLY_PROP_ONLINE,

> @@ -287,31 +262,33 @@ static int max17040_probe(struct i2c_client *client,

>  		return -ENOMEM;

>  

>  	chip->client = client;

> +	chip->regmap = devm_regmap_init_i2c(client, &max17040_regmap);

>  	chip->pdata = client->dev.platform_data;

>  	ret = max17040_get_of_data(chip);

> -	if (ret) {

> -		dev_err(&client->dev,

> -			"failed: low SOC alert OF data out of bounds\n");

> +	if (ret)

>  		return ret;

> -	}

>  

>  	i2c_set_clientdata(client, chip);

>  	psy_cfg.drv_data = chip;

>  

> -	chip->battery = power_supply_register(&client->dev,

> +	chip->battery = devm_power_supply_register(&client->dev,

>  				&max17040_battery_desc, &psy_cfg);

>  	if (IS_ERR(chip->battery)) {

>  		dev_err(&client->dev, "failed: power supply register\n");

>  		return PTR_ERR(chip->battery);

>  	}

>  

> -	max17040_reset(client);

> -	max17040_get_version(client);

> +	ret = max17040_get_version(chip);

> +	if (ret < 0)

> +		return ret;

> +	dev_dbg(&chip->client->dev, "MAX17040 Fuel-Gauge Ver 0x%x\n", ret);

> +

> +	max17040_reset(chip);

>  

>  	/* check interrupt */

>  	if (client->irq && of_device_is_compatible(client->dev.of_node,

>  						   "maxim,max77836-battery")) {

> -		ret = max17040_set_low_soc_alert(client, chip->low_soc_alert);

> +		ret = max17040_set_low_soc_alert(chip, chip->low_soc_alert);

>  		if (ret) {

>  			dev_err(&client->dev,

>  				"Failed to set low SOC alert: err %d\n", ret);

> @@ -327,18 +304,11 @@ static int max17040_probe(struct i2c_client *client,

>  	}

>  

>  	INIT_DEFERRABLE_WORK(&chip->work, max17040_work);

> -	queue_delayed_work(system_power_efficient_wq, &chip->work,

> -			   MAX17040_DELAY);

> -

> -	return 0;

> -}

> -

> -static int max17040_remove(struct i2c_client *client)

> -{

> -	struct max17040_chip *chip = i2c_get_clientdata(client);

> +	ret = devm_add_action(&client->dev, max17040_stop_work, chip);

> +	if (ret)

> +		return ret;

> +	max17040_queue_work(chip);

>  

> -	power_supply_unregister(chip->battery);

> -	cancel_delayed_work(&chip->work);

>  	return 0;

>  }

>  

> @@ -362,12 +332,11 @@ static int max17040_resume(struct device *dev)

>  	struct i2c_client *client = to_i2c_client(dev);

>  	struct max17040_chip *chip = i2c_get_clientdata(client);

>  

> -	queue_delayed_work(system_power_efficient_wq, &chip->work,

> -			   MAX17040_DELAY);

> -

>  	if (client->irq && device_may_wakeup(dev))

>  		disable_irq_wake(client->irq);

>  

> +	max17040_queue_work(chip);

> +

>  	return 0;

>  }

>  

> @@ -401,7 +370,6 @@ static struct i2c_driver max17040_i2c_driver = {

>  		.pm	= MAX17040_PM_OPS,

>  	},

>  	.probe		= max17040_probe,

> -	.remove		= max17040_remove,

>  	.id_table	= max17040_id,

>  };

>  module_i2c_driver(max17040_i2c_driver);

> -- 

> 2.27.0

>

Patch

diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index 44d3c8512fb8d..12ca79952768f 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -367,6 +367,7 @@  config AXP288_FUEL_GAUGE
 config BATTERY_MAX17040
 	tristate "Maxim MAX17040 Fuel Gauge"
 	depends on I2C
+	select REGMAP_I2C
 	help
 	  MAX17040 is fuel-gauge systems for lithium-ion (Li+) batteries
 	  in handheld and portable equipment. The MAX17040 is configured
diff --git a/drivers/power/supply/max17040_battery.c b/drivers/power/supply/max17040_battery.c
index 48aa44665e2f1..678241fcc2548 100644
--- a/drivers/power/supply/max17040_battery.c
+++ b/drivers/power/supply/max17040_battery.c
@@ -16,32 +16,30 @@ 
 #include <linux/interrupt.h>
 #include <linux/power_supply.h>
 #include <linux/max17040_battery.h>
+#include <linux/regmap.h>
 #include <linux/slab.h>
 
 #define MAX17040_VCELL	0x02
 #define MAX17040_SOC	0x04
 #define MAX17040_MODE	0x06
 #define MAX17040_VER	0x08
-#define MAX17040_RCOMP	0x0C
+#define MAX17040_CONFIG	0x0C
 #define MAX17040_CMD	0xFE
 
 
 #define MAX17040_DELAY		1000
 #define MAX17040_BATTERY_FULL	95
 
-#define MAX17040_ATHD_MASK		0xFFC0
+#define MAX17040_ATHD_MASK		0x3f
 #define MAX17040_ATHD_DEFAULT_POWER_UP	4
 
 struct max17040_chip {
 	struct i2c_client		*client;
+	struct regmap			*regmap;
 	struct delayed_work		work;
 	struct power_supply		*battery;
 	struct max17040_platform_data	*pdata;
 
-	/* State Of Connect */
-	int online;
-	/* battery voltage */
-	int vcell;
 	/* battery capacity */
 	int soc;
 	/* State Of Charge */
@@ -50,135 +48,68 @@  struct max17040_chip {
 	u32 low_soc_alert;
 };
 
-static int max17040_get_property(struct power_supply *psy,
-			    enum power_supply_property psp,
-			    union power_supply_propval *val)
-{
-	struct max17040_chip *chip = power_supply_get_drvdata(psy);
-
-	switch (psp) {
-	case POWER_SUPPLY_PROP_STATUS:
-		val->intval = chip->status;
-		break;
-	case POWER_SUPPLY_PROP_ONLINE:
-		val->intval = chip->online;
-		break;
-	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
-		val->intval = chip->vcell;
-		break;
-	case POWER_SUPPLY_PROP_CAPACITY:
-		val->intval = chip->soc;
-		break;
-	default:
-		return -EINVAL;
-	}
-	return 0;
-}
-
-static int max17040_write_reg(struct i2c_client *client, int reg, u16 value)
-{
-	int ret;
-
-	ret = i2c_smbus_write_word_swapped(client, reg, value);
-
-	if (ret < 0)
-		dev_err(&client->dev, "%s: err %d\n", __func__, ret);
-
-	return ret;
-}
-
-static int max17040_read_reg(struct i2c_client *client, int reg)
-{
-	int ret;
-
-	ret = i2c_smbus_read_word_swapped(client, reg);
-
-	if (ret < 0)
-		dev_err(&client->dev, "%s: err %d\n", __func__, ret);
-
-	return ret;
-}
-
-static void max17040_reset(struct i2c_client *client)
+static int max17040_reset(struct max17040_chip *chip)
 {
-	max17040_write_reg(client, MAX17040_CMD, 0x0054);
+	return regmap_write(chip->regmap, MAX17040_CMD, 0x0054);
 }
 
-static int max17040_set_low_soc_alert(struct i2c_client *client, u32 level)
+static int max17040_set_low_soc_alert(struct max17040_chip *chip, u32 level)
 {
-	int ret;
-	u16 data;
-
 	level = 32 - level;
-	data = max17040_read_reg(client, MAX17040_RCOMP);
-	/* clear the alrt bit and set LSb 5 bits */
-	data &= MAX17040_ATHD_MASK;
-	data |= level;
-	ret = max17040_write_reg(client, MAX17040_RCOMP, data);
-
-	return ret;
+	return regmap_update_bits(chip->regmap, MAX17040_CONFIG,
+			MAX17040_ATHD_MASK, level);
 }
 
-static void max17040_get_vcell(struct i2c_client *client)
+static int max17040_get_vcell(struct max17040_chip *chip)
 {
-	struct max17040_chip *chip = i2c_get_clientdata(client);
-	u16 vcell;
+	u32 vcell;
 
-	vcell = max17040_read_reg(client, MAX17040_VCELL);
+	regmap_read(chip->regmap, MAX17040_VCELL, &vcell);
 
-	chip->vcell = (vcell >> 4) * 1250;
+	return (vcell >> 4) * 1250;
 }
 
-static void max17040_get_soc(struct i2c_client *client)
+static int max17040_get_soc(struct max17040_chip *chip)
 {
-	struct max17040_chip *chip = i2c_get_clientdata(client);
-	u16 soc;
+	u32 soc;
 
-	soc = max17040_read_reg(client, MAX17040_SOC);
+	regmap_read(chip->regmap, MAX17040_SOC, &soc);
 
-	chip->soc = (soc >> 8);
+	return soc >> 8;
 }
 
-static void max17040_get_version(struct i2c_client *client)
+static int max17040_get_version(struct max17040_chip *chip)
 {
-	u16 version;
+	int ret;
+	u32 version;
 
-	version = max17040_read_reg(client, MAX17040_VER);
+	ret = regmap_read(chip->regmap, MAX17040_VER, &version);
 
-	dev_info(&client->dev, "MAX17040 Fuel-Gauge Ver 0x%x\n", version);
+	return ret ? ret : version;
 }
 
-static void max17040_get_online(struct i2c_client *client)
+static int max17040_get_online(struct max17040_chip *chip)
 {
-	struct max17040_chip *chip = i2c_get_clientdata(client);
-
-	if (chip->pdata && chip->pdata->battery_online)
-		chip->online = chip->pdata->battery_online();
-	else
-		chip->online = 1;
+	return chip->pdata && chip->pdata->battery_online ?
+		chip->pdata->battery_online() : 1;
 }
 
-static void max17040_get_status(struct i2c_client *client)
+static int max17040_get_status(struct max17040_chip *chip)
 {
-	struct max17040_chip *chip = i2c_get_clientdata(client);
-
 	if (!chip->pdata || !chip->pdata->charger_online
-			|| !chip->pdata->charger_enable) {
-		chip->status = POWER_SUPPLY_STATUS_UNKNOWN;
-		return;
-	}
+			|| !chip->pdata->charger_enable)
+		return POWER_SUPPLY_STATUS_UNKNOWN;
+
+	if (max17040_get_soc(chip) > MAX17040_BATTERY_FULL)
+		return POWER_SUPPLY_STATUS_FULL;
 
-	if (chip->pdata->charger_online()) {
+	if (chip->pdata->charger_online())
 		if (chip->pdata->charger_enable())
-			chip->status = POWER_SUPPLY_STATUS_CHARGING;
+			return POWER_SUPPLY_STATUS_CHARGING;
 		else
-			chip->status = POWER_SUPPLY_STATUS_NOT_CHARGING;
-	} else {
-		chip->status = POWER_SUPPLY_STATUS_DISCHARGING;
-	}
-
-	if (chip->soc > MAX17040_BATTERY_FULL)
-		chip->status = POWER_SUPPLY_STATUS_FULL;
+			return POWER_SUPPLY_STATUS_NOT_CHARGING;
+	else
+		return POWER_SUPPLY_STATUS_DISCHARGING;
 }
 
 static int max17040_get_of_data(struct max17040_chip *chip)
@@ -190,18 +121,31 @@  static int max17040_get_of_data(struct max17040_chip *chip)
 				 "maxim,alert-low-soc-level",
 				 &chip->low_soc_alert);
 
-	if (chip->low_soc_alert <= 0 || chip->low_soc_alert >= 33)
+	if (chip->low_soc_alert <= 0 || chip->low_soc_alert >= 33) {
+		dev_err(dev, "maxim,alert-low-soc-level out of bounds\n");
 		return -EINVAL;
+	}
 
 	return 0;
 }
 
-static void max17040_check_changes(struct i2c_client *client)
+static void max17040_check_changes(struct max17040_chip *chip)
+{
+	chip->soc = max17040_get_soc(chip);
+	chip->status = max17040_get_status(chip);
+}
+
+static void max17040_queue_work(struct max17040_chip *chip)
+{
+	queue_delayed_work(system_power_efficient_wq, &chip->work,
+			   MAX17040_DELAY);
+}
+
+static void max17040_stop_work(void *data)
 {
-	max17040_get_vcell(client);
-	max17040_get_soc(client);
-	max17040_get_online(client);
-	max17040_get_status(client);
+	struct max17040_chip *chip = data;
+
+	cancel_delayed_work_sync(&chip->work);
 }
 
 static void max17040_work(struct work_struct *work)
@@ -214,30 +158,29 @@  static void max17040_work(struct work_struct *work)
 	/* store SOC and status to check changes */
 	last_soc = chip->soc;
 	last_status = chip->status;
-	max17040_check_changes(chip->client);
+	max17040_check_changes(chip);
 
 	/* check changes and send uevent */
 	if (last_soc != chip->soc || last_status != chip->status)
 		power_supply_changed(chip->battery);
 
-	queue_delayed_work(system_power_efficient_wq, &chip->work,
-			   MAX17040_DELAY);
+	max17040_queue_work(chip);
 }
 
 static irqreturn_t max17040_thread_handler(int id, void *dev)
 {
 	struct max17040_chip *chip = dev;
-	struct i2c_client *client = chip->client;
 
-	dev_warn(&client->dev, "IRQ: Alert battery low level");
+	dev_warn(&chip->client->dev, "IRQ: Alert battery low level");
+
 	/* read registers */
-	max17040_check_changes(chip->client);
+	max17040_check_changes(chip);
 
 	/* send uevent */
 	power_supply_changed(chip->battery);
 
 	/* reset alert bit */
-	max17040_set_low_soc_alert(client, chip->low_soc_alert);
+	max17040_set_low_soc_alert(chip, chip->low_soc_alert);
 
 	return IRQ_HANDLED;
 }
@@ -256,6 +199,38 @@  static int max17040_enable_alert_irq(struct max17040_chip *chip)
 	return ret;
 }
 
+static int max17040_get_property(struct power_supply *psy,
+			    enum power_supply_property psp,
+			    union power_supply_propval *val)
+{
+	struct max17040_chip *chip = power_supply_get_drvdata(psy);
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_STATUS:
+		val->intval = max17040_get_status(chip);
+		break;
+	case POWER_SUPPLY_PROP_ONLINE:
+		val->intval = max17040_get_online(chip);
+		break;
+	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+		val->intval = max17040_get_vcell(chip);
+		break;
+	case POWER_SUPPLY_PROP_CAPACITY:
+		val->intval = max17040_get_soc(chip);
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static const struct regmap_config max17040_regmap = {
+	.reg_bits	= 8,
+	.reg_stride	= 2,
+	.val_bits	= 16,
+	.val_format_endian = REGMAP_ENDIAN_BIG,
+};
+
 static enum power_supply_property max17040_battery_props[] = {
 	POWER_SUPPLY_PROP_STATUS,
 	POWER_SUPPLY_PROP_ONLINE,
@@ -287,31 +262,33 @@  static int max17040_probe(struct i2c_client *client,
 		return -ENOMEM;
 
 	chip->client = client;
+	chip->regmap = devm_regmap_init_i2c(client, &max17040_regmap);
 	chip->pdata = client->dev.platform_data;
 	ret = max17040_get_of_data(chip);
-	if (ret) {
-		dev_err(&client->dev,
-			"failed: low SOC alert OF data out of bounds\n");
+	if (ret)
 		return ret;
-	}
 
 	i2c_set_clientdata(client, chip);
 	psy_cfg.drv_data = chip;
 
-	chip->battery = power_supply_register(&client->dev,
+	chip->battery = devm_power_supply_register(&client->dev,
 				&max17040_battery_desc, &psy_cfg);
 	if (IS_ERR(chip->battery)) {
 		dev_err(&client->dev, "failed: power supply register\n");
 		return PTR_ERR(chip->battery);
 	}
 
-	max17040_reset(client);
-	max17040_get_version(client);
+	ret = max17040_get_version(chip);
+	if (ret < 0)
+		return ret;
+	dev_dbg(&chip->client->dev, "MAX17040 Fuel-Gauge Ver 0x%x\n", ret);
+
+	max17040_reset(chip);
 
 	/* check interrupt */
 	if (client->irq && of_device_is_compatible(client->dev.of_node,
 						   "maxim,max77836-battery")) {
-		ret = max17040_set_low_soc_alert(client, chip->low_soc_alert);
+		ret = max17040_set_low_soc_alert(chip, chip->low_soc_alert);
 		if (ret) {
 			dev_err(&client->dev,
 				"Failed to set low SOC alert: err %d\n", ret);
@@ -327,18 +304,11 @@  static int max17040_probe(struct i2c_client *client,
 	}
 
 	INIT_DEFERRABLE_WORK(&chip->work, max17040_work);
-	queue_delayed_work(system_power_efficient_wq, &chip->work,
-			   MAX17040_DELAY);
-
-	return 0;
-}
-
-static int max17040_remove(struct i2c_client *client)
-{
-	struct max17040_chip *chip = i2c_get_clientdata(client);
+	ret = devm_add_action(&client->dev, max17040_stop_work, chip);
+	if (ret)
+		return ret;
+	max17040_queue_work(chip);
 
-	power_supply_unregister(chip->battery);
-	cancel_delayed_work(&chip->work);
 	return 0;
 }
 
@@ -362,12 +332,11 @@  static int max17040_resume(struct device *dev)
 	struct i2c_client *client = to_i2c_client(dev);
 	struct max17040_chip *chip = i2c_get_clientdata(client);
 
-	queue_delayed_work(system_power_efficient_wq, &chip->work,
-			   MAX17040_DELAY);
-
 	if (client->irq && device_may_wakeup(dev))
 		disable_irq_wake(client->irq);
 
+	max17040_queue_work(chip);
+
 	return 0;
 }
 
@@ -401,7 +370,6 @@  static struct i2c_driver max17040_i2c_driver = {
 		.pm	= MAX17040_PM_OPS,
 	},
 	.probe		= max17040_probe,
-	.remove		= max17040_remove,
 	.id_table	= max17040_id,
 };
 module_i2c_driver(max17040_i2c_driver);