Re: [PATCH 1/2] ufs: rpmb: retry power-on UNIT ATTENTION on the RPMB WLUN

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The code uses SCMD_FAILURE_ASC_ANY to retry any Unit Attention, contradicting the documented intent.
- [Low] The 'sshdr' variable is allocated and passed to scsi_execute_cmd but is never used by the caller.
--

--- Patch [1]: [PATCH 1/2] ufs: rpmb: retry power-on UNIT ATTENTION on the RPMB WLUN ---
commit 7f3e584cbe66720f776d9dd47015f86621c3fdb8
Author: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>

ufs: rpmb: retry power-on UNIT ATTENTION on the RPMB WLUN

After a power cycle, the first command to the RPMB WLUN fails with a
power-on UNIT ATTENTION. This patch asks the SCSI core to retry this
condition so RPMB works immediately after boot.

> diff --git a/drivers/ufs/core/ufs-rpmb.c b/drivers/ufs/core/ufs-rpmb.c
> index ffad049872b9a..adbec80cf3811 100644
> --- a/drivers/ufs/core/ufs-rpmb.c
> +++ b/drivers/ufs/core/ufs-rpmb.c
> @@ -40,6 +40,25 @@ struct ufs_rpmb_dev {
>  static int ufs_sec_submit(struct ufs_hba *hba, u16 spsp, void *buffer, size_t len, bool send)
>  {
>  	struct scsi_device *sdev = hba->ufs_rpmb_wlun;
> +	struct scsi_sense_hdr sshdr;

[Severity: Low]
Does this code need to allocate and pass sshdr?

It appears that sshdr is passed to scsi_execute_cmd() via exec_args but the
result is never read by the caller. Leaving .sshdr as NULL would avoid the
unnecessary sense data normalization overhead in scsi_execute_cmd().

> +	/* Retry the power-on UNIT ATTENTION (ASC 0x29); the SCSI core does not. */
> +	struct scsi_failure failure_defs[] = {
> +		{
> +			.sense = UNIT_ATTENTION,
> +			.asc = SCMD_FAILURE_ASC_ANY,

[Severity: Medium]
Will this retry ANY Unit Attention condition instead of just ASC 0x29?

The commit message and inline comment explicitly target the power-on
UNIT ATTENTION (ASC 0x29), but using SCMD_FAILURE_ASC_ANY configures the SCSI
core to retry any Unit Attention condition up to 3 times. Could this mask
other unexpected device state changes?

> +			.ascq = SCMD_FAILURE_ASCQ_ANY,
> +			.allowed = 3,
> +			.result = SAM_STAT_CHECK_CONDITION,
> +		},
> +		{}
> +	};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715214327.1933560-1-jorge.ramirez@oss.qualcomm.com?part=1