diff mbox

[5/5] sdhci: Add quirk for delayed IRQ ACK

Message ID 1479863388-23678-6-git-send-email-jeremymc@redhat.com (mailing list archive)
State Not Applicable, archived
Delegated to: Andy Gross
Headers show

Commit Message

Jeremy McNicoll Nov. 23, 2016, 1:09 a.m. UTC
On msm8992 it has been observed that IRQs were not getting
ACK'd correctly when clocked at speeds greater than 400KHz.

Signed-off-by: Jeremy McNicoll <jeremymc@redhat.com>
---
 drivers/mmc/host/sdhci-msm.c |  7 +++++++
 drivers/mmc/host/sdhci.c     | 12 ++++++++++--
 drivers/mmc/host/sdhci.h     |  2 ++
 3 files changed, 19 insertions(+), 2 deletions(-)

Comments

Jisheng Zhang Nov. 23, 2016, 3:36 a.m. UTC | #1
On Tue, 22 Nov 2016 17:09:48 -0800
Jeremy McNicoll <jeremymc@redhat.com> wrote:

> On msm8992 it has been observed that IRQs were not getting
> ACK'd correctly when clocked at speeds greater than 400KHz.
> 
> Signed-off-by: Jeremy McNicoll <jeremymc@redhat.com>
> ---
>  drivers/mmc/host/sdhci-msm.c |  7 +++++++
>  drivers/mmc/host/sdhci.c     | 12 ++++++++++--
>  drivers/mmc/host/sdhci.h     |  2 ++
>  3 files changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 1fcda96..459003c 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -1303,6 +1303,13 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>  			       CORE_VENDOR_SPEC_CAPABILITIES0);
>  	}
>  
> +	/* Enable delayed IRQ handling workaround on 8992 */
> +	if (core_major == 1 && core_minor == 0x3e) {
> +		/* Add 40us delay in interrupt handler when operating
> +		 * at initialization frequency of 400KHz. */
> +		host->quirks2 |= SDHCI_QUIRK2_SLOW_INT_CLR;
> +	}
> +
>  	/* Setup IRQ for handling power/voltage tasks with PMIC */
>  	msm_host->pwr_irq = platform_get_irq_byname(pdev, "pwr_irq");
>  	if (msm_host->pwr_irq < 0) {
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 5911f98..c1aae22 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -2703,11 +2703,19 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id)
>  			result = IRQ_WAKE_THREAD;
>  		}
>  
> -		if (intmask & SDHCI_INT_CMD_MASK)
> +		if (intmask & SDHCI_INT_CMD_MASK) {
> +			if ((host->quirks2 & SDHCI_QUIRK2_SLOW_INT_CLR) && (host->clock <= 400000)) {
> +				udelay(40);
> +			}
>  			sdhci_cmd_irq(host, intmask & SDHCI_INT_CMD_MASK);
> +		}
>  
> -		if (intmask & SDHCI_INT_DATA_MASK)
> +		if (intmask & SDHCI_INT_DATA_MASK) {
> +			if ((host->quirks2 & SDHCI_QUIRK2_SLOW_INT_CLR) && (host->clock <= 400000)) {
> +				udelay(40);
> +			}
>  			sdhci_data_irq(host, intmask & SDHCI_INT_DATA_MASK);
> +		}
>  
>  		if (intmask & SDHCI_INT_BUS_POWER)
>  			pr_err("%s: Card is consuming too much power!\n",
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index c055e24..5f8301e 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -24,6 +24,8 @@
>   * Controller registers
>   */
>  
> +#define SDHCI_QUIRK2_SLOW_INT_CLR	(1<<5)

IIRC, new quirk isn't allowed now.

Thanks,
Jisheng

--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jeremy McNicoll Nov. 23, 2016, 3:48 a.m. UTC | #2
On 2016-11-22 7:36 PM, Jisheng Zhang wrote:
> On Tue, 22 Nov 2016 17:09:48 -0800
> Jeremy McNicoll <jeremymc@redhat.com> wrote:
>
>> On msm8992 it has been observed that IRQs were not getting
>> ACK'd correctly when clocked at speeds greater than 400KHz.
>>
>> Signed-off-by: Jeremy McNicoll <jeremymc@redhat.com>
>> ---
>>  drivers/mmc/host/sdhci-msm.c |  7 +++++++
>>  drivers/mmc/host/sdhci.c     | 12 ++++++++++--
>>  drivers/mmc/host/sdhci.h     |  2 ++
>>  3 files changed, 19 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>> index 1fcda96..459003c 100644
>> --- a/drivers/mmc/host/sdhci-msm.c
>> +++ b/drivers/mmc/host/sdhci-msm.c
>> @@ -1303,6 +1303,13 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>>  			       CORE_VENDOR_SPEC_CAPABILITIES0);
>>  	}
>>
>> +	/* Enable delayed IRQ handling workaround on 8992 */
>> +	if (core_major == 1 && core_minor == 0x3e) {
>> +		/* Add 40us delay in interrupt handler when operating
>> +		 * at initialization frequency of 400KHz. */
>> +		host->quirks2 |= SDHCI_QUIRK2_SLOW_INT_CLR;
>> +	}
>> +
>>  	/* Setup IRQ for handling power/voltage tasks with PMIC */
>>  	msm_host->pwr_irq = platform_get_irq_byname(pdev, "pwr_irq");
>>  	if (msm_host->pwr_irq < 0) {
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> index 5911f98..c1aae22 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -2703,11 +2703,19 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id)
>>  			result = IRQ_WAKE_THREAD;
>>  		}
>>
>> -		if (intmask & SDHCI_INT_CMD_MASK)
>> +		if (intmask & SDHCI_INT_CMD_MASK) {
>> +			if ((host->quirks2 & SDHCI_QUIRK2_SLOW_INT_CLR) && (host->clock <= 400000)) {
>> +				udelay(40);
>> +			}
>>  			sdhci_cmd_irq(host, intmask & SDHCI_INT_CMD_MASK);
>> +		}
>>
>> -		if (intmask & SDHCI_INT_DATA_MASK)
>> +		if (intmask & SDHCI_INT_DATA_MASK) {
>> +			if ((host->quirks2 & SDHCI_QUIRK2_SLOW_INT_CLR) && (host->clock <= 400000)) {
>> +				udelay(40);
>> +			}
>>  			sdhci_data_irq(host, intmask & SDHCI_INT_DATA_MASK);
>> +		}
>>
>>  		if (intmask & SDHCI_INT_BUS_POWER)
>>  			pr_err("%s: Card is consuming too much power!\n",
>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
>> index c055e24..5f8301e 100644
>> --- a/drivers/mmc/host/sdhci.h
>> +++ b/drivers/mmc/host/sdhci.h
>> @@ -24,6 +24,8 @@
>>   * Controller registers
>>   */
>>
>> +#define SDHCI_QUIRK2_SLOW_INT_CLR	(1<<5)
>
> IIRC, new quirk isn't allowed now.
>

Why not?

-jeremy

> Thanks,
> Jisheng
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jisheng Zhang Nov. 23, 2016, 4:12 a.m. UTC | #3
On Tue, 22 Nov 2016 19:48:56 -0800 Jeremy McNicoll wrote:

> On 2016-11-22 7:36 PM, Jisheng Zhang wrote:
> > On Tue, 22 Nov 2016 17:09:48 -0800
> > Jeremy McNicoll <jeremymc@redhat.com> wrote:
> >  
> >> On msm8992 it has been observed that IRQs were not getting
> >> ACK'd correctly when clocked at speeds greater than 400KHz.
> >>
> >> Signed-off-by: Jeremy McNicoll <jeremymc@redhat.com>
> >> ---
> >>  drivers/mmc/host/sdhci-msm.c |  7 +++++++
> >>  drivers/mmc/host/sdhci.c     | 12 ++++++++++--
> >>  drivers/mmc/host/sdhci.h     |  2 ++
> >>  3 files changed, 19 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> >> index 1fcda96..459003c 100644
> >> --- a/drivers/mmc/host/sdhci-msm.c
> >> +++ b/drivers/mmc/host/sdhci-msm.c
> >> @@ -1303,6 +1303,13 @@ static int sdhci_msm_probe(struct platform_device *pdev)
> >>  			       CORE_VENDOR_SPEC_CAPABILITIES0);
> >>  	}
> >>
> >> +	/* Enable delayed IRQ handling workaround on 8992 */
> >> +	if (core_major == 1 && core_minor == 0x3e) {
> >> +		/* Add 40us delay in interrupt handler when operating
> >> +		 * at initialization frequency of 400KHz. */
> >> +		host->quirks2 |= SDHCI_QUIRK2_SLOW_INT_CLR;
> >> +	}
> >> +
> >>  	/* Setup IRQ for handling power/voltage tasks with PMIC */
> >>  	msm_host->pwr_irq = platform_get_irq_byname(pdev, "pwr_irq");
> >>  	if (msm_host->pwr_irq < 0) {
> >> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> >> index 5911f98..c1aae22 100644
> >> --- a/drivers/mmc/host/sdhci.c
> >> +++ b/drivers/mmc/host/sdhci.c
> >> @@ -2703,11 +2703,19 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id)
> >>  			result = IRQ_WAKE_THREAD;
> >>  		}
> >>
> >> -		if (intmask & SDHCI_INT_CMD_MASK)
> >> +		if (intmask & SDHCI_INT_CMD_MASK) {
> >> +			if ((host->quirks2 & SDHCI_QUIRK2_SLOW_INT_CLR) && (host->clock <= 400000)) {
> >> +				udelay(40);
> >> +			}
> >>  			sdhci_cmd_irq(host, intmask & SDHCI_INT_CMD_MASK);
> >> +		}
> >>
> >> -		if (intmask & SDHCI_INT_DATA_MASK)
> >> +		if (intmask & SDHCI_INT_DATA_MASK) {
> >> +			if ((host->quirks2 & SDHCI_QUIRK2_SLOW_INT_CLR) && (host->clock <= 400000)) {
> >> +				udelay(40);
> >> +			}
> >>  			sdhci_data_irq(host, intmask & SDHCI_INT_DATA_MASK);
> >> +		}
> >>
> >>  		if (intmask & SDHCI_INT_BUS_POWER)
> >>  			pr_err("%s: Card is consuming too much power!\n",
> >> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> >> index c055e24..5f8301e 100644
> >> --- a/drivers/mmc/host/sdhci.h
> >> +++ b/drivers/mmc/host/sdhci.h
> >> @@ -24,6 +24,8 @@
> >>   * Controller registers
> >>   */
> >>
> >> +#define SDHCI_QUIRK2_SLOW_INT_CLR	(1<<5)  
> >
> > IIRC, new quirk isn't allowed now.
> >  
> 
> Why not?

IIRC, mmc subsystem will behave as a lib in the long run, so the community
and developers call for no new quirk. For your case, we may need to handle
the udelay in sdhci-msm.c, export sdhci_irq as a helper function?

Thanks
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jeremy McNicoll Nov. 23, 2016, 5:23 a.m. UTC | #4
On 2016-11-22 8:12 PM, Jisheng Zhang wrote:
> On Tue, 22 Nov 2016 19:48:56 -0800 Jeremy McNicoll wrote:
>
>> On 2016-11-22 7:36 PM, Jisheng Zhang wrote:
>>> On Tue, 22 Nov 2016 17:09:48 -0800
>>> Jeremy McNicoll <jeremymc@redhat.com> wrote:
>>>
>>>> On msm8992 it has been observed that IRQs were not getting
>>>> ACK'd correctly when clocked at speeds greater than 400KHz.
>>>>
>>>> Signed-off-by: Jeremy McNicoll <jeremymc@redhat.com>
>>>> ---
>>>>  drivers/mmc/host/sdhci-msm.c |  7 +++++++
>>>>  drivers/mmc/host/sdhci.c     | 12 ++++++++++--
>>>>  drivers/mmc/host/sdhci.h     |  2 ++
>>>>  3 files changed, 19 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>>>> index 1fcda96..459003c 100644
>>>> --- a/drivers/mmc/host/sdhci-msm.c
>>>> +++ b/drivers/mmc/host/sdhci-msm.c
>>>> @@ -1303,6 +1303,13 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>>>>  			       CORE_VENDOR_SPEC_CAPABILITIES0);
>>>>  	}
>>>>
>>>> +	/* Enable delayed IRQ handling workaround on 8992 */
>>>> +	if (core_major == 1 && core_minor == 0x3e) {
>>>> +		/* Add 40us delay in interrupt handler when operating
>>>> +		 * at initialization frequency of 400KHz. */
>>>> +		host->quirks2 |= SDHCI_QUIRK2_SLOW_INT_CLR;
>>>> +	}
>>>> +
>>>>  	/* Setup IRQ for handling power/voltage tasks with PMIC */
>>>>  	msm_host->pwr_irq = platform_get_irq_byname(pdev, "pwr_irq");
>>>>  	if (msm_host->pwr_irq < 0) {
>>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>>> index 5911f98..c1aae22 100644
>>>> --- a/drivers/mmc/host/sdhci.c
>>>> +++ b/drivers/mmc/host/sdhci.c
>>>> @@ -2703,11 +2703,19 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id)
>>>>  			result = IRQ_WAKE_THREAD;
>>>>  		}
>>>>
>>>> -		if (intmask & SDHCI_INT_CMD_MASK)
>>>> +		if (intmask & SDHCI_INT_CMD_MASK) {
>>>> +			if ((host->quirks2 & SDHCI_QUIRK2_SLOW_INT_CLR) && (host->clock <= 400000)) {
>>>> +				udelay(40);
>>>> +			}
>>>>  			sdhci_cmd_irq(host, intmask & SDHCI_INT_CMD_MASK);
>>>> +		}
>>>>
>>>> -		if (intmask & SDHCI_INT_DATA_MASK)
>>>> +		if (intmask & SDHCI_INT_DATA_MASK) {
>>>> +			if ((host->quirks2 & SDHCI_QUIRK2_SLOW_INT_CLR) && (host->clock <= 400000)) {
>>>> +				udelay(40);
>>>> +			}
>>>>  			sdhci_data_irq(host, intmask & SDHCI_INT_DATA_MASK);
>>>> +		}
>>>>
>>>>  		if (intmask & SDHCI_INT_BUS_POWER)
>>>>  			pr_err("%s: Card is consuming too much power!\n",
>>>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
>>>> index c055e24..5f8301e 100644
>>>> --- a/drivers/mmc/host/sdhci.h
>>>> +++ b/drivers/mmc/host/sdhci.h
>>>> @@ -24,6 +24,8 @@
>>>>   * Controller registers
>>>>   */
>>>>
>>>> +#define SDHCI_QUIRK2_SLOW_INT_CLR	(1<<5)
>>>
>>> IIRC, new quirk isn't allowed now.
>>>
>>
>> Why not?
>
> IIRC, mmc subsystem will behave as a lib in the long run, so the community
> and developers call for no new quirk. For your case, we may need to handle
> the udelay in sdhci-msm.c, export sdhci_irq as a helper function?
>
Thanks for the details and suggestions.  I am going to wait until the US
Thanksgiving (and cyber Monday) is over before I do any more.

-jeremy

--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 1fcda96..459003c 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -1303,6 +1303,13 @@  static int sdhci_msm_probe(struct platform_device *pdev)
 			       CORE_VENDOR_SPEC_CAPABILITIES0);
 	}
 
+	/* Enable delayed IRQ handling workaround on 8992 */
+	if (core_major == 1 && core_minor == 0x3e) {
+		/* Add 40us delay in interrupt handler when operating
+		 * at initialization frequency of 400KHz. */
+		host->quirks2 |= SDHCI_QUIRK2_SLOW_INT_CLR;
+	}
+
 	/* Setup IRQ for handling power/voltage tasks with PMIC */
 	msm_host->pwr_irq = platform_get_irq_byname(pdev, "pwr_irq");
 	if (msm_host->pwr_irq < 0) {
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 5911f98..c1aae22 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2703,11 +2703,19 @@  static irqreturn_t sdhci_irq(int irq, void *dev_id)
 			result = IRQ_WAKE_THREAD;
 		}
 
-		if (intmask & SDHCI_INT_CMD_MASK)
+		if (intmask & SDHCI_INT_CMD_MASK) {
+			if ((host->quirks2 & SDHCI_QUIRK2_SLOW_INT_CLR) && (host->clock <= 400000)) {
+				udelay(40);
+			}
 			sdhci_cmd_irq(host, intmask & SDHCI_INT_CMD_MASK);
+		}
 
-		if (intmask & SDHCI_INT_DATA_MASK)
+		if (intmask & SDHCI_INT_DATA_MASK) {
+			if ((host->quirks2 & SDHCI_QUIRK2_SLOW_INT_CLR) && (host->clock <= 400000)) {
+				udelay(40);
+			}
 			sdhci_data_irq(host, intmask & SDHCI_INT_DATA_MASK);
+		}
 
 		if (intmask & SDHCI_INT_BUS_POWER)
 			pr_err("%s: Card is consuming too much power!\n",
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index c055e24..5f8301e 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -24,6 +24,8 @@ 
  * Controller registers
  */
 
+#define SDHCI_QUIRK2_SLOW_INT_CLR	(1<<5)
+
 #define SDHCI_DMA_ADDRESS	0x00
 #define SDHCI_ARGUMENT2		SDHCI_DMA_ADDRESS