On Wed, Jun 22, 2016 at 4:09 AM, Ulf Hansson [off-list ref] wrote:

On 21 June 2016 at 23:26, Rob Herring [off-list ref] wrote:

quoted

On Tue, Jun 21, 2016 at 10:11:17AM +0800, Peter Chen wrote:

quoted

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

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.

I somewhat understand your point.

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

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

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

quoted

quoted

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

In drivers/usb/core/hub.c

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

Why does this function need to do anything more than:

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

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

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

Complexity?

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

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

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

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

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

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

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

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

the driver model has no way to either force probe or provide a
pre-probe hook on probeable buses. This then leads to the work-around
defining the DT binding.

Rob