Re: [PATCH V4 3/4] mmc: pwrseq: Initial support for the simple MMC power sequence provider
On Wed, Feb 11, 2015 at 1:28 PM, Ulf Hansson wrote: > [...] > >>> +int mmc_pwrseq_simple_alloc(struct mmc_host *host, struct device *dev) >>> +{ >>> + struct mmc_pwrseq_simple *pwrseq; >>> + >>> + pwrseq = kzalloc(sizeof(struct mmc_pwrseq_simple), GFP_KERNEL); >>> + if (!pwrseq) >>> + return -ENOMEM; >>> + >>> + pwrseq->pwrseq.ops = &mmc_pwrseq_simple_ops; >>> + host->pwrseq = &pwrseq->pwrseq; >> >> How about making this function return a struct mmc_pwrseq * so this >> last line can be moved to mmc_pwrseq_alloc() instead of requiring all >> power sequences to do it? >> >> The same applies to >> >> host->pwrseq = NULL; >> >> in mmc_pwrseq_simple_free(), which could be done in mmc_pwrseq_free() it >> seems. > > Thanks for reviewing! > > I like you suggestion, though $subject patch is already part of the PR for > 3.20. > > Feel free to send a patch, I would happily apply it. Damn, why am I always so late to review patches... I will send a fixup patch - better to have this done early. Thanks! -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V4 3/4] mmc: pwrseq: Initial support for the simple MMC power sequence provider
[...] >> +int mmc_pwrseq_simple_alloc(struct mmc_host *host, struct device *dev) >> +{ >> + struct mmc_pwrseq_simple *pwrseq; >> + >> + pwrseq = kzalloc(sizeof(struct mmc_pwrseq_simple), GFP_KERNEL); >> + if (!pwrseq) >> + return -ENOMEM; >> + >> + pwrseq->pwrseq.ops = &mmc_pwrseq_simple_ops; >> + host->pwrseq = &pwrseq->pwrseq; > > How about making this function return a struct mmc_pwrseq * so this > last line can be moved to mmc_pwrseq_alloc() instead of requiring all > power sequences to do it? > > The same applies to > > host->pwrseq = NULL; > > in mmc_pwrseq_simple_free(), which could be done in mmc_pwrseq_free() it > seems. Thanks for reviewing! I like you suggestion, though $subject patch is already part of the PR for 3.20. Feel free to send a patch, I would happily apply it. Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V4 3/4] mmc: pwrseq: Initial support for the simple MMC power sequence provider
On Mon, Jan 19, 2015 at 6:13 PM, Ulf Hansson wrote: > To add the core part for the MMC power sequence, let's start by adding > initial support for the simple MMC power sequence provider. > > In this initial step, the MMC power sequence node are fetched and the > compatible string for the simple MMC power sequence provider are > verified. > > At this point we don't parse the node for any properties, but instead > that will be handled from following patches. Since there are no > properties supported yet, let's just implement the ->alloc() and the > ->free() callbacks. > > Signed-off-by: Ulf Hansson > --- > > Changes in v4: > - Fixed call to kfree(). > > --- > drivers/mmc/core/Makefile| 2 +- > drivers/mmc/core/pwrseq.c| 61 > +++- > drivers/mmc/core/pwrseq.h| 2 ++ > drivers/mmc/core/pwrseq_simple.c | 48 +++ > 4 files changed, 111 insertions(+), 2 deletions(-) > create mode 100644 drivers/mmc/core/pwrseq_simple.c > > diff --git a/drivers/mmc/core/Makefile b/drivers/mmc/core/Makefile > index ccdd35f..b39cbd2 100644 > --- a/drivers/mmc/core/Makefile > +++ b/drivers/mmc/core/Makefile > @@ -8,5 +8,5 @@ mmc_core-y := core.o bus.o host.o \ >sdio.o sdio_ops.o sdio_bus.o \ >sdio_cis.o sdio_io.o sdio_irq.o \ >quirks.o slot-gpio.o > -mmc_core-$(CONFIG_OF) += pwrseq.o > +mmc_core-$(CONFIG_OF) += pwrseq.o pwrseq_simple.o > mmc_core-$(CONFIG_DEBUG_FS)+= debugfs.o > diff --git a/drivers/mmc/core/pwrseq.c b/drivers/mmc/core/pwrseq.c > index bd08772..2cea00e 100644 > --- a/drivers/mmc/core/pwrseq.c > +++ b/drivers/mmc/core/pwrseq.c > @@ -7,14 +7,73 @@ > * > * MMC power sequence management > */ > +#include > +#include > +#include > +#include > +#include > + > #include > > #include "pwrseq.h" > > +struct mmc_pwrseq_match { > + const char *compatible; > + int (*alloc)(struct mmc_host *host, struct device *dev); > +}; > + > +static struct mmc_pwrseq_match pwrseq_match[] = { > + { > + .compatible = "mmc-pwrseq-simple", > + .alloc = mmc_pwrseq_simple_alloc, > + }, > +}; > + > +static struct mmc_pwrseq_match *mmc_pwrseq_find(struct device_node *np) > +{ > + struct mmc_pwrseq_match *match = ERR_PTR(-ENODEV); > + int i; > + > + for (i = 0; i < ARRAY_SIZE(pwrseq_match); i++) { > + if (of_device_is_compatible(np, pwrseq_match[i].compatible)) { > + match = &pwrseq_match[i]; > + break; > + } > + } > + > + return match; > +} > > int mmc_pwrseq_alloc(struct mmc_host *host) > { > - return 0; > + struct platform_device *pdev; > + struct device_node *np; > + struct mmc_pwrseq_match *match; > + int ret = 0; > + > + np = of_parse_phandle(host->parent->of_node, "mmc-pwrseq", 0); > + if (!np) > + return 0; > + > + pdev = of_find_device_by_node(np); > + if (!pdev) { > + ret = -ENODEV; > + goto err; > + } > + > + match = mmc_pwrseq_find(np); > + if (IS_ERR(match)) { > + ret = PTR_ERR(match); > + goto err; > + } > + > + ret = match->alloc(host, &pdev->dev); > + if (!ret) > + dev_info(host->parent, "allocated mmc-pwrseq\n"); > + > +err: > + of_node_put(np); > + return ret; > } > > void mmc_pwrseq_pre_power_on(struct mmc_host *host) > diff --git a/drivers/mmc/core/pwrseq.h b/drivers/mmc/core/pwrseq.h > index 12aaf2b..bd860d8 100644 > --- a/drivers/mmc/core/pwrseq.h > +++ b/drivers/mmc/core/pwrseq.h > @@ -27,6 +27,8 @@ void mmc_pwrseq_post_power_on(struct mmc_host *host); > void mmc_pwrseq_power_off(struct mmc_host *host); > void mmc_pwrseq_free(struct mmc_host *host); > > +int mmc_pwrseq_simple_alloc(struct mmc_host *host, struct device *dev); > + > #else > > static inline int mmc_pwrseq_alloc(struct mmc_host *host) { return 0; } > diff --git a/drivers/mmc/core/pwrseq_simple.c > b/drivers/mmc/core/pwrseq_simple.c > new file mode 100644 > index 000..61c991e > --- /dev/null > +++ b/drivers/mmc/core/pwrseq_simple.c > @@ -0,0 +1,48 @@ > +/* > + * Copyright (C) 2014 Linaro Ltd > + * > + * Author: Ulf Hansson > + * > + * License terms: GNU General Public License (GPL) version 2 > + * > + * Simple MMC power sequence management > + */ > +#include > +#include > +#include > +#include > + > +#include > + > +#include "pwrseq.h" > + > +struct mmc_pwrseq_simple { > + struct mmc_pwrseq pwrseq; > +}; > + > +static void mmc_pwrseq_simple_free(struct mmc_host *host) > +{ > + struct mmc_pwrseq_simple *pwrseq = container_of(host->pwrseq, > + struct mmc_pwrseq_simple, pwrseq); > + > + kfree(pwrse
Re: [PATCH V4 3/4] mmc: pwrseq: Initial support for the simple MMC power sequence provider
Hello Ulf, On Mon, Jan 19, 2015 at 10:13 AM, Ulf Hansson wrote: > To add the core part for the MMC power sequence, let's start by adding > initial support for the simple MMC power sequence provider. > > In this initial step, the MMC power sequence node are fetched and the > compatible string for the simple MMC power sequence provider are > verified. > > At this point we don't parse the node for any properties, but instead > that will be handled from following patches. Since there are no > properties supported yet, let's just implement the ->alloc() and the > ->free() callbacks. > > Signed-off-by: Ulf Hansson > --- > Reviewed-by: Javier Martinez Canillas Tested-by: Javier Martinez Canillas Best regards, Javier -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V4 3/4] mmc: pwrseq: Initial support for the simple MMC power sequence provider
To add the core part for the MMC power sequence, let's start by adding initial support for the simple MMC power sequence provider. In this initial step, the MMC power sequence node are fetched and the compatible string for the simple MMC power sequence provider are verified. At this point we don't parse the node for any properties, but instead that will be handled from following patches. Since there are no properties supported yet, let's just implement the ->alloc() and the ->free() callbacks. Signed-off-by: Ulf Hansson --- Changes in v4: - Fixed call to kfree(). --- drivers/mmc/core/Makefile| 2 +- drivers/mmc/core/pwrseq.c| 61 +++- drivers/mmc/core/pwrseq.h| 2 ++ drivers/mmc/core/pwrseq_simple.c | 48 +++ 4 files changed, 111 insertions(+), 2 deletions(-) create mode 100644 drivers/mmc/core/pwrseq_simple.c diff --git a/drivers/mmc/core/Makefile b/drivers/mmc/core/Makefile index ccdd35f..b39cbd2 100644 --- a/drivers/mmc/core/Makefile +++ b/drivers/mmc/core/Makefile @@ -8,5 +8,5 @@ mmc_core-y := core.o bus.o host.o \ sdio.o sdio_ops.o sdio_bus.o \ sdio_cis.o sdio_io.o sdio_irq.o \ quirks.o slot-gpio.o -mmc_core-$(CONFIG_OF) += pwrseq.o +mmc_core-$(CONFIG_OF) += pwrseq.o pwrseq_simple.o mmc_core-$(CONFIG_DEBUG_FS)+= debugfs.o diff --git a/drivers/mmc/core/pwrseq.c b/drivers/mmc/core/pwrseq.c index bd08772..2cea00e 100644 --- a/drivers/mmc/core/pwrseq.c +++ b/drivers/mmc/core/pwrseq.c @@ -7,14 +7,73 @@ * * MMC power sequence management */ +#include +#include +#include +#include +#include + #include #include "pwrseq.h" +struct mmc_pwrseq_match { + const char *compatible; + int (*alloc)(struct mmc_host *host, struct device *dev); +}; + +static struct mmc_pwrseq_match pwrseq_match[] = { + { + .compatible = "mmc-pwrseq-simple", + .alloc = mmc_pwrseq_simple_alloc, + }, +}; + +static struct mmc_pwrseq_match *mmc_pwrseq_find(struct device_node *np) +{ + struct mmc_pwrseq_match *match = ERR_PTR(-ENODEV); + int i; + + for (i = 0; i < ARRAY_SIZE(pwrseq_match); i++) { + if (of_device_is_compatible(np, pwrseq_match[i].compatible)) { + match = &pwrseq_match[i]; + break; + } + } + + return match; +} int mmc_pwrseq_alloc(struct mmc_host *host) { - return 0; + struct platform_device *pdev; + struct device_node *np; + struct mmc_pwrseq_match *match; + int ret = 0; + + np = of_parse_phandle(host->parent->of_node, "mmc-pwrseq", 0); + if (!np) + return 0; + + pdev = of_find_device_by_node(np); + if (!pdev) { + ret = -ENODEV; + goto err; + } + + match = mmc_pwrseq_find(np); + if (IS_ERR(match)) { + ret = PTR_ERR(match); + goto err; + } + + ret = match->alloc(host, &pdev->dev); + if (!ret) + dev_info(host->parent, "allocated mmc-pwrseq\n"); + +err: + of_node_put(np); + return ret; } void mmc_pwrseq_pre_power_on(struct mmc_host *host) diff --git a/drivers/mmc/core/pwrseq.h b/drivers/mmc/core/pwrseq.h index 12aaf2b..bd860d8 100644 --- a/drivers/mmc/core/pwrseq.h +++ b/drivers/mmc/core/pwrseq.h @@ -27,6 +27,8 @@ void mmc_pwrseq_post_power_on(struct mmc_host *host); void mmc_pwrseq_power_off(struct mmc_host *host); void mmc_pwrseq_free(struct mmc_host *host); +int mmc_pwrseq_simple_alloc(struct mmc_host *host, struct device *dev); + #else static inline int mmc_pwrseq_alloc(struct mmc_host *host) { return 0; } diff --git a/drivers/mmc/core/pwrseq_simple.c b/drivers/mmc/core/pwrseq_simple.c new file mode 100644 index 000..61c991e --- /dev/null +++ b/drivers/mmc/core/pwrseq_simple.c @@ -0,0 +1,48 @@ +/* + * Copyright (C) 2014 Linaro Ltd + * + * Author: Ulf Hansson + * + * License terms: GNU General Public License (GPL) version 2 + * + * Simple MMC power sequence management + */ +#include +#include +#include +#include + +#include + +#include "pwrseq.h" + +struct mmc_pwrseq_simple { + struct mmc_pwrseq pwrseq; +}; + +static void mmc_pwrseq_simple_free(struct mmc_host *host) +{ + struct mmc_pwrseq_simple *pwrseq = container_of(host->pwrseq, + struct mmc_pwrseq_simple, pwrseq); + + kfree(pwrseq); + host->pwrseq = NULL; +} + +static struct mmc_pwrseq_ops mmc_pwrseq_simple_ops = { + .free = mmc_pwrseq_simple_free, +}; + +int mmc_pwrseq_simple_alloc(struct mmc_host *host, struct device *dev) +{ + struct mmc_pwrseq_simple *pwrseq; + + pwrseq = kzalloc(sizeof(struct mmc_pwrseq_simple), GFP_KERNEL); + if (!pwrseq) +