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

[PATCH v5 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 v5:
- Delete pwrseq_register/pwrseq_unregister, which is useless currently
- Fix the linker error when the pwrseq user is compiled as module

Changes for v4:
- Create the patch on next-20160722 
- Fix the of_node is not NULL after chipidea driver is unbinded [Patch 5/6]
- Using more friendly wait method for reset gpio [Patch 2/6]
- Support multiple input clocks [Patch 2/6]
- Add Rob Herring's ack for DT changes
- Add Joshua Clayton's Tested-by

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                        |  62 ++++++++
 drivers/power/pwrseq/pwrseq_generic.c              | 168 +++++++++++++++++++++
 drivers/usb/chipidea/core.c                        |  11 ++
 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                       |  47 ++++++
 16 files changed, 513 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 v5 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>
Acked-by: Rob Herring <robh@kernel.org>
---
 .../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..ebf0d47
--- /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 clocks 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@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@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 v5 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>
Tested-by Joshua Clayton <stillcompiling@gmail.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           |  62 +++++++++++++
 drivers/power/pwrseq/pwrseq_generic.c | 168 ++++++++++++++++++++++++++++++++++
 include/linux/power/pwrseq.h          |  47 ++++++++++
 8 files changed, 310 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 1ae6c84..407254b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9283,6 +9283,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@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..dcf96c4
--- /dev/null
+++ b/drivers/power/pwrseq/core.c
@@ -0,0 +1,62 @@
+/*
+ * 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;
+}
+EXPORT_SYMBOL(pwrseq_get);
+
+int pwrseq_on(struct device_node *np, struct pwrseq *p)
+{
+	if (p && p->on)
+		return p->on(np, p);
+
+	return -ENOTSUPP;
+}
+EXPORT_SYMBOL(pwrseq_on);
+
+void pwrseq_off(struct pwrseq *p)
+{
+	if (p && p->off)
+		p->off(p);
+}
+EXPORT_SYMBOL(pwrseq_off);
+
+void pwrseq_put(struct pwrseq *p)
+{
+	if (p && p->put)
+		p->put(p);
+}
+EXPORT_SYMBOL(pwrseq_put);
+
+void pwrseq_free(struct pwrseq *p)
+{
+	if (p && p->free)
+		p->free(p);
+}
+EXPORT_SYMBOL(pwrseq_free);
diff --git a/drivers/power/pwrseq/pwrseq_generic.c b/drivers/power/pwrseq/pwrseq_generic.c
new file mode 100644
index 0000000..8af626f
--- /dev/null
+++ b/drivers/power/pwrseq/pwrseq_generic.c
@@ -0,0 +1,168 @@
+/*
+ * 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 *clks[PWRSEQ_MAX_CLKS];
+};
+
+#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);
+
+	kfree(pwrseq_gen);
+}
+
+static void pwrseq_generic_put(struct pwrseq *pwrseq)
+{
+	struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
+	int clk;
+
+	if (pwrseq_gen->gpiod_reset)
+		gpiod_put(pwrseq_gen->gpiod_reset);
+
+	for (clk = 0; clk < PWRSEQ_MAX_CLKS; clk++)
+		clk_put(pwrseq_gen->clks[clk]);
+}
+
+static void pwrseq_generic_off(struct pwrseq *pwrseq)
+{
+	struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
+	int clk;
+
+	for (clk = PWRSEQ_MAX_CLKS - 1; clk >= 0; clk--)
+		clk_disable_unprepare(pwrseq_gen->clks[clk]);
+}
+
+static int pwrseq_generic_on(struct device_node *np, struct pwrseq *pwrseq)
+{
+	struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
+	int clk, ret = 0;
+	struct gpio_desc *gpiod_reset = pwrseq_gen->gpiod_reset;
+
+	for (clk = 0; clk < PWRSEQ_MAX_CLKS && pwrseq_gen->clks[clk]; clk++) {
+		ret = clk_prepare_enable(pwrseq_gen->clks[clk]);
+		if (ret) {
+			pr_err("Can't enable clock on %s: %d\n",
+				np->full_name, ret);
+			goto err_disable_clks;
+		}
+	}
+
+	if (gpiod_reset) {
+		u32 duration_us = 50;
+
+		of_property_read_u32(np, "reset-duration-us",
+				&duration_us);
+		if (duration_us <= 10)
+			udelay(10);
+		else
+			usleep_range(duration_us, duration_us + 100);
+		gpiod_set_value(gpiod_reset, 0);
+	}
+
+	return ret;
+
+err_disable_clks:
+	while (--clk >= 0)
+		clk_disable_unprepare(pwrseq_gen->clks[clk]);
+
+	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, clk, ret = 0;
+
+	for (clk = 0; clk < PWRSEQ_MAX_CLKS; clk++) {
+		pwrseq_gen->clks[clk] = of_clk_get(np, clk);
+		if (IS_ERR(pwrseq_gen->clks[clk])) {
+			ret = PTR_ERR(pwrseq_gen->clks[clk]);
+			if (ret == -EPROBE_DEFER)
+				goto err_put_clks;
+			pwrseq_gen->clks[clk] = NULL;
+			break;
+		}
+	}
+
+	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_put_clks;
+
+		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_put_clks;
+	}
+
+	return ret;
+
+err_put_clks:
+	while (--clk >= 0)
+		clk_put(pwrseq_gen->clks[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;
+
+	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..ebb2280
--- /dev/null
+++ b/include/linux/power/pwrseq.h
@@ -0,0 +1,47 @@
+#ifndef __LINUX_PWRSEQ_H
+#define __LINUX_PWRSEQ_H
+
+#include <linux/of.h>
+
+#define PWRSEQ_MAX_CLKS		3
+
+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);
+#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) {}
+#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 v5 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>
Acked-by: Rob Herring <robh@kernel.org>
---
 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@1 {
+	genesys: hub@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 v5 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>
Tested-by Joshua Clayton <stillcompiling@gmail.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 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(&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 v5 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>
Tested-by Joshua Clayton <stillcompiling@gmail.com>
---
 drivers/usb/chipidea/core.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 69426e6..b189dc7 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -954,6 +954,15 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 		dev_err(dev, "unable to init phy: %d\n", ret);
 		return ret;
 	}
+	/*
+	 * 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->hw_bank.phys = res->start;
 
@@ -1057,6 +1066,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 stop:
 	ci_role_destroy(ci);
 deinit_phy:
+	dev->of_node = NULL;
 	ci_usb_phy_exit(ci);
 
 	return ret;
@@ -1076,6 +1086,7 @@ static int ci_hdrc_remove(struct platform_device *pdev)
 	ci_extcon_unregister(ci);
 	ci_role_destroy(ci);
 	ci_hdrc_enter_lpm(ci, true);
+	ci->dev->of_node = NULL;
 	ci_usb_phy_exit(ci);
 
 	return 0;
-- 
1.9.1

[PATCH v5 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@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@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@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

Re: [PATCH v5 2/6] power: add power sequence library

[Matthias Kaehlcke]

El Mon, Aug 08, 2016 at 04:52:07PM +0800 Peter Chen ha dit:

> 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>
> Tested-by Joshua Clayton <stillcompiling@gmail.com>

Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
Tested-by: Matthias Kaehlcke <mka@chromium.org>

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

[Joshua Clayton]

Hi Peter,

On 08/08/2016 01:52 AM, Peter Chen wrote:
> 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@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@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>;
Assuming they are needed,
#address-cells and #size-cells should go in imx6qdl.dtsi,
rather than in board dts files, shouldn't they?
> +	usb2415: hub@1 {
> +		compatible = "usb424,2514";
> +		reg = <1>;
Does <reg>  have any effect?
I couldn't find any reference to it in the patches.
(so apologies if it is in core code)
Does it matter? Would it be possible to connect
more than one hub to the same usb phy?
> +
> +		clocks = <&clks IMX6QDL_CLK_CKO>;
> +		reset-gpios = <&gpio7 12 GPIO_ACTIVE_LOW>;
> +		reset-duration-us = <3000>;
> +	};
>  };
>  
>  &usdhc3 {

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

[Stephen Boyd]

Quoting Peter Chen (2016-08-08 01:52:10)
> 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>
> Tested-by Joshua Clayton <stillcompiling@gmail.com>
> ---
>  drivers/usb/chipidea/core.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 69426e6..b189dc7 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -954,6 +954,15 @@ static int ci_hdrc_probe(struct platform_device *pdev)
>                 dev_err(dev, "unable to init phy: %d\n", ret);
>                 return ret;
>         }
> +       /*
> +        * 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;

Can this be done earlier? Perhaps after hw_device_init() in this probe
routine? That would allow me to remove the awkward parent searching in
my ULPI DT awareness patch.

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

[Peter Chen]

On Tue, Aug 09, 2016 at 04:33:35PM -0700, Joshua Clayton wrote:
> Hi Peter,
> 
> On 08/08/2016 01:52 AM, Peter Chen wrote:
> > 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@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@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>;
> Assuming they are needed,
> #address-cells and #size-cells should go in imx6qdl.dtsi,
> rather than in board dts files, shouldn't they?

Yes, you are right. All imx USB controller has only one port.


> > +	usb2415: hub@1 {
> > +		compatible = "usb424,2514";
> > +		reg = <1>;
> Does <reg>  have any effect?
> I couldn't find any reference to it in the patches.
> (so apologies if it is in core code)
> Does it matter?

Please see 69bec7259853 ("USB: core: let USB device know device node")
for detail.

> Would it be possible to connect
> more than one hub to the same usb phy?

No possible for current imx, but possible for other SoCs.

-- 

Best Regards,
Peter Chen

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

[Peter Chen]

On Tue, Aug 09, 2016 at 05:15:36PM -0700, Stephen Boyd wrote:
> Quoting Peter Chen (2016-08-08 01:52:10)
> > 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>
> > Tested-by Joshua Clayton <stillcompiling@gmail.com>
> > ---
> >  drivers/usb/chipidea/core.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> > index 69426e6..b189dc7 100644
> > --- a/drivers/usb/chipidea/core.c
> > +++ b/drivers/usb/chipidea/core.c
> > @@ -954,6 +954,15 @@ static int ci_hdrc_probe(struct platform_device *pdev)
> >                 dev_err(dev, "unable to init phy: %d\n", ret);
> >                 return ret;
> >         }
> > +       /*
> > +        * 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;
> 
> Can this be done earlier? Perhaps after hw_device_init() in this probe
> routine? That would allow me to remove the awkward parent searching in
> my ULPI DT awareness patch.

The reason why I locate it there is to avoid "goto label" for error
path during PHY's get and initialization operation.

Ok, to simplify your work, I will change it at next version.

-- 

Best Regards,
Peter Chen

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

[Jun Li]

Hi, Peter

> -----Original Message-----
> From: linux-usb-owner@vger.kernel.org [mailto:linux-usb-
> owner@vger.kernel.org] On Behalf Of Peter Chen
> Sent: Wednesday, August 10, 2016 11:12 AM
> To: Stephen Boyd <stephen.boyd@linaro.org>
> Cc: Peter Chen <peter.chen@nxp.com>; gregkh@linuxfoundation.org;
> stern@rowland.harvard.edu; ulf.hansson@linaro.org; broonie@kernel.org;
> sre@kernel.org; robh+dt@kernel.org; shawnguo@kernel.org;
> dbaryshkov@gmail.com; dwmw3@infradead.org; k.kozlowski@samsung.com; linux-
> arm-kernel@lists.infradead.org; p.zabel@pengutronix.de;
> devicetree@vger.kernel.org; pawel.moll@arm.com; mark.rutland@arm.com;
> linux-usb@vger.kernel.org; arnd@arndb.de; s.hauer@pengutronix.de;
> mail@maciej.szmigiero.name; troy.kisky@boundarydevices.com;
> festevam@gmail.com; oscar@naiandei.net; linux-pm@vger.kernel.org;
> stillcompiling@gmail.com; linux-kernel@vger.kernel.org; mka@chromium.org;
> Peter Chen <peter.chen@freescale.com>
> Subject: Re: [PATCH v5 5/6] usb: chipidea: let chipidea core device
> of_node equal's glue layer device of_node
> 
> On Tue, Aug 09, 2016 at 05:15:36PM -0700, Stephen Boyd wrote:
> > Quoting Peter Chen (2016-08-08 01:52:10)
> > > 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>
> > > Tested-by Joshua Clayton <stillcompiling@gmail.com>
> > > ---
> > >  drivers/usb/chipidea/core.c | 11 +++++++++++
> > >  1 file changed, 11 insertions(+)
> > >
> > > diff --git a/drivers/usb/chipidea/core.c
> > > b/drivers/usb/chipidea/core.c index 69426e6..b189dc7 100644
> > > --- a/drivers/usb/chipidea/core.c
> > > +++ b/drivers/usb/chipidea/core.c
> > > @@ -954,6 +954,15 @@ static int ci_hdrc_probe(struct platform_device
> *pdev)
> > >                 dev_err(dev, "unable to init phy: %d\n", ret);
> > >                 return ret;
> > >         }
> > > +       /*
> > > +        * 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;
> >
> > Can this be done earlier? Perhaps after hw_device_init() in this probe
> > routine? That would allow me to remove the awkward parent searching in
> > my ULPI DT awareness patch.
> 
> The reason why I locate it there is to avoid "goto label" for error path
> during PHY's get and initialization operation.
> 
> Ok, to simplify your work, I will change it at next version.

As iommu need this to be done even more earlier, will you consider to
put it before adding the core's platform device?

Li Jun 
> 
> --
> 
> Best Regards,
> Peter Chen
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html

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

[Peter Chen]

On Wed, Aug 10, 2016 at 09:00:15AM +0000, Jun Li wrote:
> Hi, Peter
> 
> > -----Original Message-----
> > From: linux-usb-owner@vger.kernel.org [mailto:linux-usb-
> > owner@vger.kernel.org] On Behalf Of Peter Chen
> > Sent: Wednesday, August 10, 2016 11:12 AM
> > To: Stephen Boyd <stephen.boyd@linaro.org>
> > Cc: Peter Chen <peter.chen@nxp.com>; gregkh@linuxfoundation.org;
> > stern@rowland.harvard.edu; ulf.hansson@linaro.org; broonie@kernel.org;
> > sre@kernel.org; robh+dt@kernel.org; shawnguo@kernel.org;
> > dbaryshkov@gmail.com; dwmw3@infradead.org; k.kozlowski@samsung.com; linux-
> > arm-kernel@lists.infradead.org; p.zabel@pengutronix.de;
> > devicetree@vger.kernel.org; pawel.moll@arm.com; mark.rutland@arm.com;
> > linux-usb@vger.kernel.org; arnd@arndb.de; s.hauer@pengutronix.de;
> > mail@maciej.szmigiero.name; troy.kisky@boundarydevices.com;
> > festevam@gmail.com; oscar@naiandei.net; linux-pm@vger.kernel.org;
> > stillcompiling@gmail.com; linux-kernel@vger.kernel.org; mka@chromium.org;
> > Peter Chen <peter.chen@freescale.com>
> > Subject: Re: [PATCH v5 5/6] usb: chipidea: let chipidea core device
> > of_node equal's glue layer device of_node
> > 
> > On Tue, Aug 09, 2016 at 05:15:36PM -0700, Stephen Boyd wrote:
> > > Quoting Peter Chen (2016-08-08 01:52:10)
> > > > 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>
> > > > Tested-by Joshua Clayton <stillcompiling@gmail.com>
> > > > ---
> > > >  drivers/usb/chipidea/core.c | 11 +++++++++++
> > > >  1 file changed, 11 insertions(+)
> > > >
> > > > diff --git a/drivers/usb/chipidea/core.c
> > > > b/drivers/usb/chipidea/core.c index 69426e6..b189dc7 100644
> > > > --- a/drivers/usb/chipidea/core.c
> > > > +++ b/drivers/usb/chipidea/core.c
> > > > @@ -954,6 +954,15 @@ static int ci_hdrc_probe(struct platform_device
> > *pdev)
> > > >                 dev_err(dev, "unable to init phy: %d\n", ret);
> > > >                 return ret;
> > > >         }
> > > > +       /*
> > > > +        * 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;
> > >
> > > Can this be done earlier? Perhaps after hw_device_init() in this probe
> > > routine? That would allow me to remove the awkward parent searching in
> > > my ULPI DT awareness patch.
> > 
> > The reason why I locate it there is to avoid "goto label" for error path
> > during PHY's get and initialization operation.
> > 
> > Ok, to simplify your work, I will change it at next version.
> 
> As iommu need this to be done even more earlier, will you consider to
> put it before adding the core's platform device?
> 

I can't do it, it will introduce another issue that the pinctrl has
already requested by parents. The warning is similar like below, but
occurs at the probe.

http://lkml.iu.edu/hypermail/linux/kernel/1607.2/03150.html

I will see how iommu support can be added for platform devices which
are created by runtime (like chipidea core device).

-- 

Best Regards,
Peter Chen

[PATCHES] ARM: dts: update udoo and evi hubs to use pwrseq

[Joshua Clayton]

Hi Peter,

Consider this my attempt to squeeze my own dts into your patch series.
These patches repalce patch 6 in V5. of the usb pwrseq series
I adding a #address and #size to imx6qdl.dtsi in patch 1
and remove those lines from imx6qdl-udoo.dtsi in patch 2 (formerly patch 6)
Finally I make the change in imx6q-evi.dts and hope is can be slipped into
the series with the others.

Thanks, Joshua

[PATCH 1/3] ARM: dts: imx6qdl: Enable usb node children with <reg>

[Joshua Clayton]

Give usb nodes #address and #size attributes, so that a child node
representing a permanently connected device such as an onboard hub may
be addressed with a <reg> attribute

Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
---
 arch/arm/boot/dts/imx6qdl.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index ed613eb..9b1ec83 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -934,6 +934,8 @@
 
 			usbh1: usb@02184200 {
 				compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
+				#address-cells = <1>;
+				#size-cells = <0>;
 				reg = <0x02184200 0x200>;
 				interrupts = <0 40 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX6QDL_CLK_USBOH3>;
@@ -948,6 +950,8 @@
 
 			usbh2: usb@02184400 {
 				compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
+				#address-cells = <1>;
+				#size-cells = <0>;
 				reg = <0x02184400 0x200>;
 				interrupts = <0 41 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX6QDL_CLK_USBOH3>;
@@ -961,6 +965,8 @@
 
 			usbh3: usb@02184600 {
 				compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
+				#address-cells = <1>;
+				#size-cells = <0>;
 				reg = <0x02184600 0x200>;
 				interrupts = <0 42 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX6QDL_CLK_USBOH3>;
-- 
2.7.4

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

[Joshua Clayton]

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

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>
Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
---
 arch/arm/boot/dts/imx6qdl-udoo.dtsi | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/arch/arm/boot/dts/imx6qdl-udoo.dtsi b/arch/arm/boot/dts/imx6qdl-udoo.dtsi
index 3bee2f9..44b2a24 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@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@1 {
 			compatible = "regulator-fixed";
 			reg = <1>;
@@ -259,9 +250,16 @@
 &usbh1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_usbh>;
-	vbus-supply = <&reg_usb_h1_vbus>;
-	clocks = <&clks IMX6QDL_CLK_CKO>;
 	status = "okay";
+
+	usb2415: hub@1 {
+		compatible = "usb424,2514";
+		reg = <1>;
+
+		clocks = <&clks IMX6QDL_CLK_CKO>;
+		reset-gpios = <&gpio7 12 GPIO_ACTIVE_LOW>;
+		reset-duration-us = <3000>;
+	};
 };
 
 &usdhc3 {
-- 
2.7.4

[PATCH 3/3] ARM: dts: imx6q-evi: Fix onboard hub reset line

[Joshua Clayton]

Previously the onboard hub was made to work by treating its
reset gpio as a regulator enable.
Get rid of that kludge now that pwseq has added reset gpio support
Move pin muxing the hub reset pin into the usbh1 group

Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
---
 arch/arm/boot/dts/imx6q-evi.dts | 25 +++++++------------------
 1 file changed, 7 insertions(+), 18 deletions(-)

diff --git a/arch/arm/boot/dts/imx6q-evi.dts b/arch/arm/boot/dts/imx6q-evi.dts
index 4fa5601..49c6f61 100644
--- a/arch/arm/boot/dts/imx6q-evi.dts
+++ b/arch/arm/boot/dts/imx6q-evi.dts
@@ -54,18 +54,6 @@
 		reg = <0x10000000 0x40000000>;
 	};
 
-	reg_usbh1_vbus: regulator-usbhubreset {
-		compatible = "regulator-fixed";
-		regulator-name = "usbh1_vbus";
-		regulator-min-microvolt = <5000000>;
-		regulator-max-microvolt = <5000000>;
-		enable-active-high;
-		startup-delay-us = <2>;
-		pinctrl-names = "default";
-		pinctrl-0 = <&pinctrl_usbh1_hubreset>;
-		gpio = <&gpio7 12 GPIO_ACTIVE_HIGH>;
-	};
-
 	reg_usb_otg_vbus: regulator-usbotgvbus {
 		compatible = "regulator-fixed";
 		regulator-name = "usb_otg_vbus";
@@ -204,12 +192,18 @@
 };
 
 &usbh1 {
-	vbus-supply = <&reg_usbh1_vbus>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_usbh1>;
 	dr_mode = "host";
 	disable-over-current;
 	status = "okay";
+
+	usb2415host: hub@1 {
+		compatible = "usb424,2513";
+		reg = <1>;
+		reset-gpios = <&gpio7 12 GPIO_ACTIVE_LOW>;
+		reset-duration-us = <3000>;
+	};
 };
 
 &usbotg {
@@ -467,11 +461,6 @@
 			MX6QDL_PAD_GPIO_3__USB_H1_OC 0x1b0b0
 			/* usbh1_b OC */
 			MX6QDL_PAD_GPIO_0__GPIO1_IO00 0x1b0b0
-		>;
-	};
-
-	pinctrl_usbh1_hubreset: usbh1hubresetgrp {
-		fsl,pins = <
 			MX6QDL_PAD_GPIO_17__GPIO7_IO12 0x1b0b0
 		>;
 	};
-- 
2.7.4

Re: [PATCHES] ARM: dts: update udoo and evi hubs to use pwrseq

[Peter Chen]

On Thu, Aug 11, 2016 at 09:40:29AM -0700, Joshua Clayton wrote:
> 
> Hi Peter,
> 
> Consider this my attempt to squeeze my own dts into your patch series.
> These patches repalce patch 6 in V5. of the usb pwrseq series
> I adding a #address and #size to imx6qdl.dtsi in patch 1
> and remove those lines from imx6qdl-udoo.dtsi in patch 2 (formerly patch 6)
> Finally I make the change in imx6q-evi.dts and hope is can be slipped into
> the series with the others.
> 

Thanks, I will send them at next revision

-- 

Best Regards,
Peter Chen

Re: [PATCH 3/3] ARM: dts: imx6q-evi: Fix onboard hub reset line

[Peter Chen]

On Thu, Aug 11, 2016 at 09:40:32AM -0700, Joshua Clayton wrote:
> Previously the onboard hub was made to work by treating its
> reset gpio as a regulator enable.
> Get rid of that kludge now that pwseq has added reset gpio support
> Move pin muxing the hub reset pin into the usbh1 group
> 
> Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
> ---
>  arch/arm/boot/dts/imx6q-evi.dts | 25 +++++++------------------
>  1 file changed, 7 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/imx6q-evi.dts b/arch/arm/boot/dts/imx6q-evi.dts
> index 4fa5601..49c6f61 100644
> --- a/arch/arm/boot/dts/imx6q-evi.dts
> +++ b/arch/arm/boot/dts/imx6q-evi.dts
> @@ -54,18 +54,6 @@
>  		reg = <0x10000000 0x40000000>;
>  	};
>  
> -	reg_usbh1_vbus: regulator-usbhubreset {
> -		compatible = "regulator-fixed";
> -		regulator-name = "usbh1_vbus";
> -		regulator-min-microvolt = <5000000>;
> -		regulator-max-microvolt = <5000000>;
> -		enable-active-high;
> -		startup-delay-us = <2>;
> -		pinctrl-names = "default";
> -		pinctrl-0 = <&pinctrl_usbh1_hubreset>;
> -		gpio = <&gpio7 12 GPIO_ACTIVE_HIGH>;
> -	};
> -
>  	reg_usb_otg_vbus: regulator-usbotgvbus {
>  		compatible = "regulator-fixed";
>  		regulator-name = "usb_otg_vbus";
> @@ -204,12 +192,18 @@
>  };
>  
>  &usbh1 {
> -	vbus-supply = <&reg_usbh1_vbus>;
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_usbh1>;
>  	dr_mode = "host";
>  	disable-over-current;
>  	status = "okay";
> +
> +	usb2415host: hub@1 {
> +		compatible = "usb424,2513";
> +		reg = <1>;
> +		reset-gpios = <&gpio7 12 GPIO_ACTIVE_LOW>;
> +		reset-duration-us = <3000>;
> +	};
>  };
>  
>  &usbotg {
> @@ -467,11 +461,6 @@
>  			MX6QDL_PAD_GPIO_3__USB_H1_OC 0x1b0b0
>  			/* usbh1_b OC */

This comment should be for above pin, do you mind I change it in
this patch?

>  			MX6QDL_PAD_GPIO_0__GPIO1_IO00 0x1b0b0
> -		>;
> -	};
> -
> -	pinctrl_usbh1_hubreset: usbh1hubresetgrp {
> -		fsl,pins = <
>  			MX6QDL_PAD_GPIO_17__GPIO7_IO12 0x1b0b0
>  		>;
>  	};

This changes remind me that I need to change the same thing for udoo
board.

-- 

Best Regards,
Peter Chen

Re: [PATCH 3/3] ARM: dts: imx6q-evi: Fix onboard hub reset line

[Joshua Clayton]

Hi Peter,

On 08/11/2016 08:11 PM, Peter Chen wrote:
> On Thu, Aug 11, 2016 at 09:40:32AM -0700, Joshua Clayton wrote:
>> Previously the onboard hub was made to work by treating its
>> reset gpio as a regulator enable.
>> Get rid of that kludge now that pwseq has added reset gpio support
>> Move pin muxing the hub reset pin into the usbh1 group
>>
>> Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
>> ---
>>  arch/arm/boot/dts/imx6q-evi.dts | 25 +++++++------------------
>>  1 file changed, 7 insertions(+), 18 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/imx6q-evi.dts b/arch/arm/boot/dts/imx6q-evi.dts
>> index 4fa5601..49c6f61 100644
>> --- a/arch/arm/boot/dts/imx6q-evi.dts
>> +++ b/arch/arm/boot/dts/imx6q-evi.dts
>> @@ -54,18 +54,6 @@
>>  		reg = <0x10000000 0x40000000>;
>>  	};
>>  
>> -	reg_usbh1_vbus: regulator-usbhubreset {
>> -		compatible = "regulator-fixed";
>> -		regulator-name = "usbh1_vbus";
>> -		regulator-min-microvolt = <5000000>;
>> -		regulator-max-microvolt = <5000000>;
>> -		enable-active-high;
>> -		startup-delay-us = <2>;
>> -		pinctrl-names = "default";
>> -		pinctrl-0 = <&pinctrl_usbh1_hubreset>;
>> -		gpio = <&gpio7 12 GPIO_ACTIVE_HIGH>;
>> -	};
>> -
>>  	reg_usb_otg_vbus: regulator-usbotgvbus {
>>  		compatible = "regulator-fixed";
>>  		regulator-name = "usb_otg_vbus";
>> @@ -204,12 +192,18 @@
>>  };
>>  
>>  &usbh1 {
>> -	vbus-supply = <&reg_usbh1_vbus>;
>>  	pinctrl-names = "default";
>>  	pinctrl-0 = <&pinctrl_usbh1>;
>>  	dr_mode = "host";
>>  	disable-over-current;
>>  	status = "okay";
>> +
>> +	usb2415host: hub@1 {
>> +		compatible = "usb424,2513";
>> +		reg = <1>;
>> +		reset-gpios = <&gpio7 12 GPIO_ACTIVE_LOW>;
>> +		reset-duration-us = <3000>;
>> +	};
>>  };
>>  
>>  &usbotg {
>> @@ -467,11 +461,6 @@
>>  			MX6QDL_PAD_GPIO_3__USB_H1_OC 0x1b0b0
>>  			/* usbh1_b OC */
> This comment should be for above pin, do you mind I change it in
> this patch?
Better leave it alone

I checked te schematic. GPIO_0 (below that line) is indeed
connected to a second OC detect line for a second
usb port... It can't possibly be working as designed, but
that is how it is connected up.

I guess I need to consult with my EE colleagues
>
>>  			MX6QDL_PAD_GPIO_0__GPIO1_IO00 0x1b0b0
>> -		>;
>> -	};
>> -
>> -	pinctrl_usbh1_hubreset: usbh1hubresetgrp {
>> -		fsl,pins = <
>>  			MX6QDL_PAD_GPIO_17__GPIO7_IO12 0x1b0b0
>>  		>;
>>  	};
> This changes remind me that I need to change the same thing for udoo
> board.
>
Sorry I didn't think of it first.