[PATCH v2 1/3] mmc: dw_mmc: remove the quirks flags

[PATCH v2 1/3] mmc: dw_mmc: remove the quirks flags

[Jaehoon Chung]

Remove the quirks flag. (DW_MCI_QUIRK_BROKEN_DTO)
For removing this, enabled the dto_timer by defaults.
It doesn't see any I/O performance degression.
In future, dwmmc controller should not use the quirks flag.

Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com>
---
Changelog V2:
	- Added Shawn's Reviewed-by tag

 drivers/mmc/host/dw_mmc-rockchip.c |  3 ---
 drivers/mmc/host/dw_mmc.c          | 16 +++++-----------
 include/linux/mmc/dw_mmc.h         |  9 ---------
 3 files changed, 5 insertions(+), 23 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc-rockchip.c b/drivers/mmc/host/dw_mmc-rockchip.c
index 358b0dc..d3cf1f1 100644
--- a/drivers/mmc/host/dw_mmc-rockchip.c
+++ b/drivers/mmc/host/dw_mmc-rockchip.c
@@ -285,9 +285,6 @@ static int dw_mci_rockchip_init(struct dw_mci *host)
 	/* It is slot 8 on Rockchip SoCs */
 	host->sdio_id0 = 8;
 
-	/* It needs this quirk on all Rockchip SoCs */
-	host->pdata->quirks |= DW_MCI_QUIRK_BROKEN_DTO;
-
 	if (of_device_is_compatible(host->dev->of_node,
 				    "rockchip,rk3288-dw-mshc"))
 		host->bus_hz /= RK3288_CLKGEN_DIV;
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 87f8529..5cf143b 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1802,8 +1802,7 @@ static void dw_mci_tasklet_func(unsigned long priv)
 				 * If all data-related interrupts don't come
 				 * within the given time in reading data state.
 				 */
-				if ((host->quirks & DW_MCI_QUIRK_BROKEN_DTO) &&
-				    (host->dir_status == DW_MCI_RECV_STATUS))
+				if (host->dir_status == DW_MCI_RECV_STATUS)
 					dw_mci_set_drto(host);
 				break;
 			}
@@ -1845,8 +1844,7 @@ static void dw_mci_tasklet_func(unsigned long priv)
 				 * interrupt doesn't come within the given time.
 				 * in reading data state.
 				 */
-				if ((host->quirks & DW_MCI_QUIRK_BROKEN_DTO) &&
-				    (host->dir_status == DW_MCI_RECV_STATUS))
+				if (host->dir_status == DW_MCI_RECV_STATUS)
 					dw_mci_set_drto(host);
 				break;
 			}
@@ -2412,8 +2410,7 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
 		}
 
 		if (pending & SDMMC_INT_DATA_OVER) {
-			if (host->quirks & DW_MCI_QUIRK_BROKEN_DTO)
-				del_timer(&host->dto_timer);
+			del_timer(&host->dto_timer);
 
 			mci_writel(host, RINTSTS, SDMMC_INT_DATA_OVER);
 			if (!host->data_status)
@@ -3004,11 +3001,8 @@ int dw_mci_probe(struct dw_mci *host)
 	setup_timer(&host->cmd11_timer,
 		    dw_mci_cmd11_timer, (unsigned long)host);
 
-	host->quirks = host->pdata->quirks;
-
-	if (host->quirks & DW_MCI_QUIRK_BROKEN_DTO)
-		setup_timer(&host->dto_timer,
-			    dw_mci_dto_timer, (unsigned long)host);
+	setup_timer(&host->dto_timer,
+		    dw_mci_dto_timer, (unsigned long)host);
 
 	spin_lock_init(&host->lock);
 	spin_lock_init(&host->irq_lock);
diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
index f7ed271..83b0edfc 100644
--- a/include/linux/mmc/dw_mmc.h
+++ b/include/linux/mmc/dw_mmc.h
@@ -112,7 +112,6 @@ struct dw_mci_dma_slave {
  * @part_buf: Simple buffer for partial fifo reads/writes.
  * @push_data: Pointer to FIFO push function.
  * @pull_data: Pointer to FIFO pull function.
- * @quirks: Set of quirks that apply to specific versions of the IP.
  * @vqmmc_enabled: Status of vqmmc, should be true or false.
  * @irq_flags: The flags to be passed to request_irq.
  * @irq: The irq value to be passed to request_irq.
@@ -218,9 +217,6 @@ struct dw_mci {
 	void (*push_data)(struct dw_mci *host, void *buf, int cnt);
 	void (*pull_data)(struct dw_mci *host, void *buf, int cnt);
 
-	/* Workaround flags */
-	u32			quirks;
-
 	bool			vqmmc_enabled;
 	unsigned long		irq_flags; /* IRQ flags */
 	int			irq;
@@ -242,17 +238,12 @@ struct dw_mci_dma_ops {
 	void (*exit)(struct dw_mci *host);
 };
 
-/* IP Quirks/flags. */
-/* Timer for broken data transfer over scheme */
-#define DW_MCI_QUIRK_BROKEN_DTO			BIT(0)
-
 struct dma_pdata;
 
 /* Board platform data */
 struct dw_mci_board {
 	u32 num_slots;
 
-	u32 quirks; /* Workaround / Quirk flags */
 	unsigned int bus_hz; /* Clock speed at the cclk_in pad */
 
 	u32 caps;	/* Capabilities */
-- 
1.9.1

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

[Jaehoon Chung]

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

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

[PATCH v2 3/3] mmc: dw_mmc: prevent to set the wrong value

[Jaehoon Chung]

If there ins no vqmmc, voltage should not be changed.
Then it nees to maintain the previous status for USH_REG register.
To use the standard spec, it should be returned error when voltage
can't switch.

Note:
Dwmmc controller will not make any exception case.(like exynos.)
If card isn't working well after appling this patch, it needs to check
the regulator side.

Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
---
Changelog V2:
	-Fix the typo

 drivers/mmc/host/dw_mmc.c | 40 ++++++++++++++++++++++------------------
 1 file changed, 22 insertions(+), 18 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index ec3f0a8..56c48fb 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1404,33 +1404,37 @@ static int dw_mci_switch_voltage(struct mmc_host *mmc, struct mmc_ios *ios)
 	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;
-	u32 uhs;
+	u32 uhs = 0;
 	u32 v18 = SDMMC_UHS_18V << slot->id;
 	int ret;
 
 	if (drv_data && drv_data->switch_voltage)
 		return drv_data->switch_voltage(mmc, ios);
 
-	/*
-	 * Program the voltage.  Note that some instances of dw_mmc may use
-	 * the UHS_REG for this.  For other instances (like exynos) the UHS_REG
-	 * does no harm but you need to set the regulator directly.  Try both.
-	 */
+	if (IS_ERR(mmc->supply.vqmmc)) {
+		dev_dbg(&mmc->class_dev,
+			"vqmmc not available.(Skip the switching voltage)\n");
+		return -EINVAL;
+	}
+
+	ret = mmc_regulator_set_vqmmc(mmc, ios);
+	if (ret) {
+		dev_err(&mmc->class_dev,
+				 "Regulator set error %d - %s V\n",
+				 ret, uhs & v18 ? "1.8" : "3.3");
+		return ret;
+	}
+
 	uhs = mci_readl(host, UHS_REG);
-	if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330)
+	switch (ios->signal_voltage) {
+	case MMC_SIGNAL_VOLTAGE_330:
 		uhs &= ~v18;
-	else
+		break;
+	case MMC_SIGNAL_VOLTAGE_180:
 		uhs |= v18;
-
-	if (!IS_ERR(mmc->supply.vqmmc)) {
-		ret = mmc_regulator_set_vqmmc(mmc, ios);
-
-		if (ret) {
-			dev_dbg(&mmc->class_dev,
-					 "Regulator set error %d - %s V\n",
-					 ret, uhs & v18 ? "1.8" : "3.3");
-			return ret;
-		}
+		break;
+	default:
+		uhs &= ~v18;
 	}
 	mci_writel(host, UHS_REG, uhs);
 
-- 
1.9.1

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

[Shawn Lin]

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>

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


-- 
Best Regards
Shawn Lin

Re: [PATCH v2 3/3] mmc: dw_mmc: prevent to set the wrong value

[Shawn Lin]

On 2016/6/21 13:35, Jaehoon Chung wrote:
> If there ins no vqmmc, voltage should not be changed.
> Then it nees to maintain the previous status for USH_REG register.
> To use the standard spec, it should be returned error when voltage
> can't switch.
>
> Note:
> Dwmmc controller will not make any exception case.(like exynos.)
> If card isn't working well after appling this patch, it needs to check
> the regulator side.
>
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
> Changelog V2:
> 	-Fix the typo
>
>  drivers/mmc/host/dw_mmc.c | 40 ++++++++++++++++++++++------------------
>  1 file changed, 22 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index ec3f0a8..56c48fb 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -1404,33 +1404,37 @@ static int dw_mci_switch_voltage(struct mmc_host *mmc, struct mmc_ios *ios)
>  	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;
> -	u32 uhs;
> +	u32 uhs = 0;
>  	u32 v18 = SDMMC_UHS_18V << slot->id;
>  	int ret;
>
>  	if (drv_data && drv_data->switch_voltage)
>  		return drv_data->switch_voltage(mmc, ios);
>
> -	/*
> -	 * Program the voltage.  Note that some instances of dw_mmc may use
> -	 * the UHS_REG for this.  For other instances (like exynos) the UHS_REG
> -	 * does no harm but you need to set the regulator directly.  Try both.
> -	 */
> +	if (IS_ERR(mmc->supply.vqmmc)) {
> +		dev_dbg(&mmc->class_dev,
> +			"vqmmc not available.(Skip the switching voltage)\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = mmc_regulator_set_vqmmc(mmc, ios);
> +	if (ret) {
> +		dev_err(&mmc->class_dev,
> +				 "Regulator set error %d - %s V\n",
> +				 ret, uhs & v18 ? "1.8" : "3.3");
> +		return ret;
> +	}
> +
>  	uhs = mci_readl(host, UHS_REG);
> -	if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330)

uhs is still zero now, so maybe we can remove this switch branch and
only need a simple checking like this:

if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_180)
	uhs |= v18;

otherwise, it looks good to me.

> +	switch (ios->signal_voltage) {
> +	case MMC_SIGNAL_VOLTAGE_330:
>  		uhs &= ~v18;
> -	else
> +		break;
> +	case MMC_SIGNAL_VOLTAGE_180:
>  		uhs |= v18;
> -
> -	if (!IS_ERR(mmc->supply.vqmmc)) {
> -		ret = mmc_regulator_set_vqmmc(mmc, ios);
> -
> -		if (ret) {
> -			dev_dbg(&mmc->class_dev,
> -					 "Regulator set error %d - %s V\n",
> -					 ret, uhs & v18 ? "1.8" : "3.3");
> -			return ret;
> -		}
> +		break;
> +	default:
> +		uhs &= ~v18;
>  	}
>  	mci_writel(host, UHS_REG, uhs);
>
>


-- 
Best Regards
Shawn Lin

Re: [PATCH v2 3/3] mmc: dw_mmc: prevent to set the wrong value

[Jaehoon Chung]

On 06/21/2016 05:13 PM, Shawn Lin wrote:
> On 2016/6/21 13:35, Jaehoon Chung wrote:
>> If there ins no vqmmc, voltage should not be changed.
>> Then it nees to maintain the previous status for USH_REG register.
>> To use the standard spec, it should be returned error when voltage
>> can't switch.
>>
>> Note:
>> Dwmmc controller will not make any exception case.(like exynos.)
>> If card isn't working well after appling this patch, it needs to check
>> the regulator side.
>>
>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>> ---
>> Changelog V2:
>>     -Fix the typo
>>
>>  drivers/mmc/host/dw_mmc.c | 40 ++++++++++++++++++++++------------------
>>  1 file changed, 22 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index ec3f0a8..56c48fb 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -1404,33 +1404,37 @@ static int dw_mci_switch_voltage(struct mmc_host *mmc, struct mmc_ios *ios)
>>      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;
>> -    u32 uhs;
>> +    u32 uhs = 0;
>>      u32 v18 = SDMMC_UHS_18V << slot->id;
>>      int ret;
>>
>>      if (drv_data && drv_data->switch_voltage)
>>          return drv_data->switch_voltage(mmc, ios);
>>
>> -    /*
>> -     * Program the voltage.  Note that some instances of dw_mmc may use
>> -     * the UHS_REG for this.  For other instances (like exynos) the UHS_REG
>> -     * does no harm but you need to set the regulator directly.  Try both.
>> -     */
>> +    if (IS_ERR(mmc->supply.vqmmc)) {
>> +        dev_dbg(&mmc->class_dev,
>> +            "vqmmc not available.(Skip the switching voltage)\n");
>> +        return -EINVAL;
>> +    }
>> +
>> +    ret = mmc_regulator_set_vqmmc(mmc, ios);
>> +    if (ret) {
>> +        dev_err(&mmc->class_dev,
>> +                 "Regulator set error %d - %s V\n",
>> +                 ret, uhs & v18 ? "1.8" : "3.3");
>> +        return ret;
>> +    }
>> +
>>      uhs = mci_readl(host, UHS_REG);
>> -    if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330)
> 
> uhs is still zero now, so maybe we can remove this switch branch and
> only need a simple checking like this:
> 
> if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_180)
>     uhs |= v18;

It's not enough to set the correct value.
So i will maintain this patch..

In future, i will add the MMC_SIGNAL_VOLTAGE_120 for USH_REG_EXT register.
At that time, switch statement should be better than "if" statement.. :)

Anyway, I applied this patch on my repository. If you have any objection, i will accept your opinion. :)

Best Regards,
Jaehoon Chung

> 
> otherwise, it looks good to me.
> 
>> +    switch (ios->signal_voltage) {
>> +    case MMC_SIGNAL_VOLTAGE_330:
>>          uhs &= ~v18;
>> -    else
>> +        break;
>> +    case MMC_SIGNAL_VOLTAGE_180:
>>          uhs |= v18;
>> -
>> -    if (!IS_ERR(mmc->supply.vqmmc)) {
>> -        ret = mmc_regulator_set_vqmmc(mmc, ios);
>> -
>> -        if (ret) {
>> -            dev_dbg(&mmc->class_dev,
>> -                     "Regulator set error %d - %s V\n",
>> -                     ret, uhs & v18 ? "1.8" : "3.3");
>> -            return ret;
>> -        }
>> +        break;
>> +    default:
>> +        uhs &= ~v18;
>>      }
>>      mci_writel(host, UHS_REG, uhs);
>>
>>
> 
> 

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

[Jaehoon Chung]

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

Re: [PATCH v2 1/3] mmc: dw_mmc: remove the quirks flags

[Jaehoon Chung]

On 06/21/2016 02:35 PM, Jaehoon Chung wrote:
> Remove the quirks flag. (DW_MCI_QUIRK_BROKEN_DTO)
> For removing this, enabled the dto_timer by defaults.
> It doesn't see any I/O performance degression.
> In future, dwmmc controller should not use the quirks flag.
> 
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com>

Applied this patch on my repository.

Best Regards,
Jaehoon Chung

> ---
> Changelog V2:
> 	- Added Shawn's Reviewed-by tag
> 
>  drivers/mmc/host/dw_mmc-rockchip.c |  3 ---
>  drivers/mmc/host/dw_mmc.c          | 16 +++++-----------
>  include/linux/mmc/dw_mmc.h         |  9 ---------
>  3 files changed, 5 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/mmc/host/dw_mmc-rockchip.c b/drivers/mmc/host/dw_mmc-rockchip.c
> index 358b0dc..d3cf1f1 100644
> --- a/drivers/mmc/host/dw_mmc-rockchip.c
> +++ b/drivers/mmc/host/dw_mmc-rockchip.c
> @@ -285,9 +285,6 @@ static int dw_mci_rockchip_init(struct dw_mci *host)
>  	/* It is slot 8 on Rockchip SoCs */
>  	host->sdio_id0 = 8;
>  
> -	/* It needs this quirk on all Rockchip SoCs */
> -	host->pdata->quirks |= DW_MCI_QUIRK_BROKEN_DTO;
> -
>  	if (of_device_is_compatible(host->dev->of_node,
>  				    "rockchip,rk3288-dw-mshc"))
>  		host->bus_hz /= RK3288_CLKGEN_DIV;
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 87f8529..5cf143b 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -1802,8 +1802,7 @@ static void dw_mci_tasklet_func(unsigned long priv)
>  				 * If all data-related interrupts don't come
>  				 * within the given time in reading data state.
>  				 */
> -				if ((host->quirks & DW_MCI_QUIRK_BROKEN_DTO) &&
> -				    (host->dir_status == DW_MCI_RECV_STATUS))
> +				if (host->dir_status == DW_MCI_RECV_STATUS)
>  					dw_mci_set_drto(host);
>  				break;
>  			}
> @@ -1845,8 +1844,7 @@ static void dw_mci_tasklet_func(unsigned long priv)
>  				 * interrupt doesn't come within the given time.
>  				 * in reading data state.
>  				 */
> -				if ((host->quirks & DW_MCI_QUIRK_BROKEN_DTO) &&
> -				    (host->dir_status == DW_MCI_RECV_STATUS))
> +				if (host->dir_status == DW_MCI_RECV_STATUS)
>  					dw_mci_set_drto(host);
>  				break;
>  			}
> @@ -2412,8 +2410,7 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
>  		}
>  
>  		if (pending & SDMMC_INT_DATA_OVER) {
> -			if (host->quirks & DW_MCI_QUIRK_BROKEN_DTO)
> -				del_timer(&host->dto_timer);
> +			del_timer(&host->dto_timer);
>  
>  			mci_writel(host, RINTSTS, SDMMC_INT_DATA_OVER);
>  			if (!host->data_status)
> @@ -3004,11 +3001,8 @@ int dw_mci_probe(struct dw_mci *host)
>  	setup_timer(&host->cmd11_timer,
>  		    dw_mci_cmd11_timer, (unsigned long)host);
>  
> -	host->quirks = host->pdata->quirks;
> -
> -	if (host->quirks & DW_MCI_QUIRK_BROKEN_DTO)
> -		setup_timer(&host->dto_timer,
> -			    dw_mci_dto_timer, (unsigned long)host);
> +	setup_timer(&host->dto_timer,
> +		    dw_mci_dto_timer, (unsigned long)host);
>  
>  	spin_lock_init(&host->lock);
>  	spin_lock_init(&host->irq_lock);
> diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
> index f7ed271..83b0edfc 100644
> --- a/include/linux/mmc/dw_mmc.h
> +++ b/include/linux/mmc/dw_mmc.h
> @@ -112,7 +112,6 @@ struct dw_mci_dma_slave {
>   * @part_buf: Simple buffer for partial fifo reads/writes.
>   * @push_data: Pointer to FIFO push function.
>   * @pull_data: Pointer to FIFO pull function.
> - * @quirks: Set of quirks that apply to specific versions of the IP.
>   * @vqmmc_enabled: Status of vqmmc, should be true or false.
>   * @irq_flags: The flags to be passed to request_irq.
>   * @irq: The irq value to be passed to request_irq.
> @@ -218,9 +217,6 @@ struct dw_mci {
>  	void (*push_data)(struct dw_mci *host, void *buf, int cnt);
>  	void (*pull_data)(struct dw_mci *host, void *buf, int cnt);
>  
> -	/* Workaround flags */
> -	u32			quirks;
> -
>  	bool			vqmmc_enabled;
>  	unsigned long		irq_flags; /* IRQ flags */
>  	int			irq;
> @@ -242,17 +238,12 @@ struct dw_mci_dma_ops {
>  	void (*exit)(struct dw_mci *host);
>  };
>  
> -/* IP Quirks/flags. */
> -/* Timer for broken data transfer over scheme */
> -#define DW_MCI_QUIRK_BROKEN_DTO			BIT(0)
> -
>  struct dma_pdata;
>  
>  /* Board platform data */
>  struct dw_mci_board {
>  	u32 num_slots;
>  
> -	u32 quirks; /* Workaround / Quirk flags */
>  	unsigned int bus_hz; /* Clock speed at the cclk_in pad */
>  
>  	u32 caps;	/* Capabilities */
>