[PATCH v7 0/8] power: add power sequence library

[PATCH v7 0/8] 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]. The kinds of
power sequence instances will be added at postcore_initcall, the match
criteria is compatible string first, if the compatible string is not
matched between dts and library, it will try to use generic power sequence.
	 
The host driver just needs to call of_pwrseq_on/of_pwrseq_off
if only one power sequence instance is needed, for more power sequences
are used, using of_pwrseq_on_list/of_pwrseq_off_list instead (eg, USB hub driver).

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 v7:
- Create kinds of power sequence instance at postcore_initcall, and match
  the instance with node using compatible string, the beneit of this is
  the host driver doesn't need to consider which pwrseq instance needs
  to be used, and pwrseq core will match it, however, it eats some memories
  if less power sequence instances are used. [Patch 2/8]
- Add pwrseq_compatible_sample.c to test match pwrseq using device_id. [Patch 2/8]
- Fix the comments Vaibhav Hiremath adds for error path for clock and do not
  use device_node for parameters at pwrseq_on. [Patch 2/8]
- Simplify the caller to use power sequence, follows Alan's commnets [Patch 4/8]
- Tested three pwrseq instances together using both specific compatible string and
  generic libraries.

Changes for v6:
- Add Matthias Kaehlcke's Reviewed-by and Tested-by. (patch [2/6])
- Change chipidea core of_node assignment for coming user. (patch [5/6])
- Applies Joshua Clayton's three dts changes for two boards,
  the USB device's reg has only #address-cells, but without #size-cells.

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

Joshua Clayton (2):
  ARM: dts: imx6qdl: Enable usb node children with <reg>
  ARM: dts: imx6q-evi: Fix onboard hub reset line

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/imx6q-evi.dts                    |  25 +--
 arch/arm/boot/dts/imx6qdl-udoo.dtsi                |  26 ++-
 arch/arm/boot/dts/imx6qdl.dtsi                     |   6 +
 drivers/power/Kconfig                              |   1 +
 drivers/power/Makefile                             |   1 +
 drivers/power/pwrseq/Kconfig                       |  45 +++++
 drivers/power/pwrseq/Makefile                      |   3 +
 drivers/power/pwrseq/core.c                        | 190 +++++++++++++++++++++
 drivers/power/pwrseq/pwrseq_compatible_sample.c    | 178 +++++++++++++++++++
 drivers/power/pwrseq/pwrseq_generic.c              | 177 +++++++++++++++++++
 drivers/usb/chipidea/core.c                        |  27 ++-
 drivers/usb/core/hub.c                             |  41 ++++-
 drivers/usb/core/hub.h                             |   1 +
 include/linux/power/pwrseq.h                       |  73 ++++++++
 17 files changed, 820 insertions(+), 41 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_compatible_sample.c
 create mode 100644 drivers/power/pwrseq/pwrseq_generic.c
 create mode 100644 include/linux/power/pwrseq.h

-- 
2.7.4

[PATCH v7 1/8] 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>;
+		};
+	};
+};
-- 
2.7.4

[PATCH v7 2/8] 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 just needs to call of_pwrseq_on/of_pwrseq_off
if only one power sequence is needed, else call of_pwrseq_on_list
/of_pwrseq_off_list instead (eg, USB hub driver).

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>
---
 MAINTAINERS                                     |   9 ++
 drivers/power/Kconfig                           |   1 +
 drivers/power/Makefile                          |   1 +
 drivers/power/pwrseq/Kconfig                    |  45 ++++++
 drivers/power/pwrseq/Makefile                   |   3 +
 drivers/power/pwrseq/core.c                     | 190 ++++++++++++++++++++++++
 drivers/power/pwrseq/pwrseq_compatible_sample.c | 178 ++++++++++++++++++++++
 drivers/power/pwrseq/pwrseq_generic.c           | 177 ++++++++++++++++++++++
 include/linux/power/pwrseq.h                    |  73 +++++++++
 9 files changed, 677 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_compatible_sample.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 b3e9395..b353769 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9343,6 +9343,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..dff5e35
--- /dev/null
+++ b/drivers/power/pwrseq/Kconfig
@@ -0,0 +1,45 @@
+#
+# 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.
+
+config PWRSEQ_GENERIC_INSTANCE_NUMBER
+	int "Number of Generic Power Sequence Instance"
+	depends on PWRSEQ_GENERIC
+	range 1 10
+	default 2
+	help
+	   Usually, there are not so many devices needs power sequence, we set two
+	   as default value.
+
+config PWRSEQ_SAMPLE
+	bool "sample power sequence control using compatible string"
+	depends on OF
+	select POWER_SEQUENCE
+	help
+	   It is a sample library which implements power sequence for device id,
+	   it is an example purpose.
+
+config PWRSEQ_SAMPLE_INSTANCE_NUMBER
+	int "Number of Sample Power Sequence Instance"
+	depends on PWRSEQ_SAMPLE
+	range 1 5
+	default 1
+	help
+	   Usually, this file is special for certain device, so the default for this number
+	   is 1.
+endmenu
diff --git a/drivers/power/pwrseq/Makefile b/drivers/power/pwrseq/Makefile
new file mode 100644
index 0000000..62f3cbf
--- /dev/null
+++ b/drivers/power/pwrseq/Makefile
@@ -0,0 +1,3 @@
+obj-$(CONFIG_POWER_SEQUENCE) += core.o
+obj-$(CONFIG_PWRSEQ_GENERIC) += pwrseq_generic.o
+obj-$(CONFIG_PWRSEQ_SAMPLE) += pwrseq_compatible_sample.o
diff --git a/drivers/power/pwrseq/core.c b/drivers/power/pwrseq/core.c
new file mode 100644
index 0000000..6beae20
--- /dev/null
+++ b/drivers/power/pwrseq/core.c
@@ -0,0 +1,190 @@
+/*
+ * 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/list.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/slab.h>
+#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_GPL(pwrseq_get);
+
+int pwrseq_on(struct pwrseq *p)
+{
+	if (p && p->on)
+		return p->on(p);
+
+	return -ENOTSUPP;
+}
+EXPORT_SYMBOL_GPL(pwrseq_on);
+
+void pwrseq_off(struct pwrseq *p)
+{
+	if (p && p->off)
+		p->off(p);
+}
+EXPORT_SYMBOL_GPL(pwrseq_off);
+
+void pwrseq_put(struct pwrseq *p)
+{
+	if (p && p->put)
+		p->put(p);
+}
+EXPORT_SYMBOL_GPL(pwrseq_put);
+
+void pwrseq_free(struct pwrseq *p)
+{
+	if (p && p->free)
+		p->free(p);
+}
+EXPORT_SYMBOL_GPL(pwrseq_free);
+
+int pwrseq_suspend(struct pwrseq *p)
+{
+	if (p && p->suspend)
+		return p->suspend(p);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pwrseq_suspend);
+
+int pwrseq_resume(struct pwrseq *p)
+{
+	if (p && p->resume)
+		return p->resume(p);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pwrseq_resume);
+
+void pwrseq_register(struct pwrseq *pwrseq)
+{
+	mutex_lock(&pwrseq_list_mutex);
+	list_add(&pwrseq->node, &pwrseq_list);
+	mutex_unlock(&pwrseq_list_mutex);
+}
+EXPORT_SYMBOL_GPL(pwrseq_register);
+
+static struct pwrseq *pwrseq_find_available_instance(struct device_node *np)
+{
+	struct pwrseq *pwrseq;
+
+	list_for_each_entry(pwrseq, &pwrseq_list, node) {
+		if (pwrseq->used)
+			continue;
+
+		/* compare compatible string for pwrseq node */
+		if (of_match_node(pwrseq->pwrseq_of_match_table, np)) {
+			pwrseq->used = true;
+			return pwrseq;
+		}
+
+		/* return generic pwrseq instance */
+		if (!strcmp(pwrseq->pwrseq_of_match_table->compatible,
+				"generic")) {
+			pr_debug("using generic pwrseq instance for %s\n",
+				np->full_name);
+			pwrseq->used = true;
+			return pwrseq;
+		}
+	}
+	pr_warn("Can't find any pwrseq instances for %s\n", np->full_name);
+
+	return NULL;
+}
+
+struct pwrseq *of_pwrseq_on(struct device_node *np)
+{
+	struct pwrseq *pwrseq;
+	int ret;
+
+	pwrseq = pwrseq_find_available_instance(np);
+	if (!pwrseq)
+		return ERR_PTR(-ENONET);
+
+	ret = pwrseq_get(np, pwrseq);
+	if (ret)
+		goto pwr_free;
+
+	ret = pwrseq_on(pwrseq);
+	if (ret)
+		goto pwr_put;
+
+	return pwrseq;
+
+pwr_put:
+	pwrseq_put(pwrseq);
+pwr_free:
+	pwrseq_free(pwrseq);
+	return ERR_PTR(ret);
+}
+EXPORT_SYMBOL_GPL(of_pwrseq_on);
+
+void of_pwrseq_off(struct pwrseq *pwrseq)
+{
+	pwrseq_off(pwrseq);
+	pwrseq_put(pwrseq);
+	pwrseq_free(pwrseq);
+}
+EXPORT_SYMBOL_GPL(of_pwrseq_off);
+
+int of_pwrseq_on_list(struct device_node *np, struct list_head *head)
+{
+	struct pwrseq *pwrseq;
+	struct pwrseq_list_per_dev *pwrseq_list_node;
+
+	pwrseq = of_pwrseq_on(np);
+	if (IS_ERR(pwrseq))
+		return PTR_ERR(pwrseq);
+
+	pwrseq_list_node = kzalloc(sizeof(*pwrseq_list_node), GFP_KERNEL);
+	if (!pwrseq_list_node) {
+		of_pwrseq_off(pwrseq);
+		return -ENOMEM;
+	}
+	pwrseq_list_node->pwrseq = pwrseq;
+	list_add(&pwrseq_list_node->list, head);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_pwrseq_on_list);
+
+void of_pwrseq_off_list(struct list_head *head)
+{
+	struct pwrseq *pwrseq;
+	struct pwrseq_list_per_dev *pwrseq_list_node, *tmp_node;
+
+	list_for_each_entry_safe(pwrseq_list_node, tmp_node, head, list) {
+		pwrseq = pwrseq_list_node->pwrseq;
+		of_pwrseq_off(pwrseq);
+		list_del(&pwrseq_list_node->list);
+		kfree(pwrseq_list_node);
+	}
+}
+EXPORT_SYMBOL_GPL(of_pwrseq_off_list);
diff --git a/drivers/power/pwrseq/pwrseq_compatible_sample.c b/drivers/power/pwrseq/pwrseq_compatible_sample.c
new file mode 100644
index 0000000..d6bcc6d
--- /dev/null
+++ b/drivers/power/pwrseq/pwrseq_compatible_sample.c
@@ -0,0 +1,178 @@
+/*
+ * pwrseq_compatible_sample.c	Sample power sequence handling for compatible
+ *
+ * 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_sample {
+	struct pwrseq pwrseq;
+	struct gpio_desc *gpiod_reset;
+	struct clk *clks[PWRSEQ_MAX_CLKS];
+	u32 duration_us;
+};
+
+#define to_generic_pwrseq(p) container_of(p, struct pwrseq_sample, pwrseq)
+
+static void pwrseq_sample_free(struct pwrseq *pwrseq)
+{
+	pwrseq->used = false;
+}
+
+static void pwrseq_sample_put(struct pwrseq *pwrseq)
+{
+	struct pwrseq_sample *pwrseq_sam = to_generic_pwrseq(pwrseq);
+	int clk;
+
+	if (pwrseq_sam->gpiod_reset)
+		gpiod_put(pwrseq_sam->gpiod_reset);
+
+	for (clk = 0; clk < PWRSEQ_MAX_CLKS; clk++)
+		clk_put(pwrseq_sam->clks[clk]);
+}
+
+static void pwrseq_sample_off(struct pwrseq *pwrseq)
+{
+	struct pwrseq_sample *pwrseq_sam = to_generic_pwrseq(pwrseq);
+	int clk;
+
+	for (clk = PWRSEQ_MAX_CLKS - 1; clk >= 0; clk--)
+		clk_disable_unprepare(pwrseq_sam->clks[clk]);
+}
+
+static int pwrseq_sample_on(struct pwrseq *pwrseq)
+{
+	struct pwrseq_sample *pwrseq_sam = to_generic_pwrseq(pwrseq);
+	int clk, ret = 0;
+	struct gpio_desc *gpiod_reset = pwrseq_sam->gpiod_reset;
+
+	for (clk = 0; clk < PWRSEQ_MAX_CLKS && pwrseq_sam->clks[clk]; clk++) {
+		ret = clk_prepare_enable(pwrseq_sam->clks[clk]);
+		if (ret) {
+			pr_err("Can't enable clock, ret=%d\n", ret);
+			goto err_disable_clks;
+		}
+	}
+
+	if (gpiod_reset) {
+		u32 duration_us = pwrseq_sam->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_sam->clks[clk]);
+
+	return ret;
+}
+
+static int pwrseq_sample_get(struct device_node *np, struct pwrseq *pwrseq)
+{
+	struct pwrseq_sample *pwrseq_sam = to_generic_pwrseq(pwrseq);
+	enum of_gpio_flags flags;
+	int reset_gpio, clk, ret = 0;
+
+	for (clk = 0; clk < PWRSEQ_MAX_CLKS; clk++) {
+		pwrseq_sam->clks[clk] = of_clk_get(np, clk);
+		if (IS_ERR(pwrseq_sam->clks[clk])) {
+			ret = PTR_ERR(pwrseq_sam->clks[clk]);
+			if (ret != -ENOENT)
+				goto err_put_clks;
+			pwrseq_sam->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_sam->gpiod_reset = gpio_to_desc(reset_gpio);
+		of_property_read_u32(np, "reset-duration-us",
+				&pwrseq_sam->duration_us);
+	} 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_sam->clks[clk]);
+	return ret;
+}
+
+static const struct of_device_id sample_id_table[] = {
+	{ .compatible = "usb5e3,608",},
+	{ .compatible = "usbb95,1708",},
+	{ /* sentinel */ }
+};
+
+static int __init pwrseq_compatible_sample_register(void)
+{
+	struct pwrseq_sample *pwrseq_sam;
+	int i;
+
+	for (i = 0; i < CONFIG_PWRSEQ_SAMPLE_INSTANCE_NUMBER; i++) {
+		pwrseq_sam = kzalloc(sizeof(*pwrseq_sam), GFP_KERNEL);
+		if (!pwrseq_sam)
+			return -ENOMEM;
+
+		pwrseq_sam->pwrseq.pwrseq_of_match_table = sample_id_table;
+		pwrseq_sam->pwrseq.get = pwrseq_sample_get;
+		pwrseq_sam->pwrseq.on = pwrseq_sample_on;
+		pwrseq_sam->pwrseq.off = pwrseq_sample_off;
+		pwrseq_sam->pwrseq.put = pwrseq_sample_put;
+		pwrseq_sam->pwrseq.free = pwrseq_sample_free;
+
+		pwrseq_register(&pwrseq_sam->pwrseq);
+	}
+
+	return 0;
+}
+postcore_initcall(pwrseq_compatible_sample_register)
diff --git a/drivers/power/pwrseq/pwrseq_generic.c b/drivers/power/pwrseq/pwrseq_generic.c
new file mode 100644
index 0000000..bcd16c3
--- /dev/null
+++ b/drivers/power/pwrseq/pwrseq_generic.c
@@ -0,0 +1,177 @@
+/*
+ * 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];
+	u32 duration_us;
+};
+
+#define to_generic_pwrseq(p) container_of(p, struct pwrseq_generic, pwrseq)
+
+static void pwrseq_generic_free(struct pwrseq *pwrseq)
+{
+	pwrseq->used = false;
+}
+
+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 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, ret=%d\n", ret);
+			goto err_disable_clks;
+		}
+	}
+
+	if (gpiod_reset) {
+		u32 duration_us = pwrseq_gen->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 != -ENOENT)
+				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);
+		of_property_read_u32(np, "reset-duration-us",
+				&pwrseq_gen->duration_us);
+	} 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;
+}
+
+static const struct of_device_id generic_id_table[] = {
+	{ .compatible = "generic",},
+	{ /* sentinel */ }
+};
+
+static int __init pwrseq_generic_register(void)
+{
+	struct pwrseq_generic *pwrseq_gen;
+	int i;
+
+	for (i = 0; i < CONFIG_PWRSEQ_GENERIC_INSTANCE_NUMBER; i++) {
+		pwrseq_gen = kzalloc(sizeof(*pwrseq_gen), GFP_KERNEL);
+		if (!pwrseq_gen)
+			return -ENOMEM;
+
+		pwrseq_gen->pwrseq.pwrseq_of_match_table = generic_id_table;
+		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 0;
+}
+postcore_initcall(pwrseq_generic_register)
diff --git a/include/linux/power/pwrseq.h b/include/linux/power/pwrseq.h
new file mode 100644
index 0000000..ae4753f
--- /dev/null
+++ b/include/linux/power/pwrseq.h
@@ -0,0 +1,73 @@
+#ifndef __LINUX_PWRSEQ_H
+#define __LINUX_PWRSEQ_H
+
+#include <linux/of.h>
+
+#define PWRSEQ_MAX_CLKS		3
+
+struct pwrseq {
+	const struct of_device_id *pwrseq_of_match_table;
+	struct list_head node;
+	int (*get)(struct device_node *np, struct pwrseq *p);
+	int (*on)(struct pwrseq *p);
+	void (*off)(struct pwrseq *p);
+	void (*put)(struct pwrseq *p);
+	void (*free)(struct pwrseq *p);
+	int (*suspend)(struct pwrseq *p);
+	int (*resume)(struct pwrseq *p);
+	bool used;
+};
+
+/* used for power sequence instance list in one driver */
+struct pwrseq_list_per_dev {
+	struct pwrseq *pwrseq;
+	struct list_head list;
+};
+
+#if IS_ENABLED(CONFIG_POWER_SEQUENCE)
+int pwrseq_get(struct device_node *np, struct pwrseq *p);
+int pwrseq_on(struct pwrseq *p);
+void pwrseq_off(struct pwrseq *p);
+void pwrseq_put(struct pwrseq *p);
+void pwrseq_free(struct pwrseq *p);
+int pwrseq_suspend(struct pwrseq *p);
+int pwrseq_resume(struct pwrseq *p);
+void pwrseq_register(struct pwrseq *pwrseq);
+struct pwrseq *of_pwrseq_on(struct device_node *np);
+void of_pwrseq_off(struct pwrseq *pwrseq);
+int of_pwrseq_on_list(struct device_node *np, struct list_head *head);
+void of_pwrseq_off_list(struct list_head *head);
+#else
+static inline int pwrseq_get(struct device_node *np, struct pwrseq *p)
+{
+	return 0;
+}
+static inline int pwrseq_on(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 int pwrseq_suspend(struct pwrseq *p)
+{
+	return 0;
+}
+static inline int pwrseq_resume(struct pwrseq *p)
+{
+	return 0;
+}
+static inline void pwrseq_register(struct pwrseq *pwrseq) {}
+static inline struct pwrseq *of_pwrseq_on(struct device_node *np)
+{
+	return NULL;
+}
+void of_pwrseq_off(struct pwrseq *pwrseq) {}
+int of_pwrseq_on_list(struct device_node *np, struct list_head *head)
+{
+	return 0;
+}
+void of_pwrseq_off_list(struct list_head *head) {}
+#endif /* CONFIG_POWER_SEQUENCE */
+
+#endif  /* __LINUX_PWRSEQ_H */
-- 
2.7.4

[PATCH v7 3/8] 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>;
 	};
 }
-- 
2.7.4

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

[Peter Chen]

Some hard-wired USB devices need to do power sequence to let the
device work normally, the typical power sequence like: enable USB
PHY clock, toggle reset pin, etc. But current Linux USB driver
lacks of such code to do it, it may cause some hard-wired USB devices
works abnormal or can't be recognized by controller at all.

In this patch, it calls power sequence library APIs to finish
the power sequence events. 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/hub.c | 41 ++++++++++++++++++++++++++++++++++++++---
 drivers/usb/core/hub.h |  1 +
 2 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index b48dc76..f3de1de 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -26,6 +26,7 @@
 #include <linux/mutex.h>
 #include <linux/random.h>
 #include <linux/pm_qos.h>
+#include <linux/power/pwrseq.h>
 
 #include <asm/uaccess.h>
 #include <asm/byteorder.h>
@@ -1695,6 +1696,7 @@ static void hub_disconnect(struct usb_interface *intf)
 	hub->error = 0;
 	hub_quiesce(hub, HUB_DISCONNECT);
 
+	of_pwrseq_off_list(&hub->pwrseq_on_list);
 	mutex_lock(&usb_port_peer_mutex);
 
 	/* Avoid races with recursively_mark_NOTATTACHED() */
@@ -1722,12 +1724,41 @@ static void hub_disconnect(struct usb_interface *intf)
 	kref_put(&hub->kref, hub_release);
 }
 
+#ifdef CONFIG_OF
+static int hub_of_pwrseq_on(struct usb_hub *hub)
+{
+	struct device *parent;
+	struct usb_device *hdev = hub->hdev;
+	struct device_node *np;
+	int ret;
+
+	if (hdev->parent)
+		parent = &hdev->dev;
+	else
+		parent = bus_to_hcd(hdev->bus)->self.controller;
+
+	for_each_child_of_node(parent->of_node, np) {
+		ret = of_pwrseq_on_list(np, &hub->pwrseq_on_list);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+#else
+static int hub_of_pwrseq_on(struct usb_hub *hub)
+{
+	return 0;
+}
+#endif
+
 static int hub_probe(struct usb_interface *intf, const struct usb_device_id *id)
 {
 	struct usb_host_interface *desc;
 	struct usb_endpoint_descriptor *endpoint;
 	struct usb_device *hdev;
 	struct usb_hub *hub;
+	int ret = -ENODEV;
 
 	desc = intf->cur_altsetting;
 	hdev = interface_to_usbdev(intf);
@@ -1832,6 +1863,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);
 
@@ -1845,11 +1877,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_of_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..cd86f91 100644
--- a/drivers/usb/core/hub.h
+++ b/drivers/usb/core/hub.h
@@ -78,6 +78,7 @@ struct usb_hub {
 	struct delayed_work	init_work;
 	struct work_struct      events;
 	struct usb_port		**ports;
+	struct list_head	pwrseq_on_list; /* powered pwrseq node list */
 };
 
 /**
-- 
2.7.4

[PATCH v7 5/8] 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 | 27 ++++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 69426e6..6839e19 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -927,6 +927,16 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
+	/*
+	 * 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;
+
 	if (ci->platdata->phy) {
 		ci->phy = ci->platdata->phy;
 	} else if (ci->platdata->usb_phy) {
@@ -937,11 +947,15 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 
 		/* if both generic PHY and USB PHY layers aren't enabled */
 		if (PTR_ERR(ci->phy) == -ENOSYS &&
-				PTR_ERR(ci->usb_phy) == -ENXIO)
-			return -ENXIO;
+				PTR_ERR(ci->usb_phy) == -ENXIO) {
+			ret = -ENXIO;
+			goto clear_of_node;
+		}
 
-		if (IS_ERR(ci->phy) && IS_ERR(ci->usb_phy))
-			return -EPROBE_DEFER;
+		if (IS_ERR(ci->phy) && IS_ERR(ci->usb_phy)) {
+			ret = -EPROBE_DEFER;
+			goto clear_of_node;
+		}
 
 		if (IS_ERR(ci->phy))
 			ci->phy = NULL;
@@ -952,7 +966,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 	ret = ci_usb_phy_init(ci);
 	if (ret) {
 		dev_err(dev, "unable to init phy: %d\n", ret);
-		return ret;
+		goto clear_of_node;
 	}
 
 	ci->hw_bank.phys = res->start;
@@ -1058,6 +1072,8 @@ stop:
 	ci_role_destroy(ci);
 deinit_phy:
 	ci_usb_phy_exit(ci);
+clear_of_node:
+	dev->of_node = NULL;
 
 	return ret;
 }
@@ -1076,6 +1092,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;
-- 
2.7.4

[PATCH v7 6/8] ARM: dts: imx6qdl: Enable usb node children with <reg>

[Peter Chen]

From: Joshua Clayton <stillcompiling@gmail.com>

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>
Signed-off-by: Peter Chen <peter.chen@nxp.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 b13b0b2..8fc66e1 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -935,6 +935,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>;
@@ -949,6 +951,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>;
@@ -962,6 +966,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 v7 7/8] 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. Besides, using gpio pinctrl setting for USB2415's reset pin.

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

diff --git a/arch/arm/boot/dts/imx6qdl-udoo.dtsi b/arch/arm/boot/dts/imx6qdl-udoo.dtsi
index 3bee2f9..87fe31f 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>;
@@ -188,7 +179,7 @@
 
 		pinctrl_usbh: usbhgrp {
 			fsl,pins = <
-				MX6QDL_PAD_GPIO_17__GPIO7_IO12 0x80000000
+				MX6QDL_PAD_GPIO_17__GPIO7_IO12	0x1b0b0
 				MX6QDL_PAD_NANDF_CS2__CCM_CLKO2 0x130b0
 			>;
 		};
@@ -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 v7 8/8] ARM: dts: imx6q-evi: Fix onboard hub reset line

[Peter Chen]

From: Joshua Clayton <stillcompiling@gmail.com>

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>
Signed-off-by: Peter Chen <peter.chen@nxp.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: [PATCH v7 0/8] power: add power sequence library

[Peter Chen]

On Tue, Sep 20, 2016 at 11:36:39AM +0800, 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]. The kinds of
> power sequence instances will be added at postcore_initcall, the match
> criteria is compatible string first, if the compatible string is not
> matched between dts and library, it will try to use generic power sequence.
> 	 
> The host driver just needs to call of_pwrseq_on/of_pwrseq_off
> if only one power sequence instance is needed, for more power sequences
> are used, using of_pwrseq_on_list/of_pwrseq_off_list instead (eg, USB hub driver).
> 
> 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 v7:
> - Create kinds of power sequence instance at postcore_initcall, and match
>   the instance with node using compatible string, the beneit of this is
>   the host driver doesn't need to consider which pwrseq instance needs
>   to be used, and pwrseq core will match it, however, it eats some memories
>   if less power sequence instances are used. [Patch 2/8]
> - Add pwrseq_compatible_sample.c to test match pwrseq using device_id. [Patch 2/8]
> - Fix the comments Vaibhav Hiremath adds for error path for clock and do not
>   use device_node for parameters at pwrseq_on. [Patch 2/8]
> - Simplify the caller to use power sequence, follows Alan's commnets [Patch 4/8]
> - Tested three pwrseq instances together using both specific compatible string and
>   generic libraries.
> 

Hi Vaibhav, would you please test if this series can support your case,
you can add one instance like pwrseq_compatible_sample? If you are busy
now, but think this series can satisfy your requirement, please ack it.
I will delete pwrseq_compatible_sample, and only submit the generic one
at next revision, you can add it later, thanks.

Peter

> Changes for v6:
> - Add Matthias Kaehlcke's Reviewed-by and Tested-by. (patch [2/6])
> - Change chipidea core of_node assignment for coming user. (patch [5/6])
> - Applies Joshua Clayton's three dts changes for two boards,
>   the USB device's reg has only #address-cells, but without #size-cells.
> 
> 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
> 
> Joshua Clayton (2):
>   ARM: dts: imx6qdl: Enable usb node children with <reg>
>   ARM: dts: imx6q-evi: Fix onboard hub reset line
> 
> 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/imx6q-evi.dts                    |  25 +--
>  arch/arm/boot/dts/imx6qdl-udoo.dtsi                |  26 ++-
>  arch/arm/boot/dts/imx6qdl.dtsi                     |   6 +
>  drivers/power/Kconfig                              |   1 +
>  drivers/power/Makefile                             |   1 +
>  drivers/power/pwrseq/Kconfig                       |  45 +++++
>  drivers/power/pwrseq/Makefile                      |   3 +
>  drivers/power/pwrseq/core.c                        | 190 +++++++++++++++++++++
>  drivers/power/pwrseq/pwrseq_compatible_sample.c    | 178 +++++++++++++++++++
>  drivers/power/pwrseq/pwrseq_generic.c              | 177 +++++++++++++++++++
>  drivers/usb/chipidea/core.c                        |  27 ++-
>  drivers/usb/core/hub.c                             |  41 ++++-
>  drivers/usb/core/hub.h                             |   1 +
>  include/linux/power/pwrseq.h                       |  73 ++++++++
>  17 files changed, 820 insertions(+), 41 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_compatible_sample.c
>  create mode 100644 drivers/power/pwrseq/pwrseq_generic.c
>  create mode 100644 include/linux/power/pwrseq.h
> 
> -- 
> 2.7.4
> 
> --
> 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

-- 

Best Regards,
Peter Chen

Re: [PATCH v7 0/8] power: add power sequence library

[Maciej S. Szmigiero]

On 20.09.2016 05:36, 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]. The kinds of
> power sequence instances will be added at postcore_initcall, the match
> criteria is compatible string first, if the compatible string is not
> matched between dts and library, it will try to use generic power sequence.
> 	 
> The host driver just needs to call of_pwrseq_on/of_pwrseq_off
> if only one power sequence instance is needed, for more power sequences
> are used, using of_pwrseq_on_list/of_pwrseq_off_list instead (eg, USB hub driver).
> 
> 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

Just tested this patch set on UDOO board again to make sure that it still
works after all changes done since it was last tested there and can confirm
that it does work correctly.

Maciej

Re: [PATCH v7 0/8] power: add power sequence library

[Peter Chen]

On Wed, Sep 28, 2016 at 01:30:17AM +0200, Maciej S. Szmigiero wrote:
> On 20.09.2016 05:36, 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]. The kinds of
> > power sequence instances will be added at postcore_initcall, the match
> > criteria is compatible string first, if the compatible string is not
> > matched between dts and library, it will try to use generic power sequence.
> > 	 
> > The host driver just needs to call of_pwrseq_on/of_pwrseq_off
> > if only one power sequence instance is needed, for more power sequences
> > are used, using of_pwrseq_on_list/of_pwrseq_off_list instead (eg, USB hub driver).
> > 
> > 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
> 
> Just tested this patch set on UDOO board again to make sure that it still
> works after all changes done since it was last tested there and can confirm
> that it does work correctly.
> 

Thanks, Maciej.

-- 

Best Regards,
Peter Chen

Re: [PATCH v7 0/8] power: add power sequence library

[Peter Chen]

On Mon, Sep 26, 2016 at 02:26:49PM +0800, Peter Chen wrote:
> On Tue, Sep 20, 2016 at 11:36:39AM +0800, 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]. The kinds of
> > power sequence instances will be added at postcore_initcall, the match
> > criteria is compatible string first, if the compatible string is not
> > matched between dts and library, it will try to use generic power sequence.
> > 	 
> > The host driver just needs to call of_pwrseq_on/of_pwrseq_off
> > if only one power sequence instance is needed, for more power sequences
> > are used, using of_pwrseq_on_list/of_pwrseq_off_list instead (eg, USB hub driver).
> > 
> > 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 v7:
> > - Create kinds of power sequence instance at postcore_initcall, and match
> >   the instance with node using compatible string, the beneit of this is
> >   the host driver doesn't need to consider which pwrseq instance needs
> >   to be used, and pwrseq core will match it, however, it eats some memories
> >   if less power sequence instances are used. [Patch 2/8]
> > - Add pwrseq_compatible_sample.c to test match pwrseq using device_id. [Patch 2/8]
> > - Fix the comments Vaibhav Hiremath adds for error path for clock and do not
> >   use device_node for parameters at pwrseq_on. [Patch 2/8]
> > - Simplify the caller to use power sequence, follows Alan's commnets [Patch 4/8]
> > - Tested three pwrseq instances together using both specific compatible string and
> >   generic libraries.
> > 
> 
> Hi Vaibhav, would you please test if this series can support your case,
> you can add one instance like pwrseq_compatible_sample? If you are busy
> now, but think this series can satisfy your requirement, please ack it.
> I will delete pwrseq_compatible_sample, and only submit the generic one
> at next revision, you can add it later, thanks.
> 

ping...

Vaibhav, if you have no more comments, I will delete the
pwrseq_compatible_sample, and send v8.

Peter
> Peter
> 
> > Changes for v6:
> > - Add Matthias Kaehlcke's Reviewed-by and Tested-by. (patch [2/6])
> > - Change chipidea core of_node assignment for coming user. (patch [5/6])
> > - Applies Joshua Clayton's three dts changes for two boards,
> >   the USB device's reg has only #address-cells, but without #size-cells.
> > 
> > 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
> > 
> > Joshua Clayton (2):
> >   ARM: dts: imx6qdl: Enable usb node children with <reg>
> >   ARM: dts: imx6q-evi: Fix onboard hub reset line
> > 
> > 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/imx6q-evi.dts                    |  25 +--
> >  arch/arm/boot/dts/imx6qdl-udoo.dtsi                |  26 ++-
> >  arch/arm/boot/dts/imx6qdl.dtsi                     |   6 +
> >  drivers/power/Kconfig                              |   1 +
> >  drivers/power/Makefile                             |   1 +
> >  drivers/power/pwrseq/Kconfig                       |  45 +++++
> >  drivers/power/pwrseq/Makefile                      |   3 +
> >  drivers/power/pwrseq/core.c                        | 190 +++++++++++++++++++++
> >  drivers/power/pwrseq/pwrseq_compatible_sample.c    | 178 +++++++++++++++++++
> >  drivers/power/pwrseq/pwrseq_generic.c              | 177 +++++++++++++++++++
> >  drivers/usb/chipidea/core.c                        |  27 ++-
> >  drivers/usb/core/hub.c                             |  41 ++++-
> >  drivers/usb/core/hub.h                             |   1 +
> >  include/linux/power/pwrseq.h                       |  73 ++++++++
> >  17 files changed, 820 insertions(+), 41 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_compatible_sample.c
> >  create mode 100644 drivers/power/pwrseq/pwrseq_generic.c
> >  create mode 100644 include/linux/power/pwrseq.h
> > 
> > -- 
> > 2.7.4
> > 
> > --
> > 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
> 
> -- 
> 
> Best Regards,
> Peter Chen

-- 

Best Regards,
Peter Chen

Re: [PATCH v7 2/8] power: add power sequence library

[Heiko Stuebner]

Hi,

Am Dienstag, 20. September 2016, 11:36:41 CEST schrieb 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 just needs to call of_pwrseq_on/of_pwrseq_off
> if only one power sequence is needed, else call of_pwrseq_on_list
> /of_pwrseq_off_list instead (eg, USB hub driver).
>
> 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>

first of all, glad to see this move forward. I've only some qualms with the 
static number of allocated power sequences below.

[...]

> diff --git a/drivers/power/pwrseq/Kconfig b/drivers/power/pwrseq/Kconfig
> new file mode 100644
> index 0000000..dff5e35
> --- /dev/null
> +++ b/drivers/power/pwrseq/Kconfig
> @@ -0,0 +1,45 @@
> +#
> +# 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.
> +
> +config PWRSEQ_GENERIC_INSTANCE_NUMBER
> +	int "Number of Generic Power Sequence Instance"
> +	depends on PWRSEQ_GENERIC
> +	range 1 10
> +	default 2
> +	help
> +	   Usually, there are not so many devices needs power sequence, we set two
> +	   as default value.

limiting this to some arbitary compile-time number somehow seems crippling for 
the single-image approach. I.e. a distribution might select something and 
during its lifetime the board requiring n+1 power-sequences appears and thus 
needs a different kernel version just to support that additional sequence.

Also, board designers are creative, and there were already complex examples 
mentioned elsewhere, so nothing keeps people from inventing something even 
more complex.

[...]

> diff --git a/drivers/power/pwrseq/pwrseq_generic.c
> b/drivers/power/pwrseq/pwrseq_generic.c new file mode 100644
> index 0000000..bcd16c3
> --- /dev/null
> +++ b/drivers/power/pwrseq/pwrseq_generic.c

[...]

> +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 != -ENOENT)
> +				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);
> +		of_property_read_u32(np, "reset-duration-us",
> +				&pwrseq_gen->duration_us);
> +	} 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;
> +}
> +
> +static const struct of_device_id generic_id_table[] = {
> +	{ .compatible = "generic",},
> +	{ /* sentinel */ }
> +};
> +
> +static int __init pwrseq_generic_register(void)
> +{
> +	struct pwrseq_generic *pwrseq_gen;
> +	int i;
> +
> +	for (i = 0; i < CONFIG_PWRSEQ_GENERIC_INSTANCE_NUMBER; i++) {
> +		pwrseq_gen = kzalloc(sizeof(*pwrseq_gen), GFP_KERNEL);
> +		if (!pwrseq_gen)
> +			return -ENOMEM;
> +
> +		pwrseq_gen->pwrseq.pwrseq_of_match_table = generic_id_table;
> +		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 0;
> +}
> +postcore_initcall(pwrseq_generic_register)

I see that you need to have it preallocated for the compatible matching, but 
wouldn't it also work to either just register the type and allocate 
dynamically or otherwise just allocate a new spare everytime 
pwrseq_generic_get() picks up the previous spare?

That way the total number of power sequences can still be dynamic without 
haggling over how many power sequences should be the build-default in the 
generic configs.

Re: [PATCH v7 2/8] power: add power sequence library

[Peter Chen]

On Wed, Oct 12, 2016 at 12:30:29PM +0200, Heiko Stuebner wrote:
> Hi,
> 
> Am Dienstag, 20. September 2016, 11:36:41 CEST schrieb 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 just needs to call of_pwrseq_on/of_pwrseq_off
> > if only one power sequence is needed, else call of_pwrseq_on_list
> > /of_pwrseq_off_list instead (eg, USB hub driver).
> >
> > 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>
> 
> first of all, glad to see this move forward. I've only some qualms with the 
> static number of allocated power sequences below.
> 

Thanks for commenting it, the preallocate way is not a good way, but I
can't find suitable way. See below comments.

> > +static int __init pwrseq_generic_register(void)
> > +{
> > +	struct pwrseq_generic *pwrseq_gen;
> > +	int i;
> > +
> > +	for (i = 0; i < CONFIG_PWRSEQ_GENERIC_INSTANCE_NUMBER; i++) {
> > +		pwrseq_gen = kzalloc(sizeof(*pwrseq_gen), GFP_KERNEL);
> > +		if (!pwrseq_gen)
> > +			return -ENOMEM;
> > +
> > +		pwrseq_gen->pwrseq.pwrseq_of_match_table = generic_id_table;
> > +		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 0;
> > +}
> > +postcore_initcall(pwrseq_generic_register)
> 
> I see that you need to have it preallocated for the compatible matching, but 
> wouldn't it also work to either just register the type and allocate 
> dynamically or otherwise just allocate a new spare everytime 
> pwrseq_generic_get() picks up the previous spare?

Before compatible matching, the host driver doesn't know which pwrseq type
for its child node, then doesn't know which pwrseq instance needs to be
allocated. From dts, we don't know which pwrseq type for the node.

-- 

Best Regards,
Peter Chen

Re: [PATCH v7 2/8] power: add power sequence library

[Heiko Stuebner]

Am Donnerstag, 13. Oktober 2016, 09:22:16 CEST schrieb Peter Chen:
> On Wed, Oct 12, 2016 at 12:30:29PM +0200, Heiko Stuebner wrote:
> > Hi,
> > 
> > Am Dienstag, 20. September 2016, 11:36:41 CEST schrieb 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 just needs to call of_pwrseq_on/of_pwrseq_off
> > > if only one power sequence is needed, else call of_pwrseq_on_list
> > > /of_pwrseq_off_list instead (eg, USB hub driver).
> > > 
> > > 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>
> > 
> > first of all, glad to see this move forward. I've only some qualms with
> > the
> > static number of allocated power sequences below.
> 
> Thanks for commenting it, the preallocate way is not a good way, but I
> can't find suitable way. See below comments.
> 
> > > +static int __init pwrseq_generic_register(void)
> > > +{
> > > +	struct pwrseq_generic *pwrseq_gen;
> > > +	int i;
> > > +
> > > +	for (i = 0; i < CONFIG_PWRSEQ_GENERIC_INSTANCE_NUMBER; i++) {
> > > +		pwrseq_gen = kzalloc(sizeof(*pwrseq_gen), GFP_KERNEL);
> > > +		if (!pwrseq_gen)
> > > +			return -ENOMEM;
> > > +
> > > +		pwrseq_gen->pwrseq.pwrseq_of_match_table = generic_id_table;
> > > +		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 0;
> > > +}
> > > +postcore_initcall(pwrseq_generic_register)
> > 
> > I see that you need to have it preallocated for the compatible matching,
> > but wouldn't it also work to either just register the type and allocate
> > dynamically or otherwise just allocate a new spare everytime
> > pwrseq_generic_get() picks up the previous spare?
> 
> Before compatible matching, the host driver doesn't know which pwrseq type
> for its child node, then doesn't know which pwrseq instance needs to be
> allocated. From dts, we don't know which pwrseq type for the node.

yes, that is why I was suggesting allocating one (or two) here in 
pwrseq_generic_register() and every time pwrseq_generic_get() grabs the last 
free sequence instance, you allocate a new free spare one in that function.

Re: [PATCH v7 2/8] power: add power sequence library

[Peter Chen]

On Thu, Oct 13, 2016 at 09:04:42AM +0200, Heiko Stuebner wrote:
> > > > +static int __init pwrseq_generic_register(void)
> > > > +{
> > > > +	struct pwrseq_generic *pwrseq_gen;
> > > > +	int i;
> > > > +
> > > > +	for (i = 0; i < CONFIG_PWRSEQ_GENERIC_INSTANCE_NUMBER; i++) {
> > > > +		pwrseq_gen = kzalloc(sizeof(*pwrseq_gen), GFP_KERNEL);
> > > > +		if (!pwrseq_gen)
> > > > +			return -ENOMEM;
> > > > +
> > > > +		pwrseq_gen->pwrseq.pwrseq_of_match_table = generic_id_table;
> > > > +		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 0;
> > > > +}
> > > > +postcore_initcall(pwrseq_generic_register)
> > > 
> > > I see that you need to have it preallocated for the compatible matching,
> > > but wouldn't it also work to either just register the type and allocate
> > > dynamically or otherwise just allocate a new spare everytime
> > > pwrseq_generic_get() picks up the previous spare?
> > 
> > Before compatible matching, the host driver doesn't know which pwrseq type
> > for its child node, then doesn't know which pwrseq instance needs to be
> > allocated. From dts, we don't know which pwrseq type for the node.
> 
> yes, that is why I was suggesting allocating one (or two) here in 
> pwrseq_generic_register() and every time pwrseq_generic_get() grabs the last 
> free sequence instance, you allocate a new free spare one in that function.

Good idea.

-- 

Best Regards,
Peter Chen