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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]> 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 <[email protected]>
> ---
> 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
On Thursday, November 22, 2012, Doug Anderson <[email protected]> wrote:
> On Wed, Nov 21, 2012 at 5:42 PM, Seungwon Jeon <[email protected]> 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
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 <[email protected]>
> ---
> 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)
>
On Wed, Nov 21, 2012 at 5:42 PM, Seungwon Jeon <[email protected]> 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
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 <[email protected]> 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 <[email protected]>
>> ---
>> 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)
>>
>
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
>
> ---
> 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
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 <[email protected]> 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 <[email protected]>
>>
>> ---
>> 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
>
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 <[email protected]> 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 <[email protected]>
> >>
> >> ---
> >> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Seungwon,
On Wed, Nov 28, 2012 at 11:46 PM, Seungwon Jeon <[email protected]> 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 <[email protected]> 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 <[email protected]>
>> >>
>> >> ---
>> >> 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 [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
Doug, Thanks to work.
Looks good to me with other patches.
Acked-by: Seungwon Jeon <[email protected]>
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 <[email protected]>
> ---
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html