All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] bq2415x_charger: Allow to load and use driver even if notify device is not registered yet
@ 2015-07-25 21:46 Pali Rohár
  2015-08-05 13:26 ` Pali Rohár
  2015-08-11 11:22 ` [PATCH v2] " Pali Rohár
  0 siblings, 2 replies; 4+ messages in thread
From: Pali Rohár @ 2015-07-25 21:46 UTC (permalink / raw
  To: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse
  Cc: linux-pm, linux-kernel, Pali Rohár

Driver bq2415x_charger works also without notify power supply device for
charger detection. But when charger detection is specified in DT, then
bq2415x_charger refused to loaded with -EPROBE_DEFER.

This patch rewrites code so that notify device for charger detection is
checked when power supply event is received and not when registering power
supply device. So this patch allows to use bq2415x_charger driver also when
kernel is compiled without driver for notify power supply device.

Now after this patch scheduled workqueue is called after INIT_DELAYED_WORK,
so it also fix problem when scheduled workqueue was called before init.

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/power/bq2415x_charger.c |  143 +++++++++++++++++++++------------------
 1 file changed, 79 insertions(+), 64 deletions(-)

diff --git a/drivers/power/bq2415x_charger.c b/drivers/power/bq2415x_charger.c
index e98dcb6..739ef5b 100644
--- a/drivers/power/bq2415x_charger.c
+++ b/drivers/power/bq2415x_charger.c
@@ -170,7 +170,7 @@ struct bq2415x_device {
 	struct power_supply *charger;
 	struct power_supply_desc charger_desc;
 	struct delayed_work work;
-	struct power_supply *notify_psy;
+	struct device_node *notify_node;
 	struct notifier_block nb;
 	enum bq2415x_mode reported_mode;/* mode reported by hook function */
 	enum bq2415x_mode mode;		/* currently configured mode */
@@ -792,22 +792,47 @@ static int bq2415x_set_mode(struct bq2415x_device *bq, enum bq2415x_mode mode)
 
 }
 
+static bool bq2415x_update_reported_mode(struct bq2415x_device *bq, int mA)
+{
+	enum bq2415x_mode mode;
+
+	if (mA == 0)
+		mode = BQ2415X_MODE_OFF;
+	else if (mA < 500)
+		mode = BQ2415X_MODE_NONE;
+	else if (mA < 1800)
+		mode = BQ2415X_MODE_HOST_CHARGER;
+	else
+		mode = BQ2415X_MODE_DEDICATED_CHARGER;
+
+	if (bq->reported_mode == mode)
+		return false;
+
+	bq->reported_mode = mode;
+	return true;
+}
+
 static int bq2415x_notifier_call(struct notifier_block *nb,
 		unsigned long val, void *v)
 {
 	struct bq2415x_device *bq =
 		container_of(nb, struct bq2415x_device, nb);
 	struct power_supply *psy = v;
-	enum bq2415x_mode mode;
 	union power_supply_propval prop;
 	int ret;
-	int mA;
 
 	if (val != PSY_EVENT_PROP_CHANGED)
 		return NOTIFY_OK;
 
-	if (psy != bq->notify_psy)
-		return NOTIFY_OK;
+	/* Ignore event if it was not send by notify_node/notify_device */
+	if (bq->notify_node) {
+		if (psy->dev.parent &&
+		    psy->dev.parent->of_node != bq->notify_node)
+			return NOTIFY_OK;
+	} else if (bq->init_data.notify_device) {
+		if (strcmp(psy->desc->name, bq->init_data.notify_device) != 0)
+			return NOTIFY_OK;
+	}
 
 	dev_dbg(bq->dev, "notifier call was called\n");
 
@@ -816,22 +841,9 @@ static int bq2415x_notifier_call(struct notifier_block *nb,
 	if (ret != 0)
 		return NOTIFY_OK;
 
-	mA = prop.intval;
-
-	if (mA == 0)
-		mode = BQ2415X_MODE_OFF;
-	else if (mA < 500)
-		mode = BQ2415X_MODE_NONE;
-	else if (mA < 1800)
-		mode = BQ2415X_MODE_HOST_CHARGER;
-	else
-		mode = BQ2415X_MODE_DEDICATED_CHARGER;
-
-	if (bq->reported_mode == mode)
+	if (!bq2415x_update_reported_mode(bq, prop.intval))
 		return NOTIFY_OK;
 
-	bq->reported_mode = mode;
-
 	/* if automode is not enabled do not tell about reported_mode */
 	if (bq->automode < 1)
 		return NOTIFY_OK;
@@ -1536,6 +1548,8 @@ static int bq2415x_probe(struct i2c_client *client,
 	struct device_node *np = client->dev.of_node;
 	struct bq2415x_platform_data *pdata = client->dev.platform_data;
 	const struct acpi_device_id *acpi_id = NULL;
+	struct power_supply *notify_psy = NULL;
+	union power_supply_propval prop;
 
 	if (!np && !pdata && !ACPI_HANDLE(&client->dev)) {
 		dev_err(&client->dev, "Neither devicetree, nor platform data, nor ACPI support\n");
@@ -1569,25 +1583,6 @@ static int bq2415x_probe(struct i2c_client *client,
 		goto error_2;
 	}
 
-	if (np) {
-		bq->notify_psy = power_supply_get_by_phandle(np,
-						"ti,usb-charger-detection");
-
-		if (IS_ERR(bq->notify_psy)) {
-			dev_info(&client->dev,
-				 "no 'ti,usb-charger-detection' property (err=%ld)\n",
-				PTR_ERR(bq->notify_psy));
-			bq->notify_psy = NULL;
-		} else if (!bq->notify_psy) {
-			ret = -EPROBE_DEFER;
-			goto error_2;
-		}
-	} else if (pdata && pdata->notify_device) {
-		bq->notify_psy = power_supply_get_by_name(pdata->notify_device);
-	} else {
-		bq->notify_psy = NULL;
-	}
-
 	i2c_set_clientdata(client, bq);
 
 	bq->id = num;
@@ -1607,32 +1602,35 @@ static int bq2415x_probe(struct i2c_client *client,
 					       "ti,current-limit",
 					       &bq->init_data.current_limit);
 		if (ret)
-			goto error_3;
+			goto error_2;
 		ret = device_property_read_u32(bq->dev,
 					"ti,weak-battery-voltage",
 					&bq->init_data.weak_battery_voltage);
 		if (ret)
-			goto error_3;
+			goto error_2;
 		ret = device_property_read_u32(bq->dev,
 				"ti,battery-regulation-voltage",
 				&bq->init_data.battery_regulation_voltage);
 		if (ret)
-			goto error_3;
+			goto error_2;
 		ret = device_property_read_u32(bq->dev,
 					       "ti,charge-current",
 					       &bq->init_data.charge_current);
 		if (ret)
-			goto error_3;
+			goto error_2;
 		ret = device_property_read_u32(bq->dev,
 				"ti,termination-current",
 				&bq->init_data.termination_current);
 		if (ret)
-			goto error_3;
+			goto error_2;
 		ret = device_property_read_u32(bq->dev,
 					       "ti,resistor-sense",
 					       &bq->init_data.resistor_sense);
 		if (ret)
-			goto error_3;
+			goto error_2;
+		if (np)
+			bq->notify_node = of_parse_phandle(np,
+						"ti,usb-charger-detection", 0);
 	} else {
 		memcpy(&bq->init_data, pdata, sizeof(bq->init_data));
 	}
@@ -1642,56 +1640,72 @@ static int bq2415x_probe(struct i2c_client *client,
 	ret = bq2415x_power_supply_init(bq);
 	if (ret) {
 		dev_err(bq->dev, "failed to register power supply: %d\n", ret);
-		goto error_3;
+		goto error_2;
 	}
 
 	ret = bq2415x_sysfs_init(bq);
 	if (ret) {
 		dev_err(bq->dev, "failed to create sysfs entries: %d\n", ret);
-		goto error_4;
+		goto error_3;
 	}
 
 	ret = bq2415x_set_defaults(bq);
 	if (ret) {
 		dev_err(bq->dev, "failed to set default values: %d\n", ret);
-		goto error_5;
+		goto error_4;
 	}
 
-	if (bq->notify_psy) {
+	if (bq->notify_node || bq->init_data.notify_device) {
 		bq->nb.notifier_call = bq2415x_notifier_call;
 		ret = power_supply_reg_notifier(&bq->nb);
 		if (ret) {
 			dev_err(bq->dev, "failed to reg notifier: %d\n", ret);
-			goto error_6;
+			goto error_4;
 		}
 
-		/* Query for initial reported_mode and set it */
-		bq2415x_notifier_call(&bq->nb, PSY_EVENT_PROP_CHANGED,
-				      bq->notify_psy);
-		bq2415x_set_mode(bq, bq->reported_mode);
-
 		bq->automode = 1;
-		dev_info(bq->dev, "automode enabled\n");
+		dev_info(bq->dev, "automode supported, waiting for events\n");
 	} else {
 		bq->automode = -1;
 		dev_info(bq->dev, "automode not supported\n");
 	}
 
+	/* Query for initial reported_mode and set it */
+	if (bq->nb.notifier_call) {
+		if (np) {
+			notify_psy = power_supply_get_by_phandle(np,
+						"ti,usb-charger-detection");
+			if (IS_ERR(notify_psy))
+				notify_psy = NULL;
+		} else if (bq->init_data.notify_device) {
+			notify_psy = power_supply_get_by_name(
+						bq->init_data.notify_device);
+		}
+	}
+	if (notify_psy) {
+		ret = power_supply_get_property(notify_psy,
+					POWER_SUPPLY_PROP_CURRENT_MAX, &prop);
+		power_supply_put(notify_psy);
+
+		if (ret == 0) {
+			bq2415x_update_reported_mode(bq, prop.intval);
+			bq2415x_set_mode(bq, bq->reported_mode);
+		}
+	}
+
 	INIT_DELAYED_WORK(&bq->work, bq2415x_timer_work);
 	bq2415x_set_autotimer(bq, 1);
 
 	dev_info(bq->dev, "driver registered\n");
 	return 0;
 
-error_6:
-error_5:
-	bq2415x_sysfs_exit(bq);
 error_4:
-	bq2415x_power_supply_exit(bq);
+	bq2415x_sysfs_exit(bq);
 error_3:
-	if (bq->notify_psy)
-		power_supply_put(bq->notify_psy);
+	bq2415x_power_supply_exit(bq);
 error_2:
+	if (bq->notify_node)
+		of_node_put(bq->notify_node);
 	kfree(name);
 error_1:
 	mutex_lock(&bq2415x_id_mutex);
@@ -1707,10 +1721,11 @@ static int bq2415x_remove(struct i2c_client *client)
 {
 	struct bq2415x_device *bq = i2c_get_clientdata(client);
 
-	if (bq->notify_psy) {
+	if (bq->nb.notifier_call)
 		power_supply_unreg_notifier(&bq->nb);
-		power_supply_put(bq->notify_psy);
-	}
+
+	if (bq->notify_node)
+		of_node_put(bq->notify_node);
 
 	bq2415x_sysfs_exit(bq);
 	bq2415x_power_supply_exit(bq);
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] bq2415x_charger: Allow to load and use driver even if notify device is not registered yet
  2015-07-25 21:46 [PATCH] bq2415x_charger: Allow to load and use driver even if notify device is not registered yet Pali Rohár
@ 2015-08-05 13:26 ` Pali Rohár
  2015-08-11 11:23   ` Pali Rohár
  2015-08-11 11:22 ` [PATCH v2] " Pali Rohár
  1 sibling, 1 reply; 4+ messages in thread
From: Pali Rohár @ 2015-08-05 13:26 UTC (permalink / raw
  To: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse
  Cc: linux-pm, linux-kernel

On Saturday 25 July 2015 23:46:56 Pali Rohár wrote:
> Driver bq2415x_charger works also without notify power supply device for
> charger detection. But when charger detection is specified in DT, then
> bq2415x_charger refused to loaded with -EPROBE_DEFER.
> 
> This patch rewrites code so that notify device for charger detection is
> checked when power supply event is received and not when registering power
> supply device. So this patch allows to use bq2415x_charger driver also when
> kernel is compiled without driver for notify power supply device.
> 
> Now after this patch scheduled workqueue is called after INIT_DELAYED_WORK,
> so it also fix problem when scheduled workqueue was called before init.
> 
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> ---
>  drivers/power/bq2415x_charger.c |  143 +++++++++++++++++++++------------------
>  1 file changed, 79 insertions(+), 64 deletions(-)
> 
> diff --git a/drivers/power/bq2415x_charger.c b/drivers/power/bq2415x_charger.c
> index e98dcb6..739ef5b 100644
> --- a/drivers/power/bq2415x_charger.c
> +++ b/drivers/power/bq2415x_charger.c
> @@ -170,7 +170,7 @@ struct bq2415x_device {
>  	struct power_supply *charger;
>  	struct power_supply_desc charger_desc;
>  	struct delayed_work work;
> -	struct power_supply *notify_psy;
> +	struct device_node *notify_node;
>  	struct notifier_block nb;
>  	enum bq2415x_mode reported_mode;/* mode reported by hook function */
>  	enum bq2415x_mode mode;		/* currently configured mode */
> @@ -792,22 +792,47 @@ static int bq2415x_set_mode(struct bq2415x_device *bq, enum bq2415x_mode mode)
>  
>  }
>  
> +static bool bq2415x_update_reported_mode(struct bq2415x_device *bq, int mA)
> +{
> +	enum bq2415x_mode mode;
> +
> +	if (mA == 0)
> +		mode = BQ2415X_MODE_OFF;
> +	else if (mA < 500)
> +		mode = BQ2415X_MODE_NONE;
> +	else if (mA < 1800)
> +		mode = BQ2415X_MODE_HOST_CHARGER;
> +	else
> +		mode = BQ2415X_MODE_DEDICATED_CHARGER;
> +
> +	if (bq->reported_mode == mode)
> +		return false;
> +
> +	bq->reported_mode = mode;
> +	return true;
> +}
> +
>  static int bq2415x_notifier_call(struct notifier_block *nb,
>  		unsigned long val, void *v)
>  {
>  	struct bq2415x_device *bq =
>  		container_of(nb, struct bq2415x_device, nb);
>  	struct power_supply *psy = v;
> -	enum bq2415x_mode mode;
>  	union power_supply_propval prop;
>  	int ret;
> -	int mA;
>  
>  	if (val != PSY_EVENT_PROP_CHANGED)
>  		return NOTIFY_OK;
>  
> -	if (psy != bq->notify_psy)
> -		return NOTIFY_OK;
> +	/* Ignore event if it was not send by notify_node/notify_device */
> +	if (bq->notify_node) {
> +		if (psy->dev.parent &&
> +		    psy->dev.parent->of_node != bq->notify_node)
> +			return NOTIFY_OK;

There is missing branch for case when psy->dev.parent is NULL. Logical
error... I will send new version of patch.

Correct logic should be: ignore psy dev which sent event if it is not
notify dev specified in board/DT config of bq2415x psy dev.

> +	} else if (bq->init_data.notify_device) {
> +		if (strcmp(psy->desc->name, bq->init_data.notify_device) != 0)
> +			return NOTIFY_OK;
> +	}
>  
>  	dev_dbg(bq->dev, "notifier call was called\n");
>  
> @@ -816,22 +841,9 @@ static int bq2415x_notifier_call(struct notifier_block *nb,
>  	if (ret != 0)
>  		return NOTIFY_OK;
>  
> -	mA = prop.intval;
> -
> -	if (mA == 0)
> -		mode = BQ2415X_MODE_OFF;
> -	else if (mA < 500)
> -		mode = BQ2415X_MODE_NONE;
> -	else if (mA < 1800)
> -		mode = BQ2415X_MODE_HOST_CHARGER;
> -	else
> -		mode = BQ2415X_MODE_DEDICATED_CHARGER;
> -
> -	if (bq->reported_mode == mode)
> +	if (!bq2415x_update_reported_mode(bq, prop.intval))
>  		return NOTIFY_OK;
>  
> -	bq->reported_mode = mode;
> -
>  	/* if automode is not enabled do not tell about reported_mode */
>  	if (bq->automode < 1)
>  		return NOTIFY_OK;
> @@ -1536,6 +1548,8 @@ static int bq2415x_probe(struct i2c_client *client,
>  	struct device_node *np = client->dev.of_node;
>  	struct bq2415x_platform_data *pdata = client->dev.platform_data;
>  	const struct acpi_device_id *acpi_id = NULL;
> +	struct power_supply *notify_psy = NULL;
> +	union power_supply_propval prop;
>  
>  	if (!np && !pdata && !ACPI_HANDLE(&client->dev)) {
>  		dev_err(&client->dev, "Neither devicetree, nor platform data, nor ACPI support\n");
> @@ -1569,25 +1583,6 @@ static int bq2415x_probe(struct i2c_client *client,
>  		goto error_2;
>  	}
>  
> -	if (np) {
> -		bq->notify_psy = power_supply_get_by_phandle(np,
> -						"ti,usb-charger-detection");
> -
> -		if (IS_ERR(bq->notify_psy)) {
> -			dev_info(&client->dev,
> -				 "no 'ti,usb-charger-detection' property (err=%ld)\n",
> -				PTR_ERR(bq->notify_psy));
> -			bq->notify_psy = NULL;
> -		} else if (!bq->notify_psy) {
> -			ret = -EPROBE_DEFER;
> -			goto error_2;
> -		}
> -	} else if (pdata && pdata->notify_device) {
> -		bq->notify_psy = power_supply_get_by_name(pdata->notify_device);
> -	} else {
> -		bq->notify_psy = NULL;
> -	}
> -
>  	i2c_set_clientdata(client, bq);
>  
>  	bq->id = num;
> @@ -1607,32 +1602,35 @@ static int bq2415x_probe(struct i2c_client *client,
>  					       "ti,current-limit",
>  					       &bq->init_data.current_limit);
>  		if (ret)
> -			goto error_3;
> +			goto error_2;
>  		ret = device_property_read_u32(bq->dev,
>  					"ti,weak-battery-voltage",
>  					&bq->init_data.weak_battery_voltage);
>  		if (ret)
> -			goto error_3;
> +			goto error_2;
>  		ret = device_property_read_u32(bq->dev,
>  				"ti,battery-regulation-voltage",
>  				&bq->init_data.battery_regulation_voltage);
>  		if (ret)
> -			goto error_3;
> +			goto error_2;
>  		ret = device_property_read_u32(bq->dev,
>  					       "ti,charge-current",
>  					       &bq->init_data.charge_current);
>  		if (ret)
> -			goto error_3;
> +			goto error_2;
>  		ret = device_property_read_u32(bq->dev,
>  				"ti,termination-current",
>  				&bq->init_data.termination_current);
>  		if (ret)
> -			goto error_3;
> +			goto error_2;
>  		ret = device_property_read_u32(bq->dev,
>  					       "ti,resistor-sense",
>  					       &bq->init_data.resistor_sense);
>  		if (ret)
> -			goto error_3;
> +			goto error_2;
> +		if (np)
> +			bq->notify_node = of_parse_phandle(np,
> +						"ti,usb-charger-detection", 0);
>  	} else {
>  		memcpy(&bq->init_data, pdata, sizeof(bq->init_data));
>  	}
> @@ -1642,56 +1640,72 @@ static int bq2415x_probe(struct i2c_client *client,
>  	ret = bq2415x_power_supply_init(bq);
>  	if (ret) {
>  		dev_err(bq->dev, "failed to register power supply: %d\n", ret);
> -		goto error_3;
> +		goto error_2;
>  	}
>  
>  	ret = bq2415x_sysfs_init(bq);
>  	if (ret) {
>  		dev_err(bq->dev, "failed to create sysfs entries: %d\n", ret);
> -		goto error_4;
> +		goto error_3;
>  	}
>  
>  	ret = bq2415x_set_defaults(bq);
>  	if (ret) {
>  		dev_err(bq->dev, "failed to set default values: %d\n", ret);
> -		goto error_5;
> +		goto error_4;
>  	}
>  
> -	if (bq->notify_psy) {
> +	if (bq->notify_node || bq->init_data.notify_device) {
>  		bq->nb.notifier_call = bq2415x_notifier_call;
>  		ret = power_supply_reg_notifier(&bq->nb);
>  		if (ret) {
>  			dev_err(bq->dev, "failed to reg notifier: %d\n", ret);
> -			goto error_6;
> +			goto error_4;
>  		}
>  
> -		/* Query for initial reported_mode and set it */
> -		bq2415x_notifier_call(&bq->nb, PSY_EVENT_PROP_CHANGED,
> -				      bq->notify_psy);
> -		bq2415x_set_mode(bq, bq->reported_mode);
> -
>  		bq->automode = 1;
> -		dev_info(bq->dev, "automode enabled\n");
> +		dev_info(bq->dev, "automode supported, waiting for events\n");
>  	} else {
>  		bq->automode = -1;
>  		dev_info(bq->dev, "automode not supported\n");
>  	}
>  
> +	/* Query for initial reported_mode and set it */
> +	if (bq->nb.notifier_call) {
> +		if (np) {
> +			notify_psy = power_supply_get_by_phandle(np,
> +						"ti,usb-charger-detection");
> +			if (IS_ERR(notify_psy))
> +				notify_psy = NULL;
> +		} else if (bq->init_data.notify_device) {
> +			notify_psy = power_supply_get_by_name(
> +						bq->init_data.notify_device);
> +		}
> +	}
> +	if (notify_psy) {
> +		ret = power_supply_get_property(notify_psy,
> +					POWER_SUPPLY_PROP_CURRENT_MAX, &prop);
> +		power_supply_put(notify_psy);
> +
> +		if (ret == 0) {
> +			bq2415x_update_reported_mode(bq, prop.intval);
> +			bq2415x_set_mode(bq, bq->reported_mode);
> +		}
> +	}
> +
>  	INIT_DELAYED_WORK(&bq->work, bq2415x_timer_work);
>  	bq2415x_set_autotimer(bq, 1);
>  
>  	dev_info(bq->dev, "driver registered\n");
>  	return 0;
>  
> -error_6:
> -error_5:
> -	bq2415x_sysfs_exit(bq);
>  error_4:
> -	bq2415x_power_supply_exit(bq);
> +	bq2415x_sysfs_exit(bq);
>  error_3:
> -	if (bq->notify_psy)
> -		power_supply_put(bq->notify_psy);
> +	bq2415x_power_supply_exit(bq);
>  error_2:
> +	if (bq->notify_node)
> +		of_node_put(bq->notify_node);
>  	kfree(name);
>  error_1:
>  	mutex_lock(&bq2415x_id_mutex);
> @@ -1707,10 +1721,11 @@ static int bq2415x_remove(struct i2c_client *client)
>  {
>  	struct bq2415x_device *bq = i2c_get_clientdata(client);
>  
> -	if (bq->notify_psy) {
> +	if (bq->nb.notifier_call)
>  		power_supply_unreg_notifier(&bq->nb);
> -		power_supply_put(bq->notify_psy);
> -	}
> +
> +	if (bq->notify_node)
> +		of_node_put(bq->notify_node);
>  
>  	bq2415x_sysfs_exit(bq);
>  	bq2415x_power_supply_exit(bq);

-- 
Pali Rohár
pali.rohar@gmail.com

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2] bq2415x_charger: Allow to load and use driver even if notify device is not registered yet
  2015-07-25 21:46 [PATCH] bq2415x_charger: Allow to load and use driver even if notify device is not registered yet Pali Rohár
  2015-08-05 13:26 ` Pali Rohár
@ 2015-08-11 11:22 ` Pali Rohár
  1 sibling, 0 replies; 4+ messages in thread
From: Pali Rohár @ 2015-08-11 11:22 UTC (permalink / raw
  To: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse
  Cc: linux-pm, linux-kernel, Pali Rohár

Driver bq2415x_charger works also without notify power supply device for
charger detection. But when charger detection is specified in DT, then
bq2415x_charger refused to loaded with -EPROBE_DEFER.

This patch rewrites code so that notify device for charger detection is
checked when power supply event is received and not when registering power
supply device. So this patch allows to use bq2415x_charger driver also when
kernel is compiled without driver for notify power supply device.

Now after this patch scheduled workqueue is called after INIT_DELAYED_WORK,
so it also fix problem when scheduled workqueue was called before init.

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/power/bq2415x_charger.c |  143 +++++++++++++++++++++------------------
 1 file changed, 79 insertions(+), 64 deletions(-)

diff --git a/drivers/power/bq2415x_charger.c b/drivers/power/bq2415x_charger.c
index e98dcb6..ec212b5 100644
--- a/drivers/power/bq2415x_charger.c
+++ b/drivers/power/bq2415x_charger.c
@@ -170,7 +170,7 @@ struct bq2415x_device {
 	struct power_supply *charger;
 	struct power_supply_desc charger_desc;
 	struct delayed_work work;
-	struct power_supply *notify_psy;
+	struct device_node *notify_node;
 	struct notifier_block nb;
 	enum bq2415x_mode reported_mode;/* mode reported by hook function */
 	enum bq2415x_mode mode;		/* currently configured mode */
@@ -792,22 +792,47 @@ static int bq2415x_set_mode(struct bq2415x_device *bq, enum bq2415x_mode mode)
 
 }
 
+static bool bq2415x_update_reported_mode(struct bq2415x_device *bq, int mA)
+{
+	enum bq2415x_mode mode;
+
+	if (mA == 0)
+		mode = BQ2415X_MODE_OFF;
+	else if (mA < 500)
+		mode = BQ2415X_MODE_NONE;
+	else if (mA < 1800)
+		mode = BQ2415X_MODE_HOST_CHARGER;
+	else
+		mode = BQ2415X_MODE_DEDICATED_CHARGER;
+
+	if (bq->reported_mode == mode)
+		return false;
+
+	bq->reported_mode = mode;
+	return true;
+}
+
 static int bq2415x_notifier_call(struct notifier_block *nb,
 		unsigned long val, void *v)
 {
 	struct bq2415x_device *bq =
 		container_of(nb, struct bq2415x_device, nb);
 	struct power_supply *psy = v;
-	enum bq2415x_mode mode;
 	union power_supply_propval prop;
 	int ret;
-	int mA;
 
 	if (val != PSY_EVENT_PROP_CHANGED)
 		return NOTIFY_OK;
 
-	if (psy != bq->notify_psy)
-		return NOTIFY_OK;
+	/* Ignore event if it was not send by notify_node/notify_device */
+	if (bq->notify_node) {
+		if (!psy->dev.parent ||
+		    psy->dev.parent->of_node != bq->notify_node)
+			return NOTIFY_OK;
+	} else if (bq->init_data.notify_device) {
+		if (strcmp(psy->desc->name, bq->init_data.notify_device) != 0)
+			return NOTIFY_OK;
+	}
 
 	dev_dbg(bq->dev, "notifier call was called\n");
 
@@ -816,22 +841,9 @@ static int bq2415x_notifier_call(struct notifier_block *nb,
 	if (ret != 0)
 		return NOTIFY_OK;
 
-	mA = prop.intval;
-
-	if (mA == 0)
-		mode = BQ2415X_MODE_OFF;
-	else if (mA < 500)
-		mode = BQ2415X_MODE_NONE;
-	else if (mA < 1800)
-		mode = BQ2415X_MODE_HOST_CHARGER;
-	else
-		mode = BQ2415X_MODE_DEDICATED_CHARGER;
-
-	if (bq->reported_mode == mode)
+	if (!bq2415x_update_reported_mode(bq, prop.intval))
 		return NOTIFY_OK;
 
-	bq->reported_mode = mode;
-
 	/* if automode is not enabled do not tell about reported_mode */
 	if (bq->automode < 1)
 		return NOTIFY_OK;
@@ -1536,6 +1548,8 @@ static int bq2415x_probe(struct i2c_client *client,
 	struct device_node *np = client->dev.of_node;
 	struct bq2415x_platform_data *pdata = client->dev.platform_data;
 	const struct acpi_device_id *acpi_id = NULL;
+	struct power_supply *notify_psy = NULL;
+	union power_supply_propval prop;
 
 	if (!np && !pdata && !ACPI_HANDLE(&client->dev)) {
 		dev_err(&client->dev, "Neither devicetree, nor platform data, nor ACPI support\n");
@@ -1569,25 +1583,6 @@ static int bq2415x_probe(struct i2c_client *client,
 		goto error_2;
 	}
 
-	if (np) {
-		bq->notify_psy = power_supply_get_by_phandle(np,
-						"ti,usb-charger-detection");
-
-		if (IS_ERR(bq->notify_psy)) {
-			dev_info(&client->dev,
-				 "no 'ti,usb-charger-detection' property (err=%ld)\n",
-				PTR_ERR(bq->notify_psy));
-			bq->notify_psy = NULL;
-		} else if (!bq->notify_psy) {
-			ret = -EPROBE_DEFER;
-			goto error_2;
-		}
-	} else if (pdata && pdata->notify_device) {
-		bq->notify_psy = power_supply_get_by_name(pdata->notify_device);
-	} else {
-		bq->notify_psy = NULL;
-	}
-
 	i2c_set_clientdata(client, bq);
 
 	bq->id = num;
@@ -1607,32 +1602,35 @@ static int bq2415x_probe(struct i2c_client *client,
 					       "ti,current-limit",
 					       &bq->init_data.current_limit);
 		if (ret)
-			goto error_3;
+			goto error_2;
 		ret = device_property_read_u32(bq->dev,
 					"ti,weak-battery-voltage",
 					&bq->init_data.weak_battery_voltage);
 		if (ret)
-			goto error_3;
+			goto error_2;
 		ret = device_property_read_u32(bq->dev,
 				"ti,battery-regulation-voltage",
 				&bq->init_data.battery_regulation_voltage);
 		if (ret)
-			goto error_3;
+			goto error_2;
 		ret = device_property_read_u32(bq->dev,
 					       "ti,charge-current",
 					       &bq->init_data.charge_current);
 		if (ret)
-			goto error_3;
+			goto error_2;
 		ret = device_property_read_u32(bq->dev,
 				"ti,termination-current",
 				&bq->init_data.termination_current);
 		if (ret)
-			goto error_3;
+			goto error_2;
 		ret = device_property_read_u32(bq->dev,
 					       "ti,resistor-sense",
 					       &bq->init_data.resistor_sense);
 		if (ret)
-			goto error_3;
+			goto error_2;
+		if (np)
+			bq->notify_node = of_parse_phandle(np,
+						"ti,usb-charger-detection", 0);
 	} else {
 		memcpy(&bq->init_data, pdata, sizeof(bq->init_data));
 	}
@@ -1642,56 +1640,72 @@ static int bq2415x_probe(struct i2c_client *client,
 	ret = bq2415x_power_supply_init(bq);
 	if (ret) {
 		dev_err(bq->dev, "failed to register power supply: %d\n", ret);
-		goto error_3;
+		goto error_2;
 	}
 
 	ret = bq2415x_sysfs_init(bq);
 	if (ret) {
 		dev_err(bq->dev, "failed to create sysfs entries: %d\n", ret);
-		goto error_4;
+		goto error_3;
 	}
 
 	ret = bq2415x_set_defaults(bq);
 	if (ret) {
 		dev_err(bq->dev, "failed to set default values: %d\n", ret);
-		goto error_5;
+		goto error_4;
 	}
 
-	if (bq->notify_psy) {
+	if (bq->notify_node || bq->init_data.notify_device) {
 		bq->nb.notifier_call = bq2415x_notifier_call;
 		ret = power_supply_reg_notifier(&bq->nb);
 		if (ret) {
 			dev_err(bq->dev, "failed to reg notifier: %d\n", ret);
-			goto error_6;
+			goto error_4;
 		}
 
-		/* Query for initial reported_mode and set it */
-		bq2415x_notifier_call(&bq->nb, PSY_EVENT_PROP_CHANGED,
-				      bq->notify_psy);
-		bq2415x_set_mode(bq, bq->reported_mode);
-
 		bq->automode = 1;
-		dev_info(bq->dev, "automode enabled\n");
+		dev_info(bq->dev, "automode supported, waiting for events\n");
 	} else {
 		bq->automode = -1;
 		dev_info(bq->dev, "automode not supported\n");
 	}
 
+	/* Query for initial reported_mode and set it */
+	if (bq->nb.notifier_call) {
+		if (np) {
+			notify_psy = power_supply_get_by_phandle(np,
+						"ti,usb-charger-detection");
+			if (IS_ERR(notify_psy))
+				notify_psy = NULL;
+		} else if (bq->init_data.notify_device) {
+			notify_psy = power_supply_get_by_name(
+						bq->init_data.notify_device);
+		}
+	}
+	if (notify_psy) {
+		ret = power_supply_get_property(notify_psy,
+					POWER_SUPPLY_PROP_CURRENT_MAX, &prop);
+		power_supply_put(notify_psy);
+
+		if (ret == 0) {
+			bq2415x_update_reported_mode(bq, prop.intval);
+			bq2415x_set_mode(bq, bq->reported_mode);
+		}
+	}
+
 	INIT_DELAYED_WORK(&bq->work, bq2415x_timer_work);
 	bq2415x_set_autotimer(bq, 1);
 
 	dev_info(bq->dev, "driver registered\n");
 	return 0;
 
-error_6:
-error_5:
-	bq2415x_sysfs_exit(bq);
 error_4:
-	bq2415x_power_supply_exit(bq);
+	bq2415x_sysfs_exit(bq);
 error_3:
-	if (bq->notify_psy)
-		power_supply_put(bq->notify_psy);
+	bq2415x_power_supply_exit(bq);
 error_2:
+	if (bq->notify_node)
+		of_node_put(bq->notify_node);
 	kfree(name);
 error_1:
 	mutex_lock(&bq2415x_id_mutex);
@@ -1707,10 +1721,11 @@ static int bq2415x_remove(struct i2c_client *client)
 {
 	struct bq2415x_device *bq = i2c_get_clientdata(client);
 
-	if (bq->notify_psy) {
+	if (bq->nb.notifier_call)
 		power_supply_unreg_notifier(&bq->nb);
-		power_supply_put(bq->notify_psy);
-	}
+
+	if (bq->notify_node)
+		of_node_put(bq->notify_node);
 
 	bq2415x_sysfs_exit(bq);
 	bq2415x_power_supply_exit(bq);
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] bq2415x_charger: Allow to load and use driver even if notify device is not registered yet
  2015-08-05 13:26 ` Pali Rohár
@ 2015-08-11 11:23   ` Pali Rohár
  0 siblings, 0 replies; 4+ messages in thread
From: Pali Rohár @ 2015-08-11 11:23 UTC (permalink / raw
  To: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse
  Cc: linux-pm, linux-kernel

On Wednesday 05 August 2015 15:26:52 Pali Rohár wrote:
> On Saturday 25 July 2015 23:46:56 Pali Rohár wrote:
> > -	if (psy != bq->notify_psy)
> > -		return NOTIFY_OK;
> > +	/* Ignore event if it was not send by notify_node/notify_device */
> > +	if (bq->notify_node) {
> > +		if (psy->dev.parent &&
> > +		    psy->dev.parent->of_node != bq->notify_node)
> > +			return NOTIFY_OK;
> 
> There is missing branch for case when psy->dev.parent is NULL. Logical
> error... I will send new version of patch.
> 
> Correct logic should be: ignore psy dev which sent event if it is not
> notify dev specified in board/DT config of bq2415x psy dev.
> 

Fixed in v2.

-- 
Pali Rohár
pali.rohar@gmail.com

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2015-08-11 11:23 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-25 21:46 [PATCH] bq2415x_charger: Allow to load and use driver even if notify device is not registered yet Pali Rohár
2015-08-05 13:26 ` Pali Rohár
2015-08-11 11:23   ` Pali Rohár
2015-08-11 11:22 ` [PATCH v2] " Pali Rohár

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.