Re: [PATCH 1/7] scsi: ufs: amend the ocs handling with fatal error

2013-08-13 Thread Subhash Jadavani

On 8/13/2013 5:20 PM, Seungwon Jeon wrote:

On Mon, August 12, 2013, Subhash Jadavani wrote:

On 7/30/2013 6:32 PM, Seungwon Jeon wrote:

On Mon, July 29, 2013, Sujit Reddy Thumma wrote:

On 7/29/2013 11:47 AM, Subhash Jadavani wrote:

On 7/26/2013 7:15 PM, Seungwon Jeon wrote:

Fatal error in OCS(overall command status) field indicates
error conditions which is not covered by UFSHCI.
It means that host cannot define the result of command status
and therefore host may need to check transfer response UPIU's
response and status field.
It was actually found that 'CHECK CONDITION' is stored in status
field of UPIU where OCS is 'FATAL ERROR'.

It looks like you are assuming that there will be some kind of response
from the device. But the spec. mentions - "OCS_FATAL ERROR: within host
controller that is not covered by the error conditions described above
in this table."

Yes, error interrupt doesn't happen actually.


So spec. left everything to implementers on how to trigger this error.
Also, it says "within host controller" so ufshcd_is_valid_req_rsp()
might fail as well.

I couldn't understand why there is a need for a host controller to
interpret the SCSI command status and raise an OCS_FATAL_ERROR?

I feel like OCS values are related to syntax problem except OCS_FATAL_ERROR.
Basically, host controller may need to refer the Response field[Target 
Success/Target Failure] of

response UPIU.

It's a overall result from response UPIU.
When Response field is 'Target Failure', if host controller updates the OCS 
field with 'SUCCESS',

it's not proper behavior.

In this case host may refer the Status field of response UPIU to decide the OCS 
result.
Of course, it's not clear thing and could depends on host's implementation,
because there is no obvious description for that.
But if we consider the way to report UTP error from UFSHCI 8.2.4,
we can check the Response UPIU for more detailed error condition regardless OCS 
value.
Could you check your host side?

This is what our host documentation says: " SW may check the contents of
Response UPIU when OCS !=0 for debug purposes. It may (or may not) help
to understand the error scenario better. This is needed only for debug.
When system is stable, OCS should be 0." So this clearly means that we
shouldn't rely on the response UPIU if the OCS is non-zero hence simply
ignore it.

Isn't  there any description for FATAL ERROR of OCS?
Can I take like below?
Even though Response field is 'Target Failure', OCS field can get a success 
result.
Yes, that's true. OCS could give success result for response field can 
be 'Target Failure'

I just has focused what spec. says below:
- FATAL ERROR within host controller that is not covered by the error 
conditions described above in this table.
- The field contains the status code for the request.
-  Host software may need to check Transfer Response UPIU's Response, and 
possibly Sense Data to find out more details for the cause of the error.
Because host doesn't know and decide the error status, host could need to check.
No, i don't think it should be interpreted to read the response UPIU 
when OCS is equal to FATAL ERROR.



  > "When Response field is 'Target Failure', if host controller updates
the OCS field with 'SUCCESS', it's not proper behavior."

I doubt whether your above understanding is true from Host controller
specification point of view. Host controller would not decode the
“Response” field of the “response UPIU”, it would only decode the
“Transaction Type” & “Task Tag” (and may be “LUN”) from the response
UPIU in order to find out the correct slot in the transfer/task request
list. Even 7.3.2.3 in UFSHCI spec also mentions the same.

I guess with this i don't see a need for this patch unless you feel
otherwise.

It may depend on host's implementation. HCI spec. doesn't guide for OCS in 
detail.
Anyway, it's not clear just now.

Thanks,
Seungwon Jeon


Regards,
Subhash


Thanks,
Seungwon Jeon


If it is clarified by the spec. then we can have generic implementation
otherwise I would prefer to make this specific to those host controllers
that raise OCS_FATAL_ERROR for CHECK_CONDITION.


Signed-off-by: Seungwon Jeon 
---
drivers/scsi/ufs/ufshcd.c |3 +--
1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index b743bd6..4cf3a2d 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1199,7 +1199,7 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba,
struct ufshcd_lrb *lrbp)
switch (ocs) {
case OCS_SUCCESS:
-
+case OCS_FATAL_ERROR:
/* check if the returned transfer response is valid */
result = ufshcd_is_valid_req_rsp(lrbp->ucd_rsp_ptr);

I don't see the response UPIU data of the last command response is
cleared anywhere in the driver. This means its quite possible that if
the current command failed (and if it is using the same tag as the last
succeeded command) with the OCS_FATAL_ERROR th

RE: [PATCH 1/7] scsi: ufs: amend the ocs handling with fatal error

2013-08-13 Thread Seungwon Jeon
On Mon, August 12, 2013, Subhash Jadavani wrote:
> On 7/30/2013 6:32 PM, Seungwon Jeon wrote:
> > On Mon, July 29, 2013, Sujit Reddy Thumma wrote:
> >> On 7/29/2013 11:47 AM, Subhash Jadavani wrote:
> >>> On 7/26/2013 7:15 PM, Seungwon Jeon wrote:
>  Fatal error in OCS(overall command status) field indicates
>  error conditions which is not covered by UFSHCI.
>  It means that host cannot define the result of command status
>  and therefore host may need to check transfer response UPIU's
>  response and status field.
>  It was actually found that 'CHECK CONDITION' is stored in status
>  field of UPIU where OCS is 'FATAL ERROR'.
> >> It looks like you are assuming that there will be some kind of response
> >> from the device. But the spec. mentions - "OCS_FATAL ERROR: within host
> >> controller that is not covered by the error conditions described above
> >> in this table."
> > Yes, error interrupt doesn't happen actually.
> >
> >> So spec. left everything to implementers on how to trigger this error.
> >> Also, it says "within host controller" so ufshcd_is_valid_req_rsp()
> >> might fail as well.
> >>
> >> I couldn't understand why there is a need for a host controller to
> >> interpret the SCSI command status and raise an OCS_FATAL_ERROR?
> > I feel like OCS values are related to syntax problem except OCS_FATAL_ERROR.
> > Basically, host controller may need to refer the Response field[Target 
> > Success/Target Failure] of
> response UPIU.
> > It's a overall result from response UPIU.
> > When Response field is 'Target Failure', if host controller updates the OCS 
> > field with 'SUCCESS',
> it's not proper behavior.
> > In this case host may refer the Status field of response UPIU to decide the 
> > OCS result.
> > Of course, it's not clear thing and could depends on host's implementation,
> > because there is no obvious description for that.
> > But if we consider the way to report UTP error from UFSHCI 8.2.4,
> > we can check the Response UPIU for more detailed error condition regardless 
> > OCS value.
> > Could you check your host side?
> 
> This is what our host documentation says: " SW may check the contents of
> Response UPIU when OCS !=0 for debug purposes. It may (or may not) help
> to understand the error scenario better. This is needed only for debug.
> When system is stable, OCS should be 0." So this clearly means that we
> shouldn't rely on the response UPIU if the OCS is non-zero hence simply
> ignore it.
Isn't  there any description for FATAL ERROR of OCS?
Can I take like below?
Even though Response field is 'Target Failure', OCS field can get a success 
result.
I just has focused what spec. says below:
- FATAL ERROR within host controller that is not covered by the error 
conditions described above in this table.
- The field contains the status code for the request.
-  Host software may need to check Transfer Response UPIU's Response, and 
possibly Sense Data to find out more details for the cause of the error.
Because host doesn't know and decide the error status, host could need to check.

> 
>  > "When Response field is 'Target Failure', if host controller updates
> the OCS field with 'SUCCESS', it's not proper behavior."
> 
> I doubt whether your above understanding is true from Host controller
> specification point of view. Host controller would not decode the
> “Response” field of the “response UPIU”, it would only decode the
> “Transaction Type” & “Task Tag” (and may be “LUN”) from the response
> UPIU in order to find out the correct slot in the transfer/task request
> list. Even 7.3.2.3 in UFSHCI spec also mentions the same.
> 
> I guess with this i don't see a need for this patch unless you feel
> otherwise.
It may depend on host's implementation. HCI spec. doesn't guide for OCS in 
detail.
Anyway, it's not clear just now.

Thanks,
Seungwon Jeon

> 
> Regards,
> Subhash
> 
> >
> > Thanks,
> > Seungwon Jeon
> >
> >> If it is clarified by the spec. then we can have generic implementation
> >> otherwise I would prefer to make this specific to those host controllers
> >> that raise OCS_FATAL_ERROR for CHECK_CONDITION.
> >>
>  Signed-off-by: Seungwon Jeon 
>  ---
> drivers/scsi/ufs/ufshcd.c |3 +--
> 1 files changed, 1 insertions(+), 2 deletions(-)
> 
>  diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>  index b743bd6..4cf3a2d 100644
>  --- a/drivers/scsi/ufs/ufshcd.c
>  +++ b/drivers/scsi/ufs/ufshcd.c
>  @@ -1199,7 +1199,7 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba,
>  struct ufshcd_lrb *lrbp)
> switch (ocs) {
> case OCS_SUCCESS:
>  -
>  +case OCS_FATAL_ERROR:
> /* check if the returned transfer response is valid */
> result = ufshcd_is_valid_req_rsp(lrbp->ucd_rsp_ptr);
> >>> I don't see the response UPIU data of the last command response is
> >>> cleared anywhere in the driver. This means its quite possible t

Re: [PATCH 1/7] scsi: ufs: amend the ocs handling with fatal error

2013-08-12 Thread Subhash Jadavani

On 7/30/2013 6:32 PM, Seungwon Jeon wrote:

On Mon, July 29, 2013, Sujit Reddy Thumma wrote:

On 7/29/2013 11:47 AM, Subhash Jadavani wrote:

On 7/26/2013 7:15 PM, Seungwon Jeon wrote:

Fatal error in OCS(overall command status) field indicates
error conditions which is not covered by UFSHCI.
It means that host cannot define the result of command status
and therefore host may need to check transfer response UPIU's
response and status field.
It was actually found that 'CHECK CONDITION' is stored in status
field of UPIU where OCS is 'FATAL ERROR'.

It looks like you are assuming that there will be some kind of response
from the device. But the spec. mentions - "OCS_FATAL ERROR: within host
controller that is not covered by the error conditions described above
in this table."

Yes, error interrupt doesn't happen actually.


So spec. left everything to implementers on how to trigger this error.
Also, it says "within host controller" so ufshcd_is_valid_req_rsp()
might fail as well.

I couldn't understand why there is a need for a host controller to
interpret the SCSI command status and raise an OCS_FATAL_ERROR?

I feel like OCS values are related to syntax problem except OCS_FATAL_ERROR.
Basically, host controller may need to refer the Response field[Target 
Success/Target Failure] of response UPIU.
It's a overall result from response UPIU.
When Response field is 'Target Failure', if host controller updates the OCS 
field with 'SUCCESS', it's not proper behavior.
In this case host may refer the Status field of response UPIU to decide the OCS 
result.
Of course, it's not clear thing and could depends on host's implementation,
because there is no obvious description for that.
But if we consider the way to report UTP error from UFSHCI 8.2.4,
we can check the Response UPIU for more detailed error condition regardless OCS 
value.
Could you check your host side?


This is what our host documentation says: " SW may check the contents of 
Response UPIU when OCS !=0 for debug purposes. It may (or may not) help 
to understand the error scenario better. This is needed only for debug. 
When system is stable, OCS should be 0." So this clearly means that we 
shouldn't rely on the response UPIU if the OCS is non-zero hence simply 
ignore it.


> "When Response field is 'Target Failure', if host controller updates 
the OCS field with 'SUCCESS', it's not proper behavior."


I doubt whether your above understanding is true from Host controller 
specification point of view. Host controller would not decode the 
“Response” field of the “response UPIU”, it would only decode the 
“Transaction Type” & “Task Tag” (and may be “LUN”) from the response 
UPIU in order to find out the correct slot in the transfer/task request 
list. Even 7.3.2.3 in UFSHCI spec also mentions the same.


I guess with this i don't see a need for this patch unless you feel 
otherwise.


Regards,
Subhash



Thanks,
Seungwon Jeon


If it is clarified by the spec. then we can have generic implementation
otherwise I would prefer to make this specific to those host controllers
that raise OCS_FATAL_ERROR for CHECK_CONDITION.


Signed-off-by: Seungwon Jeon 
---
   drivers/scsi/ufs/ufshcd.c |3 +--
   1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index b743bd6..4cf3a2d 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1199,7 +1199,7 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba,
struct ufshcd_lrb *lrbp)
   switch (ocs) {
   case OCS_SUCCESS:
-
+case OCS_FATAL_ERROR:
   /* check if the returned transfer response is valid */
   result = ufshcd_is_valid_req_rsp(lrbp->ucd_rsp_ptr);

I don't see the response UPIU data of the last command response is
cleared anywhere in the driver. This means its quite possible that if
the current command failed (and if it is using the same tag as the last
succeeded command) with the OCS_FATAL_ERROR then response UPIU (pointed
by lrbp->ucd_rsp_ptr) content may be still the same as the last one. If
we ensure to clear the response UPIU data after every command completion
then only we can rely on the response  UPIU content in case of fatal
errors.

Regards,
Subhash

   if (result) {
@@ -1229,7 +1229,6 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba,
struct ufshcd_lrb *lrbp)
   case OCS_MISMATCH_DATA_BUF_SIZE:
   case OCS_MISMATCH_RESP_UPIU_SIZE:
   case OCS_PEER_COMM_FAILURE:
-case OCS_FATAL_ERROR:
   default:
   result |= DID_ERROR << 16;
   dev_err(hba->dev,


--
Regards,
Sujit
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 1/7] scsi: ufs: amend the ocs handling with fatal error

2013-07-30 Thread Seungwon Jeon
On Mon, July 29, 2013, Sujit Reddy Thumma wrote:
> On 7/29/2013 11:47 AM, Subhash Jadavani wrote:
> > On 7/26/2013 7:15 PM, Seungwon Jeon wrote:
> >> Fatal error in OCS(overall command status) field indicates
> >> error conditions which is not covered by UFSHCI.
> >> It means that host cannot define the result of command status
> >> and therefore host may need to check transfer response UPIU's
> >> response and status field.
> >> It was actually found that 'CHECK CONDITION' is stored in status
> >> field of UPIU where OCS is 'FATAL ERROR'.
> 
> It looks like you are assuming that there will be some kind of response
> from the device. But the spec. mentions - "OCS_FATAL ERROR: within host
> controller that is not covered by the error conditions described above
> in this table."
Yes, error interrupt doesn't happen actually.

> 
> So spec. left everything to implementers on how to trigger this error.
> Also, it says "within host controller" so ufshcd_is_valid_req_rsp()
> might fail as well.
> 
> I couldn't understand why there is a need for a host controller to
> interpret the SCSI command status and raise an OCS_FATAL_ERROR?
I feel like OCS values are related to syntax problem except OCS_FATAL_ERROR.
Basically, host controller may need to refer the Response field[Target 
Success/Target Failure] of response UPIU.
It's a overall result from response UPIU. 
When Response field is 'Target Failure', if host controller updates the OCS 
field with 'SUCCESS', it's not proper behavior.
In this case host may refer the Status field of response UPIU to decide the OCS 
result.
Of course, it's not clear thing and could depends on host's implementation,
because there is no obvious description for that.
But if we consider the way to report UTP error from UFSHCI 8.2.4,
we can check the Response UPIU for more detailed error condition regardless OCS 
value.
Could you check your host side?

Thanks,
Seungwon Jeon

> 
> If it is clarified by the spec. then we can have generic implementation
> otherwise I would prefer to make this specific to those host controllers
> that raise OCS_FATAL_ERROR for CHECK_CONDITION.
> 
> >>
> >> Signed-off-by: Seungwon Jeon 
> >> ---
> >>   drivers/scsi/ufs/ufshcd.c |3 +--
> >>   1 files changed, 1 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> >> index b743bd6..4cf3a2d 100644
> >> --- a/drivers/scsi/ufs/ufshcd.c
> >> +++ b/drivers/scsi/ufs/ufshcd.c
> >> @@ -1199,7 +1199,7 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba,
> >> struct ufshcd_lrb *lrbp)
> >>   switch (ocs) {
> >>   case OCS_SUCCESS:
> >> -
> >> +case OCS_FATAL_ERROR:
> >>   /* check if the returned transfer response is valid */
> >>   result = ufshcd_is_valid_req_rsp(lrbp->ucd_rsp_ptr);
> >
> > I don't see the response UPIU data of the last command response is
> > cleared anywhere in the driver. This means its quite possible that if
> > the current command failed (and if it is using the same tag as the last
> > succeeded command) with the OCS_FATAL_ERROR then response UPIU (pointed
> > by lrbp->ucd_rsp_ptr) content may be still the same as the last one. If
> > we ensure to clear the response UPIU data after every command completion
> > then only we can rely on the response  UPIU content in case of fatal
> > errors.
> >
> > Regards,
> > Subhash
> >>   if (result) {
> >> @@ -1229,7 +1229,6 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba,
> >> struct ufshcd_lrb *lrbp)
> >>   case OCS_MISMATCH_DATA_BUF_SIZE:
> >>   case OCS_MISMATCH_RESP_UPIU_SIZE:
> >>   case OCS_PEER_COMM_FAILURE:
> >> -case OCS_FATAL_ERROR:
> >>   default:
> >>   result |= DID_ERROR << 16;
> >>   dev_err(hba->dev,
> >
> 
> 
> --
> Regards,
> Sujit
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/7] scsi: ufs: amend the ocs handling with fatal error

2013-07-29 Thread Santosh Y
On Fri, Jul 26, 2013 at 7:15 PM, Seungwon Jeon  wrote:
> Fatal error in OCS(overall command status) field indicates
> error conditions which is not covered by UFSHCI.
> It means that host cannot define the result of command status
> and therefore host may need to check transfer response UPIU's
> response and status field.
> It was actually found that 'CHECK CONDITION' is stored in status
> field of UPIU where OCS is 'FATAL ERROR'.

It seems specific to the host controller you are checking with.
It doesn't seem right to interpret the scsi status -- even though the
handling isn't mentioned in the spec for FATAL ERROR.

>
> Signed-off-by: Seungwon Jeon 
> ---
>  drivers/scsi/ufs/ufshcd.c |3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index b743bd6..4cf3a2d 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -1199,7 +1199,7 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct 
> ufshcd_lrb *lrbp)
>
> switch (ocs) {
> case OCS_SUCCESS:
> -
> +   case OCS_FATAL_ERROR:
> /* check if the returned transfer response is valid */
> result = ufshcd_is_valid_req_rsp(lrbp->ucd_rsp_ptr);
> if (result) {
> @@ -1229,7 +1229,6 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct 
> ufshcd_lrb *lrbp)
> case OCS_MISMATCH_DATA_BUF_SIZE:
> case OCS_MISMATCH_RESP_UPIU_SIZE:
> case OCS_PEER_COMM_FAILURE:
> -   case OCS_FATAL_ERROR:
> default:
> result |= DID_ERROR << 16;
> dev_err(hba->dev,
> --
> 1.7.0.4
>
>



-- 
~Santosh
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/7] scsi: ufs: amend the ocs handling with fatal error

2013-07-29 Thread Sujit Reddy Thumma

On 7/29/2013 11:47 AM, Subhash Jadavani wrote:

On 7/26/2013 7:15 PM, Seungwon Jeon wrote:

Fatal error in OCS(overall command status) field indicates
error conditions which is not covered by UFSHCI.
It means that host cannot define the result of command status
and therefore host may need to check transfer response UPIU's
response and status field.
It was actually found that 'CHECK CONDITION' is stored in status
field of UPIU where OCS is 'FATAL ERROR'.


It looks like you are assuming that there will be some kind of response
from the device. But the spec. mentions - "OCS_FATAL ERROR: within host
controller that is not covered by the error conditions described above
in this table."

So spec. left everything to implementers on how to trigger this error.
Also, it says "within host controller" so ufshcd_is_valid_req_rsp()
might fail as well.

I couldn't understand why there is a need for a host controller to
interpret the SCSI command status and raise an OCS_FATAL_ERROR?

If it is clarified by the spec. then we can have generic implementation
otherwise I would prefer to make this specific to those host controllers
that raise OCS_FATAL_ERROR for CHECK_CONDITION.



Signed-off-by: Seungwon Jeon 
---
  drivers/scsi/ufs/ufshcd.c |3 +--
  1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index b743bd6..4cf3a2d 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1199,7 +1199,7 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba,
struct ufshcd_lrb *lrbp)
  switch (ocs) {
  case OCS_SUCCESS:
-
+case OCS_FATAL_ERROR:
  /* check if the returned transfer response is valid */
  result = ufshcd_is_valid_req_rsp(lrbp->ucd_rsp_ptr);


I don't see the response UPIU data of the last command response is
cleared anywhere in the driver. This means its quite possible that if
the current command failed (and if it is using the same tag as the last
succeeded command) with the OCS_FATAL_ERROR then response UPIU (pointed
by lrbp->ucd_rsp_ptr) content may be still the same as the last one. If
we ensure to clear the response UPIU data after every command completion
then only we can rely on the response  UPIU content in case of fatal
errors.

Regards,
Subhash

  if (result) {
@@ -1229,7 +1229,6 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba,
struct ufshcd_lrb *lrbp)
  case OCS_MISMATCH_DATA_BUF_SIZE:
  case OCS_MISMATCH_RESP_UPIU_SIZE:
  case OCS_PEER_COMM_FAILURE:
-case OCS_FATAL_ERROR:
  default:
  result |= DID_ERROR << 16;
  dev_err(hba->dev,





--
Regards,
Sujit
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/7] scsi: ufs: amend the ocs handling with fatal error

2013-07-29 Thread Subhash Jadavani

On 7/29/2013 3:35 PM, Seungwon Jeon wrote:

On Mon, July 29, 2013, Subhash Jadavani wrote:

On 7/26/2013 7:15 PM, Seungwon Jeon wrote:

Fatal error in OCS(overall command status) field indicates
error conditions which is not covered by UFSHCI.
It means that host cannot define the result of command status
and therefore host may need to check transfer response UPIU's
response and status field.
It was actually found that 'CHECK CONDITION' is stored in status
field of UPIU where OCS is 'FATAL ERROR'.

Signed-off-by: Seungwon Jeon 
---
   drivers/scsi/ufs/ufshcd.c |3 +--
   1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index b743bd6..4cf3a2d 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1199,7 +1199,7 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct 
ufshcd_lrb *lrbp)

switch (ocs) {
case OCS_SUCCESS:
-
+   case OCS_FATAL_ERROR:
/* check if the returned transfer response is valid */
result = ufshcd_is_valid_req_rsp(lrbp->ucd_rsp_ptr);

I don't see the response UPIU data of the last command response is
cleared anywhere in the driver. This means its quite possible that if
the current command failed (and if it is using the same tag as the last
succeeded command) with the OCS_FATAL_ERROR then response UPIU (pointed
by lrbp->ucd_rsp_ptr) content may be still the same as the last one. If
we ensure to clear the response UPIU data after every command completion
then only we can rely on the response  UPIU content in case of fatal errors.

Response data of 'ucd_rsp_ptr' will be updated by UFS device if response UPIU 
is completed.
There is nowhere driver clears 'ucd_rsp_ptr', though.
Correct. But with OCS_FATAL_ERROR error, i don't think its gaurenteed 
that response UPIU would be updated.

But if it's really needed, it can be another patch?

Yes, its ok to have it as another patch.


Thanks,
Seungwon Jeon



--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 1/7] scsi: ufs: amend the ocs handling with fatal error

2013-07-29 Thread Seungwon Jeon
On Mon, July 29, 2013, Subhash Jadavani wrote:
> On 7/26/2013 7:15 PM, Seungwon Jeon wrote:
> > Fatal error in OCS(overall command status) field indicates
> > error conditions which is not covered by UFSHCI.
> > It means that host cannot define the result of command status
> > and therefore host may need to check transfer response UPIU's
> > response and status field.
> > It was actually found that 'CHECK CONDITION' is stored in status
> > field of UPIU where OCS is 'FATAL ERROR'.
> >
> > Signed-off-by: Seungwon Jeon 
> > ---
> >   drivers/scsi/ufs/ufshcd.c |3 +--
> >   1 files changed, 1 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > index b743bd6..4cf3a2d 100644
> > --- a/drivers/scsi/ufs/ufshcd.c
> > +++ b/drivers/scsi/ufs/ufshcd.c
> > @@ -1199,7 +1199,7 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba, 
> > struct ufshcd_lrb *lrbp)
> >
> > switch (ocs) {
> > case OCS_SUCCESS:
> > -
> > +   case OCS_FATAL_ERROR:
> > /* check if the returned transfer response is valid */
> > result = ufshcd_is_valid_req_rsp(lrbp->ucd_rsp_ptr);
> 
> I don't see the response UPIU data of the last command response is
> cleared anywhere in the driver. This means its quite possible that if
> the current command failed (and if it is using the same tag as the last
> succeeded command) with the OCS_FATAL_ERROR then response UPIU (pointed
> by lrbp->ucd_rsp_ptr) content may be still the same as the last one. If
> we ensure to clear the response UPIU data after every command completion
> then only we can rely on the response  UPIU content in case of fatal errors.
Response data of 'ucd_rsp_ptr' will be updated by UFS device if response UPIU 
is completed.
There is nowhere driver clears 'ucd_rsp_ptr', though.
But if it's really needed, it can be another patch?

Thanks,
Seungwon Jeon

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/7] scsi: ufs: amend the ocs handling with fatal error

2013-07-28 Thread Subhash Jadavani

On 7/26/2013 7:15 PM, Seungwon Jeon wrote:

Fatal error in OCS(overall command status) field indicates
error conditions which is not covered by UFSHCI.
It means that host cannot define the result of command status
and therefore host may need to check transfer response UPIU's
response and status field.
It was actually found that 'CHECK CONDITION' is stored in status
field of UPIU where OCS is 'FATAL ERROR'.

Signed-off-by: Seungwon Jeon 
---
  drivers/scsi/ufs/ufshcd.c |3 +--
  1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index b743bd6..4cf3a2d 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1199,7 +1199,7 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct 
ufshcd_lrb *lrbp)
  
  	switch (ocs) {

case OCS_SUCCESS:
-
+   case OCS_FATAL_ERROR:
/* check if the returned transfer response is valid */
result = ufshcd_is_valid_req_rsp(lrbp->ucd_rsp_ptr);


I don't see the response UPIU data of the last command response is 
cleared anywhere in the driver. This means its quite possible that if 
the current command failed (and if it is using the same tag as the last 
succeeded command) with the OCS_FATAL_ERROR then response UPIU (pointed 
by lrbp->ucd_rsp_ptr) content may be still the same as the last one. If 
we ensure to clear the response UPIU data after every command completion 
then only we can rely on the response  UPIU content in case of fatal errors.


Regards,
Subhash

if (result) {
@@ -1229,7 +1229,6 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct 
ufshcd_lrb *lrbp)
case OCS_MISMATCH_DATA_BUF_SIZE:
case OCS_MISMATCH_RESP_UPIU_SIZE:
case OCS_PEER_COMM_FAILURE:
-   case OCS_FATAL_ERROR:
default:
result |= DID_ERROR << 16;
dev_err(hba->dev,


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/7] scsi: ufs: amend the ocs handling with fatal error

2013-07-26 Thread Seungwon Jeon
Fatal error in OCS(overall command status) field indicates
error conditions which is not covered by UFSHCI.
It means that host cannot define the result of command status
and therefore host may need to check transfer response UPIU's
response and status field.
It was actually found that 'CHECK CONDITION' is stored in status
field of UPIU where OCS is 'FATAL ERROR'.

Signed-off-by: Seungwon Jeon 
---
 drivers/scsi/ufs/ufshcd.c |3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index b743bd6..4cf3a2d 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1199,7 +1199,7 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct 
ufshcd_lrb *lrbp)
 
switch (ocs) {
case OCS_SUCCESS:
-
+   case OCS_FATAL_ERROR:
/* check if the returned transfer response is valid */
result = ufshcd_is_valid_req_rsp(lrbp->ucd_rsp_ptr);
if (result) {
@@ -1229,7 +1229,6 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct 
ufshcd_lrb *lrbp)
case OCS_MISMATCH_DATA_BUF_SIZE:
case OCS_MISMATCH_RESP_UPIU_SIZE:
case OCS_PEER_COMM_FAILURE:
-   case OCS_FATAL_ERROR:
default:
result |= DID_ERROR << 16;
dev_err(hba->dev,
-- 
1.7.0.4


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html