| Message ID | 1485250588-24698-3-git-send-email-jeremymc@redhat.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | Andy Gross |
| Headers | show |
Hi Jeremy, From what I can see in codeaurora tree, quirk (SDHCI_QUIRK2_SLOW_INT_CLR) is only required for sdhci-msm with minor number 2B. I think this patch may not be relevant for msm8992 controller. And as you have also mentioned in your V2, that even without this change, the detection is fine. The delay you are observing could be due to something else unless we are sure. On reading history of this quirk, I see that this patch was a SW workaround for some HW issue in very initial controller. This was fixed for later controllers. For this reason, in my opinion we can drop this patch. But, as we discussed on IRC, let's investigate more on your issue before finalizing on this patch for merging. Regards Ritesh On 1/24/2017 3:06 PM, Jeremy McNicoll 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 f3f3fb3..11dc389 100644 > --- a/drivers/mmc/host/sdhci-msm.c > +++ b/drivers/mmc/host/sdhci-msm.c > @@ -1304,6 +1304,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 06dfac2..68a21a3 100644 > --- a/drivers/mmc/host/sdhci.c > +++ b/drivers/mmc/host/sdhci.c > @@ -2742,11 +2742,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 400f3a1..7fa1004 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 > >
On 2017-01-25 5:18 AM, Ritesh Harjani wrote: > Hi Jeremy, > > From what I can see in codeaurora tree, quirk > (SDHCI_QUIRK2_SLOW_INT_CLR) is only required for sdhci-msm with minor > number 2B. I think this patch may not be relevant for msm8992 controller. minor number 2B ? Is that a typo ? Please check the code from here: https://android.googlesource.com/kernel/msm.git/+/android-msm-bullhead-3.10-n-preview-1/drivers/mmc/host/sdhci-msm.c?autodive=0%2F%2F#3213 Ok, in the tree there is a check for both 0x3E and 0x2E. > And as you have also mentioned in your V2, that even without this > change, the detection is fine. The delay you are observing could be due > to something else unless we are sure. > It is, I did some more testing without this patch and a hack to deal with a _KNOWN_ rpm-smd issue on the msm899(2/4). The detection time was much quicker and very reliable. Bjorn Andersson is aware of the issue and I would like to defer to him as to what solution he would like to pursue. > On reading history of this quirk, I see that this patch was a SW > workaround for some HW issue in very initial controller. > This was fixed for later controllers. > For this reason, in my opinion we can drop this patch. > Agreed as I mentioned above the root cause (reasonably confident) of the inconsistent / slow SDHCI detection time seems to be due to the rpm-smd issue mentioned above. It seems as though this quirk was masking / altering things enough to provide an appearance of slightly better detection. Mind you my sample set was only 1 device. It would have been nice to get data over a larger sample size. > But, as we discussed on IRC, let's investigate more on your issue before > finalizing on this patch for merging. > At this point I am satisfied that dropping this patch still allows SDHCI detection to occur. Will drop as part of V4. -jeremy > Regards > Ritesh > > On 1/24/2017 3:06 PM, Jeremy McNicoll 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 f3f3fb3..11dc389 100644 >> --- a/drivers/mmc/host/sdhci-msm.c >> +++ b/drivers/mmc/host/sdhci-msm.c >> @@ -1304,6 +1304,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 06dfac2..68a21a3 100644 >> --- a/drivers/mmc/host/sdhci.c >> +++ b/drivers/mmc/host/sdhci.c >> @@ -2742,11 +2742,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 400f3a1..7fa1004 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 >> >> > -- 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 --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c index f3f3fb3..11dc389 100644 --- a/drivers/mmc/host/sdhci-msm.c +++ b/drivers/mmc/host/sdhci-msm.c @@ -1304,6 +1304,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 06dfac2..68a21a3 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -2742,11 +2742,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 400f3a1..7fa1004 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
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(-)