Re: [PATCH v6 4/8] usb: core: add power sequence handling for USB devices

[Peter Chen]

On Thu, Sep 01, 2016 at 01:32:49PM +0530, Vaibhav Hiremath wrote:
> 
> 
> On Monday 15 August 2016 02:43 PM, 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. At first, it calls pwrseq_alloc_generic
> >to create a generic power sequence instance, then 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.
> >
> >Signed-off-by: Peter Chen 
> >Tested-by Joshua Clayton 
> >---
> >  drivers/usb/core/Makefile |   1 +
> >  drivers/usb/core/hub.c|  12 --
> >  drivers/usb/core/hub.h|  12 ++
> >  drivers/usb/core/pwrseq.c | 100 
> > ++
> >  4 files changed, 122 insertions(+), 3 deletions(-)
> >  create mode 100644 drivers/usb/core/pwrseq.c
> >
> >diff --git a/drivers/usb/core/Makefile b/drivers/usb/core/Makefile
> >index 9780877..39f2149 100644
> >--- a/drivers/usb/core/Makefile
> >+++ b/drivers/usb/core/Makefile
> >@@ -9,5 +9,6 @@ usbcore-y += port.o of.o
> >  usbcore-$(CONFIG_PCI)  += hcd-pci.o
> >  usbcore-$(CONFIG_ACPI) += usb-acpi.o
> >+usbcore-$(CONFIG_PWRSEQ_GENERIC) += pwrseq.o
> >  obj-$(CONFIG_USB)  += usbcore.o
> >diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> >index bee1351..a346a8b 100644
> >--- a/drivers/usb/core/hub.c
> >+++ b/drivers/usb/core/hub.c
> >@@ -1700,6 +1700,7 @@ static void hub_disconnect(struct usb_interface *intf)
> > hub->error = 0;
> > hub_quiesce(hub, HUB_DISCONNECT);
> >+hub_pwrseq_off(hub);
> > mutex_lock(_port_peer_mutex);
> > /* Avoid races with recursively_mark_NOTATTACHED() */
> >@@ -1733,6 +1734,7 @@ static int hub_probe(struct usb_interface *intf, const 
> >struct usb_device_id *id)
> > 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);
> >@@ -1839,6 +1841,7 @@ descriptor_error:
> > INIT_DELAYED_WORK(>leds, led_work);
> > INIT_DELAYED_WORK(>init_work, NULL);
> > INIT_WORK(>events, hub_event);
> >+INIT_LIST_HEAD(>pwrseq_on_list);
> > usb_get_intf(intf);
> > usb_get_dev(hdev);
> >@@ -1852,11 +1855,14 @@ descriptor_error:
> > 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_pwrseq_on(hub);
> >+if (!ret)
> >+return 0;
> >+}
> > hub_disconnect(intf);
> >-return -ENODEV;
> >+return ret;
> >  }
> >  static int
> >diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
> >index 34c1a7e..9473f6f 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_headpwrseq_on_list; /* powered pwrseq node list */
> >  };
> >  /**
> >@@ -166,3 +167,14 @@ static inline int hub_port_debounce_be_stable(struct 
> >usb_hub *hub,
> >  {
> > return hub_port_debounce(hub, port1, false);
> >  }
> >+
> >+#if IS_ENABLED(CONFIG_PWRSEQ_GENERIC)
> >+int hub_pwrseq_on(struct usb_hub *hub);
> >+void hub_pwrseq_off(struct usb_hub *hub);
> >+#else
> >+static inline int hub_pwrseq_on(struct usb_hub *hub)
> >+{
> >+return 0;
> >+}
> >+static inline void hub_pwrseq_off(struct usb_hub *hub) {}
> >+#endif /* CONFIG_PWRSEQ_GENERIC */
> >diff --git a/drivers/usb/core/pwrseq.c b/drivers/usb/core/pwrseq.c
> >new file mode 100644
> >index 000..837fe66
> >--- /dev/null
> >+++ b/drivers/usb/core/pwrseq.c
> >@@ -0,0 +1,100 @@
> >+/*
> >+ * pwrseq.c USB device power sequence management
> >+ *
> >+ * Copyright (C) 2016 Freescale Semiconductor, Inc.
> >+ * Author: Peter Chen 
> >+ *
> >+ * This program is free software: you can redistribute it and/or modify
> >+ * it under the terms of the GNU General Public License version 2  of
> >+ * the License as published by the Free Software Foundation.
> >+ *
> >+ * 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 

Re: [PATCH v6 4/8] usb: core: add power sequence handling for USB devices

[Peter Chen]

On Thu, Sep 01, 2016 at 01:32:49PM +0530, Vaibhav Hiremath wrote:
> 
> 
> On Monday 15 August 2016 02:43 PM, 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. At first, it calls pwrseq_alloc_generic
> >to create a generic power sequence instance, then 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.
> >
> >Signed-off-by: Peter Chen 
> >Tested-by Joshua Clayton 
> >---
> >  drivers/usb/core/Makefile |   1 +
> >  drivers/usb/core/hub.c|  12 --
> >  drivers/usb/core/hub.h|  12 ++
> >  drivers/usb/core/pwrseq.c | 100 
> > ++
> >  4 files changed, 122 insertions(+), 3 deletions(-)
> >  create mode 100644 drivers/usb/core/pwrseq.c
> >
> >diff --git a/drivers/usb/core/Makefile b/drivers/usb/core/Makefile
> >index 9780877..39f2149 100644
> >--- a/drivers/usb/core/Makefile
> >+++ b/drivers/usb/core/Makefile
> >@@ -9,5 +9,6 @@ usbcore-y += port.o of.o
> >  usbcore-$(CONFIG_PCI)  += hcd-pci.o
> >  usbcore-$(CONFIG_ACPI) += usb-acpi.o
> >+usbcore-$(CONFIG_PWRSEQ_GENERIC) += pwrseq.o
> >  obj-$(CONFIG_USB)  += usbcore.o
> >diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> >index bee1351..a346a8b 100644
> >--- a/drivers/usb/core/hub.c
> >+++ b/drivers/usb/core/hub.c
> >@@ -1700,6 +1700,7 @@ static void hub_disconnect(struct usb_interface *intf)
> > hub->error = 0;
> > hub_quiesce(hub, HUB_DISCONNECT);
> >+hub_pwrseq_off(hub);
> > mutex_lock(_port_peer_mutex);
> > /* Avoid races with recursively_mark_NOTATTACHED() */
> >@@ -1733,6 +1734,7 @@ static int hub_probe(struct usb_interface *intf, const 
> >struct usb_device_id *id)
> > 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);
> >@@ -1839,6 +1841,7 @@ descriptor_error:
> > INIT_DELAYED_WORK(>leds, led_work);
> > INIT_DELAYED_WORK(>init_work, NULL);
> > INIT_WORK(>events, hub_event);
> >+INIT_LIST_HEAD(>pwrseq_on_list);
> > usb_get_intf(intf);
> > usb_get_dev(hdev);
> >@@ -1852,11 +1855,14 @@ descriptor_error:
> > 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_pwrseq_on(hub);
> >+if (!ret)
> >+return 0;
> >+}
> > hub_disconnect(intf);
> >-return -ENODEV;
> >+return ret;
> >  }
> >  static int
> >diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
> >index 34c1a7e..9473f6f 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_headpwrseq_on_list; /* powered pwrseq node list */
> >  };
> >  /**
> >@@ -166,3 +167,14 @@ static inline int hub_port_debounce_be_stable(struct 
> >usb_hub *hub,
> >  {
> > return hub_port_debounce(hub, port1, false);
> >  }
> >+
> >+#if IS_ENABLED(CONFIG_PWRSEQ_GENERIC)
> >+int hub_pwrseq_on(struct usb_hub *hub);
> >+void hub_pwrseq_off(struct usb_hub *hub);
> >+#else
> >+static inline int hub_pwrseq_on(struct usb_hub *hub)
> >+{
> >+return 0;
> >+}
> >+static inline void hub_pwrseq_off(struct usb_hub *hub) {}
> >+#endif /* CONFIG_PWRSEQ_GENERIC */
> >diff --git a/drivers/usb/core/pwrseq.c b/drivers/usb/core/pwrseq.c
> >new file mode 100644
> >index 000..837fe66
> >--- /dev/null
> >+++ b/drivers/usb/core/pwrseq.c
> >@@ -0,0 +1,100 @@
> >+/*
> >+ * pwrseq.c USB device power sequence management
> >+ *
> >+ * Copyright (C) 2016 Freescale Semiconductor, Inc.
> >+ * Author: Peter Chen 
> >+ *
> >+ * This program is free software: you can redistribute it and/or modify
> >+ * it under the terms of the GNU General Public License version 2  of
> >+ * the License as published by the Free Software Foundation.
> >+ *
> >+ * 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.
> >+ *
> >+ * You should have received a copy of the GNU 

Re: [PATCH v6 4/8] usb: core: add power sequence handling for USB devices

[Vaibhav Hiremath]

On Monday 15 August 2016 02:43 PM, 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. At first, it calls pwrseq_alloc_generic
to create a generic power sequence instance, then 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.

Signed-off-by: Peter Chen 
Tested-by Joshua Clayton 
---
  drivers/usb/core/Makefile |   1 +
  drivers/usb/core/hub.c|  12 --
  drivers/usb/core/hub.h|  12 ++
  drivers/usb/core/pwrseq.c | 100 ++
  4 files changed, 122 insertions(+), 3 deletions(-)
  create mode 100644 drivers/usb/core/pwrseq.c

diff --git a/drivers/usb/core/Makefile b/drivers/usb/core/Makefile
index 9780877..39f2149 100644
--- a/drivers/usb/core/Makefile
+++ b/drivers/usb/core/Makefile
@@ -9,5 +9,6 @@ usbcore-y += port.o of.o
  
  usbcore-$(CONFIG_PCI)		+= hcd-pci.o

  usbcore-$(CONFIG_ACPI)+= usb-acpi.o
+usbcore-$(CONFIG_PWRSEQ_GENERIC) += pwrseq.o
  
  obj-$(CONFIG_USB)		+= usbcore.o

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index bee1351..a346a8b 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1700,6 +1700,7 @@ static void hub_disconnect(struct usb_interface *intf)
hub->error = 0;
hub_quiesce(hub, HUB_DISCONNECT);
  
+	hub_pwrseq_off(hub);

mutex_lock(_port_peer_mutex);
  
  	/* Avoid races with recursively_mark_NOTATTACHED() */

@@ -1733,6 +1734,7 @@ static int hub_probe(struct usb_interface *intf, const 
struct usb_device_id *id)
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);
@@ -1839,6 +1841,7 @@ descriptor_error:
INIT_DELAYED_WORK(>leds, led_work);
INIT_DELAYED_WORK(>init_work, NULL);
INIT_WORK(>events, hub_event);
+   INIT_LIST_HEAD(>pwrseq_on_list);
usb_get_intf(intf);
usb_get_dev(hdev);
  
@@ -1852,11 +1855,14 @@ descriptor_error:

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_pwrseq_on(hub);
+   if (!ret)
+   return 0;
+   }
  
  	hub_disconnect(intf);

-   return -ENODEV;
+   return ret;
  }
  
  static int

diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
index 34c1a7e..9473f6f 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_headpwrseq_on_list; /* powered pwrseq node list */
  };
  
  /**

@@ -166,3 +167,14 @@ static inline int hub_port_debounce_be_stable(struct 
usb_hub *hub,
  {
return hub_port_debounce(hub, port1, false);
  }
+
+#if IS_ENABLED(CONFIG_PWRSEQ_GENERIC)
+int hub_pwrseq_on(struct usb_hub *hub);
+void hub_pwrseq_off(struct usb_hub *hub);
+#else
+static inline int hub_pwrseq_on(struct usb_hub *hub)
+{
+   return 0;
+}
+static inline void hub_pwrseq_off(struct usb_hub *hub) {}
+#endif /* CONFIG_PWRSEQ_GENERIC */
diff --git a/drivers/usb/core/pwrseq.c b/drivers/usb/core/pwrseq.c
new file mode 100644
index 000..837fe66
--- /dev/null
+++ b/drivers/usb/core/pwrseq.c
@@ -0,0 +1,100 @@
+/*
+ * pwrseq.cUSB device power sequence management
+ *
+ * Copyright (C) 2016 Freescale Semiconductor, Inc.
+ * Author: Peter Chen 
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2  of
+ * the License as published by the Free Software Foundation.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see .
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "hub.h"
+
+struct usb_pwrseq_node {
+   struct pwrseq *pwrseq;
+   struct list_head 

Re: [PATCH v6 4/8] usb: core: add power sequence handling for USB devices

[Vaibhav Hiremath]

On Monday 15 August 2016 02:43 PM, 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. At first, it calls pwrseq_alloc_generic
to create a generic power sequence instance, then 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.

Signed-off-by: Peter Chen 
Tested-by Joshua Clayton 
---
  drivers/usb/core/Makefile |   1 +
  drivers/usb/core/hub.c|  12 --
  drivers/usb/core/hub.h|  12 ++
  drivers/usb/core/pwrseq.c | 100 ++
  4 files changed, 122 insertions(+), 3 deletions(-)
  create mode 100644 drivers/usb/core/pwrseq.c

diff --git a/drivers/usb/core/Makefile b/drivers/usb/core/Makefile
index 9780877..39f2149 100644
--- a/drivers/usb/core/Makefile
+++ b/drivers/usb/core/Makefile
@@ -9,5 +9,6 @@ usbcore-y += port.o of.o
  
  usbcore-$(CONFIG_PCI)		+= hcd-pci.o

  usbcore-$(CONFIG_ACPI)+= usb-acpi.o
+usbcore-$(CONFIG_PWRSEQ_GENERIC) += pwrseq.o
  
  obj-$(CONFIG_USB)		+= usbcore.o

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index bee1351..a346a8b 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1700,6 +1700,7 @@ static void hub_disconnect(struct usb_interface *intf)
hub->error = 0;
hub_quiesce(hub, HUB_DISCONNECT);
  
+	hub_pwrseq_off(hub);

mutex_lock(_port_peer_mutex);
  
  	/* Avoid races with recursively_mark_NOTATTACHED() */

@@ -1733,6 +1734,7 @@ static int hub_probe(struct usb_interface *intf, const 
struct usb_device_id *id)
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);
@@ -1839,6 +1841,7 @@ descriptor_error:
INIT_DELAYED_WORK(>leds, led_work);
INIT_DELAYED_WORK(>init_work, NULL);
INIT_WORK(>events, hub_event);
+   INIT_LIST_HEAD(>pwrseq_on_list);
usb_get_intf(intf);
usb_get_dev(hdev);
  
@@ -1852,11 +1855,14 @@ descriptor_error:

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_pwrseq_on(hub);
+   if (!ret)
+   return 0;
+   }
  
  	hub_disconnect(intf);

-   return -ENODEV;
+   return ret;
  }
  
  static int

diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
index 34c1a7e..9473f6f 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_headpwrseq_on_list; /* powered pwrseq node list */
  };
  
  /**

@@ -166,3 +167,14 @@ static inline int hub_port_debounce_be_stable(struct 
usb_hub *hub,
  {
return hub_port_debounce(hub, port1, false);
  }
+
+#if IS_ENABLED(CONFIG_PWRSEQ_GENERIC)
+int hub_pwrseq_on(struct usb_hub *hub);
+void hub_pwrseq_off(struct usb_hub *hub);
+#else
+static inline int hub_pwrseq_on(struct usb_hub *hub)
+{
+   return 0;
+}
+static inline void hub_pwrseq_off(struct usb_hub *hub) {}
+#endif /* CONFIG_PWRSEQ_GENERIC */
diff --git a/drivers/usb/core/pwrseq.c b/drivers/usb/core/pwrseq.c
new file mode 100644
index 000..837fe66
--- /dev/null
+++ b/drivers/usb/core/pwrseq.c
@@ -0,0 +1,100 @@
+/*
+ * pwrseq.cUSB device power sequence management
+ *
+ * Copyright (C) 2016 Freescale Semiconductor, Inc.
+ * Author: Peter Chen 
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2  of
+ * the License as published by the Free Software Foundation.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see .
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "hub.h"
+
+struct usb_pwrseq_node {
+   struct pwrseq *pwrseq;
+   struct list_head list;
+};
+
+static int hub_of_pwrseq_on(struct device_node *np, 

Re: [PATCH v6 4/8] usb: core: add power sequence handling for USB devices

[Alan Stern]

On Tue, 23 Aug 2016, Peter Chen wrote:

> I will add #ifdef CONFIG_OF for related code. And put the content at
> hub_pwrseq_on at hub_probe directly, how about below?
> 
> hub_probe() {
> 
>   ...
> 
>   if (hub_configure(hub, endpoint) >= 0) {
> #ifdef CONFIG_OF
>   for_each_child_of_node(parent->of_node, np) {
>   ret = generic_pwrseq_on(np, hub);
>   if (ret)
>   return ret;
>   }
> #else
>   return 0;
> #endif
>   }

Please make this a separate subroutine like you had before, but now in 
hub.c:

#ifdef CONFIG_OF
static int hub_of_pwrseq_on(struct usb_hub *hub)
{
...
}

#else
static inline int hub_of_pwrseq_on(struct usb_hub *hub)
{
return 0;
}
#endif /* CONFIG_OF */


Alan Stern

Re: [PATCH v6 4/8] usb: core: add power sequence handling for USB devices

[Alan Stern]

On Tue, 23 Aug 2016, Peter Chen wrote:

> I will add #ifdef CONFIG_OF for related code. And put the content at
> hub_pwrseq_on at hub_probe directly, how about below?
> 
> hub_probe() {
> 
>   ...
> 
>   if (hub_configure(hub, endpoint) >= 0) {
> #ifdef CONFIG_OF
>   for_each_child_of_node(parent->of_node, np) {
>   ret = generic_pwrseq_on(np, hub);
>   if (ret)
>   return ret;
>   }
> #else
>   return 0;
> #endif
>   }

Please make this a separate subroutine like you had before, but now in 
hub.c:

#ifdef CONFIG_OF
static int hub_of_pwrseq_on(struct usb_hub *hub)
{
...
}

#else
static inline int hub_of_pwrseq_on(struct usb_hub *hub)
{
return 0;
}
#endif /* CONFIG_OF */


Alan Stern

Re: [PATCH v6 4/8] usb: core: add power sequence handling for USB devices

[Peter Chen]

On Mon, Aug 22, 2016 at 12:09:54PM -0400, Alan Stern wrote:
> > > +
> > > +static int hub_of_pwrseq_on(struct device_node *np, struct usb_hub *hub)
> > > +{
> > > + struct pwrseq *pwrseq;
> > > + struct usb_pwrseq_node *pwrseq_node;
> > > + int ret;
> > > +
> > > + pwrseq = pwrseq_alloc_generic();
> > > + if (IS_ERR(pwrseq))
> > > + return PTR_ERR(pwrseq);
> > > +
> > > + ret = pwrseq_get(np, pwrseq);
> > > + if (ret)
> > > + goto pwr_free;
> > > +
> > > + ret = pwrseq_on(np, pwrseq);
> > > + if (ret)
> > > + goto pwr_put;
> > > +
> > > + pwrseq_node = kzalloc(sizeof(*pwrseq_node), GFP_KERNEL);
> 
> You need to check that the kzalloc succeeded.

Ok.

> 
> > > + pwrseq_node->pwrseq = pwrseq;
> > > + list_add(_node->list, >pwrseq_on_list);
> > > +
> > > + return 0;
> > > +
> > > +pwr_put:
> > > + pwrseq_put(pwrseq);
> > > +pwr_free:
> > > + pwrseq_free(pwrseq);
> > > + return ret;
> > > +}
> 
> This subroutine looks very generic.  The only place where it cares that
> it was called for a USB hub is the list_add(), and that could easily
> become generic also (make this function take >pwrseq_on_list as
> its second argument, instead of hub).
> 
> It looks like this routine really belongs in the power sequence 
> library.
> 

I will export below two generic functions at pwrseq library:

int generic_pwrseq_on(struct device_node *np, struct list_head *head)
void generic_pwrseq_off(struct list_head *head)

delete this file, and call above APIs at hub.c.

> > > +
> > > +int hub_pwrseq_on(struct usb_hub *hub)
> > > +{
> 
> The names of this routine and the subroutine above are too similar.  
> After all, both functions require OF support.
> 

I will add #ifdef CONFIG_OF for related code. And put the content at
hub_pwrseq_on at hub_probe directly, how about below?

hub_probe() {

...

if (hub_configure(hub, endpoint) >= 0) {
#ifdef CONFIG_OF
for_each_child_of_node(parent->of_node, np) {
ret = generic_pwrseq_on(np, hub);
if (ret)
return ret;
}
#else
return 0;
#endif
}

hub_disconnect(intf);
return ret;
}

-- 

Best Regards,
Peter Chen

Re: [PATCH v6 4/8] usb: core: add power sequence handling for USB devices

[Peter Chen]

On Mon, Aug 22, 2016 at 12:09:54PM -0400, Alan Stern wrote:
> > > +
> > > +static int hub_of_pwrseq_on(struct device_node *np, struct usb_hub *hub)
> > > +{
> > > + struct pwrseq *pwrseq;
> > > + struct usb_pwrseq_node *pwrseq_node;
> > > + int ret;
> > > +
> > > + pwrseq = pwrseq_alloc_generic();
> > > + if (IS_ERR(pwrseq))
> > > + return PTR_ERR(pwrseq);
> > > +
> > > + ret = pwrseq_get(np, pwrseq);
> > > + if (ret)
> > > + goto pwr_free;
> > > +
> > > + ret = pwrseq_on(np, pwrseq);
> > > + if (ret)
> > > + goto pwr_put;
> > > +
> > > + pwrseq_node = kzalloc(sizeof(*pwrseq_node), GFP_KERNEL);
> 
> You need to check that the kzalloc succeeded.

Ok.

> 
> > > + pwrseq_node->pwrseq = pwrseq;
> > > + list_add(_node->list, >pwrseq_on_list);
> > > +
> > > + return 0;
> > > +
> > > +pwr_put:
> > > + pwrseq_put(pwrseq);
> > > +pwr_free:
> > > + pwrseq_free(pwrseq);
> > > + return ret;
> > > +}
> 
> This subroutine looks very generic.  The only place where it cares that
> it was called for a USB hub is the list_add(), and that could easily
> become generic also (make this function take >pwrseq_on_list as
> its second argument, instead of hub).
> 
> It looks like this routine really belongs in the power sequence 
> library.
> 

I will export below two generic functions at pwrseq library:

int generic_pwrseq_on(struct device_node *np, struct list_head *head)
void generic_pwrseq_off(struct list_head *head)

delete this file, and call above APIs at hub.c.

> > > +
> > > +int hub_pwrseq_on(struct usb_hub *hub)
> > > +{
> 
> The names of this routine and the subroutine above are too similar.  
> After all, both functions require OF support.
> 

I will add #ifdef CONFIG_OF for related code. And put the content at
hub_pwrseq_on at hub_probe directly, how about below?

hub_probe() {

...

if (hub_configure(hub, endpoint) >= 0) {
#ifdef CONFIG_OF
for_each_child_of_node(parent->of_node, np) {
ret = generic_pwrseq_on(np, hub);
if (ret)
return ret;
}
#else
return 0;
#endif
}

hub_disconnect(intf);
return ret;
}

-- 

Best Regards,
Peter Chen

Re: [PATCH v6 4/8] usb: core: add power sequence handling for USB devices

[Alan Stern]

On Mon, 22 Aug 2016, Peter Chen wrote:

> On Mon, Aug 15, 2016 at 05:13:14PM +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. At first, it calls pwrseq_alloc_generic
> > to create a generic power sequence instance, then 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.
> > 
> > Signed-off-by: Peter Chen 
> > Tested-by Joshua Clayton 
> 
> Hi Alan,
> 
> The power sequence library code has been reviewed several rounds, would
> you please help to review the change for USB part?

Some small problems...  See below.

> > --- a/drivers/usb/core/Makefile
> > +++ b/drivers/usb/core/Makefile
> > @@ -9,5 +9,6 @@ usbcore-y += port.o of.o
> >  
> >  usbcore-$(CONFIG_PCI)  += hcd-pci.o
> >  usbcore-$(CONFIG_ACPI) += usb-acpi.o
> > +usbcore-$(CONFIG_PWRSEQ_GENERIC) += pwrseq.o

Please use a tab to align this with the lines above.

> > --- /dev/null
> > +++ b/drivers/usb/core/pwrseq.c
> > @@ -0,0 +1,100 @@
> > +/*
> > + * pwrseq.cUSB device power sequence management
> > + *
> > + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> > + * Author: Peter Chen 
> > + *
> > + * This program is free software: you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2  of
> > + * the License as published by the Free Software Foundation.
> > + *
> > + * 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.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program.  If not, see .
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include "hub.h"
> > +
> > +struct usb_pwrseq_node {
> > +   struct pwrseq *pwrseq;
> > +   struct list_head list;
> > +};
> > +
> > +static int hub_of_pwrseq_on(struct device_node *np, struct usb_hub *hub)
> > +{
> > +   struct pwrseq *pwrseq;
> > +   struct usb_pwrseq_node *pwrseq_node;
> > +   int ret;
> > +
> > +   pwrseq = pwrseq_alloc_generic();
> > +   if (IS_ERR(pwrseq))
> > +   return PTR_ERR(pwrseq);
> > +
> > +   ret = pwrseq_get(np, pwrseq);
> > +   if (ret)
> > +   goto pwr_free;
> > +
> > +   ret = pwrseq_on(np, pwrseq);
> > +   if (ret)
> > +   goto pwr_put;
> > +
> > +   pwrseq_node = kzalloc(sizeof(*pwrseq_node), GFP_KERNEL);

You need to check that the kzalloc succeeded.

> > +   pwrseq_node->pwrseq = pwrseq;
> > +   list_add(_node->list, >pwrseq_on_list);
> > +
> > +   return 0;
> > +
> > +pwr_put:
> > +   pwrseq_put(pwrseq);
> > +pwr_free:
> > +   pwrseq_free(pwrseq);
> > +   return ret;
> > +}

This subroutine looks very generic.  The only place where it cares that
it was called for a USB hub is the list_add(), and that could easily
become generic also (make this function take >pwrseq_on_list as
its second argument, instead of hub).

It looks like this routine really belongs in the power sequence 
library.

> > +
> > +int hub_pwrseq_on(struct usb_hub *hub)
> > +{

The names of this routine and the subroutine above are too similar.  
After all, both functions require OF support.

> > +   struct device *parent;
> > +   struct usb_device *hdev = hub->hdev;
> > +   struct device_node *np;
> > +   int ret;
> > +
> > +   if (hdev->parent)
> > +   parent = >dev;
> > +   else
> > +   parent = bus_to_hcd(hdev->bus)->self.controller;
> > +
> > +   for_each_child_of_node(parent->of_node, np) {
> > +   ret = hub_of_pwrseq_on(np, hub);
> > +   if (ret)
> > +   return ret;
> > +   }
> > +
> > +   return 0;
> > +}
> > +
> > +void hub_pwrseq_off(struct usb_hub *hub)
> > +{
> > +   struct pwrseq *pwrseq;
> > +   struct usb_pwrseq_node *pwrseq_node, *tmp_node;
> > +
> > +   list_for_each_entry_safe(pwrseq_node, tmp_node,
> > +   >pwrseq_on_list, list) {
> > +   pwrseq = pwrseq_node->pwrseq;
> > +   pwrseq_off(pwrseq);
> > +   pwrseq_put(pwrseq);
> > +   pwrseq_free(pwrseq);
> > +   list_del(_node->list);
> > +   kfree(pwrseq_node);
> > +   }
> > +}

This routine also looks like it belongs in a library.

Alan Stern

Re: [PATCH v6 4/8] usb: core: add power sequence handling for USB devices

[Alan Stern]

On Mon, 22 Aug 2016, Peter Chen wrote:

> On Mon, Aug 15, 2016 at 05:13:14PM +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. At first, it calls pwrseq_alloc_generic
> > to create a generic power sequence instance, then 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.
> > 
> > Signed-off-by: Peter Chen 
> > Tested-by Joshua Clayton 
> 
> Hi Alan,
> 
> The power sequence library code has been reviewed several rounds, would
> you please help to review the change for USB part?

Some small problems...  See below.

> > --- a/drivers/usb/core/Makefile
> > +++ b/drivers/usb/core/Makefile
> > @@ -9,5 +9,6 @@ usbcore-y += port.o of.o
> >  
> >  usbcore-$(CONFIG_PCI)  += hcd-pci.o
> >  usbcore-$(CONFIG_ACPI) += usb-acpi.o
> > +usbcore-$(CONFIG_PWRSEQ_GENERIC) += pwrseq.o

Please use a tab to align this with the lines above.

> > --- /dev/null
> > +++ b/drivers/usb/core/pwrseq.c
> > @@ -0,0 +1,100 @@
> > +/*
> > + * pwrseq.cUSB device power sequence management
> > + *
> > + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> > + * Author: Peter Chen 
> > + *
> > + * This program is free software: you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2  of
> > + * the License as published by the Free Software Foundation.
> > + *
> > + * 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.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program.  If not, see .
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include "hub.h"
> > +
> > +struct usb_pwrseq_node {
> > +   struct pwrseq *pwrseq;
> > +   struct list_head list;
> > +};
> > +
> > +static int hub_of_pwrseq_on(struct device_node *np, struct usb_hub *hub)
> > +{
> > +   struct pwrseq *pwrseq;
> > +   struct usb_pwrseq_node *pwrseq_node;
> > +   int ret;
> > +
> > +   pwrseq = pwrseq_alloc_generic();
> > +   if (IS_ERR(pwrseq))
> > +   return PTR_ERR(pwrseq);
> > +
> > +   ret = pwrseq_get(np, pwrseq);
> > +   if (ret)
> > +   goto pwr_free;
> > +
> > +   ret = pwrseq_on(np, pwrseq);
> > +   if (ret)
> > +   goto pwr_put;
> > +
> > +   pwrseq_node = kzalloc(sizeof(*pwrseq_node), GFP_KERNEL);

You need to check that the kzalloc succeeded.

> > +   pwrseq_node->pwrseq = pwrseq;
> > +   list_add(_node->list, >pwrseq_on_list);
> > +
> > +   return 0;
> > +
> > +pwr_put:
> > +   pwrseq_put(pwrseq);
> > +pwr_free:
> > +   pwrseq_free(pwrseq);
> > +   return ret;
> > +}

This subroutine looks very generic.  The only place where it cares that
it was called for a USB hub is the list_add(), and that could easily
become generic also (make this function take >pwrseq_on_list as
its second argument, instead of hub).

It looks like this routine really belongs in the power sequence 
library.

> > +
> > +int hub_pwrseq_on(struct usb_hub *hub)
> > +{

The names of this routine and the subroutine above are too similar.  
After all, both functions require OF support.

> > +   struct device *parent;
> > +   struct usb_device *hdev = hub->hdev;
> > +   struct device_node *np;
> > +   int ret;
> > +
> > +   if (hdev->parent)
> > +   parent = >dev;
> > +   else
> > +   parent = bus_to_hcd(hdev->bus)->self.controller;
> > +
> > +   for_each_child_of_node(parent->of_node, np) {
> > +   ret = hub_of_pwrseq_on(np, hub);
> > +   if (ret)
> > +   return ret;
> > +   }
> > +
> > +   return 0;
> > +}
> > +
> > +void hub_pwrseq_off(struct usb_hub *hub)
> > +{
> > +   struct pwrseq *pwrseq;
> > +   struct usb_pwrseq_node *pwrseq_node, *tmp_node;
> > +
> > +   list_for_each_entry_safe(pwrseq_node, tmp_node,
> > +   >pwrseq_on_list, list) {
> > +   pwrseq = pwrseq_node->pwrseq;
> > +   pwrseq_off(pwrseq);
> > +   pwrseq_put(pwrseq);
> > +   pwrseq_free(pwrseq);
> > +   list_del(_node->list);
> > +   kfree(pwrseq_node);
> > +   }
> > +}

This routine also looks like it belongs in a library.

Alan Stern

Re: [PATCH v6 4/8] usb: core: add power sequence handling for USB devices

[Peter Chen]

On Mon, Aug 15, 2016 at 05:13:14PM +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. At first, it calls pwrseq_alloc_generic
> to create a generic power sequence instance, then 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.
> 
> Signed-off-by: Peter Chen 
> Tested-by Joshua Clayton 

Hi Alan,

The power sequence library code has been reviewed several rounds, would
you please help to review the change for USB part?

Peter

> ---
>  drivers/usb/core/Makefile |   1 +
>  drivers/usb/core/hub.c|  12 --
>  drivers/usb/core/hub.h|  12 ++
>  drivers/usb/core/pwrseq.c | 100 
> ++
>  4 files changed, 122 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/usb/core/pwrseq.c
> 
> diff --git a/drivers/usb/core/Makefile b/drivers/usb/core/Makefile
> index 9780877..39f2149 100644
> --- a/drivers/usb/core/Makefile
> +++ b/drivers/usb/core/Makefile
> @@ -9,5 +9,6 @@ usbcore-y += port.o of.o
>  
>  usbcore-$(CONFIG_PCI)+= hcd-pci.o
>  usbcore-$(CONFIG_ACPI)   += usb-acpi.o
> +usbcore-$(CONFIG_PWRSEQ_GENERIC) += pwrseq.o
>  
>  obj-$(CONFIG_USB)+= usbcore.o
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index bee1351..a346a8b 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -1700,6 +1700,7 @@ static void hub_disconnect(struct usb_interface *intf)
>   hub->error = 0;
>   hub_quiesce(hub, HUB_DISCONNECT);
>  
> + hub_pwrseq_off(hub);
>   mutex_lock(_port_peer_mutex);
>  
>   /* Avoid races with recursively_mark_NOTATTACHED() */
> @@ -1733,6 +1734,7 @@ static int hub_probe(struct usb_interface *intf, const 
> struct usb_device_id *id)
>   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);
> @@ -1839,6 +1841,7 @@ descriptor_error:
>   INIT_DELAYED_WORK(>leds, led_work);
>   INIT_DELAYED_WORK(>init_work, NULL);
>   INIT_WORK(>events, hub_event);
> + INIT_LIST_HEAD(>pwrseq_on_list);
>   usb_get_intf(intf);
>   usb_get_dev(hdev);
>  
> @@ -1852,11 +1855,14 @@ descriptor_error:
>   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_pwrseq_on(hub);
> + if (!ret)
> + return 0;
> + }
>  
>   hub_disconnect(intf);
> - return -ENODEV;
> + return ret;
>  }
>  
>  static int
> diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
> index 34c1a7e..9473f6f 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_headpwrseq_on_list; /* powered pwrseq node list */
>  };
>  
>  /**
> @@ -166,3 +167,14 @@ static inline int hub_port_debounce_be_stable(struct 
> usb_hub *hub,
>  {
>   return hub_port_debounce(hub, port1, false);
>  }
> +
> +#if IS_ENABLED(CONFIG_PWRSEQ_GENERIC)
> +int hub_pwrseq_on(struct usb_hub *hub);
> +void hub_pwrseq_off(struct usb_hub *hub);
> +#else
> +static inline int hub_pwrseq_on(struct usb_hub *hub)
> +{
> + return 0;
> +}
> +static inline void hub_pwrseq_off(struct usb_hub *hub) {}
> +#endif /* CONFIG_PWRSEQ_GENERIC */
> diff --git a/drivers/usb/core/pwrseq.c b/drivers/usb/core/pwrseq.c
> new file mode 100644
> index 000..837fe66
> --- /dev/null
> +++ b/drivers/usb/core/pwrseq.c
> @@ -0,0 +1,100 @@
> +/*
> + * pwrseq.c  USB device power sequence management
> + *
> + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> + * Author: Peter Chen 
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2  of
> + * the License as published by the Free Software Foundation.
> + *
> + * 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 

Re: [PATCH v6 4/8] usb: core: add power sequence handling for USB devices

[Peter Chen]

On Mon, Aug 15, 2016 at 05:13:14PM +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. At first, it calls pwrseq_alloc_generic
> to create a generic power sequence instance, then 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.
> 
> Signed-off-by: Peter Chen 
> Tested-by Joshua Clayton 

Hi Alan,

The power sequence library code has been reviewed several rounds, would
you please help to review the change for USB part?

Peter

> ---
>  drivers/usb/core/Makefile |   1 +
>  drivers/usb/core/hub.c|  12 --
>  drivers/usb/core/hub.h|  12 ++
>  drivers/usb/core/pwrseq.c | 100 
> ++
>  4 files changed, 122 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/usb/core/pwrseq.c
> 
> diff --git a/drivers/usb/core/Makefile b/drivers/usb/core/Makefile
> index 9780877..39f2149 100644
> --- a/drivers/usb/core/Makefile
> +++ b/drivers/usb/core/Makefile
> @@ -9,5 +9,6 @@ usbcore-y += port.o of.o
>  
>  usbcore-$(CONFIG_PCI)+= hcd-pci.o
>  usbcore-$(CONFIG_ACPI)   += usb-acpi.o
> +usbcore-$(CONFIG_PWRSEQ_GENERIC) += pwrseq.o
>  
>  obj-$(CONFIG_USB)+= usbcore.o
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index bee1351..a346a8b 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -1700,6 +1700,7 @@ static void hub_disconnect(struct usb_interface *intf)
>   hub->error = 0;
>   hub_quiesce(hub, HUB_DISCONNECT);
>  
> + hub_pwrseq_off(hub);
>   mutex_lock(_port_peer_mutex);
>  
>   /* Avoid races with recursively_mark_NOTATTACHED() */
> @@ -1733,6 +1734,7 @@ static int hub_probe(struct usb_interface *intf, const 
> struct usb_device_id *id)
>   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);
> @@ -1839,6 +1841,7 @@ descriptor_error:
>   INIT_DELAYED_WORK(>leds, led_work);
>   INIT_DELAYED_WORK(>init_work, NULL);
>   INIT_WORK(>events, hub_event);
> + INIT_LIST_HEAD(>pwrseq_on_list);
>   usb_get_intf(intf);
>   usb_get_dev(hdev);
>  
> @@ -1852,11 +1855,14 @@ descriptor_error:
>   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_pwrseq_on(hub);
> + if (!ret)
> + return 0;
> + }
>  
>   hub_disconnect(intf);
> - return -ENODEV;
> + return ret;
>  }
>  
>  static int
> diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
> index 34c1a7e..9473f6f 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_headpwrseq_on_list; /* powered pwrseq node list */
>  };
>  
>  /**
> @@ -166,3 +167,14 @@ static inline int hub_port_debounce_be_stable(struct 
> usb_hub *hub,
>  {
>   return hub_port_debounce(hub, port1, false);
>  }
> +
> +#if IS_ENABLED(CONFIG_PWRSEQ_GENERIC)
> +int hub_pwrseq_on(struct usb_hub *hub);
> +void hub_pwrseq_off(struct usb_hub *hub);
> +#else
> +static inline int hub_pwrseq_on(struct usb_hub *hub)
> +{
> + return 0;
> +}
> +static inline void hub_pwrseq_off(struct usb_hub *hub) {}
> +#endif /* CONFIG_PWRSEQ_GENERIC */
> diff --git a/drivers/usb/core/pwrseq.c b/drivers/usb/core/pwrseq.c
> new file mode 100644
> index 000..837fe66
> --- /dev/null
> +++ b/drivers/usb/core/pwrseq.c
> @@ -0,0 +1,100 @@
> +/*
> + * pwrseq.c  USB device power sequence management
> + *
> + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> + * Author: Peter Chen 
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2  of
> + * the License as published by the Free Software Foundation.
> + *
> + * 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.
> + *
> + * You should have received a 

[PATCH v6 4/8] usb: core: add power sequence handling for USB devices

[Peter Chen]

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. At first, it calls pwrseq_alloc_generic
to create a generic power sequence instance, then 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.

Signed-off-by: Peter Chen 
Tested-by Joshua Clayton 
---
 drivers/usb/core/Makefile |   1 +
 drivers/usb/core/hub.c|  12 --
 drivers/usb/core/hub.h|  12 ++
 drivers/usb/core/pwrseq.c | 100 ++
 4 files changed, 122 insertions(+), 3 deletions(-)
 create mode 100644 drivers/usb/core/pwrseq.c

diff --git a/drivers/usb/core/Makefile b/drivers/usb/core/Makefile
index 9780877..39f2149 100644
--- a/drivers/usb/core/Makefile
+++ b/drivers/usb/core/Makefile
@@ -9,5 +9,6 @@ usbcore-y += port.o of.o
 
 usbcore-$(CONFIG_PCI)  += hcd-pci.o
 usbcore-$(CONFIG_ACPI) += usb-acpi.o
+usbcore-$(CONFIG_PWRSEQ_GENERIC) += pwrseq.o
 
 obj-$(CONFIG_USB)  += usbcore.o
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index bee1351..a346a8b 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1700,6 +1700,7 @@ static void hub_disconnect(struct usb_interface *intf)
hub->error = 0;
hub_quiesce(hub, HUB_DISCONNECT);
 
+   hub_pwrseq_off(hub);
mutex_lock(_port_peer_mutex);
 
/* Avoid races with recursively_mark_NOTATTACHED() */
@@ -1733,6 +1734,7 @@ static int hub_probe(struct usb_interface *intf, const 
struct usb_device_id *id)
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);
@@ -1839,6 +1841,7 @@ descriptor_error:
INIT_DELAYED_WORK(>leds, led_work);
INIT_DELAYED_WORK(>init_work, NULL);
INIT_WORK(>events, hub_event);
+   INIT_LIST_HEAD(>pwrseq_on_list);
usb_get_intf(intf);
usb_get_dev(hdev);
 
@@ -1852,11 +1855,14 @@ descriptor_error:
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_pwrseq_on(hub);
+   if (!ret)
+   return 0;
+   }
 
hub_disconnect(intf);
-   return -ENODEV;
+   return ret;
 }
 
 static int
diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
index 34c1a7e..9473f6f 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_headpwrseq_on_list; /* powered pwrseq node list */
 };
 
 /**
@@ -166,3 +167,14 @@ static inline int hub_port_debounce_be_stable(struct 
usb_hub *hub,
 {
return hub_port_debounce(hub, port1, false);
 }
+
+#if IS_ENABLED(CONFIG_PWRSEQ_GENERIC)
+int hub_pwrseq_on(struct usb_hub *hub);
+void hub_pwrseq_off(struct usb_hub *hub);
+#else
+static inline int hub_pwrseq_on(struct usb_hub *hub)
+{
+   return 0;
+}
+static inline void hub_pwrseq_off(struct usb_hub *hub) {}
+#endif /* CONFIG_PWRSEQ_GENERIC */
diff --git a/drivers/usb/core/pwrseq.c b/drivers/usb/core/pwrseq.c
new file mode 100644
index 000..837fe66
--- /dev/null
+++ b/drivers/usb/core/pwrseq.c
@@ -0,0 +1,100 @@
+/*
+ * pwrseq.cUSB device power sequence management
+ *
+ * Copyright (C) 2016 Freescale Semiconductor, Inc.
+ * Author: Peter Chen 
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2  of
+ * the License as published by the Free Software Foundation.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see .
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "hub.h"
+
+struct usb_pwrseq_node {
+   struct pwrseq *pwrseq;
+   struct list_head list;
+};
+
+static int hub_of_pwrseq_on(struct 

[PATCH v6 4/8] usb: core: add power sequence handling for USB devices

[Peter Chen]

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. At first, it calls pwrseq_alloc_generic
to create a generic power sequence instance, then 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.

Signed-off-by: Peter Chen 
Tested-by Joshua Clayton 
---
 drivers/usb/core/Makefile |   1 +
 drivers/usb/core/hub.c|  12 --
 drivers/usb/core/hub.h|  12 ++
 drivers/usb/core/pwrseq.c | 100 ++
 4 files changed, 122 insertions(+), 3 deletions(-)
 create mode 100644 drivers/usb/core/pwrseq.c

diff --git a/drivers/usb/core/Makefile b/drivers/usb/core/Makefile
index 9780877..39f2149 100644
--- a/drivers/usb/core/Makefile
+++ b/drivers/usb/core/Makefile
@@ -9,5 +9,6 @@ usbcore-y += port.o of.o
 
 usbcore-$(CONFIG_PCI)  += hcd-pci.o
 usbcore-$(CONFIG_ACPI) += usb-acpi.o
+usbcore-$(CONFIG_PWRSEQ_GENERIC) += pwrseq.o
 
 obj-$(CONFIG_USB)  += usbcore.o
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index bee1351..a346a8b 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1700,6 +1700,7 @@ static void hub_disconnect(struct usb_interface *intf)
hub->error = 0;
hub_quiesce(hub, HUB_DISCONNECT);
 
+   hub_pwrseq_off(hub);
mutex_lock(_port_peer_mutex);
 
/* Avoid races with recursively_mark_NOTATTACHED() */
@@ -1733,6 +1734,7 @@ static int hub_probe(struct usb_interface *intf, const 
struct usb_device_id *id)
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);
@@ -1839,6 +1841,7 @@ descriptor_error:
INIT_DELAYED_WORK(>leds, led_work);
INIT_DELAYED_WORK(>init_work, NULL);
INIT_WORK(>events, hub_event);
+   INIT_LIST_HEAD(>pwrseq_on_list);
usb_get_intf(intf);
usb_get_dev(hdev);
 
@@ -1852,11 +1855,14 @@ descriptor_error:
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_pwrseq_on(hub);
+   if (!ret)
+   return 0;
+   }
 
hub_disconnect(intf);
-   return -ENODEV;
+   return ret;
 }
 
 static int
diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
index 34c1a7e..9473f6f 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_headpwrseq_on_list; /* powered pwrseq node list */
 };
 
 /**
@@ -166,3 +167,14 @@ static inline int hub_port_debounce_be_stable(struct 
usb_hub *hub,
 {
return hub_port_debounce(hub, port1, false);
 }
+
+#if IS_ENABLED(CONFIG_PWRSEQ_GENERIC)
+int hub_pwrseq_on(struct usb_hub *hub);
+void hub_pwrseq_off(struct usb_hub *hub);
+#else
+static inline int hub_pwrseq_on(struct usb_hub *hub)
+{
+   return 0;
+}
+static inline void hub_pwrseq_off(struct usb_hub *hub) {}
+#endif /* CONFIG_PWRSEQ_GENERIC */
diff --git a/drivers/usb/core/pwrseq.c b/drivers/usb/core/pwrseq.c
new file mode 100644
index 000..837fe66
--- /dev/null
+++ b/drivers/usb/core/pwrseq.c
@@ -0,0 +1,100 @@
+/*
+ * pwrseq.cUSB device power sequence management
+ *
+ * Copyright (C) 2016 Freescale Semiconductor, Inc.
+ * Author: Peter Chen 
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2  of
+ * the License as published by the Free Software Foundation.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see .
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "hub.h"
+
+struct usb_pwrseq_node {
+   struct pwrseq *pwrseq;
+   struct list_head list;
+};
+
+static int hub_of_pwrseq_on(struct device_node *np, struct usb_hub *hub)
+{
+   struct pwrseq *pwrseq;