2015-07-30 00:26:30

by NeilBrown

[permalink] [raw]
Subject: [PATCH 00/13] Enhance twl4030_charger functionality. - V3


Following is most of my twl4030_charger patches, rebased against
git://git.infradead.org/battery-2.6

Since the previous set I have added the conversion to
module_platform_driver so EPROBE_DEFER can be used, and fixed
a few minor typos.

This does not include the changes to add extcon support, in part
because extcon has seen some changes lately which leave me even more
confused about how best to use it than before.
I need to sort that out before I can resolve the rest of my usb phy
patches and then add a few more charger patches.

Thanks,
NeilBrown


---

NeilBrown (12):
twl4030_charger: use runtime_pm to keep usb phy active while charging.
twl4030_charger: correctly handle -EPROBE_DEFER from devm_usb_get_phy_by_node
twl4030_charger: trust phy to determine when USB power is available.
twl4030_charger: split uA calculation into a function.
twl4030_charger: allow fine control of charger current.
twl4030_charger: distinguish between USB current and 'AC' current
twl4030_charger: allow max_current to be managed via sysfs.
twl4030_charger: enable manual enable/disable of usb charging.
twl4030_charger: add software controlled linear charging mode.
twl4030_charger: add ac/mode to match usb/mode
twl4030_charger: Increase current carefully while watching voltage.
twl4030_charger: assume a 'charger' can supply maximum current.

Pavel Machek (1):
twl4030_charger: convert to module_platform_driver instead of ..._probe.


.../ABI/testing/sysfs-class-power-twl4030 | 45 ++
drivers/mfd/twl-core.c | 9
drivers/power/twl4030_charger.c | 541 ++++++++++++++++++--
3 files changed, 531 insertions(+), 64 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-class-power-twl4030

--
Signature


2015-07-30 00:25:28

by NeilBrown

[permalink] [raw]
Subject: [PATCH 07/13] twl4030_charger: distinguish between USB current and 'AC' current


The twl4030 charger has two current sources, 'USB' and 'AC'
(presumably "Accessory Charger" because it isn't Alternating Current).

If 'AC' is providing current, we should set the current limit
differently to when it isn't (and so USB is used).
So split 'cur' into 'usb_cur' and 'ac_cur' and use accordingly.

Now we must review the current setting on any interrupt or USB
event which might indicate that the charger-source has changed.

Acked-by: Pavel Machek <[email protected]>
Signed-off-by: NeilBrown <[email protected]>
---
drivers/power/twl4030_charger.c | 36 +++++++++++++++++++++++++++++-------
1 file changed, 29 insertions(+), 7 deletions(-)

diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index 3b7cc631bb8a..982675df21b7 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -22,6 +22,7 @@
#include <linux/power_supply.h>
#include <linux/notifier.h>
#include <linux/usb/otg.h>
+#include <linux/i2c/twl4030-madc.h>

#define TWL4030_BCIMSTATEC 0x02
#define TWL4030_BCIICHG 0x08
@@ -101,10 +102,13 @@ struct twl4030_bci {
int usb_enabled;

/*
- * ichg values in uA. If any are 'large', we set CGAIN to
- * '1' which doubles the range for half the precision.
+ * ichg_* and *_cur values in uA. If any are 'large', we set
+ * CGAIN to '1' which doubles the range for half the
+ * precision.
*/
- unsigned int ichg_eoc, ichg_lo, ichg_hi, cur;
+ unsigned int ichg_eoc, ichg_lo, ichg_hi;
+ unsigned int usb_cur, ac_cur;
+ bool ac_is_active;

unsigned long event;
};
@@ -225,11 +229,24 @@ static int ua2regval(int ua, bool cgain)
static int twl4030_charger_update_current(struct twl4030_bci *bci)
{
int status;
+ int cur;
unsigned reg, cur_reg;
u8 bcictl1, oldreg, fullreg;
bool cgain = false;
u8 boot_bci;

+ /*
+ * If AC (Accessory Charger) voltage exceeds 4.5V (MADC 11)
+ * and AC is enabled, set current for 'ac'
+ */
+ if (twl4030_get_madc_conversion(11) > 4500) {
+ cur = bci->ac_cur;
+ bci->ac_is_active = true;
+ } else {
+ cur = bci->usb_cur;
+ bci->ac_is_active = false;
+ }
+
/* First, check thresholds and see if cgain is needed */
if (bci->ichg_eoc >= 200000)
cgain = true;
@@ -237,7 +254,7 @@ static int twl4030_charger_update_current(struct twl4030_bci *bci)
cgain = true;
if (bci->ichg_hi >= 820000)
cgain = true;
- if (bci->cur > 852000)
+ if (cur > 852000)
cgain = true;

status = twl4030_bci_read(TWL4030_BCICTL1, &bcictl1);
@@ -318,7 +335,7 @@ static int twl4030_charger_update_current(struct twl4030_bci *bci)
* And finally, set the current. This is stored in
* two registers.
*/
- reg = ua2regval(bci->cur, cgain);
+ reg = ua2regval(cur, cgain);
/* we have only 10 bits */
if (reg > 0x3ff)
reg = 0x3ff;
@@ -371,6 +388,8 @@ static int twl4030_charger_enable_usb(struct twl4030_bci *bci, bool enable)

if (enable && !IS_ERR_OR_NULL(bci->transceiver)) {

+ twl4030_charger_update_current(bci);
+
/* Need to keep phy powered */
if (!bci->usb_enabled) {
pm_runtime_get_sync(bci->transceiver->dev);
@@ -463,6 +482,7 @@ static irqreturn_t twl4030_charger_interrupt(int irq, void *arg)
struct twl4030_bci *bci = arg;

dev_dbg(bci->dev, "CHG_PRES irq\n");
+ twl4030_charger_update_current(bci);
power_supply_changed(bci->ac);
power_supply_changed(bci->usb);

@@ -495,6 +515,7 @@ static irqreturn_t twl4030_bci_interrupt(int irq, void *arg)
power_supply_changed(bci->ac);
power_supply_changed(bci->usb);
}
+ twl4030_charger_update_current(bci);

/* various monitoring events, for now we just log them here */
if (irqs1 & (TWL4030_TBATOR2 | TWL4030_TBATOR1))
@@ -724,10 +745,11 @@ static int twl4030_bci_probe(struct platform_device *pdev)
bci->ichg_eoc = 80100; /* Stop charging when current drops to here */
bci->ichg_lo = 241000; /* Low threshold */
bci->ichg_hi = 500000; /* High threshold */
+ bci->ac_cur = 500000; /* 500mA */
if (allow_usb)
- bci->cur = 500000; /* 500mA */
+ bci->usb_cur = 500000; /* 500mA */
else
- bci->cur = 100000; /* 100mA */
+ bci->usb_cur = 100000; /* 100mA */

bci->dev = &pdev->dev;
bci->irq_chg = platform_get_irq(pdev, 0);

2015-07-30 00:26:12

by NeilBrown

[permalink] [raw]
Subject: [PATCH 01/13] twl4030_charger: use runtime_pm to keep usb phy active while charging.

The twl4030 usb phy needs to be active while we are using
the USB VBUS as a current source for charging.
In particular, the usb3v1 regulator must be enabled and the
PHY_PWR_PHYPWD bit must be set to keep the phy powered.

commit ab37813f4093a5f59cb8e083cde277289dc72ed3
twl4030_charger: Allow charger to control the regulator that feeds it

gave the charger control over the regulator, but didn't resolve
the PHY_PWR_PHYPWD issue.

Now that both of these are controlled by runtime_pm in
phy-twl4030-usb, we can simply take a runtime_pm reference to the USB
phy whenever the charger wants to use it as a current source.

So this patch reverts the above commit, and adds the necessary
runtime_pm calls.

Acked-by: Lee Jones <[email protected]>
Signed-off-by: NeilBrown <[email protected]>
---
drivers/mfd/twl-core.c | 9 ++++-----
drivers/power/twl4030_charger.c | 18 +++++-------------
2 files changed, 9 insertions(+), 18 deletions(-)

diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
index 489674a2497e..831696ee2472 100644
--- a/drivers/mfd/twl-core.c
+++ b/drivers/mfd/twl-core.c
@@ -788,9 +788,8 @@ add_children(struct twl4030_platform_data *pdata, unsigned irq_base,
static struct regulator_consumer_supply usb1v8 = {
.supply = "usb1v8",
};
- static struct regulator_consumer_supply usb3v1[] = {
- { .supply = "usb3v1" },
- { .supply = "bci3v1" },
+ static struct regulator_consumer_supply usb3v1 = {
+ .supply = "usb3v1",
};

/* First add the regulators so that they can be used by transceiver */
@@ -818,7 +817,7 @@ add_children(struct twl4030_platform_data *pdata, unsigned irq_base,
return PTR_ERR(child);

child = add_regulator_linked(TWL4030_REG_VUSB3V1,
- &usb_fixed, usb3v1, 2,
+ &usb_fixed, &usb3v1, 1,
features);
if (IS_ERR(child))
return PTR_ERR(child);
@@ -838,7 +837,7 @@ add_children(struct twl4030_platform_data *pdata, unsigned irq_base,
if (IS_ENABLED(CONFIG_REGULATOR_TWL4030) && child) {
usb1v5.dev_name = dev_name(child);
usb1v8.dev_name = dev_name(child);
- usb3v1[0].dev_name = dev_name(child);
+ usb3v1.dev_name = dev_name(child);
}
}

diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index 709d90dc75f1..fe71c61109f5 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -22,7 +22,6 @@
#include <linux/power_supply.h>
#include <linux/notifier.h>
#include <linux/usb/otg.h>
-#include <linux/regulator/machine.h>

#define TWL4030_BCIMSTATEC 0x02
#define TWL4030_BCIICHG 0x08
@@ -94,7 +93,6 @@ struct twl4030_bci {
struct work_struct work;
int irq_chg;
int irq_bci;
- struct regulator *usb_reg;
int usb_enabled;

unsigned long event;
@@ -208,7 +206,7 @@ static int twl4030_charger_enable_usb(struct twl4030_bci *bci, bool enable)
{
int ret;

- if (enable) {
+ if (enable && !IS_ERR_OR_NULL(bci->transceiver)) {
/* Check for USB charger connected */
if (!twl4030_bci_have_vbus(bci))
return -ENODEV;
@@ -222,14 +220,9 @@ static int twl4030_charger_enable_usb(struct twl4030_bci *bci, bool enable)
return -EACCES;
}

- /* Need to keep regulator on */
+ /* Need to keep phy powered */
if (!bci->usb_enabled) {
- ret = regulator_enable(bci->usb_reg);
- if (ret) {
- dev_err(bci->dev,
- "Failed to enable regulator\n");
- return ret;
- }
+ pm_runtime_get_sync(bci->transceiver->dev);
bci->usb_enabled = 1;
}

@@ -244,7 +237,8 @@ static int twl4030_charger_enable_usb(struct twl4030_bci *bci, bool enable)
} else {
ret = twl4030_clear_set_boot_bci(TWL4030_BCIAUTOUSB, 0);
if (bci->usb_enabled) {
- regulator_disable(bci->usb_reg);
+ pm_runtime_mark_last_busy(bci->transceiver->dev);
+ pm_runtime_put_autosuspend(bci->transceiver->dev);
bci->usb_enabled = 0;
}
}
@@ -609,8 +603,6 @@ static int __init twl4030_bci_probe(struct platform_device *pdev)
return ret;
}

- bci->usb_reg = regulator_get(bci->dev, "bci3v1");
-
bci->usb = devm_power_supply_register(&pdev->dev, &twl4030_bci_usb_desc,
NULL);
if (IS_ERR(bci->usb)) {

2015-07-30 00:25:34

by NeilBrown

[permalink] [raw]
Subject: [PATCH 12/13] twl4030_charger: Increase current carefully while watching voltage.


The USB Battery Charging spec (BC1.2) suggests a dedicated
charging port can deliver from 0.5 to 5.0A at between 4.75 and 5.25
volts.

To choose the "correct" current voltage setting requires a trial
and error approach: try to draw current and see if the voltage drops
too low.

Even with a configured Standard Downstream Port, it may not be possible
to reliably pull 500mA - depending on cable quality and source
quality I have reports of charging failure due to the voltage dropping
too low.

To address both these concerns, this patch introduce incremental
current setting.
The current pull from VBUS is increased in steps of 20mA every 100ms
until the target is reached or until the measure voltage drops below
4.75V. If the voltage does go too low, the target current is reduced
by 20mA and kept there.

This applies to currents selected automatically, or to values
set via sysfs. So setting a large value will cause the maximum
available to be used - up to the limit of 1.7A imposed by the
hardware.

Signed-off-by: NeilBrown <[email protected]>
---
drivers/power/twl4030_charger.c | 67 ++++++++++++++++++++++++++++++++++++---
1 file changed, 61 insertions(+), 6 deletions(-)

diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index 68117ad23564..2c537ee11bbe 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -119,6 +119,18 @@ struct twl4030_bci {
#define CHARGE_AUTO 1
#define CHARGE_LINEAR 2

+ /* When setting the USB current we slowly increase the
+ * requested current until target is reached or the voltage
+ * drops below 4.75V. In the latter case we step back one
+ * step.
+ */
+ unsigned int usb_cur_target;
+ struct delayed_work current_worker;
+#define USB_CUR_STEP 20000 /* 20mA at a time */
+#define USB_MIN_VOLT 4750000 /* 4.75V */
+#define USB_CUR_DELAY msecs_to_jiffies(100)
+#define USB_MAX_CURRENT 1700000 /* TWL4030 caps at 1.7A */
+
unsigned long event;
};

@@ -257,6 +269,12 @@ static int twl4030_charger_update_current(struct twl4030_bci *bci)
} else {
cur = bci->usb_cur;
bci->ac_is_active = false;
+ if (cur > bci->usb_cur_target) {
+ cur = bci->usb_cur_target;
+ bci->usb_cur = cur;
+ }
+ if (cur < bci->usb_cur_target)
+ schedule_delayed_work(&bci->current_worker, USB_CUR_DELAY);
}

/* First, check thresholds and see if cgain is needed */
@@ -391,6 +409,41 @@ static int twl4030_charger_update_current(struct twl4030_bci *bci)
return 0;
}

+static int twl4030_charger_get_current(void);
+
+static void twl4030_current_worker(struct work_struct *data)
+{
+ int v, curr;
+ int res;
+ struct twl4030_bci *bci = container_of(data, struct twl4030_bci,
+ current_worker.work);
+
+ res = twl4030bci_read_adc_val(TWL4030_BCIVBUS);
+ if (res < 0)
+ v = 0;
+ else
+ /* BCIVBUS uses ADCIN8, 7/1023 V/step */
+ v = res * 6843;
+ curr = twl4030_charger_get_current();
+
+ dev_dbg(bci->dev, "v=%d cur=%d limit=%d target=%d\n", v, curr,
+ bci->usb_cur, bci->usb_cur_target);
+
+ if (v < USB_MIN_VOLT) {
+ /* Back up and stop adjusting. */
+ bci->usb_cur -= USB_CUR_STEP;
+ bci->usb_cur_target = bci->usb_cur;
+ } else if (bci->usb_cur >= bci->usb_cur_target ||
+ bci->usb_cur + USB_CUR_STEP > USB_MAX_CURRENT) {
+ /* Reached target and voltage is OK - stop */
+ return;
+ } else {
+ bci->usb_cur += USB_CUR_STEP;
+ schedule_delayed_work(&bci->current_worker, USB_CUR_DELAY);
+ }
+ twl4030_charger_update_current(bci);
+}
+
/*
* Enable/Disable USB Charge functionality.
*/
@@ -451,6 +504,7 @@ static int twl4030_charger_enable_usb(struct twl4030_bci *bci, bool enable)
pm_runtime_put_autosuspend(bci->transceiver->dev);
bci->usb_enabled = 0;
}
+ bci->usb_cur = 0;
}

return ret;
@@ -599,7 +653,7 @@ twl4030_bci_max_current_store(struct device *dev, struct device_attribute *attr,
if (dev == &bci->ac->dev)
bci->ac_cur = cur;
else
- bci->usb_cur = cur;
+ bci->usb_cur_target = cur;

twl4030_charger_update_current(bci);
return n;
@@ -621,7 +675,7 @@ static ssize_t twl4030_bci_max_current_show(struct device *dev,
cur = bci->ac_cur;
} else {
if (bci->ac_is_active)
- cur = bci->usb_cur;
+ cur = bci->usb_cur_target;
}
if (cur < 0) {
cur = twl4030bci_read_adc_val(TWL4030_BCIIREF1);
@@ -662,9 +716,9 @@ static int twl4030_bci_usb_ncb(struct notifier_block *nb, unsigned long val,

/* reset current on each 'plug' event */
if (allow_usb)
- bci->usb_cur = 500000;
+ bci->usb_cur_target = 500000;
else
- bci->usb_cur = 100000;
+ bci->usb_cur_target = 100000;

bci->event = val;
schedule_work(&bci->work);
@@ -927,9 +981,9 @@ static int twl4030_bci_probe(struct platform_device *pdev)
bci->ichg_hi = 500000; /* High threshold */
bci->ac_cur = 500000; /* 500mA */
if (allow_usb)
- bci->usb_cur = 500000; /* 500mA */
+ bci->usb_cur_target = 500000; /* 500mA */
else
- bci->usb_cur = 100000; /* 100mA */
+ bci->usb_cur_target = 100000; /* 100mA */
bci->usb_mode = CHARGE_AUTO;
bci->ac_mode = CHARGE_AUTO;

@@ -980,6 +1034,7 @@ static int twl4030_bci_probe(struct platform_device *pdev)
}

INIT_WORK(&bci->work, twl4030_bci_usb_work);
+ INIT_DELAYED_WORK(&bci->current_worker, twl4030_current_worker);

bci->usb_nb.notifier_call = twl4030_bci_usb_ncb;
if (bci->dev->of_node) {

2015-07-30 00:26:22

by NeilBrown

[permalink] [raw]
Subject: [PATCH 03/13] twl4030_charger: correctly handle -EPROBE_DEFER from devm_usb_get_phy_by_node

Now that twl4030_bci_probe can safely return -EPROBE_DEFER,
do so when devm_usb_get_phy_by_node returns that error.

Signed-off-by: NeilBrown <[email protected]>
---
drivers/power/twl4030_charger.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index 045238370d3f..ffc123fb7158 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -636,9 +636,13 @@ static int twl4030_bci_probe(struct platform_device *pdev)

phynode = of_find_compatible_node(bci->dev->of_node->parent,
NULL, "ti,twl4030-usb");
- if (phynode)
+ if (phynode) {
bci->transceiver = devm_usb_get_phy_by_node(
bci->dev, phynode, &bci->usb_nb);
+ if (IS_ERR(bci->transceiver) &&
+ PTR_ERR(bci->transceiver) == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+ }
}

/* Enable interrupts now. */

2015-07-30 00:26:47

by NeilBrown

[permalink] [raw]
Subject: [PATCH 06/13] twl4030_charger: allow fine control of charger current.

The twl4030 allows control of the incoming current.
Part of this control is a 'CGAIN' setting which doubles
the range for half the precision. This control affects
several different current setting, so all need to be updated
at once when CGAIN is changed.

With this patch, all of these current setting are managed
by the driver, but most are left at their default settings.

The current drawn is set to 500mA if the allow_usb module parameter is
set, and to 100mA otherwise.
More fine control will appear in later patches.

Acked-by: Pavel Machek <[email protected]>
Signed-off-by: NeilBrown <[email protected]>
---
drivers/power/twl4030_charger.c | 168 +++++++++++++++++++++++++++++++++++++--
1 file changed, 160 insertions(+), 8 deletions(-)

diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index 29984b263a35..3b7cc631bb8a 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -31,6 +31,11 @@
#define TWL4030_BCIMFSTS4 0x10
#define TWL4030_BCICTL1 0x23
#define TWL4030_BB_CFG 0x12
+#define TWL4030_BCIIREF1 0x27
+#define TWL4030_BCIIREF2 0x28
+#define TWL4030_BCIMFKEY 0x11
+#define TWL4030_BCIMFTH8 0x1d
+#define TWL4030_BCIMFTH9 0x1e

#define TWL4030_BCIMFSTS1 0x01

@@ -95,6 +100,12 @@ struct twl4030_bci {
int irq_bci;
int usb_enabled;

+ /*
+ * ichg values in uA. If any are 'large', we set CGAIN to
+ * '1' which doubles the range for half the precision.
+ */
+ unsigned int ichg_eoc, ichg_lo, ichg_hi, cur;
+
unsigned long event;
};

@@ -211,6 +222,146 @@ static int ua2regval(int ua, bool cgain)
return ret;
}

+static int twl4030_charger_update_current(struct twl4030_bci *bci)
+{
+ int status;
+ unsigned reg, cur_reg;
+ u8 bcictl1, oldreg, fullreg;
+ bool cgain = false;
+ u8 boot_bci;
+
+ /* First, check thresholds and see if cgain is needed */
+ if (bci->ichg_eoc >= 200000)
+ cgain = true;
+ if (bci->ichg_lo >= 400000)
+ cgain = true;
+ if (bci->ichg_hi >= 820000)
+ cgain = true;
+ if (bci->cur > 852000)
+ cgain = true;
+
+ status = twl4030_bci_read(TWL4030_BCICTL1, &bcictl1);
+ if (status < 0)
+ return status;
+ if (twl_i2c_read_u8(TWL_MODULE_PM_MASTER, &boot_bci,
+ TWL4030_PM_MASTER_BOOT_BCI) < 0)
+ boot_bci = 0;
+ boot_bci &= 7;
+
+ if ((!!cgain) != !!(bcictl1 & TWL4030_CGAIN))
+ /* Need to turn for charging while we change the
+ * CGAIN bit. Leave it off while everything is
+ * updated.
+ */
+ twl4030_clear_set_boot_bci(boot_bci, 0);
+
+ /*
+ * For ichg_eoc, the hardware only supports reg values matching
+ * 100XXXX000, and requires the XXXX be stored in the high nibble
+ * of TWL4030_BCIMFTH8.
+ */
+ reg = ua2regval(bci->ichg_eoc, cgain);
+ if (reg > 0x278)
+ reg = 0x278;
+ if (reg < 0x200)
+ reg = 0x200;
+ reg = (reg >> 3) & 0xf;
+ fullreg = reg << 4;
+
+ /*
+ * For ichg_lo, reg value must match 10XXXX0000.
+ * XXXX is stored in low nibble of TWL4030_BCIMFTH8.
+ */
+ reg = ua2regval(bci->ichg_lo, cgain);
+ if (reg > 0x2F0)
+ reg = 0x2F0;
+ if (reg < 0x200)
+ reg = 0x200;
+ reg = (reg >> 4) & 0xf;
+ fullreg |= reg;
+
+ /* ichg_eoc and ichg_lo live in same register */
+ status = twl4030_bci_read(TWL4030_BCIMFTH8, &oldreg);
+ if (status < 0)
+ return status;
+ if (oldreg != fullreg) {
+ status = twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE, 0xF4,
+ TWL4030_BCIMFKEY);
+ if (status < 0)
+ return status;
+ twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE,
+ fullreg, TWL4030_BCIMFTH8);
+ }
+
+ /* ichg_hi threshold must be 1XXXX01100 (I think) */
+ reg = ua2regval(bci->ichg_hi, cgain);
+ if (reg > 0x3E0)
+ reg = 0x3E0;
+ if (reg < 0x200)
+ reg = 0x200;
+ fullreg = (reg >> 5) & 0xF;
+ fullreg <<= 4;
+ status = twl4030_bci_read(TWL4030_BCIMFTH9, &oldreg);
+ if (status < 0)
+ return status;
+ if ((oldreg & 0xF0) != fullreg) {
+ fullreg |= (oldreg & 0x0F);
+ status = twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE, 0xE7,
+ TWL4030_BCIMFKEY);
+ if (status < 0)
+ return status;
+ twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE,
+ fullreg, TWL4030_BCIMFTH9);
+ }
+
+ /*
+ * And finally, set the current. This is stored in
+ * two registers.
+ */
+ reg = ua2regval(bci->cur, cgain);
+ /* we have only 10 bits */
+ if (reg > 0x3ff)
+ reg = 0x3ff;
+ status = twl4030_bci_read(TWL4030_BCIIREF1, &oldreg);
+ if (status < 0)
+ return status;
+ cur_reg = oldreg;
+ status = twl4030_bci_read(TWL4030_BCIIREF2, &oldreg);
+ if (status < 0)
+ return status;
+ cur_reg |= oldreg << 8;
+ if (reg != oldreg) {
+ /* disable write protection for one write access for
+ * BCIIREF */
+ status = twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE, 0xE7,
+ TWL4030_BCIMFKEY);
+ if (status < 0)
+ return status;
+ status = twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE,
+ (reg & 0x100) ? 3 : 2,
+ TWL4030_BCIIREF2);
+ if (status < 0)
+ return status;
+ /* disable write protection for one write access for
+ * BCIIREF */
+ status = twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE, 0xE7,
+ TWL4030_BCIMFKEY);
+ if (status < 0)
+ return status;
+ status = twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE,
+ reg & 0xff,
+ TWL4030_BCIIREF1);
+ }
+ if ((!!cgain) != !!(bcictl1 & TWL4030_CGAIN)) {
+ /* Flip CGAIN and re-enable charging */
+ bcictl1 ^= TWL4030_CGAIN;
+ twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE,
+ bcictl1, TWL4030_BCICTL1);
+ twl4030_clear_set_boot_bci(0, boot_bci);
+ }
+ return 0;
+}
+
/*
* Enable/Disable USB Charge functionality.
*/
@@ -219,14 +370,6 @@ static int twl4030_charger_enable_usb(struct twl4030_bci *bci, bool enable)
int ret;

if (enable && !IS_ERR_OR_NULL(bci->transceiver)) {
- /*
- * Until we can find out what current the device can provide,
- * require a module param to enable USB charging.
- */
- if (!allow_usb) {
- dev_warn(bci->dev, "USB charging is disabled.\n");
- return -EACCES;
- }

/* Need to keep phy powered */
if (!bci->usb_enabled) {
@@ -578,6 +721,14 @@ static int twl4030_bci_probe(struct platform_device *pdev)
if (!pdata)
pdata = twl4030_bci_parse_dt(&pdev->dev);

+ bci->ichg_eoc = 80100; /* Stop charging when current drops to here */
+ bci->ichg_lo = 241000; /* Low threshold */
+ bci->ichg_hi = 500000; /* High threshold */
+ if (allow_usb)
+ bci->cur = 500000; /* 500mA */
+ else
+ bci->cur = 100000; /* 100mA */
+
bci->dev = &pdev->dev;
bci->irq_chg = platform_get_irq(pdev, 0);
bci->irq_bci = platform_get_irq(pdev, 1);
@@ -657,6 +808,7 @@ static int twl4030_bci_probe(struct platform_device *pdev)
if (ret < 0)
dev_warn(&pdev->dev, "failed to unmask interrupts: %d\n", ret);

+ twl4030_charger_update_current(bci);
twl4030_charger_enable_ac(true);
if (!IS_ERR_OR_NULL(bci->transceiver))
twl4030_bci_usb_ncb(&bci->usb_nb,

2015-07-30 00:26:38

by NeilBrown

[permalink] [raw]
Subject: [PATCH 02/13] twl4030_charger: convert to module_platform_driver instead of ..._probe.

From: Pavel Machek <[email protected]>

Drivers using module_platform_driver_probe cannot return
EPROBE_DEFER from the probe function, which makes them rather useless
these days...

Convert to module_platform_driver() so EPROBE_DEFER can be used.

Signed-off-by: Pavel Machek <[email protected]>
Signed-off-by: NeilBrown <[email protected]>
---
drivers/power/twl4030_charger.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index fe71c61109f5..045238370d3f 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -568,7 +568,7 @@ static const struct power_supply_desc twl4030_bci_usb_desc = {
.get_property = twl4030_bci_get_property,
};

-static int __init twl4030_bci_probe(struct platform_device *pdev)
+static int twl4030_bci_probe(struct platform_device *pdev)
{
struct twl4030_bci *bci;
const struct twl4030_bci_platform_data *pdata = pdev->dev.platform_data;
@@ -692,14 +692,14 @@ static const struct of_device_id twl_bci_of_match[] = {
MODULE_DEVICE_TABLE(of, twl_bci_of_match);

static struct platform_driver twl4030_bci_driver = {
+ .probe = twl4030_bci_probe,
.driver = {
.name = "twl4030_bci",
.of_match_table = of_match_ptr(twl_bci_of_match),
},
.remove = __exit_p(twl4030_bci_remove),
};
-
-module_platform_driver_probe(twl4030_bci_driver, twl4030_bci_probe);
+module_platform_driver(twl4030_bci_driver);

MODULE_AUTHOR("GraÅžvydas Ignotas");
MODULE_DESCRIPTION("TWL4030 Battery Charger Interface driver");

2015-07-30 00:26:57

by NeilBrown

[permalink] [raw]
Subject: [PATCH 08/13] twl4030_charger: allow max_current to be managed via sysfs.

'max_current' sysfs attributes are created which allow the
max to be set.
Whenever a current source changes, the default is restored.
This will be followed by a uevent, so user-space can decide to
update again.

Acked-by: Pavel Machek <[email protected]>
Signed-off-by: NeilBrown <[email protected]>
---
.../ABI/testing/sysfs-class-power-twl4030 | 15 ++++
drivers/power/twl4030_charger.c | 72 ++++++++++++++++++++
2 files changed, 87 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-class-power-twl4030

diff --git a/Documentation/ABI/testing/sysfs-class-power-twl4030 b/Documentation/ABI/testing/sysfs-class-power-twl4030
new file mode 100644
index 000000000000..0331bba4605d
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-power-twl4030
@@ -0,0 +1,15 @@
+What: /sys/class/power_supply/twl4030_ac/max_current
+ /sys/class/power_supply/twl4030_usb/max_current
+Description:
+ Read/Write limit on current which may
+ be drawn from the ac (Accessory Charger) or
+ USB port.
+
+ Value is in micro-Amps.
+
+ Value is set automatically to an appropriate
+ value when a cable is plugged or unplugged.
+
+ Value can the set by writing to the attribute.
+ The change will only persist until the next
+ plug event. These event are reported via udev.
diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index 982675df21b7..b0a50adebfda 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -482,6 +482,8 @@ static irqreturn_t twl4030_charger_interrupt(int irq, void *arg)
struct twl4030_bci *bci = arg;

dev_dbg(bci->dev, "CHG_PRES irq\n");
+ /* reset current on each 'plug' event */
+ bci->ac_cur = 500000;
twl4030_charger_update_current(bci);
power_supply_changed(bci->ac);
power_supply_changed(bci->usb);
@@ -536,6 +538,63 @@ static irqreturn_t twl4030_bci_interrupt(int irq, void *arg)
return IRQ_HANDLED;
}

+/*
+ * Provide "max_current" attribute in sysfs.
+ */
+static ssize_t
+twl4030_bci_max_current_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t n)
+{
+ struct twl4030_bci *bci = dev_get_drvdata(dev->parent);
+ int cur = 0;
+ int status = 0;
+ status = kstrtoint(buf, 10, &cur);
+ if (status)
+ return status;
+ if (cur < 0)
+ return -EINVAL;
+ if (dev == &bci->ac->dev)
+ bci->ac_cur = cur;
+ else
+ bci->usb_cur = cur;
+
+ twl4030_charger_update_current(bci);
+ return n;
+}
+
+/*
+ * sysfs max_current show
+ */
+static ssize_t twl4030_bci_max_current_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ int status = 0;
+ int cur = -1;
+ u8 bcictl1;
+ struct twl4030_bci *bci = dev_get_drvdata(dev->parent);
+
+ if (dev == &bci->ac->dev) {
+ if (!bci->ac_is_active)
+ cur = bci->ac_cur;
+ } else {
+ if (bci->ac_is_active)
+ cur = bci->usb_cur;
+ }
+ if (cur < 0) {
+ cur = twl4030bci_read_adc_val(TWL4030_BCIIREF1);
+ if (cur < 0)
+ return cur;
+ status = twl4030_bci_read(TWL4030_BCICTL1, &bcictl1);
+ if (status < 0)
+ return status;
+ cur = regval2ua(cur, bcictl1 & TWL4030_CGAIN);
+ }
+ return scnprintf(buf, PAGE_SIZE, "%u\n", cur);
+}
+
+static DEVICE_ATTR(max_current, 0644, twl4030_bci_max_current_show,
+ twl4030_bci_max_current_store);
+
static void twl4030_bci_usb_work(struct work_struct *data)
{
struct twl4030_bci *bci = container_of(data, struct twl4030_bci, work);
@@ -558,6 +617,12 @@ static int twl4030_bci_usb_ncb(struct notifier_block *nb, unsigned long val,

dev_dbg(bci->dev, "OTG notify %lu\n", val);

+ /* reset current on each 'plug' event */
+ if (allow_usb)
+ bci->usb_cur = 500000;
+ else
+ bci->usb_cur = 100000;
+
bci->event = val;
schedule_work(&bci->work);

@@ -831,6 +896,11 @@ static int twl4030_bci_probe(struct platform_device *pdev)
dev_warn(&pdev->dev, "failed to unmask interrupts: %d\n", ret);

twl4030_charger_update_current(bci);
+ if (device_create_file(&bci->usb->dev, &dev_attr_max_current))
+ dev_warn(&pdev->dev, "could not create sysfs file\n");
+ if (device_create_file(&bci->ac->dev, &dev_attr_max_current))
+ dev_warn(&pdev->dev, "could not create sysfs file\n");
+
twl4030_charger_enable_ac(true);
if (!IS_ERR_OR_NULL(bci->transceiver))
twl4030_bci_usb_ncb(&bci->usb_nb,
@@ -855,6 +925,8 @@ static int __exit twl4030_bci_remove(struct platform_device *pdev)
twl4030_charger_enable_usb(bci, false);
twl4030_charger_enable_backup(0, 0);

+ device_remove_file(&bci->usb->dev, &dev_attr_max_current);
+ device_remove_file(&bci->ac->dev, &dev_attr_max_current);
/* mask interrupts */
twl_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xff,
TWL4030_INTERRUPTS_BCIIMR1A);

2015-07-30 00:27:07

by NeilBrown

[permalink] [raw]
Subject: [PATCH 11/13] twl4030_charger: add ac/mode to match usb/mode


This allows AC charging to be turned off, much like usb charging.
"continuous" mode is not available though.

Acked-by: Pavel Machek <[email protected]>
Signed-off-by: NeilBrown <[email protected]>
---
.../ABI/testing/sysfs-class-power-twl4030 | 10 ++++++
drivers/power/twl4030_charger.c | 35 +++++++++++++++-----
2 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-class-power-twl4030 b/Documentation/ABI/testing/sysfs-class-power-twl4030
index e8baa36aaa2b..be26af0f1895 100644
--- a/Documentation/ABI/testing/sysfs-class-power-twl4030
+++ b/Documentation/ABI/testing/sysfs-class-power-twl4030
@@ -33,3 +33,13 @@ Description:
This is useful for unstable power sources
such as bicycle dynamo, but care should
be taken that battery is not over-charged.
+
+What: /sys/class/power_supply/twl4030_ac/mode
+Description:
+ Changing mode for 'ac' port.
+ Writing to this can disable charging.
+
+ Possible values are:
+ "auto" - draw power as appropriate for detected
+ power source and battery status.
+ "off" - do not draw any power.
diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index de5430deaf23..68117ad23564 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -114,7 +114,7 @@ struct twl4030_bci {
unsigned int ichg_eoc, ichg_lo, ichg_hi;
unsigned int usb_cur, ac_cur;
bool ac_is_active;
- int usb_mode; /* charging mode requested */
+ int usb_mode, ac_mode; /* charging mode requested */
#define CHARGE_OFF 0
#define CHARGE_AUTO 1
#define CHARGE_LINEAR 2
@@ -459,10 +459,13 @@ static int twl4030_charger_enable_usb(struct twl4030_bci *bci, bool enable)
/*
* Enable/Disable AC Charge funtionality.
*/
-static int twl4030_charger_enable_ac(bool enable)
+static int twl4030_charger_enable_ac(struct twl4030_bci *bci, bool enable)
{
int ret;

+ if (bci->ac_mode == CHARGE_OFF)
+ enable = false;
+
if (enable)
ret = twl4030_clear_set_boot_bci(0, TWL4030_BCIAUTOAC);
else
@@ -688,9 +691,17 @@ twl4030_bci_mode_store(struct device *dev, struct device_attribute *attr,
mode = 2;
else
return -EINVAL;
- twl4030_charger_enable_usb(bci, false);
- bci->usb_mode = mode;
- status = twl4030_charger_enable_usb(bci, true);
+ if (dev == &bci->ac->dev) {
+ if (mode == 2)
+ return -EINVAL;
+ twl4030_charger_enable_ac(bci, false);
+ bci->ac_mode = mode;
+ status = twl4030_charger_enable_ac(bci, true);
+ } else {
+ twl4030_charger_enable_usb(bci, false);
+ bci->usb_mode = mode;
+ status = twl4030_charger_enable_usb(bci, true);
+ }
return (status == 0) ? n : status;
}

@@ -704,9 +715,13 @@ twl4030_bci_mode_show(struct device *dev,
struct twl4030_bci *bci = dev_get_drvdata(dev->parent);
int len = 0;
int i;
+ int mode = bci->usb_mode;
+
+ if (dev == &bci->ac->dev)
+ mode = bci->ac_mode;

for (i = 0; i < ARRAY_SIZE(modes); i++)
- if (bci->usb_mode == i)
+ if (mode == i)
len += snprintf(buf+len, PAGE_SIZE-len,
"[%s] ", modes[i]);
else
@@ -916,6 +931,7 @@ static int twl4030_bci_probe(struct platform_device *pdev)
else
bci->usb_cur = 100000; /* 100mA */
bci->usb_mode = CHARGE_AUTO;
+ bci->ac_mode = CHARGE_AUTO;

bci->dev = &pdev->dev;
bci->irq_chg = platform_get_irq(pdev, 0);
@@ -1001,10 +1017,12 @@ static int twl4030_bci_probe(struct platform_device *pdev)
dev_warn(&pdev->dev, "could not create sysfs file\n");
if (device_create_file(&bci->usb->dev, &dev_attr_mode))
dev_warn(&pdev->dev, "could not create sysfs file\n");
+ if (device_create_file(&bci->ac->dev, &dev_attr_mode))
+ dev_warn(&pdev->dev, "could not create sysfs file\n");
if (device_create_file(&bci->ac->dev, &dev_attr_max_current))
dev_warn(&pdev->dev, "could not create sysfs file\n");

- twl4030_charger_enable_ac(true);
+ twl4030_charger_enable_ac(bci, true);
if (!IS_ERR_OR_NULL(bci->transceiver))
twl4030_bci_usb_ncb(&bci->usb_nb,
bci->transceiver->last_event,
@@ -1024,13 +1042,14 @@ static int __exit twl4030_bci_remove(struct platform_device *pdev)
{
struct twl4030_bci *bci = platform_get_drvdata(pdev);

- twl4030_charger_enable_ac(false);
+ twl4030_charger_enable_ac(bci, false);
twl4030_charger_enable_usb(bci, false);
twl4030_charger_enable_backup(0, 0);

device_remove_file(&bci->usb->dev, &dev_attr_max_current);
device_remove_file(&bci->usb->dev, &dev_attr_mode);
device_remove_file(&bci->ac->dev, &dev_attr_max_current);
+ device_remove_file(&bci->ac->dev, &dev_attr_mode);
/* mask interrupts */
twl_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xff,
TWL4030_INTERRUPTS_BCIIMR1A);

2015-07-30 00:27:18

by NeilBrown

[permalink] [raw]
Subject: [PATCH 04/13] twl4030_charger: trust phy to determine when USB power is available.

The usb phy driver already determines when VBUS is available,
so repeating the test in the charger driver is pointless duplication.

On probe, process the last event from the phy, and from then on,
do whatever the phy tells us without double-checking.

Signed-off-by: NeilBrown <[email protected]>
---
drivers/power/twl4030_charger.c | 33 ++++++---------------------------
1 file changed, 6 insertions(+), 27 deletions(-)

diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index ffc123fb7158..a075216d65ed 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -178,28 +178,6 @@ static int twl4030_is_battery_present(struct twl4030_bci *bci)
}

/*
- * Check if VBUS power is present
- */
-static int twl4030_bci_have_vbus(struct twl4030_bci *bci)
-{
- int ret;
- u8 hwsts;
-
- ret = twl_i2c_read_u8(TWL_MODULE_PM_MASTER, &hwsts,
- TWL4030_PM_MASTER_STS_HW_CONDITIONS);
- if (ret < 0)
- return 0;
-
- dev_dbg(bci->dev, "check_vbus: HW_CONDITIONS %02x\n", hwsts);
-
- /* in case we also have STS_USB_ID, VBUS is driven by TWL itself */
- if ((hwsts & TWL4030_STS_VBUS) && !(hwsts & TWL4030_STS_USB_ID))
- return 1;
-
- return 0;
-}
-
-/*
* Enable/Disable USB Charge functionality.
*/
static int twl4030_charger_enable_usb(struct twl4030_bci *bci, bool enable)
@@ -207,10 +185,6 @@ static int twl4030_charger_enable_usb(struct twl4030_bci *bci, bool enable)
int ret;

if (enable && !IS_ERR_OR_NULL(bci->transceiver)) {
- /* Check for USB charger connected */
- if (!twl4030_bci_have_vbus(bci))
- return -ENODEV;
-
/*
* Until we can find out what current the device can provide,
* require a module param to enable USB charging.
@@ -662,7 +636,12 @@ static int twl4030_bci_probe(struct platform_device *pdev)
dev_warn(&pdev->dev, "failed to unmask interrupts: %d\n", ret);

twl4030_charger_enable_ac(true);
- twl4030_charger_enable_usb(bci, true);
+ if (!IS_ERR_OR_NULL(bci->transceiver))
+ twl4030_bci_usb_ncb(&bci->usb_nb,
+ bci->transceiver->last_event,
+ NULL);
+ else
+ twl4030_charger_enable_usb(bci, false);
if (pdata)
twl4030_charger_enable_backup(pdata->bb_uvolt,
pdata->bb_uamp);

2015-07-30 00:25:54

by NeilBrown

[permalink] [raw]
Subject: [PATCH 09/13] twl4030_charger: enable manual enable/disable of usb charging.

'off' or 'auto' to

/sys/class/power/twl4030_usb/mode

will now enable or disable charging from USB port. Normally this is
enabled on 'plug' and disabled on 'unplug'.
Unplug will still disable charging. 'plug' will only enable it if
'auto' if selected.

Acked-by: Pavel Machek <[email protected]>
Signed-off-by: NeilBrown <[email protected]>
---
.../ABI/testing/sysfs-class-power-twl4030 | 11 ++++
drivers/power/twl4030_charger.c | 59 ++++++++++++++++++++
2 files changed, 70 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-class-power-twl4030 b/Documentation/ABI/testing/sysfs-class-power-twl4030
index 0331bba4605d..40e0f919cbde 100644
--- a/Documentation/ABI/testing/sysfs-class-power-twl4030
+++ b/Documentation/ABI/testing/sysfs-class-power-twl4030
@@ -13,3 +13,14 @@ Description:
Value can the set by writing to the attribute.
The change will only persist until the next
plug event. These event are reported via udev.
+
+
+What: /sys/class/power_supply/twl4030_usb/mode
+Description:
+ Changing mode for USB port.
+ Writing to this can disable charging.
+
+ Possible values are:
+ "auto" - draw power as appropriate for detected
+ power source and battery status.
+ "off" - do not draw any power.
diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index b0a50adebfda..6fa928ed3128 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -109,10 +109,16 @@ struct twl4030_bci {
unsigned int ichg_eoc, ichg_lo, ichg_hi;
unsigned int usb_cur, ac_cur;
bool ac_is_active;
+ int usb_mode; /* charging mode requested */
+#define CHARGE_OFF 0
+#define CHARGE_AUTO 1

unsigned long event;
};

+/* strings for 'usb_mode' values */
+static char *modes[] = { "off", "auto" };
+
/*
* clear and set bits on an given register on a given module
*/
@@ -386,6 +392,8 @@ static int twl4030_charger_enable_usb(struct twl4030_bci *bci, bool enable)
{
int ret;

+ if (bci->usb_mode == CHARGE_OFF)
+ enable = false;
if (enable && !IS_ERR_OR_NULL(bci->transceiver)) {

twl4030_charger_update_current(bci);
@@ -629,6 +637,53 @@ static int twl4030_bci_usb_ncb(struct notifier_block *nb, unsigned long val,
return NOTIFY_OK;
}

+/*
+ * sysfs charger enabled store
+ */
+static ssize_t
+twl4030_bci_mode_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t n)
+{
+ struct twl4030_bci *bci = dev_get_drvdata(dev->parent);
+ int mode;
+ int status;
+
+ if (sysfs_streq(buf, modes[0]))
+ mode = 0;
+ else if (sysfs_streq(buf, modes[1]))
+ mode = 1;
+ else
+ return -EINVAL;
+ twl4030_charger_enable_usb(bci, false);
+ bci->usb_mode = mode;
+ status = twl4030_charger_enable_usb(bci, true);
+ return (status == 0) ? n : status;
+}
+
+/*
+ * sysfs charger enabled show
+ */
+static ssize_t
+twl4030_bci_mode_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct twl4030_bci *bci = dev_get_drvdata(dev->parent);
+ int len = 0;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(modes); i++)
+ if (bci->usb_mode == i)
+ len += snprintf(buf+len, PAGE_SIZE-len,
+ "[%s] ", modes[i]);
+ else
+ len += snprintf(buf+len, PAGE_SIZE-len,
+ "%s ", modes[i]);
+ buf[len-1] = '\n';
+ return len;
+}
+static DEVICE_ATTR(mode, 0644, twl4030_bci_mode_show,
+ twl4030_bci_mode_store);
+
static int twl4030_charger_get_current(void)
{
int curr;
@@ -815,6 +870,7 @@ static int twl4030_bci_probe(struct platform_device *pdev)
bci->usb_cur = 500000; /* 500mA */
else
bci->usb_cur = 100000; /* 100mA */
+ bci->usb_mode = CHARGE_AUTO;

bci->dev = &pdev->dev;
bci->irq_chg = platform_get_irq(pdev, 0);
@@ -898,6 +954,8 @@ static int twl4030_bci_probe(struct platform_device *pdev)
twl4030_charger_update_current(bci);
if (device_create_file(&bci->usb->dev, &dev_attr_max_current))
dev_warn(&pdev->dev, "could not create sysfs file\n");
+ if (device_create_file(&bci->usb->dev, &dev_attr_mode))
+ dev_warn(&pdev->dev, "could not create sysfs file\n");
if (device_create_file(&bci->ac->dev, &dev_attr_max_current))
dev_warn(&pdev->dev, "could not create sysfs file\n");

@@ -926,6 +984,7 @@ static int __exit twl4030_bci_remove(struct platform_device *pdev)
twl4030_charger_enable_backup(0, 0);

device_remove_file(&bci->usb->dev, &dev_attr_max_current);
+ device_remove_file(&bci->usb->dev, &dev_attr_mode);
device_remove_file(&bci->ac->dev, &dev_attr_max_current);
/* mask interrupts */
twl_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xff,

2015-07-30 00:25:45

by NeilBrown

[permalink] [raw]
Subject: [PATCH 10/13] twl4030_charger: add software controlled linear charging mode.


Add a 'continuous' option for usb charging which enables
the "linear" charging mode of the twl4030.

Linear charging does a good job with not-so-reliable power sources.
Auto mode does not work well as it switches off when voltage drops
momentarily. Care must be taken not to over-charge.

It was used with a bike hub dynamo since a year or so. In that case
there are automatically charging stops when the cyclist needs a break.

Original-by: Andreas Kemnade <[email protected]>
Signed-off-by: NeilBrown <[email protected]>
---
.../ABI/testing/sysfs-class-power-twl4030 | 9 +++
drivers/power/twl4030_charger.c | 55 ++++++++++++++++++--
2 files changed, 59 insertions(+), 5 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-class-power-twl4030 b/Documentation/ABI/testing/sysfs-class-power-twl4030
index 40e0f919cbde..e8baa36aaa2b 100644
--- a/Documentation/ABI/testing/sysfs-class-power-twl4030
+++ b/Documentation/ABI/testing/sysfs-class-power-twl4030
@@ -24,3 +24,12 @@ Description:
"auto" - draw power as appropriate for detected
power source and battery status.
"off" - do not draw any power.
+ "continuous"
+ - activate mode described as "linear" in
+ TWL data sheets. This uses whatever
+ current is available and doesn't switch off
+ when voltage drops.
+
+ This is useful for unstable power sources
+ such as bicycle dynamo, but care should
+ be taken that battery is not over-charged.
diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index 6fa928ed3128..de5430deaf23 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -24,6 +24,8 @@
#include <linux/usb/otg.h>
#include <linux/i2c/twl4030-madc.h>

+#define TWL4030_BCIMDEN 0x00
+#define TWL4030_BCIMDKEY 0x01
#define TWL4030_BCIMSTATEC 0x02
#define TWL4030_BCIICHG 0x08
#define TWL4030_BCIVAC 0x0a
@@ -35,13 +37,16 @@
#define TWL4030_BCIIREF1 0x27
#define TWL4030_BCIIREF2 0x28
#define TWL4030_BCIMFKEY 0x11
+#define TWL4030_BCIMFEN3 0x14
#define TWL4030_BCIMFTH8 0x1d
#define TWL4030_BCIMFTH9 0x1e
+#define TWL4030_BCIWDKEY 0x21

#define TWL4030_BCIMFSTS1 0x01

#define TWL4030_BCIAUTOWEN BIT(5)
#define TWL4030_CONFIG_DONE BIT(4)
+#define TWL4030_CVENAC BIT(2)
#define TWL4030_BCIAUTOUSB BIT(1)
#define TWL4030_BCIAUTOAC BIT(0)
#define TWL4030_CGAIN BIT(5)
@@ -112,12 +117,13 @@ struct twl4030_bci {
int usb_mode; /* charging mode requested */
#define CHARGE_OFF 0
#define CHARGE_AUTO 1
+#define CHARGE_LINEAR 2

unsigned long event;
};

/* strings for 'usb_mode' values */
-static char *modes[] = { "off", "auto" };
+static char *modes[] = { "off", "auto", "continuous" };

/*
* clear and set bits on an given register on a given module
@@ -404,16 +410,42 @@ static int twl4030_charger_enable_usb(struct twl4030_bci *bci, bool enable)
bci->usb_enabled = 1;
}

- /* forcing the field BCIAUTOUSB (BOOT_BCI[1]) to 1 */
- ret = twl4030_clear_set_boot_bci(0, TWL4030_BCIAUTOUSB);
- if (ret < 0)
- return ret;
+ if (bci->usb_mode == CHARGE_AUTO)
+ /* forcing the field BCIAUTOUSB (BOOT_BCI[1]) to 1 */
+ ret = twl4030_clear_set_boot_bci(0, TWL4030_BCIAUTOUSB);

/* forcing USBFASTMCHG(BCIMFSTS4[2]) to 1 */
ret = twl4030_clear_set(TWL_MODULE_MAIN_CHARGE, 0,
TWL4030_USBFASTMCHG, TWL4030_BCIMFSTS4);
+ if (bci->usb_mode == CHARGE_LINEAR) {
+ twl4030_clear_set_boot_bci(TWL4030_BCIAUTOAC|TWL4030_CVENAC, 0);
+ /* Watch dog key: WOVF acknowledge */
+ ret = twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE, 0x33,
+ TWL4030_BCIWDKEY);
+ /* 0x24 + EKEY6: off mode */
+ ret = twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE, 0x2a,
+ TWL4030_BCIMDKEY);
+ /* EKEY2: Linear charge: USB path */
+ ret = twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE, 0x26,
+ TWL4030_BCIMDKEY);
+ /* WDKEY5: stop watchdog count */
+ ret = twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE, 0xf3,
+ TWL4030_BCIWDKEY);
+ /* enable MFEN3 access */
+ ret = twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE, 0x9c,
+ TWL4030_BCIMFKEY);
+ /* ICHGEOCEN - end-of-charge monitor (current < 80mA)
+ * (charging continues)
+ * ICHGLOWEN - current level monitor (charge continues)
+ * don't monitor over-current or heat save
+ */
+ ret = twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE, 0xf0,
+ TWL4030_BCIMFEN3);
+ }
} else {
ret = twl4030_clear_set_boot_bci(TWL4030_BCIAUTOUSB, 0);
+ ret |= twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE, 0x2a,
+ TWL4030_BCIMDKEY);
if (bci->usb_enabled) {
pm_runtime_mark_last_busy(bci->transceiver->dev);
pm_runtime_put_autosuspend(bci->transceiver->dev);
@@ -652,6 +684,8 @@ twl4030_bci_mode_store(struct device *dev, struct device_attribute *attr,
mode = 0;
else if (sysfs_streq(buf, modes[1]))
mode = 1;
+ else if (sysfs_streq(buf, modes[2]))
+ mode = 2;
else
return -EINVAL;
twl4030_charger_enable_usb(bci, false);
@@ -750,6 +784,17 @@ static int twl4030_bci_get_property(struct power_supply *psy,
is_charging = state & TWL4030_MSTATEC_USB;
else
is_charging = state & TWL4030_MSTATEC_AC;
+ if (!is_charging) {
+ u8 s;
+ twl4030_bci_read(TWL4030_BCIMDEN, &s);
+ if (psy->desc->type == POWER_SUPPLY_TYPE_USB)
+ is_charging = s & 1;
+ else
+ is_charging = s & 2;
+ if (is_charging)
+ /* A little white lie */
+ state = TWL4030_MSTATEC_QUICK1;
+ }

switch (psp) {
case POWER_SUPPLY_PROP_STATUS:

2015-07-30 00:31:53

by NeilBrown

[permalink] [raw]
Subject: [PATCH 05/13] twl4030_charger: split uA calculation into a function.

We will need this calculation in other places, so
create functions to map between register value and uA value.

Acked-by: Pavel Machek <[email protected]>
Signed-off-by: NeilBrown <[email protected]>
---
drivers/power/twl4030_charger.c | 48 ++++++++++++++++++++++++++++-----------
1 file changed, 35 insertions(+), 13 deletions(-)

diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index a075216d65ed..29984b263a35 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -178,6 +178,40 @@ static int twl4030_is_battery_present(struct twl4030_bci *bci)
}

/*
+ * TI provided formulas:
+ * CGAIN == 0: ICHG = (BCIICHG * 1.7) / (2^10 - 1) - 0.85
+ * CGAIN == 1: ICHG = (BCIICHG * 3.4) / (2^10 - 1) - 1.7
+ * Here we use integer approximation of:
+ * CGAIN == 0: val * 1.6618 - 0.85 * 1000
+ * CGAIN == 1: (val * 1.6618 - 0.85 * 1000) * 2
+ */
+/*
+ * convert twl register value for currents into uA
+ */
+static int regval2ua(int regval, bool cgain)
+{
+ if (cgain)
+ return (regval * 16618 - 8500 * 1000) / 5;
+ else
+ return (regval * 16618 - 8500 * 1000) / 10;
+}
+
+/*
+ * convert uA currents into twl register value
+ */
+static int ua2regval(int ua, bool cgain)
+{
+ int ret;
+ if (cgain)
+ ua /= 2;
+ ret = (ua * 10 + 8500 * 1000) / 16618;
+ /* rounding problems */
+ if (ret < 512)
+ ret = 512;
+ return ret;
+}
+
+/*
* Enable/Disable USB Charge functionality.
*/
static int twl4030_charger_enable_usb(struct twl4030_bci *bci, bool enable)
@@ -366,14 +400,6 @@ static int twl4030_bci_usb_ncb(struct notifier_block *nb, unsigned long val,
return NOTIFY_OK;
}

-/*
- * TI provided formulas:
- * CGAIN == 0: ICHG = (BCIICHG * 1.7) / (2^10 - 1) - 0.85
- * CGAIN == 1: ICHG = (BCIICHG * 3.4) / (2^10 - 1) - 1.7
- * Here we use integer approximation of:
- * CGAIN == 0: val * 1.6618 - 0.85
- * CGAIN == 1: (val * 1.6618 - 0.85) * 2
- */
static int twl4030_charger_get_current(void)
{
int curr;
@@ -388,11 +414,7 @@ static int twl4030_charger_get_current(void)
if (ret)
return ret;

- ret = (curr * 16618 - 850 * 10000) / 10;
- if (bcictl1 & TWL4030_CGAIN)
- ret *= 2;
-
- return ret;
+ return regval2ua(curr, bcictl1 & TWL4030_CGAIN);
}

/*

2015-07-30 00:29:58

by NeilBrown

[permalink] [raw]
Subject: [PATCH 13/13] twl4030_charger: assume a 'charger' can supply maximum current.

If it cannot, we will stop pulling more current when voltage drops.

Signed-off-by: NeilBrown <[email protected]>
---
drivers/power/twl4030_charger.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index 2c537ee11bbe..c7432f532a83 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -697,8 +697,10 @@ static void twl4030_bci_usb_work(struct work_struct *data)
struct twl4030_bci *bci = container_of(data, struct twl4030_bci, work);

switch (bci->event) {
- case USB_EVENT_VBUS:
case USB_EVENT_CHARGER:
+ bci->usb_cur = USB_MAX_CURRENT;
+ /* FALL THROUGH */
+ case USB_EVENT_VBUS:
twl4030_charger_enable_usb(bci, true);
break;
case USB_EVENT_NONE:

2015-08-05 03:35:27

by Sebastian Reichel

[permalink] [raw]
Subject: Re: [PATCH 00/13] Enhance twl4030_charger functionality. - V3

Hi,

On Thu, Jul 30, 2015 at 10:11:24AM +1000, NeilBrown wrote:
> Following is most of my twl4030_charger patches, rebased against
> git://git.infradead.org/battery-2.6
>
> Since the previous set I have added the conversion to
> module_platform_driver so EPROBE_DEFER can be used, and fixed
> a few minor typos.
>
> This does not include the changes to add extcon support, in part
> because extcon has seen some changes lately which leave me even more
> confused about how best to use it than before.
> I need to sort that out before I can resolve the rest of my usb phy
> patches and then add a few more charger patches.

Thanks, queued.

-- Sebastian


Attachments:
(No filename) (656.00 B)
signature.asc (819.00 B)
Digital signature
Download all attachments

2015-08-07 03:11:23

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 07/13] twl4030_charger: distinguish between USB current and 'AC' current

* NeilBrown <[email protected]> [150729 17:28]:
> --- a/drivers/power/twl4030_charger.c
> +++ b/drivers/power/twl4030_charger.c
> static int twl4030_charger_update_current(struct twl4030_bci *bci)
> {
> int status;
> + int cur;
> unsigned reg, cur_reg;
> u8 bcictl1, oldreg, fullreg;
> bool cgain = false;
> u8 boot_bci;
>
> + /*
> + * If AC (Accessory Charger) voltage exceeds 4.5V (MADC 11)
> + * and AC is enabled, set current for 'ac'
> + */
> + if (twl4030_get_madc_conversion(11) > 4500) {
> + cur = bci->ac_cur;
> + bci->ac_is_active = true;
> + } else {
> + cur = bci->usb_cur;
> + bci->ac_is_active = false;
> + }
> +
> /* First, check thresholds and see if cgain is needed */
> if (bci->ichg_eoc >= 200000)
> cgain = true;

Neil, you need a stub or something for twl4030_get_madc_conversion
if madc is not selected. Now at least omap2plus_defconfig and
ARM allmodconfig fails in Linux next.

Regards,

Tony

2015-08-07 03:46:10

by NeilBrown

[permalink] [raw]
Subject: Re: [PATCH 07/13] twl4030_charger: distinguish between USB current and 'AC' current

On Thu, 6 Aug 2015 20:11:16 -0700 Tony Lindgren <[email protected]>
wrote:

> * NeilBrown <[email protected]> [150729 17:28]:
> > --- a/drivers/power/twl4030_charger.c
> > +++ b/drivers/power/twl4030_charger.c
> > static int twl4030_charger_update_current(struct twl4030_bci *bci)
> > {
> > int status;
> > + int cur;
> > unsigned reg, cur_reg;
> > u8 bcictl1, oldreg, fullreg;
> > bool cgain = false;
> > u8 boot_bci;
> >
> > + /*
> > + * If AC (Accessory Charger) voltage exceeds 4.5V (MADC 11)
> > + * and AC is enabled, set current for 'ac'
> > + */
> > + if (twl4030_get_madc_conversion(11) > 4500) {
> > + cur = bci->ac_cur;
> > + bci->ac_is_active = true;
> > + } else {
> > + cur = bci->usb_cur;
> > + bci->ac_is_active = false;
> > + }
> > +
> > /* First, check thresholds and see if cgain is needed */
> > if (bci->ichg_eoc >= 200000)
> > cgain = true;
>
> Neil, you need a stub or something for twl4030_get_madc_conversion
> if madc is not selected. Now at least omap2plus_defconfig and
> ARM allmodconfig fails in Linux next.
>
> Regards,
>
> Tony

Thanks, I did get notified about that by Fengguang's test robot, but
it's still on my list....

I guess making CHARGER_TWL4030 auto-select TWL4030_MADC would not be
acceptable? That would pull in IIO (it didn't use to...).

If this OK?

Thanks,
NeilBrown


From: NeilBrown <[email protected]>
Date: Fri, 7 Aug 2015 13:44:37 +1000
Subject: [PATCH] twl4030_charger: fix compile error when TWL4030_MADC not
available.

We can only use the madc to check for 'ac' availability
if the madc has been compiled in.
If not: assume always using USB.

Reported-by: Tony Lindgren <[email protected]>
Signed-off-by: NeilBrown <[email protected]>

diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index c7432f532a83..265fd236f4c0 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -91,6 +91,21 @@
#define TWL4030_MSTATEC_COMPLETE1 0x0b
#define TWL4030_MSTATEC_COMPLETE4 0x0e

+#if IS_ENABLED(CONFIG_TWL4030_MADC)
+/*
+ * If AC (Accessory Charger) voltage exceeds 4.5V (MADC 11)
+ * then AC is available.
+ */
+static inline int ac_available(void)
+{
+ return twl4030_get_madc_conversion(11) > 4500;
+}
+#else
+static inline int ac_available(void)
+{
+ return 0;
+}
+#endif
static bool allow_usb;
module_param(allow_usb, bool, 0644);
MODULE_PARM_DESC(allow_usb, "Allow USB charge drawing default current");
@@ -263,7 +278,7 @@ static int twl4030_charger_update_current(struct twl4030_bci *bci)
* If AC (Accessory Charger) voltage exceeds 4.5V (MADC 11)
* and AC is enabled, set current for 'ac'
*/
- if (twl4030_get_madc_conversion(11) > 4500) {
+ if (ac_available()) {
cur = bci->ac_cur;
bci->ac_is_active = true;
} else {

2015-08-07 04:21:25

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 07/13] twl4030_charger: distinguish between USB current and 'AC' current

* NeilBrown <[email protected]> [150806 20:48]:
>
> Thanks, I did get notified about that by Fengguang's test robot, but
> it's still on my list....
>
> I guess making CHARGER_TWL4030 auto-select TWL4030_MADC would not be
> acceptable? That would pull in IIO (it didn't use to...).
>
> If this OK?

Looks OK to me thanks:

Acked-by: Tony Lindgren <[email protected]>

2015-08-07 05:13:17

by Sebastian Reichel

[permalink] [raw]
Subject: Re: [PATCH 07/13] twl4030_charger: distinguish between USB current and 'AC' current

Hi,

This actually slipped through my review. IMHO madc should be
accessed through IIO, as already done for twl4030-madc-battery
and rx51-battery. That way the custom API can be removed at
some point.

Anyway, I queued the below patch with Tony's ACK to fix the build
issue in next.

On Fri, Aug 07, 2015 at 01:45:25PM +1000, NeilBrown wrote:
> From: NeilBrown <[email protected]>
> Date: Fri, 7 Aug 2015 13:44:37 +1000
> Subject: [PATCH] twl4030_charger: fix compile error when TWL4030_MADC not
> available.
>
> We can only use the madc to check for 'ac' availability
> if the madc has been compiled in.
> If not: assume always using USB.
>
> Reported-by: Tony Lindgren <[email protected]>
> Signed-off-by: NeilBrown <[email protected]>
>
> diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
> index c7432f532a83..265fd236f4c0 100644
> --- a/drivers/power/twl4030_charger.c
> +++ b/drivers/power/twl4030_charger.c
> @@ -91,6 +91,21 @@
> #define TWL4030_MSTATEC_COMPLETE1 0x0b
> #define TWL4030_MSTATEC_COMPLETE4 0x0e
>
> +#if IS_ENABLED(CONFIG_TWL4030_MADC)
> +/*
> + * If AC (Accessory Charger) voltage exceeds 4.5V (MADC 11)
> + * then AC is available.
> + */
> +static inline int ac_available(void)
> +{
> + return twl4030_get_madc_conversion(11) > 4500;
> +}
> +#else
> +static inline int ac_available(void)
> +{
> + return 0;
> +}
> +#endif
> static bool allow_usb;
> module_param(allow_usb, bool, 0644);
> MODULE_PARM_DESC(allow_usb, "Allow USB charge drawing default current");
> @@ -263,7 +278,7 @@ static int twl4030_charger_update_current(struct twl4030_bci *bci)
> * If AC (Accessory Charger) voltage exceeds 4.5V (MADC 11)
> * and AC is enabled, set current for 'ac'
> */
> - if (twl4030_get_madc_conversion(11) > 4500) {
> + if (ac_available()) {
> cur = bci->ac_cur;
> bci->ac_is_active = true;
> } else {

-- Sebastian


Attachments:
(No filename) (1.84 kB)
signature.asc (819.00 B)
Digital signature
Download all attachments

2015-08-07 05:29:42

by NeilBrown

[permalink] [raw]
Subject: Re: [PATCH 07/13] twl4030_charger: distinguish between USB current and 'AC' current

On Fri, 7 Aug 2015 07:13:09 +0200 Sebastian Reichel <[email protected]>
wrote:

> Hi,
>
> This actually slipped through my review. IMHO madc should be
> accessed through IIO, as already done for twl4030-madc-battery
> and rx51-battery. That way the custom API can be removed at
> some point.
>
> Anyway, I queued the below patch with Tony's ACK to fix the build
> issue in next.
>

OK, thanks.

I'll try to figure out are more proper approach ... might be a week or
so though.

Thanks,
NeilBrown


Attachments:
(No filename) (819.00 B)
OpenPGP digital signature

2015-08-18 08:08:09

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 03/13] twl4030_charger: correctly handle -EPROBE_DEFER from devm_usb_get_phy_by_node

* NeilBrown <[email protected]> [150729 17:29]:
> Now that twl4030_bci_probe can safely return -EPROBE_DEFER,
> do so when devm_usb_get_phy_by_node returns that error.
>
> Signed-off-by: NeilBrown <[email protected]>
> ---
> drivers/power/twl4030_charger.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
> index 045238370d3f..ffc123fb7158 100644
> --- a/drivers/power/twl4030_charger.c
> +++ b/drivers/power/twl4030_charger.c
> @@ -636,9 +636,13 @@ static int twl4030_bci_probe(struct platform_device *pdev)
>
> phynode = of_find_compatible_node(bci->dev->of_node->parent,
> NULL, "ti,twl4030-usb");
> - if (phynode)
> + if (phynode) {
> bci->transceiver = devm_usb_get_phy_by_node(
> bci->dev, phynode, &bci->usb_nb);
> + if (IS_ERR(bci->transceiver) &&
> + PTR_ERR(bci->transceiver) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> + }
> }

Neil, the return with -EPROBE_DEFER here causes flakeyness booting
for me somehow at least on my logicpd-torpedo-37xx-devkit using
omap2plus_defconfig.

It seems that the twl4030_bci_probe keeps looping or something about
1/3 of the boots and that probably prevents the other twl modules
from loading? Reverting this patch alone seems to fix the issue.

I don't think I have a battery wired on this board the USB is wired
the same way as on beagle-xm. So I'd assume also flakeyness on beagle
xm with this patch.

Regards,

Tony

2015-08-19 00:29:28

by NeilBrown

[permalink] [raw]
Subject: Re: [PATCH 03/13] twl4030_charger: correctly handle -EPROBE_DEFER from devm_usb_get_phy_by_node

On Tue, 18 Aug 2015 01:07:58 -0700 Tony Lindgren <[email protected]>
wrote:

> * NeilBrown <[email protected]> [150729 17:29]:
> > Now that twl4030_bci_probe can safely return -EPROBE_DEFER,
> > do so when devm_usb_get_phy_by_node returns that error.
> >
> > Signed-off-by: NeilBrown <[email protected]>
> > ---
> > drivers/power/twl4030_charger.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
> > index 045238370d3f..ffc123fb7158 100644
> > --- a/drivers/power/twl4030_charger.c
> > +++ b/drivers/power/twl4030_charger.c
> > @@ -636,9 +636,13 @@ static int twl4030_bci_probe(struct platform_device *pdev)
> >
> > phynode = of_find_compatible_node(bci->dev->of_node->parent,
> > NULL, "ti,twl4030-usb");
> > - if (phynode)
> > + if (phynode) {
> > bci->transceiver = devm_usb_get_phy_by_node(
> > bci->dev, phynode, &bci->usb_nb);
> > + if (IS_ERR(bci->transceiver) &&
> > + PTR_ERR(bci->transceiver) == -EPROBE_DEFER)
> > + return -EPROBE_DEFER;
> > + }
> > }
>
> Neil, the return with -EPROBE_DEFER here causes flakeyness booting
> for me somehow at least on my logicpd-torpedo-37xx-devkit using
> omap2plus_defconfig.
>
> It seems that the twl4030_bci_probe keeps looping or something about
> 1/3 of the boots and that probably prevents the other twl modules
> from loading? Reverting this patch alone seems to fix the issue.
>
> I don't think I have a battery wired on this board the USB is wired
> the same way as on beagle-xm. So I'd assume also flakeyness on beagle
> xm with this patch.
>
> Regards,
>
> Tony

What dts file are you using?
I'm guessing that it doesn't have something like:

&usb_otg_hs {
interface-type = <0>;
usb-phy = <&usb2_phy>;
phys = <&usb2_phy>;
phy-names = "usb2-phy";
mode = <3>;
power = <50>;
};

? i.e. with a usb-phy=<&usb2_phy> ?

What if you add

&usb2_phy {
status = "disabled";
}

to your dts file?

Should the 'status' be disabled in twl4030.dtsi, and then marked OK in
any dts that uses it? I'm not at all clear on how 'status' is meant to
be used.

Thanks,
NeilBrown

2015-08-19 06:25:49

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 03/13] twl4030_charger: correctly handle -EPROBE_DEFER from devm_usb_get_phy_by_node

* NeilBrown <[email protected]> [150818 17:32]:
> On Tue, 18 Aug 2015 01:07:58 -0700 Tony Lindgren <[email protected]>
> wrote:
>
> > * NeilBrown <[email protected]> [150729 17:29]:
> > > Now that twl4030_bci_probe can safely return -EPROBE_DEFER,
> > > do so when devm_usb_get_phy_by_node returns that error.
> > >
> > > Signed-off-by: NeilBrown <[email protected]>
> > > ---
> > > drivers/power/twl4030_charger.c | 6 +++++-
> > > 1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
> > > index 045238370d3f..ffc123fb7158 100644
> > > --- a/drivers/power/twl4030_charger.c
> > > +++ b/drivers/power/twl4030_charger.c
> > > @@ -636,9 +636,13 @@ static int twl4030_bci_probe(struct platform_device *pdev)
> > >
> > > phynode = of_find_compatible_node(bci->dev->of_node->parent,
> > > NULL, "ti,twl4030-usb");
> > > - if (phynode)
> > > + if (phynode) {
> > > bci->transceiver = devm_usb_get_phy_by_node(
> > > bci->dev, phynode, &bci->usb_nb);
> > > + if (IS_ERR(bci->transceiver) &&
> > > + PTR_ERR(bci->transceiver) == -EPROBE_DEFER)
> > > + return -EPROBE_DEFER;
> > > + }
> > > }
> >
> > Neil, the return with -EPROBE_DEFER here causes flakeyness booting
> > for me somehow at least on my logicpd-torpedo-37xx-devkit using
> > omap2plus_defconfig.
> >
> > It seems that the twl4030_bci_probe keeps looping or something about
> > 1/3 of the boots and that probably prevents the other twl modules
> > from loading? Reverting this patch alone seems to fix the issue.
> >
> > I don't think I have a battery wired on this board the USB is wired
> > the same way as on beagle-xm. So I'd assume also flakeyness on beagle
> > xm with this patch.
>
> What dts file are you using?
> I'm guessing that it doesn't have something like:
>
> &usb_otg_hs {
> interface-type = <0>;
> usb-phy = <&usb2_phy>;
> phys = <&usb2_phy>;
> phy-names = "usb2-phy";
> mode = <3>;
> power = <50>;
> };

It's the logicpd-torpedo-37xx-devkit.dts like I mentioned above,
and yes it has the same entry above like beagle xm.

> ? i.e. with a usb-phy=<&usb2_phy> ?
>
> What if you add
>
> &usb2_phy {
> status = "disabled";
> }
>
> to your dts file?

Yes then the PHY driver won't get probed and modprobe of the
charger won't hang with -EPROBE_DEFER.. The USB is working on this
one like on beagle xm so that's not a solution.

> Should the 'status' be disabled in twl4030.dtsi, and then marked OK in
> any dts that uses it? I'm not at all clear on how 'status' is meant to
> be used.

Well status = "disabled" makes kernel completely ignore the
hardware, so the struct device is never created. I'd stay away from
using that in general for proper runtime idling of devices.. The
hardware is there for sure :)

Can you check if something like the the following allows you to
reproduce the hang of modprobe twl4030_charger:

# rmmod phy-twl4030-usb
# modprobe twl4030_charger

So just don't probe phy-twl4030-usb before twl4030_charger and
then modprobe of twl4030_charger hangs?

That is with proper usb2_phy configured in .dts file and not
set to status = "disabled".

Regards,

Tony

2015-08-27 20:51:11

by Kevin Hilman

[permalink] [raw]
Subject: Re: [PATCH 03/13] twl4030_charger: correctly handle -EPROBE_DEFER from devm_usb_get_phy_by_node

On Wed, Jul 29, 2015 at 5:11 PM, NeilBrown <[email protected]> wrote:
> Now that twl4030_bci_probe can safely return -EPROBE_DEFER,
> do so when devm_usb_get_phy_by_node returns that error.
>
> Signed-off-by: NeilBrown <[email protected]>

This patch has hit linux-next in the form of coommit 3fc3895e4fe1
(twl4030_charger: correctly handle -EPROBE_DEFER from
devm_usb_get_phy_by_node) and kernelci.org found a regression on
omap3-beagle-xm[1]. Bisecting[2] this boot failure pointed at this
commit, and I verified that reverting it on top of next-20150827 gets
the board booting again. I haven't debugged any further.

Kevin

[1] http://storage.kernelci.org/next/next-20150827/arm-omap2plus_defconfig/lab-khilman/boot-omap3-beagle-xm.html
[2] https://ci.linaro.org/view/people/job/tbaker-boot-bisect-bot/88/console