Re: [PATCH 2/8] power: add power sequence library

[Peter Chen]

On Fri, Feb 03, 2017 at 10:14:31PM +0100, Rafael J. Wysocki wrote:
> On Friday, February 03, 2017 04:16:15 PM Peter Chen wrote:
> > On Wed, Feb 01, 2017 at 09:08:17AM +0100, Greg Kroah-Hartman wrote:
> > > On Wed, Feb 01, 2017 at 12:10:17AM +0100, Rafael J. Wysocki wrote:
> > > > On Tue, Jan 3, 2017 at 7:33 AM, 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.
> > > > >
> > > > > Signed-off-by: Peter Chen 
> > > > > Tested-by: Maciej S. Szmigiero 
> > > > > Tested-by Joshua Clayton 
> > > > > Reviewed-by: Matthias Kaehlcke 
> > > > > Tested-by: Matthias Kaehlcke 
> > > > 
> > > > Quite honestly, I have a really hard time with trying to follow this
> > > > code and the total lack of documentation makes it even harder. 
> > 
> > Sorry about that, Is it ok I add the design doc at:
> > Documentation/power/power-sequence/design.rst?
> 
> You can do that if you think it will address the request to explain the 
> design.
> 
> > > > particular, the generic power sequence code is not even commented at
> > > > all, 
> > 
> > The generic power sequence code just implements the APIs which are 
> > called at power/pwrseq/core.c, and those API are commented at
> > include/linux/power/pwrseq.h. Anyway, I will add more comments at it.
> 
> It actually seems to be doing more than that and I'm not sure why the code in
> core.c is necessary at all.  The "generic" thing seems to be the only user of
> it anyway and the callbacks seem to be tailored to its needs.
> 

Currently, the "generic" pwrseq is the only use case. But some devices may have
custom power sequence [1], and MMC devices (mmc card/wifi) have two
power sequences (drivers/mmc/core/pwrseq_emmc.c, 
drivers/mmc/core/pwrseq_simple.c)

I have an example use case at v7 named pwrseq_compatible_sample.c for
those custom use case [2].

[1] https://www.spinics.net/lists/devicetree/msg139756.html
[2] https://lkml.org/lkml/2016/9/19/972

-- 

Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/8] power: add power sequence library

[Rafael J. Wysocki]

On Friday, February 03, 2017 04:16:15 PM Peter Chen wrote:
> On Wed, Feb 01, 2017 at 09:08:17AM +0100, Greg Kroah-Hartman wrote:
> > On Wed, Feb 01, 2017 at 12:10:17AM +0100, Rafael J. Wysocki wrote:
> > > On Tue, Jan 3, 2017 at 7:33 AM, 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.
> > > >
> > > > Signed-off-by: Peter Chen 
> > > > Tested-by: Maciej S. Szmigiero 
> > > > Tested-by Joshua Clayton 
> > > > Reviewed-by: Matthias Kaehlcke 
> > > > Tested-by: Matthias Kaehlcke 
> > > 
> > > Quite honestly, I have a really hard time with trying to follow this
> > > code and the total lack of documentation makes it even harder. 
> 
> Sorry about that, Is it ok I add the design doc at:
> Documentation/power/power-sequence/design.rst?

You can do that if you think it will address the request to explain the design.

> > > particular, the generic power sequence code is not even commented at
> > > all, 
> 
> The generic power sequence code just implements the APIs which are 
> called at power/pwrseq/core.c, and those API are commented at
> include/linux/power/pwrseq.h. Anyway, I will add more comments at it.

It actually seems to be doing more than that and I'm not sure why the code in
core.c is necessary at all.  The "generic" thing seems to be the only user of
it anyway and the callbacks seem to be tailored to its needs.

Thanks,
Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/8] power: add power sequence library

[Peter Chen]

On Wed, Feb 01, 2017 at 09:08:17AM +0100, Greg Kroah-Hartman wrote:
> On Wed, Feb 01, 2017 at 12:10:17AM +0100, Rafael J. Wysocki wrote:
> > On Tue, Jan 3, 2017 at 7:33 AM, 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.
> > >
> > > Signed-off-by: Peter Chen 
> > > Tested-by: Maciej S. Szmigiero 
> > > Tested-by Joshua Clayton 
> > > Reviewed-by: Matthias Kaehlcke 
> > > Tested-by: Matthias Kaehlcke 
> > 
> > Quite honestly, I have a really hard time with trying to follow this
> > code and the total lack of documentation makes it even harder. 

Sorry about that, Is it ok I add the design doc at:
Documentation/power/power-sequence/design.rst?

> > particular, the generic power sequence code is not even commented at
> > all, 

The generic power sequence code just implements the APIs which are 
called at power/pwrseq/core.c, and those API are commented at
include/linux/power/pwrseq.h. Anyway, I will add more comments at it.

-- 

Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/8] power: add power sequence library

[Greg Kroah-Hartman]

On Wed, Feb 01, 2017 at 12:10:17AM +0100, Rafael J. Wysocki wrote:
> On Tue, Jan 3, 2017 at 7:33 AM, 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.
> >
> > Signed-off-by: Peter Chen 
> > Tested-by: Maciej S. Szmigiero 
> > Tested-by Joshua Clayton 
> > Reviewed-by: Matthias Kaehlcke 
> > Tested-by: Matthias Kaehlcke 
> 
> Quite honestly, I have a really hard time with trying to follow this
> code and the total lack of documentation makes it even harder.  In
> particular, the generic power sequence code is not even commented at
> all, so it really is hard to say how this is going to work, let alone
> deciding whether or not to apply it.
> 
> Plus, of course, the USB core changes must be acked by the maintainer
> thereof for me to be able to handle the series.

Ah crap, I wanted you to explain it as I too couldn't figure it out :)

> But at this point I basically need you to explain the design to me, please.

Same here.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/8] power: add power sequence library

[Rafael J. Wysocki]

On Tue, Jan 3, 2017 at 7:33 AM, 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.
>
> Signed-off-by: Peter Chen 
> Tested-by: Maciej S. Szmigiero 
> Tested-by Joshua Clayton 
> Reviewed-by: Matthias Kaehlcke 
> Tested-by: Matthias Kaehlcke 

Quite honestly, I have a really hard time with trying to follow this
code and the total lack of documentation makes it even harder.  In
particular, the generic power sequence code is not even commented at
all, so it really is hard to say how this is going to work, let alone
deciding whether or not to apply it.

Plus, of course, the USB core changes must be acked by the maintainer
thereof for me to be able to handle the series.

But at this point I basically need you to explain the design to me, please.

Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/8] power: add power sequence library

[Peter Chen]

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 
Tested-by: Maciej S. Szmigiero 
Tested-by Joshua Clayton 
Reviewed-by: Matthias Kaehlcke 
Tested-by: Matthias Kaehlcke 
---
 MAINTAINERS   |   9 +
 drivers/power/Kconfig |   1 +
 drivers/power/Makefile|   1 +
 drivers/power/pwrseq/Kconfig  |  20 ++
 drivers/power/pwrseq/Makefile |   2 +
 drivers/power/pwrseq/core.c   | 335 ++
 drivers/power/pwrseq/pwrseq_generic.c | 224 +++
 include/linux/power/pwrseq.h  |  81 
 8 files changed, 673 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 cfff2c9..ae2aa25 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9828,6 +9828,15 @@ F:   include/linux/pm_*
 F: include/linux/powercap.h
 F: drivers/powercap/
 
+POWER SEQUENCE LIBRARY
+M: Peter Chen 
+T: git git://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git
+L: linux...@vger.kernel.org
+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 
 L: linux...@vger.kernel.org
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 000..c6b3569
--- /dev/null
+++ b/drivers/power/pwrseq/Kconfig
@@ -0,0 +1,20 @@
+#
+# Power Sequence library
+#
+
+menuconfig POWER_SEQUENCE
+   bool "Power sequence control"
+   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"
+   depends on OF
+   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 000..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 000..3d19e62
--- /dev/null
+++ b/drivers/power/pwrseq/core.c
@@ -0,0 +1,335 @@
+/*
+ * core.c  power sequence core file
+ *
+ * Copyright (C) 2016 Freescale Semiconductor, Inc.
+ * Author: Peter Chen 
+ *
+ * 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