Hi all,
This is a follow-up for my last power sequence framework patch set [1].
According to Rob Herring and Ulf Hansson's comments[2]. The kinds of
power sequence instances will be added at postcore_initcall, the match
criteria is compatible string first, if the compatible string is not
matched between dts and library, it will try to use generic power sequence.
The host driver just needs to call of_pwrseq_on/of_pwrseq_off
if only one power sequence instance is needed, for more power sequences
are used, using of_pwrseq_on_list/of_pwrseq_off_list instead (eg, USB hub driver).
In future, if there are special power sequence requirements, the special
power sequence library can be created.
This patch set is tested on i.mx6 sabresx evk using a dts change, I use
two hot-plug devices to simulate this use case, the related binding
change is updated at patch [1/6], The udoo board changes were tested
using my last power sequence patch set.[3]
Except for hard-wired MMC and USB devices, I find the USB ULPI PHY also
need to power on itself before it can be found by ULPI bus.
[1] http://www.spinics.net/lists/linux-usb/msg142755.html
[2] http://www.spinics.net/lists/linux-usb/msg143106.html
[3] http://www.spinics.net/lists/linux-usb/msg142815.html
Changes for v10:
- Improve the kernel-doc for power sequence core, including exported APIs and
main structure. [Patch 2/8]
- Change Kconfig, and let the user choose power sequence. [Patch 2/8]
- Delete EXPORT_SYMBOL and change related APIs as local, these APIs do not
be intended to export currently. [Patch 2/8]
- Selete POWER_SEQUENCE at USB core's Kconfig. [Patch 4/8]
Changes for v9:
- Add Vaibhav Hiremath's reviewed-by [Patch 4/8]
- Rebase to v4.9-rc1
Changes for v8:
- Allocate one extra pwrseq instance if pwrseq_get has succeed, it can avoid
preallocate instances problem which the number of instance is decided at
compile time, thanks for Heiko Stuebner's suggestion [Patch 2/8]
- Delete pwrseq_compatible_sample.c which is the demo purpose to show compatible
match method. [Patch 2/8]
- Add Maciej S. Szmigiero's tested-by. [Patch 7/8]
Changes for v7:
- Create kinds of power sequence instance at postcore_initcall, and match
the instance with node using compatible string, the beneit of this is
the host driver doesn't need to consider which pwrseq instance needs
to be used, and pwrseq core will match it, however, it eats some memories
if less power sequence instances are used. [Patch 2/8]
- Add pwrseq_compatible_sample.c to test match pwrseq using device_id. [Patch 2/8]
- Fix the comments Vaibhav Hiremath adds for error path for clock and do not
use device_node for parameters at pwrseq_on. [Patch 2/8]
- Simplify the caller to use power sequence, follows Alan's commnets [Patch 4/8]
- Tested three pwrseq instances together using both specific compatible string and
generic libraries.
Changes for v6:
- Add Matthias Kaehlcke's Reviewed-by and Tested-by. (patch [2/6])
- Change chipidea core of_node assignment for coming user. (patch [5/6])
- Applies Joshua Clayton's three dts changes for two boards,
the USB device's reg has only #address-cells, but without #size-cells.
Changes for v5:
- Delete pwrseq_register/pwrseq_unregister, which is useless currently
- Fix the linker error when the pwrseq user is compiled as module
Changes for v4:
- Create the patch on next-20160722
- Fix the of_node is not NULL after chipidea driver is unbinded [Patch 5/6]
- Using more friendly wait method for reset gpio [Patch 2/6]
- Support multiple input clocks [Patch 2/6]
- Add Rob Herring's ack for DT changes
- Add Joshua Clayton's Tested-by
Changes for v3:
- Delete "power-sequence" property at binding-doc, and change related code
at both library and user code.
- Change binding-doc example node name with Rob's comments
- of_get_named_gpio_flags only gets the gpio, but without setting gpio flags,
add additional code request gpio with proper gpio flags
- Add Philipp Zabel's Ack and MAINTAINER's entry
Changes for v2:
- Delete "pwrseq" prefix and clock-names for properties at dt binding
- Should use structure not but its pointer for kzalloc
- Since chipidea core has no of_node, let core's of_node equals glue
layer's at core's probe
Joshua Clayton (2):
ARM: dts: imx6qdl: Enable usb node children with <reg>
ARM: dts: imx6q-evi: Fix onboard hub reset line
Peter Chen (6):
binding-doc: power: pwrseq-generic: add binding doc for generic power
sequence library
power: add power sequence library
binding-doc: usb: usb-device: add optional properties for power
sequence
usb: core: add power sequence handling for USB devices
usb: chipidea: let chipidea core device of_node equal's glue layer
device of_node
ARM: dts: imx6qdl-udoo.dtsi: fix onboard USB HUB property
.../bindings/power/pwrseq/pwrseq-generic.txt | 48 +++++
.../devicetree/bindings/usb/usb-device.txt | 10 +-
MAINTAINERS | 9 +
arch/arm/boot/dts/imx6q-evi.dts | 25 +--
arch/arm/boot/dts/imx6qdl-udoo.dtsi | 26 ++-
arch/arm/boot/dts/imx6qdl.dtsi | 6 +
drivers/power/Kconfig | 1 +
drivers/power/Makefile | 1 +
drivers/power/pwrseq/Kconfig | 21 ++
drivers/power/pwrseq/Makefile | 2 +
drivers/power/pwrseq/core.c | 237 +++++++++++++++++++++
drivers/power/pwrseq/pwrseq_generic.c | 183 ++++++++++++++++
drivers/usb/Kconfig | 1 +
drivers/usb/chipidea/core.c | 27 ++-
drivers/usb/core/hub.c | 41 +++-
drivers/usb/core/hub.h | 1 +
include/linux/power/pwrseq.h | 60 ++++++
17 files changed, 658 insertions(+), 41 deletions(-)
create mode 100644 Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
create mode 100644 drivers/power/pwrseq/Kconfig
create mode 100644 drivers/power/pwrseq/Makefile
create mode 100644 drivers/power/pwrseq/core.c
create mode 100644 drivers/power/pwrseq/pwrseq_generic.c
create mode 100644 include/linux/power/pwrseq.h
--
2.7.4
Add binding doc for generic power sequence library.
Signed-off-by: Peter Chen <[email protected]>
Acked-by: Philipp Zabel <[email protected]>
Acked-by: Rob Herring <[email protected]>
---
.../bindings/power/pwrseq/pwrseq-generic.txt | 48 ++++++++++++++++++++++
1 file changed, 48 insertions(+)
create mode 100644 Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
diff --git a/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
new file mode 100644
index 0000000..ebf0d47
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
@@ -0,0 +1,48 @@
+The generic power sequence library
+
+Some hard-wired devices (eg USB/MMC) need to do power sequence before
+the device can be enumerated on the bus, the typical power sequence
+like: enable USB PHY clock, toggle reset pin, etc. But current
+Linux device driver lacks of such code to do it, it may cause some
+hard-wired devices works abnormal or can't be recognized by
+controller at all. The power sequence will be done before this device
+can be found at the bus.
+
+The power sequence properties is under the device node.
+
+Optional properties:
+- clocks: the input clocks for device.
+- reset-gpios: Should specify the GPIO for reset.
+- reset-duration-us: the duration in microsecond for assert reset signal.
+
+Below is the example of USB power sequence properties on USB device
+nodes which have two level USB hubs.
+
+&usbotg1 {
+ vbus-supply = <®_usb_otg1_vbus>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_usb_otg1_id>;
+ status = "okay";
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+ genesys: hub@1 {
+ compatible = "usb5e3,608";
+ reg = <1>;
+
+ clocks = <&clks IMX6SX_CLK_CKO>;
+ reset-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>; /* hub reset pin */
+ reset-duration-us = <10>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+ asix: ethernet@1 {
+ compatible = "usbb95,1708";
+ reg = <1>;
+
+ clocks = <&clks IMX6SX_CLK_IPG>;
+ reset-gpios = <&gpio4 6 GPIO_ACTIVE_LOW>; /* ethernet_rst */
+ reset-duration-us = <15>;
+ };
+ };
+};
--
2.7.4
Add optional properties for power sequence.
Signed-off-by: Peter Chen <[email protected]>
Acked-by: Rob Herring <[email protected]>
---
Documentation/devicetree/bindings/usb/usb-device.txt | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/usb/usb-device.txt b/Documentation/devicetree/bindings/usb/usb-device.txt
index 1c35e7b..3661dd2 100644
--- a/Documentation/devicetree/bindings/usb/usb-device.txt
+++ b/Documentation/devicetree/bindings/usb/usb-device.txt
@@ -13,6 +13,10 @@ Required properties:
- reg: the port number which this device is connecting to, the range
is 1-31.
+Optional properties:
+power sequence properties, see
+Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt for detail
+
Example:
&usb1 {
@@ -21,8 +25,12 @@ Example:
#address-cells = <1>;
#size-cells = <0>;
- hub: genesys@1 {
+ genesys: hub@1 {
compatible = "usb5e3,608";
reg = <1>;
+
+ clocks = <&clks IMX6SX_CLK_CKO>;
+ reset-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>; /* hub reset pin */
+ reset-duration-us = <10>;
};
}
--
2.7.4
From: Peter Chen <[email protected]>
At device tree, we have no device node for chipidea core,
the glue layer's node is the parent node for host and udc
device. But in related driver, the parent device is chipidea
core. So, in order to let the common driver get parent's node,
we let the core's device node equals glue layer device node.
Signed-off-by: Peter Chen <[email protected]>
Tested-by: Maciej S. Szmigiero <[email protected]>
Tested-by Joshua Clayton <[email protected]>
---
drivers/usb/chipidea/core.c | 27 ++++++++++++++++++++++-----
1 file changed, 22 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 69426e6..6839e19 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -927,6 +927,16 @@ static int ci_hdrc_probe(struct platform_device *pdev)
return -ENODEV;
}
+ /*
+ * At device tree, we have no device node for chipidea core,
+ * the glue layer's node is the parent node for host and udc
+ * device. But in related driver, the parent device is chipidea
+ * core. So, in order to let the common driver get parent's node,
+ * we let the core's device node equals glue layer's node.
+ */
+ if (dev->parent && dev->parent->of_node)
+ dev->of_node = dev->parent->of_node;
+
if (ci->platdata->phy) {
ci->phy = ci->platdata->phy;
} else if (ci->platdata->usb_phy) {
@@ -937,11 +947,15 @@ static int ci_hdrc_probe(struct platform_device *pdev)
/* if both generic PHY and USB PHY layers aren't enabled */
if (PTR_ERR(ci->phy) == -ENOSYS &&
- PTR_ERR(ci->usb_phy) == -ENXIO)
- return -ENXIO;
+ PTR_ERR(ci->usb_phy) == -ENXIO) {
+ ret = -ENXIO;
+ goto clear_of_node;
+ }
- if (IS_ERR(ci->phy) && IS_ERR(ci->usb_phy))
- return -EPROBE_DEFER;
+ if (IS_ERR(ci->phy) && IS_ERR(ci->usb_phy)) {
+ ret = -EPROBE_DEFER;
+ goto clear_of_node;
+ }
if (IS_ERR(ci->phy))
ci->phy = NULL;
@@ -952,7 +966,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
ret = ci_usb_phy_init(ci);
if (ret) {
dev_err(dev, "unable to init phy: %d\n", ret);
- return ret;
+ goto clear_of_node;
}
ci->hw_bank.phys = res->start;
@@ -1058,6 +1072,8 @@ static int ci_hdrc_probe(struct platform_device *pdev)
ci_role_destroy(ci);
deinit_phy:
ci_usb_phy_exit(ci);
+clear_of_node:
+ dev->of_node = NULL;
return ret;
}
@@ -1076,6 +1092,7 @@ static int ci_hdrc_remove(struct platform_device *pdev)
ci_extcon_unregister(ci);
ci_role_destroy(ci);
ci_hdrc_enter_lpm(ci, true);
+ ci->dev->of_node = NULL;
ci_usb_phy_exit(ci);
return 0;
--
2.7.4
The current dts describes USB HUB's property at USB controller's
entry, it is improper. The USB HUB should be the child node
under USB controller, and power sequence properties are under
it. Besides, using gpio pinctrl setting for USB2415's reset pin.
Signed-off-by: Peter Chen <[email protected]>
Signed-off-by: Joshua Clayton <[email protected]>
Tested-by: Maciej S. Szmigiero <[email protected]>
---
arch/arm/boot/dts/imx6qdl-udoo.dtsi | 26 ++++++++++++--------------
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/arch/arm/boot/dts/imx6qdl-udoo.dtsi b/arch/arm/boot/dts/imx6qdl-udoo.dtsi
index c96c91d..a173de2 100644
--- a/arch/arm/boot/dts/imx6qdl-udoo.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-udoo.dtsi
@@ -9,6 +9,8 @@
*
*/
+#include <dt-bindings/gpio/gpio.h>
+
/ {
aliases {
backlight = &backlight;
@@ -58,17 +60,6 @@
#address-cells = <1>;
#size-cells = <0>;
- reg_usb_h1_vbus: regulator@0 {
- compatible = "regulator-fixed";
- reg = <0>;
- regulator-name = "usb_h1_vbus";
- regulator-min-microvolt = <5000000>;
- regulator-max-microvolt = <5000000>;
- enable-active-high;
- startup-delay-us = <2>; /* USB2415 requires a POR of 1 us minimum */
- gpio = <&gpio7 12 0>;
- };
-
reg_panel: regulator@1 {
compatible = "regulator-fixed";
reg = <1>;
@@ -188,7 +179,7 @@
pinctrl_usbh: usbhgrp {
fsl,pins = <
- MX6QDL_PAD_GPIO_17__GPIO7_IO12 0x80000000
+ MX6QDL_PAD_GPIO_17__GPIO7_IO12 0x1b0b0
MX6QDL_PAD_NANDF_CS2__CCM_CLKO2 0x130b0
>;
};
@@ -259,9 +250,16 @@
&usbh1 {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_usbh>;
- vbus-supply = <®_usb_h1_vbus>;
- clocks = <&clks IMX6QDL_CLK_CKO>;
status = "okay";
+
+ usb2415: hub@1 {
+ compatible = "usb424,2514";
+ reg = <1>;
+
+ clocks = <&clks IMX6QDL_CLK_CKO>;
+ reset-gpios = <&gpio7 12 GPIO_ACTIVE_LOW>;
+ reset-duration-us = <3000>;
+ };
};
&usdhc3 {
--
2.7.4
From: Joshua Clayton <[email protected]>
Give usb nodes #address and #size attributes, so that a child node
representing a permanently connected device such as an onboard hub may
be addressed with a <reg> attribute
Signed-off-by: Joshua Clayton <[email protected]>
Signed-off-by: Peter Chen <[email protected]>
---
arch/arm/boot/dts/imx6qdl.dtsi | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index b13b0b2..8fc66e1 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -935,6 +935,8 @@
usbh1: usb@02184200 {
compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
+ #address-cells = <1>;
+ #size-cells = <0>;
reg = <0x02184200 0x200>;
interrupts = <0 40 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clks IMX6QDL_CLK_USBOH3>;
@@ -949,6 +951,8 @@
usbh2: usb@02184400 {
compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
+ #address-cells = <1>;
+ #size-cells = <0>;
reg = <0x02184400 0x200>;
interrupts = <0 41 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clks IMX6QDL_CLK_USBOH3>;
@@ -962,6 +966,8 @@
usbh3: usb@02184600 {
compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
+ #address-cells = <1>;
+ #size-cells = <0>;
reg = <0x02184600 0x200>;
interrupts = <0 42 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clks IMX6QDL_CLK_USBOH3>;
--
2.7.4
We have an well-known problem that the device needs to do some power
sequence before it can be recognized by related host, the typical
example like hard-wired mmc devices and usb devices.
This power sequence is hard to be described at device tree and handled by
related host driver, so we have created a common power sequence
library to cover this requirement. The core code has supplied
some common helpers for host driver, and individual power sequence
libraries handle kinds of power sequence for devices. The pwrseq
librares always need to allocate extra instance for compatible
string match.
pwrseq_generic is intended for general purpose of power sequence, which
handles gpios and clocks currently, and can cover other controls in
future. The host driver just needs to call of_pwrseq_on/of_pwrseq_off
if only one power sequence is needed, else call of_pwrseq_on_list
/of_pwrseq_off_list instead (eg, USB hub driver).
For new power sequence library, it can add its compatible string
to pwrseq_of_match_table, then the pwrseq core will match it with
DT's, and choose this library at runtime.
Signed-off-by: Peter Chen <[email protected]>
Tested-by: Maciej S. Szmigiero <[email protected]>
Tested-by Joshua Clayton <[email protected]>
Reviewed-by: Matthias Kaehlcke <[email protected]>
Tested-by: Matthias Kaehlcke <[email protected]>
---
MAINTAINERS | 9 ++
drivers/power/Kconfig | 1 +
drivers/power/Makefile | 1 +
drivers/power/pwrseq/Kconfig | 21 +++
drivers/power/pwrseq/Makefile | 2 +
drivers/power/pwrseq/core.c | 237 ++++++++++++++++++++++++++++++++++
drivers/power/pwrseq/pwrseq_generic.c | 183 ++++++++++++++++++++++++++
include/linux/power/pwrseq.h | 60 +++++++++
8 files changed, 514 insertions(+)
create mode 100644 drivers/power/pwrseq/Kconfig
create mode 100644 drivers/power/pwrseq/Makefile
create mode 100644 drivers/power/pwrseq/core.c
create mode 100644 drivers/power/pwrseq/pwrseq_generic.c
create mode 100644 include/linux/power/pwrseq.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 3d838cf..066b1e4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9621,6 +9621,15 @@ F: include/linux/pm_*
F: include/linux/powercap.h
F: drivers/powercap/
+POWER SEQUENCE LIBRARY
+M: Peter Chen <[email protected]>
+T: git git://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git
+L: [email protected]
+S: Maintained
+F: Documentation/devicetree/bindings/power/pwrseq/
+F: drivers/power/pwrseq/
+F: include/linux/power/pwrseq.h/
+
POWER SUPPLY CLASS/SUBSYSTEM and DRIVERS
M: Sebastian Reichel <[email protected]>
L: [email protected]
diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 63454b5..c1bb046 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -1,3 +1,4 @@
source "drivers/power/avs/Kconfig"
source "drivers/power/reset/Kconfig"
source "drivers/power/supply/Kconfig"
+source "drivers/power/pwrseq/Kconfig"
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index ff35c71..7db8035 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -1,3 +1,4 @@
obj-$(CONFIG_POWER_AVS) += avs/
obj-$(CONFIG_POWER_RESET) += reset/
obj-$(CONFIG_POWER_SUPPLY) += supply/
+obj-$(CONFIG_POWER_SEQUENCE) += pwrseq/
diff --git a/drivers/power/pwrseq/Kconfig b/drivers/power/pwrseq/Kconfig
new file mode 100644
index 0000000..88f5597
--- /dev/null
+++ b/drivers/power/pwrseq/Kconfig
@@ -0,0 +1,21 @@
+#
+# Power Sequence library
+#
+
+menuconfig POWER_SEQUENCE
+ bool "Power sequence control"
+ depends on OF
+ help
+ It is used for drivers which needs to do power sequence
+ (eg, turn on clock, toggle reset gpio) before the related
+ devices can be found by hardware, eg, USB bus.
+
+if POWER_SEQUENCE
+
+config PWRSEQ_GENERIC
+ bool "Generic power sequence control"
+ default y
+ help
+ This is the generic power sequence control library, and is
+ supposed to support common power sequence usage.
+endif
diff --git a/drivers/power/pwrseq/Makefile b/drivers/power/pwrseq/Makefile
new file mode 100644
index 0000000..ad82389
--- /dev/null
+++ b/drivers/power/pwrseq/Makefile
@@ -0,0 +1,2 @@
+obj-$(CONFIG_POWER_SEQUENCE) += core.o
+obj-$(CONFIG_PWRSEQ_GENERIC) += pwrseq_generic.o
diff --git a/drivers/power/pwrseq/core.c b/drivers/power/pwrseq/core.c
new file mode 100644
index 0000000..e3c1fbb
--- /dev/null
+++ b/drivers/power/pwrseq/core.c
@@ -0,0 +1,237 @@
+/*
+ * core.c power sequence core file
+ *
+ * Copyright (C) 2016 Freescale Semiconductor, Inc.
+ * Author: Peter Chen <[email protected]>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 of
+ * the License as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/list.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/slab.h>
+#include <linux/power/pwrseq.h>
+
+static DEFINE_MUTEX(pwrseq_list_mutex);
+static LIST_HEAD(pwrseq_list);
+
+static int pwrseq_get(struct device_node *np, struct pwrseq *p)
+{
+ if (p && p->get)
+ return p->get(np, p);
+
+ return -ENOTSUPP;
+}
+
+static int pwrseq_on(struct pwrseq *p)
+{
+ if (p && p->on)
+ return p->on(p);
+
+ return -ENOTSUPP;
+}
+
+static void pwrseq_off(struct pwrseq *p)
+{
+ if (p && p->off)
+ p->off(p);
+}
+
+static void pwrseq_put(struct pwrseq *p)
+{
+ if (p && p->put)
+ p->put(p);
+}
+
+static int pwrseq_suspend(struct pwrseq *p)
+{
+ if (p && p->suspend)
+ return p->suspend(p);
+
+ return 0;
+}
+
+static int pwrseq_resume(struct pwrseq *p)
+{
+ if (p && p->resume)
+ return p->resume(p);
+
+ return 0;
+}
+
+/**
+ * pwrseq_register: add pwrseq instance to global pwrseq list
+ *
+ * @pwrseq: the pwrseq instance
+ */
+void pwrseq_register(struct pwrseq *pwrseq)
+{
+ mutex_lock(&pwrseq_list_mutex);
+ list_add(&pwrseq->node, &pwrseq_list);
+ mutex_unlock(&pwrseq_list_mutex);
+}
+EXPORT_SYMBOL_GPL(pwrseq_register);
+
+/**
+ * pwrseq_unregister: remove pwrseq instance from global pwrseq list
+ *
+ * @pwrseq: the pwrseq instance
+ */
+void pwrseq_unregister(struct pwrseq *pwrseq)
+{
+ mutex_lock(&pwrseq_list_mutex);
+ list_del(&pwrseq->node);
+ mutex_unlock(&pwrseq_list_mutex);
+}
+EXPORT_SYMBOL_GPL(pwrseq_unregister);
+
+static struct pwrseq *pwrseq_find_available_instance(struct device_node *np)
+{
+ struct pwrseq *pwrseq;
+
+ list_for_each_entry(pwrseq, &pwrseq_list, node) {
+ if (pwrseq->used)
+ continue;
+
+ /* compare compatible string for pwrseq node */
+ if (of_match_node(pwrseq->pwrseq_of_match_table, np)) {
+ pwrseq->used = true;
+ return pwrseq;
+ }
+
+ /* return generic pwrseq instance */
+ if (!strcmp(pwrseq->pwrseq_of_match_table->compatible,
+ "generic")) {
+ pr_debug("using generic pwrseq instance for %s\n",
+ np->full_name);
+ pwrseq->used = true;
+ return pwrseq;
+ }
+ }
+ pr_warn("Can't find any pwrseq instances for %s\n", np->full_name);
+
+ return NULL;
+}
+
+/**
+ * of_pwrseq_on: do power sequence on for device node
+ *
+ * This API is used to power on single device, if the host
+ * controller only needs to handle one child device (this device
+ * node points to), use this API. If multiply devices are needed
+ * to handle on bus, use of_pwrseq_on_list.
+ *
+ * @np: the device node would like to power on
+ *
+ * On successful, it returns pwrseq instance, otherwise an error value.
+ */
+struct pwrseq *of_pwrseq_on(struct device_node *np)
+{
+ struct pwrseq *pwrseq;
+ int ret;
+
+ pwrseq = pwrseq_find_available_instance(np);
+ if (!pwrseq)
+ return ERR_PTR(-ENONET);
+
+ ret = pwrseq_get(np, pwrseq);
+ if (ret) {
+ /* Mark current pwrseq as unused */
+ pwrseq->used = false;
+ return ERR_PTR(ret);
+ }
+
+ ret = pwrseq_on(pwrseq);
+ if (ret)
+ goto pwr_put;
+
+ return pwrseq;
+
+pwr_put:
+ pwrseq_put(pwrseq);
+ return ERR_PTR(ret);
+}
+EXPORT_SYMBOL_GPL(of_pwrseq_on);
+
+/**
+ * of_pwrseq_off: do power sequence off for this pwrseq instance
+ *
+ * This API is used to power off single device, it is the opposite
+ * operation for of_pwrseq_on.
+ *
+ * @pwrseq: the pwrseq instance which related device would like to be off
+ */
+void of_pwrseq_off(struct pwrseq *pwrseq)
+{
+ pwrseq_off(pwrseq);
+ pwrseq_put(pwrseq);
+}
+EXPORT_SYMBOL_GPL(of_pwrseq_off);
+
+/**
+ * of_pwrseq_on_list: do power sequence on for list
+ *
+ * This API is used to power on multiple devices at single bus.
+ * If there are several devices on bus (eg, USB bus), uses this
+ * this API. Otherwise, use of_pwrseq_on. After the device
+ * is powered on successfully, it will be added to pwrseq list for
+ * this bus.
+ *
+ * @np: the device node would like to power on
+ * @head: the list head for pwrseq list on this bus
+ *
+ * On successful, it returns 0, otherwise an error value.
+ */
+int of_pwrseq_on_list(struct device_node *np, struct list_head *head)
+{
+ struct pwrseq *pwrseq;
+ struct pwrseq_list_per_dev *pwrseq_list_node;
+
+ pwrseq = of_pwrseq_on(np);
+ if (IS_ERR(pwrseq))
+ return PTR_ERR(pwrseq);
+
+ pwrseq_list_node = kzalloc(sizeof(*pwrseq_list_node), GFP_KERNEL);
+ if (!pwrseq_list_node) {
+ of_pwrseq_off(pwrseq);
+ return -ENOMEM;
+ }
+ pwrseq_list_node->pwrseq = pwrseq;
+ list_add(&pwrseq_list_node->list, head);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(of_pwrseq_on_list);
+
+/**
+ * of_pwrseq_off_list: do power sequence off for the list
+ *
+ * This API is used to power off all devices on this bus, it is
+ * the opposite operation for of_pwrseq_on_list.
+ *
+ * @head: the list head for pwrseq instance list on this bus
+ */
+void of_pwrseq_off_list(struct list_head *head)
+{
+ struct pwrseq *pwrseq;
+ struct pwrseq_list_per_dev *pwrseq_list_node, *tmp_node;
+
+ list_for_each_entry_safe(pwrseq_list_node, tmp_node, head, list) {
+ pwrseq = pwrseq_list_node->pwrseq;
+ of_pwrseq_off(pwrseq);
+ list_del(&pwrseq_list_node->list);
+ kfree(pwrseq_list_node);
+ }
+}
+EXPORT_SYMBOL_GPL(of_pwrseq_off_list);
diff --git a/drivers/power/pwrseq/pwrseq_generic.c b/drivers/power/pwrseq/pwrseq_generic.c
new file mode 100644
index 0000000..d7a77f2
--- /dev/null
+++ b/drivers/power/pwrseq/pwrseq_generic.c
@@ -0,0 +1,183 @@
+/*
+ * pwrseq_generic.c Generic power sequence handling
+ *
+ * Copyright (C) 2016 Freescale Semiconductor, Inc.
+ * Author: Peter Chen <[email protected]>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 of
+ * the License as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+#include <linux/slab.h>
+
+#include <linux/power/pwrseq.h>
+
+struct pwrseq_generic {
+ struct pwrseq pwrseq;
+ struct gpio_desc *gpiod_reset;
+ struct clk *clks[PWRSEQ_MAX_CLKS];
+ u32 duration_us;
+};
+
+#define to_generic_pwrseq(p) container_of(p, struct pwrseq_generic, pwrseq)
+
+static int pwrseq_generic_alloc_instance(void);
+static const struct of_device_id generic_id_table[] = {
+ { .compatible = "generic",},
+ { /* sentinel */ }
+};
+
+static void pwrseq_generic_put(struct pwrseq *pwrseq)
+{
+ struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
+ int clk;
+
+ if (pwrseq_gen->gpiod_reset)
+ gpiod_put(pwrseq_gen->gpiod_reset);
+
+ for (clk = 0; clk < PWRSEQ_MAX_CLKS; clk++)
+ clk_put(pwrseq_gen->clks[clk]);
+
+ pwrseq_unregister(&pwrseq_gen->pwrseq);
+ kfree(pwrseq_gen);
+}
+
+static void pwrseq_generic_off(struct pwrseq *pwrseq)
+{
+ struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
+ int clk;
+
+ for (clk = PWRSEQ_MAX_CLKS - 1; clk >= 0; clk--)
+ clk_disable_unprepare(pwrseq_gen->clks[clk]);
+}
+
+static int pwrseq_generic_on(struct pwrseq *pwrseq)
+{
+ struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
+ int clk, ret = 0;
+ struct gpio_desc *gpiod_reset = pwrseq_gen->gpiod_reset;
+
+ for (clk = 0; clk < PWRSEQ_MAX_CLKS && pwrseq_gen->clks[clk]; clk++) {
+ ret = clk_prepare_enable(pwrseq_gen->clks[clk]);
+ if (ret) {
+ pr_err("Can't enable clock, ret=%d\n", ret);
+ goto err_disable_clks;
+ }
+ }
+
+ if (gpiod_reset) {
+ u32 duration_us = pwrseq_gen->duration_us;
+
+ if (duration_us <= 10)
+ udelay(10);
+ else
+ usleep_range(duration_us, duration_us + 100);
+ gpiod_set_value(gpiod_reset, 0);
+ }
+
+ return ret;
+
+err_disable_clks:
+ while (--clk >= 0)
+ clk_disable_unprepare(pwrseq_gen->clks[clk]);
+
+ return ret;
+}
+
+static int pwrseq_generic_get(struct device_node *np, struct pwrseq *pwrseq)
+{
+ struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
+ enum of_gpio_flags flags;
+ int reset_gpio, clk, ret = 0;
+
+ for (clk = 0; clk < PWRSEQ_MAX_CLKS; clk++) {
+ pwrseq_gen->clks[clk] = of_clk_get(np, clk);
+ if (IS_ERR(pwrseq_gen->clks[clk])) {
+ ret = PTR_ERR(pwrseq_gen->clks[clk]);
+ if (ret != -ENOENT)
+ goto err_put_clks;
+ pwrseq_gen->clks[clk] = NULL;
+ break;
+ }
+ }
+
+ reset_gpio = of_get_named_gpio_flags(np, "reset-gpios", 0, &flags);
+ if (gpio_is_valid(reset_gpio)) {
+ unsigned long gpio_flags;
+
+ if (flags & OF_GPIO_ACTIVE_LOW)
+ gpio_flags = GPIOF_ACTIVE_LOW | GPIOF_OUT_INIT_LOW;
+ else
+ gpio_flags = GPIOF_OUT_INIT_HIGH;
+
+ ret = gpio_request_one(reset_gpio, gpio_flags,
+ "pwrseq-reset-gpios");
+ if (ret)
+ goto err_put_clks;
+
+ pwrseq_gen->gpiod_reset = gpio_to_desc(reset_gpio);
+ of_property_read_u32(np, "reset-duration-us",
+ &pwrseq_gen->duration_us);
+ } else if (reset_gpio == -ENOENT) {
+ ; /* no such gpio */
+ } else {
+ ret = reset_gpio;
+ pr_err("Failed to get reset gpio on %s, err = %d\n",
+ np->full_name, reset_gpio);
+ goto err_put_clks;
+ }
+
+ /* allocate new one for later pwrseq instance request */
+ ret = pwrseq_generic_alloc_instance();
+ if (ret)
+ goto err_put_gpio;
+
+ return 0;
+
+err_put_gpio:
+ if (pwrseq_gen->gpiod_reset)
+ gpiod_put(pwrseq_gen->gpiod_reset);
+err_put_clks:
+ while (--clk >= 0)
+ clk_put(pwrseq_gen->clks[clk]);
+ return ret;
+}
+
+static int pwrseq_generic_alloc_instance(void)
+{
+ struct pwrseq_generic *pwrseq_gen;
+
+ pwrseq_gen = kzalloc(sizeof(*pwrseq_gen), GFP_KERNEL);
+ if (!pwrseq_gen)
+ return -ENOMEM;
+
+ pwrseq_gen->pwrseq.pwrseq_of_match_table = generic_id_table;
+ pwrseq_gen->pwrseq.get = pwrseq_generic_get;
+ pwrseq_gen->pwrseq.on = pwrseq_generic_on;
+ pwrseq_gen->pwrseq.off = pwrseq_generic_off;
+ pwrseq_gen->pwrseq.put = pwrseq_generic_put;
+
+ pwrseq_register(&pwrseq_gen->pwrseq);
+ return 0;
+}
+
+static int __init pwrseq_generic_register(void)
+{
+ return pwrseq_generic_alloc_instance();
+}
+postcore_initcall(pwrseq_generic_register)
diff --git a/include/linux/power/pwrseq.h b/include/linux/power/pwrseq.h
new file mode 100644
index 0000000..598301a
--- /dev/null
+++ b/include/linux/power/pwrseq.h
@@ -0,0 +1,60 @@
+#ifndef __LINUX_PWRSEQ_H
+#define __LINUX_PWRSEQ_H
+
+#include <linux/of.h>
+
+#define PWRSEQ_MAX_CLKS 3
+
+/**
+ * struct pwrseq - the power sequence structure
+ * @pwrseq_of_match_table: the OF device id table this pwrseq library supports
+ * @node: the list pointer to be added to pwrseq list
+ * @get: the API is used to get pwrseq instance from the device node
+ * @on: do power on for this pwrseq instance
+ * @off: do power off for this pwrseq instance
+ * @put: release the resources on this pwrseq instance
+ * @suspend: do suspend operation on this pwrseq instance
+ * @resume: do resume operation on this pwrseq instance
+ * @used: this pwrseq instance is used by device
+ */
+struct pwrseq {
+ const struct of_device_id *pwrseq_of_match_table;
+ struct list_head node;
+ int (*get)(struct device_node *np, struct pwrseq *p);
+ int (*on)(struct pwrseq *p);
+ void (*off)(struct pwrseq *p);
+ void (*put)(struct pwrseq *p);
+ int (*suspend)(struct pwrseq *p);
+ int (*resume)(struct pwrseq *p);
+ bool used;
+};
+
+/* used for power sequence instance list in one driver */
+struct pwrseq_list_per_dev {
+ struct pwrseq *pwrseq;
+ struct list_head list;
+};
+
+#if IS_ENABLED(CONFIG_POWER_SEQUENCE)
+void pwrseq_register(struct pwrseq *pwrseq);
+void pwrseq_unregister(struct pwrseq *pwrseq);
+struct pwrseq *of_pwrseq_on(struct device_node *np);
+void of_pwrseq_off(struct pwrseq *pwrseq);
+int of_pwrseq_on_list(struct device_node *np, struct list_head *head);
+void of_pwrseq_off_list(struct list_head *head);
+#else
+static inline void pwrseq_register(struct pwrseq *pwrseq) {}
+static inline void pwrseq_unregister(struct pwrseq *pwrseq) {}
+static inline struct pwrseq *of_pwrseq_on(struct device_node *np)
+{
+ return NULL;
+}
+void of_pwrseq_off(struct pwrseq *pwrseq) {}
+int of_pwrseq_on_list(struct device_node *np, struct list_head *head)
+{
+ return 0;
+}
+void of_pwrseq_off_list(struct list_head *head) {}
+#endif /* CONFIG_POWER_SEQUENCE */
+
+#endif /* __LINUX_PWRSEQ_H */
--
2.7.4
Some hard-wired USB devices need to do power sequence to let the
device work normally, the typical power sequence like: enable USB
PHY clock, toggle reset pin, etc. But current Linux USB driver
lacks of such code to do it, it may cause some hard-wired USB devices
works abnormal or can't be recognized by controller at all.
In this patch, it calls power sequence library APIs to finish
the power sequence events. It will do power on sequence at hub's
probe for all devices under this hub (includes root hub).
At hub_disconnect, it will do power off sequence which is at powered
on list.
Signed-off-by: Peter Chen <[email protected]>
Tested-by Joshua Clayton <[email protected]>
Tested-by: Maciej S. Szmigiero <[email protected]>
Reviewed-by: Vaibhav Hiremath <[email protected]>
---
drivers/usb/Kconfig | 1 +
drivers/usb/core/hub.c | 41 ++++++++++++++++++++++++++++++++++++++---
drivers/usb/core/hub.h | 1 +
3 files changed, 40 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
index fbe493d..706f261 100644
--- a/drivers/usb/Kconfig
+++ b/drivers/usb/Kconfig
@@ -40,6 +40,7 @@ config USB
tristate "Support for Host-side USB"
depends on USB_ARCH_HAS_HCD
select USB_COMMON
+ select POWER_SEQUENCE
select NLS # for UTF-8 strings
---help---
Universal Serial Bus (USB) is a specification for a serial bus
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 76e80d8..991340f 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -28,6 +28,7 @@
#include <linux/mutex.h>
#include <linux/random.h>
#include <linux/pm_qos.h>
+#include <linux/power/pwrseq.h>
#include <asm/uaccess.h>
#include <asm/byteorder.h>
@@ -1697,6 +1698,7 @@ static void hub_disconnect(struct usb_interface *intf)
hub->error = 0;
hub_quiesce(hub, HUB_DISCONNECT);
+ of_pwrseq_off_list(&hub->pwrseq_on_list);
mutex_lock(&usb_port_peer_mutex);
/* Avoid races with recursively_mark_NOTATTACHED() */
@@ -1724,12 +1726,41 @@ static void hub_disconnect(struct usb_interface *intf)
kref_put(&hub->kref, hub_release);
}
+#ifdef CONFIG_OF
+static int hub_of_pwrseq_on(struct usb_hub *hub)
+{
+ struct device *parent;
+ struct usb_device *hdev = hub->hdev;
+ struct device_node *np;
+ int ret;
+
+ if (hdev->parent)
+ parent = &hdev->dev;
+ else
+ parent = bus_to_hcd(hdev->bus)->self.controller;
+
+ for_each_child_of_node(parent->of_node, np) {
+ ret = of_pwrseq_on_list(np, &hub->pwrseq_on_list);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+#else
+static int hub_of_pwrseq_on(struct usb_hub *hub)
+{
+ return 0;
+}
+#endif
+
static int hub_probe(struct usb_interface *intf, const struct usb_device_id *id)
{
struct usb_host_interface *desc;
struct usb_endpoint_descriptor *endpoint;
struct usb_device *hdev;
struct usb_hub *hub;
+ int ret = -ENODEV;
desc = intf->cur_altsetting;
hdev = interface_to_usbdev(intf);
@@ -1834,6 +1865,7 @@ static int hub_probe(struct usb_interface *intf, const struct usb_device_id *id)
INIT_DELAYED_WORK(&hub->leds, led_work);
INIT_DELAYED_WORK(&hub->init_work, NULL);
INIT_WORK(&hub->events, hub_event);
+ INIT_LIST_HEAD(&hub->pwrseq_on_list);
usb_get_intf(intf);
usb_get_dev(hdev);
@@ -1847,11 +1879,14 @@ static int hub_probe(struct usb_interface *intf, const struct usb_device_id *id)
if (id->driver_info & HUB_QUIRK_CHECK_PORT_AUTOSUSPEND)
hub->quirk_check_port_auto_suspend = 1;
- if (hub_configure(hub, endpoint) >= 0)
- return 0;
+ if (hub_configure(hub, endpoint) >= 0) {
+ ret = hub_of_pwrseq_on(hub);
+ if (!ret)
+ return 0;
+ }
hub_disconnect(intf);
- return -ENODEV;
+ return ret;
}
static int
diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
index 34c1a7e..cd86f91 100644
--- a/drivers/usb/core/hub.h
+++ b/drivers/usb/core/hub.h
@@ -78,6 +78,7 @@ struct usb_hub {
struct delayed_work init_work;
struct work_struct events;
struct usb_port **ports;
+ struct list_head pwrseq_on_list; /* powered pwrseq node list */
};
/**
--
2.7.4
From: Joshua Clayton <[email protected]>
Previously the onboard hub was made to work by treating its
reset gpio as a regulator enable.
Get rid of that kludge now that pwseq has added reset gpio support
Move pin muxing the hub reset pin into the usbh1 group
Signed-off-by: Joshua Clayton <[email protected]>
Signed-off-by: Peter Chen <[email protected]>
---
arch/arm/boot/dts/imx6q-evi.dts | 25 +++++++------------------
1 file changed, 7 insertions(+), 18 deletions(-)
diff --git a/arch/arm/boot/dts/imx6q-evi.dts b/arch/arm/boot/dts/imx6q-evi.dts
index 6de21ff..3277a06 100644
--- a/arch/arm/boot/dts/imx6q-evi.dts
+++ b/arch/arm/boot/dts/imx6q-evi.dts
@@ -54,18 +54,6 @@
reg = <0x10000000 0x40000000>;
};
- reg_usbh1_vbus: regulator-usbhubreset {
- compatible = "regulator-fixed";
- regulator-name = "usbh1_vbus";
- regulator-min-microvolt = <5000000>;
- regulator-max-microvolt = <5000000>;
- enable-active-high;
- startup-delay-us = <2>;
- pinctrl-names = "default";
- pinctrl-0 = <&pinctrl_usbh1_hubreset>;
- gpio = <&gpio7 12 GPIO_ACTIVE_HIGH>;
- };
-
reg_usb_otg_vbus: regulator-usbotgvbus {
compatible = "regulator-fixed";
regulator-name = "usb_otg_vbus";
@@ -207,12 +195,18 @@
};
&usbh1 {
- vbus-supply = <®_usbh1_vbus>;
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_usbh1>;
dr_mode = "host";
disable-over-current;
status = "okay";
+
+ usb2415host: hub@1 {
+ compatible = "usb424,2513";
+ reg = <1>;
+ reset-gpios = <&gpio7 12 GPIO_ACTIVE_LOW>;
+ reset-duration-us = <3000>;
+ };
};
&usbotg {
@@ -471,11 +465,6 @@
MX6QDL_PAD_GPIO_3__USB_H1_OC 0x1b0b0
/* usbh1_b OC */
MX6QDL_PAD_GPIO_0__GPIO1_IO00 0x1b0b0
- >;
- };
-
- pinctrl_usbh1_hubreset: usbh1hubresetgrp {
- fsl,pins = <
MX6QDL_PAD_GPIO_17__GPIO7_IO12 0x1b0b0
>;
};
--
2.7.4
On Mon, Nov 14, 2016 at 09:35:53AM +0800, Peter Chen wrote:
> We have an well-known problem that the device needs to do some power
> sequence before it can be recognized by related host, the typical
> example like hard-wired mmc devices and usb devices.
>
> This power sequence is hard to be described at device tree and handled by
> related host driver, so we have created a common power sequence
> library to cover this requirement. The core code has supplied
> some common helpers for host driver, and individual power sequence
> libraries handle kinds of power sequence for devices. The pwrseq
> librares always need to allocate extra instance for compatible
> string match.
>
> pwrseq_generic is intended for general purpose of power sequence, which
> handles gpios and clocks currently, and can cover other controls in
> future. The host driver just needs to call of_pwrseq_on/of_pwrseq_off
> if only one power sequence is needed, else call of_pwrseq_on_list
> /of_pwrseq_off_list instead (eg, USB hub driver).
>
> For new power sequence library, it can add its compatible string
> to pwrseq_of_match_table, then the pwrseq core will match it with
> DT's, and choose this library at runtime.
>
Rafael, would you get any chances to review this version, it makes
some changes according to your comments, I hope this patch set could
be in v4.10-rc1, thanks.
Peter
> Signed-off-by: Peter Chen <[email protected]>
> Tested-by: Maciej S. Szmigiero <[email protected]>
> Tested-by Joshua Clayton <[email protected]>
> Reviewed-by: Matthias Kaehlcke <[email protected]>
> Tested-by: Matthias Kaehlcke <[email protected]>
> ---
> MAINTAINERS | 9 ++
> drivers/power/Kconfig | 1 +
> drivers/power/Makefile | 1 +
> drivers/power/pwrseq/Kconfig | 21 +++
> drivers/power/pwrseq/Makefile | 2 +
> drivers/power/pwrseq/core.c | 237 ++++++++++++++++++++++++++++++++++
> drivers/power/pwrseq/pwrseq_generic.c | 183 ++++++++++++++++++++++++++
> include/linux/power/pwrseq.h | 60 +++++++++
> 8 files changed, 514 insertions(+)
> create mode 100644 drivers/power/pwrseq/Kconfig
> create mode 100644 drivers/power/pwrseq/Makefile
> create mode 100644 drivers/power/pwrseq/core.c
> create mode 100644 drivers/power/pwrseq/pwrseq_generic.c
> create mode 100644 include/linux/power/pwrseq.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3d838cf..066b1e4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9621,6 +9621,15 @@ F: include/linux/pm_*
> F: include/linux/powercap.h
> F: drivers/powercap/
>
> +POWER SEQUENCE LIBRARY
> +M: Peter Chen <[email protected]>
> +T: git git://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git
> +L: [email protected]
> +S: Maintained
> +F: Documentation/devicetree/bindings/power/pwrseq/
> +F: drivers/power/pwrseq/
> +F: include/linux/power/pwrseq.h/
> +
> POWER SUPPLY CLASS/SUBSYSTEM and DRIVERS
> M: Sebastian Reichel <[email protected]>
> L: [email protected]
> diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
> index 63454b5..c1bb046 100644
> --- a/drivers/power/Kconfig
> +++ b/drivers/power/Kconfig
> @@ -1,3 +1,4 @@
> source "drivers/power/avs/Kconfig"
> source "drivers/power/reset/Kconfig"
> source "drivers/power/supply/Kconfig"
> +source "drivers/power/pwrseq/Kconfig"
> diff --git a/drivers/power/Makefile b/drivers/power/Makefile
> index ff35c71..7db8035 100644
> --- a/drivers/power/Makefile
> +++ b/drivers/power/Makefile
> @@ -1,3 +1,4 @@
> obj-$(CONFIG_POWER_AVS) += avs/
> obj-$(CONFIG_POWER_RESET) += reset/
> obj-$(CONFIG_POWER_SUPPLY) += supply/
> +obj-$(CONFIG_POWER_SEQUENCE) += pwrseq/
> diff --git a/drivers/power/pwrseq/Kconfig b/drivers/power/pwrseq/Kconfig
> new file mode 100644
> index 0000000..88f5597
> --- /dev/null
> +++ b/drivers/power/pwrseq/Kconfig
> @@ -0,0 +1,21 @@
> +#
> +# Power Sequence library
> +#
> +
> +menuconfig POWER_SEQUENCE
> + bool "Power sequence control"
> + depends on OF
> + help
> + It is used for drivers which needs to do power sequence
> + (eg, turn on clock, toggle reset gpio) before the related
> + devices can be found by hardware, eg, USB bus.
> +
> +if POWER_SEQUENCE
> +
> +config PWRSEQ_GENERIC
> + bool "Generic power sequence control"
> + default y
> + help
> + This is the generic power sequence control library, and is
> + supposed to support common power sequence usage.
> +endif
> diff --git a/drivers/power/pwrseq/Makefile b/drivers/power/pwrseq/Makefile
> new file mode 100644
> index 0000000..ad82389
> --- /dev/null
> +++ b/drivers/power/pwrseq/Makefile
> @@ -0,0 +1,2 @@
> +obj-$(CONFIG_POWER_SEQUENCE) += core.o
> +obj-$(CONFIG_PWRSEQ_GENERIC) += pwrseq_generic.o
> diff --git a/drivers/power/pwrseq/core.c b/drivers/power/pwrseq/core.c
> new file mode 100644
> index 0000000..e3c1fbb
> --- /dev/null
> +++ b/drivers/power/pwrseq/core.c
> @@ -0,0 +1,237 @@
> +/*
> + * core.c power sequence core file
> + *
> + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> + * Author: Peter Chen <[email protected]>
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 of
> + * the License as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/list.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/slab.h>
> +#include <linux/power/pwrseq.h>
> +
> +static DEFINE_MUTEX(pwrseq_list_mutex);
> +static LIST_HEAD(pwrseq_list);
> +
> +static int pwrseq_get(struct device_node *np, struct pwrseq *p)
> +{
> + if (p && p->get)
> + return p->get(np, p);
> +
> + return -ENOTSUPP;
> +}
> +
> +static int pwrseq_on(struct pwrseq *p)
> +{
> + if (p && p->on)
> + return p->on(p);
> +
> + return -ENOTSUPP;
> +}
> +
> +static void pwrseq_off(struct pwrseq *p)
> +{
> + if (p && p->off)
> + p->off(p);
> +}
> +
> +static void pwrseq_put(struct pwrseq *p)
> +{
> + if (p && p->put)
> + p->put(p);
> +}
> +
> +static int pwrseq_suspend(struct pwrseq *p)
> +{
> + if (p && p->suspend)
> + return p->suspend(p);
> +
> + return 0;
> +}
> +
> +static int pwrseq_resume(struct pwrseq *p)
> +{
> + if (p && p->resume)
> + return p->resume(p);
> +
> + return 0;
> +}
> +
> +/**
> + * pwrseq_register: add pwrseq instance to global pwrseq list
> + *
> + * @pwrseq: the pwrseq instance
> + */
> +void pwrseq_register(struct pwrseq *pwrseq)
> +{
> + mutex_lock(&pwrseq_list_mutex);
> + list_add(&pwrseq->node, &pwrseq_list);
> + mutex_unlock(&pwrseq_list_mutex);
> +}
> +EXPORT_SYMBOL_GPL(pwrseq_register);
> +
> +/**
> + * pwrseq_unregister: remove pwrseq instance from global pwrseq list
> + *
> + * @pwrseq: the pwrseq instance
> + */
> +void pwrseq_unregister(struct pwrseq *pwrseq)
> +{
> + mutex_lock(&pwrseq_list_mutex);
> + list_del(&pwrseq->node);
> + mutex_unlock(&pwrseq_list_mutex);
> +}
> +EXPORT_SYMBOL_GPL(pwrseq_unregister);
> +
> +static struct pwrseq *pwrseq_find_available_instance(struct device_node *np)
> +{
> + struct pwrseq *pwrseq;
> +
> + list_for_each_entry(pwrseq, &pwrseq_list, node) {
> + if (pwrseq->used)
> + continue;
> +
> + /* compare compatible string for pwrseq node */
> + if (of_match_node(pwrseq->pwrseq_of_match_table, np)) {
> + pwrseq->used = true;
> + return pwrseq;
> + }
> +
> + /* return generic pwrseq instance */
> + if (!strcmp(pwrseq->pwrseq_of_match_table->compatible,
> + "generic")) {
> + pr_debug("using generic pwrseq instance for %s\n",
> + np->full_name);
> + pwrseq->used = true;
> + return pwrseq;
> + }
> + }
> + pr_warn("Can't find any pwrseq instances for %s\n", np->full_name);
> +
> + return NULL;
> +}
> +
> +/**
> + * of_pwrseq_on: do power sequence on for device node
> + *
> + * This API is used to power on single device, if the host
> + * controller only needs to handle one child device (this device
> + * node points to), use this API. If multiply devices are needed
> + * to handle on bus, use of_pwrseq_on_list.
> + *
> + * @np: the device node would like to power on
> + *
> + * On successful, it returns pwrseq instance, otherwise an error value.
> + */
> +struct pwrseq *of_pwrseq_on(struct device_node *np)
> +{
> + struct pwrseq *pwrseq;
> + int ret;
> +
> + pwrseq = pwrseq_find_available_instance(np);
> + if (!pwrseq)
> + return ERR_PTR(-ENONET);
> +
> + ret = pwrseq_get(np, pwrseq);
> + if (ret) {
> + /* Mark current pwrseq as unused */
> + pwrseq->used = false;
> + return ERR_PTR(ret);
> + }
> +
> + ret = pwrseq_on(pwrseq);
> + if (ret)
> + goto pwr_put;
> +
> + return pwrseq;
> +
> +pwr_put:
> + pwrseq_put(pwrseq);
> + return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL_GPL(of_pwrseq_on);
> +
> +/**
> + * of_pwrseq_off: do power sequence off for this pwrseq instance
> + *
> + * This API is used to power off single device, it is the opposite
> + * operation for of_pwrseq_on.
> + *
> + * @pwrseq: the pwrseq instance which related device would like to be off
> + */
> +void of_pwrseq_off(struct pwrseq *pwrseq)
> +{
> + pwrseq_off(pwrseq);
> + pwrseq_put(pwrseq);
> +}
> +EXPORT_SYMBOL_GPL(of_pwrseq_off);
> +
> +/**
> + * of_pwrseq_on_list: do power sequence on for list
> + *
> + * This API is used to power on multiple devices at single bus.
> + * If there are several devices on bus (eg, USB bus), uses this
> + * this API. Otherwise, use of_pwrseq_on. After the device
> + * is powered on successfully, it will be added to pwrseq list for
> + * this bus.
> + *
> + * @np: the device node would like to power on
> + * @head: the list head for pwrseq list on this bus
> + *
> + * On successful, it returns 0, otherwise an error value.
> + */
> +int of_pwrseq_on_list(struct device_node *np, struct list_head *head)
> +{
> + struct pwrseq *pwrseq;
> + struct pwrseq_list_per_dev *pwrseq_list_node;
> +
> + pwrseq = of_pwrseq_on(np);
> + if (IS_ERR(pwrseq))
> + return PTR_ERR(pwrseq);
> +
> + pwrseq_list_node = kzalloc(sizeof(*pwrseq_list_node), GFP_KERNEL);
> + if (!pwrseq_list_node) {
> + of_pwrseq_off(pwrseq);
> + return -ENOMEM;
> + }
> + pwrseq_list_node->pwrseq = pwrseq;
> + list_add(&pwrseq_list_node->list, head);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(of_pwrseq_on_list);
> +
> +/**
> + * of_pwrseq_off_list: do power sequence off for the list
> + *
> + * This API is used to power off all devices on this bus, it is
> + * the opposite operation for of_pwrseq_on_list.
> + *
> + * @head: the list head for pwrseq instance list on this bus
> + */
> +void of_pwrseq_off_list(struct list_head *head)
> +{
> + struct pwrseq *pwrseq;
> + struct pwrseq_list_per_dev *pwrseq_list_node, *tmp_node;
> +
> + list_for_each_entry_safe(pwrseq_list_node, tmp_node, head, list) {
> + pwrseq = pwrseq_list_node->pwrseq;
> + of_pwrseq_off(pwrseq);
> + list_del(&pwrseq_list_node->list);
> + kfree(pwrseq_list_node);
> + }
> +}
> +EXPORT_SYMBOL_GPL(of_pwrseq_off_list);
> diff --git a/drivers/power/pwrseq/pwrseq_generic.c b/drivers/power/pwrseq/pwrseq_generic.c
> new file mode 100644
> index 0000000..d7a77f2
> --- /dev/null
> +++ b/drivers/power/pwrseq/pwrseq_generic.c
> @@ -0,0 +1,183 @@
> +/*
> + * pwrseq_generic.c Generic power sequence handling
> + *
> + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> + * Author: Peter Chen <[email protected]>
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 of
> + * the License as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/of.h>
> +#include <linux/of_gpio.h>
> +#include <linux/slab.h>
> +
> +#include <linux/power/pwrseq.h>
> +
> +struct pwrseq_generic {
> + struct pwrseq pwrseq;
> + struct gpio_desc *gpiod_reset;
> + struct clk *clks[PWRSEQ_MAX_CLKS];
> + u32 duration_us;
> +};
> +
> +#define to_generic_pwrseq(p) container_of(p, struct pwrseq_generic, pwrseq)
> +
> +static int pwrseq_generic_alloc_instance(void);
> +static const struct of_device_id generic_id_table[] = {
> + { .compatible = "generic",},
> + { /* sentinel */ }
> +};
> +
> +static void pwrseq_generic_put(struct pwrseq *pwrseq)
> +{
> + struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
> + int clk;
> +
> + if (pwrseq_gen->gpiod_reset)
> + gpiod_put(pwrseq_gen->gpiod_reset);
> +
> + for (clk = 0; clk < PWRSEQ_MAX_CLKS; clk++)
> + clk_put(pwrseq_gen->clks[clk]);
> +
> + pwrseq_unregister(&pwrseq_gen->pwrseq);
> + kfree(pwrseq_gen);
> +}
> +
> +static void pwrseq_generic_off(struct pwrseq *pwrseq)
> +{
> + struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
> + int clk;
> +
> + for (clk = PWRSEQ_MAX_CLKS - 1; clk >= 0; clk--)
> + clk_disable_unprepare(pwrseq_gen->clks[clk]);
> +}
> +
> +static int pwrseq_generic_on(struct pwrseq *pwrseq)
> +{
> + struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
> + int clk, ret = 0;
> + struct gpio_desc *gpiod_reset = pwrseq_gen->gpiod_reset;
> +
> + for (clk = 0; clk < PWRSEQ_MAX_CLKS && pwrseq_gen->clks[clk]; clk++) {
> + ret = clk_prepare_enable(pwrseq_gen->clks[clk]);
> + if (ret) {
> + pr_err("Can't enable clock, ret=%d\n", ret);
> + goto err_disable_clks;
> + }
> + }
> +
> + if (gpiod_reset) {
> + u32 duration_us = pwrseq_gen->duration_us;
> +
> + if (duration_us <= 10)
> + udelay(10);
> + else
> + usleep_range(duration_us, duration_us + 100);
> + gpiod_set_value(gpiod_reset, 0);
> + }
> +
> + return ret;
> +
> +err_disable_clks:
> + while (--clk >= 0)
> + clk_disable_unprepare(pwrseq_gen->clks[clk]);
> +
> + return ret;
> +}
> +
> +static int pwrseq_generic_get(struct device_node *np, struct pwrseq *pwrseq)
> +{
> + struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
> + enum of_gpio_flags flags;
> + int reset_gpio, clk, ret = 0;
> +
> + for (clk = 0; clk < PWRSEQ_MAX_CLKS; clk++) {
> + pwrseq_gen->clks[clk] = of_clk_get(np, clk);
> + if (IS_ERR(pwrseq_gen->clks[clk])) {
> + ret = PTR_ERR(pwrseq_gen->clks[clk]);
> + if (ret != -ENOENT)
> + goto err_put_clks;
> + pwrseq_gen->clks[clk] = NULL;
> + break;
> + }
> + }
> +
> + reset_gpio = of_get_named_gpio_flags(np, "reset-gpios", 0, &flags);
> + if (gpio_is_valid(reset_gpio)) {
> + unsigned long gpio_flags;
> +
> + if (flags & OF_GPIO_ACTIVE_LOW)
> + gpio_flags = GPIOF_ACTIVE_LOW | GPIOF_OUT_INIT_LOW;
> + else
> + gpio_flags = GPIOF_OUT_INIT_HIGH;
> +
> + ret = gpio_request_one(reset_gpio, gpio_flags,
> + "pwrseq-reset-gpios");
> + if (ret)
> + goto err_put_clks;
> +
> + pwrseq_gen->gpiod_reset = gpio_to_desc(reset_gpio);
> + of_property_read_u32(np, "reset-duration-us",
> + &pwrseq_gen->duration_us);
> + } else if (reset_gpio == -ENOENT) {
> + ; /* no such gpio */
> + } else {
> + ret = reset_gpio;
> + pr_err("Failed to get reset gpio on %s, err = %d\n",
> + np->full_name, reset_gpio);
> + goto err_put_clks;
> + }
> +
> + /* allocate new one for later pwrseq instance request */
> + ret = pwrseq_generic_alloc_instance();
> + if (ret)
> + goto err_put_gpio;
> +
> + return 0;
> +
> +err_put_gpio:
> + if (pwrseq_gen->gpiod_reset)
> + gpiod_put(pwrseq_gen->gpiod_reset);
> +err_put_clks:
> + while (--clk >= 0)
> + clk_put(pwrseq_gen->clks[clk]);
> + return ret;
> +}
> +
> +static int pwrseq_generic_alloc_instance(void)
> +{
> + struct pwrseq_generic *pwrseq_gen;
> +
> + pwrseq_gen = kzalloc(sizeof(*pwrseq_gen), GFP_KERNEL);
> + if (!pwrseq_gen)
> + return -ENOMEM;
> +
> + pwrseq_gen->pwrseq.pwrseq_of_match_table = generic_id_table;
> + pwrseq_gen->pwrseq.get = pwrseq_generic_get;
> + pwrseq_gen->pwrseq.on = pwrseq_generic_on;
> + pwrseq_gen->pwrseq.off = pwrseq_generic_off;
> + pwrseq_gen->pwrseq.put = pwrseq_generic_put;
> +
> + pwrseq_register(&pwrseq_gen->pwrseq);
> + return 0;
> +}
> +
> +static int __init pwrseq_generic_register(void)
> +{
> + return pwrseq_generic_alloc_instance();
> +}
> +postcore_initcall(pwrseq_generic_register)
> diff --git a/include/linux/power/pwrseq.h b/include/linux/power/pwrseq.h
> new file mode 100644
> index 0000000..598301a
> --- /dev/null
> +++ b/include/linux/power/pwrseq.h
> @@ -0,0 +1,60 @@
> +#ifndef __LINUX_PWRSEQ_H
> +#define __LINUX_PWRSEQ_H
> +
> +#include <linux/of.h>
> +
> +#define PWRSEQ_MAX_CLKS 3
> +
> +/**
> + * struct pwrseq - the power sequence structure
> + * @pwrseq_of_match_table: the OF device id table this pwrseq library supports
> + * @node: the list pointer to be added to pwrseq list
> + * @get: the API is used to get pwrseq instance from the device node
> + * @on: do power on for this pwrseq instance
> + * @off: do power off for this pwrseq instance
> + * @put: release the resources on this pwrseq instance
> + * @suspend: do suspend operation on this pwrseq instance
> + * @resume: do resume operation on this pwrseq instance
> + * @used: this pwrseq instance is used by device
> + */
> +struct pwrseq {
> + const struct of_device_id *pwrseq_of_match_table;
> + struct list_head node;
> + int (*get)(struct device_node *np, struct pwrseq *p);
> + int (*on)(struct pwrseq *p);
> + void (*off)(struct pwrseq *p);
> + void (*put)(struct pwrseq *p);
> + int (*suspend)(struct pwrseq *p);
> + int (*resume)(struct pwrseq *p);
> + bool used;
> +};
> +
> +/* used for power sequence instance list in one driver */
> +struct pwrseq_list_per_dev {
> + struct pwrseq *pwrseq;
> + struct list_head list;
> +};
> +
> +#if IS_ENABLED(CONFIG_POWER_SEQUENCE)
> +void pwrseq_register(struct pwrseq *pwrseq);
> +void pwrseq_unregister(struct pwrseq *pwrseq);
> +struct pwrseq *of_pwrseq_on(struct device_node *np);
> +void of_pwrseq_off(struct pwrseq *pwrseq);
> +int of_pwrseq_on_list(struct device_node *np, struct list_head *head);
> +void of_pwrseq_off_list(struct list_head *head);
> +#else
> +static inline void pwrseq_register(struct pwrseq *pwrseq) {}
> +static inline void pwrseq_unregister(struct pwrseq *pwrseq) {}
> +static inline struct pwrseq *of_pwrseq_on(struct device_node *np)
> +{
> + return NULL;
> +}
> +void of_pwrseq_off(struct pwrseq *pwrseq) {}
> +int of_pwrseq_on_list(struct device_node *np, struct list_head *head)
> +{
> + return 0;
> +}
> +void of_pwrseq_off_list(struct list_head *head) {}
> +#endif /* CONFIG_POWER_SEQUENCE */
> +
> +#endif /* __LINUX_PWRSEQ_H */
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Best Regards,
Peter Chen
On Mon, Nov 14, 2016 at 2:35 AM, Peter Chen <[email protected]> wrote:
> We have an well-known problem that the device needs to do some power
> sequence before it can be recognized by related host, the typical
> example like hard-wired mmc devices and usb devices.
>
> This power sequence is hard to be described at device tree and handled by
> related host driver, so we have created a common power sequence
> library to cover this requirement. The core code has supplied
> some common helpers for host driver, and individual power sequence
> libraries handle kinds of power sequence for devices. The pwrseq
> librares always need to allocate extra instance for compatible
> string match.
>
> pwrseq_generic is intended for general purpose of power sequence, which
> handles gpios and clocks currently, and can cover other controls in
> future. The host driver just needs to call of_pwrseq_on/of_pwrseq_off
> if only one power sequence is needed, else call of_pwrseq_on_list
> /of_pwrseq_off_list instead (eg, USB hub driver).
>
> For new power sequence library, it can add its compatible string
> to pwrseq_of_match_table, then the pwrseq core will match it with
> DT's, and choose this library at runtime.
>
> Signed-off-by: Peter Chen <[email protected]>
> Tested-by: Maciej S. Szmigiero <[email protected]>
> Tested-by Joshua Clayton <[email protected]>
> Reviewed-by: Matthias Kaehlcke <[email protected]>
> Tested-by: Matthias Kaehlcke <[email protected]>
> ---
> MAINTAINERS | 9 ++
> drivers/power/Kconfig | 1 +
> drivers/power/Makefile | 1 +
> drivers/power/pwrseq/Kconfig | 21 +++
> drivers/power/pwrseq/Makefile | 2 +
> drivers/power/pwrseq/core.c | 237 ++++++++++++++++++++++++++++++++++
> drivers/power/pwrseq/pwrseq_generic.c | 183 ++++++++++++++++++++++++++
> include/linux/power/pwrseq.h | 60 +++++++++
> 8 files changed, 514 insertions(+)
> create mode 100644 drivers/power/pwrseq/Kconfig
> create mode 100644 drivers/power/pwrseq/Makefile
> create mode 100644 drivers/power/pwrseq/core.c
> create mode 100644 drivers/power/pwrseq/pwrseq_generic.c
> create mode 100644 include/linux/power/pwrseq.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3d838cf..066b1e4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9621,6 +9621,15 @@ F: include/linux/pm_*
> F: include/linux/powercap.h
> F: drivers/powercap/
>
> +POWER SEQUENCE LIBRARY
> +M: Peter Chen <[email protected]>
> +T: git git://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git
> +L: [email protected]
> +S: Maintained
> +F: Documentation/devicetree/bindings/power/pwrseq/
> +F: drivers/power/pwrseq/
> +F: include/linux/power/pwrseq.h/
> +
> POWER SUPPLY CLASS/SUBSYSTEM and DRIVERS
> M: Sebastian Reichel <[email protected]>
> L: [email protected]
> diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
> index 63454b5..c1bb046 100644
> --- a/drivers/power/Kconfig
> +++ b/drivers/power/Kconfig
> @@ -1,3 +1,4 @@
> source "drivers/power/avs/Kconfig"
> source "drivers/power/reset/Kconfig"
> source "drivers/power/supply/Kconfig"
> +source "drivers/power/pwrseq/Kconfig"
> diff --git a/drivers/power/Makefile b/drivers/power/Makefile
> index ff35c71..7db8035 100644
> --- a/drivers/power/Makefile
> +++ b/drivers/power/Makefile
> @@ -1,3 +1,4 @@
> obj-$(CONFIG_POWER_AVS) += avs/
> obj-$(CONFIG_POWER_RESET) += reset/
> obj-$(CONFIG_POWER_SUPPLY) += supply/
> +obj-$(CONFIG_POWER_SEQUENCE) += pwrseq/
> diff --git a/drivers/power/pwrseq/Kconfig b/drivers/power/pwrseq/Kconfig
> new file mode 100644
> index 0000000..88f5597
> --- /dev/null
> +++ b/drivers/power/pwrseq/Kconfig
> @@ -0,0 +1,21 @@
> +#
> +# Power Sequence library
> +#
> +
> +menuconfig POWER_SEQUENCE
> + bool "Power sequence control"
> + depends on OF
> + help
> + It is used for drivers which needs to do power sequence
> + (eg, turn on clock, toggle reset gpio) before the related
> + devices can be found by hardware, eg, USB bus.
> +
> +if POWER_SEQUENCE
> +
> +config PWRSEQ_GENERIC
> + bool "Generic power sequence control"
> + default y
> + help
> + This is the generic power sequence control library, and is
> + supposed to support common power sequence usage.
> +endif
> diff --git a/drivers/power/pwrseq/Makefile b/drivers/power/pwrseq/Makefile
> new file mode 100644
> index 0000000..ad82389
> --- /dev/null
> +++ b/drivers/power/pwrseq/Makefile
> @@ -0,0 +1,2 @@
> +obj-$(CONFIG_POWER_SEQUENCE) += core.o
> +obj-$(CONFIG_PWRSEQ_GENERIC) += pwrseq_generic.o
> diff --git a/drivers/power/pwrseq/core.c b/drivers/power/pwrseq/core.c
> new file mode 100644
> index 0000000..e3c1fbb
> --- /dev/null
> +++ b/drivers/power/pwrseq/core.c
> @@ -0,0 +1,237 @@
> +/*
> + * core.c power sequence core file
> + *
> + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> + * Author: Peter Chen <[email protected]>
> + *
> + * 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/>.
The last paragraph is not necessary AFAICS.
> + */
> +
> +#include <linux/list.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/slab.h>
> +#include <linux/power/pwrseq.h>
> +
> +static DEFINE_MUTEX(pwrseq_list_mutex);
> +static LIST_HEAD(pwrseq_list);
> +
> +static int pwrseq_get(struct device_node *np, struct pwrseq *p)
> +{
> + if (p && p->get)
> + return p->get(np, p);
> +
> + return -ENOTSUPP;
> +}
> +
> +static int pwrseq_on(struct pwrseq *p)
> +{
> + if (p && p->on)
> + return p->on(p);
> +
> + return -ENOTSUPP;
> +}
> +
> +static void pwrseq_off(struct pwrseq *p)
> +{
> + if (p && p->off)
> + p->off(p);
> +}
> +
> +static void pwrseq_put(struct pwrseq *p)
> +{
> + if (p && p->put)
> + p->put(p);
> +}
> +
> +static int pwrseq_suspend(struct pwrseq *p)
> +{
> + if (p && p->suspend)
> + return p->suspend(p);
> +
> + return 0;
> +}
> +
> +static int pwrseq_resume(struct pwrseq *p)
> +{
> + if (p && p->resume)
> + return p->resume(p);
> +
> + return 0;
> +}
> +
> +/**
> + * pwrseq_register: add pwrseq instance to global pwrseq list
> + *
> + * @pwrseq: the pwrseq instance
> + */
> +void pwrseq_register(struct pwrseq *pwrseq)
> +{
> + mutex_lock(&pwrseq_list_mutex);
> + list_add(&pwrseq->node, &pwrseq_list);
> + mutex_unlock(&pwrseq_list_mutex);
> +}
> +EXPORT_SYMBOL_GPL(pwrseq_register);
> +
> +/**
> + * pwrseq_unregister: remove pwrseq instance from global pwrseq list
> + *
> + * @pwrseq: the pwrseq instance
> + */
> +void pwrseq_unregister(struct pwrseq *pwrseq)
> +{
> + mutex_lock(&pwrseq_list_mutex);
> + list_del(&pwrseq->node);
> + mutex_unlock(&pwrseq_list_mutex);
> +}
> +EXPORT_SYMBOL_GPL(pwrseq_unregister);
> +
> +static struct pwrseq *pwrseq_find_available_instance(struct device_node *np)
> +{
> + struct pwrseq *pwrseq;
> +
> + list_for_each_entry(pwrseq, &pwrseq_list, node) {
> + if (pwrseq->used)
> + continue;
> +
> + /* compare compatible string for pwrseq node */
> + if (of_match_node(pwrseq->pwrseq_of_match_table, np)) {
> + pwrseq->used = true;
> + return pwrseq;
> + }
> +
> + /* return generic pwrseq instance */
> + if (!strcmp(pwrseq->pwrseq_of_match_table->compatible,
> + "generic")) {
> + pr_debug("using generic pwrseq instance for %s\n",
> + np->full_name);
> + pwrseq->used = true;
> + return pwrseq;
> + }
> + }
> + pr_warn("Can't find any pwrseq instances for %s\n", np->full_name);
pr_debug() ?
> +
> + return NULL;
> +}
> +
> +/**
> + * of_pwrseq_on: do power sequence on for device node
of_pwrseq_on - Carry out power sequence on for device node
Argument description should follow this line.
> + *
> + * This API is used to power on single device, if the host
> + * controller only needs to handle one child device (this device
> + * node points to), use this API. If multiply devices are needed
> + * to handle on bus, use of_pwrseq_on_list.
That's unclear.
What about "Carry out a single device power on. If multiple devices
need to be handled, use of_pwrseq_on_list() instead."
> + *
> + * @np: the device node would like to power on
> + *
> + * On successful, it returns pwrseq instance, otherwise an error value.
"Return a pointer to the power sequence instance on success, or an
error code otherwise."
> + */
> +struct pwrseq *of_pwrseq_on(struct device_node *np)
> +{
> + struct pwrseq *pwrseq;
> + int ret;
> +
> + pwrseq = pwrseq_find_available_instance(np);
What does guarantee the integrity of ths list at this point?
> + if (!pwrseq)
> + return ERR_PTR(-ENONET);
ENOENT I suppose?
> +
> + ret = pwrseq_get(np, pwrseq);
> + if (ret) {
> + /* Mark current pwrseq as unused */
> + pwrseq->used = false;
> + return ERR_PTR(ret);
> + }
> +
> + ret = pwrseq_on(pwrseq);
> + if (ret)
> + goto pwr_put;
> +
> + return pwrseq;
> +
> +pwr_put:
> + pwrseq_put(pwrseq);
> + return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL_GPL(of_pwrseq_on);
> +
> +/**
> + * of_pwrseq_off: do power sequence off for this pwrseq instance
> + *
> + * This API is used to power off single device, it is the opposite
> + * operation for of_pwrseq_on.
> + *
> + * @pwrseq: the pwrseq instance which related device would like to be off
> + */
> +void of_pwrseq_off(struct pwrseq *pwrseq)
> +{
> + pwrseq_off(pwrseq);
> + pwrseq_put(pwrseq);
> +}
> +EXPORT_SYMBOL_GPL(of_pwrseq_off);
What happens if two code paths attempt to turn the same power sequence
off in parallel? Can it ever happen? If not, then why not?
> +
> +/**
> + * of_pwrseq_on_list: do power sequence on for list
> + *
> + * This API is used to power on multiple devices at single bus.
> + * If there are several devices on bus (eg, USB bus), uses this
> + * this API. Otherwise, use of_pwrseq_on. After the device
> + * is powered on successfully, it will be added to pwrseq list for
> + * this bus.
> + *
> + * @np: the device node would like to power on
> + * @head: the list head for pwrseq list on this bus
> + *
> + * On successful, it returns 0, otherwise an error value.
Please format the kerneldoc comment in a usual way.
> + */
> +int of_pwrseq_on_list(struct device_node *np, struct list_head *head)
> +{
> + struct pwrseq *pwrseq;
> + struct pwrseq_list_per_dev *pwrseq_list_node;
> +
> + pwrseq = of_pwrseq_on(np);
> + if (IS_ERR(pwrseq))
> + return PTR_ERR(pwrseq);
> +
> + pwrseq_list_node = kzalloc(sizeof(*pwrseq_list_node), GFP_KERNEL);
Why don't you allocate memory before turning the power sequence on?
> + if (!pwrseq_list_node) {
> + of_pwrseq_off(pwrseq);
> + return -ENOMEM;
> + }
> + pwrseq_list_node->pwrseq = pwrseq;
> + list_add(&pwrseq_list_node->list, head);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(of_pwrseq_on_list);
So the caller is supposed to provide a list head of the list to put
the power sequence object into on success, right?
Can you explain to me what the idea here is, please?
Also, what's the protection of the list against concurrent access?
> +
> +/**
> + * of_pwrseq_off_list: do power sequence off for the list
> + *
> + * This API is used to power off all devices on this bus, it is
> + * the opposite operation for of_pwrseq_on_list.
> + *
> + * @head: the list head for pwrseq instance list on this bus
> + */
> +void of_pwrseq_off_list(struct list_head *head)
> +{
> + struct pwrseq *pwrseq;
> + struct pwrseq_list_per_dev *pwrseq_list_node, *tmp_node;
> +
> + list_for_each_entry_safe(pwrseq_list_node, tmp_node, head, list) {
> + pwrseq = pwrseq_list_node->pwrseq;
> + of_pwrseq_off(pwrseq);
> + list_del(&pwrseq_list_node->list);
> + kfree(pwrseq_list_node);
> + }
> +}
> +EXPORT_SYMBOL_GPL(of_pwrseq_off_list);
This looks horribly inefficient.
Is the user expected to create the list from scratch every time things
are turned on?
OK, let's stop here. I need the above to be clarified first.
Thanks,
Rafael
On Tue, Nov 22, 2016 at 03:23:12AM +0100, Rafael J. Wysocki wrote:
> > @@ -0,0 +1,237 @@
> > +/*
> > + * core.c power sequence core file
> > + *
> > + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> > + * Author: Peter Chen <[email protected]>
> > + *
> > + * 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/>.
>
> The last paragraph is not necessary AFAICS.
I just copy it from:
https://www.gnu.org/licenses/gpl-howto.en.html
If you are concerns about it, I can delete it.
> > +
> > +static struct pwrseq *pwrseq_find_available_instance(struct device_node *np)
> > +{
> > + struct pwrseq *pwrseq;
> > +
> > + list_for_each_entry(pwrseq, &pwrseq_list, node) {
> > + if (pwrseq->used)
> > + continue;
> > +
> > + /* compare compatible string for pwrseq node */
> > + if (of_match_node(pwrseq->pwrseq_of_match_table, np)) {
> > + pwrseq->used = true;
> > + return pwrseq;
> > + }
> > +
> > + /* return generic pwrseq instance */
> > + if (!strcmp(pwrseq->pwrseq_of_match_table->compatible,
> > + "generic")) {
> > + pr_debug("using generic pwrseq instance for %s\n",
> > + np->full_name);
> > + pwrseq->used = true;
> > + return pwrseq;
> > + }
> > + }
> > + pr_warn("Can't find any pwrseq instances for %s\n", np->full_name);
>
> pr_debug() ?
If there is no pwrseq instance for that node, the power sequence on routine will
return fail, so I think an warning message is useful for user.
>
> > +
> > + return NULL;
> > +}
> > +
> > +/**
> > + * of_pwrseq_on: do power sequence on for device node
>
> of_pwrseq_on - Carry out power sequence on for device node
>
> Argument description should follow this line.
>
> > + *
> > + * This API is used to power on single device, if the host
> > + * controller only needs to handle one child device (this device
> > + * node points to), use this API. If multiply devices are needed
> > + * to handle on bus, use of_pwrseq_on_list.
>
> That's unclear.
>
> What about "Carry out a single device power on. If multiple devices
> need to be handled, use of_pwrseq_on_list() instead."
>
> > + *
> > + * @np: the device node would like to power on
> > + *
> > + * On successful, it returns pwrseq instance, otherwise an error value.
>
> "Return a pointer to the power sequence instance on success, or an
> error code otherwise."
>
Ok, will change.
> > + */
> > +struct pwrseq *of_pwrseq_on(struct device_node *np)
> > +{
> > + struct pwrseq *pwrseq;
> > + int ret;
> > +
> > + pwrseq = pwrseq_find_available_instance(np);
>
> What does guarantee the integrity of ths list at this point?
Once the use selects the specific pwrseq library, the library will
create an empty one instance during the initialization, and it
will be called at postcore_initcall, the device driver has not
probed yet.
>
> > + if (!pwrseq)
> > + return ERR_PTR(-ENONET);
>
> ENOENT I suppose?
>
Good catch, thanks.
> > +/**
> > + * of_pwrseq_off: do power sequence off for this pwrseq instance
> > + *
> > + * This API is used to power off single device, it is the opposite
> > + * operation for of_pwrseq_on.
> > + *
> > + * @pwrseq: the pwrseq instance which related device would like to be off
> > + */
> > +void of_pwrseq_off(struct pwrseq *pwrseq)
> > +{
> > + pwrseq_off(pwrseq);
> > + pwrseq_put(pwrseq);
> > +}
> > +EXPORT_SYMBOL_GPL(of_pwrseq_off);
>
> What happens if two code paths attempt to turn the same power sequence
> off in parallel? Can it ever happen? If not, then why not?
>
I don't think the same pwrseq instance off will be called at the same
time, the of_pwrseq_off is supposed to be only called at error path
during power-on and at device power-off routine, and only the power-on is
successful, the device can be created, if the device is not created,
its power-off routine is not supposed to be called.
> > +
> > +/**
> > + * of_pwrseq_on_list: do power sequence on for list
> > + *
> > + * This API is used to power on multiple devices at single bus.
> > + * If there are several devices on bus (eg, USB bus), uses this
> > + * this API. Otherwise, use of_pwrseq_on. After the device
> > + * is powered on successfully, it will be added to pwrseq list for
> > + * this bus.
> > + *
> > + * @np: the device node would like to power on
> > + * @head: the list head for pwrseq list on this bus
> > + *
> > + * On successful, it returns 0, otherwise an error value.
>
> Please format the kerneldoc comment in a usual way.
>
Ok.
> > + */
> > +int of_pwrseq_on_list(struct device_node *np, struct list_head *head)
> > +{
> > + struct pwrseq *pwrseq;
> > + struct pwrseq_list_per_dev *pwrseq_list_node;
> > +
> > + pwrseq = of_pwrseq_on(np);
> > + if (IS_ERR(pwrseq))
> > + return PTR_ERR(pwrseq);
> > +
> > + pwrseq_list_node = kzalloc(sizeof(*pwrseq_list_node), GFP_KERNEL);
>
> Why don't you allocate memory before turning the power sequence on?
>
This list is only for power sequence on instance, if I allocate memory before
power sequence on, I need to free it if power sequence on is failed.
> > + if (!pwrseq_list_node) {
> > + of_pwrseq_off(pwrseq);
> > + return -ENOMEM;
> > + }
> > + pwrseq_list_node->pwrseq = pwrseq;
> > + list_add(&pwrseq_list_node->list, head);
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(of_pwrseq_on_list);
>
> So the caller is supposed to provide a list head of the list to put
> the power sequence object into on success, right?
Yes
>
> Can you explain to me what the idea here is, please?
>
Taking USB devices as an example, there is one power sequence on list
per bus, and there are several USB devices on the bus. Using a list,
we can record which device is powered sequence on, and only powers
sequence off which has already powered sequence on at error path, and
power sequence off all devices on the bus when the bus (eg, USB HUB)
is removed. (eg, when the bus driver is removed)
Usually, the power sequence is only needed for hard-wired devices,
the power sequence on is carried out during the bus driver probed,
and off if carried out during the bus driver is removed,
of_pwrseq_on_list/of_powerseq_off_list is not supposed to be
called during the other bus driver life cycles.
> Also, what's the protection of the list against concurrent access?
>
I will add comment that the list creator needs to take consideration
of concurrent access if exists.
> > +
> > +/**
> > + * of_pwrseq_off_list: do power sequence off for the list
> > + *
> > + * This API is used to power off all devices on this bus, it is
> > + * the opposite operation for of_pwrseq_on_list.
> > + *
> > + * @head: the list head for pwrseq instance list on this bus
> > + */
> > +void of_pwrseq_off_list(struct list_head *head)
> > +{
> > + struct pwrseq *pwrseq;
> > + struct pwrseq_list_per_dev *pwrseq_list_node, *tmp_node;
> > +
> > + list_for_each_entry_safe(pwrseq_list_node, tmp_node, head, list) {
> > + pwrseq = pwrseq_list_node->pwrseq;
> > + of_pwrseq_off(pwrseq);
> > + list_del(&pwrseq_list_node->list);
> > + kfree(pwrseq_list_node);
> > + }
> > +}
> > +EXPORT_SYMBOL_GPL(of_pwrseq_off_list);
>
> This looks horribly inefficient.
>
> Is the user expected to create the list from scratch every time things
> are turned on?
>
Like I explained above, the power sequence is for hard-wired device on
board, the list creation and remove are only carried out on driver's
probe and remove.
--
Best Regards,
Peter Chen
On Tue, Nov 22, 2016 at 4:53 AM, Peter Chen <[email protected]> wrote:
> On Tue, Nov 22, 2016 at 03:23:12AM +0100, Rafael J. Wysocki wrote:
>> > @@ -0,0 +1,237 @@
>> > +/*
>> > + * core.c power sequence core file
>> > + *
>> > + * Copyright (C) 2016 Freescale Semiconductor, Inc.
>> > + * Author: Peter Chen <[email protected]>
>> > + *
>> > + * 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/>.
>>
>> The last paragraph is not necessary AFAICS.
>
> I just copy it from:
>
> https://www.gnu.org/licenses/gpl-howto.en.html
>
> If you are concerns about it, I can delete it.
It is redundant, so yes, please.
>> > +
>> > +static struct pwrseq *pwrseq_find_available_instance(struct device_node *np)
>> > +{
>> > + struct pwrseq *pwrseq;
>> > +
>> > + list_for_each_entry(pwrseq, &pwrseq_list, node) {
>> > + if (pwrseq->used)
>> > + continue;
>> > +
>> > + /* compare compatible string for pwrseq node */
>> > + if (of_match_node(pwrseq->pwrseq_of_match_table, np)) {
>> > + pwrseq->used = true;
>> > + return pwrseq;
>> > + }
>> > +
>> > + /* return generic pwrseq instance */
>> > + if (!strcmp(pwrseq->pwrseq_of_match_table->compatible,
>> > + "generic")) {
>> > + pr_debug("using generic pwrseq instance for %s\n",
>> > + np->full_name);
>> > + pwrseq->used = true;
>> > + return pwrseq;
>> > + }
>> > + }
>> > + pr_warn("Can't find any pwrseq instances for %s\n", np->full_name);
>>
>> pr_debug() ?
>
> If there is no pwrseq instance for that node, the power sequence on routine will
> return fail, so I think an warning message is useful for user.
Useful in what way? How is the user supposed to know what happened
from this message?
>>
>> > +
>> > + return NULL;
>> > +}
>> > +
>> > +/**
>> > + * of_pwrseq_on: do power sequence on for device node
>>
>> of_pwrseq_on - Carry out power sequence on for device node
>>
>> Argument description should follow this line.
>>
>> > + *
>> > + * This API is used to power on single device, if the host
>> > + * controller only needs to handle one child device (this device
>> > + * node points to), use this API. If multiply devices are needed
>> > + * to handle on bus, use of_pwrseq_on_list.
>>
>> That's unclear.
>>
>> What about "Carry out a single device power on. If multiple devices
>> need to be handled, use of_pwrseq_on_list() instead."
>>
>> > + *
>> > + * @np: the device node would like to power on
>> > + *
>> > + * On successful, it returns pwrseq instance, otherwise an error value.
>>
>> "Return a pointer to the power sequence instance on success, or an
>> error code otherwise."
>>
>
> Ok, will change.
>
>> > + */
>> > +struct pwrseq *of_pwrseq_on(struct device_node *np)
>> > +{
>> > + struct pwrseq *pwrseq;
>> > + int ret;
>> > +
>> > + pwrseq = pwrseq_find_available_instance(np);
>>
>> What does guarantee the integrity of ths list at this point?
>
> Once the use selects the specific pwrseq library, the library will
> create an empty one instance during the initialization, and it
> will be called at postcore_initcall, the device driver has not
> probed yet.
Which doesn't matter really, because the list is global and some other
driver using it might have been probed already.
You have a mutex here and it is used for add/remove. Why isn't it
used for list browsing?
>
>>
>> > + if (!pwrseq)
>> > + return ERR_PTR(-ENONET);
>>
>> ENOENT I suppose?
>>
>
> Good catch, thanks.
>
>> > +/**
>> > + * of_pwrseq_off: do power sequence off for this pwrseq instance
>> > + *
>> > + * This API is used to power off single device, it is the opposite
>> > + * operation for of_pwrseq_on.
>> > + *
>> > + * @pwrseq: the pwrseq instance which related device would like to be off
>> > + */
>> > +void of_pwrseq_off(struct pwrseq *pwrseq)
>> > +{
>> > + pwrseq_off(pwrseq);
>> > + pwrseq_put(pwrseq);
>> > +}
>> > +EXPORT_SYMBOL_GPL(of_pwrseq_off);
>>
>> What happens if two code paths attempt to turn the same power sequence
>> off in parallel? Can it ever happen? If not, then why not?
>>
>
> I don't think the same pwrseq instance off will be called at the same
> time, the of_pwrseq_off is supposed to be only called at error path
> during power-on and at device power-off routine, and only the power-on is
> successful, the device can be created, if the device is not created,
> its power-off routine is not supposed to be called.
>
>> > +
>> > +/**
>> > + * of_pwrseq_on_list: do power sequence on for list
>> > + *
>> > + * This API is used to power on multiple devices at single bus.
>> > + * If there are several devices on bus (eg, USB bus), uses this
>> > + * this API. Otherwise, use of_pwrseq_on. After the device
>> > + * is powered on successfully, it will be added to pwrseq list for
>> > + * this bus.
>> > + *
>> > + * @np: the device node would like to power on
>> > + * @head: the list head for pwrseq list on this bus
>> > + *
>> > + * On successful, it returns 0, otherwise an error value.
>>
>> Please format the kerneldoc comment in a usual way.
>>
>
> Ok.
>
>> > + */
>> > +int of_pwrseq_on_list(struct device_node *np, struct list_head *head)
>> > +{
>> > + struct pwrseq *pwrseq;
>> > + struct pwrseq_list_per_dev *pwrseq_list_node;
>> > +
>> > + pwrseq = of_pwrseq_on(np);
>> > + if (IS_ERR(pwrseq))
>> > + return PTR_ERR(pwrseq);
>> > +
>> > + pwrseq_list_node = kzalloc(sizeof(*pwrseq_list_node), GFP_KERNEL);
>>
>> Why don't you allocate memory before turning the power sequence on?
>>
>
> This list is only for power sequence on instance, if I allocate memory before
> power sequence on, I need to free it if power sequence on is failed.
So why is that a problem?
>> > + if (!pwrseq_list_node) {
>> > + of_pwrseq_off(pwrseq);
>> > + return -ENOMEM;
>> > + }
>> > + pwrseq_list_node->pwrseq = pwrseq;
>> > + list_add(&pwrseq_list_node->list, head);
>> > +
>> > + return 0;
>> > +}
>> > +EXPORT_SYMBOL_GPL(of_pwrseq_on_list);
>>
>> So the caller is supposed to provide a list head of the list to put
>> the power sequence object into on success, right?
>
> Yes
>
>>
>> Can you explain to me what the idea here is, please?
>>
>
> Taking USB devices as an example, there is one power sequence on list
> per bus, and there are several USB devices on the bus. Using a list,
> we can record which device is powered sequence on, and only powers
> sequence off which has already powered sequence on at error path, and
> power sequence off all devices on the bus when the bus (eg, USB HUB)
> is removed. (eg, when the bus driver is removed)
Well, I'm not sure I understand this correctly.
What about system suspend/resume and such, for instance?
> Usually, the power sequence is only needed for hard-wired devices,
> the power sequence on is carried out during the bus driver probed,
> and off if carried out during the bus driver is removed,
> of_pwrseq_on_list/of_powerseq_off_list is not supposed to be
> called during the other bus driver life cycles.
>
>> Also, what's the protection of the list against concurrent access?
>>
>
> I will add comment that the list creator needs to take consideration
> of concurrent access if exists.
>
>> > +
>> > +/**
>> > + * of_pwrseq_off_list: do power sequence off for the list
>> > + *
>> > + * This API is used to power off all devices on this bus, it is
>> > + * the opposite operation for of_pwrseq_on_list.
>> > + *
>> > + * @head: the list head for pwrseq instance list on this bus
>> > + */
>> > +void of_pwrseq_off_list(struct list_head *head)
>> > +{
>> > + struct pwrseq *pwrseq;
>> > + struct pwrseq_list_per_dev *pwrseq_list_node, *tmp_node;
>> > +
>> > + list_for_each_entry_safe(pwrseq_list_node, tmp_node, head, list) {
>> > + pwrseq = pwrseq_list_node->pwrseq;
>> > + of_pwrseq_off(pwrseq);
>> > + list_del(&pwrseq_list_node->list);
>> > + kfree(pwrseq_list_node);
>> > + }
>> > +}
>> > +EXPORT_SYMBOL_GPL(of_pwrseq_off_list);
>>
>> This looks horribly inefficient.
>>
>> Is the user expected to create the list from scratch every time things
>> are turned on?
>>
>
> Like I explained above, the power sequence is for hard-wired device on
> board, the list creation and remove are only carried out on driver's
> probe and remove.
Which driver exactly are you referring to?
Thanks,
Rafael
On Thu, Dec 01, 2016 at 10:57:24PM +0100, Rafael J. Wysocki wrote:
> On Tue, Nov 22, 2016 at 4:53 AM, Peter Chen <[email protected]> wrote:
> > On Tue, Nov 22, 2016 at 03:23:12AM +0100, Rafael J. Wysocki wrote:
> >> > @@ -0,0 +1,237 @@
> >> > +/*
> >> > + * core.c power sequence core file
> >> > + *
> >> > + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> >> > + * Author: Peter Chen <[email protected]>
> >> > + *
> >> > + * 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/>.
> >>
> >> The last paragraph is not necessary AFAICS.
> >
> > I just copy it from:
> >
> > https://www.gnu.org/licenses/gpl-howto.en.html
> >
> > If you are concerns about it, I can delete it.
>
> It is redundant, so yes, please.
ok.
>
> >> > +
> >> > +static struct pwrseq *pwrseq_find_available_instance(struct device_node *np)
> >> > +{
> >> > + struct pwrseq *pwrseq;
> >> > +
> >> > + list_for_each_entry(pwrseq, &pwrseq_list, node) {
> >> > + if (pwrseq->used)
> >> > + continue;
> >> > +
> >> > + /* compare compatible string for pwrseq node */
> >> > + if (of_match_node(pwrseq->pwrseq_of_match_table, np)) {
> >> > + pwrseq->used = true;
> >> > + return pwrseq;
> >> > + }
> >> > +
> >> > + /* return generic pwrseq instance */
> >> > + if (!strcmp(pwrseq->pwrseq_of_match_table->compatible,
> >> > + "generic")) {
> >> > + pr_debug("using generic pwrseq instance for %s\n",
> >> > + np->full_name);
> >> > + pwrseq->used = true;
> >> > + return pwrseq;
> >> > + }
> >> > + }
> >> > + pr_warn("Can't find any pwrseq instances for %s\n", np->full_name);
> >>
> >> pr_debug() ?
> >
> > If there is no pwrseq instance for that node, the power sequence on routine will
> > return fail, so I think an warning message is useful for user.
>
> Useful in what way? How is the user supposed to know what happened
> from this message?
Ok, I will change it to debug message.
> >> > + */
> >> > +struct pwrseq *of_pwrseq_on(struct device_node *np)
> >> > +{
> >> > + struct pwrseq *pwrseq;
> >> > + int ret;
> >> > +
> >> > + pwrseq = pwrseq_find_available_instance(np);
> >>
> >> What does guarantee the integrity of ths list at this point?
> >
> > Once the use selects the specific pwrseq library, the library will
> > create an empty one instance during the initialization, and it
> > will be called at postcore_initcall, the device driver has not
> > probed yet.
>
> Which doesn't matter really, because the list is global and some other
> driver using it might have been probed already.
>
> You have a mutex here and it is used for add/remove. Why isn't it
> used for list browsing?
I will add mutex for it, thanks.
> >
> >> > + */
> >> > +int of_pwrseq_on_list(struct device_node *np, struct list_head *head)
> >> > +{
> >> > + struct pwrseq *pwrseq;
> >> > + struct pwrseq_list_per_dev *pwrseq_list_node;
> >> > +
> >> > + pwrseq = of_pwrseq_on(np);
> >> > + if (IS_ERR(pwrseq))
> >> > + return PTR_ERR(pwrseq);
> >> > +
> >> > + pwrseq_list_node = kzalloc(sizeof(*pwrseq_list_node), GFP_KERNEL);
> >>
> >> Why don't you allocate memory before turning the power sequence on?
> >>
> >
> > This list is only for power sequence on instance, if I allocate memory before
> > power sequence on, I need to free it if power sequence on is failed.
>
> So why is that a problem?
>
Not any problems, I will follow your comments.
> >> > + if (!pwrseq_list_node) {
> >> > + of_pwrseq_off(pwrseq);
> >> > + return -ENOMEM;
> >> > + }
> >> > + pwrseq_list_node->pwrseq = pwrseq;
> >> > + list_add(&pwrseq_list_node->list, head);
> >> > +
> >> > + return 0;
> >> > +}
> >> > +EXPORT_SYMBOL_GPL(of_pwrseq_on_list);
> >>
> >> So the caller is supposed to provide a list head of the list to put
> >> the power sequence object into on success, right?
> >
> > Yes
> >
> >>
> >> Can you explain to me what the idea here is, please?
> >>
> >
> > Taking USB devices as an example, there is one power sequence on list
> > per bus, and there are several USB devices on the bus. Using a list,
> > we can record which device is powered sequence on, and only powers
> > sequence off which has already powered sequence on at error path, and
> > power sequence off all devices on the bus when the bus (eg, USB HUB)
> > is removed. (eg, when the bus driver is removed)
>
> Well, I'm not sure I understand this correctly.
>
> What about system suspend/resume and such, for instance?
Thanks, yes, we need to consider PM.
The initial idea for this library is only for power on/off. It does not
take power management into consideration. As an enhancement, we need
to consider PM, and implement pwrseq_suspend/resume accordingly (will
consider concurrent issue). I will add related APIs at pwrseq_generic.c
at next version, and only call clock operations at it (reset gpio
is not needed for PM).
>
> > Usually, the power sequence is only needed for hard-wired devices,
> > the power sequence on is carried out during the bus driver probed,
> > and off if carried out during the bus driver is removed,
> > of_pwrseq_on_list/of_powerseq_off_list is not supposed to be
> > called during the other bus driver life cycles.
> >
> >> Also, what's the protection of the list against concurrent access?
> >>
> >
> > I will add comment that the list creator needs to take consideration
> > of concurrent access if exists.
> >
> >> > +
> >> > +/**
> >> > + * of_pwrseq_off_list: do power sequence off for the list
> >> > + *
> >> > + * This API is used to power off all devices on this bus, it is
> >> > + * the opposite operation for of_pwrseq_on_list.
> >> > + *
> >> > + * @head: the list head for pwrseq instance list on this bus
> >> > + */
> >> > +void of_pwrseq_off_list(struct list_head *head)
> >> > +{
> >> > + struct pwrseq *pwrseq;
> >> > + struct pwrseq_list_per_dev *pwrseq_list_node, *tmp_node;
> >> > +
> >> > + list_for_each_entry_safe(pwrseq_list_node, tmp_node, head, list) {
> >> > + pwrseq = pwrseq_list_node->pwrseq;
> >> > + of_pwrseq_off(pwrseq);
> >> > + list_del(&pwrseq_list_node->list);
> >> > + kfree(pwrseq_list_node);
> >> > + }
> >> > +}
> >> > +EXPORT_SYMBOL_GPL(of_pwrseq_off_list);
> >>
> >> This looks horribly inefficient.
> >>
> >> Is the user expected to create the list from scratch every time things
> >> are turned on?
> >>
> >
> > Like I explained above, the power sequence is for hard-wired device on
> > board, the list creation and remove are only carried out on driver's
> > probe and remove.
>
> Which driver exactly are you referring to?
>
For system PM, the list is still existed. It calls of_pwrseq_suspend/resume
accordingly. The first user for this pwrseq library is USB HUB.
(drivers/usb/core/hub.c).
--
Best Regards,
Peter Chen
On Mon, Nov 14, 2016 at 09:35:51AM +0800, Peter Chen wrote:
> Hi all,
>
> This is a follow-up for my last power sequence framework patch set [1].
> According to Rob Herring and Ulf Hansson's comments[2]. The kinds of
> power sequence instances will be added at postcore_initcall, the match
> criteria is compatible string first, if the compatible string is not
> matched between dts and library, it will try to use generic power sequence.
>
> The host driver just needs to call of_pwrseq_on/of_pwrseq_off
> if only one power sequence instance is needed, for more power sequences
> are used, using of_pwrseq_on_list/of_pwrseq_off_list instead (eg, USB hub driver).
>
> In future, if there are special power sequence requirements, the special
> power sequence library can be created.
>
> This patch set is tested on i.mx6 sabresx evk using a dts change, I use
> two hot-plug devices to simulate this use case, the related binding
> change is updated at patch [1/6], The udoo board changes were tested
> using my last power sequence patch set.[3]
>
> Except for hard-wired MMC and USB devices, I find the USB ULPI PHY also
> need to power on itself before it can be found by ULPI bus.
>
> [1] http://www.spinics.net/lists/linux-usb/msg142755.html
> [2] http://www.spinics.net/lists/linux-usb/msg143106.html
> [3] http://www.spinics.net/lists/linux-usb/msg142815.html
>
> Changes for v10:
> - Improve the kernel-doc for power sequence core, including exported APIs and
> main structure. [Patch 2/8]
> - Change Kconfig, and let the user choose power sequence. [Patch 2/8]
> - Delete EXPORT_SYMBOL and change related APIs as local, these APIs do not
> be intended to export currently. [Patch 2/8]
> - Selete POWER_SEQUENCE at USB core's Kconfig. [Patch 4/8]
Hi Peter,
It is great that you continued the work on this.
I saw (in some previous mails) your repo mentioned:
https://git.kernel.org/cgit/linux/kernel/git/peter.chen/usb.git/
Does it contain the recent version of this patchset?
I want to build on top of it fixes for usb3503 on Odroid U3 board.
Best regards,
Krzysztof
>
> Changes for v9:
> - Add Vaibhav Hiremath's reviewed-by [Patch 4/8]
> - Rebase to v4.9-rc1
>
> Changes for v8:
> - Allocate one extra pwrseq instance if pwrseq_get has succeed, it can avoid
> preallocate instances problem which the number of instance is decided at
> compile time, thanks for Heiko Stuebner's suggestion [Patch 2/8]
> - Delete pwrseq_compatible_sample.c which is the demo purpose to show compatible
> match method. [Patch 2/8]
> - Add Maciej S. Szmigiero's tested-by. [Patch 7/8]
>
> Changes for v7:
> - Create kinds of power sequence instance at postcore_initcall, and match
> the instance with node using compatible string, the beneit of this is
> the host driver doesn't need to consider which pwrseq instance needs
> to be used, and pwrseq core will match it, however, it eats some memories
> if less power sequence instances are used. [Patch 2/8]
> - Add pwrseq_compatible_sample.c to test match pwrseq using device_id. [Patch 2/8]
> - Fix the comments Vaibhav Hiremath adds for error path for clock and do not
> use device_node for parameters at pwrseq_on. [Patch 2/8]
> - Simplify the caller to use power sequence, follows Alan's commnets [Patch 4/8]
> - Tested three pwrseq instances together using both specific compatible string and
> generic libraries.
>
> Changes for v6:
> - Add Matthias Kaehlcke's Reviewed-by and Tested-by. (patch [2/6])
> - Change chipidea core of_node assignment for coming user. (patch [5/6])
> - Applies Joshua Clayton's three dts changes for two boards,
> the USB device's reg has only #address-cells, but without #size-cells.
>
> Changes for v5:
> - Delete pwrseq_register/pwrseq_unregister, which is useless currently
> - Fix the linker error when the pwrseq user is compiled as module
>
> Changes for v4:
> - Create the patch on next-20160722
> - Fix the of_node is not NULL after chipidea driver is unbinded [Patch 5/6]
> - Using more friendly wait method for reset gpio [Patch 2/6]
> - Support multiple input clocks [Patch 2/6]
> - Add Rob Herring's ack for DT changes
> - Add Joshua Clayton's Tested-by
>
> Changes for v3:
> - Delete "power-sequence" property at binding-doc, and change related code
> at both library and user code.
> - Change binding-doc example node name with Rob's comments
> - of_get_named_gpio_flags only gets the gpio, but without setting gpio flags,
> add additional code request gpio with proper gpio flags
> - Add Philipp Zabel's Ack and MAINTAINER's entry
>
> Changes for v2:
> - Delete "pwrseq" prefix and clock-names for properties at dt binding
> - Should use structure not but its pointer for kzalloc
> - Since chipidea core has no of_node, let core's of_node equals glue
> layer's at core's probe
>
> Joshua Clayton (2):
> ARM: dts: imx6qdl: Enable usb node children with <reg>
> ARM: dts: imx6q-evi: Fix onboard hub reset line
>
> Peter Chen (6):
> binding-doc: power: pwrseq-generic: add binding doc for generic power
> sequence library
> power: add power sequence library
> binding-doc: usb: usb-device: add optional properties for power
> sequence
> usb: core: add power sequence handling for USB devices
> usb: chipidea: let chipidea core device of_node equal's glue layer
> device of_node
> ARM: dts: imx6qdl-udoo.dtsi: fix onboard USB HUB property
>
> .../bindings/power/pwrseq/pwrseq-generic.txt | 48 +++++
> .../devicetree/bindings/usb/usb-device.txt | 10 +-
> MAINTAINERS | 9 +
> arch/arm/boot/dts/imx6q-evi.dts | 25 +--
> arch/arm/boot/dts/imx6qdl-udoo.dtsi | 26 ++-
> arch/arm/boot/dts/imx6qdl.dtsi | 6 +
> drivers/power/Kconfig | 1 +
> drivers/power/Makefile | 1 +
> drivers/power/pwrseq/Kconfig | 21 ++
> drivers/power/pwrseq/Makefile | 2 +
> drivers/power/pwrseq/core.c | 237 +++++++++++++++++++++
> drivers/power/pwrseq/pwrseq_generic.c | 183 ++++++++++++++++
> drivers/usb/Kconfig | 1 +
> drivers/usb/chipidea/core.c | 27 ++-
> drivers/usb/core/hub.c | 41 +++-
> drivers/usb/core/hub.h | 1 +
> include/linux/power/pwrseq.h | 60 ++++++
> 17 files changed, 658 insertions(+), 41 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
> create mode 100644 drivers/power/pwrseq/Kconfig
> create mode 100644 drivers/power/pwrseq/Makefile
> create mode 100644 drivers/power/pwrseq/core.c
> create mode 100644 drivers/power/pwrseq/pwrseq_generic.c
> create mode 100644 include/linux/power/pwrseq.h
>
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Dec 19, 2016 at 09:15:04PM +0200, Krzysztof Kozlowski wrote:
> On Mon, Nov 14, 2016 at 09:35:51AM +0800, Peter Chen wrote:
> > Hi all,
> >
> > This is a follow-up for my last power sequence framework patch set [1].
> > According to Rob Herring and Ulf Hansson's comments[2]. The kinds of
> > power sequence instances will be added at postcore_initcall, the match
> > criteria is compatible string first, if the compatible string is not
> > matched between dts and library, it will try to use generic power sequence.
> >
> > The host driver just needs to call of_pwrseq_on/of_pwrseq_off
> > if only one power sequence instance is needed, for more power sequences
> > are used, using of_pwrseq_on_list/of_pwrseq_off_list instead (eg, USB hub driver).
> >
> > In future, if there are special power sequence requirements, the special
> > power sequence library can be created.
> >
> > This patch set is tested on i.mx6 sabresx evk using a dts change, I use
> > two hot-plug devices to simulate this use case, the related binding
> > change is updated at patch [1/6], The udoo board changes were tested
> > using my last power sequence patch set.[3]
> >
> > Except for hard-wired MMC and USB devices, I find the USB ULPI PHY also
> > need to power on itself before it can be found by ULPI bus.
> >
> > [1] http://www.spinics.net/lists/linux-usb/msg142755.html
> > [2] http://www.spinics.net/lists/linux-usb/msg143106.html
> > [3] http://www.spinics.net/lists/linux-usb/msg142815.html
> >
> > Changes for v10:
> > - Improve the kernel-doc for power sequence core, including exported APIs and
> > main structure. [Patch 2/8]
> > - Change Kconfig, and let the user choose power sequence. [Patch 2/8]
> > - Delete EXPORT_SYMBOL and change related APIs as local, these APIs do not
> > be intended to export currently. [Patch 2/8]
> > - Selete POWER_SEQUENCE at USB core's Kconfig. [Patch 4/8]
>
> Hi Peter,
>
> It is great that you continued the work on this.
>
> I saw (in some previous mails) your repo mentioned:
> https://git.kernel.org/cgit/linux/kernel/git/peter.chen/usb.git/
> Does it contain the recent version of this patchset?
>
> I want to build on top of it fixes for usb3503 on Odroid U3 board.
Krzysztof, I put v10 patch set at branch: pwrseq-lib.
It seems there are no more comments I will send my v11 patch set
after new year holiday.
--
Best Regards,
Peter Chen