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

2016-12-01 Thread Peter Chen 
On Thu, Dec 01, 2016 at 10:57:24PM +0100, Rafael J. Wysocki wrote:
> On Tue, Nov 22, 2016 at 4:53 AM, Peter Chen  wrote:
> > On Tue, Nov 22, 2016 at 03:23:12AM +0100, Rafael J. Wysocki wrote:
> >> > @@ -0,0 +1,237 @@
> >> > +/*
> >> > + * 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 the
> >> > + * GNU General Public License for more details.
> >> > + *
> >> > + * You should have received a copy of the GNU General Public License
> >> > + * along with this program.  If not, see .
> >>
> >> The last paragraph is not necessary AFAICS.
> >
> > I just copy it from:
> >
> > https://www.gnu.org/licenses/gpl-howto.en.html
> >
> > If you are concerns about it, I can delete it.
> 
> It is redundant, so yes, please.

ok.

> 
> >> > +
> >> > +static struct pwrseq *pwrseq_find_available_instance(struct device_node 
> >> > *np)
> >> > +{
> >> > +   struct pwrseq *pwrseq;
> >> > +
> >> > +   list_for_each_entry(pwrseq, _list, node) {
> >> > +   if (pwrseq->used)
> >> > +   continue;
> >> > +
> >> > +   /* compare compatible string for pwrseq node */
> >> > +   if (of_match_node(pwrseq->pwrseq_of_match_table, np)) {
> >> > +   pwrseq->used = true;
> >> > +   return pwrseq;
> >> > +   }
> >> > +
> >> > +   /* return generic pwrseq instance */
> >> > +   if (!strcmp(pwrseq->pwrseq_of_match_table->compatible,
> >> > +   "generic")) {
> >> > +   pr_debug("using generic pwrseq instance for 
> >> > %s\n",
> >> > +   np->full_name);
> >> > +   pwrseq->used = true;
> >> > +   return pwrseq;
> >> > +   }
> >> > +   }
> >> > +   pr_warn("Can't find any pwrseq instances for %s\n", 
> >> > np->full_name);
> >>
> >> pr_debug() ?
> >
> > If there is no pwrseq instance for that node, the power sequence on routine 
> > will
> > return fail, so I think an warning message is useful for user.
> 
> Useful in what way?  How is the user supposed to know what happened
> from this message?

Ok, I will change it to debug message.

> >> > + */
> >> > +struct pwrseq *of_pwrseq_on(struct device_node *np)
> >> > +{
> >> > +   struct pwrseq *pwrseq;
> >> > +   int ret;
> >> > +
> >> > +   pwrseq = pwrseq_find_available_instance(np);
> >>
> >> What does guarantee the integrity of ths list at this point?
> >
> > Once the use selects the specific pwrseq library, the library will
> > create an empty one instance during the initialization, and it
> > will be called at postcore_initcall, the device driver has not
> > probed yet.
> 
> Which doesn't matter really, because the list is global and some other
> driver using it might have been probed already.
> 
> You have a mutex here and it is used for add/remove.  Why isn't it
> used for list browsing?

I will add mutex for it, thanks.

> >
> >> > + */
> >> > +int of_pwrseq_on_list(struct device_node *np, struct list_head *head)
> >> > +{
> >> > +   struct pwrseq *pwrseq;
> >> > +   struct pwrseq_list_per_dev *pwrseq_list_node;
> >> > +
> >> > +   pwrseq = of_pwrseq_on(np);
> >> > +   if (IS_ERR(pwrseq))
> >> > +   return PTR_ERR(pwrseq);
> >> > +
> >> > +   pwrseq_list_node = kzalloc(sizeof(*pwrseq_list_node), 
> >> > GFP_KERNEL);
> >>
> >> Why don't you allocate memory before turning the power sequence on?
> >>
> >
> > This list is only for power sequence on instance, if I allocate memory 
> > before
> > power sequence on, I need to free it if power sequence on is failed.
> 
> So why is that a problem?
> 

Not any problems, I will follow your comments.

> >> > +   if (!pwrseq_list_node) {
> >> > +   of_pwrseq_off(pwrseq);
> >> > +   return -ENOMEM;
> >> > +   }
> >> > +   pwrseq_list_node->pwrseq = pwrseq;
> >> > +   list_add(_list_node->list, head);
> >> > +
> >> > +   return 0;
> >> > +}
> >> > +EXPORT_SYMBOL_GPL(of_pwrseq_on_list);
> >>
> >> So the caller is supposed to provide a list head of the list to put
> >> the power sequence object into on success, right?
> >
> > Yes
> >
> >>
> >> Can you explain to me what the idea here is, please?
> >>
> >
> > Taking USB devices as an example, there is one power sequence on list
> >

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

2016-12-01 Thread Rafael J. Wysocki 
On Tue, Nov 22, 2016 at 4:53 AM, Peter Chen  wrote:
> On Tue, Nov 22, 2016 at 03:23:12AM +0100, Rafael J. Wysocki wrote:
>> > @@ -0,0 +1,237 @@
>> > +/*
>> > + * 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 the
>> > + * GNU General Public License for more details.
>> > + *
>> > + * You should have received a copy of the GNU General Public License
>> > + * along with this program.  If not, see .
>>
>> The last paragraph is not necessary AFAICS.
>
> I just copy it from:
>
> https://www.gnu.org/licenses/gpl-howto.en.html
>
> If you are concerns about it, I can delete it.

It is redundant, so yes, please.

>> > +
>> > +static struct pwrseq *pwrseq_find_available_instance(struct device_node 
>> > *np)
>> > +{
>> > +   struct pwrseq *pwrseq;
>> > +
>> > +   list_for_each_entry(pwrseq, _list, node) {
>> > +   if (pwrseq->used)
>> > +   continue;
>> > +
>> > +   /* compare compatible string for pwrseq node */
>> > +   if (of_match_node(pwrseq->pwrseq_of_match_table, np)) {
>> > +   pwrseq->used = true;
>> > +   return pwrseq;
>> > +   }
>> > +
>> > +   /* return generic pwrseq instance */
>> > +   if (!strcmp(pwrseq->pwrseq_of_match_table->compatible,
>> > +   "generic")) {
>> > +   pr_debug("using generic pwrseq instance for %s\n",
>> > +   np->full_name);
>> > +   pwrseq->used = true;
>> > +   return pwrseq;
>> > +   }
>> > +   }
>> > +   pr_warn("Can't find any pwrseq instances for %s\n", np->full_name);
>>
>> pr_debug() ?
>
> If there is no pwrseq instance for that node, the power sequence on routine 
> will
> return fail, so I think an warning message is useful for user.

Useful in what way?  How is the user supposed to know what happened
from this message?

>>
>> > +
>> > +   return NULL;
>> > +}
>> > +
>> > +/**
>> > + * of_pwrseq_on: do power sequence on for device node
>>
>> of_pwrseq_on - Carry out power sequence on for device node
>>
>> Argument description should follow this line.
>>
>> > + *
>> > + * This API is used to power on single device, if the host
>> > + * controller only needs to handle one child device (this device
>> > + * node points to), use this API. If multiply devices are needed
>> > + * to handle on bus, use of_pwrseq_on_list.
>>
>> That's unclear.
>>
>> What about "Carry out a single device power on.  If multiple devices
>> need to be handled, use of_pwrseq_on_list() instead."
>>
>> > + *
>> > + * @np: the device node would like to power on
>> > + *
>> > + * On successful, it returns pwrseq instance, otherwise an error value.
>>
>> "Return a pointer to the power sequence instance on success, or an
>> error code otherwise."
>>
>
> Ok, will change.
>
>> > + */
>> > +struct pwrseq *of_pwrseq_on(struct device_node *np)
>> > +{
>> > +   struct pwrseq *pwrseq;
>> > +   int ret;
>> > +
>> > +   pwrseq = pwrseq_find_available_instance(np);
>>
>> What does guarantee the integrity of ths list at this point?
>
> Once the use selects the specific pwrseq library, the library will
> create an empty one instance during the initialization, and it
> will be called at postcore_initcall, the device driver has not
> probed yet.

Which doesn't matter really, because the list is global and some other
driver using it might have been probed already.

You have a mutex here and it is used for add/remove.  Why isn't it
used for list browsing?

>
>>
>> > +   if (!pwrseq)
>> > +   return ERR_PTR(-ENONET);
>>
>> ENOENT I suppose?
>>
>
> Good catch, thanks.
>
>> > +/**
>> > + * of_pwrseq_off: do power sequence off for this pwrseq instance
>> > + *
>> > + * This API is used to power off single device, it is the opposite
>> > + * operation for of_pwrseq_on.
>> > + *
>> > + * @pwrseq: the pwrseq instance which related device would like to be off
>> > + */
>> > +void of_pwrseq_off(struct pwrseq *pwrseq)
>> > +{
>> > +   pwrseq_off(pwrseq);
>> > +   pwrseq_put(pwrseq);
>> > +}
>> > +EXPORT_SYMBOL_GPL(of_pwrseq_off);
>>
>> What happens if two code paths attempt to turn the same power sequence
>> off in parallel?  Can it ever happen?  If not, then why not?
>>
>
> I don't think the same pwrseq instance off will

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

2016-11-21 Thread Peter Chen 
On Tue, Nov 22, 2016 at 03:23:12AM +0100, Rafael J. Wysocki wrote:
> > @@ -0,0 +1,237 @@
> > +/*
> > + * 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 the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program.  If not, see .
> 
> The last paragraph is not necessary AFAICS.

I just copy it from:

https://www.gnu.org/licenses/gpl-howto.en.html

If you are concerns about it, I can delete it.

> > +
> > +static struct pwrseq *pwrseq_find_available_instance(struct device_node 
> > *np)
> > +{
> > +   struct pwrseq *pwrseq;
> > +
> > +   list_for_each_entry(pwrseq, _list, node) {
> > +   if (pwrseq->used)
> > +   continue;
> > +
> > +   /* compare compatible string for pwrseq node */
> > +   if (of_match_node(pwrseq->pwrseq_of_match_table, np)) {
> > +   pwrseq->used = true;
> > +   return pwrseq;
> > +   }
> > +
> > +   /* return generic pwrseq instance */
> > +   if (!strcmp(pwrseq->pwrseq_of_match_table->compatible,
> > +   "generic")) {
> > +   pr_debug("using generic pwrseq instance for %s\n",
> > +   np->full_name);
> > +   pwrseq->used = true;
> > +   return pwrseq;
> > +   }
> > +   }
> > +   pr_warn("Can't find any pwrseq instances for %s\n", np->full_name);
> 
> pr_debug() ?

If there is no pwrseq instance for that node, the power sequence on routine will
return fail, so I think an warning message is useful for user.

> 
> > +
> > +   return NULL;
> > +}
> > +
> > +/**
> > + * of_pwrseq_on: do power sequence on for device node
> 
> of_pwrseq_on - Carry out power sequence on for device node
> 
> Argument description should follow this line.
> 
> > + *
> > + * This API is used to power on single device, if the host
> > + * controller only needs to handle one child device (this device
> > + * node points to), use this API. If multiply devices are needed
> > + * to handle on bus, use of_pwrseq_on_list.
> 
> That's unclear.
> 
> What about "Carry out a single device power on.  If multiple devices
> need to be handled, use of_pwrseq_on_list() instead."
> 
> > + *
> > + * @np: the device node would like to power on
> > + *
> > + * On successful, it returns pwrseq instance, otherwise an error value.
> 
> "Return a pointer to the power sequence instance on success, or an
> error code otherwise."
> 

Ok, will change.

> > + */
> > +struct pwrseq *of_pwrseq_on(struct device_node *np)
> > +{
> > +   struct pwrseq *pwrseq;
> > +   int ret;
> > +
> > +   pwrseq = pwrseq_find_available_instance(np);
> 
> What does guarantee the integrity of ths list at this point?

Once the use selects the specific pwrseq library, the library will
create an empty one instance during the initialization, and it
will be called at postcore_initcall, the device driver has not
probed yet.

> 
> > +   if (!pwrseq)
> > +   return ERR_PTR(-ENONET);
> 
> ENOENT I suppose?
> 

Good catch, thanks.

> > +/**
> > + * of_pwrseq_off: do power sequence off for this pwrseq instance
> > + *
> > + * This API is used to power off single device, it is the opposite
> > + * operation for of_pwrseq_on.
> > + *
> > + * @pwrseq: the pwrseq instance which related device would like to be off
> > + */
> > +void of_pwrseq_off(struct pwrseq *pwrseq)
> > +{
> > +   pwrseq_off(pwrseq);
> > +   pwrseq_put(pwrseq);
> > +}
> > +EXPORT_SYMBOL_GPL(of_pwrseq_off);
> 
> What happens if two code paths attempt to turn the same power sequence
> off in parallel?  Can it ever happen?  If not, then why not?
> 

I don't think the same pwrseq instance off will be called at the same
time, the of_pwrseq_off is supposed to be only called at error path
during power-on and at device power-off routine, and only the power-on is
successful, the device can be created, if the device is not created,
its power-off routine is not supposed to be called.

> > +
> > +/**
> > + * of_pwrseq_on_list: do power sequence on for list
> > + *
> > + * This API is used to power on multiple devices at single bus.
> > + * If there are several devices on bus (eg, USB bus), uses this
> > + * this API. Otherwise, use

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

2016-11-21 Thread Rafael J. Wysocki 
On Mon, Nov 14, 2016 at 2:35 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 
> ---
>  MAINTAINERS   |   9 ++
>  drivers/power/Kconfig |   1 +
>  drivers/power/Makefile|   1 +
>  drivers/power/pwrseq/Kconfig  |  21 +++
>  drivers/power/pwrseq/Makefile |   2 +
>  drivers/power/pwrseq/core.c   | 237 
> ++
>  drivers/power/pwrseq/pwrseq_generic.c | 183 ++
>  include/linux/power/pwrseq.h  |  60 +
>  8 files changed, 514 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 3d838cf..066b1e4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9621,6 +9621,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..88f5597
> --- /dev/null
> +++ b/drivers/power/pwrseq/Kconfig
> @@ -0,0 +1,21 @@
> +#
> +# Power Sequence library
> +#
> +
> +menuconfig POWER_SEQUENCE
> +   bool "Power sequence control"
> +   depends on OF
> +   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"
> +   default y
> +   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..e3c1fbb
> --- /dev/null
> +++ b/drivers/power/pwrseq/core.c
> @@ -0,0 +1,237 @@
> +/*
> + * core.c  power sequence core file
>

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

2016-11-21 Thread Peter Chen 
On Mon, Nov 14, 2016 at 09:35:53AM +0800, 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.
> 

Rafael, would you get any chances to review this version, it makes
some changes according to your comments, I hope this patch set could
be in v4.10-rc1, thanks.

Peter
> 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  |  21 +++
>  drivers/power/pwrseq/Makefile |   2 +
>  drivers/power/pwrseq/core.c   | 237 
> ++
>  drivers/power/pwrseq/pwrseq_generic.c | 183 ++
>  include/linux/power/pwrseq.h  |  60 +
>  8 files changed, 514 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 3d838cf..066b1e4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9621,6 +9621,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..88f5597
> --- /dev/null
> +++ b/drivers/power/pwrseq/Kconfig
> @@ -0,0 +1,21 @@
> +#
> +# Power Sequence library
> +#
> +
> +menuconfig POWER_SEQUENCE
> + bool "Power sequence control"
> + depends on OF
> + 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"
> + default y
> + 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..e3c1fbb
> ---

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

2016-11-13 Thread 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  |  21 +++
 drivers/power/pwrseq/Makefile |   2 +
 drivers/power/pwrseq/core.c   | 237 ++
 drivers/power/pwrseq/pwrseq_generic.c | 183 ++
 include/linux/power/pwrseq.h  |  60 +
 8 files changed, 514 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 3d838cf..066b1e4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9621,6 +9621,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..88f5597
--- /dev/null
+++ b/drivers/power/pwrseq/Kconfig
@@ -0,0 +1,21 @@
+#
+# Power Sequence library
+#
+
+menuconfig POWER_SEQUENCE
+   bool "Power sequence control"
+   depends on OF
+   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"
+   default y
+   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..e3c1fbb
--- /dev/null
+++ b/drivers/power/pwrseq/core.c
@@ -0,0 +1,237 @@
+/*
+ * 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.
+ *
+ *