* [PATCH v2 1/6] mmc: pwrseq: Document that simple sequence support more than one GPIO
2015-01-28 18:15 [PATCH v2 0/6] Add multiple GPIO and external clock to MMC pwrseq_simple Javier Martinez Canillas
@ 2015-01-28 18:15 ` Javier Martinez Canillas
2015-01-28 18:15 ` [PATCH v2 2/6] mmc: pwrseq_simple: Extend to support more pins Javier Martinez Canillas
` (4 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Javier Martinez Canillas @ 2015-01-28 18:15 UTC (permalink / raw)
To: Ulf Hansson
Cc: Kukjin Kim, Doug Anderson, Olof Johansson, Arend van Spriel,
Srinivas Kandagatla, linux-samsung-soc, linux-arm-kernel,
linux-mmc, devicetree, linux-kernel, Javier Martinez Canillas
Many SDIO/MMC attached WLAN chips need more than one ping for their reset
sequence. Extend the pwrseq_simple binding to support more than one pin.
Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
Changes since v1:
- Make the explanation clearer by adding an explicit "they".
Suggested by Srinivas Kandagatla.
---
Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
index da333d9ed94c..eaae652213ae 100644
--- a/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
+++ b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
@@ -8,9 +8,10 @@ Required properties:
- compatible : contains "mmc-pwrseq-simple".
Optional properties:
-- reset-gpios : contains a GPIO specifier. The reset GPIO is asserted at
- initialization and prior we start the power up procedure of the card. It
- will be de-asserted right after the power has been provided to the card.
+- reset-gpios : contains a list of GPIO specifiers. The reset GPIOs are asserted
+ at initialization and prior we start the power up procedure of the card.
+ They will be de-asserted right after the power has been provided to the
+ card.
Example:
--
2.1.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v2 2/6] mmc: pwrseq_simple: Extend to support more pins
2015-01-28 18:15 [PATCH v2 0/6] Add multiple GPIO and external clock to MMC pwrseq_simple Javier Martinez Canillas
2015-01-28 18:15 ` [PATCH v2 1/6] mmc: pwrseq: Document that simple sequence support more than one GPIO Javier Martinez Canillas
@ 2015-01-28 18:15 ` Javier Martinez Canillas
2015-01-28 18:15 ` [PATCH v2 3/6] mmc: pwrseq: Document optional clock for the simple power sequence Javier Martinez Canillas
` (3 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Javier Martinez Canillas @ 2015-01-28 18:15 UTC (permalink / raw)
To: Ulf Hansson
Cc: Kukjin Kim, Doug Anderson, Olof Johansson, Arend van Spriel,
Srinivas Kandagatla, linux-samsung-soc, linux-arm-kernel,
linux-mmc, devicetree, linux-kernel, Javier Martinez Canillas
Many WLAN attached to a SDIO/MMC interface, needs more than one pin for
their reset sequence. For example, is very common for chips to have two
pins: one for reset and one for power enable.
This patch adds support for more reset pins to the pwrseq_simple driver
and instead hardcoding a fixed number, it uses the of_gpio_named_count()
since the MMC power sequence is only built when CONFIG_OF is enabled.
Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
Changes since v1:
- Many code cleanups suggested by Srinivas Kandagatla
* Rename reset_gpio array to reset_gpios.
* Use a zero length array for reset_gpios to avoid a kmalloc.
* Refactor GPIO assert and de-ssert logic into a common function.
* Simplify gpiod_put logic in case of gpiod_get error.
---
drivers/mmc/core/pwrseq_simple.c | 55 +++++++++++++++++++++++++++++-----------
1 file changed, 40 insertions(+), 15 deletions(-)
diff --git a/drivers/mmc/core/pwrseq_simple.c b/drivers/mmc/core/pwrseq_simple.c
index 0958c696137f..e53d3c7e059c 100644
--- a/drivers/mmc/core/pwrseq_simple.c
+++ b/drivers/mmc/core/pwrseq_simple.c
@@ -11,6 +11,7 @@
#include <linux/slab.h>
#include <linux/device.h>
#include <linux/err.h>
+#include <linux/of_gpio.h>
#include <linux/gpio/consumer.h>
#include <linux/mmc/host.h>
@@ -19,16 +20,26 @@
struct mmc_pwrseq_simple {
struct mmc_pwrseq pwrseq;
- struct gpio_desc *reset_gpio;
+ int nr_gpios;
+ struct gpio_desc *reset_gpios[0];
};
+static void mmc_pwrseq_simple_set_gpios_value(struct mmc_pwrseq_simple *pwrseq,
+ int value)
+{
+ int i;
+
+ for (i = 0; i < pwrseq->nr_gpios; i++)
+ if (!IS_ERR(pwrseq->reset_gpios[i]))
+ gpiod_set_value_cansleep(pwrseq->reset_gpios[i], value);
+}
+
static void mmc_pwrseq_simple_pre_power_on(struct mmc_host *host)
{
struct mmc_pwrseq_simple *pwrseq = container_of(host->pwrseq,
struct mmc_pwrseq_simple, pwrseq);
- if (!IS_ERR(pwrseq->reset_gpio))
- gpiod_set_value_cansleep(pwrseq->reset_gpio, 1);
+ mmc_pwrseq_simple_set_gpios_value(pwrseq, 1);
}
static void mmc_pwrseq_simple_post_power_on(struct mmc_host *host)
@@ -36,17 +47,18 @@ static void mmc_pwrseq_simple_post_power_on(struct mmc_host *host)
struct mmc_pwrseq_simple *pwrseq = container_of(host->pwrseq,
struct mmc_pwrseq_simple, pwrseq);
- if (!IS_ERR(pwrseq->reset_gpio))
- gpiod_set_value_cansleep(pwrseq->reset_gpio, 0);
+ mmc_pwrseq_simple_set_gpios_value(pwrseq, 0);
}
static void mmc_pwrseq_simple_free(struct mmc_host *host)
{
struct mmc_pwrseq_simple *pwrseq = container_of(host->pwrseq,
struct mmc_pwrseq_simple, pwrseq);
+ int i;
- if (!IS_ERR(pwrseq->reset_gpio))
- gpiod_put(pwrseq->reset_gpio);
+ for (i = 0; i < pwrseq->nr_gpios; i++)
+ if (!IS_ERR(pwrseq->reset_gpios[i]))
+ gpiod_put(pwrseq->reset_gpios[i]);
kfree(pwrseq);
host->pwrseq = NULL;
@@ -62,20 +74,33 @@ static struct mmc_pwrseq_ops mmc_pwrseq_simple_ops = {
int mmc_pwrseq_simple_alloc(struct mmc_host *host, struct device *dev)
{
struct mmc_pwrseq_simple *pwrseq;
- int ret = 0;
+ int i, nr_gpios, ret = 0;
- pwrseq = kzalloc(sizeof(struct mmc_pwrseq_simple), GFP_KERNEL);
+ nr_gpios = of_gpio_named_count(dev->of_node, "reset-gpios");
+ if (nr_gpios < 0)
+ nr_gpios = 0;
+
+ pwrseq = kzalloc(sizeof(struct mmc_pwrseq_simple) + nr_gpios *
+ sizeof(struct gpio_desc *), GFP_KERNEL);
if (!pwrseq)
return -ENOMEM;
- pwrseq->reset_gpio = gpiod_get_index(dev, "reset", 0, GPIOD_OUT_HIGH);
- if (IS_ERR(pwrseq->reset_gpio) &&
- PTR_ERR(pwrseq->reset_gpio) != -ENOENT &&
- PTR_ERR(pwrseq->reset_gpio) != -ENOSYS) {
- ret = PTR_ERR(pwrseq->reset_gpio);
- goto free;
+ for (i = 0; i < nr_gpios; i++) {
+ pwrseq->reset_gpios[i] = gpiod_get_index(dev, "reset", i,
+ GPIOD_OUT_HIGH);
+ if (IS_ERR(pwrseq->reset_gpios[i]) &&
+ PTR_ERR(pwrseq->reset_gpios[i]) != -ENOENT &&
+ PTR_ERR(pwrseq->reset_gpios[i]) != -ENOSYS) {
+ ret = PTR_ERR(pwrseq->reset_gpios[i]);
+
+ while (--i)
+ gpiod_put(pwrseq->reset_gpios[i]);
+
+ goto free;
+ }
}
+ pwrseq->nr_gpios = nr_gpios;
pwrseq->pwrseq.ops = &mmc_pwrseq_simple_ops;
host->pwrseq = &pwrseq->pwrseq;
--
2.1.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v2 3/6] mmc: pwrseq: Document optional clock for the simple power sequence
2015-01-28 18:15 [PATCH v2 0/6] Add multiple GPIO and external clock to MMC pwrseq_simple Javier Martinez Canillas
2015-01-28 18:15 ` [PATCH v2 1/6] mmc: pwrseq: Document that simple sequence support more than one GPIO Javier Martinez Canillas
2015-01-28 18:15 ` [PATCH v2 2/6] mmc: pwrseq_simple: Extend to support more pins Javier Martinez Canillas
@ 2015-01-28 18:15 ` Javier Martinez Canillas
2015-01-28 18:15 ` [PATCH v2 4/6] mmc: pwrseq_simple: Add optional reference clock support Javier Martinez Canillas
` (2 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Javier Martinez Canillas @ 2015-01-28 18:15 UTC (permalink / raw)
To: Ulf Hansson
Cc: Kukjin Kim, Doug Anderson, Olof Johansson, Arend van Spriel,
Srinivas Kandagatla, linux-samsung-soc, linux-arm-kernel,
linux-mmc, devicetree, linux-kernel, Javier Martinez Canillas
Some WLAN chips attached to a SDIO interface, need an external clock
to be operational. Since this is very common, extend the simple MMC
power sequence DT binding to support an optional clock.
Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
Changes since v1: None.
---
Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
index eaae652213ae..a462c50f19a8 100644
--- a/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
+++ b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
@@ -12,6 +12,10 @@ Optional properties:
at initialization and prior we start the power up procedure of the card.
They will be de-asserted right after the power has been provided to the
card.
+- clocks : Must contain an entry for the entry in clock-names.
+ See ../clocks/clock-bindings.txt for details.
+- clock-names : Must include the following entry:
+ "ext_clock" (External clock provided to the card).
Example:
--
2.1.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v2 4/6] mmc: pwrseq_simple: Add optional reference clock support
2015-01-28 18:15 [PATCH v2 0/6] Add multiple GPIO and external clock to MMC pwrseq_simple Javier Martinez Canillas
` (2 preceding siblings ...)
2015-01-28 18:15 ` [PATCH v2 3/6] mmc: pwrseq: Document optional clock for the simple power sequence Javier Martinez Canillas
@ 2015-01-28 18:15 ` Javier Martinez Canillas
2015-01-29 13:05 ` Ulf Hansson
2015-01-28 18:15 ` [PATCH v2 5/6] ARM: dts: exynos5250-snow: Enable wifi power-on Javier Martinez Canillas
2015-01-28 18:15 ` [PATCH v2 6/6] ARM: dts: exynos5250-snow: Add cap-sdio-irq to wifi mmc node Javier Martinez Canillas
5 siblings, 1 reply; 9+ messages in thread
From: Javier Martinez Canillas @ 2015-01-28 18:15 UTC (permalink / raw)
To: Ulf Hansson
Cc: Kukjin Kim, Doug Anderson, Olof Johansson, Arend van Spriel,
Srinivas Kandagatla, linux-samsung-soc, linux-arm-kernel,
linux-mmc, devicetree, linux-kernel, Javier Martinez Canillas
Some WLAN chips attached to a SDIO interface, need a reference clock.
Since this is very common, extend the prseq_simple driver to support
an optional clock that is enabled prior the card power up procedure.
Note: the external clock is optional. Thus an error is not returned
if the clock is not found.
Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
Changes since v1:
- Rebase on top of latest changes.
- Use IS_ERR() instead of checking for NULL to see if the clock exists.
---
drivers/mmc/core/pwrseq_simple.c | 34 ++++++++++++++++++++++++++++++++--
1 file changed, 32 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/core/pwrseq_simple.c b/drivers/mmc/core/pwrseq_simple.c
index e53d3c7e059c..5e34c77efa5e 100644
--- a/drivers/mmc/core/pwrseq_simple.c
+++ b/drivers/mmc/core/pwrseq_simple.c
@@ -7,6 +7,7 @@
*
* Simple MMC power sequence management
*/
+#include <linux/clk.h>
#include <linux/kernel.h>
#include <linux/slab.h>
#include <linux/device.h>
@@ -20,6 +21,7 @@
struct mmc_pwrseq_simple {
struct mmc_pwrseq pwrseq;
+ struct clk *ext_clk;
int nr_gpios;
struct gpio_desc *reset_gpios[0];
};
@@ -39,6 +41,9 @@ static void mmc_pwrseq_simple_pre_power_on(struct mmc_host *host)
struct mmc_pwrseq_simple *pwrseq = container_of(host->pwrseq,
struct mmc_pwrseq_simple, pwrseq);
+ if (!IS_ERR(pwrseq->ext_clk))
+ clk_prepare_enable(pwrseq->ext_clk);
+
mmc_pwrseq_simple_set_gpios_value(pwrseq, 1);
}
@@ -50,6 +55,17 @@ static void mmc_pwrseq_simple_post_power_on(struct mmc_host *host)
mmc_pwrseq_simple_set_gpios_value(pwrseq, 0);
}
+static void mmc_pwrseq_simple_power_off(struct mmc_host *host)
+{
+ struct mmc_pwrseq_simple *pwrseq = container_of(host->pwrseq,
+ struct mmc_pwrseq_simple, pwrseq);
+
+ mmc_pwrseq_simple_set_gpios_value(pwrseq, 1);
+
+ if (!IS_ERR(pwrseq->ext_clk))
+ clk_disable_unprepare(pwrseq->ext_clk);
+}
+
static void mmc_pwrseq_simple_free(struct mmc_host *host)
{
struct mmc_pwrseq_simple *pwrseq = container_of(host->pwrseq,
@@ -60,6 +76,9 @@ static void mmc_pwrseq_simple_free(struct mmc_host *host)
if (!IS_ERR(pwrseq->reset_gpios[i]))
gpiod_put(pwrseq->reset_gpios[i]);
+ if (!IS_ERR(pwrseq->ext_clk))
+ clk_put(pwrseq->ext_clk);
+
kfree(pwrseq);
host->pwrseq = NULL;
}
@@ -67,7 +86,7 @@ static void mmc_pwrseq_simple_free(struct mmc_host *host)
static struct mmc_pwrseq_ops mmc_pwrseq_simple_ops = {
.pre_power_on = mmc_pwrseq_simple_pre_power_on,
.post_power_on = mmc_pwrseq_simple_post_power_on,
- .power_off = mmc_pwrseq_simple_pre_power_on,
+ .power_off = mmc_pwrseq_simple_power_off,
.free = mmc_pwrseq_simple_free,
};
@@ -85,6 +104,14 @@ int mmc_pwrseq_simple_alloc(struct mmc_host *host, struct device *dev)
if (!pwrseq)
return -ENOMEM;
+ pwrseq->ext_clk = clk_get(dev, "ext_clock");
+ if (IS_ERR(pwrseq->ext_clk) &&
+ PTR_ERR(pwrseq->ext_clk) != -ENOENT &&
+ PTR_ERR(pwrseq->ext_clk) != -ENOSYS) {
+ ret = PTR_ERR(pwrseq->ext_clk);
+ goto free;
+ }
+
for (i = 0; i < nr_gpios; i++) {
pwrseq->reset_gpios[i] = gpiod_get_index(dev, "reset", i,
GPIOD_OUT_HIGH);
@@ -96,7 +123,7 @@ int mmc_pwrseq_simple_alloc(struct mmc_host *host, struct device *dev)
while (--i)
gpiod_put(pwrseq->reset_gpios[i]);
- goto free;
+ goto clk_put;
}
}
@@ -105,6 +132,9 @@ int mmc_pwrseq_simple_alloc(struct mmc_host *host, struct device *dev)
host->pwrseq = &pwrseq->pwrseq;
return 0;
+clk_put:
+ if (!IS_ERR(pwrseq->ext_clk))
+ clk_put(pwrseq->ext_clk);
free:
kfree(pwrseq);
return ret;
--
2.1.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v2 4/6] mmc: pwrseq_simple: Add optional reference clock support
2015-01-28 18:15 ` [PATCH v2 4/6] mmc: pwrseq_simple: Add optional reference clock support Javier Martinez Canillas
@ 2015-01-29 13:05 ` Ulf Hansson
2015-01-29 13:53 ` Javier Martinez Canillas
0 siblings, 1 reply; 9+ messages in thread
From: Ulf Hansson @ 2015-01-29 13:05 UTC (permalink / raw)
To: Javier Martinez Canillas
Cc: Kukjin Kim, Doug Anderson, Olof Johansson, Arend van Spriel,
Srinivas Kandagatla, linux-samsung-soc,
linux-arm-kernel@lists.infradead.org, linux-mmc,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
On 28 January 2015 at 19:15, Javier Martinez Canillas
<javier.martinez@collabora.co.uk> wrote:
> Some WLAN chips attached to a SDIO interface, need a reference clock.
>
> Since this is very common, extend the prseq_simple driver to support
> an optional clock that is enabled prior the card power up procedure.
>
> Note: the external clock is optional. Thus an error is not returned
> if the clock is not found.
>
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> ---
>
> Changes since v1:
> - Rebase on top of latest changes.
> - Use IS_ERR() instead of checking for NULL to see if the clock exists.
> ---
> drivers/mmc/core/pwrseq_simple.c | 34 ++++++++++++++++++++++++++++++++--
> 1 file changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/core/pwrseq_simple.c b/drivers/mmc/core/pwrseq_simple.c
> index e53d3c7e059c..5e34c77efa5e 100644
> --- a/drivers/mmc/core/pwrseq_simple.c
> +++ b/drivers/mmc/core/pwrseq_simple.c
> @@ -7,6 +7,7 @@
> *
> * Simple MMC power sequence management
> */
> +#include <linux/clk.h>
> #include <linux/kernel.h>
> #include <linux/slab.h>
> #include <linux/device.h>
> @@ -20,6 +21,7 @@
>
> struct mmc_pwrseq_simple {
> struct mmc_pwrseq pwrseq;
> + struct clk *ext_clk;
You need to add a bool, maybe call it clk_enabled; See why below.
> int nr_gpios;
> struct gpio_desc *reset_gpios[0];
> };
> @@ -39,6 +41,9 @@ static void mmc_pwrseq_simple_pre_power_on(struct mmc_host *host)
> struct mmc_pwrseq_simple *pwrseq = container_of(host->pwrseq,
> struct mmc_pwrseq_simple, pwrseq);
>
> + if (!IS_ERR(pwrseq->ext_clk))
> + clk_prepare_enable(pwrseq->ext_clk);
> +
There are no guarantee that the ->mmc_pwrseq_simple_pre_power_on()
will be invoked prior ->mmc_pwrseq_simple_power_off().
That means you need to keep track of if you have gated/ungated the
clock. In other words check pwrseq->clk_enabled. That will prevent
potential clk unbalance issues.
> mmc_pwrseq_simple_set_gpios_value(pwrseq, 1);
> }
>
> @@ -50,6 +55,17 @@ static void mmc_pwrseq_simple_post_power_on(struct mmc_host *host)
> mmc_pwrseq_simple_set_gpios_value(pwrseq, 0);
> }
>
> +static void mmc_pwrseq_simple_power_off(struct mmc_host *host)
> +{
> + struct mmc_pwrseq_simple *pwrseq = container_of(host->pwrseq,
> + struct mmc_pwrseq_simple, pwrseq);
> +
> + mmc_pwrseq_simple_set_gpios_value(pwrseq, 1);
> +
> + if (!IS_ERR(pwrseq->ext_clk))
> + clk_disable_unprepare(pwrseq->ext_clk);
Same comment as above.
> +}
> +
> static void mmc_pwrseq_simple_free(struct mmc_host *host)
> {
> struct mmc_pwrseq_simple *pwrseq = container_of(host->pwrseq,
> @@ -60,6 +76,9 @@ static void mmc_pwrseq_simple_free(struct mmc_host *host)
> if (!IS_ERR(pwrseq->reset_gpios[i]))
> gpiod_put(pwrseq->reset_gpios[i]);
>
> + if (!IS_ERR(pwrseq->ext_clk))
> + clk_put(pwrseq->ext_clk);
> +
> kfree(pwrseq);
> host->pwrseq = NULL;
> }
> @@ -67,7 +86,7 @@ static void mmc_pwrseq_simple_free(struct mmc_host *host)
> static struct mmc_pwrseq_ops mmc_pwrseq_simple_ops = {
> .pre_power_on = mmc_pwrseq_simple_pre_power_on,
> .post_power_on = mmc_pwrseq_simple_post_power_on,
> - .power_off = mmc_pwrseq_simple_pre_power_on,
> + .power_off = mmc_pwrseq_simple_power_off,
> .free = mmc_pwrseq_simple_free,
> };
>
> @@ -85,6 +104,14 @@ int mmc_pwrseq_simple_alloc(struct mmc_host *host, struct device *dev)
> if (!pwrseq)
> return -ENOMEM;
>
> + pwrseq->ext_clk = clk_get(dev, "ext_clock");
> + if (IS_ERR(pwrseq->ext_clk) &&
> + PTR_ERR(pwrseq->ext_clk) != -ENOENT &&
> + PTR_ERR(pwrseq->ext_clk) != -ENOSYS) {
I don't think you can get -ENOSYS.
> + ret = PTR_ERR(pwrseq->ext_clk);
> + goto free;
> + }
> +
> for (i = 0; i < nr_gpios; i++) {
> pwrseq->reset_gpios[i] = gpiod_get_index(dev, "reset", i,
> GPIOD_OUT_HIGH);
> @@ -96,7 +123,7 @@ int mmc_pwrseq_simple_alloc(struct mmc_host *host, struct device *dev)
> while (--i)
> gpiod_put(pwrseq->reset_gpios[i]);
>
> - goto free;
> + goto clk_put;
> }
> }
>
> @@ -105,6 +132,9 @@ int mmc_pwrseq_simple_alloc(struct mmc_host *host, struct device *dev)
> host->pwrseq = &pwrseq->pwrseq;
>
> return 0;
> +clk_put:
> + if (!IS_ERR(pwrseq->ext_clk))
> + clk_put(pwrseq->ext_clk);
> free:
> kfree(pwrseq);
> return ret;
> --
> 2.1.3
>
Kind regards
Uffe
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2 4/6] mmc: pwrseq_simple: Add optional reference clock support
2015-01-29 13:05 ` Ulf Hansson
@ 2015-01-29 13:53 ` Javier Martinez Canillas
0 siblings, 0 replies; 9+ messages in thread
From: Javier Martinez Canillas @ 2015-01-29 13:53 UTC (permalink / raw)
To: Ulf Hansson
Cc: Kukjin Kim, Doug Anderson, Olof Johansson, Arend van Spriel,
Srinivas Kandagatla, linux-samsung-soc,
linux-arm-kernel@lists.infradead.org, linux-mmc,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Hello Ulf,
Thanks a lot for your feedback.
On 01/29/2015 02:05 PM, Ulf Hansson wrote:
>>
>> struct mmc_pwrseq_simple {
>> struct mmc_pwrseq pwrseq;
>> + struct clk *ext_clk;
>
> You need to add a bool, maybe call it clk_enabled; See why below.
>
Ok
>> int nr_gpios;
>> struct gpio_desc *reset_gpios[0];
>> };
>> @@ -39,6 +41,9 @@ static void mmc_pwrseq_simple_pre_power_on(struct mmc_host *host)
>> struct mmc_pwrseq_simple *pwrseq = container_of(host->pwrseq,
>> struct mmc_pwrseq_simple, pwrseq);
>>
>> + if (!IS_ERR(pwrseq->ext_clk))
>> + clk_prepare_enable(pwrseq->ext_clk);
>> +
>
> There are no guarantee that the ->mmc_pwrseq_simple_pre_power_on()
> will be invoked prior ->mmc_pwrseq_simple_power_off().
>
Got it, I didn't know that mmc_pwrseq_simple_power_off() could be invoked.
without mmc_pwrseq_simple_pre_power_on() not being called before.
> That means you need to keep track of if you have gated/ungated the
> clock. In other words check pwrseq->clk_enabled. That will prevent
> potential clk unbalance issues.
Yes, I'll change to check for the boolean in _simple_power_off() and
_post_power_on() then.
>> @@ -85,6 +104,14 @@ int mmc_pwrseq_simple_alloc(struct mmc_host *host, struct device *dev)
>> if (!pwrseq)
>> return -ENOMEM;
>>
>> + pwrseq->ext_clk = clk_get(dev, "ext_clock");
>> + if (IS_ERR(pwrseq->ext_clk) &&
>> + PTR_ERR(pwrseq->ext_clk) != -ENOENT &&
>> + PTR_ERR(pwrseq->ext_clk) != -ENOSYS) {
>
> I don't think you can get -ENOSYS.
>
You are right, I'll remove that.
Best regards,
Javier
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 5/6] ARM: dts: exynos5250-snow: Enable wifi power-on
2015-01-28 18:15 [PATCH v2 0/6] Add multiple GPIO and external clock to MMC pwrseq_simple Javier Martinez Canillas
` (3 preceding siblings ...)
2015-01-28 18:15 ` [PATCH v2 4/6] mmc: pwrseq_simple: Add optional reference clock support Javier Martinez Canillas
@ 2015-01-28 18:15 ` Javier Martinez Canillas
2015-01-28 18:15 ` [PATCH v2 6/6] ARM: dts: exynos5250-snow: Add cap-sdio-irq to wifi mmc node Javier Martinez Canillas
5 siblings, 0 replies; 9+ messages in thread
From: Javier Martinez Canillas @ 2015-01-28 18:15 UTC (permalink / raw)
To: Ulf Hansson
Cc: Kukjin Kim, Doug Anderson, Olof Johansson, Arend van Spriel,
Srinivas Kandagatla, linux-samsung-soc, linux-arm-kernel,
linux-mmc, devicetree, linux-kernel, Javier Martinez Canillas
The Snow board has a MMC/SDIO wifi chip that is always powered but it
needs a power sequence involving a reset (active low) and an enable
(active high) pins. Both pins are marked as active low since the MMC
simple power sequence driver asserts the pins prior to the card power
up procedure and de-asserts the pins after the card has been powered.
So the reset line will be left de-asserted and the enable pin will be
left asserted.
The chip also needs an external 32kHz reference clock to be operational
that is by the MAX77686 PMIC clock.
Add a simple MMC power sequence provider for the wifi MMC/SDIO slot.
Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
Changes since v1:
- Remove cap-sdio-irq from mmc3 dev node since is a separate change.
Suggested by Arend van Spriel.
---
arch/arm/boot/dts/exynos5250-snow.dts | 25 ++++++++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/exynos5250-snow.dts b/arch/arm/boot/dts/exynos5250-snow.dts
index b9aeec430527..b78712058903 100644
--- a/arch/arm/boot/dts/exynos5250-snow.dts
+++ b/arch/arm/boot/dts/exynos5250-snow.dts
@@ -229,6 +229,14 @@
power-supply = <&fet6>;
backlight = <&backlight>;
};
+
+ mmc3_pwrseq: mmc3_pwrseq {
+ compatible = "mmc-pwrseq-simple";
+ reset-gpios = <&gpx0 2 GPIO_ACTIVE_LOW>, /* WIFI_RSTn */
+ <&gpx0 1 GPIO_ACTIVE_LOW>; /* WIFI_EN */
+ clocks = <&max77686 MAX77686_CLK_PMIC>;
+ clock-names = "ext_clock";
+ };
};
&dp {
@@ -536,12 +544,27 @@
samsung,dw-mshc-sdr-timing = <2 3>;
samsung,dw-mshc-ddr-timing = <1 2>;
pinctrl-names = "default";
- pinctrl-0 = <&sd3_clk &sd3_cmd &sd3_bus4>;
+ pinctrl-0 = <&sd3_clk &sd3_cmd &sd3_bus4 &wifi_en &wifi_rst>;
bus-width = <4>;
cap-sd-highspeed;
+ mmc-pwrseq = <&mmc3_pwrseq>;
};
&pinctrl_0 {
+ wifi_en: wifi-en {
+ samsung,pins = "gpx0-1";
+ samsung,pin-function = <1>;
+ samsung,pin-pud = <0>;
+ samsung,pin-drv = <0>;
+ };
+
+ wifi_rst: wifi-rst {
+ samsung,pins = "gpx0-2";
+ samsung,pin-function = <1>;
+ samsung,pin-pud = <0>;
+ samsung,pin-drv = <0>;
+ };
+
power_key_irq: power-key-irq {
samsung,pins = "gpx1-3";
samsung,pin-function = <0xf>;
--
2.1.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v2 6/6] ARM: dts: exynos5250-snow: Add cap-sdio-irq to wifi mmc node
2015-01-28 18:15 [PATCH v2 0/6] Add multiple GPIO and external clock to MMC pwrseq_simple Javier Martinez Canillas
` (4 preceding siblings ...)
2015-01-28 18:15 ` [PATCH v2 5/6] ARM: dts: exynos5250-snow: Enable wifi power-on Javier Martinez Canillas
@ 2015-01-28 18:15 ` Javier Martinez Canillas
5 siblings, 0 replies; 9+ messages in thread
From: Javier Martinez Canillas @ 2015-01-28 18:15 UTC (permalink / raw)
To: Ulf Hansson
Cc: Kukjin Kim, Doug Anderson, Olof Johansson, Arend van Spriel,
Srinivas Kandagatla, linux-samsung-soc, linux-arm-kernel,
linux-mmc, devicetree, linux-kernel, Javier Martinez Canillas
Enabling SDIO IRQ signalling for the wifi MMC/SDIO slot
doubles the transmission transfer rate.
Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
Changes since v1: None, new patch.
---
arch/arm/boot/dts/exynos5250-snow.dts | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/exynos5250-snow.dts b/arch/arm/boot/dts/exynos5250-snow.dts
index b78712058903..909edc3448d3 100644
--- a/arch/arm/boot/dts/exynos5250-snow.dts
+++ b/arch/arm/boot/dts/exynos5250-snow.dts
@@ -539,6 +539,7 @@
status = "okay";
num-slots = <1>;
broken-cd;
+ cap-sdio-irq;
card-detect-delay = <200>;
samsung,dw-mshc-ciu-div = <3>;
samsung,dw-mshc-sdr-timing = <2 3>;
--
2.1.3
^ permalink raw reply related [flat|nested] 9+ messages in thread