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

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

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: host: let the hcd know's parent device node
  ARM: dts: imx6qdl-udoo.dtsi: fix onboard USB HUB property

 .../bindings/power/pwrseq/pwrseq-generic.txt       |  56 +++++++++
 .../devicetree/bindings/usb/usb-device.txt         |  10 ++
 arch/arm/boot/dts/imx6qdl-udoo.dtsi                |  28 +++--
 drivers/power/Kconfig                              |   1 +
 drivers/power/Makefile                             |   1 +
 drivers/power/pwrseq/Kconfig                       |  20 +++
 drivers/power/pwrseq/Makefile                      |   2 +
 drivers/power/pwrseq/core.c                        |  79 ++++++++++++
 drivers/power/pwrseq/pwrseq_generic.c              | 134 +++++++++++++++++++++
 drivers/usb/chipidea/host.c                        |  18 ++-
 drivers/usb/core/Makefile                          |   1 +
 drivers/usb/core/hub.c                             |  12 +-
 drivers/usb/core/hub.h                             |  12 ++
 drivers/usb/core/pwrseq.c                          | 102 ++++++++++++++++
 include/linux/power/pwrseq.h                       |  55 +++++++++
 15 files changed, 511 insertions(+), 20 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 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

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: host: let the hcd know's parent device node
  ARM: dts: imx6qdl-udoo.dtsi: fix onboard USB HUB property

 .../bindings/power/pwrseq/pwrseq-generic.txt       |  56 +++++++++
 .../devicetree/bindings/usb/usb-device.txt         |  10 ++
 arch/arm/boot/dts/imx6qdl-udoo.dtsi                |  28 +++--
 drivers/power/Kconfig                              |   1 +
 drivers/power/Makefile                             |   1 +
 drivers/power/pwrseq/Kconfig                       |  20 +++
 drivers/power/pwrseq/Makefile                      |   2 +
 drivers/power/pwrseq/core.c                        |  79 ++++++++++++
 drivers/power/pwrseq/pwrseq_generic.c              | 134 +++++++++++++++++++++
 drivers/usb/chipidea/host.c                        |  18 ++-
 drivers/usb/core/Makefile                          |   1 +
 drivers/usb/core/hub.c                             |  12 +-
 drivers/usb/core/hub.h                             |  12 ++
 drivers/usb/core/pwrseq.c                          | 102 ++++++++++++++++
 include/linux/power/pwrseq.h                       |  55 +++++++++
 15 files changed, 511 insertions(+), 20 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 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-3arQi8VN3Tc@public.gmane.org>
---
 .../bindings/power/pwrseq/pwrseq-generic.txt       | 56 ++++++++++++++++++++++
 1 file changed, 56 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..4b23834
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
@@ -0,0 +1,56 @@
+The generic power sequence library
+
+Some hard-wired USB/MMC 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. The
+power sequence will be done before this device can be found at USB
+bus.
+
+The power sequence properties is under the device node.
+
+Required properties:
+- power-sequence: this device needs to do power sequence before enumeration
+
+Optional properties:
+- clocks: the input clock for device.
+- clock-name: must be "pwrseq-clk"
+- pwrseq-reset-gpios: Should specify the GPIO for reset.
+- pwrseq-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>;
+	hub: genesys@1 {
+		compatible = "usb5e3,608";
+		reg = <1>;
+
+		power-sequence;
+		clocks = <&clks IMX6SX_CLK_CKO>;
+		clock-names = "pwrseq-clk";
+		pwrseq-reset-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>; /* hub reset pin */
+		pwrseq-reset-duration-us = <10>;
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+		ethernet: asix@1 {
+			compatible = "usbb95,1708";
+			reg = <1>;
+
+			power-sequence;
+			clocks = <&clks IMX6SX_CLK_IPG>;
+			clock-names = "pwrseq-clk";
+			pwrseq-reset-gpios = <&gpio4 6 GPIO_ACTIVE_LOW>; /* ethernet_rst */
+			pwrseq-reset-duration-us = <15>;
+		};
+	};
+};
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 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>
---
 .../bindings/power/pwrseq/pwrseq-generic.txt       | 56 ++++++++++++++++++++++
 1 file changed, 56 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..4b23834
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
@@ -0,0 +1,56 @@
+The generic power sequence library
+
+Some hard-wired USB/MMC 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. The
+power sequence will be done before this device can be found at USB
+bus.
+
+The power sequence properties is under the device node.
+
+Required properties:
+- power-sequence: this device needs to do power sequence before enumeration
+
+Optional properties:
+- clocks: the input clock for device.
+- clock-name: must be "pwrseq-clk"
+- pwrseq-reset-gpios: Should specify the GPIO for reset.
+- pwrseq-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>;
+	hub: genesys at 1 {
+		compatible = "usb5e3,608";
+		reg = <1>;
+
+		power-sequence;
+		clocks = <&clks IMX6SX_CLK_CKO>;
+		clock-names = "pwrseq-clk";
+		pwrseq-reset-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>; /* hub reset pin */
+		pwrseq-reset-duration-us = <10>;
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+		ethernet: asix at 1 {
+			compatible = "usbb95,1708";
+			reg = <1>;
+
+			power-sequence;
+			clocks = <&clks IMX6SX_CLK_IPG>;
+			clock-names = "pwrseq-clk";
+			pwrseq-reset-gpios = <&gpio4 6 GPIO_ACTIVE_LOW>; /* ethernet_rst */
+			pwrseq-reset-duration-us = <15>;
+		};
+	};
+};
-- 
1.9.1

[PATCH 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>
---
 drivers/power/Kconfig                 |   1 +
 drivers/power/Makefile                |   1 +
 drivers/power/pwrseq/Kconfig          |  20 +++++
 drivers/power/pwrseq/Makefile         |   2 +
 drivers/power/pwrseq/core.c           |  79 ++++++++++++++++++++
 drivers/power/pwrseq/pwrseq_generic.c | 134 ++++++++++++++++++++++++++++++++++
 include/linux/power/pwrseq.h          |  55 ++++++++++++++
 7 files changed, 292 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/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..e40315c
--- /dev/null
+++ b/drivers/power/pwrseq/core.c
@@ -0,0 +1,79 @@
+/*
+ * core.c	power sequence core file
+ *
+ * Copyright (C) 2016 Freescale Semiconductor, Inc.
+ * Author: Peter Chen <peter.chen@nxp.com>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2  of
+ * the License as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/power/pwrseq.h>
+
+static DEFINE_MUTEX(pwrseq_list_mutex);
+static LIST_HEAD(pwrseq_list);
+
+int pwrseq_get(struct device_node *np, struct pwrseq *p)
+{
+	if (p && p->get)
+		return p->get(np, p);
+
+	return -ENOTSUPP;
+}
+
+int pwrseq_on(struct device_node *np, struct pwrseq *p)
+{
+	if (p && p->on)
+		return p->on(np, p);
+
+	return -ENOTSUPP;
+}
+
+void pwrseq_off(struct pwrseq *p)
+{
+	if (p && p->off)
+		p->off(p);
+}
+
+void pwrseq_put(struct pwrseq *p)
+{
+	if (p && p->put)
+		p->put(p);
+}
+
+void pwrseq_free(struct pwrseq *p)
+{
+	if (p && p->free)
+		p->free(p);
+}
+
+void pwrseq_register(struct pwrseq *pwrseq)
+{
+	mutex_lock(&pwrseq_list_mutex);
+	list_add(&pwrseq->node, &pwrseq_list);
+	mutex_unlock(&pwrseq_list_mutex);
+}
+
+void pwrseq_unregister(struct pwrseq *pwrseq)
+{
+	mutex_lock(&pwrseq_list_mutex);
+	list_del(&pwrseq->node);
+	mutex_unlock(&pwrseq_list_mutex);
+}
+
+bool is_pwrseq_supported(struct device_node *np)
+{
+	if (of_find_property(np, "power-sequence", NULL))
+		return true;
+
+	return false;
+}
diff --git a/drivers/power/pwrseq/pwrseq_generic.c b/drivers/power/pwrseq/pwrseq_generic.c
new file mode 100644
index 0000000..e3cacb0
--- /dev/null
+++ b/drivers/power/pwrseq/pwrseq_generic.c
@@ -0,0 +1,134 @@
+/*
+ * pwrseq_generic.c	Generic power sequence handling
+ *
+ * Copyright (C) 2016 Freescale Semiconductor, Inc.
+ * Author: Peter Chen <peter.chen@nxp.com>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2  of
+ * the License as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+#include <linux/slab.h>
+
+#include <linux/power/pwrseq.h>
+
+struct pwrseq_generic {
+	struct pwrseq pwrseq;
+	struct gpio_desc *gpiod_reset;
+	struct clk *clk;
+};
+
+#define to_generic_pwrseq(p) container_of(p, struct pwrseq_generic, pwrseq)
+
+static void pwrseq_generic_free(struct pwrseq *pwrseq)
+{
+	struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
+
+	pwrseq_unregister(pwrseq);
+	kfree(pwrseq_gen);
+}
+
+static void pwrseq_generic_put(struct pwrseq *pwrseq)
+{
+	struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
+
+	if (pwrseq_gen->gpiod_reset)
+		gpiod_put(pwrseq_gen->gpiod_reset);
+
+	clk_put(pwrseq_gen->clk);
+}
+
+static void pwrseq_generic_off(struct pwrseq *pwrseq)
+{
+	struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
+
+	clk_disable_unprepare(pwrseq_gen->clk);
+}
+
+static int pwrseq_generic_on(struct device_node *np, struct pwrseq *pwrseq)
+{
+	struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
+	int ret = 0;
+	u32 duration_us = 50;
+	struct gpio_desc *gpiod_reset = pwrseq_gen->gpiod_reset;
+
+	if (pwrseq_gen->clk) {
+		ret = clk_prepare_enable(pwrseq_gen->clk);
+		if (ret) {
+			pr_err("Can't enable clock on %s: %d\n",
+				np->name, ret);
+			return ret;
+		}
+	}
+
+	if (gpiod_reset) {
+		of_property_read_u32(np, "pwrseq-reset-duration-us",
+				&duration_us);
+		gpiod_direction_output(gpiod_reset, 1);
+		gpiod_set_value(gpiod_reset, 1);
+		usleep_range(duration_us, duration_us + 10);
+		gpiod_set_value(gpiod_reset, 0);
+	}
+
+	return ret;
+}
+
+static int pwrseq_generic_get(struct device_node *np, struct pwrseq *pwrseq)
+{
+	struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
+	int gpio;
+
+	pwrseq_gen->clk = of_clk_get_by_name(np, "pwrseq-clk");
+	if (IS_ERR(pwrseq_gen->clk)) {
+		pr_debug("Can't get clock on %s: %ld\n",
+			np->name, PTR_ERR(pwrseq_gen->clk));
+		pwrseq_gen->clk = NULL;
+	}
+
+	gpio = of_get_named_gpio_flags(np, "pwrseq-reset-gpios", 0, NULL);
+	if (IS_ERR_VALUE(gpio) && (gpio != -ENODEV)) {
+		pr_err("Failed to get reset gpio on %s, err = %d\n",
+				np->name, gpio);
+		clk_put(pwrseq_gen->clk);
+		return gpio;
+	}
+
+	if (gpio != -ENODEV)
+		pwrseq_gen->gpiod_reset = gpio_to_desc(gpio);
+
+	return 0;
+}
+
+struct pwrseq *pwrseq_alloc_generic(void)
+{
+	struct pwrseq_generic *pwrseq_gen;
+
+	pwrseq_gen = kzalloc(sizeof(pwrseq_gen), GFP_KERNEL);
+	if (!pwrseq_gen)
+		return ERR_PTR(-ENOMEM);
+
+	pwrseq_gen->pwrseq.get = pwrseq_generic_get;
+	pwrseq_gen->pwrseq.on = pwrseq_generic_on;
+	pwrseq_gen->pwrseq.off = pwrseq_generic_off;
+	pwrseq_gen->pwrseq.put = pwrseq_generic_put;
+	pwrseq_gen->pwrseq.free = pwrseq_generic_free;
+
+	pwrseq_register(&pwrseq_gen->pwrseq);
+	return &pwrseq_gen->pwrseq;
+}
+EXPORT_SYMBOL_GPL(pwrseq_alloc_generic);
diff --git a/include/linux/power/pwrseq.h b/include/linux/power/pwrseq.h
new file mode 100644
index 0000000..600f70c
--- /dev/null
+++ b/include/linux/power/pwrseq.h
@@ -0,0 +1,55 @@
+#ifndef __LINUX_PWRSEQ_H
+#define __LINUX_PWRSEQ_H
+
+#include <linux/of.h>
+
+struct pwrseq {
+	char *name;
+	struct list_head node;
+	int (*get)(struct device_node *np, struct pwrseq *p);
+	int (*on)(struct device_node *np, struct pwrseq *p);
+	void (*off)(struct pwrseq *p);
+	void (*put)(struct pwrseq *p);
+	void (*free)(struct pwrseq *p);
+};
+
+#if IS_ENABLED(CONFIG_POWER_SEQUENCE)
+int pwrseq_get(struct device_node *np, struct pwrseq *p);
+int pwrseq_on(struct device_node *np, struct pwrseq *p);
+void pwrseq_off(struct pwrseq *p);
+void pwrseq_put(struct pwrseq *p);
+void pwrseq_free(struct pwrseq *p);
+void pwrseq_register(struct pwrseq *pwrseq);
+void pwrseq_unregister(struct pwrseq *pwrseq);
+bool is_pwrseq_supported(struct device_node *np);
+
+#else
+static inline int pwrseq_get(struct device_node *np, struct pwrseq *p)
+{
+	return 0;
+}
+static inline int pwrseq_on(struct device_node *np, struct pwrseq *p)
+{
+	return 0;
+}
+static inline void pwrseq_off(struct pwrseq *p) {}
+static inline void pwrseq_put(struct pwrseq *p) {}
+static inline void pwrseq_free(struct pwrseq *p) {}
+static inline void pwrseq_register(struct pwrseq *pwrseq) {}
+static inline void pwrseq_unregister(struct pwrseq *pwrseq) {}
+static inline bool is_pwrseq_supported(struct device_node *np)
+{
+	return false;
+}
+#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 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>
---
 drivers/power/Kconfig                 |   1 +
 drivers/power/Makefile                |   1 +
 drivers/power/pwrseq/Kconfig          |  20 +++++
 drivers/power/pwrseq/Makefile         |   2 +
 drivers/power/pwrseq/core.c           |  79 ++++++++++++++++++++
 drivers/power/pwrseq/pwrseq_generic.c | 134 ++++++++++++++++++++++++++++++++++
 include/linux/power/pwrseq.h          |  55 ++++++++++++++
 7 files changed, 292 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/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..e40315c
--- /dev/null
+++ b/drivers/power/pwrseq/core.c
@@ -0,0 +1,79 @@
+/*
+ * core.c	power sequence core file
+ *
+ * Copyright (C) 2016 Freescale Semiconductor, Inc.
+ * Author: Peter Chen <peter.chen@nxp.com>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2  of
+ * the License as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/power/pwrseq.h>
+
+static DEFINE_MUTEX(pwrseq_list_mutex);
+static LIST_HEAD(pwrseq_list);
+
+int pwrseq_get(struct device_node *np, struct pwrseq *p)
+{
+	if (p && p->get)
+		return p->get(np, p);
+
+	return -ENOTSUPP;
+}
+
+int pwrseq_on(struct device_node *np, struct pwrseq *p)
+{
+	if (p && p->on)
+		return p->on(np, p);
+
+	return -ENOTSUPP;
+}
+
+void pwrseq_off(struct pwrseq *p)
+{
+	if (p && p->off)
+		p->off(p);
+}
+
+void pwrseq_put(struct pwrseq *p)
+{
+	if (p && p->put)
+		p->put(p);
+}
+
+void pwrseq_free(struct pwrseq *p)
+{
+	if (p && p->free)
+		p->free(p);
+}
+
+void pwrseq_register(struct pwrseq *pwrseq)
+{
+	mutex_lock(&pwrseq_list_mutex);
+	list_add(&pwrseq->node, &pwrseq_list);
+	mutex_unlock(&pwrseq_list_mutex);
+}
+
+void pwrseq_unregister(struct pwrseq *pwrseq)
+{
+	mutex_lock(&pwrseq_list_mutex);
+	list_del(&pwrseq->node);
+	mutex_unlock(&pwrseq_list_mutex);
+}
+
+bool is_pwrseq_supported(struct device_node *np)
+{
+	if (of_find_property(np, "power-sequence", NULL))
+		return true;
+
+	return false;
+}
diff --git a/drivers/power/pwrseq/pwrseq_generic.c b/drivers/power/pwrseq/pwrseq_generic.c
new file mode 100644
index 0000000..e3cacb0
--- /dev/null
+++ b/drivers/power/pwrseq/pwrseq_generic.c
@@ -0,0 +1,134 @@
+/*
+ * pwrseq_generic.c	Generic power sequence handling
+ *
+ * Copyright (C) 2016 Freescale Semiconductor, Inc.
+ * Author: Peter Chen <peter.chen@nxp.com>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2  of
+ * the License as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+#include <linux/slab.h>
+
+#include <linux/power/pwrseq.h>
+
+struct pwrseq_generic {
+	struct pwrseq pwrseq;
+	struct gpio_desc *gpiod_reset;
+	struct clk *clk;
+};
+
+#define to_generic_pwrseq(p) container_of(p, struct pwrseq_generic, pwrseq)
+
+static void pwrseq_generic_free(struct pwrseq *pwrseq)
+{
+	struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
+
+	pwrseq_unregister(pwrseq);
+	kfree(pwrseq_gen);
+}
+
+static void pwrseq_generic_put(struct pwrseq *pwrseq)
+{
+	struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
+
+	if (pwrseq_gen->gpiod_reset)
+		gpiod_put(pwrseq_gen->gpiod_reset);
+
+	clk_put(pwrseq_gen->clk);
+}
+
+static void pwrseq_generic_off(struct pwrseq *pwrseq)
+{
+	struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
+
+	clk_disable_unprepare(pwrseq_gen->clk);
+}
+
+static int pwrseq_generic_on(struct device_node *np, struct pwrseq *pwrseq)
+{
+	struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
+	int ret = 0;
+	u32 duration_us = 50;
+	struct gpio_desc *gpiod_reset = pwrseq_gen->gpiod_reset;
+
+	if (pwrseq_gen->clk) {
+		ret = clk_prepare_enable(pwrseq_gen->clk);
+		if (ret) {
+			pr_err("Can't enable clock on %s: %d\n",
+				np->name, ret);
+			return ret;
+		}
+	}
+
+	if (gpiod_reset) {
+		of_property_read_u32(np, "pwrseq-reset-duration-us",
+				&duration_us);
+		gpiod_direction_output(gpiod_reset, 1);
+		gpiod_set_value(gpiod_reset, 1);
+		usleep_range(duration_us, duration_us + 10);
+		gpiod_set_value(gpiod_reset, 0);
+	}
+
+	return ret;
+}
+
+static int pwrseq_generic_get(struct device_node *np, struct pwrseq *pwrseq)
+{
+	struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
+	int gpio;
+
+	pwrseq_gen->clk = of_clk_get_by_name(np, "pwrseq-clk");
+	if (IS_ERR(pwrseq_gen->clk)) {
+		pr_debug("Can't get clock on %s: %ld\n",
+			np->name, PTR_ERR(pwrseq_gen->clk));
+		pwrseq_gen->clk = NULL;
+	}
+
+	gpio = of_get_named_gpio_flags(np, "pwrseq-reset-gpios", 0, NULL);
+	if (IS_ERR_VALUE(gpio) && (gpio != -ENODEV)) {
+		pr_err("Failed to get reset gpio on %s, err = %d\n",
+				np->name, gpio);
+		clk_put(pwrseq_gen->clk);
+		return gpio;
+	}
+
+	if (gpio != -ENODEV)
+		pwrseq_gen->gpiod_reset = gpio_to_desc(gpio);
+
+	return 0;
+}
+
+struct pwrseq *pwrseq_alloc_generic(void)
+{
+	struct pwrseq_generic *pwrseq_gen;
+
+	pwrseq_gen = kzalloc(sizeof(pwrseq_gen), GFP_KERNEL);
+	if (!pwrseq_gen)
+		return ERR_PTR(-ENOMEM);
+
+	pwrseq_gen->pwrseq.get = pwrseq_generic_get;
+	pwrseq_gen->pwrseq.on = pwrseq_generic_on;
+	pwrseq_gen->pwrseq.off = pwrseq_generic_off;
+	pwrseq_gen->pwrseq.put = pwrseq_generic_put;
+	pwrseq_gen->pwrseq.free = pwrseq_generic_free;
+
+	pwrseq_register(&pwrseq_gen->pwrseq);
+	return &pwrseq_gen->pwrseq;
+}
+EXPORT_SYMBOL_GPL(pwrseq_alloc_generic);
diff --git a/include/linux/power/pwrseq.h b/include/linux/power/pwrseq.h
new file mode 100644
index 0000000..600f70c
--- /dev/null
+++ b/include/linux/power/pwrseq.h
@@ -0,0 +1,55 @@
+#ifndef __LINUX_PWRSEQ_H
+#define __LINUX_PWRSEQ_H
+
+#include <linux/of.h>
+
+struct pwrseq {
+	char *name;
+	struct list_head node;
+	int (*get)(struct device_node *np, struct pwrseq *p);
+	int (*on)(struct device_node *np, struct pwrseq *p);
+	void (*off)(struct pwrseq *p);
+	void (*put)(struct pwrseq *p);
+	void (*free)(struct pwrseq *p);
+};
+
+#if IS_ENABLED(CONFIG_POWER_SEQUENCE)
+int pwrseq_get(struct device_node *np, struct pwrseq *p);
+int pwrseq_on(struct device_node *np, struct pwrseq *p);
+void pwrseq_off(struct pwrseq *p);
+void pwrseq_put(struct pwrseq *p);
+void pwrseq_free(struct pwrseq *p);
+void pwrseq_register(struct pwrseq *pwrseq);
+void pwrseq_unregister(struct pwrseq *pwrseq);
+bool is_pwrseq_supported(struct device_node *np);
+
+#else
+static inline int pwrseq_get(struct device_node *np, struct pwrseq *p)
+{
+	return 0;
+}
+static inline int pwrseq_on(struct device_node *np, struct pwrseq *p)
+{
+	return 0;
+}
+static inline void pwrseq_off(struct pwrseq *p) {}
+static inline void pwrseq_put(struct pwrseq *p) {}
+static inline void pwrseq_free(struct pwrseq *p) {}
+static inline void pwrseq_register(struct pwrseq *pwrseq) {}
+static inline void pwrseq_unregister(struct pwrseq *pwrseq) {}
+static inline bool is_pwrseq_supported(struct device_node *np)
+{
+	return false;
+}
+#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 3/6] binding-doc: usb: usb-device: add optional properties for power sequence

[Peter Chen]

Add optional properties for power sequence.

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

diff --git a/Documentation/devicetree/bindings/usb/usb-device.txt b/Documentation/devicetree/bindings/usb/usb-device.txt
index 1c35e7b..ef0c9b2 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 {
@@ -24,5 +28,11 @@ Example:
 	hub: genesys@1 {
 		compatible = "usb5e3,608";
 		reg = <1>;
+
+		power-sequence;
+		clocks = <&clks IMX6SX_CLK_CKO>;
+		clock-names = "pwrseq-clk";
+		pwrseq-reset-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>; /* hub reset pin */
+		pwrseq-reset-duration-us = <10>;
 	};
 }
-- 
1.9.1

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

[Peter Chen]

Add optional properties for power sequence.

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

diff --git a/Documentation/devicetree/bindings/usb/usb-device.txt b/Documentation/devicetree/bindings/usb/usb-device.txt
index 1c35e7b..ef0c9b2 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 {
@@ -24,5 +28,11 @@ Example:
 	hub: genesys at 1 {
 		compatible = "usb5e3,608";
 		reg = <1>;
+
+		power-sequence;
+		clocks = <&clks IMX6SX_CLK_CKO>;
+		clock-names = "pwrseq-clk";
+		pwrseq-reset-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>; /* hub reset pin */
+		pwrseq-reset-duration-us = <10>;
 	};
 }
-- 
1.9.1

[PATCH 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) if this device is described at dts and there
is a property "power-sequence" for it.

At hub_disconnect, it will do power off sequence which is at powered
on list.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/usb/core/Makefile |   1 +
 drivers/usb/core/hub.c    |  12 ++++--
 drivers/usb/core/hub.h    |  12 ++++++
 drivers/usb/core/pwrseq.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 124 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..fbfa122
--- /dev/null
+++ b/drivers/usb/core/pwrseq.c
@@ -0,0 +1,102 @@
+/*
+ * 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) {
+		if (is_pwrseq_supported(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 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) if this device is described at dts and there
is a property "power-sequence" for it.

At hub_disconnect, it will do power off sequence which is at powered
on list.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/usb/core/Makefile |   1 +
 drivers/usb/core/hub.c    |  12 ++++--
 drivers/usb/core/hub.h    |  12 ++++++
 drivers/usb/core/pwrseq.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 124 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..fbfa122
--- /dev/null
+++ b/drivers/usb/core/pwrseq.c
@@ -0,0 +1,102 @@
+/*
+ * 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) {
+		if (is_pwrseq_supported(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 5/6] usb: chipidea: host: let the hcd know's parent device node

[Peter Chen]

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

Since the hcd (chipidea core device) has no device node, so
if we want to describe the child node under the hcd, we had
to put it under its parent's node (glue layer device), and
in the code, we need to let the hcd knows glue layer's code,
then the USB core can handle this node.

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

diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
index 053bac9..55120ef 100644
--- a/drivers/usb/chipidea/host.c
+++ b/drivers/usb/chipidea/host.c
@@ -109,15 +109,25 @@ static int host_start(struct ci_hdrc *ci)
 	struct ehci_hcd *ehci;
 	struct ehci_ci_priv *priv;
 	int ret;
+	struct device *dev = ci->dev;
 
-	if (usb_disabled())
+	if (usb_disabled() || !dev)
 		return -ENODEV;
 
-	hcd = usb_create_hcd(&ci_ehci_hc_driver, ci->dev, dev_name(ci->dev));
+	/*
+	 * USB Core will try to get child node under roothub,
+	 * but chipidea core has no of_node, and the child node
+	 * for controller is located at glue layer's node which
+	 * is chipidea core's parent.
+	 */
+	if (dev->parent && dev->parent->of_node)
+		dev->of_node = dev->parent->of_node;
+
+	hcd = usb_create_hcd(&ci_ehci_hc_driver, dev, dev_name(dev));
 	if (!hcd)
 		return -ENOMEM;
 
-	dev_set_drvdata(ci->dev, ci);
+	dev_set_drvdata(dev, ci);
 	hcd->rsrc_start = ci->hw_bank.phys;
 	hcd->rsrc_len = ci->hw_bank.size;
 	hcd->regs = ci->hw_bank.abs;
@@ -143,7 +153,7 @@ static int host_start(struct ci_hdrc *ci)
 		if (ci->platdata->flags & CI_HDRC_TURN_VBUS_EARLY_ON) {
 			ret = regulator_enable(ci->platdata->reg_vbus);
 			if (ret) {
-				dev_err(ci->dev,
+				dev_err(dev,
 				"Failed to enable vbus regulator, ret=%d\n",
 									ret);
 				goto put_hcd;
-- 
1.9.1

[PATCH 5/6] usb: chipidea: host: let the hcd know's parent device node

[Peter Chen]

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

Since the hcd (chipidea core device) has no device node, so
if we want to describe the child node under the hcd, we had
to put it under its parent's node (glue layer device), and
in the code, we need to let the hcd knows glue layer's code,
then the USB core can handle this node.

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

diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
index 053bac9..55120ef 100644
--- a/drivers/usb/chipidea/host.c
+++ b/drivers/usb/chipidea/host.c
@@ -109,15 +109,25 @@ static int host_start(struct ci_hdrc *ci)
 	struct ehci_hcd *ehci;
 	struct ehci_ci_priv *priv;
 	int ret;
+	struct device *dev = ci->dev;
 
-	if (usb_disabled())
+	if (usb_disabled() || !dev)
 		return -ENODEV;
 
-	hcd = usb_create_hcd(&ci_ehci_hc_driver, ci->dev, dev_name(ci->dev));
+	/*
+	 * USB Core will try to get child node under roothub,
+	 * but chipidea core has no of_node, and the child node
+	 * for controller is located at glue layer's node which
+	 * is chipidea core's parent.
+	 */
+	if (dev->parent && dev->parent->of_node)
+		dev->of_node = dev->parent->of_node;
+
+	hcd = usb_create_hcd(&ci_ehci_hc_driver, dev, dev_name(dev));
 	if (!hcd)
 		return -ENOMEM;
 
-	dev_set_drvdata(ci->dev, ci);
+	dev_set_drvdata(dev, ci);
 	hcd->rsrc_start = ci->hw_bank.phys;
 	hcd->rsrc_len = ci->hw_bank.size;
 	hcd->regs = ci->hw_bank.abs;
@@ -143,7 +153,7 @@ static int host_start(struct ci_hdrc *ci)
 		if (ci->platdata->flags & CI_HDRC_TURN_VBUS_EARLY_ON) {
 			ret = regulator_enable(ci->platdata->reg_vbus);
 			if (ret) {
-				dev_err(ci->dev,
+				dev_err(dev,
 				"Failed to enable vbus regulator, ret=%d\n",
 									ret);
 				goto put_hcd;
-- 
1.9.1

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

[Peter Chen]

From: Peter Chen <peter.chen@freescale.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@freescale.com>
Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
---
 arch/arm/boot/dts/imx6qdl-udoo.dtsi | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/arch/arm/boot/dts/imx6qdl-udoo.dtsi b/arch/arm/boot/dts/imx6qdl-udoo.dtsi
index 3bee2f9..e6c4e8c 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,20 @@
 &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>;
+	hub: usb2415@1 {
+		compatible = "usb424,2514";
+		reg = <1>;
+
+		power-sequence;
+		clocks = <&clks IMX6QDL_CLK_CKO>;
+		clock-names = "pwrseq-clk";
+		pwrseq-reset-gpios = <&gpio7 12 GPIO_ACTIVE_LOW>;
+		pwrseq-reset-duration-us = <3000>;
+	};
 };
 
 &usdhc3 {
-- 
1.9.1

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

[Peter Chen]

From: Peter Chen <peter.chen@freescale.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@freescale.com>
Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
---
 arch/arm/boot/dts/imx6qdl-udoo.dtsi | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

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

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

[Philipp Zabel]

Am Donnerstag, den 07.07.2016, 17:14 +0800 schrieb Peter Chen:
> Add binding doc for generic power sequence library.
> 
> Signed-off-by: Peter Chen <peter.chen-3arQi8VN3Tc@public.gmane.org>
> ---
>  .../bindings/power/pwrseq/pwrseq-generic.txt       | 56 ++++++++++++++++++++++
>  1 file changed, 56 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..4b23834
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
> @@ -0,0 +1,56 @@
> +The generic power sequence library
> +
> +Some hard-wired USB/MMC 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. The
> +power sequence will be done before this device can be found at USB
> +bus.
> +
> +The power sequence properties is under the device node.
> +
> +Required properties:
> +- power-sequence: this device needs to do power sequence before enumeration
> +
> +Optional properties:
> +- clocks: the input clock for device.
> +- clock-name: must be "pwrseq-clk"

The "-clk" in the clock name is redundant.

> +- pwrseq-reset-gpios: Should specify the GPIO for reset.
> +- pwrseq-reset-duration-us: the duration in microsecond for assert reset signal.

I understand you want to make it explicit that this GPIO is for the
pwrseq library, but are we really gaining anything over just calling
these reset-gpios and reset-duration-us?
The same applies to the clock name above.

> +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>;
> +	hub: genesys@1 {
> +		compatible = "usb5e3,608";
> +		reg = <1>;
> +
> +		power-sequence;
> +		clocks = <&clks IMX6SX_CLK_CKO>;
> +		clock-names = "pwrseq-clk";
> +		pwrseq-reset-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>; /* hub reset pin */
> +		pwrseq-reset-duration-us = <10>;
> +
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		ethernet: asix@1 {
> +			compatible = "usbb95,1708";
> +			reg = <1>;
> +
> +			power-sequence;
> +			clocks = <&clks IMX6SX_CLK_IPG>;
> +			clock-names = "pwrseq-clk";
> +			pwrseq-reset-gpios = <&gpio4 6 GPIO_ACTIVE_LOW>; /* ethernet_rst */
> +			pwrseq-reset-duration-us = <15>;
> +		};

This looks weird. The hub and ethernet chips don't have "pwrseq" clock
and reset input pins. I'd remove the clock-names and pwrseq- reset
prefix.

regards
Philipp

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Philipp Zabel]

Am Donnerstag, den 07.07.2016, 17:14 +0800 schrieb Peter Chen:
> Add binding doc for generic power sequence library.
> 
> Signed-off-by: Peter Chen <peter.chen@nxp.com>
> ---
>  .../bindings/power/pwrseq/pwrseq-generic.txt       | 56 ++++++++++++++++++++++
>  1 file changed, 56 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..4b23834
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
> @@ -0,0 +1,56 @@
> +The generic power sequence library
> +
> +Some hard-wired USB/MMC 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. The
> +power sequence will be done before this device can be found at USB
> +bus.
> +
> +The power sequence properties is under the device node.
> +
> +Required properties:
> +- power-sequence: this device needs to do power sequence before enumeration
> +
> +Optional properties:
> +- clocks: the input clock for device.
> +- clock-name: must be "pwrseq-clk"

The "-clk" in the clock name is redundant.

> +- pwrseq-reset-gpios: Should specify the GPIO for reset.
> +- pwrseq-reset-duration-us: the duration in microsecond for assert reset signal.

I understand you want to make it explicit that this GPIO is for the
pwrseq library, but are we really gaining anything over just calling
these reset-gpios and reset-duration-us?
The same applies to the clock name above.

> +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>;
> +	hub: genesys at 1 {
> +		compatible = "usb5e3,608";
> +		reg = <1>;
> +
> +		power-sequence;
> +		clocks = <&clks IMX6SX_CLK_CKO>;
> +		clock-names = "pwrseq-clk";
> +		pwrseq-reset-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>; /* hub reset pin */
> +		pwrseq-reset-duration-us = <10>;
> +
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		ethernet: asix at 1 {
> +			compatible = "usbb95,1708";
> +			reg = <1>;
> +
> +			power-sequence;
> +			clocks = <&clks IMX6SX_CLK_IPG>;
> +			clock-names = "pwrseq-clk";
> +			pwrseq-reset-gpios = <&gpio4 6 GPIO_ACTIVE_LOW>; /* ethernet_rst */
> +			pwrseq-reset-duration-us = <15>;
> +		};

This looks weird. The hub and ethernet chips don't have "pwrseq" clock
and reset input pins. I'd remove the clock-names and pwrseq- reset
prefix.

regards
Philipp

Re: [PATCH 5/6] usb: chipidea: host: let the hcd know's parent device node

[Stephen Boyd]

Quoting Peter Chen (2016-07-07 02:14:51)
> diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
> index 053bac9..55120ef 100644
> --- a/drivers/usb/chipidea/host.c
> +++ b/drivers/usb/chipidea/host.c
> @@ -109,15 +109,25 @@ static int host_start(struct ci_hdrc *ci)
>         struct ehci_hcd *ehci;
>         struct ehci_ci_priv *priv;
>         int ret;
> +       struct device *dev = ci->dev;
>  
> -       if (usb_disabled())
> +       if (usb_disabled() || !dev)

Does that ever happen?

>                 return -ENODEV;
>  
> -       hcd = usb_create_hcd(&ci_ehci_hc_driver, ci->dev, dev_name(ci->dev));
> +       /*
> +        * USB Core will try to get child node under roothub,
> +        * but chipidea core has no of_node, and the child node
> +        * for controller is located at glue layer's node which
> +        * is chipidea core's parent.
> +        */
> +       if (dev->parent && dev->parent->of_node)
> +               dev->of_node = dev->parent->of_node;

Why not do this during the ci device probe? How is host special for
having an of_node for the child device.

Doing that would also make my ULPI bus DT discovery patch simpler
because we wouldn't have to search the parent of the parent for a DT
node to find the ULPI node that's a child of the ci glue device.

[PATCH 5/6] usb: chipidea: host: let the hcd know's parent device node

[Stephen Boyd]

Quoting Peter Chen (2016-07-07 02:14:51)
> diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
> index 053bac9..55120ef 100644
> --- a/drivers/usb/chipidea/host.c
> +++ b/drivers/usb/chipidea/host.c
> @@ -109,15 +109,25 @@ static int host_start(struct ci_hdrc *ci)
>         struct ehci_hcd *ehci;
>         struct ehci_ci_priv *priv;
>         int ret;
> +       struct device *dev = ci->dev;
>  
> -       if (usb_disabled())
> +       if (usb_disabled() || !dev)

Does that ever happen?

>                 return -ENODEV;
>  
> -       hcd = usb_create_hcd(&ci_ehci_hc_driver, ci->dev, dev_name(ci->dev));
> +       /*
> +        * USB Core will try to get child node under roothub,
> +        * but chipidea core has no of_node, and the child node
> +        * for controller is located at glue layer's node which
> +        * is chipidea core's parent.
> +        */
> +       if (dev->parent && dev->parent->of_node)
> +               dev->of_node = dev->parent->of_node;

Why not do this during the ci device probe? How is host special for
having an of_node for the child device.

Doing that would also make my ULPI bus DT discovery patch simpler
because we wouldn't have to search the parent of the parent for a DT
node to find the ULPI node that's a child of the ci glue device.

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

[Joshua Clayton]

On 07/07/2016 02:47 AM, Philipp Zabel wrote:
> Am Donnerstag, den 07.07.2016, 17:14 +0800 schrieb Peter Chen:
>> Add binding doc for generic power sequence library.
>>
>> Signed-off-by: Peter Chen <peter.chen-3arQi8VN3Tc@public.gmane.org>
>> ---
>>  .../bindings/power/pwrseq/pwrseq-generic.txt       | 56 ++++++++++++++++++++++
>>  1 file changed, 56 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..4b23834
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
>> @@ -0,0 +1,56 @@
>> +The generic power sequence library
>> +
>> +Some hard-wired USB/MMC 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. The
>> +power sequence will be done before this device can be found at USB
>> +bus.
>> +
>> +The power sequence properties is under the device node.
>> +
>> +Required properties:
>> +- power-sequence: this device needs to do power sequence before enumeration
>> +
>> +Optional properties:
>> +- clocks: the input clock for device.
>> +- clock-name: must be "pwrseq-clk"
> The "-clk" in the clock name is redundant.
>
>> +- pwrseq-reset-gpios: Should specify the GPIO for reset.
>> +- pwrseq-reset-duration-us: the duration in microsecond for assert reset signal.
> I understand you want to make it explicit that this GPIO is for the
> pwrseq library, but are we really gaining anything over just calling
> these reset-gpios and reset-duration-us?
> The same applies to the clock name above.
using  reset-gpios makes sense to  me too.
The above "power-sequence" might then be better called "reset-on-init",
But really, if a device has a reset gpio shouldn't the default behavior be to
reset it on boot and when coming back from sleep?
Is a special property even needed?
>
>> +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>;
>> +	hub: genesys@1 {
>> +		compatible = "usb5e3,608";
>> +		reg = <1>;
>> +
>> +		power-sequence;
>> +		clocks = <&clks IMX6SX_CLK_CKO>;
>> +		clock-names = "pwrseq-clk";
>> +		pwrseq-reset-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>; /* hub reset pin */
>> +		pwrseq-reset-duration-us = <10>;
>> +
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +		ethernet: asix@1 {
>> +			compatible = "usbb95,1708";
>> +			reg = <1>;
>> +
>> +			power-sequence;
>> +			clocks = <&clks IMX6SX_CLK_IPG>;
>> +			clock-names = "pwrseq-clk";
>> +			pwrseq-reset-gpios = <&gpio4 6 GPIO_ACTIVE_LOW>; /* ethernet_rst */
>> +			pwrseq-reset-duration-us = <15>;
>> +		};
> This looks weird. The hub and ethernet chips don't have "pwrseq" clock
> and reset input pins. I'd remove the clock-names and pwrseq- reset
> prefix.
>
> regards
> Philipp
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Joshua Clayton]

On 07/07/2016 02:47 AM, Philipp Zabel wrote:
> Am Donnerstag, den 07.07.2016, 17:14 +0800 schrieb Peter Chen:
>> Add binding doc for generic power sequence library.
>>
>> Signed-off-by: Peter Chen <peter.chen@nxp.com>
>> ---
>>  .../bindings/power/pwrseq/pwrseq-generic.txt       | 56 ++++++++++++++++++++++
>>  1 file changed, 56 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..4b23834
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
>> @@ -0,0 +1,56 @@
>> +The generic power sequence library
>> +
>> +Some hard-wired USB/MMC 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. The
>> +power sequence will be done before this device can be found at USB
>> +bus.
>> +
>> +The power sequence properties is under the device node.
>> +
>> +Required properties:
>> +- power-sequence: this device needs to do power sequence before enumeration
>> +
>> +Optional properties:
>> +- clocks: the input clock for device.
>> +- clock-name: must be "pwrseq-clk"
> The "-clk" in the clock name is redundant.
>
>> +- pwrseq-reset-gpios: Should specify the GPIO for reset.
>> +- pwrseq-reset-duration-us: the duration in microsecond for assert reset signal.
> I understand you want to make it explicit that this GPIO is for the
> pwrseq library, but are we really gaining anything over just calling
> these reset-gpios and reset-duration-us?
> The same applies to the clock name above.
using  reset-gpios makes sense to  me too.
The above "power-sequence" might then be better called "reset-on-init",
But really, if a device has a reset gpio shouldn't the default behavior be to
reset it on boot and when coming back from sleep?
Is a special property even needed?
>
>> +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>;
>> +	hub: genesys at 1 {
>> +		compatible = "usb5e3,608";
>> +		reg = <1>;
>> +
>> +		power-sequence;
>> +		clocks = <&clks IMX6SX_CLK_CKO>;
>> +		clock-names = "pwrseq-clk";
>> +		pwrseq-reset-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>; /* hub reset pin */
>> +		pwrseq-reset-duration-us = <10>;
>> +
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +		ethernet: asix at 1 {
>> +			compatible = "usbb95,1708";
>> +			reg = <1>;
>> +
>> +			power-sequence;
>> +			clocks = <&clks IMX6SX_CLK_IPG>;
>> +			clock-names = "pwrseq-clk";
>> +			pwrseq-reset-gpios = <&gpio4 6 GPIO_ACTIVE_LOW>; /* ethernet_rst */
>> +			pwrseq-reset-duration-us = <15>;
>> +		};
> This looks weird. The hub and ethernet chips don't have "pwrseq" clock
> and reset input pins. I'd remove the clock-names and pwrseq- reset
> prefix.
>
> regards
> Philipp
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 5/6] usb: chipidea: host: let the hcd know's parent device node

[Peter Chen]

On Thu, Jul 07, 2016 at 03:56:51PM -0700, Stephen Boyd wrote:
> Quoting Peter Chen (2016-07-07 02:14:51)
> > diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
> > index 053bac9..55120ef 100644
> > --- a/drivers/usb/chipidea/host.c
> > +++ b/drivers/usb/chipidea/host.c
> > @@ -109,15 +109,25 @@ static int host_start(struct ci_hdrc *ci)
> >         struct ehci_hcd *ehci;
> >         struct ehci_ci_priv *priv;
> >         int ret;
> > +       struct device *dev = ci->dev;
> >  
> > -       if (usb_disabled())
> > +       if (usb_disabled() || !dev)
> 
> Does that ever happen?

I don't think so, will delete it.

> 
> >                 return -ENODEV;
> >  
> > -       hcd = usb_create_hcd(&ci_ehci_hc_driver, ci->dev, dev_name(ci->dev));
> > +       /*
> > +        * USB Core will try to get child node under roothub,
> > +        * but chipidea core has no of_node, and the child node
> > +        * for controller is located at glue layer's node which
> > +        * is chipidea core's parent.
> > +        */
> > +       if (dev->parent && dev->parent->of_node)
> > +               dev->of_node = dev->parent->of_node;
> 
> Why not do this during the ci device probe? How is host special for
> having an of_node for the child device.
> 

Good idea. I did this when I added USB device DT support, but did not
consider potential gadget/phy use cases.

-- 

Best Regards,
Peter Chen

[PATCH 5/6] usb: chipidea: host: let the hcd know's parent device node

[Peter Chen]

On Thu, Jul 07, 2016 at 03:56:51PM -0700, Stephen Boyd wrote:
> Quoting Peter Chen (2016-07-07 02:14:51)
> > diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
> > index 053bac9..55120ef 100644
> > --- a/drivers/usb/chipidea/host.c
> > +++ b/drivers/usb/chipidea/host.c
> > @@ -109,15 +109,25 @@ static int host_start(struct ci_hdrc *ci)
> >         struct ehci_hcd *ehci;
> >         struct ehci_ci_priv *priv;
> >         int ret;
> > +       struct device *dev = ci->dev;
> >  
> > -       if (usb_disabled())
> > +       if (usb_disabled() || !dev)
> 
> Does that ever happen?

I don't think so, will delete it.

> 
> >                 return -ENODEV;
> >  
> > -       hcd = usb_create_hcd(&ci_ehci_hc_driver, ci->dev, dev_name(ci->dev));
> > +       /*
> > +        * USB Core will try to get child node under roothub,
> > +        * but chipidea core has no of_node, and the child node
> > +        * for controller is located at glue layer's node which
> > +        * is chipidea core's parent.
> > +        */
> > +       if (dev->parent && dev->parent->of_node)
> > +               dev->of_node = dev->parent->of_node;
> 
> Why not do this during the ci device probe? How is host special for
> having an of_node for the child device.
> 

Good idea. I did this when I added USB device DT support, but did not
consider potential gadget/phy use cases.

-- 

Best Regards,
Peter Chen

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

[Peter Chen]

On Thu, Jul 07, 2016 at 11:47:14AM +0200, Philipp Zabel wrote:
> Am Donnerstag, den 07.07.2016, 17:14 +0800 schrieb Peter Chen:
> > Add binding doc for generic power sequence library.
> > 
> > Signed-off-by: Peter Chen <peter.chen@nxp.com>
> > ---
> >  .../bindings/power/pwrseq/pwrseq-generic.txt       | 56 ++++++++++++++++++++++
> >  1 file changed, 56 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..4b23834
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
> > @@ -0,0 +1,56 @@
> > +The generic power sequence library
> > +
> > +Some hard-wired USB/MMC 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. The
> > +power sequence will be done before this device can be found at USB
> > +bus.
> > +
> > +The power sequence properties is under the device node.
> > +
> > +Required properties:
> > +- power-sequence: this device needs to do power sequence before enumeration
> > +
> > +Optional properties:
> > +- clocks: the input clock for device.
> > +- clock-name: must be "pwrseq-clk"
> 
> The "-clk" in the clock name is redundant.
> 
> > +- pwrseq-reset-gpios: Should specify the GPIO for reset.
> > +- pwrseq-reset-duration-us: the duration in microsecond for assert reset signal.
> 
> I understand you want to make it explicit that this GPIO is for the
> pwrseq library, but are we really gaining anything over just calling
> these reset-gpios and reset-duration-us?
> The same applies to the clock name above.
> 
> > +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>;
> > +	hub: genesys@1 {
> > +		compatible = "usb5e3,608";
> > +		reg = <1>;
> > +
> > +		power-sequence;
> > +		clocks = <&clks IMX6SX_CLK_CKO>;
> > +		clock-names = "pwrseq-clk";
> > +		pwrseq-reset-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>; /* hub reset pin */
> > +		pwrseq-reset-duration-us = <10>;
> > +
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +		ethernet: asix@1 {
> > +			compatible = "usbb95,1708";
> > +			reg = <1>;
> > +
> > +			power-sequence;
> > +			clocks = <&clks IMX6SX_CLK_IPG>;
> > +			clock-names = "pwrseq-clk";
> > +			pwrseq-reset-gpios = <&gpio4 6 GPIO_ACTIVE_LOW>; /* ethernet_rst */
> > +			pwrseq-reset-duration-us = <15>;
> > +		};
> 
> This looks weird. The hub and ethernet chips don't have "pwrseq" clock
> and reset input pins. I'd remove the clock-names and pwrseq- reset
> prefix.
> 

I agree with you, and will keep property name like last version [1].

-- 

Best Regards,
Peter Chen

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

[Peter Chen]

On Thu, Jul 07, 2016 at 11:47:14AM +0200, Philipp Zabel wrote:
> Am Donnerstag, den 07.07.2016, 17:14 +0800 schrieb Peter Chen:
> > Add binding doc for generic power sequence library.
> > 
> > Signed-off-by: Peter Chen <peter.chen@nxp.com>
> > ---
> >  .../bindings/power/pwrseq/pwrseq-generic.txt       | 56 ++++++++++++++++++++++
> >  1 file changed, 56 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..4b23834
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
> > @@ -0,0 +1,56 @@
> > +The generic power sequence library
> > +
> > +Some hard-wired USB/MMC 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. The
> > +power sequence will be done before this device can be found at USB
> > +bus.
> > +
> > +The power sequence properties is under the device node.
> > +
> > +Required properties:
> > +- power-sequence: this device needs to do power sequence before enumeration
> > +
> > +Optional properties:
> > +- clocks: the input clock for device.
> > +- clock-name: must be "pwrseq-clk"
> 
> The "-clk" in the clock name is redundant.
> 
> > +- pwrseq-reset-gpios: Should specify the GPIO for reset.
> > +- pwrseq-reset-duration-us: the duration in microsecond for assert reset signal.
> 
> I understand you want to make it explicit that this GPIO is for the
> pwrseq library, but are we really gaining anything over just calling
> these reset-gpios and reset-duration-us?
> The same applies to the clock name above.
> 
> > +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>;
> > +	hub: genesys at 1 {
> > +		compatible = "usb5e3,608";
> > +		reg = <1>;
> > +
> > +		power-sequence;
> > +		clocks = <&clks IMX6SX_CLK_CKO>;
> > +		clock-names = "pwrseq-clk";
> > +		pwrseq-reset-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>; /* hub reset pin */
> > +		pwrseq-reset-duration-us = <10>;
> > +
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +		ethernet: asix at 1 {
> > +			compatible = "usbb95,1708";
> > +			reg = <1>;
> > +
> > +			power-sequence;
> > +			clocks = <&clks IMX6SX_CLK_IPG>;
> > +			clock-names = "pwrseq-clk";
> > +			pwrseq-reset-gpios = <&gpio4 6 GPIO_ACTIVE_LOW>; /* ethernet_rst */
> > +			pwrseq-reset-duration-us = <15>;
> > +		};
> 
> This looks weird. The hub and ethernet chips don't have "pwrseq" clock
> and reset input pins. I'd remove the clock-names and pwrseq- reset
> prefix.
> 

I agree with you, and will keep property name like last version [1].

-- 

Best Regards,
Peter Chen

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

[Peter Chen]

On Thu, Jul 07, 2016 at 04:53:07PM -0700, Joshua Clayton wrote:
> 
> 
> On 07/07/2016 02:47 AM, Philipp Zabel wrote:
> > Am Donnerstag, den 07.07.2016, 17:14 +0800 schrieb Peter Chen:
> >> Add binding doc for generic power sequence library.
> >>
> >> Signed-off-by: Peter Chen <peter.chen@nxp.com>
> >> ---
> >>  .../bindings/power/pwrseq/pwrseq-generic.txt       | 56 ++++++++++++++++++++++
> >>  1 file changed, 56 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..4b23834
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
> >> @@ -0,0 +1,56 @@
> >> +The generic power sequence library
> >> +
> >> +Some hard-wired USB/MMC 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. The
> >> +power sequence will be done before this device can be found at USB
> >> +bus.
> >> +
> >> +The power sequence properties is under the device node.
> >> +
> >> +Required properties:
> >> +- power-sequence: this device needs to do power sequence before enumeration
> >> +
> >> +Optional properties:
> >> +- clocks: the input clock for device.
> >> +- clock-name: must be "pwrseq-clk"
> > The "-clk" in the clock name is redundant.
> >
> >> +- pwrseq-reset-gpios: Should specify the GPIO for reset.
> >> +- pwrseq-reset-duration-us: the duration in microsecond for assert reset signal.
> > I understand you want to make it explicit that this GPIO is for the
> > pwrseq library, but are we really gaining anything over just calling
> > these reset-gpios and reset-duration-us?
> > The same applies to the clock name above.
> using  reset-gpios makes sense to  me too.
> The above "power-sequence" might then be better called "reset-on-init",
> But really, if a device has a reset gpio shouldn't the default behavior be to
> reset it on boot and when coming back from sleep?
> Is a special property even needed?

The power sequence does not include reset process only, it may include clock
and power as well.

-- 

Best Regards,
Peter Chen

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

[Peter Chen]

On Thu, Jul 07, 2016 at 04:53:07PM -0700, Joshua Clayton wrote:
> 
> 
> On 07/07/2016 02:47 AM, Philipp Zabel wrote:
> > Am Donnerstag, den 07.07.2016, 17:14 +0800 schrieb Peter Chen:
> >> Add binding doc for generic power sequence library.
> >>
> >> Signed-off-by: Peter Chen <peter.chen@nxp.com>
> >> ---
> >>  .../bindings/power/pwrseq/pwrseq-generic.txt       | 56 ++++++++++++++++++++++
> >>  1 file changed, 56 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..4b23834
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
> >> @@ -0,0 +1,56 @@
> >> +The generic power sequence library
> >> +
> >> +Some hard-wired USB/MMC 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. The
> >> +power sequence will be done before this device can be found at USB
> >> +bus.
> >> +
> >> +The power sequence properties is under the device node.
> >> +
> >> +Required properties:
> >> +- power-sequence: this device needs to do power sequence before enumeration
> >> +
> >> +Optional properties:
> >> +- clocks: the input clock for device.
> >> +- clock-name: must be "pwrseq-clk"
> > The "-clk" in the clock name is redundant.
> >
> >> +- pwrseq-reset-gpios: Should specify the GPIO for reset.
> >> +- pwrseq-reset-duration-us: the duration in microsecond for assert reset signal.
> > I understand you want to make it explicit that this GPIO is for the
> > pwrseq library, but are we really gaining anything over just calling
> > these reset-gpios and reset-duration-us?
> > The same applies to the clock name above.
> using  reset-gpios makes sense to  me too.
> The above "power-sequence" might then be better called "reset-on-init",
> But really, if a device has a reset gpio shouldn't the default behavior be to
> reset it on boot and when coming back from sleep?
> Is a special property even needed?

The power sequence does not include reset process only, it may include clock
and power as well.

-- 

Best Regards,
Peter Chen