twl4030_charger seems to be broken by switch from 'notifier' to 'mailbox'.

DISCUSSION NeilBrown 2 participants 3 messages Dec 14, 2012 – Dec 15, 2012 USB

lore

Share & tools

lore

NeilBrown

author

2012-12-14 09:04 UTC #0 lore raw

Hi,

this commit:

commit c9721438c009adf8e81d376839ed037c53b9b8d9

Author: Kishon Vijay Abraham I <[email protected]>
Date:   Fri Jun 22 17:40:52 2012 +0530

usb: musb: twl: use mailbox API to send VBUS or ID events

The atomic notifier from twl4030/twl6030 to notifiy VBUS and ID events, is replaced by a direct call to omap musb blue.

Signed-off-by Kishon Vijay Abraham I <[email protected]>

Signed-off-by Felipe Balbi <[email protected]>

Seems to have broken twl4030_charger.c.
This code relies on the notifier to know when the USB has been plugged in and so it should consider enabling the charger.

now that the notification has gone, the charger never turns on.

If I put the atomic_notifier_call_chain() calls back in, then the charger is activated as appropriate.

What is the plan here? Should the calls go back, or is there some way for twl4030_charger to hook into the "mailbox"? Or is there some other way it should work?

Thanks,
NeilBrown

Felipe Balbi

2012-12-14 09:44 UTC #1 lore raw

Hi,

Quoted 19 lines · NeilBrown

this commit:
commit c9721438c009adf8e81d376839ed037c53b9b8d9
Author: Kishon Vijay Abraham I <[email protected]>
Date:   Fri Jun 22 17:40:52 2012 +0530
usb: musb: twl: use mailbox API to send VBUS or ID events
The atomic notifier from twl4030/twl6030 to notifiy VBUS and ID events, is replaced by a direct call to omap musb blue.
Signed-off-by Kishon Vijay Abraham I <[email protected]>
Signed-off-by Felipe Balbi <[email protected]>
Seems to have broken twl4030_charger.c.
This code relies on the notifier to know when the USB has been plugged in and so it should consider enabling the charger.

Quoted 8 lines

now that the notification has gone, the charger never turns on.
If I put the atomic_notifier_call_chain() calls back in, then the charger is activated as appropriate.
What is the plan here? Should the calls go back, or is there some way for twl4030_charger to hook into the "mailbox"? Or is there some other way it should work?

hmmm, tricky situation. Maybe we add back the notifer calls but keep the mailbox API ?

Signature · 1 line

balbi

NeilBrown

author

2012-12-15 00:17 UTC #2 lore raw

Quoted 27 lines · On Fri, 14 Dec 2012 11:36:59 +0200 Felipe Balbi <[email protected]> wrote:

Hi,
Quoted 24 lines · NeilBrown
this commit:
commit c9721438c009adf8e81d376839ed037c53b9b8d9
Author: Kishon Vijay Abraham I <[email protected]>
Date:   Fri Jun 22 17:40:52 2012 +0530
usb: musb: twl: use mailbox API to send VBUS or ID events
The atomic notifier from twl4030/twl6030 to notifiy VBUS and ID events, is replaced by a direct call to omap musb blue.
Signed-off-by Kishon Vijay Abraham I <[email protected]>
Signed-off-by Felipe Balbi <[email protected]>
Seems to have broken twl4030_charger.c.
This code relies on the notifier to know when the USB has been plugged in and so it should consider enabling the charger.
now that the notification has gone, the charger never turns on.
If I put the atomic_notifier_call_chain() calls back in, then the charger is activated as appropriate.

Quoted 7 lines

Quoted 3 lines
What is the plan here? Should the calls go back, or is there some way for twl4030_charger to hook into the "mailbox"? Or is there some other way it should work?
hmmm, tricky situation. Maybe we add back the notifer calls but keep the mailbox API ?

Maybe. This is my current patch. I'm not entirely confident about the mapping from omap_musb_vbus_id_status to usb_phy_events.

Thanks,
NeilBrown

From ffafe1fe4da3c848631c082f70745017c03e23bf Mon Sep 17 00:00:00 2001
From: NeilBrown <[email protected]>
Date: Sat, 15 Dec 2012 10:35:37 +1100
Subject: [PATCH] twl4030-usb - re-instate event notifications

commit c9721438c009adf8e81d376839ed037c53b9b8d9
usb: musb: twl: use mailbox API to send VBUS or ID events

went a bit too far in removing all event notification from twl4030-usb. The notifications are still needed for twl4030_charger.c to know when to enable the charger.

Signed-off-by NeilBrown <[email protected]>

drivers/usb/otg/twl4030-usb.c +27 -0

index f0d2e75..15733f9 100644
@@ -38,6 +38,7 @@
 #include <linux/i2c/twl.h>
 #include <linux/regulator/consumer.h>
 #include <linux/err.h>
+#include <linux/notifier.h>
 #include <linux/slab.h>
 
 /* Register defines */

@@ -388,6 +389,28 @@ static void twl4030_phy_power(struct twl4030_usb *twl, int on)
 	}
 }
 
+static void do_notify(struct twl4030_usb *twl,
+		      enum omap_musb_vbus_id_status status)
+{
+	enum usb_phy_events event;
+
+	switch (status) {
+	case OMAP_MUSB_UNKNOWN:
+		event = USB_EVENT_NONE; break;
+	case OMAP_MUSB_ID_GROUND:
+		event = USB_EVENT_ID; break;
+	case OMAP_MUSB_ID_FLOAT:
+		event = USB_EVENT_NONE; break;
+	case OMAP_MUSB_VBUS_VALID:
+		event = USB_EVENT_VBUS; break;
+	case OMAP_MUSB_VBUS_OFF:
+		event = USB_EVENT_NONE; break;
+	}
+
+	atomic_notifier_call_chain(&twl->phy.notifier, event,
+				   twl->phy.otg->gadget);
+}
+
 static void twl4030_phy_suspend(struct twl4030_usb *twl, int controller_off)
 {
 	if (twl->asleep)

@@ -514,6 +537,7 @@ static irqreturn_t twl4030_usb_irq(int irq, void *_twl)
 			twl4030_phy_suspend(twl, 0);
 		else
 			twl4030_phy_resume(twl);
+		do_notify(twl, status);
 
 		omap_musb_mailbox(twl->linkstat);
 	}

@@ -537,6 +561,7 @@ static void twl4030_usb_phy_init(struct twl4030_usb *twl)
 			twl->asleep = 0;
 		}
 
+		do_notify(twl, status);
 		omap_musb_mailbox(twl->linkstat);
 	}
 	sysfs_notify(&twl->dev->kobj, NULL, "vbus");

@@ -634,6 +659,8 @@ static int __devinit twl4030_usb_probe(struct platform_device *pdev)
 	if (device_create_file(&pdev->dev, &dev_attr_vbus))
 		dev_warn(&pdev->dev, "could not create sysfs file\n");
 
+	ATOMIC_INIT_NOTIFIER_HEAD(&twl->phy.notifier);
+
 	/* Our job is to use irqs and status from the power module
 	 * to keep the transceiver disabled when nothing's connected.
 	 *