[PATCH 00/12] power: add generic power sequence framework

[PATCH 00/12] power: add generic power sequence framework

[Peter Chen]

Hi all,

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
framework to handle this requirement. The generic code is supplied
some common helpers from host driver, and individual power sequence
driver handles kinds of power sequence for devices.

Since the MMC has already done the similar things, and this power
sequence handling can be generic, we use mmc power sequence code
as base to create this framework.

This patch set is based on Krzysztof Kozlowski's RFC patch set (v4.7-rc1)
[1], and making some changes which can let it be generic. After that,
we create a generic power sequence driver for USB devices which handles
below things, it includes all input signals for devices I can consider.

- Clock and its frequencies
- GPIO for reset and the duration time
- GPIO for enable
- Regulator for power

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 udoo board changes
were tested using my last power sequence patch set. [2]

@Maciej S. Szmigiero, Oscar, would you please test them on imx6qdl udoo
boards?
@Krzysztof Kozlowski, I have not written changes for your case since I
find the other sequences have already at drivers/usb/misc/usb3503.c,
this driver should be the place for your power sequence, I am not
sure if enable regualator at usb core is suitable for you or not.
@others, it is better you can help test this patch set for MMC devices
and your USB devices with dts changes.

Thank you for your time!

[1] http://www.spinics.net/lists/linux-usb/msg142351.html
[2] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-March/413580.html

Krzysztof Kozlowski (5):
  power/mmc: Move pwrseq drivers to power/pwrseq
  MAINTAINERS: Retain Ulf Hansson as the same maintainer of pwrseq
  power: pwrseq: Enable COMPILE_TEST for drivers
  power: pwrseq: Remove mmc prefix from mmc_pwrseq
  power: pwrseq: Generalize mmc_pwrseq operations by removing mmc prefix

Peter Chen (7):
  power: pwrseq: change common helpers as generic
  power: pwrseq: rename file name for generic driver
  doc: binding: pwrseq-usb-generic: add binding doc for generic usb
    power sequence driver
  power: pwrseq: pwrseq_usb_generic: add generic power sequence support
    for USB deivces
  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

 .../{mmc => power/pwrseq}/mmc-pwrseq-simple.txt    |   0
 .../pwrseq/pwrseq-emmc.txt}                        |   0
 .../bindings/power/pwrseq/pwrseq-usb-generic.txt   |  31 ++++
 .../devicetree/bindings/usb/usb-device.txt         |   2 +
 MAINTAINERS                                        |   9 +
 arch/arm/boot/dts/imx6qdl-udoo.dtsi                |  30 +--
 drivers/mmc/Kconfig                                |   2 -
 drivers/mmc/core/Kconfig                           |  24 ---
 drivers/mmc/core/Makefile                          |   3 -
 drivers/mmc/core/core.c                            |   8 +-
 drivers/mmc/core/host.c                            |  10 +-
 drivers/mmc/core/pwrseq.h                          |  52 ------
 drivers/power/Kconfig                              |   1 +
 drivers/power/Makefile                             |   1 +
 drivers/power/pwrseq/Kconfig                       |  43 +++++
 drivers/power/pwrseq/Makefile                      |   4 +
 drivers/{mmc/core/pwrseq.c => power/pwrseq/core.c} |  71 ++++----
 drivers/{mmc/core => power/pwrseq}/pwrseq_emmc.c   |  17 +-
 .../pwrseq/pwrseq_mmc_simple.c}                    |  33 ++--
 drivers/power/pwrseq/pwrseq_usb_generic.c          | 202 +++++++++++++++++++++
 drivers/usb/chipidea/host.c                        |  18 +-
 drivers/usb/core/hub.c                             |  73 +++++++-
 drivers/usb/core/hub.h                             |   1 +
 include/linux/mmc/host.h                           |   4 +-
 include/linux/pwrseq.h                             |  61 +++++++
 25 files changed, 527 insertions(+), 173 deletions(-)
 rename Documentation/devicetree/bindings/{mmc => power/pwrseq}/mmc-pwrseq-simple.txt (100%)
 rename Documentation/devicetree/bindings/{mmc/mmc-pwrseq-emmc.txt => power/pwrseq/pwrseq-emmc.txt} (100%)
 create mode 100644 Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt
 delete mode 100644 drivers/mmc/core/Kconfig
 delete mode 100644 drivers/mmc/core/pwrseq.h
 create mode 100644 drivers/power/pwrseq/Kconfig
 create mode 100644 drivers/power/pwrseq/Makefile
 rename drivers/{mmc/core/pwrseq.c => power/pwrseq/core.c} (51%)
 rename drivers/{mmc/core => power/pwrseq}/pwrseq_emmc.c (88%)
 rename drivers/{mmc/core/pwrseq_simple.c => power/pwrseq/pwrseq_mmc_simple.c} (81%)
 create mode 100644 drivers/power/pwrseq/pwrseq_usb_generic.c
 create mode 100644 include/linux/pwrseq.h

-- 
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 01/12] power/mmc: Move pwrseq drivers to power/pwrseq

[Peter Chen]

From: Krzysztof Kozlowski <k.kozlowski@samsung.com>

The MMC power sequence drivers are useful also outside of MMC world: for
USB devices needed a hard-reset before probing. Before extending and
re-using pwrseq drivers, move them to a new place.

The commit does not introduce significant changes in the pwrseq drivers
code so still all the functions are prefixed with "mmc_pwrseq". However
the MMC-specific pwrseq functions has to be now exported and everything
is hidden not by CONFIG_OF but by new CONFIG_POWER_SEQ option.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
---
 .../pwrseq/pwrseq-emmc.txt}                            |  0
 .../pwrseq/pwrseq-simple.txt}                          |  0
 drivers/mmc/Kconfig                                    |  2 --
 drivers/mmc/core/Makefile                              |  3 ---
 drivers/mmc/core/core.c                                |  2 +-
 drivers/mmc/core/host.c                                |  2 +-
 drivers/power/Kconfig                                  |  1 +
 drivers/power/Makefile                                 |  1 +
 drivers/{mmc/core => power/pwrseq}/Kconfig             | 18 +++++++++++++-----
 drivers/power/pwrseq/Makefile                          |  3 +++
 drivers/{mmc/core => power/pwrseq}/pwrseq.c            |  8 ++++++--
 drivers/{mmc/core => power/pwrseq}/pwrseq_emmc.c       |  3 +--
 drivers/{mmc/core => power/pwrseq}/pwrseq_simple.c     |  3 +--
 {drivers/mmc/core => include/linux}/pwrseq.h           |  6 +++---
 14 files changed, 31 insertions(+), 21 deletions(-)
 rename Documentation/devicetree/bindings/{mmc/mmc-pwrseq-emmc.txt => power/pwrseq/pwrseq-emmc.txt} (100%)
 rename Documentation/devicetree/bindings/{mmc/mmc-pwrseq-simple.txt => power/pwrseq/pwrseq-simple.txt} (100%)
 rename drivers/{mmc/core => power/pwrseq}/Kconfig (66%)
 create mode 100644 drivers/power/pwrseq/Makefile
 rename drivers/{mmc/core => power/pwrseq}/pwrseq.c (90%)
 rename drivers/{mmc/core => power/pwrseq}/pwrseq_emmc.c (99%)
 rename drivers/{mmc/core => power/pwrseq}/pwrseq_simple.c (99%)
 rename {drivers/mmc/core => include/linux}/pwrseq.h (94%)

diff --git a/Documentation/devicetree/bindings/mmc/mmc-pwrseq-emmc.txt b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-emmc.txt
similarity index 100%
rename from Documentation/devicetree/bindings/mmc/mmc-pwrseq-emmc.txt
rename to Documentation/devicetree/bindings/power/pwrseq/pwrseq-emmc.txt
diff --git a/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-simple.txt
similarity index 100%
rename from Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
rename to Documentation/devicetree/bindings/power/pwrseq/pwrseq-simple.txt
diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
index f2eeb38..7ade379 100644
--- a/drivers/mmc/Kconfig
+++ b/drivers/mmc/Kconfig
@@ -21,8 +21,6 @@ config MMC_DEBUG
 
 if MMC
 
-source "drivers/mmc/core/Kconfig"
-
 source "drivers/mmc/card/Kconfig"
 
 source "drivers/mmc/host/Kconfig"
diff --git a/drivers/mmc/core/Makefile b/drivers/mmc/core/Makefile
index f007151..a901d3c 100644
--- a/drivers/mmc/core/Makefile
+++ b/drivers/mmc/core/Makefile
@@ -8,7 +8,4 @@ mmc_core-y			:= core.o bus.o host.o \
 				   sdio.o sdio_ops.o sdio_bus.o \
 				   sdio_cis.o sdio_io.o sdio_irq.o \
 				   quirks.o slot-gpio.o
-mmc_core-$(CONFIG_OF)		+= pwrseq.o
-obj-$(CONFIG_PWRSEQ_SIMPLE)	+= pwrseq_simple.o
-obj-$(CONFIG_PWRSEQ_EMMC)	+= pwrseq_emmc.o
 mmc_core-$(CONFIG_DEBUG_FS)	+= debugfs.o
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 8b4dfd4..edefc3f 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -25,6 +25,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/pm_wakeup.h>
 #include <linux/suspend.h>
+#include <linux/pwrseq.h>
 #include <linux/fault-inject.h>
 #include <linux/random.h>
 #include <linux/slab.h>
@@ -43,7 +44,6 @@
 #include "bus.h"
 #include "host.h"
 #include "sdio_bus.h"
-#include "pwrseq.h"
 
 #include "mmc_ops.h"
 #include "sd_ops.h"
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 1be42fa..1db7d58 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -21,6 +21,7 @@
 #include <linux/export.h>
 #include <linux/leds.h>
 #include <linux/slab.h>
+#include <linux/pwrseq.h>
 
 #include <linux/mmc/host.h>
 #include <linux/mmc/card.h>
@@ -29,7 +30,6 @@
 #include "core.h"
 #include "host.h"
 #include "slot-gpio.h"
-#include "pwrseq.h"
 
 #define cls_dev_to_mmc_host(d)	container_of(d, struct mmc_host, class_dev)
 
diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 421770d..2702aca 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -511,5 +511,6 @@ config AXP20X_POWER
 
 endif # POWER_SUPPLY
 
+source "drivers/power/pwrseq/Kconfig"
 source "drivers/power/reset/Kconfig"
 source "drivers/power/avs/Kconfig"
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index e46b75d..02f9d5d 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -71,6 +71,7 @@ obj-$(CONFIG_POWER_AVS)		+= avs/
 obj-$(CONFIG_CHARGER_SMB347)	+= smb347-charger.o
 obj-$(CONFIG_CHARGER_TPS65090)	+= tps65090-charger.o
 obj-$(CONFIG_CHARGER_TPS65217)	+= tps65217_charger.o
+obj-$(CONFIG_POWER_SEQ)		+= pwrseq/
 obj-$(CONFIG_POWER_RESET)	+= reset/
 obj-$(CONFIG_AXP288_FUEL_GAUGE) += axp288_fuel_gauge.o
 obj-$(CONFIG_AXP288_CHARGER)	+= axp288_charger.o
diff --git a/drivers/mmc/core/Kconfig b/drivers/power/pwrseq/Kconfig
similarity index 66%
rename from drivers/mmc/core/Kconfig
rename to drivers/power/pwrseq/Kconfig
index 250f223..7ecd66a 100644
--- a/drivers/mmc/core/Kconfig
+++ b/drivers/power/pwrseq/Kconfig
@@ -1,7 +1,13 @@
-#
-# MMC core configuration
-#
-config PWRSEQ_EMMC
+menuconfig POWER_SEQ
+	default y if OF
+	bool "Hardware reset support for specific devices"
+	help
+	  Provides drivers that implements specific power sequences for chips,
+	  using the generic power sequence management interface.
+
+if POWER_SEQ
+
+config POWER_SEQ_EMMC
 	tristate "HW reset support for eMMC"
 	default y
 	depends on OF
@@ -12,7 +18,7 @@ config PWRSEQ_EMMC
 	  This driver can also be built as a module. If so, the module
 	  will be called pwrseq_emmc.
 
-config PWRSEQ_SIMPLE
+config POWER_SEQ_SIMPLE
 	tristate "Simple HW reset support for MMC"
 	default y
 	depends on OF
@@ -22,3 +28,5 @@ config PWRSEQ_SIMPLE
 
 	  This driver can also be built as a module. If so, the module
 	  will be called pwrseq_simple.
+
+endif
diff --git a/drivers/power/pwrseq/Makefile b/drivers/power/pwrseq/Makefile
new file mode 100644
index 0000000..9e40e4b
--- /dev/null
+++ b/drivers/power/pwrseq/Makefile
@@ -0,0 +1,3 @@
+obj-$(CONFIG_POWER_SEQ)		+= pwrseq.o
+obj-$(CONFIG_POWER_SEQ_SIMPLE)	+= pwrseq_simple.o
+obj-$(CONFIG_POWER_SEQ_EMMC)	+= pwrseq_emmc.o
diff --git a/drivers/mmc/core/pwrseq.c b/drivers/power/pwrseq/pwrseq.c
similarity index 90%
rename from drivers/mmc/core/pwrseq.c
rename to drivers/power/pwrseq/pwrseq.c
index 9386c47..66310d7 100644
--- a/drivers/mmc/core/pwrseq.c
+++ b/drivers/power/pwrseq/pwrseq.c
@@ -11,11 +11,10 @@
 #include <linux/err.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/pwrseq.h>
 
 #include <linux/mmc/host.h>
 
-#include "pwrseq.h"
-
 static DEFINE_MUTEX(pwrseq_list_mutex);
 static LIST_HEAD(pwrseq_list);
 
@@ -51,6 +50,7 @@ int mmc_pwrseq_alloc(struct mmc_host *host)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(mmc_pwrseq_alloc);
 
 void mmc_pwrseq_pre_power_on(struct mmc_host *host)
 {
@@ -59,6 +59,7 @@ void mmc_pwrseq_pre_power_on(struct mmc_host *host)
 	if (pwrseq && pwrseq->ops->pre_power_on)
 		pwrseq->ops->pre_power_on(host);
 }
+EXPORT_SYMBOL_GPL(mmc_pwrseq_pre_power_on);
 
 void mmc_pwrseq_post_power_on(struct mmc_host *host)
 {
@@ -67,6 +68,7 @@ void mmc_pwrseq_post_power_on(struct mmc_host *host)
 	if (pwrseq && pwrseq->ops->post_power_on)
 		pwrseq->ops->post_power_on(host);
 }
+EXPORT_SYMBOL_GPL(mmc_pwrseq_post_power_on);
 
 void mmc_pwrseq_power_off(struct mmc_host *host)
 {
@@ -75,6 +77,7 @@ void mmc_pwrseq_power_off(struct mmc_host *host)
 	if (pwrseq && pwrseq->ops->power_off)
 		pwrseq->ops->power_off(host);
 }
+EXPORT_SYMBOL_GPL(mmc_pwrseq_power_off);
 
 void mmc_pwrseq_free(struct mmc_host *host)
 {
@@ -85,6 +88,7 @@ void mmc_pwrseq_free(struct mmc_host *host)
 		host->pwrseq = NULL;
 	}
 }
+EXPORT_SYMBOL_GPL(mmc_pwrseq_free);
 
 int mmc_pwrseq_register(struct mmc_pwrseq *pwrseq)
 {
diff --git a/drivers/mmc/core/pwrseq_emmc.c b/drivers/power/pwrseq/pwrseq_emmc.c
similarity index 99%
rename from drivers/mmc/core/pwrseq_emmc.c
rename to drivers/power/pwrseq/pwrseq_emmc.c
index adc9c0c..a0583ed 100644
--- a/drivers/mmc/core/pwrseq_emmc.c
+++ b/drivers/power/pwrseq/pwrseq_emmc.c
@@ -17,11 +17,10 @@
 #include <linux/err.h>
 #include <linux/gpio/consumer.h>
 #include <linux/reboot.h>
+#include <linux/pwrseq.h>
 
 #include <linux/mmc/host.h>
 
-#include "pwrseq.h"
-
 struct mmc_pwrseq_emmc {
 	struct mmc_pwrseq pwrseq;
 	struct notifier_block reset_nb;
diff --git a/drivers/mmc/core/pwrseq_simple.c b/drivers/power/pwrseq/pwrseq_simple.c
similarity index 99%
rename from drivers/mmc/core/pwrseq_simple.c
rename to drivers/power/pwrseq/pwrseq_simple.c
index 450d907..786f1db 100644
--- a/drivers/mmc/core/pwrseq_simple.c
+++ b/drivers/power/pwrseq/pwrseq_simple.c
@@ -16,11 +16,10 @@
 #include <linux/device.h>
 #include <linux/err.h>
 #include <linux/gpio/consumer.h>
+#include <linux/pwrseq.h>
 
 #include <linux/mmc/host.h>
 
-#include "pwrseq.h"
-
 struct mmc_pwrseq_simple {
 	struct mmc_pwrseq pwrseq;
 	bool clk_enabled;
diff --git a/drivers/mmc/core/pwrseq.h b/include/linux/pwrseq.h
similarity index 94%
rename from drivers/mmc/core/pwrseq.h
rename to include/linux/pwrseq.h
index d69e751..e230670 100644
--- a/drivers/mmc/core/pwrseq.h
+++ b/include/linux/pwrseq.h
@@ -23,7 +23,7 @@ struct mmc_pwrseq {
 	struct module *owner;
 };
 
-#ifdef CONFIG_OF
+#ifdef CONFIG_POWER_SEQ
 
 int mmc_pwrseq_register(struct mmc_pwrseq *pwrseq);
 void mmc_pwrseq_unregister(struct mmc_pwrseq *pwrseq);
@@ -34,7 +34,7 @@ void mmc_pwrseq_post_power_on(struct mmc_host *host);
 void mmc_pwrseq_power_off(struct mmc_host *host);
 void mmc_pwrseq_free(struct mmc_host *host);
 
-#else
+#else /* CONFIG_POWER_SEQ */
 
 static inline int mmc_pwrseq_register(struct mmc_pwrseq *pwrseq)
 {
@@ -47,6 +47,6 @@ static inline void mmc_pwrseq_post_power_on(struct mmc_host *host) {}
 static inline void mmc_pwrseq_power_off(struct mmc_host *host) {}
 static inline void mmc_pwrseq_free(struct mmc_host *host) {}
 
-#endif
+#endif /* CONFIG_POWER_SEQ */
 
 #endif
-- 
1.9.1

[PATCH 02/12] MAINTAINERS: Retain Ulf Hansson as the same maintainer of pwrseq

[Peter Chen]

From: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Before moving pwrseq drivers from drivers/mmc/core/ to drivers/power/,
they were maintained by Ulf Hansson.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
---
 MAINTAINERS | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index ed42cb6..0bc1821 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9072,6 +9072,15 @@ F:	include/linux/power_supply.h
 F:	drivers/power/
 X:	drivers/power/avs/
 
+POWER SEQ CORE and DRIVERS
+M:	Ulf Hansson <ulf.hansson@linaro.org>
+L:	linux-mmc@vger.kernel.org
+T:	git git://git.linaro.org/people/ulf.hansson/mmc.git
+S:	Maintained
+F:	drivers/power/pwrseq/
+F:	include/linux/pwrseq.h
+F:	Documentation/devicetree/bindings/power/pwrseq/
+
 POWER STATE COORDINATION INTERFACE (PSCI)
 M:	Mark Rutland <mark.rutland@arm.com>
 M:	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
-- 
1.9.1

[PATCH 03/12] power: pwrseq: Enable COMPILE_TEST for drivers

[Peter Chen]

From: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Allow build testing for power sequence drivers.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
---
 drivers/power/pwrseq/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/power/pwrseq/Kconfig b/drivers/power/pwrseq/Kconfig
index 7ecd66a..c7e9271 100644
--- a/drivers/power/pwrseq/Kconfig
+++ b/drivers/power/pwrseq/Kconfig
@@ -10,7 +10,7 @@ if POWER_SEQ
 config POWER_SEQ_EMMC
 	tristate "HW reset support for eMMC"
 	default y
-	depends on OF
+	depends on OF || COMPILE_TEST
 	help
 	  This selects Hardware reset support aka pwrseq-emmc for eMMC
 	  devices. By default this option is set to y.
@@ -21,7 +21,7 @@ config POWER_SEQ_EMMC
 config POWER_SEQ_SIMPLE
 	tristate "Simple HW reset support for MMC"
 	default y
-	depends on OF
+	depends on OF || COMPILE_TEST
 	help
 	  This selects simple hardware reset support aka pwrseq-simple for MMC
 	  devices. By default this option is set to y.
-- 
1.9.1

[PATCH 04/12] power: pwrseq: Remove mmc prefix from mmc_pwrseq

[Peter Chen]

From: Krzysztof Kozlowski <k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

The "mmc" prefix is no longer needed after moving the pwrseq core code
from mmc/ to power/.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Reviewed-by: Javier Martinez Canillas <javier-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
---
 drivers/power/pwrseq/pwrseq.c        | 18 +++++++++---------
 drivers/power/pwrseq/pwrseq_emmc.c   |  8 ++++----
 drivers/power/pwrseq/pwrseq_simple.c |  8 ++++----
 include/linux/mmc/host.h             |  4 ++--
 include/linux/pwrseq.h               | 20 ++++++++++----------
 5 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/drivers/power/pwrseq/pwrseq.c b/drivers/power/pwrseq/pwrseq.c
index 66310d7..9c66582 100644
--- a/drivers/power/pwrseq/pwrseq.c
+++ b/drivers/power/pwrseq/pwrseq.c
@@ -21,7 +21,7 @@ static LIST_HEAD(pwrseq_list);
 int mmc_pwrseq_alloc(struct mmc_host *host)
 {
 	struct device_node *np;
-	struct mmc_pwrseq *p;
+	struct pwrseq *p;
 
 	np = of_parse_phandle(host->parent->of_node, "mmc-pwrseq", 0);
 	if (!np)
@@ -54,7 +54,7 @@ EXPORT_SYMBOL_GPL(mmc_pwrseq_alloc);
 
 void mmc_pwrseq_pre_power_on(struct mmc_host *host)
 {
-	struct mmc_pwrseq *pwrseq = host->pwrseq;
+	struct pwrseq *pwrseq = host->pwrseq;
 
 	if (pwrseq && pwrseq->ops->pre_power_on)
 		pwrseq->ops->pre_power_on(host);
@@ -63,7 +63,7 @@ EXPORT_SYMBOL_GPL(mmc_pwrseq_pre_power_on);
 
 void mmc_pwrseq_post_power_on(struct mmc_host *host)
 {
-	struct mmc_pwrseq *pwrseq = host->pwrseq;
+	struct pwrseq *pwrseq = host->pwrseq;
 
 	if (pwrseq && pwrseq->ops->post_power_on)
 		pwrseq->ops->post_power_on(host);
@@ -72,7 +72,7 @@ EXPORT_SYMBOL_GPL(mmc_pwrseq_post_power_on);
 
 void mmc_pwrseq_power_off(struct mmc_host *host)
 {
-	struct mmc_pwrseq *pwrseq = host->pwrseq;
+	struct pwrseq *pwrseq = host->pwrseq;
 
 	if (pwrseq && pwrseq->ops->power_off)
 		pwrseq->ops->power_off(host);
@@ -81,7 +81,7 @@ EXPORT_SYMBOL_GPL(mmc_pwrseq_power_off);
 
 void mmc_pwrseq_free(struct mmc_host *host)
 {
-	struct mmc_pwrseq *pwrseq = host->pwrseq;
+	struct pwrseq *pwrseq = host->pwrseq;
 
 	if (pwrseq) {
 		module_put(pwrseq->owner);
@@ -90,7 +90,7 @@ void mmc_pwrseq_free(struct mmc_host *host)
 }
 EXPORT_SYMBOL_GPL(mmc_pwrseq_free);
 
-int mmc_pwrseq_register(struct mmc_pwrseq *pwrseq)
+int pwrseq_register(struct pwrseq *pwrseq)
 {
 	if (!pwrseq || !pwrseq->ops || !pwrseq->dev)
 		return -EINVAL;
@@ -101,9 +101,9 @@ int mmc_pwrseq_register(struct mmc_pwrseq *pwrseq)
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(mmc_pwrseq_register);
+EXPORT_SYMBOL_GPL(pwrseq_register);
 
-void mmc_pwrseq_unregister(struct mmc_pwrseq *pwrseq)
+void pwrseq_unregister(struct pwrseq *pwrseq)
 {
 	if (pwrseq) {
 		mutex_lock(&pwrseq_list_mutex);
@@ -111,4 +111,4 @@ void mmc_pwrseq_unregister(struct mmc_pwrseq *pwrseq)
 		mutex_unlock(&pwrseq_list_mutex);
 	}
 }
-EXPORT_SYMBOL_GPL(mmc_pwrseq_unregister);
+EXPORT_SYMBOL_GPL(pwrseq_unregister);
diff --git a/drivers/power/pwrseq/pwrseq_emmc.c b/drivers/power/pwrseq/pwrseq_emmc.c
index a0583ed..a68ac9a 100644
--- a/drivers/power/pwrseq/pwrseq_emmc.c
+++ b/drivers/power/pwrseq/pwrseq_emmc.c
@@ -22,7 +22,7 @@
 #include <linux/mmc/host.h>
 
 struct mmc_pwrseq_emmc {
-	struct mmc_pwrseq pwrseq;
+	struct pwrseq pwrseq;
 	struct notifier_block reset_nb;
 	struct gpio_desc *reset_gpio;
 };
@@ -54,7 +54,7 @@ static int mmc_pwrseq_emmc_reset_nb(struct notifier_block *this,
 	return NOTIFY_DONE;
 }
 
-static const struct mmc_pwrseq_ops mmc_pwrseq_emmc_ops = {
+static const struct pwrseq_ops mmc_pwrseq_emmc_ops = {
 	.post_power_on = mmc_pwrseq_emmc_reset,
 };
 
@@ -85,7 +85,7 @@ static int mmc_pwrseq_emmc_probe(struct platform_device *pdev)
 	pwrseq->pwrseq.owner = THIS_MODULE;
 	platform_set_drvdata(pdev, pwrseq);
 
-	return mmc_pwrseq_register(&pwrseq->pwrseq);
+	return pwrseq_register(&pwrseq->pwrseq);
 }
 
 static int mmc_pwrseq_emmc_remove(struct platform_device *pdev)
@@ -93,7 +93,7 @@ static int mmc_pwrseq_emmc_remove(struct platform_device *pdev)
 	struct mmc_pwrseq_emmc *pwrseq = platform_get_drvdata(pdev);
 
 	unregister_restart_handler(&pwrseq->reset_nb);
-	mmc_pwrseq_unregister(&pwrseq->pwrseq);
+	pwrseq_unregister(&pwrseq->pwrseq);
 
 	return 0;
 }
diff --git a/drivers/power/pwrseq/pwrseq_simple.c b/drivers/power/pwrseq/pwrseq_simple.c
index 786f1db..d5fbd65 100644
--- a/drivers/power/pwrseq/pwrseq_simple.c
+++ b/drivers/power/pwrseq/pwrseq_simple.c
@@ -21,7 +21,7 @@
 #include <linux/mmc/host.h>
 
 struct mmc_pwrseq_simple {
-	struct mmc_pwrseq pwrseq;
+	struct pwrseq pwrseq;
 	bool clk_enabled;
 	struct clk *ext_clk;
 	struct gpio_descs *reset_gpios;
@@ -77,7 +77,7 @@ static void mmc_pwrseq_simple_power_off(struct mmc_host *host)
 	}
 }
 
-static const struct mmc_pwrseq_ops mmc_pwrseq_simple_ops = {
+static const struct pwrseq_ops mmc_pwrseq_simple_ops = {
 	.pre_power_on = mmc_pwrseq_simple_pre_power_on,
 	.post_power_on = mmc_pwrseq_simple_post_power_on,
 	.power_off = mmc_pwrseq_simple_power_off,
@@ -115,14 +115,14 @@ static int mmc_pwrseq_simple_probe(struct platform_device *pdev)
 	pwrseq->pwrseq.owner = THIS_MODULE;
 	platform_set_drvdata(pdev, pwrseq);
 
-	return mmc_pwrseq_register(&pwrseq->pwrseq);
+	return pwrseq_register(&pwrseq->pwrseq);
 }
 
 static int mmc_pwrseq_simple_remove(struct platform_device *pdev)
 {
 	struct mmc_pwrseq_simple *pwrseq = platform_get_drvdata(pdev);
 
-	mmc_pwrseq_unregister(&pwrseq->pwrseq);
+	pwrseq_unregister(&pwrseq->pwrseq);
 
 	return 0;
 }
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 45cde8c..f3e29f4 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -203,7 +203,7 @@ struct mmc_context_info {
 };
 
 struct regulator;
-struct mmc_pwrseq;
+struct pwrseq;
 
 struct mmc_supply {
 	struct regulator *vmmc;		/* Card power supply */
@@ -215,7 +215,7 @@ struct mmc_host {
 	struct device		class_dev;
 	int			index;
 	const struct mmc_host_ops *ops;
-	struct mmc_pwrseq	*pwrseq;
+	struct pwrseq		*pwrseq;
 	unsigned int		f_min;
 	unsigned int		f_max;
 	unsigned int		f_init;
diff --git a/include/linux/pwrseq.h b/include/linux/pwrseq.h
index e230670..6d2d2a4 100644
--- a/include/linux/pwrseq.h
+++ b/include/linux/pwrseq.h
@@ -5,19 +5,19 @@
  *
  * License terms: GNU General Public License (GPL) version 2
  */
-#ifndef _MMC_CORE_PWRSEQ_H
-#define _MMC_CORE_PWRSEQ_H
+#ifndef _LINUX_PWRSEQ_H
+#define _LINUX_PWRSEQ_H
 
 #include <linux/mmc/host.h>
 
-struct mmc_pwrseq_ops {
+struct pwrseq_ops {
 	void (*pre_power_on)(struct mmc_host *host);
 	void (*post_power_on)(struct mmc_host *host);
 	void (*power_off)(struct mmc_host *host);
 };
 
-struct mmc_pwrseq {
-	const struct mmc_pwrseq_ops *ops;
+struct pwrseq {
+	const struct pwrseq_ops *ops;
 	struct device *dev;
 	struct list_head pwrseq_node;
 	struct module *owner;
@@ -25,8 +25,8 @@ struct mmc_pwrseq {
 
 #ifdef CONFIG_POWER_SEQ
 
-int mmc_pwrseq_register(struct mmc_pwrseq *pwrseq);
-void mmc_pwrseq_unregister(struct mmc_pwrseq *pwrseq);
+int pwrseq_register(struct pwrseq *pwrseq);
+void pwrseq_unregister(struct pwrseq *pwrseq);
 
 int mmc_pwrseq_alloc(struct mmc_host *host);
 void mmc_pwrseq_pre_power_on(struct mmc_host *host);
@@ -36,11 +36,11 @@ void mmc_pwrseq_free(struct mmc_host *host);
 
 #else /* CONFIG_POWER_SEQ */
 
-static inline int mmc_pwrseq_register(struct mmc_pwrseq *pwrseq)
+static inline int pwrseq_register(struct pwrseq *pwrseq)
 {
 	return -ENOSYS;
 }
-static inline void mmc_pwrseq_unregister(struct mmc_pwrseq *pwrseq) {}
+static inline void pwrseq_unregister(struct pwrseq *pwrseq) {}
 static inline int mmc_pwrseq_alloc(struct mmc_host *host) { return 0; }
 static inline void mmc_pwrseq_pre_power_on(struct mmc_host *host) {}
 static inline void mmc_pwrseq_post_power_on(struct mmc_host *host) {}
@@ -49,4 +49,4 @@ static inline void mmc_pwrseq_free(struct mmc_host *host) {}
 
 #endif /* CONFIG_POWER_SEQ */
 
-#endif
+#endif /* _LINUX_PWRSEQ_H */
-- 
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 05/12] power: pwrseq: Generalize mmc_pwrseq operations by removing mmc prefix

[Peter Chen]

From: Krzysztof Kozlowski <k.kozlowski@samsung.com>

The power sequence hooks (mmc_pwrseq_pre_power_on(),
mmc_pwrseq_post_power_on() and mmc_pwrseq_power_off()) can be made more
generic to allow re-use in other subsystems. They do not need to take
pointer to struct mmc_host but instead the struct pwrseq should be
sufficient.

Remove the "mmc" prefix and use the pointer to struct pwrseq as
argument.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
---
 drivers/mmc/core/core.c              |  6 +++---
 drivers/power/pwrseq/pwrseq.c        | 24 +++++++++---------------
 drivers/power/pwrseq/pwrseq_emmc.c   |  6 ++----
 drivers/power/pwrseq/pwrseq_simple.c | 14 ++++++--------
 include/linux/pwrseq.h               | 18 +++++++++---------
 5 files changed, 29 insertions(+), 39 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index edefc3f..09a4722 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1719,7 +1719,7 @@ void mmc_power_up(struct mmc_host *host, u32 ocr)
 	if (host->ios.power_mode == MMC_POWER_ON)
 		return;
 
-	mmc_pwrseq_pre_power_on(host);
+	pwrseq_pre_power_on(host->pwrseq);
 
 	host->ios.vdd = fls(ocr) - 1;
 	host->ios.power_mode = MMC_POWER_UP;
@@ -1740,7 +1740,7 @@ void mmc_power_up(struct mmc_host *host, u32 ocr)
 	 */
 	mmc_delay(10);
 
-	mmc_pwrseq_post_power_on(host);
+	pwrseq_post_power_on(host->pwrseq);
 
 	host->ios.clock = host->f_init;
 
@@ -1759,7 +1759,7 @@ void mmc_power_off(struct mmc_host *host)
 	if (host->ios.power_mode == MMC_POWER_OFF)
 		return;
 
-	mmc_pwrseq_power_off(host);
+	pwrseq_power_off(host->pwrseq);
 
 	host->ios.clock = 0;
 	host->ios.vdd = 0;
diff --git a/drivers/power/pwrseq/pwrseq.c b/drivers/power/pwrseq/pwrseq.c
index 9c66582..495a19d 100644
--- a/drivers/power/pwrseq/pwrseq.c
+++ b/drivers/power/pwrseq/pwrseq.c
@@ -52,32 +52,26 @@ int mmc_pwrseq_alloc(struct mmc_host *host)
 }
 EXPORT_SYMBOL_GPL(mmc_pwrseq_alloc);
 
-void mmc_pwrseq_pre_power_on(struct mmc_host *host)
+void pwrseq_pre_power_on(struct pwrseq *pwrseq)
 {
-	struct pwrseq *pwrseq = host->pwrseq;
-
 	if (pwrseq && pwrseq->ops->pre_power_on)
-		pwrseq->ops->pre_power_on(host);
+		pwrseq->ops->pre_power_on(pwrseq);
 }
-EXPORT_SYMBOL_GPL(mmc_pwrseq_pre_power_on);
+EXPORT_SYMBOL_GPL(pwrseq_pre_power_on);
 
-void mmc_pwrseq_post_power_on(struct mmc_host *host)
+void pwrseq_post_power_on(struct pwrseq *pwrseq)
 {
-	struct pwrseq *pwrseq = host->pwrseq;
-
 	if (pwrseq && pwrseq->ops->post_power_on)
-		pwrseq->ops->post_power_on(host);
+		pwrseq->ops->post_power_on(pwrseq);
 }
-EXPORT_SYMBOL_GPL(mmc_pwrseq_post_power_on);
+EXPORT_SYMBOL_GPL(pwrseq_post_power_on);
 
-void mmc_pwrseq_power_off(struct mmc_host *host)
+void pwrseq_power_off(struct pwrseq *pwrseq)
 {
-	struct pwrseq *pwrseq = host->pwrseq;
-
 	if (pwrseq && pwrseq->ops->power_off)
-		pwrseq->ops->power_off(host);
+		pwrseq->ops->power_off(pwrseq);
 }
-EXPORT_SYMBOL_GPL(mmc_pwrseq_power_off);
+EXPORT_SYMBOL_GPL(pwrseq_power_off);
 
 void mmc_pwrseq_free(struct mmc_host *host)
 {
diff --git a/drivers/power/pwrseq/pwrseq_emmc.c b/drivers/power/pwrseq/pwrseq_emmc.c
index a68ac9a..dbb7e75 100644
--- a/drivers/power/pwrseq/pwrseq_emmc.c
+++ b/drivers/power/pwrseq/pwrseq_emmc.c
@@ -19,8 +19,6 @@
 #include <linux/reboot.h>
 #include <linux/pwrseq.h>
 
-#include <linux/mmc/host.h>
-
 struct mmc_pwrseq_emmc {
 	struct pwrseq pwrseq;
 	struct notifier_block reset_nb;
@@ -37,9 +35,9 @@ static void __mmc_pwrseq_emmc_reset(struct mmc_pwrseq_emmc *pwrseq)
 	udelay(200);
 }
 
-static void mmc_pwrseq_emmc_reset(struct mmc_host *host)
+static void mmc_pwrseq_emmc_reset(struct pwrseq *_pwrseq)
 {
-	struct mmc_pwrseq_emmc *pwrseq =  to_pwrseq_emmc(host->pwrseq);
+	struct mmc_pwrseq_emmc *pwrseq = to_pwrseq_emmc(_pwrseq);
 
 	__mmc_pwrseq_emmc_reset(pwrseq);
 }
diff --git a/drivers/power/pwrseq/pwrseq_simple.c b/drivers/power/pwrseq/pwrseq_simple.c
index d5fbd65..93807a6 100644
--- a/drivers/power/pwrseq/pwrseq_simple.c
+++ b/drivers/power/pwrseq/pwrseq_simple.c
@@ -18,8 +18,6 @@
 #include <linux/gpio/consumer.h>
 #include <linux/pwrseq.h>
 
-#include <linux/mmc/host.h>
-
 struct mmc_pwrseq_simple {
 	struct pwrseq pwrseq;
 	bool clk_enabled;
@@ -46,9 +44,9 @@ static void mmc_pwrseq_simple_set_gpios_value(struct mmc_pwrseq_simple *pwrseq,
 	}
 }
 
-static void mmc_pwrseq_simple_pre_power_on(struct mmc_host *host)
+static void mmc_pwrseq_simple_pre_power_on(struct pwrseq *_pwrseq)
 {
-	struct mmc_pwrseq_simple *pwrseq = to_pwrseq_simple(host->pwrseq);
+	struct mmc_pwrseq_simple *pwrseq = to_pwrseq_simple(_pwrseq);
 
 	if (!IS_ERR(pwrseq->ext_clk) && !pwrseq->clk_enabled) {
 		clk_prepare_enable(pwrseq->ext_clk);
@@ -58,16 +56,16 @@ static void mmc_pwrseq_simple_pre_power_on(struct mmc_host *host)
 	mmc_pwrseq_simple_set_gpios_value(pwrseq, 1);
 }
 
-static void mmc_pwrseq_simple_post_power_on(struct mmc_host *host)
+static void mmc_pwrseq_simple_post_power_on(struct pwrseq *_pwrseq)
 {
-	struct mmc_pwrseq_simple *pwrseq = to_pwrseq_simple(host->pwrseq);
+	struct mmc_pwrseq_simple *pwrseq = to_pwrseq_simple(_pwrseq);
 
 	mmc_pwrseq_simple_set_gpios_value(pwrseq, 0);
 }
 
-static void mmc_pwrseq_simple_power_off(struct mmc_host *host)
+static void mmc_pwrseq_simple_power_off(struct pwrseq *_pwrseq)
 {
-	struct mmc_pwrseq_simple *pwrseq = to_pwrseq_simple(host->pwrseq);
+	struct mmc_pwrseq_simple *pwrseq = to_pwrseq_simple(_pwrseq);
 
 	mmc_pwrseq_simple_set_gpios_value(pwrseq, 1);
 
diff --git a/include/linux/pwrseq.h b/include/linux/pwrseq.h
index 6d2d2a4..fcc8fd8 100644
--- a/include/linux/pwrseq.h
+++ b/include/linux/pwrseq.h
@@ -11,9 +11,9 @@
 #include <linux/mmc/host.h>
 
 struct pwrseq_ops {
-	void (*pre_power_on)(struct mmc_host *host);
-	void (*post_power_on)(struct mmc_host *host);
-	void (*power_off)(struct mmc_host *host);
+	void (*pre_power_on)(struct pwrseq *pwrseq);
+	void (*post_power_on)(struct pwrseq *pwrseq);
+	void (*power_off)(struct pwrseq *pwrseq);
 };
 
 struct pwrseq {
@@ -28,10 +28,10 @@ struct pwrseq {
 int pwrseq_register(struct pwrseq *pwrseq);
 void pwrseq_unregister(struct pwrseq *pwrseq);
 
+void pwrseq_pre_power_on(struct pwrseq *pwrseq);
+void pwrseq_post_power_on(struct pwrseq *pwrseq);
+void pwrseq_power_off(struct pwrseq *pwrseq);
 int mmc_pwrseq_alloc(struct mmc_host *host);
-void mmc_pwrseq_pre_power_on(struct mmc_host *host);
-void mmc_pwrseq_post_power_on(struct mmc_host *host);
-void mmc_pwrseq_power_off(struct mmc_host *host);
 void mmc_pwrseq_free(struct mmc_host *host);
 
 #else /* CONFIG_POWER_SEQ */
@@ -41,10 +41,10 @@ static inline int pwrseq_register(struct pwrseq *pwrseq)
 	return -ENOSYS;
 }
 static inline void pwrseq_unregister(struct pwrseq *pwrseq) {}
+static inline void pwrseq_pre_power_on(struct pwrseq *pwrseq) {}
+static inline void pwrseq_post_power_on(struct pwrseq *pwrseq) {}
+static inline void pwrseq_power_off(struct pwrseq *pwrseq) {}
 static inline int mmc_pwrseq_alloc(struct mmc_host *host) { return 0; }
-static inline void mmc_pwrseq_pre_power_on(struct mmc_host *host) {}
-static inline void mmc_pwrseq_post_power_on(struct mmc_host *host) {}
-static inline void mmc_pwrseq_power_off(struct mmc_host *host) {}
 static inline void mmc_pwrseq_free(struct mmc_host *host) {}
 
 #endif /* CONFIG_POWER_SEQ */
-- 
1.9.1

[PATCH 06/12] power: pwrseq: change common helpers as generic

[Peter Chen]

These helpers are only used by mmc now, change them as generic and
do corresponding changes at mmc code.

Meanwhile, change return value of helper pwrseq_pre_power_on since
it may occur error during enable clock/regulator, etc.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/mmc/core/host.c              |  8 +++++--
 drivers/power/pwrseq/pwrseq.c        | 45 ++++++++++++++++++------------------
 drivers/power/pwrseq/pwrseq_simple.c | 10 +++++---
 include/linux/pwrseq.h               | 31 ++++++++++++++-----------
 4 files changed, 52 insertions(+), 42 deletions(-)

diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 1db7d58..c8ffd9b 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -322,7 +322,11 @@ int mmc_of_parse(struct mmc_host *host)
 		host->dsr_req = 0;
 	}
 
-	return mmc_pwrseq_alloc(host);
+	host->pwrseq = pwrseq_alloc(host->parent->of_node, "mmc-pwrseq");
+	if (IS_ERR(host->pwrseq))
+		return PTR_ERR(host->pwrseq);
+	else
+		return 0;
 }
 
 EXPORT_SYMBOL(mmc_of_parse);
@@ -462,7 +466,7 @@ EXPORT_SYMBOL(mmc_remove_host);
  */
 void mmc_free_host(struct mmc_host *host)
 {
-	mmc_pwrseq_free(host);
+	pwrseq_free(host->pwrseq);
 	put_device(&host->class_dev);
 }
 
diff --git a/drivers/power/pwrseq/pwrseq.c b/drivers/power/pwrseq/pwrseq.c
index 495a19d..25e4dfd 100644
--- a/drivers/power/pwrseq/pwrseq.c
+++ b/drivers/power/pwrseq/pwrseq.c
@@ -5,37 +5,38 @@
  *
  * License terms: GNU General Public License (GPL) version 2
  *
- *  MMC power sequence management
+ * Power sequence management helpers
  */
+
+#include <linux/device.h>
 #include <linux/kernel.h>
 #include <linux/err.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/pwrseq.h>
 
-#include <linux/mmc/host.h>
 
 static DEFINE_MUTEX(pwrseq_list_mutex);
 static LIST_HEAD(pwrseq_list);
 
-int mmc_pwrseq_alloc(struct mmc_host *host)
+struct pwrseq *pwrseq_alloc(const struct device_node *node,
+	const char *phandle_name)
 {
 	struct device_node *np;
-	struct pwrseq *p;
+	struct pwrseq *p, *pwrseq = NULL;
 
-	np = of_parse_phandle(host->parent->of_node, "mmc-pwrseq", 0);
+	np = of_parse_phandle(node, phandle_name, 0);
 	if (!np)
-		return 0;
+		return NULL;
 
 	mutex_lock(&pwrseq_list_mutex);
 	list_for_each_entry(p, &pwrseq_list, pwrseq_node) {
 		if (p->dev->of_node == np) {
 			if (!try_module_get(p->owner))
-				dev_err(host->parent,
+				dev_err(p->dev,
 					"increasing module refcount failed\n");
 			else
-				host->pwrseq = p;
-
+				pwrseq = p;
 			break;
 		}
 	}
@@ -43,19 +44,21 @@ int mmc_pwrseq_alloc(struct mmc_host *host)
 	of_node_put(np);
 	mutex_unlock(&pwrseq_list_mutex);
 
-	if (!host->pwrseq)
-		return -EPROBE_DEFER;
+	if (!pwrseq)
+		return ERR_PTR(-EPROBE_DEFER);
 
-	dev_info(host->parent, "allocated mmc-pwrseq\n");
+	dev_info(p->dev, "pwrseq is allocated\n");
 
-	return 0;
+	return pwrseq;
 }
-EXPORT_SYMBOL_GPL(mmc_pwrseq_alloc);
+EXPORT_SYMBOL_GPL(pwrseq_alloc);
 
-void pwrseq_pre_power_on(struct pwrseq *pwrseq)
+int pwrseq_pre_power_on(struct pwrseq *pwrseq)
 {
 	if (pwrseq && pwrseq->ops->pre_power_on)
-		pwrseq->ops->pre_power_on(pwrseq);
+		return pwrseq->ops->pre_power_on(pwrseq);
+	else
+		return 0;
 }
 EXPORT_SYMBOL_GPL(pwrseq_pre_power_on);
 
@@ -73,16 +76,12 @@ void pwrseq_power_off(struct pwrseq *pwrseq)
 }
 EXPORT_SYMBOL_GPL(pwrseq_power_off);
 
-void mmc_pwrseq_free(struct mmc_host *host)
+void pwrseq_free(struct pwrseq *pwrseq)
 {
-	struct pwrseq *pwrseq = host->pwrseq;
-
-	if (pwrseq) {
+	if (pwrseq)
 		module_put(pwrseq->owner);
-		host->pwrseq = NULL;
-	}
 }
-EXPORT_SYMBOL_GPL(mmc_pwrseq_free);
+EXPORT_SYMBOL_GPL(pwrseq_free);
 
 int pwrseq_register(struct pwrseq *pwrseq)
 {
diff --git a/drivers/power/pwrseq/pwrseq_simple.c b/drivers/power/pwrseq/pwrseq_simple.c
index 93807a6..349443f 100644
--- a/drivers/power/pwrseq/pwrseq_simple.c
+++ b/drivers/power/pwrseq/pwrseq_simple.c
@@ -44,16 +44,20 @@ static void mmc_pwrseq_simple_set_gpios_value(struct mmc_pwrseq_simple *pwrseq,
 	}
 }
 
-static void mmc_pwrseq_simple_pre_power_on(struct pwrseq *_pwrseq)
+static int mmc_pwrseq_simple_pre_power_on(struct pwrseq *_pwrseq)
 {
 	struct mmc_pwrseq_simple *pwrseq = to_pwrseq_simple(_pwrseq);
+	int ret = 0;
 
 	if (!IS_ERR(pwrseq->ext_clk) && !pwrseq->clk_enabled) {
-		clk_prepare_enable(pwrseq->ext_clk);
-		pwrseq->clk_enabled = true;
+		ret = clk_prepare_enable(pwrseq->ext_clk);
+		if (!ret)
+			pwrseq->clk_enabled = true;
 	}
 
 	mmc_pwrseq_simple_set_gpios_value(pwrseq, 1);
+
+	return 0;
 }
 
 static void mmc_pwrseq_simple_post_power_on(struct pwrseq *_pwrseq)
diff --git a/include/linux/pwrseq.h b/include/linux/pwrseq.h
index fcc8fd8..f726e7e 100644
--- a/include/linux/pwrseq.h
+++ b/include/linux/pwrseq.h
@@ -8,14 +8,6 @@
 #ifndef _LINUX_PWRSEQ_H
 #define _LINUX_PWRSEQ_H
 
-#include <linux/mmc/host.h>
-
-struct pwrseq_ops {
-	void (*pre_power_on)(struct pwrseq *pwrseq);
-	void (*post_power_on)(struct pwrseq *pwrseq);
-	void (*power_off)(struct pwrseq *pwrseq);
-};
-
 struct pwrseq {
 	const struct pwrseq_ops *ops;
 	struct device *dev;
@@ -23,16 +15,23 @@ struct pwrseq {
 	struct module *owner;
 };
 
+struct pwrseq_ops {
+	int (*pre_power_on)(struct pwrseq *pwrseq);
+	void (*post_power_on)(struct pwrseq *pwrseq);
+	void (*power_off)(struct pwrseq *pwrseq);
+};
+
 #ifdef CONFIG_POWER_SEQ
 
 int pwrseq_register(struct pwrseq *pwrseq);
 void pwrseq_unregister(struct pwrseq *pwrseq);
 
-void pwrseq_pre_power_on(struct pwrseq *pwrseq);
+int pwrseq_pre_power_on(struct pwrseq *pwrseq);
 void pwrseq_post_power_on(struct pwrseq *pwrseq);
 void pwrseq_power_off(struct pwrseq *pwrseq);
-int mmc_pwrseq_alloc(struct mmc_host *host);
-void mmc_pwrseq_free(struct mmc_host *host);
+struct pwrseq *pwrseq_alloc(const struct device_node *node,
+	const char *phandle_name);
+void pwrseq_free(struct pwrseq *pwrseq);
 
 #else /* CONFIG_POWER_SEQ */
 
@@ -41,11 +40,15 @@ static inline int pwrseq_register(struct pwrseq *pwrseq)
 	return -ENOSYS;
 }
 static inline void pwrseq_unregister(struct pwrseq *pwrseq) {}
-static inline void pwrseq_pre_power_on(struct pwrseq *pwrseq) {}
+static inline int pwrseq_pre_power_on(struct pwrseq *pwrseq) {}
 static inline void pwrseq_post_power_on(struct pwrseq *pwrseq) {}
 static inline void pwrseq_power_off(struct pwrseq *pwrseq) {}
-static inline int mmc_pwrseq_alloc(struct mmc_host *host) { return 0; }
-static inline void mmc_pwrseq_free(struct mmc_host *host) {}
+static inline struct pwrseq *pwrseq_alloc(const struct device_node *node,
+	const char *phandle_name)
+{
+	return NULL;
+}
+static inline void pwrseq_free(struct mmc_host *host) {}
 
 #endif /* CONFIG_POWER_SEQ */
 
-- 
1.9.1

[PATCH 07/12] power: pwrseq: rename file name for generic driver

[Peter Chen]

The individual driver file name is better to contain module name.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 .../power/pwrseq/{pwrseq-simple.txt => mmc-pwrseq-simple.txt}         | 0
 drivers/power/pwrseq/Makefile                                         | 4 ++--
 drivers/power/pwrseq/{pwrseq.c => core.c}                             | 0
 drivers/power/pwrseq/{pwrseq_simple.c => pwrseq_mmc_simple.c}         | 0
 4 files changed, 2 insertions(+), 2 deletions(-)
 rename Documentation/devicetree/bindings/power/pwrseq/{pwrseq-simple.txt => mmc-pwrseq-simple.txt} (100%)
 rename drivers/power/pwrseq/{pwrseq.c => core.c} (100%)
 rename drivers/power/pwrseq/{pwrseq_simple.c => pwrseq_mmc_simple.c} (100%)

diff --git a/Documentation/devicetree/bindings/power/pwrseq/pwrseq-simple.txt b/Documentation/devicetree/bindings/power/pwrseq/mmc-pwrseq-simple.txt
similarity index 100%
rename from Documentation/devicetree/bindings/power/pwrseq/pwrseq-simple.txt
rename to Documentation/devicetree/bindings/power/pwrseq/mmc-pwrseq-simple.txt
diff --git a/drivers/power/pwrseq/Makefile b/drivers/power/pwrseq/Makefile
index 9e40e4b..d475e01 100644
--- a/drivers/power/pwrseq/Makefile
+++ b/drivers/power/pwrseq/Makefile
@@ -1,3 +1,3 @@
-obj-$(CONFIG_POWER_SEQ)		+= pwrseq.o
-obj-$(CONFIG_POWER_SEQ_SIMPLE)	+= pwrseq_simple.o
+obj-$(CONFIG_POWER_SEQ)		+= core.o
+obj-$(CONFIG_POWER_SEQ_SIMPLE)	+= pwrseq_mmc_simple.o
 obj-$(CONFIG_POWER_SEQ_EMMC)	+= pwrseq_emmc.o
diff --git a/drivers/power/pwrseq/pwrseq.c b/drivers/power/pwrseq/core.c
similarity index 100%
rename from drivers/power/pwrseq/pwrseq.c
rename to drivers/power/pwrseq/core.c
diff --git a/drivers/power/pwrseq/pwrseq_simple.c b/drivers/power/pwrseq/pwrseq_mmc_simple.c
similarity index 100%
rename from drivers/power/pwrseq/pwrseq_simple.c
rename to drivers/power/pwrseq/pwrseq_mmc_simple.c
-- 
1.9.1

[PATCH 08/12] doc: binding: pwrseq-usb-generic: add binding doc for generic usb power sequence driver

[Peter Chen]

Add binding doc for generic usb power sequence driver, and update
generic usb device binding-doc accordingly.

Signed-off-by: Peter Chen <peter.chen-3arQi8VN3Tc@public.gmane.org>
---
 .../bindings/power/pwrseq/pwrseq-usb-generic.txt   | 31 ++++++++++++++++++++++
 .../devicetree/bindings/usb/usb-device.txt         |  2 ++
 2 files changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt

diff --git a/Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt
new file mode 100644
index 0000000..8ad98382
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt
@@ -0,0 +1,31 @@
+The power sequence for generic USB Devices
+
+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. The
+power sequence will be done before this device can be found at USB
+bus.
+
+Required properties:
+- compatible : contains "usb-pwrseq-generic".
+
+Optional properties:
+- clocks: the input clock for USB device.
+- clock-frequency: the frequency for device's clock.
+- reset-gpios: Should specify the GPIO for reset.
+- reset-duration-us: the duration in microsecond for assert reset signal.
+- enable-gpios: Should specify the GPIO for enable.
+- power-supply: The regulator of the power
+
+Example:
+
+usb2415_pwrseq: usb2415_pwrseq {
+	compatible = "usb-pwrseq-generic";
+	clocks = <&clks IMX6QDL_CLK_CKO>;
+	reset-gpios = <&gpio7 12 GPIO_ACTIVE_LOW>;
+	reset-duration-us = <3000>;
+	enable-gpios = <&gpio7 11 GPIO_ACTIVE_LOW>;
+	power-supply = <&reg_usb_power>;
+};
diff --git a/Documentation/devicetree/bindings/usb/usb-device.txt b/Documentation/devicetree/bindings/usb/usb-device.txt
index 1c35e7b..4c8a25e 100644
--- a/Documentation/devicetree/bindings/usb/usb-device.txt
+++ b/Documentation/devicetree/bindings/usb/usb-device.txt
@@ -12,6 +12,7 @@ Required properties:
   for usbVID,PID.
 - reg: the port number which this device is connecting to, the range
   is 1-31.
+- usb-pwrseq: phandle for power sequence.
 
 Example:
 
@@ -24,5 +25,6 @@ Example:
 	hub: genesys@1 {
 		compatible = "usb5e3,608";
 		reg = <1>;
+		usb-pwrseq = <&usb_genesys_pwrseq>;
 	};
 }
-- 
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 09/12] power: pwrseq: pwrseq_usb_generic: add generic power sequence support for USB deivces

[Peter Chen]

Add driver for generic power sequence support for USB deivces
which handles below things:

- Clock and its frequencies
- GPIO for reset and the duration time
- GPIO for enable
- Regulator for power

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/power/pwrseq/Kconfig              |  11 ++
 drivers/power/pwrseq/Makefile             |   1 +
 drivers/power/pwrseq/pwrseq_usb_generic.c | 202 ++++++++++++++++++++++++++++++
 3 files changed, 214 insertions(+)
 create mode 100644 drivers/power/pwrseq/pwrseq_usb_generic.c

diff --git a/drivers/power/pwrseq/Kconfig b/drivers/power/pwrseq/Kconfig
index c7e9271..9749c6d 100644
--- a/drivers/power/pwrseq/Kconfig
+++ b/drivers/power/pwrseq/Kconfig
@@ -29,4 +29,15 @@ config POWER_SEQ_SIMPLE
 	  This driver can also be built as a module. If so, the module
 	  will be called pwrseq_simple.
 
+config POWER_SEQ_USB_GENERIC
+	tristate "Generic power sequence support for USB devices"
+	default y
+	depends on OF || COMPILE_TEST
+	help
+	  This selects generic power sequence support for USB devices.
+	  By default this option is set to y.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called pwrseq_usb_generic.
+
 endif
diff --git a/drivers/power/pwrseq/Makefile b/drivers/power/pwrseq/Makefile
index d475e01..230bb45 100644
--- a/drivers/power/pwrseq/Makefile
+++ b/drivers/power/pwrseq/Makefile
@@ -1,3 +1,4 @@
 obj-$(CONFIG_POWER_SEQ)		+= core.o
 obj-$(CONFIG_POWER_SEQ_SIMPLE)	+= pwrseq_mmc_simple.o
 obj-$(CONFIG_POWER_SEQ_EMMC)	+= pwrseq_emmc.o
+obj-$(CONFIG_POWER_SEQ_USB_GENERIC)	+= pwrseq_usb_generic.o
diff --git a/drivers/power/pwrseq/pwrseq_usb_generic.c b/drivers/power/pwrseq/pwrseq_usb_generic.c
new file mode 100644
index 0000000..6017636
--- /dev/null
+++ b/drivers/power/pwrseq/pwrseq_usb_generic.c
@@ -0,0 +1,202 @@
+/*
+ * usb_generic.c	The generic power sequence driver for USB device
+ *
+ * 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/device.h>
+#include <linux/gpio/consumer.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+#include <linux/pwrseq.h>
+
+struct usb_pwrseq_generic {
+	struct pwrseq pwrseq;
+	struct gpio_desc *gpiod_reset;
+	struct gpio_desc *gpiod_enable;
+	struct clk *clk;
+	struct regulator *reg_power;
+};
+
+#define to_pwrseq_generic(p) container_of(p, struct usb_pwrseq_generic, pwrseq)
+
+static void usb_pwrseq_power_off(struct pwrseq *_pwrseq)
+{
+	struct usb_pwrseq_generic *pwrseq = to_pwrseq_generic(_pwrseq);
+
+	if (pwrseq->clk)
+		clk_disable_unprepare(pwrseq->clk);
+
+	if (pwrseq->gpiod_enable)
+		gpiod_set_value(pwrseq->gpiod_enable, 0);
+
+	if (pwrseq->reg_power)
+		regulator_disable(pwrseq->reg_power);
+}
+
+static int usb_pwrseq_power_on(struct pwrseq *_pwrseq)
+{
+	struct usb_pwrseq_generic *pwrseq = to_pwrseq_generic(_pwrseq);
+	struct gpio_desc *gpiod_reset = pwrseq->gpiod_reset;
+	struct gpio_desc *gpiod_enable = pwrseq->gpiod_enable;
+	struct device *dev = _pwrseq->dev;
+	struct device_node *node = dev->of_node;
+	u32 duration_us = 50, clk_rate = 0;
+	int ret;
+
+	if (pwrseq->clk) {
+		ret = clk_prepare_enable(pwrseq->clk);
+		if (ret) {
+			dev_err(dev,
+				"Can't enable external clock: %d\n",
+				ret);
+			return ret;
+		}
+
+		of_property_read_u32(node, "clock-frequency", &clk_rate);
+		if (clk_rate) {
+			ret = clk_set_rate(pwrseq->clk, clk_rate);
+			if (ret) {
+				dev_err(dev, "Error setting clock rate\n");
+				goto disable_clk;
+			}
+		}
+	}
+
+	if (gpiod_reset) {
+		of_property_read_u32(node, "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);
+	}
+
+	if (gpiod_enable) {
+		gpiod_direction_output(gpiod_enable, 1);
+		gpiod_set_value(gpiod_enable, 1);
+	}
+
+	if (pwrseq->reg_power) {
+		ret = regulator_enable(pwrseq->reg_power);
+		if (ret) {
+			dev_err(dev, "Failed to power regulator, ret=%d\n",
+					ret);
+			goto disable_gpio;
+		}
+	}
+
+	return 0;
+
+disable_gpio:
+	if (gpiod_enable)
+		gpiod_set_value(gpiod_enable, 0);
+
+disable_clk:
+	if (pwrseq->clk)
+		clk_disable_unprepare(pwrseq->clk);
+	return ret;
+}
+
+static const struct pwrseq_ops usb_pwrseq_generic_ops = {
+	.pre_power_on = usb_pwrseq_power_on,
+	.power_off = usb_pwrseq_power_off,
+};
+
+static const struct of_device_id usb_pwrseq_generic_of_match[] = {
+	{ .compatible = "usb-pwrseq-generic",},
+	{/* sentinel */},
+};
+MODULE_DEVICE_TABLE(of, usb_pwrseq_generic_of_match);
+
+static int usb_pwrseq_generic_probe(struct platform_device *pdev)
+{
+	struct usb_pwrseq_generic *pwrseq;
+	struct device *dev = &pdev->dev;
+	struct regulator *reg;
+	int ret;
+
+	pwrseq = devm_kzalloc(dev, sizeof(*pwrseq), GFP_KERNEL);
+	if (!pwrseq)
+		return -ENOMEM;
+
+	pwrseq->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(pwrseq->clk)) {
+		dev_dbg(dev, "Can't get clock: %ld\n",
+			PTR_ERR(pwrseq->clk));
+		pwrseq->clk = NULL;
+	}
+
+	pwrseq->gpiod_reset = devm_gpiod_get_optional(dev, "reset",
+			GPIOD_ASIS);
+	ret = PTR_ERR_OR_ZERO(pwrseq->gpiod_reset);
+	if (ret) {
+		dev_err(dev, "Failed to get reset gpio, err = %d\n", ret);
+		return ret;
+	}
+
+	pwrseq->gpiod_enable = devm_gpiod_get_optional(dev, "enable",
+			GPIOD_ASIS);
+	ret = PTR_ERR_OR_ZERO(pwrseq->gpiod_enable);
+	if (ret) {
+		dev_err(dev, "Failed to get enable gpio, err = %d\n", ret);
+		return ret;
+	}
+
+	reg = devm_regulator_get(dev, "power");
+	if (PTR_ERR(reg) == -EPROBE_DEFER)
+		return -EPROBE_DEFER;
+	else if (PTR_ERR(reg) == -ENODEV)
+		/* no power regulator is needed */
+		reg = NULL;
+	else if (IS_ERR(reg))
+		return PTR_ERR(reg);
+	pwrseq->reg_power = reg;
+
+	pwrseq->pwrseq.dev = dev;
+	pwrseq->pwrseq.ops = &usb_pwrseq_generic_ops;
+	pwrseq->pwrseq.owner = THIS_MODULE;
+	platform_set_drvdata(pdev, pwrseq);
+
+	return pwrseq_register(&pwrseq->pwrseq);
+}
+
+static int usb_pwrseq_generic_remove(struct platform_device *pdev)
+{
+	struct usb_pwrseq_generic *pwrseq = platform_get_drvdata(pdev);
+
+	pwrseq_unregister(&pwrseq->pwrseq);
+
+	return 0;
+}
+
+static struct platform_driver usb_pwrseq_generic_driver = {
+	.probe = usb_pwrseq_generic_probe,
+	.remove = usb_pwrseq_generic_remove,
+	.driver = {
+		.name = "usb_pwrseq_generic",
+		.of_match_table = usb_pwrseq_generic_of_match,
+	},
+};
+
+module_platform_driver(usb_pwrseq_generic_driver);
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

[PATCH 10/12] 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 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 phandle "usb-pwrseq" for it.

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

Signed-off-by: Peter Chen <peter.chen-3arQi8VN3Tc@public.gmane.org>
---
 drivers/usb/core/hub.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++---
 drivers/usb/core/hub.h |  1 +
 include/linux/pwrseq.h |  6 +++++
 3 files changed, 77 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index bee1351..cc0f942 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/pwrseq.h>
 
 #include <asm/uaccess.h>
 #include <asm/byteorder.h>
@@ -1684,6 +1685,66 @@ static void hub_release(struct kref *kref)
 
 static unsigned highspeed_hubs;
 
+static void hub_of_pwrseq_off(struct usb_interface *intf)
+{
+	struct usb_hub *hub = usb_get_intfdata(intf);
+	struct pwrseq *hdev_pwrseq;
+	struct pwrseq_node_powered_on *pwrseq_node, *tmp_node;
+
+	list_for_each_entry_safe(pwrseq_node, tmp_node,
+			&hub->pwrseq_list, list) {
+		hdev_pwrseq = pwrseq_node->pwrseq_on;
+		pwrseq_power_off(hdev_pwrseq);
+		list_del(&pwrseq_node->list);
+		pwrseq_free(hdev_pwrseq);
+		kfree(pwrseq_node);
+	}
+}
+
+static int hub_of_pwrseq_on(struct usb_interface *intf)
+{
+	struct device *parent;
+	struct device_node *node;
+	struct pwrseq *hdev_pwrseq;
+	struct usb_device *hdev = interface_to_usbdev(intf);
+	struct usb_hub *hub = usb_get_intfdata(intf);
+	struct pwrseq_node_powered_on *pwrseq_node;
+	int ret = 0;
+
+	if (hdev->parent)
+		parent = &hdev->dev;
+	else
+		parent = bus_to_hcd(hdev->bus)->self.controller;
+
+	for_each_child_of_node(parent->of_node, node) {
+		hdev_pwrseq = pwrseq_alloc(node, "usb-pwrseq");
+		if (!IS_ERR_OR_NULL(hdev_pwrseq)) {
+			pwrseq_node = kzalloc(sizeof(pwrseq_node), GFP_KERNEL);
+			if (!pwrseq_node) {
+				ret = -ENOMEM;
+				goto err1;
+			}
+			/* power on sequence */
+			ret = pwrseq_pre_power_on(hdev_pwrseq);
+			if (ret)
+				goto err2;
+
+			pwrseq_node->pwrseq_on = hdev_pwrseq;
+			list_add(&pwrseq_node->list, &hub->pwrseq_list);
+		} else if (IS_ERR(hdev_pwrseq)) {
+			return PTR_ERR(hdev_pwrseq);
+		}
+	}
+
+	return ret;
+
+err2:
+	kfree(pwrseq_node);
+err1:
+	pwrseq_free(hdev_pwrseq);
+	return ret;
+}
+
 static void hub_disconnect(struct usb_interface *intf)
 {
 	struct usb_hub *hub = usb_get_intfdata(intf);
@@ -1700,6 +1761,7 @@ static void hub_disconnect(struct usb_interface *intf)
 	hub->error = 0;
 	hub_quiesce(hub, HUB_DISCONNECT);
 
+	hub_of_pwrseq_off(intf);
 	mutex_lock(&usb_port_peer_mutex);
 
 	/* Avoid races with recursively_mark_NOTATTACHED() */
@@ -1733,6 +1795,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 +1902,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_list);
 	usb_get_intf(intf);
 	usb_get_dev(hdev);
 
@@ -1852,11 +1916,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(intf);
+		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..f52169c 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_list; /* powered on pwrseq node list */
 };
 
 /**
diff --git a/include/linux/pwrseq.h b/include/linux/pwrseq.h
index f726e7e..36dde42 100644
--- a/include/linux/pwrseq.h
+++ b/include/linux/pwrseq.h
@@ -15,6 +15,12 @@ struct pwrseq {
 	struct module *owner;
 };
 
+/* This structure is used for recording powered on pwrseq node */
+struct pwrseq_node_powered_on {
+	struct pwrseq *pwrseq_on;
+	struct list_head list;
+};
+
 struct pwrseq_ops {
 	int (*pre_power_on)(struct pwrseq *pwrseq);
 	void (*post_power_on)(struct pwrseq *pwrseq);
-- 
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 11/12] 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 12/12] ARM: dts: imx6qdl-udoo.dtsi: fix onboard USB HUB property

[Peter Chen]

From: Peter Chen <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org>

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, with "usb-pwrseq" phandle to handle power
sequence stuffs.

Signed-off-by: Peter Chen <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
Signed-off-by: Maciej S. Szmigiero <mail-APzI5cXaD1zVlRWJc41N0YvC60bnQu0Y@public.gmane.org>
---
 arch/arm/boot/dts/imx6qdl-udoo.dtsi | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/arch/arm/boot/dts/imx6qdl-udoo.dtsi b/arch/arm/boot/dts/imx6qdl-udoo.dtsi
index 3bee2f9..f921174 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>;
@@ -89,6 +80,13 @@
 		mux-int-port = <1>;
 		mux-ext-port = <6>;
 	};
+
+	usb2415_pwrseq: usb2415_pwrseq {
+		compatible = "usb-pwrseq-generic";
+		clocks = <&clks IMX6QDL_CLK_CKO>;
+		reset-gpios = <&gpio7 12 GPIO_ACTIVE_LOW>;
+		reset-duration-us = <3000>;
+	};
 };
 
 &fec {
@@ -259,9 +257,15 @@
 &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>;
+		usb-pwrseq = <&usb2415_pwrseq>;
+	};
 };
 
 &usdhc3 {
-- 
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

Re: [PATCH 10/12] usb: core: add power sequence handling for USB devices

[Alan Stern]

On Fri, 17 Jun 2016, Peter Chen wrote:

> Some hard-wired USB devices need to do power sequence to let the
> device work normally, the typical power sequence like: enable USB
> PHY clock, toggle reset pin, etc. But current Linux USB driver
> lacks of such code to do it, it may cause some hard-wired USB devices
> works abnormal or can't be recognized by controller at all.
> 
> In this patch, it 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 phandle "usb-pwrseq" for it.
> 
> At hub_disconnect, it will do power off sequence which is at powered on
> list.
> 
> Signed-off-by: Peter Chen <peter.chen-3arQi8VN3Tc@public.gmane.org>
> ---
>  drivers/usb/core/hub.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++---
>  drivers/usb/core/hub.h |  1 +
>  include/linux/pwrseq.h |  6 +++++
>  3 files changed, 77 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index bee1351..cc0f942 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/pwrseq.h>
>  
>  #include <asm/uaccess.h>
>  #include <asm/byteorder.h>
> @@ -1684,6 +1685,66 @@ static void hub_release(struct kref *kref)
>  
>  static unsigned highspeed_hubs;
>  
> +static void hub_of_pwrseq_off(struct usb_interface *intf)
> +{
> +	struct usb_hub *hub = usb_get_intfdata(intf);

It would be easier to pass hub as the argument instead of intf.

Otherwise this looks okay to me.

Alan Stern

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

Re: [PATCH 08/12] doc: binding: pwrseq-usb-generic: add binding doc for generic usb power sequence driver

[Rob Herring]

On Fri, Jun 17, 2016 at 5:09 AM, Peter Chen <peter.chen@nxp.com> wrote:
> Add binding doc for generic usb power sequence driver, and update
> generic usb device binding-doc accordingly.
>
> Signed-off-by: Peter Chen <peter.chen@nxp.com>
> ---
>  .../bindings/power/pwrseq/pwrseq-usb-generic.txt   | 31 ++++++++++++++++++++++
>  .../devicetree/bindings/usb/usb-device.txt         |  2 ++
>  2 files changed, 33 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt
>
> diff --git a/Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt
> new file mode 100644
> index 0000000..8ad98382
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt
> @@ -0,0 +1,31 @@
> +The power sequence for generic USB Devices
> +
> +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. The
> +power sequence will be done before this device can be found at USB
> +bus.
> +
> +Required properties:
> +- compatible : contains "usb-pwrseq-generic".

In case I have not been clear, no.

I am not going to accept anything along the lines of the current mmc
pwrseq. I am basically okay with Krzysztof's proposal as it is *only*
an added property and not a duplication of information. I'd suggest
you figure out how to make the kernel work with that rather than
trying to work-around whatever kernel limitations there are.

Rob

Re: [PATCH 00/12] power: add generic power sequence framework

[Maciej S. Szmigiero]

Hi Peter,

On 17.06.2016 12:09, Peter Chen wrote:
> Hi all,
> 
> 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
> framework to handle this requirement. The generic code is supplied
> some common helpers from host driver, and individual power sequence
> driver handles kinds of power sequence for devices.
> 
> Since the MMC has already done the similar things, and this power
> sequence handling can be generic, we use mmc power sequence code
> as base to create this framework.
> 
> This patch set is based on Krzysztof Kozlowski's RFC patch set (v4.7-rc1)
> [1], and making some changes which can let it be generic. After that,
> we create a generic power sequence driver for USB devices which handles
> below things, it includes all input signals for devices I can consider.
> 
> - Clock and its frequencies
> - GPIO for reset and the duration time
> - GPIO for enable
> - Regulator for power
> 
> 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 udoo board changes
> were tested using my last power sequence patch set. [2]
> 
> @Maciej S. Szmigiero, Oscar, would you please test them on imx6qdl udoo
> boards?

Thanks for the patch set, just tested them on my UDOO Quad board and can
confirm that USB and on-board hub work fine.

Tested-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>

Maciej

Re: [PATCH 08/12] doc: binding: pwrseq-usb-generic: add binding doc for generic usb power sequence driver

[Peter Chen]

On Fri, Jun 17, 2016 at 12:16:48PM -0500, Rob Herring wrote:
> On Fri, Jun 17, 2016 at 5:09 AM, Peter Chen <peter.chen@nxp.com> wrote:
> > Add binding doc for generic usb power sequence driver, and update
> > generic usb device binding-doc accordingly.
> >
> > Signed-off-by: Peter Chen <peter.chen@nxp.com>
> > ---
> >  .../bindings/power/pwrseq/pwrseq-usb-generic.txt   | 31 ++++++++++++++++++++++
> >  .../devicetree/bindings/usb/usb-device.txt         |  2 ++
> >  2 files changed, 33 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt
> >
> > diff --git a/Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt
> > new file mode 100644
> > index 0000000..8ad98382
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt
> > @@ -0,0 +1,31 @@
> > +The power sequence for generic USB Devices
> > +
> > +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. The
> > +power sequence will be done before this device can be found at USB
> > +bus.
> > +
> > +Required properties:
> > +- compatible : contains "usb-pwrseq-generic".
> 
> In case I have not been clear, no.
> 
> I am not going to accept anything along the lines of the current mmc
> pwrseq. I am basically okay with Krzysztof's proposal as it is *only*
> an added property and not a duplication of information. I'd suggest
> you figure out how to make the kernel work with that rather than
> trying to work-around whatever kernel limitations there are.
> 

I see.

Would you agree with below:

&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;
		reset-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>; /* hub reset pin */
		reset-duration-us = <10>;
		clocks = <&clks IMX6SX_CLK_CKO>;

		#address-cells = <1>;
		#size-cells = <0>;
		ethernet: asix@1 {
			compatible = "usbb95,1708";
			reg = <1>;

			power-sequence;
			reset-gpios = <&gpio4 6 GPIO_ACTIVE_LOW>; /* ethernet_rst */
			reset-duration-us = <15>;
			clocks = <&clks IMX6SX_CLK_IPG>;
		};
	};
};

If the node has property "power-sequence", the pwrseq core will create
related platform device, and the driver under pwrseq driver will handle
power sequence stuffs. The property below "power-sequence" will be
handled at pwrseq driver.

-- 

Best Regards,
Peter Chen

Re: [PATCH 00/12] power: add generic power sequence framework

[Peter Chen]

On Sat, Jun 18, 2016 at 01:29:05AM +0200, Maciej S. Szmigiero wrote:
> Hi Peter,
> 
> On 17.06.2016 12:09, Peter Chen wrote:
> > Hi all,
> > 
> > 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
> > framework to handle this requirement. The generic code is supplied
> > some common helpers from host driver, and individual power sequence
> > driver handles kinds of power sequence for devices.
> > 
> > Since the MMC has already done the similar things, and this power
> > sequence handling can be generic, we use mmc power sequence code
> > as base to create this framework.
> > 
> > This patch set is based on Krzysztof Kozlowski's RFC patch set (v4.7-rc1)
> > [1], and making some changes which can let it be generic. After that,
> > we create a generic power sequence driver for USB devices which handles
> > below things, it includes all input signals for devices I can consider.
> > 
> > - Clock and its frequencies
> > - GPIO for reset and the duration time
> > - GPIO for enable
> > - Regulator for power
> > 
> > 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 udoo board changes
> > were tested using my last power sequence patch set. [2]
> > 
> > @Maciej S. Szmigiero, Oscar, would you please test them on imx6qdl udoo
> > boards?
> 
> Thanks for the patch set, just tested them on my UDOO Quad board and can
> confirm that USB and on-board hub work fine.
> 
> Tested-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
> 
> Maciej
> 

Many thanks, Maciej.

-- 

Best Regards,
Peter Chen

Re: [PATCH 08/12] doc: binding: pwrseq-usb-generic: add binding doc for generic usb power sequence driver

[Chen-Yu Tsai]

Hi,

On Mon, Jun 20, 2016 at 7:26 PM, Peter Chen <hzpeterchen@gmail.com> wrote:
> On Fri, Jun 17, 2016 at 12:16:48PM -0500, Rob Herring wrote:
>> On Fri, Jun 17, 2016 at 5:09 AM, Peter Chen <peter.chen@nxp.com> wrote:
>> > Add binding doc for generic usb power sequence driver, and update
>> > generic usb device binding-doc accordingly.
>> >
>> > Signed-off-by: Peter Chen <peter.chen@nxp.com>
>> > ---
>> >  .../bindings/power/pwrseq/pwrseq-usb-generic.txt   | 31 ++++++++++++++++++++++
>> >  .../devicetree/bindings/usb/usb-device.txt         |  2 ++
>> >  2 files changed, 33 insertions(+)
>> >  create mode 100644 Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt
>> >
>> > diff --git a/Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt
>> > new file mode 100644
>> > index 0000000..8ad98382
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt
>> > @@ -0,0 +1,31 @@
>> > +The power sequence for generic USB Devices
>> > +
>> > +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. The
>> > +power sequence will be done before this device can be found at USB
>> > +bus.
>> > +
>> > +Required properties:
>> > +- compatible : contains "usb-pwrseq-generic".
>>
>> In case I have not been clear, no.
>>
>> I am not going to accept anything along the lines of the current mmc
>> pwrseq. I am basically okay with Krzysztof's proposal as it is *only*
>> an added property and not a duplication of information. I'd suggest
>> you figure out how to make the kernel work with that rather than
>> trying to work-around whatever kernel limitations there are.
>>
>
> I see.
>
> Would you agree with below:
>
> &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;
>                 reset-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>; /* hub reset pin */
>                 reset-duration-us = <10>;
>                 clocks = <&clks IMX6SX_CLK_CKO>;
>
>                 #address-cells = <1>;
>                 #size-cells = <0>;
>                 ethernet: asix@1 {
>                         compatible = "usbb95,1708";
>                         reg = <1>;
>
>                         power-sequence;
>                         reset-gpios = <&gpio4 6 GPIO_ACTIVE_LOW>; /* ethernet_rst */
>                         reset-duration-us = <15>;
>                         clocks = <&clks IMX6SX_CLK_IPG>;
>                 };
>         };
> };
>
> If the node has property "power-sequence", the pwrseq core will create
> related platform device, and the driver under pwrseq driver will handle
> power sequence stuffs. The property below "power-sequence" will be
> handled at pwrseq driver.

Isn't this binding what Krzysztof proposed? His series also provides example
code for enabling USB device power sequencing. Does it not work for you?

Having 2 series by 2 separate people doing the same thing but slightly
different is very confusing.

Regards
ChenYu

Re: [PATCH 07/12] power: pwrseq: rename file name for generic driver

[Krzysztof Kozlowski]

On 06/17/2016 12:09 PM, Peter Chen wrote:
> The individual driver file name is better to contain module name.
> 
> Signed-off-by: Peter Chen <peter.chen@nxp.com>
> ---
>  .../power/pwrseq/{pwrseq-simple.txt => mmc-pwrseq-simple.txt}         | 0
>  drivers/power/pwrseq/Makefile                                         | 4 ++--
>  drivers/power/pwrseq/{pwrseq.c => core.c}                             | 0
>  drivers/power/pwrseq/{pwrseq_simple.c => pwrseq_mmc_simple.c}         | 0
>  4 files changed, 2 insertions(+), 2 deletions(-)
>  rename Documentation/devicetree/bindings/power/pwrseq/{pwrseq-simple.txt => mmc-pwrseq-simple.txt} (100%)
>  rename drivers/power/pwrseq/{pwrseq.c => core.c} (100%)
>  rename drivers/power/pwrseq/{pwrseq_simple.c => pwrseq_mmc_simple.c} (100%)
> 
> diff --git a/Documentation/devicetree/bindings/power/pwrseq/pwrseq-simple.txt b/Documentation/devicetree/bindings/power/pwrseq/mmc-pwrseq-simple.txt
> similarity index 100%
> rename from Documentation/devicetree/bindings/power/pwrseq/pwrseq-simple.txt
> rename to Documentation/devicetree/bindings/power/pwrseq/mmc-pwrseq-simple.txt
> diff --git a/drivers/power/pwrseq/Makefile b/drivers/power/pwrseq/Makefile
> index 9e40e4b..d475e01 100644
> --- a/drivers/power/pwrseq/Makefile
> +++ b/drivers/power/pwrseq/Makefile
> @@ -1,3 +1,3 @@
> -obj-$(CONFIG_POWER_SEQ)		+= pwrseq.o
> -obj-$(CONFIG_POWER_SEQ_SIMPLE)	+= pwrseq_simple.o
> +obj-$(CONFIG_POWER_SEQ)		+= core.o
> +obj-$(CONFIG_POWER_SEQ_SIMPLE)	+= pwrseq_mmc_simple.o

Although the driver was developed for MMC but it is quite generic (or
rather - mmc independent). The name of the driver is just
"pwrseq_simple" so I think MMC prefix is not needed.

Best regards,
Krzysztof

Re: [PATCH 08/12] doc: binding: pwrseq-usb-generic: add binding doc for generic usb power sequence driver

[Rob Herring]

On Mon, Jun 20, 2016 at 07:26:51PM +0800, Peter Chen wrote:
> On Fri, Jun 17, 2016 at 12:16:48PM -0500, Rob Herring wrote:
> > On Fri, Jun 17, 2016 at 5:09 AM, Peter Chen <peter.chen@nxp.com> wrote:
> > > Add binding doc for generic usb power sequence driver, and update
> > > generic usb device binding-doc accordingly.
> > >
> > > Signed-off-by: Peter Chen <peter.chen@nxp.com>
> > > ---
> > >  .../bindings/power/pwrseq/pwrseq-usb-generic.txt   | 31 ++++++++++++++++++++++
> > >  .../devicetree/bindings/usb/usb-device.txt         |  2 ++
> > >  2 files changed, 33 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt
> > > new file mode 100644
> > > index 0000000..8ad98382
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt
> > > @@ -0,0 +1,31 @@
> > > +The power sequence for generic USB Devices
> > > +
> > > +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. The
> > > +power sequence will be done before this device can be found at USB
> > > +bus.
> > > +
> > > +Required properties:
> > > +- compatible : contains "usb-pwrseq-generic".
> > 
> > In case I have not been clear, no.
> > 
> > I am not going to accept anything along the lines of the current mmc
> > pwrseq. I am basically okay with Krzysztof's proposal as it is *only*
> > an added property and not a duplication of information. I'd suggest
> > you figure out how to make the kernel work with that rather than
> > trying to work-around whatever kernel limitations there are.
> > 
> 
> I see.
> 
> Would you agree with below:

In general, yes. There are some points being discussed in the other 
thread.

> &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;
> 		reset-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>; /* hub reset pin */
> 		reset-duration-us = <10>;

I wonder if this really needs to be specified. A sufficiently long 
default should be good enough for 90 something % of devices.

> 		clocks = <&clks IMX6SX_CLK_CKO>;
> 
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 		ethernet: asix@1 {
> 			compatible = "usbb95,1708";
> 			reg = <1>;
> 
> 			power-sequence;
> 			reset-gpios = <&gpio4 6 GPIO_ACTIVE_LOW>; /* ethernet_rst */
> 			reset-duration-us = <15>;
> 			clocks = <&clks IMX6SX_CLK_IPG>;
> 		};
> 	};
> };
> 
> If the node has property "power-sequence", the pwrseq core will create
> related platform device, and the driver under pwrseq driver will handle
> power sequence stuffs. 

This I have issue with. If you are creating a platform device here, you 
are trying to work-around limitations in the linux driver model. Either 
we need some sort of pre-probe hook to the drivers to call or each 
parent node driver is responsible for checking and calling pwr-seq 
functions for child nodes. e.g. The host controller calls pwr-seq for 
the hub, the hub driver calls the power seq for the asix chip. Soon as 
we have a case too complex for the generic pwr-seq, we're going to need 
the pre-probe hook as I don't want to see a continual expansion of 
generic pwr-seq binding for ever more complex cases.

> The property below "power-sequence" will be
> handled at pwrseq driver.

You cannot rely on order other than for logical readability. There are 
no guarantees of either the dtb or kernel ordering of properties.

Rob

Re: [PATCH 08/12] doc: binding: pwrseq-usb-generic: add binding doc for generic usb power sequence driver

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1115 bytes --]

On Mon, Jun 20, 2016 at 11:16:07AM -0500, Rob Herring wrote:
> On Mon, Jun 20, 2016 at 07:26:51PM +0800, Peter Chen wrote:

> > If the node has property "power-sequence", the pwrseq core will create
> > related platform device, and the driver under pwrseq driver will handle
> > power sequence stuffs. 

> This I have issue with. If you are creating a platform device here, you 
> are trying to work-around limitations in the linux driver model. Either 
> we need some sort of pre-probe hook to the drivers to call or each 
> parent node driver is responsible for checking and calling pwr-seq 
> functions for child nodes. e.g. The host controller calls pwr-seq for 
> the hub, the hub driver calls the power seq for the asix chip. Soon as 
> we have a case too complex for the generic pwr-seq, we're going to need 
> the pre-probe hook as I don't want to see a continual expansion of 
> generic pwr-seq binding for ever more complex cases.

I think it's fairly clear that we need one or both of these mechanisms
for enumerable buses in embedded contexts - it's something that keeps
croping up.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 08/12] doc: binding: pwrseq-usb-generic: add binding doc for generic usb power sequence driver

[Peter Chen]

On Mon, Jun 20, 2016 at 11:16:07AM -0500, Rob Herring wrote:
> On Mon, Jun 20, 2016 at 07:26:51PM +0800, Peter Chen wrote:
> > On Fri, Jun 17, 2016 at 12:16:48PM -0500, Rob Herring wrote:
> > > On Fri, Jun 17, 2016 at 5:09 AM, Peter Chen <peter.chen@nxp.com> wrote:
> > > > Add binding doc for generic usb power sequence driver, and update
> > > > generic usb device binding-doc accordingly.
> > > >
> > > > Signed-off-by: Peter Chen <peter.chen@nxp.com>
> > > > ---
> > > >  .../bindings/power/pwrseq/pwrseq-usb-generic.txt   | 31 ++++++++++++++++++++++
> > > >  .../devicetree/bindings/usb/usb-device.txt         |  2 ++
> > > >  2 files changed, 33 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt
> > > > new file mode 100644
> > > > index 0000000..8ad98382
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt
> > > > @@ -0,0 +1,31 @@
> > > > +The power sequence for generic USB Devices
> > > > +
> > > > +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. The
> > > > +power sequence will be done before this device can be found at USB
> > > > +bus.
> > > > +
> > > > +Required properties:
> > > > +- compatible : contains "usb-pwrseq-generic".
> > > 
> > > In case I have not been clear, no.
> > > 
> > > I am not going to accept anything along the lines of the current mmc
> > > pwrseq. I am basically okay with Krzysztof's proposal as it is *only*
> > > an added property and not a duplication of information. I'd suggest
> > > you figure out how to make the kernel work with that rather than
> > > trying to work-around whatever kernel limitations there are.
> > > 
> > 
> > I see.
> > 
> > Would you agree with below:
> 
> In general, yes. There are some points being discussed in the other 
> thread.
> 
> > &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;
> > 		reset-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>; /* hub reset pin */
> > 		reset-duration-us = <10>;
> 
> I wonder if this really needs to be specified. A sufficiently long 
> default should be good enough for 90 something % of devices.
> 

This property is optional, there is a default value in pwrseq driver.

> > 		clocks = <&clks IMX6SX_CLK_CKO>;
> > 
> > 		#address-cells = <1>;
> > 		#size-cells = <0>;
> > 		ethernet: asix@1 {
> > 			compatible = "usbb95,1708";
> > 			reg = <1>;
> > 
> > 			power-sequence;
> > 			reset-gpios = <&gpio4 6 GPIO_ACTIVE_LOW>; /* ethernet_rst */
> > 			reset-duration-us = <15>;
> > 			clocks = <&clks IMX6SX_CLK_IPG>;
> > 		};
> > 	};
> > };
> > 
> > If the node has property "power-sequence", the pwrseq core will create
> > related platform device, and the driver under pwrseq driver will handle
> > power sequence stuffs. 
> 
> This I have issue with. If you are creating a platform device here, you 
> are trying to work-around limitations in the linux driver model.

My current solution like below, but it seems you didn't agree with that.
I just double confirm here, if you don't, I give up the solution for
using generic power sequence framework.

In drivers/usb/core/hub.c

	for_each_child_of_node(parent->of_node, node) {
		hdev_pwrseq = pwrseq_alloc(node, "usb_pwrseq_generic");
		if (!IS_ERR_OR_NULL(hdev_pwrseq)) {
			pwrseq_node = kzalloc(sizeof(pwrseq_node), GFP_KERNEL);
			if (!pwrseq_node) {
				ret = -ENOMEM;
				goto err1;
			}
			/* power on sequence */
			ret = pwrseq_pre_power_on(hdev_pwrseq);
			if (ret)
				goto err2;

			pwrseq_node->pwrseq_on = hdev_pwrseq;
			list_add(&pwrseq_node->list, &hub->pwrseq_list);
		} else if (IS_ERR(hdev_pwrseq)) {
			return PTR_ERR(hdev_pwrseq);
		}
	}

In drivers/power/pwrseq/core.c

struct pwrseq *pwrseq_alloc(struct device_node *np, const char *dev_name)
{
	struct pwrseq *p, *pwrseq = NULL;
	bool created;

	/* If there is no device is associated with this node, create it */
	if (!of_find_device_by_node(np)) {
		if (of_platform_device_create(np, dev_name, NULL))
			created = true;
		else
			return ERR_PTR(-ENODEV);
	}

	mutex_lock(&pwrseq_list_mutex);
	list_for_each_entry(p, &pwrseq_list, pwrseq_node) {
		if (p->dev->of_node == np) {
			if (!try_module_get(p->owner))
				dev_err(p->dev,
					"increasing module refcount failed\n");
			else
				pwrseq = p;
			break;
		}
	}

And there is a platform driver at drivers/power/pwrseq/pwrseq_usb_generic.c
to handle these pwrseq properties, see my patch 9/12.

> Either 
> we need some sort of pre-probe hook to the drivers to call or each 
> parent node driver is responsible for checking and calling pwr-seq 
> functions for child nodes. e.g. The host controller calls pwr-seq for 
> the hub, the hub driver calls the power seq for the asix chip. Soon as 
> we have a case too complex for the generic pwr-seq, we're going to need 
> the pre-probe hook as I don't want to see a continual expansion of 
> generic pwr-seq binding for ever more complex cases.
> 

How the driver know what it needs to handle (eg, gpio, clock) if there
is no device for it? The most important we need to consider is which
device owns there power sequence properties, then the corresponding
driver can handle it.

-- 

Best Regards,
Peter Chen

Re: [PATCH 08/12] doc: binding: pwrseq-usb-generic: add binding doc for generic usb power sequence driver

[Peter Chen]

On Mon, Jun 20, 2016 at 08:29:55PM +0800, Chen-Yu Tsai wrote:
> Hi,
> 
> On Mon, Jun 20, 2016 at 7:26 PM, Peter Chen <hzpeterchen@gmail.com> wrote:
> > On Fri, Jun 17, 2016 at 12:16:48PM -0500, Rob Herring wrote:
> >> On Fri, Jun 17, 2016 at 5:09 AM, Peter Chen <peter.chen@nxp.com> wrote:
> >> > Add binding doc for generic usb power sequence driver, and update
> >> > generic usb device binding-doc accordingly.
> >> >
> >> > Signed-off-by: Peter Chen <peter.chen@nxp.com>
> >> > ---
> >> >  .../bindings/power/pwrseq/pwrseq-usb-generic.txt   | 31 ++++++++++++++++++++++
> >> >  .../devicetree/bindings/usb/usb-device.txt         |  2 ++
> >> >  2 files changed, 33 insertions(+)
> >> >  create mode 100644 Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt
> >> >
> >> > diff --git a/Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt
> >> > new file mode 100644
> >> > index 0000000..8ad98382
> >> > --- /dev/null
> >> > +++ b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt
> >> > @@ -0,0 +1,31 @@
> >> > +The power sequence for generic USB Devices
> >> > +
> >> > +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. The
> >> > +power sequence will be done before this device can be found at USB
> >> > +bus.
> >> > +
> >> > +Required properties:
> >> > +- compatible : contains "usb-pwrseq-generic".
> >>
> >> In case I have not been clear, no.
> >>
> >> I am not going to accept anything along the lines of the current mmc
> >> pwrseq. I am basically okay with Krzysztof's proposal as it is *only*
> >> an added property and not a duplication of information. I'd suggest
> >> you figure out how to make the kernel work with that rather than
> >> trying to work-around whatever kernel limitations there are.
> >>
> >
> > I see.
> >
> > Would you agree with below:
> >
> > &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;
> >                 reset-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>; /* hub reset pin */
> >                 reset-duration-us = <10>;
> >                 clocks = <&clks IMX6SX_CLK_CKO>;
> >
> >                 #address-cells = <1>;
> >                 #size-cells = <0>;
> >                 ethernet: asix@1 {
> >                         compatible = "usbb95,1708";
> >                         reg = <1>;
> >
> >                         power-sequence;
> >                         reset-gpios = <&gpio4 6 GPIO_ACTIVE_LOW>; /* ethernet_rst */
> >                         reset-duration-us = <15>;
> >                         clocks = <&clks IMX6SX_CLK_IPG>;
> >                 };
> >         };
> > };
> >
> > If the node has property "power-sequence", the pwrseq core will create
> > related platform device, and the driver under pwrseq driver will handle
> > power sequence stuffs. The property below "power-sequence" will be
> > handled at pwrseq driver.
> 
> Isn't this binding what Krzysztof proposed? His series also provides example
> code for enabling USB device power sequencing. Does it not work for you?
> 

In this series, I just change RFC to formal patch, and finished his
example code. USB power sequence should be described on USB bus, not
platform bus.

-- 

Best Regards,
Peter Chen

Re: [PATCH 07/12] power: pwrseq: rename file name for generic driver

[Peter Chen]

On Mon, Jun 20, 2016 at 02:48:20PM +0200, Krzysztof Kozlowski wrote:
> On 06/17/2016 12:09 PM, Peter Chen wrote:
> > The individual driver file name is better to contain module name.
> > 
> > Signed-off-by: Peter Chen <peter.chen@nxp.com>
> > ---
> >  .../power/pwrseq/{pwrseq-simple.txt => mmc-pwrseq-simple.txt}         | 0
> >  drivers/power/pwrseq/Makefile                                         | 4 ++--
> >  drivers/power/pwrseq/{pwrseq.c => core.c}                             | 0
> >  drivers/power/pwrseq/{pwrseq_simple.c => pwrseq_mmc_simple.c}         | 0
> >  4 files changed, 2 insertions(+), 2 deletions(-)
> >  rename Documentation/devicetree/bindings/power/pwrseq/{pwrseq-simple.txt => mmc-pwrseq-simple.txt} (100%)
> >  rename drivers/power/pwrseq/{pwrseq.c => core.c} (100%)
> >  rename drivers/power/pwrseq/{pwrseq_simple.c => pwrseq_mmc_simple.c} (100%)
> > 
> > diff --git a/Documentation/devicetree/bindings/power/pwrseq/pwrseq-simple.txt b/Documentation/devicetree/bindings/power/pwrseq/mmc-pwrseq-simple.txt
> > similarity index 100%
> > rename from Documentation/devicetree/bindings/power/pwrseq/pwrseq-simple.txt
> > rename to Documentation/devicetree/bindings/power/pwrseq/mmc-pwrseq-simple.txt
> > diff --git a/drivers/power/pwrseq/Makefile b/drivers/power/pwrseq/Makefile
> > index 9e40e4b..d475e01 100644
> > --- a/drivers/power/pwrseq/Makefile
> > +++ b/drivers/power/pwrseq/Makefile
> > @@ -1,3 +1,3 @@
> > -obj-$(CONFIG_POWER_SEQ)		+= pwrseq.o
> > -obj-$(CONFIG_POWER_SEQ_SIMPLE)	+= pwrseq_simple.o
> > +obj-$(CONFIG_POWER_SEQ)		+= core.o
> > +obj-$(CONFIG_POWER_SEQ_SIMPLE)	+= pwrseq_mmc_simple.o
> 
> Although the driver was developed for MMC but it is quite generic (or
> rather - mmc independent). The name of the driver is just
> "pwrseq_simple" so I think MMC prefix is not needed.
> 

It is may be common for MMC, but not USB, or others. Eg, its reset-gpios
is just like enable-gpios. Some reset chip process needs to toggle gpio.

-- 

Best Regards,
Peter Chen

Re: [PATCH 08/12] doc: binding: pwrseq-usb-generic: add binding doc for generic usb power sequence driver

[Rob Herring]

On Tue, Jun 21, 2016 at 10:11:17AM +0800, Peter Chen wrote:
> On Mon, Jun 20, 2016 at 11:16:07AM -0500, Rob Herring wrote:
> > On Mon, Jun 20, 2016 at 07:26:51PM +0800, Peter Chen wrote:
> > > On Fri, Jun 17, 2016 at 12:16:48PM -0500, Rob Herring wrote:
> > > > On Fri, Jun 17, 2016 at 5:09 AM, Peter Chen <peter.chen@nxp.com> wrote:
> > > > > Add binding doc for generic usb power sequence driver, and update
> > > > > generic usb device binding-doc accordingly.

[...]

> > > 		clocks = <&clks IMX6SX_CLK_CKO>;
> > > 
> > > 		#address-cells = <1>;
> > > 		#size-cells = <0>;
> > > 		ethernet: asix@1 {
> > > 			compatible = "usbb95,1708";
> > > 			reg = <1>;
> > > 
> > > 			power-sequence;
> > > 			reset-gpios = <&gpio4 6 GPIO_ACTIVE_LOW>; /* ethernet_rst */
> > > 			reset-duration-us = <15>;
> > > 			clocks = <&clks IMX6SX_CLK_IPG>;
> > > 		};
> > > 	};
> > > };
> > > 
> > > If the node has property "power-sequence", the pwrseq core will create
> > > related platform device, and the driver under pwrseq driver will handle
> > > power sequence stuffs. 
> > 
> > This I have issue with. If you are creating a platform device here, you 
> > are trying to work-around limitations in the linux driver model.
> 
> My current solution like below, but it seems you didn't agree with that.
> I just double confirm here, if you don't, I give up the solution for
> using generic power sequence framework.
> 
> In drivers/usb/core/hub.c
> 
> 	for_each_child_of_node(parent->of_node, node) {
> 		hdev_pwrseq = pwrseq_alloc(node, "usb_pwrseq_generic");
> 		if (!IS_ERR_OR_NULL(hdev_pwrseq)) {
> 			pwrseq_node = kzalloc(sizeof(pwrseq_node), GFP_KERNEL);
> 			if (!pwrseq_node) {
> 				ret = -ENOMEM;
> 				goto err1;
> 			}
> 			/* power on sequence */
> 			ret = pwrseq_pre_power_on(hdev_pwrseq);

Why does this function need to do anything more than:

- Check if the child has a "power-sequence" property
- Get the "reset-gpios" GPIO
- Assert reset for specified/default time
- Deassert reset

Then continue on as normal. That seems straight-forward to me.

There is no reason you need a platform device in the mix. Perhaps trying 
to move the MMC pwr-seq code is pointless as it adds needless 
complexity.

[...]

> > Either 
> > we need some sort of pre-probe hook to the drivers to call or each 
> > parent node driver is responsible for checking and calling pwr-seq 
> > functions for child nodes. e.g. The host controller calls pwr-seq for 
> > the hub, the hub driver calls the power seq for the asix chip. Soon as 
> > we have a case too complex for the generic pwr-seq, we're going to need 
> > the pre-probe hook as I don't want to see a continual expansion of 
> > generic pwr-seq binding for ever more complex cases.
> > 
> 
> How the driver know what it needs to handle (eg, gpio, clock) if there
> is no device for it? The most important we need to consider is which
> device owns there power sequence properties, then the corresponding
> driver can handle it.

What can be handled by is defined by presence of power-sequence 
property. There can be 1 driver for the device. That is the USB hub 
driver in this example. You should not have 2 "devices".

Rob

Re: [PATCH 08/12] doc: binding: pwrseq-usb-generic: add binding doc for generic usb power sequence driver

[Peter Chen]

On Tue, Jun 21, 2016 at 04:26:53PM -0500, Rob Herring wrote:
> On Tue, Jun 21, 2016 at 10:11:17AM +0800, Peter Chen wrote:
> > On Mon, Jun 20, 2016 at 11:16:07AM -0500, Rob Herring wrote:
> > > On Mon, Jun 20, 2016 at 07:26:51PM +0800, Peter Chen wrote:
> > > > On Fri, Jun 17, 2016 at 12:16:48PM -0500, Rob Herring wrote:
> > > > > On Fri, Jun 17, 2016 at 5:09 AM, Peter Chen <peter.chen@nxp.com> wrote:
> > > > > > Add binding doc for generic usb power sequence driver, and update
> > > > > > generic usb device binding-doc accordingly.
> 
> [...]
> 
> > > > 		clocks = <&clks IMX6SX_CLK_CKO>;
> > > > 
> > > > 		#address-cells = <1>;
> > > > 		#size-cells = <0>;
> > > > 		ethernet: asix@1 {
> > > > 			compatible = "usbb95,1708";
> > > > 			reg = <1>;
> > > > 
> > > > 			power-sequence;
> > > > 			reset-gpios = <&gpio4 6 GPIO_ACTIVE_LOW>; /* ethernet_rst */
> > > > 			reset-duration-us = <15>;
> > > > 			clocks = <&clks IMX6SX_CLK_IPG>;
> > > > 		};
> > > > 	};
> > > > };
> > > > 
> > > > If the node has property "power-sequence", the pwrseq core will create
> > > > related platform device, and the driver under pwrseq driver will handle
> > > > power sequence stuffs. 
> > > 
> > > This I have issue with. If you are creating a platform device here, you 
> > > are trying to work-around limitations in the linux driver model.
> > 
> > My current solution like below, but it seems you didn't agree with that.
> > I just double confirm here, if you don't, I give up the solution for
> > using generic power sequence framework.
> > 
> > In drivers/usb/core/hub.c
> > 
> > 	for_each_child_of_node(parent->of_node, node) {
> > 		hdev_pwrseq = pwrseq_alloc(node, "usb_pwrseq_generic");
> > 		if (!IS_ERR_OR_NULL(hdev_pwrseq)) {
> > 			pwrseq_node = kzalloc(sizeof(pwrseq_node), GFP_KERNEL);
> > 			if (!pwrseq_node) {
> > 				ret = -ENOMEM;
> > 				goto err1;
> > 			}
> > 			/* power on sequence */
> > 			ret = pwrseq_pre_power_on(hdev_pwrseq);
> 
> Why does this function need to do anything more than:
> 
> - Check if the child has a "power-sequence" property
> - Get the "reset-gpios" GPIO
> - Assert reset for specified/default time
> - Deassert reset
> 

Besides gpios, it may exist clock and regulator operation, and the
operation may be failed. I thought these operations can be easy
done belongs to a device, but now, let me try this straight-forward
way, thanks.

Peter
> Then continue on as normal. That seems straight-forward to me.
> 
> There is no reason you need a platform device in the mix. Perhaps trying 
> to move the MMC pwr-seq code is pointless as it adds needless 
> complexity.
> 
> [...]
> 
> > > Either 
> > > we need some sort of pre-probe hook to the drivers to call or each 
> > > parent node driver is responsible for checking and calling pwr-seq 
> > > functions for child nodes. e.g. The host controller calls pwr-seq for 
> > > the hub, the hub driver calls the power seq for the asix chip. Soon as 
> > > we have a case too complex for the generic pwr-seq, we're going to need 
> > > the pre-probe hook as I don't want to see a continual expansion of 
> > > generic pwr-seq binding for ever more complex cases.
> > > 
> > 
> > How the driver know what it needs to handle (eg, gpio, clock) if there
> > is no device for it? The most important we need to consider is which
> > device owns there power sequence properties, then the corresponding
> > driver can handle it.
> 
> What can be handled by is defined by presence of power-sequence 
> property. There can be 1 driver for the device. That is the USB hub 
> driver in this example. You should not have 2 "devices".
> 
> Rob

-- 

Best Regards,
Peter Chen

Re: [PATCH 08/12] doc: binding: pwrseq-usb-generic: add binding doc for generic usb power sequence driver

[Ulf Hansson]

On 21 June 2016 at 23:26, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Tue, Jun 21, 2016 at 10:11:17AM +0800, Peter Chen wrote:
>> On Mon, Jun 20, 2016 at 11:16:07AM -0500, Rob Herring wrote:
>> > On Mon, Jun 20, 2016 at 07:26:51PM +0800, Peter Chen wrote:
>> > > On Fri, Jun 17, 2016 at 12:16:48PM -0500, Rob Herring wrote:
>> > > > On Fri, Jun 17, 2016 at 5:09 AM, Peter Chen <peter.chen-3arQi8VN3Tc@public.gmane.org> wrote:
>> > > > > Add binding doc for generic usb power sequence driver, and update
>> > > > > generic usb device binding-doc accordingly.
>
> [...]
>
>> > >           clocks = <&clks IMX6SX_CLK_CKO>;
>> > >
>> > >           #address-cells = <1>;
>> > >           #size-cells = <0>;
>> > >           ethernet: asix@1 {
>> > >                   compatible = "usbb95,1708";
>> > >                   reg = <1>;
>> > >
>> > >                   power-sequence;
>> > >                   reset-gpios = <&gpio4 6 GPIO_ACTIVE_LOW>; /* ethernet_rst */
>> > >                   reset-duration-us = <15>;
>> > >                   clocks = <&clks IMX6SX_CLK_IPG>;
>> > >           };
>> > >   };
>> > > };
>> > >
>> > > If the node has property "power-sequence", the pwrseq core will create
>> > > related platform device, and the driver under pwrseq driver will handle
>> > > power sequence stuffs.
>> >
>> > This I have issue with. If you are creating a platform device here, you
>> > are trying to work-around limitations in the linux driver model.

I somewhat understand your point.

Although, having the option to use a driver (which requires a device)
has turned out to be quite convenient from many aspects - at least in
the mmc case.

Certainly one can do without it, but in the end using a driver avoids
open coding.

>>
>> My current solution like below, but it seems you didn't agree with that.
>> I just double confirm here, if you don't, I give up the solution for
>> using generic power sequence framework.
>>
>> In drivers/usb/core/hub.c
>>
>>       for_each_child_of_node(parent->of_node, node) {
>>               hdev_pwrseq = pwrseq_alloc(node, "usb_pwrseq_generic");
>>               if (!IS_ERR_OR_NULL(hdev_pwrseq)) {
>>                       pwrseq_node = kzalloc(sizeof(pwrseq_node), GFP_KERNEL);
>>                       if (!pwrseq_node) {
>>                               ret = -ENOMEM;
>>                               goto err1;
>>                       }
>>                       /* power on sequence */
>>                       ret = pwrseq_pre_power_on(hdev_pwrseq);
>
> Why does this function need to do anything more than:
>
> - Check if the child has a "power-sequence" property
> - Get the "reset-gpios" GPIO
> - Assert reset for specified/default time
> - Deassert reset
>
> Then continue on as normal. That seems straight-forward to me.
>
> There is no reason you need a platform device in the mix. Perhaps trying
> to move the MMC pwr-seq code is pointless as it adds needless
> complexity.

Complexity?

The problem we are tying to solve, is to make the various platform/SoC
specific power sequences to be able to live in generic drivers.

One could decide to encode the sequences inside the driver code
itself, but it will soon turn into a mess and more importantly, lots
of open coding as to support different platforms/SoCs. To most kernel
hackers I don't think this is an option to consider.

The MMC pwr-seq code/drivers tries to address these issues - in a
somewhat generic way.
Initially we have decided to start with only a few flavours of
supported sequences and so far these have been sufficient.

Finally, I am indeed concerned that it so hard to agree on a solution
to deal with generic power sequences. There have been many attempts
throughout the last ~4-5 years, but peoples strong opinions about
different approaches mad them all fail. Isn't it time to finally just
pick a solution and stick with it, even if it doesn't meet all peoples
expectations?

[...]

Kind regards
Uffe
--
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

Re: [PATCH 08/12] doc: binding: pwrseq-usb-generic: add binding doc for generic usb power sequence driver

[Rob Herring]

On Wed, Jun 22, 2016 at 4:09 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 21 June 2016 at 23:26, Rob Herring <robh@kernel.org> wrote:
>> On Tue, Jun 21, 2016 at 10:11:17AM +0800, Peter Chen wrote:
>>> On Mon, Jun 20, 2016 at 11:16:07AM -0500, Rob Herring wrote:
>>> > On Mon, Jun 20, 2016 at 07:26:51PM +0800, Peter Chen wrote:
>>> > > On Fri, Jun 17, 2016 at 12:16:48PM -0500, Rob Herring wrote:
>>> > > > On Fri, Jun 17, 2016 at 5:09 AM, Peter Chen <peter.chen@nxp.com> wrote:
>>> > > > > Add binding doc for generic usb power sequence driver, and update
>>> > > > > generic usb device binding-doc accordingly.
>>
>> [...]
>>
>>> > >           clocks = <&clks IMX6SX_CLK_CKO>;
>>> > >
>>> > >           #address-cells = <1>;
>>> > >           #size-cells = <0>;
>>> > >           ethernet: asix@1 {
>>> > >                   compatible = "usbb95,1708";
>>> > >                   reg = <1>;
>>> > >
>>> > >                   power-sequence;
>>> > >                   reset-gpios = <&gpio4 6 GPIO_ACTIVE_LOW>; /* ethernet_rst */
>>> > >                   reset-duration-us = <15>;
>>> > >                   clocks = <&clks IMX6SX_CLK_IPG>;
>>> > >           };
>>> > >   };
>>> > > };
>>> > >
>>> > > If the node has property "power-sequence", the pwrseq core will create
>>> > > related platform device, and the driver under pwrseq driver will handle
>>> > > power sequence stuffs.
>>> >
>>> > This I have issue with. If you are creating a platform device here, you
>>> > are trying to work-around limitations in the linux driver model.
>
> I somewhat understand your point.
>
> Although, having the option to use a driver (which requires a device)
> has turned out to be quite convenient from many aspects - at least in
> the mmc case.
>
> Certainly one can do without it, but in the end using a driver avoids
> open coding.

Why would it be open coded? Just create library functions to parse the
node and implement the generic pwr-seq steps.

>>> My current solution like below, but it seems you didn't agree with that.
>>> I just double confirm here, if you don't, I give up the solution for
>>> using generic power sequence framework.
>>>
>>> In drivers/usb/core/hub.c
>>>
>>>       for_each_child_of_node(parent->of_node, node) {
>>>               hdev_pwrseq = pwrseq_alloc(node, "usb_pwrseq_generic");
>>>               if (!IS_ERR_OR_NULL(hdev_pwrseq)) {
>>>                       pwrseq_node = kzalloc(sizeof(pwrseq_node), GFP_KERNEL);
>>>                       if (!pwrseq_node) {
>>>                               ret = -ENOMEM;
>>>                               goto err1;
>>>                       }
>>>                       /* power on sequence */
>>>                       ret = pwrseq_pre_power_on(hdev_pwrseq);
>>
>> Why does this function need to do anything more than:
>>
>> - Check if the child has a "power-sequence" property
>> - Get the "reset-gpios" GPIO
>> - Assert reset for specified/default time
>> - Deassert reset
>>
>> Then continue on as normal. That seems straight-forward to me.
>>
>> There is no reason you need a platform device in the mix. Perhaps trying
>> to move the MMC pwr-seq code is pointless as it adds needless
>> complexity.
>
> Complexity?
>
> The problem we are tying to solve, is to make the various platform/SoC
> specific power sequences to be able to live in generic drivers.
>
> One could decide to encode the sequences inside the driver code
> itself, but it will soon turn into a mess and more importantly, lots
> of open coding as to support different platforms/SoCs. To most kernel
> hackers I don't think this is an option to consider.

I'm not proposing open coding, but having generic library functions.
I'm not saying the mmc pwr-seq has to change from being a driver
either. Leave it as is. I'm only talking about Krzysztof's new
proposal.

> The MMC pwr-seq code/drivers tries to address these issues - in a
> somewhat generic way.
> Initially we have decided to start with only a few flavours of
> supported sequences and so far these have been sufficient.

Only a few because we've pushed back against defining state machines in DT.

> Finally, I am indeed concerned that it so hard to agree on a solution
> to deal with generic power sequences. There have been many attempts
> throughout the last ~4-5 years, but peoples strong opinions about
> different approaches mad them all fail. Isn't it time to finally just
> pick a solution and stick with it, even if it doesn't meet all peoples
> expectations?

No. I'm pretty that is not how kernel development works. Features get
merged when all are happy (or not paying attention).

>From what I recall, all attempts have worked around the problem that
the driver model has no way to either force probe or provide a
pre-probe hook on probeable buses. This then leads to the work-around
defining the DT binding.

Rob