[PATCH v3 0/6] power: add power sequence library

[PATCH v3 0/6] power: add power sequence library

[Peter Chen]

Hi all,

This is a follow-up for my last power sequence framework patch set [1].
According to Rob Herring and Ulf Hansson's comments[2], I use a generic
power sequence library for parsing the power sequence elements on DT,
and implement generic power sequence on library. The host driver
can allocate power sequence instance, and calls pwrseq APIs accordingly.

In future, if there are special power sequence requirements, the special
power sequence library can be created.

This patch set is tested on i.mx6 sabresx evk using a dts change, I use
two hot-plug devices to simulate this use case, the related binding
change is updated at patch [1/6], The udoo board changes were tested
using my last power sequence patch set.[3]

Except for hard-wired MMC and USB devices, I find the USB ULPI PHY also
need to power on itself before it can be found by ULPI bus.

[1] http://www.spinics.net/lists/linux-usb/msg142755.html
[2] http://www.spinics.net/lists/linux-usb/msg143106.html
[3] http://www.spinics.net/lists/linux-usb/msg142815.html

Changes for v3:
- Delete "power-sequence" property at binding-doc, and change related code
  at both library and user code.
- Change binding-doc example node name with Rob's comments
- of_get_named_gpio_flags only gets the gpio, but without setting gpio flags,
  add additional code request gpio with proper gpio flags
- Add Philipp Zabel's Ack and MAINTAINER's entry

Changes for v2:
- Delete "pwrseq" prefix and clock-names for properties at dt binding
- Should use structure not but its pointer for kzalloc
- Since chipidea core has no of_node, let core's of_node equals glue
  layer's at core's probe

Peter Chen (6):
  binding-doc: power: pwrseq-generic: add binding doc for generic power
    sequence library
  power: add power sequence library
  binding-doc: usb: usb-device: add optional properties for power
    sequence
  usb: core: add power sequence handling for USB devices
  usb: chipidea: let chipidea core device of_node equal's glue layer
    device of_node
  ARM: dts: imx6qdl-udoo.dtsi: fix onboard USB HUB property

 .../bindings/power/pwrseq/pwrseq-generic.txt       |  48 +++++++
 .../devicetree/bindings/usb/usb-device.txt         |  10 +-
 MAINTAINERS                                        |   9 ++
 arch/arm/boot/dts/imx6qdl-udoo.dtsi                |  26 ++--
 drivers/power/Kconfig                              |   1 +
 drivers/power/Makefile                             |   1 +
 drivers/power/pwrseq/Kconfig                       |  20 +++
 drivers/power/pwrseq/Makefile                      |   2 +
 drivers/power/pwrseq/core.c                        |  71 ++++++++++
 drivers/power/pwrseq/pwrseq_generic.c              | 151 +++++++++++++++++++++
 drivers/usb/chipidea/core.c                        |  10 ++
 drivers/usb/core/Makefile                          |   1 +
 drivers/usb/core/hub.c                             |  12 +-
 drivers/usb/core/hub.h                             |  12 ++
 drivers/usb/core/pwrseq.c                          | 100 ++++++++++++++
 include/linux/power/pwrseq.h                       |  50 +++++++
 16 files changed, 507 insertions(+), 17 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
 create mode 100644 drivers/power/pwrseq/Kconfig
 create mode 100644 drivers/power/pwrseq/Makefile
 create mode 100644 drivers/power/pwrseq/core.c
 create mode 100644 drivers/power/pwrseq/pwrseq_generic.c
 create mode 100644 drivers/usb/core/pwrseq.c
 create mode 100644 include/linux/power/pwrseq.h

-- 
1.9.1

[PATCH v3 1/6] binding-doc: power: pwrseq-generic: add binding doc for generic power sequence library

[Peter Chen]

Add binding doc for generic power sequence library.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
Acked-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 .../bindings/power/pwrseq/pwrseq-generic.txt       | 48 ++++++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt

diff --git a/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
new file mode 100644
index 0000000..48bb3e1
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
@@ -0,0 +1,48 @@
+The generic power sequence library
+
+Some hard-wired devices (eg USB/MMC) need to do power sequence before
+the device can be enumerated on the bus, the typical power sequence
+like: enable USB PHY clock, toggle reset pin, etc. But current
+Linux device driver lacks of such code to do it, it may cause some
+hard-wired devices works abnormal or can't be recognized by
+controller at all. The power sequence will be done before this device
+can be found at the bus.
+
+The power sequence properties is under the device node.
+
+Optional properties:
+- clocks: the input clock for device.
+- reset-gpios: Should specify the GPIO for reset.
+- reset-duration-us: the duration in microsecond for assert reset signal.
+
+Below is the example of USB power sequence properties on USB device
+nodes which have two level USB hubs.
+
+&usbotg1 {
+	vbus-supply = <&reg_usb_otg1_vbus>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_usb_otg1_id>;
+	status = "okay";
+
+	#address-cells = <1>;
+	#size-cells = <0>;
+	genesys: hub at 1 {
+		compatible = "usb5e3,608";
+		reg = <1>;
+
+		clocks = <&clks IMX6SX_CLK_CKO>;
+		reset-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>; /* hub reset pin */
+		reset-duration-us = <10>;
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+		asix: ethernet at 1 {
+			compatible = "usbb95,1708";
+			reg = <1>;
+
+			clocks = <&clks IMX6SX_CLK_IPG>;
+			reset-gpios = <&gpio4 6 GPIO_ACTIVE_LOW>; /* ethernet_rst */
+			reset-duration-us = <15>;
+		};
+	};
+};
-- 
1.9.1

[PATCH v3 2/6] power: add power sequence library

[Peter Chen]

We have an well-known problem that the device needs to do some power
sequence before it can be recognized by related host, the typical
example like hard-wired mmc devices and usb devices.

This power sequence is hard to be described at device tree and handled by
related host driver, so we have created a common power sequence
library to cover this requirement. The core code has supplied
some common helpers for host driver, and individual power sequence
libraries handle kinds of power sequence for devices.

pwrseq_generic is intended for general purpose of power sequence, which
handles gpios and clocks currently, and can cover regulator and pinctrl
in future. The host driver calls pwrseq_alloc_generic to create
an generic pwrseq instance.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 MAINTAINERS                           |   9 ++
 drivers/power/Kconfig                 |   1 +
 drivers/power/Makefile                |   1 +
 drivers/power/pwrseq/Kconfig          |  20 +++++
 drivers/power/pwrseq/Makefile         |   2 +
 drivers/power/pwrseq/core.c           |  71 ++++++++++++++++
 drivers/power/pwrseq/pwrseq_generic.c | 151 ++++++++++++++++++++++++++++++++++
 include/linux/power/pwrseq.h          |  50 +++++++++++
 8 files changed, 305 insertions(+)
 create mode 100644 drivers/power/pwrseq/Kconfig
 create mode 100644 drivers/power/pwrseq/Makefile
 create mode 100644 drivers/power/pwrseq/core.c
 create mode 100644 drivers/power/pwrseq/pwrseq_generic.c
 create mode 100644 include/linux/power/pwrseq.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 68a90d0..6fd879f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9243,6 +9243,15 @@ F:	include/linux/pm_*
 F:	include/linux/powercap.h
 F:	drivers/powercap/
 
+POWER SEQUENCE LIBRARY
+M:	Peter Chen <Peter.Chen@nxp.com>
+T:	git git://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git
+L:	linux-pm at vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/power/pwrseq/
+F:	drivers/power/pwrseq/
+F:	include/linux/power/pwrseq.h/
+
 POWER SUPPLY CLASS/SUBSYSTEM and DRIVERS
 M:	Sebastian Reichel <sre@kernel.org>
 M:	Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index acd4a15..f6aa4fd 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -515,3 +515,4 @@ endif # POWER_SUPPLY
 
 source "drivers/power/reset/Kconfig"
 source "drivers/power/avs/Kconfig"
+source "drivers/power/pwrseq/Kconfig"
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index e46b75d..4ed2e12 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -74,3 +74,4 @@ obj-$(CONFIG_CHARGER_TPS65217)	+= tps65217_charger.o
 obj-$(CONFIG_POWER_RESET)	+= reset/
 obj-$(CONFIG_AXP288_FUEL_GAUGE) += axp288_fuel_gauge.o
 obj-$(CONFIG_AXP288_CHARGER)	+= axp288_charger.o
+obj-$(CONFIG_POWER_SEQUENCE)	+= pwrseq/
diff --git a/drivers/power/pwrseq/Kconfig b/drivers/power/pwrseq/Kconfig
new file mode 100644
index 0000000..188729e
--- /dev/null
+++ b/drivers/power/pwrseq/Kconfig
@@ -0,0 +1,20 @@
+#
+# Power Sequence library
+#
+
+config POWER_SEQUENCE
+	bool
+
+menu "Power Sequence Support"
+
+config PWRSEQ_GENERIC
+	bool "Generic power sequence control"
+	depends on OF
+	select POWER_SEQUENCE
+	help
+	  It is used for drivers which needs to do power sequence
+	  (eg, turn on clock, toggle reset gpio) before the related
+	  devices can be found by hardware. This generic one can be
+	  used for common power sequence control.
+
+endmenu
diff --git a/drivers/power/pwrseq/Makefile b/drivers/power/pwrseq/Makefile
new file mode 100644
index 0000000..ad82389
--- /dev/null
+++ b/drivers/power/pwrseq/Makefile
@@ -0,0 +1,2 @@
+obj-$(CONFIG_POWER_SEQUENCE) += core.o
+obj-$(CONFIG_PWRSEQ_GENERIC) += pwrseq_generic.o
diff --git a/drivers/power/pwrseq/core.c b/drivers/power/pwrseq/core.c
new file mode 100644
index 0000000..60f1e4e
--- /dev/null
+++ b/drivers/power/pwrseq/core.c
@@ -0,0 +1,71 @@
+/*
+ * core.c	power sequence core file
+ *
+ * Copyright (C) 2016 Freescale Semiconductor, Inc.
+ * Author: Peter Chen <peter.chen@nxp.com>
+ *
+ * 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 <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/power/pwrseq.h>
+
+static DEFINE_MUTEX(pwrseq_list_mutex);
+static LIST_HEAD(pwrseq_list);
+
+int pwrseq_get(struct device_node *np, struct pwrseq *p)
+{
+	if (p && p->get)
+		return p->get(np, p);
+
+	return -ENOTSUPP;
+}
+
+int pwrseq_on(struct device_node *np, struct pwrseq *p)
+{
+	if (p && p->on)
+		return p->on(np, p);
+
+	return -ENOTSUPP;
+}
+
+void pwrseq_off(struct pwrseq *p)
+{
+	if (p && p->off)
+		p->off(p);
+}
+
+void pwrseq_put(struct pwrseq *p)
+{
+	if (p && p->put)
+		p->put(p);
+}
+
+void pwrseq_free(struct pwrseq *p)
+{
+	if (p && p->free)
+		p->free(p);
+}
+
+void pwrseq_register(struct pwrseq *pwrseq)
+{
+	mutex_lock(&pwrseq_list_mutex);
+	list_add(&pwrseq->node, &pwrseq_list);
+	mutex_unlock(&pwrseq_list_mutex);
+}
+
+void pwrseq_unregister(struct pwrseq *pwrseq)
+{
+	mutex_lock(&pwrseq_list_mutex);
+	list_del(&pwrseq->node);
+	mutex_unlock(&pwrseq_list_mutex);
+}
diff --git a/drivers/power/pwrseq/pwrseq_generic.c b/drivers/power/pwrseq/pwrseq_generic.c
new file mode 100644
index 0000000..b50cea6
--- /dev/null
+++ b/drivers/power/pwrseq/pwrseq_generic.c
@@ -0,0 +1,151 @@
+/*
+ * pwrseq_generic.c	Generic power sequence handling
+ *
+ * Copyright (C) 2016 Freescale Semiconductor, Inc.
+ * Author: Peter Chen <peter.chen@nxp.com>
+ *
+ * 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 <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+#include <linux/slab.h>
+
+#include <linux/power/pwrseq.h>
+
+struct pwrseq_generic {
+	struct pwrseq pwrseq;
+	struct gpio_desc *gpiod_reset;
+	struct clk *clk;
+};
+
+#define to_generic_pwrseq(p) container_of(p, struct pwrseq_generic, pwrseq)
+
+static void pwrseq_generic_free(struct pwrseq *pwrseq)
+{
+	struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
+
+	pwrseq_unregister(pwrseq);
+	kfree(pwrseq_gen);
+}
+
+static void pwrseq_generic_put(struct pwrseq *pwrseq)
+{
+	struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
+
+	if (pwrseq_gen->gpiod_reset)
+		gpiod_put(pwrseq_gen->gpiod_reset);
+
+	clk_put(pwrseq_gen->clk);
+}
+
+static void pwrseq_generic_off(struct pwrseq *pwrseq)
+{
+	struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
+
+	clk_disable_unprepare(pwrseq_gen->clk);
+}
+
+static int pwrseq_generic_on(struct device_node *np, struct pwrseq *pwrseq)
+{
+	struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
+	int ret = 0;
+	struct gpio_desc *gpiod_reset = pwrseq_gen->gpiod_reset;
+
+	if (pwrseq_gen->clk) {
+		ret = clk_prepare_enable(pwrseq_gen->clk);
+		if (ret) {
+			pr_err("Can't enable clock on %s: %d\n",
+				np->full_name, ret);
+			return ret;
+		}
+	}
+
+	if (gpiod_reset) {
+		u32 duration_us = 50;
+
+		of_property_read_u32(np, "reset-duration-us",
+				&duration_us);
+		usleep_range(duration_us, duration_us + 10);
+		gpiod_set_value(gpiod_reset, 0);
+	}
+
+	return ret;
+}
+
+static int pwrseq_generic_get(struct device_node *np, struct pwrseq *pwrseq)
+{
+	struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
+	enum of_gpio_flags flags;
+	int reset_gpio, ret = 0;
+
+	pwrseq_gen->clk = of_clk_get_by_name(np, NULL);
+	if (IS_ERR(pwrseq_gen->clk)) {
+		pr_debug("Can't get clock on %s: %ld\n",
+			np->name, PTR_ERR(pwrseq_gen->clk));
+		pwrseq_gen->clk = NULL;
+	}
+
+	reset_gpio = of_get_named_gpio_flags(np, "reset-gpios", 0, &flags);
+	if (gpio_is_valid(reset_gpio)) {
+		unsigned long gpio_flags;
+
+		if (flags & OF_GPIO_ACTIVE_LOW)
+			gpio_flags = GPIOF_ACTIVE_LOW | GPIOF_OUT_INIT_LOW;
+		else
+			gpio_flags = GPIOF_OUT_INIT_HIGH;
+
+		ret = gpio_request_one(reset_gpio, gpio_flags,
+				"pwrseq-reset-gpios");
+		if (ret)
+			goto err;
+
+		pwrseq_gen->gpiod_reset = gpio_to_desc(reset_gpio);
+	} else {
+		if (reset_gpio == -ENOENT)
+			return 0;
+
+		ret = reset_gpio;
+		pr_err("Failed to get reset gpio on %s, err = %d\n",
+				np->full_name, reset_gpio);
+		goto err;
+	}
+
+	return ret;
+err:
+	clk_put(pwrseq_gen->clk);
+	return ret;
+}
+
+struct pwrseq *pwrseq_alloc_generic(void)
+{
+	struct pwrseq_generic *pwrseq_gen;
+
+	pwrseq_gen = kzalloc(sizeof(*pwrseq_gen), GFP_KERNEL);
+	if (!pwrseq_gen)
+		return ERR_PTR(-ENOMEM);
+
+	pwrseq_gen->pwrseq.get = pwrseq_generic_get;
+	pwrseq_gen->pwrseq.on = pwrseq_generic_on;
+	pwrseq_gen->pwrseq.off = pwrseq_generic_off;
+	pwrseq_gen->pwrseq.put = pwrseq_generic_put;
+	pwrseq_gen->pwrseq.free = pwrseq_generic_free;
+
+	pwrseq_register(&pwrseq_gen->pwrseq);
+	return &pwrseq_gen->pwrseq;
+}
+EXPORT_SYMBOL_GPL(pwrseq_alloc_generic);
diff --git a/include/linux/power/pwrseq.h b/include/linux/power/pwrseq.h
new file mode 100644
index 0000000..6c4b9a0
--- /dev/null
+++ b/include/linux/power/pwrseq.h
@@ -0,0 +1,50 @@
+#ifndef __LINUX_PWRSEQ_H
+#define __LINUX_PWRSEQ_H
+
+#include <linux/of.h>
+
+struct pwrseq {
+	char *name;
+	struct list_head node;
+	int (*get)(struct device_node *np, struct pwrseq *p);
+	int (*on)(struct device_node *np, struct pwrseq *p);
+	void (*off)(struct pwrseq *p);
+	void (*put)(struct pwrseq *p);
+	void (*free)(struct pwrseq *p);
+};
+
+#if IS_ENABLED(CONFIG_POWER_SEQUENCE)
+int pwrseq_get(struct device_node *np, struct pwrseq *p);
+int pwrseq_on(struct device_node *np, struct pwrseq *p);
+void pwrseq_off(struct pwrseq *p);
+void pwrseq_put(struct pwrseq *p);
+void pwrseq_free(struct pwrseq *p);
+void pwrseq_register(struct pwrseq *pwrseq);
+void pwrseq_unregister(struct pwrseq *pwrseq);
+
+#else
+static inline int pwrseq_get(struct device_node *np, struct pwrseq *p)
+{
+	return 0;
+}
+static inline int pwrseq_on(struct device_node *np, struct pwrseq *p)
+{
+	return 0;
+}
+static inline void pwrseq_off(struct pwrseq *p) {}
+static inline void pwrseq_put(struct pwrseq *p) {}
+static inline void pwrseq_free(struct pwrseq *p) {}
+static inline void pwrseq_register(struct pwrseq *pwrseq) {}
+static inline void pwrseq_unregister(struct pwrseq *pwrseq) {}
+#endif /* CONFIG_POWER_SEQUENCE */
+
+#if IS_ENABLED(CONFIG_PWRSEQ_GENERIC)
+struct pwrseq *pwrseq_alloc_generic(void);
+#else
+static inline struct pwrseq *pwrseq_alloc_generic(void)
+{
+	return NULL;
+}
+#endif /* CONFIG_PWRSEQ_GENERIC */
+
+#endif  /* __LINUX_PWRSEQ_H */
-- 
1.9.1

[PATCH v3 3/6] binding-doc: usb: usb-device: add optional properties for power sequence

[Peter Chen]

Add optional properties for power sequence.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 Documentation/devicetree/bindings/usb/usb-device.txt | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/usb/usb-device.txt b/Documentation/devicetree/bindings/usb/usb-device.txt
index 1c35e7b..3661dd2 100644
--- a/Documentation/devicetree/bindings/usb/usb-device.txt
+++ b/Documentation/devicetree/bindings/usb/usb-device.txt
@@ -13,6 +13,10 @@ Required properties:
 - reg: the port number which this device is connecting to, the range
   is 1-31.
 
+Optional properties:
+power sequence properties, see
+Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt for detail
+
 Example:
 
 &usb1 {
@@ -21,8 +25,12 @@ Example:
 	#address-cells = <1>;
 	#size-cells = <0>;
 
-	hub: genesys at 1 {
+	genesys: hub at 1 {
 		compatible = "usb5e3,608";
 		reg = <1>;
+
+		clocks = <&clks IMX6SX_CLK_CKO>;
+		reset-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>; /* hub reset pin */
+		reset-duration-us = <10>;
 	};
 }
-- 
1.9.1

[PATCH v3 4/6] 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 <peter.chen@nxp.com>
---
 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 da36b78..5fd2f54 100644
--- a/drivers/usb/core/Makefile
+++ b/drivers/usb/core/Makefile
@@ -10,5 +10,6 @@ usbcore-y += port.o
 usbcore-$(CONFIG_OF)		+= 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(&usb_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(&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);
 
@@ -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_head	pwrseq_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 0000000..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 <peter.chen@nxp.com>
+ *
+ * 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 <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/list.h>
+#include <linux/of.h>
+#include <linux/power/pwrseq.h>
+#include <linux/slab.h>
+#include <linux/usb.h>
+#include <linux/usb/hcd.h>
+
+#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);
+	pwrseq_node->pwrseq = pwrseq;
+	list_add(&pwrseq_node->list, &hub->pwrseq_on_list);
+
+	return 0;
+
+pwr_put:
+	pwrseq_put(pwrseq);
+pwr_free:
+	pwrseq_free(pwrseq);
+	return ret;
+}
+
+int hub_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.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,
+			&hub->pwrseq_on_list, list) {
+		pwrseq = pwrseq_node->pwrseq;
+		pwrseq_off(pwrseq);
+		pwrseq_put(pwrseq);
+		pwrseq_free(pwrseq);
+		list_del(&pwrseq_node->list);
+		kfree(pwrseq_node);
+	}
+}
-- 
1.9.1

[PATCH v3 5/6] usb: chipidea: let chipidea core device of_node equal's glue layer device of_node

[Peter Chen]

From: Peter Chen <peter.chen@freescale.com>

At device tree, we have no device node for chipidea core,
the glue layer's node is the parent node for host and udc
device. But in related driver, the parent device is chipidea
core. So, in order to let the common driver get parent's node,
we let the core's device node equals glue layer device node.

Signed-off-by: Peter Chen <peter.chen@freescale.com>
Tested-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
---
 drivers/usb/chipidea/core.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 69426e6..0d05812 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -914,6 +914,16 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 	if (!ci)
 		return -ENOMEM;
 
+	/*
+	 * At device tree, we have no device node for chipidea core,
+	 * the glue layer's node is the parent node for host and udc
+	 * device. But in related driver, the parent device is chipidea
+	 * core. So, in order to let the common driver get parent's node,
+	 * we let the core's device node equals glue layer's node.
+	 */
+	if (dev->parent && dev->parent->of_node)
+		dev->of_node = dev->parent->of_node;
+
 	ci->dev = dev;
 	ci->platdata = dev_get_platdata(dev);
 	ci->imx28_write_fix = !!(ci->platdata->flags &
-- 
1.9.1

[PATCH v3 6/6] ARM: dts: imx6qdl-udoo.dtsi: fix onboard USB HUB property

[Peter Chen]

The current dts describes USB HUB's property at USB controller's
entry, it is improper. The USB HUB should be the child node
under USB controller, and power sequence properties are under
it.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 arch/arm/boot/dts/imx6qdl-udoo.dtsi | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/arch/arm/boot/dts/imx6qdl-udoo.dtsi b/arch/arm/boot/dts/imx6qdl-udoo.dtsi
index 3bee2f9..f29a72c2f 100644
--- a/arch/arm/boot/dts/imx6qdl-udoo.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-udoo.dtsi
@@ -9,6 +9,8 @@
  *
  */
 
+#include <dt-bindings/gpio/gpio.h>
+
 / {
 	aliases {
 		backlight = &backlight;
@@ -58,17 +60,6 @@
 		#address-cells = <1>;
 		#size-cells = <0>;
 
-		reg_usb_h1_vbus: regulator at 0 {
-			compatible = "regulator-fixed";
-			reg = <0>;
-			regulator-name = "usb_h1_vbus";
-			regulator-min-microvolt = <5000000>;
-			regulator-max-microvolt = <5000000>;
-			enable-active-high;
-			startup-delay-us = <2>; /* USB2415 requires a POR of 1 us minimum */
-			gpio = <&gpio7 12 0>;
-		};
-
 		reg_panel: regulator at 1 {
 			compatible = "regulator-fixed";
 			reg = <1>;
@@ -259,9 +250,18 @@
 &usbh1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_usbh>;
-	vbus-supply = <&reg_usb_h1_vbus>;
-	clocks = <&clks IMX6QDL_CLK_CKO>;
 	status = "okay";
+
+	#address-cells = <1>;
+	#size-cells = <0>;
+	usb2415: hub at 1 {
+		compatible = "usb424,2514";
+		reg = <1>;
+
+		clocks = <&clks IMX6QDL_CLK_CKO>;
+		reset-gpios = <&gpio7 12 GPIO_ACTIVE_LOW>;
+		reset-duration-us = <3000>;
+	};
 };
 
 &usdhc3 {
-- 
1.9.1

[PATCH v3 5/6] usb: chipidea: let chipidea core device of_node equal's glue layer device of_node

[Russell King - ARM Linux]

On Wed, Jul 20, 2016 at 05:40:28PM +0800, Peter Chen wrote:
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 69426e6..0d05812 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -914,6 +914,16 @@ static int ci_hdrc_probe(struct platform_device *pdev)
>  	if (!ci)
>  		return -ENOMEM;
>  
> +	/*
> +	 * At device tree, we have no device node for chipidea core,
> +	 * the glue layer's node is the parent node for host and udc
> +	 * device. But in related driver, the parent device is chipidea
> +	 * core. So, in order to let the common driver get parent's node,
> +	 * we let the core's device node equals glue layer's node.
> +	 */
> +	if (dev->parent && dev->parent->of_node)
> +		dev->of_node = dev->parent->of_node;

This is a dangerous thing to do.  You're changing the dev->of_node of
_this_ device, which means that _this_ driver will no longer match
the device if you remove and reinsert the driver module, or unbind
and try to re-bind the device to this driver.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH v3 5/6] usb: chipidea: let chipidea core device of_node equal's glue layer device of_node

[Peter Chen]

On Thu, Jul 21, 2016 at 10:14:38AM +0100, Russell King - ARM Linux wrote:
> On Wed, Jul 20, 2016 at 05:40:28PM +0800, Peter Chen wrote:
> > diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> > index 69426e6..0d05812 100644
> > --- a/drivers/usb/chipidea/core.c
> > +++ b/drivers/usb/chipidea/core.c
> > @@ -914,6 +914,16 @@ static int ci_hdrc_probe(struct platform_device *pdev)
> >  	if (!ci)
> >  		return -ENOMEM;
> >  
> > +	/*
> > +	 * At device tree, we have no device node for chipidea core,
> > +	 * the glue layer's node is the parent node for host and udc
> > +	 * device. But in related driver, the parent device is chipidea
> > +	 * core. So, in order to let the common driver get parent's node,
> > +	 * we let the core's device node equals glue layer's node.
> > +	 */
> > +	if (dev->parent && dev->parent->of_node)
> > +		dev->of_node = dev->parent->of_node;
> 
> This is a dangerous thing to do.  You're changing the dev->of_node of
> _this_ device, which means that _this_ driver will no longer match
> the device if you remove and reinsert the driver module, or unbind
> and try to re-bind the device to this driver.
> 

Thanks for commenting it.

I have tested load/unload, it does not show any problems.

The chipidea core device is created by code at runtime, not by device node.
And we have NO device node for this chipidea core device at dts.

-- 

Best Regards,
Peter Chen

[PATCH v3 5/6] usb: chipidea: let chipidea core device of_node equal's glue layer device of_node

[Russell King - ARM Linux]

On Thu, Jul 21, 2016 at 05:20:12PM +0800, Peter Chen wrote:
> On Thu, Jul 21, 2016 at 10:14:38AM +0100, Russell King - ARM Linux wrote:
> > On Wed, Jul 20, 2016 at 05:40:28PM +0800, Peter Chen wrote:
> > > diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> > > index 69426e6..0d05812 100644
> > > --- a/drivers/usb/chipidea/core.c
> > > +++ b/drivers/usb/chipidea/core.c
> > > @@ -914,6 +914,16 @@ static int ci_hdrc_probe(struct platform_device *pdev)
> > >  	if (!ci)
> > >  		return -ENOMEM;
> > >  
> > > +	/*
> > > +	 * At device tree, we have no device node for chipidea core,
> > > +	 * the glue layer's node is the parent node for host and udc
> > > +	 * device. But in related driver, the parent device is chipidea
> > > +	 * core. So, in order to let the common driver get parent's node,
> > > +	 * we let the core's device node equals glue layer's node.
> > > +	 */
> > > +	if (dev->parent && dev->parent->of_node)
> > > +		dev->of_node = dev->parent->of_node;
> > 
> > This is a dangerous thing to do.  You're changing the dev->of_node of
> > _this_ device, which means that _this_ driver will no longer match
> > the device if you remove and reinsert the driver module, or unbind
> > and try to re-bind the device to this driver.
> > 
> 
> Thanks for commenting it.
> 
> I have tested load/unload, it does not show any problems.
> 
> The chipidea core device is created by code at runtime, not by device node.
> And we have NO device node for this chipidea core device at dts.

Okay, so we still probably have the bind/unbind problem, where "dev"
can be matched by the driver which claimed "dev->parent".  Remember,
in an OF environment, driver matching is done by the compatible
property, which is accessed via dev->of_node.

Therefore, I would suggest that you NULL dev->of_node in the error
cleanup paths and in the remove function, so you don't have an
unbound device with a duplicated (but inappropriate) dev->of_node
pointer.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH v3 5/6] usb: chipidea: let chipidea core device of_node equal's glue layer device of_node

[Peter Chen]

On Thu, Jul 21, 2016 at 10:41:28AM +0100, Russell King - ARM Linux wrote:
> On Thu, Jul 21, 2016 at 05:20:12PM +0800, Peter Chen wrote:
> > On Thu, Jul 21, 2016 at 10:14:38AM +0100, Russell King - ARM Linux wrote:
> > > On Wed, Jul 20, 2016 at 05:40:28PM +0800, Peter Chen wrote:
> > > > diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> > > > index 69426e6..0d05812 100644
> > > > --- a/drivers/usb/chipidea/core.c
> > > > +++ b/drivers/usb/chipidea/core.c
> > > > @@ -914,6 +914,16 @@ static int ci_hdrc_probe(struct platform_device *pdev)
> > > >  	if (!ci)
> > > >  		return -ENOMEM;
> > > >  
> > > > +	/*
> > > > +	 * At device tree, we have no device node for chipidea core,
> > > > +	 * the glue layer's node is the parent node for host and udc
> > > > +	 * device. But in related driver, the parent device is chipidea
> > > > +	 * core. So, in order to let the common driver get parent's node,
> > > > +	 * we let the core's device node equals glue layer's node.
> > > > +	 */
> > > > +	if (dev->parent && dev->parent->of_node)
> > > > +		dev->of_node = dev->parent->of_node;
> > > 
> > > This is a dangerous thing to do.  You're changing the dev->of_node of
> > > _this_ device, which means that _this_ driver will no longer match
> > > the device if you remove and reinsert the driver module, or unbind
> > > and try to re-bind the device to this driver.
> > > 
> > 
> > Thanks for commenting it.
> > 
> > I have tested load/unload, it does not show any problems.
> > 
> > The chipidea core device is created by code at runtime, not by device node.
> > And we have NO device node for this chipidea core device at dts.
> 
> Okay, so we still probably have the bind/unbind problem, where "dev"
> can be matched by the driver which claimed "dev->parent".  Remember,
> in an OF environment, driver matching is done by the compatible
> property, which is accessed via dev->of_node.
> 
> Therefore, I would suggest that you NULL dev->of_node in the error
> cleanup paths and in the remove function, so you don't have an
> unbound device with a duplicated (but inappropriate) dev->of_node
> pointer.
> 

Although it does no mismatch between driver and device due to the driver
has no of_match_table, I find it has below re-request pinctrl error
after re-bind, that's due to the parent device which has of_node
and there is a pinctrl property in it.

imx6sx-pinctrl 20e0000.iomuxc: pin MX6SX_PAD_GPIO1_IO10 already requested by 2184000.usb;
cannot claim for ci_hdrc.0

After adding your suggestion, this error has gone, thanks.


-- 

Best Regards,
Peter Chen

[PATCH v3 4/6] usb: core: add power sequence handling for USB devices

[Joshua Clayton]

On 07/20/2016 02:40 AM, 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 <peter.chen@nxp.com>
> ---
>  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 da36b78..5fd2f54 100644
> --- a/drivers/usb/core/Makefile
> +++ b/drivers/usb/core/Makefile
> @@ -10,5 +10,6 @@ usbcore-y += port.o
>  usbcore-$(CONFIG_OF)		+= 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(&usb_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(&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);
>  
> @@ -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_head	pwrseq_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 0000000..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 <peter.chen@nxp.com>
> + *
> + * 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 <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/list.h>
> +#include <linux/of.h>
> +#include <linux/power/pwrseq.h>
> +#include <linux/slab.h>
> +#include <linux/usb.h>
> +#include <linux/usb/hcd.h>
> +
> +#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);
> +	pwrseq_node->pwrseq = pwrseq;
> +	list_add(&pwrseq_node->list, &hub->pwrseq_on_list);
> +
> +	return 0;
> +
> +pwr_put:
> +	pwrseq_put(pwrseq);
> +pwr_free:
> +	pwrseq_free(pwrseq);
> +	return ret;
> +}
> +
> +int hub_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.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,
> +			&hub->pwrseq_on_list, list) {
> +		pwrseq = pwrseq_node->pwrseq;
> +		pwrseq_off(pwrseq);
> +		pwrseq_put(pwrseq);
> +		pwrseq_free(pwrseq);
> +		list_del(&pwrseq_node->list);
> +		kfree(pwrseq_node);
> +	}
> +}
Patch 4 does not apply for me
(The Makefile has  a "usbcore-$(CONFIG_OF) += of.o"
line which I don't see in 4.7 or linux/next master).
Is there another patch set this series depends on?

[PATCH v3 4/6] usb: core: add power sequence handling for USB devices

[Peter Chen]

On Wed, Jul 27, 2016 at 09:25:11AM -0700, Joshua Clayton wrote:
> 
> 
> On 07/20/2016 02:40 AM, 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 <peter.chen@nxp.com>
> > ---
> >  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 da36b78..5fd2f54 100644
> > --- a/drivers/usb/core/Makefile
> > +++ b/drivers/usb/core/Makefile
> > @@ -10,5 +10,6 @@ usbcore-y += port.o
> >  usbcore-$(CONFIG_OF)		+= 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(&usb_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(&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);
> >  
> > @@ -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_head	pwrseq_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 0000000..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 <peter.chen@nxp.com>
> > + *
> > + * 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 <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#include <linux/list.h>
> > +#include <linux/of.h>
> > +#include <linux/power/pwrseq.h>
> > +#include <linux/slab.h>
> > +#include <linux/usb.h>
> > +#include <linux/usb/hcd.h>
> > +
> > +#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);
> > +	pwrseq_node->pwrseq = pwrseq;
> > +	list_add(&pwrseq_node->list, &hub->pwrseq_on_list);
> > +
> > +	return 0;
> > +
> > +pwr_put:
> > +	pwrseq_put(pwrseq);
> > +pwr_free:
> > +	pwrseq_free(pwrseq);
> > +	return ret;
> > +}
> > +
> > +int hub_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.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,
> > +			&hub->pwrseq_on_list, list) {
> > +		pwrseq = pwrseq_node->pwrseq;
> > +		pwrseq_off(pwrseq);
> > +		pwrseq_put(pwrseq);
> > +		pwrseq_free(pwrseq);
> > +		list_del(&pwrseq_node->list);
> > +		kfree(pwrseq_node);
> > +	}
> > +}
> Patch 4 does not apply for me
> (The Makefile has  a "usbcore-$(CONFIG_OF) += of.o"
> line which I don't see in 4.7 or linux/next master).
> Is there another patch set this series depends on?

It seems below patch has not been queued:

https://git.kernel.org/cgit/linux/kernel/git/peter.chen/usb.git/commit/?h=add_pwrseq_for_usb&id=2efd73d0f935ffbc66135553da0ac807c0b4c3fe

This pwrseq-lib patch set does not depend on it, I will send patch set
on top of a clean mainline next time, sorry for confusing. 

-- 

Best Regards,
Peter Chen

[PATCH v3 0/6] power: add power sequence library

[Joshua Clayton]

Hi, Peter

On 07/20/2016 02:40 AM, Peter Chen wrote:
> Hi all,
>
> This is a follow-up for my last power sequence framework patch set [1].
> According to Rob Herring and Ulf Hansson's comments[2], I use a generic
> power sequence library for parsing the power sequence elements on DT,
> and implement generic power sequence on library. The host driver
> can allocate power sequence instance, and calls pwrseq APIs accordingly.
>
> In future, if there are special power sequence requirements, the special
> power sequence library can be created.
>
> This patch set is tested on i.mx6 sabresx evk using a dts change, I use
> two hot-plug devices to simulate this use case, the related binding
> change is updated at patch [1/6], The udoo board changes were tested
> using my last power sequence patch set.[3]
>
> Except for hard-wired MMC and USB devices, I find the USB ULPI PHY also
> need to power on itself before it can be found by ULPI bus.
>
> [1] http://www.spinics.net/lists/linux-usb/msg142755.html
> [2] http://www.spinics.net/lists/linux-usb/msg143106.html
> [3] http://www.spinics.net/lists/linux-usb/msg142815.html
>
> Changes for v3:
> - Delete "power-sequence" property at binding-doc, and change related code
>   at both library and user code.
> - Change binding-doc example node name with Rob's comments
> - of_get_named_gpio_flags only gets the gpio, but without setting gpio flags,
>   add additional code request gpio with proper gpio flags
> - Add Philipp Zabel's Ack and MAINTAINER's entry
>
> Changes for v2:
> - Delete "pwrseq" prefix and clock-names for properties at dt binding
> - Should use structure not but its pointer for kzalloc
> - Since chipidea core has no of_node, let core's of_node equals glue
>   layer's at core's probe
>
> Peter Chen (6):
>   binding-doc: power: pwrseq-generic: add binding doc for generic power
>     sequence library
>   power: add power sequence library
>   binding-doc: usb: usb-device: add optional properties for power
>     sequencediff --git a/drivers/usb/core/Makefile b/drivers/usb/core/Makefile
> index 9780877..da36b78 100644
> --- a/drivers/usb/core/Makefile
> +++ b/drivers/usb/core/Makefile
> @@ -5,8 +5,9 @@
>  usbcore-y := usb.o hub.o hcd.o urb.o message.o driver.o
>  usbcore-y += config.o file.o buffer.o sysfs.o endpoint.o
>  usbcore-y += devio.o notify.o generic.o quirks.o devices.o
> -usbcore-y += port.o of.o
> +usbcore-y += port.o
>  
> +usbcore-$(CONFIG_OF)		+= of.o
>  usbcore-$(CONFIG_PCI)		+= hcd-pci.o
>  usbcore-$(CONFIG_ACPI)		+= usb-acpi.o
>  
> diff --git a/drivers/usb/core/of.c b/drivers/usb/core/of.c
> index 2289700..3de4f88 100644
> --- a/drivers/usb/core/of.c
> +++ b/drivers/usb/core/of.c
> @@ -18,6 +18,7 @@
>   */
>  
>  #include <linux/of.h>
> +#include <linux/usb/of.h>
>   usb: core: add power sequence handling for USB devices
>   usb: chipidea: let chipidea core device of_node equal's glue layer
>     device of_node
>   ARM: dts: imx6qdl-udoo.dtsi: fix onboard USB HUB property
>
>  .../bindings/power/pwrseq/pwrseq-generic.txt       |  48 +++++++
>  .../devicetree/bindings/usb/usb-device.txt         |  10 +-
>  MAINTAINERS                                        |   9 ++
>  arch/arm/boot/dts/imx6qdl-udoo.dtsi                |  26 ++--
>  drivers/power/Kconfig                              |   1 +
>  drivers/power/Makefile                             |   1 +
>  drivers/power/pwrseq/Kconfig                       |  20 +++
>  drivers/power/pwrseq/Makefile                      |   2 +
>  drivers/power/pwrseq/core.c                        |  71 ++++++++++
>  drivers/power/pwrseq/pwrseq_generic.c              | 151 +++++++++++++++++++++
>  drivers/usb/chipidea/core.c                        |  10 ++
>  drivers/usb/core/Makefile                          |   1 +
>  drivers/usb/core/hub.c                             |  12 +-
>  drivers/usb/core/hub.h                             |  12 ++
>  drivers/usb/core/pwrseq.c                          | 100 ++++++++++++++
>  include/linux/power/pwrseq.h                       |  50 +++++++
>  16 files changed, 507 insertions(+), 17 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
>  create mode 100644 drivers/power/pwrseq/Kconfig
>  create mode 100644 drivers/power/pwrseq/Makefile
>  create mode 100644 drivers/power/pwrseq/core.c
>  create mode 100644 drivers/power/pwrseq/pwrseq_generic.c
>  create mode 100644 drivers/usb/core/pwrseq.c
>  create mode 100644 include/linux/power/pwrseq.h
>

With these patches our on-board USB hub,
a Microchip USB2513BI-AEZG,  works exactly as it does
with the fake regulator kludge, so...
Tested-by Joshua Clayton <stillcompiling@gmail.com>

I assume there is a v4 coming due to rmk's comments on patch 5.
I couldn't figure out where to null the of_node in error paths, but in testing
we did put a line of code to null the of_node on release.

but...
As an aside,
I was hoping this series would magically fix a problem
with the imx6q-evi which has forced us to disable
runtime power management. But it did not. :(

If runtime power management is enabled,  when nothing is
connected to the hub it disconnects and immediately reconnects
about every 2 seconds, and after some seemingly random number
of these events the whole system hangs (possibly udev related?).

Our board has the vbus detect on the hub connected directly to 3.3v,
rather than to the imx6's usbh1_vbus line. This is listed as a supported
configuration, but might be the source of the problem.

Thanks,
Joshua

[PATCH v3 4/6] usb: core: add power sequence handling for USB devices

[Joshua Clayton]

Peter,

On 07/27/2016 06:45 PM, Peter Chen wrote:
> On Wed, Jul 27, 2016 at 09:25:11AM -0700, Joshua Clayton wrote:
>> Patch 4 does not apply for me
>> (The Makefile has  a "usbcore-$(CONFIG_OF) += of.o"
>> line which I don't see in 4.7 or linux/next master).
>> Is there another patch set this series depends on?
> It seems below patch has not been queued:
>
> https://git.kernel.org/cgit/linux/kernel/git/peter.chen/usb.git/commit/?h=add_pwrseq_for_usb&id=2efd73d0f935ffbc66135553da0ac807c0b4c3fe
>
> This pwrseq-lib patch set does not depend on it, I will send patch set
> on top of a clean mainline next time, sorry for confusing. 
>
Good to know.
I just wanted to make sure I wasn't missing something important.

Thanks,

Joshua Clayton

[PATCH v3 0/6] power: add power sequence library

[Fabio Estevam]

Hi Joshua,

On Thu, Jul 28, 2016 at 12:56 PM, Joshua Clayton
<stillcompiling@gmail.com> wrote:

> I assume there is a v4 coming due to rmk's comments on patch 5.
> I couldn't figure out where to null the of_node in error paths, but in testing
> we did put a line of code to null the of_node on release.
>
> but...
> As an aside,
> I was hoping this series would magically fix a problem
> with the imx6q-evi which has forced us to disable
> runtime power management. But it did not. :(

I also see a similar problem on a mx28 board with a hub.

Does it help in your case if you pass 'usbcore.autosuspend=-1' in the
kernel command line?

Regards,

Fabio Estevam

[PATCH v3 0/6] power: add power sequence library

[Peter Chen]

On Thu, Jul 28, 2016 at 08:56:40AM -0700, Joshua Clayton wrote:
> Hi, Peter
> 
> On 07/20/2016 02:40 AM, Peter Chen wrote:
> > Hi all,
> >
> > This is a follow-up for my last power sequence framework patch set [1].
> > According to Rob Herring and Ulf Hansson's comments[2], I use a generic
> > power sequence library for parsing the power sequence elements on DT,
> > and implement generic power sequence on library. The host driver
> > can allocate power sequence instance, and calls pwrseq APIs accordingly.
> >
> > In future, if there are special power sequence requirements, the special
> > power sequence library can be created.
> >
> > This patch set is tested on i.mx6 sabresx evk using a dts change, I use
> > two hot-plug devices to simulate this use case, the related binding
> > change is updated at patch [1/6], The udoo board changes were tested
> > using my last power sequence patch set.[3]
> >
> > Except for hard-wired MMC and USB devices, I find the USB ULPI PHY also
> > need to power on itself before it can be found by ULPI bus.
> >
> > [1] http://www.spinics.net/lists/linux-usb/msg142755.html
> > [2] http://www.spinics.net/lists/linux-usb/msg143106.html
> > [3] http://www.spinics.net/lists/linux-usb/msg142815.html
> >
> > Changes for v3:
> > - Delete "power-sequence" property at binding-doc, and change related code
> >   at both library and user code.
> > - Change binding-doc example node name with Rob's comments
> > - of_get_named_gpio_flags only gets the gpio, but without setting gpio flags,
> >   add additional code request gpio with proper gpio flags
> > - Add Philipp Zabel's Ack and MAINTAINER's entry
> >
> > Changes for v2:
> > - Delete "pwrseq" prefix and clock-names for properties at dt binding
> > - Should use structure not but its pointer for kzalloc
> > - Since chipidea core has no of_node, let core's of_node equals glue
> >   layer's at core's probe
> >
> > Peter Chen (6):
> >   binding-doc: power: pwrseq-generic: add binding doc for generic power
> >     sequence library
> >   power: add power sequence library
> >   binding-doc: usb: usb-device: add optional properties for power
> >     sequencediff --git a/drivers/usb/core/Makefile b/drivers/usb/core/Makefile
> > index 9780877..da36b78 100644
> > --- a/drivers/usb/core/Makefile
> > +++ b/drivers/usb/core/Makefile
> > @@ -5,8 +5,9 @@
> >  usbcore-y := usb.o hub.o hcd.o urb.o message.o driver.o
> >  usbcore-y += config.o file.o buffer.o sysfs.o endpoint.o
> >  usbcore-y += devio.o notify.o generic.o quirks.o devices.o
> > -usbcore-y += port.o of.o
> > +usbcore-y += port.o
> >  
> > +usbcore-$(CONFIG_OF)		+= of.o
> >  usbcore-$(CONFIG_PCI)		+= hcd-pci.o
> >  usbcore-$(CONFIG_ACPI)		+= usb-acpi.o
> >  
> > diff --git a/drivers/usb/core/of.c b/drivers/usb/core/of.c
> > index 2289700..3de4f88 100644
> > --- a/drivers/usb/core/of.c
> > +++ b/drivers/usb/core/of.c
> > @@ -18,6 +18,7 @@
> >   */
> >  
> >  #include <linux/of.h>
> > +#include <linux/usb/of.h>
> >   usb: core: add power sequence handling for USB devices
> >   usb: chipidea: let chipidea core device of_node equal's glue layer
> >     device of_node
> >   ARM: dts: imx6qdl-udoo.dtsi: fix onboard USB HUB property
> >
> >  .../bindings/power/pwrseq/pwrseq-generic.txt       |  48 +++++++
> >  .../devicetree/bindings/usb/usb-device.txt         |  10 +-
> >  MAINTAINERS                                        |   9 ++
> >  arch/arm/boot/dts/imx6qdl-udoo.dtsi                |  26 ++--
> >  drivers/power/Kconfig                              |   1 +
> >  drivers/power/Makefile                             |   1 +
> >  drivers/power/pwrseq/Kconfig                       |  20 +++
> >  drivers/power/pwrseq/Makefile                      |   2 +
> >  drivers/power/pwrseq/core.c                        |  71 ++++++++++
> >  drivers/power/pwrseq/pwrseq_generic.c              | 151 +++++++++++++++++++++
> >  drivers/usb/chipidea/core.c                        |  10 ++
> >  drivers/usb/core/Makefile                          |   1 +
> >  drivers/usb/core/hub.c                             |  12 +-
> >  drivers/usb/core/hub.h                             |  12 ++
> >  drivers/usb/core/pwrseq.c                          | 100 ++++++++++++++
> >  include/linux/power/pwrseq.h                       |  50 +++++++
> >  16 files changed, 507 insertions(+), 17 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
> >  create mode 100644 drivers/power/pwrseq/Kconfig
> >  create mode 100644 drivers/power/pwrseq/Makefile
> >  create mode 100644 drivers/power/pwrseq/core.c
> >  create mode 100644 drivers/power/pwrseq/pwrseq_generic.c
> >  create mode 100644 drivers/usb/core/pwrseq.c
> >  create mode 100644 include/linux/power/pwrseq.h
> >
> 
> With these patches our on-board USB hub,
> a Microchip USB2513BI-AEZG,  works exactly as it does
> with the fake regulator kludge, so...
> Tested-by Joshua Clayton <stillcompiling@gmail.com>
> 

Thanks.

> I assume there is a v4 coming due to rmk's comments on patch 5.
> I couldn't figure out where to null the of_node in error paths, but in testing
> we did put a line of code to null the of_node on release.

I will add it at v4.

> 
> but...
> As an aside,
> I was hoping this series would magically fix a problem
> with the imx6q-evi which has forced us to disable
> runtime power management. But it did not. :(
> 
> If runtime power management is enabled,  when nothing is
> connected to the hub it disconnects and immediately reconnects
> about every 2 seconds, and after some seemingly random number
> of these events the whole system hangs (possibly udev related?).
> 
> Our board has the vbus detect on the hub connected directly to 3.3v,
> rather than to the imx6's usbh1_vbus line. This is listed as a supported
> configuration, but might be the source of the problem.
> 

You could have a thread to discuss this problem.

But if Fabio's suggestion can work for you, it may not be hardware
problem. Make sure you use the correct low power mode flow, you can
refer to the latest mainline code for it.

-- 

Best Regards,
Peter Chen

[PATCH v3 1/6] binding-doc: power: pwrseq-generic: add binding doc for generic power sequence library

[Peter Chen]

On Wed, Jul 20, 2016 at 05:40:24PM +0800, Peter Chen wrote:
> Add binding doc for generic power sequence library.
> 
> Signed-off-by: Peter Chen <peter.chen@nxp.com>
> Acked-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
>  .../bindings/power/pwrseq/pwrseq-generic.txt       | 48 ++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
> 
> diff --git a/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
> new file mode 100644
> index 0000000..48bb3e1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
> @@ -0,0 +1,48 @@
> +The generic power sequence library
> +
> +Some hard-wired devices (eg USB/MMC) need to do power sequence before
> +the device can be enumerated on the bus, the typical power sequence
> +like: enable USB PHY clock, toggle reset pin, etc. But current
> +Linux device driver lacks of such code to do it, it may cause some
> +hard-wired devices works abnormal or can't be recognized by
> +controller at all. The power sequence will be done before this device
> +can be found at the bus.
> +
> +The power sequence properties is under the device node.
> +
> +Optional properties:
> +- clocks: the input clock for device.
> +- reset-gpios: Should specify the GPIO for reset.
> +- reset-duration-us: the duration in microsecond for assert reset signal.
> +
> +Below is the example of USB power sequence properties on USB device
> +nodes which have two level USB hubs.
> +
> +&usbotg1 {
> +	vbus-supply = <&reg_usb_otg1_vbus>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_usb_otg1_id>;
> +	status = "okay";
> +
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	genesys: hub at 1 {
> +		compatible = "usb5e3,608";
> +		reg = <1>;
> +
> +		clocks = <&clks IMX6SX_CLK_CKO>;
> +		reset-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>; /* hub reset pin */
> +		reset-duration-us = <10>;
> +
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		asix: ethernet at 1 {
> +			compatible = "usbb95,1708";
> +			reg = <1>;
> +
> +			clocks = <&clks IMX6SX_CLK_IPG>;
> +			reset-gpios = <&gpio4 6 GPIO_ACTIVE_LOW>; /* ethernet_rst */
> +			reset-duration-us = <15>;
> +		};
> +	};
> +};
> -- 
> 1.9.1

Rob, would you please help to ack dts changes in this series if you are
ok with them?

-- 

Best Regards,
Peter Chen

[v3,2/6] power: add power sequence library

[Matthias Kaehlcke]

Hi Peter,

Thanks for your work on this, a few comments inline


On 07/20/2016 02:40 AM, Peter Chen wrote:

> ...
>
> +static int pwrseq_generic_on(struct device_node *np, struct pwrseq *pwrseq)
> +{
>
> ...
>
> +	if (gpiod_reset) {
> +		u32 duration_us = 50;
> +
> +		of_property_read_u32(np, "reset-duration-us",
> +				&duration_us);
> +		usleep_range(duration_us, duration_us + 10);
The end of the range could allow for more margin. Also consider busy 
looping for very short delays as in 
http://lxr.free-electrons.com/source/drivers/regulator/core.c#L2062
> ...
>
> +static int pwrseq_generic_get(struct device_node *np, struct pwrseq *pwrseq)
> +{
> +	struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
> +	enum of_gpio_flags flags;
> +	int reset_gpio, ret = 0;
> +
> +	pwrseq_gen->clk = of_clk_get_by_name(np, NULL);
This only gets the first of potentially multiple clocks, is that intended?

[PATCH v3 1/6] binding-doc: power: pwrseq-generic: add binding doc for generic power sequence library

[Rob Herring]

On Wed, Jul 20, 2016 at 05:40:24PM +0800, Peter Chen wrote:
> Add binding doc for generic power sequence library.
> 
> Signed-off-by: Peter Chen <peter.chen@nxp.com>
> Acked-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
>  .../bindings/power/pwrseq/pwrseq-generic.txt       | 48 ++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt

Acked-by: Rob Herring <robh@kernel.org>

> 
> diff --git a/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
> new file mode 100644
> index 0000000..48bb3e1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
> @@ -0,0 +1,48 @@
> +The generic power sequence library
> +
> +Some hard-wired devices (eg USB/MMC) need to do power sequence before
> +the device can be enumerated on the bus, the typical power sequence
> +like: enable USB PHY clock, toggle reset pin, etc. But current
> +Linux device driver lacks of such code to do it, it may cause some
> +hard-wired devices works abnormal or can't be recognized by
> +controller at all. The power sequence will be done before this device
> +can be found at the bus.
> +
> +The power sequence properties is under the device node.
> +
> +Optional properties:
> +- clocks: the input clock for device.
> +- reset-gpios: Should specify the GPIO for reset.
> +- reset-duration-us: the duration in microsecond for assert reset signal.
> +
> +Below is the example of USB power sequence properties on USB device
> +nodes which have two level USB hubs.
> +
> +&usbotg1 {
> +	vbus-supply = <&reg_usb_otg1_vbus>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_usb_otg1_id>;
> +	status = "okay";
> +
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	genesys: hub at 1 {
> +		compatible = "usb5e3,608";
> +		reg = <1>;
> +
> +		clocks = <&clks IMX6SX_CLK_CKO>;
> +		reset-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>; /* hub reset pin */
> +		reset-duration-us = <10>;
> +
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		asix: ethernet at 1 {
> +			compatible = "usbb95,1708";
> +			reg = <1>;
> +
> +			clocks = <&clks IMX6SX_CLK_IPG>;
> +			reset-gpios = <&gpio4 6 GPIO_ACTIVE_LOW>; /* ethernet_rst */
> +			reset-duration-us = <15>;
> +		};
> +	};
> +};
> -- 
> 1.9.1
> 

[PATCH v3 3/6] binding-doc: usb: usb-device: add optional properties for power sequence

[Rob Herring]

On Wed, Jul 20, 2016 at 05:40:26PM +0800, Peter Chen wrote:
> Add optional properties for power sequence.
> 
> Signed-off-by: Peter Chen <peter.chen@nxp.com>
> ---
>  Documentation/devicetree/bindings/usb/usb-device.txt | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)

This probably should be put into a binding doc for the specific hub 
chip, but this is fine for now.

Acked-by: Rob Herring <robh@kernel.org>

[v3,2/6] power: add power sequence library

[Peter Chen]

On Fri, Jul 29, 2016 at 01:06:48PM -0700, Matthias Kaehlcke wrote:
> Hi Peter,
> 
> Thanks for your work on this, a few comments inline
> 
> 
> On 07/20/2016 02:40 AM, Peter Chen wrote:
> 
> >...
> >
> >+static int pwrseq_generic_on(struct device_node *np, struct pwrseq *pwrseq)
> >+{
> >
> >...
> >
> >+	if (gpiod_reset) {
> >+		u32 duration_us = 50;
> >+
> >+		of_property_read_u32(np, "reset-duration-us",
> >+				&duration_us);
> >+		usleep_range(duration_us, duration_us + 10);
> The end of the range could allow for more margin. Also consider busy
> looping for very short delays as in
> http://lxr.free-electrons.com/source/drivers/regulator/core.c#L2062

Thanks, I will change it.

> >...
> >
> >+static int pwrseq_generic_get(struct device_node *np, struct pwrseq *pwrseq)
> >+{
> >+	struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
> >+	enum of_gpio_flags flags;
> >+	int reset_gpio, ret = 0;
> >+
> >+	pwrseq_gen->clk = of_clk_get_by_name(np, NULL);
> This only gets the first of potentially multiple clocks, is that intended?

Since it is ran before the driver's probe, we thought one clock for
power sequence is enough. If your case really needs several clocks
to be enabled before your device can be found by bus, let me know.
I will add support for it. But what are the name for clocks, since
it is generic library? "gen1, gen2 and gen3"?

-- 

Best Regards,
Peter Chento 

[PATCH v3 0/6] power: add power sequence library

[Joshua Clayton]

On 07/28/2016 09:41 AM, Fabio Estevam wrote:
> Hi Joshua,
>
> On Thu, Jul 28, 2016 at 12:56 PM, Joshua Clayton
> <stillcompiling@gmail.com> wrote:
>
>> I assume there is a v4 coming due to rmk's comments on patch 5.
>> I couldn't figure out where to null the of_node in error paths, but in testing
>> we did put a line of code to null the of_node on release.
>>
>> but...
>> As an aside,
>> I was hoping this series would magically fix a problem
>> with the imx6q-evi which has forced us to disable
>> runtime power management. But it did not. :(
> I also see a similar problem on a mx28 board with a hub.
>
> Does it help in your case if you pass 'usbcore.autosuspend=-1' in the
> kernel command line?
>
> Regards,
>
> Fabio Estevam
Thanks a million, Fabio!

'usbcore.autosuspend=-1' quiets the errors.

~joshua

P.S. I guess this technically is a bug in chipidea usb.
I'll give it a quick once over, though I am not very familiar
with USB core or the CI driver.

[PATCH v3 0/6] power: add power sequence library

[Fabio Estevam]

On Mon, Aug 1, 2016 at 11:55 AM, Joshua Clayton
<stillcompiling@gmail.com> wrote:

> Thanks a million, Fabio!
>
> 'usbcore.autosuspend=-1' quiets the errors.
>
> ~joshua
>
> P.S. I guess this technically is a bug in chipidea usb.
> I'll give it a quick once over, though I am not very familiar
> with USB core or the CI driver.

Yes, it would be really nice if we can get a proper fix for this bug.

[PATCH v3 0/6] power: add power sequence library

[Peter Chen]

On Mon, Aug 01, 2016 at 07:55:10AM -0700, Joshua Clayton wrote:
> 
> 
> On 07/28/2016 09:41 AM, Fabio Estevam wrote:
> > Hi Joshua,
> >
> > On Thu, Jul 28, 2016 at 12:56 PM, Joshua Clayton
> > <stillcompiling@gmail.com> wrote:
> >
> >> I assume there is a v4 coming due to rmk's comments on patch 5.
> >> I couldn't figure out where to null the of_node in error paths, but in testing
> >> we did put a line of code to null the of_node on release.
> >>
> >> but...
> >> As an aside,
> >> I was hoping this series would magically fix a problem
> >> with the imx6q-evi which has forced us to disable
> >> runtime power management. But it did not. :(
> > I also see a similar problem on a mx28 board with a hub.
> >
> > Does it help in your case if you pass 'usbcore.autosuspend=-1' in the
> > kernel command line?
> >
> > Regards,
> >
> > Fabio Estevam
> Thanks a million, Fabio!
> 
> 'usbcore.autosuspend=-1' quiets the errors.
> 
> ~joshua
> 

If you are using imx6q, there are some patches which is not upstreamed
to workaround one SoC problem.

http://www.spinics.net/lists/linux-usb/msg98682.html

Try to implement the design for notify_suspend/resume at
drivers/usb/phy/phy-mxs-usb.c and drivers/usb/chipidea/host.c

You can refer to the code at:
http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/?h=imx_4.1.15_1.0.0_ga

-- 

Best Regards,
Peter Chen

[v3,2/6] power: add power sequence library

[Peter Chen]

On Fri, Jul 29, 2016 at 01:06:48PM -0700, Matthias Kaehlcke wrote:
> >...
> >
> >+static int pwrseq_generic_get(struct device_node *np, struct pwrseq *pwrseq)
> >+{
> >+	struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
> >+	enum of_gpio_flags flags;
> >+	int reset_gpio, ret = 0;
> >+
> >+	pwrseq_gen->clk = of_clk_get_by_name(np, NULL);
> This only gets the first of potentially multiple clocks, is that intended?

Matthias, I have added multiple input clocks support at v4 patch set,
and you are at cc list.

-- 

Best Regards,
Peter Chen