Re: [PATCH v2 1/2] mmc: dw_mmc: exynos: Stop claiming wp-gpio

2012-11-29 Thread Doug Anderson
Seungwon,


On Wed, Nov 28, 2012 at 11:46 PM, Seungwon Jeon tgih@samsung.com wrote:
 Hi Doug,

 On Thursday, November 29, 2012, Doug Anderson wrote:
 Seungwon,

 Thanks for the review.  See below for comments.  If you'd like me to
 respin then please let me know.  Otherwise I look forward to your ack.

 On Wed, Nov 28, 2012 at 1:29 AM, Seungwon Jeon tgih@samsung.com wrote:
  Yes. pin of write protection is common property.
  This change is good. I have some suggestion below.
  Could you check it?
 
  On Friday, November 23, 2012, Doug Anderson wrote:
  The exynos code claimed wp-gpio with devm_gpio_request() but never did
  anything with it.  That meant that anyone using a write protect GPIO
  would effectively be write protected all the time.
 
  A future change will move the wp-gpio support to the core dw_mmc.c
  file.  Now the exynos-specific code won't claim the GPIO but will
  just set the DW_MCI_QUIRK_NO_WRITE_PROTECT quirk if write protect
  won't be used.
 
  Signed-off-by: Doug Anderson diand...@chromium.org
 
  ---
  Changes in v2:
  - Nothing new in this patch
 
   drivers/mmc/host/dw_mmc-exynos.c |   12 ++--
   1 files changed, 6 insertions(+), 6 deletions(-)
 
  diff --git a/drivers/mmc/host/dw_mmc-exynos.c 
  b/drivers/mmc/host/dw_mmc-exynos.c
  index 4d50da6..58cc03e 100644
  --- a/drivers/mmc/host/dw_mmc-exynos.c
  +++ b/drivers/mmc/host/dw_mmc-exynos.c
  @@ -175,12 +175,12 @@ static int dw_mci_exynos_setup_bus(struct dw_mci 
  *host,
}
}
 
  - gpio = of_get_named_gpio(slot_np, wp-gpios, 0);
  - if (gpio_is_valid(gpio)) {
  - if (devm_gpio_request(host-dev, gpio, dw-mci-wp))
  - dev_info(host-dev, gpio [%d] request failed\n,
  - gpio);
  - } else {
  + /*
  +  * If there are no write-protect GPIOs present then we assume no 
  write
  +  * protect.  The mci_readl() in dw_mmc.c won't work since it's not
  +  * hooked up on exynos.
  +  */
  + if (!of_find_property(slot_np, wp-gpios, NULL)) {
dev_info(host-dev, wp gpio not available);
host-pdata-quirks |= DW_MCI_QUIRK_NO_WRITE_PROTECT;
}
  All card types need this quirk in case wp-gpio property is empty?
  I think wp-pin is valid for SD card, not eMMC/SDIO.

 Right.  It is only checked right now by the SD code (mmc/core/sd.c).
 It doesn't particularly hurt to set it the quirk in other cases though
 and it seems nice not to add special cases.  I could imagine someone
 extending the MMC code at some point to support write protect (via
 GPIO) for eMMC, so there's even a slight justification for avoiding
 the special case.


  Of course, I know origin code did it.
  How about removing whole checking routine?
  Instead, new definition for this quirk can be added into 
  'dw_mci_of_quirks'(dw_mmc.c) and dts file.

 On _exynos_ all SD cards need this quirk if there is no wp-gpio
 property.  However this is not generally true for all users of dw_mmc.
  The DesignWare IP Block actually has a write protect input that can
 be read with mci_readl(slot-host, WRTPRT) but on exynos the
 DesignWare write protect line isn't exposed on any physical pins.
 That means that the only possible way to do write protect on exynos is
 using a GPIO.

 The above means that on exynos if the GPIO isn't defined we will
 assume no write protect.  On other platforms if the GPIO isn't defined
 we'll assume that the mci_readl will work and we'll use that.

 If people would prefer it I can code up an alternate solution that
 doesn't touch any exynos code but that would introduce a new device
 tree binding.  We could accomplish what's needed for exynos using a
 property like broken-internal-wp.

 Please let me know if you'd like me to submit a new patch with this
 solution or if you like the existing solution.

 Write protect is additional interface related with SD socket.
 WP switch appears in SD standard size card.
 In case EMMC/SDIO spec, there is no mentions about this WP pin.
 As you mentioned above, that's why 'ger_ro' is called only in sd 
 path(mmc/core/sd.c).
 So, I meant that we don't need to consider WP pin status about non-SD type.

Ah, I understand now.  This is a good point.  I have updated the
documentation in the latest patch to mention this.  Thanks!



 Such as exynos5250, there is no exposed interface from host controller for 
 write protection pin.
 In that case, if general gpio pin is connected like your board environment, 
 we can define wp-gpio.
 Otherwise, 'broken-internal-wp' property will be good solution.

Latest patch (just about to send out) adds a per-slot disable-wp
property for dw_mmc.  See the patch for why I've chosen to do it that
way.  Hopefully it looks OK to you.



 Please feel free to modify.
 If you will do, I'll be happy.

 Thanks,
 Seungwon Jeon
  --
  1.7.7.3
 
 --
 To unsubscribe from this list: send the line unsubscribe linux-mmc in
 the body of 

RE: [PATCH v2 1/2] mmc: dw_mmc: exynos: Stop claiming wp-gpio

2012-11-28 Thread Seungwon Jeon
Yes. pin of write protection is common property.
This change is good. I have some suggestion below.
Could you check it?

On Friday, November 23, 2012, Doug Anderson wrote:
 The exynos code claimed wp-gpio with devm_gpio_request() but never did
 anything with it.  That meant that anyone using a write protect GPIO
 would effectively be write protected all the time.
 
 A future change will move the wp-gpio support to the core dw_mmc.c
 file.  Now the exynos-specific code won't claim the GPIO but will
 just set the DW_MCI_QUIRK_NO_WRITE_PROTECT quirk if write protect
 won't be used.
 
 Signed-off-by: Doug Anderson diand...@chromium.org
 
 ---
 Changes in v2:
 - Nothing new in this patch
 
  drivers/mmc/host/dw_mmc-exynos.c |   12 ++--
  1 files changed, 6 insertions(+), 6 deletions(-)
 
 diff --git a/drivers/mmc/host/dw_mmc-exynos.c 
 b/drivers/mmc/host/dw_mmc-exynos.c
 index 4d50da6..58cc03e 100644
 --- a/drivers/mmc/host/dw_mmc-exynos.c
 +++ b/drivers/mmc/host/dw_mmc-exynos.c
 @@ -175,12 +175,12 @@ static int dw_mci_exynos_setup_bus(struct dw_mci *host,
   }
   }
 
 - gpio = of_get_named_gpio(slot_np, wp-gpios, 0);
 - if (gpio_is_valid(gpio)) {
 - if (devm_gpio_request(host-dev, gpio, dw-mci-wp))
 - dev_info(host-dev, gpio [%d] request failed\n,
 - gpio);
 - } else {
 + /*
 +  * If there are no write-protect GPIOs present then we assume no write
 +  * protect.  The mci_readl() in dw_mmc.c won't work since it's not
 +  * hooked up on exynos.
 +  */
 + if (!of_find_property(slot_np, wp-gpios, NULL)) {
   dev_info(host-dev, wp gpio not available);
   host-pdata-quirks |= DW_MCI_QUIRK_NO_WRITE_PROTECT;
   }
All card types need this quirk in case wp-gpio property is empty?
I think wp-pin is valid for SD card, not eMMC/SDIO.
Of course, I know origin code did it.
How about removing whole checking routine?
Instead, new definition for this quirk can be added into 
'dw_mci_of_quirks'(dw_mmc.c) and dts file.

Thanks,
Seungwon Jeon
 --
 1.7.7.3

--
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 v2 1/2] mmc: dw_mmc: exynos: Stop claiming wp-gpio

2012-11-28 Thread Doug Anderson
Seungwon,

Thanks for the review.  See below for comments.  If you'd like me to
respin then please let me know.  Otherwise I look forward to your ack.

On Wed, Nov 28, 2012 at 1:29 AM, Seungwon Jeon tgih@samsung.com wrote:
 Yes. pin of write protection is common property.
 This change is good. I have some suggestion below.
 Could you check it?

 On Friday, November 23, 2012, Doug Anderson wrote:
 The exynos code claimed wp-gpio with devm_gpio_request() but never did
 anything with it.  That meant that anyone using a write protect GPIO
 would effectively be write protected all the time.

 A future change will move the wp-gpio support to the core dw_mmc.c
 file.  Now the exynos-specific code won't claim the GPIO but will
 just set the DW_MCI_QUIRK_NO_WRITE_PROTECT quirk if write protect
 won't be used.

 Signed-off-by: Doug Anderson diand...@chromium.org

 ---
 Changes in v2:
 - Nothing new in this patch

  drivers/mmc/host/dw_mmc-exynos.c |   12 ++--
  1 files changed, 6 insertions(+), 6 deletions(-)

 diff --git a/drivers/mmc/host/dw_mmc-exynos.c 
 b/drivers/mmc/host/dw_mmc-exynos.c
 index 4d50da6..58cc03e 100644
 --- a/drivers/mmc/host/dw_mmc-exynos.c
 +++ b/drivers/mmc/host/dw_mmc-exynos.c
 @@ -175,12 +175,12 @@ static int dw_mci_exynos_setup_bus(struct dw_mci *host,
   }
   }

 - gpio = of_get_named_gpio(slot_np, wp-gpios, 0);
 - if (gpio_is_valid(gpio)) {
 - if (devm_gpio_request(host-dev, gpio, dw-mci-wp))
 - dev_info(host-dev, gpio [%d] request failed\n,
 - gpio);
 - } else {
 + /*
 +  * If there are no write-protect GPIOs present then we assume no write
 +  * protect.  The mci_readl() in dw_mmc.c won't work since it's not
 +  * hooked up on exynos.
 +  */
 + if (!of_find_property(slot_np, wp-gpios, NULL)) {
   dev_info(host-dev, wp gpio not available);
   host-pdata-quirks |= DW_MCI_QUIRK_NO_WRITE_PROTECT;
   }
 All card types need this quirk in case wp-gpio property is empty?
 I think wp-pin is valid for SD card, not eMMC/SDIO.

Right.  It is only checked right now by the SD code (mmc/core/sd.c).
It doesn't particularly hurt to set it the quirk in other cases though
and it seems nice not to add special cases.  I could imagine someone
extending the MMC code at some point to support write protect (via
GPIO) for eMMC, so there's even a slight justification for avoiding
the special case.


 Of course, I know origin code did it.
 How about removing whole checking routine?
 Instead, new definition for this quirk can be added into 
 'dw_mci_of_quirks'(dw_mmc.c) and dts file.

On _exynos_ all SD cards need this quirk if there is no wp-gpio
property.  However this is not generally true for all users of dw_mmc.
 The DesignWare IP Block actually has a write protect input that can
be read with mci_readl(slot-host, WRTPRT) but on exynos the
DesignWare write protect line isn't exposed on any physical pins.
That means that the only possible way to do write protect on exynos is
using a GPIO.

The above means that on exynos if the GPIO isn't defined we will
assume no write protect.  On other platforms if the GPIO isn't defined
we'll assume that the mci_readl will work and we'll use that.

If people would prefer it I can code up an alternate solution that
doesn't touch any exynos code but that would introduce a new device
tree binding.  We could accomplish what's needed for exynos using a
property like broken-internal-wp.

Please let me know if you'd like me to submit a new patch with this
solution or if you like the existing solution.


 Thanks,
 Seungwon Jeon
 --
 1.7.7.3

--
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 v2 1/2] mmc: dw_mmc: exynos: Stop claiming wp-gpio

2012-11-28 Thread Seungwon Jeon
Hi Doug,

On Thursday, November 29, 2012, Doug Anderson wrote:
 Seungwon,
 
 Thanks for the review.  See below for comments.  If you'd like me to
 respin then please let me know.  Otherwise I look forward to your ack.
 
 On Wed, Nov 28, 2012 at 1:29 AM, Seungwon Jeon tgih@samsung.com wrote:
  Yes. pin of write protection is common property.
  This change is good. I have some suggestion below.
  Could you check it?
 
  On Friday, November 23, 2012, Doug Anderson wrote:
  The exynos code claimed wp-gpio with devm_gpio_request() but never did
  anything with it.  That meant that anyone using a write protect GPIO
  would effectively be write protected all the time.
 
  A future change will move the wp-gpio support to the core dw_mmc.c
  file.  Now the exynos-specific code won't claim the GPIO but will
  just set the DW_MCI_QUIRK_NO_WRITE_PROTECT quirk if write protect
  won't be used.
 
  Signed-off-by: Doug Anderson diand...@chromium.org
 
  ---
  Changes in v2:
  - Nothing new in this patch
 
   drivers/mmc/host/dw_mmc-exynos.c |   12 ++--
   1 files changed, 6 insertions(+), 6 deletions(-)
 
  diff --git a/drivers/mmc/host/dw_mmc-exynos.c 
  b/drivers/mmc/host/dw_mmc-exynos.c
  index 4d50da6..58cc03e 100644
  --- a/drivers/mmc/host/dw_mmc-exynos.c
  +++ b/drivers/mmc/host/dw_mmc-exynos.c
  @@ -175,12 +175,12 @@ static int dw_mci_exynos_setup_bus(struct dw_mci 
  *host,
}
}
 
  - gpio = of_get_named_gpio(slot_np, wp-gpios, 0);
  - if (gpio_is_valid(gpio)) {
  - if (devm_gpio_request(host-dev, gpio, dw-mci-wp))
  - dev_info(host-dev, gpio [%d] request failed\n,
  - gpio);
  - } else {
  + /*
  +  * If there are no write-protect GPIOs present then we assume no 
  write
  +  * protect.  The mci_readl() in dw_mmc.c won't work since it's not
  +  * hooked up on exynos.
  +  */
  + if (!of_find_property(slot_np, wp-gpios, NULL)) {
dev_info(host-dev, wp gpio not available);
host-pdata-quirks |= DW_MCI_QUIRK_NO_WRITE_PROTECT;
}
  All card types need this quirk in case wp-gpio property is empty?
  I think wp-pin is valid for SD card, not eMMC/SDIO.
 
 Right.  It is only checked right now by the SD code (mmc/core/sd.c).
 It doesn't particularly hurt to set it the quirk in other cases though
 and it seems nice not to add special cases.  I could imagine someone
 extending the MMC code at some point to support write protect (via
 GPIO) for eMMC, so there's even a slight justification for avoiding
 the special case.
 
 
  Of course, I know origin code did it.
  How about removing whole checking routine?
  Instead, new definition for this quirk can be added into 
  'dw_mci_of_quirks'(dw_mmc.c) and dts file.
 
 On _exynos_ all SD cards need this quirk if there is no wp-gpio
 property.  However this is not generally true for all users of dw_mmc.
  The DesignWare IP Block actually has a write protect input that can
 be read with mci_readl(slot-host, WRTPRT) but on exynos the
 DesignWare write protect line isn't exposed on any physical pins.
 That means that the only possible way to do write protect on exynos is
 using a GPIO.
 
 The above means that on exynos if the GPIO isn't defined we will
 assume no write protect.  On other platforms if the GPIO isn't defined
 we'll assume that the mci_readl will work and we'll use that.
 
 If people would prefer it I can code up an alternate solution that
 doesn't touch any exynos code but that would introduce a new device
 tree binding.  We could accomplish what's needed for exynos using a
 property like broken-internal-wp.
 
 Please let me know if you'd like me to submit a new patch with this
 solution or if you like the existing solution.
 
Write protect is additional interface related with SD socket. 
WP switch appears in SD standard size card. 
In case EMMC/SDIO spec, there is no mentions about this WP pin.
As you mentioned above, that's why 'ger_ro' is called only in sd 
path(mmc/core/sd.c).
So, I meant that we don't need to consider WP pin status about non-SD type.

Such as exynos5250, there is no exposed interface from host controller for 
write protection pin.
In that case, if general gpio pin is connected like your board environment, we 
can define wp-gpio.
Otherwise, 'broken-internal-wp' property will be good solution.

Please feel free to modify.
If you will do, I'll be happy.

Thanks,
Seungwon Jeon
  --
  1.7.7.3
 
 --
 To unsubscribe from this list: send the line unsubscribe linux-mmc in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
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 v2 1/2] mmc: dw_mmc: exynos: Stop claiming wp-gpio

2012-11-22 Thread Doug Anderson
The exynos code claimed wp-gpio with devm_gpio_request() but never did
anything with it.  That meant that anyone using a write protect GPIO
would effectively be write protected all the time.

A future change will move the wp-gpio support to the core dw_mmc.c
file.  Now the exynos-specific code won't claim the GPIO but will
just set the DW_MCI_QUIRK_NO_WRITE_PROTECT quirk if write protect
won't be used.

Signed-off-by: Doug Anderson diand...@chromium.org

---
Changes in v2:
- Nothing new in this patch

 drivers/mmc/host/dw_mmc-exynos.c |   12 ++--
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
index 4d50da6..58cc03e 100644
--- a/drivers/mmc/host/dw_mmc-exynos.c
+++ b/drivers/mmc/host/dw_mmc-exynos.c
@@ -175,12 +175,12 @@ static int dw_mci_exynos_setup_bus(struct dw_mci *host,
}
}
 
-   gpio = of_get_named_gpio(slot_np, wp-gpios, 0);
-   if (gpio_is_valid(gpio)) {
-   if (devm_gpio_request(host-dev, gpio, dw-mci-wp))
-   dev_info(host-dev, gpio [%d] request failed\n,
-   gpio);
-   } else {
+   /*
+* If there are no write-protect GPIOs present then we assume no write
+* protect.  The mci_readl() in dw_mmc.c won't work since it's not
+* hooked up on exynos.
+*/
+   if (!of_find_property(slot_np, wp-gpios, NULL)) {
dev_info(host-dev, wp gpio not available);
host-pdata-quirks |= DW_MCI_QUIRK_NO_WRITE_PROTECT;
}
-- 
1.7.7.3

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