From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756363AbcHBXoU (ORCPT ); Tue, 2 Aug 2016 19:44:20 -0400 Received: from mailout2.samsung.com ([203.254.224.25]:52368 "EHLO mailout2.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753779AbcHBXoP (ORCPT ); Tue, 2 Aug 2016 19:44:15 -0400 X-AuditID: cbfee68d-f79286d000007a9a-fe-57a1302a919b Message-id: <57A13028.2030506@samsung.com> Date: Wed, 03 Aug 2016 08:43:36 +0900 From: Jaehoon Chung User-Agent: Mozilla/5.0 (X11; Linux i686; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-version: 1.0 To: Icenowy Zheng , Rob Herring , Maxime Ripard , Chen-Yu Tsai , Ulf Hansson , Hans de Goede Cc: Mark Rutland , Michal Suchanek , devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-mmc@vger.kernel.org Subject: Re: [PATCH v3 2/2] mmc: host: sunxi: add support for A64 mmc controller References: <20160801151351.51854-1-icenowy@aosc.xyz> <20160801151351.51854-3-icenowy@aosc.xyz> In-reply-to: <20160801151351.51854-3-icenowy@aosc.xyz> Content-type: text/plain; charset=windows-1252 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrEIsWRmVeSWpSXmKPExsWyRsSkUFfLYGG4wYLbJhbzj5xjtXhzfDqT xfZ1LBa7H9xmsdj0+BqrxeVdc9gsjvzvZ7RYev0ik8XWTXuZLVr3HmG3OL423OLnofNMDjwe 57beY/FYM28No8eGR6tZPZ5susjosXPWXXaPTas62TzuXNvD5rF5Sb3H+31X2Tw+b5IL4Iri sklJzcksSy3St0vgylg1cwl7QYd2xaXFE5kbGFcodTFyckgImEhsPfiIGcIWk7hwbz1bFyMX h5DACkaJt6vfscIUvV0xnQkisZRRYu7p9ewQzgNGiVMPdgM5HBy8AloSmxbrgTSwCKhK3Dxw FqyZTUBHYvu340wgtqhAmMSDdXvB4rwCghI/Jt9jAZkjIvCcUWLauQNgQ5kFzjBKtB2ezAZS JSwQIHFySRvU6q2MEseOXWAB2cYpYCZx7L0XiMksoCdx/6IWSDmzgLzE5jVvmUHKJQQWckic ubCJCeIiAYlvkw+BtUoIyEpsOgD1sqTEwRU3WCYwis1CctMshKmzkExdwMi8ilE0tSC5oDgp vchQrzgxt7g0L10vOT93EyMwqk//e9a7g/H2AetDjAIcjEo8vA/OLAgXYk0sK67MPcRoCnTE RGYp0eR8YOrIK4k3NDYzsjA1MTU2Mrc0UxLnVZT6GSwkkJ5YkpqdmlqQWhRfVJqTWnyIkYmD U6qBcXnyKfYvZ9zOiFXN+3d2ZjlvSyqj7NI1jcovW33C1lzQn72M3T9nibrrjF9pm9+WPTgk l8Q67VhL47evaie4t36WO3wtaWbSfvPEZxvbZ5X3nzvtYxCk5dpsIJzJbbZTO7m8ZNah4AWM nnqzisIfFfq9qdjz4Nq7us8rrAOeltlt8C44eed2iBJLcUaioRZzUXEiAFIMkdXlAgAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprPKsWRmVeSWpSXmKPExsVy+t9jAV0tg4XhBp9f61rMP3KO1eLN8elM FtvXsVjsfnCbxWLT42usFpd3zWGzOPK/n9Fi6fWLTBZbN+1ltmjde4Td4vjacIufh84zOfB4 nNt6j8Vjzbw1jB4bHq1m9Xiy6SKjx85Zd9k9Nq3qZPO4c20Pm8fmJfUe7/ddZfP4vEkugCuq gdEmIzUxJbVIITUvOT8lMy/dVsk7ON453tTMwFDX0NLCXEkhLzE31VbJxSdA1y0zB+hsJYWy xJxSoFBAYnGxkr4dpgmhIW66FjCNEbq+IUFwPUYGaCBhDWPGqplL2As6tCsuLZ7I3MC4QqmL kZNDQsBE4u2K6UwQtpjEhXvr2boYuTiEBJYySsw9vZ4dwnnAKHHqwW4gh4ODV0BLYtNiPZAG FgFViZsHzrKC2GwCOhLbvx0HGyQqECbxYN1esDivgKDEj8n3WEDmiAg8Z5SYdu4A2FBmgTOM Em2HJ7OBVAkLBEicXNLGBLFtK6PEsWMXWEC2cQqYSRx77wViMgvoSdy/qAVSziwgL7F5zVvm CYwCs5DsmIVQNQtJ1QJG5lWMEqkFyQXFSem5Rnmp5XrFibnFpXnpesn5uZsYwYnjmfQOxsO7 3A8xCnAwKvHwdoguDBdiTSwrrsw9xCjBwawkwhuuBxTiTUmsrEotyo8vKs1JLT7EaAoMhInM UqLJ+cCkllcSb2hsYmZkaWRuaGFkbK4kzvv4/7owIYH0xJLU7NTUgtQimD4mDk6pBsbqVU6e 5SFOR97VSlst4py9j5NJuUP88jTxmN3bp/o7uGqe9F/bnXkwy1trb90pN4ZXE3KutTMou2zd 2rZpfXDqrYSsj+smmuRdrGvy4Llo9ZpnV8S+kPBpJ2pMYuW/79r3zr22JOnM//Wfkq6vW/j/ vu5Lvdae++++ds6+PtFAukT3dudxtgglluKMREMt5qLiRABDw1QvMgMAAA== DLP-Filter: Pass X-MTR: 20000000000000000@CPGS X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Icenowy, On 08/02/2016 12:13 AM, Icenowy Zheng wrote: > A64 SoC features a MMC controller which need only the mod clock, and can > calibrate delay by itself. This patch adds support for the new MMC > controller IP core. > > Signed-off-by: Icenowy Zheng > --- > Changes in v2: > - Rebased on Hans de Goede's patchset. > Changes in v3: > - Tidy up based on Hans de Goede's opinions. > > drivers/mmc/host/sunxi-mmc.c | 66 ++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 66 insertions(+) > > diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c > index 2ec91ce..3c26180 100644 > --- a/drivers/mmc/host/sunxi-mmc.c > +++ b/drivers/mmc/host/sunxi-mmc.c > @@ -72,6 +72,13 @@ > #define SDXC_REG_CHDA (0x90) > #define SDXC_REG_CBDA (0x94) > > +/* New registers introduced in A64 */ > +#define SDXC_REG_A12A 0x058 /* SMC Auto Command 12 Register */ > +#define SDXC_REG_SD_NTSR 0x05C /* SMC New Timing Set Register */ > +#define SDXC_REG_DRV_DL 0x140 /* Drive Delay Control Register */ > +#define SDXC_REG_SAMP_DL_REG 0x144 /* SMC sample delay control */ > +#define SDXC_REG_DS_DL_REG 0x148 /* SMC data strobe delay control */ > + > #define mmc_readl(host, reg) \ > readl((host)->reg_base + SDXC_##reg) > #define mmc_writel(host, reg, value) \ > @@ -217,6 +224,15 @@ > #define SDXC_CLK_50M_DDR 3 > #define SDXC_CLK_50M_DDR_8BIT 4 > > +#define SDXC_2X_TIMING_MODE BIT(31) > + > +#define SDXC_CAL_START BIT(15) > +#define SDXC_CAL_DONE BIT(14) > +#define SDXC_CAL_DL_SHIFT 8 > +#define SDXC_CAL_DL_SW_EN BIT(7) > +#define SDXC_CAL_DL_SW_SHIFT 0 > +#define SDXC_CAL_DL_MASK 0x3f > + > struct sunxi_mmc_clk_delay { > u32 output; > u32 sample; > @@ -232,6 +248,9 @@ struct sunxi_idma_des { > struct sunxi_mmc_cfg { > u32 idma_des_size_bits; > const struct sunxi_mmc_clk_delay *clk_delays; > + > + /* does the IP block support autocalibration? */ > + bool can_calibrate; > }; > > struct sunxi_mmc_host { > @@ -657,6 +676,38 @@ static int sunxi_mmc_oclk_onoff(struct sunxi_mmc_host *host, u32 oclk_en) > return 0; > } > > +static int sunxi_mmc_calibrate(struct sunxi_mmc_host *host, > + struct mmc_ios *ios, int reg_off) Where is mmc_ios structure used in this function? And why passing the reg_off? > +{ > + u32 reg = readl(host->reg_base + reg_off); > + u32 delay; delay doesn't need to use u32.. reg_off is only passed with SDXC_REG_SAMP_DL_REG..this function doesn't reuse anywhere. > + > + if (!host->cfg->can_calibrate) > + return 0; > + > + reg &= ~(SDXC_CAL_DL_MASK << SDXC_CAL_DL_SW_SHIFT); > + reg &= ~SDXC_CAL_DL_SW_EN; > + > + writel(reg | SDXC_CAL_START, host->reg_base + reg_off); > + > + dev_dbg(mmc_dev(host->mmc), "calibration started\n"); > + > + while (!((reg = readl(host->reg_base + reg_off)) & SDXC_CAL_DONE)) > + cpu_relax(); If never hit this condition, infinite loop? > + > + delay = (reg >> SDXC_CAL_DL_SHIFT) & SDXC_CAL_DL_MASK; > + > + reg &= ~SDXC_CAL_START; > + reg |= (delay << SDXC_CAL_DL_SW_SHIFT) | SDXC_CAL_DL_SW_EN; Something is strange. It seems to maintain the delay value. reg &= ~(SDXC_CAL_START | (SDXC_CAL_DL_MASK << SDXC_CAL_DL_SHIFT)); reg |= SDXC_CAL_DL_SW_EN; is it same thing? > + > + writel(reg, host->reg_base + reg_off); > + > + dev_dbg(mmc_dev(host->mmc), "calibration ended, res is 0x%x\n", reg); s/res is/reg is ? > + > + /* TODO: enable calibrate on sdc2 SDXC_REG_DS_DL_REG of A64 */ > + return 0; > +} > + > static int sunxi_mmc_clk_set_phase(struct sunxi_mmc_host *host, > struct mmc_ios *ios, u32 rate) > { > @@ -731,6 +782,10 @@ static int sunxi_mmc_clk_set_rate(struct sunxi_mmc_host *host, > if (ret) > return ret; > > + ret = sunxi_mmc_calibrate(host, ios, SDXC_REG_SAMP_DL_REG); > + if (ret) > + return ret; Never enter this condition. sunxi_mmc_calibrate() is always returned 0. Best Regards, Jaehoon Chung > + > return sunxi_mmc_oclk_onoff(host, 1); > } > > @@ -982,21 +1037,31 @@ static const struct sunxi_mmc_clk_delay sun9i_mmc_clk_delays[] = { > static const struct sunxi_mmc_cfg sun4i_a10_cfg = { > .idma_des_size_bits = 13, > .clk_delays = NULL, > + .can_calibrate = false, > }; > > static const struct sunxi_mmc_cfg sun5i_a13_cfg = { > .idma_des_size_bits = 16, > .clk_delays = NULL, > + .can_calibrate = false, > }; > > static const struct sunxi_mmc_cfg sun7i_a20_cfg = { > .idma_des_size_bits = 16, > .clk_delays = sunxi_mmc_clk_delays, > + .can_calibrate = false, > }; > > static const struct sunxi_mmc_cfg sun9i_a80_cfg = { > .idma_des_size_bits = 16, > .clk_delays = sun9i_mmc_clk_delays, > + .can_calibrate = false, > +}; > + > +static const struct sunxi_mmc_cfg sun50i_a64_cfg = { > + .idma_des_size_bits = 16, > + .clk_delays = NULL, > + .can_calibrate = true, > }; > > static const struct of_device_id sunxi_mmc_of_match[] = { > @@ -1004,6 +1069,7 @@ static const struct of_device_id sunxi_mmc_of_match[] = { > { .compatible = "allwinner,sun5i-a13-mmc", .data = &sun5i_a13_cfg }, > { .compatible = "allwinner,sun7i-a20-mmc", .data = &sun7i_a20_cfg }, > { .compatible = "allwinner,sun9i-a80-mmc", .data = &sun9i_a80_cfg }, > + { .compatible = "allwinner,sun50i-a64-mmc", .data = &sun50i_a64_cfg }, > { /* sentinel */ } > }; > MODULE_DEVICE_TABLE(of, sunxi_mmc_of_match); > From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jaehoon Chung Subject: Re: [PATCH v3 2/2] mmc: host: sunxi: add support for A64 mmc controller Date: Wed, 03 Aug 2016 08:43:36 +0900 Message-ID: <57A13028.2030506@samsung.com> References: <20160801151351.51854-1-icenowy@aosc.xyz> <20160801151351.51854-3-icenowy@aosc.xyz> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-reply-to: <20160801151351.51854-3-icenowy@aosc.xyz> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: Icenowy Zheng , Rob Herring , Maxime Ripard , Chen-Yu Tsai , Ulf Hansson , Hans de Goede Cc: Mark Rutland , devicetree@vger.kernel.org, Michal Suchanek , linux-mmc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org List-Id: devicetree@vger.kernel.org Hi Icenowy, On 08/02/2016 12:13 AM, Icenowy Zheng wrote: > A64 SoC features a MMC controller which need only the mod clock, and can > calibrate delay by itself. This patch adds support for the new MMC > controller IP core. > > Signed-off-by: Icenowy Zheng > --- > Changes in v2: > - Rebased on Hans de Goede's patchset. > Changes in v3: > - Tidy up based on Hans de Goede's opinions. > > drivers/mmc/host/sunxi-mmc.c | 66 ++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 66 insertions(+) > > diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c > index 2ec91ce..3c26180 100644 > --- a/drivers/mmc/host/sunxi-mmc.c > +++ b/drivers/mmc/host/sunxi-mmc.c > @@ -72,6 +72,13 @@ > #define SDXC_REG_CHDA (0x90) > #define SDXC_REG_CBDA (0x94) > > +/* New registers introduced in A64 */ > +#define SDXC_REG_A12A 0x058 /* SMC Auto Command 12 Register */ > +#define SDXC_REG_SD_NTSR 0x05C /* SMC New Timing Set Register */ > +#define SDXC_REG_DRV_DL 0x140 /* Drive Delay Control Register */ > +#define SDXC_REG_SAMP_DL_REG 0x144 /* SMC sample delay control */ > +#define SDXC_REG_DS_DL_REG 0x148 /* SMC data strobe delay control */ > + > #define mmc_readl(host, reg) \ > readl((host)->reg_base + SDXC_##reg) > #define mmc_writel(host, reg, value) \ > @@ -217,6 +224,15 @@ > #define SDXC_CLK_50M_DDR 3 > #define SDXC_CLK_50M_DDR_8BIT 4 > > +#define SDXC_2X_TIMING_MODE BIT(31) > + > +#define SDXC_CAL_START BIT(15) > +#define SDXC_CAL_DONE BIT(14) > +#define SDXC_CAL_DL_SHIFT 8 > +#define SDXC_CAL_DL_SW_EN BIT(7) > +#define SDXC_CAL_DL_SW_SHIFT 0 > +#define SDXC_CAL_DL_MASK 0x3f > + > struct sunxi_mmc_clk_delay { > u32 output; > u32 sample; > @@ -232,6 +248,9 @@ struct sunxi_idma_des { > struct sunxi_mmc_cfg { > u32 idma_des_size_bits; > const struct sunxi_mmc_clk_delay *clk_delays; > + > + /* does the IP block support autocalibration? */ > + bool can_calibrate; > }; > > struct sunxi_mmc_host { > @@ -657,6 +676,38 @@ static int sunxi_mmc_oclk_onoff(struct sunxi_mmc_host *host, u32 oclk_en) > return 0; > } > > +static int sunxi_mmc_calibrate(struct sunxi_mmc_host *host, > + struct mmc_ios *ios, int reg_off) Where is mmc_ios structure used in this function? And why passing the reg_off? > +{ > + u32 reg = readl(host->reg_base + reg_off); > + u32 delay; delay doesn't need to use u32.. reg_off is only passed with SDXC_REG_SAMP_DL_REG..this function doesn't reuse anywhere. > + > + if (!host->cfg->can_calibrate) > + return 0; > + > + reg &= ~(SDXC_CAL_DL_MASK << SDXC_CAL_DL_SW_SHIFT); > + reg &= ~SDXC_CAL_DL_SW_EN; > + > + writel(reg | SDXC_CAL_START, host->reg_base + reg_off); > + > + dev_dbg(mmc_dev(host->mmc), "calibration started\n"); > + > + while (!((reg = readl(host->reg_base + reg_off)) & SDXC_CAL_DONE)) > + cpu_relax(); If never hit this condition, infinite loop? > + > + delay = (reg >> SDXC_CAL_DL_SHIFT) & SDXC_CAL_DL_MASK; > + > + reg &= ~SDXC_CAL_START; > + reg |= (delay << SDXC_CAL_DL_SW_SHIFT) | SDXC_CAL_DL_SW_EN; Something is strange. It seems to maintain the delay value. reg &= ~(SDXC_CAL_START | (SDXC_CAL_DL_MASK << SDXC_CAL_DL_SHIFT)); reg |= SDXC_CAL_DL_SW_EN; is it same thing? > + > + writel(reg, host->reg_base + reg_off); > + > + dev_dbg(mmc_dev(host->mmc), "calibration ended, res is 0x%x\n", reg); s/res is/reg is ? > + > + /* TODO: enable calibrate on sdc2 SDXC_REG_DS_DL_REG of A64 */ > + return 0; > +} > + > static int sunxi_mmc_clk_set_phase(struct sunxi_mmc_host *host, > struct mmc_ios *ios, u32 rate) > { > @@ -731,6 +782,10 @@ static int sunxi_mmc_clk_set_rate(struct sunxi_mmc_host *host, > if (ret) > return ret; > > + ret = sunxi_mmc_calibrate(host, ios, SDXC_REG_SAMP_DL_REG); > + if (ret) > + return ret; Never enter this condition. sunxi_mmc_calibrate() is always returned 0. Best Regards, Jaehoon Chung > + > return sunxi_mmc_oclk_onoff(host, 1); > } > > @@ -982,21 +1037,31 @@ static const struct sunxi_mmc_clk_delay sun9i_mmc_clk_delays[] = { > static const struct sunxi_mmc_cfg sun4i_a10_cfg = { > .idma_des_size_bits = 13, > .clk_delays = NULL, > + .can_calibrate = false, > }; > > static const struct sunxi_mmc_cfg sun5i_a13_cfg = { > .idma_des_size_bits = 16, > .clk_delays = NULL, > + .can_calibrate = false, > }; > > static const struct sunxi_mmc_cfg sun7i_a20_cfg = { > .idma_des_size_bits = 16, > .clk_delays = sunxi_mmc_clk_delays, > + .can_calibrate = false, > }; > > static const struct sunxi_mmc_cfg sun9i_a80_cfg = { > .idma_des_size_bits = 16, > .clk_delays = sun9i_mmc_clk_delays, > + .can_calibrate = false, > +}; > + > +static const struct sunxi_mmc_cfg sun50i_a64_cfg = { > + .idma_des_size_bits = 16, > + .clk_delays = NULL, > + .can_calibrate = true, > }; > > static const struct of_device_id sunxi_mmc_of_match[] = { > @@ -1004,6 +1069,7 @@ static const struct of_device_id sunxi_mmc_of_match[] = { > { .compatible = "allwinner,sun5i-a13-mmc", .data = &sun5i_a13_cfg }, > { .compatible = "allwinner,sun7i-a20-mmc", .data = &sun7i_a20_cfg }, > { .compatible = "allwinner,sun9i-a80-mmc", .data = &sun9i_a80_cfg }, > + { .compatible = "allwinner,sun50i-a64-mmc", .data = &sun50i_a64_cfg }, > { /* sentinel */ } > }; > MODULE_DEVICE_TABLE(of, sunxi_mmc_of_match); > From mboxrd@z Thu Jan 1 00:00:00 1970 From: jh80.chung@samsung.com (Jaehoon Chung) Date: Wed, 03 Aug 2016 08:43:36 +0900 Subject: [PATCH v3 2/2] mmc: host: sunxi: add support for A64 mmc controller In-Reply-To: <20160801151351.51854-3-icenowy@aosc.xyz> References: <20160801151351.51854-1-icenowy@aosc.xyz> <20160801151351.51854-3-icenowy@aosc.xyz> Message-ID: <57A13028.2030506@samsung.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Icenowy, On 08/02/2016 12:13 AM, Icenowy Zheng wrote: > A64 SoC features a MMC controller which need only the mod clock, and can > calibrate delay by itself. This patch adds support for the new MMC > controller IP core. > > Signed-off-by: Icenowy Zheng > --- > Changes in v2: > - Rebased on Hans de Goede's patchset. > Changes in v3: > - Tidy up based on Hans de Goede's opinions. > > drivers/mmc/host/sunxi-mmc.c | 66 ++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 66 insertions(+) > > diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c > index 2ec91ce..3c26180 100644 > --- a/drivers/mmc/host/sunxi-mmc.c > +++ b/drivers/mmc/host/sunxi-mmc.c > @@ -72,6 +72,13 @@ > #define SDXC_REG_CHDA (0x90) > #define SDXC_REG_CBDA (0x94) > > +/* New registers introduced in A64 */ > +#define SDXC_REG_A12A 0x058 /* SMC Auto Command 12 Register */ > +#define SDXC_REG_SD_NTSR 0x05C /* SMC New Timing Set Register */ > +#define SDXC_REG_DRV_DL 0x140 /* Drive Delay Control Register */ > +#define SDXC_REG_SAMP_DL_REG 0x144 /* SMC sample delay control */ > +#define SDXC_REG_DS_DL_REG 0x148 /* SMC data strobe delay control */ > + > #define mmc_readl(host, reg) \ > readl((host)->reg_base + SDXC_##reg) > #define mmc_writel(host, reg, value) \ > @@ -217,6 +224,15 @@ > #define SDXC_CLK_50M_DDR 3 > #define SDXC_CLK_50M_DDR_8BIT 4 > > +#define SDXC_2X_TIMING_MODE BIT(31) > + > +#define SDXC_CAL_START BIT(15) > +#define SDXC_CAL_DONE BIT(14) > +#define SDXC_CAL_DL_SHIFT 8 > +#define SDXC_CAL_DL_SW_EN BIT(7) > +#define SDXC_CAL_DL_SW_SHIFT 0 > +#define SDXC_CAL_DL_MASK 0x3f > + > struct sunxi_mmc_clk_delay { > u32 output; > u32 sample; > @@ -232,6 +248,9 @@ struct sunxi_idma_des { > struct sunxi_mmc_cfg { > u32 idma_des_size_bits; > const struct sunxi_mmc_clk_delay *clk_delays; > + > + /* does the IP block support autocalibration? */ > + bool can_calibrate; > }; > > struct sunxi_mmc_host { > @@ -657,6 +676,38 @@ static int sunxi_mmc_oclk_onoff(struct sunxi_mmc_host *host, u32 oclk_en) > return 0; > } > > +static int sunxi_mmc_calibrate(struct sunxi_mmc_host *host, > + struct mmc_ios *ios, int reg_off) Where is mmc_ios structure used in this function? And why passing the reg_off? > +{ > + u32 reg = readl(host->reg_base + reg_off); > + u32 delay; delay doesn't need to use u32.. reg_off is only passed with SDXC_REG_SAMP_DL_REG..this function doesn't reuse anywhere. > + > + if (!host->cfg->can_calibrate) > + return 0; > + > + reg &= ~(SDXC_CAL_DL_MASK << SDXC_CAL_DL_SW_SHIFT); > + reg &= ~SDXC_CAL_DL_SW_EN; > + > + writel(reg | SDXC_CAL_START, host->reg_base + reg_off); > + > + dev_dbg(mmc_dev(host->mmc), "calibration started\n"); > + > + while (!((reg = readl(host->reg_base + reg_off)) & SDXC_CAL_DONE)) > + cpu_relax(); If never hit this condition, infinite loop? > + > + delay = (reg >> SDXC_CAL_DL_SHIFT) & SDXC_CAL_DL_MASK; > + > + reg &= ~SDXC_CAL_START; > + reg |= (delay << SDXC_CAL_DL_SW_SHIFT) | SDXC_CAL_DL_SW_EN; Something is strange. It seems to maintain the delay value. reg &= ~(SDXC_CAL_START | (SDXC_CAL_DL_MASK << SDXC_CAL_DL_SHIFT)); reg |= SDXC_CAL_DL_SW_EN; is it same thing? > + > + writel(reg, host->reg_base + reg_off); > + > + dev_dbg(mmc_dev(host->mmc), "calibration ended, res is 0x%x\n", reg); s/res is/reg is ? > + > + /* TODO: enable calibrate on sdc2 SDXC_REG_DS_DL_REG of A64 */ > + return 0; > +} > + > static int sunxi_mmc_clk_set_phase(struct sunxi_mmc_host *host, > struct mmc_ios *ios, u32 rate) > { > @@ -731,6 +782,10 @@ static int sunxi_mmc_clk_set_rate(struct sunxi_mmc_host *host, > if (ret) > return ret; > > + ret = sunxi_mmc_calibrate(host, ios, SDXC_REG_SAMP_DL_REG); > + if (ret) > + return ret; Never enter this condition. sunxi_mmc_calibrate() is always returned 0. Best Regards, Jaehoon Chung > + > return sunxi_mmc_oclk_onoff(host, 1); > } > > @@ -982,21 +1037,31 @@ static const struct sunxi_mmc_clk_delay sun9i_mmc_clk_delays[] = { > static const struct sunxi_mmc_cfg sun4i_a10_cfg = { > .idma_des_size_bits = 13, > .clk_delays = NULL, > + .can_calibrate = false, > }; > > static const struct sunxi_mmc_cfg sun5i_a13_cfg = { > .idma_des_size_bits = 16, > .clk_delays = NULL, > + .can_calibrate = false, > }; > > static const struct sunxi_mmc_cfg sun7i_a20_cfg = { > .idma_des_size_bits = 16, > .clk_delays = sunxi_mmc_clk_delays, > + .can_calibrate = false, > }; > > static const struct sunxi_mmc_cfg sun9i_a80_cfg = { > .idma_des_size_bits = 16, > .clk_delays = sun9i_mmc_clk_delays, > + .can_calibrate = false, > +}; > + > +static const struct sunxi_mmc_cfg sun50i_a64_cfg = { > + .idma_des_size_bits = 16, > + .clk_delays = NULL, > + .can_calibrate = true, > }; > > static const struct of_device_id sunxi_mmc_of_match[] = { > @@ -1004,6 +1069,7 @@ static const struct of_device_id sunxi_mmc_of_match[] = { > { .compatible = "allwinner,sun5i-a13-mmc", .data = &sun5i_a13_cfg }, > { .compatible = "allwinner,sun7i-a20-mmc", .data = &sun7i_a20_cfg }, > { .compatible = "allwinner,sun9i-a80-mmc", .data = &sun9i_a80_cfg }, > + { .compatible = "allwinner,sun50i-a64-mmc", .data = &sun50i_a64_cfg }, > { /* sentinel */ } > }; > MODULE_DEVICE_TABLE(of, sunxi_mmc_of_match); >