All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Add hw reset support for DesignWare MMC controller.
@ 2015-10-22  6:19 ` Shawn Lin
  0 siblings, 0 replies; 8+ messages in thread
From: Shawn Lin @ 2015-10-22  6:19 UTC (permalink / raw)
  To: Jaehoon Chung; +Cc: Ulf Hansson, linux-mmc, linux-kernel, Shawn Lin


MMC core stack and blk layer can do some recovery if mmc
device runs into broken state for any reasons. So we implement it
for DesignWare MMC host controller.

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

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.

This patchset do following things:
- add hw reset for dw_mmc
- implement specific hw_reset extention



Shawn Lin (2):
  mmc: dw_mmc: add hw_reset support
  mmc: dw_mmc: add hw_reset extension hook

 drivers/mmc/host/dw_mmc.c | 33 +++++++++++++++++++++++++++++++++
 drivers/mmc/host/dw_mmc.h |  6 ++++++
 2 files changed, 39 insertions(+)

-- 
2.3.7



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 0/2] Add hw reset support for DesignWare MMC controller.
@ 2015-10-22  6:19 ` Shawn Lin
  0 siblings, 0 replies; 8+ messages in thread
From: Shawn Lin @ 2015-10-22  6:19 UTC (permalink / raw)
  To: Jaehoon Chung; +Cc: Ulf Hansson, linux-mmc, linux-kernel, Shawn Lin


MMC core stack and blk layer can do some recovery if mmc
device runs into broken state for any reasons. So we implement it
for DesignWare MMC host controller.

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.

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.

This patchset do following things:
- add hw reset for dw_mmc
- implement specific hw_reset extention



Shawn Lin (2):
  mmc: dw_mmc: add hw_reset support
  mmc: dw_mmc: add hw_reset extension hook

 drivers/mmc/host/dw_mmc.c | 33 +++++++++++++++++++++++++++++++++
 drivers/mmc/host/dw_mmc.h |  6 ++++++
 2 files changed, 39 insertions(+)

-- 
2.3.7



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/2] mmc: dw_mmc: add hw_reset support
  2015-10-22  6:19 ` Shawn Lin
  (?)
@ 2015-10-22  6:19 ` Shawn Lin
  2015-10-23 12:07   ` Jaehoon Chung
  -1 siblings, 1 reply; 8+ messages in thread
From: Shawn Lin @ 2015-10-22  6:19 UTC (permalink / raw)
  To: Jaehoon Chung; +Cc: Ulf Hansson, linux-mmc, linux-kernel, Shawn Lin

This patch implement hw_reset function for DesignWare
MMC controller. By adding this feature, mmc blk can
do some basic recovery if emmc device cannot work any
more for unknown reasons.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>

---

 drivers/mmc/host/dw_mmc.c | 29 +++++++++++++++++++++++++++++
 drivers/mmc/host/dw_mmc.h |  4 ++++
 2 files changed, 33 insertions(+)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 6e600e8..8a0f2995 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1478,6 +1478,34 @@ 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) ||
+	    (!dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET)))
+		return;
+
+	/* CARD_RESET
+	 * According to eMMC spec
+	 * tRstW >= 1us:   RST_n pulse width
+	 * tRSCA >= 200us: RST_n to Command time
+	 * tRSTH >= 1us:   RST_n high period
+	 * Note: add some margin to make rst timing not too
+	 * "spec" for some bad qulity eMMC devices.
+	 */
+	 mci_writel(slot->host, RST_N, SDMMC_RST_HWRESET);
+	 wmb(); /* drain writebuffer */
+	 usleep_range(5, 10);
+	 mci_writel(slot->host, RST_N, SDMMC_RST_HWACTIVE);
+	 wmb(); /* drain writebuffer */
+	 usleep_range(500, 1000);
+}
+
 static void dw_mci_init_card(struct mmc_host *mmc, struct mmc_card *card)
 {
 	struct dw_mci_slot *slot = mmc_priv(mmc);
@@ -1564,6 +1592,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 f2a88d4..9df18c1 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 */
-- 
2.3.7



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/2] mmc: dw_mmc: add hw_reset extension hook
  2015-10-22  6:19 ` Shawn Lin
  (?)
  (?)
@ 2015-10-22  6:19 ` Shawn Lin
  2015-10-22  9:48   ` Ulf Hansson
  -1 siblings, 1 reply; 8+ messages in thread
From: Shawn Lin @ 2015-10-22  6:19 UTC (permalink / raw)
  To: Jaehoon Chung; +Cc: Ulf Hansson, linux-mmc, linux-kernel, Shawn Lin

This patch add hw_reset for dw_mmc to implement hw reset
procedure. It's useful for mmc core to recover emmc devices
if emmc runs into unexpected state. Add MMC_CAP_HW_RESET
capability to dw_mmc extension driver directly if it needs hw_reset.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/mmc/host/dw_mmc.c | 4 ++++
 drivers/mmc/host/dw_mmc.h | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 8a0f2995..46b12de 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1482,6 +1482,10 @@ static void dw_mci_hw_reset(struct mmc_host *mmc)
 {
 	struct dw_mci_slot *slot = mmc_priv(mmc);
 	struct dw_mci *host = slot->host;
+	const struct dw_mci_drv_data *drv_data = host->drv_data;
+
+	if (drv_data && drv_data->hw_reset)
+		return drv_data->hw_reset(host);
 
 	if (host->use_dma == TRANS_MODE_IDMAC)
 		dw_mci_idmac_reset(host);
diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
index 9df18c1..21b9df4 100644
--- a/drivers/mmc/host/dw_mmc.h
+++ b/drivers/mmc/host/dw_mmc.h
@@ -282,6 +282,7 @@ struct dw_mci_slot {
  * @set_ios: handle bus specific extensions.
  * @parse_dt: parse implementation specific device tree properties.
  * @execute_tuning: implementation specific tuning procedure.
+ * @hw_reset: implementation specific hw reset procedure.
  *
  * Provide controller implementation specific extensions. The usage of this
  * data structure is fully optional and usage of each member in this structure
@@ -299,5 +300,6 @@ struct dw_mci_drv_data {
 						struct mmc_ios *ios);
 	int		(*switch_voltage)(struct mmc_host *mmc,
 					  struct mmc_ios *ios);
+	void		(*hw_reset)(struct dw_mci *host);
 };
 #endif /* _DW_MMC_H_ */
-- 
2.3.7



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] mmc: dw_mmc: add hw_reset extension hook
  2015-10-22  6:19 ` [PATCH 2/2] mmc: dw_mmc: add hw_reset extension hook Shawn Lin
@ 2015-10-22  9:48   ` Ulf Hansson
  2015-10-22 11:25     ` Shawn Lin
  0 siblings, 1 reply; 8+ messages in thread
From: Ulf Hansson @ 2015-10-22  9:48 UTC (permalink / raw)
  To: Shawn Lin; +Cc: Jaehoon Chung, linux-mmc, linux-kernel

On 22 October 2015 at 08:19, Shawn Lin <shawn.lin@rock-chips.com> wrote:
> This patch add hw_reset for dw_mmc to implement hw reset
> procedure. It's useful for mmc core to recover emmc devices
> if emmc runs into unexpected state. Add MMC_CAP_HW_RESET
> capability to dw_mmc extension driver directly if it needs hw_reset.
>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> ---
>
>  drivers/mmc/host/dw_mmc.c | 4 ++++
>  drivers/mmc/host/dw_mmc.h | 2 ++
>  2 files changed, 6 insertions(+)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 8a0f2995..46b12de 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -1482,6 +1482,10 @@ static void dw_mci_hw_reset(struct mmc_host *mmc)
>  {
>         struct dw_mci_slot *slot = mmc_priv(mmc);
>         struct dw_mci *host = slot->host;
> +       const struct dw_mci_drv_data *drv_data = host->drv_data;
> +
> +       if (drv_data && drv_data->hw_reset)
> +               return drv_data->hw_reset(host);

dw_mmc is starting to walk the same bad path as sdhci has done.

Please, try to avoid adding non-needed callbacks and start to turn
dw_mmc into a library instead.

Kind regards
Uffe

>
>         if (host->use_dma == TRANS_MODE_IDMAC)
>                 dw_mci_idmac_reset(host);
> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
> index 9df18c1..21b9df4 100644
> --- a/drivers/mmc/host/dw_mmc.h
> +++ b/drivers/mmc/host/dw_mmc.h
> @@ -282,6 +282,7 @@ struct dw_mci_slot {
>   * @set_ios: handle bus specific extensions.
>   * @parse_dt: parse implementation specific device tree properties.
>   * @execute_tuning: implementation specific tuning procedure.
> + * @hw_reset: implementation specific hw reset procedure.
>   *
>   * Provide controller implementation specific extensions. The usage of this
>   * data structure is fully optional and usage of each member in this structure
> @@ -299,5 +300,6 @@ struct dw_mci_drv_data {
>                                                 struct mmc_ios *ios);
>         int             (*switch_voltage)(struct mmc_host *mmc,
>                                           struct mmc_ios *ios);
> +       void            (*hw_reset)(struct dw_mci *host);
>  };
>  #endif /* _DW_MMC_H_ */
> --
> 2.3.7
>
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] mmc: dw_mmc: add hw_reset extension hook
  2015-10-22  9:48   ` Ulf Hansson
@ 2015-10-22 11:25     ` Shawn Lin
  0 siblings, 0 replies; 8+ messages in thread
From: Shawn Lin @ 2015-10-22 11:25 UTC (permalink / raw)
  To: Ulf Hansson; +Cc: shawn.lin, Jaehoon Chung, linux-mmc, linux-kernel

On 2015/10/22 17:48, Ulf Hansson wrote:
> On 22 October 2015 at 08:19, Shawn Lin <shawn.lin@rock-chips.com> wrote:
>> This patch add hw_reset for dw_mmc to implement hw reset
>> procedure. It's useful for mmc core to recover emmc devices
>> if emmc runs into unexpected state. Add MMC_CAP_HW_RESET
>> capability to dw_mmc extension driver directly if it needs hw_reset.
>>
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>> ---
>>
>>   drivers/mmc/host/dw_mmc.c | 4 ++++
>>   drivers/mmc/host/dw_mmc.h | 2 ++
>>   2 files changed, 6 insertions(+)
>>
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 8a0f2995..46b12de 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -1482,6 +1482,10 @@ static void dw_mci_hw_reset(struct mmc_host *mmc)
>>   {
>>          struct dw_mci_slot *slot = mmc_priv(mmc);
>>          struct dw_mci *host = slot->host;
>> +       const struct dw_mci_drv_data *drv_data = host->drv_data;
>> +
>> +       if (drv_data && drv_data->hw_reset)
>> +               return drv_data->hw_reset(host);
>
> dw_mmc is starting to walk the same bad path as sdhci has done.
>
> Please, try to avoid adding non-needed callbacks and start to turn
> dw_mmc into a library instead.
>

sorry for posting this, but then a I am just now reading your comments 
about overstaffed quirks and callback of sdhci. Obviously dw_mmc is a 
little easier to be turned into a lib before it going to be the 
equivalent weight of sdhci.

I'm glad to try it. :)

> Kind regards
> Uffe
>
>>
>>          if (host->use_dma == TRANS_MODE_IDMAC)
>>                  dw_mci_idmac_reset(host);
>> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
>> index 9df18c1..21b9df4 100644
>> --- a/drivers/mmc/host/dw_mmc.h
>> +++ b/drivers/mmc/host/dw_mmc.h
>> @@ -282,6 +282,7 @@ struct dw_mci_slot {
>>    * @set_ios: handle bus specific extensions.
>>    * @parse_dt: parse implementation specific device tree properties.
>>    * @execute_tuning: implementation specific tuning procedure.
>> + * @hw_reset: implementation specific hw reset procedure.
>>    *
>>    * Provide controller implementation specific extensions. The usage of this
>>    * data structure is fully optional and usage of each member in this structure
>> @@ -299,5 +300,6 @@ struct dw_mci_drv_data {
>>                                                  struct mmc_ios *ios);
>>          int             (*switch_voltage)(struct mmc_host *mmc,
>>                                            struct mmc_ios *ios);
>> +       void            (*hw_reset)(struct dw_mci *host);
>>   };
>>   #endif /* _DW_MMC_H_ */
>> --
>> 2.3.7
>>
>>
>
>
>


-- 
Best Regards
Shawn Lin


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] mmc: dw_mmc: add hw_reset support
  2015-10-22  6:19 ` [PATCH 1/2] mmc: dw_mmc: add hw_reset support Shawn Lin
@ 2015-10-23 12:07   ` Jaehoon Chung
  2015-10-26  0:47     ` Shawn Lin
  0 siblings, 1 reply; 8+ messages in thread
From: Jaehoon Chung @ 2015-10-23 12:07 UTC (permalink / raw)
  To: Shawn Lin; +Cc: Ulf Hansson, linux-mmc, linux-kernel

Hi, Shawn.

On 10/22/2015 03:19 PM, Shawn Lin wrote:
> This patch implement hw_reset function for DesignWare
> MMC controller. By adding this feature, mmc blk can
> do some basic recovery if emmc device cannot work any
> more for unknown reasons.

Are there any other issue before applied this patch?

> 
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> 
> ---
> 
>  drivers/mmc/host/dw_mmc.c | 29 +++++++++++++++++++++++++++++
>  drivers/mmc/host/dw_mmc.h |  4 ++++
>  2 files changed, 33 insertions(+)
> 
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 6e600e8..8a0f2995 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -1478,6 +1478,34 @@ 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) ||
> +	    (!dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET)))

Why does it reset twice? Can't reset together with DMA and FIFO?
dw_mci_ctrl_reset(host, SDMMC_CTRL_DMA_RESET | SDMMC_CTRL_FIFO_RESET)?

> +		return;
> +
> +	/* CARD_RESET
> +	 * According to eMMC spec
> +	 * tRstW >= 1us:   RST_n pulse width
> +	 * tRSCA >= 200us: RST_n to Command time
> +	 * tRSTH >= 1us:   RST_n high period
> +	 * Note: add some margin to make rst timing not too
> +	 * "spec" for some bad qulity eMMC devices.

some bad quality? Which devices are bad quality devices?

> +	 */
> +	 mci_writel(slot->host, RST_N, SDMMC_RST_HWRESET);
> +	 wmb(); /* drain writebuffer */
> +	 usleep_range(5, 10);
> +	 mci_writel(slot->host, RST_N, SDMMC_RST_HWACTIVE);
> +	 wmb(); /* drain writebuffer */
> +	 usleep_range(500, 1000);

Need to drain writebuffer at here?
I don't know why you choose those values..Just some margin?
And if you want to apply this, i recommend to use the shift with card number.

Best Regards,
Jaehoon Chung

> +}
> +
>  static void dw_mci_init_card(struct mmc_host *mmc, struct mmc_card *card)
>  {
>  	struct dw_mci_slot *slot = mmc_priv(mmc);
> @@ -1564,6 +1592,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 f2a88d4..9df18c1 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 */
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] mmc: dw_mmc: add hw_reset support
  2015-10-23 12:07   ` Jaehoon Chung
@ 2015-10-26  0:47     ` Shawn Lin
  0 siblings, 0 replies; 8+ messages in thread
From: Shawn Lin @ 2015-10-26  0:47 UTC (permalink / raw)
  To: Jaehoon Chung; +Cc: shawn.lin, Ulf Hansson, linux-mmc, linux-kernel

在 2015/10/23 20:07, Jaehoon Chung 写道:
> Hi, Shawn.
>
> On 10/22/2015 03:19 PM, Shawn Lin wrote:
>> This patch implement hw_reset function for DesignWare
>> MMC controller. By adding this feature, mmc blk can
>> do some basic recovery if emmc device cannot work any
>> more for unknown reasons.
>
> Are there any other issue before applied this patch?
>

yes, but don't relate to dw_mmc itself. In some
low and high temperature test, I get reports that some emmcs don't
work properly for a very very small probability. I don't know what 
happend to them, but reset them can slove the problem. So I have to 
guess that these emmcs are not so robust at least for temperature test.

>>
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>>
>> ---
>>
>>   drivers/mmc/host/dw_mmc.c | 29 +++++++++++++++++++++++++++++
>>   drivers/mmc/host/dw_mmc.h |  4 ++++
>>   2 files changed, 33 insertions(+)
>>
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 6e600e8..8a0f2995 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -1478,6 +1478,34 @@ 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) ||
>> +	    (!dw_mci_ctrl_reset(host, SDMMC_CTRL_FIFO_RESET)))
>
> Why does it reset twice? Can't reset together with DMA and FIFO?
> dw_mci_ctrl_reset(host, SDMMC_CTRL_DMA_RESET | SDMMC_CTRL_FIFO_RESET)?
>

I reset these by the order provided by the instruction of dw databook.
I can setup my experiment to re-test to see if we can merge these two reset.

Need a few days to test it, I will feedback later. :)

>> +		return;
>> +
>> +	/* CARD_RESET
>> +	 * According to eMMC spec
>> +	 * tRstW >= 1us:   RST_n pulse width
>> +	 * tRSCA >= 200us: RST_n to Command time
>> +	 * tRSTH >= 1us:   RST_n high period
>> +	 * Note: add some margin to make rst timing not too
>> +	 * "spec" for some bad qulity eMMC devices.
>
> some bad quality? Which devices are bad quality devices?

I test the diff reset time for the emmcs I mentioned above to see
the minimal requirment for them. As spec just say we need to make sure 
the tRstW>1us, but I find 2us is enough for most of emmcs but still some 
need more than 4 us. So I make it 5us at least here.

>
>> +	 */
>> +	 mci_writel(slot->host, RST_N, SDMMC_RST_HWRESET);
>> +	 wmb(); /* drain writebuffer */
>> +	 usleep_range(5, 10);
>> +	 mci_writel(slot->host, RST_N, SDMMC_RST_HWACTIVE);
>> +	 wmb(); /* drain writebuffer */
>> +	 usleep_range(500, 1000);
>
> Need to drain writebuffer at here?
> I don't know why you choose those values..Just some margin?
> And if you want to apply this, i recommend to use the shift with card number.
>

Just some margin.
okay, I will use the card number shift here.

> Best Regards,
> Jaehoon Chung
>
>> +}
>> +
>>   static void dw_mci_init_card(struct mmc_host *mmc, struct mmc_card *card)
>>   {
>>   	struct dw_mci_slot *slot = mmc_priv(mmc);
>> @@ -1564,6 +1592,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 f2a88d4..9df18c1 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


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2015-10-26  0:47 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-22  6:19 [PATCH 0/2] Add hw reset support for DesignWare MMC controller Shawn Lin
2015-10-22  6:19 ` Shawn Lin
2015-10-22  6:19 ` [PATCH 1/2] mmc: dw_mmc: add hw_reset support Shawn Lin
2015-10-23 12:07   ` Jaehoon Chung
2015-10-26  0:47     ` Shawn Lin
2015-10-22  6:19 ` [PATCH 2/2] mmc: dw_mmc: add hw_reset extension hook Shawn Lin
2015-10-22  9:48   ` Ulf Hansson
2015-10-22 11:25     ` Shawn Lin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.