All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/4] mmc: slot-gpio: switch to use flags when getting GPIO
@ 2014-08-27 11:00 Linus Walleij
  2014-08-27 11:00 ` [PATCH 2/4] mmc: slot-gpio: add gpiod variant to get wp GPIO Linus Walleij
                   ` (3 more replies)
  0 siblings, 4 replies; 18+ messages in thread
From: Linus Walleij @ 2014-08-27 11:00 UTC (permalink / raw)
  To: linux-mmc, Chris Ball, Ulf Hansson; +Cc: linux-gpio, Linus Walleij

When the slot GPIO driver gets the GPIO to be used for card
detect, it is now possible to specify a flag to have the line
set up as input. Get rid of the explicit setup call for input
and use the flag.

The extra argument works as there are transition varargs
macros in place in the <linux/gpio/consumer.h> header, in
the future we will make the flags argument compulsory.

Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/mmc/core/slot-gpio.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c
index 5f89cb83d5f0..908c2b29e79f 100644
--- a/drivers/mmc/core/slot-gpio.c
+++ b/drivers/mmc/core/slot-gpio.c
@@ -308,14 +308,10 @@ int mmc_gpiod_request_cd(struct mmc_host *host, const char *con_id,
 	if (!con_id)
 		con_id = ctx->cd_label;
 
-	desc = devm_gpiod_get_index(host->parent, con_id, idx);
+	desc = devm_gpiod_get_index(host->parent, con_id, idx, GPIOD_IN);
 	if (IS_ERR(desc))
 		return PTR_ERR(desc);
 
-	ret = gpiod_direction_input(desc);
-	if (ret < 0)
-		return ret;
-
 	if (debounce) {
 		ret = gpiod_set_debounce(desc, debounce);
 		if (ret < 0)
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/4] mmc: slot-gpio: add gpiod variant to get wp GPIO
  2014-08-27 11:00 [PATCH 1/4] mmc: slot-gpio: switch to use flags when getting GPIO Linus Walleij
@ 2014-08-27 11:00 ` Linus Walleij
  2014-08-29 12:16   ` Ulf Hansson
  2014-08-27 11:00 ` [PATCH 3/4 v2] mmc: host: switch OF parser to use gpio descriptors Linus Walleij
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 18+ messages in thread
From: Linus Walleij @ 2014-08-27 11:00 UTC (permalink / raw)
  To: linux-mmc, Chris Ball, Ulf Hansson; +Cc: linux-gpio, Linus Walleij

This makes it possible to get the write protect (read only)
GPIO line from a GPIO descriptor. Written to exactly mirror
the card detect function.

Acked-by: Alexandre Courbot <acourbot@nvidia.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/mmc/core/slot-gpio.c  | 48 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/mmc/slot-gpio.h |  3 +++
 2 files changed, 51 insertions(+)

diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c
index 908c2b29e79f..e3fce4493fab 100644
--- a/drivers/mmc/core/slot-gpio.c
+++ b/drivers/mmc/core/slot-gpio.c
@@ -326,6 +326,54 @@ int mmc_gpiod_request_cd(struct mmc_host *host, const char *con_id,
 EXPORT_SYMBOL(mmc_gpiod_request_cd);
 
 /**
+ * mmc_gpiod_request_ro - request a gpio descriptor for write protection
+ * @host: mmc host
+ * @con_id: function within the GPIO consumer
+ * @idx: index of the GPIO to obtain in the consumer
+ * @override_active_level: ignore %GPIO_ACTIVE_LOW flag
+ * @debounce: debounce time in microseconds
+ *
+ * Use this function in place of mmc_gpio_request_ro() to use the GPIO
+ * descriptor API.  Note that it is paired with mmc_gpiod_free_ro() not
+ * mmc_gpio_free_ro().
+ *
+ * Returns zero on success, else an error.
+ */
+int mmc_gpiod_request_ro(struct mmc_host *host, const char *con_id,
+			 unsigned int idx, bool override_active_level,
+			 unsigned int debounce)
+{
+	struct mmc_gpio *ctx;
+	struct gpio_desc *desc;
+	int ret;
+
+	ret = mmc_gpio_alloc(host);
+	if (ret < 0)
+		return ret;
+
+	ctx = host->slot.handler_priv;
+
+	if (!con_id)
+		con_id = ctx->ro_label;
+
+	desc = devm_gpiod_get_index(host->parent, con_id, idx, GPIOD_IN);
+	if (IS_ERR(desc))
+		return PTR_ERR(desc);
+
+	if (debounce) {
+		ret = gpiod_set_debounce(desc, debounce);
+		if (ret < 0)
+			return ret;
+	}
+
+	ctx->override_ro_active_level = override_active_level;
+	ctx->ro_gpio = desc;
+
+	return 0;
+}
+EXPORT_SYMBOL(mmc_gpiod_request_ro);
+
+/**
  * mmc_gpiod_free_cd - free the card-detection gpio descriptor
  * @host: mmc host
  *
diff --git a/include/linux/mmc/slot-gpio.h b/include/linux/mmc/slot-gpio.h
index d2433381e828..a0d0442c15bf 100644
--- a/include/linux/mmc/slot-gpio.h
+++ b/include/linux/mmc/slot-gpio.h
@@ -25,6 +25,9 @@ void mmc_gpio_free_cd(struct mmc_host *host);
 int mmc_gpiod_request_cd(struct mmc_host *host, const char *con_id,
 			 unsigned int idx, bool override_active_level,
 			 unsigned int debounce);
+int mmc_gpiod_request_ro(struct mmc_host *host, const char *con_id,
+			 unsigned int idx, bool override_active_level,
+			 unsigned int debounce);
 void mmc_gpiod_free_cd(struct mmc_host *host);
 void mmc_gpiod_request_cd_irq(struct mmc_host *host);
 
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 3/4 v2] mmc: host: switch OF parser to use gpio descriptors
  2014-08-27 11:00 [PATCH 1/4] mmc: slot-gpio: switch to use flags when getting GPIO Linus Walleij
  2014-08-27 11:00 ` [PATCH 2/4] mmc: slot-gpio: add gpiod variant to get wp GPIO Linus Walleij
@ 2014-08-27 11:00 ` Linus Walleij
  2014-08-29 12:16   ` Ulf Hansson
                     ` (2 more replies)
  2014-08-27 11:00 ` [PATCH 4/4] mmc: mmci: augment driver to handle " Linus Walleij
  2014-08-29 12:16 ` [PATCH 1/4] mmc: slot-gpio: switch to use flags when getting GPIO Ulf Hansson
  3 siblings, 3 replies; 18+ messages in thread
From: Linus Walleij @ 2014-08-27 11:00 UTC (permalink / raw)
  To: linux-mmc, Chris Ball, Ulf Hansson; +Cc: linux-gpio, Linus Walleij

This switches the central MMC OF parser to use gpio descriptors
instead of grabbing GPIOs explicitly from the device tree.
This strips out an unecessary use of the integer-based GPIO
API that we want to get rid of, cuts down on code as the
gpio descriptor code will handle active low flags.

Acked-by: Alexandre Courbot <acourbot@nvidia.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Restore error reporting as done in previous stand-alone
  patch.
---
 drivers/mmc/core/host.c | 68 +++++++++++++++++--------------------------------
 1 file changed, 23 insertions(+), 45 deletions(-)

diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 95cceae96944..6f7ed9c50346 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -310,9 +310,7 @@ int mmc_of_parse(struct mmc_host *host)
 {
 	struct device_node *np;
 	u32 bus_width;
-	bool explicit_inv_wp, gpio_inv_wp = false;
-	enum of_gpio_flags flags;
-	int len, ret, gpio;
+	int len, ret;
 
 	if (!host->parent || !host->parent->of_node)
 		return 0;
@@ -360,60 +358,40 @@ int mmc_of_parse(struct mmc_host *host)
 	if (of_find_property(np, "non-removable", &len)) {
 		host->caps |= MMC_CAP_NONREMOVABLE;
 	} else {
-		bool explicit_inv_cd, gpio_inv_cd = false;
-
-		explicit_inv_cd = of_property_read_bool(np, "cd-inverted");
+		if (of_property_read_bool(np, "cd-inverted"))
+			host->caps2 |= MMC_CAP2_CD_ACTIVE_HIGH;
 
 		if (of_find_property(np, "broken-cd", &len))
 			host->caps |= MMC_CAP_NEEDS_POLL;
 
-		gpio = of_get_named_gpio_flags(np, "cd-gpios", 0, &flags);
-		if (gpio == -EPROBE_DEFER)
-			return gpio;
-		if (gpio_is_valid(gpio)) {
-			if (!(flags & OF_GPIO_ACTIVE_LOW))
-				gpio_inv_cd = true;
-
-			ret = mmc_gpio_request_cd(host, gpio, 0);
-			if (ret < 0) {
-				dev_err(host->parent,
-					"Failed to request CD GPIO #%d: %d!\n",
-					gpio, ret);
+		ret = mmc_gpiod_request_cd(host, "cd", 0, false, 0);
+		if (ret) {
+			if (ret == -EPROBE_DEFER)
 				return ret;
-			} else {
-				dev_info(host->parent, "Got CD GPIO #%d.\n",
-					 gpio);
+			if (ret != -ENOENT) {
+				dev_err(host->parent,
+					"Failed to request CD GPIO: %d\n",
+					ret);
 			}
-		}
-
-		if (explicit_inv_cd ^ gpio_inv_cd)
-			host->caps2 |= MMC_CAP2_CD_ACTIVE_HIGH;
+		} else
+			dev_info(host->parent, "Got CD GPIO\n");
 	}
 
 	/* Parse Write Protection */
-	explicit_inv_wp = of_property_read_bool(np, "wp-inverted");
-
-	gpio = of_get_named_gpio_flags(np, "wp-gpios", 0, &flags);
-	if (gpio == -EPROBE_DEFER) {
-		ret = -EPROBE_DEFER;
-		goto out;
-	}
-	if (gpio_is_valid(gpio)) {
-		if (!(flags & OF_GPIO_ACTIVE_LOW))
-			gpio_inv_wp = true;
+	if (of_property_read_bool(np, "wp-inverted"))
+		host->caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
 
-		ret = mmc_gpio_request_ro(host, gpio);
-		if (ret < 0) {
-			dev_err(host->parent,
-				"Failed to request WP GPIO: %d!\n", ret);
+	ret = mmc_gpiod_request_ro(host, "wp", 0, false, 0);
+	if (ret) {
+		if (ret == -EPROBE_DEFER)
 			goto out;
-		} else {
-				dev_info(host->parent, "Got WP GPIO #%d.\n",
-					 gpio);
+		if (ret != -ENOENT) {
+			dev_err(host->parent,
+				"Failed to request WP GPIO: %d\n",
+				ret);
 		}
-	}
-	if (explicit_inv_wp ^ gpio_inv_wp)
-		host->caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
+	} else
+		dev_info(host->parent, "Got WP GPIO\n");
 
 	if (of_find_property(np, "cap-sd-highspeed", &len))
 		host->caps |= MMC_CAP_SD_HIGHSPEED;
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 4/4] mmc: mmci: augment driver to handle gpio descriptors
  2014-08-27 11:00 [PATCH 1/4] mmc: slot-gpio: switch to use flags when getting GPIO Linus Walleij
  2014-08-27 11:00 ` [PATCH 2/4] mmc: slot-gpio: add gpiod variant to get wp GPIO Linus Walleij
  2014-08-27 11:00 ` [PATCH 3/4 v2] mmc: host: switch OF parser to use gpio descriptors Linus Walleij
@ 2014-08-27 11:00 ` Linus Walleij
  2014-08-27 11:34   ` Ulf Hansson
  2014-08-29 12:16 ` [PATCH 1/4] mmc: slot-gpio: switch to use flags when getting GPIO Ulf Hansson
  3 siblings, 1 reply; 18+ messages in thread
From: Linus Walleij @ 2014-08-27 11:00 UTC (permalink / raw)
  To: linux-mmc, Chris Ball, Ulf Hansson
  Cc: linux-gpio, Linus Walleij, Alexandre Courbot, Russell King

Currently the MMCI driver will only handle GPIO descriptors
implicitly through the device tree probe glue in mmc_of_init(),
but devices instatiated other ways such as through board files
and passing descriptors using the GPIO descriptor table will
not be able to exploit descriptors.

Augment the driver to look for a GPIO descriptor if device
tree is not used for the device, and if that doesn't work,
fall back to platform data GPIO assignment using the old
API. The end goal is to get rid of the platform data integer
GPIO assingments from the kernel.

This enable the MMCI-embedding platforms to be converted to
GPIO descritor tables.

Cc: Alexandre Courbot <gnurou@gmail.com>
Cc: Russell King <linux@arm.linux.org.uk>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/mmc/host/mmci.c | 53 +++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 43 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index e4d470704150..37a6542f056c 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -1658,16 +1658,49 @@ static int mmci_probe(struct amba_device *dev,
 	writel(0, host->base + MMCIMASK1);
 	writel(0xfff, host->base + MMCICLEAR);
 
-	/* If DT, cd/wp gpios must be supplied through it. */
-	if (!np && gpio_is_valid(plat->gpio_cd)) {
-		ret = mmc_gpio_request_cd(mmc, plat->gpio_cd, 0);
-		if (ret)
-			goto clk_disable;
-	}
-	if (!np && gpio_is_valid(plat->gpio_wp)) {
-		ret = mmc_gpio_request_ro(mmc, plat->gpio_wp);
-		if (ret)
-			goto clk_disable;
+	/*
+	 * If:
+	 * - not using DT but using a descriptor table, or
+	 * - using a table of descriptors ALONGSIDE DT, or
+	 * look up these descriptors named "cd" and "wp" right here, fail
+	 * silently of these do not exist and proceed to try platform data
+	 */
+	if (!np) {
+		ret = mmc_gpiod_request_cd(mmc, "cd", 0, false, 0);
+		if (!ret)
+			dev_info(mmc_dev(mmc), "got CD GPIO (table)\n");
+		else {
+			if (ret == -EPROBE_DEFER)
+				goto clk_disable;
+			else if (gpio_is_valid(plat->gpio_cd)) {
+				ret = mmc_gpio_request_cd(mmc, plat->gpio_cd, 0);
+				if (ret)
+					goto clk_disable;
+				else
+					dev_info(mmc_dev(mmc), "got CD GPIO (pdata)\n");
+			} else {
+				dev_dbg(mmc_dev(mmc),
+					"no CD GPIO in DT, table or pdata\n");
+			}
+		}
+
+		ret = mmc_gpiod_request_ro(mmc, "wp", 0, false, 0);
+		if (!ret)
+			dev_info(mmc_dev(mmc), "got WP GPIO (table)\n");
+		else {
+			if (ret == -EPROBE_DEFER)
+				goto clk_disable;
+			else if (gpio_is_valid(plat->gpio_wp)) {
+				ret = mmc_gpio_request_ro(mmc, plat->gpio_wp);
+				if (ret)
+					goto clk_disable;
+				else
+					dev_info(mmc_dev(mmc), "got WP GPIO (pdata)\n");
+			} else {
+				dev_dbg(mmc_dev(mmc),
+					"no WP GPIO in DT, table or pdata\n");
+			}
+		}
 	}
 
 	ret = devm_request_irq(&dev->dev, dev->irq[0], mmci_irq, IRQF_SHARED,
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/4] mmc: mmci: augment driver to handle gpio descriptors
  2014-08-27 11:00 ` [PATCH 4/4] mmc: mmci: augment driver to handle " Linus Walleij
@ 2014-08-27 11:34   ` Ulf Hansson
  0 siblings, 0 replies; 18+ messages in thread
From: Ulf Hansson @ 2014-08-27 11:34 UTC (permalink / raw)
  To: Linus Walleij
  Cc: linux-mmc, Chris Ball, linux-gpio, Alexandre Courbot, Russell King

On 27 August 2014 13:00, Linus Walleij <linus.walleij@linaro.org> wrote:
> Currently the MMCI driver will only handle GPIO descriptors
> implicitly through the device tree probe glue in mmc_of_init(),
> but devices instatiated other ways such as through board files
> and passing descriptors using the GPIO descriptor table will
> not be able to exploit descriptors.
>
> Augment the driver to look for a GPIO descriptor if device
> tree is not used for the device, and if that doesn't work,
> fall back to platform data GPIO assignment using the old
> API. The end goal is to get rid of the platform data integer
> GPIO assingments from the kernel.
>
> This enable the MMCI-embedding platforms to be converted to
> GPIO descritor tables.
>
> Cc: Alexandre Courbot <gnurou@gmail.com>
> Cc: Russell King <linux@arm.linux.org.uk>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/mmc/host/mmci.c | 53 +++++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 43 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index e4d470704150..37a6542f056c 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -1658,16 +1658,49 @@ static int mmci_probe(struct amba_device *dev,
>         writel(0, host->base + MMCIMASK1);
>         writel(0xfff, host->base + MMCICLEAR);
>
> -       /* If DT, cd/wp gpios must be supplied through it. */
> -       if (!np && gpio_is_valid(plat->gpio_cd)) {
> -               ret = mmc_gpio_request_cd(mmc, plat->gpio_cd, 0);
> -               if (ret)
> -                       goto clk_disable;
> -       }
> -       if (!np && gpio_is_valid(plat->gpio_wp)) {
> -               ret = mmc_gpio_request_ro(mmc, plat->gpio_wp);
> -               if (ret)
> -                       goto clk_disable;
> +       /*
> +        * If:
> +        * - not using DT but using a descriptor table, or
> +        * - using a table of descriptors ALONGSIDE DT, or
> +        * look up these descriptors named "cd" and "wp" right here, fail
> +        * silently of these do not exist and proceed to try platform data
> +        */
> +       if (!np) {
> +               ret = mmc_gpiod_request_cd(mmc, "cd", 0, false, 0);
> +               if (!ret)
> +                       dev_info(mmc_dev(mmc), "got CD GPIO (table)\n");
> +               else {
> +                       if (ret == -EPROBE_DEFER)
> +                               goto clk_disable;
> +                       else if (gpio_is_valid(plat->gpio_cd)) {
> +                               ret = mmc_gpio_request_cd(mmc, plat->gpio_cd, 0);
> +                               if (ret)
> +                                       goto clk_disable;
> +                               else
> +                                       dev_info(mmc_dev(mmc), "got CD GPIO (pdata)\n");
> +                       } else {
> +                               dev_dbg(mmc_dev(mmc),
> +                                       "no CD GPIO in DT, table or pdata\n");
> +                       }
> +               }
> +
> +               ret = mmc_gpiod_request_ro(mmc, "wp", 0, false, 0);
> +               if (!ret)
> +                       dev_info(mmc_dev(mmc), "got WP GPIO (table)\n");
> +               else {
> +                       if (ret == -EPROBE_DEFER)
> +                               goto clk_disable;
> +                       else if (gpio_is_valid(plat->gpio_wp)) {
> +                               ret = mmc_gpio_request_ro(mmc, plat->gpio_wp);
> +                               if (ret)
> +                                       goto clk_disable;
> +                               else
> +                                       dev_info(mmc_dev(mmc), "got WP GPIO (pdata)\n");
> +                       } else {
> +                               dev_dbg(mmc_dev(mmc),
> +                                       "no WP GPIO in DT, table or pdata\n");
> +                       }

Just a nitpick; and unless it's important to you, could you remove all
the dev_info|dbg - we didn't need them before, why now?

Kind regards
Uffe

> +               }
>         }
>
>         ret = devm_request_irq(&dev->dev, dev->irq[0], mmci_irq, IRQF_SHARED,
> --
> 1.9.3
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/4] mmc: slot-gpio: switch to use flags when getting GPIO
  2014-08-27 11:00 [PATCH 1/4] mmc: slot-gpio: switch to use flags when getting GPIO Linus Walleij
                   ` (2 preceding siblings ...)
  2014-08-27 11:00 ` [PATCH 4/4] mmc: mmci: augment driver to handle " Linus Walleij
@ 2014-08-29 12:16 ` Ulf Hansson
  2014-09-22  8:20   ` Adrian Hunter
  3 siblings, 1 reply; 18+ messages in thread
From: Ulf Hansson @ 2014-08-29 12:16 UTC (permalink / raw)
  To: Linus Walleij; +Cc: linux-mmc, Chris Ball, linux-gpio

On 27 August 2014 13:00, Linus Walleij <linus.walleij@linaro.org> wrote:
> When the slot GPIO driver gets the GPIO to be used for card
> detect, it is now possible to specify a flag to have the line
> set up as input. Get rid of the explicit setup call for input
> and use the flag.
>
> The extra argument works as there are transition varargs
> macros in place in the <linux/gpio/consumer.h> header, in
> the future we will make the flags argument compulsory.
>
> Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

Thanks! Applied for next.

Kind regards
Uffe

> ---
>  drivers/mmc/core/slot-gpio.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c
> index 5f89cb83d5f0..908c2b29e79f 100644
> --- a/drivers/mmc/core/slot-gpio.c
> +++ b/drivers/mmc/core/slot-gpio.c
> @@ -308,14 +308,10 @@ int mmc_gpiod_request_cd(struct mmc_host *host, const char *con_id,
>         if (!con_id)
>                 con_id = ctx->cd_label;
>
> -       desc = devm_gpiod_get_index(host->parent, con_id, idx);
> +       desc = devm_gpiod_get_index(host->parent, con_id, idx, GPIOD_IN);
>         if (IS_ERR(desc))
>                 return PTR_ERR(desc);
>
> -       ret = gpiod_direction_input(desc);
> -       if (ret < 0)
> -               return ret;
> -
>         if (debounce) {
>                 ret = gpiod_set_debounce(desc, debounce);
>                 if (ret < 0)
> --
> 1.9.3
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/4] mmc: slot-gpio: add gpiod variant to get wp GPIO
  2014-08-27 11:00 ` [PATCH 2/4] mmc: slot-gpio: add gpiod variant to get wp GPIO Linus Walleij
@ 2014-08-29 12:16   ` Ulf Hansson
  0 siblings, 0 replies; 18+ messages in thread
From: Ulf Hansson @ 2014-08-29 12:16 UTC (permalink / raw)
  To: Linus Walleij; +Cc: linux-mmc, Chris Ball, linux-gpio

On 27 August 2014 13:00, Linus Walleij <linus.walleij@linaro.org> wrote:
> This makes it possible to get the write protect (read only)
> GPIO line from a GPIO descriptor. Written to exactly mirror
> the card detect function.
>
> Acked-by: Alexandre Courbot <acourbot@nvidia.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

Thanks! Applied for next.

Kind regards
Uffe


> ---
>  drivers/mmc/core/slot-gpio.c  | 48 +++++++++++++++++++++++++++++++++++++++++++
>  include/linux/mmc/slot-gpio.h |  3 +++
>  2 files changed, 51 insertions(+)
>
> diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c
> index 908c2b29e79f..e3fce4493fab 100644
> --- a/drivers/mmc/core/slot-gpio.c
> +++ b/drivers/mmc/core/slot-gpio.c
> @@ -326,6 +326,54 @@ int mmc_gpiod_request_cd(struct mmc_host *host, const char *con_id,
>  EXPORT_SYMBOL(mmc_gpiod_request_cd);
>
>  /**
> + * mmc_gpiod_request_ro - request a gpio descriptor for write protection
> + * @host: mmc host
> + * @con_id: function within the GPIO consumer
> + * @idx: index of the GPIO to obtain in the consumer
> + * @override_active_level: ignore %GPIO_ACTIVE_LOW flag
> + * @debounce: debounce time in microseconds
> + *
> + * Use this function in place of mmc_gpio_request_ro() to use the GPIO
> + * descriptor API.  Note that it is paired with mmc_gpiod_free_ro() not
> + * mmc_gpio_free_ro().
> + *
> + * Returns zero on success, else an error.
> + */
> +int mmc_gpiod_request_ro(struct mmc_host *host, const char *con_id,
> +                        unsigned int idx, bool override_active_level,
> +                        unsigned int debounce)
> +{
> +       struct mmc_gpio *ctx;
> +       struct gpio_desc *desc;
> +       int ret;
> +
> +       ret = mmc_gpio_alloc(host);
> +       if (ret < 0)
> +               return ret;
> +
> +       ctx = host->slot.handler_priv;
> +
> +       if (!con_id)
> +               con_id = ctx->ro_label;
> +
> +       desc = devm_gpiod_get_index(host->parent, con_id, idx, GPIOD_IN);
> +       if (IS_ERR(desc))
> +               return PTR_ERR(desc);
> +
> +       if (debounce) {
> +               ret = gpiod_set_debounce(desc, debounce);
> +               if (ret < 0)
> +                       return ret;
> +       }
> +
> +       ctx->override_ro_active_level = override_active_level;
> +       ctx->ro_gpio = desc;
> +
> +       return 0;
> +}
> +EXPORT_SYMBOL(mmc_gpiod_request_ro);
> +
> +/**
>   * mmc_gpiod_free_cd - free the card-detection gpio descriptor
>   * @host: mmc host
>   *
> diff --git a/include/linux/mmc/slot-gpio.h b/include/linux/mmc/slot-gpio.h
> index d2433381e828..a0d0442c15bf 100644
> --- a/include/linux/mmc/slot-gpio.h
> +++ b/include/linux/mmc/slot-gpio.h
> @@ -25,6 +25,9 @@ void mmc_gpio_free_cd(struct mmc_host *host);
>  int mmc_gpiod_request_cd(struct mmc_host *host, const char *con_id,
>                          unsigned int idx, bool override_active_level,
>                          unsigned int debounce);
> +int mmc_gpiod_request_ro(struct mmc_host *host, const char *con_id,
> +                        unsigned int idx, bool override_active_level,
> +                        unsigned int debounce);
>  void mmc_gpiod_free_cd(struct mmc_host *host);
>  void mmc_gpiod_request_cd_irq(struct mmc_host *host);
>
> --
> 1.9.3
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/4 v2] mmc: host: switch OF parser to use gpio descriptors
  2014-08-27 11:00 ` [PATCH 3/4 v2] mmc: host: switch OF parser to use gpio descriptors Linus Walleij
@ 2014-08-29 12:16   ` Ulf Hansson
  2014-09-09  7:05   ` Linus Walleij
  2014-09-30 11:30   ` Javier Martinez Canillas
  2 siblings, 0 replies; 18+ messages in thread
From: Ulf Hansson @ 2014-08-29 12:16 UTC (permalink / raw)
  To: Linus Walleij; +Cc: linux-mmc, Chris Ball, linux-gpio

On 27 August 2014 13:00, Linus Walleij <linus.walleij@linaro.org> wrote:
> This switches the central MMC OF parser to use gpio descriptors
> instead of grabbing GPIOs explicitly from the device tree.
> This strips out an unecessary use of the integer-based GPIO
> API that we want to get rid of, cuts down on code as the
> gpio descriptor code will handle active low flags.
>
> Acked-by: Alexandre Courbot <acourbot@nvidia.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

Thanks! Applied for next.

Kind regards
Uffe


> ---
> ChangeLog v1->v2:
> - Restore error reporting as done in previous stand-alone
>   patch.
> ---
>  drivers/mmc/core/host.c | 68 +++++++++++++++++--------------------------------
>  1 file changed, 23 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> index 95cceae96944..6f7ed9c50346 100644
> --- a/drivers/mmc/core/host.c
> +++ b/drivers/mmc/core/host.c
> @@ -310,9 +310,7 @@ int mmc_of_parse(struct mmc_host *host)
>  {
>         struct device_node *np;
>         u32 bus_width;
> -       bool explicit_inv_wp, gpio_inv_wp = false;
> -       enum of_gpio_flags flags;
> -       int len, ret, gpio;
> +       int len, ret;
>
>         if (!host->parent || !host->parent->of_node)
>                 return 0;
> @@ -360,60 +358,40 @@ int mmc_of_parse(struct mmc_host *host)
>         if (of_find_property(np, "non-removable", &len)) {
>                 host->caps |= MMC_CAP_NONREMOVABLE;
>         } else {
> -               bool explicit_inv_cd, gpio_inv_cd = false;
> -
> -               explicit_inv_cd = of_property_read_bool(np, "cd-inverted");
> +               if (of_property_read_bool(np, "cd-inverted"))
> +                       host->caps2 |= MMC_CAP2_CD_ACTIVE_HIGH;
>
>                 if (of_find_property(np, "broken-cd", &len))
>                         host->caps |= MMC_CAP_NEEDS_POLL;
>
> -               gpio = of_get_named_gpio_flags(np, "cd-gpios", 0, &flags);
> -               if (gpio == -EPROBE_DEFER)
> -                       return gpio;
> -               if (gpio_is_valid(gpio)) {
> -                       if (!(flags & OF_GPIO_ACTIVE_LOW))
> -                               gpio_inv_cd = true;
> -
> -                       ret = mmc_gpio_request_cd(host, gpio, 0);
> -                       if (ret < 0) {
> -                               dev_err(host->parent,
> -                                       "Failed to request CD GPIO #%d: %d!\n",
> -                                       gpio, ret);
> +               ret = mmc_gpiod_request_cd(host, "cd", 0, false, 0);
> +               if (ret) {
> +                       if (ret == -EPROBE_DEFER)
>                                 return ret;
> -                       } else {
> -                               dev_info(host->parent, "Got CD GPIO #%d.\n",
> -                                        gpio);
> +                       if (ret != -ENOENT) {
> +                               dev_err(host->parent,
> +                                       "Failed to request CD GPIO: %d\n",
> +                                       ret);
>                         }
> -               }
> -
> -               if (explicit_inv_cd ^ gpio_inv_cd)
> -                       host->caps2 |= MMC_CAP2_CD_ACTIVE_HIGH;
> +               } else
> +                       dev_info(host->parent, "Got CD GPIO\n");
>         }
>
>         /* Parse Write Protection */
> -       explicit_inv_wp = of_property_read_bool(np, "wp-inverted");
> -
> -       gpio = of_get_named_gpio_flags(np, "wp-gpios", 0, &flags);
> -       if (gpio == -EPROBE_DEFER) {
> -               ret = -EPROBE_DEFER;
> -               goto out;
> -       }
> -       if (gpio_is_valid(gpio)) {
> -               if (!(flags & OF_GPIO_ACTIVE_LOW))
> -                       gpio_inv_wp = true;
> +       if (of_property_read_bool(np, "wp-inverted"))
> +               host->caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
>
> -               ret = mmc_gpio_request_ro(host, gpio);
> -               if (ret < 0) {
> -                       dev_err(host->parent,
> -                               "Failed to request WP GPIO: %d!\n", ret);
> +       ret = mmc_gpiod_request_ro(host, "wp", 0, false, 0);
> +       if (ret) {
> +               if (ret == -EPROBE_DEFER)
>                         goto out;
> -               } else {
> -                               dev_info(host->parent, "Got WP GPIO #%d.\n",
> -                                        gpio);
> +               if (ret != -ENOENT) {
> +                       dev_err(host->parent,
> +                               "Failed to request WP GPIO: %d\n",
> +                               ret);
>                 }
> -       }
> -       if (explicit_inv_wp ^ gpio_inv_wp)
> -               host->caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
> +       } else
> +               dev_info(host->parent, "Got WP GPIO\n");
>
>         if (of_find_property(np, "cap-sd-highspeed", &len))
>                 host->caps |= MMC_CAP_SD_HIGHSPEED;
> --
> 1.9.3
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/4 v2] mmc: host: switch OF parser to use gpio descriptors
  2014-08-27 11:00 ` [PATCH 3/4 v2] mmc: host: switch OF parser to use gpio descriptors Linus Walleij
  2014-08-29 12:16   ` Ulf Hansson
@ 2014-09-09  7:05   ` Linus Walleij
  2014-09-09 12:29     ` Ulf Hansson
  2014-09-30 11:30   ` Javier Martinez Canillas
  2 siblings, 1 reply; 18+ messages in thread
From: Linus Walleij @ 2014-09-09  7:05 UTC (permalink / raw)
  To: linux-mmc, Chris Ball, Ulf Hansson; +Cc: linux-gpio, Linus Walleij

On Wed, Aug 27, 2014 at 1:00 PM, Linus Walleij <linus.walleij@linaro.org> wrote:

> This switches the central MMC OF parser to use gpio descriptors
> instead of grabbing GPIOs explicitly from the device tree.
> This strips out an unecessary use of the integer-based GPIO
> API that we want to get rid of, cuts down on code as the
> gpio descriptor code will handle active low flags.
>
> Acked-by: Alexandre Courbot <acourbot@nvidia.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v1->v2:
> - Restore error reporting as done in previous stand-alone
>   patch.

Hi Ulf,

the v2 version of this patch set should be working fine on
v3.17-rc4, as I merged a patch setting up the correct
prototypes for systems without gpiolib.

Fenguang's build tests are working on it atleast, I promise...

Tell me if you want me to resend the patch set or if you can
take the same patches again, but on top of -rc4. There is
no change in them.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/4 v2] mmc: host: switch OF parser to use gpio descriptors
  2014-09-09  7:05   ` Linus Walleij
@ 2014-09-09 12:29     ` Ulf Hansson
  0 siblings, 0 replies; 18+ messages in thread
From: Ulf Hansson @ 2014-09-09 12:29 UTC (permalink / raw)
  To: Linus Walleij; +Cc: linux-mmc, Chris Ball, linux-gpio

On 9 September 2014 09:05, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Wed, Aug 27, 2014 at 1:00 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
>
>> This switches the central MMC OF parser to use gpio descriptors
>> instead of grabbing GPIOs explicitly from the device tree.
>> This strips out an unecessary use of the integer-based GPIO
>> API that we want to get rid of, cuts down on code as the
>> gpio descriptor code will handle active low flags.
>>
>> Acked-by: Alexandre Courbot <acourbot@nvidia.com>
>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>> ---
>> ChangeLog v1->v2:
>> - Restore error reporting as done in previous stand-alone
>>   patch.
>
> Hi Ulf,
>
> the v2 version of this patch set should be working fine on
> v3.17-rc4, as I merged a patch setting up the correct
> prototypes for systems without gpiolib.
>
> Fenguang's build tests are working on it atleast, I promise...
>
> Tell me if you want me to resend the patch set or if you can
> take the same patches again, but on top of -rc4. There is
> no change in them.
>

Hi Linus,

I have re-applied them again, let's hope for better luck this time. :-)

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/4] mmc: slot-gpio: switch to use flags when getting GPIO
  2014-08-29 12:16 ` [PATCH 1/4] mmc: slot-gpio: switch to use flags when getting GPIO Ulf Hansson
@ 2014-09-22  8:20   ` Adrian Hunter
  2014-09-22 12:37     ` Linus Walleij
  0 siblings, 1 reply; 18+ messages in thread
From: Adrian Hunter @ 2014-09-22  8:20 UTC (permalink / raw)
  To: Linus Walleij, Alexandre Courbot
  Cc: Ulf Hansson, linux-mmc, Chris Ball, linux-gpio

On 08/29/2014 03:16 PM, Ulf Hansson wrote:
> On 27 August 2014 13:00, Linus Walleij <linus.walleij@linaro.org> wrote:
>> When the slot GPIO driver gets the GPIO to be used for card
>> detect, it is now possible to specify a flag to have the line
>> set up as input. Get rid of the explicit setup call for input
>> and use the flag.
>>
>> The extra argument works as there are transition varargs
>> macros in place in the <linux/gpio/consumer.h> header, in
>> the future we will make the flags argument compulsory.
>>
>> Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> 
> Thanks! Applied for next.

Unfortunately it doesn't seem to work.  I needed the patch
below.


From: Adrian Hunter <adrian.hunter@intel.com>
Date: Mon, 22 Sep 2014 11:01:16 +0300
Subject: [PATCH] gpio: Fix gpio direction flags not getting set

GPIO direction flags are not getting set because
an 'if' statement is the wrong way around.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/gpio/gpiolib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 15cc0bb..3b54edf 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1674,7 +1674,7 @@ struct gpio_desc *__must_check __gpiod_get_index(struct device *dev,
 		set_bit(FLAG_OPEN_SOURCE, &desc->flags);
 
 	/* No particular flag request, return here... */
-	if (flags & GPIOD_FLAGS_BIT_DIR_SET)
+	if (!(flags & GPIOD_FLAGS_BIT_DIR_SET))
 		return desc;
 
 	/* Process flags */
-- 
1.8.3.2



> 
> Kind regards
> Uffe
> 
>> ---
>>  drivers/mmc/core/slot-gpio.c | 6 +-----
>>  1 file changed, 1 insertion(+), 5 deletions(-)
>>
>> diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c
>> index 5f89cb83d5f0..908c2b29e79f 100644
>> --- a/drivers/mmc/core/slot-gpio.c
>> +++ b/drivers/mmc/core/slot-gpio.c
>> @@ -308,14 +308,10 @@ int mmc_gpiod_request_cd(struct mmc_host *host, const char *con_id,
>>         if (!con_id)
>>                 con_id = ctx->cd_label;
>>
>> -       desc = devm_gpiod_get_index(host->parent, con_id, idx);
>> +       desc = devm_gpiod_get_index(host->parent, con_id, idx, GPIOD_IN);
>>         if (IS_ERR(desc))
>>                 return PTR_ERR(desc);
>>
>> -       ret = gpiod_direction_input(desc);
>> -       if (ret < 0)
>> -               return ret;
>> -
>>         if (debounce) {
>>                 ret = gpiod_set_debounce(desc, debounce);
>>                 if (ret < 0)
>> --
>> 1.9.3
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/4] mmc: slot-gpio: switch to use flags when getting GPIO
  2014-09-22  8:20   ` Adrian Hunter
@ 2014-09-22 12:37     ` Linus Walleij
  2014-09-24  7:38       ` Alexandre Courbot
  0 siblings, 1 reply; 18+ messages in thread
From: Linus Walleij @ 2014-09-22 12:37 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Alexandre Courbot, Ulf Hansson, linux-mmc, Chris Ball, linux-gpio

On Mon, Sep 22, 2014 at 10:20 AM, Adrian Hunter <adrian.hunter@intel.com> wrote:

> Unfortunately it doesn't seem to work.  I needed the patch
> below.
>
>
> From: Adrian Hunter <adrian.hunter@intel.com>
> Date: Mon, 22 Sep 2014 11:01:16 +0300
> Subject: [PATCH] gpio: Fix gpio direction flags not getting set
>
> GPIO direction flags are not getting set because
> an 'if' statement is the wrong way around.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>

Oopps that's a bug, patch applied for fixes, so it'll work when
this hits upstream.

Alex: confirm?

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/4] mmc: slot-gpio: switch to use flags when getting GPIO
  2014-09-22 12:37     ` Linus Walleij
@ 2014-09-24  7:38       ` Alexandre Courbot
  0 siblings, 0 replies; 18+ messages in thread
From: Alexandre Courbot @ 2014-09-24  7:38 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Adrian Hunter, Alexandre Courbot, Ulf Hansson, linux-mmc,
	Chris Ball, linux-gpio

On Mon, Sep 22, 2014 at 9:37 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Mon, Sep 22, 2014 at 10:20 AM, Adrian Hunter <adrian.hunter@intel.com> wrote:
>
>> Unfortunately it doesn't seem to work.  I needed the patch
>> below.
>>
>>
>> From: Adrian Hunter <adrian.hunter@intel.com>
>> Date: Mon, 22 Sep 2014 11:01:16 +0300
>> Subject: [PATCH] gpio: Fix gpio direction flags not getting set
>>
>> GPIO direction flags are not getting set because
>> an 'if' statement is the wrong way around.
>>
>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>
> Oopps that's a bug, patch applied for fixes, so it'll work when
> this hits upstream.
>
> Alex: confirm?

Oh yes absolutely, this was a mistake of mine. Thanks and sorry for
the inconvenience.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/4 v2] mmc: host: switch OF parser to use gpio descriptors
  2014-08-27 11:00 ` [PATCH 3/4 v2] mmc: host: switch OF parser to use gpio descriptors Linus Walleij
  2014-08-29 12:16   ` Ulf Hansson
  2014-09-09  7:05   ` Linus Walleij
@ 2014-09-30 11:30   ` Javier Martinez Canillas
  2014-09-30 14:01     ` Linus Walleij
  2 siblings, 1 reply; 18+ messages in thread
From: Javier Martinez Canillas @ 2014-09-30 11:30 UTC (permalink / raw)
  To: Linus Walleij; +Cc: linux-mmc, Chris Ball, Ulf Hansson, Linux GPIO List

Hello Linus,

On Wed, Aug 27, 2014 at 1:00 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> This switches the central MMC OF parser to use gpio descriptors
> instead of grabbing GPIOs explicitly from the device tree.
> This strips out an unecessary use of the integer-based GPIO
> API that we want to get rid of, cuts down on code as the
> gpio descriptor code will handle active low flags.
>
> Acked-by: Alexandre Courbot <acourbot@nvidia.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>

Card detection is failing in some boards because this patch changes
some semantics. I tried to come up with a fix but I didn't find an
obvious way to do it (more on that below).

>
> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> index 95cceae96944..6f7ed9c50346 100644
> --- a/drivers/mmc/core/host.c
> +++ b/drivers/mmc/core/host.c
> @@ -310,9 +310,7 @@ int mmc_of_parse(struct mmc_host *host)
>  {
>         struct device_node *np;
>         u32 bus_width;
> -       bool explicit_inv_wp, gpio_inv_wp = false;
> -       enum of_gpio_flags flags;
> -       int len, ret, gpio;
> +       int len, ret;
>
>         if (!host->parent || !host->parent->of_node)
>                 return 0;
> @@ -360,60 +358,40 @@ int mmc_of_parse(struct mmc_host *host)
>         if (of_find_property(np, "non-removable", &len)) {
>                 host->caps |= MMC_CAP_NONREMOVABLE;
>         } else {
> -               bool explicit_inv_cd, gpio_inv_cd = false;
> -
> -               explicit_inv_cd = of_property_read_bool(np, "cd-inverted");
> +               if (of_property_read_bool(np, "cd-inverted"))
> +                       host->caps2 |= MMC_CAP2_CD_ACTIVE_HIGH;
>

The card-detect signal active high flag is set if the "cd-inverted"
property is found but in the original code MMC_CAP2_CD_ACTIVE_HIGH was
set only if "cd-inverted" was true and the card detect GPIO flag was
OF_GPIO_ACTIVE_HIGH.

>                 if (of_find_property(np, "broken-cd", &len))
>                         host->caps |= MMC_CAP_NEEDS_POLL;
>
> -               gpio = of_get_named_gpio_flags(np, "cd-gpios", 0, &flags);
> -               if (gpio == -EPROBE_DEFER)
> -                       return gpio;
> -               if (gpio_is_valid(gpio)) {
> -                       if (!(flags & OF_GPIO_ACTIVE_LOW))
> -                               gpio_inv_cd = true;
> -
> -                       ret = mmc_gpio_request_cd(host, gpio, 0);
> -                       if (ret < 0) {
> -                               dev_err(host->parent,
> -                                       "Failed to request CD GPIO #%d: %d!\n",
> -                                       gpio, ret);
> +               ret = mmc_gpiod_request_cd(host, "cd", 0, false, 0);

Now false is passed as the override_active_level bool parameter but
mmc_gpio_request_cd() sets ctx->override_cd_active_level = true. Which
means that after this patch, mmc_gpio_get_cd() does not take into
account if host->caps2 has MMC_CAP2_CD_ACTIVE_HIGH set.

> +               if (ret) {
> +                       if (ret == -EPROBE_DEFER)
>                                 return ret;
> -                       } else {
> -                               dev_info(host->parent, "Got CD GPIO #%d.\n",
> -                                        gpio);
> +                       if (ret != -ENOENT) {
> +                               dev_err(host->parent,
> +                                       "Failed to request CD GPIO: %d\n",
> +                                       ret);
>                         }
> -               }
> -
> -               if (explicit_inv_cd ^ gpio_inv_cd)
> -                       host->caps2 |= MMC_CAP2_CD_ACTIVE_HIGH;
> +               } else
> +                       dev_info(host->parent, "Got CD GPIO\n");
>         }
>
>         /* Parse Write Protection */
> -       explicit_inv_wp = of_property_read_bool(np, "wp-inverted");
> -
> -       gpio = of_get_named_gpio_flags(np, "wp-gpios", 0, &flags);
> -       if (gpio == -EPROBE_DEFER) {
> -               ret = -EPROBE_DEFER;
> -               goto out;
> -       }
> -       if (gpio_is_valid(gpio)) {
> -               if (!(flags & OF_GPIO_ACTIVE_LOW))
> -                       gpio_inv_wp = true;
> +       if (of_property_read_bool(np, "wp-inverted"))
> +               host->caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
>
> -               ret = mmc_gpio_request_ro(host, gpio);
> -               if (ret < 0) {
> -                       dev_err(host->parent,
> -                               "Failed to request WP GPIO: %d!\n", ret);
> +       ret = mmc_gpiod_request_ro(host, "wp", 0, false, 0);
> +       if (ret) {
> +               if (ret == -EPROBE_DEFER)
>                         goto out;
> -               } else {
> -                               dev_info(host->parent, "Got WP GPIO #%d.\n",
> -                                        gpio);
> +               if (ret != -ENOENT) {
> +                       dev_err(host->parent,
> +                               "Failed to request WP GPIO: %d\n",
> +                               ret);
>                 }
> -       }
> -       if (explicit_inv_wp ^ gpio_inv_wp)
> -               host->caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;

Same issue with write-protect signal active high. After this patch
MMC_CAP2_RO_ACTIVE_HIGH is always set while before it was
conditionally set depending of the write-protect GPIO flag value.

> +       } else
> +               dev_info(host->parent, "Got WP GPIO\n");
>
>         if (of_find_property(np, "cap-sd-highspeed", &len))
>                 host->caps |= MMC_CAP_SD_HIGHSPEED;
> --

Passing true to mmc_gpiod_request_cd() and not setting
MMC_CAP2_RO_ACTIVE_HIGH makes card detection work again on this board.
The former is a one-liner but the later is more complicated since
AFAIU the idea of the GPIO descriptor code is to handle the flags but
in this case this detail is needed.

Should we set MMC_CAP2_{CD,RO}_ACTIVE_HIGH in
mmc_gpiod_request_{cd,ro} or add functions to get the flags for these
cd and wp GPIO in order to set the host->caps2 flags according to the
old logic?

Thanks a lot and best regards,
Javier

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/4 v2] mmc: host: switch OF parser to use gpio descriptors
  2014-09-30 11:30   ` Javier Martinez Canillas
@ 2014-09-30 14:01     ` Linus Walleij
  0 siblings, 0 replies; 18+ messages in thread
From: Linus Walleij @ 2014-09-30 14:01 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: linux-mmc, Chris Ball, Ulf Hansson, Linux GPIO List

On Tue, Sep 30, 2014 at 1:30 PM, Javier Martinez Canillas
<javier@dowhile0.org> wrote:
> On Wed, Aug 27, 2014 at 1:00 PM, Linus Walleij <linus.walleij@linaro.org> wrote:

>> This switches the central MMC OF parser to use gpio descriptors
>> instead of grabbing GPIOs explicitly from the device tree.
>> This strips out an unecessary use of the integer-based GPIO
>> API that we want to get rid of, cuts down on code as the
>> gpio descriptor code will handle active low flags.
>>
>> Acked-by: Alexandre Courbot <acourbot@nvidia.com>
>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>
> Card detection is failing in some boards because this patch changes
> some semantics. I tried to come up with a fix but I didn't find an
> obvious way to do it (more on that below).

I think some of this may be weird semantics: the CAP flags in the
MMC subsystem, MMC_CAP2_CD_ACTIVE_HIGH and
MMC_CAP2_RO_ACTIVE_HIGH are somewhat confusing since
the GPIO subsystem nowadays has its own concept of a line
active high or low.

The MMC caps make a lot of sense if the host has dedicated lines
to read CD and WP, and does not use a GPIO for this.

When a GPIO descriptor is in use, the thing gets a bit convoluted:
what if both the GPIO *and* the CAP flag is set to inversion?

So the old code did this with XOR:

        if (explicit_inv_cd ^ gpio_inv_cd)
            host->caps2 |= MMC_CAP2_CD_ACTIVE_HIGH;

Meaning double inversion -> no inversion.

I guess I was sort of hoping that nobody did this crazy thing.
Turns out I was wrong, so have to restore the old semantics...

I'm sending a patch for you to test.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/4] mmc: slot-gpio: add gpiod variant to get wp GPIO
  2014-08-12 17:25 ` [PATCH 2/4] mmc: slot-gpio: add gpiod variant to get wp GPIO Linus Walleij
  2014-08-14 14:32   ` Alexandre Courbot
@ 2014-08-18 11:25   ` Ulf Hansson
  1 sibling, 0 replies; 18+ messages in thread
From: Ulf Hansson @ 2014-08-18 11:25 UTC (permalink / raw)
  To: Linus Walleij; +Cc: linux-mmc, Chris Ball, linux-gpio, Alexandre Courbot

On 12 August 2014 19:25, Linus Walleij <linus.walleij@linaro.org> wrote:
> This makes it possible to get the write protect (read only)
> GPIO line from a GPIO descriptor. Written to exactly mirror
> the card detect function.
>
> Cc: Alexandre Courbot <gnurou@gmail.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

Thanks! Applied for next.

Kind regards
Uffe


> ---
>  drivers/mmc/core/slot-gpio.c  | 48 +++++++++++++++++++++++++++++++++++++++++++
>  include/linux/mmc/slot-gpio.h |  3 +++
>  2 files changed, 51 insertions(+)
>
> diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c
> index 908c2b29e79f..e3fce4493fab 100644
> --- a/drivers/mmc/core/slot-gpio.c
> +++ b/drivers/mmc/core/slot-gpio.c
> @@ -326,6 +326,54 @@ int mmc_gpiod_request_cd(struct mmc_host *host, const char *con_id,
>  EXPORT_SYMBOL(mmc_gpiod_request_cd);
>
>  /**
> + * mmc_gpiod_request_ro - request a gpio descriptor for write protection
> + * @host: mmc host
> + * @con_id: function within the GPIO consumer
> + * @idx: index of the GPIO to obtain in the consumer
> + * @override_active_level: ignore %GPIO_ACTIVE_LOW flag
> + * @debounce: debounce time in microseconds
> + *
> + * Use this function in place of mmc_gpio_request_ro() to use the GPIO
> + * descriptor API.  Note that it is paired with mmc_gpiod_free_ro() not
> + * mmc_gpio_free_ro().
> + *
> + * Returns zero on success, else an error.
> + */
> +int mmc_gpiod_request_ro(struct mmc_host *host, const char *con_id,
> +                        unsigned int idx, bool override_active_level,
> +                        unsigned int debounce)
> +{
> +       struct mmc_gpio *ctx;
> +       struct gpio_desc *desc;
> +       int ret;
> +
> +       ret = mmc_gpio_alloc(host);
> +       if (ret < 0)
> +               return ret;
> +
> +       ctx = host->slot.handler_priv;
> +
> +       if (!con_id)
> +               con_id = ctx->ro_label;
> +
> +       desc = devm_gpiod_get_index(host->parent, con_id, idx, GPIOD_IN);
> +       if (IS_ERR(desc))
> +               return PTR_ERR(desc);
> +
> +       if (debounce) {
> +               ret = gpiod_set_debounce(desc, debounce);
> +               if (ret < 0)
> +                       return ret;
> +       }
> +
> +       ctx->override_ro_active_level = override_active_level;
> +       ctx->ro_gpio = desc;
> +
> +       return 0;
> +}
> +EXPORT_SYMBOL(mmc_gpiod_request_ro);
> +
> +/**
>   * mmc_gpiod_free_cd - free the card-detection gpio descriptor
>   * @host: mmc host
>   *
> diff --git a/include/linux/mmc/slot-gpio.h b/include/linux/mmc/slot-gpio.h
> index d2433381e828..a0d0442c15bf 100644
> --- a/include/linux/mmc/slot-gpio.h
> +++ b/include/linux/mmc/slot-gpio.h
> @@ -25,6 +25,9 @@ void mmc_gpio_free_cd(struct mmc_host *host);
>  int mmc_gpiod_request_cd(struct mmc_host *host, const char *con_id,
>                          unsigned int idx, bool override_active_level,
>                          unsigned int debounce);
> +int mmc_gpiod_request_ro(struct mmc_host *host, const char *con_id,
> +                        unsigned int idx, bool override_active_level,
> +                        unsigned int debounce);
>  void mmc_gpiod_free_cd(struct mmc_host *host);
>  void mmc_gpiod_request_cd_irq(struct mmc_host *host);
>
> --
> 1.9.3
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/4] mmc: slot-gpio: add gpiod variant to get wp GPIO
  2014-08-12 17:25 ` [PATCH 2/4] mmc: slot-gpio: add gpiod variant to get wp GPIO Linus Walleij
@ 2014-08-14 14:32   ` Alexandre Courbot
  2014-08-18 11:25   ` Ulf Hansson
  1 sibling, 0 replies; 18+ messages in thread
From: Alexandre Courbot @ 2014-08-14 14:32 UTC (permalink / raw)
  To: Linus Walleij; +Cc: linux-mmc, Chris Ball, Ulf Hansson, linux-gpio

On Tue, Aug 12, 2014 at 10:25 AM, Linus Walleij
<linus.walleij@linaro.org> wrote:
> This makes it possible to get the write protect (read only)
> GPIO line from a GPIO descriptor. Written to exactly mirror
> the card detect function.

Acked-by: Alexandre Courbot <acourbot@nvidia.com>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 2/4] mmc: slot-gpio: add gpiod variant to get wp GPIO
  2014-08-12 17:25 Linus Walleij
@ 2014-08-12 17:25 ` Linus Walleij
  2014-08-14 14:32   ` Alexandre Courbot
  2014-08-18 11:25   ` Ulf Hansson
  0 siblings, 2 replies; 18+ messages in thread
From: Linus Walleij @ 2014-08-12 17:25 UTC (permalink / raw)
  To: linux-mmc, Chris Ball, Ulf Hansson
  Cc: linux-gpio, Linus Walleij, Alexandre Courbot

This makes it possible to get the write protect (read only)
GPIO line from a GPIO descriptor. Written to exactly mirror
the card detect function.

Cc: Alexandre Courbot <gnurou@gmail.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/mmc/core/slot-gpio.c  | 48 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/mmc/slot-gpio.h |  3 +++
 2 files changed, 51 insertions(+)

diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c
index 908c2b29e79f..e3fce4493fab 100644
--- a/drivers/mmc/core/slot-gpio.c
+++ b/drivers/mmc/core/slot-gpio.c
@@ -326,6 +326,54 @@ int mmc_gpiod_request_cd(struct mmc_host *host, const char *con_id,
 EXPORT_SYMBOL(mmc_gpiod_request_cd);
 
 /**
+ * mmc_gpiod_request_ro - request a gpio descriptor for write protection
+ * @host: mmc host
+ * @con_id: function within the GPIO consumer
+ * @idx: index of the GPIO to obtain in the consumer
+ * @override_active_level: ignore %GPIO_ACTIVE_LOW flag
+ * @debounce: debounce time in microseconds
+ *
+ * Use this function in place of mmc_gpio_request_ro() to use the GPIO
+ * descriptor API.  Note that it is paired with mmc_gpiod_free_ro() not
+ * mmc_gpio_free_ro().
+ *
+ * Returns zero on success, else an error.
+ */
+int mmc_gpiod_request_ro(struct mmc_host *host, const char *con_id,
+			 unsigned int idx, bool override_active_level,
+			 unsigned int debounce)
+{
+	struct mmc_gpio *ctx;
+	struct gpio_desc *desc;
+	int ret;
+
+	ret = mmc_gpio_alloc(host);
+	if (ret < 0)
+		return ret;
+
+	ctx = host->slot.handler_priv;
+
+	if (!con_id)
+		con_id = ctx->ro_label;
+
+	desc = devm_gpiod_get_index(host->parent, con_id, idx, GPIOD_IN);
+	if (IS_ERR(desc))
+		return PTR_ERR(desc);
+
+	if (debounce) {
+		ret = gpiod_set_debounce(desc, debounce);
+		if (ret < 0)
+			return ret;
+	}
+
+	ctx->override_ro_active_level = override_active_level;
+	ctx->ro_gpio = desc;
+
+	return 0;
+}
+EXPORT_SYMBOL(mmc_gpiod_request_ro);
+
+/**
  * mmc_gpiod_free_cd - free the card-detection gpio descriptor
  * @host: mmc host
  *
diff --git a/include/linux/mmc/slot-gpio.h b/include/linux/mmc/slot-gpio.h
index d2433381e828..a0d0442c15bf 100644
--- a/include/linux/mmc/slot-gpio.h
+++ b/include/linux/mmc/slot-gpio.h
@@ -25,6 +25,9 @@ void mmc_gpio_free_cd(struct mmc_host *host);
 int mmc_gpiod_request_cd(struct mmc_host *host, const char *con_id,
 			 unsigned int idx, bool override_active_level,
 			 unsigned int debounce);
+int mmc_gpiod_request_ro(struct mmc_host *host, const char *con_id,
+			 unsigned int idx, bool override_active_level,
+			 unsigned int debounce);
 void mmc_gpiod_free_cd(struct mmc_host *host);
 void mmc_gpiod_request_cd_irq(struct mmc_host *host);
 
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2014-09-30 14:01 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-27 11:00 [PATCH 1/4] mmc: slot-gpio: switch to use flags when getting GPIO Linus Walleij
2014-08-27 11:00 ` [PATCH 2/4] mmc: slot-gpio: add gpiod variant to get wp GPIO Linus Walleij
2014-08-29 12:16   ` Ulf Hansson
2014-08-27 11:00 ` [PATCH 3/4 v2] mmc: host: switch OF parser to use gpio descriptors Linus Walleij
2014-08-29 12:16   ` Ulf Hansson
2014-09-09  7:05   ` Linus Walleij
2014-09-09 12:29     ` Ulf Hansson
2014-09-30 11:30   ` Javier Martinez Canillas
2014-09-30 14:01     ` Linus Walleij
2014-08-27 11:00 ` [PATCH 4/4] mmc: mmci: augment driver to handle " Linus Walleij
2014-08-27 11:34   ` Ulf Hansson
2014-08-29 12:16 ` [PATCH 1/4] mmc: slot-gpio: switch to use flags when getting GPIO Ulf Hansson
2014-09-22  8:20   ` Adrian Hunter
2014-09-22 12:37     ` Linus Walleij
2014-09-24  7:38       ` Alexandre Courbot
  -- strict thread matches above, loose matches on Subject: below --
2014-08-12 17:25 Linus Walleij
2014-08-12 17:25 ` [PATCH 2/4] mmc: slot-gpio: add gpiod variant to get wp GPIO Linus Walleij
2014-08-14 14:32   ` Alexandre Courbot
2014-08-18 11:25   ` Ulf Hansson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.