mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH v1] i.MX: esdhc: optimize set_ios path
@ 2017-11-08 15:33 Oleksij Rempel
  2017-11-09 10:04 ` Sascha Hauer
  0 siblings, 1 reply; 4+ messages in thread
From: Oleksij Rempel @ 2017-11-08 15:33 UTC (permalink / raw)
  To: barebox; +Cc: Oleksij Rempel

this part of code is executed at least 4 times in eMMC probe sequence.
Optimizing it is reducing 20-30 msec of boot time.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/mci/imx-esdhc.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/mci/imx-esdhc.c b/drivers/mci/imx-esdhc.c
index 141d715c90..5c2a3a36e3 100644
--- a/drivers/mci/imx-esdhc.c
+++ b/drivers/mci/imx-esdhc.c
@@ -95,6 +95,7 @@ struct fsl_esdhc_host {
 	struct device_d		*dev;
 	struct clk		*clk;
 	const struct esdhc_soc_data *socdata;
+	u32			last_clock;
 };
 
 #define to_fsl_esdhc(mci)	container_of(mci, struct fsl_esdhc_host, mci)
@@ -408,7 +409,7 @@ esdhc_send_cmd(struct mci_host *mci, struct mci_cmd *cmd, struct mci_data *data)
 
 static void set_sysctl(struct mci_host *mci, u32 clock)
 {
-	int div, pre_div;
+	int div, pre_div, i;
 	struct fsl_esdhc_host *host = to_fsl_esdhc(mci);
 	void __iomem *regs = host->regs;
 	int sdhc_clk = clk_get_rate(host->clk);
@@ -453,13 +454,17 @@ static void set_sysctl(struct mci_host *mci, u32 clock)
 	esdhc_clrsetbits32(regs + SDHCI_CLOCK_CONTROL__TIMEOUT_CONTROL__SOFTWARE_RESET,
 			SYSCTL_CLOCK_MASK, clk);
 
-	wait_on_timeout(10 * MSECOND,
-			!(esdhc_read32(regs + SDHCI_PRESENT_STATE) & PRSSTAT_SDSTB));
+	for (i = 0; i < 1000; i++) {
+		if (esdhc_read32(regs + SDHCI_PRESENT_STATE) & PRSSTAT_SDSTB)
+			break;
+		udelay(10);
+	}
 
 	clk = SYSCTL_PEREN | SYSCTL_CKEN;
 
 	esdhc_setbits32(regs + SDHCI_CLOCK_CONTROL__TIMEOUT_CONTROL__SOFTWARE_RESET,
 			clk);
+	host->last_clock = clock;
 }
 
 static void esdhc_set_ios(struct mci_host *mci, struct mci_ios *ios)
@@ -468,7 +473,8 @@ static void esdhc_set_ios(struct mci_host *mci, struct mci_ios *ios)
 	void __iomem *regs = host->regs;
 
 	/* Set the clock speed */
-	set_sysctl(mci, ios->clock);
+	if (host->last_clock != ios->clock)
+		set_sysctl(mci, ios->clock);
 
 	/* Set the bus width */
 	esdhc_clrbits32(regs + SDHCI_HOST_CONTROL__POWER_CONTROL__BLOCK_GAP_CONTROL,
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v1] i.MX: esdhc: optimize set_ios path
  2017-11-08 15:33 [PATCH v1] i.MX: esdhc: optimize set_ios path Oleksij Rempel
@ 2017-11-09 10:04 ` Sascha Hauer
  2017-11-09 17:08   ` Oleksij Rempel
  0 siblings, 1 reply; 4+ messages in thread
From: Sascha Hauer @ 2017-11-09 10:04 UTC (permalink / raw)
  To: Oleksij Rempel; +Cc: barebox

On Wed, Nov 08, 2017 at 04:33:08PM +0100, Oleksij Rempel wrote:
> this part of code is executed at least 4 times in eMMC probe sequence.
> Optimizing it is reducing 20-30 msec of boot time.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  drivers/mci/imx-esdhc.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mci/imx-esdhc.c b/drivers/mci/imx-esdhc.c
> index 141d715c90..5c2a3a36e3 100644
> --- a/drivers/mci/imx-esdhc.c
> +++ b/drivers/mci/imx-esdhc.c
> @@ -95,6 +95,7 @@ struct fsl_esdhc_host {
>  	struct device_d		*dev;
>  	struct clk		*clk;
>  	const struct esdhc_soc_data *socdata;
> +	u32			last_clock;
>  };
>  
>  #define to_fsl_esdhc(mci)	container_of(mci, struct fsl_esdhc_host, mci)
> @@ -408,7 +409,7 @@ esdhc_send_cmd(struct mci_host *mci, struct mci_cmd *cmd, struct mci_data *data)
>  
>  static void set_sysctl(struct mci_host *mci, u32 clock)
>  {
> -	int div, pre_div;
> +	int div, pre_div, i;
>  	struct fsl_esdhc_host *host = to_fsl_esdhc(mci);
>  	void __iomem *regs = host->regs;
>  	int sdhc_clk = clk_get_rate(host->clk);
> @@ -453,13 +454,17 @@ static void set_sysctl(struct mci_host *mci, u32 clock)
>  	esdhc_clrsetbits32(regs + SDHCI_CLOCK_CONTROL__TIMEOUT_CONTROL__SOFTWARE_RESET,
>  			SYSCTL_CLOCK_MASK, clk);
>  
> -	wait_on_timeout(10 * MSECOND,
> -			!(esdhc_read32(regs + SDHCI_PRESENT_STATE) & PRSSTAT_SDSTB));
> +	for (i = 0; i < 1000; i++) {
> +		if (esdhc_read32(regs + SDHCI_PRESENT_STATE) & PRSSTAT_SDSTB)
> +			break;
> +		udelay(10);
> +	}

I hope this hunk doesn't make anything better. If yes, we have a
problem.

>  
>  	clk = SYSCTL_PEREN | SYSCTL_CKEN;
>  
>  	esdhc_setbits32(regs + SDHCI_CLOCK_CONTROL__TIMEOUT_CONTROL__SOFTWARE_RESET,
>  			clk);
> +	host->last_clock = clock;

Instead of storing the desired clock I would rather store the clock we
actually make from the desired clock. This way we could optimze for
multiple input frequencies that result in the same actual frequency.

Also please rename the variable from last_clock to cur_clock.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v1] i.MX: esdhc: optimize set_ios path
  2017-11-09 10:04 ` Sascha Hauer
@ 2017-11-09 17:08   ` Oleksij Rempel
  2017-11-09 20:13     ` Sascha Hauer
  0 siblings, 1 reply; 4+ messages in thread
From: Oleksij Rempel @ 2017-11-09 17:08 UTC (permalink / raw)
  To: barebox


[-- Attachment #1.1.1: Type: text/plain, Size: 2389 bytes --]

Am 09.11.2017 um 11:04 schrieb Sascha Hauer:
> On Wed, Nov 08, 2017 at 04:33:08PM +0100, Oleksij Rempel wrote:
>> this part of code is executed at least 4 times in eMMC probe sequence.
>> Optimizing it is reducing 20-30 msec of boot time.
>>
>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
>> ---
>>  drivers/mci/imx-esdhc.c | 14 ++++++++++----
>>  1 file changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/mci/imx-esdhc.c b/drivers/mci/imx-esdhc.c
>> index 141d715c90..5c2a3a36e3 100644
>> --- a/drivers/mci/imx-esdhc.c
>> +++ b/drivers/mci/imx-esdhc.c
>> @@ -95,6 +95,7 @@ struct fsl_esdhc_host {
>>  	struct device_d		*dev;
>>  	struct clk		*clk;
>>  	const struct esdhc_soc_data *socdata;
>> +	u32			last_clock;
>>  };
>>  
>>  #define to_fsl_esdhc(mci)	container_of(mci, struct fsl_esdhc_host, mci)
>> @@ -408,7 +409,7 @@ esdhc_send_cmd(struct mci_host *mci, struct mci_cmd *cmd, struct mci_data *data)
>>  
>>  static void set_sysctl(struct mci_host *mci, u32 clock)
>>  {
>> -	int div, pre_div;
>> +	int div, pre_div, i;
>>  	struct fsl_esdhc_host *host = to_fsl_esdhc(mci);
>>  	void __iomem *regs = host->regs;
>>  	int sdhc_clk = clk_get_rate(host->clk);
>> @@ -453,13 +454,17 @@ static void set_sysctl(struct mci_host *mci, u32 clock)
>>  	esdhc_clrsetbits32(regs + SDHCI_CLOCK_CONTROL__TIMEOUT_CONTROL__SOFTWARE_RESET,
>>  			SYSCTL_CLOCK_MASK, clk);
>>  
>> -	wait_on_timeout(10 * MSECOND,
>> -			!(esdhc_read32(regs + SDHCI_PRESENT_STATE) & PRSSTAT_SDSTB));
>> +	for (i = 0; i < 1000; i++) {
>> +		if (esdhc_read32(regs + SDHCI_PRESENT_STATE) & PRSSTAT_SDSTB)
>> +			break;
>> +		udelay(10);
>> +	}
> 
> I hope this hunk doesn't make anything better. If yes, we have a
> problem.

this is the part which need most of the time. Some times it will pass
immediately or will take exactly 10ms.

>>  
>>  	clk = SYSCTL_PEREN | SYSCTL_CKEN;
>>  
>>  	esdhc_setbits32(regs + SDHCI_CLOCK_CONTROL__TIMEOUT_CONTROL__SOFTWARE_RESET,
>>  			clk);
>> +	host->last_clock = clock;
> 
> Instead of storing the desired clock I would rather store the clock we
> actually make from the desired clock. This way we could optimze for
> multiple input frequencies that result in the same actual frequency.
> 
> Also please rename the variable from last_clock to cur_clock.

ok.

-- 
Regards,
Oleksij


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 213 bytes --]

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v1] i.MX: esdhc: optimize set_ios path
  2017-11-09 17:08   ` Oleksij Rempel
@ 2017-11-09 20:13     ` Sascha Hauer
  0 siblings, 0 replies; 4+ messages in thread
From: Sascha Hauer @ 2017-11-09 20:13 UTC (permalink / raw)
  To: Oleksij Rempel; +Cc: barebox

On Thu, Nov 09, 2017 at 06:08:30PM +0100, Oleksij Rempel wrote:
> Am 09.11.2017 um 11:04 schrieb Sascha Hauer:
> > On Wed, Nov 08, 2017 at 04:33:08PM +0100, Oleksij Rempel wrote:
> >> this part of code is executed at least 4 times in eMMC probe sequence.
> >> Optimizing it is reducing 20-30 msec of boot time.
> >>
> >> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> >> ---
> >>  drivers/mci/imx-esdhc.c | 14 ++++++++++----
> >>  1 file changed, 10 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/mci/imx-esdhc.c b/drivers/mci/imx-esdhc.c
> >> index 141d715c90..5c2a3a36e3 100644
> >> --- a/drivers/mci/imx-esdhc.c
> >> +++ b/drivers/mci/imx-esdhc.c
> >> @@ -95,6 +95,7 @@ struct fsl_esdhc_host {
> >>  	struct device_d		*dev;
> >>  	struct clk		*clk;
> >>  	const struct esdhc_soc_data *socdata;
> >> +	u32			last_clock;
> >>  };
> >>  
> >>  #define to_fsl_esdhc(mci)	container_of(mci, struct fsl_esdhc_host, mci)
> >> @@ -408,7 +409,7 @@ esdhc_send_cmd(struct mci_host *mci, struct mci_cmd *cmd, struct mci_data *data)
> >>  
> >>  static void set_sysctl(struct mci_host *mci, u32 clock)
> >>  {
> >> -	int div, pre_div;
> >> +	int div, pre_div, i;
> >>  	struct fsl_esdhc_host *host = to_fsl_esdhc(mci);
> >>  	void __iomem *regs = host->regs;
> >>  	int sdhc_clk = clk_get_rate(host->clk);
> >> @@ -453,13 +454,17 @@ static void set_sysctl(struct mci_host *mci, u32 clock)
> >>  	esdhc_clrsetbits32(regs + SDHCI_CLOCK_CONTROL__TIMEOUT_CONTROL__SOFTWARE_RESET,
> >>  			SYSCTL_CLOCK_MASK, clk);
> >>  
> >> -	wait_on_timeout(10 * MSECOND,
> >> -			!(esdhc_read32(regs + SDHCI_PRESENT_STATE) & PRSSTAT_SDSTB));
> >> +	for (i = 0; i < 1000; i++) {
> >> +		if (esdhc_read32(regs + SDHCI_PRESENT_STATE) & PRSSTAT_SDSTB)
> >> +			break;
> >> +		udelay(10);
> >> +	}
> > 
> > I hope this hunk doesn't make anything better. If yes, we have a
> > problem.
> 
> this is the part which need most of the time. Some times it will pass
> immediately or will take exactly 10ms.

Most likely because the condition is the wrong way round.
wait_on_timeout() waits for the condition to become true. We want to
wait until the PRSSTAT_SDSTB bit is set, so it should be:

wait_on_timeout(10 * MSECOND,
	esdhc_read32(regs + SDHCI_PRESENT_STATE) & PRSSTAT_SDSTB);

/me looks git blame ... Damn, me!

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-11-09 20:13 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-08 15:33 [PATCH v1] i.MX: esdhc: optimize set_ios path Oleksij Rempel
2017-11-09 10:04 ` Sascha Hauer
2017-11-09 17:08   ` Oleksij Rempel
2017-11-09 20:13     ` Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox