RE: [PATCH 2/2] mmc: dw_mmc: Handle wp-gpios from device tree
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 diand...@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 diand...@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
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/2] mmc: dw_mmc: Handle wp-gpios from device tree
On Thursday, November 22, 2012, Doug Anderson diand...@chromium.org wrote: On Wed, Nov 21, 2012 at 5:42 PM, Seungwon Jeon tgih@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 -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] mmc: dw_mmc: Handle wp-gpios from device tree
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 diand...@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)
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] mmc: dw_mmc: Handle wp-gpios from device tree
On Wed, Nov 21, 2012 at 5:42 PM, Seungwon Jeon tgih@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 -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] mmc: dw_mmc: Handle wp-gpios from device tree
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.ch...@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 diand...@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)
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
