[PATCH v2 1/2] scsi: ufs: Add the aggregated read query opcode and its reply format

[PATCH v2 1/2] scsi: ufs: Add the aggregated read query opcode and its reply format

[Hyeoncheol Jeong]

UFS 5.0 adds the AGGREGATED READ query opcode (0x9): a single QUERY
RESPONSE UPIU returns many descriptors, attributes and flags as a chain
of typed groups (JESD220H 10.7.9.14).

Add the AGGREGATION TYPE mask, the group types and the group header, and
let ufshcd_copy_query_response() copy the reply's data segment like a
descriptor read, recording its length in struct ufs_query_res.

Signed-off-by: Hyeoncheol Jeong <hyenc.jeong@samsung.com>
---
 drivers/ufs/core/ufshcd.c |  7 +++++--
 include/ufs/ufs.h         | 44 +++++++++++++++++++++++++++++++++++++++
 include/ufs/ufshcd.h      |  2 ++
 3 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 54f4e7d7de02..d07834fc4646 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -2473,10 +2473,13 @@ int ufshcd_copy_query_response(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
 	struct ufs_query_res *query_res = &hba->dev_cmd.query.response;
 
 	memcpy(&query_res->upiu_res, &lrbp->ucd_rsp_ptr->qr, QUERY_OSF_SIZE);
+	query_res->data_segment_length =
+		be16_to_cpu(lrbp->ucd_rsp_ptr->header.data_segment_length);
 
-	/* Get the descriptor */
+	/* Get the descriptor or aggregated data packet */
 	if (hba->dev_cmd.query.descriptor &&
-	    lrbp->ucd_rsp_ptr->qr.opcode == UPIU_QUERY_OPCODE_READ_DESC) {
+	    (lrbp->ucd_rsp_ptr->qr.opcode == UPIU_QUERY_OPCODE_READ_DESC ||
+	     lrbp->ucd_rsp_ptr->qr.opcode == UPIU_QUERY_OPCODE_AGGREGATED_READ)) {
 		u8 *descp = (u8 *)lrbp->ucd_rsp_ptr +
 				GENERAL_UPIU_REQUEST_SIZE;
 		u16 resp_len;
diff --git a/include/ufs/ufs.h b/include/ufs/ufs.h
index afbb32654fab..1e6ea87a5dc7 100644
--- a/include/ufs/ufs.h
+++ b/include/ufs/ufs.h
@@ -30,6 +30,7 @@ static_assert(sizeof(struct utp_upiu_query) == 20);
  * (ALIGNED_DEVMAN_RSP_SIZE) minus the fixed UPIU header it follows.
  */
 #define QUERY_AGGREGATED_MAX_SIZE (4096 - GENERAL_UPIU_REQUEST_SIZE)
+#define QUERY_AGG_GROUP_HDR_SIZE  4
 #define QUERY_DESC_MIN_SIZE       2
 #define QUERY_DESC_HDR_SIZE       2
 #define QUERY_OSF_SIZE            (GENERAL_UPIU_REQUEST_SIZE - \
@@ -569,6 +570,49 @@ enum ufs_dev_pwr_mode {
 
 #define UFS_WB_BUF_REMAIN_PERCENT(val) ((val) / 10)
 
+/* AGGREGATION TYPE field of an AGGREGATED READ query request */
+enum ufs_agg_type {
+	UFS_AGG_TYPE_ALL_FLAGS			= BIT(0),
+	UFS_AGG_TYPE_ALL_ATTRS			= BIT(1),
+	UFS_AGG_TYPE_DEVICE_DESC		= BIT(2),
+	/* Unit descriptors and the RPMB unit descriptor */
+	UFS_AGG_TYPE_UNIT_RPMB_DESC		= BIT(3),
+	UFS_AGG_TYPE_INTERCONNECT_DESC	= BIT(4),
+	UFS_AGG_TYPE_STRING_DESC		= BIT(5),
+	/* Geometry descriptor and power descriptor */
+	UFS_AGG_TYPE_GEOMETRY_POWER_DESC	= BIT(6),
+	UFS_AGG_TYPE_HEALTH_DESC		= BIT(7),
+};
+
+/* Group Type field of an aggregated data packet group header. */
+enum ufs_agg_group_type {
+	UFS_AGG_GROUP_RESERVED		= 0x00,
+	UFS_AGG_GROUP_FLAGS		= 0x01,
+	UFS_AGG_GROUP_ATTRS		= 0x02,
+	/* Any descriptor with a unique IDN, i.e. all but the string ones */
+	UFS_AGG_GROUP_DESCS		= 0x03,
+	UFS_AGG_GROUP_MANUFACTURER_STR	= 0x04,
+	UFS_AGG_GROUP_PRODUCT_NAME_STR	= 0x05,
+	UFS_AGG_GROUP_OEM_ID_STR	= 0x06,
+	UFS_AGG_GROUP_SERIAL_NUMBER_STR	= 0x07,
+	UFS_AGG_GROUP_PRODUCT_REV_STR	= 0x08,
+};
+
+/**
+ * struct utp_agg_group_header - aggregated data packet group header
+ * @group_type: type of the data carried by this group, see ufs_agg_group_type
+ * @reserved: reserved
+ * @next_group_offset: byte offset from the start of the packet to the next
+ *	group; zero ends the chain
+ */
+struct utp_agg_group_header {
+	u8 group_type;
+	u8 reserved;
+	__be16 next_group_offset;
+};
+
+static_assert(sizeof(struct utp_agg_group_header) == QUERY_AGG_GROUP_HDR_SIZE);
+
 /**
  * struct utp_cmd_rsp - RESPONSE UPIU structure
  * @residual_transfer_count: Residual transfer count DW-3
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index dfd302f2dc7c..1a5e181ff9e3 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -213,9 +213,11 @@ struct ufs_query_req {
 /**
  * struct ufs_query_res - UPIU QUERY
  * @upiu_res: query response data
+ * @data_segment_length: response data segment length
  */
 struct ufs_query_res {
 	struct utp_upiu_query upiu_res;
+	u16 data_segment_length;
 };
 
 /**
-- 
2.25.1

Re: [PATCH v2 1/2] scsi: ufs: Add the aggregated read query opcode and its reply format

[Peter Wang]

On Wed, 2026-09-30 at 14:15 +0900, Hyeoncheol Jeong wrote:
> UFS 5.0 adds the AGGREGATED READ query opcode (0x9): a single QUERY
> RESPONSE UPIU returns many descriptors, attributes and flags as a
> chain
> of typed groups (JESD220H 10.7.9.14).
> 
> Add the AGGREGATION TYPE mask, the group types and the group header,
> and
> let ufshcd_copy_query_response() copy the reply's data segment like a
> descriptor read, recording its length in struct ufs_query_res.
> 
> Signed-off-by: Hyeoncheol Jeong <hyenc.jeong@samsung.com>

Reviewed-by: Peter Wang <peter.wang@mediatek.com>

[PATCH v2 2/2] scsi: ufs: Serve the device init reads from one aggregated read

[Hyeoncheol Jeong]

Device init reads several attributes and flags one query at a time. On
UFS 5.0, issue one AGGREGATED READ once the device descriptor has
established wSpecVersion, cache the reply, and serve the following
attribute, flag and ID-string reads from it. Anything not present, and
older devices or a rejected opcode, fall back to individual queries.

The following per-item queries are folded into the one aggregated read:

Attributes (ALL_ATTRS group)
	0x0c bMaxNumOfRTT                  - ufshcd_set_rtt()
	0x17 bRefClkGatingWaitTime         - ufshcd_get_ref_clk_gating_wait()
	0x1e bWriteBoosterBufferLifeTimeEst - ufshcd_wb_probe()

Flags (ALL_FLAGS group)
	0x03 fPowerOnWPEn                  - ufshcd_device_params_init()

Strings (STRING_DESC groups)
	Product Name string                - ufs_get_device_desc()
	Serial Number string               - ufshcd_create_device_id()

(bWriteBoosterBufferLifeTimeEst is served from the packet only in the
shared-buffer mode)

Signed-off-by: Hyeoncheol Jeong <hyenc.jeong@samsung.com>
---
 drivers/ufs/core/ufshcd.c | 357 ++++++++++++++++++++++++++++++++++++--
 include/ufs/ufs.h         |   3 +
 include/ufs/ufshcd.h      |   9 +
 3 files changed, 351 insertions(+), 18 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index d07834fc4646..e4135d4a5812 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -3792,6 +3792,314 @@ int ufshcd_query_descriptor_retry(struct ufs_hba *hba,
 	return err;
 }
 
+/* Attribute data width by IDN for aggregated read (JESD220H Table 14.28). */
+static const u8 ufs_agg_attr_width[] = {
+	[QUERY_ATTR_IDN_BOOT_LU_EN] = 1, [QUERY_ATTR_IDN_POWER_MODE] = 1,
+	[QUERY_ATTR_IDN_ACTIVE_ICC_LVL] = 1, [QUERY_ATTR_IDN_OOO_DATA_EN] = 1,
+	[QUERY_ATTR_IDN_BKOPS_STATUS] = 1, [QUERY_ATTR_IDN_PURGE_STATUS] = 1,
+	[QUERY_ATTR_IDN_MAX_DATA_IN] = 1, [QUERY_ATTR_IDN_MAX_DATA_OUT] = 1,
+	[QUERY_ATTR_IDN_DYN_CAP_NEEDED] = 4, [QUERY_ATTR_IDN_REF_CLK_FREQ] = 1,
+	[QUERY_ATTR_IDN_CONF_DESC_LOCK] = 1, [QUERY_ATTR_IDN_MAX_NUM_OF_RTT] = 1,
+	[QUERY_ATTR_IDN_EE_CONTROL] = 2, [QUERY_ATTR_IDN_EE_STATUS] = 2,
+	[QUERY_ATTR_IDN_SECONDS_PASSED] = 4, [QUERY_ATTR_IDN_CNTX_CONF] = 2,
+	[QUERY_ATTR_IDN_FFU_STATUS] = 1, [QUERY_ATTR_IDN_PSA_STATE] = 1,
+	[QUERY_ATTR_IDN_PSA_DATA_SIZE] = 4,
+	[QUERY_ATTR_IDN_REF_CLK_GATING_WAIT_TIME] = 1,
+	[QUERY_ATTR_IDN_CASE_ROUGH_TEMP] = 1, [QUERY_ATTR_IDN_HIGH_TEMP_BOUND] = 1,
+	[QUERY_ATTR_IDN_LOW_TEMP_BOUND] = 1, [0x1b] = 1,
+	[QUERY_ATTR_IDN_WB_FLUSH_STATUS] = 1,
+	[QUERY_ATTR_IDN_AVAIL_WB_BUFF_SIZE] = 1,
+	[QUERY_ATTR_IDN_WB_BUFF_LIFE_TIME_EST] = 1,
+	[QUERY_ATTR_IDN_CURR_WB_BUFF_SIZE] = 4,
+};
+
+/**
+ * ufshcd_agg_group - find a group in the cached aggregated data packet
+ * @hba: per-adapter instance
+ * @type: group type to find
+ * @group_len: set to the group payload length on a hit
+ *
+ * Return: the group payload, or NULL if @type is not present.
+ */
+static const u8 *ufshcd_agg_group(const struct ufs_hba *hba, u8 type,
+				  u16 *group_len)
+{
+	const u8 *packet = hba->agg_packet;
+	u16 packet_len = hba->agg_packet_len;
+	u16 group_off = 0;
+
+	while (packet && group_off + QUERY_AGG_GROUP_HDR_SIZE <= packet_len) {
+		const struct utp_agg_group_header *hdr =
+			(const void *)(packet + group_off);
+		u16 next_group_off = be16_to_cpu(hdr->next_group_offset);
+		u16 payload_off = group_off + QUERY_AGG_GROUP_HDR_SIZE;
+
+		if (next_group_off && (next_group_off < payload_off ||
+				       next_group_off + QUERY_AGG_GROUP_HDR_SIZE > packet_len))
+			return NULL;
+
+		if (hdr->group_type == type) {
+			*group_len = (next_group_off ? next_group_off : packet_len) -
+				     payload_off;
+			return packet + payload_off;
+		}
+
+		if (!next_group_off)
+			break;
+
+		group_off = next_group_off;
+	}
+
+	return NULL;
+}
+
+/**
+ * ufshcd_agg_string - find a string descriptor in the cached aggregated packet
+ * @hba: per-adapter instance
+ * @index: string descriptor index
+ * @len: set to the descriptor length on a hit
+ *
+ * Return: the string descriptor, or NULL if not present.
+ */
+static const u8 *ufshcd_agg_string(struct ufs_hba *hba, u8 index, u8 *len)
+{
+	u16 group_len;
+	const u8 *group_buf;
+	int i;
+
+	if (!index)
+		return NULL;
+
+	for (i = 0; i < ARRAY_SIZE(hba->agg_str_idx); i++)
+		if (hba->agg_str_idx[i] == index)
+			break;
+
+	if (i == ARRAY_SIZE(hba->agg_str_idx))
+		return NULL;
+
+	group_buf = ufshcd_agg_group(hba, UFS_AGG_GROUP_MANUFACTURER_STR + i,
+				     &group_len);
+	if (!group_buf || group_len < QUERY_DESC_HDR_SIZE)
+		return NULL;
+
+	*len = group_buf[QUERY_DESC_LENGTH_OFFSET];
+	if (*len < QUERY_DESC_HDR_SIZE || *len > group_len)
+		return NULL;
+
+	return group_buf;
+}
+
+/**
+ * ufshcd_agg_desc - find a descriptor in the cached aggregated data packet
+ * @hba: per-adapter instance
+ * @idn: descriptor IDN
+ * @index: unit index, for unit descriptors; string index for string ones
+ * @len: set to the descriptor length on a hit
+ *
+ * Return: the descriptor, or NULL if not present.
+ */
+static const u8 *ufshcd_agg_desc(struct ufs_hba *hba, enum desc_idn idn,
+				 u8 index, u8 *len)
+{
+	u16 group_len, off = 0;
+	const u8 *group_buf;
+
+	if (idn == QUERY_DESC_IDN_STRING)
+		return ufshcd_agg_string(hba, index, len);
+
+	group_buf = ufshcd_agg_group(hba, UFS_AGG_GROUP_DESCS, &group_len);
+	while (group_buf && off + QUERY_DESC_HDR_SIZE <= group_len) {
+		const u8 *desc_buf = group_buf + off;
+		u8 desc_len = desc_buf[QUERY_DESC_LENGTH_OFFSET];
+
+		if (desc_len < QUERY_DESC_HDR_SIZE || off + desc_len > group_len)
+			break;
+
+		if (desc_buf[QUERY_DESC_DESC_TYPE_OFFSET] == idn &&
+		    (idn != QUERY_DESC_IDN_UNIT ||
+		     (desc_len > UNIT_DESC_PARAM_UNIT_INDEX &&
+		      desc_buf[UNIT_DESC_PARAM_UNIT_INDEX] == index))) {
+			*len = desc_len;
+			return desc_buf;
+		}
+
+		off += desc_len;
+	}
+
+	return NULL;
+}
+
+/**
+ * ufshcd_agg_attr - read an attribute from the cached aggregated data packet
+ * @hba: per-adapter instance
+ * @idn: attribute IDN (index 0 only)
+ * @out: set to the value on a hit
+ *
+ * Return: 0 on a hit, -ENOENT otherwise.
+ */
+static int ufshcd_agg_attr(struct ufs_hba *hba, u8 idn, u32 *out)
+{
+	u16 group_len, off = 0;
+	const u8 *group_buf;
+	int i;
+
+	if (idn >= ARRAY_SIZE(ufs_agg_attr_width) || !ufs_agg_attr_width[idn])
+		return -ENOENT;
+
+	group_buf = ufshcd_agg_group(hba, UFS_AGG_GROUP_ATTRS, &group_len);
+	if (!group_buf)
+		return -ENOENT;
+
+	for (i = 0; i < idn; i++)
+		off += ufs_agg_attr_width[i];
+
+	if (off + ufs_agg_attr_width[idn] > group_len)
+		return -ENOENT;
+
+	*out = 0;
+	for (i = 0; i < ufs_agg_attr_width[idn]; i++)
+		*out = (*out << 8) | group_buf[off + i];
+
+	return 0;
+}
+
+/**
+ * ufshcd_agg_flag - read a flag from the cached aggregated data packet
+ * @hba: per-adapter instance
+ * @idn: flag IDN
+ * @out: set to the value on a hit
+ *
+ * Return: 0 on a hit, -ENOENT otherwise.
+ */
+static int ufshcd_agg_flag(struct ufs_hba *hba, u8 idn, bool *out)
+{
+	u16 group_len;
+	const u8 *group_buf = ufshcd_agg_group(hba, UFS_AGG_GROUP_FLAGS,
+					       &group_len);
+
+	if (!group_buf || idn >= group_len)
+		return -ENOENT;
+
+	*out = group_buf[idn];
+	return 0;
+}
+
+/**
+ * ufshcd_read_attr - read an attribute, from the cached packet if present
+ * @hba: per-adapter instance
+ * @idn: attribute IDN
+ * @index: attribute index
+ * @out: result
+ *
+ * Return: 0 on success, < 0 on error.
+ */
+static int ufshcd_read_attr(struct ufs_hba *hba, u8 idn, u8 index, u32 *out)
+{
+	if (index == 0 && !ufshcd_agg_attr(hba, idn, out))
+		return 0;
+
+	return ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_READ_ATTR, idn,
+				       index, 0, out);
+}
+
+/**
+ * ufshcd_read_flag - read a flag, from the cached packet if present
+ * @hba: per-adapter instance
+ * @idn: flag IDN
+ * @out: result
+ *
+ * Return: 0 on success, < 0 on error.
+ */
+static int ufshcd_read_flag(struct ufs_hba *hba, u8 idn, bool *out)
+{
+	if (!ufshcd_agg_flag(hba, idn, out))
+		return 0;
+
+	return ufshcd_query_flag_retry(hba, UPIU_QUERY_OPCODE_READ_FLAG, idn, 0,
+				       out);
+}
+
+/**
+ * ufshcd_query_aggregated_read - issue an AGGREGATED READ query
+ * @hba: per-adapter instance
+ * @agg_type: AGGREGATION TYPE mask (OSF) selecting the items to fetch
+ * @buf: buffer for the reply data segment
+ * @buf_len: requested length in, received length out
+ *
+ * Return: 0 on success; > 0 on an OCS error; < 0 otherwise.
+ */
+static int ufshcd_query_aggregated_read(struct ufs_hba *hba, u8 agg_type,
+					u8 *buf, int *buf_len)
+{
+	struct ufs_query_req *request = NULL;
+	struct ufs_query_res *response = NULL;
+	int err;
+
+	if (*buf_len <= 0 || *buf_len > QUERY_AGGREGATED_MAX_SIZE)
+		return -EINVAL;
+
+	ufshcd_dev_man_lock(hba);
+	ufshcd_init_query(hba, &request, &response,
+			  UPIU_QUERY_OPCODE_AGGREGATED_READ, agg_type, 0, 0);
+	request->query_func = UPIU_QUERY_FUNC_STANDARD_READ_REQUEST;
+	request->upiu_req.length = cpu_to_be16(*buf_len);
+	hba->dev_cmd.query.descriptor = buf;
+
+	err = ufshcd_exec_dev_cmd(hba, DEV_CMD_TYPE_QUERY, dev_cmd_timeout);
+	if (!err)
+		*buf_len = response->data_segment_length;
+
+	hba->dev_cmd.query.descriptor = NULL;
+	ufshcd_dev_man_unlock(hba);
+	return err;
+}
+
+/**
+ * ufshcd_agg_read_begin - cache one AGGREGATED READ for the reads that follow
+ * @hba: per-adapter instance
+ * @agg_type: AGGREGATION TYPE mask to fetch
+ *
+ * Paired with ufshcd_agg_read_end(). Requires a UFS 5.0 device.
+ */
+static void ufshcd_agg_read_begin(struct ufs_hba *hba, u8 agg_type)
+{
+	int len = QUERY_AGGREGATED_MAX_SIZE;
+	int retries;
+	u8 *packet;
+
+	if (hba->dev_info.wspecversion < 0x500 ||
+	    hba->dev_info.agg_read_unsupported)
+		return;
+
+	/* Zeroed so a device over-reporting LENGTH terminates the walk safely. */
+	packet = kzalloc(QUERY_AGGREGATED_MAX_SIZE, GFP_KERNEL);
+	if (!packet)
+		return;
+
+	for (retries = QUERY_REQ_RETRIES; retries > 0; retries--) {
+		if (!ufshcd_query_aggregated_read(hba, agg_type, packet, &len))
+			break;
+	}
+
+	if (!retries) {
+		dev_dbg(hba->dev, "aggregated read unsupported, using individual queries\n");
+		hba->dev_info.agg_read_unsupported = true;
+		kfree(packet);
+		return;
+	}
+
+	hba->agg_packet = packet;
+	hba->agg_packet_len = len;
+}
+
+/* ufshcd_agg_read_end - drop the packet cached by ufshcd_agg_read_begin() */
+static void ufshcd_agg_read_end(struct ufs_hba *hba)
+{
+	kfree(hba->agg_packet);
+	hba->agg_packet = NULL;
+	hba->agg_packet_len = 0;
+}
+
 /**
  * ufshcd_read_desc_param - read the specified descriptor parameter
  * @hba: Pointer to adapter instance
@@ -3813,6 +4121,8 @@ int ufshcd_read_desc_param(struct ufs_hba *hba,
 {
 	int ret;
 	u8 *desc_buf;
+	const u8 *descp;
+	u8 desc_len;
 	int buff_len = QUERY_DESC_MAX_SIZE;
 	bool is_kmalloc = true;
 
@@ -3830,14 +4140,20 @@ int ufshcd_read_desc_param(struct ufs_hba *hba,
 		is_kmalloc = false;
 	}
 
-	/* Request for full descriptor */
-	ret = ufshcd_query_descriptor_retry(hba, UPIU_QUERY_OPCODE_READ_DESC,
-					    desc_id, desc_index, 0,
-					    desc_buf, &buff_len);
-	if (ret) {
-		dev_err(hba->dev, "%s: Failed reading descriptor. desc_id %d, desc_index %d, param_offset %d, ret %d\n",
-			__func__, desc_id, desc_index, param_offset, ret);
-		goto out;
+	/* Serve from the cached aggregated packet, else request the full one. */
+	descp = ufshcd_agg_desc(hba, desc_id, desc_index, &desc_len);
+	if (descp) {
+		memcpy(desc_buf, descp, desc_len);
+		ret = 0;
+	} else {
+		ret = ufshcd_query_descriptor_retry(hba, UPIU_QUERY_OPCODE_READ_DESC,
+						    desc_id, desc_index, 0,
+						    desc_buf, &buff_len);
+		if (ret) {
+			dev_err(hba->dev, "%s: Failed reading descriptor. desc_id %d, desc_index %d, param_offset %d, ret %d\n",
+				__func__, desc_id, desc_index, param_offset, ret);
+			goto out;
+		}
 	}
 
 	/* Update descriptor length */
@@ -4019,9 +4335,8 @@ static int ufshcd_get_ref_clk_gating_wait(struct ufs_hba *hba)
 	u32 gating_wait = UFSHCD_REF_CLK_GATING_WAIT_US;
 
 	if (hba->dev_info.wspecversion >= 0x300) {
-		err = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_READ_ATTR,
-				QUERY_ATTR_IDN_REF_CLK_GATING_WAIT_TIME, 0, 0,
-				&gating_wait);
+		err = ufshcd_read_attr(hba, QUERY_ATTR_IDN_REF_CLK_GATING_WAIT_TIME,
+				       0, &gating_wait);
 		if (err)
 			dev_err(hba->dev, "Failed reading bRefClkGatingWait. err = %d, use default %uus\n",
 					 err, gating_wait);
@@ -6531,9 +6846,8 @@ static bool ufshcd_is_wb_buf_lifetime_available(struct ufs_hba *hba)
 	u8 index;
 
 	index = ufshcd_wb_get_query_index(hba);
-	ret = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_READ_ATTR,
-				      QUERY_ATTR_IDN_WB_BUFF_LIFE_TIME_EST,
-				      index, 0, &lifetime);
+	ret = ufshcd_read_attr(hba, QUERY_ATTR_IDN_WB_BUFF_LIFE_TIME_EST,
+			       index, &lifetime);
 	if (ret) {
 		dev_err(hba->dev,
 			"%s: bWriteBoosterBufferLifeTimeEst read failed %d\n",
@@ -8677,8 +8991,7 @@ static void ufshcd_set_rtt(struct ufs_hba *hba)
 	if (dev_info->wspecversion < 0x400)
 		return;
 
-	if (ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_READ_ATTR,
-				    QUERY_ATTR_IDN_MAX_NUM_OF_RTT, 0, 0, &dev_rtt)) {
+	if (ufshcd_read_attr(hba, QUERY_ATTR_IDN_MAX_NUM_OF_RTT, 0, &dev_rtt)) {
 		dev_err(hba->dev, "failed reading bMaxNumOfRTT\n");
 		return;
 	}
@@ -8899,6 +9212,14 @@ static int ufs_get_device_desc(struct ufs_hba *hba)
 				      desc_buf[DEVICE_DESC_PARAM_SPEC_VER + 1];
 	dev_info->bqueuedepth = desc_buf[DEVICE_DESC_PARAM_Q_DPTH];
 
+	hba->agg_str_idx[0] = desc_buf[DEVICE_DESC_PARAM_MANF_NAME];
+	hba->agg_str_idx[1] = desc_buf[DEVICE_DESC_PARAM_PRDCT_NAME];
+	hba->agg_str_idx[2] = desc_buf[DEVICE_DESC_PARAM_OEM_ID];
+	hba->agg_str_idx[3] = desc_buf[DEVICE_DESC_PARAM_SN];
+	hba->agg_str_idx[4] = desc_buf[DEVICE_DESC_PARAM_PRDCT_REV];
+	ufshcd_agg_read_begin(hba, UFS_AGG_TYPE_ALL_ATTRS | UFS_AGG_TYPE_ALL_FLAGS |
+			      UFS_AGG_TYPE_STRING_DESC);
+
 	/*
 	 * According to the UFS standard, the UFS device queue depth
 	 * (bQueueDepth) must be in the range 1..255 if the shared queueing
@@ -9214,8 +9535,7 @@ static int ufshcd_device_params_init(struct ufs_hba *hba)
 
 	ufshcd_get_ref_clk_gating_wait(hba);
 
-	if (!ufshcd_query_flag_retry(hba, UPIU_QUERY_OPCODE_READ_FLAG,
-			QUERY_FLAG_IDN_PWR_ON_WPE, 0, &flag))
+	if (!ufshcd_read_flag(hba, QUERY_FLAG_IDN_PWR_ON_WPE, &flag))
 		hba->dev_info.f_power_on_wp_en = flag;
 
 	/* Probe maximum power mode co-supported by both UFS host and device */
@@ -9226,6 +9546,7 @@ static int ufshcd_device_params_init(struct ufs_hba *hba)
 
 	ufshcd_retrieve_tx_eq_settings(hba);
 out:
+	ufshcd_agg_read_end(hba);
 	return ret;
 }
 
diff --git a/include/ufs/ufs.h b/include/ufs/ufs.h
index 1e6ea87a5dc7..e29552d6acc2 100644
--- a/include/ufs/ufs.h
+++ b/include/ufs/ufs.h
@@ -707,6 +707,9 @@ struct ufs_dev_info {
 
 	bool hid_sup;
 
+	/* Set once the device has rejected an AGGREGATED READ query. */
+	bool agg_read_unsupported;
+
 	/* Unique device ID string (manufacturer+model+serial+version+date) */
 	char *device_id;
 	u8 rpmb_io_size;
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index 1a5e181ff9e3..53a7573890cd 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -1098,6 +1098,10 @@ enum ufshcd_mcq_opr {
  * @device_deemphasis_cap: a bitfield to indicate supported DeEmphasis dBs of device's TX lanes,
  *	cache of device M-PHY TX_HS_DeEmphasis_Setting_Capability Attribute (ID 0x12)
  * @tx_eq_params: TX Equalization settings
+ * @agg_packet: aggregated data packet cached during a read batch, or NULL
+ * @agg_packet_len: length of @agg_packet
+ * @agg_str_idx: string descriptor index per string group, indexed by
+ *	group type - UFS_AGG_GROUP_MANUFACTURER_STR
  */
 struct ufs_hba {
 	void __iomem *mmio_base;
@@ -1282,6 +1286,11 @@ struct ufs_hba {
 	u8 device_preshoot_cap;
 	u8 device_deemphasis_cap;
 	struct ufshcd_tx_eq_params tx_eq_params[UFS_HS_GEAR_MAX];
+
+	u8 *agg_packet;
+	u16 agg_packet_len;
+	u8 agg_str_idx[UFS_AGG_GROUP_PRODUCT_REV_STR -
+		       UFS_AGG_GROUP_MANUFACTURER_STR + 1];
 };
 
 /**
-- 
2.25.1

Re: [PATCH v2 2/2] scsi: ufs: Serve the device init reads from one aggregated read

[Peter Wang]

On Wed, 2026-09-30 at 14:18 +0900, Hyeoncheol Jeong wrote:
> Device init reads several attributes and flags one query at a time.
> On
> UFS 5.0, issue one AGGREGATED READ once the device descriptor has
> established wSpecVersion, cache the reply, and serve the following
> attribute, flag and ID-string reads from it. Anything not present,
> and
> older devices or a rejected opcode, fall back to individual queries.
> 
> The following per-item queries are folded into the one aggregated
> read:
> 
> Attributes (ALL_ATTRS group)
> 	0x0c bMaxNumOfRTT                  - ufshcd_set_rtt()
> 	0x17 bRefClkGatingWaitTime         -
> ufshcd_get_ref_clk_gating_wait()
> 	0x1e bWriteBoosterBufferLifeTimeEst - ufshcd_wb_probe()
> 
> Flags (ALL_FLAGS group)
> 	0x03 fPowerOnWPEn                  -
> ufshcd_device_params_init()
> 
> Strings (STRING_DESC groups)
> 	Product Name string                - ufs_get_device_desc()
> 	Serial Number string               -
> ufshcd_create_device_id()
> 
> (bWriteBoosterBufferLifeTimeEst is served from the packet only in the
> shared-buffer mode)
> 
> Signed-off-by: Hyeoncheol Jeong <hyenc.jeong@samsung.com>
> ---

Reviewed-by: Peter Wang <peter.wang@mediatek.com>

Re: [PATCH v2 2/2] scsi: ufs: Serve the device init reads from one aggregated read

[Bean Huo]

On Wed, 2026-09-30 at 14:18 +0900, Hyeoncheol Jeong wrote:
> Device init reads several attributes and flags one query at a time. On
> UFS 5.0, issue one AGGREGATED READ once the device descriptor has
> established wSpecVersion, cache the reply, and serve the following
> attribute, flag and ID-string reads from it. Anything not present, and
> older devices or a rejected opcode, fall back to individual queries.
> 
> The following per-item queries are folded into the one aggregated read:
> 
> Attributes (ALL_ATTRS group)
>         0x0c bMaxNumOfRTT                  - ufshcd_set_rtt()
>         0x17 bRefClkGatingWaitTime         - ufshcd_get_ref_clk_gating_wait()
>         0x1e bWriteBoosterBufferLifeTimeEst - ufshcd_wb_probe()
> 
> Flags (ALL_FLAGS group)
>         0x03 fPowerOnWPEn                  - ufshcd_device_params_init()
> 
> Strings (STRING_DESC groups)
>         Product Name string                - ufs_get_device_desc()
>         Serial Number string               - ufshcd_create_device_id()
> 
> (bWriteBoosterBufferLifeTimeEst is served from the packet only in the
> shared-buffer mode)


I suggest we should aggregate-read as many of them as possible. If we lose even
one descriptor, this feature becomes useless.

> 
> Signed-off-by: Hyeoncheol Jeong <hyenc.jeong@samsung.com>
> ---
>  drivers/ufs/core/ufshcd.c | 357 ++++++++++++++++++++++++++++++++++++--
>  include/ufs/ufs.h         |   3 +
>  include/ufs/ufshcd.h      |   9 +
>  3 files changed, 351 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index d07834fc4646..e4135d4a5812 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -3792,6 +3792,314 @@ int ufshcd_query_descriptor_retry(struct ufs_hba *hba,
>         return err;
>  }
>  
> +/* Attribute data width by IDN for aggregated read (JESD220H Table 14.28). */
> +static const u8 ufs_agg_attr_width[] = {
> +       [QUERY_ATTR_IDN_BOOT_LU_EN] = 1, [QUERY_ATTR_IDN_POWER_MODE] = 1,
> +       [QUERY_ATTR_IDN_ACTIVE_ICC_LVL] = 1, [QUERY_ATTR_IDN_OOO_DATA_EN] = 1,
> +       [QUERY_ATTR_IDN_BKOPS_STATUS] = 1, [QUERY_ATTR_IDN_PURGE_STATUS] = 1,
> +       [QUERY_ATTR_IDN_MAX_DATA_IN] = 1, [QUERY_ATTR_IDN_MAX_DATA_OUT] = 1,
> +       [QUERY_ATTR_IDN_DYN_CAP_NEEDED] = 4, [QUERY_ATTR_IDN_REF_CLK_FREQ] =
> 1,
> +       [QUERY_ATTR_IDN_CONF_DESC_LOCK] = 1, [QUERY_ATTR_IDN_MAX_NUM_OF_RTT] =
> 1,
> +       [QUERY_ATTR_IDN_EE_CONTROL] = 2, [QUERY_ATTR_IDN_EE_STATUS] = 2,
> +       [QUERY_ATTR_IDN_SECONDS_PASSED] = 4, [QUERY_ATTR_IDN_CNTX_CONF] = 2,
> +       [QUERY_ATTR_IDN_FFU_STATUS] = 1, [QUERY_ATTR_IDN_PSA_STATE] = 1,
> +       [QUERY_ATTR_IDN_PSA_DATA_SIZE] = 4,
> +       [QUERY_ATTR_IDN_REF_CLK_GATING_WAIT_TIME] = 1,
> +       [QUERY_ATTR_IDN_CASE_ROUGH_TEMP] = 1, [QUERY_ATTR_IDN_HIGH_TEMP_BOUND]
> = 1,
> +       [QUERY_ATTR_IDN_LOW_TEMP_BOUND] = 1, [0x1b] = 1,
> +       [QUERY_ATTR_IDN_WB_FLUSH_STATUS] = 1,
> +       [QUERY_ATTR_IDN_AVAIL_WB_BUFF_SIZE] = 1,
> +       [QUERY_ATTR_IDN_WB_BUFF_LIFE_TIME_EST] = 1,
> +       [QUERY_ATTR_IDN_CURR_WB_BUFF_SIZE] = 4,
> +};
> +


table 14.28 gives the size of each attribute, but I could not find where Spec
defines the layout of the attributes group in the aggregated data packet,
section 10.7.9.14 only defines the group header.

dDynCapNeeded (09h) is an array attribute; its number of indexes is MaxNumberLU.
1Ch-1Fh can also be per-LU in dedicated WB mode. If a device sends one entry per
index, or keeps a slot for 01h or 11h-13h, every offset after it moves. Then
bMaxNumOfRTT, bRefClkGatingWaitTime and bWriteBoosterBufferLifeTimeEst are read
from the wrong bytes, with no error and no fallback to a single query.
ufshcd_set_rtt() then writes a value based on that wrong data.

Has this been tested on more than one vendor's UFS 5.0 device?

> +/**
> + * ufshcd_agg_group - find a group in the cached aggregated data packet
> + * @hba: per-adapter instance
> + * @type: group type to find
> + * @group_len: set to the group payload length on a hit
> + *
> + * Return: the group payload, or NULL if @type is not present.
> + */
> +static const u8 *ufshcd_agg_group(const struct ufs_hba *hba, u8 type,
> +                                 u16 *group_len)
> +{
> +       const u8 *packet = hba->agg_packet;
> +       u16 packet_len = hba->agg_packet_len;
> +       u16 group_off = 0;
> +
> +       while (packet && group_off + QUERY_AGG_GROUP_HDR_SIZE <= packet_len) {
> +               const struct utp_agg_group_header *hdr =
> +                       (const void *)(packet + group_off);
> +               u16 next_group_off = be16_to_cpu(hdr->next_group_offset);
> +               u16 payload_off = group_off + QUERY_AGG_GROUP_HDR_SIZE;
> +
> +               if (next_group_off && (next_group_off < payload_off ||
> +                                      next_group_off +
> QUERY_AGG_GROUP_HDR_SIZE > packet_len))
> +                       return NULL;
> +
> +               if (hdr->group_type == type) {

if the reply is cut short (the device may return less than requested), a group
that is complete but its next offset points past the end is rejected before its
type is checked.


> +                       *group_len = (next_group_off ? next_group_off :
> packet_len) -
> +                                    payload_off;
> +                       return packet + payload_off;
> +               }
> +
> +               if (!next_group_off)
> +                       break;
> +
> +               group_off = next_group_off;
> +       }
> +
> +       return NULL;
> +}
> +
> +/**
> + * ufshcd_agg_string - find a string descriptor in the cached aggregated
> packet
> + * @hba: per-adapter instance
> + * @index: string descriptor index
> + * @len: set to the descriptor length on a hit
> + *
> + * Return: the string descriptor, or NULL if not present.
> + */
> +static const u8 *ufshcd_agg_string(struct ufs_hba *hba, u8 index, u8 *len)
> +{
> +       u16 group_len;
> +       const u8 *group_buf;
> +       int i;
> +
> +       if (!index)
> +               return NULL;
> +
> +       for (i = 0; i < ARRAY_SIZE(hba->agg_str_idx); i++)
> +               if (hba->agg_str_idx[i] == index)
> +                       break;
> +
> +       if (i == ARRAY_SIZE(hba->agg_str_idx))
> +               return NULL;
> +
> +       group_buf = ufshcd_agg_group(hba, UFS_AGG_GROUP_MANUFACTURER_STR + i,
> +                                    &group_len);
> +       if (!group_buf || group_len < QUERY_DESC_HDR_SIZE)
> +               return NULL;
> +
> +       *len = group_buf[QUERY_DESC_LENGTH_OFFSET];
> +       if (*len < QUERY_DESC_HDR_SIZE || *len > group_len)
> +               return NULL;
> +
> +       return group_buf;
> +}
> +
> +/**
> + * ufshcd_agg_desc - find a descriptor in the cached aggregated data packet
> + * @hba: per-adapter instance
> + * @idn: descriptor IDN
> + * @index: unit index, for unit descriptors; string index for string ones
> + * @len: set to the descriptor length on a hit
> + *
> + * Return: the descriptor, or NULL if not present.
> + */
> +static const u8 *ufshcd_agg_desc(struct ufs_hba *hba, enum desc_idn idn,
> +                                u8 index, u8 *len)
> +{
> +       u16 group_len, off = 0;
> +       const u8 *group_buf;
> +
> +       if (idn == QUERY_DESC_IDN_STRING)
> +               return ufshcd_agg_string(hba, index, len);
> +
> +       group_buf = ufshcd_agg_group(hba, UFS_AGG_GROUP_DESCS, &group_len);
> +       while (group_buf && off + QUERY_DESC_HDR_SIZE <= group_len) {
> +               const u8 *desc_buf = group_buf + off;
> +               u8 desc_len = desc_buf[QUERY_DESC_LENGTH_OFFSET];
> +
> +               if (desc_len < QUERY_DESC_HDR_SIZE || off + desc_len >
> group_len)
> +                       break;
> +
> +               if (desc_buf[QUERY_DESC_DESC_TYPE_OFFSET] == idn &&
> +                   (idn != QUERY_DESC_IDN_UNIT ||
> +                    (desc_len > UNIT_DESC_PARAM_UNIT_INDEX &&
> +                     desc_buf[UNIT_DESC_PARAM_UNIT_INDEX] == index))) {
> +                       *len = desc_len;
> +                       return desc_buf;
> +               }
> +
> +               off += desc_len;
> +       }
> +
> +       return NULL;
> +}
> +
> +/**
> + * ufshcd_agg_attr - read an attribute from the cached aggregated data packet
> + * @hba: per-adapter instance
> + * @idn: attribute IDN (index 0 only)
> + * @out: set to the value on a hit
> + *
> + * Return: 0 on a hit, -ENOENT otherwise.
> + */
> +static int ufshcd_agg_attr(struct ufs_hba *hba, u8 idn, u32 *out)
> +{
> +       u16 group_len, off = 0;
> +       const u8 *group_buf;
> +       int i;
> +
> +       if (idn >= ARRAY_SIZE(ufs_agg_attr_width) || !ufs_agg_attr_width[idn])
> +               return -ENOENT;
> +
> +       group_buf = ufshcd_agg_group(hba, UFS_AGG_GROUP_ATTRS, &group_len);
> +       if (!group_buf)
> +               return -ENOENT;
> +
> +       for (i = 0; i < idn; i++)
> +               off += ufs_agg_attr_width[i];
> +
> +       if (off + ufs_agg_attr_width[idn] > group_len)
> +               return -ENOENT;
> +
> +       *out = 0;
> +       for (i = 0; i < ufs_agg_attr_width[idn]; i++)
> +               *out = (*out << 8) | group_buf[off + i];
> +
> +       return 0;
> +}
> +
> +/**
> + * ufshcd_agg_flag - read a flag from the cached aggregated data packet
> + * @hba: per-adapter instance
> + * @idn: flag IDN
> + * @out: set to the value on a hit
> + *
> + * Return: 0 on a hit, -ENOENT otherwise.
> + */
> +static int ufshcd_agg_flag(struct ufs_hba *hba, u8 idn, bool *out)
> +{
> +       u16 group_len;
> +       const u8 *group_buf = ufshcd_agg_group(hba, UFS_AGG_GROUP_FLAGS,
> +                                              &group_len);
> +
> +       if (!group_buf || idn >= group_len)
> +               return -ENOENT;
> +
> +       *out = group_buf[idn];
> +       return 0;
> +}
> +
> +/**
> + * ufshcd_read_attr - read an attribute, from the cached packet if present
> + * @hba: per-adapter instance
> + * @idn: attribute IDN
> + * @index: attribute index
> + * @out: result
> + *
> + * Return: 0 on success, < 0 on error.
> + */
> +static int ufshcd_read_attr(struct ufs_hba *hba, u8 idn, u8 index, u32 *out)
> +{
> +       if (index == 0 && !ufshcd_agg_attr(hba, idn, out))
> +               return 0;
> +
> +       return ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_READ_ATTR, idn,
> +                                      index, 0, out);
> +}
> +
> +/**
> + * ufshcd_read_flag - read a flag, from the cached packet if present
> + * @hba: per-adapter instance
> + * @idn: flag IDN
> + * @out: result
> + *
> + * Return: 0 on success, < 0 on error.
> + */
> +static int ufshcd_read_flag(struct ufs_hba *hba, u8 idn, bool *out)
> +{
> +       if (!ufshcd_agg_flag(hba, idn, out))
> +               return 0;
> +
> +       return ufshcd_query_flag_retry(hba, UPIU_QUERY_OPCODE_READ_FLAG, idn,
> 0,
> +                                      out);
> +}
> +
> +/**
> + * ufshcd_query_aggregated_read - issue an AGGREGATED READ query
> + * @hba: per-adapter instance
> + * @agg_type: AGGREGATION TYPE mask (OSF) selecting the items to fetch
> + * @buf: buffer for the reply data segment
> + * @buf_len: requested length in, received length out
> + *
> + * Return: 0 on success; > 0 on an OCS error; < 0 otherwise.
> + */
> +static int ufshcd_query_aggregated_read(struct ufs_hba *hba, u8 agg_type,
> +                                       u8 *buf, int *buf_len)
> +{
> +       struct ufs_query_req *request = NULL;
> +       struct ufs_query_res *response = NULL;
> +       int err;
> +
> +       if (*buf_len <= 0 || *buf_len > QUERY_AGGREGATED_MAX_SIZE)
> +               return -EINVAL;
> +
> +       ufshcd_dev_man_lock(hba);
> +       ufshcd_init_query(hba, &request, &response,
> +                         UPIU_QUERY_OPCODE_AGGREGATED_READ, agg_type, 0, 0);
> +       request->query_func = UPIU_QUERY_FUNC_STANDARD_READ_REQUEST;
> +       request->upiu_req.length = cpu_to_be16(*buf_len);
> +       hba->dev_cmd.query.descriptor = buf;
> +
> +       err = ufshcd_exec_dev_cmd(hba, DEV_CMD_TYPE_QUERY, dev_cmd_timeout);
> +       if (!err)
> +               *buf_len = response->data_segment_length;
> +
> +       hba->dev_cmd.query.descriptor = NULL;
> +       ufshcd_dev_man_unlock(hba);
> +       return err;
> +}
> +
> +/**
> + * ufshcd_agg_read_begin - cache one AGGREGATED READ for the reads that
> follow
> + * @hba: per-adapter instance
> + * @agg_type: AGGREGATION TYPE mask to fetch
> + *
> + * Paired with ufshcd_agg_read_end(). Requires a UFS 5.0 device.
> + */
> +static void ufshcd_agg_read_begin(struct ufs_hba *hba, u8 agg_type)
> +{
> +       int len = QUERY_AGGREGATED_MAX_SIZE;
> +       int retries;
> +       u8 *packet;
> +
> +       if (hba->dev_info.wspecversion < 0x500 ||
> +           hba->dev_info.agg_read_unsupported)
> +               return;

is this feature mandatory? or you assume this feature should be supported by
defualt?

  
> +
> +       /* Zeroed so a device over-reporting LENGTH terminates the walk
> safely. */
> +       packet = kzalloc(QUERY_AGGREGATED_MAX_SIZE, GFP_KERNEL);
> +       if (!packet)
> +               return;
> +
> +       for (retries = QUERY_REQ_RETRIES; retries > 0; retries--) {
> +               if (!ufshcd_query_aggregated_read(hba, agg_type, packet,
> &len))
> +                       break;
> +       }
> +
If the device rejects the opcode with a query response error, is there a reason
to retry? Retrying makes sense for a transient error, but not for "invalid
opcode"


Kind regards,
Bean

Re: [PATCH v2 2/2] scsi: ufs: Serve the device init reads from one aggregated read

[Hyeoncheol Jeong]

Hi Bean,

Thank you for the detailed review.

On Fri, 02 Oct 2026 17:50:19 +0200, Bean Huo wrote:
> On Wed, 2026-09-30 at 14:18 +0900, Hyeoncheol Jeong wrote:
> > (bWriteBoosterBufferLifeTimeEst is served from the packet only in the
> > shared-buffer mode)
>
> I suggest we should aggregate-read as many of them as possible. If we lose even
> one descriptor, this feature becomes useless.

You are right. As this is the first step, I took a conservative approach
and left out the reads that are issued asynchronously, such as the power
and unit descriptors. I agree that covering as many of the init reads as
possible is what makes this worthwhile.

> table 14.28 gives the size of each attribute, but I could not find where Spec
> defines the layout of the attributes group in the aggregated data packet,
> section 10.7.9.14 only defines the group header.
>
> dDynCapNeeded (09h) is an array attribute; its number of indexes is MaxNumberLU.
> 1Ch-1Fh can also be per-LU in dedicated WB mode. If a device sends one entry per
> index, or keeps a slot for 01h or 11h-13h, every offset after it moves. Then
> bMaxNumOfRTT, bRefClkGatingWaitTime and bWriteBoosterBufferLifeTimeEst are read
> from the wrong bytes, with no error and no fallback to a single query.
> ufshcd_set_rtt() then writes a value based on that wrong data.
>
> Has this been tested on more than one vendor's UFS 5.0 device?

You are right, and thank you for the examples. It has been tested
only on a Samsung UFS 5.0 device.

I think this layout needs to be specified in more detail in the
specification. Once it is, I plan to rework the patch and submit it
again.

> if the reply is cut short (the device may return less than requested), a group
> that is complete but its next offset points past the end is rejected before its
> type is checked.

I will fix this when I rework the patch.

> > +       if (hba->dev_info.wspecversion < 0x500 ||
> > +           hba->dev_info.agg_read_unsupported)
> > +               return;
>
> is this feature mandatory? or you assume this feature should be supported by
> defualt?

JESD220H lists AGGREGATED READ in Table 10.32 without marking it
optional, and I could not find a capability bit for it. The patch
therefore assumes a UFS 5.0 device supports it, but falls back to
individual queries if the device rejects it.

> If the device rejects the opcode with a query response error, is there a reason
> to retry? Retrying makes sense for a transient error, but not for "invalid
> opcode"

Agreed. A query response error should not be retried.

As I replied to Stanley, I think consuming these groups should wait
until the JEDEC specification clarifies the intra-group format. In the
next version I will keep only the interface from patch 1 and drop the
users in patch 2, and I will address the points above when they come
back.

Thanks,
Hyeoncheol

Re: [PATCH v2 1/2] scsi: ufs: Add the aggregated read query opcode and its reply format

[Peter Wang (王信友)]

On Wed, 2026-09-30 at 14:15 +0900, Hyeoncheol Jeong wrote:
> UFS 5.0 adds the AGGREGATED READ query opcode (0x9): a single QUERY
> RESPONSE UPIU returns many descriptors, attributes and flags as a
> chain
> of typed groups (JESD220H 10.7.9.14).
> 
> Add the AGGREGATION TYPE mask, the group types and the group header,
> and
> let ufshcd_copy_query_response() copy the reply's data segment like a
> descriptor read, recording its length in struct ufs_query_res.
> 
> Signed-off-by: Hyeoncheol Jeong <hyenc.jeong@samsung.com>

Reviewed-by: Peter Wang <peter.wang@mediatek.com>