[PATCH] mmc: sdhci-tegra: Add support to enumerate in HS400ES mode

[PATCH] mmc: sdhci-tegra: Add support to enumerate in HS400ES mode

[Prathamesh Shete]

When CMD13 is sent after switching to HS400 mode, the bus
is operating at either MMC_HIGH_26_MAX_DTR or MMC_HIGH_52_MAX_DTR.
To meet Tegra SDHCI requirement at HS400 mode, force SDHCI
interface clock to MMC_HS200_MAX_DTR (200 MHz) so that host
controller CAR clock and the interface clock are rate matched.

Signed-off-by: Prathamesh Shete <pshete@nvidia.com>
---
 drivers/mmc/host/sdhci-tegra.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index 387ce9cdbd7c..d800396d1112 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -123,6 +123,8 @@
 					 SDHCI_TRNS_BLK_CNT_EN | \
 					 SDHCI_TRNS_DMA)
 
+static void tegra_sdhci_set_clock(struct sdhci_host *host, unsigned int clock);
+
 struct sdhci_tegra_soc_data {
 	const struct sdhci_pltfm_data *pdata;
 	u64 dma_mask;
@@ -369,6 +371,16 @@ static void tegra_sdhci_hs400_enhanced_strobe(struct mmc_host *mmc,
 
 	sdhci_writel(host, val, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
 
+	/*
+	 * When CMD13 is sent after switching to HS400 mode, the bus
+	 * is operating at either MMC_HIGH_26_MAX_DTR or
+	 * MMC_HIGH_52_MAX_DTR.
+	 * To meet Tegra SDHCI requirement at HS400 mode, force SDHCI
+	 * interface clock to MMC_HS200_MAX_DTR (200 MHz) so that host
+	 * controller CAR clock and the interface clock are rate matched.
+	 */
+	tegra_sdhci_set_clock(host, MMC_HS200_MAX_DTR);
+
 }
 
 static void tegra_sdhci_reset(struct sdhci_host *host, u8 mask)
-- 
2.17.1

Re: [PATCH] mmc: sdhci-tegra: Add support to enumerate in HS400ES mode

[Adrian Hunter]

On 29/11/2021 13:10, Prathamesh Shete wrote:
> When CMD13 is sent after switching to HS400 mode, the bus
> is operating at either MMC_HIGH_26_MAX_DTR or MMC_HIGH_52_MAX_DTR.
> To meet Tegra SDHCI requirement at HS400 mode, force SDHCI
> interface clock to MMC_HS200_MAX_DTR (200 MHz) so that host
> controller CAR clock and the interface clock are rate matched.

This sounds like an issue with or without enhanced strobe?

Also it seems more like a fix than "adding support".
i.e. is this really "mmc: sdhci-tegra: Fix switch to HS400ES mode" ?

> 
> Signed-off-by: Prathamesh Shete <pshete@nvidia.com>
> ---
>  drivers/mmc/host/sdhci-tegra.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
> index 387ce9cdbd7c..d800396d1112 100644
> --- a/drivers/mmc/host/sdhci-tegra.c
> +++ b/drivers/mmc/host/sdhci-tegra.c
> @@ -123,6 +123,8 @@
>  					 SDHCI_TRNS_BLK_CNT_EN | \
>  					 SDHCI_TRNS_DMA)
>  
> +static void tegra_sdhci_set_clock(struct sdhci_host *host, unsigned int clock);

Please move one of the functions so that the forward declaration is not necessary.

> +
>  struct sdhci_tegra_soc_data {
>  	const struct sdhci_pltfm_data *pdata;
>  	u64 dma_mask;
> @@ -369,6 +371,16 @@ static void tegra_sdhci_hs400_enhanced_strobe(struct mmc_host *mmc,
>  
>  	sdhci_writel(host, val, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
>  
> +	/*
> +	 * When CMD13 is sent after switching to HS400 mode, the bus
> +	 * is operating at either MMC_HIGH_26_MAX_DTR or
> +	 * MMC_HIGH_52_MAX_DTR.
> +	 * To meet Tegra SDHCI requirement at HS400 mode, force SDHCI
> +	 * interface clock to MMC_HS200_MAX_DTR (200 MHz) so that host
> +	 * controller CAR clock and the interface clock are rate matched.
> +	 */
> +	tegra_sdhci_set_clock(host, MMC_HS200_MAX_DTR);

This being done irrespective of ios->enhanced_strobe, which doesn't seem
right based on the comment?

> +

Please remove the blank line.

>  }
>  
>  static void tegra_sdhci_reset(struct sdhci_host *host, u8 mask)
> 

[PATCH v2] mmc: sdhci-tegra: Fix switch to HS400ES mode

[Prathamesh Shete]

When CMD13 is sent after switching to HS400ES mode, the bus
is operating at either MMC_HIGH_26_MAX_DTR or MMC_HIGH_52_MAX_DTR.
To meet Tegra SDHCI requirement at HS400ES mode, force SDHCI
interface clock to MMC_HS200_MAX_DTR (200 MHz) so that host
controller CAR clock and the interface clock are rate matched.

Signed-off-by: Prathamesh Shete <pshete@nvidia.com>
---
 drivers/mmc/host/sdhci-tegra.c | 43 ++++++++++++++++++++--------------
 1 file changed, 26 insertions(+), 17 deletions(-)

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index 387ce9cdbd7c..ca261cce9b37 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -354,23 +354,6 @@ static void tegra_sdhci_set_tap(struct sdhci_host *host, unsigned int tap)
 	}
 }
 
-static void tegra_sdhci_hs400_enhanced_strobe(struct mmc_host *mmc,
-					      struct mmc_ios *ios)
-{
-	struct sdhci_host *host = mmc_priv(mmc);
-	u32 val;
-
-	val = sdhci_readl(host, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
-
-	if (ios->enhanced_strobe)
-		val |= SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE;
-	else
-		val &= ~SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE;
-
-	sdhci_writel(host, val, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
-
-}
-
 static void tegra_sdhci_reset(struct sdhci_host *host, u8 mask)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
@@ -791,6 +774,32 @@ static void tegra_sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
 	}
 }
 
+static void tegra_sdhci_hs400_enhanced_strobe(struct mmc_host *mmc,
+					      struct mmc_ios *ios)
+{
+	struct sdhci_host *host = mmc_priv(mmc);
+	u32 val;
+
+	val = sdhci_readl(host, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
+
+	if (ios->enhanced_strobe)
+		val |= SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE;
+	else
+		val &= ~SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE;
+
+	sdhci_writel(host, val, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
+
+	/*
+	 * When CMD13 is sent from mmc_select_hs400es() after
+	 * switching to HS400ES mode, the bus is operating at
+	 * either MMC_HIGH_26_MAX_DTR or MMC_HIGH_52_MAX_DTR.
+	 * To meet Tegra SDHCI requirement at HS400ES mode, force SDHCI
+	 * interface clock to MMC_HS200_MAX_DTR (200 MHz) so that host
+	 * controller CAR clock and the interface clock are rate matched.
+	 */
+	tegra_sdhci_set_clock(host, MMC_HS200_MAX_DTR);
+}
+
 static unsigned int tegra_sdhci_get_max_clock(struct sdhci_host *host)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
-- 
2.17.1

RE: [PATCH] mmc: sdhci-tegra: Add support to enumerate in HS400ES mode

[Prathamesh Shete]

Thanks for review

This issue is specific to HS400ES mode.

Updated and pushed version v2.
Updated commit message and comments to clarify on HS400ES mode.
Also moved the function as suggested.

Answers to some questions inlined.

Thanks
Prathamesh.

-----Original Message-----
From: Adrian Hunter <adrian.hunter@intel.com> 
Sent: Thursday, December 2, 2021 12:10 PM
To: Prathamesh Shete <pshete@nvidia.com>; ulf.hansson@linaro.org; thierry.reding@gmail.com; Jonathan Hunter <jonathanh@nvidia.com>; p.zabel@pengutronix.de; linux-mmc@vger.kernel.org; linux-tegra@vger.kernel.org; linux-kernel@vger.kernel.org
Cc: Aniruddha Tvs Rao <anrao@nvidia.com>
Subject: Re: [PATCH] mmc: sdhci-tegra: Add support to enumerate in HS400ES mode

External email: Use caution opening links or attachments


On 29/11/2021 13:10, Prathamesh Shete wrote:
> When CMD13 is sent after switching to HS400 mode, the bus is operating 
> at either MMC_HIGH_26_MAX_DTR or MMC_HIGH_52_MAX_DTR.
> To meet Tegra SDHCI requirement at HS400 mode, force SDHCI interface 
> clock to MMC_HS200_MAX_DTR (200 MHz) so that host controller CAR clock 
> and the interface clock are rate matched.

This sounds like an issue with or without enhanced strobe?
[P.S]issue is specific to HS400ES mode updated commit message in v2.

Also it seems more like a fix than "adding support".
i.e. is this really "mmc: sdhci-tegra: Fix switch to HS400ES mode" ?
[P.S] yes this is fix ... updated commit subject in version v2.

>
> Signed-off-by: Prathamesh Shete <pshete@nvidia.com>
> ---
>  drivers/mmc/host/sdhci-tegra.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-tegra.c 
> b/drivers/mmc/host/sdhci-tegra.c index 387ce9cdbd7c..d800396d1112 
> 100644
> --- a/drivers/mmc/host/sdhci-tegra.c
> +++ b/drivers/mmc/host/sdhci-tegra.c
> @@ -123,6 +123,8 @@
>                                        SDHCI_TRNS_BLK_CNT_EN | \
>                                        SDHCI_TRNS_DMA)
>
> +static void tegra_sdhci_set_clock(struct sdhci_host *host, unsigned 
> +int clock);

Please move one of the functions so that the forward declaration is not necessary.
[P.S]Done.

> +
>  struct sdhci_tegra_soc_data {
>       const struct sdhci_pltfm_data *pdata;
>       u64 dma_mask;
> @@ -369,6 +371,16 @@ static void 
> tegra_sdhci_hs400_enhanced_strobe(struct mmc_host *mmc,
>
>       sdhci_writel(host, val, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
>
> +     /*
> +      * When CMD13 is sent after switching to HS400 mode, the bus
> +      * is operating at either MMC_HIGH_26_MAX_DTR or
> +      * MMC_HIGH_52_MAX_DTR.
> +      * To meet Tegra SDHCI requirement at HS400 mode, force SDHCI
> +      * interface clock to MMC_HS200_MAX_DTR (200 MHz) so that host
> +      * controller CAR clock and the interface clock are rate matched.
> +      */
> +     tegra_sdhci_set_clock(host, MMC_HS200_MAX_DTR);

This being done irrespective of ios->enhanced_strobe, which doesn't seem right based on the comment?
[P.S]Updated comments.
> +

Please remove the blank line.
[P.S]done
>  }
>
>  static void tegra_sdhci_reset(struct sdhci_host *host, u8 mask)
>

Re: [PATCH v2] mmc: sdhci-tegra: Fix switch to HS400ES mode

[Adrian Hunter]

On 02/12/2021 15:49, Prathamesh Shete wrote:
> When CMD13 is sent after switching to HS400ES mode, the bus
> is operating at either MMC_HIGH_26_MAX_DTR or MMC_HIGH_52_MAX_DTR.
> To meet Tegra SDHCI requirement at HS400ES mode, force SDHCI
> interface clock to MMC_HS200_MAX_DTR (200 MHz) so that host
> controller CAR clock and the interface clock are rate matched.
> 
> Signed-off-by: Prathamesh Shete <pshete@nvidia.com>
> ---
>  drivers/mmc/host/sdhci-tegra.c | 43 ++++++++++++++++++++--------------
>  1 file changed, 26 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
> index 387ce9cdbd7c..ca261cce9b37 100644
> --- a/drivers/mmc/host/sdhci-tegra.c
> +++ b/drivers/mmc/host/sdhci-tegra.c
> @@ -354,23 +354,6 @@ static void tegra_sdhci_set_tap(struct sdhci_host *host, unsigned int tap)
>  	}
>  }
>  
> -static void tegra_sdhci_hs400_enhanced_strobe(struct mmc_host *mmc,
> -					      struct mmc_ios *ios)
> -{
> -	struct sdhci_host *host = mmc_priv(mmc);
> -	u32 val;
> -
> -	val = sdhci_readl(host, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
> -
> -	if (ios->enhanced_strobe)
> -		val |= SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE;
> -	else
> -		val &= ~SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE;
> -
> -	sdhci_writel(host, val, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
> -
> -}
> -
>  static void tegra_sdhci_reset(struct sdhci_host *host, u8 mask)
>  {
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> @@ -791,6 +774,32 @@ static void tegra_sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
>  	}
>  }
>  
> +static void tegra_sdhci_hs400_enhanced_strobe(struct mmc_host *mmc,
> +					      struct mmc_ios *ios)
> +{
> +	struct sdhci_host *host = mmc_priv(mmc);
> +	u32 val;
> +
> +	val = sdhci_readl(host, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
> +
> +	if (ios->enhanced_strobe)
> +		val |= SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE;
> +	else
> +		val &= ~SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE;
> +
> +	sdhci_writel(host, val, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
> +
> +	/*
> +	 * When CMD13 is sent from mmc_select_hs400es() after
> +	 * switching to HS400ES mode, the bus is operating at
> +	 * either MMC_HIGH_26_MAX_DTR or MMC_HIGH_52_MAX_DTR.
> +	 * To meet Tegra SDHCI requirement at HS400ES mode, force SDHCI
> +	 * interface clock to MMC_HS200_MAX_DTR (200 MHz) so that host
> +	 * controller CAR clock and the interface clock are rate matched.

Still doesn't explain why you want to set MMC_HS200_MAX_DTR when
ios->enhanced_strobe is false e.g. mmc_set_initial_state()

> +	 */
> +	tegra_sdhci_set_clock(host, MMC_HS200_MAX_DTR);
> +}
> +
>  static unsigned int tegra_sdhci_get_max_clock(struct sdhci_host *host)
>  {
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> 

[PATCH v3] mmc: sdhci-tegra: Fix switch to HS400ES mode

[Prathamesh Shete]

When CMD13 is sent after switching to HS400ES mode, the bus
is operating at either MMC_HIGH_26_MAX_DTR or MMC_HIGH_52_MAX_DTR.
To meet Tegra SDHCI requirement at HS400ES mode, force SDHCI
interface clock to MMC_HS200_MAX_DTR (200 MHz) so that host
controller CAR clock and the interface clock are rate matched.

Signed-off-by: Prathamesh Shete <pshete@nvidia.com>
---
 drivers/mmc/host/sdhci-tegra.c | 43 ++++++++++++++++++++--------------
 1 file changed, 26 insertions(+), 17 deletions(-)

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index 387ce9cdbd7c..ddaa3d9000f6 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -354,23 +354,6 @@ static void tegra_sdhci_set_tap(struct sdhci_host *host, unsigned int tap)
 	}
 }
 
-static void tegra_sdhci_hs400_enhanced_strobe(struct mmc_host *mmc,
-					      struct mmc_ios *ios)
-{
-	struct sdhci_host *host = mmc_priv(mmc);
-	u32 val;
-
-	val = sdhci_readl(host, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
-
-	if (ios->enhanced_strobe)
-		val |= SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE;
-	else
-		val &= ~SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE;
-
-	sdhci_writel(host, val, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
-
-}
-
 static void tegra_sdhci_reset(struct sdhci_host *host, u8 mask)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
@@ -791,6 +774,32 @@ static void tegra_sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
 	}
 }
 
+static void tegra_sdhci_hs400_enhanced_strobe(struct mmc_host *mmc,
+					      struct mmc_ios *ios)
+{
+	struct sdhci_host *host = mmc_priv(mmc);
+	u32 val;
+
+	val = sdhci_readl(host, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
+
+	if (ios->enhanced_strobe) {
+		val |= SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE;
+	/*
+	 * When CMD13 is sent from mmc_select_hs400es() after
+	 * switching to HS400ES mode, the bus is operating at
+	 * either MMC_HIGH_26_MAX_DTR or MMC_HIGH_52_MAX_DTR.
+	 * To meet Tegra SDHCI requirement at HS400ES mode, force SDHCI
+	 * interface clock to MMC_HS200_MAX_DTR (200 MHz) so that host
+	 * controller CAR clock and the interface clock are rate matched.
+	 */
+	tegra_sdhci_set_clock(host, MMC_HS200_MAX_DTR);
+	} else {
+		val &= ~SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE;
+	}
+
+	sdhci_writel(host, val, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
+}
+
 static unsigned int tegra_sdhci_get_max_clock(struct sdhci_host *host)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
-- 
2.17.1

RE: [PATCH v2] mmc: sdhci-tegra: Fix switch to HS400ES mode

[Prathamesh Shete]

Thanks for review!
Updated the programming sequence and Pushed version v3.

Thanks
Prathamesh.

> -----Original Message-----
> From: Adrian Hunter <adrian.hunter@intel.com>
> Sent: Thursday, December 2, 2021 8:05 PM
> To: Prathamesh Shete <pshete@nvidia.com>; ulf.hansson@linaro.org;
> thierry.reding@gmail.com; Jonathan Hunter <jonathanh@nvidia.com>;
> p.zabel@pengutronix.de; linux-mmc@vger.kernel.org; linux-
> tegra@vger.kernel.org; linux-kernel@vger.kernel.org
> Cc: Aniruddha Tvs Rao <anrao@nvidia.com>; Suresh Mangipudi
> <smangipudi@nvidia.com>
> Subject: Re: [PATCH v2] mmc: sdhci-tegra: Fix switch to HS400ES mode
> 
> External email: Use caution opening links or attachments
> 
> 
> On 02/12/2021 15:49, Prathamesh Shete wrote:
> > When CMD13 is sent after switching to HS400ES mode, the bus is
> > operating at either MMC_HIGH_26_MAX_DTR or MMC_HIGH_52_MAX_DTR.
> > To meet Tegra SDHCI requirement at HS400ES mode, force SDHCI interface
> > clock to MMC_HS200_MAX_DTR (200 MHz) so that host controller CAR clock
> > and the interface clock are rate matched.
> >
> > Signed-off-by: Prathamesh Shete <pshete@nvidia.com>
> > ---
> >  drivers/mmc/host/sdhci-tegra.c | 43
> > ++++++++++++++++++++--------------
> >  1 file changed, 26 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/mmc/host/sdhci-tegra.c
> > b/drivers/mmc/host/sdhci-tegra.c index 387ce9cdbd7c..ca261cce9b37
> > 100644
> > --- a/drivers/mmc/host/sdhci-tegra.c
> > +++ b/drivers/mmc/host/sdhci-tegra.c
> > @@ -354,23 +354,6 @@ static void tegra_sdhci_set_tap(struct sdhci_host
> *host, unsigned int tap)
> >       }
> >  }
> >
> > -static void tegra_sdhci_hs400_enhanced_strobe(struct mmc_host *mmc,
> > -                                           struct mmc_ios *ios)
> > -{
> > -     struct sdhci_host *host = mmc_priv(mmc);
> > -     u32 val;
> > -
> > -     val = sdhci_readl(host, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
> > -
> > -     if (ios->enhanced_strobe)
> > -             val |= SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE;
> > -     else
> > -             val &= ~SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE;
> > -
> > -     sdhci_writel(host, val, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
> > -
> > -}
> > -
> >  static void tegra_sdhci_reset(struct sdhci_host *host, u8 mask)  {
> >       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); @@
> > -791,6 +774,32 @@ static void tegra_sdhci_set_clock(struct sdhci_host *host,
> unsigned int clock)
> >       }
> >  }
> >
> > +static void tegra_sdhci_hs400_enhanced_strobe(struct mmc_host *mmc,
> > +                                           struct mmc_ios *ios) {
> > +     struct sdhci_host *host = mmc_priv(mmc);
> > +     u32 val;
> > +
> > +     val = sdhci_readl(host, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
> > +
> > +     if (ios->enhanced_strobe)
> > +             val |= SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE;
> > +     else
> > +             val &= ~SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE;
> > +
> > +     sdhci_writel(host, val, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
> > +
> > +     /*
> > +      * When CMD13 is sent from mmc_select_hs400es() after
> > +      * switching to HS400ES mode, the bus is operating at
> > +      * either MMC_HIGH_26_MAX_DTR or MMC_HIGH_52_MAX_DTR.
> > +      * To meet Tegra SDHCI requirement at HS400ES mode, force SDHCI
> > +      * interface clock to MMC_HS200_MAX_DTR (200 MHz) so that host
> > +      * controller CAR clock and the interface clock are rate matched.
> 
> Still doesn't explain why you want to set MMC_HS200_MAX_DTR when
> ios->enhanced_strobe is false e.g. mmc_set_initial_state()
That’s a good catch. Updated the code sequence and pushed v3.


> > +      */
> > +     tegra_sdhci_set_clock(host, MMC_HS200_MAX_DTR); }
> > +
> >  static unsigned int tegra_sdhci_get_max_clock(struct sdhci_host
> > *host)  {
> >       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> >

Re: [PATCH v3] mmc: sdhci-tegra: Fix switch to HS400ES mode

[Adrian Hunter]

On 06/12/2021 16:05, Prathamesh Shete wrote:
> When CMD13 is sent after switching to HS400ES mode, the bus
> is operating at either MMC_HIGH_26_MAX_DTR or MMC_HIGH_52_MAX_DTR.
> To meet Tegra SDHCI requirement at HS400ES mode, force SDHCI
> interface clock to MMC_HS200_MAX_DTR (200 MHz) so that host
> controller CAR clock and the interface clock are rate matched.
> 
> Signed-off-by: Prathamesh Shete <pshete@nvidia.com>

One minor comment below otherwise:

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/host/sdhci-tegra.c | 43 ++++++++++++++++++++--------------
>  1 file changed, 26 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
> index 387ce9cdbd7c..ddaa3d9000f6 100644
> --- a/drivers/mmc/host/sdhci-tegra.c
> +++ b/drivers/mmc/host/sdhci-tegra.c
> @@ -354,23 +354,6 @@ static void tegra_sdhci_set_tap(struct sdhci_host *host, unsigned int tap)
>  	}
>  }
>  
> -static void tegra_sdhci_hs400_enhanced_strobe(struct mmc_host *mmc,
> -					      struct mmc_ios *ios)
> -{
> -	struct sdhci_host *host = mmc_priv(mmc);
> -	u32 val;
> -
> -	val = sdhci_readl(host, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
> -
> -	if (ios->enhanced_strobe)
> -		val |= SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE;
> -	else
> -		val &= ~SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE;
> -
> -	sdhci_writel(host, val, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
> -
> -}
> -
>  static void tegra_sdhci_reset(struct sdhci_host *host, u8 mask)
>  {
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> @@ -791,6 +774,32 @@ static void tegra_sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
>  	}
>  }
>  
> +static void tegra_sdhci_hs400_enhanced_strobe(struct mmc_host *mmc,
> +					      struct mmc_ios *ios)
> +{
> +	struct sdhci_host *host = mmc_priv(mmc);
> +	u32 val;
> +
> +	val = sdhci_readl(host, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
> +
> +	if (ios->enhanced_strobe) {
> +		val |= SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE;
> +	/*
> +	 * When CMD13 is sent from mmc_select_hs400es() after
> +	 * switching to HS400ES mode, the bus is operating at
> +	 * either MMC_HIGH_26_MAX_DTR or MMC_HIGH_52_MAX_DTR.
> +	 * To meet Tegra SDHCI requirement at HS400ES mode, force SDHCI
> +	 * interface clock to MMC_HS200_MAX_DTR (200 MHz) so that host
> +	 * controller CAR clock and the interface clock are rate matched.
> +	 */
> +	tegra_sdhci_set_clock(host, MMC_HS200_MAX_DTR);

Comment and line above need indenting

> +	} else {
> +		val &= ~SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE;
> +	}
> +
> +	sdhci_writel(host, val, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
> +}
> +
>  static unsigned int tegra_sdhci_get_max_clock(struct sdhci_host *host)
>  {
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> 

[PATCH v4] mmc: sdhci-tegra: Fix switch to HS400ES mode

[Prathamesh Shete]

When CMD13 is sent after switching to HS400ES mode, the bus
is operating at either MMC_HIGH_26_MAX_DTR or MMC_HIGH_52_MAX_DTR.
To meet Tegra SDHCI requirement at HS400ES mode, force SDHCI
interface clock to MMC_HS200_MAX_DTR (200 MHz) so that host
controller CAR clock and the interface clock are rate matched.

Signed-off-by: Prathamesh Shete <pshete@nvidia.com>
---
 drivers/mmc/host/sdhci-tegra.c | 43 ++++++++++++++++++++--------------
 1 file changed, 26 insertions(+), 17 deletions(-)

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index 387ce9cdbd7c..7be6674eebd5 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -354,23 +354,6 @@ static void tegra_sdhci_set_tap(struct sdhci_host *host, unsigned int tap)
 	}
 }
 
-static void tegra_sdhci_hs400_enhanced_strobe(struct mmc_host *mmc,
-					      struct mmc_ios *ios)
-{
-	struct sdhci_host *host = mmc_priv(mmc);
-	u32 val;
-
-	val = sdhci_readl(host, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
-
-	if (ios->enhanced_strobe)
-		val |= SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE;
-	else
-		val &= ~SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE;
-
-	sdhci_writel(host, val, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
-
-}
-
 static void tegra_sdhci_reset(struct sdhci_host *host, u8 mask)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
@@ -791,6 +774,32 @@ static void tegra_sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
 	}
 }
 
+static void tegra_sdhci_hs400_enhanced_strobe(struct mmc_host *mmc,
+					      struct mmc_ios *ios)
+{
+	struct sdhci_host *host = mmc_priv(mmc);
+	u32 val;
+
+	val = sdhci_readl(host, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
+
+	if (ios->enhanced_strobe) {
+		val |= SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE;
+		/*
+		 * When CMD13 is sent from mmc_select_hs400es() after
+		 * switching to HS400ES mode, the bus is operating at
+		 * either MMC_HIGH_26_MAX_DTR or MMC_HIGH_52_MAX_DTR.
+		 * To meet Tegra SDHCI requirement at HS400ES mode, force SDHCI
+		 * interface clock to MMC_HS200_MAX_DTR (200 MHz) so that host
+		 * controller CAR clock and the interface clock are rate matched.
+		 */
+		tegra_sdhci_set_clock(host, MMC_HS200_MAX_DTR);
+	} else {
+		val &= ~SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE;
+	}
+
+	sdhci_writel(host, val, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
+}
+
 static unsigned int tegra_sdhci_get_max_clock(struct sdhci_host *host)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
-- 
2.17.1

RE: [PATCH v3] mmc: sdhci-tegra: Fix switch to HS400ES mode

[Prathamesh Shete]

Thanks for acknowledging the change.
Updated and pushed version v4 to fix the indentation.

Thanks
Prathamesh.

> -----Original Message-----
> From: Adrian Hunter <adrian.hunter@intel.com>
> Sent: Tuesday, December 14, 2021 11:31 AM
> To: Prathamesh Shete <pshete@nvidia.com>; ulf.hansson@linaro.org;
> thierry.reding@gmail.com; Jonathan Hunter <jonathanh@nvidia.com>;
> p.zabel@pengutronix.de; linux-mmc@vger.kernel.org; linux-
> tegra@vger.kernel.org; linux-kernel@vger.kernel.org
> Cc: Aniruddha Tvs Rao <anrao@nvidia.com>; Suresh Mangipudi
> <smangipudi@nvidia.com>
> Subject: Re: [PATCH v3] mmc: sdhci-tegra: Fix switch to HS400ES mode
> 
> External email: Use caution opening links or attachments
> 
> 
> On 06/12/2021 16:05, Prathamesh Shete wrote:
> > When CMD13 is sent after switching to HS400ES mode, the bus is
> > operating at either MMC_HIGH_26_MAX_DTR or MMC_HIGH_52_MAX_DTR.
> > To meet Tegra SDHCI requirement at HS400ES mode, force SDHCI interface
> > clock to MMC_HS200_MAX_DTR (200 MHz) so that host controller CAR clock
> > and the interface clock are rate matched.
> >
> > Signed-off-by: Prathamesh Shete <pshete@nvidia.com>
> 
> One minor comment below otherwise:
> 
> Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> 
> > ---
> >  drivers/mmc/host/sdhci-tegra.c | 43
> > ++++++++++++++++++++--------------
> >  1 file changed, 26 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/mmc/host/sdhci-tegra.c
> > b/drivers/mmc/host/sdhci-tegra.c index 387ce9cdbd7c..ddaa3d9000f6
> > 100644
> > --- a/drivers/mmc/host/sdhci-tegra.c
> > +++ b/drivers/mmc/host/sdhci-tegra.c
> > @@ -354,23 +354,6 @@ static void tegra_sdhci_set_tap(struct sdhci_host
> *host, unsigned int tap)
> >       }
> >  }
> >
> > -static void tegra_sdhci_hs400_enhanced_strobe(struct mmc_host *mmc,
> > -                                           struct mmc_ios *ios)
> > -{
> > -     struct sdhci_host *host = mmc_priv(mmc);
> > -     u32 val;
> > -
> > -     val = sdhci_readl(host, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
> > -
> > -     if (ios->enhanced_strobe)
> > -             val |= SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE;
> > -     else
> > -             val &= ~SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE;
> > -
> > -     sdhci_writel(host, val, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
> > -
> > -}
> > -
> >  static void tegra_sdhci_reset(struct sdhci_host *host, u8 mask)  {
> >       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); @@
> > -791,6 +774,32 @@ static void tegra_sdhci_set_clock(struct sdhci_host *host,
> unsigned int clock)
> >       }
> >  }
> >
> > +static void tegra_sdhci_hs400_enhanced_strobe(struct mmc_host *mmc,
> > +                                           struct mmc_ios *ios) {
> > +     struct sdhci_host *host = mmc_priv(mmc);
> > +     u32 val;
> > +
> > +     val = sdhci_readl(host, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
> > +
> > +     if (ios->enhanced_strobe) {
> > +             val |= SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE;
> > +     /*
> > +      * When CMD13 is sent from mmc_select_hs400es() after
> > +      * switching to HS400ES mode, the bus is operating at
> > +      * either MMC_HIGH_26_MAX_DTR or MMC_HIGH_52_MAX_DTR.
> > +      * To meet Tegra SDHCI requirement at HS400ES mode, force SDHCI
> > +      * interface clock to MMC_HS200_MAX_DTR (200 MHz) so that host
> > +      * controller CAR clock and the interface clock are rate matched.
> > +      */
> > +     tegra_sdhci_set_clock(host, MMC_HS200_MAX_DTR);
> 
> Comment and line above need indenting
> 
> > +     } else {
> > +             val &= ~SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE;
> > +     }
> > +
> > +     sdhci_writel(host, val, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL); }
> > +
> >  static unsigned int tegra_sdhci_get_max_clock(struct sdhci_host
> > *host)  {
> >       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> >

Re: [PATCH v4] mmc: sdhci-tegra: Fix switch to HS400ES mode

[Adrian Hunter]

On 14/12/2021 13:36, Prathamesh Shete wrote:
> When CMD13 is sent after switching to HS400ES mode, the bus
> is operating at either MMC_HIGH_26_MAX_DTR or MMC_HIGH_52_MAX_DTR.
> To meet Tegra SDHCI requirement at HS400ES mode, force SDHCI
> interface clock to MMC_HS200_MAX_DTR (200 MHz) so that host
> controller CAR clock and the interface clock are rate matched.
> 
> Signed-off-by: Prathamesh Shete <pshete@nvidia.com>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/host/sdhci-tegra.c | 43 ++++++++++++++++++++--------------
>  1 file changed, 26 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
> index 387ce9cdbd7c..7be6674eebd5 100644
> --- a/drivers/mmc/host/sdhci-tegra.c
> +++ b/drivers/mmc/host/sdhci-tegra.c
> @@ -354,23 +354,6 @@ static void tegra_sdhci_set_tap(struct sdhci_host *host, unsigned int tap)
>  	}
>  }
>  
> -static void tegra_sdhci_hs400_enhanced_strobe(struct mmc_host *mmc,
> -					      struct mmc_ios *ios)
> -{
> -	struct sdhci_host *host = mmc_priv(mmc);
> -	u32 val;
> -
> -	val = sdhci_readl(host, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
> -
> -	if (ios->enhanced_strobe)
> -		val |= SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE;
> -	else
> -		val &= ~SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE;
> -
> -	sdhci_writel(host, val, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
> -
> -}
> -
>  static void tegra_sdhci_reset(struct sdhci_host *host, u8 mask)
>  {
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> @@ -791,6 +774,32 @@ static void tegra_sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
>  	}
>  }
>  
> +static void tegra_sdhci_hs400_enhanced_strobe(struct mmc_host *mmc,
> +					      struct mmc_ios *ios)
> +{
> +	struct sdhci_host *host = mmc_priv(mmc);
> +	u32 val;
> +
> +	val = sdhci_readl(host, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
> +
> +	if (ios->enhanced_strobe) {
> +		val |= SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE;
> +		/*
> +		 * When CMD13 is sent from mmc_select_hs400es() after
> +		 * switching to HS400ES mode, the bus is operating at
> +		 * either MMC_HIGH_26_MAX_DTR or MMC_HIGH_52_MAX_DTR.
> +		 * To meet Tegra SDHCI requirement at HS400ES mode, force SDHCI
> +		 * interface clock to MMC_HS200_MAX_DTR (200 MHz) so that host
> +		 * controller CAR clock and the interface clock are rate matched.
> +		 */
> +		tegra_sdhci_set_clock(host, MMC_HS200_MAX_DTR);
> +	} else {
> +		val &= ~SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE;
> +	}
> +
> +	sdhci_writel(host, val, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
> +}
> +
>  static unsigned int tegra_sdhci_get_max_clock(struct sdhci_host *host)
>  {
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> -- 2.17.1
> 

Re: [PATCH v4] mmc: sdhci-tegra: Fix switch to HS400ES mode

[Ulf Hansson]

On Tue, 14 Dec 2021 at 12:36, Prathamesh Shete <pshete@nvidia.com> wrote:
>
> When CMD13 is sent after switching to HS400ES mode, the bus
> is operating at either MMC_HIGH_26_MAX_DTR or MMC_HIGH_52_MAX_DTR.
> To meet Tegra SDHCI requirement at HS400ES mode, force SDHCI
> interface clock to MMC_HS200_MAX_DTR (200 MHz) so that host
> controller CAR clock and the interface clock are rate matched.
>
> Signed-off-by: Prathamesh Shete <pshete@nvidia.com>

Applied for fixes and by adding a fixes and a stable tag, thanks!

Fixes: dfc9700cef77 ("mmc: tegra: Implement HS400 enhanced strobe")

Kind regards
Uffe


> ---
>  drivers/mmc/host/sdhci-tegra.c | 43 ++++++++++++++++++++--------------
>  1 file changed, 26 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
> index 387ce9cdbd7c..7be6674eebd5 100644
> --- a/drivers/mmc/host/sdhci-tegra.c
> +++ b/drivers/mmc/host/sdhci-tegra.c
> @@ -354,23 +354,6 @@ static void tegra_sdhci_set_tap(struct sdhci_host *host, unsigned int tap)
>         }
>  }
>
> -static void tegra_sdhci_hs400_enhanced_strobe(struct mmc_host *mmc,
> -                                             struct mmc_ios *ios)
> -{
> -       struct sdhci_host *host = mmc_priv(mmc);
> -       u32 val;
> -
> -       val = sdhci_readl(host, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
> -
> -       if (ios->enhanced_strobe)
> -               val |= SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE;
> -       else
> -               val &= ~SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE;
> -
> -       sdhci_writel(host, val, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
> -
> -}
> -
>  static void tegra_sdhci_reset(struct sdhci_host *host, u8 mask)
>  {
>         struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> @@ -791,6 +774,32 @@ static void tegra_sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
>         }
>  }
>
> +static void tegra_sdhci_hs400_enhanced_strobe(struct mmc_host *mmc,
> +                                             struct mmc_ios *ios)
> +{
> +       struct sdhci_host *host = mmc_priv(mmc);
> +       u32 val;
> +
> +       val = sdhci_readl(host, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
> +
> +       if (ios->enhanced_strobe) {
> +               val |= SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE;
> +               /*
> +                * When CMD13 is sent from mmc_select_hs400es() after
> +                * switching to HS400ES mode, the bus is operating at
> +                * either MMC_HIGH_26_MAX_DTR or MMC_HIGH_52_MAX_DTR.
> +                * To meet Tegra SDHCI requirement at HS400ES mode, force SDHCI
> +                * interface clock to MMC_HS200_MAX_DTR (200 MHz) so that host
> +                * controller CAR clock and the interface clock are rate matched.
> +                */
> +               tegra_sdhci_set_clock(host, MMC_HS200_MAX_DTR);
> +       } else {
> +               val &= ~SDHCI_TEGRA_SYS_SW_CTRL_ENHANCED_STROBE;
> +       }
> +
> +       sdhci_writel(host, val, SDHCI_TEGRA_VENDOR_SYS_SW_CTRL);
> +}
> +
>  static unsigned int tegra_sdhci_get_max_clock(struct sdhci_host *host)
>  {
>         struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> --
> 2.17.1
>