* [PATCH v2 2/3] mmc: dw_mmc: add the card write threshold for HS400 mode
2016-06-21 5:35 ` [PATCH v2 1/3] mmc: dw_mmc: remove the quirks flags Jaehoon Chung
@ 2016-06-21 5:35 ` Jaehoon Chung
2016-06-21 8:05 ` Shawn Lin
2016-06-21 5:35 ` [PATCH v2 3/3] mmc: dw_mmc: prevent to set the wrong value Jaehoon Chung
2016-06-22 4:16 ` [PATCH v2 1/3] mmc: dw_mmc: remove the quirks flags Jaehoon Chung
2 siblings, 1 reply; 8+ messages in thread
From: Jaehoon Chung @ 2016-06-21 5:35 UTC (permalink / raw)
To: linux-mmc; +Cc: ulf.hansson, shawn.lin, 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
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2 2/3] mmc: dw_mmc: add the card write threshold for HS400 mode
2016-06-21 5:35 ` [PATCH v2 2/3] mmc: dw_mmc: add the card write threshold for HS400 mode Jaehoon Chung
@ 2016-06-21 8:05 ` Shawn Lin
2016-06-22 4:16 ` Jaehoon Chung
0 siblings, 1 reply; 8+ messages in thread
From: Shawn Lin @ 2016-06-21 8:05 UTC (permalink / raw)
To: Jaehoon Chung, linux-mmc; +Cc: shawn.lin, ulf.hansson
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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2 2/3] mmc: dw_mmc: add the card write threshold for HS400 mode
2016-06-21 8:05 ` Shawn Lin
@ 2016-06-22 4:16 ` Jaehoon Chung
0 siblings, 0 replies; 8+ messages in thread
From: Jaehoon Chung @ 2016-06-22 4:16 UTC (permalink / raw)
To: Shawn Lin, linux-mmc; +Cc: ulf.hansson
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 \
>>
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 3/3] mmc: dw_mmc: prevent to set the wrong value
2016-06-21 5:35 ` [PATCH v2 1/3] mmc: dw_mmc: remove the quirks flags Jaehoon Chung
2016-06-21 5:35 ` [PATCH v2 2/3] mmc: dw_mmc: add the card write threshold for HS400 mode Jaehoon Chung
@ 2016-06-21 5:35 ` Jaehoon Chung
2016-06-21 8:13 ` Shawn Lin
2016-06-22 4:16 ` [PATCH v2 1/3] mmc: dw_mmc: remove the quirks flags Jaehoon Chung
2 siblings, 1 reply; 8+ messages in thread
From: Jaehoon Chung @ 2016-06-21 5:35 UTC (permalink / raw)
To: linux-mmc; +Cc: ulf.hansson, shawn.lin, 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
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2 3/3] mmc: dw_mmc: prevent to set the wrong value
2016-06-21 5:35 ` [PATCH v2 3/3] mmc: dw_mmc: prevent to set the wrong value Jaehoon Chung
@ 2016-06-21 8:13 ` Shawn Lin
2016-06-22 4:15 ` Jaehoon Chung
0 siblings, 1 reply; 8+ messages in thread
From: Shawn Lin @ 2016-06-21 8:13 UTC (permalink / raw)
To: Jaehoon Chung, linux-mmc; +Cc: shawn.lin, ulf.hansson
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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2 3/3] mmc: dw_mmc: prevent to set the wrong value
2016-06-21 8:13 ` Shawn Lin
@ 2016-06-22 4:15 ` Jaehoon Chung
0 siblings, 0 replies; 8+ messages in thread
From: Jaehoon Chung @ 2016-06-22 4:15 UTC (permalink / raw)
To: Shawn Lin, linux-mmc; +Cc: ulf.hansson
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);
>>
>>
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/3] mmc: dw_mmc: remove the quirks flags
2016-06-21 5:35 ` [PATCH v2 1/3] mmc: dw_mmc: remove the quirks flags Jaehoon Chung
2016-06-21 5:35 ` [PATCH v2 2/3] mmc: dw_mmc: add the card write threshold for HS400 mode Jaehoon Chung
2016-06-21 5:35 ` [PATCH v2 3/3] mmc: dw_mmc: prevent to set the wrong value Jaehoon Chung
@ 2016-06-22 4:16 ` Jaehoon Chung
2 siblings, 0 replies; 8+ messages in thread
From: Jaehoon Chung @ 2016-06-22 4:16 UTC (permalink / raw)
To: linux-mmc; +Cc: ulf.hansson, shawn.lin
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 */
>
^ permalink raw reply [flat|nested] 8+ messages in thread