Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1545805 > unrolled thread

[PATCH v2 00/12] power: supply: axp288_charger: Various fixes

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 00/12] power: supply: axp288_charger: Various fixes Hans de Goede <hdegoede@redhat.com> - 2016-12-21 15:50 +0100
    [PATCH v2 07/12] power: supply: axp288_charger: Handle charger type changing without disconnect Hans de Goede <hdegoede@redhat.com> - 2016-12-21 15:50 +0100
    [PATCH v2 03/12] power: supply: axp288_charger: Use devm_power_supply_register Hans de Goede <hdegoede@redhat.com> - 2016-12-21 15:50 +0100
    [PATCH v2 05/12] power: supply: axp288_charger: Move init_hw_regs call before supply registration Hans de Goede <hdegoede@redhat.com> - 2016-12-21 15:50 +0100
    [PATCH v2 01/12] power: supply: axp288_charger: Make charger_init_hw_regs propagate i2c errors Hans de Goede <hdegoede@redhat.com> - 2016-12-21 15:50 +0100
    [PATCH v2 02/12] power: supply: axp288_charger: Drop platform_data dependency Hans de Goede <hdegoede@redhat.com> - 2016-12-21 15:50 +0100
    [PATCH v2 06/12] power: supply: axp288_charger: Actually get and use the USB_HOST extcon device Hans de Goede <hdegoede@redhat.com> - 2016-12-21 15:50 +0100
    Re: [PATCH v2 00/12] power: supply: axp288_charger: Various fixes Sebastian Reichel <sre@kernel.org> - 2016-12-23 02:40 +0100

#1545805 — [PATCH v2 00/12] power: supply: axp288_charger: Various fixes

Hi All,

Here is v2 of my axp288_charger series, which fixup the axp288_charger
code to actually work.

This implements the suggestion by Chanwoo Choi on how we can use the
existing extcon_get_extcon_dev instead of introducing a new
extcon_get_extcon_dev_by_id. So this series no longer depends on any
extcon sub-sys changes.

I've also rebased it on top of linux-power-supply/for-next-next to avoid
some conflicts with patches already there which Chanwoo pointed out,
so assuming review goes well merging these to for-next-next should go
smoothly :)

Regards,

Hans

[toc] | [next] | [standalone]

#1545806 — [PATCH v2 07/12] power: supply: axp288_charger: Handle charger type changing without disconnect

Deal with the charger type changing without a vbus-disconnect being
reported in between the 2 charger type states:

-Do not return from axp288_charger_extcon_evt_worker early in this case
 (track old_chg_type)
-Make calling axp288_charger_enable_charger with the same value as before
 a nop, to avoid the need for the caller to check this
-Do no do a dev_err when axp288_charger_enable_charger returns an error,
 axp288_charger_enable_charger already returns an error itself
-Disable the charger before changing the charge-current setting (nop if
 vbus was seen as disconnected before the change)

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/power/supply/axp288_charger.c | 31 +++++++++++++------------------
 1 file changed, 13 insertions(+), 18 deletions(-)

diff --git a/drivers/power/supply/axp288_charger.c b/drivers/power/supply/axp288_charger.c
index 4443f11..03395d2 100644
--- a/drivers/power/supply/axp288_charger.c
+++ b/drivers/power/supply/axp288_charger.c
@@ -175,7 +175,6 @@ struct axp288_chrg_info {
 	int max_cv;
 	bool online;
 	bool present;
-	bool enable_charger;
 	bool is_charger_enabled;
 };
 
@@ -305,6 +304,9 @@ static int axp288_charger_enable_charger(struct axp288_chrg_info *info,
 {
 	int ret;
 
+	if (enable == info->is_charger_enabled)
+		return 0;
+
 	if (enable)
 		ret = regmap_update_bits(info->regmap, AXP20X_CHRG_CTRL1,
 				CHRG_CCCV_CHG_EN, CHRG_CCCV_CHG_EN);
@@ -579,6 +581,7 @@ static void axp288_charger_extcon_evt_worker(struct work_struct *work)
 	bool changed = false;
 	struct extcon_dev *edev = info->cable.edev;
 	bool old_connected = info->cable.connected;
+	enum power_supply_type old_chg_type = info->cable.chg_type;
 
 	/* Determine cable/charger type */
 	if (extcon_get_state(edev, EXTCON_CHG_USB_SDP) > 0) {
@@ -601,7 +604,8 @@ static void axp288_charger_extcon_evt_worker(struct work_struct *work)
 	}
 
 	/* Cable status changed */
-	if (old_connected != info->cable.connected)
+	if (old_connected != info->cable.connected ||
+	    old_chg_type != info->cable.chg_type)
 		changed = true;
 
 	if (!changed)
@@ -609,14 +613,9 @@ static void axp288_charger_extcon_evt_worker(struct work_struct *work)
 
 	mutex_lock(&info->lock);
 
-	if (info->is_charger_enabled && !info->cable.connected) {
-		info->enable_charger = false;
-		ret = axp288_charger_enable_charger(info, info->enable_charger);
-		if (ret < 0)
-			dev_err(&info->pdev->dev,
-				"cannot disable charger (%d)", ret);
+	if (info->cable.connected) {
+		axp288_charger_enable_charger(info, false);
 
-	} else if (!info->is_charger_enabled && info->cable.connected) {
 		switch (info->cable.chg_type) {
 		case POWER_SUPPLY_TYPE_USB:
 			current_limit = ILIM_500MA;
@@ -635,17 +634,13 @@ static void axp288_charger_extcon_evt_worker(struct work_struct *work)
 
 		/* Set vbus current limit first, then enable charger */
 		ret = axp288_charger_set_vbus_inlmt(info, current_limit);
-		if (ret < 0) {
+		if (ret == 0)
+			axp288_charger_enable_charger(info, true);
+		else
 			dev_err(&info->pdev->dev,
 				"error setting current limit (%d)", ret);
-		} else {
-			info->enable_charger = (current_limit > 0);
-			ret = axp288_charger_enable_charger(info,
-							info->enable_charger);
-			if (ret < 0)
-				dev_err(&info->pdev->dev,
-					"cannot enable charger (%d)", ret);
-		}
+	} else {
+		axp288_charger_enable_charger(info, false);
 	}
 
 	if (changed)
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1545807 — [PATCH v2 03/12] power: supply: axp288_charger: Use devm_power_supply_register

Use devm_power_supply_register instead of power_supply_register,
this avoids the need to do manual cleanup and results in quite
a nice code cleanup.

Note it may seem excessive to add a "struct device *dev" helper local
variable for the 1 time it is used in this patch, but future patches
in this series also use it.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Rebase on top of for-next-next
-Only switch to devm_power_supply_register, for-next-next has already switched
 to devm_extcon_register_notifier
---
 drivers/power/supply/axp288_charger.c | 31 ++++++++-----------------------
 1 file changed, 8 insertions(+), 23 deletions(-)

diff --git a/drivers/power/supply/axp288_charger.c b/drivers/power/supply/axp288_charger.c
index 5caa154..d929742 100644
--- a/drivers/power/supply/axp288_charger.c
+++ b/drivers/power/supply/axp288_charger.c
@@ -812,6 +812,7 @@ static int axp288_charger_probe(struct platform_device *pdev)
 {
 	int ret, i, pirq;
 	struct axp288_chrg_info *info;
+	struct device *dev = &pdev->dev;
 	struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
 	struct power_supply_config charger_cfg = {};
 
@@ -862,12 +863,12 @@ static int axp288_charger_probe(struct platform_device *pdev)
 
 	/* Register with power supply class */
 	charger_cfg.drv_data = info;
-	info->psy_usb = power_supply_register(&pdev->dev, &axp288_charger_desc,
-						&charger_cfg);
+	info->psy_usb = devm_power_supply_register(dev, &axp288_charger_desc,
+						   &charger_cfg);
 	if (IS_ERR(info->psy_usb)) {
-		dev_err(&pdev->dev, "failed to register power supply charger\n");
 		ret = PTR_ERR(info->psy_usb);
-		goto psy_reg_failed;
+		dev_err(dev, "failed to register power supply: %d\n", ret);
+		return ret;
 	}
 
 	/* Register for OTG notification */
@@ -889,8 +890,7 @@ static int axp288_charger_probe(struct platform_device *pdev)
 		if (info->irq[i] < 0) {
 			dev_warn(&info->pdev->dev,
 				"failed to get virtual interrupt=%d\n", pirq);
-			ret = info->irq[i];
-			goto intr_reg_failed;
+			return info->irq[i];
 		}
 		ret = devm_request_threaded_irq(&info->pdev->dev, info->irq[i],
 					NULL, axp288_charger_irq_thread_handler,
@@ -898,34 +898,19 @@ static int axp288_charger_probe(struct platform_device *pdev)
 		if (ret) {
 			dev_err(&pdev->dev, "failed to request interrupt=%d\n",
 								info->irq[i]);
-			goto intr_reg_failed;
+			return ret;
 		}
 	}
 
 	ret = charger_init_hw_regs(info);
 	if (ret)
-		goto intr_reg_failed;
-
-	return 0;
-
-intr_reg_failed:
-	power_supply_unregister(info->psy_usb);
-psy_reg_failed:
-	return ret;
-}
-
-static int axp288_charger_remove(struct platform_device *pdev)
-{
-	struct axp288_chrg_info *info =  dev_get_drvdata(&pdev->dev);
-
-	power_supply_unregister(info->psy_usb);
+		return ret;
 
 	return 0;
 }
 
 static struct platform_driver axp288_charger_driver = {
 	.probe = axp288_charger_probe,
-	.remove = axp288_charger_remove,
 	.driver = {
 		.name = "axp288_charger",
 	},
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1545811 — [PATCH v2 05/12] power: supply: axp288_charger: Move init_hw_regs call before supply registration

Move the charger_init_hw_regs() above the power_supply_register call,
the axp288_charger_usb_set_property() uses axp288_chrg_info.max_cv and
.max_cc which get set by charger_init_hw_regs().

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/power/supply/axp288_charger.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/power/supply/axp288_charger.c b/drivers/power/supply/axp288_charger.c
index 250dd70..1588efd 100644
--- a/drivers/power/supply/axp288_charger.c
+++ b/drivers/power/supply/axp288_charger.c
@@ -836,6 +836,10 @@ static int axp288_charger_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, info);
 	mutex_init(&info->lock);
 
+	ret = charger_init_hw_regs(info);
+	if (ret)
+		return ret;
+
 	/* Register with power supply class */
 	charger_cfg.drv_data = info;
 	info->psy_usb = devm_power_supply_register(dev, &axp288_charger_desc,
@@ -890,10 +894,6 @@ static int axp288_charger_probe(struct platform_device *pdev)
 		}
 	}
 
-	ret = charger_init_hw_regs(info);
-	if (ret)
-		return ret;
-
 	return 0;
 }
 
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1545812 — [PATCH v2 01/12] power: supply: axp288_charger: Make charger_init_hw_regs propagate i2c errors

Make charger_init_hw_regs propagate i2c errors, instead of only warning
about them and then ignoring them.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/power/supply/axp288_charger.c | 62 ++++++++++++++++++++++++-----------
 1 file changed, 42 insertions(+), 20 deletions(-)

diff --git a/drivers/power/supply/axp288_charger.c b/drivers/power/supply/axp288_charger.c
index 1115052..c0f5a49 100644
--- a/drivers/power/supply/axp288_charger.c
+++ b/drivers/power/supply/axp288_charger.c
@@ -701,59 +701,73 @@ static int axp288_charger_handle_otg_evt(struct notifier_block *nb,
 	return NOTIFY_OK;
 }
 
-static void charger_init_hw_regs(struct axp288_chrg_info *info)
+static int charger_init_hw_regs(struct axp288_chrg_info *info)
 {
 	int ret, cc, cv;
 	unsigned int val;
 
 	/* Program temperature thresholds */
 	ret = regmap_write(info->regmap, AXP20X_V_LTF_CHRG, CHRG_VLTFC_0C);
-	if (ret < 0)
-		dev_warn(&info->pdev->dev, "register(%x) write error(%d)\n",
+	if (ret < 0) {
+		dev_err(&info->pdev->dev, "register(%x) write error(%d)\n",
 							AXP20X_V_LTF_CHRG, ret);
+		return ret;
+	}
 
 	ret = regmap_write(info->regmap, AXP20X_V_HTF_CHRG, CHRG_VHTFC_45C);
-	if (ret < 0)
-		dev_warn(&info->pdev->dev, "register(%x) write error(%d)\n",
+	if (ret < 0) {
+		dev_err(&info->pdev->dev, "register(%x) write error(%d)\n",
 							AXP20X_V_HTF_CHRG, ret);
+		return ret;
+	}
 
 	/* Do not turn-off charger o/p after charge cycle ends */
 	ret = regmap_update_bits(info->regmap,
 				AXP20X_CHRG_CTRL2,
 				CNTL2_CHG_OUT_TURNON, 1);
-	if (ret < 0)
-		dev_warn(&info->pdev->dev, "register(%x) write error(%d)\n",
+	if (ret < 0) {
+		dev_err(&info->pdev->dev, "register(%x) write error(%d)\n",
 						AXP20X_CHRG_CTRL2, ret);
+		return ret;
+	}
 
 	/* Enable interrupts */
 	ret = regmap_update_bits(info->regmap,
 				AXP20X_IRQ2_EN,
 				BAT_IRQ_CFG_BAT_MASK, 1);
-	if (ret < 0)
-		dev_warn(&info->pdev->dev, "register(%x) write error(%d)\n",
+	if (ret < 0) {
+		dev_err(&info->pdev->dev, "register(%x) write error(%d)\n",
 						AXP20X_IRQ2_EN, ret);
+		return ret;
+	}
 
 	ret = regmap_update_bits(info->regmap, AXP20X_IRQ3_EN,
 				TEMP_IRQ_CFG_MASK, 1);
-	if (ret < 0)
-		dev_warn(&info->pdev->dev, "register(%x) write error(%d)\n",
+	if (ret < 0) {
+		dev_err(&info->pdev->dev, "register(%x) write error(%d)\n",
 						AXP20X_IRQ3_EN, ret);
+		return ret;
+	}
 
 	/* Setup ending condition for charging to be 10% of I(chrg) */
 	ret = regmap_update_bits(info->regmap,
 				AXP20X_CHRG_CTRL1,
 				CHRG_CCCV_ITERM_20P, 0);
-	if (ret < 0)
-		dev_warn(&info->pdev->dev, "register(%x) write error(%d)\n",
+	if (ret < 0) {
+		dev_err(&info->pdev->dev, "register(%x) write error(%d)\n",
 						AXP20X_CHRG_CTRL1, ret);
+		return ret;
+	}
 
 	/* Disable OCV-SOC curve calibration */
 	ret = regmap_update_bits(info->regmap,
 				AXP20X_CC_CTRL,
 				FG_CNTL_OCV_ADJ_EN, 0);
-	if (ret < 0)
-		dev_warn(&info->pdev->dev, "register(%x) write error(%d)\n",
+	if (ret < 0) {
+		dev_err(&info->pdev->dev, "register(%x) write error(%d)\n",
 						AXP20X_CC_CTRL, ret);
+		return ret;
+	}
 
 	/* Init charging current and voltage */
 	info->max_cc = info->pdata->max_cc;
@@ -796,15 +810,21 @@ static void charger_init_hw_regs(struct axp288_chrg_info *info)
 		cv = min(info->pdata->def_cv, info->max_cv);
 
 		ret = axp288_charger_set_cc(info, cc);
-		if (ret < 0)
-			dev_warn(&info->pdev->dev,
+		if (ret < 0) {
+			dev_err(&info->pdev->dev,
 					"error(%d) in setting CC\n", ret);
+			return ret;
+		}
 
 		ret = axp288_charger_set_cv(info, cv);
-		if (ret < 0)
-			dev_warn(&info->pdev->dev,
+		if (ret < 0) {
+			dev_err(&info->pdev->dev,
 					"error(%d) in setting CV\n", ret);
+			return ret;
+		}
 	}
+
+	return 0;
 }
 
 static int axp288_charger_probe(struct platform_device *pdev)
@@ -910,7 +930,9 @@ static int axp288_charger_probe(struct platform_device *pdev)
 		}
 	}
 
-	charger_init_hw_regs(info);
+	ret = charger_init_hw_regs(info);
+	if (ret)
+		goto intr_reg_failed;
 
 	return 0;
 
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1545816 — [PATCH v2 02/12] power: supply: axp288_charger: Drop platform_data dependency

When the axp288_charger driver was originally merged, it was merged with
a dependency on some other driver providing platform data for it.

However the battery-data-framework which should provide that data never
got merged, so the axp288_charger as merged upstream has never worked,
its probe method simply always returns -ENODEV.

This commit removes the dependency on the platform_data instead reading
back the charging current and charging voltage that the firmware has set
and using those values as the maximum values the user may set.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/power/supply/axp288_charger.c | 88 ++++++++++++-----------------------
 include/linux/mfd/axp20x.h            |  7 ---
 2 files changed, 30 insertions(+), 65 deletions(-)

diff --git a/drivers/power/supply/axp288_charger.c b/drivers/power/supply/axp288_charger.c
index c0f5a49..5caa154 100644
--- a/drivers/power/supply/axp288_charger.c
+++ b/drivers/power/supply/axp288_charger.c
@@ -143,7 +143,6 @@ enum {
 
 struct axp288_chrg_info {
 	struct platform_device *pdev;
-	struct axp20x_chrg_pdata *pdata;
 	struct regmap *regmap;
 	struct regmap_irq_chip_data *regmap_irqc;
 	int irq[CHRG_INTR_END];
@@ -769,60 +768,42 @@ static int charger_init_hw_regs(struct axp288_chrg_info *info)
 		return ret;
 	}
 
-	/* Init charging current and voltage */
-	info->max_cc = info->pdata->max_cc;
-	info->max_cv = info->pdata->max_cv;
-
 	/* Read current charge voltage and current limit */
 	ret = regmap_read(info->regmap, AXP20X_CHRG_CTRL1, &val);
 	if (ret < 0) {
-		/* Assume default if cannot read */
-		info->cc = info->pdata->def_cc;
-		info->cv = info->pdata->def_cv;
-	} else {
-		/* Determine charge voltage */
-		cv = (val & CHRG_CCCV_CV_MASK) >> CHRG_CCCV_CV_BIT_POS;
-		switch (cv) {
-		case CHRG_CCCV_CV_4100MV:
-			info->cv = CV_4100MV;
-			break;
-		case CHRG_CCCV_CV_4150MV:
-			info->cv = CV_4150MV;
-			break;
-		case CHRG_CCCV_CV_4200MV:
-			info->cv = CV_4200MV;
-			break;
-		case CHRG_CCCV_CV_4350MV:
-			info->cv = CV_4350MV;
-			break;
-		default:
-			info->cv = INT_MAX;
-			break;
-		}
-
-		/* Determine charge current limit */
-		cc = (ret & CHRG_CCCV_CC_MASK) >> CHRG_CCCV_CC_BIT_POS;
-		cc = (cc * CHRG_CCCV_CC_LSB_RES) + CHRG_CCCV_CC_OFFSET;
-		info->cc = cc;
+		dev_err(&info->pdev->dev, "register(%x) read error(%d)\n",
+			AXP20X_CHRG_CTRL1, ret);
+		return ret;
+	}
 
-		/* Program default charging voltage and current */
-		cc = min(info->pdata->def_cc, info->max_cc);
-		cv = min(info->pdata->def_cv, info->max_cv);
+	/* Determine charge voltage */
+	cv = (val & CHRG_CCCV_CV_MASK) >> CHRG_CCCV_CV_BIT_POS;
+	switch (cv) {
+	case CHRG_CCCV_CV_4100MV:
+		info->cv = CV_4100MV;
+		break;
+	case CHRG_CCCV_CV_4150MV:
+		info->cv = CV_4150MV;
+		break;
+	case CHRG_CCCV_CV_4200MV:
+		info->cv = CV_4200MV;
+		break;
+	case CHRG_CCCV_CV_4350MV:
+		info->cv = CV_4350MV;
+		break;
+	}
 
-		ret = axp288_charger_set_cc(info, cc);
-		if (ret < 0) {
-			dev_err(&info->pdev->dev,
-					"error(%d) in setting CC\n", ret);
-			return ret;
-		}
+	/* Determine charge current limit */
+	cc = (ret & CHRG_CCCV_CC_MASK) >> CHRG_CCCV_CC_BIT_POS;
+	cc = (cc * CHRG_CCCV_CC_LSB_RES) + CHRG_CCCV_CC_OFFSET;
+	info->cc = cc;
 
-		ret = axp288_charger_set_cv(info, cv);
-		if (ret < 0) {
-			dev_err(&info->pdev->dev,
-					"error(%d) in setting CV\n", ret);
-			return ret;
-		}
-	}
+	/*
+	 * Do not allow the user to configure higher settings then those
+	 * set by the firmware
+	 */
+	info->max_cv = info->cv;
+	info->max_cc = info->cc;
 
 	return 0;
 }
@@ -841,15 +822,6 @@ static int axp288_charger_probe(struct platform_device *pdev)
 	info->pdev = pdev;
 	info->regmap = axp20x->regmap;
 	info->regmap_irqc = axp20x->regmap_irqc;
-	info->pdata = pdev->dev.platform_data;
-
-	if (!info->pdata) {
-		/* Try ACPI provided pdata via device properties */
-		if (!device_property_present(&pdev->dev,
-						"axp288_charger_data\n"))
-			dev_err(&pdev->dev, "failed to get platform data\n");
-		return -ENODEV;
-	}
 
 	info->cable.edev = extcon_get_extcon_dev(AXP288_EXTCON_DEV_NAME);
 	if (info->cable.edev == NULL) {
diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
index 1d07ada..6715df3 100644
--- a/include/linux/mfd/axp20x.h
+++ b/include/linux/mfd/axp20x.h
@@ -532,13 +532,6 @@ struct axp20x_dev {
 	const struct regmap_irq_chip	*regmap_irq_chip;
 };
 
-struct axp20x_chrg_pdata {
-	int max_cc;
-	int max_cv;
-	int def_cc;
-	int def_cv;
-};
-
 struct axp288_extcon_pdata {
 	/* GPIO pin control to switch D+/D- lines b/w PMIC and SOC */
 	struct gpio_desc *gpio_mux_cntl;
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1545818 — [PATCH v2 06/12] power: supply: axp288_charger: Actually get and use the USB_HOST extcon device

Nothing was setting info->otg.cable, so the extcon_get_cable_state_
calls on it would always return -EINVAL.

This commit fixes this by actually setting info->otg.cable using the new
extcon_get_extcon_dev_by_cable_id function.

This commit also makes failing to register the extcon notifier for the
USB_HOST cable an error rather then a warning, because we MUST have this
notfier to properly disable the VBUS path when in host mode so that we're
not drawing current from the 5V boost converter which is supplying power
to the otg port when in host mode.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Use extcon_get_extcon_dev instead of the new extcon_get_extcon_dev_by_id
 which was not accepted by the extcon upstream maintainer
---
 drivers/power/supply/axp288_charger.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/power/supply/axp288_charger.c b/drivers/power/supply/axp288_charger.c
index 1588efd..4443f11 100644
--- a/drivers/power/supply/axp288_charger.c
+++ b/drivers/power/supply/axp288_charger.c
@@ -127,6 +127,7 @@
 #define ILIM_3000MA			3000	/* 3000mA */
 
 #define AXP288_EXTCON_DEV_NAME		"axp288_extcon"
+#define USB_HOST_EXTCON_DEV_NAME	"INT3496:00"
 
 enum {
 	VBUS_OV_IRQ = 0,
@@ -833,6 +834,12 @@ static int axp288_charger_probe(struct platform_device *pdev)
 		return -EPROBE_DEFER;
 	}
 
+	info->otg.cable = extcon_get_extcon_dev(USB_HOST_EXTCON_DEV_NAME);
+	if (info->otg.cable == NULL) {
+		dev_dbg(dev, "EXTCON_USB_HOST is not ready, probe deferred\n");
+		return -EPROBE_DEFER;
+	}
+
 	platform_set_drvdata(pdev, info);
 	mutex_init(&info->lock);
 
@@ -868,12 +875,12 @@ static int axp288_charger_probe(struct platform_device *pdev)
 	info->otg.id_nb.notifier_call = axp288_charger_handle_otg_evt;
 	ret = devm_extcon_register_notifier(&pdev->dev, 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)
-		info->otg.id_short = extcon_get_state(
-					info->otg.cable, EXTCON_USB_HOST);
+	if (ret) {
+		dev_err(dev, "failed to register EXTCON_USB_HOST notifier\n");
+		return ret;
+	}
+	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++) {
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1546707

[Multipart message — attachments visible in raw view] — view raw

Hi,

On Wed, Dec 21, 2016 at 03:36:45PM +0100, Hans de Goede wrote:
> Here is v2 of my axp288_charger series, which fixup the axp288_charger
> code to actually work.
> 
> This implements the suggestion by Chanwoo Choi on how we can use the
> existing extcon_get_extcon_dev instead of introducing a new
> extcon_get_extcon_dev_by_id. So this series no longer depends on any
> extcon sub-sys changes.
> 
> I've also rebased it on top of linux-power-supply/for-next-next to avoid
> some conflicts with patches already there which Chanwoo pointed out,
> so assuming review goes well merging these to for-next-next should go
> smoothly :)

Thanks, queued.

-- Sebastian

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web