[PATCH] scsi: ufs: core: Fix error return with query response

[PATCH] scsi: ufs: core: Fix error return with query response

[Seunghui Lee]

There is currently no mechanism to return error from query responses.
Return the error and print the corresponding error message with it.

Signed-off-by: Seunghui Lee <sh043.lee@samsung.com>
---
 drivers/ufs/core/ufshcd.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 9c26e8767515..6b27ea1a7a1b 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -3118,8 +3118,13 @@ ufshcd_dev_cmd_completion(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
 	case UPIU_TRANSACTION_QUERY_RSP: {
 		u8 response = lrbp->ucd_rsp_ptr->header.response;
 
-		if (response == 0)
+		if (response == 0) {
 			err = ufshcd_copy_query_response(hba, lrbp);
+		} else {
+			err = -EINVAL;
+			dev_err(hba->dev, "%s: unexpected response %x\n",
+					__func__, resp);
+		}
 		break;
 	}
 	case UPIU_TRANSACTION_REJECT_UPIU:
-- 
2.43.0

Re: [PATCH] scsi: ufs: core: Fix error return with query response

[Bean Huo]

On Fri, 2025-01-17 at 16:16 +0900, Seunghui Lee wrote:
> There is currently no mechanism to return error from query responses.
> Return the error and print the corresponding error message with it.
> 
> Signed-off-by: Seunghui Lee <sh043.lee@samsung.com>
> ---
>  drivers/ufs/core/ufshcd.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 9c26e8767515..6b27ea1a7a1b 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -3118,8 +3118,13 @@ ufshcd_dev_cmd_completion(struct ufs_hba *hba,
> struct ufshcd_lrb *lrbp)
>         case UPIU_TRANSACTION_QUERY_RSP: {
>                 u8 response = lrbp->ucd_rsp_ptr->header.response;
>  
> -               if (response == 0)
> +               if (response == 0) {
>                         err = ufshcd_copy_query_response(hba, lrbp);
> +               } else {
> +                       err = -EINVAL;
> +                       dev_err(hba->dev, "%s: unexpected response
> %x\n",
> +                                       __func__, resp);
> +               }
>                 break;
>         }
>         case UPIU_TRANSACTION_REJECT_UPIU:



There is a confusing mix of UPIU transaction code and response in UPIU
here, I think you want to print "response" instead of transaction code.

dev_err(hba->dev, "%s: Unexpected response in Query RSP: %x\n",
__func__, response);

RE: [PATCH] scsi: ufs: core: Fix error return with query response

[이승희]

> -----Original Message-----
> From: Bean Huo <huobean@gmail.com>
> Sent: Friday, January 17, 2025 7:47 PM
> To: Seunghui Lee <sh043.lee@samsung.com>; linux-scsi@vger.kernel.org
> Subject: Re: [PATCH] scsi: ufs: core: Fix error return with query response
> 
> On Fri, 2025-01-17 at 16:16 +0900, Seunghui Lee wrote:
> > There is currently no mechanism to return error from query responses.
> > Return the error and print the corresponding error message with it.
> >
> > Signed-off-by: Seunghui Lee <sh043.lee@samsung.com>
> > ---
> >  drivers/ufs/core/ufshcd.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> > index 9c26e8767515..6b27ea1a7a1b 100644
> > --- a/drivers/ufs/core/ufshcd.c
> > +++ b/drivers/ufs/core/ufshcd.c
> > @@ -3118,8 +3118,13 @@ ufshcd_dev_cmd_completion(struct ufs_hba *hba,
> > struct ufshcd_lrb *lrbp)
> >         case UPIU_TRANSACTION_QUERY_RSP: {
> >                 u8 response = lrbp->ucd_rsp_ptr->header.response;
> >
> > -               if (response == 0)
> > +               if (response == 0) {
> >                         err = ufshcd_copy_query_response(hba, lrbp);
> > +               } else {
> > +                       err = -EINVAL;
> > +                       dev_err(hba->dev, "%s: unexpected response
> > %x\n",
> > +                                       __func__, resp);
> > +               }
> >                 break;
> >         }
> >         case UPIU_TRANSACTION_REJECT_UPIU:
> 
> 
> 
> There is a confusing mix of UPIU transaction code and response in UPIU
> here, I think you want to print "response" instead of transaction code.
> 
> dev_err(hba->dev, "%s: Unexpected response in Query RSP: %x\n", __func__,
> response);
> 
> 

I think that your suggestion is much better.
I'll modify this as your suggestion with v2.
Thank you.