* [U-Boot] [PATCH] mmc: dw_mmc: reduce timeout detection cycle @ 2016-07-19 1:38 ` Ziyuan Xu 2016-07-19 2:03 ` Jaehoon Chung 0 siblings, 1 reply; 8+ messages in thread From: Ziyuan Xu @ 2016-07-19 1:38 UTC (permalink / raw) To: u-boot It's no need to speed 10 seconds to wait the mmc device out from busy status. 500 milliseconds enough. Signed-off-by: Ziyuan Xu <xzy.xu@rock-chips.com> --- drivers/mmc/dw_mmc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c index 2cf7bae..790a166 100644 --- a/drivers/mmc/dw_mmc.c +++ b/drivers/mmc/dw_mmc.c @@ -195,7 +195,7 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, ALLOC_CACHE_ALIGN_BUFFER(struct dwmci_idmac, cur_idmac, data ? DIV_ROUND_UP(data->blocks, 8) : 0); int ret = 0, flags = 0, i; - unsigned int timeout = 100000; + unsigned int timeout = 500; u32 retry = 100000; u32 mask, ctrl; ulong start = get_timer(0); -- 1.9.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] mmc: dw_mmc: reduce timeout detection cycle 2016-07-19 1:38 ` [U-Boot] [PATCH] mmc: dw_mmc: reduce timeout detection cycle Ziyuan Xu @ 2016-07-19 2:03 ` Jaehoon Chung 2016-07-19 2:33 ` Ziyuan Xu 0 siblings, 1 reply; 8+ messages in thread From: Jaehoon Chung @ 2016-07-19 2:03 UTC (permalink / raw) To: u-boot Hi Ziyuan, On 07/19/2016 10:38 AM, Ziyuan Xu wrote: > It's no need to speed 10 seconds to wait the mmc device out from busy > status. 500 milliseconds enough. I agreed that 10 seconds is too big.. Could you explain more how you get 500ms and feel enough? Best Regards, Jaehoon Chung > > Signed-off-by: Ziyuan Xu <xzy.xu@rock-chips.com> > --- > > drivers/mmc/dw_mmc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c > index 2cf7bae..790a166 100644 > --- a/drivers/mmc/dw_mmc.c > +++ b/drivers/mmc/dw_mmc.c > @@ -195,7 +195,7 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, > ALLOC_CACHE_ALIGN_BUFFER(struct dwmci_idmac, cur_idmac, > data ? DIV_ROUND_UP(data->blocks, 8) : 0); > int ret = 0, flags = 0, i; > - unsigned int timeout = 100000; > + unsigned int timeout = 500; > u32 retry = 100000; > u32 mask, ctrl; > ulong start = get_timer(0); > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] mmc: dw_mmc: reduce timeout detection cycle 2016-07-19 2:03 ` Jaehoon Chung @ 2016-07-19 2:33 ` Ziyuan Xu 2016-07-19 4:22 ` Jaehoon Chung 0 siblings, 1 reply; 8+ messages in thread From: Ziyuan Xu @ 2016-07-19 2:33 UTC (permalink / raw) To: u-boot Hi Jaehoon, On 2016?07?19? 10:03, Jaehoon Chung wrote: > Hi Ziyuan, > > On 07/19/2016 10:38 AM, Ziyuan Xu wrote: >> It's no need to speed 10 seconds to wait the mmc device out from busy >> status. 500 milliseconds enough. > I agreed that 10 seconds is too big.. > Could you explain more how you get 500ms and feel enough? Ordinarily, there are 3 types of scenarios that the mmc device was busy: 1. The mmc interface didn't initialize (eg. gpio iomux) The device will be busy status until gpio iomux. 2. The last command with data transfer. The maximum value of data timeout is 0xffffff cyles(see dwi databook Timeout Register), and the clock is up to 52MHZ under high speed mode. timeout = 0xffffff * 1/52M = 0.32s 3. voltage switch U-BOOT doesn't support voltage switch. In summary, I think 500 milliseconds is enough. What do you think? > > Best Regards, > Jaehoon Chung > >> Signed-off-by: Ziyuan Xu <xzy.xu@rock-chips.com> >> --- >> >> drivers/mmc/dw_mmc.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c >> index 2cf7bae..790a166 100644 >> --- a/drivers/mmc/dw_mmc.c >> +++ b/drivers/mmc/dw_mmc.c >> @@ -195,7 +195,7 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, >> ALLOC_CACHE_ALIGN_BUFFER(struct dwmci_idmac, cur_idmac, >> data ? DIV_ROUND_UP(data->blocks, 8) : 0); >> int ret = 0, flags = 0, i; >> - unsigned int timeout = 100000; >> + unsigned int timeout = 500; >> u32 retry = 100000; >> u32 mask, ctrl; >> ulong start = get_timer(0); >> > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] mmc: dw_mmc: reduce timeout detection cycle 2016-07-19 2:33 ` Ziyuan Xu @ 2016-07-19 4:22 ` Jaehoon Chung 2016-07-19 7:40 ` Ziyuan Xu 0 siblings, 1 reply; 8+ messages in thread From: Jaehoon Chung @ 2016-07-19 4:22 UTC (permalink / raw) To: u-boot Hi Ziyuan, On 07/19/2016 11:33 AM, Ziyuan Xu wrote: > Hi Jaehoon, > > On 2016?07?19? 10:03, Jaehoon Chung wrote: >> Hi Ziyuan, >> >> On 07/19/2016 10:38 AM, Ziyuan Xu wrote: >>> It's no need to speed 10 seconds to wait the mmc device out from busy >>> status. 500 milliseconds enough. >> I agreed that 10 seconds is too big.. >> Could you explain more how you get 500ms and feel enough? > Ordinarily, there are 3 types of scenarios that the mmc device was busy: > 1. The mmc interface didn't initialize (eg. gpio iomux) > The device will be busy status until gpio iomux. > > 2. The last command with data transfer. > The maximum value of data timeout is 0xffffff cyles(see dwi databook Timeout Register), and the clock is up to 52MHZ under high speed mode. > timeout = 0xffffff * 1/52M = 0.32s > > 3. voltage switch > U-BOOT doesn't support voltage switch. > > In summary, I think 500 milliseconds is enough. What do you think? I think it's not important thing. This is for checking whether card is busy or not before sending command. I think it's not relevant to Timeout register. Just ensure that card is not busy before sending command. And there is no effect for I/O performance, isn't? But 50ms is not bad. :) It's personal preference. Best Regards, Jaehoon Chung > >> >> Best Regards, >> Jaehoon Chung >> >>> Signed-off-by: Ziyuan Xu <xzy.xu@rock-chips.com> >>> --- >>> >>> drivers/mmc/dw_mmc.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c >>> index 2cf7bae..790a166 100644 >>> --- a/drivers/mmc/dw_mmc.c >>> +++ b/drivers/mmc/dw_mmc.c >>> @@ -195,7 +195,7 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, >>> ALLOC_CACHE_ALIGN_BUFFER(struct dwmci_idmac, cur_idmac, >>> data ? DIV_ROUND_UP(data->blocks, 8) : 0); >>> int ret = 0, flags = 0, i; >>> - unsigned int timeout = 100000; >>> + unsigned int timeout = 500; >>> u32 retry = 100000; >>> u32 mask, ctrl; >>> ulong start = get_timer(0); >>> >> >> >> > > > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] mmc: dw_mmc: reduce timeout detection cycle 2016-07-19 4:22 ` Jaehoon Chung @ 2016-07-19 7:40 ` Ziyuan Xu 2016-07-19 7:51 ` Jaehoon Chung 0 siblings, 1 reply; 8+ messages in thread From: Ziyuan Xu @ 2016-07-19 7:40 UTC (permalink / raw) To: u-boot Hi Jaehoon, On 2016?07?19? 12:22, Jaehoon Chung wrote: > Hi Ziyuan, > > On 07/19/2016 11:33 AM, Ziyuan Xu wrote: >> Hi Jaehoon, >> >> On 2016?07?19? 10:03, Jaehoon Chung wrote: >>> Hi Ziyuan, >>> >>> On 07/19/2016 10:38 AM, Ziyuan Xu wrote: >>>> It's no need to speed 10 seconds to wait the mmc device out from busy >>>> status. 500 milliseconds enough. >>> I agreed that 10 seconds is too big.. >>> Could you explain more how you get 500ms and feel enough? >> Ordinarily, there are 3 types of scenarios that the mmc device was busy: >> 1. The mmc interface didn't initialize (eg. gpio iomux) >> The device will be busy status until gpio iomux. >> >> 2. The last command with data transfer. >> The maximum value of data timeout is 0xffffff cyles(see dwi databook Timeout Register), and the clock is up to 52MHZ under high speed mode. >> timeout = 0xffffff * 1/52M = 0.32s >> >> 3. voltage switch >> U-BOOT doesn't support voltage switch. >> >> In summary, I think 500 milliseconds is enough. What do you think? > I think it's not important thing. > > This is for checking whether card is busy or not before sending command. > I think it's not relevant to Timeout register. Just ensure that card is not busy before sending command. > And there is no effect for I/O performance, isn't? Yup, I agree with you. For scenarios 2, I mean that if the last command with data transfer, we will hit data_busy assertion probably. If the mmc device remains in a busy state more than 500ms, I think it may also be busy state after 10s. > > But 50ms is not bad. :) It's personal preference. BTW, the timeout value is 500ms in kernel. > > Best Regards, > Jaehoon Chung > >>> Best Regards, >>> Jaehoon Chung >>> >>>> Signed-off-by: Ziyuan Xu <xzy.xu@rock-chips.com> >>>> --- >>>> >>>> drivers/mmc/dw_mmc.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c >>>> index 2cf7bae..790a166 100644 >>>> --- a/drivers/mmc/dw_mmc.c >>>> +++ b/drivers/mmc/dw_mmc.c >>>> @@ -195,7 +195,7 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, >>>> ALLOC_CACHE_ALIGN_BUFFER(struct dwmci_idmac, cur_idmac, >>>> data ? DIV_ROUND_UP(data->blocks, 8) : 0); >>>> int ret = 0, flags = 0, i; >>>> - unsigned int timeout = 100000; >>>> + unsigned int timeout = 500; >>>> u32 retry = 100000; >>>> u32 mask, ctrl; >>>> ulong start = get_timer(0); >>>> >>> >>> >> >> >> >> > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] mmc: dw_mmc: reduce timeout detection cycle 2016-07-19 7:40 ` Ziyuan Xu @ 2016-07-19 7:51 ` Jaehoon Chung 2016-07-19 8:08 ` Ziyuan Xu 0 siblings, 1 reply; 8+ messages in thread From: Jaehoon Chung @ 2016-07-19 7:51 UTC (permalink / raw) To: u-boot Hi, On 07/19/2016 04:40 PM, Ziyuan Xu wrote: > Hi Jaehoon, > > On 2016?07?19? 12:22, Jaehoon Chung wrote: >> Hi Ziyuan, >> >> On 07/19/2016 11:33 AM, Ziyuan Xu wrote: >>> Hi Jaehoon, >>> >>> On 2016?07?19? 10:03, Jaehoon Chung wrote: >>>> Hi Ziyuan, >>>> >>>> On 07/19/2016 10:38 AM, Ziyuan Xu wrote: >>>>> It's no need to speed 10 seconds to wait the mmc device out from busy >>>>> status. 500 milliseconds enough. >>>> I agreed that 10 seconds is too big.. >>>> Could you explain more how you get 500ms and feel enough? >>> Ordinarily, there are 3 types of scenarios that the mmc device was busy: >>> 1. The mmc interface didn't initialize (eg. gpio iomux) >>> The device will be busy status until gpio iomux. >>> >>> 2. The last command with data transfer. >>> The maximum value of data timeout is 0xffffff cyles(see dwi databook Timeout Register), and the clock is up to 52MHZ under high speed mode. >>> timeout = 0xffffff * 1/52M = 0.32s >>> >>> 3. voltage switch >>> U-BOOT doesn't support voltage switch. >>> >>> In summary, I think 500 milliseconds is enough. What do you think? >> I think it's not important thing. >> >> This is for checking whether card is busy or not before sending command. >> I think it's not relevant to Timeout register. Just ensure that card is not busy before sending command. >> And there is no effect for I/O performance, isn't? > Yup, I agree with you. For scenarios 2, I mean that if the last command with data transfer, we will hit data_busy assertion probably. If the mmc device remains in a busy state more than 500ms, I think it may also be busy state after 10s. >> >> But 50ms is not bad. :) It's personal preference. > BTW, the timeout value is 500ms in kernel. Yep, it looks good to me. :) I have tested your patch with Exynos SoCs. Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com> Tested-by: Jaehoon Chung <jh80.chung@samsung.com> Best Regards, Jaehoon Chung >> >> Best Regards, >> Jaehoon Chung >> >>>> Best Regards, >>>> Jaehoon Chung >>>> >>>>> Signed-off-by: Ziyuan Xu <xzy.xu@rock-chips.com> >>>>> --- >>>>> >>>>> drivers/mmc/dw_mmc.c | 2 +- >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c >>>>> index 2cf7bae..790a166 100644 >>>>> --- a/drivers/mmc/dw_mmc.c >>>>> +++ b/drivers/mmc/dw_mmc.c >>>>> @@ -195,7 +195,7 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, >>>>> ALLOC_CACHE_ALIGN_BUFFER(struct dwmci_idmac, cur_idmac, >>>>> data ? DIV_ROUND_UP(data->blocks, 8) : 0); >>>>> int ret = 0, flags = 0, i; >>>>> - unsigned int timeout = 100000; >>>>> + unsigned int timeout = 500; >>>>> u32 retry = 100000; >>>>> u32 mask, ctrl; >>>>> ulong start = get_timer(0); >>>>> >>>> >>>> >>> >>> >>> >>> >> >> >> > > > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] mmc: dw_mmc: reduce timeout detection cycle 2016-07-19 7:51 ` Jaehoon Chung @ 2016-07-19 8:08 ` Ziyuan Xu 2016-07-25 2:07 ` Simon Glass 0 siblings, 1 reply; 8+ messages in thread From: Ziyuan Xu @ 2016-07-19 8:08 UTC (permalink / raw) To: u-boot Hi Jaehoon, On 2016?07?19? 15:51, Jaehoon Chung wrote: > Hi, > > On 07/19/2016 04:40 PM, Ziyuan Xu wrote: >> Hi Jaehoon, >> >> On 2016?07?19? 12:22, Jaehoon Chung wrote: >>> Hi Ziyuan, >>> >>> On 07/19/2016 11:33 AM, Ziyuan Xu wrote: >>>> Hi Jaehoon, >>>> >>>> On 2016?07?19? 10:03, Jaehoon Chung wrote: >>>>> Hi Ziyuan, >>>>> >>>>> On 07/19/2016 10:38 AM, Ziyuan Xu wrote: >>>>>> It's no need to speed 10 seconds to wait the mmc device out from busy >>>>>> status. 500 milliseconds enough. >>>>> I agreed that 10 seconds is too big.. >>>>> Could you explain more how you get 500ms and feel enough? >>>> Ordinarily, there are 3 types of scenarios that the mmc device was busy: >>>> 1. The mmc interface didn't initialize (eg. gpio iomux) >>>> The device will be busy status until gpio iomux. >>>> >>>> 2. The last command with data transfer. >>>> The maximum value of data timeout is 0xffffff cyles(see dwi databook Timeout Register), and the clock is up to 52MHZ under high speed mode. >>>> timeout = 0xffffff * 1/52M = 0.32s >>>> >>>> 3. voltage switch >>>> U-BOOT doesn't support voltage switch. >>>> >>>> In summary, I think 500 milliseconds is enough. What do you think? >>> I think it's not important thing. >>> >>> This is for checking whether card is busy or not before sending command. >>> I think it's not relevant to Timeout register. Just ensure that card is not busy before sending command. >>> And there is no effect for I/O performance, isn't? >> Yup, I agree with you. For scenarios 2, I mean that if the last command with data transfer, we will hit data_busy assertion probably. If the mmc device remains in a busy state more than 500ms, I think it may also be busy state after 10s. >>> But 50ms is not bad. :) It's personal preference. >> BTW, the timeout value is 500ms in kernel. > Yep, it looks good to me. :) > I have tested your patch with Exynos SoCs. > > Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com> > Tested-by: Jaehoon Chung <jh80.chung@samsung.com> Thanks for your test and review! > > Best Regards, > Jaehoon Chung > >>> Best Regards, >>> Jaehoon Chung >>> >>>>> Best Regards, >>>>> Jaehoon Chung >>>>> >>>>>> Signed-off-by: Ziyuan Xu <xzy.xu@rock-chips.com> >>>>>> --- >>>>>> >>>>>> drivers/mmc/dw_mmc.c | 2 +- >>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c >>>>>> index 2cf7bae..790a166 100644 >>>>>> --- a/drivers/mmc/dw_mmc.c >>>>>> +++ b/drivers/mmc/dw_mmc.c >>>>>> @@ -195,7 +195,7 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, >>>>>> ALLOC_CACHE_ALIGN_BUFFER(struct dwmci_idmac, cur_idmac, >>>>>> data ? DIV_ROUND_UP(data->blocks, 8) : 0); >>>>>> int ret = 0, flags = 0, i; >>>>>> - unsigned int timeout = 100000; >>>>>> + unsigned int timeout = 500; >>>>>> u32 retry = 100000; >>>>>> u32 mask, ctrl; >>>>>> ulong start = get_timer(0); >>>>>> >>>>> >>>> >>>> >>>> >>> >>> >> >> >> >> > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] mmc: dw_mmc: reduce timeout detection cycle 2016-07-19 8:08 ` Ziyuan Xu @ 2016-07-25 2:07 ` Simon Glass 0 siblings, 0 replies; 8+ messages in thread From: Simon Glass @ 2016-07-25 2:07 UTC (permalink / raw) To: u-boot On 19 July 2016 at 02:08, Ziyuan Xu <xzy.xu@rock-chips.com> wrote: > Hi Jaehoon, > > > On 2016?07?19? 15:51, Jaehoon Chung wrote: >> >> Hi, >> >> On 07/19/2016 04:40 PM, Ziyuan Xu wrote: >>> >>> Hi Jaehoon, >>> >>> On 2016?07?19? 12:22, Jaehoon Chung wrote: >>>> >>>> Hi Ziyuan, >>>> >>>> On 07/19/2016 11:33 AM, Ziyuan Xu wrote: >>>>> >>>>> Hi Jaehoon, >>>>> >>>>> On 2016?07?19? 10:03, Jaehoon Chung wrote: >>>>>> >>>>>> Hi Ziyuan, >>>>>> >>>>>> On 07/19/2016 10:38 AM, Ziyuan Xu wrote: >>>>>>> >>>>>>> It's no need to speed 10 seconds to wait the mmc device out from busy >>>>>>> status. 500 milliseconds enough. >>>>>> >>>>>> I agreed that 10 seconds is too big.. >>>>>> Could you explain more how you get 500ms and feel enough? >>>>> >>>>> Ordinarily, there are 3 types of scenarios that the mmc device was >>>>> busy: >>>>> 1. The mmc interface didn't initialize (eg. gpio iomux) >>>>> The device will be busy status until gpio iomux. >>>>> >>>>> 2. The last command with data transfer. >>>>> The maximum value of data timeout is 0xffffff cyles(see dwi databook >>>>> Timeout Register), and the clock is up to 52MHZ under high speed mode. >>>>> timeout = 0xffffff * 1/52M = 0.32s >>>>> >>>>> 3. voltage switch >>>>> U-BOOT doesn't support voltage switch. >>>>> >>>>> In summary, I think 500 milliseconds is enough. What do you think? >>>> >>>> I think it's not important thing. >>>> >>>> This is for checking whether card is busy or not before sending command. >>>> I think it's not relevant to Timeout register. Just ensure that card is >>>> not busy before sending command. >>>> And there is no effect for I/O performance, isn't? >>> >>> Yup, I agree with you. For scenarios 2, I mean that if the last command >>> with data transfer, we will hit data_busy assertion probably. If the mmc >>> device remains in a busy state more than 500ms, I think it may also be busy >>> state after 10s. >>>> >>>> But 50ms is not bad. :) It's personal preference. >>> >>> BTW, the timeout value is 500ms in kernel. >> >> Yep, it looks good to me. :) >> I have tested your patch with Exynos SoCs. >> >> Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com> >> Tested-by: Jaehoon Chung <jh80.chung@samsung.com> > > Thanks for your test and review! > Let's get others to test this. Applied to u-boot-dm, thanks! ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-07-25 2:07 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20160719013855epcas1p473dab263942467b94c8b5f1a89271d5b@epcas1p4.samsung.com>
2016-07-19 1:38 ` [U-Boot] [PATCH] mmc: dw_mmc: reduce timeout detection cycle Ziyuan Xu
2016-07-19 2:03 ` Jaehoon Chung
2016-07-19 2:33 ` Ziyuan Xu
2016-07-19 4:22 ` Jaehoon Chung
2016-07-19 7:40 ` Ziyuan Xu
2016-07-19 7:51 ` Jaehoon Chung
2016-07-19 8:08 ` Ziyuan Xu
2016-07-25 2:07 ` Simon Glass
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox