[PATCH 0/2] mmc: dw_mmc: Make the use of the hold reg generic

[PATCH 0/2] mmc: dw_mmc: Make the use of the hold reg generic

[dinguyen]

From: Dinh Nguyen <dinguyen@altera.com>

Hi,

This patch series makes the setting of the SDMMC_CMD_USE_HOLD_REG bit generic
for all platforms that requires it. According the Synopsys spec on the dw_mmc,
setting the SDMMC_CMD_USE_HOLD_REG should be done for all speeds except for the
following higher speed modes: SDR104, SDR50, DDR50. I am also include MMC_HS200
speed as not needing the SDMMC_CMD_USE_HOLD_REG bit set as well.

Currently, Rockchip and SOCFPGA's variant of the dw_mmc requires that the
SDMMC_CMD_USE_HOLD_REG be set. For SOCFPGA, the dw_mmc is operating at 
MMC_TIMING_SD_HS mode. I don't know Rockchip's variant is operating at.

Thanks,

Dinh Nguyen (2):
  mmc: dw_mmc: Enable the hold reg for certain speed modes
  mmc: dw_mmc-pltm: Remove Rockchip's custom dw_mmc driver structure

 drivers/mmc/host/dw_mmc-pltfm.c |   12 +-----------
 drivers/mmc/host/dw_mmc.c       |   14 ++++++++++++++
 include/linux/mmc/dw_mmc.h      |    1 +
 3 files changed, 16 insertions(+), 11 deletions(-)

-- 
1.7.9.5

[PATCH 1/2] mmc: dw_mmc: Enable the hold reg for certain speed modes

[dinguyen]

From: Dinh Nguyen <dinguyen@altera.com>

This patch will enable the SDMMC_CMD_USE_HOLD_REG bit when the slot is
operating all timing modes, except for SDR50, DDR50, SDR104, and MMC_HS200.

According to the Synopsys databook :"To meet the relatively high Input Hold
Time requirement for SDR12, SDR25, and other MMC speed modes, you should
program bit[29]use_hold_Reg of the CMD register to 1'b1;"..."However, for
the higher speed modes of SDR104, SDR50 and DDR50, you can meet the much
smaller Input Hold Time requirement of 0.8ns by bypassing the Hold Register
(Path A in Figure 10-8, programming CMD.use_hold_reg = 1'b0) and then adding
delay elements on the output path as indicated."

This information is taking from the v2.50a of the Synopsys Designware Cores
Mobile Storage Host Databook.

Signed-off-by: Dinh Nguyen <dinguyen@altera.com>
---
 drivers/mmc/host/dw_mmc.c  |   14 ++++++++++++++
 include/linux/mmc/dw_mmc.h |    1 +
 2 files changed, 15 insertions(+)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 4bce0de..7075248 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -279,6 +279,9 @@ static u32 dw_mci_prepare_command(struct mmc_host *mmc, struct mmc_command *cmd)
 			cmdr |= SDMMC_CMD_DAT_WR;
 	}
 
+	if (slot->host->use_hold_reg)
+		cmdr |= SDMMC_CMD_USE_HOLD_REG;
+
 	if (drv_data && drv_data->prepare_command)
 		drv_data->prepare_command(slot->host, &cmdr);
 
@@ -969,6 +972,17 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	mci_writel(slot->host, UHS_REG, regs);
 	slot->host->timing = ios->timing;
 
+	switch (slot->host->timing) {
+	case MMC_TIMING_UHS_SDR50:
+	case MMC_TIMING_UHS_SDR104:
+	case MMC_TIMING_UHS_DDR50:
+	case MMC_TIMING_MMC_HS200:
+		slot->host->use_hold_reg = 0;
+		break;
+	default:
+		slot->host->use_hold_reg = 1;
+	}
+
 	/*
 	 * Use mirror of ios->clock to prevent race with mmc
 	 * core ios update when finding the minimum.
diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
index 6ce7d2c..b9bf3b8 100644
--- a/include/linux/mmc/dw_mmc.h
+++ b/include/linux/mmc/dw_mmc.h
@@ -191,6 +191,7 @@ struct dw_mci {
 	struct regulator	*vmmc;	/* Power regulator */
 	unsigned long		irq_flags; /* IRQ flags */
 	int			irq;
+	bool			use_hold_reg;
 };
 
 /* DMA ops for Internal/External DMAC interface */
-- 
1.7.9.5

[PATCH 2/2] mmc: dw_mmc-pltm: Remove Rockchip's custom dw_mmc driver structure

[dinguyen]

From: Dinh Nguyen <dinguyen@altera.com>

Rockchip's implementation of the dw_mmc controller only requires the setting
of the SDMMC_CMD_USE_HOLD_REG on every command. With the patch to set the
SDMMC_CMD_USE_HOLD_REG by checking the slot's speed mode, this Rockchip
custom driver structure is no longer necessary.

Signed-off-by: Dinh Nguyen <dinguyen@altera.com>
---
 drivers/mmc/host/dw_mmc-pltfm.c |   12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc-pltfm.c b/drivers/mmc/host/dw_mmc-pltfm.c
index 5c49656..8f15d05 100644
--- a/drivers/mmc/host/dw_mmc-pltfm.c
+++ b/drivers/mmc/host/dw_mmc-pltfm.c
@@ -25,15 +25,6 @@
 #include "dw_mmc.h"
 #include "dw_mmc-pltfm.h"
 
-static void dw_mci_rockchip_prepare_command(struct dw_mci *host, u32 *cmdr)
-{
-	*cmdr |= SDMMC_CMD_USE_HOLD_REG;
-}
-
-static const struct dw_mci_drv_data rockchip_drv_data = {
-	.prepare_command	= dw_mci_rockchip_prepare_command,
-};
-
 int dw_mci_pltfm_register(struct platform_device *pdev,
 			  const struct dw_mci_drv_data *drv_data)
 {
@@ -90,8 +81,7 @@ EXPORT_SYMBOL_GPL(dw_mci_pltfm_pmops);
 
 static const struct of_device_id dw_mci_pltfm_match[] = {
 	{ .compatible = "snps,dw-mshc", },
-	{ .compatible = "rockchip,rk2928-dw-mshc",
-		.data = &rockchip_drv_data },
+	{ .compatible = "rockchip,rk2928-dw-mshc", },
 	{},
 };
 MODULE_DEVICE_TABLE(of, dw_mci_pltfm_match);
-- 
1.7.9.5

Re: [PATCH 2/2] mmc: dw_mmc-pltm: Remove Rockchip's custom dw_mmc driver structure

[Heiko Stübner]

Am Freitag, 6. Dezember 2013, 17:10:23 schrieb dinguyen@altera.com:
> From: Dinh Nguyen <dinguyen@altera.com>
> 
> Rockchip's implementation of the dw_mmc controller only requires the
> setting of the SDMMC_CMD_USE_HOLD_REG on every command. With the patch to
> set the SDMMC_CMD_USE_HOLD_REG by checking the slot's speed mode, this
> Rockchip custom driver structure is no longer necessary.
> 
> Signed-off-by: Dinh Nguyen <dinguyen@altera.com>

hmm, testing will need a bit more time, as it seems one of the changes merged 
during the 3.13 merge window broke the dw-mmc on the rockchip.

So I'll need to bisect this first.


Heiko

> ---
>  drivers/mmc/host/dw_mmc-pltfm.c |   12 +-----------
>  1 file changed, 1 insertion(+), 11 deletions(-)
> 
> diff --git a/drivers/mmc/host/dw_mmc-pltfm.c
> b/drivers/mmc/host/dw_mmc-pltfm.c index 5c49656..8f15d05 100644
> --- a/drivers/mmc/host/dw_mmc-pltfm.c
> +++ b/drivers/mmc/host/dw_mmc-pltfm.c
> @@ -25,15 +25,6 @@
>  #include "dw_mmc.h"
>  #include "dw_mmc-pltfm.h"
> 
> -static void dw_mci_rockchip_prepare_command(struct dw_mci *host, u32
> *cmdr) -{
> -	*cmdr |= SDMMC_CMD_USE_HOLD_REG;
> -}
> -
> -static const struct dw_mci_drv_data rockchip_drv_data = {
> -	.prepare_command	= dw_mci_rockchip_prepare_command,
> -};
> -
>  int dw_mci_pltfm_register(struct platform_device *pdev,
>  			  const struct dw_mci_drv_data *drv_data)
>  {
> @@ -90,8 +81,7 @@ EXPORT_SYMBOL_GPL(dw_mci_pltfm_pmops);
> 
>  static const struct of_device_id dw_mci_pltfm_match[] = {
>  	{ .compatible = "snps,dw-mshc", },
> -	{ .compatible = "rockchip,rk2928-dw-mshc",
> -		.data = &rockchip_drv_data },
> +	{ .compatible = "rockchip,rk2928-dw-mshc", },
>  	{},
>  };
>  MODULE_DEVICE_TABLE(of, dw_mci_pltfm_match);

Re: [PATCH 2/2] mmc: dw_mmc-pltm: Remove Rockchip's custom dw_mmc driver structure

[Dinh Nguyen]

On 12/6/13 5:14 PM, Heiko Stübner wrote:
> Am Freitag, 6. Dezember 2013, 17:10:23 schrieb dinguyen@altera.com:
>> From: Dinh Nguyen <dinguyen@altera.com>
>>
>> Rockchip's implementation of the dw_mmc controller only requires the
>> setting of the SDMMC_CMD_USE_HOLD_REG on every command. With the patch to
>> set the SDMMC_CMD_USE_HOLD_REG by checking the slot's speed mode, this
>> Rockchip custom driver structure is no longer necessary.
>>
>> Signed-off-by: Dinh Nguyen <dinguyen@altera.com>
> hmm, testing will need a bit more time, as it seems one of the changes merged 
> during the 3.13 merge window broke the dw-mmc on the rockchip.
Thanks Heiko. Don't bother with v1, I overlooked another aspect of using
the hold
reg. Let me send out a v2 for you to test.

Dinh
>
> So I'll need to bisect this first.
>
>
> Heiko
>
>> ---
>>  drivers/mmc/host/dw_mmc-pltfm.c |   12 +-----------
>>  1 file changed, 1 insertion(+), 11 deletions(-)
>>
>> diff --git a/drivers/mmc/host/dw_mmc-pltfm.c
>> b/drivers/mmc/host/dw_mmc-pltfm.c index 5c49656..8f15d05 100644
>> --- a/drivers/mmc/host/dw_mmc-pltfm.c
>> +++ b/drivers/mmc/host/dw_mmc-pltfm.c
>> @@ -25,15 +25,6 @@
>>  #include "dw_mmc.h"
>>  #include "dw_mmc-pltfm.h"
>>
>> -static void dw_mci_rockchip_prepare_command(struct dw_mci *host, u32
>> *cmdr) -{
>> -	*cmdr |= SDMMC_CMD_USE_HOLD_REG;
>> -}
>> -
>> -static const struct dw_mci_drv_data rockchip_drv_data = {
>> -	.prepare_command	= dw_mci_rockchip_prepare_command,
>> -};
>> -
>>  int dw_mci_pltfm_register(struct platform_device *pdev,
>>  			  const struct dw_mci_drv_data *drv_data)
>>  {
>> @@ -90,8 +81,7 @@ EXPORT_SYMBOL_GPL(dw_mci_pltfm_pmops);
>>
>>  static const struct of_device_id dw_mci_pltfm_match[] = {
>>  	{ .compatible = "snps,dw-mshc", },
>> -	{ .compatible = "rockchip,rk2928-dw-mshc",
>> -		.data = &rockchip_drv_data },
>> +	{ .compatible = "rockchip,rk2928-dw-mshc", },
>>  	{},
>>  };
>>  MODULE_DEVICE_TABLE(of, dw_mci_pltfm_match);

Re: [PATCH 2/2] mmc: dw_mmc-pltm: Remove Rockchip's custom dw_mmc driver structure

[Heiko Stübner]

Am Samstag, 7. Dezember 2013, 00:18:07 schrieb Dinh Nguyen:
> On 12/6/13 5:14 PM, Heiko Stübner wrote:
> > Am Freitag, 6. Dezember 2013, 17:10:23 schrieb dinguyen@altera.com:
> >> From: Dinh Nguyen <dinguyen@altera.com>
> >> 
> >> Rockchip's implementation of the dw_mmc controller only requires the
> >> setting of the SDMMC_CMD_USE_HOLD_REG on every command. With the patch
> >> to set the SDMMC_CMD_USE_HOLD_REG by checking the slot's speed mode,
> >> this Rockchip custom driver structure is no longer necessary.
> >> 
> >> Signed-off-by: Dinh Nguyen <dinguyen@altera.com>
> > 
> > hmm, testing will need a bit more time, as it seems one of the changes
> > merged during the 3.13 merge window broke the dw-mmc on the rockchip.
> 
> Thanks Heiko. Don't bother with v1, I overlooked another aspect of using
> the hold
> reg. Let me send out a v2 for you to test.

Don't worry ... as I've said, first I need to find the current issue :-)


Heiko

Re: [PATCH 2/2] mmc: dw_mmc-pltm: Remove Rockchip's custom dw_mmc driver structure

[Dinh Nguyen]

On 12/6/13 5:22 PM, Heiko Stübner wrote:
> Am Samstag, 7. Dezember 2013, 00:18:07 schrieb Dinh Nguyen:
>> On 12/6/13 5:14 PM, Heiko Stübner wrote:
>>> Am Freitag, 6. Dezember 2013, 17:10:23 schrieb dinguyen@altera.com:@@ -42,7 +42,7 @@
>>>  /* Common flag combinations */
>>>  #define DW_MCI_DATA_ERROR_FLAGS        (SDMMC_INT_DRTO | SDMMC_INT_DCRC | \
>>>                                  SDMMC_INT_HTO | SDMMC_INT_SBE  | \
>>> -                                SDMMC_INT_EBE)
>>> +                                SDMMC_INT_EBE | SDMMC_INT_FRUN)
>>>  #define DW_MCI_CMD_ERROR_FLAGS (SDMMC_INT_RTO | SDMMC_INT_RCRC | \
>>>> From: Dinh Nguyen <dinguyen@altera.com>
>>>>
>>>> Rockchip's implementation of the dw_mmc controller only requires the
>>>> setting of the SDMMC_CMD_USE_HOLD_REG on every command. With the patch
>>>> to set the SDMMC_CMD_USE_HOLD_REG by checking the slot's speed mode,
>>>> this Rockchip custom driver structure is no longer necessary.
>>>>
>>>> Signed-off-by: Dinh Nguyen <dinguyen@altera.com>
>>> hmm, testing will need a bit more time, as it seems one of the changes
>>> merged during the 3.13 merge window broke the dw-mmc on the rockchip.
>> Thanks Heiko. Don't bother with v1, I overlooked another aspect of using
>> the hold
>> reg. Let me send out a v2 for you to test.
> Don't worry ... as I've said, first I need to find the current issue :-)
What behavior are you seeing? I had to do this apply this for 3.13 when the
SD driver is using IDMAC:

--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -42,7 +42,7 @@
 /* Common flag combinations */
 #define DW_MCI_DATA_ERROR_FLAGS        (SDMMC_INT_DRTO | SDMMC_INT_DCRC | \
                                 SDMMC_INT_HTO | SDMMC_INT_SBE  | \
-                                SDMMC_INT_EBE)
+                                SDMMC_INT_EBE | SDMMC_INT_FRUN)
 #define DW_MCI_CMD_ERROR_FLAGS (SDMMC_INT_RTO | SDMMC_INT_RCRC | \
                                 SDMMC_INT_RESP_ERR)
 #define DW_MCI_ERROR_FLAGS     (DW_MCI_DATA_ERROR_FLAGS | \
>
> Heiko
>

Re: [PATCH 2/2] mmc: dw_mmc-pltm: Remove Rockchip's custom dw_mmc driver structure

[Heiko Stübner]

Am Samstag, 7. Dezember 2013, 00:27:01 schrieb Dinh Nguyen:
> On 12/6/13 5:22 PM, Heiko Stübner wrote:
> > Am Samstag, 7. Dezember 2013, 00:18:07 schrieb Dinh Nguyen:
> >> On 12/6/13 5:14 PM, Heiko Stübner wrote:
> >>> Am Freitag, 6. Dezember 2013, 17:10:23 schrieb dinguyen@altera.com:@@
> >>> -42,7 +42,7 @@
> >>> 
> >>>  /* Common flag combinations */
> >>>  #define DW_MCI_DATA_ERROR_FLAGS        (SDMMC_INT_DRTO |
> >>>  SDMMC_INT_DCRC | \
> >>>  
> >>>                                  SDMMC_INT_HTO | SDMMC_INT_SBE  | \
> >>> 
> >>> -                                SDMMC_INT_EBE)
> >>> +                                SDMMC_INT_EBE | SDMMC_INT_FRUN)
> >>> 
> >>>  #define DW_MCI_CMD_ERROR_FLAGS (SDMMC_INT_RTO | SDMMC_INT_RCRC | \
> >>>  
> >>>> From: Dinh Nguyen <dinguyen@altera.com>
> >>>> 
> >>>> Rockchip's implementation of the dw_mmc controller only requires the
> >>>> setting of the SDMMC_CMD_USE_HOLD_REG on every command. With the patch
> >>>> to set the SDMMC_CMD_USE_HOLD_REG by checking the slot's speed mode,
> >>>> this Rockchip custom driver structure is no longer necessary.
> >>>> 
> >>>> Signed-off-by: Dinh Nguyen <dinguyen@altera.com>
> >>> 
> >>> hmm, testing will need a bit more time, as it seems one of the changes
> >>> merged during the 3.13 merge window broke the dw-mmc on the rockchip.
> >> 
> >> Thanks Heiko. Don't bother with v1, I overlooked another aspect of using
> >> the hold
> >> reg. Let me send out a v2 for you to test.
> > 
> > Don't worry ... as I've said, first I need to find the current issue :-)
> 
> What behavior are you seeing?

forget that I said anything :-)

It seems I messed up when bringing my rockchip development tree forward to the 
current 3.13-rc. It's working now with and without the change you sent.

So now I can wait for your v2 and test it then.

bedtime now,


Heiko

Re: [PATCH 0/2] mmc: dw_mmc: Make the use of the hold reg generic

[Arnd Bergmann]

On Friday 06 December 2013, dinguyen@altera.com wrote:
> From: Dinh Nguyen <dinguyen@altera.com>
> 
> Hi,
> 
> This patch series makes the setting of the SDMMC_CMD_USE_HOLD_REG bit generic
> for all platforms that requires it. According the Synopsys spec on the dw_mmc,
> setting the SDMMC_CMD_USE_HOLD_REG should be done for all speeds except for the
> following higher speed modes: SDR104, SDR50, DDR50. I am also include MMC_HS200
> speed as not needing the SDMMC_CMD_USE_HOLD_REG bit set as well.
> 
> Currently, Rockchip and SOCFPGA's variant of the dw_mmc requires that the
> SDMMC_CMD_USE_HOLD_REG be set. For SOCFPGA, the dw_mmc is operating at 
> MMC_TIMING_SD_HS mode. I don't know Rockchip's variant is operating at.
> 


Very nice, thanks for implementing this!

Acked-by: Arnd Bergmann <arnd@arndb.de>

Obviously this needs to be tested on at least the rockchips variant, but
ideally on most others too.

	Arnd

Re: [PATCH 0/2] mmc: dw_mmc: Make the use of the hold reg generic

[Dinh Nguyen]

On 12/6/13 11:36 AM, Arnd Bergmann wrote:
> On Friday 06 December 2013, dinguyen@altera.com wrote:
>> From: Dinh Nguyen <dinguyen@altera.com>
>>
>> Hi,
>>
>> This patch series makes the setting of the SDMMC_CMD_USE_HOLD_REG bit generic
>> for all platforms that requires it. According the Synopsys spec on the dw_mmc,
>> setting the SDMMC_CMD_USE_HOLD_REG should be done for all speeds except for the
>> following higher speed modes: SDR104, SDR50, DDR50. I am also include MMC_HS200
>> speed as not needing the SDMMC_CMD_USE_HOLD_REG bit set as well.
>>
>> Currently, Rockchip and SOCFPGA's variant of the dw_mmc requires that the
>> SDMMC_CMD_USE_HOLD_REG be set. For SOCFPGA, the dw_mmc is operating at 
>> MMC_TIMING_SD_HS mode. I don't know Rockchip's variant is operating at.
>>
>
> Very nice, thanks for implementing this!
>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
>
> Obviously this needs to be tested on at least the rockchips variant, but
> ideally on most others too.
Thanks Arnd! But I think I will have to send out a v2 shortly. I missed
a subtle
line in the databook that the hold_reg should be cleared if there is no
clock phase
selected. I think the patch as it stands will break dw_mmc-exynos.

Dinh
>
> 	Arnd