| Message ID | 1495068543-6938-5-git-send-email-peter.chen@nxp.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Thu, May 18, 2017 at 08:49:00AM +0800, Peter Chen wrote: > Some hard-wired USB devices need to do power sequence to let the > device work normally, the typical power sequence like: enable USB > PHY clock, toggle reset pin, etc. But current Linux USB driver > lacks of such code to do it, it may cause some hard-wired USB devices > works abnormal or can't be recognized by controller at all. > > In this patch, it calls power sequence library APIs to finish > the power sequence events. It will do power on sequence at hub's > probe for all devices under this hub (includes root hub). > At hub_disconnect, it will do power off sequence which is at powered > on list. > Greg, Alan, would you please help on reviewing it? It seems Rafael is waiting for USB MAINTAINERS's comments or ack. It resolves some USB HUB issues for several persons. Peter > Signed-off-by: Peter Chen <peter.chen@nxp.com> > Tested-by Joshua Clayton <stillcompiling@gmail.com> > Tested-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name> > Reviewed-by: Vaibhav Hiremath <hvaibhav.linux@gmail.com> > --- > drivers/usb/Kconfig | 1 + > drivers/usb/core/hub.c | 49 +++++++++++++++++++++++++++++++++++++++++++++---- > drivers/usb/core/hub.h | 1 + > 3 files changed, 47 insertions(+), 4 deletions(-) > > diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig > index 939a63b..b6f626e 100644 > --- a/drivers/usb/Kconfig > +++ b/drivers/usb/Kconfig > @@ -39,6 +39,7 @@ config USB > tristate "Support for Host-side USB" > depends on USB_ARCH_HAS_HCD > select USB_COMMON > + select POWER_SEQUENCE > select NLS # for UTF-8 strings > ---help--- > Universal Serial Bus (USB) is a specification for a serial bus > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c > index 9dca59e..7a67296 100644 > --- a/drivers/usb/core/hub.c > +++ b/drivers/usb/core/hub.c > @@ -28,6 +28,7 @@ > #include <linux/mutex.h> > #include <linux/random.h> > #include <linux/pm_qos.h> > +#include <linux/power/pwrseq.h> > > #include <linux/uaccess.h> > #include <asm/byteorder.h> > @@ -1619,6 +1620,7 @@ static void hub_disconnect(struct usb_interface *intf) > hub->error = 0; > hub_quiesce(hub, HUB_DISCONNECT); > > + of_pwrseq_off_list(&hub->pwrseq_on_list); > mutex_lock(&usb_port_peer_mutex); > > /* Avoid races with recursively_mark_NOTATTACHED() */ > @@ -1646,12 +1648,42 @@ static void hub_disconnect(struct usb_interface *intf) > kref_put(&hub->kref, hub_release); > } > > +#ifdef CONFIG_OF > +static int hub_of_pwrseq_on(struct usb_hub *hub) > +{ > + struct device *parent; > + struct usb_device *hdev = hub->hdev; > + struct device_node *np; > + int ret; > + > + if (hdev->parent) > + parent = &hdev->dev; > + else > + parent = bus_to_hcd(hdev->bus)->self.sysdev; > + > + for_each_child_of_node(parent->of_node, np) { > + ret = of_pwrseq_on_list(np, &hub->pwrseq_on_list); > + /* Maybe no power sequence library is chosen */ > + if (ret && ret != -ENOENT) > + return ret; > + } > + > + return 0; > +} > +#else > +static int hub_of_pwrseq_on(struct usb_hub *hub) > +{ > + return 0; > +} > +#endif > + > static int hub_probe(struct usb_interface *intf, const struct usb_device_id *id) > { > struct usb_host_interface *desc; > struct usb_endpoint_descriptor *endpoint; > struct usb_device *hdev; > struct usb_hub *hub; > + int ret = -ENODEV; > > desc = intf->cur_altsetting; > hdev = interface_to_usbdev(intf); > @@ -1756,6 +1788,7 @@ static int hub_probe(struct usb_interface *intf, const struct usb_device_id *id) > INIT_DELAYED_WORK(&hub->leds, led_work); > INIT_DELAYED_WORK(&hub->init_work, NULL); > INIT_WORK(&hub->events, hub_event); > + INIT_LIST_HEAD(&hub->pwrseq_on_list); > usb_get_intf(intf); > usb_get_dev(hdev); > > @@ -1769,11 +1802,14 @@ static int hub_probe(struct usb_interface *intf, const struct usb_device_id *id) > if (id->driver_info & HUB_QUIRK_CHECK_PORT_AUTOSUSPEND) > hub->quirk_check_port_auto_suspend = 1; > > - if (hub_configure(hub, endpoint) >= 0) > - return 0; > + if (hub_configure(hub, endpoint) >= 0) { > + ret = hub_of_pwrseq_on(hub); > + if (!ret) > + return 0; > + } > > hub_disconnect(intf); > - return -ENODEV; > + return ret; > } > > static int > @@ -3593,14 +3629,19 @@ static int hub_suspend(struct usb_interface *intf, pm_message_t msg) > > /* stop hub_wq and related activity */ > hub_quiesce(hub, HUB_SUSPEND); > - return 0; > + return pwrseq_suspend_list(&hub->pwrseq_on_list); > } > > static int hub_resume(struct usb_interface *intf) > { > struct usb_hub *hub = usb_get_intfdata(intf); > + int ret; > > dev_dbg(&intf->dev, "%s\n", __func__); > + ret = pwrseq_resume_list(&hub->pwrseq_on_list); > + if (ret) > + return ret; > + > hub_activate(hub, HUB_RESUME); > return 0; > } > diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h > index 34c1a7e..cd86f91 100644 > --- a/drivers/usb/core/hub.h > +++ b/drivers/usb/core/hub.h > @@ -78,6 +78,7 @@ struct usb_hub { > struct delayed_work init_work; > struct work_struct events; > struct usb_port **ports; > + struct list_head pwrseq_on_list; /* powered pwrseq node list */ > }; > > /** > -- > 2.7.4 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Mon, 5 Jun 2017, Peter Chen wrote: > On Thu, May 18, 2017 at 08:49:00AM +0800, Peter Chen wrote: > > Some hard-wired USB devices need to do power sequence to let the > > device work normally, the typical power sequence like: enable USB > > PHY clock, toggle reset pin, etc. But current Linux USB driver > > lacks of such code to do it, it may cause some hard-wired USB devices > > works abnormal or can't be recognized by controller at all. > > > > In this patch, it calls power sequence library APIs to finish > > the power sequence events. It will do power on sequence at hub's > > probe for all devices under this hub (includes root hub). > > At hub_disconnect, it will do power off sequence which is at powered > > on list. > > > > Greg, Alan, would you please help on reviewing it? It seems Rafael > is waiting for USB MAINTAINERS's comments or ack. It resolves some > USB HUB issues for several persons. > > Peter > > > Signed-off-by: Peter Chen <peter.chen@nxp.com> > > Tested-by Joshua Clayton <stillcompiling@gmail.com> > > Tested-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name> > > Reviewed-by: Vaibhav Hiremath <hvaibhav.linux@gmail.com> Acked-by: Alan Stern <stern@rowland.harvard.edu> > > --- > > drivers/usb/Kconfig | 1 + > > drivers/usb/core/hub.c | 49 +++++++++++++++++++++++++++++++++++++++++++++---- > > drivers/usb/core/hub.h | 1 + > > 3 files changed, 47 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig > > index 939a63b..b6f626e 100644 > > --- a/drivers/usb/Kconfig > > +++ b/drivers/usb/Kconfig > > @@ -39,6 +39,7 @@ config USB > > tristate "Support for Host-side USB" > > depends on USB_ARCH_HAS_HCD > > select USB_COMMON > > + select POWER_SEQUENCE > > select NLS # for UTF-8 strings > > ---help--- > > Universal Serial Bus (USB) is a specification for a serial bus > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c > > index 9dca59e..7a67296 100644 > > --- a/drivers/usb/core/hub.c > > +++ b/drivers/usb/core/hub.c > > @@ -28,6 +28,7 @@ > > #include <linux/mutex.h> > > #include <linux/random.h> > > #include <linux/pm_qos.h> > > +#include <linux/power/pwrseq.h> > > > > #include <linux/uaccess.h> > > #include <asm/byteorder.h> > > @@ -1619,6 +1620,7 @@ static void hub_disconnect(struct usb_interface *intf) > > hub->error = 0; > > hub_quiesce(hub, HUB_DISCONNECT); > > > > + of_pwrseq_off_list(&hub->pwrseq_on_list); Things like this look a little peculiar -- you are passing an "on_list" to a function named "...off_list". How about naming the new field hub->pwrseq_list instead? Alan Stern
On Mon, Jun 05, 2017 at 10:16:34AM -0400, Alan Stern wrote: > On Mon, 5 Jun 2017, Peter Chen wrote: > > > On Thu, May 18, 2017 at 08:49:00AM +0800, Peter Chen wrote: > > > Some hard-wired USB devices need to do power sequence to let the > > > device work normally, the typical power sequence like: enable USB > > > PHY clock, toggle reset pin, etc. But current Linux USB driver > > > lacks of such code to do it, it may cause some hard-wired USB devices > > > works abnormal or can't be recognized by controller at all. > > > > > > In this patch, it calls power sequence library APIs to finish > > > the power sequence events. It will do power on sequence at hub's > > > probe for all devices under this hub (includes root hub). > > > At hub_disconnect, it will do power off sequence which is at powered > > > on list. > > > > > > > Greg, Alan, would you please help on reviewing it? It seems Rafael > > is waiting for USB MAINTAINERS's comments or ack. It resolves some > > USB HUB issues for several persons. > > > > Peter > > > > > Signed-off-by: Peter Chen <peter.chen@nxp.com> > > > Tested-by Joshua Clayton <stillcompiling@gmail.com> > > > Tested-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name> > > > Reviewed-by: Vaibhav Hiremath <hvaibhav.linux@gmail.com> > > Acked-by: Alan Stern <stern@rowland.harvard.edu> > > > > --- > > > drivers/usb/Kconfig | 1 + > > > drivers/usb/core/hub.c | 49 +++++++++++++++++++++++++++++++++++++++++++++---- > > > drivers/usb/core/hub.h | 1 + > > > 3 files changed, 47 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig > > > index 939a63b..b6f626e 100644 > > > --- a/drivers/usb/Kconfig > > > +++ b/drivers/usb/Kconfig > > > @@ -39,6 +39,7 @@ config USB > > > tristate "Support for Host-side USB" > > > depends on USB_ARCH_HAS_HCD > > > select USB_COMMON > > > + select POWER_SEQUENCE > > > select NLS # for UTF-8 strings > > > ---help--- > > > Universal Serial Bus (USB) is a specification for a serial bus > > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c > > > index 9dca59e..7a67296 100644 > > > --- a/drivers/usb/core/hub.c > > > +++ b/drivers/usb/core/hub.c > > > @@ -28,6 +28,7 @@ > > > #include <linux/mutex.h> > > > #include <linux/random.h> > > > #include <linux/pm_qos.h> > > > +#include <linux/power/pwrseq.h> > > > > > > #include <linux/uaccess.h> > > > #include <asm/byteorder.h> > > > @@ -1619,6 +1620,7 @@ static void hub_disconnect(struct usb_interface *intf) > > > hub->error = 0; > > > hub_quiesce(hub, HUB_DISCONNECT); > > > > > > + of_pwrseq_off_list(&hub->pwrseq_on_list); > > Things like this look a little peculiar -- you are passing an "on_list" > to a function named "...off_list". > > How about naming the new field hub->pwrseq_list instead? > Thanks, Alan. I will change it at next revision.
diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig index 939a63b..b6f626e 100644 --- a/drivers/usb/Kconfig +++ b/drivers/usb/Kconfig @@ -39,6 +39,7 @@ config USB tristate "Support for Host-side USB" depends on USB_ARCH_HAS_HCD select USB_COMMON + select POWER_SEQUENCE select NLS # for UTF-8 strings ---help--- Universal Serial Bus (USB) is a specification for a serial bus diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 9dca59e..7a67296 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -28,6 +28,7 @@ #include <linux/mutex.h> #include <linux/random.h> #include <linux/pm_qos.h> +#include <linux/power/pwrseq.h> #include <linux/uaccess.h> #include <asm/byteorder.h> @@ -1619,6 +1620,7 @@ static void hub_disconnect(struct usb_interface *intf) hub->error = 0; hub_quiesce(hub, HUB_DISCONNECT); + of_pwrseq_off_list(&hub->pwrseq_on_list); mutex_lock(&usb_port_peer_mutex); /* Avoid races with recursively_mark_NOTATTACHED() */ @@ -1646,12 +1648,42 @@ static void hub_disconnect(struct usb_interface *intf) kref_put(&hub->kref, hub_release); } +#ifdef CONFIG_OF +static int hub_of_pwrseq_on(struct usb_hub *hub) +{ + struct device *parent; + struct usb_device *hdev = hub->hdev; + struct device_node *np; + int ret; + + if (hdev->parent) + parent = &hdev->dev; + else + parent = bus_to_hcd(hdev->bus)->self.sysdev; + + for_each_child_of_node(parent->of_node, np) { + ret = of_pwrseq_on_list(np, &hub->pwrseq_on_list); + /* Maybe no power sequence library is chosen */ + if (ret && ret != -ENOENT) + return ret; + } + + return 0; +} +#else +static int hub_of_pwrseq_on(struct usb_hub *hub) +{ + return 0; +} +#endif + static int hub_probe(struct usb_interface *intf, const struct usb_device_id *id) { struct usb_host_interface *desc; struct usb_endpoint_descriptor *endpoint; struct usb_device *hdev; struct usb_hub *hub; + int ret = -ENODEV; desc = intf->cur_altsetting; hdev = interface_to_usbdev(intf); @@ -1756,6 +1788,7 @@ static int hub_probe(struct usb_interface *intf, const struct usb_device_id *id) INIT_DELAYED_WORK(&hub->leds, led_work); INIT_DELAYED_WORK(&hub->init_work, NULL); INIT_WORK(&hub->events, hub_event); + INIT_LIST_HEAD(&hub->pwrseq_on_list); usb_get_intf(intf); usb_get_dev(hdev); @@ -1769,11 +1802,14 @@ static int hub_probe(struct usb_interface *intf, const struct usb_device_id *id) if (id->driver_info & HUB_QUIRK_CHECK_PORT_AUTOSUSPEND) hub->quirk_check_port_auto_suspend = 1; - if (hub_configure(hub, endpoint) >= 0) - return 0; + if (hub_configure(hub, endpoint) >= 0) { + ret = hub_of_pwrseq_on(hub); + if (!ret) + return 0; + } hub_disconnect(intf); - return -ENODEV; + return ret; } static int @@ -3593,14 +3629,19 @@ static int hub_suspend(struct usb_interface *intf, pm_message_t msg) /* stop hub_wq and related activity */ hub_quiesce(hub, HUB_SUSPEND); - return 0; + return pwrseq_suspend_list(&hub->pwrseq_on_list); } static int hub_resume(struct usb_interface *intf) { struct usb_hub *hub = usb_get_intfdata(intf); + int ret; dev_dbg(&intf->dev, "%s\n", __func__); + ret = pwrseq_resume_list(&hub->pwrseq_on_list); + if (ret) + return ret; + hub_activate(hub, HUB_RESUME); return 0; } diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h index 34c1a7e..cd86f91 100644 --- a/drivers/usb/core/hub.h +++ b/drivers/usb/core/hub.h @@ -78,6 +78,7 @@ struct usb_hub { struct delayed_work init_work; struct work_struct events; struct usb_port **ports; + struct list_head pwrseq_on_list; /* powered pwrseq node list */ }; /**