On Thu, Feb 28, 2013 at 11:38:52AM +0100, Fabio Baltieri wrote:
> Add transceiver notifier event handling to the ux500 driver to set vbus
> on specific transceiver events.
>
> Acked-by: Linus Walleij <[email protected]>
> Signed-off-by: Fabio Baltieri <[email protected]>
> ---
> drivers/usb/musb/ux500.c | 41 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 41 insertions(+)
>
> diff --git a/drivers/usb/musb/ux500.c b/drivers/usb/musb/ux500.c
> index 5b742ba..b20326bb 100644
> --- a/drivers/usb/musb/ux500.c
> +++ b/drivers/usb/musb/ux500.c
> @@ -98,6 +98,36 @@ static void ux500_musb_set_vbus(struct musb *musb, int is_on)
> musb_readb(musb->mregs, MUSB_DEVCTL));
> }
>
> +static int musb_otg_notifications(struct notifier_block *nb,
> + unsigned long event, void *unused)
> +{
> + struct musb *musb = container_of(nb, struct musb, nb);
> +
> + dev_dbg(musb->controller, "musb_otg_notifications %ld %s\n",
> + event, otg_state_string(musb->xceiv->state));
> +
> + switch (event) {
> + case USB_EVENT_ID:
> + dev_dbg(musb->controller, "ID GND\n");
> + ux500_musb_set_vbus(musb, 1);
> + break;
> + case USB_EVENT_VBUS:
> + dev_dbg(musb->controller, "VBUS Connect\n");
are you sure this is correct ? you're not doing anything in case of
vbus event. Shouldn't you make sure your vbus is off ? What if your user
uses a non-standard cable which has id-pin grounded on both sides ?
--
balbi
Hello Felipe,
On Mon, Mar 04, 2013 at 04:38:10PM +0200, Felipe Balbi wrote:
> On Thu, Feb 28, 2013 at 11:38:52AM +0100, Fabio Baltieri wrote:
> > Add transceiver notifier event handling to the ux500 driver to set vbus
> > on specific transceiver events.
> >
> > Acked-by: Linus Walleij <[email protected]>
> > Signed-off-by: Fabio Baltieri <[email protected]>
> > ---
> > drivers/usb/musb/ux500.c | 41 +++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 41 insertions(+)
> >
> > diff --git a/drivers/usb/musb/ux500.c b/drivers/usb/musb/ux500.c
> > index 5b742ba..b20326bb 100644
> > --- a/drivers/usb/musb/ux500.c
> > +++ b/drivers/usb/musb/ux500.c
> > @@ -98,6 +98,36 @@ static void ux500_musb_set_vbus(struct musb *musb, int is_on)
> > musb_readb(musb->mregs, MUSB_DEVCTL));
> > }
> >
> > +static int musb_otg_notifications(struct notifier_block *nb,
> > + unsigned long event, void *unused)
> > +{
> > + struct musb *musb = container_of(nb, struct musb, nb);
> > +
> > + dev_dbg(musb->controller, "musb_otg_notifications %ld %s\n",
> > + event, otg_state_string(musb->xceiv->state));
> > +
> > + switch (event) {
> > + case USB_EVENT_ID:
> > + dev_dbg(musb->controller, "ID GND\n");
> > + ux500_musb_set_vbus(musb, 1);
> > + break;
> > + case USB_EVENT_VBUS:
> > + dev_dbg(musb->controller, "VBUS Connect\n");
>
> are you sure this is correct ? you're not doing anything in case of
> vbus event. Shouldn't you make sure your vbus is off ?
The implementation I'm taking as reference for this patches does not do
anything on VBUS event and as I can see from commmit history, it has
been based on the omap2430 one, that in turns used to just run phy_init
- and that's not implemented. But...
> What if your user uses a non-standard cable which has id-pin grounded
> on both sides ?
A cable with two micro/mini connectors with grounded IDs? Did you ever
encountered such a thing? I guess a ux500_musb_set_vbus(musb, 0) would
not hurt in that case.
Fabio
--
Fabio Baltieri
Hi,
On Wed, Mar 06, 2013 at 09:40:28AM +0800, Fabio Baltieri wrote:
> > > Add transceiver notifier event handling to the ux500 driver to set vbus
> > > on specific transceiver events.
> > >
> > > Acked-by: Linus Walleij <[email protected]>
> > > Signed-off-by: Fabio Baltieri <[email protected]>
> > > ---
> > > drivers/usb/musb/ux500.c | 41 +++++++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 41 insertions(+)
> > >
> > > diff --git a/drivers/usb/musb/ux500.c b/drivers/usb/musb/ux500.c
> > > index 5b742ba..b20326bb 100644
> > > --- a/drivers/usb/musb/ux500.c
> > > +++ b/drivers/usb/musb/ux500.c
> > > @@ -98,6 +98,36 @@ static void ux500_musb_set_vbus(struct musb *musb, int is_on)
> > > musb_readb(musb->mregs, MUSB_DEVCTL));
> > > }
> > >
> > > +static int musb_otg_notifications(struct notifier_block *nb,
> > > + unsigned long event, void *unused)
> > > +{
> > > + struct musb *musb = container_of(nb, struct musb, nb);
> > > +
> > > + dev_dbg(musb->controller, "musb_otg_notifications %ld %s\n",
> > > + event, otg_state_string(musb->xceiv->state));
> > > +
> > > + switch (event) {
> > > + case USB_EVENT_ID:
> > > + dev_dbg(musb->controller, "ID GND\n");
> > > + ux500_musb_set_vbus(musb, 1);
> > > + break;
> > > + case USB_EVENT_VBUS:
> > > + dev_dbg(musb->controller, "VBUS Connect\n");
> >
> > are you sure this is correct ? you're not doing anything in case of
> > vbus event. Shouldn't you make sure your vbus is off ?
>
> The implementation I'm taking as reference for this patches does not do
> anything on VBUS event and as I can see from commmit history, it has
> been based on the omap2430 one, that in turns used to just run phy_init
> - and that's not implemented. But...
>
> > What if your user uses a non-standard cable which has id-pin grounded
> > on both sides ?
>
> A cable with two micro/mini connectors with grounded IDs? Did you ever
> encountered such a thing? I guess a ux500_musb_set_vbus(musb, 0) would
in fact, no...
> not hurt in that case.
right, just to be sure. Maybe I should fix omap2430 as well. I'll add it
to my TODO list.
--
balbi
On Wed, Mar 06, 2013 at 10:54:46AM +0200, Felipe Balbi wrote:
> > > are you sure this is correct ? you're not doing anything in case of
> > > vbus event. Shouldn't you make sure your vbus is off ?
> >
> > The implementation I'm taking as reference for this patches does not do
> > anything on VBUS event and as I can see from commmit history, it has
> > been based on the omap2430 one, that in turns used to just run phy_init
> > - and that's not implemented. But...
> >
> > > What if your user uses a non-standard cable which has id-pin grounded
> > > on both sides ?
> >
> > A cable with two micro/mini connectors with grounded IDs? Did you ever
> > encountered such a thing? I guess a ux500_musb_set_vbus(musb, 0) would
>
> in fact, no...
>
> > not hurt in that case.
>
> right, just to be sure. Maybe I should fix omap2430 as well. I'll add it
> to my TODO list.
Fair enough, I'll send a v2 of this patch with the added vbus-off as
soon as I manage to test it.
Thanks,
Fabio
--
Fabio Baltieri
Add transceiver notifier event handling to the ux500 driver to set vbus
on specific transceiver events.
Acked-by: Linus Walleij <[email protected]>
Signed-off-by: Fabio Baltieri <[email protected]>
---
drivers/usb/musb/ux500.c | 42 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/drivers/usb/musb/ux500.c b/drivers/usb/musb/ux500.c
index 5b742ba..55f24c6 100644
--- a/drivers/usb/musb/ux500.c
+++ b/drivers/usb/musb/ux500.c
@@ -98,6 +98,37 @@ static void ux500_musb_set_vbus(struct musb *musb, int is_on)
musb_readb(musb->mregs, MUSB_DEVCTL));
}
+static int musb_otg_notifications(struct notifier_block *nb,
+ unsigned long event, void *unused)
+{
+ struct musb *musb = container_of(nb, struct musb, nb);
+
+ dev_dbg(musb->controller, "musb_otg_notifications %ld %s\n",
+ event, otg_state_string(musb->xceiv->state));
+
+ switch (event) {
+ case USB_EVENT_ID:
+ dev_dbg(musb->controller, "ID GND\n");
+ ux500_musb_set_vbus(musb, 1);
+ break;
+ case USB_EVENT_VBUS:
+ dev_dbg(musb->controller, "VBUS Connect\n");
+ ux500_musb_set_vbus(musb, 0);
+ break;
+ case USB_EVENT_NONE:
+ dev_dbg(musb->controller, "VBUS Disconnect\n");
+ if (is_host_active(musb))
+ ux500_musb_set_vbus(musb, 0);
+ else
+ musb->xceiv->state = OTG_STATE_B_IDLE;
+ break;
+ default:
+ dev_dbg(musb->controller, "ID float\n");
+ return NOTIFY_DONE;
+ }
+ return NOTIFY_OK;
+}
+
static irqreturn_t ux500_musb_interrupt(int irq, void *__hci)
{
unsigned long flags;
@@ -120,12 +151,21 @@ static irqreturn_t ux500_musb_interrupt(int irq, void *__hci)
static int ux500_musb_init(struct musb *musb)
{
+ int status;
+
musb->xceiv = usb_get_phy(USB_PHY_TYPE_USB2);
if (IS_ERR_OR_NULL(musb->xceiv)) {
pr_err("HS USB OTG: no transceiver configured\n");
return -EPROBE_DEFER;
}
+ musb->nb.notifier_call = musb_otg_notifications;
+ status = usb_register_notifier(musb->xceiv, &musb->nb);
+ if (status < 0) {
+ dev_dbg(musb->controller, "notification register failed\n");
+ return status;
+ }
+
musb->isr = ux500_musb_interrupt;
return 0;
@@ -133,6 +173,8 @@ static int ux500_musb_init(struct musb *musb)
static int ux500_musb_exit(struct musb *musb)
{
+ usb_unregister_notifier(musb->xceiv, &musb->nb);
+
usb_put_phy(musb->xceiv);
return 0;
--
1.8.1.3
On Thu, Mar 07, 2013 at 04:57:40PM +0800, Fabio Baltieri wrote:
> Add transceiver notifier event handling to the ux500 driver to set vbus
> on specific transceiver events.
>
> Acked-by: Linus Walleij <[email protected]>
> Signed-off-by: Fabio Baltieri <[email protected]>
> ---
Sorry, I forgot about:
v2:
- turn off vbus on USB_EVENT_VBUS event
Note that this causes a trivial context conflict with patch 5, let me
know if you want me to resend the whole set.
Thanks,
Fabio
> drivers/usb/musb/ux500.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 42 insertions(+)
>
> diff --git a/drivers/usb/musb/ux500.c b/drivers/usb/musb/ux500.c
> index 5b742ba..55f24c6 100644
> --- a/drivers/usb/musb/ux500.c
> +++ b/drivers/usb/musb/ux500.c
> @@ -98,6 +98,37 @@ static void ux500_musb_set_vbus(struct musb *musb, int is_on)
> musb_readb(musb->mregs, MUSB_DEVCTL));
> }
>
> +static int musb_otg_notifications(struct notifier_block *nb,
> + unsigned long event, void *unused)
> +{
> + struct musb *musb = container_of(nb, struct musb, nb);
> +
> + dev_dbg(musb->controller, "musb_otg_notifications %ld %s\n",
> + event, otg_state_string(musb->xceiv->state));
> +
> + switch (event) {
> + case USB_EVENT_ID:
> + dev_dbg(musb->controller, "ID GND\n");
> + ux500_musb_set_vbus(musb, 1);
> + break;
> + case USB_EVENT_VBUS:
> + dev_dbg(musb->controller, "VBUS Connect\n");
> + ux500_musb_set_vbus(musb, 0);
> + break;
> + case USB_EVENT_NONE:
> + dev_dbg(musb->controller, "VBUS Disconnect\n");
> + if (is_host_active(musb))
> + ux500_musb_set_vbus(musb, 0);
> + else
> + musb->xceiv->state = OTG_STATE_B_IDLE;
> + break;
> + default:
> + dev_dbg(musb->controller, "ID float\n");
> + return NOTIFY_DONE;
> + }
> + return NOTIFY_OK;
> +}
> +
> static irqreturn_t ux500_musb_interrupt(int irq, void *__hci)
> {
> unsigned long flags;
> @@ -120,12 +151,21 @@ static irqreturn_t ux500_musb_interrupt(int irq, void *__hci)
>
> static int ux500_musb_init(struct musb *musb)
> {
> + int status;
> +
> musb->xceiv = usb_get_phy(USB_PHY_TYPE_USB2);
> if (IS_ERR_OR_NULL(musb->xceiv)) {
> pr_err("HS USB OTG: no transceiver configured\n");
> return -EPROBE_DEFER;
> }
>
> + musb->nb.notifier_call = musb_otg_notifications;
> + status = usb_register_notifier(musb->xceiv, &musb->nb);
> + if (status < 0) {
> + dev_dbg(musb->controller, "notification register failed\n");
> + return status;
> + }
> +
> musb->isr = ux500_musb_interrupt;
>
> return 0;
> @@ -133,6 +173,8 @@ static int ux500_musb_init(struct musb *musb)
>
> static int ux500_musb_exit(struct musb *musb)
> {
> + usb_unregister_notifier(musb->xceiv, &musb->nb);
> +
> usb_put_phy(musb->xceiv);
>
> return 0;
> --
> 1.8.1.3
>
--
Fabio Baltieri
On Thu, Mar 07, 2013 at 05:10:57PM +0800, Fabio Baltieri wrote:
> On Thu, Mar 07, 2013 at 04:57:40PM +0800, Fabio Baltieri wrote:
> > Add transceiver notifier event handling to the ux500 driver to set vbus
> > on specific transceiver events.
> >
> > Acked-by: Linus Walleij <[email protected]>
> > Signed-off-by: Fabio Baltieri <[email protected]>
> > ---
>
> Sorry, I forgot about:
>
> v2:
> - turn off vbus on USB_EVENT_VBUS event
>
> Note that this causes a trivial context conflict with patch 5, let me
> know if you want me to resend the whole set.
please do, I rather you resend otherwise I could mis-solve the conflict
:-p
--
balbi