On 20 June 2014 10:14, Hans de Goede [off-list ref] wrote:

Hi,

On 06/20/2014 10:02 AM, Olof Johansson wrote:

quoted

On Fri, Jun 20, 2014 at 12:27 AM, Hans de Goede [off-list ref] wrote:

quoted

Hi,

On 06/19/2014 07:18 PM, Olof Johansson wrote:

quoted

Hi,



On Thu, Jun 19, 2014 at 6:04 AM, Ulf Hansson [off-list ref] wrote:

quoted

The pwrseq subsystem handles complex power sequences, typically useful
for subsystems that makes use of discoverable buses, like for example
MMC and I2C.

The pwrseq subsystem is dependant on CONFIG_OF to be able to parse a DT
childnode to find out what power sequence method to bind for a device.

From the DT childnode, the pwrseq DT parser tries to locate a
"power-method" property, which string is matched towards the list of
supported pwrseq methods.

Each pwrseq method implements it's own power sequence and interfaces
the pwrseq core through a few callback functions.

To instantiate a pwrseq method, clients shall use the devm_pwrseq_get()
API. If needed, clients can explicity drop the references to a pwrseq
method using devm_pwrseq_put() API.

Besides instantiation, the pwrseq API provides clients opportunity to
select a certain power state. In this intial version, PWRSEQ_POWER_ON
and PWRSEQ_POWER_OFF are supported. Those are also mandatory for each
pwrseq method to support.

Signed-off-by: Ulf Hansson <redacted>
---
 .../devicetree/bindings/pwrseq/pwrseq.txt          |   48 ++++++
 drivers/Makefile                                   |    2 +-
 drivers/pwrseq/Makefile                            |    2 +
 drivers/pwrseq/core.c                              |  175 ++++++++++++++++++++
 drivers/pwrseq/core.h                              |   37 +++++
 drivers/pwrseq/method.c                            |   38 +++++
 include/linux/pwrseq.h                             |   50 ++++++
 7 files changed, 351 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/pwrseq/pwrseq.txt
 create mode 100644 drivers/pwrseq/Makefile
 create mode 100644 drivers/pwrseq/core.c
 create mode 100644 drivers/pwrseq/core.h
 create mode 100644 drivers/pwrseq/method.c
 create mode 100644 include/linux/pwrseq.h

diff --git a/Documentation/devicetree/bindings/pwrseq/pwrseq.txt b/Documentation/devicetree/bindings/pwrseq/pwrseq.txt
new file mode 100644
index 0000000..80848ae
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwrseq/pwrseq.txt

@@ -0,0 +1,48 @@
+Power sequence DT bindings
+
+Each power sequence method has a corresponding "power-method" property string.
+This property shall be set in a subnode for a device. That subnode should also
+describe resourses which are specific to that power method.
+
+Do note, power sequences as such isn't encoded through DT. Instead those are
+implemented by each power method.
+
+Required subnode properties:
+- power-method: should contain the string for the power method to bind.
+
+       Supported power methods: None.
+
+Example:
+
+Note, the "clock" power method in this example isn't actually supported, but
+used to visualize how a childnode could be described.
+
+// WLAN SDIO channel
+sdi1_per2 at 80118000 {
+       compatible = "arm,pl18x", "arm,primecell";
+       reg = <0x80118000 0x1000>;
+       interrupts = <0 50 IRQ_TYPE_LEVEL_HIGH>;
+
+       dmas = <&dma 32 0 0x2>, /* Logical - DevToMem */
+              <&dma 32 0 0x0>; /* Logical - MemToDev */
+       dma-names = "rx", "tx";
+
+       clocks = <&prcc_kclk 2 4>, <&prcc_pclk 2 6>;
+       clock-names = "sdi", "apb_pclk";
+
+       arm,primecell-periphid = <0x10480180>;
+       max-frequency = <100000000>;
+       bus-width = <4>;
+       non-removable;
+       pinctrl-names = "default", "sleep";
+       pinctrl-0 = <&sdi1_default_mode>;
+       pinctrl-1 = <&sdi1_sleep_mode>;
+
+       status = "okay";
+
+       pwrseq: pwrseq1 {
+               power-method = "clock";
+               clocks = <&someclk 1 2>, <&someclk 3 4>;
+               clock-names = "pwrseq1", "pwrseq2";
+       };

I am strongly against the subnode approach as a general framework. We
don't have a subnode for interrupts, nor for clocks or pinctrl. So why
should we have it for the power sequencing?

Sure, that fits the linux driver model better, but that's irrelevant
w.r.t. describing the hardware.

Actually this is about describing the hardware, when you have e.g. an
mmc device which needs pwrseq, there will be 2 sets of certain
resources, ie clocks for the host controller and clocks going directly
to the mmc device. I think putting those both in the same subnode is
a BAD idea, so we really do need a subnode to group the pwrseq resources
together.

I disagree.

The clock is the input to the module, and it is what needs to be
enabled for the module to work. It's not the input to some
power-sequence component on the module, or next to the module on the
bus.

Right, it is an input to the sdio-module, not to the mmc-host, so its an
input to a different piece of hardware (at different ends of the mmc bus),
but since the mmc-bus normally is fully discoverable we've no node for the
other end of the bus.

So from the mmc-host pov, which is the one which needs to bind the pwrseq
driver, as that needs to be done before it can probe its bus, this is
a different piece of hardware, hence a subnode to the host makes perfect
sense.  This is in no way part of the host, so certainly it does not belong
inside the hosts subnode.

I fully agree with you Hans here.

If we were to put this information in the host's DT node, that would
be a wrong description of the hardware. Currently, I can't think of
anything better than a subnode, but I am open to suggestions.

Kind regards
Uffe

quoted

It probably makes sense to not use the standard names for the new
resources.

I disagree, being able to use standard names is very useful, and actually
is a must if we don't want to have to have special versions of devm_get_clk,
and all other devm_get_xxx esp. for pwrseq stuff.

Regards,

Hans