Date: Fri, 31 Jul 2015 22:15:08 +0800
From: Dong Aisheng <aisheng.dong@freescale.com>
To: Haibo Chen <haibo.chen@freescale.com>
CC: <robh+dt@kernel.org>, <pawel.moll@arm.com>, <mark.rutland@arm.com>,
        <ijc+devicetree@hellion.org.uk>, <galak@codeaurora.org>,
        <shawnguo@kernel.org>, <kernel@pengutronix.de>,
        <linux@arm.linux.org.uk>, <ulf.hansson@linaro.org>,
        <johan.derycke@barco.com>, <fabio.estevam@freescale.com>,
        <b29396@freescale.com>, <devicetree@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <linux-arm-kernel@lists.infradead.org>,
        <linux-mmc@vger.kernel.org>
Subject: Re: [PATCH v3 1/6] mmc: sdhci-esdhc-imx: add imx7d support and
 support HS400
Message-ID: <20150731141507.GB27806@shlinux1.ap.freescale.net>
References: <1438160637-28061-1-git-send-email-haibo.chen@freescale.com>
 <1438160637-28061-2-git-send-email-haibo.chen@freescale.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <1438160637-28061-2-git-send-email-haibo.chen@freescale.com>
User-Agent: Mutt/1.5.20 (2009-06-14)
BN3PR03MB1383: X-MS-Exchange-Organization-RulesExecuted
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 31 Jul 2015 14:23:55.2384
 (UTC)
X-MS-Exchange-CrossTenant-Id: 710a03f5-10f6-4d38-9ff4-a80b81da590d
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=710a03f5-10f6-4d38-9ff4-a80b81da590d;Ip=[192.88.158.2];Helo=[az84smr01.freescale.net]
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN3PR03MB1383
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 6437
Lines: 182

On Wed, Jul 29, 2015 at 05:03:52PM +0800, Haibo Chen wrote:
> The imx7d usdhc is derived from imx6sx, the difference is that
> imx7d support HS400.
> 
> So introduce a new compatible string for imx7d and add HS400
> support for imx7d usdhc.
> 
> Signed-off-by: Haibo Chen <haibo.chen@freescale.com>
> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 66 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 66 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index c6b9f64..b441eed 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -44,6 +44,7 @@
>  #define  ESDHC_MIX_CTRL_EXE_TUNE	(1 << 22)
>  #define  ESDHC_MIX_CTRL_SMPCLK_SEL	(1 << 23)
>  #define  ESDHC_MIX_CTRL_FBCLK_SEL	(1 << 25)
> +#define  ESDHC_MIX_CTRL_HS400_EN	(1 << 26)
>  /* Bits 3 and 6 are not SDHCI standard definitions */
>  #define  ESDHC_MIX_CTRL_SDHCI_MASK	0xb7
>  /* Tuning bits */
> @@ -60,6 +61,16 @@
>  #define  ESDHC_TUNE_CTRL_MIN		0
>  #define  ESDHC_TUNE_CTRL_MAX		((1 << 7) - 1)
>  
> +/* strobe dll register */
> +#define ESDHC_STROBE_DLL_CTRL		0x70
> +#define ESDHC_STROBE_DLL_CTRL_ENABLE	(1 << 0)
> +#define ESDHC_STROBE_DLL_CTRL_RESET	(1 << 1)
> +#define ESDHC_STROBE_DLL_CTRL_SLV_DLY_TARGET_SHIFT	3
> +
> +#define ESDHC_STROBE_DLL_STATUS		0x74
> +#define ESDHC_STROBE_DLL_STS_REF_LOCK	(1 << 1)
> +#define ESDHC_STROBE_DLL_STS_SLV_LOCK	0x1
> +
>  #define ESDHC_TUNING_CTRL		0xcc
>  #define ESDHC_STD_TUNING_EN		(1 << 24)
>  /* NOTE: the minimum valid tuning start tap for mx6sl is 1 */
> @@ -120,6 +131,8 @@
>  #define ESDHC_FLAG_ERR004536		BIT(7)
>  /* The IP supports HS200 mode */
>  #define ESDHC_FLAG_HS200		BIT(8)
> +/* The IP supports HS400 mode */
> +#define ESDHC_FLAG_SUP_HS400		BIT(9)
>  
>  struct esdhc_soc_data {
>  	u32 flags;
> @@ -156,6 +169,12 @@ static struct esdhc_soc_data usdhc_imx6sx_data = {
>  			| ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200,
>  };
>  
> +static struct esdhc_soc_data usdhc_imx7d_data = {
> +	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
> +			| ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200
> +			| ESDHC_FLAG_SUP_HS400,
> +};
> +
>  struct pltfm_imx_data {
>  	u32 scratchpad;
>  	struct pinctrl *pinctrl;
> @@ -199,6 +218,7 @@ static const struct of_device_id imx_esdhc_dt_ids[] = {
>  	{ .compatible = "fsl,imx6sx-usdhc", .data = &usdhc_imx6sx_data, },
>  	{ .compatible = "fsl,imx6sl-usdhc", .data = &usdhc_imx6sl_data, },
>  	{ .compatible = "fsl,imx6q-usdhc", .data = &usdhc_imx6q_data, },
> +	{ .compatible = "fsl,imx7d-usdhc", .data = &usdhc_imx7d_data, },
>  	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, imx_esdhc_dt_ids);
> @@ -274,6 +294,10 @@ static u32 esdhc_readl_le(struct sdhci_host *host, int reg)
>  				val = SDHCI_SUPPORT_DDR50 | SDHCI_SUPPORT_SDR104
>  					| SDHCI_SUPPORT_SDR50
>  					| SDHCI_USE_SDR50_TUNING;
> +
> +			/* imx7d does not have a support_hs400 register, fake one */

You could remove this line.
It's bit, not register and i think no need such comment.

> +			if (imx_data->socdata->flags & ESDHC_FLAG_SUP_HS400)
> +				val |= SDHCI_SUPPORT_HS400;
>  		}
>  	}
>  
> @@ -774,6 +798,7 @@ static int esdhc_change_pinstate(struct sdhci_host *host,
>  		break;
>  	case MMC_TIMING_UHS_SDR104:
>  	case MMC_TIMING_MMC_HS200:
> +	case MMC_TIMING_MMC_HS400:
>  		pinctrl = imx_data->pins_200mhz;
>  		break;
>  	default:
> @@ -784,6 +809,30 @@ static int esdhc_change_pinstate(struct sdhci_host *host,
>  	return pinctrl_select_state(imx_data->pinctrl, pinctrl);
>  }
>  
> +static void esdhc_set_strobe_dll(struct sdhci_host *host)

It would be good if we can add some comments for this function for better
understand.

> +{
> +	u32 v;
> +
> +	/* force a reset on strobe dll */
> +	writel(ESDHC_STROBE_DLL_CTRL_RESET, host->ioaddr + ESDHC_STROBE_DLL_CTRL);
> +	/*
> +	 * enable strobe dll ctrl and adjust the delay target
> +	 * for the uSDHC loopback read clock
> +	 */
> +	v = ESDHC_STROBE_DLL_CTRL_ENABLE |
> +		(7 << ESDHC_STROBE_DLL_CTRL_SLV_DLY_TARGET_SHIFT);
> +	writel(v, host->ioaddr + ESDHC_STROBE_DLL_CTRL);
> +	/* wait 1us to make sure strobe dll status register stable */
> +	udelay(1);
> +	v = readl(host->ioaddr + ESDHC_STROBE_DLL_STATUS);
> +	if (!(v & ESDHC_STROBE_DLL_STS_REF_LOCK))
> +		dev_warn(mmc_dev(host->mmc),
> +			"warning! HS400 strobe DLL status REF not lock!\n");
> +	if (!(v & ESDHC_STROBE_DLL_STS_SLV_LOCK))
> +		dev_warn(mmc_dev(host->mmc),
> +			"warning! HS400 strobe DLL status SLV not lock!\n");
> +}
> +
>  static void esdhc_set_uhs_signaling(struct sdhci_host *host, unsigned timing)
>  {
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> @@ -795,7 +844,13 @@ static void esdhc_set_uhs_signaling(struct sdhci_host *host, unsigned timing)
>  	case MMC_TIMING_UHS_SDR25:
>  	case MMC_TIMING_UHS_SDR50:
>  	case MMC_TIMING_UHS_SDR104:
> +		break;
>  	case MMC_TIMING_MMC_HS200:
> +		/* disable ddr mode and disable HS400 mode */
> +		writel(readl(host->ioaddr + ESDHC_MIX_CTRL) &
> +			~(ESDHC_MIX_CTRL_DDREN | ESDHC_MIX_CTRL_HS400_EN),
> +			host->ioaddr + ESDHC_MIX_CTRL);
> +		imx_data->is_ddr = 0;
>  		break;
>  	case MMC_TIMING_UHS_DDR50:
>  	case MMC_TIMING_MMC_DDR52:
> @@ -813,6 +868,14 @@ static void esdhc_set_uhs_signaling(struct sdhci_host *host, unsigned timing)
>  			writel(v, host->ioaddr + ESDHC_DLL_CTRL);
>  		}
>  		break;
> +	case MMC_TIMING_MMC_HS400:
> +		writel(readl(host->ioaddr + ESDHC_MIX_CTRL) |
> +				ESDHC_MIX_CTRL_DDREN | ESDHC_MIX_CTRL_HS400_EN,
> +				host->ioaddr + ESDHC_MIX_CTRL);
> +		imx_data->is_ddr = 1;
> +		if (host->clock == 200000000)

I can't remember, but could this be a range required by SoC?

> +			esdhc_set_strobe_dll(host);
> +		break;
>  	}
>  
>  	esdhc_change_pinstate(host, timing);
> @@ -1100,6 +1163,9 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
>  	if (imx_data->socdata->flags & ESDHC_FLAG_ERR004536)
>  		host->quirks |= SDHCI_QUIRK_BROKEN_ADMA;
>  
> +	if (imx_data->socdata->flags & ESDHC_FLAG_SUP_HS400)
> +		host->quirks2 |= SDHCI_QUIRK2_CAPS_BIT63_FOR_HS400;
> +
>  	if (of_id)
>  		err = sdhci_esdhc_imx_probe_dt(pdev, host, imx_data);
>  	else
> -- 
> 1.9.1
> 

Regards
Dong Aisheng
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/