[PATCH 0/5] power: supply: tps65217: Support USB charger feature

[PATCH 0/5] power: supply: tps65217: Support USB charger feature

[Milo Kim]

TPS65217 device supports two charger inputs - AC and USB.
Currently, only AC charger is supported. This patch-set adds USB charger 
feature. Tested on Beaglebone black.

Milo Kim (5):
  power: supply: tps65217: Move IRQ related operation into single
    function
  power: supply: tps65217: Remove IRQ data from driver data
  power: supply: tps65217: Support USB charger interrupt
  power: supply: tps65217: Use generic name for charger online
  power: supply: tps65217: Use generic name for power supply structures

 drivers/power/supply/tps65217_charger.c | 133 ++++++++++++++++++--------------
 1 file changed, 73 insertions(+), 60 deletions(-)

-- 
2.9.3

[PATCH 1/5] power: supply: tps65217: Move IRQ related operation into single function

[Milo Kim]

TPS65217 charger driver handles the charger interrupt through the IRQ or
polling. Both cases can be requested in single function.

Cc: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Signed-off-by: Milo Kim <woogyom.kim@gmail.com>
---
 drivers/power/supply/tps65217_charger.c | 70 ++++++++++++++++++---------------
 1 file changed, 38 insertions(+), 32 deletions(-)

diff --git a/drivers/power/supply/tps65217_charger.c b/drivers/power/supply/tps65217_charger.c
index 9fd019f..04f8322 100644
--- a/drivers/power/supply/tps65217_charger.c
+++ b/drivers/power/supply/tps65217_charger.c
@@ -195,12 +195,48 @@ static const struct power_supply_desc tps65217_charger_desc = {
 	.num_properties		= ARRAY_SIZE(tps65217_ac_props),
 };
 
+static int tps65217_charger_request_interrupt(struct platform_device *pdev)
+{
+	struct tps65217_charger *charger = platform_get_drvdata(pdev);
+	int irq;
+	int ret;
+
+	irq = platform_get_irq_byname(pdev, "AC");
+	if (irq < 0)
+		irq = -ENXIO;
+
+	charger->irq = irq;
+
+	if (irq != -ENXIO) {
+		ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
+						tps65217_charger_irq, 0,
+						"tps65217-charger", charger);
+		if (ret) {
+			dev_err(charger->dev,
+				"Unable to register irq %d err %d\n", irq, ret);
+			return ret;
+		}
+
+		/* Check current state */
+		tps65217_charger_irq(irq, charger);
+		return 0;
+	}
+
+	charger->poll_task = kthread_run(tps65217_charger_poll_task, charger,
+					 "ktps65217charger");
+	if (IS_ERR(charger->poll_task)) {
+		ret = PTR_ERR(charger->poll_task);
+		dev_err(charger->dev, "Unable to run kthread err %d\n", ret);
+	}
+
+	return 0;
+}
+
 static int tps65217_charger_probe(struct platform_device *pdev)
 {
 	struct tps65217 *tps = dev_get_drvdata(pdev->dev.parent);
 	struct tps65217_charger *charger;
 	struct power_supply_config cfg = {};
-	int irq;
 	int ret;
 
 	dev_dbg(&pdev->dev, "%s\n", __func__);
@@ -224,43 +260,13 @@ static int tps65217_charger_probe(struct platform_device *pdev)
 		return PTR_ERR(charger->ac);
 	}
 
-	irq = platform_get_irq_byname(pdev, "AC");
-	if (irq < 0)
-		irq = -ENXIO;
-	charger->irq = irq;
-
 	ret = tps65217_config_charger(charger);
 	if (ret < 0) {
 		dev_err(charger->dev, "charger config failed, err %d\n", ret);
 		return ret;
 	}
 
-	if (irq != -ENXIO) {
-		ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
-						tps65217_charger_irq,
-						0, "tps65217-charger",
-						charger);
-		if (ret) {
-			dev_err(charger->dev,
-				"Unable to register irq %d err %d\n", irq,
-				ret);
-			return ret;
-		}
-
-		/* Check current state */
-		tps65217_charger_irq(irq, charger);
-	} else {
-		charger->poll_task = kthread_run(tps65217_charger_poll_task,
-						charger, "ktps65217charger");
-		if (IS_ERR(charger->poll_task)) {
-			ret = PTR_ERR(charger->poll_task);
-			dev_err(charger->dev,
-				"Unable to run kthread err %d\n", ret);
-			return ret;
-		}
-	}
-
-	return 0;
+	return tps65217_charger_request_interrupt(pdev);
 }
 
 static int tps65217_charger_remove(struct platform_device *pdev)
-- 
2.9.3

[PATCH 2/5] power: supply: tps65217: Remove IRQ data from driver data

[Milo Kim]

IRQ number is only used on requesting the interrupt, so no need to keep
it inside the driver data.
In case of polling, poll_task is valid only when polling thread is
activated.

Cc: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Signed-off-by: Milo Kim <woogyom.kim@gmail.com>
---
 drivers/power/supply/tps65217_charger.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/power/supply/tps65217_charger.c b/drivers/power/supply/tps65217_charger.c
index 04f8322..55a4f34 100644
--- a/drivers/power/supply/tps65217_charger.c
+++ b/drivers/power/supply/tps65217_charger.c
@@ -46,8 +46,6 @@ struct tps65217_charger {
 	int	prev_ac_online;
 
 	struct task_struct	*poll_task;
-
-	int	irq;
 };
 
 static enum power_supply_property tps65217_ac_props[] = {
@@ -198,6 +196,7 @@ static const struct power_supply_desc tps65217_charger_desc = {
 static int tps65217_charger_request_interrupt(struct platform_device *pdev)
 {
 	struct tps65217_charger *charger = platform_get_drvdata(pdev);
+	struct task_struct *poll_task;
 	int irq;
 	int ret;
 
@@ -205,8 +204,6 @@ static int tps65217_charger_request_interrupt(struct platform_device *pdev)
 	if (irq < 0)
 		irq = -ENXIO;
 
-	charger->irq = irq;
-
 	if (irq != -ENXIO) {
 		ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
 						tps65217_charger_irq, 0,
@@ -222,13 +219,16 @@ static int tps65217_charger_request_interrupt(struct platform_device *pdev)
 		return 0;
 	}
 
-	charger->poll_task = kthread_run(tps65217_charger_poll_task, charger,
-					 "ktps65217charger");
-	if (IS_ERR(charger->poll_task)) {
-		ret = PTR_ERR(charger->poll_task);
+	poll_task = kthread_run(tps65217_charger_poll_task, charger,
+				"ktps65217charger");
+	if (IS_ERR(poll_task)) {
+		ret = PTR_ERR(poll_task);
 		dev_err(charger->dev, "Unable to run kthread err %d\n", ret);
+		return ret;
 	}
 
+	charger->poll_task = poll_task;
+
 	return 0;
 }
 
@@ -273,7 +273,7 @@ static int tps65217_charger_remove(struct platform_device *pdev)
 {
 	struct tps65217_charger *charger = platform_get_drvdata(pdev);
 
-	if (charger->irq == -ENXIO)
+	if (charger->poll_task)
 		kthread_stop(charger->poll_task);
 
 	return 0;
-- 
2.9.3

[PATCH 3/5] power: supply: tps65217: Support USB charger interrupt

[Milo Kim]

TPS65217 has two charger interrupts - AC or USB power status change.

Interrupt request:
  If an interrupt number is not defined, then use legacy polling thread.
  Otherwise, create IRQ threads to handle AC or USB charger event.

Interrupt handler:
  Check not only AC but also USB charger status.
  In both cases, enable charging operation.

Cc: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Signed-off-by: Milo Kim <woogyom.kim@gmail.com>
---
 drivers/power/supply/tps65217_charger.c | 52 +++++++++++++++++++--------------
 1 file changed, 30 insertions(+), 22 deletions(-)

diff --git a/drivers/power/supply/tps65217_charger.c b/drivers/power/supply/tps65217_charger.c
index 55a4f34..2894132 100644
--- a/drivers/power/supply/tps65217_charger.c
+++ b/drivers/power/supply/tps65217_charger.c
@@ -35,7 +35,9 @@
 #include <linux/mfd/core.h>
 #include <linux/mfd/tps65217.h>
 
+#define CHARGER_STATUS_PRESENT	(TPS65217_STATUS_ACPWR | TPS65217_STATUS_USBPWR)
 #define POLL_INTERVAL		(HZ * 2)
+#define NUM_CHARGER_IRQS	2
 
 struct tps65217_charger {
 	struct tps65217 *tps;
@@ -142,8 +144,8 @@ static irqreturn_t tps65217_charger_irq(int irq, void *dev)
 
 	dev_dbg(charger->dev, "%s: 0x%x\n", __func__, val);
 
-	/* check for AC status bit */
-	if (val & TPS65217_STATUS_ACPWR) {
+	/* check for charger status bit */
+	if (val & CHARGER_STATUS_PRESENT) {
 		ret = tps65217_enable_charging(charger);
 		if (ret) {
 			dev_err(charger->dev,
@@ -197,37 +199,43 @@ static int tps65217_charger_request_interrupt(struct platform_device *pdev)
 {
 	struct tps65217_charger *charger = platform_get_drvdata(pdev);
 	struct task_struct *poll_task;
-	int irq;
+	int irq[NUM_CHARGER_IRQS];
 	int ret;
+	int i;
+
+	irq[0] = platform_get_irq_byname(pdev, "AC");
+	irq[1] = platform_get_irq_byname(pdev, "USB");
+
+	/* Create a polling thread if an interrupt is invalid */
+	if (irq[0] < 0 || irq[1] < 0) {
+		poll_task = kthread_run(tps65217_charger_poll_task, charger,
+					"ktps65217charger");
+		if (IS_ERR(poll_task)) {
+			ret = PTR_ERR(poll_task);
+			dev_err(charger->dev, "Unable to run kthread err %d\n", ret);
+			return ret;
+		}
+
+		charger->poll_task = poll_task;
 
-	irq = platform_get_irq_byname(pdev, "AC");
-	if (irq < 0)
-		irq = -ENXIO;
+		return 0;
+	}
 
-	if (irq != -ENXIO) {
-		ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
+	/* Create IRQ threads for charger interrupts */
+	for (i = 0; i < NUM_CHARGER_IRQS; i++) {
+		ret = devm_request_threaded_irq(&pdev->dev, irq[i], NULL,
 						tps65217_charger_irq, 0,
 						"tps65217-charger", charger);
 		if (ret) {
 			dev_err(charger->dev,
-				"Unable to register irq %d err %d\n", irq, ret);
+				"Unable to register irq %d err %d\n", irq[i],
+				ret);
 			return ret;
 		}
-
-		/* Check current state */
-		tps65217_charger_irq(irq, charger);
-		return 0;
-	}
-
-	poll_task = kthread_run(tps65217_charger_poll_task, charger,
-				"ktps65217charger");
-	if (IS_ERR(poll_task)) {
-		ret = PTR_ERR(poll_task);
-		dev_err(charger->dev, "Unable to run kthread err %d\n", ret);
-		return ret;
 	}
 
-	charger->poll_task = poll_task;
+	/* Check current state */
+	tps65217_charger_irq(-1, charger);
 
 	return 0;
 }
-- 
2.9.3

[PATCH 4/5] power: supply: tps65217: Use generic name for charger online

[Milo Kim]

This driver supports AC and USB chargers. Generic name is preferred.

  ac_online -> online
  prev_ac_online -> prev_online

Cc: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Signed-off-by: Milo Kim <woogyom.kim@gmail.com>
---
 drivers/power/supply/tps65217_charger.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/power/supply/tps65217_charger.c b/drivers/power/supply/tps65217_charger.c
index 2894132..8b7ce61 100644
--- a/drivers/power/supply/tps65217_charger.c
+++ b/drivers/power/supply/tps65217_charger.c
@@ -44,8 +44,8 @@ struct tps65217_charger {
 	struct device *dev;
 	struct power_supply *ac;
 
-	int	ac_online;
-	int	prev_ac_online;
+	int	online;
+	int	prev_online;
 
 	struct task_struct	*poll_task;
 };
@@ -95,7 +95,7 @@ static int tps65217_enable_charging(struct tps65217_charger *charger)
 	int ret;
 
 	/* charger already enabled */
-	if (charger->ac_online)
+	if (charger->online)
 		return 0;
 
 	dev_dbg(charger->dev, "%s: enable charging\n", __func__);
@@ -110,7 +110,7 @@ static int tps65217_enable_charging(struct tps65217_charger *charger)
 		return ret;
 	}
 
-	charger->ac_online = 1;
+	charger->online = 1;
 
 	return 0;
 }
@@ -122,7 +122,7 @@ static int tps65217_ac_get_property(struct power_supply *psy,
 	struct tps65217_charger *charger = power_supply_get_drvdata(psy);
 
 	if (psp == POWER_SUPPLY_PROP_ONLINE) {
-		val->intval = charger->ac_online;
+		val->intval = charger->online;
 		return 0;
 	}
 	return -EINVAL;
@@ -133,7 +133,7 @@ static irqreturn_t tps65217_charger_irq(int irq, void *dev)
 	int ret, val;
 	struct tps65217_charger *charger = dev;
 
-	charger->prev_ac_online = charger->ac_online;
+	charger->prev_online = charger->online;
 
 	ret = tps65217_reg_read(charger->tps, TPS65217_REG_STATUS, &val);
 	if (ret < 0) {
@@ -153,10 +153,10 @@ static irqreturn_t tps65217_charger_irq(int irq, void *dev)
 			return IRQ_HANDLED;
 		}
 	} else {
-		charger->ac_online = 0;
+		charger->online = 0;
 	}
 
-	if (charger->prev_ac_online != charger->ac_online)
+	if (charger->prev_online != charger->online)
 		power_supply_changed(charger->ac);
 
 	ret = tps65217_reg_read(charger->tps, TPS65217_REG_CHGCONFIG0, &val);
-- 
2.9.3

[PATCH 5/5] power: supply: tps65217: Use generic name for power supply structures

[Milo Kim]

AC and USB charger are supported, so generic names are preferred.

  ac in tps65217_charger -> psy
  tps65217_ac_props -> tps65217_charger_props
  tps65217_ac_get_property -> tps65217_charger_get_property
  tps65217-ac -> tps65217-charger

Cc: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Signed-off-by: Milo Kim <woogyom.kim@gmail.com>
---
 drivers/power/supply/tps65217_charger.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/power/supply/tps65217_charger.c b/drivers/power/supply/tps65217_charger.c
index 8b7ce61..4e3f5cb 100644
--- a/drivers/power/supply/tps65217_charger.c
+++ b/drivers/power/supply/tps65217_charger.c
@@ -42,7 +42,7 @@
 struct tps65217_charger {
 	struct tps65217 *tps;
 	struct device *dev;
-	struct power_supply *ac;
+	struct power_supply *psy;
 
 	int	online;
 	int	prev_online;
@@ -50,7 +50,7 @@ struct tps65217_charger {
 	struct task_struct	*poll_task;
 };
 
-static enum power_supply_property tps65217_ac_props[] = {
+static enum power_supply_property tps65217_charger_props[] = {
 	POWER_SUPPLY_PROP_ONLINE,
 };
 
@@ -115,9 +115,9 @@ static int tps65217_enable_charging(struct tps65217_charger *charger)
 	return 0;
 }
 
-static int tps65217_ac_get_property(struct power_supply *psy,
-			enum power_supply_property psp,
-			union power_supply_propval *val)
+static int tps65217_charger_get_property(struct power_supply *psy,
+					 enum power_supply_property psp,
+					 union power_supply_propval *val)
 {
 	struct tps65217_charger *charger = power_supply_get_drvdata(psy);
 
@@ -157,7 +157,7 @@ static irqreturn_t tps65217_charger_irq(int irq, void *dev)
 	}
 
 	if (charger->prev_online != charger->online)
-		power_supply_changed(charger->ac);
+		power_supply_changed(charger->psy);
 
 	ret = tps65217_reg_read(charger->tps, TPS65217_REG_CHGCONFIG0, &val);
 	if (ret < 0) {
@@ -188,11 +188,11 @@ static int tps65217_charger_poll_task(void *data)
 }
 
 static const struct power_supply_desc tps65217_charger_desc = {
-	.name			= "tps65217-ac",
+	.name			= "tps65217-charger",
 	.type			= POWER_SUPPLY_TYPE_MAINS,
-	.get_property		= tps65217_ac_get_property,
-	.properties		= tps65217_ac_props,
-	.num_properties		= ARRAY_SIZE(tps65217_ac_props),
+	.get_property		= tps65217_charger_get_property,
+	.properties		= tps65217_charger_props,
+	.num_properties		= ARRAY_SIZE(tps65217_charger_props),
 };
 
 static int tps65217_charger_request_interrupt(struct platform_device *pdev)
@@ -260,12 +260,11 @@ static int tps65217_charger_probe(struct platform_device *pdev)
 	cfg.of_node = pdev->dev.of_node;
 	cfg.drv_data = charger;
 
-	charger->ac = devm_power_supply_register(&pdev->dev,
-						 &tps65217_charger_desc,
-						 &cfg);
-	if (IS_ERR(charger->ac)) {
+	charger->psy = devm_power_supply_register(&pdev->dev,
+						  &tps65217_charger_desc, &cfg);
+	if (IS_ERR(charger->psy)) {
 		dev_err(&pdev->dev, "failed: power supply register\n");
-		return PTR_ERR(charger->ac);
+		return PTR_ERR(charger->psy);
 	}
 
 	ret = tps65217_config_charger(charger);
-- 
2.9.3

Re: [PATCH 1/5] power: supply: tps65217: Move IRQ related operation into single function

[Sebastian Reichel]

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

Hi,

On Tue, Nov 15, 2016 at 10:18:51PM +0900, Milo Kim wrote:
> TPS65217 charger driver handles the charger interrupt through the IRQ or
> polling. Both cases can be requested in single function.
> 
> Cc: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Signed-off-by: Milo Kim <woogyom.kim@gmail.com>
> ---
>  drivers/power/supply/tps65217_charger.c | 70 ++++++++++++++++++---------------
>  1 file changed, 38 insertions(+), 32 deletions(-)

I don't see the advantage of this. It's more lines of codes than
before and does not really increase readability.

-- Sebastian

> 
> diff --git a/drivers/power/supply/tps65217_charger.c b/drivers/power/supply/tps65217_charger.c
> index 9fd019f..04f8322 100644
> --- a/drivers/power/supply/tps65217_charger.c
> +++ b/drivers/power/supply/tps65217_charger.c
> @@ -195,12 +195,48 @@ static const struct power_supply_desc tps65217_charger_desc = {
>  	.num_properties		= ARRAY_SIZE(tps65217_ac_props),
>  };
>  
> +static int tps65217_charger_request_interrupt(struct platform_device *pdev)
> +{
> +	struct tps65217_charger *charger = platform_get_drvdata(pdev);
> +	int irq;
> +	int ret;
> +
> +	irq = platform_get_irq_byname(pdev, "AC");
> +	if (irq < 0)
> +		irq = -ENXIO;
> +
> +	charger->irq = irq;
> +
> +	if (irq != -ENXIO) {
> +		ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
> +						tps65217_charger_irq, 0,
> +						"tps65217-charger", charger);
> +		if (ret) {
> +			dev_err(charger->dev,
> +				"Unable to register irq %d err %d\n", irq, ret);
> +			return ret;
> +		}
> +
> +		/* Check current state */
> +		tps65217_charger_irq(irq, charger);
> +		return 0;
> +	}
> +
> +	charger->poll_task = kthread_run(tps65217_charger_poll_task, charger,
> +					 "ktps65217charger");
> +	if (IS_ERR(charger->poll_task)) {
> +		ret = PTR_ERR(charger->poll_task);
> +		dev_err(charger->dev, "Unable to run kthread err %d\n", ret);
> +	}
> +
> +	return 0;
> +}
> +
>  static int tps65217_charger_probe(struct platform_device *pdev)
>  {
>  	struct tps65217 *tps = dev_get_drvdata(pdev->dev.parent);
>  	struct tps65217_charger *charger;
>  	struct power_supply_config cfg = {};
> -	int irq;
>  	int ret;
>  
>  	dev_dbg(&pdev->dev, "%s\n", __func__);
> @@ -224,43 +260,13 @@ static int tps65217_charger_probe(struct platform_device *pdev)
>  		return PTR_ERR(charger->ac);
>  	}
>  
> -	irq = platform_get_irq_byname(pdev, "AC");
> -	if (irq < 0)
> -		irq = -ENXIO;
> -	charger->irq = irq;
> -
>  	ret = tps65217_config_charger(charger);
>  	if (ret < 0) {
>  		dev_err(charger->dev, "charger config failed, err %d\n", ret);
>  		return ret;
>  	}
>  
> -	if (irq != -ENXIO) {
> -		ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
> -						tps65217_charger_irq,
> -						0, "tps65217-charger",
> -						charger);
> -		if (ret) {
> -			dev_err(charger->dev,
> -				"Unable to register irq %d err %d\n", irq,
> -				ret);
> -			return ret;
> -		}
> -
> -		/* Check current state */
> -		tps65217_charger_irq(irq, charger);
> -	} else {
> -		charger->poll_task = kthread_run(tps65217_charger_poll_task,
> -						charger, "ktps65217charger");
> -		if (IS_ERR(charger->poll_task)) {
> -			ret = PTR_ERR(charger->poll_task);
> -			dev_err(charger->dev,
> -				"Unable to run kthread err %d\n", ret);
> -			return ret;
> -		}
> -	}
> -
> -	return 0;
> +	return tps65217_charger_request_interrupt(pdev);
>  }
>  
>  static int tps65217_charger_remove(struct platform_device *pdev)
> -- 
> 2.9.3
> 

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

Re: [PATCH 2/5] power: supply: tps65217: Remove IRQ data from driver data

[Sebastian Reichel]

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

Hi,

On Tue, Nov 15, 2016 at 10:18:52PM +0900, Milo Kim wrote:
> IRQ number is only used on requesting the interrupt, so no need to keep
> it inside the driver data.
> In case of polling, poll_task is valid only when polling thread is
> activated.
> 
> Cc: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Signed-off-by: Milo Kim <woogyom.kim@gmail.com>
> ---
>  drivers/power/supply/tps65217_charger.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/power/supply/tps65217_charger.c b/drivers/power/supply/tps65217_charger.c
> index 04f8322..55a4f34 100644
> --- a/drivers/power/supply/tps65217_charger.c
> +++ b/drivers/power/supply/tps65217_charger.c
> @@ -46,8 +46,6 @@ struct tps65217_charger {
>  	int	prev_ac_online;
>  
>  	struct task_struct	*poll_task;
> -
> -	int	irq;

Removal of this (and related changes) look ok. The poll_task changes
are actually only needed due to Patch 1 (which should be dropped imho).

-- Sebastian

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

Re: [PATCH 1/5] power: supply: tps65217: Move IRQ related operation into single function

[Sebastian Reichel]

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

Hi,

On Tue, Nov 22, 2016 at 05:49:11PM +0100, Sebastian Reichel wrote:
> On Tue, Nov 15, 2016 at 10:18:51PM +0900, Milo Kim wrote:
> > TPS65217 charger driver handles the charger interrupt through the IRQ or
> > polling. Both cases can be requested in single function.
> > 
> > Cc: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> > Signed-off-by: Milo Kim <woogyom.kim@gmail.com>
> > ---
> >  drivers/power/supply/tps65217_charger.c | 70 ++++++++++++++++++---------------
> >  1 file changed, 38 insertions(+), 32 deletions(-)
> 
> I don't see the advantage of this. It's more lines of codes than
> before and does not really increase readability.

Ok it makes sense with changes from PATCH 3. Please add a note to
the commit description, that this prep-work for introducing more
irqs.

-- Sebastian

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

Re: [PATCH 2/5] power: supply: tps65217: Remove IRQ data from driver data

[Sebastian Reichel]

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

Hi,

[it's me again after reading Patch 3]

On Tue, Nov 22, 2016 at 05:51:07PM +0100, Sebastian Reichel wrote:
> On Tue, Nov 15, 2016 at 10:18:52PM +0900, Milo Kim wrote:
> > IRQ number is only used on requesting the interrupt, so no need to keep
> > it inside the driver data.

Please move simple cleanup patches like this one to the
begin of the patch series.

> > In case of polling, poll_task is valid only when polling thread is
> > activated.

And merge this change into the patch were you introduced the
problem...

-- Sebastian

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

Re: [PATCH 2/5] power: supply: tps65217: Remove IRQ data from driver data

[Milo Kim]

On 11/23/2016 01:58 AM, Sebastian Reichel wrote:
> On Tue, Nov 22, 2016 at 05:51:07PM +0100, Sebastian Reichel wrote:
>> > On Tue, Nov 15, 2016 at 10:18:52PM +0900, Milo Kim wrote:
>>> > > IRQ number is only used on requesting the interrupt, so no need to keep
>>> > > it inside the driver data.
> Please move simple cleanup patches like this one to the
> begin of the patch series.
>
>>> > > In case of polling, poll_task is valid only when polling thread is
>>> > > activated.
> And merge this change into the patch were you introduced the
> problem...

Thanks for your feedback. Let me regenerate the patch-set for better review.

Best regards,
Milo