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

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

[Doug Anderson]

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

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

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

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

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

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

[Seungwon Jeon]

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

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

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

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

[Doug Anderson]

Chris,

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

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

Thanks much!  :)

-Doug

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

[Doug Anderson]

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

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

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

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

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

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

[Olof Johansson]

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

Hi,

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

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


-Olof

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

[Doug Anderson]

Olof,

Thanks for your comments.

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

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

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

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


Thanks!

-Doug

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

[Doug Anderson]

Hi,

A followup as I did more investigation here...

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

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

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

-Doug

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

[Doug Anderson]

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

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

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

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

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

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

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

[Will Newton]

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

This looks ok to me too.

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

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

[Olof Johansson]

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

Acked-by: Olof Johansson <olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org>

Nit below.

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

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


-Olof