* [PATCH 1/3] mmc: sdhci-pci: Add MSI support @ 2012-03-13 17:16 Alexander Stein 2012-03-13 17:16 ` [PATCH 2/3] mmc: sdhci: check interrupt flags in ISR again Alexander Stein ` (2 more replies) 0 siblings, 3 replies; 13+ messages in thread From: Alexander Stein @ 2012-03-13 17:16 UTC (permalink / raw) To: Chris Ball Cc: Jesse Barnes, Adrian Hunter, linux-mmc, linux-kernel, linux-pci, Alexander Stein Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com> --- drivers/mmc/host/sdhci-pci.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/drivers/mmc/host/sdhci-pci.c b/drivers/mmc/host/sdhci-pci.c index 7165e6a..9382f27 100644 --- a/drivers/mmc/host/sdhci-pci.c +++ b/drivers/mmc/host/sdhci-pci.c @@ -1379,6 +1379,8 @@ static int __devinit sdhci_pci_probe(struct pci_dev *pdev, slots = chip->num_slots; /* Quirk may have changed this */ + pci_enable_msi(pdev); + for (i = 0; i < slots; i++) { slot = sdhci_pci_probe_slot(pdev, chip, first_bar, i); if (IS_ERR(slot)) { @@ -1397,6 +1399,8 @@ static int __devinit sdhci_pci_probe(struct pci_dev *pdev, return 0; free: + pci_disable_msi(pdev); + pci_set_drvdata(pdev, NULL); kfree(chip); @@ -1419,6 +1423,8 @@ static void __devexit sdhci_pci_remove(struct pci_dev *pdev) for (i = 0; i < chip->num_slots; i++) sdhci_pci_remove_slot(chip->slots[i]); + pci_disable_msi(pdev); + pci_set_drvdata(pdev, NULL); kfree(chip); } -- 1.7.3.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/3] mmc: sdhci: check interrupt flags in ISR again 2012-03-13 17:16 [PATCH 1/3] mmc: sdhci-pci: Add MSI support Alexander Stein @ 2012-03-13 17:16 ` Alexander Stein 2012-03-14 7:39 ` Adrian Hunter 2012-03-13 17:16 ` [PATCH 3/3] mmc: sdhci-pci: allow 8-bit bus width for Intel PCH Alexander Stein 2012-03-16 3:41 ` [PATCH 1/3] mmc: sdhci-pci: Add MSI support Chris Ball 2 siblings, 1 reply; 13+ messages in thread From: Alexander Stein @ 2012-03-13 17:16 UTC (permalink / raw) To: Chris Ball Cc: Jesse Barnes, Adrian Hunter, linux-mmc, linux-kernel, linux-pci, Alexander Stein When using MSI it is possible that a new MSI is sent while an earlier MSI is currently handled. In this case SDHCI_INT_STATUS only contains SDHCI_INT_RESPONSE and the ISR would not be called again. But at the end of the ISR SDHCI_INT_DATA_END is now also pending which would be ignored. Fix this by rereading the interrupt flags in the ISR until no interrupt we care is pending. Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com> --- drivers/mmc/host/sdhci.c | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-) diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index 8d66706..654ab32 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -2268,6 +2268,7 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id) irqreturn_t result; struct sdhci_host *host = dev_id; u32 intmask; + u32 intmask_unhandled; int cardint = 0; spin_lock(&host->lock); @@ -2286,6 +2287,7 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id) goto out; } +again: DBG("*** %s got interrupt: 0x%08x\n", mmc_hostname(host->mmc), intmask); @@ -2336,6 +2338,14 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id) sdhci_writel(host, SDHCI_INT_BUS_POWER, SDHCI_INT_STATUS); } + intmask_unhandled = intmask; + + intmask = sdhci_readl(host, SDHCI_INT_STATUS); + + /* Do interrupt handling again if we got new flags */ + if (intmask & ~intmask_unhandled) + goto again; + intmask &= ~SDHCI_INT_BUS_POWER; if (intmask & SDHCI_INT_CARD_INT) -- 1.7.3.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] mmc: sdhci: check interrupt flags in ISR again 2012-03-13 17:16 ` [PATCH 2/3] mmc: sdhci: check interrupt flags in ISR again Alexander Stein @ 2012-03-14 7:39 ` Adrian Hunter 2012-03-14 7:53 ` Alexander Stein 0 siblings, 1 reply; 13+ messages in thread From: Adrian Hunter @ 2012-03-14 7:39 UTC (permalink / raw) To: Alexander Stein Cc: Chris Ball, Jesse Barnes, linux-mmc, linux-kernel, linux-pci On 13/03/12 19:16, Alexander Stein wrote: > When using MSI it is possible that a new MSI is sent while an earlier > MSI is currently handled. In this case SDHCI_INT_STATUS only contains > SDHCI_INT_RESPONSE and the ISR would not be called again. But at the end > of the ISR SDHCI_INT_DATA_END is now also pending which would be ignored. > > Fix this by rereading the interrupt flags in the ISR until no interrupt > we care is pending. > > Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com> > --- > drivers/mmc/host/sdhci.c | 10 ++++++++++ > 1 files changed, 10 insertions(+), 0 deletions(-) > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > index 8d66706..654ab32 100644 > --- a/drivers/mmc/host/sdhci.c > +++ b/drivers/mmc/host/sdhci.c > @@ -2268,6 +2268,7 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id) > irqreturn_t result; > struct sdhci_host *host = dev_id; > u32 intmask; > + u32 intmask_unhandled; > int cardint = 0; > > spin_lock(&host->lock); > @@ -2286,6 +2287,7 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id) > goto out; > } > > +again: > DBG("*** %s got interrupt: 0x%08x\n", > mmc_hostname(host->mmc), intmask); > > @@ -2336,6 +2338,14 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id) > sdhci_writel(host, SDHCI_INT_BUS_POWER, SDHCI_INT_STATUS); > } > > + intmask_unhandled = intmask; > + > + intmask = sdhci_readl(host, SDHCI_INT_STATUS); > + > + /* Do interrupt handling again if we got new flags */ > + if (intmask & ~intmask_unhandled) > + goto again; > + > intmask &= ~SDHCI_INT_BUS_POWER; > > if (intmask & SDHCI_INT_CARD_INT) Why not just replace mmiowb() i.e. diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index 8d66706..da8a101 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -2353,7 +2353,9 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id) result = IRQ_HANDLED; - mmiowb(); + intmask = sdhci_readl(host, SDHCI_INT_STATUS); + if (intmask) + goto again; out: spin_unlock(&host->lock); But maybe it would be safer limiting the number of loops i.e. diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index 8d66706..d88247d 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -2268,7 +2268,7 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id) irqreturn_t result; struct sdhci_host *host = dev_id; u32 intmask; - int cardint = 0; + int cardint = 0, max_loops = 16; spin_lock(&host->lock); @@ -2353,7 +2353,9 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id) result = IRQ_HANDLED; - mmiowb(); + intmask = sdhci_readl(host, SDHCI_INT_STATUS); + if (intmask && --max_loops) + goto again; out: spin_unlock(&host->lock); ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] mmc: sdhci: check interrupt flags in ISR again 2012-03-14 7:39 ` Adrian Hunter @ 2012-03-14 7:53 ` Alexander Stein 2012-03-14 8:13 ` Adrian Hunter 0 siblings, 1 reply; 13+ messages in thread From: Alexander Stein @ 2012-03-14 7:53 UTC (permalink / raw) To: Adrian Hunter Cc: Chris Ball, Jesse Barnes, linux-mmc, linux-kernel, linux-pci Hello, Am Mittwoch, 14. März 2012, 09:39:02 schrieb Adrian Hunter: > On 13/03/12 19:16, Alexander Stein wrote: > > When using MSI it is possible that a new MSI is sent while an earlier > > MSI is currently handled. In this case SDHCI_INT_STATUS only contains > > SDHCI_INT_RESPONSE and the ISR would not be called again. But at the end > > of the ISR SDHCI_INT_DATA_END is now also pending which would be > > ignored. > > > > Fix this by rereading the interrupt flags in the ISR until no interrupt > > we care is pending. > > > > Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com> > [...] > > @@ -2336,6 +2338,14 @@ static irqreturn_t sdhci_irq(int irq, void > > *dev_id)> > > sdhci_writel(host, SDHCI_INT_BUS_POWER, SDHCI_INT_STATUS); > > > > } > > > > + intmask_unhandled = intmask; > > + > > + intmask = sdhci_readl(host, SDHCI_INT_STATUS); > > + > > + /* Do interrupt handling again if we got new flags */ > > + if (intmask & ~intmask_unhandled) > > + goto again; > > + > > > > intmask &= ~SDHCI_INT_BUS_POWER; > > > > if (intmask & SDHCI_INT_CARD_INT) > > Why not just replace mmiowb() i.e. > > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > index 8d66706..da8a101 100644 > --- a/drivers/mmc/host/sdhci.c > +++ b/drivers/mmc/host/sdhci.c > @@ -2353,7 +2353,9 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id) > > result = IRQ_HANDLED; > > - mmiowb(); > + intmask = sdhci_readl(host, SDHCI_INT_STATUS); > + if (intmask) > + goto again; > out: > spin_unlock(&host->lock); > Well, I chose this way to only printk the error once. With your suggestion it might be printed in each loop, dunno how often/fast these IRQ stats are set again after clearing. This would end in an endless loop if error flags are set again fast enough, but see below. But in general I like this approach. > But maybe it would be safer limiting the number of loops i.e. > > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > index 8d66706..d88247d 100644 > --- a/drivers/mmc/host/sdhci.c > +++ b/drivers/mmc/host/sdhci.c > @@ -2268,7 +2268,7 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id) > irqreturn_t result; > struct sdhci_host *host = dev_id; > u32 intmask; > - int cardint = 0; > + int cardint = 0, max_loops = 16; > > spin_lock(&host->lock); > > @@ -2353,7 +2353,9 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id) > > result = IRQ_HANDLED; > > - mmiowb(); > + intmask = sdhci_readl(host, SDHCI_INT_STATUS); > + if (intmask && --max_loops) > + goto again; > out: > spin_unlock(&host->lock); The actual problem I saw was a CMD6 command with an R1b response where the IRQ for the 'not busy' event was sent during ISR for the response. So I think normally this should only occur once. Regarding error flags I masked the unhandled flags out in order to print an error only once, even if they might be set again in the next loop. With a simple check on intmask they might occur up to 16 times in the kernel log. IMHO it makes no sense to repeatedly print errors about interrupt flags we don't handle. Suggestions to get a more clean way? Best regards, Alexander -- Dipl.-Inf. Alexander Stein SYS TEC electronic GmbH August-Bebel-Str. 29 D-07973 Greiz Tel: +49-3661-6279-0, Fax: +49-3661-6279-99 eMail: Alexander.Stein@systec-electronic.com Internet: http://www.systec-electronic.com Managing Director: Dipl.-Phys. Siegmar Schmidt Commercial registry: Amtsgericht Jena, HRB 205563 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] mmc: sdhci: check interrupt flags in ISR again 2012-03-14 7:53 ` Alexander Stein @ 2012-03-14 8:13 ` Adrian Hunter 2012-03-14 8:52 ` [PATCH v2] " Alexander Stein 0 siblings, 1 reply; 13+ messages in thread From: Adrian Hunter @ 2012-03-14 8:13 UTC (permalink / raw) To: Alexander Stein Cc: Chris Ball, Jesse Barnes, linux-mmc, linux-kernel, linux-pci On 14/03/12 09:53, Alexander Stein wrote: > Hello, > > Am Mittwoch, 14. März 2012, 09:39:02 schrieb Adrian Hunter: >> On 13/03/12 19:16, Alexander Stein wrote: >>> When using MSI it is possible that a new MSI is sent while an earlier >>> MSI is currently handled. In this case SDHCI_INT_STATUS only contains >>> SDHCI_INT_RESPONSE and the ISR would not be called again. But at the end >>> of the ISR SDHCI_INT_DATA_END is now also pending which would be >>> ignored. >>> >>> Fix this by rereading the interrupt flags in the ISR until no interrupt >>> we care is pending. >>> >>> Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com> >> [...] >>> @@ -2336,6 +2338,14 @@ static irqreturn_t sdhci_irq(int irq, void >>> *dev_id)> >>> sdhci_writel(host, SDHCI_INT_BUS_POWER, SDHCI_INT_STATUS); >>> >>> } >>> >>> + intmask_unhandled = intmask; >>> + >>> + intmask = sdhci_readl(host, SDHCI_INT_STATUS); >>> + >>> + /* Do interrupt handling again if we got new flags */ >>> + if (intmask & ~intmask_unhandled) >>> + goto again; >>> + >>> >>> intmask &= ~SDHCI_INT_BUS_POWER; >>> >>> if (intmask & SDHCI_INT_CARD_INT) >> >> Why not just replace mmiowb() i.e. >> >> >> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c >> index 8d66706..da8a101 100644 >> --- a/drivers/mmc/host/sdhci.c >> +++ b/drivers/mmc/host/sdhci.c >> @@ -2353,7 +2353,9 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id) >> >> result = IRQ_HANDLED; >> >> - mmiowb(); >> + intmask = sdhci_readl(host, SDHCI_INT_STATUS); >> + if (intmask) >> + goto again; >> out: >> spin_unlock(&host->lock); >> > > Well, I chose this way to only printk the error once. With your suggestion it > might be printed in each loop, dunno how often/fast these IRQ stats are set > again after clearing. This would end in an endless loop if error flags are set > again fast enough, but see below. > But in general I like this approach. > >> But maybe it would be safer limiting the number of loops i.e. >> >> >> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c >> index 8d66706..d88247d 100644 >> --- a/drivers/mmc/host/sdhci.c >> +++ b/drivers/mmc/host/sdhci.c >> @@ -2268,7 +2268,7 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id) >> irqreturn_t result; >> struct sdhci_host *host = dev_id; >> u32 intmask; >> - int cardint = 0; >> + int cardint = 0, max_loops = 16; >> >> spin_lock(&host->lock); >> >> @@ -2353,7 +2353,9 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id) >> >> result = IRQ_HANDLED; >> >> - mmiowb(); >> + intmask = sdhci_readl(host, SDHCI_INT_STATUS); >> + if (intmask && --max_loops) >> + goto again; >> out: >> spin_unlock(&host->lock); > > The actual problem I saw was a CMD6 command with an R1b response where the IRQ > for the 'not busy' event was sent during ISR for the response. So I think > normally this should only occur once. > Regarding error flags I masked the unhandled flags out in order to print an > error only once, even if they might be set again in the next loop. With a > simple check on intmask they might occur up to 16 times in the kernel log. > IMHO it makes no sense to repeatedly print errors about interrupt flags we > don't handle. > > Suggestions to get a more clean way? I don't know about clean, but there is this: diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index 8d66706..e0909da 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -2267,8 +2267,8 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id) { irqreturn_t result; struct sdhci_host *host = dev_id; - u32 intmask; - int cardint = 0; + u32 intmask, unexpected = 0; + int cardint = 0, max_loops = 16; spin_lock(&host->lock); @@ -2344,19 +2344,24 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id) intmask &= ~SDHCI_INT_CARD_INT; if (intmask) { - pr_err("%s: Unexpected interrupt 0x%08x.\n", - mmc_hostname(host->mmc), intmask); - sdhci_dumpregs(host); - + unexpected |= intmask; sdhci_writel(host, intmask, SDHCI_INT_STATUS); } result = IRQ_HANDLED; - mmiowb(); + intmask = sdhci_readl(host, SDHCI_INT_STATUS); + if (intmask && --max_loops) + goto again; out: spin_unlock(&host->lock); + if (unexpected) { + pr_err("%s: Unexpected interrupt 0x%08x.\n", + mmc_hostname(host->mmc), unexpected); + sdhci_dumpregs(host); + } + /* * We have to delay this as it calls back into the driver. */ ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2] mmc: sdhci: check interrupt flags in ISR again 2012-03-14 8:13 ` Adrian Hunter @ 2012-03-14 8:52 ` Alexander Stein 2012-03-14 9:23 ` Adrian Hunter 0 siblings, 1 reply; 13+ messages in thread From: Alexander Stein @ 2012-03-14 8:52 UTC (permalink / raw) To: Adrian Hunter Cc: Chris Ball, Jesse Barnes, linux-mmc, linux-kernel, linux-pci, Alexander Stein When using MSI it is possible that a new MSI is sent while an earlier MSI is currently handled. In this case SDHCI_INT_STATUS only contains SDHCI_INT_RESPONSE and the ISR would not be called again. But at the end of the ISR SDHCI_INT_DATA_END is now also pending which would be ignored. Fix this by rereading the interrupt flags in the ISR until no interrupt we care is pending. Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com> --- Changes in v2: * rearranged code a bit and added a maximum loop counter as suggested by Adrian drivers/mmc/host/sdhci.c | 19 ++++++++++++------- 1 files changed, 12 insertions(+), 7 deletions(-) diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index 8d66706..4fe4461 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -2267,8 +2267,8 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id) { irqreturn_t result; struct sdhci_host *host = dev_id; - u32 intmask; - int cardint = 0; + u32 intmask, unexpected = 0; + int cardint = 0, max_loops = 16; spin_lock(&host->lock); @@ -2286,6 +2286,7 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id) goto out; } +again: DBG("*** %s got interrupt: 0x%08x\n", mmc_hostname(host->mmc), intmask); @@ -2344,19 +2345,23 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id) intmask &= ~SDHCI_INT_CARD_INT; if (intmask) { - pr_err("%s: Unexpected interrupt 0x%08x.\n", - mmc_hostname(host->mmc), intmask); - sdhci_dumpregs(host); - + unexpected |= intmask; sdhci_writel(host, intmask, SDHCI_INT_STATUS); } result = IRQ_HANDLED; - mmiowb(); + intmask = sdhci_readl(host, SDHCI_INT_STATUS); + if (intmask && --max_loops) + goto again; out: spin_unlock(&host->lock); + if (unexpected) { + pr_err("%s: Unexpected interrupt 0x%08x.\n", + mmc_hostname(host->mmc), unexpected); + sdhci_dumpregs(host); + } /* * We have to delay this as it calls back into the driver. */ -- 1.7.3.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2] mmc: sdhci: check interrupt flags in ISR again 2012-03-14 8:52 ` [PATCH v2] " Alexander Stein @ 2012-03-14 9:23 ` Adrian Hunter 0 siblings, 0 replies; 13+ messages in thread From: Adrian Hunter @ 2012-03-14 9:23 UTC (permalink / raw) To: Alexander Stein Cc: Chris Ball, Jesse Barnes, linux-mmc, linux-kernel, linux-pci On 14/03/12 10:52, Alexander Stein wrote: > When using MSI it is possible that a new MSI is sent while an earlier > MSI is currently handled. In this case SDHCI_INT_STATUS only contains > SDHCI_INT_RESPONSE and the ISR would not be called again. But at the end > of the ISR SDHCI_INT_DATA_END is now also pending which would be ignored. > > Fix this by rereading the interrupt flags in the ISR until no interrupt > we care is pending. > > Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com> > --- > Changes in v2: > * rearranged code a bit and added a maximum loop counter as suggested > by Adrian > > drivers/mmc/host/sdhci.c | 19 ++++++++++++------- > 1 files changed, 12 insertions(+), 7 deletions(-) Acked-by: Adrian Hunter <adrian.hunter@intel.com> ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/3] mmc: sdhci-pci: allow 8-bit bus width for Intel PCH 2012-03-13 17:16 [PATCH 1/3] mmc: sdhci-pci: Add MSI support Alexander Stein 2012-03-13 17:16 ` [PATCH 2/3] mmc: sdhci: check interrupt flags in ISR again Alexander Stein @ 2012-03-13 17:16 ` Alexander Stein 2012-03-13 17:25 ` Greg KH 2012-03-14 1:17 ` [PATCH 3/3] " Tomoya MORINAGA 2012-03-16 3:41 ` [PATCH 1/3] mmc: sdhci-pci: Add MSI support Chris Ball 2 siblings, 2 replies; 13+ messages in thread From: Alexander Stein @ 2012-03-13 17:16 UTC (permalink / raw) To: Chris Ball Cc: Jesse Barnes, Adrian Hunter, linux-mmc, linux-kernel, linux-pci, Alexander Stein Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com> --- drivers/mmc/host/sdhci-pci.c | 27 +++++++++++++++++++++++++++ include/linux/pci_ids.h | 2 ++ 2 files changed, 29 insertions(+), 0 deletions(-) diff --git a/drivers/mmc/host/sdhci-pci.c b/drivers/mmc/host/sdhci-pci.c index 9382f27..0218cb2 100644 --- a/drivers/mmc/host/sdhci-pci.c +++ b/drivers/mmc/host/sdhci-pci.c @@ -172,6 +172,12 @@ static int mrst_hc_probe(struct sdhci_pci_chip *chip) return 0; } +static int pch_hc_probe_slot(struct sdhci_pci_slot *slot) +{ + slot->host->mmc->caps |= MMC_CAP_8_BIT_DATA; + return 0; +} + #ifdef CONFIG_PM_RUNTIME static irqreturn_t sdhci_pci_sd_cd(int irq, void *dev_id) @@ -281,6 +287,11 @@ static const struct sdhci_pci_fixes sdhci_intel_mfd_emmc = { .probe_slot = mfd_emmc_probe_slot, }; +static const struct sdhci_pci_fixes sdhci_intel_pch_sdio = { + .quirks = SDHCI_QUIRK_BROKEN_ADMA, + .probe_slot = pch_hc_probe_slot, +}; + /* O2Micro extra registers */ #define O2_SD_LOCK_WP 0xD3 #define O2_SD_MULTI_VCC3V 0xEE @@ -817,6 +828,22 @@ static const struct pci_device_id pci_ids[] __devinitdata = { }, { + .vendor = PCI_VENDOR_ID_INTEL, + .device = PCI_DEVICE_ID_INTEL_PCH_SDIO0, + .subvendor = PCI_ANY_ID, + .subdevice = PCI_ANY_ID, + .driver_data = (kernel_ulong_t)&sdhci_intel_pch_sdio, + }, + + { + .vendor = PCI_VENDOR_ID_INTEL, + .device = PCI_DEVICE_ID_INTEL_PCH_SDIO1, + .subvendor = PCI_ANY_ID, + .subdevice = PCI_ANY_ID, + .driver_data = (kernel_ulong_t)&sdhci_intel_pch_sdio, + }, + + { .vendor = PCI_VENDOR_ID_O2, .device = PCI_DEVICE_ID_O2_8120, .subvendor = PCI_ANY_ID, diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h index 31d77af..28cd019 100644 --- a/include/linux/pci_ids.h +++ b/include/linux/pci_ids.h @@ -2800,6 +2800,8 @@ #define PCI_DEVICE_ID_INTEL_82454NX 0x84cb #define PCI_DEVICE_ID_INTEL_84460GX 0x84ea #define PCI_DEVICE_ID_INTEL_IXP4XX 0x8500 +#define PCI_DEVICE_ID_INTEL_PCH_SDIO0 0x8809 +#define PCI_DEVICE_ID_INTEL_PCH_SDIO1 0x880a #define PCI_DEVICE_ID_INTEL_IXP2800 0x9004 #define PCI_DEVICE_ID_INTEL_S21152BB 0xb152 -- 1.7.3.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] mmc: sdhci-pci: allow 8-bit bus width for Intel PCH 2012-03-13 17:16 ` [PATCH 3/3] mmc: sdhci-pci: allow 8-bit bus width for Intel PCH Alexander Stein @ 2012-03-13 17:25 ` Greg KH 2012-03-14 7:38 ` [PATCH v2] " Alexander Stein 2012-03-14 1:17 ` [PATCH 3/3] " Tomoya MORINAGA 1 sibling, 1 reply; 13+ messages in thread From: Greg KH @ 2012-03-13 17:25 UTC (permalink / raw) To: Alexander Stein Cc: Chris Ball, Jesse Barnes, Adrian Hunter, linux-mmc, linux-kernel, linux-pci On Tue, Mar 13, 2012 at 06:16:42PM +0100, Alexander Stein wrote: > --- a/include/linux/pci_ids.h > +++ b/include/linux/pci_ids.h > @@ -2800,6 +2800,8 @@ > #define PCI_DEVICE_ID_INTEL_82454NX 0x84cb > #define PCI_DEVICE_ID_INTEL_84460GX 0x84ea > #define PCI_DEVICE_ID_INTEL_IXP4XX 0x8500 > +#define PCI_DEVICE_ID_INTEL_PCH_SDIO0 0x8809 > +#define PCI_DEVICE_ID_INTEL_PCH_SDIO1 0x880a > #define PCI_DEVICE_ID_INTEL_IXP2800 0x9004 > #define PCI_DEVICE_ID_INTEL_S21152BB 0xb152 Did you read the comment at the top of this file before adding your new device ids? If not, please do so, and redo your patch. thanks, greg k-h ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2] mmc: sdhci-pci: allow 8-bit bus width for Intel PCH 2012-03-13 17:25 ` Greg KH @ 2012-03-14 7:38 ` Alexander Stein 0 siblings, 0 replies; 13+ messages in thread From: Alexander Stein @ 2012-03-14 7:38 UTC (permalink / raw) To: Greg KH Cc: Chris Ball, Jesse Barnes, Adrian Hunter, linux-mmc, linux-kernel, linux-pci, Alexander Stein Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com> --- Changes in v2: * moved PCI device ID definition to sdhci-pci.c. Thanks Greg for the hint drivers/mmc/host/sdhci-pci.c | 33 +++++++++++++++++++++++++++++++++ 1 files changed, 33 insertions(+), 0 deletions(-) diff --git a/drivers/mmc/host/sdhci-pci.c b/drivers/mmc/host/sdhci-pci.c index 9382f27..c9db8f8 100644 --- a/drivers/mmc/host/sdhci-pci.c +++ b/drivers/mmc/host/sdhci-pci.c @@ -29,6 +29,12 @@ #include "sdhci.h" /* + * PCI device IDs + */ +#define PCI_DEVICE_ID_INTEL_PCH_SDIO0 0x8809 +#define PCI_DEVICE_ID_INTEL_PCH_SDIO1 0x880a + +/* * PCI registers */ @@ -172,6 +178,12 @@ static int mrst_hc_probe(struct sdhci_pci_chip *chip) return 0; } +static int pch_hc_probe_slot(struct sdhci_pci_slot *slot) +{ + slot->host->mmc->caps |= MMC_CAP_8_BIT_DATA; + return 0; +} + #ifdef CONFIG_PM_RUNTIME static irqreturn_t sdhci_pci_sd_cd(int irq, void *dev_id) @@ -281,6 +293,11 @@ static const struct sdhci_pci_fixes sdhci_intel_mfd_emmc = { .probe_slot = mfd_emmc_probe_slot, }; +static const struct sdhci_pci_fixes sdhci_intel_pch_sdio = { + .quirks = SDHCI_QUIRK_BROKEN_ADMA, + .probe_slot = pch_hc_probe_slot, +}; + /* O2Micro extra registers */ #define O2_SD_LOCK_WP 0xD3 #define O2_SD_MULTI_VCC3V 0xEE @@ -817,6 +834,22 @@ static const struct pci_device_id pci_ids[] __devinitdata = { }, { + .vendor = PCI_VENDOR_ID_INTEL, + .device = PCI_DEVICE_ID_INTEL_PCH_SDIO0, + .subvendor = PCI_ANY_ID, + .subdevice = PCI_ANY_ID, + .driver_data = (kernel_ulong_t)&sdhci_intel_pch_sdio, + }, + + { + .vendor = PCI_VENDOR_ID_INTEL, + .device = PCI_DEVICE_ID_INTEL_PCH_SDIO1, + .subvendor = PCI_ANY_ID, + .subdevice = PCI_ANY_ID, + .driver_data = (kernel_ulong_t)&sdhci_intel_pch_sdio, + }, + + { .vendor = PCI_VENDOR_ID_O2, .device = PCI_DEVICE_ID_O2_8120, .subvendor = PCI_ANY_ID, -- 1.7.3.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] mmc: sdhci-pci: allow 8-bit bus width for Intel PCH 2012-03-13 17:16 ` [PATCH 3/3] mmc: sdhci-pci: allow 8-bit bus width for Intel PCH Alexander Stein 2012-03-13 17:25 ` Greg KH @ 2012-03-14 1:17 ` Tomoya MORINAGA 2012-03-14 7:26 ` Alexander Stein 1 sibling, 1 reply; 13+ messages in thread From: Tomoya MORINAGA @ 2012-03-14 1:17 UTC (permalink / raw) To: Alexander Stein Cc: Chris Ball, Jesse Barnes, Adrian Hunter, linux-mmc, linux-kernel, linux-pci Hi Alexander As a matter of interest, do you mean that current MMC of eg20t on Linux doesn't support 8-bit access? thanks, -- ROHM Co., Ltd. tomoya On Wed, Mar 14, 2012 at 2:16 AM, Alexander Stein <alexander.stein@systec-electronic.com> wrote: > Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com> > --- > drivers/mmc/host/sdhci-pci.c | 27 +++++++++++++++++++++++++++ > include/linux/pci_ids.h | 2 ++ > 2 files changed, 29 insertions(+), 0 deletions(-) > > diff --git a/drivers/mmc/host/sdhci-pci.c b/drivers/mmc/host/sdhci-pci.c > index 9382f27..0218cb2 100644 > --- a/drivers/mmc/host/sdhci-pci.c > +++ b/drivers/mmc/host/sdhci-pci.c > @@ -172,6 +172,12 @@ static int mrst_hc_probe(struct sdhci_pci_chip *chip) > return 0; > } > > +static int pch_hc_probe_slot(struct sdhci_pci_slot *slot) > +{ > + slot->host->mmc->caps |= MMC_CAP_8_BIT_DATA; > + return 0; > +} > + > #ifdef CONFIG_PM_RUNTIME > > static irqreturn_t sdhci_pci_sd_cd(int irq, void *dev_id) > @@ -281,6 +287,11 @@ static const struct sdhci_pci_fixes sdhci_intel_mfd_emmc = { > .probe_slot = mfd_emmc_probe_slot, > }; > > +static const struct sdhci_pci_fixes sdhci_intel_pch_sdio = { > + .quirks = SDHCI_QUIRK_BROKEN_ADMA, > + .probe_slot = pch_hc_probe_slot, > +}; > + > /* O2Micro extra registers */ > #define O2_SD_LOCK_WP 0xD3 > #define O2_SD_MULTI_VCC3V 0xEE > @@ -817,6 +828,22 @@ static const struct pci_device_id pci_ids[] __devinitdata = { > }, > > { > + .vendor = PCI_VENDOR_ID_INTEL, > + .device = PCI_DEVICE_ID_INTEL_PCH_SDIO0, > + .subvendor = PCI_ANY_ID, > + .subdevice = PCI_ANY_ID, > + .driver_data = (kernel_ulong_t)&sdhci_intel_pch_sdio, > + }, > + > + { > + .vendor = PCI_VENDOR_ID_INTEL, > + .device = PCI_DEVICE_ID_INTEL_PCH_SDIO1, > + .subvendor = PCI_ANY_ID, > + .subdevice = PCI_ANY_ID, > + .driver_data = (kernel_ulong_t)&sdhci_intel_pch_sdio, > + }, > + > + { > .vendor = PCI_VENDOR_ID_O2, > .device = PCI_DEVICE_ID_O2_8120, > .subvendor = PCI_ANY_ID, > diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h > index 31d77af..28cd019 100644 > --- a/include/linux/pci_ids.h > +++ b/include/linux/pci_ids.h > @@ -2800,6 +2800,8 @@ > #define PCI_DEVICE_ID_INTEL_82454NX 0x84cb > #define PCI_DEVICE_ID_INTEL_84460GX 0x84ea > #define PCI_DEVICE_ID_INTEL_IXP4XX 0x8500 > +#define PCI_DEVICE_ID_INTEL_PCH_SDIO0 0x8809 > +#define PCI_DEVICE_ID_INTEL_PCH_SDIO1 0x880a > #define PCI_DEVICE_ID_INTEL_IXP2800 0x9004 > #define PCI_DEVICE_ID_INTEL_S21152BB 0xb152 > > -- > 1.7.3.4 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] mmc: sdhci-pci: allow 8-bit bus width for Intel PCH 2012-03-14 1:17 ` [PATCH 3/3] " Tomoya MORINAGA @ 2012-03-14 7:26 ` Alexander Stein 0 siblings, 0 replies; 13+ messages in thread From: Alexander Stein @ 2012-03-14 7:26 UTC (permalink / raw) To: Tomoya MORINAGA Cc: Chris Ball, Jesse Barnes, Adrian Hunter, linux-mmc, linux-kernel, linux-pci Hello Tomoya, Am Mittwoch, 14. März 2012, 10:17:35 schrieb Tomoya MORINAGA: > Hi Alexander > > As a matter of interest, > do you mean that current MMC of eg20t on Linux doesn't support 8-bit access? We have a board with an 8-bit connected eMMC and Linux only used 4-bit wide bus access. After some research IMO the device is only matched to the default PCI id class > PCI_DEVICE_CLASS((PCI_CLASS_SYSTEM_SDHCI << 8), 0xFFFF00) AFAICS it's the same issue with the Medfield eMMC where 8-bit bus support is added in 0d013bcf5c272faea1f8e7a5ef3cb2e98103d5cb This also states the default is 4-bit bus width which I saw before. Regards, Alexander -- Dipl.-Inf. Alexander Stein SYS TEC electronic GmbH August-Bebel-Str. 29 D-07973 Greiz Tel: +49-3661-6279-0, Fax: +49-3661-6279-99 eMail: Alexander.Stein@systec-electronic.com Internet: http://www.systec-electronic.com Managing Director: Dipl.-Phys. Siegmar Schmidt Commercial registry: Amtsgericht Jena, HRB 205563 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] mmc: sdhci-pci: Add MSI support 2012-03-13 17:16 [PATCH 1/3] mmc: sdhci-pci: Add MSI support Alexander Stein 2012-03-13 17:16 ` [PATCH 2/3] mmc: sdhci: check interrupt flags in ISR again Alexander Stein 2012-03-13 17:16 ` [PATCH 3/3] mmc: sdhci-pci: allow 8-bit bus width for Intel PCH Alexander Stein @ 2012-03-16 3:41 ` Chris Ball 2 siblings, 0 replies; 13+ messages in thread From: Chris Ball @ 2012-03-16 3:41 UTC (permalink / raw) To: Alexander Stein Cc: Jesse Barnes, Adrian Hunter, linux-mmc, linux-kernel, linux-pci Hi Alexander, On Tue, Mar 13 2012, Alexander Stein wrote: > Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com> > --- > drivers/mmc/host/sdhci-pci.c | 6 ++++++ > 1 files changed, 6 insertions(+), 0 deletions(-) > > diff --git a/drivers/mmc/host/sdhci-pci.c b/drivers/mmc/host/sdhci-pci.c > index 7165e6a..9382f27 100644 > --- a/drivers/mmc/host/sdhci-pci.c > +++ b/drivers/mmc/host/sdhci-pci.c > @@ -1379,6 +1379,8 @@ static int __devinit sdhci_pci_probe(struct pci_dev *pdev, > > slots = chip->num_slots; /* Quirk may have changed this */ > > + pci_enable_msi(pdev); > + > for (i = 0; i < slots; i++) { > slot = sdhci_pci_probe_slot(pdev, chip, first_bar, i); > if (IS_ERR(slot)) { > @@ -1397,6 +1399,8 @@ static int __devinit sdhci_pci_probe(struct pci_dev *pdev, > return 0; > > free: > + pci_disable_msi(pdev); > + > pci_set_drvdata(pdev, NULL); > kfree(chip); > > @@ -1419,6 +1423,8 @@ static void __devexit sdhci_pci_remove(struct pci_dev *pdev) > for (i = 0; i < chip->num_slots; i++) > sdhci_pci_remove_slot(chip->slots[i]); > > + pci_disable_msi(pdev); > + > pci_set_drvdata(pdev, NULL); > kfree(chip); > } Thanks, I've merged all three patches to mmc-next for 3.4, using v2 of patches 2 and 3, and adding Adrian's ACK to patch 2. - Chris. -- Chris Ball <cjb@laptop.org> <http://printf.net/> One Laptop Per Child ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2012-03-16 3:42 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-03-13 17:16 [PATCH 1/3] mmc: sdhci-pci: Add MSI support Alexander Stein 2012-03-13 17:16 ` [PATCH 2/3] mmc: sdhci: check interrupt flags in ISR again Alexander Stein 2012-03-14 7:39 ` Adrian Hunter 2012-03-14 7:53 ` Alexander Stein 2012-03-14 8:13 ` Adrian Hunter 2012-03-14 8:52 ` [PATCH v2] " Alexander Stein 2012-03-14 9:23 ` Adrian Hunter 2012-03-13 17:16 ` [PATCH 3/3] mmc: sdhci-pci: allow 8-bit bus width for Intel PCH Alexander Stein 2012-03-13 17:25 ` Greg KH 2012-03-14 7:38 ` [PATCH v2] " Alexander Stein 2012-03-14 1:17 ` [PATCH 3/3] " Tomoya MORINAGA 2012-03-14 7:26 ` Alexander Stein 2012-03-16 3:41 ` [PATCH 1/3] mmc: sdhci-pci: Add MSI support Chris Ball
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).