mmc: fix division by zero when calculate mmc erase time
PATCH 1/1 mmc: fix division by zero when calculate mmc erase time
Since if clock gating feature is enabled, the clock frequency may be zero when host clock is gated. In such scenario, mmc_set_mmc_erase_timeout() may have a division by zero bug.
So this patch used mmc_host_clk_rate() to fix this.
--- drivers/mmc/core/core.c | 10 ++++++++-- 1 files changed, 8 insertions(+), 2 deletions(-)
drivers/mmc/core/core.c +8 -2
@@ -1201,8 +1201,14 @@ static void mmc_set_mmc_erase_timeout(struct mmc_card *card,* less but not that much less, so fudge it by multiplying by 2. */ timeout_clks <<=1 ; - timeout_us += (timeout_clks *1000 ) / - (card->host->ios.clock /1000 ); + +/* +* at this moment, host controller maybe clock gated, so make +* sure we can get a correct host clock freq. +*/ +if (mmc_host_clk_rate (card->host)) + timeout_us += (timeout_clks *1000 ) / + (mmc_host_clk_rate (card->host) /1000 ); erase_timeout = timeout_us /1000 ;
Signature · 1 line
1.6.6.1
Hi,
Quoted 19 lines · Chuanxiao Dong
Since if clock gating feature is enabled, the clock frequency may be zero when host clock is gated. In such scenario,
mmc_set_mmc_erase_timeout()may have a division by zero bug.So this patch used
mmc_host_clk_rate()to fix this.Signed-off-by Chuanxiao Dong <[email protected]>--- drivers/mmc/core/core.c | 10 ++++++++-- 1 files changed, 8 insertions(+), 2 deletions(-)drivers/mmc/core/core.c +0
-0@@ -1201,8 +1201,14 @@ static void mmc_set_mmc_erase_timeout(struct mmc_card *card,* less but not that much less, so fudge it by multiplying by 2. */ timeout_clks <<=1 ;
drivers/mmc/core/core.c static void mmc_set_mmc_erase_timeout(struct mmc_card *card, · line 1211
Quoted 10 lines
drivers/mmc/core/core.c +8
-2- timeout_us += (timeout_clks *1000 ) / - (card->host->ios.clock /1000 ); + +/* +* at this moment, host controller maybe clock gated, so make +* sure we can get a correct host clock freq. +*/ +if (mmc_host_clk_rate (card->host)) + timeout_us += (timeout_clks *1000 ) / + (mmc_host_clk_rate (card->host) /1000 );
Why don't you just reuse mmc_host_clk_rate()'s result instead of calling it twice?
Cheers,
Marc
Quoted 13 lines
drivers/mmc/core/core.c +0
-0erase_timeout = timeout_us /1000 ;Signature · 7 lines
1.6.6.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Signature · 1 line
Marc Koschewski
Quoted 31 lines
-----Original Message-----
From: Marc Koschewski [mailto:[email protected]]
Sent: Thursday, February 24, 2011 7:38 PM
To: Dong, Chuanxiao
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 1/1]mmc: fix division by zero when calculate mmc erase timeHi,
drivers/mmc/core/core.c static void mmc_set_mmc_erase_timeout(struct
Quoted 16 lines · Chuanxiao Dong
Since if clock gating feature is enabled, the clock frequency may be zero when host clock is gated. In such scenario,
mmc_set_mmc_erase_timeout()may have a division by zero bug.So this patch used
mmc_host_clk_rate()to fix this.Signed-off-by Chuanxiao Dong <[email protected]>--- drivers/mmc/core/core.c | 10 ++++++++-- 1 files changed, 8 insertions(+), 2 deletions(-)drivers/mmc/core/core.c +0
-0@@ -1201,8 +1201,14 @@ static void mmc_set_mmc_erase_timeout(struct
mmc_card*card,
Quoted 15 lines
drivers/mmc/core/core.c static void mmc_set_mmc_erase_timeout(struct mmc_card *card, · line 1211
Quoted 13 lines
drivers/mmc/core/core.c +8
-2* less but not that much less, so fudge it by multiplying by 2. */ timeout_clks <<=1 ; - timeout_us += (timeout_clks *1000 ) / - (card->host->ios.clock /1000 ); + +/* +* at this moment, host controller maybe clock gated, so make +* sure we can get a correct host clock freq. +*/ +if (mmc_host_clk_rate (card->host)) + timeout_us += (timeout_clks *1000 ) / + (mmc_host_clk_rate (card->host) /1000 );Why don't you just reuse mmc_host_clk_rate()'s result instead of calling it twice?
This is a incline function and just return host->ios.clock. Reuse mmc_host_clk_rate()'s result need to add a new variable to save the value.
Thanks
Chuanxiao
Quoted 46 lines · Dong, Chuanxiao
Quoted 31 lines
-----Original Message-----
From: Marc Koschewski [mailto:[email protected]]
Sent: Thursday, February 24, 2011 7:38 PM
To: Dong, Chuanxiao
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 1/1]mmc: fix division by zero when calculate mmc erase timeHi,
drivers/mmc/core/core.c static void mmc_set_mmc_erase_timeout(struct
Quoted 16 lines · Chuanxiao Dong
Since if clock gating feature is enabled, the clock frequency may be zero when host clock is gated. In such scenario,
mmc_set_mmc_erase_timeout()may have a division by zero bug.So this patch used
mmc_host_clk_rate()to fix this.Signed-off-by Chuanxiao Dong <[email protected]>--- drivers/mmc/core/core.c | 10 ++++++++-- 1 files changed, 8 insertions(+), 2 deletions(-)drivers/mmc/core/core.c +0
-0@@ -1201,8 +1201,14 @@ static void mmc_set_mmc_erase_timeout(struct
mmc_card*card,Quoted 15 lines
drivers/mmc/core/core.c static void mmc_set_mmc_erase_timeout(struct mmc_card *card, · line 1211
Quoted 13 lines
drivers/mmc/core/core.c +8
-2* less but not that much less, so fudge it by multiplying by 2. */ timeout_clks <<=1 ; - timeout_us += (timeout_clks *1000 ) / - (card->host->ios.clock /1000 ); + +/* +* at this moment, host controller maybe clock gated, so make +* sure we can get a correct host clock freq. +*/ +if (mmc_host_clk_rate (card->host)) + timeout_us += (timeout_clks *1000 ) / + (mmc_host_clk_rate (card->host) /1000 );Why don't you just reuse mmc_host_clk_rate()'s result instead of calling it twice?
Quoted 1 line
This is a incline function and just return
host->ios.clock. Reuse mmc_host_clk_rate()'s result need to add a new variable to save the value.
It's not inline on trunk and it spinlocks.
drivers/mmc/core/host.c:195
194 */
195 unsigned int mmc_host_clk_rate(struct mmc_host *host)
196 {Cheers,
Marc
Quoted 10 lines
Thanks
Chuanxiao
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Signature · 1 line
Marc Koschewski
Quoted 62 lines
-----Original Message-----
From: Marc Koschewski [mailto:[email protected]]
Sent: Thursday, February 24, 2011 8:23 PM
To: Dong, Chuanxiao
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 1/1]mmc: fix division by zero when calculate mmc erase timeQuoted 9 lines · Dong, Chuanxiao
Quoted 9 lines
-----Original Message-----
From: Marc Koschewski [mailto:[email protected]]
Sent: Thursday, February 24, 2011 7:38 PM
To: Dong, Chuanxiao
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 1/1]mmc: fix division by zero when calculate mmc erasetime
Quoted 7 lines
Quoted 7 lines
Hi,
Quoted 2 lines · Chuanxiao Dong
Since if clock gating feature is enabled, the clock frequency may be zero when host clock is gated. In such scenario,
mmc_set_mmc_erase_timeout()mayhave a
Quoted 15 lines
Quoted 15 lines
drivers/mmc/core/core.c static void mmc_set_mmc_erase_timeout(struct
Quoted 14 lines
division by zero bug.
So this patch used
mmc_host_clk_rate()to fix this.Signed-off-by Chuanxiao Dong <[email protected]>--- drivers/mmc/core/core.c | 10 ++++++++-- 1 files changed, 8 insertions(+), 2 deletions(-)drivers/mmc/core/core.c +0
-0@@ -1201,8 +1201,14 @@ static void mmc_set_mmc_erase_timeout(struct
mmc_card*card,Quoted 15 lines
Quoted 7 lines
Quoted 7 lines
drivers/mmc/core/core.c +2
-2* less but not that much less, so fudge it by multiplying by 2. */ timeout_clks <<=1 ; - timeout_us += (timeout_clks *1000 ) / - (card->host->ios.clock /1000 ); + +/* Quoted 8 lines
drivers/mmc/core/core.c static void mmc_set_mmc_erase_timeout(struct mmc_card *card, · line 1211
Quoted 6 lines
drivers/mmc/core/core.c +6
-0+* at this moment, host controller maybe clock gated, so make +* sure we can get a correct host clock freq. +*/ +if (mmc_host_clk_rate (card->host)) + timeout_us += (timeout_clks *1000 ) / + (mmc_host_clk_rate (card->host) /1000 );Why don't you just reuse mmc_host_clk_rate()'s result instead of calling it
twice?
Quoted 1 line
This is a incline function and just return
host->ios.clock. Reusemmc_host_clk_rate()'s result need to add a new variable to save the value.
Quoted 7 lines
It's not inline on trunk and it spinlocks.
drivers/mmc/core/host.c:195194 */ 195 unsigned int mmc_host_clk_rate(struct mmc_host *host) 196 {
OK. With the clock gating framework enabled... I agree. So, what do you think? Add a new variable is better?
Thanks
Chuanxiao
Quoted 62 lines · Dong, Chuanxiao
Quoted 62 lines
-----Original Message-----
From: Marc Koschewski [mailto:[email protected]]
Sent: Thursday, February 24, 2011 8:23 PM
To: Dong, Chuanxiao
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 1/1]mmc: fix division by zero when calculate mmc erase timeQuoted 9 lines · Dong, Chuanxiao
Quoted 9 lines
-----Original Message-----
From: Marc Koschewski [mailto:[email protected]]
Sent: Thursday, February 24, 2011 7:38 PM
To: Dong, Chuanxiao
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 1/1]mmc: fix division by zero when calculate mmc erasetime
Quoted 7 lines
Quoted 7 lines
Hi,
Quoted 2 lines · Chuanxiao Dong
Since if clock gating feature is enabled, the clock frequency may be zero when host clock is gated. In such scenario,
mmc_set_mmc_erase_timeout()mayhave a
Quoted 15 lines
Quoted 15 lines
drivers/mmc/core/core.c static void mmc_set_mmc_erase_timeout(struct
Quoted 14 lines
division by zero bug.
So this patch used
mmc_host_clk_rate()to fix this.Signed-off-by Chuanxiao Dong <[email protected]>--- drivers/mmc/core/core.c | 10 ++++++++-- 1 files changed, 8 insertions(+), 2 deletions(-)drivers/mmc/core/core.c +0
-0@@ -1201,8 +1201,14 @@ static void mmc_set_mmc_erase_timeout(struct
mmc_card*card,Quoted 15 lines
Quoted 7 lines
Quoted 7 lines
drivers/mmc/core/core.c +2
-2* less but not that much less, so fudge it by multiplying by 2. */ timeout_clks <<=1 ; - timeout_us += (timeout_clks *1000 ) / - (card->host->ios.clock /1000 ); + +/* Quoted 8 lines
drivers/mmc/core/core.c static void mmc_set_mmc_erase_timeout(struct mmc_card *card, · line 1211
Quoted 6 lines
drivers/mmc/core/core.c +6
-0+* at this moment, host controller maybe clock gated, so make +* sure we can get a correct host clock freq. +*/ +if (mmc_host_clk_rate (card->host)) + timeout_us += (timeout_clks *1000 ) / + (mmc_host_clk_rate (card->host) /1000 );Why don't you just reuse mmc_host_clk_rate()'s result instead of calling it
twice?
Quoted 1 line
This is a incline function and just return
host->ios.clock. Reusemmc_host_clk_rate()'s result need to add a new variable to save the value.
Quoted 8 lines
Quoted 7 lines
It's not inline on trunk and it spinlocks.
drivers/mmc/core/host.c:195194 */ 195 unsigned int mmc_host_clk_rate(struct mmc_host *host) 196 {OK. With the clock gating framework enabled... I agree. So, what do you think? Add a new variable is better?
I personally would prefer the variable over the spinlock and function call, yes.
And calling the same method with the same parameters on a line and another time on the next line is generally a bad idea I think. But maybe that's kind of a 'taste', moreover. It just hit my eye when I saw it...
Cheers,
Marc
Quoted 10 lines
Thanks
Chuanxiao
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Signature · 1 line
Marc Koschewski
2011/2/24 Chuanxiao Dong <[email protected]>:
Quoted 7 lines
Since if clock gating feature is enabled, the clock frequency may be zero when host clock is gated. In such scenario,
mmc_set_mmc_erase_timeout()may have a division by zero bug.So this patch used
mmc_host_clk_rate()to fix this.Signed-off-by Chuanxiao Dong <[email protected]>
Actually I think these divide-by zero bugs were there before since clock 0 is perfectly legal, we just didn't trigger them before.
Yours,
Linus Walleij