power: add generic power sequence framework

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

Signed-off-by Peter Chen <[email protected]>

---
 .../power/pwrseq/{pwrseq-simple.txt => mmc-pwrseq-simple.txt}         | 0
 drivers/power/pwrseq/Makefile                                         | 4 ++--
 drivers/power/pwrseq/{pwrseq.c => core.c}                             | 0
 drivers/power/pwrseq/{pwrseq_simple.c => pwrseq_mmc_simple.c}         | 0
 4 files changed, 2 insertions(+), 2 deletions(-)
 rename Documentation/devicetree/bindings/power/pwrseq/{pwrseq-simple.txt => mmc-pwrseq-simple.txt} (100%)
 rename drivers/power/pwrseq/{pwrseq.c => core.c} (100%)
 rename drivers/power/pwrseq/{pwrseq_simple.c => pwrseq_mmc_simple.c} (100%)

Documentation/devicetree/bindings/power/pwrseq/mmc-pwrseq-simple.txt renamed from Documentation/devicetree/bindings/power/pwrseq/pwrseq-simple.txt +0 -0

similarity index 100%
rename from Documentation/devicetree/bindings/power/pwrseq/pwrseq-simple.txt
rename to Documentation/devicetree/bindings/power/pwrseq/mmc-pwrseq-simple.txt

drivers/power/pwrseq/Makefile +0 -0

index 9e40e4b..d475e01 100644

Quoted 22 lines · Peter Chen

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

Signed-off-by Peter Chen <[email protected]>

---
 .../power/pwrseq/{pwrseq-simple.txt => mmc-pwrseq-simple.txt}         | 0
 drivers/power/pwrseq/Makefile                                         | 4 ++--
 drivers/power/pwrseq/{pwrseq.c => core.c}                             | 0
 drivers/power/pwrseq/{pwrseq_simple.c => pwrseq_mmc_simple.c}         | 0
 4 files changed, 2 insertions(+), 2 deletions(-)
 rename Documentation/devicetree/bindings/power/pwrseq/{pwrseq-simple.txt => mmc-pwrseq-simple.txt} (100%)
 rename drivers/power/pwrseq/{pwrseq.c => core.c} (100%)
 rename drivers/power/pwrseq/{pwrseq_simple.c => pwrseq_mmc_simple.c} (100%)

Documentation/devicetree/bindings/power/pwrseq/mmc-pwrseq-simple.txt renamed from Documentation/devicetree/bindings/power/pwrseq/pwrseq-simple.txt +0 -0

similarity index 100%
rename from Documentation/devicetree/bindings/power/pwrseq/pwrseq-simple.txt
rename to Documentation/devicetree/bindings/power/pwrseq/mmc-pwrseq-simple.txt

drivers/power/pwrseq/Makefile +0 -0

index 9e40e4b..d475e01 100644
@@ -1,3 +1,3 @@

drivers/power/pwrseq/Makefile line 2

Quoted 4 lines

drivers/power/pwrseq/Makefile +2 -2

-obj-$(CONFIG_POWER_SEQ)		+= pwrseq.o
-obj-$(CONFIG_POWER_SEQ_SIMPLE)	+= pwrseq_simple.o
+obj-$(CONFIG_POWER_SEQ)		+= core.o
+obj-$(CONFIG_POWER_SEQ_SIMPLE)	+= pwrseq_mmc_simple.o

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

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

Signed-off-by Peter Chen <[email protected]>

---
 .../bindings/power/pwrseq/pwrseq-usb-generic.txt   | 31 ++++++++++++++++++++++
 .../devicetree/bindings/usb/usb-device.txt         |  2 ++
 2 files changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt

Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt new file +2 -0

new file mode 100644
index 0000000..8ad98382
@@ -0,0 +1,31 @@
+The power sequence for generic USB Devices
+

Quoted 18 lines · Peter Chen

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

Signed-off-by Peter Chen <[email protected]>

---
 .../bindings/power/pwrseq/pwrseq-usb-generic.txt   | 31 ++++++++++++++++++++++
 .../devicetree/bindings/usb/usb-device.txt         |  2 ++
 2 files changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt

Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt new file +2 -0

new file mode 100644
index 0000000..8ad98382
@@ -0,0 +1,31 @@
+The power sequence for generic USB Devices
+

Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt line 12

Quoted 10 lines

Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt new file +10 -0

+Some hard-wired USB devices need to do power sequence to let the
+device work normally, the typical power sequence like: enable USB
+PHY clock, toggle reset pin, etc. But current Linux USB driver
+lacks of such code to do it, it may cause some hard-wired USB devices
+works abnormal or can't be recognized by controller at all. The
+power sequence will be done before this device can be found at USB
+bus.
+
+Required properties:
+- compatible : contains "usb-pwrseq-generic".

In case I have not been clear, no.

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

Quoted 32 lines · Rob Herring

Quoted 18 lines · Peter Chen

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

Signed-off-by Peter Chen <[email protected]>

---
 .../bindings/power/pwrseq/pwrseq-usb-generic.txt   | 31 ++++++++++++++++++++++
 .../devicetree/bindings/usb/usb-device.txt         |  2 ++
 2 files changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt

Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt new file +2 -0

new file mode 100644
index 0000000..8ad98382
@@ -0,0 +1,31 @@
+The power sequence for generic USB Devices
+

Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt line 12

Quoted 10 lines

Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt new file +10 -0

+Some hard-wired USB devices need to do power sequence to let the
+device work normally, the typical power sequence like: enable USB
+PHY clock, toggle reset pin, etc. But current Linux USB driver
+lacks of such code to do it, it may cause some hard-wired USB devices
+works abnormal or can't be recognized by controller at all. The
+power sequence will be done before this device can be found at USB
+bus.
+
+Required properties:
+- compatible : contains "usb-pwrseq-generic".

In case I have not been clear, no.

Quoted 6 lines

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

I see.

Would you agree with below:

&usbotg1 {
        vbus-supply = <&reg_usb_otg1_vbus>;
        pinctrl-names = "default";
        pinctrl-0 = <&pinctrl_usb_otg1_id>;
        status = "okay";

        #address-cells = <1>;
        #size-cells = <0>;
        hub: genesys at 1 {
                compatible = "usb5e3,608";
                reg = <1>;

                power-sequence;
                reset-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>; /* hub reset pin */
                reset-duration-us = <10>;
                clocks = <&clks IMX6SX_CLK_CKO>;

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

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

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

Hi,

Quoted 75 lines · Peter Chen

Quoted 32 lines · Rob Herring

Quoted 18 lines · Peter Chen

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

Signed-off-by Peter Chen <[email protected]>

---
 .../bindings/power/pwrseq/pwrseq-usb-generic.txt   | 31 ++++++++++++++++++++++
 .../devicetree/bindings/usb/usb-device.txt         |  2 ++
 2 files changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt

Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt new file +2 -0

new file mode 100644
index 0000000..8ad98382
@@ -0,0 +1,31 @@
+The power sequence for generic USB Devices
+

Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt line 12

Quoted 10 lines

Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt new file +10 -0

+Some hard-wired USB devices need to do power sequence to let the
+device work normally, the typical power sequence like: enable USB
+PHY clock, toggle reset pin, etc. But current Linux USB driver
+lacks of such code to do it, it may cause some hard-wired USB devices
+works abnormal or can't be recognized by controller at all. The
+power sequence will be done before this device can be found at USB
+bus.
+
+Required properties:
+- compatible : contains "usb-pwrseq-generic".

In case I have not been clear, no.

Quoted 6 lines

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

I see.

Would you agree with below:

&usbotg1 {
        vbus-supply = <&reg_usb_otg1_vbus>;
        pinctrl-names = "default";
        pinctrl-0 = <&pinctrl_usb_otg1_id>;
        status = "okay";

        #address-cells = <1>;
        #size-cells = <0>;
        hub: genesys at 1 {
                compatible = "usb5e3,608";
                reg = <1>;

                power-sequence;
                reset-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>; /* hub reset pin */
                reset-duration-us = <10>;
                clocks = <&clks IMX6SX_CLK_CKO>;

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

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

Quoted 4 lines

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

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

Quoted 32 lines · Rob Herring

Quoted 18 lines · Peter Chen

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

Signed-off-by Peter Chen <[email protected]>

---
 .../bindings/power/pwrseq/pwrseq-usb-generic.txt   | 31 ++++++++++++++++++++++
 .../devicetree/bindings/usb/usb-device.txt         |  2 ++
 2 files changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt

Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt new file +2 -0

new file mode 100644
index 0000000..8ad98382
@@ -0,0 +1,31 @@
+The power sequence for generic USB Devices
+

Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt line 12

Quoted 10 lines

Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt new file +10 -0

+Some hard-wired USB devices need to do power sequence to let the
+device work normally, the typical power sequence like: enable USB
+PHY clock, toggle reset pin, etc. But current Linux USB driver
+lacks of such code to do it, it may cause some hard-wired USB devices
+works abnormal or can't be recognized by controller at all. The
+power sequence will be done before this device can be found at USB
+bus.
+
+Required properties:
+- compatible : contains "usb-pwrseq-generic".

In case I have not been clear, no.

Quoted 6 lines

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

I see.

Would you agree with below:

&usbotg1 {
	vbus-supply = <&reg_usb_otg1_vbus>;
	pinctrl-names = "default";
	pinctrl-0 = <&pinctrl_usb_otg1_id>;
	status = "okay";

	#address-cells = <1>;

	#size-cells = <0>;
	hub: genesys at 1 {
		compatible = "usb5e3,608";
		reg = <1>;

		power-sequence;
		reset-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>; /* hub reset pin */
		reset-duration-us = <10>;

		clocks = <&clks IMX6SX_CLK_CKO>;

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

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

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

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

Quoted 3 lines · Peter Chen

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

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

Quoted 40 lines · Peter Chen

Quoted 32 lines · Rob Herring

Quoted 18 lines · Peter Chen

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

Signed-off-by Peter Chen <[email protected]>

---
 .../bindings/power/pwrseq/pwrseq-usb-generic.txt   | 31 ++++++++++++++++++++++
 .../devicetree/bindings/usb/usb-device.txt         |  2 ++
 2 files changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt

Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt new file +2 -0

new file mode 100644
index 0000000..8ad98382
@@ -0,0 +1,31 @@
+The power sequence for generic USB Devices
+

Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt line 12

Quoted 10 lines

Documentation/devicetree/bindings/power/pwrseq/pwrseq-usb-generic.txt new file +10 -0

+Some hard-wired USB devices need to do power sequence to let the
+device work normally, the typical power sequence like: enable USB
+PHY clock, toggle reset pin, etc. But current Linux USB driver
+lacks of such code to do it, it may cause some hard-wired USB devices
+works abnormal or can't be recognized by controller at all. The
+power sequence will be done before this device can be found at USB
+bus.
+
+Required properties:
+- compatible : contains "usb-pwrseq-generic".

In case I have not been clear, no.

Quoted 6 lines

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

Quoted 3 lines

I see.

Would you agree with below:

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

Quoted 12 lines

&usbotg1 {
	vbus-supply = <&reg_usb_otg1_vbus>;
	pinctrl-names = "default";
	pinctrl-0 = <&pinctrl_usb_otg1_id>;
	status = "okay";

	#address-cells = <1>;
	#size-cells = <0>;
	hub: genesys at 1 {
		compatible = "usb5e3,608";
		reg = <1>;

Quoted 3 lines

		power-sequence;
		reset-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>; /* hub reset pin */
		reset-duration-us = <10>;

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

Quoted 16 lines

		clocks = <&clks IMX6SX_CLK_CKO>;

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

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

Quoted 3 lines

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

This I have issue with. If you are creating a platform device here, you are trying to work-around limitations in the linux driver model.

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

Quoted 5 lines · Rob Herring

Quoted 4 lines · Peter Chen

Quoted 3 lines · Rob Herring

Quoted 2 lines · Peter Chen

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

Quoted 16 lines

Quoted 16 lines

		clocks = <&clks IMX6SX_CLK_CKO>;

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

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

Quoted 6 lines

Quoted 3 lines

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

This I have issue with. If you are creating a platform device here, you are trying to work-around limitations in the linux driver model.

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

In drivers/usb/core/hub.c

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

Quoted 9 lines

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

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

Quoted 6 lines · Peter Chen

Quoted 5 lines · Rob Herring

Quoted 4 lines · Peter Chen

Quoted 3 lines · Rob Herring

Quoted 2 lines · Peter Chen

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

[...]

Quoted 29 lines

Quoted 16 lines

Quoted 16 lines

		clocks = <&clks IMX6SX_CLK_CKO>;

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

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

Quoted 6 lines

Quoted 3 lines

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

This I have issue with. If you are creating a platform device here, you are trying to work-around limitations in the linux driver model.

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

In drivers/usb/core/hub.c

Quoted 10 lines

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

Why does this function need to do anything more than:

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

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

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

[...]

Quoted 10 lines

Quoted 9 lines

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

Quoted 4 lines

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

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

Rob

Quoted 6 lines · Peter Chen

Quoted 5 lines · Rob Herring

Quoted 4 lines · Peter Chen

Quoted 3 lines · Rob Herring

Quoted 2 lines · Peter Chen

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

[...]

Quoted 16 lines

Quoted 16 lines

Quoted 16 lines

          clocks = <&clks IMX6SX_CLK_CKO>;

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

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

};
};

Quoted 6 lines

Quoted 6 lines

Quoted 3 lines

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

This I have issue with. If you are creating a platform device here, you are trying to work-around limitations in the linux driver model.

Quoted 7 lines

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

In drivers/usb/core/hub.c

Quoted 10 lines

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

Why does this function need to do anything more than:

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

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

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

Quoted 26 lines · Rob Herring

Quoted 6 lines · Peter Chen

Quoted 5 lines · Rob Herring

Quoted 4 lines · Peter Chen

Quoted 3 lines · Rob Herring

Quoted 2 lines · Peter Chen

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

[...]

Quoted 16 lines

Quoted 16 lines

Quoted 16 lines

          clocks = <&clks IMX6SX_CLK_CKO>;

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

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

};
};

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Quoted 3 lines

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

This I have issue with. If you are creating a platform device here, you are trying to work-around limitations in the linux driver model.

I somewhat understand your point.

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

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

Quoted 24 lines

Quoted 6 lines

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

In drivers/usb/core/hub.c

Quoted 10 lines

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

Why does this function need to do anything more than:

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

Quoted 5 lines

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

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

Complexity?

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

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

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

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

From what I recall, all attempts have worked around the problem that

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

In this patch, it will do power on sequence at hub's probe for all devices under this hub (includes root hub) if this device is described at dts and there is a phandle "usb-pwrseq" for it.

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

Signed-off-by Peter Chen <[email protected]>

---
 drivers/usb/core/hub.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++---
 drivers/usb/core/hub.h |  1 +
 include/linux/pwrseq.h |  6 +++++
 3 files changed, 77 insertions(+), 3 deletions(-)

drivers/usb/core/hub.c +1 -0

index bee1351..cc0f942 100644
@@ -26,6 +26,7 @@
 #include <linux/mutex.h>
 #include <linux/random.h>
 #include <linux/pm_qos.h>
+#include <linux/pwrseq.h>
 
 #include <asm/uaccess.h>
 #include <asm/byteorder.h>

Hi all,

We have an well-known problem that the device needs to do some power sequence before it can be recognized by related host, the typical example like hard-wired mmc devices and usb devices. This power sequence is hard to be described at device tree and handled by related host driver, so we have created a common power sequence framework to handle this requirement. The generic code is supplied some common helpers from host driver, and individual power sequence driver handles kinds of power sequence for devices.

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

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

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

This patch set is tested on i.mx6 sabresx evk using a dts change, I use two hot-plug devices to simulate this use case. The udoo board changes were tested using my last power sequence patch set. [2]

@Maciej S. Szmigiero, Oscar, would you please test them on imx6qdl udoo boards?

Hi Peter,

Quoted 25 lines · Peter Chen

Hi all,

We have an well-known problem that the device needs to do some power sequence before it can be recognized by related host, the typical example like hard-wired mmc devices and usb devices. This power sequence is hard to be described at device tree and handled by related host driver, so we have created a common power sequence framework to handle this requirement. The generic code is supplied some common helpers from host driver, and individual power sequence driver handles kinds of power sequence for devices.

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

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

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

Quoted 6 lines

This patch set is tested on i.mx6 sabresx evk using a dts change, I use two hot-plug devices to simulate this use case. The udoo board changes were tested using my last power sequence patch set. [2]

@Maciej S. Szmigiero, Oscar, would you please test them on imx6qdl udoo boards?

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

Tested-by Maciej S. Szmigiero <[email protected]>

Maciej