* [PATCH] mmc: omap_hsmmc: Fix Oops in case of data errors
@ 2012-11-09 14:41 Balaji T K
2012-11-09 14:52 ` Felipe Balbi
0 siblings, 1 reply; 14+ messages in thread
From: Balaji T K @ 2012-11-09 14:41 UTC (permalink / raw)
To: linux-mmc, cjb; +Cc: linux-omap, svenkatr, Balaji T K
Setting end_cmd to 1 for Data Timeout/CRC leads to NULL pointer dereference of
host->cmd as the command complete has previously been handled.
Set end_cmd only in case of Data Timeout/CRC.
While at it restore error handling behaviour as was before the
"commit ae4bf788ee9bf7c2d51b0309117d1fcccbdd50a2
mmc: omap_hsmmc: consolidate error report handling of HSMMC IRQ"
host->cmd->error should not be updated on data error case, only
host->data->error needs to be updated.
end_trans and end_crc should not to be set together, to avoid
mmc_request_done being called twice in case of Command CRC or command
Timeout.
Avoid soft reset of command internal state machine on data errors.
Signed-off-by: Balaji T K <balajitk@ti.com>
---
based on mmc-fixes-for-3.7-rc5 in mmc_next
drivers/mmc/host/omap_hsmmc.c | 23 +++++++++++++++--------
1 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index fedd258..6ea1da3 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -968,15 +968,20 @@ static inline void omap_hsmmc_reset_controller_fsm(struct omap_hsmmc_host *host,
__func__);
}
-static void hsmmc_command_incomplete(struct omap_hsmmc_host *host, int err)
+static void hsmmc_command_incomplete(struct omap_hsmmc_host *host,
+ int err, int end_cmd)
{
- omap_hsmmc_reset_controller_fsm(host, SRC);
- host->cmd->error = err;
+ if (end_cmd) {
+ omap_hsmmc_reset_controller_fsm(host, SRC);
+ if (host->cmd)
+ host->cmd->error = err;
+ }
if (host->data) {
omap_hsmmc_reset_controller_fsm(host, SRD);
omap_hsmmc_dma_cleanup(host, err);
- }
+ } else if (host->mrq && host->mrq->cmd)
+ host->mrq->cmd->error = err;
}
@@ -990,14 +995,16 @@ static void omap_hsmmc_do_irq(struct omap_hsmmc_host *host, int status)
if (status & ERR) {
omap_hsmmc_dbg_report_irq(host, status);
+
+ if (status & (CMD_TIMEOUT | CMD_CRC))
+ end_cmd = 1;
if (status & (CMD_TIMEOUT | DATA_TIMEOUT))
- hsmmc_command_incomplete(host, -ETIMEDOUT);
+ hsmmc_command_incomplete(host, -ETIMEDOUT, end_cmd);
else if (status & (CMD_CRC | DATA_CRC))
- hsmmc_command_incomplete(host, -EILSEQ);
+ hsmmc_command_incomplete(host, -EILSEQ, end_cmd);
- end_cmd = 1;
if (host->data || host->response_busy) {
- end_trans = 1;
+ end_trans = !end_cmd;
host->response_busy = 0;
}
}
--
1.7.5.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH] mmc: omap_hsmmc: Fix Oops in case of data errors
2012-11-09 14:41 [PATCH] mmc: omap_hsmmc: Fix Oops in case of data errors Balaji T K
@ 2012-11-09 14:52 ` Felipe Balbi
2012-11-09 15:28 ` Balaji T K
2012-11-09 16:06 ` [PATCH v2 0/3] mmc: omap_hsmmc: fixes for irq error handler Balaji T K
0 siblings, 2 replies; 14+ messages in thread
From: Felipe Balbi @ 2012-11-09 14:52 UTC (permalink / raw)
To: Balaji T K; +Cc: linux-mmc, cjb, linux-omap, svenkatr
[-- Attachment #1: Type: text/plain, Size: 3133 bytes --]
On Fri, Nov 09, 2012 at 08:11:19PM +0530, Balaji T K wrote:
> Setting end_cmd to 1 for Data Timeout/CRC leads to NULL pointer dereference of
> host->cmd as the command complete has previously been handled.
> Set end_cmd only in case of Data Timeout/CRC.
this comment doesnt match code as code is setting end_cmd for cmd
timeout and cmd crc, not data timeout/crc.
> While at it restore error handling behaviour as was before the
> "commit ae4bf788ee9bf7c2d51b0309117d1fcccbdd50a2
> mmc: omap_hsmmc: consolidate error report handling of HSMMC IRQ"
>
> host->cmd->error should not be updated on data error case, only
> host->data->error needs to be updated.
> end_trans and end_crc should not to be set together, to avoid
> mmc_request_done being called twice in case of Command CRC or command
> Timeout.
> Avoid soft reset of command internal state machine on data errors.
looks to me you're doing to many changes in a single patch. Perhaps it
deserves some splitting...
>
> Signed-off-by: Balaji T K <balajitk@ti.com>
> ---
> based on mmc-fixes-for-3.7-rc5 in mmc_next
>
> drivers/mmc/host/omap_hsmmc.c | 23 +++++++++++++++--------
> 1 files changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index fedd258..6ea1da3 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -968,15 +968,20 @@ static inline void omap_hsmmc_reset_controller_fsm(struct omap_hsmmc_host *host,
> __func__);
> }
>
> -static void hsmmc_command_incomplete(struct omap_hsmmc_host *host, int err)
> +static void hsmmc_command_incomplete(struct omap_hsmmc_host *host,
> + int err, int end_cmd)
> {
> - omap_hsmmc_reset_controller_fsm(host, SRC);
> - host->cmd->error = err;
> + if (end_cmd) {
> + omap_hsmmc_reset_controller_fsm(host, SRC);
> + if (host->cmd)
> + host->cmd->error = err;
> + }
>
> if (host->data) {
> omap_hsmmc_reset_controller_fsm(host, SRD);
> omap_hsmmc_dma_cleanup(host, err);
> - }
> + } else if (host->mrq && host->mrq->cmd)
> + host->mrq->cmd->error = err;
>
> }
>
> @@ -990,14 +995,16 @@ static void omap_hsmmc_do_irq(struct omap_hsmmc_host *host, int status)
>
> if (status & ERR) {
> omap_hsmmc_dbg_report_irq(host, status);
> +
> + if (status & (CMD_TIMEOUT | CMD_CRC))
> + end_cmd = 1;
> if (status & (CMD_TIMEOUT | DATA_TIMEOUT))
> - hsmmc_command_incomplete(host, -ETIMEDOUT);
> + hsmmc_command_incomplete(host, -ETIMEDOUT, end_cmd);
> else if (status & (CMD_CRC | DATA_CRC))
> - hsmmc_command_incomplete(host, -EILSEQ);
> + hsmmc_command_incomplete(host, -EILSEQ, end_cmd);
>
> - end_cmd = 1;
> if (host->data || host->response_busy) {
> - end_trans = 1;
> + end_trans = !end_cmd;
> host->response_busy = 0;
> }
> }
> --
> 1.7.5.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] mmc: omap_hsmmc: Fix Oops in case of data errors
2012-11-09 14:52 ` Felipe Balbi
@ 2012-11-09 15:28 ` Balaji T K
2012-11-09 16:06 ` [PATCH v2 0/3] mmc: omap_hsmmc: fixes for irq error handler Balaji T K
1 sibling, 0 replies; 14+ messages in thread
From: Balaji T K @ 2012-11-09 15:28 UTC (permalink / raw)
To: balbi; +Cc: linux-mmc, cjb, linux-omap, svenkatr
On Friday 09 November 2012 08:22 PM, Felipe Balbi wrote:
> On Fri, Nov 09, 2012 at 08:11:19PM +0530, Balaji T K wrote:
>> Setting end_cmd to 1 for Data Timeout/CRC leads to NULL pointer dereference of
>> host->cmd as the command complete has previously been handled.
>> Set end_cmd only in case of Data Timeout/CRC.
>
> this comment doesnt match code as code is setting end_cmd for cmd
> timeout and cmd crc, not data timeout/crc.
yes, you are right
s/Data/Cmd :-)
>
>> While at it restore error handling behaviour as was before the
>> "commit ae4bf788ee9bf7c2d51b0309117d1fcccbdd50a2
>> mmc: omap_hsmmc: consolidate error report handling of HSMMC IRQ"
>>
>> host->cmd->error should not be updated on data error case, only
>> host->data->error needs to be updated.
>> end_trans and end_crc should not to be set together, to avoid
>> mmc_request_done being called twice in case of Command CRC or command
>> Timeout.
>> Avoid soft reset of command internal state machine on data errors.
>
> looks to me you're doing to many changes in a single patch. Perhaps it
> deserves some splitting...
>
Let me check if I can split
>>
>> Signed-off-by: Balaji T K <balajitk@ti.com>
>> ---
>> based on mmc-fixes-for-3.7-rc5 in mmc_next
>>
>> drivers/mmc/host/omap_hsmmc.c | 23 +++++++++++++++--------
>> 1 files changed, 15 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
>> index fedd258..6ea1da3 100644
>> --- a/drivers/mmc/host/omap_hsmmc.c
>> +++ b/drivers/mmc/host/omap_hsmmc.c
>> @@ -968,15 +968,20 @@ static inline void omap_hsmmc_reset_controller_fsm(struct omap_hsmmc_host *host,
>> __func__);
>> }
>>
>> -static void hsmmc_command_incomplete(struct omap_hsmmc_host *host, int err)
>> +static void hsmmc_command_incomplete(struct omap_hsmmc_host *host,
>> + int err, int end_cmd)
>> {
>> - omap_hsmmc_reset_controller_fsm(host, SRC);
This conditional reset can be separate patch if needed.
>> - host->cmd->error = err;
>> + if (end_cmd) {
>> + omap_hsmmc_reset_controller_fsm(host, SRC);
>> + if (host->cmd)
>> + host->cmd->error = err;
>> + }
>>
>> if (host->data) {
>> omap_hsmmc_reset_controller_fsm(host, SRD);
>> omap_hsmmc_dma_cleanup(host, err);
>> - }
>> + } else if (host->mrq && host->mrq->cmd)
>> + host->mrq->cmd->error = err;
Updating the error code can also be split.
Let me know If you are Ok with this, I can send new series.
>>
>> }
>>
>> @@ -990,14 +995,16 @@ static void omap_hsmmc_do_irq(struct omap_hsmmc_host *host, int status)
>>
>> if (status & ERR) {
>> omap_hsmmc_dbg_report_irq(host, status);
>> +
>> + if (status & (CMD_TIMEOUT | CMD_CRC))
>> + end_cmd = 1;
>> if (status & (CMD_TIMEOUT | DATA_TIMEOUT))
>> - hsmmc_command_incomplete(host, -ETIMEDOUT);
>> + hsmmc_command_incomplete(host, -ETIMEDOUT, end_cmd);
>> else if (status & (CMD_CRC | DATA_CRC))
>> - hsmmc_command_incomplete(host, -EILSEQ);
>> + hsmmc_command_incomplete(host, -EILSEQ, end_cmd);
>>
>> - end_cmd = 1;
>> if (host->data || host->response_busy) {
>> - end_trans = 1;
>> + end_trans = !end_cmd;
>> host->response_busy = 0;
>> }
>> }
>> --
>> 1.7.5.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-omap" 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] 14+ messages in thread* [PATCH v2 0/3] mmc: omap_hsmmc: fixes for irq error handler
2012-11-09 14:52 ` Felipe Balbi
2012-11-09 15:28 ` Balaji T K
@ 2012-11-09 16:06 ` Balaji T K
2012-11-09 16:06 ` [PATCH v2 1/3] mmc: omap_hsmmc: Fix Oops in case of data errors Balaji T K
` (3 more replies)
1 sibling, 4 replies; 14+ messages in thread
From: Balaji T K @ 2012-11-09 16:06 UTC (permalink / raw)
To: linux-mmc, cjb; +Cc: linux-omap, svenkatr, Balaji T K
Balaji T K (3):
mmc: omap_hsmmc: Fix Oops in case of data errors
mmc: omap_hsmmc: no reset of cmd state machine for DCRC
mmc: omap_hsmmc: update error code for response_busy cmd
drivers/mmc/host/omap_hsmmc.c | 24 +++++++++++++++---------
1 files changed, 15 insertions(+), 9 deletions(-)
--
1.7.5.4
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH v2 1/3] mmc: omap_hsmmc: Fix Oops in case of data errors
2012-11-09 16:06 ` [PATCH v2 0/3] mmc: omap_hsmmc: fixes for irq error handler Balaji T K
@ 2012-11-09 16:06 ` Balaji T K
2012-11-09 16:39 ` Felipe Balbi
2012-11-09 16:06 ` [PATCH v2 2/3] mmc: omap_hsmmc: no reset of cmd state machine for DCRC Balaji T K
` (2 subsequent siblings)
3 siblings, 1 reply; 14+ messages in thread
From: Balaji T K @ 2012-11-09 16:06 UTC (permalink / raw)
To: linux-mmc, cjb; +Cc: linux-omap, svenkatr, Balaji T K
"commit ae4bf788ee9bf7c2d51b0309117d1fcccbdd50a2
mmc: omap_hsmmc: consolidate error report handling of HSMMC IRQ"
sets both end_cmd and end_trans to 1.
Setting end_cmd to 1 for Data Timeout/CRC leads to NULL pointer dereference of
host->cmd as the command complete has previously been handled.
Set end_cmd only in case of command Timeout/CRC.
Moreover host->cmd->error should not be updated on data error case, only
host->data->error needs to be updated.
Avoid soft reset of command internal state machine on data errors.
Signed-off-by: Balaji T K <balajitk@ti.com>
---
based on mmc-fixes-for-3.7-rc5 in mmc_next
Since v1: split into 3 patches
drivers/mmc/host/omap_hsmmc.c | 18 ++++++++++++------
1 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index fedd258..245d7b5 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -968,10 +968,14 @@ static inline void omap_hsmmc_reset_controller_fsm(struct omap_hsmmc_host *host,
__func__);
}
-static void hsmmc_command_incomplete(struct omap_hsmmc_host *host, int err)
+static void hsmmc_command_incomplete(struct omap_hsmmc_host *host,
+ int err, int end_cmd)
{
omap_hsmmc_reset_controller_fsm(host, SRC);
- host->cmd->error = err;
+ if (end_cmd) {
+ if (host->cmd)
+ host->cmd->error = err;
+ }
if (host->data) {
omap_hsmmc_reset_controller_fsm(host, SRD);
@@ -990,14 +994,16 @@ static void omap_hsmmc_do_irq(struct omap_hsmmc_host *host, int status)
if (status & ERR) {
omap_hsmmc_dbg_report_irq(host, status);
+
+ if (status & (CMD_TIMEOUT | CMD_CRC))
+ end_cmd = 1;
if (status & (CMD_TIMEOUT | DATA_TIMEOUT))
- hsmmc_command_incomplete(host, -ETIMEDOUT);
+ hsmmc_command_incomplete(host, -ETIMEDOUT, end_cmd);
else if (status & (CMD_CRC | DATA_CRC))
- hsmmc_command_incomplete(host, -EILSEQ);
+ hsmmc_command_incomplete(host, -EILSEQ, end_cmd);
- end_cmd = 1;
if (host->data || host->response_busy) {
- end_trans = 1;
+ end_trans = !end_cmd;
host->response_busy = 0;
}
}
--
1.7.5.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v2 1/3] mmc: omap_hsmmc: Fix Oops in case of data errors
2012-11-09 16:06 ` [PATCH v2 1/3] mmc: omap_hsmmc: Fix Oops in case of data errors Balaji T K
@ 2012-11-09 16:39 ` Felipe Balbi
0 siblings, 0 replies; 14+ messages in thread
From: Felipe Balbi @ 2012-11-09 16:39 UTC (permalink / raw)
To: Balaji T K; +Cc: linux-mmc, cjb, linux-omap, svenkatr
[-- Attachment #1: Type: text/plain, Size: 2674 bytes --]
On Fri, Nov 09, 2012 at 09:36:14PM +0530, Balaji T K wrote:
> "commit ae4bf788ee9bf7c2d51b0309117d1fcccbdd50a2
> mmc: omap_hsmmc: consolidate error report handling of HSMMC IRQ"
> sets both end_cmd and end_trans to 1.
>
> Setting end_cmd to 1 for Data Timeout/CRC leads to NULL pointer dereference of
> host->cmd as the command complete has previously been handled.
> Set end_cmd only in case of command Timeout/CRC.
>
> Moreover host->cmd->error should not be updated on data error case, only
> host->data->error needs to be updated.
>
> Avoid soft reset of command internal state machine on data errors.
>
> Signed-off-by: Balaji T K <balajitk@ti.com>
this looks a lot better to me.
Reviewed-by: Felipe Balbi <balbi@ti.com>
> ---
> based on mmc-fixes-for-3.7-rc5 in mmc_next
> Since v1: split into 3 patches
>
> drivers/mmc/host/omap_hsmmc.c | 18 ++++++++++++------
> 1 files changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index fedd258..245d7b5 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -968,10 +968,14 @@ static inline void omap_hsmmc_reset_controller_fsm(struct omap_hsmmc_host *host,
> __func__);
> }
>
> -static void hsmmc_command_incomplete(struct omap_hsmmc_host *host, int err)
> +static void hsmmc_command_incomplete(struct omap_hsmmc_host *host,
> + int err, int end_cmd)
> {
> omap_hsmmc_reset_controller_fsm(host, SRC);
> - host->cmd->error = err;
> + if (end_cmd) {
> + if (host->cmd)
> + host->cmd->error = err;
> + }
>
> if (host->data) {
> omap_hsmmc_reset_controller_fsm(host, SRD);
> @@ -990,14 +994,16 @@ static void omap_hsmmc_do_irq(struct omap_hsmmc_host *host, int status)
>
> if (status & ERR) {
> omap_hsmmc_dbg_report_irq(host, status);
> +
> + if (status & (CMD_TIMEOUT | CMD_CRC))
> + end_cmd = 1;
> if (status & (CMD_TIMEOUT | DATA_TIMEOUT))
> - hsmmc_command_incomplete(host, -ETIMEDOUT);
> + hsmmc_command_incomplete(host, -ETIMEDOUT, end_cmd);
> else if (status & (CMD_CRC | DATA_CRC))
> - hsmmc_command_incomplete(host, -EILSEQ);
> + hsmmc_command_incomplete(host, -EILSEQ, end_cmd);
>
> - end_cmd = 1;
> if (host->data || host->response_busy) {
> - end_trans = 1;
> + end_trans = !end_cmd;
> host->response_busy = 0;
> }
> }
> --
> 1.7.5.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 2/3] mmc: omap_hsmmc: no reset of cmd state machine for DCRC
2012-11-09 16:06 ` [PATCH v2 0/3] mmc: omap_hsmmc: fixes for irq error handler Balaji T K
2012-11-09 16:06 ` [PATCH v2 1/3] mmc: omap_hsmmc: Fix Oops in case of data errors Balaji T K
@ 2012-11-09 16:06 ` Balaji T K
2012-11-09 16:39 ` Felipe Balbi
2012-11-09 16:06 ` [PATCH v2 3/3] mmc: omap_hsmmc: update error code for response_busy cmd Balaji T K
2012-11-12 13:06 ` [PATCH v3 0/3] mmc: omap_hsmmc: fixes for irq error handler Balaji T K
3 siblings, 1 reply; 14+ messages in thread
From: Balaji T K @ 2012-11-09 16:06 UTC (permalink / raw)
To: linux-mmc, cjb; +Cc: linux-omap, svenkatr, Balaji T K
Avoid soft reset of command internal state machine on data errors.
Signed-off-by: Balaji T K <balajitk@ti.com>
---
drivers/mmc/host/omap_hsmmc.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 245d7b5..d931e08 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -971,8 +971,8 @@ static inline void omap_hsmmc_reset_controller_fsm(struct omap_hsmmc_host *host,
static void hsmmc_command_incomplete(struct omap_hsmmc_host *host,
int err, int end_cmd)
{
- omap_hsmmc_reset_controller_fsm(host, SRC);
if (end_cmd) {
+ omap_hsmmc_reset_controller_fsm(host, SRC);
if (host->cmd)
host->cmd->error = err;
}
--
1.7.5.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v2 2/3] mmc: omap_hsmmc: no reset of cmd state machine for DCRC
2012-11-09 16:06 ` [PATCH v2 2/3] mmc: omap_hsmmc: no reset of cmd state machine for DCRC Balaji T K
@ 2012-11-09 16:39 ` Felipe Balbi
0 siblings, 0 replies; 14+ messages in thread
From: Felipe Balbi @ 2012-11-09 16:39 UTC (permalink / raw)
To: Balaji T K; +Cc: linux-mmc, cjb, linux-omap, svenkatr
[-- Attachment #1: Type: text/plain, Size: 1157 bytes --]
On Fri, Nov 09, 2012 at 09:36:15PM +0530, Balaji T K wrote:
> Avoid soft reset of command internal state machine on data errors.
>
> Signed-off-by: Balaji T K <balajitk@ti.com>
Reviewed-by: Felipe Balbi <balbi@ti.com>
> ---
> drivers/mmc/host/omap_hsmmc.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index 245d7b5..d931e08 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -971,8 +971,8 @@ static inline void omap_hsmmc_reset_controller_fsm(struct omap_hsmmc_host *host,
> static void hsmmc_command_incomplete(struct omap_hsmmc_host *host,
> int err, int end_cmd)
> {
> - omap_hsmmc_reset_controller_fsm(host, SRC);
> if (end_cmd) {
> + omap_hsmmc_reset_controller_fsm(host, SRC);
> if (host->cmd)
> host->cmd->error = err;
> }
> --
> 1.7.5.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 3/3] mmc: omap_hsmmc: update error code for response_busy cmd
2012-11-09 16:06 ` [PATCH v2 0/3] mmc: omap_hsmmc: fixes for irq error handler Balaji T K
2012-11-09 16:06 ` [PATCH v2 1/3] mmc: omap_hsmmc: Fix Oops in case of data errors Balaji T K
2012-11-09 16:06 ` [PATCH v2 2/3] mmc: omap_hsmmc: no reset of cmd state machine for DCRC Balaji T K
@ 2012-11-09 16:06 ` Balaji T K
2012-11-09 16:40 ` Felipe Balbi
2012-11-12 13:06 ` [PATCH v3 0/3] mmc: omap_hsmmc: fixes for irq error handler Balaji T K
3 siblings, 1 reply; 14+ messages in thread
From: Balaji T K @ 2012-11-09 16:06 UTC (permalink / raw)
To: linux-mmc, cjb; +Cc: linux-omap, svenkatr, Balaji T K
update error code to cmd->error for commands with response_busy and no data
Signed-off-by: Balaji T K <balajitk@ti.com>
---
drivers/mmc/host/omap_hsmmc.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index d931e08..1503808 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -980,8 +980,8 @@ static void hsmmc_command_incomplete(struct omap_hsmmc_host *host,
if (host->data) {
omap_hsmmc_reset_controller_fsm(host, SRD);
omap_hsmmc_dma_cleanup(host, err);
- }
-
+ } else if (host->mrq && host->mrq->cmd)
+ host->mrq->cmd->error = err;
}
static void omap_hsmmc_do_irq(struct omap_hsmmc_host *host, int status)
--
1.7.5.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v2 3/3] mmc: omap_hsmmc: update error code for response_busy cmd
2012-11-09 16:06 ` [PATCH v2 3/3] mmc: omap_hsmmc: update error code for response_busy cmd Balaji T K
@ 2012-11-09 16:40 ` Felipe Balbi
0 siblings, 0 replies; 14+ messages in thread
From: Felipe Balbi @ 2012-11-09 16:40 UTC (permalink / raw)
To: Balaji T K; +Cc: linux-mmc, cjb, linux-omap, svenkatr
[-- Attachment #1: Type: text/plain, Size: 1071 bytes --]
Hi,
On Fri, Nov 09, 2012 at 09:36:16PM +0530, Balaji T K wrote:
> update error code to cmd->error for commands with response_busy and no data
>
> Signed-off-by: Balaji T K <balajitk@ti.com>
Just one comment below, otherwise:
Reviewed-by: Felipe Balbi <balbi@ti.com>
> ---
> drivers/mmc/host/omap_hsmmc.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index d931e08..1503808 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -980,8 +980,8 @@ static void hsmmc_command_incomplete(struct omap_hsmmc_host *host,
> if (host->data) {
> omap_hsmmc_reset_controller_fsm(host, SRD);
> omap_hsmmc_dma_cleanup(host, err);
> - }
> -
> + } else if (host->mrq && host->mrq->cmd)
> + host->mrq->cmd->error = err;
if you have curly braces in one branch, you need to put in all of them,
so this should be:
+ } else if (host->mrq && host->mrq->cmd) {
+ host->mrq->cmd->error = err;
+ }
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 0/3] mmc: omap_hsmmc: fixes for irq error handler
2012-11-09 16:06 ` [PATCH v2 0/3] mmc: omap_hsmmc: fixes for irq error handler Balaji T K
` (2 preceding siblings ...)
2012-11-09 16:06 ` [PATCH v2 3/3] mmc: omap_hsmmc: update error code for response_busy cmd Balaji T K
@ 2012-11-12 13:06 ` Balaji T K
2012-11-12 13:06 ` [PATCH v3 1/3] mmc: omap_hsmmc: Fix Oops in case of data errors Balaji T K
` (2 more replies)
3 siblings, 3 replies; 14+ messages in thread
From: Balaji T K @ 2012-11-12 13:06 UTC (permalink / raw)
To: linux-mmc, cjb; +Cc: linux-omap, svenkatr, balbi, Balaji T K
Balaji T K (3):
mmc: omap_hsmmc: Fix Oops in case of data errors
mmc: omap_hsmmc: no reset of cmd state machine for DCRC
mmc: omap_hsmmc: update error code for response_busy cmd
drivers/mmc/host/omap_hsmmc.c | 23 +++++++++++++++--------
1 files changed, 15 insertions(+), 8 deletions(-)
based on mmc-fixes-for-3.7-rc5 in mmc_next
Since v1: split into 3 patches
Address comments from Felipe Balbi
--
1.7.5.4
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH v3 1/3] mmc: omap_hsmmc: Fix Oops in case of data errors
2012-11-12 13:06 ` [PATCH v3 0/3] mmc: omap_hsmmc: fixes for irq error handler Balaji T K
@ 2012-11-12 13:06 ` Balaji T K
2012-11-12 13:06 ` [PATCH v3 2/3] mmc: omap_hsmmc: no reset of cmd state machine for DCRC Balaji T K
2012-11-12 13:06 ` [PATCH v3 3/3] mmc: omap_hsmmc: update error code for response_busy cmd Balaji T K
2 siblings, 0 replies; 14+ messages in thread
From: Balaji T K @ 2012-11-12 13:06 UTC (permalink / raw)
To: linux-mmc, cjb; +Cc: linux-omap, svenkatr, balbi, Balaji T K
"commit ae4bf788ee9bf7c2d51b0309117d1fcccbdd50a2
mmc: omap_hsmmc: consolidate error report handling of HSMMC IRQ"
sets both end_cmd and end_trans to 1.
Setting end_cmd to 1 for Data Timeout/CRC leads to NULL pointer dereference of
host->cmd as the command complete has previously been handled.
Set end_cmd only in case of command Timeout/CRC.
Moreover host->cmd->error should not be updated on data error case, only
host->data->error needs to be updated.
Signed-off-by: Balaji T K <balajitk@ti.com>
Reviewed-by: Felipe Balbi <balbi@ti.com>
---
drivers/mmc/host/omap_hsmmc.c | 18 ++++++++++++------
1 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index fedd258..245d7b5 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -968,10 +968,14 @@ static inline void omap_hsmmc_reset_controller_fsm(struct omap_hsmmc_host *host,
__func__);
}
-static void hsmmc_command_incomplete(struct omap_hsmmc_host *host, int err)
+static void hsmmc_command_incomplete(struct omap_hsmmc_host *host,
+ int err, int end_cmd)
{
omap_hsmmc_reset_controller_fsm(host, SRC);
- host->cmd->error = err;
+ if (end_cmd) {
+ if (host->cmd)
+ host->cmd->error = err;
+ }
if (host->data) {
omap_hsmmc_reset_controller_fsm(host, SRD);
@@ -990,14 +994,16 @@ static void omap_hsmmc_do_irq(struct omap_hsmmc_host *host, int status)
if (status & ERR) {
omap_hsmmc_dbg_report_irq(host, status);
+
+ if (status & (CMD_TIMEOUT | CMD_CRC))
+ end_cmd = 1;
if (status & (CMD_TIMEOUT | DATA_TIMEOUT))
- hsmmc_command_incomplete(host, -ETIMEDOUT);
+ hsmmc_command_incomplete(host, -ETIMEDOUT, end_cmd);
else if (status & (CMD_CRC | DATA_CRC))
- hsmmc_command_incomplete(host, -EILSEQ);
+ hsmmc_command_incomplete(host, -EILSEQ, end_cmd);
- end_cmd = 1;
if (host->data || host->response_busy) {
- end_trans = 1;
+ end_trans = !end_cmd;
host->response_busy = 0;
}
}
--
1.7.5.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v3 2/3] mmc: omap_hsmmc: no reset of cmd state machine for DCRC
2012-11-12 13:06 ` [PATCH v3 0/3] mmc: omap_hsmmc: fixes for irq error handler Balaji T K
2012-11-12 13:06 ` [PATCH v3 1/3] mmc: omap_hsmmc: Fix Oops in case of data errors Balaji T K
@ 2012-11-12 13:06 ` Balaji T K
2012-11-12 13:06 ` [PATCH v3 3/3] mmc: omap_hsmmc: update error code for response_busy cmd Balaji T K
2 siblings, 0 replies; 14+ messages in thread
From: Balaji T K @ 2012-11-12 13:06 UTC (permalink / raw)
To: linux-mmc, cjb; +Cc: linux-omap, svenkatr, balbi, Balaji T K
Avoid soft reset of command internal state machine on data errors.
Signed-off-by: Balaji T K <balajitk@ti.com>
Reviewed-by: Felipe Balbi <balbi@ti.com>
---
drivers/mmc/host/omap_hsmmc.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 245d7b5..d931e08 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -971,8 +971,8 @@ static inline void omap_hsmmc_reset_controller_fsm(struct omap_hsmmc_host *host,
static void hsmmc_command_incomplete(struct omap_hsmmc_host *host,
int err, int end_cmd)
{
- omap_hsmmc_reset_controller_fsm(host, SRC);
if (end_cmd) {
+ omap_hsmmc_reset_controller_fsm(host, SRC);
if (host->cmd)
host->cmd->error = err;
}
--
1.7.5.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v3 3/3] mmc: omap_hsmmc: update error code for response_busy cmd
2012-11-12 13:06 ` [PATCH v3 0/3] mmc: omap_hsmmc: fixes for irq error handler Balaji T K
2012-11-12 13:06 ` [PATCH v3 1/3] mmc: omap_hsmmc: Fix Oops in case of data errors Balaji T K
2012-11-12 13:06 ` [PATCH v3 2/3] mmc: omap_hsmmc: no reset of cmd state machine for DCRC Balaji T K
@ 2012-11-12 13:06 ` Balaji T K
2 siblings, 0 replies; 14+ messages in thread
From: Balaji T K @ 2012-11-12 13:06 UTC (permalink / raw)
To: linux-mmc, cjb; +Cc: linux-omap, svenkatr, balbi, Balaji T K
update error code to cmd->error for commands with response_busy and no data
Signed-off-by: Balaji T K <balajitk@ti.com>
Reviewed-by: Felipe Balbi <balbi@ti.com>
---
drivers/mmc/host/omap_hsmmc.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index d931e08..352e18e 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -980,8 +980,9 @@ static void hsmmc_command_incomplete(struct omap_hsmmc_host *host,
if (host->data) {
omap_hsmmc_reset_controller_fsm(host, SRD);
omap_hsmmc_dma_cleanup(host, err);
+ } else if (host->mrq && host->mrq->cmd) {
+ host->mrq->cmd->error = err;
}
-
}
static void omap_hsmmc_do_irq(struct omap_hsmmc_host *host, int status)
--
1.7.5.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
end of thread, other threads:[~2012-11-12 13:06 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-11-09 14:41 [PATCH] mmc: omap_hsmmc: Fix Oops in case of data errors Balaji T K
2012-11-09 14:52 ` Felipe Balbi
2012-11-09 15:28 ` Balaji T K
2012-11-09 16:06 ` [PATCH v2 0/3] mmc: omap_hsmmc: fixes for irq error handler Balaji T K
2012-11-09 16:06 ` [PATCH v2 1/3] mmc: omap_hsmmc: Fix Oops in case of data errors Balaji T K
2012-11-09 16:39 ` Felipe Balbi
2012-11-09 16:06 ` [PATCH v2 2/3] mmc: omap_hsmmc: no reset of cmd state machine for DCRC Balaji T K
2012-11-09 16:39 ` Felipe Balbi
2012-11-09 16:06 ` [PATCH v2 3/3] mmc: omap_hsmmc: update error code for response_busy cmd Balaji T K
2012-11-09 16:40 ` Felipe Balbi
2012-11-12 13:06 ` [PATCH v3 0/3] mmc: omap_hsmmc: fixes for irq error handler Balaji T K
2012-11-12 13:06 ` [PATCH v3 1/3] mmc: omap_hsmmc: Fix Oops in case of data errors Balaji T K
2012-11-12 13:06 ` [PATCH v3 2/3] mmc: omap_hsmmc: no reset of cmd state machine for DCRC Balaji T K
2012-11-12 13:06 ` [PATCH v3 3/3] mmc: omap_hsmmc: update error code for response_busy cmd Balaji T K
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).