[PATCH v4 0/3] i3c: Improve CCC reliability for DesignWare master

[PATCH v4 0/3] i3c: Improve CCC reliability for DesignWare master

[tze.yee.ng]

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

This series improves I3C Common Command Code (CCC) handling for the
DesignWare I3C master used on SoCFPGA platforms, and adds core
infrastructure so the I3C subsystem can validate GET CCC responses and
retry failed Direct GET transfers.

Patch 1/3 adds actual_len to struct i3c_ccc_cmd_payload and has the
DesignWare driver report the number of bytes actually received on
successful GET CCC transfers, without overwriting the caller's
requested buffer size in len.

Patch 2/3 maps DesignWare response-queue errors to I3C M0/M2 in
ccc->err. M2 is reported only when the broadcast address (7'h7E) is not
ACKed. Target-address NACK returns -EIO and is not reported as M2.

Patch 3/3 adds optional_bytes and a per-command retries field, validates
GET payload length generically in i3c_master_send_ccc_cmd_locked(),
and retries failed Direct GET CCCs once by default. GETMRL and GETMXDS
describe their variable-length responses at the call site via
optional_bytes. Other I3C master drivers are updated to set actual_len
on successful GET transfers.

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 (3):
  i3c: master: dw: 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

 drivers/i3c/master.c                   | 92 ++++++++++++++++++++++----
 drivers/i3c/master/adi-i3c-master.c    |  2 +
 drivers/i3c/master/dw-i3c-master.c     | 31 +++++++--
 drivers/i3c/master/i3c-master-cdns.c   |  2 +
 drivers/i3c/master/mipi-i3c-hci/core.c |  5 +-
 drivers/i3c/master/renesas-i3c.c       |  2 +
 drivers/i3c/master/svc-i3c-master.c    |  4 +-
 include/linux/i3c/ccc.h                | 11 ++-
 8 files changed, 127 insertions(+), 22 deletions(-)

-- 
2.43.7

[PATCH v4 1/3] i3c: master: dw: Report actual GET CCC payload length on success

[tze.yee.ng]

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

On successful GET CCC transfers, set dests[0].payload.actual_len from
RESPONSE_PORT_DATA_LEN so the I3C core receives the number of bytes
actually read. Add actual_len to struct i3c_ccc_cmd_payload to keep
the requested buffer size 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>
---
 drivers/i3c/master/dw-i3c-master.c | 5 ++++-
 include/linux/i3c/ccc.h            | 4 +++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
index 655693a2187e..856961da1827 100644
--- a/drivers/i3c/master/dw-i3c-master.c
+++ b/drivers/i3c/master/dw-i3c-master.c
@@ -742,7 +742,10 @@ 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)
+	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/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 v4 2/3] 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 response-queue status to ccc->err:

- RESPONSE_ERROR_IBA_NACK -> I3C_ERROR_M2 (broadcast address not ACKed)
- RESPONSE_ERROR_CRC, RESPONSE_ERROR_PARITY, RESPONSE_ERROR_FRAME and
  RESPONSE_ERROR_TRANSF_ABORT -> I3C_ERROR_M0

Per the I3C spec, M2 is the case where the master does not receive ACK
for the broadcast address (7'h7E). Target-address NACK is not reported
as M2.

Return -EIO for RESPONSE_ERROR_ADDRESS_NACK so bus NACKs are not
reported as -EINVAL.

Reset ccc->err to I3C_ERROR_UNKNOWN before each transfer.

Signed-off-by: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
---
 drivers/i3c/master/dw-i3c-master.c | 32 +++++++++++++++++++++++-------
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
index 856961da1827..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)
@@ -743,10 +760,7 @@ static int dw_i3c_ccc_set(struct dw_i3c_master *master,
 
 	ret = xfer->ret;
 	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;
+	ccc->err = dw_i3c_ccc_map_err(cmd->error);
 
 	return ret;
 }
@@ -756,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;
@@ -783,8 +799,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)
-		ccc->err = I3C_ERROR_M2;
+	cmd = &xfer->cmds[0];
+	ccc->err = dw_i3c_ccc_map_err(cmd->error);
+	if (!ret)
+		ccc->dests[0].payload.actual_len = cmd->rx_len;
 
 	return ret;
 }
-- 
2.43.7

[PATCH v4 3/3] 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 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.

Validate GET payload length in i3c_master_send_ccc_cmd_locked() using
actual_len and optional_bytes. Retry failed Direct GET CCCs up to
cmd->retries times (default I3C_CCC_RETRIES) on any error; SET CCCs are
not retried by default.

Add i3c_ccc_cmd_init_retries() and set actual_len in I3C master drivers
on successful GET transfers.

Signed-off-by: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
---
 drivers/i3c/master.c                   | 92 ++++++++++++++++++++++----
 drivers/i3c/master/adi-i3c-master.c    |  2 +
 drivers/i3c/master/i3c-master-cdns.c   |  2 +
 drivers/i3c/master/mipi-i3c-hci/core.c |  5 +-
 drivers/i3c/master/renesas-i3c.c       |  2 +
 drivers/i3c/master/svc-i3c-master.c    |  4 +-
 include/linux/i3c/ccc.h                |  7 ++
 7 files changed, 98 insertions(+), 16 deletions(-)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index 5cd4e5da2233..29dc0793a5a4 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -901,6 +901,8 @@ 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
@@ -914,17 +916,55 @@ 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;
+		u16 min_len;
+
+		if (p->optional_bytes > p->len)
+			return -EINVAL;
+
+		if (p->actual_len > p->len)
+			return -EIO;
+
+		if (!p->len)
+			continue;
+
+		min_len = p->len - p->optional_bytes;
+		if (p->actual_len < min_len ||
+		    (!p->optional_bytes && 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
@@ -936,6 +976,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;
 
@@ -953,7 +996,24 @@ 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->rnw)
+			ret = i3c_ccc_validate_payload_len(cmd);
+		if (!ret && cmd->err == I3C_ERROR_UNKNOWN)
+			break;
+	}
+
+	return ret;
 }
 
 static struct i2c_dev_desc *
@@ -1291,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);
@@ -1302,7 +1366,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 +1401,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;
 	}
@@ -1363,6 +1427,8 @@ 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) {
@@ -1371,19 +1437,21 @@ static int i3c_master_getmxds_locked(struct i3c_master_controller *master,
 		 * while expecting shorter length from this CCC command.
 		 */
 		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)
 			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 +1480,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..64735b488726 100644
--- a/drivers/i3c/master/adi-i3c-master.c
+++ b/drivers/i3c/master/adi-i3c-master.c
@@ -360,6 +360,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 = cmd->dests[0].payload.len;
 
 	return xfer->ret;
 }
diff --git a/drivers/i3c/master/i3c-master-cdns.c b/drivers/i3c/master/i3c-master-cdns.c
index 5cfec6761494..803c27983852 100644
--- a/drivers/i3c/master/i3c-master-cdns.c
+++ b/drivers/i3c/master/i3c-master-cdns.c
@@ -715,6 +715,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 = cmd->dests[0].payload.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..fec614700843 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;
 }
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);
diff --git a/include/linux/i3c/ccc.h b/include/linux/i3c/ccc.h
index d8052949e57e..c8af5db63f7b 100644
--- a/include/linux/i3c/ccc.h
+++ b/include/linux/i3c/ccc.h
@@ -340,16 +340,20 @@ struct i3c_ccc_getxtime {
 	u8 inaccuracy;
 } __packed;
 
+#define I3C_CCC_RETRIES	1
+
 /**
  * struct i3c_ccc_cmd_payload - CCC payload
  *
  * @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;
 };
 
@@ -374,12 +378,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 v4 1/3] i3c: master: dw: Report actual GET CCC payload length on success

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] A pre-existing bug allows a NULL pointer dereference in dw_i3c_master_end_xfer_locked() if a SET CCC command or write transfer returns unread data without an error flag.
--

commit 6edcc13942ab9dc3576c652f10a4cea8ca077f26
Author: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>

i3c: master: dw: Report actual GET CCC payload length on success

This commit updates successful GET CCC transfers to report the actual
number of bytes read by setting dests[0].payload.actual_len from
RESPONSE_PORT_DATA_LEN. It also updates struct i3c_ccc_cmd_payload to
store this actual length alongside the requested buffer size.

> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> index 2f8c0c4683e06..e659ede79fc03 100644
> --- a/drivers/i3c/master/dw-i3c-master.c
> +++ b/drivers/i3c/master/dw-i3c-master.c
> @@ -742,7 +742,10 @@ 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)
> +	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;

[Severity: High]
This is a pre-existing issue, but can this lead to a NULL pointer dereference
during a SET CCC command if the target terminates the transfer early?

If a SET CCC (write) command terminates early, the hardware can populate the
response queue with DATA_LEN > 0 (indicating remaining un-sent bytes) and
ERR_STATUS == 0.

Looking at dw_i3c_master_end_xfer_locked():

    if (cmd->rx_len && !cmd->error)
        dw_i3c_master_read_rx_fifo(master, cmd->rx_buf, cmd->rx_len);

Since cmd->rx_buf is uninitialized (NULL) for write transfers, won't this
unconditionally pass a NULL pointer to the FIFO string-read I/O function
when handling the remaining bytes?

>  
>  	return ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1782824628.git.tze.yee.ng@altera.com?part=1

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Undocumented behavioral change and silent bug fix
--

commit 5afd5fbee8dc926110284bae98310b5970134131
Author: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>

i3c: master: dw: Map CCC hardware errors to I3C M0/M2

This commit updates the DesignWare I3C master driver to map specific
response queue errors to standard I3C error codes M0 and M2. It also returns
-EIO for address NACKs and introduces dw_i3c_ccc_map_err() to centralize
error mapping.

> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> index e659ede79fc03..27d2025fcf041 100644
> --- a/drivers/i3c/master/dw-i3c-master.c
> +++ b/drivers/i3c/master/dw-i3c-master.c

[ ... ]

> @@ -743,10 +760,7 @@ static int dw_i3c_ccc_set(struct dw_i3c_master *master,
>  
>  	ret = xfer->ret;
>  	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;
> +	ccc->err = dw_i3c_ccc_map_err(cmd->error);

[Severity: Medium]
Is this an unrelated bug fix? The commit message mentions mapping hardware
errors to I3C M0/M2, but it does not mention removing the assignment of
actual_len from the transmit operation here.

>  
>  	return ret;
>  }

[ ... ]

> @@ -783,8 +799,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)
> -		ccc->err = I3C_ERROR_M2;
> +	cmd = &xfer->cmds[0];
> +	ccc->err = dw_i3c_ccc_map_err(cmd->error);
> +	if (!ret)
> +		ccc->dests[0].payload.actual_len = cmd->rx_len;

[Severity: Medium]
Does this silently add the actual_len assignment to the receive operation?

Since moving this assignment from dw_i3c_ccc_set() to dw_i3c_ccc_get() is
not documented in the commit message, should it be split into a separate
fix or explicitly explained?

>  
>  	return ret;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1782824628.git.tze.yee.ng@altera.com?part=2

Re: [PATCH v4 3/3] i3c: master: Validate GET CCC payload length and retry Direct GET once

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The Renesas I3C driver fails to set `rx_count` on completion, forcing `actual_len` to 0 and failing all GET CCC validations
- [Low] Dead code in `i3c_master_getstatus_locked()` due to missed update from `len` to `actual_len`
- [Low] The payload validation function evaluates a mathematically impossible and redundant condition
--

commit 3c7b45c2572b29be23202a83e0e0241b82a602bb
Author: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>

i3c: master: Validate GET CCC payload length and retry Direct GET once

This commit adds optional_bytes to struct i3c_ccc_cmd_payload to describe
variable-length GET CCC responses and validates payload lengths in
i3c_master_send_ccc_cmd_locked(). It also retries failed Direct GET CCCs
and updates I3C master drivers to set actual_len on successful GET transfers.

> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index f1be38a640ca1..5741c3f80ed58 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c

[ ... ]

> +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;
> +		u16 min_len;
> +
> +		if (p->optional_bytes > p->len)
> +			return -EINVAL;
> +
> +		if (p->actual_len > p->len)
> +			return -EIO;
> +
> +		if (!p->len)
> +			continue;
> +
> +		min_len = p->len - p->optional_bytes;
> +		if (p->actual_len < min_len ||
> +		    (!p->optional_bytes && p->actual_len != p->len))
> +			return -EIO;
> +	}
> +
> +	return 0;
> +}

[Severity: Low]
Does this evaluate a redundant condition? Since p->actual_len > p->len is
already caught above, when !p->optional_bytes is true, min_len equals
p->len. The left side of the OR captures all cases where actual_len < len.
The right side is only evaluated if actual_len == len, making
actual_len != p->len always false.

> @@ -1409,7 +1473,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;
>  	}

[Severity: Low]
Was i3c_master_getstatus_locked() missed in these updates?

This patch changes drivers to write the received payload length to
actual_len instead of modifying len, but i3c_master_getstatus_locked()
still checks the unmodified length:

drivers/i3c/master.c:i3c_master_getstatus_locked() {
    ...
    if (dest.payload.len != sizeof(*getstatus)) {
        ret = -EIO;
        goto out;
    }
    ...
}

Since len is initialized to sizeof(*getstatus) and is no longer modified
by the drivers, does this make the error path unreachable dead code?

> diff --git a/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
> index f39c449922ca2..fec614700843f 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;
>  }

[Severity: High]
Can this cause all GET CCC operations to fail on this hardware?

This code assigns cmd->rx_count to actual_len, but it appears
cmd->rx_count is never incremented during
I3C_INTERNAL_STATE_CONTROLLER_COMMAND_READ.

Looking at renesas_i3c_resp_isr():

    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 rx_count remains 0, actual_len will always be 0. Will the new
i3c_ccc_validate_payload_len() logic then unconditionally reject the
payload with -EIO due to a short read?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1782824628.git.tze.yee.ng@altera.com?part=3

Re: [PATCH v4 1/3] i3c: master: dw: Report actual GET CCC payload length on success

[Frank Li]

On Tue, Jun 30, 2026 at 06:20:25AM -0700, tze.yee.ng@altera.com wrote:
> From: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
>
> On successful GET CCC transfers, set dests[0].payload.actual_len from
> RESPONSE_PORT_DATA_LEN so the I3C core receives the number of bytes
> actually read. Add actual_len to struct i3c_ccc_cmd_payload to keep
> the requested buffer size 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>
> ---
>  drivers/i3c/master/dw-i3c-master.c | 5 ++++-
>  include/linux/i3c/ccc.h            | 4 +++-

Can you split into two patches, first patch just add actual_len in ccc.h

Frank

>  2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> index 655693a2187e..856961da1827 100644
> --- a/drivers/i3c/master/dw-i3c-master.c
> +++ b/drivers/i3c/master/dw-i3c-master.c
> @@ -742,7 +742,10 @@ 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)
> +	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/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 v4 2/3] i3c: master: dw: Map CCC hardware errors to I3C M0/M2

[Frank Li]

On Tue, Jun 30, 2026 at 01:32:38PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Undocumented behavioral change and silent bug fix
> --
>
> commit 5afd5fbee8dc926110284bae98310b5970134131
> Author: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
>
> i3c: master: dw: Map CCC hardware errors to I3C M0/M2
>
> This commit updates the DesignWare I3C master driver to map specific
> response queue errors to standard I3C error codes M0 and M2. It also returns
> -EIO for address NACKs and introduces dw_i3c_ccc_map_err() to centralize
> error mapping.
>
> > diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> > index e659ede79fc03..27d2025fcf041 100644
> > --- a/drivers/i3c/master/dw-i3c-master.c
> > +++ b/drivers/i3c/master/dw-i3c-master.c
>
> [ ... ]
>
> > @@ -743,10 +760,7 @@ static int dw_i3c_ccc_set(struct dw_i3c_master *master,
> >
> >  	ret = xfer->ret;
> >  	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;
> > +	ccc->err = dw_i3c_ccc_map_err(cmd->error);
>
> [Severity: Medium]
> Is this an unrelated bug fix? The commit message mentions mapping hardware
> errors to I3C M0/M2, but it does not mention removing the assignment of
> actual_len from the transmit operation here.

Yes, don't touch .payload.actual_len in this patch


>
> >
> >  	return ret;
> >  }
>
> [ ... ]
>
> > @@ -783,8 +799,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)
> > -		ccc->err = I3C_ERROR_M2;
> > +	cmd = &xfer->cmds[0];
> > +	ccc->err = dw_i3c_ccc_map_err(cmd->error);
> > +	if (!ret)
> > +		ccc->dests[0].payload.actual_len = cmd->rx_len;
>
> [Severity: Medium]
> Does this silently add the actual_len assignment to the receive operation?

The same here.

update actual_len should be new patch.

Frank

>
> Since moving this assignment from dw_i3c_ccc_set() to dw_i3c_ccc_get() is
> not documented in the commit message, should it be split into a separate
> fix or explicitly explained?
>
> >
> >  	return ret;
> >  }
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/cover.1782824628.git.tze.yee.ng@altera.com?part=2

Re: [PATCH v4 3/3] i3c: master: Validate GET CCC payload length and retry Direct GET once

[Frank Li]

On Tue, Jun 30, 2026 at 06:20:27AM -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.
>
> Validate GET payload length in i3c_master_send_ccc_cmd_locked() using
> actual_len and optional_bytes. Retry failed Direct GET CCCs up to
> cmd->retries times (default I3C_CCC_RETRIES) on any error; SET CCCs are
> not retried by default.
>
> Add i3c_ccc_cmd_init_retries() and set actual_len in I3C master drivers
> on successful GET transfers.
>
> Signed-off-by: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
> Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
> ---
>  drivers/i3c/master.c                   | 92 ++++++++++++++++++++++----
>  drivers/i3c/master/adi-i3c-master.c    |  2 +
>  drivers/i3c/master/i3c-master-cdns.c   |  2 +
>  drivers/i3c/master/mipi-i3c-hci/core.c |  5 +-
>  drivers/i3c/master/renesas-i3c.c       |  2 +
>  drivers/i3c/master/svc-i3c-master.c    |  4 +-
>  include/linux/i3c/ccc.h                |  7 ++

Can you spit ccc.h and master.c to one patch, other driver change to
anthoer patche.

>  7 files changed, 98 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 5cd4e5da2233..29dc0793a5a4 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -901,6 +901,8 @@ 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
> @@ -914,17 +916,55 @@ 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)
> +{
> +	i3c_ccc_cmd_init_retries(cmd, rnw, id, dests, ndests,
> +				 rnw ? I3C_CCC_RETRIES : 0);

why only read need retry?

> +}
> +
...
>
> @@ -953,7 +996,24 @@ 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->rnw)

i3c_ccc_validate_payload_len() already checked cmd->rnw, needn't check here
again.

> +			ret = i3c_ccc_validate_payload_len(cmd);
> +		if (!ret && cmd->err == I3C_ERROR_UNKNOWN)
> +			break;

if i3c_ccc_validate_payload_len() return failure, why need try here.
suppose only need retry when target NACK request.

> +	}
> +
> +	return ret;
>  }
>
>  static struct i2c_dev_desc *
...
> @@ -1363,6 +1427,8 @@ static int i3c_master_getmxds_locked(struct i3c_master_controller *master,
>  	if (!getmaxds)
>  		return -ENOMEM;
>
> +	dest.payload.optional_bytes = 3;
> +

move these set optional_bytes to new patch

>  	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/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
> index f39c449922ca..fec614700843 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;

update actual_len's patch should just flow add field actual_len's patch

>
>  	return ret;
>  }
...
> +
>  /**
>   * struct i3c_ccc_cmd_payload - CCC payload
>   *
>   * @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

					   up to @len - @optional_bytes

Frank
>   * @data: payload data. This buffer must be DMA-able
>   */
>  struct i3c_ccc_cmd_payload {
>  	u16 len;
>  	u16 actual_len;
> +	u16 optional_bytes;
>  	void *data;
>  };
>
> @@ -374,12 +378,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 v4 2/3] i3c: master: dw: Map CCC hardware errors to I3C M0/M2

[NG, TZE YEE]

On 1/7/2026 1:06 am, Frank Li wrote:
> On Tue, Jun 30, 2026 at 01:32:38PM +0000, sashiko-bot@kernel.org wrote:
>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>> - [Medium] Undocumented behavioral change and silent bug fix
>> --
>>
>> commit 5afd5fbee8dc926110284bae98310b5970134131
>> Author: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
>>
>> i3c: master: dw: Map CCC hardware errors to I3C M0/M2
>>
>> This commit updates the DesignWare I3C master driver to map specific
>> response queue errors to standard I3C error codes M0 and M2. It also returns
>> -EIO for address NACKs and introduces dw_i3c_ccc_map_err() to centralize
>> error mapping.
>>
>>> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
>>> index e659ede79fc03..27d2025fcf041 100644
>>> --- a/drivers/i3c/master/dw-i3c-master.c
>>> +++ b/drivers/i3c/master/dw-i3c-master.c
>>
>> [ ... ]
>>
>>> @@ -743,10 +760,7 @@ static int dw_i3c_ccc_set(struct dw_i3c_master *master,
>>>
>>>     ret = xfer->ret;
>>>     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;
>>> +   ccc->err = dw_i3c_ccc_map_err(cmd->error);
>>
>> [Severity: Medium]
>> Is this an unrelated bug fix? The commit message mentions mapping hardware
>> errors to I3C M0/M2, but it does not mention removing the assignment of
>> actual_len from the transmit operation here.
>
> Yes, don't touch .payload.actual_len in this patch
>
>
>>
>>>
>>>     return ret;
>>>   }
>>
>> [ ... ]
>>
>>> @@ -783,8 +799,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)
>>> -           ccc->err = I3C_ERROR_M2;
>>> +   cmd = &xfer->cmds[0];
>>> +   ccc->err = dw_i3c_ccc_map_err(cmd->error);
>>> +   if (!ret)
>>> +           ccc->dests[0].payload.actual_len = cmd->rx_len;
>>
>> [Severity: Medium]
>> Does this silently add the actual_len assignment to the receive operation?
>
> The same here.
>
> update actual_len should be new patch.
>
> Frank
>

Hi Frank,

Thanks for review. I will move all changes to update actual_len into a
new patch.

Tze Yee

>>
>> Since moving this assignment from dw_i3c_ccc_set() to dw_i3c_ccc_get() is
>> not documented in the commit message, should it be split into a separate
>> fix or explicitly explained?
>>
>>>
>>>     return ret;
>>>   }
>>
>> --
>> Sashiko AI review · https://sashiko.dev/#/patchset/cover.1782824628.git.tze.yee.ng@altera.com?part=2

Re: [PATCH v4 1/3] i3c: master: dw: Report actual GET CCC payload length on success

[NG, TZE YEE]

On 1/7/2026 1:03 am, Frank Li wrote:
> On Tue, Jun 30, 2026 at 06:20:25AM -0700, tze.yee.ng@altera.com wrote:
>> From: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
>>
>> On successful GET CCC transfers, set dests[0].payload.actual_len from
>> RESPONSE_PORT_DATA_LEN so the I3C core receives the number of bytes
>> actually read. Add actual_len to struct i3c_ccc_cmd_payload to keep
>> the requested buffer size 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>
>> ---
>>   drivers/i3c/master/dw-i3c-master.c | 5 ++++-
>>   include/linux/i3c/ccc.h            | 4 +++-
> 
> Can you split into two patches, first patch just add actual_len in ccc.h
> 
> Frank
> 

Sure, I will fix it in v5.

Thanks,
Tze Yee

>>   2 files changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
>> index 655693a2187e..856961da1827 100644
>> --- a/drivers/i3c/master/dw-i3c-master.c
>> +++ b/drivers/i3c/master/dw-i3c-master.c
>> @@ -742,7 +742,10 @@ 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)
>> +	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/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 v4 1/3] i3c: master: dw: Report actual GET CCC payload length on success

[Alexandre Mergnat]

On Tue, 30 Jun 2026 06:20:25 -0700, tze.yee.ng@altera.com <tze.yee.ng@altera.com> wrote:
> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> index a7593d6efac5..4a984a5be264 100644
> --- a/drivers/i3c/master/dw-i3c-master.c
> +++ b/drivers/i3c/master/dw-i3c-master.c
> @@ -742,7 +742,10 @@ 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)
> +	cmd = &xfer->cmds[0];
> +	if (!ret)
> +		ccc->dests[0].payload.actual_len = cmd->rx_len;

In v3 this assignment lived in dw_i3c_ccc_get(); in v4 it has moved into
dw_i3c_ccc_set(), the write (SET) path. GET CCCs are dispatched to
dw_i3c_ccc_get() (dw_i3c_master_send_ccc_cmd() calls dw_i3c_ccc_get()
when ccc->rnw is set), so as it stands actual_len is only written for
writes - where cmd->rx_len is the count of un-sent bytes, normally 0 -
and stays 0 for the GET CCCs this commit targets (patch 2 later moves it
back into dw_i3c_ccc_get()). Was there a reason for moving it in v4?
As-is it reads like a regression; keeping it in dw_i3c_ccc_get() would
make the patch match its title and work on its own.

-- 
Alexandre Mergnat <amergnat@baylibre.com>

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

[Alexandre Mergnat]

On Tue, 30 Jun 2026 06:20:26 -0700, tze.yee.ng@altera.com <tze.yee.ng@altera.com> wrote:
> Map DesignWare response-queue status to ccc->err:
> 
> - RESPONSE_ERROR_IBA_NACK -> I3C_ERROR_M2 (broadcast address not ACKed)
> - RESPONSE_ERROR_CRC, RESPONSE_ERROR_PARITY, RESPONSE_ERROR_FRAME and
>   RESPONSE_ERROR_TRANSF_ABORT -> I3C_ERROR_M0
> 
> Per the I3C spec, M2 is the case where the master does not receive ACK
> for the broadcast address (7'h7E). Target-address NACK is not reported
> as M2.
> 
> [...]

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

-- 
Alexandre Mergnat <amergnat@baylibre.com>

Re: [PATCH v4 3/3] i3c: master: Validate GET CCC payload length and retry Direct GET once

[Alexandre Mergnat]

On Tue, 30 Jun 2026 06:20:27 -0700, tze.yee.ng@altera.com <tze.yee.ng@altera.com> wrote:
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 6b8df8089a35..3ad8a232340e 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -914,17 +916,55 @@ static void i3c_ccc_cmd_dest_cleanup(struct i3c_ccc_cmd_dest *dest)
> [ ... skip 44 lines ... ]
> +			continue;
> +
> +		min_len = p->len - p->optional_bytes;
> +		if (p->actual_len < min_len ||
> +		    (!p->optional_bytes && p->actual_len != p->len))
> +			return -EIO;

Unless I'm misreading, the second condition looks already covered by the
checks above: when optional_bytes == 0, min_len == len, so
`actual_len < min_len` rejects anything below len while the earlier
`actual_len > len` check rejects anything above, leaving only
actual_len == len. Could `(!p->optional_bytes && p->actual_len != p->len)`
be dropped to simplify? Not a correctness concern.

> @@ -1371,19 +1437,21 @@ static int i3c_master_getmxds_locked(struct i3c_master_controller *master,
>  		 * while expecting shorter length from this CCC command.
>  		 */
>  		dest.payload.len -= 3;

Now that optional_bytes = 3 lets a 2-byte GETMXDS response pass
i3c_ccc_validate_payload_len() directly, this fallback only triggers
when the controller/device returns an error on the longer (5-byte)
request rather than a short read. Is that the intended remaining case? A
one-line note here would help; otherwise the fallback may now be
redundant.

-- 
Alexandre Mergnat <amergnat@baylibre.com>