Re: [PATCH v1 01/16] ufs: core: Probe for ext_iid support

[Bart Van Assche]

On 9/22/22 18:05, Asutosh Das wrote:
> +	hba->mcq_capabilities = ufshcd_readl(hba, REG_MCQCAP);

What value is reported when reading the REG_MCQCAP register on an UFSHCI 
3.0 controller? -1 or 0?

> +	hba->ext_iid_sup = (hba->mcq_capabilities & MASK_EXT_IID_SUPPORT) >>
> +		EXT_IID_CAP_SHIFT;

[ ... ]

> +	if (dev_info->wspecversion < 0x400)
> +		goto out;

Isn't this version check superfluous? Only UFSHCI 4.0 controllers should 
support the extended IID feature.

> +	ext_ufs_feature = get_unaligned_be32(desc_buf + DEVICE_DESC_PARAM_EXT_UFS_FEATURE_SUP);

The above change introduces a third instance of this code. Please 
introduce a helper function that does something like the following and 
replace the above line with a call to that helper function:

	if (hba->desc_size[QUERY_DESC_IDN_DEVICE] <
	    DEVICE_DESC_PARAM_EXT_UFS_FEATURE_SUP + 4)
		return 0;

	return get_unaligned_be32(desc_buf +
				 DEVICE_DESC_PARAM_EXT_UFS_FEATURE_SUP);

> +out:
> +	dev_info->b_ext_iid_en = !!ext_iid_en;

Please remove "!!". This conversion happens implicitly when assigning to 
a boolean variable.

Thanks,

Bart.