[RESEND PATCH 0/3] power: Remove the deprecated extcon functions

[RESEND PATCH 0/3] power: Remove the deprecated extcon functions

[Chanwoo Choi]

This patch-set removes the deprecated notifier API of extcon framework and
then use the new extcon API[2] with the unique id[1] to indicate the each
external connector. Alter deprecated API as following:
- extcon_register_interest() -> extcon_register_notifier()
- extcon_unregister_interest() -> extcon_unregister_notifier()
- extcon_set_cable_state() -> extcon_set_cable_state_()
- extcon_get_cable_state() -> extcon_get_cable_state_()

And, extcon alters the name of USB charger connector in patch[3] as following:
- EXTCON_CHG_USB_SDP /* Standard Downstream Port */
- EXTCON_CHG_USB_DCP /* Dedicated Charging Port */
- EXTCON_CHG_USB_CDP /* Charging Downstream Port */
- EXTCON_CHG_USB_ACA /* Accessory Charger Adapter */

[1] Commit 2a9de9c0f08d61
- ("extcon: Use the unique id for external connector instead of string)
[2] Commit 046050f6e623e4
- ("extcon: Update the prototype of extcon_register_notifier() with enum extcon
[3] Commit 11eecf910bd81d
- ("extcon: Modify the id and name of external connector")

Chanwoo Choi (3):
  power: charger-manager: Replace deprecatd API of extcon
  power: axp288_charger: Replace deprecatd API of extcon
  extcon: Remove the deprecated extcon functions

 .../bindings/power_supply/charger-manager.txt      |   4 +-
 drivers/extcon/extcon.c                            | 201 +++------------------
 drivers/power/axp288_charger.c                     |  77 +++++---
 drivers/power/charger-manager.c                    |  31 ++--
 include/linux/extcon.h                             |  59 ------
 include/linux/power/charger-manager.h              |   4 +-
 6 files changed, 101 insertions(+), 275 deletions(-)

-- 
1.9.1

[RESEND PATCH 1/3] power: charger-manager: Replace deprecatd API of extcon

[Chanwoo Choi]

This patch removes the deprecated notifier API of extcon framework and then use
the new extcon API[2] with the unique id[1] to indicate the each external
connector. Alter deprecated API as following:
- extcon_register_interest() -> extcon_register_notifier()
- extcon_unregister_interest() -> extcon_unregister_notifier()

And, extcon alters the name of USB charger connector in patch[3] as following:
- EXTCON_CHG_USB_SDP /* Standard Downstream Port */
- EXTCON_CHG_USB_DCP /* Dedicated Charging Port */
- EXTCON_CHG_USB_CDP /* Charging Downstream Port */
- EXTCON_CHG_USB_ACA /* Accessory Charger Adapter */

So, the name of external charger connector are changed as following:
- "USB" -> "SDP"
- "TA" -> "DCP"

[1] Commit 2a9de9c0f08d61
- ("extcon: Use the unique id for external connector instead of string)
[2] Commit 046050f6e623e4
- ("extcon: Update the prototype of extcon_register_notifier() with enum extcon
[3] Commit 11eecf910bd81d
- ("extcon: Modify the id and name of external connector")

Signed-off-by: Chanwoo Choi <cw00.choi-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 .../bindings/power_supply/charger-manager.txt      |  4 +--
 drivers/power/charger-manager.c                    | 31 ++++++++++++++--------
 include/linux/power/charger-manager.h              |  4 +--
 3 files changed, 24 insertions(+), 15 deletions(-)

diff --git a/Documentation/devicetree/bindings/power_supply/charger-manager.txt b/Documentation/devicetree/bindings/power_supply/charger-manager.txt
index ec4fe9de3137..73193e380dc2 100644
--- a/Documentation/devicetree/bindings/power_supply/charger-manager.txt
+++ b/Documentation/devicetree/bindings/power_supply/charger-manager.txt
@@ -65,13 +65,13 @@ Example :
 		regulator@0 {
 			cm-regulator-name = "chg-reg";
 			cable@0 {
-				cm-cable-name = "USB";
+				cm-cable-id = 5; /* EXTCON_CHG_USB_SDP */
 				cm-cable-extcon = "extcon-dev.0";
 				cm-cable-min = <475000>;
 				cm-cable-max = <500000>;
 			};
 			cable@1 {
-				cm-cable-name = "TA";
+				cm-cable-id = 6; /* EXTCON_CHG_USB_DCP */
 				cm-cable-extcon = "extcon-dev.0";
 				cm-cable-min = <650000>;
 				cm-cable-max = <675000>;
diff --git a/drivers/power/charger-manager.c b/drivers/power/charger-manager.c
index e664ca7c0afd..ed63dc086eb6 100644
--- a/drivers/power/charger-manager.c
+++ b/drivers/power/charger-manager.c
@@ -1137,8 +1137,8 @@ static void charger_extcon_work(struct work_struct *work)
 		ret = regulator_set_current_limit(cable->charger->consumer,
 					cable->min_uA, cable->max_uA);
 		if (ret < 0) {
-			pr_err("Cannot set current limit of %s (%s)\n",
-			       cable->charger->regulator_name, cable->name);
+			pr_err("Cannot set current limit of %s (%d)\n",
+			       cable->charger->regulator_name, cable->id);
 			return;
 		}
 
@@ -1207,11 +1207,18 @@ 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);
+
+	cable->extcon_dev = extcon_get_extcon_dev(cable->extcon_name);
+	if (IS_ERR(cable->extcon_dev)) {
+		pr_err("Cannot get extcon_dev for %s\n", cable->extcon_name);
+		return PTR_ERR(cable->extcon_dev);
+	}
+
+	ret = extcon_register_notifier(cable->extcon_dev, cable->id,
+			&cable->nb);
 	if (ret < 0) {
-		pr_info("Cannot register extcon_dev for %s(cable: %s)\n",
-			cable->extcon_name, cable->name);
+		pr_info("Cannot register extcon_dev for %s(cable: %d)\n",
+			cable->extcon_name, cable->id);
 		ret = -EINVAL;
 	}
 
@@ -1589,8 +1596,8 @@ static struct charger_desc *of_cm_parse_desc(struct device *dev)
 				chg_regs->cables = cables;
 
 				for_each_child_of_node(child, _child) {
-					of_property_read_string(_child,
-					"cm-cable-name", &cables->name);
+					of_property_read_u32(_child,
+					"cm-cable-id", &cables->id);
 					of_property_read_string(_child,
 					"cm-cable-extcon",
 					&cables->extcon_name);
@@ -1833,8 +1840,9 @@ err_reg_extcon:
 		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);
+			if (cable->extcon_dev)
+				extcon_unregister_notifier(cable->extcon_dev,
+						cable->id, &cable->nb);
 		}
 
 		regulator_put(desc->charger_regulators[i].consumer);
@@ -1865,7 +1873,8 @@ static int charger_manager_remove(struct platform_device *pdev)
 				= &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);
+			extcon_unregister_notifier(cable->extcon_dev,
+						cable->id, &cable->nb);
 		}
 	}
 
diff --git a/include/linux/power/charger-manager.h b/include/linux/power/charger-manager.h
index c4fa907c8f14..b3404bb825d6 100644
--- a/include/linux/power/charger-manager.h
+++ b/include/linux/power/charger-manager.h
@@ -63,10 +63,10 @@ enum cm_event_types {
  */
 struct charger_cable {
 	const char *extcon_name;
-	const char *name;
+	unsigned int id;
 
 	/* 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.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RESEND PATCH 1/3] power: charger-manager: Replace deprecatd API of extcon

[Chanwoo Choi]

This patch removes the deprecated notifier API of extcon framework and then use
the new extcon API[2] with the unique id[1] to indicate the each external
connector. Alter deprecated API as following:
- extcon_register_interest() -> extcon_register_notifier()
- extcon_unregister_interest() -> extcon_unregister_notifier()

And, extcon alters the name of USB charger connector in patch[3] as following:
- EXTCON_CHG_USB_SDP /* Standard Downstream Port */
- EXTCON_CHG_USB_DCP /* Dedicated Charging Port */
- EXTCON_CHG_USB_CDP /* Charging Downstream Port */
- EXTCON_CHG_USB_ACA /* Accessory Charger Adapter */

So, the name of external charger connector are changed as following:
- "USB" -> "SDP"
- "TA" -> "DCP"

[1] Commit 2a9de9c0f08d61
- ("extcon: Use the unique id for external connector instead of string)
[2] Commit 046050f6e623e4
- ("extcon: Update the prototype of extcon_register_notifier() with enum extcon
[3] Commit 11eecf910bd81d
- ("extcon: Modify the id and name of external connector")

Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 .../bindings/power_supply/charger-manager.txt      |  4 +--
 drivers/power/charger-manager.c                    | 31 ++++++++++++++--------
 include/linux/power/charger-manager.h              |  4 +--
 3 files changed, 24 insertions(+), 15 deletions(-)

diff --git a/Documentation/devicetree/bindings/power_supply/charger-manager.txt b/Documentation/devicetree/bindings/power_supply/charger-manager.txt
index ec4fe9de3137..73193e380dc2 100644
--- a/Documentation/devicetree/bindings/power_supply/charger-manager.txt
+++ b/Documentation/devicetree/bindings/power_supply/charger-manager.txt
@@ -65,13 +65,13 @@ Example :
 		regulator@0 {
 			cm-regulator-name = "chg-reg";
 			cable@0 {
-				cm-cable-name = "USB";
+				cm-cable-id = 5; /* EXTCON_CHG_USB_SDP */
 				cm-cable-extcon = "extcon-dev.0";
 				cm-cable-min = <475000>;
 				cm-cable-max = <500000>;
 			};
 			cable@1 {
-				cm-cable-name = "TA";
+				cm-cable-id = 6; /* EXTCON_CHG_USB_DCP */
 				cm-cable-extcon = "extcon-dev.0";
 				cm-cable-min = <650000>;
 				cm-cable-max = <675000>;
diff --git a/drivers/power/charger-manager.c b/drivers/power/charger-manager.c
index e664ca7c0afd..ed63dc086eb6 100644
--- a/drivers/power/charger-manager.c
+++ b/drivers/power/charger-manager.c
@@ -1137,8 +1137,8 @@ static void charger_extcon_work(struct work_struct *work)
 		ret = regulator_set_current_limit(cable->charger->consumer,
 					cable->min_uA, cable->max_uA);
 		if (ret < 0) {
-			pr_err("Cannot set current limit of %s (%s)\n",
-			       cable->charger->regulator_name, cable->name);
+			pr_err("Cannot set current limit of %s (%d)\n",
+			       cable->charger->regulator_name, cable->id);
 			return;
 		}
 
@@ -1207,11 +1207,18 @@ 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);
+
+	cable->extcon_dev = extcon_get_extcon_dev(cable->extcon_name);
+	if (IS_ERR(cable->extcon_dev)) {
+		pr_err("Cannot get extcon_dev for %s\n", cable->extcon_name);
+		return PTR_ERR(cable->extcon_dev);
+	}
+
+	ret = extcon_register_notifier(cable->extcon_dev, cable->id,
+			&cable->nb);
 	if (ret < 0) {
-		pr_info("Cannot register extcon_dev for %s(cable: %s)\n",
-			cable->extcon_name, cable->name);
+		pr_info("Cannot register extcon_dev for %s(cable: %d)\n",
+			cable->extcon_name, cable->id);
 		ret = -EINVAL;
 	}
 
@@ -1589,8 +1596,8 @@ static struct charger_desc *of_cm_parse_desc(struct device *dev)
 				chg_regs->cables = cables;
 
 				for_each_child_of_node(child, _child) {
-					of_property_read_string(_child,
-					"cm-cable-name", &cables->name);
+					of_property_read_u32(_child,
+					"cm-cable-id", &cables->id);
 					of_property_read_string(_child,
 					"cm-cable-extcon",
 					&cables->extcon_name);
@@ -1833,8 +1840,9 @@ err_reg_extcon:
 		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);
+			if (cable->extcon_dev)
+				extcon_unregister_notifier(cable->extcon_dev,
+						cable->id, &cable->nb);
 		}
 
 		regulator_put(desc->charger_regulators[i].consumer);
@@ -1865,7 +1873,8 @@ static int charger_manager_remove(struct platform_device *pdev)
 				= &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);
+			extcon_unregister_notifier(cable->extcon_dev,
+						cable->id, &cable->nb);
 		}
 	}
 
diff --git a/include/linux/power/charger-manager.h b/include/linux/power/charger-manager.h
index c4fa907c8f14..b3404bb825d6 100644
--- a/include/linux/power/charger-manager.h
+++ b/include/linux/power/charger-manager.h
@@ -63,10 +63,10 @@ enum cm_event_types {
  */
 struct charger_cable {
 	const char *extcon_name;
-	const char *name;
+	unsigned int id;
 
 	/* 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.9.1

Re: [RESEND PATCH 1/3] power: charger-manager: Replace deprecatd API of extcon

[Sebastian Reichel]

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

Hi,

On Fri, Apr 15, 2016 at 09:43:34AM +0900, Chanwoo Choi wrote:
> This patch removes the deprecated notifier API of extcon framework and then use
> the new extcon API[2] with the unique id[1] to indicate the each external
> connector. Alter deprecated API as following:
> - extcon_register_interest() -> extcon_register_notifier()
> - extcon_unregister_interest() -> extcon_unregister_notifier()
> 
> And, extcon alters the name of USB charger connector in patch[3] as following:
> - EXTCON_CHG_USB_SDP /* Standard Downstream Port */
> - EXTCON_CHG_USB_DCP /* Dedicated Charging Port */
> - EXTCON_CHG_USB_CDP /* Charging Downstream Port */
> - EXTCON_CHG_USB_ACA /* Accessory Charger Adapter */
> 
> So, the name of external charger connector are changed as following:
> - "USB" -> "SDP"
> - "TA" -> "DCP"
> 
> [1] Commit 2a9de9c0f08d61
> - ("extcon: Use the unique id for external connector instead of string)
> [2] Commit 046050f6e623e4
> - ("extcon: Update the prototype of extcon_register_notifier() with enum extcon
> [3] Commit 11eecf910bd81d
> - ("extcon: Modify the id and name of external connector")
> 
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> ---
>  .../bindings/power_supply/charger-manager.txt      |  4 +--
>  drivers/power/charger-manager.c                    | 31 ++++++++++++++--------
>  include/linux/power/charger-manager.h              |  4 +--
>  3 files changed, 24 insertions(+), 15 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/power_supply/charger-manager.txt b/Documentation/devicetree/bindings/power_supply/charger-manager.txt
> index ec4fe9de3137..73193e380dc2 100644
> --- a/Documentation/devicetree/bindings/power_supply/charger-manager.txt
> +++ b/Documentation/devicetree/bindings/power_supply/charger-manager.txt
> @@ -65,13 +65,13 @@ Example :
>  		regulator@0 {
>  			cm-regulator-name = "chg-reg";
>  			cable@0 {
> -				cm-cable-name = "USB";
> +				cm-cable-id = 5; /* EXTCON_CHG_USB_SDP */
>  				cm-cable-extcon = "extcon-dev.0";
>  				cm-cable-min = <475000>;
>  				cm-cable-max = <500000>;
>  			};
>  			cable@1 {
> -				cm-cable-name = "TA";
> +				cm-cable-id = 6; /* EXTCON_CHG_USB_DCP */
>  				cm-cable-extcon = "extcon-dev.0";
>  				cm-cable-min = <650000>;
>  				cm-cable-max = <675000>;

This breaks DT ABI. Looks like charger-manager is not used in
mainline, but I guess ther should be an explicit Acked-By from
a DT binding maintainer. Also I think the defines should be in
some header includable from DTS, so that something like this
can be done:

cm-cable-id = <CHARGER_USB_DCP>;

Apart from that:

Acked-By: Sebastian Reichel <sre@kernel.org>

-- Sebastian

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

Re: [RESEND PATCH 1/3] power: charger-manager: Replace deprecatd API of extcon

[Chanwoo Choi]

Hi Sebastian,

On 2016년 04월 15일 23:13, Sebastian Reichel wrote:
> Hi,
> 
> On Fri, Apr 15, 2016 at 09:43:34AM +0900, Chanwoo Choi wrote:
>> This patch removes the deprecated notifier API of extcon framework and then use
>> the new extcon API[2] with the unique id[1] to indicate the each external
>> connector. Alter deprecated API as following:
>> - extcon_register_interest() -> extcon_register_notifier()
>> - extcon_unregister_interest() -> extcon_unregister_notifier()
>>
>> And, extcon alters the name of USB charger connector in patch[3] as following:
>> - EXTCON_CHG_USB_SDP /* Standard Downstream Port */
>> - EXTCON_CHG_USB_DCP /* Dedicated Charging Port */
>> - EXTCON_CHG_USB_CDP /* Charging Downstream Port */
>> - EXTCON_CHG_USB_ACA /* Accessory Charger Adapter */
>>
>> So, the name of external charger connector are changed as following:
>> - "USB" -> "SDP"
>> - "TA" -> "DCP"
>>
>> [1] Commit 2a9de9c0f08d61
>> - ("extcon: Use the unique id for external connector instead of string)
>> [2] Commit 046050f6e623e4
>> - ("extcon: Update the prototype of extcon_register_notifier() with enum extcon
>> [3] Commit 11eecf910bd81d
>> - ("extcon: Modify the id and name of external connector")
>>
>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>> ---
>>  .../bindings/power_supply/charger-manager.txt      |  4 +--
>>  drivers/power/charger-manager.c                    | 31 ++++++++++++++--------
>>  include/linux/power/charger-manager.h              |  4 +--
>>  3 files changed, 24 insertions(+), 15 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/power_supply/charger-manager.txt b/Documentation/devicetree/bindings/power_supply/charger-manager.txt
>> index ec4fe9de3137..73193e380dc2 100644
>> --- a/Documentation/devicetree/bindings/power_supply/charger-manager.txt
>> +++ b/Documentation/devicetree/bindings/power_supply/charger-manager.txt
>> @@ -65,13 +65,13 @@ Example :
>>  		regulator@0 {
>>  			cm-regulator-name = "chg-reg";
>>  			cable@0 {
>> -				cm-cable-name = "USB";
>> +				cm-cable-id = 5; /* EXTCON_CHG_USB_SDP */
>>  				cm-cable-extcon = "extcon-dev.0";
>>  				cm-cable-min = <475000>;
>>  				cm-cable-max = <500000>;
>>  			};
>>  			cable@1 {
>> -				cm-cable-name = "TA";
>> +				cm-cable-id = 6; /* EXTCON_CHG_USB_DCP */
>>  				cm-cable-extcon = "extcon-dev.0";
>>  				cm-cable-min = <650000>;
>>  				cm-cable-max = <675000>;
> 
> This breaks DT ABI. Looks like charger-manager is not used in
> mainline,but I guess ther should be an explicit Acked-By from
> a DT binding maintainer.

As I mentioned on other mail to Rob, the purpose of this patch
removes the deprecated the EXTCON APIs. So, I touched the DT binding.

About DT ABI about charger-manager, we should handle it on separate patches.

Also I think the defines should be in
> some header includable from DTS, so that something like this
> can be done:
> 
> cm-cable-id = <CHARGER_USB_DCP>;


I agree to use some definition for the kind of charger cable.
But, Not yet. I'm preparing the EXTCON update to use the definitions
on Device Tree file. I'll send some separate patches in the near future.
> 
> Apart from that:
> 
> Acked-By: Sebastian Reichel <sre@kernel.org>

Thanks for review.

Best Regards,
Chanwoo Choi

Re: [RESEND PATCH 1/3] power: charger-manager: Replace deprecatd API of extcon

[Rob Herring]

On Fri, Apr 15, 2016 at 04:13:18PM +0200, Sebastian Reichel wrote:
> Hi,
> 
> On Fri, Apr 15, 2016 at 09:43:34AM +0900, Chanwoo Choi wrote:
> > This patch removes the deprecated notifier API of extcon framework and then use
> > the new extcon API[2] with the unique id[1] to indicate the each external
> > connector. Alter deprecated API as following:
> > - extcon_register_interest() -> extcon_register_notifier()
> > - extcon_unregister_interest() -> extcon_unregister_notifier()
> > 
> > And, extcon alters the name of USB charger connector in patch[3] as following:
> > - EXTCON_CHG_USB_SDP /* Standard Downstream Port */
> > - EXTCON_CHG_USB_DCP /* Dedicated Charging Port */
> > - EXTCON_CHG_USB_CDP /* Charging Downstream Port */
> > - EXTCON_CHG_USB_ACA /* Accessory Charger Adapter */
> > 
> > So, the name of external charger connector are changed as following:
> > - "USB" -> "SDP"
> > - "TA" -> "DCP"
> > 
> > [1] Commit 2a9de9c0f08d61
> > - ("extcon: Use the unique id for external connector instead of string)
> > [2] Commit 046050f6e623e4
> > - ("extcon: Update the prototype of extcon_register_notifier() with enum extcon
> > [3] Commit 11eecf910bd81d
> > - ("extcon: Modify the id and name of external connector")
> > 
> > Signed-off-by: Chanwoo Choi <cw00.choi-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> > ---
> >  .../bindings/power_supply/charger-manager.txt      |  4 +--
> >  drivers/power/charger-manager.c                    | 31 ++++++++++++++--------
> >  include/linux/power/charger-manager.h              |  4 +--
> >  3 files changed, 24 insertions(+), 15 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/power_supply/charger-manager.txt b/Documentation/devicetree/bindings/power_supply/charger-manager.txt
> > index ec4fe9de3137..73193e380dc2 100644
> > --- a/Documentation/devicetree/bindings/power_supply/charger-manager.txt
> > +++ b/Documentation/devicetree/bindings/power_supply/charger-manager.txt
> > @@ -65,13 +65,13 @@ Example :
> >  		regulator@0 {
> >  			cm-regulator-name = "chg-reg";
> >  			cable@0 {
> > -				cm-cable-name = "USB";
> > +				cm-cable-id = 5; /* EXTCON_CHG_USB_SDP */
> >  				cm-cable-extcon = "extcon-dev.0";
> >  				cm-cable-min = <475000>;
> >  				cm-cable-max = <500000>;
> >  			};
> >  			cable@1 {
> > -				cm-cable-name = "TA";
> > +				cm-cable-id = 6; /* EXTCON_CHG_USB_DCP */
> >  				cm-cable-extcon = "extcon-dev.0";
> >  				cm-cable-min = <650000>;
> >  				cm-cable-max = <675000>;
> 
> This breaks DT ABI. Looks like charger-manager is not used in
> mainline, but I guess ther should be an explicit Acked-By from
> a DT binding maintainer. Also I think the defines should be in
> some header includable from DTS, so that something like this
> can be done:

I already commented that the binding to start with looks like crap 
(For starters, "extcon-dev.0" is a Linux device name?) and this does not 
appear to be an improvement.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RESEND PATCH 1/3] power: charger-manager: Replace deprecatd API of extcon

[Rob Herring]

On Fri, Apr 15, 2016 at 04:13:18PM +0200, Sebastian Reichel wrote:
> Hi,
> 
> On Fri, Apr 15, 2016 at 09:43:34AM +0900, Chanwoo Choi wrote:
> > This patch removes the deprecated notifier API of extcon framework and then use
> > the new extcon API[2] with the unique id[1] to indicate the each external
> > connector. Alter deprecated API as following:
> > - extcon_register_interest() -> extcon_register_notifier()
> > - extcon_unregister_interest() -> extcon_unregister_notifier()
> > 
> > And, extcon alters the name of USB charger connector in patch[3] as following:
> > - EXTCON_CHG_USB_SDP /* Standard Downstream Port */
> > - EXTCON_CHG_USB_DCP /* Dedicated Charging Port */
> > - EXTCON_CHG_USB_CDP /* Charging Downstream Port */
> > - EXTCON_CHG_USB_ACA /* Accessory Charger Adapter */
> > 
> > So, the name of external charger connector are changed as following:
> > - "USB" -> "SDP"
> > - "TA" -> "DCP"
> > 
> > [1] Commit 2a9de9c0f08d61
> > - ("extcon: Use the unique id for external connector instead of string)
> > [2] Commit 046050f6e623e4
> > - ("extcon: Update the prototype of extcon_register_notifier() with enum extcon
> > [3] Commit 11eecf910bd81d
> > - ("extcon: Modify the id and name of external connector")
> > 
> > Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> > ---
> >  .../bindings/power_supply/charger-manager.txt      |  4 +--
> >  drivers/power/charger-manager.c                    | 31 ++++++++++++++--------
> >  include/linux/power/charger-manager.h              |  4 +--
> >  3 files changed, 24 insertions(+), 15 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/power_supply/charger-manager.txt b/Documentation/devicetree/bindings/power_supply/charger-manager.txt
> > index ec4fe9de3137..73193e380dc2 100644
> > --- a/Documentation/devicetree/bindings/power_supply/charger-manager.txt
> > +++ b/Documentation/devicetree/bindings/power_supply/charger-manager.txt
> > @@ -65,13 +65,13 @@ Example :
> >  		regulator@0 {
> >  			cm-regulator-name = "chg-reg";
> >  			cable@0 {
> > -				cm-cable-name = "USB";
> > +				cm-cable-id = 5; /* EXTCON_CHG_USB_SDP */
> >  				cm-cable-extcon = "extcon-dev.0";
> >  				cm-cable-min = <475000>;
> >  				cm-cable-max = <500000>;
> >  			};
> >  			cable@1 {
> > -				cm-cable-name = "TA";
> > +				cm-cable-id = 6; /* EXTCON_CHG_USB_DCP */
> >  				cm-cable-extcon = "extcon-dev.0";
> >  				cm-cable-min = <650000>;
> >  				cm-cable-max = <675000>;
> 
> This breaks DT ABI. Looks like charger-manager is not used in
> mainline, but I guess ther should be an explicit Acked-By from
> a DT binding maintainer. Also I think the defines should be in
> some header includable from DTS, so that something like this
> can be done:

I already commented that the binding to start with looks like crap 
(For starters, "extcon-dev.0" is a Linux device name?) and this does not 
appear to be an improvement.

Rob

Re: [RESEND PATCH 1/3] power: charger-manager: Replace deprecatd API of extcon

[Chanwoo Choi]

Hi Rob,

On 2016년 04월 19일 02:24, Rob Herring wrote:
> On Fri, Apr 15, 2016 at 04:13:18PM +0200, Sebastian Reichel wrote:
>> Hi,
>>
>> On Fri, Apr 15, 2016 at 09:43:34AM +0900, Chanwoo Choi wrote:
>>> This patch removes the deprecated notifier API of extcon framework and then use
>>> the new extcon API[2] with the unique id[1] to indicate the each external
>>> connector. Alter deprecated API as following:
>>> - extcon_register_interest() -> extcon_register_notifier()
>>> - extcon_unregister_interest() -> extcon_unregister_notifier()
>>>
>>> And, extcon alters the name of USB charger connector in patch[3] as following:
>>> - EXTCON_CHG_USB_SDP /* Standard Downstream Port */
>>> - EXTCON_CHG_USB_DCP /* Dedicated Charging Port */
>>> - EXTCON_CHG_USB_CDP /* Charging Downstream Port */
>>> - EXTCON_CHG_USB_ACA /* Accessory Charger Adapter */
>>>
>>> So, the name of external charger connector are changed as following:
>>> - "USB" -> "SDP"
>>> - "TA" -> "DCP"
>>>
>>> [1] Commit 2a9de9c0f08d61
>>> - ("extcon: Use the unique id for external connector instead of string)
>>> [2] Commit 046050f6e623e4
>>> - ("extcon: Update the prototype of extcon_register_notifier() with enum extcon
>>> [3] Commit 11eecf910bd81d
>>> - ("extcon: Modify the id and name of external connector")
>>>
>>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>>> ---
>>>  .../bindings/power_supply/charger-manager.txt      |  4 +--
>>>  drivers/power/charger-manager.c                    | 31 ++++++++++++++--------
>>>  include/linux/power/charger-manager.h              |  4 +--
>>>  3 files changed, 24 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/power_supply/charger-manager.txt b/Documentation/devicetree/bindings/power_supply/charger-manager.txt
>>> index ec4fe9de3137..73193e380dc2 100644
>>> --- a/Documentation/devicetree/bindings/power_supply/charger-manager.txt
>>> +++ b/Documentation/devicetree/bindings/power_supply/charger-manager.txt
>>> @@ -65,13 +65,13 @@ Example :
>>>  		regulator@0 {
>>>  			cm-regulator-name = "chg-reg";
>>>  			cable@0 {
>>> -				cm-cable-name = "USB";
>>> +				cm-cable-id = 5; /* EXTCON_CHG_USB_SDP */
>>>  				cm-cable-extcon = "extcon-dev.0";
>>>  				cm-cable-min = <475000>;
>>>  				cm-cable-max = <500000>;
>>>  			};
>>>  			cable@1 {
>>> -				cm-cable-name = "TA";
>>> +				cm-cable-id = 6; /* EXTCON_CHG_USB_DCP */
>>>  				cm-cable-extcon = "extcon-dev.0";
>>>  				cm-cable-min = <650000>;
>>>  				cm-cable-max = <675000>;
>>
>> This breaks DT ABI. Looks like charger-manager is not used in
>> mainline, but I guess ther should be an explicit Acked-By from
>> a DT binding maintainer. Also I think the defines should be in
>> some header includable from DTS, so that something like this
>> can be done:
> 
> I already commented that the binding to start with looks like crap 
> (For starters, "extcon-dev.0" is a Linux device name?) and this does not 
> appear to be an improvement.

I already replied for your comment on other mail[1].
[1] https://lkml.org/lkml/2016/3/31/199

This patch never update the charger-manager.
Just remove the deprecated EXTCON API to prevent build error.

Thanks,
Chanwoo Choi

[RESEND PATCH 2/3] power: axp288_charger: Replace deprecatd API of extcon

[Chanwoo Choi]

This patch removes the deprecated notifier API of extcon framework and then use
the new extcon API[2] with the unique id[1] to indicate the each external
connector. Alter deprecated API as following:
- extcon_register_interest() -> extcon_register_notifier()
- extcon_unregister_interest() -> extcon_unregister_notifier()
- extcon_get_cable_state() -> extcon_get_cable_state_()

And, extcon alters the name of USB charger connector in patch[3] as following:
- EXTCON_CHG_USB_SDP /* Standard Downstream Port */
- EXTCON_CHG_USB_DCP /* Dedicated Charging Port */
- EXTCON_CHG_USB_CDP /* Charging Downstream Port */
- EXTCON_CHG_USB_ACA /* Accessory Charger Adapter */

[1] Commit 2a9de9c0f08d61
- ("extcon: Use the unique id for external connector instead of string)
[2] Commit 046050f6e623e4
- ("extcon: Update the prototype of extcon_register_notifier() with enum extcon
[3] Commit 11eecf910bd81d
- ("extcon: Modify the id and name of external connector")

Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 drivers/power/axp288_charger.c | 77 +++++++++++++++++++++++++++++-------------
 1 file changed, 53 insertions(+), 24 deletions(-)

diff --git a/drivers/power/axp288_charger.c b/drivers/power/axp288_charger.c
index e4d569f57acc..e5c2569befa6 100644
--- a/drivers/power/axp288_charger.c
+++ b/drivers/power/axp288_charger.c
@@ -129,10 +129,6 @@
 
 #define AXP288_EXTCON_DEV_NAME		"axp288_extcon"
 
-#define AXP288_EXTCON_SLOW_CHARGER		"SLOW-CHARGER"
-#define AXP288_EXTCON_DOWNSTREAM_CHARGER	"CHARGE-DOWNSTREAM"
-#define AXP288_EXTCON_FAST_CHARGER		"FAST-CHARGER"
-
 enum {
 	VBUS_OV_IRQ = 0,
 	CHARGE_DONE_IRQ,
@@ -158,7 +154,7 @@ struct axp288_chrg_info {
 	/* OTG/Host mode */
 	struct {
 		struct work_struct work;
-		struct extcon_specific_cable_nb cable;
+		struct extcon_dev *cable;
 		struct notifier_block id_nb;
 		bool id_short;
 	} otg;
@@ -586,17 +582,15 @@ static void axp288_charger_extcon_evt_worker(struct work_struct *work)
 	bool old_connected = info->cable.connected;
 
 	/* Determine cable/charger type */
-	if (extcon_get_cable_state(edev, AXP288_EXTCON_SLOW_CHARGER) > 0) {
+	if (extcon_get_cable_state_(edev, EXTCON_CHG_USB_SDP) > 0) {
 		dev_dbg(&info->pdev->dev, "USB SDP charger  is connected");
 		info->cable.connected = true;
 		info->cable.chg_type = POWER_SUPPLY_TYPE_USB;
-	} else if (extcon_get_cable_state(edev,
-				AXP288_EXTCON_DOWNSTREAM_CHARGER) > 0) {
+	} else if (extcon_get_cable_state_(edev, EXTCON_CHG_USB_CDP) > 0) {
 		dev_dbg(&info->pdev->dev, "USB CDP charger is connected");
 		info->cable.connected = true;
 		info->cable.chg_type = POWER_SUPPLY_TYPE_USB_CDP;
-	} else if (extcon_get_cable_state(edev,
-					AXP288_EXTCON_FAST_CHARGER) > 0) {
+	} else if (extcon_get_cable_state_(edev, EXTCON_CHG_USB_DCP) > 0) {
 		dev_dbg(&info->pdev->dev, "USB DCP charger is connected");
 		info->cable.connected = true;
 		info->cable.chg_type = POWER_SUPPLY_TYPE_USB_DCP;
@@ -692,8 +686,8 @@ static int axp288_charger_handle_otg_evt(struct notifier_block *nb,
 {
 	struct axp288_chrg_info *info =
 	    container_of(nb, struct axp288_chrg_info, otg.id_nb);
-	struct extcon_dev *edev = param;
-	int usb_host = extcon_get_cable_state(edev, "USB-Host");
+	struct extcon_dev *edev = info->otg.cable;
+	int usb_host = extcon_get_cable_state_(edev, EXTCON_USB_HOST);
 
 	dev_dbg(&info->pdev->dev, "external connector USB-Host is %s\n",
 				usb_host ? "attached" : "detached");
@@ -848,10 +842,33 @@ static int axp288_charger_probe(struct platform_device *pdev)
 	/* Register for extcon notification */
 	INIT_WORK(&info->cable.work, axp288_charger_extcon_evt_worker);
 	info->cable.nb.notifier_call = axp288_charger_handle_cable_evt;
-	ret = extcon_register_notifier(info->cable.edev, EXTCON_NONE, &info->cable.nb);
+	ret = extcon_register_notifier(info->cable.edev, EXTCON_CHG_USB_SDP,
+					&info->cable.nb);
+	if (ret) {
+		dev_err(&info->pdev->dev,
+			"failed to register extcon notifier for SDP %d\n", ret);
+		return ret;
+	}
+
+	ret = extcon_register_notifier(info->cable.edev, EXTCON_CHG_USB_CDP,
+					&info->cable.nb);
+	if (ret) {
+		dev_err(&info->pdev->dev,
+			"failed to register extcon notifier for CDP %d\n", ret);
+		extcon_unregister_notifier(info->cable.edev,
+				EXTCON_CHG_USB_SDP, &info->cable.nb);
+		return ret;
+	}
+
+	ret = extcon_register_notifier(info->cable.edev, EXTCON_CHG_USB_DCP,
+					&info->cable.nb);
 	if (ret) {
 		dev_err(&info->pdev->dev,
-			"failed to register extcon notifier %d\n", ret);
+			"failed to register extcon notifier for DCP %d\n", ret);
+		extcon_unregister_notifier(info->cable.edev,
+				EXTCON_CHG_USB_SDP, &info->cable.nb);
+		extcon_unregister_notifier(info->cable.edev,
+				EXTCON_CHG_USB_CDP, &info->cable.nb);
 		return ret;
 	}
 
@@ -871,14 +888,14 @@ static int axp288_charger_probe(struct platform_device *pdev)
 	/* Register for OTG notification */
 	INIT_WORK(&info->otg.work, axp288_charger_otg_evt_worker);
 	info->otg.id_nb.notifier_call = axp288_charger_handle_otg_evt;
-	ret = extcon_register_interest(&info->otg.cable, NULL, "USB-Host",
+	ret = extcon_register_notifier(info->otg.cable, EXTCON_USB_HOST,
 				       &info->otg.id_nb);
 	if (ret)
 		dev_warn(&pdev->dev, "failed to register otg notifier\n");
 
-	if (info->otg.cable.edev)
-		info->otg.id_short = extcon_get_cable_state(
-					info->otg.cable.edev, "USB-Host");
+	if (info->otg.cable)
+		info->otg.id_short = extcon_get_cable_state_(
+					info->otg.cable, EXTCON_USB_HOST);
 
 	/* Register charger interrupts */
 	for (i = 0; i < CHRG_INTR_END; i++) {
@@ -905,11 +922,17 @@ static int axp288_charger_probe(struct platform_device *pdev)
 	return 0;
 
 intr_reg_failed:
-	if (info->otg.cable.edev)
-		extcon_unregister_interest(&info->otg.cable);
+	if (info->otg.cable)
+		extcon_unregister_notifier(info->otg.cable, EXTCON_USB_HOST,
+					&info->otg.id_nb);
 	power_supply_unregister(info->psy_usb);
 psy_reg_failed:
-	extcon_unregister_notifier(info->cable.edev, EXTCON_NONE, &info->cable.nb);
+	extcon_unregister_notifier(info->cable.edev, EXTCON_CHG_USB_SDP,
+					&info->cable.nb);
+	extcon_unregister_notifier(info->cable.edev, EXTCON_CHG_USB_SDP,
+					&info->cable.nb);
+	extcon_unregister_notifier(info->cable.edev, EXTCON_CHG_USB_DCP,
+					&info->cable.nb);
 	return ret;
 }
 
@@ -917,10 +940,16 @@ static int axp288_charger_remove(struct platform_device *pdev)
 {
 	struct axp288_chrg_info *info =  dev_get_drvdata(&pdev->dev);
 
-	if (info->otg.cable.edev)
-		extcon_unregister_interest(&info->otg.cable);
+	if (info->otg.cable)
+		extcon_unregister_notifier(info->otg.cable, EXTCON_USB_HOST,
+					&info->otg.id_nb);
 
-	extcon_unregister_notifier(info->cable.edev, EXTCON_NONE, &info->cable.nb);
+	extcon_unregister_notifier(info->cable.edev, EXTCON_CHG_USB_SDP,
+					&info->cable.nb);
+	extcon_unregister_notifier(info->cable.edev, EXTCON_CHG_USB_SDP,
+					&info->cable.nb);
+	extcon_unregister_notifier(info->cable.edev, EXTCON_CHG_USB_DCP,
+					&info->cable.nb);
 	power_supply_unregister(info->psy_usb);
 
 	return 0;
-- 
1.9.1

Re: [RESEND PATCH 2/3] power: axp288_charger: Replace deprecatd API of extcon

[Sebastian Reichel]

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

Hi,

On Fri, Apr 15, 2016 at 09:43:35AM +0900, Chanwoo Choi wrote:
> This patch removes the deprecated notifier API of extcon framework and then use
> the new extcon API[2] with the unique id[1] to indicate the each external
> connector. Alter deprecated API as following:
> - extcon_register_interest() -> extcon_register_notifier()
> - extcon_unregister_interest() -> extcon_unregister_notifier()
> - extcon_get_cable_state() -> extcon_get_cable_state_()
> 
> And, extcon alters the name of USB charger connector in patch[3] as following:
> - EXTCON_CHG_USB_SDP /* Standard Downstream Port */
> - EXTCON_CHG_USB_DCP /* Dedicated Charging Port */
> - EXTCON_CHG_USB_CDP /* Charging Downstream Port */
> - EXTCON_CHG_USB_ACA /* Accessory Charger Adapter */
> 
> [1] Commit 2a9de9c0f08d61
> - ("extcon: Use the unique id for external connector instead of string)
> [2] Commit 046050f6e623e4
> - ("extcon: Update the prototype of extcon_register_notifier() with enum extcon
> [3] Commit 11eecf910bd81d
> - ("extcon: Modify the id and name of external connector")
> 
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> ---
>  drivers/power/axp288_charger.c | 77 +++++++++++++++++++++++++++++-------------
>  1 file changed, 53 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/power/axp288_charger.c b/drivers/power/axp288_charger.c
> index e4d569f57acc..e5c2569befa6 100644
> --- a/drivers/power/axp288_charger.c
> +++ b/drivers/power/axp288_charger.c
> @@ -129,10 +129,6 @@
>  
>  #define AXP288_EXTCON_DEV_NAME		"axp288_extcon"
>  
> -#define AXP288_EXTCON_SLOW_CHARGER		"SLOW-CHARGER"
> -#define AXP288_EXTCON_DOWNSTREAM_CHARGER	"CHARGE-DOWNSTREAM"
> -#define AXP288_EXTCON_FAST_CHARGER		"FAST-CHARGER"
> -
>  enum {
>  	VBUS_OV_IRQ = 0,
>  	CHARGE_DONE_IRQ,
> @@ -158,7 +154,7 @@ struct axp288_chrg_info {
>  	/* OTG/Host mode */
>  	struct {
>  		struct work_struct work;
> -		struct extcon_specific_cable_nb cable;
> +		struct extcon_dev *cable;
>  		struct notifier_block id_nb;
>  		bool id_short;
>  	} otg;
> @@ -586,17 +582,15 @@ static void axp288_charger_extcon_evt_worker(struct work_struct *work)
>  	bool old_connected = info->cable.connected;
>  
>  	/* Determine cable/charger type */
> -	if (extcon_get_cable_state(edev, AXP288_EXTCON_SLOW_CHARGER) > 0) {
> +	if (extcon_get_cable_state_(edev, EXTCON_CHG_USB_SDP) > 0) {
>  		dev_dbg(&info->pdev->dev, "USB SDP charger  is connected");
>  		info->cable.connected = true;
>  		info->cable.chg_type = POWER_SUPPLY_TYPE_USB;
> -	} else if (extcon_get_cable_state(edev,
> -				AXP288_EXTCON_DOWNSTREAM_CHARGER) > 0) {
> +	} else if (extcon_get_cable_state_(edev, EXTCON_CHG_USB_CDP) > 0) {
>  		dev_dbg(&info->pdev->dev, "USB CDP charger is connected");
>  		info->cable.connected = true;
>  		info->cable.chg_type = POWER_SUPPLY_TYPE_USB_CDP;
> -	} else if (extcon_get_cable_state(edev,
> -					AXP288_EXTCON_FAST_CHARGER) > 0) {
> +	} else if (extcon_get_cable_state_(edev, EXTCON_CHG_USB_DCP) > 0) {
>  		dev_dbg(&info->pdev->dev, "USB DCP charger is connected");
>  		info->cable.connected = true;
>  		info->cable.chg_type = POWER_SUPPLY_TYPE_USB_DCP;
> @@ -692,8 +686,8 @@ static int axp288_charger_handle_otg_evt(struct notifier_block *nb,
>  {
>  	struct axp288_chrg_info *info =
>  	    container_of(nb, struct axp288_chrg_info, otg.id_nb);
> -	struct extcon_dev *edev = param;
> -	int usb_host = extcon_get_cable_state(edev, "USB-Host");
> +	struct extcon_dev *edev = info->otg.cable;
> +	int usb_host = extcon_get_cable_state_(edev, EXTCON_USB_HOST);
>  
>  	dev_dbg(&info->pdev->dev, "external connector USB-Host is %s\n",
>  				usb_host ? "attached" : "detached");
> @@ -848,10 +842,33 @@ static int axp288_charger_probe(struct platform_device *pdev)
>  	/* Register for extcon notification */
>  	INIT_WORK(&info->cable.work, axp288_charger_extcon_evt_worker);
>  	info->cable.nb.notifier_call = axp288_charger_handle_cable_evt;
> -	ret = extcon_register_notifier(info->cable.edev, EXTCON_NONE, &info->cable.nb);
> +	ret = extcon_register_notifier(info->cable.edev, EXTCON_CHG_USB_SDP,
> +					&info->cable.nb);
> +	if (ret) {
> +		dev_err(&info->pdev->dev,
> +			"failed to register extcon notifier for SDP %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = extcon_register_notifier(info->cable.edev, EXTCON_CHG_USB_CDP,
> +					&info->cable.nb);
> +	if (ret) {
> +		dev_err(&info->pdev->dev,
> +			"failed to register extcon notifier for CDP %d\n", ret);
> +		extcon_unregister_notifier(info->cable.edev,
> +				EXTCON_CHG_USB_SDP, &info->cable.nb);
> +		return ret;
> +	}
> +
> +	ret = extcon_register_notifier(info->cable.edev, EXTCON_CHG_USB_DCP,
> +					&info->cable.nb);
>  	if (ret) {
>  		dev_err(&info->pdev->dev,
> -			"failed to register extcon notifier %d\n", ret);
> +			"failed to register extcon notifier for DCP %d\n", ret);
> +		extcon_unregister_notifier(info->cable.edev,
> +				EXTCON_CHG_USB_SDP, &info->cable.nb);
> +		extcon_unregister_notifier(info->cable.edev,
> +				EXTCON_CHG_USB_CDP, &info->cable.nb);
>  		return ret;
>  	}
>  
> @@ -871,14 +888,14 @@ static int axp288_charger_probe(struct platform_device *pdev)
>  	/* Register for OTG notification */
>  	INIT_WORK(&info->otg.work, axp288_charger_otg_evt_worker);
>  	info->otg.id_nb.notifier_call = axp288_charger_handle_otg_evt;
> -	ret = extcon_register_interest(&info->otg.cable, NULL, "USB-Host",
> +	ret = extcon_register_notifier(info->otg.cable, EXTCON_USB_HOST,
>  				       &info->otg.id_nb);
>  	if (ret)
>  		dev_warn(&pdev->dev, "failed to register otg notifier\n");
>  
> -	if (info->otg.cable.edev)
> -		info->otg.id_short = extcon_get_cable_state(
> -					info->otg.cable.edev, "USB-Host");
> +	if (info->otg.cable)
> +		info->otg.id_short = extcon_get_cable_state_(
> +					info->otg.cable, EXTCON_USB_HOST);
>  
>  	/* Register charger interrupts */
>  	for (i = 0; i < CHRG_INTR_END; i++) {
> @@ -905,11 +922,17 @@ static int axp288_charger_probe(struct platform_device *pdev)
>  	return 0;
>  
>  intr_reg_failed:
> -	if (info->otg.cable.edev)
> -		extcon_unregister_interest(&info->otg.cable);
> +	if (info->otg.cable)
> +		extcon_unregister_notifier(info->otg.cable, EXTCON_USB_HOST,
> +					&info->otg.id_nb);
>  	power_supply_unregister(info->psy_usb);
>  psy_reg_failed:
> -	extcon_unregister_notifier(info->cable.edev, EXTCON_NONE, &info->cable.nb);
> +	extcon_unregister_notifier(info->cable.edev, EXTCON_CHG_USB_SDP,
> +					&info->cable.nb);
> +	extcon_unregister_notifier(info->cable.edev, EXTCON_CHG_USB_SDP,
> +					&info->cable.nb);

EXTCON_CHG_USB_SDP -> EXTCON_CHG_USB_CDP?

> +	extcon_unregister_notifier(info->cable.edev, EXTCON_CHG_USB_DCP,
> +					&info->cable.nb);
>  	return ret;
>  }
>  
> @@ -917,10 +940,16 @@ static int axp288_charger_remove(struct platform_device *pdev)
>  {
>  	struct axp288_chrg_info *info =  dev_get_drvdata(&pdev->dev);
>  
> -	if (info->otg.cable.edev)
> -		extcon_unregister_interest(&info->otg.cable);
> +	if (info->otg.cable)
> +		extcon_unregister_notifier(info->otg.cable, EXTCON_USB_HOST,
> +					&info->otg.id_nb);
>  
> -	extcon_unregister_notifier(info->cable.edev, EXTCON_NONE, &info->cable.nb);
> +	extcon_unregister_notifier(info->cable.edev, EXTCON_CHG_USB_SDP,
> +					&info->cable.nb);
> +	extcon_unregister_notifier(info->cable.edev, EXTCON_CHG_USB_SDP,
> +					&info->cable.nb);

EXTCON_CHG_USB_SDP -> EXTCON_CHG_USB_CDP?

> +	extcon_unregister_notifier(info->cable.edev, EXTCON_CHG_USB_DCP,
> +					&info->cable.nb);
>  	power_supply_unregister(info->psy_usb);
>  
>  	return 0;

Has this dependencies to your tree or are all dependencies already
in torvalds tree?

-- Sebastian

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

Re: [RESEND PATCH 2/3] power: axp288_charger: Replace deprecatd API of extcon

[Chanwoo Choi]

Hi Sebastian,

On 2016년 04월 15일 23:20, Sebastian Reichel wrote:
> Hi,
> 
> On Fri, Apr 15, 2016 at 09:43:35AM +0900, Chanwoo Choi wrote:
>> This patch removes the deprecated notifier API of extcon framework and then use
>> the new extcon API[2] with the unique id[1] to indicate the each external
>> connector. Alter deprecated API as following:
>> - extcon_register_interest() -> extcon_register_notifier()
>> - extcon_unregister_interest() -> extcon_unregister_notifier()
>> - extcon_get_cable_state() -> extcon_get_cable_state_()
>>
>> And, extcon alters the name of USB charger connector in patch[3] as following:
>> - EXTCON_CHG_USB_SDP /* Standard Downstream Port */
>> - EXTCON_CHG_USB_DCP /* Dedicated Charging Port */
>> - EXTCON_CHG_USB_CDP /* Charging Downstream Port */
>> - EXTCON_CHG_USB_ACA /* Accessory Charger Adapter */
>>
>> [1] Commit 2a9de9c0f08d61
>> - ("extcon: Use the unique id for external connector instead of string)
>> [2] Commit 046050f6e623e4
>> - ("extcon: Update the prototype of extcon_register_notifier() with enum extcon
>> [3] Commit 11eecf910bd81d
>> - ("extcon: Modify the id and name of external connector")
>>
>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>> ---
>>  drivers/power/axp288_charger.c | 77 +++++++++++++++++++++++++++++-------------
>>  1 file changed, 53 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/power/axp288_charger.c b/drivers/power/axp288_charger.c
>> index e4d569f57acc..e5c2569befa6 100644
>> --- a/drivers/power/axp288_charger.c
>> +++ b/drivers/power/axp288_charger.c
>> @@ -129,10 +129,6 @@
>>  

[snip]

>>  	/* Register charger interrupts */
>>  	for (i = 0; i < CHRG_INTR_END; i++) {
>> @@ -905,11 +922,17 @@ static int axp288_charger_probe(struct platform_device *pdev)
>>  	return 0;
>>  
>>  intr_reg_failed:
>> -	if (info->otg.cable.edev)
>> -		extcon_unregister_interest(&info->otg.cable);
>> +	if (info->otg.cable)
>> +		extcon_unregister_notifier(info->otg.cable, EXTCON_USB_HOST,
>> +					&info->otg.id_nb);
>>  	power_supply_unregister(info->psy_usb);
>>  psy_reg_failed:
>> -	extcon_unregister_notifier(info->cable.edev, EXTCON_NONE, &info->cable.nb);
>> +	extcon_unregister_notifier(info->cable.edev, EXTCON_CHG_USB_SDP,
>> +					&info->cable.nb);
>> +	extcon_unregister_notifier(info->cable.edev, EXTCON_CHG_USB_SDP,
>> +					&info->cable.nb);
> 
> EXTCON_CHG_USB_SDP -> EXTCON_CHG_USB_CDP?

I was mistaken. I'll fix it.

> 
>> +	extcon_unregister_notifier(info->cable.edev, EXTCON_CHG_USB_DCP,
>> +					&info->cable.nb);
>>  	return ret;
>>  }
>>  
>> @@ -917,10 +940,16 @@ static int axp288_charger_remove(struct platform_device *pdev)
>>  {
>>  	struct axp288_chrg_info *info =  dev_get_drvdata(&pdev->dev);
>>  
>> -	if (info->otg.cable.edev)
>> -		extcon_unregister_interest(&info->otg.cable);
>> +	if (info->otg.cable)
>> +		extcon_unregister_notifier(info->otg.cable, EXTCON_USB_HOST,
>> +					&info->otg.id_nb);
>>  
>> -	extcon_unregister_notifier(info->cable.edev, EXTCON_NONE, &info->cable.nb);
>> +	extcon_unregister_notifier(info->cable.edev, EXTCON_CHG_USB_SDP,
>> +					&info->cable.nb);
>> +	extcon_unregister_notifier(info->cable.edev, EXTCON_CHG_USB_SDP,
>> +					&info->cable.nb);
> 
> EXTCON_CHG_USB_SDP -> EXTCON_CHG_USB_CDP?

ditto.

> 
>> +	extcon_unregister_notifier(info->cable.edev, EXTCON_CHG_USB_DCP,
>> +					&info->cable.nb);
>>  	power_supply_unregister(info->psy_usb);
>>  
>>  	return 0;
> 
> Has this dependencies to your tree or are all dependencies already
> in torvalds tree?

This patch-set have the dependency on Linus torvalds tree[1].
[1] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/

So, when picking this patch-set, someone make the immutable branch for this patchset
to prevent the merge conflict between power_supply and extcon tree.

Thanks,
Chanwoo Choi

[RESEND PATCH 3/3] extcon: Remove the deprecated extcon functions

[Chanwoo Choi]

This patch removes the deprecated extcon functions using string type
to identify the type of external connector. The Commit 2a9de9c0f08d61
("extcon: Use the unique id for external connector instead of string)
uses the unique id to separate the type of external connector instead of
string name.
- extcon_register_interest()
- extcon_unregister_interest()
- extcon_set_cable_state()
- extcon_get_cable_state()

And, extcon_register_interest() finds the first extcon device to include the
requested external connector from extcon client device and then register the
notifier if extcon device argument is NULL. Instead, extcon_register_notifier()
supports this feature.

Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 drivers/extcon/extcon.c | 201 ++++++------------------------------------------
 include/linux/extcon.h  |  59 --------------
 2 files changed, 24 insertions(+), 236 deletions(-)

diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
index 21a123cadf78..4fef9ab4b148 100644
--- a/drivers/extcon/extcon.c
+++ b/drivers/extcon/extcon.c
@@ -127,38 +127,6 @@ static int find_cable_index_by_id(struct extcon_dev *edev, const unsigned int id
 	return -EINVAL;
 }
 
-static int find_cable_id_by_name(struct extcon_dev *edev, const char *name)
-{
-	int id = -EINVAL;
-	int i = 0;
-
-	/* Find the id of extcon cable */
-	while (extcon_name[i]) {
-		if (!strncmp(extcon_name[i], name, CABLE_NAME_MAX)) {
-			id = i;
-			break;
-		}
-		i++;
-	}
-
-	return id;
-}
-
-static int find_cable_index_by_name(struct extcon_dev *edev, const char *name)
-{
-	int id;
-
-	if (edev->max_supported == 0)
-		return -EINVAL;
-
-	/* Find the the number of extcon cable */
-	id = find_cable_id_by_name(edev, name);
-	if (id < 0)
-		return id;
-
-	return find_cable_index_by_id(edev, id);
-}
-
 static bool is_extcon_changed(u32 prev, u32 new, int idx, bool *attached)
 {
 	if (((prev >> idx) & 0x1) != ((new >> idx) & 0x1)) {
@@ -374,25 +342,6 @@ int extcon_get_cable_state_(struct extcon_dev *edev, const unsigned int id)
 EXPORT_SYMBOL_GPL(extcon_get_cable_state_);
 
 /**
- * extcon_get_cable_state() - Get the status of a specific cable.
- * @edev:	the extcon device that has the cable.
- * @cable_name:	cable name.
- *
- * Note that this is slower than extcon_get_cable_state_.
- */
-int extcon_get_cable_state(struct extcon_dev *edev, const char *cable_name)
-{
-	int id;
-
-	id = find_cable_id_by_name(edev, cable_name);
-	if (id < 0)
-		return id;
-
-	return extcon_get_cable_state_(edev, id);
-}
-EXPORT_SYMBOL_GPL(extcon_get_cable_state);
-
-/**
  * extcon_set_cable_state_() - Set the status of a specific cable.
  * @edev:		the extcon device that has the cable.
  * @id:			the unique id of each external connector
@@ -422,28 +371,6 @@ int extcon_set_cable_state_(struct extcon_dev *edev, unsigned int id,
 EXPORT_SYMBOL_GPL(extcon_set_cable_state_);
 
 /**
- * extcon_set_cable_state() - Set the status of a specific cable.
- * @edev:		the extcon device that has the cable.
- * @cable_name:		cable name.
- * @cable_state:	the new cable status. The default semantics is
- *			true: attached / false: detached.
- *
- * Note that this is slower than extcon_set_cable_state_.
- */
-int extcon_set_cable_state(struct extcon_dev *edev,
-			const char *cable_name, bool cable_state)
-{
-	int id;
-
-	id = find_cable_id_by_name(edev, cable_name);
-	if (id < 0)
-		return id;
-
-	return extcon_set_cable_state_(edev, id, cable_state);
-}
-EXPORT_SYMBOL_GPL(extcon_set_cable_state);
-
-/**
  * extcon_get_extcon_dev() - Get the extcon device instance from the name
  * @extcon_name:	The extcon name provided with extcon_dev_register()
  */
@@ -467,105 +394,6 @@ out:
 EXPORT_SYMBOL_GPL(extcon_get_extcon_dev);
 
 /**
- * extcon_register_interest() - Register a notifier for a state change of a
- *				specific cable, not an entier set of cables of a
- *				extcon device.
- * @obj:		an empty extcon_specific_cable_nb object to be returned.
- * @extcon_name:	the name of extcon device.
- *			if NULL, extcon_register_interest will register
- *			every cable with the target cable_name given.
- * @cable_name:		the target cable name.
- * @nb:			the notifier block to get notified.
- *
- * Provide an empty extcon_specific_cable_nb. extcon_register_interest() sets
- * the struct for you.
- *
- * extcon_register_interest is a helper function for those who want to get
- * notification for a single specific cable's status change. If a user wants
- * to get notification for any changes of all cables of a extcon device,
- * he/she should use the general extcon_register_notifier().
- *
- * Note that the second parameter given to the callback of nb (val) is
- * "old_state", not the current state. The current state can be retrieved
- * by looking at the third pameter (edev pointer)'s state value.
- */
-int extcon_register_interest(struct extcon_specific_cable_nb *obj,
-			     const char *extcon_name, const char *cable_name,
-			     struct notifier_block *nb)
-{
-	unsigned long flags;
-	int ret;
-
-	if (!obj || !cable_name || !nb)
-		return -EINVAL;
-
-	if (extcon_name) {
-		obj->edev = extcon_get_extcon_dev(extcon_name);
-		if (!obj->edev)
-			return -ENODEV;
-
-		obj->cable_index = find_cable_index_by_name(obj->edev,
-							cable_name);
-		if (obj->cable_index < 0)
-			return obj->cable_index;
-
-		obj->user_nb = nb;
-
-		spin_lock_irqsave(&obj->edev->lock, flags);
-		ret = raw_notifier_chain_register(
-					&obj->edev->nh[obj->cable_index],
-					obj->user_nb);
-		spin_unlock_irqrestore(&obj->edev->lock, flags);
-	} else {
-		struct class_dev_iter iter;
-		struct extcon_dev *extd;
-		struct device *dev;
-
-		if (!extcon_class)
-			return -ENODEV;
-		class_dev_iter_init(&iter, extcon_class, NULL, NULL);
-		while ((dev = class_dev_iter_next(&iter))) {
-			extd = dev_get_drvdata(dev);
-
-			if (find_cable_index_by_name(extd, cable_name) < 0)
-				continue;
-
-			class_dev_iter_exit(&iter);
-			return extcon_register_interest(obj, extd->name,
-						cable_name, nb);
-		}
-
-		ret = -ENODEV;
-	}
-
-	return ret;
-}
-EXPORT_SYMBOL_GPL(extcon_register_interest);
-
-/**
- * extcon_unregister_interest() - Unregister the notifier registered by
- *				  extcon_register_interest().
- * @obj:	the extcon_specific_cable_nb object returned by
- *		extcon_register_interest().
- */
-int extcon_unregister_interest(struct extcon_specific_cable_nb *obj)
-{
-	unsigned long flags;
-	int ret;
-
-	if (!obj)
-		return -EINVAL;
-
-	spin_lock_irqsave(&obj->edev->lock, flags);
-	ret = raw_notifier_chain_unregister(
-			&obj->edev->nh[obj->cable_index], obj->user_nb);
-	spin_unlock_irqrestore(&obj->edev->lock, flags);
-
-	return ret;
-}
-EXPORT_SYMBOL_GPL(extcon_unregister_interest);
-
-/**
  * extcon_register_notifier() - Register a notifiee to get notified by
  *				any attach status changes from the extcon.
  * @edev:	the extcon device that has the external connecotr.
@@ -582,14 +410,33 @@ int extcon_register_notifier(struct extcon_dev *edev, unsigned int id,
 	unsigned long flags;
 	int ret, idx;
 
-	if (!edev || !nb)
+	if (!nb)
 		return -EINVAL;
 
-	idx = find_cable_index_by_id(edev, id);
+	if (edev) {
+		idx = find_cable_index_by_id(edev, id);
 
-	spin_lock_irqsave(&edev->lock, flags);
-	ret = raw_notifier_chain_register(&edev->nh[idx], nb);
-	spin_unlock_irqrestore(&edev->lock, flags);
+		spin_lock_irqsave(&edev->lock, flags);
+		ret = raw_notifier_chain_register(&edev->nh[idx], nb);
+		spin_unlock_irqrestore(&edev->lock, flags);
+	} else {
+		struct extcon_dev *extd;
+
+		mutex_lock(&extcon_dev_list_lock);
+		list_for_each_entry(extd, &extcon_dev_list, entry) {
+			idx = find_cable_index_by_id(extd, id);
+			if (idx >= 0)
+				break;
+		}
+		mutex_unlock(&extcon_dev_list_lock);
+
+		if (idx >= 0) {
+			edev = extd;
+			return extcon_register_notifier(extd, id, nb);
+		} else {
+			ret = -ENODEV;
+		}
+	}
 
 	return ret;
 }
diff --git a/include/linux/extcon.h b/include/linux/extcon.h
index 7abf674c388c..dc7fc5dc9d6b 100644
--- a/include/linux/extcon.h
+++ b/include/linux/extcon.h
@@ -146,22 +146,6 @@ struct extcon_cable {
 	struct attribute *attrs[3]; /* to be fed to attr_g.attrs */
 };
 
-/**
- * struct extcon_specific_cable_nb - An internal data for
- *				     extcon_register_interest().
- * @user_nb:		user provided notifier block for events from
- *			a specific cable.
- * @cable_index:	the target cable.
- * @edev:		the target extcon device.
- * @previous_value:	the saved previous event value.
- */
-struct extcon_specific_cable_nb {
-	struct notifier_block *user_nb;
-	int cable_index;
-	struct extcon_dev *edev;
-	unsigned long previous_value;
-};
-
 #if IS_ENABLED(CONFIG_EXTCON)
 
 /*
@@ -207,23 +191,6 @@ extern int extcon_get_cable_state_(struct extcon_dev *edev, unsigned int id);
 extern int extcon_set_cable_state_(struct extcon_dev *edev, unsigned int id,
 				   bool cable_state);
 
-extern int extcon_get_cable_state(struct extcon_dev *edev,
-				  const char *cable_name);
-extern int extcon_set_cable_state(struct extcon_dev *edev,
-				  const char *cable_name, bool cable_state);
-
-/*
- * Following APIs are for notifiees (those who want to be notified)
- * to register a callback for events from a specific cable of the extcon.
- * Notifiees are the connected device drivers wanting to get notified by
- * a specific external port of a connection device.
- */
-extern int extcon_register_interest(struct extcon_specific_cable_nb *obj,
-				    const char *extcon_name,
-				    const char *cable_name,
-				    struct notifier_block *nb);
-extern int extcon_unregister_interest(struct extcon_specific_cable_nb *nb);
-
 /*
  * Following APIs are to monitor every action of a notifier.
  * Registrar gets notified for every external port of a connection device.
@@ -306,18 +273,6 @@ static inline int extcon_set_cable_state_(struct extcon_dev *edev,
 	return 0;
 }
 
-static inline int extcon_get_cable_state(struct extcon_dev *edev,
-			const char *cable_name)
-{
-	return 0;
-}
-
-static inline int extcon_set_cable_state(struct extcon_dev *edev,
-			const char *cable_name, int state)
-{
-	return 0;
-}
-
 static inline struct extcon_dev *extcon_get_extcon_dev(const char *extcon_name)
 {
 	return NULL;
@@ -337,20 +292,6 @@ static inline int extcon_unregister_notifier(struct extcon_dev *edev,
 	return 0;
 }
 
-static inline int extcon_register_interest(struct extcon_specific_cable_nb *obj,
-					   const char *extcon_name,
-					   const char *cable_name,
-					   struct notifier_block *nb)
-{
-	return 0;
-}
-
-static inline int extcon_unregister_interest(struct extcon_specific_cable_nb
-						    *obj)
-{
-	return 0;
-}
-
 static inline struct extcon_dev *extcon_get_edev_by_phandle(struct device *dev,
 							    int index)
 {
-- 
1.9.1