[PATCH v6 0/5] i3c: Improve CCC reliability with actual_len, validation, and Direct GET retry

[PATCH v6 0/5] i3c: Improve CCC reliability with actual_len, validation, and Direct GET retry

[tze.yee.ng]

From: Tze Yee Ng <tze.yee.ng@altera.com>

This series improves I3C Common Command Code (CCC) handling in the I3C
core and master drivers. It separates requested and received GET payload
lengths, validates GET responses centrally, and retries failed Direct GET
CCCs once by default.

The DesignWare I3C master driver reports standard I3C M0/M2 error codes so
the core can decide when a retry is appropriate. Variable-length GET CCC
rules (GETMRL, GETMXDS) are described by callers via optional_bytes.

Patch 1/5 adds actual_len to struct i3c_ccc_cmd_payload.

Patch 2/5 has all I3C master drivers set actual_len on successful GET
transfers and updates core GET helpers to interpret results from actual_len
instead of len.

Patch 3/5 maps DesignWare CCC hardware errors to I3C M0/M2. M2 is reported
only for broadcast address-header NACK (7'h7E). Target address NACK
returns -EIO.

Patch 4/5 adds retries to struct i3c_ccc_cmd, validates GET payload length
in i3c_master_send_ccc_cmd_locked() after a successful transfer, and
retries failed Direct GET CCCs when the driver reports failure or an I3C
error. Validation failures are not retried.

Patch 5/5 adds optional_bytes so callers describe variable-length GET CCC
responses. GETMRL and GETMXDS set optional_bytes at the call site.

Changes in v6:
- Fix ADI and Cadence GET actual_len: store hardware-reported xfer bytes
  in cmd->rx_len and set actual_len from that, not from payload.len.
- Fix Renesas GET actual_len: drain the response FIFO at rx_buf + rx_count
  in resp_isr, clamp rx_count to data_len before subtracting (avoids
  unsigned underflow when rx_isr reads round up to 32-bit words), and
  report actual_len from cmd->rx_count.
- Fix dw_i3c_ccc_set() to use dw_i3c_ccc_map_err() so SET CCCs get the
  same M0/M2 mapping as GET.

Changes in v5:
- Split the series into five patches for clearer review:
  actual_len API, driver/core adoption, DW error mapping, core validation/
  retry, and optional_bytes.
- Fix regression in v4 patch 1: keep actual_len assignment in
  dw_i3c_ccc_get(), not dw_i3c_ccc_set().
- Move actual_len adoption in drivers/i3c/master.c into patch 2 so patch 2
  matches its title and works on its own.
- Split optional_bytes into its own patch (5/5); patch 4/5 adds strict
  exact-length validation and retry only.
- Do not retry on validation failure; retry only on driver/I3C errors.
- Reset actual_len before each GET retry only when cmd->rnw is set.
- Add @retries kernel-doc for struct i3c_ccc_cmd.
- Simplify i3c_ccc_validate_payload_len() per review (drop redundant
  exact-length check when optional_bytes == 0).
- Clarify GETMXDS fallback comment: with optional_bytes = 3, a 2-byte
  success on the first attempt no longer needs the shorter retry path.
- Add Reviewed-by: Alexandre Mergnat on patch 3/5.

Changes in v4:
- Add actual_len to keep requested and received lengths separate.
- Map M2 only for broadcast-address NACK; not for target-address NACK.
- Replace CCC-ID-specific validation with generic actual_len /
  optional_bytes checks.
- Retry Direct GET CCCs on any error (default once), not only M0/M2.
- Add optional_bytes and cmd->retries to ccc.h; drop req_len and
  payload.len save/restore in the core.
- Update SVC, Cadence, ADI, Renesas, and MIPI HCI master drivers.

Changes in v3:
- In dw_i3c_master_end_xfer_locked(), move RESPONSE_ERROR_ADDRESS_NACK to
  return -EIO.

Changes in v2:
- Split the monolithic patch into three patches (per review feedback).
- Move GET payload validation and CCC retry from the DW driver to
  drivers/i3c/master.c.
- Validate GET CCCs only; drop SET payload-length checks (DW
  RESPONSE_PORT_DATA_LEN is 0 on SET).
- Retry GET CCCs only; do not repeat side-effecting SET CCCs.
- Tighten GETMRL validation to exactly 2 or 3 bytes; add GETMXDS
  2/5-byte handling.
- Expand M0 mapping to CRC/parity/transfer-abort, not only frame
  errors.
- Restore dests[].payload.len before retry and on error return.
- Avoid kmalloc on the common single-destination GET path.


Adrian Ng Ho Yin (5):
  i3c: ccc: Add actual_len to struct i3c_ccc_cmd_payload
  i3c: master: Report actual GET CCC payload length on success
  i3c: master: dw: Map CCC hardware errors to I3C M0/M2
  i3c: master: Validate GET CCC payload length and retry Direct GET once
  i3c: master: Add optional_bytes for variable-length GET CCC validation

 drivers/i3c/master.c                   | 97 ++++++++++++++++++++++----
 drivers/i3c/master/adi-i3c-master.c    |  3 +
 drivers/i3c/master/dw-i3c-master.c     | 31 ++++++--
 drivers/i3c/master/i3c-master-cdns.c   |  3 +
 drivers/i3c/master/mipi-i3c-hci/core.c |  5 +-
 drivers/i3c/master/renesas-i3c.c       | 16 +++--
 drivers/i3c/master/svc-i3c-master.c    |  4 +-
 include/linux/i3c/ccc.h                | 11 ++-
 8 files changed, 142 insertions(+), 28 deletions(-)

-- 
2.43.7

[PATCH v6 1/5] i3c: ccc: Add actual_len to struct i3c_ccc_cmd_payload

[tze.yee.ng]

From: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>

Add actual_len to struct i3c_ccc_cmd_payload so drivers can report how
many bytes were received on a GET CCC without overwriting the requested
buffer length in len.

Signed-off-by: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
---
Changes in v5:
- Split from v4 patch 1/3: ccc.h only; no driver changes in this patch.
---
 include/linux/i3c/ccc.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/linux/i3c/ccc.h b/include/linux/i3c/ccc.h
index ad59a4ae60d1..d8052949e57e 100644
--- a/include/linux/i3c/ccc.h
+++ b/include/linux/i3c/ccc.h
@@ -343,11 +343,13 @@ struct i3c_ccc_getxtime {
 /**
  * struct i3c_ccc_cmd_payload - CCC payload
  *
- * @len: payload length
+ * @len: requested payload length
+ * @actual_len: number of bytes received on a GET CCC (filled by the driver)
  * @data: payload data. This buffer must be DMA-able
  */
 struct i3c_ccc_cmd_payload {
 	u16 len;
+	u16 actual_len;
 	void *data;
 };
 
-- 
2.43.7

[PATCH v6 2/5] i3c: master: Report actual GET CCC payload length on success

[tze.yee.ng]

From: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>

Set dests[].payload.actual_len on successful GET CCC transfers in I3C
master drivers so the core can distinguish requested and received
buffer lengths. Switch core GET helpers to use actual_len instead of
len when interpreting GET CCC results.

Signed-off-by: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
---
Changes in v6:
- Fix ADI and Cadence GET actual_len: store hardware-reported xfer bytes
  in cmd->rx_len and set actual_len from that, not from payload.len.
- Fix Renesas GET actual_len: drain the response FIFO at rx_buf + rx_count
  in resp_isr, clamp rx_count to data_len before subtracting (avoids
  unsigned underflow when rx_isr reads round up to 32-bit words), and
  report actual_len from cmd->rx_count.

Changes in v5:
- New patch scope: all I3C master drivers (adi, dw, cdns, renesas, svc,
  mipi-hci) set actual_len on successful GET transfers.
- dw_i3c_ccc_get() sets actual_len from cmd->rx_len; dw_i3c_ccc_set()
  does not touch actual_len (fixes v4 regression).
- Initialise actual_len to 0 in i3c_ccc_cmd_dest_init().
- Switch core GET helpers (getmrl, getmwl, getmxds, gethdrcap) to use
  actual_len instead of len when interpreting GET results.

Changes in v4:
- Refactor dw_i3c_ccc_map_err() to return enum i3c_error_code; callers
  assign ccc->err.
- Map I3C_ERROR_M2 only for RESPONSE_ERROR_IBA_NACK (broadcast address
  7'h7E not ACKed). Do not report target-address NACK as M2.
- Keep RESPONSE_ERROR_ADDRESS_NACK returning -EIO in
  dw_i3c_master_end_xfer_locked().
---
 drivers/i3c/master.c                   | 11 ++++++-----
 drivers/i3c/master/adi-i3c-master.c    |  3 +++
 drivers/i3c/master/dw-i3c-master.c     |  5 ++++-
 drivers/i3c/master/i3c-master-cdns.c   |  3 +++
 drivers/i3c/master/mipi-i3c-hci/core.c |  5 +++--
 drivers/i3c/master/renesas-i3c.c       | 16 ++++++++++++----
 drivers/i3c/master/svc-i3c-master.c    |  4 ++--
 7 files changed, 33 insertions(+), 14 deletions(-)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index 5cd4e5da2233..037b3b980717 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -901,6 +901,7 @@ static void *i3c_ccc_cmd_dest_init(struct i3c_ccc_cmd_dest *dest, u8 addr,
 {
 	dest->addr = addr;
 	dest->payload.len = payloadlen;
+	dest->payload.actual_len = 0;
 	if (payloadlen)
 		dest->payload.data = kzalloc(payloadlen, GFP_KERNEL);
 	else
@@ -1302,7 +1303,7 @@ static int i3c_master_getmrl_locked(struct i3c_master_controller *master,
 	if (ret)
 		goto out;
 
-	switch (dest.payload.len) {
+	switch (dest.payload.actual_len) {
 	case 3:
 		info->max_ibi_len = mrl->ibi_len;
 		fallthrough;
@@ -1337,7 +1338,7 @@ static int i3c_master_getmwl_locked(struct i3c_master_controller *master,
 	if (ret)
 		goto out;
 
-	if (dest.payload.len != sizeof(*mwl)) {
+	if (dest.payload.actual_len != sizeof(*mwl)) {
 		ret = -EIO;
 		goto out;
 	}
@@ -1376,14 +1377,14 @@ static int i3c_master_getmxds_locked(struct i3c_master_controller *master,
 			goto out;
 	}
 
-	if (dest.payload.len != 2 && dest.payload.len != 5) {
+	if (dest.payload.actual_len != 2 && dest.payload.actual_len != 5) {
 		ret = -EIO;
 		goto out;
 	}
 
 	info->max_read_ds = getmaxds->maxrd;
 	info->max_write_ds = getmaxds->maxwr;
-	if (dest.payload.len == 5)
+	if (dest.payload.actual_len == 5)
 		info->max_read_turnaround = getmaxds->maxrdturn[0] |
 					    ((u32)getmaxds->maxrdturn[1] << 8) |
 					    ((u32)getmaxds->maxrdturn[2] << 16);
@@ -1412,7 +1413,7 @@ static int i3c_master_gethdrcap_locked(struct i3c_master_controller *master,
 	if (ret)
 		goto out;
 
-	if (dest.payload.len != 1) {
+	if (dest.payload.actual_len != 1) {
 		ret = -EIO;
 		goto out;
 	}
diff --git a/drivers/i3c/master/adi-i3c-master.c b/drivers/i3c/master/adi-i3c-master.c
index 047081c9f064..b5ab2e4e5d57 100644
--- a/drivers/i3c/master/adi-i3c-master.c
+++ b/drivers/i3c/master/adi-i3c-master.c
@@ -246,6 +246,7 @@ static void adi_i3c_master_end_xfer_locked(struct adi_i3c_master *master,
 		if (cmd->cmd0 & REG_CMD_FIFO_0_RNW) {
 			rx_len = min_t(u32, REG_CMDR_FIFO_XFER_BYTES(cmdr), cmd->rx_len);
 			adi_i3c_master_rd_from_rx_fifo(master, cmd->rx_buf, rx_len);
+			cmd->rx_len = rx_len;
 		}
 		cmd->error = REG_CMDR_FIFO_ERROR(cmdr);
 	}
@@ -360,6 +361,8 @@ static int adi_i3c_master_send_ccc_cmd(struct i3c_master_controller *m,
 		adi_i3c_master_unqueue_xfer(master, xfer);
 
 	cmd->err = adi_i3c_cmd_get_err(&xfer->cmds[0]);
+	if (!xfer->ret && cmd->rnw)
+		cmd->dests[0].payload.actual_len = ccmd->rx_len;
 
 	return xfer->ret;
 }
diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
index 655693a2187e..6998ffd15f5f 100644
--- a/drivers/i3c/master/dw-i3c-master.c
+++ b/drivers/i3c/master/dw-i3c-master.c
@@ -780,7 +780,10 @@ static int dw_i3c_ccc_get(struct dw_i3c_master *master, struct i3c_ccc_cmd *ccc)
 		dw_i3c_master_dequeue_xfer(master, xfer);
 
 	ret = xfer->ret;
-	if (xfer->cmds[0].error == RESPONSE_ERROR_IBA_NACK)
+	cmd = &xfer->cmds[0];
+	if (!ret)
+		ccc->dests[0].payload.actual_len = cmd->rx_len;
+	if (cmd->error == RESPONSE_ERROR_IBA_NACK)
 		ccc->err = I3C_ERROR_M2;
 
 	return ret;
diff --git a/drivers/i3c/master/i3c-master-cdns.c b/drivers/i3c/master/i3c-master-cdns.c
index 5cfec6761494..cd6adbcecc26 100644
--- a/drivers/i3c/master/i3c-master-cdns.c
+++ b/drivers/i3c/master/i3c-master-cdns.c
@@ -574,6 +574,7 @@ static void cdns_i3c_master_end_xfer_locked(struct cdns_i3c_master *master,
 		cmd = &xfer->cmds[CMDR_CMDID(cmdr)];
 		rx_len = min_t(u32, CMDR_XFER_BYTES(cmdr), cmd->rx_len);
 		cdns_i3c_master_rd_from_rx_fifo(master, cmd->rx_buf, rx_len);
+		cmd->rx_len = rx_len;
 		cmd->error = CMDR_ERROR(cmdr);
 	}
 
@@ -715,6 +716,8 @@ static int cdns_i3c_master_send_ccc_cmd(struct i3c_master_controller *m,
 
 	ret = xfer->ret;
 	cmd->err = cdns_i3c_cmd_get_err(&xfer->cmds[0]);
+	if (!ret && cmd->rnw)
+		cmd->dests[0].payload.actual_len = ccmd->rx_len;
 	cdns_i3c_master_free_xfer(xfer);
 
 	return ret;
diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
index b781dbed2165..2b215658e093 100644
--- a/drivers/i3c/master/mipi-i3c-hci/core.c
+++ b/drivers/i3c/master/mipi-i3c-hci/core.c
@@ -326,7 +326,7 @@ static int i3c_hci_send_ccc_cmd(struct i3c_master_controller *m,
 		goto out;
 	for (i = prefixed; i < nxfers; i++) {
 		if (ccc->rnw)
-			ccc->dests[i - prefixed].payload.len =
+			ccc->dests[i - prefixed].payload.actual_len =
 				RESP_DATA_LENGTH(xfer[i].response);
 		switch (RESP_STATUS(xfer[i].response)) {
 		case RESP_SUCCESS:
@@ -343,7 +343,8 @@ static int i3c_hci_send_ccc_cmd(struct i3c_master_controller *m,
 
 	if (ccc->rnw)
 		dev_dbg(&hci->master.dev, "got: %*ph",
-			ccc->dests[0].payload.len, ccc->dests[0].payload.data);
+			ccc->dests[0].payload.actual_len,
+			ccc->dests[0].payload.data);
 
 out:
 	hci_free_xfer(xfer, nxfers);
diff --git a/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
index f39c449922ca..25a2b2ed618e 100644
--- a/drivers/i3c/master/renesas-i3c.c
+++ b/drivers/i3c/master/renesas-i3c.c
@@ -805,6 +805,8 @@ static int renesas_i3c_send_ccc_cmd(struct i3c_master_controller *m,
 	ret = xfer->ret;
 	if (ret)
 		ccc->err = I3C_ERROR_M2;
+	else if (ccc->rnw)
+		ccc->dests[0].payload.actual_len = cmd->rx_count;
 
 	return ret;
 }
@@ -1072,10 +1074,16 @@ static irqreturn_t renesas_i3c_resp_isr(int irq, void *data)
 			break;
 		case I3C_INTERNAL_STATE_CONTROLLER_READ:
 		case I3C_INTERNAL_STATE_CONTROLLER_COMMAND_READ:
-			if (NDBSTLV0_RDBLV(renesas_readl(i3c->regs, NDBSTLV0)) && !cmd->err)
-				bytes_remaining = data_len - cmd->rx_count;
-
-			i3c_readl_fifo(i3c->regs + NTDTBP0, cmd->rx_buf, bytes_remaining);
+			if (!cmd->err) {
+				u32 rx_count = min(cmd->rx_count, data_len);
+
+				bytes_remaining = data_len - rx_count;
+				if (bytes_remaining)
+					i3c_readl_fifo(i3c->regs + NTDTBP0,
+						       cmd->rx_buf + rx_count,
+						       bytes_remaining);
+				cmd->rx_count = data_len;
+			}
 			renesas_clear_bit(i3c->regs, NTIE, NTIE_RDBFIE0);
 			break;
 		default:
diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
index e2d99a3ac07d..c92d96e6b4d8 100644
--- a/drivers/i3c/master/svc-i3c-master.c
+++ b/drivers/i3c/master/svc-i3c-master.c
@@ -1706,8 +1706,8 @@ static int svc_i3c_master_send_direct_ccc_cmd(struct svc_i3c_master *master,
 		svc_i3c_master_dequeue_xfer(master, xfer);
 	mutex_unlock(&master->lock);
 
-	if (cmd->actual_len != xfer_len)
-		ccc->dests[0].payload.len = cmd->actual_len;
+	if (ccc->rnw)
+		ccc->dests[0].payload.actual_len = cmd->actual_len;
 
 	ret = xfer->ret;
 	svc_i3c_master_free_xfer(xfer);
-- 
2.43.7

[PATCH v6 3/5] i3c: master: dw: Map CCC hardware errors to I3C M0/M2

[tze.yee.ng]

From: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>

Map DesignWare I3C master CCC hardware errors to I3C M0/M2 error codes.
I3C_ERROR_M2 is reported only for broadcast address-header NACK
(RESPONSE_ERROR_IBA_NACK). Target address NACK remains -EIO.

Signed-off-by: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
---
Changes in v6:
- Fix dw_i3c_ccc_set() to use dw_i3c_ccc_map_err() so SET CCCs get the
  same M0/M2 mapping as GET.

Changes in v5:
- Unchanged from v4 patch 2/3.
- Add Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>.
---
 drivers/i3c/master/dw-i3c-master.c | 28 +++++++++++++++++++++++-----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
index 6998ffd15f5f..c70f6876bd38 100644
--- a/drivers/i3c/master/dw-i3c-master.c
+++ b/drivers/i3c/master/dw-i3c-master.c
@@ -493,6 +493,7 @@ static void dw_i3c_master_end_xfer_locked(struct dw_i3c_master *master, u32 isr)
 			break;
 		case RESPONSE_ERROR_PARITY:
 		case RESPONSE_ERROR_IBA_NACK:
+		case RESPONSE_ERROR_ADDRESS_NACK:
 		case RESPONSE_ERROR_TRANSF_ABORT:
 		case RESPONSE_ERROR_CRC:
 		case RESPONSE_ERROR_FRAME:
@@ -502,7 +503,6 @@ static void dw_i3c_master_end_xfer_locked(struct dw_i3c_master *master, u32 isr)
 			ret = -ENOSPC;
 			break;
 		case RESPONSE_ERROR_I2C_W_NACK_ERR:
-		case RESPONSE_ERROR_ADDRESS_NACK:
 		default:
 			ret = -EINVAL;
 			break;
@@ -708,12 +708,29 @@ static void dw_i3c_master_bus_cleanup(struct i3c_master_controller *m)
 	dw_i3c_master_disable(master);
 }
 
+static enum i3c_error_code dw_i3c_ccc_map_err(u8 dw_err)
+{
+	switch (dw_err) {
+	case RESPONSE_ERROR_IBA_NACK:
+		return I3C_ERROR_M2;
+	case RESPONSE_ERROR_CRC:
+	case RESPONSE_ERROR_PARITY:
+	case RESPONSE_ERROR_FRAME:
+	case RESPONSE_ERROR_TRANSF_ABORT:
+		return I3C_ERROR_M0;
+	default:
+		return I3C_ERROR_UNKNOWN;
+	}
+}
+
 static int dw_i3c_ccc_set(struct dw_i3c_master *master,
 			  struct i3c_ccc_cmd *ccc)
 {
 	struct dw_i3c_cmd *cmd;
 	int ret, pos = 0;
 
+	ccc->err = I3C_ERROR_UNKNOWN;
+
 	if (ccc->id & I3C_CCC_DIRECT) {
 		pos = dw_i3c_master_get_addr_pos(master, ccc->dests[0].addr);
 		if (pos < 0)
@@ -742,8 +759,8 @@ static int dw_i3c_ccc_set(struct dw_i3c_master *master,
 		dw_i3c_master_dequeue_xfer(master, xfer);
 
 	ret = xfer->ret;
-	if (xfer->cmds[0].error == RESPONSE_ERROR_IBA_NACK)
-		ccc->err = I3C_ERROR_M2;
+	cmd = &xfer->cmds[0];
+	ccc->err = dw_i3c_ccc_map_err(cmd->error);
 
 	return ret;
 }
@@ -753,6 +770,8 @@ static int dw_i3c_ccc_get(struct dw_i3c_master *master, struct i3c_ccc_cmd *ccc)
 	struct dw_i3c_cmd *cmd;
 	int ret, pos;
 
+	ccc->err = I3C_ERROR_UNKNOWN;
+
 	pos = dw_i3c_master_get_addr_pos(master, ccc->dests[0].addr);
 	if (pos < 0)
 		return pos;
@@ -781,10 +800,9 @@ static int dw_i3c_ccc_get(struct dw_i3c_master *master, struct i3c_ccc_cmd *ccc)
 
 	ret = xfer->ret;
 	cmd = &xfer->cmds[0];
+	ccc->err = dw_i3c_ccc_map_err(cmd->error);
 	if (!ret)
 		ccc->dests[0].payload.actual_len = cmd->rx_len;
-	if (cmd->error == RESPONSE_ERROR_IBA_NACK)
-		ccc->err = I3C_ERROR_M2;
 
 	return ret;
 }
-- 
2.43.7

[PATCH v6 4/5] i3c: master: Validate GET CCC payload length and retry Direct GET once

[tze.yee.ng]

From: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>

Add retries to struct i3c_ccc_cmd. Validate GET payload length in
i3c_master_send_ccc_cmd_locked() after a successful transfer.

Retry failed Direct GET CCCs up to cmd->retries times when the driver
reports failure or an I3C error; validation failures are not retried.
SET CCCs are not retried by default.

Signed-off-by: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
---
Changes in v5:
- Split from v4 patch 3/3: retry and strict validation only; optional_bytes
  moved to patch 5/5.
- Validate GET payload length after a successful transfer.
- Retry failed Direct GET CCCs when the driver reports failure or an I3C
  error; validation failures are not retried.

Changes in v4:
- Add optional_bytes to struct i3c_ccc_cmd_payload and retries to
  struct i3c_ccc_cmd (default I3C_CCC_RETRIES for GET, 0 for SET).
- Replace CCC-ID-specific payload checks with generic validation using
  actual_len, len, and optional_bytes.
- GETMRL and GETMXDS set optional_bytes at the call site instead of
  hardcoding lengths in the core.
- Retry failed Direct GET CCCs on any error up to cmd->retries times,
  per I3C spec Direct GET single-retry model (§5.1.9.2.3); drop M0/M2-
  gated retry logic.
- Drop req_lens save/restore and the stack/kmalloc bookkeeping for
  payload.len.
- Update I3C master drivers (SVC, Cadence, ADI, Renesas, MIPI HCI) to
  populate actual_len on successful GET transfers.
- Use actual_len in getmrl_locked(), getmwl_locked(), getmxds_locked(),
  and gethdrcap_locked().
---
 drivers/i3c/master.c    | 59 ++++++++++++++++++++++++++++++++++++++---
 include/linux/i3c/ccc.h |  5 ++++
 2 files changed, 60 insertions(+), 4 deletions(-)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index 037b3b980717..3b6d3ecd9d12 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -915,17 +915,46 @@ static void i3c_ccc_cmd_dest_cleanup(struct i3c_ccc_cmd_dest *dest)
 	kfree(dest->payload.data);
 }
 
-static void i3c_ccc_cmd_init(struct i3c_ccc_cmd *cmd, bool rnw, u8 id,
-			     struct i3c_ccc_cmd_dest *dests,
-			     unsigned int ndests)
+static void i3c_ccc_cmd_init_retries(struct i3c_ccc_cmd *cmd, bool rnw, u8 id,
+				     struct i3c_ccc_cmd_dest *dests,
+				     unsigned int ndests, unsigned int retries)
 {
 	cmd->rnw = rnw ? 1 : 0;
 	cmd->id = id;
 	cmd->dests = dests;
 	cmd->ndests = ndests;
+	cmd->retries = retries;
 	cmd->err = I3C_ERROR_UNKNOWN;
 }
 
+static void i3c_ccc_cmd_init(struct i3c_ccc_cmd *cmd, bool rnw, u8 id,
+			     struct i3c_ccc_cmd_dest *dests,
+			     unsigned int ndests)
+{
+	i3c_ccc_cmd_init_retries(cmd, rnw, id, dests, ndests,
+				 rnw ? I3C_CCC_RETRIES : 0);
+}
+
+static int i3c_ccc_validate_payload_len(struct i3c_ccc_cmd *cmd)
+{
+	unsigned int i;
+
+	if (!cmd->rnw)
+		return 0;
+
+	for (i = 0; i < cmd->ndests; i++) {
+		struct i3c_ccc_cmd_payload *p = &cmd->dests[i].payload;
+
+		if (p->actual_len > p->len)
+			return -EIO;
+
+		if (p->len && p->actual_len != p->len)
+			return -EIO;
+	}
+
+	return 0;
+}
+
 /**
  * i3c_master_send_ccc_cmd_locked() - send a CCC (Common Command Codes)
  * @master: master used to send frames on the bus
@@ -937,6 +966,9 @@ static void i3c_ccc_cmd_init(struct i3c_ccc_cmd *cmd, bool rnw, u8 id,
 static int i3c_master_send_ccc_cmd_locked(struct i3c_master_controller *master,
 					  struct i3c_ccc_cmd *cmd)
 {
+	unsigned int attempt, max_attempts;
+	int ret;
+
 	if (!cmd || !master)
 		return -EINVAL;
 
@@ -954,7 +986,25 @@ static int i3c_master_send_ccc_cmd_locked(struct i3c_master_controller *master,
 	    !master->ops->supports_ccc_cmd(master, cmd))
 		return -EOPNOTSUPP;
 
-	return master->ops->send_ccc_cmd(master, cmd);
+	max_attempts = cmd->retries + 1;
+	ret = -EIO;
+	for (attempt = 0; attempt < max_attempts; attempt++) {
+		unsigned int i;
+
+		if (cmd->rnw)
+			for (i = 0; i < cmd->ndests; i++)
+				cmd->dests[i].payload.actual_len = 0;
+
+		cmd->err = I3C_ERROR_UNKNOWN;
+		ret = master->ops->send_ccc_cmd(master, cmd);
+		if (!ret && cmd->err == I3C_ERROR_UNKNOWN)
+			break;
+	}
+
+	if (!ret)
+		ret = i3c_ccc_validate_payload_len(cmd);
+
+	return ret;
 }
 
 static struct i2c_dev_desc *
@@ -1372,6 +1422,7 @@ static int i3c_master_getmxds_locked(struct i3c_master_controller *master,
 		 * while expecting shorter length from this CCC command.
 		 */
 		dest.payload.len -= 3;
+		i3c_ccc_cmd_init(&cmd, true, I3C_CCC_GETMXDS, &dest, 1);
 		ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
 		if (ret)
 			goto out;
diff --git a/include/linux/i3c/ccc.h b/include/linux/i3c/ccc.h
index d8052949e57e..2506d83b8255 100644
--- a/include/linux/i3c/ccc.h
+++ b/include/linux/i3c/ccc.h
@@ -12,6 +12,8 @@
 #include <linux/i3c/device.h>
 
 /* I3C CCC (Common Command Codes) related definitions */
+#define I3C_CCC_RETRIES	1
+
 #define I3C_CCC_DIRECT			BIT(7)
 
 #define I3C_CCC_ID(id, broadcast)	\
@@ -374,12 +376,15 @@ struct i3c_ccc_cmd_dest {
  * @ndests: number of destinations. Should always be one for broadcast commands
  * @dests: array of destinations and associated payload for this CCC. Most of
  *	   the time, only one destination is provided
+ * @retries: number of times to retry a failed Direct GET CCC (see
+ *	     &I3C_CCC_RETRIES)
  * @err: I3C error code
  */
 struct i3c_ccc_cmd {
 	u8 rnw;
 	u8 id;
 	unsigned int ndests;
+	unsigned int retries;
 	struct i3c_ccc_cmd_dest *dests;
 	enum i3c_error_code err;
 };
-- 
2.43.7

[PATCH v6 5/5] i3c: master: Add optional_bytes for variable-length GET CCC validation

[tze.yee.ng]

From: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>

Add optional_bytes to struct i3c_ccc_cmd_payload so callers describe
variable-length GET CCC responses. GETMRL and GETMXDS set optional_bytes
at the call site. Extend i3c_ccc_validate_payload_len() to honour it.

Signed-off-by: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
---
Changes in v5:
- Split from v4 patch 3/3: optional_bytes and variable-length GET rules only.
- Simplify validation: drop redundant exact-length check when
  optional_bytes == 0 (per review).
- Clarify GETMXDS fallback: with optional_bytes = 3, a 2-byte success on
  the first attempt no longer needs the fallback; it remains for when the
  5-byte request fails rather than short-reading.
---
 drivers/i3c/master.c    | 29 +++++++++++++++++++++++------
 include/linux/i3c/ccc.h |  2 ++
 2 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index 3b6d3ecd9d12..12ca7c929ee5 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -902,6 +902,7 @@ static void *i3c_ccc_cmd_dest_init(struct i3c_ccc_cmd_dest *dest, u8 addr,
 	dest->addr = addr;
 	dest->payload.len = payloadlen;
 	dest->payload.actual_len = 0;
+	dest->payload.optional_bytes = 0;
 	if (payloadlen)
 		dest->payload.data = kzalloc(payloadlen, GFP_KERNEL);
 	else
@@ -944,11 +945,19 @@ static int i3c_ccc_validate_payload_len(struct i3c_ccc_cmd *cmd)
 
 	for (i = 0; i < cmd->ndests; i++) {
 		struct i3c_ccc_cmd_payload *p = &cmd->dests[i].payload;
+		u16 min_len;
+
+		if (p->optional_bytes > p->len)
+			return -EINVAL;
 
 		if (p->actual_len > p->len)
 			return -EIO;
 
-		if (p->len && p->actual_len != p->len)
+		if (!p->len)
+			continue;
+
+		min_len = p->len - p->optional_bytes;
+		if (p->actual_len < min_len)
 			return -EIO;
 	}
 
@@ -1342,10 +1351,14 @@ static int i3c_master_getmrl_locked(struct i3c_master_controller *master,
 		return -ENOMEM;
 
 	/*
-	 * When the device does not have IBI payload GETMRL only returns 2
-	 * bytes of data.
+	 * GETMRL returns 2 bytes (max read length) when the device does not
+	 * advertise IBI payload, or 2 or 3 bytes when it does (the optional
+	 * third byte is max IBI length). Use optional_bytes to allow either
+	 * length when IBI payload is supported.
 	 */
-	if (!(info->bcr & I3C_BCR_IBI_PAYLOAD))
+	if (info->bcr & I3C_BCR_IBI_PAYLOAD)
+		dest.payload.optional_bytes = 1;
+	else
 		dest.payload.len -= 1;
 
 	i3c_ccc_cmd_init(&cmd, true, I3C_CCC_GETMRL, &dest, 1);
@@ -1414,14 +1427,18 @@ static int i3c_master_getmxds_locked(struct i3c_master_controller *master,
 	if (!getmaxds)
 		return -ENOMEM;
 
+	dest.payload.optional_bytes = 3;
+
 	i3c_ccc_cmd_init(&cmd, true, I3C_CCC_GETMXDS, &dest, 1);
 	ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
 	if (ret) {
 		/*
-		 * Retry when the device does not support max read turnaround
-		 * while expecting shorter length from this CCC command.
+		 * optional_bytes = 3 accepts a 2-byte response on the first
+		 * attempt, so this fallback runs only when the 5-byte request
+		 * fails rather than returning a short read.
 		 */
 		dest.payload.len -= 3;
+		dest.payload.optional_bytes = 0;
 		i3c_ccc_cmd_init(&cmd, true, I3C_CCC_GETMXDS, &dest, 1);
 		ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
 		if (ret)
diff --git a/include/linux/i3c/ccc.h b/include/linux/i3c/ccc.h
index 2506d83b8255..7ad677baf761 100644
--- a/include/linux/i3c/ccc.h
+++ b/include/linux/i3c/ccc.h
@@ -347,11 +347,13 @@ struct i3c_ccc_getxtime {
  *
  * @len: requested payload length
  * @actual_len: number of bytes received on a GET CCC (filled by the driver)
+ * @optional_bytes: GET CCCs may return up to this many fewer bytes than @len
  * @data: payload data. This buffer must be DMA-able
  */
 struct i3c_ccc_cmd_payload {
 	u16 len;
 	u16 actual_len;
+	u16 optional_bytes;
 	void *data;
 };
 
-- 
2.43.7

Re: [PATCH v6 1/5] i3c: ccc: Add actual_len to struct i3c_ccc_cmd_payload

[Frank Li]

On Wed, Jul 08, 2026 at 12:17:37AM -0700, tze.yee.ng@altera.com wrote:
> From: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
>
> Add actual_len to struct i3c_ccc_cmd_payload so drivers can report how
> many bytes were received on a GET CCC without overwriting the requested
> buffer length in len.
>
> Signed-off-by: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
> Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> Changes in v5:
> - Split from v4 patch 1/3: ccc.h only; no driver changes in this patch.
> ---
>  include/linux/i3c/ccc.h | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/i3c/ccc.h b/include/linux/i3c/ccc.h
> index ad59a4ae60d1..d8052949e57e 100644
> --- a/include/linux/i3c/ccc.h
> +++ b/include/linux/i3c/ccc.h
> @@ -343,11 +343,13 @@ struct i3c_ccc_getxtime {
>  /**
>   * struct i3c_ccc_cmd_payload - CCC payload
>   *
> - * @len: payload length
> + * @len: requested payload length
> + * @actual_len: number of bytes received on a GET CCC (filled by the driver)
>   * @data: payload data. This buffer must be DMA-able
>   */
>  struct i3c_ccc_cmd_payload {
>  	u16 len;
> +	u16 actual_len;
>  	void *data;
>  };
>
> --
> 2.43.7
>
>

Re: [PATCH v6 2/5] i3c: master: Report actual GET CCC payload length on success

[Frank Li]

On Wed, Jul 08, 2026 at 12:17:38AM -0700, tze.yee.ng@altera.com wrote:
> From: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
>
> Set dests[].payload.actual_len on successful GET CCC transfers in I3C
> master drivers so the core can distinguish requested and received
> buffer lengths. Switch core GET helpers to use actual_len instead of
> len when interpreting GET CCC results.
>
> Signed-off-by: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
> Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> Changes in v6:
> - Fix ADI and Cadence GET actual_len: store hardware-reported xfer bytes
>   in cmd->rx_len and set actual_len from that, not from payload.len.
> - Fix Renesas GET actual_len: drain the response FIFO at rx_buf + rx_count
>   in resp_isr, clamp rx_count to data_len before subtracting (avoids
>   unsigned underflow when rx_isr reads round up to 32-bit words), and
>   report actual_len from cmd->rx_count.
>
> Changes in v5:
> - New patch scope: all I3C master drivers (adi, dw, cdns, renesas, svc,
>   mipi-hci) set actual_len on successful GET transfers.
> - dw_i3c_ccc_get() sets actual_len from cmd->rx_len; dw_i3c_ccc_set()
>   does not touch actual_len (fixes v4 regression).
> - Initialise actual_len to 0 in i3c_ccc_cmd_dest_init().
> - Switch core GET helpers (getmrl, getmwl, getmxds, gethdrcap) to use
>   actual_len instead of len when interpreting GET results.
>
> Changes in v4:
> - Refactor dw_i3c_ccc_map_err() to return enum i3c_error_code; callers
>   assign ccc->err.
> - Map I3C_ERROR_M2 only for RESPONSE_ERROR_IBA_NACK (broadcast address
>   7'h7E not ACKed). Do not report target-address NACK as M2.
> - Keep RESPONSE_ERROR_ADDRESS_NACK returning -EIO in
>   dw_i3c_master_end_xfer_locked().
> ---
>  drivers/i3c/master.c                   | 11 ++++++-----
>  drivers/i3c/master/adi-i3c-master.c    |  3 +++
>  drivers/i3c/master/dw-i3c-master.c     |  5 ++++-
>  drivers/i3c/master/i3c-master-cdns.c   |  3 +++
>  drivers/i3c/master/mipi-i3c-hci/core.c |  5 +++--
>  drivers/i3c/master/renesas-i3c.c       | 16 ++++++++++++----
>  drivers/i3c/master/svc-i3c-master.c    |  4 ++--
>  7 files changed, 33 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 5cd4e5da2233..037b3b980717 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -901,6 +901,7 @@ static void *i3c_ccc_cmd_dest_init(struct i3c_ccc_cmd_dest *dest, u8 addr,
>  {
>  	dest->addr = addr;
>  	dest->payload.len = payloadlen;
> +	dest->payload.actual_len = 0;
>  	if (payloadlen)
>  		dest->payload.data = kzalloc(payloadlen, GFP_KERNEL);
>  	else
> @@ -1302,7 +1303,7 @@ static int i3c_master_getmrl_locked(struct i3c_master_controller *master,
>  	if (ret)
>  		goto out;
>
> -	switch (dest.payload.len) {
> +	switch (dest.payload.actual_len) {
>  	case 3:
>  		info->max_ibi_len = mrl->ibi_len;
>  		fallthrough;
> @@ -1337,7 +1338,7 @@ static int i3c_master_getmwl_locked(struct i3c_master_controller *master,
>  	if (ret)
>  		goto out;
>
> -	if (dest.payload.len != sizeof(*mwl)) {
> +	if (dest.payload.actual_len != sizeof(*mwl)) {
>  		ret = -EIO;
>  		goto out;
>  	}
> @@ -1376,14 +1377,14 @@ static int i3c_master_getmxds_locked(struct i3c_master_controller *master,
>  			goto out;
>  	}
>
> -	if (dest.payload.len != 2 && dest.payload.len != 5) {
> +	if (dest.payload.actual_len != 2 && dest.payload.actual_len != 5) {
>  		ret = -EIO;
>  		goto out;
>  	}
>
>  	info->max_read_ds = getmaxds->maxrd;
>  	info->max_write_ds = getmaxds->maxwr;
> -	if (dest.payload.len == 5)
> +	if (dest.payload.actual_len == 5)
>  		info->max_read_turnaround = getmaxds->maxrdturn[0] |
>  					    ((u32)getmaxds->maxrdturn[1] << 8) |
>  					    ((u32)getmaxds->maxrdturn[2] << 16);
> @@ -1412,7 +1413,7 @@ static int i3c_master_gethdrcap_locked(struct i3c_master_controller *master,
>  	if (ret)
>  		goto out;
>
> -	if (dest.payload.len != 1) {
> +	if (dest.payload.actual_len != 1) {
>  		ret = -EIO;
>  		goto out;
>  	}
> diff --git a/drivers/i3c/master/adi-i3c-master.c b/drivers/i3c/master/adi-i3c-master.c
> index 047081c9f064..b5ab2e4e5d57 100644
> --- a/drivers/i3c/master/adi-i3c-master.c
> +++ b/drivers/i3c/master/adi-i3c-master.c
> @@ -246,6 +246,7 @@ static void adi_i3c_master_end_xfer_locked(struct adi_i3c_master *master,
>  		if (cmd->cmd0 & REG_CMD_FIFO_0_RNW) {
>  			rx_len = min_t(u32, REG_CMDR_FIFO_XFER_BYTES(cmdr), cmd->rx_len);
>  			adi_i3c_master_rd_from_rx_fifo(master, cmd->rx_buf, rx_len);
> +			cmd->rx_len = rx_len;
>  		}
>  		cmd->error = REG_CMDR_FIFO_ERROR(cmdr);
>  	}
> @@ -360,6 +361,8 @@ static int adi_i3c_master_send_ccc_cmd(struct i3c_master_controller *m,
>  		adi_i3c_master_unqueue_xfer(master, xfer);
>
>  	cmd->err = adi_i3c_cmd_get_err(&xfer->cmds[0]);
> +	if (!xfer->ret && cmd->rnw)
> +		cmd->dests[0].payload.actual_len = ccmd->rx_len;
>
>  	return xfer->ret;
>  }
> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> index 655693a2187e..6998ffd15f5f 100644
> --- a/drivers/i3c/master/dw-i3c-master.c
> +++ b/drivers/i3c/master/dw-i3c-master.c
> @@ -780,7 +780,10 @@ static int dw_i3c_ccc_get(struct dw_i3c_master *master, struct i3c_ccc_cmd *ccc)
>  		dw_i3c_master_dequeue_xfer(master, xfer);
>
>  	ret = xfer->ret;
> -	if (xfer->cmds[0].error == RESPONSE_ERROR_IBA_NACK)
> +	cmd = &xfer->cmds[0];
> +	if (!ret)
> +		ccc->dests[0].payload.actual_len = cmd->rx_len;
> +	if (cmd->error == RESPONSE_ERROR_IBA_NACK)
>  		ccc->err = I3C_ERROR_M2;
>
>  	return ret;
> diff --git a/drivers/i3c/master/i3c-master-cdns.c b/drivers/i3c/master/i3c-master-cdns.c
> index 5cfec6761494..cd6adbcecc26 100644
> --- a/drivers/i3c/master/i3c-master-cdns.c
> +++ b/drivers/i3c/master/i3c-master-cdns.c
> @@ -574,6 +574,7 @@ static void cdns_i3c_master_end_xfer_locked(struct cdns_i3c_master *master,
>  		cmd = &xfer->cmds[CMDR_CMDID(cmdr)];
>  		rx_len = min_t(u32, CMDR_XFER_BYTES(cmdr), cmd->rx_len);
>  		cdns_i3c_master_rd_from_rx_fifo(master, cmd->rx_buf, rx_len);
> +		cmd->rx_len = rx_len;
>  		cmd->error = CMDR_ERROR(cmdr);
>  	}
>
> @@ -715,6 +716,8 @@ static int cdns_i3c_master_send_ccc_cmd(struct i3c_master_controller *m,
>
>  	ret = xfer->ret;
>  	cmd->err = cdns_i3c_cmd_get_err(&xfer->cmds[0]);
> +	if (!ret && cmd->rnw)
> +		cmd->dests[0].payload.actual_len = ccmd->rx_len;
>  	cdns_i3c_master_free_xfer(xfer);
>
>  	return ret;
> diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
> index b781dbed2165..2b215658e093 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/core.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/core.c
> @@ -326,7 +326,7 @@ static int i3c_hci_send_ccc_cmd(struct i3c_master_controller *m,
>  		goto out;
>  	for (i = prefixed; i < nxfers; i++) {
>  		if (ccc->rnw)
> -			ccc->dests[i - prefixed].payload.len =
> +			ccc->dests[i - prefixed].payload.actual_len =
>  				RESP_DATA_LENGTH(xfer[i].response);
>  		switch (RESP_STATUS(xfer[i].response)) {
>  		case RESP_SUCCESS:
> @@ -343,7 +343,8 @@ static int i3c_hci_send_ccc_cmd(struct i3c_master_controller *m,
>
>  	if (ccc->rnw)
>  		dev_dbg(&hci->master.dev, "got: %*ph",
> -			ccc->dests[0].payload.len, ccc->dests[0].payload.data);
> +			ccc->dests[0].payload.actual_len,
> +			ccc->dests[0].payload.data);
>
>  out:
>  	hci_free_xfer(xfer, nxfers);
> diff --git a/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
> index f39c449922ca..25a2b2ed618e 100644
> --- a/drivers/i3c/master/renesas-i3c.c
> +++ b/drivers/i3c/master/renesas-i3c.c
> @@ -805,6 +805,8 @@ static int renesas_i3c_send_ccc_cmd(struct i3c_master_controller *m,
>  	ret = xfer->ret;
>  	if (ret)
>  		ccc->err = I3C_ERROR_M2;
> +	else if (ccc->rnw)
> +		ccc->dests[0].payload.actual_len = cmd->rx_count;
>
>  	return ret;
>  }
> @@ -1072,10 +1074,16 @@ static irqreturn_t renesas_i3c_resp_isr(int irq, void *data)
>  			break;
>  		case I3C_INTERNAL_STATE_CONTROLLER_READ:
>  		case I3C_INTERNAL_STATE_CONTROLLER_COMMAND_READ:
> -			if (NDBSTLV0_RDBLV(renesas_readl(i3c->regs, NDBSTLV0)) && !cmd->err)
> -				bytes_remaining = data_len - cmd->rx_count;
> -
> -			i3c_readl_fifo(i3c->regs + NTDTBP0, cmd->rx_buf, bytes_remaining);
> +			if (!cmd->err) {
> +				u32 rx_count = min(cmd->rx_count, data_len);
> +
> +				bytes_remaining = data_len - rx_count;
> +				if (bytes_remaining)
> +					i3c_readl_fifo(i3c->regs + NTDTBP0,
> +						       cmd->rx_buf + rx_count,
> +						       bytes_remaining);
> +				cmd->rx_count = data_len;
> +			}
>  			renesas_clear_bit(i3c->regs, NTIE, NTIE_RDBFIE0);
>  			break;
>  		default:
> diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
> index e2d99a3ac07d..c92d96e6b4d8 100644
> --- a/drivers/i3c/master/svc-i3c-master.c
> +++ b/drivers/i3c/master/svc-i3c-master.c
> @@ -1706,8 +1706,8 @@ static int svc_i3c_master_send_direct_ccc_cmd(struct svc_i3c_master *master,
>  		svc_i3c_master_dequeue_xfer(master, xfer);
>  	mutex_unlock(&master->lock);
>
> -	if (cmd->actual_len != xfer_len)
> -		ccc->dests[0].payload.len = cmd->actual_len;
> +	if (ccc->rnw)
> +		ccc->dests[0].payload.actual_len = cmd->actual_len;
>
>  	ret = xfer->ret;
>  	svc_i3c_master_free_xfer(xfer);
> --
> 2.43.7
>
>

Re: [PATCH v6 3/5] i3c: master: dw: Map CCC hardware errors to I3C M0/M2

[Frank Li]

On Wed, Jul 08, 2026 at 12:17:39AM -0700, tze.yee.ng@altera.com wrote:
> From: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
>
> Map DesignWare I3C master CCC hardware errors to I3C M0/M2 error codes.
> I3C_ERROR_M2 is reported only for broadcast address-header NACK
> (RESPONSE_ERROR_IBA_NACK). Target address NACK remains -EIO.
>
> Signed-off-by: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
> Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
> Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> Changes in v6:
> - Fix dw_i3c_ccc_set() to use dw_i3c_ccc_map_err() so SET CCCs get the
>   same M0/M2 mapping as GET.
>
> Changes in v5:
> - Unchanged from v4 patch 2/3.
> - Add Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>.
> ---
>  drivers/i3c/master/dw-i3c-master.c | 28 +++++++++++++++++++++++-----
>  1 file changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> index 6998ffd15f5f..c70f6876bd38 100644
> --- a/drivers/i3c/master/dw-i3c-master.c
> +++ b/drivers/i3c/master/dw-i3c-master.c
> @@ -493,6 +493,7 @@ static void dw_i3c_master_end_xfer_locked(struct dw_i3c_master *master, u32 isr)
>  			break;
>  		case RESPONSE_ERROR_PARITY:
>  		case RESPONSE_ERROR_IBA_NACK:
> +		case RESPONSE_ERROR_ADDRESS_NACK:
>  		case RESPONSE_ERROR_TRANSF_ABORT:
>  		case RESPONSE_ERROR_CRC:
>  		case RESPONSE_ERROR_FRAME:
> @@ -502,7 +503,6 @@ static void dw_i3c_master_end_xfer_locked(struct dw_i3c_master *master, u32 isr)
>  			ret = -ENOSPC;
>  			break;
>  		case RESPONSE_ERROR_I2C_W_NACK_ERR:
> -		case RESPONSE_ERROR_ADDRESS_NACK:
>  		default:
>  			ret = -EINVAL;
>  			break;
> @@ -708,12 +708,29 @@ static void dw_i3c_master_bus_cleanup(struct i3c_master_controller *m)
>  	dw_i3c_master_disable(master);
>  }
>
> +static enum i3c_error_code dw_i3c_ccc_map_err(u8 dw_err)
> +{
> +	switch (dw_err) {
> +	case RESPONSE_ERROR_IBA_NACK:
> +		return I3C_ERROR_M2;
> +	case RESPONSE_ERROR_CRC:
> +	case RESPONSE_ERROR_PARITY:
> +	case RESPONSE_ERROR_FRAME:
> +	case RESPONSE_ERROR_TRANSF_ABORT:
> +		return I3C_ERROR_M0;
> +	default:
> +		return I3C_ERROR_UNKNOWN;
> +	}
> +}
> +
>  static int dw_i3c_ccc_set(struct dw_i3c_master *master,
>  			  struct i3c_ccc_cmd *ccc)
>  {
>  	struct dw_i3c_cmd *cmd;
>  	int ret, pos = 0;
>
> +	ccc->err = I3C_ERROR_UNKNOWN;
> +
>  	if (ccc->id & I3C_CCC_DIRECT) {
>  		pos = dw_i3c_master_get_addr_pos(master, ccc->dests[0].addr);
>  		if (pos < 0)
> @@ -742,8 +759,8 @@ static int dw_i3c_ccc_set(struct dw_i3c_master *master,
>  		dw_i3c_master_dequeue_xfer(master, xfer);
>
>  	ret = xfer->ret;
> -	if (xfer->cmds[0].error == RESPONSE_ERROR_IBA_NACK)
> -		ccc->err = I3C_ERROR_M2;
> +	cmd = &xfer->cmds[0];
> +	ccc->err = dw_i3c_ccc_map_err(cmd->error);
>
>  	return ret;
>  }
> @@ -753,6 +770,8 @@ static int dw_i3c_ccc_get(struct dw_i3c_master *master, struct i3c_ccc_cmd *ccc)
>  	struct dw_i3c_cmd *cmd;
>  	int ret, pos;
>
> +	ccc->err = I3C_ERROR_UNKNOWN;
> +
>  	pos = dw_i3c_master_get_addr_pos(master, ccc->dests[0].addr);
>  	if (pos < 0)
>  		return pos;
> @@ -781,10 +800,9 @@ static int dw_i3c_ccc_get(struct dw_i3c_master *master, struct i3c_ccc_cmd *ccc)
>
>  	ret = xfer->ret;
>  	cmd = &xfer->cmds[0];
> +	ccc->err = dw_i3c_ccc_map_err(cmd->error);
>  	if (!ret)
>  		ccc->dests[0].payload.actual_len = cmd->rx_len;
> -	if (cmd->error == RESPONSE_ERROR_IBA_NACK)
> -		ccc->err = I3C_ERROR_M2;
>
>  	return ret;
>  }
> --
> 2.43.7
>
>

Re: [PATCH v6 1/5] i3c: ccc: Add actual_len to struct i3c_ccc_cmd_payload

[Alexandre Mergnat]

On Wed, 08 Jul 2026 00:17:37 -0700, tze.yee.ng@altera.com <tze.yee.ng@altera.com> wrote:
> Add actual_len to struct i3c_ccc_cmd_payload so drivers can report how
> many bytes were received on a GET CCC without overwriting the requested
> buffer length in len.

Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>

-- 
Alexandre Mergnat <amergnat@baylibre.com>

Re: [PATCH v6 2/5] i3c: master: Report actual GET CCC payload length on success

[Alexandre Mergnat]

On Wed, 08 Jul 2026 00:17:38 -0700, tze.yee.ng@altera.com <tze.yee.ng@altera.com> wrote:
> Set dests[].payload.actual_len on successful GET CCC transfers in I3C
> master drivers so the core can distinguish requested and received
> buffer lengths. Switch core GET helpers to use actual_len instead of
> len when interpreting GET CCC results.

Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>

-- 
Alexandre Mergnat <amergnat@baylibre.com>

Re: [PATCH v6 4/5] i3c: master: Validate GET CCC payload length and retry Direct GET once

[Frank Li]

On Wed, Jul 08, 2026 at 12:17:40AM -0700, tze.yee.ng@altera.com wrote:
> From: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
>
> Add retries to struct i3c_ccc_cmd. Validate GET payload length in
> i3c_master_send_ccc_cmd_locked() after a successful transfer.
>
> Retry failed Direct GET CCCs up to cmd->retries times when the driver
> reports failure or an I3C error; validation failures are not retried.
> SET CCCs are not retried by default.
>
> Signed-off-by: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
> Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> Changes in v5:
> - Split from v4 patch 3/3: retry and strict validation only; optional_bytes
>   moved to patch 5/5.
> - Validate GET payload length after a successful transfer.
> - Retry failed Direct GET CCCs when the driver reports failure or an I3C
>   error; validation failures are not retried.
>
> Changes in v4:
> - Add optional_bytes to struct i3c_ccc_cmd_payload and retries to
>   struct i3c_ccc_cmd (default I3C_CCC_RETRIES for GET, 0 for SET).
> - Replace CCC-ID-specific payload checks with generic validation using
>   actual_len, len, and optional_bytes.
> - GETMRL and GETMXDS set optional_bytes at the call site instead of
>   hardcoding lengths in the core.
> - Retry failed Direct GET CCCs on any error up to cmd->retries times,
>   per I3C spec Direct GET single-retry model (§5.1.9.2.3); drop M0/M2-
>   gated retry logic.
> - Drop req_lens save/restore and the stack/kmalloc bookkeeping for
>   payload.len.
> - Update I3C master drivers (SVC, Cadence, ADI, Renesas, MIPI HCI) to
>   populate actual_len on successful GET transfers.
> - Use actual_len in getmrl_locked(), getmwl_locked(), getmxds_locked(),
>   and gethdrcap_locked().
> ---
>  drivers/i3c/master.c    | 59 ++++++++++++++++++++++++++++++++++++++---
>  include/linux/i3c/ccc.h |  5 ++++
>  2 files changed, 60 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 037b3b980717..3b6d3ecd9d12 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -915,17 +915,46 @@ static void i3c_ccc_cmd_dest_cleanup(struct i3c_ccc_cmd_dest *dest)
>  	kfree(dest->payload.data);
>  }
>
> -static void i3c_ccc_cmd_init(struct i3c_ccc_cmd *cmd, bool rnw, u8 id,
> -			     struct i3c_ccc_cmd_dest *dests,
> -			     unsigned int ndests)
> +static void i3c_ccc_cmd_init_retries(struct i3c_ccc_cmd *cmd, bool rnw, u8 id,
> +				     struct i3c_ccc_cmd_dest *dests,
> +				     unsigned int ndests, unsigned int retries)
>  {
>  	cmd->rnw = rnw ? 1 : 0;
>  	cmd->id = id;
>  	cmd->dests = dests;
>  	cmd->ndests = ndests;
> +	cmd->retries = retries;
>  	cmd->err = I3C_ERROR_UNKNOWN;
>  }
>
> +static void i3c_ccc_cmd_init(struct i3c_ccc_cmd *cmd, bool rnw, u8 id,
> +			     struct i3c_ccc_cmd_dest *dests,
> +			     unsigned int ndests)
> +{
> +	i3c_ccc_cmd_init_retries(cmd, rnw, id, dests, ndests,
> +				 rnw ? I3C_CCC_RETRIES : 0);
> +}
> +
> +static int i3c_ccc_validate_payload_len(struct i3c_ccc_cmd *cmd)
> +{
> +	unsigned int i;
> +
> +	if (!cmd->rnw)
> +		return 0;
> +
> +	for (i = 0; i < cmd->ndests; i++) {
> +		struct i3c_ccc_cmd_payload *p = &cmd->dests[i].payload;
> +
> +		if (p->actual_len > p->len)
> +			return -EIO;
> +
> +		if (p->len && p->actual_len != p->len)
> +			return -EIO;
> +	}
> +
> +	return 0;
> +}
> +
>  /**
>   * i3c_master_send_ccc_cmd_locked() - send a CCC (Common Command Codes)
>   * @master: master used to send frames on the bus
> @@ -937,6 +966,9 @@ static void i3c_ccc_cmd_init(struct i3c_ccc_cmd *cmd, bool rnw, u8 id,
>  static int i3c_master_send_ccc_cmd_locked(struct i3c_master_controller *master,
>  					  struct i3c_ccc_cmd *cmd)
>  {
> +	unsigned int attempt, max_attempts;
> +	int ret;
> +
>  	if (!cmd || !master)
>  		return -EINVAL;
>
> @@ -954,7 +986,25 @@ static int i3c_master_send_ccc_cmd_locked(struct i3c_master_controller *master,
>  	    !master->ops->supports_ccc_cmd(master, cmd))
>  		return -EOPNOTSUPP;
>
> -	return master->ops->send_ccc_cmd(master, cmd);
> +	max_attempts = cmd->retries + 1;
> +	ret = -EIO;
> +	for (attempt = 0; attempt < max_attempts; attempt++) {
> +		unsigned int i;
> +
> +		if (cmd->rnw)
> +			for (i = 0; i < cmd->ndests; i++)
> +				cmd->dests[i].payload.actual_len = 0;
> +
> +		cmd->err = I3C_ERROR_UNKNOWN;
> +		ret = master->ops->send_ccc_cmd(master, cmd);
> +		if (!ret && cmd->err == I3C_ERROR_UNKNOWN)
> +			break;
> +	}
> +
> +	if (!ret)
> +		ret = i3c_ccc_validate_payload_len(cmd);
> +
> +	return ret;
>
>  static struct i2c_dev_desc *
> @@ -1372,6 +1422,7 @@ static int i3c_master_getmxds_locked(struct i3c_master_controller *master,
>  		 * while expecting shorter length from this CCC command.
>  		 */
>  		dest.payload.len -= 3;
> +		i3c_ccc_cmd_init(&cmd, true, I3C_CCC_GETMXDS, &dest, 1);
>  		ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
>  		if (ret)
>  			goto out;
> diff --git a/include/linux/i3c/ccc.h b/include/linux/i3c/ccc.h
> index d8052949e57e..2506d83b8255 100644
> --- a/include/linux/i3c/ccc.h
> +++ b/include/linux/i3c/ccc.h
> @@ -12,6 +12,8 @@
>  #include <linux/i3c/device.h>
>
>  /* I3C CCC (Common Command Codes) related definitions */
> +#define I3C_CCC_RETRIES	1
> +
>  #define I3C_CCC_DIRECT			BIT(7)
>
>  #define I3C_CCC_ID(id, broadcast)	\
> @@ -374,12 +376,15 @@ struct i3c_ccc_cmd_dest {
>   * @ndests: number of destinations. Should always be one for broadcast commands
>   * @dests: array of destinations and associated payload for this CCC. Most of
>   *	   the time, only one destination is provided
> + * @retries: number of times to retry a failed Direct GET CCC (see
> + *	     &I3C_CCC_RETRIES)
>   * @err: I3C error code
>   */
>  struct i3c_ccc_cmd {
>  	u8 rnw;
>  	u8 id;
>  	unsigned int ndests;
> +	unsigned int retries;
>  	struct i3c_ccc_cmd_dest *dests;
>  	enum i3c_error_code err;
>  };
> --
> 2.43.7
>
>

Re: [PATCH v6 5/5] i3c: master: Add optional_bytes for variable-length GET CCC validation

[Frank Li]

On Wed, Jul 08, 2026 at 12:17:41AM -0700, tze.yee.ng@altera.com wrote:
> From: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
>
> Add optional_bytes to struct i3c_ccc_cmd_payload so callers describe
> variable-length GET CCC responses. GETMRL and GETMXDS set optional_bytes
> at the call site. Extend i3c_ccc_validate_payload_len() to honour it.
>
> Signed-off-by: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
> Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
> ---
> Changes in v5:
> - Split from v4 patch 3/3: optional_bytes and variable-length GET rules only.
> - Simplify validation: drop redundant exact-length check when
>   optional_bytes == 0 (per review).
> - Clarify GETMXDS fallback: with optional_bytes = 3, a 2-byte success on
>   the first attempt no longer needs the fallback; it remains for when the
>   5-byte request fails rather than short-reading.
> ---
>  drivers/i3c/master.c    | 29 +++++++++++++++++++++++------
>  include/linux/i3c/ccc.h |  2 ++
>  2 files changed, 25 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 3b6d3ecd9d12..12ca7c929ee5 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -902,6 +902,7 @@ static void *i3c_ccc_cmd_dest_init(struct i3c_ccc_cmd_dest *dest, u8 addr,
>  	dest->addr = addr;
>  	dest->payload.len = payloadlen;
>  	dest->payload.actual_len = 0;
> +	dest->payload.optional_bytes = 0;
>  	if (payloadlen)
>  		dest->payload.data = kzalloc(payloadlen, GFP_KERNEL);
>  	else
> @@ -944,11 +945,19 @@ static int i3c_ccc_validate_payload_len(struct i3c_ccc_cmd *cmd)
>
>  	for (i = 0; i < cmd->ndests; i++) {
>  		struct i3c_ccc_cmd_payload *p = &cmd->dests[i].payload;
> +		u16 min_len;
> +
> +		if (p->optional_bytes > p->len)
> +			return -EINVAL;
>
>  		if (p->actual_len > p->len)
>  			return -EIO;
>
> -		if (p->len && p->actual_len != p->len)
> +		if (!p->len)
> +			continue;
> +
> +		min_len = p->len - p->optional_bytes;
> +		if (p->actual_len < min_len)
>  			return -EIO;
>  	}
>
> @@ -1342,10 +1351,14 @@ static int i3c_master_getmrl_locked(struct i3c_master_controller *master,
>  		return -ENOMEM;
>
>  	/*
> -	 * When the device does not have IBI payload GETMRL only returns 2
> -	 * bytes of data.
> +	 * GETMRL returns 2 bytes (max read length) when the device does not
> +	 * advertise IBI payload, or 2 or 3 bytes when it does (the optional
> +	 * third byte is max IBI length). Use optional_bytes to allow either
> +	 * length when IBI payload is supported.
>  	 */
> -	if (!(info->bcr & I3C_BCR_IBI_PAYLOAD))
> +	if (info->bcr & I3C_BCR_IBI_PAYLOAD)
> +		dest.payload.optional_bytes = 1;
> +	else
>  		dest.payload.len -= 1;
>
>  	i3c_ccc_cmd_init(&cmd, true, I3C_CCC_GETMRL, &dest, 1);
> @@ -1414,14 +1427,18 @@ static int i3c_master_getmxds_locked(struct i3c_master_controller *master,
>  	if (!getmaxds)
>  		return -ENOMEM;
>
> +	dest.payload.optional_bytes = 3;
> +
>  	i3c_ccc_cmd_init(&cmd, true, I3C_CCC_GETMXDS, &dest, 1);
>  	ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
>  	if (ret) {
>  		/*
> -		 * Retry when the device does not support max read turnaround
> -		 * while expecting shorter length from this CCC command.
> +		 * optional_bytes = 3 accepts a 2-byte response on the first
> +		 * attempt, so this fallback runs only when the 5-byte request
> +		 * fails rather than returning a short read.
>  		 */
>  		dest.payload.len -= 3;
> +		dest.payload.optional_bytes = 0;
>  		i3c_ccc_cmd_init(&cmd, true, I3C_CCC_GETMXDS, &dest, 1);

where use retry != 1 case ? if all is 1, patch 4 is not necessary.

Frank

>  		ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
>  		if (ret)
> diff --git a/include/linux/i3c/ccc.h b/include/linux/i3c/ccc.h
> index 2506d83b8255..7ad677baf761 100644
> --- a/include/linux/i3c/ccc.h
> +++ b/include/linux/i3c/ccc.h
> @@ -347,11 +347,13 @@ struct i3c_ccc_getxtime {
>   *
>   * @len: requested payload length
>   * @actual_len: number of bytes received on a GET CCC (filled by the driver)
> + * @optional_bytes: GET CCCs may return up to this many fewer bytes than @len
>   * @data: payload data. This buffer must be DMA-able
>   */
>  struct i3c_ccc_cmd_payload {
>  	u16 len;
>  	u16 actual_len;
> +	u16 optional_bytes;
>  	void *data;
>  };
>
> --
> 2.43.7
>
>

Re: [PATCH v6 4/5] i3c: master: Validate GET CCC payload length and retry Direct GET once

[Alexandre Mergnat]

On Wed, 08 Jul 2026 00:17:40 -0700, tze.yee.ng@altera.com <tze.yee.ng@altera.com> wrote:
> Add retries to struct i3c_ccc_cmd. Validate GET payload length in
> i3c_master_send_ccc_cmd_locked() after a successful transfer.
> 
> Retry failed Direct GET CCCs up to cmd->retries times when the driver
> reports failure or an I3C error; validation failures are not retried.
> SET CCCs are not retried by default.
> 
> [...]

Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>

-- 
Alexandre Mergnat <amergnat@baylibre.com>

Re: [PATCH v6 5/5] i3c: master: Add optional_bytes for variable-length GET CCC validation

[NG, TZE YEE]

On 8/7/2026 11:01 pm, Frank Li wrote:
> On Wed, Jul 08, 2026 at 12:17:41AM -0700, tze.yee.ng@altera.com wrote:
>> From: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
>>
>> Add optional_bytes to struct i3c_ccc_cmd_payload so callers describe
>> variable-length GET CCC responses. GETMRL and GETMXDS set optional_bytes
>> at the call site. Extend i3c_ccc_validate_payload_len() to honour it.
>>
>> Signed-off-by: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
>> Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
>> ---
>> Changes in v5:
>> - Split from v4 patch 3/3: optional_bytes and variable-length GET rules only.
>> - Simplify validation: drop redundant exact-length check when
>>    optional_bytes == 0 (per review).
>> - Clarify GETMXDS fallback: with optional_bytes = 3, a 2-byte success on
>>    the first attempt no longer needs the fallback; it remains for when the
>>    5-byte request fails rather than short-reading.
>> ---
>>   drivers/i3c/master.c    | 29 +++++++++++++++++++++++------
>>   include/linux/i3c/ccc.h |  2 ++
>>   2 files changed, 25 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
>> index 3b6d3ecd9d12..12ca7c929ee5 100644
>> --- a/drivers/i3c/master.c
>> +++ b/drivers/i3c/master.c
>> @@ -902,6 +902,7 @@ static void *i3c_ccc_cmd_dest_init(struct i3c_ccc_cmd_dest *dest, u8 addr,
>>   	dest->addr = addr;
>>   	dest->payload.len = payloadlen;
>>   	dest->payload.actual_len = 0;
>> +	dest->payload.optional_bytes = 0;
>>   	if (payloadlen)
>>   		dest->payload.data = kzalloc(payloadlen, GFP_KERNEL);
>>   	else
>> @@ -944,11 +945,19 @@ static int i3c_ccc_validate_payload_len(struct i3c_ccc_cmd *cmd)
>>
>>   	for (i = 0; i < cmd->ndests; i++) {
>>   		struct i3c_ccc_cmd_payload *p = &cmd->dests[i].payload;
>> +		u16 min_len;
>> +
>> +		if (p->optional_bytes > p->len)
>> +			return -EINVAL;
>>
>>   		if (p->actual_len > p->len)
>>   			return -EIO;
>>
>> -		if (p->len && p->actual_len != p->len)
>> +		if (!p->len)
>> +			continue;
>> +
>> +		min_len = p->len - p->optional_bytes;
>> +		if (p->actual_len < min_len)
>>   			return -EIO;
>>   	}
>>
>> @@ -1342,10 +1351,14 @@ static int i3c_master_getmrl_locked(struct i3c_master_controller *master,
>>   		return -ENOMEM;
>>
>>   	/*
>> -	 * When the device does not have IBI payload GETMRL only returns 2
>> -	 * bytes of data.
>> +	 * GETMRL returns 2 bytes (max read length) when the device does not
>> +	 * advertise IBI payload, or 2 or 3 bytes when it does (the optional
>> +	 * third byte is max IBI length). Use optional_bytes to allow either
>> +	 * length when IBI payload is supported.
>>   	 */
>> -	if (!(info->bcr & I3C_BCR_IBI_PAYLOAD))
>> +	if (info->bcr & I3C_BCR_IBI_PAYLOAD)
>> +		dest.payload.optional_bytes = 1;
>> +	else
>>   		dest.payload.len -= 1;
>>
>>   	i3c_ccc_cmd_init(&cmd, true, I3C_CCC_GETMRL, &dest, 1);
>> @@ -1414,14 +1427,18 @@ static int i3c_master_getmxds_locked(struct i3c_master_controller *master,
>>   	if (!getmaxds)
>>   		return -ENOMEM;
>>
>> +	dest.payload.optional_bytes = 3;
>> +
>>   	i3c_ccc_cmd_init(&cmd, true, I3C_CCC_GETMXDS, &dest, 1);
>>   	ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
>>   	if (ret) {
>>   		/*
>> -		 * Retry when the device does not support max read turnaround
>> -		 * while expecting shorter length from this CCC command.
>> +		 * optional_bytes = 3 accepts a 2-byte response on the first
>> +		 * attempt, so this fallback runs only when the 5-byte request
>> +		 * fails rather than returning a short read.
>>   		 */
>>   		dest.payload.len -= 3;
>> +		dest.payload.optional_bytes = 0;
>>   		i3c_ccc_cmd_init(&cmd, true, I3C_CCC_GETMXDS, &dest, 1);
> 
> where use retry != 1 case ? if all is 1, patch 4 is not necessary.
> 
> Frank
> 

Hi Frank,

The retry count is not always 1. retries is 1 for GET and 0 for SET (rnw 
? I3C_CCC_RETRIES : 0). max_attempts = retries + 1 gives 2 attempts for 
Direct GET (spec §5.1.9.2.3) and 1 for side-effecting SET CCCs. This 
follows Adrian Hunter’s v4 suggestion in 
https://lore.kernel.org/all/382a7c2d-2e9e-4a83-94d1-b917921d91f1@intel.com/; 
patch 4 is still required for validation and the central retry path, not 
only for the field itself.

Patch 4 is still needed for the central retry path, GET payload 
validation after a successful transfer, and resetting actual_len between 
attempts — not only for exposing retries in struct i3c_ccc_cmd.

Thanks,
Tze Yee

>>   		ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
>>   		if (ret)
>> diff --git a/include/linux/i3c/ccc.h b/include/linux/i3c/ccc.h
>> index 2506d83b8255..7ad677baf761 100644
>> --- a/include/linux/i3c/ccc.h
>> +++ b/include/linux/i3c/ccc.h
>> @@ -347,11 +347,13 @@ struct i3c_ccc_getxtime {
>>    *
>>    * @len: requested payload length
>>    * @actual_len: number of bytes received on a GET CCC (filled by the driver)
>> + * @optional_bytes: GET CCCs may return up to this many fewer bytes than @len
>>    * @data: payload data. This buffer must be DMA-able
>>    */
>>   struct i3c_ccc_cmd_payload {
>>   	u16 len;
>>   	u16 actual_len;
>> +	u16 optional_bytes;
>>   	void *data;
>>   };
>>
>> --
>> 2.43.7
>>
>>

Re: [PATCH v6 5/5] i3c: master: Add optional_bytes for variable-length GET CCC validation

[Frank Li]

On Thu, Jul 09, 2026 at 02:56:37AM +0000, NG, TZE YEE wrote:
> On 8/7/2026 11:01 pm, Frank Li wrote:
> > On Wed, Jul 08, 2026 at 12:17:41AM -0700, tze.yee.ng@altera.com wrote:
> >> From: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
> >>
> >> Add optional_bytes to struct i3c_ccc_cmd_payload so callers describe
> >> variable-length GET CCC responses. GETMRL and GETMXDS set optional_bytes
> >> at the call site. Extend i3c_ccc_validate_payload_len() to honour it.
> >>
> >> Signed-off-by: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
> >> Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
> >> ---
> >> Changes in v5:
> >> - Split from v4 patch 3/3: optional_bytes and variable-length GET rules only.
> >> - Simplify validation: drop redundant exact-length check when
> >>    optional_bytes == 0 (per review).
> >> - Clarify GETMXDS fallback: with optional_bytes = 3, a 2-byte success on
> >>    the first attempt no longer needs the fallback; it remains for when the
> >>    5-byte request fails rather than short-reading.
> >> ---
> >>   drivers/i3c/master.c    | 29 +++++++++++++++++++++++------
> >>   include/linux/i3c/ccc.h |  2 ++
> >>   2 files changed, 25 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> >> index 3b6d3ecd9d12..12ca7c929ee5 100644
> >> --- a/drivers/i3c/master.c
> >> +++ b/drivers/i3c/master.c
> >> @@ -902,6 +902,7 @@ static void *i3c_ccc_cmd_dest_init(struct i3c_ccc_cmd_dest *dest, u8 addr,
> >>   	dest->addr = addr;
> >>   	dest->payload.len = payloadlen;
> >>   	dest->payload.actual_len = 0;
> >> +	dest->payload.optional_bytes = 0;
> >>   	if (payloadlen)
> >>   		dest->payload.data = kzalloc(payloadlen, GFP_KERNEL);
> >>   	else
> >> @@ -944,11 +945,19 @@ static int i3c_ccc_validate_payload_len(struct i3c_ccc_cmd *cmd)
> >>
> >>   	for (i = 0; i < cmd->ndests; i++) {
> >>   		struct i3c_ccc_cmd_payload *p = &cmd->dests[i].payload;
> >> +		u16 min_len;
> >> +
> >> +		if (p->optional_bytes > p->len)
> >> +			return -EINVAL;
> >>
> >>   		if (p->actual_len > p->len)
> >>   			return -EIO;
> >>
> >> -		if (p->len && p->actual_len != p->len)
> >> +		if (!p->len)
> >> +			continue;
> >> +
> >> +		min_len = p->len - p->optional_bytes;
> >> +		if (p->actual_len < min_len)
> >>   			return -EIO;
> >>   	}
> >>
> >> @@ -1342,10 +1351,14 @@ static int i3c_master_getmrl_locked(struct i3c_master_controller *master,
> >>   		return -ENOMEM;
> >>
> >>   	/*
> >> -	 * When the device does not have IBI payload GETMRL only returns 2
> >> -	 * bytes of data.
> >> +	 * GETMRL returns 2 bytes (max read length) when the device does not
> >> +	 * advertise IBI payload, or 2 or 3 bytes when it does (the optional
> >> +	 * third byte is max IBI length). Use optional_bytes to allow either
> >> +	 * length when IBI payload is supported.
> >>   	 */
> >> -	if (!(info->bcr & I3C_BCR_IBI_PAYLOAD))
> >> +	if (info->bcr & I3C_BCR_IBI_PAYLOAD)
> >> +		dest.payload.optional_bytes = 1;
> >> +	else
> >>   		dest.payload.len -= 1;
> >>
> >>   	i3c_ccc_cmd_init(&cmd, true, I3C_CCC_GETMRL, &dest, 1);
> >> @@ -1414,14 +1427,18 @@ static int i3c_master_getmxds_locked(struct i3c_master_controller *master,
> >>   	if (!getmaxds)
> >>   		return -ENOMEM;
> >>
> >> +	dest.payload.optional_bytes = 3;
> >> +
> >>   	i3c_ccc_cmd_init(&cmd, true, I3C_CCC_GETMXDS, &dest, 1);
> >>   	ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
> >>   	if (ret) {
> >>   		/*
> >> -		 * Retry when the device does not support max read turnaround
> >> -		 * while expecting shorter length from this CCC command.
> >> +		 * optional_bytes = 3 accepts a 2-byte response on the first
> >> +		 * attempt, so this fallback runs only when the 5-byte request
> >> +		 * fails rather than returning a short read.
> >>   		 */
> >>   		dest.payload.len -= 3;
> >> +		dest.payload.optional_bytes = 0;
> >>   		i3c_ccc_cmd_init(&cmd, true, I3C_CCC_GETMXDS, &dest, 1);
> >
> > where use retry != 1 case ? if all is 1, patch 4 is not necessary.
> >
> > Frank
> >
>
> Hi Frank,
>
> The retry count is not always 1. retries is 1 for GET and 0 for SET (rnw
> ? I3C_CCC_RETRIES : 0). max_attempts = retries + 1 gives 2 attempts for
> Direct GET (spec §5.1.9.2.3) and 1 for side-effecting SET CCCs. This
> follows Adrian Hunter’s v4 suggestion in
> https://lore.kernel.org/all/382a7c2d-2e9e-4a83-94d1-b917921d91f1@intel.com/;
> patch 4 is still required for validation and the central retry path, not
> only for the field itself.
>
> Patch 4 is still needed for the central retry path, GET payload
> validation after a successful transfer, and resetting actual_len between
> attempts — not only for exposing retries in struct i3c_ccc_cmd.

Okay

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>
> Thanks,
> Tze Yee
>
> >>   		ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
> >>   		if (ret)
> >> diff --git a/include/linux/i3c/ccc.h b/include/linux/i3c/ccc.h
> >> index 2506d83b8255..7ad677baf761 100644
> >> --- a/include/linux/i3c/ccc.h
> >> +++ b/include/linux/i3c/ccc.h
> >> @@ -347,11 +347,13 @@ struct i3c_ccc_getxtime {
> >>    *
> >>    * @len: requested payload length
> >>    * @actual_len: number of bytes received on a GET CCC (filled by the driver)
> >> + * @optional_bytes: GET CCCs may return up to this many fewer bytes than @len
> >>    * @data: payload data. This buffer must be DMA-able
> >>    */
> >>   struct i3c_ccc_cmd_payload {
> >>   	u16 len;
> >>   	u16 actual_len;
> >> +	u16 optional_bytes;
> >>   	void *data;
> >>   };
> >>
> >> --
> >> 2.43.7
> >>
> >>
>

Re: [PATCH v6 5/5] i3c: master: Add optional_bytes for variable-length GET CCC validation

[Alexandre Mergnat]

On Wed, 08 Jul 2026 00:17:41 -0700, tze.yee.ng@altera.com <tze.yee.ng@altera.com> wrote:
> Add optional_bytes to struct i3c_ccc_cmd_payload so callers describe
> variable-length GET CCC responses. GETMRL and GETMXDS set optional_bytes
> at the call site. Extend i3c_ccc_validate_payload_len() to honour it.

Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>

-- 
Alexandre Mergnat <amergnat@baylibre.com>