| Message ID | 001c01cdcd52$e58a2260$b09e6720$%jun@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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; > -- 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
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 -- 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
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 -- 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
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 -- 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
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;
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(-)