usb: musb: omap2430: handle charger OTG xceiver event

Set a flag on OTG charger event and check it on cable remove event (i.e. USB_EVENT_NONE). This way we will not need to power up the PHY when an external charger is detected by the transceiver itself.

---
 drivers/usb/musb/musb_core.h |    1 +
 drivers/usb/musb/omap2430.c  |   12 ++++++++++++
 2 files changed, 13 insertions(+), 0 deletions(-)

index 0e053b5..f65269d 100644
@@ -497,6 +497,7 @@ struct musb {
 	struct usb_gadget	g;			/* the gadget */
 	struct usb_gadget_driver *gadget_driver;	/* its driver */
 #endif
+	bool			is_ac_charger:1;
 
 	/*
 	 * FIXME: Remove this flag.

index c5d4c44..ac4cfbc 100644
@@ -257,6 +257,11 @@ static int musb_otg_notifications(struct notifier_block *nb,
 		}
 		break;
 
+	case USB_EVENT_CHARGER:
+		dev_dbg(musb->controller, "Dedicated charger connect\n");
+		musb->is_ac_charger = true;
+		break;
+
 	case USB_EVENT_VBUS:
 		dev_dbg(musb->controller, "VBUS Connect\n");
 

@@ -268,6 +273,13 @@ static int musb_otg_notifications(struct notifier_block *nb,
 		break;
 
 	case USB_EVENT_NONE:
+		if (musb->is_ac_charger) {
+			dev_dbg(musb->controller,
+				"Dedicated charger disconnect\n");
+			musb->is_ac_charger = false;
+			break;
+		}
+
 		dev_dbg(musb->controller, "VBUS Disconnect\n");
 
 #ifdef CONFIG_USB_GADGET_MUSB_HDRC

1.7.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hi,

s/PHY/LINK

... your logic is inversed. Link shouldn't be powered up to start with. We should only power up the Link after we know it *will* be needed. The way you're doing this is:

connect cable -> resume PHY -> resume Link -> if is_charger suspend Link.

Where it should be:

connect cable -> resume PHY -> if is_charger goto done.

Now, to make a generic solution (which is what we want in the long run) we still need a few changes on the OTG support and otg_transceiver on Linux.

balbi

Actually it's both, I should update the commit message.

The link *isn't* being powered up by my patch. On an already detected charger you should do nothing at all in musb. That's the whole point of the patch: add the ability to process the USB_EVENT_CHARGER event in musb from the otg_transceiver that has already detected the charger. Today, on USB_EVENT_VBUS, you'd resume your link and then call otg_init, which would powerup the phy on omap4, which we do not need to do for USB_EVENT_CHARGER.

Not at all. Have even you looked at the patch? With the patch,

1) "connect cable", detected by otg_transceiver.
2) otg_transceiver does the detection of ID pin and/or VBUS and/or dedicated charger.
3) otg_transceiver sends USB_EVENT_xxx
4) In musb_otg_notification(), check if the transceiver detected a charger (USB_EVENT_CHARGER).
4.1) If yes, then do nothing and leave the LINK *and* the PHY in their off states.

We only have to resume the PHY if the transceiver didn't do the charger detection. The TWL6030 doesn't do that, but depending on your board, you may have other devices upstream from the OMAP on the USB lines that may have done the detection for you.

--Dima

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Hi,

the PHY has to be powered up during charger detection. You could power it down after charger has been started and power it back up on the rising edge (or high level, don't remember if this is edge of level triggered) of the VBUS IRQ.

not by your patch, true. My point is that we're powering it up too early and if you want to change anything regarding Charger Detection you need to change at the right place.

and what I said (or tried to) is that USB_EVENT_VBUS isn't an event to be handled by the link at all. The link should be pm_runtime_suspend()ed right after probe() and should be kept that way until PHY sends USB_EVENT_CHARGER_DETECTION_DONE or something similar.

That is wrong true. Still your patch only adds a flag. It's not implementing what you're describing here.

yes, and it will send USB_EVENT_VBUS which is resuming link.

this is not what your patch is doing. It only enables/disables a flag which your patch added.

PHY == transceiver.

Like I said before, if you want to put any changes regarding charger detection, do it at the right location. First add some events for different USB port types (SDP, CDP, DCP), then notify those, then add handling of those events to MUSB and PHY drivers, then fix up musb to be runtime suspended right after probe, then you only need to wake musb UP in case of SDP and CDP. If you're attached to a DCP you don't even need MUSB to handle that.

Also, while you are doing Charger detection, be sure to disable SoftConnect bit on MUSB and put PHY in NONDRIVING mode, this will prevent MUSB from even knowing there's a cable connected.

balbi