Thread (12 messages) 12 messages, 7 authors, 2016-06-24

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

From: robh@kernel.org (Rob Herring)
Date: 2016-06-21 21:26:53
Also in: linux-devicetree, linux-mmc, linux-pm

On Tue, Jun 21, 2016 at 10:11:17AM +0800, Peter Chen wrote:
On Mon, Jun 20, 2016 at 11:16:07AM -0500, Rob Herring wrote:
quoted
On Mon, Jun 20, 2016 at 07:26:51PM +0800, Peter Chen wrote:
quoted
On Fri, Jun 17, 2016 at 12:16:48PM -0500, Rob Herring wrote:
quoted
On Fri, Jun 17, 2016 at 5:09 AM, Peter Chen [off-list ref] wrote:
quoted
Add binding doc for generic usb power sequence driver, and update
generic usb device binding-doc accordingly.
[...]
quoted
quoted
		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. 
This I have issue with. If you are creating a platform device here, you 
are trying to work-around limitations in the linux driver model.
My current solution like below, but it seems you didn't agree with that.
I just double confirm here, if you don't, I give up the solution for
using generic power sequence framework.

In drivers/usb/core/hub.c

	for_each_child_of_node(parent->of_node, node) {
		hdev_pwrseq = pwrseq_alloc(node, "usb_pwrseq_generic");
		if (!IS_ERR_OR_NULL(hdev_pwrseq)) {
			pwrseq_node = kzalloc(sizeof(pwrseq_node), GFP_KERNEL);
			if (!pwrseq_node) {
				ret = -ENOMEM;
				goto err1;
			}
			/* power on sequence */
			ret = pwrseq_pre_power_on(hdev_pwrseq);
Why does this function need to do anything more than:

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

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

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

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

Rob
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help