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

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

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 1010 bytes --]


Hi,

 this commit:

commit c9721438c009adf8e81d376839ed037c53b9b8d9
Author: Kishon Vijay Abraham I <kishon@ti.com>
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 <kishon@ti.com>
    Signed-off-by: Felipe Balbi <balbi@ti.com>


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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

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

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1205 bytes --]

Hi,

On Fri, Dec 14, 2012 at 08:04:27PM +1100, NeilBrown wrote:
>  this commit:
> 
> commit c9721438c009adf8e81d376839ed037c53b9b8d9
> Author: Kishon Vijay Abraham I <kishon@ti.com>
> 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 <kishon@ti.com>
>     Signed-off-by: Felipe Balbi <balbi@ti.com>
> 
> 
> 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?

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

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 4029 bytes --]

On Fri, 14 Dec 2012 11:36:59 +0200 Felipe Balbi <balbi@ti.com> wrote:

> Hi,
> 
> On Fri, Dec 14, 2012 at 08:04:27PM +1100, NeilBrown wrote:
> >  this commit:
> > 
> > commit c9721438c009adf8e81d376839ed037c53b9b8d9
> > Author: Kishon Vijay Abraham I <kishon@ti.com>
> > 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 <kishon@ti.com>
> >     Signed-off-by: Felipe Balbi <balbi@ti.com>
> > 
> > 
> > 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?
> 
> 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 <neilb@suse.de>
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 <neilb@suse.de>

diff --git a/drivers/usb/otg/twl4030-usb.c b/drivers/usb/otg/twl4030-usb.c
index f0d2e75..15733f9 100644
--- a/drivers/usb/otg/twl4030-usb.c
+++ b/drivers/usb/otg/twl4030-usb.c
@@ -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.
 	 *

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]