Re: [PATCH 1/4] max17042: Move power suppply registration to a worker thread

Re: [PATCH 1/4] max17042: Move power suppply registration to a worker thread

[MyungJoo Ham]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=euc-kr, Size: 4257 bytes --]

Sender : dirk.brandewie@gmail.com<dirk.brandewie@gmail.com>
Date : 2011-11-29 04:49 (GMT+09:00)
Title : [PATCH 1/4] max17042: Move power suppply registration to a worker thread
> 
> From: Dirk Brandewie 
> 
> This patch move the final registration of the battery to a worker
> thread in preperation for adding the POR proceedure recommended by
> maxim. This is needed since the Maxim init proceedure requires two
> long delays totaling 850ms. This patch will reduce the impact on
> system boot time.  The battery will not be available to the power
> supply subsystem until the init proceedure is complete
> 
> Signed-off-by: Dirk Brandewie 

Hello.

Could you please move "ret = power_supply_register(&client->dev, &chip->battery);" back to max17042_probe()?

That will enable other depending devices to probe without much concerning about synchronizing probe time.

For the data availability, max17042 can return -EAGAIN with its property function when its init function is not complete.


Cheers!
MyungJoo.

> ---
> drivers/power/max17042_battery.c |   53 +++++++++++++++++++++++---------------
> 1 files changed, 32 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/power/max17042_battery.c b/drivers/power/max17042_battery.c
> index 9f0183c..33fdce0 100644
> --- a/drivers/power/max17042_battery.c
> +++ b/drivers/power/max17042_battery.c
> @@ -34,6 +34,7 @@ struct max17042_chip {
> struct i2c_client *client;
> struct power_supply battery;
> struct max17042_platform_data *pdata;
> + struct work_struct work;
> };
> 
> static int max17042_write_reg(struct i2c_client *client, u8 reg, u16 value)
> @@ -180,12 +181,40 @@ static int max17042_get_property(struct power_supply *psy,
> return 0;
> }
> 
> +static void max17042_init_worker(struct work_struct *work)
> +{
> + struct max17042_chip *chip = container_of(work,
> + struct max17042_chip, work);
> + struct i2c_client *client = chip->client;
> + int ret;
> +
> +
> + /* Initialize registers according to values from the platform data */
> + if (chip->pdata->init_data)
> + max17042_set_reg(client, chip->pdata->init_data,
> + chip->pdata->num_init_data);
> +
> + if (!chip->pdata->enable_current_sense) {
> + max17042_write_reg(client, MAX17042_CGAIN, 0x0000);
> + max17042_write_reg(client, MAX17042_MiscCFG, 0x0003);
> + max17042_write_reg(client, MAX17042_LearnCFG, 0x0007);
> + } else {
> + if (chip->pdata->r_sns == 0)
> + chip->pdata->r_sns = MAX17042_DEFAULT_SNS_RESISTOR;
> + }
> +
> + ret = power_supply_register(&client->dev, &chip->battery);
> + if (ret) {
> + dev_err(&client->dev, "failed: power supply register ");
> + kfree(chip);
> + }
> +}
> +
> static int __devinit max17042_probe(struct i2c_client *client,
> const struct i2c_device_id *id)
> {
> struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
> struct max17042_chip *chip;
> - int ret;
> 
> if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_WORD_DATA))
> return -EIO;
> @@ -210,26 +239,8 @@ static int __devinit max17042_probe(struct i2c_client *client,
> if (!chip->pdata->enable_current_sense)
> chip->battery.num_properties -= 2;
> 
> - ret = power_supply_register(&client->dev, &chip->battery);
> - if (ret) {
> - dev_err(&client->dev, "failed: power supply register ");
> - kfree(chip);
> - return ret;
> - }
> -
> - /* Initialize registers according to values from the platform data */
> - if (chip->pdata->init_data)
> - max17042_set_reg(client, chip->pdata->init_data,
> - chip->pdata->num_init_data);
> -
> - if (!chip->pdata->enable_current_sense) {
> - max17042_write_reg(client, MAX17042_CGAIN, 0x0000);
> - max17042_write_reg(client, MAX17042_MiscCFG, 0x0003);
> - max17042_write_reg(client, MAX17042_LearnCFG, 0x0007);
> - } else {
> - if (chip->pdata->r_sns == 0)
> - chip->pdata->r_sns = MAX17042_DEFAULT_SNS_RESISTOR;
> - }
> + INIT_WORK(&chip->work, max17042_init_worker);
> + schedule_work(&chip->work);
> 
> return 0;
> }
> -- 
> 1.7.7.3
> 
> 




 MyungJoo Ham (ÇÔ¸íÁÖ)

Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: +82-10-6714-2858 / office: +82-31-279-8033
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

[PATCH 0/4] MAX17042 add support for maxim POR procedure

[dirk.brandewie]

From: Dirk Brandewie <dirk.brandewie@gmail.com>

This patch set adds support for the power on reset procedure for the
max17042 battery fuel gauge.  The accuracy of the fuel gauge is
improved by configuring the fuel gauge with the characterization data
for the battery present in the platform.



Dirk Brandewie (4):
  max17042: Move power suppply registration to a worker thread
  max17042: Align register definitions with data sheet and init appnote
  max17042: Add POR init procedure from Maxim appnote
  x86-mrst: Add battery fuel guage platform data

 arch/x86/platform/mrst/mrst.c          |   46 ++++
 drivers/power/max17042_battery.c       |  401 ++++++++++++++++++++++++++++++--
 include/linux/power/max17042_battery.h |   93 +++++++-
 3 files changed, 510 insertions(+), 30 deletions(-)

-- 
1.7.7.3

[PATCH 1/4] max17042: Move power suppply registration to a worker thread

[dirk.brandewie]

From: Dirk Brandewie <dirk.brandewie@gmail.com>

This patch move the final registration of the battery to a worker
thread in preperation for adding the POR proceedure recommended by
maxim. This is needed since the Maxim init proceedure requires two
long delays totaling 850ms. This patch will reduce the impact on
system boot time.  The battery will not be available to the power
supply subsystem until the init proceedure is complete

Signed-off-by: Dirk Brandewie <dirk.brandewie@gmail.com>
---
 drivers/power/max17042_battery.c |   53 +++++++++++++++++++++++---------------
 1 files changed, 32 insertions(+), 21 deletions(-)

diff --git a/drivers/power/max17042_battery.c b/drivers/power/max17042_battery.c
index 9f0183c..33fdce0 100644
--- a/drivers/power/max17042_battery.c
+++ b/drivers/power/max17042_battery.c
@@ -34,6 +34,7 @@ struct max17042_chip {
 	struct i2c_client *client;
 	struct power_supply battery;
 	struct max17042_platform_data *pdata;
+	struct work_struct work;
 };
 
 static int max17042_write_reg(struct i2c_client *client, u8 reg, u16 value)
@@ -180,12 +181,40 @@ static int max17042_get_property(struct power_supply *psy,
 	return 0;
 }
 
+static void max17042_init_worker(struct work_struct *work)
+{
+	struct max17042_chip *chip = container_of(work,
+				struct max17042_chip, work);
+	struct i2c_client *client = chip->client;
+	int ret;
+
+
+	/* Initialize registers according to values from the platform data */
+	if (chip->pdata->init_data)
+		max17042_set_reg(client, chip->pdata->init_data,
+				 chip->pdata->num_init_data);
+
+	if (!chip->pdata->enable_current_sense) {
+		max17042_write_reg(client, MAX17042_CGAIN, 0x0000);
+		max17042_write_reg(client, MAX17042_MiscCFG, 0x0003);
+		max17042_write_reg(client, MAX17042_LearnCFG, 0x0007);
+	} else {
+		if (chip->pdata->r_sns == 0)
+			chip->pdata->r_sns = MAX17042_DEFAULT_SNS_RESISTOR;
+	}
+
+	ret = power_supply_register(&client->dev, &chip->battery);
+	if (ret) {
+		dev_err(&client->dev, "failed: power supply register\n");
+		kfree(chip);
+	}
+}
+
 static int __devinit max17042_probe(struct i2c_client *client,
 			const struct i2c_device_id *id)
 {
 	struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
 	struct max17042_chip *chip;
-	int ret;
 
 	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_WORD_DATA))
 		return -EIO;
@@ -210,26 +239,8 @@ static int __devinit max17042_probe(struct i2c_client *client,
 	if (!chip->pdata->enable_current_sense)
 		chip->battery.num_properties -= 2;
 
-	ret = power_supply_register(&client->dev, &chip->battery);
-	if (ret) {
-		dev_err(&client->dev, "failed: power supply register\n");
-		kfree(chip);
-		return ret;
-	}
-
-	/* Initialize registers according to values from the platform data */
-	if (chip->pdata->init_data)
-		max17042_set_reg(client, chip->pdata->init_data,
-				 chip->pdata->num_init_data);
-
-	if (!chip->pdata->enable_current_sense) {
-		max17042_write_reg(client, MAX17042_CGAIN, 0x0000);
-		max17042_write_reg(client, MAX17042_MiscCFG, 0x0003);
-		max17042_write_reg(client, MAX17042_LearnCFG, 0x0007);
-	} else {
-		if (chip->pdata->r_sns == 0)
-			chip->pdata->r_sns = MAX17042_DEFAULT_SNS_RESISTOR;
-	}
+	INIT_WORK(&chip->work, max17042_init_worker);
+	schedule_work(&chip->work);
 
 	return 0;
 }
-- 
1.7.7.3