Re: [PATCH 3/3] scsi: ufs: core: Clear stale UIC result byte before peer DME retries

Re: [PATCH 3/3] scsi: ufs: core: Clear stale UIC result byte before peer DME retries

[Stanley Jhu]

On Tue, Oct 13, 2026 at 10:41:55AM +0000, sashiko-bot@kernel.org wrote:
> Should uic_cmd.argument3 be restored to mib_val before retrying, similar
> to how argument2 is cleared here?

Yes, done in v2:
https://lore.kernel.org/all/20261015001855.3450573-6-stanleyjhu@google.com/

Re: [PATCH 3/3] scsi: ufs: core: Clear stale UIC result byte before peer DME retries

[Bean Huo]

On Tue, 2026-10-13 at 18:29 +0800, Stanley Jhu wrote:
> Peer DME get and set retry a failed attempt with the same struct
> uic_command. UIC completion ORs the ConfigResultCode into
> argument2[7:0], and nothing clears it before the next attempt. Once an
> attempt fails, every retry keeps reporting that error code, even when
> the device returns success, so a transient failure becomes a hard
> failure after three retries. Each such retry also triggers the
> WARN_ON_ONCE() in ufshcd_dispatch_uic_cmd().
> 
> Clear the result byte before each attempt.
> 
> Fixes: 64238fbd15ea ("scsi: ufs: add retries to dme_peer get and set
> attribute")
> Cc: stable@vger.kernel.org
> Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
> ---
>  drivers/ufs/core/ufshcd.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index bcc2512bf34b..9adb3b576e28 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -4398,6 +4398,7 @@ int ufshcd_dme_set_attr(struct ufs_hba *hba, u32
> attr_sel,
>         int retries = UFS_UIC_COMMAND_RETRIES;
>  
>         do {
> +               uic_cmd.argument2 &= ~MASK_UIC_COMMAND_RESULT;
>                 /* for peer attributes we retry upon failure */
>                 ret = ufshcd_send_uic_cmd(hba, &uic_cmd);
>                 if (ret)
> @@ -4465,6 +4466,7 @@ int ufshcd_dme_get_attr(struct ufs_hba *hba, u32
> attr_sel,
>         }
>  
>         do {
> +               uic_cmd.argument2 &= ~MASK_UIC_COMMAND_RESULT;
>                 /* for peer attributes we retry upon failure */
>                 ret = ufshcd_send_uic_cmd(hba, &uic_cmd);
>                 if (ret)

Stanley,

AI Sashiko comment on arg3 should not happen on a spec compliant ufshci
controller, since arg3 is an input for dme_set, arg2 indeed has an output byte
for every command, but restoring arg3 in the set loop would be cheap safety
enhancement, but up to you:

for me this patch is ok:

Reviewed-by: Bean Huo <beanhuo@micron.com>

[PATCH 3/3] scsi: ufs: core: Clear stale UIC result byte before peer DME retries

[Stanley Jhu]

Peer DME get and set retry a failed attempt with the same struct
uic_command. UIC completion ORs the ConfigResultCode into
argument2[7:0], and nothing clears it before the next attempt. Once an
attempt fails, every retry keeps reporting that error code, even when
the device returns success, so a transient failure becomes a hard
failure after three retries. Each such retry also triggers the
WARN_ON_ONCE() in ufshcd_dispatch_uic_cmd().

Clear the result byte before each attempt.

Fixes: 64238fbd15ea ("scsi: ufs: add retries to dme_peer get and set attribute")
Cc: stable@vger.kernel.org
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
 drivers/ufs/core/ufshcd.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index bcc2512bf34b..9adb3b576e28 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -4398,6 +4398,7 @@ int ufshcd_dme_set_attr(struct ufs_hba *hba, u32 attr_sel,
 	int retries = UFS_UIC_COMMAND_RETRIES;
 
 	do {
+		uic_cmd.argument2 &= ~MASK_UIC_COMMAND_RESULT;
 		/* for peer attributes we retry upon failure */
 		ret = ufshcd_send_uic_cmd(hba, &uic_cmd);
 		if (ret)
@@ -4465,6 +4466,7 @@ int ufshcd_dme_get_attr(struct ufs_hba *hba, u32 attr_sel,
 	}
 
 	do {
+		uic_cmd.argument2 &= ~MASK_UIC_COMMAND_RESULT;
 		/* for peer attributes we retry upon failure */
 		ret = ufshcd_send_uic_cmd(hba, &uic_cmd);
 		if (ret)
-- 
2.56.0.385.gd3acb90ef8-goog