devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/4] mmc: Add support for disabling write-protect detection
@ 2015-05-06 18:31 Lars-Peter Clausen
       [not found] ` <1430937082-27149-1-git-send-email-lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Lars-Peter Clausen @ 2015-05-06 18:31 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Seungwon Jeon, Jaehoon Chung, Michal Simek, Sören Brinkmann,
	Thomas Abraham, Mike Looijmans, linux-mmc, devicetree,
	Lars-Peter Clausen

It is not uncommon to see systems where there is no physical write-protect
signal (e.g. when using eMMC or microSD card slots). For some controllers,
which have a dedicated write-protection detection logic (like SDHCI
controllers), the get_ro() callback can return bogus data in such a case.

Instead of handling this on a per controller basis this patch adds a new
capability flag to the MMC core that can be set to specify that the result
of get_ro() is invalid. When the flag is set the core will not call
get_ro() and assume that the card is always read-write.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/mmc/core/sd.c    | 30 +++++++++++++++++++++++-------
 include/linux/mmc/host.h |  1 +
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index 31a9ef2..8f6864a 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -804,6 +804,28 @@ int mmc_sd_get_csd(struct mmc_host *host, struct mmc_card *card)
 	return 0;
 }
 
+static int mmc_sd_get_ro(struct mmc_host *host)
+{
+	int ro;
+
+	/*
+	 * Some systems don't feature a write-protect pin and don't need one.
+	 * E.g. because they only have micro-SD card slot. For those systems
+	 * assume that the SD card is always read-write.
+	 */
+	if (host->caps2 & MMC_CAP2_NO_WRITE_PROTECT)
+		return 0;
+
+	if (!host->ops->get_ro)
+		return -1;
+
+	mmc_host_clk_hold(host);
+	ro = host->ops->get_ro(host);
+	mmc_host_clk_release(host);
+
+	return ro;
+}
+
 int mmc_sd_setup_card(struct mmc_host *host, struct mmc_card *card,
 	bool reinit)
 {
@@ -855,13 +877,7 @@ int mmc_sd_setup_card(struct mmc_host *host, struct mmc_card *card,
 	 * Check if read-only switch is active.
 	 */
 	if (!reinit) {
-		int ro = -1;
-
-		if (host->ops->get_ro) {
-			mmc_host_clk_hold(card->host);
-			ro = host->ops->get_ro(host);
-			mmc_host_clk_release(card->host);
-		}
+		int ro = mmc_sd_get_ro(host);
 
 		if (ro < 0) {
 			pr_warn("%s: host does not support reading read-only switch, assuming write-enable\n",
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index b5bedae..3b92fd4 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -285,6 +285,7 @@ struct mmc_host {
 				 MMC_CAP2_HS400_1_2V)
 #define MMC_CAP2_HSX00_1_2V	(MMC_CAP2_HS200_1_2V_SDR | MMC_CAP2_HS400_1_2V)
 #define MMC_CAP2_SDIO_IRQ_NOTHREAD (1 << 17)
+#define MMC_CAP2_NO_WRITE_PROTECT (1 << 18)	/* No physical write protect pin, assume that card is always read-write */
 
 	mmc_pm_flag_t		pm_caps;	/* supported pm features */
 
-- 
1.8.0


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

* [PATCH v2 2/4] mmc: dt: Allow to specify that no write protect signal is present
       [not found] ` <1430937082-27149-1-git-send-email-lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
@ 2015-05-06 18:31   ` Lars-Peter Clausen
  2015-05-11 10:30     ` Ulf Hansson
  0 siblings, 1 reply; 11+ messages in thread
From: Lars-Peter Clausen @ 2015-05-06 18:31 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Seungwon Jeon, Jaehoon Chung, Michal Simek, Sören Brinkmann,
	Thomas Abraham, Mike Looijmans, linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Lars-Peter Clausen,
	Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala

Allow to specify in the device-tree that no physical write-protect signal
is connected to a particular instance of a MMC controller. Setting the
property will cause the core will assume that the SD card is always
read-write.

The name for the new property is 'disable-wp' and was chosen based on the
property with the same function from the Synopsys designware mobile storage
host controller DT bindings specification.

Signed-off-by: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>
Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
Cc: Ian Campbell <ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>
Cc: Kumar Gala <galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
---
 Documentation/devicetree/bindings/mmc/mmc.txt | 5 +++++
 drivers/mmc/core/host.c                       | 3 +++
 2 files changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt
index 438899e..0384fc3 100644
--- a/Documentation/devicetree/bindings/mmc/mmc.txt
+++ b/Documentation/devicetree/bindings/mmc/mmc.txt
@@ -21,6 +21,11 @@ Optional properties:
   below for the case, when a GPIO is used for the CD line
 - wp-inverted: when present, polarity on the WP line is inverted. See the note
   below for the case, when a GPIO is used for the WP line
+- disable-wp: When set no physical WP line is present. This property should
+  only be specified when the controller has a dedicated write-protect
+  detection logic. If a GPIO is always used for the write-protect detection
+  logic it is sufficient to not specify wp-gpios property in the absence of a WP
+  line.
 - max-frequency: maximum operating clock frequency
 - no-1-8-v: when present, denotes that 1.8v card voltage is not supported on
   this system, even if the controller claims it is.
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 8be0df7..1eabdd0 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -400,6 +400,9 @@ int mmc_of_parse(struct mmc_host *host)
 	else if (ret != -ENOENT)
 		return ret;
 
+	if (of_property_read_bool(np, "disable-wp"))
+		host->caps2 |= MMC_CAP2_NO_WRITE_PROTECT;
+
 	/* See the comment on CD inversion above */
 	if (ro_cap_invert ^ ro_gpio_invert)
 		host->caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
-- 
1.8.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH v2 3/4] mmc: sdhci-of-arasan: Call mmc_of_parse()
  2015-05-06 18:31 [PATCH v2 1/4] mmc: Add support for disabling write-protect detection Lars-Peter Clausen
       [not found] ` <1430937082-27149-1-git-send-email-lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
@ 2015-05-06 18:31 ` Lars-Peter Clausen
  2015-05-07  5:00   ` Michal Simek
  2015-05-06 18:31 ` [PATCH v2 4/4] mmc: dw_mmc: Use core to handle absent write protect line Lars-Peter Clausen
  2015-05-11 10:30 ` [PATCH v2 1/4] mmc: Add support for disabling write-protect detection Ulf Hansson
  3 siblings, 1 reply; 11+ messages in thread
From: Lars-Peter Clausen @ 2015-05-06 18:31 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Seungwon Jeon, Jaehoon Chung, Michal Simek, Sören Brinkmann,
	Thomas Abraham, Mike Looijmans, linux-mmc, devicetree,
	Lars-Peter Clausen

The device-tree documentation for the Arasan SDHCI controller already
claims that it supports the standard MMC devicetree properties. Update the
driver implementation accordingly by calling mmc_of_parse().

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/mmc/host/sdhci-of-arasan.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
index 21c0c08..538fd81 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -175,6 +175,10 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
 	}
 
 	sdhci_get_of_property(pdev);
+	ret = mmc_of_parse(host->mmc);
+	if (ret)
+		goto clk_disable_all;
+
 	pltfm_host = sdhci_priv(host);
 	pltfm_host->priv = sdhci_arasan;
 	pltfm_host->clk = clk_xin;
-- 
1.8.0


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

* [PATCH v2 4/4] mmc: dw_mmc: Use core to handle absent write protect line
  2015-05-06 18:31 [PATCH v2 1/4] mmc: Add support for disabling write-protect detection Lars-Peter Clausen
       [not found] ` <1430937082-27149-1-git-send-email-lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
  2015-05-06 18:31 ` [PATCH v2 3/4] mmc: sdhci-of-arasan: Call mmc_of_parse() Lars-Peter Clausen
@ 2015-05-06 18:31 ` Lars-Peter Clausen
       [not found]   ` <1430937082-27149-4-git-send-email-lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
  2015-05-11 10:30 ` [PATCH v2 1/4] mmc: Add support for disabling write-protect detection Ulf Hansson
  3 siblings, 1 reply; 11+ messages in thread
From: Lars-Peter Clausen @ 2015-05-06 18:31 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Seungwon Jeon, Jaehoon Chung, Michal Simek, Sören Brinkmann,
	Thomas Abraham, Mike Looijmans, linux-mmc, devicetree,
	Lars-Peter Clausen

Use the new MMC_CAP2_NO_WRITE_PROTECT to let the core handle the case where
no write protect line is present instead of having custom driver code to
handle it.

dw_mci_of_get_slot_quirks() is slightly refactored to directly modify the
mmc_host capabilities instead of returning a quirk mask.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
Only compile tested.
---
 drivers/mmc/host/dw_mmc.c  | 53 +++++++++++++++-------------------------------
 drivers/mmc/host/dw_mmc.h  |  3 ---
 include/linux/mmc/dw_mmc.h |  6 ------
 3 files changed, 17 insertions(+), 45 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 38b2926..0eedc5a 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1276,10 +1276,7 @@ static int dw_mci_get_ro(struct mmc_host *mmc)
 	int gpio_ro = mmc_gpio_get_ro(mmc);
 
 	/* Use platform get_ro function, else try on board write protect */
-	if ((slot->quirks & DW_MCI_SLOT_QUIRK_NO_WRITE_PROTECT) ||
-			(slot->host->quirks & DW_MCI_QUIRK_NO_WRITE_PROTECT))
-		read_only = 0;
-	else if (!IS_ERR_VALUE(gpio_ro))
+	if (!IS_ERR_VALUE(gpio_ro))
 		read_only = gpio_ro;
 	else
 		read_only =
@@ -2277,9 +2274,10 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
 }
 
 #ifdef CONFIG_OF
-/* given a slot id, find out the device node representing that slot */
-static struct device_node *dw_mci_of_find_slot_node(struct device *dev, u8 slot)
+/* given a slot, find out the device node representing that slot */
+static struct device_node *dw_mci_of_find_slot_node(struct dw_mci_slot *slot)
 {
+	struct device *dev = slot->mmc->parent;
 	struct device_node *np;
 	const __be32 *addr;
 	int len;
@@ -2291,42 +2289,28 @@ static struct device_node *dw_mci_of_find_slot_node(struct device *dev, u8 slot)
 		addr = of_get_property(np, "reg", &len);
 		if (!addr || (len < sizeof(int)))
 			continue;
-		if (be32_to_cpup(addr) == slot)
+		if (be32_to_cpup(addr) == slot->id)
 			return np;
 	}
 	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)
+static void dw_mci_slot_of_parse(struct dw_mci_slot *slot)
 {
-	struct device_node *np = dw_mci_of_find_slot_node(dev, slot);
-	int quirks = 0;
-	int idx;
+	struct device_node *np = dw_mci_of_find_slot_node(slot);
 
-	/* get quirks */
-	for (idx = 0; idx < ARRAY_SIZE(of_slot_quirks); idx++)
-		if (of_get_property(np, of_slot_quirks[idx].quirk, NULL)) {
-			dev_warn(dev, "Slot quirk %s is deprecated\n",
-					of_slot_quirks[idx].quirk);
-			quirks |= of_slot_quirks[idx].id;
-		}
+	if (!np)
+		return;
 
-	return quirks;
+	if (of_property_read_bool(np, "disable-wp")) {
+		slot->mmc->caps2 |= MMC_CAP2_NO_WRITE_PROTECT;
+		dev_warn(slot->mmc->parent,
+			"Slot quirk 'disable-wp' is deprecated\n");
+	}
 }
 #else /* CONFIG_OF */
-static int dw_mci_of_get_slot_quirks(struct device *dev, u8 slot)
+static void dw_mci_slot_of_parse(struct dw_mci_slot *slot)
 {
-	return 0;
 }
 #endif /* CONFIG_OF */
 
@@ -2349,8 +2333,6 @@ 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;
 	if (of_property_read_u32_array(host->dev->of_node,
 				       "clock-freq-min-max", freq, 2)) {
@@ -2388,6 +2370,8 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
 	if (host->pdata->caps2)
 		mmc->caps2 = host->pdata->caps2;
 
+	dw_mci_slot_of_parse(slot);
+
 	ret = mmc_of_parse(mmc);
 	if (ret)
 		goto err_host_allocated;
@@ -2615,9 +2599,6 @@ static struct dw_mci_of_quirks {
 	{
 		.quirk	= "broken-cd",
 		.id	= DW_MCI_QUIRK_BROKEN_CARD_DETECTION,
-	}, {
-		.quirk	= "disable-wp",
-		.id	= DW_MCI_QUIRK_NO_WRITE_PROTECT,
 	},
 };
 
diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
index f45ab91..2c6d350 100644
--- a/drivers/mmc/host/dw_mmc.h
+++ b/drivers/mmc/host/dw_mmc.h
@@ -227,7 +227,6 @@ extern int dw_mci_resume(struct dw_mci *host);
  * 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.
@@ -245,8 +244,6 @@ struct dw_mci_slot {
 	struct mmc_host		*mmc;
 	struct dw_mci		*host;
 
-	int			quirks;
-
 	u32			ctype;
 
 	struct mmc_request	*mrq;
diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
index 1211199..5be9767 100644
--- a/include/linux/mmc/dw_mmc.h
+++ b/include/linux/mmc/dw_mmc.h
@@ -226,12 +226,6 @@ struct dw_mci_dma_ops {
 #define DW_MCI_QUIRK_HIGHSPEED			BIT(2)
 /* Unreliable card detection */
 #define DW_MCI_QUIRK_BROKEN_CARD_DETECTION	BIT(3)
-/* No write protect */
-#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;
 
-- 
1.8.0


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

* Re: [PATCH v2 3/4] mmc: sdhci-of-arasan: Call mmc_of_parse()
  2015-05-06 18:31 ` [PATCH v2 3/4] mmc: sdhci-of-arasan: Call mmc_of_parse() Lars-Peter Clausen
@ 2015-05-07  5:00   ` Michal Simek
  2015-05-07  5:44     ` Lars-Peter Clausen
  0 siblings, 1 reply; 11+ messages in thread
From: Michal Simek @ 2015-05-07  5:00 UTC (permalink / raw)
  To: Lars-Peter Clausen, Ulf Hansson
  Cc: Seungwon Jeon, Jaehoon Chung, Michal Simek, Sören Brinkmann,
	Thomas Abraham, Mike Looijmans, linux-mmc, devicetree

Hi Lars,

On 05/06/2015 08:31 PM, Lars-Peter Clausen wrote:
> The device-tree documentation for the Arasan SDHCI controller already
> claims that it supports the standard MMC devicetree properties. Update the
> driver implementation accordingly by calling mmc_of_parse().
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---
>  drivers/mmc/host/sdhci-of-arasan.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
> index 21c0c08..538fd81 100644
> --- a/drivers/mmc/host/sdhci-of-arasan.c
> +++ b/drivers/mmc/host/sdhci-of-arasan.c
> @@ -175,6 +175,10 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
>  	}
>  
>  	sdhci_get_of_property(pdev);
> +	ret = mmc_of_parse(host->mmc);
> +	if (ret)
> +		goto clk_disable_all;
> +
>  	pltfm_host = sdhci_priv(host);
>  	pltfm_host->priv = sdhci_arasan;
>  	pltfm_host->clk = clk_xin;
> 

I have sent this patch already and I also see it in linux-next.
https://lkml.org/lkml/2015/4/7/21

Thanks,
Michal

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

* Re: [PATCH v2 3/4] mmc: sdhci-of-arasan: Call mmc_of_parse()
  2015-05-07  5:00   ` Michal Simek
@ 2015-05-07  5:44     ` Lars-Peter Clausen
  0 siblings, 0 replies; 11+ messages in thread
From: Lars-Peter Clausen @ 2015-05-07  5:44 UTC (permalink / raw)
  To: Michal Simek, Ulf Hansson
  Cc: Seungwon Jeon, Jaehoon Chung, Sören Brinkmann,
	Thomas Abraham, Mike Looijmans, linux-mmc, devicetree

On 05/07/2015 07:00 AM, Michal Simek wrote:
> Hi Lars,
>
> On 05/06/2015 08:31 PM, Lars-Peter Clausen wrote:
>> The device-tree documentation for the Arasan SDHCI controller already
>> claims that it supports the standard MMC devicetree properties. Update the
>> driver implementation accordingly by calling mmc_of_parse().
>>
>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
>> ---
>>   drivers/mmc/host/sdhci-of-arasan.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
>> index 21c0c08..538fd81 100644
>> --- a/drivers/mmc/host/sdhci-of-arasan.c
>> +++ b/drivers/mmc/host/sdhci-of-arasan.c
>> @@ -175,6 +175,10 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
>>   	}
>>
>>   	sdhci_get_of_property(pdev);
>> +	ret = mmc_of_parse(host->mmc);
>> +	if (ret)
>> +		goto clk_disable_all;
>> +
>>   	pltfm_host = sdhci_priv(host);
>>   	pltfm_host->priv = sdhci_arasan;
>>   	pltfm_host->clk = clk_xin;
>>
>
> I have sent this patch already and I also see it in linux-next.
> https://lkml.org/lkml/2015/4/7/21


Thanks, somehow missed that. Sorry for the noise.


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

* Re: [PATCH v2 1/4] mmc: Add support for disabling write-protect detection
  2015-05-06 18:31 [PATCH v2 1/4] mmc: Add support for disabling write-protect detection Lars-Peter Clausen
                   ` (2 preceding siblings ...)
  2015-05-06 18:31 ` [PATCH v2 4/4] mmc: dw_mmc: Use core to handle absent write protect line Lars-Peter Clausen
@ 2015-05-11 10:30 ` Ulf Hansson
  3 siblings, 0 replies; 11+ messages in thread
From: Ulf Hansson @ 2015-05-11 10:30 UTC (permalink / raw)
  To: Lars-Peter Clausen
  Cc: Seungwon Jeon, Jaehoon Chung, Michal Simek, Sören Brinkmann,
	Thomas Abraham, Mike Looijmans, linux-mmc,
	devicetree@vger.kernel.org

On 6 May 2015 at 20:31, Lars-Peter Clausen <lars@metafoo.de> wrote:
> It is not uncommon to see systems where there is no physical write-protect
> signal (e.g. when using eMMC or microSD card slots). For some controllers,
> which have a dedicated write-protection detection logic (like SDHCI
> controllers), the get_ro() callback can return bogus data in such a case.
>
> Instead of handling this on a per controller basis this patch adds a new
> capability flag to the MMC core that can be set to specify that the result
> of get_ro() is invalid. When the flag is set the core will not call
> get_ro() and assume that the card is always read-write.
>
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>

Thanks, applied!

Kind regards
Uffe

> ---
>  drivers/mmc/core/sd.c    | 30 +++++++++++++++++++++++-------
>  include/linux/mmc/host.h |  1 +
>  2 files changed, 24 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
> index 31a9ef2..8f6864a 100644
> --- a/drivers/mmc/core/sd.c
> +++ b/drivers/mmc/core/sd.c
> @@ -804,6 +804,28 @@ int mmc_sd_get_csd(struct mmc_host *host, struct mmc_card *card)
>         return 0;
>  }
>
> +static int mmc_sd_get_ro(struct mmc_host *host)
> +{
> +       int ro;
> +
> +       /*
> +        * Some systems don't feature a write-protect pin and don't need one.
> +        * E.g. because they only have micro-SD card slot. For those systems
> +        * assume that the SD card is always read-write.
> +        */
> +       if (host->caps2 & MMC_CAP2_NO_WRITE_PROTECT)
> +               return 0;
> +
> +       if (!host->ops->get_ro)
> +               return -1;
> +
> +       mmc_host_clk_hold(host);
> +       ro = host->ops->get_ro(host);
> +       mmc_host_clk_release(host);
> +
> +       return ro;
> +}
> +
>  int mmc_sd_setup_card(struct mmc_host *host, struct mmc_card *card,
>         bool reinit)
>  {
> @@ -855,13 +877,7 @@ int mmc_sd_setup_card(struct mmc_host *host, struct mmc_card *card,
>          * Check if read-only switch is active.
>          */
>         if (!reinit) {
> -               int ro = -1;
> -
> -               if (host->ops->get_ro) {
> -                       mmc_host_clk_hold(card->host);
> -                       ro = host->ops->get_ro(host);
> -                       mmc_host_clk_release(card->host);
> -               }
> +               int ro = mmc_sd_get_ro(host);
>
>                 if (ro < 0) {
>                         pr_warn("%s: host does not support reading read-only switch, assuming write-enable\n",
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index b5bedae..3b92fd4 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -285,6 +285,7 @@ struct mmc_host {
>                                  MMC_CAP2_HS400_1_2V)
>  #define MMC_CAP2_HSX00_1_2V    (MMC_CAP2_HS200_1_2V_SDR | MMC_CAP2_HS400_1_2V)
>  #define MMC_CAP2_SDIO_IRQ_NOTHREAD (1 << 17)
> +#define MMC_CAP2_NO_WRITE_PROTECT (1 << 18)    /* No physical write protect pin, assume that card is always read-write */
>
>         mmc_pm_flag_t           pm_caps;        /* supported pm features */
>
> --
> 1.8.0
>

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

* Re: [PATCH v2 2/4] mmc: dt: Allow to specify that no write protect signal is present
  2015-05-06 18:31   ` [PATCH v2 2/4] mmc: dt: Allow to specify that no write protect signal is present Lars-Peter Clausen
@ 2015-05-11 10:30     ` Ulf Hansson
  0 siblings, 0 replies; 11+ messages in thread
From: Ulf Hansson @ 2015-05-11 10:30 UTC (permalink / raw)
  To: Lars-Peter Clausen
  Cc: Seungwon Jeon, Jaehoon Chung, Michal Simek, Sören Brinkmann,
	Thomas Abraham, Mike Looijmans, linux-mmc,
	devicetree@vger.kernel.org, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala

On 6 May 2015 at 20:31, Lars-Peter Clausen <lars@metafoo.de> wrote:
> Allow to specify in the device-tree that no physical write-protect signal
> is connected to a particular instance of a MMC controller. Setting the
> property will cause the core will assume that the SD card is always
> read-write.
>
> The name for the new property is 'disable-wp' and was chosen based on the
> property with the same function from the Synopsys designware mobile storage
> host controller DT bindings specification.
>
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: Kumar Gala <galak@codeaurora.org>

Thanks, applied!

Kind regards
Uffe


> ---
>  Documentation/devicetree/bindings/mmc/mmc.txt | 5 +++++
>  drivers/mmc/core/host.c                       | 3 +++
>  2 files changed, 8 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt
> index 438899e..0384fc3 100644
> --- a/Documentation/devicetree/bindings/mmc/mmc.txt
> +++ b/Documentation/devicetree/bindings/mmc/mmc.txt
> @@ -21,6 +21,11 @@ Optional properties:
>    below for the case, when a GPIO is used for the CD line
>  - wp-inverted: when present, polarity on the WP line is inverted. See the note
>    below for the case, when a GPIO is used for the WP line
> +- disable-wp: When set no physical WP line is present. This property should
> +  only be specified when the controller has a dedicated write-protect
> +  detection logic. If a GPIO is always used for the write-protect detection
> +  logic it is sufficient to not specify wp-gpios property in the absence of a WP
> +  line.
>  - max-frequency: maximum operating clock frequency
>  - no-1-8-v: when present, denotes that 1.8v card voltage is not supported on
>    this system, even if the controller claims it is.
> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> index 8be0df7..1eabdd0 100644
> --- a/drivers/mmc/core/host.c
> +++ b/drivers/mmc/core/host.c
> @@ -400,6 +400,9 @@ int mmc_of_parse(struct mmc_host *host)
>         else if (ret != -ENOENT)
>                 return ret;
>
> +       if (of_property_read_bool(np, "disable-wp"))
> +               host->caps2 |= MMC_CAP2_NO_WRITE_PROTECT;
> +
>         /* See the comment on CD inversion above */
>         if (ro_cap_invert ^ ro_gpio_invert)
>                 host->caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
> --
> 1.8.0
>

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

* Re: [PATCH v2 4/4] mmc: dw_mmc: Use core to handle absent write protect line
       [not found]   ` <1430937082-27149-4-git-send-email-lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
@ 2015-05-11 10:30     ` Ulf Hansson
  2015-05-12  7:33       ` Jaehoon Chung
  0 siblings, 1 reply; 11+ messages in thread
From: Ulf Hansson @ 2015-05-11 10:30 UTC (permalink / raw)
  To: Lars-Peter Clausen, Jaehoon Chung
  Cc: Seungwon Jeon, Michal Simek, Sören Brinkmann, Thomas Abraham,
	Mike Looijmans, linux-mmc,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

On 6 May 2015 at 20:31, Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org> wrote:
> Use the new MMC_CAP2_NO_WRITE_PROTECT to let the core handle the case where
> no write protect line is present instead of having custom driver code to
> handle it.
>
> dw_mci_of_get_slot_quirks() is slightly refactored to directly modify the
> mmc_host capabilities instead of returning a quirk mask.
>
> Signed-off-by: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>

This looks good to me, but I leave it to Jaehoon to pick it up once tested.

Kind regards
Uffe

> ---
> Only compile tested.
> ---
>  drivers/mmc/host/dw_mmc.c  | 53 +++++++++++++++-------------------------------
>  drivers/mmc/host/dw_mmc.h  |  3 ---
>  include/linux/mmc/dw_mmc.h |  6 ------
>  3 files changed, 17 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 38b2926..0eedc5a 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -1276,10 +1276,7 @@ static int dw_mci_get_ro(struct mmc_host *mmc)
>         int gpio_ro = mmc_gpio_get_ro(mmc);
>
>         /* Use platform get_ro function, else try on board write protect */
> -       if ((slot->quirks & DW_MCI_SLOT_QUIRK_NO_WRITE_PROTECT) ||
> -                       (slot->host->quirks & DW_MCI_QUIRK_NO_WRITE_PROTECT))
> -               read_only = 0;
> -       else if (!IS_ERR_VALUE(gpio_ro))
> +       if (!IS_ERR_VALUE(gpio_ro))
>                 read_only = gpio_ro;
>         else
>                 read_only =
> @@ -2277,9 +2274,10 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
>  }
>
>  #ifdef CONFIG_OF
> -/* given a slot id, find out the device node representing that slot */
> -static struct device_node *dw_mci_of_find_slot_node(struct device *dev, u8 slot)
> +/* given a slot, find out the device node representing that slot */
> +static struct device_node *dw_mci_of_find_slot_node(struct dw_mci_slot *slot)
>  {
> +       struct device *dev = slot->mmc->parent;
>         struct device_node *np;
>         const __be32 *addr;
>         int len;
> @@ -2291,42 +2289,28 @@ static struct device_node *dw_mci_of_find_slot_node(struct device *dev, u8 slot)
>                 addr = of_get_property(np, "reg", &len);
>                 if (!addr || (len < sizeof(int)))
>                         continue;
> -               if (be32_to_cpup(addr) == slot)
> +               if (be32_to_cpup(addr) == slot->id)
>                         return np;
>         }
>         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)
> +static void dw_mci_slot_of_parse(struct dw_mci_slot *slot)
>  {
> -       struct device_node *np = dw_mci_of_find_slot_node(dev, slot);
> -       int quirks = 0;
> -       int idx;
> +       struct device_node *np = dw_mci_of_find_slot_node(slot);
>
> -       /* get quirks */
> -       for (idx = 0; idx < ARRAY_SIZE(of_slot_quirks); idx++)
> -               if (of_get_property(np, of_slot_quirks[idx].quirk, NULL)) {
> -                       dev_warn(dev, "Slot quirk %s is deprecated\n",
> -                                       of_slot_quirks[idx].quirk);
> -                       quirks |= of_slot_quirks[idx].id;
> -               }
> +       if (!np)
> +               return;
>
> -       return quirks;
> +       if (of_property_read_bool(np, "disable-wp")) {
> +               slot->mmc->caps2 |= MMC_CAP2_NO_WRITE_PROTECT;
> +               dev_warn(slot->mmc->parent,
> +                       "Slot quirk 'disable-wp' is deprecated\n");
> +       }
>  }
>  #else /* CONFIG_OF */
> -static int dw_mci_of_get_slot_quirks(struct device *dev, u8 slot)
> +static void dw_mci_slot_of_parse(struct dw_mci_slot *slot)
>  {
> -       return 0;
>  }
>  #endif /* CONFIG_OF */
>
> @@ -2349,8 +2333,6 @@ 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;
>         if (of_property_read_u32_array(host->dev->of_node,
>                                        "clock-freq-min-max", freq, 2)) {
> @@ -2388,6 +2370,8 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
>         if (host->pdata->caps2)
>                 mmc->caps2 = host->pdata->caps2;
>
> +       dw_mci_slot_of_parse(slot);
> +
>         ret = mmc_of_parse(mmc);
>         if (ret)
>                 goto err_host_allocated;
> @@ -2615,9 +2599,6 @@ static struct dw_mci_of_quirks {
>         {
>                 .quirk  = "broken-cd",
>                 .id     = DW_MCI_QUIRK_BROKEN_CARD_DETECTION,
> -       }, {
> -               .quirk  = "disable-wp",
> -               .id     = DW_MCI_QUIRK_NO_WRITE_PROTECT,
>         },
>  };
>
> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
> index f45ab91..2c6d350 100644
> --- a/drivers/mmc/host/dw_mmc.h
> +++ b/drivers/mmc/host/dw_mmc.h
> @@ -227,7 +227,6 @@ extern int dw_mci_resume(struct dw_mci *host);
>   * 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.
> @@ -245,8 +244,6 @@ struct dw_mci_slot {
>         struct mmc_host         *mmc;
>         struct dw_mci           *host;
>
> -       int                     quirks;
> -
>         u32                     ctype;
>
>         struct mmc_request      *mrq;
> diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
> index 1211199..5be9767 100644
> --- a/include/linux/mmc/dw_mmc.h
> +++ b/include/linux/mmc/dw_mmc.h
> @@ -226,12 +226,6 @@ struct dw_mci_dma_ops {
>  #define DW_MCI_QUIRK_HIGHSPEED                 BIT(2)
>  /* Unreliable card detection */
>  #define DW_MCI_QUIRK_BROKEN_CARD_DETECTION     BIT(3)
> -/* No write protect */
> -#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;
>
> --
> 1.8.0
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH v2 4/4] mmc: dw_mmc: Use core to handle absent write protect line
  2015-05-11 10:30     ` Ulf Hansson
@ 2015-05-12  7:33       ` Jaehoon Chung
  2015-05-15  9:40         ` Jaehoon Chung
  0 siblings, 1 reply; 11+ messages in thread
From: Jaehoon Chung @ 2015-05-12  7:33 UTC (permalink / raw)
  To: Ulf Hansson, Lars-Peter Clausen
  Cc: Seungwon Jeon, Michal Simek, Sören Brinkmann, Thomas Abraham,
	Mike Looijmans, linux-mmc, devicetree@vger.kernel.org

Hi,

On 05/11/2015 07:30 PM, Ulf Hansson wrote:
> On 6 May 2015 at 20:31, Lars-Peter Clausen <lars@metafoo.de> wrote:
>> Use the new MMC_CAP2_NO_WRITE_PROTECT to let the core handle the case where
>> no write protect line is present instead of having custom driver code to
>> handle it.
>>
>> dw_mci_of_get_slot_quirks() is slightly refactored to directly modify the
>> mmc_host capabilities instead of returning a quirk mask.
>>
>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> 
> This looks good to me, but I leave it to Jaehoon to pick it up once tested.

I will review and test on tomorrow. Sorry for late..
But it seems to work fine.

Best Regards,
Jaehoon Chung

> 
> Kind regards
> Uffe
> 
>> ---
>> Only compile tested.
>> ---
>>  drivers/mmc/host/dw_mmc.c  | 53 +++++++++++++++-------------------------------
>>  drivers/mmc/host/dw_mmc.h  |  3 ---
>>  include/linux/mmc/dw_mmc.h |  6 ------
>>  3 files changed, 17 insertions(+), 45 deletions(-)
>>
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 38b2926..0eedc5a 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -1276,10 +1276,7 @@ static int dw_mci_get_ro(struct mmc_host *mmc)
>>         int gpio_ro = mmc_gpio_get_ro(mmc);
>>
>>         /* Use platform get_ro function, else try on board write protect */
>> -       if ((slot->quirks & DW_MCI_SLOT_QUIRK_NO_WRITE_PROTECT) ||
>> -                       (slot->host->quirks & DW_MCI_QUIRK_NO_WRITE_PROTECT))
>> -               read_only = 0;
>> -       else if (!IS_ERR_VALUE(gpio_ro))
>> +       if (!IS_ERR_VALUE(gpio_ro))
>>                 read_only = gpio_ro;
>>         else
>>                 read_only =
>> @@ -2277,9 +2274,10 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
>>  }
>>
>>  #ifdef CONFIG_OF
>> -/* given a slot id, find out the device node representing that slot */
>> -static struct device_node *dw_mci_of_find_slot_node(struct device *dev, u8 slot)
>> +/* given a slot, find out the device node representing that slot */
>> +static struct device_node *dw_mci_of_find_slot_node(struct dw_mci_slot *slot)
>>  {
>> +       struct device *dev = slot->mmc->parent;
>>         struct device_node *np;
>>         const __be32 *addr;
>>         int len;
>> @@ -2291,42 +2289,28 @@ static struct device_node *dw_mci_of_find_slot_node(struct device *dev, u8 slot)
>>                 addr = of_get_property(np, "reg", &len);
>>                 if (!addr || (len < sizeof(int)))
>>                         continue;
>> -               if (be32_to_cpup(addr) == slot)
>> +               if (be32_to_cpup(addr) == slot->id)
>>                         return np;
>>         }
>>         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)
>> +static void dw_mci_slot_of_parse(struct dw_mci_slot *slot)
>>  {
>> -       struct device_node *np = dw_mci_of_find_slot_node(dev, slot);
>> -       int quirks = 0;
>> -       int idx;
>> +       struct device_node *np = dw_mci_of_find_slot_node(slot);
>>
>> -       /* get quirks */
>> -       for (idx = 0; idx < ARRAY_SIZE(of_slot_quirks); idx++)
>> -               if (of_get_property(np, of_slot_quirks[idx].quirk, NULL)) {
>> -                       dev_warn(dev, "Slot quirk %s is deprecated\n",
>> -                                       of_slot_quirks[idx].quirk);
>> -                       quirks |= of_slot_quirks[idx].id;
>> -               }
>> +       if (!np)
>> +               return;
>>
>> -       return quirks;
>> +       if (of_property_read_bool(np, "disable-wp")) {
>> +               slot->mmc->caps2 |= MMC_CAP2_NO_WRITE_PROTECT;
>> +               dev_warn(slot->mmc->parent,
>> +                       "Slot quirk 'disable-wp' is deprecated\n");
>> +       }
>>  }
>>  #else /* CONFIG_OF */
>> -static int dw_mci_of_get_slot_quirks(struct device *dev, u8 slot)
>> +static void dw_mci_slot_of_parse(struct dw_mci_slot *slot)
>>  {
>> -       return 0;
>>  }
>>  #endif /* CONFIG_OF */
>>
>> @@ -2349,8 +2333,6 @@ 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;
>>         if (of_property_read_u32_array(host->dev->of_node,
>>                                        "clock-freq-min-max", freq, 2)) {
>> @@ -2388,6 +2370,8 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
>>         if (host->pdata->caps2)
>>                 mmc->caps2 = host->pdata->caps2;
>>
>> +       dw_mci_slot_of_parse(slot);
>> +
>>         ret = mmc_of_parse(mmc);
>>         if (ret)
>>                 goto err_host_allocated;
>> @@ -2615,9 +2599,6 @@ static struct dw_mci_of_quirks {
>>         {
>>                 .quirk  = "broken-cd",
>>                 .id     = DW_MCI_QUIRK_BROKEN_CARD_DETECTION,
>> -       }, {
>> -               .quirk  = "disable-wp",
>> -               .id     = DW_MCI_QUIRK_NO_WRITE_PROTECT,
>>         },
>>  };
>>
>> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
>> index f45ab91..2c6d350 100644
>> --- a/drivers/mmc/host/dw_mmc.h
>> +++ b/drivers/mmc/host/dw_mmc.h
>> @@ -227,7 +227,6 @@ extern int dw_mci_resume(struct dw_mci *host);
>>   * 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.
>> @@ -245,8 +244,6 @@ struct dw_mci_slot {
>>         struct mmc_host         *mmc;
>>         struct dw_mci           *host;
>>
>> -       int                     quirks;
>> -
>>         u32                     ctype;
>>
>>         struct mmc_request      *mrq;
>> diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
>> index 1211199..5be9767 100644
>> --- a/include/linux/mmc/dw_mmc.h
>> +++ b/include/linux/mmc/dw_mmc.h
>> @@ -226,12 +226,6 @@ struct dw_mci_dma_ops {
>>  #define DW_MCI_QUIRK_HIGHSPEED                 BIT(2)
>>  /* Unreliable card detection */
>>  #define DW_MCI_QUIRK_BROKEN_CARD_DETECTION     BIT(3)
>> -/* No write protect */
>> -#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;
>>
>> --
>> 1.8.0
>>
> 


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

* Re: [PATCH v2 4/4] mmc: dw_mmc: Use core to handle absent write protect line
  2015-05-12  7:33       ` Jaehoon Chung
@ 2015-05-15  9:40         ` Jaehoon Chung
  0 siblings, 0 replies; 11+ messages in thread
From: Jaehoon Chung @ 2015-05-15  9:40 UTC (permalink / raw)
  To: Jaehoon Chung, Ulf Hansson, Lars-Peter Clausen
  Cc: Seungwon Jeon, Michal Simek, Sören Brinkmann, Thomas Abraham,
	Mike Looijmans, linux-mmc, devicetree@vger.kernel.org

Hi, Lars-Peter

I have tested this patch on my board. It's fine.
Looks good to me. I will apply this at my dw-mmc tree.

Best Regards,
Jaehoon Chung

On 05/12/2015 04:33 PM, Jaehoon Chung wrote:
> Hi,
> 
> On 05/11/2015 07:30 PM, Ulf Hansson wrote:
>> On 6 May 2015 at 20:31, Lars-Peter Clausen <lars@metafoo.de> wrote:
>>> Use the new MMC_CAP2_NO_WRITE_PROTECT to let the core handle the case where
>>> no write protect line is present instead of having custom driver code to
>>> handle it.
>>>
>>> dw_mci_of_get_slot_quirks() is slightly refactored to directly modify the
>>> mmc_host capabilities instead of returning a quirk mask.
>>>
>>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
>>
>> This looks good to me, but I leave it to Jaehoon to pick it up once tested.
> 
> I will review and test on tomorrow. Sorry for late..
> But it seems to work fine.
> 
> Best Regards,
> Jaehoon Chung
> 
>>
>> Kind regards
>> Uffe
>>
>>> ---
>>> Only compile tested.
>>> ---
>>>  drivers/mmc/host/dw_mmc.c  | 53 +++++++++++++++-------------------------------
>>>  drivers/mmc/host/dw_mmc.h  |  3 ---
>>>  include/linux/mmc/dw_mmc.h |  6 ------
>>>  3 files changed, 17 insertions(+), 45 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>>> index 38b2926..0eedc5a 100644
>>> --- a/drivers/mmc/host/dw_mmc.c
>>> +++ b/drivers/mmc/host/dw_mmc.c
>>> @@ -1276,10 +1276,7 @@ static int dw_mci_get_ro(struct mmc_host *mmc)
>>>         int gpio_ro = mmc_gpio_get_ro(mmc);
>>>
>>>         /* Use platform get_ro function, else try on board write protect */
>>> -       if ((slot->quirks & DW_MCI_SLOT_QUIRK_NO_WRITE_PROTECT) ||
>>> -                       (slot->host->quirks & DW_MCI_QUIRK_NO_WRITE_PROTECT))
>>> -               read_only = 0;
>>> -       else if (!IS_ERR_VALUE(gpio_ro))
>>> +       if (!IS_ERR_VALUE(gpio_ro))
>>>                 read_only = gpio_ro;
>>>         else
>>>                 read_only =
>>> @@ -2277,9 +2274,10 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
>>>  }
>>>
>>>  #ifdef CONFIG_OF
>>> -/* given a slot id, find out the device node representing that slot */
>>> -static struct device_node *dw_mci_of_find_slot_node(struct device *dev, u8 slot)
>>> +/* given a slot, find out the device node representing that slot */
>>> +static struct device_node *dw_mci_of_find_slot_node(struct dw_mci_slot *slot)
>>>  {
>>> +       struct device *dev = slot->mmc->parent;
>>>         struct device_node *np;
>>>         const __be32 *addr;
>>>         int len;
>>> @@ -2291,42 +2289,28 @@ static struct device_node *dw_mci_of_find_slot_node(struct device *dev, u8 slot)
>>>                 addr = of_get_property(np, "reg", &len);
>>>                 if (!addr || (len < sizeof(int)))
>>>                         continue;
>>> -               if (be32_to_cpup(addr) == slot)
>>> +               if (be32_to_cpup(addr) == slot->id)
>>>                         return np;
>>>         }
>>>         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)
>>> +static void dw_mci_slot_of_parse(struct dw_mci_slot *slot)
>>>  {
>>> -       struct device_node *np = dw_mci_of_find_slot_node(dev, slot);
>>> -       int quirks = 0;
>>> -       int idx;
>>> +       struct device_node *np = dw_mci_of_find_slot_node(slot);
>>>
>>> -       /* get quirks */
>>> -       for (idx = 0; idx < ARRAY_SIZE(of_slot_quirks); idx++)
>>> -               if (of_get_property(np, of_slot_quirks[idx].quirk, NULL)) {
>>> -                       dev_warn(dev, "Slot quirk %s is deprecated\n",
>>> -                                       of_slot_quirks[idx].quirk);
>>> -                       quirks |= of_slot_quirks[idx].id;
>>> -               }
>>> +       if (!np)
>>> +               return;
>>>
>>> -       return quirks;
>>> +       if (of_property_read_bool(np, "disable-wp")) {
>>> +               slot->mmc->caps2 |= MMC_CAP2_NO_WRITE_PROTECT;
>>> +               dev_warn(slot->mmc->parent,
>>> +                       "Slot quirk 'disable-wp' is deprecated\n");
>>> +       }
>>>  }
>>>  #else /* CONFIG_OF */
>>> -static int dw_mci_of_get_slot_quirks(struct device *dev, u8 slot)
>>> +static void dw_mci_slot_of_parse(struct dw_mci_slot *slot)
>>>  {
>>> -       return 0;
>>>  }
>>>  #endif /* CONFIG_OF */
>>>
>>> @@ -2349,8 +2333,6 @@ 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;
>>>         if (of_property_read_u32_array(host->dev->of_node,
>>>                                        "clock-freq-min-max", freq, 2)) {
>>> @@ -2388,6 +2370,8 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
>>>         if (host->pdata->caps2)
>>>                 mmc->caps2 = host->pdata->caps2;
>>>
>>> +       dw_mci_slot_of_parse(slot);
>>> +
>>>         ret = mmc_of_parse(mmc);
>>>         if (ret)
>>>                 goto err_host_allocated;
>>> @@ -2615,9 +2599,6 @@ static struct dw_mci_of_quirks {
>>>         {
>>>                 .quirk  = "broken-cd",
>>>                 .id     = DW_MCI_QUIRK_BROKEN_CARD_DETECTION,
>>> -       }, {
>>> -               .quirk  = "disable-wp",
>>> -               .id     = DW_MCI_QUIRK_NO_WRITE_PROTECT,
>>>         },
>>>  };
>>>
>>> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
>>> index f45ab91..2c6d350 100644
>>> --- a/drivers/mmc/host/dw_mmc.h
>>> +++ b/drivers/mmc/host/dw_mmc.h
>>> @@ -227,7 +227,6 @@ extern int dw_mci_resume(struct dw_mci *host);
>>>   * 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.
>>> @@ -245,8 +244,6 @@ struct dw_mci_slot {
>>>         struct mmc_host         *mmc;
>>>         struct dw_mci           *host;
>>>
>>> -       int                     quirks;
>>> -
>>>         u32                     ctype;
>>>
>>>         struct mmc_request      *mrq;
>>> diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
>>> index 1211199..5be9767 100644
>>> --- a/include/linux/mmc/dw_mmc.h
>>> +++ b/include/linux/mmc/dw_mmc.h
>>> @@ -226,12 +226,6 @@ struct dw_mci_dma_ops {
>>>  #define DW_MCI_QUIRK_HIGHSPEED                 BIT(2)
>>>  /* Unreliable card detection */
>>>  #define DW_MCI_QUIRK_BROKEN_CARD_DETECTION     BIT(3)
>>> -/* No write protect */
>>> -#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;
>>>
>>> --
>>> 1.8.0
>>>
>>
> 
> --
> 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
> 


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

end of thread, other threads:[~2015-05-15  9:40 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-05-06 18:31 [PATCH v2 1/4] mmc: Add support for disabling write-protect detection Lars-Peter Clausen
     [not found] ` <1430937082-27149-1-git-send-email-lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
2015-05-06 18:31   ` [PATCH v2 2/4] mmc: dt: Allow to specify that no write protect signal is present Lars-Peter Clausen
2015-05-11 10:30     ` Ulf Hansson
2015-05-06 18:31 ` [PATCH v2 3/4] mmc: sdhci-of-arasan: Call mmc_of_parse() Lars-Peter Clausen
2015-05-07  5:00   ` Michal Simek
2015-05-07  5:44     ` Lars-Peter Clausen
2015-05-06 18:31 ` [PATCH v2 4/4] mmc: dw_mmc: Use core to handle absent write protect line Lars-Peter Clausen
     [not found]   ` <1430937082-27149-4-git-send-email-lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
2015-05-11 10:30     ` Ulf Hansson
2015-05-12  7:33       ` Jaehoon Chung
2015-05-15  9:40         ` Jaehoon Chung
2015-05-11 10:30 ` [PATCH v2 1/4] mmc: Add support for disabling write-protect detection Ulf Hansson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).