MIME-version: 1.0
Content-type: text/plain; charset="utf-8"
Subject: Re: [PATCH v3] mmc: dw_mmc-k3: add sd support for hi3660
To: Jaehoon Chung <jh80.chung@gmail.com>, ulf.hansson@linaro.org,
        adrian.hunter@intel.com, shawn.lin@rock-chips.com,
        wsa+renesas@sang-engineering.com, hkallweit1@gmail.com,
        linux-mmc@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: guodong.xu@linaro.org
From: Jaehoon Chung <jh80.chung@samsung.com>
Message-id: <a1af9970-f7e0-375a-fdd0-7e247769ca5f@samsung.com>
Date: Tue, 13 Jun 2017 07:23:39 +0900
User-Agent: Mozilla/5.0 (X11; Linux i686; rv:45.0) Gecko/20100101
        Thunderbird/45.3.0
In-reply-to: <06ee8525-0845-871f-d8c3-7b15e9773ccc@gmail.com>
Content-transfer-encoding: 8bit
CMS-TYPE: 105P
DLP-Filter: Pass
References: <20170612074602.55280-1-liwei213@huawei.com>
        <CGME20170612142756epcas4p196bf757f69fd44a50b0f8b9902738c6a@epcas4p1.samsung.com>
        <06ee8525-0845-871f-d8c3-7b15e9773ccc@gmail.com>
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 11239
Lines: 403

Hi,

On 06/12/2017 11:27 PM, Jaehoon Chung wrote:
> Hi Li,
> 
> On 2017년 06월 12일 16:46, liwei wrote:
>> Add sd card support for hi3660 soc
>>
>> 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().
> 
> I had added the some comments about your previous patch.
> Refer to below...
> 
> https://patchwork.kernel.org/patch/9747495/

Before sending patch, run checkpatch...I will not apply this patch.

Best Regards,
Jaehoon Chung

> 
>>
>> Signed-off-by: Li Wei <liwei213@huawei.com>
>> Signed-off-by: Chen Jun <chenjun14@huawei.com>
>> ---
> 
> Locate changelog at here.
> 
> Best Regards,
> Jaehoon Chung
> 
>>  drivers/mmc/host/dw_mmc-k3.c | 314 +++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 314 insertions(+)
>>
>> diff --git a/drivers/mmc/host/dw_mmc-k3.c b/drivers/mmc/host/dw_mmc-k3.c
>> index e38fb0020bb1..a6e13bd83b9f 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 <linux/bitops.h>
>> +#include <linux/bitfield.h>
>>  #include <linux/clk.h>
>>  #include <linux/mfd/syscon.h>
>>  #include <linux/mmc/host.h>
>> @@ -28,7 +30,40 @@
>>  #define AO_SCTRL_SEL18		BIT(10)
>>  #define AO_SCTRL_CTRL3		0x40C
>>  
>> +#define DWMMC_SD_ID   1
>> +#define DWMMC_SDIO_ID 2
>> +
>> +#define SOC_SCTRL_SCPERCTRL5    (0x314)
>> +#define SDCARD_IO_SEL18         BIT(2)
>> +
>> +#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)
>> +#define UHS_REG_EXT_SAMPLE_DLY_MASK       GENMASK(30, 26)
>> +
>> +#define SDMMC_UHS_REG_EXT	0x108
>> +#define SDMMC_ENABLE_SHIFT	0x110
>> +
>> +#define TIMING_MODE     3
>> +#define TIMING_CFG_NUM 10
>> +
>> +#define PULL_DOWN BIT(1)
>> +#define PULL_UP   BIT(0)
>> +
>> +#define NUM_PHASES (40)
>> +
>> +#define ENABLE_SHIFT_MIN_SMPL (4)
>> +#define ENABLE_SHIFT_MAX_SMPL (12)
>> +#define USE_DLY_MIN_SMPL (11)
>> +#define USE_DLY_MAX_SMPL (14)
>> +
>>  struct k3_priv {
>> +	u8 ctrl_id;
>> +	u32 cur_speed;
>>  	struct regmap	*reg;
>>  };
>>  
>> @@ -38,6 +73,41 @@ static unsigned long dw_mci_hi6220_caps[] = {
>>  	0
>>  };
>>  
>> +struct hs_timing {
>> +	int drv_phase;
>> +	int sam_dly;
>> +	int sam_phase_max;
>> +	int sam_phase_min;
>> +};
>> +
>> +struct hs_timing hs_timing_cfg[TIMING_MODE][TIMING_CFG_NUM] = {
>> +	{ /* reserved */ },
>> +	{ /* SD */
>> +		{7, 0, 15, 15,},  /* 0: LEGACY 400k */
>> +		{6, 0,  4,  4,},  /* 1: MMC_HS */
>> +		{6, 0,  3,  3,},  /* 2: SD_HS */
>> +		{6, 0, 15, 15,},  /* 3: SDR12 */
>> +		{6, 0,  2,  2,},  /* 4: SDR25 */
>> +		{4, 0, 11,  0,},  /* 5: SDR50 */
>> +		{6, 4, 15,  0,},  /* 6: SDR104 */
>> +		{0},              /* 7: DDR50 */
>> +		{0},              /* 8: DDR52 */
>> +		{0},              /* 9: HS200 */
>> +	},
>> +	{ /* SDIO */
>> +		{7, 0, 15, 15,},  /* 0: LEGACY 400k */
>> +		{0},              /* 1: MMC_HS */
>> +		{6, 0, 15, 15,},  /* 2: SD_HS */
>> +		{6, 0, 15, 15,},  /* 3: SDR12 */
>> +		{6, 0,  0,  0,},  /* 4: SDR25 */
>> +		{4, 0, 12,  0,},  /* 5: SDR50 */
>> +		{5, 4, 15,  0,},  /* 6: SDR104 */
>> +		{0},              /* 7: DDR50 */
>> +		{0},              /* 8: DDR52 */
>> +		{0},              /* 9: HS200 */
>> +	}
>> +};
>> +
>>  static void dw_mci_k3_set_ios(struct dw_mci *host, struct mmc_ios *ios)
>>  {
>>  	int ret;
>> @@ -66,6 +136,10 @@ static int dw_mci_hi6220_parse_dt(struct dw_mci *host)
>>  	if (IS_ERR(priv->reg))
>>  		priv->reg = NULL;
>>  
>> +	priv->ctrl_id = of_alias_get_id(host->dev->of_node, "mshc");
>> +	if (priv->ctrl_id < 0)
>> +		priv->ctrl_id = 0;
>> +
>>  	host->priv = priv;
>>  	return 0;
>>  }
>> @@ -144,7 +218,242 @@ static const struct dw_mci_drv_data hi6220_data = {
>>  	.execute_tuning		= dw_mci_hi6220_execute_tuning,
>>  };
>>  
>> +static void dw_mci_hs_set_timing(struct dw_mci *host, int timing, int sam_phase)
>> +{
>> +	int drv_phase;
>> +	int sam_dly;
>> +	int ctrl_id;
>> +	int use_sam_dly = 0;
>> +	int enable_shift = 0;
>> +	int reg_value;
>> +	struct k3_priv *priv;
>> +
>> +	priv = host->priv;
>> +	ctrl_id = priv->ctrl_id;
>> +
>> +	drv_phase = hs_timing_cfg[ctrl_id][timing].drv_phase;
>> +	sam_dly   = hs_timing_cfg[ctrl_id][timing].sam_dly;
>> +	if (sam_phase == -1)
>> +		sam_phase = (hs_timing_cfg[ctrl_id][timing].sam_phase_max +
>> +			     hs_timing_cfg[ctrl_id][timing].sam_phase_min) / 2;
>> +
>> +	if (timing == MMC_TIMING_UHS_SDR50 ||
>> +	    timing == MMC_TIMING_UHS_SDR104) {
>> +		if (sam_phase >= ENABLE_SHIFT_MIN_SMPL &&
>> +		    sam_phase <= ENABLE_SHIFT_MAX_SMPL)
>> +			enable_shift = 1;
>> +	}
>> +	if (timing == MMC_TIMING_UHS_SDR104) {
>> +		if (sam_phase >= USE_DLY_MIN_SMPL &&
>> +		    sam_phase <= USE_DLY_MAX_SMPL)
>> +			use_sam_dly = 1;
>> +	}
>> +
>> +	mci_writel(host, GPIO, 0x0);
>> +	usleep_range(5, 10);
>> +
>> +	reg_value = FIELD_PREP(UHS_REG_EXT_SAMPLE_PHASE_MASK, sam_phase) |\
>> +				FIELD_PREP(UHS_REG_EXT_SAMPLE_DLY_MASK ,sam_dly) |\
>> +				FIELD_PREP(UHS_REG_EXT_SAMPLE_DRVPHASE_MASK,drv_phase);		
>> +	mci_writel(host, UHS_REG_EXT, reg_value);
>> +
>> +	mci_writel(host, ENABLE_SHIFT, enable_shift);
>> +
>> +	reg_value = FIELD_PREP(GPIO_CLK_DIV_MASK, GENCLK_DIV) |\
>> +				FIELD_PREP(GPIO_USE_SAMPLE_DLY_MASK,use_sam_dly);
>> +	mci_writel(host, GPIO, (unsigned int)reg_value | GPIO_CLK_ENABLE);
>> +
>> +	/* We should delay 1ms wait for timing setting finished. */
>> +	usleep_range(1000, 2000);
>> +}
>> +
>> +int dw_mci_hi3660_init(struct dw_mci *host)
>> +{
>> +	/* set threshold to 512 bytes */
>> +	mci_writel(host, CDTHRCTL, 0x02000001);
>> +
>> +	dw_mci_hs_set_timing(host, MMC_TIMING_LEGACY, -1);
>> +	host->bus_hz /= (GENCLK_DIV + 1);
>> +
>> +	return 0;
>> +}
>> +
>> +static int dw_mci_set_sel18(struct dw_mci *host, bool set)
>> +{
>> +	int ret;
>> +	unsigned int val;
>> +	struct k3_priv *priv;
>> +
>> +	priv = host->priv;
>> +
>> +	val = set ? SDCARD_IO_SEL18 : 0;
>> +	ret = regmap_update_bits(priv->reg, SOC_SCTRL_SCPERCTRL5,
>> +				 SDCARD_IO_SEL18, val);
>> +	if (ret) {
>> +		dev_err(host->dev, "sel18 %u error\n", val);
>> +		return ret;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +void dw_mci_hi3660_set_ios(struct dw_mci *host, struct mmc_ios *ios)
>> +{
>> +	int ret;
>> +	unsigned long wanted;
>> +	unsigned long actual;
>> +	struct k3_priv *priv = host->priv;
>> +
>> +	if (!ios->clock || ios->clock == priv->cur_speed)
>> +		return;
>> +
>> +	wanted = ios->clock * (GENCLK_DIV + 1);
>> +	ret = clk_set_rate(host->ciu_clk, wanted);
>> +	if (ret) {
>> +		dev_err(host->dev, "failed to set rate %luHz\n", wanted);
>> +		return;
>> +	}
>> +	actual = clk_get_rate(host->ciu_clk);
>> +
>> +	dw_mci_hs_set_timing(host, ios->timing, -1);
>> +	host->bus_hz = actual / (GENCLK_DIV + 1);
>> +	host->current_speed = 0;
>> +	priv->cur_speed = host->bus_hz;
>> +}
>> +
>> +static int dw_mci_get_best_clksmpl(unsigned int sample_flag)
>> +{
>> +	int i;
>> +	int interval;
>> +	unsigned int v;
>> +	unsigned int len;
>> +	unsigned int range_start = 0;
>> +	unsigned int range_length = 0;
>> +	unsigned int middle_range = 0;
>> +
>> +	if (!sample_flag)
>> +		return -EIO;
>> +
>> +	if (~sample_flag == 0)
>> +		return 0;
>> +
>> +	i = ffs(sample_flag) - 1;
>> +
>> +	/*A clock cycle is divided into 32 phases, 
>> +	*each of which is represented by a bit, finding the optimal phase.
>> +	*/
>> +	while (i < 32) {
>> +		v = ror32(sample_flag, i);
>> +		len = ffs(~v) - 1;
>> +
>> +		if (len > range_length) {
>> +			range_length = len;
>> +			range_start = i;
>> +		}
>> +
>> +		interval = ffs(v >> len) - 1;
>> +		if (interval < 0)
>> +			break;
>> +
>> +		i += len + interval;
>> +	}
>> +
>> +	middle_range = range_start + range_length / 2;
>> +	if (middle_range >= 32)
>> +		middle_range %= 32;
>> +
>> +	return middle_range;
>> +}
>> +
>> +static int dw_mci_hi3660_execute_tuning(struct dw_mci_slot *slot, u32 opcode)
>> +{
>> +	int i = 0;
>> +	struct dw_mci *host = slot->host;
>> +	struct mmc_host *mmc = slot->mmc;
>> +	int sam_phase = 0;
>> +	u32 tuning_sample_flag = 0;
>> +	int best_clksmpl = 0;
>> +
>> +	for (i = 0; i < NUM_PHASES; ++i, ++sam_phase) {
>> +		sam_phase %= 32;
>> +
>> +		mci_writel(host, TMOUT, ~0);
>> +		dw_mci_hs_set_timing(host, mmc->ios.timing, sam_phase);
>> +
>> +		if (!mmc_send_tuning(mmc, opcode, NULL))
>> +			tuning_sample_flag |= (1 << sam_phase);
>> +		else
>> +			tuning_sample_flag &= ~(1 << sam_phase);
>> +	}
>> +
>> +	best_clksmpl = dw_mci_get_best_clksmpl(tuning_sample_flag);
>> +	if (best_clksmpl < 0) {
>> +		dev_err(host->dev, "All phases bad!\n");
>> +		return -EIO;
>> +	}
>> +
>> +	dw_mci_hs_set_timing(host, mmc->ios.timing, best_clksmpl);
>> +
>> +	dev_info(host->dev, "tuning ok best_clksmpl %u tuning_sample_flag %x\n",
>> +		 best_clksmpl, tuning_sample_flag);
>> +	return 0;
>> +}
>> +
>> +static int dw_mci_hi3660_switch_voltage(struct mmc_host *mmc,
>> +					struct mmc_ios *ios)
>> +{
>> +	int ret;
>> +	int min_uv = 0;
>> +	int max_uv = 0;
>> +	struct dw_mci_slot *slot = mmc_priv(mmc);
>> +	struct k3_priv *priv;
>> +	struct dw_mci *host;
>> +
>> +	host = slot->host;
>> +	priv = host->priv;
>> +
>> +	if (!priv || !priv->reg)
>> +		return 0;
>> +
>> +	if (priv->ctrl_id == DWMMC_SDIO_ID)
>> +		return 0;
>> +
>> +	if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330) {
>> +		ret = dw_mci_set_sel18(host, 0);
>> +		if (ret)
>> +			return ret;
>> +		min_uv = 2950000;
>> +		max_uv = 2950000;
>> +	} else if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_180) {
>> +		ret = dw_mci_set_sel18(host, 1);
>> +		if (ret)
>> +			return ret;
>> +		min_uv = 1800000;
>> +		max_uv = 1800000;
>> +	}
>> +
>> +	if (IS_ERR_OR_NULL(mmc->supply.vqmmc))
>> +		return 0;
>> +
>> +	ret = regulator_set_voltage(mmc->supply.vqmmc, min_uv, max_uv);
>> +	if (ret) {
>> +		dev_dbg(host->dev, "Regulator set error %d: %d - %d\n",
>> +			ret, min_uv, max_uv);
>> +		return ret;
>> +	}
>> +	return 0;
>> +}
>> +
>> +static const struct dw_mci_drv_data hi3660_data = {
>> +	.init = dw_mci_hi3660_init,
>> +	.set_ios = dw_mci_hi3660_set_ios,
>> +	.parse_dt = dw_mci_hi6220_parse_dt,
>> +	.execute_tuning = dw_mci_hi3660_execute_tuning,
>> +	.switch_voltage  = dw_mci_hi3660_switch_voltage,
>> +};
>> +
>>  static const struct of_device_id dw_mci_k3_match[] = {
>> +	{ .compatible = "hisilicon,hi3660-dw-mshc", .data = &hi3660_data, },
>>  	{ .compatible = "hisilicon,hi4511-dw-mshc", .data = &k3_drv_data, },
>>  	{ .compatible = "hisilicon,hi6220-dw-mshc", .data = &hi6220_data, },
>>  	{},
>> @@ -155,6 +464,11 @@ static int dw_mci_k3_probe(struct platform_device *pdev)
>>  {
>>  	const struct dw_mci_drv_data *drv_data;
>>  	const struct of_device_id *match;
>> +	struct reset_control	*rst;
>> +
>> +	rst = devm_reset_control_get(&pdev->dev, NULL);
>> +	if (!IS_ERR(rst))
>> +		reset_control_reset(rst);
>>  
>>  	match = of_match_node(dw_mci_k3_match, pdev->dev.of_node);
>>  	drv_data = match->data;
>>
> 
> 
>