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

2013-01-14 Thread 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 diand...@chromium.org
 Acked-by: Seungwon Jeon tgih@samsung.com

Acked-by: Olof Johansson o...@lixom.net

Nit below.

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

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


-Olof
--
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


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

2013-01-11 Thread 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 diand...@chromium.org
Acked-by: Seungwon Jeon tgih@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