linux-kernel - Re: [PATCH v2] mmc: dw_mmc: add hw

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:	Thu, 14 Jan 2016 08:58:07 +0800
From:	Shawn Lin <shawn.lin@...k-chips.com>
To:	Jaehoon Chung <jh80.chung@...sung.com>
Cc:	shawn.lin@...k-chips.com, Ulf Hansson <ulf.hansson@...aro.org>,
	linux-mmc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] mmc: dw_mmc: add hw_reset support

On 2016/1/12 10:37, Jaehoon Chung wrote:
> Hi, Shawn.
>
> On 01/04/2016 04:24 PM, Shawn Lin wrote:
>> On 2016/1/4 13:52, Jaehoon Chung wrote:
>>> Hi, Shawn.
>>>
>>> On 01/04/2016 11:50 AM, Shawn Lin wrote:
>>>> This patch implement hw_reset function for DesignWare
>>>> MMC controller. By adding this feature, mmc blk can
>>>> do some basic recovery.
>>>>
>>>> >From Synopsys DesignWare Cores Mobile Storage Host Databook
>>>> (Section 7.4.4), we get the details:
>>>> 1. Program CMD12 to end any transfer in process.
>>>> 2. Wait for DTO, even if no response is sent back by the card.
>>>> 3. Set the following resets:
>>>>      Software reset – BMOD[0] for IDMAC only
>>>>      DMA reset– CTRL[2]
>>>>      FIFO reset – CTRL[1] bits
>>>> 4. Program the CARD_RESET register with a value of 0 for the bit
>>>> corresponding to the card number; This can be done at any time when
>>>> the card is connected to the controller. This programming asserts the
>>>> RST_n signal and resets the card.
>>>> 5. Wait for minimum of 1 μs or cclk_in period, which ever is greater
>>>> 6. After a minimum of 1 μs, the application should program a value of
>>>> 0 into the CARD_RESET register for the bit corresponding to the card
>>>> number. This de-asserts the RST_n signal and takes the card out of reset.
>>>> 7. The application can program a new CMD only after a minimum of 200 us
>>>> after the de-assertion of the RST_n signal, as per the MMC 4.41 standard.
>>>>
>>>> HW reset producer will be call in mmc_init_card before mmc_go_idle. At that
>>>> time，dw mmc hasn't update clk for itself, so CMD12 is inappropriate and
>>>> unnecessary. Moreover, if mmc device runs into broken states, DRTO or RTO
>>>> generated by previous cmd w/ data will make mmc core issue stop already. Then
>>>> it will retry again and again, issue stop and card status again until the
>>>> cmd's retry number decrease to zero. That will finally trigger HW reset producer
>>>> if we declare MMC_CAP_HW_RESET. So there's no need to do step 1 and 2 for the
>>>> reasons we mentioned above.
>
> I think it doesn't need to write the all contents mentioned in Designware IP Spec.
> So As you possible, how about adding the more simple commit message than now?
>

:) I will simplify the commit msg in v3. Thanks.

> Best Regards,
> Jaehoon Chung
>
>>>
>>> Which IP version did you use? Could you share it..?
>>> Did you know about CHIP RESET sequence?
>>>
>>
>> My dw_mmc IP version is 270a（0x5342270a from VERID register).
>> Do you mean "emmc chip hardware reset sequence"?
>>  From JESD84-B51 spec(section 6.15.10),
>>
>> host pull down reset pin for at least 1us, and pull high it.
>> then emmc device will detect it as a IO interrupt to enter internal reset flow. Host should not issue CMD1 within 200us, which means emmc device MUST finish internal reset flow within 200us.
>>
>>>>
>>>> This implementation can be easily tested by cutting off->On vmmc while doing data
>>>> accessing in background to simulate that case. And dw_mmc can generate timeout
>>>> interrupt and make mmc core trigger hw reset producer before re-init mmc card
>>>> to recover itself.
>>>>
>>>> Signed-off-by: Shawn Lin <shawn.lin@...k-chips.com>
>>>>
>>>> ---
>>>>
>>>> Changes in v2:
>>>> - remove unecessary mb
>>>> - reduce time cost for hw_reset
>>>> - combine SDMMC_CTRL_DMA_RESET and SDMMC_CTRL_FIFO_RESET
>>>>
>>>>    drivers/mmc/host/dw_mmc.c | 25 +++++++++++++++++++++++++
>>>>    drivers/mmc/host/dw_mmc.h |  4 ++++
>>>>    2 files changed, 29 insertions(+)
>>>>
>>>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>>>> index 7128351..98e75fc 100644
>>>> --- a/drivers/mmc/host/dw_mmc.c
>>>> +++ b/drivers/mmc/host/dw_mmc.c
>>>> @@ -1477,6 +1477,30 @@ static int dw_mci_get_cd(struct mmc_host *mmc)
>>>>        return present;
>>>>    }
>>>>
>>>> +static void dw_mci_hw_reset(struct mmc_host *mmc)
>>>> +{
>>>> +    struct dw_mci_slot *slot = mmc_priv(mmc);
>>>> +    struct dw_mci *host = slot->host;
>>>> +
>>>> +    if (host->use_dma == TRANS_MODE_IDMAC)
>>>> +        dw_mci_idmac_reset(host);
>>>> +
>>>> +    if (!dw_mci_ctrl_reset(host, SDMMC_CTRL_DMA_RESET |
>>>> +                     SDMMC_CTRL_FIFO_RESET))
>>>> +        return;
>>>> +
>>>> +    /*
>>>> +     * According to eMMC spec, card reset procedure:
>>>> +     * tRstW >= 1us:   RST_n pulse width
>>>> +     * tRSCA >= 200us: RST_n to Command time
>>>> +     * tRSTH >= 1us:   RST_n high period
>>>> +     */
>>>> +     mci_writel(slot->host, RST_N, SDMMC_RST_HWRESET);
>>>
>>> Even though we used only one slot, but it needs to control bit with slot->id.
>>>
>>
>> yep, got it.
>>
>>> Best Regards,
>>> Jaehoon Chung
>>>
>>>> +     usleep_range(1, 2);
>>>> +     mci_writel(slot->host, RST_N, SDMMC_RST_HWACTIVE);
>>>> +     usleep_range(200, 300);
>>>> +}
>>>> +
>>>>    static void dw_mci_init_card(struct mmc_host *mmc, struct mmc_card *card)
>>>>    {
>>>>        struct dw_mci_slot *slot = mmc_priv(mmc);
>>>> @@ -1563,6 +1587,7 @@ static const struct mmc_host_ops dw_mci_ops = {
>>>>        .set_ios        = dw_mci_set_ios,
>>>>        .get_ro            = dw_mci_get_ro,
>>>>        .get_cd            = dw_mci_get_cd,
>>>> +    .hw_reset               = dw_mci_hw_reset,
>>>>        .enable_sdio_irq    = dw_mci_enable_sdio_irq,
>>>>        .execute_tuning        = dw_mci_execute_tuning,
>>>>        .card_busy        = dw_mci_card_busy,
>>>> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
>>>> index f695b58..684ee34 100644
>>>> --- a/drivers/mmc/host/dw_mmc.h
>>>> +++ b/drivers/mmc/host/dw_mmc.h
>>>> @@ -46,6 +46,7 @@
>>>>    #define SDMMC_VERID        0x06c
>>>>    #define SDMMC_HCON        0x070
>>>>    #define SDMMC_UHS_REG        0x074
>>>> +#define SDMMC_RST_N        0x078
>>>>    #define SDMMC_BMOD        0x080
>>>>    #define SDMMC_PLDMND        0x084
>>>>    #define SDMMC_DBADDR        0x088
>>>> @@ -169,6 +170,9 @@
>>>>    #define SDMMC_IDMAC_ENABLE        BIT(7)
>>>>    #define SDMMC_IDMAC_FB            BIT(1)
>>>>    #define SDMMC_IDMAC_SWRESET        BIT(0)
>>>> +/* H/W reset */
>>>> +#define SDMMC_RST_HWACTIVE        0x1
>>>> +#define SDMMC_RST_HWRESET        0x0
>>>>    /* Version ID register define */
>>>>    #define SDMMC_GET_VERID(x)        ((x) & 0xFFFF)
>>>>    /* Card read threshold */
>>>>
>>>
>>>
>>>
>>>
>>
>>
>
>
>
>


-- 
Best Regards
Shawn Lin