* [PATCH 2/2] mmc: dw_mmc: make sure of clearing HLE interrupt @ 2012-11-28 10:26 Seungwon Jeon 2012-11-28 10:45 ` James Hogan 0 siblings, 1 reply; 5+ messages in thread From: Seungwon Jeon @ 2012-11-28 10:26 UTC (permalink / raw) To: linux-mmc Cc: 'Chris Ball', 'Will Newton', 'James Hogan', 'Jaehoon Chung' Even though HLE interrupt is enabled, there is no touch. This patch clears HLE interrupt which is not unhandled. Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com> --- drivers/mmc/host/dw_mmc.c | 11 +++++++++++ 1 files changed, 11 insertions(+), 0 deletions(-) diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c index 6785d62..b6db0ae 100644 --- a/drivers/mmc/host/dw_mmc.c +++ b/drivers/mmc/host/dw_mmc.c @@ -1009,6 +1009,11 @@ static void dw_mci_tasklet_func(unsigned long priv) state = host->state; data = host->data; + if (host->cmd_status & SDMMC_INT_HLE) { + dev_err(host->dev, "hardware locked write error\n"); + goto unlock; + } + do { prev_state = state; @@ -1577,6 +1582,12 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id) if (!pending) break; + if (pending & SDMMC_INT_HLE) { + mci_writel(host, RINTSTS, SDMMC_INT_HLE); + host->cmd_status = pending; + tasklet_schedule(&host->tasklet); + } + if (pending & DW_MCI_CMD_ERROR_FLAGS) { mci_writel(host, RINTSTS, DW_MCI_CMD_ERROR_FLAGS); host->cmd_status = pending; -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] mmc: dw_mmc: make sure of clearing HLE interrupt 2012-11-28 10:26 [PATCH 2/2] mmc: dw_mmc: make sure of clearing HLE interrupt Seungwon Jeon @ 2012-11-28 10:45 ` James Hogan 2012-11-29 7:35 ` Seungwon Jeon 0 siblings, 1 reply; 5+ messages in thread From: James Hogan @ 2012-11-28 10:45 UTC (permalink / raw) To: Seungwon Jeon Cc: linux-mmc, 'Chris Ball', 'Will Newton', 'Jaehoon Chung' Hi, On 28/11/12 10:26, Seungwon Jeon wrote: > Even though HLE interrupt is enabled, there is no touch. > This patch clears HLE interrupt which is not unhandled. It's not entirely clear from this description what the patch is trying to do. I presume from the patch you're trying to say something like: "Even though the HLE interrupt is enabled, it isn't handled, so handle the HLE interrupt by printing an error message." According to the TRM though, in the section Error Handling (HLE is also mentioned elsewhere): > Hardware locked error – Set when the DWC_mobile_storage cannot load a command issued by > software. When software sets the start_cmd bit in the CMD register, the DWC_mobile_storage tries > to load the command. If the command buffer is already filled with a command, this error is raised. > The software then has to reload the command. So it sounds like the last command should be reloaded (either on interrupt or the interrupt status should be checked after starting a command). Is this done elsewhere already? Cheers James > > Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com> > --- > drivers/mmc/host/dw_mmc.c | 11 +++++++++++ > 1 files changed, 11 insertions(+), 0 deletions(-) > > diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c > index 6785d62..b6db0ae 100644 > --- a/drivers/mmc/host/dw_mmc.c > +++ b/drivers/mmc/host/dw_mmc.c > @@ -1009,6 +1009,11 @@ static void dw_mci_tasklet_func(unsigned long priv) > state = host->state; > data = host->data; > > + if (host->cmd_status & SDMMC_INT_HLE) { > + dev_err(host->dev, "hardware locked write error\n"); > + goto unlock; > + } > + > do { > prev_state = state; > > @@ -1577,6 +1582,12 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id) > if (!pending) > break; > > + if (pending & SDMMC_INT_HLE) { > + mci_writel(host, RINTSTS, SDMMC_INT_HLE); > + host->cmd_status = pending; > + tasklet_schedule(&host->tasklet); > + } > + > if (pending & DW_MCI_CMD_ERROR_FLAGS) { > mci_writel(host, RINTSTS, DW_MCI_CMD_ERROR_FLAGS); > host->cmd_status = pending; > ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH 2/2] mmc: dw_mmc: make sure of clearing HLE interrupt 2012-11-28 10:45 ` James Hogan @ 2012-11-29 7:35 ` Seungwon Jeon 2012-11-29 14:59 ` Jae hoon Chung 0 siblings, 1 reply; 5+ messages in thread From: Seungwon Jeon @ 2012-11-29 7:35 UTC (permalink / raw) To: 'James Hogan' Cc: linux-mmc, 'Chris Ball', 'Will Newton', 'Jaehoon Chung' Hi James, On Wednesday, November 28, 2012, James Hogan wrote: > Hi, > > On 28/11/12 10:26, Seungwon Jeon wrote: > > Even though HLE interrupt is enabled, there is no touch. > > This patch clears HLE interrupt which is not unhandled. > > It's not entirely clear from this description what the patch is trying > to do. I presume from the patch you're trying to say something like: > "Even though the HLE interrupt is enabled, it isn't handled, so handle > the HLE interrupt by printing an error message." Currently, there is no code to clean the HLE interrupt, when it happens. So, interrupt will signaled permanently and then system is affected badly. The purpose of this patch to prevent this. Printing an message is just to inform error. > > According to the TRM though, in the section Error Handling (HLE is also > mentioned elsewhere): > > Hardware locked error – Set when the DWC_mobile_storage cannot load a command issued by > > software. When software sets the start_cmd bit in the CMD register, the DWC_mobile_storage tries > > to load the command. If the command buffer is already filled with a command, this error is raised. > > The software then has to reload the command. > > So it sounds like the last command should be reloaded (either on > interrupt or the interrupt status should be checked after starting a > command). Is this done elsewhere already? I didn't see that for normal command. We need to investigate about HLE case more. It's difficult to meet HLE. For this, I think this patch is effective at least. If you have any good idea, please let me know. Thanks, Seungwon Jeon > > Cheers > James > > > > > Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com> > > --- > > drivers/mmc/host/dw_mmc.c | 11 +++++++++++ > > 1 files changed, 11 insertions(+), 0 deletions(-) > > > > diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c > > index 6785d62..b6db0ae 100644 > > --- a/drivers/mmc/host/dw_mmc.c > > +++ b/drivers/mmc/host/dw_mmc.c > > @@ -1009,6 +1009,11 @@ static void dw_mci_tasklet_func(unsigned long priv) > > state = host->state; > > data = host->data; > > > > + if (host->cmd_status & SDMMC_INT_HLE) { > > + dev_err(host->dev, "hardware locked write error\n"); > > + goto unlock; > > + } > > + > > do { > > prev_state = state; > > > > @@ -1577,6 +1582,12 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id) > > if (!pending) > > break; > > > > + if (pending & SDMMC_INT_HLE) { > > + mci_writel(host, RINTSTS, SDMMC_INT_HLE); > > + host->cmd_status = pending; > > + tasklet_schedule(&host->tasklet); > > + } > > + > > if (pending & DW_MCI_CMD_ERROR_FLAGS) { > > mci_writel(host, RINTSTS, DW_MCI_CMD_ERROR_FLAGS); > > host->cmd_status = pending; > > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] mmc: dw_mmc: make sure of clearing HLE interrupt 2012-11-29 7:35 ` Seungwon Jeon @ 2012-11-29 14:59 ` Jae hoon Chung 2012-11-30 11:49 ` Seungwon Jeon 0 siblings, 1 reply; 5+ messages in thread From: Jae hoon Chung @ 2012-11-29 14:59 UTC (permalink / raw) To: Seungwon Jeon Cc: James Hogan, linux-mmc, Chris Ball, Will Newton, Jaehoon Chung 2012/11/29 Seungwon Jeon <tgih.jun@samsung.com>: > Hi James, > > On Wednesday, November 28, 2012, James Hogan wrote: >> Hi, >> >> On 28/11/12 10:26, Seungwon Jeon wrote: >> > Even though HLE interrupt is enabled, there is no touch. >> > This patch clears HLE interrupt which is not unhandled. >> >> It's not entirely clear from this description what the patch is trying >> to do. I presume from the patch you're trying to say something like: >> "Even though the HLE interrupt is enabled, it isn't handled, so handle >> the HLE interrupt by printing an error message." > Currently, there is no code to clean the HLE interrupt, when it happens. > So, interrupt will signaled permanently and then system is affected badly. > The purpose of this patch to prevent this. Printing an message is just to inform error. > >> >> According to the TRM though, in the section Error Handling (HLE is also >> mentioned elsewhere): >> > Hardware locked error – Set when the DWC_mobile_storage cannot load a command issued by >> > software. When software sets the start_cmd bit in the CMD register, the DWC_mobile_storage tries >> > to load the command. If the command buffer is already filled with a command, this error is raised. >> > The software then has to reload the command. >> >> So it sounds like the last command should be reloaded (either on >> interrupt or the interrupt status should be checked after starting a >> command). Is this done elsewhere already? > I didn't see that for normal command. > We need to investigate about HLE case more. It's difficult to meet HLE. > For this, I think this patch is effective at least. > If you have any good idea, please let me know. As mentioned at spec, if raised HLE when buffer is already filled with a command and try to load the other command, how about the clear the buffer and reload the command? If we need to consider others, i will also think this problem. Best Regards, Jaehoon Chung > > Thanks, > Seungwon Jeon >> >> Cheers >> James >> >> > >> > Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com> >> > --- >> > drivers/mmc/host/dw_mmc.c | 11 +++++++++++ >> > 1 files changed, 11 insertions(+), 0 deletions(-) >> > >> > diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c >> > index 6785d62..b6db0ae 100644 >> > --- a/drivers/mmc/host/dw_mmc.c >> > +++ b/drivers/mmc/host/dw_mmc.c >> > @@ -1009,6 +1009,11 @@ static void dw_mci_tasklet_func(unsigned long priv) >> > state = host->state; >> > data = host->data; >> > >> > + if (host->cmd_status & SDMMC_INT_HLE) { >> > + dev_err(host->dev, "hardware locked write error\n"); >> > + goto unlock; >> > + } >> > + >> > do { >> > prev_state = state; >> > >> > @@ -1577,6 +1582,12 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id) >> > if (!pending) >> > break; >> > >> > + if (pending & SDMMC_INT_HLE) { >> > + mci_writel(host, RINTSTS, SDMMC_INT_HLE); >> > + host->cmd_status = pending; >> > + tasklet_schedule(&host->tasklet); >> > + } >> > + >> > if (pending & DW_MCI_CMD_ERROR_FLAGS) { >> > mci_writel(host, RINTSTS, DW_MCI_CMD_ERROR_FLAGS); >> > host->cmd_status = pending; >> > >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-mmc" 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-mmc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH 2/2] mmc: dw_mmc: make sure of clearing HLE interrupt 2012-11-29 14:59 ` Jae hoon Chung @ 2012-11-30 11:49 ` Seungwon Jeon 0 siblings, 0 replies; 5+ messages in thread From: Seungwon Jeon @ 2012-11-30 11:49 UTC (permalink / raw) To: 'Jae hoon Chung' Cc: 'James Hogan', linux-mmc, 'Chris Ball', 'Will Newton', 'Jaehoon Chung' On Thursday, November 29, 2012, Jae hoon Chung wrote: > 2012/11/29 Seungwon Jeon <tgih.jun@samsung.com>: > > Hi James, > > > > On Wednesday, November 28, 2012, James Hogan wrote: > >> Hi, > >> > >> On 28/11/12 10:26, Seungwon Jeon wrote: > >> > Even though HLE interrupt is enabled, there is no touch. > >> > This patch clears HLE interrupt which is not unhandled. > >> > >> It's not entirely clear from this description what the patch is trying > >> to do. I presume from the patch you're trying to say something like: > >> "Even though the HLE interrupt is enabled, it isn't handled, so handle > >> the HLE interrupt by printing an error message." > > Currently, there is no code to clean the HLE interrupt, when it happens. > > So, interrupt will signaled permanently and then system is affected badly. > > The purpose of this patch to prevent this. Printing an message is just to inform error. > > > >> > >> According to the TRM though, in the section Error Handling (HLE is also > >> mentioned elsewhere): > >> > Hardware locked error – Set when the DWC_mobile_storage cannot load a command issued by > >> > software. When software sets the start_cmd bit in the CMD register, the DWC_mobile_storage tries > >> > to load the command. If the command buffer is already filled with a command, this error is raised. > >> > The software then has to reload the command. > >> > >> So it sounds like the last command should be reloaded (either on > >> interrupt or the interrupt status should be checked after starting a > >> command). Is this done elsewhere already? > > I didn't see that for normal command. > > We need to investigate about HLE case more. It's difficult to meet HLE. > > For this, I think this patch is effective at least. > > If you have any good idea, please let me know. > As mentioned at spec, if raised HLE when buffer is already filled with > a command and try to load the other command, > how about the clear the buffer and reload the command? > If we need to consider others, i will also think this problem. I focused to clear unhanded interrupt in this patch. Do you mean retry for command after clearing HLE? I think HLE seems not happen during normal command actually. We may need to check the happening route of HLE. Please let me know if you have other opinion. Thanks, Seungwon Jeon > > Best Regards, > Jaehoon Chung > > > > Thanks, > > Seungwon Jeon > >> > >> Cheers > >> James > >> > >> > > >> > Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com> > >> > --- > >> > drivers/mmc/host/dw_mmc.c | 11 +++++++++++ > >> > 1 files changed, 11 insertions(+), 0 deletions(-) > >> > > >> > diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c > >> > index 6785d62..b6db0ae 100644 > >> > --- a/drivers/mmc/host/dw_mmc.c > >> > +++ b/drivers/mmc/host/dw_mmc.c > >> > @@ -1009,6 +1009,11 @@ static void dw_mci_tasklet_func(unsigned long priv) > >> > state = host->state; > >> > data = host->data; > >> > > >> > + if (host->cmd_status & SDMMC_INT_HLE) { > >> > + dev_err(host->dev, "hardware locked write error\n"); > >> > + goto unlock; > >> > + } > >> > + > >> > do { > >> > prev_state = state; > >> > > >> > @@ -1577,6 +1582,12 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id) > >> > if (!pending) > >> > break; > >> > > >> > + if (pending & SDMMC_INT_HLE) { > >> > + mci_writel(host, RINTSTS, SDMMC_INT_HLE); > >> > + host->cmd_status = pending; > >> > + tasklet_schedule(&host->tasklet); > >> > + } > >> > + > >> > if (pending & DW_MCI_CMD_ERROR_FLAGS) { > >> > mci_writel(host, RINTSTS, DW_MCI_CMD_ERROR_FLAGS); > >> > host->cmd_status = pending; > >> > > >> > >> -- > >> To unsubscribe from this list: send the line "unsubscribe linux-mmc" 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-mmc" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-11-30 11:50 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-11-28 10:26 [PATCH 2/2] mmc: dw_mmc: make sure of clearing HLE interrupt Seungwon Jeon 2012-11-28 10:45 ` James Hogan 2012-11-29 7:35 ` Seungwon Jeon 2012-11-29 14:59 ` Jae hoon Chung 2012-11-30 11:49 ` Seungwon Jeon
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.