The Samsung SDHCI controller block seems to fail to generate an
INT_DATA_END after the transfer has completed and the bus busy
state finished.
Changes in e809517f6fa5803a5a1cd56026f0e2190fc13d5c to use the
new busy method are the cause of the behaviour change.
Signed-off-by: Ben Dooks <[email protected]>
Index: linux.git/drivers/mmc/host/sdhci.c
===================================================================
--- linux.git.orig/drivers/mmc/host/sdhci.c 2008-11-03 10:01:03.000000000 +0000
+++ linux.git/drivers/mmc/host/sdhci.c 2008-11-03 12:20:40.000000000 +0000
@@ -1291,11 +1291,24 @@ static void sdhci_cmd_irq(struct sdhci_h
* controllers.
*/
if (host->cmd->flags & MMC_RSP_BUSY) {
+ u32 present;
+
if (host->cmd->data)
DBG("Cannot wait for busy signal when also "
"doing a data transfer");
- else
+ else if (!(host->quirks & SDHCI_QUIRK_NO_TCIRQ_ON_NOT_BUSY))
return;
+
+ /* The Samsung SDHCI does not seem to provide an INT_DATA_END
+ * when the system goes non-busy, so check the state of the
+ * transfer by reading SDHCI_PRESENT_STATE to see if the
+ * controller is ready
+ */
+
+ present = readl(host->ioaddr + SDHCI_PRESENT_STATE);
+ DBG("busy? present %08x, intstat %08x\n", present, intmask);
+
+ /* fall through and take the SDHCI_INT_RESPONSE */
}
if (intmask & SDHCI_INT_RESPONSE)
Index: linux.git/drivers/mmc/host/sdhci.h
===================================================================
--- linux.git.orig/drivers/mmc/host/sdhci.h 2008-11-03 10:01:03.000000000 +0000
+++ linux.git/drivers/mmc/host/sdhci.h 2008-11-03 12:20:00.000000000 +0000
@@ -57,6 +57,7 @@
#define SDHCI_DATA_AVAILABLE 0x00000800
#define SDHCI_CARD_PRESENT 0x00010000
#define SDHCI_WRITE_PROTECT 0x00080000
+#define SDHCI_DATA_BIT(x) (1 << ((x) + 20))
#define SDHCI_HOST_CONTROL 0x28
#define SDHCI_CTRL_LED 0x01
@@ -210,6 +211,8 @@ struct sdhci_host {
#define SDHCI_QUIRK_BROKEN_SMALL_PIO (1<<13)
/* Controller supports high speed but doesn't have the caps bit set */
#define SDHCI_QUIRK_FORCE_HIGHSPEED (1<<14)
+/* Controller does not provide transfer-complete interrupt when not busy */
+#define SDHCI_QUIRK_NO_TCIRQ_ON_NOT_BUSY (1<<15)
int irq; /* Device IRQ */
void __iomem * ioaddr; /* Mapped address */
--
Ben ([email protected], http://www.fluff.org/)
'a smiley only costs 4 bytes'
On Mon, 03 Nov 2008 20:09:48 +0000
Ben Dooks <[email protected]> wrote:
> The Samsung SDHCI controller block seems to fail to generate an
> INT_DATA_END after the transfer has completed and the bus busy
> state finished.
>
> Changes in e809517f6fa5803a5a1cd56026f0e2190fc13d5c to use the
> new busy method are the cause of the behaviour change.
>
> Signed-off-by: Ben Dooks <[email protected]>
>
> Index: linux.git/drivers/mmc/host/sdhci.c
> ===================================================================
> --- linux.git.orig/drivers/mmc/host/sdhci.c 2008-11-03 10:01:03.000000000 +0000
> +++ linux.git/drivers/mmc/host/sdhci.c 2008-11-03 12:20:40.000000000 +0000
> @@ -1291,11 +1291,24 @@ static void sdhci_cmd_irq(struct sdhci_h
> * controllers.
> */
> if (host->cmd->flags & MMC_RSP_BUSY) {
> + u32 present;
> +
> if (host->cmd->data)
> DBG("Cannot wait for busy signal when also "
> "doing a data transfer");
> - else
> + else if (!(host->quirks & SDHCI_QUIRK_NO_TCIRQ_ON_NOT_BUSY))
> return;
Not the clearest naming I've ever seen. :)
How about NO_BUSY_IRQ?
> +
> + /* The Samsung SDHCI does not seem to provide an INT_DATA_END
> + * when the system goes non-busy, so check the state of the
> + * transfer by reading SDHCI_PRESENT_STATE to see if the
> + * controller is ready
> + */
There is already a note about this being a problem earlier up, so I
don't think we need another.
> +
> + present = readl(host->ioaddr + SDHCI_PRESENT_STATE);
> + DBG("busy? present %08x, intstat %08x\n", present, intmask);
> +
And what does this add really? Controllers not being able to wait for
the busy state to end is so common that we can just ignore the problem
here.
> +#define SDHCI_DATA_BIT(x) (1 << ((x) + 20))
This was not supposed to be included I suppose :)
Rgds
--
-- Pierre Ossman
Linux kernel, MMC maintainer http://www.kernel.org
rdesktop, core developer http://www.rdesktop.org
WARNING: This correspondence is being monitored by the
Swedish government. Make sure your server uses encryption
for SMTP traffic and consider using PGP for end-to-end
encryption.
On Fri, Nov 14, 2008 at 10:41:14PM +0100, Pierre Ossman wrote:
> On Mon, 03 Nov 2008 20:09:48 +0000
> Ben Dooks <[email protected]> wrote:
>
> > The Samsung SDHCI controller block seems to fail to generate an
> > INT_DATA_END after the transfer has completed and the bus busy
> > state finished.
> >
> > Changes in e809517f6fa5803a5a1cd56026f0e2190fc13d5c to use the
> > new busy method are the cause of the behaviour change.
> >
> > Signed-off-by: Ben Dooks <[email protected]>
> >
> > Index: linux.git/drivers/mmc/host/sdhci.c
> > ===================================================================
> > --- linux.git.orig/drivers/mmc/host/sdhci.c 2008-11-03 10:01:03.000000000 +0000
> > +++ linux.git/drivers/mmc/host/sdhci.c 2008-11-03 12:20:40.000000000 +0000
> > @@ -1291,11 +1291,24 @@ static void sdhci_cmd_irq(struct sdhci_h
> > * controllers.
> > */
> > if (host->cmd->flags & MMC_RSP_BUSY) {
> > + u32 present;
> > +
> > if (host->cmd->data)
> > DBG("Cannot wait for busy signal when also "
> > "doing a data transfer");
> > - else
> > + else if (!(host->quirks & SDHCI_QUIRK_NO_TCIRQ_ON_NOT_BUSY))
> > return;
>
> Not the clearest naming I've ever seen. :)
> How about NO_BUSY_IRQ?
ok, will change.
> > +
> > + /* The Samsung SDHCI does not seem to provide an INT_DATA_END
> > + * when the system goes non-busy, so check the state of the
> > + * transfer by reading SDHCI_PRESENT_STATE to see if the
> > + * controller is ready
> > + */
>
> There is already a note about this being a problem earlier up, so I
> don't think we need another.
right.
> > +
> > + present = readl(host->ioaddr + SDHCI_PRESENT_STATE);
> > + DBG("busy? present %08x, intstat %08x\n", present, intmask);
> > +
>
> And what does this add really? Controllers not being able to wait for
> the busy state to end is so common that we can just ignore the problem
> here.
>
that was debug.
> > +#define SDHCI_DATA_BIT(x) (1 << ((x) + 20))
>
> This was not supposed to be included I suppose :)
was with teh debug.
--
Ben ([email protected], http://www.fluff.org/)
'a smiley only costs 4 bytes'