diff mbox

mmc: dw_mmc: enable controller interrupt before calling mmc_start_host

Message ID 1349686791-19652-1-git-send-email-yuvaraj.cd@samsung.com
State Accepted
Commit 2da1d7f2948900cd50d38643db39f790edb3cc96
Headers show

Commit Message

Yuvaraj Oct. 8, 2012, 8:59 a.m. UTC
As mmc_start_host is getting called before enabling the dw_mmc controller
interrupt, there is a problem of missing the SDMMC_INT_CMD_DONE for the
very first command sent by the sdio_reset.
This problem occurs only when we disable MMC debugging i.e, MMC_DEBUG [=n].
Hence this patch enables the dw_mmc controller interrupt before mmc_start_host.

Signed-off-by: Yuvaraj CD <yuvaraj.cd@samsung.com>
---
 drivers/mmc/host/dw_mmc.c |   29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

Comments

Girish K S Oct. 8, 2012, 12:55 p.m. UTC | #1
On 8 October 2012 17:59, Yuvaraj CD <yuvaraj.cd@gmail.com> wrote:
> As mmc_start_host is getting called before enabling the dw_mmc controller
> interrupt, there is a problem of missing the SDMMC_INT_CMD_DONE for the
> very first command sent by the sdio_reset.
> This problem occurs only when we disable MMC debugging i.e, MMC_DEBUG [=n].
> Hence this patch enables the dw_mmc controller interrupt before mmc_start_host.
>
> Signed-off-by: Yuvaraj CD <yuvaraj.cd@samsung.com>
> ---
>  drivers/mmc/host/dw_mmc.c |   29 +++++++++++++++--------------
>  1 file changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index a23af77..729c031 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -2233,6 +2233,21 @@ int dw_mci_probe(struct dw_mci *host)
>         else
>                 host->num_slots = ((mci_readl(host, HCON) >> 1) & 0x1F) + 1;
>
> +       /*
> +        * Enable interrupts for command done, data over, data empty, card det,
> +        * receive ready and error such as transmit, receive timeout, crc error
> +        */
> +       mci_writel(host, RINTSTS, 0xFFFFFFFF);
> +       mci_writel(host, INTMASK, SDMMC_INT_CMD_DONE | SDMMC_INT_DATA_OVER |
> +                  SDMMC_INT_TXDR | SDMMC_INT_RXDR |
> +                  DW_MCI_ERROR_FLAGS | SDMMC_INT_CD);
> +       mci_writel(host, CTRL, SDMMC_CTRL_INT_ENABLE); /* Enable mci interrupt */
> +
> +       dev_info(host->dev, "DW MMC controller at irq %d, "
> +                "%d bit host data width, "
> +                "%u deep fifo\n",
> +                host->irq, width, fifo_size);
> +
Makes sense
Reviewed By: Girish K S <girish.shivananjappa@linaro.org>
>         /* We need at least one slot to succeed */
>         for (i = 0; i < host->num_slots; i++) {
>                 ret = dw_mci_init_slot(host, i);
> @@ -2262,20 +2277,6 @@ int dw_mci_probe(struct dw_mci *host)
>         else
>                 host->data_offset = DATA_240A_OFFSET;
>
> -       /*
> -        * Enable interrupts for command done, data over, data empty, card det,
> -        * receive ready and error such as transmit, receive timeout, crc error
> -        */
> -       mci_writel(host, RINTSTS, 0xFFFFFFFF);
> -       mci_writel(host, INTMASK, SDMMC_INT_CMD_DONE | SDMMC_INT_DATA_OVER |
> -                  SDMMC_INT_TXDR | SDMMC_INT_RXDR |
> -                  DW_MCI_ERROR_FLAGS | SDMMC_INT_CD);
> -       mci_writel(host, CTRL, SDMMC_CTRL_INT_ENABLE); /* Enable mci interrupt */
> -
> -       dev_info(host->dev, "DW MMC controller at irq %d, "
> -                "%d bit host data width, "
> -                "%u deep fifo\n",
> -                host->irq, width, fifo_size);
>         if (host->quirks & DW_MCI_QUIRK_IDMAC_DTO)
>                 dev_info(host->dev, "Internal DMAC interrupt fix enabled.\n");
>
> --
> 1.7.9.5
>
Will Newton Oct. 8, 2012, 12:58 p.m. UTC | #2
On Mon, Oct 8, 2012 at 9:59 AM, Yuvaraj CD <yuvaraj.cd@gmail.com> wrote:
> As mmc_start_host is getting called before enabling the dw_mmc controller
> interrupt, there is a problem of missing the SDMMC_INT_CMD_DONE for the
> very first command sent by the sdio_reset.
> This problem occurs only when we disable MMC debugging i.e, MMC_DEBUG [=n].
> Hence this patch enables the dw_mmc controller interrupt before mmc_start_host.
>
> Signed-off-by: Yuvaraj CD <yuvaraj.cd@samsung.com>
> ---
>  drivers/mmc/host/dw_mmc.c |   29 +++++++++++++++--------------
>  1 file changed, 15 insertions(+), 14 deletions(-)

Ideally I think it would be cleaner to break up the slot
initialization into two parts, one that allocates and sets up the slot
and the other that calls mmc_add_host, but that's a more substantial
bit of work and would also complicate the error handling. This looks
like it should be safe though, so:

Acked-by: Will Newton <will.newton@imgtec.com>
Chris Ball Oct. 23, 2012, 9:19 p.m. UTC | #3
Hi,

On Mon, Oct 08 2012, Yuvaraj CD wrote:
> As mmc_start_host is getting called before enabling the dw_mmc controller
> interrupt, there is a problem of missing the SDMMC_INT_CMD_DONE for the
> very first command sent by the sdio_reset.
> This problem occurs only when we disable MMC debugging i.e, MMC_DEBUG [=n].
> Hence this patch enables the dw_mmc controller interrupt before mmc_start_host.
>
> Signed-off-by: Yuvaraj CD <yuvaraj.cd@samsung.com>

Is "Yuvaraj CD" your legal name?  We should use your full name so that
we can track copyright attribution properly.

Thanks!

- Chris.
Yuvaraj CD Oct. 25, 2012, 10:09 a.m. UTC | #4
Yes,its legal name."Yuvaraj C D".

On Wed, Oct 24, 2012 at 2:49 AM, Chris Ball <cjb@laptop.org> wrote:
> Hi,
>
> On Mon, Oct 08 2012, Yuvaraj CD wrote:
>> As mmc_start_host is getting called before enabling the dw_mmc controller
>> interrupt, there is a problem of missing the SDMMC_INT_CMD_DONE for the
>> very first command sent by the sdio_reset.
>> This problem occurs only when we disable MMC debugging i.e, MMC_DEBUG [=n].
>> Hence this patch enables the dw_mmc controller interrupt before mmc_start_host.
>>
>> Signed-off-by: Yuvaraj CD <yuvaraj.cd@samsung.com>
>
> Is "Yuvaraj CD" your legal name?  We should use your full name so that
> we can track copyright attribution properly.
>
> Thanks!
>
> - Chris.
> --
> Chris Ball   <cjb@laptop.org>   <http://printf.net/>
> One Laptop Per Child
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chris Ball Oct. 25, 2012, 12:43 p.m. UTC | #5
Hi,

On Thu, Oct 25 2012, Yuvaraj CD wrote:
> Yes,its legal name."Yuvaraj C D".

Thanks for letting me know; pushed to mmc-next for 3.7.

- Chris.
diff mbox

Patch

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index a23af77..729c031 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2233,6 +2233,21 @@  int dw_mci_probe(struct dw_mci *host)
 	else
 		host->num_slots = ((mci_readl(host, HCON) >> 1) & 0x1F) + 1;
 
+	/*
+	 * Enable interrupts for command done, data over, data empty, card det,
+	 * receive ready and error such as transmit, receive timeout, crc error
+	 */
+	mci_writel(host, RINTSTS, 0xFFFFFFFF);
+	mci_writel(host, INTMASK, SDMMC_INT_CMD_DONE | SDMMC_INT_DATA_OVER |
+		   SDMMC_INT_TXDR | SDMMC_INT_RXDR |
+		   DW_MCI_ERROR_FLAGS | SDMMC_INT_CD);
+	mci_writel(host, CTRL, SDMMC_CTRL_INT_ENABLE); /* Enable mci interrupt */
+
+	dev_info(host->dev, "DW MMC controller at irq %d, "
+		 "%d bit host data width, "
+		 "%u deep fifo\n",
+		 host->irq, width, fifo_size);
+
 	/* We need at least one slot to succeed */
 	for (i = 0; i < host->num_slots; i++) {
 		ret = dw_mci_init_slot(host, i);
@@ -2262,20 +2277,6 @@  int dw_mci_probe(struct dw_mci *host)
 	else
 		host->data_offset = DATA_240A_OFFSET;
 
-	/*
-	 * Enable interrupts for command done, data over, data empty, card det,
-	 * receive ready and error such as transmit, receive timeout, crc error
-	 */
-	mci_writel(host, RINTSTS, 0xFFFFFFFF);
-	mci_writel(host, INTMASK, SDMMC_INT_CMD_DONE | SDMMC_INT_DATA_OVER |
-		   SDMMC_INT_TXDR | SDMMC_INT_RXDR |
-		   DW_MCI_ERROR_FLAGS | SDMMC_INT_CD);
-	mci_writel(host, CTRL, SDMMC_CTRL_INT_ENABLE); /* Enable mci interrupt */
-
-	dev_info(host->dev, "DW MMC controller at irq %d, "
-		 "%d bit host data width, "
-		 "%u deep fifo\n",
-		 host->irq, width, fifo_size);
 	if (host->quirks & DW_MCI_QUIRK_IDMAC_DTO)
 		dev_info(host->dev, "Internal DMAC interrupt fix enabled.\n");