[PATCH v2 1/2] mmc: sdhci-esdhc-imx: disable clocks before changing frequency

[PATCH v2 1/2] mmc: sdhci-esdhc-imx: disable clocks before changing frequency

[Stefan Agner]

In the uSDHC case (e.g. i.MX 6) clocks only get disabled if frequency
is set to 0. However, it could be that the stack asks for a frequency
change while clocks are on. In that case the function clears the
divider registers (by clearing ESDHC_CLOCK_MASK) while the clock is
enabled! This causes a short period of time where the clock is
undivided (on a i.MX 6DL a clock of 196MHz has been measured).

For older IP variants the driver disables clock by clearing some bits
in ESDHC_SYSTEM_CONTROL.

Make sure to disable card clock before changing frequency for uSDHC
IP variants too.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 85fd5a8b0b6d..acacd8481473 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -708,14 +708,14 @@ static inline void esdhc_pltfm_set_clock(struct sdhci_host *host,
 	int div = 1;
 	u32 temp, val;
 
+	if (esdhc_is_usdhc(imx_data)) {
+		val = readl(host->ioaddr + ESDHC_VENDOR_SPEC);
+		writel(val & ~ESDHC_VENDOR_SPEC_FRC_SDCLK_ON,
+			host->ioaddr + ESDHC_VENDOR_SPEC);
+	}
+
 	if (clock == 0) {
 		host->mmc->actual_clock = 0;
-
-		if (esdhc_is_usdhc(imx_data)) {
-			val = readl(host->ioaddr + ESDHC_VENDOR_SPEC);
-			writel(val & ~ESDHC_VENDOR_SPEC_FRC_SDCLK_ON,
-					host->ioaddr + ESDHC_VENDOR_SPEC);
-		}
 		return;
 	}
 
-- 
2.18.0

[PATCH v2 2/2] mmc: sdhci-esdhc-imx: fix indent

[Stefan Agner]

Fix indent. This also makes disable/enable clock blocks look
alike.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index acacd8481473..aa48f4b2541a 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -761,7 +761,7 @@ static inline void esdhc_pltfm_set_clock(struct sdhci_host *host,
 	if (esdhc_is_usdhc(imx_data)) {
 		val = readl(host->ioaddr + ESDHC_VENDOR_SPEC);
 		writel(val | ESDHC_VENDOR_SPEC_FRC_SDCLK_ON,
-		host->ioaddr + ESDHC_VENDOR_SPEC);
+			host->ioaddr + ESDHC_VENDOR_SPEC);
 	}
 
 	mdelay(1);
-- 
2.18.0

RE: [PATCH v2 2/2] mmc: sdhci-esdhc-imx: fix indent

[A.s. Dong]

> -----Original Message-----
> From: Stefan Agner [mailto:stefan@agner.ch]
> Sent: Thursday, July 12, 2018 4:07 PM
> To: adrian.hunter@intel.com; ulf.hansson@linaro.org
> Cc: Fabio Estevam <fabio.estevam@nxp.com>; Bough Chen
> <haibo.chen@nxp.com>; A.s. Dong <aisheng.dong@nxp.com>; linux-
> mmc@vger.kernel.org; linux-kernel@vger.kernel.org; Stefan Agner
> <stefan@agner.ch>
> Subject: [PATCH v2 2/2] mmc: sdhci-esdhc-imx: fix indent
> 
> Fix indent. This also makes disable/enable clock blocks look alike.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>

Acked-by: Dong Aisheng <aisheng.dong@nxp.com>

Regards
Dong Aisheng

Re: [PATCH v2 2/2] mmc: sdhci-esdhc-imx: fix indent

[Adrian Hunter]

On 12/07/18 11:07, Stefan Agner wrote:
> Fix indent. This also makes disable/enable clock blocks look
> alike.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>

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

> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index acacd8481473..aa48f4b2541a 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -761,7 +761,7 @@ static inline void esdhc_pltfm_set_clock(struct sdhci_host *host,
>  	if (esdhc_is_usdhc(imx_data)) {
>  		val = readl(host->ioaddr + ESDHC_VENDOR_SPEC);
>  		writel(val | ESDHC_VENDOR_SPEC_FRC_SDCLK_ON,
> -		host->ioaddr + ESDHC_VENDOR_SPEC);
> +			host->ioaddr + ESDHC_VENDOR_SPEC);
>  	}
>  
>  	mdelay(1);
> 

Re: [PATCH v2 2/2] mmc: sdhci-esdhc-imx: fix indent

[Ulf Hansson]

On 12 July 2018 at 10:07, Stefan Agner <stefan@agner.ch> wrote:
> Fix indent. This also makes disable/enable clock blocks look
> alike.
>
> Signed-off-by: Stefan Agner <stefan@agner.ch>

Thanks, applied for next!

Kind regards
Uffe

> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index acacd8481473..aa48f4b2541a 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -761,7 +761,7 @@ static inline void esdhc_pltfm_set_clock(struct sdhci_host *host,
>         if (esdhc_is_usdhc(imx_data)) {
>                 val = readl(host->ioaddr + ESDHC_VENDOR_SPEC);
>                 writel(val | ESDHC_VENDOR_SPEC_FRC_SDCLK_ON,
> -               host->ioaddr + ESDHC_VENDOR_SPEC);
> +                       host->ioaddr + ESDHC_VENDOR_SPEC);
>         }
>
>         mdelay(1);
> --
> 2.18.0
>

RE: [PATCH v2 1/2] mmc: sdhci-esdhc-imx: disable clocks before changing frequency

[A.s. Dong]

> -----Original Message-----
> From: Stefan Agner [mailto:stefan@agner.ch]
> Sent: Thursday, July 12, 2018 4:07 PM
> To: adrian.hunter@intel.com; ulf.hansson@linaro.org
> Cc: Fabio Estevam <fabio.estevam@nxp.com>; Bough Chen
> <haibo.chen@nxp.com>; A.s. Dong <aisheng.dong@nxp.com>; linux-
> mmc@vger.kernel.org; linux-kernel@vger.kernel.org; Stefan Agner
> <stefan@agner.ch>
> Subject: [PATCH v2 1/2] mmc: sdhci-esdhc-imx: disable clocks before
> changing frequency
> 
> In the uSDHC case (e.g. i.MX 6) clocks only get disabled if frequency is set to 0.
> However, it could be that the stack asks for a frequency change while clocks
> are on. In that case the function clears the divider registers (by clearing
> ESDHC_CLOCK_MASK) while the clock is enabled! This causes a short period
> of time where the clock is undivided (on a i.MX 6DL a clock of 196MHz has
> been measured).
> 
> For older IP variants the driver disables clock by clearing some bits in
> ESDHC_SYSTEM_CONTROL.
> 
> Make sure to disable card clock before changing frequency for uSDHC IP
> variants too.

Looks ok to me.
Acked-by: Dong Aisheng <aisheng.dong@nxp.com>

Regards
Dong Aisheng

> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-
> esdhc-imx.c
> index 85fd5a8b0b6d..acacd8481473 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -708,14 +708,14 @@ static inline void esdhc_pltfm_set_clock(struct
> sdhci_host *host,
>  	int div = 1;
>  	u32 temp, val;
> 
> +	if (esdhc_is_usdhc(imx_data)) {
> +		val = readl(host->ioaddr + ESDHC_VENDOR_SPEC);
> +		writel(val & ~ESDHC_VENDOR_SPEC_FRC_SDCLK_ON,
> +			host->ioaddr + ESDHC_VENDOR_SPEC);
> +	}
> +
>  	if (clock == 0) {
>  		host->mmc->actual_clock = 0;
> -
> -		if (esdhc_is_usdhc(imx_data)) {
> -			val = readl(host->ioaddr + ESDHC_VENDOR_SPEC);
> -			writel(val & ~ESDHC_VENDOR_SPEC_FRC_SDCLK_ON,
> -					host->ioaddr +
> ESDHC_VENDOR_SPEC);
> -		}
>  		return;
>  	}
> 
> --
> 2.18.0

Re: [PATCH v2 1/2] mmc: sdhci-esdhc-imx: disable clocks before changing frequency

[Adrian Hunter]

On 12/07/18 11:07, Stefan Agner wrote:
> In the uSDHC case (e.g. i.MX 6) clocks only get disabled if frequency
> is set to 0. However, it could be that the stack asks for a frequency
> change while clocks are on. In that case the function clears the
> divider registers (by clearing ESDHC_CLOCK_MASK) while the clock is
> enabled! This causes a short period of time where the clock is
> undivided (on a i.MX 6DL a clock of 196MHz has been measured).
> 
> For older IP variants the driver disables clock by clearing some bits
> in ESDHC_SYSTEM_CONTROL.
> 
> Make sure to disable card clock before changing frequency for uSDHC
> IP variants too.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>

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


> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 85fd5a8b0b6d..acacd8481473 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -708,14 +708,14 @@ static inline void esdhc_pltfm_set_clock(struct sdhci_host *host,
>  	int div = 1;
>  	u32 temp, val;
>  
> +	if (esdhc_is_usdhc(imx_data)) {
> +		val = readl(host->ioaddr + ESDHC_VENDOR_SPEC);
> +		writel(val & ~ESDHC_VENDOR_SPEC_FRC_SDCLK_ON,
> +			host->ioaddr + ESDHC_VENDOR_SPEC);
> +	}
> +
>  	if (clock == 0) {
>  		host->mmc->actual_clock = 0;
> -
> -		if (esdhc_is_usdhc(imx_data)) {
> -			val = readl(host->ioaddr + ESDHC_VENDOR_SPEC);
> -			writel(val & ~ESDHC_VENDOR_SPEC_FRC_SDCLK_ON,
> -					host->ioaddr + ESDHC_VENDOR_SPEC);
> -		}
>  		return;
>  	}
>  
> 

[PATCH v2 1/2] mmc: sdhci-esdhc-imx: disable clocks before changing frequency

[Stefan Agner]

In the uSDHC case (e.g. i.MX 6) clocks only get disabled if frequency
is set to 0. However, it could be that the stack asks for a frequency
change while clocks are on. In that case the function clears the
divider registers (by clearing ESDHC_CLOCK_MASK) while the clock is
enabled! This causes a short period of time where the clock is
undivided (on a i.MX 6DL a clock of 196MHz has been measured).

For older IP variants the driver disables clock by clearing some bits
in ESDHC_SYSTEM_CONTROL.

Make sure to disable card clock before changing frequency for uSDHC
IP variants too.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 85fd5a8b0b6d..acacd8481473 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -708,14 +708,14 @@ static inline void esdhc_pltfm_set_clock(struct sdhci_host *host,
 	int div = 1;
 	u32 temp, val;
 
+	if (esdhc_is_usdhc(imx_data)) {
+		val = readl(host->ioaddr + ESDHC_VENDOR_SPEC);
+		writel(val & ~ESDHC_VENDOR_SPEC_FRC_SDCLK_ON,
+			host->ioaddr + ESDHC_VENDOR_SPEC);
+	}
+
 	if (clock == 0) {
 		host->mmc->actual_clock = 0;
-
-		if (esdhc_is_usdhc(imx_data)) {
-			val = readl(host->ioaddr + ESDHC_VENDOR_SPEC);
-			writel(val & ~ESDHC_VENDOR_SPEC_FRC_SDCLK_ON,
-					host->ioaddr + ESDHC_VENDOR_SPEC);
-		}
 		return;
 	}
 
-- 
2.18.0

Re: [PATCH v2 1/2] mmc: sdhci-esdhc-imx: disable clocks before changing frequency

[Stefan Agner]

On 12.07.2018 10:06, Stefan Agner wrote:
> In the uSDHC case (e.g. i.MX 6) clocks only get disabled if frequency
> is set to 0. However, it could be that the stack asks for a frequency
> change while clocks are on. In that case the function clears the
> divider registers (by clearing ESDHC_CLOCK_MASK) while the clock is
> enabled! This causes a short period of time where the clock is
> undivided (on a i.MX 6DL a clock of 196MHz has been measured).
> 
> For older IP variants the driver disables clock by clearing some bits
> in ESDHC_SYSTEM_CONTROL.
> 
> Make sure to disable card clock before changing frequency for uSDHC
> IP variants too.

Ignore this patch, its a duplicate.

I should not have pressed Ctrl-C while git send-email is working :-)

--
Stefan

> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c
> b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 85fd5a8b0b6d..acacd8481473 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -708,14 +708,14 @@ static inline void esdhc_pltfm_set_clock(struct
> sdhci_host *host,
>  	int div = 1;
>  	u32 temp, val;
>  
> +	if (esdhc_is_usdhc(imx_data)) {
> +		val = readl(host->ioaddr + ESDHC_VENDOR_SPEC);
> +		writel(val & ~ESDHC_VENDOR_SPEC_FRC_SDCLK_ON,
> +			host->ioaddr + ESDHC_VENDOR_SPEC);
> +	}
> +
>  	if (clock == 0) {
>  		host->mmc->actual_clock = 0;
> -
> -		if (esdhc_is_usdhc(imx_data)) {
> -			val = readl(host->ioaddr + ESDHC_VENDOR_SPEC);
> -			writel(val & ~ESDHC_VENDOR_SPEC_FRC_SDCLK_ON,
> -					host->ioaddr + ESDHC_VENDOR_SPEC);
> -		}
>  		return;
>  	}

Re: [PATCH v2 1/2] mmc: sdhci-esdhc-imx: disable clocks before changing frequency

[Ulf Hansson]

On 12 July 2018 at 10:06, Stefan Agner <stefan@agner.ch> wrote:
> In the uSDHC case (e.g. i.MX 6) clocks only get disabled if frequency
> is set to 0. However, it could be that the stack asks for a frequency
> change while clocks are on. In that case the function clears the
> divider registers (by clearing ESDHC_CLOCK_MASK) while the clock is
> enabled! This causes a short period of time where the clock is
> undivided (on a i.MX 6DL a clock of 196MHz has been measured).
>
> For older IP variants the driver disables clock by clearing some bits
> in ESDHC_SYSTEM_CONTROL.
>
> Make sure to disable card clock before changing frequency for uSDHC
> IP variants too.
>
> Signed-off-by: Stefan Agner <stefan@agner.ch>

Thanks, applied for next!

Kind regards
Uffe

> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 85fd5a8b0b6d..acacd8481473 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -708,14 +708,14 @@ static inline void esdhc_pltfm_set_clock(struct sdhci_host *host,
>         int div = 1;
>         u32 temp, val;
>
> +       if (esdhc_is_usdhc(imx_data)) {
> +               val = readl(host->ioaddr + ESDHC_VENDOR_SPEC);
> +               writel(val & ~ESDHC_VENDOR_SPEC_FRC_SDCLK_ON,
> +                       host->ioaddr + ESDHC_VENDOR_SPEC);
> +       }
> +
>         if (clock == 0) {
>                 host->mmc->actual_clock = 0;
> -
> -               if (esdhc_is_usdhc(imx_data)) {
> -                       val = readl(host->ioaddr + ESDHC_VENDOR_SPEC);
> -                       writel(val & ~ESDHC_VENDOR_SPEC_FRC_SDCLK_ON,
> -                                       host->ioaddr + ESDHC_VENDOR_SPEC);
> -               }
>                 return;
>         }
>
> --
> 2.18.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html