Hi,
the following 4 patches make some improvements to the twl4030
phy. Together with some other patches I have for twl4030_charger,
they allow for better automatic control of charging.
In particular, the status of the ID pin is assessed and the
result in communicated to the charger drivers.
There is also support for conveying the max current negotiated by a
host to the charger.
Comments most welcome.
thanks,
NeilBrown
---
NeilBrown (4):
usb: phy: twl4030: make runtime pm more reliable.
usb: phy: twl4030: allow charger to see usb current draw limits.
usb: phy: twl4030: add support for reading restore on ID pin.
usb: phy: twl4030: test ID resistance to see if charger is present.
drivers/phy/phy-twl4030-usb.c | 138 +++++++++++++++++++++++++++++++++++++----
1 file changed, 125 insertions(+), 13 deletions(-)
--
Signature
A construct like:
if (pm_runtime_suspended(twl->dev))
pm_runtime_get_sync(twl->dev);
is against the spirit of the runtime_pm interface as it
makes the internal refcounting useless.
In this case it is also racy, particularly as 'put_autosuspend'
is use to drop a reference.
When that happens a timer is started and the device is
runtime-suspended after the timeout.
If the above code runs in this window, the device will not be
found to be suspended so no pm_runtime reference is taken.
When the timer expires the device will be suspended, which is
against the intention of the code.
So be more direct is taking and dropping references.
If twl->linkstat is VBUS_VALID or ID_GROUND, then hold a
pm_runtime reference, otherwise don't.
Signed-off-by: NeilBrown <[email protected]>
---
drivers/phy/phy-twl4030-usb.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/phy/phy-twl4030-usb.c b/drivers/phy/phy-twl4030-usb.c
index 8e87f54671f3..97c59074233f 100644
--- a/drivers/phy/phy-twl4030-usb.c
+++ b/drivers/phy/phy-twl4030-usb.c
@@ -536,8 +536,13 @@ static irqreturn_t twl4030_usb_irq(int irq, void *_twl)
mutex_lock(&twl->lock);
if (status >= 0 && status != twl->linkstat) {
+ status_changed =
+ (twl->linkstat == OMAP_MUSB_VBUS_VALID ||
+ twl->linkstat == OMAP_MUSB_ID_GROUND)
+ !=
+ (status == OMAP_MUSB_VBUS_VALID ||
+ status == OMAP_MUSB_ID_GROUND);
twl->linkstat = status;
- status_changed = true;
}
mutex_unlock(&twl->lock);
@@ -555,13 +560,10 @@ static irqreturn_t twl4030_usb_irq(int irq, void *_twl)
*/
if ((status == OMAP_MUSB_VBUS_VALID) ||
(status == OMAP_MUSB_ID_GROUND)) {
- if (pm_runtime_suspended(twl->dev))
- pm_runtime_get_sync(twl->dev);
+ pm_runtime_get_sync(twl->dev);
} else {
- if (pm_runtime_active(twl->dev)) {
- pm_runtime_mark_last_busy(twl->dev);
- pm_runtime_put_autosuspend(twl->dev);
- }
+ pm_runtime_mark_last_busy(twl->dev);
+ pm_runtime_put_autosuspend(twl->dev);
}
omap_musb_mailbox(status);
}
@@ -768,6 +770,10 @@ static int twl4030_usb_remove(struct platform_device *pdev)
/* disable complete OTG block */
twl4030_usb_clear_bits(twl, POWER_CTRL, POWER_CTRL_OTG_ENAB);
+
+ if (twl->linkstat == OMAP_MUSB_VBUS_VALID ||
+ twl->linkstat == OMAP_MUSB_ID_GROUND)
+ pm_runtime_put_noidle(twl->dev);
pm_runtime_mark_last_busy(twl->dev);
pm_runtime_put(twl->dev);
The twl4030 phy can measure, with low precision, the
resistance-to-ground of the ID pin.
Add a function to read the value, and export the result
via sysfs.
If the read fails, which it does sometimes, try again in 50msec.
Signed-off-by: NeilBrown <[email protected]>
---
drivers/phy/phy-twl4030-usb.c | 63 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 63 insertions(+)
diff --git a/drivers/phy/phy-twl4030-usb.c b/drivers/phy/phy-twl4030-usb.c
index 023fe150c7a1..759950898df9 100644
--- a/drivers/phy/phy-twl4030-usb.c
+++ b/drivers/phy/phy-twl4030-usb.c
@@ -374,6 +374,56 @@ static void twl4030_i2c_access(struct twl4030_usb *twl, int on)
}
}
+enum twl4030_id_status {
+ TWL4030_GROUND,
+ TWL4030_102K,
+ TWL4030_200K,
+ TWL4030_440K,
+ TWL4030_FLOATING,
+ TWL4030_ID_UNKNOWN,
+};
+static char *twl4030_id_names[] = {
+ "ground",
+ "102k",
+ "200k",
+ "440k",
+ "floating",
+ "unknown"
+};
+
+enum twl4030_id_status twl4030_get_id(struct twl4030_usb *twl)
+{
+ int ret;
+
+ pm_runtime_get_sync(twl->dev);
+ if (twl->usb_mode == T2_USB_MODE_ULPI)
+ twl4030_i2c_access(twl, 1);
+ ret = twl4030_usb_read(twl, ULPI_OTG_CTRL);
+ if (ret < 0 || !(ret & ULPI_OTG_ID_PULLUP)) {
+ /* Need pull-up to read ID */
+ twl4030_usb_set_bits(twl, ULPI_OTG_CTRL,
+ ULPI_OTG_ID_PULLUP);
+ mdelay(50);
+ }
+ ret = twl4030_usb_read(twl, ID_STATUS);
+ if (ret < 0 || (ret & 0x1f) == 0) {
+ mdelay(50);
+ ret = twl4030_usb_read(twl, ID_STATUS);
+ }
+
+ if (twl->usb_mode == T2_USB_MODE_ULPI)
+ twl4030_i2c_access(twl, 0);
+ pm_runtime_put_autosuspend(twl->dev);
+
+ if (ret < 0)
+ return TWL4030_ID_UNKNOWN;
+ ret = ffs(ret) - 1;
+ if (ret < TWL4030_GROUND || ret > TWL4030_FLOATING)
+ return TWL4030_ID_UNKNOWN;
+
+ return ret;
+}
+
static void __twl4030_phy_power(struct twl4030_usb *twl, int on)
{
u8 pwr = twl4030_usb_read(twl, PHY_PWR_CTRL);
@@ -531,6 +581,16 @@ static ssize_t twl4030_usb_vbus_show(struct device *dev,
}
static DEVICE_ATTR(vbus, 0444, twl4030_usb_vbus_show, NULL);
+static ssize_t twl4030_usb_id_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct twl4030_usb *twl = dev_get_drvdata(dev);
+ return scnprintf(buf, PAGE_SIZE, "%s\n",
+ twl4030_id_names[twl4030_get_id(twl)]);
+}
+static DEVICE_ATTR(id, 0444, twl4030_usb_id_show, NULL);
+
static irqreturn_t twl4030_usb_irq(int irq, void *_twl)
{
struct twl4030_usb *twl = _twl;
@@ -723,6 +783,8 @@ static int twl4030_usb_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, twl);
if (device_create_file(&pdev->dev, &dev_attr_vbus))
dev_warn(&pdev->dev, "could not create sysfs file\n");
+ if (device_create_file(&pdev->dev, &dev_attr_id))
+ dev_warn(&pdev->dev, "could not create sysfs file\n");
ATOMIC_INIT_NOTIFIER_HEAD(&twl->phy.notifier);
@@ -768,6 +830,7 @@ static int twl4030_usb_remove(struct platform_device *pdev)
pm_runtime_get_sync(twl->dev);
cancel_delayed_work(&twl->id_workaround_work);
device_remove_file(twl->dev, &dev_attr_vbus);
+ device_remove_file(twl->dev, &dev_attr_id);
/* set transceiver mode to power on defaults */
twl4030_usb_set_mode(twl, -1);
The charger needs to know when a USB gadget has been enumerated
and what the agreed maximum current was so that it can adjust
charging accordingly.
So define a "set_power()" function to record the permitted
draw, and pass a pointer to that when sending USB_EVENT_ENUMERATED
notification.
Signed-off-by: NeilBrown <[email protected]>
---
drivers/phy/phy-twl4030-usb.c | 27 +++++++++++++++++++++------
1 file changed, 21 insertions(+), 6 deletions(-)
diff --git a/drivers/phy/phy-twl4030-usb.c b/drivers/phy/phy-twl4030-usb.c
index 97c59074233f..023fe150c7a1 100644
--- a/drivers/phy/phy-twl4030-usb.c
+++ b/drivers/phy/phy-twl4030-usb.c
@@ -163,6 +163,11 @@ struct twl4030_usb {
enum omap_musb_vbus_id_status linkstat;
bool vbus_supplied;
+ /* Permitted vbus draw - only meaningful after
+ * USB_EVENT_ENUMERATED
+ */
+ unsigned vbus_draw;
+
struct delayed_work id_workaround_work;
};
@@ -547,12 +552,7 @@ static irqreturn_t twl4030_usb_irq(int irq, void *_twl)
mutex_unlock(&twl->lock);
if (status_changed) {
- /* FIXME add a set_power() method so that B-devices can
- * configure the charger appropriately. It's not always
- * correct to consume VBUS power, and how much current to
- * consume is a function of the USB configuration chosen
- * by the host.
- *
+ /*
* REVISIT usb_gadget_vbus_connect(...) as needed, ditto
* its disconnect() sibling, when changing to/from the
* USB_LINK_VBUS state. musb_hdrc won't care until it
@@ -625,6 +625,20 @@ static int twl4030_set_host(struct usb_otg *otg, struct usb_bus *host)
return 0;
}
+static int twl4030_set_power(struct usb_phy *phy, unsigned mA)
+{
+ struct twl4030_usb *twl = phy_to_twl(phy);
+
+ if (twl->vbus_draw != mA) {
+ phy->last_event = USB_EVENT_ENUMERATED;
+ twl->vbus_draw = mA;
+ atomic_notifier_call_chain(&phy->notifier,
+ USB_EVENT_ENUMERATED,
+ &twl->vbus_draw);
+ }
+ return 0;
+}
+
static const struct phy_ops ops = {
.init = twl4030_phy_init,
.power_on = twl4030_phy_power_on,
@@ -675,6 +689,7 @@ static int twl4030_usb_probe(struct platform_device *pdev)
twl->phy.label = "twl4030";
twl->phy.otg = otg;
twl->phy.type = USB_PHY_TYPE_USB2;
+ twl->phy.set_power = twl4030_set_power;
otg->usb_phy = &twl->phy;
otg->set_host = twl4030_set_host;
If an 'A' plug is inserted, ID should be pulled to ground.
If a 'B' plug, then ID should be floating.
If an Accessory Charger Adapter is inserted, then ID will
be neither grounded nor floating. In this case tell the
USB subsystem that it is an A plug, and the battery
charging subsystem that it is a charger.
Fortunately, this will treat the Openmoko charger (and
other similar chargers) as a charger.
Signed-off-by: NeilBrown <[email protected]>
---
drivers/phy/phy-twl4030-usb.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/drivers/phy/phy-twl4030-usb.c b/drivers/phy/phy-twl4030-usb.c
index 759950898df9..8a43080cdbd7 100644
--- a/drivers/phy/phy-twl4030-usb.c
+++ b/drivers/phy/phy-twl4030-usb.c
@@ -596,9 +596,31 @@ static irqreturn_t twl4030_usb_irq(int irq, void *_twl)
struct twl4030_usb *twl = _twl;
enum omap_musb_vbus_id_status status;
bool status_changed = false;
+ bool found_charger = false;
status = twl4030_usb_linkstat(twl);
+ if (status == OMAP_MUSB_ID_GROUND ||
+ status == OMAP_MUSB_VBUS_VALID) {
+ /* We should check the resistance on the ID pin.
+ * If not a Ground or Floating, then this is
+ * likely a charger
+ */
+ enum twl4030_id_status sts = twl4030_get_id(twl);
+ if (sts > TWL4030_GROUND &&
+ sts < TWL4030_FLOATING) {
+ /*
+ * This might be a charger, or an
+ * Accessory Charger Adapter.
+ * In either case we can charge, and it
+ * makes sense to tell the USB system
+ * that we might be acting as a HOST.
+ */
+ status = OMAP_MUSB_ID_GROUND;
+ found_charger = true;
+ }
+ }
+
mutex_lock(&twl->lock);
if (status >= 0 && status != twl->linkstat) {
status_changed =
@@ -627,6 +649,12 @@ static irqreturn_t twl4030_usb_irq(int irq, void *_twl)
}
omap_musb_mailbox(status);
}
+ if (found_charger && twl->phy.last_event != USB_EVENT_CHARGER) {
+ twl->phy.last_event = USB_EVENT_CHARGER;
+ atomic_notifier_call_chain(&twl->phy.notifier,
+ USB_EVENT_CHARGER,
+ NULL);
+ }
/* don't schedule during sleep - irq works right then */
if (status == OMAP_MUSB_ID_GROUND && pm_runtime_active(twl->dev)) {
* NeilBrown <[email protected]> [150223 19:45]:
> A construct like:
>
> if (pm_runtime_suspended(twl->dev))
> pm_runtime_get_sync(twl->dev);
>
> is against the spirit of the runtime_pm interface as it
> makes the internal refcounting useless.
>
> In this case it is also racy, particularly as 'put_autosuspend'
> is use to drop a reference.
> When that happens a timer is started and the device is
> runtime-suspended after the timeout.
> If the above code runs in this window, the device will not be
> found to be suspended so no pm_runtime reference is taken.
> When the timer expires the device will be suspended, which is
> against the intention of the code.
>
> So be more direct is taking and dropping references.
> If twl->linkstat is VBUS_VALID or ID_GROUND, then hold a
> pm_runtime reference, otherwise don't.
Looks good to me, thanks for fixing it. I've tested this series
on beagleboard-xm by plugging and unplugging devices and
switching between host and peripheral mode, things still
idle properly for off-idle. So please feel free to add:
Tested-by: Tony Lindgren <[email protected]>
> Signed-off-by: NeilBrown <[email protected]>
> ---
> drivers/phy/phy-twl4030-usb.c | 20 +++++++++++++-------
> 1 file changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/phy/phy-twl4030-usb.c b/drivers/phy/phy-twl4030-usb.c
> index 8e87f54671f3..97c59074233f 100644
> --- a/drivers/phy/phy-twl4030-usb.c
> +++ b/drivers/phy/phy-twl4030-usb.c
> @@ -536,8 +536,13 @@ static irqreturn_t twl4030_usb_irq(int irq, void *_twl)
>
> mutex_lock(&twl->lock);
> if (status >= 0 && status != twl->linkstat) {
> + status_changed =
> + (twl->linkstat == OMAP_MUSB_VBUS_VALID ||
> + twl->linkstat == OMAP_MUSB_ID_GROUND)
> + !=
> + (status == OMAP_MUSB_VBUS_VALID ||
> + status == OMAP_MUSB_ID_GROUND);
> twl->linkstat = status;
> - status_changed = true;
> }
> mutex_unlock(&twl->lock);
>
> @@ -555,13 +560,10 @@ static irqreturn_t twl4030_usb_irq(int irq, void *_twl)
> */
> if ((status == OMAP_MUSB_VBUS_VALID) ||
> (status == OMAP_MUSB_ID_GROUND)) {
> - if (pm_runtime_suspended(twl->dev))
> - pm_runtime_get_sync(twl->dev);
> + pm_runtime_get_sync(twl->dev);
> } else {
> - if (pm_runtime_active(twl->dev)) {
> - pm_runtime_mark_last_busy(twl->dev);
> - pm_runtime_put_autosuspend(twl->dev);
> - }
> + pm_runtime_mark_last_busy(twl->dev);
> + pm_runtime_put_autosuspend(twl->dev);
> }
> omap_musb_mailbox(status);
> }
> @@ -768,6 +770,10 @@ static int twl4030_usb_remove(struct platform_device *pdev)
>
> /* disable complete OTG block */
> twl4030_usb_clear_bits(twl, POWER_CTRL, POWER_CTRL_OTG_ENAB);
> +
> + if (twl->linkstat == OMAP_MUSB_VBUS_VALID ||
> + twl->linkstat == OMAP_MUSB_ID_GROUND)
> + pm_runtime_put_noidle(twl->dev);
> pm_runtime_mark_last_busy(twl->dev);
> pm_runtime_put(twl->dev);
>
>
>
* NeilBrown <[email protected]> [150223 19:45]:
> Hi,
> the following 4 patches make some improvements to the twl4030
> phy. Together with some other patches I have for twl4030_charger,
> they allow for better automatic control of charging.
>
> In particular, the status of the ID pin is assessed and the
> result in communicated to the charger drivers.
> There is also support for conveying the max current negotiated by a
> host to the charger.
>
> Comments most welcome.
Looks good to me, for the whole series, please feel free to add:
Tested-by: Tony Lindgren <[email protected]>