Re: [PATCH v2 2/3] mmc: dw_mmc: add the card write threshold for HS400 mode

[Jaehoon Chung <jh80.chung@samsung.com>]

On 06/21/2016 05:05 PM, Shawn Lin wrote:
> On 2016/6/21 13:35, Jaehoon Chung wrote:
>> Since v2.80a, dwmmc controller introduced the card write threshold for
>> HS400 mode. So CardThrCtl can be supported during write operation, not
>> only read operation.
>> (Note: Only use the write threshold when mode is HS400.)
>>
>> To use more compatible, removed "_rd_" from function name.
>>
>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>> ---
>> Changelog V2:
>>     - Add the DW_MMC_280A for checking IP-version.
>>     - Applied the Shawn's suggestion.
>>
>>  drivers/mmc/host/dw_mmc.c | 34 +++++++++++++++++++++++-----------
>>  drivers/mmc/host/dw_mmc.h |  6 +++++-
>>  2 files changed, 28 insertions(+), 12 deletions(-)
>>
> 
> Looks good to me,
> 
> Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com>

Applied this patch on my repository.

Best Regards,
Jaehoon Chung

> 
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 5cf143b..ec3f0a8 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -899,23 +899,35 @@ done:
>>      mci_writel(host, FIFOTH, fifoth_val);
>>  }
>>
>> -static void dw_mci_ctrl_rd_thld(struct dw_mci *host, struct mmc_data *data)
>> +static void dw_mci_ctrl_thld(struct dw_mci *host, struct mmc_data *data)
>>  {
>>      unsigned int blksz = data->blksz;
>>      u32 blksz_depth, fifo_depth;
>>      u16 thld_size;
>> -
>> -    WARN_ON(!(data->flags & MMC_DATA_READ));
>> +    u8 enable;
>>
>>      /*
>>       * CDTHRCTL doesn't exist prior to 240A (in fact that register offset is
>>       * in the FIFO region, so we really shouldn't access it).
>>       */
>> -    if (host->verid < DW_MMC_240A)
>> +    if (host->verid < DW_MMC_240A ||
>> +        (host->verid < DW_MMC_280A && data->flags & MMC_DATA_WRITE))
>>          return;
>>
>> +    /*
>> +     * Card write Threshold is introduced since 2.80a
>> +     * It's used when HS400 mode is enabled.
>> +     */
>> +    if (data->flags & MMC_DATA_WRITE &&
>> +        !(host->timing != MMC_TIMING_MMC_HS400))
>> +        return;
>> +
>> +    if (data->flags & MMC_DATA_WRITE)
>> +        enable = SDMMC_CARD_WR_THR_EN;
>> +    else
>> +        enable = SDMMC_CARD_RD_THR_EN;
>> +
>>      if (host->timing != MMC_TIMING_MMC_HS200 &&
>> -        host->timing != MMC_TIMING_MMC_HS400 &&
>>          host->timing != MMC_TIMING_UHS_SDR104)
>>          goto disable;
>>
>> @@ -931,11 +943,11 @@ static void dw_mci_ctrl_rd_thld(struct dw_mci *host, struct mmc_data *data)
>>       * Currently just choose blksz.
>>       */
>>      thld_size = blksz;
>> -    mci_writel(host, CDTHRCTL, SDMMC_SET_RD_THLD(thld_size, 1));
>> +    mci_writel(host, CDTHRCTL, SDMMC_SET_THLD(thld_size, enable));
>>      return;
>>
>>  disable:
>> -    mci_writel(host, CDTHRCTL, SDMMC_SET_RD_THLD(0, 0));
>> +    mci_writel(host, CDTHRCTL, 0);
>>  }
>>
>>  static int dw_mci_submit_data_dma(struct dw_mci *host, struct mmc_data *data)
>> @@ -1006,12 +1018,12 @@ static void dw_mci_submit_data(struct dw_mci *host, struct mmc_data *data)
>>      host->sg = NULL;
>>      host->data = data;
>>
>> -    if (data->flags & MMC_DATA_READ) {
>> +    if (data->flags & MMC_DATA_READ)
>>          host->dir_status = DW_MCI_RECV_STATUS;
>> -        dw_mci_ctrl_rd_thld(host, data);
>> -    } else {
>> +    else
>>          host->dir_status = DW_MCI_SEND_STATUS;
>> -    }
>> +
>> +    dw_mci_ctrl_thld(host, data);
>>
>>      if (dw_mci_submit_data_dma(host, data)) {
>>          if (host->data->flags & MMC_DATA_READ)
>> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
>> index 1e8d838..a186d4b 100644
>> --- a/drivers/mmc/host/dw_mmc.h
>> +++ b/drivers/mmc/host/dw_mmc.h
>> @@ -15,6 +15,7 @@
>>  #define _DW_MMC_H_
>>
>>  #define DW_MMC_240A        0x240a
>> +#define DW_MMC_280A        0x280a
>>
>>  #define SDMMC_CTRL        0x000
>>  #define SDMMC_PWREN        0x004
>> @@ -175,7 +176,10 @@
>>  /* Version ID register define */
>>  #define SDMMC_GET_VERID(x)        ((x) & 0xFFFF)
>>  /* Card read threshold */
>> -#define SDMMC_SET_RD_THLD(v, x)        (((v) & 0xFFF) << 16 | (x))
>> +#define SDMMC_SET_THLD(v, x)        (((v) & 0xFFF) << 16 | (x))
>> +#define SDMMC_CARD_WR_THR_EN        BIT(2)
>> +#define SDMMC_CARD_RD_THR_EN        BIT(0)
>> +/* UHS-1 register defines */
>>  #define SDMMC_UHS_18V            BIT(0)
>>  /* All ctrl reset bits */
>>  #define SDMMC_CTRL_ALL_RESET_FLAGS \
>>
> 
>