[PATCH 1/2] mmc: dw_mmc: exynos: Stop claiming wp-gpio

[PATCH 1/2] mmc: dw_mmc: exynos: Stop claiming wp-gpio

[Doug Anderson]

The exynos code claimed wp-gpio with devm_gpio_request() but never did
anything with it.  That meant that anyone using a write protect GPIO
would effectively be write protected all the time.

A future change will move the wp-gpio support to the core dw_mmc.c
file.  Now the exynos-specific code won't claim the GPIO but will
just set the DW_MCI_QUIRK_NO_WRITE_PROTECT quirk if write protect
won't be used.

Signed-off-by: Doug Anderson <dianders@chromium.org>

---
 drivers/mmc/host/dw_mmc-exynos.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
index 4d50da6..58cc03e 100644
--- a/drivers/mmc/host/dw_mmc-exynos.c
+++ b/drivers/mmc/host/dw_mmc-exynos.c
@@ -175,12 +175,12 @@ static int dw_mci_exynos_setup_bus(struct dw_mci *host,
 		}
 	}
 
-	gpio = of_get_named_gpio(slot_np, "wp-gpios", 0);
-	if (gpio_is_valid(gpio)) {
-		if (devm_gpio_request(host->dev, gpio, "dw-mci-wp"))
-			dev_info(host->dev, "gpio [%d] request failed\n",
-						gpio);
-	} else {
+	/*
+	 * If there are no write-protect GPIOs present then we assume no write
+	 * protect.  The mci_readl() in dw_mmc.c won't work since it's not
+	 * hooked up on exynos.
+	 */
+	if (!of_find_property(slot_np, "wp-gpios", NULL)) {
 		dev_info(host->dev, "wp gpio not available");
 		host->pdata->quirks |= DW_MCI_QUIRK_NO_WRITE_PROTECT;
 	}
-- 
1.7.7.3

[PATCH 2/2] mmc: dw_mmc: Handle wp-gpios from device tree

[Doug Anderson]

On some SoCs (like exynos5250) you need to use an external GPIO for
write protect.  Add support for wp-gpios to the core dw_mmc driver
since it could be useful across multiple SoCs.

With this change I am able to make use of the write protect for the
external SD slot on exynos5250-snow.

Signed-off-by: Doug Anderson <dianders@chromium.org>
---
 drivers/mmc/host/dw_mmc.c |   35 +++++++++++++++++++++++++++++++++++
 1 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 5b41348..9c79870 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -34,6 +34,7 @@
 #include <linux/regulator/consumer.h>
 #include <linux/workqueue.h>
 #include <linux/of.h>
+#include <linux/of_gpio.h>
 
 #include "dw_mmc.h"
 
@@ -74,6 +75,7 @@ struct idmac_desc {
  * struct dw_mci_slot - MMC slot state
  * @mmc: The mmc_host representing this slot.
  * @host: The MMC controller this slot is using.
+ * @wp_gpio: If gpio_is_valid() we'll use this to read write protect.
  * @ctype: Card type for this slot.
  * @mrq: mmc_request currently being processed or waiting to be
  *	processed, or NULL when the slot is idle.
@@ -88,6 +90,8 @@ struct dw_mci_slot {
 	struct mmc_host		*mmc;
 	struct dw_mci		*host;
 
+	int			wp_gpio;
+
 	u32			ctype;
 
 	struct mmc_request	*mrq;
@@ -832,6 +836,8 @@ static int dw_mci_get_ro(struct mmc_host *mmc)
 		read_only = 0;
 	else if (brd->get_ro)
 		read_only = brd->get_ro(slot->id);
+	else if (gpio_is_valid(slot->wp_gpio))
+		read_only = gpio_get_value(slot->wp_gpio);
 	else
 		read_only =
 			mci_readl(slot->host, WRTPRT) & (1 << slot->id) ? 1 : 0;
@@ -1802,6 +1808,29 @@ static u32 dw_mci_of_get_bus_wd(struct device *dev, u8 slot)
 			       " as 1\n");
 	return bus_wd;
 }
+
+/* find the write protect gpio for a given slot; or -1 if none specified */
+static u32 dw_mci_of_get_wp_gpio(struct device *dev, u8 slot)
+{
+	struct device_node *np = dw_mci_of_find_slot_node(dev, slot);
+	int gpio;
+
+	if (!np)
+		return -1;
+
+	gpio = of_get_named_gpio(np, "wp-gpios", 0);
+
+	/* Having a missing entry is valid; return silently */
+	if (!gpio_is_valid(gpio))
+		return -1;
+
+	if (devm_gpio_request(dev, gpio, "dw-mci-wp")) {
+		dev_warn(dev, "gpio [%d] request failed\n", gpio);
+		return -1;
+	}
+
+	return gpio;
+}
 #else /* CONFIG_OF */
 static u32 dw_mci_of_get_bus_wd(struct device *dev, u8 slot)
 {
@@ -1811,6 +1840,10 @@ static struct device_node *dw_mci_of_find_slot_node(struct device *dev, u8 slot)
 {
 	return NULL;
 }
+static u32 dw_mci_of_get_wp_gpio(struct device *dev, u8 slot)
+{
+	return -1;
+}
 #endif /* CONFIG_OF */
 
 static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
@@ -1923,6 +1956,8 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
 	else
 		clear_bit(DW_MMC_CARD_PRESENT, &slot->flags);
 
+	slot->wp_gpio = dw_mci_of_get_wp_gpio(host->dev, slot->id);
+
 	mmc_add_host(mmc);
 
 #if defined(CONFIG_DEBUG_FS)
-- 
1.7.7.3

RE: [PATCH 2/2] mmc: dw_mmc: Handle wp-gpios from device tree

[Seungwon Jeon]

Hi,

wp-gpios has been implemented in dw_mmc-exynos.c
It can be reused for EXYNOS platform? We need to modify some though.

Thanks,
Seungwon Jeon

On Thursday, November 22, 2012, Doug Anderson <dianders@chromium.org> wrote:
> On some SoCs (like exynos5250) you need to use an external GPIO for
> write protect.  Add support for wp-gpios to the core dw_mmc driver
> since it could be useful across multiple SoCs.
> 
> With this change I am able to make use of the write protect for the
> external SD slot on exynos5250-snow.
> 
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> ---
>  drivers/mmc/host/dw_mmc.c |   35 +++++++++++++++++++++++++++++++++++
>  1 files changed, 35 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 5b41348..9c79870 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -34,6 +34,7 @@
>  #include <linux/regulator/consumer.h>
>  #include <linux/workqueue.h>
>  #include <linux/of.h>
> +#include <linux/of_gpio.h>
> 
>  #include "dw_mmc.h"
> 
> @@ -74,6 +75,7 @@ struct idmac_desc {
>   * struct dw_mci_slot - MMC slot state
>   * @mmc: The mmc_host representing this slot.
>   * @host: The MMC controller this slot is using.
> + * @wp_gpio: If gpio_is_valid() we'll use this to read write protect.
>   * @ctype: Card type for this slot.
>   * @mrq: mmc_request currently being processed or waiting to be
>   *	processed, or NULL when the slot is idle.
> @@ -88,6 +90,8 @@ struct dw_mci_slot {
>  	struct mmc_host		*mmc;
>  	struct dw_mci		*host;
> 
> +	int			wp_gpio;
> +
>  	u32			ctype;
> 
>  	struct mmc_request	*mrq;
> @@ -832,6 +836,8 @@ static int dw_mci_get_ro(struct mmc_host *mmc)
>  		read_only = 0;
>  	else if (brd->get_ro)
>  		read_only = brd->get_ro(slot->id);
> +	else if (gpio_is_valid(slot->wp_gpio))
> +		read_only = gpio_get_value(slot->wp_gpio);
>  	else
>  		read_only =
>  			mci_readl(slot->host, WRTPRT) & (1 << slot->id) ? 1 : 0;
> @@ -1802,6 +1808,29 @@ static u32 dw_mci_of_get_bus_wd(struct device *dev, u8 slot)
>  			       " as 1\n");
>  	return bus_wd;
>  }
> +
> +/* find the write protect gpio for a given slot; or -1 if none specified */
> +static u32 dw_mci_of_get_wp_gpio(struct device *dev, u8 slot)
> +{
> +	struct device_node *np = dw_mci_of_find_slot_node(dev, slot);
> +	int gpio;
> +
> +	if (!np)
> +		return -1;
> +
> +	gpio = of_get_named_gpio(np, "wp-gpios", 0);
> +
> +	/* Having a missing entry is valid; return silently */
> +	if (!gpio_is_valid(gpio))
> +		return -1;
> +
> +	if (devm_gpio_request(dev, gpio, "dw-mci-wp")) {
> +		dev_warn(dev, "gpio [%d] request failed\n", gpio);
> +		return -1;
> +	}
> +
> +	return gpio;
> +}
>  #else /* CONFIG_OF */
>  static u32 dw_mci_of_get_bus_wd(struct device *dev, u8 slot)
>  {
> @@ -1811,6 +1840,10 @@ static struct device_node *dw_mci_of_find_slot_node(struct device *dev, u8 slot)
>  {
>  	return NULL;
>  }
> +static u32 dw_mci_of_get_wp_gpio(struct device *dev, u8 slot)
> +{
> +	return -1;
> +}
>  #endif /* CONFIG_OF */
> 
>  static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
> @@ -1923,6 +1956,8 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
>  	else
>  		clear_bit(DW_MMC_CARD_PRESENT, &slot->flags);
> 
> +	slot->wp_gpio = dw_mci_of_get_wp_gpio(host->dev, slot->id);
> +
>  	mmc_add_host(mmc);
> 
>  #if defined(CONFIG_DEBUG_FS)
> --
> 1.7.7.3

Re: [PATCH 2/2] mmc: dw_mmc: Handle wp-gpios from device tree

[Jaehoon Chung]

On 11/22/2012 07:03 AM, Doug Anderson wrote:
> On some SoCs (like exynos5250) you need to use an external GPIO for
> write protect.  Add support for wp-gpios to the core dw_mmc driver
> since it could be useful across multiple SoCs.
> 
> With this change I am able to make use of the write protect for the
> external SD slot on exynos5250-snow.
> 
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> ---
>  drivers/mmc/host/dw_mmc.c |   35 +++++++++++++++++++++++++++++++++++
>  1 files changed, 35 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 5b41348..9c79870 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -34,6 +34,7 @@
>  #include <linux/regulator/consumer.h>
>  #include <linux/workqueue.h>
>  #include <linux/of.h>
> +#include <linux/of_gpio.h>
>  
>  #include "dw_mmc.h"
>  
> @@ -74,6 +75,7 @@ struct idmac_desc {
>   * struct dw_mci_slot - MMC slot state
>   * @mmc: The mmc_host representing this slot.
>   * @host: The MMC controller this slot is using.
> + * @wp_gpio: If gpio_is_valid() we'll use this to read write protect.
>   * @ctype: Card type for this slot.
>   * @mrq: mmc_request currently being processed or waiting to be
>   *	processed, or NULL when the slot is idle.
> @@ -88,6 +90,8 @@ struct dw_mci_slot {
>  	struct mmc_host		*mmc;
>  	struct dw_mci		*host;
>  
> +	int			wp_gpio;
> +
>  	u32			ctype;
>  
>  	struct mmc_request	*mrq;
> @@ -832,6 +836,8 @@ static int dw_mci_get_ro(struct mmc_host *mmc)
>  		read_only = 0;
>  	else if (brd->get_ro)
>  		read_only = brd->get_ro(slot->id);
> +	else if (gpio_is_valid(slot->wp_gpio))
> +		read_only = gpio_get_value(slot->wp_gpio);
>  	else
>  		read_only =
>  			mci_readl(slot->host, WRTPRT) & (1 << slot->id) ? 1 : 0;
> @@ -1802,6 +1808,29 @@ static u32 dw_mci_of_get_bus_wd(struct device *dev, u8 slot)
>  			       " as 1\n");
>  	return bus_wd;
>  }
> +
> +/* find the write protect gpio for a given slot; or -1 if none specified */
> +static u32 dw_mci_of_get_wp_gpio(struct device *dev, u8 slot)
> +{
> +	struct device_node *np = dw_mci_of_find_slot_node(dev, slot);
> +	int gpio;
> +
> +	if (!np)
> +		return -1;
I think good that use the error number instead of -1. Also the below code.

> +
> +	gpio = of_get_named_gpio(np, "wp-gpios", 0);
> +
> +	/* Having a missing entry is valid; return silently */
> +	if (!gpio_is_valid(gpio))
> +		return -1;
> +
> +	if (devm_gpio_request(dev, gpio, "dw-mci-wp")) {
> +		dev_warn(dev, "gpio [%d] request failed\n", gpio);
> +		return -1;
> +	}
> +
> +	return gpio;
gpio is int type, but return type is u32?

Best Regards,
Jaehoon Chung
> +}
>  #else /* CONFIG_OF */
>  static u32 dw_mci_of_get_bus_wd(struct device *dev, u8 slot)
>  {
> @@ -1811,6 +1840,10 @@ static struct device_node *dw_mci_of_find_slot_node(struct device *dev, u8 slot)
>  {
>  	return NULL;
>  }
> +static u32 dw_mci_of_get_wp_gpio(struct device *dev, u8 slot)
> +{
> +	return -1;
> +}
>  #endif /* CONFIG_OF */
>  
>  static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
> @@ -1923,6 +1956,8 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
>  	else
>  		clear_bit(DW_MMC_CARD_PRESENT, &slot->flags);
>  
> +	slot->wp_gpio = dw_mci_of_get_wp_gpio(host->dev, slot->id);
> +
>  	mmc_add_host(mmc);
>  
>  #if defined(CONFIG_DEBUG_FS)
> 

Re: [PATCH 2/2] mmc: dw_mmc: Handle wp-gpios from device tree

[Doug Anderson]

On Wed, Nov 21, 2012 at 5:42 PM, Seungwon Jeon <tgih.jun@samsung.com> wrote:
> Hi,
>
> wp-gpios has been implemented in dw_mmc-exynos.c
> It can be reused for EXYNOS platform? We need to modify some though.

Yup, I've seen that.  Patch 1/2 ("mmc: dw_mmc: exynos: Stop claiming
wp-gpio") addressed that.  For some reason I can't find that on
LKML.org yet.  Strange.  :-/  I'll forward it on to you shortly.

In any case: I found that the exynos code didn't actually work.  It
claimed the GPIO but didn't ever look at it.

I have the beginnings of the code to implement this properly in the
exynos code but I stopped working on it when I decided that other SoCs
could also benefit from the code and it fit better in the general
dw_mmc driver.

If you disagree and would like me to cleanup the version of this patch
that's just in the exynos driver and post that, I will.  Just let me
know.


Thanks for the review!

-Doug

Re: [PATCH 2/2] mmc: dw_mmc: Handle wp-gpios from device tree

[Doug Anderson]

Jaehoon,

Thanks for the review.  See below for comments.  I'll plan on a new
patch either Monday or Tuesday when I have a chance to spin and
re-test.


On Wed, Nov 21, 2012 at 5:55 PM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> On 11/22/2012 07:03 AM, Doug Anderson wrote:
>> On some SoCs (like exynos5250) you need to use an external GPIO for
>> write protect.  Add support for wp-gpios to the core dw_mmc driver
>> since it could be useful across multiple SoCs.
>>
>> With this change I am able to make use of the write protect for the
>> external SD slot on exynos5250-snow.
>>
>> Signed-off-by: Doug Anderson <dianders@chromium.org>
>> ---
>>  drivers/mmc/host/dw_mmc.c |   35 +++++++++++++++++++++++++++++++++++
>>  1 files changed, 35 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 5b41348..9c79870 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -34,6 +34,7 @@
>>  #include <linux/regulator/consumer.h>
>>  #include <linux/workqueue.h>
>>  #include <linux/of.h>
>> +#include <linux/of_gpio.h>
>>
>>  #include "dw_mmc.h"
>>
>> @@ -74,6 +75,7 @@ struct idmac_desc {
>>   * struct dw_mci_slot - MMC slot state
>>   * @mmc: The mmc_host representing this slot.
>>   * @host: The MMC controller this slot is using.
>> + * @wp_gpio: If gpio_is_valid() we'll use this to read write protect.
>>   * @ctype: Card type for this slot.
>>   * @mrq: mmc_request currently being processed or waiting to be
>>   *   processed, or NULL when the slot is idle.
>> @@ -88,6 +90,8 @@ struct dw_mci_slot {
>>       struct mmc_host         *mmc;
>>       struct dw_mci           *host;
>>
>> +     int                     wp_gpio;
>> +
>>       u32                     ctype;
>>
>>       struct mmc_request      *mrq;
>> @@ -832,6 +836,8 @@ static int dw_mci_get_ro(struct mmc_host *mmc)
>>               read_only = 0;
>>       else if (brd->get_ro)
>>               read_only = brd->get_ro(slot->id);
>> +     else if (gpio_is_valid(slot->wp_gpio))
>> +             read_only = gpio_get_value(slot->wp_gpio);
>>       else
>>               read_only =
>>                       mci_readl(slot->host, WRTPRT) & (1 << slot->id) ? 1 : 0;
>> @@ -1802,6 +1808,29 @@ static u32 dw_mci_of_get_bus_wd(struct device *dev, u8 slot)
>>                              " as 1\n");
>>       return bus_wd;
>>  }
>> +
>> +/* find the write protect gpio for a given slot; or -1 if none specified */
>> +static u32 dw_mci_of_get_wp_gpio(struct device *dev, u8 slot)
>> +{
>> +     struct device_node *np = dw_mci_of_find_slot_node(dev, slot);
>> +     int gpio;
>> +
>> +     if (!np)
>> +             return -1;
> I think good that use the error number instead of -1. Also the below code.

In this case it's not really returning an error code which is why I
chose -1.  It's returning a gpio number or anything that is a sentinel
value indicating that the GPIO is not valid.

...but you're right, an error code would work.  I'll replace with
"-EINVAL" in my next patch.

>
>> +
>> +     gpio = of_get_named_gpio(np, "wp-gpios", 0);
>> +
>> +     /* Having a missing entry is valid; return silently */
>> +     if (!gpio_is_valid(gpio))
>> +             return -1;
>> +
>> +     if (devm_gpio_request(dev, gpio, "dw-mci-wp")) {
>> +             dev_warn(dev, "gpio [%d] request failed\n", gpio);
>> +             return -1;
>> +     }
>> +
>> +     return gpio;
> gpio is int type, but return type is u32?

Good catch.  Will fix.

>
> Best Regards,
> Jaehoon Chung
>> +}
>>  #else /* CONFIG_OF */
>>  static u32 dw_mci_of_get_bus_wd(struct device *dev, u8 slot)
>>  {
>> @@ -1811,6 +1840,10 @@ static struct device_node *dw_mci_of_find_slot_node(struct device *dev, u8 slot)
>>  {
>>       return NULL;
>>  }
>> +static u32 dw_mci_of_get_wp_gpio(struct device *dev, u8 slot)
>> +{
>> +     return -1;
>> +}
>>  #endif /* CONFIG_OF */
>>
>>  static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
>> @@ -1923,6 +1956,8 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
>>       else
>>               clear_bit(DW_MMC_CARD_PRESENT, &slot->flags);
>>
>> +     slot->wp_gpio = dw_mci_of_get_wp_gpio(host->dev, slot->id);
>> +
>>       mmc_add_host(mmc);
>>
>>  #if defined(CONFIG_DEBUG_FS)
>>
>

RE: [PATCH 2/2] mmc: dw_mmc: Handle wp-gpios from device tree

[Seungwon Jeon]

On Thursday, November 22, 2012, Doug Anderson <dianders@chromium.org> wrote:
> On Wed, Nov 21, 2012 at 5:42 PM, Seungwon Jeon <tgih.jun@samsung.com> wrote:
> > Hi,
> >
> > wp-gpios has been implemented in dw_mmc-exynos.c
> > It can be reused for EXYNOS platform? We need to modify some though.
> 
> Yup, I've seen that.  Patch 1/2 ("mmc: dw_mmc: exynos: Stop claiming
> wp-gpio") addressed that.  For some reason I can't find that on
> LKML.org yet.  Strange.  :-/  I'll forward it on to you shortly.
> 
> In any case: I found that the exynos code didn't actually work.  It
> claimed the GPIO but didn't ever look at it.
> 
> I have the beginnings of the code to implement this properly in the
> exynos code but I stopped working on it when I decided that other SoCs
> could also benefit from the code and it fit better in the general
> dw_mmc driver.
> 
> If you disagree and would like me to cleanup the version of this patch
> that's just in the exynos driver and post that, I will.  Just let me
> know.
Yes, origin code of dw_mmc-exynos didn't work fine. We need to modify it.
Anyway, some problem in mailing? I didn't get 1/2 of patch.
In addition, it's not found in any mail-archive. After resolved, I can review.

Thanks, 
Seungwon Jeon
> 
> 
> Thanks for the review!
> 
> -Doug

[PATCH v2 1/2] mmc: dw_mmc: exynos: Stop claiming wp-gpio

[Doug Anderson]

The exynos code claimed wp-gpio with devm_gpio_request() but never did
anything with it.  That meant that anyone using a write protect GPIO
would effectively be write protected all the time.

A future change will move the wp-gpio support to the core dw_mmc.c
file.  Now the exynos-specific code won't claim the GPIO but will
just set the DW_MCI_QUIRK_NO_WRITE_PROTECT quirk if write protect
won't be used.

Signed-off-by: Doug Anderson <dianders@chromium.org>

---
Changes in v2:
- Nothing new in this patch

 drivers/mmc/host/dw_mmc-exynos.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
index 4d50da6..58cc03e 100644
--- a/drivers/mmc/host/dw_mmc-exynos.c
+++ b/drivers/mmc/host/dw_mmc-exynos.c
@@ -175,12 +175,12 @@ static int dw_mci_exynos_setup_bus(struct dw_mci *host,
 		}
 	}
 
-	gpio = of_get_named_gpio(slot_np, "wp-gpios", 0);
-	if (gpio_is_valid(gpio)) {
-		if (devm_gpio_request(host->dev, gpio, "dw-mci-wp"))
-			dev_info(host->dev, "gpio [%d] request failed\n",
-						gpio);
-	} else {
+	/*
+	 * If there are no write-protect GPIOs present then we assume no write
+	 * protect.  The mci_readl() in dw_mmc.c won't work since it's not
+	 * hooked up on exynos.
+	 */
+	if (!of_find_property(slot_np, "wp-gpios", NULL)) {
 		dev_info(host->dev, "wp gpio not available");
 		host->pdata->quirks |= DW_MCI_QUIRK_NO_WRITE_PROTECT;
 	}
-- 
1.7.7.3

[PATCH v2 2/2] mmc: dw_mmc: Handle wp-gpios from device tree

[Doug Anderson]

On some SoCs (like exynos5250) you need to use an external GPIO for
write protect.  Add support for wp-gpios to the core dw_mmc driver
since it could be useful across multiple SoCs.

With this change I am able to make use of the write protect for the
external SD slot on exynos5250-snow.

Signed-off-by: Doug Anderson <dianders@chromium.org>

---
Changes in v2:
- Fixed return type from u32 to int
- Return -EINVAL instead of -1

 drivers/mmc/host/dw_mmc.c |   35 +++++++++++++++++++++++++++++++++++
 1 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 5b41348..42d4f19 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -34,6 +34,7 @@
 #include <linux/regulator/consumer.h>
 #include <linux/workqueue.h>
 #include <linux/of.h>
+#include <linux/of_gpio.h>
 
 #include "dw_mmc.h"
 
@@ -74,6 +75,7 @@ struct idmac_desc {
  * struct dw_mci_slot - MMC slot state
  * @mmc: The mmc_host representing this slot.
  * @host: The MMC controller this slot is using.
+ * @wp_gpio: If gpio_is_valid() we'll use this to read write protect.
  * @ctype: Card type for this slot.
  * @mrq: mmc_request currently being processed or waiting to be
  *	processed, or NULL when the slot is idle.
@@ -88,6 +90,8 @@ struct dw_mci_slot {
 	struct mmc_host		*mmc;
 	struct dw_mci		*host;
 
+	int			wp_gpio;
+
 	u32			ctype;
 
 	struct mmc_request	*mrq;
@@ -832,6 +836,8 @@ static int dw_mci_get_ro(struct mmc_host *mmc)
 		read_only = 0;
 	else if (brd->get_ro)
 		read_only = brd->get_ro(slot->id);
+	else if (gpio_is_valid(slot->wp_gpio))
+		read_only = gpio_get_value(slot->wp_gpio);
 	else
 		read_only =
 			mci_readl(slot->host, WRTPRT) & (1 << slot->id) ? 1 : 0;
@@ -1802,6 +1808,29 @@ static u32 dw_mci_of_get_bus_wd(struct device *dev, u8 slot)
 			       " as 1\n");
 	return bus_wd;
 }
+
+/* find the write protect gpio for a given slot; or -1 if none specified */
+static int dw_mci_of_get_wp_gpio(struct device *dev, u8 slot)
+{
+	struct device_node *np = dw_mci_of_find_slot_node(dev, slot);
+	int gpio;
+
+	if (!np)
+		return -EINVAL;
+
+	gpio = of_get_named_gpio(np, "wp-gpios", 0);
+
+	/* Having a missing entry is valid; return silently */
+	if (!gpio_is_valid(gpio))
+		return -EINVAL;
+
+	if (devm_gpio_request(dev, gpio, "dw-mci-wp")) {
+		dev_warn(dev, "gpio [%d] request failed\n", gpio);
+		return -EINVAL;
+	}
+
+	return gpio;
+}
 #else /* CONFIG_OF */
 static u32 dw_mci_of_get_bus_wd(struct device *dev, u8 slot)
 {
@@ -1811,6 +1840,10 @@ static struct device_node *dw_mci_of_find_slot_node(struct device *dev, u8 slot)
 {
 	return NULL;
 }
+static int dw_mci_of_get_wp_gpio(struct device *dev, u8 slot)
+{
+	return -EINVAL;
+}
 #endif /* CONFIG_OF */
 
 static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
@@ -1923,6 +1956,8 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
 	else
 		clear_bit(DW_MMC_CARD_PRESENT, &slot->flags);
 
+	slot->wp_gpio = dw_mci_of_get_wp_gpio(host->dev, slot->id);
+
 	mmc_add_host(mmc);
 
 #if defined(CONFIG_DEBUG_FS)
-- 
1.7.7.3

RE: [PATCH v2 1/2] mmc: dw_mmc: exynos: Stop claiming wp-gpio

[Seungwon Jeon]

Yes. pin of write protection is common property.
This change is good. I have some suggestion below.
Could you check it?

On Friday, November 23, 2012, Doug Anderson wrote:
> The exynos code claimed wp-gpio with devm_gpio_request() but never did
> anything with it.  That meant that anyone using a write protect GPIO
> would effectively be write protected all the time.
> 
> A future change will move the wp-gpio support to the core dw_mmc.c
> file.  Now the exynos-specific code won't claim the GPIO but will
> just set the DW_MCI_QUIRK_NO_WRITE_PROTECT quirk if write protect
> won't be used.
> 
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> 
> ---
> Changes in v2:
> - Nothing new in this patch
> 
>  drivers/mmc/host/dw_mmc-exynos.c |   12 ++++++------
>  1 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
> index 4d50da6..58cc03e 100644
> --- a/drivers/mmc/host/dw_mmc-exynos.c
> +++ b/drivers/mmc/host/dw_mmc-exynos.c
> @@ -175,12 +175,12 @@ static int dw_mci_exynos_setup_bus(struct dw_mci *host,
>  		}
>  	}
> 
> -	gpio = of_get_named_gpio(slot_np, "wp-gpios", 0);
> -	if (gpio_is_valid(gpio)) {
> -		if (devm_gpio_request(host->dev, gpio, "dw-mci-wp"))
> -			dev_info(host->dev, "gpio [%d] request failed\n",
> -						gpio);
> -	} else {
> +	/*
> +	 * If there are no write-protect GPIOs present then we assume no write
> +	 * protect.  The mci_readl() in dw_mmc.c won't work since it's not
> +	 * hooked up on exynos.
> +	 */
> +	if (!of_find_property(slot_np, "wp-gpios", NULL)) {
>  		dev_info(host->dev, "wp gpio not available");
>  		host->pdata->quirks |= DW_MCI_QUIRK_NO_WRITE_PROTECT;
>  	}
All card types need this quirk in case wp-gpio property is empty?
I think wp-pin is valid for SD card, not eMMC/SDIO.
Of course, I know origin code did it.
How about removing whole checking routine?
Instead, new definition for this quirk can be added into 'dw_mci_of_quirks'(dw_mmc.c) and dts file.

Thanks,
Seungwon Jeon
> --
> 1.7.7.3

Re: [PATCH v2 1/2] mmc: dw_mmc: exynos: Stop claiming wp-gpio

[Doug Anderson]

Seungwon,

Thanks for the review.  See below for comments.  If you'd like me to
respin then please let me know.  Otherwise I look forward to your ack.

On Wed, Nov 28, 2012 at 1:29 AM, Seungwon Jeon <tgih.jun@samsung.com> wrote:
> Yes. pin of write protection is common property.
> This change is good. I have some suggestion below.
> Could you check it?
>
> On Friday, November 23, 2012, Doug Anderson wrote:
>> The exynos code claimed wp-gpio with devm_gpio_request() but never did
>> anything with it.  That meant that anyone using a write protect GPIO
>> would effectively be write protected all the time.
>>
>> A future change will move the wp-gpio support to the core dw_mmc.c
>> file.  Now the exynos-specific code won't claim the GPIO but will
>> just set the DW_MCI_QUIRK_NO_WRITE_PROTECT quirk if write protect
>> won't be used.
>>
>> Signed-off-by: Doug Anderson <dianders@chromium.org>
>>
>> ---
>> Changes in v2:
>> - Nothing new in this patch
>>
>>  drivers/mmc/host/dw_mmc-exynos.c |   12 ++++++------
>>  1 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
>> index 4d50da6..58cc03e 100644
>> --- a/drivers/mmc/host/dw_mmc-exynos.c
>> +++ b/drivers/mmc/host/dw_mmc-exynos.c
>> @@ -175,12 +175,12 @@ static int dw_mci_exynos_setup_bus(struct dw_mci *host,
>>               }
>>       }
>>
>> -     gpio = of_get_named_gpio(slot_np, "wp-gpios", 0);
>> -     if (gpio_is_valid(gpio)) {
>> -             if (devm_gpio_request(host->dev, gpio, "dw-mci-wp"))
>> -                     dev_info(host->dev, "gpio [%d] request failed\n",
>> -                                             gpio);
>> -     } else {
>> +     /*
>> +      * If there are no write-protect GPIOs present then we assume no write
>> +      * protect.  The mci_readl() in dw_mmc.c won't work since it's not
>> +      * hooked up on exynos.
>> +      */
>> +     if (!of_find_property(slot_np, "wp-gpios", NULL)) {
>>               dev_info(host->dev, "wp gpio not available");
>>               host->pdata->quirks |= DW_MCI_QUIRK_NO_WRITE_PROTECT;
>>       }
> All card types need this quirk in case wp-gpio property is empty?
> I think wp-pin is valid for SD card, not eMMC/SDIO.

Right.  It is only checked right now by the SD code (mmc/core/sd.c).
It doesn't particularly hurt to set it the quirk in other cases though
and it seems nice not to add special cases.  I could imagine someone
extending the MMC code at some point to support write protect (via
GPIO) for eMMC, so there's even a slight justification for avoiding
the special case.


> Of course, I know origin code did it.
> How about removing whole checking routine?
> Instead, new definition for this quirk can be added into 'dw_mci_of_quirks'(dw_mmc.c) and dts file.

On _exynos_ all SD cards need this quirk if there is no wp-gpio
property.  However this is not generally true for all users of dw_mmc.
 The DesignWare IP Block actually has a write protect input that can
be read with "mci_readl(slot->host, WRTPRT)" but on exynos the
DesignWare write protect line isn't exposed on any physical pins.
That means that the only possible way to do write protect on exynos is
using a GPIO.

The above means that on exynos if the GPIO isn't defined we will
assume no write protect.  On other platforms if the GPIO isn't defined
we'll assume that the "mci_readl" will work and we'll use that.

If people would prefer it I can code up an alternate solution that
doesn't touch any exynos code but that would introduce a new device
tree binding.  We could accomplish what's needed for exynos using a
property like "broken-internal-wp".

Please let me know if you'd like me to submit a new patch with this
solution or if you like the existing solution.


> Thanks,
> Seungwon Jeon
>> --
>> 1.7.7.3
>

RE: [PATCH v2 1/2] mmc: dw_mmc: exynos: Stop claiming wp-gpio

[Seungwon Jeon]

Hi Doug,

On Thursday, November 29, 2012, Doug Anderson wrote:
> Seungwon,
> 
> Thanks for the review.  See below for comments.  If you'd like me to
> respin then please let me know.  Otherwise I look forward to your ack.
> 
> On Wed, Nov 28, 2012 at 1:29 AM, Seungwon Jeon <tgih.jun@samsung.com> wrote:
> > Yes. pin of write protection is common property.
> > This change is good. I have some suggestion below.
> > Could you check it?
> >
> > On Friday, November 23, 2012, Doug Anderson wrote:
> >> The exynos code claimed wp-gpio with devm_gpio_request() but never did
> >> anything with it.  That meant that anyone using a write protect GPIO
> >> would effectively be write protected all the time.
> >>
> >> A future change will move the wp-gpio support to the core dw_mmc.c
> >> file.  Now the exynos-specific code won't claim the GPIO but will
> >> just set the DW_MCI_QUIRK_NO_WRITE_PROTECT quirk if write protect
> >> won't be used.
> >>
> >> Signed-off-by: Doug Anderson <dianders@chromium.org>
> >>
> >> ---
> >> Changes in v2:
> >> - Nothing new in this patch
> >>
> >>  drivers/mmc/host/dw_mmc-exynos.c |   12 ++++++------
> >>  1 files changed, 6 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
> >> index 4d50da6..58cc03e 100644
> >> --- a/drivers/mmc/host/dw_mmc-exynos.c
> >> +++ b/drivers/mmc/host/dw_mmc-exynos.c
> >> @@ -175,12 +175,12 @@ static int dw_mci_exynos_setup_bus(struct dw_mci *host,
> >>               }
> >>       }
> >>
> >> -     gpio = of_get_named_gpio(slot_np, "wp-gpios", 0);
> >> -     if (gpio_is_valid(gpio)) {
> >> -             if (devm_gpio_request(host->dev, gpio, "dw-mci-wp"))
> >> -                     dev_info(host->dev, "gpio [%d] request failed\n",
> >> -                                             gpio);
> >> -     } else {
> >> +     /*
> >> +      * If there are no write-protect GPIOs present then we assume no write
> >> +      * protect.  The mci_readl() in dw_mmc.c won't work since it's not
> >> +      * hooked up on exynos.
> >> +      */
> >> +     if (!of_find_property(slot_np, "wp-gpios", NULL)) {
> >>               dev_info(host->dev, "wp gpio not available");
> >>               host->pdata->quirks |= DW_MCI_QUIRK_NO_WRITE_PROTECT;
> >>       }
> > All card types need this quirk in case wp-gpio property is empty?
> > I think wp-pin is valid for SD card, not eMMC/SDIO.
> 
> Right.  It is only checked right now by the SD code (mmc/core/sd.c).
> It doesn't particularly hurt to set it the quirk in other cases though
> and it seems nice not to add special cases.  I could imagine someone
> extending the MMC code at some point to support write protect (via
> GPIO) for eMMC, so there's even a slight justification for avoiding
> the special case.
> 
> 
> > Of course, I know origin code did it.
> > How about removing whole checking routine?
> > Instead, new definition for this quirk can be added into 'dw_mci_of_quirks'(dw_mmc.c) and dts file.
> 
> On _exynos_ all SD cards need this quirk if there is no wp-gpio
> property.  However this is not generally true for all users of dw_mmc.
>  The DesignWare IP Block actually has a write protect input that can
> be read with "mci_readl(slot->host, WRTPRT)" but on exynos the
> DesignWare write protect line isn't exposed on any physical pins.
> That means that the only possible way to do write protect on exynos is
> using a GPIO.
> 
> The above means that on exynos if the GPIO isn't defined we will
> assume no write protect.  On other platforms if the GPIO isn't defined
> we'll assume that the "mci_readl" will work and we'll use that.
> 
> If people would prefer it I can code up an alternate solution that
> doesn't touch any exynos code but that would introduce a new device
> tree binding.  We could accomplish what's needed for exynos using a
> property like "broken-internal-wp".
> 
> Please let me know if you'd like me to submit a new patch with this
> solution or if you like the existing solution.
> 
Write protect is additional interface related with SD socket. 
WP switch appears in SD standard size card. 
In case EMMC/SDIO spec, there is no mentions about this WP pin.
As you mentioned above, that's why 'ger_ro' is called only in sd path(mmc/core/sd.c).
So, I meant that we don't need to consider WP pin status about non-SD type.

Such as exynos5250, there is no exposed interface from host controller for write protection pin.
In that case, if general gpio pin is connected like your board environment, we can define wp-gpio.
Otherwise, 'broken-internal-wp' property will be good solution.

Please feel free to modify.
If you will do, I'll be happy.

Thanks,
Seungwon Jeon
> >> --
> >> 1.7.7.3
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 1/4] mmc: dw_mmc: Add "disable-wp" device tree property

[Doug Anderson]

The "disable-wp" property is used to specify that a given SD card slot
doesn't have a concept of write protect.  This eliminates the need for
special case code for SD slots that should never be write protected
(like a micro SD slot or a dev board).

The dw_mmc driver is special in needing to specify "disable-wp"
because the lack of a "wp-gpios" property means to use the special
purpose write protect line.  On some other mmc devices the lack of
"wp-gpios" means that write protect should be disabled.

Signed-off-by: Doug Anderson <dianders@chromium.org>
---
Changes in v3:
- New for this version of the patch series.  Chose "disable-wp" rather
  than the discussed "broken-internal-wp" since it mapped more cleanly
  to an existing quirk (and the only reason to specify that the
  internal wp is broken is if you're disabling the write protect
  anyway).

 .../devicetree/bindings/mmc/synopsis-dw-mshc.txt   |   12 +++++-
 drivers/mmc/host/dw_mmc.c                          |   36 +++++++++++++++++++-
 include/linux/mmc/dw_mmc.h                         |    4 ++
 3 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt
index 06cd32d08..726fd21 100644
--- a/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt
+++ b/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt
@@ -26,8 +26,16 @@ Required Properties:
 	* bus-width: as documented in mmc core bindings.
 
 	* wp-gpios: specifies the write protect gpio line. The format of the
-	  gpio specifier depends on the gpio controller. If the write-protect
-	  line is not available, this property is optional.
+	  gpio specifier depends on the gpio controller. If a GPIO is not used
+	  for write-protect, this property is optional.
+
+	* disable-wp: If the wp-gpios property isn't present then (by default)
+	  we'd assume that the write protect is hooked up directly to the
+	  controller's special purpose write protect line (accessible via
+	  the WRTPRT register).  However, it's possible that we simply don't
+	  want write protect.  In that case specify 'disable-wp'.
+	  NOTE: This property is not required for slots known to always
+	  connect to eMMC or SDIO cards.
 
 Optional properties:
 
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 7342029..b47b1e9 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -74,6 +74,7 @@ struct idmac_desc {
  * struct dw_mci_slot - MMC slot state
  * @mmc: The mmc_host representing this slot.
  * @host: The MMC controller this slot is using.
+ * @quirks: Slot-level quirks (DW_MCI_SLOT_QUIRK_XXX)
  * @ctype: Card type for this slot.
  * @mrq: mmc_request currently being processed or waiting to be
  *	processed, or NULL when the slot is idle.
@@ -88,6 +89,8 @@ struct dw_mci_slot {
 	struct mmc_host		*mmc;
 	struct dw_mci		*host;
 
+	int			quirks;
+
 	u32			ctype;
 
 	struct mmc_request	*mrq;
@@ -828,7 +831,8 @@ static int dw_mci_get_ro(struct mmc_host *mmc)
 	struct dw_mci_board *brd = slot->host->pdata;
 
 	/* Use platform get_ro function, else try on board write protect */
-	if (brd->quirks & DW_MCI_QUIRK_NO_WRITE_PROTECT)
+	if ((brd->quirks & DW_MCI_QUIRK_NO_WRITE_PROTECT) ||
+	    (slot->quirks & DW_MCI_SLOT_QUIRK_NO_WRITE_PROTECT))
 		read_only = 0;
 	else if (brd->get_ro)
 		read_only = brd->get_ro(slot->id);
@@ -1788,6 +1792,30 @@ static struct device_node *dw_mci_of_find_slot_node(struct device *dev, u8 slot)
 	return NULL;
 }
 
+static struct dw_mci_of_slot_quirks {
+	char *quirk;
+	int id;
+} of_slot_quirks[] = {
+	{
+		.quirk	= "disable-wp",
+		.id	= DW_MCI_SLOT_QUIRK_NO_WRITE_PROTECT,
+	},
+};
+
+static int dw_mci_of_get_slot_quirks(struct device *dev, u8 slot)
+{
+	struct device_node *np = dw_mci_of_find_slot_node(dev, slot);
+	int quirks = 0;
+	int idx;
+
+	/* get quirks */
+	for (idx = 0; idx < ARRAY_SIZE(of_slot_quirks); idx++)
+		if (of_get_property(np, of_slot_quirks[idx].quirk, NULL))
+			quirks |= of_slot_quirks[idx].id;
+
+	return quirks;
+}
+
 /* find out bus-width for a given slot */
 static u32 dw_mci_of_get_bus_wd(struct device *dev, u8 slot)
 {
@@ -1803,6 +1831,10 @@ static u32 dw_mci_of_get_bus_wd(struct device *dev, u8 slot)
 	return bus_wd;
 }
 #else /* CONFIG_OF */
+static int dw_mci_of_get_slot_quirks(struct device *dev, u8 slot)
+{
+	return 0;
+}
 static u32 dw_mci_of_get_bus_wd(struct device *dev, u8 slot)
 {
 	return 1;
@@ -1831,6 +1863,8 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
 	slot->host = host;
 	host->slot[id] = slot;
 
+	slot->quirks = dw_mci_of_get_slot_quirks(host->dev, slot->id);
+
 	mmc->ops = &dw_mci_ops;
 	mmc->f_min = DIV_ROUND_UP(host->bus_hz, 510);
 	mmc->f_max = host->bus_hz;
diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
index 34be4f4..24dc3a8 100644
--- a/include/linux/mmc/dw_mmc.h
+++ b/include/linux/mmc/dw_mmc.h
@@ -212,6 +212,10 @@ struct dw_mci_dma_ops {
 /* Write Protect detection not available */
 #define DW_MCI_QUIRK_NO_WRITE_PROTECT		BIT(4)
 
+/* Slot level quirks */
+/* This slot has no write protect */
+#define DW_MCI_SLOT_QUIRK_NO_WRITE_PROTECT	BIT(0)
+
 struct dma_pdata;
 
 struct block_settings {
-- 
1.7.7.3

[PATCH v3 2/4] ARM: dts: Add disable-wp for sd card slot on smdk5250

[Doug Anderson]

The next change will remove the code from the dw_mmc-exynos that added
the DW_MCI_QUIRK_NO_WRITE_PROTECT.  Keep existing functionality of
having no write protect pin on smdk5250 by adding the disable-wp
property.

Signed-off-by: Doug Anderson <dianders@chromium.org>
---
Changes in v3:
- New for this version of the patch series.

 arch/arm/boot/dts/exynos5250-smdk5250.dts |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/exynos5250-smdk5250.dts b/arch/arm/boot/dts/exynos5250-smdk5250.dts
index f30ca18..5538b13 100644
--- a/arch/arm/boot/dts/exynos5250-smdk5250.dts
+++ b/arch/arm/boot/dts/exynos5250-smdk5250.dts
@@ -146,6 +146,7 @@
 			reg = <0>;
 			bus-width = <4>;
 			samsung,cd-pinmux-gpio = <&gpc3 2 2 3 3>;
+			disable-wp;
 			gpios = <&gpc3 0 2 0 3>, <&gpc3 1 2 0 3>,
 				<&gpc3 3 2 3 3>, <&gpc3 4 2 3 3>,
 				<&gpc3 5 2 3 3>, <&gpc3 6 2 3 3>,
-- 
1.7.7.3

[PATCH v3 3/4] mmc: dw_mmc: exynos: Remove code for wp-gpios

[Doug Anderson]

The exynos code claimed the write protect with devm_gpio_request() but
never did anything with it.  That meant that anyone using a write
protect GPIO would effectively be write protected all the time.

The handling for wp-gpios belongs in the main dw_mmc driver and has
been moved there.

Signed-off-by: Doug Anderson <dianders@chromium.org>
---
Changes in v3:
- Totally removed wp-gpios handling from exynos code.

Changes in v2: None

 drivers/mmc/host/dw_mmc-exynos.c |   10 ----------
 1 files changed, 0 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
index 4d50da6..72fd0f2 100644
--- a/drivers/mmc/host/dw_mmc-exynos.c
+++ b/drivers/mmc/host/dw_mmc-exynos.c
@@ -175,16 +175,6 @@ static int dw_mci_exynos_setup_bus(struct dw_mci *host,
 		}
 	}
 
-	gpio = of_get_named_gpio(slot_np, "wp-gpios", 0);
-	if (gpio_is_valid(gpio)) {
-		if (devm_gpio_request(host->dev, gpio, "dw-mci-wp"))
-			dev_info(host->dev, "gpio [%d] request failed\n",
-						gpio);
-	} else {
-		dev_info(host->dev, "wp gpio not available");
-		host->pdata->quirks |= DW_MCI_QUIRK_NO_WRITE_PROTECT;
-	}
-
 	if (host->pdata->quirks & DW_MCI_QUIRK_BROKEN_CARD_DETECTION)
 		return 0;
 
-- 
1.7.7.3

[PATCH v3 4/4] mmc: dw_mmc: Handle wp-gpios from device tree

[Doug Anderson]

On some SoCs (like exynos5250) you need to use an external GPIO for
write protect.  Add support for wp-gpios to the core dw_mmc driver
since it could be useful across multiple SoCs.

With this change I am able to make use of the write protect for the
external SD slot on exynos5250-snow.

Signed-off-by: Doug Anderson <dianders@chromium.org>
---
Changes in v3: None
Changes in v2:
- Fixed return type from u32 to int
- Return -EINVAL instead of -1

 drivers/mmc/host/dw_mmc.c |   34 ++++++++++++++++++++++++++++++++++
 1 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index b47b1e9..8f8bac5 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -34,6 +34,7 @@
 #include <linux/regulator/consumer.h>
 #include <linux/workqueue.h>
 #include <linux/of.h>
+#include <linux/of_gpio.h>
 
 #include "dw_mmc.h"
 
@@ -75,6 +76,7 @@ struct idmac_desc {
  * @mmc: The mmc_host representing this slot.
  * @host: The MMC controller this slot is using.
  * @quirks: Slot-level quirks (DW_MCI_SLOT_QUIRK_XXX)
+ * @wp_gpio: If gpio_is_valid() we'll use this to read write protect.
  * @ctype: Card type for this slot.
  * @mrq: mmc_request currently being processed or waiting to be
  *	processed, or NULL when the slot is idle.
@@ -90,6 +92,7 @@ struct dw_mci_slot {
 	struct dw_mci		*host;
 
 	int			quirks;
+	int			wp_gpio;
 
 	u32			ctype;
 
@@ -836,6 +839,8 @@ static int dw_mci_get_ro(struct mmc_host *mmc)
 		read_only = 0;
 	else if (brd->get_ro)
 		read_only = brd->get_ro(slot->id);
+	else if (gpio_is_valid(slot->wp_gpio))
+		read_only = gpio_get_value(slot->wp_gpio);
 	else
 		read_only =
 			mci_readl(slot->host, WRTPRT) & (1 << slot->id) ? 1 : 0;
@@ -1830,6 +1835,29 @@ static u32 dw_mci_of_get_bus_wd(struct device *dev, u8 slot)
 			       " as 1\n");
 	return bus_wd;
 }
+
+/* find the write protect gpio for a given slot; or -1 if none specified */
+static int dw_mci_of_get_wp_gpio(struct device *dev, u8 slot)
+{
+	struct device_node *np = dw_mci_of_find_slot_node(dev, slot);
+	int gpio;
+
+	if (!np)
+		return -EINVAL;
+
+	gpio = of_get_named_gpio(np, "wp-gpios", 0);
+
+	/* Having a missing entry is valid; return silently */
+	if (!gpio_is_valid(gpio))
+		return -EINVAL;
+
+	if (devm_gpio_request(dev, gpio, "dw-mci-wp")) {
+		dev_warn(dev, "gpio [%d] request failed\n", gpio);
+		return -EINVAL;
+	}
+
+	return gpio;
+}
 #else /* CONFIG_OF */
 static int dw_mci_of_get_slot_quirks(struct device *dev, u8 slot)
 {
@@ -1843,6 +1871,10 @@ static struct device_node *dw_mci_of_find_slot_node(struct device *dev, u8 slot)
 {
 	return NULL;
 }
+static int dw_mci_of_get_wp_gpio(struct device *dev, u8 slot)
+{
+	return -EINVAL;
+}
 #endif /* CONFIG_OF */
 
 static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
@@ -1960,6 +1992,8 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
 	else
 		clear_bit(DW_MMC_CARD_PRESENT, &slot->flags);
 
+	slot->wp_gpio = dw_mci_of_get_wp_gpio(host->dev, slot->id);
+
 	mmc_add_host(mmc);
 
 #if defined(CONFIG_DEBUG_FS)
-- 
1.7.7.3

Re: [PATCH v2 1/2] mmc: dw_mmc: exynos: Stop claiming wp-gpio

[Doug Anderson]

Seungwon,


On Wed, Nov 28, 2012 at 11:46 PM, Seungwon Jeon <tgih.jun@samsung.com> wrote:
> Hi Doug,
>
> On Thursday, November 29, 2012, Doug Anderson wrote:
>> Seungwon,
>>
>> Thanks for the review.  See below for comments.  If you'd like me to
>> respin then please let me know.  Otherwise I look forward to your ack.
>>
>> On Wed, Nov 28, 2012 at 1:29 AM, Seungwon Jeon <tgih.jun@samsung.com> wrote:
>> > Yes. pin of write protection is common property.
>> > This change is good. I have some suggestion below.
>> > Could you check it?
>> >
>> > On Friday, November 23, 2012, Doug Anderson wrote:
>> >> The exynos code claimed wp-gpio with devm_gpio_request() but never did
>> >> anything with it.  That meant that anyone using a write protect GPIO
>> >> would effectively be write protected all the time.
>> >>
>> >> A future change will move the wp-gpio support to the core dw_mmc.c
>> >> file.  Now the exynos-specific code won't claim the GPIO but will
>> >> just set the DW_MCI_QUIRK_NO_WRITE_PROTECT quirk if write protect
>> >> won't be used.
>> >>
>> >> Signed-off-by: Doug Anderson <dianders@chromium.org>
>> >>
>> >> ---
>> >> Changes in v2:
>> >> - Nothing new in this patch
>> >>
>> >>  drivers/mmc/host/dw_mmc-exynos.c |   12 ++++++------
>> >>  1 files changed, 6 insertions(+), 6 deletions(-)
>> >>
>> >> diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
>> >> index 4d50da6..58cc03e 100644
>> >> --- a/drivers/mmc/host/dw_mmc-exynos.c
>> >> +++ b/drivers/mmc/host/dw_mmc-exynos.c
>> >> @@ -175,12 +175,12 @@ static int dw_mci_exynos_setup_bus(struct dw_mci *host,
>> >>               }
>> >>       }
>> >>
>> >> -     gpio = of_get_named_gpio(slot_np, "wp-gpios", 0);
>> >> -     if (gpio_is_valid(gpio)) {
>> >> -             if (devm_gpio_request(host->dev, gpio, "dw-mci-wp"))
>> >> -                     dev_info(host->dev, "gpio [%d] request failed\n",
>> >> -                                             gpio);
>> >> -     } else {
>> >> +     /*
>> >> +      * If there are no write-protect GPIOs present then we assume no write
>> >> +      * protect.  The mci_readl() in dw_mmc.c won't work since it's not
>> >> +      * hooked up on exynos.
>> >> +      */
>> >> +     if (!of_find_property(slot_np, "wp-gpios", NULL)) {
>> >>               dev_info(host->dev, "wp gpio not available");
>> >>               host->pdata->quirks |= DW_MCI_QUIRK_NO_WRITE_PROTECT;
>> >>       }
>> > All card types need this quirk in case wp-gpio property is empty?
>> > I think wp-pin is valid for SD card, not eMMC/SDIO.
>>
>> Right.  It is only checked right now by the SD code (mmc/core/sd.c).
>> It doesn't particularly hurt to set it the quirk in other cases though
>> and it seems nice not to add special cases.  I could imagine someone
>> extending the MMC code at some point to support write protect (via
>> GPIO) for eMMC, so there's even a slight justification for avoiding
>> the special case.
>>
>>
>> > Of course, I know origin code did it.
>> > How about removing whole checking routine?
>> > Instead, new definition for this quirk can be added into 'dw_mci_of_quirks'(dw_mmc.c) and dts file.
>>
>> On _exynos_ all SD cards need this quirk if there is no wp-gpio
>> property.  However this is not generally true for all users of dw_mmc.
>>  The DesignWare IP Block actually has a write protect input that can
>> be read with "mci_readl(slot->host, WRTPRT)" but on exynos the
>> DesignWare write protect line isn't exposed on any physical pins.
>> That means that the only possible way to do write protect on exynos is
>> using a GPIO.
>>
>> The above means that on exynos if the GPIO isn't defined we will
>> assume no write protect.  On other platforms if the GPIO isn't defined
>> we'll assume that the "mci_readl" will work and we'll use that.
>>
>> If people would prefer it I can code up an alternate solution that
>> doesn't touch any exynos code but that would introduce a new device
>> tree binding.  We could accomplish what's needed for exynos using a
>> property like "broken-internal-wp".
>>
>> Please let me know if you'd like me to submit a new patch with this
>> solution or if you like the existing solution.
>>
> Write protect is additional interface related with SD socket.
> WP switch appears in SD standard size card.
> In case EMMC/SDIO spec, there is no mentions about this WP pin.
> As you mentioned above, that's why 'ger_ro' is called only in sd path(mmc/core/sd.c).
> So, I meant that we don't need to consider WP pin status about non-SD type.

Ah, I understand now.  This is a good point.  I have updated the
documentation in the latest patch to mention this.  Thanks!


>
> Such as exynos5250, there is no exposed interface from host controller for write protection pin.
> In that case, if general gpio pin is connected like your board environment, we can define wp-gpio.
> Otherwise, 'broken-internal-wp' property will be good solution.

Latest patch (just about to send out) adds a per-slot "disable-wp"
property for dw_mmc.  See the patch for why I've chosen to do it that
way.  Hopefully it looks OK to you.


>
> Please feel free to modify.
> If you will do, I'll be happy.
>
> Thanks,
> Seungwon Jeon
>> >> --
>> >> 1.7.7.3
>> >
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

RE: [PATCH v3 1/4] mmc: dw_mmc: Add "disable-wp" device tree property

[Seungwon Jeon]

Doug, Thanks to work.
Looks good to me with other patches.

Acked-by: Seungwon Jeon <tgih.jun@samsung.com>

On Friday, November 30, 2012, Doug Anderson wrote:
> The "disable-wp" property is used to specify that a given SD card slot
> doesn't have a concept of write protect.  This eliminates the need for
> special case code for SD slots that should never be write protected
> (like a micro SD slot or a dev board).
> 
> The dw_mmc driver is special in needing to specify "disable-wp"
> because the lack of a "wp-gpios" property means to use the special
> purpose write protect line.  On some other mmc devices the lack of
> "wp-gpios" means that write protect should be disabled.
> 
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> ---
> Changes in v3:
> - New for this version of the patch series.  Chose "disable-wp" rather
>   than the discussed "broken-internal-wp" since it mapped more cleanly
>   to an existing quirk (and the only reason to specify that the
>   internal wp is broken is if you're disabling the write protect
>   anyway).
> 
>  .../devicetree/bindings/mmc/synopsis-dw-mshc.txt   |   12 +++++-
>  drivers/mmc/host/dw_mmc.c                          |   36 +++++++++++++++++++-
>  include/linux/mmc/dw_mmc.h                         |    4 ++
>  3 files changed, 49 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt
> b/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt
> index 06cd32d08..726fd21 100644
> --- a/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt
> +++ b/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt
> @@ -26,8 +26,16 @@ Required Properties:
>  	* bus-width: as documented in mmc core bindings.
> 
>  	* wp-gpios: specifies the write protect gpio line. The format of the
> -	  gpio specifier depends on the gpio controller. If the write-protect
> -	  line is not available, this property is optional.
> +	  gpio specifier depends on the gpio controller. If a GPIO is not used
> +	  for write-protect, this property is optional.
> +
> +	* disable-wp: If the wp-gpios property isn't present then (by default)
> +	  we'd assume that the write protect is hooked up directly to the
> +	  controller's special purpose write protect line (accessible via
> +	  the WRTPRT register).  However, it's possible that we simply don't
> +	  want write protect.  In that case specify 'disable-wp'.
> +	  NOTE: This property is not required for slots known to always
> +	  connect to eMMC or SDIO cards.
> 
>  Optional properties:
> 
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 7342029..b47b1e9 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -74,6 +74,7 @@ struct idmac_desc {
>   * struct dw_mci_slot - MMC slot state
>   * @mmc: The mmc_host representing this slot.
>   * @host: The MMC controller this slot is using.
> + * @quirks: Slot-level quirks (DW_MCI_SLOT_QUIRK_XXX)
>   * @ctype: Card type for this slot.
>   * @mrq: mmc_request currently being processed or waiting to be
>   *	processed, or NULL when the slot is idle.
> @@ -88,6 +89,8 @@ struct dw_mci_slot {
>  	struct mmc_host		*mmc;
>  	struct dw_mci		*host;
> 
> +	int			quirks;
> +
>  	u32			ctype;
> 
>  	struct mmc_request	*mrq;
> @@ -828,7 +831,8 @@ static int dw_mci_get_ro(struct mmc_host *mmc)
>  	struct dw_mci_board *brd = slot->host->pdata;
> 
>  	/* Use platform get_ro function, else try on board write protect */
> -	if (brd->quirks & DW_MCI_QUIRK_NO_WRITE_PROTECT)
> +	if ((brd->quirks & DW_MCI_QUIRK_NO_WRITE_PROTECT) ||
> +	    (slot->quirks & DW_MCI_SLOT_QUIRK_NO_WRITE_PROTECT))
>  		read_only = 0;
>  	else if (brd->get_ro)
>  		read_only = brd->get_ro(slot->id);
> @@ -1788,6 +1792,30 @@ static struct device_node *dw_mci_of_find_slot_node(struct device *dev, u8 slot)
>  	return NULL;
>  }
> 
> +static struct dw_mci_of_slot_quirks {
> +	char *quirk;
> +	int id;
> +} of_slot_quirks[] = {
> +	{
> +		.quirk	= "disable-wp",
> +		.id	= DW_MCI_SLOT_QUIRK_NO_WRITE_PROTECT,
> +	},
> +};
> +
> +static int dw_mci_of_get_slot_quirks(struct device *dev, u8 slot)
> +{
> +	struct device_node *np = dw_mci_of_find_slot_node(dev, slot);
> +	int quirks = 0;
> +	int idx;
> +
> +	/* get quirks */
> +	for (idx = 0; idx < ARRAY_SIZE(of_slot_quirks); idx++)
> +		if (of_get_property(np, of_slot_quirks[idx].quirk, NULL))
> +			quirks |= of_slot_quirks[idx].id;
> +
> +	return quirks;
> +}
> +
>  /* find out bus-width for a given slot */
>  static u32 dw_mci_of_get_bus_wd(struct device *dev, u8 slot)
>  {
> @@ -1803,6 +1831,10 @@ static u32 dw_mci_of_get_bus_wd(struct device *dev, u8 slot)
>  	return bus_wd;
>  }
>  #else /* CONFIG_OF */
> +static int dw_mci_of_get_slot_quirks(struct device *dev, u8 slot)
> +{
> +	return 0;
> +}
>  static u32 dw_mci_of_get_bus_wd(struct device *dev, u8 slot)
>  {
>  	return 1;
> @@ -1831,6 +1863,8 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
>  	slot->host = host;
>  	host->slot[id] = slot;
> 
> +	slot->quirks = dw_mci_of_get_slot_quirks(host->dev, slot->id);
> +
>  	mmc->ops = &dw_mci_ops;
>  	mmc->f_min = DIV_ROUND_UP(host->bus_hz, 510);
>  	mmc->f_max = host->bus_hz;
> diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
> index 34be4f4..24dc3a8 100644
> --- a/include/linux/mmc/dw_mmc.h
> +++ b/include/linux/mmc/dw_mmc.h
> @@ -212,6 +212,10 @@ struct dw_mci_dma_ops {
>  /* Write Protect detection not available */
>  #define DW_MCI_QUIRK_NO_WRITE_PROTECT		BIT(4)
> 
> +/* Slot level quirks */
> +/* This slot has no write protect */
> +#define DW_MCI_SLOT_QUIRK_NO_WRITE_PROTECT	BIT(0)
> +
>  struct dma_pdata;
> 
>  struct block_settings {
> --
> 1.7.7.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/4] mmc: dw_mmc: Add "disable-wp" device tree property

[Doug Anderson]

Chris,

On Fri, Nov 30, 2012 at 3:57 AM, Seungwon Jeon <tgih.jun@samsung.com> wrote:
> Doug, Thanks to work.
> Looks good to me with other patches.
>
> Acked-by: Seungwon Jeon <tgih.jun@samsung.com>

Does this series look reasonable to you?  I can check back later when
things are less hectic, but I wanted to make sure this wasn't
forgotten.

Thanks much!  :)

-Doug

[REPOST PATCH v3 1/4] mmc: dw_mmc: Add "disable-wp" device tree property

[Doug Anderson]

The "disable-wp" property is used to specify that a given SD card slot
doesn't have a concept of write protect.  This eliminates the need for
special case code for SD slots that should never be write protected
(like a micro SD slot or a dev board).

The dw_mmc driver is special in needing to specify "disable-wp"
because the lack of a "wp-gpios" property means to use the special
purpose write protect line.  On some other mmc devices the lack of
"wp-gpios" means that write protect should be disabled.

Signed-off-by: Doug Anderson <dianders@chromium.org>
Acked-by: Seungwon Jeon <tgih.jun@samsung.com>
---
Changes in v3:
- New for this version of the patch series.  Chose "disable-wp" rather
  than the discussed "broken-internal-wp" since it mapped more cleanly
  to an existing quirk (and the only reason to specify that the
  internal wp is broken is if you're disabling the write protect
  anyway).
- Reposted series 3 with Seungwon's ack, since it hasn't been picked
  up by anyone.

 .../devicetree/bindings/mmc/synopsis-dw-mshc.txt   |   12 +++++-
 drivers/mmc/host/dw_mmc.c                          |   36 +++++++++++++++++++-
 include/linux/mmc/dw_mmc.h                         |    4 ++
 3 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt
index 06cd32d08..726fd21 100644
--- a/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt
+++ b/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt
@@ -26,8 +26,16 @@ Required Properties:
 	* bus-width: as documented in mmc core bindings.
 
 	* wp-gpios: specifies the write protect gpio line. The format of the
-	  gpio specifier depends on the gpio controller. If the write-protect
-	  line is not available, this property is optional.
+	  gpio specifier depends on the gpio controller. If a GPIO is not used
+	  for write-protect, this property is optional.
+
+	* disable-wp: If the wp-gpios property isn't present then (by default)
+	  we'd assume that the write protect is hooked up directly to the
+	  controller's special purpose write protect line (accessible via
+	  the WRTPRT register).  However, it's possible that we simply don't
+	  want write protect.  In that case specify 'disable-wp'.
+	  NOTE: This property is not required for slots known to always
+	  connect to eMMC or SDIO cards.
 
 Optional properties:
 
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 323c502..bc0b030 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -74,6 +74,7 @@ struct idmac_desc {
  * struct dw_mci_slot - MMC slot state
  * @mmc: The mmc_host representing this slot.
  * @host: The MMC controller this slot is using.
+ * @quirks: Slot-level quirks (DW_MCI_SLOT_QUIRK_XXX)
  * @ctype: Card type for this slot.
  * @mrq: mmc_request currently being processed or waiting to be
  *	processed, or NULL when the slot is idle.
@@ -88,6 +89,8 @@ struct dw_mci_slot {
 	struct mmc_host		*mmc;
 	struct dw_mci		*host;
 
+	int			quirks;
+
 	u32			ctype;
 
 	struct mmc_request	*mrq;
@@ -825,7 +828,8 @@ static int dw_mci_get_ro(struct mmc_host *mmc)
 	struct dw_mci_board *brd = slot->host->pdata;
 
 	/* Use platform get_ro function, else try on board write protect */
-	if (brd->quirks & DW_MCI_QUIRK_NO_WRITE_PROTECT)
+	if ((brd->quirks & DW_MCI_QUIRK_NO_WRITE_PROTECT) ||
+	    (slot->quirks & DW_MCI_SLOT_QUIRK_NO_WRITE_PROTECT))
 		read_only = 0;
 	else if (brd->get_ro)
 		read_only = brd->get_ro(slot->id);
@@ -1785,6 +1789,30 @@ static struct device_node *dw_mci_of_find_slot_node(struct device *dev, u8 slot)
 	return NULL;
 }
 
+static struct dw_mci_of_slot_quirks {
+	char *quirk;
+	int id;
+} of_slot_quirks[] = {
+	{
+		.quirk	= "disable-wp",
+		.id	= DW_MCI_SLOT_QUIRK_NO_WRITE_PROTECT,
+	},
+};
+
+static int dw_mci_of_get_slot_quirks(struct device *dev, u8 slot)
+{
+	struct device_node *np = dw_mci_of_find_slot_node(dev, slot);
+	int quirks = 0;
+	int idx;
+
+	/* get quirks */
+	for (idx = 0; idx < ARRAY_SIZE(of_slot_quirks); idx++)
+		if (of_get_property(np, of_slot_quirks[idx].quirk, NULL))
+			quirks |= of_slot_quirks[idx].id;
+
+	return quirks;
+}
+
 /* find out bus-width for a given slot */
 static u32 dw_mci_of_get_bus_wd(struct device *dev, u8 slot)
 {
@@ -1800,6 +1828,10 @@ static u32 dw_mci_of_get_bus_wd(struct device *dev, u8 slot)
 	return bus_wd;
 }
 #else /* CONFIG_OF */
+static int dw_mci_of_get_slot_quirks(struct device *dev, u8 slot)
+{
+	return 0;
+}
 static u32 dw_mci_of_get_bus_wd(struct device *dev, u8 slot)
 {
 	return 1;
@@ -1828,6 +1860,8 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
 	slot->host = host;
 	host->slot[id] = slot;
 
+	slot->quirks = dw_mci_of_get_slot_quirks(host->dev, slot->id);
+
 	mmc->ops = &dw_mci_ops;
 	mmc->f_min = DIV_ROUND_UP(host->bus_hz, 510);
 	mmc->f_max = host->bus_hz;
diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
index 34be4f4..24dc3a8 100644
--- a/include/linux/mmc/dw_mmc.h
+++ b/include/linux/mmc/dw_mmc.h
@@ -212,6 +212,10 @@ struct dw_mci_dma_ops {
 /* Write Protect detection not available */
 #define DW_MCI_QUIRK_NO_WRITE_PROTECT		BIT(4)
 
+/* Slot level quirks */
+/* This slot has no write protect */
+#define DW_MCI_SLOT_QUIRK_NO_WRITE_PROTECT	BIT(0)
+
 struct dma_pdata;
 
 struct block_settings {
-- 
1.7.7.3

[REPOST PATCH v3 2/4] ARM: dts: Add disable-wp for sd card slot on smdk5250

[Doug Anderson]

The next change will remove the code from the dw_mmc-exynos that added
the DW_MCI_QUIRK_NO_WRITE_PROTECT.  Keep existing functionality of
having no write protect pin on smdk5250 by adding the disable-wp
property.

Signed-off-by: Doug Anderson <dianders@chromium.org>
Acked-by: Seungwon Jeon <tgih.jun@samsung.com>
---
Changes in v3:
- New for this version of the patch series.

 arch/arm/boot/dts/exynos5250-smdk5250.dts |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/exynos5250-smdk5250.dts b/arch/arm/boot/dts/exynos5250-smdk5250.dts
index 942d576..75b9d83 100644
--- a/arch/arm/boot/dts/exynos5250-smdk5250.dts
+++ b/arch/arm/boot/dts/exynos5250-smdk5250.dts
@@ -146,6 +146,7 @@
 			reg = <0>;
 			bus-width = <4>;
 			samsung,cd-pinmux-gpio = <&gpc3 2 2 3 3>;
+			disable-wp;
 			gpios = <&gpc3 0 2 0 3>, <&gpc3 1 2 0 3>,
 				<&gpc3 3 2 3 3>, <&gpc3 4 2 3 3>,
 				<&gpc3 5 2 3 3>, <&gpc3 6 2 3 3>,
-- 
1.7.7.3

[REPOST PATCH v3 3/4] mmc: dw_mmc: exynos: Remove code for wp-gpios

[Doug Anderson]

The exynos code claimed the write protect with devm_gpio_request() but
never did anything with it.  That meant that anyone using a write
protect GPIO would effectively be write protected all the time.

The handling for wp-gpios belongs in the main dw_mmc driver and has
been moved there.

Signed-off-by: Doug Anderson <dianders@chromium.org>
Acked-by: Seungwon Jeon <tgih.jun@samsung.com>
---
Changes in v3:
- Totally removed wp-gpios handling from exynos code.

Changes in v2: None

 drivers/mmc/host/dw_mmc-exynos.c |   10 ----------
 1 files changed, 0 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
index 4d50da6..72fd0f2 100644
--- a/drivers/mmc/host/dw_mmc-exynos.c
+++ b/drivers/mmc/host/dw_mmc-exynos.c
@@ -175,16 +175,6 @@ static int dw_mci_exynos_setup_bus(struct dw_mci *host,
 		}
 	}
 
-	gpio = of_get_named_gpio(slot_np, "wp-gpios", 0);
-	if (gpio_is_valid(gpio)) {
-		if (devm_gpio_request(host->dev, gpio, "dw-mci-wp"))
-			dev_info(host->dev, "gpio [%d] request failed\n",
-						gpio);
-	} else {
-		dev_info(host->dev, "wp gpio not available");
-		host->pdata->quirks |= DW_MCI_QUIRK_NO_WRITE_PROTECT;
-	}
-
 	if (host->pdata->quirks & DW_MCI_QUIRK_BROKEN_CARD_DETECTION)
 		return 0;
 
-- 
1.7.7.3

[REPOST PATCH v3 4/4] mmc: dw_mmc: Handle wp-gpios from device tree

[Doug Anderson]

On some SoCs (like exynos5250) you need to use an external GPIO for
write protect.  Add support for wp-gpios to the core dw_mmc driver
since it could be useful across multiple SoCs.

With this change I am able to make use of the write protect for the
external SD slot on exynos5250-snow.

Signed-off-by: Doug Anderson <dianders@chromium.org>
Acked-by: Seungwon Jeon <tgih.jun@samsung.com>
---
Changes in v3: None
Changes in v2:
- Fixed return type from u32 to int
- Return -EINVAL instead of -1

 drivers/mmc/host/dw_mmc.c |   34 ++++++++++++++++++++++++++++++++++
 1 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index bc0b030..e7e22ad 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -34,6 +34,7 @@
 #include <linux/regulator/consumer.h>
 #include <linux/workqueue.h>
 #include <linux/of.h>
+#include <linux/of_gpio.h>
 
 #include "dw_mmc.h"
 
@@ -75,6 +76,7 @@ struct idmac_desc {
  * @mmc: The mmc_host representing this slot.
  * @host: The MMC controller this slot is using.
  * @quirks: Slot-level quirks (DW_MCI_SLOT_QUIRK_XXX)
+ * @wp_gpio: If gpio_is_valid() we'll use this to read write protect.
  * @ctype: Card type for this slot.
  * @mrq: mmc_request currently being processed or waiting to be
  *	processed, or NULL when the slot is idle.
@@ -90,6 +92,7 @@ struct dw_mci_slot {
 	struct dw_mci		*host;
 
 	int			quirks;
+	int			wp_gpio;
 
 	u32			ctype;
 
@@ -833,6 +836,8 @@ static int dw_mci_get_ro(struct mmc_host *mmc)
 		read_only = 0;
 	else if (brd->get_ro)
 		read_only = brd->get_ro(slot->id);
+	else if (gpio_is_valid(slot->wp_gpio))
+		read_only = gpio_get_value(slot->wp_gpio);
 	else
 		read_only =
 			mci_readl(slot->host, WRTPRT) & (1 << slot->id) ? 1 : 0;
@@ -1827,6 +1832,29 @@ static u32 dw_mci_of_get_bus_wd(struct device *dev, u8 slot)
 			       " as 1\n");
 	return bus_wd;
 }
+
+/* find the write protect gpio for a given slot; or -1 if none specified */
+static int dw_mci_of_get_wp_gpio(struct device *dev, u8 slot)
+{
+	struct device_node *np = dw_mci_of_find_slot_node(dev, slot);
+	int gpio;
+
+	if (!np)
+		return -EINVAL;
+
+	gpio = of_get_named_gpio(np, "wp-gpios", 0);
+
+	/* Having a missing entry is valid; return silently */
+	if (!gpio_is_valid(gpio))
+		return -EINVAL;
+
+	if (devm_gpio_request(dev, gpio, "dw-mci-wp")) {
+		dev_warn(dev, "gpio [%d] request failed\n", gpio);
+		return -EINVAL;
+	}
+
+	return gpio;
+}
 #else /* CONFIG_OF */
 static int dw_mci_of_get_slot_quirks(struct device *dev, u8 slot)
 {
@@ -1840,6 +1868,10 @@ static struct device_node *dw_mci_of_find_slot_node(struct device *dev, u8 slot)
 {
 	return NULL;
 }
+static int dw_mci_of_get_wp_gpio(struct device *dev, u8 slot)
+{
+	return -EINVAL;
+}
 #endif /* CONFIG_OF */
 
 static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
@@ -1957,6 +1989,8 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
 	else
 		clear_bit(DW_MMC_CARD_PRESENT, &slot->flags);
 
+	slot->wp_gpio = dw_mci_of_get_wp_gpio(host->dev, slot->id);
+
 	mmc_add_host(mmc);
 
 #if defined(CONFIG_DEBUG_FS)
-- 
1.7.7.3

Re: [REPOST PATCH v3 1/4] mmc: dw_mmc: Add "disable-wp" device tree property

[Olof Johansson]

On Thu, Jan 10, 2013 at 10:24:26AM -0800, Doug Anderson wrote:
> The "disable-wp" property is used to specify that a given SD card slot
> doesn't have a concept of write protect.  This eliminates the need for
> special case code for SD slots that should never be write protected
> (like a micro SD slot or a dev board).
> 
> The dw_mmc driver is special in needing to specify "disable-wp"
> because the lack of a "wp-gpios" property means to use the special
> purpose write protect line.  On some other mmc devices the lack of
> "wp-gpios" means that write protect should be disabled.

Hi,

The previous code used the controller-common quirk field to set a
per-controller DW_MCI_QUIRK_NO_WRITE_PROTECT. Is there really need to
do this per-slot? And if so, please explain in the commit message why
there is need for a brand new quirk for the same functionality.

It's even uncertain if there are any systems out there that uses the
multi-slot functionality of this driver to date, most seem to map one
slot per controller in the first place...


-Olof

Re: [REPOST PATCH v3 1/4] mmc: dw_mmc: Add "disable-wp" device tree property

[Doug Anderson]

Olof,

Thanks for your comments.

On Thu, Jan 10, 2013 at 3:01 PM, Olof Johansson <olof@lixom.net> wrote:
>
> The previous code used the controller-common quirk field to set a
> per-controller DW_MCI_QUIRK_NO_WRITE_PROTECT. Is there really need to
> do this per-slot? And if so, please explain in the commit message why
> there is need for a brand new quirk for the same functionality.

I'm happy to move back to using a per-controller quirk here--it
simplifies the code quite a bit since it can use all of the
preexisting quirks code.  I originally coded it up as per-slot since
generally each slot needs its own write-protect line.  Without ever
seeing any hardware using multiple slots per controller I can't say
how common this would be, so it may be overkill to handle that
situation.

Note that I think of the new code as being the device-tree equivalent
of the non-device-tree "brd->get_ro(slot_id)" function.  That function
is per-slot.

I will wait a day or two to see if others have feedback.  If I hear no
feedback I'll send up a patch that moves this to be at the controller
level.


Thanks!

-Doug

Re: [REPOST PATCH v3 1/4] mmc: dw_mmc: Add "disable-wp" device tree property

[Doug Anderson]

Hi,

A followup as I did more investigation here...

On Thu, Jan 10, 2013 at 3:23 PM, Doug Anderson <dianders@chromium.org> wrote:
> Olof,
>
> Thanks for your comments.
>
> On Thu, Jan 10, 2013 at 3:01 PM, Olof Johansson <olof@lixom.net> wrote:
>>
>> The previous code used the controller-common quirk field to set a
>> per-controller DW_MCI_QUIRK_NO_WRITE_PROTECT. Is there really need to
>> do this per-slot? And if so, please explain in the commit message why
>> there is need for a brand new quirk for the same functionality.
>
> I'm happy to move back to using a per-controller quirk here--it
> simplifies the code quite a bit since it can use all of the
> preexisting quirks code.  I originally coded it up as per-slot since
> generally each slot needs its own write-protect line.  Without ever
> seeing any hardware using multiple slots per controller I can't say
> how common this would be, so it may be overkill to handle that
> situation.

Actually, it looks like the per-controller
DW_MCI_QUIRK_NO_WRITE_PROTECT was added at the same time as the code
using it was added to 'drivers/mmc/host/dw_mmc-exynos.c'.  Since this
patch series removes the code from dw_mmc-exynos.c I can also remove
the controller-level quirk.

...I'll plan to spin a new rev tomorrow that leaves the 'no write
protect' quirk at the slot level but removes the old controller-level
quirk.  :)

-Doug

[PATCH v4 1/5] mmc: dw_mmc: Add "disable-wp" device tree property

[Doug Anderson]

The "disable-wp" property is used to specify that a given SD card slot
doesn't have a concept of write protect.  This eliminates the need for
special case code for SD slots that should never be write protected
(like a micro SD slot or a dev board).

The dw_mmc driver is special in needing to specify "disable-wp"
because the lack of a "wp-gpios" property means to use the special
purpose write protect line.  On some other mmc devices the lack of
"wp-gpios" means that write protect should be disabled.

Signed-off-by: Doug Anderson <dianders@chromium.org>
Acked-by: Seungwon Jeon <tgih.jun@samsung.com>
---
Changes in v4:
- Added a comment about the fact that a future patch will remove the
  controller-level quirk; the comments and quirk will be removed in
  the future change "mmc: dw_mmc: Remove
  DW_MCI_QUIRK_NO_WRITE_PROTECT".  I split the patch up this way to
  preserve bisectability and also to keep changes logically separated.
  I will squash if desired.

Changes in v3:
- New for this version of the patch series.  Chose "disable-wp" rather
  than the discussed "broken-internal-wp" since it mapped more cleanly
  to an existing quirk (and the only reason to specify that the
  internal wp is broken is if you're disabling the write protect
  anyway).

 .../devicetree/bindings/mmc/synopsis-dw-mshc.txt   |   12 +++++-
 drivers/mmc/host/dw_mmc.c                          |   41 +++++++++++++++++++-
 include/linux/mmc/dw_mmc.h                         |    9 ++++
 3 files changed, 59 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt
index 06cd32d08..726fd21 100644
--- a/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt
+++ b/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt
@@ -26,8 +26,16 @@ Required Properties:
 	* bus-width: as documented in mmc core bindings.
 
 	* wp-gpios: specifies the write protect gpio line. The format of the
-	  gpio specifier depends on the gpio controller. If the write-protect
-	  line is not available, this property is optional.
+	  gpio specifier depends on the gpio controller. If a GPIO is not used
+	  for write-protect, this property is optional.
+
+	* disable-wp: If the wp-gpios property isn't present then (by default)
+	  we'd assume that the write protect is hooked up directly to the
+	  controller's special purpose write protect line (accessible via
+	  the WRTPRT register).  However, it's possible that we simply don't
+	  want write protect.  In that case specify 'disable-wp'.
+	  NOTE: This property is not required for slots known to always
+	  connect to eMMC or SDIO cards.
 
 Optional properties:
 
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 323c502..90f7d99 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -74,6 +74,7 @@ struct idmac_desc {
  * struct dw_mci_slot - MMC slot state
  * @mmc: The mmc_host representing this slot.
  * @host: The MMC controller this slot is using.
+ * @quirks: Slot-level quirks (DW_MCI_SLOT_QUIRK_XXX)
  * @ctype: Card type for this slot.
  * @mrq: mmc_request currently being processed or waiting to be
  *	processed, or NULL when the slot is idle.
@@ -88,6 +89,8 @@ struct dw_mci_slot {
 	struct mmc_host		*mmc;
 	struct dw_mci		*host;
 
+	int			quirks;
+
 	u32			ctype;
 
 	struct mmc_request	*mrq;
@@ -825,7 +828,13 @@ static int dw_mci_get_ro(struct mmc_host *mmc)
 	struct dw_mci_board *brd = slot->host->pdata;
 
 	/* Use platform get_ro function, else try on board write protect */
-	if (brd->quirks & DW_MCI_QUIRK_NO_WRITE_PROTECT)
+
+	/*
+	 * NOTE: DW_MCI_QUIRK_NO_WRITE_PROTECT will be removed in a future
+	 * patch in the series once reference to it is removed.
+	 */
+	if ((brd->quirks & DW_MCI_QUIRK_NO_WRITE_PROTECT) ||
+	    (slot->quirks & DW_MCI_SLOT_QUIRK_NO_WRITE_PROTECT))
 		read_only = 0;
 	else if (brd->get_ro)
 		read_only = brd->get_ro(slot->id);
@@ -1785,6 +1794,30 @@ static struct device_node *dw_mci_of_find_slot_node(struct device *dev, u8 slot)
 	return NULL;
 }
 
+static struct dw_mci_of_slot_quirks {
+	char *quirk;
+	int id;
+} of_slot_quirks[] = {
+	{
+		.quirk	= "disable-wp",
+		.id	= DW_MCI_SLOT_QUIRK_NO_WRITE_PROTECT,
+	},
+};
+
+static int dw_mci_of_get_slot_quirks(struct device *dev, u8 slot)
+{
+	struct device_node *np = dw_mci_of_find_slot_node(dev, slot);
+	int quirks = 0;
+	int idx;
+
+	/* get quirks */
+	for (idx = 0; idx < ARRAY_SIZE(of_slot_quirks); idx++)
+		if (of_get_property(np, of_slot_quirks[idx].quirk, NULL))
+			quirks |= of_slot_quirks[idx].id;
+
+	return quirks;
+}
+
 /* find out bus-width for a given slot */
 static u32 dw_mci_of_get_bus_wd(struct device *dev, u8 slot)
 {
@@ -1800,6 +1833,10 @@ static u32 dw_mci_of_get_bus_wd(struct device *dev, u8 slot)
 	return bus_wd;
 }
 #else /* CONFIG_OF */
+static int dw_mci_of_get_slot_quirks(struct device *dev, u8 slot)
+{
+	return 0;
+}
 static u32 dw_mci_of_get_bus_wd(struct device *dev, u8 slot)
 {
 	return 1;
@@ -1828,6 +1865,8 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
 	slot->host = host;
 	host->slot[id] = slot;
 
+	slot->quirks = dw_mci_of_get_slot_quirks(host->dev, slot->id);
+
 	mmc->ops = &dw_mci_ops;
 	mmc->f_min = DIV_ROUND_UP(host->bus_hz, 510);
 	mmc->f_max = host->bus_hz;
diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
index 34be4f4..de61de5 100644
--- a/include/linux/mmc/dw_mmc.h
+++ b/include/linux/mmc/dw_mmc.h
@@ -209,9 +209,18 @@ struct dw_mci_dma_ops {
 #define DW_MCI_QUIRK_HIGHSPEED			BIT(2)
 /* Unreliable card detection */
 #define DW_MCI_QUIRK_BROKEN_CARD_DETECTION	BIT(3)
+
 /* Write Protect detection not available */
+/*
+ * NOTE: DW_MCI_QUIRK_NO_WRITE_PROTECT will be removed in a future
+ * patch in the series once reference to it is removed.
+ */
 #define DW_MCI_QUIRK_NO_WRITE_PROTECT		BIT(4)
 
+/* Slot level quirks */
+/* This slot has no write protect */
+#define DW_MCI_SLOT_QUIRK_NO_WRITE_PROTECT	BIT(0)
+
 struct dma_pdata;
 
 struct block_settings {
-- 
1.7.7.3

[PATCH v4 2/5] ARM: dts: Add disable-wp for sd card slot on smdk5250

[Doug Anderson]

The next change will remove the code from the dw_mmc-exynos that added
the DW_MCI_QUIRK_NO_WRITE_PROTECT.  Keep existing functionality of
having no write protect pin on smdk5250 by adding the disable-wp
property.

Signed-off-by: Doug Anderson <dianders@chromium.org>
Acked-by: Seungwon Jeon <tgih.jun@samsung.com>
---
Changes in v4: None
Changes in v3:
- New for this version of the patch series.

 arch/arm/boot/dts/exynos5250-smdk5250.dts |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/exynos5250-smdk5250.dts b/arch/arm/boot/dts/exynos5250-smdk5250.dts
index 942d576..75b9d83 100644
--- a/arch/arm/boot/dts/exynos5250-smdk5250.dts
+++ b/arch/arm/boot/dts/exynos5250-smdk5250.dts
@@ -146,6 +146,7 @@
 			reg = <0>;
 			bus-width = <4>;
 			samsung,cd-pinmux-gpio = <&gpc3 2 2 3 3>;
+			disable-wp;
 			gpios = <&gpc3 0 2 0 3>, <&gpc3 1 2 0 3>,
 				<&gpc3 3 2 3 3>, <&gpc3 4 2 3 3>,
 				<&gpc3 5 2 3 3>, <&gpc3 6 2 3 3>,
-- 
1.7.7.3

[PATCH v4 3/5] mmc: dw_mmc: exynos: Remove code for wp-gpios

[Doug Anderson]

The exynos code claimed the write protect with devm_gpio_request() but
never did anything with it.  That meant that anyone using a write
protect GPIO would effectively be write protected all the time.

The handling for wp-gpios belongs in the main dw_mmc driver and has
been moved there.

Signed-off-by: Doug Anderson <dianders@chromium.org>
Acked-by: Seungwon Jeon <tgih.jun@samsung.com>
---
Changes in v4: None
Changes in v3:
- Totally removed wp-gpios handling from exynos code.

Changes in v2: None

 drivers/mmc/host/dw_mmc-exynos.c |   10 ----------
 1 files changed, 0 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
index 4d50da6..72fd0f2 100644
--- a/drivers/mmc/host/dw_mmc-exynos.c
+++ b/drivers/mmc/host/dw_mmc-exynos.c
@@ -175,16 +175,6 @@ static int dw_mci_exynos_setup_bus(struct dw_mci *host,
 		}
 	}
 
-	gpio = of_get_named_gpio(slot_np, "wp-gpios", 0);
-	if (gpio_is_valid(gpio)) {
-		if (devm_gpio_request(host->dev, gpio, "dw-mci-wp"))
-			dev_info(host->dev, "gpio [%d] request failed\n",
-						gpio);
-	} else {
-		dev_info(host->dev, "wp gpio not available");
-		host->pdata->quirks |= DW_MCI_QUIRK_NO_WRITE_PROTECT;
-	}
-
 	if (host->pdata->quirks & DW_MCI_QUIRK_BROKEN_CARD_DETECTION)
 		return 0;
 
-- 
1.7.7.3

[PATCH v4 4/5] mmc: dw_mmc: Handle wp-gpios from device tree

[Doug Anderson]

On some SoCs (like exynos5250) you need to use an external GPIO for
write protect.  Add support for wp-gpios to the core dw_mmc driver
since it could be useful across multiple SoCs.

With this change I am able to make use of the write protect for the
external SD slot on exynos5250-snow.

Signed-off-by: Doug Anderson <dianders@chromium.org>
Acked-by: Seungwon Jeon <tgih.jun@samsung.com>
---
Changes in v4: None
Changes in v3: None
Changes in v2:
- Fixed return type from u32 to int
- Return -EINVAL instead of -1

 drivers/mmc/host/dw_mmc.c |   34 ++++++++++++++++++++++++++++++++++
 1 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 90f7d99..df62079 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -34,6 +34,7 @@
 #include <linux/regulator/consumer.h>
 #include <linux/workqueue.h>
 #include <linux/of.h>
+#include <linux/of_gpio.h>
 
 #include "dw_mmc.h"
 
@@ -75,6 +76,7 @@ struct idmac_desc {
  * @mmc: The mmc_host representing this slot.
  * @host: The MMC controller this slot is using.
  * @quirks: Slot-level quirks (DW_MCI_SLOT_QUIRK_XXX)
+ * @wp_gpio: If gpio_is_valid() we'll use this to read write protect.
  * @ctype: Card type for this slot.
  * @mrq: mmc_request currently being processed or waiting to be
  *	processed, or NULL when the slot is idle.
@@ -90,6 +92,7 @@ struct dw_mci_slot {
 	struct dw_mci		*host;
 
 	int			quirks;
+	int			wp_gpio;
 
 	u32			ctype;
 
@@ -838,6 +841,8 @@ static int dw_mci_get_ro(struct mmc_host *mmc)
 		read_only = 0;
 	else if (brd->get_ro)
 		read_only = brd->get_ro(slot->id);
+	else if (gpio_is_valid(slot->wp_gpio))
+		read_only = gpio_get_value(slot->wp_gpio);
 	else
 		read_only =
 			mci_readl(slot->host, WRTPRT) & (1 << slot->id) ? 1 : 0;
@@ -1832,6 +1837,29 @@ static u32 dw_mci_of_get_bus_wd(struct device *dev, u8 slot)
 			       " as 1\n");
 	return bus_wd;
 }
+
+/* find the write protect gpio for a given slot; or -1 if none specified */
+static int dw_mci_of_get_wp_gpio(struct device *dev, u8 slot)
+{
+	struct device_node *np = dw_mci_of_find_slot_node(dev, slot);
+	int gpio;
+
+	if (!np)
+		return -EINVAL;
+
+	gpio = of_get_named_gpio(np, "wp-gpios", 0);
+
+	/* Having a missing entry is valid; return silently */
+	if (!gpio_is_valid(gpio))
+		return -EINVAL;
+
+	if (devm_gpio_request(dev, gpio, "dw-mci-wp")) {
+		dev_warn(dev, "gpio [%d] request failed\n", gpio);
+		return -EINVAL;
+	}
+
+	return gpio;
+}
 #else /* CONFIG_OF */
 static int dw_mci_of_get_slot_quirks(struct device *dev, u8 slot)
 {
@@ -1845,6 +1873,10 @@ static struct device_node *dw_mci_of_find_slot_node(struct device *dev, u8 slot)
 {
 	return NULL;
 }
+static int dw_mci_of_get_wp_gpio(struct device *dev, u8 slot)
+{
+	return -EINVAL;
+}
 #endif /* CONFIG_OF */
 
 static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
@@ -1962,6 +1994,8 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
 	else
 		clear_bit(DW_MMC_CARD_PRESENT, &slot->flags);
 
+	slot->wp_gpio = dw_mci_of_get_wp_gpio(host->dev, slot->id);
+
 	mmc_add_host(mmc);
 
 #if defined(CONFIG_DEBUG_FS)
-- 
1.7.7.3

[PATCH v4 5/5] mmc: dw_mmc: Remove DW_MCI_QUIRK_NO_WRITE_PROTECT

[Doug Anderson]

The original quirk was added in the change 'mmc: dw_mmc: add quirk to
indicate missing write protect line'.  The original quirk was added at
a controller level even though each slot has its own write protect (so
the quirk should be at the slot level).  A recent change (mmc: dw_mmc:
Add "disable-wp" device tree property) added a slot-level quirk and
support for the quirk directly to dw_mmc.

Signed-off-by: Doug Anderson <dianders@chromium.org>
---
Changes in v4:
- New for this version; addresses comments from Olof Johansson about
  the fact that we shouldn't have both a slot and controller level
  quirk for the same thing.

 drivers/mmc/host/dw_mmc.c  |    8 +-------
 include/linux/mmc/dw_mmc.h |    7 -------
 2 files changed, 1 insertions(+), 14 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index df62079..60063cc 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -831,13 +831,7 @@ static int dw_mci_get_ro(struct mmc_host *mmc)
 	struct dw_mci_board *brd = slot->host->pdata;
 
 	/* Use platform get_ro function, else try on board write protect */
-
-	/*
-	 * NOTE: DW_MCI_QUIRK_NO_WRITE_PROTECT will be removed in a future
-	 * patch in the series once reference to it is removed.
-	 */
-	if ((brd->quirks & DW_MCI_QUIRK_NO_WRITE_PROTECT) ||
-	    (slot->quirks & DW_MCI_SLOT_QUIRK_NO_WRITE_PROTECT))
+	if (slot->quirks & DW_MCI_SLOT_QUIRK_NO_WRITE_PROTECT)
 		read_only = 0;
 	else if (brd->get_ro)
 		read_only = brd->get_ro(slot->id);
diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
index de61de5..198f0fa 100644
--- a/include/linux/mmc/dw_mmc.h
+++ b/include/linux/mmc/dw_mmc.h
@@ -210,13 +210,6 @@ struct dw_mci_dma_ops {
 /* Unreliable card detection */
 #define DW_MCI_QUIRK_BROKEN_CARD_DETECTION	BIT(3)
 
-/* Write Protect detection not available */
-/*
- * NOTE: DW_MCI_QUIRK_NO_WRITE_PROTECT will be removed in a future
- * patch in the series once reference to it is removed.
- */
-#define DW_MCI_QUIRK_NO_WRITE_PROTECT		BIT(4)
-
 /* Slot level quirks */
 /* This slot has no write protect */
 #define DW_MCI_SLOT_QUIRK_NO_WRITE_PROTECT	BIT(0)
-- 
1.7.7.3

Re: [PATCH v4 1/5] mmc: dw_mmc: Add "disable-wp" device tree property

[Will Newton]

On Fri, Jan 11, 2013 at 5:03 PM, Doug Anderson <dianders@chromium.org> wrote:
> The "disable-wp" property is used to specify that a given SD card slot
> doesn't have a concept of write protect.  This eliminates the need for
> special case code for SD slots that should never be write protected
> (like a micro SD slot or a dev board).
>
> The dw_mmc driver is special in needing to specify "disable-wp"
> because the lack of a "wp-gpios" property means to use the special
> purpose write protect line.  On some other mmc devices the lack of
> "wp-gpios" means that write protect should be disabled.
>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Acked-by: Seungwon Jeon <tgih.jun@samsung.com>

This looks ok to me too.

Acked-by: Will Newton <will.newton@imgtec.com>

Re: [PATCH v4 5/5] mmc: dw_mmc: Remove DW_MCI_QUIRK_NO_WRITE_PROTECT

[Will Newton]

On Fri, Jan 11, 2013 at 5:03 PM, Doug Anderson <dianders@chromium.org> wrote:
> The original quirk was added in the change 'mmc: dw_mmc: add quirk to
> indicate missing write protect line'.  The original quirk was added at
> a controller level even though each slot has its own write protect (so
> the quirk should be at the slot level).  A recent change (mmc: dw_mmc:
> Add "disable-wp" device tree property) added a slot-level quirk and
> support for the quirk directly to dw_mmc.
>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> ---
> Changes in v4:
> - New for this version; addresses comments from Olof Johansson about
>   the fact that we shouldn't have both a slot and controller level
>   quirk for the same thing.
>
>  drivers/mmc/host/dw_mmc.c  |    8 +-------
>  include/linux/mmc/dw_mmc.h |    7 -------
>  2 files changed, 1 insertions(+), 14 deletions(-)

Hi Doug,

These changes look ok to me. I don't know if patch 4 made it to the
list, I only got a copy via the cc and the only archive I can find of
linux-mmc (gmane) seems to be missing a lot of messages besides this
one.

So for the dw_mmc parts:

Acked-by: Will Newton <will.newton@imgtec.com>

Re: [PATCH v4 5/5] mmc: dw_mmc: Remove DW_MCI_QUIRK_NO_WRITE_PROTECT

[Doug Anderson]

Will,

On Mon, Jan 14, 2013 at 2:47 AM, Will Newton <will.newton@gmail.com> wrote:
> These changes look ok to me. I don't know if patch 4 made it to the
> list, I only got a copy via the cc and the only archive I can find of
> linux-mmc (gmane) seems to be missing a lot of messages besides this
> one.

It seems to have made it onto patchwork, in any case.  Here's the link:
    https://patchwork.kernel.org/patch/1966771/

The whole bundle of all 5 patches at v4 is here:
    http://patchwork.kernel.org/bundle/dianders/mmc_wp/


> So for the dw_mmc parts:
>
> Acked-by: Will Newton <will.newton@imgtec.com>

Thank you for your review and your ack.  :)

-Doug

Re: [PATCH v4 1/5] mmc: dw_mmc: Add "disable-wp" device tree property

[Olof Johansson]

On Fri, Jan 11, 2013 at 09:03:50AM -0800, Doug Anderson wrote:
> The "disable-wp" property is used to specify that a given SD card slot
> doesn't have a concept of write protect.  This eliminates the need for
> special case code for SD slots that should never be write protected
> (like a micro SD slot or a dev board).
> 
> The dw_mmc driver is special in needing to specify "disable-wp"
> because the lack of a "wp-gpios" property means to use the special
> purpose write protect line.  On some other mmc devices the lack of
> "wp-gpios" means that write protect should be disabled.
> 
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Acked-by: Seungwon Jeon <tgih.jun@samsung.com>

Acked-by: Olof Johansson <olof@lixom.net>

Nit below.

> @@ -825,7 +828,13 @@ static int dw_mci_get_ro(struct mmc_host *mmc)
>  	struct dw_mci_board *brd = slot->host->pdata;
>  
>  	/* Use platform get_ro function, else try on board write protect */
> -	if (brd->quirks & DW_MCI_QUIRK_NO_WRITE_PROTECT)
> +
> +	/*
> +	 * NOTE: DW_MCI_QUIRK_NO_WRITE_PROTECT will be removed in a future
> +	 * patch in the series once reference to it is removed.
> +	 */
> +	if ((brd->quirks & DW_MCI_QUIRK_NO_WRITE_PROTECT) ||
> +	    (slot->quirks & DW_MCI_SLOT_QUIRK_NO_WRITE_PROTECT))

Given that it never worked properly, you could have nuked it first and avoid
the extra churn. Still, not a strong enough reason to respin the series, IMHO.


-Olof

Re: [PATCH v4 3/5] mmc: dw_mmc: exynos: Remove code for wp-gpios

[Olof Johansson]

On Fri, Jan 11, 2013 at 09:03:52AM -0800, Doug Anderson wrote:
> The exynos code claimed the write protect with devm_gpio_request() but
> never did anything with it.  That meant that anyone using a write
> protect GPIO would effectively be write protected all the time.
> 
> The handling for wp-gpios belongs in the main dw_mmc driver and has
> been moved there.
> 
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Acked-by: Seungwon Jeon <tgih.jun@samsung.com>

Acked-by: Olof Johansson <olof@lixom.net>


-Olof

Re: [PATCH v4 2/5] ARM: dts: Add disable-wp for sd card slot on smdk5250

[Olof Johansson]

On Fri, Jan 11, 2013 at 09:03:51AM -0800, Doug Anderson wrote:
> The next change will remove the code from the dw_mmc-exynos that added
> the DW_MCI_QUIRK_NO_WRITE_PROTECT.  Keep existing functionality of
> having no write protect pin on smdk5250 by adding the disable-wp
> property.
> 
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Acked-by: Seungwon Jeon <tgih.jun@samsung.com>

Acked-by: Olof Johansson <olof@lixom.net>


-Olof

Re: [PATCH v4 4/5] mmc: dw_mmc: Handle wp-gpios from device tree

[Olof Johansson]

On Fri, Jan 11, 2013 at 09:03:53AM -0800, Doug Anderson wrote:
> On some SoCs (like exynos5250) you need to use an external GPIO for
> write protect.  Add support for wp-gpios to the core dw_mmc driver
> since it could be useful across multiple SoCs.
> 
> With this change I am able to make use of the write protect for the
> external SD slot on exynos5250-snow.
> 
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Acked-by: Seungwon Jeon <tgih.jun@samsung.com>

Acked-by: Olof Johansson <olof@lixom.net>

Re: [PATCH v4 5/5] mmc: dw_mmc: Remove DW_MCI_QUIRK_NO_WRITE_PROTECT

[Olof Johansson]

On Fri, Jan 11, 2013 at 09:03:54AM -0800, Doug Anderson wrote:
> The original quirk was added in the change 'mmc: dw_mmc: add quirk to
> indicate missing write protect line'.  The original quirk was added at
> a controller level even though each slot has its own write protect (so
> the quirk should be at the slot level).  A recent change (mmc: dw_mmc:
> Add "disable-wp" device tree property) added a slot-level quirk and
> support for the quirk directly to dw_mmc.
> 
> Signed-off-by: Doug Anderson <dianders@chromium.org>

Acked-by: Olof Johansson <olof@lixom.net>

Re: [PATCH v4 5/5] mmc: dw_mmc: Remove DW_MCI_QUIRK_NO_WRITE_PROTECT

[Chris Ball]

Hi Doug,

On Mon, Jan 14 2013, Doug Anderson wrote:
> The whole bundle of all 5 patches at v4 is here:
>     http://patchwork.kernel.org/bundle/dianders/mmc_wp/

Thanks, I've pushed all 5 to mmc-next for 3.9 now with the collected ACKs.

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child