| Message ID | 1412673344-25443-1-git-send-email-kiran.kumar@linaro.org |
|---|---|
| State | New |
| Headers | show |
Hi, On Tue, Oct 07, 2014 at 02:45:44PM +0530, Kiran Kumar Raparthy wrote: > diff --git a/drivers/usb/phy/otg-wakeupsource.c b/drivers/usb/phy/otg-wakeupsource.c > new file mode 100644 > index 0000000..00d3359 > --- /dev/null > +++ b/drivers/usb/phy/otg-wakeupsource.c > @@ -0,0 +1,134 @@ > +/* > + * otg-wakeupsource.c > + * > + * Copyright (C) 2011 Google, Inc. > + * > + * This software is licensed under the terms of the GNU General Public > + * License version 2, as published by the Free Software Foundation, and > + * may be copied, distributed, and modified under those terms. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + */ > + > +#include <linux/kernel.h> > +#include <linux/device.h> > +#include <linux/module.h> > +#include <linux/notifier.h> > +#include <linux/pm_wakeup.h> > +#include <linux/spinlock.h> > +#include <linux/usb/otg.h> > + > +static void otgws_handle_event(struct usb_phy *otgws_xceiv, unsigned long event) > +{ > + unsigned long irqflags; > + > + spin_lock_irqsave(&otgws_xceiv->otgws_slock, irqflags); > + > + switch (event) { > + case USB_EVENT_VBUS: Looks like VBUS should be temporary too. > + case USB_EVENT_ENUMERATED: > + __pm_stay_awake(&otgws_xceiv->wsource); > + break; > + > + case USB_EVENT_NONE: > + case USB_EVENT_ID: > + case USB_EVENT_CHARGER: > + __pm_wakeup_event(&otgws_xceiv->wsource, > + msecs_to_jiffies(TEMPORARY_HOLD_TIME)); > + break; > + > + default: > + break; > + } > + > + spin_unlock_irqrestore(&otgws_xceiv->otgws_slock, irqflags); > +} > + > +static int otgws_otg_usb2_notifications(struct notifier_block *nb, > + unsigned long event, void *unused) > +{ > + static struct usb_phy *otgws_xceiv; > + > + otgws_xceiv = usb_get_phy(USB_PHY_TYPE_USB2); > + > + if (IS_ERR(otgws_xceiv)) { > + pr_err("%s: No OTG transceiver found\n", __func__); > + return PTR_ERR(otgws_xceiv); > + } > + > + otgws_handle_event(otgws_xceiv, event); > + > + return NOTIFY_OK; > +} > + > +static int otgws_otg_usb3_notifications(struct notifier_block *nb, > + unsigned long event, void *unused) > +{ > + static struct usb_phy *otgws_xceiv; > + > + otgws_xceiv = usb_get_phy(USB_PHY_TYPE_USB3); > + > + if (IS_ERR(otgws_xceiv)) { > + pr_err("%s: No OTG transceiver found\n", __func__); > + return PTR_ERR(otgws_xceiv); > + } > + > + otgws_handle_event(otgws_xceiv, event); > + > + return NOTIFY_OK; > +} > + > +static int otg_wakeupsource_init(void) > +{ > + int ret_usb2; > + int ret_usb3; > + char wsource_name_usb2[40]; > + char wsource_name_usb3[40]; > + static struct usb_phy *otgws_xceiv_usb2; > + static struct usb_phy *otgws_xceiv_usb3; > + > + otgws_xceiv_usb2 = usb_get_phy(USB_PHY_TYPE_USB2); > + otgws_xceiv_usb3 = usb_get_phy(USB_PHY_TYPE_USB3); > + > + if (IS_ERR(otgws_xceiv_usb2) && IS_ERR(otgws_xceiv_usb3)) { > + pr_err("%s: No OTG transceiver found\n", __func__); > + return PTR_ERR(otgws_xceiv_usb2); > + } > + > + spin_lock_init(&otgws_xceiv_usb2->otgws_slock); > + spin_lock_init(&otgws_xceiv_usb3->otgws_slock); > + > + snprintf(wsource_name_usb2, sizeof(wsource_name_usb2), "vbus-%s", > + dev_name(otgws_xceiv_usb2->dev)); > + wakeup_source_init(&otgws_xceiv_usb2->wsource, wsource_name_usb2); > + > + snprintf(wsource_name_usb3, sizeof(wsource_name_usb3), "vbus-%s", > + dev_name(otgws_xceiv_usb3->dev)); > + wakeup_source_init(&otgws_xceiv_usb3->wsource, wsource_name_usb3); > + > + otgws_xceiv_usb2->otgws_nb.notifier_call = otgws_otg_usb2_notifications; > + ret_usb2 = usb_register_notifier(otgws_xceiv_usb2, > + &otgws_xceiv_usb2->otgws_nb); > + > + otgws_xceiv_usb3->otgws_nb.notifier_call = otgws_otg_usb3_notifications; > + ret_usb3 = usb_register_notifier(otgws_xceiv_usb3, > + &otgws_xceiv_usb3->otgws_nb); > + > + if (ret_usb2 && ret_usb3) { > + pr_err("%s: usb_register_notifier on transceiver failed\n", > + __func__); > + wakeup_source_trash(&otgws_xceiv_usb2->wsource); > + wakeup_source_trash(&otgws_xceiv_usb3->wsource); > + otgws_xceiv_usb2 = NULL; > + otgws_xceiv_usb3 = NULL; > + return ret_usb2 | ret_usb3; > + } > + > + return 0; > +} > + > +late_initcall(otg_wakeupsource_init); you guys are really not getting what I mean. I asked for this to be built into the core itself. This means that you shouldn't need to use notifications nor should you need to call usb_get_phy(). You're part of the PHY framework. All this late_initcall() nonsense should go. This code won't even work if we have more than one phy of the same type (AM437x SoC, for example, has up to 4 instances of dwc3, so that's 4 USB2 PHYs), because you can't grab the PHY you want. What you need is to: 1) make PHY notifiers generic (move all of that phy-core.c) 2) introduce usb_phy_set_event(phy, event) (which just sets the even to a phy->event member for now) 3) make all PHY drivers use usb_phy_set_event() 4) add the following to usb_phy_set_event() switch (event) { case USB_EVENT_ENUMERATED: pm_stay_awake(&otgws_xceiv->wsource); break; case USB_EVENT_NONE: case USB_EVENT_VBUS: case USB_EVENT_ID: case USB_EVENT_CHARGER: pm_wakeup_event(&otgws_xceiv->wsource, msecs_to_jiffies(TEMPORARY_HOLD_TIME)); break; default: break; } note that I'm calling locked versions of those functions so you can drop the spinlock you added. But, dependending on when usb_phy_set_event() is called, you might want to switch back to unlocked versions. In any case, the new spinlock is unnecessary because you can either use dev->power.lock or you're calling usb_phy_set_event() from and IRQ handler. > diff --git a/include/linux/usb/phy.h b/include/linux/usb/phy.h > index 353053a..dd64e2e 100644 > --- a/include/linux/usb/phy.h > +++ b/include/linux/usb/phy.h > @@ -12,6 +12,8 @@ > #include <linux/notifier.h> > #include <linux/usb.h> > > +#define TEMPORARY_HOLD_TIME 2000 > + > enum usb_phy_interface { > USBPHY_INTERFACE_MODE_UNKNOWN, > USBPHY_INTERFACE_MODE_UTMI, > @@ -88,6 +90,12 @@ struct usb_phy { > > /* for notification of usb_phy_events */ > struct atomic_notifier_head notifier; > + struct notifier_block otgws_nb; drop this notifier block. > + > + /* wakeup source */ > + struct wakeup_source wsource; this is the only thing you need. > + > + spinlock_t otgws_slock; drop this lock.
Hi Felipe, Thank you very much for taking time in reviewing the patch. I will try to improve the patch as per your suggestions. however,i have few queries which i wanted to understand from you. On 7 October 2014 19:55, Felipe Balbi <balbi@ti.com> wrote: > Hi, > > On Tue, Oct 07, 2014 at 02:45:44PM +0530, Kiran Kumar Raparthy wrote: >> diff --git a/drivers/usb/phy/otg-wakeupsource.c b/drivers/usb/phy/otg-wakeupsource.c >> new file mode 100644 >> index 0000000..00d3359 >> --- /dev/null >> +++ b/drivers/usb/phy/otg-wakeupsource.c >> @@ -0,0 +1,134 @@ >> +/* >> + * otg-wakeupsource.c >> + * >> + * Copyright (C) 2011 Google, Inc. >> + * >> + * This software is licensed under the terms of the GNU General Public >> + * License version 2, as published by the Free Software Foundation, and >> + * may be copied, distributed, and modified under those terms. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + */ >> + >> +#include <linux/kernel.h> >> +#include <linux/device.h> >> +#include <linux/module.h> >> +#include <linux/notifier.h> >> +#include <linux/pm_wakeup.h> >> +#include <linux/spinlock.h> >> +#include <linux/usb/otg.h> >> + >> +static void otgws_handle_event(struct usb_phy *otgws_xceiv, unsigned long event) >> +{ >> + unsigned long irqflags; >> + >> + spin_lock_irqsave(&otgws_xceiv->otgws_slock, irqflags); >> + >> + switch (event) { >> + case USB_EVENT_VBUS: > > Looks like VBUS should be temporary too. > >> + case USB_EVENT_ENUMERATED: >> + __pm_stay_awake(&otgws_xceiv->wsource); >> + break; >> + >> + case USB_EVENT_NONE: >> + case USB_EVENT_ID: >> + case USB_EVENT_CHARGER: >> + __pm_wakeup_event(&otgws_xceiv->wsource, >> + msecs_to_jiffies(TEMPORARY_HOLD_TIME)); >> + break; >> + >> + default: >> + break; >> + } >> + >> + spin_unlock_irqrestore(&otgws_xceiv->otgws_slock, irqflags); >> +} >> + >> +static int otgws_otg_usb2_notifications(struct notifier_block *nb, >> + unsigned long event, void *unused) >> +{ >> + static struct usb_phy *otgws_xceiv; >> + >> + otgws_xceiv = usb_get_phy(USB_PHY_TYPE_USB2); >> + >> + if (IS_ERR(otgws_xceiv)) { >> + pr_err("%s: No OTG transceiver found\n", __func__); >> + return PTR_ERR(otgws_xceiv); >> + } >> + >> + otgws_handle_event(otgws_xceiv, event); >> + >> + return NOTIFY_OK; >> +} >> + >> +static int otgws_otg_usb3_notifications(struct notifier_block *nb, >> + unsigned long event, void *unused) >> +{ >> + static struct usb_phy *otgws_xceiv; >> + >> + otgws_xceiv = usb_get_phy(USB_PHY_TYPE_USB3); >> + >> + if (IS_ERR(otgws_xceiv)) { >> + pr_err("%s: No OTG transceiver found\n", __func__); >> + return PTR_ERR(otgws_xceiv); >> + } >> + >> + otgws_handle_event(otgws_xceiv, event); >> + >> + return NOTIFY_OK; >> +} >> + >> +static int otg_wakeupsource_init(void) >> +{ >> + int ret_usb2; >> + int ret_usb3; >> + char wsource_name_usb2[40]; >> + char wsource_name_usb3[40]; >> + static struct usb_phy *otgws_xceiv_usb2; >> + static struct usb_phy *otgws_xceiv_usb3; >> + >> + otgws_xceiv_usb2 = usb_get_phy(USB_PHY_TYPE_USB2); >> + otgws_xceiv_usb3 = usb_get_phy(USB_PHY_TYPE_USB3); >> + >> + if (IS_ERR(otgws_xceiv_usb2) && IS_ERR(otgws_xceiv_usb3)) { >> + pr_err("%s: No OTG transceiver found\n", __func__); >> + return PTR_ERR(otgws_xceiv_usb2); >> + } >> + >> + spin_lock_init(&otgws_xceiv_usb2->otgws_slock); >> + spin_lock_init(&otgws_xceiv_usb3->otgws_slock); >> + >> + snprintf(wsource_name_usb2, sizeof(wsource_name_usb2), "vbus-%s", >> + dev_name(otgws_xceiv_usb2->dev)); >> + wakeup_source_init(&otgws_xceiv_usb2->wsource, wsource_name_usb2); >> + >> + snprintf(wsource_name_usb3, sizeof(wsource_name_usb3), "vbus-%s", >> + dev_name(otgws_xceiv_usb3->dev)); >> + wakeup_source_init(&otgws_xceiv_usb3->wsource, wsource_name_usb3); >> + >> + otgws_xceiv_usb2->otgws_nb.notifier_call = otgws_otg_usb2_notifications; >> + ret_usb2 = usb_register_notifier(otgws_xceiv_usb2, >> + &otgws_xceiv_usb2->otgws_nb); >> + >> + otgws_xceiv_usb3->otgws_nb.notifier_call = otgws_otg_usb3_notifications; >> + ret_usb3 = usb_register_notifier(otgws_xceiv_usb3, >> + &otgws_xceiv_usb3->otgws_nb); >> + >> + if (ret_usb2 && ret_usb3) { >> + pr_err("%s: usb_register_notifier on transceiver failed\n", >> + __func__); >> + wakeup_source_trash(&otgws_xceiv_usb2->wsource); >> + wakeup_source_trash(&otgws_xceiv_usb3->wsource); >> + otgws_xceiv_usb2 = NULL; >> + otgws_xceiv_usb3 = NULL; >> + return ret_usb2 | ret_usb3; >> + } >> + >> + return 0; >> +} >> + >> +late_initcall(otg_wakeupsource_init); > > you guys are really not getting what I mean. I asked for this to be > built into the core itself. This means that you shouldn't need to use > notifications nor should you need to call usb_get_phy(). You're part of > the PHY framework. > > All this late_initcall() nonsense should go. > > This code won't even work if we have more than one phy of the same type > (AM437x SoC, for example, has up to 4 instances of dwc3, so that's 4 > USB2 PHYs), because you can't grab the PHY you want. Apologies,I am new to usb sub system,so i missed this point before i posted my patch,Thanks for the information. > > What you need is to: > > 1) make PHY notifiers generic (move all of that phy-core.c) From the above points,you mentioned that "if we built it into core,we shouldn't need to use notifications" and your first point here says that make phy notifiers generic in phy-core.c can you help me understanding it better so that there wont be any understanding gap. > 2) introduce usb_phy_set_event(phy, event) (which just sets the even to a > phy->event member for now) > 3) make all PHY drivers use usb_phy_set_event() > 4) add the following to usb_phy_set_event() > > switch (event) { > case USB_EVENT_ENUMERATED: > pm_stay_awake(&otgws_xceiv->wsource); > break; > > case USB_EVENT_NONE: > case USB_EVENT_VBUS: > case USB_EVENT_ID: > case USB_EVENT_CHARGER: > pm_wakeup_event(&otgws_xceiv->wsource, > msecs_to_jiffies(TEMPORARY_HOLD_TIME)); > break; > > default: > break; > } > Once the phy drivers receives per-PHY event notification(if we use notifier,else "for any event") we can call usb_phy_set_event from phy driver to hold the wakeup source. Please correct me if my understanding is incorrect. I have gone through some phy drivers in drivers/phy,since the each driver implementation is different from others, i didn't get the best place in PHY driver where we can trigger(use phy-core functionality) per-PHY notifier registration. any pointers here? Regards, Kiran > note that I'm calling locked versions of those functions so you can drop > the spinlock you added. But, dependending on when usb_phy_set_event() is > called, you might want to switch back to unlocked versions. In any case, > the new spinlock is unnecessary because you can either use > dev->power.lock or you're calling usb_phy_set_event() from and IRQ > handler. > >> diff --git a/include/linux/usb/phy.h b/include/linux/usb/phy.h >> index 353053a..dd64e2e 100644 >> --- a/include/linux/usb/phy.h >> +++ b/include/linux/usb/phy.h >> @@ -12,6 +12,8 @@ >> #include <linux/notifier.h> >> #include <linux/usb.h> >> >> +#define TEMPORARY_HOLD_TIME 2000 >> + >> enum usb_phy_interface { >> USBPHY_INTERFACE_MODE_UNKNOWN, >> USBPHY_INTERFACE_MODE_UTMI, >> @@ -88,6 +90,12 @@ struct usb_phy { >> >> /* for notification of usb_phy_events */ >> struct atomic_notifier_head notifier; >> + struct notifier_block otgws_nb; > > drop this notifier block. > >> + >> + /* wakeup source */ >> + struct wakeup_source wsource; > > this is the only thing you need. > >> + >> + spinlock_t otgws_slock; > > drop this lock. > > -- > balbi -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Fri, Oct 10, 2014 at 11:37:31AM +0530, Kiran Raparthy wrote: > Hi Felipe, > Thank you very much for taking time in reviewing the patch. > I will try to improve the patch as per your suggestions. > however,i have few queries which i wanted to understand from you. sure thing. > On 7 October 2014 19:55, Felipe Balbi <balbi@ti.com> wrote: > >> +static int otg_wakeupsource_init(void) > >> +{ > >> + int ret_usb2; > >> + int ret_usb3; > >> + char wsource_name_usb2[40]; > >> + char wsource_name_usb3[40]; > >> + static struct usb_phy *otgws_xceiv_usb2; > >> + static struct usb_phy *otgws_xceiv_usb3; > >> + > >> + otgws_xceiv_usb2 = usb_get_phy(USB_PHY_TYPE_USB2); > >> + otgws_xceiv_usb3 = usb_get_phy(USB_PHY_TYPE_USB3); > >> + > >> + if (IS_ERR(otgws_xceiv_usb2) && IS_ERR(otgws_xceiv_usb3)) { > >> + pr_err("%s: No OTG transceiver found\n", __func__); > >> + return PTR_ERR(otgws_xceiv_usb2); > >> + } > >> + > >> + spin_lock_init(&otgws_xceiv_usb2->otgws_slock); > >> + spin_lock_init(&otgws_xceiv_usb3->otgws_slock); > >> + > >> + snprintf(wsource_name_usb2, sizeof(wsource_name_usb2), "vbus-%s", > >> + dev_name(otgws_xceiv_usb2->dev)); > >> + wakeup_source_init(&otgws_xceiv_usb2->wsource, wsource_name_usb2); > >> + > >> + snprintf(wsource_name_usb3, sizeof(wsource_name_usb3), "vbus-%s", > >> + dev_name(otgws_xceiv_usb3->dev)); > >> + wakeup_source_init(&otgws_xceiv_usb3->wsource, wsource_name_usb3); > >> + > >> + otgws_xceiv_usb2->otgws_nb.notifier_call = otgws_otg_usb2_notifications; > >> + ret_usb2 = usb_register_notifier(otgws_xceiv_usb2, > >> + &otgws_xceiv_usb2->otgws_nb); > >> + > >> + otgws_xceiv_usb3->otgws_nb.notifier_call = otgws_otg_usb3_notifications; > >> + ret_usb3 = usb_register_notifier(otgws_xceiv_usb3, > >> + &otgws_xceiv_usb3->otgws_nb); > >> + > >> + if (ret_usb2 && ret_usb3) { > >> + pr_err("%s: usb_register_notifier on transceiver failed\n", > >> + __func__); > >> + wakeup_source_trash(&otgws_xceiv_usb2->wsource); > >> + wakeup_source_trash(&otgws_xceiv_usb3->wsource); > >> + otgws_xceiv_usb2 = NULL; > >> + otgws_xceiv_usb3 = NULL; > >> + return ret_usb2 | ret_usb3; > >> + } > >> + > >> + return 0; > >> +} > >> + > >> +late_initcall(otg_wakeupsource_init); > > > > you guys are really not getting what I mean. I asked for this to be > > built into the core itself. This means that you shouldn't need to use > > notifications nor should you need to call usb_get_phy(). You're part of > > the PHY framework. > > > > All this late_initcall() nonsense should go. > > > > This code won't even work if we have more than one phy of the same type > > (AM437x SoC, for example, has up to 4 instances of dwc3, so that's 4 > > USB2 PHYs), because you can't grab the PHY you want. > > Apologies,I am new to usb sub system,so i missed this point before i > posted my patch,Thanks for the information. np. > > What you need is to: > > > > 1) make PHY notifiers generic (move all of that phy-core.c) > From the above points,you mentioned that "if we built it into core,we > shouldn't need to use notifications" > and your first point here says that make phy notifiers generic in phy-core.c > can you help me understanding it better so that there wont be any > understanding gap. yeah, notifiers should go but if you really must use them, then at least make all of that generic ;-) > > 2) introduce usb_phy_set_event(phy, event) (which just sets the even to a > > phy->event member for now) > > 3) make all PHY drivers use usb_phy_set_event() > > 4) add the following to usb_phy_set_event() > > > > switch (event) { > > case USB_EVENT_ENUMERATED: > > pm_stay_awake(&otgws_xceiv->wsource); > > break; > > > > case USB_EVENT_NONE: > > case USB_EVENT_VBUS: > > case USB_EVENT_ID: > > case USB_EVENT_CHARGER: > > pm_wakeup_event(&otgws_xceiv->wsource, > > msecs_to_jiffies(TEMPORARY_HOLD_TIME)); > > break; > > > > default: > > break; > > } > > > Once the phy drivers receives per-PHY event notification(if we use > notifier,else "for any event") we can call usb_phy_set_event from phy > driver to hold the wakeup source. > Please correct me if my understanding is incorrect. yeah. In fact, you can call usb_phy_set_event() directly from PHY's IRQ handler. > I have gone through some phy drivers in drivers/phy,since the each > driver implementation is different from others, i didn't get the best > place in PHY driver > where we can trigger(use phy-core functionality) per-PHY notifier > registration. any pointers here? registration ? probe(), they all have probe() functions. Now to figure out where to call usb_phy_set_event(). That's something completely different, and that's where the core of this change is :-) For PHYs which have IRQ lines, easy: just call usb_phy_set_event() from IRQ handler. For those who don't, then it's a little more difficult and will require your investigation.
On 10 October 2014 20:50, Felipe Balbi <balbi@ti.com> wrote: > Hi, > > On Fri, Oct 10, 2014 at 11:37:31AM +0530, Kiran Raparthy wrote: >> Hi Felipe, >> Thank you very much for taking time in reviewing the patch. >> I will try to improve the patch as per your suggestions. >> however,i have few queries which i wanted to understand from you. > > sure thing. > >> On 7 October 2014 19:55, Felipe Balbi <balbi@ti.com> wrote: >> >> +static int otg_wakeupsource_init(void) >> >> +{ >> >> + int ret_usb2; >> >> + int ret_usb3; >> >> + char wsource_name_usb2[40]; >> >> + char wsource_name_usb3[40]; >> >> + static struct usb_phy *otgws_xceiv_usb2; >> >> + static struct usb_phy *otgws_xceiv_usb3; >> >> + >> >> + otgws_xceiv_usb2 = usb_get_phy(USB_PHY_TYPE_USB2); >> >> + otgws_xceiv_usb3 = usb_get_phy(USB_PHY_TYPE_USB3); >> >> + >> >> + if (IS_ERR(otgws_xceiv_usb2) && IS_ERR(otgws_xceiv_usb3)) { >> >> + pr_err("%s: No OTG transceiver found\n", __func__); >> >> + return PTR_ERR(otgws_xceiv_usb2); >> >> + } >> >> + >> >> + spin_lock_init(&otgws_xceiv_usb2->otgws_slock); >> >> + spin_lock_init(&otgws_xceiv_usb3->otgws_slock); >> >> + >> >> + snprintf(wsource_name_usb2, sizeof(wsource_name_usb2), "vbus-%s", >> >> + dev_name(otgws_xceiv_usb2->dev)); >> >> + wakeup_source_init(&otgws_xceiv_usb2->wsource, wsource_name_usb2); >> >> + >> >> + snprintf(wsource_name_usb3, sizeof(wsource_name_usb3), "vbus-%s", >> >> + dev_name(otgws_xceiv_usb3->dev)); >> >> + wakeup_source_init(&otgws_xceiv_usb3->wsource, wsource_name_usb3); >> >> + >> >> + otgws_xceiv_usb2->otgws_nb.notifier_call = otgws_otg_usb2_notifications; >> >> + ret_usb2 = usb_register_notifier(otgws_xceiv_usb2, >> >> + &otgws_xceiv_usb2->otgws_nb); >> >> + >> >> + otgws_xceiv_usb3->otgws_nb.notifier_call = otgws_otg_usb3_notifications; >> >> + ret_usb3 = usb_register_notifier(otgws_xceiv_usb3, >> >> + &otgws_xceiv_usb3->otgws_nb); >> >> + >> >> + if (ret_usb2 && ret_usb3) { >> >> + pr_err("%s: usb_register_notifier on transceiver failed\n", >> >> + __func__); >> >> + wakeup_source_trash(&otgws_xceiv_usb2->wsource); >> >> + wakeup_source_trash(&otgws_xceiv_usb3->wsource); >> >> + otgws_xceiv_usb2 = NULL; >> >> + otgws_xceiv_usb3 = NULL; >> >> + return ret_usb2 | ret_usb3; >> >> + } >> >> + >> >> + return 0; >> >> +} >> >> + >> >> +late_initcall(otg_wakeupsource_init); >> > >> > you guys are really not getting what I mean. I asked for this to be >> > built into the core itself. This means that you shouldn't need to use >> > notifications nor should you need to call usb_get_phy(). You're part of >> > the PHY framework. >> > >> > All this late_initcall() nonsense should go. >> > >> > This code won't even work if we have more than one phy of the same type >> > (AM437x SoC, for example, has up to 4 instances of dwc3, so that's 4 >> > USB2 PHYs), because you can't grab the PHY you want. >> >> Apologies,I am new to usb sub system,so i missed this point before i >> posted my patch,Thanks for the information. > > np. > >> > What you need is to: >> > >> > 1) make PHY notifiers generic (move all of that phy-core.c) >> From the above points,you mentioned that "if we built it into core,we >> shouldn't need to use notifications" >> and your first point here says that make phy notifiers generic in phy-core.c >> can you help me understanding it better so that there wont be any >> understanding gap. > > yeah, notifiers should go but if you really must use them, then at least > make all of that generic ;-) > >> > 2) introduce usb_phy_set_event(phy, event) (which just sets the even to a >> > phy->event member for now) >> > 3) make all PHY drivers use usb_phy_set_event() >> > 4) add the following to usb_phy_set_event() >> > >> > switch (event) { >> > case USB_EVENT_ENUMERATED: >> > pm_stay_awake(&otgws_xceiv->wsource); >> > break; >> > >> > case USB_EVENT_NONE: >> > case USB_EVENT_VBUS: >> > case USB_EVENT_ID: >> > case USB_EVENT_CHARGER: >> > pm_wakeup_event(&otgws_xceiv->wsource, >> > msecs_to_jiffies(TEMPORARY_HOLD_TIME)); >> > break; >> > >> > default: >> > break; >> > } >> > >> Once the phy drivers receives per-PHY event notification(if we use >> notifier,else "for any event") we can call usb_phy_set_event from phy >> driver to hold the wakeup source. >> Please correct me if my understanding is incorrect. > > yeah. In fact, you can call usb_phy_set_event() directly from PHY's IRQ > handler. > >> I have gone through some phy drivers in drivers/phy,since the each >> driver implementation is different from others, i didn't get the best >> place in PHY driver >> where we can trigger(use phy-core functionality) per-PHY notifier >> registration. any pointers here? > > registration ? probe(), they all have probe() functions. Now to figure > out where to call usb_phy_set_event(). That's something completely > different, and that's where the core of this change is :-) > > For PHYs which have IRQ lines, easy: just call usb_phy_set_event() from > IRQ handler. For those who don't, then it's a little more difficult and > will require your investigation. Thanks for your inputs. Regards, Kiran > > -- > balbi -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
HI Felipe, On 27 October 2014 15:06, Kiran Raparthy <kiran.kumar@linaro.org> wrote: > Hi Felipe, > > On 10 October 2014 20:50, Felipe Balbi <balbi@ti.com> wrote: >> Hi, >> >> On Fri, Oct 10, 2014 at 11:37:31AM +0530, Kiran Raparthy wrote: >>> Hi Felipe, >>> Thank you very much for taking time in reviewing the patch. >>> I will try to improve the patch as per your suggestions. >>> however,i have few queries which i wanted to understand from you. >> >> sure thing. >> >>> On 7 October 2014 19:55, Felipe Balbi <balbi@ti.com> wrote: >>> >> +static int otg_wakeupsource_init(void) >>> >> +{ >>> >> + int ret_usb2; >>> >> + int ret_usb3; >>> >> + char wsource_name_usb2[40]; >>> >> + char wsource_name_usb3[40]; >>> >> + static struct usb_phy *otgws_xceiv_usb2; >>> >> + static struct usb_phy *otgws_xceiv_usb3; >>> >> + >>> >> + otgws_xceiv_usb2 = usb_get_phy(USB_PHY_TYPE_USB2); >>> >> + otgws_xceiv_usb3 = usb_get_phy(USB_PHY_TYPE_USB3); >>> >> + >>> >> + if (IS_ERR(otgws_xceiv_usb2) && IS_ERR(otgws_xceiv_usb3)) { >>> >> + pr_err("%s: No OTG transceiver found\n", __func__); >>> >> + return PTR_ERR(otgws_xceiv_usb2); >>> >> + } >>> >> + >>> >> + spin_lock_init(&otgws_xceiv_usb2->otgws_slock); >>> >> + spin_lock_init(&otgws_xceiv_usb3->otgws_slock); >>> >> + >>> >> + snprintf(wsource_name_usb2, sizeof(wsource_name_usb2), >>> >> "vbus-%s", >>> >> + dev_name(otgws_xceiv_usb2->dev)); >>> >> + wakeup_source_init(&otgws_xceiv_usb2->wsource, >>> >> wsource_name_usb2); >>> >> + >>> >> + snprintf(wsource_name_usb3, sizeof(wsource_name_usb3), >>> >> "vbus-%s", >>> >> + dev_name(otgws_xceiv_usb3->dev)); >>> >> + wakeup_source_init(&otgws_xceiv_usb3->wsource, >>> >> wsource_name_usb3); >>> >> + >>> >> + otgws_xceiv_usb2->otgws_nb.notifier_call = >>> >> otgws_otg_usb2_notifications; >>> >> + ret_usb2 = usb_register_notifier(otgws_xceiv_usb2, >>> >> + &otgws_xceiv_usb2->otgws_nb); >>> >> + >>> >> + otgws_xceiv_usb3->otgws_nb.notifier_call = >>> >> otgws_otg_usb3_notifications; >>> >> + ret_usb3 = usb_register_notifier(otgws_xceiv_usb3, >>> >> + &otgws_xceiv_usb3->otgws_nb); >>> >> + >>> >> + if (ret_usb2 && ret_usb3) { >>> >> + pr_err("%s: usb_register_notifier on transceiver >>> >> failed\n", >>> >> + __func__); >>> >> + wakeup_source_trash(&otgws_xceiv_usb2->wsource); >>> >> + wakeup_source_trash(&otgws_xceiv_usb3->wsource); >>> >> + otgws_xceiv_usb2 = NULL; >>> >> + otgws_xceiv_usb3 = NULL; >>> >> + return ret_usb2 | ret_usb3; >>> >> + } >>> >> + >>> >> + return 0; >>> >> +} >>> >> + >>> >> +late_initcall(otg_wakeupsource_init); >>> > >>> > you guys are really not getting what I mean. I asked for this to be >>> > built into the core itself. This means that you shouldn't need to use >>> > notifications nor should you need to call usb_get_phy(). You're part of >>> > the PHY framework. >>> > >>> > All this late_initcall() nonsense should go. >>> > >>> > This code won't even work if we have more than one phy of the same type >>> > (AM437x SoC, for example, has up to 4 instances of dwc3, so that's 4 >>> > USB2 PHYs), because you can't grab the PHY you want. >>> >>> Apologies,I am new to usb sub system,so i missed this point before i >>> posted my patch,Thanks for the information. >> >> np. >> >>> > What you need is to: >>> > >>> > 1) make PHY notifiers generic (move all of that phy-core.c) >>> From the above points,you mentioned that "if we built it into core,we >>> shouldn't need to use notifications" >>> and your first point here says that make phy notifiers generic in >>> phy-core.c >>> can you help me understanding it better so that there wont be any >>> understanding gap. >> >> yeah, notifiers should go but if you really must use them, then at least >> make all of that generic ;-) >> >>> > 2) introduce usb_phy_set_event(phy, event) (which just sets the even to >>> > a >>> > phy->event member for now) >>> > 3) make all PHY drivers use usb_phy_set_event() >>> > 4) add the following to usb_phy_set_event() >>> > >>> > switch (event) { >>> > case USB_EVENT_ENUMERATED: >>> > pm_stay_awake(&otgws_xceiv->wsource); >>> > break; >>> > >>> > case USB_EVENT_NONE: >>> > case USB_EVENT_VBUS: >>> > case USB_EVENT_ID: >>> > case USB_EVENT_CHARGER: >>> > pm_wakeup_event(&otgws_xceiv->wsource, >>> > msecs_to_jiffies(TEMPORARY_HOLD_TIME)); >>> > break; >>> > >>> > default: >>> > break; >>> > } >>> > >>> Once the phy drivers receives per-PHY event notification(if we use >>> notifier,else "for any event") we can call usb_phy_set_event from phy >>> driver to hold the wakeup source. >>> Please correct me if my understanding is incorrect. >> >> yeah. In fact, you can call usb_phy_set_event() directly from PHY's IRQ >> handler. >> >>> I have gone through some phy drivers in drivers/phy,since the each >>> driver implementation is different from others, i didn't get the best >>> place in PHY driver >>> where we can trigger(use phy-core functionality) per-PHY notifier >>> registration. any pointers here? >> >> registration ? probe(), they all have probe() functions. Now to figure >> out where to call usb_phy_set_event(). That's something completely >> different, and that's where the core of this change is :-) >> >> For PHYs which have IRQ lines, easy: just call usb_phy_set_event() from >> IRQ handler. For those who don't, then it's a little more difficult and >> will require your investigation. > just a gentle reminder, can you have a look at below points and share your thoughts? > I am following below approach,please suggest/correct me, if you feel > something is wrong. > > 1. Adding usb_phy_set_event function in drivers/usb/phy/phy.c(you asked me > to add this in phy-core.c but i felt phy.c is right place to add this > function). > 2. Also add usb_phy_wsource_init and usb_phy_wsource_trash in phy.c so that > PHY drivers can use these interfaces to initialize and trash per-PHY > wakeupsource. > 3. call usb_phy_wsource_init from probe and usb_phy_wsource_trash from probe > and xxx_remove functions respectively in PHY drivers. > 4. call usb_phy_set_event from PHY drivers where complete USB enumeration(as > peripheral) event is handled(not simply on VBUS detection). > 5. As of now,i am not using any notification mechanism. > > Below are some issues for which i need your suggestions: > 1. In previous patches(till V3), i have used "enabled" field which is a > module param(/sys/module/otg_wakeupsource/parameters/enabled) to disallow > "holding wakeupsource". > > your comment for the above field was "This shouldn't be kernel > parameter" and you asked me to drop it,Just wanted to understand whether you > want me to drop it completely > or implement it per-PHY(if we need to implement it per-PHY,we may have > to add module param entry in each PHY driver which leads to N number of > sysfs entries). > > If you still prefers to have this functionality, can we use single > "enabled" field for all PHY's?(to avoid N number of sysfs entries,just > wanted to check if this is okay?) > If you want me to remove this field completely,then i can make change > accordingly to my patch.Please clarify. > > 2. I have gone through all the PHY drivers,but i could able to change only 6 > drivers to use wakeup source mechanism(call usb_phy_set_event after USB > enumerated in peripheral > mode).Is it okay if i submit the patch modifying only 6 PHY drivers as > initial patch? > > I have classified as below based on my observations(please let me know > if you have any suggestions). > > I have modified below phy drivers to use wakeupsource mechanism: > drivers/phy/phy-omap-control.c > drivers/usb/phy/phy-ab8500-usb.c > phy-gpio-vbus-usb.c > drivers/usb/phy/phy-tahvo.c > drivers/usb/phy/phy-mv-usb.c > drivers/usb/phy/phy-gpio-vbus-usb.c > > For below phy drivers,no PHY events(Enumeration in peripheral mode) are > handled in driver > drivers/phy/phy-bcm-kona-usb2.c > drivers/phy/phy-exynos4210-usb2.c > drivers/phy/phy-exynos4x12-usb2.c > drivers/phy/phy-exynos5250-sata.c > drivers/phy/phy-exynos5-usbdrd.c > drivers/phy/phy-exynos-dp-video.c > drivers/phy/phy-exynos-mipi-video.c > drivers/phy/phy-mvebu-sata.c > drivers/phy/phy-samsung-usb2.c > drivers/phy/phy-sun4i-usb.c > drivers/phy/phy-ti-pipe3.c > drivers/phy/phy-xgene.c > drivers/phy/phy-omap-usb2.c > drivers/phy/phy-twl4030-usb.c > drivers/usb/phy/phy-am335x.c > drivers/usb/phy/phy-am335x-control.c > drivers/usb/phy/phy-generic.c > drivers/usb/phy/phy-isp1301.c > drivers/usb/phy/phy-rcar-gen2-usb.c > drivers/usb/phy/phy-rcar-usb.c > drivers/usb/phy/phy-samsung-usb2.c > drivers/usb/phy/phy-samsung-usb3.c > drivers/usb/phy/phy-samsung-usb.c > drivers/usb/phy/phy-tegra-usb.c > drivers/usb/phy/phy-ulpi-viewport.c > drivers/usb/phy/phy-keystone.c > drivers/usb/phy/phy-mxs-usb.c > drivers/usb/phy-omap-otg.c > drivers/usb/phy/phy-ulpi.c > > For below PHY driver,disconnect event not handled,so we can't hold > wakeupsource. > drivers/usb/phy/phy-fsl-usb.c > > For below PHY driver,only VBUS events are handled(Enumeration event not > handled). > drivers/usb/phy/phy-twl6030-usb.c > > For below PHY drivers,i am not clear where to call usb_phy_set_event > drivers/usb/phy/phy-isp1301-omap.c > drivers/usb/phy/phy-msm-usb.c > > Regards, Kiran > Regards, > Kiran >> >> -- >> balbi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Hi, On Fri, Oct 31, 2014 at 09:27:43AM +0530, Kiran Raparthy wrote: > >>> Thank you very much for taking time in reviewing the patch. > >>> I will try to improve the patch as per your suggestions. > >>> however,i have few queries which i wanted to understand from you. > >> > >> sure thing. > >> > >>> On 7 October 2014 19:55, Felipe Balbi <balbi@ti.com> wrote: > >>> >> +static int otg_wakeupsource_init(void) > >>> >> +{ > >>> >> + int ret_usb2; > >>> >> + int ret_usb3; > >>> >> + char wsource_name_usb2[40]; > >>> >> + char wsource_name_usb3[40]; > >>> >> + static struct usb_phy *otgws_xceiv_usb2; > >>> >> + static struct usb_phy *otgws_xceiv_usb3; > >>> >> + > >>> >> + otgws_xceiv_usb2 = usb_get_phy(USB_PHY_TYPE_USB2); > >>> >> + otgws_xceiv_usb3 = usb_get_phy(USB_PHY_TYPE_USB3); > >>> >> + > >>> >> + if (IS_ERR(otgws_xceiv_usb2) && IS_ERR(otgws_xceiv_usb3)) { > >>> >> + pr_err("%s: No OTG transceiver found\n", __func__); > >>> >> + return PTR_ERR(otgws_xceiv_usb2); > >>> >> + } > >>> >> + > >>> >> + spin_lock_init(&otgws_xceiv_usb2->otgws_slock); > >>> >> + spin_lock_init(&otgws_xceiv_usb3->otgws_slock); > >>> >> + > >>> >> + snprintf(wsource_name_usb2, sizeof(wsource_name_usb2), > >>> >> "vbus-%s", > >>> >> + dev_name(otgws_xceiv_usb2->dev)); > >>> >> + wakeup_source_init(&otgws_xceiv_usb2->wsource, > >>> >> wsource_name_usb2); > >>> >> + > >>> >> + snprintf(wsource_name_usb3, sizeof(wsource_name_usb3), > >>> >> "vbus-%s", > >>> >> + dev_name(otgws_xceiv_usb3->dev)); > >>> >> + wakeup_source_init(&otgws_xceiv_usb3->wsource, > >>> >> wsource_name_usb3); > >>> >> + > >>> >> + otgws_xceiv_usb2->otgws_nb.notifier_call = > >>> >> otgws_otg_usb2_notifications; > >>> >> + ret_usb2 = usb_register_notifier(otgws_xceiv_usb2, > >>> >> + &otgws_xceiv_usb2->otgws_nb); > >>> >> + > >>> >> + otgws_xceiv_usb3->otgws_nb.notifier_call = > >>> >> otgws_otg_usb3_notifications; > >>> >> + ret_usb3 = usb_register_notifier(otgws_xceiv_usb3, > >>> >> + &otgws_xceiv_usb3->otgws_nb); > >>> >> + > >>> >> + if (ret_usb2 && ret_usb3) { > >>> >> + pr_err("%s: usb_register_notifier on transceiver > >>> >> failed\n", > >>> >> + __func__); > >>> >> + wakeup_source_trash(&otgws_xceiv_usb2->wsource); > >>> >> + wakeup_source_trash(&otgws_xceiv_usb3->wsource); > >>> >> + otgws_xceiv_usb2 = NULL; > >>> >> + otgws_xceiv_usb3 = NULL; > >>> >> + return ret_usb2 | ret_usb3; > >>> >> + } > >>> >> + > >>> >> + return 0; > >>> >> +} > >>> >> + > >>> >> +late_initcall(otg_wakeupsource_init); > >>> > > >>> > you guys are really not getting what I mean. I asked for this to be > >>> > built into the core itself. This means that you shouldn't need to use > >>> > notifications nor should you need to call usb_get_phy(). You're part of > >>> > the PHY framework. > >>> > > >>> > All this late_initcall() nonsense should go. > >>> > > >>> > This code won't even work if we have more than one phy of the same type > >>> > (AM437x SoC, for example, has up to 4 instances of dwc3, so that's 4 > >>> > USB2 PHYs), because you can't grab the PHY you want. > >>> > >>> Apologies,I am new to usb sub system,so i missed this point before i > >>> posted my patch,Thanks for the information. > >> > >> np. > >> > >>> > What you need is to: > >>> > > >>> > 1) make PHY notifiers generic (move all of that phy-core.c) > >>> From the above points,you mentioned that "if we built it into core,we > >>> shouldn't need to use notifications" > >>> and your first point here says that make phy notifiers generic in > >>> phy-core.c > >>> can you help me understanding it better so that there wont be any > >>> understanding gap. > >> > >> yeah, notifiers should go but if you really must use them, then at least > >> make all of that generic ;-) > >> > >>> > 2) introduce usb_phy_set_event(phy, event) (which just sets the even to > >>> > a > >>> > phy->event member for now) > >>> > 3) make all PHY drivers use usb_phy_set_event() > >>> > 4) add the following to usb_phy_set_event() > >>> > > >>> > switch (event) { > >>> > case USB_EVENT_ENUMERATED: > >>> > pm_stay_awake(&otgws_xceiv->wsource); > >>> > break; > >>> > > >>> > case USB_EVENT_NONE: > >>> > case USB_EVENT_VBUS: > >>> > case USB_EVENT_ID: > >>> > case USB_EVENT_CHARGER: > >>> > pm_wakeup_event(&otgws_xceiv->wsource, > >>> > msecs_to_jiffies(TEMPORARY_HOLD_TIME)); > >>> > break; > >>> > > >>> > default: > >>> > break; > >>> > } > >>> > > >>> Once the phy drivers receives per-PHY event notification(if we use > >>> notifier,else "for any event") we can call usb_phy_set_event from phy > >>> driver to hold the wakeup source. > >>> Please correct me if my understanding is incorrect. > >> > >> yeah. In fact, you can call usb_phy_set_event() directly from PHY's IRQ > >> handler. > >> > >>> I have gone through some phy drivers in drivers/phy,since the each > >>> driver implementation is different from others, i didn't get the best > >>> place in PHY driver > >>> where we can trigger(use phy-core functionality) per-PHY notifier > >>> registration. any pointers here? > >> > >> registration ? probe(), they all have probe() functions. Now to figure > >> out where to call usb_phy_set_event(). That's something completely > >> different, and that's where the core of this change is :-) > >> > >> For PHYs which have IRQ lines, easy: just call usb_phy_set_event() from > >> IRQ handler. For those who don't, then it's a little more difficult and > >> will require your investigation. > > > just a gentle reminder, can you have a look at below points and share > your thoughts? Send the patch, I have reviewed this multiple times and all those comments are archived on multiple mailing list archives. Just read them again if you need.
diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig index e253fa0..d9ddd85 100644 --- a/drivers/usb/phy/Kconfig +++ b/drivers/usb/phy/Kconfig @@ -6,6 +6,14 @@ menu "USB Physical Layer drivers" config USB_PHY def_bool n +config USB_OTG_WAKEUPSOURCE + bool "Hold wakeupsource when USB is enumerated in peripheral mode" + depends on PM_SLEEP + select USB_PHY + help + Prevent the system going into automatic suspend while + it is attached as a USB peripheral by holding a wakeupsource. + # # USB Transceiver Drivers # diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile index 24a9133..ca2fbaf 100644 --- a/drivers/usb/phy/Makefile +++ b/drivers/usb/phy/Makefile @@ -3,6 +3,7 @@ # obj-$(CONFIG_USB_PHY) += phy.o obj-$(CONFIG_OF) += of.o +obj-$(CONFIG_USB_OTG_WAKEUPSOURCE) += otg-wakeupsource.o # transceiver drivers, keep the list sorted diff --git a/drivers/usb/phy/otg-wakeupsource.c b/drivers/usb/phy/otg-wakeupsource.c new file mode 100644 index 0000000..00d3359 --- /dev/null +++ b/drivers/usb/phy/otg-wakeupsource.c @@ -0,0 +1,134 @@ +/* + * otg-wakeupsource.c + * + * Copyright (C) 2011 Google, Inc. + * + * This software is licensed under the terms of the GNU General Public + * License version 2, as published by the Free Software Foundation, and + * may be copied, distributed, and modified under those terms. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ + +#include <linux/kernel.h> +#include <linux/device.h> +#include <linux/module.h> +#include <linux/notifier.h> +#include <linux/pm_wakeup.h> +#include <linux/spinlock.h> +#include <linux/usb/otg.h> + +static void otgws_handle_event(struct usb_phy *otgws_xceiv, unsigned long event) +{ + unsigned long irqflags; + + spin_lock_irqsave(&otgws_xceiv->otgws_slock, irqflags); + + switch (event) { + case USB_EVENT_VBUS: + case USB_EVENT_ENUMERATED: + __pm_stay_awake(&otgws_xceiv->wsource); + break; + + case USB_EVENT_NONE: + case USB_EVENT_ID: + case USB_EVENT_CHARGER: + __pm_wakeup_event(&otgws_xceiv->wsource, + msecs_to_jiffies(TEMPORARY_HOLD_TIME)); + break; + + default: + break; + } + + spin_unlock_irqrestore(&otgws_xceiv->otgws_slock, irqflags); +} + +static int otgws_otg_usb2_notifications(struct notifier_block *nb, + unsigned long event, void *unused) +{ + static struct usb_phy *otgws_xceiv; + + otgws_xceiv = usb_get_phy(USB_PHY_TYPE_USB2); + + if (IS_ERR(otgws_xceiv)) { + pr_err("%s: No OTG transceiver found\n", __func__); + return PTR_ERR(otgws_xceiv); + } + + otgws_handle_event(otgws_xceiv, event); + + return NOTIFY_OK; +} + +static int otgws_otg_usb3_notifications(struct notifier_block *nb, + unsigned long event, void *unused) +{ + static struct usb_phy *otgws_xceiv; + + otgws_xceiv = usb_get_phy(USB_PHY_TYPE_USB3); + + if (IS_ERR(otgws_xceiv)) { + pr_err("%s: No OTG transceiver found\n", __func__); + return PTR_ERR(otgws_xceiv); + } + + otgws_handle_event(otgws_xceiv, event); + + return NOTIFY_OK; +} + +static int otg_wakeupsource_init(void) +{ + int ret_usb2; + int ret_usb3; + char wsource_name_usb2[40]; + char wsource_name_usb3[40]; + static struct usb_phy *otgws_xceiv_usb2; + static struct usb_phy *otgws_xceiv_usb3; + + otgws_xceiv_usb2 = usb_get_phy(USB_PHY_TYPE_USB2); + otgws_xceiv_usb3 = usb_get_phy(USB_PHY_TYPE_USB3); + + if (IS_ERR(otgws_xceiv_usb2) && IS_ERR(otgws_xceiv_usb3)) { + pr_err("%s: No OTG transceiver found\n", __func__); + return PTR_ERR(otgws_xceiv_usb2); + } + + spin_lock_init(&otgws_xceiv_usb2->otgws_slock); + spin_lock_init(&otgws_xceiv_usb3->otgws_slock); + + snprintf(wsource_name_usb2, sizeof(wsource_name_usb2), "vbus-%s", + dev_name(otgws_xceiv_usb2->dev)); + wakeup_source_init(&otgws_xceiv_usb2->wsource, wsource_name_usb2); + + snprintf(wsource_name_usb3, sizeof(wsource_name_usb3), "vbus-%s", + dev_name(otgws_xceiv_usb3->dev)); + wakeup_source_init(&otgws_xceiv_usb3->wsource, wsource_name_usb3); + + otgws_xceiv_usb2->otgws_nb.notifier_call = otgws_otg_usb2_notifications; + ret_usb2 = usb_register_notifier(otgws_xceiv_usb2, + &otgws_xceiv_usb2->otgws_nb); + + otgws_xceiv_usb3->otgws_nb.notifier_call = otgws_otg_usb3_notifications; + ret_usb3 = usb_register_notifier(otgws_xceiv_usb3, + &otgws_xceiv_usb3->otgws_nb); + + if (ret_usb2 && ret_usb3) { + pr_err("%s: usb_register_notifier on transceiver failed\n", + __func__); + wakeup_source_trash(&otgws_xceiv_usb2->wsource); + wakeup_source_trash(&otgws_xceiv_usb3->wsource); + otgws_xceiv_usb2 = NULL; + otgws_xceiv_usb3 = NULL; + return ret_usb2 | ret_usb3; + } + + return 0; +} + +late_initcall(otg_wakeupsource_init); diff --git a/include/linux/usb/phy.h b/include/linux/usb/phy.h index 353053a..dd64e2e 100644 --- a/include/linux/usb/phy.h +++ b/include/linux/usb/phy.h @@ -12,6 +12,8 @@ #include <linux/notifier.h> #include <linux/usb.h> +#define TEMPORARY_HOLD_TIME 2000 + enum usb_phy_interface { USBPHY_INTERFACE_MODE_UNKNOWN, USBPHY_INTERFACE_MODE_UTMI, @@ -88,6 +90,12 @@ struct usb_phy { /* for notification of usb_phy_events */ struct atomic_notifier_head notifier; + struct notifier_block otgws_nb; + + /* wakeup source */ + struct wakeup_source wsource; + + spinlock_t otgws_slock; /* to pass extra port status to the root hub */ u16 port_status;