[PATCH 0/5] Add multiple GPIO and external clock to MMC pwrseq_simple

[PATCH 0/5] Add multiple GPIO and external clock to MMC pwrseq_simple

[Javier Martinez Canillas]

Hello Ulf,

Many WLAN chips attached to an SDIO interface needs more than one GPIO
for their reset sequence and also need an external clock to be operational.

Since this is very common, this series extend the simple MMC power sequence
to support more than one reset GPIO and also an optional external clock.

The series depend on v4 "mmc: core: Add support for MMC power sequences":

http://comments.gmane.org/gmane.linux.kernel.mmc/30665

Javier Martinez Canillas (5):
  mmc: pwrseq: Document that simple sequence support more than one GPIO
  mmc: pwrseq_simple: Extend to support more pins
  mmc: pwrseq: Document optional clock for the simple power sequence
  mmc: pwrseq_simple: Add optional reference clock support
  ARM: dts: exynos5250-snow: Enable wifi power-on

 .../devicetree/bindings/mmc/mmc-pwrseq-simple.txt  | 10 ++-
 arch/arm/boot/dts/exynos5250-snow.dts              | 25 ++++++-
 drivers/mmc/core/pwrseq_simple.c                   | 85 ++++++++++++++++++----
 3 files changed, 103 insertions(+), 17 deletions(-)

Patch #1 extends the simple MMC power sequence DT binding to support more
than one GPIO and patch #2 adds the actual implementation.

In the same way, patch #3 and #4 extend the simple MMC power sequence DT
binding and pwrseq_simple driver to support an optional external clock.

Finally as an example, patch #5 adds support for the wifi chip in the
Exynos5250 Snow that is attached to a SDIO interface and needs all these.

Best regards,
Javier

[PATCH 0/5] Add multiple GPIO and external clock to MMC pwrseq_simple

[Javier Martinez Canillas]

Hello Ulf,

Many WLAN chips attached to an SDIO interface needs more than one GPIO
for their reset sequence and also need an external clock to be operational.

Since this is very common, this series extend the simple MMC power sequence
to support more than one reset GPIO and also an optional external clock.

The series depend on v4 "mmc: core: Add support for MMC power sequences":

http://comments.gmane.org/gmane.linux.kernel.mmc/30665

Javier Martinez Canillas (5):
  mmc: pwrseq: Document that simple sequence support more than one GPIO
  mmc: pwrseq_simple: Extend to support more pins
  mmc: pwrseq: Document optional clock for the simple power sequence
  mmc: pwrseq_simple: Add optional reference clock support
  ARM: dts: exynos5250-snow: Enable wifi power-on

 .../devicetree/bindings/mmc/mmc-pwrseq-simple.txt  | 10 ++-
 arch/arm/boot/dts/exynos5250-snow.dts              | 25 ++++++-
 drivers/mmc/core/pwrseq_simple.c                   | 85 ++++++++++++++++++----
 3 files changed, 103 insertions(+), 17 deletions(-)

Patch #1 extends the simple MMC power sequence DT binding to support more
than one GPIO and patch #2 adds the actual implementation.

In the same way, patch #3 and #4 extend the simple MMC power sequence DT
binding and pwrseq_simple driver to support an optional external clock.

Finally as an example, patch #5 adds support for the wifi chip in the
Exynos5250 Snow that is attached to a SDIO interface and needs all these.

Best regards,
Javier

[PATCH 0/5] Add multiple GPIO and external clock to MMC pwrseq_simple

[Javier Martinez Canillas]

Hello Ulf,

Many WLAN chips attached to an SDIO interface needs more than one GPIO
for their reset sequence and also need an external clock to be operational.

Since this is very common, this series extend the simple MMC power sequence
to support more than one reset GPIO and also an optional external clock.

The series depend on v4 "mmc: core: Add support for MMC power sequences":

http://comments.gmane.org/gmane.linux.kernel.mmc/30665

Javier Martinez Canillas (5):
  mmc: pwrseq: Document that simple sequence support more than one GPIO
  mmc: pwrseq_simple: Extend to support more pins
  mmc: pwrseq: Document optional clock for the simple power sequence
  mmc: pwrseq_simple: Add optional reference clock support
  ARM: dts: exynos5250-snow: Enable wifi power-on

 .../devicetree/bindings/mmc/mmc-pwrseq-simple.txt  | 10 ++-
 arch/arm/boot/dts/exynos5250-snow.dts              | 25 ++++++-
 drivers/mmc/core/pwrseq_simple.c                   | 85 ++++++++++++++++++----
 3 files changed, 103 insertions(+), 17 deletions(-)

Patch #1 extends the simple MMC power sequence DT binding to support more
than one GPIO and patch #2 adds the actual implementation.

In the same way, patch #3 and #4 extend the simple MMC power sequence DT
binding and pwrseq_simple driver to support an optional external clock.

Finally as an example, patch #5 adds support for the wifi chip in the
Exynos5250 Snow that is attached to a SDIO interface and needs all these.

Best regards,
Javier

[PATCH 1/5] mmc: pwrseq: Document that simple sequence support more than one GPIO

[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>
---
 Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt | 6 +++---
 1 file changed, 3 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..9a77deb164c7 100644
--- a/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
+++ b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
@@ -8,9 +8,9 @@ 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.
+	Will be de-asserted right after the power has been provided to the card.
 
 Example:
 
-- 
2.1.3

[PATCH 1/5] mmc: pwrseq: Document that simple sequence support more than one GPIO

[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>
---
 Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt | 6 +++---
 1 file changed, 3 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..9a77deb164c7 100644
--- a/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
+++ b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
@@ -8,9 +8,9 @@ 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.
+	Will be de-asserted right after the power has been provided to the card.
 
 Example:
 
-- 
2.1.3

[PATCH 1/5] mmc: pwrseq: Document that simple sequence support more than one GPIO

[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>
---
 Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt | 6 +++---
 1 file changed, 3 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..9a77deb164c7 100644
--- a/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
+++ b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
@@ -8,9 +8,9 @@ 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.
+	Will be de-asserted right after the power has been provided to the card.
 
 Example:
 
-- 
2.1.3

Re: [PATCH 1/5] mmc: pwrseq: Document that simple sequence support more than one GPIO

[Srinivas Kandagatla]

On 28/01/15 10:10, Javier Martinez Canillas wrote:
> 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>
> ---
>   Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt | 6 +++---
>   1 file changed, 3 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..9a77deb164c7 100644
> --- a/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
> +++ b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
> @@ -8,9 +8,9 @@ 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.
> +	Will be de-asserted right after the power has been provided to the card.
Makes sense if you add "They" before last sentence.
>
>   Example:
>
>

[PATCH 1/5] mmc: pwrseq: Document that simple sequence support more than one GPIO

[Srinivas Kandagatla]

On 28/01/15 10:10, Javier Martinez Canillas wrote:
> 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>
> ---
>   Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt | 6 +++---
>   1 file changed, 3 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..9a77deb164c7 100644
> --- a/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
> +++ b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
> @@ -8,9 +8,9 @@ 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.
> +	Will be de-asserted right after the power has been provided to the card.
Makes sense if you add "They" before last sentence.
>
>   Example:
>
>

Re: [PATCH 1/5] mmc: pwrseq: Document that simple sequence support more than one GPIO

[Javier Martinez Canillas]

Srinivas,

On 01/28/2015 05:34 PM, Srinivas Kandagatla wrote:
>> -- 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.
>> +	Will be de-asserted right after the power has been provided to the card.
> Makes sense if you add "They" before last sentence.

Thanks, I'll change it.

Best regards,
Javier

[PATCH 1/5] mmc: pwrseq: Document that simple sequence support more than one GPIO

[Javier Martinez Canillas]

Srinivas,

On 01/28/2015 05:34 PM, Srinivas Kandagatla wrote:
>> -- 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.
>> +	Will be de-asserted right after the power has been provided to the card.
> Makes sense if you add "They" before last sentence.

Thanks, I'll change it.

Best regards,
Javier

[PATCH 2/5] mmc: pwrseq_simple: Extend to support more pins

[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>
---
 drivers/mmc/core/pwrseq_simple.c | 54 ++++++++++++++++++++++++++++++----------
 1 file changed, 41 insertions(+), 13 deletions(-)

diff --git a/drivers/mmc/core/pwrseq_simple.c b/drivers/mmc/core/pwrseq_simple.c
index 0958c696137f..9e51fe1051c5 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,34 +20,44 @@
 
 struct mmc_pwrseq_simple {
 	struct mmc_pwrseq pwrseq;
-	struct gpio_desc *reset_gpio;
+	struct gpio_desc **reset_gpio;
+	int nr_gpios;
 };
 
 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);
+	int i;
 
-	if (!IS_ERR(pwrseq->reset_gpio))
-		gpiod_set_value_cansleep(pwrseq->reset_gpio, 1);
+	for (i = 0; i < pwrseq->nr_gpios; i++)
+		if (!IS_ERR(pwrseq->reset_gpio[i]))
+			gpiod_set_value_cansleep(pwrseq->reset_gpio[i], 1);
 }
 
 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);
+	int i;
 
-	if (!IS_ERR(pwrseq->reset_gpio))
-		gpiod_set_value_cansleep(pwrseq->reset_gpio, 0);
+	for (i = 0; i < pwrseq->nr_gpios; i++)
+		if (!IS_ERR(pwrseq->reset_gpio[i]))
+			gpiod_set_value_cansleep(pwrseq->reset_gpio[i], 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);
+	if (pwrseq->nr_gpios > 0) {
+		for (i = 0; i < pwrseq->nr_gpios; i++)
+			if (!IS_ERR(pwrseq->reset_gpio[i]))
+				gpiod_put(pwrseq->reset_gpio[i]);
+		kfree(pwrseq->reset_gpio);
+	}
 
 	kfree(pwrseq);
 	host->pwrseq = NULL;
@@ -63,17 +74,27 @@ int mmc_pwrseq_simple_alloc(struct mmc_host *host, struct device *dev)
 {
 	struct mmc_pwrseq_simple *pwrseq;
 	int ret = 0;
+	int i;
 
 	pwrseq = kzalloc(sizeof(struct mmc_pwrseq_simple), 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;
+	pwrseq->nr_gpios = of_gpio_named_count(dev->of_node, "reset-gpios");
+	if (pwrseq->nr_gpios > 0) {
+		pwrseq->reset_gpio = kzalloc(sizeof(struct gpio_desc *) *
+					     pwrseq->nr_gpios, GFP_KERNEL);
+
+		for (i = 0; i < pwrseq->nr_gpios; i++) {
+			pwrseq->reset_gpio[i] = gpiod_get_index(dev, "reset", i,
+								GPIOD_OUT_HIGH);
+			if (IS_ERR(pwrseq->reset_gpio[i]) &&
+			    PTR_ERR(pwrseq->reset_gpio[i]) != -ENOENT &&
+			    PTR_ERR(pwrseq->reset_gpio[i]) != -ENOSYS) {
+				ret = PTR_ERR(pwrseq->reset_gpio[i]);
+				goto free;
+			}
+		}
 	}
 
 	pwrseq->pwrseq.ops = &mmc_pwrseq_simple_ops;
@@ -81,6 +102,13 @@ int mmc_pwrseq_simple_alloc(struct mmc_host *host, struct device *dev)
 
 	return 0;
 free:
+	if (pwrseq->nr_gpios > 0) {
+		for (i = 0; i < pwrseq->nr_gpios; i++)
+			if (!IS_ERR_OR_NULL(pwrseq->reset_gpio[i]))
+				gpiod_put(pwrseq->reset_gpio[i]);
+		kfree(pwrseq->reset_gpio);
+	}
+
 	kfree(pwrseq);
 	return ret;
 }
-- 
2.1.3

[PATCH 2/5] mmc: pwrseq_simple: Extend to support more pins

[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>
---
 drivers/mmc/core/pwrseq_simple.c | 54 ++++++++++++++++++++++++++++++----------
 1 file changed, 41 insertions(+), 13 deletions(-)

diff --git a/drivers/mmc/core/pwrseq_simple.c b/drivers/mmc/core/pwrseq_simple.c
index 0958c696137f..9e51fe1051c5 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,34 +20,44 @@
 
 struct mmc_pwrseq_simple {
 	struct mmc_pwrseq pwrseq;
-	struct gpio_desc *reset_gpio;
+	struct gpio_desc **reset_gpio;
+	int nr_gpios;
 };
 
 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);
+	int i;
 
-	if (!IS_ERR(pwrseq->reset_gpio))
-		gpiod_set_value_cansleep(pwrseq->reset_gpio, 1);
+	for (i = 0; i < pwrseq->nr_gpios; i++)
+		if (!IS_ERR(pwrseq->reset_gpio[i]))
+			gpiod_set_value_cansleep(pwrseq->reset_gpio[i], 1);
 }
 
 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);
+	int i;
 
-	if (!IS_ERR(pwrseq->reset_gpio))
-		gpiod_set_value_cansleep(pwrseq->reset_gpio, 0);
+	for (i = 0; i < pwrseq->nr_gpios; i++)
+		if (!IS_ERR(pwrseq->reset_gpio[i]))
+			gpiod_set_value_cansleep(pwrseq->reset_gpio[i], 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);
+	if (pwrseq->nr_gpios > 0) {
+		for (i = 0; i < pwrseq->nr_gpios; i++)
+			if (!IS_ERR(pwrseq->reset_gpio[i]))
+				gpiod_put(pwrseq->reset_gpio[i]);
+		kfree(pwrseq->reset_gpio);
+	}
 
 	kfree(pwrseq);
 	host->pwrseq = NULL;
@@ -63,17 +74,27 @@ int mmc_pwrseq_simple_alloc(struct mmc_host *host, struct device *dev)
 {
 	struct mmc_pwrseq_simple *pwrseq;
 	int ret = 0;
+	int i;
 
 	pwrseq = kzalloc(sizeof(struct mmc_pwrseq_simple), 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;
+	pwrseq->nr_gpios = of_gpio_named_count(dev->of_node, "reset-gpios");
+	if (pwrseq->nr_gpios > 0) {
+		pwrseq->reset_gpio = kzalloc(sizeof(struct gpio_desc *) *
+					     pwrseq->nr_gpios, GFP_KERNEL);
+
+		for (i = 0; i < pwrseq->nr_gpios; i++) {
+			pwrseq->reset_gpio[i] = gpiod_get_index(dev, "reset", i,
+								GPIOD_OUT_HIGH);
+			if (IS_ERR(pwrseq->reset_gpio[i]) &&
+			    PTR_ERR(pwrseq->reset_gpio[i]) != -ENOENT &&
+			    PTR_ERR(pwrseq->reset_gpio[i]) != -ENOSYS) {
+				ret = PTR_ERR(pwrseq->reset_gpio[i]);
+				goto free;
+			}
+		}
 	}
 
 	pwrseq->pwrseq.ops = &mmc_pwrseq_simple_ops;
@@ -81,6 +102,13 @@ int mmc_pwrseq_simple_alloc(struct mmc_host *host, struct device *dev)
 
 	return 0;
 free:
+	if (pwrseq->nr_gpios > 0) {
+		for (i = 0; i < pwrseq->nr_gpios; i++)
+			if (!IS_ERR_OR_NULL(pwrseq->reset_gpio[i]))
+				gpiod_put(pwrseq->reset_gpio[i]);
+		kfree(pwrseq->reset_gpio);
+	}
+
 	kfree(pwrseq);
 	return ret;
 }
-- 
2.1.3

[PATCH 2/5] mmc: pwrseq_simple: Extend to support more pins

[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>
---
 drivers/mmc/core/pwrseq_simple.c | 54 ++++++++++++++++++++++++++++++----------
 1 file changed, 41 insertions(+), 13 deletions(-)

diff --git a/drivers/mmc/core/pwrseq_simple.c b/drivers/mmc/core/pwrseq_simple.c
index 0958c696137f..9e51fe1051c5 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,34 +20,44 @@
 
 struct mmc_pwrseq_simple {
 	struct mmc_pwrseq pwrseq;
-	struct gpio_desc *reset_gpio;
+	struct gpio_desc **reset_gpio;
+	int nr_gpios;
 };
 
 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);
+	int i;
 
-	if (!IS_ERR(pwrseq->reset_gpio))
-		gpiod_set_value_cansleep(pwrseq->reset_gpio, 1);
+	for (i = 0; i < pwrseq->nr_gpios; i++)
+		if (!IS_ERR(pwrseq->reset_gpio[i]))
+			gpiod_set_value_cansleep(pwrseq->reset_gpio[i], 1);
 }
 
 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);
+	int i;
 
-	if (!IS_ERR(pwrseq->reset_gpio))
-		gpiod_set_value_cansleep(pwrseq->reset_gpio, 0);
+	for (i = 0; i < pwrseq->nr_gpios; i++)
+		if (!IS_ERR(pwrseq->reset_gpio[i]))
+			gpiod_set_value_cansleep(pwrseq->reset_gpio[i], 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);
+	if (pwrseq->nr_gpios > 0) {
+		for (i = 0; i < pwrseq->nr_gpios; i++)
+			if (!IS_ERR(pwrseq->reset_gpio[i]))
+				gpiod_put(pwrseq->reset_gpio[i]);
+		kfree(pwrseq->reset_gpio);
+	}
 
 	kfree(pwrseq);
 	host->pwrseq = NULL;
@@ -63,17 +74,27 @@ int mmc_pwrseq_simple_alloc(struct mmc_host *host, struct device *dev)
 {
 	struct mmc_pwrseq_simple *pwrseq;
 	int ret = 0;
+	int i;
 
 	pwrseq = kzalloc(sizeof(struct mmc_pwrseq_simple), 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;
+	pwrseq->nr_gpios = of_gpio_named_count(dev->of_node, "reset-gpios");
+	if (pwrseq->nr_gpios > 0) {
+		pwrseq->reset_gpio = kzalloc(sizeof(struct gpio_desc *) *
+					     pwrseq->nr_gpios, GFP_KERNEL);
+
+		for (i = 0; i < pwrseq->nr_gpios; i++) {
+			pwrseq->reset_gpio[i] = gpiod_get_index(dev, "reset", i,
+								GPIOD_OUT_HIGH);
+			if (IS_ERR(pwrseq->reset_gpio[i]) &&
+			    PTR_ERR(pwrseq->reset_gpio[i]) != -ENOENT &&
+			    PTR_ERR(pwrseq->reset_gpio[i]) != -ENOSYS) {
+				ret = PTR_ERR(pwrseq->reset_gpio[i]);
+				goto free;
+			}
+		}
 	}
 
 	pwrseq->pwrseq.ops = &mmc_pwrseq_simple_ops;
@@ -81,6 +102,13 @@ int mmc_pwrseq_simple_alloc(struct mmc_host *host, struct device *dev)
 
 	return 0;
 free:
+	if (pwrseq->nr_gpios > 0) {
+		for (i = 0; i < pwrseq->nr_gpios; i++)
+			if (!IS_ERR_OR_NULL(pwrseq->reset_gpio[i]))
+				gpiod_put(pwrseq->reset_gpio[i]);
+		kfree(pwrseq->reset_gpio);
+	}
+
 	kfree(pwrseq);
 	return ret;
 }
-- 
2.1.3

Re: [PATCH 2/5] mmc: pwrseq_simple: Extend to support more pins

[Srinivas Kandagatla]

Hi Javier,

You are in a lead of 3 hrs from me..
Surprisingly I send very much same patch just few Mins ago :-)
May be we can merge goods in both :-)

On 28/01/15 10:10, Javier Martinez Canillas wrote:
> 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>
> ---
>   drivers/mmc/core/pwrseq_simple.c | 54 ++++++++++++++++++++++++++++++----------
>   1 file changed, 41 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/mmc/core/pwrseq_simple.c b/drivers/mmc/core/pwrseq_simple.c
> index 0958c696137f..9e51fe1051c5 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,34 +20,44 @@
>
>   struct mmc_pwrseq_simple {
>   	struct mmc_pwrseq pwrseq;
> -	struct gpio_desc *reset_gpio;
> +	struct gpio_desc **reset_gpio;

May be renaming it to reset_gpios makes more sense..

If you make this struct gpio_desc *reset_gpios[0]; You can aviod an 
extra kmalloc and free ..


> +	int nr_gpios;
>   };
>
>   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);
> +	int i;
>
> -	if (!IS_ERR(pwrseq->reset_gpio))
> -		gpiod_set_value_cansleep(pwrseq->reset_gpio, 1);
> +	for (i = 0; i < pwrseq->nr_gpios; i++)
> +		if (!IS_ERR(pwrseq->reset_gpio[i]))
> +			gpiod_set_value_cansleep(pwrseq->reset_gpio[i], 1);

...]

>   }
>
>   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);
> +	int i;
>
> -	if (!IS_ERR(pwrseq->reset_gpio))
> -		gpiod_set_value_cansleep(pwrseq->reset_gpio, 0);
> +	for (i = 0; i < pwrseq->nr_gpios; i++)
> +		if (!IS_ERR(pwrseq->reset_gpio[i]))
> +			gpiod_set_value_cansleep(pwrseq->reset_gpio[i], 0);
...]

Now that we have more code in mmc_pwrseq_simple_post_power_on() and 
mmc_pwrseq_simple_pre_power_on(), Just move most of them into a common 
function like:

static void __mmc_pwrseq_simple_power_on_off(struct mmc_host *host,
						      bool on)
{
	struct mmc_pwrseq_simple *pwrseq = container_of(host->pwrseq,
					struct mmc_pwrseq_simple, pwrseq);
	int i;

	if (!IS_ERR(pwrseq->reset_gpios)) {
		for (i = 0; i < pwrseq->ngpios; i++)
			gpiod_set_value_cansleep(pwrseq->reset_gpios[i],
						 on ? : 0);
	}
}

static void mmc_pwrseq_simple_pre_power_on(struct mmc_host *host)
{
	__mmc_pwrseq_simple_power_on_off(host, true);
}

static void mmc_pwrseq_simple_post_power_on(struct mmc_host *host)
{
	__mmc_pwrseq_simple_power_on_off(host, false);
}


>   }
>
>   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);
> +	if (pwrseq->nr_gpios > 0) {
> +		for (i = 0; i < pwrseq->nr_gpios; i++)
> +			if (!IS_ERR(pwrseq->reset_gpio[i]))
> +				gpiod_put(pwrseq->reset_gpio[i]);
> +		kfree(pwrseq->reset_gpio);
> +	}
>
>   	kfree(pwrseq);
>   	host->pwrseq = NULL;
> @@ -63,17 +74,27 @@ int mmc_pwrseq_simple_alloc(struct mmc_host *host, struct device *dev)
>   {
>   	struct mmc_pwrseq_simple *pwrseq;
>   	int ret = 0;
> +	int i;
>
>   	pwrseq = kzalloc(sizeof(struct mmc_pwrseq_simple), 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;
> +	pwrseq->nr_gpios = of_gpio_named_count(dev->of_node, "reset-gpios");
> +	if (pwrseq->nr_gpios > 0) {

What happens if there are no gpios? This fuction should return -ENOENT 
and should not even try to allocate pwrseq?
Probably you should do of_gpio_named_count before allocating memory.

> +		pwrseq->reset_gpio = kzalloc(sizeof(struct gpio_desc *) *
> +					     pwrseq->nr_gpios, GFP_KERNEL);
> +
> +		for (i = 0; i < pwrseq->nr_gpios; i++) {
> +			pwrseq->reset_gpio[i] = gpiod_get_index(dev, "reset", i,
> +								GPIOD_OUT_HIGH);
> +			if (IS_ERR(pwrseq->reset_gpio[i]) &&
> +			    PTR_ERR(pwrseq->reset_gpio[i]) != -ENOENT &&
> +			    PTR_ERR(pwrseq->reset_gpio[i]) != -ENOSYS) {
> +				ret = PTR_ERR(pwrseq->reset_gpio[i]);

is simple to add:
	while(--i)
		gpiod_put(pwrseq->reset_gpio[i])


		
> +				goto free;
> +			}
> +		}


>   	}
>
>   	pwrseq->pwrseq.ops = &mmc_pwrseq_simple_ops;
> @@ -81,6 +102,13 @@ int mmc_pwrseq_simple_alloc(struct mmc_host *host, struct device *dev)
>
>   	return 0;
>   free:
> +	if (pwrseq->nr_gpios > 0) {
> +		for (i = 0; i < pwrseq->nr_gpios; i++)
> +			if (!IS_ERR_OR_NULL(pwrseq->reset_gpio[i]))
> +				gpiod_put(pwrseq->reset_gpio[i]);
> +		kfree(pwrseq->reset_gpio);
> +	}
> +
>   	kfree(pwrseq);
>   	return ret;
>   }
>

I get a feeling that am just dumping my patch here.. If possible could 
you have look at it too.

Thanks,
srini

[PATCH 2/5] mmc: pwrseq_simple: Extend to support more pins

[Srinivas Kandagatla]

Hi Javier,

You are in a lead of 3 hrs from me..
Surprisingly I send very much same patch just few Mins ago :-)
May be we can merge goods in both :-)

On 28/01/15 10:10, Javier Martinez Canillas wrote:
> 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>
> ---
>   drivers/mmc/core/pwrseq_simple.c | 54 ++++++++++++++++++++++++++++++----------
>   1 file changed, 41 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/mmc/core/pwrseq_simple.c b/drivers/mmc/core/pwrseq_simple.c
> index 0958c696137f..9e51fe1051c5 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,34 +20,44 @@
>
>   struct mmc_pwrseq_simple {
>   	struct mmc_pwrseq pwrseq;
> -	struct gpio_desc *reset_gpio;
> +	struct gpio_desc **reset_gpio;

May be renaming it to reset_gpios makes more sense..

If you make this struct gpio_desc *reset_gpios[0]; You can aviod an 
extra kmalloc and free ..


> +	int nr_gpios;
>   };
>
>   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);
> +	int i;
>
> -	if (!IS_ERR(pwrseq->reset_gpio))
> -		gpiod_set_value_cansleep(pwrseq->reset_gpio, 1);
> +	for (i = 0; i < pwrseq->nr_gpios; i++)
> +		if (!IS_ERR(pwrseq->reset_gpio[i]))
> +			gpiod_set_value_cansleep(pwrseq->reset_gpio[i], 1);

...]

>   }
>
>   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);
> +	int i;
>
> -	if (!IS_ERR(pwrseq->reset_gpio))
> -		gpiod_set_value_cansleep(pwrseq->reset_gpio, 0);
> +	for (i = 0; i < pwrseq->nr_gpios; i++)
> +		if (!IS_ERR(pwrseq->reset_gpio[i]))
> +			gpiod_set_value_cansleep(pwrseq->reset_gpio[i], 0);
...]

Now that we have more code in mmc_pwrseq_simple_post_power_on() and 
mmc_pwrseq_simple_pre_power_on(), Just move most of them into a common 
function like:

static void __mmc_pwrseq_simple_power_on_off(struct mmc_host *host,
						      bool on)
{
	struct mmc_pwrseq_simple *pwrseq = container_of(host->pwrseq,
					struct mmc_pwrseq_simple, pwrseq);
	int i;

	if (!IS_ERR(pwrseq->reset_gpios)) {
		for (i = 0; i < pwrseq->ngpios; i++)
			gpiod_set_value_cansleep(pwrseq->reset_gpios[i],
						 on ? : 0);
	}
}

static void mmc_pwrseq_simple_pre_power_on(struct mmc_host *host)
{
	__mmc_pwrseq_simple_power_on_off(host, true);
}

static void mmc_pwrseq_simple_post_power_on(struct mmc_host *host)
{
	__mmc_pwrseq_simple_power_on_off(host, false);
}


>   }
>
>   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);
> +	if (pwrseq->nr_gpios > 0) {
> +		for (i = 0; i < pwrseq->nr_gpios; i++)
> +			if (!IS_ERR(pwrseq->reset_gpio[i]))
> +				gpiod_put(pwrseq->reset_gpio[i]);
> +		kfree(pwrseq->reset_gpio);
> +	}
>
>   	kfree(pwrseq);
>   	host->pwrseq = NULL;
> @@ -63,17 +74,27 @@ int mmc_pwrseq_simple_alloc(struct mmc_host *host, struct device *dev)
>   {
>   	struct mmc_pwrseq_simple *pwrseq;
>   	int ret = 0;
> +	int i;
>
>   	pwrseq = kzalloc(sizeof(struct mmc_pwrseq_simple), 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;
> +	pwrseq->nr_gpios = of_gpio_named_count(dev->of_node, "reset-gpios");
> +	if (pwrseq->nr_gpios > 0) {

What happens if there are no gpios? This fuction should return -ENOENT 
and should not even try to allocate pwrseq?
Probably you should do of_gpio_named_count before allocating memory.

> +		pwrseq->reset_gpio = kzalloc(sizeof(struct gpio_desc *) *
> +					     pwrseq->nr_gpios, GFP_KERNEL);
> +
> +		for (i = 0; i < pwrseq->nr_gpios; i++) {
> +			pwrseq->reset_gpio[i] = gpiod_get_index(dev, "reset", i,
> +								GPIOD_OUT_HIGH);
> +			if (IS_ERR(pwrseq->reset_gpio[i]) &&
> +			    PTR_ERR(pwrseq->reset_gpio[i]) != -ENOENT &&
> +			    PTR_ERR(pwrseq->reset_gpio[i]) != -ENOSYS) {
> +				ret = PTR_ERR(pwrseq->reset_gpio[i]);

is simple to add:
	while(--i)
		gpiod_put(pwrseq->reset_gpio[i])


		
> +				goto free;
> +			}
> +		}


>   	}
>
>   	pwrseq->pwrseq.ops = &mmc_pwrseq_simple_ops;
> @@ -81,6 +102,13 @@ int mmc_pwrseq_simple_alloc(struct mmc_host *host, struct device *dev)
>
>   	return 0;
>   free:
> +	if (pwrseq->nr_gpios > 0) {
> +		for (i = 0; i < pwrseq->nr_gpios; i++)
> +			if (!IS_ERR_OR_NULL(pwrseq->reset_gpio[i]))
> +				gpiod_put(pwrseq->reset_gpio[i]);
> +		kfree(pwrseq->reset_gpio);
> +	}
> +
>   	kfree(pwrseq);
>   	return ret;
>   }
>

I get a feeling that am just dumping my patch here.. If possible could 
you have look at it too.

Thanks,
srini

Re: [PATCH 2/5] mmc: pwrseq_simple: Extend to support more pins

[Javier Martinez Canillas]

Hello Srinivas,

Thanks a lot for your feedback.

On 01/28/2015 03:01 PM, Srinivas Kandagatla wrote:
> Hi Javier,
> 
> You are in a lead of 3 hrs from me..
> Surprisingly I send very much same patch just few Mins ago :-)

:-)

I didn't find the posted patch you are referring too though, did you cc
linux-mmc?

> May be we can merge goods in both :-)
>

Sure, I want $subject to be generic enough to be useful for other platforms.

> On 28/01/15 10:10, Javier Martinez Canillas wrote:
>> 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>
>> ---
>>   drivers/mmc/core/pwrseq_simple.c | 54 ++++++++++++++++++++++++++++++----------
>>   1 file changed, 41 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/mmc/core/pwrseq_simple.c b/drivers/mmc/core/pwrseq_simple.c
>> index 0958c696137f..9e51fe1051c5 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,34 +20,44 @@
>>
>>   struct mmc_pwrseq_simple {
>>   	struct mmc_pwrseq pwrseq;
>> -	struct gpio_desc *reset_gpio;
>> +	struct gpio_desc **reset_gpio;
> 
> May be renaming it to reset_gpios makes more sense..
>

Ok

> If you make this struct gpio_desc *reset_gpios[0]; You can aviod an 
> extra kmalloc and free ..
> 
>

That's a very good idea, thanks.

>> +	int nr_gpios;
>>   };
>>
>>   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);
>> +	int i;
>>
>> -	if (!IS_ERR(pwrseq->reset_gpio))
>> -		gpiod_set_value_cansleep(pwrseq->reset_gpio, 1);
>> +	for (i = 0; i < pwrseq->nr_gpios; i++)
>> +		if (!IS_ERR(pwrseq->reset_gpio[i]))
>> +			gpiod_set_value_cansleep(pwrseq->reset_gpio[i], 1);
> 
> ...]
> 
>>   }
>>
>>   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);
>> +	int i;
>>
>> -	if (!IS_ERR(pwrseq->reset_gpio))
>> -		gpiod_set_value_cansleep(pwrseq->reset_gpio, 0);
>> +	for (i = 0; i < pwrseq->nr_gpios; i++)
>> +		if (!IS_ERR(pwrseq->reset_gpio[i]))
>> +			gpiod_set_value_cansleep(pwrseq->reset_gpio[i], 0);
> ...]
> 
> Now that we have more code in mmc_pwrseq_simple_post_power_on() and 
> mmc_pwrseq_simple_pre_power_on(), Just move most of them into a common 
> function like:
> 
> static void __mmc_pwrseq_simple_power_on_off(struct mmc_host *host,
> 						      bool on)
> {
> 	struct mmc_pwrseq_simple *pwrseq = container_of(host->pwrseq,
> 					struct mmc_pwrseq_simple, pwrseq);
> 	int i;
> 
> 	if (!IS_ERR(pwrseq->reset_gpios)) {
> 		for (i = 0; i < pwrseq->ngpios; i++)
> 			gpiod_set_value_cansleep(pwrseq->reset_gpios[i],
> 						 on ? : 0);
> 	}
> }
>
> static void mmc_pwrseq_simple_pre_power_on(struct mmc_host *host)
> {
> 	__mmc_pwrseq_simple_power_on_off(host, true);
> }
> 
> static void mmc_pwrseq_simple_post_power_on(struct mmc_host *host)
> {
> 	__mmc_pwrseq_simple_power_on_off(host, false);
> }
> 
>

Sure, will do.

>>   }
>>
>>   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);
>> +	if (pwrseq->nr_gpios > 0) {
>> +		for (i = 0; i < pwrseq->nr_gpios; i++)
>> +			if (!IS_ERR(pwrseq->reset_gpio[i]))
>> +				gpiod_put(pwrseq->reset_gpio[i]);
>> +		kfree(pwrseq->reset_gpio);
>> +	}
>>
>>   	kfree(pwrseq);
>>   	host->pwrseq = NULL;
>> @@ -63,17 +74,27 @@ int mmc_pwrseq_simple_alloc(struct mmc_host *host, struct device *dev)
>>   {
>>   	struct mmc_pwrseq_simple *pwrseq;
>>   	int ret = 0;
>> +	int i;
>>
>>   	pwrseq = kzalloc(sizeof(struct mmc_pwrseq_simple), 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;
>> +	pwrseq->nr_gpios = of_gpio_named_count(dev->of_node, "reset-gpios");
>> +	if (pwrseq->nr_gpios > 0) {
> 
> What happens if there are no gpios? This fuction should return -ENOENT 
> and should not even try to allocate pwrseq?

Not quite, the DT binding states that the GPIOs are optional so it should
not fail if no GPIOs are defined. 

> Probably you should do of_gpio_named_count before allocating memory.
>

I didn't do that because patch #4 "mmc: pwrseq_simple: Add optional reference
clock support" will need the struct mmc_pwrseq_simple even if no GPIOs are
defined.

A SDIO attached chip could require only an external clock or someone could
extend the pwrseq_simple driver to support an external regulator for example.

>> +		pwrseq->reset_gpio = kzalloc(sizeof(struct gpio_desc *) *
>> +					     pwrseq->nr_gpios, GFP_KERNEL);
>> +
>> +		for (i = 0; i < pwrseq->nr_gpios; i++) {
>> +			pwrseq->reset_gpio[i] = gpiod_get_index(dev, "reset", i,
>> +								GPIOD_OUT_HIGH);
>> +			if (IS_ERR(pwrseq->reset_gpio[i]) &&
>> +			    PTR_ERR(pwrseq->reset_gpio[i]) != -ENOENT &&
>> +			    PTR_ERR(pwrseq->reset_gpio[i]) != -ENOSYS) {
>> +				ret = PTR_ERR(pwrseq->reset_gpio[i]);
> 
> is simple to add:
> 	while(--i)
> 		gpiod_put(pwrseq->reset_gpio[i])
> 
> 
>

That's true, will change.

>> +				goto free;
>> +			}
>> +		}
> 
> 
>>   	}
>>
>>   	pwrseq->pwrseq.ops = &mmc_pwrseq_simple_ops;
>> @@ -81,6 +102,13 @@ int mmc_pwrseq_simple_alloc(struct mmc_host *host, struct device *dev)
>>
>>   	return 0;
>>   free:
>> +	if (pwrseq->nr_gpios > 0) {
>> +		for (i = 0; i < pwrseq->nr_gpios; i++)
>> +			if (!IS_ERR_OR_NULL(pwrseq->reset_gpio[i]))
>> +				gpiod_put(pwrseq->reset_gpio[i]);
>> +		kfree(pwrseq->reset_gpio);
>> +	}
>> +
>>   	kfree(pwrseq);
>>   	return ret;
>>   }
>>
> 
> I get a feeling that am just dumping my patch here.. If possible could 
> you have look at it too.
>

Of course, do you have a link archive since I can't find it on my inbox.

> Thanks,
> srini
>

Again, thanks a lot and best regards,
Javier

[PATCH 2/5] mmc: pwrseq_simple: Extend to support more pins

[Javier Martinez Canillas]

Hello Srinivas,

Thanks a lot for your feedback.

On 01/28/2015 03:01 PM, Srinivas Kandagatla wrote:
> Hi Javier,
> 
> You are in a lead of 3 hrs from me..
> Surprisingly I send very much same patch just few Mins ago :-)

:-)

I didn't find the posted patch you are referring too though, did you cc
linux-mmc?

> May be we can merge goods in both :-)
>

Sure, I want $subject to be generic enough to be useful for other platforms.

> On 28/01/15 10:10, Javier Martinez Canillas wrote:
>> 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>
>> ---
>>   drivers/mmc/core/pwrseq_simple.c | 54 ++++++++++++++++++++++++++++++----------
>>   1 file changed, 41 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/mmc/core/pwrseq_simple.c b/drivers/mmc/core/pwrseq_simple.c
>> index 0958c696137f..9e51fe1051c5 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,34 +20,44 @@
>>
>>   struct mmc_pwrseq_simple {
>>   	struct mmc_pwrseq pwrseq;
>> -	struct gpio_desc *reset_gpio;
>> +	struct gpio_desc **reset_gpio;
> 
> May be renaming it to reset_gpios makes more sense..
>

Ok

> If you make this struct gpio_desc *reset_gpios[0]; You can aviod an 
> extra kmalloc and free ..
> 
>

That's a very good idea, thanks.

>> +	int nr_gpios;
>>   };
>>
>>   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);
>> +	int i;
>>
>> -	if (!IS_ERR(pwrseq->reset_gpio))
>> -		gpiod_set_value_cansleep(pwrseq->reset_gpio, 1);
>> +	for (i = 0; i < pwrseq->nr_gpios; i++)
>> +		if (!IS_ERR(pwrseq->reset_gpio[i]))
>> +			gpiod_set_value_cansleep(pwrseq->reset_gpio[i], 1);
> 
> ...]
> 
>>   }
>>
>>   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);
>> +	int i;
>>
>> -	if (!IS_ERR(pwrseq->reset_gpio))
>> -		gpiod_set_value_cansleep(pwrseq->reset_gpio, 0);
>> +	for (i = 0; i < pwrseq->nr_gpios; i++)
>> +		if (!IS_ERR(pwrseq->reset_gpio[i]))
>> +			gpiod_set_value_cansleep(pwrseq->reset_gpio[i], 0);
> ...]
> 
> Now that we have more code in mmc_pwrseq_simple_post_power_on() and 
> mmc_pwrseq_simple_pre_power_on(), Just move most of them into a common 
> function like:
> 
> static void __mmc_pwrseq_simple_power_on_off(struct mmc_host *host,
> 						      bool on)
> {
> 	struct mmc_pwrseq_simple *pwrseq = container_of(host->pwrseq,
> 					struct mmc_pwrseq_simple, pwrseq);
> 	int i;
> 
> 	if (!IS_ERR(pwrseq->reset_gpios)) {
> 		for (i = 0; i < pwrseq->ngpios; i++)
> 			gpiod_set_value_cansleep(pwrseq->reset_gpios[i],
> 						 on ? : 0);
> 	}
> }
>
> static void mmc_pwrseq_simple_pre_power_on(struct mmc_host *host)
> {
> 	__mmc_pwrseq_simple_power_on_off(host, true);
> }
> 
> static void mmc_pwrseq_simple_post_power_on(struct mmc_host *host)
> {
> 	__mmc_pwrseq_simple_power_on_off(host, false);
> }
> 
>

Sure, will do.

>>   }
>>
>>   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);
>> +	if (pwrseq->nr_gpios > 0) {
>> +		for (i = 0; i < pwrseq->nr_gpios; i++)
>> +			if (!IS_ERR(pwrseq->reset_gpio[i]))
>> +				gpiod_put(pwrseq->reset_gpio[i]);
>> +		kfree(pwrseq->reset_gpio);
>> +	}
>>
>>   	kfree(pwrseq);
>>   	host->pwrseq = NULL;
>> @@ -63,17 +74,27 @@ int mmc_pwrseq_simple_alloc(struct mmc_host *host, struct device *dev)
>>   {
>>   	struct mmc_pwrseq_simple *pwrseq;
>>   	int ret = 0;
>> +	int i;
>>
>>   	pwrseq = kzalloc(sizeof(struct mmc_pwrseq_simple), 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;
>> +	pwrseq->nr_gpios = of_gpio_named_count(dev->of_node, "reset-gpios");
>> +	if (pwrseq->nr_gpios > 0) {
> 
> What happens if there are no gpios? This fuction should return -ENOENT 
> and should not even try to allocate pwrseq?

Not quite, the DT binding states that the GPIOs are optional so it should
not fail if no GPIOs are defined. 

> Probably you should do of_gpio_named_count before allocating memory.
>

I didn't do that because patch #4 "mmc: pwrseq_simple: Add optional reference
clock support" will need the struct mmc_pwrseq_simple even if no GPIOs are
defined.

A SDIO attached chip could require only an external clock or someone could
extend the pwrseq_simple driver to support an external regulator for example.

>> +		pwrseq->reset_gpio = kzalloc(sizeof(struct gpio_desc *) *
>> +					     pwrseq->nr_gpios, GFP_KERNEL);
>> +
>> +		for (i = 0; i < pwrseq->nr_gpios; i++) {
>> +			pwrseq->reset_gpio[i] = gpiod_get_index(dev, "reset", i,
>> +								GPIOD_OUT_HIGH);
>> +			if (IS_ERR(pwrseq->reset_gpio[i]) &&
>> +			    PTR_ERR(pwrseq->reset_gpio[i]) != -ENOENT &&
>> +			    PTR_ERR(pwrseq->reset_gpio[i]) != -ENOSYS) {
>> +				ret = PTR_ERR(pwrseq->reset_gpio[i]);
> 
> is simple to add:
> 	while(--i)
> 		gpiod_put(pwrseq->reset_gpio[i])
> 
> 
>

That's true, will change.

>> +				goto free;
>> +			}
>> +		}
> 
> 
>>   	}
>>
>>   	pwrseq->pwrseq.ops = &mmc_pwrseq_simple_ops;
>> @@ -81,6 +102,13 @@ int mmc_pwrseq_simple_alloc(struct mmc_host *host, struct device *dev)
>>
>>   	return 0;
>>   free:
>> +	if (pwrseq->nr_gpios > 0) {
>> +		for (i = 0; i < pwrseq->nr_gpios; i++)
>> +			if (!IS_ERR_OR_NULL(pwrseq->reset_gpio[i]))
>> +				gpiod_put(pwrseq->reset_gpio[i]);
>> +		kfree(pwrseq->reset_gpio);
>> +	}
>> +
>>   	kfree(pwrseq);
>>   	return ret;
>>   }
>>
> 
> I get a feeling that am just dumping my patch here.. If possible could 
> you have look at it too.
>

Of course, do you have a link archive since I can't find it on my inbox.

> Thanks,
> srini
>

Again, thanks a lot and best regards,
Javier

Re: [PATCH 2/5] mmc: pwrseq_simple: Extend to support more pins

[Srinivas Kandagatla]

On 28/01/15 16:13, Javier Martinez Canillas wrote:
> Hello Srinivas,
>
> Thanks a lot for your feedback.
>
> On 01/28/2015 03:01 PM, Srinivas Kandagatla wrote:
>> Hi Javier,
>>
>> You are in a lead of 3 hrs from me..
>> Surprisingly I send very much same patch just few Mins ago :-)
>
> :-)
>
> I didn't find the posted patch you are referring too though, did you cc
> linux-mmc?
>
>> May be we can merge goods in both :-)
>>
>
> Sure, I want $subject to be generic enough to be useful for other platforms.
>
>> On 28/01/15 10:10, Javier Martinez Canillas wrote:
>>> 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>
>>> ---
>>>    drivers/mmc/core/pwrseq_simple.c | 54 ++++++++++++++++++++++++++++++----------
>>>    1 file changed, 41 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/mmc/core/pwrseq_simple.c b/drivers/mmc/core/pwrseq_simple.c
>>> index 0958c696137f..9e51fe1051c5 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,34 +20,44 @@
>>>
>>>    struct mmc_pwrseq_simple {
>>>    	struct mmc_pwrseq pwrseq;
>>> -	struct gpio_desc *reset_gpio;
>>> +	struct gpio_desc **reset_gpio;
>>
>> May be renaming it to reset_gpios makes more sense..
>>
>
> Ok
>
>> If you make this struct gpio_desc *reset_gpios[0]; You can aviod an
>> extra kmalloc and free ..
>>
>>
>
> That's a very good idea, thanks.
>
>>> +	int nr_gpios;
>>>    };
>>>
>>>    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);
>>> +	int i;
>>>
>>> -	if (!IS_ERR(pwrseq->reset_gpio))
>>> -		gpiod_set_value_cansleep(pwrseq->reset_gpio, 1);
>>> +	for (i = 0; i < pwrseq->nr_gpios; i++)
>>> +		if (!IS_ERR(pwrseq->reset_gpio[i]))
>>> +			gpiod_set_value_cansleep(pwrseq->reset_gpio[i], 1);
>>
>> ...]
>>
>>>    }
>>>
>>>    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);
>>> +	int i;
>>>
>>> -	if (!IS_ERR(pwrseq->reset_gpio))
>>> -		gpiod_set_value_cansleep(pwrseq->reset_gpio, 0);
>>> +	for (i = 0; i < pwrseq->nr_gpios; i++)
>>> +		if (!IS_ERR(pwrseq->reset_gpio[i]))
>>> +			gpiod_set_value_cansleep(pwrseq->reset_gpio[i], 0);
>> ...]
>>
>> Now that we have more code in mmc_pwrseq_simple_post_power_on() and
>> mmc_pwrseq_simple_pre_power_on(), Just move most of them into a common
>> function like:
>>
>> static void __mmc_pwrseq_simple_power_on_off(struct mmc_host *host,
>> 						      bool on)
>> {
>> 	struct mmc_pwrseq_simple *pwrseq = container_of(host->pwrseq,
>> 					struct mmc_pwrseq_simple, pwrseq);
>> 	int i;
>>
>> 	if (!IS_ERR(pwrseq->reset_gpios)) {
>> 		for (i = 0; i < pwrseq->ngpios; i++)
>> 			gpiod_set_value_cansleep(pwrseq->reset_gpios[i],
>> 						 on ? : 0);
>> 	}
>> }
>>
>> static void mmc_pwrseq_simple_pre_power_on(struct mmc_host *host)
>> {
>> 	__mmc_pwrseq_simple_power_on_off(host, true);
>> }
>>
>> static void mmc_pwrseq_simple_post_power_on(struct mmc_host *host)
>> {
>> 	__mmc_pwrseq_simple_power_on_off(host, false);
>> }
>>
>>
>
> Sure, will do.
>
>>>    }
>>>
>>>    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);
>>> +	if (pwrseq->nr_gpios > 0) {
>>> +		for (i = 0; i < pwrseq->nr_gpios; i++)
>>> +			if (!IS_ERR(pwrseq->reset_gpio[i]))
>>> +				gpiod_put(pwrseq->reset_gpio[i]);
>>> +		kfree(pwrseq->reset_gpio);
>>> +	}
>>>
>>>    	kfree(pwrseq);
>>>    	host->pwrseq = NULL;
>>> @@ -63,17 +74,27 @@ int mmc_pwrseq_simple_alloc(struct mmc_host *host, struct device *dev)
>>>    {
>>>    	struct mmc_pwrseq_simple *pwrseq;
>>>    	int ret = 0;
>>> +	int i;
>>>
>>>    	pwrseq = kzalloc(sizeof(struct mmc_pwrseq_simple), 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;
>>> +	pwrseq->nr_gpios = of_gpio_named_count(dev->of_node, "reset-gpios");
>>> +	if (pwrseq->nr_gpios > 0) {
>>
>> What happens if there are no gpios? This fuction should return -ENOENT
>> and should not even try to allocate pwrseq?
>
> Not quite, the DT binding states that the GPIOs are optional so it should
> not fail if no GPIOs are defined.
>
>> Probably you should do of_gpio_named_count before allocating memory.
>>
>
> I didn't do that because patch #4 "mmc: pwrseq_simple: Add optional reference
> clock support" will need the struct mmc_pwrseq_simple even if no GPIOs are
> defined.
>
> A SDIO attached chip could require only an external clock or someone could
> extend the pwrseq_simple driver to support an external regulator for example.

That makes sense.

>
>>> +		pwrseq->reset_gpio = kzalloc(sizeof(struct gpio_desc *) *
>>> +					     pwrseq->nr_gpios, GFP_KERNEL);
>>> +
>>> +		for (i = 0; i < pwrseq->nr_gpios; i++) {
>>> +			pwrseq->reset_gpio[i] = gpiod_get_index(dev, "reset", i,
>>> +								GPIOD_OUT_HIGH);
>>> +			if (IS_ERR(pwrseq->reset_gpio[i]) &&
>>> +			    PTR_ERR(pwrseq->reset_gpio[i]) != -ENOENT &&
>>> +			    PTR_ERR(pwrseq->reset_gpio[i]) != -ENOSYS) {
>>> +				ret = PTR_ERR(pwrseq->reset_gpio[i]);
>>
>> is simple to add:
>> 	while(--i)
>> 		gpiod_put(pwrseq->reset_gpio[i])
>>
>>
>>
>
> That's true, will change.
>
>>> +				goto free;
>>> +			}
>>> +		}
>>
>>
>>>    	}
>>>
>>>    	pwrseq->pwrseq.ops = &mmc_pwrseq_simple_ops;
>>> @@ -81,6 +102,13 @@ int mmc_pwrseq_simple_alloc(struct mmc_host *host, struct device *dev)
>>>
>>>    	return 0;
>>>    free:
>>> +	if (pwrseq->nr_gpios > 0) {
>>> +		for (i = 0; i < pwrseq->nr_gpios; i++)
>>> +			if (!IS_ERR_OR_NULL(pwrseq->reset_gpio[i]))
>>> +				gpiod_put(pwrseq->reset_gpio[i]);
>>> +		kfree(pwrseq->reset_gpio);
>>> +	}
>>> +
>>>    	kfree(pwrseq);
>>>    	return ret;
>>>    }
>>>
>>
>> I get a feeling that am just dumping my patch here.. If possible could
>> you have look at it too.
>>
>
> Of course, do you have a link archive since I can't find it on my inbox.
I did send to linux-mmc@vger.kernel.org, Anyway i did forward you the patch.

thanks,
--srini

[PATCH 2/5] mmc: pwrseq_simple: Extend to support more pins

[Srinivas Kandagatla]

On 28/01/15 16:13, Javier Martinez Canillas wrote:
> Hello Srinivas,
>
> Thanks a lot for your feedback.
>
> On 01/28/2015 03:01 PM, Srinivas Kandagatla wrote:
>> Hi Javier,
>>
>> You are in a lead of 3 hrs from me..
>> Surprisingly I send very much same patch just few Mins ago :-)
>
> :-)
>
> I didn't find the posted patch you are referring too though, did you cc
> linux-mmc?
>
>> May be we can merge goods in both :-)
>>
>
> Sure, I want $subject to be generic enough to be useful for other platforms.
>
>> On 28/01/15 10:10, Javier Martinez Canillas wrote:
>>> 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>
>>> ---
>>>    drivers/mmc/core/pwrseq_simple.c | 54 ++++++++++++++++++++++++++++++----------
>>>    1 file changed, 41 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/mmc/core/pwrseq_simple.c b/drivers/mmc/core/pwrseq_simple.c
>>> index 0958c696137f..9e51fe1051c5 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,34 +20,44 @@
>>>
>>>    struct mmc_pwrseq_simple {
>>>    	struct mmc_pwrseq pwrseq;
>>> -	struct gpio_desc *reset_gpio;
>>> +	struct gpio_desc **reset_gpio;
>>
>> May be renaming it to reset_gpios makes more sense..
>>
>
> Ok
>
>> If you make this struct gpio_desc *reset_gpios[0]; You can aviod an
>> extra kmalloc and free ..
>>
>>
>
> That's a very good idea, thanks.
>
>>> +	int nr_gpios;
>>>    };
>>>
>>>    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);
>>> +	int i;
>>>
>>> -	if (!IS_ERR(pwrseq->reset_gpio))
>>> -		gpiod_set_value_cansleep(pwrseq->reset_gpio, 1);
>>> +	for (i = 0; i < pwrseq->nr_gpios; i++)
>>> +		if (!IS_ERR(pwrseq->reset_gpio[i]))
>>> +			gpiod_set_value_cansleep(pwrseq->reset_gpio[i], 1);
>>
>> ...]
>>
>>>    }
>>>
>>>    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);
>>> +	int i;
>>>
>>> -	if (!IS_ERR(pwrseq->reset_gpio))
>>> -		gpiod_set_value_cansleep(pwrseq->reset_gpio, 0);
>>> +	for (i = 0; i < pwrseq->nr_gpios; i++)
>>> +		if (!IS_ERR(pwrseq->reset_gpio[i]))
>>> +			gpiod_set_value_cansleep(pwrseq->reset_gpio[i], 0);
>> ...]
>>
>> Now that we have more code in mmc_pwrseq_simple_post_power_on() and
>> mmc_pwrseq_simple_pre_power_on(), Just move most of them into a common
>> function like:
>>
>> static void __mmc_pwrseq_simple_power_on_off(struct mmc_host *host,
>> 						      bool on)
>> {
>> 	struct mmc_pwrseq_simple *pwrseq = container_of(host->pwrseq,
>> 					struct mmc_pwrseq_simple, pwrseq);
>> 	int i;
>>
>> 	if (!IS_ERR(pwrseq->reset_gpios)) {
>> 		for (i = 0; i < pwrseq->ngpios; i++)
>> 			gpiod_set_value_cansleep(pwrseq->reset_gpios[i],
>> 						 on ? : 0);
>> 	}
>> }
>>
>> static void mmc_pwrseq_simple_pre_power_on(struct mmc_host *host)
>> {
>> 	__mmc_pwrseq_simple_power_on_off(host, true);
>> }
>>
>> static void mmc_pwrseq_simple_post_power_on(struct mmc_host *host)
>> {
>> 	__mmc_pwrseq_simple_power_on_off(host, false);
>> }
>>
>>
>
> Sure, will do.
>
>>>    }
>>>
>>>    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);
>>> +	if (pwrseq->nr_gpios > 0) {
>>> +		for (i = 0; i < pwrseq->nr_gpios; i++)
>>> +			if (!IS_ERR(pwrseq->reset_gpio[i]))
>>> +				gpiod_put(pwrseq->reset_gpio[i]);
>>> +		kfree(pwrseq->reset_gpio);
>>> +	}
>>>
>>>    	kfree(pwrseq);
>>>    	host->pwrseq = NULL;
>>> @@ -63,17 +74,27 @@ int mmc_pwrseq_simple_alloc(struct mmc_host *host, struct device *dev)
>>>    {
>>>    	struct mmc_pwrseq_simple *pwrseq;
>>>    	int ret = 0;
>>> +	int i;
>>>
>>>    	pwrseq = kzalloc(sizeof(struct mmc_pwrseq_simple), 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;
>>> +	pwrseq->nr_gpios = of_gpio_named_count(dev->of_node, "reset-gpios");
>>> +	if (pwrseq->nr_gpios > 0) {
>>
>> What happens if there are no gpios? This fuction should return -ENOENT
>> and should not even try to allocate pwrseq?
>
> Not quite, the DT binding states that the GPIOs are optional so it should
> not fail if no GPIOs are defined.
>
>> Probably you should do of_gpio_named_count before allocating memory.
>>
>
> I didn't do that because patch #4 "mmc: pwrseq_simple: Add optional reference
> clock support" will need the struct mmc_pwrseq_simple even if no GPIOs are
> defined.
>
> A SDIO attached chip could require only an external clock or someone could
> extend the pwrseq_simple driver to support an external regulator for example.

That makes sense.

>
>>> +		pwrseq->reset_gpio = kzalloc(sizeof(struct gpio_desc *) *
>>> +					     pwrseq->nr_gpios, GFP_KERNEL);
>>> +
>>> +		for (i = 0; i < pwrseq->nr_gpios; i++) {
>>> +			pwrseq->reset_gpio[i] = gpiod_get_index(dev, "reset", i,
>>> +								GPIOD_OUT_HIGH);
>>> +			if (IS_ERR(pwrseq->reset_gpio[i]) &&
>>> +			    PTR_ERR(pwrseq->reset_gpio[i]) != -ENOENT &&
>>> +			    PTR_ERR(pwrseq->reset_gpio[i]) != -ENOSYS) {
>>> +				ret = PTR_ERR(pwrseq->reset_gpio[i]);
>>
>> is simple to add:
>> 	while(--i)
>> 		gpiod_put(pwrseq->reset_gpio[i])
>>
>>
>>
>
> That's true, will change.
>
>>> +				goto free;
>>> +			}
>>> +		}
>>
>>
>>>    	}
>>>
>>>    	pwrseq->pwrseq.ops = &mmc_pwrseq_simple_ops;
>>> @@ -81,6 +102,13 @@ int mmc_pwrseq_simple_alloc(struct mmc_host *host, struct device *dev)
>>>
>>>    	return 0;
>>>    free:
>>> +	if (pwrseq->nr_gpios > 0) {
>>> +		for (i = 0; i < pwrseq->nr_gpios; i++)
>>> +			if (!IS_ERR_OR_NULL(pwrseq->reset_gpio[i]))
>>> +				gpiod_put(pwrseq->reset_gpio[i]);
>>> +		kfree(pwrseq->reset_gpio);
>>> +	}
>>> +
>>>    	kfree(pwrseq);
>>>    	return ret;
>>>    }
>>>
>>
>> I get a feeling that am just dumping my patch here.. If possible could
>> you have look at it too.
>>
>
> Of course, do you have a link archive since I can't find it on my inbox.
I did send to linux-mmc at vger.kernel.org, Anyway i did forward you the patch.

thanks,
--srini

[PATCH 3/5] mmc: pwrseq: Document optional clock for the simple power sequence

[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>
---
 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 9a77deb164c7..6b5c10d1144c 100644
--- a/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
+++ b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
@@ -11,6 +11,10 @@ Optional properties:
 - 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.
 	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

[PATCH 3/5] mmc: pwrseq: Document optional clock for the simple power sequence

[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>
---
 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 9a77deb164c7..6b5c10d1144c 100644
--- a/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
+++ b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
@@ -11,6 +11,10 @@ Optional properties:
 - 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.
 	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

[PATCH 3/5] mmc: pwrseq: Document optional clock for the simple power sequence

[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>
---
 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 9a77deb164c7..6b5c10d1144c 100644
--- a/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
+++ b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
@@ -11,6 +11,10 @@ Optional properties:
 - 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.
 	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

[PATCH 4/5] mmc: pwrseq_simple: Add optional reference clock support

[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>
---
 drivers/mmc/core/pwrseq_simple.c | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/core/pwrseq_simple.c b/drivers/mmc/core/pwrseq_simple.c
index 9e51fe1051c5..5ef6db7323f3 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>
@@ -16,12 +17,14 @@
 
 #include <linux/mmc/host.h>
 
+#include "core.h"
 #include "pwrseq.h"
 
 struct mmc_pwrseq_simple {
 	struct mmc_pwrseq pwrseq;
 	struct gpio_desc **reset_gpio;
 	int nr_gpios;
+	struct clk *ext_clk;
 };
 
 static void mmc_pwrseq_simple_pre_power_on(struct mmc_host *host)
@@ -30,6 +33,9 @@ static void mmc_pwrseq_simple_pre_power_on(struct mmc_host *host)
 					struct mmc_pwrseq_simple, pwrseq);
 	int i;
 
+	if (pwrseq->ext_clk)
+		clk_prepare_enable(pwrseq->ext_clk);
+
 	for (i = 0; i < pwrseq->nr_gpios; i++)
 		if (!IS_ERR(pwrseq->reset_gpio[i]))
 			gpiod_set_value_cansleep(pwrseq->reset_gpio[i], 1);
@@ -46,12 +52,29 @@ static void mmc_pwrseq_simple_post_power_on(struct mmc_host *host)
 			gpiod_set_value_cansleep(pwrseq->reset_gpio[i], 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);
+	int i;
+
+	for (i = 0; i < pwrseq->nr_gpios; i++)
+		if (!IS_ERR(pwrseq->reset_gpio[i]))
+			gpiod_set_value_cansleep(pwrseq->reset_gpio[i], 1);
+
+	if (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,
 					struct mmc_pwrseq_simple, pwrseq);
 	int i;
 
+	if (pwrseq->ext_clk)
+		clk_put(pwrseq->ext_clk);
+
 	if (pwrseq->nr_gpios > 0) {
 		for (i = 0; i < pwrseq->nr_gpios; i++)
 			if (!IS_ERR(pwrseq->reset_gpio[i]))
@@ -66,7 +89,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,
 };
 
@@ -97,6 +120,14 @@ int mmc_pwrseq_simple_alloc(struct mmc_host *host, struct device *dev)
 		}
 	}
 
+	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;
+	}
+
 	pwrseq->pwrseq.ops = &mmc_pwrseq_simple_ops;
 	host->pwrseq = &pwrseq->pwrseq;
 
-- 
2.1.3

[PATCH 4/5] mmc: pwrseq_simple: Add optional reference clock support

[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>
---
 drivers/mmc/core/pwrseq_simple.c | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/core/pwrseq_simple.c b/drivers/mmc/core/pwrseq_simple.c
index 9e51fe1051c5..5ef6db7323f3 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>
@@ -16,12 +17,14 @@
 
 #include <linux/mmc/host.h>
 
+#include "core.h"
 #include "pwrseq.h"
 
 struct mmc_pwrseq_simple {
 	struct mmc_pwrseq pwrseq;
 	struct gpio_desc **reset_gpio;
 	int nr_gpios;
+	struct clk *ext_clk;
 };
 
 static void mmc_pwrseq_simple_pre_power_on(struct mmc_host *host)
@@ -30,6 +33,9 @@ static void mmc_pwrseq_simple_pre_power_on(struct mmc_host *host)
 					struct mmc_pwrseq_simple, pwrseq);
 	int i;
 
+	if (pwrseq->ext_clk)
+		clk_prepare_enable(pwrseq->ext_clk);
+
 	for (i = 0; i < pwrseq->nr_gpios; i++)
 		if (!IS_ERR(pwrseq->reset_gpio[i]))
 			gpiod_set_value_cansleep(pwrseq->reset_gpio[i], 1);
@@ -46,12 +52,29 @@ static void mmc_pwrseq_simple_post_power_on(struct mmc_host *host)
 			gpiod_set_value_cansleep(pwrseq->reset_gpio[i], 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);
+	int i;
+
+	for (i = 0; i < pwrseq->nr_gpios; i++)
+		if (!IS_ERR(pwrseq->reset_gpio[i]))
+			gpiod_set_value_cansleep(pwrseq->reset_gpio[i], 1);
+
+	if (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,
 					struct mmc_pwrseq_simple, pwrseq);
 	int i;
 
+	if (pwrseq->ext_clk)
+		clk_put(pwrseq->ext_clk);
+
 	if (pwrseq->nr_gpios > 0) {
 		for (i = 0; i < pwrseq->nr_gpios; i++)
 			if (!IS_ERR(pwrseq->reset_gpio[i]))
@@ -66,7 +89,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,
 };
 
@@ -97,6 +120,14 @@ int mmc_pwrseq_simple_alloc(struct mmc_host *host, struct device *dev)
 		}
 	}
 
+	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;
+	}
+
 	pwrseq->pwrseq.ops = &mmc_pwrseq_simple_ops;
 	host->pwrseq = &pwrseq->pwrseq;
 
-- 
2.1.3

[PATCH 4/5] mmc: pwrseq_simple: Add optional reference clock support

[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>
---
 drivers/mmc/core/pwrseq_simple.c | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/core/pwrseq_simple.c b/drivers/mmc/core/pwrseq_simple.c
index 9e51fe1051c5..5ef6db7323f3 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>
@@ -16,12 +17,14 @@
 
 #include <linux/mmc/host.h>
 
+#include "core.h"
 #include "pwrseq.h"
 
 struct mmc_pwrseq_simple {
 	struct mmc_pwrseq pwrseq;
 	struct gpio_desc **reset_gpio;
 	int nr_gpios;
+	struct clk *ext_clk;
 };
 
 static void mmc_pwrseq_simple_pre_power_on(struct mmc_host *host)
@@ -30,6 +33,9 @@ static void mmc_pwrseq_simple_pre_power_on(struct mmc_host *host)
 					struct mmc_pwrseq_simple, pwrseq);
 	int i;
 
+	if (pwrseq->ext_clk)
+		clk_prepare_enable(pwrseq->ext_clk);
+
 	for (i = 0; i < pwrseq->nr_gpios; i++)
 		if (!IS_ERR(pwrseq->reset_gpio[i]))
 			gpiod_set_value_cansleep(pwrseq->reset_gpio[i], 1);
@@ -46,12 +52,29 @@ static void mmc_pwrseq_simple_post_power_on(struct mmc_host *host)
 			gpiod_set_value_cansleep(pwrseq->reset_gpio[i], 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);
+	int i;
+
+	for (i = 0; i < pwrseq->nr_gpios; i++)
+		if (!IS_ERR(pwrseq->reset_gpio[i]))
+			gpiod_set_value_cansleep(pwrseq->reset_gpio[i], 1);
+
+	if (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,
 					struct mmc_pwrseq_simple, pwrseq);
 	int i;
 
+	if (pwrseq->ext_clk)
+		clk_put(pwrseq->ext_clk);
+
 	if (pwrseq->nr_gpios > 0) {
 		for (i = 0; i < pwrseq->nr_gpios; i++)
 			if (!IS_ERR(pwrseq->reset_gpio[i]))
@@ -66,7 +89,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,
 };
 
@@ -97,6 +120,14 @@ int mmc_pwrseq_simple_alloc(struct mmc_host *host, struct device *dev)
 		}
 	}
 
+	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;
+	}
+
 	pwrseq->pwrseq.ops = &mmc_pwrseq_simple_ops;
 	host->pwrseq = &pwrseq->pwrseq;
 
-- 
2.1.3

[PATCH 5/5] ARM: dts: exynos5250-snow: Enable wifi power-on

[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>
---
 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..0f7971ba8238 100644
--- a/arch/arm/boot/dts/exynos5250-snow.dts
+++ b/arch/arm/boot/dts/exynos5250-snow.dts
@@ -229,6 +229,13 @@
 		power-supply = <&fet6>;
 		backlight = <&backlight>;
 	};
+
+	mmc3_pwrseq: mmc3_pwrseq {
+		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 {
@@ -531,17 +538,33 @@
 	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>;
 	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

[PATCH 5/5] ARM: dts: exynos5250-snow: Enable wifi power-on

[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>
---
 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..0f7971ba8238 100644
--- a/arch/arm/boot/dts/exynos5250-snow.dts
+++ b/arch/arm/boot/dts/exynos5250-snow.dts
@@ -229,6 +229,13 @@
 		power-supply = <&fet6>;
 		backlight = <&backlight>;
 	};
+
+	mmc3_pwrseq: mmc3_pwrseq {
+		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 {
@@ -531,17 +538,33 @@
 	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>;
 	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

[PATCH 5/5] ARM: dts: exynos5250-snow: Enable wifi power-on

[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>
---
 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..0f7971ba8238 100644
--- a/arch/arm/boot/dts/exynos5250-snow.dts
+++ b/arch/arm/boot/dts/exynos5250-snow.dts
@@ -229,6 +229,13 @@
 		power-supply = <&fet6>;
 		backlight = <&backlight>;
 	};
+
+	mmc3_pwrseq: mmc3_pwrseq {
+		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 {
@@ -531,17 +538,33 @@
 	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>;
 	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

Re: [PATCH 5/5] ARM: dts: exynos5250-snow: Enable wifi power-on

[Arend van Spriel]

On 01/28/15 11:10, Javier Martinez Canillas wrote:
> 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>
> ---
>   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..0f7971ba8238 100644
> --- a/arch/arm/boot/dts/exynos5250-snow.dts
> +++ b/arch/arm/boot/dts/exynos5250-snow.dts
> @@ -229,6 +229,13 @@
>   		power-supply =<&fet6>;
>   		backlight =<&backlight>;
>   	};
> +
> +	mmc3_pwrseq: mmc3_pwrseq {
> +		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 {
> @@ -531,17 +538,33 @@
>   	status = "okay";
>   	num-slots =<1>;
>   	broken-cd;
> +	cap-sdio-irq;

This seems like an unrelated change, right?

Regards,
Arend

>   	card-detect-delay =<200>;
>   	samsung,dw-mshc-ciu-div =<3>;
>   	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>;

[PATCH 5/5] ARM: dts: exynos5250-snow: Enable wifi power-on

[Arend van Spriel]

On 01/28/15 11:10, Javier Martinez Canillas wrote:
> 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>
> ---
>   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..0f7971ba8238 100644
> --- a/arch/arm/boot/dts/exynos5250-snow.dts
> +++ b/arch/arm/boot/dts/exynos5250-snow.dts
> @@ -229,6 +229,13 @@
>   		power-supply =<&fet6>;
>   		backlight =<&backlight>;
>   	};
> +
> +	mmc3_pwrseq: mmc3_pwrseq {
> +		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 {
> @@ -531,17 +538,33 @@
>   	status = "okay";
>   	num-slots =<1>;
>   	broken-cd;
> +	cap-sdio-irq;

This seems like an unrelated change, right?

Regards,
Arend

>   	card-detect-delay =<200>;
>   	samsung,dw-mshc-ciu-div =<3>;
>   	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>;

Re: [PATCH 5/5] ARM: dts: exynos5250-snow: Enable wifi power-on

[Arend van Spriel]

On 01/28/15 11:10, Javier Martinez Canillas wrote:
> 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>
> ---
>   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..0f7971ba8238 100644
> --- a/arch/arm/boot/dts/exynos5250-snow.dts
> +++ b/arch/arm/boot/dts/exynos5250-snow.dts
> @@ -229,6 +229,13 @@
>   		power-supply =<&fet6>;
>   		backlight =<&backlight>;
>   	};
> +
> +	mmc3_pwrseq: mmc3_pwrseq {
> +		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 {
> @@ -531,17 +538,33 @@
>   	status = "okay";
>   	num-slots =<1>;
>   	broken-cd;
> +	cap-sdio-irq;

This seems like an unrelated change, right?

Regards,
Arend

>   	card-detect-delay =<200>;
>   	samsung,dw-mshc-ciu-div =<3>;
>   	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>;

Re: [PATCH 5/5] ARM: dts: exynos5250-snow: Enable wifi power-on

[Javier Martinez Canillas]

Hello Arend,

Thanks for your feedback.

On 01/28/2015 03:03 PM, Arend van Spriel wrote:
> On 01/28/15 11:10, Javier Martinez Canillas wrote:

>>   &dp {
>> @@ -531,17 +538,33 @@
>>   	status = "okay";
>>   	num-slots =<1>;
>>   	broken-cd;
>> +	cap-sdio-irq;
> 
> This seems like an unrelated change, right?
> 

Well, I found that enabling the SDIO IRQ signalling I had the double
of the transfer rate and I didn't think it justified a separate patch
but you are right and I'll split on v2.

> Regards,
> Arend
> 

Best regards,
Javier

[PATCH 5/5] ARM: dts: exynos5250-snow: Enable wifi power-on

[Javier Martinez Canillas]

Hello Arend,

Thanks for your feedback.

On 01/28/2015 03:03 PM, Arend van Spriel wrote:
> On 01/28/15 11:10, Javier Martinez Canillas wrote:

>>   &dp {
>> @@ -531,17 +538,33 @@
>>   	status = "okay";
>>   	num-slots =<1>;
>>   	broken-cd;
>> +	cap-sdio-irq;
> 
> This seems like an unrelated change, right?
> 

Well, I found that enabling the SDIO IRQ signalling I had the double
of the transfer rate and I didn't think it justified a separate patch
but you are right and I'll split on v2.

> Regards,
> Arend
> 

Best regards,
Javier

Re: [PATCH 5/5] ARM: dts: exynos5250-snow: Enable wifi power-on

[Javier Martinez Canillas]

Hello Arend,

Thanks for your feedback.

On 01/28/2015 03:03 PM, Arend van Spriel wrote:
> On 01/28/15 11:10, Javier Martinez Canillas wrote:

>>   &dp {
>> @@ -531,17 +538,33 @@
>>   	status = "okay";
>>   	num-slots =<1>;
>>   	broken-cd;
>> +	cap-sdio-irq;
> 
> This seems like an unrelated change, right?
> 

Well, I found that enabling the SDIO IRQ signalling I had the double
of the transfer rate and I didn't think it justified a separate patch
but you are right and I'll split on v2.

> Regards,
> Arend
> 

Best regards,
Javier