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.jun@xxxxxxxxxxx> 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 <dianders@xxxxxxxxxxxx> >> >> --- >> 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html