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