Re: 答复: [PATCH v9 2/2] mmc: dw_mmc-k3: add sd support for hi3660
On 2017/8/9 14:56, liwei (CM) wrote:
Hi, Shawn
Thank you for your patience and carefulness, I'm sorry maybe I miss your
comments.
1.for the changelog,do you me as following:
【liwei】Major changes in v8:
- modify patch v7 name and dependency order.
Major changes in v9:
- solve review comments for Ulf Hansson.
*use mmc_regulator_set_vqmmc() instead of regulator_set_voltage().
Firstly send the patch to yourself and check it in your inbox and you
could see that there is already a '--' between your commit msg and
the files you change. So inline your changelog there.
-
2. some of this macro newly added aren't used anywhere.
【liwei】yes, PULL_DOWN BIT(1) / PULL_UP BIT(0) aren't used anywhere, I will
remove them in patch v10
3. > +#define SDMMC_UHS_REG_EXT 0x108
+#define SDMMC_ENABLE_SHIFT 0x110
【liwei】They modified at patch v4, I move common register for dwmmc controller
to dwmmc header file, and they are used as following in dw_mci_hs_set_timing()
mci_writel(host, UHS_REG_EXT, reg_value);
mci_writel(host, ENABLE_SHIFT, enable_shift);
4. Does it work for hard-coding the delay timing? All the different boards with
different cards work fine?
And tuning process is also work for SDR50 although it's optional. So how would
that work for your case?
【liwei】Yes, they work fine with different boards with different cards, we
tested it. In fact, these values are the generally appropriate values given by
our controller chip colleagues.
For SDR50 it will not come into dw_mci_hi3660_execute_tuning() and
use the given value in hs_timing_cfg[][]{} when dw_mci_hi3660_init();
5. > + for (i = 0; i < NUM_PHASES; ++i, ++smpl_phase) {
+ smpl_phase %= 32;
+
+ mci_writel(host, TMOUT, ~0);
Why you need this?
【liwei】This is what SDR104 needs to find the optimum phase point.
I meant I didn't get the point of why you set the TMOUT here.
Tips: For further wise, please don't top post and inline your answer
in the previous mail so that we could better trace down the issue
we are talking about.
Thank you again and look forward to your reply!
-邮件原件-
发件人: Shawn Lin [mailto:shawn@rock-chips.com]
发送时间: 2017年8月9日 11:48
收件人: liwei (CM); ulf.hans...@linaro.org; jh80.ch...@samsung.com;
wsa+rene...@sang-engineering.com; hkallwe...@gmail.com;
linux-...@vger.kernel.org; linux-kernel@vger.kernel.org
抄送: adrian.hun...@intel.com; shawn@rock-chips.com; guodong...@linaro.org
主题: Re: [PATCH v9 2/2] mmc: dw_mmc-k3: add sd support for hi3660
On 2017/8/9 11:25, Li Wei wrote:
Add sd card support for hi3660 soc
Signed-off-by: Li Wei
Signed-off-by: Chen Jun
I did some comment for your v6 but probably you miss them.
And it's still incorrect for your changelog.
--
Major changes in v3:
- solve review comments from Heiner Kallweit.
*use the GENMASK and FIELD_PREP macros replace the bit shift operation.
*use usleep_range() replace udelay() and mdelay().
Major changes in v4:
- solve review comments from Jaehoon Chung.
*move common register for dwmmc controller to dwmmc header file.
*modify definitions type of some register variables.
*get rid of the magic numbers.
Major changes in v5:
- further improve coding style.
Major changes in v6:
- solve review comments for Jaehoon Chung.
*modify dw_mci_hi3660_set_ios() to static.
*fix the comment style.
Major changes in v7:
- solve review comments for John Stultz.
*remove reset code in dw_mmc-k3.c,use reset in core mmc.
Major changes in v8:
- modify patch v7 name and dependency order.
Major changes in v9:
- solve review comments for Ulf Hansson.
*use mmc_regulator_set_vqmmc() instead of regulator_set_voltage().
---
should be here...
drivers/mmc/host/dw_mmc-k3.c | 301
+++
drivers/mmc/host/dw_mmc.h| 2 +
2 files changed, 303 insertions(+)
diff --git a/drivers/mmc/host/dw_mmc-k3.c
b/drivers/mmc/host/dw_mmc-k3.c index e38fb0020bb1..f6910bed55ef 100644
--- a/drivers/mmc/host/dw_mmc-k3.c
+++ b/drivers/mmc/host/dw_mmc-k3.c
@@ -8,6 +8,8 @@
* (at your option) any later version.
*/
+#include
+#include
#include
#include
#include
@@ -28,7 +30,38 @@
#define AO_SCTRL_SEL18 BIT(10)
#define AO_SCTRL_CTRL3 0x40C
+#define DWMMC_SDIO_ID 2
+
+#define SOC_SCTRL_SCPERCTRL5(0x314)
+#define SDCARD_IO_SEL18 BIT(2)
+
+#define SDCARD_RD_THRESHOLD (512)
+
+#define GENCLK_DIV (7)
+
+#define GPIO_CLK_ENABLE BIT(16)
+#define GPIO_CLK_DIV_MASK GENMASK(11, 8)
+#define GPIO_USE_SAMPLE_DLY_MASK GENMASK(13, 13)
+#define UHS_REG_EXT_SAMPLE_PHASE_MASK GENMASK(20, 16)
+#define
Re: 答复: [PATCH v9 2/2] mmc: dw_mmc-k3: add sd support for hi3660
On 2017/8/9 14:56, liwei (CM) wrote:
Hi, Shawn
Thank you for your patience and carefulness, I'm sorry maybe I miss your
comments.
1.for the changelog,do you me as following:
【liwei】Major changes in v8:
- modify patch v7 name and dependency order.
Major changes in v9:
- solve review comments for Ulf Hansson.
*use mmc_regulator_set_vqmmc() instead of regulator_set_voltage().
Firstly send the patch to yourself and check it in your inbox and you
could see that there is already a '--' between your commit msg and
the files you change. So inline your changelog there.
-
2. some of this macro newly added aren't used anywhere.
【liwei】yes, PULL_DOWN BIT(1) / PULL_UP BIT(0) aren't used anywhere, I will
remove them in patch v10
3. > +#define SDMMC_UHS_REG_EXT 0x108
+#define SDMMC_ENABLE_SHIFT 0x110
【liwei】They modified at patch v4, I move common register for dwmmc controller
to dwmmc header file, and they are used as following in dw_mci_hs_set_timing()
mci_writel(host, UHS_REG_EXT, reg_value);
mci_writel(host, ENABLE_SHIFT, enable_shift);
4. Does it work for hard-coding the delay timing? All the different boards with
different cards work fine?
And tuning process is also work for SDR50 although it's optional. So how would
that work for your case?
【liwei】Yes, they work fine with different boards with different cards, we
tested it. In fact, these values are the generally appropriate values given by
our controller chip colleagues.
For SDR50 it will not come into dw_mci_hi3660_execute_tuning() and
use the given value in hs_timing_cfg[][]{} when dw_mci_hi3660_init();
5. > + for (i = 0; i < NUM_PHASES; ++i, ++smpl_phase) {
+ smpl_phase %= 32;
+
+ mci_writel(host, TMOUT, ~0);
Why you need this?
【liwei】This is what SDR104 needs to find the optimum phase point.
I meant I didn't get the point of why you set the TMOUT here.
Tips: For further wise, please don't top post and inline your answer
in the previous mail so that we could better trace down the issue
we are talking about.
Thank you again and look forward to your reply!
-邮件原件-
发件人: Shawn Lin [mailto:shawn@rock-chips.com]
发送时间: 2017年8月9日 11:48
收件人: liwei (CM); ulf.hans...@linaro.org; jh80.ch...@samsung.com;
wsa+rene...@sang-engineering.com; hkallwe...@gmail.com;
linux-...@vger.kernel.org; linux-kernel@vger.kernel.org
抄送: adrian.hun...@intel.com; shawn@rock-chips.com; guodong...@linaro.org
主题: Re: [PATCH v9 2/2] mmc: dw_mmc-k3: add sd support for hi3660
On 2017/8/9 11:25, Li Wei wrote:
Add sd card support for hi3660 soc
Signed-off-by: Li Wei
Signed-off-by: Chen Jun
I did some comment for your v6 but probably you miss them.
And it's still incorrect for your changelog.
--
Major changes in v3:
- solve review comments from Heiner Kallweit.
*use the GENMASK and FIELD_PREP macros replace the bit shift operation.
*use usleep_range() replace udelay() and mdelay().
Major changes in v4:
- solve review comments from Jaehoon Chung.
*move common register for dwmmc controller to dwmmc header file.
*modify definitions type of some register variables.
*get rid of the magic numbers.
Major changes in v5:
- further improve coding style.
Major changes in v6:
- solve review comments for Jaehoon Chung.
*modify dw_mci_hi3660_set_ios() to static.
*fix the comment style.
Major changes in v7:
- solve review comments for John Stultz.
*remove reset code in dw_mmc-k3.c,use reset in core mmc.
Major changes in v8:
- modify patch v7 name and dependency order.
Major changes in v9:
- solve review comments for Ulf Hansson.
*use mmc_regulator_set_vqmmc() instead of regulator_set_voltage().
---
should be here...
drivers/mmc/host/dw_mmc-k3.c | 301
+++
drivers/mmc/host/dw_mmc.h| 2 +
2 files changed, 303 insertions(+)
diff --git a/drivers/mmc/host/dw_mmc-k3.c
b/drivers/mmc/host/dw_mmc-k3.c index e38fb0020bb1..f6910bed55ef 100644
--- a/drivers/mmc/host/dw_mmc-k3.c
+++ b/drivers/mmc/host/dw_mmc-k3.c
@@ -8,6 +8,8 @@
* (at your option) any later version.
*/
+#include
+#include
#include
#include
#include
@@ -28,7 +30,38 @@
#define AO_SCTRL_SEL18 BIT(10)
#define AO_SCTRL_CTRL3 0x40C
+#define DWMMC_SDIO_ID 2
+
+#define SOC_SCTRL_SCPERCTRL5(0x314)
+#define SDCARD_IO_SEL18 BIT(2)
+
+#define SDCARD_RD_THRESHOLD (512)
+
+#define GENCLK_DIV (7)
+
+#define GPIO_CLK_ENABLE BIT(16)
+#define GPIO_CLK_DIV_MASK GENMASK(11, 8)
+#define GPIO_USE_SAMPLE_DLY_MASK GENMASK(13, 13)
+#define UHS_REG_EXT_SAMPLE_PHASE_MASK GENMASK(20, 16)
+#define UHS_REG_EXT_SAMPLE_DRVPHASE_MASK GENMASK(25, 21)
