Hi Alexey,
On 2026-09-18T06:35:41, Alexey Charkov <[email protected]> wrote:
> ufs: report the logical unit the boot W-LU presents
>
> The boot W-LU carries the medium of whichever boot logical unit bBootLunEn
> selects, and describes none of its own, so the SCSI scan cannot size it.
> Resolve the attribute against each unit descriptor's bBootLunID to say
> which unit that is.
>
> Signed-off-by: Alexey Charkov <[email protected]>
>
> drivers/ufs/ufs-uclass.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
> drivers/ufs/ufs.h | 28 ++++++++++++++++++++++++++++
> 2 files changed, 72 insertions(+)
> diff --git a/drivers/ufs/ufs-uclass.c b/drivers/ufs/ufs-uclass.c
> @@ -2293,6 +2293,49 @@ int ufshcd_probe(struct udevice *ufs_dev, struct
> ufs_hba_ops *hba_ops)
> +/**
> + * ufs_scsi_wlun_alias - find the logical unit the boot W-LU presents
> + *
> + * The boot W-LU stands in for whichever of the two boot logical units
> + * bBootLunEn selects, and reports no medium of its own. Report that unit so
> + * that its geometry can be borrowed.
> + */
Please can you document the two parameters and the return value here,
including the -ENOENT cases.
> diff --git a/drivers/ufs/ufs-uclass.c b/drivers/ufs/ufs-uclass.c
> @@ -2293,6 +2293,49 @@ int ufshcd_probe(struct udevice *ufs_dev, struct
> ufs_hba_ops *hba_ops)
> + err = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_READ_ATTR,
> + QUERY_ATTR_IDN_BOOT_LU_EN, 0, 0,
> + &boot_lun_en);
> + if (err)
> + return err;
This can return a positive value. When the device rejects the query,
ufshcd_exec_dev_cmd() passes back the result of
ufshcd_check_query_response(), which is the query response code (0xf6
to 0xff, e.g. QUERY_RESULT_NOT_READABLE). The caller in
scsi_detect_dev() only checks for alias < 0, so it takes that code as
a LUN and sends READ CAPACITY to it.
Please can you return a negative errno here, e.g. -EIO when err is positive?
> diff --git a/drivers/ufs/ufs-uclass.c b/drivers/ufs/ufs-uclass.c
> @@ -2293,6 +2293,49 @@ int ufshcd_probe(struct udevice *ufs_dev, struct
> ufs_hba_ops *hba_ops)
> + for (i = 0; i < UFS_MAX_LUNS; i++) {
> + err = ufshcd_read_desc_param(hba, QUERY_DESC_IDN_UNIT, i,
> + UNIT_DESC_PARAM_BOOT_LUN_ID,
> + &boot_lun_id, 1);
> + if (err)
> + continue;
UFS_MAX_LUNS is 0x7f, but a device only has 8 or 32 unit descriptors,
depending on bMaxNumberLU in the geometry descriptor. I suspect every
index past that fails with a query response code. That is not -EINVAL,
so ufshcd_query_descriptor_retry() tries it three times, with a
dev_err() from __ufshcd_query_descriptor() each time and another from
ufshcd_read_desc_param().
So when no unit matches, this likely prints several hundred error
lines before the 'No logical unit carries boot LU' message, and sends
a lot of pointless queries, in SPL as well.
Please can you bound the loop by the number of units the device
supports? Linux reads bMaxNumberLU into max_lu_supported for this.
Stopping at the first failure would be simpler, although it also gives
up on a real error. What do you think?
Just to check, can a unit with bLUEnable == 0 still carry a non-zero
bBootLunID? If so, this could pick a disabled unit, so it may be worth
checking UNIT_DESC_PARAM_LU_ENABLE as well. I am not sure what the
spec says here.
Regards,
Simon
