linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V1] mmc: sdhci: Update the software timeout value for sdhc
@ 2021-06-16  9:25 Shaik Sajida Bhanu
  2021-06-16 12:16 ` Konrad Dybcio
  2021-06-18 15:30 ` Veerabhadrarao Badiganti
  0 siblings, 2 replies; 5+ messages in thread
From: Shaik Sajida Bhanu @ 2021-06-16  9:25 UTC (permalink / raw)
  To: adrian.hunter, ulf.hansson
  Cc: asutoshd, stummala, vbadigan, rampraka, sayalil, sartgarg,
	rnayak, cang, pragalla, nitirawa, linux-mmc, linux-kernel,
	linux-arm-msm, agross, bjorn.andersson, robh+dt,
	Shaik Sajida Bhanu

Whenever SDHC run at clock rate 50MHZ or below, the hardware data
timeout value will be 21.47secs, which is approx. 22secs and we have
a current software timeout value as 10secs. We have to set software
timeout value more than the hardware data timeout value to avioid seeing
the below register dumps.

[  332.953670] mmc2: Timeout waiting for hardware interrupt.
[  332.959608] mmc2: sdhci: ============ SDHCI REGISTER DUMP ===========
[  332.966450] mmc2: sdhci: Sys addr:  0x00000000 | Version:  0x00007202
[  332.973256] mmc2: sdhci: Blk size:  0x00000200 | Blk cnt:  0x00000001
[  332.980054] mmc2: sdhci: Argument:  0x00000000 | Trn mode: 0x00000027
[  332.986864] mmc2: sdhci: Present:   0x01f801f6 | Host ctl: 0x0000001f
[  332.993671] mmc2: sdhci: Power:     0x00000001 | Blk gap:  0x00000000
[  333.000583] mmc2: sdhci: Wake-up:   0x00000000 | Clock:    0x00000007
[  333.007386] mmc2: sdhci: Timeout:   0x0000000e | Int stat: 0x00000000
[  333.014182] mmc2: sdhci: Int enab:  0x03ff100b | Sig enab: 0x03ff100b
[  333.020976] mmc2: sdhci: ACmd stat: 0x00000000 | Slot int: 0x00000000
[  333.027771] mmc2: sdhci: Caps:      0x322dc8b2 | Caps_1:   0x0000808f
[  333.034561] mmc2: sdhci: Cmd:       0x0000183a | Max curr: 0x00000000
[  333.041359] mmc2: sdhci: Resp[0]:   0x00000900 | Resp[1]:  0x00000000
[  333.048157] mmc2: sdhci: Resp[2]:   0x00000000 | Resp[3]:  0x00000000
[  333.054945] mmc2: sdhci: Host ctl2: 0x00000000
[  333.059657] mmc2: sdhci: ADMA Err:  0x00000000 | ADMA Ptr:
0x0000000ffffff218
[  333.067178] mmc2: sdhci_msm: ----------- VENDOR REGISTER DUMP
-----------
[  333.074343] mmc2: sdhci_msm: DLL sts: 0x00000000 | DLL cfg:
0x6000642c | DLL cfg2: 0x0020a000
[  333.083417] mmc2: sdhci_msm: DLL cfg3: 0x00000000 | DLL usr ctl:
0x00000000 | DDR cfg: 0x80040873
[  333.092850] mmc2: sdhci_msm: Vndr func: 0x00008a9c | Vndr func2 :
0xf88218a8 Vndr func3: 0x02626040
[  333.102371] mmc2: sdhci: ============================================

So, set software timeout value more than hardware timeout value.

Signed-off-by: Shaik Sajida Bhanu <sbhanu@codeaurora.org>
---
 drivers/mmc/host/sdhci.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index bf238ad..1386f7d 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1670,7 +1670,14 @@ static bool sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
 	else if (!cmd->data && cmd->busy_timeout > 9000)
 		timeout += DIV_ROUND_UP(cmd->busy_timeout, 1000) * HZ + HZ;
 	else
-		timeout += 10 * HZ;
+	       /*
+		* In some of the conditions hardware data timeout value could be
+		* approx 21.5 seconds and driver is setting software data timeout
+		* value less than the hardware data timeout value and software data
+		* timeout value should be more than the hardware data timeout value.
+		* So, set software data timeout value more than 21.5 sec i.e. 22sec.
+		*/
+		timeout += 22 * HZ;
 	sdhci_mod_timer(host, cmd->mrq, timeout);
 
 	if (host->use_external_dma)
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member 
of Code Aurora Forum, hosted by The Linux Foundation


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

* Re: [PATCH V1] mmc: sdhci: Update the software timeout value for sdhc
  2021-06-16  9:25 [PATCH V1] mmc: sdhci: Update the software timeout value for sdhc Shaik Sajida Bhanu
@ 2021-06-16 12:16 ` Konrad Dybcio
  2021-06-18 15:30 ` Veerabhadrarao Badiganti
  1 sibling, 0 replies; 5+ messages in thread
From: Konrad Dybcio @ 2021-06-16 12:16 UTC (permalink / raw)
  To: Shaik Sajida Bhanu, adrian.hunter, ulf.hansson
  Cc: asutoshd, stummala, vbadigan, rampraka, sayalil, sartgarg,
	rnayak, cang, pragalla, nitirawa, linux-mmc, linux-kernel,
	linux-arm-msm, agross, bjorn.andersson, robh+dt

Hi,


I am not sure which SoC is this change supposed to address, but it does not fix the longstanding issue of a similar kind on msm8996:


[   25.448575] mmc0: Timeout waiting for hardware interrupt.
[   25.464270] mmc0: sdhci: ============ SDHCI REGISTER DUMP ===========
[   25.479768] mmc0: sdhci: Sys addr:  0x00000008 | Version:  0x00004902
[   25.495361] mmc0: sdhci: Blk size:  0x00004200 | Blk cnt:  0x00000006
[   25.510963] mmc0: sdhci: Argument:  0x01cd9f80 | Trn mode: 0x0000003b
[   25.526517] mmc0: sdhci: Present:   0x03f80206 | Host ctl: 0x0000001f
[   25.542178] mmc0: sdhci: Power:     0x0000000d | Blk gap:  0x00000000
[   25.557767] mmc0: sdhci: Wake-up:   0x00000000 | Clock:    0x00000007
[   25.573427] mmc0: sdhci: Timeout:   0x0000000a | Int stat: 0x00000000
[   25.588983] mmc0: sdhci: Int enab:  0x03ff900b | Sig enab: 0x03ff100b
[   25.604645] mmc0: sdhci: ACmd stat: 0x00000000 | Slot int: 0x00000000
[   25.620273] mmc0: sdhci: Caps:      0x322dc8b2 | Caps_1:   0x00008007
[   25.635752] mmc0: sdhci: Cmd:       0x0000123a | Max curr: 0x00000000
[   25.651351] mmc0: sdhci: Resp[0]:   0x00000900 | Resp[1]:  0x5b590000
[   25.666879] mmc0: sdhci: Resp[2]:   0x73677f80 | Resp[3]:  0x00000900
[   25.682496] mmc0: sdhci: Host ctl2: 0x0000000c
[   25.697991] mmc0: sdhci: ADMA Err:  0x00000000 | ADMA Ptr: 0x0000000101cd2200
[   25.713751] mmc0: sdhci_msm: ----------- VENDOR REGISTER DUMP -----------
[   25.729510] mmc0: sdhci_msm: DLL sts: 0x00000000 | DLL cfg:  0x6008642c | DLL cfg2: 0x0020a000
[   25.745564] mmc0: sdhci_msm: DLL cfg3: 0x00000008 | DLL usr ctl:  0x00000008 | DDR cfg: 0x80040873
[   25.761759] mmc0: sdhci_msm: Vndr func: 0x00018a9c | Vndr func2 : 0xfa8018a8 Vndr func3: 0x00024040
[   25.778080] mmc0: sdhci: ============================================
[   25.894360] mmc0: Reset 0x4 never completed.
[   25.910719] mmc0: sdhci: ============ SDHCI REGISTER DUMP ===========
[   25.927143] mmc0: sdhci: Sys addr:  0x00000008 | Version:  0x00004902
[   25.943717] mmc0: sdhci: Blk size:  0x00004200 | Blk cnt:  0x00000006
[   25.960225] mmc0: sdhci: Argument:  0x01cd9f80 | Trn mode: 0x0000003b
[   25.976845] mmc0: sdhci: Present:   0x03f80206 | Host ctl: 0x0000001f
[   25.993490] mmc0: sdhci: Power:     0x0000000d | Blk gap:  0x00000000
[   26.010019] mmc0: sdhci: Wake-up:   0x00000000 | Clock:    0x00000007
[   26.026693] mmc0: sdhci: Timeout:   0x0000000a | Int stat: 0x00000000
[   26.043262] mmc0: sdhci: Int enab:  0x03ff900b | Sig enab: 0x03ff100b
[   26.059955] mmc0: sdhci: ACmd stat: 0x00000000 | Slot int: 0x00000000
[   26.076546] mmc0: sdhci: Caps:      0x322dc8b2 | Caps_1:   0x00008007
[   26.093229] mmc0: sdhci: Cmd:       0x0000123a | Max curr: 0x00000000
[   26.109783] mmc0: sdhci: Resp[0]:   0x00000000 | Resp[1]:  0x00000000
[   26.126074] mmc0: sdhci: Resp[2]:   0x00000000 | Resp[3]:  0x00000000
[   26.142271] mmc0: sdhci: Host ctl2: 0x0000000c
[   26.158161] mmc0: sdhci: ADMA Err:  0x00000000 | ADMA Ptr: 0x0000000101cd2200
[   26.174287] mmc0: sdhci_msm: ----------- VENDOR REGISTER DUMP -----------
[   26.190295] mmc0: sdhci_msm: DLL sts: 0x00000000 | DLL cfg:  0x6008642c | DLL cfg2: 0x0020a000
[   26.206518] mmc0: sdhci_msm: DLL cfg3: 0x00000008 | DLL usr ctl:  0x00000008 | DDR cfg: 0x80040873
[   26.222916] mmc0: sdhci_msm: Vndr func: 0x00018a9c | Vndr func2 : 0xfa8018a8 Vndr func3: 0x00024040
[   26.239484] mmc0: sdhci: ============================================


Konrad


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

* Re: [PATCH V1] mmc: sdhci: Update the software timeout value for sdhc
  2021-06-16  9:25 [PATCH V1] mmc: sdhci: Update the software timeout value for sdhc Shaik Sajida Bhanu
  2021-06-16 12:16 ` Konrad Dybcio
@ 2021-06-18 15:30 ` Veerabhadrarao Badiganti
  2021-06-18 22:49   ` Doug Anderson
  1 sibling, 1 reply; 5+ messages in thread
From: Veerabhadrarao Badiganti @ 2021-06-18 15:30 UTC (permalink / raw)
  To: Shaik Sajida Bhanu, adrian.hunter, ulf.hansson
  Cc: asutoshd, stummala, rampraka, sayalil, sartgarg, rnayak, cang,
	pragalla, nitirawa, linux-mmc, linux-kernel, linux-arm-msm,
	agross, bjorn.andersson, robh+dt


On 6/16/2021 2:55 PM, Shaik Sajida Bhanu wrote:
> Whenever SDHC run at clock rate 50MHZ or below, the hardware data
> timeout value will be 21.47secs, which is approx. 22secs and we have
> a current software timeout value as 10secs. We have to set software
> timeout value more than the hardware data timeout value to avioid seeing
> the below register dumps.
>
> [  332.953670] mmc2: Timeout waiting for hardware interrupt.
> [  332.959608] mmc2: sdhci: ============ SDHCI REGISTER DUMP ===========
> [  332.966450] mmc2: sdhci: Sys addr:  0x00000000 | Version:  0x00007202
> [  332.973256] mmc2: sdhci: Blk size:  0x00000200 | Blk cnt:  0x00000001
> [  332.980054] mmc2: sdhci: Argument:  0x00000000 | Trn mode: 0x00000027
> [  332.986864] mmc2: sdhci: Present:   0x01f801f6 | Host ctl: 0x0000001f
> [  332.993671] mmc2: sdhci: Power:     0x00000001 | Blk gap:  0x00000000
> [  333.000583] mmc2: sdhci: Wake-up:   0x00000000 | Clock:    0x00000007
> [  333.007386] mmc2: sdhci: Timeout:   0x0000000e | Int stat: 0x00000000
> [  333.014182] mmc2: sdhci: Int enab:  0x03ff100b | Sig enab: 0x03ff100b
> [  333.020976] mmc2: sdhci: ACmd stat: 0x00000000 | Slot int: 0x00000000
> [  333.027771] mmc2: sdhci: Caps:      0x322dc8b2 | Caps_1:   0x0000808f
> [  333.034561] mmc2: sdhci: Cmd:       0x0000183a | Max curr: 0x00000000
> [  333.041359] mmc2: sdhci: Resp[0]:   0x00000900 | Resp[1]:  0x00000000
> [  333.048157] mmc2: sdhci: Resp[2]:   0x00000000 | Resp[3]:  0x00000000
> [  333.054945] mmc2: sdhci: Host ctl2: 0x00000000
> [  333.059657] mmc2: sdhci: ADMA Err:  0x00000000 | ADMA Ptr:
> 0x0000000ffffff218
> [  333.067178] mmc2: sdhci_msm: ----------- VENDOR REGISTER DUMP
> -----------
> [  333.074343] mmc2: sdhci_msm: DLL sts: 0x00000000 | DLL cfg:
> 0x6000642c | DLL cfg2: 0x0020a000
> [  333.083417] mmc2: sdhci_msm: DLL cfg3: 0x00000000 | DLL usr ctl:
> 0x00000000 | DDR cfg: 0x80040873
> [  333.092850] mmc2: sdhci_msm: Vndr func: 0x00008a9c | Vndr func2 :
> 0xf88218a8 Vndr func3: 0x02626040
> [  333.102371] mmc2: sdhci: ============================================
>
> So, set software timeout value more than hardware timeout value.
>
> Signed-off-by: Shaik Sajida Bhanu <sbhanu@codeaurora.org>
> ---
>   drivers/mmc/host/sdhci.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index bf238ad..1386f7d 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1670,7 +1670,14 @@ static bool sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
>   	else if (!cmd->data && cmd->busy_timeout > 9000)
>   		timeout += DIV_ROUND_UP(cmd->busy_timeout, 1000) * HZ + HZ;
>   	else
> -		timeout += 10 * HZ;
> +	       /*
> +		* In some of the conditions hardware data timeout value could be
> +		* approx 21.5 seconds and driver is setting software data timeout
> +		* value less than the hardware data timeout value and software data
> +		* timeout value should be more than the hardware data timeout value.
> +		* So, set software data timeout value more than 21.5 sec i.e. 22sec.
> +		*/
> +		timeout += 22 * HZ;

This timeout is qcom SDHC specific.
I think right way is to, define your own set_timeout op and update 
host->data_timeout
in that as per qcom SDHC requirements.

>   	sdhci_mod_timer(host, cmd->mrq, timeout);
>   
>   	if (host->use_external_dma)

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

* Re: [PATCH V1] mmc: sdhci: Update the software timeout value for sdhc
  2021-06-18 15:30 ` Veerabhadrarao Badiganti
@ 2021-06-18 22:49   ` Doug Anderson
  2021-06-21  9:16     ` Adrian Hunter
  0 siblings, 1 reply; 5+ messages in thread
From: Doug Anderson @ 2021-06-18 22:49 UTC (permalink / raw)
  To: Veerabhadrarao Badiganti
  Cc: Shaik Sajida Bhanu, Adrian Hunter, Ulf Hansson, Asutosh Das,
	Sahitya Tummala, Ram Prakash Gupta, Sayali Lokhande, sartgarg,
	Rajendra Nayak, cang, pragalla, nitirawa, Linux MMC List, LKML,
	linux-arm-msm, Andy Gross, Bjorn Andersson, Rob Herring

Hi,

On Fri, Jun 18, 2021 at 8:31 AM Veerabhadrarao Badiganti
<vbadigan@codeaurora.org> wrote:
>
>
> On 6/16/2021 2:55 PM, Shaik Sajida Bhanu wrote:
> > Whenever SDHC run at clock rate 50MHZ or below, the hardware data
> > timeout value will be 21.47secs, which is approx. 22secs and we have
> > a current software timeout value as 10secs. We have to set software
> > timeout value more than the hardware data timeout value to avioid seeing
> > the below register dumps.
> >
> > [  332.953670] mmc2: Timeout waiting for hardware interrupt.
> > [  332.959608] mmc2: sdhci: ============ SDHCI REGISTER DUMP ===========
> > [  332.966450] mmc2: sdhci: Sys addr:  0x00000000 | Version:  0x00007202
> > [  332.973256] mmc2: sdhci: Blk size:  0x00000200 | Blk cnt:  0x00000001
> > [  332.980054] mmc2: sdhci: Argument:  0x00000000 | Trn mode: 0x00000027
> > [  332.986864] mmc2: sdhci: Present:   0x01f801f6 | Host ctl: 0x0000001f
> > [  332.993671] mmc2: sdhci: Power:     0x00000001 | Blk gap:  0x00000000
> > [  333.000583] mmc2: sdhci: Wake-up:   0x00000000 | Clock:    0x00000007
> > [  333.007386] mmc2: sdhci: Timeout:   0x0000000e | Int stat: 0x00000000
> > [  333.014182] mmc2: sdhci: Int enab:  0x03ff100b | Sig enab: 0x03ff100b
> > [  333.020976] mmc2: sdhci: ACmd stat: 0x00000000 | Slot int: 0x00000000
> > [  333.027771] mmc2: sdhci: Caps:      0x322dc8b2 | Caps_1:   0x0000808f
> > [  333.034561] mmc2: sdhci: Cmd:       0x0000183a | Max curr: 0x00000000
> > [  333.041359] mmc2: sdhci: Resp[0]:   0x00000900 | Resp[1]:  0x00000000
> > [  333.048157] mmc2: sdhci: Resp[2]:   0x00000000 | Resp[3]:  0x00000000
> > [  333.054945] mmc2: sdhci: Host ctl2: 0x00000000
> > [  333.059657] mmc2: sdhci: ADMA Err:  0x00000000 | ADMA Ptr:
> > 0x0000000ffffff218
> > [  333.067178] mmc2: sdhci_msm: ----------- VENDOR REGISTER DUMP
> > -----------
> > [  333.074343] mmc2: sdhci_msm: DLL sts: 0x00000000 | DLL cfg:
> > 0x6000642c | DLL cfg2: 0x0020a000
> > [  333.083417] mmc2: sdhci_msm: DLL cfg3: 0x00000000 | DLL usr ctl:
> > 0x00000000 | DDR cfg: 0x80040873
> > [  333.092850] mmc2: sdhci_msm: Vndr func: 0x00008a9c | Vndr func2 :
> > 0xf88218a8 Vndr func3: 0x02626040
> > [  333.102371] mmc2: sdhci: ============================================
> >
> > So, set software timeout value more than hardware timeout value.
> >
> > Signed-off-by: Shaik Sajida Bhanu <sbhanu@codeaurora.org>
> > ---
> >   drivers/mmc/host/sdhci.c | 9 ++++++++-
> >   1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> > index bf238ad..1386f7d 100644
> > --- a/drivers/mmc/host/sdhci.c
> > +++ b/drivers/mmc/host/sdhci.c
> > @@ -1670,7 +1670,14 @@ static bool sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
> >       else if (!cmd->data && cmd->busy_timeout > 9000)
> >               timeout += DIV_ROUND_UP(cmd->busy_timeout, 1000) * HZ + HZ;
> >       else
> > -             timeout += 10 * HZ;
> > +            /*
> > +             * In some of the conditions hardware data timeout value could be
> > +             * approx 21.5 seconds and driver is setting software data timeout
> > +             * value less than the hardware data timeout value and software data
> > +             * timeout value should be more than the hardware data timeout value.
> > +             * So, set software data timeout value more than 21.5 sec i.e. 22sec.
> > +             */
> > +             timeout += 22 * HZ;
>
> This timeout is qcom SDHC specific.
> I think right way is to, define your own set_timeout op and update
> host->data_timeout
> in that as per qcom SDHC requirements.

It is? Off-thread Shaik was indicating that the problem had to do with
the inaccuracy of the "SDHCI_TIMEOUT_CONTROL" register. That seems to
be in the common SDHCI code. Specifically looking at
sdhci_calc_timeout() it can be seen that the possible hardware values
for the timeout double each time, so if you need a hardware timeout
that's slightly higher than one of the possible values you might end
up with a hardware timeout that's almost double what you want.

Assuming that the problem actually is with the inaccuracy of
SDHCI_TIMEOUT_CONTROL (I didn't walk through and validate), it
actually seems like we should generally be doubling the value we were
programming for the software timeout (in other words, not just ones
that are <= 9 seconds). I haven't done all the math, but I presume the
reason that we need 22 instead of 20 is some type of extra fudge
factor somewhere?

Maybe the only reason that Qualcomm hits this is due to the PLL that's
sourcing the SDHCI controller at a non-standard rate?

I suppose another reason maybe why people aren't hitting it is just
the random chance of what rate the integer overflow in
mmc_set_data_timeout() leaves you at? I pointed this out to Shiak and
was hoping a patch would be included for that, but I can always try to
write one later if not. To be concrete, I have
card->csd.taac_ns=5000000 and card->csd.r2w_factor=5. Multiplying
things out (and accounting for mult=100 for SD cards), I end up with a
timeout_ns of 0x3b9aca000 (16 seconds) which doesn't fit in the 32-bit
data->timeout_ns earlier. The truncation was making it look like the
card requested a max timeout of 3,115,098,112 ns = ~3.1 seconds.

-Doug

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

* Re: [PATCH V1] mmc: sdhci: Update the software timeout value for sdhc
  2021-06-18 22:49   ` Doug Anderson
@ 2021-06-21  9:16     ` Adrian Hunter
  0 siblings, 0 replies; 5+ messages in thread
From: Adrian Hunter @ 2021-06-21  9:16 UTC (permalink / raw)
  To: Doug Anderson, Veerabhadrarao Badiganti
  Cc: Shaik Sajida Bhanu, Ulf Hansson, Asutosh Das, Sahitya Tummala,
	Ram Prakash Gupta, Sayali Lokhande, sartgarg, Rajendra Nayak,
	cang, pragalla, nitirawa, Linux MMC List, LKML, linux-arm-msm,
	Andy Gross, Bjorn Andersson, Rob Herring

On 19/06/21 1:49 am, Doug Anderson wrote:
> Hi,
> 
> On Fri, Jun 18, 2021 at 8:31 AM Veerabhadrarao Badiganti
> <vbadigan@codeaurora.org> wrote:
>>
>>
>> On 6/16/2021 2:55 PM, Shaik Sajida Bhanu wrote:
>>> Whenever SDHC run at clock rate 50MHZ or below, the hardware data
>>> timeout value will be 21.47secs, which is approx. 22secs and we have
>>> a current software timeout value as 10secs. We have to set software
>>> timeout value more than the hardware data timeout value to avioid seeing
>>> the below register dumps.
>>>
>>> [  332.953670] mmc2: Timeout waiting for hardware interrupt.
>>> [  332.959608] mmc2: sdhci: ============ SDHCI REGISTER DUMP ===========
>>> [  332.966450] mmc2: sdhci: Sys addr:  0x00000000 | Version:  0x00007202
>>> [  332.973256] mmc2: sdhci: Blk size:  0x00000200 | Blk cnt:  0x00000001
>>> [  332.980054] mmc2: sdhci: Argument:  0x00000000 | Trn mode: 0x00000027
>>> [  332.986864] mmc2: sdhci: Present:   0x01f801f6 | Host ctl: 0x0000001f
>>> [  332.993671] mmc2: sdhci: Power:     0x00000001 | Blk gap:  0x00000000
>>> [  333.000583] mmc2: sdhci: Wake-up:   0x00000000 | Clock:    0x00000007
>>> [  333.007386] mmc2: sdhci: Timeout:   0x0000000e | Int stat: 0x00000000
>>> [  333.014182] mmc2: sdhci: Int enab:  0x03ff100b | Sig enab: 0x03ff100b
>>> [  333.020976] mmc2: sdhci: ACmd stat: 0x00000000 | Slot int: 0x00000000
>>> [  333.027771] mmc2: sdhci: Caps:      0x322dc8b2 | Caps_1:   0x0000808f
>>> [  333.034561] mmc2: sdhci: Cmd:       0x0000183a | Max curr: 0x00000000
>>> [  333.041359] mmc2: sdhci: Resp[0]:   0x00000900 | Resp[1]:  0x00000000
>>> [  333.048157] mmc2: sdhci: Resp[2]:   0x00000000 | Resp[3]:  0x00000000
>>> [  333.054945] mmc2: sdhci: Host ctl2: 0x00000000
>>> [  333.059657] mmc2: sdhci: ADMA Err:  0x00000000 | ADMA Ptr:
>>> 0x0000000ffffff218
>>> [  333.067178] mmc2: sdhci_msm: ----------- VENDOR REGISTER DUMP
>>> -----------
>>> [  333.074343] mmc2: sdhci_msm: DLL sts: 0x00000000 | DLL cfg:
>>> 0x6000642c | DLL cfg2: 0x0020a000
>>> [  333.083417] mmc2: sdhci_msm: DLL cfg3: 0x00000000 | DLL usr ctl:
>>> 0x00000000 | DDR cfg: 0x80040873
>>> [  333.092850] mmc2: sdhci_msm: Vndr func: 0x00008a9c | Vndr func2 :
>>> 0xf88218a8 Vndr func3: 0x02626040
>>> [  333.102371] mmc2: sdhci: ============================================
>>>
>>> So, set software timeout value more than hardware timeout value.
>>>
>>> Signed-off-by: Shaik Sajida Bhanu <sbhanu@codeaurora.org>
>>> ---
>>>   drivers/mmc/host/sdhci.c | 9 ++++++++-
>>>   1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>> index bf238ad..1386f7d 100644
>>> --- a/drivers/mmc/host/sdhci.c
>>> +++ b/drivers/mmc/host/sdhci.c
>>> @@ -1670,7 +1670,14 @@ static bool sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
>>>       else if (!cmd->data && cmd->busy_timeout > 9000)
>>>               timeout += DIV_ROUND_UP(cmd->busy_timeout, 1000) * HZ + HZ;
>>>       else
>>> -             timeout += 10 * HZ;
>>> +            /*
>>> +             * In some of the conditions hardware data timeout value could be
>>> +             * approx 21.5 seconds and driver is setting software data timeout
>>> +             * value less than the hardware data timeout value and software data
>>> +             * timeout value should be more than the hardware data timeout value.
>>> +             * So, set software data timeout value more than 21.5 sec i.e. 22sec.
>>> +             */
>>> +             timeout += 22 * HZ;
>>
>> This timeout is qcom SDHC specific.
>> I think right way is to, define your own set_timeout op and update
>> host->data_timeout
>> in that as per qcom SDHC requirements.
> 
> It is? Off-thread Shaik was indicating that the problem had to do with
> the inaccuracy of the "SDHCI_TIMEOUT_CONTROL" register. That seems to
> be in the common SDHCI code. Specifically looking at
> sdhci_calc_timeout() it can be seen that the possible hardware values
> for the timeout double each time, so if you need a hardware timeout
> that's slightly higher than one of the possible values you might end
> up with a hardware timeout that's almost double what you want.
> 
> Assuming that the problem actually is with the inaccuracy of
> SDHCI_TIMEOUT_CONTROL (I didn't walk through and validate), it
> actually seems like we should generally be doubling the value we were
> programming for the software timeout (in other words, not just ones
> that are <= 9 seconds). I haven't done all the math, but I presume the
> reason that we need 22 instead of 20 is some type of extra fudge
> factor somewhere?
> 
> Maybe the only reason that Qualcomm hits this is due to the PLL that's
> sourcing the SDHCI controller at a non-standard rate?
> 
> I suppose another reason maybe why people aren't hitting it is just
> the random chance of what rate the integer overflow in
> mmc_set_data_timeout() leaves you at? I pointed this out to Shiak and
> was hoping a patch would be included for that, but I can always try to
> write one later if not. To be concrete, I have
> card->csd.taac_ns=5000000 and card->csd.r2w_factor=5. Multiplying
> things out (and accounting for mult=100 for SD cards), I end up with a
> timeout_ns of 0x3b9aca000 (16 seconds) which doesn't fit in the 32-bit
> data->timeout_ns earlier. The truncation was making it look like the
> card requested a max timeout of 3,115,098,112 ns = ~3.1 seconds.

The software timeout is meant to catch cases where the hardware
timeout has failed, but SDHCI also supports a mechanism to use
software timeouts when the timeout is too big for the hardware
 - refer SDHCI_QUIRK2_DISABLE_HW_TIMEOUT and sdhci_calc_sw_timeout()

Historically, R/W timeouts were never longer than 2.5 seconds.
(5ms x 5 x 100 == 2.5s), so 10s has been enough ever since the
driver was written around 2006

Longer timeouts are for erase etc, but they are now always set
in cmd->busy_timeout.

So 22s is a strangely long timeout for R/W operations, and I would say
even 10s is longer than we really want.

One option would be to make it a variable that host controllers
can re-assign if needed e.g.

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 6aaf5c3ce34c..e5b4317937ed 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1670,7 +1670,7 @@ static bool sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
 	else if (!cmd->data && cmd->busy_timeout > 9000)
 		timeout += DIV_ROUND_UP(cmd->busy_timeout, 1000) * HZ + HZ;
 	else
-		timeout += 10 * HZ;
+		timeout += host->fallback_timeout_secs * HZ;
 	sdhci_mod_timer(host, cmd->mrq, timeout);
 
 	if (host->use_external_dma)
@@ -3936,6 +3936,8 @@ struct sdhci_host *sdhci_alloc_host(struct device *dev,
 	 */
 	host->adma_table_cnt = SDHCI_MAX_SEGS * 2 + 1;
 
+	host->fallback_timeout_secs = 10;
+
 	return host;
 }





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

end of thread, other threads:[~2021-06-21  9:16 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-16  9:25 [PATCH V1] mmc: sdhci: Update the software timeout value for sdhc Shaik Sajida Bhanu
2021-06-16 12:16 ` Konrad Dybcio
2021-06-18 15:30 ` Veerabhadrarao Badiganti
2021-06-18 22:49   ` Doug Anderson
2021-06-21  9:16     ` Adrian Hunter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).