[PATCH 1/6] power: supply: charger-manager: Remove unused index counting

[PATCH 1/6] power: supply: charger-manager: Remove unused index counting

[Baolin Wang]

Remove unused index counting.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 drivers/power/supply/charger-manager.c |    4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/power/supply/charger-manager.c b/drivers/power/supply/charger-manager.c
index faa1a67..a1b420a 100644
--- a/drivers/power/supply/charger-manager.c
+++ b/drivers/power/supply/charger-manager.c
@@ -1687,10 +1687,6 @@ static int charger_manager_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	/* Counting index only */
-	while (desc->psy_charger_stat[i])
-		i++;
-
 	/* Check if charger's supplies are present at probe */
 	for (i = 0; desc->psy_charger_stat[i]; i++) {
 		struct power_supply *psy;
-- 
1.7.9.5

[PATCH 2/6] power: supply: charger-manager: Fix some misspelled words

[Baolin Wang]

Fix some misspelled words.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 drivers/power/supply/charger-manager.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/power/supply/charger-manager.c b/drivers/power/supply/charger-manager.c
index a1b420a..bd6c450 100644
--- a/drivers/power/supply/charger-manager.c
+++ b/drivers/power/supply/charger-manager.c
@@ -363,7 +363,7 @@ static int try_charger_enable(struct charger_manager *cm, bool enable)
 	int err = 0, i;
 	struct charger_desc *desc = cm->desc;
 
-	/* Ignore if it's redundent command */
+	/* Ignore if it's redundant command */
 	if (enable == cm->charger_enabled)
 		return 0;
 
@@ -1799,7 +1799,7 @@ static int charger_manager_probe(struct platform_device *pdev)
 
 	/*
 	 * Charger-manager is capable of waking up the systme from sleep
-	 * when event is happend through cm_notify_event()
+	 * when event is happened through cm_notify_event()
 	 */
 	device_init_wakeup(&pdev->dev, true);
 	device_set_wakeup_capable(&pdev->dev, false);
@@ -2019,7 +2019,7 @@ static void __exit charger_manager_cleanup(void)
  * cm_notify_event - charger driver notify Charger Manager of charger event
  * @psy: pointer to instance of charger's power_supply
  * @type: type of charger event
- * @msg: optional message passed to uevent_notify fuction
+ * @msg: optional message passed to uevent_notify function
  */
 void cm_notify_event(struct power_supply *psy, enum cm_event_types type,
 		     char *msg)
-- 
1.7.9.5

[PATCH 3/6] power: supply: charger-manager: Fix incorrect return value

[Baolin Wang]

Fix incorrect return value.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 drivers/power/supply/charger-manager.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/power/supply/charger-manager.c b/drivers/power/supply/charger-manager.c
index bd6c450..a52bc77 100644
--- a/drivers/power/supply/charger-manager.c
+++ b/drivers/power/supply/charger-manager.c
@@ -1212,7 +1212,6 @@ static int charger_extcon_init(struct charger_manager *cm,
 	if (ret < 0) {
 		pr_info("Cannot register extcon_dev for %s(cable: %s)\n",
 			cable->extcon_name, cable->name);
-		ret = -EINVAL;
 	}
 
 	return ret;
@@ -1633,7 +1632,7 @@ static int charger_manager_probe(struct platform_device *pdev)
 
 	if (IS_ERR(desc)) {
 		dev_err(&pdev->dev, "No platform data (desc) found\n");
-		return -ENODEV;
+		return PTR_ERR(desc);
 	}
 
 	cm = devm_kzalloc(&pdev->dev, sizeof(*cm), GFP_KERNEL);
-- 
1.7.9.5

[PATCH 4/6] power: supply: charger-manager: Make code more readable

[Baolin Wang]

Make code more readable.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 drivers/power/supply/charger-manager.c |   14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/power/supply/charger-manager.c b/drivers/power/supply/charger-manager.c
index a52bc77..dc0c9a6 100644
--- a/drivers/power/supply/charger-manager.c
+++ b/drivers/power/supply/charger-manager.c
@@ -1520,19 +1520,19 @@ static struct charger_desc *of_cm_parse_desc(struct device *dev)
 	/* chargers */
 	of_property_read_u32(np, "cm-num-chargers", &num_chgs);
 	if (num_chgs) {
+		int i;
+
 		/* Allocate empty bin at the tail of array */
 		desc->psy_charger_stat = devm_kcalloc(dev,
 						      num_chgs + 1,
 						      sizeof(char *),
 						      GFP_KERNEL);
-		if (desc->psy_charger_stat) {
-			int i;
-			for (i = 0; i < num_chgs; i++)
-				of_property_read_string_index(np, "cm-chargers",
-						i, &desc->psy_charger_stat[i]);
-		} else {
+		if (!desc->psy_charger_stat)
 			return ERR_PTR(-ENOMEM);
-		}
+
+		for (i = 0; i < num_chgs; i++)
+			of_property_read_string_index(np, "cm-chargers",
+						      i, &desc->psy_charger_stat[i]);
 	}
 
 	of_property_read_string(np, "cm-fuel-gauge", &desc->psy_fuel_gauge);
-- 
1.7.9.5

[PATCH 5/6] power: supply: charger-manager: Remove deprecated extcon APIs

[Baolin Wang]

The struct extcon_specific_cable_nb and related APIs are deprecated now,
so we should use new method to get one extcon device and register extcon
notifier.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 .../bindings/power/supply/charger-manager.txt      |    6 +--
 drivers/power/supply/charger-manager.c             |   51 ++++++++------------
 include/linux/power/charger-manager.h              |    2 +-
 3 files changed, 23 insertions(+), 36 deletions(-)

diff --git a/Documentation/devicetree/bindings/power/supply/charger-manager.txt b/Documentation/devicetree/bindings/power/supply/charger-manager.txt
index ec4fe9d..315b0cb 100644
--- a/Documentation/devicetree/bindings/power/supply/charger-manager.txt
+++ b/Documentation/devicetree/bindings/power/supply/charger-manager.txt
@@ -11,7 +11,7 @@ Required properties :
 	- cm-regulator-name : name of charger regulator
 	- subnode <cable> :
 		- cm-cable-name : name of charger cable
-		- cm-cable-extcon : name of extcon dev
+		- extcon : phandles to external connector devices
 (optional)	- cm-cable-min : minimum current of cable
 (optional)	- cm-cable-max : maximum current of cable
 
@@ -66,13 +66,13 @@ Example :
 			cm-regulator-name = "chg-reg";
 			cable@0 {
 				cm-cable-name = "USB";
-				cm-cable-extcon = "extcon-dev.0";
+				extcon = <&extcon_usb>;
 				cm-cable-min = <475000>;
 				cm-cable-max = <500000>;
 			};
 			cable@1 {
 				cm-cable-name = "TA";
-				cm-cable-extcon = "extcon-dev.0";
+				extcon = <&extcon_usb>;
 				cm-cable-min = <650000>;
 				cm-cable-max = <675000>;
 			};
diff --git a/drivers/power/supply/charger-manager.c b/drivers/power/supply/charger-manager.c
index dc0c9a6..4f28c03 100644
--- a/drivers/power/supply/charger-manager.c
+++ b/drivers/power/supply/charger-manager.c
@@ -1207,12 +1207,11 @@ static int charger_extcon_init(struct charger_manager *cm,
 	 */
 	INIT_WORK(&cable->wq, charger_extcon_work);
 	cable->nb.notifier_call = charger_extcon_notifier;
-	ret = extcon_register_interest(&cable->extcon_dev,
-			cable->extcon_name, cable->name, &cable->nb);
-	if (ret < 0) {
-		pr_info("Cannot register extcon_dev for %s(cable: %s)\n",
-			cable->extcon_name, cable->name);
-	}
+	ret = devm_extcon_register_notifier(cm->dev, cable->extcon_dev,
+					    EXTCON_USB, &cable->nb);
+	if (ret < 0)
+		dev_err(cm->dev, "Cannot register extcon_dev for (cable: %s)\n",
+			cable->name);
 
 	return ret;
 }
@@ -1589,15 +1588,25 @@ static struct charger_desc *of_cm_parse_desc(struct device *dev)
 				for_each_child_of_node(child, _child) {
 					of_property_read_string(_child,
 					"cm-cable-name", &cables->name);
-					of_property_read_string(_child,
-					"cm-cable-extcon",
-					&cables->extcon_name);
 					of_property_read_u32(_child,
 					"cm-cable-min",
 					&cables->min_uA);
 					of_property_read_u32(_child,
 					"cm-cable-max",
 					&cables->max_uA);
+
+					if (of_property_read_bool(_child, "extcon")) {
+						struct device_node *np;
+
+						np = of_parse_phandle(_child, "extcon", 0);
+						if (!np)
+							return ERR_PTR(-ENODEV);
+
+						cables->extcon_dev = extcon_find_edev_by_node(np);
+						of_node_put(np);
+						if (IS_ERR(cables->extcon_dev))
+							return ERR_PTR(PTR_ERR(cables->extcon_dev));
+					}
 					cables++;
 				}
 			}
@@ -1625,7 +1634,6 @@ static int charger_manager_probe(struct platform_device *pdev)
 	struct charger_desc *desc = cm_get_drv_data(pdev);
 	struct charger_manager *cm;
 	int ret, i = 0;
-	int j = 0;
 	union power_supply_propval val;
 	struct power_supply *fuel_gauge;
 	struct power_supply_config psy_cfg = {};
@@ -1823,19 +1831,8 @@ static int charger_manager_probe(struct platform_device *pdev)
 				&charger->attr_g);
 	}
 err_reg_extcon:
-	for (i = 0; i < desc->num_charger_regulators; i++) {
-		struct charger_regulator *charger;
-
-		charger = &desc->charger_regulators[i];
-		for (j = 0; j < charger->num_cables; j++) {
-			struct charger_cable *cable = &charger->cables[j];
-			/* Remove notifier block if only edev exists */
-			if (cable->extcon_dev.edev)
-				extcon_unregister_interest(&cable->extcon_dev);
-		}
-
+	for (i = 0; i < desc->num_charger_regulators; i++)
 		regulator_put(desc->charger_regulators[i].consumer);
-	}
 
 	power_supply_unregister(cm->charger_psy);
 
@@ -1847,7 +1844,6 @@ static int charger_manager_remove(struct platform_device *pdev)
 	struct charger_manager *cm = platform_get_drvdata(pdev);
 	struct charger_desc *desc = cm->desc;
 	int i = 0;
-	int j = 0;
 
 	/* Remove from the list */
 	mutex_lock(&cm_list_mtx);
@@ -1857,15 +1853,6 @@ static int charger_manager_remove(struct platform_device *pdev)
 	cancel_work_sync(&setup_polling);
 	cancel_delayed_work_sync(&cm_monitor_work);
 
-	for (i = 0 ; i < desc->num_charger_regulators ; i++) {
-		struct charger_regulator *charger
-				= &desc->charger_regulators[i];
-		for (j = 0 ; j < charger->num_cables ; j++) {
-			struct charger_cable *cable = &charger->cables[j];
-			extcon_unregister_interest(&cable->extcon_dev);
-		}
-	}
-
 	for (i = 0 ; i < desc->num_charger_regulators ; i++)
 		regulator_put(desc->charger_regulators[i].consumer);
 
diff --git a/include/linux/power/charger-manager.h b/include/linux/power/charger-manager.h
index c4fa907..e4d0269 100644
--- a/include/linux/power/charger-manager.h
+++ b/include/linux/power/charger-manager.h
@@ -66,7 +66,7 @@ struct charger_cable {
 	const char *name;
 
 	/* The charger-manager use Extcon framework */
-	struct extcon_specific_cable_nb extcon_dev;
+	struct extcon_dev *extcon_dev;
 	struct work_struct wq;
 	struct notifier_block nb;
 
-- 
1.7.9.5

[PATCH 6/6] power: supply: charger-manager: Add new method to start/stop charging

[Baolin Wang]

For some chargers (such as Spreadtrum SC2731 charger), they don't use
regulators to control charging, instead charging control was implemented
in their drivers, so we can add some supports to start or stop charging
by POWER_SUPPLY_PROP_STATUS property.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 drivers/power/supply/charger-manager.c |   35 ++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/drivers/power/supply/charger-manager.c b/drivers/power/supply/charger-manager.c
index 4f28c03..d0d0b82 100644
--- a/drivers/power/supply/charger-manager.c
+++ b/drivers/power/supply/charger-manager.c
@@ -348,6 +348,32 @@ static bool is_polling_required(struct charger_manager *cm)
 	return false;
 }
 
+static int try_charger_enable_by_psy(struct charger_manager *cm, bool enable)
+{
+	struct charger_desc *desc = cm->desc;
+	union power_supply_propval val;
+	struct power_supply *psy;
+	int i, err;
+
+	for (i = 0; desc->psy_charger_stat[i]; i++) {
+		psy = power_supply_get_by_name(desc->psy_charger_stat[i]);
+		if (!psy) {
+			dev_err(cm->dev, "Cannot find power supply \"%s\"\n",
+				desc->psy_charger_stat[i]);
+			continue;
+		}
+
+		val.intval = enable;
+		err = power_supply_set_property(psy, POWER_SUPPLY_PROP_STATUS,
+						&val);
+		power_supply_put(psy);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
 /**
  * try_charger_enable - Enable/Disable chargers altogether
  * @cm: the Charger Manager representing the battery.
@@ -378,6 +404,10 @@ static int try_charger_enable(struct charger_manager *cm, bool enable)
 		cm->charging_start_time = ktime_to_ms(ktime_get());
 		cm->charging_end_time = 0;
 
+		err = try_charger_enable_by_psy(cm, enable);
+		if (!err)
+			goto out;
+
 		for (i = 0 ; i < desc->num_charger_regulators ; i++) {
 			if (desc->charger_regulators[i].externally_control)
 				continue;
@@ -396,6 +426,10 @@ static int try_charger_enable(struct charger_manager *cm, bool enable)
 		cm->charging_start_time = 0;
 		cm->charging_end_time = ktime_to_ms(ktime_get());
 
+		err = try_charger_enable_by_psy(cm, enable);
+		if (!err)
+			goto out;
+
 		for (i = 0 ; i < desc->num_charger_regulators ; i++) {
 			if (desc->charger_regulators[i].externally_control)
 				continue;
@@ -422,6 +456,7 @@ static int try_charger_enable(struct charger_manager *cm, bool enable)
 		}
 	}
 
+out:
 	if (!err)
 		cm->charger_enabled = enable;
 
-- 
1.7.9.5

Re: [PATCH 5/6] power: supply: charger-manager: Remove deprecated extcon APIs

[Rob Herring]

On Fri, Nov 16, 2018 at 07:01:12PM +0800, Baolin Wang wrote:
> The struct extcon_specific_cable_nb and related APIs are deprecated now,
> so we should use new method to get one extcon device and register extcon
> notifier.
> 
> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
> ---
>  .../bindings/power/supply/charger-manager.txt      |    6 +--

Bindings should be a separate patch.

>  drivers/power/supply/charger-manager.c             |   51 ++++++++------------
>  include/linux/power/charger-manager.h              |    2 +-
>  3 files changed, 23 insertions(+), 36 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/power/supply/charger-manager.txt b/Documentation/devicetree/bindings/power/supply/charger-manager.txt
> index ec4fe9d..315b0cb 100644
> --- a/Documentation/devicetree/bindings/power/supply/charger-manager.txt
> +++ b/Documentation/devicetree/bindings/power/supply/charger-manager.txt
> @@ -11,7 +11,7 @@ Required properties :
>  	- cm-regulator-name : name of charger regulator
>  	- subnode <cable> :
>  		- cm-cable-name : name of charger cable
> -		- cm-cable-extcon : name of extcon dev
> +		- extcon : phandles to external connector devices

Somewhat less terrible, but not really. I consider extcon binding itself 
deprecated. I suspect 'charger-manager' as a whole probably needs to be 
reworked. This also is not a backwards compatible change.

>  (optional)	- cm-cable-min : minimum current of cable
>  (optional)	- cm-cable-max : maximum current of cable
>  
> @@ -66,13 +66,13 @@ Example :
>  			cm-regulator-name = "chg-reg";
>  			cable@0 {
>  				cm-cable-name = "USB";
> -				cm-cable-extcon = "extcon-dev.0";
> +				extcon = <&extcon_usb>;
>  				cm-cable-min = <475000>;
>  				cm-cable-max = <500000>;
>  			};
>  			cable@1 {
>  				cm-cable-name = "TA";
> -				cm-cable-extcon = "extcon-dev.0";
> +				extcon = <&extcon_usb>;
>  				cm-cable-min = <650000>;
>  				cm-cable-max = <675000>;
>  			};
> diff --git a/drivers/power/supply/charger-manager.c b/drivers/power/supply/charger-manager.c
> index dc0c9a6..4f28c03 100644
> --- a/drivers/power/supply/charger-manager.c
> +++ b/drivers/power/supply/charger-manager.c
> @@ -1207,12 +1207,11 @@ static int charger_extcon_init(struct charger_manager *cm,
>  	 */
>  	INIT_WORK(&cable->wq, charger_extcon_work);
>  	cable->nb.notifier_call = charger_extcon_notifier;
> -	ret = extcon_register_interest(&cable->extcon_dev,
> -			cable->extcon_name, cable->name, &cable->nb);
> -	if (ret < 0) {
> -		pr_info("Cannot register extcon_dev for %s(cable: %s)\n",
> -			cable->extcon_name, cable->name);
> -	}
> +	ret = devm_extcon_register_notifier(cm->dev, cable->extcon_dev,
> +					    EXTCON_USB, &cable->nb);
> +	if (ret < 0)
> +		dev_err(cm->dev, "Cannot register extcon_dev for (cable: %s)\n",
> +			cable->name);
>  
>  	return ret;
>  }
> @@ -1589,15 +1588,25 @@ static struct charger_desc *of_cm_parse_desc(struct device *dev)
>  				for_each_child_of_node(child, _child) {
>  					of_property_read_string(_child,
>  					"cm-cable-name", &cables->name);
> -					of_property_read_string(_child,
> -					"cm-cable-extcon",
> -					&cables->extcon_name);
>  					of_property_read_u32(_child,
>  					"cm-cable-min",
>  					&cables->min_uA);
>  					of_property_read_u32(_child,
>  					"cm-cable-max",
>  					&cables->max_uA);
> +
> +					if (of_property_read_bool(_child, "extcon")) {
> +						struct device_node *np;
> +
> +						np = of_parse_phandle(_child, "extcon", 0);
> +						if (!np)
> +							return ERR_PTR(-ENODEV);
> +
> +						cables->extcon_dev = extcon_find_edev_by_node(np);
> +						of_node_put(np);
> +						if (IS_ERR(cables->extcon_dev))
> +							return ERR_PTR(PTR_ERR(cables->extcon_dev));
> +					}
>  					cables++;
>  				}
>  			}
> @@ -1625,7 +1634,6 @@ static int charger_manager_probe(struct platform_device *pdev)
>  	struct charger_desc *desc = cm_get_drv_data(pdev);
>  	struct charger_manager *cm;
>  	int ret, i = 0;
> -	int j = 0;
>  	union power_supply_propval val;
>  	struct power_supply *fuel_gauge;
>  	struct power_supply_config psy_cfg = {};
> @@ -1823,19 +1831,8 @@ static int charger_manager_probe(struct platform_device *pdev)
>  				&charger->attr_g);
>  	}
>  err_reg_extcon:
> -	for (i = 0; i < desc->num_charger_regulators; i++) {
> -		struct charger_regulator *charger;
> -
> -		charger = &desc->charger_regulators[i];
> -		for (j = 0; j < charger->num_cables; j++) {
> -			struct charger_cable *cable = &charger->cables[j];
> -			/* Remove notifier block if only edev exists */
> -			if (cable->extcon_dev.edev)
> -				extcon_unregister_interest(&cable->extcon_dev);
> -		}
> -
> +	for (i = 0; i < desc->num_charger_regulators; i++)
>  		regulator_put(desc->charger_regulators[i].consumer);
> -	}
>  
>  	power_supply_unregister(cm->charger_psy);
>  
> @@ -1847,7 +1844,6 @@ static int charger_manager_remove(struct platform_device *pdev)
>  	struct charger_manager *cm = platform_get_drvdata(pdev);
>  	struct charger_desc *desc = cm->desc;
>  	int i = 0;
> -	int j = 0;
>  
>  	/* Remove from the list */
>  	mutex_lock(&cm_list_mtx);
> @@ -1857,15 +1853,6 @@ static int charger_manager_remove(struct platform_device *pdev)
>  	cancel_work_sync(&setup_polling);
>  	cancel_delayed_work_sync(&cm_monitor_work);
>  
> -	for (i = 0 ; i < desc->num_charger_regulators ; i++) {
> -		struct charger_regulator *charger
> -				= &desc->charger_regulators[i];
> -		for (j = 0 ; j < charger->num_cables ; j++) {
> -			struct charger_cable *cable = &charger->cables[j];
> -			extcon_unregister_interest(&cable->extcon_dev);
> -		}
> -	}
> -
>  	for (i = 0 ; i < desc->num_charger_regulators ; i++)
>  		regulator_put(desc->charger_regulators[i].consumer);
>  
> diff --git a/include/linux/power/charger-manager.h b/include/linux/power/charger-manager.h
> index c4fa907..e4d0269 100644
> --- a/include/linux/power/charger-manager.h
> +++ b/include/linux/power/charger-manager.h
> @@ -66,7 +66,7 @@ struct charger_cable {
>  	const char *name;
>  
>  	/* The charger-manager use Extcon framework */
> -	struct extcon_specific_cable_nb extcon_dev;
> +	struct extcon_dev *extcon_dev;
>  	struct work_struct wq;
>  	struct notifier_block nb;
>  
> -- 
> 1.7.9.5
> 

Re: [PATCH 5/6] power: supply: charger-manager: Remove deprecated extcon APIs

[Baolin Wang]

Hi Rob，
On Wed, 5 Dec 2018 at 05:52, Rob Herring <robh@kernel.org> wrote:
>
> On Fri, Nov 16, 2018 at 07:01:12PM +0800, Baolin Wang wrote:
> > The struct extcon_specific_cable_nb and related APIs are deprecated now,
> > so we should use new method to get one extcon device and register extcon
> > notifier.
> >
> > Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
> > ---
> >  .../bindings/power/supply/charger-manager.txt      |    6 +--
>
> Bindings should be a separate patch.

Sure.

>
> >  drivers/power/supply/charger-manager.c             |   51 ++++++++------------
> >  include/linux/power/charger-manager.h              |    2 +-
> >  3 files changed, 23 insertions(+), 36 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/power/supply/charger-manager.txt b/Documentation/devicetree/bindings/power/supply/charger-manager.txt
> > index ec4fe9d..315b0cb 100644
> > --- a/Documentation/devicetree/bindings/power/supply/charger-manager.txt
> > +++ b/Documentation/devicetree/bindings/power/supply/charger-manager.txt
> > @@ -11,7 +11,7 @@ Required properties :
> >       - cm-regulator-name : name of charger regulator
> >       - subnode <cable> :
> >               - cm-cable-name : name of charger cable
> > -             - cm-cable-extcon : name of extcon dev
> > +             - extcon : phandles to external connector devices
>
> Somewhat less terrible, but not really. I consider extcon binding itself
> deprecated. I suspect 'charger-manager' as a whole probably needs to be
> reworked. This also is not a backwards compatible change.

We are do some optimization for 'charger-manager', like this patch
did, we are trying to remove the deprecated extcon API.
And now no one use the incorrect 'cm-cable-extcon' property on
mainline, so no need worry backwards compatibility.

> >  (optional)   - cm-cable-min : minimum current of cable
> >  (optional)   - cm-cable-max : maximum current of cable
> >
> > @@ -66,13 +66,13 @@ Example :
> >                       cm-regulator-name = "chg-reg";
> >                       cable@0 {
> >                               cm-cable-name = "USB";
> > -                             cm-cable-extcon = "extcon-dev.0";
> > +                             extcon = <&extcon_usb>;
> >                               cm-cable-min = <475000>;
> >                               cm-cable-max = <500000>;
> >                       };
> >                       cable@1 {
> >                               cm-cable-name = "TA";
> > -                             cm-cable-extcon = "extcon-dev.0";
> > +                             extcon = <&extcon_usb>;
> >                               cm-cable-min = <650000>;
> >                               cm-cable-max = <675000>;
> >                       };
> > diff --git a/drivers/power/supply/charger-manager.c b/drivers/power/supply/charger-manager.c
> > index dc0c9a6..4f28c03 100644
> > --- a/drivers/power/supply/charger-manager.c
> > +++ b/drivers/power/supply/charger-manager.c
> > @@ -1207,12 +1207,11 @@ static int charger_extcon_init(struct charger_manager *cm,
> >        */
> >       INIT_WORK(&cable->wq, charger_extcon_work);
> >       cable->nb.notifier_call = charger_extcon_notifier;
> > -     ret = extcon_register_interest(&cable->extcon_dev,
> > -                     cable->extcon_name, cable->name, &cable->nb);
> > -     if (ret < 0) {
> > -             pr_info("Cannot register extcon_dev for %s(cable: %s)\n",
> > -                     cable->extcon_name, cable->name);
> > -     }
> > +     ret = devm_extcon_register_notifier(cm->dev, cable->extcon_dev,
> > +                                         EXTCON_USB, &cable->nb);
> > +     if (ret < 0)
> > +             dev_err(cm->dev, "Cannot register extcon_dev for (cable: %s)\n",
> > +                     cable->name);
> >
> >       return ret;
> >  }
> > @@ -1589,15 +1588,25 @@ static struct charger_desc *of_cm_parse_desc(struct device *dev)
> >                               for_each_child_of_node(child, _child) {
> >                                       of_property_read_string(_child,
> >                                       "cm-cable-name", &cables->name);
> > -                                     of_property_read_string(_child,
> > -                                     "cm-cable-extcon",
> > -                                     &cables->extcon_name);
> >                                       of_property_read_u32(_child,
> >                                       "cm-cable-min",
> >                                       &cables->min_uA);
> >                                       of_property_read_u32(_child,
> >                                       "cm-cable-max",
> >                                       &cables->max_uA);
> > +
> > +                                     if (of_property_read_bool(_child, "extcon")) {
> > +                                             struct device_node *np;
> > +
> > +                                             np = of_parse_phandle(_child, "extcon", 0);
> > +                                             if (!np)
> > +                                                     return ERR_PTR(-ENODEV);
> > +
> > +                                             cables->extcon_dev = extcon_find_edev_by_node(np);
> > +                                             of_node_put(np);
> > +                                             if (IS_ERR(cables->extcon_dev))
> > +                                                     return ERR_PTR(PTR_ERR(cables->extcon_dev));
> > +                                     }
> >                                       cables++;
> >                               }
> >                       }
> > @@ -1625,7 +1634,6 @@ static int charger_manager_probe(struct platform_device *pdev)
> >       struct charger_desc *desc = cm_get_drv_data(pdev);
> >       struct charger_manager *cm;
> >       int ret, i = 0;
> > -     int j = 0;
> >       union power_supply_propval val;
> >       struct power_supply *fuel_gauge;
> >       struct power_supply_config psy_cfg = {};
> > @@ -1823,19 +1831,8 @@ static int charger_manager_probe(struct platform_device *pdev)
> >                               &charger->attr_g);
> >       }
> >  err_reg_extcon:
> > -     for (i = 0; i < desc->num_charger_regulators; i++) {
> > -             struct charger_regulator *charger;
> > -
> > -             charger = &desc->charger_regulators[i];
> > -             for (j = 0; j < charger->num_cables; j++) {
> > -                     struct charger_cable *cable = &charger->cables[j];
> > -                     /* Remove notifier block if only edev exists */
> > -                     if (cable->extcon_dev.edev)
> > -                             extcon_unregister_interest(&cable->extcon_dev);
> > -             }
> > -
> > +     for (i = 0; i < desc->num_charger_regulators; i++)
> >               regulator_put(desc->charger_regulators[i].consumer);
> > -     }
> >
> >       power_supply_unregister(cm->charger_psy);
> >
> > @@ -1847,7 +1844,6 @@ static int charger_manager_remove(struct platform_device *pdev)
> >       struct charger_manager *cm = platform_get_drvdata(pdev);
> >       struct charger_desc *desc = cm->desc;
> >       int i = 0;
> > -     int j = 0;
> >
> >       /* Remove from the list */
> >       mutex_lock(&cm_list_mtx);
> > @@ -1857,15 +1853,6 @@ static int charger_manager_remove(struct platform_device *pdev)
> >       cancel_work_sync(&setup_polling);
> >       cancel_delayed_work_sync(&cm_monitor_work);
> >
> > -     for (i = 0 ; i < desc->num_charger_regulators ; i++) {
> > -             struct charger_regulator *charger
> > -                             = &desc->charger_regulators[i];
> > -             for (j = 0 ; j < charger->num_cables ; j++) {
> > -                     struct charger_cable *cable = &charger->cables[j];
> > -                     extcon_unregister_interest(&cable->extcon_dev);
> > -             }
> > -     }
> > -
> >       for (i = 0 ; i < desc->num_charger_regulators ; i++)
> >               regulator_put(desc->charger_regulators[i].consumer);
> >
> > diff --git a/include/linux/power/charger-manager.h b/include/linux/power/charger-manager.h
> > index c4fa907..e4d0269 100644
> > --- a/include/linux/power/charger-manager.h
> > +++ b/include/linux/power/charger-manager.h
> > @@ -66,7 +66,7 @@ struct charger_cable {
> >       const char *name;
> >
> >       /* The charger-manager use Extcon framework */
> > -     struct extcon_specific_cable_nb extcon_dev;
> > +     struct extcon_dev *extcon_dev;
> >       struct work_struct wq;
> >       struct notifier_block nb;
> >
> > --
> > 1.7.9.5
> >



-- 
Baolin Wang
Best Regards

Re: [PATCH 5/6] power: supply: charger-manager: Remove deprecated extcon APIs

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 9590 bytes --]

Hi,

On Wed, Dec 05, 2018 at 10:57:12AM +0800, Baolin Wang wrote:
> Hi Rob，
> On Wed, 5 Dec 2018 at 05:52, Rob Herring <robh@kernel.org> wrote:
> >
> > On Fri, Nov 16, 2018 at 07:01:12PM +0800, Baolin Wang wrote:
> > > The struct extcon_specific_cable_nb and related APIs are deprecated now,
> > > so we should use new method to get one extcon device and register extcon
> > > notifier.
> > >
> > > Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
> > > ---
> > >  .../bindings/power/supply/charger-manager.txt      |    6 +--
> >
> > Bindings should be a separate patch.
> 
> Sure.
> 
> >
> > >  drivers/power/supply/charger-manager.c             |   51 ++++++++------------
> > >  include/linux/power/charger-manager.h              |    2 +-
> > >  3 files changed, 23 insertions(+), 36 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/power/supply/charger-manager.txt b/Documentation/devicetree/bindings/power/supply/charger-manager.txt
> > > index ec4fe9d..315b0cb 100644
> > > --- a/Documentation/devicetree/bindings/power/supply/charger-manager.txt
> > > +++ b/Documentation/devicetree/bindings/power/supply/charger-manager.txt
> > > @@ -11,7 +11,7 @@ Required properties :
> > >       - cm-regulator-name : name of charger regulator
> > >       - subnode <cable> :
> > >               - cm-cable-name : name of charger cable
> > > -             - cm-cable-extcon : name of extcon dev
> > > +             - extcon : phandles to external connector devices
> >
> > Somewhat less terrible, but not really. I consider extcon binding itself
> > deprecated. I suspect 'charger-manager' as a whole probably needs to be
> > reworked. This also is not a backwards compatible change.

Right, charger-manager is a big hack. The DT node does not represent
any hardware. The feature should be integrated into the power-supply
core and work without any special DT bindings. I don't have the time
to work on this, though.

> We are do some optimization for 'charger-manager', like this patch
> did, we are trying to remove the deprecated extcon API.
> And now no one use the incorrect 'cm-cable-extcon' property on
> mainline, so no need worry backwards compatibility.

As far as I can see there is no charger-manager user in mainline at
all.

-- Sebastian

> > >  (optional)   - cm-cable-min : minimum current of cable
> > >  (optional)   - cm-cable-max : maximum current of cable
> > >
> > > @@ -66,13 +66,13 @@ Example :
> > >                       cm-regulator-name = "chg-reg";
> > >                       cable@0 {
> > >                               cm-cable-name = "USB";
> > > -                             cm-cable-extcon = "extcon-dev.0";
> > > +                             extcon = <&extcon_usb>;
> > >                               cm-cable-min = <475000>;
> > >                               cm-cable-max = <500000>;
> > >                       };
> > >                       cable@1 {
> > >                               cm-cable-name = "TA";
> > > -                             cm-cable-extcon = "extcon-dev.0";
> > > +                             extcon = <&extcon_usb>;
> > >                               cm-cable-min = <650000>;
> > >                               cm-cable-max = <675000>;
> > >                       };
> > > diff --git a/drivers/power/supply/charger-manager.c b/drivers/power/supply/charger-manager.c
> > > index dc0c9a6..4f28c03 100644
> > > --- a/drivers/power/supply/charger-manager.c
> > > +++ b/drivers/power/supply/charger-manager.c
> > > @@ -1207,12 +1207,11 @@ static int charger_extcon_init(struct charger_manager *cm,
> > >        */
> > >       INIT_WORK(&cable->wq, charger_extcon_work);
> > >       cable->nb.notifier_call = charger_extcon_notifier;
> > > -     ret = extcon_register_interest(&cable->extcon_dev,
> > > -                     cable->extcon_name, cable->name, &cable->nb);
> > > -     if (ret < 0) {
> > > -             pr_info("Cannot register extcon_dev for %s(cable: %s)\n",
> > > -                     cable->extcon_name, cable->name);
> > > -     }
> > > +     ret = devm_extcon_register_notifier(cm->dev, cable->extcon_dev,
> > > +                                         EXTCON_USB, &cable->nb);
> > > +     if (ret < 0)
> > > +             dev_err(cm->dev, "Cannot register extcon_dev for (cable: %s)\n",
> > > +                     cable->name);
> > >
> > >       return ret;
> > >  }
> > > @@ -1589,15 +1588,25 @@ static struct charger_desc *of_cm_parse_desc(struct device *dev)
> > >                               for_each_child_of_node(child, _child) {
> > >                                       of_property_read_string(_child,
> > >                                       "cm-cable-name", &cables->name);
> > > -                                     of_property_read_string(_child,
> > > -                                     "cm-cable-extcon",
> > > -                                     &cables->extcon_name);
> > >                                       of_property_read_u32(_child,
> > >                                       "cm-cable-min",
> > >                                       &cables->min_uA);
> > >                                       of_property_read_u32(_child,
> > >                                       "cm-cable-max",
> > >                                       &cables->max_uA);
> > > +
> > > +                                     if (of_property_read_bool(_child, "extcon")) {
> > > +                                             struct device_node *np;
> > > +
> > > +                                             np = of_parse_phandle(_child, "extcon", 0);
> > > +                                             if (!np)
> > > +                                                     return ERR_PTR(-ENODEV);
> > > +
> > > +                                             cables->extcon_dev = extcon_find_edev_by_node(np);
> > > +                                             of_node_put(np);
> > > +                                             if (IS_ERR(cables->extcon_dev))
> > > +                                                     return ERR_PTR(PTR_ERR(cables->extcon_dev));
> > > +                                     }
> > >                                       cables++;
> > >                               }
> > >                       }
> > > @@ -1625,7 +1634,6 @@ static int charger_manager_probe(struct platform_device *pdev)
> > >       struct charger_desc *desc = cm_get_drv_data(pdev);
> > >       struct charger_manager *cm;
> > >       int ret, i = 0;
> > > -     int j = 0;
> > >       union power_supply_propval val;
> > >       struct power_supply *fuel_gauge;
> > >       struct power_supply_config psy_cfg = {};
> > > @@ -1823,19 +1831,8 @@ static int charger_manager_probe(struct platform_device *pdev)
> > >                               &charger->attr_g);
> > >       }
> > >  err_reg_extcon:
> > > -     for (i = 0; i < desc->num_charger_regulators; i++) {
> > > -             struct charger_regulator *charger;
> > > -
> > > -             charger = &desc->charger_regulators[i];
> > > -             for (j = 0; j < charger->num_cables; j++) {
> > > -                     struct charger_cable *cable = &charger->cables[j];
> > > -                     /* Remove notifier block if only edev exists */
> > > -                     if (cable->extcon_dev.edev)
> > > -                             extcon_unregister_interest(&cable->extcon_dev);
> > > -             }
> > > -
> > > +     for (i = 0; i < desc->num_charger_regulators; i++)
> > >               regulator_put(desc->charger_regulators[i].consumer);
> > > -     }
> > >
> > >       power_supply_unregister(cm->charger_psy);
> > >
> > > @@ -1847,7 +1844,6 @@ static int charger_manager_remove(struct platform_device *pdev)
> > >       struct charger_manager *cm = platform_get_drvdata(pdev);
> > >       struct charger_desc *desc = cm->desc;
> > >       int i = 0;
> > > -     int j = 0;
> > >
> > >       /* Remove from the list */
> > >       mutex_lock(&cm_list_mtx);
> > > @@ -1857,15 +1853,6 @@ static int charger_manager_remove(struct platform_device *pdev)
> > >       cancel_work_sync(&setup_polling);
> > >       cancel_delayed_work_sync(&cm_monitor_work);
> > >
> > > -     for (i = 0 ; i < desc->num_charger_regulators ; i++) {
> > > -             struct charger_regulator *charger
> > > -                             = &desc->charger_regulators[i];
> > > -             for (j = 0 ; j < charger->num_cables ; j++) {
> > > -                     struct charger_cable *cable = &charger->cables[j];
> > > -                     extcon_unregister_interest(&cable->extcon_dev);
> > > -             }
> > > -     }
> > > -
> > >       for (i = 0 ; i < desc->num_charger_regulators ; i++)
> > >               regulator_put(desc->charger_regulators[i].consumer);
> > >
> > > diff --git a/include/linux/power/charger-manager.h b/include/linux/power/charger-manager.h
> > > index c4fa907..e4d0269 100644
> > > --- a/include/linux/power/charger-manager.h
> > > +++ b/include/linux/power/charger-manager.h
> > > @@ -66,7 +66,7 @@ struct charger_cable {
> > >       const char *name;
> > >
> > >       /* The charger-manager use Extcon framework */
> > > -     struct extcon_specific_cable_nb extcon_dev;
> > > +     struct extcon_dev *extcon_dev;
> > >       struct work_struct wq;
> > >       struct notifier_block nb;
> > >
> > > --
> > > 1.7.9.5
> > >
> 
> 
> 
> -- 
> Baolin Wang
> Best Regards

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/6] power: supply: charger-manager: Remove unused index counting

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 898 bytes --]

Hi,

On Fri, Nov 16, 2018 at 07:01:08PM +0800, Baolin Wang wrote:
> Remove unused index counting.
> 
> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
> ---

Thanks, queued.

-- Sebastian

>  drivers/power/supply/charger-manager.c |    4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/power/supply/charger-manager.c b/drivers/power/supply/charger-manager.c
> index faa1a67..a1b420a 100644
> --- a/drivers/power/supply/charger-manager.c
> +++ b/drivers/power/supply/charger-manager.c
> @@ -1687,10 +1687,6 @@ static int charger_manager_probe(struct platform_device *pdev)
>  		return -EINVAL;
>  	}
>  
> -	/* Counting index only */
> -	while (desc->psy_charger_stat[i])
> -		i++;
> -
>  	/* Check if charger's supplies are present at probe */
>  	for (i = 0; desc->psy_charger_stat[i]; i++) {
>  		struct power_supply *psy;
> -- 
> 1.7.9.5
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/6] power: supply: charger-manager: Fix some misspelled words

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1742 bytes --]

Hi,

On Fri, Nov 16, 2018 at 07:01:09PM +0800, Baolin Wang wrote:
> Fix some misspelled words.
> 
> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
> ---

Thanks, queued.

-- Sebastian

>  drivers/power/supply/charger-manager.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/power/supply/charger-manager.c b/drivers/power/supply/charger-manager.c
> index a1b420a..bd6c450 100644
> --- a/drivers/power/supply/charger-manager.c
> +++ b/drivers/power/supply/charger-manager.c
> @@ -363,7 +363,7 @@ static int try_charger_enable(struct charger_manager *cm, bool enable)
>  	int err = 0, i;
>  	struct charger_desc *desc = cm->desc;
>  
> -	/* Ignore if it's redundent command */
> +	/* Ignore if it's redundant command */
>  	if (enable == cm->charger_enabled)
>  		return 0;
>  
> @@ -1799,7 +1799,7 @@ static int charger_manager_probe(struct platform_device *pdev)
>  
>  	/*
>  	 * Charger-manager is capable of waking up the systme from sleep
> -	 * when event is happend through cm_notify_event()
> +	 * when event is happened through cm_notify_event()
>  	 */
>  	device_init_wakeup(&pdev->dev, true);
>  	device_set_wakeup_capable(&pdev->dev, false);
> @@ -2019,7 +2019,7 @@ static void __exit charger_manager_cleanup(void)
>   * cm_notify_event - charger driver notify Charger Manager of charger event
>   * @psy: pointer to instance of charger's power_supply
>   * @type: type of charger event
> - * @msg: optional message passed to uevent_notify fuction
> + * @msg: optional message passed to uevent_notify function
>   */
>  void cm_notify_event(struct power_supply *psy, enum cm_event_types type,
>  		     char *msg)
> -- 
> 1.7.9.5
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 3/6] power: supply: charger-manager: Fix incorrect return value

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1121 bytes --]

Hi,

On Fri, Nov 16, 2018 at 07:01:10PM +0800, Baolin Wang wrote:
> Fix incorrect return value.
> 
> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
> ---

Thanks, queued.

-- Sebastian

>  drivers/power/supply/charger-manager.c |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/power/supply/charger-manager.c b/drivers/power/supply/charger-manager.c
> index bd6c450..a52bc77 100644
> --- a/drivers/power/supply/charger-manager.c
> +++ b/drivers/power/supply/charger-manager.c
> @@ -1212,7 +1212,6 @@ static int charger_extcon_init(struct charger_manager *cm,
>  	if (ret < 0) {
>  		pr_info("Cannot register extcon_dev for %s(cable: %s)\n",
>  			cable->extcon_name, cable->name);
> -		ret = -EINVAL;
>  	}
>  
>  	return ret;
> @@ -1633,7 +1632,7 @@ static int charger_manager_probe(struct platform_device *pdev)
>  
>  	if (IS_ERR(desc)) {
>  		dev_err(&pdev->dev, "No platform data (desc) found\n");
> -		return -ENODEV;
> +		return PTR_ERR(desc);
>  	}
>  
>  	cm = devm_kzalloc(&pdev->dev, sizeof(*cm), GFP_KERNEL);
> -- 
> 1.7.9.5
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 4/6] power: supply: charger-manager: Make code more readable

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1481 bytes --]

Hi,

On Fri, Nov 16, 2018 at 07:01:11PM +0800, Baolin Wang wrote:
> Make code more readable.
> 
> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
> ---

Thanks, queued.

-- Sebastian

>  drivers/power/supply/charger-manager.c |   14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/power/supply/charger-manager.c b/drivers/power/supply/charger-manager.c
> index a52bc77..dc0c9a6 100644
> --- a/drivers/power/supply/charger-manager.c
> +++ b/drivers/power/supply/charger-manager.c
> @@ -1520,19 +1520,19 @@ static struct charger_desc *of_cm_parse_desc(struct device *dev)
>  	/* chargers */
>  	of_property_read_u32(np, "cm-num-chargers", &num_chgs);
>  	if (num_chgs) {
> +		int i;
> +
>  		/* Allocate empty bin at the tail of array */
>  		desc->psy_charger_stat = devm_kcalloc(dev,
>  						      num_chgs + 1,
>  						      sizeof(char *),
>  						      GFP_KERNEL);
> -		if (desc->psy_charger_stat) {
> -			int i;
> -			for (i = 0; i < num_chgs; i++)
> -				of_property_read_string_index(np, "cm-chargers",
> -						i, &desc->psy_charger_stat[i]);
> -		} else {
> +		if (!desc->psy_charger_stat)
>  			return ERR_PTR(-ENOMEM);
> -		}
> +
> +		for (i = 0; i < num_chgs; i++)
> +			of_property_read_string_index(np, "cm-chargers",
> +						      i, &desc->psy_charger_stat[i]);
>  	}
>  
>  	of_property_read_string(np, "cm-fuel-gauge", &desc->psy_fuel_gauge);
> -- 
> 1.7.9.5
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 5/6] power: supply: charger-manager: Remove deprecated extcon APIs

[Baolin Wang]

Hi Sebastian,

On Thu, 6 Dec 2018 at 04:34, Sebastian Reichel
<sebastian.reichel@collabora.com> wrote:
>
> Hi,
>
> On Wed, Dec 05, 2018 at 10:57:12AM +0800, Baolin Wang wrote:
> > Hi Rob，
> > On Wed, 5 Dec 2018 at 05:52, Rob Herring <robh@kernel.org> wrote:
> > >
> > > On Fri, Nov 16, 2018 at 07:01:12PM +0800, Baolin Wang wrote:
> > > > The struct extcon_specific_cable_nb and related APIs are deprecated now,
> > > > so we should use new method to get one extcon device and register extcon
> > > > notifier.
> > > >
> > > > Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
> > > > ---
> > > >  .../bindings/power/supply/charger-manager.txt      |    6 +--
> > >
> > > Bindings should be a separate patch.
> >
> > Sure.
> >
> > >
> > > >  drivers/power/supply/charger-manager.c             |   51 ++++++++------------
> > > >  include/linux/power/charger-manager.h              |    2 +-
> > > >  3 files changed, 23 insertions(+), 36 deletions(-)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/power/supply/charger-manager.txt b/Documentation/devicetree/bindings/power/supply/charger-manager.txt
> > > > index ec4fe9d..315b0cb 100644
> > > > --- a/Documentation/devicetree/bindings/power/supply/charger-manager.txt
> > > > +++ b/Documentation/devicetree/bindings/power/supply/charger-manager.txt
> > > > @@ -11,7 +11,7 @@ Required properties :
> > > >       - cm-regulator-name : name of charger regulator
> > > >       - subnode <cable> :
> > > >               - cm-cable-name : name of charger cable
> > > > -             - cm-cable-extcon : name of extcon dev
> > > > +             - extcon : phandles to external connector devices
> > >
> > > Somewhat less terrible, but not really. I consider extcon binding itself
> > > deprecated. I suspect 'charger-manager' as a whole probably needs to be
> > > reworked. This also is not a backwards compatible change.
>
> Right, charger-manager is a big hack. The DT node does not represent
> any hardware. The feature should be integrated into the power-supply
> core and work without any special DT bindings. I don't have the time
> to work on this, though.

Now we are trying to use charger manager to monitor our charging and
battery. So what you mean here is you want to remove the charger
manager driver and implement them in power_supply_core.c file, right?
If this is the future plan for charger manager, then we will stop to
optimize the charger manger driver.

I can help to implement or test the charger manager thing in power
supply core, so I am curious what's your thoughts if we try to
implement it in power supply core, could you explicit on? Thanks.

>
> > We are do some optimization for 'charger-manager', like this patch
> > did, we are trying to remove the deprecated extcon API.
> > And now no one use the incorrect 'cm-cable-extcon' property on
> > mainline, so no need worry backwards compatibility.
>
> As far as I can see there is no charger-manager user in mainline at
> all.
>
> -- Sebastian
>
> > > >  (optional)   - cm-cable-min : minimum current of cable
> > > >  (optional)   - cm-cable-max : maximum current of cable
> > > >
> > > > @@ -66,13 +66,13 @@ Example :
> > > >                       cm-regulator-name = "chg-reg";
> > > >                       cable@0 {
> > > >                               cm-cable-name = "USB";
> > > > -                             cm-cable-extcon = "extcon-dev.0";
> > > > +                             extcon = <&extcon_usb>;
> > > >                               cm-cable-min = <475000>;
> > > >                               cm-cable-max = <500000>;
> > > >                       };
> > > >                       cable@1 {
> > > >                               cm-cable-name = "TA";
> > > > -                             cm-cable-extcon = "extcon-dev.0";
> > > > +                             extcon = <&extcon_usb>;
> > > >                               cm-cable-min = <650000>;
> > > >                               cm-cable-max = <675000>;
> > > >                       };
> > > > diff --git a/drivers/power/supply/charger-manager.c b/drivers/power/supply/charger-manager.c
> > > > index dc0c9a6..4f28c03 100644
> > > > --- a/drivers/power/supply/charger-manager.c
> > > > +++ b/drivers/power/supply/charger-manager.c
> > > > @@ -1207,12 +1207,11 @@ static int charger_extcon_init(struct charger_manager *cm,
> > > >        */
> > > >       INIT_WORK(&cable->wq, charger_extcon_work);
> > > >       cable->nb.notifier_call = charger_extcon_notifier;
> > > > -     ret = extcon_register_interest(&cable->extcon_dev,
> > > > -                     cable->extcon_name, cable->name, &cable->nb);
> > > > -     if (ret < 0) {
> > > > -             pr_info("Cannot register extcon_dev for %s(cable: %s)\n",
> > > > -                     cable->extcon_name, cable->name);
> > > > -     }
> > > > +     ret = devm_extcon_register_notifier(cm->dev, cable->extcon_dev,
> > > > +                                         EXTCON_USB, &cable->nb);
> > > > +     if (ret < 0)
> > > > +             dev_err(cm->dev, "Cannot register extcon_dev for (cable: %s)\n",
> > > > +                     cable->name);
> > > >
> > > >       return ret;
> > > >  }
> > > > @@ -1589,15 +1588,25 @@ static struct charger_desc *of_cm_parse_desc(struct device *dev)
> > > >                               for_each_child_of_node(child, _child) {
> > > >                                       of_property_read_string(_child,
> > > >                                       "cm-cable-name", &cables->name);
> > > > -                                     of_property_read_string(_child,
> > > > -                                     "cm-cable-extcon",
> > > > -                                     &cables->extcon_name);
> > > >                                       of_property_read_u32(_child,
> > > >                                       "cm-cable-min",
> > > >                                       &cables->min_uA);
> > > >                                       of_property_read_u32(_child,
> > > >                                       "cm-cable-max",
> > > >                                       &cables->max_uA);
> > > > +
> > > > +                                     if (of_property_read_bool(_child, "extcon")) {
> > > > +                                             struct device_node *np;
> > > > +
> > > > +                                             np = of_parse_phandle(_child, "extcon", 0);
> > > > +                                             if (!np)
> > > > +                                                     return ERR_PTR(-ENODEV);
> > > > +
> > > > +                                             cables->extcon_dev = extcon_find_edev_by_node(np);
> > > > +                                             of_node_put(np);
> > > > +                                             if (IS_ERR(cables->extcon_dev))
> > > > +                                                     return ERR_PTR(PTR_ERR(cables->extcon_dev));
> > > > +                                     }
> > > >                                       cables++;
> > > >                               }
> > > >                       }
> > > > @@ -1625,7 +1634,6 @@ static int charger_manager_probe(struct platform_device *pdev)
> > > >       struct charger_desc *desc = cm_get_drv_data(pdev);
> > > >       struct charger_manager *cm;
> > > >       int ret, i = 0;
> > > > -     int j = 0;
> > > >       union power_supply_propval val;
> > > >       struct power_supply *fuel_gauge;
> > > >       struct power_supply_config psy_cfg = {};
> > > > @@ -1823,19 +1831,8 @@ static int charger_manager_probe(struct platform_device *pdev)
> > > >                               &charger->attr_g);
> > > >       }
> > > >  err_reg_extcon:
> > > > -     for (i = 0; i < desc->num_charger_regulators; i++) {
> > > > -             struct charger_regulator *charger;
> > > > -
> > > > -             charger = &desc->charger_regulators[i];
> > > > -             for (j = 0; j < charger->num_cables; j++) {
> > > > -                     struct charger_cable *cable = &charger->cables[j];
> > > > -                     /* Remove notifier block if only edev exists */
> > > > -                     if (cable->extcon_dev.edev)
> > > > -                             extcon_unregister_interest(&cable->extcon_dev);
> > > > -             }
> > > > -
> > > > +     for (i = 0; i < desc->num_charger_regulators; i++)
> > > >               regulator_put(desc->charger_regulators[i].consumer);
> > > > -     }
> > > >
> > > >       power_supply_unregister(cm->charger_psy);
> > > >
> > > > @@ -1847,7 +1844,6 @@ static int charger_manager_remove(struct platform_device *pdev)
> > > >       struct charger_manager *cm = platform_get_drvdata(pdev);
> > > >       struct charger_desc *desc = cm->desc;
> > > >       int i = 0;
> > > > -     int j = 0;
> > > >
> > > >       /* Remove from the list */
> > > >       mutex_lock(&cm_list_mtx);
> > > > @@ -1857,15 +1853,6 @@ static int charger_manager_remove(struct platform_device *pdev)
> > > >       cancel_work_sync(&setup_polling);
> > > >       cancel_delayed_work_sync(&cm_monitor_work);
> > > >
> > > > -     for (i = 0 ; i < desc->num_charger_regulators ; i++) {
> > > > -             struct charger_regulator *charger
> > > > -                             = &desc->charger_regulators[i];
> > > > -             for (j = 0 ; j < charger->num_cables ; j++) {
> > > > -                     struct charger_cable *cable = &charger->cables[j];
> > > > -                     extcon_unregister_interest(&cable->extcon_dev);
> > > > -             }
> > > > -     }
> > > > -
> > > >       for (i = 0 ; i < desc->num_charger_regulators ; i++)
> > > >               regulator_put(desc->charger_regulators[i].consumer);
> > > >
> > > > diff --git a/include/linux/power/charger-manager.h b/include/linux/power/charger-manager.h
> > > > index c4fa907..e4d0269 100644
> > > > --- a/include/linux/power/charger-manager.h
> > > > +++ b/include/linux/power/charger-manager.h
> > > > @@ -66,7 +66,7 @@ struct charger_cable {
> > > >       const char *name;
> > > >
> > > >       /* The charger-manager use Extcon framework */
> > > > -     struct extcon_specific_cable_nb extcon_dev;
> > > > +     struct extcon_dev *extcon_dev;
> > > >       struct work_struct wq;
> > > >       struct notifier_block nb;
> > > >
> > > > --
> > > > 1.7.9.5
> > > >
> >
> >
> >
> > --
> > Baolin Wang
> > Best Regards



-- 
Baolin Wang
Best Regards