Re: [PATCH 2/8] power: add power sequence library
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
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
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
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
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
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 distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See t